diff --git a/.gitignore b/.gitignore index 0a73d51b0..fbc2c8a4e 100644 --- a/.gitignore +++ b/.gitignore @@ -294,3 +294,12 @@ scripts/security/scan-tokens.local.txt # considered both and left them: the 21 bench handoffs carry the measurement narrative for the ~133 # data files beside them, so untracking either half strips the rationale from records that stay. # Re-proposing this is re-deriving a decision that is already written down. + +# A PR BODY IS SCRATCH, NOT A DELIVERABLE, AND ONE ALREADY REACHED A BRANCH TIP ONCE. +# `PR_BODY.md` was committed at the repo root on 2026-09-03 because a push hold stopped a pull +# request being opened, so the body had nowhere else to live. That is a real need and the file is +# genuinely useful in the worktree -- but it describes ONE branch, it goes stale the moment that +# branch merges, and `main` is the one place it must never land. Untracked-but-present is exactly +# the state that serves both, so the rule is here rather than in a reviewer's memory. +# Rooted with a leading slash: only the repo root, never a `docs/` or `tests/` file of that name. +/PR_BODY.md diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 7606f2383..f612d77cc 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -21634,6 +21634,42 @@ The concatenated-quoting mechanism was read directly rather than through a marke - **It does not install or modify the gate.** `scripts/worktree/install-gate.ps1` was never run: **#1247** records that the installer overwrites the live gate with no backup and no receipt, which is not a Builder's call. Everything was driven against the repository copy. - **It does not score itself, and it does not close, reopen or re-verdict #1229.** +## 1429. the worktree gate scans per line, so a quoted span crossing a newline straddles and deletes the live command between its two lines + +> 🔢 **Filed 2026-09-03, REPRODUCED here before filing rather than taken on report, and it is a COUNTEREXAMPLE TO #1427.** That item records the per-line split as producing no hole -- six spellings tested, zero found, and it cites the gate's own docstring agreeing that a quoted argument spanning lines denies today anyway. The reproduction below says otherwise, so one of the two readings is wrong and this filing is where that gets settled. #1229 is left CLOSED: its closure scoped this class out as an inherited residual, and a class that defeats every rule in the gate is its own item rather than evidence that one item is unfinished. `Get-ScannableSegments` splits the command on newlines **before** any quoting is considered, so a quoted span that crosses a newline is never one span to the gate. It is an unterminated quote on the first line and a stray quote on the last -- and the line in the middle carries **one quote from each surrounding span**. Those two pair ACROSS the gated command and blank it, so no rule ever sees it. +> +> **THIS CLASS RE-OPENS EVERY SHAPE #1229 CLOSED, ITS TITLED SHAPE INCLUDED, and an earlier draft of this banner got that backwards.** It called the newline case a distinct straddle while conceding in the next breath that it is *"#1229's straddle exactly"*. Those two statements pull opposite ways, and the measured reading is the second one: **this is a separate FIX, not a separate CLASS.** Insert one newline into a quoted word and #1229's own titled shape goes back to ALLOW -- `echo 'say "hii' ; ; echo 'bye" now'` reads ALLOW on `origin/main` and on the branch that closed #1229, and the middle statement runs (`expr 111 \* 3` prints 333). So this item stays open and correctly allocated as its own fix, and it does **NOT** license closing #1229. **#1229 carries the reproduced three-row table and links back here; read the two together.** +> +> **THE GATE'S OWN RESIDUAL LIST SAID THIS CLASS DENIED, and that claim is measured FALSE.** It read *"Both multi-line forms deny today anyway: every line of such a span reaches the scanner RAW and the payload line carries the git token and the verb by itself."* The first half is right and the second misses that the payload line carries the two straddling quotes too. The sentence is corrected in place in `scripts/hooks/worktree_gate.ps1` rather than deleted, because a compensating control resting on a false premise is itself the defect -- and because that sentence is why nobody probed the class. +> +> **Measured on the shipped gate at `46ea10a78`**, cwd inside the governed repo, with a governed repos file and a positive control (plain `git -C checkout main` -> DENY). Every row is pinned to whether the middle statement really RUNS, using an inert marker that COMPUTES (`expr 111 \* 3` under bash, `111*3` under pwsh, both printing 333) so an echo-back cannot be mistaken for a run: +> +> ``` +> shape runs? gate +> echo 'ab' ; git -C checkout main ; echo 'cd' 333 ALLOW +> echo "ab" ; git -C checkout main ; echo "cd" 333 ALLOW +> Write-Output 'ab' ; git -C reset --hard ; ... 333 ALLOW +> Write-Output "ab" ; git -C reset --hard ; ... 333 ALLOW +> the same four with the span on ONE line (control) 333 DENY +> ``` +> +> The control is what makes those readings evidence: varying only the newline flips the verdict, so the split is the cause and not the shape. +> +> **THE FOURTH ROW WAS MISSING AND THE COUNT WAS WRONG.** This banner and the branch's PR body both said **three** fail-opens remained in this class. The missing corner is `Write-Output "ab" ; ; Write-Output "cd"` -- the PowerShell tool with the DOUBLE quote -- which also ALLOWs and also runs. Both tools times both quote characters is four, so the honest floor is **at least four**, and the number is a floor rather than an enumeration: nothing here ranged over the whole input space. Re-measured 2026-09-03 against gate copies hash-verified byte-identical to `origin/main` and to the branch head, all four rows ALLOW on both blobs and all four print 333. +> +> **Pinned, not merely described.** `tests/test_worktree_gate_quote_straddle.py` carries all four rows as a tripwire asserting the current ALLOW, each with its one-line control. **When that test reds, somebody closed this item** -- delete the rows and invert this banner; do not restore the ALLOW. +> +> **WHY IT WAS NOT CLOSED IN THE SAME CHANGE, stated so the deferral is reviewable.** Closing it means carrying quote state ACROSS the split, which changes what every rule sees on every multi-line command. #1086's own residual note already treats the per-line split as a fixed property, at least four sibling suites assert behaviour that rests on it in their own words (`tests/test_worktree_gate_hijack.py`, `..._interpreter_flags.py`, `..._interpreter_sigils.py`, `..._scope_wording.py`), and the gate's rule sites read `Raw` and `Scan` as a pair, so a change here is a behaviour change to a security control rather than a scanner repair. It wants its own adversarial pass with a paired must-trip and must-not-trip suite, which is more than the span-ownership fix it travelled beside. +> +> **#1086 DOES NOT CLOSE THIS**, and the residual list used to imply it might. #1086's change is message-flag blanking, which decides WHICH spans are blanked; this defect is about where a span BEGINS AND ENDS. Blanking fewer message bodies leaves the straddling pair intact. +> +> **How to prove a fix, in both directions.** The four rows above must DENY. Every control in the two quote suites must keep its current verdict -- the quoted commit message must still ALLOW, the unterminated quote must still DENY, and the interpreter-argument recursion must still reach its inner code. And the fail-OPEN axis must be counted, not just the false denies: a change that removes a false deny while adding one new ALLOW is a net loss on a guard that fails open. **Do not fix this by making the scanner swallow everything after a lone quote** -- that turns one stray character into a total bypass, which is the regression the single-line scanner was built to avoid. +> Verdict: build +> Closing-act: code + +**Cluster:** Tooling / developer guardrail. **Priority:** P3. **Verdict:** build. +**Severity:** minor -- a local maintainer-workstation guardrail that its own synopsis declines to call a security boundary. **No engine effect, no PHI axis, and no deployment axis (sec. 0).** What it costs is the guardrail's reliability on a multi-line command, which is an ordinary shape rather than an exotic one: a heredoc, a quoted commit body, or any message a session writes across lines. + ## 1430. the connscale in-hold sampler yields one sample per cell, so the in-hold window has zero width > 🔢 **Filed 2026-09-03 -- not started. Surfaced by the #1420 measurement, which could not have found it by reading.** The in-hold sampler produces **exactly two samples per cell** -- one in-hold, one post-drain -- in **20 of 20 cells** at hold 1.5 and hold 3.0. So `_empty_claim_rates`' window, which five sites describe as *"first to last in-hold samples"*, does not span a slightly-wrong range. **It spans no range at all: there is one in-hold sample.** diff --git a/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index 9fffc16e8..8d5c4d502 100644 --- a/scripts/hooks/worktree_gate.ps1 +++ b/scripts/hooks/worktree_gate.ps1 @@ -664,34 +664,128 @@ function Get-GitTargetCandidatesRaw([string]$Line, [string]$Prefix, [string]$Cwd $out | Where-Object { $_ } } +# THE ESCAPE TABLE LIVES HERE AND ONLY HERE (BACKLOG #1229 residual, fifth round). `[char]0` means +# this host escapes nothing. +# +# TWO PLACES NEED THIS FACT -- the span scanner below, and the interpreter-argument EXTRACTION regex +# in Get-ScannableSegments -- AND THE THIRD ROUND WAS A MEASURED FAIL-OPEN CAUSED BY EXACTLY THOSE +# TWO DISAGREEING about where an argument ends. That round fixed the disagreement and left the +# structural condition intact: the character was still spelled twice, once as a `[char]` and once +# inside a hand-written regex. Spelling it twice re-creates the defect even while the current values +# happen to agree, and no test can see the gap, because a source-text assertion pins each spelling +# separately. So a sixth host is added HERE, and the extraction regex is DERIVED from this. +# +# WHAT AN UNMEASURED HOST GETS, and why it is not a guess: nothing. `cmd` escapes with `^`, which +# nobody has measured here, so it falls to the default rather than borrowing a neighbour's rule. +# Both regressions in this function's history came from applying one host's escape to another +# host's text. +function Get-EscapeChar([string]$Convention) { + switch ($Convention) { + 'posix' { [char]0x5C } # backslash: sh, bash, dash, zsh + 'pwsh' { [char]0x60 } # backtick: PowerShell's own escape + default { [char]0 } # cmd, none, and any tool name not recognised + } +} + +# THE TEXT AFTER THE OUTER HOST HAS FINISHED WITH IT (BACKLOG #1229 residual, SIXTH round). +# +# An interpreter argument is extracted from the OUTER line, so it arrives in the OUTER host's +# ENCODING -- its escapes are still spelled out, because the outer host has not run yet. The inner +# interpreter never sees them: by the time it is handed the argument, the outer host has consumed +# them. This turns that text into what the interpreter actually receives. +# +# ESCAPED QUOTES ONLY, AND THE NARROWNESS IS MEASURED RATHER THAN CAUTIOUS. Stripping EVERY escape +# is what an escape rule reads like, and it is wrong on the host that matters most here: inside a +# double-quoted word `sh` honours the backslash before `$` `` ` `` `"` `\` and newline and NOWHERE +# ELSE, so a Windows path keeps its separators. MEASURED with `printf '%s'` over a double-quoted +# `D:\Work\x`, which prints back unchanged. A blanket strip turned that into `D:Workx`, which stops +# resolving to the repository it names, so the view built to SEE a gated command lost its target. +# The escaped QUOTE is the whole of the defect this exists for, on both conventions. +# +# READS THE SAME TABLE AS EVERYTHING ELSE, for the reason stated above it: a second spelling of the +# escape character is what round 3 was. +function Remove-EscapeChars([string]$s, [string]$Convention) { + $esc = Get-EscapeChar $Convention + if ($esc -eq [char]0) { return $s } + $out = [System.Text.StringBuilder]::new() + for ($i = 0; $i -lt $s.Length; $i++) { + $nxt = $(if ($i + 1 -lt $s.Length) { $s[$i + 1] } else { [char]0 }) + if ($s[$i] -eq $esc -and ($nxt -eq '"' -or $nxt -eq "'")) { [void]$out.Append($nxt); $i++ } + else { [void]$out.Append($s[$i]) } + } + $out.ToString() +} + +# `cmd /c ""` RUNS ``, and the quotes are cmd's, not the command's. This is cmd.exe's +# own documented rule ("cmd /?", the /C and /K quote logic): where the first character is a quote, +# cmd strips that quote and the LAST quote on the line, then executes what is left. Its other arm -- +# quotes PRESERVED when the quoted text names an executable file -- is a filesystem question this +# hook cannot answer, and getting it wrong here only makes MORE text visible, never less. +function Remove-CmdWrapperQuotes([string]$s) { + $t = $s.TrimStart() + if (-not $t.StartsWith('"')) { return $s } + $last = $t.LastIndexOf('"') + if ($last -le 0) { return $s } + $t.Substring(1, $last - 1) + $t.Substring($last + 1) +} + # Decide from the COMMAND, never from prose inside it -- but "prose" and "code" are not the same as # "quoted" and "unquoted", and conflating them was a measured regression. # # Three false positives came from scanning the raw string: a two-line command whose second line read # `echo about to merge stuff` denied with verb=merge; `echo "git checkout main"` denied; and -function Remove-QuotedSpans([string]$s, [bool]$PosixEscapes = $false) { +function Remove-QuotedSpans([string]$s, [string]$Convention = 'none') { <# - ``$PosixEscapes`` -- DOES THIS HOST TREAT A BACKSLASH AS AN ESCAPE? (BACKLOG #1229 residual, second - round.) `sh` does; **PowerShell does NOT** -- its escape is the BACKTICK, so `"C:\Temp\"` is a - COMPLETE string there and whatever follows it RUNS. + ``$Convention`` -- WHICH CHARACTER ESCAPES A QUOTE ON THIS HOST? `posix` means the BACKSLASH (sh, + bash, dash, zsh); `pwsh` means the BACKTICK, which is PowerShell's own escape; anything else -- + `cmd`, `none`, an unrecognised tool name -- means NOTHING is treated as an escape. - THIS PARAMETER EXISTS BECAUSE ITS ABSENCE RE-CREATED #1229's OWN DEFECT ON THE OTHER HOST. The - first version of this fix honoured the escape unconditionally, which is correct POSIX -- but line - 999 scans BOTH tool names through ONE matcher, so on a PowerShell payload the scan held a span open - that PowerShell had already closed, straddled the live command between it and a later quote, and - blanked it. MEASURED on the shipped fix, both tool names: + THIS PARAMETER EXISTS BECAUSE A SINGLE ESCAPE RULE RE-CREATED #1229's OWN DEFECT ON THE OTHER HOST, + TWICE, ONCE IN EACH DIRECTION. Both are recorded because each one refutes the obvious reading of + the other. + + ROUND 2 -- HONOURING THE BACKSLASH EVERYWHERE. The first version of this fix honoured it + unconditionally, which is correct POSIX -- but this file scans BOTH tool names through ONE matcher, + so on a PowerShell payload the scan held a span open that PowerShell had already closed, straddled + the live command between it and a later quote, and blanked it. MEASURED, both tool names: Write-Output "C:\Temp\" ; git -C reset --hard ; Write-Output "x" ALLOW ... same line with ONE FEWER backslash (control) DENY ... same line with TWO backslashes (even count) DENY - An ODD count before the closer was the trigger. Verified the middle statement really executes with - an inert payload that COMPUTES rather than echoes, so an echo-back could not be mistaken for a run. + An ODD count before the closer was the trigger. + + ROUND 5 -- REFUSING THE HOST'S OWN ESCAPE. Round 2 left `pwsh` with NO escape at all, on the + reasoning quoted below, and that reasoning is WRONG AS A GENERAL CLAIM. It is corrected here rather + than deleted, because it is the sentence the next reader would act on: + + "Honouring the escape makes spans LONGER, so it BLANKS MORE and can hide a command -- fail + OPEN. Refusing it makes spans shorter, leaving more text visible to the rules -- fail CLOSED." + + THAT IS TRUE OF SPAN LENGTH AND FALSE OF SPAN POSITION. Refusing an escape the host really honours + does not just shorten the first span -- it SHIFTS every pairing after it, and the shifted pair + straddles the live command. Which is #1229's own mechanism, arriving through the door built to + keep it out. MEASURED on the PowerShell tool with a payload that COMPUTES (`111*3` -> 333, so an + echo-back cannot be mistaken for a run): + + Write-Output "a`"b" ; git -C reset --hard ; Write-Output "c`"d" 333 printed: RAN + on the round-2 scanner ALLOW fail-open + ... same shape with the DOUBLED-quote escape ("") instead (control) 333 printed: DENY + ... same shape with no escape at all (control) 333 printed: DENY + ... the IDENTICAL characters under the Bash tool (control) unexpected EOF: INERT - DEFAULT IS $false, AND THE DIRECTION IS THE WHOLE POINT. Honouring the escape makes spans LONGER, - so it BLANKS MORE and can hide a command -- fail OPEN. Refusing it makes spans shorter, leaving more - text visible to the rules -- fail CLOSED. An unknown or unrecognised host therefore gets the - conservative reading, and only a host known to use backslash escapes opts in. + So the rule is not "opt in to escapes when it is safe". It is: MODEL THE HOST YOU WERE GIVEN. A + scanner that agrees with the shell has no straddle by construction, in either direction. + + THE DEFAULT IS STILL THE CONSERVATIVE ONE AND STILL 'none'. An unknown host gets no escape rule, + because guessing one is how both rounds above happened; and `cmd` keeps 'none' deliberately, since + cmd.exe's escape is `^` and nobody has measured it here. Only a host whose escape was measured + opts in. + + NOT MODELLED, STATED SO NO STRONGER CLAIM IS INFERRED: PowerShell's OTHER escape, the doubled quote + `""`. Naive pairing already covers the same extent for it -- it closes at the first of the pair and + reopens at the second, leaving no gap for live code -- so it needs no rule here, and the extraction + regex below is left equally blind to it so the two cannot disagree (see round 3). #> <# @@ -718,30 +812,39 @@ function Remove-QuotedSpans([string]$s, [bool]$PosixEscapes = $false) { a lone quote would fail OPEN, turning one stray character into a total bypass, so on reaching the end still inside a quote this emits the original text from the opener onward. #> + # HOISTED OUT OF THE PER-CHARACTER LOOP, both of them. `$esc` is one table lookup and `$hasEsc` + # is one comparison; inside the loop each would be paid per character of every scanned line, on a + # hook that runs on every tool call. + $esc = Get-EscapeChar $Convention + $hasEsc = $esc -ne [char]0 + $out = [System.Text.StringBuilder]::new() $quote = [char]0 $openAt = -1 for ($i = 0; $i -lt $s.Length; $i++) { $ch = $s[$i] if ($quote -eq [char]0) { - # A BACKSLASH ESCAPE OUTSIDE A SPAN IS A LITERAL AND OPENS NOTHING (BACKLOG #1229 - # residual). `\"` is an ordinary character to the shell, so the command around it RUNS -- - # but this scan treated it as an opener, paired it with the next escaped quote, and blanked - # the live command between them. Same straddle as the two-regex defect above, one character - # class over, and RULE-AGNOSTIC: it disarms whatever rule sits behind it, so it hid - # `reset --hard` and `worktree add` and not only `checkout`. - if ($PosixEscapes -and $ch -eq '\' -and $i + 1 -lt $s.Length) { + # AN ESCAPED QUOTE OUTSIDE A SPAN IS A LITERAL AND OPENS NOTHING (BACKLOG #1229 + # residual). `\"` in sh, and `` `" `` in PowerShell, are ordinary characters, so the + # command around them RUNS -- but this scan treated one as an opener, paired it with the + # next escaped quote, and blanked the live command between them. Same straddle as the + # two-regex defect above, one character class over, and RULE-AGNOSTIC: it disarms whatever + # rule sits behind it, so it hid `reset --hard` and not only `checkout`. + if ($hasEsc -and $ch -eq $esc -and $i + 1 -lt $s.Length) { [void]$out.Append($ch); [void]$out.Append($s[$i + 1]); $i++ } elseif ($ch -eq '"' -or $ch -eq "'") { $quote = $ch; $openAt = $i } else { [void]$out.Append($ch) } } - elseif ($PosixEscapes -and $quote -eq '"' -and $ch -eq '\' -and $i + 1 -lt $s.Length) { - # Inside a DOUBLE-quoted span a backslash escapes the next character, so `\"` does not - # close it. DELIBERATELY NOT APPLIED INSIDE A SINGLE-QUOTED SPAN: sh gives the backslash no - # special meaning there, so `'a\'` really does close at that quote. Treating the two alike - # would swallow the rest of the line from a trailing backslash -- fail-open, which is the - # direction this whole function exists to avoid. + elseif ($hasEsc -and $quote -eq '"' -and $ch -eq $esc -and $i + 1 -lt $s.Length) { + # Inside a DOUBLE-quoted span the escape character escapes the next one, so the quote after + # it does not close the span. DELIBERATELY NOT APPLIED INSIDE A SINGLE-QUOTED SPAN, ON BOTH + # HOSTS AND FOR THE SAME REASON: sh gives the backslash no special meaning there, and a + # PowerShell single-quoted string is fully literal too. MEASURED on pwsh 7.6.4 -- + # `Write-Output 'a`' ; 111*3 ; Write-Output 'b`'` prints 333, so the middle RUNS and the + # span really does close at that apostrophe. Treating the two alike would swallow the rest + # of the line from a trailing escape -- fail-open, the direction this function exists to + # avoid. $i++ } elseif ($ch -eq $quote) { @@ -831,7 +934,15 @@ function Remove-QuotedSpans([string]$s, [bool]$PosixEscapes = $false) { function Get-FlagOwner([string]$Left) { <# WHICH PROGRAM OWNS THE FLAG THAT WAS JUST MATCHED, and does it EXECUTE its argument? - (BACKLOG #1229 residual, fourth round.) Returns 'posix', 'win' or 'none'. + (BACKLOG #1229 residual, fourth round.) Returns 'posix', 'pwsh', 'cmd' or 'none' -- the name of an + ESCAPE CONVENTION, which is what the caller actually needs, not a family label. + + THE WINDOWS FAMILY IS SPLIT AND THAT IS NOT COSMETIC (residual, fifth round). This returned a + single 'win' for pwsh, powershell, cmd and wsl, which was harmless while 'win' meant "no escapes" + -- and stopped being harmless the moment PowerShell got its backtick rule, because cmd.exe escapes + with `^` and nobody has measured it here. Folding cmd in with pwsh would have handed it an escape + it does not have, which lengthens spans and hides commands: a fail-open, manufactured by tidiness. + `cmd` therefore keeps the reading it has today, byte for byte. THE FLAG SHAPE IS NOT THE QUESTION, AND IT IS BARELY CORRELATED WITH THE ANSWER. `$shFlag` is `-[a-z]*c` under (?i), which matches `-C`, `-ic`, `-rc`, `-static`, `-sync`, `-exec` -- and @@ -869,8 +980,12 @@ function Get-FlagOwner([string]$Left) { IDENTITY, and this function does not close them: an unknown name gets no recursion, which is the disclosed cost recorded at the caller. Four of five, not five of five. #> - # Both hosts take their code under `/c` as well as `-Command`, so a cmd-family match is theirs. - $winSet = @('pwsh', 'powershell', 'cmd', 'wsl') + # Both PowerShell hosts take their code under `/c` as well as `-Command`, so a cmd-family FLAG + # match still belongs to them -- which is why the flag shape cannot decide the convention and the + # PROGRAM NAME has to. + $pwshSet = @('pwsh', 'powershell') + # Kept apart from the two above, with no escape rule of its own. See the docstring. + $cmdSet = @('cmd', 'wsl') # Anything that runs the string it is handed. `find` is NOT optional: `-exec` ends in `c`, so the # matcher reaches it, and `find . -name x -exec '' \;` really executes -- dropping find from # this list was measured to regress it from DENY to ALLOW. @@ -896,14 +1011,29 @@ function Get-FlagOwner([string]$Left) { # cannot be a blanket "starts with a slash" skip. if ($t -match '^/[^/]*$') { continue } if ($t -match '^[A-Za-z_][A-Za-z0-9_]*=') { continue } # FOO=1, an assignment prefix - $name = ($t -split '[\\/]')[-1] -replace '(?i)\.exe$', '' - if ($winSet -contains $name.ToLowerInvariant()) { return 'win' } - if ($posixSet -contains $name.ToLowerInvariant()) { return 'posix' } + # Casefolded ONCE. Splitting the Windows family in two added a third membership test, and + # each one used to re-lowercase the name -- an allocation per token, per leftward scan. + $name = (($t -split '[\\/]')[-1] -replace '(?i)\.exe$', '').ToLowerInvariant() + if ($pwshSet -contains $name) { return 'pwsh' } + if ($cmdSet -contains $name) { return 'cmd' } + if ($posixSet -contains $name) { return 'posix' } } 'none' } -function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { +# WHICH ESCAPE CONVENTION DOES THE TOOL'S OWN COMMAND LINE USE (BACKLOG #1229 residual, fifth round). +# The tool name is the only evidence available about the outer line, and it is good evidence: the Bash +# tool's line really is sh, and the PowerShell tool's really is PowerShell. An unrecognised name gets +# 'none', which escapes nothing -- see Remove-QuotedSpans on why guessing is the expensive move. +function Get-HostConvention([string]$ToolName) { + switch ($ToolName) { + 'Bash' { 'posix' } + 'PowerShell' { 'pwsh' } + default { 'none' } + } +} + +function Get-ScannableSegments([string]$Cmd, [string]$Convention = 'none') { # Fold line continuations FIRST, or the per-line split below separates `git \` from its verb and the # rule stops seeing the command at all. Prose does not end a line with a continuation character, so # this does not resurrect the `echo about to merge stuff` false positive. @@ -967,12 +1097,34 @@ function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { # main"` is the shape sitting next to it that must keep allowing). It is the one residual this # audit ADDED to this list rather than inherited. # * More than one level of nesting, unchanged from before (BACKLOG #1066/#1067 record it). - # * A quoted argument SPANNING LINES, because the split above is per line. Both multi-line forms - # deny today anyway: every line of such a span reaches the scanner RAW and the payload line - # carries the git token and the verb by itself. That is an accident of the raw scan rather than a - # property of this function, and a change that blanks message bodies removes it (BACKLOG #1086). - # It is left alone here because no test on THIS gate could tell the two mechanisms apart, and a - # fix whose green nobody could watch fail is not evidence. + # * A quoted argument SPANNING LINES, because the split above is per line. **THIS ENTRY USED TO + # SAY "Both multi-line forms deny today anyway", AND THAT IS MEASURED FALSE.** The claim is + # corrected in place rather than deleted, because it is a compensating control resting on a + # false premise: it told the next reader the class was harmless, so nobody probed it. + # + # What it got right is that every line reaches the scanner RAW. What it missed is that the + # payload line does not only carry the git token -- it can also carry ONE QUOTE FROM EACH of the + # two multi-line spans around it, and those two pair ACROSS the gated command and blank it. + # Which is #1229's straddle exactly, reached through the line split instead of the pass order. + # MEASURED on the shipped gate, cwd inside the governed repo, with the middle statement pinned + # to whether it RUNS (`expr 111 \* 3` and `111*3` -> 333, so an echo-back proves nothing): + # + # echo 'ab' ; git -C checkout main ; echo 'cd' bash 333 ALLOW + # echo "ab" ; git -C checkout main ; echo "cd" bash 333 ALLOW + # Write-Output 'ab' ; git -C reset --hard ; ... pwsh 333 ALLOW + # Write-Output "ab" ; git -C reset --hard ; ... pwsh 333 ALLOW + # + # THE FOURTH ROW WAS MISSING AND THE COUNT WENT OUT AS THREE. Both tools times both quote + # characters is four; the double-quoted PowerShell row measures identically and was simply + # never probed. Read the number as AT LEAST four -- nothing here ranged over the whole input + # space, so it is a floor rather than an enumeration. + # + # STILL NOT FIXED HERE, and now for a stated reason rather than a false one: closing it means + # carrying quote state ACROSS the split, which changes what every rule sees on every multi-line + # command -- a far wider blast radius than the span-ownership fix this function is. Filed as + # BACKLOG #1429 with the rows above, and pinned as a tripwire in + # tests/test_worktree_gate_quote_straddle.py so the ALLOW is KNOWN rather than assumed absent. + # BACKLOG #1086's message-flag blanking is a different change and does not close this. # # COST, measured rather than assumed: recursion only ADDS a scan line, and a line still needs a git # token AND a gated verb to deny, so a path argument behind a family flag (`git -C ""`, @@ -1152,34 +1304,54 @@ function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { # The payload is a NAMED group. It was Groups[1], which still resolves correctly (.NET numbers # unnamed groups before named ones) -- but only by an ordering rule no reader should have to know, # now that $sigil contributes a named group of its own. + # THE EXTRACTION MUST AGREE WITH THE BLANKING ABOUT WHERE THE ARGUMENT ENDS + # (BACKLOG #1229 residual, third round). `[^"]*` is escape-BLIND: it stops at the first + # quote, INCLUDING an escaped one. Once Remove-QuotedSpans became escape-AWARE, the two + # disagreed -- and the inner code was never re-scanned: + # + # bash -c "bash -c \\"git -C reset --hard\\"" + # extraction got: `bash -c \\` -- truncated at the escaped quote, no verb + # blanking removed: the whole span -- so nothing reached any rule -> ALLOW + # + # MEASURED: main DENY x3, the escape-aware fix ALLOW x3, and the control (same nesting, + # NO escape) DENY on both -- so the trigger is the ESCAPE, not the nesting. The inner + # command really runs: `bash -c "bash -c \\"expr 111 \\* 3\\""` prints 333. + # + # ON MAIN THE TWO AGREED BY ACCIDENT, both being escape-blind, which left the verb visible + # OUTSIDE the span. Making one side escape-aware removed the accident without replacing it. + # This is why a host flag alone cannot close it: the failing host is BASH, where the escape + # is real and honouring it is correct. + # + # DERIVED FROM Get-EscapeChar RATHER THAN SPELLED OUT (residual, fifth round). Three literal + # patterns used to stand here, one per convention -- correct, and a second copy of the escape + # table 500 lines from the first. Round 3 above is what that costs when the copies drift, so the + # copy is removed instead of being watched: this pattern is now UNABLE to disagree with the + # scanner, rather than merely observed to agree. Verified byte-identical to the three literals it + # replaces, all four conventions. + # + # The SINGLE-quoted arm stays escape-blind on purpose, on BOTH hosts: sh gives the backslash no + # special meaning inside a single-quoted word and a PowerShell single-quoted string is fully + # literal, which is the same asymmetry Remove-QuotedSpans keeps. + # + # HOISTED OUT OF THE PER-LINE LOOP: it depends only on $Convention, a parameter. + $extractEsc = Get-EscapeChar $Convention + $dqCode = if ($extractEsc -eq [char]0) { + '(?[^"]*)' + } else { + $e = [regex]::Escape($extractEsc) + "(?(?:${e}.|[^`"${e}])*)" + } + $inner = @() foreach ($ln in $lines) { - # THE EXTRACTION MUST AGREE WITH THE BLANKING ABOUT WHERE THE ARGUMENT ENDS - # (BACKLOG #1229 residual, third round). `[^"]*` is escape-BLIND: it stops at the first - # quote, INCLUDING an escaped one. Once Remove-QuotedSpans became escape-AWARE, the two - # disagreed -- and the inner code was never re-scanned: - # - # bash -c "bash -c \\"git -C reset --hard\\"" - # extraction got: `bash -c \\` -- truncated at the escaped quote, no verb - # blanking removed: the whole span -- so nothing reached any rule -> ALLOW - # - # MEASURED: main DENY x3, the escape-aware fix ALLOW x3, and the control (same nesting, - # NO escape) DENY on both -- so the trigger is the ESCAPE, not the nesting. The inner - # command really runs: `bash -c "bash -c \\"expr 111 \\* 3\\""` prints 333. - # - # ON MAIN THE TWO AGREED BY ACCIDENT, both being escape-blind, which left the verb visible - # OUTSIDE the span. Making one side escape-aware removed the accident without replacing it. - # This is why a host flag alone cannot close it: the failing host is BASH, where the escape - # is real and honouring it is correct. - # - # The SINGLE-quoted arm stays escape-blind on purpose: sh gives the backslash no special - # meaning inside a single-quoted word, which is the same asymmetry Remove-QuotedSpans keeps. - $dqCode = if ($PosixEscapes) { "(?(?:\\.|[^`"\\])*)" } else { "(?[^`"]*)" } - foreach ($pat in @( - "(?i)(?:^|\s)$flagThenSep`"$dqCode`"" - "(?i)(?:^|\s)$flagThenSep'(?[^']*)'" + # WHICH ARM MATCHED IS RECORDED, because only the double-quoted one can carry an outer escape. + # A single-quoted word is fully literal on BOTH hosts, so its payload already IS what the + # interpreter receives and re-decoding it would corrupt a legitimate backslash or backtick. + foreach ($spec in @( + @{ Pat = "(?i)(?:^|\s)$flagThenSep`"$dqCode`""; Escaped = $true } + @{ Pat = "(?i)(?:^|\s)$flagThenSep'(?[^']*)'"; Escaped = $false } )) { - foreach ($m in [regex]::Matches($ln, $pat)) { + foreach ($m in [regex]::Matches($ln, $spec.Pat)) { # WHO OWNS THIS FLAG DECIDES BOTH QUESTIONS -- whether to recurse at all, and under # WHICH ESCAPE CONVENTION (BACKLOG #1229 residual, fourth round). `(?:^|\s)` consumes # the separator, so $m.Index lands on the whitespace before the flag and the text left @@ -1195,7 +1367,7 @@ function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { # accidental rather than designed, but it is a deliberate move against origin/main. if ($owner -eq 'none') { continue } # THE CONVENTION MUST COME FROM THE INTERPRETER, NOT THE OUTER TOOL NAME, and that was - # a live fail-open. `$PosixEscapes` is decided once from the tool name at each call + # a live fail-open. The convention is decided once from the tool name at each call # site, so a Bash tool call invoking pwsh applied POSIX backslash rules to a PowerShell # payload; the span straddled `C:\Temp\` and swallowed the gated command between it and # a later quote. MEASURED to really run, with a payload that COMPUTES (marker 333): @@ -1204,12 +1376,20 @@ function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { # under `bash -c` is INERT on this host (bash reports an unterminated quote), so the # ALLOW there is CORRECT -- the same characters have opposite right answers depending # on which interpreter receives them, which is why one flag for the whole line cannot - # express it. `win` maps to $false, which is also the direction the parameter's own - # docstring names conservative: shorter spans, more text left visible, fail CLOSED. + # express it. + # + # `$owner` IS PASSED THROUGH WHOLE rather than collapsed to a bool, which is the fifth + # round's change here. It used to become `Posix = ($owner -eq 'posix')`, and that bool + # could say only "sh" or "not sh" -- so a `pwsh` payload and a `cmd` payload arrived + # identical, and PowerShell's own backtick escape had nowhere to live. # # The EXTRACTION regex above keeps the OUTER convention on purpose: it is parsing the # OUTER command line's quoting, and that line really is the outer host's. - $inner += [pscustomobject]@{ Text = $m.Groups['code'].Value; Posix = ($owner -eq 'posix') } + $inner += [pscustomobject]@{ + Text = $m.Groups['code'].Value + Conv = $owner + Escaped = [bool]$spec.Escaped + } } } } @@ -1228,16 +1408,57 @@ function Get-ScannableSegments([string]$Cmd, [bool]$PosixEscapes = $false) { # with a bare token -- verb and arguments gone, nothing left for any rule to match. Ownership # cannot be decided by a regex that has no idea which quote opened first, which is the same # sentence this file already wrote about the blanking order. - $s = Remove-QuotedSpans $line $PosixEscapes + $s = Remove-QuotedSpans $line $Convention [pscustomobject]@{ Raw = $line; Scan = $s } } # Each extracted payload carries ITS OWN convention, taken from the interpreter that was matched # rather than from the tool name at the call site. See the note at the extraction above. foreach ($item in $inner) { - $s = Remove-QuotedSpans $item.Text $item.Posix + $s = Remove-QuotedSpans $item.Text $item.Conv [pscustomobject]@{ Raw = $item.Text; Scan = $s } } + + # ================================================================================================= + # THE SAME PAYLOAD AGAIN, IN THE ENCODING ITS INTERPRETER ACTUALLY RECEIVES + # (BACKLOG #1229 residual, SIXTH round -- a fail-open THE FIFTH ROUND'S OWN FIX INTRODUCED). + # + # THE DEFECT. The loop above scans the extracted text under the INNER interpreter's convention, + # but that text is still in the OUTER host's encoding -- so the same characters get read twice, + # under two different conventions, and the pairing shifts. #1229's own straddle, one level in, + # arriving through the door built to keep it out. MEASURED against gate copies hash-verified + # byte-identical to `origin/main` and to the fifth round, cwd inside the governed repo, with the + # middle statement pinned to whether it RUNS (an inert marker computing `set /a 111*3` -> 333): + # + # cmd /c "`"git -C reset --hard`"" PowerShell tool main DENY round5 ALLOW RUNS + # cmd /k "`"git -C reset --hard`"" PowerShell tool main DENY round5 ALLOW RUNS + # cmd /c "git -C reset --hard" no backtick, ctl main DENY round5 DENY + # git -C reset --hard positive control main DENY round5 DENY + # + # TWO STEPS ARE NEEDED AND NEITHER IS ENOUGH ALONE, measured: decoding leaves `"git -C + # reset --hard"`, which the scanner then blanks as an ordinary quoted span; unwrapping without + # decoding finds a BACKTICK in first position and does nothing. + # + # THIS IS AN EXTRA VIEW, NOT A REPLACEMENT, AND THAT IS THE WHOLE SAFETY ARGUMENT. Decoding IN + # PLACE is the tidier change and it is REFUTED: it re-opens round 3's nested `bash -c "bash -c + # \"\""` (DENY -> ALLOW), because this function recurses ONE level and the round-3 DENY + # depends on the escaped text staying visible at this level. Two new fail-opens, measured, for a + # change that reads as a correction. Adding a segment cannot do that: every rule here reaches a + # segment through a `continue`-or-deny loop, so an extra segment can only ADD a deny. Appended + # AFTER every existing segment, so rule 3's first-verb-wins bookkeeping sees exactly what it saw. + # + # SCOPED TO `cmd` BECAUSE THAT IS WHAT WAS MEASURED. `Get-FlagOwner` answers `cmd` for cmd and + # wsl alike; the wrapper rule is cmd.exe's, so for wsl this view is an over-approximation, which + # is the harmless direction. Do NOT widen it to the other conventions on the strength of this + # note -- no probe here separates them, and widening a security control by analogy is how the + # rounds above happened. + # ================================================================================================= + foreach ($item in $inner) { + if (-not $item.Escaped -or $item.Conv -ne 'cmd') { continue } + $unwrapped = Remove-CmdWrapperQuotes (Remove-EscapeChars $item.Text $Convention) + if ($unwrapped -ceq $item.Text) { continue } + [pscustomobject]@{ Raw = $unwrapped; Scan = (Remove-QuotedSpans $unwrapped $item.Conv) } + } } try { $hook = [Console]::In.ReadToEnd() | ConvertFrom-Json } catch { exit 0 } @@ -1727,7 +1948,7 @@ if ($tool -in @("Bash", "PowerShell")) { # sibling worktrees and the primary alike. Any git failure falls through to ALLOW. # ----------------------------------------------------------------------------------------------- $dangerKeys = 'core\.hookspath|core\.worktree|alias\.[\w.-]+|include\.path|includeif\.' - foreach ($seg in (Get-ScannableSegments $cmd ($tool -eq "Bash"))) { + foreach ($seg in (Get-ScannableSegments $cmd (Get-HostConvention $tool))) { if ($seg.Scan -cnotmatch $gitInvocation) { continue } # [regex]::Match RATHER THAN `-notmatch`, FOR ITS INDEX (BACKLOG #1065). The pattern STRING is # unchanged; what is new is that the rule can now ask WHERE on the line the disarm sits, which is @@ -2111,7 +2332,7 @@ What to do instead: # entirely. Ask git whether the path is a registered worktree of a governed repo instead. Any git # failure -- a path that is not a worktree, or does not exist -- falls through to ALLOW. # ----------------------------------------------------------------------------------------------- - foreach ($seg in (Get-ScannableSegments $cmd ($tool -eq "Bash"))) { + foreach ($seg in (Get-ScannableSegments $cmd (Get-HostConvention $tool))) { if ($seg.Scan -cnotmatch $gitInvocation) { continue } if ($seg.Scan -cnotmatch '\bworktree\s+(?remove|move)(?=\s|$)') { continue } $wtVerb = $Matches['wtverb'] @@ -2400,7 +2621,7 @@ $cleanupBullet # must not second-guess it -- `cd && git -C rebase` acts on the sibling, and # denying it because the primary's path appears in the `cd` is a false positive. $anyInferredTarget = $false - foreach ($seg in (Get-ScannableSegments $cmd ($tool -eq "Bash"))) { + foreach ($seg in (Get-ScannableSegments $cmd (Get-HostConvention $tool))) { # Match a git invocation however it is spelled: git, git.exe, or an absolute path to either. if ($seg.Scan -cnotmatch $gitInvocation) { continue } if ($seg.Scan -cnotmatch "\bgit(\.exe)?\b[^|;&]*?\s(?$verbs)(?=\s|$)") { continue } diff --git a/tests/test_worktree_gate_escaped_quote.py b/tests/test_worktree_gate_escaped_quote.py index 0c4ccd748..940c7b377 100644 --- a/tests/test_worktree_gate_escaped_quote.py +++ b/tests/test_worktree_gate_escaped_quote.py @@ -42,7 +42,7 @@ import pytest -from tests.test_worktree_gate import assert_denied, run_gate # reuse the subprocess harness +from tests.test_worktree_gate import GATE, assert_denied, run_gate # reuse the subprocess harness # Built by concatenation, matching the sibling straddle suite: a test about quote handling must not # depend on how this file's own literals nest. The escape is spelled once, here, for the same reason. @@ -50,6 +50,7 @@ SQ = "'" ESC_DQ = "\\" + DQ # a BACKSLASH then a quote -- one shell literal, not a span opener ESC_SQ = "\\" + SQ +BT = "`" # PowerShell's escape character. Spelled once, for the same reason as the two above. @pytest.fixture @@ -64,9 +65,14 @@ def repos_file(tmp_path: Path, primary: Path) -> Path: return f -def shell(command: str, cwd: Path) -> dict[str, object]: - """A Bash tool payload, matching the sibling suites' harness.""" - return {"tool_name": "Bash", "tool_input": {"command": command}, "cwd": str(cwd)} +def shell(command: str, cwd: Path, tool: str = "Bash") -> dict[str, object]: + """A tool payload, matching the sibling suites' harness. + + ``tool`` defaults to Bash because most rows here are sh shapes. It is a PARAMETER rather than a + second helper: the host is the variable half of this file's fifth round, and spelling the payload + dict inline at every PowerShell row is how the ``cwd`` convention drifts between neighbours. + """ + return {"tool_name": tool, "tool_input": {"command": command}, "cwd": str(cwd)} # THE VERB SET IS THE POINT OF THIS PARAMETRISATION, not thoroughness for its own sake. The defect is @@ -273,6 +279,26 @@ def test_an_ordinary_command_is_still_allowed(primary: Path, repos_file: Path) - ("PowerShell", _STRADDLE, False, "no 333: middle did NOT run"), # bash printed 333 -> the middle RAN. ("Bash", _STRADDLE, True, "333 printed: middle RAN"), + # --- FIFTH ROUND, the BACKTICK. These two are the must-ALLOW half of that round and they + # --- belong in this table rather than in one of their own: same property, same shape. + # An ODD trailing backtick escape with nothing to re-close it. pwsh reports "The string is + # missing the terminator" and runs NOTHING, so a deny would refuse a line that cannot execute. + # This row is a DENY-to-ALLOW move against the pre-fix gate: a false deny removed, not a hole. + ( + "PowerShell", + f"Write-Output {DQ}a{BT}{DQ} ; {{gated}} ; Write-Output {DQ}x{DQ}", + False, + "ParserError: the string is missing its terminator", + ), + # The backtick straddle's OWN characters under the Bash tool, where a backtick is command + # substitution rather than an escape. bash reports an unexpected EOF and runs nothing, so the + # SAME text has opposite right answers on the two hosts -- which is what the convention buys. + ( + "Bash", + f"echo {DQ}a{BT}{DQ}b{DQ} ; {{gated}} ; echo {DQ}c{BT}{DQ}d{DQ}", + False, + "unexpected EOF while looking for a matching quote", + ), ], ) def test_the_verdict_matches_whether_the_command_RUNS_on_that_host( @@ -285,10 +311,7 @@ def test_the_verdict_matches_whether_the_command_RUNS_on_that_host( A row that only asserted "deny" would be satisfied by a gate that denies everything. """ command = template.format(gated=f"git -C {primary} reset --hard") - result = run_gate( - {"tool_name": tool, "tool_input": {"command": command}, "cwd": str(primary.parent)}, - repos_file, - ) + result = run_gate(shell(command, primary.parent, tool), repos_file) if expect_deny: assert_denied(result), f"{tool}: {measured} -- the gate must see it" else: @@ -296,24 +319,356 @@ def test_the_verdict_matches_whether_the_command_RUNS_on_that_host( def test_an_unknown_host_gets_the_CONSERVATIVE_reading() -> None: - """The default is fail-CLOSED, and the direction is the whole reason it is a default. + """An unrecognised tool name must get NO escape rule, because guessing one has cost twice. - Honouring the escape makes spans LONGER, so it blanks MORE and can hide a command -- fail OPEN. - Refusing it leaves more text visible to the rules -- fail CLOSED. So `$PosixEscapes` defaults to - false and only a host known to use backslash escapes opts in. + Refusing an escape is not universally fail-closed -- the fifth round below measures the opposite + -- so the reason for this default is narrower and it is the right one: a host nobody has MEASURED + gets no rule invented for it. Both regressions in this file's history came from applying one + host's escape to another host's text. Asserted on the SOURCE because the parameter default is the guarantee; a behavioural probe would need a third tool name the gate does not currently accept. """ - gate = ( - Path(__file__).resolve().parents[1] / "scripts" / "hooks" / "worktree_gate.ps1" - ).read_text(encoding="utf-8") - assert "[bool]$PosixEscapes = $false" in gate, ( - "the escape rule must default to OFF: an unrecognised host has to get the reading that blanks " - "less, or a future tool name silently inherits sh semantics (BACKLOG #1229 residual)" + gate = GATE.read_text(encoding="utf-8") + assert "[string]$Convention = 'none'" in gate, ( + "the escape rule must default to 'none': an unmeasured host has to get no escape rule at " + "all, or a future tool name silently inherits some other shell's semantics (BACKLOG #1229 " + "residual)" + ) + assert "Get-HostConvention $tool" in gate, ( + "the convention must be keyed on the host, not left unconditional" + ) + assert "default { 'none' }" in gate, ( + "Get-HostConvention must fall through to 'none' for a tool name it does not recognise" + ) + + +# --- BACKLOG #1229 residual, FIFTH ROUND: refusing the HOST'S OWN escape straddles too ------------ +# +# Round 2 gave PowerShell NO escape rule and wrote down why: "Honouring the escape makes spans LONGER, +# so it BLANKS MORE and can hide a command -- fail OPEN. Refusing it makes spans shorter, leaving more +# text visible to the rules -- fail CLOSED." +# +# THAT IS TRUE OF SPAN LENGTH AND FALSE OF SPAN POSITION, and the difference is a live fail-open. +# Refusing an escape the host really honours does not merely shorten the first span, it SHIFTS every +# pairing after it -- and the shifted pair straddles the gated command and blanks it. #1229's own +# mechanism, arriving through the door built to keep it out. +# +# PowerShell's escape is the BACKTICK. Measured on pwsh 7.6.4 with a payload that COMPUTES (`111*3` +# -> 333) rather than echoes, so an echo-back cannot be mistaken for a run: +# +# Write-Output "a`"b" ; 111*3 ; Write-Output "c`"d" 333 printed: the middle RAN +# Write-Output "a""b" ; 111*3 ; Write-Output "c""d" 333 printed (doubled-quote escape) +# Write-Output "ab" ; 111*3 ; Write-Output "cd" 333 printed (no escape at all) +# Write-Output 'a`' ; 111*3 ; Write-Output 'b`' 333 printed (single-quoted: LITERAL) +# Write-Output "a`" ; 111*3 ; Write-Output "x" ParserError: nothing runs +# the FIRST line's characters under bash unexpected EOF: nothing runs +# +# Every row below is pinned to one of those observations, never to a previous verdict. The two +# must-ALLOW rows live in the host matrix above rather than in a table of their own: they have that +# test's exact shape, and splitting the deny half from the allow half of one property into two tables +# is how the two drift. + + +def _pwsh_backtick_straddle(gated: str) -> str: + """The straddle: one backtick-escaped quote on each side of the gated command.""" + return f"Write-Output {DQ}a{BT}{DQ}b{DQ} ; {gated} ; Write-Output {DQ}c{BT}{DQ}d{DQ}" + + +# THE VERB IS ZIPPED INTO THE ROWS, NOT CROSSED WITH THEM. Crossing two verbs over three rows costs +# six `pwsh` launches for three shapes, and this file already measured why that buys nothing: both +# verbs reach the SAME rule. The defect is upstream of any rule anyway -- it is in the blanking -- so +# varying the verb across the rows keeps both spellings present at half the launches. +@pytest.mark.parametrize( + "tool,wrap,verb", + [ + # The OUTER line of a PowerShell tool call really is PowerShell. + ("PowerShell", None, "reset --hard"), + # ...and an EXTRACTED pwsh payload gets the same convention from Get-FlagOwner, whichever tool + # typed the outer line. Without this row the fix could have been keyed on the tool name alone. + ("Bash", "pwsh -Command", "checkout main"), + ("PowerShell", "pwsh -c", "reset --hard"), + ], +) +def test_a_powershell_backtick_escaped_quote_does_not_hide_a_gated_command( + primary: Path, repos_file: Path, tool: str, wrap: str | None, verb: str +) -> None: + """pwsh printed 333 for this shape, so the middle statement RUNS and the gate must see it.""" + payload = _pwsh_backtick_straddle(f"git -C {primary} {verb}") + command = payload if wrap is None else f"{wrap} {SQ}{payload}{SQ}" + assert_denied(run_gate(shell(command, primary.parent, tool), repos_file)) + + +@pytest.mark.parametrize( + "middle,measured", + [ + # PowerShell's OTHER escape. Naive pairing already covers the same extent for it, so this row + # denies before and after the fix -- it is here to show the straddle above is about the + # BACKTICK and not about escaped quotes in general. + (f"{DQ}a{DQ}{DQ}b{DQ}", "doubled-quote escape: 333 printed"), + # No escape at all. Denies on every version of this scanner, so it separates the rows above + # from a gate that has simply started denying `Write-Output`. + (f"{DQ}ab{DQ}", "no escape at all: 333 printed"), + ], +) +def test_the_controls_that_deny_either_way_still_deny( + primary: Path, repos_file: Path, middle: str, measured: str +) -> None: + """Same shape, same host, no backtick. Both RUN on pwsh and both must be seen.""" + command = f"Write-Output {middle} ; git -C {primary} reset --hard ; Write-Output {DQ}z{DQ}" + result = run_gate(shell(command, primary.parent, "PowerShell"), repos_file) + assert result is not None, f"{measured} -- the gate must see it" + assert_denied(result) + + +def test_the_backtick_is_LITERAL_inside_a_SINGLE_quoted_powershell_span( + primary: Path, repos_file: Path +) -> None: + """THE ASYMMETRY ARM, and without it the fix could be 'honour the backtick everywhere'. + + A PowerShell single-quoted string is fully literal -- the backtick escapes nothing there, so the + span really does close at the apostrophe and the middle statement RUNS. Measured: pwsh printed 333 + for ``Write-Output 'a`' ; 111*3 ; Write-Output 'b`'``. + + Applying the escape inside single-quoted spans would hold the first one open past its real closer, + pair it with the next apostrophe, and blank the gated command -- the same fail-open one span type + over. This is the PowerShell twin of the sh row that keeps ``'a\\'`` closing at its quote. + """ + command = ( + f"Write-Output {SQ}a{BT}{SQ} ; git -C {primary} reset --hard ; Write-Output {SQ}b{BT}{SQ}" + ) + assert_denied(run_gate(shell(command, primary.parent, "PowerShell"), repos_file)) + + +def test_a_backtick_straddle_AROUND_a_cmd_call_is_seen(primary: Path, repos_file: Path) -> None: + """A fourth fail-open the fifth round closes, found while probing the cmd family. + + The straddling quotes sit on the OUTER PowerShell line and the thing between them happens to be a + ``cmd /c`` invocation. Nothing about cmd decides this -- the outer line is PowerShell and its + backtick is what pairs the spans -- but it is pinned because it measured differently from the + shapes above and a class found by accident is the one that goes unrecorded:: + + cmd /c "echo a`"b" & git -C reset --hard & Write-Output "c`"d" + + MEASURED: origin/main ALLOW, this build DENY. The middle really runs -- pwsh started it as + background Job3 under ``&`` and printed 333 under ``;``. + """ + gated = f"git -C {primary} reset --hard" + command = f"cmd /c {DQ}echo a{BT}{DQ}b{DQ} & {gated} & Write-Output {DQ}c{BT}{DQ}d{DQ}" + assert_denied(run_gate(shell(command, primary.parent, "PowerShell"), repos_file)) + + +def test_the_cmd_family_keeps_its_NO_ESCAPE_reading() -> None: + """cmd.exe escapes with ``^``, not with a backtick, and it must not inherit PowerShell's rule. + + ``Get-FlagOwner`` used to answer a single ``win`` for pwsh, powershell, cmd and wsl. That was + harmless while ``win`` meant "no escapes" and became a hazard the moment PowerShell got a rule, so + the set is split. Splitting is the NO-CHANGE option: cmd and wsl get exactly the reading they had. + + **THE REASON THIS USED TO GIVE FOR BEING A SOURCE ASSERTION WAS FALSE, AND IT IS REPLACED BY A + MEASUREMENT.** It said no probe can separate ``cmd`` from PowerShell because the RAW line is + scanned under the OUTER host's convention and carries the git text, so the outer line reaches a + verdict before the payload's convention can matter. That holds only while the payload contains an + unescaped quote. **It fails whenever the payload's quotes are ALL backtick-escaped**: the outer + pwsh scan then treats the whole argument as ONE span, blanks it, and reaches NO verdict -- so the + extracted payload's own convention is the only thing left to decide. A justification that told the + next reader the probe could not exist is a compensating control resting on a false premise, which + is the same defect this file's #1429 tripwire was written for, so it is corrected in place rather + than deleted. + + **THE PROBE THAT SEPARATES THEM, built and measured 2026-09-03** against this build and against a + mutant folding ``cmd`` and ``wsl`` into the PowerShell set, cwd inside the governed repo:: + + probe this cmdfold separates? + cmd /c "`"`"" (PowerShell tool) ALLOW DENY YES + wsl -c "`"`"" (PowerShell tool) ALLOW DENY YES + cmd /c "`"`"" (Bash tool) DENY DENY no, outer conv is posix + cmd /c "" no backtick DENY DENY no (control) + wsl -c "" no backtick DENY DENY no (control) + + The no-backtick controls are what make this a separation rather than a coincidence: only the + escape varies between a separating row and its own control. + + **THE CONCLUSION IS UNCHANGED AND STILL CORRECT.** ``cmd`` must not inherit a backtick: cmd.exe + escapes with ``^``, and handing it an escape it does not have lengthens its spans and can hide a + command inside one. What changed is that the split now rests on a measurement rather than on a + claim that no probe exists. + + **IT IS NO LONGER A SOURCE ASSERTION ALONE (residual, SIXTH round).** The separating probe above + used to read ALLOW on this build, so pinning DENY would have pinned a verdict the gate did not + produce; that ALLOW is closed below and the row now denies either way, which costs the table its + separating power. A DIFFERENT probe replaces it, and this one is the better instrument because it + separates the two conventions in the direction that MATTERS -- it shows the fold LOSING a deny the + gate has today, rather than gaining one:: + + cmd /c 'echo "x`" & & echo "y"' PowerShell tool this DENY cmd-folded ALLOW + cmd /c 'echo "x" & & echo "y"' the control this DENY cmd-folded DENY + + Measured 2026-09-03 against a mutant that folds ``cmd`` and ``wsl`` into ``$pwshSet``, cwd inside + the governed repo, gate copies hash-verified byte-identical to ``origin/main`` and to this build. + The middle statement REALLY RUNS: the inert marker ``set /a 111*3`` prints 333 through both rows, + so neither is a dead probe, and ``&`` is used rather than ``;`` because ``;`` is not a command + separator in cmd. + + **WHY THE FOLD LOSES IT.** A backtick is an ORDINARY CHARACTER to cmd.exe, so the quote after it + really closes the span and the gated command is left in plain view. Reading that quote as escaped + holds the span open ACROSS the gated command and blanks it -- #1229's own straddle, bought by + handing a host an escape it does not have. The PowerShell single-quoted argument is what carries + the backtick through to cmd intact, since a pwsh single-quoted string is fully literal. + """ + gate = GATE.read_text(encoding="utf-8") + assert "$cmdSet = @('cmd', 'wsl')" in gate, ( + "cmd and wsl must stay in their own set: folding them in with pwsh hands them a backtick " + "escape neither has (BACKLOG #1229 residual, fifth round)" + ) + assert "$pwshSet = @('pwsh', 'powershell')" in gate, ( + "only the two PowerShell hosts may carry the backtick convention" + ) + assert "'pwsh' { [char]0x60 }" in gate, ( + "the backtick must be reachable ONLY from the 'pwsh' arm of Get-EscapeChar" + ) + + +def test_a_LITERAL_backtick_in_a_cmd_payload_does_not_hide_a_gated_command( + primary: Path, repos_file: Path +) -> None: + """The BEHAVIOURAL half of the split above, and the arm the cmd-folded mutant fails. + + See ``test_the_cmd_family_keeps_its_NO_ESCAPE_reading`` for the measurement and the mechanism. + WHEN THIS REDS, somebody handed ``cmd`` an escape convention it does not have. + """ + gated = f"git -C {primary} reset --hard" + probe = f"""cmd /c {SQ}echo {DQ}x{BT}{DQ} & {gated} & echo {DQ}y{DQ}{SQ}""" + assert_denied(run_gate(shell(probe, primary.parent, "PowerShell"), repos_file)) + # THE CONTROL, and it is what makes the row above a separation rather than a coincidence: the + # identical shape with the backtick removed. Only the escape varies between the two. + control = f"""cmd /c {SQ}echo {DQ}x{DQ} & {gated} & echo {DQ}y{DQ}{SQ}""" + assert_denied(run_gate(shell(control, primary.parent, "PowerShell"), repos_file)) + # THE ANTI-VACUITY ROW. Two DENY assertions go green against a gate that denies everything, and + # the evidence that they do not -- the cmd-folded mutant ALLOWS the probe -- lives in a scratchpad + # a reader cannot re-run. So the same shape aimed at a NON-governed tree must still ALLOW. + ungoverned = f"git -C {primary.parent / 'Elsewhere'} reset --hard" + benign = f"""cmd /c {SQ}echo {DQ}x{BT}{DQ} & {ungoverned} & echo {DQ}y{DQ}{SQ}""" + assert run_gate(shell(benign, primary.parent, "PowerShell"), repos_file) is None, ( + "the identical shape aimed at an ungoverned tree must ALLOW -- if it denies, the two rows " + "above prove nothing about escape handling" + ) + + +@pytest.mark.parametrize("flag", ["/c", "/k"]) +def test_a_fully_ESCAPED_cmd_payload_is_read_in_the_encoding_cmd_RECEIVES( + primary: Path, repos_file: Path, flag: str +) -> None: + """BACKLOG #1229 residual, SIXTH round -- a fail-open THE FIFTH ROUND'S OWN FIX INTRODUCED. + + **THIS ROW ASSERTED ALLOW UNTIL THE SIXTH ROUND, AS A DELIBERATE TRIPWIRE OVER A LIVE REGRESSION.** + The ALLOW is closed; the history stays because the mechanism is the item's own and the next reader + needs it. Measured 2026-09-03 against gate copies hash-verified byte-identical to ``origin/main`` + and to each build, cwd inside the governed repo, every row pinned to whether the middle statement + really RUNS (the inert marker ``set /a 111*3`` -> 333, so an echo-back proves nothing):: + + cmd /c "`"git -C reset --hard`"" main DENY round 5 ALLOW here DENY RUNS + cmd /k "`"git -C reset --hard`"" main DENY round 5 ALLOW here DENY RUNS + cmd /c "git -C reset --hard" main DENY round 5 DENY here DENY (control) + git -C reset --hard main DENY round 5 DENY here DENY (POSCTL) + + **THE MECHANISM.** The OUTER line is PowerShell, so the outer scan honours the backticks, sees one + span, blanks it and reaches NO verdict. Extraction then asks ``Get-FlagOwner``, which answers + ``cmd``, whose convention is ``none``. But the extracted text STILL CARRIES THE OUTER HOST'S + BACKTICKS -- pwsh has not run, so nothing has consumed them -- and scanning it with no escape rule + pairs the two escaped-quote sequences ACROSS the git command and blanks it. Span ownership + deciding the wrong way, one level in. + + **HOW IT IS CLOSED, AND THE TWO WAYS THAT WERE MEASURED AND REJECTED.** ``Get-ScannableSegments`` + now emits ONE EXTRA SEGMENT for a cmd-owned payload: the same text with the outer host's escaped + quotes resolved and cmd's own ``/c`` wrapper quotes removed -- the encoding cmd.exe actually + receives. Both steps are needed and neither is enough alone, measured. + + * FOLDING ``cmd`` INTO THE PowerShell SET denies these rows and LOSES a deny the gate has + today. See ``test_a_LITERAL_backtick_in_a_cmd_payload_does_not_hide_a_gated_command``. + * DECODING IN PLACE -- replacing the payload rather than adding a view -- re-opens round 3's + nested ``bash -c "bash -c \\"\\""`` (DENY -> ALLOW), because this function recurses ONE + level and that round-3 deny depends on the escaped text staying visible at this level. + + An EXTRA segment cannot do either: every rule reaches a segment through a continue-or-deny loop, + so adding one can only ADD a deny. + """ + gated = f"git -C {primary} reset --hard" + # Spelled once, locally: the run of quote and backtick placeholders below is the whole subject of + # this test, and `{DQ}{BT}{DQ}{gated}{BT}{DQ}{DQ}` is unreadable as an unbroken sequence. + esc_dq = BT + DQ # PowerShell's escaped quote -- a literal `"` that does NOT close a span + command = f"cmd {flag} {DQ}{esc_dq}{gated}{esc_dq}{DQ}" + assert_denied(run_gate(shell(command, primary.parent, "PowerShell"), repos_file)) + # THE CONTROL, and it is what keeps this attached to the ESCAPE rather than to the cmd shape: the + # identical command with NO backticks DENIES, on origin/main and on every build since. Without it + # the row above would pass against a gate that had simply stopped recognising `cmd` altogether. + # + # BOTH FLAGS KEEP THEIR OWN ROW DELIBERATELY, against this file's own zip-don't-cross rule. That + # rule is for dimensions that reach the SAME rule and buy nothing; here both `cmd /c` and + # `cmd /k` were INDEPENDENTLY MEASURED to allow AND to execute (333) before this fix. + assert_denied( + run_gate(shell(f"cmd {flag} {DQ}{gated}{DQ}", primary.parent, "PowerShell"), repos_file) + ) + + +def test_the_extra_cmd_view_is_ADDITIVE_and_cannot_remove_a_deny() -> None: + """The safety argument of the sixth round, made structural rather than remembered. + + THE FIX IS AN EXTRA SEGMENT, NOT A REPLACED ONE, and that is the whole reason it cannot introduce + a fail-open. Rewriting the payload in place is the tidier change and it is MEASURED to re-open + round 3 -- see the test above. So the emission the round-3 pin depends on must stay, and the new + view must be APPENDED after every existing segment, where rule 3's first-verb-wins bookkeeping + cannot see it. + + WHEN THIS REDS, somebody turned the extra view into a rewrite. Re-measure round 3 before agreeing + that is safe. + """ + gate = GATE.read_text(encoding="utf-8") + assert "$s = Remove-QuotedSpans $item.Text $item.Conv" in gate, ( + "the ORIGINAL payload view must still be emitted: the round-3 nested-escape deny depends on " + "the outer-encoded text staying visible at this level (BACKLOG #1229 residual, sixth round)" + ) + assert ( + "$unwrapped = Remove-CmdWrapperQuotes (Remove-EscapeChars $item.Text $Convention)" in gate + ), ( + "the extra cmd view must resolve the OUTER host's escaped quotes before applying cmd's own " + "wrapper rule -- neither step closes the shape alone" + ) + # The extra view must come after the loop that emits the original, never inside it. + original = gate.index("$s = Remove-QuotedSpans $item.Text $item.Conv") + extra = gate.index("$unwrapped = Remove-CmdWrapperQuotes") + assert original < extra, "the extra view must be APPENDED, so segment order is unchanged" + + +def test_the_scanner_and_the_extractor_READ_THE_SAME_ESCAPE_TABLE() -> None: + """The invariant round 3 was a fail-open for, made structural instead of watched. + + THE SCANNER AND THE INTERPRETER-ARGUMENT EXTRACTION MUST NOT DISAGREE ABOUT WHERE A SPAN ENDS. + They did once, measurably (see the round-3 test below). The fix at the time made the two AGREE + while leaving the escape character spelled TWICE -- once as a ``[char]`` in ``Remove-QuotedSpans`` + and once inside a hand-written regex in ``Get-ScannableSegments``, some 500 lines apart. A sixth + host added to one and not the other re-opens the round-3 hole, and a source assertion on each + spelling separately cannot see that gap, because both would still pass. + + So the duplicate is gone rather than watched. ``Get-EscapeChar`` is the only table and the + extraction pattern is DERIVED from it through ``[regex]::Escape``, which makes divergence + unrepresentable rather than merely absent today. Verified byte-identical to the three literals it + replaced, on all four conventions. + + WHEN THIS TEST REDS, somebody re-introduced a second spelling of the escape character. That is + the defect; the test is not the thing to fix. + """ + gate = GATE.read_text(encoding="utf-8") + assert gate.count("function Get-EscapeChar(") == 1, "the escape table must be defined once" + assert "$esc = Get-EscapeChar $Convention" in gate, ( + "Remove-QuotedSpans must read the shared table rather than spelling the character itself" + ) + assert "$extractEsc = Get-EscapeChar $Convention" in gate, ( + "the extraction regex must read the SAME table -- see round 3 in this file's history" ) - assert '($tool -eq "Bash")' in gate, ( - "the opt-in must be keyed on the host, not left unconditional" + assert "[regex]::Escape($extractEsc)" in gate, ( + "the extraction pattern must be DERIVED from the shared character, not re-spelled as a " + "literal: a second literal is what lets the two drift apart silently" ) diff --git a/tests/test_worktree_gate_quote_straddle.py b/tests/test_worktree_gate_quote_straddle.py index 11db30ab8..128b7d902 100644 --- a/tests/test_worktree_gate_quote_straddle.py +++ b/tests/test_worktree_gate_quote_straddle.py @@ -121,6 +121,80 @@ def test_an_ordinary_quoted_commit_message_still_does_not_supply_a_verb( ) +@pytest.mark.parametrize( + "tool,program,quote", + [ + ("Bash", "echo", SQ), + ("Bash", "echo", DQ), + ("PowerShell", "Write-Output", SQ), + # THE FOURTH CORNER, MISSING WHILE THE RECORD CLAIMED THERE WERE THREE. Both tools times both + # quote characters is four. Measured ALLOW on origin/main and on this build, and it RUNS + # (`111*3` -> 333). Its absence is the whole reason "fail-opens remaining: 3" got written down. + ("PowerShell", "Write-Output", DQ), + ], +) +def test_a_quoted_span_CROSSING_A_NEWLINE_is_a_known_open_straddle( + primary: Path, repos_file: Path, tool: str, program: str, quote: str +) -> None: + """A TRIPWIRE OVER BACKLOG #1429. It asserts ALLOW and that is NOT an endorsement. + + ``Get-ScannableSegments`` splits the command on newlines before any quoting is considered, so a + quoted span that crosses a newline is not one span to the gate -- it is an unterminated quote on + one line and a stray quote on the next. The middle line then carries ONE QUOTE FROM EACH + surrounding span, those two pair ACROSS the gated command, and it is blanked. Same straddle this + file exists for, reached through the line split instead of the pass order. + + Measured on the shipped gate, cwd inside the governed repo, with the middle statement pinned to + whether it RUNS (``expr 111 \\* 3`` under bash and ``111*3`` under pwsh both print 333):: + + echo 'ab' ; git -C checkout main ; echo 'cd' 333 ALLOW + echo "ab" ; git -C checkout main ; echo "cd" 333 ALLOW + Write-Output 'ab' ; git -C ... ; Write-Output ... 333 ALLOW + Write-Output "ab" ; git -C ... ; Write-Output ... 333 ALLOW + + ALL FOUR CORNERS ARE HERE, and the fourth is why this comment says so. The record carried three + for a while -- both Bash rows and only the single-quoted PowerShell one -- and the missing + double-quoted PowerShell row made "fail-opens remaining: 3" read as an enumeration when it was a + floor. Re-measured 2026-09-03 against gate copies hash-verified byte-identical to origin/main and + to this build: all four ALLOW on both, all four print 333. Treat the count as AT LEAST four. + + THE GATE'S OWN RESIDUAL LIST SAID THESE DENIED, and that claim is corrected in place there. It is + pinned here rather than left in prose because a residual that lives only in a comment is one + nobody notices closing -- and because the false claim is exactly what stopped anyone probing it. + + NOT FIXED IN THE CHANGE THAT ADDED THIS ROW: closing it means carrying quote state across the + split, which changes what every rule sees on every multi-line command. That is a wider blast + radius than the span-ownership fixes this file covers, so it is filed rather than half-done. + + WHEN THIS TEST REDS, that is the success signal: somebody closed BACKLOG #1429. Delete the row and + invert #1429's banner; do not restore the ALLOW. + """ + gated = ( + f"git -C {primary} checkout main" if tool == "Bash" else f"git -C {primary} reset --hard" + ) + command = f"{program} {quote}a\nb{quote} ; {gated} ; {program} {quote}c\nd{quote}" + assert ( + run_gate( + {"tool_name": tool, "tool_input": {"command": command}, "cwd": str(primary)}, + repos_file, + ) + is None + ), ( + f"the multi-line {quote} span under {tool} now DENIES. That is progress -- BACKLOG #1429 is " + "closed. Delete this row and invert the item's banner; do NOT restore the ALLOW." + ) + # THE CONTROL, and it is what keeps the tripwire attached to the NEWLINE rather than to the whole + # shape: the identical command with the span on one line DENIES, on the shipped gate and on this + # one. Without it the row above would pass against a gate that had stopped seeing `git` entirely. + one_line = f"{program} {quote}ab{quote} ; {gated} ; {program} {quote}cd{quote}" + assert_denied( + run_gate( + {"tool_name": tool, "tool_input": {"command": one_line}, "cwd": str(primary)}, + repos_file, + ) + ) + + def test_an_unterminated_quote_fails_closed(primary: Path, repos_file: Path) -> None: """An unpaired quote must leave the rest of the line VISIBLE, not swallow it.