Count only alias-reached nodes and expand chart aliases before index overrides

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)

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

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

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

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