diff --git a/internal/formula/expand.go b/internal/formula/expand.go index d980f98f..4ca9d4c6 100644 --- a/internal/formula/expand.go +++ b/internal/formula/expand.go @@ -10,6 +10,9 @@ // // Templates use {target} and {target.description} placeholders that are // substituted with the target step's values during expansion. +// +// A maximum expansion depth (default 5) prevents runaway nested expansions. +// This allows massive work generation while providing a safety bound. package formula import ( @@ -17,6 +20,11 @@ import ( "strings" ) +// DefaultMaxExpansionDepth is the maximum depth for recursive template expansion. +// This prevents runaway nested expansions while still allowing substantial work +// generation. The limit applies to template children, not to expansion rules. +const DefaultMaxExpansionDepth = 5 + // ApplyExpansions applies all expand and map rules to a formula's steps. // Returns a new steps slice with expansions applied. // The original steps slice is not modified. @@ -64,8 +72,11 @@ func ApplyExpansions(steps []*Step, compose *ComposeRules, parser *Parser) ([]*S return nil, fmt.Errorf("expand: %q has no template steps", rule.With) } - // Expand the target step - expandedSteps := expandStep(targetStep, expFormula.Template) + // Expand the target step (start at depth 0) + expandedSteps, err := expandStep(targetStep, expFormula.Template, 0) + if err != nil { + return nil, fmt.Errorf("expand %q: %w", rule.Target, err) + } // Replace the target step with expanded steps result = replaceStep(result, rule.Target, expandedSteps) @@ -104,7 +115,10 @@ func ApplyExpansions(steps []*Step, compose *ComposeRules, parser *Parser) ([]*S // Expand each matching step for _, targetStep := range toExpand { - expandedSteps := expandStep(targetStep, expFormula.Template) + expandedSteps, err := expandStep(targetStep, expFormula.Template, 0) + if err != nil { + return nil, fmt.Errorf("map %q -> %q: %w", rule.Select, targetStep.ID, err) + } result = replaceStep(result, targetStep.ID, expandedSteps) expanded[targetStep.ID] = true @@ -121,7 +135,14 @@ func ApplyExpansions(steps []*Step, compose *ComposeRules, parser *Parser) ([]*S // expandStep expands a target step using the given template. // Returns the expanded steps with placeholders substituted. -func expandStep(target *Step, template []*Step) []*Step { +// The depth parameter tracks recursion depth for children; if it exceeds +// DefaultMaxExpansionDepth, an error is returned. +func expandStep(target *Step, template []*Step, depth int) ([]*Step, error) { + if depth > DefaultMaxExpansionDepth { + return nil, fmt.Errorf("expansion depth limit exceeded: max %d levels (currently at %d) - step %q", + DefaultMaxExpansionDepth, depth, target.ID) + } + result := make([]*Step, 0, len(template)) for _, tmpl := range template { @@ -157,15 +178,19 @@ func expandStep(target *Step, template []*Step) []*Step { } } - // Handle children recursively + // Handle children recursively with depth tracking if len(tmpl.Children) > 0 { - expanded.Children = expandStep(target, tmpl.Children) + children, err := expandStep(target, tmpl.Children, depth+1) + if err != nil { + return nil, err + } + expanded.Children = children } result = append(result, expanded) } - return result + return result, nil } // substituteTargetPlaceholders replaces {target} and {target.*} placeholders. diff --git a/internal/formula/expand_test.go b/internal/formula/expand_test.go index 54eb8006..0f4c6445 100644 --- a/internal/formula/expand_test.go +++ b/internal/formula/expand_test.go @@ -1,8 +1,10 @@ package formula import ( + "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -85,7 +87,10 @@ func TestExpandStep(t *testing.T) { }, } - result := expandStep(target, template) + result, err := expandStep(target, template, 0) + if err != nil { + t.Fatalf("expandStep failed: %v", err) + } if len(result) != 2 { t.Fatalf("expected 2 steps, got %d", len(result)) @@ -111,6 +116,59 @@ func TestExpandStep(t *testing.T) { } } +func TestExpandStepDepthLimit(t *testing.T) { + target := &Step{ + ID: "root", + Title: "Root step", + Description: "A deeply nested template", + } + + // Create a deeply nested template that exceeds the depth limit + // Build from inside out: depth 6 is the deepest + deepChild := &Step{ID: "level-6", Title: "Level 6"} + for i := 5; i >= 0; i-- { + deepChild = &Step{ + ID: fmt.Sprintf("level-%d", i), + Title: fmt.Sprintf("Level %d", i), + Children: []*Step{deepChild}, + } + } + + template := []*Step{deepChild} + + // With depth 0 start, going to level 6 means 7 levels total (0-6) + // DefaultMaxExpansionDepth is 5, so this should fail + _, err := expandStep(target, template, 0) + if err == nil { + t.Fatal("expected depth limit error, got nil") + } + + if !strings.Contains(err.Error(), "expansion depth limit exceeded") { + t.Errorf("expected depth limit error, got: %v", err) + } + + // Verify that templates within the limit succeed + // Build a 5-level deep template (levels 0-4, which is exactly at the limit) + shallowChild := &Step{ID: "level-4", Title: "Level 4"} + for i := 3; i >= 0; i-- { + shallowChild = &Step{ + ID: fmt.Sprintf("level-%d", i), + Title: fmt.Sprintf("Level %d", i), + Children: []*Step{shallowChild}, + } + } + + shallowTemplate := []*Step{shallowChild} + result, err := expandStep(target, shallowTemplate, 0) + if err != nil { + t.Fatalf("expected shallow template to succeed, got: %v", err) + } + + if len(result) != 1 { + t.Fatalf("expected 1 top-level step, got %d", len(result)) + } +} + func TestReplaceStep(t *testing.T) { steps := []*Step{ {ID: "design", Title: "Design"},