Guard allocation size calculations flagged by CodeQL

docs/adr/55480-safe-allocation-capacity-in-typeutil.md

@@ -0,0 +1,54 @@
+# ADR-55480: Centralize Safe Allocation Capacity Calculation in typeutil
+
+**Date**: 2026-08-24
+**Status**: Accepted
+**Deciders**: Copilot
+
+---
+
+### Context
+
+CodeQL flagged several `go/allocation-size-overflow` paths where summed `len(...)` values were passed directly as allocation capacity hints. If extremely large or malformed inputs caused those sums to overflow `int`, the resulting capacity could become negative or otherwise unsafe before reaching `make`.
+
+The affected call sites were in multiple packages, including `pkg/workflow` and `pkg/cli`, so a package-private helper would either duplicate the overflow logic or leave future call sites without a shared convention.
+
+### Decision
+
+We will provide `typeutil.SafeAllocationCapacity(parts ...int) int` as the shared helper for allocation capacity hints built from multiple integer parts. The helper returns the summed capacity when every part is non-negative and the addition does not overflow; otherwise it returns zero so callers still allocate correctly without unsafe preallocation.
+
+Call sites that previously used direct additive capacity expressions will use this shared helper when summing length-derived allocation hints across packages.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep package-local helpers
+
+Keeping separate helpers in `pkg/workflow` and `pkg/cli` avoids a new shared API, but it duplicates security-sensitive overflow handling and lets behavior drift between packages.
+
+#### Alternative 2: Inline overflow checks at each allocation site
+
+Inlining checks keeps each call site self-contained, but it makes the overflow policy harder to audit and increases the chance that a future allocation hint misses one of the required checks.
+
+#### Alternative 3: Remove capacity hints entirely
+
+Removing all summed capacity hints would also avoid overflow, but it discards useful preallocation for normal inputs and obscures the intended size relationship between the source collections and the destination allocation.
+
+### Consequences
+
+#### Positive
+
+- CodeQL-flagged allocation capacity calculations now use a single overflow-safe helper.
+- The zero-capacity fallback preserves correctness while avoiding unsafe preallocation on overflow or negative input.
+- Future callers have one reusable helper for length-derived allocation hints.
+
+#### Negative
+
+- `pkg/typeutil` gains a small public API that should keep its current overflow semantics stable.
+
+#### Neutral
+
+- Valid inputs preserve the existing capacity hint behavior.
+- Overflow and negative inputs may allocate with default growth instead of the original precomputed capacity.
+
+---
+
+*ADR finalized after implementation and CodeQL review.*

pkg/cli/experiments_analyze_statistics.go

@@ -11,6 +11,7 @@ import (
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/sliceutil"
+	"github.com/github/gh-aw/pkg/typeutil"
 	"github.com/github/gh-aw/pkg/workflow"
 )
 
@@ -428,7 +429,7 @@ func experimentVariantCounts(exp ExperimentVariantStats, cfg *workflow.Experimen
 	if !includeDeclared || cfg == nil {
 		return exp.Variants
 	}
-	counts := make(map[string]int, len(exp.Variants)+len(cfg.Variants))
+	counts := make(map[string]int, typeutil.SafeAllocationCapacity(len(exp.Variants), len(cfg.Variants)))
 	maps.Copy(counts, exp.Variants)
 	for _, name := range cfg.Variants {
 		if _, ok := counts[name]; !ok {

pkg/cli/experiments_analyze_statistics_test.go

@@ -174,6 +174,46 @@ func TestExpectedProportions(t *testing.T) {
 	})
 }
 
+func TestExperimentVariantCounts(t *testing.T) {
+	t.Parallel()
+
+	t.Run("includes declared variants with zero counts", func(t *testing.T) {
+		exp := ExperimentVariantStats{
+			Variants: map[string]int{"control": 3},
+		}
+		cfg := &workflow.ExperimentConfig{
+			Variants: []string{"control", "candidate"},
+		}
+
+		got := experimentVariantCounts(exp, cfg, true)
+
+		assert.Equal(t, map[string]int{"control": 3, "candidate": 0}, got)
+	})
+
+	t.Run("returns observed variants when declared variants are excluded", func(t *testing.T) {
+		exp := ExperimentVariantStats{
+			Variants: map[string]int{"control": 3},
+		}
+		cfg := &workflow.ExperimentConfig{
+			Variants: []string{"control", "candidate"},
+		}
+
+		got := experimentVariantCounts(exp, cfg, false)
+
+		assert.Equal(t, exp.Variants, got)
+	})
+
+	t.Run("returns observed variants when config is nil", func(t *testing.T) {
+		exp := ExperimentVariantStats{
+			Variants: map[string]int{"control": 3},
+		}
+
+		got := experimentVariantCounts(exp, nil, true)
+
+		assert.Equal(t, exp.Variants, got)
+	})
+}
+
 // TestComputeExperimentAnalysis verifies the end-to-end statistical computation.
 func TestComputeExperimentAnalysis(t *testing.T) {
 	t.Parallel()

pkg/typeutil/allocation.go

@@ -0,0 +1,18 @@
+package typeutil
+
+import "math"
+
+// SafeAllocationCapacity returns the summed capacity hint when it fits in int.
+// When the total would overflow, it falls back to 0 so callers can skip
+// preallocation without changing correctness. The helper is intentionally
+// side-effect free so utility callers do not inherit logging dependencies.
+func SafeAllocationCapacity(parts ...int) int {
+	total := 0
+	for _, part := range parts {
+		if part < 0 || total > math.MaxInt-part {
+			return 0
+		}
+		total += part
+	}
+	return total
+}

pkg/typeutil/allocation_test.go

@@ -0,0 +1,39 @@
+//go:build !integration
+
+package typeutil
+
+import (
+	"math"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSafeAllocationCapacity(t *testing.T) {
+	t.Parallel()
+
+	t.Run("handles zero inputs", func(t *testing.T) {
+		assert.Zero(t, SafeAllocationCapacity())
+		assert.Zero(t, SafeAllocationCapacity(0, 0))
+		assert.Equal(t, 5, SafeAllocationCapacity(0, 5))
+		assert.Equal(t, 5, SafeAllocationCapacity(5, 0))
+	})
+
+	t.Run("sums sizes when the result fits in int", func(t *testing.T) {
+		assert.Equal(t, 5, SafeAllocationCapacity(2, 3))
+		assert.Equal(t, 6000, SafeAllocationCapacity(1000, 5000))
+		assert.Equal(t, math.MaxInt, SafeAllocationCapacity(math.MaxInt-1, 1))
+		assert.Equal(t, math.MaxInt, SafeAllocationCapacity(math.MaxInt-2, 1, 1))
+	})
+
+	t.Run("returns zero when the sum would overflow int", func(t *testing.T) {
+		assert.Zero(t, SafeAllocationCapacity(math.MaxInt, 1))
+		assert.Zero(t, SafeAllocationCapacity(math.MaxInt-1, 2))
+		assert.Zero(t, SafeAllocationCapacity(math.MaxInt-2, 2, 1))
+	})
+
+	t.Run("returns zero for negative parts", func(t *testing.T) {
+		assert.Zero(t, SafeAllocationCapacity(-1))
+		assert.Zero(t, SafeAllocationCapacity(2, -1))
+	})
+}

pkg/workflow/awf_helpers.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var awfHelpersLog = logger.New("workflow:awf_helpers")
@@ -120,7 +121,7 @@ func buildModelsJSONPathExportScript(isArcDind bool) string {
 func buildWorkflowCallNetworkAllowedUpdateScript() (string, error) {
 	ecosystemDomains := getLoadedEcosystemDomains()
 	awfHelpersLog.Printf("buildWorkflowCallNetworkAllowedUpdateScript: ecosystems=%d, compoundEcosystems=%d", len(ecosystemDomains), len(compoundEcosystems))
-	ecosystemMap := make(map[string][]string, safeAllocationCapacity(len(ecosystemDomains), len(compoundEcosystems)))
+	ecosystemMap := make(map[string][]string, typeutil.SafeAllocationCapacity(len(ecosystemDomains), len(compoundEcosystems)))
 	for ecosystem := range ecosystemDomains {
 		ecosystemMap[ecosystem] = getEcosystemDomains(ecosystem)
 	}

pkg/workflow/compiler_activation_job.go

@@ -6,11 +6,11 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/goccy/go-yaml"
-
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/setutil"
+	"github.com/github/gh-aw/pkg/typeutil"
+	"github.com/goccy/go-yaml"
 )
 
 var compilerActivationJobLog = logger.New("workflow:compiler_activation_job")
@@ -610,7 +610,7 @@ func injectIfConditionAfterName(step, condition string) string {
 		fieldIndent = nameIndent + "  "
 	}
 
-	newLines := make([]string, 0, safeAllocationCapacity(len(lines), 1))
+	newLines := make([]string, 0, typeutil.SafeAllocationCapacity(len(lines), 1))
 	newLines = append(newLines, lines[:nameLineIdx+1]...)
 	newLines = append(newLines, fieldIndent+"if: "+condition)
 	newLines = append(newLines, lines[nameLineIdx+1:]...)

pkg/workflow/compiler_aw_context.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var awContextLog = logger.New("workflow:compiler_aw_context")
@@ -118,7 +119,7 @@ func injectInputIntoTrigger(onSection string, triggerName string, inputName stri
 
 	inputLines := buildInputLines(triggerIndent)
 
-	result := make([]string, 0, safeAllocationCapacity(len(lines), len(inputLines), 1))
+	result := make([]string, 0, typeutil.SafeAllocationCapacity(len(lines), len(inputLines), 1))
 	for i, line := range lines {
 		// When the trigger line contains an explicit null/~ value,
 		// replace it with a bare trigger so sub-keys can follow.

pkg/workflow/compiler_builtin_job_augmentation.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 func (c *Compiler) applyBuiltinJobPreSteps(data *WorkflowData) error {
@@ -97,7 +98,7 @@ func insertActivationStepsBeforeArtifactStaging(jobName string, steps []string,
 		}
 	}
 
-	result := make([]string, 0, safeAllocationCapacity(len(steps), len(activationSteps)))
+	result := make([]string, 0, typeutil.SafeAllocationCapacity(len(steps), len(activationSteps)))
 	result = append(result, steps[:insertIdx]...)
 	result = append(result, activationSteps...)
 	result = append(result, steps[insertIdx:]...)

pkg/workflow/compiler_job_step_helpers.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var exactSetupStepIDPattern = regexp.MustCompile(`(?m)^\s*id:\s*setup\s*$`)
@@ -71,7 +72,7 @@ func insertSetupStepsAtStart(steps []string, setupSteps []string) []string {
 		return steps
 	}
 
-	result := make([]string, 0, safeAllocationCapacity(len(steps), len(setupSteps)))
+	result := make([]string, 0, typeutil.SafeAllocationCapacity(len(steps), len(setupSteps)))
 	result = append(result, setupSteps...)
 	result = append(result, steps...)
 	return result
@@ -143,7 +144,7 @@ func insertPreStepsAtEarliestBoundary(steps []string, preSteps []string) []strin
 		insertIdx = len(steps)
 	}
 
-	result := make([]string, 0, safeAllocationCapacity(len(steps), len(preSteps)))
+	result := make([]string, 0, typeutil.SafeAllocationCapacity(len(steps), len(preSteps)))
 	result = append(result, steps[:insertIdx]...)
 	result = append(result, preSteps...)
 	result = append(result, steps[insertIdx:]...)

pkg/workflow/concurrency.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var concurrencyLog = logger.New("workflow:concurrency")
@@ -182,7 +183,7 @@ func isSlashCommandWorkflow(on string) bool {
 // inserted between the primary identifiers and the tail, providing a stable per-item
 // key for manual workflow_dispatch runs triggered via the label trigger shorthand.
 func entityConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumber bool) string {
-	parts := make([]string, 0, safeAllocationCapacity(len(primaryParts), len(tailParts), 1))
+	parts := make([]string, 0, typeutil.SafeAllocationCapacity(len(primaryParts), len(tailParts), 1))
 	parts = append(parts, primaryParts...)
 	if hasItemNumber {
 		parts = append(parts, "inputs.item_number")
@@ -197,7 +198,7 @@ func entityConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumb
 // When contains(github.actor, '[bot]') is true, the expression short-circuits to
 // github.run_id so that bot-triggered runs never share a group with human runs.
 func botIsolatedConcurrencyKey(primaryParts []string, tailParts []string, hasItemNumber bool) string {
-	parts := make([]string, 0, safeAllocationCapacity(len(primaryParts), len(tailParts), 2))
+	parts := make([]string, 0, typeutil.SafeAllocationCapacity(len(primaryParts), len(tailParts), 2))
 	// Prepend the bot-actor isolation check: bot runs always get a unique key
 	parts = append(parts, "contains(github.actor, '[bot]') && github.run_id")
 	parts = append(parts, primaryParts...)

pkg/workflow/domains.go

@@ -12,6 +12,7 @@ import (
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/sliceutil"
 	"github.com/github/gh-aw/pkg/stringutil"
+	"github.com/github/gh-aw/pkg/typeutil"
 )
 
 var domainsLog = logger.New("workflow:domains")
@@ -176,7 +177,7 @@ func getPiDefaultDomains(model string) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-	domains := make([]string, 0, safeAllocationCapacity(len(PiBaseDefaultDomains), 1))
+	domains := make([]string, 0, typeutil.SafeAllocationCapacity(len(PiBaseDefaultDomains), 1))
 	domains = append(domains, PiBaseDefaultDomains...)
 
 	if domain, ok := piProviderDomains[provider]; ok {
@@ -608,7 +609,7 @@ func resolveEngineNetworkDomains(network *EngineNetworkDefinition, model string)
 	if provider == "" {
 		provider = network.DefaultProvider
 	}
-	domains := make([]string, 0, safeAllocationCapacity(len(network.Defaults), 1))
+	domains := make([]string, 0, typeutil.SafeAllocationCapacity(len(network.Defaults), 1))
 	domains = append(domains, network.Defaults...)
 	if domain, ok := network.ProviderDomains[provider]; ok {
 		domains = append(domains, domain)