Merge pull request from GHSA-9vp5-m38w-j776

pkg/chart/chart.go

@@ -17,6 +17,7 @@ package chart
 
 import (
 	"path/filepath"
+	"regexp"
 	"strings"
 )
 
@@ -26,6 +27,9 @@ const APIVersionV1 = "v1"
 // APIVersionV2 is the API version number for version 2.
 const APIVersionV2 = "v2"
 
+// aliasNameFormat defines the characters that are legal in an alias name.
+var aliasNameFormat = regexp.MustCompile("^[a-zA-Z0-9_-]+$")
+
 // Chart is a helm package that contains metadata, a default config, zero or more
 // optionally parameterizable templates, and zero or more charts (dependencies).
 type Chart struct {

pkg/chart/errors.go

@@ -15,9 +15,16 @@ limitations under the License.
 
 package chart
 
+import "fmt"
+
 // ValidationError represents a data validation error.
 type ValidationError string
 
 func (v ValidationError) Error() string {
 	return "validation: " + string(v)
 }
+
+// ValidationErrorf takes a message and formatting options and creates a ValidationError
+func ValidationErrorf(msg string, args ...interface{}) ValidationError {
+	return ValidationError(fmt.Sprintf(msg, args...))
+}

pkg/chart/metadata.go

@@ -81,6 +81,15 @@ func (md *Metadata) Validate() error {
 	if !isValidChartType(md.Type) {
 		return ValidationError("chart.metadata.type must be application or library")
 	}
+
+	// Aliases need to be validated here to make sure that the alias name does
+	// not contain any illegal characters.
+	for _, dependency := range md.Dependencies {
+		if err := validateDependency(dependency); err != nil {
+			return err
+		}
+	}
+
 	// TODO validate valid semver here?
 	return nil
 }
@@ -92,3 +101,13 @@ func isValidChartType(in string) bool {
 	}
 	return false
 }
+
+// validateDependency checks for common problems with the dependency datastructure in
+// the chart. This check must be done at load time before the dependency's charts are
+// loaded.
+func validateDependency(dep *Dependency) error {
+	if len(dep.Alias) > 0 && !aliasNameFormat.MatchString(dep.Alias) {
+		return ValidationErrorf("dependency %q has disallowed characters in the alias", dep.Name)
+	}
+	return nil
+}

pkg/chart/metadata_test.go

@@ -48,12 +48,60 @@ func TestValidate(t *testing.T) {
 			&Metadata{Name: "test", APIVersion: "v2", Version: "1.0", Type: "application"},
 			nil,
 		},
+		{
+			&Metadata{
+				Name:       "test",
+				APIVersion: "v2",
+				Version:    "1.0",
+				Type:       "application",
+				Dependencies: []*Dependency{
+					{Name: "dependency", Alias: "legal-alias"},
+				},
+			},
+			nil,
+		},
+		{
+			&Metadata{
+				Name:       "test",
+				APIVersion: "v2",
+				Version:    "1.0",
+				Type:       "application",
+				Dependencies: []*Dependency{
+					{Name: "bad", Alias: "illegal alias"},
+				},
+			},
+			ValidationError("dependency \"bad\" has disallowed characters in the alias"),
+		},
 	}
 
 	for _, tt := range tests {
 		result := tt.md.Validate()
 		if result != tt.err {
-			t.Errorf("expected %s, got %s", tt.err, result)
+			t.Errorf("expected '%s', got '%s'", tt.err, result)
+		}
+	}
+}
+
+func TestValidateDependency(t *testing.T) {
+	dep := &Dependency{
+		Name: "example",
+	}
+	for value, shouldFail := range map[string]bool{
+		"abcdefghijklmenopQRSTUVWXYZ-0123456780_": false,
+		"-okay":      false,
+		"_okay":      false,
+		"- bad":      true,
+		" bad":       true,
+		"bad\nvalue": true,
+		"bad ":       true,
+		"bad$":       true,
+	} {
+		dep.Alias = value
+		res := validateDependency(dep)
+		if res != nil && !shouldFail {
+			t.Errorf("Failed on case %q", dep.Alias)
+		} else if res == nil && shouldFail {
+			t.Errorf("Expected failure for %q", dep.Alias)
 		}
 	}
 }