From 5746561428cef1869fca906a06c2091435dffb9d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 20 May 2025 11:55:25 -0700 Subject: [PATCH 1/5] chore: move types out of config.rs into config_types.rs (#1054) `config.rs` is already quite long without these definitions. Since they have no real dependencies of their own, let's move them to their own file so `config.rs` can focus on the business logic of loading a config. --- codex-rs/core/src/config.rs | 76 ++---------------- codex-rs/core/src/config_types.rs | 88 +++++++++++++++++++++ codex-rs/core/src/lib.rs | 2 +- codex-rs/core/src/mcp_connection_manager.rs | 2 +- codex-rs/core/src/mcp_server_config.rs | 14 ---- codex-rs/core/src/message_history.rs | 2 +- codex-rs/tui/src/markdown.rs | 2 +- 7 files changed, 98 insertions(+), 88 deletions(-) create mode 100644 codex-rs/core/src/config_types.rs delete mode 100644 codex-rs/core/src/mcp_server_config.rs diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index fd6356ab5f..de97b36e88 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,6 +1,9 @@ use crate::config_profile::ConfigProfile; +use crate::config_types::History; +use crate::config_types::McpServerConfig; +use crate::config_types::Tui; +use crate::config_types::UriBasedFileOpener; use crate::flags::OPENAI_DEFAULT_MODEL; -use crate::mcp_server_config::McpServerConfig; use crate::model_provider_info::ModelProviderInfo; use crate::model_provider_info::built_in_model_providers; use crate::protocol::AskForApproval; @@ -93,75 +96,6 @@ pub struct Config { pub tui: Tui, } -/// Settings that govern if and what will be written to `~/.codex/history.jsonl`. -#[derive(Deserialize, Debug, Clone, PartialEq, Default)] -pub struct History { - /// If true, history entries will not be written to disk. - pub persistence: HistoryPersistence, - - /// If set, the maximum size of the history file in bytes. - /// TODO(mbolin): Not currently honored. - pub max_bytes: Option, -} - -#[derive(Deserialize, Debug, Copy, Clone, PartialEq, Default)] -#[serde(rename_all = "kebab-case")] -pub enum HistoryPersistence { - /// Save all history entries to disk. - #[default] - SaveAll, - /// Do not write history to disk. - None, -} - -/// Collection of settings that are specific to the TUI. -#[derive(Deserialize, Debug, Clone, PartialEq, Default)] -pub struct Tui { - /// By default, mouse capture is enabled in the TUI so that it is possible - /// to scroll the conversation history with a mouse. This comes at the cost - /// of not being able to use the mouse to select text in the TUI. - /// (Most terminals support a modifier key to allow this. For example, - /// text selection works in iTerm if you hold down the `Option` key while - /// clicking and dragging.) - /// - /// Setting this option to `true` disables mouse capture, so scrolling with - /// the mouse is not possible, though the keyboard shortcuts e.g. `b` and - /// `space` still work. This allows the user to select text in the TUI - /// using the mouse without needing to hold down a modifier key. - pub disable_mouse_capture: bool, -} - -#[derive(Deserialize, Debug, Copy, Clone, PartialEq)] -pub enum UriBasedFileOpener { - #[serde(rename = "vscode")] - VsCode, - - #[serde(rename = "vscode-insiders")] - VsCodeInsiders, - - #[serde(rename = "windsurf")] - Windsurf, - - #[serde(rename = "cursor")] - Cursor, - - /// Option to disable the URI-based file opener. - #[serde(rename = "none")] - None, -} - -impl UriBasedFileOpener { - pub fn get_scheme(&self) -> Option<&str> { - match self { - UriBasedFileOpener::VsCode => Some("vscode"), - UriBasedFileOpener::VsCodeInsiders => Some("vscode-insiders"), - UriBasedFileOpener::Windsurf => Some("windsurf"), - UriBasedFileOpener::Cursor => Some("cursor"), - UriBasedFileOpener::None => None, - } - } -} - /// Base config deserialized from ~/.codex/config.toml. #[derive(Deserialize, Debug, Clone, Default)] pub struct ConfigToml { @@ -523,6 +457,8 @@ pub fn parse_sandbox_permission_with_base_path( #[cfg(test)] mod tests { #![allow(clippy::expect_used, clippy::unwrap_used)] + use crate::config_types::HistoryPersistence; + use super::*; use pretty_assertions::assert_eq; use tempfile::TempDir; diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs new file mode 100644 index 0000000000..22c3e8565f --- /dev/null +++ b/codex-rs/core/src/config_types.rs @@ -0,0 +1,88 @@ +//! Types used to define the fields of [`crate::config::Config`]. + +// Note this file should generally be restricted to simple struct/enum +// definitions that do not contain business logic. + +use std::collections::HashMap; + +use serde::Deserialize; + +#[derive(Deserialize, Debug, Clone, PartialEq)] +pub struct McpServerConfig { + pub command: String, + + #[serde(default)] + pub args: Vec, + + #[serde(default)] + pub env: Option>, +} + +#[derive(Deserialize, Debug, Copy, Clone, PartialEq)] +pub enum UriBasedFileOpener { + #[serde(rename = "vscode")] + VsCode, + + #[serde(rename = "vscode-insiders")] + VsCodeInsiders, + + #[serde(rename = "windsurf")] + Windsurf, + + #[serde(rename = "cursor")] + Cursor, + + /// Option to disable the URI-based file opener. + #[serde(rename = "none")] + None, +} + +impl UriBasedFileOpener { + pub fn get_scheme(&self) -> Option<&str> { + match self { + UriBasedFileOpener::VsCode => Some("vscode"), + UriBasedFileOpener::VsCodeInsiders => Some("vscode-insiders"), + UriBasedFileOpener::Windsurf => Some("windsurf"), + UriBasedFileOpener::Cursor => Some("cursor"), + UriBasedFileOpener::None => None, + } + } +} + +/// Settings that govern if and what will be written to `~/.codex/history.jsonl`. +#[derive(Deserialize, Debug, Clone, PartialEq, Default)] +pub struct History { + /// If true, history entries will not be written to disk. + pub persistence: HistoryPersistence, + + /// If set, the maximum size of the history file in bytes. + /// TODO(mbolin): Not currently honored. + pub max_bytes: Option, +} + +#[derive(Deserialize, Debug, Copy, Clone, PartialEq, Default)] +#[serde(rename_all = "kebab-case")] +pub enum HistoryPersistence { + /// Save all history entries to disk. + #[default] + SaveAll, + /// Do not write history to disk. + None, +} + +/// Collection of settings that are specific to the TUI. +#[derive(Deserialize, Debug, Clone, PartialEq, Default)] +pub struct Tui { + /// By default, mouse capture is enabled in the TUI so that it is possible + /// to scroll the conversation history with a mouse. This comes at the cost + /// of not being able to use the mouse to select text in the TUI. + /// (Most terminals support a modifier key to allow this. For example, + /// text selection works in iTerm if you hold down the `Option` key while + /// clicking and dragging.) + /// + /// Setting this option to `true` disables mouse capture, so scrolling with + /// the mouse is not possible, though the keyboard shortcuts e.g. `b` and + /// `space` still work. This allows the user to select text in the TUI + /// using the mouse without needing to hold down a modifier key. + pub disable_mouse_capture: bool, +} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 00a65a6725..759f10291b 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -13,6 +13,7 @@ pub use codex::Codex; pub mod codex_wrapper; pub mod config; pub mod config_profile; +pub mod config_types; mod conversation_history; pub mod error; pub mod exec; @@ -22,7 +23,6 @@ mod is_safe_command; #[cfg(target_os = "linux")] pub mod landlock; mod mcp_connection_manager; -pub mod mcp_server_config; mod mcp_tool_call; mod message_history; mod model_provider_info; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 714c9452ff..6ae1865f16 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -19,7 +19,7 @@ use mcp_types::Tool; use tokio::task::JoinSet; use tracing::info; -use crate::mcp_server_config::McpServerConfig; +use crate::config_types::McpServerConfig; /// Delimiter used to separate the server name from the tool name in a fully /// qualified tool name. diff --git a/codex-rs/core/src/mcp_server_config.rs b/codex-rs/core/src/mcp_server_config.rs deleted file mode 100644 index 30845431fa..0000000000 --- a/codex-rs/core/src/mcp_server_config.rs +++ /dev/null @@ -1,14 +0,0 @@ -use std::collections::HashMap; - -use serde::Deserialize; - -#[derive(Deserialize, Debug, Clone, PartialEq)] -pub struct McpServerConfig { - pub command: String, - - #[serde(default)] - pub args: Vec, - - #[serde(default)] - pub env: Option>, -} diff --git a/codex-rs/core/src/message_history.rs b/codex-rs/core/src/message_history.rs index 6c201dfd43..29970c2a74 100644 --- a/codex-rs/core/src/message_history.rs +++ b/codex-rs/core/src/message_history.rs @@ -28,7 +28,7 @@ use tokio::io::AsyncReadExt; use uuid::Uuid; use crate::config::Config; -use crate::config::HistoryPersistence; +use crate::config_types::HistoryPersistence; #[cfg(unix)] use std::os::unix::fs::OpenOptionsExt; diff --git a/codex-rs/tui/src/markdown.rs b/codex-rs/tui/src/markdown.rs index 118eaa59b6..a56ce7749e 100644 --- a/codex-rs/tui/src/markdown.rs +++ b/codex-rs/tui/src/markdown.rs @@ -1,5 +1,5 @@ use codex_core::config::Config; -use codex_core::config::UriBasedFileOpener; +use codex_core::config_types::UriBasedFileOpener; use ratatui::text::Line; use ratatui::text::Span; use std::borrow::Cow; From ef7208359f20f20a38781b14c9c13b31c20c67f4 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 21 May 2025 22:53:02 -0700 Subject: [PATCH 2/5] feat: show Config overview at start of exec (#1073) Now the `exec` output starts with something like: ``` -------- workdir: /Users/mbolin/code/codex/codex-rs model: o3 provider: openai approval: Never sandbox: SandboxPolicy { permissions: [DiskFullReadAccess, DiskWritePlatformUserTempFolder, DiskWritePlatformGlobalTempFolder, DiskWriteCwd, DiskWriteFolder { folder: "/Users/mbolin/.pyenv/shims" }] } -------- ``` which makes it easier to reason about when looking at logs. --- codex-rs/exec/src/event_processor.rs | 52 +++++++++++++++++++++------- codex-rs/exec/src/lib.rs | 3 ++ 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index 65f2204dae..676b47d64f 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -1,5 +1,6 @@ use chrono::Utc; use codex_common::elapsed::format_elapsed; +use codex_core::config::Config; use codex_core::protocol::AgentMessageEvent; use codex_core::protocol::BackgroundEventEvent; use codex_core::protocol::ErrorEvent; @@ -13,7 +14,6 @@ use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::PatchApplyEndEvent; use codex_core::protocol::SessionConfiguredEvent; -use codex_core::protocol::TaskCompleteEvent; use owo_colors::OwoColorize; use owo_colors::Style; use shlex::try_join; @@ -103,9 +103,36 @@ macro_rules! ts_println { }}; } +/// Print a concise summary of the effective configuration that will be used +/// for the session. This mirrors the information shown in the TUI welcome +/// screen. +pub(crate) fn print_config_summary(config: &Config, with_ansi: bool) { + let bold = if with_ansi { + Style::new().bold() + } else { + Style::new() + }; + + ts_println!("OpenAI Codex (research preview)\n--------"); + + let entries = vec![ + ("workdir", config.cwd.display().to_string()), + ("model", config.model.clone()), + ("provider", config.model_provider_id.clone()), + ("approval", format!("{:?}", config.approval_policy)), + ("sandbox", format!("{:?}", config.sandbox_policy)), + ]; + + for (key, value) in entries { + println!("{} {}", format!("{key}: ").style(bold), value); + } + + println!("--------\n"); +} + impl EventProcessor { pub(crate) fn process_event(&mut self, event: Event) { - let Event { id, msg } = event; + let Event { id: _, msg } = event; match msg { EventMsg::Error(ErrorEvent { message }) => { let prefix = "ERROR:".style(self.red); @@ -114,15 +141,8 @@ impl EventProcessor { EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => { ts_println!("{}", message.style(self.dimmed)); } - EventMsg::TaskStarted => { - let msg = format!("Task started: {id}"); - ts_println!("{}", msg.style(self.dimmed)); - } - EventMsg::TaskComplete(TaskCompleteEvent { - last_agent_message: _, - }) => { - let msg = format!("Task complete: {id}"); - ts_println!("{}", msg.style(self.bold)); + EventMsg::TaskStarted | EventMsg::TaskComplete(_) => { + // Ignore. } EventMsg::AgentMessage(AgentMessageEvent { message }) => { let prefix = "Agent message:".style(self.bold); @@ -385,7 +405,15 @@ impl EventProcessor { history_log_id: _, history_entry_count: _, } = session_configured_event; - println!("session {session_id} with model {model}"); + + ts_println!( + "{} {}", + "codex session".style(self.magenta).style(self.bold), + session_id.to_string().style(self.dimmed) + ); + + ts_println!("model: {}", model); + println!(); } EventMsg::GetHistoryEntryResponse(_) => { // Currently ignored in exec output. diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index d405a2d2e2..e615de79a9 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -18,6 +18,7 @@ use codex_core::protocol::SandboxPolicy; use codex_core::protocol::TaskCompleteEvent; use codex_core::util::is_inside_git_repo; use event_processor::EventProcessor; +use event_processor::print_config_summary; use tracing::debug; use tracing::error; use tracing::info; @@ -70,6 +71,8 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { model_provider: None, }; let config = Config::load_with_overrides(overrides)?; + // Print the effective configuration so users can see what Codex is using. + print_config_summary(&config, stdout_with_ansi); if !skip_git_repo_check && !is_inside_git_repo(&config) { eprintln!("Not inside a Git repo and --skip-git-repo-check was not specified."); From cb379d779744aada3e44e861bb9a653e22e5917a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 22 May 2025 09:51:19 -0700 Subject: [PATCH 3/5] feat: introduce support for shell_environment_policy in config.toml (#1061) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To date, when handling `shell` and `local_shell` tool calls, we were spawning new processes using the environment inherited from the Codex process itself. This means that the sensitive `OPENAI_API_KEY` that Codex needs to talk to OpenAI models was made available to everything run by `shell` and `local_shell`. While there are cases where that might be useful, it does not seem like a good default. This PR introduces a complex `shell_environment_policy` config option to control the `env` used with these tool calls. It is inevitably a bit complex so that it is possible to override individual components of the policy so without having to restate the entire thing. Details are in the updated `README.md` in this PR, but here is the relevant bit that explains the individual fields of `shell_environment_policy`: | Field | Type | Default | Description | | ------------------------- | -------------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | | `inherit` | string | `core` | Starting template for the environment:
`core` (`HOME`, `PATH`, `USER`, …), `all` (clone full parent env), or `none` (start empty). | | `ignore_default_excludes` | boolean | `false` | When `false`, Codex removes any var whose **name** contains `KEY`, `SECRET`, or `TOKEN` (case-insensitive) before other rules run. | | `exclude` | array<string> | `[]` | Case-insensitive glob patterns to drop after the default filter.
Examples: `"AWS_*"`, `"AZURE_*"`. | | `set` | table<string,string> | `{}` | Explicit key/value overrides or additions – always win over inherited values. | | `include_only` | array<string> | `[]` | If non-empty, a whitelist of patterns; only variables that match _one_ pattern survive the final step. (Generally used with `inherit = "all"`.) | In particular, note that the default is `inherit = "core"`, so: * if you have extra env variables that you want to inherit from the parent process, use `inherit = "all"` and then specify `include_only` * if you have extra env variables where you want to hardcode the values, the default `inherit = "core"` will work fine, but then you need to specify `set` This configuration is not battle-tested, so we will probably still have to play with it a bit. `core/src/exec_env.rs` has the critical business logic as well as unit tests. Though if nothing else, previous to this change: ``` $ cargo run --bin codex -- debug seatbelt -- printenv OPENAI_API_KEY # ...prints OPENAI_API_KEY... ``` But after this change it does not print anything (as desired). One final thing to call out about this PR is that the `configure_command!` macro we use in `core/src/exec.rs` has to do some complex logic with respect to how it builds up the `env` for the process being spawned under Landlock/seccomp. Specifically, doing `cmd.env_clear()` followed by `cmd.envs(&$env_map)` (which is arguably the most intuitive way to do it) caused the Landlock unit tests to fail because the processes spawned by the unit tests started failing in unexpected ways! If we forgo `env_clear()` in favor of updating env vars one at a time, the tests still pass. The comment in the code talks about this a bit, and while I would like to investigate this more, I need to move on for the moment, but I do plan to come back to it to fully understand what is going on. For example, this suggests that we might not be able to spawn a C program that calls `env_clear()`, which would be...weird. We may still have to fiddle with our Landlock config if that is the case. --- .github/workflows/rust-ci.yml | 2 + codex-rs/Cargo.lock | 8 + codex-rs/README.md | 43 ++++++ codex-rs/cli/src/landlock.rs | 8 +- codex-rs/cli/src/linux-sandbox/main.rs | 8 +- codex-rs/cli/src/main.rs | 14 +- codex-rs/cli/src/seatbelt.rs | 19 ++- codex-rs/core/Cargo.toml | 2 + codex-rs/core/src/codex.rs | 5 + codex-rs/core/src/config.rs | 13 ++ codex-rs/core/src/config_types.rs | 89 +++++++++++ codex-rs/core/src/exec.rs | 51 ++++++- codex-rs/core/src/exec_env.rs | 196 +++++++++++++++++++++++++ codex-rs/core/src/exec_linux.rs | 2 + codex-rs/core/src/landlock.rs | 12 +- codex-rs/core/src/lib.rs | 1 + 16 files changed, 455 insertions(+), 18 deletions(-) create mode 100644 codex-rs/core/src/exec_env.rs diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index c1d231f9e3..f0eadaf254 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -100,6 +100,8 @@ jobs: id: test continue-on-error: true run: cargo test --all-features --target ${{ matrix.target }} + env: + RUST_BACKTRACE: 1 # Fail the job if any of the previous steps failed. - name: verify all steps passed diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5358065cd5..6408e8de6f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -526,6 +526,7 @@ dependencies = [ "futures", "landlock", "libc", + "maplit", "mcp-types", "mime_guess", "openssl-sys", @@ -548,6 +549,7 @@ dependencies = [ "tree-sitter", "tree-sitter-bash", "uuid", + "wildmatch", "wiremock", ] @@ -4309,6 +4311,12 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "wildmatch" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68ce1ab1f8c62655ebe1350f589c61e505cf94d385bc6a12899442d9081e71fd" + [[package]] name = "winapi" version = "0.3.9" diff --git a/codex-rs/README.md b/codex-rs/README.md index bedce9f22d..705d313071 100644 --- a/codex-rs/README.md +++ b/codex-rs/README.md @@ -222,6 +222,49 @@ Currently, customers whose accounts are set to use Zero Data Retention (ZDR) mus disable_response_storage = true ``` +### shell_environment_policy + +Codex spawns subprocesses (e.g. when executing a `local_shell` tool-call suggested by the assistant). By default it passes **only a minimal core subset** of your environment to those subprocesses to avoid leaking credentials. You can tune this behavior via the **`shell_environment_policy`** block in +`config.toml`: + +```toml +[shell_environment_policy] +# inherit can be "core" (default), "all", or "none" +inherit = "core" +# set to true to *skip* the filter for `"*KEY*"` and `"*TOKEN*"` +ignore_default_excludes = false +# exclude patterns (case-insensitive globs) +exclude = ["AWS_*", "AZURE_*"] +# force-set / override values +set = { CI = "1" } +# if provided, *only* vars matching these patterns are kept +include_only = ["PATH", "HOME"] +``` + +| Field | Type | Default | Description | +| ------------------------- | -------------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | +| `inherit` | string | `core` | Starting template for the environment:
`core` (`HOME`, `PATH`, `USER`, …), `all` (clone full parent env), or `none` (start empty). | +| `ignore_default_excludes` | boolean | `false` | When `false`, Codex removes any var whose **name** contains `KEY`, `SECRET`, or `TOKEN` (case-insensitive) before other rules run. | +| `exclude` | array<string> | `[]` | Case-insensitive glob patterns to drop after the default filter.
Examples: `"AWS_*"`, `"AZURE_*"`. | +| `set` | table<string,string> | `{}` | Explicit key/value overrides or additions – always win over inherited values. | +| `include_only` | array<string> | `[]` | If non-empty, a whitelist of patterns; only variables that match _one_ pattern survive the final step. (Generally used with `inherit = "all"`.) | + +The patterns are **glob style**, not full regular expressions: `*` matches any +number of characters, `?` matches exactly one, and character classes like +`[A-Z]`/`[^0-9]` are supported. Matching is always **case-insensitive**. This +syntax is documented in code as `EnvironmentVariablePattern` (see +`core/src/config_types.rs`). + +If you just need a clean slate with a few custom entries you can write: + +```toml +[shell_environment_policy] +inherit = "none" +set = { PATH = "/usr/bin", MY_FLAG = "1" } +``` + +Currently, `CODEX_SANDBOX_NETWORK_DISABLED=1` is also added to the environment, assuming network is disabled. This is not configurable. + ### notify Specify a program that will be executed to get notified about events generated by Codex. Note that the program will receive the notification argument as a string of JSON, e.g.: diff --git a/codex-rs/cli/src/landlock.rs b/codex-rs/cli/src/landlock.rs index 998072c5ad..5a65fcbca4 100644 --- a/codex-rs/cli/src/landlock.rs +++ b/codex-rs/cli/src/landlock.rs @@ -3,27 +3,29 @@ //! On Linux the command is executed inside a Landlock + seccomp sandbox by //! calling the low-level `exec_linux` helper from `codex_core::linux`. +use codex_core::config::Config; use codex_core::exec::StdioPolicy; use codex_core::exec::spawn_child_sync; use codex_core::exec_linux::apply_sandbox_policy_to_current_thread; -use codex_core::protocol::SandboxPolicy; use std::process::ExitStatus; use crate::exit_status::handle_exit_status; /// Execute `command` in a Linux sandbox (Landlock + seccomp) the way Codex /// would. -pub fn run_landlock(command: Vec, sandbox_policy: SandboxPolicy) -> anyhow::Result<()> { +pub fn run_landlock(command: Vec, config: &Config) -> anyhow::Result<()> { if command.is_empty() { anyhow::bail!("command args are empty"); } // Spawn a new thread and apply the sandbox policies there. + let env = codex_core::exec_env::create_env(&config.shell_environment_policy); + let sandbox_policy = config.sandbox_policy.clone(); let handle = std::thread::spawn(move || -> anyhow::Result { let cwd = std::env::current_dir()?; apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?; - let mut child = spawn_child_sync(command, cwd, &sandbox_policy, StdioPolicy::Inherit)?; + let mut child = spawn_child_sync(command, cwd, &sandbox_policy, StdioPolicy::Inherit, env)?; let status = child.wait()?; Ok(status) }); diff --git a/codex-rs/cli/src/linux-sandbox/main.rs b/codex-rs/cli/src/linux-sandbox/main.rs index f71f9b863b..3141656595 100644 --- a/codex-rs/cli/src/linux-sandbox/main.rs +++ b/codex-rs/cli/src/linux-sandbox/main.rs @@ -10,6 +10,8 @@ fn main() -> anyhow::Result<()> { use codex_cli::LandlockCommand; use codex_cli::create_sandbox_policy; use codex_cli::landlock; + use codex_core::config::Config; + use codex_core::config::ConfigOverrides; let LandlockCommand { full_auto, @@ -17,6 +19,10 @@ fn main() -> anyhow::Result<()> { command, } = LandlockCommand::parse(); let sandbox_policy = create_sandbox_policy(full_auto, sandbox); - landlock::run_landlock(command, sandbox_policy)?; + let config = Config::load_with_overrides(ConfigOverrides { + sandbox_policy: Some(sandbox_policy), + ..Default::default() + })?; + landlock::run_landlock(command, &config)?; Ok(()) } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index aa0691d81e..b2b1b8cf9a 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -4,6 +4,8 @@ use codex_cli::SeatbeltCommand; use codex_cli::create_sandbox_policy; use codex_cli::proto; use codex_cli::seatbelt; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_exec::Cli as ExecCli; use codex_tui::Cli as TuiCli; @@ -86,7 +88,11 @@ async fn main() -> anyhow::Result<()> { full_auto, }) => { let sandbox_policy = create_sandbox_policy(full_auto, sandbox); - seatbelt::run_seatbelt(command, sandbox_policy).await?; + let config = Config::load_with_overrides(ConfigOverrides { + sandbox_policy: Some(sandbox_policy), + ..Default::default() + })?; + seatbelt::run_seatbelt(command, &config).await?; } #[cfg(unix)] DebugCommand::Landlock(LandlockCommand { @@ -95,7 +101,11 @@ async fn main() -> anyhow::Result<()> { full_auto, }) => { let sandbox_policy = create_sandbox_policy(full_auto, sandbox); - codex_cli::landlock::run_landlock(command, sandbox_policy)?; + let config = Config::load_with_overrides(ConfigOverrides { + sandbox_policy: Some(sandbox_policy), + ..Default::default() + })?; + codex_cli::landlock::run_landlock(command, &config)?; } #[cfg(not(unix))] DebugCommand::Landlock(_) => { diff --git a/codex-rs/cli/src/seatbelt.rs b/codex-rs/cli/src/seatbelt.rs index e40848ca0f..d4a7840420 100644 --- a/codex-rs/cli/src/seatbelt.rs +++ b/codex-rs/cli/src/seatbelt.rs @@ -1,16 +1,21 @@ +use codex_core::config::Config; use codex_core::exec::StdioPolicy; use codex_core::exec::spawn_command_under_seatbelt; -use codex_core::protocol::SandboxPolicy; +use codex_core::exec_env::create_env; use crate::exit_status::handle_exit_status; -pub async fn run_seatbelt( - command: Vec, - sandbox_policy: SandboxPolicy, -) -> anyhow::Result<()> { +pub async fn run_seatbelt(command: Vec, config: &Config) -> anyhow::Result<()> { let cwd = std::env::current_dir()?; - let mut child = - spawn_command_under_seatbelt(command, &sandbox_policy, cwd, StdioPolicy::Inherit).await?; + let env = create_env(&config.shell_environment_policy); + let mut child = spawn_command_under_seatbelt( + command, + &config.sandbox_policy, + cwd, + StdioPolicy::Inherit, + env, + ) + .await?; let status = child.wait().await?; handle_exit_status(status); } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index e2979497d8..2d4ed8f36a 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -46,6 +46,7 @@ tracing = { version = "0.1.41", features = ["log"] } tree-sitter = "0.25.3" tree-sitter-bash = "0.23.3" uuid = { version = "1", features = ["serde", "v4"] } +wildmatch = "2.4.0" [target.'cfg(target_os = "linux")'.dependencies] libc = "0.2.172" @@ -58,6 +59,7 @@ openssl-sys = { version = "*", features = ["vendored"] } [dev-dependencies] assert_cmd = "2" +maplit = "1.0.2" predicates = "3" pretty_assertions = "1.4.1" tempfile = "3" diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0f91472768..69e504781f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -37,6 +37,7 @@ use crate::client::ModelClient; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; use crate::config::Config; +use crate::config_types::ShellEnvironmentPolicy; use crate::conversation_history::ConversationHistory; use crate::error::CodexErr; use crate::error::Result as CodexResult; @@ -45,6 +46,7 @@ use crate::exec::ExecParams; use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; use crate::exec::process_exec_tool_call; +use crate::exec_env::create_env; use crate::flags::OPENAI_STREAM_MAX_RETRIES; use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_connection_manager::try_parse_fully_qualified_tool_name; @@ -171,6 +173,7 @@ pub(crate) struct Session { instructions: Option, approval_policy: AskForApproval, sandbox_policy: SandboxPolicy, + shell_environment_policy: ShellEnvironmentPolicy, writable_roots: Mutex>, /// Manager for external MCP servers/tools. @@ -634,6 +637,7 @@ async fn submission_loop( instructions, approval_policy, sandbox_policy, + shell_environment_policy: config.shell_environment_policy.clone(), cwd, writable_roots, mcp_connection_manager, @@ -1124,6 +1128,7 @@ fn to_exec_params(params: ShellToolCallParams, sess: &Session) -> ExecParams { command: params.command, cwd: sess.resolve_path(params.workdir.clone()), timeout_ms: params.timeout_ms, + env: create_env(&sess.shell_environment_policy), } } diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index de97b36e88..2a3f454342 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,6 +1,8 @@ use crate::config_profile::ConfigProfile; use crate::config_types::History; use crate::config_types::McpServerConfig; +use crate::config_types::ShellEnvironmentPolicy; +use crate::config_types::ShellEnvironmentPolicyToml; use crate::config_types::Tui; use crate::config_types::UriBasedFileOpener; use crate::flags::OPENAI_DEFAULT_MODEL; @@ -37,6 +39,8 @@ pub struct Config { pub sandbox_policy: SandboxPolicy, + pub shell_environment_policy: ShellEnvironmentPolicy, + /// Disable server-side response storage (sends the full conversation /// context with every request). Currently necessary for OpenAI customers /// who have opted into Zero Data Retention (ZDR). @@ -108,6 +112,9 @@ pub struct ConfigToml { /// Default approval policy for executing commands. pub approval_policy: Option, + #[serde(default)] + pub shell_environment_policy: ShellEnvironmentPolicyToml, + // The `default` attribute ensures that the field is treated as `None` when // the key is omitted from the TOML. Without it, Serde treats the field as // required because we supply a custom deserializer. @@ -302,6 +309,8 @@ impl Config { })? .clone(); + let shell_environment_policy = cfg.shell_environment_policy.into(); + let resolved_cwd = { use std::env; @@ -336,6 +345,7 @@ impl Config { .or(cfg.approval_policy) .unwrap_or_else(AskForApproval::default), sandbox_policy, + shell_environment_policy, disable_response_storage: disable_response_storage .or(config_profile.disable_response_storage) .or(cfg.disable_response_storage) @@ -677,6 +687,7 @@ disable_response_storage = true model_provider: fixture.openai_provider.clone(), approval_policy: AskForApproval::Never, sandbox_policy: SandboxPolicy::new_read_only_policy(), + shell_environment_policy: ShellEnvironmentPolicy::default(), disable_response_storage: false, instructions: None, notify: None, @@ -714,6 +725,7 @@ disable_response_storage = true model_provider: fixture.openai_chat_completions_provider.clone(), approval_policy: AskForApproval::UnlessAllowListed, sandbox_policy: SandboxPolicy::new_read_only_policy(), + shell_environment_policy: ShellEnvironmentPolicy::default(), disable_response_storage: false, instructions: None, notify: None, @@ -766,6 +778,7 @@ disable_response_storage = true model_provider: fixture.openai_provider.clone(), approval_policy: AskForApproval::OnFailure, sandbox_policy: SandboxPolicy::new_read_only_policy(), + shell_environment_policy: ShellEnvironmentPolicy::default(), disable_response_storage: true, instructions: None, notify: None, diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs index 22c3e8565f..6696f76f0b 100644 --- a/codex-rs/core/src/config_types.rs +++ b/codex-rs/core/src/config_types.rs @@ -4,6 +4,7 @@ // definitions that do not contain business logic. use std::collections::HashMap; +use wildmatch::WildMatchPattern; use serde::Deserialize; @@ -86,3 +87,91 @@ pub struct Tui { /// using the mouse without needing to hold down a modifier key. pub disable_mouse_capture: bool, } + +#[derive(Deserialize, Debug, Clone, PartialEq, Default)] + +pub enum ShellEnvironmentPolicyInherit { + /// "Core" environment variables for the platform. On UNIX, this would + /// include HOME, LOGNAME, PATH, SHELL, and USER, among others. + #[default] + Core, + + /// Inherits the full environment from the parent process. + All, + + /// Do not inherit any environment variables from the parent process. + None, +} + +/// Policy for building the `env` when spawning a process via either the +/// `shell` or `local_shell` tool. +#[derive(Deserialize, Debug, Clone, PartialEq, Default)] +pub struct ShellEnvironmentPolicyToml { + pub inherit: Option, + + pub ignore_default_excludes: Option, + + /// List of regular expressions. + pub exclude: Option>, + + pub r#set: Option>, + + /// List of regular expressions. + pub include_only: Option>, +} + +pub type EnvironmentVariablePattern = WildMatchPattern<'*', '?'>; + +/// Deriving the `env` based on this policy works as follows: +/// 1. Create an initial map based on the `inherit` policy. +/// 2. If `ignore_default_excludes` is false, filter the map using the default +/// exclude pattern(s), which are: `"*KEY*"` and `"*TOKEN*"`. +/// 3. If `exclude` is not empty, filter the map using the provided patterns. +/// 4. Insert any entries from `r#set` into the map. +/// 5. If non-empty, filter the map using the `include_only` patterns. +#[derive(Debug, Clone, PartialEq, Default)] +pub struct ShellEnvironmentPolicy { + /// Starting point when building the environment. + pub inherit: ShellEnvironmentPolicyInherit, + + /// True to skip the check to exclude default environment variables that + /// contain "KEY" or "TOKEN" in their name. + pub ignore_default_excludes: bool, + + /// Environment variable names to exclude from the environment. + pub exclude: Vec, + + /// (key, value) pairs to insert in the environment. + pub r#set: HashMap, + + /// Environment variable names to retain in the environment. + pub include_only: Vec, +} + +impl From for ShellEnvironmentPolicy { + fn from(toml: ShellEnvironmentPolicyToml) -> Self { + let inherit = toml.inherit.unwrap_or(ShellEnvironmentPolicyInherit::Core); + let ignore_default_excludes = toml.ignore_default_excludes.unwrap_or(false); + let exclude = toml + .exclude + .unwrap_or_default() + .into_iter() + .map(|s| EnvironmentVariablePattern::new_case_insensitive(&s)) + .collect(); + let r#set = toml.r#set.unwrap_or_default(); + let include_only = toml + .include_only + .unwrap_or_default() + .into_iter() + .map(|s| EnvironmentVariablePattern::new_case_insensitive(&s)) + .collect(); + + Self { + inherit, + ignore_default_excludes, + exclude, + r#set, + include_only, + } + } +} diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 158a0da9b4..96b601b613 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -1,6 +1,7 @@ #[cfg(unix)] use std::os::unix::process::ExitStatusExt; +use std::collections::HashMap; use std::io; use std::path::Path; use std::path::PathBuf; @@ -59,6 +60,7 @@ pub struct ExecParams { pub command: Vec, pub cwd: PathBuf, pub timeout_ms: Option, + pub env: HashMap, } #[derive(Clone, Copy, Debug, PartialEq)] @@ -87,12 +89,14 @@ pub async fn process_exec_tool_call( command, cwd, timeout_ms, + env, } = params; let child = spawn_command_under_seatbelt( command, sandbox_policy, cwd, StdioPolicy::RedirectForShellTool, + env, ) .await?; consume_truncated_output(child, ctrl_c, timeout_ms).await @@ -145,9 +149,10 @@ pub async fn spawn_command_under_seatbelt( sandbox_policy: &SandboxPolicy, cwd: PathBuf, stdio_policy: StdioPolicy, + env: HashMap, ) -> std::io::Result { let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &cwd); - spawn_child_async(seatbelt_command, cwd, sandbox_policy, stdio_policy).await + spawn_child_async(seatbelt_command, cwd, sandbox_policy, stdio_policy, env).await } fn create_seatbelt_command( @@ -233,6 +238,7 @@ async fn exec( command, cwd, timeout_ms, + env, }: ExecParams, sandbox_policy: &SandboxPolicy, ctrl_c: Arc, @@ -242,6 +248,7 @@ async fn exec( cwd, sandbox_policy, StdioPolicy::RedirectForShellTool, + env, ) .await?; consume_truncated_output(child, ctrl_c, timeout_ms).await @@ -259,7 +266,8 @@ macro_rules! configure_command { $command: expr, $cwd: expr, $sandbox_policy: expr, - $stdio_policy: expr + $stdio_policy: expr, + $env_map: expr ) => {{ // For now, we take `SandboxPolicy` as a parameter to spawn_child() because // we need to determine whether to set the @@ -279,6 +287,38 @@ macro_rules! configure_command { cmd.args(&$command[1..]); cmd.current_dir($cwd); + // Previously, to update the env for `cmd`, we did the straightforward + // thing of calling `env_clear()` followed by `envs(&env_map)` so + // that the spawned process inherited *only* the variables explicitly + // provided by the caller. On Linux, the combination of `env_clear()` + // and Landlock/seccomp caused a permission error whereas this more + // "surgical" approach of setting variables individually appears to + // work fine. More time with `strace` and friends is merited to fully + // debug thus, though we will soon use a helper binary like we do for + // Seatbelt, which will simplify this logic. + + // Iterate through the current process environment first so we can + // decide, for every variable that already exists, whether we need to + // override its value. + let mut remaining_overrides = $env_map.clone(); + for (key, current_val) in std::env::vars() { + if let Some(desired_val) = remaining_overrides.remove(&key) { + // The caller provided a value for this variable. Override it + // only if the value differs from what is currently set. + if desired_val != current_val { + cmd.env(&key, desired_val); + } + } + // If the variable was not in `env_map`, we leave it unchanged. + } + + // Any entries still left in `remaining_overrides` were not present in + // the parent environment. Add them now so that the child process sees + // the complete set requested by the caller. + for (key, val) in remaining_overrides { + cmd.env(key, val); + } + if !$sandbox_policy.has_full_network_access() { cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); } @@ -313,8 +353,9 @@ pub(crate) async fn spawn_child_async( cwd: PathBuf, sandbox_policy: &SandboxPolicy, stdio_policy: StdioPolicy, + env: HashMap, ) -> std::io::Result { - let mut cmd = configure_command!(Command, command, cwd, sandbox_policy, stdio_policy)?; + let mut cmd = configure_command!(Command, command, cwd, sandbox_policy, stdio_policy, env)?; cmd.kill_on_drop(true).spawn() } @@ -326,13 +367,15 @@ pub fn spawn_child_sync( cwd: PathBuf, sandbox_policy: &SandboxPolicy, stdio_policy: StdioPolicy, + env: HashMap, ) -> std::io::Result { let mut cmd = configure_command!( std::process::Command, command, cwd, sandbox_policy, - stdio_policy + stdio_policy, + env )?; cmd.spawn() } diff --git a/codex-rs/core/src/exec_env.rs b/codex-rs/core/src/exec_env.rs new file mode 100644 index 0000000000..2957f3da15 --- /dev/null +++ b/codex-rs/core/src/exec_env.rs @@ -0,0 +1,196 @@ +use crate::config_types::EnvironmentVariablePattern; +use crate::config_types::ShellEnvironmentPolicy; +use crate::config_types::ShellEnvironmentPolicyInherit; +use std::collections::HashMap; +use std::collections::HashSet; + +/// Construct an environment map based on the rules in the specified policy. The +/// resulting map can be passed directly to `Command::envs()` after calling +/// `env_clear()` to ensure no unintended variables are leaked to the spawned +/// process. +/// +/// The derivation follows the algorithm documented in the struct-level comment +/// for [`ShellEnvironmentPolicy`]. +pub fn create_env(policy: &ShellEnvironmentPolicy) -> HashMap { + populate_env(std::env::vars(), policy) +} + +fn populate_env(vars: I, policy: &ShellEnvironmentPolicy) -> HashMap +where + I: IntoIterator, +{ + // Step 1 – determine the starting set of variables based on the + // `inherit` strategy. + let mut env_map: HashMap = match policy.inherit { + ShellEnvironmentPolicyInherit::All => vars.into_iter().collect(), + ShellEnvironmentPolicyInherit::None => HashMap::new(), + ShellEnvironmentPolicyInherit::Core => { + const CORE_VARS: &[&str] = &[ + "HOME", "LOGNAME", "PATH", "SHELL", "USER", "USERNAME", "TMPDIR", "TEMP", "TMP", + ]; + let allow: HashSet<&str> = CORE_VARS.iter().copied().collect(); + vars.into_iter() + .filter(|(k, _)| allow.contains(k.as_str())) + .collect() + } + }; + + // Internal helper – does `name` match **any** pattern in `patterns`? + let matches_any = |name: &str, patterns: &[EnvironmentVariablePattern]| -> bool { + patterns.iter().any(|pattern| pattern.matches(name)) + }; + + // Step 2 – Apply the default exclude if not disabled. + if !policy.ignore_default_excludes { + let default_excludes = vec![ + EnvironmentVariablePattern::new_case_insensitive("*KEY*"), + EnvironmentVariablePattern::new_case_insensitive("*SECRET*"), + EnvironmentVariablePattern::new_case_insensitive("*TOKEN*"), + ]; + env_map.retain(|k, _| !matches_any(k, &default_excludes)); + } + + // Step 3 – Apply custom excludes. + if !policy.exclude.is_empty() { + env_map.retain(|k, _| !matches_any(k, &policy.exclude)); + } + + // Step 4 – Apply user-provided overrides. + for (key, val) in &policy.r#set { + env_map.insert(key.clone(), val.clone()); + } + + // Step 5 – If include_only is non-empty, keep *only* the matching vars. + if !policy.include_only.is_empty() { + env_map.retain(|k, _| matches_any(k, &policy.include_only)); + } + + env_map +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used)] + + use super::*; + use crate::config_types::ShellEnvironmentPolicyInherit; + use maplit::hashmap; + + fn make_vars(pairs: &[(&str, &str)]) -> Vec<(String, String)> { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + #[test] + fn test_core_inherit_and_default_excludes() { + let vars = make_vars(&[ + ("PATH", "/usr/bin"), + ("HOME", "/home/user"), + ("API_KEY", "secret"), + ("SECRET_TOKEN", "t"), + ]); + + let policy = ShellEnvironmentPolicy::default(); // inherit Core, default excludes on + let result = populate_env(vars, &policy); + + let expected: HashMap = hashmap! { + "PATH".to_string() => "/usr/bin".to_string(), + "HOME".to_string() => "/home/user".to_string(), + }; + + assert_eq!(result, expected); + } + + #[test] + fn test_include_only() { + let vars = make_vars(&[("PATH", "/usr/bin"), ("FOO", "bar")]); + + let policy = ShellEnvironmentPolicy { + // skip default excludes so nothing is removed prematurely + ignore_default_excludes: true, + include_only: vec![EnvironmentVariablePattern::new_case_insensitive("*PATH")], + ..Default::default() + }; + + let result = populate_env(vars, &policy); + + let expected: HashMap = hashmap! { + "PATH".to_string() => "/usr/bin".to_string(), + }; + + assert_eq!(result, expected); + } + + #[test] + fn test_set_overrides() { + let vars = make_vars(&[("PATH", "/usr/bin")]); + + let mut policy = ShellEnvironmentPolicy { + ignore_default_excludes: true, + ..Default::default() + }; + policy.r#set.insert("NEW_VAR".to_string(), "42".to_string()); + + let result = populate_env(vars, &policy); + + let expected: HashMap = hashmap! { + "PATH".to_string() => "/usr/bin".to_string(), + "NEW_VAR".to_string() => "42".to_string(), + }; + + assert_eq!(result, expected); + } + + #[test] + fn test_inherit_all() { + let vars = make_vars(&[("PATH", "/usr/bin"), ("FOO", "bar")]); + + let policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ignore_default_excludes: true, // keep everything + ..Default::default() + }; + + let result = populate_env(vars.clone(), &policy); + let expected: HashMap = vars.into_iter().collect(); + assert_eq!(result, expected); + } + + #[test] + fn test_inherit_all_with_default_excludes() { + let vars = make_vars(&[("PATH", "/usr/bin"), ("API_KEY", "secret")]); + + let policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ..Default::default() + }; + + let result = populate_env(vars, &policy); + let expected: HashMap = hashmap! { + "PATH".to_string() => "/usr/bin".to_string(), + }; + assert_eq!(result, expected); + } + + #[test] + fn test_inherit_none() { + let vars = make_vars(&[("PATH", "/usr/bin"), ("HOME", "/home")]); + + let mut policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::None, + ignore_default_excludes: true, + ..Default::default() + }; + policy + .r#set + .insert("ONLY_VAR".to_string(), "yes".to_string()); + + let result = populate_env(vars, &policy); + let expected: HashMap = hashmap! { + "ONLY_VAR".to_string() => "yes".to_string(), + }; + assert_eq!(result, expected); + } +} diff --git a/codex-rs/core/src/exec_linux.rs b/codex-rs/core/src/exec_linux.rs index e74c56219c..76bd428a7f 100644 --- a/codex-rs/core/src/exec_linux.rs +++ b/codex-rs/core/src/exec_linux.rs @@ -34,6 +34,7 @@ pub fn exec_linux( command, cwd, timeout_ms, + env, } = params; apply_sandbox_policy_to_current_thread(&sandbox_policy, &cwd)?; let child = spawn_child_async( @@ -41,6 +42,7 @@ pub fn exec_linux( cwd, &sandbox_policy, StdioPolicy::RedirectForShellTool, + env, ) .await?; consume_truncated_output(child, ctrl_c_copy, timeout_ms).await diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 6e9b8de7c6..07c568151a 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -143,20 +143,29 @@ mod tests { #![expect(clippy::unwrap_used, clippy::expect_used)] use super::*; + use crate::config_types::ShellEnvironmentPolicy; use crate::exec::ExecParams; use crate::exec::SandboxType; use crate::exec::process_exec_tool_call; + use crate::exec_env::create_env; use crate::protocol::SandboxPolicy; + use std::collections::HashMap; use std::sync::Arc; use tempfile::NamedTempFile; use tokio::sync::Notify; + fn create_env_from_core_vars() -> HashMap { + let policy = ShellEnvironmentPolicy::default(); + create_env(&policy) + } + #[allow(clippy::print_stdout)] async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { let params = ExecParams { command: cmd.iter().map(|elm| elm.to_string()).collect(), cwd: std::env::current_dir().expect("cwd should exist"), timeout_ms: Some(timeout_ms), + env: create_env_from_core_vars(), }; let sandbox_policy = @@ -236,9 +245,10 @@ mod tests { let params = ExecParams { command: cmd.iter().map(|s| s.to_string()).collect(), cwd: std::env::current_dir().expect("cwd should exist"), - // Give the tool a generous 2‑second timeout so even slow DNS timeouts + // Give the tool a generous 2-second timeout so even slow DNS timeouts // do not stall the suite. timeout_ms: Some(2_000), + env: create_env_from_core_vars(), }; let sandbox_policy = SandboxPolicy::new_read_only_policy(); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 759f10291b..261ae0a0fd 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -17,6 +17,7 @@ pub mod config_types; mod conversation_history; pub mod error; pub mod exec; +pub mod exec_env; pub mod exec_linux; mod flags; mod is_safe_command; From 63deb7c36937030f1a53ccffbc7c9045e9e33e9f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 22 May 2025 13:42:55 -0700 Subject: [PATCH 4/5] fix: for the @native release of the Node module, use the Rust version by default (#1084) Added logic so that when we run `./scripts/stage_release.sh --native` (for the `@native` version of the Node module), we drop a `use-native` file next to `codex.js`. If present, `codex.js` will now run the Rust CLI. Ran `./scripts/stage_release.sh --native` and verified that when the running `codex.js` in the staged folder: ``` $ /var/folders/wm/f209bc1n2bd_r0jncn9s6j_00000gp/T/tmp.efvEvBlSN6/bin/codex.js --version codex-cli 0.0.2505220956 ``` it ran the expected Rust version of the CLI, as desired. While here, I also updated the Rust version to one that I cut today, which includes the new shell environment policy config option: https://github.com/openai/codex/pull/1061. Note this may "break" some users if the processes spawned by Codex need extra environment variables. (We are still working to determine what the right defaults should be for this option.) --- codex-cli/bin/codex.js | 23 ++++++++++++----------- codex-cli/scripts/install_native_deps.sh | 2 +- codex-cli/scripts/stage_release.sh | 8 +++++--- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/codex-cli/bin/codex.js b/codex-cli/bin/codex.js index 818b362700..1bfb9f5d5d 100755 --- a/codex-cli/bin/codex.js +++ b/codex-cli/bin/codex.js @@ -16,14 +16,23 @@ */ import { spawnSync } from "child_process"; +import fs from "fs"; import path from "path"; import { fileURLToPath, pathToFileURL } from "url"; // Determine whether the user explicitly wants the Rust CLI. -const wantsNative = - process.env.CODEX_RUST != null + +// __dirname equivalent in ESM +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +// For the @native release of the Node module, the `use-native` file is added, +// indicating we should default to the native binary. For other releases, +// setting CODEX_RUST=1 will opt-in to the native binary, if included. +const wantsNative = fs.existsSync(path.join(__dirname, "use-native")) || + (process.env.CODEX_RUST != null ? ["1", "true", "yes"].includes(process.env.CODEX_RUST.toLowerCase()) - : false; + : false); // Try native binary if requested. if (wantsNative) { @@ -63,10 +72,6 @@ if (wantsNative) { throw new Error(`Unsupported platform: ${platform} (${arch})`); } - // __dirname equivalent in ESM - const __filename = fileURLToPath(import.meta.url); - const __dirname = path.dirname(__filename); - const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`); const result = spawnSync(binaryPath, process.argv.slice(2), { stdio: "inherit", @@ -78,10 +83,6 @@ if (wantsNative) { // Fallback: execute the original JavaScript CLI. -// Determine this script's directory -const __filename = fileURLToPath(import.meta.url); -const __dirname = path.dirname(__filename); - // Resolve the path to the compiled CLI bundle const cliPath = path.resolve(__dirname, "../dist/cli.js"); const cliUrl = pathToFileURL(cliPath).href; diff --git a/codex-cli/scripts/install_native_deps.sh b/codex-cli/scripts/install_native_deps.sh index 00f355ca7d..5275627f6e 100755 --- a/codex-cli/scripts/install_native_deps.sh +++ b/codex-cli/scripts/install_native_deps.sh @@ -65,7 +65,7 @@ mkdir -p "$BIN_DIR" # Until we start publishing stable GitHub releases, we have to grab the binaries # from the GitHub Action that created them. Update the URL below to point to the # appropriate workflow run: -WORKFLOW_URL="https://github.com/openai/codex/actions/runs/15087655786" +WORKFLOW_URL="https://github.com/openai/codex/actions/runs/15192425904" WORKFLOW_ID="${WORKFLOW_URL##*/}" ARTIFACTS_DIR="$(mktemp -d)" diff --git a/codex-cli/scripts/stage_release.sh b/codex-cli/scripts/stage_release.sh index fb641d35d9..9e251b9059 100755 --- a/codex-cli/scripts/stage_release.sh +++ b/codex-cli/scripts/stage_release.sh @@ -122,6 +122,7 @@ jq --arg version "$VERSION" \ if [[ "$INCLUDE_NATIVE" -eq 1 ]]; then ./scripts/install_native_deps.sh "$TMPDIR" --full-native + touch "${TMPDIR}/bin/use-native" else ./scripts/install_native_deps.sh "$TMPDIR" fi @@ -130,11 +131,12 @@ popd >/dev/null echo "Staged version $VERSION for release in $TMPDIR" -echo "Test Node:" -echo " node ${TMPDIR}/bin/codex.js --help" if [[ "$INCLUDE_NATIVE" -eq 1 ]]; then echo "Test Rust:" - echo " CODEX_RUST=1 node ${TMPDIR}/bin/codex.js --help" + echo " node ${TMPDIR}/bin/codex.js --help" +else + echo "Test Node:" + echo " node ${TMPDIR}/bin/codex.js --help" fi # Print final hint for convenience From d1de7bb383552e8fadd94be79d65d188e00fd562 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Thu, 22 May 2025 21:52:28 -0700 Subject: [PATCH 5/5] feat: add `codex_linux_sandbox_exe: Option` field to Config (#1089) https://github.com/openai/codex/pull/1086 is a work-in-progress to make Linux sandboxing work more like Seatbelt where, for the command we want to sandbox, we build up the command and then hand it, and some sandbox configuration flags, to another command to set up the sandbox and then run it. In the case of Seatbelt, macOS provides this helper binary and provides it at `/usr/bin/sandbox-exec`. For Linux, we have to build our own and pass it through (which is what #1086 does), so this makes the new `codex_linux_sandbox_exe` available on `Config` so that it will later be available in `exec.rs` when we need it in #1086. --- codex-rs/cli/src/main.rs | 14 +++++++++++--- codex-rs/core/src/config.rs | 14 ++++++++++++++ codex-rs/exec/src/lib.rs | 4 +++- codex-rs/exec/src/main.rs | 10 +++++++++- codex-rs/mcp-server/src/codex_tool_config.rs | 6 +++++- codex-rs/mcp-server/src/lib.rs | 5 +++-- codex-rs/mcp-server/src/main.rs | 10 +++++++++- codex-rs/mcp-server/src/message_processor.rs | 11 +++++++++-- codex-rs/tui/src/lib.rs | 4 +++- codex-rs/tui/src/main.rs | 10 +++++++++- 10 files changed, 75 insertions(+), 13 deletions(-) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index b2b1b8cf9a..725a82c255 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use clap::Parser; use codex_cli::LandlockCommand; use codex_cli::SeatbeltCommand; @@ -66,17 +68,23 @@ struct ReplProto {} #[tokio::main] async fn main() -> anyhow::Result<()> { + let codex_linux_sandbox_exe: Option = if cfg!(target_os = "linux") { + std::env::current_exe().ok() + } else { + None + }; + let cli = MultitoolCli::parse(); match cli.subcommand { None => { - codex_tui::run_main(cli.interactive)?; + codex_tui::run_main(cli.interactive, codex_linux_sandbox_exe)?; } Some(Subcommand::Exec(exec_cli)) => { - codex_exec::run_main(exec_cli).await?; + codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?; } Some(Subcommand::Mcp) => { - codex_mcp_server::run_main().await?; + codex_mcp_server::run_main(codex_linux_sandbox_exe).await?; } Some(Subcommand::Proto(proto_cli)) => { proto::run_main(proto_cli).await?; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index 2a3f454342..d643d00660 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -98,6 +98,14 @@ pub struct Config { /// Collection of settings that are specific to the TUI. pub tui: Tui, + + /// Path to the `codex-linux-sandbox` executable. This must be set if + /// [`crate::exec::SandboxType::LinuxSeccomp`] is used. Note that this + /// cannot be set in the config file: it must be set in code via + /// [`ConfigOverrides`]. + /// + /// When this program is invoked, arg0 will be set to `codex-linux-sandbox`. + pub codex_linux_sandbox_exe: Option, } /// Base config deserialized from ~/.codex/config.toml. @@ -222,6 +230,7 @@ pub struct ConfigOverrides { pub disable_response_storage: Option, pub model_provider: Option, pub config_profile: Option, + pub codex_linux_sandbox_exe: Option, } impl Config { @@ -258,6 +267,7 @@ impl Config { disable_response_storage, model_provider, config_profile: config_profile_key, + codex_linux_sandbox_exe, } = overrides; let config_profile = match config_profile_key.or(cfg.profile) { @@ -359,6 +369,7 @@ impl Config { history, file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), tui: cfg.tui.unwrap_or_default(), + codex_linux_sandbox_exe, }; Ok(config) } @@ -699,6 +710,7 @@ disable_response_storage = true history: History::default(), file_opener: UriBasedFileOpener::VsCode, tui: Tui::default(), + codex_linux_sandbox_exe: None, }, o3_profile_config ); @@ -737,6 +749,7 @@ disable_response_storage = true history: History::default(), file_opener: UriBasedFileOpener::VsCode, tui: Tui::default(), + codex_linux_sandbox_exe: None, }; assert_eq!(expected_gpt3_profile_config, gpt3_profile_config); @@ -790,6 +803,7 @@ disable_response_storage = true history: History::default(), file_opener: UriBasedFileOpener::VsCode, tui: Tui::default(), + codex_linux_sandbox_exe: None, }; assert_eq!(expected_zdr_profile_config, zdr_profile_config); diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index e615de79a9..dbf01f025b 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -3,6 +3,7 @@ mod event_processor; use std::io::IsTerminal; use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; pub use cli::Cli; @@ -24,7 +25,7 @@ use tracing::error; use tracing::info; use tracing_subscriber::EnvFilter; -pub async fn run_main(cli: Cli) -> anyhow::Result<()> { +pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> anyhow::Result<()> { let Cli { images, model, @@ -69,6 +70,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { }, cwd: cwd.map(|p| p.canonicalize().unwrap_or(p)), model_provider: None, + codex_linux_sandbox_exe, }; let config = Config::load_with_overrides(overrides)?; // Print the effective configuration so users can see what Codex is using. diff --git a/codex-rs/exec/src/main.rs b/codex-rs/exec/src/main.rs index 3a40da2336..3cb7bd0b66 100644 --- a/codex-rs/exec/src/main.rs +++ b/codex-rs/exec/src/main.rs @@ -1,11 +1,19 @@ +use std::path::PathBuf; + use clap::Parser; use codex_exec::Cli; use codex_exec::run_main; #[tokio::main] async fn main() -> anyhow::Result<()> { + let codex_linux_sandbox_exe: Option = if cfg!(target_os = "linux") { + std::env::current_exe().ok() + } else { + None + }; + let cli = Cli::parse(); - run_main(cli).await?; + run_main(cli, codex_linux_sandbox_exe).await?; Ok(()) } diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 2ddc00fbf9..d04a5c80bc 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -144,7 +144,10 @@ pub(crate) fn create_tool_for_codex_tool_call_param() -> Tool { impl CodexToolCallParam { /// Returns the initial user prompt to start the Codex conversation and the /// Config. - pub fn into_config(self) -> std::io::Result<(String, codex_core::config::Config)> { + pub fn into_config( + self, + codex_linux_sandbox_exe: Option, + ) -> std::io::Result<(String, codex_core::config::Config)> { let Self { prompt, model, @@ -167,6 +170,7 @@ impl CodexToolCallParam { sandbox_policy, disable_response_storage, model_provider: None, + codex_linux_sandbox_exe, }; let cfg = codex_core::config::Config::load_with_overrides(overrides)?; diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index e621f779f0..0f29eb7826 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -2,6 +2,7 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] use std::io::Result as IoResult; +use std::path::PathBuf; use mcp_types::JSONRPCMessage; use tokio::io::AsyncBufReadExt; @@ -24,7 +25,7 @@ use crate::message_processor::MessageProcessor; /// plenty for an interactive CLI. const CHANNEL_CAPACITY: usize = 128; -pub async fn run_main() -> IoResult<()> { +pub async fn run_main(codex_linux_sandbox_exe: Option) -> IoResult<()> { // Install a simple subscriber so `tracing` output is visible. Users can // control the log level with `RUST_LOG`. tracing_subscriber::fmt() @@ -61,7 +62,7 @@ pub async fn run_main() -> IoResult<()> { // Task: process incoming messages. let processor_handle = tokio::spawn({ - let mut processor = MessageProcessor::new(outgoing_tx.clone()); + let mut processor = MessageProcessor::new(outgoing_tx.clone(), codex_linux_sandbox_exe); async move { while let Some(msg) = incoming_rx.recv().await { match msg { diff --git a/codex-rs/mcp-server/src/main.rs b/codex-rs/mcp-server/src/main.rs index baef8587f7..8ce727e920 100644 --- a/codex-rs/mcp-server/src/main.rs +++ b/codex-rs/mcp-server/src/main.rs @@ -1,7 +1,15 @@ +use std::path::PathBuf; + use codex_mcp_server::run_main; #[tokio::main] async fn main() -> std::io::Result<()> { - run_main().await?; + let codex_linux_sandbox_exe: Option = if cfg!(target_os = "linux") { + std::env::current_exe().ok() + } else { + None + }; + + run_main(codex_linux_sandbox_exe).await?; Ok(()) } diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 299523f9c5..bf6f42e569 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use crate::codex_tool_config::CodexToolCallParam; use crate::codex_tool_config::create_tool_for_codex_tool_call_param; @@ -28,15 +30,20 @@ use tokio::task; pub(crate) struct MessageProcessor { outgoing: mpsc::Sender, initialized: bool, + codex_linux_sandbox_exe: Option, } impl MessageProcessor { /// Create a new `MessageProcessor`, retaining a handle to the outgoing /// `Sender` so handlers can enqueue messages to be written to stdout. - pub(crate) fn new(outgoing: mpsc::Sender) -> Self { + pub(crate) fn new( + outgoing: mpsc::Sender, + codex_linux_sandbox_exe: Option, + ) -> Self { Self { outgoing, initialized: false, + codex_linux_sandbox_exe, } } @@ -339,7 +346,7 @@ impl MessageProcessor { let (initial_prompt, config): (String, CodexConfig) = match arguments { Some(json_val) => match serde_json::from_value::(json_val) { - Ok(tool_cfg) => match tool_cfg.into_config() { + Ok(tool_cfg) => match tool_cfg.into_config(self.codex_linux_sandbox_exe.clone()) { Ok(cfg) => cfg, Err(e) => { let result = CallToolResult { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index f4391785f8..4ab68724aa 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -10,6 +10,7 @@ use codex_core::protocol::SandboxPolicy; use codex_core::util::is_inside_git_repo; use log_layer::TuiLogLayer; use std::fs::OpenOptions; +use std::path::PathBuf; use tracing_appender::non_blocking; use tracing_subscriber::EnvFilter; use tracing_subscriber::prelude::*; @@ -36,7 +37,7 @@ mod user_approval_widget; pub use cli::Cli; -pub fn run_main(cli: Cli) -> std::io::Result<()> { +pub fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> std::io::Result<()> { let (sandbox_policy, approval_policy) = if cli.full_auto { ( Some(SandboxPolicy::new_full_auto_policy()), @@ -61,6 +62,7 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> { cwd: cli.cwd.clone().map(|p| p.canonicalize().unwrap_or(p)), model_provider: None, config_profile: cli.config_profile.clone(), + codex_linux_sandbox_exe, }; #[allow(clippy::print_stderr)] match Config::load_with_overrides(overrides) { diff --git a/codex-rs/tui/src/main.rs b/codex-rs/tui/src/main.rs index 531682daae..08738ba245 100644 --- a/codex-rs/tui/src/main.rs +++ b/codex-rs/tui/src/main.rs @@ -1,10 +1,18 @@ +use std::path::PathBuf; + use clap::Parser; use codex_tui::Cli; use codex_tui::run_main; #[tokio::main] async fn main() -> std::io::Result<()> { + let codex_linux_sandbox_exe: Option = if cfg!(target_os = "linux") { + std::env::current_exe().ok() + } else { + None + }; + let cli = Cli::parse(); - run_main(cli)?; + run_main(cli, codex_linux_sandbox_exe)?; Ok(()) }