Skip to content

Commit 6ce9ba6

Browse files
authored
Merge pull request from GHSA-c38g-469g-cmgx
fix(*): Validate metadata semver and printable characters
2 parents 46d80f6 + 657ce55 commit 6ce9ba6

30 files changed

+394
-226
lines changed

Diff for: cmd/helm/repo_update.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,15 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command {
6363

6464
func (o *repoUpdateOptions) run(out io.Writer) error {
6565
f, err := repo.LoadFile(o.repoFile)
66-
if isNotExist(err) || len(f.Repositories) == 0 {
66+
switch {
67+
case isNotExist(err):
68+
return errNoRepositories
69+
case err != nil:
70+
return errors.Wrapf(err, "failed loading file: %s", o.repoFile)
71+
case len(f.Repositories) == 0:
6772
return errNoRepositories
6873
}
74+
6975
var repos []*repo.ChartRepository
7076
for _, cfg := range f.Repositories {
7177
r, err := repo.NewChartRepository(cfg, getter.All(settings))

Diff for: cmd/helm/repo_update_test.go

+13-5
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"bytes"
2020
"fmt"
2121
"io"
22+
"io/ioutil"
2223
"os"
2324
"path/filepath"
2425
"strings"
@@ -53,20 +54,27 @@ func TestUpdateCmd(t *testing.T) {
5354
}
5455

5556
func TestUpdateCustomCacheCmd(t *testing.T) {
56-
var out bytes.Buffer
5757
rootDir := ensure.TempDir(t)
5858
cachePath := filepath.Join(rootDir, "updcustomcache")
59-
_ = os.Mkdir(cachePath, os.ModePerm)
59+
os.Mkdir(cachePath, os.ModePerm)
6060
defer os.RemoveAll(cachePath)
61+
62+
ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
63+
if err != nil {
64+
t.Fatal(err)
65+
}
66+
defer ts.Stop()
67+
6168
o := &repoUpdateOptions{
6269
update: updateCharts,
63-
repoFile: "testdata/repositories.yaml",
70+
repoFile: filepath.Join(ts.Root(), "repositories.yaml"),
6471
repoCache: cachePath,
6572
}
66-
if err := o.run(&out); err != nil {
73+
b := ioutil.Discard
74+
if err := o.run(b); err != nil {
6775
t.Fatal(err)
6876
}
69-
if _, err := os.Stat(filepath.Join(cachePath, "charts-index.yaml")); err != nil {
77+
if _, err := os.Stat(filepath.Join(cachePath, "test-index.yaml")); err != nil {
7078
t.Fatalf("error finding created index file in custom cache: %v", err)
7179
}
7280
}

Diff for: cmd/helm/search_repo.go

+10-16
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (o *searchRepoOptions) setupSearchedVersion() {
143143
}
144144

145145
func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Result, error) {
146-
if len(o.version) == 0 {
146+
if o.version == "" {
147147
return res, nil
148148
}
149149

@@ -154,26 +154,19 @@ func (o *searchRepoOptions) applyConstraint(res []*search.Result) ([]*search.Res
154154

155155
data := res[:0]
156156
foundNames := map[string]bool{}
157-
appendSearchResults := func(res *search.Result) {
158-
data = append(data, res)
159-
if !o.versions {
160-
foundNames[res.Name] = true // If user hasn't requested all versions, only show the latest that matches
161-
}
162-
}
163157
for _, r := range res {
164-
if _, found := foundNames[r.Name]; found {
158+
// if not returning all versions and already have found a result,
159+
// you're done!
160+
if !o.versions && foundNames[r.Name] {
165161
continue
166162
}
167163
v, err := semver.NewVersion(r.Chart.Version)
168-
169164
if err != nil {
170-
// If the current version number check appears ErrSegmentStartsZero or ErrInvalidPrerelease error and not devel mode, ignore
171-
if (err == semver.ErrSegmentStartsZero || err == semver.ErrInvalidPrerelease) && !o.devel {
172-
continue
173-
}
174-
appendSearchResults(r)
175-
} else if constraint.Check(v) {
176-
appendSearchResults(r)
165+
continue
166+
}
167+
if constraint.Check(v) {
168+
data = append(data, r)
169+
foundNames[r.Name] = true
177170
}
178171
}
179172

@@ -194,6 +187,7 @@ func (o *searchRepoOptions) buildIndex() (*search.Index, error) {
194187
ind, err := repo.LoadIndexFile(f)
195188
if err != nil {
196189
warning("Repo %q is corrupt or missing. Try 'helm repo update'.", n)
190+
warning("%s", err)
197191
continue
198192
}
199193

Diff for: cmd/helm/search_repo_test.go

-8
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,6 @@ func TestSearchRepositoriesCmd(t *testing.T) {
6868
name: "search for 'maria', expect valid json output",
6969
cmd: "search repo maria --output json",
7070
golden: "output/search-output-json.txt",
71-
}, {
72-
name: "search for 'maria', expect one match with semver begin with zero development version",
73-
cmd: "search repo maria --devel",
74-
golden: "output/search-semver-pre-zero-devel-release.txt",
75-
}, {
76-
name: "search for 'nginx-ingress', expect one match with invalid development pre version",
77-
cmd: "search repo nginx-ingress --devel",
78-
golden: "output/search-semver-pre-invalid-release.txt",
7971
}, {
8072
name: "search for 'alpine', expect valid yaml output",
8173
cmd: "search repo alpine --output yaml",

Diff for: cmd/helm/testdata/helmhome/helm/repository/testing-index.yaml

+4-30
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ entries:
1313
keywords: []
1414
maintainers: []
1515
icon: ""
16+
apiVersion: v2
1617
- name: alpine
1718
url: https://charts.helm.sh/stable/alpine-0.2.0.tgz
1819
checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
@@ -25,6 +26,7 @@ entries:
2526
keywords: []
2627
maintainers: []
2728
icon: ""
29+
apiVersion: v2
2830
- name: alpine
2931
url: https://charts.helm.sh/stable/alpine-0.3.0-rc.1.tgz
3032
checksum: 0e6661f193211d7a5206918d42f5c2a9470b737d
@@ -37,6 +39,7 @@ entries:
3739
keywords: []
3840
maintainers: []
3941
icon: ""
42+
apiVersion: v2
4043
mariadb:
4144
- name: mariadb
4245
url: https://charts.helm.sh/stable/mariadb-0.3.0.tgz
@@ -55,33 +58,4 @@ entries:
5558
- name: Bitnami
5659
email: containers@bitnami.com
5760
icon: ""
58-
- name: mariadb
59-
url: https://charts.helm.sh/stable/mariadb-0.3.0-0565674.tgz
60-
checksum: 65229f6de44a2be9f215d11dbff311673fc8ba56
61-
home: https://mariadb.org
62-
sources:
63-
- https://github.com/bitnami/bitnami-docker-mariadb
64-
version: 0.3.0-0565674
65-
description: Chart for MariaDB
66-
keywords:
67-
- mariadb
68-
- mysql
69-
- database
70-
- sql
71-
maintainers:
72-
- name: Bitnami
73-
email: containers@bitnami.com
74-
icon: ""
75-
nginx-ingress:
76-
- name: nginx-ingress
77-
url: https://github.com/kubernetes/ingress-nginx/ingress-a.b.c.sdfsdf.tgz
78-
checksum: 25229f6de44a2be9f215d11dbff31167ddc8ba56
79-
home: https://github.com/kubernetes/ingress-nginx
80-
sources:
81-
- https://github.com/kubernetes/ingress-nginx
82-
version: a.b.c.sdfsdf
83-
description: Chart for nginx-ingress
84-
keywords:
85-
- ingress
86-
- nginx
87-
icon: ""
61+
apiVersion: v2

Diff for: cmd/helm/testdata/output/search-semver-pre-invalid-release.txt

-2
This file was deleted.

Diff for: cmd/helm/testdata/output/search-semver-pre-zero-devel-release.txt

-2
This file was deleted.

Diff for: internal/resolver/testdata/repository/kubernetes-charts-index.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ entries:
1313
keywords: []
1414
maintainers: []
1515
icon: ""
16+
apiVersion: v2
1617
- name: alpine
1718
urls:
1819
- https://charts.helm.sh/stable/alpine-0.2.0.tgz
@@ -25,6 +26,7 @@ entries:
2526
keywords: []
2627
maintainers: []
2728
icon: ""
29+
apiVersion: v2
2830
mariadb:
2931
- name: mariadb
3032
urls:
@@ -44,3 +46,4 @@ entries:
4446
- name: Bitnami
4547
email: containers@bitnami.com
4648
icon: ""
49+
apiVersion: v2

Diff for: pkg/chart/dependency.go

+17
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,23 @@ type Dependency struct {
4949
Alias string `json:"alias,omitempty"`
5050
}
5151

52+
// Validate checks for common problems with the dependency datastructure in
53+
// the chart. This check must be done at load time before the dependency's charts are
54+
// loaded.
55+
func (d *Dependency) Validate() error {
56+
d.Name = sanitizeString(d.Name)
57+
d.Version = sanitizeString(d.Version)
58+
d.Repository = sanitizeString(d.Repository)
59+
d.Condition = sanitizeString(d.Condition)
60+
for i := range d.Tags {
61+
d.Tags[i] = sanitizeString(d.Tags[i])
62+
}
63+
if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) {
64+
return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name)
65+
}
66+
return nil
67+
}
68+
5269
// Lock is a lock file for dependencies.
5370
//
5471
// It represents the state that the dependencies should be in.

Diff for: pkg/chart/dependency_test.go

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
Copyright The Helm Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package chart
17+
18+
import (
19+
"testing"
20+
)
21+
22+
func TestValidateDependency(t *testing.T) {
23+
dep := &Dependency{
24+
Name: "example",
25+
}
26+
for value, shouldFail := range map[string]bool{
27+
"abcdefghijklmenopQRSTUVWXYZ-0123456780_": false,
28+
"-okay": false,
29+
"_okay": false,
30+
"- bad": true,
31+
" bad": true,
32+
"bad\nvalue": true,
33+
"bad ": true,
34+
"bad$": true,
35+
} {
36+
dep.Alias = value
37+
res := dep.Validate()
38+
if res != nil && !shouldFail {
39+
t.Errorf("Failed on case %q", dep.Alias)
40+
} else if res == nil && shouldFail {
41+
t.Errorf("Expected failure for %q", dep.Alias)
42+
}
43+
}
44+
}

0 commit comments

Comments
 (0)