diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index dfe7555d5852..42e350a79418 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -1,4 +1,5 @@ use super::*; +use codex_protocol::shell_environment::is_non_inheritable_env_var; #[derive(Clone)] pub(crate) struct CommandExecRequestProcessor { @@ -163,6 +164,7 @@ impl CommandExecRequestProcessor { } } } + env.retain(|name, _| !is_non_inheritable_env_var(name)); let timeout_ms = match timeout_ms { Some(timeout_ms) => match u64::try_from(timeout_ms) { Ok(timeout_ms) => Some(timeout_ms), diff --git a/codex-rs/app-server/src/request_processors/process_exec_processor.rs b/codex-rs/app-server/src/request_processors/process_exec_processor.rs index 8b9b50843005..e44a75e929e6 100644 --- a/codex-rs/app-server/src/request_processors/process_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/process_exec_processor.rs @@ -25,6 +25,7 @@ use codex_core::exec::ExecExpirationOutcome; use codex_core::exec::IO_DRAIN_TIMEOUT_MS; use codex_exec_server::EnvironmentManager; use codex_protocol::exec_output::bytes_to_string_smart; +use codex_protocol::shell_environment::is_non_inheritable_env_var; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use codex_utils_pty::ProcessHandle; @@ -107,6 +108,7 @@ impl ProcessExecRequestProcessor { } } } + env.retain(|name, _| !is_non_inheritable_env_var(name)); let expiration = match timeout_ms { Some(Some(timeout_ms)) => match u64::try_from(timeout_ms) { Ok(timeout_ms) => timeout_ms.into(), diff --git a/codex-rs/app-server/tests/suite/v2/command_exec.rs b/codex-rs/app-server/tests/suite/v2/command_exec.rs index f3955e7ffc7a..700af7049e72 100644 --- a/codex-rs/app-server/tests/suite/v2/command_exec.rs +++ b/codex-rs/app-server/tests/suite/v2/command_exec.rs @@ -18,6 +18,7 @@ use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SandboxPolicy; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; +use codex_protocol::shell_environment::OPENAI_FEDERATION_RULE_ID_ENV_VAR; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::path::Path; @@ -151,7 +152,7 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un command: vec![ "/bin/sh".to_string(), "-lc".to_string(), - "printf '%s|%s|%s|%s' \"$COMMAND_EXEC_BASELINE\" \"$COMMAND_EXEC_EXTRA\" \"${RUST_LOG-unset}\" \"$CODEX_HOME\"".to_string(), + "printf '%s|%s|%s|%s|%s|%s' \"$COMMAND_EXEC_BASELINE\" \"$COMMAND_EXEC_EXTRA\" \"${RUST_LOG-unset}\" \"$CODEX_HOME\" \"$OPENAI_FEDERATION_RULE_ID\" \"$openai_identity_token_file\"".to_string(), ], process_id: None, tty: false, @@ -169,6 +170,14 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un ), ("COMMAND_EXEC_EXTRA".to_string(), Some("added".to_string())), ("RUST_LOG".to_string(), None), + ( + OPENAI_FEDERATION_RULE_ID_ENV_VAR.to_string(), + Some("rule".to_string()), + ), + ( + "openai_identity_token_file".to_string(), + Some("/run/identity-token".to_string()), + ), ])), size: None, sandbox_policy: None, @@ -181,7 +190,7 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un response, CommandExecResponse { exit_code: 0, - stdout: format!("request|added|unset|{}", codex_home.path().display()), + stdout: format!("request|added|unset|{}||", codex_home.path().display()), stderr: String::new(), } ); diff --git a/codex-rs/app-server/tests/suite/v2/process_exec.rs b/codex-rs/app-server/tests/suite/v2/process_exec.rs index de7930e28d74..0050872a5b4e 100644 --- a/codex-rs/app-server/tests/suite/v2/process_exec.rs +++ b/codex-rs/app-server/tests/suite/v2/process_exec.rs @@ -42,7 +42,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu "while (!(Test-Path -LiteralPath $env:CODEX_PROCESS_EXEC_RELEASE_FILE)) { ", "Start-Sleep -Milliseconds 20 ", "}; ", - "[Console]::Out.Write('process-out'); ", + "[Console]::Out.Write(('process-out|{0}|{1}' -f $env:OpenAI_Federation_Rule_Id, $env:OPENAI_IDENTITY_TOKEN_FILE)); ", "[Console]::Error.Write('process-err')", ) .to_string(), @@ -54,7 +54,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu concat!( "printf process > \"$CODEX_PROCESS_EXEC_PROBE_FILE\"; ", "while [ ! -e \"$CODEX_PROCESS_EXEC_RELEASE_FILE\" ]; do sleep 0.05; done; ", - "printf process-out; ", + "printf 'process-out|%s|%s' \"$OpenAI_Federation_Rule_Id\" \"$OPENAI_IDENTITY_TOKEN_FILE\"; ", "printf process-err >&2", ) .to_string(), @@ -69,6 +69,14 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu "CODEX_PROCESS_EXEC_RELEASE_FILE".to_string(), Some(release_file.display().to_string()), ), + ( + "OpenAI_Federation_Rule_Id".to_string(), + Some("rule".to_string()), + ), + ( + "OPENAI_IDENTITY_TOKEN_FILE".to_string(), + Some("/run/identity-token".to_string()), + ), ]); let spawn_request_id = mcp .send_process_spawn_request(ProcessSpawnParams { @@ -94,7 +102,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu ProcessExitedNotification { process_handle, exit_code: 0, - stdout: "process-out".to_string(), + stdout: "process-out||".to_string(), stdout_cap_reached: false, stderr: "process-err".to_string(), stderr_cap_reached: false, diff --git a/codex-rs/code-mode/Cargo.toml b/codex-rs/code-mode/Cargo.toml index 60c0fdcfef7f..80121599fa48 100644 --- a/codex-rs/code-mode/Cargo.toml +++ b/codex-rs/code-mode/Cargo.toml @@ -16,6 +16,7 @@ workspace = true codex-code-mode-protocol = { workspace = true } codex-http-client = { workspace = true } codex-install-context = { workspace = true } +codex-protocol = { workspace = true } codex-websocket-client = { workspace = true } futures = { workspace = true } tokio = { workspace = true, features = ["io-util", "macros", "net", "process", "rt", "sync", "time"] } @@ -24,6 +25,5 @@ tokio-util = { workspace = true, features = ["rt"] } tracing = { workspace = true } [dev-dependencies] -codex-protocol = { workspace = true } pretty_assertions = { workspace = true } tokio = { workspace = true, features = ["test-util"] } diff --git a/codex-rs/code-mode/src/remote_session/connection.rs b/codex-rs/code-mode/src/remote_session/connection.rs index e5c7a4c3d32c..355ee6c7920c 100644 --- a/codex-rs/code-mode/src/remote_session/connection.rs +++ b/codex-rs/code-mode/src/remote_session/connection.rs @@ -33,6 +33,7 @@ use codex_code_mode_protocol::host::SESSION_RESOURCE_LIMITS_CAPABILITY; use codex_code_mode_protocol::host::SupportedProtocolVersions; use codex_code_mode_protocol::host::TransportLane; use codex_http_client::HttpClientFactory; +use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; use codex_websocket_client::WebSocketConnector; use futures::StreamExt; use tokio::io::AsyncBufReadExt; @@ -215,16 +216,16 @@ impl Connection { let mut command = Command::new(host_program); #[cfg(unix)] command.process_group(0); - let mut child = command + command .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .kill_on_drop(true) - .spawn() - .map_err(|error| ConnectionError::Spawn { - host_program: host_program.to_path_buf(), - error, - })?; + .kill_on_drop(true); + scrub_non_inheritable_env_vars(command.as_std_mut()); + let mut child = command.spawn().map_err(|error| ConnectionError::Spawn { + host_program: host_program.to_path_buf(), + error, + })?; if let Some(stderr) = child.stderr.take() { tokio::spawn(async move { diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index a23a1d749e83..a11b48ba3797 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -8,6 +8,7 @@ use tokio::process::Command; use tracing::trace; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::shell_environment::is_non_inheritable_env_var; /// Experimental environment variable that will be set to some non-empty value /// if both of the following are true: @@ -60,6 +61,8 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io mut env, } = request; + env.retain(|name, _| !is_non_inheritable_env_var(name)); + trace!( "spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {network_sandbox_policy:?} {stdio_policy:?} {env:?}" ); diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index fe85cb7153dd..9fc740fca49a 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -23,6 +23,7 @@ pub(crate) use codex_network_proxy::is_managed_proxy_env_var; pub(crate) use codex_network_proxy::strip_managed_proxy_env; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::AdditionalPermissionProfile; +use codex_protocol::shell_environment::is_non_inheritable_env_var; use codex_sandboxing::SandboxCommand; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -304,6 +305,7 @@ fn build_override_exports( .keys() .map(String::as_str) .chain(restore_even_when_absent.iter().copied()) + .filter(|key| !is_non_inheritable_env_var(key)) .filter(|key| is_valid_shell_variable_name(key)) .collect::>(); keys.sort_unstable(); diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 34c89581476c..3b1fb1b9dbc3 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -1082,10 +1082,16 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() { "-lc".to_string(), "printf '%s' \"$OPENAI_API_KEY\"".to_string(), ]; - let explicit_env_overrides = HashMap::from([( - "OPENAI_API_KEY".to_string(), - "super-secret-value".to_string(), - )]); + let explicit_env_overrides = HashMap::from([ + ( + "OPENAI_API_KEY".to_string(), + "super-secret-value".to_string(), + ), + ( + "openai_identity_token_file".to_string(), + "/run/identity-token".to_string(), + ), + ]); let rewritten = maybe_wrap_shell_lc_with_snapshot( &command, &session_shell, @@ -1099,6 +1105,7 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() { ); assert!(!rewritten[2].contains("super-secret-value")); + assert!(!rewritten[2].contains("openai_identity_token_file")); let output = Command::new(&rewritten[0]) .args(&rewritten[1..]) .env("OPENAI_API_KEY", "super-secret-value") diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 5d628a105979..6511d03d8dea 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -66,6 +66,7 @@ use codex_protocol::error::SandboxErr; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandSource; use codex_protocol::protocol::TerminalInteractionEvent; +use codex_protocol::shell_environment::is_non_inheritable_env_var; use codex_sandboxing::SandboxCommand; use codex_tools::ToolName; use codex_utils_output_truncation::approx_tokens_from_byte_count; @@ -123,7 +124,10 @@ fn exec_env_policy_from_shell_policy( .collect::>(); exclude.push(CODEX_PERMISSION_PROFILE_ENV_VAR.to_string()); let mut r#set = policy.r#set.clone(); - r#set.retain(|key, _| !key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR)); + r#set.retain(|key, _| { + !key.eq_ignore_ascii_case(CODEX_PERMISSION_PROFILE_ENV_VAR) + && !is_non_inheritable_env_var(key) + }); codex_exec_server::ExecEnvPolicy { inherit: policy.inherit.clone(), ignore_default_excludes: policy.ignore_default_excludes, @@ -144,8 +148,9 @@ fn env_overlay_for_exec_server( request_env .iter() .filter(|(key, value)| { - key.as_str() == CODEX_PERMISSION_PROFILE_ENV_VAR - || local_policy_env.get(*key) != Some(*value) + !is_non_inheritable_env_var(key) + && (key.as_str() == CODEX_PERMISSION_PROFILE_ENV_VAR + || local_policy_env.get(*key) != Some(*value)) }) .map(|(key, value)| (key.clone(), value.clone())) .collect() diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index ead8d6257bc2..6a8ba5f4c721 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -51,6 +51,7 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() { let request_env = HashMap::from([ ("HOME".to_string(), "/client-home".to_string()), ("PATH".to_string(), "/sandbox-path".to_string()), + ("OpenAI_Federation_Rule_Id".to_string(), "rule".to_string()), ("SHELL_SET".to_string(), "policy".to_string()), ("CODEX_THREAD_ID".to_string(), "thread-1".to_string()), ( @@ -81,13 +82,17 @@ fn env_overlay_for_exec_server_keeps_runtime_changes_only() { } #[test] -fn exec_env_policy_excludes_runtime_permission_profile() { +fn exec_env_policy_excludes_non_inheritable_and_runtime_variables() { let policy = ShellEnvironmentPolicy { r#set: HashMap::from([ ( "codex_permission_profile".to_string(), "stale-profile".to_string(), ), + ( + "openai_identity_token_file".to_string(), + "/run/identity-token".to_string(), + ), ("KEEP".to_string(), "value".to_string()), ]), ..Default::default() diff --git a/codex-rs/exec-server/src/client_transport.rs b/codex-rs/exec-server/src/client_transport.rs index 139211bf1a9e..20fba497386d 100644 --- a/codex-rs/exec-server/src/client_transport.rs +++ b/codex-rs/exec-server/src/client_transport.rs @@ -11,6 +11,7 @@ use tracing::debug; use tracing::warn; use codex_http_client::HttpClientFactory; +use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; use codex_utils_rustls_provider::ensure_rustls_crypto_provider; use codex_websocket_client::WebSocketConnector; use codex_websocket_client::WebSocketTlsMode; @@ -441,6 +442,7 @@ fn stdio_command_process(stdio_command: &StdioExecServerCommand) -> Command { let mut command = Command::new(&stdio_command.program); command.args(&stdio_command.args); command.envs(&stdio_command.env); + scrub_non_inheritable_env_vars(command.as_std_mut()); if let Some(cwd) = &stdio_command.cwd { command.current_dir(cwd); } diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 26567f68cf96..303d86a0d5c7 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -327,7 +327,7 @@ fn spawn_command( SandboxExecRequest { command: argv, cwd, - env, + mut env, arg0, .. }: SandboxExecRequest, @@ -346,6 +346,7 @@ fn spawn_command( // TODO(anp): Keep PathUri through the filesystem helper launch boundary. let cwd = cwd.to_abs_path().map_err(io_error)?; command.current_dir(cwd.as_path()); + env.retain(|name, _| !codex_protocol::shell_environment::is_non_inheritable_env_var(name)); command.env_clear(); command.envs(env); command.stdin(std::process::Stdio::piped()); diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 1c80d8564bdc..9bf0329f49d4 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -649,6 +649,7 @@ fn child_env(params: &ExecParams) -> HashMap { None => params.env.clone(), }; env.remove(crate::CODEX_EXEC_SERVER_EXIT_ON_STDIN_CLOSE_ENV_VAR); + env.retain(|name, _| !shell_environment::is_non_inheritable_env_var(name)); env } @@ -1268,12 +1269,19 @@ mod tests { let mut params = test_exec_params(HashMap::from([ ("OVERLAY".to_string(), "overlay".to_string()), ("POLICY_SET".to_string(), "overlay-wins".to_string()), + ( + "openai_identity_token_file".to_string(), + "/run/identity-token".to_string(), + ), ])); params.env_policy = Some(ExecEnvPolicy { inherit: ShellEnvironmentPolicyInherit::None, ignore_default_excludes: true, exclude: Vec::new(), - r#set: HashMap::from([("POLICY_SET".to_string(), "policy".to_string())]), + r#set: HashMap::from([ + ("POLICY_SET".to_string(), "policy".to_string()), + ("OpenAI_Federation_Rule_Id".to_string(), "rule".to_string()), + ]), include_only: Vec::new(), }); diff --git a/codex-rs/git-utils/src/apply.rs b/codex-rs/git-utils/src/apply.rs index 54f8b06b0b3b..56bd416914e8 100644 --- a/codex-rs/git-utils/src/apply.rs +++ b/codex-rs/git-utils/src/apply.rs @@ -124,12 +124,14 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { } fn resolve_git_root(cwd: &Path) -> io::Result { - let out = std::process::Command::new("git") + let mut command = std::process::Command::new("git"); + command .args(["-c", crate::SAFE_BARE_REPOSITORY_CONFIG]) .arg("rev-parse") .arg("--show-toplevel") - .current_dir(cwd) - .output()?; + .current_dir(cwd); + crate::scrub_non_inheritable_environment(&mut command); + let out = command.output()?; let code = out.status.code().unwrap_or(-1); if code != 0 { return Err(io::Error::other(format!( @@ -158,6 +160,7 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, for a in args { cmd.arg(a); } + crate::scrub_non_inheritable_environment(&mut cmd); let out = cmd.current_dir(cwd).output()?; let code = out.status.code().unwrap_or(-1); let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); @@ -338,6 +341,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { for p in &existing { cmd.arg(OsStr::new(p)); } + crate::scrub_non_inheritable_environment(&mut cmd); let out = cmd.current_dir(git_root).output()?; let _code = out.status.code().unwrap_or(-1); // We do not hard fail staging; best-effort is OK. Return Ok even on non-zero. diff --git a/codex-rs/git-utils/src/git_process.rs b/codex-rs/git-utils/src/git_process.rs index f9b6566232f2..d6af3fd26e5b 100644 --- a/codex-rs/git-utils/src/git_process.rs +++ b/codex-rs/git-utils/src/git_process.rs @@ -29,6 +29,7 @@ impl Drop for KillGitProcessTreeOnDrop { } fn spawn_git_command(command: &mut Command) -> Option<(Child, KillGitProcessTreeOnDrop)> { + crate::scrub_non_inheritable_environment(command.as_std_mut()); #[cfg(unix)] command.process_group(0); command.kill_on_drop(true); diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index 1c75e7e7dabe..99fe12f4e96d 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -48,3 +48,7 @@ pub use info::recent_commits; pub use info::resolve_root_git_project_for_trust; pub use platform::create_symlink; pub use status::get_has_changes_in_repo; + +pub(crate) fn scrub_non_inheritable_environment(command: &mut std::process::Command) { + codex_protocol::shell_environment::scrub_non_inheritable_env_vars(command); +} diff --git a/codex-rs/git-utils/src/operations.rs b/codex-rs/git-utils/src/operations.rs index abd9665976fe..0dce4524b850 100644 --- a/codex-rs/git-utils/src/operations.rs +++ b/codex-rs/git-utils/src/operations.rs @@ -120,6 +120,7 @@ where } } command.args(&args_vec); + crate::scrub_non_inheritable_environment(&mut command); let output = command.output()?; if !output.status.success() { let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index aec7e723b332..d9bef0151def 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -8,6 +8,7 @@ use std::time::Duration; use std::time::Instant; use async_channel::Sender; +use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; #[cfg(windows)] use codex_utils_pty::JobObject; use tokio::io::AsyncWriteExt; @@ -395,6 +396,7 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { command.arg(&handler.command); } command.envs(&handler.env); + scrub_non_inheritable_env_vars(command.as_std_mut()); command } diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 3d0e47584e5b..9d857602feef 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -28,6 +28,7 @@ use async_channel::Receiver; use codex_config::ConfigLayerStack; use codex_plugin::PluginHookSource; use codex_protocol::ThreadId; +use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; use std::time::Duration; use tokio::process::Command; @@ -271,5 +272,6 @@ pub fn command_from_argv(argv: &[String]) -> Option { } let mut command = Command::new(program); command.args(args); + scrub_non_inheritable_env_vars(command.as_std_mut()); Some(command) } diff --git a/codex-rs/protocol/src/shell_environment.rs b/codex-rs/protocol/src/shell_environment.rs index 9edb91c2f33b..c7ee880d5bf7 100644 --- a/codex-rs/protocol/src/shell_environment.rs +++ b/codex-rs/protocol/src/shell_environment.rs @@ -4,6 +4,44 @@ use crate::config_types::ShellEnvironmentPolicyInherit; use std::collections::HashMap; pub const CODEX_THREAD_ID_ENV_VAR: &str = "CODEX_THREAD_ID"; +pub const OPENAI_FEDERATION_RULE_ID_ENV_VAR: &str = "OPENAI_FEDERATION_RULE_ID"; +pub const OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR: &str = "OPENAI_IDENTITY_TOKEN_FILE"; + +/// Environment variables that model-reachable child processes must not inherit. +pub const NON_INHERITABLE_ENV_VARS: &[&str] = &[ + OPENAI_FEDERATION_RULE_ID_ENV_VAR, + OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR, +]; + +pub fn is_non_inheritable_env_var(name: &str) -> bool { + NON_INHERITABLE_ENV_VARS + .iter() + .any(|restricted| restricted.eq_ignore_ascii_case(name)) +} + +/// Configures a child command to omit non-inheritable variables from the +/// process environment and explicit command overrides. +/// +/// This prevents accidental propagation of Codex launch context; it is not a +/// filesystem security boundary for the referenced identity-token file. +pub fn scrub_non_inheritable_env_vars(command: &mut std::process::Command) { + let configured_names = command + .get_envs() + .map(|(name, _)| name.to_os_string()) + .collect::>(); + + for name in NON_INHERITABLE_ENV_VARS { + command.env_remove(name); + } + for name in std::env::vars_os() + .map(|(name, _)| name) + .chain(configured_names) + { + if name.to_str().is_some_and(is_non_inheritable_env_var) { + command.env_remove(name); + } + } +} /// Construct a shell environment from the supplied process environment and /// shell-environment policy. @@ -106,6 +144,10 @@ where env_map.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string()); } + // Restricted launch context cannot be restored through user-provided shell + // environment overrides. + env_map.retain(|name, _| !is_non_inheritable_env_var(name)); + env_map } @@ -148,6 +190,10 @@ pub const WINDOWS_CORE_ENV_VARS: &[&str] = &[ "PWSH", ]; +#[cfg(test)] +#[path = "shell_environment_tests.rs"] +mod tests; + #[cfg(all(test, target_os = "windows"))] mod windows_tests { use super::*; diff --git a/codex-rs/protocol/src/shell_environment_tests.rs b/codex-rs/protocol/src/shell_environment_tests.rs new file mode 100644 index 000000000000..3b7c5831cce6 --- /dev/null +++ b/codex-rs/protocol/src/shell_environment_tests.rs @@ -0,0 +1,84 @@ +use std::collections::HashMap; +use std::process::Command; + +use pretty_assertions::assert_eq; + +use super::*; + +const CHILD_MODE_ENV_VAR: &str = "CODEX_SHELL_ENVIRONMENT_SCRUBBER_TEST_MODE"; +const TEST_NAME: &str = + "shell_environment::tests::command_scrubber_removes_names_from_real_child_environment"; + +#[test] +fn non_inheritable_environment_is_removed_after_policy_overrides() { + let vars = [ + ("SAFE".to_string(), "inherited".to_string()), + ( + "openai_federation_rule_id".to_string(), + "inherited-rule".to_string(), + ), + ]; + let policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ignore_default_excludes: true, + r#set: HashMap::from([ + ("SAFE".to_string(), "override".to_string()), + ( + "OpenAI_Identity_Token_File".to_string(), + "/run/identity-token".to_string(), + ), + ]), + ..Default::default() + }; + + assert_eq!( + populate_env(vars, &policy, /*thread_id*/ None), + HashMap::from([("SAFE".to_string(), "override".to_string())]) + ); +} + +#[test] +fn command_scrubber_removes_names_from_real_child_environment() { + if std::env::var_os(CHILD_MODE_ENV_VAR).is_none() { + let output = Command::new(std::env::current_exe().expect("locate current test binary")) + .args([TEST_NAME, "--exact", "--nocapture"]) + .env(CHILD_MODE_ENV_VAR, "1") + .env("OpenAI_Federation_Rule_Id", "inherited-rule") + .output() + .expect("run inherited-environment test process"); + assert!( + output.status.success(), + "child failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + return; + } + + let mut command = environment_command(); + command + .env("openai_identity_token_file", "/run/identity-token") + .env("SAFE", "value"); + scrub_non_inheritable_env_vars(&mut command); + let output = command.output().expect("read child environment"); + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + let restricted_names = stdout + .lines() + .filter_map(|line| line.split_once('=').map(|(name, _)| name)) + .filter(|name| is_non_inheritable_env_var(name)) + .collect::>(); + assert_eq!(restricted_names, Vec::<&str>::new()); +} + +#[cfg(windows)] +fn environment_command() -> Command { + let mut command = Command::new("cmd.exe"); + command.args(["/D", "/C", "set"]); + command +} + +#[cfg(not(windows))] +fn environment_command() -> Command { + Command::new("env") +} diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index b3b97ca24cb1..f6267d38d928 100644 --- a/codex-rs/rmcp-client/src/utils.rs +++ b/codex-rs/rmcp-client/src/utils.rs @@ -1,6 +1,7 @@ use anyhow::Result; use anyhow::anyhow; use codex_config::types::McpServerEnvVar; +use codex_protocol::shell_environment::is_non_inheritable_env_var; use http::HeaderMap; use http::HeaderName; use http::HeaderValue; @@ -16,13 +17,17 @@ pub(crate) fn create_env_for_mcp_server( env_vars: &[McpServerEnvVar], ) -> Result> { let additional_env_vars = local_stdio_env_var_names(env_vars)?; - let env = DEFAULT_ENV_VARS + let mut env: HashMap = DEFAULT_ENV_VARS .iter() .copied() .chain(additional_env_vars) .filter_map(|var| env::var_os(var).map(|value| (OsString::from(var), value))) .chain(extra_env.unwrap_or_default()) .collect(); + env.retain(|name, _| { + name.to_str() + .is_none_or(|name| !is_non_inheritable_env_var(name)) + }); Ok(env) } @@ -33,18 +38,24 @@ pub(crate) fn create_env_overlay_for_remote_mcp_server( // Remote stdio should inherit PATH/HOME/etc. from the executor side, not // from the orchestrator process. Only forward variables explicitly named // by the MCP config plus literal env overrides from that config. - env_vars + let mut env: HashMap = env_vars .iter() .filter(|var| !var.is_remote_source()) .filter_map(|var| env::var_os(var.name()).map(|value| (OsString::from(var.name()), value))) .chain(extra_env.unwrap_or_default()) - .collect() + .collect(); + env.retain(|name, _| { + name.to_str() + .is_none_or(|name| !is_non_inheritable_env_var(name)) + }); + env } pub(crate) fn remote_mcp_env_var_names(env_vars: &[McpServerEnvVar]) -> Vec { env_vars .iter() .filter(|var| var.is_remote_source()) + .filter(|var| !is_non_inheritable_env_var(var.name())) .map(|var| var.name().to_string()) .collect() } @@ -56,7 +67,10 @@ fn local_stdio_env_var_names(env_vars: &[McpServerEnvVar]) -> Result std::io::Result { - let mut child = Command::new(executable) + let mut command = Command::new(executable); + command .args([ "-NoLogo", "-NoProfile", @@ -123,8 +124,9 @@ impl PowershellParserProcess { ]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn()?; + .stderr(Stdio::null()); + codex_protocol::shell_environment::scrub_non_inheritable_env_vars(&mut command); + let mut child = command.spawn()?; let stdin = match take_child_stdin(&mut child) { Ok(stdin) => stdin, Err(error) => { diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 7d01bc04736d..f45c4317e66f 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -9,6 +9,7 @@ use std::sync::Mutex; use std::time::Duration; use anyhow::Context as _; +use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; use codex_utils_absolute_path::AbsolutePathBuf; use socket2::Socket; use tokio::process::Command; @@ -341,6 +342,7 @@ async fn handle_escalate_session_with_policy( .stdout(Stdio::null()) .stderr(Stdio::null()) .kill_on_drop(true); + scrub_non_inheritable_env_vars(command.as_std_mut()); unsafe { command.pre_exec(move || { for (dst_fd, src_fd) in msg.fds.iter().zip(&fds) {