From a56f5c0dc2287a4ff456e577f8aadc2988241eed Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 17 Aug 2026 19:29:52 +0200 Subject: [PATCH] fix(agents): keep redirects and chained writes off the allow path Co-Authored-By: Claude Opus 5 (1M context) --- .claude/hooks/allow-fileops-in-tmp.sh | 18 +++++++++++++----- .claude/hooks/guard-rm-outside-tmp.sh | 3 +++ .claude/hooks/test-hooks.sh | 9 +++++++-- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.claude/hooks/allow-fileops-in-tmp.sh b/.claude/hooks/allow-fileops-in-tmp.sh index 71800a9313..62dbbe18ad 100755 --- a/.claude/hooks/allow-fileops-in-tmp.sh +++ b/.claude/hooks/allow-fileops-in-tmp.sh @@ -238,7 +238,7 @@ split_segments "$cmd" seg_cwd="${cwd:-$PWD}" alt_cwd="" # where a `cd` that failed would have left the command saw_cd=0 # a `cd` moved the working directory somewhere -proved=0 # at least one op came out inside a single root +proved=0 # how many ops came out inside a single root only_ours=1 # ... and nothing else shares the command line for seg in "${SEGMENTS[@]}"; do @@ -247,12 +247,12 @@ for seg in "${SEGMENTS[@]}"; do "") continue ;; mkdir | cp | mv | touch | chmod) check_fileops_segment "${SEG_TOKS[0]}" - proved=1 + proved=$((proved + 1)) continue ;; tar | unzip) check_archive_segment "${SEG_TOKS[0]}" - proved=1 + proved=$((proved + 1)) continue ;; cd) @@ -265,7 +265,10 @@ for seg in "${SEGMENTS[@]}"; do alt_cwd="$seg_cwd" seg_cwd="$new_cwd" else + # Not the harmless segment an allow assumes: whatever this guard could not account for + # may be a redirect, and a redirect writes. Leave the line to the normal flow. seg_cwd="" alt_cwd="" + only_ours=0 fi saw_cd=1 continue @@ -279,6 +282,11 @@ for seg in "${SEGMENTS[@]}"; do only_ours=0 done -[ "$proved" = 1 ] || exit 0 -[ "$only_ours" = 1 ] && decide allow "every path operand is under /tmp" +# Exactly one write per line. Each segment is proved against the filesystem as it stands now, +# and an earlier write can change what a later operand means: `cp -r /tmp/tree /tmp/live` that +# recreates a symlink out of /tmp turns `/tmp/live/link` — a path under /tmp when this ran — +# into a write through that symlink. Deletes compose safely and guard-rm-outside-tmp.sh allows +# several, because `rm` unlinks a symlink rather than following it. +[ "$proved" -ge 1 ] || exit 0 +[ "$only_ours" = 1 ] && [ "$proved" = 1 ] && decide allow "every path operand is inside a single root" exit 0 diff --git a/.claude/hooks/guard-rm-outside-tmp.sh b/.claude/hooks/guard-rm-outside-tmp.sh index 453d010147..7c352dd3a7 100755 --- a/.claude/hooks/guard-rm-outside-tmp.sh +++ b/.claude/hooks/guard-rm-outside-tmp.sh @@ -122,7 +122,10 @@ for seg in "${SEGMENTS[@]}"; do alt_cwd="$seg_cwd" seg_cwd="$new_cwd" else + # Not the harmless segment an allow assumes: whatever this guard could not account for + # may be a redirect, and a redirect writes. Leave the line to the normal flow. seg_cwd="" alt_cwd="" + only_ours=0 fi saw_cd=1 continue diff --git a/.claude/hooks/test-hooks.sh b/.claude/hooks/test-hooks.sh index 2e4f0d8dd6..d2ad4378c1 100644 --- a/.claude/hooks/test-hooks.sh +++ b/.claude/hooks/test-hooks.sh @@ -131,6 +131,7 @@ run $G ask "cd /tmp/does-not-exist; rm -rf backend/.env" run $G ask "cd /tmp/a && cd /tmp/b && rm -rf sub" run $G ask "rm -rf /tmp/clone/.git" # history is never in a class run $G ask "rm -rf /tmp/scratch/id_rsa.key" +run $G none "cd /tmp >$OUT; rm -f /tmp/a" echo echo "== allow-fileops-in-tmp.sh ==" @@ -154,8 +155,8 @@ run $A none "cp $CWD/AGENTS.md /tmp/a" run $A none "tar -xzf /tmp/a.tar.gz -C $OUT" run $A none "cargo build" -run $A allow "mkdir -p /tmp/x; mv /tmp/a /tmp/x; chmod 755 /tmp/x" -run $A allow "$(printf 'mv /tmp/a /tmp/b\nchmod 755 /tmp/b')" +run $A none "mkdir -p /tmp/x; mv /tmp/a /tmp/x; chmod 755 /tmp/x" # one write per line +run $A none "$(printf 'mv /tmp/a /tmp/b\nchmod 755 /tmp/b')" run $A ask "ls && mv /tmp/a /etc" run $A ask "$(printf 'mkdir -p /tmp/x\nchmod -R 777 %s' "$CWD")" run $A allow "cd /tmp/x && tar -xzf /tmp/a.tar.gz -C /tmp/out" @@ -175,6 +176,10 @@ run $A none "cp backend/secrets/token frontend/token.txt" # cp has no prompt # so what matters is it is not allowed run $A ask "mv $CWD/backend/credentials.json /tmp/x" run $A ask "cd /tmp/does-not-exist; mv .claude/settings.json settings.bak" +# A segment this hook cannot read whole may carry a redirect, and an earlier write can change +# what a later operand resolves to — neither may ride along on an allow. +run $A none "cd /tmp >$OUT; mv /tmp/a /tmp/b" +run $A none "cp -r /tmp/tree /tmp/live; cp /tmp/payload /tmp/live/link" echo [ "$fails" = 0 ] && echo "ALL PASS" || { echo "$fails FAILURES"; exit 1; }