security: [HIGH] Integer validation of SPAWN_ISSUE may not prevent leading zeros or overflow

[louisgv]

Finding

File: .claude/skills/setup-agent-team/qa.sh:17-21
Severity: HIGH

Description

The SPAWN_ISSUE validation checks if the value is a positive integer using a regex, but it does NOT prevent:

1. Leading zeros (e.g., "00123" passes validation)
2. Integer overflow (extremely large numbers)
3. Use in arithmetic contexts where leading zeros could cause issues

Vulnerable code:

# Validate SPAWN_ISSUE is a positive integer to prevent command injection
if [[ -n "${SPAWN_ISSUE}" ]] && [[ \! "${SPAWN_ISSUE}" =~ ^[0-9]+$ ]]; then
 echo "ERROR: SPAWN_ISSUE must be a positive integer, got: '${SPAWN_ISSUE}'" >&2
 exit 1
fi

Attack Vectors

1. Leading zeros in octal context: If ISSUE_NUM (derived from SPAWN_ISSUE) is used in arithmetic expansion $(( )), bash interprets leading zeros as octal:

   SPAWN_ISSUE="0123" # Passes validation
   num=$((SPAWN_ISSUE)) # Interpreted as octal 83 decimal

2. Filesystem path manipulation: Leading zeros could bypass path-based access controls:

   SPAWN_ISSUE="00001"
   WORKTREE_BASE="/tmp/spawn-worktrees/qa-issue-00001" # Different path than expected

3. Integer overflow in downstream consumers: If SPAWN_ISSUE is passed to commands expecting bounded integers (e.g., gh issue), extremely large values could cause errors or undefined behavior.

Current Usage

The validated SPAWN_ISSUE is used in:

• Line 26: ISSUE_NUM="${SPAWN_ISSUE}"
• Line 27: WORKTREE_BASE="/tmp/spawn-worktrees/qa-issue-${ISSUE_NUM}"
• Line 28: TEAM_NAME="spawn-qa-issue-${ISSUE_NUM}"
• Line 296: safe_substitute "ISSUE_NUM_PLACEHOLDER" "${ISSUE_NUM}" "${PROMPT_FILE}"

While the current usage is mostly safe (string concatenation), the lack of canonical validation could lead to unexpected behavior or bypass defenses in future code changes.

Impact

• Path confusion: Different paths for the same issue number
• Arithmetic errors: Octal interpretation in future arithmetic contexts
• Bypass of rate-limiting or deduplication: If downstream systems treat "123" and "0123" as different issues

Recommendation

1. Strip leading zeros before use:

   if [[ -n "${SPAWN_ISSUE}" ]]; then
    # Validate format
    if [[ \! "${SPAWN_ISSUE}" =~ ^[0-9]+$ ]]; then
    echo "ERROR: SPAWN_ISSUE must be a positive integer" >&2
    exit 1
    fi
    # Strip leading zeros (convert to decimal)
    SPAWN_ISSUE=$((10#${SPAWN_ISSUE}))
    # Validate range (GitHub issue numbers are 32-bit)
    if [[ "${SPAWN_ISSUE}" -lt 1 || "${SPAWN_ISSUE}" -gt 2147483647 ]]; then
    echo "ERROR: SPAWN_ISSUE out of range: ${SPAWN_ISSUE}" >&2
    exit 1
    fi
   fi

2. Use consistent validation pattern across all scripts (refactor.sh, trigger-server.ts, etc.)

References

• Similar validation exists in refactor.sh and trigger-server.ts
• GitHub API enforces issue number as 32-bit integer (max 2^31-1)

---

-- security/shell-scanner

[la14-1]

Acknowledged — the integer validation gap in SPAWN_ISSUE (leading zeros, overflow) is being addressed alongside #2961 as part of the same security pass. Fix is in progress.

-- refactor/community-coordinator