Merge pull request #8750 from technosophos/fix/helm-create-name

validate the name passed in during helm create

pkg/chartutil/create.go

@@ -21,6 +21,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -30,6 +31,12 @@ import (
 	"helm.sh/helm/v3/pkg/chart/loader"
 )
 
+// chartName is a regular expression for testing the supplied name of a chart.
+// This regular expression is probably stricter than it needs to be. We can relax it
+// somewhat. Newline characters, as well as $, quotes, +, parens, and % are known to be
+// problematic.
+var chartName = regexp.MustCompile("^[a-zA-Z0-9._-]+$")
+
 const (
 	// ChartfileName is the default Chart file name.
 	ChartfileName = "Chart.yaml"
@@ -63,6 +70,10 @@ const (
 	TestConnectionName = TemplatesTestsDir + sep + "test-connection.yaml"
 )
 
+// maxChartNameLength is lower than the limits we know of with certain file systems,
+// and with certain Kubernetes fields.
+const maxChartNameLength = 250
+
 const sep = string(filepath.Separator)
 
 const defaultChartfile = `apiVersion: v2
@@ -522,6 +533,12 @@ func CreateFrom(chartfile *chart.Metadata, dest, src string) error {
 // error. In such a case, this will attempt to clean up by removing the
 // new chart directory.
 func Create(name, dir string) (string, error) {
+
+	// Sanity-check the name of a chart so user doesn't create one that causes problems.
+	if err := validateChartName(name); err != nil {
+		return "", err
+	}
+
 	path, err := filepath.Abs(dir)
 	if err != nil {
 		return path, err
@@ -627,3 +644,13 @@ func writeFile(name string, content []byte) error {
 	}
 	return ioutil.WriteFile(name, content, 0644)
 }
+
+func validateChartName(name string) error {
+	if name == "" || len(name) > maxChartNameLength {
+		return fmt.Errorf("chart name must be between 1 and %d characters", maxChartNameLength)
+	}
+	if !chartName.MatchString(name) {
+		return fmt.Errorf("chart name must match the regular expression %q", chartName.String())
+	}
+	return nil
+}

pkg/chartutil/create_test.go

@@ -117,3 +117,30 @@ func TestCreateFrom(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateChartName(t *testing.T) {
+	for name, shouldPass := range map[string]bool{
+		"":                              false,
+		"abcdefghijklmnopqrstuvwxyz-_.": true,
+		"ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": true,
+		"$hello":                        false,
+		"Hellô":                         false,
+		"he%%o":                         false,
+		"he\nllo":                       false,
+
+		"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"abcdefghijklmnopqrstuvwxyz-_." +
+			"ABCDEFGHIJKLMNOPQRSTUVWXYZ-_.": false,
+	} {
+		if err := validateChartName(name); (err != nil) == shouldPass {
+			t.Errorf("test for %q failed", name)
+		}
+	}
+}