From 2248bbad82e23e4034274fc00908cd7081d81c97 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 14:33:59 +0000 Subject: [PATCH] fix(hooks): a linked worktree is one whose git-dir differs from its git-common-dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #7259). 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. The executable lines stay byte-identical to the sibling repo's copies of both hooks. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox --- .../guard-main-checkout-bash.selftest.sh | 29 +++++++++- .claude/hooks/guard-main-checkout-bash.sh | 24 ++++++--- .claude/hooks/guard-main-checkout.selftest.sh | 53 +++++++++---------- .claude/hooks/guard-main-checkout.sh | 23 ++++++-- 4 files changed, 89 insertions(+), 40 deletions(-) diff --git a/.claude/hooks/guard-main-checkout-bash.selftest.sh b/.claude/hooks/guard-main-checkout-bash.selftest.sh index 2dc939487c..8ebe430e86 100755 --- a/.claude/hooks/guard-main-checkout-bash.selftest.sh +++ b/.claude/hooks/guard-main-checkout-bash.selftest.sh @@ -27,7 +27,12 @@ trap 'rm -rf "$tmp"' EXIT MAIN="$tmp/mainrepo" WT="$tmp/wt" PLAIN="$tmp/plain" -mkdir -p "$MAIN/pkg" "$PLAIN" +# ODD: a PRIMARY checkout whose own path carries a literal `worktrees` segment. Every write +# into it must BLOCK — it is a primary checkout, and the verdict comes from the git-dir vs +# git-common-dir structure, never from the spelling of the path (#7259). $WT is its positive +# twin: a real linked worktree at a path with no such segment. +ODD="$tmp/worktrees/oddrepo" +mkdir -p "$MAIN/pkg" "$PLAIN" "$ODD/pkg" ( cd "$MAIN" || exit 1 git init -q . @@ -38,6 +43,14 @@ mkdir -p "$MAIN/pkg" "$PLAIN" git add -A git commit -qm init git worktree add -q "$WT" -b selftest-wt + cd "$ODD" || exit 1 + git init -q . + git config user.email selftest@example.com + git config user.name selftest + : > README.md + : > pkg/x.ts + git add -A + git commit -qm init ) >/dev/null 2>&1 || { echo "could not build the git fixture" >&2; exit 1; } CWD="$MAIN" # payload cwd for the cases that follow; reassigned per section @@ -62,6 +75,7 @@ expect() { # expect [env…] local got; got="$(verdict "$cmd" "$@")" local shown="${cmd//$'\n'/ ⏎ }" shown="${shown//$MAIN/\$MAIN}"; shown="${shown//$WT/\$WT}"; shown="${shown//$PLAIN/\$PLAIN}" + shown="${shown//$ODD/\$ODD}" if [ "$got" = "$want" ]; then pass=$((pass + 1)); printf ' ok %-5s %s\n' "$got" "$shown" else @@ -109,6 +123,19 @@ expect allow 'touch pkg/new.ts' expect allow 'cp /tmp/a.txt pkg/a.txt' expect allow "cd $WT && tee pkg/a.ts" +echo "== a PRIMARY checkout whose own path carries a 'worktrees' segment is BLOCKED ==" +# Every target here is ABSOLUTE, so the verdict can only have come from the path's own repo. +# The two $ODD SUBDIRECTORY cases were `allow` under the `*/worktrees/*` substring test this +# replaced — unguarded writes into a primary checkout — because git prints an ABSOLUTE +# git-dir from a subdirectory and a RELATIVE one at the toplevel, so one checkout got +# opposite verdicts by depth (#7259). The structural test is spelling-independent. +CWD="$PLAIN" +expect block "sed -i s/a/b/ $ODD/pkg/x.ts" # a SUBDIRECTORY — the depth the substring test lost +expect block "echo x > $ODD/pkg/x.ts" # same depth, reached through redirection +expect block "sed -i s/a/b/ $ODD/README.md" # the toplevel, which blocked only by accident +expect block "sed -i s/a/b/ $MAIN/pkg/x.ts" # control: an ordinary shared primary checkout +expect allow "sed -i s/a/b/ $WT/pkg/x.ts" # control: a real linked worktree still allows + echo "== writes outside any repo are fine (/tmp, scratchpad, \$HOME dotfiles) ==" CWD="$MAIN" expect allow 'echo x > /tmp/os-selftest-out.log' diff --git a/.claude/hooks/guard-main-checkout-bash.sh b/.claude/hooks/guard-main-checkout-bash.sh index 94bb98fc86..117e48f364 100755 --- a/.claude/hooks/guard-main-checkout-bash.sh +++ b/.claude/hooks/guard-main-checkout-bash.sh @@ -17,9 +17,9 @@ # # Repo predicate — lifted verbatim from guard-main-checkout.sh so the two hooks can never # disagree about what "shared checkout" means: -# resolve the target's nearest EXISTING ancestor dir -> `git rev-parse --git-dir` +# resolve the target's nearest EXISTING ancestor dir -> `git rev-parse` # * not a git repo at all (/tmp, $HOME dotfiles, the scratchpad) -> allow -# * git-dir matches */worktrees/* (a linked worktree) -> allow +# * git-dir differs from git-common-dir (a linked worktree) -> allow # * anything else (the shared PRIMARY checkout, any sibling repo) -> BLOCK # # PRECISION OVER RECALL. Recognising a write target inside an arbitrary shell command is @@ -308,9 +308,14 @@ tokenize() { } # --- the repo predicate, identical to guard-main-checkout.sh's ------------------------ +# Canonicalise an existing directory to its physical absolute path, so both sides of the +# comparison below are spelled the same way: git prints the common-dir RELATIVE, and some +# hosts hand out symlinked temp dirs. +canon_dir() { ( cd "$1" 2>/dev/null && pwd -P ) || printf '%s' "$1"; } + # 0 = this target lands in a shared primary checkout (block it), 1 = fine / unknowable. target_is_shared_checkout() { - local p="$1" d gitdir + local p="$1" d gitdir commondir [ -n "$p" ] || return 1 [ "$p" = "-" ] && return 1 # stdout, not a file @@ -330,10 +335,15 @@ target_is_shared_checkout() { while [ -n "$d" ] && [ "$d" != "/" ] && [ ! -d "$d" ]; do d="$(dirname "$d")"; done [ -d "$d" ] || return 1 - gitdir="$(git -C "$d" rev-parse --git-dir 2>/dev/null)" || return 1 - case "$gitdir" in - */worktrees/*) return 1 ;; - esac + # 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. Structural, so it + # holds whatever the path is spelled like — the `*/worktrees/*` substring match it replaces + # did not (#7259). --git-common-dir prints RELATIVE to $d, so resolve it against $d first or + # the guard fails open at EVERY depth. + gitdir="$(git -C "$d" rev-parse --absolute-git-dir 2>/dev/null)" || return 1 + commondir="$(git -C "$d" rev-parse --git-common-dir 2>/dev/null)" || return 1 + case "$commondir" in /*) ;; *) commondir="$d/$commondir" ;; esac + [ "$(canon_dir "$gitdir")" != "$(canon_dir "$commondir")" ] && return 1 return 0 } diff --git a/.claude/hooks/guard-main-checkout.selftest.sh b/.claude/hooks/guard-main-checkout.selftest.sh index 4844703e2a..d9b415ccd8 100755 --- a/.claude/hooks/guard-main-checkout.selftest.sh +++ b/.claude/hooks/guard-main-checkout.selftest.sh @@ -17,12 +17,12 @@ # # PORTED from objectstack's copy of this matrix (objectstack-ai/objectstack, .claude/hooks/ # guard-main-checkout.selftest.sh @ d63c8a2) under objectui#6451, and the port is VERBATIM: -# the two repos' guard-main-checkout.sh differ by 9 diff lines that are all inside one -# comment block, no executable line differs, and both settings.json route the identical +# the two repos' guard-main-checkout.sh differ by 10 diff lines that are all inside comment +# blocks, no executable line differs, and both settings.json route the identical # Edit|Write|NotebookEdit matcher — so the sibling file was first run here BYTE-FOR-BYTE # unmodified (via the two env vars below) and returned 87 passed, 0 failed. Not one case -# needed adapting. The only edit below the header is the one remaining KNOWN HOLE section, -# whose issue reference is re-pointed at this repo's own card for the same defect. +# needed adapting. The only edit below the header is the `worktrees`-segment section, whose +# issue reference is re-pointed at this repo's own card for the same defect. # ⛔ Keep the two copies converged: a case that has to differ is evidence the HOOKS have # drifted, and that drift is the finding — not something to paper over here. # @@ -142,6 +142,10 @@ expect block "$MAIN/.changeset/x.md" expect block "$MAIN/pkg/x.ipynb" echo "== the SAME files inside a linked worktree are allowed ==" +# The POSITIVE twin of the `worktrees`-segment section below. $WT is a REAL linked worktree +# (`git worktree add`) whose path carries NO `worktrees` segment, so these allows can only +# come from the structural test and never from the path's spelling. Both depths are pinned +# below — the repo toplevel and a subdirectory — because git answers them differently. expect allow "$WT/pkg/x.ts" expect allow "$WT/pkg/deep/y.ts" expect allow "$WT/README.md" @@ -374,26 +378,21 @@ PROJ="$PLAIN" check block 'NotebookEdit, new file in a new dir under $MAIN' "$(nbpay "$MAIN/brand/new/nb.ipynb")" check allow 'NotebookEdit, new file in a new dir under $WT' "$(nbpay "$WT/brand/new/nb.ipynb")" -# ── KNOWN HOLES ───────────────────────────────────────────────────────────────────────── -# The cases below pin what the hook does TODAY, and what it does today is WRONG. They are -# here so the matrix says the hole out loud rather than being silent about it, and so that -# fixing it is a mechanical edit to this file. They are NOT statements of intended -# behaviour. Each names the issue that must flip it. -echo "== KNOWN HOLE #7259: any git-dir path containing /worktrees/ reads as a linked worktree ==" -# `case "$gitdir" in */worktrees/*) exit 0` is a substring match on a path, not a test for a -# linked worktree. $ODD is a PRIMARY checkout that merely lives under a directory named -# `worktrees`. git prints a RELATIVE git-dir (`.git`) at a repo's toplevel and an ABSOLUTE -# one from any subdirectory, so the same unguarded checkout gets opposite verdicts by depth. -# When #7259 is fixed both of these become `block`. (Same defect as the sibling repo's -# objectstack-ai/objectstack#11809 — the hooks share these lines; fix them together.) -# The deciding detail, measured rather than assumed: it is the NEAREST EXISTING ANCESTOR -# that is handed to git, so a path whose nearest existing ancestor is the repo toplevel gets -# the relative git-dir and blocks, while anything resolving to a subdirectory gets the -# absolute one and slips through. -expect block "$ODD/README.md" # correct today, but only because git-dir was relative -expect block "$ODD/brand/new/f.ts" # ditto — resolves up to the toplevel -expect allow "$ODD/pkg/x.ts" # ⛔ WRONG — an unguarded edit into a PRIMARY checkout -expect allow "$ODD/pkg/brand/new/f.ts" # ⛔ WRONG — same hole, reached through the ancestor walk +echo "== a PRIMARY checkout whose own path carries a 'worktrees' segment is BLOCKED ==" +# $ODD is a PRIMARY checkout that merely lives under a directory named `worktrees`. The +# verdict comes from the STRUCTURE — git-dir differs from git-common-dir in a linked +# worktree, and only there — never from the spelling of the path, so all four block. The two +# SUBDIRECTORY cases were `allow` under the `*/worktrees/*` substring test this replaced: +# unguarded edits into a primary checkout, which is the exact failure worktree-first exists +# to stop (#7259). The pair of DEPTHS is what makes the section discriminating, and the +# deciding detail was measured rather than assumed: it is the NEAREST EXISTING ANCESTOR that +# is handed to git, so a path resolving to the repo toplevel got a RELATIVE git-dir and +# blocked by accident, while anything resolving to a subdirectory got the absolute one and +# slipped through. +expect block "$ODD/README.md" # nearest existing ancestor = the repo toplevel +expect block "$ODD/brand/new/f.ts" # ditto — the ancestor walk climbs to the toplevel +expect block "$ODD/pkg/x.ts" # a SUBDIRECTORY — the depth the substring test lost +expect block "$ODD/pkg/brand/new/f.ts" # same depth, reached through the ancestor walk echo "== BOUNDARY: the jq-less fallback is a text scan, not a JSON parser ==" # Not filed as a defect: jq is present wherever this hook runs, and Claude Code emits plain @@ -415,11 +414,11 @@ printf '\n' # # cp .claude/hooks/guard-main-checkout.sh /tmp/mutant.sh # # e.g. delete the linked-worktree escape, which should redden every `allow` in a worktree: -# perl -0pi -e 's{^\s*\*/worktrees/\*\) exit 0 ;;\n}{}m' /tmp/mutant.sh +# perl -0pi -e 's{^\[ "\$\(canon_dir .*\n}{}m' /tmp/mutant.sh # GUARD_MAIN_CHECKOUT_HOOK=/tmp/mutant.sh .claude/hooks/guard-main-checkout.selftest.sh # -# The mutations used, one per class: drop the */worktrees/* arm (core verdict) · drop the -# nearest-existing-ancestor walk (new-file class) · replace dirname "$file" with +# The mutations used, one per class: drop the linked-worktree escape (core verdict) · drop +# the nearest-existing-ancestor walk (new-file class) · replace dirname "$file" with # CLAUDE_PROJECT_DIR (the file's-own-repo class) · drop the OS_ALLOW_MAIN_EDITS line (escape # hatch) · turn the no-path else branch into exit 0 (the fails-closed class) · rename the key # in the grep fallback (the jq-less class) · change the final exit 2 to exit 0 (every block) · diff --git a/.claude/hooks/guard-main-checkout.sh b/.claude/hooks/guard-main-checkout.sh index 619260d707..7d8f176df0 100755 --- a/.claude/hooks/guard-main-checkout.sh +++ b/.claude/hooks/guard-main-checkout.sh @@ -94,11 +94,24 @@ if [ -n "$file" ]; then d="$(dirname "$file")"; else d="${CLAUDE_PROJECT_DIR:-$P while [ -n "$d" ] && [ "$d" != "/" ] && [ ! -d "$d" ]; do d="$(dirname "$d")"; done [ -d "$d" ] || d="${CLAUDE_PROJECT_DIR:-$PWD}" -gitdir="$(git -C "$d" rev-parse --git-dir 2>/dev/null)" || exit 0 - -case "$gitdir" in - */worktrees/*) exit 0 ;; -esac +# Canonicalise an existing directory to its physical absolute path, so both sides of the +# comparison below are spelled the same way: git prints the common-dir RELATIVE, and some +# hosts hand out symlinked temp dirs. +canon_dir() { ( cd "$1" 2>/dev/null && pwd -P ) || printf '%s' "$1"; } + +# Am I in a LINKED WORKTREE? Structurally: 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) — which is why neither needs a special case. The +# test holds whatever the path is spelled like; the `*/worktrees/*` substring match it +# replaces did not, so a PRIMARY checkout that merely lived under a directory named +# `worktrees` read as a linked worktree and went unguarded (#7259). --git-common-dir prints +# RELATIVE to $d (`.git` at a toplevel, `../.git` from a subdirectory), so it MUST be +# resolved against $d first: compared raw it never equals the absolute git-dir, and the +# guard would fail open at EVERY depth instead of some. +gitdir="$(git -C "$d" rev-parse --absolute-git-dir 2>/dev/null)" || exit 0 +commondir="$(git -C "$d" rev-parse --git-common-dir 2>/dev/null)" || exit 0 +case "$commondir" in /*) ;; *) commondir="$d/$commondir" ;; esac +[ "$(canon_dir "$gitdir")" != "$(canon_dir "$commondir")" ] && exit 0 root="$(git -C "$d" rev-parse --show-toplevel 2>/dev/null || printf '%s' "$d")" branch="$(git -C "$d" rev-parse --abbrev-ref HEAD 2>/dev/null || printf '?')"