diff --git a/AGENTS.md b/AGENTS.md index 6a550d81..309483f6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,9 +96,9 @@ splitting big types — `Shell` lives in `API/Shell.swift` plus ~15 Substring assertions hide whitespace bugs and ordering issues that exact-string assertions catch. - **Sandbox tests use `MountedFileSystem`** with a real temp dir - mounted at `/batch` and host `/tmp` mounted at `/tmp` (the same - shape `swift-bash exec --sandbox` installs). Clean up the temp - dir in `defer`. + mounted at `/batch` and a per-instance temp dir mounted at `/tmp` + (the same shape `swift-bash exec --sandbox` installs). Clean up + the temp dirs in `defer`. - **Never re-test the parser from interpreter tests.** Parser behaviour belongs in `BashSyntax` tests; interpreter tests exercise execution against pre-parsed shapes. diff --git a/Docs/Sandboxing.md b/Docs/Sandboxing.md index 36b41c8f..14ef60f1 100644 --- a/Docs/Sandboxing.md +++ b/Docs/Sandboxing.md @@ -110,12 +110,17 @@ axes at once: - `fileSystem = MountedFileSystem` with mounts on real disk: - virtual `/batch` (or `--workspace`) → host `PATH` (read-write) - - virtual `/tmp` → host `NSTemporaryDirectory()` (read-write) - - the host's real temp dir → itself, when it isn't `/tmp` (so - `$TMPDIR/foo` and `/tmp/foo` reach the same files on macOS, - iOS, Windows) -- `TMPDIR = NSTemporaryDirectory()` in the script's environment -- `sandbox = Sandbox.bashWorkspace(workspace: workspace)` (URL gate for SwiftPorts CLIs) + - virtual `/tmp` → a per-instance `swiftbash-` dir created + under host `NSTemporaryDirectory()` (read-write), removed when + the script ends — concurrent instances and other host processes + can't see each other's scratch (#82) + - the per-instance dir → itself (so `$TMPDIR/foo` and `/tmp/foo` + reach the same files on every platform) +- `TMPDIR = ` (host spelling) in the script's + environment +- `sandbox = Sandbox.bashWorkspace(workspace:temporaryDirectory:)` + (URL gate for SwiftPorts CLIs, accepting the workspace and the + per-instance temp dir) - `networkConfig = nil` (deny-all) - `hostInfo = .synthetic` - Process table is always virtual (no flag needed) @@ -126,27 +131,46 @@ $ swift-bash exec --sandbox /tmp/work script.sh The script sees `/batch` as its workspace, can `cd /batch && ls`, write files there (and they persist at `/tmp/work` on the host), and -can use `/tmp` as scratch — writes through either virtual path land -on real disk so FileManager-backed callers (SwiftPorts CLIs, -SwiftScript bridges) see them immediately. The script can't reach -`/Users/`, `~/Documents`, `/etc/passwd`, or anything else on the host. -It can't make network requests. `whoami` says "user". `hostname` says -"sandbox". Issues #48 / #49. +can use `/tmp` as scratch — writes land on real disk in the +instance's own temp dir so FileManager-backed callers (SwiftPorts +CLIs, SwiftScript bridges) see them immediately via `$TMPDIR`-spelled +paths. The script can't reach `/Users/`, `~/Documents`, +`/etc/passwd`, or anything else on the host. It can't make network +requests. `whoami` says "user". `hostname` says "sandbox". +Issues #48 / #49 / #82. + +Until the path-mapping core lands in ShellKit (#83), bash builtins +and FileManager-backed callers agree on temp paths through the +**host** spelling only: bash scripts may say `/tmp/foo` or +`$TMPDIR/foo` interchangeably, but a SwiftPorts CLI / SwiftScript / +JS call must be handed `$TMPDIR/foo` — a literal `/tmp/foo` argument +to those is denied, because they would do real I/O on the host's +shared `/tmp` instead of the instance's temp dir. For embedders not using the CLI, mirror the same setup: ```swift let workspace = NSHomeDirectory() + "/Documents/scratch" +// Per-instance scratch dir backing virtual /tmp — create it up +// front, remove it when the session ends (#82). +let tempHost = URL(fileURLWithPath: NSTemporaryDirectory(), + isDirectory: true) + .appendingPathComponent("myapp-\(UUID().uuidString)", + isDirectory: true) +try FileManager.default.createDirectory( + at: tempHost, withIntermediateDirectories: true) let shell = Shell( environment: { var env = Environment.empty() env.workingDirectory = "/batch" + env["TMPDIR"] = tempHost.path return env }(), fileSystem: MountedFileSystem( mounts: [ .init(virtual: "/batch", host: workspace), - .init(virtual: "/tmp", host: NSTemporaryDirectory()) + .init(virtual: "/tmp", host: tempHost.path), + .init(virtual: tempHost.path, host: tempHost.path) ], backing: RealFileSystem()) ) @@ -173,8 +197,9 @@ state changes, not memory-level exploitation of the runtime itself. **Inside the mounts**, writes persist to real disk (the CLI's choice for `--sandbox`). The workspace is wherever the user pointed -`--sandbox`; `/tmp` resolves to `NSTemporaryDirectory()` on the host, -shared with other processes per the platform's convention. +`--sandbox`; `/tmp` resolves to a per-instance dir under +`NSTemporaryDirectory()` that no other sandbox instance or host +process shares, and that is removed when the script ends (#82). **Out of scope:** - Memory-corruption attacks against Swift runtime / Foundation. diff --git a/README.md b/README.md index 6e5a5946..c02f9913 100644 --- a/README.md +++ b/README.md @@ -132,8 +132,9 @@ differently from a native shell. None of these are bugs: - **The filesystem is a chroot-style mount table.** `/` is the read-only sandbox root; the workspace (`/batch` under `--sandbox`, `/home` in document apps) is writable; `/tmp` is writable scratch on - the host temp dir; everything else returns *No such file or - directory*. `mount` prints the table. + a per-instance dir under the host temp dir (removed when the script + ends); everything else returns *No such file or directory*. `mount` + prints the table. - **Identity is synthetic by default** — `whoami` → `user`, `id` → `uid=1000(user) gid=1000(users)`, and `uname` a Darwin-flavoured kernel string over a generic Unix layout. `stat` / `ls -l` report diff --git a/Sources/BashCommandKit/Commands/MountCommand.swift b/Sources/BashCommandKit/Commands/MountCommand.swift index 99259a56..a14156c8 100644 --- a/Sources/BashCommandKit/Commands/MountCommand.swift +++ b/Sources/BashCommandKit/Commands/MountCommand.swift @@ -40,14 +40,12 @@ public struct MountCommand: ParsableBashCommand { for mount in table { // Resolve the mountpoint to its virtual spelling before // printing. Some mounts exist only as a real-path alias: the - // CLI mounts `$TMPDIR`'s host path (e.g. `/var/folders/…/T`) - // next to `/tmp` so `$TMPDIR` resolves, and printing that - // virtual verbatim would leak a host path. When another mount - // exposes this one's `virtual` (itself a host path) under a - // cleaner name, print that name; the de-dup below then folds - // the alias into `/tmp`. A genuine `/tmp → /tmp` mount (Linux, - // where `NSTemporaryDirectory()` normalises to `/tmp`) has no - // such alias and stays visible. + // CLI mounts `$TMPDIR`'s host path (the per-instance + // `…/swiftbash-` dir) next to `/tmp` so `$TMPDIR` + // resolves, and printing that virtual verbatim would leak a + // host path. When another mount exposes this one's `virtual` + // (itself a host path) under a cleaner name, print that + // name; the de-dup below then folds the alias into `/tmp`. let mountPoint = table.first { $0.host == mount.virtual && $0.virtual != mount.virtual }?.virtual ?? mount.virtual diff --git a/Sources/BashInterpreter/API/Sandbox+BashWorkspace.swift b/Sources/BashInterpreter/API/Sandbox+BashWorkspace.swift index 986d2765..2cd13373 100644 --- a/Sources/BashInterpreter/API/Sandbox+BashWorkspace.swift +++ b/Sources/BashInterpreter/API/Sandbox+BashWorkspace.swift @@ -4,44 +4,50 @@ import ShellKit extension ShellKit.Sandbox { /// Build the URL gate the SwiftBash sandbox CLI pairs with the - /// real-disk ``MountedFileSystem`` mounting `workspace` and the - /// platform's temp dir. + /// real-disk ``MountedFileSystem`` mounting `workspace` and a + /// per-instance temp dir. /// - /// The gate accepts two virtual roots: the `workspace` mount and - /// the host's real temp dir (``Foundation.NSTemporaryDirectory()``), - /// addressable as either `/tmp` (the Unix-y virtual path scripts - /// expect) or its true platform path. Bash builtins write through - /// `Shell.fileSystem` to the same host directories; SwiftPorts CLIs - /// (fd, rg, jq, …) and the SwiftScript interpreter resolve the - /// same paths through ``Shell/currentDirectory`` / + /// The gate accepts two roots: the `workspace` mount and the + /// sandbox's temp dir, in the dir's **host** spelling (the same + /// value `$TMPDIR` carries inside the sandbox). Bash builtins + /// write through `Shell.fileSystem` to the same host directories; + /// SwiftPorts CLIs (fd, rg, jq, …) and the SwiftScript interpreter + /// resolve the same paths through ``Shell/currentDirectory`` / /// ``Shell/resolve(_:)`` and authorise them here. Because both - /// sides land on the same real-disk files, `cd /tmp; mkdir foo; - /// echo > foo/x; fd x foo` finds the file (#48 / #55) — and the - /// same works on hosts where `/tmp` doesn't exist (Windows) or - /// isn't writable (Android emulator) because the host backing is - /// always the platform's real temp dir (#58). + /// sides land on the same real-disk files, `cd "$TMPDIR"; mkdir + /// foo; echo > foo/x; fd x foo` finds the file (#48 / #55) — and + /// the same works on hosts where `/tmp` doesn't exist (Windows) or + /// isn't writable (Android emulator) because the backing always + /// lives under the platform's real temp root (#58). + /// + /// The gate does NOT accept the virtual `/tmp` spelling: callers + /// that reach it carry no virtual→host translation (that lands + /// with #83), so a literal `/tmp/...` here would do real I/O on + /// the host's `/tmp` — a directory shared with every other + /// process — instead of this sandbox's temp dir (#82). Until #83, + /// FileManager-backed callers reach scratch via `$TMPDIR`-spelled + /// paths only; bash builtins use `/tmp` as usual through the + /// mount table. /// /// The temp carve-out checks **both** the unresolved standardised /// path (what the script asked for) and the symlink-resolved path /// (what `FileManager` would actually read) — without the second - /// check a script's `ln -s /etc/passwd /tmp/p` would let + /// check a script's `ln -s /etc/passwd "$TMPDIR/p"` would let /// FileManager-backed bridges follow the link out of the sandbox. /// The bash-side `MountedFileSystem.canonicalGate` already rejects /// this; the URL gate has to match. /// - /// The returned sandbox's `temporaryDirectory` is the host's real - /// temp dir (the same value `$TMPDIR` carries inside the sandbox) - /// so ``Shell/temporaryDirectory`` agrees with bash, scripts using - /// `$TMPDIR`, and FileManager-backed callers. /// - Parameter temporaryDirectory: the host dir reported as - /// ``Shell/temporaryDirectory``. Defaults to - /// `NSTemporaryDirectory()` (the CLI's choice); an embedder that - /// isolates `/tmp` per session (e.g. a per-document subdir) passes - /// its own. The `/tmp` carve-out still covers it as long as it - /// sits under one of the accepted temp prefixes. + /// ``Shell/temporaryDirectory`` and accepted by the temp + /// carve-out; it should exist when this factory runs so its + /// symlink-resolved spelling is computed correctly. The CLI + /// passes its per-instance `swiftbash-` dir (#82). + /// Defaults to `NSTemporaryDirectory()` — note that the default + /// shares the platform temp root with every other process and + /// sandbox instance. /// - Parameter authorizeNetwork: embedder policy for non-file URLs /// (e.g. routing host access through a permission prompt) while - /// still reusing this file-URL gate — `/tmp` carve-out included. + /// still reusing this file-URL gate — temp carve-out included. /// When `nil`, non-file URLs are denied (the base gate's /// `allowedHosts: []`), matching the CLI's offline default. public static func bashWorkspace( @@ -53,6 +59,7 @@ extension ShellKit.Sandbox { isDirectory: true) let tmpURL = temporaryDirectory ?? URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true) + let tempPrefixes = Self.temporaryPrefixes(for: tmpURL) let baseSandbox = ShellKit.Sandbox.rooted( at: workspaceURL, allowedHosts: []) @@ -84,39 +91,44 @@ extension ShellKit.Sandbox { try await baseSandbox.authorize(url) } catch let denial as ShellKit.Sandbox.Denial { // File URL denied by the workspace root — allow it - // only if it lands under an accepted temp prefix. + // only if it lands under the sandbox's temp dir. // Compare against `standardizedFileURL` so - // `/tmp/./foo` and `/tmp/foo` agree. + // `$TMPDIR/./foo` and `$TMPDIR/foo` agree. let unresolved = url.standardizedFileURL.path - guard Self.pathIsInTemp(unresolved) else { throw denial } + guard Self.pathIsInTemp(unresolved, + prefixes: tempPrefixes) + else { throw denial } // Canonical (symlink-resolved) path must stay in a // temp prefix too — defends against a bash-staged - // `ln -s /etc/passwd /tmp/p` escape. + // `ln -s /etc/passwd "$TMPDIR/p"` escape. let resolved = url.resolvingSymlinksInPath() .standardizedFileURL.path - if !Self.pathIsInTemp(resolved) { throw denial } + if !Self.pathIsInTemp(resolved, + prefixes: tempPrefixes) { + throw denial + } } }) } - /// Path prefixes the bash sandbox's URL gate treats as "inside - /// the temp mount": - /// - /// - `/tmp` — the Unix-y virtual path scripts use. - /// - `/private/tmp` — what `/tmp` symlink-resolves to on macOS. - /// - The platform's real temp dir (`NSTemporaryDirectory()`), - /// plus its symlink-resolved spelling, so callers using - /// `$TMPDIR` (which carries the same real path) also pass. - /// - /// Computed once at first use; Foundation's temp dir is stable - /// for the lifetime of the process. - private static let temporaryPrefixes: [String] = { - var prefixes: [String] = ["/tmp", "/private/tmp"] - let raw = NSTemporaryDirectory() - let normalized = (raw as NSString).standardizingPath + /// Path prefixes the URL gate treats as "inside this sandbox's + /// temp dir": the dir's verbatim spelling, its normalised one, + /// and its symlink-resolved one (macOS' temp root lives behind + /// the `/var → /private/var` link, so `FileManager`'s canonical + /// paths carry the `/private` form while `$TMPDIR` carries the + /// bare one). The verbatim spelling matters because + /// `standardizingPath` may rewrite it — swift-corelibs-foundation + /// follows symlinks, Darwin strips `/private` — while callers + /// hand the gate paths built from the `$TMPDIR` value exactly as + /// the embedder spelled it. Derived per instance — a sibling + /// sandbox's temp dir under the same platform root never + /// matches (#82). + private static func temporaryPrefixes(for tempDir: URL) -> [String] { + var prefixes: [String] = [] + let normalized = (tempDir.path as NSString).standardizingPath let resolved = URL(fileURLWithPath: normalized) .resolvingSymlinksInPath().path - for path in [normalized, resolved] { + for path in [tempDir.path, normalized, resolved] { let stripped: String = { if path.count > 1 && path.hasSuffix("/") { return String(path.dropLast()) @@ -128,12 +140,13 @@ extension ShellKit.Sandbox { } } return prefixes - }() + } /// Whether `path` (unresolved or canonical) names a location under - /// any accepted temp prefix — see ``temporaryPrefixes``. - private static func pathIsInTemp(_ path: String) -> Bool { - for prefix in temporaryPrefixes { + /// any accepted temp prefix — see ``temporaryPrefixes(for:)``. + private static func pathIsInTemp(_ path: String, + prefixes: [String]) -> Bool { + for prefix in prefixes { if path == prefix || path.hasPrefix(prefix + "/") { return true } diff --git a/Sources/swift-bash/ExecCommand.swift b/Sources/swift-bash/ExecCommand.swift index d5a39225..b9b5d941 100644 --- a/Sources/swift-bash/ExecCommand.swift +++ b/Sources/swift-bash/ExecCommand.swift @@ -34,12 +34,13 @@ struct ExecCommand: AsyncParsableCommand { help: ArgumentHelp( "Confine the script to a sandboxed view of HOST_DIR. " + "The host directory is mounted at the virtual --workspace " - + "path (default /batch); the platform's real temp dir " - + "(NSTemporaryDirectory) is mounted at virtual /tmp and at " - + "its own path so $TMPDIR works too. Writes inside either " - + "mount land on real disk and are visible to " - + "FileManager-backed callers. Paths outside the mounts " - + "return ENOENT.")) + + "path (default /batch); a per-instance temp dir created " + + "under the platform's temp root (NSTemporaryDirectory) " + + "is mounted at virtual /tmp and at its own path so " + + "$TMPDIR works too, and is removed when the script ends. " + + "Writes inside either mount land on real disk and are " + + "visible to FileManager-backed callers. Paths outside " + + "the mounts return ENOENT.")) var sandbox: String? @Option(name: .long, @@ -90,6 +91,14 @@ struct ExecCommand: AsyncParsableCommand { let source = try Self.readScript(at: scriptPath) let setup = try makeShellSetup() + // The per-instance temp dir (virtual /tmp) is scratch scoped to + // this invocation — remove it however the run ends (normal exit, + // interpreter error, cancellation). + defer { + if let tempDir = setup.temporaryDirectory { + try? FileManager.default.removeItem(at: tempDir) + } + } let shell = Shell(fileSystem: setup.fileSystem, environment: setup.environment) shell.hostInfo = setup.hostInfo @@ -122,6 +131,9 @@ struct ExecCommand: AsyncParsableCommand { let fileSystem: FileSystem let hostInfo: HostInfo let urlSandbox: ShellKit.Sandbox? + /// Per-instance host dir backing virtual `/tmp`, removed when + /// the run ends. `nil` without --sandbox. + let temporaryDirectory: URL? } /// Build the per-mode host-facing state. Sandbox mode: scrub every @@ -135,11 +147,13 @@ struct ExecCommand: AsyncParsableCommand { environment: Environment.current(), fileSystem: RealFileSystem(), hostInfo: .real(), - urlSandbox: nil) + urlSandbox: nil, + temporaryDirectory: nil) } let fileSystem: FileSystem + let tempHost: URL do { - fileSystem = try Self.makeSandboxFileSystem( + (fileSystem, tempHost) = try Self.makeSandboxFileSystem( sandboxRoot: sandboxRoot, workspace: workspace) } catch let err as FileSystemError { @@ -157,54 +171,62 @@ struct ExecCommand: AsyncParsableCommand { // and similar HOME-relative idioms a sensible answer. env["HOME"] = workspace env["PWD"] = workspace - // Foundation chooses the platform-appropriate scratch dir: - // `/tmp` on Linux, `/var/folders/…/T/` on macOS, `%TEMP%` on - // Windows. Setting `$TMPDIR` to that real path means - // FileManager-backed callers (SwiftPorts CLIs, SwiftScript - // bridges) and bash agree on where temp writes land — and - // bash scripts using `mktemp -t foo` get a path that exists - // on every host (#58). - env["TMPDIR"] = NSTemporaryDirectory() + // `$TMPDIR` carries the per-instance dir's HOST path, not the + // virtual `/tmp` spelling: FileManager-backed callers + // (SwiftPorts CLIs, SwiftScript bridges) do real I/O on the + // path as given, with no virtual→host translation yet — that + // lands with #83. The host spelling is the one both bash (via + // the identity mount) and those callers (via the URL gate) + // agree on, on every platform (#58, #82). + env["TMPDIR"] = tempHost.path // ShellKit-side URL gate paired with the real-disk // `MountedFileSystem` above. Bash builtins resolve writes - // through the mount table (host workspace + the host's real - // temp dir, mounted at both virtual `/tmp` and its true path); + // through the mount table (host workspace + the per-instance + // temp dir, mounted at both virtual `/tmp` and its host path); // ShellKit-aware bridges (registered SwiftPorts CLIs, the // SwiftScript interpreter) authorise the same paths via // `Shell.sandbox`. Because both sides hit the same real-disk - // files now, `mkdir -p /tmp/foo && echo > /tmp/foo/x && - // fd x /tmp/foo` actually finds the file (regression fix for - // #48 / #55). - let urlSandbox = ShellKit.Sandbox.bashWorkspace(workspace: workspace) + // files for `$TMPDIR`-spelled paths, `mkdir -p "$TMPDIR/foo" + // && echo > "$TMPDIR/foo/x" && fd x "$TMPDIR/foo"` actually + // finds the file (#48 / #55 / #82). + let urlSandbox = ShellKit.Sandbox.bashWorkspace( + workspace: workspace, + temporaryDirectory: tempHost) return ShellSetup( environment: env, fileSystem: fileSystem, hostInfo: hostInfo, - urlSandbox: urlSandbox) + urlSandbox: urlSandbox, + temporaryDirectory: tempHost) } /// Build the real-disk-backed mount table the `--sandbox` flag - /// installs. `sandboxRoot` (the host workspace dir) appears at the - /// virtual `workspace` mount; the host's real temp dir - /// (``Foundation.NSTemporaryDirectory()``) is mounted at virtual - /// `/tmp` and — when the platform's real temp dir is somewhere - /// other than `/tmp` (macOS, iOS, Windows) — also at its own - /// path so callers using `$TMPDIR` agree with bash's `/tmp`-using - /// scripts. All three mounts are writable. + /// installs, plus the per-instance temp dir backing it. `sandboxRoot` + /// (the host workspace dir) appears at the virtual `workspace` + /// mount; a freshly created `swiftbash-` dir under the + /// platform's temp root (``Foundation.NSTemporaryDirectory()``) is + /// mounted at virtual `/tmp` and at its own host path so callers + /// using `$TMPDIR` agree with bash's `/tmp`-using scripts. All + /// three mounts are writable. + /// + /// The per-instance dir is what keeps concurrent sandboxes (and + /// the host's own temp files) out of each other's `/tmp` (#82). + /// The caller owns its lifetime — the CLI removes it when the + /// script ends. /// /// `MountedFileSystem` enforces confinement: a symlink under any /// mount that points outside its host root reads back as ENOENT, /// and paths outside every mount (`/etc`, `/Users`, …) likewise /// look missing. /// - /// Asking Foundation for the temp dir (rather than hardcoding + /// Asking Foundation for the temp root (rather than hardcoding /// `/tmp`) is what makes the sandbox work on hosts where `/tmp` /// doesn't exist (Windows) or isn't writable (Android emulator's /// read-only root volume). See #58. static func makeSandboxFileSystem( sandboxRoot: String, workspace: String - ) throws -> FileSystem { + ) throws -> (fileSystem: FileSystem, tempHost: URL) { var isDir: ObjCBool = false guard FileManager.default.fileExists( atPath: sandboxRoot, isDirectory: &isDir), @@ -216,24 +238,33 @@ struct ExecCommand: AsyncParsableCommand { throw CLIError( "--workspace: must be an absolute path other than /") } - let tmpHost = NSTemporaryDirectory() - var mounts: [MountedFileSystem.Mount] = [ + // Created eagerly: the mount target must exist for `cd /tmp` + // and friends to see a directory there. + let tempHost = URL(fileURLWithPath: NSTemporaryDirectory(), + isDirectory: true) + .appendingPathComponent("swiftbash-\(UUID().uuidString)", + isDirectory: true) + do { + try FileManager.default.createDirectory( + at: tempHost, withIntermediateDirectories: true) + } catch { + throw CLIError("--sandbox: could not create temp dir " + + "\(tempHost.path): \(error.localizedDescription)") + } + let mounts: [MountedFileSystem.Mount] = [ .init(virtual: workspace, host: sandboxRoot), - .init(virtual: "/tmp", host: tmpHost) + .init(virtual: "/tmp", host: tempHost.path), + // Expose the per-instance dir at its own host path too so + // `$TMPDIR/foo` and `/tmp/foo` resolve to the same files. + // On Linux the temp root IS `/tmp`, so this identity mount + // nests inside the `/tmp` mount above — longest-virtual- + // prefix routing makes the more specific entry win there, + // keeping both spellings on the same host files instead of + // double-nesting the instance dir. + .init(virtual: tempHost.path, host: tempHost.path) ] - // Expose the host's real temp dir at its true path too so - // `$TMPDIR/foo` and `/tmp/foo` resolve to the same files — - // matters on platforms where Foundation's temp dir isn't - // `/tmp` (macOS `/var/folders/…/T/`, Windows `%TEMP%`). On - // Linux Foundation returns `/tmp`, so this would duplicate - // the entry above; the Mount's normalised `virtual` field - // tells us when to skip. - let identityMount = MountedFileSystem.Mount( - virtual: tmpHost, host: tmpHost) - if identityMount.virtual != "/tmp" { - mounts.append(identityMount) - } - return MountedFileSystem(mounts: mounts, backing: RealFileSystem()) + return (MountedFileSystem(mounts: mounts, backing: RealFileSystem()), + tempHost) } /// Apply --allow-url / --allow-method / --dangerous-full-network / diff --git a/Tests/BashInterpreterTests/BashWorkspaceSandboxTests.swift b/Tests/BashInterpreterTests/BashWorkspaceSandboxTests.swift index e7cac71e..70b913d3 100644 --- a/Tests/BashInterpreterTests/BashWorkspaceSandboxTests.swift +++ b/Tests/BashInterpreterTests/BashWorkspaceSandboxTests.swift @@ -3,16 +3,28 @@ import Foundation import ShellKit @testable import BashInterpreter -/// Coverage for `Sandbox.bashWorkspace(workspace:)` — the URL gate the -/// SwiftBash `--sandbox` CLI pairs with the real-disk -/// ``MountedFileSystem`` mounting the workspace and the platform's -/// real temp dir. The gate accepts the virtual workspace mount point, -/// `/tmp`, and the host's real temp dir (`NSTemporaryDirectory()`), so -/// SwiftPorts CLIs and the SwiftScript interpreter authorize the same -/// paths the bash side writes to on every platform. Regression cover -/// for #48 / #55 / #58. +/// Coverage for `Sandbox.bashWorkspace(workspace:temporaryDirectory:)` — +/// the URL gate the SwiftBash `--sandbox` CLI pairs with the real-disk +/// ``MountedFileSystem`` mounting the workspace and a per-instance temp +/// dir. The gate accepts the virtual workspace mount point and the temp +/// dir in its host spelling (what `$TMPDIR` carries); it does NOT accept +/// the virtual `/tmp` spelling — callers of this gate do real I/O on the +/// path as given, so literal `/tmp` would reach the host's shared temp +/// dir, not this sandbox's. Regression cover for #48 / #55 / #58 / #82. @Suite(.timeLimit(.minutes(1))) struct BashWorkspaceSandboxTests { + /// Create (on disk) a per-instance temp dir shaped like the CLI's + /// `swiftbash-`. Caller removes it. + private static func makeInstanceTempDir() throws -> URL { + let dir = URL(fileURLWithPath: NSTemporaryDirectory(), + isDirectory: true) + .appendingPathComponent("swiftbash-gatetest-\(UUID().uuidString)", + isDirectory: true) + try FileManager.default.createDirectory( + at: dir, withIntermediateDirectories: true) + return dir + } + @Test func authorizesWorkspaceRoot() async throws { let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") try await sandbox.authorize( @@ -21,25 +33,70 @@ import ShellKit URL(fileURLWithPath: "/batch/nested/dir/file")) } - @Test func authorizesTmpScratchRoot() async throws { - // The bash sandbox mounts the host's real temp dir at virtual - // `/tmp`; a script's `cd /tmp; fd X` lands at virtual `/tmp` — - // without the carve-out, every SwiftPorts CLI invocation from - // there tripped "file URL is outside sandbox root" (#48). - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") - try await sandbox.authorize(URL(fileURLWithPath: "/tmp")) + @Test func authorizesInstanceTempDir() async throws { + // The CLI passes its per-instance temp dir; the carve-out + // accepts the dir and anything under it, so `$TMPDIR`-spelled + // paths from SwiftPorts CLIs / SwiftScript authorize the same + // files the bash side writes through the mount (#48 / #82). + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) + try await sandbox.authorize(tempDir) try await sandbox.authorize( - URL(fileURLWithPath: "/tmp/retest_fd_repro")) + tempDir.appendingPathComponent("retest_fd_repro")) try await sandbox.authorize( - URL(fileURLWithPath: "/tmp/retest_fd_repro/data.txt")) + tempDir.appendingPathComponent("retest_fd_repro") + .appendingPathComponent("data.txt")) + } + + @Test func deniesVirtualTmpSpelling() async throws { + // Virtual `/tmp` is bash's spelling, translated by the mount + // table. Callers of this gate carry no virtual→host translation + // (that lands with #83) — a literal `/tmp/...` would do real + // I/O on the host's shared `/tmp`, not this sandbox's temp dir, + // so the gate denies it (#82). + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) + await #expect(throws: ShellKit.Sandbox.Denial.self) { + try await sandbox.authorize(URL(fileURLWithPath: "/tmp")) + } + await #expect(throws: ShellKit.Sandbox.Denial.self) { + try await sandbox.authorize( + URL(fileURLWithPath: "/tmp/retest_fd_repro")) + } + } + + @Test func deniesSiblingInstanceTempDir() async throws { + // Two concurrent sandboxes get sibling `swiftbash-` dirs + // under the same platform temp root. Instance A's gate must not + // authorize instance B's dir — nor the shared root itself (#82). + let mine = try Self.makeInstanceTempDir() + let sibling = try Self.makeInstanceTempDir() + defer { + try? FileManager.default.removeItem(at: mine) + try? FileManager.default.removeItem(at: sibling) + } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: mine) + await #expect(throws: ShellKit.Sandbox.Denial.self) { + try await sandbox.authorize( + sibling.appendingPathComponent("leak.txt")) + } + await #expect(throws: ShellKit.Sandbox.Denial.self) { + try await sandbox.authorize( + URL(fileURLWithPath: NSTemporaryDirectory(), + isDirectory: true)) + } } - @Test func authorizesRealTempPath() async throws { - // Callers using `$TMPDIR` (set to `NSTemporaryDirectory()` by - // the CLI) hand the gate the real host path, not `/tmp`. The - // mount table also exposes that real path at its true location - // so both spellings reach the same files; the gate must accept - // both. Regression for #58. + @Test func defaultGateAcceptsPlatformTempRoot() async throws { + // Without an explicit `temporaryDirectory:` the gate falls back + // to the platform temp root — shared with the whole process, + // but what bare API callers had before #82. The CLI always + // passes a per-instance dir instead. let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") let realTemp = NSTemporaryDirectory() try await sandbox.authorize(URL(fileURLWithPath: realTemp)) @@ -60,13 +117,16 @@ import ShellKit } @Test func deniesPrefixSiblings() async throws { - // The classic prefix-collision bug: `/tmpfile` (no trailing - // slash) must not be treated as inside `/tmp`. Same for - // workspace prefix overlap. - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") + // The classic prefix-collision bug: a path that merely starts + // with an accepted prefix's characters (no `/` boundary) must + // not pass. Cover both the temp dir and the workspace. + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) await #expect(throws: ShellKit.Sandbox.Denial.self) { try await sandbox.authorize( - URL(fileURLWithPath: "/tmpfile")) + URL(fileURLWithPath: tempDir.path + "extra")) } await #expect(throws: ShellKit.Sandbox.Denial.self) { try await sandbox.authorize( @@ -93,48 +153,72 @@ import ShellKit // `Docs/Sandboxing.md`). #if !os(Windows) @Test func deniesTmpSymlinkEscape() async throws { - // Regression coverage for the #55 review concern: once the - // temp dir is mounted at virtual `/tmp`, a bash-side - // `ln -s / /tmp/p` plants a real symlink whose *unresolved* - // path the carve-out would otherwise happily authorize — - // letting FileManager-backed bridges follow the link out of - // the sandbox. Plant the fixture at the host's real temp dir - // (always writable, even where `/tmp` isn't) and aim the link - // at `/` so it resolves on every platform — `/etc/passwd` - // doesn't exist on the Android emulator and dangling-link - // resolution behaves differently across the libc backends. - let host = (NSTemporaryDirectory() as NSString) - .appendingPathComponent("swiftbash-escape-\(UUID().uuidString)") + // Regression coverage for the #55 review concern: a bash-side + // `ln -s / "$TMPDIR/p"` plants a real symlink inside the temp + // dir whose *unresolved* path the carve-out would otherwise + // happily authorize — letting FileManager-backed bridges follow + // the link out of the sandbox. Aim the link at `/` so it + // resolves on every platform — `/etc/passwd` doesn't exist on + // the Android emulator and dangling-link resolution behaves + // differently across the libc backends. + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let host = tempDir.appendingPathComponent("escape-link").path try FileManager.default.createSymbolicLink( atPath: host, withDestinationPath: "/") - defer { try? FileManager.default.removeItem(atPath: host) } - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) await #expect(throws: ShellKit.Sandbox.Denial.self) { try await sandbox.authorize(URL(fileURLWithPath: host)) } } #endif +#if !os(Windows) + @Test func acceptsSymlinkSpelledTempDir() async throws { + // An embedder may hand the gate a temp dir reached through a + // symlink and put that spelling in `$TMPDIR`. The carve-out + // must keep the verbatim spelling — `standardizingPath` + // rewrites symlinked components on corelibs-foundation, which + // would otherwise drop the only spelling callers actually use. + let real = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: real) } + let linkPath = (NSTemporaryDirectory() as NSString) + .appendingPathComponent( + "swiftbash-gatelink-\(UUID().uuidString)") + try FileManager.default.createSymbolicLink( + atPath: linkPath, withDestinationPath: real.path) + defer { try? FileManager.default.removeItem(atPath: linkPath) } + + let linkURL = URL(fileURLWithPath: linkPath, isDirectory: true) + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: linkURL) + try await sandbox.authorize(linkURL.appendingPathComponent("f")) + } +#endif + @Test func allowsLegitimateTmpFilesAfterSymlinkResolution() async throws { // The canonical re-check must not regress the legitimate case - // where a real file exists under the host's temp dir (which on - // macOS may symlink-resolve through `/private/var/folders/…` — - // both spellings stay authorized). - let path = (NSTemporaryDirectory() as NSString) - .appendingPathComponent("swiftbash-legit-\(UUID().uuidString)") + // where a real file exists under the temp dir (which on macOS + // symlink-resolves through `/private/var/folders/…` — both + // spellings stay authorized). + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let path = tempDir.appendingPathComponent("legit.txt").path FileManager.default.createFile(atPath: path, contents: Data("hi".utf8)) - defer { try? FileManager.default.removeItem(atPath: path) } - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) try await sandbox.authorize(URL(fileURLWithPath: path)) } @Test func temporaryDirectoryIsRealTempPath() { // `Shell.temporaryDirectory` reads `sandbox.temporaryDirectory`. - // The CLI sets `$TMPDIR = NSTemporaryDirectory()`; the gate has - // to agree so SwiftJSCore's `os.tmpdir()` and similar consumers - // return the same real path the bash environment exposes. + // Without an explicit override the gate reports the platform + // temp root; the CLI overrides with its per-instance dir so + // SwiftJSCore's `os.tmpdir()` and similar consumers return the + // same path `$TMPDIR` carries. let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") let expected = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true).standardizedFileURL.path @@ -159,30 +243,36 @@ import ShellKit @Test func authorizeNetworkRoutesNonFileURLs() async throws { let recorder = URLRecorder() - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") { url in + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) { url in await recorder.record(url) } // Non-file URL → the embedder's closure handles it. try await sandbox.authorize(URL(string: "https://api.example.com/x")!) #expect(await recorder.hosts == ["api.example.com"]) - // File URLs keep using the workspace + /tmp carve-out, never the + // File URLs keep using the workspace + temp carve-out, never the // network closure. try await sandbox.authorize(URL(fileURLWithPath: "/batch/f")) - try await sandbox.authorize(URL(fileURLWithPath: "/tmp/f")) + try await sandbox.authorize(tempDir.appendingPathComponent("f")) #expect(await recorder.hosts.count == 1) } @Test func authorizeNetworkCanDeny() async throws { struct Blocked: Error {} - let sandbox = ShellKit.Sandbox.bashWorkspace(workspace: "/batch") { _ in + let tempDir = try Self.makeInstanceTempDir() + defer { try? FileManager.default.removeItem(at: tempDir) } + let sandbox = ShellKit.Sandbox.bashWorkspace( + workspace: "/batch", temporaryDirectory: tempDir) { _ in throw Blocked() } await #expect(throws: Blocked.self) { try await sandbox.authorize( URL(string: "https://blocked.example/")!) } - // …a /tmp file URL is still allowed (carve-out unaffected). - try await sandbox.authorize(URL(fileURLWithPath: "/tmp/ok")) + // …a temp-dir file URL is still allowed (carve-out unaffected). + try await sandbox.authorize(tempDir.appendingPathComponent("ok")) } } diff --git a/Tests/SwiftBashTests/ExecCommandFileSystemTests.swift b/Tests/SwiftBashTests/ExecCommandFileSystemTests.swift index fc53415e..b73a6eb3 100644 --- a/Tests/SwiftBashTests/ExecCommandFileSystemTests.swift +++ b/Tests/SwiftBashTests/ExecCommandFileSystemTests.swift @@ -9,9 +9,10 @@ import BashInterpreter /// write — workspace AND `/tmp` — in an in-memory layer, so SwiftPorts /// CLIs (`gzip`, `gunzip`, `fd`, …) couldn't see them through /// `Data(contentsOf:)`. The CLI now uses a ``MountedFileSystem`` that -/// puts both mounts on real disk, with the temp dir backed by -/// `NSTemporaryDirectory()` so the sandbox works on every host the -/// rest of the toolkit builds for. Issues #48 / #49 / #58. +/// puts both mounts on real disk, with virtual `/tmp` backed by a +/// per-instance `swiftbash-` dir under `NSTemporaryDirectory()` +/// so the sandbox works on every host AND concurrent instances can't +/// see each other's scratch. Issues #48 / #49 / #58 / #82. @Suite(.timeLimit(.minutes(1))) struct ExecCommandFileSystemTests { /// Each test gets its own scratch dir under `NSTemporaryDirectory()` @@ -29,8 +30,9 @@ import BashInterpreter let host = try Self.makeScratchDir() defer { try? FileManager.default.removeItem(at: host) } - let fileSystem = try ExecCommand.makeSandboxFileSystem( + let (fileSystem, tempHost) = try ExecCommand.makeSandboxFileSystem( sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempHost) } try await fileSystem.writeData( Data("hello\n".utf8), to: "/batch/foo.txt", append: false) @@ -41,41 +43,84 @@ import BashInterpreter #expect(String(bytes: bytes, encoding: .utf8) == "hello\n") } - @Test func tmpWritesPersistToRealTempDir() async throws { + @Test func tmpWritesPersistToPerInstanceTempDir() async throws { let host = try Self.makeScratchDir() defer { try? FileManager.default.removeItem(at: host) } - // Stamp a unique subdir under the host's real temp dir so we - // don't clash with other runs / leak past the test. - let probeName = "swift-bash-tmptest-\(UUID().uuidString)" - let realTemp = NSTemporaryDirectory() - let hostProbe = URL(fileURLWithPath: realTemp) - .appendingPathComponent(probeName) - defer { try? FileManager.default.removeItem(at: hostProbe) } - - let fileSystem = try ExecCommand.makeSandboxFileSystem( + let (fileSystem, tempHost) = try ExecCommand.makeSandboxFileSystem( sandboxRoot: host.path, workspace: "/batch") - try await fileSystem.createDirectory("/tmp/\(probeName)", + defer { try? FileManager.default.removeItem(at: tempHost) } + try await fileSystem.createDirectory("/tmp/probe", intermediates: true) try await fileSystem.writeData( Data("scratch\n".utf8), - to: "/tmp/\(probeName)/data.txt", append: false) + to: "/tmp/probe/data.txt", append: false) // Bash wrote through virtual `/tmp/...`; the mount table sends - // that to the host's real temp dir. FileManager-backed callers - // reading the real path see the same file (#58 — same agreement - // property as #48 / #55, now portable). - let hostFile = hostProbe.appendingPathComponent("data.txt") + // that to this instance's own temp dir — NOT the shared + // platform temp root (#82). FileManager-backed callers reading + // the instance dir's real path see the same file (#48 / #58). + let hostFile = tempHost.appendingPathComponent("probe") + .appendingPathComponent("data.txt") let bytes = try Data(contentsOf: hostFile) #expect(String(bytes: bytes, encoding: .utf8) == "scratch\n") + let rootProbe = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("probe") + #expect(!FileManager.default.fileExists(atPath: rootProbe.path)) + } + + @Test func tmpIsIsolatedPerInstance() async throws { + // Two sandboxes in the same process must not share /tmp: a + // hardcoded name like `/tmp/secret.txt` resolves into each + // instance's own backing dir (#82). + let host = try Self.makeScratchDir() + defer { try? FileManager.default.removeItem(at: host) } + + let (fsA, tempA) = try ExecCommand.makeSandboxFileSystem( + sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempA) } + let (fsB, tempB) = try ExecCommand.makeSandboxFileSystem( + sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempB) } + #expect(tempA.path != tempB.path) + + try await fsA.writeData( + Data("A's secret\n".utf8), to: "/tmp/secret.txt", append: false) + // B sees no such file — not A's content, not a collision. + #expect(try await fsB.metadata("/tmp/secret.txt") == nil) + try await fsB.writeData( + Data("B's notes\n".utf8), to: "/tmp/secret.txt", append: false) + let backA = try await fsA.readData("/tmp/secret.txt") + #expect(String(bytes: backA, encoding: .utf8) == "A's secret\n") + } + + @Test func tmpSpellingsReachTheSameFiles() async throws { + // `$TMPDIR` carries the instance dir's host path; the mount + // table exposes that dir at virtual `/tmp` AND at its own host + // path. Both spellings must hit the same files — including on + // Linux, where the host path nests inside the `/tmp` mount and + // longest-prefix routing has to keep it from double-nesting. + let host = try Self.makeScratchDir() + defer { try? FileManager.default.removeItem(at: host) } + + let (fileSystem, tempHost) = try ExecCommand.makeSandboxFileSystem( + sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempHost) } + + try await fileSystem.writeData( + Data("agree\n".utf8), to: "/tmp/agree.txt", append: false) + let viaHostSpelling = try await fileSystem.readData( + tempHost.appendingPathComponent("agree.txt").path) + #expect(String(bytes: viaHostSpelling, encoding: .utf8) == "agree\n") } @Test func pathsOutsideMountsAreMissing() async throws { let host = try Self.makeScratchDir() defer { try? FileManager.default.removeItem(at: host) } - let fileSystem = try ExecCommand.makeSandboxFileSystem( + let (fileSystem, tempHost) = try ExecCommand.makeSandboxFileSystem( sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempHost) } // `/etc` and `/Users` exist on the host but aren't mounted. // The FS reports `nil` metadata (not a thrown error) so bash // tests like `[ -f /etc/passwd ]` behave as on a chroot. @@ -92,8 +137,9 @@ import BashInterpreter let seed = host.appendingPathComponent("seed.txt") try Data("preseed\n".utf8).write(to: seed) - let fileSystem = try ExecCommand.makeSandboxFileSystem( + let (fileSystem, tempHost) = try ExecCommand.makeSandboxFileSystem( sandboxRoot: host.path, workspace: "/batch") + defer { try? FileManager.default.removeItem(at: tempHost) } let bytes = try await fileSystem.readData("/batch/seed.txt") #expect(String(bytes: bytes, encoding: .utf8) == "preseed\n") }