mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-14 08:30:36 +00:00
security: add defensive guards to rm -rf cleanup paths (#1814)
Adds safe_rm_worktree() helper to all 4 agent team scripts that validates the target path starts with /tmp/spawn-worktrees/ before executing rm -rf. This prevents accidental deletion if WORKTREE_BASE is empty or contains an unexpected path. Affected files: discovery.sh, refactor.sh, security.sh, qa.sh Fixes #1791 Co-authored-by: lab <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
A
GitHub
parent
aa88e70488
commit
de2ba517ed
4 changed files with 54 additions and 10 deletions
|
|
@ -34,6 +34,17 @@ log_info() { printf "${GREEN}[discovery]${NC} %s\n" "$1"; echo "[$(date +'%Y-%m
|
|||
log_warn() { printf "${YELLOW}[discovery]${NC} %s\n" "$1"; echo "[$(date +'%Y-%m-%d %H:%M:%S')] [discovery] WARN: $1" >> "${LOG_FILE}"; }
|
||||
log_error() { printf "${RED}[discovery]${NC} %s\n" "$1"; echo "[$(date +'%Y-%m-%d %H:%M:%S')] [discovery] ERROR: $1" >> "${LOG_FILE}"; }
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
if [[ -z "${target}" ]]; then return; fi
|
||||
if [[ "${target}" != /tmp/spawn-worktrees/* ]]; then
|
||||
log_error "Refusing to rm -rf: '${target}' is not under /tmp/spawn-worktrees/"
|
||||
return 1
|
||||
fi
|
||||
rm -rf "${target}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
# --- Cleanup trap ---
|
||||
cleanup() {
|
||||
if [[ -n "${_cleanup_done:-}" ]]; then return; fi
|
||||
|
|
@ -42,7 +53,7 @@ cleanup() {
|
|||
log_info "Running cleanup (exit_code=${exit_code})..."
|
||||
cd "${REPO_ROOT}" 2>/dev/null || true
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
rm -f "${PROMPT_FILE:-}" 2>/dev/null || true
|
||||
log_info "=== Cycle Done (exit_code=${exit_code}) ==="
|
||||
exit $exit_code
|
||||
|
|
@ -99,7 +110,7 @@ _cleanup_stale_artifacts() {
|
|||
log_info "Pre-cycle cleanup..."
|
||||
git worktree prune 2>/dev/null || true
|
||||
if [[ -d "${WORKTREE_BASE}" ]]; then
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
log_info "Removed stale ${WORKTREE_BASE} directory"
|
||||
fi
|
||||
|
||||
|
|
@ -212,7 +223,7 @@ run_team_cycle() {
|
|||
rm -f "${PROMPT_FILE}" 2>/dev/null || true
|
||||
PROMPT_FILE=""
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
|
||||
return $CLAUDE_EXIT
|
||||
}
|
||||
|
|
@ -224,7 +235,7 @@ cleanup_between_cycles() {
|
|||
git fetch --prune origin 2>/dev/null || true
|
||||
git pull --rebase origin main 2>/dev/null || true
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 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
|
||||
log_info "Cleanup complete"
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,6 +55,17 @@ log() {
|
|||
printf '[%s] [qa/%s] %s\n' "$(date +'%Y-%m-%d %H:%M:%S')" "${RUN_MODE}" "$*" | tee -a "${LOG_FILE}"
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
if [[ -z "${target}" ]]; then return; fi
|
||||
if [[ "${target}" != /tmp/spawn-worktrees/* ]]; then
|
||||
log "ERROR: Refusing to rm -rf: '${target}' is not under /tmp/spawn-worktrees/"
|
||||
return 1
|
||||
fi
|
||||
rm -rf "${target}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
# Cleanup function — runs on normal exit, SIGTERM, and SIGINT
|
||||
cleanup() {
|
||||
# Guard against re-entry (SIGTERM trap calls exit, which fires EXIT trap again)
|
||||
|
|
@ -68,7 +79,7 @@ cleanup() {
|
|||
|
||||
# Prune worktrees and clean up only OUR worktree base
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
|
||||
# Clean up test directories from CLI integration tests
|
||||
TEST_DIR_COUNT=$(find "${HOME}" -maxdepth 1 -type d -name 'spawn-cmdlist-test-*' 2>/dev/null | wc -l)
|
||||
|
|
@ -110,7 +121,7 @@ fi
|
|||
# Clean stale worktrees
|
||||
git worktree prune 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if [[ -d "${WORKTREE_BASE}" ]]; then
|
||||
rm -rf "${WORKTREE_BASE}" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
log "Removed stale ${WORKTREE_BASE} directory"
|
||||
fi
|
||||
|
||||
|
|
|
|||
|
|
@ -44,6 +44,17 @@ log() {
|
|||
echo "[$(date +'%Y-%m-%d %H:%M:%S')] [${RUN_MODE}] $*" | tee -a "${LOG_FILE}"
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
if [[ -z "${target}" ]]; then return; fi
|
||||
if [[ "${target}" != /tmp/spawn-worktrees/* ]]; then
|
||||
log "ERROR: Refusing to rm -rf: '${target}' is not under /tmp/spawn-worktrees/"
|
||||
return 1
|
||||
fi
|
||||
rm -rf "${target}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
# Cleanup function — runs on normal exit, SIGTERM, and SIGINT
|
||||
cleanup() {
|
||||
# Guard against re-entry (SIGTERM trap calls exit, which fires EXIT trap again)
|
||||
|
|
@ -58,7 +69,7 @@ cleanup() {
|
|||
|
||||
# Prune worktrees and clean up only OUR worktree base
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
|
||||
# Clean up prompt and PID files
|
||||
rm -f "${PROMPT_FILE:-}" 2>/dev/null || true
|
||||
|
|
@ -94,7 +105,7 @@ if [[ "${RUN_MODE}" == "refactor" ]]; then
|
|||
log "Pre-cycle cleanup: stale worktrees and branches..."
|
||||
git worktree prune 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if [[ -d "${WORKTREE_BASE}" ]]; then
|
||||
rm -rf "${WORKTREE_BASE}" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
log "Removed stale ${WORKTREE_BASE} directory"
|
||||
fi
|
||||
|
||||
|
|
|
|||
|
|
@ -87,6 +87,17 @@ log() {
|
|||
echo "[$(date +'%Y-%m-%d %H:%M:%S')] [${RUN_MODE}] $*" | tee -a "${LOG_FILE}"
|
||||
}
|
||||
|
||||
# --- Safe rm -rf for worktree paths (defense-in-depth) ---
|
||||
safe_rm_worktree() {
|
||||
local target="${1:-}"
|
||||
if [[ -z "${target}" ]]; then return; fi
|
||||
if [[ "${target}" != /tmp/spawn-worktrees/* ]]; then
|
||||
log "ERROR: Refusing to rm -rf: '${target}' is not under /tmp/spawn-worktrees/"
|
||||
return 1
|
||||
fi
|
||||
rm -rf "${target}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
# Cleanup function — runs on normal exit, SIGTERM, and SIGINT
|
||||
cleanup() {
|
||||
# Guard against re-entry (SIGTERM trap calls exit, which fires EXIT trap again)
|
||||
|
|
@ -100,7 +111,7 @@ cleanup() {
|
|||
|
||||
# Prune worktrees and clean up only OUR worktree base
|
||||
git worktree prune 2>/dev/null || true
|
||||
rm -rf "${WORKTREE_BASE}" 2>/dev/null || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
|
||||
# Clean up test directories from CLI integration tests
|
||||
TEST_DIR_COUNT=$(find "${HOME}" -maxdepth 1 -type d -name 'spawn-cmdlist-test-*' 2>/dev/null | wc -l)
|
||||
|
|
@ -138,7 +149,7 @@ git pull --rebase origin main 2>&1 | tee -a "${LOG_FILE}" || true
|
|||
# Clean stale worktrees
|
||||
git worktree prune 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
if [[ -d "${WORKTREE_BASE}" ]]; then
|
||||
rm -rf "${WORKTREE_BASE}" 2>&1 | tee -a "${LOG_FILE}" || true
|
||||
safe_rm_worktree "${WORKTREE_BASE}"
|
||||
log "Removed stale ${WORKTREE_BASE} directory"
|
||||
fi
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue