Skip to content

Commit

Permalink
Print security notice when index is added (#616)
Browse files Browse the repository at this point in the history
* Print security notice when index is added

Only print security notice on install/upgrade when plugin comes from
krew-index

* Add integration tests for security warnings

* Update index add warning

* Code review changes

* Add test for upgrade not showing warning
  • Loading branch information
chriskim06 authored Jun 29, 2020
1 parent 2aa4db6 commit bc5bea2
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 4 deletions.
12 changes: 10 additions & 2 deletions cmd/krew/cmd/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,15 @@ var indexAddCmd = &cobra.Command{
if !indexoperations.IsValidIndexName(name) {
return errInvalidIndexName
}
return indexoperations.AddIndex(paths, name, args[1])
err := indexoperations.AddIndex(paths, name, args[1])
if err != nil {
return err
}
internal.PrintWarning(os.Stderr, `You have added a new index from %q
The plugins in this index are not audited for security by the Krew maintainers.
Install them at your own risk.
`, args[1])
return nil
},
}

Expand All @@ -86,7 +94,7 @@ var indexDeleteCmd = &cobra.Command{
It is only safe to remove indexes without installed plugins. Removing an index
while there are plugins installed will result in an error, unless the --force
option is used ( not recommended).`,
option is used (not recommended).`,

Args: cobra.ExactArgs(1),
RunE: indexDelete,
Expand Down
5 changes: 4 additions & 1 deletion cmd/krew/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"sigs.k8s.io/krew/internal/index/validation"
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/internal/pathutil"
"sigs.k8s.io/krew/pkg/constants"
"sigs.k8s.io/krew/pkg/index"
)

Expand Down Expand Up @@ -173,7 +174,9 @@ Remarks:
output += fmt.Sprintf("Caveats:\n%s\n", indent(plugin.Spec.Caveats))
}
fmt.Fprintln(os.Stderr, indent(output))
internal.PrintSecurityNotice(plugin.Name)
if entry.indexName == constants.DefaultIndexName {
internal.PrintSecurityNotice(plugin.Name)
}
}
if len(failed) > 0 {
return errors.Wrapf(returnErr, "failed to install some plugins: %+v", failed)
Expand Down
5 changes: 4 additions & 1 deletion cmd/krew/cmd/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"sigs.k8s.io/krew/internal/installation"
"sigs.k8s.io/krew/internal/installation/receipt"
"sigs.k8s.io/krew/internal/pathutil"
"sigs.k8s.io/krew/pkg/constants"
)

func init() {
Expand Down Expand Up @@ -104,7 +105,9 @@ kubectl krew upgrade foo bar"`,
return errors.Wrapf(err, "failed to upgrade plugin %q", pluginDisplayName)
}
fmt.Fprintf(os.Stderr, "Upgraded plugin: %s\n", pluginDisplayName)
internal.PrintSecurityNotice(plugin.Name)
if indexName == constants.DefaultIndexName {
internal.PrintSecurityNotice(plugin.Name)
}
}
if nErrors > 0 {
fmt.Fprintf(os.Stderr, "WARNING: Some plugins failed to upgrade, check logs above.\n")
Expand Down
13 changes: 13 additions & 0 deletions integration_test/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@ func TestKrewIndexAddUnsafe(t *testing.T) {
}
}

func TestKrewIndexAddShowsSecurityWarning(t *testing.T) {
skipShort(t)

test, cleanup := NewTest(t)
defer cleanup()

test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithDefaultIndex()
out := string(test.Krew("index", "add", "foo", test.TempDir().Path("index/"+constants.DefaultIndexName)).RunOrFailOutput())
if !strings.Contains(out, "WARNING: You have added a new index") {
t.Errorf("expected output to contain warning when adding custom index: %v", out)
}
}

func TestKrewIndexList(t *testing.T) {
skipShort(t)

Expand Down
13 changes: 13 additions & 0 deletions integration_test/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ func TestKrewInstall_CustomIndex(t *testing.T) {
test.AssertExecutableNotInPATH("kubectl-" + validPlugin2)
}

func TestKrewInstallNoSecurityWarningForCustomIndex(t *testing.T) {
skipShort(t)

test, cleanup := NewTest(t)
defer cleanup()

test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithDefaultIndex().WithCustomIndexFromDefault("foo")
out := string(test.Krew("install", "foo/"+validPlugin).RunOrFailOutput())
if strings.Contains(out, "Run them at your own risk") {
t.Errorf("expected install of custom plugin to not show security warning: %v", out)
}
}

func TestKrewInstall_Manifest(t *testing.T) {
skipShort(t)

Expand Down
17 changes: 17 additions & 0 deletions integration_test/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,23 @@ func TestKrewUpgradePluginsFromCustomIndex(t *testing.T) {
}
}

func TestKrewUpgradeNoSecurityWarningForCustomIndex(t *testing.T) {
skipShort(t)

test, cleanup := NewTest(t)
defer cleanup()

test.WithEnv(constants.EnableMultiIndexSwitch, 1).WithDefaultIndex().WithCustomIndexFromDefault("foo")
test.Krew("install", "foo/"+validPlugin).RunOrFail()

receipt := environment.NewPaths(test.Root()).PluginInstallReceiptPath(validPlugin)
modifyManifestVersion(t, receipt, "v0.0.1")
out := string(test.Krew("upgrade").RunOrFailOutput())
if strings.Contains(out, "Run them at your own risk") {
t.Errorf("expected install of custom plugin to not show security warning: %v", out)
}
}

func TestKrewUpgrade_CannotUseIndexSyntax(t *testing.T) {
skipShort(t)

Expand Down

0 comments on commit bc5bea2

Please sign in to comment.