fix: resolve kiro-cli via resolve_kiro_cli() in auto-update paths

[bolichen97]

Fixes #7704.

Problem

Two auto-update steps gated on shutil.which("kiro-cli") and then spawned the bare name, so they resolved only through the process's inherited PATH:

• src/kiro_crew/cli_server.py (CLI-side auto-update)
• src/kiro_crew/slack/gateway.py (gateway auto-update)

Because the spawn was which-guarded, a working install reachable only through a fixed known_kiro_cli_dirs() entry was silently skipped by kiro-cli update. Same root cause as #7674 (bare-name resolution instead of resolve_kiro_cli()); deliberately left out of PR #7692's scope (diagnostics-only).

Fix

Both sites now resolve via the existing resolve_kiro_cli() helper (kiro_cli.py): skip when it returns None (unchanged best-effort no-op), otherwise spawn the returned absolute path. Same containment posture as before (update remains best-effort).

• cli_server.py: added from kiro_crew.kiro_cli import resolve_kiro_cli; replaced the shutil.which gate with resolve_kiro_cli() and spawn [kiro_cli_bin, "update"].
• slack/gateway.py: same import; replaced the gate and spawn the resolved path. shutil remains used elsewhere in both files.
• slack/gateway.py also moves the resolution off the event loop — kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli). This is a timing change shipped deliberately with the fix, not a drive-by: resolve_kiro_cli() stats every fixed candidate directory and every inherited-PATH entry and reaches env.augmented_path(), which globs every mise/asdf/nvm/fnm version root, whereas the shutil.which it replaces walked os.environ["PATH"] only. Resolving on the loop would therefore newly stall chat and heartbeat on the first cold call. cli_server.py::_update stays synchronous — it is a CLI verb with no event loop, so the stat walk blocks nothing but the command the operator invoked. test/test_slack_gateway.py::TestAutoApplyUpdateVenvPath::test_kiro_cli_update_timeout_kills_child_and_stays_nonfatal asserts the resolver does not run on the thread running the coroutine.

resolve_kiro_cli() is a strict superset of the old shutil.which gate: it honours KIROCREW_KIRO_BIN, then the fixed known directories, then the inherited PATH, and returns None when nothing is executable. Where a host has copies both on PATH and in a fixed directory, update now targets the fixed-directory copy — which is the binary the agent spawn itself resolves (acp/client.py), so the updater and the agent now agree on which binary they are talking about.

Scope: the sibling bare-name sites are deliberately not in this PR

Four other call sites still resolve the bare name. None of them is on an auto-update path, none is made worse by this change, and each needs its own reasoning — so they are left for a follow-up rather than folded in here:

• slack/gateway.py::_warn_if_kiro_cli_outdated — the boot version probe. Not the one-line swap it looks like: it is a declared test_spawn_audit.py sandbox exemption annotated "fixed argv", it carries a documented never-raises contract, and seven existing tests in test/test_slack_gateway_more_coverage.py patch asyncio.create_subprocess_exec and would need the resolver patched too. It was already probing a binary the agent may not be running, both before and after this PR.
• diagnostics.py::_kiro_cli_version — reports unavailable for a fixed-directory install. A kirocrew doctor surface this PR does not touch.
• cli_setup.py — its message reads "kiro-cli not found on PATH", so PATH is the intended semantics; changing the probe without rewriting the advice would make the advice wrong.
• cli_doctor.py — already imports resolve_kiro_cli and uses it elsewhere; the remaining sites need a per-site read.

Tests

Updated existing auto-update tests to patch resolve_kiro_cli and assert on the resolved absolute path. Added regression tests:

• test_fixed_dir_only_kiro_cli_is_updated_not_skipped — resolvable via a fixed directory while absent from PATH; the update must run and must spawn the absolute path.
• test_no_kiro_cli_skips_the_backend_update — None resolves to a skip, preserving the best-effort no-op.

Also updated test/test_slack_gateway.py to patch resolve_kiro_cli instead of shutil.which.

Verification

Run against this branch rebased on main:

• isort --check-only, flake8, mypy --platform linux src/kiro_crew — clean.
• scripts/check_black_formatting.py, check_subprocess_encoding.py, check_agent_sdk_boundary.py, check_sync_io_in_async.py, check_lockdown_before_publish.py — all pass. (The earlier Backend Lint & Type Check (3.10) red was this gate: test_cli_server_more_coverage.py was not black-formatted. Fixed.)
• test_cli_server_more_coverage.py, test_slack_gateway.py, test_slack_gateway_more_coverage.py, test_spawn_audit.py, test_security_posture.py, test_kiro_prerequisite.py — 683 passed.

The earlier Backend Tests (3.10, 4) red was test_subagent_state_write_serialization.py::TestOnLoopCallerDoesNotWait::test_a_coroutine_does_not_wait_on_a_held_lock failing a timed threshold by 0.08s (0.58s against < 0.5) on a shared runner. That file is not in this diff and the test passes on both main and this branch locally; Coverage Gate failed closed on that shard (backend-test=failure -- failing closed), so it is the same failure reported twice.

Pattern harvest

Rule candidate: semgrep

Pattern: a hardcoded kiro-cli path or a bare "kiro-cli" argv name on a spawn
path, where the repo already owns a resolver (resolve_kiro_cli()). Every such
call site silently diverges from the resolver's search order, so an operator whose
binary lives outside the assumed location gets a working agent everywhere except
the one path that hardcoded it. Review prompt: when a binary has a named resolver,
grep for every other spelling of that binary before adding a call site.

The spawn-audit record for these two sites, and the residual

Both changed functions sit in test/test_spawn_audit.py's BENIGN_SPAWNS, on the
module docstring's stated basis that a self-update spawn is a "fixed argv against our
own install". That basis moves the moment argv[0] becomes a resolved path, so each
entry now carries its own justification naming what actually bounds it — one resolver,
shared with the ACP session spawn (acp/client.py::_resolve_kiro_bin), so the
maintenance step can only ever name the binary the agent itself runs, and no
agent-supplied value reaches the argv.

test_kiro_cli_update_spawns_resolve_through_the_shared_resolver pins that invariant
across all three sites, so the update target and the launch target cannot drift apart
and no site can reintroduce a bare name or a shutil.which gate. Confirmed to bite by
mutating production code on each of its four branches.

docs/system-specs/modules/cli.md § Update Command documents the previously unlisted
kiro-cli step and how its path is resolved.

The residual is stated in both places rather than left implicit: the candidate order
includes user-writable install directories — ~/.local/bin is where Kiro CLI's own
installer puts the binary — and security.is_sensitive_path / is_sensitive_write_path
both answer False for it, so an auto-approved agent write there is not refused. That
residual belongs to the resolver and to the write side, not to a maintenance spawn: the
ACP session spawn resolves through the same function, reaches the same candidate ahead
of every PATH entry, and runs at every session start.

Closing it is a keystone decision with no cheap form, which is why it is filed for its
own PR rather than carried here. _WRITE_PROTECTED_HOME_PATHS alone would not close it:
that list is enforced at the file-edit gate only, so an entry there leaves
cp payload ~/.local/bin/kiro-cli permitted. The path-shaped alternative in the
sensitive-command regex — the mechanism that actually refuses a write under
.kiro/agents — is verb-independent, so applying its shape to a general-purpose user
bin directory would also refuse routine reads and listings there. And kiro-cli is a
multi-call binary that execs sibling executables resolved relative to its own path, so a
leaf-only fence on the kiro-cli name is insufficient in any case.

One property of this diff belongs in the same record rather than only in a review
thread: on Linux the ACP session spawn runs inside the namespace sandbox (the
is_kiro_cli delegation is gated to darwin and win32), while both update sites are
unrouted BENIGN_SPAWNS entries. So on a host where ~/.local/bin is not on the
invoking PATH, this change adds one execution of that candidate outside the
credential-hiding sandbox. Routing the maintenance spawn is not the remedy available
here — the audit already records that a self-update spawn cannot run confined because
the installer must write its own install directory, and any routed spawn must
additionally carry a kernel RLIMIT ceiling, which is not something to put on a
downloader on an untested assumption. It is part of the same escalation.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 152fb457182d29a3af8d521fbd38a585087418f4 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 152fb45

Verdict parsed from the review's SHA-scoped output markers for commit 152fb457182d29a3af8d521fbd38a585087418f4.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 152fb457182d29a3af8d521fbd38a585087418f4: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 152fb457182d29a3af8d521fbd38a585087418f4 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root-cause fix through the existing shared resolver, with the update/launch-target-divergence invariant pinned by an AST audit — right shape, right layer, proportionate scope.

[DESIGN-REVIEWED] 152fb45

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 152fb457182d29a3af8d521fbd38a585087418f4 — 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.

First-Principles-Verdict: CONCERNS

The fix is cause-level and its deferred siblings are counted — but a standing repo-wide AST enforcement and a thread-offload behavior change ship undeclared inside it.

What this change ships

Intent: make auto-update maintain the kiro-cli binary the agent actually launches, instead of silently skipping installs the inherited PATH never names — a FIX (#7704).

1. kirocrew update now updates a fixed-dir-only kiro-cli install — justified
2. Gateway unattended auto-apply does the same — justified
3. With copies on both PATH and a fixed dir, update now targets the agent's own binary — justified, cause-level
4. Gateway resolution moved off the event loop to a worker thread — undeclared, rides along
5. New AST audit pinning all three kiro-cli exec sites to the shared resolver and fixed argv — undeclared, rides along
6. Regression tests for fixed-dir-only and not-installed cases — justified
7. kirocrew update's kiro-cli step now documented in cli.md — justified (AGENTS.md same-commit spec rule)
8. BENIGN_SPAWNS entries gain written justifications naming a residual — undeclared, rides along

Watch

• Item 4 is a shipped timing change the description's Fix section never states ("replaced the gate and spawn the resolved path" — no mention of asyncio.to_thread); it is derived (resolve_kiro_cli globs tool-manager roots via env.augmented_path, so on-loop it stalls chats), but a human should see it declared.
• Item 5's resolver-presence half is a second spelling for the two update sites: 3 behavioral tests in this same PR (test_fixed_dir_only_…, test_no_kiro_cli_…, the gateway venv test) already fail on a revert to shutil.which. Its unique value is the acp/client.py::_resolve_kiro_bin pin and the argv-shape check the BENIGN_SPAWNS exemption rests on.
• Siblings verified: 3 bare-name sites remain (slack/gateway.py:2388, diagnostics.py:281, cli_setup.py:146; grepped "kiro-cli" spawns/which) — declared with per-site reasons, so accepted-and-deferred; diagnostics.py::_kiro_cli_version shares the root cause and looks one-swap-sized, so it should be the follow-up's first line.

[FIRST-PRINCIPLES-REVIEWED] 152fb45

[github-actions]

GPT 5.6 Review — 🔴 changes requested (blocking)

GPT 5.6 found at least one blocking issue that must be resolved before merging 152fb457182d29a3af8d521fbd38a585087418f4.

This comment is updated in place on each push.

BLOCKING -- src/kiro_crew/slack/gateway.py:9642 -- Fixed-directory resolution enables unsandboxed agent-planted execution
kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli)
kiro_update = await asyncio.create_subprocess_exec(kiro_cli_bin, "update", ...)
Agent plants executable ~/.local/bin/kiro-cli -> auto-update resolves it before PATH -> gateway executes attacker code outside the sandbox.
Anchor: residual/security
Fix: Revert this path to the previous PATH-only lookup.
[BLOCK-MERGE] 152fb45
[GPT-REVIEWED] 152fb45
False positive or not applicable? A repository writer can comment:
/ai-review override gpt 152fb457182d29a3af8d521fbd38a585087418f4: <one-sentence reason>

[bolichen97]

• BLOCKING -- src/kiro_crew/slack/gateway.py:9615 -- CLI resolution blocks the gateway event loop — fixed in 717bdd4dfe031a832ccd2540e9ad30c4240717b4.

kiro_cli_bin = resolve_kiro_cli()
First auto-update with slow/large tool-manager directories -> synchronous glob/stat discovery -> chat and heartbeat stall.
Anchor: no-blocking-call-on-event-loop
Fix: use kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli).

Legitimate, and legitimate specifically because this PR introduced it. I verified the chain rather than taking it on report:

• _auto_apply_update is an async def running on the gateway event loop, and the resolution sat directly on it.
• resolve_kiro_cli() -> find_kiro_cli_candidates(..., include_inherited_path=True) -> known_kiro_cli_dirs() -> env.augmented_path(...), which calls env._node_all_bin_dirs — a glob over every mise/asdf/nvm/fnm version root, @functools.lru_cache(maxsize=1), so the first cold call pays the entire scan — and then one platform_compat.is_executable_file() stat per fixed dir, per inherited PATH entry, and per per-version bin dir.
• The PR made it strictly worse on a line it added: the replaced shutil.which("kiro-cli") walked only os.environ["PATH"]; it never touched augmented_path, so it never globbed the tool-manager roots.
• Not speculative: the same function, 59 lines earlier, already offloads a smaller synchronous os.path walk (_obstructions) to run_in_executor(subprocess_executor(), ...) under a comment naming the identical no-blocking-call-on-event-loop anchor. And scripts/check_sync_io_in_async.py's own header records that a stall past dashboard.loop_stall_exit_after_secs makes the watchdog kill the gateway.

Applied the suggested form verbatim — kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli) — and restated the comment above it to say the resolution is offloaded because it stats every candidate dir and globs the version roots. src/kiro_crew/cli_server.py's _update() is left synchronous and unwrapped: it is a synchronous CLI verb with no event loop, so there is nothing there to block but the command the operator invoked. Its comment now says so.

One correction to the record for anyone reading the earlier verification notes: scripts/check_sync_io_in_async.py exiting 0 does not exonerate this call. Its FAMILY_REMEDY families are only db, subprocess, http, and sleep — filesystem stat/glob is outside its scope, so its pass was never evidence either way. That gate was mis-cited before; the finding stands on the mechanism above.

Pinned by a test rather than left to review: test/test_slack_gateway.py::TestAutoApplyUpdateVenvPath::test_kiro_cli_update_timeout_kills_child_and_stays_nonfatal now records the thread resolve_kiro_cli is invoked on and asserts it is not the thread running the coroutine. Asserting the thread is deterministic, where a duration assertion would be timing-dependent on a shared runner. I confirmed the pin bites: reverting just the to_thread wrapper fails it on assert resolve_threads[0] is not loop_thread.

Gates re-run on the rebased head (origin/main = d84b21391696): black-scope, isort, flake8, mypy --platform linux (1261 files), subprocess-encoding, brand, harness-parity all exit 0; 614 tests pass across test_slack_gateway.py, test_cli_server_more_coverage.py, test_slack_gateway_more_coverage.py, test_spawn_audit.py, test_security_posture.py, test_no_blocking_call_on_loop.py, test_update_provider.py.

[bolichen97]

• Point patch with counted siblings — 4 sites still resolve the bare name, one in the same file this PR edits — rebutted on the code, accepted and already closed on the disclosure.

grepping "kiro-cli" spawns/gates in src/kiro_crew leaves 4 sites still resolving the bare name — slack/gateway.py:2382 (_warn_if_kiro_cli_outdated; the exact #7704 operator never gets the outdated-version warning, same file this PR edits), diagnostics.py:281 (reports "unavailable" for a fixed-dir install), cli_setup.py:146, and cli_doctor.py:2504/2510/3105. The gateway one is a one-line fold-in; the description's scope statement ("Two auto-update steps") never mentions any of them.

The four sites are real — I verified each one — and the disclosure complaint was fair. That half is closed: the description now carries a ## Scope: the sibling bare-name sites are deliberately not in this PR section naming all four and the reasoning for each. Widening the code is what I am declining, for four independent reasons.

1. This PR does not create the incoherence, and does not deepen it. Before it, the updater and the version probe agreed with each other while both disagreed with the binary the agent actually spawns (acp/client.py, through this same resolver). After it, the updater is newly correct; the probe is not newly wrong. So the premise that this change manufactures a split is not what the diff does.

2. The gateway remedy is not the "one-line fold-in" the finding describes. _warn_if_kiro_cli_outdated is a declared test/test_spawn_audit.py sandbox exemption, annotated Fixed argv ("kiro-cli --version") — changing the argv to a resolved path changes what that audit row asserts. It also carries a documented never-raises contract, and test/test_slack_gateway_more_coverage.py carries 9 references to the helper across tests that patch asyncio.create_subprocess_exec, each of which would need the resolver patched too. Folding it in drags a security-audit annotation and a third test file into a targeted auto-update fix.

3. The strongest argument against the fold-in is the finding one lane over. _warn_if_kiro_cli_outdated runs on the gateway boot path. Putting resolve_kiro_cli() there would place the same synchronous stat-and-glob walk on the event loop — manufacturing a second instance of exactly the defect GPT 5.6 blocked this PR on, in the one place where a stall is most expensive. The prescribed remedy is not merely disproportional here; applied naively it is a regression.

4. The remaining two are a different surface with different intended semantics. cli_setup.py:146's message literally reads kiro-cli not found on PATH — PATH is the semantics that advice is written against, so swapping the probe without rewriting the advice makes the advice wrong. diagnostics.py and cli_doctor.py are kirocrew doctor / setup surfaces this PR deliberately does not touch, and docs/system-specs/modules/cli.md:848 documents the doctor check as kiro-cli binary in PATH — changing it is a documented-behaviour change owing a spec edit, which is a separate PR, not a rider.

None of the four is a reachable Critical/High: no crash, no security hole, no data loss, no removed guard. The worst case is a suppressed advisory warning and a misleading diagnostics row, both pre-existing and neither made worse here. That is what makes them deferrable to a follow-up rather than in scope for a fix whose stated purpose is the update path.

[bolichen97]

• Suggestion: gateway.py::_warn_if_kiro_cli_outdated and diagnostics.py::_kiro_cli_version still spawn the bare name — rebutted as disproportional to this PR; the concern itself is real and now disclosed.

gateway.py::_warn_if_kiro_cli_outdated and diagnostics.py::_kiro_cli_version still spawn the bare name — the same fixed-dir-only install this PR now updates still evades the version warning; a one-line follow-up with the same resolver closes the family.

Agreed on the diagnosis, and the verdict is PASS so nothing is blocked. I am declining the fold-in for this PR, and the description now names all the sibling sites under ## Scope: the sibling bare-name sites are deliberately not in this PR so no reader has to rediscover them.

The decisive reason is that the suggested change would land the defect the blocking lane just flagged. _warn_if_kiro_cli_outdated runs on the gateway boot path, and resolve_kiro_cli() is a synchronous stat-and-glob walk (every fixed candidate dir, every inherited PATH entry, plus a cold-cache glob over each mise/asdf/nvm/fnm version root). GPT 5.6 blocked this PR precisely for putting that call on the event loop in _auto_apply_update; I have now offloaded it with asyncio.to_thread. Repeating the bare call in a boot-path helper would recreate the stall in the costliest place, so "one line with the same resolver" is not actually the shape of a correct follow-up — it needs the offload too, which makes it its own change.

Secondary cost, for the record: _warn_if_kiro_cli_outdated is a declared test/test_spawn_audit.py sandbox exemption annotated Fixed argv ("kiro-cli --version"), and test/test_slack_gateway_more_coverage.py has 9 references to it across tests that patch asyncio.create_subprocess_exec. diagnostics.py is a kirocrew doctor surface whose PATH semantics are documented at docs/system-specs/modules/cli.md:848, so changing it owes a spec edit in the same commit. Both belong to a follow-up scoped to the reporting surfaces, not to a fix scoped to the update path.

Neither site is a reachable Critical/High — the worst case is a suppressed advisory version warning and a diagnostics row reading "unavailable", both pre-existing and neither made worse by this diff.

[bolichen97]

• BLOCKING -- src/kiro_crew/slack/gateway.py:9637 -- fixed-directory lookup executes agent-writable binaries unsandboxed (Anchor: residual/security) — rebutted. The premise is real; the chain is main's, not this diff's, and the demanded remedy closes none of it.

kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli)
Prompt-injected auto-approved write plants ~/.local/bin/kiro-cli -> unattended update resolves it -> attacker code runs with gateway privileges.
Anchor: residual/security
Fix: Restore the PATH-only lookup at this unattended call site.

The half that is true, stated plainly. ~/.local/bin is candidate #1 on POSIX (kiro_cli.py:195), and it is fenced from an agent write by nothing: security.is_sensitive_path("~/.local/bin/kiro-cli") and is_sensitive_write_path(...) both return False, so it is in neither _SENSITIVE_HOME_DIRS nor _WRITE_PROTECTED_HOME_PATHS. An auto-approved agent write there is not refused. I did not take that on report — I ran both predicates.

Where the chain completes, though, is acp/client.py:298, on main. _resolve_kiro_bin calls the same resolve_kiro_cli(), and the ACP session spawn execs its answer at every session start. I measured the precedence rather than assuming it: with PATH=/opt/kiro/bin:/usr/bin:/home/u/.local/bin, known_kiro_cli_dirs() returns ~/.local/bin at index 0 and the PATH entry /opt/kiro/bin at index 20 (linux) / 22 (darwin). So a planted ~/.local/bin/kiro-cli already displaces a real PATH-named install for the agent process itself. And that spawn is the one Crew's own seatbelt is deliberately not applied to: acp/client.py:3021 passes is_kiro_cli=self.backend in ACP_BACKENDS_INTERNAL_SANDBOX, and sandbox.py:5089's delegate_to_kiro skips the seatbelt on macOS and grants the Windows delegation — a delegation a planted binary does not honour. An attacker who can write that file owns the agent process, with its credentials, its MCP tools and its session, before any timer fires. The maintenance spawn hands over nothing further.

The remedy does not remove the branch it names. shutil.which("kiro-cli") and a bare-name create_subprocess_exec("kiro-cli", ...) both resolve through the gateway's PATH — and ~/.local/bin is where Kiro CLI's own installer puts the binary (docs/reference/kiro-cli/installation.md:41: "installs to ~/.local/bin"), which is why it is normally on PATH. On the ordinary install the pre-PR code execs the identical planted file. What PATH-only actually subtracts is the case where the directory is not on PATH — which is #7704 itself, and not a corner case: env.augmented_path's own docstring (env.py:636) records that under systemd or another non-login shell the inherited $PATH "rarely includes directories like ~/.local/bin".

And this very function already refuses that pattern, 486 lines above the flagged line. gateway.py:9151 resolves git off PATH — _git = platform_compat.trusted_git_bin() — under a comment that names the exact hazard: "A gateway's PATH can lead with an agent-writable directory (a worktree venv's bin, ~/.local/bin), so a bare "git" lets a planted shim run". On _auto_apply_update, PATH is the less trusted input, and a refusal there aborts the whole apply. Restoring a PATH-only lookup for kiro-cli would re-adopt for one binary the precise pattern the function hardened away for the other.

I weighed the four alternatives and rejected each on this repo's own recorded decisions, not on taste:

Branch	Verdict
Restore PATH-only at the unattended site (as demanded)	Rejected. Subtracts only the off-PATH case = #7704; leaves the planted file reachable whenever ~/.local/bin is on PATH; contradicts trusted_git_bin() in the same function.
Restrict the search order on unattended paths only	Rejected. Splits resolution, so the update targets an install the agent does not launch — the launched one then stays stale forever, and nothing reports it. Also a per-caller capability keyed on "is this attended", which is the absence-shaped test AGENTS.md forbids.
Make the resolver refuse an agent-writable candidate always	Rejected. kiro_prerequisite.py:628 is the standing decision: "Trust is 'the CLI runs': install source, owner, and path do not gate launch". The only "location an agent cannot write" model in the tree is platform_compat.trusted_system_bin, whose POSIX set is ("/usr/bin", "/bin", "/usr/sbin", "/sbin", "/run/current-system/sw/bin") (platform_compat.py:1235) — kiro-cli is never installed there, so applying it resolves None on every supported host and disables the agent outright.
Sandbox-route the update spawn / verify provenance before spawning	Rejected. The sibling entry in test_spawn_audit.py:1198 states the reason for its own class: _auto_apply_wheel_update is "NOT sandbox-routed because the installer must write to the managed venv and symlink ~/.local/bin/kirocrew". A self-update cannot run confined. Provenance verification is the row above.
Fence the trusted-runtime binary from agent writes (security.py)	Correct, and not this PR's. See below.

What I landed instead, since the finding did surface something this diff genuinely owed. Both sites sit in test_spawn_audit.py's BENIGN_SPAWNS on the stated basis that their argv is "fixed argv against our own install" — and that basis moves the moment argv[0] becomes a resolved path. Each entry now carries its own justification naming what actually bounds it (one resolver, shared with the ACP session spawn, so the step can only ever name the binary the agent itself runs, and no agent-supplied value reaches the argv) and records the residual it does not close. test_kiro_cli_update_spawns_resolve_through_the_shared_resolver pins that invariant across all three sites — cli_server.py::_update, slack/gateway.py::_auto_apply_update, acp/client.py::_resolve_kiro_bin — so the update target and the launch target cannot drift apart, and neither site can reintroduce a bare name or a shutil.which gate. docs/system-specs/modules/cli.md § Update Command documents the step and how its path is resolved. I confirmed all four of the test's branches bite by mutating production code and watching it fail: re-adding shutil.which("kiro-cli"), reverting argv[0] to the bare name, adding an argv element, and swapping the resolver out of acp/client.py.

The residual is escalated, not waived. Fencing the trusted agent runtime from agent writes is the change that would close every current and future site at once, and it is a keystone security.py decision rather than a line in an auto-update patch — for one concrete reason beyond scope: kiro-cli 2.15+ is a multi-call binary that execs SIBLING executables resolved relative to its own path (kiro_prerequisite.py's snapshot docstring), so a write fence on the kiro-cli leaf alone leaves kiro-cli-chat plantable, while fencing the whole ~/.local/bin directory would deny the legitimate agent writes that put tools there (pip install --user, uv tool install, cargo install). Choosing between those is a product ruling, it belongs to _WRITE_PROTECTED_HOME_PATHS alongside .kiro/agents — which is fenced for the strictly weaker version of this same hazard — and it is filed for its own PR. Narrowing one maintenance spawn would not have moved it.

Gates on the rebased head (origin/main = 73d959df80c4): black-scope, subprocess-encoding, isort, flake8, mypy --platform linux (1262 files), docs-lint, brand and harness-parity all exit 0; 3327 tests pass across test_spawn_audit.py, test_security_posture.py, test_security.py, test_slack_gateway.py, test_slack_gateway_more_coverage.py, test_cli_server_more_coverage.py, test_kiro_prerequisite.py, test_acp_client.py, test_update_agent_refresh.py, test_env.py, test_cli_doctor.py, test_no_blocking_call_on_loop.py, test_denied_commands_security.py. Coverage Gate on the previous head was a dependent, not a second defect: its log reads backend-test=cancelled -- failing closed, and Backend Tests (3.12, 4) was cancelled by the 40-minute job timeout with pytest still running and no test failure reported.

[bolichen97]

• BLOCKING -- src/kiro_crew/slack/gateway.py:9637, src/kiro_crew/cli_server.py:1377 -- Fixed-directory resolution executes an agent-writable CLI unsandboxed (Anchor: backend-security-controls) -- needs-a-decision.

kiro_cli_bin = await asyncio.to_thread(resolve_kiro_cli) / kiro_cli_bin = resolve_kiro_cli()
Approved agent write to ~/.local/bin/kiro-cli -> auto-update selects it before PATH -> payload executes outside the agent sandbox.
Anchor: backend-security-controls
Fix: Revert both update sites to the previous PATH-gated lookup until fixed candidate directories are write-protected.

I am not overriding this and I am not applying the demanded remedy. Below is what I verified holds, what does not, and the two rulings the residual needs. An earlier disposition of mine argued the marginal capability of this diff is empty; that argument was wrong on Linux and I am withdrawing it.

Conceded: on Linux there is a real confinement delta, and it is this diff's

Every step run against this branch:

1. agent.sandbox defaults to "auto" (config/loader.py:2247), and sandbox.py's module docstring states "auto" uses the namespace sandbox on Linux.
2. The ACP session spawn is wrapped: acp/client.py:3163 calls wrap_argv_async(...) with is_kiro_cli=self.backend in ACP_BACKENDS_INTERNAL_SANDBOX.
3. Every is_kiro_cli waiver is platform-gated to darwin or win32 -- sandbox.py:5157-5159 is delegate_to_kiro = (sys.platform == "darwin" and kiro_spawn and kiro_internal_sandbox_enabled()) or (sys.platform == "win32" and is_kiro_cli is True). There is no Linux waiver, so on Linux the session spawn genuinely runs in the namespace sandbox.
4. Both changed update sites are unrouted, which is exactly what their BENIGN_SPAWNS entries declare.
5. The resolver puts ~/.local/bin at index 0 and ~/.cargo/bin at index 1, ahead of every inherited-PATH entry (kiro_cli.py:194-197), and kiro_prerequisite.py:628-629 records that trust is "the CLI runs": install source, owner and path do not gate launch.
6. The runtime is unfenced from an agent write. Against this branch, security.is_sensitive_path('~/.local/bin/kiro-cli') is False, security.is_sensitive_write_path(...) is False, and security.is_sensitive_bash_command('cp payload ~/.local/bin/kiro-cli') is None -- all three, for both the tilde and the absolute spelling.

So on a Linux host whose invoking PATH does not name ~/.local/bin while the real Kiro CLI is elsewhere on PATH, a planted ~/.local/bin/kiro-cli was previously exec'd only by the namespace-confined session spawn, and after this diff the update site also execs it with no credential isolation and no RLIMIT ceiling. That is the reviewer's sentence, and I am not disputing it.

Rejected: the demanded remedy, because it narrows the class rather than closing it

On origin/main both sites spawn a bare name with no env=: subprocess.run(["kiro-cli", "update"], ...) at cli_server.py:1371-1375 and await asyncio.create_subprocess_exec("kiro-cli", "update", ...) at slack/gateway.py:9625-9630. shutil.which gates the call and its answer is then discarded, so the file that actually executes is re-resolved by the child's execvp through the inherited PATH.

Two consequences for the remedy:

• It does not remove unconfined execution of an agent-writable file at these sites. Any agent-writable directory on the inherited PATH already yields one, and ~/.local/bin is on PATH on an ordinary install -- which is where Kiro CLI's own installer puts the binary and why it is the resolver's index-0 candidate. On that ordinary host the pre-PR code execs the identical planted file. What reverting subtracts is only the off-PATH case.
• The off-PATH case is the bug (Bare-name kiro-cli spawns: auto-update checks and gateway boot version probe miss fixed-dir installs #7704). Reverting restores a lookup that resolves differently from the launch path, so kiro-cli update updates an install the agent does not launch and leaves the launched one stale, with nothing reporting it. It also re-adopts, for this binary, the pattern the same gateway function hardened away for another (_git = platform_compat.trusted_git_bin(), whose comment names this exact hazard).

test_kiro_cli_update_spawns_resolve_through_the_shared_resolver in test/test_spawn_audit.py pins the one-resolver invariant across all three sites that exec the user's CLI, and I confirmed it bites: reverting either site to shutil.which plus a bare-name argv fails it.

Why neither half of the closure is inside this PR

The confinement half. Routing the maintenance spawn is not additive here. test_every_routed_spawn_applies_resource_limits requires every sandbox-routed spawn to also carry preexec_fn=resource_limit_preexec() (RLIMIT_AS / CPU / NPROC / NOFILE) or a justified PREEXEC_EXEMPT entry, and the repo already ruled on this exact class of spawn: slack/gateway.py::_auto_apply_wheel_update is listed "NOT sandbox-routed because the installer must write to the managed venv and symlink ~/.local/bin/kirocrew". kiro-cli update has the same shape -- it must write its own install directory and reach the network. Whether it survives the namespace sandbox plus an address-space and CPU ceiling is an empirical question I cannot answer here, because the only way to run it is to mutate the operator's real Kiro CLI installation. I will not land a confinement change to a best-effort maintenance step on an untested assumption that the step still works.

The write half, which is the reviewer's own precondition ("until fixed candidate directories are write-protected"). Three mechanisms exist and each costs something a maintainer should price, not an auto-update patch:

• _WRITE_PROTECTED_HOME_PATHS is file-edit-only. I verified the split: is_sensitive_bash_command('cp evil.py ~/.kiro/crew/app-sources/x/main.py') returns None even though app-sources is in that list. So an entry there leaves cp payload ~/.local/bin/kiro-cli permitted and does not make the index-0 candidate non-agent-writable -- it would read as a closure and enforce half of one.
• _WRITE_PROTECTED_BASH_LEAVES matches any command naming the leaf, and that list's own scope note forbids a generic leaf for this reason: .local/bin would refuse ls ~/.local/bin, pip install --user, and a PATH export.
• The path-shaped alternative in the sensitive-command regex (agents_write_path / win_agents_write_path, security.py:7351-7367, wired in at :7435-7436) is what actually blocks cp payload ~/.kiro/agents/evil.json. But it is verb-independent -- I confirmed cat, grep -r and ls against ~/.kiro/agents are all blocked too -- so the same shape over a general-purpose user bin directory would refuse routine agent reads, listings and tool invocations under ~/.local/bin and ~/.cargo/bin.

A leaf-only fence on the kiro-cli name is also insufficient by this repo's own record: kiro_prerequisite.py:611 and :2850 document Kiro CLI 2.15+ as a multi-call binary that execs sibling executables resolved relative to its own path, so a sibling stays plantable. And no fixed-directory fence reaches the resolver's inherited-PATH tail, which cannot be enumerated.

One more piece of the repo's own record, because it frames what the ruling is about rather than excusing the delta. kiro_prerequisite.py:624-626 states that a provenance gate on the CLI "defends against an attacker who already has write access to the user's own machine -- a threat the rest of the product does not defend against either -- and the cost was breaking every multi-call and multiplexer install outright." An approved agent write to ~/.local/bin/kiro-cli is that attacker. The question a maintainer has to settle is whether the agent's own write tools should be inside that boundary even though a human with the same shell is not, and if so at what cost to routine writes under a general-purpose user bin directory. That is a product decision on the keystone file, with its own reproduction, and it does not depend on #7704 being fixed or unfixed.

What I am doing

Keeping the code, because the fix it makes is correct and the remedy would undo it while leaving the class open. Escalating the residual as its own keystone change with its reproduction: it is a security.py decision about what the agent may write into the trusted runtime, and it is main's condition today at a higher-privilege site than either line flagged here. I am deliberately not using the lane's override line -- the attribution half of my earlier rebuttal was wrong, the residual is real, and an unrefuted security finding is not something to override.

[bolichen97]

• The fix is cause-level and its deferred siblings are counted — but a standing repo-wide AST enforcement and a thread-offload behavior change ship undeclared inside it — fixed on the disclosure (the description now declares all three), rebutted on removing any of them.

Item 4 is a shipped timing change the description's Fix section never states ("replaced the gate and spawn the resolved path" — no mention of asyncio.to_thread); it is derived (resolve_kiro_cli globs tool-manager roots via env.augmented_path, so on-loop it stalls chats), but a human should see it declared.
Item 5's resolver-presence half is a second spelling for the two update sites: 3 behavioral tests in this same PR (test_fixed_dir_only_…, test_no_kiro_cli_…, the gateway venv test) already fail on a revert to shutil.which. Its unique value is the acp/client.py::_resolve_kiro_bin pin and the argv-shape check the BENIGN_SPAWNS exemption rests on.

The disclosure complaint is correct and it is the part I owed. The description now carries the thread offload as an explicit bullet in the Fix section — the mechanism (resolve_kiro_cli() reaches env.augmented_path(), which globs every mise/asdf/nvm/fnm version root, where the shutil.which it replaces walked os.environ["PATH"] only), why cli_server.py::_update is deliberately left synchronous, and the test that pins it. Items 5 and 8 already had their own section further down, but nothing tied them to the Fix section a reader starts from; the residual paragraph there is also corrected in this same edit, because it named _WRITE_PROTECTED_HOME_PATHS as the closure and that list is enforced at the file-edit gate only.

On removing any of the three, the reasoning differs per item and none of them is optional for this diff:

Item 4 is not separable from the fix. The offload exists because of the fix: the new resolver call is what puts a glob of every tool-manager version root on the gateway event loop, so shipping the resolver swap without the offload would introduce a loop stall that dashboard.loop_stall_exit_after_secs can turn into a watchdog kill. The GPT lane blocked an earlier head on exactly that. A separate PR for the offload would mean landing the stall first on purpose.

Item 8 is owed by the diff rather than riding on it. Both functions were in BENIGN_SPAWNS on the module docstring's stated basis that a self-update spawn is a "fixed argv against our own install". This change makes argv[0] a resolved path, so that basis no longer describes either site. Leaving the entries unannotated would leave the audit asserting something the code stopped doing — a silent exemption is the failure mode that set is written to prevent.

Item 5 I agree with on the narrow point and am keeping anyway, for the reason the lane itself names. The three behavioural tests do fail a revert to shutil.which at the two update sites; what they do not cover is (a) acp/client.py::_resolve_kiro_bin, which has no behavioural test in this diff at all and is the site that makes "one resolver" a shared invariant rather than a local habit, (b) the argv-shape assertion that the BENIGN_SPAWNS justification literally rests on — that the resolved path stays the only non-literal argv element and the subcommand list stays exactly ["update"] — and (c) a revert to some other second lookup that still yields a path, which a test patching resolve_kiro_cli would not notice because the patch simply never gets reached. Its overlap with the behavioural tests is one of its four branches; I confirmed each branch bites by mutating production code, including the acp/client.py one.

On the siblings, the lane's own verdict this round is that they are declared with per-site reasons and accepted-and-deferred, which matches the record; I am not re-litigating them here. I am taking the closing suggestion: diagnostics.py::_kiro_cli_version is the shape-identical one and will be the follow-up's first line.

[bolichen97]

Audit note — #7964 fixes the same six lines; it is being closed, and it leaves you one thing to absorb

#7964 ("fix(security): pin the auto-update kiro-cli spawn off PATH") rewrites the same
six-line block this PR does, so only one could land. Three reviewers — the last two working
independently and told that this PR is the repository owner's while #7964 is an outside
contributor's — ruled this the survivor, on a measured ground rather than a stylistic one:
trusted_system_bin("kiro-cli") returns None on every documented install (the intersection of
known_kiro_cli_dirs() with _TRUSTED_SYSTEM_BIN_DIRS is empty on linux, darwin and win32), so
#7964's guard would skip the update step rather than harden it. This PR is also the broader fix:
it covers the second identical site in cli_server.py::_update, adds the AST audit pinning all
three kiro-cli exec sites, moves the gateway resolution off the event loop, and updates the
cli.md spec.

Please pick this up — it is real, and it is the contributor's

env=_trusted_path_env() on the spawn. This PR pins argv[0] but lets the child inherit the
gateway's whole environment, so the child still resolves its own helper words through an
agent-influenceable PATH, and still inherits PYTHONPATH / PYTHONHOME / LD_PRELOAD /
LD_LIBRARY_PATH / DYLD_*. #7964 narrows PATH to the trusted dirs and strips those loader
variables. It composes cleanly here because it is orthogonal to which resolver picks argv[0] —
layer it on this PR's absolute path, never on trusted_system_bin.

Caveats for that follow-up, both flagged by the reviewers: kiro-cli 2.15+ resolves its siblings
relative to its own path rather than through PATH, so the PATH-narrowing is defence-in-depth
rather than the main win — the loader-variable strip is the substantive part; and kiro-cli update is a downloader, so it needs the proxy/TLS variables (_trusted_path_env() preserves
them). Keep #7964's fail-closed refusal when _trusted_path_env() returns None.

Worth taking too: #7964's TestAutoApplyUpdateKiroCliExecPin asserts the spawn directly —
argv[0] equals the resolver's answer, os.path.isabs, env equals the trusted mapping, and no
spawn at all on a refusal path.

One note on this PR's own state

It is not mergeable as it stands — its blocking review finding is dispositioned rather than
cleared and mergeable_state is blocked. That is separate from the overlap ruling, but it is
the thing standing between this and landing.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (origin/main 680baf9448dc). Both PRs were read as their full merge-base diffs plus every comment and review. Because the surviving PR is the repository owner's and this one is not, the survivor question was deliberately re-decided by two further reviewers who were told the conflict of interest, were not shown the earlier answer, and were asked to settle the mechanism question by running the resolvers rather than by reading them. All three agreed, independently. If this is wrong, reopening costs nothing — say so, and treat the reasoning rather than the outcome as the thing to correct.}

[bolichen97]

• Fixed-directory resolution enables unsandboxed agent-planted execution (slack/gateway.py:9642, Anchor: residual/security) — needs-a-decision, and the decision has now been made: the fence is declined

Fix: Revert this path to the previous PATH-only lookup.

The mechanism is real and is main's condition rather than this diff's: resolve_kiro_cli() returns ~/.local/bin at index 0 and ~/.cargo/bin at index 1, ahead of every PATH entry, and none of is_sensitive_path, is_sensitive_write_path or is_sensitive_bash_command refuses an agent write to either, so acp/client.py::_resolve_kiro_bin execs whatever is there at every session start.

The remedy that would close it at the root — fencing the agent's write tools out of the trusted agent runtime, the way .kiro/agents is already fenced in _WRITE_PROTECTED_HOME_PATHS — has been put to the maintainer and declined. So this finding is not going to be closed by code, here or in a follow-up: the resolver's search order stays as it is by decision, and this PR's change (routing the two auto-update sites through the one resolver instead of hardcoding a path) neither creates nor widens the condition.

Recorded here so the ruling is visible on the PR rather than only in a chat. This PR still needs either an explicit override of this lane or a decision to accept the lane's revert, and the revert reinstates the bug #7704 reports.

[iamwhatever]

Drive-by from #8118, which briefly carried a duplicate of this fix before I found #7704 and dropped it — this PR is the better version and owns it. Handing over the one thing mine had that this one does not, then getting out of the way.

The gateway spawn inherits its environment; the git spawns beside it do not.

In _auto_apply_update, every git child is given env=_git_env — built by git_command_env(), which constructs the environment rather than merging over os.environ specifically so an inherited GIT_DIR is absent rather than overwritten, then adds the exec-vector pins and GIT_NO_REPLACE_OBJECTS=1. The kiro-cli update child a few lines later passes no env= at all, so it inherits the gateway's environment whole.

That matters here for a reason narrower than the residual you already adjudicated: kiro-cli update is a program that runs git of its own. With no env=, the git it spawns sees the untouched inherited environment — including any GIT_DIR, GIT_WORK_TREE, GIT_CONFIG_* or replace-ref exposure that _git_env exists to strip for the sibling calls in the same function. The pins the update path relies on stop at the process boundary.

Adding env=_git_env to that spawn is a one-line change that carries the same pins across the boundary. It is inert for anything in kiro-cli that is not git (the GIT_CONFIG_* keys mean nothing to a non-git process) and PATH / HOME survive, since git_command_env() is os.environ minus the git location vars.

Entirely your call whether it belongs in this PR or a follow-up — it is adjacent to your stated scope, not inside it, and I would not want to widen a PR that is already through review on the strength of a drive-by. Flagging it rather than pushing it, since #8118 no longer touches this file.

[bolichen97]

Closing without merging, by the author's decision.

The GPT lane's blocking finding on this branch has one remedy it will accept — restore the shutil.which("kiro-cli") PATH-only lookup on both auto-update sites — and that is exactly the behaviour #7704 reports as broken. The alternative remedy (fencing the resolved fixed-directory binary before the updater spawns it) was considered and declined: it would put the agent's own write tooling behind a trust check that the agent runtime is supposed to be inside, which is a larger posture change than an auto-update fix should carry.

With both ends closed, this branch has no path to green that leaves the fix in it, so it is withdrawn rather than overridden.

#7704 stays open. Nothing here is a rebuttal of the bug: kiro-cli update still silently skips a working install reachable only through a fixed known_kiro_cli_dirs() entry, and the updater and the agent spawn still disagree about which binary they are talking about. A future fix needs to settle the trust question for the resolved path first, and can then reuse this branch's tests (test_fixed_dir_only_kiro_cli_is_updated_not_skipped, test_no_kiro_cli_skips_the_backend_update) unchanged.

The four sibling bare-name sites documented in the description above (_warn_if_kiro_cli_outdated, diagnostics.py::_kiro_cli_version, cli_setup.py, cli_doctor.py) are unaffected and remain as described in #7704.