diff --git a/.claude/skills/setup-agent-team/discovery.sh b/.claude/skills/setup-agent-team/discovery.sh index 38b3fc09..0d984cf5 100755 --- a/.claude/skills/setup-agent-team/discovery.sh +++ b/.claude/skills/setup-agent-team/discovery.sh @@ -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" } diff --git a/.claude/skills/setup-agent-team/qa.sh b/.claude/skills/setup-agent-team/qa.sh index cef2df34..cf93c132 100644 --- a/.claude/skills/setup-agent-team/qa.sh +++ b/.claude/skills/setup-agent-team/qa.sh @@ -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 diff --git a/.claude/skills/setup-agent-team/refactor.sh b/.claude/skills/setup-agent-team/refactor.sh index 25845e28..b1c8a5d6 100755 --- a/.claude/skills/setup-agent-team/refactor.sh +++ b/.claude/skills/setup-agent-team/refactor.sh @@ -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 diff --git a/.claude/skills/setup-agent-team/security.sh b/.claude/skills/setup-agent-team/security.sh index 6072aab6..4463f00c 100644 --- a/.claude/skills/setup-agent-team/security.sh +++ b/.claude/skills/setup-agent-team/security.sh @@ -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