Merge pull request from GHSA-c52f-pq47-2r9j

validate plugin metadata before loading

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)

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"},

pkg/plugin/plugin.go

@@ -22,9 +22,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"
@@ -120,12 +121,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.

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)

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"

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..."