Skip to content

Commit a5f29af

Browse files
claude[bot]claude
andauthored
fix(hooks): read the edited path from the key the routed tool carries (#15665)
guard-main-checkout.sh extracted exactly one key, .tool_input.file_path, while settings.json routes Edit, Write AND NotebookEdit at it. NotebookEdit carries its path in notebook_path, so every notebook edit yielded an empty path, took the no-path branch and was judged by CLAUDE_PROJECT_DIR instead of by the file. That verdict is a constant per session and wrong in both directions: notebook edits inside a correctly-created worktree were refused when the session was rooted in the shared checkout, and notebook edits INTO the shared checkout were waved through when it was not. The jq-less fallback grepped the same single key and missed it too. The hook now carries a tool-to-path-key table and reads the key the routed tool actually carries, in the jq path and in the text-scan fallback alike. A tool the table does not name keeps the permissive read: any known key it carries, else the project dir. A tool the table DOES name that carries no path under its own key is schema drift, not a missing path, and blocks with a message naming the tool and the key rather than falling back to a verdict about the session. The matcher and the table are now a checked relation, which was the acceptance condition: the self-test reads known_path_keys out of the hook and reds if settings.json routes a tool that has no row in it, so the next tool routed here cannot repeat this defect silently. A row with no matcher entry prints a note. The matrix's known-hole section for this defect flips to the intended verdicts and its banner goes; the unrelated known hole below it is untouched, as are settings.json, the */worktrees/* predicate and guard-main-checkout-bash.sh. Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8e8860e commit a5f29af

2 files changed

Lines changed: 192 additions & 39 deletions

File tree

‎.claude/hooks/guard-main-checkout.selftest.sh‎

Lines changed: 131 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
#
1212
# Companion to guard-main-checkout-bash.selftest.sh, which covers the Bash half of the same
1313
# worktree-first pair. That matrix is mostly SHELL SPLITTING; this hook parses no shell at
14-
# all — it reads .tool_input.file_path and makes a PATH-AND-WORKTREE decision — so these
15-
# cases are derived from what this hook actually decides, not ported from the sibling.
14+
# all — it picks the payload's path key from the tool that sent it and makes a
15+
# PATH-AND-WORKTREE decision — so these cases are derived from what this hook actually
16+
# decides, not ported from the sibling.
1617
#
1718
# Fail-open by default, on purpose: the process cwd AND CLAUDE_PROJECT_DIR both default to a
1819
# directory in no repo at all, which is the input on which this hook allows everything. A
@@ -109,6 +110,14 @@ payload() { # payload <file_path> [tool_name] -> the Edit/Write payload shape
109110
tool_input:{file_path:$f,old_string:"a",new_string:"b"}}'
110111
}
111112

113+
nbpay() { # nbpay <notebook_path> [tool_name] -> the NotebookEdit payload shape
114+
# Matches the tool's documented input schema: notebook_path (absolute, required) and
115+
# new_source, with no file_path key anywhere in the payload.
116+
jq -nc --arg f "$1" --arg t "${2:-NotebookEdit}" \
117+
'{session_id:"selftest",cwd:"/payload-cwd-must-be-ignored",tool_name:$t,
118+
tool_input:{notebook_path:$f,new_source:"x",edit_mode:"replace"}}'
119+
}
120+
112121
expect() { # expect <block|allow> <file_path> [env…] — the common case
113122
local want="$1" f="$2"; shift 2
114123
check "$want" "$f" "$(payload "$f")" "$@"
@@ -155,13 +164,21 @@ expect block "$MAIN/pkg/x.ts"
155164
expect allow "$WT/pkg/x.ts"
156165
PROJ="$PLAIN"
157166

158-
echo "== tool_name is never consulted — scoping lives in the settings.json matcher =="
159-
# The hook decides on the path alone. Which tools reach it is the matcher's job, asserted
160-
# in the wiring section below; these cases pin that the hook itself does not second-guess it.
161-
for t in Edit Write NotebookEdit MultiEdit AnythingElse; do
162-
check block "tool_name=$t into \$MAIN" "$(payload "$MAIN/pkg/x.ts" "$t")"
163-
check allow "tool_name=$t into \$WT" "$(payload "$WT/pkg/x.ts" "$t")"
167+
echo "== tool_name selects the path key — one row per tool, and the matcher is its pair =="
168+
# tool_name is consulted for exactly one thing: which key of tool_input holds the path. The
169+
# verdict itself still comes from the path alone. Tools the table names are read for their
170+
# own key; a tool it does not name is not routed here by the matcher, and keeps the
171+
# permissive read — any known key it happens to carry, else the session's dir.
172+
for t in Edit Write MultiEdit; do
173+
check block "tool_name=$t (file_path) into \$MAIN" "$(payload "$MAIN/pkg/x.ts" "$t")"
174+
check allow "tool_name=$t (file_path) into \$WT" "$(payload "$WT/pkg/x.ts" "$t")"
164175
done
176+
check block 'tool_name=NotebookEdit (notebook_path) into $MAIN' "$(nbpay "$MAIN/pkg/x.ipynb")"
177+
check allow 'tool_name=NotebookEdit (notebook_path) into $WT' "$(nbpay "$WT/pkg/x.ipynb")"
178+
check block 'unrouted tool carrying file_path, into $MAIN' "$(payload "$MAIN/pkg/x.ts" AnythingElse)"
179+
check allow 'unrouted tool carrying file_path, into $WT' "$(payload "$WT/pkg/x.ts" AnythingElse)"
180+
check block 'unrouted tool carrying notebook_path, into $MAIN' "$(nbpay "$MAIN/pkg/x.ipynb" AnythingElse)"
181+
check allow 'unrouted tool carrying notebook_path, into $WT' "$(nbpay "$WT/pkg/x.ipynb" AnythingElse)"
165182

166183
echo "== escape hatch: OS_ALLOW_MAIN_EDITS must be exactly 1 =="
167184
check allow 'OS_ALLOW_MAIN_EDITS=1 into $MAIN' "$(payload "$MAIN/pkg/x.ts")" OS_ALLOW_MAIN_EDITS=1
@@ -173,19 +190,41 @@ check block 'OS_ALLOW_MAIN_EDITS=yes' "$(payload "$MAIN/pkg/x.ts")"
173190
check block 'OS_ALLOW_MAIN_EDITS=11' "$(payload "$MAIN/pkg/x.ts")" OS_ALLOW_MAIN_EDITS=11
174191
check block 'OS_ALLOW_MAIN_EDITS=" 1" (padded)' "$(payload "$MAIN/pkg/x.ts")" OS_ALLOW_MAIN_EDITS=" 1"
175192

176-
echo "== a payload carrying no usable path is judged by CLAUDE_PROJECT_DIR — fails CLOSED =="
177-
# This is the branch the hook takes when it learns nothing from the payload. It is the
178-
# opposite posture from the Bash sibling (which fails open on an unparseable command): here
179-
# an unreadable payload on the shared checkout still BLOCKS. Safe direction, and deliberate
180-
# — it is an explicit `else` in the hook, not a fall-through.
181-
for probe in '{"tool_name":"Edit","tool_input":{}}' '{}' 'not json at all' '' '{"tool_input":{"file_path":""}}'; do
193+
echo "== a ROUTED tool whose payload lacks its own path key is drift — it blocks, never guesses =="
194+
# The dangerous branch is the one that turns an unreadable payload into a confident verdict.
195+
# For a tool this guard is routed, an absent path key is not "no path given", it is the
196+
# tool's schema moving under the guard — and judging the session's dir instead would answer
197+
# the same way all session long, right or wrong by where that session is rooted. Every row
198+
# here is run with CLAUDE_PROJECT_DIR pointed somewhere that would ALLOW under a fallback,
199+
# except the first of each trio, so a passing `block` can only have come from the drift arm.
200+
for probe in \
201+
'{"tool_name":"Edit","tool_input":{}}' \
202+
'{"tool_name":"Write","tool_input":{"content":"x"}}' \
203+
'{"tool_name":"NotebookEdit","tool_input":{"new_source":"x","edit_mode":"replace"}}'
204+
do
205+
PROJ="$MAIN"; check block "routed tool, no path key, CLAUDE_PROJECT_DIR=\$MAIN [$probe]" "$probe"
206+
PROJ="$WT"; check block "routed tool, no path key, CLAUDE_PROJECT_DIR=\$WT [$probe]" "$probe"
207+
PROJ="$PLAIN"; check block "routed tool, no path key, CLAUDE_PROJECT_DIR=\$PLAIN [$probe]" "$probe"
208+
done
209+
PROJ="$PLAIN"
210+
# the cross-key pair: the right tool, the other tool's key. Drift in both directions.
211+
check block 'NotebookEdit carrying file_path (the wrong key), into $WT' "$(payload "$WT/pkg/x.ipynb" NotebookEdit)"
212+
check block 'Edit carrying notebook_path (the wrong key), into $WT' "$(nbpay "$WT/pkg/x.ipynb" Edit)"
213+
214+
echo "== an UNROUTED payload carrying no usable path is judged by CLAUDE_PROJECT_DIR — fails CLOSED =="
215+
# The remaining no-path branch: nothing in the payload names a tool this guard knows, so
216+
# there is no contract to have drifted. It is the opposite posture from the Bash sibling
217+
# (which fails open on an unparseable command): here an unreadable payload on the shared
218+
# checkout still BLOCKS. Safe direction, and deliberate — an explicit `else`, not a
219+
# fall-through.
220+
for probe in '{}' 'not json at all' '' '{"tool_input":{"file_path":""}}' '{"tool_name":"AnythingElse","tool_input":{}}'; do
182221
PROJ="$MAIN"; check block "no usable path, CLAUDE_PROJECT_DIR=\$MAIN [$probe]" "$probe"
183222
PROJ="$WT"; check allow "no usable path, CLAUDE_PROJECT_DIR=\$WT [$probe]" "$probe"
184223
PROJ="$PLAIN"; check allow "no usable path, CLAUDE_PROJECT_DIR=\$PLAIN [$probe]" "$probe"
185224
done
186225

187226
echo "== with CLAUDE_PROJECT_DIR unset the no-path branch falls back to the PROCESS cwd =="
188-
nopath='{"tool_name":"Edit","tool_input":{}}'
227+
nopath='{"tool_name":"AnythingElse","tool_input":{}}'
189228
for pair in "$MAIN:block" "$WT:allow" "$PLAIN:allow"; do
190229
CWD="${pair%:*}"
191230
( cd "$CWD" && printf '%s' "$nopath" | env -u CLAUDE_PROJECT_DIR "$hook" >/dev/null 2>&1 )
@@ -237,11 +276,26 @@ decoy="$(jq -nc --arg f "$MAIN/pkg/x.ts" --arg c "see \"file_path\": \"$PLAIN/de
237276
'{tool_name:"Write",tool_input:{content:$c,file_path:$f}}')"
238277
check block 'no jq: escaped decoy file_path in content loses to the real key' "$decoy" PATH="$nojq"
239278
check block 'with jq: same decoy payload' "$decoy"
279+
# the fallback mirrors the whole table, not just one key: it must read tool_name and the
280+
# notebook key too, or notebook edits silently rejoin the no-path branch whenever jq is away
281+
check block 'no jq: NotebookEdit into $MAIN' "$(nbpay "$MAIN/pkg/x.ipynb")" PATH="$nojq"
282+
check allow 'no jq: NotebookEdit into $WT' "$(nbpay "$WT/pkg/x.ipynb")" PATH="$nojq"
283+
check allow 'no jq: NotebookEdit into $PLAIN' "$(nbpay "$PLAIN/x.ipynb")" PATH="$nojq"
284+
check block 'no jq: NotebookEdit with no notebook_path is drift' \
285+
'{"tool_name":"NotebookEdit","tool_input":{"new_source":"x"}}' PATH="$nojq"
286+
# tool_name gets the same decoy treatment as the path keys: quoted inside a string value its
287+
# quotes are escaped, so prose about a tool cannot re-key the scan
288+
decoy2="$(jq -nc --arg f "$MAIN/pkg/x.ts" --arg c 'prose mentioning "tool_name": "NotebookEdit" verbatim' \
289+
'{tool_name:"Write",tool_input:{content:$c,file_path:$f}}')"
290+
check block 'no jq: escaped decoy tool_name in content loses to the real one' "$decoy2" PATH="$nojq"
291+
check block 'with jq: same tool_name decoy payload' "$decoy2"
240292

241-
echo "== wiring: settings.json must route Edit, Write and NotebookEdit to this hook =="
242-
# The hook is deliberately tool-agnostic, so the matcher is the ONLY thing that decides which
243-
# tools it sees. Nothing else in the repo checks that. Additions to the matcher are fine;
244-
# a removal is what this pins.
293+
echo "== wiring: the matcher routes Edit, Write and NotebookEdit, and every routed tool has a row =="
294+
# The matcher is the ONLY thing that decides which tools this hook sees, and the hook's
295+
# known_path_keys table is the only thing that decides what it reads out of each one.
296+
# Nothing else in the repo checks either half, so both are pinned here: a removal from the
297+
# matcher, and a tool routed with no row to read it by. Additions to the matcher are fine
298+
# — provided they bring their row.
245299
if [ -f "$settings" ]; then
246300
matcher="$(jq -r '[.hooks.PreToolUse[]? | select([.hooks[]?.command] | join(" ") | contains("guard-main-checkout.sh"))
247301
| .matcher] | join(" ")' "$settings" 2>/dev/null || printf '')"
@@ -251,10 +305,64 @@ if [ -f "$settings" ]; then
251305
*) fail=$((fail + 1)); printf ' FAIL %s is not routed to guard-main-checkout.sh (matcher: %s)\n' "$tool" "$matcher" ;;
252306
esac
253307
done
308+
309+
# ── the pairing ───────────────────────────────────────────────────────────────────────
310+
# "a tool the matcher routes here" and "a path key this hook knows" must be a CHECKABLE
311+
# relation, not a convention: a tool routed here with no row is read for a key its payload
312+
# never carries, and the guard silently goes back to judging the session. So: every name
313+
# in the matcher must have a row in the hook's table. A row with no matcher entry is the
314+
# harmless direction — a tool the hook is ready for that nothing routes yet — so it prints
315+
# a note, not a failure.
316+
table="$(sed -n "s/^known_path_keys='\(.*\)'\$/\1/p" "$hook" | head -1)"
317+
if [ -z "$table" ]; then
318+
fail=$((fail + 1)); printf ' FAIL the hook has no known_path_keys table for the matcher to pair with\n'
319+
else
320+
routed="$(printf '%s' "$matcher" | tr '|' ' ')"
321+
for tool in $routed; do
322+
row=""
323+
for r in $table; do case "$r" in "$tool="*) row="${r#*=}" ;; esac; done
324+
if [ -n "$row" ]; then
325+
pass=$((pass + 1)); printf ' ok pair %-13s -> .tool_input.%s\n' "$tool" "$row"
326+
else
327+
fail=$((fail + 1)); printf ' FAIL %s is routed to this hook but has no row in known_path_keys (%s)\n' "$tool" "$table"
328+
fi
329+
done
330+
for r in $table; do
331+
t="${r%%=*}"
332+
case " $routed " in
333+
*" $t "*) ;;
334+
*) printf ' note %s has a row in known_path_keys; the matcher does not route it\n' "$t" ;;
335+
esac
336+
done
337+
fi
254338
else
255339
fail=$((fail + 1)); printf ' FAIL settings.json not found at %s\n' "$(short "$settings")"
256340
fi
257341

342+
echo "== a notebook is judged by the NOTEBOOK's own path, exactly as a file edit is =="
343+
# The verdict must depend on where the notebook lives, never on where the session happens to
344+
# be rooted, so CLAUDE_PROJECT_DIR points at the WRONG place in every row here: a guard that
345+
# judged the session would answer the same way down each column instead of following the
346+
# path. The three `expect` rows are the same three notebooks through the Edit payload shape
347+
# — the two shapes must reach the same verdict, or the guard has one rule per tool.
348+
PROJ="$MAIN"
349+
check block 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$MAIN/pkg/x.ipynb")"
350+
check allow 'NotebookEdit into $WT, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$WT/pkg/x.ipynb")"
351+
check allow 'NotebookEdit into $PLAIN, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$PLAIN/x.ipynb")"
352+
PROJ="$WT"
353+
check block 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$WT' "$(nbpay "$MAIN/pkg/x.ipynb")"
354+
check allow 'NotebookEdit into $WT, CLAUDE_PROJECT_DIR=$WT' "$(nbpay "$WT/pkg/x.ipynb")"
355+
PROJ="$PLAIN"
356+
check block 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$PLAIN' "$(nbpay "$MAIN/pkg/x.ipynb")"
357+
check allow 'NotebookEdit into $WT, CLAUDE_PROJECT_DIR=$PLAIN' "$(nbpay "$WT/pkg/x.ipynb")"
358+
PROJ="$WT"; expect block "$MAIN/pkg/x.ipynb"
359+
PROJ="$MAIN"; expect allow "$WT/pkg/x.ipynb"
360+
PROJ="$MAIN"; expect allow "$PLAIN/x.ipynb"
361+
PROJ="$PLAIN"
362+
# and the ancestor walk reaches notebooks too: a new notebook in a not-yet-created directory
363+
check block 'NotebookEdit, new file in a new dir under $MAIN' "$(nbpay "$MAIN/brand/new/nb.ipynb")"
364+
check allow 'NotebookEdit, new file in a new dir under $WT' "$(nbpay "$WT/brand/new/nb.ipynb")"
365+
258366
# ── KNOWN HOLES ─────────────────────────────────────────────────────────────────────────
259367
# The cases below pin what the hook does TODAY, and what it does today is WRONG. They are
260368
# here so the matrix says the hole out loud rather than being silent about it, and so that
@@ -275,21 +383,6 @@ expect block "$ODD/brand/new/f.ts" # ditto — resolves up to the toplevel
275383
expect allow "$ODD/pkg/x.ts" # ⛔ WRONG — an unguarded edit into a PRIMARY checkout
276384
expect allow "$ODD/pkg/brand/new/f.ts" # ⛔ WRONG — same hole, reached through the ancestor walk
277385

278-
echo "== KNOWN HOLE #11810: NotebookEdit's path key is notebook_path, which this hook never reads =="
279-
# The matcher routes NotebookEdit here, but the hook extracts only .tool_input.file_path, so
280-
# every notebook edit takes the no-path branch and is judged by CLAUDE_PROJECT_DIR instead of
281-
# by the file. The verdict below is a constant per session and is wrong in both directions.
282-
# When #11810 is fixed, these become block / allow / allow by the notebook's own path.
283-
nbpay() { jq -nc --arg f "$1" '{tool_name:"NotebookEdit",tool_input:{notebook_path:$f,new_source:"x",edit_mode:"replace"}}'; }
284-
PROJ="$MAIN"
285-
check block 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$MAIN/pkg/x.ipynb")"
286-
check block 'NotebookEdit into $WT, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$WT/pkg/x.ipynb")" # ⛔ WRONG — refuses the mandated location
287-
check block 'NotebookEdit into $PLAIN, CLAUDE_PROJECT_DIR=$MAIN' "$(nbpay "$PLAIN/x.ipynb")" # ⛔ WRONG — refuses a file in no repo
288-
PROJ="$WT"
289-
check allow 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$WT' "$(nbpay "$MAIN/pkg/x.ipynb")" # ⛔ WRONG — unguarded edit into the shared checkout
290-
PROJ="$PLAIN"
291-
check allow 'NotebookEdit into $MAIN, CLAUDE_PROJECT_DIR=$PLAIN' "$(nbpay "$MAIN/pkg/x.ipynb")" # ⛔ WRONG — same, from a session rooted outside any repo
292-
293386
echo "== BOUNDARY: the jq-less fallback is a text scan, not a JSON parser =="
294387
# Not filed as a defect: jq is present wherever this hook runs, and Claude Code emits plain
295388
# UTF-8 paths, never \u-escaped ones. Recorded so that the first thing to fix is known if the
@@ -317,4 +410,7 @@ printf '\n'
317410
# nearest-existing-ancestor walk (new-file class) · replace dirname "$file" with
318411
# CLAUDE_PROJECT_DIR (the file's-own-repo class) · drop the OS_ALLOW_MAIN_EDITS line (escape
319412
# hatch) · turn the no-path else branch into exit 0 (the fails-closed class) · rename the key
320-
# in the grep fallback (the jq-less class) · change the final exit 2 to exit 0 (every block).
413+
# in the grep fallback (the jq-less class) · change the final exit 2 to exit 0 (every block) ·
414+
# point NotebookEdit's row at file_path (the notebook class) · delete NotebookEdit's row
415+
# altogether (the wiring pair, which reds on the matcher relation and not on a verdict) ·
416+
# replace the drift block with the project-dir fallback (the schema-drift class).

0 commit comments

Comments
 (0)