diff --git a/.gitignore b/.gitignore index 31328cb6..0c6286cd 100644 --- a/.gitignore +++ b/.gitignore @@ -152,9 +152,10 @@ scripts/security/scan-tokens.local.txt # adds there later, which is ignored by DEFAULT rather than by enumeration. # # WHY settings.json IS TRACKED. It carries ENFORCED controls -- the deny-list covering `.env`, -# `secrets/**`, keys and the local `*.db` store, plus the `block-blanket-git-stage` PreToolUse -# guard -- and settings are enforced by the client where CLAUDE.md is only context, so section 5's -# prose is not a substitute for it. Untracked, it hit the exact CLAUDE.md failure described above: +# `secrets/**`, keys and the local `*.db` store -- and settings are enforced by the client where +# CLAUDE.md is only context, so section 5's prose is not a substitute for it. It does NOT wire +# `block-blanket-git-stage.ps1`: that script is present and tested but referenced by no matcher +# (BACKLOG #1339). Untracked, it hit the exact CLAUDE.md failure described above: # measured 2026-08-13, of 62 local checkouts carrying CLAUDE.md only 12 had `.claude/settings.json`, # so 50 ran with no deny-list and no staging guard. `git worktree add` delivers tracked files only. /.claude/* diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 088c99ae..79e6c341 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -127,20 +127,29 @@ concrete features go in **Issues**; security vulnerabilities go through a Building two changes in parallel? Don't share one checkout — give each its own **git worktree** (`scripts\worktree\new.ps1 -Name `). See [docs/WORKTREES.md](docs/WORKTREES.md). -### If you use Claude Code: this repo ships two hooks +### If you use Claude Code: what the tracked settings file actually wires [`.claude/settings.json`](.claude/settings.json) is **tracked**, so cloning this repo configures Claude Code, and you should read it before you trust it. It is the only tracked file under `.claude/`; everything else there is session state and stays ignored. -- **It wires two PowerShell scripts to run automatically.** - [`scripts/hooks/block-blanket-git-stage.ps1`](scripts/hooks/block-blanket-git-stage.ps1) runs - before any git command the agent issues, and - [`scripts/worktree/session-context.ps1`](scripts/worktree/session-context.ps1) runs at session - start. Both are in-repo, reviewable, and covered by the same review as any other script here. -- **They need PowerShell 7 (`pwsh`).** A hook that cannot start is **non-blocking** — the action - proceeds and you get a notice, not a refusal. So on a machine without `pwsh` the staging guard is - absent rather than failing loudly. Do not treat it as coverage you can rely on; the leak gate +- **It wires one PowerShell script to run automatically**, and the count is asserted by a test + rather than by this sentence: + [`scripts/hooks/seat-declare-prompt.ps1`](scripts/hooks/seat-declare-prompt.ps1) at session + start. It is in-repo, reviewable, and covered by the same review as any other script here. +- **The repo contains other hook scripts that this file does NOT wire, and the difference matters.** + [`scripts/worktree/session-context.ps1`](scripts/worktree/session-context.ps1) is wired at + **user** level by [`scripts/coord/install-coordination.ps1`](scripts/coord/install-coordination.ps1), + which the maintainer runs from a plain terminal — so it is live on a configured box and absent on + a fresh clone. + [`scripts/hooks/block-blanket-git-stage.ps1`](scripts/hooks/block-blanket-git-stage.ps1) is + **present and fully tested but wired nowhere at all**, and must not be counted as a control until + it is (BACKLOG #1339). `tests/test_claude_settings_contract.py` asserts that every script under + `scripts/hooks/` is either wired or named, with its reason, on an explicit unwired list — so this + paragraph cannot drift from the code without a red test. +- **Hooks need PowerShell 7 (`pwsh`).** A hook that cannot start is **non-blocking** — the action + proceeds and you get a notice, not a refusal. So on a machine without `pwsh` a guard is absent + rather than failing loudly. Do not treat any of them as coverage you can rely on; the leak gate above is the control that fails closed. - **The deny rules cover the directory you started the agent in.** They keep `.env`, `secrets/`, keys and the local `*.db` store away from the agent's file tools at any depth *below that diff --git a/docs/Secure_AI_Development_Standards.md b/docs/Secure_AI_Development_Standards.md index ba9c5717..be73fa16 100644 --- a/docs/Secure_AI_Development_Standards.md +++ b/docs/Secure_AI_Development_Standards.md @@ -302,21 +302,35 @@ python -m messagefoundry check # exit-coded validate + dryrun, reused by git-h **Deterministic guardrails the model cannot bypass** — the `.claude/settings.json` deny-list and the PreToolUse hook: ```json -// .claude/settings.json — deny-list + PreToolUse wiring (excerpt — abridged; the real file has more denies + timeout/statusMessage) +// .claude/settings.json — deny-list (excerpt — abridged; the real file carries 27 deny rules) "deny": [ - "Read(./.env)", "Read(./.env.*)", "Read(./secrets/**)", - "Read(./*.key)", "Read(./*.pem)", "Read(./*.pfx)", "Read(./*.db)", - "Edit(./secrets/**)", "Write(./secrets/**)", + "Read(.env)", "Read(.env.*)", "Read(secrets/**)", + "Read(*.key)", "Read(*.pem)", "Read(*.pfx)", "Read(*.db)", + "Edit(secrets/**)", "Write(secrets/**)", "Bash(rm -rf:*)", "Bash(git push --force:*)", "Bash(git reset --hard:*)" -], -"PreToolUse": [ - { "matcher": "Bash", "hooks": [ { "type": "command", "if": "Bash(git *)", - "command": "pwsh -NoProfile -File scripts/hooks/block-blanket-git-stage.ps1" } ] }, - { "matcher": "PowerShell", "hooks": [ { "type": "command", "if": "PowerShell(git *)", - "command": "pwsh -NoProfile -File scripts/hooks/block-blanket-git-stage.ps1" } ] } ] ``` +> **THIS EXCERPT WAS WRONG IN THREE MEASURED WAYS AND A READER WHO APPLIED IT PRODUCED A +> CONFIGURATION THIS REPO'S OWN SUITE FAILS.** Corrected 2026-08-25 under BACKLOG #1339; recorded +> rather than silently fixed, because the failure mode is the point of SDS-3.7. +> +> 1. **Every deny rule was `./`-anchored** (`Read(./.env)`). Bare patterns follow gitignore +> semantics and match at **any depth**; the `./` form matches one directory and is strictly +> narrower, which is the worst combination for a control whose whole job is to be broad. +> `tests/test_claude_settings_contract.py::test_no_deny_rule_uses_the_narrow_dot_anchor` +> asserts **zero** such rules, and the tracked file has zero of 27. +> 2. **The hook path was bare** (`scripts/hooks/...`), which resolves against the session's working +> directory rather than the repo. `test_every_hook_resolves_through_the_project_dir_placeholder` +> requires `${CLAUDE_PROJECT_DIR}` in exec form. *(This one was not in the original report; it +> was found while correcting the other two.)* +> 3. **The `"if"` key appears in 0 of 9 settings files** on this machine and is not a key this +> schema carries. +> +> **The PreToolUse block is removed rather than corrected, because it documented wiring that does +> not exist.** `block-blanket-git-stage.ps1` is present and fully tested but is referenced by **no +> matcher in any settings file** — see the note under the guardrail tables below. + > **Transferable principle — fail-OPEN by design.** The git-staging guard blocks *blanket* staging (`git add -A`/`.`) so the human curates each commit; if the guard itself errors it lets the command **through** (fail-open) rather than wedging the workflow — a deliberate tradeoff for a *workflow* guard. (Contrast the engine's *fail-closed* bind guard for a *security* boundary.) **The blocking security CI** ([`.github/workflows/security.yml`](../.github/workflows/security.yml)) — **bandit** (SAST), **pip-audit** (SCA, hash-locked), **gitleaks** (secret scan), **semgrep** (project rules, [`.semgrep/messagefoundry.yml`](../.semgrep/messagefoundry.yml)), **crypto-inventory**, and **forbidden-content** — is **BLOCKING**: bandit/gitleaks/semgrep/pip-audit started from a clean baseline so a regression turns CI **red**, and crypto-inventory/forbidden-content fail the build on any inventory drift or forbidden-string hit. The CycloneDX **SBOM** job is **advisory** (`continue-on-error: true` — it fails only if the bill of materials cannot render), not a blocking gate. Separately, the maintainer runs the **`/security-review`** and **`/code-review`** Claude Code skills on the diff as **advisory** reviews — local, human-invoked AI reviews the human arbitrates, **never** a deterministic CI gate (§7). @@ -445,13 +459,33 @@ This standard governs *how* to use AI at each tier; it **does not mandate** usin | Claude Code primitive | Control it implements | Guardrail type | Live MEFOR file | |---|---|---|---| | `.claude/settings.json` deny-list | No secrets/keys/`*.db`/`.env` to the assistant | **Deterministic gate** | [`.claude/settings.json`](../.claude/settings.json) | -| PreToolUse hook | Block blanket git staging (fail-open) | **Deterministic gate** | [`scripts/hooks/block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) | +| PreToolUse hook | Block blanket git staging (fail-open) | **PRESENT, NOT WIRED** -- see the note below this table | [`scripts/hooks/block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) | | SessionStart hook | Inject worktree/branch context | **Context** | [`scripts/worktree/session-context.ps1`](../scripts/worktree/session-context.ps1) | | Blocking security CI | SAST/SCA/secret-scan/forbidden-content | **Deterministic gate** | [`.github/workflows/security.yml`](../.github/workflows/security.yml) | | `messagefoundry check` | Validate + dryrun, exit-coded | **Deterministic gate** | [`messagefoundry/checks.py`](../messagefoundry/checks.py) | | `CLAUDE.md` | Standing contract / invariants | **Context** | [`../CLAUDE.md`](../CLAUDE.md) | | Plan mode, `/code-review`, `/security-review` | Plan approval, diff review | **Advisory** (human arbitrates) | — (Claude Code feature) | +> **NOTE ON THE PreToolUse ROW -- THE FILE EXISTS AND THE CONTROL DOES NOT RUN (BACKLOG #1339).** +> `block-blanket-git-stage.ps1` is written, reviewable and thoroughly tested, and is referenced by +> **no PreToolUse matcher in any settings file** -- measured zero across every settings file on a +> fully configured machine, against a positive control (`collision_gate` and `worktree_gate`, wired +> at user level by [`install-coordination.ps1`](../scripts/coord/install-coordination.ps1) and +> [`install-gate.ps1`](../scripts/worktree/install-gate.ps1), return non-zero by the same probe). +> +> **The gap is not "does not reach a fresh clone" -- that was BACKLOG #327 and its fix shipped** +> when `.claude/settings.json` became tracked. This is the different finding underneath it: nobody +> ever added the matcher. **A control described in a table as built, that runs in no session, is a +> compensating control resting on a false premise -- the defect SDS-3.7 names.** +> +> **What stops this recurring is an instrument, not this paragraph.** Three earlier prose +> corrections on this same claim each landed a new false statement. +> `tests/test_claude_settings_contract.py` now asserts that every script under `scripts/hooks/` is +> either wired, or wired by a tracked installer, or named with its reason on an explicit unwired +> list -- so the next drift is a red test rather than a sentence somebody has to notice. **Owner +> ruling 2026-08-25: the guard IS a control and is to be wired after the quote-state splitter +> repair** (BACKLOG #1341); this row becomes *Deterministic gate* at that point and not before. + This document **owns and expands** the SDS §A.6 line — *"AI-assisted review as a compensating control"* — for the solo-maintainer **PO.2 / PW.7** deviation. The detailed record is Appendix A.6. --- @@ -480,7 +514,7 @@ The repo's tiered-honesty taxonomy, applied to the **dev-process tooling itself* **Built (in code today):** -- PreToolUse [`block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) (Bash + PowerShell, fail-open). +- **NOT a live control:** [`block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) is written and fully tested but is wired by **no** PreToolUse matcher in any settings file (BACKLOG #1339). It is listed here so it is not counted twice: the file exists, the control does not run. - [`.claude/settings.json`](../.claude/settings.json) secrets/keys/`*.db` **path-based** deny-list + destructive-command denies. - Blocking security CI: bandit, semgrep ([`.semgrep/messagefoundry.yml`](../.semgrep/messagefoundry.yml)), pip-audit, gitleaks, crypto-inventory, forbidden-content ([`scripts/security/scan_forbidden.py`](../scripts/security/scan_forbidden.py)). The CycloneDX **SBOM** job is **advisory** (`continue-on-error`), not blocking. - **Dependency-CVE fast response (SSDF RV.2 evidence):** dependency-vulnerability metrics ([`vuln-metrics.yml`](../.github/workflows/vuln-metrics.yml)), scoped Dependabot auto-merge + supply-chain cooldown ([`dependabot-auto-merge.yml`](../.github/workflows/dependabot-auto-merge.yml)), auto lock-resync ([`dependabot-lock-resync.yml`](../.github/workflows/dependabot-lock-resync.yml)), and the adopter vulnerable-pin CI tripwire. The **`CI gate` roll-up** required check gates the conditional/matrix legs in [`ci.yml`](../.github/workflows/ci.yml). *(This is the **audit/known-CVE** posture. The distinct hallucinated/typosquatted **new-dependency-introduction** check is now built alongside it — [`new_dependency_check.py`](../scripts/security/new_dependency_check.py), a step in the same required `pip-audit` job — with a documented residual; see below.)* @@ -565,8 +599,8 @@ MEFOR is an open-source HL7 v2.x integration engine (Python; FastAPI; SQLite/WAL | Guardrail | File | Tag | |---|---|---| -| Secrets/keys/`*.db` deny-list + destructive-cmd denies; PreToolUse + SessionStart hooks | [`.claude/settings.json`](../.claude/settings.json) | Built | -| Blanket-git-stage guard (fail-open) | [`scripts/hooks/block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) | Built | +| Secrets/keys/`*.db` deny-list + destructive-cmd denies; SessionStart hook | [`.claude/settings.json`](../.claude/settings.json) | Built | +| Blanket-git-stage guard (fail-open) | [`scripts/hooks/block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) | **Written, NOT wired** (BACKLOG #1339) | | Worktree context + isolation | [`scripts/worktree/session-context.ps1`](../scripts/worktree/session-context.ps1), [`new.ps1`/`remove.ps1`](../scripts/worktree/) | Built | | Exit-coded validate + dryrun gate | [`messagefoundry/checks.py`](../messagefoundry/checks.py) | Built | | Blocking SAST/SCA/secret-scan/crypto-inventory/forbidden-content (+ advisory SBOM) | [`.github/workflows/security.yml`](../.github/workflows/security.yml) | Built | diff --git a/scripts/hooks/block-blanket-git-stage.ps1 b/scripts/hooks/block-blanket-git-stage.ps1 index 86a9a575..b0800ef2 100644 --- a/scripts/hooks/block-blanket-git-stage.ps1 +++ b/scripts/hooks/block-blanket-git-stage.ps1 @@ -5,11 +5,114 @@ # command does a broad stage (git add -A/--all/-u/. or git commit -a/-am/--all) it returns a # PreToolUse "deny" decision asking for explicit paths. Anything else passes silently (exit 0). # Fail-OPEN on any error: a guardrail must never wedge all git work. -# Wired in .claude/settings.json (PreToolUse, Bash + PowerShell). See docs/WORKTREES.md. +# +# WIRING IS NOT ASSERTED HERE ON PURPOSE. Whether this script is referenced by a PreToolUse matcher +# is a property of settings.json, not of this file, and a comment claiming otherwise cannot be +# checked. tests/test_claude_settings_contract.py holds that assertion where it can fail. +# See docs/WORKTREES.md. # ASCII-only on purpose (PS 5.1 ANSI-read lesson); run under pwsh 7 by the hook. $ErrorActionPreference = 'SilentlyContinue' +# --------------------------------------------------------------------------------------------- +# WHY THIS FILE HAS FUNCTIONS NOW, AND WHY THEY ARE LOCAL (BACKLOG #1341) +# +# The old scan split on '(\|\||&&|[;|&\n])' and then matched the subcommand and flag tokens +# ANYWHERE in a segment. Both halves were wrong in the SAME direction -- toward denying: +# * a separator inside a quoted span split the command, so quoted prose landed at a segment +# front and was read there as a program name; +# * 'add' appearing as an ARGUMENT ('git grep -n add -- .') was read as the subcommand. +# +# The sibling worktree_gate.ps1 has a quote-blanking pass (Remove-QuotedSpans) that solves the +# first half. It is NOT dot-sourced here, deliberately: BACKLOG #1332 is rewriting that tokeniser +# right now, and sharing a seam under two concurrent lanes costs more than a local copy. This is +# written against its SHAPE, not copied from it. When #1332 settles, this should be replaced by +# the shared helper and deleted -- it is one function with one caller for exactly that reason. +# +# THE POLARITY RULE, AND IT IS LOAD-BEARING (BACKLOG #1229's reverted experiment). +# A program-position predicate was built for worktree_gate.ps1 and withdrawn six hours later. It +# put an ALLOWLIST of transparent wrapper words on the path to a deny: a name it did not know +# ended the chain, and an ended chain meant no verb, and no verb meant ALLOW. At least 11 of 21 +# measured dispatch prefixes flipped DENY to ALLOW that way -- 'cmd /c', 'pwsh -File', a +# PowerShell dot-source, 'source', an unlisted wrapper. Its own docstring priced the risk +# backwards, reasoning about a name wrongly ADDED when the failure mode is a name MISSING. +# +# So: RECOGNITION MAY ONLY EVER SUPPRESS A DENY, NEVER BE REQUIRED TO PRODUCE ONE. The only +# construct below that can turn a deny into an allow is $READ_ONLY_SUBCOMMANDS, and a name +# missing from it costs a FALSE DENY -- noisy, visible, self-reporting -- never a silent hole. +# Every unrecognised shape falls through to the old substring predicates, which deny. +# --------------------------------------------------------------------------------------------- + +# Blank the BODY of every heredoc, preserving line structure. A heredoc body is data being +# written to a file, not a command, but its lines sit at the front of a newline-split segment and +# are read there as program position. Handles < add -A' resolving its subcommand to . An option missing from this + # list only ever mis-resolves toward a non-subcommand, which falls through to a deny. + if ($t -cmatch '^(-C|-c|--git-dir|--work-tree|--namespace|--exec-path|--super-prefix)$') { $i++ } + } + return $null +} + $cmd = $null try { $raw = [Console]::In.ReadToEnd() @@ -22,28 +125,125 @@ try { } if ([string]::IsNullOrWhiteSpace($cmd)) { exit 0 } +# Scan the BLANKED form. Quoted spans and heredoc bodies are data; everything outside them keeps +# its exact offsets, so a real command is unchanged by this pass. +$scan = Hide-QuotedSpans (Hide-HeredocBodies $cmd) + $reason = $null # Examine each shell-separated simple command on its own, so '... && git add -A' is still caught. -foreach ($seg in [regex]::Split($cmd, '(\|\||&&|[;|&\n])')) { - $s = $seg.Trim() +# +# TWO VIEWS OF THE SAME SEGMENT, AND THE SPLIT BETWEEN THEM IS DELIBERATE. Both Hide- passes above +# preserve LENGTH exactly, so an offset into $scan is the same offset into $cmd. The blanked view +# answers "where does a command start, and is this git" -- questions where quoted text must not +# count. The raw view answers "what are this command's arguments" -- where a quoted token is an +# ordinary argument and blanking it would hide a real pathspec. `git add ':(top)'` needs the raw +# view; `git commit -m "wip; git add -a"` needs the blanked one. The resolved SUBCOMMAND is what +# selects between them, below. +$bounds = New-Object 'System.Collections.Generic.List[int[]]' +$cursor = 0 +foreach ($m in [regex]::Matches($scan, '(\|\||&&|[;|&\n])')) { + $bounds.Add(@($cursor, ($m.Index - $cursor))) + $cursor = $m.Index + $m.Length +} +$bounds.Add(@($cursor, ($scan.Length - $cursor))) + +foreach ($b in $bounds) { + if ($b[1] -le 0) { continue } + $s = $scan.Substring($b[0], $b[1]).Trim() + $rawSeg = $cmd.Substring($b[0], $b[1]).Trim() # The PROGRAM NAME is matched case-INSENSITIVELY, and only it. Windows resolves git, Git and # GIT to the same git.exe, so 'Git add -A' staged the tree while 'git add -A' was denied. The # subcommand and flag tests below stay -cmatch on purpose: git rejects 'git ADD', and '-A' and # '-a' are different flags. # - # '^' pins this to the front of a SEGMENT, which is NOT the same as program position -- the - # splitter above carries no quote or line state, so quoted text after a newline, ';', '|' or - # '&' also lands at a segment front. That makes prose quoting a blanket-stage command deny. - # The class is pre-existing: every measured case has a lowercase twin that already denied, so - # this widens it from one spelling to all rather than creating it. What it costs, and what - # would actually fix it, are recorded ONCE in tests/test_blanket_stage_guard.py. Read that - # before widening this line further; the sibling worktree_gate.ps1 had a case fix rejected - # over this same class (BACKLOG #1305). - if ($s -inotmatch '^git(\s|$)') { continue } - - # git add with -A / --all / -u / a bare '.' (stages the whole tree). - if ($s -cmatch '\badd\b' -and $s -cmatch '(^|\s)(-A|--all|-u|\.)(\s|$)') { - $reason = "git add -A/--all/-u/. stages everything, including files another session may be editing." + # '^' pins this to the front of a SEGMENT. That now IS program position for the cases this + # guard covers, because the splitter above no longer breaks inside quoted spans or heredoc + # bodies. A command reached through a dispatching wrapper ('cmd /c "git add -A"') is still + # not covered -- that is BACKLOG #1305's axis, on a different file, and deliberately not + # widened here: doing so needs a wrapper allowlist, which is the construct #1229 proved + # fails open. + # `git.exe` is the SAME executable spelled with its extension, and it was one of the seven + # measured bypasses. Only the BARE spelling is covered: a path-qualified or wrapper-dispatched + # git ('C:/.../git.exe add -A', 'env git add -A') is BACKLOG #1305's axis and is left alone, + # because closing it needs a wrapper allowlist -- the construct BACKLOG #1229 measured as + # fail-open on the sibling gate. + if ($s -inotmatch '^git(\.exe)?(\s|$)') { continue } + + $tokens = @($s -split '\s+' | Where-Object { $_ }) + + # THE ONE SUPPRESSION. A recognised read-only subcommand cannot stage, so 'git grep -n add' + # and 'git log --all --grep commit' allow. Anything unrecognised -- including $null -- falls + # through to the predicates below unchanged. + $sub = Resolve-GitSubcommand $tokens + if ($null -ne $sub -and $READ_ONLY_SUBCOMMANDS -ccontains $sub) { continue } + + # LIMB 1 -- the SUBCOMMAND, including the synonym. `stage` is documented and dispatches to the + # same builtin (`git stage -h` prints `usage: git add`), so testing only `add` let every flag + # and pathspec row below be defeated by one word. + # + # WHICH VIEW THE ARGUMENTS ARE READ FROM IS DECIDED HERE, and it is decided by the RESOLVED + # subcommand rather than by searching for the word. When the subcommand IS add/stage, every + # non-flag argument is a pathspec, so the raw view is correct and safe -- `git add` has no + # message flag whose quoted value could be mistaken for one. When it is anything else, the + # blanked view is correct: `git commit -m "wip; git add -a"` must not be read as a stage. + # A subcommand that does not resolve falls back to the ORIGINAL token search on the blanked + # view, which preserves every deny this guard made before. + $argView = if ($sub -ceq 'add' -or $sub -ceq 'stage') { $rawSeg } else { $s } + $staging = ($sub -ceq 'add' -or $sub -ceq 'stage') -or + ($null -eq $sub -and $s -cmatch '(^|\s)(?:add|stage)(\s|$)') -or + ($null -ne $sub -and $sub -cne 'commit' -and $s -cmatch '(^|\s)(?:add|stage)(\s|$)') + + # LIMB 2 -- the blanket-stage FLAG family, GENERATED from the option words rather than typed, + # per the method BACKLOG #1097 settled for worktree_gate.ps1's interpreter flag. A longer list + # has the same shape as the defect and decays the same way. + # + # WHY A LADDER AND NOT TWO SPELLINGS: git's parse-options binds a long option by any + # UNAMBIGUOUS ABBREVIATION, so `--a`, `--al`, `--up`, `--upd` and `--updat` all stage the tree. + # Generating an ambiguous rung costs nothing -- git refuses it, and a command git refuses is + # not a command to protect. + $blanketWords = @('all', 'update', 'no-ignore-removal') + $longNames = @( + $blanketWords | ForEach-Object { $w = $_; 1..$w.Length | ForEach-Object { $w.Substring(0, $_) } } + ) | Sort-Object -Property Length -Descending # longest first: `--all` binds as `all`, not `a`+`ll` + $longFlag = "--(?:$($longNames -join '|'))" + # A short-option CLUSTER, the same construction the commit branch below already used. The + # trigger set is EXACTLY {A, u} and CASE-SENSITIVE, and the case is the whole bound: + # -A --all stages everything MUST trigger + # -u --update stages every tracked change MUST trigger + # -a NOT a git add flag; `git add -a` exits 129 `unknown switch`. Folding case + # here would deny commands git itself refuses: pure false-deny, zero gain. + # -U --unified IS a real flag and stages nothing. Matching it denies legitimate work. + # `-a` and `-U` are pinned as ALLOW tests so the next reader cannot "simplify" this to (?i)[au]. + $shortFlag = '-[A-Za-z]*[Au][A-Za-z]*' + $blanketFlag = "(?:$longFlag|$shortFlag)" + + # LIMB 3 -- a whole-tree PATHSPEC, kept a SEPARATE limb from the flags on purpose. A flag can be + # anchored on a leading `-`; a pathspec cannot, and the only thing separating the blanket + # `git add ./` from the scoped `git add ./src/x.py` is the trailing boundary. One fused rule + # would need one boundary to satisfy both tests and would be wrong for one of them. The two + # also earn different deny messages: telling an operator who typed `:/` that the problem was a + # flag is the wrong sentence. + # AT LEAST these four -- this is not an enumeration of git's pathspec grammar. + # QUOTES ARE TOLERATED AROUND THE WHOLE TOKEN because `:(top)` cannot be typed unquoted in + # either shell -- the parentheses are syntax. The token must still be EXACTLY the pathspec: + # `git add './src/x.py'` stays allowed, because the trailing boundary is outside the quote. + $treeRoot = '["'']?(?:\.|\./|:/|:\(top\))["'']?' + + # WHAT THESE THREE LIMBS DELIBERATELY DO NOT REACH, stated beside the rule per CLAUDE.md + # section 11 rather than left for a reader to discover: + # --renormalize implies -u; `git add --renormalize .` stages tracked changes + # --pathspec-from-file=- the pathspec is on stdin, so no pathspec appears in the command + # :^x / :!x / :(glob) / * the rest of magic pathspec; an exclude-only spec is a blanket + # stage spelled as an exclusion + # any dispatching wrapper `cmd /c "git add -A"`, `env git add -A`, a path-qualified git. + # That is BACKLOG #1305's axis and needs a wrapper allowlist, the + # construct BACKLOG #1229 measured as fail-open. + if ($staging -and $argView -cmatch "(^|\s)$blanketFlag(\s|$)") { + $reason = "git add/stage -A/--all/-u/--update (or a cluster containing A or u) stages everything, including files another session may be editing." + break + } + if ($staging -and $argView -cmatch "(^|\s)$treeRoot(\s|$)") { + $reason = "git add/stage with a whole-tree pathspec (. ./ :/ :(top)) stages everything, including files another session may be editing." break } # git commit with -a / -am / --all (auto-stages every tracked change). A single-dash flag diff --git a/tests/test_announce_hook.py b/tests/test_announce_hook.py index efa56ee4..287f2dc4 100644 --- a/tests/test_announce_hook.py +++ b/tests/test_announce_hook.py @@ -856,9 +856,14 @@ def test_selftest_does_not_read_stdin(repo: Path, tmp_path: Path) -> None: def test_two_concurrent_runs_announce_once(repo: Path, tmp_path: Path) -> None: - """session-context.ps1 is registered TWICE on this box today and block-blanket-git-stage twice in - the project file, so double firing is a live pattern; lock.ps1 records PowerShell silently losing - 4 of 8 concurrent writes.""" + """session-context.ps1 is registered TWICE on this box today, so double firing is a live + pattern; lock.ps1 records PowerShell silently losing 4 of 8 concurrent writes. + + CORRECTED (BACKLOG #1339): this used to add "and block-blanket-git-stage twice in the project + file". That is FALSE -- that script is referenced ZERO times, in the project file and in every + other settings file. The assertion below was never affected; only this rationale was wrong, + which is the harder kind to notice because nothing goes red. + """ sd = tmp_path / "state" presence = presence_stub(tmp_path, [SELF_ROW, PEER]) with concurrent.futures.ThreadPoolExecutor(max_workers=2) as ex: diff --git a/tests/test_blanket_stage_guard.py b/tests/test_blanket_stage_guard.py index 0fd9ae88..1cb8c832 100644 --- a/tests/test_blanket_stage_guard.py +++ b/tests/test_blanket_stage_guard.py @@ -16,21 +16,30 @@ `git add -a` returns "error: unknown switch `a'", and `git commit --ALL` returns "error: unknown option `ALL'". Folding case there would deny commands git itself refuses to run. -KNOWN OVER-DENY, AND THE CASE FIX WIDENED IT. The guard splits on `(\|\||&&|[;|&\n])`, which -carries no quote or line state, so quoted text after a newline, `;`, `|` or `&` lands at the front -of a segment and is read there as a program name. Prose that quotes a blanket-stage command is -denied on that path: a heredoc writing a doc, a commit message body, a `gh pr create --body`, even -a single-line markdown table cell. Two non-prose commands go the same way -- `git log --all --grep -commit` and `git grep -n add -- .` are read-only and both deny -- because the subcommand and flag -tokens are matched ANYWHERE in the segment rather than at argv position. - -THE CLASS IS PRE-EXISTING, WHICH IS WHY THE CASE FIX STILL LANDED. Every case below was driven -against the committed guard in its lowercase spelling first, and every one already denied. The fix -widens the class from one spelling to all of them; it creates no new class, and across roughly 800 -driven payloads nothing flipped from DENY to ALLOW. The real repair is a program-position test over -a quote-aware splitter -- `Test-GitProgramPosition` exists at `c0d6cef8^` and was removed by the -revert at `c0d6cef8` -- which belongs to the shared segment-scanner work and has no allocated -number here. The cases are pinned below so that work flips them deliberately. +THE OVER-DENY CLASS THIS FILE USED TO PIN IS NOW FIXED (BACKLOG #1341). The guard split on +`(\|\||&&|[;|&\n])`, which carried no quote or line state, so quoted text after a newline, `;`, `|` +or `&` landed at the front of a segment and was read there as a program name -- a heredoc writing a +doc, a commit message body, a `gh pr create --body`, a markdown table cell. Two non-prose commands +went the same way, `git log --all --grep commit` and `git grep -n add -- .`, because the subcommand +and flag tokens were matched ANYWHERE in a segment rather than at argv position. The guard now +blanks quoted spans and heredoc bodies before splitting, and resolves the subcommand past git's +global options. Those twelve payloads are still driven, under +`test_prose_and_read_only_commands_are_allowed`, with the opposite expectation. + +WHY THE HISTORY IS KEPT RATHER THAN DELETED. The class was PRE-EXISTING and the case fix only +widened it from one spelling to all of them -- every case was driven in its lowercase spelling +first and already denied. That is what made the case fix landable while a known over-deny sat +beside it, and a future reader deciding whether a similar trade is acceptable needs the precedent, +not just the outcome. + +THE ADD VOCABULARY IS A GENERATED FAMILY, NOT A LIST (BACKLOG #1340). Seven forms were filed; a +ground-truth pass measured at least 33 that really stage the whole tree, so patching literals would +have fixed almost nothing. The flag rule is generated from the option words per the method BACKLOG +#1097 settled, because a longer list has the same shape as the defect and decays the same way. +WHAT IS STILL NOT REACHED is stated in the guard beside the rules rather than left to be +discovered: magic pathspec beyond `.`, `./`, `:/` and `:(top)`; `--renormalize`; +`--pathspec-from-file`; and any wrapper-dispatched or path-qualified git, which is BACKLOG #1305's +axis and needs the allowlist construct BACKLOG #1229 measured as fail-open. A MEASUREMENT THAT DID NOT ANSWER THIS QUESTION, recorded so it is not repeated. A scan of every tracked file found 1 segment that already trips the guard and 0 that the case fix newly trips. The @@ -194,24 +203,50 @@ def test_the_anchor_holds_so_a_word_ending_in_git_is_not_the_program() -> None: # ------------------------------------------------------------------------------ the deny message -def test_the_add_deny_names_the_flag_and_the_way_forward() -> None: +def test_the_flag_deny_names_the_flag_family_and_the_way_forward() -> None: + """The message changed with BACKLOG #1340, and the change is the point. + + It used to read `-A/--all/-u/.` -- one sentence covering flags AND a pathspec. That told an + operator who typed `:/` the problem was a flag, and it named neither `stage` nor `--update`, + both of which now deny. The flag limb and the pathspec limb carry their own messages. + """ reason = assert_denied(run_guard(bash("GIT add -A"))) - assert "-A/--all/-u/." in reason + assert "add/stage" in reason # the synonym is real; the message must not hide it + assert "--update" in reason assert "git add " in reason # a deny must say how to proceed, not just say no +def test_the_pathspec_deny_names_the_pathspec_not_a_flag() -> None: + reason = assert_denied(run_guard(bash("git add :/"))) + assert "pathspec" in reason + assert "git add " in reason + + def test_the_commit_deny_names_its_own_rule() -> None: reason = assert_denied(run_guard(bash("Git commit -am wip"))) assert "-a/-am/--all" in reason -# -------------------------------------------------------------- the known over-deny, pinned - -# These are WRONG, and they are pinned so a future program-position fix flips them on purpose -# rather than finding them. Each pair asserts the capitalised form the case fix newly denies AND -# the lowercase form that already denied before it -- the pair is the evidence that this widens an -# existing class rather than creating one. See the module docstring. -OVER_DENY_PAIRS = [ +# ------------------------------------------------- the former over-deny class, FIXED (#1341) + +# THESE SIX USED TO DENY AND NOW ALLOW. THE FLIP IS DELIBERATE (BACKLOG #1341). +# +# They were pinned as known-wrong so that whoever repaired the splitter would flip them on purpose +# instead of discovering them. This is that flip. Nothing here is a coverage reduction: each row +# asserts the SAME payload as before, with the opposite expectation, so the case is still driven +# and a regression that re-denies any of them fails this test. +# +# Two mechanisms, and the pairs separate them: +# * rows 1-4 were segmentation. A separator inside a quoted span or a heredoc body split the +# command, so prose landed at a segment front and was read as program position. The guard now +# blanks quoted spans and heredoc bodies before splitting. +# * rows 5-6 were argv position. `add` and `commit` were matched ANYWHERE in a segment, so a +# read-only search whose ARGUMENT was the word `add` denied. The guard now resolves the +# subcommand past git's global options and suppresses on a recognised read-only one. +# +# The capitalised/lowercase pairing is KEPT rather than collapsed. It is what shows the class was +# pre-existing and not created by the case fix, and it costs one extra driven payload per row. +FIXED_FORMER_OVER_DENY_PAIRS = [ pytest.param( 'git commit -m "fix\nGit add -A is blocked"', 'git commit -m "fix\ngit add -A is blocked"', @@ -245,10 +280,162 @@ def test_the_commit_deny_names_its_own_rule() -> None: ] -@pytest.mark.parametrize(("capitalised", "lowercase"), OVER_DENY_PAIRS) -def test_prose_and_read_only_commands_are_over_denied(capitalised: str, lowercase: str) -> None: - assert_denied(run_guard(bash(capitalised))) - assert_denied(run_guard(bash(lowercase))) +@pytest.mark.parametrize(("capitalised", "lowercase"), FIXED_FORMER_OVER_DENY_PAIRS) +def test_prose_and_read_only_commands_are_allowed(capitalised: str, lowercase: str) -> None: + """Prose quoting a blanket-stage command, and read-only searches, must not be refused.""" + assert_allowed(run_guard(bash(capitalised))) + assert_allowed(run_guard(bash(lowercase))) + + +# ------------------------------------------- the add vocabulary, closed as a family (#1340) + +# EVERY ROW HERE WAS MEASURED TO REALLY STAGE THE WHOLE TREE, against real git 2.53.0.windows.2, +# in a throwaway repo, BEFORE being driven through the guard -- and every one was ALLOWED by the +# committed guard. An alleged bypass that does not actually stage anything is not a bypass, so the +# real-git step is what makes these rows evidence rather than assertion. +# +# The item named seven. The ground-truth pass measured at least 33 and stopped searching, not +# because the surface was exhausted. Patching seven literals would have fixed almost nothing -- +# which is precisely why the flag rule below is GENERATED from the option words per BACKLOG #1097's +# settled method, rather than being a longer list. +NEWLY_DENIED_BLANKET_STAGES = [ + # the synonym, which alone defeated every flag row and the bare-dot row together + "git stage -A", + "git stage .", + "git stage --all", + "git stage :/", + "git stage -Av", + "git stage -u", + "git stage --update", + # the long-flag family, including git's unambiguous-abbreviation binding + "git add --update", + "git add --al", + "git add --a", + "git add --up", + "git add --upd", + "git add --no-ignore-removal", + # single-dash clusters + "git add -Av", + "git add -vA", + "git add -uv", + # whole-tree pathspecs + "git add :/", + "git add ./", + "git add ':(top)'", + "git add -f :/", + "git add --update :/", + # the bare .exe spelling of the same executable + "git.exe add -A", + "git.exe commit -am wip", +] + + +@pytest.mark.parametrize("command", NEWLY_DENIED_BLANKET_STAGES) +def test_a_real_blanket_stage_is_denied_however_it_is_spelled(command: str) -> None: + assert_denied(run_guard(bash(command))) + + +# THE NEGATIVE THAT BOUNDS THE FLAG FAMILY. Without these the next reader "simplifies" the +# case-sensitive `[Au]` cluster to `(?i)[au]` and the rule stops describing the family. +# +# `A`/`a` and `u`/`U` are FOUR DIFFERENT THINGS in this one command, and only two stage: +# -a is not a git add flag at all -- `git add -a` exits 129, `unknown switch 'a'` +# -U IS a real flag (--unified) and stages nothing +# Denying either buys zero protection and costs real work. Measured, not read off documentation. +CASE_BOUND_FLAG_ALLOW = [ + "git add -a", # exit 129 in real git; denying it refuses what git already refuses + "git add -na", # same, clustered + "git add -U 3 tracked.txt", # --unified, a real flag that stages nothing + "git add -p", # patch mode, interactive and scoped + "git add -n README.md", # dry run on one path + "git add -N newfile", # intent-to-add, NOT --all despite the capital +] + + +@pytest.mark.parametrize("command", CASE_BOUND_FLAG_ALLOW) +def test_a_flag_that_is_not_a_blanket_stage_is_allowed(command: str) -> None: + assert_allowed(run_guard(bash(command))) + + +# THE PATHSPEC LIMB'S OWN BOUND. A scoped path that merely CONTAINS a dot or a slash is ordinary +# work. The trailing boundary is the only thing separating these from the blanket forms above, +# which is why the pathspec limb cannot be fused into the flag rule. +SCOPED_PATHSPEC_ALLOW = [ + "git add ./src/x.py", + "git add .gitignore", + "git add src/.", + "git add ./sub", + "git add README.md", +] + + +@pytest.mark.parametrize("command", SCOPED_PATHSPEC_ALLOW) +def test_a_scoped_path_is_not_a_whole_tree_pathspec(command: str) -> None: + assert_allowed(run_guard(bash(command))) + + +# ------------------------------------------------- the fix must not have bought a fail-open + +# WHY THIS TEST EXISTS AND WHY IT IS NOT A FALSE-DENY CORPUS (BACKLOG #1229's reverted experiment). +# +# A program-position predicate was built for the sibling worktree_gate.ps1 and withdrawn six hours +# later. Its measurement was 93 rows of "does the shape that should allow, allow?" -- and A +# FALSE-DENY CORPUS CANNOT FIND A FAIL-OPEN BY CONSTRUCTION. It disclosed one fail-open and shipped +# at least ten more it never probed, because every row it drove asked the other question. +# +# The rows below are the other direction: shapes that MUST still be refused. On this guard a +# fail-open is the direction that loses coverage silently, so widening the allow side without this +# test is how the same mistake gets made on a second file. +MUST_STILL_DENY = [ + # The plain forms. If any of these ever allows, the guard is off. + "git add -A", + "git add --all", + "git add -u", + "git add .", + "git commit -a", + "git commit -am wip", + "git commit --all", + # A GLOBAL OPTION BEFORE THE SUBCOMMAND. This is the specific fail-open the subcommand + # resolver could have introduced: if `-C` did not consume its value, the resolver would read + # the PATH as the subcommand, fail to recognise it, and -- in a design where recognition were + # required to keep the token -- allow. It must deny. + "git -C /some/path add -A", + "git -c user.name=x add -A", + "git --git-dir=/tmp/x add -A", + # An UNKNOWN global option must not become an escape hatch either. The resolver cannot know + # whether it takes a value, so the subcommand resolves to something unrecognised -- which must + # fall through to a deny, never to an allow. + "git --some-future-option add -A", + "git --some-future-option value add -A", + # A read-only subcommand NAME appearing as an argument must not suppress a real stage. + "git add -A -- log", + "git add -A -- status", + # Separators outside quotes still split, so a real stage after one is still caught. + "echo hi && git add -A", + "echo hi ; git add -A", + "echo hi | git add -A", + # A quoted argument elsewhere on the line must not hide a real stage outside the quotes. + 'git commit -m "message" && git add -A', + # A heredoc that ENDS before the real command does not blank it. + "cat <<'EOF' > f.txt\nsome body\nEOF\ngit add -A", +] + + +@pytest.mark.parametrize("command", MUST_STILL_DENY) +def test_the_fix_did_not_buy_a_fail_open(command: str) -> None: + assert_denied(run_guard(bash(command))) + + +def test_an_unrecognised_subcommand_falls_through_to_deny_not_allow() -> None: + """The polarity rule, asserted directly rather than left to the rows above. + + Recognition may only ever SUPPRESS a deny. `zzz-not-a-subcommand` is not in the read-only + list and never will be, so a command carrying it must still be judged by the staging + predicates -- which deny. If this ever allows, the resolver has been rewritten so that failing + to recognise something produces an allow, and that is the exact defect that killed the + predicate in BACKLOG #1229. + """ + assert_denied(run_guard(bash("git zzz-not-a-subcommand add -A"))) # --------------------------------------------------------------------------------- still fail-open diff --git a/tests/test_claude_settings_contract.py b/tests/test_claude_settings_contract.py index f35e3c66..0b089a0e 100644 --- a/tests/test_claude_settings_contract.py +++ b/tests/test_claude_settings_contract.py @@ -3,9 +3,17 @@ """`.claude/settings.json` is now a TRACKED control, so its shape gets a test. Tracking the file (see `tests/test_private_paths_stay_ignored.py` for the boundary half) is what -carries the deny-list and the `block-blanket-git-stage` guard to a fresh clone and to every -`git worktree add`. That only buys anything if the payload still works when it arrives, and the two -ways it silently stops working are both invisible to review: +carries the deny-list, and whatever matchers the file wires, to a fresh clone and to every +`git worktree add`. That only buys anything if the payload still works when it arrives, and the +ways it silently stops working are invisible to review: + +CORRECTED, AND THE SENTENCE WAS IN THIS FILE (BACKLOG #1339). This paragraph used to say tracking +the file carries "the deny-list and the `block-blanket-git-stage` guard". It carries the deny-list. +It carried nothing about that guard, because no matcher in it names the script -- so the module +whose job is to catch a control that reads as enforced and is not was itself asserting one. The +third check below is what makes that statement checkable instead of merely rewritten; three earlier +prose corrections on this claim each landed a new false statement, which is why the fix had to be +an instrument. * **A hook that cannot start does not block.** Claude Code's hooks reference is explicit that a command hook which fails to launch "lands in the same non-blocking bucket" and that for most @@ -87,6 +95,72 @@ def _dot_anchored_denies(settings: dict[str, Any]) -> list[str]: return [r for r in settings["permissions"]["deny"] if "(./" in r] +# --------------------------------------------------------- is every hook script wired AT ALL? + +# THE HOLE THIS CLOSES (BACKLOG #1339). Every check above walks `hooks.[].hooks[]` -- that +# is, over REFERENCED scripts. A script referenced by NO handler yields an empty reference list, so +# every assertion passes VACUOUSLY over it. `block-blanket-git-stage.ps1` was referenced by nothing, +# on any settings file, while at least eight tracked sites described it as a live control -- and +# nothing here could see that, because the thing to see was an ABSENCE. +# +# WHY THIS IS THREE STATES AND NOT TWO, WHICH IS THE WHOLE DESIGN. A wired/unwired instrument would +# be WRONG and would assert the exact falsehood this test exists to stop. Six scripts are wired at +# USER level by a TRACKED INSTALLER, not by the tracked settings.json: +# `scripts/coord/install-coordination.ps1` wires five, `scripts/worktree/install-gate.ps1` wires +# `worktree_gate.ps1`. Those are installed and live. Under two states they would all land on the +# "deliberately not wired" list, producing a reviewed record claiming six live hooks are switched +# off. So the second state is MEASURED FROM THE INSTALLERS rather than hand-listed, which is also +# what stops it decaying into another enumeration. +_INSTALLERS = ( + Path("scripts/coord/install-coordination.ps1"), + Path("scripts/worktree/install-gate.ps1"), + Path("scripts/coord/install-git-hooks.ps1"), +) + +# Scripts that are genuinely wired NOWHERE. Each carries its reason, because a bare list is a +# dumping ground and an entry nobody can justify is how this decays back into a false record. +# Keep it SHORT. If it grows, that is the signal, not the workaround. +_KNOWN_UNWIRED: dict[str, str] = { + "block-blanket-git-stage.ps1": ( + "BACKLOG #1339. Present and fully tested, wired nowhere. Owner ruled 2026-08-25 (relayed " + "via the Liaison) that it IS a control and is to be wired AFTER the quote-state splitter " + "repair -- wiring it before that shipped a false-deny class to every seat on every clone, " + "and that friction is what gets a control disarmed. The repair is BACKLOG #1341." + ), + "lane-level.ps1": "Not a PreToolUse guard; invoked directly by coordination scripts.", + "steer-inject.ps1": "Opt-in steering channel, armed per-session rather than by a matcher.", + "steer-send.ps1": "The sending half of the same opt-in channel; never a hook handler.", +} + + +def _hook_scripts() -> list[str]: + return sorted(p.name for p in (_ROOT / "scripts" / "hooks").glob("*.ps1")) + + +def _installer_wired(root: Path | None = None) -> set[str]: + """Hook script basenames a TRACKED installer wires. Measured, never asserted.""" + base = root or _ROOT + wired: set[str] = set() + for rel in _INSTALLERS: + f = base / rel + if not f.is_file(): + continue + for name in _hook_scripts(): + if name in f.read_text(encoding="utf-8"): + wired.add(name) + return wired + + +def _unclassified_hook_scripts(settings: dict[str, Any], root: Path | None = None) -> list[str]: + """Hook scripts that are in NONE of the three states. This is the thing that must be empty.""" + referenced: set[str] = set() + for _event, handler in _hook_handlers(settings): + for ref in _repo_script_refs(handler): + referenced.add(ref.rsplit("/", 1)[-1]) + accounted = referenced | _installer_wired(root) | set(_KNOWN_UNWIRED) + return [n for n in _hook_scripts() if n not in accounted] + + def test_settings_is_valid_json() -> None: """A malformed tracked settings file is a repo-wide outage, not a local one.""" assert _load()["permissions"], "permissions block is missing or empty" @@ -141,6 +215,64 @@ def test_every_hook_script_actually_exists() -> None: ) +def test_every_hook_script_is_wired_or_explicitly_named_as_unwired() -> None: + """A hook script in none of the three states is an UNRECORDED absence, which is the defect. + + This is the assertion the module could not previously make, because every other check walks + the handler lists and therefore cannot see a script no handler names. + """ + unclassified = _unclassified_hook_scripts(_load()) + assert not unclassified, ( + f"{len(unclassified)} hook script(s) are wired nowhere and are not named as unwired: " + f"{unclassified}.\n" + "Either wire it in .claude/settings.json, or add it to _KNOWN_UNWIRED WITH ITS REASON. " + "The point is that an unwired hook is a DECISION somebody made and can defend, not a " + "state the repo drifted into -- BACKLOG #1339 exists because at least eight tracked sites " + "described a control that was wired nowhere, and nothing could see it." + ) + + +def test_the_unwired_list_does_not_name_a_script_that_is_actually_wired() -> None: + """The list must not rot in the other direction either. + + A name left on _KNOWN_UNWIRED after the script gets wired produces a reviewed record asserting + a live control is switched off -- the same false-record defect, pointing the other way. + """ + settings = _load() + referenced = { + ref.rsplit("/", 1)[-1] + for _event, handler in _hook_handlers(settings) + for ref in _repo_script_refs(handler) + } + wired = referenced | _installer_wired() + stale = sorted(set(_KNOWN_UNWIRED) & wired) + assert not stale, ( + f"{len(stale)} script(s) are named as deliberately unwired but ARE wired: {stale}.\n" + "Remove them from _KNOWN_UNWIRED. A list that keeps a wired hook is a record claiming a " + "live control is off." + ) + + +def test_the_unwired_list_only_names_scripts_that_exist() -> None: + missing = sorted(set(_KNOWN_UNWIRED) - set(_hook_scripts())) + assert not missing, ( + f"_KNOWN_UNWIRED names {len(missing)} script(s) that are not in scripts/hooks/: {missing}.\n" + "A renamed or deleted script leaves an entry that silently excuses nothing." + ) + + +def _unclassified_for_planted(settings: dict[str, Any]) -> list[str]: + """Detector shim for the planted-defect row below. + + The planted document is checked against a root with NO installers and NO real hooks directory, + so `_installer_wired` contributes nothing and the only thing accounting for a script is the + settings document itself. That isolates what this row is testing. + """ + return _unclassified_hook_scripts( + settings, root=Path(__file__).resolve().parent / "_nonexistent" + ) + + @pytest.mark.parametrize( ("planted", "checker", "label"), [ @@ -161,8 +293,18 @@ def test_every_hook_script_actually_exists() -> None: _dot_anchored_denies, "dot-anchored deny rule", ), + ( + # A settings document that wires NOTHING, checked against a root with no installers. + # Every real hook script is then unaccounted for except the four on _KNOWN_UNWIRED, so + # the detector must return a non-empty list. If it returns nothing here, it cannot see + # an unwired script at all and the assertion above is passing for the wrong reason -- + # which is exactly the vacuity BACKLOG #1339 is about. + {"permissions": {"deny": []}, "hooks": {}}, + _unclassified_for_planted, + "hook script wired nowhere", + ), ], - ids=["unanchored-hook", "dot-anchored-deny"], + ids=["unanchored-hook", "dot-anchored-deny", "unwired-hook-script"], ) def test_the_checks_can_actually_fail(planted: dict[str, Any], checker: Any, label: str) -> None: """A guard that cannot be shown to fail is not a guard. diff --git a/tests/test_private_paths_stay_ignored.py b/tests/test_private_paths_stay_ignored.py index e8d77147..2abf794f 100644 --- a/tests/test_private_paths_stay_ignored.py +++ b/tests/test_private_paths_stay_ignored.py @@ -46,9 +46,14 @@ # The ONE negated path in the block, and the only tracked file any private rule may cover. # # `/.claude/` became `/.claude/*` plus `!/.claude/settings.json` so the enforced controls -- the -# deny-list and the `block-blanket-git-stage` PreToolUse guard -- reach a fresh clone and every +# deny-list, and whatever matchers the file wires -- reach a fresh clone and every # `git worktree add`, which deliver tracked files only. Before that, this repo's own #327 note -# recorded the guard as one that "does not actually travel" and said not to count it as coverage. +# recorded `block-blanket-git-stage.ps1` as one that "does not actually travel". +# +# #327's CAUSE WAS REMOVED AND ITS SYMPTOM WAS NOT, so do not read this rule as covering that +# guard. Tracking the file fixed "does not reach a fresh clone"; the script is still referenced by +# NO matcher in it, so tracking carries it as a FILE and not as a wired control. Measured and +# asserted in tests/test_claude_settings_contract.py, BACKLOG #1339. # # This is an exact SET, not a floor. Adding a second negation to the block -- `.claude/rules/`, # `.claude/skills/`, an agent definition, anything -- fails here until someone writes it down, and