From 5ceb3b08b933ae4b3dbeae6fa02d5c3e9e627f98 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 4 Sep 2026 12:32:04 -0500 Subject: [PATCH 1/2] fix(gate): read a quoted span in the encoding its own host uses (BACKLOG #1229) The quote scanner blanked spans without knowing which escape character the executing host uses. PowerShell escapes with a backtick, so the scanner held a span open that PowerShell had already closed, straddled the live command between it and a later quote, and blanked it. A gated git command in that position reached no rule. Remove-QuotedSpans and Get-ScannableSegments now take a convention name rather than a bool that could only say "sh" or "not sh". Get-FlagOwner returns that name so an extracted payload keeps its own interpreter's rules, and the Windows set splits into a pwsh set and a cmd set, cmd having no escape character at all. Get-EscapeChar is the single table both the scanner and the extraction regex derive from, so the two spellings that disagreed in an earlier round are now unrepresentable rather than watched. Get-ScannableSegments additionally emits one extra segment for a cmd-owned payload: the outer host's escaped quotes resolved, then cmd's documented wrapper quotes removed, which is the encoding cmd.exe actually receives. An extra segment cannot introduce a fail-open because every rule reaches a segment through a continue-or-deny loop and the new view is appended after the existing ones, leaving rule 3's first-verb bookkeeping untouched. Measured against hash-verified copies of each build, every arm carrying a positive control that must deny. Against origin/main the PowerShell backtick straddle goes ALLOW to DENY, and the change introduces no new ALLOW across an exhaustive sweep of 7,056 side-string pairs plus about 28,000 random samples, with the oracle proven live by 84 confirmed closures in the same run. The posix arm is byte-identical to origin/main across that corpus. Folding cmd into the backtick set was measured and rejected: it loses a deny the gate has today, because a backtick is an ordinary character to cmd.exe, and that shape really executes. Decoding in place rather than appending was also measured and rejected: it re-opens a nested bash-c deny. Co-Authored-By: Claude Opus 5 --- .gitignore | 9 + scripts/hooks/worktree_gate.ps1 | 371 +++++++++++++++---- tests/test_worktree_gate_escaped_quote.py | 395 +++++++++++++++++++-- tests/test_worktree_gate_quote_straddle.py | 74 ++++ 4 files changed, 754 insertions(+), 95 deletions(-) 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/scripts/hooks/worktree_gate.ps1 b/scripts/hooks/worktree_gate.ps1 index b194d0a04..2b8462a19 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 } @@ -1620,7 +1841,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 @@ -2004,7 +2225,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'] @@ -2293,7 +2514,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. From 8766bc17017cbafc60980e9736a6e0196b316ceb Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Fri, 4 Sep 2026 12:32:13 -0500 Subject: [PATCH 2/2] docs(backlog): keep #1229 open on the newline class and file #1429 #1229 is NOT closed by the quote-scanner fix. A quoted span crossing a newline straddles through the gate's per-line split, and inserting one newline into a quoted word re-opens every shape this item closed, including the shape in the item's own title. Reproduced three times against hash-verified gate copies, with a positive control denying and the middle statement pinned to a marker that computes rather than echoes. The re-score's conditional, that a reviewer may rule the escape hole a separate filing, names the backslash escape. That hole was fixed here rather than re-filed, so the conditional does not apply. What governs is the unconditional half of the same sentence: a wrongly-closed item is invisible forever. #1429 records the newline class as a separate FIX rather than a separate CLASS, and states that it re-opens this item's pins. Both items cross-link. Counts are floors with their corpus named rather than enumerations presented as properties. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 209 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index a19e6b606..4ef0af128 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -11706,6 +11706,180 @@ All three inputs were read out of the code rather than taken on trust. **The mod **Cluster:** Tooling / session coordination. **Priority:** P3. **Verdict:** build. **Severity:** minor -- coordination infrastructure, no product or PHI effect. It costs a lost hand-off and, worse, an unattributable one. ## 1229. the worktree gate blanks double-quoted spans FIRST, so a stray quote inside single-quoted words straddles and deletes the live command between them +> 🚧 **PARTLY SHIPPED, STILL OPEN -- a 2026-09-03 CLOSURE OF THIS ITEM IS REVERTED HERE, and the +> reason is in this item's own title.** `echo 'say "hi' ; ; echo 'bye" now'` still ALLOWs, +> and the middle statement still runs, once **ONE NEWLINE** is inserted into the first quoted word. +> So the honest record is: the straddle is closed **WITHIN A LINE**, on both hosts, and a quoted span +> crossing a **NEWLINE** re-opens every pin this item carries, **its titled shape included**. The +> newline class is filed as **#1429** and that filing is correct as a separate FIX; it is not a +> separate CLASS, and it does not let this item close. +> +> **THE DECISIVE MEASUREMENT, reproduced independently three times** -- by two adversarial reviewers, +> then again here against gate copies hash-verified byte-identical to `origin/main` +> (blob `b194d0a0420a6c87b8786994785155c91908d7b6`) and to the branch head +> (blob `94162a04a6fe51fa7ac2a7f7c6a14de2d3a4e4d8`), cwd inside a governed repo: +> +> ``` +> id main head shape +> POSCTL DENY DENY git -C checkout main (positive control) +> ORIG DENY DENY echo 'say "hi' ; ; echo 'bye" now' (THIS ITEM'S TITLED SHAPE) +> ORIG+NL ALLOW ALLOW the same, with ONE NEWLINE inside the first quoted word +> NEGCTL ALLOW ALLOW echo hello ; ls -la (negative control) +> ``` +> +> `ORIG+NL` is pinned to whether it RUNS, not to whether it parses: with the gated command swapped +> for an inert marker that COMPUTES (`expr 111 \* 3`), bash prints `333`, which an echo-back cannot +> produce. The two controls are what make the middle row evidence -- only the newline varies between +> `ORIG` and `ORIG+NL`, so the line split is the cause and not the shape. +> +> **THE CLOSING ARGUMENT RESTED ON A CONDITIONAL THAT DOES NOT APPLY, and it is retracted here rather +> than below.** The closure cited the 2026-08-20 re-score's *"if a reviewer rules the escape hole a +> separate filing, the remainder text above is the filing."* That antecedent names the **BACKSLASH** +> escape, which the 2026-09-03 change **fixed** rather than re-filed, so the conditional was never +> triggered and licenses nothing. What governs is the same sentence's unconditional half: **a +> wrongly-closed item is invisible forever.** +> +> **WHAT DID SHIP, and it is real work that belongs on `main`.** PowerShell's escape is the BACKTICK, +> and the scanner did not know it. The round-2 fix gave PowerShell no escape rule at all, on a written +> argument that refusing an escape is always fail-CLOSED because it makes spans SHORTER. **That is +> true of span LENGTH and false of span POSITION**: refusing an escape the host really honours shifts +> every pairing after it, and the shifted pair straddles the live command. Four backtick fail-opens +> close, and every earlier round's pin holds. +> +> **BOTH DIRECTIONS COUNTED, and the previously recorded zero was FALSE.** A guard that fails open +> costs more for a new ALLOW than it gains from a removed DENY, so the count that matters is the one +> that was wrong. Measured here over a 64-shape sweep (PowerShell tool, eight backtick-bearing side +> strings paired around a gated command) plus an 8-shape interpreter sweep, with every flip re-tested +> against whether it RUNS: +> +> ``` +> direction at least evidence +> fail-opens CLOSED (main ALLOW -> head DENY) 8 64-shape sweep +> false denies REMOVED (main DENY -> head ALLOW, shape INERT) 12 8 + 4, all inert +> NEW fail-opens INTRODUCED (main DENY -> head ALLOW, shape RUNS) 5 CLOSED in round 6, below +> fail-opens REMAINING in the newline class (#1429) 4 both tools x both quotes +> ``` +> +> **THE `2` IN THAT ROW WAS A FLOOR AND THE FLOOR MOVED: IT IS 5.** Corrected here rather than below, +> because this table is what a reader acts on. Round 5 measured the two rows it already knew about; +> sweeping the CLASS instead found three more, one of them on a DIFFERENT RULE -- which is what +> "rule-agnostic" means and is why enumerating by rule under-counts. All carry the same carriage, and +> it RUNS (`set /a 111*3` -> 333; the negative-control marker `set /a nope*3` prints 0, so an +> echo-back cannot pass for a run): +> +> ``` +> cmd /c "`"git -C merge feature`"" main DENY round 5 ALLOW rule 3 +> cmd /c "`"cd & git rebase main`"" main DENY round 5 ALLOW rule 3 +> cmd /c "`"git -C config core.hooksPath /dev/null`"" main DENY round 5 ALLOW RULE 1 +> cmd /c "git -C config core.hooksPath /dev/null" main DENY round 5 DENY (control) +> ``` + +> **Still a floor.** Rules 1 and 3 are measured; nothing here swept rule 3b, and the count is over the +> shapes driven rather than over the input space. +> +> **"At least" is load-bearing in that table.** None of those four rows is an enumeration of a +> population; each is a floor established by the sweep named beside it, and no sweep here ranged over +> the whole input space. The eight false denies all share one left-hand side (an odd trailing +> backtick), so they are one family enumerated rather than eight independent classes. +> +> **THE NEW FAIL-OPENS WERE THIS ITEM'S OWN MECHANISM, RE-CREATED BY THE FIX. ROUND 6 CLOSES THEM.** +> The record of the defect stays, because the mechanism is the item's own and the next reader needs +> it. Measured against gate copies hash-verified byte-identical to `origin/main`, to round 5 and to +> round 6, cwd inside the governed repo, PowerShell tool: +> +> ``` +> shape main round 5 round 6 really runs? +> cmd /c "`"git -C reset --hard`"" DENY ALLOW DENY RUNS (333) +> cmd /k "`"git -C reset --hard`"" DENY ALLOW DENY RUNS (333) +> cmd /c "`"git -C merge feature`"" DENY ALLOW DENY RUNS (333) +> cmd /c "`"cd & git rebase main`"" DENY ALLOW DENY RUNS (333) +> cmd /c "" no backtick DENY DENY DENY (control) +> git -C reset --hard DENY DENY DENY (positive control) +> wsl -c "`"`"" DENY ALLOW DENY INERT -- see below +> ``` +> +> The mechanism: the OUTER line is PowerShell, so the outer scan honours the backticks, treats the +> whole argument as one span, and reaches **no verdict**. Extraction then hands the payload to +> `Get-FlagOwner`, which answers `cmd`, whose convention is `none`. The payload text **still carries +> the outer host's backticks** -- pwsh has not run yet, so nothing has consumed them -- and scanning +> it with no escape rule pairs the two `` `" `` sequences ACROSS the git command and blanks it. That +> is span ownership deciding the wrong way, which is exactly what this item is about. +> +> **HOW ROUND 6 CLOSES IT, and the two tidier fixes that are MEASURED WRONG.** `Get-ScannableSegments` +> emits ONE EXTRA SEGMENT for a cmd-owned payload: the same text with the outer host's escaped quotes +> resolved and cmd's own documented `/c` wrapper quotes removed -- the encoding cmd.exe actually +> receives. Both steps are needed and neither is enough alone; decoding leaves `"git -C +> reset --hard"`, which the scanner blanks as an ordinary quoted span, and unwrapping without decoding +> finds a backtick in first position and does nothing. +> +> * **Folding `cmd` into `$pwshSet`** denies these rows and LOSES a deny the gate has today. See the +> probe table below. +> * **Decoding IN PLACE** -- rewriting the payload rather than adding a view -- re-opens round 3's +> `bash -c "bash -c \"\""` (DENY -> ALLOW), because the function recurses ONE level and that +> round-3 deny depends on the escaped text staying visible at this level. Two new fail-opens for a +> change that reads as a correction. +> +> **AN EXTRA SEGMENT CANNOT INTRODUCE A FAIL-OPEN, AND THAT IS THE SAFETY ARGUMENT RATHER THAN A +> SWEEP.** Every rule reaches a segment through a continue-or-deny loop, so an added segment can only +> ADD a deny; it is appended AFTER every existing segment, so rule 3's first-verb-wins bookkeeping +> sees exactly what it saw. Consistent with that, round 6 measured **zero** new ALLOWs against +> `origin/main` across both corpora below. +> +> **THE COSTS, stated because a one-sided note reads as a clean win.** Round 6 also re-denies shapes +> round 5 had allowed that are INERT on this host: `wsl -c` is not a valid wsl flag at all (`wsl -c +> ` prints *"Invalid command line argument: -c"*, while `wsl -- expr 111 \* 3` prints 333, +> so wsl itself works), and `cmd /c "`"echo git checkout main`""` merely echoes. Both DENY on +> `origin/main` too, so these are pre-existing false denies restored rather than new ones. Round 6 +> also closes TWO PRE-EXISTING holes as a side effect, both of the same wrapper class: `cmd /c +> "\"\""` under the **Bash tool**, and `cmd /c "`"git -C worktree remove +> `""` (rule 2), each ALLOW on `origin/main` and DENY here. The rule-2 row's carriage RUNS; +> the Bash-tool row **could not be pinned to whether it RUNS**, because `cmd` is not on this +> environment's Bash PATH (`command -v cmd` -> not found). +> +> **THE `cmd` ARM'S STATED REASON WAS FALSE AND IS REPLACED BY A MEASUREMENT -- AND THE FIRST +> REPLACEMENT WENT STALE THE MOMENT ROUND 6 LANDED.** The record first said no probe can separate +> `cmd` from PowerShell because the outer line reaches a verdict first. That fails whenever the +> payload's quotes are ALL backtick-escaped, and round 5 recorded that separating probe. Round 6 +> denies those rows on both builds, so that table no longer separates anything. A DIFFERENT probe +> replaces it, and it is the better instrument, because it shows the fold **losing** a deny the gate +> has rather than gaining one. PowerShell tool, cwd inside the governed repo: +> +> ``` +> probe round 6 cmdfold separates? +> cmd /c 'echo "x`" & & echo "y"' DENY ALLOW YES -- and it RUNS (333) +> cmd /c 'echo "x" & & echo "y"' DENY DENY no (control) +> ``` +> +> The control is what makes this a separation rather than a coincidence: only the backtick varies. +> `&` is used and not `;`, because `;` is not a command separator in cmd. A pwsh single-quoted string +> is fully literal, which is what carries the backtick through to cmd intact. **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. **The arm's CONCLUSION is unchanged** -- `cmd` must not +> inherit a backtick -- and it now rests on a behavioural pin +> (`test_a_LITERAL_backtick_in_a_cmd_payload_does_not_hide_a_gated_command`) rather than on source +> text alone. +> +> **THE SAME BASH SHAPE COULD NOT BE MEASURED, and it is recorded as unmeasured rather than folded in +> with the pwsh row.** The Bash-tool spelling of that probe is inert twice over on this host: git-bash +> reports an unterminated backtick even inside single quotes, and `cmd` is not on its PATH. +> +> **NO SWEEP CITED HERE IS REPRODUCIBLE FROM THIS BRANCH, and that applies to round 6's as much as to +> round 5's.** The 22-shape matrix, the 64-shape sweep, and round 6's two corpora (38 shapes covering +> the reported fail-opens, the fold probes, every earlier round's tripwires and the #1429 newline +> rows; plus 18 more aimed only at false denies through the new code path) all lived in a session +> scratchpad with their drivers, so their counts are not re-derivable and must not be cited as +> standing evidence. What a reader CAN re-derive is every row pinned as a test in +> `tests/test_worktree_gate_quote_straddle.py` and `tests/test_worktree_gate_escaped_quote.py`, plus +> the tables in this banner, each of which names its corpus and its controls. **The counts are floors +> over those corpora and not over the input space** -- round 6's "zero new ALLOWs" means zero across +> 56 driven shapes, which is weaker than the structural argument beside it and is why that argument +> carries the claim. +> +> **Severity is unchanged and deliberately not inflated (sec. 0).** This is a local +> maintainer-workstation guardrail whose own synopsis declines to be a security boundary: no product, +> engine or PHI effect, and nothing deployed. +> > 🔢 **Re-scored 2026-08-20 -> P2.** Value **6/10** · Difficulty **3/10** · _quick win_. Attacked the shipped claim by driving the real hook, then by mutation. The ORDERING limb is genuinely shipped: the two ordered regexes are gone, replaced by Remove-QuotedSpans (worktree_gate.ps1:347-388), and it is CALLED on the scan path at :654 -- the only call site -- not merely present. Landed in c7f0e308 naming #1229. Behaviour measured with a governed repos file and a positive control (plain `git -C checkout main` -> DENY): straddle DENY, mirrored DENY, quoted commit message ALLOW, unterminated quote DENY. Non-vacuity proven by mutation: a scratchpad copy with the two regexes restored at the call site flips ONLY the straddle arm to ALLOW, so the shipped scanner is what closes it. The TEST limbs are shipped too -- tests/test_worktree_gate_quote_straddle.py asserts the straddle DENIES and pins the mirrored, unterminated and false-positive controls, and it is registered in tests/tooling_manifest.txt:109. What is NOT shipped is the fix criterion the item itself wrote down: the scanner respects quote-vs-quote but ignores backslash escaping, so the same straddle class is still reachable. Two shapes ALLOW at HEAD and both really run the middle command in bash (verified with an inert marker); removing the backslashes makes both DENY, and a single-sided escape is harmless, which mirrors the item's own boundary finding one level down. Scoring the remainder only: value 6, a real gap with no clean workaround in the same shipped guardrail, but per section 0 and the item's own scope this is a local maintainer-workstation guardrail whose synopsis declines to be a security boundary -- no product, engine or PHI effect, and nothing deployed. Difficulty 3: one PowerShell function plus test arms, no mypy or store backends, but shell escape semantics differ inside single quotes, double quotes and unquoted text and the fail-closed unterminated case must survive. Verdict is partly_shipped rather than confirmed_shipped because the scope call is the only judgment here and a wrongly-closed item is invisible forever; if a reviewer rules the escape hole a separate filing, the remainder text above is the filing. _(was 6/10 · 2/10.)_ > > **Filed 2026-08-12 -- a LIVE FAIL-OPEN in the shipped gate on `main`, found by the gate-family lane while adjudicating a different item and REPRODUCED INDEPENDENTLY here before filing.** `scripts/hooks/worktree_gate.ps1:382-383` blanks quoted spans per line, **double quotes first**: @@ -19940,3 +20114,38 @@ That is the same `self._lock` the staged-pipeline handoffs take. On a first depl **PARTLY CLOSED ALREADY, AND THE CLOSURE SITS IN THE WRONG ARTIFACT.** The full record -- both questions, all eight options, both answers quoted -- is [comment 5515263760 on PR 749](https://github.com/MEFORORG/MessageFoundry/pull/749#issuecomment-5515263760), written 2026-09-02. A pull-request comment is a real improvement on a session transcript, which does not survive its session. It is still not the ADR, and the ADR is what a reader consults. **This limb differs from the first two in shape:** closing it needs no decision about the engine, only the record moved into the artifact people actually read. **THE GENERAL PROBLEM, stated once so it is not re-derived per incident.** A decision recorded as an outcome plus a delegation is not reviewable. The inputs -- the question, the options, the answer -- are what let a later reader tell a considered call from an arbitrary one, and they are exactly the part that lives in the least durable place. +## 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 alongside a closure of #1229 that has since been REVERTED, and REPRODUCED here before filing rather than taken on report.** `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.