From 1ac8f60196fe47995a44eea49d72ff621a541de1 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 17 Oct 2026 12:52:35 +1100 Subject: [PATCH] Count only alias-reached nodes and expand chart aliases before index overrides --- internal/provider/helm/helm.go | 11 +++-- internal/provider/helm/helm_local_test.go | 19 +++++++++ internal/provider/yamlalias.go | 46 +++++++++++++++++++++ internal/virtual/helm_merger.go | 50 ++++------------------- internal/virtual/helm_merger_test.go | 29 +++++++++++++ 5 files changed, 110 insertions(+), 45 deletions(-) create mode 100644 internal/provider/yamlalias.go diff --git a/internal/provider/helm/helm.go b/internal/provider/helm/helm.go index a39ae95..4904ff6 100644 --- a/internal/provider/helm/helm.go +++ b/internal/provider/helm/helm.go @@ -167,13 +167,16 @@ func (p *Provider) GenerateLocalIndex(ctx context.Context, files provider.FileSt func generateIndex(metas []provider.HelmMetadata) ([]byte, error) { entries := map[string][]map[string]yaml.Node{} var newest time.Time + budget := provider.MaxAliasExpandedNodes for _, m := range metas { - var chart map[string]yaml.Node - if err := yaml.Unmarshal([]byte(m.ChartYAML), &chart); err != nil { + var raw map[string]yaml.Node + if err := yaml.Unmarshal([]byte(m.ChartYAML), &raw); err != nil { return nil, fmt.Errorf("chart %s: %w", m.FilePath, err) } - if chart == nil { - chart = map[string]yaml.Node{} + chart, err := provider.ExpandYAMLAliases(raw, &budget) + if err != nil { + slog.Warn("helm index: dropping chart", "chart", m.Name, "path", m.FilePath, "error", err) + continue } chart["name"] = node(m.Name) chart["version"] = node(m.Version) diff --git a/internal/provider/helm/helm_local_test.go b/internal/provider/helm/helm_local_test.go index 4e8c775..abe2538 100644 --- a/internal/provider/helm/helm_local_test.go +++ b/internal/provider/helm/helm_local_test.go @@ -215,3 +215,22 @@ func TestIndexEscapesURLs(t *testing.T) { t.Errorf("url not escaped:\n%s", out) } } + +func TestIndexExpandsAliasesBeforeOverride(t *testing.T) { + body, err := generateIndex([]provider.HelmMetadata{{ + FilePath: "app-1.0.0.tgz", ContentHash: "sha256:aa", Name: "app", Version: "1.0.0", + ChartYAML: "apiVersion: v2\nname: app\nversion: &v 1.0.0\nappVersion: *v\n", + }}) + if err != nil { + t.Fatal(err) + } + var idx struct { + Entries map[string][]map[string]any + } + if err := yaml.Unmarshal(body, &idx); err != nil { + t.Fatalf("index does not parse: %v\n%s", err, body) + } + if e := idx.Entries["app"]; len(e) != 1 || e[0]["appVersion"] != "1.0.0" { + t.Errorf("entries %v", idx.Entries) + } +} diff --git a/internal/provider/yamlalias.go b/internal/provider/yamlalias.go new file mode 100644 index 0000000..af6f719 --- /dev/null +++ b/internal/provider/yamlalias.go @@ -0,0 +1,46 @@ +package provider + +import ( + "errors" + + "gopkg.in/yaml.v3" +) + +const MaxAliasExpandedNodes = 100000 + +// ExpandYAMLAliases inlines alias targets and drops anchors so a node map +// stays valid when copied away from the document that held its anchors; +// budget caps nodes reached through aliases against alias bombs. +func ExpandYAMLAliases(m map[string]yaml.Node, budget *int) (map[string]yaml.Node, error) { + var expand func(n *yaml.Node, viaAlias bool) (*yaml.Node, error) + expand = func(n *yaml.Node, viaAlias bool) (*yaml.Node, error) { + if viaAlias { + *budget-- + if *budget < 0 { + return nil, errors.New("yaml aliases expand to too many nodes") + } + } + if n.Kind == yaml.AliasNode { + return expand(n.Alias, true) + } + 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, viaAlias); err != nil { + return nil, err + } + } + return &c, nil + } + out := make(map[string]yaml.Node, len(m)) + for k, v := range m { + e, err := expand(&v, false) + if err != nil { + return nil, err + } + out[k] = *e + } + return out, nil +} diff --git a/internal/virtual/helm_merger.go b/internal/virtual/helm_merger.go index f90e803..e7d9802 100644 --- a/internal/virtual/helm_merger.go +++ b/internal/virtual/helm_merger.go @@ -1,12 +1,15 @@ package virtual import ( - "errors" "fmt" + "log/slog" + "maps" + "slices" "strings" "gopkg.in/yaml.v3" + "git.unkin.net/unkin/artifactapi/internal/provider" "git.unkin.net/unkin/artifactapi/pkg/models" ) @@ -38,14 +41,16 @@ func (m *HelmMerger) MergeIndexes(members []MemberIndex, proxyBaseURL string) ([ continue } - budget := maxExpandedNodes - for chart, versions := range idx.Entries { + budget := provider.MaxAliasExpandedNodes + for _, chart := range slices.Sorted(maps.Keys(idx.Entries)) { + versions := idx.Entries[chart] if seen[chart] == nil { seen[chart] = map[string]bool{} } for _, raw := range versions { - ver, err := expandAliases(raw, &budget) + ver, err := provider.ExpandYAMLAliases(raw, &budget) if err != nil { + slog.Warn("helm merge: dropping chart entry", "member", member.RemoteName, "chart", chart, "error", err) continue } version := ver["version"].Value @@ -75,43 +80,6 @@ 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 6d762b3..c7908be 100644 --- a/internal/virtual/helm_merger_test.go +++ b/internal/virtual/helm_merger_test.go @@ -1,6 +1,7 @@ package virtual_test import ( + "fmt" "strings" "testing" @@ -222,3 +223,31 @@ entries: t.Errorf("alias bomb entry was kept") } } + +func TestHelmMerger_LargeIndexWithoutAliasesIsComplete(t *testing.T) { + var b strings.Builder + b.WriteString("apiVersion: v1\nentries:\n") + for c := 0; c < 100; c++ { + fmt.Fprintf(&b, " chart%d:\n", c) + for v := 0; v < 100; v++ { + fmt.Fprintf(&b, " - name: chart%d\n version: \"1.0.%d\"\n description: d\n maintainers: [{name: ops, email: ops@example.com}]\n urls: [chart%d-1.0.%d.tgz]\n", c, v, c, v) + } + } + result, err := (&virtual.HelmMerger{}).MergeIndexes([]virtual.MemberIndex{{RemoteName: "a", RepoType: "local", Body: []byte(b.String())}}, "https://proxy.example.com") + if err != nil { + t.Fatal(err) + } + var idx struct { + Entries map[string][]map[string]any `yaml:"entries"` + } + if err := yaml.Unmarshal(result, &idx); err != nil { + t.Fatal(err) + } + total := 0 + for _, vs := range idx.Entries { + total += len(vs) + } + if len(idx.Entries) != 100 || total != 10000 { + t.Errorf("got %d charts, %d versions; want 100, 10000", len(idx.Entries), total) + } +}