mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
c528f32acb4cb8a91c73fa2b41e6a96fb8a50955
6 Commits
-
fix: keep shell escalation exec paths absolute (#12750)
## Why In the `shell_zsh_fork` flow, `codex-shell-escalation` receives the executable path exactly as the shell passed it to `execve()`. That path is not guaranteed to be absolute. For commands such as `./scripts/hello-mbolin.sh`, if the shell was launched with a different `workdir`, resolving the intercepted `file` against the server process working directory makes policy checks and skill matching inspect the wrong executable. This change pushes that fix a step further by keeping the normalized path typed as `AbsolutePathBuf` throughout the rest of the escalation pipeline. That makes the absolute-path invariant explicit, so later code cannot accidentally treat the resolved executable path as an arbitrary `PathBuf`. ## What Changed - record the wrapper process working directory as an `AbsolutePathBuf` - update the escalation protocol so `workdir` is explicitly absolute while `file` remains the raw intercepted exec path - resolve a relative intercepted `file` against the request `workdir` as soon as the server receives the request - thread `AbsolutePathBuf` through `EscalationPolicy`, `CoreShellActionProvider`, and command normalization helpers so the resolved executable path stays type-checked as absolute - replace the `path-absolutize` dependency in `codex-shell-escalation` with `codex-utils-absolute-path` - add a regression test that covers a relative `file` with a distinct `workdir` ## Verification - `cargo test -p codex-shell-escalation`
Michael Bolin ·
2026-02-24 23:52:36 -08:00 -
fix: make EscalateServer public and remove shell escalation wrappers (#12724)
## Why `codex-shell-escalation` exposed a `codex-core`-specific adapter layer (`ShellActionProvider`, `ShellPolicyFactory`, and `run_escalate_server`) that existed only to bridge `codex-core` to `EscalateServer`. That indirection increased API surface and obscured crate ownership without adding behavior. This change moves orchestration into `codex-core` so boundaries are clearer: `codex-shell-escalation` provides reusable escalation primitives, and `codex-core` provides shell-tool policy decisions. Admittedly, @pakrym rightfully requested this sort of cleanup as part of https://github.com/openai/codex/pull/12649, though this avoids moving all of `codex-shell-escalation` into `codex-core`. ## What changed - Made `EscalateServer` public and exported it from `shell-escalation`. - Removed the adapter layer from `shell-escalation`: - deleted `shell-escalation/src/unix/core_shell_escalation.rs` - removed exports for `ShellActionProvider`, `ShellPolicyFactory`, `EscalationPolicyFactory`, and `run_escalate_server` - Updated `core/src/tools/runtimes/shell/unix_escalation.rs` to: - create `Stopwatch`/cancellation in `codex-core` - instantiate `EscalateServer` directly - implement `EscalationPolicy` directly on `CoreShellActionProvider` Net effect: same escalation flow with fewer wrappers and a smaller public API. ## Verification - Manually reviewed the old vs. new escalation call flow to confirm timeout/cancellation behavior and approval policy decisions are preserved while removing wrapper types.
Michael Bolin ·
2026-02-24 16:20:08 -08:00 -
feat: run zsh fork shell tool via shell-escalation (#12649)
## Why This PR switches the `shell_command` zsh-fork path over to `codex-shell-escalation` so the new shell tool can use the shared exec-wrapper/escalation protocol instead of the `zsh_exec_bridge` implementation that was introduced in https://github.com/openai/codex/pull/12052. `zsh_exec_bridge` relied on UNIX domain sockets, which is not as tamper-proof as the FD-based approach in `codex-shell-escalation`. ## What Changed - Added a Unix zsh-fork runtime adapter in `core` (`core/src/tools/runtimes/shell/unix_escalation.rs`) that: - runs zsh-fork commands through `codex_shell_escalation::run_escalate_server` - bridges exec-policy / approval decisions into `ShellActionProvider` - executes escalated commands via a `ShellCommandExecutor` that calls `process_exec_tool_call` - Updated `ShellRuntime` / `ShellCommandHandler` / tool spec wiring to select a `shell_command` backend (`classic` vs `zsh-fork`) while leaving the generic `shell` tool path unchanged. - Removed the `zsh_exec_bridge`-based session service and deleted `core/src/zsh_exec_bridge/mod.rs`. - Moved exec-wrapper entrypoint dispatch to `arg0` by handling the `codex-execve-wrapper` arg0 alias there, and removed the old `codex_core::maybe_run_zsh_exec_wrapper_mode()` hooks from `cli` and `app-server` mains. - Added the needed `codex-shell-escalation` dependencies for `core` and `arg0`. ## Tests - `cargo test -p codex-core shell_zsh_fork_prefers_shell_command_over_unified_exec` - `cargo test -p codex-app-server turn_start_shell_zsh_fork -- --nocapture` - verifies zsh-fork command execution and approval flows through the new backend - includes subcommand approve/decline coverage using the shared zsh DotSlash fixture in `app-server/tests/suite/zsh` - To test manually, I added the following to `~/.codex/config.toml`: ```toml zsh_path = "/Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh" [features] shell_zsh_fork = true ``` Then I ran `just c` to run the dev build of Codex with these changes and sent it the message: ``` run `echo $0` ``` And it replied with: ``` echo $0 printed: /Users/mbolin/code/codex3/codex-rs/app-server/tests/suite/zsh In this tool context, $0 reflects the script path used to invoke the shell, not just zsh. ``` so the tool appears to be wired up correctly. ## Notes - The zsh subcommand-decline integration test now uses `rm` under a `WorkspaceWrite` sandbox. The previous `/usr/bin/true` scenario is auto-allowed by the new `shell-escalation` policy path, which no longer produces subcommand approval prompts.
Michael Bolin ·
2026-02-24 10:31:08 -08:00 -
refactor: decouple shell-escalation from codex-core (#12638)
## Why After removing `exec-server`, the next step is to wire a new shell tool to `codex-rs/shell-escalation` directly. That is blocked while `codex-shell-escalation` depends on `codex-core`, because the new integration would require `codex-core` to depend on `codex-shell-escalation` and create a dependency cycle. This change ports the reusable pieces from the earlier prep work, but drops the old compatibility shim because `exec-server`/MCP support is already gone. ## What Changed ### Decouple `shell-escalation` from `codex-core` - Introduce a crate-local `SandboxState` in `shell-escalation` - Introduce a `ShellCommandExecutor` trait so callers provide process execution/sandbox integration - Update `EscalateServer::exec(...)` and `run_escalate_server(...)` to use the injected executor - Remove the direct `codex_core::exec::process_exec_tool_call(...)` call from `shell-escalation` - Remove the `codex-core` dependency from `codex-shell-escalation` ### Restore reusable policy adapter exports - Re-enable `unix::core_shell_escalation` - Export `ShellActionProvider` and `ShellPolicyFactory` from `shell-escalation` - Keep the crate root API simple (no `legacy_api` compatibility layer) ### Port socket fixes from the earlier prep commit - Use `socket2::Socket::pair_raw(...)` for AF_UNIX socketpairs and restore `CLOEXEC` explicitly on both endpoints - Keep `CLOEXEC` cleared only on the single datagram client FD that is intentionally passed across `exec` - Clean up `tokio::AsyncFd::try_io(...)` error handling in the socket helpers ## Verification - `cargo shear` - `cargo clippy -p codex-shell-escalation --tests` - `cargo test -p codex-shell-escalation`
Michael Bolin ·
2026-02-23 20:58:24 -08:00 -
refactor: delete exec-server and move execve wrapper into shell-escalation (#12632)
## Why We already plan to remove the shell-tool MCP path, and doing that cleanup first makes the follow-on `shell-escalation` work much simpler. This change removes the last remaining reason to keep `codex-rs/exec-server` around by moving the `codex-execve-wrapper` binary and shared shell test fixtures to the crates/tests that now own that functionality. ## What Changed ### Delete `codex-rs/exec-server` - Remove the `exec-server` crate, including the MCP server binary, MCP-specific modules, and its test support/test suite - Remove `exec-server` from the `codex-rs` workspace and update `Cargo.lock` ### Move `codex-execve-wrapper` into `codex-rs/shell-escalation` - Move the wrapper implementation into `shell-escalation` (`src/unix/execve_wrapper.rs`) - Add the `codex-execve-wrapper` binary entrypoint under `shell-escalation/src/bin/` - Update `shell-escalation` exports/module layout so the wrapper entrypoint is hosted there - Move the wrapper README content from `exec-server` to `shell-escalation/README.md` ### Move shared shell test fixtures to `app-server` - Move the DotSlash `bash`/`zsh` test fixtures from `exec-server/tests/suite/` to `app-server/tests/suite/` - Update `app-server` zsh-fork tests to reference the new fixture paths ### Keep `shell-tool-mcp` as a shell-assets package - Update `.github/workflows/shell-tool-mcp.yml` packaging so the npm artifact contains only patched Bash/Zsh payloads (no Rust binaries) - Update `shell-tool-mcp/package.json`, `shell-tool-mcp/src/index.ts`, and docs to reflect the shell-assets-only package shape - `shell-tool-mcp-ci.yml` does not need changes because it is already JS-only ## Verification - `cargo shear` - `cargo clippy -p codex-shell-escalation --tests` - `just clippy`
Michael Bolin ·
2026-02-23 20:10:22 -08:00 -
refactor: normalize unix module layout for exec-server and shell-escalation (#12556)
## Why Shell execution refactoring in `exec-server` had become split between duplicated code paths, which blocked a clean introduction of the new reusable shell escalation flow. This commit creates a dedicated foundation crate so later shell tooling changes can share one implementation. ## What changed - Added the `codex-shell-escalation` crate and moved the core escalation pieces (`mcp` protocol/socket/session flow, policy glue) that were previously in `exec-server` into it. - Normalized `exec-server` Unix structure under a dedicated `unix` module layout and kept non-Unix builds narrow. - Wired crate/build metadata so `shell-escalation` is a first-class workspace dependency for follow-on integration work. ## Verification - Built and linted the stack at this commit point with `just clippy`. [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/12556). * #12584 * #12583 * __->__ #12556
Michael Bolin ·
2026-02-23 09:28:17 -08:00