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

[pepmach]

What broke

Since #7808 merged, shard 3 of Backend Tests fails on every platform — Linux and Windows alike — on nine push-gate tests (test_push_branch_gate.py::TestUnrecognisedOptionsReadProtectively, test_security.py::TestGitPublishSubshellGluing). This is blocking every open PR, and main's own CI run 2d69e0836 fails identically.

The cause is a semantic merge conflict, not any single commit's logic: #7808 was written against a security.py where _git_push_args was a plain whitespace split(), and its token walk does all operator reasoning on raw argv words. Between #7808's CI run and its merge, the subshell-gluing lineage landed an operator-cut argument builder. The squash resolution kept both — so the builder handed the walk pre-cut words, and every reading the walk performs on operator characters went blind:

• extglob @(main) arrived as the phantom refspec @, tagged ambiguous-ref instead of wildcard-refspec
• a branch literally named feature|x (quoted) was cut at the pipe at the segment level and denied as cross-segment evasion
• glued redirections, heredoc <<-, live trailing escapes: all invisible

Both lineages' tests never coexisted green on any tree; #7808's own head fails its own tests once rebased.

The reconciliation

One job per layer, so both suites' invariants hold simultaneously:

• _git_push_args returns RAW argv words. It consumes only what the shell removes from argv: redirection operators, their one-word targets, and redirect-position process substitution (> >(cmd)) through its matching ). Word-position <(cmd) passes through and lands on the ungated branch, joining $( — bash substitutes a /dev/fd path no static reading can check (round 8's exact pin).
• The token walk owns operator semantics: redirect arity, extglob (kept whole by the quote/escape state machine), glued word+operator decomposition, and a live trailing backslash — read off the raw spelling, with protected-name compares still on the cut+dequoted view.
• Anchoring skips a git inside a still-open quote: that is a wrapper's quoted payload (bash -c '(cd x && git push …)'), judged by the nested-payload reading, not this line's program.
• Publish segments split quote-aware (;, |, &&, ||, newline — outside quotes only), so 'feature|x' is one word and stays pushable, while backslash-newline splicing keeps its ungated posture via the live-escape rule.
• Paren-only glue is a subshell edge, not an untrusted split: (cd /tmp; git push origin main) keeps the precise protected-branch-name tag and (git push origin fix/x) stays allowed. One metacharacter-inventory row is updated accordingly — its old expectation (unparseable → ungated) directly contradicted the sibling suite's requirement that feature pushes inside subshells stay allowed, and the precise tag is strictly stronger.

Testing

• The nine broken tests, plus the full security battery: test_push_branch_gate.py, test_security.py, test_denied_commands_security.py, test_deny_guidance.py, test_credential_prefilter.py, test_mcp_cron_security.py, test_hooks_coverage.py, test_denied_commands_authority.py — 3,436 passed.
• CI's exact shard 3 invocation (--splits 4 --group 3): 21,946 passed, zero push-gate failures; the five residual failures are pre-existing dev-box artifacts (/local/home uid ownership in test_service.py, one admission-cost flake) that also fail on an unmodified checkout.
• flake8, mypy --platform linux (1,293 files), black gate (one graduated baseline entry pruned).

Pattern harvest

Rule candidate: a CI check (or merge-queue requirement) that re-runs a PR's own changed test files against the MERGE RESULT whenever src/kiro_crew/security.py is modified on both sides of the merge — the defect class is squash-merge semantic conflicts between security PRs that raced each other's rewrites of the same functions: each PR green on its own base, red combined, and nothing runs the combination until after merge.

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 3044d6617b319a896822a4a78bf2441efbe208a0 — 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.

First-Principles-Verdict: CONCERNS

The fix earns its place — main's CI is red — but it leaves _CMD_SEPARATOR_RE orphaned and plants the same quote-state machine at four sites that must now evolve in lockstep.

What this change ships

Intent: make the push-publish gate green again after two security lineages squash-merged into a semantically conflicting security.py. This is a FIX (main's own CI run fails; DERIVED).

1. _git_push_args hands the walk raw argv words, so extglob/glued operators read correctly — justified (the fix)
2. Publish segments split quote-aware via new _shell_push_segments, so 'feature|x' stays pushable — justified, declared
3. Process-substitution targets consumed quote-aware (_consume_process_substitution), unbalanced → fail closed — justified, declared
4. A git inside a still-open quote no longer anchors the segment — justified, declared
5. (git push origin main) now gets the precise protected-branch tag instead of ungated — changed behavior, declared
6. Word-position <(cmd) moves from ambiguous-deny to the ungated branch (_AMBIGUOUS_EXPANSION_RE shrinks) — declared
7. $$'…' parity fix and live-trailing-escape ungating in the token walk — part of the fix
8. <<- added to _REDIRECT_START_RE — part of the fix
9. ~700 lines of black reformatting plus baseline prune — rides along, sanctioned by AGENTS.md (which asks for it in its own commit)
10. Five new tests pinning the review-round bypasses — justified

Watch

• The plain/double/ANSI-C-parity state machine is now hand-rolled at 4 sites (grepped dollar_run % 2 / _run % 2: security.py:6316, 6524, 6647, 6741), three new here. This PR itself proves the cost: the $$-parity fix had to be applied at all four in lockstep, and _push_token_shell_read's own docstring records that a second copy of this exact machine was once subtracted. The next parity-class bug must be found four times.
• The named root cause (nothing runs the merged combination of two security PRs pre-merge) is explicitly deferred to the "Pattern harvest" rule candidate — this fix sits at mechanism level and says so; fine, but a human should confirm the deferral lands somewhere.

Subtractions

• Delete _CMD_SEPARATOR_RE (security.py:6428). After _git_publish_floor_tags switched to _shell_push_segments, its consumers are zero (grepped _CMD_SEPARATOR_RE: only the definition, one docstring cross-reference, one test comment). Inline the separator vocabulary into _shell_push_segments' docstring and drop the constant.

[FIRST-PRINCIPLES-REVIEWED] 3044d66

[github-actions]

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging 3044d6617b319a896822a4a78bf2441efbe208a0. Adjudication emitted 0 well-formed verdict line(s) for total=1, so GPT's verdict stands.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/security.py:6589 -- Redirect-position process substitution bypasses the publish gate
_AMBIGUOUS_EXPANSION_RE = re.compile(r"\$\(|\$\{||{[^{}](?:,|..)[^{}]}") git push origin feature > >(echo ')'; git push origin main)-> payload extraction misses the nested push, the outer push is allowed, andmainis pushed. Anchor: backend-security-controls Fix: Restore[<>](` to the fail-closed regex until process-substitution payload extraction is quote-aware.
[BLOCK-MERGE] 3044d66
[GPT-REVIEWED] 3044d66

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've traced the finding through the code. Let me verify my trace is complete by confirming the split behavior keeps the nested push in one segment and that args is None deferral never fires.

My analysis confirms the behavior:

For git push origin feature > >(echo ')'; git push origin main):

• The segment splitter (security.py:6499–6512) increments depth on >( and suppresses the ; split at security.py:6527 (depth == 0 guard), so the whole command stays one segment with the nested git push origin main inside it; no anomaly, depth returns to 0.
• _AMBIGUOUS_EXPANSION_RE (security.py:6589) no longer contains [<>]\(, so _AMBIGUOUS_EXPANSION_RE.search(command) at security.py:7281 does not fire — no fail-closed.
• _git_push_args treats > >(...) as a redirect-position process substitution and consumes it whole via _consume_process_substitution (security.py:6853), correctly walking past the quoted ')' and the real closing ) after main, returning only the outer args ['origin', 'feature'].
• Since args is not None (security.py:7284), the _nested_shell_payloads deferral at security.py:7315–7325 never runs. The outer push targets feature (allowed), and the nested git push origin main — which bash actually executes inside the process substitution — is never judged.

The gate allows the command; main (a protected branch) is pushed. This bypasses the publish gate, a backend security/governance control. That is the "bypassing a governance ceiling" class = UNBOUNDED harm, decided without weighing.

HARM: UNBOUNDED — a redirect-position process-substitution payload carrying a protected-branch push escapes the publish gate entirely.
CONDITIONS: split keeps it one segment (security.py:6499,6527), no ambiguity match (security.py:6589,7281), clean outer args so nested payload never judged (security.py:6853,7284,7315).
RECOVERY/COST: none — the push reaches the server; this is a security-control bypass, not weighed.

[ADJUDICATION] 3044d6617b319a896822a4a78bf2441efbe208a0 total=1 uphold=1 downgrade=0
UPHOLD F1 src/kiro_crew/security.py:6589 reason=security-class
[GPT-ADJUDICATED] 3044d6617b319a896822a4a78bf2441efbe208a0

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

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound layer-ownership reconciliation, but it multiplies near-identical hand-rolled quote-state machines whose divergence is the exact defect class being fixed.

Watch

• The diff now carries four parallel shell readers that must agree on quote/escape/ANSI-C semantics (_push_token_shell_read, _shell_push_segments, _consume_process_substitution, the inline entry_open walk in _git_push_args) — the $$' parity fix had to be hand-replicated into each ("ANSI-C detection reads the PARITY of the preceding unescaped $ run" appears three times). Two lineages disagreeing on one reading is what broke main; four copies of one state machine recreate that precondition, and the next divergence fails silently in whichever copy wasn't patched.
• The full-file black reformat (baseline entry pruned) ships in the same commit as ~300 lines of publish-gate semantics, contrary to AGENTS.md's "do it in its own commit" — on the security keystone file, this buries the semantic diff and degrades future bisection of the gate.
• docs/system-specs/modules/security.md documents the publish floor at function granularity (_git_push_args anchoring, the _GIT_PUBLISH_UNGATED posture), and this PR changes the split semantics and flips the subshell posture ((git push origin main): ungated → precise tag; feature pushes in subshells: denied → allowed) with no spec update in the commit.

Suggestions

• Extract one shared quote/escape/ANSI-C state stepper the four walks drive, as an immediate follow-up; land the "Pattern harvest" merge-result CI check rather than leaving it prose.

[DESIGN-REVIEWED] 3044d66

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 3044d6617b319a896822a4a78bf2441efbe208a0 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 3044d66

Verdict parsed from the review's SHA-scoped output markers for commit 3044d6617b319a896822a4a78bf2441efbe208a0.

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

[pepmach]

Closing as redundant: main healed independently via #8712 (raw words to the arity scan — the same core reconciliation), #8719 (fail-closed unreadable process-substitution bodies), #8672 and #8282. Verified on current main: the nine broken shard-3 tests pass (51/51 across both classes), and all five adversarial cases this branch additionally fixed (ANSI $$-parity anchor skip, quoted paren inside a substitution, ANSI phase drift across separators, case grammar inside a substitution, comment inside a substitution) are correctly denied or ungated by main's implementation. Nothing left to merge.