diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 55a22d015598..5e5bcb6c4e1d 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -282,9 +282,11 @@ async fn run_command_under_sandbox( network_sandbox_policy, sandbox_policy_cwd: sandbox_policy_cwd.as_path(), enforce_managed_network, + environment_id: None, network: network.as_ref(), extra_allow_unix_sockets: allow_unix_sockets, - }); + }) + .map_err(|err| anyhow::anyhow!(err))?; spawn_debug_sandbox_child( PathBuf::from("/usr/bin/sandbox-exec"), args, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 0142f995f5a6..9d55ae886401 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -128,6 +128,16 @@ fn select_process_exec_tool_sandbox_type( ) } +fn network_proxy_environment_error( + network_environment_id: Option<&str>, + err: impl std::fmt::Display, +) -> CodexErr { + let environment_id = network_environment_id.unwrap_or("default"); + CodexErr::Io(io::Error::other(format!( + "failed to prepare network proxy for environment `{environment_id}`: {err}" + ))) +} + /// Mechanism to terminate an exec invocation before it finishes naturally. #[derive(Clone, Debug)] pub enum ExecExpiration { @@ -345,7 +355,11 @@ pub fn build_exec_request( tracing::debug!("Sandbox type: {sandbox_type:?}"); if let Some(network) = network.as_ref() { - network.apply_to_env(&mut env); + network + .apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref()) + .map_err(|err| { + network_proxy_environment_error(network_environment_id.as_deref(), err) + })?; } let (program, args) = command.split_first().ok_or_else(|| { CodexErr::Io(io::Error::new( @@ -598,7 +612,7 @@ async fn exec_windows_sandbox( cwd, mut env, network, - network_environment_id: _, + network_environment_id, expiration, capture_policy, windows_sandbox_level, @@ -606,7 +620,11 @@ async fn exec_windows_sandbox( .. } = params; if let Some(network) = network.as_ref() { - network.apply_to_env(&mut env); + network + .apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref()) + .map_err(|err| { + network_proxy_environment_error(network_environment_id.as_deref(), err) + })?; } // Windows sandbox capture still receives timeout and cancellation separately. @@ -947,7 +965,7 @@ async fn exec( cwd, mut env, network, - network_environment_id: _, + network_environment_id, arg0, expiration, capture_policy, @@ -961,7 +979,11 @@ async fn exec( justification: _, } = params; if let Some(network) = network.as_ref() { - network.apply_to_env(&mut env); + network + .apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref()) + .map_err(|err| { + network_proxy_environment_error(network_environment_id.as_deref(), err) + })?; } let (program, args) = command.split_first().ok_or_else(|| { diff --git a/codex-rs/core/src/tools/handlers/shell/shell_command.rs b/codex-rs/core/src/tools/handlers/shell/shell_command.rs index 6176a07d62df..f10c7363715f 100644 --- a/codex-rs/core/src/tools/handlers/shell/shell_command.rs +++ b/codex-rs/core/src/tools/handlers/shell/shell_command.rs @@ -105,7 +105,10 @@ impl ShellCommandHandler { Some(thread_id), ), network: turn_context.network.clone(), - network_environment_id: None, + network_environment_id: turn_context + .environments + .primary() + .map(|environment| environment.environment_id.clone()), sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index b05a7455267d..2b63164b4df4 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -51,6 +51,7 @@ pub(crate) struct NetworkApprovalSpec { pub mode: NetworkApprovalMode, pub trigger: GuardianNetworkAccessTrigger, pub command: String, + pub environment_id: String, } #[derive(Clone, Debug)] @@ -120,14 +121,20 @@ impl ActiveNetworkApproval { #[derive(Clone, Debug, Eq, Hash, PartialEq)] struct HostApprovalKey { + environment_id: String, host: String, protocol: &'static str, port: u16, } impl HostApprovalKey { - fn from_request(request: &NetworkPolicyRequest, protocol: NetworkApprovalProtocol) -> Self { + fn from_request( + request: &NetworkPolicyRequest, + protocol: NetworkApprovalProtocol, + environment_id: String, + ) -> Self { Self { + environment_id, host: request.host.to_ascii_lowercase(), protocol: protocol_key_label(protocol), port: request.port, @@ -224,9 +231,16 @@ struct ActiveNetworkApprovalCall { turn_id: String, trigger: GuardianNetworkAccessTrigger, command: String, + environment_id: String, cancellation_token: CancellationToken, } +enum ActiveNetworkApprovalAttribution { + None, + Single(Arc), + Ambiguous, +} + #[derive(Default)] struct NetworkApprovalCallState { active_calls: IndexMap>, @@ -267,6 +281,7 @@ impl NetworkApprovalService { turn_id: String, trigger: GuardianNetworkAccessTrigger, command: String, + environment_id: String, cancellation_token: CancellationToken, ) { let mut calls = self.calls.lock().await; @@ -278,6 +293,7 @@ impl NetworkApprovalService { turn_id, trigger, command, + environment_id, cancellation_token, }), ); @@ -299,6 +315,18 @@ impl NetworkApprovalService { None } + async fn resolve_active_call_attribution(&self) -> ActiveNetworkApprovalAttribution { + let calls = self.calls.lock().await; + match calls.active_calls.len() { + 0 => ActiveNetworkApprovalAttribution::None, + 1 => calls.active_calls.values().next().cloned().map_or( + ActiveNetworkApprovalAttribution::None, + ActiveNetworkApprovalAttribution::Single, + ), + _ => ActiveNetworkApprovalAttribution::Ambiguous, + } + } + async fn get_or_create_pending_approval( &self, key: HostApprovalKey, @@ -384,7 +412,10 @@ impl NetworkApprovalService { } fn approval_id_for_key(key: &HostApprovalKey) -> String { - format!("network#{}#{}#{}", key.protocol, key.host, key.port) + format!( + "network#{}#{}#{}#{}", + key.environment_id, key.protocol, key.host, key.port + ) } pub(crate) async fn handle_inline_policy_request( @@ -400,7 +431,38 @@ impl NetworkApprovalService { NetworkProtocol::Socks5Tcp => NetworkApprovalProtocol::Socks5Tcp, NetworkProtocol::Socks5Udp => NetworkApprovalProtocol::Socks5Udp, }; - let key = HostApprovalKey::from_request(&request, protocol); + let (owner_call, active_environment_id) = + if let Some(environment_id) = request.environment_id.clone() { + let owner_call = match self.resolve_active_call_attribution().await { + ActiveNetworkApprovalAttribution::Single(call) => { + (call.environment_id == environment_id).then_some(call) + } + ActiveNetworkApprovalAttribution::None + | ActiveNetworkApprovalAttribution::Ambiguous => None, + }; + (owner_call, Some(environment_id)) + } else { + match self.resolve_active_call_attribution().await { + ActiveNetworkApprovalAttribution::None => (None, None), + ActiveNetworkApprovalAttribution::Single(call) => { + let environment_id = call.environment_id.clone(); + (Some(call), Some(environment_id)) + } + ActiveNetworkApprovalAttribution::Ambiguous => { + return NetworkDecision::deny(REASON_NOT_ALLOWED); + } + } + }; + let turn_context = Self::active_turn_context(session.as_ref()).await; + let Some(environment_id) = active_environment_id.or_else(|| { + turn_context + .as_ref() + .and_then(|turn_context| turn_context.environments.primary()) + .map(|environment| environment.environment_id.clone()) + }) else { + return NetworkDecision::deny(REASON_NOT_ALLOWED); + }; + let key = HostApprovalKey::from_request(&request, protocol, environment_id.clone()); { let denied_hosts = self.session_denied_hosts.lock().await; @@ -426,35 +488,43 @@ impl NetworkApprovalService { format!("Network access to \"{target}\" was blocked by policy."); let prompt_reason = format!("{} is not in the allowed_domains", request.host); - let Some(turn_context) = Self::active_turn_context(session.as_ref()).await else { + let Some(turn_context) = turn_context else { pending.set_decision(PendingApprovalDecision::Deny).await; self.pending_host_approvals.lock().await.remove(&key); - self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy( - policy_denial_message, - )) - .await; + if let Some(owner_call) = owner_call.as_ref() { + self.record_call_outcome( + &owner_call.registration_id, + NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message), + ) + .await; + } return NetworkDecision::deny(REASON_NOT_ALLOWED); }; if !permission_profile_allows_network_approval_flow(&turn_context.permission_profile()) { pending.set_decision(PendingApprovalDecision::Deny).await; self.pending_host_approvals.lock().await.remove(&key); - self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy( - policy_denial_message, - )) - .await; + if let Some(owner_call) = owner_call.as_ref() { + self.record_call_outcome( + &owner_call.registration_id, + NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message), + ) + .await; + } return NetworkDecision::deny(REASON_NOT_ALLOWED); } if !allows_network_approval_flow(turn_context.approval_policy.value()) { pending.set_decision(PendingApprovalDecision::Deny).await; self.pending_host_approvals.lock().await.remove(&key); - self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByPolicy( - policy_denial_message, - )) - .await; + if let Some(owner_call) = owner_call.as_ref() { + self.record_call_outcome( + &owner_call.registration_id, + NetworkApprovalOutcome::DeniedByPolicy(policy_denial_message), + ) + .await; + } return NetworkDecision::deny(REASON_NOT_ALLOWED); } - let owner_call = self.resolve_single_active_call().await; let network_approval_context = NetworkApprovalContext { host: request.host.clone(), protocol, @@ -519,15 +589,28 @@ impl NetworkApprovalService { .await } else { let available_decisions = None; + let cwd = if let Some(owner_call) = owner_call.as_ref() { + owner_call.trigger.cwd.clone() + } else { + turn_context + .environments + .turn_environments + .iter() + .find(|environment| environment.environment_id == environment_id) + .and_then(|environment| environment.cwd().to_abs_path().ok()) + .unwrap_or_else(|| { + #[allow(deprecated)] + turn_context.cwd.clone() + }) + }; session .request_command_approval( turn_context.as_ref(), guardian_approval_id, /*approval_id*/ None, - /*environment_id*/ None, + Some(environment_id), prompt_command, - #[allow(deprecated)] - turn_context.cwd.clone(), + cwd, Some(prompt_reason), Some(network_approval_context.clone()), /*proposed_execpolicy_amendment*/ None, @@ -712,6 +795,7 @@ pub(crate) async fn begin_network_approval( mode, trigger, command, + environment_id, } = spec?; if !managed_network_active || network.is_none() { return None; @@ -727,6 +811,7 @@ pub(crate) async fn begin_network_approval( turn_id.to_string(), trigger, command, + environment_id, cancellation_token.clone(), ) .await; diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 25bcfe255dc0..60c7215b1b0b 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -13,6 +13,7 @@ use tokio_util::sync::CancellationToken; async fn pending_approvals_are_deduped_per_host_protocol_and_port() { let service = NetworkApprovalService::default(); let key = HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "http", port: 443, @@ -30,11 +31,13 @@ async fn pending_approvals_are_deduped_per_host_protocol_and_port() { async fn pending_approvals_do_not_dedupe_across_ports() { let service = NetworkApprovalService::default(); let first_key = HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 443, }; let second_key = HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 8443, @@ -48,6 +51,56 @@ async fn pending_approvals_do_not_dedupe_across_ports() { assert!(!Arc::ptr_eq(&first, &second)); } +#[tokio::test] +async fn pending_approvals_do_not_dedupe_across_environments() { + let service = NetworkApprovalService::default(); + let first_key = HostApprovalKey { + environment_id: "local".to_string(), + host: "example.com".to_string(), + protocol: "https", + port: 443, + }; + let second_key = HostApprovalKey { + environment_id: "remote".to_string(), + ..first_key.clone() + }; + + let (first, first_is_owner) = service.get_or_create_pending_approval(first_key).await; + let (second, second_is_owner) = service.get_or_create_pending_approval(second_key).await; + + assert!(first_is_owner); + assert!(second_is_owner); + assert!(!Arc::ptr_eq(&first, &second)); +} + +#[tokio::test] +async fn session_approved_hosts_are_scoped_by_environment() { + let service = NetworkApprovalService::default(); + let local_key = HostApprovalKey { + environment_id: "local".to_string(), + host: "example.com".to_string(), + protocol: "https", + port: 443, + }; + let remote_key = HostApprovalKey { + environment_id: "remote".to_string(), + ..local_key.clone() + }; + service + .session_approved_hosts + .lock() + .await + .insert(local_key); + + assert!( + !service + .session_approved_hosts + .lock() + .await + .contains(&remote_key) + ); +} + #[tokio::test] async fn session_approved_hosts_preserve_protocol_and_port_scope() { let source = NetworkApprovalService::default(); @@ -55,16 +108,19 @@ async fn session_approved_hosts_preserve_protocol_and_port_scope() { let mut approved_hosts = source.session_approved_hosts.lock().await; approved_hosts.extend([ HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 443, }, HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 8443, }, HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "http", port: 80, @@ -82,22 +138,32 @@ async fn session_approved_hosts_preserve_protocol_and_port_scope() { .iter() .cloned() .collect::>(); - copied.sort_by(|a, b| (&a.host, a.protocol, a.port).cmp(&(&b.host, b.protocol, b.port))); + copied.sort_by(|a, b| { + (&a.environment_id, &a.host, a.protocol, a.port).cmp(&( + &b.environment_id, + &b.host, + b.protocol, + b.port, + )) + }); assert_eq!( copied, vec![ HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "http", port: 80, }, HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 443, }, HostApprovalKey { + environment_id: "local".to_string(), host: "example.com".to_string(), protocol: "https", port: 8443, @@ -112,6 +178,7 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { { let mut approved_hosts = source.session_approved_hosts.lock().await; approved_hosts.insert(HostApprovalKey { + environment_id: "local".to_string(), host: "source.example.com".to_string(), protocol: "https", port: 443, @@ -122,6 +189,7 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { { let mut approved_hosts = target.session_approved_hosts.lock().await; approved_hosts.insert(HostApprovalKey { + environment_id: "local".to_string(), host: "stale.example.com".to_string(), protocol: "https", port: 8443, @@ -141,6 +209,7 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { assert_eq!( copied, vec![HostApprovalKey { + environment_id: "local".to_string(), host: "source.example.com".to_string(), protocol: "https", port: 443, @@ -237,6 +306,7 @@ async fn register_call_with_default_shell_trigger( tty: None, }, "curl https://example.com".to_string(), + "local".to_string(), cancellation_token.clone(), ) .await; @@ -263,6 +333,7 @@ async fn active_call_preserves_triggering_command_context() { "turn-1".to_string(), expected.clone(), "curl https://example.com".to_string(), + "remote".to_string(), CancellationToken::new(), ) .await; @@ -274,6 +345,21 @@ async fn active_call_preserves_triggering_command_context() { assert_eq!(&call.trigger, &expected); assert_eq!(call.command, "curl https://example.com"); + assert_eq!(call.environment_id, "remote"); +} + +#[tokio::test] +async fn multiple_active_calls_are_ambiguous_even_in_the_same_environment() { + let service = NetworkApprovalService::default(); + register_call_with_default_shell_trigger(&service, "registration-1").await; + register_call_with_default_shell_trigger(&service, "registration-2").await; + + match service.resolve_active_call_attribution().await { + ActiveNetworkApprovalAttribution::Ambiguous => {} + ActiveNetworkApprovalAttribution::None | ActiveNetworkApprovalAttribution::Single(_) => { + panic!("multiple active calls should be ambiguous") + } + } } #[tokio::test] diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 542589ad89c2..91db26aaaeab 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -126,6 +126,7 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: Some(&proxy), SandboxPermissions::RequireEscalated, ), + /*environment_id*/ None, ) .expect("prepare exec request"); diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index f88c2be3d4d7..0d386103db77 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -236,6 +236,7 @@ impl ToolRuntime for ShellRuntime { tty: None, }, command: req.hook_command.clone(), + environment_id: req.turn_environment.environment_id.clone(), }) } @@ -321,7 +322,12 @@ impl ToolRuntime for ShellRuntime { capture_policy: ExecCapturePolicy::ShellTool, }; let env = attempt - .env_for(command, options, managed_network) + .env_for( + command, + options, + managed_network, + Some(&req.turn_environment.environment_id), + ) .map_err(ToolError::Codex)?; let out = execute_env(env, Self::stdout_stream(ctx)) .await diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 2acbf3778159..c08148c7ee15 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -68,6 +68,7 @@ use codex_shell_escalation::Stopwatch; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::collections::HashMap; +use std::io; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -143,6 +144,7 @@ pub(super) async fn try_run_zsh_fork( command, options, managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions), + Some(&req.turn_environment.environment_id), ) .map_err(ToolError::Codex)?; let crate::sandboxing::ExecRequest { @@ -151,7 +153,7 @@ pub(super) async fn try_run_zsh_fork( env: sandbox_env, exec_server_env_config: _, network: sandbox_network, - network_environment_id: _, + network_environment_id, expiration: _sandbox_expiration, capture_policy: _capture_policy, sandbox, @@ -190,6 +192,7 @@ pub(super) async fn try_run_zsh_fork( sandbox, env: sandbox_env, network: sandbox_network, + network_environment_id, windows_sandbox_level, arg0, sandbox_policy_cwd, @@ -302,6 +305,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( sandbox: exec_request.sandbox, env: exec_request.env.clone(), network: exec_request.network.clone(), + network_environment_id: exec_request.network_environment_id.clone(), windows_sandbox_level: exec_request.windows_sandbox_level, arg0: exec_request.arg0.clone(), sandbox_policy_cwd, @@ -810,6 +814,7 @@ struct CoreShellCommandExecutor { sandbox: SandboxType, env: HashMap, network: Option, + network_environment_id: Option, windows_sandbox_level: WindowsSandboxLevel, arg0: Option, sandbox_policy_cwd: AbsolutePathBuf, @@ -880,7 +885,7 @@ impl CoreShellCommandExecutor { env: exec_env, exec_server_env_config: None, network: self.network.clone(), - network_environment_id: None, + network_environment_id: self.network_environment_id.clone(), expiration: ExecExpiration::Cancellation(cancel_rx), capture_policy: ExecCapturePolicy::ShellTool, sandbox: self.sandbox, @@ -1012,7 +1017,7 @@ impl CoreShellCommandExecutor { permissions: permission_profile, sandbox, enforce_managed_network: self.network.is_some(), - environment_id: None, + environment_id: self.network_environment_id.as_deref(), network: self.network.as_ref(), sandbox_policy_cwd: &sandbox_policy_cwd, codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.as_deref(), @@ -1026,7 +1031,18 @@ impl CoreShellCommandExecutor { self.windows_sandbox_workspace_roots.clone(), ); if let Some(network) = exec_request.network.as_ref() { - network.apply_to_env(&mut exec_request.env); + network + .apply_to_env_for_optional_environment( + &mut exec_request.env, + self.network_environment_id.as_deref(), + ) + .map_err(|err| { + let environment_id = + self.network_environment_id.as_deref().unwrap_or("default"); + CodexErr::Io(io::Error::other(format!( + "failed to prepare network proxy for environment `{environment_id}`: {err}" + ))) + })?; } Ok(PreparedExec { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 2b0014761298..5478c1d946c6 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -366,6 +366,7 @@ async fn unsandboxed_intercepted_exec_strips_managed_network_env() -> anyhow::Re sandbox: SandboxType::None, env: HashMap::new(), network: None, + network_environment_id: None, windows_sandbox_level: WindowsSandboxLevel::Disabled, arg0: None, sandbox_policy_cwd: workdir.clone(), diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 80f0990b5c9e..0d6c93b4d055 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -53,6 +53,7 @@ use codex_tools::UnifiedExecShellMode; use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use std::collections::HashMap; +use std::io; use tokio_util::sync::CancellationToken; use tracing::error; @@ -283,6 +284,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt tty: Some(req.tty), }, command: req.hook_command.clone(), + environment_id: req.turn_environment.environment_id.clone(), }) } @@ -321,7 +323,17 @@ impl<'a> ToolRuntime for UnifiedExecRunt ); let mut env = exec_env_for_sandbox_permissions(&req.env, launch_sandbox_permissions); if let Some(network) = managed_network { - network.apply_to_env(&mut env); + network + .apply_to_env_for_optional_environment( + &mut env, + Some(&req.turn_environment.environment_id), + ) + .map_err(|err| { + ToolError::Codex(CodexErr::Io(io::Error::other(format!( + "failed to prepare network proxy for environment `{}`: {err}", + req.turn_environment.environment_id + )))) + })?; } let explicit_env_overrides = req.explicit_env_overrides.clone(); #[cfg(unix)] @@ -383,7 +395,12 @@ impl<'a> ToolRuntime for UnifiedExecRunt })?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); let mut exec_env = attempt - .env_for(command, options, managed_network) + .env_for( + command, + options, + managed_network, + Some(&req.turn_environment.environment_id), + ) .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); match zsh_fork_backend::maybe_prepare_unified_exec( @@ -443,7 +460,12 @@ impl<'a> ToolRuntime for UnifiedExecRunt })?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); let mut exec_env = attempt - .env_for(command, options, managed_network) + .env_for( + command, + options, + managed_network, + Some(&req.turn_environment.environment_id), + ) .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); self.manager diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index e80cbbba1a3e..48c06413b155 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -426,6 +426,7 @@ impl<'a> SandboxAttempt<'a> { command: SandboxCommand, options: ExecOptions, network: Option<&NetworkProxy>, + environment_id: Option<&str>, ) -> Result { let request = self .manager @@ -434,7 +435,7 @@ impl<'a> SandboxAttempt<'a> { permissions: self.permissions, sandbox: self.sandbox, enforce_managed_network: self.enforce_managed_network, - environment_id: None, + environment_id, network, sandbox_policy_cwd: self.sandbox_cwd, codex_linux_sandbox_exe: self diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 800b52c7757a..492f3097951e 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -74,6 +74,7 @@ mod model_visible_layout; mod models_cache_ttl; mod models_etag_responses; mod multi_agent_mode; +mod network_approval; mod openai_file_mcp; mod otel; mod override_updates; diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs new file mode 100644 index 000000000000..14db16c60a01 --- /dev/null +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -0,0 +1,339 @@ +use anyhow::Context; +use anyhow::Result; +use codex_config::types::ApprovalsReviewer; +use codex_core::config::Constrained; +use codex_exec_server::CreateDirectoryOptions; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_exec_server::REMOTE_ENVIRONMENT_ID; +use codex_exec_server::RemoveOptions; +use codex_features::Feature; +use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ExecApprovalRequestEvent; +use codex_protocol::protocol::Op; +use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::TurnEnvironmentSelection; +use codex_protocol::protocol::TurnEnvironmentSelections; +use codex_protocol::user_input::UserInput; +use codex_utils_path_uri::PathUri; +use core_test_support::PathBufExt; +use core_test_support::PathExt; +use core_test_support::get_remote_test_env; +use core_test_support::managed_network_requirements_loader; +use core_test_support::responses::ResponseMock; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::skip_if_sandbox; +use core_test_support::skip_if_windows; +use core_test_support::skip_if_wine_exec; +use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::local; +use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_with_timeout; +use pretty_assertions::assert_eq; +use serde_json::Value; +use serde_json::json; +use std::fs; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; +use tempfile::TempDir; + +const NETWORK_TEST_HOST: &str = "codex-network-test.invalid"; +const NETWORK_TEST_TARGET: &str = "http://codex-network-test.invalid:80"; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn approved_network_host_for_one_environment_still_prompts_in_another() -> Result<()> { + skip_if_wine_exec!(Ok(()), "uses the POSIX/Python network fixture"); + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + skip_if_windows!(Ok(())); + let Some(_remote_env) = get_remote_test_env() else { + return Ok(()); + }; + + let server = start_mock_server().await; + let test = managed_network_unified_exec_test(&server).await?; + let local_cwd = TempDir::new()?; + let remote_cwd = PathBuf::from(format!( + "/tmp/codex-network-approval-{}", + SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() + )) + .abs(); + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; + test.fs() + .create_directory( + &remote_cwd_uri, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + let environments = vec![ + local(local_cwd.path().abs()), + TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + }, + ]; + + mount_exec_network_turn( + &server, + "resp-network-local", + "exec-network-local", + network_fetch_args(LOCAL_ENVIRONMENT_ID), + ) + .await?; + submit_managed_network_turn( + &test, + "fetch from the local environment", + environments.clone(), + ) + .await?; + let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::ApprovedForSession, + }) + .await?; + wait_for_turn_complete(&test).await; + + mount_exec_network_turn( + &server, + "resp-network-remote", + "exec-network-remote", + network_fetch_args(REMOTE_ENVIRONMENT_ID), + ) + .await?; + submit_managed_network_turn( + &test, + "fetch from the remote environment", + environments.clone(), + ) + .await?; + let approval = expect_network_approval(&test, REMOTE_ENVIRONMENT_ID).await?; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Denied, + }) + .await?; + wait_for_turn_complete(&test).await; + + test.fs() + .remove( + &remote_cwd_uri, + RemoveOptions { + recursive: true, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + + Ok(()) +} + +async fn managed_network_unified_exec_test(server: &wiremock::MockServer) -> Result { + let home = Arc::new(TempDir::new()?); + fs::write( + home.path().join("config.toml"), + r#"default_permissions = "workspace" + +[permissions.workspace.filesystem] +":minimal" = "read" + +[permissions.workspace.network] +enabled = true +mode = "limited" +allow_local_binding = true +"#, + )?; + let approval_policy = AskForApproval::OnFailure; + let permission_profile = PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ); + let permission_profile_for_config = permission_profile.clone(); + let mut builder = test_codex() + .with_home(home) + .with_cloud_config_bundle(managed_network_requirements_loader()) + .with_config(move |config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config + .permissions + .set_permission_profile(permission_profile_for_config) + .expect("set permission profile"); + }); + let test = builder.build_with_remote_and_local_env(server).await?; + assert!( + test.config.managed_network_requirements_enabled(), + "expected managed network requirements to be enabled" + ); + assert!( + test.config.permissions.network.is_some(), + "expected managed network proxy config to be present" + ); + test.session_configured + .network_proxy + .as_ref() + .expect("expected runtime managed network proxy addresses"); + + Ok(test) +} + +async fn mount_exec_network_turn( + server: &wiremock::MockServer, + response_prefix: &str, + call_id: &str, + args: Value, +) -> Result { + let responses = vec![ + sse(vec![ + ev_response_created(&format!("{response_prefix}-1")), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed(&format!("{response_prefix}-1")), + ]), + sse(vec![ + ev_response_created(&format!("{response_prefix}-2")), + ev_assistant_message(&format!("{response_prefix}-msg"), "done"), + ev_completed(&format!("{response_prefix}-2")), + ]), + ]; + Ok(mount_sse_sequence(server, responses).await) +} + +fn network_fetch_args(environment_id: &str) -> Value { + json!({ + "shell": "/bin/sh", + "cmd": format!("python3 -c \"import urllib.request; opener = urllib.request.build_opener(urllib.request.ProxyHandler()); print('OK:' + opener.open('http://{NETWORK_TEST_HOST}', timeout=2).read().decode(errors='replace'))\""), + "login": false, + "yield_time_ms": 1_000, + "environment_id": environment_id, + }) +} + +async fn submit_managed_network_turn( + test: &TestCodex, + prompt: &str, + environments: Vec, +) -> Result<()> { + let permission_profile = PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ); + let (sandbox_policy, permission_profile) = + turn_permission_fields(permission_profile, test.config.cwd.as_path()); + let turn_environment_selections = + TurnEnvironmentSelections::new(test.config.cwd.clone(), environments); + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: prompt.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + environments: Some(turn_environment_selections), + approval_policy: Some(AskForApproval::OnFailure), + approvals_reviewer: Some(ApprovalsReviewer::User), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: test.session_configured.model.clone(), + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + Ok(()) +} + +async fn expect_network_approval( + test: &TestCodex, + expected_environment_id: &str, +) -> Result { + let deadline = std::time::Instant::now() + Duration::from_secs(30); + let remaining = deadline + .checked_duration_since(std::time::Instant::now()) + .context("timed out waiting for network approval request")?; + let event = wait_for_event_with_timeout( + &test.codex, + |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }, + remaining, + ) + .await; + match event { + EventMsg::ExecApprovalRequest(approval) => { + assert_eq!( + approval.command, + vec![ + "network-access".to_string(), + NETWORK_TEST_TARGET.to_string() + ] + ); + assert_eq!( + approval.network_approval_context, + Some(NetworkApprovalContext { + host: NETWORK_TEST_HOST.to_string(), + protocol: NetworkApprovalProtocol::Http, + }) + ); + assert_eq!( + approval.environment_id.as_deref(), + Some(expected_environment_id) + ); + Ok(approval) + } + EventMsg::TurnComplete(_) => { + panic!("expected network approval request before completion"); + } + other => panic!("unexpected event: {other:?}"), + } +} + +async fn wait_for_turn_complete(test: &TestCodex) { + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; +} diff --git a/codex-rs/network-proxy/src/http_proxy.rs b/codex-rs/network-proxy/src/http_proxy.rs index c07ff17606ae..3f02ad3fc07d 100644 --- a/codex-rs/network-proxy/src/http_proxy.rs +++ b/codex-rs/network-proxy/src/http_proxy.rs @@ -87,6 +87,7 @@ pub async fn run_http_proxy( state: Arc, addr: SocketAddr, policy_decider: Option>, + environment_id: Option, ) -> Result<()> { let listener = TcpListener::build() .bind(addr) @@ -99,23 +100,25 @@ pub async fn run_http_proxy( .map_err(anyhow::Error::from) .with_context(|| format!("bind HTTP proxy: {addr}"))?; - run_http_proxy_with_listener(state, listener, policy_decider).await + run_http_proxy_with_listener(state, listener, policy_decider, environment_id).await } pub async fn run_http_proxy_with_std_listener( state: Arc, listener: StdTcpListener, policy_decider: Option>, + environment_id: Option, ) -> Result<()> { let listener = TcpListener::try_from(listener).context("convert std listener to HTTP proxy listener")?; - run_http_proxy_with_listener(state, listener, policy_decider).await + run_http_proxy_with_listener(state, listener, policy_decider, environment_id).await } async fn run_http_proxy_with_listener( state: Arc, listener: TcpListener, policy_decider: Option>, + environment_id: Option, ) -> Result<()> { ensure_rustls_crypto_provider(); @@ -133,7 +136,10 @@ async fn run_http_proxy_with_listener( MethodMatcher::CONNECT, service_fn({ let policy_decider = policy_decider.clone(); - move |req| http_connect_accept(policy_decider.clone(), req) + let environment_id = environment_id.clone(); + move |req| { + http_connect_accept(policy_decider.clone(), environment_id.clone(), req) + } }), service_fn(http_connect_proxy), ), @@ -141,7 +147,8 @@ async fn run_http_proxy_with_listener( ) .into_layer(service_fn({ let policy_decider = policy_decider.clone(); - move |req| http_plain_proxy(policy_decider.clone(), req) + let environment_id = environment_id.clone(); + move |req| http_plain_proxy(policy_decider.clone(), environment_id.clone(), req) })), ); @@ -155,6 +162,7 @@ async fn run_http_proxy_with_listener( async fn http_connect_accept( policy_decider: Option>, + environment_id: Option, mut req: Request, ) -> Result<(Response, Request), Response> { let app_state = req @@ -200,7 +208,7 @@ async fn http_connect_accept( protocol: NetworkProtocol::HttpsConnect, host: host.clone(), port: authority.port, - environment_id: None, + environment_id, client_addr: client.clone(), method: Some("CONNECT".to_string()), command: None, @@ -479,6 +487,7 @@ async fn forward_connect_tunnel( async fn http_plain_proxy( policy_decider: Option>, + environment_id: Option, mut req: Request, ) -> Result { let app_state = match req.extensions().get::>().cloned() { @@ -684,7 +693,7 @@ async fn http_plain_proxy( protocol: NetworkProtocol::Http, host: host.clone(), port, - environment_id: None, + environment_id, client_addr: client.clone(), method: Some(req.method().as_str().to_string()), command: None, @@ -1052,6 +1061,7 @@ mod tests { use std::net::Ipv4Addr; use std::net::TcpListener as StdTcpListener; use std::sync::Arc; + use std::sync::Mutex; use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; use tokio::net::TcpListener as TokioTcpListener; @@ -1076,9 +1086,11 @@ mod tests { .unwrap(); req.extensions_mut().insert(state); - let response = http_connect_accept(/*policy_decider*/ None, req) - .await - .unwrap_err(); + let response = http_connect_accept( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap_err(); assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_eq!( response.headers().get("x-proxy-error").unwrap(), @@ -1106,10 +1118,51 @@ mod tests { .unwrap(); req.extensions_mut().insert(state); - let (response, _request) = http_connect_accept(/*policy_decider*/ None, req) - .await + let (response, _request) = http_connect_accept( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + } + + #[tokio::test] + async fn http_connect_accept_passes_environment_id_to_decider() { + let state = Arc::new(network_proxy_state_for_policy( + NetworkProxySettings::default(), + )); + let seen_environment_id = Arc::new(Mutex::new(None)); + let decider: Arc = Arc::new({ + let seen_environment_id = seen_environment_id.clone(); + move |request: NetworkPolicyRequest| { + *seen_environment_id + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) = request.environment_id; + async { NetworkDecision::Allow } + } + }); + + let mut req = Request::builder() + .method(Method::CONNECT) + .uri("https://example.com:443") + .header("host", "example.com:443") + .body(Body::empty()) .unwrap(); + req.extensions_mut().insert(state); + + let (response, _request) = + http_connect_accept(Some(decider), Some("remote".to_string()), req) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + seen_environment_id + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_deref(), + Some("remote") + ); } #[tokio::test] @@ -1138,9 +1191,11 @@ mod tests { .unwrap(); req.extensions_mut().insert(state); - let response = http_connect_accept(/*policy_decider*/ None, req) - .await - .unwrap_err(); + let response = http_connect_accept( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap_err(); assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_eq!( response.headers().get("x-proxy-error").unwrap(), @@ -1177,7 +1232,7 @@ mod tests { .local_addr() .expect("proxy listener should expose local addr"); let proxy_task = tokio::spawn(run_http_proxy_with_std_listener( - state, listener, /*policy_decider*/ None, + state, listener, /*policy_decider*/ None, /*environment_id*/ None, )); let mut stream = tokio::net::TcpStream::connect(proxy_addr) @@ -1228,9 +1283,11 @@ mod tests { .expect("request should build"); req.extensions_mut().insert(state); - let response = http_plain_proxy(/*policy_decider*/ None, req) - .await - .unwrap(); + let response = http_plain_proxy( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap(); assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_eq!( @@ -1253,9 +1310,11 @@ mod tests { .expect("request should build"); req.extensions_mut().insert(state); - let response = http_plain_proxy(/*policy_decider*/ None, req) - .await - .unwrap(); + let response = http_plain_proxy( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap(); if cfg!(target_os = "macos") { assert_eq!(response.status(), StatusCode::FORBIDDEN); @@ -1285,9 +1344,11 @@ mod tests { .expect("request should build"); req.extensions_mut().insert(state); - let response = http_plain_proxy(/*policy_decider*/ None, req) - .await - .unwrap(); + let response = http_plain_proxy( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap(); assert_eq!(response.status(), StatusCode::BAD_GATEWAY); } @@ -1309,9 +1370,11 @@ mod tests { .unwrap(); req.extensions_mut().insert(state); - let response = http_connect_accept(/*policy_decider*/ None, req) - .await - .unwrap_err(); + let response = http_connect_accept( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await + .unwrap_err(); assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_eq!( response.headers().get("x-proxy-error").unwrap(), @@ -1332,7 +1395,10 @@ mod tests { .unwrap(); req.extensions_mut().insert(state); - let response = http_plain_proxy(/*policy_decider*/ None, req).await; + let response = http_plain_proxy( + /*policy_decider*/ None, /*environment_id*/ None, req, + ) + .await; assert_eq!(response.unwrap().status(), StatusCode::BAD_REQUEST); } diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index c3685a4310aa..ef56846599ae 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -222,11 +222,13 @@ impl NetworkProxyBuilder { http_addr, socks_addr, socks_enabled: current_cfg.network.enable_socks5, + socks5_udp_enabled: current_cfg.network.enable_socks5_udp, runtime_settings: Arc::new(RwLock::new(NetworkProxyRuntimeSettings::from_config( ¤t_cfg, )?)), reserved_listeners, policy_decider: self.policy_decider, + environment_proxies: Arc::new(Mutex::new(HashMap::new())), }) } } @@ -323,15 +325,29 @@ impl NetworkProxyRuntimeSettings { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct EnvironmentProxyAddrs { + http_addr: SocketAddr, + socks_addr: SocketAddr, +} + +struct EnvironmentProxy { + addrs: EnvironmentProxyAddrs, + http_task: JoinHandle>, + socks_task: Option>>, +} + #[derive(Clone)] pub struct NetworkProxy { state: Arc, http_addr: SocketAddr, socks_addr: SocketAddr, socks_enabled: bool, + socks5_udp_enabled: bool, runtime_settings: Arc>, reserved_listeners: Option>, policy_decider: Option>, + environment_proxies: Arc>>, } impl std::fmt::Debug for NetworkProxy { @@ -640,20 +656,135 @@ impl NetworkProxy { }) } - pub fn apply_to_env(&self, env: &mut HashMap) { + fn apply_to_env_for_addrs( + &self, + env: &mut HashMap, + addrs: EnvironmentProxyAddrs, + ) { let runtime_settings = self.runtime_settings(); // Enforce proxying for child processes. Proxy endpoint values are always rewritten; // managed MITM CA vars preserve child-scoped overrides after proxy startup. apply_proxy_env_overrides( env, - self.http_addr, - self.socks_addr, + addrs.http_addr, + addrs.socks_addr, self.socks_enabled, runtime_settings.allow_local_binding, runtime_settings.mitm_ca_trust_bundle.as_ref(), ); } + pub fn apply_to_env(&self, env: &mut HashMap) { + self.apply_to_env_for_addrs( + env, + EnvironmentProxyAddrs { + http_addr: self.http_addr, + socks_addr: self.socks_addr, + }, + ); + } + + pub fn apply_to_env_for_environment( + &self, + env: &mut HashMap, + environment_id: &str, + ) -> Result<()> { + let addrs = self.environment_proxy_addrs(environment_id)?; + self.apply_to_env_for_addrs(env, addrs); + Ok(()) + } + + pub fn apply_to_env_for_optional_environment( + &self, + env: &mut HashMap, + environment_id: Option<&str>, + ) -> Result<()> { + match environment_id { + Some(environment_id) => self.apply_to_env_for_environment(env, environment_id), + None => { + self.apply_to_env(env); + Ok(()) + } + } + } + + fn environment_proxy_addrs(&self, environment_id: &str) -> Result { + let mut proxies = self + .environment_proxies + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(proxy) = proxies.get(environment_id) { + return Ok(proxy.addrs); + } + + let runtime = tokio::runtime::Handle::try_current().with_context(|| { + format!("failed to create network proxy for environment `{environment_id}`") + })?; + let listeners = + reserve_loopback_ephemeral_listeners(self.socks_enabled).with_context(|| { + format!("failed to reserve network proxy for environment `{environment_id}`") + })?; + let http_addr = listeners.http_addr().with_context(|| { + format!("failed to read HTTP proxy address for environment `{environment_id}`") + })?; + let socks_addr = listeners.socks_addr(self.socks_addr).with_context(|| { + format!("failed to read SOCKS proxy address for environment `{environment_id}`") + })?; + let addrs = EnvironmentProxyAddrs { + http_addr, + socks_addr, + }; + let ReservedListenerSet { + http_listener, + socks_listener, + } = listeners; + + let environment_id = environment_id.to_string(); + let http_state = self.state.clone(); + let http_decider = self.policy_decider.clone(); + let http_environment_id = Some(environment_id.clone()); + let http_task = runtime.spawn(async move { + http_proxy::run_http_proxy_with_std_listener( + http_state, + http_listener, + http_decider, + http_environment_id, + ) + .await + }); + + let socks_task = if self.socks_enabled { + let socks_state = self.state.clone(); + let socks_decider = self.policy_decider.clone(); + let socks_environment_id = Some(environment_id.clone()); + let socks5_udp_enabled = self.socks5_udp_enabled; + socks_listener.map(|listener| { + runtime.spawn(async move { + socks5::run_socks5_with_std_listener( + socks_state, + listener, + socks_decider, + socks_environment_id, + socks5_udp_enabled, + ) + .await + }) + }) + } else { + None + }; + + proxies.insert( + environment_id, + EnvironmentProxy { + addrs, + http_task, + socks_task, + }, + ); + Ok(addrs) + } + pub async fn replace_config_state(&self, new_state: ConfigState) -> Result<()> { let current_cfg = self.state.current_cfg().await?; anyhow::ensure!( @@ -717,10 +848,23 @@ impl NetworkProxy { let http_task = tokio::spawn(async move { match http_listener { Some(listener) => { - http_proxy::run_http_proxy_with_std_listener(http_state, listener, http_decider) - .await + http_proxy::run_http_proxy_with_std_listener( + http_state, + listener, + http_decider, + /*environment_id*/ None, + ) + .await + } + None => { + http_proxy::run_http_proxy( + http_state, + http_addr, + http_decider, + /*environment_id*/ None, + ) + .await } - None => http_proxy::run_http_proxy(http_state, http_addr, http_decider).await, } }); @@ -736,6 +880,7 @@ impl NetworkProxy { socks_state, listener, socks_decider, + /*environment_id*/ None, enable_socks5_udp, ) .await @@ -745,6 +890,7 @@ impl NetworkProxy { socks_state, socks_addr, socks_decider, + /*environment_id*/ None, enable_socks5_udp, ) .await @@ -758,6 +904,7 @@ impl NetworkProxy { Ok(NetworkProxyHandle { http_task: Some(http_task), socks_task, + environment_proxies: self.environment_proxies.clone(), completed: false, }) } @@ -766,6 +913,7 @@ impl NetworkProxy { pub struct NetworkProxyHandle { http_task: Option>>, socks_task: Option>>, + environment_proxies: Arc>>, completed: bool, } @@ -774,6 +922,7 @@ impl NetworkProxyHandle { Self { http_task: Some(tokio::spawn(async { Ok(()) })), socks_task: None, + environment_proxies: Arc::new(Mutex::new(HashMap::new())), completed: true, } } @@ -787,6 +936,7 @@ impl NetworkProxyHandle { None => None, }; self.completed = true; + abort_environment_proxies(self.environment_proxies.clone()).await; http_result??; if let Some(socks_result) = socks_result { socks_result??; @@ -796,6 +946,7 @@ impl NetworkProxyHandle { pub async fn shutdown(mut self) -> Result<()> { abort_tasks(self.http_task.take(), self.socks_task.take()).await; + abort_environment_proxies(self.environment_proxies.clone()).await; self.completed = true; Ok(()) } @@ -816,6 +967,21 @@ async fn abort_tasks( abort_task(socks_task).await; } +async fn abort_environment_proxies( + environment_proxies: Arc>>, +) { + let proxies = { + let mut guard = environment_proxies + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + guard.drain().map(|(_, proxy)| proxy).collect::>() + }; + for proxy in proxies { + abort_task(Some(proxy.http_task)).await; + abort_task(proxy.socks_task).await; + } +} + impl Drop for NetworkProxyHandle { fn drop(&mut self) { if self.completed { @@ -823,8 +989,10 @@ impl Drop for NetworkProxyHandle { } let http_task = self.http_task.take(); let socks_task = self.socks_task.take(); + let environment_proxies = self.environment_proxies.clone(); tokio::spawn(async move { abort_tasks(http_task, socks_task).await; + abort_environment_proxies(environment_proxies).await; }); } } @@ -905,6 +1073,33 @@ mod tests { ); } + #[tokio::test] + async fn apply_to_env_for_environment_uses_distinct_proxy_ports() -> Result<()> { + let state = Arc::new(network_proxy_state_for_policy( + NetworkProxySettings::default(), + )); + let proxy = NetworkProxy::builder().state(state).build().await?; + let handle = proxy.run().await?; + + let mut local_env = HashMap::new(); + proxy.apply_to_env_for_environment(&mut local_env, "local")?; + let mut remote_env = HashMap::new(); + proxy.apply_to_env_for_environment(&mut remote_env, "remote")?; + + assert_ne!(local_env.get("HTTP_PROXY"), remote_env.get("HTTP_PROXY")); + assert_ne!( + local_env.get("HTTP_PROXY"), + Some(&format!("http://{}", proxy.http_addr())) + ); + assert_ne!( + remote_env.get("HTTP_PROXY"), + Some(&format!("http://{}", proxy.http_addr())) + ); + + handle.shutdown().await?; + Ok(()) + } + #[tokio::test] async fn managed_proxy_builder_does_not_reserve_socks_listener_when_disabled() { let settings = NetworkProxySettings { diff --git a/codex-rs/network-proxy/src/socks5.rs b/codex-rs/network-proxy/src/socks5.rs index a4f36db562aa..5cc7b2eed39f 100644 --- a/codex-rs/network-proxy/src/socks5.rs +++ b/codex-rs/network-proxy/src/socks5.rs @@ -64,6 +64,7 @@ pub async fn run_socks5( state: Arc, addr: SocketAddr, policy_decider: Option>, + environment_id: Option, enable_socks5_udp: bool, ) -> Result<()> { let listener = TcpListener::build() @@ -74,24 +75,40 @@ pub async fn run_socks5( .map_err(anyhow::Error::from) .with_context(|| format!("bind SOCKS5 proxy: {addr}"))?; - run_socks5_with_listener(state, listener, policy_decider, enable_socks5_udp).await + run_socks5_with_listener( + state, + listener, + policy_decider, + environment_id, + enable_socks5_udp, + ) + .await } pub async fn run_socks5_with_std_listener( state: Arc, listener: StdTcpListener, policy_decider: Option>, + environment_id: Option, enable_socks5_udp: bool, ) -> Result<()> { let listener = TcpListener::try_from(listener).context("convert std listener to SOCKS5 proxy listener")?; - run_socks5_with_listener(state, listener, policy_decider, enable_socks5_udp).await + run_socks5_with_listener( + state, + listener, + policy_decider, + environment_id, + enable_socks5_udp, + ) + .await } async fn run_socks5_with_listener( state: Arc, listener: TcpListener, policy_decider: Option>, + environment_id: Option, enable_socks5_udp: bool, ) -> Result<()> { let addr = listener @@ -115,10 +132,12 @@ async fn run_socks5_with_listener( let tcp_connector = TargetCheckedTcpConnector::new(state.clone()); let policy_tcp_connector = service_fn({ let policy_decider = policy_decider.clone(); + let environment_id = environment_id.clone(); move |req: TcpRequest| { let tcp_connector = tcp_connector.clone(); let policy_decider = policy_decider.clone(); - async move { handle_socks5_tcp(req, tcp_connector, policy_decider).await } + let environment_id = environment_id.clone(); + async move { handle_socks5_tcp(req, tcp_connector, policy_decider, environment_id).await } } }); @@ -131,13 +150,18 @@ async fn run_socks5_with_listener( if enable_socks5_udp { let udp_state = state.clone(); let udp_decider = policy_decider.clone(); - let udp_relay = DefaultUdpRelay::default().with_async_inspector(service_fn({ - move |request: RelayRequest| { - let udp_state = udp_state.clone(); - let udp_decider = udp_decider.clone(); - async move { inspect_socks5_udp(request, udp_state, udp_decider).await } - } - })); + let udp_relay = + DefaultUdpRelay::default().with_async_inspector(service_fn({ + let environment_id = environment_id.clone(); + move |request: RelayRequest| { + let udp_state = udp_state.clone(); + let udp_decider = udp_decider.clone(); + let environment_id = environment_id.clone(); + async move { + inspect_socks5_udp(request, udp_state, udp_decider, environment_id).await + } + } + })); let socks_acceptor = base.with_udp_associator(udp_relay); listener .serve(AddInputExtensionLayer::new(state).into_layer(socks_acceptor)) @@ -154,6 +178,7 @@ async fn handle_socks5_tcp( req: TcpRequest, tcp_connector: TargetCheckedTcpConnector, policy_decider: Option>, + environment_id: Option, ) -> Result, BoxError> { let app_state = req .extensions() @@ -268,7 +293,7 @@ async fn handle_socks5_tcp( protocol: NetworkProtocol::Socks5Tcp, host: host.clone(), port, - environment_id: None, + environment_id, client_addr: client.clone(), method: None, command: None, @@ -519,6 +544,7 @@ async fn inspect_socks5_udp( request: RelayRequest, state: Arc, policy_decider: Option>, + environment_id: Option, ) -> io::Result { let RelayRequest { server_address, @@ -625,7 +651,7 @@ async fn inspect_socks5_udp( protocol: NetworkProtocol::Socks5Udp, host: host.clone(), port, - environment_id: None, + environment_id, client_addr: client.clone(), method: None, command: None, @@ -783,6 +809,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state.clone()), /*policy_decider*/ None, + /*environment_id*/ None, ) .await }) @@ -826,6 +853,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state), /*policy_decider*/ None, + /*environment_id*/ None, ) .await .expect("limited-mode HTTPS should use MITM"); @@ -851,6 +879,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state), /*policy_decider*/ None, + /*environment_id*/ None, ) .await }) @@ -896,6 +925,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state), /*policy_decider*/ None, + /*environment_id*/ None, ) .await .expect_err("limited-mode HTTPS requires MITM"); @@ -933,6 +963,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state), /*policy_decider*/ None, + /*environment_id*/ None, ) .await .expect("hooked HTTPS should use MITM"); @@ -967,6 +998,7 @@ mod tests { request, TargetCheckedTcpConnector::new(state), /*policy_decider*/ None, + /*environment_id*/ None, ) .await .expect_err("hooked non-HTTPS SOCKS should require MITM"); @@ -992,7 +1024,10 @@ mod tests { }; let (result, events) = capture_events(|| async { - inspect_socks5_udp(request, state, /*policy_decider*/ None).await + inspect_socks5_udp( + request, state, /*policy_decider*/ None, /*environment_id*/ None, + ) + .await }) .await; assert!(result.is_err(), "limited-mode UDP request should be denied"); diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index bebfefe067cb..22c4984c5a6f 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -48,6 +48,9 @@ impl From for CodexErr { SandboxTransformError::MissingLinuxSandboxExecutable => { CodexErr::LandlockSandboxExecutableNotProvided } + SandboxTransformError::EnvironmentNetworkProxy(message) => { + CodexErr::UnsupportedOperation(message) + } #[cfg(target_os = "linux")] SandboxTransformError::Wsl1UnsupportedForBubblewrap => { CodexErr::UnsupportedOperation(crate::bwrap::WSL1_BWRAP_WARNING.to_string()) diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index f00e056907d3..dd7aba3614cd 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -209,6 +209,7 @@ pub enum SandboxTransformError { source: io::Error, }, MissingLinuxSandboxExecutable, + EnvironmentNetworkProxy(String), #[cfg(target_os = "linux")] Wsl1UnsupportedForBubblewrap, #[cfg(not(target_os = "macos"))] @@ -233,6 +234,9 @@ impl std::fmt::Display for SandboxTransformError { Self::MissingLinuxSandboxExecutable => { write!(f, "missing codex-linux-sandbox executable path") } + Self::EnvironmentNetworkProxy(err) => { + write!(f, "failed to prepare environment network proxy: {err}") + } #[cfg(target_os = "linux")] Self::Wsl1UnsupportedForBubblewrap => write!(f, "{WSL1_BWRAP_WARNING}"), #[cfg(not(target_os = "macos"))] @@ -251,6 +255,7 @@ impl std::error::Error for SandboxTransformError { Self::InvalidCommandCwd { source, .. } | Self::InvalidSandboxPolicyCwd { source, .. } => Some(source), Self::MissingLinuxSandboxExecutable => None, + Self::EnvironmentNetworkProxy(_) => None, #[cfg(target_os = "linux")] Self::Wsl1UnsupportedForBubblewrap => None, #[cfg(not(target_os = "macos"))] @@ -347,9 +352,11 @@ impl SandboxManager { network_sandbox_policy: pending.effective_network_policy, sandbox_policy_cwd: pending.native_sandbox_policy_cwd.as_path(), enforce_managed_network, + environment_id, network, extra_allow_unix_sockets: &[], - }); + }) + .map_err(SandboxTransformError::EnvironmentNetworkProxy)?; let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); full_command.append(&mut args); diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index 7057ec46b96a..d3233e705b3e 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -104,8 +104,9 @@ struct UnixSocketPathParam { fn proxy_policy_inputs( network: Option<&NetworkProxy>, + environment_id: Option<&str>, extra_allow_unix_sockets: &[AbsolutePathBuf], -) -> ProxyPolicyInputs { +) -> Result { let extra_allowed = extra_allow_unix_sockets .iter() .filter_map(|socket_path| normalize_path_for_sandbox(socket_path.as_path())) @@ -114,7 +115,9 @@ fn proxy_policy_inputs( match network { Some(network) => { let mut env = HashMap::new(); - network.apply_to_env(&mut env); + network + .apply_to_env_for_optional_environment(&mut env, environment_id) + .map_err(|err| err.to_string())?; let unix_domain_socket_policy = if network.dangerously_allow_all_unix_sockets() { UnixDomainSocketPolicy::AllowAll } else { @@ -136,19 +139,19 @@ fn proxy_policy_inputs( allowed.extend(extra_allowed); UnixDomainSocketPolicy::Restricted { allowed } }; - ProxyPolicyInputs { + Ok(ProxyPolicyInputs { ports: proxy_loopback_ports_from_env(&env), has_proxy_config: has_proxy_url_env_vars(&env), allow_local_binding: network.allow_local_binding(), unix_domain_socket_policy, - } + }) } - None => ProxyPolicyInputs { + None => Ok(ProxyPolicyInputs { unix_domain_socket_policy: UnixDomainSocketPolicy::Restricted { allowed: extra_allowed, }, ..Default::default() - }, + }), } } @@ -572,7 +575,7 @@ fn create_seatbelt_command_args_for_legacy_policy( sandbox_policy_cwd: &Path, enforce_managed_network: bool, network: Option<&NetworkProxy>, -) -> Vec { +) -> Result, String> { let file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( sandbox_policy, sandbox_policy_cwd, @@ -583,6 +586,7 @@ fn create_seatbelt_command_args_for_legacy_policy( network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy), sandbox_policy_cwd, enforce_managed_network, + environment_id: None, network, extra_allow_unix_sockets: &[], }) @@ -595,17 +599,21 @@ pub struct CreateSeatbeltCommandArgsParams<'a> { pub network_sandbox_policy: NetworkSandboxPolicy, pub sandbox_policy_cwd: &'a Path, pub enforce_managed_network: bool, + pub environment_id: Option<&'a str>, pub network: Option<&'a NetworkProxy>, pub extra_allow_unix_sockets: &'a [AbsolutePathBuf], } -pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -> Vec { +pub fn create_seatbelt_command_args( + args: CreateSeatbeltCommandArgsParams<'_>, +) -> Result, String> { let CreateSeatbeltCommandArgsParams { command, file_system_sandbox_policy, network_sandbox_policy, sandbox_policy_cwd, enforce_managed_network, + environment_id, network, extra_allow_unix_sockets, } = args; @@ -701,7 +709,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - } }; - let proxy = proxy_policy_inputs(network, extra_allow_unix_sockets); + let proxy = proxy_policy_inputs(network, environment_id, extra_allow_unix_sockets)?; let network_policy = dynamic_network_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); @@ -737,7 +745,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) - seatbelt_args.extend(definition_args); seatbelt_args.push("--".to_string()); seatbelt_args.extend(command); - seatbelt_args + Ok(seatbelt_args) } #[cfg(test)] diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 337eb56378ea..e0ff0e66523b 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -205,9 +205,11 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() network_sandbox_policy: NetworkSandboxPolicy::Restricted, sandbox_policy_cwd: Path::new("/"), enforce_managed_network: false, + environment_id: None, network: None, extra_allow_unix_sockets: &[], - }); + }) + .unwrap(); let policy = seatbelt_policy_arg(&args); let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/")); @@ -277,9 +279,11 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { network_sandbox_policy: NetworkSandboxPolicy::Restricted, sandbox_policy_cwd: Path::new("/"), enforce_managed_network: false, + environment_id: None, network: None, extra_allow_unix_sockets: &[], - }); + }) + .unwrap(); let policy = seatbelt_policy_arg(&args); let readable_roots = file_system_policy.get_readable_roots_with_cwd(Path::new("/")); @@ -392,7 +396,8 @@ fn seatbelt_args_without_extension_profile_keep_legacy_preferences_read_access() cwd.as_path(), /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let policy = &args[1]; assert!(policy.contains("(allow user-preference-read)")); assert!(!policy.contains("(allow user-preference-write)")); @@ -580,9 +585,11 @@ fn create_seatbelt_args_allowlists_explicit_unix_socket_paths_without_proxy() { network_sandbox_policy: NetworkSandboxPolicy::Restricted, sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, + environment_id: None, network: None, extra_allow_unix_sockets: &extra_allow_unix_sockets, - }); + }) + .unwrap(); let policy = seatbelt_policy_arg(&args); assert!( @@ -638,9 +645,11 @@ async fn create_seatbelt_args_merges_proxy_and_explicit_unix_socket_paths() -> a network_sandbox_policy: NetworkSandboxPolicy::Restricted, sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, + environment_id: None, network: Some(&network_proxy), extra_allow_unix_sockets: &extra_allow_unix_sockets, - }); + }) + .unwrap(); let expected_explicit_socket = normalize_path_for_sandbox(Path::new(explicit_socket)) .expect("explicit socket root should normalize"); @@ -679,9 +688,11 @@ fn create_seatbelt_args_preserves_full_network_with_explicit_unix_socket_paths() network_sandbox_policy: NetworkSandboxPolicy::Enabled, sandbox_policy_cwd: cwd.path(), enforce_managed_network: false, + environment_id: None, network: None, extra_allow_unix_sockets: &extra_allow_unix_sockets, - }); + }) + .unwrap(); let policy = seatbelt_policy_arg(&args); assert!( @@ -869,7 +880,8 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { &cwd, /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let policy_text = seatbelt_policy_arg(&args); assert!( @@ -1008,7 +1020,8 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { &cwd, /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_hooks_file_args) .current_dir(&cwd) @@ -1044,7 +1057,8 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() { &cwd, /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_allowed_file_args) .current_dir(&cwd) @@ -1108,7 +1122,8 @@ fn create_seatbelt_args_block_first_time_dot_codex_creation_with_metadata_name_r repo_root.as_path(), /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let policy_text = seatbelt_policy_arg(&args); assert!( @@ -1160,7 +1175,8 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() { &cwd, /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&args) @@ -1196,7 +1212,8 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() { &cwd, /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&gitdir_args) .current_dir(&cwd) @@ -1259,7 +1276,8 @@ fn create_seatbelt_args_for_cwd_as_git_repo() { vulnerable_root.as_path(), /*enforce_managed_network*/ false, /*network*/ None, - ); + ) + .unwrap(); let slash_tmp = PathBuf::from("/tmp") .canonicalize()