fix(agents): let the scratch-dir hooks own their permission prompt (#10702)

This commit is contained in:
Ruben Fiszel
2026-08-14 14:30:52 +02:00
committed by GitHub
parent 578d5e9a7d
commit 0a40b3806f
6 changed files with 212 additions and 44 deletions
+34 -24
View File
@@ -2,10 +2,12 @@
# PreToolUse allowance for scratch file ops: auto-allow a single, plain, single-line
# `mkdir` / `cp` / `mv` / `touch` / `chmod` / `tar` / `unzip` whose every path operand
# resolves under /tmp. Anything else makes no decision (exit 0) and falls back to the normal
# permission flow — where `Bash(mv:*)` and `Bash(chmod:*)` in the `ask` list prompt. A
# PreToolUse `allow` overrides those ask rules, which is why this is a hook and not an allow
# rule: permission rules match a command prefix, so they can only constrain the FIRST operand.
# `cp /tmp/x ~/.zshrc` matches a `cp /tmp/` prefix, and requiring every operand is the point.
# permission flow, except for `mv` and `chmod`: those get an explicit `ask`, the only prompt
# they get (see lib-guarded-verb.sh).
#
# This is a hook rather than an allow rule because permission rules match a command prefix, so
# they can only constrain the FIRST operand. `cp /tmp/x ~/.zshrc` matches a `cp /tmp/` prefix,
# and requiring every operand is the point.
#
# Requiring the sources under /tmp too (not just the destination) keeps this from becoming a
# read-exfiltration path around the `Read(**/.env)` / `Read(**/secrets/**)` deny rules: a copy
@@ -31,6 +33,7 @@
#
# Assumes GNU `realpath` (-m) and `jq`, both present in this repo's Linux dev env.
set -uo pipefail
. "${BASH_SOURCE[0]%/*}/lib-guarded-verb.sh"
input=$(cat)
command -v jq >/dev/null 2>&1 || exit 0
@@ -38,8 +41,19 @@ cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)
[ -z "$cmd" ] && exit 0
cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null)
# Every bail-out below goes through `defer`: `mv` and `chmod` prompt from here, since no rule
# covers them, while the other verbs stay silent and leave the decision to the normal flow.
guarded=0
for verb in mv chmod; do
runs_verb "$verb" "$cmd" && { guarded=1; break; }
done
defer() {
[ "$guarded" = 1 ] && decide ask "$1"
exit 0
}
# A newline separates commands, and the tokenizer below only reads the first line — defer.
case "$cmd" in *$'\n'*) exit 0 ;; esac
case "$cmd" in *$'\n'*) defer "multi-line command" ;; esac
read -r -a toks <<< "$cmd"
@@ -65,11 +79,6 @@ under_tmp() {
return 1
}
allow() {
jq -nc --arg r "$1" '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"allow",permissionDecisionReason:$r}}'
exit 0
}
# Bare command word only; wrappers (`timeout cp`), env prefixes, and `/bin/cp` defer.
# Options are an allowlist per command, so anything that changes how symlinks are followed
# defers instead of needing enumeration. `cp -L` / `-H` matter most: they dereference while
@@ -84,7 +93,7 @@ case "${toks[0]:-}" in
chmod) takes_mode=1; ok_opts='Rvfc' ;; # chmod's first operand is a mode, not a path
tar) ok_flags='xctzjJavfC'; val_flags='fC' ;;
unzip) ok_flags='oqnljvd'; val_flags='d' ;;
*) exit 0 ;;
*) defer "not the leading command word" ;;
esac
# ---------------------------------------------------------------- tar / unzip
@@ -101,18 +110,18 @@ if [ -n "${ok_flags:-}" ]; then
flags="${t#-}"
# Allowlist: a long option, -P/--absolute-names, --transform, -I and friends all
# leave a residue here and defer rather than being enumerated as denials.
[ -n "$(printf '%s' "$flags" | tr -d "$ok_flags")" ] && exit 0
[ -n "$(printf '%s' "$flags" | tr -d "$ok_flags")" ] && defer "unrecognized option \`$t\`"
case "$flags" in *x*) extracting=1 ;; esac
case "${toks[0]}$flags" in unzip*[lv]*) listing=1 ;; esac
# A flag consuming the next token must be alone in its bundle's final position
# (`-xzf a.tar`), else the token it eats is ambiguous.
case "${flags%?}" in *[$val_flags]*) exit 0 ;; esac
case "${flags%?}" in *[$val_flags]*) defer "ambiguous option bundle \`$t\`" ;; esac
case "${flags: -1}" in
[$val_flags])
val="${toks[$i]:-}"
i=$((i + 1))
[ -n "$val" ] || exit 0
under_tmp "$val" || exit 0
[ -n "$val" ] || defer "option \`$t\` has no value"
under_tmp "$val" || defer "\`$val\` is outside /tmp"
case "${flags: -1}" in
f) saw_archive=1 ;;
C | d) saw_dest=1 ;;
@@ -126,17 +135,18 @@ if [ -n "${ok_flags:-}" ]; then
# Positional. For tar these are sources (create) or member names (extract); for unzip the
# first is the archive. Requiring every one under /tmp is conservative for member names,
# which are not filesystem paths — those defer rather than being wrongly allowed.
under_tmp "$t" || exit 0
under_tmp "$t" || defer "\`$t\` is outside /tmp"
[ "${toks[0]}" = "unzip" ] && saw_archive=1
done
[ "$saw_archive" = 1 ] || exit 0 # tar without -f reads a tape/stdin; unzip needs an archive
# tar without -f reads a tape/stdin; unzip needs an archive
[ "$saw_archive" = 1 ] || defer "no archive operand"
# Writes land relative to the working directory unless a destination was given. `unzip -l`
# and `-v` only list, so they need no destination.
if [ "$extracting" = 1 ] || { [ "${toks[0]}" = "unzip" ] && [ "$listing" = 0 ]; }; then
[ "$saw_dest" = 1 ] || under_tmp "${cwd:-$PWD}" || exit 0
[ "$saw_dest" = 1 ] || under_tmp "${cwd:-$PWD}" || defer "extraction target is outside /tmp"
fi
allow "archive paths and extraction target are under /tmp"
decide allow "archive paths and extraction target are under /tmp"
fi
# ------------------------------------------- mkdir / cp / mv / touch / chmod
@@ -155,7 +165,7 @@ while [ "$i" -lt "${#toks[@]}" ]; do
case "$t" in
-?*)
# Allowlist: long options and the dereferencing flags leave a residue and defer.
[ -n "$(printf '%s' "${t#-}" | tr -d "$ok_opts")" ] && exit 0
[ -n "$(printf '%s' "${t#-}" | tr -d "$ok_opts")" ] && defer "unrecognized option \`$t\`"
continue
;;
esac
@@ -165,15 +175,15 @@ while [ "$i" -lt "${#toks[@]}" ]; do
if [ "$takes_mode" = 1 ] && [ "$seen_mode" = 0 ]; then
case "$t" in
[0-7] | [0-7][0-7] | [0-7][0-7][0-7] | [0-7][0-7][0-7][0-7]) ;;
*) printf '%s' "$t" | grep -Eq '^[ugoa]*[+=-][rwxXst]*(,[ugoa]*[+=-][rwxXst]*)*$' || exit 0 ;;
*) printf '%s' "$t" | grep -Eq '^[ugoa]*[+=-][rwxXst]*(,[ugoa]*[+=-][rwxXst]*)*$' || defer "unrecognized mode \`$t\`" ;;
esac
seen_mode=1
continue
fi
under_tmp "$t" || exit 0
under_tmp "$t" || defer "\`$t\` is outside /tmp"
path_operand=1
done
[ "$path_operand" = 1 ] || exit 0
allow "every path operand is under /tmp"
[ "$path_operand" = 1 ] || defer "no path operand"
decide allow "every path operand is under /tmp"
+32 -17
View File
@@ -1,9 +1,9 @@
#!/usr/bin/env bash
# PreToolUse guard for `rm`: auto-allow ONLY a single, plain, single-line `rm` whose every
# operand is a whitelisted target — under /tmp, or inside a git working tree located in $HOME
# (a version-controlled project dir). Anything else makes no decision (exit 0) and falls back
# to the normal permission flow, where the `Bash(rm:*)` ask rule prompts (classifier as a
# backstop).
# (a version-controlled project dir). Any other command that runs `rm` gets an explicit `ask`,
# which is the ordinary permission prompt and the only one `rm` gets (see lib-guarded-verb.sh);
# a command that runs no `rm` at all makes no decision (exit 0).
#
# The git-tree allowance trades on "this is a project under version control" being lower-stakes
# than a delete elsewhere — NOT on full recoverability: committed content is restorable via git,
@@ -19,13 +19,14 @@
#
# The git-repo allowance covers targets inside a git working tree under $HOME, and the tree's
# own root folder only when it is a linked worktree (`.git` is a pointer file, so history in
# the main repo survives); a primary checkout's root (`.git` is a history dir) and any `.git`
# path are never auto-allowed. Globs auto-allow only under /tmp — elsewhere their expansion
# the main repo survives); a primary checkout's root (`.git` is a history dir) and any `.git`,
# `.claude` or `.env` path are never auto-allowed. Globs auto-allow only under /tmp — elsewhere their expansion
# could reach `.git` or a dotfile the literal checks never see. Relative operands resolve
# against the command's cwd (from the hook input). A PreToolUse `allow` overrides the ask rule.
# against the command's cwd (from the hook input).
#
# Assumes GNU `realpath` (-m) and `jq`, both present in this repo's Linux dev env.
set -uo pipefail
. "${BASH_SOURCE[0]%/*}/lib-guarded-verb.sh"
input=$(cat)
command -v jq >/dev/null 2>&1 || exit 0
@@ -33,12 +34,20 @@ cmd=$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)
[ -z "$cmd" ] && exit 0
cwd=$(printf '%s' "$input" | jq -r '.cwd // empty' 2>/dev/null)
# Every bail-out below goes through `defer`, so the forms this guard refuses to reason about —
# compound, quoted, wrapped — still reach the user as a prompt whenever an `rm` runs among them.
runs_verb rm "$cmd" && guarded=1 || guarded=0
defer() {
[ "$guarded" = 1 ] && decide ask "$1"
exit 0
}
# A newline separates commands, and the tokenizer below only reads the first line — defer.
case "$cmd" in *$'\n'*) exit 0 ;; esac
case "$cmd" in *$'\n'*) defer "multi-line command" ;; esac
read -r -a toks <<< "$cmd"
# Bare leading `rm` only; wrappers (`timeout rm`), env prefixes, and `/bin/rm` defer.
[ "${toks[0]:-}" = "rm" ] || exit 0
[ "${toks[0]:-}" = "rm" ] || defer "rm is not the leading command word"
# 0 (allow) iff the canonical path is an auto-allowable rm target: under /tmp, or strictly
# inside a git working tree located under $HOME. The walk stops at $HOME, so a dotfiles repo at
@@ -48,7 +57,13 @@ allowed_target() {
case "$canon" in /tmp/?*) return 0 ;; esac
[ -n "${HOME:-}" ] || return 1
case "$canon" in "$HOME"/?*) ;; *) return 1 ;; esac
case "$canon" in *"/.git" | *"/.git/"*) return 1 ;; esac # protect history, not recoverable
# Never auto-allow: history, and the two kinds of path the "it's under version control"
# premise doesn't hold for — the agent's own guards and settings (deleting them is what
# removes the prompt on everything else), and gitignored `.env` files.
case "$canon" in
*"/.git" | *"/.git/"* | *"/.claude" | *"/.claude/"*) return 1 ;;
*"/.env" | *"/.env."*) return 1 ;;
esac
d="$canon"
while [ "$d" != "/" ] && [ "$d" != "$HOME" ]; do
[ -e "$d/.git" ] && { root="$d"; break; }
@@ -73,10 +88,10 @@ while [ "$i" -lt "${#toks[@]}" ]; do
i=$((i + 1))
# Whitelist every token (flags included, so an operator hidden in a flag like `-rf;rm`
# can't slip past): any character outside the safe set makes it unsafe to reason about.
[ -n "$(printf '%s' "$t" | tr -d 'A-Za-z0-9._/*?[]-')" ] && exit 0
[ -n "$(printf '%s' "$t" | tr -d 'A-Za-z0-9._/*?[]-')" ] && defer "unsafe characters in \`$t\`"
# A glob in an option-looking token (`-[-]`) can expand to `--` and turn a later `-name`
# into an operand — never a real option, so defer.
case "$t" in -*[*?[]*) exit 0 ;; esac
case "$t" in -*[*?[]*) defer "glob inside the option \`$t\`" ;; esac
if [ "$end_opts" = 0 ]; then
[ "$t" = "--" ] && { end_opts=1; continue; }
# Skip real options only before the first operand. A bare `-` is a filename, and under
@@ -89,18 +104,18 @@ while [ "$i" -lt "${#toks[@]}" ]; do
had_operand=1
# No wildcard in a non-final path segment (`a/*/b`): it can expand through a symlink
# realpath can't see. A slashless glob (`*.rs`) is a final-segment match — fine.
case "$t" in */*) case "${t%/*}" in *[*?[]*) exit 0 ;; esac ;; esac
case "$t" in */*) case "${t%/*}" in *[*?[]*) defer "glob in a non-final segment of \`$t\`" ;; esac ;; esac
case "$t" in
/*) canon=$(realpath -m -- "$t" 2>/dev/null) ;;
*) canon=$(realpath -m -- "${cwd:-$PWD}/$t" 2>/dev/null) ;;
esac
[ -n "$canon" ] || exit 0
[ -n "$canon" ] || defer "cannot resolve \`$t\`"
# A glob may auto-allow only under /tmp, where everything is deletable. Elsewhere its
# expansion could match `.git`, a dotfile like `.*`, or a nested checkout root that the
# literal-path checks never see — so require literal operands in git repos.
case "$t" in *[*?[]*) case "$canon" in /tmp/?*) ;; *) exit 0 ;; esac ;; esac
allowed_target "$canon" || exit 0
case "$t" in *[*?[]*) case "$canon" in /tmp/?*) ;; *) defer "glob \`$t\` is outside /tmp" ;; esac ;; esac
allowed_target "$canon" || defer "\`$canon\` is outside /tmp and not inside a git checkout in \$HOME"
done
[ "$had_operand" = 1 ] || exit 0
jq -nc '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"allow",permissionDecisionReason:"rm operands are under /tmp or inside a git checkout in $HOME"}}'
[ "$had_operand" = 1 ] || defer "no operand"
decide allow 'rm operands are under /tmp or inside a git checkout in $HOME'
+57
View File
@@ -0,0 +1,57 @@
#!/usr/bin/env bash
# Sourced by the PreToolUse guards; not a hook itself.
#
# A permission rule beats a hook: an `ask` rule prompts whatever a PreToolUse hook returns, which
# makes the hook's `allow` dead weight. So settings.json carries no `ask` rule for `rm`, `mv` or
# `chmod`, and the guards own both halves — `allow` what they can prove safe, `ask` for the rest.
# Removing a guard's `ask` path therefore removes that verb's prompt entirely.
#
# `set -f` is global to the sourcing script so that the unquoted word split in runs_verb cannot
# expand a glob operand against the filesystem. Neither guard relies on pathname expansion.
set -f
# 0 iff <verb> ($1) runs as a command word anywhere in <command> ($2). Mirrors how a Bash
# permission rule matches, so that owning the prompt here doesn't narrow what used to prompt:
# the command splits on `; & |` and newlines, and a leading env assignment or process wrapper
# (`timeout 5 rm`, `xargs rm`) is skipped before the command word is read.
#
# The split set also carries the characters that open a nested command — `$(`, backticks, `( )`
# and `{ }` — because a rule matches the verb inside one (`echo $(rm -rf ~)` prompts), and a
# separator that only ends statements would read that as an `echo`.
#
# The scan is textual, so a heredoc body that merely contains the verb (`cat > s.sh <<EOF` …)
# reads as a command and prompts. Left that way on purpose: parsing heredocs to suppress it
# would risk dropping a real trailing command, and an extra prompt is the safe failure.
runs_verb() {
local verb="$1" seg w wrapped
while IFS= read -r seg; do
wrapped=0
for w in $seg; do
# The shell strips quotes and backslashes before it looks up the command, so `'rm'` and
# `r\m` run rm and have to compare equal to it.
w="${w//[\"\'\\]/}"
case "$w" in
"$verb" | */"$verb") return 0 ;;
*=*) ;; # leading env assignment
-* | [0-9]*) ;; # a wrapper's own flag, or its duration
*'>'* | *'<'*) ;; # leading redirect, `>/dev/null rm ...`
'!' | if | then | elif | else | while | until | do) ;; # keywords, never the command
timeout | time | nice | nohup | stdbuf | command | builtin | noglob | xargs | sudo | env) wrapped=1 ;;
# A wrapper's option value is indistinguishable from a command name (`stdbuf -o L rm`),
# so past a wrapper the whole segment is scanned instead of stopping at the first
# ordinary word. Before one, that word is the command and the verb cannot follow it.
*) [ "$wrapped" = 1 ] || break ;;
esac
done
# `tr` and not `${2//[...]}`: a `}` inside the bracket expression closes the expansion
# itself, which silently leaves the command unsplit and every separator unseen.
done <<< "$(printf '%s' "$2" | tr ';&|(){}`' '\n')"
return 1
}
# Emit a PreToolUse decision and exit. `ask` is the ordinary permission prompt.
decide() {
jq -nc --arg d "$1" --arg r "$2" \
'{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:$d,permissionDecisionReason:$r}}'
exit 0
}
+88
View File
@@ -0,0 +1,88 @@
#!/usr/bin/env bash
# Decision table for the two scratch-dir PreToolUse guards. Run: bash .claude/hooks/test-hooks.sh
#
# What this pins is the `ask` column: a matcher change that turns one into a no-decision drops
# that command's only prompt (see lib-guarded-verb.sh). The wrapper, nested-command and quoted
# rows are the ones that catch it.
set -uo pipefail
H="$(cd "${BASH_SOURCE[0]%/*}" && pwd)"
CWD="$(git -C "$H" rev-parse --show-toplevel)"
OUT="$HOME/not-a-git-tree" # never written to; only the guards' path checks look at it
fails=0
run() { # run <hook> <allow|ask|none> <command>
local hook="$1" want="$2" cmd="$3" out got
out=$(jq -nc --arg c "$cmd" --arg w "$CWD" \
'{tool_name:"Bash",tool_input:{command:$c},cwd:$w}' | "$H/$hook" 2>&1)
if [ -z "$out" ]; then
got=none
else
got=$(printf '%s' "$out" | jq -r '.hookSpecificOutput.permissionDecision // "PARSE-ERROR"' 2>/dev/null || echo PARSE-ERROR)
fi
if [ "$got" = "$want" ]; then
printf ' ok %-5s %s\n' "$got" "$cmd"
else
printf 'FAIL want=%-5s got=%-5s %s\n %s\n' "$want" "$got" "$cmd" "$out"
fails=$((fails + 1))
fi
}
echo "== guard-rm-outside-tmp.sh =="
G=guard-rm-outside-tmp.sh
run $G allow "rm -rf /tmp/scratch/x"
run $G allow "rm -rf /tmp/scratch/*"
run $G allow "rm -rf $CWD/frontend/scratch"
run $G ask "rm -rf /tmp"
run $G ask "rm -rf $OUT"
run $G ask "rm -rf $CWD/.git"
run $G ask "rm -rf $CWD/.claude/hooks" # the guards may not delete themselves
run $G ask "rm $CWD/.claude/settings.json"
run $G ask "rm $CWD/.claude/settings.local.json"
run $G ask "rm -rf $CWD/backend/.env"
run $G ask "rm -rf $CWD/.env.local"
run $G ask "rm -rf $CWD"
run $G ask "rm -rf $CWD/*"
run $G ask "rm -rf /etc/passwd"
run $G ask 'rm -rf "$HOME/x"'
run $G ask "rm -rf /tmp/../$OUT"
run $G ask "ls /tmp && rm -rf /tmp/x"
run $G ask 'echo $(rm -rf /etc)'
run $G ask 'echo `rm -rf /etc`'
run $G ask "{ rm -rf /etc; }"
run $G ask "find . -name x | xargs rm"
run $G ask "timeout 5 rm -rf /tmp/x"
run $G ask "stdbuf -o L rm -rf /etc"
run $G ask "FOO=bar rm -rf /tmp/x"
run $G ask "/bin/rm -rf /tmp/x"
run $G ask "'rm' -rf /etc"
run $G ask 'r\m -rf /etc'
run $G ask "! rm -rf /etc"
run $G ask "if true; then rm -rf /etc; fi"
run $G ask ">/dev/null rm -rf $OUT"
run $G none "git rm frontend/foo.ts"
run $G none 'echo $(ls /tmp)'
run $G none 'grep -rn "rm" backend/'
run $G none "cargo build --release"
echo
echo "== allow-fileops-in-tmp.sh =="
A=allow-fileops-in-tmp.sh
run $A allow "mv /tmp/a /tmp/b"
run $A allow "chmod 755 /tmp/a"
run $A allow "cp -r /tmp/a /tmp/b"
run $A allow "tar -xzf /tmp/a.tar.gz -C /tmp/out"
run $A ask "mv /tmp/a $OUT"
run $A ask "mv $CWD/AGENTS.md /tmp/a"
run $A ask "chmod -R 777 $CWD"
run $A ask "ls && mv /tmp/a /tmp/b"
run $A ask 'echo $(mv /tmp/a /etc)'
run $A ask "timeout --signal KILL 5 mv /tmp/a /etc"
run $A ask "time -f FORMAT chmod 777 $OUT"
run $A ask "'mv' /tmp/a /etc"
run $A ask 'ch\mod 777 /etc'
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"
echo
[ "$fails" = 0 ] && echo "ALL PASS" || { echo "$fails FAILURES"; exit 1; }
-3
View File
@@ -73,10 +73,7 @@
"Edit(**/.env.*)"
],
"ask": [
"Bash(rm:*)",
"Bash(rmdir:*)",
"Bash(mv:*)",
"Bash(chmod:*)",
"Bash(chown:*)",
"Bash(truncate:*)",
"Bash(shred:*)",
+1
View File
@@ -39,6 +39,7 @@ After all code changes are done, run `./update_sqlx.sh` from `backend/` to regen
| Modified Flow structures | Also update `openflow.openapi.yaml` |
| Changed DB schema | Update `backend/summarized_schema.txt` if needed |
| Enterprise file changes | Companion PR in `windmill-ee-private` (see `docs/enterprise.md`) |
| Changed a hook in `.claude/hooks/` | `bash .claude/hooks/test-hooks.sh` — pins which commands prompt |
## When to Write Tests