[codex] Tighten unified exec sandbox setup#22207
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
aafd6f8 to
afec29e
Compare
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/core/src/tools/runtimes/unified_exec.rs
Lines 280 to 284 in afec29e
The zsh-fork path passes an ExecRequest whose policy cwd now comes from the orchestrator default. For a non-primary environment_id, intercepted execs in the long-lived shell are evaluated against the primary cwd rather than the environment actually running the process, so later subcommands can be incorrectly allowed, prompted, or escalated.
ℹ️ 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".
Refine unified exec sandbox initialization while preserving the requested process workdir. Co-authored-by: Codex <noreply@openai.com>
Keep sandbox and approval evaluation aligned with the selected turn environment while allowing the process workdir to vary independently. Co-authored-by: Codex <noreply@openai.com>
f635339 to
ca2e4bb
Compare
evawong-oai
left a comment
There was a problem hiding this comment.
Approved based on macOS validation.
Validated PR head ca2e4bbca18cb814776d0aca0a6038c3eb65269e.
-
macOS: full report shaped validation passed on the macOS validation host. The model supplied
workdirno longer becomes the sandbox policy root, and the outside marker write is denied. -
Windows: same report shaped validation still reproduces on the Windows validation host. Base and PR head both allowed the outside marker write. This looks like a Windows specific remaining gap, because the Windows sandbox setup still grants write access to the command cwd for workspace write. I am treating that as next PR scope, not a blocker for this macOS scoped fix.
This approval is for the macOS fix in this PR. Windows should be handled in a follow up PR.
Summary
Validation
/tmp/cargo-tools/bin/just fmt.codex-coreregression tests successfully.cargo test -p codex-core; it did not complete cleanly because unrelated existing agent/config-loader tests failed and the run later aborted on a stack overflow intools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed.