[pepmach]

Description

Pins the macOS sandbox capability probe to /usr/bin/sandbox-exec and /usr/bin/true. If either trusted system binary is missing, the probe now fails closed instead of resolving and executing a user-writable PATH fallback.

This closes security scan Finding 8 and adds host-independent regression coverage.

Related Issues

Security scan Finding 8; no public GitHub issue.

Testing

• 7 sandbox probe tests pass
• 114 dependent sandbox/spawn-audit tests pass
• New regression proves no subprocess runs when the trusted target is missing
• Black, isort, flake8, mypy, py_compile, and git diff --check pass

Checklist

• Code follows the project style guidelines
• Self-review completed
• Documentation updated
• No secrets, credentials, or internal references in the diff

[github-actions]

Codex Review (GPT-5.6 Sol via Bedrock)

No issues found - LGTM.

[github-actions]

Review — PR #27: fail closed on untrusted sandbox probes

Reviewed the diff against the base-branch AutoSDE rule set (backend-security-controls, no-blocking-call-on-event-loop, top-level-imports) and the additional security concerns.

Verdict: clean — this is a correct security hardening, approve.

The two changes in _probe_sandbox_exec() both move the probe toward fail-closed / deny-by-default, matching the backend-security-controls blocking rule:

• Fixed trusted path for the sandbox binary — shutil.which("sandbox-exec") -> /usr/bin/sandbox-exec + os.path.exists. Removes PATH-based resolution so a manipulated PATH can't substitute a spoofed sandbox-exec.
• Fail closed on a missing probe target — the old target = shutil.which("true") or target fell back to a PATH-resolved (user-writable/manipulable) true; the probe runs under (allow default) with KiroCrew's credentials, so exec'ing a planted target was a real risk. Now it return False if /usr/bin/true is absent. Correct.

Checks performed:

• No keystone / sensitive-path control weakened.
• shutil is still used elsewhere (sandbox.py:1378), so no unused-import (F401) regression.
• subprocess.run in the probe is pre-existing (not added by this PR) and runs on the sandbox-selection/setup path, not the asyncio event loop — no-blocking-call-on-event-loop does not apply.
• Tests updated correctly; the new test_missing_trusted_probe_binary_fails_closed asserts the probe fails closed without invoking subprocess.run.

No CRITICAL/HIGH/MEDIUM/LOW issues found.

[CLAUDE-REVIEWED] 6c2ff38

[pepmach]

Superseded by #38, which replays the same reviewed one-commit fix onto current main without force-pushing published history. Prior Codex and Claude reviews on this PR were clean.

[github-actions]

Codex Review (GPT-5.6 Sol via Bedrock)

No issues found - LGTM.

[CODEX-REVIEWED] 91376de

[github-actions]

Claude Review — ✅ no blocking findings

Review of PR #27 — fix: fail closed on untrusted sandbox probes

No blocking findings. This is a clean, security-positive change.

What the diff does

src/kiro_crew/sandbox.py::_probe_sandbox_exec() is hardened to use trusted fixed system paths and fail closed:

• sb = shutil.which("sandbox-exec") → sb = "/usr/bin/sandbox-exec" + os.path.exists(sb) (sandbox.py:192-193). Replaces a PATH lookup with a fixed system path, so a PATH-planted sandbox-exec can no longer be selected.
• target = shutil.which("true") or target → return False (sandbox.py:202-203). Previously, if /usr/bin/true was absent the probe fell back to a PATH lookup for true. Since the probe runs a target under an (allow default) profile with KiroCrew's credentials, a planted/user-writable true could have executed effectively unsandboxed. It now fails closed, matching the in-code rationale at sandbox.py:195-200.

Verification

• shutil is still referenced (sandbox.py:1378 shutil.which("systemd-run")), so no F401 regression.
• Tests updated to patch os.path.exists instead of shutil.which, and the new test_missing_trusted_probe_binary_fails_closed correctly asserts the fail-closed path (side_effect=[True, False], mock_run.assert_not_called(), call_count == 2).
• Behavior is unchanged on a normal macOS host where /usr/bin/sandbox-exec and /usr/bin/true both exist.

The change strengthens a security control (fails closed rather than open) with matching test coverage. Nothing meets the blocking bar.

Verdict recorded via the action's structured output for commit 91376dec28e18a5b0327316cd6ce2c8177e1f732.