mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
3a16fc6c19f97543202b37bfcc5a5b967cbd0746
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 jif
|
e476fc16ce |
Prepare managed network sandbox context (#29456)
## Why Managed network configures commands to use local HTTP and SOCKS proxies. For commands delegated to the exec server, the proxy environment and the sandbox policy were prepared separately. On macOS, that meant a command could receive `HTTPS_PROXY=http://127.0.0.1:43123` while Seatbelt still denied access to port `43123`. ## What changed `NetworkProxy` now prepares the command environment and sandbox context together from the same runtime snapshot: ```text Prepared managed network ├── command environment: HTTPS_PROXY=http://127.0.0.1:43123 └── sandbox context: allow outbound to 127.0.0.1:43123 ``` That context travels with remote exec requests. The exec server preserves the managed proxy and CA environment, and macOS Seatbelt allows only the prepared loopback proxy ports without enabling broad network access or local binding. The protocol field is optional and the existing enforcement flag remains in place, preserving compatibility with callers that do not send the new context. |
||
|
jif
|
9c3b10e5d4 |
Apply sandbox intent inside remote exec servers (#29113)
## 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. |
||
|
jif
|
bd2968a4db |
Carry sandbox intent to remote exec servers (#29108)
## What changed PR #29099 stopped sending the orchestrator's concrete sandbox wrapper to a remote exec-server. Remote commands now arrive as plain native argv. This PR adds the next piece: Codex also sends portable sandbox intent next to that plain argv. For a remote unified-exec command, the request can now include: - the canonical permission profile before local workspace-root materialization - the sandbox cwd and workspace roots as `PathUri` values - Windows sandbox settings - the legacy Landlock setting - whether managed networking must be enforced The important part is that symbolic entries such as `:workspace_roots` stay symbolic while crossing the boundary. The executor can then bind them to its own workspace-root paths instead of receiving orchestrator-local absolute paths. The data travels through `ExecRequest` into `ExecParams`. Older exec-servers can still deserialize requests because the new fields have defaults. ## Why The orchestrator should not decide how another machine implements sandboxing. For example: - a local macOS Codex would normally build a Seatbelt command - a remote Linux executor needs a Linux sandbox command instead The orchestrator now sends the plain command plus the policy it intended to enforce. A later PR can let the exec-server choose and build the correct sandbox for its own operating system. ## Important detail This keeps the portable intent separate from the local `SandboxType`. `SandboxType::None` is ambiguous: - it can mean the command was explicitly approved to run without a sandbox - it can also mean the orchestrator host has no concrete sandbox implementation available Those cases are different for remote execution. This PR adds `sandbox_requested` so an executor can still receive sandbox intent when the orchestrator cannot build a local wrapper. Explicit unsandboxed retries still send no sandbox context. ## Behavior today This PR only transports the intent. The exec-server accepts the new fields but does not apply them yet. Remote commands therefore remain unsandboxed after this PR, just as they are after PR #29099. ## Follow-up The next PR will make exec-server read this portable intent, bind symbolic workspace permissions to executor-native roots, choose the sandbox for its own operating system, build the wrapper locally, and then spawn the command. |
||
|
jif
|
04483f4ce5 |
Keep remote exec commands native to the executor (#29099)
## Summary
- Remote unified-exec now sends the original command argv to exec-server
instead of materializing the orchestrator's sandbox wrapper first.
- Local unified-exec keeps the existing sandbox path unchanged.
- Add a focused regression test for a macOS-selected sandbox producing
plain remote argv.
Before:
macOS orchestrator -> /usr/bin/sandbox-exec ... -> Linux exec-server
After:
macOS orchestrator -> /bin/bash -lc pwd -> Linux exec-server
This is intentionally only the first cleanup step. Remote unified-exec
commands are sent without a process sandbox until the targeted
follow-ups below land. For the macOS-to-Linux path this is not a
practical regression: the old sandboxed attempt failed before process
launch because the Linux executor could not spawn macOS sandbox paths.
## Targeted follow-ups
1. Carry sandbox intent separately from argv.
- Add an optional sandbox field to exec-server process params.
- Reuse FileSystemSandboxContext rather than introducing a new sandbox
model.
- Carry managed-network enforcement as one explicit bit.
- Keep argv plain.
2. Apply that intent inside exec-server.
- Add a small process-start adapter before LocalProcess::exec.
- Reuse the existing codex-sandboxing SandboxManager and exec-server
runtime paths.
- Follow the same shape already used by exec-server filesystem
sandboxing.
- Do not duplicate or move the sandbox implementations.
3. Report the sandbox actually used.
- Return the executor-selected sandbox type from process/start.
- Use that value in core for sandbox-denial detection and retry
behavior.
## End state
The orchestrator sends plain commands plus portable sandbox intent. The
executor chooses and applies its own native sandbox: Linux executors use
Linux sandboxing, macOS executors use Seatbelt, and Windows executors
use Windows sandboxing. Concrete wrapper argv, helper paths, and sandbox
env markers never cross the executor boundary.
|
||
|
Michael Bolin
|
6e10142199 |
fix: preserve deny-read sandboxing for safe commands (#23943)
## Why Permission profiles can mark filesystem entries as unreadable with `deny` rules, including glob patterns. Several shell execution paths treated known-safe commands or execpolicy `allow` rules as sufficient to run outside the filesystem sandbox. That is not valid for read-capable commands: for example, `cat` or `ls` may be reasonable to allow generally, but dropping the sandbox would also drop deny-read constraints such as `**/*.env`. ## What changed - Added a shared check that treats active deny-read restrictions as incompatible with unsandboxed execution. - Kept first-attempt execution sandboxed for explicit escalation and execpolicy allow bypasses when deny-read entries are present. - Prevented no-sandbox retry after a sandbox denial when the active filesystem policy contains deny-read entries. - Updated the zsh-fork execve path so prefix-rule `allow` decisions continue inside the current sandbox when deny-read restrictions are active. ## Verification - `cargo test -p codex-core tools::sandboxing::tests` - `cargo test -p codex-core tools::runtimes::shell::unix_escalation::tests` - `cargo test -p codex-core shell_command_enforces_glob_deny_read_policy` |
||
|
viyatb-oai
|
3c76081876 |
Make deny canonical for filesystem permission entries (#23493)
## Why Filesystem permission profiles used `none` for deny-read entries, which is less direct than the action the entry actually represents. This change makes `deny` the canonical filesystem permission spelling while preserving compatibility for older configs that still send `none`. ## What changed - rename `FileSystemAccessMode::None` to `Deny` - serialize and generate schemas with `deny` as the canonical value - retain `none` only as a legacy input alias for temporary config compatibility - update filesystem glob diagnostics and regression coverage to use the canonical spelling - refresh config and app-server schema fixtures to match the new wire shape ## Validation - `cargo test -p codex-protocol` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-core config_toml_deserializes_permission_profiles --lib` - `cargo test -p codex-core read_write_glob_patterns_still_reject_non_subpath_globs --lib` Earlier in the session, a broad `cargo test -p codex-core` run reached unrelated pre-existing failures in timing/snapshot/git-info tests under this environment; the targeted surfaces touched by this PR passed cleanly. |
||
|
Michael Bolin
|
0a83353ca3 |
test: reduce core sandbox policy test setup (#23036)
## Why `SandboxPolicy` is a legacy compatibility shape, but several core tests still used it for ordinary turn setup even when the runtime path now carries `PermissionProfile`. With the first cleanup PR merged, this follow-up trims more core test scaffolding so remaining `SandboxPolicy` matches are easier to classify as production compatibility, legacy-boundary coverage, or explicit conversion tests. ## What Changed - Updated apply-patch handler and runtime tests to pass `PermissionProfile` directly. - Changed sandboxing test helpers to build permission profiles without first creating `SandboxPolicy` values. - Converted request-permissions integration turns to pass `PermissionProfile` through the test helper, leaving legacy sandbox projection at the `Op::UserTurn` boundary. - Converted unified exec integration helpers and direct turn submissions to use `PermissionProfile` values instead of `SandboxPolicy` setup. - Removed now-unused `SandboxPolicy` imports from the touched core tests. ## Test Plan - `just fmt` - `cargo test -p codex-core --lib tools::sandboxing::tests` - `cargo test -p codex-core --lib tools::runtimes::apply_patch::tests` - `cargo test -p codex-core --lib tools::handlers::apply_patch::tests` - `cargo test -p codex-core --lib unified_exec::process_manager::tests` - `cargo test -p codex-core --test all request_permissions::` - `cargo test -p codex-core --test all unified_exec::` - `just fix -p codex-core` |
||
|
|
6506765168 |
fix(permissions): preserve managed deny-read during escalation (#15977)
## Why
Managed filesystem `deny_read` requirements are administrator-enforced
restrictions on specific paths. Once those requirements are active,
Codex should not drop them just because an execution path would
otherwise leave the sandbox.
Before this change, an explicit escalation, a prefix-rule allow, a
sandbox-denial retry, or an app-server legacy sandbox override could
rebuild the runtime policy without those managed read-deny entries and
expose a path the administrator had marked unreadable.
This is narrower than general sandbox-mode constraints. If an enterprise
only sets `allowed_sandbox_modes`, a trusted `prefix_rule(..., decision
= "allow")` can still run its matching command unsandboxed; this PR only
preserves managed filesystem `deny_read` restrictions across those
paths.
## What Changed
- Mark filesystem policies built from managed `deny_read` requirements
so callers can tell when those deny entries must survive escalation.
- Preserve managed deny-read entries when runtime permission profiles
are rebuilt through protocol, app-server, or legacy sandbox-policy
compatibility paths.
- Keep managed deny-read attempts inside the selected sandbox on the
first attempt and after sandbox-denial retries.
- Preserve the same behavior in the zsh-fork escalation path, including
prefix-rule-driven escalation.
- Add a regression test showing the opposite case too: without managed
deny-read, a prefix-rule allow still chooses unsandboxed execution.
## Verification
Targeted automated verification:
```shell
cargo test -p codex-core shell_request_escalation_execution_is_explicit -- --nocapture
cargo test -p codex-core prefix_rule_uses_unsandboxed_execution_without_managed_deny_read -- --nocapture
cargo test -p codex-core prefix_rule_preserves_managed_deny_read_escalation -- --nocapture
cargo test -p codex-protocol permission_profile_round_trip_preserves_filesystem_policy_metadata -- --nocapture
cargo test -p codex-protocol preserving_deny_entries_keeps_unrestricted_policy_enforceable -- --nocapture
cargo test -p codex-app-server-protocol permission_profile_file_system_permissions_preserves_policy_metadata -- --nocapture
cargo check -p codex-app-server -p codex-tui
```
Smoke-test invocations:
```shell
# macOS exact deny + allowed control
codex exec --skip-git-repo-check -C "$ROOT" \
-c 'default_permissions="deny_read_smoke"' \
-c 'permissions.deny_read_smoke.filesystem={":minimal"="read",":project_roots"={"."="write","secrets"="none","future-secret"="none","**/*.env"="none"}}' \
'Run shell commands only. Print the contents of allowed.txt. Then test whether reading secrets/exact-secret.txt succeeds without printing that file if it does. End with exactly two lines: allowed=<contents> and exact_secret=<BLOCKED or READABLE>.'
# Linux exact deny + allowed control
codex exec --skip-git-repo-check -C "$ROOT" \
-c 'default_permissions="deny_read_smoke"' \
-c 'permissions.deny_read_smoke.filesystem={":minimal"="read",glob_scan_max_depth=3,":project_roots"={"."="write","secrets"="none","future-secret"="none","**/*.env"="none"}}' \
'Run shell commands only. Print the contents of allowed.txt. Then test whether reading secrets/exact-secret.txt succeeds without printing that file if it does. End with exactly two lines: allowed=<contents> and exact_secret=<BLOCKED or READABLE>.'
```
Observed manual smoke matrix:
| Case | macOS Seatbelt | Linux bubblewrap |
| --- | --- | --- |
| `cat allowed.txt` | Pass | Pass |
| `cat secrets/exact-secret.txt` | Blocked | Blocked |
| `cat envs/root.env` | Blocked | Blocked |
| `cat envs/nested/one.env` | Blocked | Blocked |
| `cat envs/nested/two.env` | Blocked | Blocked |
| `cat alias-to-secrets/exact-secret.txt` | Blocked | Blocked |
| Missing denied path | A file created after sandbox setup remained
unreadable | Creation was blocked by the reserved missing-path
placeholder, and the placeholder was cleaned up after exit |
| Real `codex exec` shell turn | Pass | Pass |
Notes:
- The Linux smoke run used the fallback glob walker because the devbox
did not have `rg` installed.
- The smoke matrix verifies the end-to-end filesystem behavior on macOS
and Linux; the escalation-specific behavior is covered by the focused
tests above.
---------
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Charlie Marsh <charliemarsh@openai.com>
|
||
|
|
305825abd9 |
Support MCP tools in hooks (#18385)
## Summary Lifecycle hooks currently treat `PreToolUse`, `PostToolUse`, and `PermissionRequest` as Bash-only flows - hook schema constrains `tool_name` to `Bash` - hook input assumes a command-shaped `tool_input` - core hook dispatch path passes only shell command strings That means hooks cannot target MCP tools even though MCP tool names are model-visible and stable This change generalizes those hook paths so they can match and receive payloads for MCP tools while preserving the existing Bash behavior. ## Reviewer Notes I think these are the key files - `codex-rs/core/src/tools/handlers/mcp.rs` - `codex-rs/core/src/mcp_tool_call.rs` Otherwise the changes across apply_patch, shell, and unified_exec are mainly to rewire everything to be `tool_input` based instead of just `command` so that it'll make sense for MCP tools. ## Changes - Allow `PreToolUse`, `PostToolUse`, and `PermissionRequest` hook inputs to carry arbitrary `tool_name` and `tool_input` values instead of hard-coding `Bash` and command-only payloads. - Add MCP hook payload support through `McpHandler`, using the model-visible tool name from `ToolInvocation` and the raw MCP arguments as `tool_input`. - Include MCP tool responses in `PostToolUse` by serializing `McpToolOutput` into the hook response payload. - Run `PermissionRequest` hooks for MCP approval requests after remembered approval checks and before falling back to user-facing MCP elicitation. - Preserve exact matching for literal hook matchers like `Bash` and `mcp__memory__create_entities`, while keeping regex matcher support for patterns like `mcp__memory__.*` and `mcp__.*__write.*`. --------- Co-authored-by: Andrei Eternal <eternal@openai.com> Co-authored-by: Codex <noreply@openai.com> |
||
|
Jack Mousseau
|
b7dba72dbd | Rename reject approval policy to granular (#14516) | ||
|
Michael Bolin
|
0c8a36676a |
fix: move inline codex-rs/core unit tests into sibling files (#14444)
## Why PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This applies the same extraction pattern across the rest of `codex-rs/core` so the production modules stay focused on runtime code instead of large inline test blocks. Keeping the tests in sibling files also makes follow-up edits easier to review because product changes no longer have to share a file with hundreds or thousands of lines of test scaffolding. ## What changed - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**` with a path-based module declaration - moved each extracted unit test module into a sibling `*_tests.rs` file, using `mod_tests.rs` for `mod.rs` modules - preserved the existing `cfg(...)` guards and module-local structure so the refactor remains structural rather than behavioral ## Testing - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`) - `just fix -p codex-core` - `cargo fmt --check` - `cargo shear` |