From e045cf6f788684ddc14b348f9bd1a37dd38ea270 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Tue, 24 Mar 2026 11:51:41 -0700 Subject: [PATCH] fix(security): prevent sed delimiter injection and harden SPAWN_ISSUE validation (#2964) safe_substitute: Switch sed delimiter from | to \x01 (SOH control char) across qa.sh, refactor.sh, security.sh, and discovery.sh. This eliminates delimiter injection regardless of value content, since \x01 cannot appear in normal input. Values containing \x01 are explicitly rejected as defense-in-depth. SPAWN_ISSUE: Fix qa.sh validation from ^[0-9]+$ to ^[1-9][0-9]*$ to reject leading zeros and zero itself. Add 32-bit signed integer range check (max 2147483647) to all three scripts (qa.sh, refactor.sh, security.sh) to prevent integer overflow in downstream consumers. Fixes #2961 Fixes #2962 Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- .claude/skills/setup-agent-team/discovery.sh | 13 +++++++--- .claude/skills/setup-agent-team/qa.sh | 27 +++++++++++++++----- .claude/skills/setup-agent-team/refactor.sh | 27 +++++++++++++++----- .claude/skills/setup-agent-team/security.sh | 26 ++++++++++++++----- 4 files changed, 70 insertions(+), 23 deletions(-) diff --git a/.claude/skills/setup-agent-team/discovery.sh b/.claude/skills/setup-agent-team/discovery.sh index 163baefc..19b3a547 100755 --- a/.claude/skills/setup-agent-team/discovery.sh +++ b/.claude/skills/setup-agent-team/discovery.sh @@ -36,16 +36,23 @@ log_error() { printf "${RED}[discovery]${NC} %s\n" "$1"; echo "[$(date +'%Y-%m-% # --- Safe sed substitution (escapes sed metacharacters in replacement) --- # Usage: safe_substitute PLACEHOLDER VALUE FILE -# Escapes \, &, |, and newlines in VALUE to prevent sed injection. +# Escapes \, &, and newlines in VALUE to prevent sed injection. +# Uses \x01 (SOH control char) as sed delimiter to prevent delimiter injection. safe_substitute() { local placeholder="$1" local value="$2" local file="$3" + # Reject values containing the \x01 delimiter (should never occur in normal input) + if printf '%s' "$value" | grep -qP '\x01'; then + log_error "safe_substitute value contains illegal \\x01 character" + return 1 + fi + # Escape backslashes first, then & (sed metacharacters in replacement) local escaped - escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g' -e 's/[|]/\\|/g') + escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g') # Escape literal newlines for sed replacement (backslash + newline) escaped="${escaped//$'\n'/\\$'\n'}" - sed -i.bak "s|${placeholder}|${escaped}|g" "$file" + sed -i.bak "s$(printf '\x01')${placeholder}$(printf '\x01')${escaped}$(printf '\x01')g" "$file" rm -f "${file}.bak" } diff --git a/.claude/skills/setup-agent-team/qa.sh b/.claude/skills/setup-agent-team/qa.sh index 557297c3..2fcaf32d 100644 --- a/.claude/skills/setup-agent-team/qa.sh +++ b/.claude/skills/setup-agent-team/qa.sh @@ -18,9 +18,16 @@ SPAWN_ISSUE="${SPAWN_ISSUE:-}" SPAWN_REASON="${SPAWN_REASON:-manual}" # 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 +# Rejects leading zeros, zero itself, and values exceeding 32-bit signed int max (GitHub limit) +if [[ -n "${SPAWN_ISSUE}" ]]; then + if [[ ! "${SPAWN_ISSUE}" =~ ^[1-9][0-9]*$ ]]; then + echo "ERROR: SPAWN_ISSUE must be a positive integer (1 or greater), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi + if [[ "${#SPAWN_ISSUE}" -gt 10 ]] || [[ "${SPAWN_ISSUE}" -gt 2147483647 ]]; then + echo "ERROR: SPAWN_ISSUE out of range (max 2147483647), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi fi if [[ "${SPAWN_REASON}" == "soak" ]]; then @@ -74,17 +81,23 @@ log() { # --- Safe sed substitution (escapes sed metacharacters in replacement) --- # Usage: safe_substitute PLACEHOLDER VALUE FILE # Replaces all occurrences of PLACEHOLDER with VALUE in FILE, escaping -# sed-special characters (\, &, |, newline) in VALUE to prevent misinterpretation. +# sed-special characters (\, &, newline) in VALUE to prevent misinterpretation. +# Uses \x01 (SOH control char) as sed delimiter to prevent delimiter injection. safe_substitute() { local placeholder="$1" local value="$2" local file="$3" - # Escape backslashes first, then &, then the delimiter | + # Reject values containing the \x01 delimiter (should never occur in normal input) + if printf '%s' "$value" | grep -qP '\x01'; then + log "ERROR: safe_substitute value contains illegal \\x01 character" + return 1 + fi + # Escape backslashes first, then & (sed metacharacters in replacement) local escaped - escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g' -e 's/[|]/\\|/g') + escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g') # Escape literal newlines for sed replacement (backslash + newline) escaped="${escaped//$'\n'/\\$'\n'}" - sed -i.bak "s|${placeholder}|${escaped}|g" "$file" + sed -i.bak "s$(printf '\x01')${placeholder}$(printf '\x01')${escaped}$(printf '\x01')g" "$file" rm -f "${file}.bak" } diff --git a/.claude/skills/setup-agent-team/refactor.sh b/.claude/skills/setup-agent-team/refactor.sh index 79e6e134..107ee808 100755 --- a/.claude/skills/setup-agent-team/refactor.sh +++ b/.claude/skills/setup-agent-team/refactor.sh @@ -16,10 +16,16 @@ SPAWN_ISSUE="${SPAWN_ISSUE:-}" SPAWN_REASON="${SPAWN_REASON:-manual}" # Validate SPAWN_ISSUE is a positive integer to prevent command injection -# Check both for valid format AND ensure it's not an empty string that passes -n check -if [[ -n "${SPAWN_ISSUE}" ]] && [[ ! "${SPAWN_ISSUE}" =~ ^[1-9][0-9]*$ ]]; then - echo "ERROR: SPAWN_ISSUE must be a positive integer (1 or greater), got: '${SPAWN_ISSUE}'" >&2 - exit 1 +# Rejects leading zeros, zero itself, and values exceeding 32-bit signed int max (GitHub limit) +if [[ -n "${SPAWN_ISSUE}" ]]; then + if [[ ! "${SPAWN_ISSUE}" =~ ^[1-9][0-9]*$ ]]; then + echo "ERROR: SPAWN_ISSUE must be a positive integer (1 or greater), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi + if [[ "${#SPAWN_ISSUE}" -gt 10 ]] || [[ "${SPAWN_ISSUE}" -gt 2147483647 ]]; then + echo "ERROR: SPAWN_ISSUE out of range (max 2147483647), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi fi if [[ -n "${SPAWN_ISSUE}" ]]; then @@ -46,16 +52,23 @@ log() { # --- Safe sed substitution (escapes sed metacharacters in replacement) --- # Usage: safe_substitute PLACEHOLDER VALUE FILE -# Escapes \, &, |, and newlines in VALUE to prevent sed injection. +# Escapes \, &, and newlines in VALUE to prevent sed injection. +# Uses \x01 (SOH control char) as sed delimiter to prevent delimiter injection. safe_substitute() { local placeholder="$1" local value="$2" local file="$3" + # Reject values containing the \x01 delimiter (should never occur in normal input) + if printf '%s' "$value" | grep -qP '\x01'; then + log "ERROR: safe_substitute value contains illegal \\x01 character" + return 1 + fi + # Escape backslashes first, then & (sed metacharacters in replacement) local escaped - escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g' -e 's/[|]/\\|/g') + escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g') # Escape literal newlines for sed replacement (backslash + newline) escaped="${escaped//$'\n'/\\$'\n'}" - sed -i.bak "s|${placeholder}|${escaped}|g" "$file" + sed -i.bak "s$(printf '\x01')${placeholder}$(printf '\x01')${escaped}$(printf '\x01')g" "$file" rm -f "${file}.bak" } diff --git a/.claude/skills/setup-agent-team/security.sh b/.claude/skills/setup-agent-team/security.sh index d5a8dd38..b9fbbecb 100644 --- a/.claude/skills/setup-agent-team/security.sh +++ b/.claude/skills/setup-agent-team/security.sh @@ -19,9 +19,16 @@ SPAWN_REASON="${SPAWN_REASON:-manual}" SLACK_WEBHOOK="${SLACK_WEBHOOK:-}" # Validate SPAWN_ISSUE is a positive integer to prevent command injection -if [[ -n "${SPAWN_ISSUE}" ]] && [[ ! "${SPAWN_ISSUE}" =~ ^[1-9][0-9]*$ ]]; then - echo "ERROR: SPAWN_ISSUE must be a positive integer (1 or greater), got: '${SPAWN_ISSUE}'" >&2 - exit 1 +# Rejects leading zeros, zero itself, and values exceeding 32-bit signed int max (GitHub limit) +if [[ -n "${SPAWN_ISSUE}" ]]; then + if [[ ! "${SPAWN_ISSUE}" =~ ^[1-9][0-9]*$ ]]; then + echo "ERROR: SPAWN_ISSUE must be a positive integer (1 or greater), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi + if [[ "${#SPAWN_ISSUE}" -gt 10 ]] || [[ "${SPAWN_ISSUE}" -gt 2147483647 ]]; then + echo "ERROR: SPAWN_ISSUE out of range (max 2147483647), got: '${SPAWN_ISSUE}'" >&2 + exit 1 + fi fi # Validate SLACK_WEBHOOK format to prevent sed delimiter injection via pipe chars @@ -93,16 +100,23 @@ log() { # --- Safe sed substitution (escapes sed metacharacters in replacement) --- # Usage: safe_substitute PLACEHOLDER VALUE FILE -# Escapes \, &, |, and newlines in VALUE to prevent sed injection. +# Escapes \, &, and newlines in VALUE to prevent sed injection. +# Uses \x01 (SOH control char) as sed delimiter to prevent delimiter injection. safe_substitute() { local placeholder="$1" local value="$2" local file="$3" + # Reject values containing the \x01 delimiter (should never occur in normal input) + if printf '%s' "$value" | grep -qP '\x01'; then + log "ERROR: safe_substitute value contains illegal \\x01 character" + return 1 + fi + # Escape backslashes first, then & (sed metacharacters in replacement) local escaped - escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g' -e 's/[|]/\\|/g') + escaped=$(printf '%s' "$value" | sed -e 's/[\\]/\\&/g' -e 's/[&]/\\&/g') # Escape literal newlines for sed replacement (backslash + newline) escaped="${escaped//$'\n'/\\$'\n'}" - sed -i.bak "s|${placeholder}|${escaped}|g" "$file" + sed -i.bak "s$(printf '\x01')${placeholder}$(printf '\x01')${escaped}$(printf '\x01')g" "$file" rm -f "${file}.bak" }