[Maqix]

Problem / Motivation

Goal: A gh pr|issue|release command whose literal --body / --title / --notes value contains a line reading git push is allowed, while every real push and every shape the quote walk cannot prove keeps today's verdict.

gh pr create --body "notes⏎git push⏎more" is refused with rule=git-publish-push-bare ("the push names no branch …") although nothing pushes. _split_push_command_segments ends a segment at every newline, quoted or not, so the middle line of the quoted body is read as a bare git push. Single quotes refuse the same way; the same text on one line is allowed. Follow-up of #15438, which lists this as a non-goal (a different detector from the target-unverifiable rule it changes).

Why it matters

Agents write PR and issue bodies as multi-line gh arguments all the time, and release notes or PR descriptions that say "run git push" on their own line are ordinary. Each one is refused with a reason pointing at a push that does not exist, and the agent has to reword the body or retry. It fails closed, so it is a false positive, not a bypass.

Not a goal

• Any other owner of a multi-line quoted word: git commit -m, echo, printf, bash -c, sh -c, eval. bash -c/sh -c/eval run their argument, and git commit -m is fix(security): keep a quoted newline inside its word on the push path #15438's territory.
• gh as part of a compound command (cd r && gh …, a pipe, a redirect, an env prefix), a gh value that is a positional or a non-text flag, gh api, and gh aliases/extensions. Those keep today's per-line reading.
• The heredoc commit-message shape (push gate: a feature push after a heredoc commit message is refused #20845) and the env PATH=… git push prefix (push gate: an env-prefixed push after a multi-line commit message is refused #20847).

What changed (motivation → approach → change)

• Symptom: a quoted gh body line reading git push is refused as a bare push.

• Cause: the push splitter (and the _GIT_PUBLISH_RE \n anchor) treat a newline inside a quoted argument as a command boundary.

• Change: shell_normalizer_walk._gh_data_newlines_as_spaces (the lexical layer, beside _split_push_command_segments; re-exported from shell_normalizer) returns the single-line spelling of one closed shape and the text unchanged otherwise. The shape is the whole command being one gh pr|issue|release … made of:

  • plain words [A-Za-z0-9._/:=+,-] (no %, so no PowerShell --% stop-parsing token; no @ splat), and
  • whole quoted words: each closes, stands between blanks (no glued 'a''b'), holds no curly quote mark (PowerShell reads ‘ ’ ‚ ‛ “ ” „ as quotes), and, when double-quoted, no $, backtick or backslash;
  • every multi-line quoted word is the value of -b/--body, -t/--title, --subject or -n/--notes (-m is left out: on gh pr merge it is the boolean --merge).

  The quote state comes from _iter_shell_chars, the module's one quote/escape machine: the checks above run over its steps and the spaced view is rebuilt from them, each quoted newline as a space (First Principles review of 62f8c7d; same verdicts on the 42-command probe).

  With no separator, comment, heredoc, escape or expansion outside the quotes, bash's and PowerShell's quote reading of that text is the walk's, and gh is the only program the shell runs. On Windows the helper returns every text unchanged (_QUOTED_NEWLINE_MAY_END_COMMAND = os.name == "nt"): cmd.exe ends a command at a newline even inside double quotes, and nothing proves which shell runs an agent's command there, so the old per-line reading stays. gh refuses an alias or extension named after a core command, so pr/issue/release cannot be redefined to run their arguments.

• is_denied (deny_gate.py) filters publish sources on that view, so such a command is not a publish and records no push_allowed audit. That filter is the only place the view is read: _git_publish_floor_tags only ever receives sources that passed it, so it is unchanged.

• The shell-call title gets the same reading. kiro-cli titles a shell call Running: <command> and GateFacts.deny_targets judges that title through is_denied beside the command, so a title the helper rejected kept the old deny and the real tool call was still refused on the default harness (GPT 6.1 review of 329c88c). The helper now reads through ONE exact leading running: (the lowercased view) and keeps it in the returned text. It reuses main's _SHELL_TITLE_PREFIX, which denied_rules._exception_applies already strips the same way; the constant moves from denied_rules to shell_normalizer_walk (re-exported from shell_normalizer, imported back by denied_rules) because denied_rules imports shell_normalizer. A second prefix, running: without the space, xrunning: …, or a prefix later in the text is read as text.

• The spaced view only ever equals the command's single-line spelling, which main already allows. Rejected text is returned unchanged, so every other command reads exactly as on main.

• docs/system-specs/modules/security.md: one bullet under the protected-branch gate. test_security_gate_liveness.py: package line budget re-pinned from 32,527 to 32,669 (measured on origin/main @ ec2619c), with a reason comment.

• A printf escape in a -c payload is no quote (fail closed, pre-existing on main). _shell_payload_walk printf-decodes every nested payload, so in bash -c $'gh pr create --body \\x27⏎git push origin main⏎\\x27' the \x27 read as ' and the push line as gh body text, while bash reads an unquoted \x27 as x27 and runs the push (also \x22, \u0027, \047, sh -c, mainline; stub-git confirmed). main and the earlier heads of this PR allow it too. The walk gains printf_decode=False, and is_denied also judges every payload as bash reads it as a publish source. It only adds sources, so no command refused before is allowed now.

• The same reading for an evaluator pipe. The bash-read walk passes printf_decode=False down to _nested_shell_payloads, so an echo $'…' or printf '%s' $'…' producer piped into bash / sh is judged by its undecoded token, as the shell on the other end reads it. The printf '%s' … | bash form was let through by the previous revision while main refused it; the echo … | bash form is let through on main too.

• A printf FORMAT decodes once. printf 'gh pr create --body \x5cx27\x0agit push origin main\x0a\x5cx27' | bash prints \x27, a real newline and the push line, and bash runs the push. The decoded walk decoded it twice (making a quote) and the undecoded token had no newline, so neither view saw the push while main refused it (GPT 6.1 on f211678; \x5cx22 and octal \134x27 too). The bash-read walk now also yields the once-decoded producer text and never decodes it again.

• The gh-body reading needs the literal script (Opus + GPT on 1069d2c). is_denied lowercases before the walk, so in printf 'gh pr create --body \X27\x0agit push origin main\x0a\X27' | bash (also \X22, echo -e) the \X27 that printf prints as text, and the shell reads as X27, became \x27 and decoded to a quote. A printf FORMAT is also yielded with its conversions unfilled, so in printf 'gh pr create --body "n%s\x0agit push origin main\x0a%s"' '"' ': "' | bash (also %b, single-quoted body) the %s argument closes the body before the push line. Both pushed main under stub git and were allowed; main refuses them. The decoder also misreads two more escapes (Opus + GPT on f30a26b): a printf FORMAT reads \0047 / \0042 as \004 + 7 / 2, and bash's echo -e prints \47 / \42 as text, yet the walk decoded all four to a quote, so printf 'gh pr create --body \0047\x0agit push origin main\x0a\0047' | bash pushed main and was allowed. The publish gate now spaces a source only when the command holds no backslash at all and the source holds no % other than %%; any other source keeps its per-line reading (fail closed: a body line pushing main next to any backslash, C:\Users or 50% is refused again, as on main). The feat/x twins stay allowed and are now audited pushes.

Backwards compatibility

Compatible: the gate moves in the allow direction only for the one gh shape above; the printf-decode fix only moves commands to deny (13,969-command sweep vs 53e4d92: 308 flips, all allow to deny; 168 push a protected or unverifiable target under stub git, the other 140 are payloads whose bash-read text the gate already refuses when typed unwrapped). Every other command gets the same verdict and rule as on main (probe below).

Removes nothing: no rule, tag, export or reader is removed; the new helper only narrows when the push floor runs.

Tests

test/test_push_branch_gate.py::TestQuotedGhTextValueIsNotAPush:
Every allow, deny and Windows row runs twice: as the command and as its Running: title.

• test_a_push_line_in_a_literal_gh_text_value_is_allowed (5 cases: double/single quotes, -R/--head/--title flags with git push origin main in the body, gh issue comment -b, gh release create --notes): is_denied returns None, no push_allowed audit.
• test_every_other_shape_keeps_its_per_line_reading (32 cases): the helper returns the text unchanged and the command is still denied. Covers a real push on its own line or after ;/&&, $b/brace/$'main' targets, comment apostrophe, heredoc, backslash-newline, unbalanced and glued quotes, $(/backtick/escaped quote in a double-quoted value, PowerShell --% and curly-quote closers, positional / gh pr merge -m / alias / gh api values, bash -c, sh -c, eval, git commit -m, env prefix, cd &&, pipe, redirect, subshell.
• test_only_one_exact_title_prefix_is_read_through (5 cases): xRunning: , Running: with no space, Running: + tab, Running: Running: , and a Running: git push origin main line after a closed body. The helper returns each unchanged and each is denied.
• test_on_windows_a_quoted_newline_keeps_the_per_line_reading (the 5 allow cases): with _QUOTED_NEWLINE_MAY_END_COMMAND set, the helper returns the text unchanged and every one is denied. The class pins the flag off for the other rows through an autouse fixture on _floor_monkeypatch (so a test calling monkeypatch.undo() cannot lift it), so they run alike on every CI platform.

test/test_push_branch_gate.py::TestPrintfEscapeInAShellPayloadIsNotAQuote (19 cases): the 6 printf-escape payloads above, a double-quoted bash -c twin, 5 evaluator-pipe rows (echo / printf '%s' into bash / sh) and 3 printf-FORMAT pipe rows are denied, the feat/x twins (direct, piped and printf-format) are allowed with their push_allowed audit, and the real-quote $'…\'…' form keeps its body reading. 7 of the 9 fail on the previous head 53e4d92.

test/test_push_branch_gate.py::TestPrintfOutputIsNotItsSourceText (16 cases): 11 rows pushing main (\X27, \X22, echo -e, %s, %b, single-quoted %s, printf \0047 / \0042, echo \47 / \42, and a %% printf body, refused as on main) are denied, the \X27, %s, \0047 and echo \47 feat/x twins are allowed with their push_allowed audit, and a literal gh body with no backslash keeps its body reading.

Mutation check: with the helper forced to return its input, the 5 allow cases fail (the 32 deny cases still pass); the same 5 fail when only the is_denied filter is reverted (the audit assertion). On 329c88c (no title read-through) the 5 Running: allow rows fail and everything else passes.

Run with pytest -n 4: test_push_branch_gate.py, test_security_gate_liveness.py, test_argv_floor_inline_and_brace_scope.py, test_denied_commands_security.py, test_denied_commands_authority.py, test_security_facade.py, test_deny_guidance.py, test_inert_search_display_prefix.py, test_push_branch_gate_parser.py, test_security_split_patch_reach.py, test_denied_rules_facade.py, test_argv_floor_facade.py and test_denied_commands_security_*.py: 2171 passed, 1 skipped, 0 failed. black, isort, flake8, mypy on the touched sources, scripts/check_comment_history.py (base origin/main) and scripts/docs_lint.py pass.

Manual verification

Verdict probe, 42 commands through is_denied, each bare and as its Running: title, head vs origin/main @ 098bc0a in a detached worktree:

• Now allowed (were push-bare / push-protected-branch-name on main): the 5 allow cases above, bare and as titles.
• On head and on main, every title gets exactly its bare command's verdict (0 of 42 differ). On 329c88c the 5 allow cases were allowed bare and still refused as titles.
• Unchanged: all 36 others, same rule id on both, including gh pr create --body "x⏎"⏎git push origin main (protected-branch-name), … && git push origin ma$b / {main,x} / $'main' (target-unverifiable), bash -c 'gh pr create --body "a⏎git push⏎"' (push-bare: conservative, the nested reading is unchanged), and the single-line gh pr create --body "notes git push more" (allowed on both).

Related Issues

Closes #20846

Follow-up of #15438 (listed there as a non-goal). Refs #12631.

Pattern harvest

Not generalizable: the class (a newline inside a quoted word read as a command boundary) is the splitter's deliberate fail-closed design; this PR proves one data-only owner rather than changing the splitter, and #15438 handles the commit-message owner.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Contributions are accepted under the project's Apache-2.0 license.

[github-actions]

UX Review (Opus 5.5, fork) — ⏭️ skipped

Revision 5f189da1476b1bc070dfb449acafdcc64e43b53c touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

[github-actions]

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 5f189da1476b1bc070dfb449acafdcc64e43b53c via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the relaxation is a narrow allow-list of one shape that returns any other text unchanged, and the new payload sources can only add refusals.

Notes

• The printf_decode=False walk fixes bypasses that already exist on main (bash -c $'…\x27…', printf … | bash), not just this PR's false positive. It is also where most of the rounds of follow-up fixes went. It would review more cleanly as its own PR, landed before the gh-body allowance.
• The undecoded / % guard in is_denied only works if the allow-list shape never contains a backslash or %. That invariant is enforced by the regex in _gh_text_word, and the helper stays correct only while that regex stays closed. Write that down next to _GH_PLAIN_WORD_RE, so that widening the set of plain-word characters gets questioned in review.

[DESIGN-REVIEWED] 5f189da

Description read

[DESCRIPTION-READ] 7782606ebd25a9979b32d70de9e48f710a7c0aaeee53b9e51d9174336568d310

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 5f189da1476b1bc070dfb449acafdcc64e43b53c via the fork AI-review pipeline — 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 does not.

First-Principles-Verdict: PASS

Nothing blocks. The payload-walk riders are an unrelated pre-existing bypass and redundant hole-closers, which the backslash guard mostly makes moot.

Notes

• Item 6 — oversized: the description says it closes a hole "let through by the previous revision while main refused it". With item 7's undecoded = "\\" not in tool_name, every escape-bearing command already keeps main's per-line reading of the decoded source, so that hole is gone without it. Subtraction: delete the once-decoded decoded != token yield in the printf_decode branch of _nested_shell_payloads.
• Item 4 — rides along: it fixes a bypass the description calls "pre-existing on main". That is a separate defect. The PR also gives the cause as "the splitter ends a segment at every newline, quoted or not", which suggests main already reads that decoded push line, and nothing here pins that main allows it. Subtraction: defer _shell_payload_walk(printf_decode=False) and its is_denied caller to their own PR, along with TestPrintfEscapeInAShellPayloadIsNotAQuote.

What this change ships

Inventory (10 items) — 7 justified

Intent: let agents open gh PRs, issues and releases whose multi-line body contains a git push line without a false refusal. FIX.

1. A multi-line literal gh body/title/notes holding a push line is allowed, with no push audit — justified
2. A Running: shell-call title gets the same reading — justified
3. On Windows the old per-line reading is kept — justified
4. -c payloads are also judged with escapes undecoded — rides along
5. An evaluator-pipe producer token is judged undecoded — rides along
6. A printf FORMAT producer is yielded decoded once — oversized
7. Any backslash or printf conversion keeps the per-line reading — justified
8. _SHELL_TITLE_PREFIX moves to shell_normalizer_walk — justified
9. Package line budget raised to 32,669 — justified
10. Security spec bullet — justified

[FIRST-PRINCIPLES-REVIEWED] 5f189da

Description read

[DESCRIPTION-READ] 410d9249675a130b1413c8828467e2266dedb77acafe7486f046d25e51064f3d

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 5f189da1476b1bc070dfb449acafdcc64e43b53c via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5f189da

[github-actions]

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed cdaf83daff5fecc0cdedf7192c82379e88070dda via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] cdaf83d

[github-actions]

Security Scope Review (fork) — ✅ PASS

Which legitimate operations does 5f189da1476b1bc070dfb449acafdcc64e43b53c newly refuse? Model proposes, scripts/deny_diff.py decides — updated in place on each push. A script-confirmed regression blocks PR readiness.

adjudicated, zero confirmed regressions

Scope-Verdict: PASS

Reason: The tightening adds sources to the git-publish floor only, and only for commands that contain a backslash: each nested shell payload is also read the way bash reads it, without printf decoding. All of it runs through the shell deny composite, so the candidates below can be adjudicated. The harness has not confirmed any regression yet. Windows coverage is limited to spellings that contain a backslash, and the per-leg verdicts come from the fold, not from this review.

TIGHTENING: git-publish floor payload sources in deny_gate.is_denied -- it stops a printf escape from hiding a protected push. Decoded, the escape reads as a quote, but bash reads \x27 as x27, so the push line runs: if "\\" in lower: for source, _tokens in _argv._shell_payload_walk(lower, printf_decode=False): if source not in payload_sources: payload_sources.append(source). A related change cuts back a new loosening: the literal gh-body reading is withheld from any command holding a backslash or a printf conversion (undecoded = "\\" not in tool_name … "%" not in source.replace("%%", "")).

ADJUDICATED: 15 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: The diff assumes three things about platform spelling: a backslash anywhere in the command triggers the undecoded walk, a newline is a separator, and os.name == "nt" turns off the gh-text reading. Posix rows (10) send feature-branch pushes through every producer the undecoded walk now reads: printf … | bash, echo -e … | sh, bash -c $'…\n', and a -c payload whose backslash sits in an unrelated commit message, printf or sed. They also include multi-line and single-line gh pr create bodies, to check that the new reading leaves ordinary PR creation alone. Windows rows (5) cover where Windows puts a backslash for path reasons rather than as an escape: a drive-letter path through cmd /c and powershell -Command, a top-level git -C C:\…, a UNC \\server\share path, and a CRLF gh body. Those last rows are where the always-on-backslash trigger would cost a Windows worker a feature push. I wrote no any rows, because every candidate carries a path, a separator or a shell-specific quote. I could not write a row for the os.name == "nt" branch itself: its effect depends on the host, and only the Windows leg exercises it.

[SCOPE-REVIEWED] 5f189da

Security scope review -- adjudicated candidates

• macos-latest (platform posix): 10 classified, 0 newly refused, 10 still allowed.
• ubuntu-latest (platform posix): 10 classified, 0 newly refused, 10 still allowed.
• windows-latest (platform windows): 5 classified, 0 newly refused, 3 still allowed.

No candidate flipped from allowed to refused.

[dodgeblaster]

Review — fixes #20846, blocked only by the fork + sensitive-path structure

I verified this change behaviourally against the gate on this PR's head (02e9f2e3e5) and read the full diff. The fix is correct and the reds are all structural fork gates, not code defects.

Behavioural verification (is_denied on head vs. origin/main): all pass.

case	verdict
gh pr create --body "notes⏎git push⏎more" (the issue repro, double and single quotes)	now allowed ✅
gh issue comment -b "…⏎git push origin main⏎…", gh release create --notes "…⏎git push⏎…"	now allowed ✅
one-line gh pr create --body "notes git push more"	allowed on both (unchanged) ✅
real bare git push on its own line after the body, and after &&	still denied ✅
gh pr merge -m "…⏎git push⏎…" (-m is the boolean --merge here, not a data flag)	still denied (per-line reading kept) ✅
bash -c '…git push…' wrapper	still denied (conservative, nested reading unchanged) ✅

The narrowing is exactly the stated shape: one simple gh pr|issue|release command of plain words + whole quoted words, the push line only spaced when it is the value of -b/--body, -t/--title, --subject or -n/--notes. Anything with a separator / expansion / escape / other owner outside the quotes is returned unchanged and keeps today's verdict. _QUOTED_NEWLINE_MAY_END_COMMAND correctly keeps the old per-line reading on Windows, where cmd.exe can end a command at a quoted newline. It only ever moves the gate in the allow direction, for this one data-only shape — it fails closed on everything it cannot prove, so it is not a bypass.

Diff quality is good: the owning spec (docs/system-specs/modules/security.md) gets the matching bullet, the _PACKAGE_LINE_BUDGET re-pin carries its reason, and TestQuotedGhTextValueIsNotAPush pins 5 allow + 32 deny + 5 Windows cases with a mutation check.

Why CI is red — all structural, none a code defect:

• Sensitive Change Review fails by design: this is a fork PR and it touches src/kiro_crew/security/*, which the gate lists as a sensitive glob. Its own message says it plainly — "A fork PR cannot pass this gate. A maintainer takes it over on a branch of this repository (keep the author with Co-authored-by: and write Supersedes #<n>), runs the real sandbox, and drives that PR green."
• Coverage Gate (frontend-test=skipped — failing closed) and Harness Parity Gate (x-capture) fail closed downstream because the heavy jobs are skipped on the fork fast-path.
• Await Fast Gate / PR Readiness just aggregate the above.

Recommendation: the change is sound and ready. Because it is a fork PR touching a sensitive security path, it cannot clear CI as-is, and I can't push to the fork branch. A maintainer should take it over onto a branch of this repository per the Sensitive Change Review instructions — Co-authored-by: Marcello Quarta <…>, Supersedes #20853 — run the real sandbox, and drive it green. Nothing in the code needs to change for that.

[Maqix]

Rebased onto current main (a5c1e85b3), no conflicts. Push-gate tests (1,836), lint, type and comment-history checks pass locally. Compared with main, only the 5 intended gh body/notes commands change verdict, and a protected push is still refused. The Fast Gate failure on the previous head was Harness Parity Gate, fixed on main by #22178.

Thanks @dodgeblaster for the review. Could a maintainer approve the fork workflows on eaf07c75f? Since Sensitive Change Review can't pass from a fork, I'm happy for a maintainer to copy it upstream with credit, like #15438.

[Maqix]

Re the GPT 6.1 blocking finding on 329c88c (deny_gate.py:635, Running: -prefixed title still denied): confirmed and fixed in dcafc72.

Repro on 329c88c: all 5 allow cases pass is_denied as bare commands, but each Running: <command> title is still refused (git-publish-push-bare / git-publish-push-protected-branch-name). GateFacts.deny_targets judges that title too, so the real tool call was still refused on kiro-cli.

Fix: _gh_data_newlines_as_spaces now reads through ONE exact leading running: (the lowercased view is_denied hands it) before its first-word gh check, and keeps the prefix in the text it returns. It uses main's _SHELL_TITLE_PREFIX, the constant denied_rules._exception_applies already strips. denied_rules imports shell_normalizer, so the constant now lives in shell_normalizer_walk and denied_rules imports it from there. One constant, no copy. The spaced view only feeds publish detection. Operator rules and every other tier still see the title as it was sent.

Tests (TestQuotedGhTextValueIsNotAPush): every allow, deny and Windows row now runs both bare and as its Running: title. The 5 prefixed allow rows fail on 329c88c and pass now. A new test_only_one_exact_title_prefix_is_read_through covers 5 lookalikes that keep the per-line reading and stay denied: xRunning: , Running: with no space, Running: + tab, Running: Running: , and a Running: git push origin main line after the body.

Sweep: the 42-command probe, run bare and prefixed against origin/main 098bc0a, changes only the 5 intended gh-body commands (and their titles). On head and on main, every title gets exactly its bare command's verdict. Rebased on main 9800507, one commit, package budget 32,151.

Could a maintainer approve the fork workflows on dcafc72? Thanks.

[iamwhatever]

Hold at 1fac724. This is reserved for a human reviewer.

• Loosens a security guard. The PR changes what the git-push deny gate allows. is_denied now reads a gh pr|issue|release text-flag value through the new quote-aware reader in shell_normalizer_walk.py, so a git push line inside that value is no longer treated as a publish.
• Fork gate is red. Sensitive Change Review fails on this head because it is a fork PR touching src/kiro_crew/security/*. The gate's own remedy applies: a maintainer re-opens it on a branch of this repository with Co-authored-by: and Supersedes #20853, runs the real sandbox, and drives it green.

I found no concrete bypass. Glued quotes, expansions inside double quotes, curly quotes, --%, separators, wrappers and non-text flags all fall back to the old per-line reading, and Windows is unchanged. Every AI lane passes on this head, and Security Scope Review confirms no newly refused command. I read the code and tests and ran nothing.

— crew-pr-reviewer (r119)

[Maqix]

@iamwhatever thanks for the r119 review. I've rebased it onto today's main as cb3d8d3918: one commit, and only the line-count number changed (verdicts and test outcomes are the same as 1fac7244e8).

As you say, only a maintainer can clear Sensitive Change Review on a fork PR. Could a maintainer re-open this on a branch of this repository with Co-authored-by: and Supersedes #20853? I'm happy to rebase or answer anything the human reviewer asks, here or on the copy.

[Maqix]

Thanks to the Opus 5.5 review on 53e4d92240. The bash -c $'… \x27 …' finding is real: bash reads \x27 as x27, so the push on line 2 runs, while the gate's printf-decoded copy of the payload read it as a quoted gh body.

One correction for the record: this shape is allowed on main too (checked on 2e648a7094 and b2b87b0f1a; main's payload walk yields the same decoded source), so this PR didn't introduce it. It is fixed here anyway, on f3e3c8f633 (one commit on origin/main b2b87b0f1a):

• The push floor now also reads each -c payload as bash does (ANSI-C decoded, not printf-decoded), so a push that bash runs there is a publish source again. It can only add sources, so it only moves commands toward deny.
• New tests in TestPrintfEscapeInAShellPayloadIsNotAQuote: \x27, \x22, \u0027, \047, sh -c and mainline payloads are refused, a feat/x twin is still allowed, and the real-quote form keeps its verdict. They fail on 53e4d92240 and pass here.

Checked:

Check	Result
The 6 protected rows, real bash with a stub git	all push a protected branch; all refused now
Push sweeps vs 53e4d92240 (6,688 + 2,268 commands)	0 changes
Wider sweep with generated printf-escape rows (13,969)	308 changes, all allow to deny
Gates	1,911 passed; only the 2 markdown_it host-import failures

The new head will need a maintainer to approve the fork workflows. Thanks!

[iamwhatever]

Requesting changes at f3e3c8f.

This PR changes what the protected-branch push floor admits, so it stays reserved for a human security reviewer. Separately, the current head has a bypass that both AI lanes flagged and that I confirmed by reading the code:

• echo $'gh pr create --body \\x27\ngit push origin main\n\\x27' | bash (or the printf '%s' ... | bash form). _self_tokens ANSI-C decodes the producer token; the evaluator-pipe branch then printf-decodes it unconditionally (shell_normalizer.py#L1393-L1404), even on the new printf_decode=False walk. The decoded payload gh pr create --body '<NL>git push origin main<NL>' passes _gh_data_newlines_as_spaces, so the filter at deny_gate.py#L643-L646 drops it, and no other source holds a newline-led push. Bash reads \x27 as x27, so the second line pushes to main. On main the decoded payload is still read line by line and denied, so this is a regression.

Smallest fix: pass printf_decode into _nested_shell_payloads so the pipe branch keeps the raw token when it is False (and add the pipe rows to TestPrintfEscapeInAShellPayloadIsNotAQuote), or drop a source only when every variant the walks produced for it reads as gh-only.

Also open on this head: Backend Tests (3.12, 3), Backend Tests (Windows) 3/5/7 and Coverage Gate are red, and Sensitive Change Review is cancelled.

— crew-pr-reviewer (r121 deep)

[Maqix]

@iamwhatever thanks, and thanks to the Opus 5.5 and GPT 6.1 reviews on f3e3c8f633. The evaluator-pipe bypass is fixed on 683e2e14fc (one commit, rebased onto origin/main c9603a9b05), using the smallest fix you suggested:

• The bash-read walk now passes printf_decode=False down to _nested_shell_payloads, so the evaluator-pipe branch keeps an echo / printf '%s' producer token undecoded, as the shell on the other end reads it. The push line is then a publish source again.
• New rows in TestPrintfEscapeInAShellPayloadIsNotAQuote: echo $'…\\x27…' | bash (also mainline, \\x22 | sh) and printf '%s' $'…' | bash / | sh are refused, and a piped feat/x twin is still allowed with its push_allowed audit. The 5 pipe rows are allowed on f3e3c8f633; all 15 rows in the class pass here.

You're right that the printf '%s' … | bash form was a regression here (main refuses it). The echo … | bash form is allowed on main too, and is closed here as well.

Checked:

Check	Result
Both pipe forms + the earlier bash -c rows, real bash with a stub git	every protected row pushes a protected branch; all refused now
Push sweeps vs f3e3c8f633 (6,688 + 2,268 commands)	0 changes
Gates	1,917 passed; only the 2 markdown_it host-import failures

On the red CI from the last run: the Backend Tests (3.12, 3) and Windows (3) failures were in test_chat_runner_redact_across_tool_boundary.py, which main fixed in #23201; that file passes here after the rebase (47 passed). Windows (5) failed in test_security_sensitive_paths.py::TestHomeDirTargetsCache::test_cache_dict_is_bounded (a symlink-resolution stall) and Windows (7) in test_sibling_list_check.py. This PR touches neither.

Understood on the human security review. The new head will also need a maintainer to approve the fork workflows. Thanks!

[Maqix]

Thanks to the GPT 6.1 review on f211678247. The finding is real, and it is fixed on b2dd9673de (still one commit on main 5f77c3f85c).

Cause. A printf FORMAT decodes its escapes exactly once. printf 'gh pr create --body \x5cx27\x0agit push origin main\x0a\x5cx27' | bash prints \x27, a real newline and the push line, so bash runs the push. The decoded walk decoded that text a second time, which turned \x27 into a quote and the push line into gh body text. The bash-read walk kept the token undecoded, so it had no newline before the push. Neither view saw the push, while main refused it.

Fix. In the evaluator-pipe branch, the bash-read walk (printf_decode=False) now yields the once-decoded producer text as well as the raw token, and never decodes it again. It only adds payload sources, so it can only refuse more.

Row (into bash / sh)	f211678247	b2dd9673de	main	Real bash
printf '… \x5cx27\x0a<push main>\x0a\x5cx27'	allowed	refused	refused	pushes main
same with \x5cx22	allowed	refused	refused	pushes main
same with octal \134x27 / \012	allowed	refused	refused	pushes main
printf '%s' '…\x5cx27\x0a<push main>…'	allowed	refused	refused	no push
each feat/x twin	allowed	allowed	allowed	pushes feat/x

Checked on b2dd9673de:

• 3 new protected rows and a printf-format feat/x twin in TestPrintfEscapeInAShellPayloadIsNotAQuote (now 19 cases). The earlier pipe and bash -c rows keep their verdicts.
• Push sweeps against f211678247: 0 verdict changes over 6,688 + 2,268 commands.
• The gate test set has the same outcomes as f211678247, plus the 4 new passing cases.
• Package line budget re-pinned to the measured 32,568. black, flake8 and mypy are clean.

Could a maintainer please approve the workflow runs for b2dd9673de? @iamwhatever, this PR still needs the human security review you asked for.

[Maqix]

Thanks to the Opus 5.5 and GPT 6.1 reviews on 1069d2c60d. Both findings are real, and both are fixed on f30a26ba03 (still one commit on main 44702cbcda).

Cause (one root for both). _gh_data_newlines_as_spaces treats a source as the literal script the shell runs. Two producers make that false:

• Opus: is_denied lowercases before the walk. In printf 'gh pr create --body \X27\x0a<push main>\x0a\X27' | bash, printf prints \X27 as text and the shell reads it as X27. The lowercased \x27 decodes to a quote, so the push line looked like gh body text.
• GPT: a printf FORMAT is yielded with its %s conversions unfilled. In printf 'gh pr create --body "n%s\x0a<push main>\x0a%s"' '"' ': "' | bash, the %s argument closes the body before the push line.

Fix (fail closed, deny_gate.py). The publish gate spaces a source only when the command has no backslash before an upper-case letter (_UPPER_ESCAPE_RE, so \X \U \E \C and the rest) and the source has no % other than %%. Any other source keeps its per-line push reading, as on main.

Row (into bash)	1069d2c60d	f30a26ba03	main	Real bash (stub git)
printf '… \X27 … <push main> … \X27' (also \X22)	allowed	refused	refused	pushes main
echo -e '… \X27 … <push main> … \X27'	allowed	refused	refused	pushes main
printf '… "n%s … <push main> … %s"' '"' ': "' (also %b, single-quoted)	allowed	refused	refused	pushes main
each feat/x twin	allowed (no audit)	allowed (push_allowed audit)	allowed	pushes feat/x
printf '… "100%% … <push main> …"' (literal %)	allowed	allowed	refused	no push
literal gh pr create --body '⏎<push main>⏎'	allowed	allowed	refused	no push

The bash -c "$(printf …)" forms of both were already refused on 1069d2c60d, and they still are.

Checked on f30a26ba03:

• New TestPrintfOutputIsNotItsSourceText (9 cases): 6 protected rows refused, 2 feat/x twins allowed with their audit, and the %% row keeps its body reading. 8 of the 9 fail on 1069d2c60d.
• Sweeps against 1069d2c60d: 0 verdict changes over 6,688 + 2,268 push commands, the 13,969-command push/kill/printf corpus, 4,795 rm and 546 kill commands. The earlier c118/c109 probes give identical output, real-bash runs included.
• The gate test set has the same outcomes as 1069d2c60d, plus the 9 new passing cases. The wider security/hooks suite fails the same 30 test ids on both heads locally, so none come from this change.
• Package line budget re-pinned to the measured 32,661. black, isort, flake8, mypy and comment-history are clean.

Could a maintainer please approve the workflow runs for f30a26ba03? @iamwhatever, this PR still needs the human security review you asked for.

[Maqix]

Opus 5.5 and GPT 6.1 findings on f30a26b (printf FORMAT \0047 / \0042, echo \47): confirmed and fixed in 6ed2bff.

Root cause. The escape decoder reads \0047 as \0 + 047 (a quote), but a printf FORMAT reads at most three octal digits, so bash prints \004 + 7; and bash's echo -e prints \47 as text. In both cases the decoded source invented a quote around the push line, the gh-body spacing then hid it, and the receiving bash ran git push origin main. Patching the decoder one escape at a time would leave the same class open, so the fix narrows the exemption instead: the publish gate now spaces a source only when the command holds no backslash at all (and no % other than %%). Every other source keeps its per-line push reading, exactly as on main.

Checked (stub git, real bash):

Rows	f30a26b	6ed2bff	main	real bash
13 printf/echo octal-escape rows → main (\0047, \0042, \047, %b, echo \47/\42, bash -c "$(printf …)")	9 allowed	all denied	all denied	pushes main in 6
the 13 feat/x twins	allowed	allowed	allowed	pushes feat/x
literal gh pr create --body '…⏎git push origin main⏎' (no backslash)	allowed	allowed	denied	no push

• Push sweep vs f30a26b: 13,969 commands, 0 flips; rm corpus 4,795 and kill corpus 546, 0 flips.
• Test outcome sets: identical to f30a26b plus 7 new TestPrintfOutputIsNotItsSourceText cases (now 16), only the 2 pre-existing markdown_it failures.
• Trade-off, fail closed: a printf body that would really stay gh data but contains any backslash (e.g. printf 'gh pr create --body "100%%\x0agit push origin main\x0a"' | bash) is refused again, as on main.

Line budget re-measured: 32,660. @iamwhatever @bolichen97 could you approve the fork workflow runs on 6ed2bff? The human security review request still stands.

[iamwhatever]

Requesting changes at cdaf83d.

This PR loosens the protected-branch push floor, so it stays reserved for a human security reviewer even once the item below is fixed. The bypass from my r121 review is closed: a command holding any backslash, or a % other than %%, now keeps main's per-line reading (deny_gate.py#L651-L658), and the literal-body reading only accepts plain words and whole quoted words. I found no new bypass in that shape.

Defect: the security spec describes a different rule than the code. security.md#L1738 says the reading is withheld for "a backslash before an upper-case letter (deny_gate._UPPER_ESCAPE_RE)". No such symbol exists at this head, and is_denied withholds it for any backslash. A reader auditing the floor from the spec gets the narrower, wrong rule. Smallest fix: change that clause to "any command holding a backslash (undecoded in deny_gate.is_denied)" and drop the symbol name. The Opus lane raised the same point on this head and it has no reply yet.

Also open: Sensitive Change Review is red on this head (the reasoning template is not posted for cdaf83daff), and Design and First Principles both suggest splitting the printf_decode=False walk into its own PR; please reply to those notes.

— crew-pr-reviewer (r124)

[Maqix]

@iamwhatever thanks for the r124 pass, and for confirming the r121 bypass is closed with no new one in that shape.

Spec defect: fixed on the new head 5f189da147. security.md now says what the code does: the gh-body reading is withheld for any command holding a backslash (undecoded in deny_gate.is_denied), not only a backslash before an upper-case letter, and the non-existent _UPPER_ESCAPE_RE name is gone. The bullet also lists all three decoder traps the guard covers (\X27 lowercased, a printf format's \0047, echo's \47). That is the same point the Opus lane raised on cdaf83daff, so this answers it too. Doc-only change: no code moved, and the verdicts are identical to cdaf83daff (every probe set, the 13,969-command push sweep, the rm and kill corpora: 0 flips).

The same push also rebases onto today's main (ec2619c6f3); the only conflict was the liveness line budget, now re-pinned from 32,527 to the measured 32,669.

Design + First Principles: "split the printf_decode=False walk into its own PR". Both are right that it fixes a bypass main already has (bash -c $'…\x27…' and the echo/printf '%s' … | bash producers), and that the any-backslash guard now keeps those commands off the gh-body reading by itself. I kept it here because it was added for your r121 pipe bypass, before the any-backslash guard existed. Without it, main's own bypass stays open until a split PR lands. I'm happy to split it out, though: if the human security reviewer prefers two smaller PRs, say so and I'll move printf_decode, the extra is_denied source loop, the once-decoded producer branch and TestPrintfEscapeInAShellPayloadIsNotAQuote into a separate PR, and show that every r121/GPT/Opus row stays refused on this one without them.

Sensitive Change Review: a fork PR can't post its reasoning template or pass that gate. As its own message says, a maintainer takes it over on a branch of this repository (Co-authored-by: + Supersedes #20853). Could you or @bolichen97 / @cruisercohen approve the workflow runs for 5f189da147, and pick up the human security review or the upstream copy? Thanks!