openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

[auth] keep workload credentials out of child processes [ci changed_files]

Browse Source
This commit is contained in:
Cooper Gamble
2026-07-10 05:07:18 +00:00
parent 1f0566d3f5
commit 898cee38c6
15 changed files with 466 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/command_exec_processor.rs
+2
View File
@@ -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),
codex-rs/app-server/src/request_processors/process_exec_processor.rs
+2
View File
@@ -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(),
codex-rs/app-server/tests/suite/v2/command_exec.rs
+15 -3
View File
@@ -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(),
}
);
codex-rs/app-server/tests/suite/v2/process_exec.rs
+69
View File
@@ -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()?;
codex-rs/codex-mcp/src/rmcp_client.rs
+21
View File
@@ -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(
codex-rs/core/src/shell_snapshot.rs
+37 -4
View File
@@ -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<String
ShellType::Zsh => 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::<Vec<_>>()
.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::<Vec<_>>()
.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
codex-rs/core/src/shell_snapshot_tests.rs
+9
View File
@@ -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(())
}
codex-rs/exec-server/src/local_process.rs
+28 -9
View File
@@ -574,13 +574,16 @@ impl LocalProcess {
}
fn child_env(params: &ExecParams) -> HashMap<String, String> {
let Some(env_policy) = &params.env_policy else {
return params.env.clone();
let mut env = match &params.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(&params),
@@ -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(),
});
codex-rs/hooks/src/engine/command_runner.rs
+15
View File
@@ -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;
codex-rs/hooks/src/engine/command_runner_tests.rs
+54
View File
@@ -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::<HashMap<_, _>>();
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));
}
codex-rs/hooks/src/legacy_notify.rs
+18
View File
@@ -71,6 +71,9 @@ pub fn notify_hook(argv: Vec<String>) -> 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::<HashMap<_, _>>();
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 {
codex-rs/hooks/src/registry.rs
+10
View File
@@ -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<Command> {
}
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)
}
codex-rs/protocol/src/shell_environment.rs
+66
View File
@@ -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()
);
}
}
codex-rs/rmcp-client/src/auth_status.rs
+28 -1
View File
@@ -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<AuthStatusCheck> {
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() {
codex-rs/rmcp-client/src/utils.rs
+92 -4
View File
@@ -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<HashMap<OsString, OsString>> {
let additional_env_vars = local_stdio_env_var_names(env_vars)?;
let env = DEFAULT_ENV_VARS
let mut env: HashMap<OsString, OsString> = 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<OsString, OsString> = 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<String> {
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<impl Iterat
remote_var.name()
));
}
Ok(env_vars.iter().map(McpServerEnvVar::name))
Ok(env_vars
.iter()
.map(McpServerEnvVar::name)
.filter(|name| !is_process_only_env_var(name)))
}
pub(crate) fn build_default_headers(
@@ -85,6 +99,9 @@ pub(crate) fn build_default_headers(
if let Some(env_headers) = env_http_headers {
for (name, env_var) in env_headers {
if is_process_only_env_var(&env_var) {
continue;
}
if let Ok(value) = env::var(&env_var) {
if value.trim().is_empty() {
continue;
@@ -148,6 +165,8 @@ pub(crate) const DEFAULT_ENV_VARS: &[&str] =
#[cfg(test)]
mod tests {
use super::*;
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 serial_test::serial;
@@ -209,6 +228,75 @@ mod tests {
assert_eq!(env.get(OsStr::new(custom_var)), Some(&expected));
}
#[test]
#[serial(extra_rmcp_env)]
fn create_env_excludes_process_only_variables_from_all_sources() {
let _token_guard = EnvVarGuard::set(OPENAI_IDENTITY_TOKEN_ENV_VAR, "assertion");
let _file_guard =
EnvVarGuard::set(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR, "/run/identity-token");
let env = create_env_for_mcp_server(
Some(HashMap::from([
(
OsString::from(OPENAI_IDENTITY_TOKEN_ENV_VAR),
OsString::from("configured-assertion"),
),
(
OsString::from(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR),
OsString::from("/configured/identity-token"),
),
])),
&[
OPENAI_IDENTITY_TOKEN_ENV_VAR.into(),
OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.into(),
],
)
.expect("local MCP env should build");
let expected =
create_env_for_mcp_server(/*extra_env*/ None, &[]).expect("baseline MCP env");
assert_eq!(env, expected);
}
#[test]
#[serial(extra_rmcp_env)]
fn create_remote_env_excludes_process_only_variables() {
let _token_guard = EnvVarGuard::set(OPENAI_IDENTITY_TOKEN_ENV_VAR, "assertion");
let env = create_env_overlay_for_remote_mcp_server(
Some(HashMap::from([(
OsString::from(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR),
OsString::from("/configured/identity-token"),
)])),
&[OPENAI_IDENTITY_TOKEN_ENV_VAR.into()],
);
assert_eq!(env, HashMap::new());
}
#[test]
#[serial(extra_rmcp_env)]
fn default_headers_exclude_process_only_environment_variables() {
let _token_guard = EnvVarGuard::set(OPENAI_IDENTITY_TOKEN_ENV_VAR, "assertion");
let _file_guard =
EnvVarGuard::set(OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR, "/run/identity-token");
let headers = build_default_headers(
/*http_headers*/ None,
Some(HashMap::from([
(
"x-openai-assertion".to_string(),
OPENAI_IDENTITY_TOKEN_ENV_VAR.to_string(),
),
(
"x-openai-assertion-file".to_string(),
OPENAI_IDENTITY_TOKEN_FILE_ENV_VAR.to_string(),
),
])),
)
.expect("default headers should build");
assert_eq!(headers, HeaderMap::new());
}
#[test]
#[serial(extra_rmcp_env)]
fn create_remote_env_overlay_only_forwards_explicit_variables() {