diff --git a/codex-rs/core/src/config/network_proxy_spec_tests.rs b/codex-rs/core/src/config/network_proxy_spec_tests.rs index afdbd894a2..20e8c02282 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -104,7 +104,9 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun Some(composed.environment_policy()), /*fallback_policy_decider*/ None, )?; - let launch = scoped.remote_launch_config(DefaultFalse).await?; + let launch = scoped + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) + .await?; assert_eq!( launch.proxy.dangerously_allow_all_unix_sockets, expected, "{name}" @@ -120,7 +122,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun // Inheritance must not grant sockets to ordinary controller-only commands. assert!( !proxy - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) .await? .proxy .dangerously_allow_all_unix_sockets @@ -128,7 +130,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun state.add_allowed_domain("granted.example").await?; assert!( scoped - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) .await? .proxy .dangerously_allow_all_unix_sockets @@ -149,7 +151,7 @@ async fn attachment_socket_grants_respect_configured_restrictions_at_remote_laun .await?; assert!( !scoped - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) .await? .proxy .dangerously_allow_all_unix_sockets @@ -890,7 +892,9 @@ async fn environment_local_binding_preserves_explicit_denials_and_inherits_omitt Some(policy), Some(Arc::new(|_request| async { NetworkDecision::Allow })), )?; - let launch = scoped.remote_launch_config(local_binding_policy).await; + let launch = scoped + .remote_launch_config(local_binding_policy, /*credential_masking*/ false) + .await; assert_eq!(proxy.current_cfg().await?, carrier.config, "{name}"); if local_binding_policy == RequireTrue && !expected { assert!( diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 61d66c6927..cde30c959d 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -40,12 +40,16 @@ use crate::unified_exec::TerminalSandboxSource; use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; use codex_core_plugins::PluginMetricsSidecar; +use codex_exec_server::EnvironmentCapabilities; use codex_features::Feature; use codex_network_proxy::CREDENTIAL_BROKER_ACTIVE_ENV_KEY; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::NetworkProxy; +use codex_network_proxy::RemoteNetworkProxyConfig; use codex_protocol::error::CodexErr; use codex_protocol::models::AdditionalPermissionProfile; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::WarningEvent; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxablePreference; use codex_sandboxing::policy_transforms::merge_permission_profiles; @@ -338,15 +342,42 @@ impl<'a> ToolRuntime for UnifiedExecRunt }; let (mut env, managed_network_context, network_proxy_launch) = match managed_network { Some(network) if environment_is_remote => { + let environment_info = + req.turn_environment + .environment + .info() + .await + .map_err(|err| { + ToolError::Codex(CodexErr::Io(io::Error::other(format!( + "failed to query exec-server capabilities: {err}" + )))) + })?; + let credential_masking = ctx + .step_context + .turn + .config + .features + .enabled(Feature::CredentialMasking) + && environment_info.capabilities.credential_masking; let mut launch = network - .remote_launch_config(crate::windows_sandbox::local_binding_policy_for_sandbox( - req.turn_environment.config().windows_sandbox_type, - req.turn_environment.executor_platform_os.as_deref(), - )) + .remote_launch_config( + crate::windows_sandbox::local_binding_policy_for_sandbox( + req.turn_environment.config().windows_sandbox_type, + req.turn_environment.executor_platform_os.as_deref(), + ), + credential_masking, + ) .await .map_err(|err| { ToolError::Codex(CodexErr::Io(io::Error::other(err.to_string()))) })?; + warn_if_remote_credential_masking_unavailable( + ctx, + req, + &environment_info.capabilities, + &launch.proxy, + ) + .await; if routes_approval_policy_to_guardian( ctx.step_context.settings.approval_policy(), ctx.step_context.settings.approvals_reviewer(), @@ -371,16 +402,6 @@ impl<'a> ToolRuntime for UnifiedExecRunt if !launch.proxy.enabled { (env, None, None) } else { - let environment_info = - req.turn_environment - .environment - .info() - .await - .map_err(|err| { - ToolError::Codex(CodexErr::Io(io::Error::other(format!( - "failed to query exec-server capabilities: {err}" - )))) - })?; if !environment_info.capabilities.network_proxy_launch { return Err(ToolError::Rejected( "selected exec-server does not support executor-local network proxy launches" @@ -677,6 +698,52 @@ impl<'a> ToolRuntime for UnifiedExecRunt } } +async fn warn_if_remote_credential_masking_unavailable( + ctx: &ToolCtx, + req: &UnifiedExecRequest, + capabilities: &EnvironmentCapabilities, + proxy_config: &RemoteNetworkProxyConfig, +) { + if !ctx + .step_context + .turn + .config + .features + .enabled(Feature::CredentialMasking) + { + return; + } + + let environment_id = &req.turn_environment.selection.environment_id; + let message = if !capabilities.credential_masking { + format!( + "Credential masking is unavailable on environment `{environment_id}` because its exec-server does not support it. This command will run with unmasked credentials." + ) + } else if !proxy_config.credential_masking { + format!( + "Credential masking is unavailable for this network proxy configuration on environment `{environment_id}`. This command will run with unmasked credentials." + ) + } else if req.shell.shell.shell_type != ShellType::Bash { + format!( + "Credential masking is unavailable for shell `{}` on environment `{environment_id}`. This command will run with unmasked credentials.", + req.shell.shell.name(), + ) + } else if req.shell.use_login_shell || req.shell_snapshot.is_some() { + format!( + "Credential masking is unavailable for this shell startup on environment `{environment_id}`. This command will run with unmasked credentials." + ) + } else { + return; + }; + + ctx.session + .send_event( + ctx.step_context.turn.as_ref(), + EventMsg::Warning(WarningEvent { message }), + ) + .await; +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/tests/common/exec_server.rs b/codex-rs/core/tests/common/exec_server.rs index aa36d1a4d2..3d9b392a7c 100644 --- a/codex-rs/core/tests/common/exec_server.rs +++ b/codex-rs/core/tests/common/exec_server.rs @@ -25,10 +25,15 @@ pub struct ExecServerProcess { impl ExecServerProcess { pub async fn start() -> Result { + Self::start_with_env(&[]).await + } + + pub async fn start_with_env(env: &[(&str, &str)]) -> Result { let codex_home = TempDir::new()?; let mut child = Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) .args(["exec-server", "--listen", "ws://127.0.0.1:0"]) .env("CODEX_HOME", codex_home.path()) + .envs(env.iter().copied()) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 6211559f76..59894c49f8 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -160,6 +160,10 @@ mod preparation; #[path = "scenarios_content_filter.rs"] mod content_filter; +#[cfg(any(target_os = "linux", target_os = "macos"))] +#[path = "scenarios_credential_masking.rs"] +mod credential_masking; + #[path = "scenarios_provider_capabilities.rs"] mod provider_capabilities; diff --git a/codex-rs/core/tests/suite/scenarios_credential_masking.rs b/codex-rs/core/tests/suite/scenarios_credential_masking.rs new file mode 100644 index 0000000000..bdc9d748cf --- /dev/null +++ b/codex-rs/core/tests/suite/scenarios_credential_masking.rs @@ -0,0 +1,200 @@ +//! Remote credential masking and its effects on model-visible context. + +use super::*; +use anyhow::Context; +use codex_protocol::models::PermissionProfile; +use core_test_support::exec_server::ExecServerProcess; +use core_test_support::managed_network_requirements_loader; +use core_test_support::responses::ev_function_call; +use core_test_support::skip_if_sandbox; +use pretty_assertions::assert_eq; +use pretty_assertions::assert_ne; + +#[test_case::test_case(false, false, None; "builtin_providers")] +#[test_case::test_case(true, false, None; "configured_provider_fallback")] +#[test_case::test_case(true, true, None; "configured_provider_login_fallback")] +#[test_case::test_case(true, false, Some("BASH_ENV"); "configured_provider_executor_startup_fallback")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_credential_masking_request_history( + configured_provider: bool, + use_login_shell: bool, + executor_startup_env: Option<&'static str>, +) -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + if codex_core::shell::default_user_shell().name() != "bash" { + eprintln!("skipping credential masking: this scenario requires a Bash executor"); + return Ok(()); + } + let home = Arc::new(TempDir::new()?); + if let Some(warning) = + codex_sandboxing::system_bwrap_warning(&PermissionProfile::workspace_write(), home.path()) + { + eprintln!("skipping sandbox test: {warning}"); + return Ok(()); + } + const API_KEY: &str = "sk-proj-executor-only-0000000000000000000000000000000000"; + const CALL_ID: &str = "print-credential"; + // Use our own native executor so only its environment contains the credential. + let mut executor_env = vec![ + ("OPENAI_API_KEY", API_KEY), + ("OPENAI_BASE_URL", "https://api.openai.com/v1"), + ]; + executor_env.extend(executor_startup_env.map(|key| (key, "/dev/null"))); + let executor = ExecServerProcess::start_with_env(&executor_env).await?; + let server = start_mock_server().await; + let test = test_codex() + .with_home(home) + .with_exec_server_url(executor.websocket_url.clone()) + .with_cloud_config_bundle(managed_network_requirements_loader()) + .with_pre_build_hook(move |home| { + let provider_config = if configured_provider { + r#"[features.network_proxy.credentials.vendor] +env = ["VENDOR_TOKEN"] +patterns = ["vendor_[a-z]{8}"] +url_prefixes = ["https://api.vendor.example"] +auth = ["bearer"] +"# + } else { + "" + }; + let startup_env = executor_startup_env + .map(|key| format!(", \"{key}\"")) + .unwrap_or_default(); + fs::write( + home.join("config.toml"), + format!( + r#"default_permissions = "brokered" + +[permissions.brokered] +extends = ":workspace" + +[permissions.brokered.network] +enabled = true +mode = "full" +allow_local_binding = true + +[shell_environment_policy] +ignore_default_excludes = true +include_only = ["HOME", "PATH", "OPENAI_API_KEY", "OPENAI_BASE_URL"{startup_env}] + +[features] +unified_exec = true +credential_masking = true +{provider_config}"# + ), + ) + .expect("write credential masking config"); + }) + .with_config(|config| { + configure_scenario_catalog(config); + config.project_doc_max_bytes = 0; + }) + .with_model_info_override("gpt-5.5", |model| { + model.tool_mode = Some(ToolMode::Direct); + model.use_responses_lite = false; + }) + .build(&server) + .await?; + let mut command = if configured_provider { + // Keep the real credential out of the model-visible command text. + fs::write(test.workspace_path("expected-api-key"), API_KEY)?; + "test \"$OPENAI_API_KEY\" = \"$("); + for (pattern, replacement) in [ + (r"Chunk ID: [a-z0-9]+", "Chunk ID: "), + ( + r"Wall time: [0-9]+(?:\.[0-9]+)? seconds", + "Wall time: seconds", + ), + ( + r"Original token count: [0-9]+", + "Original token count: ", + ), + ] { + snapshot = regex_lite::Regex::new(pattern)? + .replace_all(&snapshot, replacement) + .into_owned(); + } + insta::assert_snapshot!("remote_credential_masking", snapshot); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__credential_masking__remote_credential_masking.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__credential_masking__remote_credential_masking.snap new file mode 100644 index 0000000000..bca45a7e74 --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__credential_masking__remote_credential_masking.snap @@ -0,0 +1,24 @@ +--- +source: core/tests/suite/scenarios_credential_masking.rs +expression: snapshot +--- +Scenario: A remote executor masks its credential before the command output reaches the model. + +## Window 1 +-- request 1 (turn) -- +00:message/developer: + +01:message/developer: + +02:message/user: + +03:message/user: + Print OPENAI_API_KEY from the executor environment. +-- request 2 (turn) -- +04:function_call/exec_command:{"cmd":"printf '%s' \\"$OPENAI_API_KEY\\"","environment_id":"remote","login":false,"yield_time_ms":1000} +05:function_call_output:Chunk ID: + Wall time: seconds + Process exited with code 0 + Original token count: + Output: + diff --git a/codex-rs/core/tests/suite/unified_exec_process_events.rs b/codex-rs/core/tests/suite/unified_exec_process_events.rs index 3ead08bb0f..08e50dfcd3 100644 --- a/codex-rs/core/tests/suite/unified_exec_process_events.rs +++ b/codex-rs/core/tests/suite/unified_exec_process_events.rs @@ -75,6 +75,9 @@ const REPLAY_RETAINED_OUTPUT_SEQ: u64 = 800; #[cfg_attr(windows, allow(dead_code))] enum PushedExecScenario { Complete, + CredentialMasking, + CredentialMaskingUnsupportedShell, + CredentialMaskingUnsupported, OutputAfterExit, OutputAfterRecoveredExit, OutputOpenAtDeadline, @@ -191,6 +194,16 @@ async fn respond_environment_info( | PushedExecScenario::RejectedLongWindowsDangerousCommand ) { json!({ "name": "powershell", "path": "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" }) + } else if matches!( + scenario, + PushedExecScenario::CredentialMaskingUnsupportedShell + ) { + json!({ "name": "sh", "path": "/bin/sh" }) + } else if matches!( + scenario, + PushedExecScenario::CredentialMasking | PushedExecScenario::CredentialMaskingUnsupported + ) { + json!({ "name": "bash", "path": "/bin/bash" }) } else { json!({ "name": "zsh", "path": "/bin/zsh" }) }; @@ -200,10 +213,20 @@ async fn respond_environment_info( | PushedExecScenario::RejectedLongWindowsDangerousCommand => Some("windows"), PushedExecScenario::DenyOldLinux | PushedExecScenario::DenyOldLinuxWithRoot(_) - | PushedExecScenario::DenyLinux => Some("linux"), + | PushedExecScenario::DenyLinux + | PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + | PushedExecScenario::CredentialMaskingUnsupported => Some("linux"), _ => None, }; let mut capabilities = json!({ "networkProxyLaunch": true }); + if matches!( + scenario, + PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + ) { + capabilities["credentialMasking"] = json!(true); + } if matches!( scenario, PushedExecScenario::DenyOldLinuxWithRoot(_) | PushedExecScenario::DenyLinux @@ -542,6 +565,9 @@ async fn serve_exec_with_pushed_events( } } PushedExecScenario::Complete + | PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + | PushedExecScenario::CredentialMaskingUnsupported | PushedExecScenario::DenyWin | PushedExecScenario::DenyWinUnchanged | PushedExecScenario::DenyOldLinux @@ -695,6 +721,9 @@ async fn serve_exec_with_pushed_events( panic!("unexpected read after the delayed-output sequence") } PushedExecScenario::Complete + | PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + | PushedExecScenario::CredentialMaskingUnsupported | PushedExecScenario::CodexPath | PushedExecScenario::CodexPathDefaultFeature | PushedExecScenario::CodexPathDisabledFeature @@ -829,6 +858,9 @@ async fn serve_exec_with_pushed_events( #[test_case(PushedExecScenario::LegacyExit, ManagedNetworkScenario::None, false ; "legacy_exit_metadata")] #[test_case(PushedExecScenario::ReplayGap, ManagedNetworkScenario::None, false ; "truncated_event_replay")] #[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: true }, false ; "managed_network_uses_executor_proxy_launch")] +#[test_case(PushedExecScenario::CredentialMasking, ManagedNetworkScenario::Enabled { policy_callbacks: true }, false ; "credential_masking_launches_on_supported_executor")] +#[test_case(PushedExecScenario::CredentialMaskingUnsupported, ManagedNetworkScenario::Enabled { policy_callbacks: true }, false ; "credential_masking_warns_and_runs_unsupported_executor")] +#[test_case(PushedExecScenario::CredentialMaskingUnsupportedShell, ManagedNetworkScenario::Enabled { policy_callbacks: true }, false ; "credential_masking_warns_and_runs_unsupported_shell")] #[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: false }, false ; "strict_managed_allowlist_omits_policy_callbacks")] #[test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Disabled, false ; "disabled_managed_network_omits_executor_proxy_launch")] #[cfg_attr(not(windows), test_case(PushedExecScenario::Complete, ManagedNetworkScenario::Enabled { policy_callbacks: true }, true ; "foreign_windows_managed_network_preserves_approval_registration"))] @@ -869,6 +901,12 @@ async fn exec_command_consumes_pushed_remote_process_events( let terminate_after_exit = terminate_initial_collection || matches!(scenario, PushedExecScenario::OutputOpenWhenTerminated); let explicit_path = "/user/configured/bin:/user/other/bin"; + let credential_masking = matches!( + scenario, + PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupported + | PushedExecScenario::CredentialMaskingUnsupportedShell + ); let managed_network_configured = !matches!(managed_network, ManagedNetworkScenario::None); let managed_network_enabled = matches!(managed_network, ManagedNetworkScenario::Enabled { .. }); let policy_callbacks = matches!( @@ -917,6 +955,11 @@ async fn exec_command_consumes_pushed_remote_process_events( }) .to_string(), ), + PushedExecScenario::CredentialMasking => ev_function_call( + CALL_ID, + "exec_command", + &json!({ "cmd": "pwd", "login": false, "yield_time_ms": 1_000 }).to_string(), + ), _ => ev_function_call( CALL_ID, "exec_command", @@ -1041,10 +1084,11 @@ enabled = false }; builder = builder .with_cloud_config_bundle(cloud_config_bundle) - .with_pre_build_hook(|home| { + .with_pre_build_hook(move |home| { fs::write( home.join("config.toml"), - r#"default_permissions = "workspace" + format!( + r#"default_permissions = "workspace" [permissions.workspace.filesystem] ":minimal" = "read" @@ -1056,6 +1100,7 @@ allow_local_binding = true [features] hooks = true +credential_masking = {credential_masking} [hooks] @@ -1065,7 +1110,8 @@ hooks = true type = "command" command = "unused" timeout = 900 -"#, +"# + ), ) .expect("write managed-network test config"); }); @@ -1698,6 +1744,18 @@ timeout = 900 assert_eq!(params["managedNetwork"], Value::Null); assert_eq!(params["env"]["HTTP_PROXY"], Value::Null); assert_eq!(params["networkProxy"]["proxy"]["enabled"], true); + assert_eq!( + params["networkProxy"]["proxy"]["credentialMasking"], + if matches!( + scenario, + PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + ) { + json!(true) + } else { + Value::Null + } + ); assert_eq!(params["networkProxy"]["proxy"]["mode"], "full"); assert_eq!( params["networkProxy"]["policyDecisionTimeoutMs"].as_u64(), @@ -1712,6 +1770,45 @@ timeout = 900 }) .await .context("model should receive the remote exec output")?; + if matches!( + scenario, + PushedExecScenario::CredentialMaskingUnsupportedShell + | PushedExecScenario::CredentialMaskingUnsupported + ) { + let shell = if matches!( + scenario, + PushedExecScenario::CredentialMaskingUnsupportedShell + ) { + "/bin/sh" + } else { + "/bin/bash" + }; + assert_eq!(params["argv"], json!([shell, "-lc", "pwd"])); + let warning = core_test_support::wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::Warning(warning) if warning.message.starts_with("Credential masking is unavailable")) + }) + .await; + let EventMsg::Warning(warning) = warning else { + unreachable!("waited for a credential masking warning") + }; + if matches!( + scenario, + PushedExecScenario::CredentialMaskingUnsupportedShell + ) { + insta::assert_snapshot!(warning.message, @"Credential masking is unavailable for shell `sh` on environment `remote`. This command will run with unmasked credentials."); + } else { + insta::assert_snapshot!(warning.message, @"Credential masking is unavailable on environment `remote` because its exec-server does not support it. This command will run with unmasked credentials."); + } + let request = response_mock + .last_request() + .context("remote exec response")?; + assert!( + request + .function_call_output_text(CALL_ID) + .context("remote exec output")? + .contains(COMPLETE_OUTPUT) + ); + } return Ok(()); } if matches!(managed_network, ManagedNetworkScenario::Disabled) { @@ -1786,6 +1883,9 @@ timeout = 900 ); } PushedExecScenario::Complete + | PushedExecScenario::CredentialMasking + | PushedExecScenario::CredentialMaskingUnsupportedShell + | PushedExecScenario::CredentialMaskingUnsupported | PushedExecScenario::DenyWin | PushedExecScenario::DenyWinUnchanged | PushedExecScenario::DenyOldLinux diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index c0bb0966ee..b745e4e7a6 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -13,6 +13,7 @@ use codex_protocol::ThreadId; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; use codex_shell_command::shell_detect::DetectedShell; +use codex_shell_command::shell_detect::ShellType; use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde::Serialize; @@ -151,6 +152,9 @@ pub struct EnvironmentCapabilities { /// Whether `exec` accepts instructions for launching an executor-local network proxy. #[serde(default)] pub network_proxy_launch: bool, + /// Whether the selected shell can mask executor environment credentials. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub credential_masking: bool, /// Whether capability discovery applies the filesystem sandbox sent with each root. #[serde(default)] pub capability_discovery_sandbox: bool, @@ -260,8 +264,11 @@ impl EnvironmentInfo { }; let temp_dir = normalize_temp_path(std::env::temp_dir().into_os_string()); + let shell = codex_shell_command::shell_detect::default_user_shell(); + let credential_masking = cfg!(any(target_os = "linux", target_os = "macos")) + && shell.shell_type == ShellType::Bash; Self { - shell: codex_shell_command::shell_detect::default_user_shell().into(), + shell: shell.into(), executor_version: unknown_executor_version(), provider_id: None, cwd: cwd.and_then(|cwd| PathUri::from_host_native_path(cwd).ok()), @@ -278,6 +285,7 @@ impl EnvironmentInfo { temp_dir, capabilities: EnvironmentCapabilities { network_proxy_launch: true, + credential_masking, capability_discovery_sandbox: true, capability_discovery_v2: true, environment_config_read: true, @@ -1499,6 +1507,7 @@ mod tests { capabilities, EnvironmentCapabilities { network_proxy_launch: true, + credential_masking: false, capability_discovery_sandbox: true, capability_discovery_v2: false, environment_config_read: false, diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 271f8aaaf0..f3f174e998 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -161,6 +161,7 @@ struct Inner { notifications: std::sync::RwLock>, requests: Arc>>, processes: Mutex>, + credential_brokers: codex_network_proxy::RemoteCredentialBrokerCache, #[cfg(unix)] shell_snapshots: crate::shell_snapshot::ShellSnapshotCache, telemetry: ExecServerTelemetry, @@ -220,6 +221,7 @@ impl LocalProcess { notifications: std::sync::RwLock::new(Some(notifications)), requests: Arc::new(std::sync::RwLock::new(Some(requests))), processes: Mutex::new(HashMap::new()), + credential_brokers: codex_network_proxy::RemoteCredentialBrokerCache::default(), #[cfg(unix)] shell_snapshots: crate::shell_snapshot::ShellSnapshotCache::default(), telemetry, @@ -368,6 +370,7 @@ impl LocalProcess { self.runtime_paths.as_ref(), network_policy_decider, network_policy_audit_observer, + &self.inner.credential_brokers, &telemetry, ) .await?; @@ -838,6 +841,7 @@ impl ExecBackend for LocalProcess { self.runtime_paths.as_ref(), /*network_policy_decider*/ None, /*network_policy_audit_observer*/ None, + &self.inner.credential_brokers, &ProcessTelemetry::default(), ) .await @@ -1928,6 +1932,8 @@ mod tests { let state = NetworkProxyState::from_remote_launch_config( RemoteNetworkProxyLaunchConfig::new(proxy_config), codex_utils_path_uri::Platform::native(), + &codex_network_proxy::RemoteCredentialBrokerCache::default(), + &HashMap::new(), ) .expect("build network proxy state"); let proxy = NetworkProxy::builder() diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index cb7f061961..74efe6c352 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -1,5 +1,4 @@ use std::collections::HashMap; -#[cfg(windows)] use std::path::Path; use std::sync::Arc; @@ -7,6 +6,7 @@ use crate::process_telemetry::ProcessTelemetry; use codex_exec_server_protocol::JSONRPCErrorError; use codex_file_system::WindowsSandboxSelection; use codex_network_proxy::CUSTOM_CA_ENV_KEYS; +use codex_network_proxy::CredentialMaskingFailureMode; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::ManagedProxyRouting; use codex_network_proxy::NetworkPolicyAuditObserver; @@ -14,7 +14,7 @@ use codex_network_proxy::NetworkPolicyDecider; use codex_network_proxy::NetworkProxy; use codex_network_proxy::NetworkProxyHandle; use codex_network_proxy::NetworkProxyState; -use codex_network_proxy::RemoteNetworkProxyLaunchConfig; +use codex_network_proxy::RemoteCredentialBrokerCache; use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path; #[cfg(target_os = "windows")] use codex_network_proxy::strip_managed_proxy_env; @@ -46,6 +46,9 @@ use crate::rpc::internal_error; use crate::rpc::invalid_params; use crate::sandbox_selection::select_sandbox; +const CREDENTIAL_MASKING_FAILURE_MODE: CredentialMaskingFailureMode = + CredentialMaskingFailureMode::Warn; + pub(crate) struct PreparedExecRequest { pub(crate) command: Vec, pub(crate) cwd: AbsolutePathBuf, @@ -93,8 +96,10 @@ pub(crate) async fn prepare_exec_request_with_telemetry( runtime_paths: Option<&ExecServerRuntimeOptions>, network_policy_decider: Option>, network_policy_audit_observer: Option, + credential_brokers: &RemoteCredentialBrokerCache, telemetry: &ProcessTelemetry, ) -> Result { + let network_proxy = params.network_proxy.clone(); if let Some(sandbox) = params.sandbox.as_ref() && sandbox.windows_sandbox_selection == WindowsSandboxSelection::Mxc { @@ -120,10 +125,19 @@ pub(crate) async fn prepare_exec_request_with_telemetry( } None } else { - params.network_proxy.as_ref() + network_proxy }; - #[cfg(not(target_os = "windows"))] - let network_proxy = params.network_proxy.as_ref(); + let network_proxy = network_proxy + .map(|launch| { + NetworkProxyState::from_remote_launch_config( + launch, + codex_utils_path_uri::Platform::native(), + credential_brokers, + &env, + ) + }) + .transpose() + .map_err(|err| invalid_params(format!("invalid network proxy config: {err}")))?; let (env, managed_network, network_proxy_handle, network_proxy_restricting_sid) = prepare_managed_network( @@ -348,7 +362,7 @@ pub(crate) async fn prepare_exec_request_with_telemetry( async fn prepare_managed_network( params: &ExecParams, - network_proxy: Option<&RemoteNetworkProxyLaunchConfig>, + network_proxy: Option, proxy_private_ips_via_upstream: bool, env: HashMap, network_policy_decider: Option>, @@ -363,9 +377,18 @@ async fn prepare_managed_network( ), JSONRPCErrorError, > { - let Some(network_proxy) = network_proxy.cloned() else { + let Some(mut state) = network_proxy else { return Ok((env, params.managed_network.clone(), None, None)); }; + let credential_masking = state + .current_cfg() + .await + .map_err(|err| { + internal_error(format!( + "failed to read executor network proxy config: {err}" + )) + })? + .credential_broker; let routing = if params.sandbox.as_ref().is_some_and(|sandbox| { sandbox.windows_sandbox_selection == WindowsSandboxSelection::Mxc @@ -374,11 +397,6 @@ async fn prepare_managed_network( } else { ManagedProxyRouting::SharedIngress }; - let mut state = NetworkProxyState::from_remote_launch_config( - network_proxy, - codex_utils_path_uri::Platform::native(), - ) - .map_err(|err| invalid_params(format!("invalid network proxy config: {err}")))?; state.set_proxy_private_ips_via_upstream(proxy_private_ips_via_upstream); if let Some(observer) = network_policy_audit_observer { state.set_policy_audit_observer(observer); @@ -427,11 +445,34 @@ async fn prepare_managed_network( }; #[cfg(not(target_os = "windows"))] let network_proxy_restricting_sid = None; - let prepared = proxy + let supported_shell = matches!(params.argv.as_slice(), [program, flag, _] + if Path::new(program).file_name().is_some_and(|name| name == "bash") && flag == "-c") + && !["BASH_ENV", "ENV", "SSH_CLIENT", "SSH2_CLIENT"] + .iter() + .any(|key| env.contains_key(*key)); + let mut prepared = proxy .prepare_for_optional_environment(env, /*environment_id*/ None) .map_err(|err| { internal_error(format!("failed to prepare executor network proxy: {err}")) })?; + if credential_masking { + if cfg!(any(target_os = "linux", target_os = "macos")) + && supported_shell + && params.arg0.is_none() + && params.shell_snapshot.is_none() + { + proxy + .handle_unmasked_credentials(&mut prepared.env, CREDENTIAL_MASKING_FAILURE_MODE) + .map_err(|err| invalid_params(err.to_string()))?; + } else { + tracing::warn!( + "Credential masking is unavailable for this shell startup; preserving shell profiles and snapshots and running with unmasked credentials." + ); + // Restore this child's environment without disabling translation of placeholders + // issued to earlier commands in the same environment. + proxy.restore_and_disable_brokered_credentials(&mut prepared.env, &mut []); + } + } Ok(( prepared.env, Some(prepared.sandbox_context), diff --git a/codex-rs/exec-server/src/process_sandbox_tests.rs b/codex-rs/exec-server/src/process_sandbox_tests.rs index ef3c1e0f11..1ec954ae32 100644 --- a/codex-rs/exec-server/src/process_sandbox_tests.rs +++ b/codex-rs/exec-server/src/process_sandbox_tests.rs @@ -29,7 +29,6 @@ use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; #[cfg(unix)] use tempfile::tempdir; -#[cfg(windows)] use test_case::test_case; use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; @@ -59,6 +58,7 @@ async fn prepare_exec_request( runtime_paths, network_policy_decider, network_policy_audit_observer, + &codex_network_proxy::RemoteCredentialBrokerCache::default(), &ProcessTelemetry::default(), ) .await @@ -502,8 +502,16 @@ async fn native_request_preserves_native_launch_fields() { assert_eq!(prepared.arg0, params.arg0); } +#[test_case(false, None, &["-c", "true"]; "unmasked")] +#[test_case(true, Some("BASH_ENV"), &["-c", "true"]; "masking_with_startup_hook")] +#[test_case(true, Some("SSH_CLIENT"), &["-c", "true"]; "masking_with_remote_shell_startup")] +#[test_case(true, None, &["-c", "-l", "true"]; "masking_with_extra_login_option")] #[tokio::test] -async fn native_request_handles_remote_proxy_config_for_platform() { +async fn native_request_handles_remote_proxy_config_for_platform( + credential_masking: bool, + startup_env: Option<&str>, + shell_args: &[&str], +) { let cwd: AbsolutePathBuf = std::env::current_dir() .expect("current directory") .try_into() @@ -513,12 +521,16 @@ async fn native_request_handles_remote_proxy_config_for_platform() { ..NetworkProxyConfig::default() }; config.set_allowed_domains(vec!["allowed.example".to_string()]); - let proxy_config = RemoteNetworkProxyConfig::from_effective_config(&config) + let mut proxy_config = RemoteNetworkProxyConfig::from_effective_config(&config) .expect("supported remote proxy config"); + proxy_config.credential_masking = credential_masking; let params = ExecParams { metadata: Default::default(), process_id: ProcessId::from("process-remote-proxy"), - argv: vec!["echo".to_string(), "hello".to_string()], + argv: std::iter::once("/bin/bash") + .chain(shell_args.iter().copied()) + .map(str::to_string) + .collect(), cwd: PathUri::from_abs_path(&cwd), shell_snapshot: None, env_policy: None, @@ -535,22 +547,38 @@ async fn native_request_handles_remote_proxy_config_for_platform() { ), }; let stale_proxy = "http://127.0.0.1:9".to_string(); - let env = HashMap::from([ + let mut env = HashMap::from([ ("HTTP_PROXY".to_string(), stale_proxy.clone()), ("TEST_ENV".to_string(), "value".to_string()), + ("PATH".to_string(), "/devbox/bin:/usr/bin:/bin".to_string()), + ( + "OPENAI_API_KEY".to_string(), + "sk-proj-executor-only-0000000000000000000000000000000000".to_string(), + ), ( PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(), "foreign-token".to_string(), ), ]); + if let Some(key) = startup_env { + env.insert(key.to_string(), "startup-hook".to_string()); + } let prepared = prepare_exec_request( - ¶ms, env, /*runtime_paths*/ None, /*network_policy_decider*/ None, + ¶ms, + env.clone(), + /*runtime_paths*/ None, + /*network_policy_decider*/ None, /*network_policy_audit_observer*/ None, ) .await .expect("prepare request with executor-local proxy"); + assert_eq!(prepared.command, params.argv); + for key in ["PATH", "OPENAI_API_KEY"].into_iter().chain(startup_env) { + assert_eq!(prepared.env.get(key), env.get(key), "{key}"); + } + if cfg!(target_os = "windows") { assert_eq!(prepared.env.get("HTTP_PROXY"), None); assert_eq!(prepared.env.get("TEST_ENV"), Some(&"value".to_string())); @@ -596,13 +624,18 @@ async fn disabled_remote_proxy_config_is_rejected_before_exporting_ports() { .expect("current directory") .try_into() .expect("absolute cwd"); - let proxy_config = + let mut proxy_config = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig::default()) .expect("serializable disabled proxy config"); + proxy_config.credential_masking = true; let params = ExecParams { metadata: Default::default(), process_id: ProcessId::from("process-disabled-remote-proxy"), - argv: vec!["echo".to_string(), "hello".to_string()], + argv: vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "true".to_string(), + ], cwd: PathUri::from_abs_path(&cwd), shell_snapshot: None, env_policy: None, diff --git a/codex-rs/exec-server/src/shell_snapshot_tests.rs b/codex-rs/exec-server/src/shell_snapshot_tests.rs index 82a34ff739..66dc90e23e 100644 --- a/codex-rs/exec-server/src/shell_snapshot_tests.rs +++ b/codex-rs/exec-server/src/shell_snapshot_tests.rs @@ -101,6 +101,7 @@ async fn snapshot_failure_retries_are_bounded_and_single_flight( /*runtime_paths*/ None, /*network_policy_decider*/ None, /*network_policy_audit_observer*/ None, + &codex_network_proxy::RemoteCredentialBrokerCache::default(), &ProcessTelemetry::default(), ) .await @@ -111,6 +112,7 @@ async fn snapshot_failure_retries_are_bounded_and_single_flight( /*runtime_paths*/ None, /*network_policy_decider*/ None, /*network_policy_audit_observer*/ None, + &codex_network_proxy::RemoteCredentialBrokerCache::default(), &ProcessTelemetry::default(), ) .await diff --git a/codex-rs/exec-server/tests/chatgpt_cloudflare_affinity.rs b/codex-rs/exec-server/tests/chatgpt_cloudflare_affinity.rs index d754fa8c4b..25291910e1 100644 --- a/codex-rs/exec-server/tests/chatgpt_cloudflare_affinity.rs +++ b/codex-rs/exec-server/tests/chatgpt_cloudflare_affinity.rs @@ -1,19 +1,11 @@ #![cfg(unix)] mod common; +#[path = "common/tls_proxy.rs"] +mod tls_proxy; -use std::collections::BTreeMap; use std::ffi::OsString; use std::fs; -use std::io; -use std::io::Read; -use std::io::Write; -use std::net::TcpListener; -use std::net::TcpStream; -use std::sync::Arc; -use std::sync::mpsc; -use std::thread; -use std::time::Duration; use codex_exec_server::HttpRedirectPolicy; use codex_exec_server::HttpRequestParams; @@ -25,45 +17,14 @@ use codex_exec_server_protocol::RequestId; use common::exec_server::ExecServerHarness; use common::exec_server::exec_server_with_env; use pretty_assertions::assert_eq; -use rcgen::BasicConstraints; -use rcgen::CertificateParams; -use rcgen::CertifiedIssuer; -use rcgen::DistinguishedName; -use rcgen::DnType; -use rcgen::ExtendedKeyUsagePurpose; -use rcgen::IsCa; -use rcgen::KeyPair; -use rcgen::KeyUsagePurpose; -use rcgen::PKCS_ECDSA_P256_SHA256; -use rustls::pki_types::CertificateDer; -use rustls::pki_types::PrivateKeyDer; use serde::de::DeserializeOwned; use serde_json::Value; use tempfile::TempDir; +use tls_proxy::TlsInterceptingProxy; const CHATGPT_MCP_URL: &str = "https://chatgpt.com/backend-api/ps/mcp"; const NON_CHATGPT_MCP_URL: &str = "https://api.openai.com/backend-api/ps/mcp"; -#[derive(Debug)] -struct CapturedRequest { - connect_authority: String, - request_line: String, - headers: BTreeMap>, -} - -struct TlsMaterial { - ca_cert_pem: String, - server_cert: CertificateDer<'static>, - server_key: PrivateKeyDer<'static>, -} - -struct TlsInterceptingProxy { - ca_cert_pem: String, - request_rx: mpsc::Receiver>, - thread: thread::JoinHandle>, - url: String, -} - /// Exercises the same `http/request` route used by remotely executed Streamable HTTP MCP calls. /// Each RPC uses the shared route-aware client. The first response sets `__cflb`, and the second response /// replaces it, proving cross-client persistence through the shared cookie store. @@ -156,188 +117,6 @@ async fn exec_server_replays_only_chatgpt_cloudflare_cookies() -> anyhow::Result Ok(()) } -impl TlsInterceptingProxy { - fn start(expected_requests: usize) -> anyhow::Result { - codex_utils_rustls_provider::ensure_rustls_crypto_provider(); - let material = generate_tls_material()?; - let listener = TcpListener::bind(("127.0.0.1", 0))?; - let address = listener.local_addr()?; - let config = Arc::new( - rustls::ServerConfig::builder_with_protocol_versions(&[&rustls::version::TLS13]) - .with_no_client_auth() - .with_single_cert(vec![material.server_cert], material.server_key)?, - ); - let (request_tx, request_rx) = mpsc::channel(); - let thread = thread::spawn(move || { - run_tls_intercepting_proxy(listener, config, request_tx, expected_requests) - .map_err(|error| error.to_string()) - }); - - Ok(Self { - ca_cert_pem: material.ca_cert_pem, - request_rx, - thread, - url: format!("http://{address}"), - }) - } - - fn next_request(&self) -> anyhow::Result { - self.request_rx - .recv_timeout(Duration::from_secs(5)) - .map_err(anyhow::Error::from)? - .map_err(anyhow::Error::msg) - } - - fn finish(self) -> anyhow::Result<()> { - self.thread - .join() - .map_err(|_| anyhow::anyhow!("TLS proxy thread panicked"))? - .map_err(anyhow::Error::msg) - } -} - -fn generate_tls_material() -> anyhow::Result { - let mut ca_params = CertificateParams::default(); - ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); - ca_params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; - let mut ca_distinguished_name = DistinguishedName::new(); - ca_distinguished_name.push(DnType::CommonName, "Codex affinity test CA"); - ca_params.distinguished_name = ca_distinguished_name; - let ca_key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256)?; - let ca = CertifiedIssuer::self_signed(ca_params, ca_key_pair)?; - - let mut server_params = CertificateParams::new(vec![ - "chatgpt.com".to_string(), - "api.openai.com".to_string(), - ])?; - server_params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ServerAuth]; - server_params.key_usages = vec![ - KeyUsagePurpose::DigitalSignature, - KeyUsagePurpose::KeyEncipherment, - ]; - let server_key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256)?; - let server_cert = server_params.signed_by(&server_key_pair, &ca)?; - - Ok(TlsMaterial { - ca_cert_pem: ca.pem(), - server_cert: server_cert.der().clone(), - server_key: PrivateKeyDer::from(server_key_pair), - }) -} - -fn run_tls_intercepting_proxy( - listener: TcpListener, - config: Arc, - request_tx: mpsc::Sender>, - expected_requests: usize, -) -> io::Result<()> { - for request_index in 0..expected_requests { - let (mut stream, _) = listener.accept()?; - configure_stream(&stream)?; - let connect_head = read_http_head(&mut stream)?; - let connect_authority = connect_head - .lines() - .next() - .and_then(|line| line.split_whitespace().nth(1)) - .ok_or_else(|| io::Error::other("CONNECT request line should include an authority"))? - .to_string(); - stream.write_all(b"HTTP/1.1 200 Connection Established\r\n\r\n")?; - stream.flush()?; - - let connection = - rustls::ServerConnection::new(Arc::clone(&config)).map_err(io::Error::other)?; - let mut tls = rustls::StreamOwned::new(connection, stream); - let request = capture_http_request(&mut tls, connect_authority); - match request { - Ok(request) => request_tx - .send(Ok(request)) - .map_err(|_| io::Error::other("request receiver was dropped"))?, - Err(error) => { - let message = error.to_string(); - let _ = request_tx.send(Err(message)); - return Err(error); - } - } - - let response = match request_index { - 0 => concat!( - "HTTP/1.1 200 OK\r\n", - "content-length: 2\r\n", - "connection: close\r\n", - "set-cookie: __cflb=west; Path=/; Secure; HttpOnly\r\n", - "set-cookie: chatgpt_session=secret; Path=/; Secure; HttpOnly\r\n", - "\r\n", - "ok", - ), - 1 => concat!( - "HTTP/1.1 200 OK\r\n", - "content-length: 2\r\n", - "connection: close\r\n", - "set-cookie: __cflb=central; Path=/; Secure; HttpOnly\r\n", - "set-cookie: chatgpt_session=still-secret; Path=/; Secure; HttpOnly\r\n", - "\r\n", - "ok", - ), - _ => concat!( - "HTTP/1.1 200 OK\r\n", - "content-length: 2\r\n", - "connection: close\r\n", - "\r\n", - "ok", - ), - }; - tls.write_all(response.as_bytes())?; - tls.flush()?; - } - Ok(()) -} - -fn configure_stream(stream: &TcpStream) -> io::Result<()> { - stream.set_read_timeout(Some(Duration::from_secs(5)))?; - stream.set_write_timeout(Some(Duration::from_secs(5))) -} - -fn capture_http_request( - stream: &mut impl Read, - connect_authority: String, -) -> io::Result { - let request_head = read_http_head(stream)?; - let mut lines = request_head.lines(); - let request_line = lines - .next() - .ok_or_else(|| io::Error::other("HTTP request should include a request line"))? - .to_string(); - let mut headers: BTreeMap> = BTreeMap::new(); - for line in lines.filter(|line| !line.is_empty()) { - let (name, value) = line - .split_once(':') - .ok_or_else(|| io::Error::other(format!("invalid HTTP header: {line}")))?; - headers - .entry(name.to_ascii_lowercase()) - .or_default() - .push(value.trim().to_string()); - } - Ok(CapturedRequest { - connect_authority, - request_line, - headers, - }) -} - -fn read_http_head(stream: &mut impl Read) -> io::Result { - const MAX_HEADER_BYTES: usize = 64 * 1024; - let mut bytes = Vec::new(); - while !bytes.ends_with(b"\r\n\r\n") { - if bytes.len() == MAX_HEADER_BYTES { - return Err(io::Error::other("HTTP headers exceeded test limit")); - } - let mut byte = [0]; - stream.read_exact(&mut byte)?; - bytes.push(byte[0]); - } - String::from_utf8(bytes).map_err(io::Error::other) -} - async fn initialize_exec_server(server: &mut ExecServerHarness) -> anyhow::Result<()> { let initialize_id = server .send_request( diff --git a/codex-rs/exec-server/tests/common/tls_proxy.rs b/codex-rs/exec-server/tests/common/tls_proxy.rs new file mode 100644 index 0000000000..9ab5adbaec --- /dev/null +++ b/codex-rs/exec-server/tests/common/tls_proxy.rs @@ -0,0 +1,227 @@ +//! Local TLS CONNECT proxy shared by executor integration tests. + +use std::collections::BTreeMap; +use std::io; +use std::io::Read; +use std::io::Write; +use std::net::TcpListener; +use std::net::TcpStream; +use std::sync::Arc; +use std::sync::mpsc; +use std::thread; +use std::time::Duration; + +use rcgen::BasicConstraints; +use rcgen::CertificateParams; +use rcgen::CertifiedIssuer; +use rcgen::DistinguishedName; +use rcgen::DnType; +use rcgen::ExtendedKeyUsagePurpose; +use rcgen::IsCa; +use rcgen::KeyPair; +use rcgen::KeyUsagePurpose; +use rcgen::PKCS_ECDSA_P256_SHA256; +use rustls::pki_types::CertificateDer; +use rustls::pki_types::PrivateKeyDer; + +#[derive(Debug)] +pub(crate) struct CapturedRequest { + pub(crate) connect_authority: String, + pub(crate) request_line: String, + pub(crate) headers: BTreeMap>, +} + +struct TlsMaterial { + ca_cert_pem: String, + server_cert: CertificateDer<'static>, + server_key: PrivateKeyDer<'static>, +} + +pub(crate) struct TlsInterceptingProxy { + pub(crate) ca_cert_pem: String, + request_rx: mpsc::Receiver>, + thread: thread::JoinHandle>, + pub(crate) url: String, +} + +impl TlsInterceptingProxy { + pub(crate) fn start(expected_requests: usize) -> anyhow::Result { + codex_utils_rustls_provider::ensure_rustls_crypto_provider(); + let material = generate_tls_material()?; + let listener = TcpListener::bind(("127.0.0.1", 0))?; + let address = listener.local_addr()?; + let config = Arc::new( + rustls::ServerConfig::builder_with_protocol_versions(&[&rustls::version::TLS13]) + .with_no_client_auth() + .with_single_cert(vec![material.server_cert], material.server_key)?, + ); + let (request_tx, request_rx) = mpsc::channel(); + let thread = thread::spawn(move || { + run_tls_intercepting_proxy(listener, config, request_tx, expected_requests) + .map_err(|error| error.to_string()) + }); + + Ok(Self { + ca_cert_pem: material.ca_cert_pem, + request_rx, + thread, + url: format!("http://{address}"), + }) + } + + pub(crate) fn next_request(&self) -> anyhow::Result { + self.request_rx + .recv_timeout(Duration::from_secs(5)) + .map_err(anyhow::Error::from)? + .map_err(anyhow::Error::msg) + } + + pub(crate) fn finish(self) -> anyhow::Result<()> { + self.thread + .join() + .map_err(|_| anyhow::anyhow!("TLS proxy thread panicked"))? + .map_err(anyhow::Error::msg) + } +} + +fn generate_tls_material() -> anyhow::Result { + let mut ca_params = CertificateParams::default(); + ca_params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + ca_params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; + let mut ca_distinguished_name = DistinguishedName::new(); + ca_distinguished_name.push(DnType::CommonName, "Codex affinity test CA"); + ca_params.distinguished_name = ca_distinguished_name; + let ca_key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256)?; + let ca = CertifiedIssuer::self_signed(ca_params, ca_key_pair)?; + + let mut server_params = CertificateParams::new(vec![ + "chatgpt.com".to_string(), + "api.openai.com".to_string(), + ])?; + server_params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ServerAuth]; + server_params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyEncipherment, + ]; + let server_key_pair = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256)?; + let server_cert = server_params.signed_by(&server_key_pair, &ca)?; + + Ok(TlsMaterial { + ca_cert_pem: ca.pem(), + server_cert: server_cert.der().clone(), + server_key: PrivateKeyDer::from(server_key_pair), + }) +} + +fn run_tls_intercepting_proxy( + listener: TcpListener, + config: Arc, + request_tx: mpsc::Sender>, + expected_requests: usize, +) -> io::Result<()> { + for request_index in 0..expected_requests { + let (mut stream, _) = listener.accept()?; + configure_stream(&stream)?; + let connect_head = read_http_head(&mut stream)?; + let connect_authority = connect_head + .lines() + .next() + .and_then(|line| line.split_whitespace().nth(1)) + .ok_or_else(|| io::Error::other("CONNECT request line should include an authority"))? + .to_string(); + stream.write_all(b"HTTP/1.1 200 Connection Established\r\n\r\n")?; + stream.flush()?; + + let connection = + rustls::ServerConnection::new(Arc::clone(&config)).map_err(io::Error::other)?; + let mut tls = rustls::StreamOwned::new(connection, stream); + let request = capture_http_request(&mut tls, connect_authority); + match request { + Ok(request) => request_tx + .send(Ok(request)) + .map_err(|_| io::Error::other("request receiver was dropped"))?, + Err(error) => { + let message = error.to_string(); + let _ = request_tx.send(Err(message)); + return Err(error); + } + } + + let response = match request_index { + 0 => concat!( + "HTTP/1.1 200 OK\r\n", + "content-length: 2\r\n", + "connection: close\r\n", + "set-cookie: __cflb=west; Path=/; Secure; HttpOnly\r\n", + "set-cookie: chatgpt_session=secret; Path=/; Secure; HttpOnly\r\n", + "\r\n", + "ok", + ), + 1 => concat!( + "HTTP/1.1 200 OK\r\n", + "content-length: 2\r\n", + "connection: close\r\n", + "set-cookie: __cflb=central; Path=/; Secure; HttpOnly\r\n", + "set-cookie: chatgpt_session=still-secret; Path=/; Secure; HttpOnly\r\n", + "\r\n", + "ok", + ), + _ => concat!( + "HTTP/1.1 200 OK\r\n", + "content-length: 2\r\n", + "connection: close\r\n", + "\r\n", + "ok", + ), + }; + tls.write_all(response.as_bytes())?; + tls.flush()?; + } + Ok(()) +} + +fn configure_stream(stream: &TcpStream) -> io::Result<()> { + stream.set_read_timeout(Some(Duration::from_secs(5)))?; + stream.set_write_timeout(Some(Duration::from_secs(5))) +} + +fn capture_http_request( + stream: &mut impl Read, + connect_authority: String, +) -> io::Result { + let request_head = read_http_head(stream)?; + let mut lines = request_head.lines(); + let request_line = lines + .next() + .ok_or_else(|| io::Error::other("HTTP request should include a request line"))? + .to_string(); + let mut headers: BTreeMap> = BTreeMap::new(); + for line in lines.filter(|line| !line.is_empty()) { + let (name, value) = line + .split_once(':') + .ok_or_else(|| io::Error::other(format!("invalid HTTP header: {line}")))?; + headers + .entry(name.to_ascii_lowercase()) + .or_default() + .push(value.trim().to_string()); + } + Ok(CapturedRequest { + connect_authority, + request_line, + headers, + }) +} + +fn read_http_head(stream: &mut impl Read) -> io::Result { + const MAX_HEADER_BYTES: usize = 64 * 1024; + let mut bytes = Vec::new(); + while !bytes.ends_with(b"\r\n\r\n") { + if bytes.len() == MAX_HEADER_BYTES { + return Err(io::Error::other("HTTP headers exceeded test limit")); + } + let mut byte = [0]; + stream.read_exact(&mut byte)?; + bytes.push(byte[0]); + } + String::from_utf8(bytes).map_err(io::Error::other) +} diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index 956873ee56..3c74a7e176 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -2,6 +2,9 @@ mod common; #[cfg(unix)] #[path = "exec_process/shell_snapshot.rs"] mod shell_snapshot; +#[cfg(unix)] +#[path = "common/tls_proxy.rs"] +mod tls_proxy; #[path = "exec_process/windows_sandbox.rs"] mod windows_sandbox; @@ -492,19 +495,87 @@ async fn exec_metadata_overrides_policy_and_cached_snapshot( } #[cfg(unix)] +#[test_case(false, "bash", &["-lc", "-lc"]; "unmasked")] +#[cfg_attr(any(target_os = "linux", target_os = "macos"), test_case(true, "bash", &["-c", "-c", "-lc"]; "masked_bash"))] +#[cfg_attr(any(target_os = "linux", target_os = "macos"), test_case(true, "bash", &["-lc", "-lc"]; "bash_profile_fallback"))] +#[cfg_attr(target_os = "macos", test_case(true, "zsh", &["-lc", "-lc"]; "zsh_profile_gateway_fallback"))] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[serial_test::serial(remote_exec_server)] -async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context() -> Result<()> { - let context = create_process_context(/*use_remote*/ true).await?; +async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context( + credential_masking: bool, + shell_name: &str, + shell_args: &[&str], +) -> Result<()> { + const API_KEY: &str = "sk-proj-executor-only-0000000000000000000000000000000000"; + let shell_path = format!("/bin/{shell_name}"); + let api_host = if shell_name == "zsh" { + "chatgpt.com" + } else { + "api.openai.com" + }; let home = TempDir::new()?; + if credential_masking + && let Some(warning) = codex_sandboxing::system_bwrap_warning( + &PermissionProfile::workspace_write(), + home.path(), + ) + { + eprintln!("skipping sandbox test: {warning}"); + return Ok(()); + } + let upstream = credential_masking + .then(|| tls_proxy::TlsInterceptingProxy::start(shell_args.len())) + .transpose()?; + let mut executor_env = vec![("OPENAI_API_KEY", API_KEY.to_string())]; + if let Some(upstream) = &upstream { + let ca_path = home.path().join("upstream-ca.pem"); + std::fs::write(&ca_path, &upstream.ca_cert_pem)?; + executor_env.extend([ + ("AUTH_ALIAS", format!("Bearer {API_KEY}")), + ("OPENAI_BASE_URL", "https://api.openai.com/v1".to_string()), + ("GH_HOST", String::new()), + ( + "GH_ENTERPRISE_TOKEN", + "executor-unbound-enterprise-token".to_string(), + ), + ( + "CODEX_CA_CERTIFICATE", + ca_path.to_string_lossy().into_owned(), + ), + ("CURL_CA_BUNDLE", ca_path.to_string_lossy().into_owned()), + ("HTTPS_PROXY", upstream.url.clone()), + ("https_proxy", upstream.url.clone()), + ("ALL_PROXY", upstream.url.clone()), + ("all_proxy", upstream.url.clone()), + ("NO_PROXY", String::new()), + ("no_proxy", String::new()), + ]); + } + let mut server = common::exec_server::exec_server_with_env(executor_env, &[]).await?; + let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?; + let backend = environment.get_exec_backend(); let cwd = PathUri::from_host_native_path(home.path())?; - std::fs::write( - home.path().join(".bashrc"), - "printf '%s\\n' \"$HTTP_PROXY\" >> \"$HOME/captures\"\ntest \"$CODEX_NETWORK_PROXY_ACTIVE\" = 1 || exit 41\nexport PROFILE_ALLOWED=profile\nprofile_helper() { printf helper; }\n", + let profile_bin = home.path().join("profile-bin"); + std::fs::create_dir(&profile_bin)?; + codex_utils_cargo_bin::write_executable( + &profile_bin.join("profile_command"), + "#!/bin/sh\nprintf helper\n", )?; - let policy = ExecEnvPolicy { + let profile = "printf '%s\\n' \"$HTTP_PROXY\" >> \"$HOME/captures\"\ntest \"$CODEX_NETWORK_PROXY_ACTIVE\" = 1 || exit 41\nexport PROFILE_ALLOWED=profile\nprofile_helper() { profile_command; }\n"; + std::fs::write(home.path().join(".bashrc"), profile)?; + std::fs::write( + home.path().join(".bash_profile"), + "export PATH=\"$HOME/profile-bin:$PATH\"\n", + )?; + std::fs::write( + home.path().join(".zshenv"), + format!( + "export PATH=\"$HOME/profile-bin:$PATH\"\nexport OPENAI_BASE_URL=https://{api_host}/v1\n{profile}" + ), + )?; + let mut policy = ExecEnvPolicy { inherit: ShellEnvironmentPolicyInherit::All, - ignore_default_excludes: false, + ignore_default_excludes: true, exclude: Vec::new(), r#set: HashMap::from([( "HOME".to_string(), @@ -514,39 +585,77 @@ async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context( "HOME".to_string(), "PATH".to_string(), "PROFILE_*".to_string(), + "OPENAI_API_KEY".to_string(), ], }; - let proxy_config = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig { + if credential_masking { + policy.include_only.extend( + [ + "AUTH_ALIAS", + "GH_HOST", + "GH_ENTERPRISE_TOKEN", + "CURL_CA_BUNDLE", + ] + .map(str::to_string), + ); + } + let mut proxy_settings = NetworkProxyConfig { enabled: true, ..NetworkProxyConfig::default() - })?; + }; + proxy_settings.set_allowed_domains(vec![api_host.to_string()]); + let mut proxy_config = RemoteNetworkProxyConfig::from_effective_config(&proxy_settings)?; + proxy_config.credential_masking = credential_masking; let mut proxy_addresses = Vec::new(); + let mut saved_api_key = None; - for attempt in 0..2 { - let started = context - .backend + for (attempt, &shell_args) in shell_args.iter().enumerate() { + let use_snapshot = shell_name == "bash" && shell_args == "-lc"; + let expect_masking = credential_masking && shell_name == "bash" && !use_snapshot; + let expect_profile = use_snapshot || shell_name == "zsh"; + let command = if expect_profile { + "profile_helper; printf '|%s|%s|%s' \"$PROFILE_ALLOWED\" \"$CODEX_NETWORK_PROXY_ACTIVE\" \"$HTTP_PROXY\"" + } else { + "printf 'masked|%s|%s' \"$CODEX_NETWORK_PROXY_ACTIVE\" \"$HTTP_PROXY\"" + }; + let command = if credential_masking { + format!( + "{command}; test \"$GH_ENTERPRISE_TOKEN\" = executor-unbound-enterprise-token || exit 45; test \"$AUTH_ALIAS\" = \"Bearer $OPENAI_API_KEY\" || exit 42; printf '|%s' \"$OPENAI_API_KEY\"; curl --silent --show-error --fail --max-time 5 --noproxy '' --config \"$HOME/curl.conf\" \"${{OPENAI_BASE_URL:-https://api.openai.com/v1}}/models\" >/dev/null" + ) + } else { + format!("{command}; test \"$OPENAI_API_KEY\" = '{API_KEY}' || exit 46") + }; + let command = if credential_masking && attempt == 0 { + format!( + "printf 'header = \"Authorization: %s\"\\n' \"$AUTH_ALIAS\" > \"$HOME/curl.conf\" || exit 44; {command}" + ) + } else { + command + }; + let started = backend .start(ExecParams { metadata: Default::default(), process_id: ProcessId::from(format!("managed-snapshot-{attempt}")), - argv: vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "profile_helper; printf '|%s|%s|%s' \"$PROFILE_ALLOWED\" \"$CODEX_NETWORK_PROXY_ACTIVE\" \"$HTTP_PROXY\"".to_string(), - ], + argv: vec![shell_path.clone(), shell_args.to_string(), command], cwd: cwd.clone(), env_policy: Some(policy.clone()), - shell_snapshot: Some(ShellSnapshotRequest { + shell_snapshot: use_snapshot.then(|| ShellSnapshotRequest { scope_id: "managed-attachment".to_string(), shell: ShellInfo { - name: "bash".to_string(), - path: "/bin/bash".to_string(), + name: shell_name.to_string(), + path: shell_path.clone(), }, }), env: HashMap::new(), tty: false, pipe_stdin: false, arg0: None, - sandbox: None, + sandbox: credential_masking.then(|| { + FileSystemSandboxContext::from_permission_profile( + PermissionProfile::workspace_write(), + cwd.clone(), + ) + }), enforce_managed_network: true, managed_network: None, network_proxy: Some( @@ -559,18 +668,72 @@ async fn shell_snapshot_v2_remote_managed_proxy_uses_prepared_execution_context( .await?; let (stdout, stderr, status, closed) = collect_process_output_from_events(started.process).await?; - let proxy_address = stdout - .strip_prefix("helper|profile|1|") - .context("snapshot should restore profile functions and live proxy state")?; - assert!(proxy_address.starts_with("http://127.0.0.1:")); assert_eq!((stderr, status, closed), (String::new(), Some(0), true)); + let prefix = if expect_profile { + "helper|profile|1|" + } else { + "masked|1|" + }; + let proxy_state = stdout + .strip_prefix(prefix) + .context("command should preserve live proxy state")?; + let proxy_address = if let Some(upstream) = &upstream { + let (proxy_address, child_api_key) = proxy_state + .split_once('|') + .context("command should expose the child API key")?; + if expect_masking { + assert!(child_api_key.starts_with("sk-proj-")); + assert_ne!(child_api_key, API_KEY); + assert!(!stdout.contains(API_KEY)); + } else { + assert_eq!(child_api_key, API_KEY); + } + let saved_api_key = saved_api_key.get_or_insert_with(|| child_api_key.to_string()); + assert_eq!( + std::fs::read_to_string(home.path().join("curl.conf"))?, + format!("header = \"Authorization: Bearer {saved_api_key}\"\n") + ); + let request = upstream.next_request()?; + assert_eq!( + ( + request.connect_authority.as_str(), + request.request_line.as_str(), + request.headers.get("authorization"), + ), + ( + format!("{api_host}:443").as_str(), + "GET /v1/models HTTP/1.1", + Some(&vec![format!("Bearer {API_KEY}")]), + ) + ); + proxy_address + } else { + proxy_state + }; + assert!(proxy_address.starts_with("http://127.0.0.1:")); proxy_addresses.push(proxy_address.to_string()); } - assert_eq!( - std::fs::read_to_string(home.path().join("captures"))?, - format!("{}\n", proxy_addresses[0]) - ); + let expected_captures = if shell_name == "zsh" { + Some(format!("{}\n", proxy_addresses.join("\n"))) + } else { + shell_args + .iter() + .position(|flag| *flag == "-lc") + .map(|attempt| format!("{}\n", proxy_addresses[attempt])) + }; + if let Some(expected_captures) = expected_captures { + assert_eq!( + std::fs::read_to_string(home.path().join("captures"))?, + expected_captures + ); + } else { + assert!(!home.path().join("captures").exists()); + } + if let Some(upstream) = upstream { + upstream.finish()?; + } + server.shutdown().await?; Ok(()) } diff --git a/codex-rs/network-proxy/src/credential_broker.rs b/codex-rs/network-proxy/src/credential_broker.rs index 57805c446d..7e8c019921 100644 --- a/codex-rs/network-proxy/src/credential_broker.rs +++ b/codex-rs/network-proxy/src/credential_broker.rs @@ -45,6 +45,17 @@ pub(crate) const BROKERED_CREDENTIALS_ENV_KEY: &str = "CODEX_NETWORK_PROXY_BROKE const MIN_EMBEDDED_CREDENTIAL_LENGTH: usize = 16; const BROKERED_CREDENTIAL_ALIAS_MARKER_PREFIX: &str = "@alias:"; +/// How to handle recognized credentials left raw in a prepared command environment. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum CredentialMaskingFailureMode { + /// Warn and keep the unmasked values. + Warn, + /// Remove variables containing unmasked values, including copied aliases. + Deny, + /// Reject the command environment. + Error, +} + #[derive(Clone)] pub(crate) struct CredentialBroker { state: Arc>, @@ -1120,6 +1131,59 @@ impl CredentialBroker { matching::virtualize_text(&state, text, env) } + pub(crate) fn handle_unmasked_credentials( + &self, + env: &mut HashMap, + mode: CredentialMaskingFailureMode, + ) -> anyhow::Result<()> { + let state = self.read_state(); + anyhow::ensure!( + state.enabled, + "credential masking requires an enabled credential broker" + ); + let mut unmasked_keys = Vec::new(); + for (key, value) in env.iter() { + let value = value.trim(); + let known = matching::known_credential_matches(&state, value, env); + let contains_real_credential = known + .iter() + .any(|matched| matched.value == matched.real_value); + let remainder = matching::mask_known_credentials(value, &known); + let mut masked = remainder.clone(); + if contains_real_credential + || !matching::virtualize_text(&state, &mut masked, env) + || masked != remainder + { + unmasked_keys.push(key.clone()); + } + } + drop(state); + if unmasked_keys.is_empty() { + return Ok(()); + } + match mode { + CredentialMaskingFailureMode::Warn => { + tracing::warn!( + "Credential masking could not protect some environment credentials; continuing with those values unmasked." + ); + } + CredentialMaskingFailureMode::Deny => { + for key in unmasked_keys { + env.remove(&key); + } + tracing::warn!( + "Credential masking could not protect some environment credentials; removed those variables before execution." + ); + } + CredentialMaskingFailureMode::Error => { + anyhow::bail!( + "credential masking left an unmasked credential in the command environment" + ); + } + } + Ok(()) + } + pub(crate) fn restore_text(&self, text: &mut String) -> bool { let state = self.read_state(); if !state.enabled { diff --git a/codex-rs/network-proxy/src/credential_broker_tests.rs b/codex-rs/network-proxy/src/credential_broker_tests.rs index 43b62632fe..fc7c356725 100644 --- a/codex-rs/network-proxy/src/credential_broker_tests.rs +++ b/codex-rs/network-proxy/src/credential_broker_tests.rs @@ -46,6 +46,62 @@ fn assert_credential_shape(real_value: &str, dummy_value: &str, prefix: &str) { assert!(same_shape); } +#[test] +fn unmasked_credentials_follow_the_selected_failure_mode() { + for (enabled, source, base_url, expected) in [ + (true, "GH_TOKEN", None, true), + (true, "GH_ENTERPRISE_TOKEN", None, false), + ( + true, + "OPENAI_API_KEY", + Some("https://localhost:1234/v1"), + false, + ), + (false, "GH_TOKEN", None, false), + ] { + let broker = CredentialBroker::new(enabled); + broker.configure(&NetworkProxyConfig { + credential_broker: enabled, + allow_local_binding: Some(true), + ..NetworkProxyConfig::default() + }); + let mut env = env_map([ + (source, "opaque-executor-credential"), + ("AUTH_HEADER", "Bearer opaque-executor-credential"), + ]); + if let Some(base_url) = base_url { + env.insert("OPENAI_BASE_URL".to_string(), base_url.to_string()); + } + broker.virtualize_child_env(&mut env); + for alias_only in [false, true] { + let mut input = env.clone(); + if alias_only { + input.remove(source); + } + for mode in [ + CredentialMaskingFailureMode::Warn, + CredentialMaskingFailureMode::Deny, + CredentialMaskingFailureMode::Error, + ] { + let mut output = input.clone(); + assert_eq!( + broker + .handle_unmasked_credentials(&mut output, mode) + .is_ok(), + enabled && (expected || mode != CredentialMaskingFailureMode::Error), + "{source} {mode:?} (alias_only={alias_only})" + ); + let mut expected_env = input.clone(); + if enabled && !expected && mode == CredentialMaskingFailureMode::Deny { + expected_env.remove(source); + expected_env.remove("AUTH_HEADER"); + } + assert_eq!(output, expected_env); + } + } + } +} + #[test] fn virtualize_child_env_replaces_supported_credentials() { let broker = CredentialBroker::new(/*enabled*/ true); diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 451d956da3..eeec200cf0 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -54,6 +54,7 @@ pub use credential_broker::CREDENTIAL_BROKER_ACTIVE_ENV_KEY; pub use credential_broker::CredentialAuthMethod; pub use credential_broker::CredentialBrokerContext; pub use credential_broker::CredentialBrokerEnvironment; +pub use credential_broker::CredentialMaskingFailureMode; pub use credential_broker::CredentialProviderConfig; pub use credential_broker::brokered_credential_binding_env_keys; pub use credential_broker::brokered_credential_dummy_env_keys; @@ -105,6 +106,7 @@ pub use proxy::has_proxy_url_env_vars; pub use proxy::is_managed_proxy_env_var; pub use proxy::proxy_url_env_value; pub use proxy::strip_managed_proxy_env; +pub use remote_config::RemoteCredentialBrokerCache; pub use remote_config::RemoteNetworkProxyConfig; pub use remote_config::RemoteNetworkProxyLaunchConfig; pub use request_cancellation::NetworkRequestCancellation; diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index a862bac6d9..ac69847ae4 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -933,13 +933,27 @@ impl NetworkProxy { } /// Captures the static inputs needed to launch a matching executor-local proxy. + /// Credential masking requires an explicit caller opt-in and a capable executor. pub async fn remote_launch_config( &self, local_binding_policy: crate::LocalBindingPolicy, + credential_masking: bool, ) -> Result { let (mut config, brokerage_created_default_allowlist, _) = self.state.current_cfg_with_brokerage_provenance().await?; - // Proxy enablement and credential brokerage remain controller-owned. + // Configured providers are not transported yet, so these commands run unmasked. + let credential_masking = credential_masking + && config.enabled + && config.credential_broker + && config.credential_providers.is_empty(); + anyhow::ensure!( + !credential_masking + || (!config.dangerously_allow_plaintext_credential_injection + && config.mitm_hooks.is_empty() + && config.mitm_ca.is_none()), + "executor credential masking does not support plaintext injection, MITM hooks, or external CAs" + ); + // Legacy controller-only brokerage must not enable a remote proxy by itself. let mut broker_only_config = config::NetworkProxyConfig { enabled: config.enabled, allow_local_binding: config.allow_local_binding.filter(|binding| !binding), @@ -973,7 +987,7 @@ impl NetworkProxy { .as_ref() .and_then(|scope| scope.environment_policy.as_ref()); if config.credential_broker { - config.enabled &= !broker_only; + config.enabled &= !broker_only || credential_masking; config.credential_broker = false; config.dangerously_allow_plaintext_credential_injection = false; if config.mitm && config.mitm_hooks.is_empty() && config.mitm_ca.is_none() { @@ -994,7 +1008,8 @@ impl NetworkProxy { || config.allow_local_binding(), "MXC cannot enforce allow_local_binding=false: native host-loopback access is bidirectional; use true or a different sandbox backend" ); - let proxy = crate::RemoteNetworkProxyConfig::from_effective_config(&config)?; + let mut proxy = crate::RemoteNetworkProxyConfig::from_effective_config(&config)?; + proxy.credential_masking = credential_masking; tracing::debug!(network_config = ?proxy, "resolved remote execution network configuration"); let (environment_id, execution_id) = self .execution_scope @@ -1149,11 +1164,13 @@ impl NetworkProxy { runtime_settings.allow_local_binding, runtime_settings.mitm_ca_trust_bundle.as_ref(), ); - let credential_environment_id = environment_id.or_else(|| { - self.execution_scope - .as_ref() - .map(|scope| scope.environment_id.as_str()) - }); + let credential_environment_id = environment_id + .or_else(|| { + self.execution_scope + .as_ref() + .map(|scope| scope.environment_id.as_str()) + }) + .or(self.state.environment_id()); self.state .virtualize_child_credentials_for_environment(&mut env, credential_environment_id); if let Some(execution_scope) = self.execution_scope.as_ref() { @@ -1288,6 +1305,15 @@ impl NetworkProxy { self.state.virtualize_brokered_text(text, env) } + /// Applies the selected failure mode to recognized credentials left unmasked after preparation. + pub fn handle_unmasked_credentials( + &self, + env: &mut HashMap, + mode: crate::CredentialMaskingFailureMode, + ) -> anyhow::Result<()> { + self.state.handle_unmasked_credentials(env, mode) + } + /// Returns trusted provider metadata and active bindings for a child environment. pub fn credential_broker_environment( &self, @@ -1918,7 +1944,9 @@ mod tests { ); assert_eq!(proxy.current_cfg().await?, config); - let remote = proxy.remote_launch_config(DefaultFalse).await?; + let remote = proxy + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) + .await?; assert_eq!( ( remote.proxy.unix_sockets, @@ -2015,7 +2043,7 @@ mod tests { owner_config.set_denied_domains(vec!["denied.example".to_string()]); assert_eq!( scoped - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) .await? .proxy .domains, @@ -2432,7 +2460,7 @@ mod tests { "GH_HOST".to_string(), "local-github.example".to_string(), )])); - let state = Arc::new(network_proxy_state_for_policy(config)); + let state = Arc::new(network_proxy_state_for_policy(config.clone())); let proxy = match NetworkProxy::builder().state(state).build().await { Ok(proxy) => proxy, Err(err) => { @@ -2453,7 +2481,9 @@ mod tests { /*environment_policy*/ None, /*fallback_policy_decider*/ None, )?; - let launch = scoped.remote_launch_config(RequireTrue).await?; + let launch = scoped + .remote_launch_config(RequireTrue, /*credential_masking*/ false) + .await?; let prepared = scoped.prepare_for_optional_environment( HashMap::from([( PROXY_ATTRIBUTION_TOKEN_ENV_KEY.to_string(), @@ -2465,6 +2495,34 @@ mod tests { assert_eq!(launch.environment_id.as_deref(), Some("remote-env")); assert_eq!(launch.execution_id.as_deref(), Some("execution-1")); assert!(!launch.proxy.enabled); + assert!( + scoped + .remote_launch_config(DefaultFalse, /*credential_masking*/ true) + .await + .is_err() + ); + + config.credential_providers.insert( + "vendor".to_string(), + crate::CredentialProviderConfig { + env: vec!["VENDOR_TOKEN".to_string()], + patterns: vec!["vendor_[a-z]{8}".to_string()], + url_prefixes: vec!["https://api.vendor.example".to_string()], + auth: vec![crate::CredentialAuthMethod::Bearer], + ..crate::CredentialProviderConfig::default() + }, + ); + let custom_provider_proxy = NetworkProxy::builder() + .state(Arc::new(network_proxy_state_for_policy(config))) + .build() + .await?; + assert_eq!( + custom_provider_proxy + .remote_launch_config(DefaultFalse, /*credential_masking*/ true) + .await? + .proxy, + launch.proxy + ); let mut owner_config = NetworkProxyConfig::default(); owner_config.set_allowed_domains(vec!["owner.example".to_string()]); @@ -2480,7 +2538,7 @@ mod tests { )?; assert!( owner_scoped - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, /*credential_masking*/ false) .await .is_err() ); @@ -2512,9 +2570,14 @@ mod tests { .state(Arc::new(network_proxy_state_for_policy(config))) .build() .await?; - let launch = proxy.remote_launch_config(DefaultFalse).await?; - assert!(launch.proxy.enabled); - assert_eq!(launch.proxy.mode, mode); + for credential_masking in [false, true] { + let launch = proxy + .remote_launch_config(DefaultFalse, credential_masking) + .await?; + assert!(launch.proxy.enabled); + assert_eq!(launch.proxy.mode, mode); + assert_eq!(launch.proxy.credential_masking, credential_masking); + } } assert_eq!( prepared diff --git a/codex-rs/network-proxy/src/remote_config.rs b/codex-rs/network-proxy/src/remote_config.rs index acb698e178..5260ef6b00 100644 --- a/codex-rs/network-proxy/src/remote_config.rs +++ b/codex-rs/network-proxy/src/remote_config.rs @@ -2,12 +2,72 @@ use anyhow::Result; use anyhow::ensure; use serde::Deserialize; use serde::Serialize; +use std::sync::Mutex; use crate::NetworkDomainPermissions; use crate::NetworkMode; use crate::NetworkProxyAuditMetadata; use crate::NetworkProxyConfig; use crate::NetworkUnixSocketPermissions; +use crate::credential_broker::CredentialBroker; + +/// Credential mappings retained by one executor session across process launches. +/// Only mappings are shared; proxy listeners, network policy, and approvals remain per command. +#[derive(Default)] +pub struct RemoteCredentialBrokerCache { + brokers: Mutex>, +} + +struct CachedCredentialBroker { + environment_id: Option, + conversation_id: Option, + config: NetworkProxyConfig, + broker: CredentialBroker, +} + +impl RemoteCredentialBrokerCache { + pub(crate) fn broker_if_enabled( + &self, + environment_id: Option<&str>, + conversation_id: Option<&str>, + config: &NetworkProxyConfig, + ) -> Option { + if !config.credential_broker { + return None; + } + // Retain broker configuration and the command's destination context so ordinary + // network-policy changes preserve placeholders, while destination changes cannot + // invalidate another command's mappings. + let config = NetworkProxyConfig { + credential_broker: config.credential_broker, + allow_local_binding: Some(config.allow_local_binding()), + credential_broker_openai_host: config.credential_broker_openai_host.clone(), + credential_broker_context: config.credential_broker_context.clone(), + credential_providers: config.credential_providers.clone(), + ..NetworkProxyConfig::default() + }; + let mut brokers = self + .brokers + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(cached) = brokers.iter().find(|cached| { + cached.environment_id.as_deref() == environment_id + && cached.conversation_id.as_deref() == conversation_id + && cached.config == config + }) { + return Some(cached.broker.clone()); + } + let broker = CredentialBroker::new(config.credential_broker); + broker.configure(&config); + brokers.push(CachedCredentialBroker { + environment_id: environment_id.map(str::to_owned), + conversation_id: conversation_id.map(str::to_owned), + config, + broker: broker.clone(), + }); + Some(broker) + } +} /// Executor-local proxy launch inputs transported with one process start. /// @@ -55,13 +115,15 @@ impl RemoteNetworkProxyLaunchConfig { /// Effective network proxy settings that are safe to send to a remote executor. /// /// Listener addresses are deliberately omitted because the executor chooses its own loopback -/// ports. MITM, credential injection, and hooks are not represented so their configuration cannot -/// cross the exec-server boundary accidentally. +/// ports. Masking carries only built-in broker activation; credentials, destination context, custom +/// providers, CA configuration, and MITM hooks stay on their execution host. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] #[non_exhaustive] pub struct RemoteNetworkProxyConfig { pub enabled: bool, + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub credential_masking: bool, pub enable_socks5: bool, pub enable_socks5_udp: bool, pub allow_upstream_proxy: bool, @@ -85,6 +147,7 @@ impl RemoteNetworkProxyConfig { ); Ok(Self { enabled: config.enabled, + credential_masking: false, enable_socks5: config.enable_socks5, enable_socks5_udp: config.enable_socks5_udp, allow_upstream_proxy: config.allow_upstream_proxy, @@ -99,7 +162,7 @@ impl RemoteNetworkProxyConfig { } pub(crate) fn into_network_proxy_config(self) -> NetworkProxyConfig { - NetworkProxyConfig { + let mut config = NetworkProxyConfig { enabled: self.enabled, enable_socks5: self.enable_socks5, enable_socks5_udp: self.enable_socks5_udp, @@ -110,7 +173,9 @@ impl RemoteNetworkProxyConfig { unix_sockets: self.unix_sockets, allow_local_binding: Some(self.allow_local_binding), ..NetworkProxyConfig::default() - } + }; + config.set_credential_broker_enabled(self.credential_masking); + config } } diff --git a/codex-rs/network-proxy/src/remote_config_tests.rs b/codex-rs/network-proxy/src/remote_config_tests.rs index 9522fe1ecd..e589868be7 100644 --- a/codex-rs/network-proxy/src/remote_config_tests.rs +++ b/codex-rs/network-proxy/src/remote_config_tests.rs @@ -47,11 +47,13 @@ fn optional_socket_policy_preserves_input_and_resolves_at_remote_boundary() { #[tokio::test] async fn round_trip_preserves_supported_effective_settings() { - for (executor_os, socket) in [ - (Platform::Linux, "/var/run/example.sock"), - (Platform::Macos, "/var/run/example.sock"), - (Platform::Windows, r"C:\example.sock"), - (Platform::Unknown, r"C:\example.sock"), + for (executor_os, socket, credential_masking) in [ + (Platform::Linux, "/var/run/example.sock", false), + (Platform::Linux, "/var/run/example.sock", true), + (Platform::Macos, "/var/run/example.sock", false), + (Platform::Macos, "/var/run/example.sock", true), + (Platform::Windows, r"C:\example.sock", false), + (Platform::Unknown, r"C:\example.sock", false), ] { let mut config = NetworkProxyConfig { enabled: true, @@ -67,8 +69,10 @@ async fn round_trip_preserves_supported_effective_settings() { config.set_denied_domains(vec!["blocked.example.com".into()]); config.set_allow_unix_sockets(vec![socket.into()]); - let remote = RemoteNetworkProxyConfig::from_effective_config(&config) + let mut remote = RemoteNetworkProxyConfig::from_effective_config(&config) .expect("supported remote config"); + remote.credential_masking = credential_masking; + config.set_credential_broker_enabled(credential_masking); let round_trip = remote.clone().into_network_proxy_config(); assert_eq!(round_trip, config); @@ -77,6 +81,8 @@ async fn round_trip_preserves_supported_effective_settings() { NetworkProxyState::from_remote_launch_config( RemoteNetworkProxyLaunchConfig::new(remote), executor_os, + &crate::RemoteCredentialBrokerCache::default(), + &std::collections::HashMap::new(), ) .unwrap(), ); @@ -96,7 +102,7 @@ async fn round_trip_preserves_supported_effective_settings() { ); assert_eq!( proxy - .remote_launch_config(DefaultFalse) + .remote_launch_config(DefaultFalse, credential_masking) .await .unwrap() .proxy @@ -210,6 +216,8 @@ fn launch_config_materializes_audit_and_execution_attribution() { policy_decision_timeout_ms: None, }, Platform::native(), + &crate::RemoteCredentialBrokerCache::default(), + &std::collections::HashMap::new(), ) .expect("remote launch state"); @@ -218,6 +226,104 @@ fn launch_config_materializes_audit_and_execution_attribution() { assert_eq!(state.execution_id().as_deref(), Some("execution-1")); } +#[test] +fn remote_credential_mappings_follow_the_environment_and_broker_config() { + const TOKEN: &str = "ghp_abcdefghijklmnopqrstuvwxyz0123456789"; + let brokers = crate::RemoteCredentialBrokerCache::default(); + let launch = |environment_id: &str, + conversation_id: &str, + mode, + allow_local_binding, + env: &std::collections::HashMap| { + let mut proxy = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig { + enabled: true, + allow_local_binding: Some(allow_local_binding), + mode, + ..NetworkProxyConfig::default() + }) + .unwrap(); + proxy.credential_masking = true; + NetworkProxyState::from_remote_launch_config( + RemoteNetworkProxyLaunchConfig::new(proxy) + .for_execution(environment_id.to_string(), "execution".to_string()) + .with_audit_metadata(NetworkProxyAuditMetadata { + conversation_id: Some(conversation_id.to_string()), + ..NetworkProxyAuditMetadata::default() + }), + Platform::Linux, + &brokers, + env, + ) + .unwrap() + }; + let mut env = std::collections::HashMap::from([ + ("GH_ENTERPRISE_TOKEN".to_string(), TOKEN.to_string()), + ( + "GH_HOST".to_string(), + "github.enterprise.example".to_string(), + ), + ]); + let first = launch( + "remote", + "thread", + NetworkMode::Full, + /*allow_local_binding*/ false, + &env, + ); + first.virtualize_child_credentials(&mut env); + let placeholder = env["GH_ENTERPRISE_TOKEN"].clone(); + assert_ne!(placeholder, TOKEN); + + let mut next_env = std::collections::HashMap::from([ + ("GH_ENTERPRISE_TOKEN".to_string(), TOKEN.to_string()), + ("GH_HOST".to_string(), String::new()), + ]); + let next = launch( + "remote", + "thread", + NetworkMode::Full, + /*allow_local_binding*/ false, + &next_env, + ); + next.virtualize_child_credentials(&mut next_env); + let mut headers = rama_http::HeaderMap::from_iter([( + rama_http::header::AUTHORIZATION, + rama_http::HeaderValue::from_str(&format!("Bearer {placeholder}")).unwrap(), + )]); + first.inject_request_credentials("github.enterprise.example", &mut headers); + assert_eq!( + headers[rama_http::header::AUTHORIZATION].to_str().unwrap(), + format!("Bearer {TOKEN}") + ); + drop(first); + + for (environment_id, conversation_id, allow_local_binding, expected) in [ + ("remote", "thread", false, TOKEN), + ("other", "thread", false, placeholder.as_str()), + ("remote", "other", false, placeholder.as_str()), + ("remote", "thread", true, placeholder.as_str()), + ] { + // A different network mode must not discard mappings for the same scope. + let state = launch( + environment_id, + conversation_id, + NetworkMode::Limited, + allow_local_binding, + &env, + ); + let mut headers = rama_http::HeaderMap::new(); + headers.insert( + rama_http::header::AUTHORIZATION, + rama_http::HeaderValue::from_str(&format!("Bearer {placeholder}")).unwrap(), + ); + state.inject_request_credentials("github.enterprise.example", &mut headers); + assert_eq!( + headers[rama_http::header::AUTHORIZATION].to_str().unwrap(), + format!("Bearer {expected}") + ); + } +} + #[test] fn policy_decision_callback_timeout_round_trips() { let config = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig::default()) @@ -258,7 +364,7 @@ async fn local_binding_is_resolved_for_each_executor() -> anyhow::Result<()> { .await?; assert_eq!( proxy - .remote_launch_config(policy) + .remote_launch_config(policy, /*credential_masking*/ false) .await .ok() .map(|launch| launch.proxy.allow_local_binding), diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index c816bc358e..63d50400b7 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -334,6 +334,8 @@ impl NetworkProxyState { pub fn from_remote_launch_config( launch: crate::RemoteNetworkProxyLaunchConfig, executor_os: crate::Platform, + credential_brokers: &crate::RemoteCredentialBrokerCache, + environment: &HashMap, ) -> Result { let crate::RemoteNetworkProxyLaunchConfig { proxy, @@ -346,9 +348,15 @@ impl NetworkProxyState { proxy.enabled, "executor-local network proxy launch requires an enabled proxy" ); - let config = proxy.into_network_proxy_config(); + let mut config = proxy.into_network_proxy_config(); + config.configure_credential_broker_environment(environment); let state = build_config_state(config, NetworkProxyConstraints::default(), executor_os)?; - Ok(Self { + let cached_broker = credential_brokers.broker_if_enabled( + environment_id.as_deref(), + audit_metadata.conversation_id.as_deref(), + &state.config, + ); + let mut proxy_state = Self { environment_id: environment_id.map(Into::into), execution_id: execution_id.map(Into::into), ..Self::with_reloader_and_audit_metadata( @@ -357,7 +365,11 @@ impl NetworkProxyState { audit_metadata, LocalBindingPolicy::DefaultFalse, ) - }) + }; + if let Some(broker) = cached_broker { + proxy_state.credential_broker = broker; + } + Ok(proxy_state) } pub fn with_reloader(state: ConfigState, reloader: Arc) -> Self { @@ -570,6 +582,15 @@ impl NetworkProxyState { self.credential_broker.virtualize_text(text, env) } + pub(crate) fn handle_unmasked_credentials( + &self, + env: &mut HashMap, + mode: crate::CredentialMaskingFailureMode, + ) -> anyhow::Result<()> { + self.credential_broker + .handle_unmasked_credentials(env, mode) + } + pub(crate) fn credential_broker_environment( &self, env: &HashMap,