Injection attack in Helm
Description
Helm is open-source software which is essentially "The Kubernetes Package Manager". Helm is a tool for managing Charts. Charts are packages of pre-configured Kubernetes resources. In Helm from version 3.0 and before version 3.5.2, there a few cases where data loaded from potentially untrusted sources was not properly sanitized. When a SemVer in the version field of a chart is invalid, in some cases Helm allows the string to be used "as is" without sanitizing. Helm fails to properly sanitized some fields present on Helm repository index.yaml files. Helm does not properly sanitized some fields in the plugin.yaml file for plugins In some cases, Helm does not properly sanitize the fields in the Chart.yaml file. By exploiting these attack vectors, core maintainers were able to send deceptive information to a terminal screen running the helm command, as well as obscure or alter information on the screen. In some cases, we could send codes that terminals used to execute higher-order logic, like clearing a terminal screen. Further, during evaluation, the Helm maintainers discovered a few other fields that were not properly sanitized when read out of repository index files. This fix remedies all such cases, and once again enforces SemVer2 policies on version fields. All users of the Helm 3 should upgrade to the fixed version 3.5.2 or later. Those who use Helm as a library should verify that they either sanitize this data on their own, or use the proper Helm API calls to sanitize the data.
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
helm.sh/helm/v3Go | >= 3.0.0, < 3.5.2 | 3.5.2 |
Affected products
1Patches
16ce9ba60b730Merge pull request from GHSA-c38g-469g-cmgx
30 files changed · +394 −226
cmd/helm/repo_update.go+7 −1 modified@@ -63,9 +63,15 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { func (o *repoUpdateOptions) run(out io.Writer) error { f, err := repo.LoadFile(o.repoFile) - if isNotExist(err) || len(f.Repositories) == 0 { + switch { + case isNotExist(err): + return errNoRepositories + case err != nil: + return errors.Wrapf(err, "failed loading file: %s", o.repoFile) + case len(f.Repositories) == 0: return errNoRepositories } + var repos []*repo.ChartRepository for _, cfg := range f.Repositories { r, err := repo.NewChartRepository(cfg, getter.All(settings))
cmd/helm/repo_update_test.go+13 −5 modified@@ -19,6 +19,7 @@ import ( "bytes" "fmt" "io" + "io/ioutil" "os" "path/filepath" "strings" @@ -53,20 +54,27 @@ func TestUpdateCmd(t *testing.T) { } func TestUpdateCustomCacheCmd(t *testing.T) { - var out bytes.Buffer rootDir := ensure.TempDir(t) cachePath := filepath.Join(rootDir, "updcustomcache") - _ = os.Mkdir(cachePath, os.ModePerm) + os.Mkdir(cachePath, os.ModePerm) defer os.RemoveAll(cachePath) + + ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") + if err != nil { + t.Fatal(err) + } + defer ts.Stop() + o := &repoUpdateOptions{ update: updateCharts, - repoFile: "testdata/repositories.yaml", + repoFile: filepath.Join(ts.Root(), "repositories.yaml"), repoCache: cachePath, } - if err := o.run(&out); err != nil { + b := ioutil.Discard + if err := o.run(b); err != nil { t.Fatal(err) } - if _, err := os.Stat(filepath.Join(cachePath, "charts-index.yaml")); err != nil { + if _, err := os.Stat(filepath.Join(cachePath, "test-index.yaml")); err != nil { t.Fatalf("error finding created index file in custom cache: %v", err) } }
cmd/helm/search_repo.go+10 −16 modified@@ -143,7 +143,7 @@ func (o *searchRepoOptions) setupSearchedVersion() { } func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Result, error) { - if len(o.version) == 0 { + if o.version == "" { return res, nil } @@ -154,26 +154,19 @@ func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Res data := res[:0] foundNames := map[string]bool{} - appendSearchResults := func(res *search.Result) { - data = append(data, res) - if !o.versions { - foundNames[res.Name] = true // If user hasn't requested all versions, only show the latest that matches - } - } for _, r := range res { - if _, found := foundNames[r.Name]; found { + // if not returning all versions and already have found a result, + // you're done! + if !o.versions && foundNames[r.Name] { continue } v, err := semver.NewVersion(r.Chart.Version) - if err != nil { - // If the current version number check appears ErrSegmentStartsZero or ErrInvalidPrerelease error and not devel mode, ignore - if (err == semver.ErrSegmentStartsZero || err == semver.ErrInvalidPrerelease) && !o.devel { - continue - } - appendSearchResults(r) - } else if constraint.Check(v) { - appendSearchResults(r) + continue + } + if constraint.Check(v) { + data = append(data, r) + foundNames[r.Name] = true } } @@ -194,6 +187,7 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) { ind, err := repo.LoadIndexFile(f) if err != nil { warning("Repo %q is corrupt or missing. Try 'helm repo update'.", n) + warning("%s", err) continue }
cmd/helm/search_repo_test.go+0 −8 modified@@ -68,14 +68,6 @@ func TestSearchRepositoriesCmd(t *testing.T) { name: "search for 'maria', expect valid json output", cmd: "search repo maria --output json", golden: "output/search-output-json.txt", - }, { - name: "search for 'maria', expect one match with semver begin with zero development version", - cmd: "search repo maria --devel", - golden: "output/search-semver-pre-zero-devel-release.txt", - }, { - name: "search for 'nginx-ingress', expect one match with invalid development pre version", - cmd: "search repo nginx-ingress --devel", - golden: "output/search-semver-pre-invalid-release.txt", }, { name: "search for 'alpine', expect valid yaml output", cmd: "search repo alpine --output yaml",
cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml+4 −30 modified@@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine url: https://charts.helm.sh/stable/alpine-0.2.0.tgz checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine url: https://charts.helm.sh/stable/alpine-0.3.0-rc.1.tgz checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d @@ -37,6 +39,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb url: https://charts.helm.sh/stable/mariadb-0.3.0.tgz @@ -55,33 +58,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" - - name: mariadb - url: https://charts.helm.sh/stable/mariadb-0.3.0-0565674.tgz - checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56 - home: https://mariadb.org - sources: - - https://github.com/bitnami/bitnami-docker-mariadb - version: 0.3.0-0565674 - description: Chart for MariaDB - keywords: - - mariadb - - mysql - - database - - sql - maintainers: - - name: Bitnami - email: containers@bitnami.com - icon: "" - nginx-ingress: - - name: nginx-ingress - url: https://github.com/kubernetes/ingress-nginx/ingress-a.b.c.sdfsdf.tgz - checksum: 25229f6de44a2be9f215d11dbff31167ddc8ba56 - home: https://github.com/kubernetes/ingress-nginx - sources: - - https://github.com/kubernetes/ingress-nginx - version: a.b.c.sdfsdf - description: Chart for nginx-ingress - keywords: - - ingress - - nginx - icon: "" + apiVersion: v2
cmd/helm/testdata/output/search-semver-pre-invalid-release.txt+0 −2 removed@@ -1,2 +0,0 @@ -NAME CHART VERSION APP VERSION DESCRIPTION -testing/nginx-ingress a.b.c.sdfsdf Chart for nginx-ingress
cmd/helm/testdata/output/search-semver-pre-zero-devel-release.txt+0 −2 removed@@ -1,2 +0,0 @@ -NAME CHART VERSION APP VERSION DESCRIPTION -testing/mariadb 0.3.0-0565674 Chart for MariaDB
internal/resolver/testdata/repository/kubernetes-charts-index.yaml+3 −0 modified@@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - https://charts.helm.sh/stable/alpine-0.2.0.tgz @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb urls: @@ -44,3 +46,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" + apiVersion: v2
pkg/chart/dependency.go+17 −0 modified@@ -49,6 +49,23 @@ type Dependency struct { Alias string `json:"alias,omitempty"` } +// Validate 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 (d *Dependency) Validate() error { + d.Name = sanitizeString(d.Name) + d.Version = sanitizeString(d.Version) + d.Repository = sanitizeString(d.Repository) + d.Condition = sanitizeString(d.Condition) + for i := range d.Tags { + d.Tags[i] = sanitizeString(d.Tags[i]) + } + if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) { + return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name) + } + return nil +} + // Lock is a lock file for dependencies. // // It represents the state that the dependencies should be in.
pkg/chart/dependency_test.go+44 −0 added@@ -0,0 +1,44 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package chart + +import ( + "testing" +) + +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 := dep.Validate() + 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) + } + } +}
pkg/chart/metadata.go+62 −15 modified@@ -15,6 +15,13 @@ limitations under the License. package chart +import ( + "strings" + "unicode" + + "github.com/Masterminds/semver/v3" +) + // Maintainer describes a Chart maintainer. type Maintainer struct { // Name is a user name or organization name @@ -25,15 +32,23 @@ type Maintainer struct { URL string `json:"url,omitempty"` } +// Validate checks valid data and sanitizes string characters. +func (m *Maintainer) Validate() error { + m.Name = sanitizeString(m.Name) + m.Email = sanitizeString(m.Email) + m.URL = sanitizeString(m.URL) + return nil +} + // Metadata for a Chart file. This models the structure of a Chart.yaml file. type Metadata struct { - // The name of the chart + // The name of the chart. Required. Name string `json:"name,omitempty"` // The URL to a relevant project page, git repo, or contact person Home string `json:"home,omitempty"` // Source is the URL to the source code of this chart Sources []string `json:"sources,omitempty"` - // A SemVer 2 conformant version string of the chart + // A SemVer 2 conformant version string of the chart. Required. Version string `json:"version,omitempty"` // A one-sentence description of the chart Description string `json:"description,omitempty"` @@ -43,7 +58,7 @@ type Metadata struct { Maintainers []*Maintainer `json:"maintainers,omitempty"` // The URL to an icon file. Icon string `json:"icon,omitempty"` - // The API Version of this chart. + // The API Version of this chart. Required. APIVersion string `json:"apiVersion,omitempty"` // The condition to check to enable chart Condition string `json:"condition,omitempty"` @@ -64,11 +79,28 @@ type Metadata struct { Type string `json:"type,omitempty"` } -// Validate checks the metadata for known issues, returning an error if metadata is not correct +// Validate checks the metadata for known issues and sanitizes string +// characters. func (md *Metadata) Validate() error { if md == nil { return ValidationError("chart.metadata is required") } + + md.Name = sanitizeString(md.Name) + md.Description = sanitizeString(md.Description) + md.Home = sanitizeString(md.Home) + md.Icon = sanitizeString(md.Icon) + md.Condition = sanitizeString(md.Condition) + md.Tags = sanitizeString(md.Tags) + md.AppVersion = sanitizeString(md.AppVersion) + md.KubeVersion = sanitizeString(md.KubeVersion) + for i := range md.Sources { + md.Sources[i] = sanitizeString(md.Sources[i]) + } + for i := range md.Keywords { + md.Keywords[i] = sanitizeString(md.Keywords[i]) + } + if md.APIVersion == "" { return ValidationError("chart.metadata.apiVersion is required") } @@ -78,19 +110,26 @@ func (md *Metadata) Validate() error { if md.Version == "" { return ValidationError("chart.metadata.version is required") } + if !isValidSemver(md.Version) { + return ValidationErrorf("chart.metadata.version %q is invalid", md.Version) + } if !isValidChartType(md.Type) { return ValidationError("chart.metadata.type must be application or library") } + for _, m := range md.Maintainers { + if err := m.Validate(); err != nil { + return err + } + } + // 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 { + if err := dependency.Validate(); err != nil { return err } } - - // TODO validate valid semver here? return nil } @@ -102,12 +141,20 @@ 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 +func isValidSemver(v string) bool { + _, err := semver.NewVersion(v) + return err == nil +} + +// sanitizeString normalize spaces and removes non-printable characters. +func sanitizeString(str string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return ' ' + } + if unicode.IsPrint(r) { + return r + } + return -1 + }, str) }
pkg/chart/metadata_test.go+13 −20 modified@@ -72,6 +72,10 @@ func TestValidate(t *testing.T) { }, ValidationError("dependency \"bad\" has disallowed characters in the alias"), }, + { + &Metadata{APIVersion: "v2", Name: "test", Version: "1.2.3.4"}, + ValidationError("chart.metadata.version \"1.2.3.4\" is invalid"), + }, } for _, tt := range tests { @@ -82,26 +86,15 @@ func TestValidate(t *testing.T) { } } -func TestValidateDependency(t *testing.T) { - dep := &Dependency{ - Name: "example", +func TestValidate_sanitize(t *testing.T) { + md := &Metadata{APIVersion: "v2", Name: "test", Version: "1.0", Description: "\adescr\u0081iption\rtest", Maintainers: []*Maintainer{{Name: "\r"}}} + if err := md.Validate(); err != nil { + t.Fatalf("unexpected error: %s", err) } - 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) - } + if md.Description != "description test" { + t.Fatalf("description was not sanitized: %q", md.Description) + } + if md.Maintainers[0].Name != " " { + t.Fatal("maintainer name was not sanitized") } }
pkg/chartutil/save_test.go+1 −1 modified@@ -139,7 +139,7 @@ func TestSavePreservesTimestamps(t *testing.T) { Metadata: &chart.Metadata{ APIVersion: chart.APIVersionV1, Name: "ahab", - Version: "1.2.3.4", + Version: "1.2.3", }, Values: map[string]interface{}{ "imageName": "testimage",
pkg/downloader/testdata/repository/kubernetes-charts-index.yaml+3 −0 modified@@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - https://charts.helm.sh/stable/alpine-0.2.0.tgz @@ -25,6 +26,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 mariadb: - name: mariadb urls: @@ -44,3 +46,4 @@ entries: - name: Bitnami email: containers@bitnami.com icon: "" + apiVersion: v2
pkg/downloader/testdata/repository/malformed-index.yaml+1 −0 modified@@ -13,3 +13,4 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2
pkg/downloader/testdata/repository/testing-basicauth-index.yaml+1 −0 modified@@ -12,3 +12,4 @@ entries: - http://username:password@example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/downloader/testdata/repository/testing-ca-file-index.yaml+1 −0 modified@@ -12,3 +12,4 @@ entries: - https://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/downloader/testdata/repository/testing-https-index.yaml+1 −0 modified@@ -12,3 +12,4 @@ entries: - https://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/downloader/testdata/repository/testing-index.yaml+3 −0 modified@@ -13,6 +13,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 - name: alpine urls: - http://example.com/alpine-0.2.0.tgz @@ -26,6 +27,7 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2 foo: - name: foo description: Foo Chart @@ -38,3 +40,4 @@ entries: - http://example.com/foo-1.2.3.tgz version: 1.2.3 checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/downloader/testdata/repository/testing-querystring-index.yaml+1 −0 modified@@ -13,3 +13,4 @@ entries: keywords: [] maintainers: [] icon: "" + apiVersion: v2
pkg/downloader/testdata/repository/testing-relative-index.yaml+28 −26 modified@@ -1,26 +1,28 @@ -apiVersion: v1 -entries: - foo: - - name: foo - description: Foo Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - charts/foo-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d - bar: - - name: bar - description: Bar Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - bar-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - charts/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 + bar: + - name: bar + description: Bar Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - bar-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/downloader/testdata/repository/testing-relative-trailing-slash-index.yaml+28 −26 modified@@ -1,26 +1,28 @@ -apiVersion: v1 -entries: - foo: - - name: foo - description: Foo Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - charts/foo-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d - bar: - - name: bar - description: Bar Chart With Relative Path - home: https://helm.sh/helm - keywords: [] - maintainers: [] - sources: - - https://github.com/helm/charts - urls: - - bar-1.2.3.tgz - version: 1.2.3 - checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d +apiVersion: v1 +entries: + foo: + - name: foo + description: Foo Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - charts/foo-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2 + bar: + - name: bar + description: Bar Chart With Relative Path + home: https://helm.sh/helm + keywords: [] + maintainers: [] + sources: + - https://github.com/helm/charts + urls: + - bar-1.2.3.tgz + version: 1.2.3 + checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d + apiVersion: v2
pkg/plugin/plugin.go+16 −0 modified@@ -23,6 +23,7 @@ import ( "regexp" "runtime" "strings" + "unicode" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -175,10 +176,25 @@ func validatePluginData(plug *Plugin, filepath string) error { if !validPluginName.MatchString(plug.Metadata.Name) { return fmt.Errorf("invalid plugin name at %q", filepath) } + plug.Metadata.Usage = sanitizeString(plug.Metadata.Usage) + // We could also validate SemVer, executable, and other fields should we so choose. return nil } +// sanitizeString normalize spaces and removes non-printable characters. +func sanitizeString(str string) string { + return strings.Map(func(r rune) rune { + if unicode.IsSpace(r) { + return ' ' + } + if unicode.IsPrint(r) { + return r + } + return -1 + }, str) +} + func detectDuplicates(plugs []*Plugin) error { names := map[string]string{}
pkg/repo/chartrepo.go+7 −3 modified@@ -82,6 +82,8 @@ func NewChartRepository(cfg *Entry, getters getter.Providers) (*ChartRepository, // Load loads a directory of charts as if it were a repository. // // It requires the presence of an index.yaml file in the directory. +// +// Deprecated: remove in Helm 4. func (r *ChartRepository) Load() error { dirInfo, err := os.Stat(r.Config.Name) if err != nil { @@ -99,7 +101,7 @@ func (r *ChartRepository) Load() error { if strings.Contains(f.Name(), "-index.yaml") { i, err := LoadIndexFile(path) if err != nil { - return nil + return err } r.IndexFile = i } else if strings.HasSuffix(f.Name(), ".tgz") { @@ -137,7 +139,7 @@ func (r *ChartRepository) DownloadIndexFile() (string, error) { return "", err } - indexFile, err := loadIndex(index) + indexFile, err := loadIndex(index, r.Config.URL) if err != nil { return "", err } @@ -187,7 +189,9 @@ func (r *ChartRepository) generateIndex() error { } if !r.IndexFile.Has(ch.Name(), ch.Metadata.Version) { - r.IndexFile.Add(ch.Metadata, path, r.Config.URL, digest) + if err := r.IndexFile.MustAdd(ch.Metadata, path, r.Config.URL, digest); err != nil { + return errors.Wrapf(err, "failed adding to %s to index", path) + } } // TODO: If a chart exists, but has a different Digest, should we error? }
pkg/repo/index.go+44 −10 modified@@ -19,6 +19,7 @@ package repo import ( "bytes" "io/ioutil" + "log" "os" "path" "path/filepath" @@ -105,16 +106,27 @@ func LoadIndexFile(path string) (*IndexFile, error) { if err != nil { return nil, err } - return loadIndex(b) + i, err := loadIndex(b, path) + if err != nil { + return nil, errors.Wrapf(err, "error loading %s", path) + } + return i, nil } -// Add adds a file to the index +// MustAdd adds a file to the index // This can leave the index in an unsorted state -func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { +func (i IndexFile) MustAdd(md *chart.Metadata, filename, baseURL, digest string) error { + if md.APIVersion == "" { + md.APIVersion = chart.APIVersionV1 + } + if err := md.Validate(); err != nil { + return errors.Wrapf(err, "validate failed for %s", filename) + } + u := filename if baseURL != "" { - var err error _, file := filepath.Split(filename) + var err error u, err = urlutil.URLJoin(baseURL, file) if err != nil { u = path.Join(baseURL, file) @@ -126,10 +138,17 @@ func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { Digest: digest, Created: time.Now(), } - if ee, ok := i.Entries[md.Name]; !ok { - i.Entries[md.Name] = ChartVersions{cr} - } else { - i.Entries[md.Name] = append(ee, cr) + ee := i.Entries[md.Name] + i.Entries[md.Name] = append(ee, cr) + return nil +} + +// Add adds a file to the index and logs an error. +// +// Deprecated: Use index.MustAdd instead. +func (i IndexFile) Add(md *chart.Metadata, filename, baseURL, digest string) { + if err := i.MustAdd(md, filename, baseURL, digest); err != nil { + log.Printf("skipping loading invalid entry for chart %q %q from %s: %s", md.Name, md.Version, filename, err) } } @@ -294,19 +313,34 @@ func IndexDirectory(dir, baseURL string) (*IndexFile, error) { if err != nil { return index, err } - index.Add(c.Metadata, fname, parentURL, hash) + if err := index.MustAdd(c.Metadata, fname, parentURL, hash); err != nil { + return index, errors.Wrapf(err, "failed adding to %s to index", fname) + } } return index, nil } // loadIndex loads an index file and does minimal validity checking. // +// The source parameter is only used for logging. // This will fail if API Version is not set (ErrNoAPIVersion) or if the unmarshal fails. -func loadIndex(data []byte) (*IndexFile, error) { +func loadIndex(data []byte, source string) (*IndexFile, error) { i := &IndexFile{} if err := yaml.UnmarshalStrict(data, i); err != nil { return i, err } + + for name, cvs := range i.Entries { + for idx := len(cvs) - 1; idx >= 0; idx-- { + if cvs[idx].APIVersion == "" { + cvs[idx].APIVersion = chart.APIVersionV1 + } + if err := cvs[idx].Validate(); err != nil { + log.Printf("skipping loading invalid entry for chart %q %q from %s: %s", name, cvs[idx].Version, source, err) + cvs = append(cvs[:idx], cvs[idx+1:]...) + } + } + } i.SortEntries() if i.APIVersion == "" { return i, ErrNoAPIVersion
pkg/repo/index_test.go+70 −61 modified@@ -27,11 +27,10 @@ import ( "strings" "testing" + "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/helmpath" - - "helm.sh/helm/v3/pkg/chart" ) const ( @@ -65,12 +64,23 @@ entries: func TestIndexFile(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.1.1"}, "cutter-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.1.0"}, "cutter-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "cutter", Version: "0.2.0"}, "cutter-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "setter", Version: "0.1.9+alpha"}, "setter-0.1.9+alpha.tgz", "http://example.com/charts", "sha256:1234567890abc") - i.Add(&chart.Metadata{Name: "setter", Version: "0.1.9+beta"}, "setter-0.1.9+beta.tgz", "http://example.com/charts", "sha256:1234567890abc") + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + {&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.1.1"}, "cutter-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.1.0"}, "cutter-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "cutter", Version: "0.2.0"}, "cutter-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "setter", Version: "0.1.9+alpha"}, "setter-0.1.9+alpha.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + {&chart.Metadata{APIVersion: "v2", Name: "setter", Version: "0.1.9+beta"}, "setter-0.1.9+beta.tgz", "http://example.com/charts", "sha256:1234567890abc"}, + } { + if err := i.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error adding to index: %s", err) + } + } i.SortEntries() @@ -126,11 +136,7 @@ func TestLoadIndex(t *testing.T) { tc := tc t.Run(tc.Name, func(t *testing.T) { t.Parallel() - b, err := ioutil.ReadFile(tc.Filename) - if err != nil { - t.Fatal(err) - } - i, err := loadIndex(b) + i, err := LoadIndexFile(tc.Filename) if err != nil { t.Fatal(err) } @@ -141,19 +147,11 @@ func TestLoadIndex(t *testing.T) { // TestLoadIndex_Duplicates is a regression to make sure that we don't non-deterministically allow duplicate packages. func TestLoadIndex_Duplicates(t *testing.T) { - if _, err := loadIndex([]byte(indexWithDuplicates)); err == nil { + if _, err := loadIndex([]byte(indexWithDuplicates), "indexWithDuplicates"); err == nil { t.Errorf("Expected an error when duplicate entries are present") } } -func TestLoadIndexFile(t *testing.T) { - i, err := LoadIndexFile(testfile) - if err != nil { - t.Fatal(err) - } - verifyLocalIndex(t, i) -} - func TestLoadIndexFileAnnotations(t *testing.T) { i, err := LoadIndexFile(annotationstestfile) if err != nil { @@ -170,11 +168,7 @@ func TestLoadIndexFileAnnotations(t *testing.T) { } func TestLoadUnorderedIndex(t *testing.T) { - b, err := ioutil.ReadFile(unorderedTestfile) - if err != nil { - t.Fatal(err) - } - i, err := loadIndex(b) + i, err := LoadIndexFile(unorderedTestfile) if err != nil { t.Fatal(err) } @@ -183,20 +177,26 @@ func TestLoadUnorderedIndex(t *testing.T) { func TestMerge(t *testing.T) { ind1 := NewIndexFile() - ind1.Add(&chart.Metadata{ - Name: "dreadnought", - Version: "0.1.0", - }, "dreadnought-0.1.0.tgz", "http://example.com", "aaaa") + + if err := ind1.MustAdd(&chart.Metadata{APIVersion: "v2", Name: "dreadnought", Version: "0.1.0"}, "dreadnought-0.1.0.tgz", "http://example.com", "aaaa"); err != nil { + t.Fatalf("unexpected error: %s", err) + } ind2 := NewIndexFile() - ind2.Add(&chart.Metadata{ - Name: "dreadnought", - Version: "0.2.0", - }, "dreadnought-0.2.0.tgz", "http://example.com", "aaaabbbb") - ind2.Add(&chart.Metadata{ - Name: "doughnut", - Version: "0.2.0", - }, "doughnut-0.2.0.tgz", "http://example.com", "ccccbbbb") + + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + {&chart.Metadata{APIVersion: "v2", Name: "dreadnought", Version: "0.2.0"}, "dreadnought-0.2.0.tgz", "http://example.com", "aaaabbbb"}, + {&chart.Metadata{APIVersion: "v2", Name: "doughnut", Version: "0.2.0"}, "doughnut-0.2.0.tgz", "http://example.com", "ccccbbbb"}, + } { + if err := ind2.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error: %s", err) + } + } ind1.Merge(ind2) @@ -239,12 +239,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - b, err := ioutil.ReadFile(idx) - if err != nil { - t.Fatalf("error reading index file: %#v", err) - } - - i, err := loadIndex(b) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -256,7 +251,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created charts file: %#v", err) } - b, err = ioutil.ReadFile(idx) + b, err := ioutil.ReadFile(idx) if err != nil { t.Fatalf("error reading charts file: %#v", err) } @@ -297,12 +292,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created index file: %#v", err) } - b, err := ioutil.ReadFile(idx) - if err != nil { - t.Fatalf("error reading index file: %#v", err) - } - - i, err := loadIndex(b) + i, err := LoadIndexFile(idx) if err != nil { t.Fatalf("Index %q failed to parse: %s", testfile, err) } @@ -314,7 +304,7 @@ func TestDownloadIndexFile(t *testing.T) { t.Fatalf("error finding created charts file: %#v", err) } - b, err = ioutil.ReadFile(idx) + b, err := ioutil.ReadFile(idx) if err != nil { t.Fatalf("error reading charts file: %#v", err) } @@ -345,6 +335,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { expects := []*ChartVersion{ { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "alpine", Description: "string", Version: "1.0.0", @@ -359,6 +350,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { }, { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "nginx", Description: "string", Version: "0.2.0", @@ -372,6 +364,7 @@ func verifyLocalIndex(t *testing.T, i *IndexFile) { }, { Metadata: &chart.Metadata{ + APIVersion: "v2", Name: "nginx", Description: "string", Version: "0.1.0", @@ -476,28 +469,44 @@ func TestIndexDirectory(t *testing.T) { func TestIndexAdd(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") + + for _, x := range []struct { + md *chart.Metadata + filename string + baseURL string + digest string + }{ + + {&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "alpine", Version: "0.1.0"}, "/home/charts/alpine-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"}, + {&chart.Metadata{APIVersion: "v2", Name: "deis", Version: "0.1.0"}, "/home/charts/deis-0.1.0.tgz", "http://example.com/charts/", "sha256:1234567890"}, + } { + if err := i.MustAdd(x.md, x.filename, x.baseURL, x.digest); err != nil { + t.Errorf("unexpected error adding to index: %s", err) + } + } if i.Entries["clipper"][0].URLs[0] != "http://example.com/charts/clipper-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/clipper-0.1.0.tgz, got %s", i.Entries["clipper"][0].URLs[0]) } - - i.Add(&chart.Metadata{Name: "alpine", Version: "0.1.0"}, "/home/charts/alpine-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") - if i.Entries["alpine"][0].URLs[0] != "http://example.com/charts/alpine-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/alpine-0.1.0.tgz, got %s", i.Entries["alpine"][0].URLs[0]) } - - i.Add(&chart.Metadata{Name: "deis", Version: "0.1.0"}, "/home/charts/deis-0.1.0.tgz", "http://example.com/charts/", "sha256:1234567890") - if i.Entries["deis"][0].URLs[0] != "http://example.com/charts/deis-0.1.0.tgz" { t.Errorf("Expected http://example.com/charts/deis-0.1.0.tgz, got %s", i.Entries["deis"][0].URLs[0]) } + + // test error condition + if err := i.MustAdd(&chart.Metadata{}, "error-0.1.0.tgz", "", ""); err == nil { + t.Fatal("expected error adding to index") + } } func TestIndexWrite(t *testing.T) { i := NewIndexFile() - i.Add(&chart.Metadata{Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890") + if err := i.MustAdd(&chart.Metadata{APIVersion: "v2", Name: "clipper", Version: "0.1.0"}, "clipper-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890"); err != nil { + t.Fatalf("unexpected error: %s", err) + } dir, err := ioutil.TempDir("", "helm-tmp") if err != nil { t.Fatal(err)
pkg/repo/testdata/chartmuseum-index.yaml+4 −0 modified@@ -14,6 +14,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -25,6 +26,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -39,6 +41,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -48,3 +51,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2
pkg/repo/testdata/local-index-annotations.yaml+4 −0 modified@@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,5 +49,6 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 annotations: helm.sh/test: foo bar
pkg/repo/testdata/local-index-unordered.yaml+4 −0 modified@@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.2.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,3 +49,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2
pkg/repo/testdata/local-index.yaml+4 −0 modified@@ -12,6 +12,7 @@ entries: - popular - web server - proxy + apiVersion: v2 - urls: - https://charts.helm.sh/stable/nginx-0.1.0.tgz name: nginx @@ -23,6 +24,7 @@ entries: - popular - web server - proxy + apiVersion: v2 alpine: - urls: - https://charts.helm.sh/stable/alpine-1.0.0.tgz @@ -37,6 +39,7 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2 chartWithNoURL: - name: chartWithNoURL description: string @@ -46,3 +49,4 @@ entries: - small - sumtin digest: "sha256:1234567890abcdef" + apiVersion: v2
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
6- github.com/advisories/GHSA-c38g-469g-cmgxghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2021-21303ghsaADVISORY
- github.com/helm/helm/commit/6ce9ba60b73013857e2e7c73d3f86ed70bc1ac9aghsax_refsource_MISCWEB
- github.com/helm/helm/releases/tag/v3.5.2ghsax_refsource_MISCWEB
- github.com/helm/helm/security/advisories/GHSA-c38g-469g-cmgxghsax_refsource_CONFIRMWEB
- pkg.go.dev/vuln/GO-2022-1040ghsaWEB
News mentions
0No linked articles in our index yet.