diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index 97bfc2c0c..a08a97cd1 100644 --- a/pkg/chart/metadata.go +++ b/pkg/chart/metadata.go @@ -16,6 +16,7 @@ limitations under the License. package chart import ( + "path/filepath" "strings" "unicode" @@ -110,6 +111,11 @@ func (md *Metadata) Validate() error { if md.Name == "" { return ValidationError("chart.metadata.name is required") } + + if md.Name != filepath.Base(md.Name) { + return ValidationErrorf("chart.metadata.name %q is invalid", md.Name) + } + if md.Version == "" { return ValidationError("chart.metadata.version is required") } diff --git a/pkg/chart/metadata_test.go b/pkg/chart/metadata_test.go index c8b89ae55..62aea7261 100644 --- a/pkg/chart/metadata_test.go +++ b/pkg/chart/metadata_test.go @@ -40,6 +40,11 @@ func TestValidate(t *testing.T) { &Metadata{APIVersion: "v2", Version: "1.0"}, ValidationError("chart.metadata.name is required"), }, + { + "chart without name", + &Metadata{Name: "../../test", APIVersion: "v2", Version: "1.0"}, + ValidationError("chart.metadata.name \"../../test\" is invalid"), + }, { "chart without version", &Metadata{Name: "test", APIVersion: "v2"}, diff --git a/pkg/chartutil/errors.go b/pkg/chartutil/errors.go index fcdcc27ea..0a4046d2e 100644 --- a/pkg/chartutil/errors.go +++ b/pkg/chartutil/errors.go @@ -33,3 +33,11 @@ type ErrNoValue struct { } func (e ErrNoValue) Error() string { return fmt.Sprintf("%q is not a value", e.Key) } + +type ErrInvalidChartName struct { + Name string +} + +func (e ErrInvalidChartName) Error() string { + return fmt.Sprintf("%q is not a valid chart name", e.Name) +} diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go index 2ce4eddaf..4ee90709c 100644 --- a/pkg/chartutil/save.go +++ b/pkg/chartutil/save.go @@ -39,6 +39,10 @@ var headerBytes = []byte("+aHR0cHM6Ly95b3V0dS5iZS96OVV6MWljandyTQo=") // directory, writing the chart's contents to that subdirectory. func SaveDir(c *chart.Chart, dest string) error { // Create the chart directory + err := validateName(c.Name()) + if err != nil { + return err + } outdir := filepath.Join(dest, c.Name()) if fi, err := os.Stat(outdir); err == nil && !fi.IsDir() { return errors.Errorf("file %s already exists and is not a directory", outdir) @@ -149,6 +153,10 @@ func Save(c *chart.Chart, outDir string) (string, error) { } func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error { + err := validateName(c.Name()) + if err != nil { + return err + } base := filepath.Join(prefix, c.Name()) // Pull out the dependencies of a v1 Chart, since there's no way @@ -242,3 +250,15 @@ func writeToTar(out *tar.Writer, name string, body []byte) error { _, err := out.Write(body) return err } + +// If the name has directory name has characters which would change the location +// they need to be removed. +func validateName(name string) error { + nname := filepath.Base(name) + + if nname != name { + return ErrInvalidChartName{name} + } + + return nil +} diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go index db485c7cb..98b4e641b 100644 --- a/pkg/chartutil/save_test.go +++ b/pkg/chartutil/save_test.go @@ -106,6 +106,24 @@ func TestSave(t *testing.T) { } }) } + + c := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: chart.APIVersionV1, + Name: "../ahab", + Version: "1.2.3", + }, + Lock: &chart.Lock{ + Digest: "testdigest", + }, + Files: []*chart.File{ + {Name: "scheherazade/shahryar.txt", Data: []byte("1,001 Nights")}, + }, + } + _, err := Save(c, tmp) + if err == nil { + t.Fatal("Expected error saving chart with invalid name") + } } // Creates a copy with a different schema; does not modify anything. @@ -232,4 +250,15 @@ func TestSaveDir(t *testing.T) { if len(c2.Files) != 1 || c2.Files[0].Name != c.Files[0].Name { t.Fatal("Files data did not match") } + + tmp2 := t.TempDir() + c.Metadata.Name = "../ahab" + pth := filepath.Join(tmp2, "tmpcharts") + if err := os.MkdirAll(filepath.Join(pth), 0755); err != nil { + t.Fatal(err) + } + + if err := SaveDir(c, pth); err.Error() != "\"../ahab\" is not a valid chart name" { + t.Fatalf("Did not get expected error for chart named %q", c.Name()) + } } diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index f7ab1a568..db2487d16 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -262,6 +262,32 @@ func TestDownloadAll(t *testing.T) { if _, err := os.Stat(filepath.Join(chartPath, "charts", "signtest-0.1.0.tgz")); os.IsNotExist(err) { t.Error(err) } + + // A chart with a bad name like this cannot be loaded and saved. Handling in + // the loading and saving will return an error about the invalid name. In + // this case, the chart needs to be created directly. + badchartyaml := `apiVersion: v2 +description: A Helm chart for Kubernetes +name: ../bad-local-subchart +version: 0.1.0` + if err := os.MkdirAll(filepath.Join(chartPath, "testdata", "bad-local-subchart"), 0755); err != nil { + t.Fatal(err) + } + err = os.WriteFile(filepath.Join(chartPath, "testdata", "bad-local-subchart", "Chart.yaml"), []byte(badchartyaml), 0644) + if err != nil { + t.Fatal(err) + } + + badLocalDep := &chart.Dependency{ + Name: "../bad-local-subchart", + Repository: "file://./testdata/bad-local-subchart", + Version: "0.1.0", + } + + err = m.downloadAll([]*chart.Dependency{badLocalDep}) + if err == nil { + t.Fatal("Expected error for bad dependency name") + } } func TestUpdateBeforeBuild(t *testing.T) { diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 70532ad4f..910602b7d 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -106,6 +106,10 @@ func validateChartName(cf *chart.Metadata) error { if cf.Name == "" { return errors.New("name is required") } + name := filepath.Base(cf.Name) + if name != cf.Name { + return fmt.Errorf("chart name %q is invalid", cf.Name) + } return nil } diff --git a/pkg/lint/rules/chartfile_test.go b/pkg/lint/rules/chartfile_test.go index 087cda047..a06d7dc31 100644 --- a/pkg/lint/rules/chartfile_test.go +++ b/pkg/lint/rules/chartfile_test.go @@ -30,16 +30,19 @@ import ( ) const ( + badCharNametDir = "testdata/badchartname" badChartDir = "testdata/badchartfile" anotherBadChartDir = "testdata/anotherbadchartfile" ) var ( + badChartNamePath = filepath.Join(badCharNametDir, "Chart.yaml") badChartFilePath = filepath.Join(badChartDir, "Chart.yaml") nonExistingChartFilePath = filepath.Join(os.TempDir(), "Chart.yaml") ) var badChart, _ = chartutil.LoadChartfile(badChartFilePath) +var badChartName, _ = chartutil.LoadChartfile(badChartNamePath) // Validation functions Test func TestValidateChartYamlNotDirectory(t *testing.T) { @@ -69,6 +72,11 @@ func TestValidateChartName(t *testing.T) { if err == nil { t.Errorf("validateChartName to return a linter error, got no error") } + + err = validateChartName(badChartName) + if err == nil { + t.Error("expected validateChartName to return a linter error for an invalid name, got no error") + } } func TestValidateChartVersion(t *testing.T) { diff --git a/pkg/lint/rules/testdata/badchartname/Chart.yaml b/pkg/lint/rules/testdata/badchartname/Chart.yaml new file mode 100644 index 000000000..64f8fb8bf --- /dev/null +++ b/pkg/lint/rules/testdata/badchartname/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v2 +description: A Helm chart for Kubernetes +version: 0.1.0 +name: "../badchartname" +type: application diff --git a/pkg/lint/rules/testdata/badchartname/values.yaml b/pkg/lint/rules/testdata/badchartname/values.yaml new file mode 100644 index 000000000..9f367033b --- /dev/null +++ b/pkg/lint/rules/testdata/badchartname/values.yaml @@ -0,0 +1 @@ +# Default values for badchartfile.