fix(setup): ask for the unsandboxed-exec opt-in on a backend-less host

[chenmingwei23]

What is the problem?

On a host with no OS-level sandbox backend — any Windows host, or a Linux kernel that refuses user namespaces — a fresh install has zero working MCP tooling. wrap_argv() fail-closes, so kirocrew doctor reports every managed server failing:

@kirocrew-cron:     FAIL  Sandbox backend unavailable and allow_unsandboxed_exec is not set. Probe detail: not Linux.
@kirocrew-core:     FAIL  ...
@kirocrew-computer: FAIL  ...

and Dev Fleet answers {"error": "app 'dev-fleet' has no reachable backend"}.

The refusal is correct. What was missing is discoverability: the one config key that resolves it was something the user had to find on their own.

Note on scope: this PR originally proposed defaulting agent.sandbox_allow_unsandboxed_exec to true on win32. Both blocking reviewers rejected that, correctly — see the disposition comment below. The approach has been replaced.

Why this issue matters to the user

Every Windows user hit this on first run, and the failure looked like a bug rather than a decision awaiting their input. Note that the message half of this has since been fixed on main (_PROBE_ERROR_MAX_CHARS = 1200 — the old 200-char cap truncated the remedy sentence mid-word), so the remedy text now reaches doctor. What remains is that acting on it still means hand-editing config.json before anything works.

How our fix solves it (symptom → root cause → change)

Symptom: managed MCP servers, Dev Fleet and the Papyrus compiler all refuse to spawn on a fresh Windows install.

Root cause: detect_backend() is always "none" on win32, and agent.sandbox_allow_unsandboxed_exec defaults to False, so wrap_argv() raises for every non-off mode. Correct posture, undiscoverable remedy.

Why not a platform default. Deriving the fallback from sys.platform deletes a deny-by-default authorization and puts nothing in its place. The mode="strict" callers depend on it specifically:

• dashboard/handlers/worktree.py picks strict so a repo-controlled include.path cannot make git read ~/.aws/credentials
• the Papyrus compile path picks it so a crafted .tex cannot typeset a secret into a PDF

On win32 those would all have become unconfined spawns that no operator ever declared.

The change. The default stays platform-independent False, and kirocrew setup gains _setup_sandbox_consent():

• asks detect_backend() — platform knowledge stays with the probe rather than moving into the config layer (per the Design Review suggestion)
• silent no-op when a backend exists, and when the key is already present in either state, so it never re-asks
• on "none", states what becomes unconfined (~/.aws, ~/.ssh) and prompts, defaulting to no
• writes the key only on an explicit yes; declining, a bare Enter, and a non-interactive EOF (_input_or_skip → None) all leave the config untouched
• runs before the --agent-only early return, because the servers it unblocks are the ones install_agent() just wrote
• refuses to coerce a malformed agent section rather than clobbering it

Net effect: the posture the specs describe is unchanged — unconfined execution stays operator-declared — but the decision is now presented once, in context, with its cost stated.

What tests we did

New — test/test_sandbox_unsandboxed_exec_consent.py (12 tests): no prompt when a backend exists; no prompt when the key is already true or already false; y / YES write the opt-in; unrelated config keys preserved across the write; n, bare Enter, and EOF all write nothing; the decline path names both the config key and the absolute config path; the prompt names ~/.aws and ~/.ssh; a non-dict agent section is left untouched.

Extended — test_config_loader.py::test_sandbox_allow_unsandboxed_exec_default_is_platform_independent: a ratchet asserting the default is False under win32, linux and darwin, with and without an agent section. A future sys.platform == "win32" fallback now fails this test instead of shipping.

Local gate: pytest on the 10 files that touch cli_setup → 302 passed, 2 skipped. isort --check-only, flake8 -j 1, mypy on both changed modules → clean. scripts/docs_lint.py --test, scripts/docs-lint.sh (181 files), scripts/scrub-lint.sh --no-history → all pass.

Docs updated in the same commit (the same-commit spec rule): docs/system-specs/modules/security.md, docs/architecture/security-deep-dive.md, docs/guides/windows-install.md. The posture they document is unchanged; the wizard is now the primary path to the opt-in and hand-editing config.json is documented as the manual equivalent.

Rebased onto current main (was 13 behind).

Any other suggestions on the work

• A first-party carve-out is the better long-term answer, and belongs in its own PR — filed as Split first-party MCP spawns from hostile-input spawns in the unsandboxed-exec opt-in #1563. Every exploit path the reviewers named involves agent- or repo-controlled input reaching an unconfined spawn. The three managed MCP servers are first-party binaries with KiroCrew-controlled argv, so they are a different risk class. Splitting those would let Windows work with no opt-in at all while worktree.py and Papyrus stay fail-closed. That means adding an authorization axis to a security-critical function with ~30 call sites, so it wants its own design pass rather than riding along here.
• The dashboard could offer the same consent for users who never run kirocrew setup from a terminal (the packaged Electron app). Same fail-closed default, same explicit yes; only the surface differs.
• No UI screenshots: the change is a CLI wizard step and three doc edits. The behaviour is observable via kirocrew setup on a backend-less host and via kirocrew doctor before/after.

Closes #1525

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 56be8eaff0ae4b05eb9db18b6dbfce8ec4f63db8 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 56be8ea

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 56be8eaff0ae4b05eb9db18b6dbfce8ec4f63db8: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 56be8eaff0ae4b05eb9db18b6dbfce8ec4f63db8 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Consent-at-setup fixes the discoverability gap without weakening the fail-closed posture; classification logic correctly stays in sandbox.py, shared with wrap_argv so they cannot drift.

[DESIGN-REVIEWED] 56be8ea

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed 56be8eaff0ae4b05eb9db18b6dbfce8ec4f63db8 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 56be8ea

Verdict parsed from the review's SHA-scoped output markers for commit 56be8eaff0ae4b05eb9db18b6dbfce8ec4f63db8.

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

[chenmingwei23]

Disposition of every concern raised on d41c130e8

Both blocking findings were legitimate and are fixed by replacing the approach, not by narrowing the exploit. New SHA: 679c40c6f.

GPT 5.6 — BLOCKING — config/loader.py:4334, "Windows silently enables unsandboxed execution" → FIXED

Accepted in full. The fallback is restored to an unconditional False and the sys import that existed only for the platform check is gone. Your exploit chain (crafted TeX → Papyrus compile → sandboxed_spawn_argv() → unwrapped Tectonic → secret in the PDF) is closed because the Papyrus path fail-closes again exactly as before.

Opus 5 — BLOCKING — same site, plus dashboard/handlers/worktree.py:212 → FIXED

Accepted in full, and the worktree.py citation is the sharpest argument against the original patch: that call site chooses mode="strict" specifically so a repo-controlled include.path cannot make git read ~/.aws/credentials, and on win32 detect_backend() is always "none", so a platform default would have unwrapped it with nothing in its place. Restored as you asked, and the field metadata sentence about a Windows default is gone.

Both of you named "restore the fallback to False" as the fix. That alone would have left the original user problem unsolved, so the PR now also carries the missing half — see the Design Review response below.

Design Review (Fable 5) — 🟡 CONCERNS → FIXED (all three points adopted)

Advisory, but every point was right and each one changed the diff:

1. "the rejected alternative worth stating: keep fail-closed and make the probe failure actionable." Adopted as the shape of the fix. Worth recording that half of it had already landed on main while this branch sat 13 commits behind: _PROBE_ERROR_MAX_CHARS = 1200 replaced a 200-char cap that truncated the remedy sentence mid-word, so doctor now surfaces the key. That is why the remaining gap is acting on the remedy, not seeing it — which the new consent step covers.

2. "express the default inside sandbox.py … platform knowledge stays with the backend-probe logic; the config layer stays platform-agnostic." Adopted. The config layer is now platform-agnostic again, and the wizard asks detect_backend() rather than testing sys.platform. It keys on "none", so a Linux kernel that refuses user namespaces gets the same treatment as Windows — the condition is "no backend", not "is Windows".

3. "update security.md, security-deep-dive.md and windows-install.md in this PR." Done, in the same commit. The posture they document is unchanged, so these are additive: the wizard becomes the primary path to the opt-in and hand-editing config.json is documented as the manual equivalent. windows-install.md's manual-opt-in section is no longer the only route described.

4. "a maintainer should explicitly own that posture change; the code alone doesn't record the decision." This is the point that decided the redesign. There is now no posture change to own: unconfined execution stays operator-declared, and the operator declares it by answering a prompt that names ~/.aws and ~/.ssh as what becomes readable. Declining, a bare Enter, and a non-interactive EOF all leave the config untouched.

Guard against the same mistake recurring

test_config_loader.py::test_sandbox_allow_unsandboxed_exec_default_is_platform_independent asserts the default is False under win32, linux and darwin, with and without an agent section. A future sys.platform == "win32" fallback fails that test instead of shipping.

Deliberately deferred

A first-party carve-out — letting the three managed MCP servers (fixed, KiroCrew-controlled argv) spawn while worktree.py and Papyrus stay fail-closed — would make Windows work with no opt-in at all, and it targets the actual distinction both blocking findings turn on: agent/repo-controlled input versus first-party argv. It is deferred because it adds an authorization axis to a security-critical function with ~30 call sites and deserves its own design pass. Tracked as a follow-up rather than folded in here.

[chenmingwei23]

Round 2 dispositions — c9973980a → 451725e90

Both new GPT findings were real. Fixed, not narrowed.

GPT 5.6 — BLOCKING — cli_setup.py:664, "Unsandboxed-execution grant is not SEL-audited" → FIXED

Correct, and the asymmetry you point at is the strongest form of the argument: wrap_argv already emits a denied SEL event on every refusal, and the nested-sandbox passthrough emits allowed. Persisting the grant that silences those refusals was the one security-relevant decision on this path with no record.

The step now emits the event before the write, on an audit-or-deny basis (critical=True, so SEL writes synchronously and re-raises on a filesystem failure):

sel().log_tool_invocation(
    session_key="setup", agent="system",
    source="cli_setup._setup_sandbox_consent",
    tool_name="sandbox_allow_unsandboxed_exec", tool_kind="config",
    outcome="allowed", resources=str(cfg_file),
    metadata={"reason": "operator_consent_at_setup", "probe_backend": backend},
    critical=True,
)

If the audit cannot be written the grant is refused and the host stays fail-closed — a grant is never persisted unaudited. Ordering matters here and is deliberate: audit first, then write, so a crash between the two can only leave a record without a grant, never a grant without a record.

Scope note so this is not read as a stronger claim than it is: the documented manual config.json edit is still unaudited. That path predates this PR and is outside the wizard's control; what changed is that the path this PR adds is audited.

Covered by TestSetupSandboxConsentIsAudited: the grant emits exactly one event with outcome="allowed" and critical=True; an unwritable SEL log refuses the grant and writes nothing; declining emits no event; and skipping (a backend exists) emits no event.

GPT 5.6 — BLOCKING — cli_setup.py:636, "Non-object config crashes setup" → FIXED

Correct — config.json containing [] made cfg.get("agent") raise AttributeError and abort the whole wizard, which is worse than the problem this PR set out to solve. The read now type-checks the loaded document and returns early on a non-object, without rewriting it: coercing our own object over a top-level [] would destroy whatever the operator meant. Tests cover [] (skipped, file untouched) and malformed JSON ({not json — reported, byte-identical afterwards).

Design Review (Fable 5) — ✅ PASS, one suggestion → ACCEPTED-AND-DEFERRED

having doctor's sandbox-error output name the setup step (follow-up PR) would close the loop for the users the commit message describes

Agreed, and correctly scoped by you as a follow-up. Existing installs meet this failure at kirocrew doctor / a probe error, and today that text names the config key but not kirocrew setup. Deferring rather than folding in: the remedy string lives in sandbox.py's guidance block, so editing it touches the message asserted by the mcp_discovery truncation path and the Papyrus 422 — a small change, but a different blast radius than this PR's, and it deserves its own test pass.

Local gate on 451725e90

533 passed / 2 skipped across every test file that touches cli_setup, plus test_config_loader.py. isort --check-only, flake8 -j 1, mypy clean on the changed modules. docs-lint.sh passes. No added line uses the joined product name.

The security posture is unchanged from the previous round: agent.sandbox_allow_unsandboxed_exec still defaults to an unconditional False, and test_sandbox_allow_unsandboxed_exec_default_is_platform_independent still pins that across win32 / linux / darwin.

[chenmingwei23]

Round 3 dispositions — 451725e90 → 84e87d3d7

All three GPT findings were real. Fixed. Design Review's suggestion is now a filed issue.

GPT 5.6 — BLOCKING — cli_setup.py:701, "Config write failures crash setup" → FIXED

Correct, and the failure mode is worse than the bug this PR fixes: write_config_atomically raising OSError on a locked or read-only config (routine on Windows when another process holds it) would abort the entire wizard after the user had already answered the prompt. The write is now wrapped, and the message says plainly that nothing was granted and how to opt in by hand, so the host is left fail-closed rather than half-configured.

The audit-then-write ordering is unchanged and deliberate: a failure between the two can only leave a record without a grant, never a grant without a record. I made that explicit in the comment so the next reader does not "fix" it by moving the audit after the write.

GPT 5.6 — FINDING — cli_setup.py:645, "config.local.json override is ignored" → FIXED

Legitimate, and the sharper version of it is that the step could have lied: config_local_path() (loader.py:391) deep-merges over config.json at load (loader.py:4118), so a user with sandbox_allow_unsandboxed_exec: false in the overlay would have been prompted despite having already decided, and then told "Recorded: … = true" while the effective value stayed false.

The declared-check now inspects both documents via a shared _declared(path) helper, in either state. Because the write only happens when neither declares the key, the success message can no longer contradict the effective config.

Tests: an overlay declaring false suppresses the prompt; declaring true suppresses it; an unrelated overlay key (approval_mode) does not suppress it.

GPT 5.6 — FINDING — cli_setup.py:618, function-local imports → FIXED

Complied rather than leaned on the local convention. cli_setup.py does carry justified function-local imports (from kiro_crew.agent import install_agent # circular import: agent imports cli), but mine had no such justification because there is no cycle to avoid — I verified sandbox.py and sel.py contain zero references to cli_setup, and that importing all three at module scope resolves cleanly. detect_backend, sel and config_local_path are now module-scope imports.

Consequence worth noting for reviewers: the tests now patch cli_setup.detect_backend / cli_setup.sel rather than the source modules, because module-scope binding is what the function resolves.

Design Review (Fable 5) — ✅ PASS, one suggestion → FIXED (filed)

Track the first-party carve-out follow-up (issue, not prose): the single boolean still conflates "unblock Kiro Crew's own MCP servers" with "unconfine the mode=strict hostile-input paths" — resolving that granularity should land before the same consent pattern is replicated in the dashboard surface.

Agreed on both the substance and the form — prose in a PR body is not tracking. Filed as #1563, which records the two conflated decisions, the concrete mode="strict" call sites, the ~30-call-site classification problem, the _clamp_sandbox_mode() governance-floor interaction, the third SEL outcome a first-party unconfined spawn needs, and your ordering constraint that it lands before a dashboard consent surface. The PR body's prose now points at the issue instead of standing in for it.

Local gate on 84e87d3d7

537 passed / 2 skipped across every test file touching cli_setup, plus test_config_loader.py. isort --check-only, flake8 -j 1, mypy clean. docs-lint.sh passes. No added line uses the joined product name.

Security posture unchanged for the third round running: agent.sandbox_allow_unsandboxed_exec still defaults to an unconditional False, and test_sandbox_allow_unsandboxed_exec_default_is_platform_independent still pins it across win32 / linux / darwin.

[chenmingwei23]

Round 4 dispositions — 84e87d3d7 → 56be8eaff

Both findings were real, and the first one is the sharpest catch of this review — it caught the step contradicting a rule the sandbox layer already states in its own guidance text.

GPT 5.6 — BLOCKING — cli_setup.py:625, "Transient probe failure can persist a sandbox bypass" → FIXED

Correct, and worse than a hypothetical: sandbox.py's own transient guidance says in so many words that callers "must NOT advise disabling the sandbox for this; retry instead" — and my step was doing exactly that, because detect_backend() returns "none" for a momentary fork/resource failure as well as for a permanent absence. A Linux box under brief memory pressure could have been talked into a permanent opt-in for a condition that self-heals on the next spawn. Same defect for foreign_sandbox, where the host's sandbox works fine and the correct remedy hands isolation back to Kiro Crew rather than disabling it.

Rather than re-deriving the classification in the wizard (which would drift from wrap_argv's), I factored the rule into one implementation in sandbox.py:

• _classify_unavailable(transient) — the single expression, now called by both wrap_argv's SandboxUnavailableError(kind=…) and the new public accessor, so the two can never disagree about the same host.
• unavailable_kind() — returns "" when a backend exists, else "transient" / "foreign_sandbox" / "no_backend". Its docstring states the invariant for callers persisting an opt-in.

The wizard now acts only on "no_backend". "transient" prints a short note saying the probe re-probes automatically and deliberately does not name the flag; "foreign_sandbox" and "" are silent.

This also keeps platform knowledge in sandbox.py rather than the config or CLI layer, which is where the Design Review asked for it in round 1.

GPT 5.6 — BLOCKING — cli_setup.py:323, "Update can hang on an invisible consent prompt" → FIXED

Correct and confirmed against the call graph: cli.py:1953 invokes _setup(...), and on the update path stdout is captured while stdin is still inherited — so the prompt would have been invisible and blocking until that path's timeout killed the update. A question nobody can see is a hang, not consent.

The prompt is now gated on sys.stdin.isatty() and sys.stdout.isatty(). Non-interactive stdio prints the remedy instead (it lands in the captured output, so it is not lost) and returns without prompting, leaving the host fail-closed.

Test-suite note (transparency on a wider local run)

I widened the local run to every file matching cli_setup or sandbox and saw 13 failures. Per the worktree skill's triage rule I re-ran that same set on clean main (ec41aa8e8) and got the identical 13 test IDs — test_sandbox_argv.py (10), test_source_providers.py (2), test_service.py::TestTrustedToolResolution (1), test_dev_fleet_app.py::test_trusted_bin_pins_… (1). They are host-environment failures on this box (unshare(CLONE_NEWUSER) → EPERM inside the agent sandbox, plus root-ownership assertions that cannot be judged from a uid-mapped namespace), not regressions from this branch. Flagging rather than quietly labelling them flaky.

Local gate on 56be8eaff

540 passed / 2 skipped across the consent tests, test_config_loader.py, and every file touching cli_setup. isort --check-only, flake8 -j 1, mypy clean on both changed modules. docs-lint.sh passes. No added line uses the joined product name.

New coverage this round: a transient classification offers no opt-in and does not name the flag; a foreign sandbox is silent; non-interactive stdio prints the remedy and never prompts.

Design Review (Fable 5) — ✅ PASS, no new suggestions → noted

Third consecutive PASS, and the round-3 follow-up it asked for is filed as #1563.

Security posture unchanged for the fourth round: agent.sandbox_allow_unsandboxed_exec still defaults to an unconditional False, and the platform-independence ratchet still pins it across win32 / linux / darwin. This round made the opt-in harder to reach, not easier.