Bound chart parsing, require name-version filenames, expand index aliases

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/<repo>/files/<chart>-<version>.tgz` and `index.yaml` is
 generated from each chart's `Chart.yaml` (with sha256 digests) at
-`/api/v1/local/<repo>/index.yaml`. Add the local as a member of a helm virtual
-to serve it alongside remotes.
+`/api/v1/local/<repo>/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

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>/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"`

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)
+	}
+}

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" {

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")
+	}
+}