From b0296c0522e837d65f944beefa3fb64fd08ac304 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Thu, 17 Sep 2020 08:50:21 -0700 Subject: [PATCH] validate plugin metadata before loading Signed-off-by: Matthew Fisher --- pkg/plugin/installer/local_installer_test.go | 2 +- pkg/plugin/installer/vcs_installer_test.go | 2 +- pkg/plugin/plugin.go | 13 +++++++++++-- pkg/plugin/plugin_test.go | 13 ++++++++++--- .../plugdir/bad/duplicate-entries/plugin.yaml | 12 ++++++++++++ .../plugdir/{ => good}/downloader/plugin.yaml | 0 .../testdata/plugdir/{ => good}/echo/plugin.yaml | 0 .../testdata/plugdir/{ => good}/hello/hello.sh | 0 .../testdata/plugdir/{ => good}/hello/plugin.yaml | 1 - 9 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 pkg/plugin/testdata/plugdir/bad/duplicate-entries/plugin.yaml rename pkg/plugin/testdata/plugdir/{ => good}/downloader/plugin.yaml (100%) rename pkg/plugin/testdata/plugdir/{ => good}/echo/plugin.yaml (100%) rename pkg/plugin/testdata/plugdir/{ => good}/hello/hello.sh (100%) rename pkg/plugin/testdata/plugdir/{ => good}/hello/plugin.yaml (86%) diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index fb5fa2675..321d20511 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -48,7 +48,7 @@ func TestLocalInstaller(t *testing.T) { t.Fatal(err) } - source := "../testdata/plugdir/echo" + source := "../testdata/plugdir/good/echo" i, err := NewForSource(source, "", home) if err != nil { t.Errorf("unexpected error: %s", err) diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index 548a7a49a..871e8dd00 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -61,7 +61,7 @@ func TestVCSInstaller(t *testing.T) { } source := "https://github.com/adamreese/helm-env" - testRepoPath, _ := filepath.Abs("../testdata/plugdir/echo") + testRepoPath, _ := filepath.Abs("../testdata/plugdir/good/echo") repo := &testRepo{ local: testRepoPath, tags: []string{"0.1.0", "0.1.1"}, diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 07fcc700a..223e9d48b 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -21,9 +21,10 @@ import ( "path/filepath" "strings" - helm_env "k8s.io/helm/pkg/helm/environment" - "github.com/ghodss/yaml" + yaml2 "gopkg.in/yaml.v2" + + helm_env "k8s.io/helm/pkg/helm/environment" ) const pluginFileName = "plugin.yaml" @@ -119,12 +120,20 @@ func LoadDir(dirname string) (*Plugin, error) { } plug := &Plugin{Dir: dirname} + if err := validateMeta(data); err != nil { + return nil, err + } if err := yaml.Unmarshal(data, &plug.Metadata); err != nil { return nil, err } return plug, nil } +func validateMeta(data []byte) error { + // This is done ONLY for validation. We need to use ghodss/yaml for the actual parsing. + return yaml2.UnmarshalStrict(data, &Metadata{}) +} + // LoadAll loads all plugins found beneath the base directory. // // This scans only one directory level. diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go index 338d949f8..cc73fe0d0 100644 --- a/pkg/plugin/plugin_test.go +++ b/pkg/plugin/plugin_test.go @@ -64,7 +64,7 @@ func TestPrepareCommand(t *testing.T) { } func TestLoadDir(t *testing.T) { - dirname := "testdata/plugdir/hello" + dirname := "testdata/plugdir/good/hello" plug, err := LoadDir(dirname) if err != nil { t.Fatalf("error loading Hello plugin: %s", err) @@ -92,8 +92,15 @@ func TestLoadDir(t *testing.T) { } } +func TestLoadDirDuplicateEntries(t *testing.T) { + dirname := "testdata/plugdir/bad/duplicate-entries" + if _, err := LoadDir(dirname); err == nil { + t.Errorf("successfully loaded plugin with duplicate entries when it should've failed") + } +} + func TestDownloader(t *testing.T) { - dirname := "testdata/plugdir/downloader" + dirname := "testdata/plugdir/good/downloader" plug, err := LoadDir(dirname) if err != nil { t.Fatalf("error loading Hello plugin: %s", err) @@ -131,7 +138,7 @@ func TestLoadAll(t *testing.T) { t.Fatalf("expected empty dir to have 0 plugins") } - basedir := "testdata/plugdir" + basedir := "testdata/plugdir/good" plugs, err := LoadAll(basedir) if err != nil { t.Fatalf("Could not load %q: %s", basedir, err) diff --git a/pkg/plugin/testdata/plugdir/bad/duplicate-entries/plugin.yaml b/pkg/plugin/testdata/plugdir/bad/duplicate-entries/plugin.yaml new file mode 100644 index 000000000..b5fb49ea6 --- /dev/null +++ b/pkg/plugin/testdata/plugdir/bad/duplicate-entries/plugin.yaml @@ -0,0 +1,12 @@ +name: "duplicate-entries" +version: "0.1.0" +usage: "usage" +description: |- + description +command: "echo hello" +useTunnel: true +ignoreFlags: true +hooks: + install: "echo installing..." +hooks: + install: "echo installing something different" diff --git a/pkg/plugin/testdata/plugdir/downloader/plugin.yaml b/pkg/plugin/testdata/plugdir/good/downloader/plugin.yaml similarity index 100% rename from pkg/plugin/testdata/plugdir/downloader/plugin.yaml rename to pkg/plugin/testdata/plugdir/good/downloader/plugin.yaml diff --git a/pkg/plugin/testdata/plugdir/echo/plugin.yaml b/pkg/plugin/testdata/plugdir/good/echo/plugin.yaml similarity index 100% rename from pkg/plugin/testdata/plugdir/echo/plugin.yaml rename to pkg/plugin/testdata/plugdir/good/echo/plugin.yaml diff --git a/pkg/plugin/testdata/plugdir/hello/hello.sh b/pkg/plugin/testdata/plugdir/good/hello/hello.sh similarity index 100% rename from pkg/plugin/testdata/plugdir/hello/hello.sh rename to pkg/plugin/testdata/plugdir/good/hello/hello.sh diff --git a/pkg/plugin/testdata/plugdir/hello/plugin.yaml b/pkg/plugin/testdata/plugdir/good/hello/plugin.yaml similarity index 86% rename from pkg/plugin/testdata/plugdir/hello/plugin.yaml rename to pkg/plugin/testdata/plugdir/good/hello/plugin.yaml index cdb27b291..dbed85378 100644 --- a/pkg/plugin/testdata/plugdir/hello/plugin.yaml +++ b/pkg/plugin/testdata/plugdir/good/hello/plugin.yaml @@ -6,6 +6,5 @@ description: |- command: "$HELM_PLUGIN_SELF/hello.sh" useTunnel: true ignoreFlags: true -install: "echo installing..." hooks: install: "echo installing..."