diff --git a/docs/adr/0177-rule-3c-governs-a-repository-by-identity-not-by-path-prefix.md b/docs/adr/0177-rule-3c-governs-a-repository-by-identity-not-by-path-prefix.md new file mode 100644 index 000000000..d275a0120 --- /dev/null +++ b/docs/adr/0177-rule-3c-governs-a-repository-by-identity-not-by-path-prefix.md @@ -0,0 +1,105 @@ +# 0177 — Rule 3c governs a repository by identity, not by path prefix + +- **Status:** Accepted +- **Date:** 2026-08-27 +- **Related:** BACKLOG #1067 · #1066 (the pass that measured this and deferred it) · #1061 · #1082 · `scripts/hooks/worktree_gate.ps1` + +--- + +## Context + +Rule 3c of the worktree gate refuses a write to a disarm key such as `core.hooksPath` in a **governed** +repository, because that one write turns off the ledger, claim and secret-leak commit gates for every +worktree of that repository at once. + +It decided *"is this governed"* by an equality-or-slash-prefix test comparing the **target's git common +dir** against each allowlisted root's **working tree path**. Path containment was standing in for +repository membership, and the two are not the same relation. + +So any repository living anywhere under a governed root inherited that root's governance, including an +independent clone that shares nothing with it but its path. Measured on the shipped gate at +`58e710ad4`: a disarm write in `/vendor/thirdparty` DENIED, from that repo's own cwd and by an +absolute path token from the primary, and both refusals named the **primary**. + +The refusal's own text is what makes this more than a nuisance. It asserts that every worktree of the +repository shares one `.git` directory, which is simply untrue of a vendored clone. The rule-3 comment +in the same file already records what a misdescribing refusal produces: people route around the gate, +and then it guards nothing. + +This is a **false deny in developer tooling**, not a product surface, and not reachable on this box +today — no independent repo currently lives under the primary. It fires the day someone vendors a clone +or drops a scratch repo there. + +## Decision + +**Compare the target's common dir against the root's OWN common dir, equality-or-under — not against +the root's working tree path.** + +Repository identity is what rule 3c was always trying to ask. Every worktree of the primary, sibling or +nested under `.claude/worktrees/`, answers the *same* common dir, so they keep denying without the path +shape being what decides it. A vendored clone answers a different one and is no longer governed. + +**Equality-or-under, rather than equality alone, is the load-bearing half.** A submodule's git dir is +`/.git/modules/`. The obvious identity-only predicate would have flipped submodules from +DENY to ALLOW as a silent side effect of fixing the vendored case — a control weakened by accident, +under cover of a fix. Under-the-common-dir leaves them exactly where they were. + +**Where a root is not a repository's top level, the old path test stays, unchanged.** An allowlist entry +may legitimately name a directory that merely contains checkouts. There is no identity to compare there, +so the fallback keeps catching what it used to catch rather than failing open on it. The top-level check +is not decoration: without it, a root that is a *subdirectory* of some repo would report that repo's git +dir and quietly govern the whole thing. + +Cost is two `git rev-parse` calls per root, cached per invocation, on a path only reached once a disarm +key is already present. + +## Acceptance Criteria + +- **AC-1** — WHEN a disarm key is set in an independent repository that lives under a governed root, + THE SYSTEM SHALL allow it. + → `tests/test_worktree_gate_control_plane.py::test_a_repo_VENDORED_UNDER_a_governed_root_is_not_governed_by_it` +- **AC-2** — WHEN that same repository is named by an absolute path from the governed root's own cwd, + THE SYSTEM SHALL allow it. + → `tests/test_worktree_gate_control_plane.py::test_a_vendored_repo_is_ungoverned_when_named_by_an_ABSOLUTE_PATH_too` +- **AC-3** — WHILE a worktree is nested under `/.claude/worktrees/`, THE SYSTEM SHALL keep denying + a disarm key set from it. + → `tests/test_worktree_gate_control_plane.py::test_a_worktree_NESTED_under_the_governed_root_still_denies` +- **AC-4** — IF the target is a submodule of a governed root, THEN THE SYSTEM SHALL deny, unchanged from + before this decision. + → `tests/test_worktree_gate_control_plane.py::test_a_SUBMODULE_of_a_governed_root_still_denies` + +## Options considered + +1. **Equality-or-under against the root's own common dir** — **CHOSEN.** Fixes the vendored case, keeps + every worktree denying, and leaves the submodule answer untouched so that changing it has to be a + decision rather than a side effect. +2. **Identity alone (`-eq` against the root's common dir)** — Rejected. Correct on the vendored case and + on every worktree, and it silently flips submodules to ALLOW. The item warned about this before the + work started; a mutation of the shipped fix to this form reds AC-4 and nothing else. +3. **Leave the prefix test and repair only the deny WORDING** — Rejected. The verdict is wrong, not just + the sentence. Wording is #1082's subject and is deliberately untouched here. +4. **Resolve the submodule question in this change** — Rejected as out of scope. It is a real question + about what a submodule's config is worth protecting, and it deserves its own item rather than being + answered by whichever predicate happened to be convenient. + +## Consequences + +**Positive** — the gate stops refusing work it has no business refusing, and stops telling the person it +refused something false about their repository. The predicate now compares two things of the same kind. + +**Negative / risks** — two extra `git rev-parse` calls per root the first time a disarm key appears in a +command. A root whose git dir is unreadable falls back to the path test, which is the prior behaviour. + +**Out of scope** — the deny message's wording (#1082). `Get-SessionRoot` carries the same path-prefix +shape, so a session standing in a vendored repo is still attributed to the enclosing governed root; that +value feeds a remedy STRING inside a rule-4 deny message and gates no verdict, so it is a reporting +defect and is recorded rather than fixed here. + +## To resolve on acceptance + +- [x] Does the fix flip submodules? No — pinned by AC-4, and a mutation to the identity-only form reds + that row alone. +- [ ] Should a submodule's shared config be governed at all? Deliberately unanswered. Deciding it means + deciding whether `/.git/modules//config` is worth the same protection as the + superproject's, which this change does not need to settle. Not filed as a number — naming the + subject rather than citing an unallocated one. diff --git a/docs/adr/README.md b/docs/adr/README.md index 55421f6ce..3f876ea9c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -195,3 +195,4 @@ what is withheld and what you can request. | [0170](0170-constant-work-recovery-code-verification-pad-to-the-configured-slot-count-rather-than-short-circuit.md) | **Constant-work recovery-code verification: pad to the configured slot count rather than short-circuit** (BACKLOG #1167, ASVS 11.2.4) -- `_verify_second_factor` walked the argon2id recovery-code hashes and `return`ed on the first match, so the NUMBER of ~64 MiB verifications was a function of which code was presented. **Two leaks and only one matters:** the matched INDEX is worthless (the attacker holds the code and the response answers them anyway), but on the FAILURE path the cost is one verify per REMAINING code -- so anyone holding the password can time a wrong-code refusal and learn how many recovery codes an account has left, without authenticating to the second factor. **The item rated this difficulty 7 on a premise that does not survive measurement:** the re-score says a constant loop 'converts a timing leak into a memory and CPU amplification target', which is the right objection to raise -- and the failure path ALREADY verifies every remaining hash, so making the walk unconditional introduces no new cost, it makes today's WORST CASE the only case. Decision: always run exactly `mfa_recovery_code_count` verifies, padding with the same fixed `_DUMMY_PASSWORD_HASH` the local login leg uses, and select the winner AFTER the loop. Ceiling unmoved (default 10, validator-capped 50); `_argon2`'s semaphore means the concurrent-argon2 footprint cannot widen either; and the path sits behind primary authentication, so it is not an unauthenticated flood surface. **Claims constant WORK, not constant TIME** -- the store round trip on a match is not equalized, the TOTP branch returns earlier, and argon2's own constant-timeness is INHERITED from `argon2-cffi` and has never been measured in this tree, a gap #1167 names and this does not close. No timing measurement was run by the item or by this change. Rejected: leaving the short-circuit as accepted (the fix cost nothing against the existing ceiling, so 'accepted' would have been a judgement made before the amplification premise was checked); and a non-secret lookup index so only ONE verify ever runs -- strictly better on both axes, rejected as OUT OF SCOPE rather than wrong, needing a schema change across three backends and a migration, and recorded so it is not re-derived if the constant walk's cost ever bites | **Accepted (2026-08-22)** -- built with the change. Three parametrized tests pin the count for a first-slot match, a last-slot match and a non-match; proven red-first, removing the padding reds ALL THREE and the file restores byte-identical by SHA-256. Severity conditional per CLAUDE.md section 0 -- **zero deployments**, so this is what a first deployment would have inherited | | [0171](0171-offline-administrator-unlock-a-host-gated-cli-recovery-path-for-a-sole-administrator-lockout.md) | **Offline administrator unlock: a host-gated CLI recovery path for a sole-administrator lockout** (BACKLOG #1236) -- a deployment with ONE administrator had no recovery from account lockout, and every exit is individually deliberate: the bootstrap account is literally `admin`, it is created with no email so the ACCOUNT_LOCKED notice never leaves the process, self-reset is refused, an admin reset needs ANOTHER admin, re-bootstrap fires only on an EMPTY users table, and none of 38 CLI subcommands managed users. **The defect is that they close SIMULTANEOUSLY for that deployment** and nothing notices the conjunction. **The filed acceptance criterion could not discriminate and was amended 2026-08-21:** "recover without hand-editing the database and without a second admin" PASSES ON THE SHIPPED SYSTEM BY WAITING, since the lock self-expires after `lockout_minutes`; a test both a fixed and a broken system pass is not a test. Decision: `messagefoundry admin-unlock --username `. **The gate is HOST ACCESS and it is a real gate rather than an absent one** -- reaching it needs the config, the store path and on an encrypted store the key material, so anyone holding all three already has the database and does not need an unlock to reach an account; that is why it ships unauthenticated, and it is the load-bearing claim. **Clears the lockout and does NOT reset the password** -- deliberately narrower, since a reset would hand the runner a working account. **Reuses `record_login_failure(failed_attempts=0, locked_until=None)` rather than adding a protocol method**, decided by a MEASURED cross-lane fact rather than taste: a named `clear_lockout` would touch base/store/postgres/sqlserver, and all four were uncommitted in a peer lane at the time, so reuse avoided a four-file collision. Exit codes follow the `--json` convention (`_emit_error`, 1) not the M-31 lineage (stderr, 2), verified against `audit-verify` which has no `--json` flag. Carries M-31 forward: a typo'd `--db` is refused rather than creating an empty SQLite store and reporting a false "no such account" | **Accepted (2026-08-22)** -- built with the change. Four tests; **exactly ONE is the control** and the other three are deliberately insensitive -- neutering the clearing call reds only the acceptance test, and the audit-row test still passes under that plant, so it evidences the flow RAN and never that it WORKED. Does NOT address #1236's repetition limb: lock cycles remain unbounded and an attacker can re-lock. Severity conditional per CLAUDE.md section 0 -- **zero deployments** | | [0173](0173-tls-peer-revocation-checking-and-ocsp-stapling-across-terminating-and-originating-surfaces.md) | **TLS peer revocation checking and OCSP stapling across terminating and originating surfaces** (BACKLOG #1005, ASVS 12.1.4) -- the requirement reaches in two directions and the engine answers neither: where the product TERMINATES TLS it does not staple its own certificate's status, and where it ORIGINATES it does not check the peer's revocation. Direction 1 is RUNTIME-BLOCKED rather than unbuilt -- CPython 3.14.6 exposes no stapling surface at all, measured against live positive controls, so no amount of engineering here reaches it. The opt-in client-certificate CRL checking that DOES ship (`config/tls_policy.py:215-276`, three PROTOCOL_TLS_SERVER call sites) is a THIRD combination -- peer revocation on the terminating side -- and moves neither graded direction; that is the single easiest thing in this area to misread. DECISION: accept and document both directions, with one build rider the accept reasoning does not cover -- three originating hops that never reach the existing revocation guard, filed by subject and deliberately unallocated. | **Proposed (2026-08-23)** -- no code changed. Severity is conditional per CLAUDE.md section 0: on a first deployment a revoked partner certificate would keep verifying on the unguarded hops; there are zero deployments today. Five citation errors from the adversarial pass were repaired before filing. | +| [0177](0177-rule-3c-governs-a-repository-by-identity-not-by-path-prefix.md) | **Rule 3c governs a repository by identity, not by path prefix** (BACKLOG #1067) -- the worktree gate decided *"is this repository governed"* with an equality-or-slash-prefix test comparing the target's git COMMON DIR against each allowlisted root's WORKING TREE path, so path containment stood in for repository membership. Any repository living under a governed root inherited its governance, including an independent clone sharing nothing with it but its path -- and the refusal then asserted a shared `.git` that such a clone does not have. A refusal which misdescribes what it blocked is what teaches people to route around a gate; the rule-3 comment in the same file records that having happened. Decision: compare against the root's OWN common dir, **equality-or-under**. Every worktree of the primary, sibling or nested under `.claude/worktrees/`, answers the SAME common dir and keeps denying; a vendored clone answers a different one and is released. **Equality-or-UNDER rather than equality alone is the load-bearing half:** a submodule's git dir is `/.git/modules/`, so the obvious identity-only predicate would have flipped submodules from DENY to ALLOW as a silent side effect of fixing the vendored case -- a control weakened by accident, under cover of a fix. Where a root is not a repository's top level there is no identity to compare and the old path test stays unchanged, so a directory that merely contains checkouts cannot start failing open. Rejected: identity alone; repairing only the deny WORDING (that is #1082's subject and the verdict is wrong, not just the sentence); and answering the submodule question here | **Accepted (2026-08-27)** -- built with the change. Four rows, proven red-first: the two vendored rows FAILED on the shipped gate and the two deny controls passed on both sides. Four mutants, each refused scoring unless it changed the file's SHA-256, all killed -- and the two directions red **disjoint** sets (identity-only reds the submodule row alone; reverting to the path prefix reds the two vendored rows alone, overlap 0), so neither arm is standing in for the other. Gate suite 679 passed / 2 skipped at base and after. Severity conditional per CLAUDE.md section 0 -- a FALSE DENY in developer tooling with no product surface, and not reachable until someone vendors a clone under a governed root | diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 1a8e0cff3..0db96b217 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -1121,6 +1121,31 @@ function Test-Governed([string]$Candidate) { return $null } +# The COMMON DIR of a governed root, as a comparable path -- the object rule 3c actually governs +# (BACKLOG #1067). Empty when the root is not a repository, or is not a repository's TOP LEVEL: an +# allowlist entry may legitimately name a directory that merely CONTAINS checkouts, and a root that is a +# SUBDIRECTORY of some repo would otherwise report THAT repo's git dir and quietly govern all of it. +# +# Cached because the roots cannot change within one invocation and each answer costs two git calls. Rule +# 3c reaches here only once a disarm key is already present, so this is never on the ordinary path. +$rootCommonCache = @{} +function Get-RootCommonDirCmp($Root) { + $key = $Root.Compare + if ($rootCommonCache.ContainsKey($key)) { return $rootCommonCache[$key] } + $value = "" + # The RAW spelling, never the Compare form: that one is lowercased, and this file warns twice that a + # lowercased path handed to `git -C` passes on Windows and misses the real directory on a + # case-sensitive filesystem. Any git failure just means "no answer", which is the empty string. + $where = $Root.Display + $top = "$(& git -C $where rev-parse --path-format=absolute --show-toplevel 2>$null)".Trim() + if ($LASTEXITCODE -eq 0 -and $top -and (Get-ComparablePath $top) -eq $key) { + $common = "$(& git -C $where rev-parse --path-format=absolute --git-common-dir 2>$null)".Trim() + if ($LASTEXITCODE -eq 0 -and $common) { $value = Get-ComparablePath $common } + } + $rootCommonCache[$key] = $value + return $value +} + # --------------------------------------------------------------------------------------------------- # Rule 3b -- hijacking a LINKED WORKTREE by switching it onto an ALREADY-EXISTING branch. Rule 3 below # protects only the shared PRIMARY; this protects every OTHER governed worktree from the one move that @@ -1411,9 +1436,36 @@ What to do instead: $common = "$(& git -C $whereRaw rev-parse --git-common-dir 2>$null)".Trim() if ($LASTEXITCODE -ne 0 -or -not $common) { continue } $commonCmp = Get-ComparablePath $common $whereRaw + # GOVERNANCE IS REPOSITORY IDENTITY, NOT A PATH PREFIX (BACKLOG #1067). This compared the + # TARGET's common dir against the root's WORKING TREE path, so any repository living anywhere + # UNDER a governed root inherited its governance -- including an independent clone vendored + # there, which shares nothing with it but its path. The refusal then went on to assert a shared + # .git directory that such a clone does not have, and a refusal which misdescribes what it + # blocked is exactly what teaches people to route around the gate; rule 3's own comment in this + # file records that having happened. + # + # Comparing against the ROOT'S OWN common dir keeps every case that must keep denying: the + # primary and each of its worktrees -- sibling, or nested under .claude/worktrees -- all answer + # the SAME common dir, so path shape stops being what decides it. + # + # EQUALITY-OR-UNDER, not equality alone, and that is the load-bearing half: a SUBMODULE's git + # dir is /.git/modules/, so the identity-only predicate the item warned about would + # have flipped submodules from DENY to ALLOW as a silent side effect of fixing the vendored + # case. Under-the-common-dir leaves them exactly where they were. Whether a submodule SHOULD be + # governed is its own decision, pinned by a test so that answering it has to be one. $govCfg = $null foreach ($r in $roots) { - if ($commonCmp -eq $r.Compare -or $commonCmp.StartsWith("$($r.Compare)/")) { $govCfg = $r; break } + $rootCommon = Get-RootCommonDirCmp $r + if (-not $rootCommon) { + # The root is not a repository's top level, so there is no identity to compare and the + # path test remains the only answer available. Unchanged deliberately: a root that + # merely contains checkouts must not start failing OPEN on a shape it used to catch. + if ($commonCmp -eq $r.Compare -or + $commonCmp.StartsWith("$($r.Compare)/", [System.StringComparison]::Ordinal)) { $govCfg = $r; break } + continue + } + if ($commonCmp -eq $rootCommon -or + $commonCmp.StartsWith("$rootCommon/", [System.StringComparison]::Ordinal)) { $govCfg = $r; break } } if (-not $govCfg) { continue } diff --git a/tests/test_worktree_gate_control_plane.py b/tests/test_worktree_gate_control_plane.py index 2a8e1eee9..9539799fb 100644 --- a/tests/test_worktree_gate_control_plane.py +++ b/tests/test_worktree_gate_control_plane.py @@ -195,6 +195,106 @@ def test_config_in_an_ungoverned_repo_is_untouched(tmp_path: Path, repo: SimpleN assert run_gate(shell("git config core.hooksPath /dev/null", cwd=other), repo.repos) is None +def _init_independent_repo(path: Path) -> None: + """A real, independent repository: its own objects, its own config, its own git dir.""" + subprocess.run(["git", "init", "-b", "main", str(path)], check=True, capture_output=True) + for key, value in (("user.email", "t@example.com"), ("user.name", "t")): + subprocess.run( + ["git", "config", key, value], cwd=str(path), check=True, capture_output=True + ) + + +@pytest.fixture +def vendored(repo: SimpleNamespace) -> Path: + """An independent clone living UNDER the governed root -- sharing its path and nothing else.""" + path = repo.primary / "vendor" / "thirdparty" + path.mkdir(parents=True) + _init_independent_repo(path) + return path + + +def test_a_repo_VENDORED_UNDER_a_governed_root_is_not_governed_by_it( + repo: SimpleNamespace, vendored: Path +) -> None: + """BACKLOG #1067. Governance is repository IDENTITY, not a path prefix. + + An independent clone living under a governed root shares nothing with it but its path, so a disarm + write there cannot reach the governed repo's config. The committed gate compared the TARGET's common + dir against the root's WORKING TREE path, so every repository under that root inherited its + governance -- and the refusal went on to assert a shared ``.git`` the vendored clone does not have. + A refusal that misdescribes what it blocked teaches people to route around the gate. + """ + assert run_gate(shell("git config core.hooksPath /dev/null", cwd=vendored), repo.repos) is None + + +def test_a_vendored_repo_is_ungoverned_when_named_by_an_ABSOLUTE_PATH_too( + repo: SimpleNamespace, vendored: Path +) -> None: + """The same defect through the other door, and the reason this is a second row rather than a second + assertion: ``git -C `` is judged from the PRIMARY's cwd, so a fix that consulted only the + session's own repository would leave this spelling denying while the row above went green.""" + command = f'git -C "{vendored}" config core.hooksPath /dev/null' + assert run_gate(shell(command, cwd=repo.primary), repo.repos) is None + + +def test_a_worktree_NESTED_under_the_governed_root_still_denies(repo: SimpleNamespace) -> None: + """The control that makes the two rows above a fix and not a hole. + + A tree under ``.claude/worktrees/`` has the SAME PATH SHAPE as the vendored clone and the opposite + right answer, because its common dir really is the primary's. Path shape cannot tell them apart; + repository identity can. Without this row the item's fix is indistinguishable from deleting rule 3c's + governance test altogether. + """ + nested = repo.primary / ".claude" / "worktrees" / "wt-nested" + nested.parent.mkdir(parents=True) + subprocess.run( + ["git", "worktree", "add", "-b", "nested-branch", str(nested)], + cwd=str(repo.primary), + check=True, + capture_output=True, + ) + reason = assert_denied( + run_gate(shell("git config core.hooksPath /dev/null", cwd=nested), repo.repos) + ) + assert "SHARED git configuration" in reason + + +def test_a_SUBMODULE_of_a_governed_root_still_denies(repo: SimpleNamespace, tmp_path: Path) -> None: + """Pins the answer this item deliberately did NOT decide. + + A submodule's git dir is ``/.git/modules/``, so an identity-ONLY predicate would flip + it from DENY to ALLOW as a side effect of fixing the vendored case. Comparing equality-or-UNDER the + root's own common dir leaves it exactly where it was. Whether submodules SHOULD be governed is its + own question, and this row exists so that answering it has to be a decision. + """ + upstream = tmp_path / "Upstream" + _init_independent_repo(upstream) + (upstream / "f.txt").write_text("x\n", encoding="utf-8") + subprocess.run(["git", "add", "-A"], cwd=str(upstream), check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "s"], cwd=str(upstream), check=True, capture_output=True) + added = subprocess.run( + [ + "git", + "-c", + "protocol.file.allow=always", + "submodule", + "add", + str(upstream).replace("\\", "/"), + "sub", + ], + cwd=str(repo.primary), + capture_output=True, + text=True, + ) + if added.returncode != 0: + pytest.skip(f"git refused a file-protocol submodule here: {added.stderr.strip()[:200]}") + sub = repo.primary / "sub" + reason = assert_denied( + run_gate(shell("git config core.hooksPath /dev/null", cwd=sub), repo.repos) + ) + assert "SHARED git configuration" in reason + + 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."""