sandbox: isolate /tmp per instance (fixes #82) - #87
Merged
Conversation
Virtual /tmp in --sandbox mode previously mounted the bare NSTemporaryDirectory() root, so every SwiftBash instance — and every other process using the platform temp dir — shared one namespace: cross-instance reads, predictable-name collisions, and a TOCTOU surface, as reported in #82. Each sandbox now gets its own swiftbash-<UUID> dir under the platform temp root: - ExecCommand.makeSandboxFileSystem creates the dir, mounts it at virtual /tmp, and identity-mounts it at its own host path so $TMPDIR-spelled and /tmp-spelled paths reach the same files. On Linux the identity mount nests inside the /tmp mount; longest-virtual-prefix routing keeps it from double-nesting. - $TMPDIR carries the instance dir's host path (not virtual /tmp yet: FileManager-backed callers have no virtual->host translation until the #83 mapping core lands in ShellKit). - The CLI removes the dir however the run ends. - Sandbox.bashWorkspace derives the URL gate's temp carve-out from the passed temporaryDirectory (normalized + symlink-resolved spellings) instead of a process-wide static list. The hardcoded /tmp, /private/tmp, and bare-root prefixes are gone, so a sibling instance's dir or the shared root no longer authorizes. Interim tradeoff, documented in Docs/Sandboxing.md: ports/JS handed a literal /tmp/... path are denied until #83 — that spelling only ever worked on Linux by coincidence (on macOS it silently diverged to /private/tmp). $TMPDIR-spelled access keeps working on every platform, and bash builtins are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb8e97b341
ℹ️ 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".
The carve-out derived its accepted prefixes from standardizingPath + resolvingSymlinksInPath only. standardizingPath can rewrite the spelling (corelibs-foundation follows symlinks; Darwin strips /private), and the gate checks the unresolved path first — so on a host where the temp dir sits behind a symlink, the one spelling callers actually use ($TMPDIR as the embedder set it) could vanish from the list and deny legitimate access. Keep tempDir.path verbatim alongside the other two; all three name the same directory. Review feedback on #87. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
In
--sandboxmode, virtual/tmpwas mounted onto the bareNSTemporaryDirectory()root — one namespace shared by every SwiftBash instance in the process and by everything else using the platform temp dir (#82). Each sandbox now gets its ownswiftbash-<UUID>dir under the platform temp root, removed when the run ends.This is the minimal fix ahead of the #83 refactoring (mapping + confinement core in ShellKit); nothing here gets thrown away by it — #83 retires the identity mount and what's left of the carve-out, then flips
$TMPDIRto the virtual/tmpspelling.What changed
ExecCommand.makeSandboxFileSystemcreates the per-instance dir, mounts it at virtual/tmp, and identity-mounts it at its own host path so$TMPDIR/fooand/tmp/fooreach the same files. On Linux the identity mount nests inside the/tmpmount; longest-virtual-prefix routing keeps it from double-nesting (regression-tested).$TMPDIRcarries the instance dir's host path — not virtual/tmpyet, because FileManager-backed callers (SwiftPorts CLIs, SwiftScript, JS) have no virtual→host translation until Sandbox path enforcement: move the mapping into ShellKit and offer two facades over one core #83. The CLIdefer-removes the dir however the script ends.Sandbox.bashWorkspacederives the URL gate's temp carve-out from the passedtemporaryDirectory(normalized + symlink-resolved spellings) instead of a process-widestatic let. The hardcoded/tmp,/private/tmp, and bare-temp-root prefixes are gone — a sibling instance's dir or the shared root no longer authorizes, so isolation is enforced for bridge callers too, not just bash builtins.SwiftJSCore'sos.tmpdir()picks up the per-instance dir automatically viasandbox.temporaryDirectory./tmpnow expects denial; new sibling-instance and shared-root denial coverage); newtmpIsIsolatedPerInstance(two sandboxes write the same hardcoded/tmp/secret.txtwithout cross-talk) andtmpSpellingsReachTheSameFiles(the Linux nesting guard).Sandboxing.mdmount layout, embedder sample, and the threat-model paragraph that previously documented the shared/tmpas a tradeoff;README.md;AGENTS.md.Interim tradeoff (until #83)
Ports/JS handed a literal
/tmp/...path are denied. That spelling only ever worked on Linux by coincidence (NSTemporaryDirectory()is/tmpthere) and silently diverged to/private/tmpon macOS — failing loudly beats reading the shared root.$TMPDIR-spelled access — the only spelling that worked on every platform — keeps working everywhere, and bash builtins are unaffected (/tmptranslates through the mount table as before). Documented inDocs/Sandboxing.md. #83'sresolve()translation restores the literal spelling for ports properly.Verification
main; pass in isolation). SwiftLint strict: clean.$TMPDIRpoints at the per-instance dir;/tmpwrites land there and not in the shared root;mountoutput leaks no host path;mktempworks;jqreads bash-written files via$TMPDIRspelling;jqon a literal/tmp/...path is denied; zeroswiftbash-*dirs remain after runs exit.Fixes #82.
🤖 Generated with Claude Code