sandbox: one PathMapping core, two facades (fixes #83) - #88
Merged
Conversation
The virtual<->host translation that lived only inside MountedFileSystem moves down into ShellKit as PathMapping; SwiftBash becomes a consumer. Every cooperative caller now resolves and is confined through the same table: - Facade A (bash builtins, FileSystem protocol): MountedFileSystem wraps a PathMapping; its Mount type is now a typealias of PathMapping.Mount and the lexical mount routing is the shared implementation. Shell.normalizePath moved down to ShellKit.Shell. - Facade B (SwiftPorts CLIs, JS runtime, SwiftScript): ExecCommand installs Sandbox.confined(to:home:temporaryDirectory:) over the SAME mapping, so ShellKit's Shell.resolve translates virtual spellings to the mapped host dirs and authorize gates exactly that host space. Workspace access works for non-builtin callers; with --sandbox dir --workspace /batch, fd//rg/jq/git/JS no longer resolve to a literal nonexistent /batch. /tmp follow-ups from #82's interim contract: - $TMPDIR now carries the virtual /tmp spelling; Facade B callers translate it back to the per-instance dir through resolve(). - The identity mount (tempHost at its own host path) is retired; the mount table is just workspace + /tmp, and the Linux nesting subtlety is gone. - Sandbox.bashWorkspace - the host-rooted gate with the temp carve-out that only matched by string coincidence - is deleted; the general containment check covers FileManager-backed callers uniformly, symlink-escape rejection included. JS runtime (SwiftJSCore): fs.*/require/chdir resolve the script's virtual spelling to the mapped host path for BOTH the gate and the I/O (translating for only one of them would authorize one file and touch another); process.cwd(), os.tmpdir(), os.homedir(), and __filename stay in (or fold back to) the virtual spelling. The exception code-frame reader now translates AND gates its source read - sourceURL is script-influenced, so the ungated read could echo host file contents into error frames. Tests: BashWorkspaceSandboxTests (pinning the retired gate) is replaced by ConfinedSandboxTests, which pins the two-doors contract: both facades land on the same files for /tmp and the workspace, mktemp answers round-trip through resolve, $TMPDIR translates, display paths fold back to virtual, and a planted symlink escape is rejected identically at both doors. SandboxGatePathMappingTests covers the JS bridge; ExecCommandFileSystemTests pins the identity-mount retirement. Requires Cocoanetics/ShellKit's PathMapping core (merge that first; this repo pins ShellKit branch:main).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28d5bf3581
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Companion to ShellKit's resolve sentinel (Codex P1 on ShellKit#17): a script holding the host path of a mounted dir gets ENOENT from the bash filesystem AND a voided, gate-denied resolution from Facade B.
Same Linux nuance as the ShellKit-side fix: the platform temp root IS /tmp there, so the fixture's host dirs prefix-match the virtual /tmp mount and translate instead of voiding. Probe through a local /work + /scratch mapping so the no-mount-matches contract is what runs on every platform.
Codex P2 on #88: loadFileModule probed `fileExists` on the translated host path before authorizing it, so a workspace symlink escaping the sandbox got stat'd through on the host before the gate ran. A script could `require('/batch/link')` and read the error shape to tell an existing outside target (EACCES after the probe) from a missing one (module-not-found) — an existence oracle past the boundary. Resolution now walks Node's candidate order (bare, then .js/.mjs/ .cjs/.json) authorizing each host translation BEFORE any fileExists touch, and folds a denied candidate into the same not-found path as a missing one. So a symlink escape — whether it points at an existing file or nothing — is indistinguishable from a plain missing module: uniform MODULE_NOT_FOUND, no stat before the gate, no EACCES that would confirm the escape landed on something real. Cache hits still short-circuit before any gate/disk work. Regression test plants both escape flavors and asserts identical MODULE_NOT_FOUND.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Implements #83 on top of ShellKit's new
PathMappingcore (Cocoanetics/ShellKit#17 — merge that first, this repo pins ShellKitbranch: main):MountedFileSystemnow wraps aPathMapping; itsMountis a typealias ofPathMapping.Mountand the lexical mount routing is the shared implementation.Shell.normalizePathmoved down toShellKit.Shell(call sites unchanged — inherited static).ExecCommandinstallsSandbox.confined(to:home:temporaryDirectory:)over the same mapping, soShell.resolvetranslates virtual spellings to the mapped host dirs andauthorizegates exactly that host space. With--sandbox <dir> --workspace /batch, fd/rg/jq/git/JS stop resolving a literal nonexistent/batch— workspace access works for every non-builtin caller./tmpfollow-ups from #82's interim contract$TMPDIRnow carries the virtual/tmpspelling; Facade B callers translate it back to the per-instance dir throughresolve(). The per-instance host path no longer leaks into the script's env.tempHostat its own host path) is retired — the mount table is just workspace +/tmp; the Linux nesting subtlety is gone.Sandbox.bashWorkspace(host-rooted gate + temp carve-out that matched only by string coincidence) is deleted; the general containment check covers FileManager-backed callers uniformly, symlink-escape rejection included. Embedders (iBash) migrate toSandbox.confined(to: fs.mapping, home:, temporaryDirectory:)— a compile-time break, intentionally loud since the gate semantics changed.JS runtime (SwiftJSCore)
fs.*/require/process.chdirresolve the script's virtual spelling to the mapped host path for both the gate and the I/O (translating for only one of them would authorize one file and touch another — under a sandbox, a literal/tmp/xwould otherwise reach the host's shared temp dir).process.cwd(),os.tmpdir(),os.homedir(), and__filenamestay in (or fold back to) the virtual spelling. The exception code-frame reader now translates and gates its source read —sourceURLis script-influenced, so the previously ungated read could echo host file contents into error frames.Tests
ConfinedSandboxTests(replacesBashWorkspaceSandboxTests, which pinned the retired gate): both facades land on the same files for/tmp+ workspace,mktempanswers round-trip throughresolve,$TMPDIRtranslates, display paths fold back, and a planted symlink escape is rejected identically at both doors.SandboxGatePathMappingTests: the JS bridge under a mapped sandbox (writes land in the instance temp dir, not host/tmp; virtual cwd preserved acrosschdir;os.*answers virtual;requireloads through the mapping with virtual__filename).ExecCommandFileSystemTestspins the identity-mount retirement.Companions / follow-up
authorizePathto return the translated path, aBridgeGeneratorToolchange so generated bridges consume it, and a bridge regeneration (which currently drags ~2k lines of symbol-graph drift on a newer SDK — needs a pinned regen environment). Interim behavior is fail-closed: virtual spellings are denied, no escape. Half-adopting (translate-for-check-only) would be an escape, so it ships whole or not at all.Fixes #83
🤖 Generated with Claude Code