From e790dadfc34cbf2e53712a628e3cf3544742cc46 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 10 Apr 2026 13:50:10 -0700 Subject: [PATCH] fix(sandboxing): keep managed yolo unsandboxed Co-authored-by: Codex noreply@openai.com --- codex-rs/sandboxing/src/manager.rs | 37 +---- codex-rs/sandboxing/src/manager_tests.rs | 57 +------ codex-rs/sandboxing/src/policy_transforms.rs | 7 +- .../sandboxing/src/policy_transforms_tests.rs | 24 +++ codex-rs/sandboxing/src/seatbelt.rs | 5 - .../sandboxing/src/seatbelt/network_only.rs | 141 ------------------ codex-rs/sandboxing/src/seatbelt_tests.rs | 117 --------------- 7 files changed, 38 insertions(+), 350 deletions(-) delete mode 100644 codex-rs/sandboxing/src/seatbelt/network_only.rs diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index b811768d76..cbebda8947 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -8,14 +8,10 @@ use crate::policy_transforms::should_require_platform_sandbox; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; #[cfg(target_os = "macos")] -use crate::seatbelt::create_network_only_seatbelt_command_args; -#[cfg(target_os = "macos")] use crate::seatbelt::create_seatbelt_command_args_for_policies; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; -#[cfg(target_os = "macos")] -use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; @@ -66,15 +62,6 @@ pub fn get_platform_sandbox(windows_sandbox_enabled: bool) -> Option bool { - enforce_managed_network - && matches!(file_system_policy.kind, FileSystemSandboxKind::Unrestricted) -} - #[derive(Debug)] pub struct SandboxCommand { pub program: OsString, @@ -214,26 +201,14 @@ impl SandboxManager { #[cfg(target_os = "macos")] SandboxType::MacosSeatbelt => { let command = os_argv_to_strings(argv); - let mut args = if should_use_network_only_seatbelt( + let mut args = create_seatbelt_command_args_for_policies( + command, &effective_file_system_policy, + effective_network_policy, + sandbox_policy_cwd, enforce_managed_network, - ) { - create_network_only_seatbelt_command_args( - command, - effective_network_policy, - enforce_managed_network, - network, - ) - } else { - create_seatbelt_command_args_for_policies( - command, - &effective_file_system_policy, - effective_network_policy, - sandbox_policy_cwd, - enforce_managed_network, - network, - ) - }; + network, + ); 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/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index ddc1227e3a..67fffd2d7c 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -37,10 +37,8 @@ fn danger_full_access_defaults_to_no_sandbox_without_network_requirements() { } #[test] -fn danger_full_access_uses_platform_sandbox_with_network_requirements() { +fn danger_full_access_skips_platform_sandbox_with_managed_network() { let manager = SandboxManager::new(); - let expected = - get_platform_sandbox(/*windows_sandbox_enabled*/ false).unwrap_or(SandboxType::None); let sandbox = manager.select_initial( &FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Enabled, @@ -48,58 +46,7 @@ fn danger_full_access_uses_platform_sandbox_with_network_requirements() { WindowsSandboxLevel::Disabled, /*has_managed_network_requirements*/ true, ); - assert_eq!(sandbox, expected); -} - -#[cfg(target_os = "macos")] -#[test] -fn transform_uses_network_only_seatbelt_for_managed_danger_full_access() { - let manager = SandboxManager::new(); - let cwd = AbsolutePathBuf::current_dir().expect("current dir"); - let exec_request = manager - .transform(SandboxTransformRequest { - command: SandboxCommand { - program: "true".into(), - args: Vec::new(), - cwd: cwd.clone(), - env: HashMap::new(), - additional_permissions: None, - }, - policy: &SandboxPolicy::DangerFullAccess, - file_system_policy: &FileSystemSandboxPolicy::unrestricted(), - network_policy: NetworkSandboxPolicy::Enabled, - sandbox: SandboxType::MacosSeatbelt, - enforce_managed_network: true, - network: None, - sandbox_policy_cwd: cwd.as_path(), - codex_linux_sandbox_exe: None, - use_legacy_landlock: false, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - windows_sandbox_private_desktop: false, - }) - .expect("transform"); - let policy_index = exec_request - .command - .iter() - .position(|arg| arg == "-p") - .expect("seatbelt command should include policy"); - let policy = exec_request - .command - .get(policy_index + 1) - .expect("seatbelt command should include policy text"); - - assert!( - policy.starts_with("(version 1)\n(allow default)"), - "managed full-access Seatbelt should use the open network-only profile:\n{policy}" - ); - assert!( - policy.contains("(deny network-outbound"), - "managed full-access Seatbelt should still enforce network restrictions:\n{policy}" - ); - assert!( - !policy.contains("(deny default)"), - "managed full-access Seatbelt should not use the closed base profile:\n{policy}" - ); + assert_eq!(sandbox, SandboxType::None); } #[test] diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 25c79fcc9e..5bcbf719d9 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -425,7 +425,12 @@ pub fn should_require_platform_sandbox( has_managed_network_requirements: bool, ) -> bool { if has_managed_network_requirements { - return true; + // DangerFullAccess should not become a platform sandbox just because a + // managed proxy is configured; even open Seatbelt profiles break some + // macOS APIs such as launchctl bootstrap. + return !network_policy.is_enabled() + || !file_system_policy.has_full_disk_read_access() + || !file_system_policy.has_full_disk_write_access(); } if !network_policy.is_enabled() { diff --git a/codex-rs/sandboxing/src/policy_transforms_tests.rs b/codex-rs/sandboxing/src/policy_transforms_tests.rs index 10f49d95f3..94b1a4c595 100644 --- a/codex-rs/sandboxing/src/policy_transforms_tests.rs +++ b/codex-rs/sandboxing/src/policy_transforms_tests.rs @@ -47,6 +47,30 @@ fn full_access_restricted_policy_skips_platform_sandbox_when_network_is_enabled( ); } +#[test] +fn full_access_policy_skips_platform_sandbox_for_managed_enabled_network() { + assert_eq!( + should_require_platform_sandbox( + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Enabled, + /*has_managed_network_requirements*/ true + ), + false + ); +} + +#[test] +fn full_access_policy_uses_platform_sandbox_for_managed_restricted_network() { + assert_eq!( + should_require_platform_sandbox( + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Restricted, + /*has_managed_network_requirements*/ true + ), + true + ); +} + #[test] fn root_write_policy_with_carveouts_still_uses_platform_sandbox() { let blocked = AbsolutePathBuf::resolve_path_against_base( diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index f23f7ec518..bb784bc0df 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -15,15 +15,10 @@ use std::path::PathBuf; use tracing::warn; use url::Url; -mod network_only; - const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_policy.sbpl"); const MACOS_RESTRICTED_READ_ONLY_PLATFORM_DEFAULTS: &str = include_str!("restricted_read_only_platform_defaults.sbpl"); -pub(crate) use network_only::create_network_only_seatbelt_command_args; -#[cfg(test)] -use network_only::network_only_policy_for_network; /// When working with `sandbox-exec`, only consider `sandbox-exec` in `/usr/bin` /// to defend against an attacker trying to inject a malicious version on the diff --git a/codex-rs/sandboxing/src/seatbelt/network_only.rs b/codex-rs/sandboxing/src/seatbelt/network_only.rs deleted file mode 100644 index 364826940b..0000000000 --- a/codex-rs/sandboxing/src/seatbelt/network_only.rs +++ /dev/null @@ -1,141 +0,0 @@ -use super::ProxyPolicyInputs; -use super::UnixDomainSocketPolicy; -use super::proxy_policy_inputs; -use super::unix_socket_path_param_key; -use super::unix_socket_path_params; -use codex_network_proxy::NetworkProxy; -use codex_protocol::permissions::NetworkSandboxPolicy; -use std::path::PathBuf; - -const MACOS_SEATBELT_NETWORK_ONLY_BASE_POLICY: &str = "(version 1)\n(allow default)"; - -pub(crate) fn create_network_only_seatbelt_command_args( - command: Vec, - network_sandbox_policy: NetworkSandboxPolicy, - enforce_managed_network: bool, - network: Option<&NetworkProxy>, -) -> Vec { - let proxy = proxy_policy_inputs(network); - let network_policy = - network_only_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); - let full_policy = [MACOS_SEATBELT_NETWORK_ONLY_BASE_POLICY, &network_policy].join("\n"); - - let mut seatbelt_args: Vec = vec!["-p".to_string(), full_policy]; - seatbelt_args.extend( - unix_socket_dir_params(&proxy) - .into_iter() - .map(|(key, value): (String, PathBuf)| format!("-D{key}={}", value.to_string_lossy())), - ); - seatbelt_args.push("--".to_string()); - seatbelt_args.extend(command); - seatbelt_args -} - -pub(super) fn network_only_policy_for_network( - network_policy: NetworkSandboxPolicy, - enforce_managed_network: bool, - proxy: &ProxyPolicyInputs, -) -> String { - if network_policy.is_enabled() - && !enforce_managed_network - && proxy.ports.is_empty() - && !proxy.has_proxy_config - { - return String::new(); - } - - let mut policy = String::new(); - push_ip_network_policy(&mut policy, proxy); - push_unix_socket_network_policy(&mut policy, proxy); - policy -} - -fn push_ip_network_policy(policy: &mut String, proxy: &ProxyPolicyInputs) { - let mut outbound_exceptions = Vec::new(); - if proxy.allow_local_binding { - outbound_exceptions.push(r#"(remote ip "localhost:*")"#.to_string()); - } - outbound_exceptions.extend( - proxy - .ports - .iter() - .map(|port| format!(r#"(remote ip "localhost:{port}")"#)), - ); - push_deny_rule( - policy, - "network-outbound", - r#"(remote ip "*:*")"#, - &outbound_exceptions, - ); - - let local_exceptions = if proxy.allow_local_binding { - vec![r#"(local ip "localhost:*")"#.to_string()] - } else { - Vec::new() - }; - push_deny_rule( - policy, - "network-bind", - r#"(local ip "*:*")"#, - &local_exceptions, - ); - push_deny_rule( - policy, - "network-inbound", - r#"(local ip "*:*")"#, - &local_exceptions, - ); -} - -fn push_unix_socket_network_policy(policy: &mut String, proxy: &ProxyPolicyInputs) { - let UnixDomainSocketPolicy::Restricted { .. } = proxy.unix_domain_socket_policy else { - return; - }; - let exceptions = unix_socket_path_params(proxy) - .into_iter() - .map(|param| { - let key = unix_socket_path_param_key(param.index); - format!(r#"(remote unix-socket (subpath (param "{key}")))"#) - }) - .collect::>(); - push_deny_rule( - policy, - "network-outbound", - "(remote unix-socket)", - &exceptions, - ); - - let exceptions = unix_socket_path_params(proxy) - .into_iter() - .map(|param| { - let key = unix_socket_path_param_key(param.index); - format!(r#"(local unix-socket (subpath (param "{key}")))"#) - }) - .collect::>(); - push_deny_rule(policy, "network-bind", "(local unix-socket)", &exceptions); -} - -fn push_deny_rule(policy: &mut String, operation: &str, selector: &str, exceptions: &[String]) { - if exceptions.is_empty() { - policy.push_str(&format!("(deny {operation} {selector})\n")); - return; - } - - policy.push_str(&format!("(deny {operation} (require-all {selector}")); - for exception in exceptions { - policy.push_str(&format!(" (require-not {exception})")); - } - policy.push_str("))\n"); -} - -fn unix_socket_dir_params(proxy: &ProxyPolicyInputs) -> Vec<(String, PathBuf)> { - unix_socket_path_params(proxy) - .into_iter() - .map(|param| { - ( - unix_socket_path_param_key(param.index), - param.path.into_path_buf(), - ) - }) - .collect() -} diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index 5de28cac9b..0daff82e15 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -2,12 +2,10 @@ use super::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use super::MACOS_SEATBELT_BASE_POLICY; use super::ProxyPolicyInputs; use super::UnixDomainSocketPolicy; -use super::create_network_only_seatbelt_command_args; use super::create_seatbelt_command_args; use super::create_seatbelt_command_args_for_policies; use super::dynamic_network_policy; use super::macos_dir_params; -use super::network_only_policy_for_network; use super::normalize_path_for_sandbox; use super::unix_socket_dir_params; use super::unix_socket_policy; @@ -309,121 +307,6 @@ fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() { ); } -#[test] -fn network_only_policy_denies_direct_ip_except_proxy_ports() { - let policy = network_only_policy_for_network( - NetworkSandboxPolicy::Enabled, - /*enforce_managed_network*/ true, - &ProxyPolicyInputs { - ports: vec![43128], - has_proxy_config: true, - allow_local_binding: false, - ..ProxyPolicyInputs::default() - }, - ); - - assert!( - policy.contains( - "(deny network-outbound (require-all (remote ip \"*:*\") (require-not (remote ip \"localhost:43128\"))))" - ), - "policy should deny direct outbound IP while preserving proxy access:\n{policy}" - ); - assert!( - policy.contains("(deny network-bind (local ip \"*:*\"))"), - "policy should deny local IP binding unless explicitly allowed:\n{policy}" - ); - assert!( - policy.contains("(deny network-inbound (local ip \"*:*\"))"), - "policy should deny inbound IP unless explicitly allowed:\n{policy}" - ); - assert!( - !policy.contains("(deny default)"), - "network-only policy should not use the closed Seatbelt base policy:\n{policy}" - ); -} - -#[test] -fn network_only_policy_allows_loopback_when_local_binding_is_enabled() { - let policy = network_only_policy_for_network( - NetworkSandboxPolicy::Enabled, - /*enforce_managed_network*/ true, - &ProxyPolicyInputs { - ports: vec![43128], - has_proxy_config: true, - allow_local_binding: true, - ..ProxyPolicyInputs::default() - }, - ); - - assert!( - policy.contains("(require-not (remote ip \"localhost:*\"))"), - "policy should preserve loopback outbound when local binding is allowed:\n{policy}" - ); - assert!( - policy.contains( - "(deny network-bind (require-all (local ip \"*:*\") (require-not (local ip \"localhost:*\"))))" - ), - "policy should only allow loopback binding when local binding is enabled:\n{policy}" - ); - assert!( - policy.contains( - "(deny network-inbound (require-all (local ip \"*:*\") (require-not (local ip \"localhost:*\"))))" - ), - "policy should only allow loopback inbound when local binding is enabled:\n{policy}" - ); -} - -#[test] -fn network_only_policy_respects_unix_socket_allowlist() { - let policy = network_only_policy_for_network( - NetworkSandboxPolicy::Enabled, - /*enforce_managed_network*/ true, - &ProxyPolicyInputs { - unix_domain_socket_policy: UnixDomainSocketPolicy::Restricted { - allowed: vec![absolute_path("/tmp/example.sock")], - }, - ..ProxyPolicyInputs::default() - }, - ); - - assert!( - policy.contains( - "(deny network-outbound (require-all (remote unix-socket) (require-not (remote unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))))" - ), - "policy should deny unix-socket outbound except allowlisted paths:\n{policy}" - ); - assert!( - policy.contains( - "(deny network-bind (require-all (local unix-socket) (require-not (local unix-socket (subpath (param \"UNIX_SOCKET_PATH_0\"))))))" - ), - "policy should deny unix-socket binding except allowlisted paths:\n{policy}" - ); -} - -#[test] -fn create_network_only_seatbelt_args_uses_open_base_policy() { - let args = create_network_only_seatbelt_command_args( - vec!["true".to_string()], - NetworkSandboxPolicy::Enabled, - /*enforce_managed_network*/ true, - /*network*/ None, - ); - let policy = seatbelt_policy_arg(&args); - - assert!( - policy.starts_with("(version 1)\n(allow default)"), - "network-only Seatbelt profile should start from an open default:\n{policy}" - ); - assert!( - policy.contains("(deny network-outbound (remote ip \"*:*\")"), - "network-only Seatbelt profile should still deny direct IP outbound:\n{policy}" - ); - assert!( - !policy.contains("(deny default)"), - "network-only Seatbelt profile should not inherit the closed base profile:\n{policy}" - ); -} - #[test] fn dynamic_network_policy_preserves_restricted_policy_when_proxy_config_without_ports() { let policy = dynamic_network_policy(