From 5280817d801d0bc959935b105b445611319a7bdf Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:37:29 +1100 Subject: [PATCH] Bound chart parsing, require name-version filenames, expand index aliases --- README.md | 5 +- internal/provider/helm/helm.go | 44 +++++++++++++-- internal/provider/helm/helm_local_test.go | 55 +++++++++++++++++++ internal/virtual/helm_merger.go | 45 ++++++++++++++- internal/virtual/helm_merger_test.go | 67 +++++++++++++++++++++++ 5 files changed, 208 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index e3b29e2..a653235 100644 --- a/README.md +++ b/README.md @@ -267,8 +267,9 @@ there's nothing to provision. To bring your own key instead, point A local `helm` repo is a real chart repository: `PUT` a packaged chart to `/api/v2/remotes//files/-.tgz` and `index.yaml` is generated from each chart's `Chart.yaml` (with sha256 digests) at -`/api/v1/local//index.yaml`. Add the local as a member of a helm virtual -to serve it alongside remotes. +`/api/v1/local//index.yaml`; a file not named after its `Chart.yaml` +name and version is stored but left out of the index. Add the local as a +member of a helm virtual to serve it alongside remotes. ```bash helm package ./mychart diff --git a/internal/provider/helm/helm.go b/internal/provider/helm/helm.go index 4884618..a39ae95 100644 --- a/internal/provider/helm/helm.go +++ b/internal/provider/helm/helm.go @@ -9,6 +9,7 @@ import ( "io" "log/slog" "net/http" + "net/url" "path" "strings" "time" @@ -99,6 +100,10 @@ func (p *Provider) AfterUpload(ctx context.Context, repoName, storagePath, conte slog.Error("helm metadata: parse failed", "repo", repoName, "path", storagePath, "error", err) return } + if want := meta.Name + "-" + meta.Version + ".tgz"; storagePath != want { + slog.Error("helm metadata: filename does not match chart", "repo", repoName, "path", storagePath, "expected", want) + return + } meta.RepoName = repoName meta.FilePath = storagePath meta.ContentHash = contentHash @@ -172,7 +177,7 @@ func generateIndex(metas []provider.HelmMetadata) ([]byte, error) { } chart["name"] = node(m.Name) chart["version"] = node(m.Version) - chart["urls"] = node([]string{m.FilePath}) + chart["urls"] = node([]string{url.PathEscape(m.FilePath)}) chart["digest"] = node(strings.TrimPrefix(m.ContentHash, "sha256:")) chart["created"] = node(m.CreatedAt.UTC().Format(time.RFC3339Nano)) entries[m.Name] = append(entries[m.Name], chart) @@ -193,7 +198,30 @@ func node(v any) yaml.Node { return n } -const maxChartYAML = 1 << 20 +const ( + maxChartYAML = 1 << 20 + maxChartBytes = 50 << 20 + maxChartEntries = 10000 +) + +var errChartTooLarge = fmt.Errorf("chart archive exceeds %d decompressed bytes", maxChartBytes) + +type capReader struct { + r io.Reader + n int64 +} + +func (c *capReader) Read(p []byte) (int, error) { + if c.n <= 0 { + return 0, errChartTooLarge + } + if int64(len(p)) > c.n { + p = p[:c.n] + } + n, err := c.r.Read(p) + c.n -= int64(n) + return n, err +} // parseChart reads the top-level /Chart.yaml from a packaged chart. func parseChart(r io.Reader) (*provider.HelmMetadata, error) { @@ -201,8 +229,11 @@ func parseChart(r io.Reader) (*provider.HelmMetadata, error) { if err != nil { return nil, err } - tr := tar.NewReader(zr) - for { + tr := tar.NewReader(&capReader{r: zr, n: maxChartBytes}) + for entries := 0; ; entries++ { + if entries >= maxChartEntries { + return nil, fmt.Errorf("chart archive exceeds %d entries", maxChartEntries) + } hdr, err := tr.Next() if err == io.EOF { return nil, errors.New("no Chart.yaml in chart archive") @@ -214,10 +245,13 @@ func parseChart(r io.Reader) (*provider.HelmMetadata, error) { if file != "Chart.yaml" || strings.Count(dir, "/") != 1 { continue } - raw, err := io.ReadAll(io.LimitReader(tr, maxChartYAML)) + raw, err := io.ReadAll(io.LimitReader(tr, maxChartYAML+1)) if err != nil { return nil, err } + if len(raw) > maxChartYAML { + return nil, fmt.Errorf("parse Chart.yaml: exceeds %d bytes", maxChartYAML) + } var chart struct { Name string `yaml:"name"` Version string `yaml:"version"` diff --git a/internal/provider/helm/helm_local_test.go b/internal/provider/helm/helm_local_test.go index 71f9ebf..4e8c775 100644 --- a/internal/provider/helm/helm_local_test.go +++ b/internal/provider/helm/helm_local_test.go @@ -7,6 +7,8 @@ import ( "context" "io" "net/http/httptest" + "strconv" + "strings" "testing" "time" @@ -160,3 +162,56 @@ func TestServeLocalIndex(t *testing.T) { t.Errorf("index not deterministic: %v", err) } } + +func TestAfterUploadRejectsFilenameMismatch(t *testing.T) { + store := &fakeStore{} + data := chartTgz(t, map[string]string{"mychart/Chart.yaml": "apiVersion: v2\nname: mychart\nversion: 1.0.0\n"}) + (&Provider{}).AfterUpload(context.Background(), "r", "other-1.0.0.tgz", "sha256:ab", fakeBlobs{data}, store) + if store.inserted != nil { + t.Fatal("chart indexed under a filename that does not match name-version") + } +} + +func TestParseChartLimits(t *testing.T) { + chart := "apiVersion: v2\nname: c\nversion: 1.0.0\n" + for name, tc := range map[string]struct { + files map[string]string + want string + }{ + "decompressed": {map[string]string{"c/a.bin": strings.Repeat("\x00", maxChartBytes+1)}, "decompressed bytes"}, + "chart yaml": {map[string]string{"c/Chart.yaml": chart + "#" + strings.Repeat("x", maxChartYAML)}, "Chart.yaml: exceeds"}, + } { + _, err := parseChart(bytes.NewReader(chartTgz(t, tc.files))) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: err = %v, want %q", name, err, tc.want) + } + } + + var buf bytes.Buffer + zw := gzip.NewWriter(&buf) + tw := tar.NewWriter(zw) + for i := 0; i <= maxChartEntries; i++ { + _ = tw.WriteHeader(&tar.Header{Name: "c/templates/" + strconv.Itoa(i), Mode: 0o644}) + } + _ = tw.WriteHeader(&tar.Header{Name: "c/Chart.yaml", Mode: 0o644, Size: int64(len(chart))}) + _, _ = tw.Write([]byte(chart)) + _ = tw.Close() + _ = zw.Close() + if _, err := parseChart(&buf); err == nil || !strings.Contains(err.Error(), "entries") { + t.Errorf("entries: err = %v", err) + } + + if m, err := parseChart(bytes.NewReader(chartTgz(t, map[string]string{"c/Chart.yaml": chart}))); err != nil || m.Name != "c" { + t.Errorf("valid chart: %v %v", m, err) + } +} + +func TestIndexEscapesURLs(t *testing.T) { + out, err := generateIndex([]provider.HelmMetadata{{FilePath: "my chart-1.0.0.tgz", Name: "my chart", Version: "1.0.0", ChartYAML: "name: x\n"}}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), "my%20chart-1.0.0.tgz") { + t.Errorf("url not escaped:\n%s", out) + } +} diff --git a/internal/virtual/helm_merger.go b/internal/virtual/helm_merger.go index e1ba5e3..f90e803 100644 --- a/internal/virtual/helm_merger.go +++ b/internal/virtual/helm_merger.go @@ -1,6 +1,7 @@ package virtual import ( + "errors" "fmt" "strings" @@ -37,11 +38,16 @@ func (m *HelmMerger) MergeIndexes(members []MemberIndex, proxyBaseURL string) ([ continue } + budget := maxExpandedNodes for chart, versions := range idx.Entries { if seen[chart] == nil { seen[chart] = map[string]bool{} } - for _, ver := range versions { + for _, raw := range versions { + ver, err := expandAliases(raw, &budget) + if err != nil { + continue + } version := ver["version"].Value if seen[chart][version] { continue @@ -69,6 +75,43 @@ func (m *HelmMerger) MergeIndexes(members []MemberIndex, proxyBaseURL string) ([ return yaml.Marshal(merged) } +const maxExpandedNodes = 100000 + +// expandAliases inlines alias targets and drops anchors so an entry stays +// valid after the version that held its anchor is deduplicated away; budget +// caps total expanded nodes per index against alias bombs. +func expandAliases(ver map[string]yaml.Node, budget *int) (map[string]yaml.Node, error) { + var expand func(n *yaml.Node) (*yaml.Node, error) + expand = func(n *yaml.Node) (*yaml.Node, error) { + *budget-- + if *budget < 0 { + return nil, errors.New("chart entry expands to too many yaml nodes") + } + if n.Kind == yaml.AliasNode { + return expand(n.Alias) + } + c := *n + c.Anchor = "" + c.Content = make([]*yaml.Node, len(n.Content)) + for i, ch := range n.Content { + var err error + if c.Content[i], err = expand(ch); err != nil { + return nil, err + } + } + return &c, nil + } + out := make(map[string]yaml.Node, len(ver)) + for k, v := range ver { + e, err := expand(&v) + if err != nil { + return nil, err + } + out[k] = *e + } + return out, nil +} + func rewriteHelmURLs(urls []string, member MemberIndex, proxyBaseURL string) { routePrefix := "remote" if member.RepoType == "local" { diff --git a/internal/virtual/helm_merger_test.go b/internal/virtual/helm_merger_test.go index 2b8697b..6d762b3 100644 --- a/internal/virtual/helm_merger_test.go +++ b/internal/virtual/helm_merger_test.go @@ -4,6 +4,8 @@ import ( "strings" "testing" + "gopkg.in/yaml.v3" + "git.unkin.net/unkin/artifactapi/internal/virtual" ) @@ -155,3 +157,68 @@ entries: } } } + +func TestHelmMerger_ExpandsAliasesAcrossDedupe(t *testing.T) { + m := &virtual.HelmMerger{} + first := []byte(`apiVersion: v1 +entries: + app: + - version: "1.0" + urls: [app-1.0.tgz] +`) + second := []byte(`apiVersion: v1 +entries: + app: + - version: "1.0" + maintainers: &team + - name: ops + urls: [app-1.0.tgz] + - version: "2.0" + maintainers: *team + urls: [app-2.0.tgz] +`) + result, err := m.MergeIndexes([]virtual.MemberIndex{ + {RemoteName: "a", RepoType: "local", Body: first}, + {RemoteName: "b", RepoType: "local", Body: second}, + }, "https://proxy.example.com") + if err != nil { + t.Fatal(err) + } + var idx struct { + Entries map[string][]struct { + Version string `yaml:"version"` + Maintainers []struct { + Name string `yaml:"name"` + } `yaml:"maintainers"` + } `yaml:"entries"` + } + if err := yaml.Unmarshal(result, &idx); err != nil { + t.Fatalf("merged index does not parse: %v\n%s", err, result) + } + app := idx.Entries["app"] + if len(app) != 2 || app[1].Version != "2.0" || len(app[1].Maintainers) != 1 || app[1].Maintainers[0].Name != "ops" { + t.Errorf("alias not expanded: %+v\n%s", app, result) + } +} + +func TestHelmMerger_DropsAliasBomb(t *testing.T) { + m := &virtual.HelmMerger{} + body := []byte(`apiVersion: v1 +entries: + app: + - version: "1.0" + a: &a [x, x, x, x, x, x, x, x, x, x] + b: &b [*a, *a, *a, *a, *a, *a, *a, *a, *a, *a] + c: &c [*b, *b, *b, *b, *b, *b, *b, *b, *b, *b] + d: &d [*c, *c, *c, *c, *c, *c, *c, *c, *c, *c] + e: &e [*d, *d, *d, *d, *d, *d, *d, *d, *d, *d] + urls: [app-1.0.tgz] +`) + result, err := m.MergeIndexes([]virtual.MemberIndex{{RemoteName: "a", RepoType: "local", Body: body}}, "https://proxy.example.com") + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(result), "app-1.0.tgz") { + t.Errorf("alias bomb entry was kept") + } +}