diff --git a/docs/adr/55480-safe-allocation-capacity-in-typeutil.md b/docs/adr/55480-safe-allocation-capacity-in-typeutil.md new file mode 100644 index 00000000000..ff0f2b46f6c --- /dev/null +++ b/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.* diff --git a/pkg/cli/experiments_analyze_statistics.go b/pkg/cli/experiments_analyze_statistics.go index 229086a0ca0..6022c9b3d4a 100644 --- a/pkg/cli/experiments_analyze_statistics.go +++ b/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 { diff --git a/pkg/cli/experiments_analyze_statistics_test.go b/pkg/cli/experiments_analyze_statistics_test.go index c0815e9855e..5b7209c918f 100644 --- a/pkg/cli/experiments_analyze_statistics_test.go +++ b/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() diff --git a/pkg/typeutil/allocation.go b/pkg/typeutil/allocation.go new file mode 100644 index 00000000000..1edd5992150 --- /dev/null +++ b/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 +} diff --git a/pkg/typeutil/allocation_test.go b/pkg/typeutil/allocation_test.go new file mode 100644 index 00000000000..9921ed5aff3 --- /dev/null +++ b/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)) + }) +} diff --git a/pkg/workflow/allocation_helpers.go b/pkg/workflow/allocation_helpers.go deleted file mode 100644 index 7121be6188d..00000000000 --- a/pkg/workflow/allocation_helpers.go +++ /dev/null @@ -1,24 +0,0 @@ -package workflow - -import ( - "math" - - "github.com/github/gh-aw/pkg/logger" -) - -var allocationLog = logger.New("workflow:allocation_helpers") - -// 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. -func safeAllocationCapacity(parts ...int) int { - total := 0 - for _, part := range parts { - if part < 0 || total > math.MaxInt-part { - allocationLog.Printf("Capacity hint overflow or negative part (part=%d, running total=%d), falling back to 0", part, total) - return 0 - } - total += part - } - return total -} diff --git a/pkg/workflow/allocation_helpers_test.go b/pkg/workflow/allocation_helpers_test.go deleted file mode 100644 index 1a0f109d2e4..00000000000 --- a/pkg/workflow/allocation_helpers_test.go +++ /dev/null @@ -1,37 +0,0 @@ -//go:build !integration - -package workflow - -import ( - "math" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestSafeAllocationCapacity(t *testing.T) { - 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)) - }) -} diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index f49a4b8c9e3..03eeeafe031 100644 --- a/pkg/workflow/awf_helpers.go +++ b/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) } diff --git a/pkg/workflow/compiler_activation_job.go b/pkg/workflow/compiler_activation_job.go index 67d6e35061b..b1473b3c72a 100644 --- a/pkg/workflow/compiler_activation_job.go +++ b/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:]...) diff --git a/pkg/workflow/compiler_aw_context.go b/pkg/workflow/compiler_aw_context.go index 0e57b7f7c46..ebc17cc9431 100644 --- a/pkg/workflow/compiler_aw_context.go +++ b/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. diff --git a/pkg/workflow/compiler_builtin_job_augmentation.go b/pkg/workflow/compiler_builtin_job_augmentation.go index 46c7c7f852e..4bc2fe49d05 100644 --- a/pkg/workflow/compiler_builtin_job_augmentation.go +++ b/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:]...) diff --git a/pkg/workflow/compiler_job_step_helpers.go b/pkg/workflow/compiler_job_step_helpers.go index b6ddf929063..fe5ef251f5f 100644 --- a/pkg/workflow/compiler_job_step_helpers.go +++ b/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:]...) diff --git a/pkg/workflow/concurrency.go b/pkg/workflow/concurrency.go index 3cf471e6961..a9b8329ac82 100644 --- a/pkg/workflow/concurrency.go +++ b/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...) diff --git a/pkg/workflow/domains.go b/pkg/workflow/domains.go index 9ebe4f58e3f..44afa571ec8 100644 --- a/pkg/workflow/domains.go +++ b/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) diff --git a/pkg/workflow/known_action_credentials.go b/pkg/workflow/known_action_credentials.go index 054ccb62e0b..039ae356fdb 100644 --- a/pkg/workflow/known_action_credentials.go +++ b/pkg/workflow/known_action_credentials.go @@ -30,9 +30,9 @@ import ( "maps" "strings" - "github.com/goccy/go-yaml" - "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/typeutil" + "github.com/goccy/go-yaml" ) var knownActionCredentialsLog = logger.New("workflow:known_action_credentials") @@ -199,7 +199,7 @@ func mergeKnownActionEnvVars(a, b map[string]struct { return nil } merged := make(map[string]struct { - }, safeAllocationCapacity(len(a), len(b))) + }, typeutil.SafeAllocationCapacity(len(a), len(b))) maps.Copy(merged, a) maps.Copy(merged, b) return merged diff --git a/pkg/workflow/mcp_setup_safe_outputs.go b/pkg/workflow/mcp_setup_safe_outputs.go index 1bdaa3b45c5..d63b3cdd64c 100644 --- a/pkg/workflow/mcp_setup_safe_outputs.go +++ b/pkg/workflow/mcp_setup_safe_outputs.go @@ -8,6 +8,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/sliceutil" + "github.com/github/gh-aw/pkg/typeutil" ) var safeOutputsSetupLog = logger.New("workflow:mcp_setup_safe_outputs") @@ -131,7 +132,7 @@ func buildSafeOutputsConfigRuntimeEnvVars(safeOutputConfig string) ([]string, ma configSecrets := ExtractSecretsFromValue(safeOutputConfig) configContextVars := ExtractGitHubContextExpressionsFromValue(safeOutputConfig) configWorkflowInputs := ExtractWorkflowInputExpressionsFromValue(safeOutputConfig) - envValues := make(map[string]string, safeAllocationCapacity(len(configSecrets), len(configContextVars), len(configWorkflowInputs))) + envValues := make(map[string]string, typeutil.SafeAllocationCapacity(len(configSecrets), len(configContextVars), len(configWorkflowInputs))) addEnvValue := func(key, value string) { envValues[key] = value } diff --git a/pkg/workflow/network_firewall_validation.go b/pkg/workflow/network_firewall_validation.go index ced6c14c8b4..20ed64774a4 100644 --- a/pkg/workflow/network_firewall_validation.go +++ b/pkg/workflow/network_firewall_validation.go @@ -20,6 +20,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/typeutil" ) var networkFirewallValidationLog = logger.New("workflow:network_firewall_validation") @@ -156,7 +157,7 @@ func isKnownEcosystemIdentifier(id string) bool { // including both the base identifiers from ecosystemDomains and compound identifiers. func getValidEcosystemIdentifiers() []string { ecosystemDomains := getLoadedEcosystemDomains() - ids := make([]string, 0, safeAllocationCapacity(len(ecosystemDomains), len(compoundEcosystems))) + ids := make([]string, 0, typeutil.SafeAllocationCapacity(len(ecosystemDomains), len(compoundEcosystems))) for id := range ecosystemDomains { ids = append(ids, id) } diff --git a/pkg/workflow/observability_otlp.go b/pkg/workflow/observability_otlp.go index e8bd432e452..de1eb482edd 100644 --- a/pkg/workflow/observability_otlp.go +++ b/pkg/workflow/observability_otlp.go @@ -11,6 +11,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/sliceutil" + "github.com/github/gh-aw/pkg/typeutil" ) var otlpLog = logger.New("workflow:observability_otlp") @@ -539,7 +540,7 @@ func mergeOTLPStringMaps(base, override map[string]string) map[string]string { if len(base) == 0 && len(override) == 0 { return nil } - merged := make(map[string]string, safeAllocationCapacity(len(base), len(override))) + merged := make(map[string]string, typeutil.SafeAllocationCapacity(len(base), len(override))) maps.Copy(merged, override) // base takes precedence maps.Copy(merged, base) diff --git a/pkg/workflow/permissions.go b/pkg/workflow/permissions.go index 7896f79c18b..35d4c76e2e2 100644 --- a/pkg/workflow/permissions.go +++ b/pkg/workflow/permissions.go @@ -4,6 +4,7 @@ import ( "slices" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/typeutil" ) var permissionsLog = logger.New("workflow:permissions") @@ -12,7 +13,7 @@ var validPermissionScopes = func() map[string]struct{} { scopes := GetAllPermissionScopes() appOnlyScopes := GetAllGitHubAppOnlyScopes() - m := make(map[string]struct{}, safeAllocationCapacity(len(scopes), len(appOnlyScopes), 1)) + m := make(map[string]struct{}, typeutil.SafeAllocationCapacity(len(scopes), len(appOnlyScopes), 1)) for _, scope := range scopes { m[string(scope)] = struct{}{} } diff --git a/pkg/workflow/permissions_validation.go b/pkg/workflow/permissions_validation.go index 9b312d8a71c..aa785b846a0 100644 --- a/pkg/workflow/permissions_validation.go +++ b/pkg/workflow/permissions_validation.go @@ -14,6 +14,7 @@ import ( "github.com/github/gh-aw/pkg/setutil" "github.com/github/gh-aw/pkg/sliceutil" "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/typeutil" "github.com/goccy/go-yaml" ) @@ -21,7 +22,7 @@ var allPermissionScopeNames = sync.OnceValue(func() []string { ghTokenScopes := GetAllPermissionScopes() appOnlyScopes := GetAllGitHubAppOnlyScopes() // +1 for copilot-requests which is not in GetAllPermissionScopes - all := make([]string, 0, safeAllocationCapacity(len(ghTokenScopes), len(appOnlyScopes), 1)) + all := make([]string, 0, typeutil.SafeAllocationCapacity(len(ghTokenScopes), len(appOnlyScopes), 1)) for _, scope := range ghTokenScopes { all = append(all, string(scope)) } diff --git a/pkg/workflow/run_step_sanitizer.go b/pkg/workflow/run_step_sanitizer.go index 645ab1a3cf2..1742efe21f2 100644 --- a/pkg/workflow/run_step_sanitizer.go +++ b/pkg/workflow/run_step_sanitizer.go @@ -50,10 +50,10 @@ import ( "slices" "strings" - "github.com/goccy/go-yaml" - "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 runStepSanitizerLog = logger.New("workflow:run_step_sanitizer") @@ -147,7 +147,7 @@ func sanitizeRunStepExpressions(step map[string]any) (map[string]any, []string, // - If it exists with a different value → pick an alternate name by appending // a numeric suffix (_2, _3, …) so the original user-defined value is preserved. existingEnv, _ := step["env"].(map[string]any) - newEnv := make(map[string]any, safeAllocationCapacity(len(existingEnv), len(ordered))) + newEnv := make(map[string]any, typeutil.SafeAllocationCapacity(len(existingEnv), len(ordered))) maps.Copy(newEnv, existingEnv) for i := range ordered { diff --git a/pkg/workflow/safe_jobs_needs_validation.go b/pkg/workflow/safe_jobs_needs_validation.go index f5e827eb30c..5e8f66618ee 100644 --- a/pkg/workflow/safe_jobs_needs_validation.go +++ b/pkg/workflow/safe_jobs_needs_validation.go @@ -9,6 +9,7 @@ import ( "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/setutil" "github.com/github/gh-aw/pkg/stringutil" + "github.com/github/gh-aw/pkg/typeutil" ) var safeJobsNeedsValidationLog = logger.New("workflow:safe_jobs_needs_validation") @@ -195,7 +196,7 @@ func detectSafeJobCycles(jobs map[string]*SafeJobConfig) error { } if state[node] == visiting { // Build the cycle description using original names where available - cycleNodes := make([]string, 0, safeAllocationCapacity(len(path), 1)) + cycleNodes := make([]string, 0, typeutil.SafeAllocationCapacity(len(path), 1)) for _, p := range path { if orig, ok := originalNames[p]; ok { cycleNodes = append(cycleNodes, orig) diff --git a/pkg/workflow/safe_output_handlers.go b/pkg/workflow/safe_output_handlers.go index 44d05276155..18b90ae6982 100644 --- a/pkg/workflow/safe_output_handlers.go +++ b/pkg/workflow/safe_output_handlers.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/typeutil" ) var safeOutputHandlerLog = logger.New("workflow:safe_output_handlers") @@ -621,7 +622,7 @@ var safeOutputHandlers = []safeOutputHandlerDescriptor{ var safeOutputHandlersByKey = buildSafeOutputHandlersByKey() func buildSafeOutputHandlersByKey() map[string]safeOutputHandlerDescriptor { - result := make(map[string]safeOutputHandlerDescriptor, safeAllocationCapacity(len(safeOutputHandlers), 1)) + result := make(map[string]safeOutputHandlerDescriptor, typeutil.SafeAllocationCapacity(len(safeOutputHandlers), 1)) for _, handler := range safeOutputHandlers { if handler.Key != "" { result[handler.Key] = handler diff --git a/pkg/workflow/threat_detection_steps.go b/pkg/workflow/threat_detection_steps.go index 7a1ca51fd36..d6e4ce645f4 100644 --- a/pkg/workflow/threat_detection_steps.go +++ b/pkg/workflow/threat_detection_steps.go @@ -7,6 +7,7 @@ import ( "strconv" "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/typeutil" ) // buildDetectionJobSteps builds the threat detection steps to be run in the separate detection job. @@ -483,7 +484,7 @@ func (c *Compiler) buildCustomThreatDetectionSteps(steps []any) []string { // Inject the detection guard condition unless the user already provided an if: condition. if _, hasIf := stepMap["if"]; !hasIf { // Clone the map to avoid mutating the original config. - injected := make(map[string]any, safeAllocationCapacity(len(stepMap), 1)) + injected := make(map[string]any, typeutil.SafeAllocationCapacity(len(stepMap), 1)) maps.Copy(injected, stepMap) injected["if"] = detectionStepCondition stepMap = injected diff --git a/pkg/workflow/tools.go b/pkg/workflow/tools.go index 53f5a25fafd..280ce308134 100644 --- a/pkg/workflow/tools.go +++ b/pkg/workflow/tools.go @@ -11,6 +11,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/typeutil" "github.com/github/gh-aw/pkg/workflow/compilerenv" "github.com/goccy/go-yaml" ) @@ -262,11 +263,11 @@ func (c *Compiler) buildCommandTriggerEventsMap(data *WorkflowData) (map[string] if existingMap, ok := existingAny.(map[string]any); ok { switch t := existingMap["types"].(type) { case []string: - newTypes := make([]any, len(t)+1) - for i, s := range t { - newTypes[i] = s + newTypes := make([]any, 0, typeutil.SafeAllocationCapacity(len(t), 1)) + for _, s := range t { + newTypes = append(newTypes, s) } - newTypes[len(t)] = "labeled" + newTypes = append(newTypes, "labeled") existingMap["types"] = newTypes case []any: existingMap["types"] = append(t, "labeled") @@ -348,7 +349,7 @@ func mergeLabelCommandOtherEvents(labelEventsMap map[string]any, otherEvents map if existingMap != nil && userMap != nil { existingTypes, _ := existingMap["types"].([]any) userTypes, _ := userMap["types"].([]any) - merged := make([]any, 0, safeAllocationCapacity(len(existingTypes), len(userTypes))) + merged := make([]any, 0, typeutil.SafeAllocationCapacity(len(existingTypes), len(userTypes))) merged = append(merged, existingTypes...) merged = append(merged, userTypes...) existingMap["types"] = merged @@ -759,7 +760,7 @@ func ensureGitHubAllowedTool(githubConfig map[string]any, tool string) { } func appendStringAny(values []string, value string) []any { - result := make([]any, 0, len(values)+1) + result := make([]any, 0, typeutil.SafeAllocationCapacity(len(values), 1)) for _, existing := range values { result = append(result, existing) } diff --git a/pkg/workflow/workflow_import_merge.go b/pkg/workflow/workflow_import_merge.go index cb6d885fd7c..e3dfcec87bd 100644 --- a/pkg/workflow/workflow_import_merge.go +++ b/pkg/workflow/workflow_import_merge.go @@ -11,6 +11,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/typeutil" "github.com/goccy/go-yaml" ) @@ -208,7 +209,7 @@ func mergeJobStepField(mainJob map[string]any, importedJob map[string]any, field return append([]any(nil), mainSteps...), true } - mergedSteps := make([]any, 0, safeAllocationCapacity(len(importedSteps), len(mainSteps))) + mergedSteps := make([]any, 0, typeutil.SafeAllocationCapacity(len(importedSteps), len(mainSteps))) mergedSteps = append(mergedSteps, importedSteps...) mergedSteps = append(mergedSteps, mainSteps...) return mergedSteps, true