mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-11 05:00:07 +00:00
fix(security): prevent command injection in branch deletion loops (#1989)
Add is_safe_branch_name() validation to all four cycle scripts (discovery.sh, refactor.sh, qa.sh, security.sh) to reject branch names containing shell metacharacters before passing them to git push --delete or git branch -D. Also adds -- end-of-options separator to all git branch commands to prevent flag injection. Fixes #1960 Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
A
GitHub
parent
d02e298488
commit
2eb623e386
4 changed files with 65 additions and 15 deletions
|
|
@ -46,6 +46,13 @@ safe_substitute() {
|
|||
rm -f "${file}.bak"
|
||||
}
|
||||
|
||||
# --- Validate branch name against safe pattern (defense-in-depth) ---
|
||||
# Prevents command injection via shell metacharacters in branch names
|
||||
is_safe_branch_name() {
|
||||
local name="${1:-}"
|
||||
[[ -n "${name}" ]] && [[ "${name}" =~ ^[a-zA-Z0-9._/-]+$ ]]
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
|
|
@ -129,8 +136,10 @@ _cleanup_stale_artifacts() {
|
|||
local MERGED_BRANCHES
|
||||
MERGED_BRANCHES=$(git branch -r --merged origin/main | grep -v 'origin/main\|origin/HEAD' | grep -E 'origin/(add-|impl-|gap-filler-)' | sed 's|origin/||' | tr -d ' ') || true
|
||||
for branch in $MERGED_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git push origin --delete "$branch" 2>&1 && log_info "Deleted merged branch: $branch" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git push origin --delete -- "$branch" 2>&1 && log_info "Deleted merged branch: $branch" || true
|
||||
else
|
||||
log_warn "Skipping branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
|
|
@ -248,7 +257,15 @@ cleanup_between_cycles() {
|
|||
git pull --rebase origin main 2>/dev/null || true
|
||||
git worktree prune 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
git branch --merged main | grep -v 'main' | grep -v '^\*' | xargs -r git branch -d 2>/dev/null || true
|
||||
local LOCAL_MERGED
|
||||
LOCAL_MERGED=$(git branch --merged main | grep -v 'main' | grep -v '^\*' | tr -d ' ') || true
|
||||
for branch in $LOCAL_MERGED; do
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git branch -d -- "$branch" 2>/dev/null || true
|
||||
else
|
||||
log_warn "Skipping local branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
log_info "Cleanup complete"
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -76,6 +76,13 @@ safe_substitute() {
|
|||
rm -f "${file}.bak"
|
||||
}
|
||||
|
||||
# --- Validate branch name against safe pattern (defense-in-depth) ---
|
||||
# Prevents command injection via shell metacharacters in branch names
|
||||
is_safe_branch_name() {
|
||||
local name="${1:-}"
|
||||
[[ -n "${name}" ]] && [[ "${name}" =~ ^[a-zA-Z0-9._/-]+$ ]]
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
|
|
@ -157,16 +164,20 @@ fi
|
|||
# Delete merged qa-related remote branches
|
||||
MERGED_BRANCHES=$(git branch -r --merged origin/main | grep -E 'origin/qa/' | sed 's|origin/||' | tr -d ' ') || true
|
||||
for branch in $MERGED_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git push origin --delete "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git push origin --delete -- "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
else
|
||||
log "WARNING: Skipping branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
# Delete stale local qa branches
|
||||
LOCAL_BRANCHES=$(git branch --list 'qa/*' | tr -d ' *') || true
|
||||
for branch in $LOCAL_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git branch -D "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git branch -D -- "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
else
|
||||
log "WARNING: Skipping local branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
|
|
|
|||
|
|
@ -56,6 +56,13 @@ safe_substitute() {
|
|||
rm -f "${file}.bak"
|
||||
}
|
||||
|
||||
# --- Validate branch name against safe pattern (defense-in-depth) ---
|
||||
# Prevents command injection via shell metacharacters in branch names
|
||||
is_safe_branch_name() {
|
||||
local name="${1:-}"
|
||||
[[ -n "${name}" ]] && [[ "${name}" =~ ^[a-zA-Z0-9._/-]+$ ]]
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
|
|
@ -124,16 +131,20 @@ if [[ "${RUN_MODE}" == "refactor" ]]; then
|
|||
# Delete merged refactor-related remote branches (fix/*, refactor/*, test/*, ux/*)
|
||||
MERGED_BRANCHES=$(git branch -r --merged origin/main | grep -v 'origin/main\|origin/HEAD' | grep -E 'origin/(fix/|refactor/|test/|ux/)' | sed 's|origin/||' | tr -d ' ') || true
|
||||
for branch in $MERGED_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git push origin --delete "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git push origin --delete -- "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
else
|
||||
log "WARNING: Skipping branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
# Delete stale local refactor-related branches
|
||||
LOCAL_BRANCHES=$(git branch --list 'fix/*' --list 'refactor/*' --list 'test/*' --list 'ux/*' | tr -d ' *') || true
|
||||
for branch in $LOCAL_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git branch -D "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git branch -D -- "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
else
|
||||
log "WARNING: Skipping local branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
|
|
|
|||
|
|
@ -103,6 +103,13 @@ safe_substitute() {
|
|||
rm -f "${file}.bak"
|
||||
}
|
||||
|
||||
# --- Validate branch name against safe pattern (defense-in-depth) ---
|
||||
# Prevents command injection via shell metacharacters in branch names
|
||||
is_safe_branch_name() {
|
||||
local name="${1:-}"
|
||||
[[ -n "${name}" ]] && [[ "${name}" =~ ^[a-zA-Z0-9._/-]+$ ]]
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
|
|
@ -180,16 +187,20 @@ fi
|
|||
# Delete merged security-related remote branches (team-building/*, review-pr-*)
|
||||
MERGED_BRANCHES=$(git branch -r --merged origin/main | grep -E 'origin/(team-building/|review-pr-)' | sed 's|origin/||' | tr -d ' ') || true
|
||||
for branch in $MERGED_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git push origin --delete "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git push origin --delete -- "$branch" 2>&1 | tee -a "${LOG_FILE}" && log "Deleted merged branch: $branch" || true
|
||||
else
|
||||
log "WARNING: Skipping branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
# Delete stale local security-related branches
|
||||
LOCAL_BRANCHES=$(git branch --list 'team-building/*' --list 'review-pr-*' | tr -d ' *') || true
|
||||
for branch in $LOCAL_BRANCHES; do
|
||||
if [[ -n "$branch" ]]; then
|
||||
git branch -D "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if is_safe_branch_name "$branch"; then
|
||||
git branch -D -- "$branch" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
else
|
||||
log "WARNING: Skipping local branch with unsafe name: ${branch}"
|
||||
fi
|
||||
done
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue