Run fs helper through Windows sandbox wrapper#28359
Conversation
![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: 6f14dbd816
ℹ️ 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".
| @@ -53,6 +54,8 @@ pub struct FileSystemSandboxContext { | |||
| pub permissions: PermissionProfile, | |||
| #[serde(default, skip_serializing_if = "Option::is_none")] | |||
| pub cwd: Option<PathUri>, | |||
| #[serde(default, skip_serializing_if = "Vec::is_empty")] | |||
| pub workspace_roots: Vec<AbsolutePathBuf>, | |||
There was a problem hiding this comment.
When the filesystem environment is remote on another OS, this new AbsolutePathBuf field serializes the client host's native paths while cwd and operation paths use PathUri specifically to avoid native conversion. A Windows client with workspaceRoots: ["C:\\repo"] talking to a Unix exec-server will fail deserializing the sandbox context before any sandboxed fs/* operation can run. Please carry these roots as URIs or populate them only after translating on the exec-server host.
Useful? React with 👍 / 👎.
6f14dbd to
e68bc1a
Compare
36421a9 to
a6155c2
Compare
a6155c2 to
60d8329
Compare
6eb226d to
653ab5f
Compare
2 participants
Why
This is the final PR in the Windows fs-helper sandbox stack and contains the actual bug fix.
The exec-server filesystem helper is a direct-spawn path: it asks
SandboxManagerfor aSandboxExecRequest, then launches the returned argv itself. That works on macOS and Linux because the transformed argv is already a self-contained sandbox wrapper. On Windows, the transformed request carriedWindowsRestrictedTokenmetadata, but the direct-spawn fs-helper runner still launched the helper argv directly.That means Windows filesystem built-ins backed by the fs-helper could run with the parent Codex process permissions instead of the configured Windows sandbox. This PR makes the direct-spawn transform produce a self-contained Windows wrapper argv before fs-helper launches it.
What Changed
SandboxManager::transform_for_direct_spawn()for callers that launch the returned argv themselves.codex.exe --run-as-windows-sandboxand then marked the outer request as unsandboxed, matching the macOS/Linux wrapper argv shape.exec-server/src/fs_sandbox.rsto use the direct-spawn transform for fs-helper launches.codex.exe --codex-run-as-fs-helperexecutable into.sandbox-binso the sandboxed user can run it.FileSystemSandboxContextasPathUrivalues so:workspace_rootspolicies resolve correctly without sending native client paths over exec-server JSON.Verification
just bazel-lock-updatejust bazel-lock-checkjust test -p codex-sandboxing transform_for_direct_spawn_windowsjust test -p codex-exec-server fs_sandbox::testsjust fix -p codex-windows-sandbox -p codex-sandboxing -p codex-exec-server -p codex-core -p codex-file-systemLocal note:
just fmtcompleted Rust formatting, but this workstation still fails the non-Rust formatter phases because uv cannot open its cache and the local buildifier/dotslash path is missing.