diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index c1d231f9..f0eadaf2 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 5358065c..6408e8de 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 bedce9f2..705d3130 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 998072c5..5a65fcbc 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 f71f9b86..31416565 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 aa0691d8..b2b1b8cf 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 e40848ca..d4a78404 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 e2979497..2d4ed8f3 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 0f914727..69e50478 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 de97b36e..2a3f4543 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 22c3e856..6696f76f 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 158a0da9..96b601b6 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 00000000..2957f3da --- /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 e74c5621..76bd428a 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 6e9b8de7..07c56815 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 759f1029..261ae0a0 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;