fix(hooks): blanket-stage guard closes the add-vocabulary and quote-argv gaps (BACKLOG #1339/#1340/#1341) - #579
Merged
Conversation
…d an argument as a subcommand (BACKLOG #1341)
The guard split on '(\|\||&&|[;|&\n])', which carries no quote or line state, and then matched
the subcommand and flag tokens ANYWHERE in a segment. Both halves failed in the same direction --
toward denying -- so a working session was refused for writing prose or running a read-only
search:
git commit -m "wip; git add -A was the trap" the quoted ';' split the command
cat >> docs/X.md <<'EOF' ... git add -A ... EOF the heredoc BODY landed at a segment front
echo "| git add -A | denied |" >> docs/X.md the quoted '|' split the command
git log --all --grep commit 'commit' was the ARGUMENT to --grep
git grep -n add -- . 'add' was the SEARCH PATTERN
Three changes, all local to this file:
Hide-HeredocBodies blanks heredoc bodies, preserving line structure
Hide-QuotedSpans blanks the CONTENTS of quoted spans, preserving length and the quotes
Resolve-GitSubcommand walks past git's global options to the token in subcommand position,
and a recognised read-only subcommand suppresses the staging predicates
POLARITY, AND IT IS THE WHOLE DESIGN. A program-position predicate was built for the sibling
worktree_gate.ps1 and withdrawn six hours later (BACKLOG #1229). It put an ALLOWLIST of
transparent wrapper words on the path to a deny: a name it did not know ended the chain, an ended
chain carried no verb, and no verb meant ALLOW. Measured on the recovered blobs, at least 11 of
21 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 the only construct here that can turn a deny into an allow is the read-only subcommand list,
and a name missing from it costs a FALSE DENY -- visible and self-reporting -- never a silent
hole. Every unrecognised shape, including an unknown global option and an unknown subcommand,
falls through to the original predicates, which deny. Two tests assert that directly.
NOT WIDENED HERE, DELIBERATELY: a stage reached through a dispatching wrapper ('cmd /c "git add
-A"') is still allowed, exactly as before. Closing it needs a wrapper allowlist, which is the
construct #1229 proved fails open. That is BACKLOG #1305's axis, on a different file.
NO SHARED HELPER. worktree_gate.ps1 has a quote-blanking pass (Remove-QuotedSpans) that solves
one half, and this file does not dot-source it: BACKLOG #1332 is rewriting that tokeniser now,
and sharing a seam under two concurrent lanes costs more than a local copy. Written against its
SHAPE, not copied. One function, one caller, so it is cheap to delete when that settles.
TESTS -- COVERAGE IS UP, AND THE ONE APPARENT REMOVAL IS A DELIBERATE FLIP, NOT A LOSS.
test_prose_and_read_only_commands_are_over_denied asserted that all six of these DENY. It was
pinned as known-wrong precisely so that whoever fixed the splitter would flip it on purpose
rather than discover it. This is that flip: the same twelve payloads are still driven, under
test_prose_and_read_only_commands_are_allowed, with the opposite expectation. Nothing stopped
being exercised. The capitalised/lowercase pairing is kept rather than collapsed, because it is
what shows the class was pre-existing and not created by the case fix.
Added 20 cases, 48 -> 68 in this module:
MUST_STILL_DENY, 18 rows, asserting the fix bought no fail-open. This is deliberately NOT a
false-deny corpus -- #1229's experiment measured 93 rows of "does what should allow, allow?"
and shipped at least ten unprobed fail-opens, because that question cannot find one. Includes
'git -C <path> add -A', the specific fail-open the subcommand resolver could have introduced.
test_an_unrecognised_subcommand_falls_through_to_deny_not_allow, asserting the polarity rule.
VERIFIED, and the scope is named rather than counted:
ruff check, ruff format --check, mypy -- clean on tests/test_blanket_stage_guard.py
pytest, 139 passed, over test_blanket_stage_guard.py + the three other modules that reference
this guard (test_claude_settings_contract.py, test_announce_hook.py,
test_private_paths_stay_ignored.py), in the lane venv built to ci.yml's test-leg line
a 104-payload differential driving the origin/main blob and this one side by side:
12 DENY -> ALLOW flips, ALL TWELVE the intended six pairs; 0 elsewhere; 0 ALLOW -> DENY;
0 harness errors. The 14 dispatch-prefix rows are identical old and new, confirming the
#1305 gap is neither closed nor widened.
NOT a full-suite run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ily, not a longer list (BACKLOG #1340)
The guard tested `\badd\b` plus a four-member flag enumeration, so at least 33 command forms that
really stage the whole tree were allowed. The item filed seven; a ground-truth pass drove each
candidate against real git 2.53.0.windows.2 in a throwaway repo BEFORE driving it through the
hook, and stopped searching rather than exhausting the surface. Patching seven literals would have
fixed almost nothing.
Five mechanism classes, three limbs:
LIMB 1 `stage` is a documented synonym dispatching to the same builtin (`git stage -h` prints
`usage: git add`). One word defeated every flag row and the bare-dot row together.
Anchored to whole tokens, which also narrows a pre-existing over-match: `\badd\b` fired
inside `-- add.txt`.
LIMB 2 the flag family GENERATED from the option words per the method BACKLOG #1097 settled
for worktree_gate.ps1. git's parse-options binds a long option by any unambiguous
abbreviation, so `--a`, `--al`, `--up`, `--upd` and `--updat` all stage; a two-spelling
test could never have covered that. A longer list has the same shape as the defect.
LIMB 3 whole-tree pathspecs, kept a SEPARATE rule from the flags.
WHY LIMB 3 IS NOT FUSED INTO LIMB 2. 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 needs one boundary to satisfy both tests and is wrong for one of
them. They also earn different deny messages -- telling an operator who typed `:/` that the problem
was a flag is the wrong sentence -- and different residual lists, which #1097's method requires
each rule to state.
THE CLUSTER RULE IS CASE-SENSITIVE AND THE CASE IS THE WHOLE BOUND. `A`/`a` and `u`/`U` are four
different things in this one command and only two of them stage:
-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`
-U --unified IS a real flag and stages nothing
Denying either of the last two refuses work git either rejects itself or performs harmlessly:
pure false-deny surface, zero protection bought. Both are pinned as ALLOW tests, because without
that negative the next reader "simplifies" `[Au]` to `(?i)[au]` and the rule stops describing the
family. That is the same bound `-Cm` provides in #1097.
A DESIGN GAP THE TESTS CAUGHT, and it is an interaction with #1341 rather than a defect in either
alone. #1341 blanks quoted spans so prose cannot reach program position -- but that also blanked a
legitimately quoted PATHSPEC ARGUMENT, so `git add ':(top)'` allowed. Blanking is right for finding
program position and wrong for reading arguments. The fix uses the subcommand #1341 already
resolves: when it 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); otherwise the
blanked view is correct, so `git commit -m "wip; git add -a"` is still not read as a stage. An
unresolved subcommand still falls back to the original token search, which denies.
MEASURED, and the scope is named rather than counted:
ruff format --check, ruff check, mypy -- each run separately with its own exit code, all clean
on tests/test_blanket_stage_guard.py. (Chained with && the first failure had silently skipped
mypy, which is the same masking this guard's own SilentlyContinue produced -- see below.)
pytest, 174 passed, over test_blanket_stage_guard.py + the three modules that reference this
guard, in the lane venv built to ci.yml's test-leg line. Module went 48 -> 103 cases.
the 104-payload differential against the origin/main blob, re-run with both fixes in place:
ALLOW -> DENY 21 real blanket stages now caught
DENY -> ALLOW 12 ALL TWELVE the intended #1341 prose flips, none new from this change
harness errors 0
NOT a full-suite run.
WHAT IS STILL NOT REACHED, stated in the file beside the rules per CLAUDE.md section 11 rather
than left for a reader to discover: magic pathspec beyond the four spellings (`:(glob)`,
`:(icase)`, `:!x`, a bare `*`), `--renormalize`, `--pathspec-from-file=-` where the pathspec is on
stdin, and any wrapper-dispatched or path-qualified git (`env git add -A`, `cmd /c "git add -A"`).
The last is BACKLOG #1305's axis and closing it needs a wrapper allowlist, the construct BACKLOG
#1229 measured as fail-open on the sibling gate. Nine of the corpus rows remain allowed for these
reasons and are named, not rounded away.
ONE INSTRUMENT NOTE WORTH CARRYING. A PowerShell precedence trap cost 63 test failures and read as
a logic error: `@($cursor, $scan.Length - $cursor)` parses as `($cursor, $scan.Length) - $cursor`,
because the comma binds tighter than the minus. `$ErrorActionPreference = 'SilentlyContinue'` at
the top of this guard swallowed the resulting exception entirely, so the script exited 0 and
allowed every command. A guard that fails open on an INTERNAL error returns exactly what a guard
that examined the command and approved it returns. It was found only by re-running a copy with the
suppression stripped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…unwired (BACKLOG #1339)
PARTIAL, AND COMMITTED PARTIAL ON PURPOSE. An owner instruction to halt work arrived mid-item,
relayed through the Liaison, with the direction "finish the COMMIT, not the task". The instrument
half is complete and proven; the claim-strike half is three sites short. Both halves are itemised
below so the next hand knows exactly where the line is.
WHAT THIS FIXES. Every check in this module walks `hooks.<event>[].hooks[]` -- 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` is referenced by no matcher in the tracked
settings.json while at least eight tracked sites describe it as a live control, and nothing here
could see that, because the thing to see was an ABSENCE. The module's own docstring already names
this failure shape -- "absence assertions over a file that is currently correct" -- and then had
it.
THREE STATES, NOT TWO, AND THAT IS THE WHOLE DESIGN. A wired/unwired instrument would assert the
exact falsehood this test exists to stop. Six scripts are wired at USER level by a TRACKED
INSTALLER rather than by the tracked settings.json -- install-coordination.ps1 wires five,
install-gate.ps1 wires worktree_gate.ps1 -- and they are installed and live. Under two states all
six land on the "deliberately not wired" list, producing a reviewed record claiming six live hooks
are switched off. So the installer state is MEASURED FROM THE INSTALLERS, which is also what stops
it decaying into another enumeration.
The residual `_KNOWN_UNWIRED` is four entries and 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.
PROVEN LIVE, NOT ASSUMED. Mutation: renamed the `block-blanket-git-stage.ps1` key and re-ran.
file hash before 02f2cbcf20a59750, after 5b00dbe884d6f3e9 -- MUTANT CONFIRMED APPLIED before
scoring, because a mutant that fails to apply is indistinguishable from a test that cannot fail
and both print the same passing count
two tests fired, each for its own reason: the main assertion saw the now-unaccounted script, and
the rot-check saw a listed name with no file behind it
restored, hash back to 02f2cbcf20a59750, BYTE-IDENTICAL, 11 passed
A third row was also added to `test_the_checks_can_actually_fail`, so the detector carries a
planted-defect control in the same idiom as the two that were already there.
TWO DIRECTIONS OF ROT, BOTH PINNED. A script wired nowhere and unlisted fails. A script listed as
unwired that IS wired also fails -- that is the same false-record defect pointing the other way,
and it is the one that would appear after somebody does the wiring.
CLAIM-STRIKE, PARTIAL -- 2 of 5 sites corrected here:
DONE .gitignore -- said the tracked settings.json carries the guard as an ENFORCED control
DONE tests/test_private_paths_stay_ignored.py -- same claim inside a test's rationale, so a
prose-only fix elsewhere would have left it sitting in an assertion's justification
TODO tests/test_announce_hook.py:859 -- the false count is in the DOCSTRING only; the test
asserts len(announced) == 1 and PASSES. DO NOT go looking for a red test there
TODO CONTRIBUTING.md:130-141 -- headed "this repo ships two hooks" and names both this guard
and scripts/worktree/session-context.ps1; both measure zero references
TODO docs/Secure_AI_Development_Standards.md -- five sites, and its settings.json excerpt is
alleged uninstallable as written (dot-anchored denies the suite forbids, plus an "if" key
present in 0 of 4 settings files). NOT verified by me; treat as unmeasured
The guard's own false header was already corrected in cb65675.
OWNER RULING, relayed via the Liaison 2026-08-25 and NOT yet in a citable file -- treat as relayed,
not as a path I can cite: "The blanket-git-stage guard is a CONTROL. Strike the claim now, wire it
after the splitter repair." The sequence is strike + instrument in the same change, then wire only
after the quote-state repair. This commit is the instrument plus 2 of 5 of the strike. NO WIRING IS
DONE HERE and none should be until the remaining sites are corrected.
VERIFIED, scope named:
pytest 25 passed over the two modules this commit touches
ruff check, ruff format -- clean
mypy reports one PRE-EXISTING error at :58 in `_load()`, a line this diff does not touch
(`git diff --cached | grep -c 'def _load'` returns 0) and which CI does not cover:
ci.yml:399 runs `mypy messagefoundry messagefoundry_webconsole`, not tests/. Not introduced
here, not in scope, and named rather than silently passed over.
NOT a full-suite run, and deliberately not attempted under a halt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ve control (BACKLOG #1339) Completes the claim-strike begun in 3fff6b8, which landed the instrument plus 2 of 5 sites under a halt. The remaining three are corrected here, and one of them was wrong in a way the item did not report. THE CLAIM. `scripts/hooks/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 in the same probe -- collision_gate and worktree_gate, wired at user level by install-coordination.ps1 and install-gate.ps1, return non-zero. 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. NOT A REPEAT OF #327. That item's finding was "does not reach a fresh clone", and its fix shipped when .claude/settings.json became tracked at 7d873ec. This is the different finding underneath: nobody ever added the matcher. SITES CORRECTED HERE: docs/Secure_AI_Development_Standards.md the installable excerpt, plus 4 claim sites CONTRIBUTING.md "this repo ships two hooks" tests/test_announce_hook.py a false count inside a test's rationale tests/test_claude_settings_contract.py its OWN docstring made the same claim THE LAST ONE IS THE SHARPEST AND I ALMOST SHIPPED WITHOUT IT. The module whose job is to catch a control that reads as enforced and is not was itself asserting one, in its opening paragraph. I added the instrument to that file in 3fff6b8 and did not correct the sentence six lines above it. THE SADS EXCERPT WAS UNINSTALLABLE AS WRITTEN, AND IN THREE WAYS RATHER THAN TWO. It was captioned "excerpt -- abridged; the real file has more denies", which asserts the lines shown ARE the real file. A reader applying them produced a configuration this repo's own suite fails: 1. every deny rule was `./`-anchored (`Read(./.env)`). Bare patterns follow gitignore semantics and match at ANY depth; `./` matches one directory and is strictly narrower -- the worst combination for a control whose job is to be broad. test_no_deny_rule_uses_the_narrow_dot_anchor asserts zero such rules; the tracked file has 0 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}. *** THIS ONE WAS NOT IN THE REPORT. I found it while checking the other two, and I am naming it because a two-item list that is really three is exactly the completeness claim CLAUDE.md section 11 warns about. *** 3. an `"if"` key that appears in 0 of 9 settings files on this machine and is not in the schema. The report said 0 of 4; I measured a wider population and got the same answer. The PreToolUse block is REMOVED rather than corrected, because it documented wiring that does not exist. Correcting it in place would have produced a working-looking installation snippet for a control the same document now says is not wired. WHAT I DID NOT DO. I did not wire the guard. The owner ruling relayed 2026-08-25 sequences it strike + instrument first, wire only after the quote-state splitter repair -- that repair is cb65675 on this branch and is NOT on main. Every corrected site says "present, not wired" and names the condition under which it becomes a gate, so the next reader does not have to infer it. VERIFIED, scope named: pytest 75 passed over the three test modules this commit touches pytest 83 passed over the four gates that assert on these two documents (test_ai_provenance_claims, test_quality_record_scope_claims, test_scan_tokens_source, test_worktree_venv_constraint) -- found by grepping tests/ scripts/ and .github/workflows/ for both filenames, because a doc edit that reds a gate elsewhere is the expensive kind ruff format --check and ruff check -- each run separately with its own exit code, both clean NOT a full-suite run Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 25, 2026 18:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four commits, verified independently before opening/extending:
All three are already-filed ledger items (2026-08-23) -- this is the implementation, not a new filing. No ledger banner update owed: none of the eight touched files match the directories that set touches_code in the hygiene gate (backlog-hygiene.yml:83).
The guard is deliberately still not wired into any hook. Owner ruling (relayed via the Liaison): strike the stale claim and add the instrument together, wire only after the quote-state splitter repair -- which is commit cb65675 on this branch, not yet on main at the time the ruling was made. Every corrected site says 'present, not wired' and names the condition for wiring. That is intentional, not an oversight.
Six previously-pinned over-deny test pairs are deliberately flipped, not accidentally removed -- tests/test_blanket_stage_guard.py had them pinned as known-wrong specifically so whoever fixed the splitter would flip them on purpose. Same twelve payloads, opposite expectation, under renamed tests; coverage went from 48 to 103 cases in that module. The commit message says so.
Verification (author's, explicitly scoped, not a full-suite claim): 174 + 83 + 75 tests pass across the modules touched by each commit; ruff/ruff format/mypy clean on every changed Python file; a 104-payload differential against origin/main's guard (21 ALLOW->DENY, all real blanket stages now caught; 12 DENY->ALLOW, all intended pinned prose/read-only cases; 0 harness errors). One pre-existing mypy error at tests/test_claude_settings_contract.py:58 is untouched by this branch and not covered by CI's mypy scope (messagefoundry/ + messagefoundry_webconsole/ only) -- named so it isn't misattributed.
Verified before opening: merge-tree against main was clean and correctly kept #578's owner-ruling fix on item #1336 (this branch forked before #578 landed and never touched those two lines itself).