fix(security): hand the push arity scan raw words, not cut ones by pepmach · Pull Request #8712 · kirodotdev/KiroCrew

pepmach · 2026-09-05T08:57:16Z

Root cause

eaa8a45bb (#7808, model publish option arity so the floor tag holds) landed unrebased over d402acdf8 (#7356, close four enforcement bypasses in the bash command gates). Both rewrote the same git push argument tokenizer, in incompatible directions, with no textual conflict — so main went red the moment the second one merged.

This is not a Windows-only failure. The Windows shard was simply the first lane to finish; the six test_push_branch_gate.py failures reproduce on Linux with a plain pytest test/test_push_branch_gate.py, and #8697's Linux Backend Tests (3.12, 3) job showed the identical six. Linux shards 2–4 on main's own run 33953620339 never completed, which is why the first read looked platform-specific.

The mechanism

#7808's option-arity and shell-syntax model is defined over RAW words. It splits each word at its own unquoted operators (_push_token_shell_read), models redirection arity itself (_push_token_redirection), truncates at a word-initial #, and classifies extglob/glob shapes.

#7356 had meanwhile taught the outer parse (_git_push_args) to cut a word at its glued operators and to step over redirections with a second, coarser reading of the same grammar. The outer parse therefore pre-chewed the words the inner model reads, and every shape whose identity lives in the operator lost it:

command	read as	should be
`git push origin>/dev/null`	plain remote `origin`	remote-only push (single-arg)
`git push origin @(main)`	ambiguous ref `@`	extglob wildcard refspec
`git push origin & echo x`	empty token	`&` is a command boundary → bare
`git push origin <<- EOF`	`EOF` as a refspec	`<<-` consumes its delimiter
`git push origin> log`	two positionals	remote-only push (single-arg)
`git push --force>/dev/null origin feature-x`	(no tag at all)	protective fallback

Four of those returned no tag at all — an allow — and git push origin & git push origin2 main lost its bare row. In the other direction, #7808's new whole-segment [<>]\( alternative denied git push origin my-feature > >(tee log.txt), an ordinary feature push whose output is teed, which #7356 explicitly pinned as allowed.

The fix — one decision, one place

_git_push_args returns the RAW spelling of the words it keeps, and takes its redirection arity from _push_token_redirection instead of reading the grammar a second time. That alone fixes <<-, whose - the local reading took for an attached target.
Process substitution is a WORD, not a redirection. A token opening with <( / >( is returned rather than skipped, so -o <(echo) keeps its value and cannot shift the positional split onto the remote. The word-position ungate moves out of _AMBIGUOUS_EXPANSION_RE (which fired on the whole segment) into the scan that sees the tokens surviving redirection removal — so a process substitution the shell removes keeps the precise reading, and one that survives as an argv word is still ungated.
_dequote_token no longer cuts at operators. _cut_at_operator stays where an operator can hide a program name (the git anchor); for an argument it destroyed the evidence the scan reads. mainline)>log still resolves to protected mainline through the piece scan.
Segment and word splitting are quote/escape aware (_split_push_command_segments, _split_shell_words). Splitting inside quotes truncated a word mid-quote, and the fragment then arrived with the shell state open — which the fragment rule reads as a splice across the boundary. That denied legal refnames ('feature|x', 'a;b') and made a wrapper's quoted payload yield a bare git token, so the outer line parsed as a push and its fragmented ref denied ordinary work (bash -c '(cd /tmp && git push origin my-feature)'). A backslash-newline still splits and still keeps its trailing escape: it is the one separator that vanishes, and that escape is the signal the ungated sentinel is drawn from.
A word whose operator tail is only subshell punctuation keeps its exact identity, so a refname pushed inside ( ... ) is read precisely. The word is still scanned as a refspec candidate, so a protected name inside the parens is caught exactly as it is without them.

Nothing is weakened

Every protective assertion holds. Two readings become more precise, and both are pinned in the metacharacter inventory:

(git push origin main) now reports the protected-branch-name row instead of the unparseable-fallback sentinel. The test asserts it equals the row its unparenthesised spelling gets — a spelling-specific row would itself be an escape hatch — and it is still a denial.
A quoted whitespace-spanning option value no longer needs the protective fallback, because it is no longer torn into two fragments. The regression that matters is kept explicit: a protected refspec behind such a value is still seen (--push-option='ci skip' origin main → protected-branch-name).

What was tested

test/test_push_branch_gate.py — 109 passed (103 pre-existing + 6 new).
2,663 passed / 1 skipped across test_push_branch_gate.py, test_security.py, test_denied_commands_security.py, test_denied_commands_api.py — the two suites that pin fix(security): close four enforcement bypasses in the bash command gates #7356's and fix(security): model publish option arity so the floor tag holds #7808's opposing constraints, so both PRs' intents are held simultaneously.
test_security_posture.py and test_security_regex_linearity.py included in the same run (repo-wide drift and ReDoS guards, required because this adds functions under src/kiro_crew).
Red-first proof for every new assertion by restoring the pre-fix helper in-memory: reverting the word split, the segment split, or the operator cut each reproduces the exact failure the assertion now catches.
Gates: isort clean, flake8 clean, mypy --platform linux src/kiro_crew — Success: no issues found in 1293 source files. black --check is clean on test/test_push_branch_gate.py and on every line this PR adds to security.py; that file is listed in .github/black-baseline.txt, so it is exempt from the gate and was deliberately not wholesale-reformatted (doing so adds ~594 lines of unrelated churn to an urgent fix).

New test class

TestTokenizerContractBetweenTheLayers pins the seam in both directions — what the argument scan is handed and by whom — so the two layers cannot silently drift apart again.

Why no screenshot: backend-only change. The touched surfaces are the git push deny-gate matcher in src/kiro_crew/security.py (_git_push_args, _dequote_token, _push_segment_targets_protected, _git_publish_floor_tags) and its unit tests. No dashboard, renderer, CLI output or user-visible UI is involved.

Pattern harvest

Rule candidate: When two PRs edit the same parser and the second merges without rebasing onto the first, a semantic merge conflict lands with no textual conflict — before triaging a red as platform-specific, reproduce it locally on the other platform and check whether that platform's shards actually COMPLETED, because an incomplete shard reads as green in the lane.

Rule candidate: A test whose comment explains WHY a value is expected is the place to detect a layer-contract change — when a rationale ("no clean git token survives", "fragments poison the split") has become false because another PR fixed that layer, the fix is to re-derive the expected value and say which layer now accounts for it, not to restore the old behaviour.

Not generalizable: The specific arity table entries (<<- consuming its delimiter, <(/>( being words rather than redirections, >&- keeping its self-contained reading) are bash grammar facts particular to this matcher and carry no lesson beyond it.

github-actions · 2026-09-05T08:59:27Z

Design Review (Fable 5) — ✅ PASS

Design-level review of 5084a94bec5cdc84fd63ab16e95d78ca86916b44 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Fixes the semantic merge conflict at its root: one shared shell state machine replaces the divergent private copies that let two green PRs compose into a red, permissive main.

Both conflicting commits are confirmed in the base ancestry, the diff's mechanism matches the description exactly (raw words handed to the arity scan, one redirection model, quote-aware splits), and the extra bypasses closed along the way are the same defect class in the same seam, all tightening. The new TestTokenizerContractBetweenTheLayers class pins the inter-layer contract in both directions, which is the guard the original incident lacked.

[DESIGN-REVIEWED] 5084a94

github-actions · 2026-09-05T09:02:51Z

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 5084a94bec5cdc84fd63ab16e95d78ca86916b44 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. Composing the review.

First-Principles-Verdict: CONCERNS

Every item removes a traced defect at cause level, but two riders are undeclared and the change orphans _CMD_SEPARATOR_RE while claiming to retain it.

What this change ships

Intent: unbreak main's six test_push_branch_gate.py failures from the #7808/#7356 semantic merge conflict — a FIX.

Six red push-gate tests go green — the fix, justified.
Teed feature push (> >(tee log.txt)) allowed again — justified (pinned by fix(security): close four enforcement bypasses in the bash command gates #7356).
Quoted refnames/values ('feature|x', -o 'a b') no longer falsely denied — justified.
Wrapped payloads (bash -c '...') no longer denied at the outer line — justified.
Subshell push reports the precise denial row, not the sentinel — justified, pinned to equal the bare spelling.
Quoted-paren boundary-inflation bypass closed — rides along (fixes this PR's own round 1), justified.
ANSI-C quote-desync bypass closed via one shared state machine — rides along, cause-level, justified.
Nested-payload extractor (_substitution_bodies) now quote-aware; >(X=')' git push origin main) denied — undeclared, justified.
_protected_name_in_substitution sees names past a quoted ) — undeclared, justified.
(git push --repo=origin -f) reclassification bypass closed via _classify_word — rides along, justified.

Watch

The description's touched-surfaces sentence ("the git push deny-gate matcher ... and its unit tests") omits items 8–9, which sit on is_denied's substitution path, not the push matcher. Both are derived (each cites a traced allow of a command bash runs), so they earn their place; only the declaration is stale.
The "one machine" cure is push-path-scoped: 4 private quote walkers remain (quote: str | None at security.py:5599, 11659, 12406, 19722). The pre-existing _split_shell_segments docstring records "do not unify" for segmentation policy, and the general quote-model unification is larger than this change — accepted-and-deferred.

Subtractions

Delete _CMD_SEPARATOR_RE (src/kiro_crew/security.py:6487). Grepped _CMD_SEPARATOR_RE across src/: 1 definition, 0 consumers after _git_publish_floor_tags moved to _split_push_command_segments. The new comment "the pattern itself is retained as the separator vocabulary" is contradicted by the code: _SHELL_SEGMENT_SEPARATORS is the vocabulary actually read, so the regex is a dead second spelling that will diverge (it already differs: it carries \n, the tuple does not).

[FIRST-PRINCIPLES-REVIEWED] 5084a94

github-actions · 2026-09-05T09:06:15Z

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5084a94bec5cdc84fd63ab16e95d78ca86916b44 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5084a94

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5084a94bec5cdc84fd63ab16e95d78ca86916b44: <one-sentence reason>

github-actions · 2026-09-05T09:06:52Z

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5084a94bec5cdc84fd63ab16e95d78ca86916b44 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5084a94

Verdict parsed from the review's SHA-scoped output markers for commit 5084a94bec5cdc84fd63ab16e95d78ca86916b44.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5084a94bec5cdc84fd63ab16e95d78ca86916b44: <one-sentence reason>

eaa8a45 (#7808) landed unrebased over d402acd (#7356). Both rewrote the same git-publish argument tokenizer, in incompatible directions, with no textual conflict -- so main went red on nine push-gate tests on every platform, not just Windows. #7808's option-arity and shell-syntax model is defined over RAW words: it splits each word at its own unquoted operators, models redirection arity itself, truncates at a word-initial '#', and classifies extglob and glob shapes. #7356 had meanwhile taught the outer parse to cut a word at its glued operators and to step over redirections with a second, coarser reading of the same grammar. The outer parse therefore pre-chewed the words the inner model reads, and every shape whose identity lives in the operator lost it: 'origin>' read as a plain remote, '@(main)' as the ambiguous ref '@', a lone '&' as an empty token, and '<<-' left its heredoc delimiter behind as a phantom refspec. Four shapes came back with NO tag at all -- an allow -- and 'git push origin & git push origin2 main' lost its bare row. Reconciled so each decision lives in one place: * _git_push_args returns the RAW spelling of the words it keeps, and gets its redirection arity from _push_token_redirection instead of reading the grammar a second time. That alone fixes '<<-', whose '-' the local reading took for an attached target. * Process substitution is a WORD, not a redirection: a token opening with '<(' / '>(' is returned rather than skipped, so '-o <(echo)' keeps its value and cannot shift the positional split onto the remote. The word- position ungate moves out of _AMBIGUOUS_EXPANSION_RE, which fired on the whole segment and so also denied 'git push origin my-feature > >(tee log.txt)' -- an ordinary feature push whose output is teed, and one #7356 explicitly pinned as allowed. * _dequote_token no longer cuts at operators. _cut_at_operator stays where an operator can hide a PROGRAM name (the git anchor); for an ARGUMENT it destroyed the evidence the scan reads. 'mainline)>log' still resolves to protected 'mainline' via the piece scan. * Segment and word splitting are quote/escape aware. Splitting inside quotes truncated a word mid-quote, and the fragment then arrived with the shell state open -- which the fragment rule reads as a splice across the boundary. That denied legal refnames ("feature|x", 'a;b') and made a wrapper's quoted payload yield a bare 'git' token, so the OUTER line parsed as a push and its fragmented ref denied ordinary work. A backslash-newline still splits and still keeps its trailing escape: it is the one separator that VANISHES, and that escape is the signal the ungated sentinel is drawn from. * A word whose operator tail is only subshell punctuation keeps its exact identity, so a refname pushed inside '( ... )' is read precisely. The word is still scanned as a refspec candidate. Option-vs-positional classification lives in ONE function every path shares. The subshell-punctuation branch above originally appended its recovered word to the positional list directly, skipping the classifier -- so '(git push --repo=origin -f)' filed '-f' as the only refspec, it matched no protected name, and the segment came back with NO tags: a force push to a possibly-protected current branch, admitted by adding one parenthesis, while the same spelling without parens is correctly bare. '(git push -f)' also reported the remote-only row instead of bare, the wrong-identity hazard rounds 10, 13 and 14 of #7808 each turned into a bypass. A punctuation strip changes WHERE a word came from, never WHAT it is, so the glued-redirection branch routes through the same classifier rather than relying on its own flag-shaped-prefix guard. The bare-operator fallback still appends its pieces directly, deliberately: it has already marked the split untrusted, so both no-refspec rows fire and every positional is only a refspec CANDIDATE -- a flag landing there can add a tag, never remove one. Shell quoting likewise has ONE state machine. _iter_shell_chars owns it and five consumers drive it: the operator/fragment read, the segment split, the word split, the operator cut, and the substitution-span helper _matching_close_paren. Each used to carry its own copy, and they did not agree, which cost three more bypasses: * the process-substitution BOUNDARY counted parens with str.count, so in "git push origin feature > >(echo '(' ) main" the QUOTED '(' inflated the depth, the real ')' only returned it to 1, and 'main' was swallowed into the substitution -- a protected-branch push ALLOWED; * the WORD SPLIT had no ANSI-C awareness while the boundary walk did, so in "git push origin feature > >(echo $'a\\'b') main" the escaped quote read as a real closer, the next quote reopened, and 'main' fused into one unterminated word the walk could no longer rescue; * the nested-payload EXTRACTOR walked parens on its own, and there a quote-unaware span does not merely mis-size the body -- it loses the nested command outright. "git push origin my-feature > >(X=')' git push origin main)" extracted the body "X='", so the publish of a protected branch inside the substitution was never scanned and is_denied returned None for a command bash executes. Extraction and boundary now read the SAME span, which is the only arrangement in which they cannot disagree. The rule is explicit: a PROVEN-complete substitution is a word, an UNPROVABLE one is ambiguous. When the words run out with the substitution still open ('>(echo main'), or a quote is still open at the end, the push parse returns None and the segment takes the unparseable branch's non-opt-out-able sentinel; for the extractor the fail-closed reading is the whole remainder, since scanning text that is not really in the body can only add findings. _protected_name_in_substitution carried the same private walk and is repointed too. That corrects an earlier audit note on it: its truncation was recorded as over-deny-only, but stopping at a quoted ')' returns "", so a product name hidden after one was never seen -- an UNDER-deny for the self-protection rule. The remaining paren walkers are deliberate, and each fails safe: _substitution_depth_delta closes an argv EARLY on a quoted ')', so the rest is scanned as its own command (over-scan); _operand_span_end spans to the LAST closer by documented design because quoting is already stripped before it runs; _output_redirect_scan can only over-long a target span; _split_shell_segments serves the cd-tracking pass and is documented as not interchangeable; the _find_brace_* and _find_pattern_operand walkers parse find-command grammar, not shell payloads; _has_top_level_alternation and _split_deny_frags parse deny-rule REGEX text. Round 2's subshell-punctuation branch is a character-membership test over the operator tail rather than a depth count, and a quoted paren needs a quote character there, which is not in "()". Nothing is weakened. Every protective assertion holds, and two readings become MORE precise: '(git push origin main)' now reports the protected-branch row instead of the unparseable-fallback sentinel (the same row its unparenthesised spelling gets -- a spelling-specific row would itself be an escape hatch), and a quoted whitespace-spanning option value no longer needs the fallback because it is no longer torn in two. Verified: 127 passed in test/test_push_branch_gate.py, and 2,792 passed / 1 skipped across test_push_branch_gate.py, test_security.py, test_denied_commands_security.py, test_denied_commands_api.py, test_security_posture.py and test_security_regex_linearity.py. Every new assertion was proven red-first against the previous commit: each exploit spelling publishes a protected branch with no tag at all or is allowed outright, an unclosed substitution silently swallows the rest of the segment, the word split disagrees with the boundary walk about where an ANSI-C quote ends, and a name after a quoted ')' escapes the self-protection scan. Fixes the main-wide push-gate breakage blocking every open PR's shard.

buluoray

Approving — this unblocks every open PR in the repo, and the diagnosis holds up against the evidence.

Verified independently before approving

The breakage is real and repo-wide: Backend Tests (3.12, 3) is 9 failed / 21943 passed on an
unrelated frontend PR whose merged tree's test/ and src/kiro_crew/ hashes are byte-identical to
origin/main's (that PR contains zero Python) — so the failures belong to main, exactly as this
description says. Six in test_push_branch_gate.py::TestUnrecognisedOptionsReadProtectively, three in
test_security.py::TestGitPublishSubshellGluing.
The named cause matches the history: 166ff5acc (#8197) committed 06:53:36Z and eaa8a45bb (#7808)
committed 07:03:28Z, both landing on the same tokenizer with no textual conflict. main is red from
07:03:28Z onward.
This PR's green is not a stale green: its Backend Tests (3.12, 3) ran 10:51:53Z → 11:15:40Z,
after the second commit landed. (By contrast #8672's shard 3 started 06:47:04Z, before eaa8a45bb,
so its green predates the breakage and proves nothing.)
It does not buy green by relaxing tests — the decisive check for a "make main green" fix.
test/test_push_branch_gate.py adds 108 assert lines and removes 1, alongside security.py
+540/−200. Every lane is green: 57 pass, 8 skipping, 0 fail; GPT 5.6 and Opus 4.8 report no blocking
findings and Design Review passes.

One note, not a request to change anything

First Principles' CONCERNS is a stale-declaration point, not a defect: its own list judges all ten
shipped items justified, and says items 8–9 (_substitution_bodies becoming quote-aware,
_protected_name_in_substitution reading past a quoted )) "earn their place; only the declaration is
stale." Worth a one-line edit to the touched-surfaces sentence at some point, since those two sit on
is_denied's substitution path rather than the push matcher — but that is documentation, and holding a
repo-wide unblock for it would be the wrong trade.

Merging so the open PRs can go green again.

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Three constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed program word (`> >(/usr/bin/g?t push origin main)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Modelling bash constructs one at a time is unbounded, so the boundary is refused as a class: a body word the walk cannot read -- a word-initial unquoted `#`, an unquoted reserved word, an unquoted glob or expansion character (`$'` excepted, the walk models it) -- returns None and the segment takes the non-opt-out-able sentinel. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Three constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed program word (`> >(/usr/bin/g?t push origin main)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Modelling bash constructs one at a time is unbounded, so the boundary is refused as a class: a body word the walk cannot read -- a word-initial unquoted `#`, an unquoted reserved word, an unquoted glob or expansion character (`$'` excepted, the walk models it), or a nested unquoted `(` (an extglob program `/usr/bin/@(git)` balances the count and resolves only at run time) -- returns None and the segment takes the non-opt-out-able sentinel. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Three constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed program word (`> >(/usr/bin/g?t push origin main)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Modelling bash constructs one at a time is unbounded, so the boundary is refused as a class: a body word the walk cannot read -- a word-initial unquoted `#`, an unquoted reserved word, an unquoted glob or expansion character (`$'` excepted, the walk models it), a nested unquoted `(` (an extglob program `/usr/bin/@(git)` balances the count and resolves only at run time), or an unquoted control operator / `#` inside the word (`>(cat&# fake )` opens a comment after the `&`, and to bash `#` is word-initial after an operator) -- returns None and the segment takes the non-opt-out-able sentinel. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main; so does `>(cat&# fake )`, since `#` is word-initial after an operator), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed or extglob program word (`> >(/usr/bin/g?t push origin main)`, `>(/usr/bin/@(git) ...)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Enumerating the offending metacharacters grew by one per review round, so the rule is an ALLOWLIST: an unquoted body word may consist only of letters, digits and `/ - _ . = :` (the alphabet of an ordinary program invocation) plus the quote delimiters the walk owns, the `$` of an ANSI-C `$'...'`, and a closing `)`; any other unquoted character, or an unquoted reserved word, makes the body opaque and `_git_push_args` returns None so the segment takes the non-opt-out-able sentinel. Known over-block: a redirection or other metacharacter inside the body (`> >(cat >/dev/null)`) is ungated rather than allowed. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main; so does `>(cat&# fake )`, since `#` is word-initial after an operator), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed or extglob program word (`> >(/usr/bin/g?t push origin main)`, `>(/usr/bin/@(git) ...)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Enumerating the offending metacharacters grew by one per review round, so the rule is an ALLOWLIST: an unquoted body word may consist only of letters, digits and `/ - _ . = :` (the alphabet of an ordinary program invocation) plus the quote delimiters the walk owns, the `$` of an ANSI-C `$'...'`, and a closing `)`; any other unquoted character, an unquoted backslash escape (`\g\i\t` reaches the program as `git` while no scanner word spells it), or an unquoted reserved word, makes the body opaque and `_git_push_args` returns None so the segment takes the non-opt-out-able sentinel. Known over-block: a redirection or other metacharacter inside the body (`> >(cat >/dev/null)`) is ungated rather than allowed. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main; so does `>(cat&# fake )`, since `#` is word-initial after an operator), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed or extglob program word (`> >(/usr/bin/g?t push origin main)`, `>(/usr/bin/@(git) ...)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Enumerating the offending metacharacters grew by one per review round, so the rule is an ALLOWLIST: an unquoted body word may consist only of letters, digits and `/ - _ . = :` (the alphabet of an ordinary program invocation) plus the quote delimiters the walk owns, the `$` of an ANSI-C `$'...'`, and a closing `)`; any other unquoted character, an unquoted backslash escape (`\g\i\t` reaches the program as `git` while no scanner word spells it), an unescaped `$` or backtick inside DOUBLE quotes (`"$GIT" push origin main` expands), or an unquoted reserved word, makes the body opaque and `_git_push_args` returns None so the segment takes the non-opt-out-able sentinel. Known over-block: a redirection or other metacharacter inside the body (`> >(cat >/dev/null)`) is ungated rather than allowed. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main; so does `>(cat&# fake )`, since `#` is word-initial after an operator), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed or extglob program word (`> >(/usr/bin/g?t push origin main)`, `>(/usr/bin/@(git) ...)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Enumerating the offending metacharacters grew by one per review round, so the rule is an ALLOWLIST: an unquoted body word may consist only of letters, digits and `/ - _ . = :` (the alphabet of an ordinary program invocation) plus the quote delimiters the walk owns, the `$` of an ANSI-C `$'...'`, and a closing `)`; any other unquoted character, an unquoted backslash escape (`\g\i\t` reaches the program as `git` while no scanner word spells it), an unescaped `$` or backtick inside DOUBLE quotes (`"$GIT" push origin main` expands), or an unquoted reserved word, makes the body opaque and `_git_push_args` returns None so the segment takes the non-opt-out-able sentinel. The same fail-closed answer applies to a redirection whose ATTACHED target opens a paren (`--push-option 2>(cat >/dev/null # fake )`): skipping it as a self-contained redirection left the body's remainder to be read as argv, where a `#` truncated the real refspecs. Known over-block: a redirection or other metacharacter inside the body (`> >(cat >/dev/null)`) is ungated rather than allowed. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

…8719) Follow-up to #8712 (c791f0f), which reconciled #7356 with #7808 and turned main green. Two allow-direction gaps survive it, both found by the GPT 5.6 review lane over eight rounds on #8719 and each verified with a read-only probe against main: * The ANSI-C decision in _iter_shell_chars used a raw `text[i-1] == "$"` lookback, so an ESCAPED dollar (`\$'foo\'`) opened `$'...'`, kept the quote open across a `;`, and hid the publish behind it -- with the segment split and the program anchor now reading this state, a false "still open" is an allow-direction error, not the over-flag the docstring claimed. `$'` now opens ANSI-C only after a literal, unpaired dollar (an odd run; `$$` is the PID parameter). * The process-substitution boundary walk proves the closer for QUOTING only. Constructs outside quoting moved it or hid the program while the count still balanced: a word-initial `#` comments out the `)` after it (`> >(cat >/dev/null # fake )` + newline + `) main` pushed main; so does `>(cat&# fake )`, since `#` is word-initial after an operator), a `case` pattern's `)` is syntax (`> >(case x in x) git push;; esac)` ran the nested bare push), and a globbed or extglob program word (`> >(/usr/bin/g?t push origin main)`, `>(/usr/bin/@(git) ...)`) closed cleanly but resolves to `git` only at run time, so the payload walk that judges the skipped body saw no push. Enumerating the offending metacharacters grew by one per review round, so the rule is an ALLOWLIST: an unquoted body word may consist only of letters, digits and `/ - _ . = :` (the alphabet of an ordinary program invocation) plus the quote delimiters the walk owns, the `$` of an ANSI-C `$'...'`, and a closing `)`; any other unquoted character, an unquoted backslash escape (`\g\i\t` reaches the program as `git` while no scanner word spells it), an unescaped `$` or backtick inside DOUBLE quotes (`"$GIT" push origin main` expands), or an unquoted reserved word, makes the body opaque and `_git_push_args` returns None so the segment takes the non-opt-out-able sentinel. The same fail-closed answer applies to a redirection whose ATTACHED target opens a paren (`--push-option 2>(cat >/dev/null # fake )`): skipping it as a self-contained redirection left the body's remainder to be read as argv, where a `#` truncated the real refspecs. Known over-block: a redirection or other metacharacter inside the body (`> >(cat >/dev/null)`) is ungated rather than allowed. #8712's proven quote-aware walk stays exactly as merged; the rule is layered in front of it, so every shape #8712 pins (quoted parens, ANSI-C escapes, `> >(tee log.txt)`, `2> >(cmd)`) keeps its precise reading. No test assertion was changed. Refs #8695, #8706 (both resolved by #8712).

pepmach requested a review from a team as a code owner September 5, 2026 08:57

pepmach requested a review from iamwhatever September 5, 2026 08:57

github-actions Bot added the readiness: checking Automated validation is still running label Sep 5, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 5, 2026

pepmach force-pushed the fix/push-gate-windows-shard branch from a8c53d0 to f9fcef7 Compare September 5, 2026 09:22

pepmach force-pushed the fix/push-gate-windows-shard branch from f9fcef7 to b0f0c1a Compare September 5, 2026 09:45

pepmach force-pushed the fix/push-gate-windows-shard branch from b0f0c1a to 7cb3d43 Compare September 5, 2026 10:31

pepmach force-pushed the fix/push-gate-windows-shard branch from 7cb3d43 to 5084a94 Compare September 5, 2026 10:50

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 5, 2026

buluoray mentioned this pull request Sep 5, 2026

fix: stop the chat transcript moving on its own #8574

Merged

github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Sep 5, 2026

buluoray approved these changes Sep 5, 2026

View reviewed changes

buluoray merged commit c791f0f into main Sep 5, 2026
65 checks passed

buluoray deleted the fix/push-gate-windows-shard branch September 5, 2026 12:27

github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 5, 2026

CrysisDeu mentioned this pull request Sep 5, 2026

feat(theme): white chat canvas with grey shell in kiro-light #8711

Merged

5 tasks

This was referenced Sep 5, 2026

fix(security): fail closed on unreadable process-substitution bodies #8719

Merged

fix(chat): route error states through ErrorNotice (batch chat-2) #8743

Merged

bolichen97 mentioned this pull request Sep 5, 2026

ci: the GPT review lane cannot tell a reviewer that crashed from one that refused, so a refused diff is permanently unmergeable #8685

Closed

github-actions Bot mentioned this pull request Sep 5, 2026

fix(dashboard): single-source the one-shot approval decision mapping #8742

Merged

bolichen97 mentioned this pull request Sep 5, 2026

Dashboard AutoNudge loop can be stranded active-but-unarmed, with no reconciler and no fire logging #8636

Closed

This was referenced Sep 5, 2026

fix(security): make the push argument boundary shell-faithful #8727

Closed

main is red: #7808 broke test_push_branch_gate.py (6 failures), cascading to every PR #8695

Closed

CrysisDeu mentioned this pull request Sep 5, 2026

flaky: test_the_scan_is_linear_not_quadratic asserts a wall-clock ratio — the site its own siblings cite as the observed false-red (#7321 one site over) #8687

Closed

CrysisDeu mentioned this pull request Sep 5, 2026

feat: four-layer member system prompt for crew DM threads #7235

Merged

bolichen97 mentioned this pull request Sep 5, 2026

fix(dashboard): make edit-resend a real conversation boundary #8431

Merged

jingchaodev mentioned this pull request Sep 5, 2026

feat(dashboard): configurable text link patterns in transcripts #8302

Open

5 tasks

pepmach mentioned this pull request Sep 5, 2026

fix(security): reconcile the publish gate's two merged lineages #8721

Closed

Navigation Menu

Uh oh!

fix(security): hand the push arity scan raw words, not cut ones - #8712

fix(security): hand the push arity scan raw words, not cut ones#8712
buluoray merged 1 commit into
mainfrom
fix/push-gate-windows-shard

pepmach commented Sep 5, 2026

Uh oh!

github-actions Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

buluoray left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pepmach commented Sep 5, 2026

Root cause

The mechanism

The fix — one decision, one place

Nothing is weakened

What was tested

New test class

Pattern harvest

Uh oh!

github-actions Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Uh oh!

github-actions Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

First Principles Review (Fable 5) — 🟡 CONCERNS

What this change ships

Watch

Subtractions

Uh oh!

github-actions Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 4.8 Review — ✅ no blocking findings

Uh oh!

buluoray left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 5, 2026 •

edited

Loading

github-actions Bot commented Sep 5, 2026 •

edited

Loading

github-actions Bot commented Sep 5, 2026 •

edited

Loading

github-actions Bot commented Sep 5, 2026 •

edited

Loading