diff --git a/codex-rs/exec-server/src/process_sandbox_tests.rs b/codex-rs/exec-server/src/process_sandbox_tests.rs index 0e80e938dd..3fed99c833 100644 --- a/codex-rs/exec-server/src/process_sandbox_tests.rs +++ b/codex-rs/exec-server/src/process_sandbox_tests.rs @@ -9,6 +9,10 @@ use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::NetworkPolicyAuditObserver; use codex_network_proxy::NetworkPolicyDecider; use codex_network_proxy::NetworkProxyConfig; +#[cfg(target_os = "macos")] +use codex_network_proxy::NetworkUnixSocketPermission; +#[cfg(target_os = "macos")] +use codex_network_proxy::NetworkUnixSocketPermissions; use codex_network_proxy::PROXY_ATTRIBUTION_TOKEN_ENV_KEY; use codex_network_proxy::RemoteNetworkProxyConfig; use codex_network_proxy::RemoteNetworkProxyLaunchConfig; @@ -210,8 +214,7 @@ async fn sandbox_request_routes_custom_arg0_to_inner_helper() { } #[cfg(target_os = "macos")] -#[tokio::test] -async fn sandbox_request_allows_prepared_managed_proxy_port() { +fn managed_network_sandbox_request() -> (ExecParams, ExecServerRuntimePaths) { let cwd: AbsolutePathBuf = std::env::current_dir() .expect("current directory") .try_into() @@ -237,13 +240,38 @@ async fn sandbox_request_allows_prepared_managed_proxy_port() { arg0: None, sandbox: Some(sandbox), enforce_managed_network: true, - managed_network: Some(ManagedNetworkSandboxContext { - loopback_ports: vec![43123], - allow_local_binding: false, - ..Default::default() - }), + managed_network: None, network_proxy: None, }; + (params, runtime_paths) +} + +#[cfg(target_os = "macos")] +fn seatbelt_policy_arg(command: &[String]) -> &str { + command + .windows(2) + .find_map(|args| (args[0] == "-p").then_some(args[1].as_str())) + .expect("Seatbelt policy argument") +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn sandbox_request_preserves_prepared_managed_network_policy() { + let (mut params, runtime_paths) = managed_network_sandbox_request(); + let socket_dir = tempfile::tempdir().expect("temporary socket directory"); + let allowed_socket = socket_dir + .path() + .canonicalize() + .expect("canonical socket directory") + .join("allowed.sock") + .to_string_lossy() + .into_owned(); + params.managed_network = Some(ManagedNetworkSandboxContext { + loopback_ports: vec![43123], + allow_local_binding: false, + allow_unix_sockets: vec![allowed_socket.clone()], + dangerously_allow_all_unix_sockets: false, + }); let prepared = prepare_exec_request( ¶ms, @@ -254,13 +282,151 @@ async fn sandbox_request_allows_prepared_managed_proxy_port() { ) .await .expect("prepare managed-network sandbox request"); - let policy = prepared + let policy = seatbelt_policy_arg(&prepared.command); + let unix_socket_definitions = prepared .command - .windows(2) - .find_map(|args| (args[0] == "-p").then_some(args[1].as_str())) - .expect("Seatbelt policy argument"); + .iter() + .filter(|arg| arg.starts_with("-DUNIX_SOCKET_PATH_")) + .cloned() + .collect::>(); assert!(policy.contains("(allow network-outbound (remote ip \"localhost:43123\"))")); + assert!(policy.contains("(allow system-socket (socket-domain AF_UNIX))")); + assert!(policy.contains( + "(allow network-outbound (remote unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))" + )); + assert_eq!( + unix_socket_definitions, + vec![format!("-DUNIX_SOCKET_PATH_0={allowed_socket}")] + ); + assert!(!policy.contains("(allow network-outbound (remote unix-socket))")); + assert!(!policy.contains("(allow network-outbound)\n")); +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn sandbox_request_only_allows_all_unix_sockets_when_configured() { + let (mut params, runtime_paths) = managed_network_sandbox_request(); + params.managed_network = Some(ManagedNetworkSandboxContext { + loopback_ports: vec![43123], + ..ManagedNetworkSandboxContext::default() + }); + + for allow_all in [false, true] { + params + .managed_network + .as_mut() + .expect("managed network context") + .dangerously_allow_all_unix_sockets = allow_all; + let prepared = prepare_exec_request( + ¶ms, + HashMap::new(), + Some(&runtime_paths), + /*network_policy_decider*/ None, + /*network_policy_audit_observer*/ None, + ) + .await + .expect("prepare managed-network sandbox request"); + let policy = seatbelt_policy_arg(&prepared.command); + + assert_eq!( + ( + policy.contains("(allow system-socket (socket-domain AF_UNIX))"), + policy.contains("(allow network-bind (local unix-socket))"), + policy.contains("(allow network-outbound (remote unix-socket))"), + ), + (allow_all, allow_all, allow_all) + ); + assert!(!policy.contains("(allow network-outbound)\n")); + assert!( + !prepared + .command + .iter() + .any(|arg| arg.starts_with("-DUNIX_SOCKET_PATH_")) + ); + } +} + +#[cfg(target_os = "macos")] +#[tokio::test] +async fn sandbox_request_preserves_executor_local_proxy_unix_socket_policy() { + let (mut params, runtime_paths) = managed_network_sandbox_request(); + let socket_dir = tempfile::tempdir().expect("temporary socket directory"); + let socket_root = socket_dir + .path() + .canonicalize() + .expect("canonical socket directory"); + let allowed_socket = socket_root + .join("allowed.sock") + .to_string_lossy() + .into_owned(); + let denied_socket = socket_root + .join("denied.sock") + .to_string_lossy() + .into_owned(); + let config = NetworkProxyConfig { + enabled: true, + enable_socks5: false, + unix_sockets: Some(NetworkUnixSocketPermissions { + entries: [ + (allowed_socket.clone(), NetworkUnixSocketPermission::Allow), + (denied_socket, NetworkUnixSocketPermission::Deny), + ] + .into_iter() + .collect(), + }), + ..NetworkProxyConfig::default() + }; + params.network_proxy = Some(RemoteNetworkProxyLaunchConfig::new( + RemoteNetworkProxyConfig::from_effective_config(&config) + .expect("supported remote proxy config"), + )); + + let prepared = prepare_exec_request( + ¶ms, + HashMap::new(), + Some(&runtime_paths), + /*network_policy_decider*/ None, + /*network_policy_audit_observer*/ None, + ) + .await + .expect("prepare sandbox request with executor-local proxy"); + let policy = seatbelt_policy_arg(&prepared.command); + let proxy_addr: SocketAddr = prepared + .env + .get("HTTP_PROXY") + .expect("HTTP proxy env") + .strip_prefix("http://") + .expect("HTTP proxy scheme") + .parse() + .expect("HTTP proxy address"); + let proxy_port = proxy_addr.port(); + let unix_socket_definitions = prepared + .command + .iter() + .filter(|arg| arg.starts_with("-DUNIX_SOCKET_PATH_")) + .cloned() + .collect::>(); + + assert!(policy.contains(&format!( + "(allow network-outbound (remote ip \"localhost:{proxy_port}\"))" + ))); + assert!(policy.contains( + "(allow network-outbound (remote unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))" + )); + assert_eq!( + unix_socket_definitions, + vec![format!("-DUNIX_SOCKET_PATH_0={allowed_socket}")] + ); + assert!(!policy.contains("(allow network-outbound (remote unix-socket))")); + assert!(!policy.contains("(allow network-outbound)\n")); + + prepared + .network_proxy_handle + .expect("running executor proxy") + .shutdown() + .await + .expect("shut down executor proxy"); } #[tokio::test] diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index 4e5c9a7571..765c89d7dd 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -130,6 +130,27 @@ impl Default for UnixDomainSocketPolicy { } } +impl UnixDomainSocketPolicy { + fn from_allowlist(paths: &[String], extra_allowed: Vec) -> Self { + let mut allowed = paths + .iter() + .filter_map(|socket_path| { + match normalize_path_for_sandbox(Path::new(socket_path)) { + Some(path) => Some(path), + None => { + warn!( + "ignoring network.allow_unix_sockets entry because it could not be normalized: {socket_path}" + ); + None + } + } + }) + .collect::>(); + allowed.extend(extra_allowed); + Self::Restricted { allowed } + } +} + #[derive(Debug, Clone)] struct UnixSocketPathParam { index: usize, @@ -147,30 +168,22 @@ fn proxy_policy_inputs( .filter_map(|socket_path| normalize_path_for_sandbox(socket_path.as_path())) .collect::>(); - let unix_domain_socket_policy = match network { - Some(network) if network.dangerously_allow_all_unix_sockets() => { + // Prefer this command's prepared Unix-socket policy. + // Fall back to the live proxy only when no prepared context exists. + let unix_domain_socket_policy = match (managed_network, network) { + (Some(context), _) if context.dangerously_allow_all_unix_sockets => { UnixDomainSocketPolicy::AllowAll } - Some(network) => { - let mut allowed = network - .allow_unix_sockets() - .iter() - .filter_map(|socket_path| { - match normalize_path_for_sandbox(Path::new(socket_path)) { - Some(path) => Some(path), - None => { - warn!( - "ignoring network.allow_unix_sockets entry because it could not be normalized: {socket_path}" - ); - None - } - } - }) - .collect::>(); - allowed.extend(extra_allowed); - UnixDomainSocketPolicy::Restricted { allowed } + (Some(context), _) => { + UnixDomainSocketPolicy::from_allowlist(&context.allow_unix_sockets, extra_allowed) } - None => UnixDomainSocketPolicy::Restricted { + (None, Some(network)) if network.dangerously_allow_all_unix_sockets() => { + UnixDomainSocketPolicy::AllowAll + } + (None, Some(network)) => { + UnixDomainSocketPolicy::from_allowlist(&network.allow_unix_sockets(), extra_allowed) + } + (None, None) => UnixDomainSocketPolicy::Restricted { allowed: extra_allowed, }, }; diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index e46c5c921e..4b2a36f670 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -798,6 +798,109 @@ fn prepared_managed_network_context_allows_only_its_proxy_ports() { assert!(!policy.contains("(allow network-outbound (remote ip \"localhost:9999\"))")); assert!(!policy.contains("(allow network-bind (local ip \"*:*\"))")); assert!(!policy.contains("(allow network-outbound)\n")); + assert!(!policy.contains("(allow system-socket (socket-domain AF_UNIX))")); + assert!(!policy.contains("(allow network-outbound (remote unix-socket))")); +} + +#[tokio::test] +async fn prepared_managed_network_context_takes_precedence_over_live_proxy_socket_policy() +-> anyhow::Result<()> { + let cwd = TempDir::new().expect("temp cwd"); + let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &SandboxPolicy::new_read_only_policy(), + cwd.path(), + ); + let network_config = NetworkProxyConfig { + enabled: true, + mode: NetworkMode::Full, + dangerously_allow_all_unix_sockets: true, + ..Default::default() + }; + let state = build_config_state(network_config, NetworkProxyConstraints::default())?; + let network_proxy = NetworkProxy::builder() + .state(Arc::new(NetworkProxyState::with_reloader( + state, + Arc::new(TestConfigReloader), + ))) + .managed_by_codex(/*managed_by_codex*/ false) + .build() + .await?; + let prepared_socket = "/tmp/codex-prepared-use"; + let explicit_socket = "/tmp/codex-browser-use"; + let managed_network = ManagedNetworkSandboxContext { + loopback_ports: vec![43123], + allow_unix_sockets: vec![prepared_socket.to_string(), "relative.sock".to_string()], + ..Default::default() + }; + let extra_allow_unix_sockets = vec![absolute_path(explicit_socket)]; + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec!["/usr/bin/true".to_string()], + file_system_sandbox_policy: &file_system_policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: cwd.path(), + enforce_managed_network: true, + managed_network: Some(&managed_network), + environment_id: None, + network: Some(&network_proxy), + extra_allow_unix_sockets: &extra_allow_unix_sockets, + }) + .expect("create seatbelt args"); + + let policy = seatbelt_policy_arg(&args); + assert!(policy.contains("(allow network-outbound (remote ip \"localhost:43123\"))")); + assert!(policy.contains("(allow system-socket (socket-domain AF_UNIX))")); + assert!(!policy.contains("(allow network-bind (local unix-socket))")); + assert!(!policy.contains("(allow network-outbound (remote unix-socket))")); + let expected_explicit_socket = normalize_path_for_sandbox(Path::new(explicit_socket)) + .expect("explicit socket root should normalize"); + let expected_prepared_socket = normalize_path_for_sandbox(Path::new(prepared_socket)) + .expect("prepared socket root should normalize"); + assert_eq!( + args.iter() + .filter(|arg| arg.starts_with("-DUNIX_SOCKET_PATH_")) + .cloned() + .collect::>(), + vec![ + format!( + "-DUNIX_SOCKET_PATH_0={}", + expected_explicit_socket.display() + ), + format!( + "-DUNIX_SOCKET_PATH_1={}", + expected_prepared_socket.display() + ), + ] + ); + + // An empty prepared policy must not inherit the live proxy's allow-all grant. + let managed_network = ManagedNetworkSandboxContext { + loopback_ports: vec![43123], + ..Default::default() + }; + let args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { + command: vec!["/usr/bin/true".to_string()], + file_system_sandbox_policy: &file_system_policy, + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + sandbox_policy_cwd: cwd.path(), + enforce_managed_network: true, + managed_network: Some(&managed_network), + environment_id: None, + network: Some(&network_proxy), + extra_allow_unix_sockets: &[], + }) + .expect("create seatbelt args for empty prepared policy"); + + let policy = seatbelt_policy_arg(&args); + assert!(policy.contains("(allow network-outbound (remote ip \"localhost:43123\"))")); + assert!(!policy.contains("(allow system-socket (socket-domain AF_UNIX))")); + assert!(!policy.contains("(allow network-bind (local unix-socket))")); + assert!(!policy.contains("(allow network-outbound (remote unix-socket))")); + assert!( + !args + .iter() + .any(|arg| arg.starts_with("-DUNIX_SOCKET_PATH_")) + ); + Ok(()) } #[test]