Close #6/#7/#8/#9: PathMapping sandbox, loud divergences, bridged subscripts, bridge-coverage overhaul - #10
Merged
Conversation
…ift-subprocess to 0.5.0 Prerequisite for adopting Sandbox.pathMapping / Shell.resolve virtual→host translation (issue #6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Re-ran Tools/regen-foundation-bridge.sh against the local SDK with an unmodified generator so the PathMapping regen (issue #6) diffs clean. Insertions == deletions: pure entry reordering, no semantic change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… returns the host path Fixes #6 (SwiftBash#83 follow-up). The contract change: authorizePath (String and URL overloads) now resolves the script-supplied spelling through Shell.current.resolve — anchoring relative paths to the shell's virtual CWD and, under a sandbox carrying a PathMapping, translating the virtual spelling to the host directory that backs it — authorizes the *host* form, and returns it. The return is deliberately non-discardable: check and I/O must consume the same path, or the gate authorizes one file while Foundation touches another. - BridgeGeneratorTool: fs-gated args bind as `var` and rebind to the authorized host form (`arg0 = try await authorizePath(arg0, for: .read)`); network gates stay checked-not-rewritten. Bridges regenerated. - Hand-written sites (Interpreter+FileIO) consume the return; gatePath now returns the host path. - FileManager.changeCurrentDirectoryPath becomes a virtual cd on the bound shell's logical CWD instead of erroring — a host chdir would escape the mapping. - Display stays virtual: FileManager.temporaryDirectory reports the sandbox temp region folded through Shell.displayPath (`/tmp` under a mapping); path-echoing returns on FileManager/Bundle (destinationOfSymbolicLink, bundlePath, url(forResource:) …) fold the same way. - Gating policy fixes surfaced by the audit: CharacterSet(contentsOfFile:) was the one file-reading init shipping ungated — now gated; FileManager.containerURL's security-group *identifier* is no longer treated as a path (rewriting it would corrupt the lookup). - Tests: new "Confined sandbox (PathMapping)" suite mirroring SwiftBash's ConfinedSandboxTests — bytes land in mapped host dirs (absolute, relative, URL and FileHandle doors), virtual cd, host spellings and out-of-mount paths denied, symlink escape denied, no host path in script-visible answers or denial text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ional Fixes #8. Silent divergences from stock Swift, in issue order: 1. Unsupported attributes now refuse the script before execution. A preflight scanner rejects @propertyWrapper, @resultBuilder, and every custom attribute (@clamped, @State, attached macros) with an 'unsupported attribute' boundary error — previously the attribute was ignored and the program silently computed a different value (wrapper setters never ran; only a builder block's last statement survived). Availability/optimization/interop/isolation attributes (@available, @discardableResult, @mainactor, …) stay ignorable; type-position attributes (@escaping, @sendable) are exempt. 2. String code-unit views report code units. .utf8/.utf16 are eager arrays of code-unit Ints ('héllo'.utf8.count == 6, for-loops see bytes, Data(s.utf8) still works); .unicodeScalars is an array of opaque Unicode.Scalar with .value. Registered as stdlib surface — no import required, matching stock Swift. Previously .utf8 was a pass-through returning the String (Character count), .utf16 and .unicodeScalars errored. 3. Dynamic casts unwrap Optional layers. dict["k"] as? Int now yields Optional(1) instead of nil; as!/is/switch-case-as follow the same castValue path and bind the unwrapped value. Collection casts also check element types now — ["a"] as? [Int] is nil instead of smuggling Strings through an Int-typed slot. 4. Conformance harness: every Examples/llm_probes/*.swift runs under the interpreter in CI and must byte-match a checked-in golden produced by stock swift (Tools/regen-probe-expectations.sh). Two new probes cover the view counts and optional casts. All 41 probes currently match stock Swift exactly. README: actors and some-P returns actually work (understated); the wrapper/builder line now says they are refused loudly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Data indexing Fixes #9. Bridged types can now expose subscripts — the load-bearing shape of the XCUITest-style APIs the issue targets (app.buttons["Sign In"]): - Bridge gains .subscriptGet((Value, [Value]) async throws -> Value) keyed "subscript Type.get" and .subscriptSet((Value, [Value], Value) async throws -> Value) keyed "subscript Type.set". Args are variadic so string keys, ranges, and element(boundBy:)-shaped access all fit. One deviation from the issue's sketch: the setter returns the receiver to store back rather than Void — value-typed carriers (Data) cannot be mutated through the opaque box, so the body hands back a fresh box and the assignment site writes it to the variable; reference carriers just return the receiver. - doSubscript consults the bridge table for .opaque receivers before the built-in container cases; subscript assignment handles .opaque through .subscriptSet. Unbridged opaque subscripts fail loudly. - The optional-chain walker now shares the full doSubscript dispatch instead of a private array-only copy, so dict/string/bridged subscripts behave identically inside chains. - Data grows the subscripts whose absence was mis-filed as a coverage gap in #7: data[i] byte reads/writes, data[lo..<hi] / data[lo...hi] sub-Data (rebased to zero). - Tests include a fake XCUI-shaped module registered from outside the interpreter — init/computed/subscript/method chain, variadic subscript args — validating the external-module story end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial review of the #6 diff confirmed six defects; all fixed: - CRITICAL: authorizePath resolved an empty path to the CWD itself, turning removeItem(atPath: "") — the classic unset-variable bug — from a guaranteed Foundation error into a recursive delete of the working directory (and fileExists("") into true). Empty paths now pass through untouched so Foundation rejects them as it always did. - URL(fileURLWithPath:) absolutized relative spellings against the host process CWD at construction, so the URL door and the String door could silently name two different files (and relative file URLs were unusable under a mapping). The bridged inits now anchor relative spellings lexically to the shell's logical CWD. - URL.temporaryDirectory / URL.homeDirectory / URL.currentDirectory() and the NSTemporaryDirectory/NSHomeDirectory/NSUserName/ NSFullUserName globals leaked host-captured answers into confined scripts. New .staticComputed bridge kind re-reads the bound shell on every access; the globals fold through Shell.displayPath under a sandbox and keep byte-identical stock answers standalone. Regression tests for each: empty-path guarantees (with an on-disk canary), URL-door/String-door agreement, and virtual spellings for all statics and globals under the confined fixture. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ck, numeric widths Fixes #7. The five root causes, in the issue's leverage order: 1. Collection-shaped signatures bridge. [T], [K: V] and Set<T> parameters and returns compose element bridges into list/dict/set templates (nested collections stay out). Unlocks joined(separator:), Process.arguments, [String] returns, dictionary args across Foundation. 2. Optional parameters bridge through an unwrapping template (.optional boxes and bare values both arrive as T?). Optional path-shaped args on gated IO receivers are skipped outright — an ungated path door would be a sandbox hole. Optional and struct-typed property setters now emit too, and setters peel one Optional layer so Foundation's IUO members take failable-init results, matching stock Swift. 3. Overloads are label-keyed (func URLSession.data(for:) next to data(from:)), with a bare-key alias on the simplest overload for label-less dispatch sites; defaulted suffixes emit one variant per omission so URLRequest(url:) works though the symbol spells (url:cachePolicy:timeoutInterval:). Runtime tries the labeled key first everywhere (methods, FileManager sentinel). The URLSession blocklist shrinks to the unbridgeable bytes trio. 4. Mutating methods and struct property setters write back: new .mutatingMethod and .structSetter bridge kinds return the updated receiver box; dispatch fires on mutable variables and the l-value chain stores the result. data.append(...), request.httpMethod = "POST", request.setValue(_:forHTTPHeaderField:) all work; let bindings stay immutable (isAutoBridgedClass now keys on class setters only). 5. Fixed-width integers and Float cross the boundary with range-checked narrowing (toInt32 throws on overflow; unsigned results wider than Int.max throw instead of wrapping). Concrete bugs from the issue: - FileManager.default's sentinel now falls through to the generated bridges with a real opaque box — copyItem/moveItem/contents/… were generated-but-dead; the virtualised hardcoded methods stay first. - Resources/foundation-allowlist.txt deleted (unreferenced, subsumed). - Tools/bridge-metrics.sh counts the split per-type files (was grepping deleted monoliths, reporting 0) and gains the foundation-types-with-methods axis the issue asked for. Targeted unlocks for the "cannot do today" table: - Task.sleep(nanoseconds:)/(for:) — the wait primitive (bridge closures are async, so it genuinely suspends; Thread stays unbridged: Thread.sleep is noasync). - Regex idiom: String.range(of:options:) returning an opaque Range<String.Index>, string slicing by that range, 3-arg replacingOccurrences, .regularExpression implicit-member context. - JSONSerialization.jsonObject/data/isValidJSONObject mapping untyped JSON onto interpreter values. - JSONEncoder/JSONDecoder date/key strategies (statics + setters). - response as? HTTPURLResponse: opaque casts check the payload's dynamic type (ObjC superclass chain on Darwin) and re-box under the target spelling. - DateFormatter, ISO8601DateFormatter, Pipe promoted (with synthesized inherited no-arg inits); Process stdio slots hand-bridged (Any-typed), still denied under a sandbox; signature-less ObjC methods (Process.waitUntilExit) and Void-typeIdentifier returns now emit. Surface: 2786 -> 4144 generated Foundation bridges, 72/216 types with a working method. Seven new stock-Swift conformance probes (FileManager, polling, untyped JSON, URLRequest/HTTPURLResponse, regex, Data mutation, date formatting) — all 48 probes byte-match stock swift; 495 tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two sandbox escapes (critical/major) and eight correctness/divergence bugs, all confirmed by running the built binary against stock swift: Sandbox: - Generated static methods shipped ungated — Bundle's static resource enumerators (paths/urls/url forResource… inDirectory:/in:) read an arbitrary host directory under a confined sandbox. The static-method emit arm now runs the same gates() as instance methods/inits; the four directory-taking Bundle statics (optional/URL dir args the gate can't rewrite) are blocklisted. - FileManager.mountedVolumeURLs disclosed the full host volume topology through the .default sentinel (no path arg, nothing to gate) — blocklisted. Correctness / silent divergence: - stripped the Optional wrapper (printed 5, not Optional(5)); castValue now never unwraps for an Any/AnyObject target. - Data range subscripts rebased slices to index 0; stock Data slices keep the parent's absolute indices. Slice (not subdata) and index absolutely, so d[1..<3][1] reads absolute 1 and [0] is out of bounds — matching stock. - JSONSerialization collapsed every NSNumber to one native case, so json["price"] as? Double silently failed when the JSON held an integer. castValue now allows int↔integral-double cross-casts (the NSNumber idiom); unsigned JSON integers past Int64.max no longer two's-complement-wrap (route via uint64Value, fall to Double). - JSONSerialization options were dropped unless passed as a bare opaque (array literals silently defaulted to []), and jsonObject force-enabled .fragmentsAllowed. Options are now honored (array literal or bare, via contextual typing for the options: arg) and default to [], so a bare-scalar top level rejects like stock. - Fixed-width/Float value inits (UInt8(3), Int32(7), Float(2.5)) threw "expected String" — the generated init T(_:) bound the String overload. A dispatcher owns init T(_:) and routes numeric → range-checked conversion, String → the failable parse. - Data.append(byte) was unreachable — the UInt8 and Data overloads collide on one label key. A dispatcher type-switches (UInt8 / Data / [UInt8]). - Bridged mutating methods only fired on bare-variable receivers; obj.buf.append(…) through a member-access l-value now reads, runs the .mutatingMethod bridge, and writes the box back through the path (works through class boundaries; let-struct chains error loudly). Subscript/optional-chain receivers remain a limitation, matching subscript-assignment, and fail loudly rather than silently. 18 new regression tests (BridgeReviewRegressionTests) plus two confined-sandbox escape tests; 513 tests pass, all 48 probes still byte-match stock swift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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
Virtualize URL.homeDirectory(forUser:)
Under a bound sandbox, URL.homeDirectory(forUser: "root")?.path executes this newly exposed static bridge against the host account database and returns the real host home directory, unlike the virtualized URL.homeDirectory property and the blocklisted equivalent FileManager.homeDirectory(forUser:). This leaks host identity/layout to confined scripts, so the API needs a shell-aware override or must remain blocked.
ℹ️ 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".
CI (Linux/Windows/iOS/Android were red; macOS green): The wider generator surface newly emitted symbols the name-based scl oracle can't correctly platform-classify. Blocklisted them: - URLSession async overloads whose async form is Apple-only — corelibs ships only the completionHandler spellings (data(for:), download(*), upload(for:from:), flush(), reset(), plus the delegate: variants). This also re-points the bare data()/upload() aliases at the cross-platform data(from:)/upload(for:fromFile:), restoring base behaviour, and removes the download bridges Codex flagged. - StringProtocol.propertyListFromStringsFileFormat() (Apple-only), ProcessInfo sudden/automatic-termination hints and FileManager.unmountVolume (macOS-only, unavailable on iOS), FileHandle.fileDescriptor (unavailable on Windows). Proactively scanned the symbol graphs for every other emitted method/ property that is async-underlying or iOS/tvOS/watchOS/Windows- unavailable — none remain beyond the base-safe data(from:)/ upload(for:fromFile:). Codex review: - P1 URL.homeDirectory(forUser:) leaked the host home dir under a sandbox — blocklisted (matches the already-blocked FileManager twin). - P1 URL(fileURLWithPath:relativeTo:) / (isDirectory:relativeTo:) with a nil base absolutized against the host process CWD; now anchors to the shell's virtual CWD like the no-base inits (honours a real base when given). - P1 URLSession.download temp-URL leak — resolved by the blocklist above (the download bridges are gone). - P2 the attribute preflight rejected @unknown default: and other non-declaration attributes; it now flags only declaration attributes, so switch-case/statement/closure attributes pass. Tests: new confined-sandbox tests (relativeTo-nil anchoring, homeDirectory(forUser:) unreachable) and an @unknown-default probe. 513 tests pass; all 48 stock-Swift probes still byte-match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Re: the Codex review's 🤖 Addressed by Claude Code |
…rwin-only probe goldens The build fixes turned the red platforms' compile errors into test failures; two root causes: 1. ownerAndMember(forBridgeKey:) never stripped the 'mutating func ' (or 'set var ' / 'subscript ') prefix, so every mutating method and struct-property setter was classified with owner "mutating func <Type>" — absent from the scl oracle — and wrongly Darwin-gated. Data.append(contentsOf:), URLRequest.setValue, and the struct setters are in swift-corelibs-foundation and now emit cross-platform, so d.append(...) / request.setValue(...) work on Linux/Windows/Android too (and the regression tests pass there). 2. ProbeConformanceTests replayed the checked-in goldens on every platform, but the goldens are captured from macOS stock swift and several probes exercise Apple-Foundation-only behaviour (regex range(of:options:), CharacterSet set-algebra the scl extractor doesn't surface) that is correctly Darwin-gated elsewhere. Guarded the suite with #if canImport(Darwin) — it still runs on macOS/iOS where the goldens are valid; cross-platform build is covered by the build jobs. 516 tests pass on macOS (probes included); the mutating-method regressions now use cross-platform bridges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On Linux/Android `NSTemporaryDirectory()` is `/tmp`, so the host spelling of a `/tmp`-backed mount starts with `/tmp` and re-matches the `/tmp` virtual prefix instead of landing outside the namespace — resolve() maps it to a nonexistent doubled path (no security escape, but a file-not-found rather than a Sandbox.Denial), so the assertion was nondeterministic there. The test now uses a dedicated single `/work` mount whose host spelling matches no mount on either platform and reliably voids, mirroring how SwiftBash's own ConfinedSandboxTests sidesteps a `/tmp` mount for this case. It also asserts the virtual `/work` spelling still reads the file, proving the mount is live. 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
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.
Closes #6, #7, #8, #9.
Four open issues, implemented on one branch (the commits build on each other, so they don't split cleanly). Each phase was adversarially reviewed and every confirmed defect fixed. 513 tests pass; all 48
Examples/llm_probes/scripts byte-match stockswiftin a new CI conformance harness.#6 — Adopt ShellKit's PathMapping (#6)
authorizePath(String + URL overloads) now resolves the script path throughShell.current.resolve— anchoring relative paths to the shell's virtual CWD and, under aPathMapping-carrying sandbox, translating the virtual spelling to the host directory that backs it — authorizes the host form, and returns it. The bridge generator rebinds every fs-gated arg to that return (arg0 = try await authorizePath(arg0, …)) so the check and the I/O consume the same path (translating one but not the other is an escape).FileManager.changeCurrentDirectoryPathbecomes a virtualcdon the shell's logical CWD (a realchdirwould escape the mapping).temporaryDirectory, path-echoing returns on FileManager/Bundle, and theURLstatics /NS*globals fold throughShell.displayPath.ConfinedSandboxTests.#8 — Ignored constructs fail loudly; utf8/
as?fixed (#8)@propertyWrapper/@resultBuilder/ custom attributes now refuse the script before execution instead of silently computing a different value.String.utf8/.utf16/.unicodeScalarsreport code units ("héllo".utf8.count == 6), not the Character count.as?unwrapsOptionallayers (dict["k"] as? Int→Optional(1)) and checks collection element types.Tools/regen-probe-expectations.sh,ProbeConformanceTests) so divergence fails CI. README corrected (actors /some Pwork; wrapper/builder now refused loudly).#9 — Bridged-type subscripts (#9)
New
Bridge.subscriptGet/subscriptSetkinds (variadic args, struct writeback), opaque dispatch indoSubscriptand subscript-assignment,Dataindexing/slicing/byte-writes, and optional-chain parity. Validated end-to-end with a fake XCUITest-shaped module registered from outside the interpreter — the exactapp.buttons["Sign In"].tap()shape the issue targets.#7 — Foundation bridge coverage (#7)
All five generator root causes fixed, in leverage order:
[T],[K: V],Set<T>params/returns compose element bridges.URLRequest(url:)alongside the full(url:cachePolicy:timeoutInterval:))..mutatingMethod/.structSetterkinds.Floatcross the boundary with range-checked narrowing.Concrete bugs from the issue: FileManager's
.defaultsentinel now falls through to its generated bridges (copyItem/moveItem/contents/… were dead); deadfoundation-allowlist.txtdeleted;bridge-metrics.shcounts the split per-type files and gains a "types with a working method" axis. Every "cannot do today" row now works —Task.sleep, the regex idiom,JSONSerialization, encoder strategies,response as? HTTPURLResponse,DateFormatter/ISO8601DateFormatter/Pipe, and Process pipelines. Foundation surface: ~2,786 → ~4,140 generated bridges.Adversarial review fixes
Two review rounds ran against the built binary vs stock
swift. Round 1 (on #6) caught a critical empty-path bug (removeItem("")would recursively delete the CWD) plus five host-path leaks. Round 2 (on #7/#8/#9) caught two sandbox escapes — ungatedBundlestatic directory-enumerators, andFileManager.mountedVolumeURLsdisclosing the host volume topology — plus eight correctness bugs (x as Anystripping Optionals,Dataslice index rebasing, JSONNSNumbercast collapse + unsigned overflow + dropped write options, missing numeric-conversion inits, unreachableData.append(byte), mutating methods through property chains). All confirmed findings are fixed and regression-tested.Known limitation: mutating a bridged value through a subscript or optional-chain receiver (
list[0].append(…)) isn't supported — it errors loudly, matching the existing subscript-assignment constraint.🤖 Generated with Claude Code