Skip to content

Commit

Permalink
validation fix
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Farina <matt.farina@suse.com>
(cherry picked from commit 8e6a514)
  • Loading branch information
mattfarina committed Feb 14, 2024
1 parent 3fc9f4b commit e8858f8
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/chart/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
package chart

import (
"path/filepath"
"strings"
"unicode"

Expand Down Expand Up @@ -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")
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/chart/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
8 changes: 8 additions & 0 deletions pkg/chartutil/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
20 changes: 20 additions & 0 deletions pkg/chartutil/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
29 changes: 29 additions & 0 deletions pkg/chartutil/save_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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())
}
}
26 changes: 26 additions & 0 deletions pkg/downloader/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/lint/rules/chartfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/lint/rules/chartfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/lint/rules/testdata/badchartname/Chart.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: v2
description: A Helm chart for Kubernetes
version: 0.1.0
name: "../badchartname"
type: application
1 change: 1 addition & 0 deletions pkg/lint/rules/testdata/badchartname/values.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Default values for badchartfile.

0 comments on commit e8858f8

Please sign in to comment.