Guard allocation size calculations flagged by CodeQL

[Copilot]

CodeQL flagged potential go/allocation-size-overflow paths where summed len(...) values fed allocation sizes. Large or malformed inputs could overflow capacity calculations and cause unexpected allocation behavior.

• Overflow-safe allocation hints

  • Replaced direct additive capacity expressions with safeAllocationCapacity(...).
  • Applied this in pkg/workflow/tools.go and pkg/cli/experiments_analyze_statistics.go.

• CLI allocation helper

  • Added a CLI-local helper matching the existing workflow package behavior: overflow or negative parts fall back to zero capacity, preserving correctness while avoiding unsafe preallocation.

• Boundary coverage

  • Added tests for valid sums, integer overflow, and negative capacity parts.

Example:

counts := make(map[string]int, safeAllocationCapacity(len(exp.Variants), len(cfg.Variants)))

• Fixes [uk-ai-resilience] go/allocation-size-overflow findings in tools.go and experiments_analyze_statistics.go (Tier B) #55434

---

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 21.6 AIC · ⌖ 13.1 AIC · ⊞ 8.7K · ◷
_{Comment /souschef to run again}

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

❌ Design Decision Gate 🏗️ failed during design decision gate check.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

Completed PR review for #55480. No GitHub write action was needed because I found no actionable changed-line issues warranting review comments or a blocking review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Generated by Ponytail Reviewer for #55480

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs, /tdd, and /codebase-design — COMMENT (no blocking issues, minor improvements noted).

📋 Key Themes & Highlights

Key Themes

• Silent overflow fallback — the typeutil version drops the allocationLog.Printf present in the old workflow package. Consider a comment or call-site logging for observability.
• Test branch coverage — TestExperimentVariantCounts covers the main path; the two early-exit branches (!includeDeclared, cfg == nil) are untested.
• Test file naming — allocation_helpers_test.go should match the source file name allocation.go → allocation_test.go.

Positive Highlights

• ✅ Excellent extraction of safeAllocationCapacity into pkg/typeutil — correct response to the prior review comment.
• ✅ Variadic signature is clean and extensible.
• ✅ Overflow boundary tests (math.MaxInt edges) are thorough.
• ✅ Consistent rollout across all 20+ call sites with no functional changes.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 29 AIC · ⌖ 9.99 AIC · ⊞ 7.6K
_{Comment /matt to run again}

Comments that could not be inline-anchored

pkg/typeutil/allocation.go:13

[/diagnosing-bugs] Silent overflow fallback: the old pkg/workflow/allocation_helpers.go logged on overflow (allocationLog.Printf(...)); the new typeutil version drops that signal entirely. If a caller ever hits this path in production, there will be no trace in logs.

<details>
<summary>💡 Suggestion</summary>

Either add a comment explaining the intentional omission (e.g. import-cycle constraints), or thread a debug log through at the call site. The overflow case is rare but actionab…

pkg/cli/experiments_analyze_statistics_test.go:43

[/tdd] TestExperimentVariantCounts only tests the includeDeclared=true with a non-nil cfg path. The two short-circuit branches (!includeDeclared and cfg == nil) return exp.Variants directly and are untested here.

<details>
<summary>💡 Suggested additions</summary>

t.Run("returns exp.Variants when includeDeclared=false", func(t *testing.T) {
    got := experimentVariantCounts(exp, cfg, false)
    assert.Equal(t, exp.Variants, got)
})

t.Run("returns exp.Variants when cfg …

</details>

<details><summary>pkg/typeutil/allocation_helpers_test.go:1</summary>

**[/codebase-design]** Minor naming inconsistency: the source file is `pkg/typeutil/allocation.go` but the test file is named `allocation_helpers_test.go`. Go convention is `allocation_test.go` to match the source.

@copilot please address this.

</details>

[github-actions]

Clean and well-structured refactor. Moving safeAllocationCapacity to pkg/typeutil as exported SafeAllocationCapacity is the right call — it makes the guard reusable across packages without circular dependencies.

The one behavioral change is removal of the overflow log (allocationLog.Printf). This is appropriate for a utility package that should not carry a logger dependency, and overflow in these allocation hints only occurs at astronomically large slice sizes where the 0-fallback is safe.

Tests are thorough, t.Parallel() is correctly added, and all callers are updated. LGTM.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 23.6 AIC · ⌖ 8.99 AIC · ⊞ 6.2K

[github-actions]

One small simplification opportunity: a shared helper for this single overflow-check pattern is more abstraction than the fix needs. net: -14 lines possible.

Generated by ✂️ Ponytail Reviewer for #55480 · codex · mai10 · 5.19 AIC · ⌖ 1.3 AIC · ⊞ 18.3K
_{Comment /ponytail to run again}

[github-actions]

pkg/typeutil/allocation.go:L8: yagni: exported varargs helper for a single overflow-check pattern. Inline the small sum/overflow guard in the few sites that need it.

[Copilot]

Kept the shared helper because the maintainer requested moving this overflow guard into a helper package, and the pattern now has multiple call sites. Added ADR-55480 to document the decision and tradeoff.

[Copilot]

Pull request overview

Centralizes overflow-safe allocation sizing and applies it to CodeQL-flagged paths.

Changes:

• Adds and tests typeutil.SafeAllocationCapacity.
• Migrates workflow allocation hints to the shared helper.
• Protects experiment variant-count allocation from overflow.

Show a summary per file

File	Description
pkg/workflow/workflow_import_merge.go	Uses safe merged-step capacity.
pkg/workflow/tools.go	Guards event-type allocation hints.
pkg/workflow/threat_detection_steps.go	Guards cloned-map capacity.
pkg/workflow/safe_output_handlers.go	Guards handler-map capacity.
pkg/workflow/safe_jobs_needs_validation.go	Guards cycle-path capacity.
pkg/workflow/run_step_sanitizer.go	Guards environment-map capacity.
pkg/workflow/permissions.go	Guards permission-map capacity.
pkg/workflow/permissions_validation.go	Guards permission-name capacity.
pkg/workflow/observability_otlp.go	Guards merged-map capacity.
pkg/workflow/network_firewall_validation.go	Guards ecosystem-list capacity.
pkg/workflow/mcp_setup_safe_outputs.go	Guards environment-map capacity.
pkg/workflow/known_action_credentials.go	Guards credential-map capacity.
pkg/workflow/domains.go	Guards domain-list capacities.
pkg/workflow/concurrency.go	Guards concurrency-key capacities.
pkg/workflow/compiler_job_step_helpers.go	Guards inserted-step capacities.
pkg/workflow/compiler_builtin_job_augmentation.go	Guards augmented-step capacity.
pkg/workflow/compiler_aw_context.go	Guards generated-line capacity.
pkg/workflow/compiler_activation_job.go	Guards condition-injection capacity.
pkg/workflow/awf_helpers.go	Guards ecosystem-map capacity.
pkg/workflow/allocation_helpers.go	Removes superseded local helper.
pkg/workflow/allocation_helpers_test.go	Removes relocated helper tests.
pkg/typeutil/allocation.go	Adds shared overflow-safe helper.
pkg/typeutil/allocation_helpers_test.go	Covers valid, overflow, and negative inputs.
pkg/cli/experiments_analyze_statistics.go	Fixes flagged map capacity calculation.
pkg/cli/experiments_analyze_statistics_test.go	Verifies variant-count behavior.

Review details

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 25/25 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Addressed in 3d2f4a8 by moving pkg/typeutil into the repository import group in pkg/workflow/domains.go.

[gh-aw-bot]

gh-aw-pr-sous-chef
@copilot PR Sous Chef triage: this PR still needs forward progress before maintainer investigation is likely to be productive.

Please do one focused follow-up pass:

• fix the failing checks:
  • agent — https://github.com/github/gh-aw/actions/runs/32776331176/job/97588504206
  • lint-go-golangci — https://github.com/github/gh-aw/actions/runs/32774437985/job/97588322516
• review the latest non-blocking reviewer feedback and either apply the smallest worthwhile follow-up or reply with rationale if no code change is needed
• refresh the branch if needed and use the pr-finisher skill before handing back for review

I also attempted a branch refresh for this PR in this run.

Run: https://github.com/github/gh-aw/actions/runs/32781466813

Generated by 👨‍🍳 PR Sous Chef · pi · gpt54 · 21.6 AIC · ⌖ 13.1 AIC · ⊞ 8.7K · ◷
_{Comment /souschef to run again}

[Copilot]

Skills-Based Review 🧠

Applied /diagnosing-bugs, /tdd, and /codebase-design — COMMENT (no blocking issues, minor improvements noted)....

Addressed in 38b0a59: added a side-effect-free rationale comment to typeutil.SafeAllocationCapacity, renamed the test file to allocation_test.go, and added coverage for the includeDeclared=false and nil-config branches.

gh-aw-pr-sous-chef
@copilot PR Sous Chef triage: this PR still needs forward progress before maintainer investigation is likely to be productive.
...

Addressed with 3d2f4a8 and 38b0a59: fixed the goimports issue in pkg/workflow/domains.go, added the ADR expected by Design Decision Gate, and applied the smallest worthwhile non-blocking review follow-ups (test rename, extra branch coverage, and helper comment). Targeted tests passed locally.