Skip to content

Commit 8527fa0

Browse files
committed
fix(hooks): a linked worktree is one whose git-dir differs from its git-common-dir
Both worktree-first guards decided "am I in a linked worktree?" by substring-matching the git-dir path against `*/worktrees/*`. That is a test for the characters `worktrees` appearing anywhere in a path, not a test for a linked worktree: a PRIMARY checkout that merely lives under a directory named `worktrees` matched it, and both guards allowed edits into a shared primary checkout — the exact failure worktree-first exists to stop (see #11809). The verdict was also depth-dependent, which is what made it reachable in practice. `git rev-parse --git-dir` prints a RELATIVE `.git` at a repo toplevel and an ABSOLUTE path from any subdirectory, and the guards hand git the edited file's nearest EXISTING ancestor. So the same unguarded checkout blocked for a path resolving to the toplevel and allowed for anything resolving to a subdirectory — and in a real repo almost every edit is to a file in a subdirectory that already exists. Replaced with the structural test: a linked worktree's git-dir (.git/worktrees/NAME) differs from its git-common-dir (.git); a primary checkout has the two equal, and so does a submodule (.git/modules/NAME for both), so neither needs a special case. Two details are load-bearing and both are measured, not assumed: * `--git-common-dir` prints RELATIVE to the directory queried (`.git` at a toplevel, `../.git` from a subdirectory), so it must be resolved against that directory before the comparison. Compared raw it never equals the absolute git-dir and the guard fails open at EVERY depth — ablating just that line turns 47 matrix cases from block to allow. * `--absolute-git-dir` alone is NOT a fix. It removes the toplevel/subdirectory asymmetry by making the guard fail open everywhere instead of somewhere. Both sides are canonicalised through one helper so symlinked temp dirs and git's relative printing cannot make two spellings of the same directory look different. Self-tests: the four `worktrees`-segment cases are re-homed out of the KNOWN HOLE section and all four now expect `block`; the Bash matrix gains that fixture and five cases it never had. The non-vacuity recipe is re-aimed at the line that now exists, so it is not a dead mutation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
1 parent b31ebfe commit 8527fa0

4 files changed

Lines changed: 85 additions & 35 deletions

File tree

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ trap 'rm -rf "$tmp"' EXIT
2828
MAIN="$tmp/mainrepo"
2929
WT="$tmp/wt"
3030
PLAIN="$tmp/plain"
31-
mkdir -p "$MAIN/pkg" "$PLAIN"
31+
# ODD: a PRIMARY checkout whose own path carries a literal `worktrees` segment. Every write
32+
# into it must BLOCK — it is a primary checkout, and the verdict comes from the git-dir vs
33+
# git-common-dir structure, never from the spelling of the path (#11809). $WT is its positive
34+
# twin: a real linked worktree at a path with no such segment.
35+
ODD="$tmp/worktrees/oddrepo"
36+
mkdir -p "$MAIN/pkg" "$PLAIN" "$ODD/pkg"
3237
(
3338
cd "$MAIN" || exit 1
3439
git init -q .
@@ -39,6 +44,14 @@ mkdir -p "$MAIN/pkg" "$PLAIN"
3944
git add -A
4045
git commit -qm init
4146
git worktree add -q "$WT" -b selftest-wt
47+
cd "$ODD" || exit 1
48+
git init -q .
49+
git config user.email selftest@example.com
50+
git config user.name selftest
51+
: > README.md
52+
: > pkg/x.ts
53+
git add -A
54+
git commit -qm init
4255
) >/dev/null 2>&1 || { echo "could not build the git fixture" >&2; exit 1; }
4356

4457
CWD="$MAIN" # payload cwd for the cases that follow; reassigned per section
@@ -63,6 +76,7 @@ expect() { # expect <block|allow> <command> [env…]
6376
local got; got="$(verdict "$cmd" "$@")"
6477
local shown="${cmd//$'\n'/ ⏎ }"
6578
shown="${shown//$MAIN/\$MAIN}"; shown="${shown//$WT/\$WT}"; shown="${shown//$PLAIN/\$PLAIN}"
79+
shown="${shown//$ODD/\$ODD}"
6680
if [ "$got" = "$want" ]; then
6781
pass=$((pass + 1)); printf ' ok %-5s %s\n' "$got" "$shown"
6882
else
@@ -110,6 +124,19 @@ expect allow 'touch pkg/new.ts'
110124
expect allow 'cp /tmp/a.txt pkg/a.txt'
111125
expect allow "cd $WT && tee pkg/a.ts"
112126

127+
echo "== a PRIMARY checkout whose own path carries a 'worktrees' segment is BLOCKED =="
128+
# Every target here is ABSOLUTE, so the verdict can only have come from the path's own repo.
129+
# The two $ODD SUBDIRECTORY cases were `allow` under the `*/worktrees/*` substring test this
130+
# replaced — unguarded writes into a primary checkout — because git prints an ABSOLUTE
131+
# git-dir from a subdirectory and a RELATIVE one at the toplevel, so one checkout got
132+
# opposite verdicts by depth (#11809). The structural test is spelling-independent.
133+
CWD="$PLAIN"
134+
expect block "sed -i s/a/b/ $ODD/pkg/x.ts" # a SUBDIRECTORY — the depth the substring test lost
135+
expect block "echo x > $ODD/pkg/x.ts" # same depth, reached through redirection
136+
expect block "sed -i s/a/b/ $ODD/README.md" # the toplevel, which blocked only by accident
137+
expect block "sed -i s/a/b/ $MAIN/pkg/x.ts" # control: an ordinary shared primary checkout
138+
expect allow "sed -i s/a/b/ $WT/pkg/x.ts" # control: a real linked worktree still allows
139+
113140
echo "== writes outside any repo are fine (/tmp, scratchpad, \$HOME dotfiles) =="
114141
CWD="$MAIN"
115142
expect allow 'echo x > /tmp/os-selftest-out.log'

.claude/hooks/guard-main-checkout-bash.sh

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
#
2525
# Repo predicate — lifted verbatim from guard-main-checkout.sh so the two hooks can never
2626
# disagree about what "shared checkout" means:
27-
# resolve the target's nearest EXISTING ancestor dir -> `git rev-parse --git-dir`
27+
# resolve the target's nearest EXISTING ancestor dir -> `git rev-parse`
2828
# * not a git repo at all (/tmp, $HOME dotfiles, the scratchpad) -> allow
29-
# * git-dir matches */worktrees/* (a linked worktree) -> allow
29+
# * git-dir differs from git-common-dir (a linked worktree) -> allow
3030
# * anything else (the shared PRIMARY checkout, any sibling repo) -> BLOCK
3131
#
3232
# PRECISION OVER RECALL. Recognising a write target inside an arbitrary shell command is
@@ -312,9 +312,14 @@ tokenize() {
312312
}
313313

314314
# --- the repo predicate, identical to guard-main-checkout.sh's ------------------------
315+
# Canonicalise an existing directory to its physical absolute path, so both sides of the
316+
# comparison below are spelled the same way: git prints the common-dir RELATIVE, and some
317+
# hosts hand out symlinked temp dirs.
318+
canon_dir() { ( cd "$1" 2>/dev/null && pwd -P ) || printf '%s' "$1"; }
319+
315320
# 0 = this target lands in a shared primary checkout (block it), 1 = fine / unknowable.
316321
target_is_shared_checkout() {
317-
local p="$1" d gitdir
322+
local p="$1" d gitdir commondir
318323
[ -n "$p" ] || return 1
319324
[ "$p" = "-" ] && return 1 # stdout, not a file
320325

@@ -334,10 +339,15 @@ target_is_shared_checkout() {
334339
while [ -n "$d" ] && [ "$d" != "/" ] && [ ! -d "$d" ]; do d="$(dirname "$d")"; done
335340
[ -d "$d" ] || return 1
336341

337-
gitdir="$(git -C "$d" rev-parse --git-dir 2>/dev/null)" || return 1
338-
case "$gitdir" in
339-
*/worktrees/*) return 1 ;;
340-
esac
342+
# A linked worktree's git-dir (.git/worktrees/NAME) differs from its git-COMMON-dir
343+
# (.git); a primary checkout has the two equal, and so does a submodule. Structural, so it
344+
# holds whatever the path is spelled like — the `*/worktrees/*` substring match it replaces
345+
# did not (#11809). --git-common-dir prints RELATIVE to $d, so resolve it against $d first or
346+
# the guard fails open at EVERY depth.
347+
gitdir="$(git -C "$d" rev-parse --absolute-git-dir 2>/dev/null)" || return 1
348+
commondir="$(git -C "$d" rev-parse --git-common-dir 2>/dev/null)" || return 1
349+
case "$commondir" in /*) ;; *) commondir="$d/$commondir" ;; esac
350+
[ "$(canon_dir "$gitdir")" != "$(canon_dir "$commondir")" ] && return 1
341351
return 0
342352
}
343353

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

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ expect block "$MAIN/.changeset/x.md"
131131
expect block "$MAIN/pkg/x.ipynb"
132132

133133
echo "== the SAME files inside a linked worktree are allowed =="
134+
# The POSITIVE twin of the `worktrees`-segment section below. $WT is a REAL linked worktree
135+
# (`git worktree add`) whose path carries NO `worktrees` segment, so these allows can only
136+
# come from the structural test and never from the path's spelling. Both depths are pinned
137+
# below — the repo toplevel and a subdirectory — because git answers them differently.
134138
expect allow "$WT/pkg/x.ts"
135139
expect allow "$WT/pkg/deep/y.ts"
136140
expect allow "$WT/README.md"
@@ -363,25 +367,21 @@ PROJ="$PLAIN"
363367
check block 'NotebookEdit, new file in a new dir under $MAIN' "$(nbpay "$MAIN/brand/new/nb.ipynb")"
364368
check allow 'NotebookEdit, new file in a new dir under $WT' "$(nbpay "$WT/brand/new/nb.ipynb")"
365369

366-
# ── KNOWN HOLES ─────────────────────────────────────────────────────────────────────────
367-
# The cases below pin what the hook does TODAY, and what it does today is WRONG. They are
368-
# here so the matrix says the hole out loud rather than being silent about it, and so that
369-
# fixing it is a mechanical edit to this file. They are NOT statements of intended
370-
# behaviour. Each names the issue that must flip it.
371-
echo "== KNOWN HOLE #11809: any git-dir path containing /worktrees/ reads as a linked worktree =="
372-
# `case "$gitdir" in */worktrees/*) exit 0` is a substring match on a path, not a test for a
373-
# linked worktree. $ODD is a PRIMARY checkout that merely lives under a directory named
374-
# `worktrees`. git prints a RELATIVE git-dir (`.git`) at a repo's toplevel and an ABSOLUTE
375-
# one from any subdirectory, so the same unguarded checkout gets opposite verdicts by depth.
376-
# When #11809 is fixed both of these become `block`.
377-
# The deciding detail, measured rather than assumed: it is the NEAREST EXISTING ANCESTOR
378-
# that is handed to git, so a path whose nearest existing ancestor is the repo toplevel gets
379-
# the relative git-dir and blocks, while anything resolving to a subdirectory gets the
380-
# absolute one and slips through.
381-
expect block "$ODD/README.md" # correct today, but only because git-dir was relative
382-
expect block "$ODD/brand/new/f.ts" # ditto — resolves up to the toplevel
383-
expect allow "$ODD/pkg/x.ts" # ⛔ WRONG — an unguarded edit into a PRIMARY checkout
384-
expect allow "$ODD/pkg/brand/new/f.ts" # ⛔ WRONG — same hole, reached through the ancestor walk
370+
echo "== a PRIMARY checkout whose own path carries a 'worktrees' segment is BLOCKED =="
371+
# $ODD is a PRIMARY checkout that merely lives under a directory named `worktrees`. The
372+
# verdict comes from the STRUCTURE — git-dir differs from git-common-dir in a linked
373+
# worktree, and only there — never from the spelling of the path, so all four block. The two
374+
# SUBDIRECTORY cases were `allow` under the `*/worktrees/*` substring test this replaced:
375+
# unguarded edits into a primary checkout, which is the exact failure worktree-first exists
376+
# to stop (#11809). The pair of DEPTHS is what makes the section discriminating, and the
377+
# deciding detail was measured rather than assumed: it is the NEAREST EXISTING ANCESTOR that
378+
# is handed to git, so a path resolving to the repo toplevel got a RELATIVE git-dir and
379+
# blocked by accident, while anything resolving to a subdirectory got the absolute one and
380+
# slipped through.
381+
expect block "$ODD/README.md" # nearest existing ancestor = the repo toplevel
382+
expect block "$ODD/brand/new/f.ts" # ditto — the ancestor walk climbs to the toplevel
383+
expect block "$ODD/pkg/x.ts" # a SUBDIRECTORY — the depth the substring test lost
384+
expect block "$ODD/pkg/brand/new/f.ts" # same depth, reached through the ancestor walk
385385

386386
echo "== BOUNDARY: the jq-less fallback is a text scan, not a JSON parser =="
387387
# Not filed as a defect: jq is present wherever this hook runs, and Claude Code emits plain
@@ -403,11 +403,11 @@ printf '\n'
403403
#
404404
# cp .claude/hooks/guard-main-checkout.sh /tmp/mutant.sh
405405
# # e.g. delete the linked-worktree escape, which should redden every `allow` in a worktree:
406-
# perl -0pi -e 's{^\s*\*/worktrees/\*\) exit 0 ;;\n}{}m' /tmp/mutant.sh
406+
# perl -0pi -e 's{^\[ "\$\(canon_dir .*\n}{}m' /tmp/mutant.sh
407407
# GUARD_MAIN_CHECKOUT_HOOK=/tmp/mutant.sh .claude/hooks/guard-main-checkout.selftest.sh
408408
#
409-
# The mutations used, one per class: drop the */worktrees/* arm (core verdict) · drop the
410-
# nearest-existing-ancestor walk (new-file class) · replace dirname "$file" with
409+
# The mutations used, one per class: drop the linked-worktree escape (core verdict) · drop
410+
# the nearest-existing-ancestor walk (new-file class) · replace dirname "$file" with
411411
# CLAUDE_PROJECT_DIR (the file's-own-repo class) · drop the OS_ALLOW_MAIN_EDITS line (escape
412412
# hatch) · turn the no-path else branch into exit 0 (the fails-closed class) · rename the key
413413
# in the grep fallback (the jq-less class) · change the final exit 2 to exit 0 (every block) ·

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,24 @@ if [ -n "$file" ]; then d="$(dirname "$file")"; else d="${CLAUDE_PROJECT_DIR:-$P
9494
while [ -n "$d" ] && [ "$d" != "/" ] && [ ! -d "$d" ]; do d="$(dirname "$d")"; done
9595
[ -d "$d" ] || d="${CLAUDE_PROJECT_DIR:-$PWD}"
9696

97-
gitdir="$(git -C "$d" rev-parse --git-dir 2>/dev/null)" || exit 0
98-
99-
case "$gitdir" in
100-
*/worktrees/*) exit 0 ;;
101-
esac
97+
# Canonicalise an existing directory to its physical absolute path, so both sides of the
98+
# comparison below are spelled the same way: git prints the common-dir RELATIVE, and some
99+
# hosts hand out symlinked temp dirs.
100+
canon_dir() { ( cd "$1" 2>/dev/null && pwd -P ) || printf '%s' "$1"; }
101+
102+
# Am I in a LINKED WORKTREE? Structurally: a linked worktree's git-dir (.git/worktrees/NAME)
103+
# differs from its git-COMMON-dir (.git). A primary checkout has the two equal, and so does a
104+
# submodule (.git/modules/NAME for both) — which is why neither needs a special case. The
105+
# test holds whatever the path is spelled like; the `*/worktrees/*` substring match it
106+
# replaces did not, so a PRIMARY checkout that merely lived under a directory named
107+
# `worktrees` read as a linked worktree and went unguarded (#11809). --git-common-dir prints
108+
# RELATIVE to $d (`.git` at a toplevel, `../.git` from a subdirectory), so it MUST be
109+
# resolved against $d first: compared raw it never equals the absolute git-dir, and the
110+
# guard would fail open at EVERY depth instead of some.
111+
gitdir="$(git -C "$d" rev-parse --absolute-git-dir 2>/dev/null)" || exit 0
112+
commondir="$(git -C "$d" rev-parse --git-common-dir 2>/dev/null)" || exit 0
113+
case "$commondir" in /*) ;; *) commondir="$d/$commondir" ;; esac
114+
[ "$(canon_dir "$gitdir")" != "$(canon_dir "$commondir")" ] && exit 0
102115

103116
root="$(git -C "$d" rev-parse --show-toplevel 2>/dev/null || printf '%s' "$d")"
104117
branch="$(git -C "$d" rev-parse --abbrev-ref HEAD 2>/dev/null || printf '?')"

0 commit comments

Comments
 (0)