Skip to content

fix(kas): backport the allowedCommands chain grant to 0.12.1 (batch 26) - #32149

Merged
buluoray merged 1 commit into
release/0.12.0from
crew/slack/0.12.1-batch-26
Nov 7, 2026
Merged

buluoray merged 1 commit into
release/0.12.0from
crew/slack/0.12.1-batch-26

Conversation

@buluoray

@buluoray buluoray commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Touches the permission gate; needs Bolin's OK before merge. #30847 changes hook_runtime/gate_tiers.py, which decides when Crew grants a KAS shell command from allowedCommands. That is above the low-risk bar the other 0.12.1 batches use, so it is in a PR of its own.

Problem / Motivation

Goal: Bring #30847, a user-reported KAS fix that merged on main after v0.12.0, onto release/0.12.0 as one commit in a PR of its own, so it can be reviewed and reverted apart from the low-risk batches.

Why it matters

PR Reported What a 0.12.0 user sees today
#30847 a user running an unattended KAS agent (autopilot blocked) A chained command such as cd dir && ls -la file always asks on KAS, even when allowedCommands matches 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

  • No fix that is not already merged on main; nothing is changed only here.
  • No version bump, no tag, no CHANGELOG.md change. Cutting v0.12.1-rc.N and the bare tag follow docs/build/release.md and stay with the maintainers.
  • Not merged into any other 0.12.1 batch.

What changed (motivation → approach → change)

The main squash commit was cherry-picked with -x onto release/0.12.0 @ e908209aa9.

PR main commit release commit Pick
#30847 grant an allowedCommands chain when every command matches 0fbbb7104d 3fa431c206 one doc hunk resolved by hand

The one hand-resolved hunk is documentation. src/kiro_crew/docs/agent-spec-fields.md conflicted in its toolsSettings row, because the row differs between main and 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) and docs/system-specs/modules/security.md applied 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.

Command shape 0.12.0 With this change
plain command an entry matches granted granted
chain joined by &&, ||, ; or |, every part plain and matched whole by an entry asks granted
chain holding a quote, backslash, non-ASCII character or PowerShell --% asks asks
redirect, substitution, background &, newline asks asks
anything the deny floor matches refused refused; the floor still runs on the whole line first

builder-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 an allowedCommands entry 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

  • CI does not run the backend suite on PRs into release/*, so the related set was run here, one pytest process per batch of 20, -n 0:
    • every test file that names gate_tiers or allowed_commands, 8 files: 768 passed, none failed;
    • every hook-runtime, gate, hooks and denied-commands test file besides those, 85 files: 4450 passed. One failed: test_gateway_secret_senders_prove_peer.py::TestArtifactSendsThroughThePeerProof::test_list_goes_through_the_proof. It fails the same way on a clean release/0.12.0 checkout.
  • flake8 passes on the changed Python files.

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

  • 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; see the Licensing section of CONTRIBUTING.md.

…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.
@buluoray
buluoray requested review from a team and KiroCrewCaptain as code owners November 7, 2026 06:34
@buluoray
buluoray requested review from tim-finnigan and removed request for a team November 7, 2026 06:34
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Sensitive Change Review

Sensitive files changed:

  • src/kiro_crew/hook_runtime/gate_tiers.py

  • No reasoning template for 3fa431c yet.

How to clear this check:

  1. The PR author posts the template below as a PR comment, with
    every field filled (N/A needs a reason). Keep the
    Head commit line: it must name the current head SHA.
  2. The check turns green within a minute, with no push.
  3. A new push resets this check: post it again for the new head.

Only the author's comment counts this way. A reviewer can instead
paste the filled template into an "Approve" review on this head.
Merging still needs a reviewer's approval.

## Sensitive change review
- Head commit: 3fa431c20678367df14a65449ce9f42c3127ba59
- What rule changed:
- Before -> after (what was allowed/blocked, now):
- Worst case if wrong (breaks commands / leaks secret / locks user out):
- Evidence checked (which sandbox result proves it):
- No regression:
  - Tools still work (which tools/commands were run, result):
  - Backward compatible (old config / old data / old callers still work, how checked):
  - Existing tests (which suites passed on this head):
- How to undo:

@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — ⏭️ skipped

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

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 3fa431c20678367df14a65449ce9f42c3127ba59 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: this is a faithful single-commit backport of the gate change already reviewed and merged on main as #30847. The deny floor still runs on the whole line before the chain grant, which only widens one bounded case.

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 .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

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 3fa431c20678367df14a65449ce9f42c3127ba59 — 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

This is a clean backport of a fix that already merged on main. Every item is #30847's own code, tests or docs, and none adds new scope.

Notes

What this change ships

Inventory (5 items) — 4 justified

Intent: a person on 0.12.0 wants an unattended KAS agent to run chained commands that allowedCommands fully covers, without stalling on a prompt. This is a FIX, backported from main.

  1. A chain joined by &&, ||, ; or |, where every part is plain and matched by an entry, now runs without asking. — inherited, defect not linked
  2. A chain that holds a quote, a backslash, a non-ASCII character or --%, or has more than 16 parts, still asks. — justified
  3. The wording of the denyByDefault refusal and the composition notice now describes the chain rule. — justified
  4. The removed "never granted" test cases (git status && make, and others) are the decision this fix reverses, as the description states. — justified
  5. The security.md and agent-spec-fields.md text now describes the chain grant. — justified

[FIRST-PRINCIPLES-REVIEWED] 3fa431c

Description read

[DESCRIPTION-READ] b11010edaec833467ca475c6be58c4945db0330b0736454416df3b2b70a55526

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

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — ✅ no blocking findings

Reviewed 3fa431c20678367df14a65449ce9f42c3127ba59 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 3fa431c

Verdict parsed from the review's SHA-scoped output markers for commit 3fa431c20678367df14a65449ce9f42c3127ba59.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 3fa431c20678367df14a65449ce9f42c3127ba59 false-positive: <reason, 20 to 500 characters>

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of 3fa431c20678367df14a65449ce9f42c3127ba59 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 3fa431c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 3fa431c20678367df14a65449ce9f42c3127ba59 false-positive: <reason, 20 to 500 characters>

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Security Scope Review -- ✅ PASS

Which legitimate operations does 3fa431c20678367df14a65449ce9f42c3127ba59 newly refuse? Updated in place on each push. A script-confirmed regression blocks; the model proposes candidates and never decides a refusal.

Verdict basis: adjudicated, zero confirmed regressions.

Security scope review -- adjudicated candidates

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

No candidate flipped from allowed to refused.

Scope-Verdict: PASS

Reason: The diff loosens one guard and adds no new refusal. _command_matches_any now grants a chain whose every part an allowedCommands entry matches. A single command takes the same path as before, through plain_command and one shared match budget. None of the three shell deny tiers (sensitive-bash, exfil, deny-rules) is touched. The toolsSettings.shell.denyByDefault refusal (spec-shell-allowlist) depends on each agent's own spec, so the deny-composite classifier does not measure it. That gap is named under UNADJUDICATED.

TIGHTENING: No deny tier got stricter. The only stricter-looking part is the new chain refusal in allowed_commands.chain_commands -- it stops a quote, backslash, non-ASCII character or --% from carrying a separator into one argument, so the split parts are never different from what the shell runs: if (not command.isascii() or _CHAIN_QUOTING_CHARS.intersection(command) or _STOP_PARSING in command): return None (applied only when len(parts) > 1; a single command is still judged by plain_command as before).

ADJUDICATED: 15 candidates, 0 confirmed newly-refused

PLATFORM COVERAGE: posix, windows and any.

  • posix: the rows use / paths, .venv/bin/python, and && chains of the maintainers' build, lint and test commands.
  • windows: the rows use \ separators, .venv\Scripts\python.exe, PowerShell Get-ChildItem … | Select-String, and ; chains. They probe the backslash rule: in a chain a backslash refuses the grant, while a single command with a backslash path keeps the old plain_command behaviour.
  • any: the git, gh and kirocrew chains are written the same on every host. They use each of the four new separators &&, ||, ; and |.

The diff assumes PowerShell's typographic quotes U+2018 to U+201E and --%. These are refused only inside a chain, so a candidate holding them would be the threat shape itself, and none was written.

UNADJUDICATED: The changed guard is the spec-shell-allowlist deny tier (_tier_spec_shell_allowlist → _command_matches_any → allowed_commands.chain_matches_all). The spec-builtin-grant row (_spec_command_granted) uses the same path. A shell call reaches it only when the agent spec carries toolsSettings.execute_bash.denyByDefault or allowedCommands, and the classifier does not model that per-spec input. Neither tier is in SHELL_DENY_TIERS, so no verdict is available for either.

Inputs I would have submitted against a spec with allowedCommands: ["cd .*", "ls.*", "git .*"] and denyByDefault:

  • cd build && ls (new grant)
  • git commit -m "fix(x): y" (single quoted command, must stay granted as before)
  • ls "a" && ls (now refused with the quote reason, as before)
  • git status --porcelain; git log --oneline -5 (PowerShell spelling)

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

@buluoray
buluoray merged commit 4d9f6b1 into release/0.12.0 Nov 7, 2026
25 of 28 checks passed
@buluoray
buluoray deleted the crew/slack/0.12.1-batch-26 branch November 7, 2026 06:45
@github-actions github-actions Bot removed the readiness: checking Automated validation is still running label Nov 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

⌂ 换网址