Skip to content

Guard OTLP attribute merge against allocation-size overflow#34117

Merged
pelikhan merged 4 commits into
mainfrom
copilot/fix-code-scanning-alerts-606
May 22, 2026
Merged

Guard OTLP attribute merge against allocation-size overflow#34117
pelikhan merged 4 commits into
mainfrom
copilot/fix-code-scanning-alerts-606

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 22, 2026

Code scanning flagged an integer-overflow risk in OTLP custom attribute merging. This change hardens the allocation path so extremely large input maps cannot overflow the capacity calculation and panic during merge.

  • Overflow guard in OTLP attribute merging

    • Replaced the raw len(base) + len(override) capacity hint in mergeOTLPCustomAttributes with a bounded helper.
    • Returns the summed capacity when it fits in int; otherwise falls back to 0 so the merge remains correct without unsafe preallocation.
  • Focused regression coverage

    • Added targeted tests for:
      • zero-length inputs
      • normal merged sizes
      • the exact math.MaxInt boundary
      • overflow cases just past the safe boundary
func mergedOTLPCustomAttributesCapacity(baseLen, overrideLen int) int {
	if baseLen > math.MaxInt-overrideLen {
		return 0
	}
	return baseLen + overrideLen
}

Copilot AI and others added 2 commits May 22, 2026 22:01
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code scanning alert #606 Guard OTLP attribute merge against allocation-size overflow May 22, 2026
Copilot AI requested a review from pelikhan May 22, 2026 22:02
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot create a helper to allocate array size to be reuse on every allocation site to avoid security warning.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 22, 2026

@copilot create a helper to allocate array size to be reuse on every allocation site to avoid security warning.

Fixed in e861607. I added a reusable safeAllocationCapacity(...) helper in pkg/workflow and switched the additive map/slice preallocation hints in this package, including the OTLP merge path, to use it.

@pelikhan pelikhan marked this pull request as ready for review May 22, 2026 22:41
Copilot AI review requested due to automatic review settings May 22, 2026 22:41
@pelikhan pelikhan merged commit 51c749c into main May 22, 2026
@pelikhan pelikhan deleted the copilot/fix-code-scanning-alerts-606 branch May 22, 2026 22:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens slice/map preallocation across the pkg/workflow package to avoid int overflow when computing capacity hints (e.g., len(a)+len(b)), including the originally flagged OTLP custom attribute merge path.

Changes:

  • Introduce safeAllocationCapacity(...int) to safely sum allocation “capacity hints” and fall back to 0 on overflow.
  • Replace direct len(x)+len(y)(+n) preallocation expressions with safeAllocationCapacity(...) in multiple workflow components (OTLP merge, permissions, generators, validators).
  • Add unit tests covering boundary and overflow behavior for the new helper.
Show a summary per file
File Description
pkg/workflow/workflow_import_merge.go Use safe capacity hint when merging imported and main job pre-steps.
pkg/workflow/tools.go Guard preallocation when merging types lists in defaults application.
pkg/workflow/threat_detection.go Guard map preallocation when injecting threat-detection if conditions.
pkg/workflow/safe_output_handlers.go Guard map preallocation when building handler lookup map.
pkg/workflow/safe_jobs_needs_validation.go Guard slice preallocation when assembling cycle descriptions.
pkg/workflow/run_step_sanitizer.go Guard map preallocation when rebuilding env maps during sanitization.
pkg/workflow/permissions.go Guard map preallocation when building valid permission scope set.
pkg/workflow/permissions_validation.go Guard slice preallocation when building scope list for fuzzy matching.
pkg/workflow/observability_otlp.go Guard OTLP custom-attributes merge map preallocation.
pkg/workflow/network_firewall_validation.go Guard slice preallocation when collecting ecosystem identifiers.
pkg/workflow/mcp_setup_generator.go Guard preallocation for env key list + env map during setup YAML generation.
pkg/workflow/known_action_credentials.go Guard map preallocation when merging known-action env var sets.
pkg/workflow/domains.go Guard slice preallocation when building default domain lists.
pkg/workflow/concurrency.go Guard slice preallocation when building concurrency expression parts.
pkg/workflow/compiler_safe_outputs.go Guard map/slice preallocation when merging event types.
pkg/workflow/compiler_jobs.go Guard slice preallocation when inserting pre-steps into step list.
pkg/workflow/compiler_aw_context.go Guard slice preallocation when injecting workflow trigger inputs.
pkg/workflow/compiler_activation_job.go Guard slice preallocation when injecting if after name.
pkg/workflow/awf_helpers.go Guard map preallocation when building ecosystem domain map for scripts.
pkg/workflow/allocation_helpers.go Add safeAllocationCapacity helper to prevent int overflow in capacity hints.
pkg/workflow/allocation_helpers_test.go Add regression tests for overflow/boundary behavior of the helper.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 21/21 changed files
  • Comments generated: 1

Comment on lines +5 to +8
// 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 {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants