diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index 918e9cc6fa..72fdd9fbc3 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -16,8 +16,6 @@ use codex_network_proxy::NetworkProxyHandle; use codex_network_proxy::NetworkProxyState; 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; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxCommand; @@ -120,7 +118,6 @@ pub(crate) async fn prepare_exec_request_with_telemetry( 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 { @@ -135,35 +132,10 @@ pub(crate) async fn prepare_exec_request_with_telemetry( )); } } - #[cfg(target_os = "windows")] - let mut env = env; - #[cfg(target_os = "windows")] - let network_proxy = if params.sandbox.is_none() { - // Shared Windows ingress selects a route from the sandbox token's SID. Native launches - // have no route SID, so leave them direct. - if params.network_proxy.is_some() { - strip_managed_proxy_env(&mut env); - } - None - } else { - network_proxy - }; - 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( params, - network_proxy, + credential_brokers, runtime_paths.is_some_and(|paths| paths.proxy_private_ips_via_upstream), env, network_policy_decider, @@ -384,7 +356,7 @@ pub(crate) async fn prepare_exec_request_with_telemetry( async fn prepare_managed_network( params: &ExecParams, - network_proxy: Option, + credential_brokers: &RemoteCredentialBrokerCache, proxy_private_ips_via_upstream: bool, env: HashMap, network_policy_decider: Option>, @@ -399,9 +371,37 @@ async fn prepare_managed_network( ), JSONRPCErrorError, > { - let Some(mut state) = network_proxy else { + let Some(launch) = ¶ms.network_proxy else { return Ok((env, params.managed_network.clone(), None, None)); }; + #[cfg(windows)] + let routing = match params + .sandbox + .as_ref() + .map(|sandbox| sandbox.windows_sandbox_selection) + { + None => { + // Native Windows commands have no sandbox token SID for shared ingress. + let mut native_env = env; + codex_network_proxy::strip_managed_proxy_env(&mut native_env); + return Ok((native_env, params.managed_network.clone(), None, None)); + } + Some(WindowsSandboxSelection::Mxc) => ManagedProxyRouting::DedicatedListeners, + Some( + WindowsSandboxSelection::Disabled + | WindowsSandboxSelection::RestrictedToken + | WindowsSandboxSelection::Elevated, + ) => ManagedProxyRouting::SharedIngress, + }; + #[cfg(not(windows))] + let routing = ManagedProxyRouting::SharedIngress; + let mut state = NetworkProxyState::from_remote_launch_config( + launch.clone(), + codex_utils_path_uri::Platform::native(), + credential_brokers, + &env, + ) + .map_err(|err| invalid_params(format!("invalid network proxy config: {err}")))?; let credential_masking = state .current_cfg() .await @@ -411,14 +411,6 @@ async fn prepare_managed_network( )) })? .credential_broker; - let routing = - if params.sandbox.as_ref().is_some_and(|sandbox| { - sandbox.windows_sandbox_selection == WindowsSandboxSelection::Mxc - }) { - ManagedProxyRouting::DedicatedListeners - } else { - ManagedProxyRouting::SharedIngress - }; 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); diff --git a/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs b/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs index f34aa493ea..f4405d8856 100644 --- a/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs +++ b/codex-rs/exec-server/tests/exec_process/windows_sandbox.rs @@ -12,6 +12,70 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use pretty_assertions::assert_eq; +#[cfg(windows)] +#[test_case::test_case(false; "invalid_disabled_proxy")] +#[test_case::test_case(true; "valid_enabled_proxy")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial_test::serial(remote_exec_server)] +async fn native_windows_skips_owned_proxy_before_brokering(proxy_enabled: bool) -> Result<()> { + use codex_network_proxy::NetworkProxyConfig; + use codex_network_proxy::RemoteNetworkProxyConfig; + use codex_network_proxy::RemoteNetworkProxyLaunchConfig; + + const API_KEY: &str = "sk-proj-executor-only-0000000000000000000000000000000000"; + let context = create_process_context(/*use_remote*/ true).await?; + let mut proxy = RemoteNetworkProxyConfig::from_effective_config(&NetworkProxyConfig { + enabled: proxy_enabled, + ..NetworkProxyConfig::default() + })?; + proxy.enable_credential_masking(Default::default()); + let started = context + .backend + .start(ExecParams { + metadata: None, + process_id: ProcessId::from("native-windows-owned-proxy"), + argv: vec![ + std::env::var("ComSpec")?, + "/d".into(), + "/c".into(), + "if defined HTTP_PROXY (exit /b 1) else if defined CODEX_NETWORK_PROXY_ACTIVE (exit /b 2) else (echo %CALLER_VALUE% %OPENAI_API_KEY%)".into(), + ], + cwd: PathUri::from_host_native_path(std::env::current_dir()?)?, + env: HashMap::from([ + ("HTTP_PROXY".into(), "http://127.0.0.1:1".into()), + ("SystemRoot".into(), std::env::var("SystemRoot")?), + ("CODEX_NETWORK_PROXY_ACTIVE".into(), "1".into()), + ("OPENAI_API_KEY".into(), API_KEY.into()), + ("CALLER_VALUE".into(), "preserved".into()), + ]), + env_policy: None, + shell_snapshot: None, + tty: false, + pipe_stdin: false, + arg0: None, + sandbox: None, + enforce_managed_network: false, + managed_network: None, + network_proxy: Some(RemoteNetworkProxyLaunchConfig::new(proxy)), + }) + .await?; + let (stdout, stderr, code, exited) = collect_process_output_from_events_with_timeout( + started.process, + Duration::from_secs(/*secs*/ 30), + ) + .await?; + assert_eq!( + (stdout.trim(), stderr, code, exited), + ( + format!("preserved {API_KEY}").as_str(), + String::new(), + Some(0), + true + ) + ); + Ok(()) +} + /// Windows alias casing must not prevent fallback when the primary runtime is absent from PATH. #[test_case::test_case("pwsh.exe"; "lowercase_pwsh")] #[test_case::test_case("PWSH.exe"; "uppercase_pwsh")] diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 829b920296..81f48b9479 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -373,23 +373,24 @@ impl SandboxManager { "managed networking requires the elevated Windows sandbox backend".into(), )); } + let backend = request.sandbox; let command = request.command; + let managed_network = command + .managed_network + .filter(|_| request.enforce_managed_network || backend == SandboxType::MacosSeatbelt); + let managed_mitm_ca_trust_bundle_path = request + .network + .and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); let permissions = effective_permission_profile( request.permissions, command.additional_permissions.as_ref(), ); - let managed_mitm_ca_trust_bundle_path = request - .network - .and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); let policy = PreparedLaunchPolicy::resolve( permissions, request.sandbox_policy_cwd, request.workspace_roots, ) .map_err(SandboxTransformError::InvalidPolicy)?; - let backend = request.sandbox; - let enforce_managed_network = request.enforce_managed_network; - let mut managed_network = command.managed_network; let mut argv = Vec::with_capacity(1 + command.args.len()); argv.push(command.program.to_string_lossy().into_owned()); argv.extend(command.args); @@ -423,23 +424,23 @@ impl SandboxManager { "native MXC is unavailable on this executor".into(), )); } - if enforce_managed_network && managed_network.is_none() { - let network = request.network.ok_or_else(|| { - SandboxTransformError::WindowsMxcPreparation( - "managed networking requires an executor-local proxy".to_string(), - ) - })?; - let prepared = network - .prepare_for_optional_environment( - std::mem::take(&mut exec.env), - request.environment_id, - ) - .map_err(|err| { - SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) + let route = match managed_network { + None if request.enforce_managed_network => { + let proxy = request.network.ok_or_else(|| { + SandboxTransformError::WindowsMxcPreparation( + "managed networking requires an executor-local proxy".to_string(), + ) })?; - exec.env = prepared.env; - managed_network = Some(prepared.sandbox_context); - } + let prepared = proxy + .prepare_for_optional_environment(exec.env, request.environment_id) + .map_err(|err| { + SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) + })?; + exec.env = prepared.env; + Some(prepared.sandbox_context) + } + route => route, + }; let pending = pending?; exec.permission_profile = pending.effective_permission_profile; let exe = request.sandbox_exe.ok_or_else(|| { @@ -447,7 +448,6 @@ impl SandboxManager { "missing Codex executable path".into(), ) })?; - let route = managed_network.filter(|_| enforce_managed_network); let args = codex_mxc_sandbox::create_command_args( codex_mxc_sandbox::CreateMxcCommandArgsParams { command: std::mem::take(&mut exec.command), @@ -479,10 +479,10 @@ impl SandboxManager { file_system_sandbox_policy: &file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd: &pending.native_sandbox_policy_cwd, - enforce_managed_network, + enforce_managed_network: request.enforce_managed_network, managed_network: managed_network.as_ref(), - environment_id: exec.network_environment_id.as_deref(), - network: exec.network.as_ref(), + environment_id: request.environment_id, + network: request.network, extra_allow_unix_sockets: &[], }, self.seatbelt_profile, @@ -508,22 +508,23 @@ impl SandboxManager { let exe = request .sandbox_exe .ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?; - if enforce_managed_network - && managed_network.is_none() - && let Some(network) = request.network - { - let prepared = network - .prepare_for_optional_environment( - std::mem::take(&mut exec.env), - request.environment_id, - ) - .map_err(|err| { - SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) - })?; - exec.env = prepared.env; - managed_network = Some(prepared.sandbox_context); - } - let route = enforce_managed_network.then(|| managed_network.unwrap_or_default()); + let route = match ( + request.enforce_managed_network, + managed_network, + request.network, + ) { + (false, _, _) => None, + (true, None, Some(proxy)) => { + let prepared = proxy + .prepare_for_optional_environment(exec.env, request.environment_id) + .map_err(|err| { + SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) + })?; + exec.env = prepared.env; + Some(prepared.sandbox_context) + } + (true, context, _) => Some(context.unwrap_or_default()), + }; self.wrap_linux( &mut exec, (