fix(runtime): allow bounded Apple Git startup

[Dante-dan]

Summary

• resolve the active DEVELOPER_DIR or xcode-select result through realpath, require a valid Apple signature and CLT/Xcode layout, and grant only its runtime library directories for writable command profiles
• keep explicit exact, aliased, and parent denies effective over executable and runtime-readable roots, including filesystem-worker dylib roots; restricted read-only profiles receive no new developer-toolchain grant
• leave Git config, includes, excludes, credential helpers, $HOME, arbitrary PATH prefixes, and network access entirely under the child process's existing permission profile; the host no longer invokes Git to discover or authorize config files
• require canonical usr/lib and SharedFrameworks roots to stay inside the selected developer directory and Xcode bundle, respectively, before granting them to Seatbelt; symlink escapes fail closed

The selector path is canonicalized and libxcrun.dylib is signature-checked immediately before Seatbelt policy construction. The canonical path is not fd-pinned, so replacement of the canonical toolchain directory between validation and process startup remains a residual limitation; this change does not claim to eliminate that race.

This builds on hqhq1025's investigation in #4279 and the security/compatibility constraints identified in that review.

Refs #5260

Verification

Validated locally on 9f8d3d71c after merging main at c64680be5:

• npm run build
• npm run lint and npm run format:check
• npm run windows:inventory (3 tests; 119 declarations)
• 31 focused resolver, signer, and Seatbelt policy tests, including both symlink escape regressions

The Windows sandbox W0 job on the prior head failed when Node could not read package.json inside AppContainer; the scheduled main job failed with the same package-config error. I reported both logs to the Windows maintainer in this comment. The updated head awaits remote checks.

The managed host blocks nested Seatbelt at sandbox_apply (exit 71), including the repository's pre-existing positive smoke cases, so no end-to-end Xcode/CLT Seatbelt pass is claimed.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the sandbox path resolver, policy changes, and regression tests.

Checklist

• Tests cover the change and fail without it
• Lint, format, affected-workspace typecheck/build, and affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[likun666661]

Thanks for digging into the Apple Git failure. The direction is reasonable, but I do not think this is ready to merge yet. The current evidence proves resolver behavior and policy construction; it does not yet prove the end-to-end contract on the environments this change is intended to fix.

The main gaps I would like addressed:

1. No real full-Xcode Seatbelt smoke test. The PR explicitly says the nested sandbox-exec smoke could not complete and that a physical Xcode.app installation was not tested. Fixtures and a CLT-only host cannot establish that /usr/bin/git can start, load the selected Xcode toolchain, and execute a normal Git command under the actual Seatbelt policy. Please either add a real-host test/recorded acceptance evidence or narrow the PR's claim to CLT-only support.

2. The permission-composition invariant needs an explicit negative test. Runtime-root allowances are generated separately from ordinary path rules. Please add tests showing that an explicit deny still wins when it overlaps an admitted toolchain/runtime root, including aliases, symlinks, and a deny on a parent of the selected root. This is the security boundary, not just a resolver detail.

3. Git config discovery must not become a host-side read bypass. include, conditional include, core.excludesFile, and nested includes need to be resolved under the same authorization contract as the child process. Please demonstrate that the host does not read an arbitrary config first and then grant the discovered paths, and test that readable config files remain non-writable.

4. Path replacement/TOCTOU needs a stated invariant. Resolving and signing the developer directory before launch is not sufficient if the path can be replaced before or during process startup. Please document what is pinned, when it is revalidated, and add a test for replacement between discovery and launch (or explicitly state the residual limitation).

5. Scope should stay bounded. This fix should not silently change existing restricted-read sessions or broaden access to /Applications, $HOME, arbitrary PATH prefixes, credential helpers, or network access. Please make that compatibility/security contract explicit in the PR description and acceptance tests.

The CI results are useful, but they do not substitute for the missing end-to-end macOS evidence. I would be happy to re-review once the real Xcode path and the deny/authorization composition cases are demonstrated.

[Dante-dan]

Addressed the authorization and scope concerns in 7894f8b75:

• Removed the host-side /usr/bin/git config --global --includes discovery path and the associated exact-file grants. Git config, nested includes, excludes, credentials, and $HOME now remain entirely subject to the child process's existing profile; a Bash wiring regression verifies that GIT_CONFIG_GLOBAL cannot turn config paths into policy parameters.
• Limited the new selected-toolchain roots to writable command profiles. Restricted read-only profiles return no developer roots, and no /Applications, home, PATH-prefix, credential, write, or network grant was added.
• Applied explicit denies to executable/runtime root clauses and added coverage for a canonicalized symlink alias plus a deny on the selected root's parent.
• Canonicalization and signature validation happen immediately before policy construction. The selector-alias replacement regression shows that changing the alias afterward does not redirect the emitted root. The canonical directory itself is not fd-pinned, so replacement between validation and process startup remains an explicit residual limitation rather than a claimed guarantee.

Validation completed: lint and format; affected runtime build; 87 focused resolver/policy/Bash-wiring tests; both knip checks; and resolver validation against this host's selected CLT plus its physical /Applications/Xcode.app. The host rejects nested Seatbelt at sandbox_apply with exit 71 (the pre-existing positive smoke cases fail the same way), so I am not claiming an end-to-end Seatbelt pass. The root build/typecheck/test commands also stop in @maka/ui on existing settledText, autoScroll, and trailingAction type mismatches; the affected runtime checks pass.

[Astro-Han]

Thanks — the direction looks right and the resolver is well structured. I focused on what the "bounded" bound actually is, the platform gating, and the security claim the Summary makes.

What is bounded

The bounds are the two synchronous discovery subprocesses in packages/runtime/src/sandbox/macos-command-paths.ts: xcode-select -p (1 s — lines 27, 97–103) and codesign --verify (1 s — lines 27–28, 106–112). spawnSync with timeout kills and reaps the child, and stdio: 'ignore' on the codesign probe means there's no pipe or fd to leak, so cleanup on timeout is fine. There are no retries, which I think is correct — retrying a toolchain probe just adds latency. The numbers look reasonable; my concerns below are about coverage and cost, not the values.

codesign --verify --strict does not establish an Apple signature

macos-command-paths.ts:107. --verify only checks that whatever signature is present is internally valid; it does not pin the signer. Verified locally on macOS:

# /bin/ls copied and ad-hoc signed with `codesign -s -`
$ codesign --verify --strict ./libxcrun.dylib ; echo $?
0
$ codesign --verify --strict -R="anchor apple" ./libxcrun.dylib ; echo $?
3
$ codesign --verify --strict -R="anchor apple" \
    /Library/Developer/CommandLineTools/usr/lib/libxcrun.dylib ; echo $?
0

The real libxcrun.dylib carries Authority=Apple Code Signing / TeamIdentifier=59GAB85EFG; an ad-hoc one carries Signature=adhoc and passes the current check. So the Summary's "require a valid Apple signature" and the code comment's "code-sign validated" both promise more than the check delivers.

Why I think this is the load-bearing detail rather than cosmetic: the grant is file-read* + file-test-existence + file-map-executable (macos-seatbelt.ts, buildRuntimeRootsPolicy). The signature check is the only thing that makes it acceptable to hand a sandboxed child that grant for a directory derived from an environment variable. The remaining structural guard is a filename heuristic — basename(developerRoot) === 'CommandLineTools' (line 66) or …/Xcode.app/Contents/Developer (line 68) — and $HOME is the only location excluded (lines 57–58). DEVELOPER_DIR is inherited from the host's shell environment, so anything able to set it (shell rc, a plugin-supplied shellEnvironment, user typo) plus write access to a world-writable path can name a directory that satisfies every remaining check once ad-hoc signed.

Adding -R="anchor apple" to the codesign argv closes this at no measurable cost (verify is ~14 ms). I have no strong preference between codesign -R and spctl --assess; the important part is that the signer is pinned.

Related: the test at macos-command-paths.test.ts:150 is named "rejects a structurally plausible toolchain whose libxcrun is not Apple-signed", but the fixture writes the literal text not signed, so it only proves that a non-Mach-O file is rejected. Ad-hoc signing needs no certificate and no keychain, so it is CI-friendly — making the fixture an actually-ad-hoc-signed dylib would let the test name be true.

Nits

1. Discovery runs — and blocks the event loop — even when no sandbox will be applied. In builtin-tools.ts sandboxCommand, macosPaths is computed after the commandSandboxUnavailable early return. For an unrestricted/full-access profile under the default auto preference, shouldSandbox is false, so selectInitial returns sandboxType: 'none' and canEnforce then returns true (sandbox-manager.ts), meaning we do reach the resolver, spawn up to two subprocesses synchronously, and transform returns none and discards executableRoots entirely. Memoizing the canonical root for the process lifetime (the PR already accepts the post-validation TOCTOU race, so caching doesn't weaken that) or gating on profileRequiresSandbox would remove the waste. Measured ~14 ms for codesign plus spawn overhead, ×2 sequential spawns, uncached, per Bash invocation.

2. The timeout bounds aren't testable and aren't surfaced. XCODE_SELECT_TIMEOUT_MS / CODESIGN_TIMEOUT_MS and readSelectedDeveloperDirectory / validateAppleBinary are module-private; only selectDeveloperDir and validateAppleBinary are injectable through options. So the "bounded" half of the title has no coverage in the focused suite — an injectable spawner (or exported timeout) would fix that. Separately, a timeout and "no toolchain found" both silently return [] with no diagnostic, so a user who hits the 1 s limit sees a bare Git failure with nothing pointing at the cause.

3. The buildRuntimeRootsPolicy change reaches beyond the Apple toolchain. The diff also wraps the RUNTIME_READABLE_ROOT_* clauses in deniedRootRequirements. Those roots come from the filesystem worker's Mach-O dependency walk (filesystem-worker/macos-executable-dependencies.ts, filesystem-worker/launch-spec.ts), not from the toolchain. The tightening is right in spirit — explicit denies should win — and I could not find an existing test asserting the old deny-exempt behavior. But there's no new test for this half, and a profile that denies a directory holding the worker's dylibs now loses file-read* there and can fail to launch the worker. Worth a line in the Summary (it's a separate behavior change from the Git fix) and a focused case in macos-seatbelt.test.ts.

4. Smoke assertion compares canonical output to a non-canonical path. In the new "starts Apple Git…" smoke test, assert.deepEqual(runtimePaths.executableRoots, [join(selectedDeveloperDirectory, 'usr', 'lib')]), while the resolver returns realpathSync(...). It holds here (xcode-select -p → /Library/Developer/CommandLineTools, already canonical — I checked), but realpathSync would match the resolver's contract exactly. Also canRunAppleClt gates on the path ending in /CommandLineTools, so Xcode-only machines get no coverage of this PR's path at all.

5. Sandbox README conventions. packages/runtime/src/sandbox/README.md enumerates every module under "Ownership" and every test file under "Verification". The new macos-command-paths.ts and macos-command-paths.test.ts appear in neither, and the new Apple-toolchain allowance is a user-visible "Current behavior" change that deserves a bullet.

Verified OK

• Platform gating — macosPaths is only computed under platform === 'darwin'; the Linux and Windows path contexts are untouched, and the new module is not imported into platform-neutral code.
• macosRuntimeExecutableRoots preserved — the new roots are appended rather than replacing them, so the pre-existing -DEXECUTABLE_ROOT_n assertion in builtin-tools.test.ts still holds and an empty resolver result no longer drops the runtime roots.
• Read-only profiles get no grant (macos-command-paths.ts:85), and the test correctly asserts validateAppleBinary is not even invoked.
• Canonicalization — realpathSync before the layout checks makes the "alias replaced after resolution" test meaningful, and both / and $HOME are excluded up front.
• No env-precedence regression at the background_command call site — the Seatbelt backend returns command.env verbatim and options.shellEnvironment is still applied last by the caller, so threading it into sandboxCommand is additive. (I checked the 6-argument runSeatbeltCommand call in the smoke test against the extended helper signature; the argument order is correct.)
• ASF headers present on both new files, and every import is used.

Comment-only review, no approval or change request.

[Dante-dan]

Fixed the signer check in d7ceeb1: codesign --verify --strict -R=anchor apple now rejects a valid ad-hoc signature. The new macOS regression compiles a dylib, signs it ad hoc, first proves ordinary strict verification accepts it, and then checks that the resolver grants no root. It fails with the old verifier and passes with the Apple-anchor requirement.

I also gated discovery on actual sandbox selection, extended the deny-composition case to runtime-readable roots, documented the filesystem-worker dependency impact, and made the Apple Git smoke canonicalize its expected paths and accept both selected CLT and Xcode layouts. The sandbox README now lists the resolver/tests and the allowance contract. I kept validation adjacent to each sandbox launch instead of caching it for the process lifetime.

The one-second subprocess limits and fail-closed behavior are unchanged and now documented. I have left timeout-specific diagnostics and a new subprocess injection interface out of this correction; the focused suite does not claim timeout fault-injection coverage.

Validation: 101 focused tests passed, including the real ad-hoc regression; runtime build, lint, format, and both knip checks passed. Root build/typecheck still fail in @maka/ui on settledText, autoScroll, and trailingAction type mismatches. The Seatbelt suite still hits the host's sandbox_apply: Operation not permitted (exit 71), including its pre-existing positive cases, so end-to-end Xcode/CLT acceptance remains unproven here.

CI then caught the generated Windows skip inventory missing the new macOS-only suite (failed job). I regenerated it in de5757c; npm run windows:inventory now passes locally (including its three tests).

Merged current main in 5957562f6 and regenerated the Windows skip inventory from the combined test sources (99 declarations), resolving the merge conflict without dropping either branch's tests. After refreshing dependencies, the runtime build and 89 focused resolver/policy/Bash tests pass. Lint, format, both knip checks, and the Windows inventory check (including its three tests) pass. Root build/typecheck still fail in @maka/ui on component API mismatches, including settledText, SelectorProps, defaultSize, and trailingAction. The Seatbelt smoke remains 3 passed / 6 failed with sandbox_apply: Operation not permitted (exit 71), including pre-existing positive cases.

AI-generated with Codex.

Follow-up in 78f27855e: the two bounded discovery subprocesses now share an injectable command seam, and focused regressions assert that both xcode-select and codesign receive the one-second deadline and fail closed when either probe times out. The runtime build passes, along with the 29 focused resolver and Seatbelt policy tests; targeted Biome lint/format checks pass. I kept user-facing timeout diagnostics out of this change because surfacing them requires a separate command-error contract rather than changing the resolver's existing fail-closed result.

[hqhq1025]

Reviewed this exact head. The 9-file change bounds Apple toolchain discovery and pins the libxcrun.dylib signer with codesign -R=anchor apple (macos-command-paths.ts:57-99,120-140), gates resolution on an active Darwin sandbox (builtin-tools.ts:839-865), and preserves explicit-deny precedence for runtime/executable roots (macos-seatbelt.ts:492-511). I found no substantiated P0–P3 issue in the inspected paths. Node 24 npm ci, build:test, 90 focused tests, and current-head CI test/package/Windows checks pass. The PR currently conflicts with main in docs/windows-test-inventory.md, so it cannot merge as-is. I could not run real macOS codesign, Seatbelt, or Apple Git startup on this Linux host; please validate both CLT/Xcode configurations on a Mac after resolving the conflict. Directory replacement after path validation is a documented residual TOCTOU limitation.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Dante-dan]

I merged the current main into this PR and pushed 02489b5bd. The docs/windows-test-inventory.md conflict is resolved; GitHub now reports the branch mergeable. The regenerated inventory contains 106 Windows skip declarations, and npm run windows:inventory passes its three tests and inventory check. The runtime build, repository lint, and format check also pass on this head.

On this macOS host, the compiled resolver accepts the actual selected Command Line Tools path and the installed Xcode path, returning their narrow runtime roots; codesign --verify --strict -R=anchor apple succeeds for Xcode's libxcrun.dylib. The focused run passed 94 of 100 tests. All six failures are Seatbelt smoke cases, including pre-existing positive cases: this host rejects sandbox_apply with Operation not permitted (exit 71). I therefore cannot claim a successful sandboxed Apple Git startup or end-to-end CLT/Xcode validation here. The new-head CI checks are running.

AI-generated with Codex.

[Astro-Han]

Thanks for the update! Some checks are falling, could we take a quick look?

[Dante-dan]

Thanks for flagging the checks. I traced both failures on 02489b5bd and pushed 24fd9fc11 with current main merged.

The test job stopped at a generated docs/astryx-surface-file-inventory.md mismatch (PR job). The target branch at f362f1de7 failed that same check (main job); the subsequent 538c37cb6 main run passed it (main job). The merge brings that inventory update into this PR. On the new local head, the inventory check, build, typecheck, lint, format, both knip checks, Windows skip inventory, and 91 focused Apple resolver/policy/Bash tests pass. I am leaving the new-head CI result to the remote checks.

The Windows sandbox job has a separate failure: the broker launches a child, but Node cannot read the repository package.json (operation not permitted) while loading the worker; the client then reports SANDBOX_FILESYSTEM_OPERATION_FAILED (PR job). The scheduled main job at fa9be2fa5 fails in the same step with the same Node package-config error (main job). @liugddx, could you look at the Windows broker's AppContainer read grant or Node startup path? I have not verified a Windows repair on this macOS host.

[hqhq1025]

Reviewed exact head 24fd9fc1112e1b5caa67bdd0b7acce810bfcdffb. This PR still changes nine files: it resolves only canonical Apple CLT/Xcode library roots and verifies the libxcrun.dylib Apple signer with bounded probes (packages/runtime/src/sandbox/macos-command-paths.ts:58-99,120-140), adds those roots only for an active Darwin command sandbox (packages/runtime/src/builtin-tools.ts:834-867), and keeps explicit path denies effective for runtime/executable roots (packages/runtime/src/sandbox/macos-seatbelt.ts:491-513). The commits since the previous review sync current main and refresh the Windows test inventory; they do not change those Apple-path or Seatbelt implementations. I found no substantiated P0–P3 issue in the inspected PR paths.

Node 24 clean install, build:test, 90 focused tests, diff check, and a merge-tree against current main pass. Current-head test, package, and windows_recovery checks pass, and the PR is mergeable. windows_sandbox_w0_protocol is red: its worker cannot read the root package.json inside the Windows AppContainer; the same error also appears on main's run 36226849517, so I cannot attribute it to these macOS-only changes. The Windows gate still needs resolution before merge. I could not run macOS codesign, Seatbelt, or real CLT/Xcode Git startup on this Linux host; those platform paths still need Mac validation. Directory replacement after validation remains a documented TOCTOU limitation.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed at this head by three independent AI reviewers (different model families), with a focus on what the sandbox now allows. The mechanism is sound; all three found the same containment gap (inline). Because this is sandbox policy and the fix is one check, we suggest addressing it before merging.

What checks out: the resolver only runs for an active Darwin command sandbox and adds nothing for read-only profiles; DEVELOPER_DIR must be absolute, canonical, not /, not inside $HOME, and shaped like CommandLineTools or <…>/Contents/Developer; libxcrun.dylib must be a regular file inside the library root after canonicalisation and pass codesign --verify --strict -R=anchor apple (the ad-hoc-signed and non-Mach-O tests show this check is load-bearing); both discovery subprocesses are bounded to one second and fail closed; explicit path denies now also subtract from runtime/executable grants. Runtime tests pass (3553 in one reviewer's full run; 90/90 focused).

On reachability: the gap is not directly reachable by the model or plugins today. Sandboxed commands cannot change the Host environment, plugin shells cannot pass environment, and plugin env keys are restricted to MAKA_PLUGIN_*, so DEVELOPER_DIR comes only from the user's Host process. It becomes more serious if a new env source is ever added.

Other notes:

• Git still fails under read-only (Explore) profiles; the positive smoke uses an empty HOME, which hides the ~/.gitconfig failure reported in #5260.
• Every sandboxed command now runs two blocking discovery subprocesses (up to 1 s each), uncached, and a timeout produces no diagnostic.
• No test has yet shown Apple Git actually starting inside Seatbelt; the Git-config Bash test also passes on the old code.
• The remaining TOCTOU window between validation and child start is documented as a known limit.

windows_sandbox_w0_protocol is red on this head, as on main.

---

Automated review notice: This review was posted by an automated review agent operated by Astro-Han. It combines independent reviews from several AI models. It is not an independent human review and does not replace one.

[Astro-Han]

P3 (sandbox boundary; please fix before merge) — SharedFrameworks is checked with statSync (follows symlinks) and returned via realpathSync without the containment check applied to libxcrun.dylib. If Contents/SharedFrameworks is a symlink, its target outside the toolchain is returned and macos-seatbelt.ts:503-509 grants it file-read*, file-test-existence and file-map-executable; with the signer stubbed, a symlink to / yielded an executable root of /. The signature check covers only libxcrun.dylib, not this directory. Suggest requiring isPathWithin(realpathSync(sharedFrameworks), contentsRoot) (and the same for the library root), plus a symlink-escape regression test.

[Dante-dan]

Fixed in ab11ad1ba (now on PR head 9f8d3d71c). Both returned roots are canonicalized and checked against their containing developer directory or Xcode Contents directory. A SharedFrameworks symlink to / and a CLT usr/lib symlink outside the selected developer directory now return no grant. The two new regressions pass; the focused resolver/Seatbelt policy suite passed 31/31, and the full build, lint, format, and Windows inventory checks passed after merging current main. The existing canonical-directory replacement race remains the limitation documented in the PR.

[me2seeks]

Automated review notice: This comment was posted by an automated review agent operated by me2seeks make. It is not an independent human review and does not replace one.

Summary

Sandboxes macOS Apple Git by canonicalizing DEVELOPER_DIR/xcode-select, codesign-verifying libxcrun.dylib (Apple anchor), granting only toolchain library roots, making explicit denies carve into executable/runtime-readable roots, and dropping host-side Git config discovery. The issue (Seatbelt blocking libxcrun → Git can't start, #5260) is real, and deny-propagation plus read-only exclusion are correct hardening.

Findings

1. [P2] packages/runtime/src/builtin-tools.ts:838-846 + sandbox/macos-command-paths.ts:132-153 — discovery runs on the operation hot path: every sandboxed macOS Bash call performs up to two spawnSync subprocesses (xcode-select, codesign) with no caching, blocking the runtime event loop (tens-to-hundreds of ms typical, up to the 1s timeouts each). The repo's own invariant says spawnSync "must never run on the operation hot path" (default-sandbox-manager.ts:107, cached hot path at :150); Windows/Linux probes are composition-time and cached. Per-call revalidation doesn't even close the documented TOCTOU (policy construction and child spawn remain separate), so a (developerDir, signature)-keyed cache keeps the same security posture without per-command stalls.
2. [P3] macos-command-paths.ts:55-102 — only libxcrun.dylib is signature-checked; a copied (still validly Apple-signed) dylib in an attacker-writable fake CommandLineTools layout passes, and the whole usr/lib (+SharedFrameworks) tree then receives file-map-executable. Reachable only via session/host env control of DEVELOPER_DIR, but worth a documented assumption since workspace-adjacent env config would cross it.
3. [P2] windows_sandbox_w0_protocol fails on head (run 36263504448). The change is darwin-scoped but touches shared builtin-tools.ts; confirm unrelated/flake before merge.

Verdict

needs-changes — move toolchain verification off the per-command hot path (or cache it) and triage the red Windows sandbox lane.

[hqhq1025]

Reviewed exact head 9f8d3d71cc5d39e2713a2763e8a2c24d99b98031, including the new Apple toolchain containment change and its two regression tests. I found no substantiated P0–P3 issue in the PR delta. macos-command-paths.ts:77-98 now requires both the canonical library and SharedFrameworks directories to remain inside the selected developer/Xcode bundle; the new tests exercise escapes through each symlink. The previous Apple Git permission and explicit-deny paths remain unchanged by this follow-up.

Node 24 builds of core/storage/runtime and 30 focused macOS-path/Seatbelt tests pass. Hosted test, package, and windows_recovery pass; diff-check and a merge-tree against current main are clean. windows_sandbox_w0_protocol is still failing, so this is not a green-check or merge-readiness claim. I have not run a real macOS CLT/Xcode/Seatbelt startup or a Windows sandbox run locally.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Dante-dan]

Addressed the blocking probe concern in the review in 22af4d5c4. Toolchain selection and Apple-signature verification now use asynchronous execFile; both foreground and background Bash await the result before building the sandbox command. The probes keep their one-second deadlines and fail-closed behavior. This removes spawnSync from the operation path while retaining per-launch verification rather than reusing a potentially stale signature result. The command can still wait for those bounded probes, but they no longer block the Runtime Host event loop.

The sandbox README also states the trust assumption: DEVELOPER_DIR is a Host-process setting, and the selected toolchain tree must be trusted by that Host. The Apple signature check authenticates libxcrun.dylib, not every file in the returned library directories. The previously documented canonical-directory replacement race remains.

Validation: 48 resolver, Seatbelt-policy, and shell tests pass, including the real ad-hoc signer rejection; the builtin Bash tests also pass. Repository lint, format, and both Desktop/UI knip checks pass. Root build/typecheck fail in the DeepSeek web-search codec against the installed OpenResponses API, with subsequent typecheck errors from unbuilt downstream declarations; the changed runtime files compile to updated test output without reported errors. The existing Seatbelt smoke still has six positive/startup failures because this host rejects sandbox_apply with exit 71, including pre-existing cases, so real CLT/Xcode Git startup remains unverified. The Windows baseline episode already reported above is unchanged.

AI-generated with Codex.

[hqhq1025]

Reviewed 22af4d5c4ca9408108b7868e559bcff5360f56dd against current main c838e1fa779a48832f08134547576a0471033187. No substantiated P0–P3 finding in the code and tests checked.

This PR adds narrowly scoped Apple toolchain library grants for writable sandboxed macOS commands, without discovering Git configuration grants. The latest increment changes toolchain selection and signature verification to asynchronous execFile probes, each configured with a one-second timeout and a 64 KiB output limit (packages/runtime/src/sandbox/macos-command-paths.ts:120, :146). Failed probes return no additional roots. Both managed shell launch and executor Bash await the transformed command (packages/runtime/src/shell-tools.ts:293, packages/runtime/src/builtin-tools.ts:230, :690), so policy construction does not consume an unresolved verification result.

I independently checked canonical CLT/Xcode library containment and symlink rejection (packages/runtime/src/sandbox/macos-command-paths.ts:77), the read-only early return (:108), and explicit denied-root requirements on executable/runtime-readable grants (packages/runtime/src/sandbox/macos-seatbelt.ts:491). No storage migration or protocol epoch change is introduced. The diff and a synthetic merge with current main are clean.

The trust assumption is material: DEVELOPER_DIR comes from Host-side configuration, and verifying libxcrun.dylib does not authenticate every file in the granted library directories. The selected installation must be trusted. Canonicalization does not pin those directories against replacement before launch; the updated sandbox documentation states this residual limitation (packages/runtime/src/sandbox/README.md:59).

Validation on Linux with Node 24.18.1: clean npm ci, core/storage/mcp/runtime builds, command-path/Seatbelt/Bash tests 96/96, adjacent shell-tools tests 17/17, and git diff --check pass. The current-head hosted test, package, and windows_recovery checks pass. windows_sandbox_w0_protocol remains red: run 36775701766 shows the Windows filesystem worker cannot read the repository package configuration (ERR_INVALID_PACKAGE_CONFIG, operation not permitted). I did not reproduce this Windows failure or establish its root cause, and do not treat the passing checks as merge readiness while it remains unresolved.

I did not execute actual macOS Apple Git, native signature verification or Seatbelt enforcement, nor validate the filesystem replacement race. Those platform-specific behaviors remain outside the dynamically verified scope. Existing reviews are attached to older commits; this conclusion is limited to the head above.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Review of exact head 5b15444d. The previous review head was 22af4d5c. The only commit since then is a merge from main (5b15444d, main at 7c90bac2). The PR's own diff against its merge-base is byte-identical to the one at 22af4d5c. The only exception is docs/windows-test-inventory.md, which shifted counts by one (52->53 portable-candidate, 120->121 total) because of the new main baseline. None of the merged main commits touch builtin-tools.ts, shell-tools.ts, or sandbox/.

Earlier findings: none of them is open as a code defect at this head.

• Astro-Han's signer finding (codesign --verify accepted ad-hoc signatures) is fixed: -R=anchor apple is at macos-command-paths.ts:137.
• Astro-Han's SharedFrameworks and usr/lib symlink-escape finding is fixed with containment checks at :78, :80 and :97.
• me2seeks's P2 (blocking spawnSync on the hot path) is fixed: async execFile is at :146-159, and both Bash paths now await it (builtin-tools.ts:229-230,690, shell-tools.ts:293). Each command still runs the probes uncached, which the author chose on purpose.
• me2seeks's P2 on CI: windows_sandbox_w0_protocol is still red, but it fails with the same ERR_INVALID_PACKAGE_CONFIG ... operation not permitted as main's scheduled run 37106919146. It is pre-existing, not from this PR.
• The trust assumption and the TOCTOU limit are documented (sandbox/README.md:56-59).
• Still not delivered: any end-to-end proof that Apple Git starts under Seatbelt (likun666661 #1, Astro-Han). See P2 below.

New findings

• P2, unverified core flow; the grant set looks incomplete for Apple Git itself. See the inline comment on macos-command-paths.ts:89.
• P3: the timeout is bounded only if the child honours SIGTERM, and probes ignore command abort. Inline at :151.
• P3: DEVELOPER_DIR set to an Xcode.app bundle root fails closed. Inline at :91.
• P3, carried over: a probe timeout and "no toolchain" both return [] silently. A user who hits the 1s limit sees a bare Git failure with nothing in the logs.

Verified OK: Non-macOS paths are unaffected: macosPaths is undefined unless platform === 'darwin' (builtin-tools.ts:841-846), and Linux and Windows path contexts are unchanged. Read-only and unsandboxed profiles do no discovery (macos-command-paths.ts:108, builtin-tools.ts:842). The resolver cannot throw: every fs call is in try/catch and the runner always resolves. So the already-prepared onCompletion (builtin-tools.ts:837) is not orphaned by the new await. On timeout or a maxBuffer overrun, execFile kills the child and returns status: null, which fails closed. Explicit denies now also apply to runtime and executable roots (macos-seatbelt.ts:494-509).

Validation (Linux, Node 22.23.2): clean npm ci, then core/storage/mcp/runtime builds pass. The resolver, Seatbelt policy, Seatbelt smoke and shell-tools tests pass 47/47; the smoke and macOS-only cases are skipped because this is not Darwin. builtin-tools.test.js has 63/66 passing; the 3 failures are only RipgrepUnavailableError (no rg on this host) and are unrelated. git diff --check is clean. Hosted test, package and windows_recovery pass. The PR is mergeable, but merge is BLOCKED because review is required. CI has no macOS runner, so none of the Darwin-only tests run in CI. I did not run real codesign, Seatbelt, or CLT/Xcode Git.

[Astro-Han]

P2 (medium confidence, unverified on macOS). The only new grant is file-read*/file-map-executable on <developer>/usr/lib (plus SharedFrameworks for Xcode). That is enough for the /usr/bin/git shim to load libxcrun.dylib. But xcrun then execs <developer>/usr/bin/git. Under workspace-write, Seatbelt reads are limited to readable roots (macos-seatbelt.ts:463-480), plus system defaults (/usr, /System, /bin, /etc, ...; :41-47, :51-61). Nothing covers <developer>/usr/bin, usr/libexec/git-core (helpers), or usr/share/git-core. Apple Git's system config (.../usr/share/git-core/gitconfig, which sets credential.helper=osxkeychain) and the git init templates live there. Git treats an unreadable config file as fatal (unknown error occurred while reading the configuration files), so git status could still fail one step after this fix. CI has no macOS runner, and every reported local smoke run hit sandbox_apply exit 71. So starts Apple Git with the selected Apple toolchain (macos-seatbelt-smoke.test.ts:160) has never passed anywhere. Please run that smoke on a real Mac, not inside another sandbox, for both CLT and Xcode. If it fails, add read-only (not file-map-executable) roots for usr/bin, usr/libexec/git-core and usr/share/git-core. If it passes, please post the output so the PR's core claim is backed by evidence.

[Dante-dan]

The core Seatbelt acceptance claim is still unverified. In 5e9975234 the existing smoke test now honors DEVELOPER_DIR, so it can select CLT and Xcode separately. I reran node --test --test-name-pattern='starts Apple Git' packages/runtime/dist/__tests__/macos-seatbelt-smoke.test.js with /Library/Developer/CommandLineTools and /Applications/Xcode.app/Contents/Developer. Both reached the sandboxed Git invocation and failed with sandbox-exec: sandbox_apply: Operation not permitted (exit 71). That is the outer sandbox rejecting Seatbelt application; it supplies no result about Git's binary/helpers/system config inside this policy.

I have not added the proposed read-only roots or marked this finding resolved. Both toolchains still need this smoke run on a Mac process permitted to apply Seatbelt; its result should determine the smallest additional read grants. The timeout/cancellation and bundle-selection fixes are delivered separately above.

Automated follow-up from Codex.

[Astro-Han]

P3. execFile's timeout sends killSignal (default SIGTERM), and the callback only fires on close. So the 1s bound holds only if the child exits on SIGTERM. Pass killSignal: 'SIGKILL' to make the bound unconditional. Two smaller points: the probes don't receive the tool's ctx.abortSignal, so cancelling a command still waits up to about 2s. The runner's stdio option is accepted by the type (:41) but ignored here, so codesign's stderr is buffered into maxBuffer rather than discarded. That is harmless, but the contract is misleading.

[Dante-dan]

Fixed in 5e9975234. Discovery now uses spawn with killSignal: 'SIGKILL' for both one-second timeouts and receives the command's ctx.abortSignal. The declared stdio is now actually applied: codesign output and both probes' stderr are discarded, while xcode-select stdout remains capped at 64 KiB. The existing probe test now checks SIGKILL and abort-signal propagation. The 65 focused resolver/Seatbelt/manager tests, two macOS Bash wiring tests, root build/typecheck/lint/format and desktop/UI knip passed.

Automated follow-up from Codex.

[Astro-Han]

P3. A DEVELOPER_DIR that points at the bundle root (/Applications/Xcode.app, which xcode-select -s also accepts) fails at :77 because usr/lib is missing, so no roots are granted. It fails closed, so this is not a regression, but Git stays broken for that setup and nothing tells the user why. Consider normalising *.app to *.app/Contents/Developer before the layout checks, or at least logging the rejected path.

[Dante-dan]

Fixed in 5e9975234. A canonical *.app selection is normalized to Contents/Developer before the existing layout and Apple-signature checks. The normalized directory must remain inside the canonical bundle, so a symlink escape still fails closed. The existing Xcode layout test now exercises both the developer-directory and bundle-root forms and returns the same bounded library/SharedFrameworks roots.

Automated follow-up from Codex.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 5e997523. The previous reviewed head was 5b15444d. The merge-base is unchanged (7c90bac2), and the increment is one commit, 5e997523 ("bound Apple toolchain probes and accept Xcode bundles"): +72/-27 across macos-command-paths.ts, builtin-tools.ts and two test files.

Status of the previous findings

• P2, Apple Git under Seatbelt never shown to start: still open. No real-Mac evidence has been provided, and the grants are unchanged. The only grants are still <developer>/usr/lib, plus SharedFrameworks for Xcode (macos-command-paths.ts:100, :109). There is still nothing for <developer>/usr/bin, usr/libexec/git-core or usr/share/git-core. The author says so directly in the open thread. The smoke test now honours DEVELOPER_DIR (macos-seatbelt-smoke.test.ts:42-43), which is useful. But both the CLT and the Xcode reruns again failed at sandbox-exec: sandbox_apply: Operation not permitted (exit 71), because they ran inside another sandbox. So starts Apple Git with the selected Apple toolchain (macos-seatbelt-smoke.test.ts:161) has still never passed anywhere, and the PR's core claim is still unverified. This is the remaining blocker: a run on a Mac outside any outer sandbox, for both toolchains, then either the passing output or the added read-only roots.
• P3, SIGTERM-only timeout, abort ignored, stdio ignored: fixed. The runner now uses spawn with killSignal: 'SIGKILL' (:137, :152). It passes the declared stdio through, and it receives ctx.abortSignal (builtin-tools.ts:844 -> macos-command-paths.ts:64-67). I ran the new runner body under Node 22.23.2. A child that traps SIGTERM was killed at about 1.0s and returned status: null. An abort at 200ms returned status: null at about 200ms. A stdout overflow (more than 64 KiB) was killed and returned null. A normal echo returned status 0 with its stdout. Every failure path resolves null, so they all fail closed.
• P3, DEVELOPER_DIR pointing at an Xcode.app bundle root: fixed. A *.app realpath is normalised to Contents/Developer and must stay inside the canonical bundle (:76-80). The existing layout, signature and Contents/Developer checks still run after it. The test now covers both forms.
• P3, carried over: a probe timeout and "no toolchain" both still return [] silently. There is still no log line when discovery fails closed, so a user whose probe hit the 1s limit or was rejected sees only a bare Git failure. This was not addressed in this round and is not blocking.

New findings in the increment: none. Minor note: the new abort test only checks that the signal is forwarded to an injected runner, not that the real runner aborts. My direct check above covers that.

Validation (Linux, Node 22.23.2): at 5e997523, clean npm ci, then core/storage/mcp/runtime builds pass. macos-command-paths, macos-seatbelt, macos-seatbelt-smoke and shell-tools tests pass 47/47; the Darwin-only cases are no-ops on this host. CI: test, package and windows_recovery pass. windows_sandbox_w0_protocol fails with the same ERR_INVALID_PACKAGE_CONFIG ... operation not permitted that main already has, so it is pre-existing and not caused by this PR. The PR is mergeable; merge is BLOCKED because review is required. CI still has no macOS runner.

[Dante-dan]

Addressed the carried-over silent-discovery P3 in 57229ba1a. Failed discovery now emits a reason-specific [sandbox:macos] warning: an interrupted/timed-out xcode-select probe is distinguished from no selected toolchain, and invalid layout, containment and Apple-signature failures are explained. The warning contains no selected path; cancellation remains silent. Discovery still fails closed, with unchanged grants. The existing timeout test now checks the diagnostic distinction and cancellation behavior.

Root build, lint, format, typecheck, desktop/UI knip and 44 focused resolver/Seatbelt/manager tests passed. The P2 real CLT/Xcode Seatbelt acceptance remains open, as documented in the existing thread.

Automated follow-up from Codex.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 57229ba1. The previous reviewed head was 5e997523. The merge-base is unchanged (7c90bac2), so the increment is exactly one commit, 57229ba1 ("explain failed Apple toolchain discovery"): +76/-12 across macos-command-paths.ts and its test.

Status of the previous findings

• P2, Apple Git under Seatbelt never shown to start: still open, and still the blocker. The grants are unchanged: <developer>/usr/lib (macos-command-paths.ts:108, returned at :124), plus SharedFrameworks for Xcode (:131, :135). There is still nothing for <developer>/usr/bin, usr/libexec/git-core or usr/share/git-core. The author's latest comment (2026-10-04 01:37) says so directly: "Discovery still fails closed, with unchanged grants … The P2 real CLT/Xcode Seatbelt acceptance remains open." No new smoke output has been posted. The only reruns are still the two sandbox_apply: Operation not permitted (exit 71) runs from inside an outer sandbox, so starts Apple Git with the selected Apple toolchain (macos-seatbelt-smoke.test.ts:161) has still never passed anywhere. Needed: a smoke run on a Mac outside any outer sandbox, for both CLT and Xcode, and then either the passing output or the read-only roots that run shows are missing.
• P3, carried over: silent [] when discovery fails: fixed. Every fail-closed return after the abort check now goes through fail(reason) (:66-73), which defaults to console.warn('[sandbox:macos] Apple toolchain discovery failed closed: <reason>. No additional executable roots granted.'). A probe timeout ("xcode-select probe interrupted or timed out") is now told apart from a non-zero exit ("xcode-select probe failed") (:167-175). Each validation step has its own reason. No reason string includes a path, and cancellation stays silent (:65, :67). The new test covers the timeout, no-directory and pre-aborted cases (macos-command-paths.test.ts:234-264).

New findings in the increment

• P3: the warning is not deduplicated. resolveMacosCommandPaths runs on every sandboxed shell command on darwin (builtin-tools.ts:844), and nothing caches the result. On a Mac without Command Line Tools, every sandboxed command, including ls, will log "xcode-select probe failed". Suggest warning once per reason per process, or caching the negative result briefly.
• P3, test gap: the signature-failure reason (:121) and the layout and containment reasons are not covered by tests.

CI and merge state: test, package and windows_recovery pass. windows_sandbox_w0_protocol fails with the same ERR_INVALID_PACKAGE_CONFIG that main already has, so it is pre-existing and not caused by this PR. The PR is MERGEABLE; merge is BLOCKED because review is required. CI still has no macOS runner.

Verdict: this round's change is good. The PR stays blocked only on the P2 real-Mac evidence or grant widening.

[Dante-dan]

Addressed the two P3 findings in review 5404054274 at 213c4ad7c. Default discovery warnings are now deduplicated by reason with a bounded per-process set. Discovery and signature verification still run for every command; no successful or negative permission result is cached, and injected diagnostic sinks retain per-attempt reporting. Added regression coverage for repeated warnings, different reasons, signature failure, unsupported layout and an escaping libxcrun symlink.

Root build, typecheck, lint, format, desktop/UI knip and all 33 focused resolver/Seatbelt tests passed. The P2 remains open: real Apple Git startup with both CLT and Xcode outside an outer sandbox has not been verified, so executable/read grants are unchanged.

Automated follow-up from Codex.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental re-review of exact head 213c4ad7. The previous reviewed head was 57229ba1. The merge-base is unchanged (7c90bac2), so the increment is exactly one commit, 213c4ad7 ("deduplicate Apple toolchain diagnostics"): +104/-1 across macos-command-paths.ts and its test.

Status of the previous findings

• P2, Apple Git under Seatbelt never shown to start: still open, and still the only blocker. The grants are unchanged: CLT gets <developer>/usr/lib only (macos-command-paths.ts:138), and Xcode gets usr/lib plus SharedFrameworks (:149). There is still nothing for <developer>/usr/bin, usr/libexec/git-core or usr/share/git-core. The author's comment (2026-10-04 04:35) confirms this: "real Apple Git startup with both CLT and Xcode outside an outer sandbox has not been verified, so executable/read grants are unchanged." No smoke output has been posted. starts Apple Git with the selected Apple toolchain (macos-seatbelt-smoke.test.ts:161) has still never passed anywhere. What would close this: a run on a Mac outside any outer sandbox, for both CLT and Xcode, and then either the passing output or the minimal read-only roots that run shows are missing. Those roots should stay inside the already Apple-signed, containment-checked developer root.
• P3, warning not deduplicated: fixed. The default sink goes through reportDiscoveryFailure (:30-42). It warns at most once per reason per process, with a bounded set of 16 entries. Reasons are a small fixed set with no paths, so the bound never churns. Discovery and codesign verification still run for every command, and no grant is cached, so a changed toolchain is never served from a stale result. Injected onDiscoveryFailure sinks still receive every attempt.
• P3, test gap for the signature, layout and containment reasons: fixed. A new test covers the signature failure, an unsupported layout and an escaping libxcrun symlink. A second test covers repeated warnings, different reasons and per-attempt injected sinks.

New findings: none at P0-P3. One optional nit: the dedup set is module-global and has no reset hook for tests, so the new dedup test depends on no earlier test in the file having emitted "xcode-select probe failed" through the default sink. It passes today. A small resetMacosDiscoveryDiagnosticsForTest() export, or a unique reason in that test, would remove the ordering dependency.

CI and merge state: test, package and windows_recovery pass. windows_sandbox_w0_protocol fails, as it does on main, so the failure is pre-existing and not caused by this PR. The PR is MERGEABLE; merge is BLOCKED because review is required. CI still has no macOS runner. I reviewed the code only this round; the change is diagnostics-only.

Verdict: this round's change is good and both P3s are closed. The PR stays blocked only on the P2: real-Mac CLT and Xcode evidence, or a justified, contained widening of the grants.