From 6aab63765f99050b115f0aec3d6350c85e8da946 Mon Sep 17 00:00:00 2001
From: Matt Butcher <matt.butcher@microsoft.com>
Date: Tue, 15 Sep 2020 17:07:54 -0600
Subject: [PATCH] backported fixes from helm3

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
(cherry picked from commit 7c287078c1505fbe662fcb77fcb2b873b01d501f)
---
 pkg/chartutil/requirements.go                 | 13 ++++++++++
 pkg/chartutil/requirements_test.go            | 21 ++++++++++++++++
 pkg/plugin/plugin.go                          | 10 ++++++++
 pkg/plugin/plugin_test.go                     |  1 +
 pkg/plugin/testdata/plugdir/hello2/hello.sh   | 13 ++++++++++
 .../testdata/plugdir/hello2/plugin.yaml       | 11 +++++++++
 pkg/repo/index.go                             | 24 +++++++++++++++++++
 pkg/repo/index_test.go                        | 16 +++++++++++++
 8 files changed, 109 insertions(+)
 create mode 100755 pkg/plugin/testdata/plugdir/hello2/hello.sh
 create mode 100644 pkg/plugin/testdata/plugdir/hello2/plugin.yaml

diff --git a/pkg/chartutil/requirements.go b/pkg/chartutil/requirements.go
index d72aa9da5..0bd16d2eb 100644
--- a/pkg/chartutil/requirements.go
+++ b/pkg/chartutil/requirements.go
@@ -17,7 +17,9 @@ package chartutil
 
 import (
 	"errors"
+	"fmt"
 	"log"
+	"regexp"
 	"strings"
 	"time"
 
@@ -219,6 +221,9 @@ func ProcessRequirementsTags(reqs *Requirements, cvals Values) {
 
 }
 
+// Validate alias names against this regexp
+var aliasRegexp = regexp.MustCompile("^[a-zA-Z0-9-_]+$")
+
 func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Chart {
 	var chartFound chart.Chart
 	for _, existingChart := range charts {
@@ -237,6 +242,11 @@ func getAliasDependency(charts []*chart.Chart, aliasChart *Dependency) *chart.Ch
 		chartFound = *existingChart
 		newMetadata := *existingChart.Metadata
 		if aliasChart.Alias != "" {
+			// Make sure Alias is well-formed
+			if !aliasRegexp.MatchString(aliasChart.Alias) {
+				fmt.Printf("Invalid alias in dependency %q. Skipping.", aliasChart.Name)
+				continue
+			}
 			newMetadata.Name = aliasChart.Alias
 		}
 		chartFound.Metadata = &newMetadata
@@ -286,6 +296,9 @@ func doProcessRequirementsEnabled(c *chart.Chart, v *chart.Config, path string)
 			chartDependencies = append(chartDependencies, chartDependency)
 		}
 		if req.Alias != "" {
+			if !aliasRegexp.MatchString(req.Alias) {
+				return fmt.Errorf("illegal alias name in %q", req.Name)
+			}
 			req.Name = req.Alias
 		}
 	}
diff --git a/pkg/chartutil/requirements_test.go b/pkg/chartutil/requirements_test.go
index 640987fdd..723f9bab3 100644
--- a/pkg/chartutil/requirements_test.go
+++ b/pkg/chartutil/requirements_test.go
@@ -370,11 +370,19 @@ func TestGetAliasDependency(t *testing.T) {
 	}
 
 	// Failure case
+	resetName := req.Dependencies[0].Name
 	req.Dependencies[0].Name = "something-else"
 	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
 		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
 	}
 
+	// Add a bad alias name
+	req.Dependencies[0].Name = resetName
+	req.Dependencies[0].Alias = "$foobar"
+	if aliasChart := getAliasDependency(c.Dependencies, req.Dependencies[0]); aliasChart != nil {
+		t.Fatalf("expected no chart but got %s", aliasChart.Metadata.Name)
+	}
+
 	req.Dependencies[0].Version = "something else which is not in the compatible range"
 	if version.IsCompatibleRange(req.Dependencies[0].Version, aliasChart.Metadata.Version) {
 		t.Fatalf("Dependency chart version which is not in the compatible range should cause a failure other than a success ")
@@ -516,3 +524,16 @@ func TestDependentChartsWithSomeSubchartsSpecifiedInRequirements(t *testing.T) {
 	}
 
 }
+
+func TestAliasRegexp(t *testing.T) {
+	for name, shouldPass := range map[string]bool{
+		"abcdefghijklmnopqrstuvwxyzABCDEFG0987654321_-": true,
+		"$foo":     false,
+		"bar$":     false,
+		"foo\nbar": false,
+	} {
+		if aliasRegexp.MatchString(name) != shouldPass {
+			t.Errorf("name %q failed to pass its test", name)
+		}
+	}
+}
diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go
index 07fcc700a..c4873918a 100644
--- a/pkg/plugin/plugin.go
+++ b/pkg/plugin/plugin.go
@@ -16,6 +16,7 @@ limitations under the License.
 package plugin // import "k8s.io/helm/pkg/plugin"
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -141,13 +142,22 @@ func LoadAll(basedir string) ([]*Plugin, error) {
 		return plugins, nil
 	}
 
+	loaded := map[string]bool{}
 	for _, yaml := range matches {
 		dir := filepath.Dir(yaml)
 		p, err := LoadDir(dir)
+		pname := p.Metadata.Name
 		if err != nil {
 			return plugins, err
 		}
+
+		if _, ok := loaded[pname]; ok {
+			fmt.Fprintf(os.Stderr, "A plugin named %q already exists. Skipping.", pname)
+			continue
+		}
+
 		plugins = append(plugins, p)
+		loaded[pname] = true
 	}
 	return plugins, nil
 }
diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go
index 338d949f8..2d52e2954 100644
--- a/pkg/plugin/plugin_test.go
+++ b/pkg/plugin/plugin_test.go
@@ -137,6 +137,7 @@ func TestLoadAll(t *testing.T) {
 		t.Fatalf("Could not load %q: %s", basedir, err)
 	}
 
+	// This would fail if the duplicate plugin were loaded.
 	if l := len(plugs); l != 3 {
 		t.Fatalf("expected 3 plugins, found %d", l)
 	}
diff --git a/pkg/plugin/testdata/plugdir/hello2/hello.sh b/pkg/plugin/testdata/plugdir/hello2/hello.sh
new file mode 100755
index 000000000..db7c0f54d
--- /dev/null
+++ b/pkg/plugin/testdata/plugdir/hello2/hello.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+
+echo "Hello from a Helm plugin"
+
+echo "PARAMS"
+echo $*
+
+echo "ENVIRONMENT"
+echo $TILLER_HOST
+echo $HELM_HOME
+
+$HELM_BIN --host $TILLER_HOST ls --all
+
diff --git a/pkg/plugin/testdata/plugdir/hello2/plugin.yaml b/pkg/plugin/testdata/plugdir/hello2/plugin.yaml
new file mode 100644
index 000000000..cdb27b291
--- /dev/null
+++ b/pkg/plugin/testdata/plugdir/hello2/plugin.yaml
@@ -0,0 +1,11 @@
+name: "hello"
+version: "0.1.0"
+usage: "usage"
+description: |-
+  description
+command: "$HELM_PLUGIN_SELF/hello.sh"
+useTunnel: true
+ignoreFlags: true
+install: "echo installing..."
+hooks:
+  install: "echo installing..."
diff --git a/pkg/repo/index.go b/pkg/repo/index.go
index 12f3308de..1fcb7d9cb 100644
--- a/pkg/repo/index.go
+++ b/pkg/repo/index.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/Masterminds/semver"
 	"github.com/ghodss/yaml"
+	yaml2 "gopkg.in/yaml.v2"
 
 	"k8s.io/helm/pkg/chartutil"
 	"k8s.io/helm/pkg/proto/hapi/chart"
@@ -83,6 +84,14 @@ type IndexFile struct {
 	PublicKeys []string                 `json:"publicKeys,omitempty"`
 }
 
+// IndexValidation is used to validate the integrity of an index file
+type IndexValidation struct {
+	APIVersion string                 `yaml:"apiVersion"`
+	Generated  time.Time              `yaml:"generated"`
+	Entries    map[string]interface{} `yaml:"entries"`
+	PublicKeys []string               `yaml:"publicKeys,omitempty"`
+}
+
 // NewIndexFile initializes an index.
 func NewIndexFile() *IndexFile {
 	return &IndexFile{
@@ -283,9 +292,14 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) {
 // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails.
 func loadIndex(data []byte) (*IndexFile, error) {
 	i := &IndexFile{}
+	if err := validateIndex(data); err != nil {
+		return i, err
+	}
+
 	if err := yaml.Unmarshal(data, i); err != nil {
 		return i, err
 	}
+
 	i.SortEntries()
 	if i.APIVersion == "" {
 		// When we leave Beta, we should remove legacy support and just
@@ -296,6 +310,16 @@ func loadIndex(data []byte) (*IndexFile, error) {
 	return i, nil
 }
 
+// validateIndex validates that the index is well-formed.
+func validateIndex(data []byte) error {
+	// This is done ONLY for validation. We need to use ghodss/yaml for the actual parsing.
+	validation := &IndexValidation{}
+	if err := yaml2.UnmarshalStrict(data, validation); err != nil {
+		return err
+	}
+	return nil
+}
+
 // unversionedEntry represents a deprecated pre-Alpha.5 format.
 //
 // This will be removed prior to v2.0.0
diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go
index de53efacf..425de70ef 100644
--- a/pkg/repo/index_test.go
+++ b/pkg/repo/index_test.go
@@ -422,3 +422,19 @@ func TestIndexAdd(t *testing.T) {
 		t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0])
 	}
 }
+
+const mockDuplicateIndex = `
+entries:
+  foo: {}
+  bar: {}
+  baz: {}
+  bar: {}
+`
+
+func TestValidateIndex(t *testing.T) {
+	expect := `key "bar" already set in map`
+	err := validateIndex([]byte(mockDuplicateIndex))
+	if strings.Contains(expect, err.Error()) {
+		t.Errorf("Unexpected error: %s", err)
+	}
+}
-- 
GitLab