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 dfe7555d58..e7252ce2c0 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_process_only_env_var; #[derive(Clone)] pub(crate) struct CommandExecRequestProcessor { @@ -163,6 +164,7 @@ impl CommandExecRequestProcessor { } } } + env.retain(|name, _| !is_process_only_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 0b84c7f7b9..66c09fd692 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_process_only_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_process_only_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 edd704c9fb..6c7db05c8d 100644 --- a/codex-rs/app-server/tests/suite/v2/command_exec.rs +++ b/codex-rs/app-server/tests/suite/v2/command_exec.rs @@ -19,6 +19,8 @@ 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_IDENTITY_TOKEN_ENV_VAR; +use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::path::Path; @@ -151,7 +153,10 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) .without_auto_env() - .with_env_overrides(&[("COMMAND_EXEC_BASELINE", Some("server"))]) + .with_env_overrides(&[ + ("COMMAND_EXEC_BASELINE", Some("server")), + (OPENAI_IDENTITY_TOKEN_ENV_VAR, Some("host-assertion")), + ]) .build() .await?; timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; @@ -161,7 +166,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_IDENTITY_TOKEN-unset}\" \"${OPENAI_IDENTITY_TOKEN_FILE-unset}\"".to_string(), ], process_id: None, tty: false, @@ -179,6 +184,10 @@ 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_IDENTITY_TOKEN_FILE_ENV_VAR.to_string(), + Some("request-token-file".to_string()), + ), ])), size: None, sandbox_policy: None, @@ -194,7 +203,10 @@ 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|{}|unset|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 de7930e28d..bdf0abe0af 100644 --- a/codex-rs/app-server/tests/suite/v2/process_exec.rs +++ b/codex-rs/app-server/tests/suite/v2/process_exec.rs @@ -7,6 +7,8 @@ use codex_app_server_protocol::ProcessKillParams; use codex_app_server_protocol::ProcessSpawnParams; use codex_app_server_protocol::RequestId; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; +use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_ENV_VAR; +use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -103,6 +105,73 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu Ok(()) } +#[tokio::test] +async fn process_spawn_removes_process_only_environment_variables() -> Result<()> { + let codex_home = TempDir::new()?; + let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; + create_config_toml(codex_home.path(), &server.uri(), "never")?; + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[(OPENAI_IDENTITY_TOKEN_ENV_VAR, Some("host-assertion"))]) + .build() + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let command = if cfg!(windows) { + vec![ + "powershell.exe".to_string(), + "-NoProfile".to_string(), + "-NonInteractive".to_string(), + "-Command".to_string(), + concat!( + "$token = if ($null -eq $env:OPENAI_IDENTITY_TOKEN) { 'unset' } ", + "else { $env:OPENAI_IDENTITY_TOKEN }; ", + "$file = if ($null -eq $env:OPENAI_IDENTITY_TOKEN_FILE) { 'unset' } ", + "else { $env:OPENAI_IDENTITY_TOKEN_FILE }; ", + "[Console]::Out.Write(\"$token|$file\")", + ) + .to_string(), + ] + } else { + vec![ + "sh".to_string(), + "-c".to_string(), + concat!( + "printf '%s|%s' ", + "\"${OPENAI_IDENTITY_TOKEN-unset}\" ", + "\"${OPENAI_IDENTITY_TOKEN_FILE-unset}\"", + ) + .to_string(), + ] + }; + let process_handle = "filtered-environment".to_string(); + let request_id = mcp + .send_process_spawn_request(ProcessSpawnParams { + env: Some(HashMap::from([( + OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.to_string(), + Some("request-token-file".to_string()), + )])), + ..process_spawn_params(process_handle.clone(), codex_home.path(), command)? + }) + .await?; + mcp.read_stream_until_response_message(RequestId::Integer(request_id)) + .await?; + + assert_eq!( + read_process_exited(&mut mcp).await?, + ProcessExitedNotification { + process_handle, + exit_code: 0, + stdout: "unset|unset".to_string(), + stdout_cap_reached: false, + stderr: String::new(), + stderr_cap_reached: false, + } + ); + Ok(()) +} + #[tokio::test] async fn process_spawn_returns_error_when_local_environment_is_disabled() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index f9210aea3e..eb4583771c 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -52,6 +52,7 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::McpStartupStatus; use codex_protocol::protocol::McpStartupUpdateEvent; +use codex_protocol::shell_environment::is_process_only_env_var; use codex_rmcp_client::ExecutorStdioServerLauncher; use codex_rmcp_client::LocalStdioServerLauncher; use codex_rmcp_client::RmcpClient; @@ -781,6 +782,11 @@ fn resolve_bearer_token( let Some(env_var) = bearer_token_env_var else { return Ok(None); }; + if is_process_only_env_var(env_var) { + return Err(anyhow!( + "MCP server '{server_name}' cannot use process-only environment variable {env_var} as a bearer token" + )); + } match env::var(env_var) { Ok(value) => { @@ -1045,6 +1051,21 @@ mod tests { assert!(error.is_authentication_required()); } + #[test] + fn resolve_bearer_token_rejects_process_only_environment_variables() { + let error = resolve_bearer_token( + "server", + Some(codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_ENV_VAR), + ) + .expect_err("process-only variable should be rejected"); + + assert!( + error + .to_string() + .contains("process-only environment variable") + ); + } + #[test] fn mcp_initialize_advertises_openai_form_only_when_supported() { let unsupported = mcp_initialize_request_params( diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 6a95093003..92a35d5477 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -17,6 +17,8 @@ use anyhow::anyhow; use anyhow::bail; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; +use codex_protocol::shell_environment::PROCESS_ONLY_ENV_VARS; +use codex_protocol::shell_environment::is_process_only_env_var; use codex_utils_absolute_path::AbsolutePathBuf; use tokio::fs; use tokio::process::Command; @@ -230,7 +232,7 @@ async fn capture_snapshot(shell: &Shell, cwd: &AbsolutePathBuf) -> Result run_shell_script(shell, &zsh_snapshot_script(), cwd).await, ShellType::Bash => run_shell_script(shell, &bash_snapshot_script(), cwd).await, ShellType::Sh => run_shell_script(shell, &sh_snapshot_script(), cwd).await, - ShellType::PowerShell => run_shell_script(shell, powershell_snapshot_script(), cwd).await, + ShellType::PowerShell => run_shell_script(shell, &powershell_snapshot_script(), cwd).await, ShellType::Cmd => bail!("Shell snapshotting is not yet supported for {shell_type:?}"), } } @@ -289,6 +291,11 @@ async fn run_script_with_timeout( handler.args(&args[1..]); handler.stdin(Stdio::null()); handler.current_dir(cwd); + for (name, _) in std::env::vars_os() { + if name.to_str().is_some_and(is_process_only_env_var) { + handler.env_remove(name); + } + } #[cfg(unix)] unsafe { handler.pre_exec(|| { @@ -312,7 +319,25 @@ async fn run_script_with_timeout( } fn excluded_exports_regex() -> String { - EXCLUDED_EXPORT_VARS.join("|") + EXCLUDED_EXPORT_VARS + .iter() + .map(|name| (*name).to_string()) + .chain(PROCESS_ONLY_ENV_VARS.iter().map(|name| { + let mut pattern = String::with_capacity(name.len() * 4); + for character in name.chars() { + if character.is_ascii_alphabetic() { + pattern.push('['); + pattern.push(character.to_ascii_uppercase()); + pattern.push(character.to_ascii_lowercase()); + pattern.push(']'); + } else { + pattern.push(character); + } + } + pattern + })) + .collect::>() + .join("|") } fn zsh_snapshot_script() -> String { @@ -469,7 +494,13 @@ fi script.replace("EXCLUDED_EXPORTS", &excluded) } -fn powershell_snapshot_script() -> &'static str { +fn powershell_snapshot_script() -> String { + let excluded = EXCLUDED_EXPORT_VARS + .iter() + .chain(PROCESS_ONLY_ENV_VARS) + .map(|name| format!("'{name}'")) + .collect::>() + .join(", "); r##"$ErrorActionPreference = 'Stop' Write-Output '# Snapshot file' Write-Output '# Unset all aliases to avoid conflicts with functions' @@ -485,13 +516,15 @@ $aliases | ForEach-Object { "Set-Alias -Name {0} -Value {1}" -f $_.Name, $_.Definition } Write-Output '' -$envVars = Get-ChildItem Env: +$excludedEnvVars = @(EXCLUDED_EXPORTS) +$envVars = Get-ChildItem Env: | Where-Object { $_.Name -notin $excludedEnvVars } Write-Output ("# exports " + $envVars.Count) $envVars | ForEach-Object { $escaped = $_.Value -replace "'", "''" "`$env:{0}='{1}'" -f $_.Name, $escaped } "## + .replace("EXCLUDED_EXPORTS", &excluded) } /// Removes shell snapshots that either lack a matching session rollout file or diff --git a/codex-rs/core/src/shell_snapshot_tests.rs b/codex-rs/core/src/shell_snapshot_tests.rs index dcba3e2459..b594fdeef0 100644 --- a/codex-rs/core/src/shell_snapshot_tests.rs +++ b/codex-rs/core/src/shell_snapshot_tests.rs @@ -132,17 +132,26 @@ fn bash_snapshot_filters_invalid_exports() -> Result<()> { .env("BASH_ENV", "/dev/null") .env("VALID_NAME", "ok") .env("PWD", "/tmp/stale") + .env("pwd", "keep-lowercase") .env("NEXTEST_BIN_EXE_codex-write-config-schema", "/path/to/bin") .env("BAD-NAME", "broken") + .env("OPENAI_IDENTITY_TOKEN", "assertion") + .env("openai_identity_token_file", "/run/identity-token") .output()?; assert!(output.status.success()); let stdout = String::from_utf8_lossy(&output.stdout); assert!(stdout.contains("VALID_NAME")); + assert!(stdout.contains("pwd=\"keep-lowercase\"")); assert!(!stdout.contains("PWD=/tmp/stale")); assert!(!stdout.contains("NEXTEST_BIN_EXE_codex-write-config-schema")); assert!(!stdout.contains("BAD-NAME")); + assert!( + !stdout + .to_ascii_lowercase() + .contains("openai_identity_token") + ); Ok(()) } diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 3c488f0ad7..5b338e70ea 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -574,13 +574,16 @@ impl LocalProcess { } fn child_env(params: &ExecParams) -> HashMap { - let Some(env_policy) = ¶ms.env_policy else { - return params.env.clone(); + let mut env = match ¶ms.env_policy { + Some(env_policy) => { + let policy = shell_environment_policy(env_policy); + let mut env = shell_environment::create_env(&policy, /*thread_id*/ None); + env.extend(params.env.clone()); + env + } + None => params.env.clone(), }; - - let policy = shell_environment_policy(env_policy); - let mut env = shell_environment::create_env(&policy, /*thread_id*/ None); - env.extend(params.env.clone()); + env.retain(|name, _| !shell_environment::is_process_only_env_var(name)); env } @@ -1086,8 +1089,14 @@ mod tests { } #[test] - fn child_env_defaults_to_exact_env() { - let params = test_exec_params(HashMap::from([("ONLY_THIS".to_string(), "1".to_string())])); + fn child_env_without_policy_filters_process_only_variables() { + let params = test_exec_params(HashMap::from([ + ("ONLY_THIS".to_string(), "1".to_string()), + ( + shell_environment::OPENAI_IDENTITY_TOKEN_ENV_VAR.to_string(), + "assertion".to_string(), + ), + ])); assert_eq!( child_env(¶ms), @@ -1100,12 +1109,22 @@ mod tests { let mut params = test_exec_params(HashMap::from([ ("OVERLAY".to_string(), "overlay".to_string()), ("POLICY_SET".to_string(), "overlay-wins".to_string()), + ( + shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.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()), + ( + shell_environment::OPENAI_IDENTITY_TOKEN_ENV_VAR.to_string(), + "policy-assertion".to_string(), + ), + ]), include_only: Vec::new(), }); diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index 9f4c4e6059..b98d25904a 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -3,6 +3,7 @@ use std::process::Stdio; use std::time::Duration; use std::time::Instant; +use codex_protocol::shell_environment::is_process_only_env_var; use tokio::io::AsyncWriteExt; use tokio::process::Command; use tokio::time::timeout; @@ -174,6 +175,16 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { command.arg(&handler.command); } command.envs(&handler.env); + for (name, _) in std::env::vars_os() { + if name.to_str().is_some_and(is_process_only_env_var) { + command.env_remove(name); + } + } + for name in handler.env.keys() { + if is_process_only_env_var(name) { + command.env_remove(name); + } + } command } @@ -194,3 +205,7 @@ fn default_shell_command() -> Command { command } } + +#[cfg(test)] +#[path = "command_runner_tests.rs"] +mod tests; diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs new file mode 100644 index 0000000000..e080491774 --- /dev/null +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -0,0 +1,54 @@ +use std::collections::HashMap; +use std::ffi::OsStr; + +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookSource; +use codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_ENV_VAR; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; + +use super::*; + +#[test] +fn build_command_removes_process_only_variables_after_handler_overrides() { + let token_name = OPENAI_IDENTITY_TOKEN_ENV_VAR.to_ascii_lowercase(); + let token_file_name = "OpenAI_Identity_Token_File".to_string(); + let handler = ConfiguredHandler { + event_name: HookEventName::SessionStart, + matcher: None, + command: "true".to_string(), + timeout_sec: 1, + status_message: None, + source_path: AbsolutePathBuf::from_absolute_path("/tmp/hooks.json") + .expect("absolute source path"), + source: HookSource::User, + display_order: 0, + env: HashMap::from([ + (token_name.clone(), "configured-assertion".to_string()), + ( + token_file_name.clone(), + "/configured/identity-token".to_string(), + ), + ]), + }; + + let command = build_command( + &CommandShell { + program: String::new(), + args: Vec::new(), + }, + &handler, + ); + let configured_env = command + .as_std() + .get_envs() + .map(|(name, value)| (name.to_owned(), value.map(ToOwned::to_owned))) + .collect::>(); + + assert_eq!(configured_env.get(OsStr::new(&token_name)), Some(&None)); + assert_eq!( + configured_env.get(OsStr::new(&token_file_name)), + Some(&None) + ); + assert!(configured_env.values().all(Option::is_none)); +} diff --git a/codex-rs/hooks/src/legacy_notify.rs b/codex-rs/hooks/src/legacy_notify.rs index 0e273b421f..fd970bd98e 100644 --- a/codex-rs/hooks/src/legacy_notify.rs +++ b/codex-rs/hooks/src/legacy_notify.rs @@ -71,6 +71,9 @@ pub fn notify_hook(argv: Vec) -> Hook { #[cfg(test)] mod tests { + use std::collections::HashMap; + use std::ffi::OsStr; + use anyhow::Result; use codex_protocol::ThreadId; use codex_utils_absolute_path::test_support::PathBufExt; @@ -95,6 +98,21 @@ mod tests { }) } + #[test] + fn legacy_notify_command_removes_process_only_environment_variables() { + let command = command_from_argv(&["true".to_string()]).expect("valid command"); + let configured_env = command + .as_std() + .get_envs() + .map(|(name, value)| (name.to_owned(), value.map(ToOwned::to_owned))) + .collect::>(); + + for name in codex_protocol::shell_environment::PROCESS_ONLY_ENV_VARS { + assert_eq!(configured_env.get(OsStr::new(name)), Some(&None)); + } + assert!(configured_env.values().all(Option::is_none)); + } + #[test] fn test_user_notification() -> Result<()> { let notification = UserNotification::AgentTurnComplete { diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 1f4e01aa59..52c874435d 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -1,5 +1,7 @@ use codex_config::ConfigLayerStack; use codex_plugin::PluginHookSource; +use codex_protocol::shell_environment::PROCESS_ONLY_ENV_VARS; +use codex_protocol::shell_environment::is_process_only_env_var; use tokio::process::Command; use crate::engine::ClaudeHooksEngine; @@ -229,5 +231,13 @@ pub fn command_from_argv(argv: &[String]) -> Option { } let mut command = Command::new(program); command.args(args); + for name in PROCESS_ONLY_ENV_VARS { + command.env_remove(name); + } + for (name, _) in std::env::vars_os() { + if name.to_str().is_some_and(is_process_only_env_var) { + command.env_remove(name); + } + } Some(command) } diff --git a/codex-rs/protocol/src/shell_environment.rs b/codex-rs/protocol/src/shell_environment.rs index 9edb91c2f3..9e8fb3d948 100644 --- a/codex-rs/protocol/src/shell_environment.rs +++ b/codex-rs/protocol/src/shell_environment.rs @@ -4,6 +4,21 @@ use crate::config_types::ShellEnvironmentPolicyInherit; use std::collections::HashMap; pub const CODEX_THREAD_ID_ENV_VAR: &str = "CODEX_THREAD_ID"; +pub const OPENAI_IDENTITY_TOKEN_ENV_VAR: &str = "OPENAI_IDENTITY_TOKEN"; +pub const OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR: &str = "OPENAI_IDENTITY_TOKEN_FILE"; + +/// Environment variables consumed by Codex itself that must not be forwarded +/// to model-controlled commands or extension processes. +pub const PROCESS_ONLY_ENV_VARS: &[&str] = &[ + OPENAI_IDENTITY_TOKEN_ENV_VAR, + OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR, +]; + +pub fn is_process_only_env_var(name: &str) -> bool { + PROCESS_ONLY_ENV_VARS + .iter() + .any(|process_only| process_only.eq_ignore_ascii_case(name)) +} /// Construct a shell environment from the supplied process environment and /// shell-environment policy. @@ -106,6 +121,10 @@ where env_map.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string()); } + // Process-only credentials cannot be restored through user-provided shell + // environment overrides. + env_map.retain(|name, _| !is_process_only_env_var(name)); + env_map } @@ -209,6 +228,25 @@ mod windows_tests { assert_eq!(result, expected); } + + #[test] + fn process_only_variables_are_removed_case_insensitively() { + let vars = make_vars(&[("openai_identity_token", "assertion")]); + let policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ignore_default_excludes: true, + r#set: HashMap::from([( + OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.to_string(), + "C:\\run\\identity-token".to_string(), + )]), + ..Default::default() + }; + + assert_eq!( + populate_env(vars, &policy, /*thread_id*/ None), + HashMap::new() + ); + } } #[cfg(all(test, not(target_os = "windows")))] @@ -247,4 +285,32 @@ mod non_windows_tests { assert_eq!(result, expected); } + + #[test] + fn process_only_variables_cannot_be_inherited_or_set() { + let vars = make_vars(&[ + (OPENAI_IDENTITY_TOKEN_ENV_VAR, "assertion"), + (OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR, "/run/identity-token"), + ]); + let policy = ShellEnvironmentPolicy { + inherit: ShellEnvironmentPolicyInherit::All, + ignore_default_excludes: true, + r#set: HashMap::from([ + ( + OPENAI_IDENTITY_TOKEN_ENV_VAR.to_string(), + "configured-assertion".to_string(), + ), + ( + OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.to_string(), + "/configured/identity-token".to_string(), + ), + ]), + ..Default::default() + }; + + assert_eq!( + populate_env(vars, &policy, /*thread_id*/ None), + HashMap::new() + ); + } } diff --git a/codex-rs/rmcp-client/src/auth_status.rs b/codex-rs/rmcp-client/src/auth_status.rs index 32c244af8e..83daaab95a 100644 --- a/codex-rs/rmcp-client/src/auth_status.rs +++ b/codex-rs/rmcp-client/src/auth_status.rs @@ -5,6 +5,7 @@ use std::time::Duration; use anyhow::Result; use codex_exec_server::HttpClient; use codex_protocol::protocol::McpAuthStatus; +use codex_protocol::shell_environment::is_process_only_env_var; use futures::FutureExt; use reqwest::Client; use reqwest::header::AUTHORIZATION; @@ -134,7 +135,12 @@ fn auth_status_before_discovery( store_mode: OAuthCredentialsStoreMode, keyring_backend_kind: AuthKeyringBackendKind, ) -> Result { - if bearer_token_env_var.is_some() { + if let Some(env_var) = bearer_token_env_var { + if is_process_only_env_var(env_var) { + anyhow::bail!( + "MCP server '{server_name}' cannot use process-only environment variable {env_var} as a bearer token" + ); + } return Ok(AuthStatusCheck::Complete(McpAuthState::BearerToken)); } @@ -366,6 +372,27 @@ mod tests { assert_eq!(status, McpAuthState::BearerToken); } + #[tokio::test] + async fn determine_auth_status_rejects_process_only_bearer_token_environment_variables() { + let error = determine_streamable_http_auth_status( + "server", + "not-a-url", + Some(codex_protocol::shell_environment::OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR), + /*http_headers*/ None, + /*env_http_headers*/ None, + OAuthCredentialsStoreMode::Keyring, + AuthKeyringBackendKind::default(), + ) + .await + .expect_err("process-only variable should be rejected"); + + assert!( + error + .to_string() + .contains("process-only environment variable") + ); + } + #[tokio::test] #[serial(auth_status_env)] async fn determine_auth_status_uses_bearer_token_when_env_authorization_header_present() { diff --git a/codex-rs/rmcp-client/src/utils.rs b/codex-rs/rmcp-client/src/utils.rs index 892df544cb..f9fcccc0a6 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_process_only_env_var; use reqwest::ClientBuilder; use reqwest::header::HeaderMap; use reqwest::header::HeaderName; @@ -14,13 +15,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_process_only_env_var(name)) + }); Ok(env) } @@ -31,18 +36,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_process_only_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_process_only_env_var(var.name())) .map(|var| var.name().to_string()) .collect() } @@ -54,7 +65,10 @@ fn local_stdio_env_var_names(env_vars: &[McpServerEnvVar]) -> Result