Report remote sandbox denials semantically#29424
Merged
Merged
Conversation
jif-oai
added a commit
that referenced
this pull request
Jun 22, 2026
## Why PR #29108 lets the orchestrator send sandbox intent with `process/start` without wrapping the command for its own operating system. This PR completes that boundary by making the executor interpret and enforce the intent using its own filesystem paths and sandbox implementation. For example, a macOS TUI targeting a Linux devbox sends `/bin/bash -lc pwd`. The Linux executor turns that into its own `codex-linux-sandbox ... /bin/bash -lc pwd` launch. ## What changes - Keep `process/start` unchanged when no sandbox intent is present. - Convert sandbox `PathUri` values into native paths on the executor. - Bind symbolic `:workspace_roots` permissions to the executor's native sandbox cwd. - Select the sandbox implementation on the executor and wrap the original command immediately before spawning it. - Reject sandbox-required execution before spawning when the executor cannot enforce the intent. - Pass exec-server runtime paths into process creation so Linux can locate `codex-linux-sandbox`. The boundary is therefore: ```text orchestrator executor original argv + sandbox intent -> select and enforce local sandbox ``` This PR intentionally treats a denied remote command as an ordinary command failure. Draft follow-up #29424 carries a semantic `sandboxDenied` result back to unified exec for the existing approval and retry flow. ## Platform scope Linux and macOS use their existing direct-spawn sandbox transforms. Windows sandboxed remote process launch is intentionally unsupported in this PR. The current Windows direct-spawn wrapper does not correctly preserve arbitrary argv, TTY behavior, or pass the full child environment out of band. The executor rejects the request instead of running it incorrectly or unsandboxed. ## Known follow-ups - The transported permission profile can still contain orchestrator-materialized helper or explicit paths. A `TODO(jif)` marks where the executor boundary should receive pre-host-materialization permission intent. - The sandbox wrapper currently replaces a requested custom inner `arg0`. A `TODO(jif)` marks where this must be preserved or rejected explicitly. - Draft PR #29424 contains the deferred sandbox-denial classification and approval/retry behavior. ## Rollout assumption This executor-sandbox stack is unreleased and its client and executor are expected to move together. This PR does not add mixed-version negotiation with older exec servers.
Base automatically changed from
jif/remote-exec-server-sandbox-enforcement
to
main
June 22, 2026 10:45
f1e5329 to
62bd4e6
Compare
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62bd4e6684
ℹ️ 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".
62bd4e6 to
4cb3f32
Compare
d244fdc to
dec63c3
Compare
sayan-oai
approved these changes
Jun 22, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Why
#29113 moved remote sandbox setup and enforcement to the exec server. That gives the executor ownership of the platform-specific work: a Linux executor chooses and runs a Linux sandbox even when the Codex orchestrator is running on macOS or Windows.
It also means the orchestrator no longer knows which concrete sandbox the executor selected. When that sandbox blocks a remote command, the orchestrator currently sees only a failed process and can treat the denial as an ordinary command failure. The existing sandbox approval and retry path is then skipped.
This PR lets the executor report one portable fact:
The executor keeps its concrete sandbox type private. The protocol sends only the semantic result.
Example
Suppose a local macOS Codex session asks a Linux devbox to write outside the allowed workspace.
Before this PR:
With this PR:
What changes
The executor remembers its selected sandbox
The prepared remote process now retains the executor-selected
SandboxType. This value never crosses the executor boundary.Commands started without a sandbox retain
SandboxType::Noneand are never reported as sandbox denials.The executor uses the existing denial heuristic
The existing local denial heuristic moves from
codex-coreinto the sharedcodex-sandboxingcrate.When a sandboxed remote process exits, the executor:
This deliberately matches the old local unified-exec behavior. It does not add a new streaming classifier, another output buffer, or stronger output-retention guarantees.
The protocol reports a portable boolean
process/readgainssandboxDenied:{ "exited": true, "exitCode": 1, "closed": false, "sandboxDenied": true }The field defaults to
falsewhen an older executor omits it. The response does not expose the executor sandbox implementation or executor-native paths.Unified exec uses the existing error path
The exec-server client carries
sandboxDeniedinto the unified process state. If it is true, unified exec returns the existingSandboxDeniederror instead of trying to classify remote output using an orchestrator-side sandbox type.Remote process exit remains visible as soon as the process exits. This PR does not wait for stdout or stderr to close and does not change the existing process lifecycle.
Scope
This PR is intentionally limited to matching the existing local unified-exec behavior for the initial command execution path.
It does not add:
write_stdin;Those can be considered separately if the same behavior is added for local execution.
Test coverage
One remote end-to-end integration test covers the complete intended flow:
Existing lifecycle coverage continues to verify that remote process exit is reported before late output streams close.