From 1d1359222264d2d45965b702cb1972733e527af1 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 26 Aug 2026 21:37:03 -0500 Subject: [PATCH 1/2] fix(gate): a QUOTED disarm key was blanked before rule 3c could see it (BACKLOG #1069) Get-ScannableSegments builds each segment's Scan through Remove-QuotedSpans, which blanks every closed quoted span, and rule 3c matched the danger key against Scan. Quoting the key therefore erased it before the disarm regex ran -- and quoting an argument is ordinary, so this needed no unusual spelling. It disarmed the ledger, claim and leak commit gates for every worktree at once. Reproduced with the UNQUOTED spelling as a known-answer control, which denied in both arms. All five spellings the row lists allowed before and deny now. MY FIRST PROBE REPORTED THE CONTROL ITSELF AS ALLOW, which would have read as a far larger finding. It was a broken probe, and only the known-answer case separated "the gate is blind" from "my harness never reached a governed repo". THE FIX IS A BARE-WORD UNMASK, NOT A RAW-TEXT MATCH. Matching raw text would refuse a commit message quoting the rule's own name, and this workstream writes those constantly. The discriminator is WHITESPACE: prose has it and stays masked, a config key does not and becomes visible. Three such prose messages are pinned as ALLOW. THE LENGTH-PRESERVING MASK THE ROW PRESCRIBES IS DELIBERATELY NOT BUILT. Its stated rationale is that "the same offsets read paths back out of the raw text afterwards" -- and no rule does that: every path site re-runs [regex]::Match($seg.Raw, ...) and computes offsets inside Raw from scratch. Length-preserving masking would change what every other rule sees for zero benefit here. Widening scope past the defect is exactly how the earlier attempt acquired five new fail-opens, which is why that patch carries a do-not-ship order. It remains unshipped and untouched. ONE SPELLING STAYS OPEN BY DESIGN: a quoted multi-word alias value. Its value contains a space, so the carve-out cannot reach it without re-admitting the prose false deny. Pinned as an ALLOW test whose docstring says it is an open hole rather than a passing property, and says a later tokeniser should make it FAIL and be inverted alongside the banner rather than deleted. Regression: 631 passed, 0 failed across all eight worktree-gate suites. Mutation: disabling the unmask reds exactly the five deny rows and leaves the control, the three prose rows and the by-design gap green. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 20 ++++++ scripts/hooks/worktree_gate.ps1 | 26 +++++++- tests/test_worktree_gate_quoted_key.py | 92 ++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 tests/test_worktree_gate_quoted_key.py diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index d7bf9efb7..bc07d7415 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -6155,6 +6155,26 @@ the blanking here without measuring `#1086`'s false-deny rows in the same table. **Source:** found 2026-08-06 by the must-keep-allowing inventory built for #1066, which enumerated what rule 3c allows today and found this in the gap between the shipped tests. ## 1069. Rule 3c matched the disarm key on the quote-blanked scan string, so a QUOTED key was invisible +> **THE QUOTED-KEY FAIL-OPEN IS FIXED 2026-08-27; the banner stays open for the archive pass, and the +> multi-word spelling below REMAINS OPEN BY DESIGN.** `Remove-QuotedSpans` now UNMASKS a quoted span +> holding a single BARE WORD -- no whitespace, quote, `$`, bracket, brace, semicolon, ampersand, pipe +> or backtick. Prose keeps its spaces and stays masked; a config key has none and becomes visible. +> +> **Measured before and after, with the UNQUOTED spelling as a known-answer control** (it denied in +> both arms, so an ALLOW below is a reading and not a dead probe): all five quoted spellings this row +> lists ALLOWED before and DENY after; three prose commit messages quoting `core.hooksPath` ALLOW in +> both arms, so the false-deny this item warns about was not admitted. +> +> **THE LENGTH-PRESERVING MASK THIS ROW PRESCRIBES WAS DELIBERATELY NOT BUILT, and the reason is a +> measurement rather than a preference.** Its stated rationale is that *"length preservation is what +> lets the same offsets read paths back out of the raw text afterwards"* -- and NO RULE DOES THAT. +> Every path site re-runs `[regex]::Match($seg.Raw, ...)` and computes its offsets inside `Raw` from +> scratch. Length-preserving masking would change what every OTHER rule sees, for zero benefit to the +> defect being closed. **Widening scope beyond the defect is precisely how the earlier attempt at this +> item acquired five new fail-opens**, which is the outcome this row's own DO-NOT-SHIP order records. +> +> The banked patch remains unshipped and untouched. +> > 🔢 **Re-scored 2026-08-20 -> P2.** Value **7/10** · Difficulty **4/10** · _quick win_. Rule 3c still decides on $seg.Scan at worktree_gate.ps1:976-978 while Remove-QuotedSpans at :347-388 blanks every closed quoted span, so a quoted danger key is erased before the disarm regex runs; grep finds no length-preserving mask, no bare-word unmask, and the pinned ALLOW test the item names is absent from tests/. Value 7 because the fail-open needs no unusual spelling and disarms the ledger, claim and leak commit gates for every worktree at once with no compensating detection, but its blast radius is the developer harness rather than a deployment; difficulty 4 because the one written fix was rejected on verification after acquiring five new fail-opens, so the remainder is a scanner rewrite plus an adversarial test round. _(was 8/10 · 3/10.)_ > diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 1a8e0cff3..abd77c29b 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -554,7 +554,31 @@ function Remove-QuotedSpans([string]$s, [bool]$PosixEscapes = $false) { # main denies, so it was reverted. Do NOT re-add the lowercase emit without that # discriminator, and do not add the discriminator without re-measuring those two. $span = $s.Substring($openAt + 1, $i - $openAt - 1) - if ($span -cmatch '[\\/](git(?:\.exe)?)$') { + # BACKLOG #1069: A QUOTED SPAN HOLDING ONE BARE WORD IS UNMASKED, because quoting an argument + # is ORDINARY and blanking it erased the disarm key before rule 3c ever ran. Measured on the + # shipped gate, all ALLOW where the unquoted spelling DENIES: + # git -c "core.hooksPath=/dev/null" commit -m x + # git config 'core.hooksPath' '/dev/null' + # git config --add "core.hooksPath" /dev/null + # + # WHY NOT MATCH THE RAW TEXT INSTEAD: a commit message quoting the rule's own name would then + # refuse, and this workstream writes such messages constantly. The discriminator is WHITESPACE + # -- prose has it and stays masked; a config key does not and becomes visible. + # + # DELIBERATELY NOT LENGTH-PRESERVING, against this item's own prose. That rationale is "the same + # offsets read paths back out of the raw text afterwards", and NO RULE DOES THAT: every path + # site re-runs [regex]::Match($seg.Raw, ...) and computes offsets inside Raw from scratch. + # Length-preserving masking would change what every other rule sees for zero benefit, and + # widening scope is exactly how the earlier attempt at this item acquired five new fail-opens. + # + # ONE SPELLING STAYS OPEN BY DESIGN: a quoted MULTI-WORD value such as + # -c 'alias.ci=commit --no-verify'. Its value contains a space, so quoting is its only writable + # spelling and this carve-out cannot reach it without re-admitting the prose false-deny. Pinned + # as an ALLOW test so a later change cannot close it silently or claim it was never there. + if ($span.Length -gt 0 -and $span -cnotmatch '[\s''"$(){};&|`]') { + [void]$out.Append($span) + } + elseif ($span -cmatch '[\\/](git(?:\.exe)?)$') { [void]$out.Append($Matches[1]) } else { diff --git a/tests/test_worktree_gate_quoted_key.py b/tests/test_worktree_gate_quoted_key.py new file mode 100644 index 000000000..2873f12a1 --- /dev/null +++ b/tests/test_worktree_gate_quoted_key.py @@ -0,0 +1,92 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""BACKLOG #1069: a QUOTED disarm key was blanked before rule 3c could see it. + +``Get-ScannableSegments`` builds each segment's ``Scan`` through ``Remove-QuotedSpans``, which blanks +every closed quoted span. Rule 3c matched the danger key against ``Scan``. So quoting the key erased it +before the disarm regex ran, and **quoting an argument is ordinary** -- this needed no unusual spelling +and disarmed the ledger, claim and leak commit gates for every worktree at once. + +THE FIX IS A BARE-WORD UNMASK, NOT A RAW-TEXT MATCH. Matching the raw text instead would refuse a commit +message that quotes the rule's own name, and this workstream writes such messages constantly. The +discriminator is WHITESPACE: prose has it and stays masked, a config key does not and becomes visible. + +EVERY DENY ROW HERE IS PAIRED WITH THE UNQUOTED CONTROL BELOW. Without it an ALLOW cannot be told from a +probe that never reached a governed repo -- the first version of this file reported the unquoted control +as ALLOW, which would have read as a far larger finding than the real one. It was a broken probe. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + +from tests.test_worktree_gate import run_gate +from tests.test_worktree_gate_control_plane import repo, shell # noqa: F401 + +#: The unquoted spelling. Denied before this fix and after it, so it is a KNOWN-ANSWER control on the +#: probe itself rather than a test of the fix. +UNQUOTED_CONTROL = "git config core.hooksPath /dev/null" + +#: Every spelling BACKLOG #1069 measured as ALLOW on the shipped gate. All five deny now. +QUOTED_DISARMS = [ + 'git -c "core.hooksPath=/dev/null" commit -m x', + "git -c 'core.hooksPath=/dev/null' commit -m x", + 'git config "core.hooksPath" /dev/null', + "git config 'core.hooksPath' '/dev/null'", + 'git config --add "core.hooksPath" /dev/null', +] + +#: Prose that NAMES the danger key inside a quoted commit message. These are the false denies a +#: raw-text match would have introduced, and they are the reason the carve-out is bare-word-only. +PROSE_MUST_STILL_ALLOW = [ + 'git commit -m "do not set core.hooksPath in a worktree"', + "git commit -m 'BACKLOG #1069: core.hooksPath was invisible when quoted'", + 'git commit -m "see rule 3c and its core.hooksPath disarm list"', +] + + +def test_the_unquoted_key_is_denied(repo: SimpleNamespace) -> None: # noqa: F811 + """THE CONTROL EVERY OTHER ROW DEPENDS ON. If this allows, the probe never reached a governed repo + and every ALLOW in this file is meaningless rather than reassuring.""" + assert run_gate(shell(UNQUOTED_CONTROL, cwd=repo.primary), repo.repos) is not None, ( + "the UNQUOTED disarm key was allowed -- the probe is broken, so nothing else here is a reading" + ) + + +@pytest.mark.parametrize("command", QUOTED_DISARMS) +def test_a_quoted_disarm_key_is_seen(repo: SimpleNamespace, command: str) -> None: # noqa: F811 + assert run_gate(shell(command, cwd=repo.primary), repo.repos) is not None, ( + f"FAIL-OPEN: quoting the key hid it from rule 3c -- {command}" + ) + + +@pytest.mark.parametrize("command", PROSE_MUST_STILL_ALLOW) +def test_prose_naming_the_key_is_not_denied(repo: SimpleNamespace, command: str) -> None: # noqa: F811 + assert run_gate(shell(command, cwd=repo.primary), repo.repos) is None, ( + f"FALSE DENY: a commit message that merely NAMES the key was refused -- {command}" + ) + + +def test_a_QUOTED_MULTI_WORD_alias_value_is_still_invisible(repo: SimpleNamespace) -> None: # noqa: F811 + """PINNED AS AN ALLOW ON PURPOSE. THIS IS AN OPEN HOLE, NOT A PASSING PROPERTY. + + ``-c 'alias.ci=commit --no-verify'`` disarms the hooks and is still not seen, because its value + contains a SPACE -- so the bare-word carve-out cannot reach it without re-admitting the prose false + deny the tests above pin. Quoting is that value's only writable spelling, so the whole class is open. + + IT IS PINNED SO A LATER CHANGE CANNOT CLOSE IT SILENTLY OR CLAIM IT WAS NEVER THERE. If someone + builds a real argument tokeniser, this test SHOULD fail, and the correct response is to invert it + together with BACKLOG #1069's banner -- not to delete it. The one pass that attempted that tokeniser + acquired five new fail-opens, which is why the narrow carve-out shipped instead. + """ + assert ( + run_gate( + shell("git -c 'alias.ci=commit --no-verify' ci -m x", cwd=repo.primary), repo.repos + ) + is None + ), ( + "the multi-word alias spelling now DENIES. That is progress, not a regression -- invert this " + "test and BACKLOG #1069's banner together rather than deleting either." + ) From 3956f941ad38bdd3642e634dab1dd884be13e6a2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 27 Aug 2026 18:27:56 -0500 Subject: [PATCH 2/2] test(tooling): list the quoted-key gate test in the tooling manifest (BACKLOG #1069) PR 634 is RED on three required legs with one deterministic cause: tests/test_tooling_partition.py::test_every_non_engine_test_is_classified "unclassified: ['test_worktree_gate_quoted_key.py']" The branch adds a test file that imports no engine module and never lists it. The assertion names both remedies itself; this is the manifest one, not _STAYS_WITHOUT_IMPORTING, because the test drives the hook as a subprocess and does not read engine source. NOT A FLAKE, AND NOT RE-RUNNABLE. The lander playbook records that tooling_partition is never a flake and that a re-run past one lands a defect on purpose. Three legs red on one cause is the signature. INSERTED IN THE FILE'S OWN ORDER, NEVER SORTED. tests/tooling_manifest.txt is NOT plain lexicographic -- test_worktree_gate.py sits AFTER its underscore-suffixed siblings -- so a sort() rewrites unrelated lines. A previous session did exactly that and moved 22 lines it did not intend to touch. Anchored on the following entry instead. Measured, interpreter borrowed from a sibling worktree because this one has no venv, and VERIFIED to read THIS tree rather than the lender's (messagefoundry.__file__ resolves under wt1069, since cwd wins over an editable install -- a borrowed interpreter testing the wrong tree is silent): red-first: reverting only this line reds test_every_non_engine_test_is_classified and nothing else (1 failed / 8 passed); restoring it returns 9 passed git diff --numstat reads 1/0 -- a re-sort would have shown roughly 22 on both sides rebased onto a490993b4 first; the rebase was clean Rebase note for whoever pushes: this branch was 24 behind and is now ahead 1 of a490993b4, so the PR head moves and needs a force-update with a lease. Co-Authored-By: Claude Opus 5 --- tests/tooling_manifest.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tooling_manifest.txt b/tests/tooling_manifest.txt index 433d8cd67..294ffb3e2 100644 --- a/tests/tooling_manifest.txt +++ b/tests/tooling_manifest.txt @@ -127,6 +127,7 @@ tests/test_worktree_gate_hijack.py tests/test_worktree_gate_interpreter_flags.py tests/test_worktree_gate_interpreter_sigils.py tests/test_worktree_gate_quote_straddle.py +tests/test_worktree_gate_quoted_key.py tests/test_worktree_gate_receipts.py tests/test_worktree_gate_remedy_families.py tests/test_worktree_gate_rule_agnostic_coverage.py