[Maqix]

Stacked on #15438. This branch contains #15438's commit (7f57006ef, on main ec2619c6f) plus one new commit on top (fix(security): allow an env-prefixed push after a multi-line message). Only the top commit is new; review it alone with the "Commits" tab. Once #15438 merges, this PR will be rebased onto main and the base commit will drop out. It is ready for review now: please review the top commit only. Merge #15438 (or its upstream copy) first; if this PR lands first, it lands #15438's commit with it.

Problem / Motivation

Goal: a feature-branch push behind a PATH prefix after a multi-line commit message is allowed, the same as that push on its own line.

With #15438 applied, git commit -m 'a\n\nb' && env PATH=/x:$PATH git push origin feat and ... && PATH=/x git push origin feat are still refused with rule=git-publish-target-unverifiable (with the "put the push on its own line" hint), while the same push on its own line is allowed on main.

Why it matters

Agents often prefix a push with PATH=... (for example a toolchain or a credential helper on PATH) in the same chained command that commits with a multi-line message. The refusal forces a retry with the push on its own line, and the reason it gives does not explain why the chained form is different.

Not a goal

• Any variable other than PATH, any env option, or a prefix on a command other than git push.
• Heredoc messages and the bare quoted push (the other two follow-ups of fix(security): keep a quoted newline inside its word on the push path #15438).
• Changing the per-line floor that judges the push itself.

What changed (motivation → approach → change)

• Symptom → cause: fix(security): keep a quoted newline inside its word on the push path #15438 treats a quoted newline as harmless message text only when every other segment is on a closed allowlist (cd <dir>, git add, git commit -m/-q, git push -u/-q), checked by _quoted_word_is_inert / _inert_segment in src/kiro_crew/security/shell_normalizer_walk.py (where main's split of the shell reader moved the push segment splitter and fix(security): keep a quoted newline inside its word on the push path #15438's helpers with it). An env/assignment prefix, and the $ in $PATH, were outside that list, so the sentinel stayed.
• Change: a git push segment may now start with PATH=<value> assignments, bare or after a plain env with no option (_inert_segment checks the name is PATH; no other variable is admitted). The value may hold plain characters and $PATH; $PATH is now the one expansion the plain check admits, and _inert_segment refuses any $ outside that prefix.
• Why only PATH: it can change which git runs, but not the argv the per-line reading judges, and that reading still checks the push after the message in full (the protected-branch, bare, $b, brace and $'…' checks are unchanged). A literal PATH=/x can already point at any git, and the standalone form is allowed on main. Other variables were rejected deliberately: GIT_CONFIG_* / git -c remote.origin.push=feat:main can remap git push origin feat onto main, and BASH_ENV/ENV can make a shell that git starts source .git/COMMIT_EDITMSG, which would execute the message.
• Oracle: two new rows in _INERT_ORACLE_ROWS (env PATH=/x:$PATH git push origin main, PATH=/x git push origin main after a multi-line message) run in real bash and stay denied. The oracle stubs env as a shell function that exports its leading assignments and runs the rest, so it reaches the stub git (its PATH has no git). Only the fixed rows bypass the corpus's env/PATH filter.
• Spec paragraph in docs/system-specs/modules/security.md and _PACKAGE_LINE_BUDGET (32,731 → 32,749) updated.

Backwards compatibility

Compatible: this only allows commands that were refused; every command allowed before is still allowed, and nothing is newly refused. The hint fix changes only the wording of some refusals (the substitution reason in place of a hint that did not apply).

Removes nothing: the change widens one allowlist and takes no path, row, key or function away.

Tests

In test/test_push_branch_gate_parser.py:

• test_quoted_text_that_quotes_a_push_line_is_judged_line_by_line: both issue repros (plus PATH=/x git push -u fork fix/x) are allowed. 17 adversarial shapes stay refused through is_denied: env GIT_DIR=…, GIT_CONFIG_* refspec remap, BASH_ENV=.git/COMMIT_EDITMSG, PATH=/x GIT_SSH_COMMAND=sh, env -i, bare env, PATH=/x env git push, a lone PATH=/x;, PATH=$b, PATH=$PATH'x', PATH=/x*, PATH=/x git -c remote.origin.push=f:main push, PATH=/x git add, a prefixed commit, $PATH as a commit argument or cd target, $PATH in the push args. The out-of-allowlist hint example is now env GIT_DIR=/x. The no-hint rows add bash -c '<push> origin $b' and eval '<push> origin {a,main}' in front of a multi-line message plus a feature push: both keep the substitution reason.
• test_no_command_real_bash_runs_as_a_protected_push_is_allowed: runs the two new rows in real bash, and each one logs a protected push that is_denied refuses (with the construct list on and off).
• test_every_inert_allowlist_entry_has_a_real_bash_oracle_row: also requires a bare PATH= row and an env PATH= row.
• test_the_own_line_hint_is_left_out_when_the_rewrite_cannot_be_read: a payload walk that fails on the rewritten command leaves the hint out.

Hint mutation check: with the old _own_line_fix_applies, both hint tests fail.

Mutation check: with the source reverted, 2 tests fail. Each widening mutant (any variable name; a prefix on any command; $PATH in any word) fails 1 test.

Run (pytest -n 4): test_push_branch_gate.py, test_push_branch_gate_parser.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_split_patch_reach.py, test_denied_commands_security_*.py, test_denied_rules_facade.py, test_argv_floor_facade.py: 1852 passed, 1 skipped, 2 failed (ModuleNotFoundError: markdown_it, a local venv gap unrelated to this change). black, isort, flake8, mypy, check_comment_history.py and docs-lint.sh pass.

Manual verification

39-command probe through is_denied, run on origin/main, on #15438 and on this branch. Only the 4 intended env/PATH feature pushes change, from DENY to ALLOW. Every protected, bare, $b, brace, $'main', HEAD:main, --force main and feat:main push behind the prefix stays DENY, as do the non-PATH variables and env options above. One case is allowed on both #15438 and this branch: a message whose body is a literal single-quoted $1 push origin main line, followed by a feature push. #15438 already allows that message with an unprefixed push, because the body is literal data.

Hint sweep: 11,024 commands (the parser module's multi-line push corpus, plus multi-line messages behind bash -c/eval/sh -c payloads that push to a feature branch, $b, {a,main} or main), run on the previous head dfd95d228 and on this one. 0 verdict changes, and 0 changes in the refusal reason apart from the hint sentence. The hint appeared on 2,766 refusals before and appears on 1,036 now. For each of those commands the probe resubmits the own-line rewrite. All 1,036 remaining hints are true (the rewrite is allowed). The 1,730 dropped hints were all false (the rewrite is still refused). No true hint was lost.

Related Issues

Closes #20847

Follow-up of #15438. Refs #12631.

Pattern harvest

Not generalizable: the inert-word allowlist is deliberately closed and grows one reported shape at a time, each with its own real-bash oracle row; the coverage test already enforces that pattern.

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

Contributed under the project's Apache-2.0 license.

[github-actions]

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

Revision 99508c3e7dc9f6322998b3bb54ce6576fe8e969f 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 b9399caab84b8dff64e0f97f0901482bd0bfe750 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 change only turns refusals into allows, inside a closed PATH-only allowlist that a real-bash oracle checks, and the push line is still judged in full.

Notes

• The authentic patch contains all of fix(security): keep a quoted newline inside its word on the push path #15438 (the quoted-newline splitter, _inert_segment, the hint rewrite) as well as the PATH prefix. Whatever merges first carries that combined design, so it should not land ahead of fix(security): keep a quoted newline inside its word on the push path #15438's own review.
• This is the third allowlist shape added one reported command at a time (flags, title prefix, now PATH). If more follow, the cheaper design is to drop the ungated sentinel per push line, rather than keep growing what counts as an inert neighbour.

[DESIGN-REVIEWED] b9399ca

Description read

[DESCRIPTION-READ] 5bd2c795e085d0bbaaaee32a17e3ff181500af712bac51d1f1d1873ee297757c

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 99508c3e7dc9f6322998b3bb54ce6576fe8e969f 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 only change this PR claims is a narrow PATH-only prefix on the push allowlist, with a linked issue and real-bash oracle rows. Everything else in the diff is #15438's stacked commit, which the description declares.

What this change ships

Inventory (9 items) — 5 justified

Intent: let an agent's PATH=…/env PATH=… feature-branch push, chained after a multi-line commit message, through the same as the push on its own line. This is a FIX.

1. A feature push after a multi-line quoted git commit message is allowed, because a quoted newline now stays in its word — rides along (stacked fix(security): keep a quoted newline inside its word on the push path #15438, declared)
2. A PATH=<value> prefix on that push is admitted, bare or after a plain env — justified
3. $PATH is the one expansion the plain check admits, and only inside the push prefix — justified
4. Multi-line quoted text that only mentions a push (echo, --body) is judged line by line — rides along (stacked fix(security): keep a quoted newline inside its word on the push path #15438)
5. The "Put the push on its own line" reason now appears only when that rewrite would actually pass — rides along (1,730 false hints measured)
6. The inert proof reads through one exact leading Running: title prefix — rides along (stacked fix(security): keep a quoted newline inside its word on the push path #15438)
7. _own_line_fix_applies is listed on the security facade — justified
8. _PACKAGE_LINE_BUDGET goes from 32,527 to 32,749 — justified
9. The spec paragraph in security.md is updated — justified

[FIRST-PRINCIPLES-REVIEWED] 99508c3

Description read

[DESCRIPTION-READ] 8804d16db64bce69d4d2350a32bfca7b048ba1456747ff8c5e96508b09dfc37c

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]

GPT 6.1 Review (fork) — ✅ no blocking findings

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

Review details

No findings.
[GPT-REVIEWED] 99508c3

[github-actions]

Opus 5.5 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 99508c3

[github-actions]

Security Scope Review (fork) — ✅ PASS

Which legitimate operations does 99508c3e7dc9f6322998b3bb54ce6576fe8e969f 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: Mostly this change loosens the push gate: a newline inside quotes now stays part of its word. The one stricter part is a new per-line backstop in _git_publish_floor_tags, which judges every push line of a multi-line command as if it were a standalone command and adds that line's tags to the verdict. No regression has been confirmed by the script for the candidates written. The refusal-reason rewrite (_own_line_fix_applies) changes only the advice text, not what gets refused, so it is not a guard and needs no verdict.

TIGHTENING: Push-floor per-line reading in the sensitive-bash tier (argv_floor_git._git_publish_floor_tags) -- it refuses a protected, bare or unverifiable push hidden behind a phantom quote that the whole-text walk models wrongly: if "\n" in text_lower: ... for line, close in _inert_quoted_line_heads(text_lower, _is_git_publish): line_tags = set(_git_publish_floor_tags(line)) ... tags |= line_tags, together with the new re-split if "\n" in command and _git_push_args(command) is None: pending[:0] = _split_push_command_segments(command, keep_quoted_newlines=False).

ADJUDICATED: 16 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: Most rows are platform-neutral (any): multi-line commit-then-push scripts, rebase plus --force-with-lease, status/log then push, the Running: title form, and quoted PR bodies and comments that contain apostrophes, #, ; and <…> and mention a push. These probe the per-line reading, which restarts each line unquoted. The posix row covers git commit -F /tmp/... followed by a push. The windows rows cover three assumptions the diff makes. First, the quote walk treats \ as an escape, probed with cd C:\Users\dev\repo and cd C:\r before a multi-line commit and push. Second, the per-line reading splits only on \n and leaves the \r in place, probed with CRLF inside a quoted message and as the script's line ending. Third, _INERT_PLAIN_RE excludes \ and \r. No row probes a UNC path or a .exe git name, so it is an open question whether either changes the result.

UNADJUDICATED: The refusal text chosen by _own_line_fix_applies (is_denied → git-publish-target-unverifiable → choice between the "Put the push on its own line" reason and the "fuses text" reason) is advice text only. No classifier verdict covers whether the hint is accurate. That matters for a command like cd C:\r && git commit -m "a\n\nb" && git push origin feat if the Windows row comes back refused: the hint would then name a fix whose rewrite also has to clear the floor on Windows spellings.

[SCOPE-REVIEWED] 99508c3

Security scope review -- adjudicated candidates

• macos-latest (platform posix): 12 classified, 0 newly refused, 7 still allowed.
• ubuntu-latest (platform posix): 12 classified, 0 newly refused, 7 still allowed.
• windows-latest (platform windows): 15 classified, 0 newly refused, 8 still allowed.

No candidate flipped from allowed to refused.

[Maqix]

Thanks for the review. The Opus 5.5 finding on dfd95d228 is fixed in f465a22d4 (still 2 commits: #15438's 65b3df737 plus the amended top commit).

Finding: _own_line_fix_applies (deny_gate.py) checked the rewritten command on the top-level text only, while the floor gathers floor_tags over every _shell_payload_sources entry. bash -c 'git push origin $b'; git commit -m 'a⏎b' && git push origin f got "Put the push on its own line", and the rewritten command came back with the same refusal and the same hint.

Fix: the hint check now runs the rewritten command through _shell_payload_sources and shows the hint only when no publish source still carries a floor tag, the same way a resubmission is judged. If the walk fails, the hint is left out. Only the reason text changes.

Tests (test/test_push_branch_gate_parser.py): the trigger and an eval '… {a,main}' twin are added to the no-hint rows, and they keep the substitution reason. A new test checks that a walk failing on the rewrite leaves the hint out. Both fail with the old check. The existing hint rows are unchanged.

Sweep vs dfd95d228: 11,024 commands (the parser module's multi-line corpus plus bash -c/eval/sh -c payloads in front of multi-line messages): 0 verdict changes and 0 reason changes apart from the hint. The probe resubmitted every hinted command's own-line rewrite. Before the fix, 1,730 of 2,766 hints were false. Now all 1,036 remaining hints are true, and no true hint was lost.

Gates: 1,852 passed (the 2 failures are a local markdown_it venv gap), plus black/isort/flake8/mypy, comment-history, docs-lint and the line cap. Budget 32,275.

Could a maintainer please approve the workflows on f465a22d4?

[Maqix]

Status on 28b48cfe7b (restacked on #15438's 62b1ee50b7; only the line-count number changed, re-pinned to the measured 32,425): GPT 6.1, Design, First Principles, Backend Tests (3.12) and Coverage Gate pass. Two lanes did not run, so they have no verdict: Opus 5.5 Review ("review incomplete") and Security Scope Review ("no leg reported"). On the previous head f2ddfd9560, with the same code, every AI lane passed.

Could a maintainer please re-run those two lanes? This PR still merges after #15438.

[Maqix]

Opus 5.5 finding on 8849ce5: confirmed. A quoted ; git push inside the message was split like a real separator, so git commit -m 'a; git push origin f⏎⏎b' && git push origin feat got the "own line" hint. The resubmission with the push on its own line still had ; git push inside the quote and got the same hint forever.

Cause. The rewrite in _own_line_fix_applies was a quote-blind re.sub. It split at every ;/&/&&/|| that a git command follows, including the ones inside quotes.

Fix. The fix belongs to the hint itself, so it went into #15438's commit (5986a0e), together with the nested-payload reading this PR used to carry. The rewrite now moves a git command only after an active separator, using the offsets from the shared quote walk (_iter_shell_chars, step.active). It judges the result over every nested shell payload, as the floor does. This PR is restacked on it as b9399ca. Its top commit is now only the PATH push prefix, and its commit message and body no longer claim the hint change.

Verdicts did not change. Against 8849ce5:

• probe.py: 5,104 commands, 0 flips.
• c100p probe: 6,688 commands, 0 flips. nested_probe: 2,268 commands, 0 flips.
• c93 sweep: 11,026 commands, 0 verdict flips and 0 masked-reason diffs. The hint flag changed on exactly 1 command, your quoted-separator command, where it was removed.

The test pass set is identical: 1,868 passed, plus the 2 known markdown_it failures. The package line budget is 32,748 (from 32,730).

@iamwhatever @bolichen97 @cruisercohen could one of you approve the workflow runs on b9399ca? This PR is stacked on #15438, so it can land right after it.

[iamwhatever]

HOLD (policy: sensitive change) at b9399ca.

This PR widens the git-publish deny floor: a git push after a multi-line commit message may now carry a PATH= prefix, bare or after env (shell_normalizer_walk.py#L1302). Changes to what the command sandbox allows are reserved for a human maintainer.

The branch also contains all of #15438 (commit 5986a0e), which is still open. Merging this PR first would land #15438's security changes without their own review; the Design and First Principles lanes say the same.

I found no concrete defect in the top commit: only PATH is admitted, any other variable, env option or $ outside the prefix keeps the sentinel, and the push itself is still judged in full. All AI lanes pass and CI is green.

Clears when: #15438 merges, this PR is rebased onto main with only its own commit, and a maintainer reviews the deny-floor widening.

— crew-pr-reviewer (r124)

[Maqix]

@iamwhatever thanks for the careful read, and for confirming the top commit admits only PATH with the push still judged in full.

Confirming the plan that clears the HOLD:

• #20854 is deliberately stacked on #15438 (fork stacked PR). Its own change is the single top commit b9399caab8 — the PATH= / env PATH= prefix admission in shell_normalizer_walk.py. The commit below it is all of fix(security): keep a quoted newline inside its word on the push path #15438.
• I'll keep fix(security): allow an env-prefixed push after a multi-line message #20854 pointed at fix(security): keep a quoted newline inside its word on the push path #15438 until fix(security): keep a quoted newline inside its word on the push path #15438 lands, then rebase fix(security): allow an env-prefixed push after a multi-line message #20854 onto main so it carries only b9399caab8 for the deny-floor review — exactly the end state you asked for.
• fix(security): keep a quoted newline inside its word on the push path #15438 is green on every AI lane with no open findings; it waits on the same human review, or the upstream copy a maintainer offered. If fix(security): keep a quoted newline inside its word on the push path #15438 is taken upstream (crediting the author), fix(security): allow an env-prefixed push after a multi-line message #20854 follows as a single-commit PR and the deny-floor widening is all that is left to review.

No change needed on the top commit meanwhile. I'll rebase both if main touches the security package.