Repository navigation
fix(kas): backport the allowedCommands chain grant to 0.12.1 (batch 26) - #32149
Conversation
…30847) An unattended agent's chained command (cd dir && ls -la file) always asked on KAS, even when allowedCommands matched each command in it: Crew granted only a plain command with no chaining character at all. A chain joined by &&, ||, ; or | is now granted when every part is a plain command and an entry matches each part whole. A chain holding a quote or backslash is never granted, since a quote can carry a separator into one argument. Redirects, substitutions, background & and newlines still never match, and the deny floor still runs on the whole line first. (cherry picked from commit 0fbbb71) Release backport: src/kiro_crew/docs/agent-spec-fields.md conflicted in the toolsSettings row; resolved by keeping release's text and adding this change's two chain sentences. Code and tests applied unchanged.
Sensitive Change ReviewSensitive files changed:
How to clear this check:
Only the author's comment counts this way. A reviewer can instead |
UX Review (Opus 5.5) — ⏭️ skippedRevision |
Design Review (Opus 5.5) — ✅ PASSDesign-level review of Design-Verdict: PASS Nothing blocks: this is a faithful single-commit backport of the gate change already reviewed and merged on Notes
[DESIGN-REVIEWED] 3fa431c Description read[DESCRIPTION-READ] ff2d0ad7fead69ce58e07e8353b7ac8c13074f000cccc146fdc0a310a9a6ddff 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 |
First Principles Review (Opus 5.5) — ✅ PASSPremise-level review of First-Principles-Verdict: PASS This is a clean backport of a fix that already merged on Notes
What this change shipsInventory (5 items) — 4 justifiedIntent: a person on 0.12.0 wants an unattended KAS agent to run chained commands that
[FIRST-PRINCIPLES-REVIEWED] 3fa431c Description read[DESCRIPTION-READ] b11010edaec833467ca475c6be58c4945db0330b0736454416df3b2b70a55526 That sha256 names the PR description this verdict read. Recompute it with |
Opus 5.5 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 6.1 Review — ✅ no blocking findingsGPT 6.1 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Security Scope Review -- ✅ PASSWhich legitimate operations does Verdict basis: adjudicated, zero confirmed regressions. Security scope review -- adjudicated candidates
No candidate flipped from allowed to refused. Scope-Verdict: PASS Reason: The diff loosens one guard and adds no new refusal. TIGHTENING: No deny tier got stricter. The only stricter-looking part is the new chain refusal in ADJUDICATED: 15 candidates, 0 confirmed newly-refused PLATFORM COVERAGE: posix, windows and any.
The diff assumes PowerShell's typographic quotes U+2018 to U+201E and UNADJUDICATED: The changed guard is the Inputs I would have submitted against a spec with
From the code, no single command that was granted before is refused now. The one wording change is the refusal message, which now explains chains instead of naming every composed command as never matching. [SCOPE-REVIEWED] 3fa431c |
Problem / Motivation
Goal: Bring #30847, a user-reported KAS fix that merged on
mainafterv0.12.0, ontorelease/0.12.0as one commit in a PR of its own, so it can be reviewed and reverted apart from the low-risk batches.Why it matters
cd dir && ls -la filealways asks on KAS, even whenallowedCommandsmatches every command in it. An unattended run has nobody to answer, so it stalls. Crew grants only a plain command with no chaining character at all.Not a goal
main; nothing is changed only here.CHANGELOG.mdchange. Cuttingv0.12.1-rc.Nand the bare tag follow docs/build/release.md and stay with the maintainers.What changed (motivation → approach → change)
The main squash commit was cherry-picked with
-xontorelease/0.12.0@e908209aa9.allowedCommandschain when every command matches0fbbb7104d3fa431c206The one hand-resolved hunk is documentation.
src/kiro_crew/docs/agent-spec-fields.mdconflicted in itstoolsSettingsrow, because the row differs betweenmainand this branch. It was resolved by keeping this branch's text and adding #30847's two chain sentences to it. The code and tests (hook_runtime/gate_tiers.py,platform/allowed_commands.py,test/test_kas_allowed_commands.py) anddocs/system-specs/modules/security.mdapplied with no conflict, and their added and removed lines are identical to the main commit. The commit is authored with the public identity the push guard requires.What the gate grants now, and what it still refuses.
&&,||,;or|, every part plain and matched whole by an entry--%&, newlinebuilder-security checked: no bind, listener, route, auth mechanism or storage grant changes. The change widens one auto-approve case in the permission gate, bounded as the table shows, and keeps the deny floor and governance ahead of it.
Backwards compatibility
Compatible: the change is the one already merged on
main, and its own compatibility note applies unchanged. A chained command whose every part anallowedCommandsentry matches now runs without asking.Removes nothing: no setting, matcher form or refusal that 0.12.0 applies to a deny rule is removed.
Tests
Proof: not provable -- other: a backport of a main commit whose proof ran on its own PR
release/*, so the related set was run here, one pytest process per batch of 20,-n 0:gate_tiersorallowed_commands, 8 files: 768 passed, none failed;test_gateway_secret_senders_prove_peer.py::TestArtifactSendsThroughThePeerProof::test_list_goes_through_the_proof. It fails the same way on a cleanrelease/0.12.0checkout.Manual verification
N/A - the fix was verified on its own main PR. The 0.12.1 RC soak is the end-to-end check.
Screenshots / video
Why no screenshot: backend-only change.
Related Issues
Backport to the 0.12 line for 0.12.1 of #30847.
Sibling sweep
Siblings: none -- the source PR carried its own sibling sweep; this PR only carries that fix.
Pattern harvest
Not generalizable: this PR only carries a fix already on
main. The source PR has its own harvest.Checklist
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)Contribution License Agreement
Contributions are accepted under the project's Apache-2.0 license; see the Licensing section of CONTRIBUTING.md.