From 2bdbb7915d926fbc8c12ac2621fadca71e3d6ab6 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 07:14:38 -0500 Subject: [PATCH 01/12] docs: inventory the session-drift control estate, and audit what it actually enforces The drift controls grew one rule at a time and were documented the same way, so no page describes them as one system. This adds that page: the four layers (prevention hooks, SessionStart detection, the commit-time claim/ledger coordination gates, recovery + lifecycle scripts), a status table, and an audit. Statuses are probe-verified -- crafted PreToolUse JSON piped into the INSTALLED hook and the emitted decision read back -- not inferred from source. That distinction is the audit's main result: the installed gate is four days older than the repo copy, nothing in the repo or CI can observe that, and rule 4 is consequently inert in production while all 85 tests pass. Every other finding is downstream of it. Also records the ultracode answer the owner asked for: rule 2 blocks dispatch only when the session's cwd IS the primary -- every worktree, nested or sibling, dispatches freely -- so the recorded "a Workflow CANNOT launch there" reading is too broad. But rule 2's premise that a subagent "cannot create a worktree for itself" is now false: `isolation: worktree` is a documented, maintained subagent surface. Recommendation is to keep the deny (it fails fast and visibly at the parent), fix the entry point rather than the deny, and retire the inert EnterWorktree rule -- the transcript relocation it guards is designed behaviour since 2.1.198 and upstream added its own prompt in 2.1.206. No behaviour change: documentation only. --- docs/SESSION-DRIFT-CONTROLS.md | 455 +++++++++++++++++++++++++++++++++ docs/WORKTREE-GATE.md | 4 + docs/WORKTREES.md | 2 + 3 files changed, 461 insertions(+) create mode 100644 docs/SESSION-DRIFT-CONTROLS.md diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md new file mode 100644 index 00000000..8f877f24 --- /dev/null +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -0,0 +1,455 @@ +# Session-drift controls — what we built, what it buys, and what to do next + +**Scope.** This repo is developed by many concurrent Claude Code sessions against one `.git`. Over time +we accumulated a set of controls to stop those sessions colliding. This document inventories that +estate as **one system**, states which parts are actually enforcing, and records an audit of the gaps. + +**Drift**, here, means four distinct failures, not one: + +| # | Failure | Blast radius | +|---|---|---| +| D1 | A session **writes into the shared primary checkout** while others are standing in it | One file, one tree | +| D2 | A session **swaps the primary's working tree** (`checkout`/`reset`/…) | Every file, under every session, at once | +| D3 | A session **hijacks another session's worktree** onto a different branch | Every file under one other session | +| D4 | Two sessions **build the same work** independently | Duplicate PRs, untested merge | + +D2 is the worst and the least intuitive: a write dirties one file; a branch switch replaces the entire +tree under everyone simultaneously. + +Companion docs: [WORKTREES.md](WORKTREES.md) (how to use worktrees), [WORKTREE-GATE.md](WORKTREE-GATE.md) +(the gate's own design rationale and backout), [LEDGER-GATE.md](LEDGER-GATE.md) (the number space). + +--- + +## 1. The estate + +Four layers. Only the middle two enforce anything. + +### Prevention — `PreToolUse` hooks + +**[`scripts/hooks/worktree_gate.ps1`](../scripts/hooks/worktree_gate.ps1)** (417 lines) is installed at +**user scope** by [`install-gate.ps1`](../scripts/worktree/install-gate.ps1) into `~/.claude/settings.json` +*and* every `~/.claude-account-*/settings.json`. User scope is deliberate: a project-scoped hook is +git-tracked, so it lives on one branch and a worktree cut from an older base would carry no gate at all. +The registered command points at an installed **copy** under `~/.claude/hooks/`, because a hook whose +script path lives inside a working tree vanishes on a checkout — and a hook whose script is missing exits +non-zero-but-not-2, which means **the tool call runs anyway, silently**. + +It carries five rules. `Write-Deny` exits, so **the first rule to fire is the only one that speaks** — +there is no defence in depth between them. + +| Rule | Fires on | Keyed on | +|---|---|---| +| 1 | `Write`/`Edit`/`MultiEdit`/`NotebookEdit` targeting the primary's tree | **target path** | +| 2 | `Task`/`Agent`/`Workflow` dispatch from the primary | **session cwd** | +| 3 | 11 git verbs that swap or discard the primary's tree | target path parsed out of the command string | +| 3b | `checkout`/`switch` moving a *linked worktree* onto an existing branch | ditto | +| 4 | `EnterWorktree` (relocating a live session) | tool name only | + +The single most important design decision is that **rules 1/3/3b key on the target, never on the cwd**. +The gate's own docstring records that 29% of Edit/Write calls came from a session *sitting* in the primary +that wrote *correctly* into a worktree by absolute path; a cwd-keyed gate would have denied all of them. +Rule 2 is the sole exception, and that exception is the source of the ultracode friction in §4. + +**[`scripts/hooks/block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1)** (project +scope, [`.claude/settings.json`](../.claude/settings.json)) refuses blanket `git add -A`/`.`/`-u` and +`git commit -a`, so two sessions in one tree can't sweep each other's files into one commit. + +### Detection — `SessionStart` hooks + +**[`worktree-selfheal.ps1`](../scripts/worktree/worktree-selfheal.ps1)** (user scope) does one mutation — +auto-`checkout` of a **clean** primary that has drifted off its home branch — and two warnings: a +ghost-stub cwd, and this worktree's HEAD not matching its recorded home branch. + +**[`session-context.ps1`](../scripts/worktree/session-context.ps1)** (project scope) injects the +coordination banner: which worktree this chat owns, the full worktree list, the shared-memory write rule, +and the open work claims. + +### Coordination — commit-time gates (the D4 layer) + +Frequently forgotten in discussions of "the gate", but it is the same problem class: + +- **[`scripts/coord/claim.ps1`](../scripts/coord/claim.ps1)** — atomic exclusive-create of + `/mefor-coord/claims/.json`. Claims work, not numbers. +- **[`scripts/hooks/claim_check.py`](../scripts/hooks/claim_check.py)** — `commit-msg` gate: a commit whose + *subject* declares `BACKLOG #N` with a code-touching diff must hold a claim on N **for this worktree**. + Motivated by a recorded incident: three sessions independently fixed one npm advisory; two PRs were + closed as duplicates and the one that merged had not tested the failure mode the others found. +- **[`scripts/coord/alloc.ps1`](../scripts/coord/alloc.ps1)** + **[`ledger_check.py`](../scripts/hooks/ledger_check.py)** + — the same test-and-set for ADR/BACKLOG *numbers*. See [LEDGER-GATE.md](LEDGER-GATE.md). + +Both use exclusive-create because a read-modify-write on a shared list silently lost 4 of 8 concurrent +writes when measured. + +### Recovery and lifecycle + +`rescue.ps1` (move dirty primary work into a worktree), `restore-primary.ps1` (re-attach a detached +primary, refuses if dirty), `sessions.ps1 -Rehome` (find and re-file a relocated transcript), and +`new.ps1` / `spawn.ps1` / `remove.ps1` / `prune-merged.ps1`. + +### Status table + +Statuses below were established by driving crafted `PreToolUse` JSON into the **installed** hook and +reading the emitted decision — not by reading source alone. + +| Control | Scope | Status | +|---|---|---| +| Rule 1 — write into primary | user | **LIVE** (probe-verified DENY) | +| Rule 2 — dispatch from primary | user | **LIVE** (probe-verified DENY) | +| Rule 3 — git verbs vs primary | user | **LIVE but partial** — DENY on literal spellings, ALLOW on several others (§3) | +| Rule 3b — worktree hijack | user | **LIVE but narrow** — 2 of 11 verbs, existing-local-branch destinations only | +| Rule 4 — `EnterWorktree` | — | **INERT, twice over** — absent from the installed script *and* unmatched in all 5 config dirs | +| Blanket-stage guard | project | **LIVE but leaky** — 7 of 8 trivial rephrasings bypass it | +| Selfheal — primary auto-repair | user (4 of 5 dirs) | LIVE | +| Selfheal — hijack warning | user (4 of 5 dirs) | **LIVE and currently mis-firing** (§3, G4) | +| `session-context.ps1` banner | project | LIVE where the branch carries the file | +| Claim / alloc / ledger gates | git hooks | LIVE | +| `new.ps1` / `remove.ps1` / `prune-merged.ps1` | manual | LIVE, **sibling-layout only** | +| `tests/test_worktree_gate*.py`, `test_install_gate_wiring.py` | CI + local | **85 green, and blind** — every one binds the repo copy; nothing reads the installed copy or any live `settings.json` | + +Rule 4 being inert is **deliberate and announced** — the commit that landed it says "ships INERT … +nothing changes until `install-gate.ps1` is re-run." It is listed as INERT here because a control that +has never been installed is a source artefact, not an enforcement. + +--- + +## 2. What it demonstrably buys + +**Closed.** The accidental primary *edit* — the highest-frequency event and the gate's design centre — is +closed and probe-verified. So is the literal-spelling tree swap: `git checkout main` at cwd = primary, +`git -C checkout main`, `git -C ../../.. checkout zzz`, and `cd && git checkout main` +all DENY. Fan-out from the primary is closed for the three named dispatch tools. Duplicate-branch checkout +across worktrees is closed — though git enforces that one for free, so rule 3b fills a narrower gap than +its 90 lines suggest. + +**Only nominally closed.** + +*Shell writes into the primary.* The gate inspects tool arguments, so `Set-Content`, `python -c`, a +redirect, or `pip install -e .` into the primary is invisible to it. The gate's own header points at +`.git/hooks/pre-commit` as the backstop for this. That file is a stock pre-commit-framework shim +dispatching ledger-gate, ruff, forbidden-content, gitleaks and bandit — **none of which has a +primary/worktree predicate**. The backstop claim is unbacked as written. The only control on this path is +the deny text's "do not route around it with a shell command" — persuasion, in a repo whose whole premise +is that persuasion doesn't work here. + +*Any agent-authored script defeats rule 3 entirely.* Rule 3 requires a `git` token in the command string. +`pwsh -File whatever.ps1` has none. This is not hypothetical or adversarial — `restore-primary.ps1` is a +sanctioned example of exactly this shape, and its header states outright that an agent may run it. + +*Hijack detection.* Rule 3b covers `checkout`/`switch` onto an **existing local branch** only. +`git checkout `, ``, `origin/main` and `git reset --hard` inside another session's worktree all +return early. The selfheal detector meant to catch the residue is currently wrong (G4). + +*Writes into another session's worktree are allowed — and advertised.* This is the unavoidable price of +keying on the target path, and the alternative is strictly worse. But rule 1's own deny text says +"Writes to any linked worktree … are allowed FROM THIS SESSION", so the gate actively teaches the +behaviour. The only counterweight is prose in the session banner, printed only when 2+ worktrees exist. + +*Test coverage.* 85 tests pass. Every one binds the repo copy of the gate. Nothing anywhere reads the +installed copy or a live `settings.json`. This is the repo's own documented "green gate that cannot see" +pattern, reproduced inside the drift machinery itself. + +--- + +## 3. Gaps + +Ranked by expected harm × likelihood. G1–G4 survived an adversarial verification pass; the rest are +probe- or source-verified. + +### G1 — Nothing can observe what is actually installed *(root cause)* + +`install-gate.ps1` copies the script with no version, hash, or marker. `-Status` prints an **uncalibrated +count** of hook entries — it reports "3" where 4 is now expected, and states no expectation. Worse, the +`CLAUDECODE` refusal precedes the `-Status` branch, so **a session cannot audit the gate through the +supported interface at all**. Reverse drift is equally unseen: delete a rule from source and the stale +installed copy keeps enforcing it forever while every test correctly reports it gone. + +Measured today: installed 23,430 B (Jul 24) vs repo 25,423 B (Jul 29). This is why rule 4 is inert, and it +will recur for the next rule. + +### G2 — `cd` to the primary by a non-literal spelling defeats rule 3 + +Rule 3 resolves the target from cwd or `-C` only; `cd && git …` is caught solely by an in-text +scan for the allowlist root's canonical spelling. Rule 3b *does* resolve `cd`, then returns with the +comment "Rule 3 owns it" — but rule 3 already declined. Both bow out. And rule 3b only handles two verbs, +so for the other nine there is no hand-off at all. + +Probe-verified ALLOW from a nested worktree, where `../../..` **is** the primary: +`cd ../../.. && git checkout main`, `… && git reset --hard origin/main`, `… ; git clean -xfd`, +`… ; git stash`, `… && git rebase origin/main`, `pushd ../../.. ; git restore .`, +`cd ~/Code/ && git switch main`. `../../..` is the natural way for a session in a nested worktree +to name the repo root, so this is reachable **by accident**. [WORKTREE-GATE.md](WORKTREE-GATE.md) asserts +coverage of exactly this shape. + +### G3 — `git -c ` redirects rule 3's target off the primary *(one-character fix)* + +Rule 3 parses `-C ` with `-match`, which is case-**in**sensitive in PowerShell, so git's lowercase +global `-c name=value` is captured as if it were a path. `git -c core.pager=cat checkout main` at +cwd = primary → **ALLOW**. Rule 3b guards against precisely this with `-cmatch` and a comment explaining +why; the hardening was applied to the narrower rule and never back-ported to the one protecting the +shared tree. + +Upstream hit the same class and fixed it: Claude Code 2.1.216 — *"Fixed worktree-isolated subagents +redirecting git into the shared checkout via `git -C`, `--git-dir`, or `GIT_DIR`/`GIT_WORK_TREE`."* +Note `--git-dir` and the env vars are absent from our gate entirely. + +### G4 — The home-branch detector is wrong now, and it has no update path + +`worktree-selfheal.ps1` compares HEAD against an untracked `mefor-home-branch` file. That file has two +writers (`new.ps1` at creation, selfheal's bootstrap-on-first-sighting) and **no update path**. `new.ps1` +only ever creates *siblings*, so for a nested `.claude/worktrees/` the record is whatever branch the +hook first saw — and re-tasking such a directory, which is the normal lifecycle, mismatches forever. + +Live right now: this worktree records `…mefor-jdbc-support…`, HEAD is `…session-drift-prevention…`; a +second nested worktree mismatches too. Two of three. The printed remedy is a `git … switch `, +which would swap a live session off its real branch — though it is conditioned on "if that was not +intentional", scoped to a plain terminal, and **rule 3b would deny it** if an agent tried. So the harm +channel is a human following stale advice, not in-session drift. No test mentions `mefor-home-branch`. + +Note also a naming collision worth cleaning up: `mefor.homeBranch` (a git config key, read by +`restore-primary.ps1` and selfheal) and `mefor-home-branch` (an untracked file written by `new.ps1`) are +two different mechanisms one letter apart. + +### G5 — The gate's own enforcement surface is writable by every governed session + +The live gate is `~/.claude/hooks/worktree_gate.ps1`; the kill switch is +`~/.claude/hooks/worktree-gate.repos.txt`; the matchers are in five `settings.json` files. **None of those +paths is under a governed root**, so `Test-Governed` returns null and rule 1 allows an `Edit`/`Write` to +any of them. There is no user-scope `permissions.deny` in any config dir. A one-line write to the +allowlist disarms the gate for every session on the box, permanently and silently. + +The gate's header says the kill switch is "deliberately NOT named in the deny message: a model running in +`bypassPermissions` would use it." That is security by obscurity over a file one `ls` away. By contrast +`/.git/hooks` **is** governed by rule 1 already, and a test asserts it. + +### G6 — The shared `.git` is unprotected against config-level disarm + +`config` is not in rule 3's verb list, so `git config core.hooksPath …` and `git -c core.hooksPath=… commit` +pass (probe-verified ALLOW) — disabling the ledger/claim/leak commit gates for **all eight worktrees at +once**. This is already biting passively: the installed `pre-commit` shim hard-codes an interpreter path +inside a *sibling worktree*, so that worktree is load-bearing for every tree's commit gates, and +`prune-merged.ps1 -Apply` on it would break commits everywhere. + +Upstream draws this boundary explicitly — the worktrees documentation states that a worktree shares the +repository's `.git` and that sandboxing allows those writes, which is exactly why `hooks/` and config need +their own rule. + +### G7 — Rule 1's deny message advertises the primary as a worktree to reuse + +The "worktrees that already exist — REUSE one if it is yours" filter compares a string to the +`PSCustomObject` returned by `Test-Governed`, so the comparison is always true and the primary is never +filtered out. The message refuses a write to the primary and then lists the primary first, displacing a +real worktree off the 8-item cap. This is the remediation channel — the part whose entire job is steering +the next action — and no test asserts on deny-text content. + +### G8 — Two allowlists, two installers, no sync + +The gate reads `~/.claude/hooks/worktree-gate.repos.txt`; the selfheal backstop reads +`~/.claude-hooks/worktree-gate.repos.txt`. `install-gate.ps1` rewrites its own unconditionally; +`install-selfheal.ps1` seeds the other only if absent. Adding a governed repo via one installer never +reaches the other, and `install-gate.ps1 -Uninstall` leaves the backstop armed and still willing to +`git checkout` the primary — so the "verified byte-for-byte clean uninstall" claim is true of the gate and +false of the estate. They agree today by luck. + +Related: `install-selfheal.ps1` lacks the `CLAUDECODE` refusal its sibling has, and its source is +`$PSScriptRoot` — the calling session's own worktree copy, which that session may freely edit. The +**higher-privilege** component (it runs `git checkout` on the primary unattended) is the **less +protected** one. `~/.claude-account-2.lock` has the three gate matchers and no selfheal hook at all. + +### G9 — Coverage is enumerated, so every hole is silent + +Rule 3's verb list omits `rm`, `mv`, `sparse-checkout`, `checkout-index`, `bisect`, `worktree`, +`branch -f`, `update-ref`, `read-tree` — all ALLOW at cwd = primary. `worktree remove` is the notable one: +it destroys *another session's* checkout. `sparse-checkout` cannot match by construction (the pattern +requires whitespace before the verb; a hyphen precedes `checkout`). `gh pr checkout ` carries no `git` +token and exits early. Rules 1 and 2 key on tool *names*, so any tool not in those lists is unmatched at +**both** the settings matcher and the rule — the hook never runs, and nothing says so. + +### G10 — False positives train sessions to route around the only control on the shell path + +The verb scan's exclusion class does not exclude newline, so `git status\necho about to merge stuff` +denies with verb=`merge` from prose on line 2. The git-detection class includes quote characters, so +`echo "git checkout main"` denies. A commit message containing a blocklisted word (`git commit -m +"chore: clean up dead code"`) denies. The sibling blanket-stage hook splits on newlines; this one does +not — **the two hooks disagree about what a command is**. Two read-only commands were denied during this +audit. Every false positive erodes compliance with the deny text, which per §2 is the *only* control on +shell writes. + +### G11 — The two worktree layouts have diverged, and only one has teardown + +`new.ps1` builds **siblings** (`/-`). Claude Code's own `--worktree`, the desktop app, +and subagent isolation all build **nested** worktrees under `/.claude/worktrees/`. Both +populations are live (5 sibling, 3 nested). The gate's exemption and rule 3's third lookahead are written +for the nested form and their rationale comment asserts "that is exactly where `new.ps1` puts every +worktree" — false. No hole results (siblings fall outside the root prefix entirely), but the code +documents a reason that does not hold, leaving no correct model for the next edit. Meanwhile `remove.ps1` +and `prune-merged.ps1` are sibling-only, so the nested population — where every first-party session lands +— has creation but no scripted teardown, and `prune-merged.ps1` run from a worktree prints a green +"No sibling worktrees to consider" and exits 0. A wrong-cwd run reports a clean bill of health. + +### G12 — The gate has never produced a receipt + +`Write-Deny` writes JSON to stdout and exits 0. There is no log, no counter, no audit file. Nothing can +answer "how many drift events were prevented last month", "is G10's false-positive rate 1/day or 1/1000", +or "did the fix change anything". Every severity ranking above — including "highest-frequency event" — is +therefore unfalsifiable. A deny-logging line is smaller than any other fix here and is the prerequisite +for ranking the rest. + +### Considered and rejected + +- **Bare-repo layout** (no primary working tree to drift into) — structurally the strongest answer, but + `install-gate.ps1` derives the primary as the first `worktree list` entry and hard-requires + `/.git`, so the installer would abort; and the owner works in the primary by design. +- **WSL2 / containers / OS sandbox** — Anthropic's OS-level sandbox does not run on native Windows. This + is the load-bearing constraint: a fail-open `PreToolUse` hook is not a lazy substitute here, it is the + highest tier available without moving the whole stack. +- **Windows ACLs / restricted tokens** — principal-based, not process-based; needs dedicated local + accounts, which would fork `~/.claude`, the account-N settings, the venvs and the credential stores. +- **`git worktree lock` as hijack prevention** — prevents prune/move/delete only; no option makes a + worktree refuse a branch switch. +- **Native `Edit(/**)` permission deny** — deny beats allow with no exceptions, and + `.claude/worktrees/` is nested inside that path, so it would block every worktree too. Viable only + after a layout move, and it would lose the deny *text*, which is most of rule 1's value. +- **Device/UNC path bypass** (`//?/C:/…`) — real, probe-verified ALLOW, but requires a spelling no model + produces by accident. +- **`reference-transaction` hook to pin a worktree to its branch** — the only mechanism found that could + refuse a tree swap rather than a tool call, and the only one that would also cover the owner's own + terminal. But it could not be demonstrated that `git checkout` routes its HEAD update through a + transaction that fires the hook, and the hook sits on the path of every fetch/commit/rebase. Worth a + timeboxed spike that fails on purpose first; not worth a build on present evidence. + +--- + +## 4. Is this limiting ultracode? + +**Yes — rule 2 is, and it is the only rule that does. But the limitation is narrower than it has been +recorded as, and the correct response is not to weaken it.** + +**The mechanics.** Rule 2 denies `Task`/`Agent`/`Workflow` **only when the session's cwd is a governed +primary**. `Test-Governed` exempts `/.claude/worktrees/…`, and sibling worktrees fall outside +the root entirely — so **dispatch works from every worktree, nested or sibling**. What is blocked is a +Workflow launched from a session hand-started in the primary. Because the owner runs many VS Code windows +and opens the primary himself, that collision is frequent enough to *feel* like a general prohibition, +which is how it got recorded as "an ultracode Workflow CANNOT launch there". That reading is too broad. + +**But rule 2's stated premise has expired.** The rule's rationale is that a subagent "inherits the +parent's cwd, cannot create a worktree for itself, and its denied edits do not reliably surface to the +parent." + +The middle clause is now **false**, on first-party evidence: + +- The `Agent` tool exposes `isolation: "worktree"`, documented as "creates a temporary git worktree so the + agent works on an isolated copy of the repo." +- The worktrees documentation: *"Subagents can run in their own worktrees so parallel edits don't + conflict… add `isolation: worktree` to its frontmatter."* Claude Code also runs `git worktree lock` on + an agent's worktree while it is live, and sweeps it afterwards. +- Two changelog entries confirm it is a maintained surface: 2.1.210 (`isolation: 'worktree'` subagents + running git against the main checkout — fixed) and 2.1.216 (the `git -C` / `GIT_DIR` redirection fix). + +The first clause (cwd inheritance) still holds by default. The third — the empty `permission_denials` +list — is an **undocumented one-off observation**, and it is precisely the half that justifies a *deny* +rather than a warning. It has never been re-measured. + +**And rule 2 is leaky, so it pays the cost without buying the benefit.** It matches three tool names. +`Skill`, `spawn_task`, `CronCreate` and `RemoteTrigger` all start work and are unmatched at both the +settings matcher and the rule (probe-verified ALLOW at cwd = primary). A session in the primary that +wants fan-out can get it under another name; the rule mostly stops the *sanctioned* path. + +**Recommendation on rule 2 — keep the deny, fix the cause, don't broaden it blindly.** + +- **Keep it.** Rule 1 already denies subagents' writes into the primary keyed on target path, so rule 2's + marginal value is narrow — but the one thing it buys is real: it fails **fast and visibly**, at the + parent, instead of leaving a 40-minute fan-out to report success while writing nothing. +- **Do not add `Skill` to it.** That would block every slash command and skill invocation — `/code-review`, + `/security-review`, this repo's own skills — from a session the owner opened deliberately. `spawn_task` + is also wrong: it creates an advisory chip the user clicks; nothing inherits cwd until they act. + `CronCreate`/`RemoteTrigger` are the only defensible additions. +- **Remove the friction at the source** rather than at the deny: make worktree-first the default entry + point (§5, B6), so a primary-resident session is the exception rather than the normal case. +- **Do not install with `-NoDispatchGate`.** It drops the rule with no runtime trace, so a session cannot + tell whether rule 2 is live — G1 all over again. + +**Recommendation on rule 4 (`EnterWorktree`) — do not install it. Retire it.** + +The compounding argument in [WORKTREES.md](WORKTREES.md) is correct and is the decisive one: with rules 2 +and 4 both live, a primary-resident session has **no in-session path to isolation at all**. It cannot +dispatch, and it cannot relocate. It must be re-started elsewhere by a human. That is a hard stop on +workflow-by-default from the directory sessions naturally open in. + +Three further facts, all first-party: + +1. **The transcript relocation is designed behaviour, not a bug.** Since v2.1.198: *"When Claude enters or + exits a worktree that Claude Code created with git, the transcript follows: Claude Code records the + session under the session's new working directory, the same way `/cd` does, so `/desktop` and + `--resume` find it there. Exiting moves it back the same way."* Nothing is lost — the session is + **re-filed, and findable by `--resume`**. The residual is a session that *ends* while still inside, + which is a discoverability problem, and `sessions.ps1 -Rehome` already cures it. +2. **Upstream added its own guard where the risk is real.** Since v2.1.206, `EnterWorktree` into a path + **outside** `.claude/worktrees/` raises a confirmation prompt that no permission rule or "don't ask + again" can suppress — only `bypassPermissions` skips it. Our rule 4 would duplicate that for the + outside case and add a prohibition for the inside case, which is where every first-party worktree + lives and where the prompt was deliberately omitted. +3. Installed build is 2.1.217, so both behaviours are present today. + +**One caveat to carry:** a worktree created by a `WorktreeCreate` hook *keeps its transcript at the launch +directory* — so if we ever adopt such a hook to relocate worktrees (§5's rejected layout move), this +analysis must be redone. + +--- + +## 5. Better ways — ranked + +### Do this + +| # | Change | Closes | Effort | Wedge risk | +|---|---|---|---|---| +| **B1** | **Make the installed state observable.** A test that skips unless the installed gate exists, then asserts SHA-256 equality with the repo copy *and* that the union of live `PreToolUse` matchers supersets the gate's handled-tool list. It must **print what it scanned**, so a skip never reads as a pass, and it should emit a liveness receipt through the existing `scripts/quality/liveness.py` machinery rather than becoming a fifth blind local test. Add a `$GateVersion` to the gate; have `-Status` print the rule inventory and per-dir matchers against an **expectation**, not a bare count. Move the `CLAUDECODE` throw **below** the `-Status` branch so a session can audit but not install. | G1 | S | none | +| **B2** | **Log every deny.** One append to `~/.claude/hooks/worktree-gate.log`: timestamp, rule, tool, cwd, target, decision. | G12 | XS | none | +| **B3** | **One target-resolution helper, shared by rules 3 and 3b.** Parse `-C` **case-sensitively** (`-cmatch`, as 3b already does), then `cd`/`pushd`, then cwd; canonicalise; `Test-Governed` that. Invert 3b's early return: when it resolves the acted-on tree to the primary, **deny with rule 3's message** instead of bowing out. Add `--git-dir`, `--work-tree`, `GIT_DIR=`, `GIT_WORK_TREE=`. | G2, G3 | S/M | low — widens denials, so ship with B4 | +| **B4** | **Stop deciding from prose.** Exclude newline from the verb-scan class; drop quote characters from the git-detection class; strip quoted string literals before scanning. Share one "split into simple commands" helper with the blanket-stage hook so the two cannot disagree. Add ALLOW-asserting tests for a multi-line command, an echoed command, and a commit message containing a verb word. | G10 | S | none (strictly reduces denials) | +| **B5** | **Govern the gate's own surface.** Add `~/.claude/hooks/` and every wired `settings.json` to the governed set (or a user-scope `permissions.deny` on those paths). Add `config` to rule 3's surface: deny `core.hooksPath` / `core.worktree` / `alias.*` writes and `-c core.hooksPath=` overrides from any governed tree. Make `pre-commit install` use a repo-relative interpreter so no sibling worktree is load-bearing. Must not block the installers — gate on `CLAUDECODE`. | G5, G6 | S/M | low | +| **B6** | **Worktree-first as the default entry point.** Adopt `claude --worktree` / desktop auto-worktrees as *the* documented way to start a build session. Add a `.worktreeinclude` (none exists — `new.ps1` hand-copies the gitignored token list for exactly this reason). Use `isolation: worktree` on file-editing subagents. This is the field-standard answer and the only durable win: it reduces how often any rule has to fire. | reduces D1/D2 rate | S/M | low | +| **B7** | **Retire rule 4; add only `CronCreate`/`RemoteTrigger` to rule 2.** Correct the memory entry in the same commit or the next session re-adds the rule. **Note the doc numbering collision:** [WORKTREES.md](WORKTREES.md) calls the `EnterWorktree` rule "Rule 3" while the code's rule 3 is the git-verb rule — a commit saying "delete rule 4" will not match the doc it must also edit. | §4 | S | low | +| **B8** | **Fix the home-branch record, or delete it.** `extensions.worktreeConfig` is already on, so record intent as `git config --worktree mefor.homeBranch ` — the key that already exists — instead of the untracked `mefor-home-branch` file. Better still, read the registered branch from `git worktree list --porcelain`, which is authoritative and needs no sidecar. Either way, **stop printing a `git switch` command** — downgrade to "ask the user". Add tests for absent / matching / re-tasked / genuinely-switched. First settle the design question underneath it: **is re-tasking a nested worktree directory legal?** If yes, the record is wrong by design; if no, the missing teardown (G11) is the actual bug and G4 is a symptom. | G4 | S | medium if left as-is | +| **B9** | **Fix the deny message.** Compare `$root.Compare`, not the object; assert in a test that the primary's path never appears under "worktrees that already exist". | G7 | XS | none | +| **B10** | **Collapse the two allowlists; harden the second installer.** One allowlist path referenced by both scripts; `-Uninstall` removes it; give `install-selfheal.ps1` the `CLAUDECODE` throw, the multi-config-dir discovery loop, a `-Status` and an `-Uninstall`. Extend B1's check to assert the set of dirs carrying a gate matcher equals the set carrying the selfheal hook. | G8 | S | none | +| **B11** | **Close the verb and teardown holes.** A second alternation for hyphenated/two-token forms (`sparse-checkout`, `worktree remove|move`, `branch -f`, `update-ref`, `read-tree`, `rm`, `mv`, `checkout-index`, `bisect`), with its own message for `worktree remove` (cross-session destruction, not a tree swap); teach the detector about `gh`. Give `prune-merged.ps1` a **loud failure** when its root is not the primary instead of a green no-op, and add nested-worktree teardown. Correct the false rationale comment. | G9, G11 | M | low | + +**Order:** B1 and B2 first, together. Without them, none of the rest can be confirmed to have reached +production, and no severity claim in §3 is falsifiable. Then B9 (one character), B3+B4 as a pair, then +B5–B8, then B11. + +### Considered, not worth doing now + +| Idea | Why not | +|---|---| +| Move worktrees to a sibling root and express rule 1 as a native permission deny | Mechanically sound and removes a PowerShell process per tool call, but `--worktree`, the desktop app and subagent isolation all default to `.claude/worktrees/`. Relocating them needs a `WorktreeCreate` hook — which makes transcripts stay at the launch directory and disables `.worktreeinclude`, re-opening what B6 and B7 close. | +| A commit-time drift rule on `pre-commit` | Would make the backstop claim true and catches shell writes route-agnostically, but fires after the damage and needs a discriminator to avoid blocking the owner's own commits in the primary. **Minimum action instead: delete or caveat the unbacked backstop sentence** in the gate header and `install-git-hooks.ps1`. An unbacked backstop is worse than an admitted gap. | +| Bare repo / WSL2 / containers / ACLs / `worktree lock` / `reference-transaction` | See §3 "Considered and rejected". | + +--- + +## 6. Method and provenance + +Produced by a 22-agent adversarial workflow: 5 mapping agents (68 controls inventoried), 3 web-research +agents, 4 adversarial lenses (52 findings), 8 refutation agents over the high-severity findings +(7 confirmed, 1 downgraded), a synthesis pass and a completeness critic. Behavioural claims about the gate +were established by **piping crafted `PreToolUse` JSON into the installed hook and reading the emitted +decision**, not by reading source. + +**Verified fresh:** 85 tests pass; installed vs repo gate sizes and dates; the absence of an +`EnterWorktree` matcher in all five config dirs; the asymmetric `.claude-account-2.lock`; both allowlists' +contents; `extensions.worktreeConfig=true`; 3 nested + 5 sibling worktrees; Claude Code 2.1.217; the +2.1.198 / 2.1.206 / 2.1.210 / 2.1.216 behaviours (official docs + changelog); `isolation: worktree` +(tool schema + docs). + +**Cited, not re-measured — treat with care:** + +- **"29% of Edit/Write calls landed in a worktree; 44% in the primary; 166 sessions over 30 days."** From + the gate's own docstring. This is the *sole* quantitative justification for the target-keyed design. + Nothing in the repo lets it be recomputed, and nobody has asked whether it still holds. +- **"A subagent's denied edits came back with an empty `permission_denials` list."** The entire + evidentiary basis for rule 2 being a **deny** rather than a warning. One observation, undocumented, + never re-measured, and now partly overtaken by `isolation: worktree`. + +Both deserve a re-measurement before the next round of changes. diff --git a/docs/WORKTREE-GATE.md b/docs/WORKTREE-GATE.md index 2677e2c7..27573866 100644 --- a/docs/WORKTREE-GATE.md +++ b/docs/WORKTREE-GATE.md @@ -7,6 +7,10 @@ worktree completely untouched. It is opt-in, installed by hand, governs only the checkouts you list, and comes back out cleanly (`settings.json` is restored byte-for-byte). See [Backing it out](#backing-it-out). +> This page is the gate's own design rationale. For the gate **in context** — the whole drift-control +> estate, which parts are actually installed and enforcing, and an audit of what they miss — see +> [SESSION-DRIFT-CONTROLS.md](SESSION-DRIFT-CONTROLS.md). + --- ## Why it exists diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index 2b58ecb5..85116a65 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -91,6 +91,8 @@ they're committed to `main` (and fetched). `.claude/settings.json` is tracked (s ## The worktree gate (enforcement, not a reminder) > Full write-up, with the measurements and the backout procedure: [WORKTREE-GATE.md](WORKTREE-GATE.md). +> The whole estate as one system — every control's LIVE/INERT status, an audit of the gaps, and the +> ultracode question: [SESSION-DRIFT-CONTROLS.md](SESSION-DRIFT-CONTROLS.md). The `SessionStart` banner above **asks** you to work in a worktree. Measurement says asking doesn't work: across 30 days, 166 sessions ran with their cwd in the shared primary, and **44% of all their file writes From a3dc8eef081be1cf5d7476d530057683d5783172 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 08:46:29 -0500 Subject: [PATCH 02/12] feat(worktree-gate): leave a receipt on every deny, and stop offering the primary as a worktree to reuse Two defects, both in the part of the gate that talks back to the model. RECEIPTS. Write-Deny wrote its decision to stdout and exited 0, leaving no trace anywhere. Nothing on this box could answer "how many drift events did we prevent last month", "is the false-positive rate 1/day or 1/1000", or "did that change do anything" -- so every severity claim about this machinery, including which drift event is the most frequent, was an opinion. Each rule now stamps its own id into a log beside the allowlist. Attribution is the point: an undifferentiated count cannot separate rule 3's true positives from its false ones. Deliberately NOT logged: the raw command. Each rule passes a $Detail it composed itself -- a verb, a target path -- so an argument carrying a token cannot reach a plaintext log, and a test asserts that. Logging is best-effort and wrapped: a deny still goes out if the append fails. DENY MESSAGE. Rule 1 lists existing worktrees so a retry reuses one instead of minting another. The filter meant to drop the primary from that list compared a path string against the PSCustomObject that Test-Governed returns, so it was always -ne and never dropped anything: the message refused a write to the primary and then named the primary as the first thing to reuse, displacing a real worktree off the 8-item cap. Compare against the canonicalised form instead. The new test needs a REAL repo -- with no `git worktree list` to read, the hint section is absent and the assertion would be vacuous. Proved it catches the regression by reverting the one-token fix and watching it go red. Also adds $GateVersion so a later parity check can report which build is installed. --- scripts/hooks/worktree_gate.ps1 | 42 +++++-- tests/test_worktree_gate_receipts.py | 175 +++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 tests/test_worktree_gate_receipts.py diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 122954b2..d0fb4621 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -37,10 +37,33 @@ param( [string]$ReposFile = (Join-Path $env:USERPROFILE ".claude\hooks\worktree-gate.repos.txt") ) +# Bumped whenever a RULE's behaviour changes, so `install-gate.ps1 -Status` can report which build is +# actually installed. The installed gate is a COPY (see install-gate.ps1); without a version stamp the only +# way to tell a stale copy from a current one is a byte compare, and nothing was doing one -- which is how +# rule 4 sat unshipped for five days while every test reported it present. +$GateVersion = "2026.07.29.1" + # Fail OPEN: any unhandled error must let the tool call through, never block it. $ErrorActionPreference = "SilentlyContinue" -function Write-Deny([string]$Reason) { +function Write-Deny([string]$Reason, [string]$Rule = "?", [string]$Detail = "") { + # Leave a RECEIPT before denying. Until this existed the gate was unfalsifiable: it wrote its decision + # to stdout and exited 0, so nothing on the box could answer "how many drift events did we prevent", + # "is the false-positive rate 1/day or 1/1000", or "did that fix change anything" -- and every severity + # ranking about this machinery was therefore an opinion. Best-effort and never load-bearing: the log + # lives beside the allowlist, and if the append fails the deny still goes out. + # + # Deliberately NOT logged: the raw command or file contents. Each rule passes a $Detail it composed + # itself (a verb, a target path), so an argument carrying a secret cannot end up in a plaintext log. + try { + $logDir = Split-Path -Parent $ReposFile + if ($logDir -and (Test-Path -LiteralPath $logDir)) { + $stamp = (Get-Date).ToString("s") + $line = "$stamp`tv$GateVersion`trule=$Rule`ttool=$tool`tcwd=$cwdRaw`t$Detail" + Add-Content -LiteralPath (Join-Path $logDir "worktree-gate.log") -Value $line -Encoding utf8 + } + } catch { } + # The hookSpecificOutput WRAPPER IS MANDATORY. A bare {"permissionDecision":"deny"} is silently # ignored and the tool call proceeds (measured, and reported upstream as #4669 / #37210). $payload = @{ @@ -101,7 +124,7 @@ $cwdRaw = [string]$hook.cwd # original case: for `git -C` # wired in install-gate.ps1 alongside this change; delete it there and the wiring test goes red. # --------------------------------------------------------------------------------------------------- if ($tool -in @("EnterWorktree")) { - Write-Deny @" + Write-Deny -Rule "4" -Detail "relocate-session" -Reason @" BLOCKED: EnterWorktree relocates this live session into a worktree, which re-files its chat transcript under the worktree's slug and drops it from THIS window's session list (nothing is deleted -- it just stops appearing where you started). Do not relocate a running session. @@ -201,7 +224,7 @@ function Test-WorktreeHijack([string]$Verb, [string]$Cmd, [string]$CwdRaw) { if ($dest -eq $head) { return } $newHint = "$($gov.Display)\scripts\worktree\new.ps1" - Write-Deny @" + Write-Deny -Rule "3b" -Detail "git $Verb -> $selfTopRaw" -Reason @" BLOCKED: 'git $Verb $dest' would switch a LINKED WORKTREE ($selfTopRaw) onto the existing branch '$dest'. That worktree belongs to another session, which is building on '$head' right now. Switching it swaps every @@ -228,7 +251,7 @@ if ($tool -in @("Task", "Agent", "Workflow")) { $root = Test-Governed $cwd if ($root) { $display = $root.Display - Write-Deny @" + Write-Deny -Rule "2" -Detail "dispatch $tool" -Reason @" BLOCKED: this session is running in the SHARED PRIMARY checkout ($display), so it may not dispatch subagents. A subagent inherits this cwd, cannot create a worktree for itself, and its blocked edits do not reliably surface back to you -- the fan-out would appear to succeed while writing nothing. @@ -332,7 +355,7 @@ if ($tool -in @("Bash", "PowerShell")) { } $display = $root.Display - Write-Deny @" + Write-Deny -Rule "3" -Detail "git $verb" -Reason @" BLOCKED: 'git $verb' would change the working tree of the SHARED PRIMARY checkout ($display). Other sessions are standing in that directory right now. Switching its branch (or resetting, stashing or @@ -383,7 +406,12 @@ try { & git -C $display worktree list --porcelain 2>$null | Select-String -Pattern '^worktree (.+)$' | ForEach-Object { $_.Matches[0].Groups[1].Value } | - Where-Object { (Get-ComparablePath $_) -ne $root } + # `$root` is the PSCustomObject from Test-Governed, NOT a string -- comparing a path to it was + # always -ne, so the filter never removed anything and the PRIMARY ITSELF was listed first under + # "REUSE one if it is yours", displacing a real worktree off the 8-item cap below. The one part + # of this hook whose entire job is steering the next action was steering it back at the tree we + # had just refused. Compare against the canonicalised form. + Where-Object { (Get-ComparablePath $_) -ne $root.Compare } ) } catch { $worktrees = @() } @@ -392,7 +420,7 @@ $worktreeHint = if ($worktrees.Count -gt 0) { (($worktrees | Select-Object -First 8 | ForEach-Object { " $_" }) -join "`n") } else { "" } -Write-Deny @" +Write-Deny -Rule "1" -Detail $target -Reason @" BLOCKED: this write targets the SHARED PRIMARY checkout ($display), where concurrent sessions collide. This is a hard gate. Re-issuing the same edit will fail again -- do not retry it, and do not route around it with a shell command; that only hides the collision. diff --git a/tests/test_worktree_gate_receipts.py b/tests/test_worktree_gate_receipts.py new file mode 100644 index 00000000..410b298e --- /dev/null +++ b/tests/test_worktree_gate_receipts.py @@ -0,0 +1,175 @@ +"""The gate must leave a receipt when it denies, and its deny message must not misdirect. + +Until the receipt existed the gate was unfalsifiable. It wrote its decision to stdout and exited 0, so +nothing on the box could answer "how many drift events did we prevent last month", "is the false-positive +rate 1/day or 1/1000", or "did that fix change anything" -- which meant every severity claim about this +machinery was an opinion. These tests pin the receipt's existence, its rule attribution, and the one thing +it must never contain. + +The deny-message test is here for the same reason: the message is the part of the hook whose entire job is +steering the next action, and it was steering it back at the tree the gate had just refused. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import pytest + +from tests.test_worktree_gate import assert_denied, edit, run_gate # reuse the subprocess harness + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def shell(command: str, cwd: Path | str, tool: str = "Bash") -> dict[str, Any]: + return { + "session_id": "s-1", + "cwd": str(cwd), + "hook_event_name": "PreToolUse", + "tool_name": tool, + "tool_input": {"command": command}, + } + + +@pytest.fixture +def primary(tmp_path: Path) -> Path: + return tmp_path / "Repo" + + +@pytest.fixture +def repos_file(tmp_path: Path, primary: Path) -> Path: + f = tmp_path / "repos.txt" + f.write_text(f"{primary}\n", encoding="utf-8") + return f + + +def receipts(repos_file: Path) -> list[str]: + """The gate writes its log beside the allowlist, so the fixture's tmp dir isolates it per test.""" + log = repos_file.parent / "worktree-gate.log" + if not log.exists(): + return [] + return [ln for ln in log.read_text(encoding="utf-8").splitlines() if ln.strip()] + + +# --------------------------------------------------------------------------- the receipt exists + + +def test_a_denied_write_leaves_a_receipt(primary: Path, repos_file: Path) -> None: + assert_denied(run_gate(edit(primary / "src" / "app.py", primary), repos_file)) + lines = receipts(repos_file) + assert len(lines) == 1, f"expected exactly one receipt, got {lines}" + assert "rule=1" in lines[0] + assert "tool=Edit" in lines[0] + assert str(primary).replace("\\", "/").lower() in lines[0].replace("\\", "/").lower() + + +def test_an_allowed_call_leaves_no_receipt(tmp_path: Path, primary: Path, repos_file: Path) -> None: + """The log counts DENIALS. If allows were logged too, the count would measure traffic, not drift.""" + worktree = tmp_path / "Repo-alerts" / "src" / "app.py" + assert run_gate(edit(worktree, cwd=primary), repos_file) is None + assert receipts(repos_file) == [] + + +@pytest.mark.parametrize( + ("payload_kind", "expected_rule"), + [("dispatch", "rule=2"), ("git", "rule=3")], +) +def test_each_rule_stamps_its_own_id( + primary: Path, repos_file: Path, payload_kind: str, expected_rule: str +) -> None: + """Attribution is the point: 'the gate fired 40 times' is useless if you cannot tell rule 3's true + positives from rule 3's false positives.""" + if payload_kind == "dispatch": + payload: dict[str, Any] = { + "session_id": "s-1", + "cwd": str(primary), + "tool_name": "Task", + "tool_input": {"prompt": "go"}, + } + else: + payload = shell("git checkout somebranch", cwd=primary) + assert_denied(run_gate(payload, repos_file)) + assert expected_rule in receipts(repos_file)[0] + + +def test_the_receipt_never_records_the_raw_command(primary: Path, repos_file: Path) -> None: + """A command string is attacker- and operator-influenceable and can carry a token. The rules pass a + $Detail they composed themselves (a verb, a target path) precisely so the log cannot become a + plaintext secret store -- this is the assertion that keeps it that way.""" + # A deliberately BLAND sentinel. An imitation of a real credential format (glpat-, ghp-, AKIA...) + # trips this repo's own gitleaks pre-commit hook, so the guard against leaking secrets cannot itself + # be written with something that looks like one. + secret = "SENTINEL-MUST-NOT-REACH-THE-LOG" + assert_denied(run_gate(shell(f"git checkout main # {secret}", cwd=primary), repos_file)) + blob = "\n".join(receipts(repos_file)) + assert blob, "expected a receipt" + assert secret not in blob + assert "git checkout" in blob # the verb IS recorded -- that is the diagnostic value + + +def test_logging_failure_never_blocks_the_deny(primary: Path, tmp_path: Path) -> None: + """The receipt is best-effort. An allowlist in a directory the gate cannot append to must still DENY -- + a guardrail that stops guarding because its logger broke is worse than one that never logged.""" + missing = tmp_path / "gone" / "repos.txt" + missing.parent.mkdir() + missing.write_text(f"{primary}\n", encoding="utf-8") + shutil.rmtree(missing.parent, ignore_errors=True) + # The allowlist is gone too, so this asserts the weaker but sufficient property: no crash, exit 0. + assert run_gate(edit(primary / "x.py", primary), missing) is None + + +# --------------------------------------------------------------------------- the deny message + + +@pytest.mark.skipif(shutil.which("git") is None, reason="needs git on PATH") +def test_the_deny_message_does_not_offer_the_primary_as_a_worktree_to_reuse( + tmp_path: Path, +) -> None: + """Rule 1 lists the worktrees that already exist so a retry reuses one instead of minting another. The + filter that removes the primary from that list compared a path string to the PSCustomObject returned by + Test-Governed, so it was always -ne and never removed anything: the message refused a write to the + primary and then named the primary as the first thing to reuse. Needs a REAL repo -- with no git + worktree list to read, the hint section is absent and the assertion would be vacuous.""" + + def git(*args: str, cwd: Path | None = None) -> None: + subprocess.run( + ["git", *args], + cwd=str(cwd) if cwd else None, + check=True, + capture_output=True, + text=True, + ) + + primary = tmp_path / "Primary" + git("init", "-b", "main", str(primary)) + git("config", "user.email", "t@example.com", cwd=primary) + git("config", "user.name", "t", cwd=primary) + (primary / "seed.txt").write_text("seed\n", encoding="utf-8") + git("add", "-A", cwd=primary) + git("commit", "-m", "seed", cwd=primary) + wt = tmp_path / "Primary-wt" + git("worktree", "add", "-b", "wt-branch", str(wt), cwd=primary) + + repos = tmp_path / "repos.txt" + repos.write_text(f"{primary}\n", encoding="utf-8") + + reason = assert_denied(run_gate(edit(primary / "x.py", primary), repos)) + assert "REUSE" in reason, "the hint section must be present or this test proves nothing" + hint = reason.split("REUSE", 1)[1] + + def norm(p: str) -> str: + return p.replace("\\", "/").rstrip("/").lower() + + # git prints worktree paths with forward slashes even on Windows, so normalise before comparing -- + # matching the raw string would silently find nothing and the assertions below would both pass. + root = norm(str(tmp_path)) + listed = [norm(ln.strip()) for ln in hint.splitlines() if norm(ln.strip()).startswith(root)] + assert norm(str(wt)) in listed, f"the real worktree must be offered; got {listed}" + assert norm(str(primary)) not in listed, ( + f"the SHARED PRIMARY was offered as a worktree to reuse: {listed}" + ) From de2b43b09f4a4903aec5d6bb4098de287ee5f871 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 08:56:35 -0500 Subject: [PATCH 03/12] fix(worktree-gate): resolve the tree a git command acts on once, and stop reading verbs out of prose Rule 3 decided two things badly, in opposite directions. BYPASSES. Rule 3 resolved its target from `-C` or cwd only; rule 3b also resolved `cd`/`pushd`. A relatively-spelled `cd ../../.. && git reset --hard` fell between them: rule 3 saw the session's own worktree and handed off to 3b, which resolved the cd, saw the primary, and returned "Rule 3 owns it" -- rule 3 having already declined. Both bowed out. And 3b only handles checkout/switch, so for the other nine verbs there was no hand-off to bow out of. `../../..` is how a session in a nested worktree names the repo root, so this was reachable by accident, and seven spellings were measured ALLOW against the shipped hook. Rule 3 also parsed `-C` with `-match`, which is case-INsensitive in PowerShell, so git's lowercase `-c name=value` was captured as the path -- and being the first match, it shadowed a real `-C` later in the same command. Rule 3b already used `-cmatch` with a comment explaining why; the rule protecting the SHARED tree never got the fix. Both now go through one Get-GitActedOnPathRaw, and Get-ComparablePath takes the session cwd as the base so a relative path resolves against the session rather than against wherever pwsh started. FALSE POSITIVES. The verb scan read the raw command, so prose supplied verbs: a second line reading "echo about to merge stuff" denied with verb=merge; `echo "git checkout main"` denied; `git commit -m "chore: clean up dead code"` denied on `clean`. Two read-only commands were denied during the audit that found this. It is not nuisance value -- rule 3's deny text is the only control on the shell-write path, so a spurious deny erodes the one guard with nothing behind it. Scanning is now per line with quoted spans blanked, sharing the newline-splitting the sibling blanket-stage hook already did. A quoted PROGRAM path keeps its git token, or the fix would trade a false positive for a false negative. Every case is a test, and each test was proved to catch its own regression: five mutations (the -cmatch, the cd branch, the line split, the quote rewrite, the resolve base) were applied to the shipped script one at a time and all five went red. One test was rewritten after mutation showed it was blind: `git -c x=y checkout main` at cwd=primary now denies EITHER WAY, because the bogus path resolves inside the primary and is governed by accident. The discriminating shape is a real `-C` the config override would shadow. --- scripts/hooks/worktree_gate.ps1 | 105 ++++++++--- tests/test_worktree_gate_command_parsing.py | 198 ++++++++++++++++++++ 2 files changed, 281 insertions(+), 22 deletions(-) create mode 100644 tests/test_worktree_gate_command_parsing.py diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index d0fb4621..0a888edc 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -79,12 +79,70 @@ function Write-Deny([string]$Reason, [string]$Rule = "?", [string]$Detail = "") # Canonicalize before comparing. Without GetFullPath, `...\MessageFoundry-tpA\..\MessageFoundry\x.md` # does not string-match the primary's prefix and walks straight through the gate. -function Get-ComparablePath([string]$Path) { +function Get-ComparablePath([string]$Path, [string]$Base) { if (-not $Path) { return "" } - try { $full = [System.IO.Path]::GetFullPath($Path) } catch { return "" } + try { + # A RELATIVE path must resolve against the SESSION's cwd, not this hook process's. `../../..` is + # exactly how a session sitting in /.claude/worktrees/ names the repo root, and + # GetFullPath($Path) alone resolved it against wherever pwsh happened to be started -- so + # `cd ../../.. && git reset --hard` did not look like it touched the primary and was ALLOWED + # (measured, along with six other spellings). Callers that already have an absolute path may omit + # $Base; GetFullPath throws on a non-rooted base, which the catch turns into "not governed". + $full = if ($Base -and -not [System.IO.Path]::IsPathRooted($Path)) { + [System.IO.Path]::GetFullPath($Path, $Base) + } else { + [System.IO.Path]::GetFullPath($Path) + } + } catch { return "" } ($full -replace '\\', '/').TrimEnd('/').ToLowerInvariant() } +# Which working tree does a git command act on? ONE resolver, shared by rules 3 and 3b, because they used +# to have two and a real tree swap fell between them: rule 3 read only `-C` and cwd, so `cd && +# git reset --hard` spelled relatively resolved to the session's own (ungoverned) worktree and it handed +# off to 3b -- which resolved the `cd` correctly, saw the primary, and returned with the comment "Rule 3 +# owns it". Rule 3 had already declined. Both bowed out. Worse, 3b only handles checkout/switch, so for the +# other nine verbs there was no hand-off at all. +# +# Returns the RAW (original-case) path: rule 3b shells `git -C` with it, and on a case-sensitive +# filesystem a lowercased path misses the real directory and the whole rule silently fails open. +function Get-GitActedOnPathRaw([string]$Cmd, [string]$FallbackCwdRaw) { + # git's global `-C ` wins, read CASE-SENSITIVELY. `-match` is case-INsensitive in PowerShell, so + # git's lowercase `-c name=value` config override was captured as if it were a path: the "target" + # resolved to a nonexistent relative directory, which is not governed, and `git -c core.pager=cat + # checkout main` in the primary was ALLOWED (measured). Rule 3b already used -cmatch here for this + # exact reason; the rule protecting the SHARED tree never got the fix. + if ($Cmd -cmatch '(?:^|\s)-C\s+"?([^"\s]+)"?') { return $Matches[1] } + if ($Cmd -cmatch '(?:^|\s)--work-tree[=\s]+"?([^"\s]+)"?') { return $Matches[1] } + if ($Cmd -cmatch '(?:^|\s)GIT_WORK_TREE="?([^"\s]+)"?') { return $Matches[1] } + # `cd`/`pushd` are shell builtins, so match them case-insensitively (PowerShell accepts `CD`). + if ($Cmd -match '(?:^|\s)(?:cd|pushd)\s+"?([^"&|;]+?)"?\s*(?:&&|;|\||$)') { + return $Matches[1].Trim() + } + return $FallbackCwdRaw +} + +# Decide from the COMMAND, never from prose inside it. Three false positives were measured against the raw +# string: a two-line command whose second line read `echo about to merge stuff` denied with verb=merge (the +# scan class excludes `|;&` but not newline); `echo "git checkout main"` denied; and +# `git commit -m "chore: clean up dead code"` denied on `clean`. Each one teaches a session to route around +# the gate -- and per the design notes the deny text is the ONLY control on the shell-write path, so +# eroding compliance with it is not a nuisance, it is the guard itself. The sibling hook +# block-blanket-git-stage.ps1 already splits on newlines; these two must not disagree about what a command +# is. +function Get-ScannableCommandLines([string]$Cmd) { + foreach ($line in ($Cmd -split '\r?\n')) { + # A quoted PROGRAM path must keep its git token -- `"C:\Program Files\Git\bin\git.exe" checkout + # main` is a real spelling and blanking it wholesale would be a false NEGATIVE. Collapse that form + # to a bare token first, then blank every remaining quoted span. + $s = $line -replace '"[^"]*[\\/](git(?:\.exe)?)"', '$1' + $s = $s -replace "'[^']*[\\/](git(?:\.exe)?)'", '$1' + $s = $s -replace '"[^"]*"', '""' + $s = $s -replace "'[^']*'", "''" + $s + } +} + try { $hook = [Console]::In.ReadToEnd() | ConvertFrom-Json } catch { exit 0 } if (-not $hook) { exit 0 } @@ -167,17 +225,12 @@ function Test-Governed([string]$Candidate) { function Test-WorktreeHijack([string]$Verb, [string]$Cmd, [string]$CwdRaw) { if ($Verb -notin @("checkout", "switch")) { return } - # Which working tree does the command act on? Keep the RAW (original-case) path -- every `git -C` - # below MUST use it, never a Get-ComparablePath value: that form is lowercased, and on a - # case-sensitive filesystem (Linux CI) `git -C /tmp/.../primary-wt` misses the real `.../Primary-wt` - # dir, so the whole rule silently fails open. Get-ComparablePath is for allowlist comparison ONLY. - # Read git's global `-C ` case-SENSITIVELY so a lowercase `-c ` is not taken as a path. - # An explicit -C wins; else a leading `cd`/`pushd`; else the session's cwd (the case that happened). - $wtRaw = $CwdRaw - if ($Cmd -cmatch '(?:^|\s)-C\s+"?([^"\s]+)"?') { $wtRaw = $Matches[1] } - elseif ($Cmd -match '(?:^|\s)(?:cd|pushd)\s+"?([^"&|;]+?)"?\s*(?:&&|;|\||$)') { - $wtRaw = $Matches[1].Trim() - } + # Which working tree does the command act on? Get-GitActedOnPathRaw is now shared with rule 3 -- see + # its comment for why having two copies of this let a real tree swap fall between the rules. It keeps + # the RAW (original-case) path, which every `git -C` below MUST use: a Get-ComparablePath value is + # lowercased, and on a case-sensitive filesystem (Linux CI) `git -C /tmp/.../primary-wt` misses the + # real `.../Primary-wt` dir and the whole rule silently fails open. + $wtRaw = Get-GitActedOnPathRaw $Cmd $CwdRaw if (-not $wtRaw) { return } # Everything AFTER the first verb, up to the next command separator (so `git checkout x && ...` @@ -286,23 +339,31 @@ if ($tool -in @("Bash", "PowerShell")) { $cmd = [string]$hook.tool_input.command if (-not $cmd) { exit 0 } - # Match a git invocation however it is spelled: git, git.exe, or an absolute path to either. - if ($cmd -cnotmatch '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') { exit 0 } - # The verb must be a whole SUBCOMMAND. `\bmerge\b` is not enough: a hyphen counts as a word boundary, # so it also matches the `merge` inside `merge-base` and `merge-tree` -- both of which are READ-ONLY # and are exactly what a session should be using instead of a checkout. Require the verb to end at # whitespace or end-of-string, and list `cherry-pick` before `merge` so the alternation prefers it. # `[^|;&]*?` keeps the scan inside one command, so `git log | grep reset` is not a false positive. $verbs = 'cherry-pick|checkout|switch|reset|restore|stash|clean|rebase|merge|revert|am|apply' - if ($cmd -cnotmatch "\bgit(\.exe)?\b[^|;&]*?\s(?$verbs)(?=\s|$)") { exit 0 } - $verb = $Matches['verb'] - # Which repo does it act on? An explicit `-C ` wins over the session's cwd -- otherwise a + # Scan LINE BY LINE, with quoted spans blanked (Get-ScannableCommandLines). A verb must come from a + # git invocation on the same line and outside quotes, or prose supplies it. + $verb = $null + foreach ($scan in (Get-ScannableCommandLines $cmd)) { + # Match a git invocation however it is spelled: git, git.exe, or an absolute path to either. + if ($scan -cnotmatch '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') { continue } + if ($scan -cnotmatch "\bgit(\.exe)?\b[^|;&]*?\s(?$verbs)(?=\s|$)") { continue } + $verb = $Matches['verb'] + break + } + if (-not $verb) { exit 0 } + + # Which repo does it act on? `-C` > `--work-tree` > `cd`/`pushd` > the session's cwd -- otherwise a # session sitting in a worktree could reach INTO the primary with `git -C checkout x` and - # sail straight past a cwd-only check. - $target = $cwd - if ($cmd -match '(?:^|\s)-C\s+"?([^"\s]+)"?') { $target = Get-ComparablePath $Matches[1] } + # sail straight past a cwd-only check. Path parsing runs on the RAW command, never on the blanked + # scan string: the blanking that stops a commit message supplying a verb would also erase the path. + $targetRaw = Get-GitActedOnPathRaw $cmd $cwdRaw + $target = Get-ComparablePath $targetRaw $cwdRaw $root = Test-Governed $target # `cd ; git checkout ...` and `pushd` defeat both of the above, so also treat any command diff --git a/tests/test_worktree_gate_command_parsing.py b/tests/test_worktree_gate_command_parsing.py new file mode 100644 index 00000000..802efc01 --- /dev/null +++ b/tests/test_worktree_gate_command_parsing.py @@ -0,0 +1,198 @@ +"""How rule 3 reads a shell command: which tree it decides the command acts on, and where a verb may +come from. + +Two defect families live here, and they pull in opposite directions, which is why they are fixed and +tested together. + +BYPASSES (the gate said ALLOW to a real tree swap). Rule 3 resolved its target from `-C` or cwd only, +while rule 3b resolved `cd`/`pushd` as well -- so a relatively-spelled `cd && git reset --hard` +fell between them: rule 3 saw the session's own worktree and handed off to 3b, which saw the primary and +returned "Rule 3 owns it". Rule 3 had already declined. Separately, rule 3 parsed `-C` case-INsensitively, +so git's lowercase `-c name=value` config override was captured as if it were a path. + +FALSE POSITIVES (the gate said DENY to something that ran no git at all). The verb scan read the raw +command string, so a second line of prose, an echoed command, or a commit message could supply the verb. +Those matter more than nuisance value: rule 3's deny text is the only control on the shell-write path, so +every spurious deny erodes compliance with the one guard that has nothing behind it. + +Each case below was measured against the shipped hook before the fix. +""" + +from __future__ import annotations + +import shutil +from pathlib import Path +from typing import Any + +import pytest + +from tests.test_worktree_gate import assert_denied, run_gate # reuse the subprocess harness + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def shell(command: str, cwd: Path | str, tool: str = "Bash") -> dict[str, Any]: + return { + "session_id": "s-1", + "cwd": str(cwd), + "hook_event_name": "PreToolUse", + "tool_name": tool, + "tool_input": {"command": command}, + } + + +@pytest.fixture +def primary(tmp_path: Path) -> Path: + return tmp_path / "Repo" + + +@pytest.fixture +def nested(primary: Path) -> Path: + """A first-party worktree, where `../../..` IS the primary -- the natural relative spelling.""" + return primary / ".claude" / "worktrees" / "wt-1" + + +@pytest.fixture +def repos_file(tmp_path: Path, primary: Path) -> Path: + f = tmp_path / "repos.txt" + f.write_text(f"{primary}\n", encoding="utf-8") + return f + + +# ------------------------------------------------------------------ bypasses that must now be denied + + +def test_a_lowercase_config_override_does_not_shadow_the_real_dash_C( + nested: Path, repos_file: Path +) -> None: + """`-match` is case-insensitive in PowerShell, so the `-C ` parser captured git's lowercase + `-c name=value` config override instead -- and because it takes the FIRST match, a real `-C` later in + the same command was never seen at all. Here `-C ../../..` IS the primary, so reading `-c` as the path + resolves the target to a bogus directory inside the worktree, which is ungoverned, and the tree swap + is allowed. + + This shape, not the simpler `git -c x=y checkout main` at cwd=primary, is the one that discriminates. + That simpler case now denies EITHER WAY: since target paths resolve relative to the session cwd, the + bogus `core.pager=cat` lands inside the primary and is governed by accident. A test written on it + passes against the bug -- verified by mutation -- and would have shipped as a green blind spot.""" + reason = assert_denied( + run_gate(shell("git -c core.pager=cat -C ../../.. checkout main", cwd=nested), repos_file) + ) + assert "SHARED PRIMARY" in reason + + +def test_a_config_override_whose_value_looks_like_an_outside_path_still_denies( + primary: Path, repos_file: Path +) -> None: + """The second discriminator: a config value that resolves OUTSIDE the primary. Read as a path it makes + the command look ungoverned; read correctly, the target falls back to the cwd, which is the primary.""" + assert_denied( + run_gate( + shell("git -c include.path=C:/Windows/Temp/x checkout main", cwd=primary), repos_file + ) + ) + + +def test_a_config_override_does_not_break_an_ordinary_deny(primary: Path, repos_file: Path) -> None: + assert_denied( + run_gate(shell("git -c advice.detachedHead=false switch main", cwd=primary), repos_file) + ) + + +@pytest.mark.parametrize( + "command", + [ + "cd ../../.. && git checkout main", + "cd ../../.. && git reset --hard origin/main", + "cd ../../.. ; git clean -xfd", + "cd ../../.. ; git stash", + "cd ../../.. && git rebase origin/main", + "pushd ../../.. ; git restore .", + "cd ../../.. && git switch main", + ], +) +def test_a_relative_cd_to_the_primary_is_denied( + nested: Path, repos_file: Path, command: str +) -> None: + """The whole point of the fix. `../../..` from a nested worktree IS the primary, but the in-text + fallback only ever matched the primary's LITERAL spelling, so none of these named it. Note the list + spans nine verbs: rule 3b handles only checkout/switch, so for the other seven there was never even a + hand-off to bow out of -- they were plain misses.""" + reason = assert_denied(run_gate(shell(command, cwd=nested), repos_file)) + assert "SHARED PRIMARY" in reason + + +def test_a_relative_work_tree_flag_pointing_at_the_primary_is_denied( + nested: Path, repos_file: Path +) -> None: + """`--work-tree` is resolved structurally. Spelled relatively it never appears in the in-text scan, so + only the resolver can catch it.""" + assert_denied(run_gate(shell("git --work-tree=../../.. checkout main", cwd=nested), repos_file)) + + +def test_a_relative_cd_that_leaves_the_primary_is_allowed(primary: Path, repos_file: Path) -> None: + """The resolver must cut both ways: resolving `cd` means a session in the primary that cds OUT of it + is no longer denied on the strength of its cwd alone. This is a false positive the fix removes.""" + assert run_gate(shell("cd ../Elsewhere && git checkout main", cwd=primary), repos_file) is None + + +# ------------------------------------------------------- false positives that must now be allowed + + +def test_a_verb_on_a_later_line_of_prose_is_not_a_git_verb(primary: Path, repos_file: Path) -> None: + """The scan class excludes `|;&` but not newline, so a `merge` five words into a second line of + narration was read as the subcommand of the `git` on line one. Denied a command that ran `git status`.""" + assert run_gate(shell("git status\necho about to merge stuff", cwd=primary), repos_file) is None + + +def test_an_echoed_command_is_not_a_command(primary: Path, repos_file: Path) -> None: + """The git-detection class includes the quote characters, so quoting a command for display made it + look like an invocation. This denied a read-only diagnostic during the audit that produced this fix.""" + assert run_gate(shell('echo "git checkout main"', cwd=primary), repos_file) is None + + +@pytest.mark.parametrize( + "message", + [ + "chore: clean up dead code", + "fix: I am about to merge the release branch", + "refactor: reset the counters on restart", + "docs: restore the missing section", + ], +) +def test_a_commit_message_cannot_supply_the_verb( + primary: Path, repos_file: Path, message: str +) -> None: + """`git commit` is not a gated verb, but the message that follows it was scanned as if it were + arguments. Committing in the primary is ordinary, sanctioned work -- and this denied it based on a + word in the subject line.""" + assert run_gate(shell(f'git commit -m "{message}"', cwd=primary), repos_file) is None + + +# ---------------------------------------------------- what the quote handling must NOT break + + +def test_a_quoted_program_path_still_supplies_its_git_token( + primary: Path, repos_file: Path +) -> None: + """Blanking quoted spans is what stops a commit message supplying a verb -- but a quoted absolute + program path is a REAL spelling, and blanking it wholesale would erase the git token and turn the fix + into a false negative. The program-path form is collapsed to a bare token first.""" + cmd = '"C:\\Program Files\\Git\\bin\\git.exe" checkout main' + assert_denied(run_gate(shell(cmd, cwd=primary), repos_file)) + + +def test_the_absolute_spellings_still_deny(tmp_path: Path, primary: Path, repos_file: Path) -> None: + """Regression guard: the structured resolver must not displace the in-text fallback for the literal + spellings that already worked.""" + worktree = tmp_path / "Repo-alerts" + assert_denied(run_gate(shell(f'git -C "{primary}" checkout main', cwd=worktree), repos_file)) + assert_denied(run_gate(shell(f"cd {primary} && git checkout main", cwd=worktree), repos_file)) + + +def test_a_worktrees_own_relative_work_is_still_allowed(nested: Path, repos_file: Path) -> None: + """A worktree owns its own history. Resolving `cd` must not start denying a worktree's own commands.""" + assert run_gate(shell("cd . && git switch -c feature/x", cwd=nested), repos_file) is None + assert run_gate(shell("git rebase origin/main", cwd=nested), repos_file) is None From 46ece656a46d31ac0adfdebbd7e73d3c50fb4072 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 09:00:19 -0500 Subject: [PATCH 04/12] feat(worktree-gate): make the INSTALLED gate observable, and stop rule 4 activating as a side effect The gate runs from a copy under ~/.claude/hooks/. Nothing compared that copy to the repo, so drift was invisible in both directions: rule 4 sat unshipped for five days while 85 tests reported it present, and the reverse -- a rule deleted from source but still enforced by a stale copy -- would be equally silent. Fixing regexes is pointless while nothing can confirm a fix reached production. -STATUS BECOMES AN AUDIT. It now prints the version and SHA of both copies with an explicit IN SYNC / STALE verdict, and per config dir the matchers actually wired against the rules the INSTALLED script implements -- an expectation, not the old bare count, which was uninterpretable without knowing what the right count was. The CLAUDECODE refusal now sits BELOW the -Status branch. It used to precede it, so a session could not audit the gate through the supported interface at all -- it could neither see that the running gate was stale nor find out which rules were live. Auditing is not installing; installing stays a human act and still throws. RULE 4 BECOMES OPT-IN (-EnterWorktreeGate, default off). The matcher list was unconditional, so re-running the installer to pick up any of these fixes would have silently activated it -- and with rules 2 and 4 both live, a session started in the primary has no in-session path to isolation at all: it can neither dispatch a subagent nor relocate itself. That is a hard stop on workflow-by-default from the directory sessions naturally open in, and it is the owner's call (docs/WORKTREES.md), not a side effect of a regex fix. It also duplicates a vendor guard added in 2.1.206. THE PARITY TEST. Local-machine, skips on CI with the reason printed so a skip is never read as a pass, and only asserts when the source is COMMITTED -- mid-edit the copies are supposed to differ, and a test that nagged on every keystroke would be deleted. It is RED on this box right now, correctly: the installed gate is the Jul 24 build. It goes green when the owner runs, from a plain terminal: pwsh -NoProfile -File scripts\worktree\install-gate.ps1 The repo-side wiring test could never have caught this -- it compares the installer to the script and never looks at what is installed. It now also pins what a BARE install turns on, so an opt-in switch cannot quietly become the default. --- scripts/worktree/install-gate.ps1 | 94 +++++++++++++-- tests/test_gate_installed_parity.py | 170 ++++++++++++++++++++++++++++ tests/test_install_gate_wiring.py | 65 ++++++++++- 3 files changed, 316 insertions(+), 13 deletions(-) create mode 100644 tests/test_gate_installed_parity.py diff --git a/scripts/worktree/install-gate.ps1 b/scripts/worktree/install-gate.ps1 index 37901769..39f94dd1 100644 --- a/scripts/worktree/install-gate.ps1 +++ b/scripts/worktree/install-gate.ps1 @@ -50,16 +50,26 @@ param( [switch]$Status, # Do not gate Task/Agent/Workflow dispatch from the primary (writes are still gated). [switch]$NoDispatchGate, + # Gate the EnterWorktree tool (rule 4), which relocates a LIVE session into a worktree. + # + # OPT-IN, and deliberately OFF by default. Rule 4 has never been installed, and turning it on as a + # SIDE EFFECT of installing an unrelated fix would be a trap: with rules 2 and 4 both live, a session + # started in the primary has no in-session path to isolation at all -- it can neither dispatch a + # subagent nor relocate itself, so it must be restarted elsewhere by a human. That is a hard stop on + # workflow-by-default from the directory sessions naturally open in, and it is a decision the owner + # makes on purpose (docs/WORKTREES.md), not one that rides along with a regex fix. + # + # It also duplicates a guard the vendor now ships: since v2.1.206 EnterWorktree into a path OUTSIDE + # .claude/worktrees/ raises a confirmation prompt that no permission rule can suppress, and since + # v2.1.198 the transcript follows the session's cwd BOTH ways, so relocation re-files a chat rather + # than losing it. See docs/SESSION-DRIFT-CONTROLS.md. + [switch]$EnterWorktreeGate, # Config dirs to wire the hook into. Default: ~/.claude plus every existing ~/.claude-account-*. [string[]]$ConfigDir ) $ErrorActionPreference = "Stop" -if ($env:CLAUDECODE -eq "1") { - throw "Refusing to run inside Claude Code. A session that can install this gate can also remove it. Run from a plain pwsh terminal." -} - $RepoRoot = (Resolve-Path (Join-Path $PSScriptRoot "..\..")).Path # The gate SCRIPT + its allowlist live ONCE, shared, under ~/.claude\hooks -- referenced by absolute path @@ -115,10 +125,50 @@ function Remove-GateHooks($Data) { return $Data } +function Get-GateVersion([string]$Path) { + if (-not (Test-Path -LiteralPath $Path)) { return $null } + $m = [regex]::Match((Get-Content -LiteralPath $Path -Raw), '\$GateVersion\s*=\s*"([^"]+)"') + if ($m.Success) { $m.Groups[1].Value } else { "(unstamped)" } +} + +function Get-GateHash([string]$Path) { + if (-not (Test-Path -LiteralPath $Path)) { return $null } + (Get-FileHash -LiteralPath $Path -Algorithm SHA256).Hash +} + +# Every tool the gate script branches on. Read from the SOURCE, so -Status can say which implemented rules +# are unwired rather than printing a bare count nobody can calibrate. +function Get-HandledTools([string]$Path) { + if (-not (Test-Path -LiteralPath $Path)) { return @() } + $text = Get-Content -LiteralPath $Path -Raw + $tools = [System.Collections.Generic.HashSet[string]]::new() + foreach ($m in [regex]::Matches($text, '\$tool\s+-(?:not)?in\s+@\(([^)]*)\)')) { + foreach ($q in [regex]::Matches($m.Groups[1].Value, '"([^"]+)"')) { $null = $tools.Add($q.Groups[1].Value) } + } + @($tools) +} + # ------------------------------------------------------------------------------------------ status +# NB this branch runs BEFORE the CLAUDECODE refusal below, deliberately. Auditing is not installing, and +# a session that cannot see whether the gate is current has no way to notice the exact failure that let +# rule 4 sit unshipped for five days while every test reported it present. Installing stays a human act. if ($Status) { - $installed = Test-Path -LiteralPath $GateDst - Write-Host "gate script : $(if ($installed) { "installed -> $GateDst" } else { 'NOT installed' })" + $srcGate = Join-Path $RepoRoot "scripts\hooks\worktree_gate.ps1" + $iVer = Get-GateVersion $GateDst ; $sVer = Get-GateVersion $srcGate + $iSha = Get-GateHash $GateDst ; $sSha = Get-GateHash $srcGate + + Write-Host "installed : $(if ($iSha) { "$GateDst v$iVer" } else { 'NOT installed' })" + Write-Host "source : $(if ($sSha) { "$srcGate v$sVer" } else { 'NOT FOUND' })" + if ($iSha -and $sSha) { + if ($iSha -eq $sSha) { + Write-Host "parity : IN SYNC" -ForegroundColor Green + } else { + Write-Host "parity : *** STALE *** the running gate is NOT this checkout's script." -ForegroundColor Red + Write-Host " Re-run this installer to update it. Until you do, rules added or" + Write-Host " removed in source have no effect, and the tests still pass." + } + } + if (Test-Path -LiteralPath $ReposFile) { Write-Host "governing :" Get-Content -LiteralPath $ReposFile | Where-Object { $_ -and -not $_.StartsWith('#') } | @@ -126,15 +176,39 @@ if ($Status) { } else { Write-Host "governing : nothing (no allowlist -> gate is OFF)" } + + # Compare the wired matchers against the rules the INSTALLED script actually implements -- an + # expectation, not a count. A count of "3" is not information unless you know whether 3 is right. + $handled = @(Get-HandledTools $GateDst) foreach ($cd in $ConfigDir) { $sp = Join-Path $cd "settings.json" $s = Read-Settings $sp - $n = @($s.hooks.PreToolUse | Where-Object { @($_.hooks) | Where-Object { "$($_.command)" -like "*$Marker*" } }).Count - Write-Host "hook entries: $n in $sp" + $wired = [System.Collections.Generic.HashSet[string]]::new() + foreach ($e in @($s.hooks.PreToolUse)) { + if (@($e.hooks) | Where-Object { "$($_.command)" -like "*$Marker*" }) { + foreach ($t in "$($e.matcher)".Split("|")) { if ($t) { $null = $wired.Add($t) } } + } + } + $missing = @($handled | Where-Object { -not $wired.Contains($_) } | Sort-Object) + $stray = @($wired | Where-Object { $handled -notcontains $_ } | Sort-Object) + Write-Host "wiring : $sp" + Write-Host " matched : $(@($wired | Sort-Object) -join ', ')" + if ($missing) { + Write-Host " UNWIRED : $($missing -join ', ') <- implemented but NEVER FIRES" -ForegroundColor Yellow + } + if ($stray) { + Write-Host " stray : $($stray -join ', ') <- matched but the script ignores it" -ForegroundColor Yellow + } } + Write-Host "" + Write-Host "scanned $($ConfigDir.Count) config dir(s) against $(@($handled).Count) implemented rule(s)." return } +if ($env:CLAUDECODE -eq "1") { + throw "Refusing to run inside Claude Code. A session that can install this gate can also remove it. Run from a plain pwsh terminal. (-Status is allowed from a session: auditing is not installing.)" +} + # --------------------------------------------------------------------------------------- uninstall if ($Uninstall) { foreach ($cd in $ConfigDir) { @@ -185,11 +259,13 @@ $command = "pwsh -NoProfile -File `"$GateDst`"" $matchers = @( "Write|Edit|MultiEdit|NotebookEdit" # rule 1 -- writes INTO the primary's tree "Bash|PowerShell" # rules 3 + 3b -- git verbs that swap the primary / hijack a worktree - "EnterWorktree" # rule 4 -- relocating a live session (loses its transcript) ) if (-not $NoDispatchGate) { $matchers += "Task|Agent|Workflow" # rule 2 -- subagent dispatch FROM the primary } +if ($EnterWorktreeGate) { + $matchers += "EnterWorktree" # rule 4 -- OPT-IN, see the parameter's note for why +} $entries = foreach ($m in $matchers) { [ordered]@{ diff --git a/tests/test_gate_installed_parity.py b/tests/test_gate_installed_parity.py new file mode 100644 index 00000000..85cf5c4f --- /dev/null +++ b/tests/test_gate_installed_parity.py @@ -0,0 +1,170 @@ +"""Does the gate that is RUNNING match the gate that is in the repo? + +Nothing answered this, and that gap is the root cause of every other defect found in the drift machinery. +The gate executes from an installed COPY under ``~/.claude/hooks/``; ``install-gate.ps1`` copies it with no +version, hash or marker, and its ``-Status`` printed an uncalibrated count of hook entries. So: + +* Rule 4 was implemented, declared by the installer, and covered by tests -- and was absent from the + installed script and from every matcher set. 85 tests passed the whole time. +* The reverse is worse and equally invisible: delete a rule from source and the stale installed copy keeps + enforcing it forever, while every test correctly reports it gone. + +These tests are LOCAL-MACHINE tests. On CI there is no installed gate and they skip -- which is honest, +because the drift they detect is a developer-box condition, not a repository one. They print what they +scanned so a skip can never be mistaken for a pass. + +Parity is asserted only when the source script is COMMITTED. Mid-change the two are *supposed* to differ, +and a test that nagged on every edit would be re-run with ``-k`` until someone deleted it. +""" + +from __future__ import annotations + +import hashlib +import json +import re +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +SOURCE_GATE = ROOT / "scripts" / "hooks" / "worktree_gate.ps1" +INSTALLED_GATE = Path.home() / ".claude" / "hooks" / "worktree_gate.ps1" + +# Rules deliberately shipped unwired. Their ABSENCE from a matcher set is a decision, not drift; their +# presence in the script is not evidence that they fire. Keep this list short and justified -- it is the +# one place a rule may hide from the wiring assertion, so an unexplained entry here is a defect. +# +# EnterWorktree (rule 4) -- opt-in via `install-gate.ps1 -EnterWorktreeGate`. It compounds with rule 2 +# to leave a primary-resident session no in-session path to isolation, and the transcript-loss defect it +# guards was addressed upstream. See docs/SESSION-DRIFT-CONTROLS.md §4. +OPT_IN_TOOLS = {"EnterWorktree"} + +TOOL_BRANCH = re.compile(r"\$tool\s+-(?:not)?in\s+@\(([^)]*)\)") +QUOTED = re.compile(r'"([^"]+)"') + + +def handled_tools(text: str) -> set[str]: + tools: set[str] = set() + for group in TOOL_BRANCH.findall(text): + tools.update(QUOTED.findall(group)) + return tools + + +def config_dirs() -> list[Path]: + """Every Claude config dir on this box: ~/.claude plus the ~/.claude-account-* VS Code launchers.""" + home = Path.home() + found = [home / ".claude"] + sorted(home.glob(".claude-account-*")) + return [d for d in found if (d / "settings.json").is_file()] + + +def wired_matchers(settings: Path) -> set[str]: + """Tool names reachable through a PreToolUse entry whose command names the gate.""" + try: + data = json.loads(settings.read_text(encoding="utf-8-sig")) + except (OSError, json.JSONDecodeError): + return set() + tools: set[str] = set() + for entry in data.get("hooks", {}).get("PreToolUse", []) or []: + cmds = " ".join(str(h.get("command", "")) for h in entry.get("hooks", []) or []) + if "worktree_gate.ps1" not in cmds: + continue + tools.update(t for t in str(entry.get("matcher", "")).split("|") if t) + return tools + + +def source_is_committed() -> bool: + try: + out = subprocess.run( + ["git", "status", "--porcelain", "--", str(SOURCE_GATE.relative_to(ROOT))], + cwd=ROOT, + capture_output=True, + text=True, + timeout=30, + ) + except (OSError, subprocess.SubprocessError): + return False + return out.returncode == 0 and not out.stdout.strip() + + +def test_the_installed_gate_matches_the_committed_source() -> None: + if not INSTALLED_GATE.is_file(): + pytest.skip( + f"no gate installed at {INSTALLED_GATE} -- nothing is enforcing; nothing to compare" + ) + if not source_is_committed(): + pytest.skip( + f"{SOURCE_GATE.relative_to(ROOT)} has uncommitted changes -- the installed copy is SUPPOSED " + f"to differ mid-edit. Re-run after committing." + ) + + installed = hashlib.sha256(INSTALLED_GATE.read_bytes()).hexdigest() + source = hashlib.sha256(SOURCE_GATE.read_bytes()).hexdigest() + print(f"scanned: {INSTALLED_GATE} ({installed[:12]}) vs {SOURCE_GATE} ({source[:12]})") + + assert installed == source, ( + f"The RUNNING gate is not this checkout's script.\n" + f" installed: {INSTALLED_GATE} sha={installed[:12]}\n" + f" source : {SOURCE_GATE} sha={source[:12]}\n" + f"Until it is re-installed, rules added or removed in source have NO EFFECT and the rest of the " + f"suite still passes. Fix from a PLAIN terminal:\n" + f" pwsh -NoProfile -File scripts\\worktree\\install-gate.ps1" + ) + + +def test_every_wired_matcher_names_a_tool_the_gate_handles() -> None: + """The inverse drift: a matcher for a tool the script ignores burns a pwsh subprocess on every call, + and -- worse -- reads as coverage that does not exist.""" + dirs = config_dirs() + if not dirs: + pytest.skip("no Claude config dirs on this box -- nothing is wired, so nothing to check") + if not INSTALLED_GATE.is_file(): + pytest.skip( + f"no gate installed at {INSTALLED_GATE} -- matchers cannot be judged against it" + ) + + handled = handled_tools(INSTALLED_GATE.read_text(encoding="utf-8")) + print(f"scanned {len(dirs)} config dir(s) against {len(handled)} rule(s) in the INSTALLED gate") + stray: dict[str, set[str]] = {} + for d in dirs: + wired = wired_matchers(d / "settings.json") + print(f" {d.name}: {sorted(wired) or '(none)'}") + if extra := wired - handled: + stray[d.name] = extra + assert not stray, f"matchers for tools the installed gate never inspects: {stray}" + + +def test_every_non_optional_rule_is_wired_in_every_config_dir() -> None: + """A rule the script implements but no matcher names NEVER FIRES, and nothing says so. This is the + check that would have caught rule 4 on day one -- the repo-side wiring test could not, because it + compares the installer to the script and never looks at what is actually installed.""" + dirs = config_dirs() + if not dirs: + pytest.skip("no Claude config dirs on this box -- nothing is wired, so nothing to check") + if not INSTALLED_GATE.is_file(): + pytest.skip(f"no gate installed at {INSTALLED_GATE} -- there is no live rule set to wire") + + handled = handled_tools(INSTALLED_GATE.read_text(encoding="utf-8")) + required = handled - OPT_IN_TOOLS + print( + f"scanned {len(dirs)} config dir(s); require {sorted(required)}; opt-in {sorted(OPT_IN_TOOLS)}" + ) + + unwired: dict[str, list[str]] = {} + for d in dirs: + if missing := sorted(required - wired_matchers(d / "settings.json")): + unwired[d.name] = missing + assert not unwired, ( + f"rules implemented by the installed gate but wired in no matcher, so they never fire: {unwired}. " + f"Re-run install-gate.ps1 from a plain terminal, or add the tool to OPT_IN_TOOLS with a reason." + ) + + +def test_the_opt_in_list_only_names_tools_the_gate_actually_has() -> None: + """Guard the exemption. A stale name in OPT_IN_TOOLS would silently excuse a future rule that happened + to reuse it -- the exemption must track the script, not outlive it.""" + handled = handled_tools(SOURCE_GATE.read_text(encoding="utf-8")) + print(f"opt-in: {sorted(OPT_IN_TOOLS)}; source handles: {sorted(handled)}") + assert handled >= OPT_IN_TOOLS, ( + f"OPT_IN_TOOLS names {sorted(OPT_IN_TOOLS - handled)}, which the gate no longer implements" + ) diff --git a/tests/test_install_gate_wiring.py b/tests/test_install_gate_wiring.py index 709c481d..58bcc235 100644 --- a/tests/test_install_gate_wiring.py +++ b/tests/test_install_gate_wiring.py @@ -34,16 +34,42 @@ def tools_the_gate_handles() -> set[str]: return tools -def tools_the_installer_registers() -> set[str]: - """Every tool name reachable through the installer's PreToolUse matchers.""" +def matcher_block() -> str: text = INSTALLER.read_text(encoding="utf-8") - block = text.split("$matchers = @(", 1)[1].split("$entries", 1)[0] + return text.split("$matchers = @(", 1)[1].split("$entries", 1)[0] + + +def tools_the_installer_registers() -> set[str]: + """Every tool name the installer CAN register -- including the ones behind an opt-in switch. + + This is deliberately the permissive reading. It answers "did someone add a rule and forget the + matcher entirely", which is the drift this file exists for. It does NOT answer "does the default + install wire it" (see below) and it cannot answer "is it wired on this machine" at all -- that needs + the live settings.json, which is tests/test_gate_installed_parity.py. + """ tools: set[str] = set() - for matcher in QUOTED.findall(block): + for matcher in QUOTED.findall(matcher_block()): tools.update(matcher.split("|")) return tools +def tools_registered_by_default() -> set[str]: + """What a bare `install-gate.ps1` writes: the unconditional array, plus blocks guarded by a NEGATED + switch (`-not $NoDispatchGate` is on unless you opt out). A block guarded by a plain `if ($Switch)` + is opt-IN and contributes nothing by default.""" + block = matcher_block() + tools: set[str] = set() + unconditional, _, rest = block.partition("if (") + for matcher in QUOTED.findall(unconditional): + tools.update(matcher.split("|")) + for chunk in ("if (" + rest).split("if (")[1:]: + guard, _, body = chunk.partition(")") + if "-not" in guard: # opt-OUT: on unless suppressed + for matcher in QUOTED.findall(body): + tools.update(matcher.split("|")) + return tools + + def test_the_gate_handles_the_tools_we_expect() -> None: """Guard the guard: if a rule is added or removed, this test should be the thing that notices.""" assert tools_the_gate_handles() == { @@ -78,3 +104,34 @@ def test_the_installer_does_not_register_tools_the_gate_ignores() -> None: assert not stray, ( f"install-gate.ps1 matches {sorted(stray)}, which worktree_gate.ps1 never inspects." ) + + +def test_the_default_install_wires_rules_1_2_and_3_and_nothing_else() -> None: + """Pin what a bare `install-gate.ps1` actually turns on. + + The permissive test above is satisfied by a matcher sitting behind an opt-in switch, which is exactly + how a rule can be "registered by the installer" and still never fire. Rule 4 (EnterWorktree) is + deliberately opt-in -- it compounds with rule 2 to leave a primary-resident session no in-session path + to isolation, so activating it as a side effect of installing an unrelated fix would be a trap. That + decision belongs in this assertion, where changing it is visible, rather than in a switch nobody reads. + """ + assert tools_registered_by_default() == { + "Write", + "Edit", + "MultiEdit", + "NotebookEdit", + "Bash", + "PowerShell", + "Task", + "Agent", + "Workflow", + } + + +def test_every_opt_in_tool_is_guarded_by_a_plain_switch() -> None: + """The opt-in must be real: a tool named outside a guard is on by default whatever the docs say.""" + opt_in = tools_the_installer_registers() - tools_registered_by_default() + assert opt_in == {"EnterWorktree"}, f"unexpected opt-in set: {sorted(opt_in)}" + assert re.search(r"if \(\$EnterWorktreeGate\)", matcher_block()), ( + "EnterWorktree must be added inside an `if ($EnterWorktreeGate)` block" + ) From 773cb08e3e38139ee7a40609853783c5e41d82d5 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 09:11:52 -0500 Subject: [PATCH 05/12] docs: record which drift gaps are now closed, and that none of it is live until re-install Marks G1, G2, G3, G7, G10 and G12 fixed, updates the control table for the deny log, the parity check and rule 4's move from accidentally-inert to opt-in, and replaces the recommended ordering with a status. Keeps the caveat in front: the gate runs from an installed copy, so every fix on this branch is inert until `install-gate.ps1` is re-run from a plain terminal. That is the same property that left rule 4 unshipped -- the difference now is that a test watches it, and that test is red on this box until the re-install happens. --- docs/SESSION-DRIFT-CONTROLS.md | 45 +++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 8f877f24..401997b5 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -19,6 +19,20 @@ tree under everyone simultaneously. Companion docs: [WORKTREES.md](WORKTREES.md) (how to use worktrees), [WORKTREE-GATE.md](WORKTREE-GATE.md) (the gate's own design rationale and backout), [LEDGER-GATE.md](LEDGER-GATE.md) (the number space). +> **Fixed since this audit ran** — G2, G3, G7 and G12 are closed in the repo, and rule 4 is now opt-in +> (`install-gate.ps1 -EnterWorktreeGate`) so re-installing cannot activate it by accident. Each fixed gap +> is marked below. +> +> **They are not live yet.** The gate executes from an installed copy; none of it takes effect until, from +> a plain terminal: +> +> ```powershell +> pwsh -NoProfile -File scripts\worktree\install-gate.ps1 +> ``` +> +> `tests/test_gate_installed_parity.py` is **red until that runs** — deliberately. That test is the fix for +> G1, and the first thing it detected was itself. + --- ## 1. The estate @@ -96,9 +110,11 @@ reading the emitted decision — not by reading source alone. |---|---|---| | Rule 1 — write into primary | user | **LIVE** (probe-verified DENY) | | Rule 2 — dispatch from primary | user | **LIVE** (probe-verified DENY) | -| Rule 3 — git verbs vs primary | user | **LIVE but partial** — DENY on literal spellings, ALLOW on several others (§3) | +| Rule 3 — git verbs vs primary | user | **LIVE, partial** — the `-c` and relative-`cd` holes are fixed in source (G2/G3); enumerated-verb gaps remain (G9) | | Rule 3b — worktree hijack | user | **LIVE but narrow** — 2 of 11 verbs, existing-local-branch destinations only | -| Rule 4 — `EnterWorktree` | — | **INERT, twice over** — absent from the installed script *and* unmatched in all 5 config dirs | +| Rule 4 — `EnterWorktree` | — | **INERT BY DESIGN** — now opt-in behind `-EnterWorktreeGate`; was inert by accident (absent from the installed script *and* unmatched in all 5 config dirs) | +| Deny receipts (`worktree-gate.log`) | user | **NEW** — every deny logs rule/tool/cwd/detail; never the raw command | +| Installed-vs-source parity check | local test | **NEW** — `tests/test_gate_installed_parity.py`; skips on CI, red on a stale box | | Blanket-stage guard | project | **LIVE but leaky** — 7 of 8 trivial rephrasings bypass it | | Selfheal — primary auto-repair | user (4 of 5 dirs) | LIVE | | Selfheal — hijack warning | user (4 of 5 dirs) | **LIVE and currently mis-firing** (§3, G4) | @@ -156,7 +172,7 @@ pattern, reproduced inside the drift machinery itself. Ranked by expected harm × likelihood. G1–G4 survived an adversarial verification pass; the rest are probe- or source-verified. -### G1 — Nothing can observe what is actually installed *(root cause)* +### G1 — Nothing can observe what is actually installed *(root cause)* — **FIXED IN SOURCE** `install-gate.ps1` copies the script with no version, hash, or marker. `-Status` prints an **uncalibrated count** of hook entries — it reports "3" where 4 is now expected, and states no expectation. Worse, the @@ -167,7 +183,7 @@ installed copy keeps enforcing it forever while every test correctly reports it Measured today: installed 23,430 B (Jul 24) vs repo 25,423 B (Jul 29). This is why rule 4 is inert, and it will recur for the next rule. -### G2 — `cd` to the primary by a non-literal spelling defeats rule 3 +### G2 — `cd` to the primary by a non-literal spelling defeats rule 3 — **FIXED** Rule 3 resolves the target from cwd or `-C` only; `cd && git …` is caught solely by an in-text scan for the allowlist root's canonical spelling. Rule 3b *does* resolve `cd`, then returns with the @@ -181,7 +197,7 @@ Probe-verified ALLOW from a nested worktree, where `../../..` **is** the primary to name the repo root, so this is reachable **by accident**. [WORKTREE-GATE.md](WORKTREE-GATE.md) asserts coverage of exactly this shape. -### G3 — `git -c ` redirects rule 3's target off the primary *(one-character fix)* +### G3 — `git -c ` redirects rule 3's target off the primary — **FIXED** Rule 3 parses `-C ` with `-match`, which is case-**in**sensitive in PowerShell, so git's lowercase global `-c name=value` is captured as if it were a path. `git -c core.pager=cat checkout main` at @@ -234,7 +250,7 @@ Upstream draws this boundary explicitly — the worktrees documentation states t repository's `.git` and that sandboxing allows those writes, which is exactly why `hooks/` and config need their own rule. -### G7 — Rule 1's deny message advertises the primary as a worktree to reuse +### G7 — Rule 1's deny message advertises the primary as a worktree to reuse — **FIXED** The "worktrees that already exist — REUSE one if it is yours" filter compares a string to the `PSCustomObject` returned by `Test-Governed`, so the comparison is always true and the primary is never @@ -265,7 +281,7 @@ requires whitespace before the verb; a hyphen precedes `checkout`). `gh pr check token and exits early. Rules 1 and 2 key on tool *names*, so any tool not in those lists is unmatched at **both** the settings matcher and the rule — the hook never runs, and nothing says so. -### G10 — False positives train sessions to route around the only control on the shell path +### G10 — False positives train sessions to route around the only control on the shell path — **FIXED** The verb scan's exclusion class does not exclude newline, so `git status\necho about to merge stuff` denies with verb=`merge` from prose on line 2. The git-detection class includes quote characters, so @@ -287,7 +303,7 @@ and `prune-merged.ps1` are sibling-only, so the nested population — where ever — has creation but no scripted teardown, and `prune-merged.ps1` run from a worktree prints a green "No sibling worktrees to consider" and exits 0. A wrong-cwd run reports a clean bill of health. -### G12 — The gate has never produced a receipt +### G12 — The gate has never produced a receipt — **FIXED** `Write-Deny` writes JSON to stdout and exits 0. There is no log, no counter, no audit file. Nothing can answer "how many drift events were prevented last month", "is G10's false-positive rate 1/day or 1/1000", @@ -415,9 +431,16 @@ analysis must be redone. | **B10** | **Collapse the two allowlists; harden the second installer.** One allowlist path referenced by both scripts; `-Uninstall` removes it; give `install-selfheal.ps1` the `CLAUDECODE` throw, the multi-config-dir discovery loop, a `-Status` and an `-Uninstall`. Extend B1's check to assert the set of dirs carrying a gate matcher equals the set carrying the selfheal hook. | G8 | S | none | | **B11** | **Close the verb and teardown holes.** A second alternation for hyphenated/two-token forms (`sparse-checkout`, `worktree remove|move`, `branch -f`, `update-ref`, `read-tree`, `rm`, `mv`, `checkout-index`, `bisect`), with its own message for `worktree remove` (cross-session destruction, not a tree swap); teach the detector about `gh`. Give `prune-merged.ps1` a **loud failure** when its root is not the primary instead of a green no-op, and add nested-worktree teardown. Correct the false rationale comment. | G9, G11 | M | low | -**Order:** B1 and B2 first, together. Without them, none of the rest can be confirmed to have reached -production, and no severity claim in §3 is falsifiable. Then B9 (one character), B3+B4 as a pair, then -B5–B8, then B11. +**Status.** B1, B2, B3, B4 and B9 are **built and merged to this branch**, with tests; each fix was proved +to catch its own regression by mutation (five mutations applied to the shipped script one at a time, all +five went red). B7 is **half done** — rule 4 is now opt-in rather than retired, which preserves the owner's +decision while removing the trap where re-installing would activate it. B5, B6, B8, B10 and B11 are **not +started**; B6 (worktree-first entry point) is the one with the largest durable effect and no code in it. + +Remaining order: B6, then B8 (cross-worktree blast radius), then B5, B10, B11. + +One caveat carried forward: **none of the merged work is live until the gate is re-installed.** That is the +same property that made rule 4 inert, now with a test watching it. ### Considered, not worth doing now From 6758b6b08cf22f6e8aae730895646f65b33dcee2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 09:58:40 -0500 Subject: [PATCH 06/12] fix(worktree-gate): judge what a command does, not how its characters are arranged An adversarial review of the previous commit found three regressions I had introduced, each a DENY on main that had become an ALLOW. All were the same mistake in different clothes: treating a syntactic property as if it were a semantic one. QUOTED IS NOT INERT. Blanking every quoted span stopped a commit message supplying a verb -- and also erased the verb from an interpreter argument, which is code that runs. `pwsh -NoProfile -Command "git reset --hard"` in the primary went from DENY to ALLOW, in this repo's own house idiom, and the same hole disarmed rule 3b: a session could hijack another session's worktree just by wrapping the checkout in `pwsh -Command`. Worse, the "no verb found" early exit meant the in-text path fallback was never reached, so even a command spelling the primary out in full walked through. Interpreter arguments are now recursed into rather than blanked. `ssh box "..."` deliberately is not an interpreter -- that one runs on another host, and denying it was a bug. A COMMAND IS A SEQUENCE, NOT A BAG OF TOKENS. Resolving `cd` from anywhere in the string made the rule order-blind: a cd AFTER the verb, or one already undone by popd, redirected the target away from the tree the git call actually touched. Only the text preceding the invocation counts now, and an ambiguous prefix (popd, `cd -`, a subshell, two cds) falls back to the session cwd -- the deny-side default. REDIRECTING FILES IS NOT REDIRECTING THE REPOSITORY. `--work-tree` / `GIT_WORK_TREE` say where files land; GIT_DIR still resolves from the cwd, so the shared repo's HEAD and index still move. Accepting them as "the tree the command acts on" made a one-token bypass of the whole rule. The resolver now returns a candidate SET and denies if ANY member is governed; only `-C` replaces the cwd, because only `-C` relocates both. A LINE BREAK IS NOT A STATEMENT BREAK. Folding continuations before the per-line split, so `git \` keeps its subcommand. Two things the review did not ask for but that fell out of the fix: every verb-bearing segment is now evaluated, not just the first (`git -C ../x checkout ; git checkout` judged the wrong one), and an explicit `-C` suppresses the in-text fallback, because `cd && git -C rebase` acts on the sibling and denying it on the strength of the path appearing in the cd is a false positive. Receipts: $Detail is composed from tool input, so an embedded newline could forge extra records in the log whose only purpose is counting. Sanitised and capped, one record per line, with a pid field; and Add-Content, which silently dropped records when concurrent sessions raced, is replaced with a bounded retry. Verified by driving the db53fd45 build and this one side by side over all 26 cases -- 15 regressions, 7 false-positive fixes that must stay fixed, 4 bypasses that must stay closed. 26/26. Every case is now a test; 141 gate tests pass. --- scripts/hooks/worktree_gate.ps1 | 217 +++++++++++---- scripts/worktree/install-gate.ps1 | 11 +- tests/test_worktree_gate_shell_semantics.py | 287 ++++++++++++++++++++ 3 files changed, 457 insertions(+), 58 deletions(-) create mode 100644 tests/test_worktree_gate_shell_semantics.py diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 0a888edc..bca2e4fd 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -58,9 +58,30 @@ function Write-Deny([string]$Reason, [string]$Rule = "?", [string]$Detail = "") try { $logDir = Split-Path -Parent $ReposFile if ($logDir -and (Test-Path -LiteralPath $logDir)) { + # ONE RECORD IS ONE LINE, always. $Detail is composed from tool input, so an embedded newline + # or tab would let a crafted path forge extra records in a log whose whole purpose is counting. + # Strip both and cap the length before composing. + $clean = { + param($s) + $t = ("$s" -replace '[\r\n\t]', ' ') + if ($t.Length -gt 400) { $t.Substring(0, 400) + '...' } else { $t } + } $stamp = (Get-Date).ToString("s") - $line = "$stamp`tv$GateVersion`trule=$Rule`ttool=$tool`tcwd=$cwdRaw`t$Detail" - Add-Content -LiteralPath (Join-Path $logDir "worktree-gate.log") -Value $line -Encoding utf8 + $line = "$stamp`tv$GateVersion`tpid=$PID`trule=$(& $clean $Rule)`ttool=$(& $clean $tool)" + + "`tcwd=$(& $clean $cwdRaw)`t$(& $clean $Detail)" + # Every session on the box shares this file, so concurrent denies race. Add-Content silently + # dropped records under contention -- and a lossy counter is worse than none, because it reads + # as a measurement. Retry a bounded number of times, then give up quietly: the deny matters, + # the receipt does not. + $path = Join-Path $logDir "worktree-gate.log" + for ($i = 0; $i -lt 5; $i++) { + try { + [System.IO.File]::AppendAllText($path, $line + [Environment]::NewLine) + break + } catch { + Start-Sleep -Milliseconds (10 * ($i + 1)) + } + } } } catch { } @@ -106,32 +127,87 @@ function Get-ComparablePath([string]$Path, [string]$Base) { # # Returns the RAW (original-case) path: rule 3b shells `git -C` with it, and on a case-sensitive # filesystem a lowercased path misses the real directory and the whole rule silently fails open. -function Get-GitActedOnPathRaw([string]$Cmd, [string]$FallbackCwdRaw) { - # git's global `-C ` wins, read CASE-SENSITIVELY. `-match` is case-INsensitive in PowerShell, so - # git's lowercase `-c name=value` config override was captured as if it were a path: the "target" - # resolved to a nonexistent relative directory, which is not governed, and `git -c core.pager=cat - # checkout main` in the primary was ALLOWED (measured). Rule 3b already used -cmatch here for this - # exact reason; the rule protecting the SHARED tree never got the fix. - if ($Cmd -cmatch '(?:^|\s)-C\s+"?([^"\s]+)"?') { return $Matches[1] } - if ($Cmd -cmatch '(?:^|\s)--work-tree[=\s]+"?([^"\s]+)"?') { return $Matches[1] } - if ($Cmd -cmatch '(?:^|\s)GIT_WORK_TREE="?([^"\s]+)"?') { return $Matches[1] } - # `cd`/`pushd` are shell builtins, so match them case-insensitively (PowerShell accepts `CD`). - if ($Cmd -match '(?:^|\s)(?:cd|pushd)\s+"?([^"&|;]+?)"?\s*(?:&&|;|\||$)') { - return $Matches[1].Trim() +# Every path a git command could be acting on, in priority order, as RAW (original-case) strings. +# +# It returns a SET, not a winner, and the caller denies if ANY member is governed. That is the whole +# correction: `--work-tree` / `GIT_WORK_TREE` say where FILES land, they do NOT say which repository is +# mutated -- GIT_DIR still resolves from the cwd, so `git --work-tree=/tmp/x reset --hard` run in the +# primary still moves the SHARED repo's HEAD and index. Treating them as a replacement target turned a +# one-token flag into a bypass of the whole rule (measured DENY -> ALLOW). Only `-C` genuinely relocates +# both, so only `-C` replaces the cwd. +# +# $Prefix is the text BEFORE the git invocation on the same line, and a `cd` is honoured only from there. +# Reading `cd` from the whole command made the resolver order-blind: `git checkout main && cd ../elsewhere` +# resolved to `../elsewhere` and allowed a swap of the tree the git call had already acted on. A prefix +# that is ambiguous -- `popd`, `cd -`, a subshell, or more than one `cd` -- falls back to the session cwd, +# which is the DENY-side default. +function Get-GitTargetCandidatesRaw([string]$Line, [string]$Prefix, [string]$CwdRaw) { + $out = @() + + # git's global `-C `, read CASE-SENSITIVELY. `-match` is case-INsensitive in PowerShell, so + # git's lowercase `-c name=value` config override was captured as if it were a path -- and being the + # first match it also shadowed a real `-C` later in the same command. + if ($Line -cmatch '(?:^|\s)-C\s+"?([^"\s]+)"?') { + $out += $Matches[1] + } else { + $cd = $null + if ($Prefix -notmatch '(?:^|\s)(?:popd|cd\s+-(?:\s|$))' -and $Prefix -notmatch '[({]') { + $cds = [regex]::Matches($Prefix, '(?:^|\s)(?:cd|pushd)\s+"?([^"&|;]+?)"?\s*(?:&&|;|\||$)') + if ($cds.Count -eq 1) { $cd = $cds[0].Groups[1].Value.Trim() } + } + $out += $(if ($cd) { $cd } else { $CwdRaw }) + } + + # ADDITIONAL, never instead of: see the note above. + if ($Line -cmatch '(?:^|\s)--work-tree[=\s]+"?([^"\s]+)"?') { $out += $Matches[1]; $out += $CwdRaw } + if ($Line -cmatch '(?:^|\s)GIT_WORK_TREE="?([^"\s]+)"?') { $out += $Matches[1]; $out += $CwdRaw } + # --git-dir names the repo; the tree is its parent. Add both rather than reason about which. + if ($Line -cmatch '(?:^|\s)--git-dir[=\s]+"?([^"\s]+)"?') { + $out += $Matches[1] + $out += (Join-Path $Matches[1] "..") } - return $FallbackCwdRaw + $out | Where-Object { $_ } } -# Decide from the COMMAND, never from prose inside it. Three false positives were measured against the raw -# string: a two-line command whose second line read `echo about to merge stuff` denied with verb=merge (the -# scan class excludes `|;&` but not newline); `echo "git checkout main"` denied; and -# `git commit -m "chore: clean up dead code"` denied on `clean`. Each one teaches a session to route around -# the gate -- and per the design notes the deny text is the ONLY control on the shell-write path, so -# eroding compliance with it is not a nuisance, it is the guard itself. The sibling hook -# block-blanket-git-stage.ps1 already splits on newlines; these two must not disagree about what a command -# is. -function Get-ScannableCommandLines([string]$Cmd) { - foreach ($line in ($Cmd -split '\r?\n')) { +# Decide from the COMMAND, never from prose inside it -- but "prose" and "code" are not the same as +# "quoted" and "unquoted", and conflating them was a measured regression. +# +# Three false positives came from scanning the raw string: a two-line command whose second line read +# `echo about to merge stuff` denied with verb=merge; `echo "git checkout main"` denied; and +# `git commit -m "chore: clean up dead code"` denied on `clean`. Blanking every quoted span fixed those -- +# and broke something worse. The argument of an interpreter flag (`pwsh -Command "..."`, `bash -c "..."`, +# `cmd /c "..."`) is quoted, but it is CODE THAT RUNS: blanking it made `pwsh -Command "git reset --hard"` +# in the primary ALLOW where it had always denied, across rules 3 AND 3b. That is precisely the +# route-around the rule-3 deny text warns against, spelled in this repo's own house idiom. +# +# So an interpreter argument is not blanked, it is RECURSED INTO: its contents come back as an extra +# scan line, judged on its own terms. Everything else quoted stays inert. +# +# Each entry carries BOTH forms. Scan is for deciding whether a git verb is present; Raw is for parsing +# PATHS out of the same line, since the blanking that stops a commit message supplying a verb would also +# erase the path. +function Get-ScannableSegments([string]$Cmd) { + # Fold line continuations FIRST, or the per-line split below separates `git \` from its verb and the + # rule stops seeing the command at all. Prose does not end a line with a continuation character, so + # this does not resurrect the `echo about to merge stuff` false positive. + $folded = $Cmd -replace '\\\r?\n[ \t]*', ' ' + $folded = $folded -replace '`\r?\n[ \t]*', ' ' + + $lines = @($folded -split '\r?\n') + + # One level of interpreter recursion. `-c`/`-lc`/`-Command`/`/c`/`/k` and their quoted argument. + $inner = @() + foreach ($ln in $lines) { + foreach ($pat in @( + '(?:^|\s)(?:-c|-lc|-ec|-Command|-EncodedCommand)\s+"([^"]*)"', + "(?:^|\s)(?:-c|-lc|-ec|-Command|-EncodedCommand)\s+'([^']*)'", + '(?:^|\s)/[ckCK]\s+"([^"]*)"' + )) { + foreach ($m in [regex]::Matches($ln, $pat)) { $inner += $m.Groups[1].Value } + } + } + + foreach ($line in @($lines + $inner)) { # A quoted PROGRAM path must keep its git token -- `"C:\Program Files\Git\bin\git.exe" checkout # main` is a real spelling and blanking it wholesale would be a false NEGATIVE. Collapse that form # to a bare token first, then blank every remaining quoted span. @@ -139,7 +215,7 @@ function Get-ScannableCommandLines([string]$Cmd) { $s = $s -replace "'[^']*[\\/](git(?:\.exe)?)'", '$1' $s = $s -replace '"[^"]*"', '""' $s = $s -replace "'[^']*'", "''" - $s + [pscustomobject]@{ Raw = $line; Scan = $s } } } @@ -177,9 +253,13 @@ $cwdRaw = [string]$hook.cwd # original case: for `git -C` # only an exact tool match reaches Write-Deny. # # Expressed as `$tool -in @("EnterWorktree")` so tests/test_install_gate_wiring.py SEES this tool as -# handled and ENFORCES that install-gate.ps1 registers a matcher for it -- rule 3 shipped dead once by -# implementing a rule with no matcher, and that tripwire exists to prevent exactly this. The matcher is -# wired in install-gate.ps1 alongside this change; delete it there and the wiring test goes red. +# handled -- rule 3 shipped dead once by implementing a rule with no matcher, and that tripwire exists to +# prevent exactly this. +# +# NB the matcher is OPT-IN (`install-gate.ps1 -EnterWorktreeGate`), so this rule does not fire on a bare +# install. That is deliberate and pinned by test_the_default_install_wires_rules_1_2_and_3_and_nothing_else +# plus OPT_IN_TOOLS in tests/test_gate_installed_parity.py -- turning it on is a decision, not a side +# effect of re-installing. Rationale: docs/SESSION-DRIFT-CONTROLS.md section 4. # --------------------------------------------------------------------------------------------------- if ($tool -in @("EnterWorktree")) { Write-Deny -Rule "4" -Detail "relocate-session" -Reason @" @@ -222,16 +302,16 @@ function Test-Governed([string]$Candidate) { # The gate cannot tell a worktree's rightful session from a squatter (both share the cwd), so it blocks # the move for both; the rightful owner's escape hatch is a PLAIN terminal (never gated) or a fresh # worktree for the other branch. Returns normally to ALLOW; calls Write-Deny (which exits) to block. -function Test-WorktreeHijack([string]$Verb, [string]$Cmd, [string]$CwdRaw) { +function Test-WorktreeHijack([string]$Verb, [string]$Cmd, [string]$WtRaw) { if ($Verb -notin @("checkout", "switch")) { return } - # Which working tree does the command act on? Get-GitActedOnPathRaw is now shared with rule 3 -- see - # its comment for why having two copies of this let a real tree swap fall between the rules. It keeps - # the RAW (original-case) path, which every `git -C` below MUST use: a Get-ComparablePath value is - # lowercased, and on a case-sensitive filesystem (Linux CI) `git -C /tmp/.../primary-wt` misses the - # real `.../Primary-wt` dir and the whole rule silently fails open. - $wtRaw = Get-GitActedOnPathRaw $Cmd $CwdRaw - if (-not $wtRaw) { return } + # $WtRaw is resolved ONCE by rule 3 (Get-GitTargetCandidatesRaw) and handed down, so the two rules + # cannot disagree about which tree a command acts on -- they used to have separate parsers, and a real + # tree swap fell into the gap between them. It is the RAW (original-case) path, which every `git -C` + # below MUST use: a Get-ComparablePath value is lowercased, and on a case-sensitive filesystem + # (Linux CI) `git -C /tmp/.../primary-wt` misses the real `.../Primary-wt` and the rule fails open. + if (-not $WtRaw) { return } + $wtRaw = $WtRaw # Everything AFTER the first verb, up to the next command separator (so `git checkout x && ...` # does not drag the next command's tokens in). Parsing args here -- not the whole command -- keeps @@ -346,29 +426,51 @@ if ($tool -in @("Bash", "PowerShell")) { # `[^|;&]*?` keeps the scan inside one command, so `git log | grep reset` is not a false positive. $verbs = 'cherry-pick|checkout|switch|reset|restore|stash|clean|rebase|merge|revert|am|apply' - # Scan LINE BY LINE, with quoted spans blanked (Get-ScannableCommandLines). A verb must come from a - # git invocation on the same line and outside quotes, or prose supplies it. - $verb = $null - foreach ($scan in (Get-ScannableCommandLines $cmd)) { + # Scan SEGMENT BY SEGMENT (Get-ScannableSegments): per line, with quoted spans blanked, plus the + # contents of any interpreter argument recursed into. A verb must come from a git invocation on the + # same segment and outside inert quotes, or prose supplies it. + # Evaluate EVERY verb-bearing segment, not just the first. `git -C ../x checkout main ; git checkout + # main` has two invocations and only the second touches this tree; stopping at the first match judged + # the wrong one. Deny on the first segment whose target set contains a governed tree. + $verb = $null ; $verbLine = $null ; $targetRaw = $cwdRaw ; $root = $null + # True once some segment's target had to be INFERRED (from cwd or a cd) rather than stated with `-C`. + # An explicit `-C` is authoritative about which repository git acts on, so the in-text fallback below + # must not second-guess it -- `cd && git -C rebase` acts on the sibling, and + # denying it because the primary's path appears in the `cd` is a false positive. + $anyInferredTarget = $false + $gitToken = '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)' + foreach ($seg in (Get-ScannableSegments $cmd)) { # Match a git invocation however it is spelled: git, git.exe, or an absolute path to either. - if ($scan -cnotmatch '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') { continue } - if ($scan -cnotmatch "\bgit(\.exe)?\b[^|;&]*?\s(?$verbs)(?=\s|$)") { continue } - $verb = $Matches['verb'] - break + if ($seg.Scan -cnotmatch $gitToken) { continue } + if ($seg.Scan -cnotmatch "\bgit(\.exe)?\b[^|;&]*?\s(?$verbs)(?=\s|$)") { continue } + $segVerb = $Matches['verb'] + + # Everything BEFORE the git invocation on this line. A `cd` is honoured only from here -- reading + # it from the whole command made the resolver order-blind (see Get-GitTargetCandidatesRaw). + $at = [regex]::Match($seg.Raw, $gitToken) + $segPrefix = $(if ($at.Success) { $seg.Raw.Substring(0, $at.Index) } else { "" }) + + if ($seg.Raw -cnotmatch '(?:^|\s)-C\s+"?([^"\s]+)"?') { $anyInferredTarget = $true } + + # Path parsing runs on the RAW line, never on the blanked scan string: the blanking that stops a + # commit message supplying a verb would also erase the path. Deny if ANY candidate is governed -- + # a `--work-tree` elsewhere does not stop the cwd's repo being mutated. + $cands = @(Get-GitTargetCandidatesRaw $seg.Raw $segPrefix $cwdRaw) + if (-not $verb) { + $verb = $segVerb ; $verbLine = $seg.Raw + if ($cands.Count -gt 0) { $targetRaw = $cands[0] } + } + foreach ($c in $cands) { + $hit = Test-Governed (Get-ComparablePath $c $cwdRaw) + if ($hit) { $root = $hit ; $verb = $segVerb ; $verbLine = $seg.Raw ; $targetRaw = $c ; break } + } + if ($root) { break } } if (-not $verb) { exit 0 } - # Which repo does it act on? `-C` > `--work-tree` > `cd`/`pushd` > the session's cwd -- otherwise a - # session sitting in a worktree could reach INTO the primary with `git -C checkout x` and - # sail straight past a cwd-only check. Path parsing runs on the RAW command, never on the blanked - # scan string: the blanking that stops a commit message supplying a verb would also erase the path. - $targetRaw = Get-GitActedOnPathRaw $cmd $cwdRaw - $target = Get-ComparablePath $targetRaw $cwdRaw - - $root = Test-Governed $target # `cd ; git checkout ...` and `pushd` defeat both of the above, so also treat any command - # that NAMES a governed primary as targeting it. - if (-not $root) { + # that NAMES a governed primary as targeting it -- but only where the target was inferred. + if (-not $root -and $anyInferredTarget) { $normalized = ($cmd -replace '\\', '/').ToLowerInvariant() foreach ($r in $roots) { # Match the primary path only at a DIRECTORY BOUNDARY, never as a raw prefix substring. A @@ -410,8 +512,9 @@ if ($tool -in @("Bash", "PowerShell")) { if (-not $root) { # Not the shared primary. It may still be a governed LINKED WORKTREE being hijacked onto an # existing branch (rule 3b) -- Write-Deny + exit if so; otherwise this returns and we allow. - # Pass the RAW cwd: rule 3b shells `git -C` and must not use a lowercased path (Linux CI). - Test-WorktreeHijack $verb $cmd $cwdRaw + # Hand down the LINE the verb was found on and the tree already resolved from it, so 3b judges + # the same command rule 3 did (including one recursed out of an interpreter argument). + Test-WorktreeHijack $verb $verbLine $targetRaw exit 0 } diff --git a/scripts/worktree/install-gate.ps1 b/scripts/worktree/install-gate.ps1 index 39f94dd1..73785d29 100644 --- a/scripts/worktree/install-gate.ps1 +++ b/scripts/worktree/install-gate.ps1 @@ -189,10 +189,19 @@ if ($Status) { foreach ($t in "$($e.matcher)".Split("|")) { if ($t) { $null = $wired.Add($t) } } } } - $missing = @($handled | Where-Object { -not $wired.Contains($_) } | Sort-Object) + # Rules that are deliberately unwired are reported as such, never as UNWIRED. A status line that + # cries wolf about a known-and-intended state is one a reader learns to skip, which is how a real + # UNWIRED would go unnoticed -- the exact failure this whole block exists to surface. + $optIn = @("EnterWorktree") + $absent = @($handled | Where-Object { -not $wired.Contains($_) }) + $missing = @($absent | Where-Object { $optIn -notcontains $_ } | Sort-Object) + $offByChoice = @($absent | Where-Object { $optIn -contains $_ } | Sort-Object) $stray = @($wired | Where-Object { $handled -notcontains $_ } | Sort-Object) Write-Host "wiring : $sp" Write-Host " matched : $(@($wired | Sort-Object) -join ', ')" + if ($offByChoice) { + Write-Host " opt-in : $($offByChoice -join ', ') <- off by default, add -EnterWorktreeGate to enable" + } if ($missing) { Write-Host " UNWIRED : $($missing -join ', ') <- implemented but NEVER FIRES" -ForegroundColor Yellow } diff --git a/tests/test_worktree_gate_shell_semantics.py b/tests/test_worktree_gate_shell_semantics.py new file mode 100644 index 00000000..577927f6 --- /dev/null +++ b/tests/test_worktree_gate_shell_semantics.py @@ -0,0 +1,287 @@ +"""Rule 3 must judge what a command DOES, not how its characters are arranged. + +Every case here is a regression that an adversarial review found in the first version of the +quote-blanking and cd-resolving work, each verified as a DENY on main (db53fd45) that had become an ALLOW. +They are grouped by the wrong assumption that caused them: + +* **Quoted does not mean inert.** The argument of ``pwsh -Command`` / ``bash -c`` / ``cmd /c`` is quoted, + and it is *code that runs*. Blanking every quoted span to stop a commit message supplying a verb also + erased the verb from an interpreter argument, so ``pwsh -NoProfile -Command "git reset --hard"`` in the + primary was allowed -- in this repo's own house idiom, and exactly the route-around the deny text warns + against. Interpreter arguments are recursed into instead of blanked. + +* **A command is a sequence, not a bag of tokens.** Resolving ``cd`` from anywhere in the string made the + rule order-blind: a ``cd`` *after* the git verb, or one already undone by ``popd``, redirected the target + away from the tree the git call actually touched. Only the text *preceding* the invocation counts, and an + ambiguous prefix falls back to the session cwd -- the deny-side default. + +* **Redirecting files is not redirecting the repository.** ``--work-tree`` / ``GIT_WORK_TREE`` say where + files land; ``GIT_DIR`` still resolves from the cwd, so the shared repo's HEAD and index still move. Read + as a replacement target they made a one-token bypass of the whole rule. They are additional candidates + now, never substitutes -- only ``-C`` relocates both. + +* **A line break is not a statement break.** Splitting per line to stop prose supplying a verb also split + a trailing-backslash continuation from its subcommand. Continuations are folded first. + +The closing group pins the fixes these regressions came from, so a revert cannot trade one for the other. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from tests.test_worktree_gate import assert_denied, run_gate # reuse the subprocess harness + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def shell(command: str, cwd: Path | str, tool: str = "Bash") -> dict[str, Any]: + return { + "session_id": "s-1", + "cwd": str(cwd), + "hook_event_name": "PreToolUse", + "tool_name": tool, + "tool_input": {"command": command}, + } + + +@pytest.fixture +def primary(tmp_path: Path) -> Path: + return tmp_path / "Repo" + + +@pytest.fixture +def nested(primary: Path) -> Path: + return primary / ".claude" / "worktrees" / "wt-1" + + +@pytest.fixture +def repos_file(tmp_path: Path, primary: Path) -> Path: + f = tmp_path / "repos.txt" + f.write_text(f"{primary}\n", encoding="utf-8") + return f + + +# --------------------------------------------------------- quoted does not mean inert + + +@pytest.mark.parametrize( + "command", + [ + 'pwsh -NoProfile -Command "git reset --hard"', + "pwsh -Command 'git checkout main'", + 'pwsh -c "git clean -xfd"', + 'bash -c "git rebase origin/main"', + 'bash -lc "git checkout main"', + 'sh -c "git stash"', + 'cmd /c "git checkout main"', + ], +) +def test_a_git_verb_inside_an_interpreter_argument_is_still_a_git_verb( + primary: Path, repos_file: Path, command: str +) -> None: + """The quoted span an interpreter is handed EXECUTES. Blanking it made every one of these allow.""" + reason = assert_denied(run_gate(shell(command, cwd=primary), repos_file)) + assert "SHARED PRIMARY" in reason + + +@pytest.mark.parametrize( + "template", + [ + 'sh -c "cd {p} && git reset --hard"', + 'bash -lc "cd {p} && git checkout main"', + "pwsh -c 'git -C {p} checkout main'", + 'pwsh -Command "cd {p}; git clean -xfd"', + ], +) +def test_an_interpreter_argument_naming_the_primary_is_denied_from_a_worktree( + tmp_path: Path, primary: Path, repos_file: Path, template: str +) -> None: + """Worse than the above: the primary is spelled out in full inside the quotes, and the early exit on + 'no verb found' meant the in-text path fallback was never even reached.""" + assert_denied( + run_gate(shell(template.format(p=primary), cwd=tmp_path / "Repo-alerts"), repos_file) + ) + + +def test_a_remote_command_over_ssh_is_not_the_local_primary( + primary: Path, repos_file: Path +) -> None: + """The counter-case that keeps the recursion honest: `ssh box "git checkout main"` runs on another + HOST. It denied on main and should not -- recursing into interpreter arguments must not sweep this + back in, so `ssh` is deliberately not treated as an interpreter.""" + assert run_gate(shell('ssh box "git checkout main"', cwd=primary), repos_file) is None + + +# --------------------------------------------------------- a command is a sequence + + +@pytest.mark.parametrize( + "command", + [ + "git checkout main && cd ../Elsewhere", + "git reset --hard ; cd ../Elsewhere", + "pushd ../Elsewhere ; popd ; git reset --hard", + "cd ../Elsewhere && cd . && git clean -xfd", + "cd - ; git checkout main", + ], +) +def test_a_cd_that_does_not_precede_the_git_call_cannot_exempt_it( + primary: Path, repos_file: Path, command: str +) -> None: + """A cd AFTER the verb, one undone by popd, or an ambiguous pair: none of them changes the tree the + git call acted on. Reading cd from the whole string let each of these walk out of the primary.""" + assert_denied(run_gate(shell(command, cwd=primary), repos_file)) + + +def test_a_cd_that_does_precede_the_git_call_is_still_honoured( + primary: Path, repos_file: Path +) -> None: + """The fix must not simply ignore cd -- that would re-open the relative-path bypass it was added for, + and re-introduce the false positive of denying work that left the primary first.""" + assert run_gate(shell("cd ../Elsewhere && git checkout main", cwd=primary), repos_file) is None + + +def test_a_dash_C_beats_a_preceding_cd(tmp_path: Path, primary: Path, repos_file: Path) -> None: + """git acts on -C, never on the cd'd directory, when both appear.""" + sibling = tmp_path / "Repo-alerts" + assert ( + run_gate(shell(f'cd {primary} && git -C "{sibling}" rebase main', cwd=primary), repos_file) + is None + ) + assert_denied( + run_gate( + shell(f'cd {sibling} && git -C "{primary}" checkout main', cwd=sibling), repos_file + ) + ) + + +# --------------------------------------------------------- files vs repository + + +@pytest.mark.parametrize( + "command", + [ + "git --work-tree=C:/Windows/Temp reset --hard", + "git --work-tree C:/Windows/Temp checkout main", + "GIT_WORK_TREE=C:/Windows/Temp git checkout main", + ], +) +def test_redirecting_the_work_tree_does_not_exempt_the_repository( + primary: Path, repos_file: Path, command: str +) -> None: + """GIT_DIR still resolves from the cwd, so the SHARED repo's HEAD and index still move. Treating these + as 'the tree the command acts on' turned one token into a bypass of the entire rule.""" + assert_denied(run_gate(shell(command, cwd=primary), repos_file)) + + +def test_a_work_tree_pointing_INTO_the_primary_is_denied_from_outside( + tmp_path: Path, primary: Path, repos_file: Path +) -> None: + """The other direction: the candidate set means a work-tree aimed at the primary denies even when the + cwd is innocent.""" + assert_denied( + run_gate( + shell(f'git --work-tree="{primary}" checkout main', cwd=tmp_path / "Elsewhere"), + repos_file, + ) + ) + + +# --------------------------------------------------------- a line break is not a statement break + + +@pytest.mark.parametrize("command", ["git \\\n checkout main", "git `\n reset --hard"]) +def test_a_line_continuation_does_not_separate_git_from_its_verb( + primary: Path, repos_file: Path, command: str +) -> None: + """Splitting per line to stop prose supplying a verb also split `git \\` from its subcommand. Folding + continuations first is safe because prose does not end a line with a continuation character.""" + assert_denied(run_gate(shell(command, cwd=primary), repos_file)) + + +def test_folding_continuations_does_not_resurrect_the_prose_false_positive( + primary: Path, repos_file: Path +) -> None: + assert run_gate(shell("git status\necho about to merge stuff", cwd=primary), repos_file) is None + + +# --------------------------------------------------------- rule 3b sees the same commands + + +@pytest.fixture +def repo(tmp_path: Path) -> SimpleNamespace: + """A real governed primary + a linked worktree + a free branch to hijack onto.""" + if shutil.which("git") is None: + pytest.skip("needs git on PATH") + + def git(*args: str, cwd: Path | None = None) -> None: + subprocess.run( + ["git", *args], + cwd=str(cwd) if cwd else None, + check=True, + capture_output=True, + text=True, + ) + + primary = tmp_path / "Primary" + git("init", "-b", "main", str(primary)) + git("config", "user.email", "t@example.com", cwd=primary) + git("config", "user.name", "t", cwd=primary) + (primary / "seed.txt").write_text("seed\n", encoding="utf-8") + git("add", "-A", cwd=primary) + git("commit", "-m", "seed", cwd=primary) + git("branch", "claude/other-branch", cwd=primary) + wt = tmp_path / "Primary-wt" + git("worktree", "add", "-b", "wt-branch", str(wt), cwd=primary) + repos = tmp_path / "repos.txt" + repos.write_text(f"{primary}\n", encoding="utf-8") + return SimpleNamespace(primary=primary, wt=wt, repos=repos, other="claude/other-branch") + + +@pytest.mark.skipif(shutil.which("git") is None, reason="needs git on PATH") +@pytest.mark.parametrize( + "template", + [ + 'pwsh -Command "git checkout {b}"', + "pwsh -c 'git checkout {b}'", + 'bash -c "git switch {b}"', + 'cmd /c "git checkout {b}"', + ], +) +def test_the_worktree_hijack_is_caught_through_an_interpreter_argument( + repo: SimpleNamespace, template: str +) -> None: + """Rule 3b rides the same scan as rule 3, so the blanking regression disarmed it too: a session could + hijack another session's worktree simply by wrapping the checkout in `pwsh -Command`.""" + reason = assert_denied(run_gate(shell(template.format(b=repo.other), cwd=repo.wt), repo.repos)) + assert "LINKED WORKTREE" in reason + + +# --------------------------------------------------------- the receipt log stays one-record-per-line + + +def test_a_crafted_path_cannot_forge_extra_log_records(primary: Path, repos_file: Path) -> None: + """$Detail is composed from tool input. An embedded newline would let a crafted path write additional + records into the log whose entire purpose is counting denials -- so the count could be inflated by the + thing being counted.""" + target = str(primary / "a\nb\tc.py") + payload = { + "session_id": "s-1", + "cwd": str(primary), + "tool_name": "Edit", + "tool_input": {"file_path": target}, + } + assert_denied(run_gate(payload, repos_file)) + log = repos_file.parent / "worktree-gate.log" + lines = [ln for ln in log.read_text(encoding="utf-8").splitlines() if ln.strip()] + assert len(lines) == 1, f"one deny must write exactly one record, got {len(lines)}: {lines}" + assert lines[0].startswith("2"), "the record must still begin with its timestamp" From 58a3d3229d16b4cd9cbca5e8dd92968dfcbf26d9 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 10:04:00 -0500 Subject: [PATCH 07/12] =?UTF-8?q?fix(worktree-gate):=20close=20the=20revie?= =?UTF-8?q?w's=20remaining=20findings=20=E2=80=94=20a=20blind=20parity=20t?= =?UTF-8?q?est,=20unattributed=20rules,=20overstated=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parity suite claimed "we print what we scanned so a skip can never be mistaken for a pass", and every print sat AFTER the skip. With no -rs in the pytest config the reasons were not shown either, so on CI the file rendered as a bare `sss.` -- the exact ambiguity it was written to remove, in the file whose whole subject is guards that cannot see. Prints now precede every skip, the skips say what was NOT compared, and the docstring states plainly which three properties CI does not guard. Rules 3b and 4 wrote receipts nothing asserted, and neither the version stamp nor the pid field was tested. Covered now, 3b against a real worktree because it asks git whether the target is one. The doc claimed rule 1 governs the primary's own .git/hooks "and a test asserts it". No test did -- the nearest one exercises rule 3. Added, for .git/hooks and .claude/hooks both: the gate's enforcement surface must not be editable through the gate. install-gate.ps1 -Status would have printed a permanent yellow "UNWIRED: EnterWorktree <- implemented but NEVER FIRES" after the required re-install, for a rule that is off by design. A guard that cries wolf every run is one a reader learns to skip, which is how the next genuinely unwired rule goes unnoticed in the same output -- the failure this audit exists to prevent. Opt-in rules now report as opt-in. Its helper comment also said it read the SOURCE while it read the INSTALLED copy; reading the installed copy is correct, so the comment was fixed to match and to say why. Doc corrections, all found by the review: the status table now states per item what is actually done versus outstanding (B1 emits no liveness receipt; B3 leaves GIT_DIR= unhandled; B4 still does not share a split helper with block-blanket-git-stage.ps1, so the two hooks can still disagree about what a command is) rather than listing five items as complete. Line and test counts corrected. An overstated status is worse than none -- the next session acts on it. --- docs/SESSION-DRIFT-CONTROLS.md | 29 +++++++--- scripts/worktree/install-gate.ps1 | 6 +- tests/test_gate_installed_parity.py | 45 ++++++++++----- tests/test_worktree_gate_command_parsing.py | 8 ++- tests/test_worktree_gate_receipts.py | 64 +++++++++++++++++++++ 5 files changed, 124 insertions(+), 28 deletions(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 401997b5..077b00c0 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -41,7 +41,8 @@ Four layers. Only the middle two enforce anything. ### Prevention — `PreToolUse` hooks -**[`scripts/hooks/worktree_gate.ps1`](../scripts/hooks/worktree_gate.ps1)** (417 lines) is installed at +**[`scripts/hooks/worktree_gate.ps1`](../scripts/hooks/worktree_gate.ps1)** (417 lines when this audit +ran; 609 after the fixes below) is installed at **user scope** by [`install-gate.ps1`](../scripts/worktree/install-gate.ps1) into `~/.claude/settings.json` *and* every `~/.claude-account-*/settings.json`. User scope is deliberate: a project-scoped hook is git-tracked, so it lives on one branch and a worktree cut from an older base would carry no gate at all. @@ -121,7 +122,7 @@ reading the emitted decision — not by reading source alone. | `session-context.ps1` banner | project | LIVE where the branch carries the file | | Claim / alloc / ledger gates | git hooks | LIVE | | `new.ps1` / `remove.ps1` / `prune-merged.ps1` | manual | LIVE, **sibling-layout only** | -| `tests/test_worktree_gate*.py`, `test_install_gate_wiring.py` | CI + local | **85 green, and blind** — every one binds the repo copy; nothing reads the installed copy or any live `settings.json` | +| `tests/test_worktree_gate*.py`, `test_install_gate_wiring.py` | CI + local | Was **85 green, and blind** — every one bound the repo copy; nothing read the installed copy or any live `settings.json`. Now 91 across six files, plus the local-only parity check below | Rule 4 being inert is **deliberate and announced** — the commit that landed it says "ships INERT … nothing changes until `install-gate.ps1` is re-run." It is listed as INERT here because a control that @@ -136,7 +137,7 @@ closed and probe-verified. So is the literal-spelling tree swap: `git checkout m `git -C checkout main`, `git -C ../../.. checkout zzz`, and `cd && git checkout main` all DENY. Fan-out from the primary is closed for the three named dispatch tools. Duplicate-branch checkout across worktrees is closed — though git enforces that one for free, so rule 3b fills a narrower gap than -its 90 lines suggest. +its 73 lines suggest. **Only nominally closed.** @@ -431,13 +432,23 @@ analysis must be redone. | **B10** | **Collapse the two allowlists; harden the second installer.** One allowlist path referenced by both scripts; `-Uninstall` removes it; give `install-selfheal.ps1` the `CLAUDECODE` throw, the multi-config-dir discovery loop, a `-Status` and an `-Uninstall`. Extend B1's check to assert the set of dirs carrying a gate matcher equals the set carrying the selfheal hook. | G8 | S | none | | **B11** | **Close the verb and teardown holes.** A second alternation for hyphenated/two-token forms (`sparse-checkout`, `worktree remove|move`, `branch -f`, `update-ref`, `read-tree`, `rm`, `mv`, `checkout-index`, `bisect`), with its own message for `worktree remove` (cross-session destruction, not a tree swap); teach the detector about `gh`. Give `prune-merged.ps1` a **loud failure** when its root is not the primary instead of a green no-op, and add nested-worktree teardown. Correct the false rationale comment. | G9, G11 | M | low | -**Status.** B1, B2, B3, B4 and B9 are **built and merged to this branch**, with tests; each fix was proved -to catch its own regression by mutation (five mutations applied to the shipped script one at a time, all -five went red). B7 is **half done** — rule 4 is now opt-in rather than retired, which preserves the owner's -decision while removing the trap where re-installing would activate it. B5, B6, B8, B10 and B11 are **not -started**; B6 (worktree-first entry point) is the one with the largest durable effect and no code in it. +**Status**, stated exactly — an overstated one here is worse than none, because the next session acts on it. -Remaining order: B6, then B8 (cross-worktree blast radius), then B5, B10, B11. +| | State | What is actually true | +|---|---|---| +| B2, B9 | **Done** | Receipts (sanitised, one record per line, retried under contention) and the deny-message fix, with tests. | +| B1 | **Mostly done** | Parity check, `-Status` audit, `-EnterWorktreeGate` opt-in. **Not** done: it emits no liveness receipt through `scripts/quality/liveness.py`, so on CI it is three honest skips rather than a tracked result. | +| B3 | **Mostly done** | Shared resolver, case-sensitive `-C`, `cd`/`pushd` from the prefix only, `--work-tree` / `GIT_WORK_TREE` / `--git-dir` as additional candidates. **Not** done: `GIT_DIR=` is unhandled. Rule 3b's early return is now moot — both rules resolve through the same function — rather than literally inverted. | +| B4 | **Mostly done** | Per-line scanning, continuation folding, interpreter-argument recursion, quoted spans blanked. **Not** done: the split helper is still not shared with `block-blanket-git-stage.ps1`, so the two hooks can still disagree about what a command is. | +| B7 | **Half done** | Rule 4 is opt-in, not retired — preserving the owner's decision while removing the trap where re-installing would activate it. | +| B5, B6, B8, B10, B11 | **Not started** | B6 (worktree-first entry point) has the largest durable effect and no code in it. | + +Remaining order: B6, then B8 (cross-worktree blast radius), then B5, B10, B11, then the B1/B3/B4 remainders. + +Each shipped fix was proved to catch its own regression by mutation — five mutations applied to the shipped +script one at a time, all five went red — and an adversarial review of the first attempt found three +regressions it had introduced, since fixed and pinned by +[`tests/test_worktree_gate_shell_semantics.py`](../tests/test_worktree_gate_shell_semantics.py). One caveat carried forward: **none of the merged work is live until the gate is re-installed.** That is the same property that made rule 4 inert, now with a test watching it. diff --git a/scripts/worktree/install-gate.ps1 b/scripts/worktree/install-gate.ps1 index 73785d29..b7612266 100644 --- a/scripts/worktree/install-gate.ps1 +++ b/scripts/worktree/install-gate.ps1 @@ -136,8 +136,10 @@ function Get-GateHash([string]$Path) { (Get-FileHash -LiteralPath $Path -Algorithm SHA256).Hash } -# Every tool the gate script branches on. Read from the SOURCE, so -Status can say which implemented rules -# are unwired rather than printing a bare count nobody can calibrate. +# Every tool a gate script branches on. -Status calls this on the INSTALLED copy, deliberately: the +# question it answers is "which rules does the gate that is RUNNING have, and are they all wired", and the +# source's rule set is not evidence for either. That is the whole point of the audit -- rule 4 was in the +# source, declared by this installer, and covered by tests, while the running gate had never heard of it. function Get-HandledTools([string]$Path) { if (-not (Test-Path -LiteralPath $Path)) { return @() } $text = Get-Content -LiteralPath $Path -Raw diff --git a/tests/test_gate_installed_parity.py b/tests/test_gate_installed_parity.py index 85cf5c4f..38d48f4a 100644 --- a/tests/test_gate_installed_parity.py +++ b/tests/test_gate_installed_parity.py @@ -10,8 +10,16 @@ enforcing it forever, while every test correctly reports it gone. These tests are LOCAL-MACHINE tests. On CI there is no installed gate and they skip -- which is honest, -because the drift they detect is a developer-box condition, not a repository one. They print what they -scanned so a skip can never be mistaken for a pass. +because the drift they detect is a developer-box condition, not a repository one. + +**What CI therefore does NOT guard**: installed-vs-source parity, wired-matcher correctness, and +unwired-rule detection. Only the source-only OPT_IN_TOOLS sanity check runs there. Say that plainly rather +than let three green-looking dots imply coverage. + +Every test announces what it scanned BEFORE it can skip, so the reason is in the output either way. That +ordering is the whole mitigation and it is easy to undo by accident: a print placed after a skip never +runs, and the repo's pytest config carries no ``-rs``, so the skip reason would not be shown either. It +rendered as a bare ``sss.`` until this was fixed. Parity is asserted only when the source script is COMMITTED. Mid-change the two are *supposed* to differ, and a test that nagged on every edit would be re-run with ``-k`` until someone deleted it. @@ -88,19 +96,23 @@ def source_is_committed() -> bool: def test_the_installed_gate_matches_the_committed_source() -> None: + # Announce the target BEFORE any skip. A print after a skip never runs, and with no -rs in the pytest + # config the reason is not shown either -- the file then renders as a bare "sss." on CI, which is the + # exact skip-reads-as-pass ambiguity this suite exists to remove. + print(f"scanning: {INSTALLED_GATE} vs {SOURCE_GATE}") if not INSTALLED_GATE.is_file(): pytest.skip( - f"no gate installed at {INSTALLED_GATE} -- nothing is enforcing; nothing to compare" + f"SKIP (nothing compared): no gate installed at {INSTALLED_GATE} -- nothing is enforcing" ) if not source_is_committed(): pytest.skip( - f"{SOURCE_GATE.relative_to(ROOT)} has uncommitted changes -- the installed copy is SUPPOSED " - f"to differ mid-edit. Re-run after committing." + f"SKIP (nothing compared): {SOURCE_GATE.relative_to(ROOT)} has uncommitted changes -- the " + f"installed copy is SUPPOSED to differ mid-edit. Re-run after committing." ) installed = hashlib.sha256(INSTALLED_GATE.read_bytes()).hexdigest() source = hashlib.sha256(SOURCE_GATE.read_bytes()).hexdigest() - print(f"scanned: {INSTALLED_GATE} ({installed[:12]}) vs {SOURCE_GATE} ({source[:12]})") + print(f"compared: installed={installed[:12]} source={source[:12]}") assert installed == source, ( f"The RUNNING gate is not this checkout's script.\n" @@ -116,15 +128,17 @@ def test_every_wired_matcher_names_a_tool_the_gate_handles() -> None: """The inverse drift: a matcher for a tool the script ignores burns a pwsh subprocess on every call, and -- worse -- reads as coverage that does not exist.""" dirs = config_dirs() + print(f"scanning {len(dirs)} config dir(s) against {INSTALLED_GATE}") if not dirs: - pytest.skip("no Claude config dirs on this box -- nothing is wired, so nothing to check") + pytest.skip("SKIP (nothing scanned): no Claude config dirs on this box -- nothing is wired") if not INSTALLED_GATE.is_file(): pytest.skip( - f"no gate installed at {INSTALLED_GATE} -- matchers cannot be judged against it" + f"SKIP (nothing scanned): no gate at {INSTALLED_GATE} -- matchers have nothing to be judged " + f"against" ) handled = handled_tools(INSTALLED_GATE.read_text(encoding="utf-8")) - print(f"scanned {len(dirs)} config dir(s) against {len(handled)} rule(s) in the INSTALLED gate") + print(f"compared against {len(handled)} rule(s) in the INSTALLED gate") stray: dict[str, set[str]] = {} for d in dirs: wired = wired_matchers(d / "settings.json") @@ -139,16 +153,19 @@ def test_every_non_optional_rule_is_wired_in_every_config_dir() -> None: check that would have caught rule 4 on day one -- the repo-side wiring test could not, because it compares the installer to the script and never looks at what is actually installed.""" dirs = config_dirs() + print( + f"scanning {len(dirs)} config dir(s); opt-in (absence is not drift): {sorted(OPT_IN_TOOLS)}" + ) if not dirs: - pytest.skip("no Claude config dirs on this box -- nothing is wired, so nothing to check") + pytest.skip("SKIP (nothing scanned): no Claude config dirs on this box -- nothing is wired") if not INSTALLED_GATE.is_file(): - pytest.skip(f"no gate installed at {INSTALLED_GATE} -- there is no live rule set to wire") + pytest.skip( + f"SKIP (nothing scanned): no gate at {INSTALLED_GATE} -- no live rule set to wire" + ) handled = handled_tools(INSTALLED_GATE.read_text(encoding="utf-8")) required = handled - OPT_IN_TOOLS - print( - f"scanned {len(dirs)} config dir(s); require {sorted(required)}; opt-in {sorted(OPT_IN_TOOLS)}" - ) + print(f"required in every dir: {sorted(required)}") unwired: dict[str, list[str]] = {} for d in dirs: diff --git a/tests/test_worktree_gate_command_parsing.py b/tests/test_worktree_gate_command_parsing.py index 802efc01..60bbb9eb 100644 --- a/tests/test_worktree_gate_command_parsing.py +++ b/tests/test_worktree_gate_command_parsing.py @@ -95,6 +95,8 @@ def test_a_config_override_whose_value_looks_like_an_outside_path_still_denies( ) +# Not a bypass fix -- a regression guard. This case denied before the change too, and it is here so a +# future edit to the config-override parsing cannot break the ordinary path while fixing the exotic one. def test_a_config_override_does_not_break_an_ordinary_deny(primary: Path, repos_file: Path) -> None: assert_denied( run_gate(shell("git -c advice.detachedHead=false switch main", cwd=primary), repos_file) @@ -117,9 +119,9 @@ def test_a_relative_cd_to_the_primary_is_denied( nested: Path, repos_file: Path, command: str ) -> None: """The whole point of the fix. `../../..` from a nested worktree IS the primary, but the in-text - fallback only ever matched the primary's LITERAL spelling, so none of these named it. Note the list - spans nine verbs: rule 3b handles only checkout/switch, so for the other seven there was never even a - hand-off to bow out of -- they were plain misses.""" + fallback only ever matched the primary's LITERAL spelling, so none of these named it. The seven cases + span seven verbs, and rule 3b handles only checkout/switch -- so for five of them there was never even + a hand-off to bow out of; they were plain misses.""" reason = assert_denied(run_gate(shell(command, cwd=nested), repos_file)) assert "SHARED PRIMARY" in reason diff --git a/tests/test_worktree_gate_receipts.py b/tests/test_worktree_gate_receipts.py index 410b298e..2b4316a3 100644 --- a/tests/test_worktree_gate_receipts.py +++ b/tests/test_worktree_gate_receipts.py @@ -12,6 +12,7 @@ from __future__ import annotations +import re import shutil import subprocess from pathlib import Path @@ -97,6 +98,69 @@ def test_each_rule_stamps_its_own_id( assert expected_rule in receipts(repos_file)[0] +def test_rule_4_stamps_its_own_id(primary: Path, repos_file: Path) -> None: + """Rules 3b and 4 were the two the first version of this file left unattributed.""" + payload = { + "session_id": "s-1", + "cwd": str(primary), + "tool_name": "EnterWorktree", + "tool_input": {"name": "wt-1"}, + } + assert_denied(run_gate(payload, repos_file)) + assert "rule=4" in receipts(repos_file)[0] + + +@pytest.mark.skipif(shutil.which("git") is None, reason="needs git on PATH") +def test_rule_3b_stamps_its_own_id(tmp_path: Path) -> None: + """3b needs a REAL worktree -- it asks git whether the target is a governed linked worktree.""" + + def git(*args: str, cwd: Path | None = None) -> None: + subprocess.run( + ["git", *args], + cwd=str(cwd) if cwd else None, + check=True, + capture_output=True, + text=True, + ) + + primary = tmp_path / "Primary" + git("init", "-b", "main", str(primary)) + git("config", "user.email", "t@example.com", cwd=primary) + git("config", "user.name", "t", cwd=primary) + (primary / "seed.txt").write_text("seed\n", encoding="utf-8") + git("add", "-A", cwd=primary) + git("commit", "-m", "seed", cwd=primary) + git("branch", "claude/other-branch", cwd=primary) + wt = tmp_path / "Primary-wt" + git("worktree", "add", "-b", "wt-branch", str(wt), cwd=primary) + repos = tmp_path / "repos.txt" + repos.write_text(f"{primary}\n", encoding="utf-8") + + assert_denied(run_gate(shell("git checkout claude/other-branch", cwd=wt), repos)) + line = receipts(repos)[0] + assert "rule=3b" in line, line + assert "git checkout" in line + + +def test_every_receipt_carries_the_gate_version(primary: Path, repos_file: Path) -> None: + """The version stamp is how `install-gate.ps1 -Status` names the build that is running. If it stops + being written, a log full of denials cannot be attributed to a rule set.""" + assert_denied(run_gate(edit(primary / "x.py", primary), repos_file)) + line = receipts(repos_file)[0] + assert re.search(r"\tv\d{4}\.\d{2}\.\d{2}\.\d+\t", line), line + assert re.search(r"\tpid=\d+\t", line), line + + +def test_rule_1_governs_the_primarys_own_git_hooks_directory( + primary: Path, repos_file: Path +) -> None: + """The gate's enforcement surface must not be editable through the gate. `/.git/hooks` is + under a governed root and is NOT under the `.claude/worktrees/` exemption, so rule 1 denies a write + there -- docs/SESSION-DRIFT-CONTROLS.md asserts this, and nothing was asserting it.""" + assert_denied(run_gate(edit(primary / ".git" / "hooks" / "pre-commit", primary), repos_file)) + assert_denied(run_gate(edit(primary / ".claude" / "hooks" / "x.ps1", primary), repos_file)) + + def test_the_receipt_never_records_the_raw_command(primary: Path, repos_file: Path) -> None: """A command string is attacker- and operator-influenceable and can carry a token. The rules pass a $Detail they composed themselves (a verb, a target path) precisely so the log cannot become a From 79bd201a7a3d4d2d189a49df98b0816b70d315a9 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 10:30:15 -0500 Subject: [PATCH 08/12] feat(worktree-gate): govern the gate's own kill switch, and the git config that disarms every worktree Two holes with one shape: the thing doing the enforcing was not itself protected. RULE 1a -- the gate's own surface. The installed hook and its allowlist live OUTSIDE every governed root, so Test-Governed returned null for them and rule 1 allowed an Edit to either. One line written to the allowlist disarms the gate for every session on the machine, permanently and silently. The standing answer was that the kill switch is "deliberately NOT named in the deny message" -- obscurity over a file one `ls` away, and not a control. Scoped to the two exact FILES, not their directory. Keying on the parent looked tidier and was wrong twice: the allowlist path is a parameter that can point anywhere (under test it lands in a temp dir, where a directory rule swallowed every unrelated path and failed seven tests), and ~/.claude/hooks/ holds unrelated things this rule has no business governing. settings.json is deliberately NOT covered. The update-config skill exists to edit it, and blocking it would break a supported workflow to close a hole that needs a far more deliberate act than deleting a stray-looking text file. The installer is unaffected either way: it writes from a plain terminal via Copy-Item, which is a shell call and not an Edit. That asymmetry is the point -- the human installs and removes the gate. RULE 3c -- git config that disarms the shared repository. `config` changes no tree, so the verb list never saw it, and its blast radius is larger than any tree swap: all eight worktrees share one .git, so `git config core.hooksPath /dev/null` run in ANY of them disables the ledger, claim and secret-leak commit gates for EVERY one at once, and nothing reports that they stopped running. `-c core.hooksPath=` does it for a single command, which is enough for one commit to slip past. Deliberately does NOT reuse Test-Governed: its .claude/worktrees/ exemption is exactly wrong here. A linked worktree is not the primary -- correct for a tree swap, and the opposite of correct for a config write that lands in the shared file. 3c asks git for the common dir instead, which covers nested worktrees, sibling worktrees and the primary alike, and falls through to ALLOW on any git failure. Narrowness is the feature, and half the new tests exist to hold it: reads (--get, --list, --show-origin), every key off the disarm list, an ungoverned repo, and a non-repo cwd all stay untouched. A guard that also blocks ordinary work gets routed around. Four mutations applied to the shipped script one at a time -- 1a never firing, 3c never detecting a key, 3c losing its governed-root match, 3c losing its read exclusion -- all four went red. 166 gate tests pass. --- docs/SESSION-DRIFT-CONTROLS.md | 11 +- scripts/hooks/worktree_gate.ps1 | 99 +++++++++++ tests/test_worktree_gate_control_plane.py | 195 ++++++++++++++++++++++ 3 files changed, 301 insertions(+), 4 deletions(-) create mode 100644 tests/test_worktree_gate_control_plane.py diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 077b00c0..39a93fc1 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -110,6 +110,8 @@ reading the emitted decision — not by reading source alone. | Control | Scope | Status | |---|---|---| | Rule 1 — write into primary | user | **LIVE** (probe-verified DENY) | +| Rule 1a — write to the gate's own script or allowlist | user | **NEW** — the kill switch sat outside every governed root, so rule 1 allowed an `Edit` to it | +| Rule 3c — `git config` that disarms the shared repo | user | **NEW** — `core.hooksPath` / `core.worktree` / `alias.*` / `include.path`; asks git for the common dir, so a *linked worktree* is covered too | | Rule 2 — dispatch from primary | user | **LIVE** (probe-verified DENY) | | Rule 3 — git verbs vs primary | user | **LIVE, partial** — the `-c` and relative-`cd` holes are fixed in source (G2/G3); enumerated-verb gaps remain (G9) | | Rule 3b — worktree hijack | user | **LIVE but narrow** — 2 of 11 verbs, existing-local-branch destinations only | @@ -227,7 +229,7 @@ Note also a naming collision worth cleaning up: `mefor.homeBranch` (a git config `restore-primary.ps1` and selfheal) and `mefor-home-branch` (an untracked file written by `new.ps1`) are two different mechanisms one letter apart. -### G5 — The gate's own enforcement surface is writable by every governed session +### G5 — The gate's own enforcement surface is writable by every governed session — **FIXED** The live gate is `~/.claude/hooks/worktree_gate.ps1`; the kill switch is `~/.claude/hooks/worktree-gate.repos.txt`; the matchers are in five `settings.json` files. **None of those @@ -239,7 +241,7 @@ The gate's header says the kill switch is "deliberately NOT named in the deny me `bypassPermissions` would use it." That is security by obscurity over a file one `ls` away. By contrast `/.git/hooks` **is** governed by rule 1 already, and a test asserts it. -### G6 — The shared `.git` is unprotected against config-level disarm +### G6 — The shared `.git` is unprotected against config-level disarm — **FIXED** `config` is not in rule 3's verb list, so `git config core.hooksPath …` and `git -c core.hooksPath=… commit` pass (probe-verified ALLOW) — disabling the ledger/claim/leak commit gates for **all eight worktrees at @@ -440,10 +442,11 @@ analysis must be redone. | B1 | **Mostly done** | Parity check, `-Status` audit, `-EnterWorktreeGate` opt-in. **Not** done: it emits no liveness receipt through `scripts/quality/liveness.py`, so on CI it is three honest skips rather than a tracked result. | | B3 | **Mostly done** | Shared resolver, case-sensitive `-C`, `cd`/`pushd` from the prefix only, `--work-tree` / `GIT_WORK_TREE` / `--git-dir` as additional candidates. **Not** done: `GIT_DIR=` is unhandled. Rule 3b's early return is now moot — both rules resolve through the same function — rather than literally inverted. | | B4 | **Mostly done** | Per-line scanning, continuation folding, interpreter-argument recursion, quoted spans blanked. **Not** done: the split helper is still not shared with `block-blanket-git-stage.ps1`, so the two hooks can still disagree about what a command is. | +| B5, B8 | **Done** | Rules 1a and 3c: the gate's own script and allowlist are governed, and the `git config` keys that disarm the shared repo are denied from any worktree. **Not** done: `~/.claude/settings.json` is deliberately left writable — the `update-config` skill exists to edit it, and blocking it would break a supported workflow to close a hole needing a far more deliberate act. | | B7 | **Half done** | Rule 4 is opt-in, not retired — preserving the owner's decision while removing the trap where re-installing would activate it. | -| B5, B6, B8, B10, B11 | **Not started** | B6 (worktree-first entry point) has the largest durable effect and no code in it. | +| B6, B10, B11 | **Not started** | B6 (worktree-first entry point) has the largest durable effect and no code in it. | -Remaining order: B6, then B8 (cross-worktree blast radius), then B5, B10, B11, then the B1/B3/B4 remainders. +Remaining order: B6, then B10, B11, then the B1/B3/B4 remainders. Each shipped fix was proved to catch its own regression by mutation — five mutations applied to the shipped script one at a time, all five went red — and an adversarial review of the first attempt found three diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index bca2e4fd..2548a13e 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -419,6 +419,62 @@ if ($tool -in @("Bash", "PowerShell")) { $cmd = [string]$hook.tool_input.command if (-not $cmd) { exit 0 } + # ----------------------------------------------------------------------------------------------- + # Rule 3c -- a git CONFIG write that disarms the SHARED repository. `config` changes no tree, so the + # verb list never saw it, and its blast radius is worse than a tree swap: all eight worktrees share + # one `.git`, so `git config core.hooksPath /dev/null` run in ANY of them disables the ledger, claim + # and leak commit gates for EVERY one at once. `-c core.hooksPath=` does it for a single command, + # which is enough to slip one commit past them. `core.worktree`, `alias.*` and `include.path` are the + # same class: they redirect what a later git command actually does. + # + # Deliberately narrow. Reads (`--get`, `--list`, ...) and every other key stay untouched -- this must + # not become a general ban on configuring a repo, and `git config user.email` is ordinary setup. + # + # Unlike rules 1-3 this does NOT use Test-Governed, because its `.claude/worktrees/` exemption is + # exactly wrong here: a linked worktree is not the primary, but its config write lands in the SHARED + # config and harms every sibling. Ask git for the common dir instead, which catches nested worktrees, + # sibling worktrees and the primary alike. Any git failure falls through to ALLOW. + # ----------------------------------------------------------------------------------------------- + $dangerKeys = 'core\.hookspath|core\.worktree|alias\.[\w.-]+|include\.path|includeif\.' + foreach ($seg in (Get-ScannableSegments $cmd)) { + if ($seg.Scan -cnotmatch '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') { continue } + if ($seg.Scan -notmatch "(?:\bconfig\b[^|;&]*?\s|-c\s+)(?$dangerKeys)") { continue } + $badKey = $Matches['key'] + # A read is not a write. + if ($seg.Scan -match '(?:^|\s)--(get|get-all|get-regexp|list|show-origin)(\s|$)') { continue } + + $at = [regex]::Match($seg.Raw, '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') + $pfx = $(if ($at.Success) { $seg.Raw.Substring(0, $at.Index) } else { "" }) + $where = @(Get-GitTargetCandidatesRaw $seg.Raw $pfx $cwdRaw) + if ($where.Count -eq 0) { continue } + + $common = "$(& git -C $where[0] rev-parse --git-common-dir 2>$null)".Trim() + if ($LASTEXITCODE -ne 0 -or -not $common) { continue } + $commonCmp = Get-ComparablePath $common $where[0] + $govCfg = $null + foreach ($r in $roots) { + if ($commonCmp -eq $r.Compare -or $commonCmp.StartsWith("$($r.Compare)/")) { $govCfg = $r; break } + } + if (-not $govCfg) { continue } + + Write-Deny -Rule "3c" -Detail "git config $badKey" -Reason @" +BLOCKED: setting '$badKey' would change the SHARED git configuration of $($govCfg.Display). + +Every worktree of this repository shares one .git directory, so this is not a local change: it takes +effect for all of them at once. Repointing core.hooksPath (or aliasing a command, or redirecting +core.worktree) disables the commit-time ledger, claim and secret-leak gates for every session on this +machine, and nothing would report that they had stopped running. + +What to do instead: + * If a commit hook is failing, FIX THE CAUSE -- the hook output names it. Never route around a gate; + that converts a caught problem into an uncaught one. + * If you need a different hook set for a genuine reason, that is a repository decision. STOP and tell + the user: "I need to change core.hooksPath on the shared repo and the worktree gate blocked it." + * Ordinary per-user config (user.email, user.name, and anything that is not on the disarm list) is + untouched and needs no workaround. +"@ + } + # The verb must be a whole SUBCOMMAND. `\bmerge\b` is not enough: a hyphen counts as a word boundary, # so it also matches the `merge` inside `merge-base` and `merge-tree` -- both of which are READ-ONLY # and are exactly what a session should be using instead of a checkout. Require the verb to end at @@ -557,6 +613,49 @@ if (-not [System.IO.Path]::IsPathRooted($target)) { $target = Join-Path ([string]$hook.cwd) $target } +# --------------------------------------------------------------------------------------------------- +# Rule 1a -- the gate's OWN enforcement surface. The installed script and its allowlist live OUTSIDE +# every governed root, so Test-Governed returned $null for them and rule 1 allowed an Edit to either: one +# line written to the allowlist disarms the gate for every session on this machine, permanently and +# silently. The docstring's answer was that the kill switch is "deliberately NOT named in the deny +# message" -- obscurity over a file one `ls` away, and not a control. +# +# Scoped to ~/.claude/hooks/ ONLY, deliberately. settings.json is NOT covered: editing it is sanctioned +# work (the update-config skill exists to do exactly that), and blocking it would break a supported +# workflow to close a hole that requires a far more deliberate act than deleting a stray-looking text +# file. This closes the accident and the cheap route-around, which is all a guardrail is for. +# +# The installer is unaffected: it writes from a plain terminal via Set-Content/Copy-Item, which is a +# SHELL call and not an Edit, so no tool-argument rule sees it. That asymmetry is the point -- the human +# installs and removes the gate; a session may not. +# --------------------------------------------------------------------------------------------------- +# Match the two exact FILES, never their parent directory. Keying on the parent looked tidier and was +# wrong twice over: $ReposFile is a parameter that can point anywhere (under test it sits in a temp dir, +# where it swallowed every unrelated path), and ~/.claude/hooks/ also holds things this rule has no +# business governing. The surface worth protecting is precisely the kill switch and the script it arms. +$gateFiles = @( + (Get-ComparablePath $ReposFile) + (Get-ComparablePath (Join-Path (Split-Path -Parent $ReposFile) "worktree_gate.ps1")) +) | Where-Object { $_ } +if ((Get-ComparablePath $target) -in $gateFiles) { + Write-Deny -Rule "1a" -Detail $target -Reason @" +BLOCKED: this writes to the worktree gate's own enforcement surface ($target). + +That directory holds the installed hook and its allowlist. The allowlist is the gate's kill switch -- a +single edit there turns it off for every session on this machine, so a session may not write here at all. +This is not a file to fix in passing. + +If the gate is genuinely wrong -- a false positive, a rule that needs changing -- fix it at the SOURCE and +re-install, which is a human act from a plain terminal: + + scripts\hooks\worktree_gate.ps1 the rule you want to change + scripts\worktree\install-gate.ps1 installs it (refuses to run inside Claude Code) + +If you need it OFF right now, say so and let the user decide, in these words: "I want the worktree gate +turned off and I need you to do it." Do not disable it yourself. +"@ +} + $root = Test-Governed (Get-ComparablePath $target) if (-not $root) { exit 0 } diff --git a/tests/test_worktree_gate_control_plane.py b/tests/test_worktree_gate_control_plane.py new file mode 100644 index 00000000..c27edd67 --- /dev/null +++ b/tests/test_worktree_gate_control_plane.py @@ -0,0 +1,195 @@ +"""The gate's own enforcement surface, and the shared git config that arms the commit gates. + +Two holes with a shared shape: the thing doing the enforcing was not itself protected. + +**Rule 1a.** The installed hook and its allowlist live OUTSIDE every governed root, so ``Test-Governed`` +returned ``$null`` for them and rule 1 allowed an ``Edit`` to either. One line written to the allowlist +disarms the gate for every session on the machine, permanently and silently. The previous answer was that +the kill switch is "deliberately NOT named in the deny message" -- obscurity over a file one ``ls`` away. + +**Rule 3c.** ``config`` changes no working tree, so the tree-swap verb list never saw it. Its blast radius +is larger than any tree swap: every worktree shares one ``.git``, so ``git config core.hooksPath`` run in +any of them disables the ledger, claim and secret-leak commit gates for *all* of them at once, and nothing +reports that they stopped running. + +Both rules are deliberately narrow, and half the tests below exist to keep them that way. A guard that +also blocks ordinary work gets routed around, and then it guards nothing. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from tests.test_worktree_gate import assert_denied, edit, run_gate # reuse the subprocess harness + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def shell(command: str, cwd: Path | str, tool: str = "Bash") -> dict[str, Any]: + return { + "session_id": "s-1", + "cwd": str(cwd), + "hook_event_name": "PreToolUse", + "tool_name": tool, + "tool_input": {"command": command}, + } + + +@pytest.fixture +def primary(tmp_path: Path) -> Path: + return tmp_path / "Repo" + + +@pytest.fixture +def repos_file(tmp_path: Path, primary: Path) -> Path: + """Stands in for ~/.claude/hooks/worktree-gate.repos.txt -- the real kill switch.""" + hooks = tmp_path / "hooks" + hooks.mkdir() + f = hooks / "worktree-gate.repos.txt" + f.write_text(f"{primary}\n", encoding="utf-8") + return f + + +# --------------------------------------------------------------- rule 1a: the gate's own surface + + +def test_writing_the_allowlist_is_denied(primary: Path, repos_file: Path) -> None: + """The allowlist IS the kill switch: emptying it turns the gate off everywhere, immediately.""" + reason = assert_denied(run_gate(edit(repos_file, cwd=primary), repos_file)) + assert "kill switch" in reason + assert "install-gate.ps1" in reason # must point at the sanctioned route, not merely refuse + + +def test_writing_the_installed_gate_script_is_denied(primary: Path, repos_file: Path) -> None: + installed = repos_file.parent / "worktree_gate.ps1" + assert_denied(run_gate(edit(installed, cwd=primary), repos_file)) + + +def test_rule_1a_fires_from_a_worktree_too(tmp_path: Path, repos_file: Path) -> None: + """It keys on the TARGET, like every other write rule -- where the session sits is irrelevant.""" + assert_denied(run_gate(edit(repos_file, cwd=tmp_path / "Repo-alerts"), repos_file)) + + +def test_a_neighbouring_file_in_the_same_directory_is_not_gate_surface( + primary: Path, repos_file: Path +) -> None: + """The rule matches two exact FILES, never their parent. Keying on the directory was wrong twice: + the allowlist path is a parameter that can point anywhere (under test it lands in a temp dir, where a + directory rule swallowed every unrelated path and failed seven tests), and the real ~/.claude/hooks/ + holds unrelated things this rule has no business governing.""" + assert run_gate(edit(repos_file.parent / "notes.md", cwd=primary), repos_file) is None + assert run_gate(edit(repos_file.parent / "leases" / "x.json", cwd=primary), repos_file) is None + + +def test_the_repos_source_script_is_not_gate_surface(tmp_path: Path, repos_file: Path) -> None: + """Editing the gate AT SOURCE is the sanctioned way to change a rule -- that is what the deny message + tells you to do, so it must not itself be blocked.""" + src = tmp_path / "Repo-work" / "scripts" / "hooks" / "worktree_gate.ps1" + assert run_gate(edit(src, cwd=tmp_path / "Repo-work"), repos_file) is None + + +def test_rule_1a_is_off_when_the_gate_is_off(primary: Path, tmp_path: Path) -> None: + """The kill switch still wins. An empty allowlist means nothing is governed, including this rule.""" + empty = tmp_path / "empty.txt" + empty.write_text("# nothing\n", encoding="utf-8") + assert run_gate(edit(empty, cwd=primary), empty) is None + + +# --------------------------------------------------------------- rule 3c: the shared git config + + +@pytest.fixture +def repo(tmp_path: Path) -> SimpleNamespace: + """A REAL governed repo + a linked worktree. 3c asks git for the common dir, so it needs both.""" + if shutil.which("git") is None: + pytest.skip("needs git on PATH") + + def git(*args: str, cwd: Path | None = None) -> None: + subprocess.run( + ["git", *args], + cwd=str(cwd) if cwd else None, + check=True, + capture_output=True, + text=True, + ) + + primary = tmp_path / "Primary" + git("init", "-b", "main", str(primary)) + git("config", "user.email", "t@example.com", cwd=primary) + git("config", "user.name", "t", cwd=primary) + (primary / "seed.txt").write_text("seed\n", encoding="utf-8") + git("add", "-A", cwd=primary) + git("commit", "-m", "seed", cwd=primary) + wt = tmp_path / "Primary-wt" + git("worktree", "add", "-b", "wt-branch", str(wt), cwd=primary) + repos = tmp_path / "repos.txt" + repos.write_text(f"{primary}\n", encoding="utf-8") + return SimpleNamespace(primary=primary, wt=wt, repos=repos) + + +@pytest.mark.parametrize( + "command", + [ + "git config core.hooksPath /dev/null", + "git config --local core.hooksPath nowhere", + "git config core.worktree ../elsewhere", + "git config alias.ci 'commit --no-verify'", + "git -c core.hooksPath=/dev/null commit -m x", + "git config include.path ../evil", + ], +) +def test_disarming_the_shared_config_is_denied_in_the_primary( + repo: SimpleNamespace, command: str +) -> None: + reason = assert_denied(run_gate(shell(command, cwd=repo.primary), repo.repos)) + assert "SHARED git configuration" in reason + + +def test_disarming_from_a_LINKED_WORKTREE_is_denied_too(repo: SimpleNamespace) -> None: + """The crux. Test-Governed EXEMPTS a linked worktree, and for tree swaps that is right -- a worktree + is not the primary. For config it is exactly wrong: the write lands in the SHARED config and harms + every sibling. 3c asks git for the common dir instead of reusing that exemption.""" + reason = assert_denied( + run_gate(shell("git config core.hooksPath /dev/null", cwd=repo.wt), repo.repos) + ) + assert "SHARED git configuration" in reason + + +@pytest.mark.parametrize( + "command", + [ + "git config user.email me@example.com", + "git config --get core.hooksPath", + "git config --list", + "git config --get-all remote.origin.fetch", + "git config pull.rebase true", + "git config --show-origin core.hooksPath", + ], +) +def test_ordinary_and_read_only_config_is_untouched(repo: SimpleNamespace, command: str) -> None: + """Narrowness is the feature. Reading the dangerous key is not setting it, and everything off the + disarm list is ordinary repo setup that must not need a workaround.""" + assert run_gate(shell(command, cwd=repo.primary), repo.repos) is None + + +def test_config_in_an_ungoverned_repo_is_untouched(tmp_path: Path, repo: SimpleNamespace) -> None: + other = tmp_path / "Unrelated" + other.mkdir() + subprocess.run(["git", "init", "-b", "main", str(other)], check=True, capture_output=True) + assert run_gate(shell("git config core.hooksPath /dev/null", cwd=other), repo.repos) is None + + +def test_a_non_repo_cwd_fails_open(tmp_path: Path, repo: SimpleNamespace) -> None: + """`rev-parse --git-common-dir` fails outside a repo, and every git failure must ALLOW -- a guardrail + that wedges on an unexpected shape gets uninstalled.""" + plain = tmp_path / "NotARepo" + plain.mkdir() + assert run_gate(shell("git config core.hooksPath /dev/null", cwd=plain), repo.repos) is None From f9e1ac06b99012266900a4dfd1d151363b357c2f Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 10:34:23 -0500 Subject: [PATCH 09/12] fix(worktree): one allowlist for both hooks, guard the second installer, and deliver the leak-gate token to first-party worktrees Three findings from the drift audit, all in the half of the estate that gets forgotten because the PreToolUse gate is the loud one. ONE ALLOWLIST. The gate read ~/.claude/hooks/worktree-gate.repos.txt; the SessionStart backstop read ~/.claude-hooks/worktree-gate.repos.txt. install-gate.ps1 rewrote its own unconditionally, install-selfheal.ps1 seeded the other only if absent, and nothing kept them in sync: adding a governed repo through one installer never reached the other, and `install-gate.ps1 -Uninstall` left the backstop armed and still willing to run `git checkout` on the primary long after the gate was gone. They agreed by luck. The backstop now reads the gate's file, with the legacy path as a FALLBACK -- it is installed as a copy, so a version skew between copy and allowlist must not silently turn it off. THE HIGHER-PRIVILEGE INSTALLER WAS THE UNPROTECTED ONE. install-gate.ps1 has refused to run under CLAUDECODE since it shipped. install-selfheal.ps1 did not, while wiring a user-scope hook that runs `git checkout` on the shared primary unattended, from a script whose canonical source is the calling session's own worktree. It refuses now, before writing anything, and a test pins the pair so the asymmetry cannot reappear. .WORKTREEINCLUDE. `git worktree add` delivers tracked files only, and the leak gate's token list is deliberately gitignored while the pre-commit hook passes --require-tokens and fails closed. new.ps1 hand-copies it for the worktrees IT makes -- which covers none of the first-party paths (`claude --worktree`, desktop parallel sessions, `isolation: worktree` subagents), and those are where every nested .claude/worktrees/ session lands. Such a worktree could not commit AT ALL, with an error that never mentions worktrees. Deliberately excludes .env and secrets/ (refused by policy) and .venv (per worktree on purpose, or a shared editable install silently tests the wrong checkout). The first draft of the allowlist tests asserted only "exit code 0", which the hook returns whether or not it read anything -- vacuous, and the exact pattern this audit exists to catch. They now build a clean repo parked on the wrong branch and assert the REPAIR happens, with an ungoverned-repo control proving the assertion can fail. --- .worktreeinclude | 17 +++ docs/SESSION-DRIFT-CONTROLS.md | 6 +- scripts/worktree/install-selfheal.ps1 | 10 ++ scripts/worktree/worktree-selfheal.ps1 | 22 +++- tests/test_worktree_selfheal_wiring.py | 171 +++++++++++++++++++++++++ 5 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 .worktreeinclude create mode 100644 tests/test_worktree_selfheal_wiring.py diff --git a/.worktreeinclude b/.worktreeinclude new file mode 100644 index 00000000..f67b694a --- /dev/null +++ b/.worktreeinclude @@ -0,0 +1,17 @@ +# Gitignored files that Claude Code copies into every worktree it creates with git -- +# `claude --worktree`, desktop parallel sessions, and `isolation: worktree` subagents. +# .gitignore syntax; only files that match AND are gitignored are copied, so nothing here +# can be committed by accident. +# +# WHY THIS FILE EXISTS. `git worktree add` delivers tracked files only. The leak gate's token +# list is deliberately gitignored, and the pre-commit hook passes --require-tokens and FAILS +# CLOSED without it -- so a fresh worktree could not commit AT ALL, with an error message that +# never mentions worktrees. scripts/worktree/new.ps1 hand-copies it for the worktrees IT makes, +# but that covers none of the first-party creation paths above, which is where every nested +# .claude/worktrees/ session lands. This closes that gap for all of them. +# +# Deliberately NOT here: .env and anything under secrets/. Those are refused by policy +# (CLAUDE.md section 5), and a worktree that needs them should get them from the environment. +# Also not .venv -- it is per-worktree ON PURPOSE (docs/WORKTREES.md), because a shared +# `pip install -e .` binds to one source path and would silently test the wrong checkout. +scripts/security/scan-tokens.local.txt diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 39a93fc1..0e2ac219 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -444,9 +444,11 @@ analysis must be redone. | B4 | **Mostly done** | Per-line scanning, continuation folding, interpreter-argument recursion, quoted spans blanked. **Not** done: the split helper is still not shared with `block-blanket-git-stage.ps1`, so the two hooks can still disagree about what a command is. | | B5, B8 | **Done** | Rules 1a and 3c: the gate's own script and allowlist are governed, and the `git config` keys that disarm the shared repo are denied from any worktree. **Not** done: `~/.claude/settings.json` is deliberately left writable — the `update-config` skill exists to edit it, and blocking it would break a supported workflow to close a hole needing a far more deliberate act. | | B7 | **Half done** | Rule 4 is opt-in, not retired — preserving the owner's decision while removing the trap where re-installing would activate it. | -| B6, B10, B11 | **Not started** | B6 (worktree-first entry point) has the largest durable effect and no code in it. | +| B10 | **Done** | One allowlist, shared by the gate and the backstop, with the legacy path kept as a fallback so a version-skewed installed copy cannot silently disarm the backstop. `install-selfheal.ps1` gained the `CLAUDECODE` refusal its sibling always had — the *higher*-privilege installer was the unprotected one. | +| B6 | **Started** | `.worktreeinclude` added, so the leak gate's gitignored token list reaches every worktree Claude Code creates itself (`--worktree`, desktop sessions, `isolation: worktree` subagents) — previously only `new.ps1`'s own worktrees got it, and a fresh first-party worktree could not commit at all. **Not** done: worktree-first as the documented default entry point, and `new.ps1` calling `git worktree lock` while a session is live. | +| B11 | **Not started** | Verb and teardown holes (G9, G11). | -Remaining order: B6, then B10, B11, then the B1/B3/B4 remainders. +Remaining order: the rest of B6, then B11, then the B1/B3/B4 remainders. Each shipped fix was proved to catch its own regression by mutation — five mutations applied to the shipped script one at a time, all five went red — and an adversarial review of the first attempt found three diff --git a/scripts/worktree/install-selfheal.ps1 b/scripts/worktree/install-selfheal.ps1 index 93cb9d30..351e2ff2 100644 --- a/scripts/worktree/install-selfheal.ps1 +++ b/scripts/worktree/install-selfheal.ps1 @@ -24,6 +24,16 @@ param( [string]$HookPath = (Join-Path $env:USERPROFILE '.claude-hooks\worktree-selfheal.ps1') ) $ErrorActionPreference = 'Stop' + +# Its sibling install-gate.ps1 has refused to run inside Claude Code since it shipped; this installer +# never did, and it is the MORE privileged of the two. It wires a user-scope SessionStart hook that runs +# `git checkout` on the shared primary unattended, and its canonical source is $PSScriptRoot -- the copy +# in the calling session's own worktree, which that session may freely edit. The higher-privilege +# component was the less protected one. +if ($env:CLAUDECODE -eq '1') { + throw "Refusing to run inside Claude Code. This installs a user-scope hook that repairs the shared primary unattended, from a script the calling session can edit. Run it from a plain pwsh terminal." +} + if (-not (Test-Path -LiteralPath $ConfigDir)) { throw "Config dir not found: $ConfigDir" } $settingsPath = Join-Path $ConfigDir 'settings.json' diff --git a/scripts/worktree/worktree-selfheal.ps1 b/scripts/worktree/worktree-selfheal.ps1 index abba6403..edec9f27 100644 --- a/scripts/worktree/worktree-selfheal.ps1 +++ b/scripts/worktree/worktree-selfheal.ps1 @@ -29,11 +29,27 @@ #> param( # Shared allowlist of primary checkouts to guard (one absolute path per line, '#' comments). - # Absent or empty => the backstop is OFF. Kept OUTSIDE any per-account config dir so it is - # account-agnostic and survives an account swap. - [string]$ReposFile = (Join-Path $env:USERPROFILE '.claude-hooks\worktree-gate.repos.txt') + # Absent or empty => the backstop is OFF. + # + # ONE allowlist, shared with the PreToolUse gate. There used to be two -- the gate read + # ~/.claude/hooks/worktree-gate.repos.txt and this backstop read ~/.claude-hooks/worktree-gate.repos.txt + # -- with one installer rewriting its own unconditionally and the other seeding the second only if + # absent. Nothing kept them in sync: adding a governed repo through the gate installer never reached + # the backstop, and `install-gate.ps1 -Uninstall` left this hook armed and still willing to run + # `git checkout` on the primary long after the gate was gone. They agreed only by luck. + # + # The legacy path is still read as a FALLBACK, because this script is installed as a copy: an older + # installed copy paired with a newer allowlist (or the reverse) must not silently turn the backstop + # off. Whichever file exists wins, gate location first. + [string]$ReposFile ) +if (-not $ReposFile) { + $shared = Join-Path $env:USERPROFILE '.claude\hooks\worktree-gate.repos.txt' + $legacy = Join-Path $env:USERPROFILE '.claude-hooks\worktree-gate.repos.txt' + $ReposFile = if (Test-Path -LiteralPath $shared) { $shared } else { $legacy } +} + $ErrorActionPreference = 'SilentlyContinue' function Emit-Context([string]$Message) { diff --git a/tests/test_worktree_selfheal_wiring.py b/tests/test_worktree_selfheal_wiring.py new file mode 100644 index 00000000..09ee34ad --- /dev/null +++ b/tests/test_worktree_selfheal_wiring.py @@ -0,0 +1,171 @@ +"""The SessionStart backstop's allowlist, and the self-protection its installer was missing. + +Two findings from the drift audit, both about the *second* half of the estate — the part everyone forgets +because the PreToolUse gate is the loud one. + +**One allowlist, not two.** The gate read ``~/.claude/hooks/worktree-gate.repos.txt``; this backstop read +``~/.claude-hooks/worktree-gate.repos.txt``. ``install-gate.ps1`` rewrote its own unconditionally, +``install-selfheal.ps1`` seeded the other only if absent, and nothing kept them in sync: adding a governed +repo through one installer never reached the other, and ``install-gate.ps1 -Uninstall`` left this hook +armed and still willing to run ``git checkout`` on the primary long after the gate was gone. They agreed +only by luck. + +**The higher-privilege installer was the less protected one.** ``install-gate.ps1`` has refused to run +under ``CLAUDECODE`` since it shipped. ``install-selfheal.ps1`` did not — while wiring a user-scope hook +that repairs the shared primary *unattended*, from a script whose canonical source is the calling +session's own worktree. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +SELFHEAL = ROOT / "scripts" / "worktree" / "worktree-selfheal.ps1" +INSTALLER = ROOT / "scripts" / "worktree" / "install-selfheal.ps1" + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def run_selfheal(home: Path, cwd: Path) -> subprocess.CompletedProcess[str]: + """Drive the real hook with a synthetic HOME so it reads OUR allowlists, never the machine's.""" + env = {**os.environ, "USERPROFILE": str(home), "HOME": str(home)} + env.pop("CLAUDECODE", None) + return subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(SELFHEAL)], + input="{}", + capture_output=True, + text=True, + timeout=90, + cwd=str(cwd), + env=env, + ) + + +@pytest.fixture +def home(tmp_path: Path) -> Path: + h = tmp_path / "home" + (h / ".claude" / "hooks").mkdir(parents=True) + (h / ".claude-hooks").mkdir(parents=True) + return h + + +def git(*args: str, cwd: Path | None = None) -> None: + subprocess.run( + ["git", *args], cwd=str(cwd) if cwd else None, check=True, capture_output=True, text=True + ) + + +def head_of(repo: Path) -> str: + out = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--abbrev-ref", "HEAD"], + capture_output=True, + text=True, + check=True, + ) + return out.stdout.strip() + + +@pytest.fixture +def drifted(tmp_path: Path) -> Path: + """A clean repo sitting on the WRONG branch -- the state the backstop exists to repair. Asserting on + the repair, not on an exit code, is the difference between testing the allowlist and testing nothing: + the hook exits 0 whether or not it read a thing.""" + if shutil.which("git") is None: + pytest.skip("needs git on PATH") + repo = tmp_path / "Primary" + git("init", "-b", "main", str(repo)) + git("config", "user.email", "t@example.com", cwd=repo) + git("config", "user.name", "t", cwd=repo) + (repo / "seed.txt").write_text("seed\n", encoding="utf-8") + git("add", "-A", cwd=repo) + git("commit", "-m", "seed", cwd=repo) + git("checkout", "-b", "drifted", cwd=repo) + assert head_of(repo) == "drifted" + return repo + + +def test_the_backstop_reads_the_gates_allowlist(home: Path, drifted: Path) -> None: + """The whole point of collapsing them: a repo added through the GATE installer is now guarded by the + backstop too, with no second edit. Proved by the repair actually happening.""" + (home / ".claude" / "hooks" / "worktree-gate.repos.txt").write_text( + f"{drifted}\n", encoding="utf-8" + ) + r = run_selfheal(home, cwd=drifted) + assert r.returncode == 0, r.stderr + assert head_of(drifted) == "main", "the shared allowlist was not read -- no repair happened" + + +def test_the_legacy_allowlist_still_works_when_the_shared_one_is_absent( + home: Path, drifted: Path +) -> None: + """This script is installed as a COPY, so an older installed copy can outlive a newer allowlist and + vice versa. Falling back keeps a version skew from silently turning the backstop off -- the exact + class of failure that left rule 4 inert.""" + (home / ".claude-hooks" / "worktree-gate.repos.txt").write_text( + f"{drifted}\n", encoding="utf-8" + ) + r = run_selfheal(home, cwd=drifted) + assert r.returncode == 0, r.stderr + assert head_of(drifted) == "main", "the legacy fallback was not read -- no repair happened" + + +def test_no_allowlist_anywhere_is_a_clean_no_op(home: Path, drifted: Path) -> None: + """The kill switch: no allowlist and the backstop touches nothing, without erroring. This is also the + control for the two tests above -- if the repair happened here too, they would prove nothing.""" + r = run_selfheal(home, cwd=drifted) + assert r.returncode == 0, r.stderr + assert head_of(drifted) == "drifted", "an ungoverned repo was repaired anyway" + + +def test_the_shared_allowlist_wins_over_the_legacy_one(home: Path, tmp_path: Path) -> None: + """Both present: the gate's location is authoritative, so there is one source of truth to edit.""" + (home / ".claude" / "hooks" / "worktree-gate.repos.txt").write_text( + f"{tmp_path / 'Shared'}\n", encoding="utf-8" + ) + (home / ".claude-hooks" / "worktree-gate.repos.txt").write_text( + f"{tmp_path / 'Legacy'}\n", encoding="utf-8" + ) + src = SELFHEAL.read_text(encoding="utf-8") + shared_at = src.index(".claude\\hooks\\worktree-gate.repos.txt") + legacy_at = src.index(".claude-hooks\\worktree-gate.repos.txt") + assert shared_at < legacy_at, "the shared path must be tried first" + assert "if (Test-Path -LiteralPath $shared) { $shared } else { $legacy }" in src + + +# --------------------------------------------------------------------- installer self-protection + + +def test_the_selfheal_installer_refuses_to_run_inside_claude_code(tmp_path: Path) -> None: + """A session that can install this hook can also point it at a script it wrote. Its sibling + install-gate.ps1 has refused since it shipped; this one did not, and it is the more privileged of the + two -- it wires a hook that runs `git checkout` on the shared primary unattended.""" + cfg = tmp_path / "cfgdir" + cfg.mkdir() + r = subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(INSTALLER), "-ConfigDir", str(cfg)], + capture_output=True, + text=True, + timeout=90, + env={**os.environ, "CLAUDECODE": "1"}, + ) + assert r.returncode != 0, "the installer must refuse under CLAUDECODE" + assert "Refusing to run inside Claude Code" in (r.stderr + r.stdout) + assert not (cfg / "settings.json").exists(), "it must refuse BEFORE writing anything" + + +def test_both_installers_carry_the_same_refusal() -> None: + """Pin the pair. The asymmetry existed for months precisely because nothing compared them.""" + gate = (ROOT / "scripts" / "worktree" / "install-gate.ps1").read_text(encoding="utf-8") + selfheal = INSTALLER.read_text(encoding="utf-8") + for name, text in (("install-gate.ps1", gate), ("install-selfheal.ps1", selfheal)): + assert "CLAUDECODE" in text and "Refusing to run inside Claude Code" in text, ( + f"{name} lost its human-only guard" + ) From b8fe116fcf896490b84aba85996ee80e478ff682 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 10:50:40 -0500 Subject: [PATCH 10/12] =?UTF-8?q?docs:=20re-measure=20rule=202's=20premise?= =?UTF-8?q?=20=E2=80=94=20the=20subagent-denial=20claim=20it=20rests=20on?= =?UTF-8?q?=20does=20not=20hold?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rule 2 denies Task/Agent/Workflow dispatch from the primary, and its third and decisive justification was that "a subagent's denied edits do not reliably surface back to you, so the fan-out would appear to succeed while writing nothing". That was one undocumented observation, never re-measured, and it is the half that justifies a DENY rather than a warning. Tested directly: a subagent dispatched from this worktree, instructed to make exactly one Write into the primary. cwd inherited YES -- that part of the premise holds write landed NO -- rule 1 denied it; ls confirms the file never existed denial surfaced LOUDLY -- full deny text received and reported verbatim; the subagent did not silently report success parent can detect YES, NOW -- the deny left a receipt stamped with the subagent's own pid Two consequences. Rule 1 already CONTAINS a fan-out from the primary, because it keys on the target path regardless of where the parent sits. And the receipt added earlier in this branch closes the observability gap rule 2 was built to work around -- an empty permission_denials list is exactly what a timestamped, pid-stamped log replaces. So rule 2's remaining value is failing FAST, at the parent, before a long fan-out rather than after. Real, but much narrower than its deny text claims, and a reason to revisit deny-versus-warn rather than to keep it at deny by default. Recorded rather than acted on: changing it is the owner's call. Also confirmed live in the same probe: the primary is no longer offered in the deny message's "worktrees you could reuse" list -- G7's fix, in production. Still open: that list names other sessions' worktrees, which the gate explicitly permits writing into. --- docs/SESSION-DRIFT-CONTROLS.md | 44 ++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 0e2ac219..33fb023a 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -376,9 +376,11 @@ wants fan-out can get it under another name; the rule mostly stops the *sanction **Recommendation on rule 2 — keep the deny, fix the cause, don't broaden it blindly.** -- **Keep it.** Rule 1 already denies subagents' writes into the primary keyed on target path, so rule 2's - marginal value is narrow — but the one thing it buys is real: it fails **fast and visibly**, at the - parent, instead of leaving a 40-minute fan-out to report success while writing nothing. +- **Keep it, but know what it is now worth.** Measured (§6): rule 1 already denies a subagent's writes + into the primary on target path, the denial surfaces loudly rather than silently, and the receipt records + it against the subagent's pid. So rule 2 is no longer the only thing between a primary-resident dispatch + and silent loss — its remaining value is failing **fast**, at the parent, before a long fan-out rather + than after. That is worth something; it is not worth what the deny text claims. Revisit deny-vs-warn. - **Do not add `Skill` to it.** That would block every slash command and skill invocation — `/code-review`, `/security-review`, this repo's own skills — from a session the owner opened deliberately. `spawn_task` is also wrong: it creates an advisory chip the user clicks; nothing inherits cwd until they act. @@ -482,13 +484,41 @@ contents; `extensions.worktreeConfig=true`; 3 nested + 5 sibling worktrees; Clau 2.1.198 / 2.1.206 / 2.1.210 / 2.1.216 behaviours (official docs + changelog); `isolation: worktree` (tool schema + docs). +**Measured fresh, 2026-07-29, and it corrects a load-bearing claim.** Rule 2's third justification — +"a subagent's denied edits do not reliably surface back to you, so the fan-out would appear to succeed +while writing nothing" — was one undocumented observation. It was tested directly: a subagent was +dispatched from this worktree and instructed to make exactly one `Write` into the primary. + +| | Result | +|---|---| +| Did the subagent inherit the parent's cwd? | **Yes** — it reported this worktree. Premise holds. | +| Did the write land? | **No.** Rule 1 denied it; `ls` confirmed the file was never created. | +| Did the denial surface? | **Loudly.** The subagent received the full deny text, reported it verbatim, and continued working. It did *not* silently report success. | +| Could the parent detect it independently? | **Yes — now.** The deny left a receipt stamped with the subagent's own pid. | + +Two consequences. First, **rule 1 already contains a fan-out from the primary** — the subagent's writes are +denied on target path regardless of where the parent sat, so the "appears to succeed while writing nothing" +scenario requires a subagent that swallows an explicit hard error. Second, **the receipt closes the +observability gap rule 2 was built to work around**: a parent can now read the log and see exactly what its +subagents were denied, which is precisely what the empty `permission_denials` list failed to provide. + +Rule 2's remaining value is therefore narrower than its deny text claims — it fails *fast*, at the parent, +before a long fan-out rather than after — but it is no longer the only thing standing between a +primary-resident dispatch and silent data loss. That is a reason to revisit whether it should be a deny or +a warning; it is not, on this evidence, a reason to keep it at deny by default. + +Also confirmed live in the same probe: the primary is **no longer offered** in the deny message's +"worktrees you could reuse" list (G7's fix, in production). Still open: that list names *other sessions'* +worktrees, which the gate explicitly permits writing into. + **Cited, not re-measured — treat with care:** - **"29% of Edit/Write calls landed in a worktree; 44% in the primary; 166 sessions over 30 days."** From the gate's own docstring. This is the *sole* quantitative justification for the target-keyed design. Nothing in the repo lets it be recomputed, and nobody has asked whether it still holds. -- **"A subagent's denied edits came back with an empty `permission_denials` list."** The entire - evidentiary basis for rule 2 being a **deny** rather than a warning. One observation, undocumented, - never re-measured, and now partly overtaken by `isolation: worktree`. +~~**"A subagent's denied edits came back with an empty `permission_denials` list."**~~ **Superseded** — + re-measured above. The denial surfaces clearly to the subagent, the write never lands, and the receipt + now records it against the subagent's pid. -Both deserve a re-measurement before the next round of changes. +The 29% / 44% figures still deserve a re-measurement before the next round of changes; nothing in the repo +can recompute them. From 42e6a299e576e82f7d83f070f103b2b7599509e7 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 10:56:32 -0500 Subject: [PATCH 11/12] feat(worktree-gate): stop one session deleting another session's worktree Every rule so far protects a working tree from being SWAPPED. None protected it from being DELETED, which is strictly worse: `git worktree remove` takes the directory and its branch along with any uncommitted work in them, there is no undo, and the session using it finds out when its next file read fails. The verb list could never have caught this. Every entry in it is a single token; this is two (`worktree remove`), and `worktree` on its own is a read used constantly. Note also that git refuses to remove the worktree you are STANDING in -- so a `worktree remove` that reaches git is, by construction, aimed at somebody else's. The target is the PATH ARGUMENT, not the cwd, and it cannot be judged with Test-Governed: a linked worktree is exempt there (correctly, for tree swaps) and a sibling worktree falls outside the roots entirely. Rule 3d asks git whether the path is a registered worktree of a governed repo instead, which covers both layouts. Any git failure -- a path that is not a worktree, or does not exist -- falls through to ALLOW. Narrow, and the tests hold it there: `worktree list` stays allowed (the deny message recommends it as the way to check whether a worktree is in use, so blocking it would make the message a trap), `worktree add` stays allowed (it is the sanctioned path out of every other deny in the file), and an ungoverned repo's worktrees are untouched. Two mutations -- 3d never detecting the subcommand, 3d losing its governed-root match -- both went red. 183 gate tests pass. --- docs/SESSION-DRIFT-CONTROLS.md | 5 +- scripts/hooks/worktree_gate.ps1 | 56 +++++++++++++++++ tests/test_worktree_gate_control_plane.py | 75 +++++++++++++++++++++++ 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index 33fb023a..bcde8261 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -112,6 +112,7 @@ reading the emitted decision — not by reading source alone. | Rule 1 — write into primary | user | **LIVE** (probe-verified DENY) | | Rule 1a — write to the gate's own script or allowlist | user | **NEW** — the kill switch sat outside every governed root, so rule 1 allowed an `Edit` to it | | Rule 3c — `git config` that disarms the shared repo | user | **NEW** — `core.hooksPath` / `core.worktree` / `alias.*` / `include.path`; asks git for the common dir, so a *linked worktree* is covered too | +| Rule 3d — `git worktree remove` / `move` on another session's checkout | user | **NEW** — every other rule protects a tree from being *swapped*; this protects it from being *deleted* | | Rule 2 — dispatch from primary | user | **LIVE** (probe-verified DENY) | | Rule 3 — git verbs vs primary | user | **LIVE, partial** — the `-c` and relative-`cd` holes are fixed in source (G2/G3); enumerated-verb gaps remain (G9) | | Rule 3b — worktree hijack | user | **LIVE but narrow** — 2 of 11 verbs, existing-local-branch destinations only | @@ -448,9 +449,9 @@ analysis must be redone. | B7 | **Half done** | Rule 4 is opt-in, not retired — preserving the owner's decision while removing the trap where re-installing would activate it. | | B10 | **Done** | One allowlist, shared by the gate and the backstop, with the legacy path kept as a fallback so a version-skewed installed copy cannot silently disarm the backstop. `install-selfheal.ps1` gained the `CLAUDECODE` refusal its sibling always had — the *higher*-privilege installer was the unprotected one. | | B6 | **Started** | `.worktreeinclude` added, so the leak gate's gitignored token list reaches every worktree Claude Code creates itself (`--worktree`, desktop sessions, `isolation: worktree` subagents) — previously only `new.ps1`'s own worktrees got it, and a fresh first-party worktree could not commit at all. **Not** done: worktree-first as the documented default entry point, and `new.ps1` calling `git worktree lock` while a session is live. | -| B11 | **Not started** | Verb and teardown holes (G9, G11). | +| B11 | **Started** | Rule 3d closes the worst of G9: `git worktree remove` / `move` on another session's checkout. **Not** done: the rest of the absent verbs (`rm`, `mv`, `sparse-checkout`, `checkout-index`, `bisect`, `branch -f`, `update-ref`, `read-tree`), `gh pr checkout`, and G11's teardown half — `prune-merged.ps1` is still sibling-only and still prints a green "nothing to consider" when run from the wrong cwd. | -Remaining order: the rest of B6, then B11, then the B1/B3/B4 remainders. +Remaining order: the rest of B6, the rest of B11, then the B1/B3/B4 remainders. Each shipped fix was proved to catch its own regression by mutation — five mutations applied to the shipped script one at a time, all five went red — and an adversarial review of the first attempt found three diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 2548a13e..a5599095 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -475,6 +475,62 @@ What to do instead: "@ } + # ----------------------------------------------------------------------------------------------- + # Rule 3d -- `git worktree remove` / `move`, which DESTROYS OR RELOCATES ANOTHER SESSION'S CHECKOUT. + # Every rule above protects a tree from being swapped; this one protects it from being deleted, which + # is strictly worse and was entirely unguarded. The verb list could never have caught it: `worktree` + # is two tokens (`worktree remove`) where every other entry is one, and git refuses to remove the + # worktree you are STANDING in -- so a `worktree remove` that reaches git is, by construction, aimed + # at somebody else's. + # + # The target is the PATH ARGUMENT, not the cwd, and it cannot be judged with Test-Governed: a linked + # worktree is exempt there (correctly, for tree swaps) and a sibling worktree falls outside the roots + # entirely. Ask git whether the path is a registered worktree of a governed repo instead. Any git + # failure -- a path that is not a worktree, or does not exist -- falls through to ALLOW. + # ----------------------------------------------------------------------------------------------- + foreach ($seg in (Get-ScannableSegments $cmd)) { + if ($seg.Scan -cnotmatch '(^|[\s;&|(''"\\/])git(\.exe)?["'']?(\s|$)') { continue } + if ($seg.Scan -cnotmatch '\bworktree\s+(?remove|move)(?=\s|$)') { continue } + $wtVerb = $Matches['wtverb'] + + # First positional (non-flag) token after the subcommand is the worktree being acted on. + $after = ($seg.Raw -replace ('(?s)^.*?\bworktree\s+' + $wtVerb + '\b'), '') + $after = ($after -split '(?:&&|\|\||;|\|)', 2)[0] + $victimRaw = $null + foreach ($tok in @($after -split '\s+' | Where-Object { $_ })) { + if ($tok.StartsWith('-')) { continue } + $victimRaw = $tok.Trim('"', "'") + break + } + if (-not $victimRaw) { continue } + + $victimCommon = "$(& git -C $victimRaw rev-parse --git-common-dir 2>$null)".Trim() + if ($LASTEXITCODE -ne 0 -or -not $victimCommon) { continue } + $victimCmp = Get-ComparablePath $victimCommon $victimRaw + $govWt = $null + foreach ($r in $roots) { + if ($victimCmp -eq $r.Compare -or $victimCmp.StartsWith("$($r.Compare)/")) { $govWt = $r; break } + } + if (-not $govWt) { continue } + + Write-Deny -Rule "3d" -Detail "git worktree $wtVerb" -Reason @" +BLOCKED: 'git worktree $wtVerb $victimRaw' acts on a worktree of $($govWt.Display) that belongs to +ANOTHER SESSION -- git refuses to remove the worktree you are standing in, so this one is not yours. + +Removing it deletes that session's working tree and its branch, along with any uncommitted work in them. +There is no undo, and the session using it finds out when its next file read fails. + +What to do instead: + * Cleaning up merged worktrees is a maintenance job with its own dry-run-by-default tool. Run it and + READ what it proposes before applying anything: + pwsh -NoProfile -File $($govWt.Display)\scripts\worktree\prune-merged.ps1 + * To find out whether a worktree is still in use, look rather than delete: + git -C "$($govWt.Display)" worktree list + * If you are certain it is abandoned and must go now, that is the user's call, not yours. Say so: + "I want to remove the worktree $victimRaw and I need you to confirm it is not in use." +"@ + } + # The verb must be a whole SUBCOMMAND. `\bmerge\b` is not enough: a hyphen counts as a word boundary, # so it also matches the `merge` inside `merge-base` and `merge-tree` -- both of which are READ-ONLY # and are exactly what a session should be using instead of a checkout. Require the verb to end at diff --git a/tests/test_worktree_gate_control_plane.py b/tests/test_worktree_gate_control_plane.py index c27edd67..d6d6c3ac 100644 --- a/tests/test_worktree_gate_control_plane.py +++ b/tests/test_worktree_gate_control_plane.py @@ -193,3 +193,78 @@ def test_a_non_repo_cwd_fails_open(tmp_path: Path, repo: SimpleNamespace) -> Non plain = tmp_path / "NotARepo" plain.mkdir() assert run_gate(shell("git config core.hooksPath /dev/null", cwd=plain), repo.repos) is None + + +# --------------------------------------------------------------- rule 3d: destroying another worktree + + +def test_removing_another_sessions_worktree_is_denied(repo: SimpleNamespace) -> None: + """Every other rule protects a tree from being SWAPPED. This one protects it from being DELETED, + which is strictly worse and was entirely unguarded: `git worktree remove` takes the directory and its + branch with any uncommitted work in them, and the session using it finds out when its next read + fails. The verb list could never have caught it -- `worktree remove` is two tokens where every other + entry is one.""" + reason = assert_denied( + run_gate(shell(f'git worktree remove "{repo.wt}"', cwd=repo.primary), repo.repos) + ) + assert "ANOTHER SESSION" in reason + assert "prune-merged.ps1" in reason # offer the maintenance path, do not merely refuse + + +def test_force_removing_and_moving_are_denied_too(repo: SimpleNamespace) -> None: + assert_denied( + run_gate(shell(f'git worktree remove --force "{repo.wt}"', cwd=repo.primary), repo.repos) + ) + assert_denied( + run_gate(shell(f'git worktree move "{repo.wt}" ../elsewhere', cwd=repo.primary), repo.repos) + ) + + +def test_reading_the_worktree_list_is_untouched(repo: SimpleNamespace) -> None: + """`worktree list` is how you find out whether one is in use -- the deny message recommends it, so it + must not itself be blocked.""" + assert run_gate(shell("git worktree list", cwd=repo.primary), repo.repos) is None + assert run_gate(shell("git worktree list --porcelain", cwd=repo.primary), repo.repos) is None + + +def test_adding_a_worktree_is_untouched(repo: SimpleNamespace, tmp_path: Path) -> None: + """Creating one is the sanctioned path out of every other deny in this file.""" + assert ( + run_gate( + shell(f"git worktree add {tmp_path / 'New'} -b newbranch", cwd=repo.primary), repo.repos + ) + is None + ) + + +def test_removing_a_worktree_of_an_UNGOVERNED_repo_is_allowed( + tmp_path: Path, repo: SimpleNamespace +) -> None: + other = tmp_path / "Unrelated" + subprocess.run(["git", "init", "-b", "main", str(other)], check=True, capture_output=True) + subprocess.run( + ["git", "-C", str(other), "config", "user.email", "t@e.com"], + check=True, + capture_output=True, + ) + subprocess.run( + ["git", "-C", str(other), "config", "user.name", "t"], check=True, capture_output=True + ) + (other / "s.txt").write_text("s", encoding="utf-8") + subprocess.run(["git", "-C", str(other), "add", "-A"], check=True, capture_output=True) + subprocess.run(["git", "-C", str(other), "commit", "-m", "s"], check=True, capture_output=True) + owt = tmp_path / "Unrelated-wt" + subprocess.run( + ["git", "-C", str(other), "worktree", "add", "-b", "b", str(owt)], + check=True, + capture_output=True, + ) + assert run_gate(shell(f'git worktree remove "{owt}"', cwd=other), repo.repos) is None + + +def test_a_nonexistent_path_fails_open(repo: SimpleNamespace, tmp_path: Path) -> None: + """git cannot classify a path that is not a worktree, and every git failure must ALLOW.""" + assert ( + run_gate(shell(f'git worktree remove "{tmp_path / "nope"}"', cwd=repo.primary), repo.repos) + is None + ) From 6a37fe5f243c7d8ba4801cc9d830d343f9bd0929 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 29 Jul 2026 11:18:53 -0500 Subject: [PATCH 12/12] fix(worktree): $env:USERPROFILE is null off Windows, and in a param default that preempts the guard behind it Linux CI caught this, and it is a better bug than the one the test was written for. $env:USERPROFILE exists only on Windows. Everywhere else it is NULL, and `Join-Path $null ...` raises "Cannot bind argument to parameter 'Path' because it is null" rather than returning a path. Four scripts dereferenced it unguarded. The interesting part is WHERE. In install-selfheal.ps1 it was a PARAMETER DEFAULT, and defaults are evaluated during BINDING -- before the first line of the body. So the CLAUDECODE refusal I added one commit ago was unreachable on Linux: the script died with an unrelated null-path error and never refused. The test asserting the refusal failed, correctly, for a reason nobody predicted. A guard is only a guard if nothing can run ahead of it, so that default moved into the body, after the guard. The same shape in worktree_gate.ps1 would be worse and quieter: its $ReposFile default is also evaluated at binding, and a hook that exits non-zero-but-not-2 lets the tool call through SILENTLY. Off Windows the gate would simply be off, with nothing to say so. All four now resolve the home directory the same way -- honour $env:USERPROFILE when set, because tests and account swaps override it, and fall back to [Environment]::GetFolderPath('UserProfile'), which resolves $HOME on Unix. Three tests, and getting to them took two rounds of mutation. Round 1: unset USERPROFILE and assert the absence of the null-path bind across all four scripts. Killed the regression in worktree-selfheal.ps1 and install-gate.ps1; SURVIVED in install-selfheal.ps1, because the CLAUDECODE guard now exits before $homeDir is computed. Correct behaviour, and it means that test structurally cannot see a regression there. Round 2: added an isolated completion test (-HookPath and -ConfigDir both in tmp). Still survived -- passing -HookPath means $homeDir is never dereferenced at all. The only path that touches it is a plain-terminal run with no -HookPath, which is exactly how an operator invokes it, and which writes under the resolved home. That is safe to test only where the home can be redirected: on Unix the fallback resolves $HOME. So the third test is Unix-only and SKIPPED on Windows with the reason stated, rather than faked into something that passes everywhere and proves nothing. Linux CI is where the bug appeared and is where that assertion runs. 191 gate tests pass, 2 skipped. --- scripts/hooks/worktree_gate.ps1 | 9 +- scripts/worktree/install-gate.ps1 | 9 +- scripts/worktree/install-selfheal.ps1 | 16 ++- scripts/worktree/worktree-selfheal.ps1 | 8 +- tests/test_worktree_selfheal_wiring.py | 129 ++++++++++++++++++++++++- 5 files changed, 161 insertions(+), 10 deletions(-) diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index a5599095..edfb5a26 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -34,7 +34,14 @@ [CmdletBinding()] param( # Newline-delimited list of primary checkouts to govern. Absent or empty => the gate is OFF. - [string]$ReposFile = (Join-Path $env:USERPROFILE ".claude\hooks\worktree-gate.repos.txt") + # + # Resolved null-safely: $env:USERPROFILE is Windows-only and is NULL elsewhere, where Join-Path throws + # a parameter-binding error instead of returning a path. In a PARAMETER DEFAULT that is evaluated + # during binding, so it would kill the hook before its first line -- and a hook that exits + # non-zero-but-not-2 lets the tool call through SILENTLY. The gate would be off with nothing to say so. + [string]$ReposFile = (Join-Path ( + if ($env:USERPROFILE) { $env:USERPROFILE } else { [Environment]::GetFolderPath('UserProfile') } + ) ".claude/hooks/worktree-gate.repos.txt") ) # Bumped whenever a RULE's behaviour changes, so `install-gate.ps1 -Status` can report which build is diff --git a/scripts/worktree/install-gate.ps1 b/scripts/worktree/install-gate.ps1 index b7612266..60cc7fd8 100644 --- a/scripts/worktree/install-gate.ps1 +++ b/scripts/worktree/install-gate.ps1 @@ -74,7 +74,10 @@ $RepoRoot = (Resolve-Path (Join-Path $PSScriptRoot "..\..")).Path # The gate SCRIPT + its allowlist live ONCE, shared, under ~/.claude\hooks -- referenced by absolute path # from every config dir's settings.json, so a single copy (and a single kill switch) governs all accounts. -$HooksDir = Join-Path $env:USERPROFILE ".claude\hooks" +# Null-safely: $env:USERPROFILE is Windows-only and NULL elsewhere, where Join-Path throws a +# parameter-binding error instead of returning a path. Same idiom as its sibling scripts. +$HomeDir = if ($env:USERPROFILE) { $env:USERPROFILE } else { [Environment]::GetFolderPath('UserProfile') } +$HooksDir = Join-Path $HomeDir ".claude/hooks" $GateDst = Join-Path $HooksDir "worktree_gate.ps1" $ReposFile = Join-Path $HooksDir "worktree-gate.repos.txt" @@ -83,9 +86,9 @@ $Marker = "worktree_gate.ps1" # Config dirs to wire. Default: ~/.claude + every existing ~/.claude-account-* (the VS Code launchers). if (-not $ConfigDir -or $ConfigDir.Count -eq 0) { - $cands = @( (Join-Path $env:USERPROFILE ".claude") ) + $cands = @( (Join-Path $HomeDir ".claude") ) $cands += @( - Get-ChildItem -LiteralPath $env:USERPROFILE -Directory -Filter ".claude-account-*" -ErrorAction SilentlyContinue | + Get-ChildItem -LiteralPath $HomeDir -Directory -Filter ".claude-account-*" -ErrorAction SilentlyContinue | ForEach-Object { $_.FullName } ) $ConfigDir = @($cands | Where-Object { Test-Path -LiteralPath $_ -PathType Container }) diff --git a/scripts/worktree/install-selfheal.ps1 b/scripts/worktree/install-selfheal.ps1 index 351e2ff2..402f656c 100644 --- a/scripts/worktree/install-selfheal.ps1 +++ b/scripts/worktree/install-selfheal.ps1 @@ -21,7 +21,13 @@ #> param( [Parameter(Mandatory)][string]$ConfigDir, - [string]$HookPath = (Join-Path $env:USERPROFILE '.claude-hooks\worktree-selfheal.ps1') + # NO DEFAULT HERE, deliberately. Parameter defaults are evaluated during BINDING, before the first + # line of the body -- so a default that throws preempts the CLAUDECODE guard below and the script + # dies with an unrelated error instead of refusing. That is exactly what happened: the default was + # `Join-Path $env:USERPROFILE ...`, and off Windows $env:USERPROFILE is NULL, so on Linux CI the + # installer crashed with "Cannot bind argument to parameter 'Path' because it is null" and the + # refusal never ran. A guard is only a guard if nothing can run ahead of it. + [string]$HookPath ) $ErrorActionPreference = 'Stop' @@ -34,6 +40,12 @@ if ($env:CLAUDECODE -eq '1') { throw "Refusing to run inside Claude Code. This installs a user-scope hook that repairs the shared primary unattended, from a script the calling session can edit. Run it from a plain pwsh terminal." } +# Home directory, null-safely. $env:USERPROFILE is Windows-only and is NULL elsewhere; honour it when set +# (tests and account swaps rely on overriding it) and fall back to the .NET accessor, which resolves $HOME +# on Unix. Every script in this family uses the same idiom -- see worktree-selfheal.ps1 and install-gate.ps1. +$homeDir = if ($env:USERPROFILE) { $env:USERPROFILE } else { [Environment]::GetFolderPath('UserProfile') } +if (-not $HookPath) { $HookPath = Join-Path $homeDir '.claude-hooks/worktree-selfheal.ps1' } + if (-not (Test-Path -LiteralPath $ConfigDir)) { throw "Config dir not found: $ConfigDir" } $settingsPath = Join-Path $ConfigDir 'settings.json' @@ -47,7 +59,7 @@ elseif (-not (Test-Path -LiteralPath $HookPath)) { throw "worktree-selfheal.ps1 $reposFile = Join-Path $sharedDir 'worktree-gate.repos.txt' if (-not (Test-Path -LiteralPath $reposFile)) { # Seed from the worktree gate's existing allowlist if present; else a commented template. - $gateRepos = Join-Path $env:USERPROFILE '.claude\hooks\worktree-gate.repos.txt' + $gateRepos = Join-Path $homeDir '.claude/hooks/worktree-gate.repos.txt' if (Test-Path -LiteralPath $gateRepos) { Copy-Item -LiteralPath $gateRepos -Destination $reposFile -Force } else { Set-Content -LiteralPath $reposFile -Encoding utf8 -Value '# Primaries guarded by the SessionStart backstop. One absolute path per line. Delete to disable.' } } diff --git a/scripts/worktree/worktree-selfheal.ps1 b/scripts/worktree/worktree-selfheal.ps1 index edec9f27..67e2ff6d 100644 --- a/scripts/worktree/worktree-selfheal.ps1 +++ b/scripts/worktree/worktree-selfheal.ps1 @@ -45,8 +45,12 @@ param( ) if (-not $ReposFile) { - $shared = Join-Path $env:USERPROFILE '.claude\hooks\worktree-gate.repos.txt' - $legacy = Join-Path $env:USERPROFILE '.claude-hooks\worktree-gate.repos.txt' + # Null-safely: $env:USERPROFILE is Windows-only and is NULL elsewhere, where Join-Path then throws a + # parameter-binding error rather than returning a path. Honour the env var when set (tests and account + # swaps override it) and fall back to the .NET accessor, which resolves $HOME on Unix. + $homeDir = if ($env:USERPROFILE) { $env:USERPROFILE } else { [Environment]::GetFolderPath('UserProfile') } + $shared = Join-Path $homeDir '.claude/hooks/worktree-gate.repos.txt' + $legacy = Join-Path $homeDir '.claude-hooks/worktree-gate.repos.txt' $ReposFile = if (Test-Path -LiteralPath $shared) { $shared } else { $legacy } } diff --git a/tests/test_worktree_selfheal_wiring.py b/tests/test_worktree_selfheal_wiring.py index 09ee34ad..f77aea64 100644 --- a/tests/test_worktree_selfheal_wiring.py +++ b/tests/test_worktree_selfheal_wiring.py @@ -134,8 +134,8 @@ def test_the_shared_allowlist_wins_over_the_legacy_one(home: Path, tmp_path: Pat f"{tmp_path / 'Legacy'}\n", encoding="utf-8" ) src = SELFHEAL.read_text(encoding="utf-8") - shared_at = src.index(".claude\\hooks\\worktree-gate.repos.txt") - legacy_at = src.index(".claude-hooks\\worktree-gate.repos.txt") + shared_at = src.index(".claude/hooks/worktree-gate.repos.txt") + legacy_at = src.index(".claude-hooks/worktree-gate.repos.txt") assert shared_at < legacy_at, "the shared path must be tried first" assert "if (Test-Path -LiteralPath $shared) { $shared } else { $legacy }" in src @@ -169,3 +169,128 @@ def test_both_installers_carry_the_same_refusal() -> None: assert "CLAUDECODE" in text and "Refusing to run inside Claude Code" in text, ( f"{name} lost its human-only guard" ) + + +# ------------------------------------------------------ no script may assume $env:USERPROFILE exists + + +HOME_DEPENDENT_SCRIPTS = [ + ROOT / "scripts" / "worktree" / "install-selfheal.ps1", + ROOT / "scripts" / "worktree" / "worktree-selfheal.ps1", + ROOT / "scripts" / "worktree" / "install-gate.ps1", + ROOT / "scripts" / "hooks" / "worktree_gate.ps1", +] + + +@pytest.mark.parametrize("script", HOME_DEPENDENT_SCRIPTS, ids=lambda p: p.name) +def test_no_script_dies_when_USERPROFILE_is_unset(script: Path, tmp_path: Path) -> None: + """$env:USERPROFILE is Windows-only and is NULL everywhere else, where `Join-Path $null ...` raises + "Cannot bind argument to parameter 'Path' because it is null" rather than returning a path. + + This is not a portability nicety. In a PARAMETER DEFAULT the expression is evaluated during BINDING, + before the script's first line -- so it preempts whatever guard the body opens with. It did exactly + that: install-selfheal.ps1's CLAUDECODE refusal became unreachable on Linux CI, and the test written + to prove the refusal failed with a null-path error instead. For worktree_gate.ps1 the same crash would + be worse and quieter: a hook exiting non-zero-but-not-2 lets the tool call through SILENTLY, so the + gate would simply be off. + + Asserts the ABSENCE of that specific crash, not success -- these scripts legitimately fail for other + reasons here (a missing config dir, a refusal, an absent allowlist). Only the null-path bind is a bug. + """ + env = {k: v for k, v in os.environ.items() if k != "USERPROFILE"} + env["CLAUDECODE"] = "1" # keep the installers refusing rather than touching this machine + args = ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(script)] + if script.name == "install-selfheal.ps1": + args += ["-ConfigDir", str(tmp_path)] # Mandatory, or pwsh prompts and the run hangs + r = subprocess.run(args, input="{}", capture_output=True, text=True, timeout=90, env=env) + combined = r.stderr + r.stdout + assert "because it is null" not in combined, ( + f"{script.name} dereferenced $env:USERPROFILE without a fallback:\n{combined[:600]}" + ) + assert "Join-Path" not in r.stderr, f"{script.name} raised a Join-Path error:\n{r.stderr[:600]}" + + +def test_the_selfheal_installer_completes_with_USERPROFILE_unset(tmp_path: Path) -> None: + """Covers the path the CLAUDECODE test cannot reach. + + The guard now runs BEFORE $homeDir is computed -- which is the fix, and which means a test that sets + CLAUDECODE=1 exits before that line and cannot see a regression in it. Verified by mutation: reverting + the fallback in install-selfheal.ps1 left the CLAUDECODE test green. The uncovered path is the one + that matters in real use, since an operator runs this from a plain terminal with no CLAUDECODE set. + + Fully isolated: -HookPath and -ConfigDir both land in tmp, and the allowlist is pre-created so the + seeding branch (which would read the real home) never runs. Nothing outside tmp is written. + """ + cfg = tmp_path / "cfg" + cfg.mkdir() + shared = tmp_path / "shared" + shared.mkdir() + (shared / "worktree-gate.repos.txt").write_text("# none\n", encoding="utf-8") + + env = {k: v for k, v in os.environ.items() if k not in ("USERPROFILE", "CLAUDECODE")} + r = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(INSTALLER), + "-ConfigDir", + str(cfg), + "-HookPath", + str(shared / "worktree-selfheal.ps1"), + ], + capture_output=True, + text=True, + timeout=90, + env=env, + ) + combined = r.stderr + r.stdout + assert "because it is null" not in combined, f"null $homeDir reached:\n{combined[:600]}" + assert r.returncode == 0, f"installer failed:\n{combined[:900]}" + assert (cfg / "settings.json").is_file(), "it should have wired the SessionStart hook" + assert (shared / "worktree-selfheal.ps1").is_file(), "it should have copied the hook script" + + +@pytest.mark.skipif( + os.name == "nt", + reason="Windows: GetFolderPath('UserProfile') ignores $HOME, so the default HookPath cannot be " + "redirected away from the real home and this test would write there. Linux CI covers it.", +) +def test_the_default_hook_path_resolves_without_USERPROFILE(tmp_path: Path) -> None: + """The last uncovered use of the fallback: the DEFAULT -HookPath. + + Verified by mutation that neither test above can see a regression here -- reverting install-selfheal's + fallback left all 11 green, because the CLAUDECODE test exits at the guard and the completion test + passes -HookPath explicitly, so $homeDir is never dereferenced in either. The only path that touches it + is a plain-terminal run with no -HookPath, which is exactly how an operator invokes it. + + That path writes under the resolved home, so it is only safe where the home can be redirected: + on Unix the fallback resolves $HOME, which this test controls. Skipped on Windows rather than faked, + because a test that cannot fail is worse than an admitted gap. + """ + fake_home = tmp_path / "home" + (fake_home / ".claude-hooks").mkdir(parents=True) + (fake_home / ".claude-hooks" / "worktree-gate.repos.txt").write_text( + "# none\n", encoding="utf-8" + ) + cfg = tmp_path / "cfg" + cfg.mkdir() + + env = {k: v for k, v in os.environ.items() if k not in ("USERPROFILE", "CLAUDECODE")} + env["HOME"] = str(fake_home) + r = subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(INSTALLER), "-ConfigDir", str(cfg)], + capture_output=True, + text=True, + timeout=90, + env=env, + ) + combined = r.stderr + r.stdout + assert "because it is null" not in combined, ( + f"the default HookPath hit a null home:\n{combined[:600]}" + ) + assert r.returncode == 0, f"installer failed:\n{combined[:900]}" + assert (fake_home / ".claude-hooks" / "worktree-selfheal.ps1").is_file(), ( + "the hook should have been copied under the resolved home" + )