From 0e7b5f01028c1cc49c44b9bd9d7972a1c46e6a58 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:58 -0800 Subject: [PATCH 1/8] sandboxing: plumb split sandbox policies through runtime --- .../app-server/src/codex_message_processor.rs | 22 +- codex-rs/core/src/codex.rs | 18 + codex-rs/core/src/codex_tests.rs | 20 +- codex-rs/core/src/exec.rs | 91 +++- codex-rs/core/src/landlock.rs | 3 +- codex-rs/core/src/sandboxing/mod.rs | 426 ++++++++++++++++-- codex-rs/core/src/seatbelt.rs | 3 +- codex-rs/core/src/spawn.rs | 16 +- codex-rs/core/src/tasks/user_shell.rs | 4 + codex-rs/core/src/tools/js_repl/mod.rs | 5 +- codex-rs/core/src/tools/orchestrator.rs | 7 +- .../tools/runtimes/shell/unix_escalation.rs | 32 +- .../runtimes/shell/unix_escalation_tests.rs | 11 +- codex-rs/core/src/tools/sandboxing.rs | 6 + codex-rs/core/src/unified_exec/mod.rs | 4 + codex-rs/core/tests/suite/exec.rs | 14 +- codex-rs/core/tests/suite/user_shell_cmd.rs | 31 ++ .../linux-sandbox/tests/suite/landlock.rs | 6 + 18 files changed, 655 insertions(+), 64 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 7898e2ffbb..9ebe881ae2 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1534,9 +1534,19 @@ impl CodexMessageProcessor { }; let requested_policy = params.sandbox_policy.map(|policy| policy.to_core()); - let effective_policy = match requested_policy { + let ( + effective_policy, + effective_file_system_sandbox_policy, + effective_network_sandbox_policy, + ) = match requested_policy { Some(policy) => match self.config.permissions.sandbox_policy.can_set(&policy) { - Ok(()) => policy, + Ok(()) => { + let file_system_sandbox_policy = + codex_protocol::protocol::FileSystemSandboxPolicy::from(&policy); + let network_sandbox_policy = + codex_protocol::protocol::NetworkSandboxPolicy::from(&policy); + (policy, file_system_sandbox_policy, network_sandbox_policy) + } Err(err) => { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, @@ -1547,7 +1557,11 @@ impl CodexMessageProcessor { return; } }, - None => self.config.permissions.sandbox_policy.get().clone(), + None => ( + self.config.permissions.sandbox_policy.get().clone(), + self.config.permissions.file_system_sandbox_policy.clone(), + self.config.permissions.network_sandbox_policy, + ), }; let codex_linux_sandbox_exe = self.arg0_paths.codex_linux_sandbox_exe.clone(); @@ -1562,6 +1576,8 @@ impl CodexMessageProcessor { match codex_core::exec::process_exec_tool_call( exec_params, &effective_policy, + &effective_file_system_sandbox_policy, + effective_network_sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, use_linux_sandbox_bwrap, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index bd32dfe029..69d7729d5b 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -223,10 +223,12 @@ use crate::protocol::ErrorEvent; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecApprovalRequestEvent; +use crate::protocol::FileSystemSandboxPolicy; use crate::protocol::McpServerRefreshConfig; use crate::protocol::ModelRerouteEvent; use crate::protocol::ModelRerouteReason; use crate::protocol::NetworkApprovalContext; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::Op; use crate::protocol::PlanDeltaEvent; use crate::protocol::RateLimitSnapshot; @@ -488,6 +490,8 @@ impl Codex { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -683,6 +687,8 @@ pub(crate) struct TurnContext { pub(crate) personality: Option, pub(crate) approval_policy: Constrained, pub(crate) sandbox_policy: Constrained, + pub(crate) file_system_sandbox_policy: FileSystemSandboxPolicy, + pub(crate) network_sandbox_policy: NetworkSandboxPolicy, pub(crate) network: Option, pub(crate) windows_sandbox_level: WindowsSandboxLevel, pub(crate) shell_environment_policy: ShellEnvironmentPolicy, @@ -773,6 +779,8 @@ impl TurnContext { personality: self.personality, approval_policy: self.approval_policy.clone(), sandbox_policy: self.sandbox_policy.clone(), + file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), + network_sandbox_policy: self.network_sandbox_policy, network: self.network.clone(), windows_sandbox_level: self.windows_sandbox_level, shell_environment_policy: self.shell_environment_policy.clone(), @@ -878,6 +886,8 @@ pub(crate) struct SessionConfiguration { approval_policy: Constrained, /// How to sandbox commands executed in the system sandbox_policy: Constrained, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, windows_sandbox_level: WindowsSandboxLevel, /// Working directory that should be treated as the *root* of the @@ -944,6 +954,10 @@ impl SessionConfiguration { } if let Some(sandbox_policy) = updates.sandbox_policy.clone() { next_configuration.sandbox_policy.set(sandbox_policy)?; + next_configuration.file_system_sandbox_policy = + FileSystemSandboxPolicy::from(next_configuration.sandbox_policy.get()); + next_configuration.network_sandbox_policy = + NetworkSandboxPolicy::from(next_configuration.sandbox_policy.get()); } if let Some(windows_sandbox_level) = updates.windows_sandbox_level { next_configuration.windows_sandbox_level = windows_sandbox_level; @@ -1156,6 +1170,8 @@ impl Session { personality: session_configuration.personality, approval_policy: session_configuration.approval_policy.clone(), sandbox_policy: session_configuration.sandbox_policy.clone(), + file_system_sandbox_policy: session_configuration.file_system_sandbox_policy.clone(), + network_sandbox_policy: session_configuration.network_sandbox_policy, network, windows_sandbox_level: session_configuration.windows_sandbox_level, shell_environment_policy: per_turn_config.permissions.shell_environment_policy.clone(), @@ -4983,6 +4999,8 @@ async fn spawn_review_thread( personality: parent_turn_context.personality, approval_policy: parent_turn_context.approval_policy.clone(), sandbox_policy: parent_turn_context.sandbox_policy.clone(), + file_system_sandbox_policy: parent_turn_context.file_system_sandbox_policy.clone(), + network_sandbox_policy: parent_turn_context.network_sandbox_policy, network: parent_turn_context.network.clone(), windows_sandbox_level: parent_turn_context.windows_sandbox_level, shell_environment_policy: parent_turn_context.shell_environment_policy.clone(), diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index cb44d1cada..31d9c75520 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -1416,6 +1416,8 @@ async fn set_rate_limits_retains_previous_credits() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -1510,6 +1512,8 @@ async fn set_rate_limits_updates_plan_type_when_present() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -1862,6 +1866,8 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -1919,6 +1925,8 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -2009,6 +2017,8 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -2414,6 +2424,8 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( compact_prompt: config.compact_prompt.clone(), approval_policy: config.permissions.approval_policy.clone(), sandbox_policy: config.permissions.sandbox_policy.clone(), + file_system_sandbox_policy: config.permissions.file_system_sandbox_policy.clone(), + network_sandbox_policy: config.permissions.network_sandbox_policy, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), codex_home: config.codex_home.clone(), @@ -3841,11 +3853,15 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { // Now retry the same command WITHOUT escalated permissions; should succeed. // Force DangerFullAccess to avoid platform sandbox dependencies in tests. - Arc::get_mut(&mut turn_context) - .expect("unique turn context Arc") + let turn_context_mut = Arc::get_mut(&mut turn_context).expect("unique turn context Arc"); + turn_context_mut .sandbox_policy .set(SandboxPolicy::DangerFullAccess) .expect("test setup should allow updating sandbox policy"); + turn_context_mut.file_system_sandbox_policy = + FileSystemSandboxPolicy::from(turn_context_mut.sandbox_policy.get()); + turn_context_mut.network_sandbox_policy = + NetworkSandboxPolicy::from(turn_context_mut.sandbox_policy.get()); let resp2 = handler .handle(ToolInvocation { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 8779b2e1c3..cd4da20616 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -24,6 +24,9 @@ use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; use crate::protocol::ExecOutputStream; +use crate::protocol::FileSystemSandboxKind; +use crate::protocol::FileSystemSandboxPolicy; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::sandboxing::CommandSpec; use crate::sandboxing::ExecRequest; @@ -149,9 +152,12 @@ pub struct StdoutStream { pub tx_event: Sender, } +#[allow(clippy::too_many_arguments)] pub async fn process_exec_tool_call( params: ExecParams, sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, sandbox_cwd: &Path, codex_linux_sandbox_exe: &Option, use_linux_sandbox_bwrap: bool, @@ -159,8 +165,8 @@ pub async fn process_exec_tool_call( ) -> Result { let windows_sandbox_level = params.windows_sandbox_level; let enforce_managed_network = params.network.is_some(); - let sandbox_type = match &sandbox_policy { - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { + let sandbox_type = match file_system_sandbox_policy.kind { + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => { if enforce_managed_network { get_platform_sandbox( windows_sandbox_level @@ -215,6 +221,8 @@ pub async fn process_exec_tool_call( .transform(crate::sandboxing::SandboxTransformRequest { spec, policy: sandbox_policy, + file_system_policy: file_system_sandbox_policy, + network_policy: network_sandbox_policy, sandbox: sandbox_type, enforce_managed_network, network: network.as_ref(), @@ -247,9 +255,12 @@ pub(crate) async fn execute_exec_request( windows_sandbox_level, sandbox_permissions, sandbox_policy: _sandbox_policy_from_env, + file_system_sandbox_policy, + network_sandbox_policy, justification, arg0, } = exec_request; + let _ = _sandbox_policy_from_env; let params = ExecParams { command, @@ -264,7 +275,16 @@ pub(crate) async fn execute_exec_request( }; let start = Instant::now(); - let raw_output_result = exec(params, sandbox, sandbox_policy, stdout_stream, after_spawn).await; + let raw_output_result = exec( + params, + sandbox, + sandbox_policy, + &file_system_sandbox_policy, + network_sandbox_policy, + stdout_stream, + after_spawn, + ) + .await; let duration = start.elapsed(); finalize_exec_result(raw_output_result, sandbox, duration) } @@ -693,16 +713,17 @@ async fn exec( params: ExecParams, sandbox: SandboxType, sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, stdout_stream: Option, after_spawn: Option>, ) -> Result { #[cfg(target_os = "windows")] - if sandbox == SandboxType::WindowsRestrictedToken - && !matches!( - sandbox_policy, - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } - ) - { + if should_use_windows_restricted_token_sandbox( + sandbox, + sandbox_policy, + file_system_sandbox_policy, + ) { return exec_windows_sandbox(params, sandbox_policy).await; } let ExecParams { @@ -731,7 +752,7 @@ async fn exec( args: args.into(), arg0: arg0_ref, cwd, - sandbox_policy, + network_sandbox_policy, // The environment already has attempt-scoped proxy settings from // apply_to_env_for_attempt above. Passing network here would reapply // non-attempt proxy vars and drop attempt correlation metadata. @@ -746,6 +767,20 @@ async fn exec( consume_truncated_output(child, expiration, stdout_stream).await } +#[cfg_attr(not(target_os = "windows"), allow(dead_code))] +fn should_use_windows_restricted_token_sandbox( + sandbox: SandboxType, + sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> bool { + sandbox == SandboxType::WindowsRestrictedToken + && file_system_sandbox_policy.kind == FileSystemSandboxKind::Restricted + && !matches!( + sandbox_policy, + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + ) +} + /// Consumes the output of a child process, truncating it so it is suitable for /// use as the output of a `shell` tool call. Also enforces specified timeout. async fn consume_truncated_output( @@ -1098,6 +1133,38 @@ mod tests { assert_eq!(aggregated.truncated_after_lines, None); } + #[test] + fn windows_restricted_token_skips_external_sandbox_policies() { + let policy = SandboxPolicy::ExternalSandbox { + network_access: codex_protocol::protocol::NetworkAccess::Restricted, + }; + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![]); + + assert_eq!( + should_use_windows_restricted_token_sandbox( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + ), + false + ); + } + + #[test] + fn windows_restricted_token_runs_for_legacy_restricted_policies() { + let policy = SandboxPolicy::new_read_only_policy(); + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![]); + + assert_eq!( + should_use_windows_restricted_token_sandbox( + SandboxType::WindowsRestrictedToken, + &policy, + &file_system_policy, + ), + true + ); + } + #[cfg(unix)] #[test] fn sandbox_detection_flags_sigsys_exit_code() { @@ -1140,6 +1207,8 @@ mod tests { params, SandboxType::None, &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()), + NetworkSandboxPolicy::Restricted, None, None, ) @@ -1196,6 +1265,8 @@ mod tests { let result = process_exec_tool_call( params, &SandboxPolicy::DangerFullAccess, + &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + NetworkSandboxPolicy::Enabled, cwd.as_path(), &None, false, diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 65b2a68073..7e49191067 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -1,3 +1,4 @@ +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; @@ -44,7 +45,7 @@ where args, arg0, cwd: command_cwd, - sandbox_policy, + network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy), network, stdio_policy, env, diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 9258889c79..623c97d301 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -15,6 +15,13 @@ use crate::exec::StdoutStream; use crate::exec::execute_exec_request; use crate::landlock::allow_network_for_proxy; use crate::landlock::create_linux_sandbox_command_args; +use crate::protocol::FileSystemAccessMode; +use crate::protocol::FileSystemPath; +use crate::protocol::FileSystemSandboxEntry; +use crate::protocol::FileSystemSandboxKind; +use crate::protocol::FileSystemSandboxPolicy; +use crate::protocol::FileSystemSpecialPathKind; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; @@ -30,6 +37,7 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; +use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; @@ -62,6 +70,8 @@ pub struct ExecRequest { pub windows_sandbox_level: WindowsSandboxLevel, pub sandbox_permissions: SandboxPermissions, pub sandbox_policy: SandboxPolicy, + pub file_system_sandbox_policy: FileSystemSandboxPolicy, + pub network_sandbox_policy: NetworkSandboxPolicy, pub justification: Option, pub arg0: Option, } @@ -72,6 +82,8 @@ pub struct ExecRequest { pub(crate) struct SandboxTransformRequest<'a> { pub spec: CommandSpec, pub policy: &'a SandboxPolicy, + pub file_system_policy: &'a FileSystemSandboxPolicy, + pub network_policy: NetworkSandboxPolicy, pub sandbox: SandboxType, pub enforce_managed_network: bool, // TODO(viyatb): Evaluate switching this to Option> @@ -203,6 +215,41 @@ fn additional_permission_roots( ) } +#[cfg_attr(not(test), allow(dead_code))] +fn merge_file_system_policy_with_additional_permissions( + file_system_policy: &FileSystemSandboxPolicy, + extra_reads: Vec, + extra_writes: Vec, +) -> FileSystemSandboxPolicy { + match file_system_policy.kind { + FileSystemSandboxKind::Restricted => { + let mut merged_policy = file_system_policy.clone(); + for path in extra_reads { + let entry = FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Read, + }; + if !merged_policy.entries.contains(&entry) { + merged_policy.entries.push(entry); + } + } + for path in extra_writes { + let entry = FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + }; + if !merged_policy.entries.contains(&entry) { + merged_policy.entries.push(entry); + } + } + merged_policy + } + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => { + file_system_policy.clone() + } + } +} + fn merge_read_only_access_with_additional_reads( read_only_access: &ReadOnlyAccess, extra_reads: Vec, @@ -246,9 +293,17 @@ fn sandbox_policy_with_additional_permissions( let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions); match sandbox_policy { - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { - sandbox_policy.clone() - } + SandboxPolicy::DangerFullAccess => SandboxPolicy::DangerFullAccess, + SandboxPolicy::ExternalSandbox { network_access } => SandboxPolicy::ExternalSandbox { + network_access: if merge_network_access( + network_access.is_enabled(), + additional_permissions, + ) { + NetworkAccess::Enabled + } else { + NetworkAccess::Restricted + }, + }, SandboxPolicy::WorkspaceWrite { writable_roots, read_only_access, @@ -297,6 +352,36 @@ fn sandbox_policy_with_additional_permissions( } } +pub(crate) fn should_require_platform_sandbox( + file_system_policy: &FileSystemSandboxPolicy, + network_policy: NetworkSandboxPolicy, + has_managed_network_requirements: bool, +) -> bool { + if has_managed_network_requirements { + return true; + } + + if !network_policy.is_enabled() { + return !matches!( + file_system_policy.kind, + FileSystemSandboxKind::ExternalSandbox + ); + } + + match file_system_policy.kind { + FileSystemSandboxKind::Restricted => !file_system_policy.entries.iter().any(|entry| { + entry.access == FileSystemAccessMode::Write + && matches!( + &entry.path, + FileSystemPath::Special { value } + if value.kind == FileSystemSpecialPathKind::Root + && value.subpath.is_none() + ) + }), + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => false, + } +} + #[derive(Default)] pub struct SandboxManager; @@ -307,7 +392,8 @@ impl SandboxManager { pub(crate) fn select_initial( &self, - policy: &SandboxPolicy, + file_system_policy: &FileSystemSandboxPolicy, + network_policy: NetworkSandboxPolicy, pref: SandboxablePreference, windows_sandbox_level: WindowsSandboxLevel, has_managed_network_requirements: bool, @@ -322,22 +408,20 @@ impl SandboxManager { ) .unwrap_or(SandboxType::None) } - SandboxablePreference::Auto => match policy { - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { - if has_managed_network_requirements { - crate::safety::get_platform_sandbox( - windows_sandbox_level != WindowsSandboxLevel::Disabled, - ) - .unwrap_or(SandboxType::None) - } else { - SandboxType::None - } + SandboxablePreference::Auto => { + if should_require_platform_sandbox( + file_system_policy, + network_policy, + has_managed_network_requirements, + ) { + crate::safety::get_platform_sandbox( + windows_sandbox_level != WindowsSandboxLevel::Disabled, + ) + .unwrap_or(SandboxType::None) + } else { + SandboxType::None } - _ => crate::safety::get_platform_sandbox( - windows_sandbox_level != WindowsSandboxLevel::Disabled, - ) - .unwrap_or(SandboxType::None), - }, + } } } @@ -348,6 +432,8 @@ impl SandboxManager { let SandboxTransformRequest { mut spec, policy, + file_system_policy, + network_policy, sandbox, enforce_managed_network, network, @@ -360,16 +446,38 @@ impl SandboxManager { } = request; #[cfg(not(target_os = "macos"))] let macos_seatbelt_profile_extensions = None; - let effective_permissions = EffectiveSandboxPermissions::new( + let additional_permissions = spec.additional_permissions.take(); + let EffectiveSandboxPermissions { + sandbox_policy: effective_policy, + macos_seatbelt_profile_extensions: effective_macos_seatbelt_profile_extensions, + } = EffectiveSandboxPermissions::new( policy, macos_seatbelt_profile_extensions, - spec.additional_permissions.as_ref(), + additional_permissions.as_ref(), ); + let (effective_file_system_policy, effective_network_policy) = + if let Some(additional_permissions) = additional_permissions { + let (extra_reads, extra_writes) = + additional_permission_roots(&additional_permissions); + let file_system_sandbox_policy = + if extra_reads.is_empty() && extra_writes.is_empty() { + file_system_policy.clone() + } else { + match file_system_policy.kind { + FileSystemSandboxKind::Restricted => { + FileSystemSandboxPolicy::from(&effective_policy) + } + FileSystemSandboxKind::Unrestricted + | FileSystemSandboxKind::ExternalSandbox => file_system_policy.clone(), + } + }; + let network_sandbox_policy = NetworkSandboxPolicy::from(&effective_policy); + (file_system_sandbox_policy, network_sandbox_policy) + } else { + (file_system_policy.clone(), network_policy) + }; let mut env = spec.env; - if !effective_permissions - .sandbox_policy - .has_full_network_access() - { + if !effective_network_policy.is_enabled() { env.insert( CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(), "1".to_string(), @@ -388,13 +496,11 @@ impl SandboxManager { seatbelt_env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); let mut args = create_seatbelt_command_args_with_extensions( command.clone(), - &effective_permissions.sandbox_policy, + &effective_policy, sandbox_policy_cwd, enforce_managed_network, network, - effective_permissions - .macos_seatbelt_profile_extensions - .as_ref(), + effective_macos_seatbelt_profile_extensions.as_ref(), ); let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); @@ -409,7 +515,7 @@ impl SandboxManager { let allow_proxy_network = allow_network_for_proxy(enforce_managed_network); let mut args = create_linux_sandbox_command_args( command.clone(), - &effective_permissions.sandbox_policy, + &effective_policy, sandbox_policy_cwd, use_linux_sandbox_bwrap, allow_proxy_network, @@ -444,7 +550,9 @@ impl SandboxManager { sandbox, windows_sandbox_level, sandbox_permissions: spec.sandbox_permissions, - sandbox_policy: effective_permissions.sandbox_policy, + sandbox_policy: effective_policy, + file_system_sandbox_policy: effective_file_system_policy, + network_sandbox_policy: effective_network_policy, justification: spec.justification, arg0: arg0_override, }) @@ -477,9 +585,19 @@ mod tests { #[cfg(target_os = "macos")] use super::EffectiveSandboxPermissions; use super::SandboxManager; + use super::merge_file_system_policy_with_additional_permissions; use super::normalize_additional_permissions; use super::sandbox_policy_with_additional_permissions; + use super::should_require_platform_sandbox; use crate::exec::SandboxType; + use crate::protocol::FileSystemAccessMode; + use crate::protocol::FileSystemPath; + use crate::protocol::FileSystemSandboxEntry; + use crate::protocol::FileSystemSandboxPolicy; + use crate::protocol::FileSystemSpecialPath; + use crate::protocol::FileSystemSpecialPathKind; + use crate::protocol::NetworkAccess; + use crate::protocol::NetworkSandboxPolicy; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::tools::sandboxing::SandboxablePreference; @@ -496,13 +614,15 @@ mod tests { use codex_utils_absolute_path::AbsolutePathBuf; use dunce::canonicalize; use pretty_assertions::assert_eq; + use std::collections::HashMap; use tempfile::TempDir; #[test] fn danger_full_access_defaults_to_no_sandbox_without_network_requirements() { let manager = SandboxManager::new(); let sandbox = manager.select_initial( - &SandboxPolicy::DangerFullAccess, + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Enabled, SandboxablePreference::Auto, WindowsSandboxLevel::Disabled, false, @@ -515,7 +635,8 @@ mod tests { let manager = SandboxManager::new(); let expected = crate::safety::get_platform_sandbox(false).unwrap_or(SandboxType::None); let sandbox = manager.select_initial( - &SandboxPolicy::DangerFullAccess, + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Enabled, SandboxablePreference::Auto, WindowsSandboxLevel::Disabled, true, @@ -523,6 +644,107 @@ mod tests { assert_eq!(sandbox, expected); } + #[test] + fn restricted_file_system_uses_platform_sandbox_without_managed_network() { + let manager = SandboxManager::new(); + let expected = crate::safety::get_platform_sandbox(false).unwrap_or(SandboxType::None); + let sandbox = manager.select_initial( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Read, + }]), + NetworkSandboxPolicy::Enabled, + SandboxablePreference::Auto, + WindowsSandboxLevel::Disabled, + false, + ); + assert_eq!(sandbox, expected); + } + + #[test] + fn full_access_restricted_policy_skips_platform_sandbox_when_network_is_enabled() { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }]); + + assert_eq!( + should_require_platform_sandbox(&policy, NetworkSandboxPolicy::Enabled, false), + false + ); + } + + #[test] + fn full_access_restricted_policy_still_uses_platform_sandbox_for_restricted_network() { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }]); + + assert_eq!( + should_require_platform_sandbox(&policy, NetworkSandboxPolicy::Restricted, false), + true + ); + } + + #[test] + fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() { + let manager = SandboxManager::new(); + let cwd = std::env::current_dir().expect("current dir"); + let exec_request = manager + .transform(super::SandboxTransformRequest { + spec: super::CommandSpec { + program: "true".to_string(), + args: Vec::new(), + cwd: cwd.clone(), + env: HashMap::new(), + expiration: crate::exec::ExecExpiration::DefaultTimeout, + sandbox_permissions: super::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + }, + policy: &SandboxPolicy::ExternalSandbox { + network_access: crate::protocol::NetworkAccess::Restricted, + }, + file_system_policy: &FileSystemSandboxPolicy::unrestricted(), + network_policy: NetworkSandboxPolicy::Restricted, + sandbox: SandboxType::None, + enforce_managed_network: false, + network: None, + sandbox_policy_cwd: cwd.as_path(), + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: None, + codex_linux_sandbox_exe: None, + use_linux_sandbox_bwrap: false, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }) + .expect("transform"); + + assert_eq!( + exec_request.file_system_sandbox_policy, + FileSystemSandboxPolicy::unrestricted() + ); + assert_eq!( + exec_request.network_sandbox_policy, + NetworkSandboxPolicy::Restricted + ); + } + #[test] fn normalize_additional_permissions_preserves_network() { let temp_dir = TempDir::new().expect("create temp dir"); @@ -624,7 +846,6 @@ mod tests { } ); } - #[cfg(target_os = "macos")] #[test] fn effective_permissions_merge_macos_extensions_with_additional_permissions() { @@ -679,4 +900,141 @@ mod tests { }) ); } + + #[test] + fn external_sandbox_additional_permissions_can_enable_network() { + let temp_dir = TempDir::new().expect("create temp dir"); + let path = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let policy = sandbox_policy_with_additional_permissions( + &SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Restricted, + }, + &PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: Some(FileSystemPermissions { + read: Some(vec![path]), + write: Some(Vec::new()), + }), + ..Default::default() + }, + ); + + assert_eq!( + policy, + SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Enabled, + } + ); + } + + #[test] + fn transform_additional_permissions_enable_network_for_external_sandbox() { + let manager = SandboxManager::new(); + let cwd = std::env::current_dir().expect("current dir"); + let temp_dir = TempDir::new().expect("create temp dir"); + let path = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let exec_request = manager + .transform(super::SandboxTransformRequest { + spec: super::CommandSpec { + program: "true".to_string(), + args: Vec::new(), + cwd: cwd.clone(), + env: HashMap::new(), + expiration: crate::exec::ExecExpiration::DefaultTimeout, + sandbox_permissions: super::SandboxPermissions::WithAdditionalPermissions, + additional_permissions: Some(PermissionProfile { + network: Some(NetworkPermissions { + enabled: Some(true), + }), + file_system: Some(FileSystemPermissions { + read: Some(vec![path]), + write: Some(Vec::new()), + }), + ..Default::default() + }), + justification: None, + }, + policy: &SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Restricted, + }, + file_system_policy: &FileSystemSandboxPolicy::unrestricted(), + network_policy: NetworkSandboxPolicy::Restricted, + sandbox: SandboxType::None, + enforce_managed_network: false, + network: None, + sandbox_policy_cwd: cwd.as_path(), + #[cfg(target_os = "macos")] + macos_seatbelt_profile_extensions: None, + codex_linux_sandbox_exe: None, + use_linux_sandbox_bwrap: false, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }) + .expect("transform"); + + assert_eq!( + exec_request.sandbox_policy, + SandboxPolicy::ExternalSandbox { + network_access: NetworkAccess::Enabled, + } + ); + assert_eq!( + exec_request.network_sandbox_policy, + NetworkSandboxPolicy::Enabled + ); + } + + #[test] + fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roots() { + let temp_dir = TempDir::new().expect("create temp dir"); + let cwd = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let allowed_path = cwd.join("allowed").expect("allowed path"); + let denied_path = cwd.join("denied").expect("denied path"); + let merged_policy = merge_file_system_policy_with_additional_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: denied_path.clone(), + }, + access: FileSystemAccessMode::None, + }, + ]), + vec![allowed_path.clone()], + Vec::new(), + ); + + assert_eq!( + merged_policy.entries.contains(&FileSystemSandboxEntry { + path: FileSystemPath::Path { path: denied_path }, + access: FileSystemAccessMode::None, + }), + true + ); + assert_eq!( + merged_policy.entries.contains(&FileSystemSandboxEntry { + path: FileSystemPath::Path { path: allowed_path }, + access: FileSystemAccessMode::Read, + }), + true + ); + } } diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 8d556a8eec..fc6d4624c9 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -15,6 +15,7 @@ use tokio::process::Child; use tracing::warn; use url::Url; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; use crate::seatbelt_permissions::build_seatbelt_extensions; @@ -51,7 +52,7 @@ pub async fn spawn_command_under_seatbelt( args, arg0, cwd: command_cwd, - sandbox_policy, + network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy), network, stdio_policy, env, diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index 67e6ace044..480d1ea5ee 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -6,13 +6,13 @@ use tokio::process::Child; use tokio::process::Command; use tracing::trace; -use crate::protocol::SandboxPolicy; +use crate::protocol::NetworkSandboxPolicy; /// Experimental environment variable that will be set to some non-empty value /// if both of the following are true: /// /// 1. The process was spawned by Codex as part of a shell tool call. -/// 2. SandboxPolicy.has_full_network_access() was false for the tool call. +/// 2. NetworkSandboxPolicy is restricted for the tool call. /// /// We may try to have just one environment variable for all sandboxing /// attributes, so this may change in the future. @@ -33,15 +33,15 @@ pub enum StdioPolicy { /// ensuring the args and environment variables used to create the `Command` /// (and `Child`) honor the configuration. /// -/// For now, we take `SandboxPolicy` as a parameter to spawn_child() because -/// we need to determine whether to set the +/// For now, we take `NetworkSandboxPolicy` as a parameter to spawn_child() +/// because we need to determine whether to set the /// `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` environment variable. pub(crate) struct SpawnChildRequest<'a> { pub program: PathBuf, pub args: Vec, pub arg0: Option<&'a str>, pub cwd: PathBuf, - pub sandbox_policy: &'a SandboxPolicy, + pub network_sandbox_policy: NetworkSandboxPolicy, pub network: Option<&'a NetworkProxy>, pub stdio_policy: StdioPolicy, pub env: HashMap, @@ -53,14 +53,14 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io args, arg0, cwd, - sandbox_policy, + network_sandbox_policy, network, stdio_policy, mut env, } = request; trace!( - "spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {sandbox_policy:?} {stdio_policy:?} {env:?}" + "spawn_child_async: {program:?} {args:?} {arg0:?} {cwd:?} {network_sandbox_policy:?} {stdio_policy:?} {env:?}" ); let mut cmd = Command::new(&program); @@ -74,7 +74,7 @@ pub(crate) async fn spawn_child_async(request: SpawnChildRequest<'_>) -> std::io cmd.env_clear(); cmd.envs(env); - if !sandbox_policy.has_full_network_access() { + if !network_sandbox_policy.is_enabled() { cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 2f77d9fcee..c7fc18f9a2 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -22,6 +22,8 @@ use crate::protocol::ExecCommandBeginEvent; use crate::protocol::ExecCommandEndEvent; use crate::protocol::ExecCommandSource; use crate::protocol::ExecCommandStatus; +use crate::protocol::FileSystemSandboxPolicy; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::protocol::TurnStartedEvent; use crate::sandboxing::ExecRequest; @@ -167,6 +169,8 @@ pub(crate) async fn execute_user_shell_command( windows_sandbox_level: turn_context.windows_sandbox_level, sandbox_permissions: SandboxPermissions::UseDefault, sandbox_policy: sandbox_policy.clone(), + file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), justification: None, arg0: None, }; diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 42016ff9bf..f9cc3d854b 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -852,7 +852,8 @@ impl JsReplManager { .network .is_some(); let sandbox_type = sandbox.select_initial( - &turn.sandbox_policy, + &turn.file_system_sandbox_policy, + turn.network_sandbox_policy, SandboxablePreference::Auto, turn.windows_sandbox_level, has_managed_network_requirements, @@ -861,6 +862,8 @@ impl JsReplManager { .transform(crate::sandboxing::SandboxTransformRequest { spec, policy: &turn.sandbox_policy, + file_system_policy: &turn.file_system_sandbox_policy, + network_policy: turn.network_sandbox_policy, sandbox: sandbox_type, enforce_managed_network: has_managed_network_requirements, network: None, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index f66c79bbb0..7bed13ce4f 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -169,7 +169,8 @@ impl ToolOrchestrator { let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) { SandboxOverride::BypassSandboxFirstAttempt => crate::exec::SandboxType::None, SandboxOverride::NoOverride => self.sandbox.select_initial( - &turn_ctx.sandbox_policy, + &turn_ctx.file_system_sandbox_policy, + turn_ctx.network_sandbox_policy, tool.sandbox_preference(), turn_ctx.windows_sandbox_level, has_managed_network_requirements, @@ -182,6 +183,8 @@ impl ToolOrchestrator { let initial_attempt = SandboxAttempt { sandbox: initial_sandbox, policy: &turn_ctx.sandbox_policy, + file_system_policy: &turn_ctx.file_system_sandbox_policy, + network_policy: turn_ctx.network_sandbox_policy, enforce_managed_network: has_managed_network_requirements, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, @@ -296,6 +299,8 @@ impl ToolOrchestrator { let escalated_attempt = SandboxAttempt { sandbox: crate::exec::SandboxType::None, policy: &turn_ctx.sandbox_policy, + file_system_policy: &turn_ctx.file_system_sandbox_policy, + network_policy: turn_ctx.network_sandbox_policy, enforce_managed_network: has_managed_network_requirements, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, 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 e4f7c80ab7..8942c802b3 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -25,7 +25,9 @@ use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkPolicyRuleAction; +use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::RejectConfig; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; @@ -98,6 +100,8 @@ pub(super) async fn try_run_zsh_fork( windows_sandbox_level, sandbox_permissions, sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, justification, arg0, } = sandbox_exec_request; @@ -113,6 +117,8 @@ pub(super) async fn try_run_zsh_fork( command, cwd: sandbox_cwd, sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, sandbox, env: sandbox_env, network: sandbox_network, @@ -220,6 +226,8 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( command: exec_request.command.clone(), cwd: exec_request.cwd.clone(), sandbox_policy: exec_request.sandbox_policy.clone(), + file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(), + network_sandbox_policy: exec_request.network_sandbox_policy, sandbox: exec_request.sandbox, env: exec_request.env.clone(), network: exec_request.network.clone(), @@ -728,6 +736,8 @@ struct CoreShellCommandExecutor { command: Vec, cwd: PathBuf, sandbox_policy: SandboxPolicy, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, sandbox: SandboxType, env: HashMap, network: Option, @@ -747,6 +757,8 @@ struct PrepareSandboxedExecParams<'a> { workdir: &'a AbsolutePathBuf, env: HashMap, sandbox_policy: &'a SandboxPolicy, + file_system_sandbox_policy: &'a FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, additional_permissions: Option, #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: Option<&'a MacOsSeatbeltProfileExtensions>, @@ -782,6 +794,8 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { windows_sandbox_level: self.windows_sandbox_level, sandbox_permissions: self.sandbox_permissions, sandbox_policy: self.sandbox_policy.clone(), + file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), + network_sandbox_policy: self.network_sandbox_policy, justification: self.justification.clone(), arg0: self.arg0.clone(), }, @@ -828,6 +842,8 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { workdir, env, sandbox_policy: &self.sandbox_policy, + file_system_sandbox_policy: &self.file_system_sandbox_policy, + network_sandbox_policy: self.network_sandbox_policy, additional_permissions: None, #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: self @@ -845,6 +861,8 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { workdir, env, sandbox_policy: &self.sandbox_policy, + file_system_sandbox_policy: &self.file_system_sandbox_policy, + network_sandbox_policy: self.network_sandbox_policy, additional_permissions: Some(permission_profile), #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: self @@ -854,11 +872,17 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { } EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => { // Use a fully specified sandbox policy instead of merging into the turn policy. + let file_system_sandbox_policy = + FileSystemSandboxPolicy::from(&permissions.sandbox_policy); + let network_sandbox_policy = + NetworkSandboxPolicy::from(&permissions.sandbox_policy); self.prepare_sandboxed_exec(PrepareSandboxedExecParams { command, workdir, env, sandbox_policy: &permissions.sandbox_policy, + file_system_sandbox_policy: &file_system_sandbox_policy, + network_sandbox_policy, additional_permissions: None, #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: permissions @@ -873,6 +897,7 @@ impl ShellCommandExecutor for CoreShellCommandExecutor { } impl CoreShellCommandExecutor { + #[allow(clippy::too_many_arguments)] fn prepare_sandboxed_exec( &self, params: PrepareSandboxedExecParams<'_>, @@ -882,6 +907,8 @@ impl CoreShellCommandExecutor { workdir, env, sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, additional_permissions, #[cfg(target_os = "macos")] macos_seatbelt_profile_extensions, @@ -891,7 +918,8 @@ impl CoreShellCommandExecutor { .ok_or_else(|| anyhow::anyhow!("prepared command must not be empty"))?; let sandbox_manager = crate::sandboxing::SandboxManager::new(); let sandbox = sandbox_manager.select_initial( - sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, SandboxablePreference::Auto, self.windows_sandbox_level, self.network.is_some(), @@ -913,6 +941,8 @@ impl CoreShellCommandExecutor { justification: self.justification.clone(), }, policy: sandbox_policy, + file_system_policy: file_system_sandbox_policy, + network_policy: network_sandbox_policy, sandbox, enforce_managed_network: self.network.is_some(), network: self.network.as_ref(), 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 e1c9380d49..5069cce929 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 @@ -478,6 +478,10 @@ async fn prepare_escalated_exec_turn_default_preserves_macos_seatbelt_extensions network: None, sandbox: SandboxType::None, sandbox_policy: SandboxPolicy::new_read_only_policy(), + file_system_sandbox_policy: FileSystemSandboxPolicy::from( + &SandboxPolicy::new_read_only_policy(), + ), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, justification: None, @@ -528,6 +532,8 @@ async fn prepare_escalated_exec_permissions_preserve_macos_seatbelt_extensions() network: None, sandbox: SandboxType::None, sandbox_policy: SandboxPolicy::DangerFullAccess, + file_system_sandbox_policy: FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + network_sandbox_policy: NetworkSandboxPolicy::Enabled, windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, justification: None, @@ -592,13 +598,16 @@ async fn prepare_escalated_exec_permissions_preserve_macos_seatbelt_extensions() #[tokio::test] async fn prepare_escalated_exec_permission_profile_unions_turn_and_requested_macos_extensions() { let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir()).unwrap(); + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let executor = CoreShellCommandExecutor { command: vec!["echo".to_string(), "ok".to_string()], cwd: cwd.to_path_buf(), env: HashMap::new(), network: None, sandbox: SandboxType::None, - sandbox_policy: SandboxPolicy::new_read_only_policy(), + sandbox_policy: sandbox_policy.clone(), + file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), windows_sandbox_level: WindowsSandboxLevel::Disabled, sandbox_permissions: SandboxPermissions::UseDefault, justification: None, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 28d87b5bf3..5ef0297956 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -7,6 +7,8 @@ use crate::codex::Session; use crate::codex::TurnContext; use crate::error::CodexErr; +use crate::protocol::FileSystemSandboxPolicy; +use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxManager; @@ -318,6 +320,8 @@ pub(crate) trait ToolRuntime: Approvable + Sandboxable { pub(crate) struct SandboxAttempt<'a> { pub sandbox: crate::exec::SandboxType, pub policy: &'a crate::protocol::SandboxPolicy, + pub file_system_policy: &'a FileSystemSandboxPolicy, + pub network_policy: NetworkSandboxPolicy, pub enforce_managed_network: bool, pub(crate) manager: &'a SandboxManager, pub(crate) sandbox_cwd: &'a Path, @@ -336,6 +340,8 @@ impl<'a> SandboxAttempt<'a> { .transform(crate::sandboxing::SandboxTransformRequest { spec, policy: self.policy, + file_system_policy: self.file_system_policy, + network_policy: self.network_policy, sandbox: self.sandbox, enforce_managed_network: self.enforce_managed_network, network, diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 6323857150..bd29840937 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -205,6 +205,10 @@ mod tests { turn.sandbox_policy .set(SandboxPolicy::DangerFullAccess) .expect("test setup should allow updating sandbox policy"); + turn.file_system_sandbox_policy = + crate::protocol::FileSystemSandboxPolicy::from(turn.sandbox_policy.get()); + turn.network_sandbox_policy = + crate::protocol::NetworkSandboxPolicy::from(turn.sandbox_policy.get()); (Arc::new(session), Arc::new(turn)) } diff --git a/codex-rs/core/tests/suite/exec.rs b/codex-rs/core/tests/suite/exec.rs index 6546600681..d809bd0ed3 100644 --- a/codex-rs/core/tests/suite/exec.rs +++ b/codex-rs/core/tests/suite/exec.rs @@ -10,6 +10,8 @@ use codex_core::exec::process_exec_tool_call; use codex_core::sandboxing::SandboxPermissions; use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use tempfile::TempDir; @@ -45,7 +47,17 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result anyh Ok(()) } +#[tokio::test] +async fn user_shell_command_does_not_set_network_sandbox_env_var() -> anyhow::Result<()> { + let server = responses::start_mock_server().await; + let mut builder = core_test_support::test_codex::test_codex().with_config(|config| { + config.permissions.network_sandbox_policy = NetworkSandboxPolicy::Restricted; + }); + let test = builder.build(&server).await?; + + #[cfg(windows)] + let command = r#"$val = $env:CODEX_SANDBOX_NETWORK_DISABLED; if ([string]::IsNullOrEmpty($val)) { $val = 'not-set' } ; [System.Console]::Write($val)"#.to_string(); + #[cfg(not(windows))] + let command = + r#"sh -c "printf '%s' \"${CODEX_SANDBOX_NETWORK_DISABLED:-not-set}\"""#.to_string(); + + test.codex + .submit(Op::RunUserShellCommand { command }) + .await?; + + let end_event = wait_for_event_match(&test.codex, |ev| match ev { + EventMsg::ExecCommandEnd(event) => Some(event.clone()), + _ => None, + }) + .await; + assert_eq!(end_event.exit_code, 0); + assert_eq!(end_event.stdout.trim(), "not-set"); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[cfg(not(target_os = "windows"))] // TODO: unignore on windows async fn user_shell_command_output_is_truncated_in_history() -> anyhow::Result<()> { diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 362fcaf35f..b12170584c 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -9,6 +9,8 @@ use codex_core::exec::process_exec_tool_call; use codex_core::exec_env::create_env; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -102,6 +104,8 @@ async fn run_cmd_result_with_writable_roots( process_exec_tool_call( params, &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::from(&sandbox_policy), sandbox_cwd.as_path(), &codex_linux_sandbox_exe, use_bwrap_sandbox, @@ -333,6 +337,8 @@ async fn assert_network_blocked(cmd: &[&str]) { let result = process_exec_tool_call( params, &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::from(&sandbox_policy), sandbox_cwd.as_path(), &codex_linux_sandbox_exe, false, From ebe6f772ead4cc179e1781fb1987bfb9eaf45bb0 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 2/8] protocol: derive effective file access from filesystem policies --- codex-rs/protocol/src/protocol.rs | 304 ++++++++++++++++++++++++++---- 1 file changed, 265 insertions(+), 39 deletions(-) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 3c8e956363..2ccd8d82f1 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -672,6 +672,113 @@ impl FileSystemSandboxPolicy { } } + /// Returns true when filesystem reads are unrestricted. + pub fn has_full_disk_read_access(&self) -> bool { + match self.kind { + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true, + FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| { + matches!( + &entry.path, + FileSystemPath::Special { value } + if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_read() + ) + }), + } + } + + /// Returns true when filesystem writes are unrestricted. + pub fn has_full_disk_write_access(&self) -> bool { + match self.kind { + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true, + FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| { + matches!( + &entry.path, + FileSystemPath::Special { value } + if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_write() + ) + }), + } + } + + /// Returns true when platform-default readable roots should be included. + pub fn include_platform_defaults(&self) -> bool { + !self.has_full_disk_read_access() + && matches!(self.kind, FileSystemSandboxKind::Restricted) + && self.entries.iter().any(|entry| { + matches!( + &entry.path, + FileSystemPath::Special { value } + if value.kind == FileSystemSpecialPathKind::Minimal + && entry.access.can_read() + ) + }) + } + + /// Returns the explicit readable roots resolved against the provided cwd. + pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec { + if self.has_full_disk_read_access() { + return Vec::new(); + } + + let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); + dedup_absolute_paths( + self.entries + .iter() + .filter(|entry| entry.access.can_read()) + .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref())) + .collect(), + ) + } + + /// Returns the writable roots together with read-only carveouts resolved + /// against the provided cwd. + pub fn get_writable_roots_with_cwd(&self, cwd: &Path) -> Vec { + if self.has_full_disk_write_access() { + return Vec::new(); + } + + let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); + let unreadable_roots = self.get_unreadable_roots_with_cwd(cwd); + dedup_absolute_paths( + self.entries + .iter() + .filter(|entry| entry.access.can_write()) + .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref())) + .collect(), + ) + .into_iter() + .map(|root| { + let mut read_only_subpaths = default_read_only_subpaths_for_writable_root(&root); + read_only_subpaths.extend( + unreadable_roots + .iter() + .filter(|path| path.as_path().starts_with(root.as_path())) + .cloned(), + ); + WritableRoot { + root, + read_only_subpaths: dedup_absolute_paths(read_only_subpaths), + } + }) + .collect() + } + + /// Returns explicit unreadable roots resolved against the provided cwd. + pub fn get_unreadable_roots_with_cwd(&self, cwd: &Path) -> Vec { + if !matches!(self.kind, FileSystemSandboxKind::Restricted) { + return Vec::new(); + } + + let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); + dedup_absolute_paths( + self.entries + .iter() + .filter(|entry| entry.access == FileSystemAccessMode::None) + .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref())) + .collect(), + ) + } + pub fn to_legacy_sandbox_policy( &self, network_policy: NetworkSandboxPolicy, @@ -1157,45 +1264,11 @@ impl SandboxPolicy { // For each root, compute subpaths that should remain read-only. roots .into_iter() - .map(|writable_root| { - let mut subpaths: Vec = Vec::new(); - #[allow(clippy::expect_used)] - let top_level_git = writable_root - .join(".git") - .expect(".git is a valid relative path"); - // This applies to typical repos (directory .git), worktrees/submodules - // (file .git with gitdir pointer), and bare repos when the gitdir is the - // writable root itself. - let top_level_git_is_file = top_level_git.as_path().is_file(); - let top_level_git_is_dir = top_level_git.as_path().is_dir(); - if top_level_git_is_dir || top_level_git_is_file { - if top_level_git_is_file - && is_git_pointer_file(&top_level_git) - && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git) - && !subpaths - .iter() - .any(|subpath| subpath.as_path() == gitdir.as_path()) - { - subpaths.push(gitdir); - } - subpaths.push(top_level_git); - } - - // Make .agents/skills and .codex/config.toml and - // related files read-only to the agent, by default. - for subdir in &[".agents", ".codex"] { - #[allow(clippy::expect_used)] - let top_level_codex = - writable_root.join(subdir).expect("valid relative path"); - if top_level_codex.as_path().is_dir() { - subpaths.push(top_level_codex); - } - } - - WritableRoot { - root: writable_root, - read_only_subpaths: subpaths, - } + .map(|writable_root| WritableRoot { + read_only_subpaths: default_read_only_subpaths_for_writable_root( + &writable_root, + ), + root: writable_root, }) .collect() } @@ -1352,6 +1425,16 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { } } +fn resolve_file_system_path( + path: &FileSystemPath, + cwd: Option<&AbsolutePathBuf>, +) -> Option { + match path { + FileSystemPath::Path { path } => Some(path.clone()), + FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd), + } +} + fn resolve_file_system_special_path( value: &FileSystemSpecialPath, cwd: Option<&AbsolutePathBuf>, @@ -1409,6 +1492,42 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { deduped } +fn default_read_only_subpaths_for_writable_root( + writable_root: &AbsolutePathBuf, +) -> Vec { + let mut subpaths: Vec = Vec::new(); + #[allow(clippy::expect_used)] + let top_level_git = writable_root + .join(".git") + .expect(".git is a valid relative path"); + // This applies to typical repos (directory .git), worktrees/submodules + // (file .git with gitdir pointer), and bare repos when the gitdir is the + // writable root itself. + let top_level_git_is_file = top_level_git.as_path().is_file(); + let top_level_git_is_dir = top_level_git.as_path().is_dir(); + if top_level_git_is_dir || top_level_git_is_file { + if top_level_git_is_file + && is_git_pointer_file(&top_level_git) + && let Some(gitdir) = resolve_gitdir_from_file(&top_level_git) + { + subpaths.push(gitdir); + } + subpaths.push(top_level_git); + } + + // Make .agents/skills and .codex/config.toml and related files read-only + // to the agent, by default. + for subdir in &[".agents", ".codex"] { + #[allow(clippy::expect_used)] + let top_level_codex = writable_root.join(subdir).expect("valid relative path"); + if top_level_codex.as_path().is_dir() { + subpaths.push(top_level_codex); + } + } + + dedup_absolute_paths(subpaths) +} + fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git")) } @@ -3636,6 +3755,7 @@ mod tests { use pretty_assertions::assert_eq; use serde_json::json; use tempfile::NamedTempFile; + use tempfile::TempDir; #[test] fn external_sandbox_reports_full_access_flags() { @@ -3716,6 +3836,112 @@ mod tests { } } + #[test] + fn restricted_file_system_policy_reports_full_access_from_root_entries() { + let read_only = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Read, + }]); + assert!(read_only.has_full_disk_read_access()); + assert!(!read_only.has_full_disk_write_access()); + assert!(!read_only.include_platform_defaults()); + + let writable = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }]); + assert!(writable.has_full_disk_read_access()); + assert!(writable.has_full_disk_write_access()); + } + + #[test] + fn restricted_file_system_policy_derives_effective_paths() { + let cwd = TempDir::new().expect("tempdir"); + std::fs::create_dir_all(cwd.path().join(".agents")).expect("create .agents"); + std::fs::create_dir_all(cwd.path().join(".codex")).expect("create .codex"); + let cwd_absolute = + AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir"); + let secret = AbsolutePathBuf::resolve_path_against_base("secret", cwd.path()) + .expect("resolve unreadable path"); + let agents = AbsolutePathBuf::resolve_path_against_base(".agents", cwd.path()) + .expect("resolve .agents"); + let codex = AbsolutePathBuf::resolve_path_against_base(".codex", cwd.path()) + .expect("resolve .codex"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Minimal, + subpath: None, + }, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::CurrentWorkingDirectory, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::CurrentWorkingDirectory, + subpath: Some(PathBuf::from("secret")), + }, + }, + access: FileSystemAccessMode::None, + }, + ]); + + assert!(!policy.has_full_disk_read_access()); + assert!(!policy.has_full_disk_write_access()); + assert!(policy.include_platform_defaults()); + assert_eq!( + policy.get_readable_roots_with_cwd(cwd.path()), + vec![cwd_absolute] + ); + assert_eq!( + policy.get_unreadable_roots_with_cwd(cwd.path()), + vec![secret.clone()] + ); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root.as_path(), cwd.path()); + assert!( + writable_roots[0] + .read_only_subpaths + .iter() + .any(|path| path.as_path() == secret.as_path()) + ); + assert!( + writable_roots[0] + .read_only_subpaths + .iter() + .any(|path| path.as_path() == agents.as_path()) + ); + assert!( + writable_roots[0] + .read_only_subpaths + .iter() + .any(|path| path.as_path() == codex.as_path()) + ); + } + #[test] fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() { let cwd = if cfg!(windows) { From e919dbaa59c13de49ea0fbdd80f5d9e470810a52 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 3/8] safety: honor filesystem policy carveouts in apply_patch --- codex-rs/core/src/apply_patch.rs | 1 + codex-rs/core/src/safety.rs | 94 ++++++++++++++++++++++++++------ 2 files changed, 79 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 0b9cca1c9d..1928a95621 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -40,6 +40,7 @@ pub(crate) async fn apply_patch( &action, turn_context.approval_policy.value(), turn_context.sandbox_policy.get(), + &turn_context.file_system_sandbox_policy, &turn_context.cwd, turn_context.windows_sandbox_level, ) { diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 350e7dad0f..c448e3e1aa 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -9,6 +9,7 @@ use crate::exec::SandboxType; use crate::util::resolve_path; use crate::protocol::AskForApproval; +use crate::protocol::FileSystemSandboxPolicy; use crate::protocol::SandboxPolicy; use codex_protocol::config_types::WindowsSandboxLevel; @@ -28,6 +29,7 @@ pub fn assess_patch_safety( action: &ApplyPatchAction, policy: AskForApproval, sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, windows_sandbox_level: WindowsSandboxLevel, ) -> SafetyCheck { @@ -60,7 +62,7 @@ pub fn assess_patch_safety( // Even though the patch appears to be constrained to writable paths, it is // possible that paths in the patch are hard links to files outside the // writable roots, so we should still run `apply_patch` in a sandbox in that case. - if is_write_patch_constrained_to_writable_paths(action, sandbox_policy, cwd) + if is_write_patch_constrained_to_writable_paths(action, file_system_sandbox_policy, cwd) || matches!(policy, AskForApproval::OnFailure) { if matches!( @@ -122,20 +124,9 @@ pub fn get_platform_sandbox(windows_sandbox_enabled: bool) -> Option bool { - // Early‑exit if there are no declared writable roots. - let writable_roots = match sandbox_policy { - SandboxPolicy::ReadOnly { .. } => { - return false; - } - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { - return true; - } - SandboxPolicy::WorkspaceWrite { .. } => sandbox_policy.get_writable_roots_with_cwd(cwd), - }; - // Normalize a path by removing `.` and resolving `..` without touching the // filesystem (works even if the file does not exist). fn normalize(path: &Path) -> Option { @@ -152,6 +143,9 @@ fn is_write_patch_constrained_to_writable_paths( Some(out) } + let unreadable_roots = file_system_sandbox_policy.get_unreadable_roots_with_cwd(cwd); + let writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(cwd); + // Determine whether `path` is inside **any** writable root. Both `path` // and roots are converted to absolute, normalized forms before the // prefix check. @@ -162,6 +156,17 @@ fn is_write_patch_constrained_to_writable_paths( None => return false, }; + if unreadable_roots + .iter() + .any(|root| abs.starts_with(root.as_path())) + { + return false; + } + + if file_system_sandbox_policy.has_full_disk_write_access() { + return true; + } + writable_roots .iter() .any(|writable_root| writable_root.is_path_writable(&abs)) @@ -193,6 +198,11 @@ fn is_write_patch_constrained_to_writable_paths( #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::FileSystemAccessMode; + use codex_protocol::protocol::FileSystemPath; + use codex_protocol::protocol::FileSystemSandboxEntry; + use codex_protocol::protocol::FileSystemSpecialPath; + use codex_protocol::protocol::FileSystemSpecialPathKind; use codex_protocol::protocol::RejectConfig; use codex_utils_absolute_path::AbsolutePathBuf; use tempfile::TempDir; @@ -223,13 +233,13 @@ mod tests { assert!(is_write_patch_constrained_to_writable_paths( &add_inside, - &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, )); assert!(!is_write_patch_constrained_to_writable_paths( &add_outside, - &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, )); @@ -244,7 +254,7 @@ mod tests { }; assert!(is_write_patch_constrained_to_writable_paths( &add_outside, - &policy_with_parent, + &FileSystemSandboxPolicy::from(&policy_with_parent), &cwd, )); } @@ -264,6 +274,7 @@ mod tests { &add_inside, AskForApproval::OnRequest, &policy, + &FileSystemSandboxPolicy::from(&policy), &cwd, WindowsSandboxLevel::Disabled ), @@ -294,6 +305,7 @@ mod tests { &add_outside, AskForApproval::OnRequest, &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -308,6 +320,7 @@ mod tests { mcp_elicitations: false, }), &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -339,6 +352,7 @@ mod tests { mcp_elicitations: false, }), &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -348,4 +362,52 @@ mod tests { }, ); } + + #[test] + fn explicit_unreadable_paths_prevent_auto_approval_for_external_sandbox() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path().to_path_buf(); + let blocked_path = cwd.join("blocked.txt"); + let action = ApplyPatchAction::new_add_for_test(&blocked_path, "".to_string()); + let sandbox_policy = SandboxPolicy::ExternalSandbox { + network_access: codex_protocol::protocol::NetworkAccess::Restricted, + }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::CurrentWorkingDirectory, + subpath: Some(PathBuf::from("blocked.txt")), + }, + }, + access: FileSystemAccessMode::None, + }, + ]); + + assert!(!is_write_patch_constrained_to_writable_paths( + &action, + &file_system_sandbox_policy, + &cwd, + )); + assert_eq!( + assess_patch_safety( + &action, + AskForApproval::OnRequest, + &sandbox_policy, + &file_system_sandbox_policy, + &cwd, + WindowsSandboxLevel::Disabled, + ), + SafetyCheck::AskUser, + ); + } } From f0f11deed7b44b73e3e02eaf19c0491ed3605639 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 4/8] seatbelt: honor split filesystem sandbox policies --- codex-rs/core/src/sandboxing/mod.rs | 7 +- codex-rs/core/src/seatbelt.rs | 489 +++++++++++++++++++------ codex-rs/core/src/tools/js_repl/mod.rs | 22 +- 3 files changed, 385 insertions(+), 133 deletions(-) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 623c97d301..eab5d0430e 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -26,7 +26,7 @@ use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; #[cfg(target_os = "macos")] -use crate::seatbelt::create_seatbelt_command_args_with_extensions; +use crate::seatbelt::create_seatbelt_command_args_for_policies_with_extensions; #[cfg(target_os = "macos")] use crate::spawn::CODEX_SANDBOX_ENV_VAR; use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; @@ -494,9 +494,10 @@ impl SandboxManager { SandboxType::MacosSeatbelt => { let mut seatbelt_env = HashMap::new(); seatbelt_env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); - let mut args = create_seatbelt_command_args_with_extensions( + let mut args = create_seatbelt_command_args_for_policies_with_extensions( command.clone(), - &effective_policy, + &effective_file_system_policy, + effective_network_policy, sandbox_policy_cwd, enforce_managed_network, network, diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index fc6d4624c9..aa54495d61 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -15,6 +15,7 @@ use tokio::process::Child; use tracing::warn; use url::Url; +use crate::protocol::FileSystemSandboxPolicy; use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; @@ -260,10 +261,23 @@ fn unix_socket_policy(proxy: &ProxyPolicyInputs) -> String { policy } +#[cfg_attr(not(test), allow(dead_code))] fn dynamic_network_policy( sandbox_policy: &SandboxPolicy, enforce_managed_network: bool, proxy: &ProxyPolicyInputs, +) -> String { + dynamic_network_policy_for_network( + NetworkSandboxPolicy::from(sandbox_policy), + enforce_managed_network, + proxy, + ) +} + +fn dynamic_network_policy_for_network( + network_policy: NetworkSandboxPolicy, + enforce_managed_network: bool, + proxy: &ProxyPolicyInputs, ) -> String { let should_use_restricted_network_policy = !proxy.ports.is_empty() || proxy.has_proxy_config || enforce_managed_network; @@ -288,7 +302,19 @@ fn dynamic_network_policy( return format!("{policy}{MACOS_SEATBELT_NETWORK_POLICY}"); } - if sandbox_policy.has_full_network_access() { + if proxy.has_proxy_config { + // Proxy configuration is present but we could not infer any valid loopback endpoints. + // Fail closed to avoid silently widening network access in proxy-enforced sessions. + return String::new(); + } + + if enforce_managed_network { + // Managed network requirements are active but no usable proxy endpoints + // are available. Fail closed for network access. + return String::new(); + } + + if network_policy.is_enabled() { // No proxy env is configured: retain the existing full-network behavior. format!( "(allow network-outbound)\n(allow network-inbound)\n{MACOS_SEATBELT_NETWORK_POLICY}" @@ -305,9 +331,28 @@ pub(crate) fn create_seatbelt_command_args( enforce_managed_network: bool, network: Option<&NetworkProxy>, ) -> Vec { - create_seatbelt_command_args_with_extensions( + create_seatbelt_command_args_for_policies( command, - sandbox_policy, + &FileSystemSandboxPolicy::from(sandbox_policy), + NetworkSandboxPolicy::from(sandbox_policy), + sandbox_policy_cwd, + enforce_managed_network, + network, + ) +} + +pub(crate) fn create_seatbelt_command_args_for_policies( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + enforce_managed_network: bool, + network: Option<&NetworkProxy>, +) -> Vec { + create_seatbelt_command_args_for_policies_with_extensions( + command, + file_system_sandbox_policy, + network_sandbox_policy, sandbox_policy_cwd, enforce_managed_network, network, @@ -315,6 +360,64 @@ pub(crate) fn create_seatbelt_command_args( ) } +fn root_absolute_path() -> AbsolutePathBuf { + match AbsolutePathBuf::from_absolute_path(Path::new("/")) { + Ok(path) => path, + Err(err) => panic!("root path must be absolute: {err}"), + } +} + +#[derive(Debug, Clone)] +struct SeatbeltAccessRoot { + root: AbsolutePathBuf, + excluded_subpaths: Vec, +} + +fn build_seatbelt_access_policy( + action: &str, + param_prefix: &str, + roots: Vec, +) -> (String, Vec<(String, PathBuf)>) { + let mut policy_components = Vec::new(); + let mut params = Vec::new(); + + for (index, access_root) in roots.into_iter().enumerate() { + let root = + normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root); + let root_param = format!("{param_prefix}_{index}"); + params.push((root_param.clone(), root.into_path_buf())); + + if access_root.excluded_subpaths.is_empty() { + policy_components.push(format!("(subpath (param \"{root_param}\"))")); + continue; + } + + let mut require_parts = vec![format!("(subpath (param \"{root_param}\"))")]; + for (excluded_index, excluded_subpath) in + access_root.excluded_subpaths.into_iter().enumerate() + { + let excluded_subpath = + normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath); + let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}"); + params.push((excluded_param.clone(), excluded_subpath.into_path_buf())); + require_parts.push(format!( + "(require-not (subpath (param \"{excluded_param}\")))" + )); + } + policy_components.push(format!("(require-all {} )", require_parts.join(" "))); + } + + if policy_components.is_empty() { + (String::new(), Vec::new()) + } else { + ( + format!("(allow {action}\n{}\n)", policy_components.join(" ")), + params, + ) + } +} + +#[cfg_attr(not(test), allow(dead_code))] pub(crate) fn create_seatbelt_command_args_with_extensions( command: Vec, sandbox_policy: &SandboxPolicy, @@ -323,101 +426,132 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( network: Option<&NetworkProxy>, extensions: Option<&MacOsSeatbeltProfileExtensions>, ) -> Vec { - let (file_write_policy, file_write_dir_params) = { - if sandbox_policy.has_full_disk_write_access() { - // Allegedly, this is more permissive than `(allow file-write*)`. - ( - r#"(allow file-write* (regex #"^/"))"#.to_string(), - Vec::new(), - ) - } else { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd); + create_seatbelt_command_args_for_policies_with_extensions( + command, + &FileSystemSandboxPolicy::from(sandbox_policy), + NetworkSandboxPolicy::from(sandbox_policy), + sandbox_policy_cwd, + enforce_managed_network, + network, + extensions, + ) +} - let mut writable_folder_policies: Vec = Vec::new(); - let mut file_write_params = Vec::new(); +pub(crate) fn create_seatbelt_command_args_for_policies_with_extensions( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + enforce_managed_network: bool, + network: Option<&NetworkProxy>, + extensions: Option<&MacOsSeatbeltProfileExtensions>, +) -> Vec { + create_seatbelt_command_args_from_policies_inner( + command, + file_system_sandbox_policy, + network_sandbox_policy, + sandbox_policy_cwd, + enforce_managed_network, + network, + extensions, + ) +} - for (index, wr) in writable_roots.iter().enumerate() { - // Canonicalize to avoid mismatches like /var vs /private/var on macOS. - let canonical_root = wr - .root - .as_path() - .canonicalize() - .unwrap_or_else(|_| wr.root.to_path_buf()); - let root_param = format!("WRITABLE_ROOT_{index}"); - file_write_params.push((root_param.clone(), canonical_root)); - - if wr.read_only_subpaths.is_empty() { - writable_folder_policies.push(format!("(subpath (param \"{root_param}\"))")); - } else { - // Add parameters for each read-only subpath and generate - // the `(require-not ...)` clauses. - let mut require_parts: Vec = Vec::new(); - require_parts.push(format!("(subpath (param \"{root_param}\"))")); - for (subpath_index, ro) in wr.read_only_subpaths.iter().enumerate() { - let canonical_ro = ro - .as_path() - .canonicalize() - .unwrap_or_else(|_| ro.to_path_buf()); - let ro_param = format!("WRITABLE_ROOT_{index}_RO_{subpath_index}"); - require_parts - .push(format!("(require-not (subpath (param \"{ro_param}\")))")); - file_write_params.push((ro_param, canonical_ro)); - } - let policy_component = format!("(require-all {} )", require_parts.join(" ")); - writable_folder_policies.push(policy_component); - } - } - - if writable_folder_policies.is_empty() { - ("".to_string(), Vec::new()) +fn create_seatbelt_command_args_from_policies_inner( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + enforce_managed_network: bool, + network: Option<&NetworkProxy>, + extensions: Option<&MacOsSeatbeltProfileExtensions>, +) -> Vec { + let unreadable_roots = + file_system_sandbox_policy.get_unreadable_roots_with_cwd(sandbox_policy_cwd); + let (file_write_policy, file_write_dir_params) = + if file_system_sandbox_policy.has_full_disk_write_access() { + if unreadable_roots.is_empty() { + // Allegedly, this is more permissive than `(allow file-write*)`. + ( + r#"(allow file-write* (regex #"^/"))"#.to_string(), + Vec::new(), + ) } else { - let file_write_policy = format!( - "(allow file-write*\n{}\n)", - writable_folder_policies.join(" ") - ); - (file_write_policy, file_write_params) + build_seatbelt_access_policy( + "file-write*", + "WRITABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots.clone(), + }], + ) } - } - }; - - let (file_read_policy, file_read_dir_params) = if sandbox_policy.has_full_disk_read_access() { - ( - "; allow read-only file operations\n(allow file-read*)".to_string(), - Vec::new(), - ) - } else { - let mut readable_roots_policies: Vec = Vec::new(); - let mut file_read_params = Vec::new(); - for (index, root) in sandbox_policy - .get_readable_roots_with_cwd(sandbox_policy_cwd) - .into_iter() - .enumerate() - { - // Canonicalize to avoid mismatches like /var vs /private/var on macOS. - let canonical_root = root - .as_path() - .canonicalize() - .unwrap_or_else(|_| root.to_path_buf()); - let root_param = format!("READABLE_ROOT_{index}"); - file_read_params.push((root_param.clone(), canonical_root)); - readable_roots_policies.push(format!("(subpath (param \"{root_param}\"))")); - } - - if readable_roots_policies.is_empty() { - ("".to_string(), Vec::new()) } else { - ( - format!( - "; allow read-only file operations\n(allow file-read*\n{}\n)", - readable_roots_policies.join(" ") - ), - file_read_params, + build_seatbelt_access_policy( + "file-write*", + "WRITABLE_ROOT", + file_system_sandbox_policy + .get_writable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(|root| SeatbeltAccessRoot { + root: root.root, + excluded_subpaths: root.read_only_subpaths, + }) + .collect(), ) - } - }; + }; + + let (file_read_policy, file_read_dir_params) = + if file_system_sandbox_policy.has_full_disk_read_access() { + if unreadable_roots.is_empty() { + ( + "; allow read-only file operations\n(allow file-read*)".to_string(), + Vec::new(), + ) + } else { + let (policy, params) = build_seatbelt_access_policy( + "file-read*", + "READABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots, + }], + ); + ( + format!("; allow read-only file operations\n{policy}"), + params, + ) + } + } else { + let (policy, params) = build_seatbelt_access_policy( + "file-read*", + "READABLE_ROOT", + file_system_sandbox_policy + .get_readable_roots_with_cwd(sandbox_policy_cwd) + .into_iter() + .map(|root| SeatbeltAccessRoot { + excluded_subpaths: unreadable_roots + .iter() + .filter(|path| path.as_path().starts_with(root.as_path())) + .cloned() + .collect(), + root, + }) + .collect(), + ); + if policy.is_empty() { + (String::new(), params) + } else { + ( + format!("; allow read-only file operations\n{policy}"), + params, + ) + } + }; let proxy = proxy_policy_inputs(network); - let network_policy = dynamic_network_policy(sandbox_policy, enforce_managed_network, &proxy); + let network_policy = + dynamic_network_policy_for_network(network_sandbox_policy, enforce_managed_network, &proxy); let seatbelt_extensions = extensions.map_or_else( || { // Backward-compatibility default when no extension profile is provided. @@ -426,7 +560,7 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( build_seatbelt_extensions, ); - let include_platform_defaults = sandbox_policy.include_platform_defaults(); + let include_platform_defaults = file_system_sandbox_policy.include_platform_defaults(); let mut policy_sections = vec![ MACOS_SEATBELT_BASE_POLICY.to_string(), file_read_policy, @@ -493,12 +627,18 @@ mod tests { use super::ProxyPolicyInputs; use super::UnixDomainSocketPolicy; use super::create_seatbelt_command_args; + use super::create_seatbelt_command_args_for_policies; use super::create_seatbelt_command_args_with_extensions; use super::dynamic_network_policy; use super::macos_dir_params; use super::normalize_path_for_sandbox; use super::unix_socket_dir_params; use super::unix_socket_policy; + use crate::protocol::FileSystemAccessMode; + use crate::protocol::FileSystemPath; + use crate::protocol::FileSystemSandboxEntry; + use crate::protocol::FileSystemSandboxPolicy; + use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; use crate::seatbelt_permissions::MacOsAutomationPermission; @@ -526,6 +666,15 @@ mod tests { AbsolutePathBuf::from_absolute_path(Path::new(path)).expect("absolute path") } + fn seatbelt_policy_arg(args: &[String]) -> &str { + let policy_index = args + .iter() + .position(|arg| arg == "-p") + .expect("seatbelt args should include -p"); + args.get(policy_index + 1) + .expect("seatbelt args should include policy text") + } + #[test] fn base_policy_allows_node_cpu_sysctls() { assert!( @@ -573,6 +722,96 @@ mod tests { ); } + #[test] + fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() { + let unreadable = absolute_path("/tmp/codex-unreadable"); + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: crate::protocol::FileSystemSpecialPath { + kind: crate::protocol::FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: unreadable }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_seatbelt_command_args_for_policies( + vec!["/bin/true".to_string()], + &file_system_policy, + NetworkSandboxPolicy::Restricted, + Path::new("/"), + false, + None, + ); + + let policy = seatbelt_policy_arg(&args); + assert!( + policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), + "expected read carveout in policy:\n{policy}" + ); + assert!( + policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_RO_0\")))"), + "expected write carveout in policy:\n{policy}" + ); + assert!( + args.iter() + .any(|arg| arg == "-DREADABLE_ROOT_0_RO_0=/tmp/codex-unreadable"), + "expected read carveout parameter in args: {args:#?}" + ); + assert!( + args.iter() + .any(|arg| arg == "-DWRITABLE_ROOT_0_RO_0=/tmp/codex-unreadable"), + "expected write carveout parameter in args: {args:#?}" + ); + } + + #[test] + fn explicit_unreadable_paths_are_excluded_from_readable_roots() { + let root = absolute_path("/tmp/codex-readable"); + let unreadable = absolute_path("/tmp/codex-readable/private"); + let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: root }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: unreadable }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_seatbelt_command_args_for_policies( + vec!["/bin/true".to_string()], + &file_system_policy, + NetworkSandboxPolicy::Restricted, + Path::new("/"), + false, + None, + ); + + let policy = seatbelt_policy_arg(&args); + assert!( + policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"), + "expected read carveout in policy:\n{policy}" + ); + assert!( + args.iter() + .any(|arg| arg == "-DREADABLE_ROOT_0=/tmp/codex-readable"), + "expected readable root parameter in args: {args:#?}" + ); + assert!( + args.iter() + .any(|arg| arg == "-DREADABLE_ROOT_0_RO_0=/tmp/codex-readable/private"), + "expected read carveout parameter in args: {args:#?}" + ); + } + #[test] fn seatbelt_args_include_macos_permission_extensions() { let cwd = std::env::temp_dir(); @@ -991,7 +1230,7 @@ sys.exit(0 if allowed else 13) ; allow read-only file operations (allow file-read*) (allow file-write* -(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_RO_1"))) ) (subpath (param "WRITABLE_ROOT_1")) (subpath (param "WRITABLE_ROOT_2")) +(subpath (param "WRITABLE_ROOT_0")) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (subpath (param "WRITABLE_ROOT_1_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_RO_1"))) ) (subpath (param "WRITABLE_ROOT_2")) ) ; macOS permission profile extensions @@ -1004,43 +1243,51 @@ sys.exit(0 if allowed else 13) "#, ); - let mut expected_args = vec![ - "-p".to_string(), - expected_policy, + assert_eq!(seatbelt_policy_arg(&args), expected_policy); + + let expected_definitions = [ format!( "-DWRITABLE_ROOT_0={}", - vulnerable_root_canonical.to_string_lossy() - ), - format!( - "-DWRITABLE_ROOT_0_RO_0={}", - dot_git_canonical.to_string_lossy() - ), - format!( - "-DWRITABLE_ROOT_0_RO_1={}", - dot_codex_canonical.to_string_lossy() - ), - format!( - "-DWRITABLE_ROOT_1={}", - empty_root_canonical.to_string_lossy() - ), - format!( - "-DWRITABLE_ROOT_2={}", cwd.canonicalize() .expect("canonicalize cwd") .to_string_lossy() ), + format!( + "-DWRITABLE_ROOT_1={}", + vulnerable_root_canonical.to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1_RO_0={}", + dot_git_canonical.to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1_RO_1={}", + dot_codex_canonical.to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_2={}", + empty_root_canonical.to_string_lossy() + ), ]; + for expected_definition in expected_definitions { + assert!( + args.contains(&expected_definition), + "expected definition arg `{expected_definition}` in {args:#?}" + ); + } + for (key, value) in macos_dir_params() { + let expected_definition = format!("-D{key}={}", value.to_string_lossy()); + assert!( + args.contains(&expected_definition), + "expected definition arg `{expected_definition}` in {args:#?}" + ); + } - expected_args.extend( - macos_dir_params() - .into_iter() - .map(|(key, value)| format!("-D{key}={value}", value = value.to_string_lossy())), - ); - - expected_args.push("--".to_string()); - expected_args.extend(shell_command); - - assert_eq!(expected_args, args); + let command_index = args + .iter() + .position(|arg| arg == "--") + .expect("seatbelt args should include command separator"); + assert_eq!(args[command_index + 1..], shell_command); // Verify that .codex/config.toml cannot be modified under the generated // Seatbelt policy. diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index f9cc3d854b..bc2a5342ce 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1750,6 +1750,16 @@ mod tests { use std::path::Path; use tempfile::tempdir; + fn set_danger_full_access(turn: &mut crate::codex::TurnContext) { + turn.sandbox_policy + .set(SandboxPolicy::DangerFullAccess) + .expect("test setup should allow updating sandbox policy"); + turn.file_system_sandbox_policy = + crate::protocol::FileSystemSandboxPolicy::from(turn.sandbox_policy.get()); + turn.network_sandbox_policy = + crate::protocol::NetworkSandboxPolicy::from(turn.sandbox_policy.get()); + } + #[test] fn node_version_parses_v_prefix_and_suffix() { let version = NodeVersion::parse("v25.1.0-nightly.2024").unwrap(); @@ -2467,9 +2477,7 @@ mod tests { turn.approval_policy .set(AskForApproval::Never) .expect("test setup should allow updating approval policy"); - turn.sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); + set_danger_full_access(&mut turn); let session = Arc::new(session); let turn = Arc::new(turn); @@ -2521,9 +2529,7 @@ console.log("cell-complete"); turn.approval_policy .set(AskForApproval::Never) .expect("test setup should allow updating approval policy"); - turn.sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); + set_danger_full_access(&mut turn); let session = Arc::new(session); let turn = Arc::new(turn); @@ -2579,9 +2585,7 @@ console.log(out.type); turn.approval_policy .set(AskForApproval::Never) .expect("test setup should allow updating approval policy"); - turn.sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); + set_danger_full_access(&mut turn); let session = Arc::new(session); let turn = Arc::new(turn); From 082fff3668fa304ef338eab6317f4c31f84cca4f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 5/8] linux-sandbox: plumb split sandbox policies through helper --- codex-rs/core/src/landlock.rs | 113 +++++++++++++---- codex-rs/core/src/sandboxing/mod.rs | 6 +- codex-rs/linux-sandbox/src/landlock.rs | 12 +- codex-rs/linux-sandbox/src/linux_run_main.rs | 115 ++++++++++++++++-- .../linux-sandbox/src/linux_run_main_tests.rs | 98 ++++++++++++++- codex-rs/protocol/src/protocol.rs | 16 +++ 6 files changed, 313 insertions(+), 47 deletions(-) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 7e49191067..9a55badcdf 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -1,3 +1,4 @@ +use crate::protocol::FileSystemSandboxPolicy; use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; use crate::spawn::SpawnChildRequest; @@ -14,9 +15,9 @@ use tokio::process::Child; /// isolation plus seccomp for network restrictions. /// /// Unlike macOS Seatbelt where we directly embed the policy text, the Linux -/// helper accepts a list of `--sandbox-permission`/`-s` flags mirroring the -/// public CLI. We convert the internal [`SandboxPolicy`] representation into -/// the equivalent CLI options. +/// helper is a separate executable. We pass the legacy [`SandboxPolicy`] plus +/// split filesystem/network policies as JSON so the helper can migrate +/// incrementally without breaking older call sites. #[allow(clippy::too_many_arguments)] pub async fn spawn_command_under_linux_sandbox

( codex_linux_sandbox_exe: P, @@ -32,9 +33,13 @@ pub async fn spawn_command_under_linux_sandbox

( where P: AsRef, { - let args = create_linux_sandbox_command_args( + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(sandbox_policy); + let network_sandbox_policy = NetworkSandboxPolicy::from(sandbox_policy); + let args = create_linux_sandbox_command_args_for_policies( command, sandbox_policy, + &file_system_sandbox_policy, + network_sandbox_policy, sandbox_policy_cwd, use_bwrap_sandbox, allow_network_for_proxy(false), @@ -45,7 +50,7 @@ where args, arg0, cwd: command_cwd, - network_sandbox_policy: NetworkSandboxPolicy::from(sandbox_policy), + network_sandbox_policy, network, stdio_policy, env, @@ -60,13 +65,55 @@ pub(crate) fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { enforce_managed_network } -/// Converts the sandbox policy into the CLI invocation for `codex-linux-sandbox`. +/// Converts the sandbox policies into the CLI invocation for +/// `codex-linux-sandbox`. /// /// The helper performs the actual sandboxing (bubblewrap + seccomp) after /// parsing these arguments. See `docs/linux_sandbox.md` for the Linux semantics. -pub(crate) fn create_linux_sandbox_command_args( +#[allow(clippy::too_many_arguments)] +pub(crate) fn create_linux_sandbox_command_args_for_policies( command: Vec, sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + sandbox_policy_cwd: &Path, + use_bwrap_sandbox: bool, + allow_network_for_proxy: bool, +) -> Vec { + #[expect(clippy::expect_used)] + let sandbox_policy_json = + serde_json::to_string(sandbox_policy).expect("Failed to serialize SandboxPolicy to JSON"); + #[expect(clippy::expect_used)] + let file_system_policy_json = serde_json::to_string(file_system_sandbox_policy) + .expect("Failed to serialize FileSystemSandboxPolicy to JSON"); + #[expect(clippy::expect_used)] + let network_policy_json = serde_json::to_string(&network_sandbox_policy) + .expect("Failed to serialize NetworkSandboxPolicy to JSON"); + + let mut linux_cmd = create_linux_sandbox_command_args( + command, + sandbox_policy_cwd, + use_bwrap_sandbox, + allow_network_for_proxy, + ); + linux_cmd.splice( + 2..2, + [ + "--sandbox-policy".to_string(), + sandbox_policy_json, + "--file-system-sandbox-policy".to_string(), + file_system_policy_json, + "--network-sandbox-policy".to_string(), + network_policy_json, + ], + ); + linux_cmd +} + +/// Converts the sandbox cwd and execution options into the CLI invocation for +/// `codex-linux-sandbox`. +pub(crate) fn create_linux_sandbox_command_args( + command: Vec, sandbox_policy_cwd: &Path, use_bwrap_sandbox: bool, allow_network_for_proxy: bool, @@ -77,16 +124,7 @@ pub(crate) fn create_linux_sandbox_command_args( .expect("cwd must be valid UTF-8") .to_string(); - #[expect(clippy::expect_used)] - let sandbox_policy_json = - serde_json::to_string(sandbox_policy).expect("Failed to serialize SandboxPolicy to JSON"); - - let mut linux_cmd: Vec = vec![ - "--sandbox-policy-cwd".to_string(), - sandbox_policy_cwd, - "--sandbox-policy".to_string(), - sandbox_policy_json, - ]; + let mut linux_cmd: Vec = vec!["--sandbox-policy-cwd".to_string(), sandbox_policy_cwd]; if use_bwrap_sandbox { linux_cmd.push("--use-bwrap-sandbox".to_string()); } @@ -113,16 +151,14 @@ mod tests { fn bwrap_flags_are_feature_gated() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::new_read_only_policy(); - let with_bwrap = - create_linux_sandbox_command_args(command.clone(), &policy, cwd, true, false); + let with_bwrap = create_linux_sandbox_command_args(command.clone(), cwd, true, false); assert_eq!( with_bwrap.contains(&"--use-bwrap-sandbox".to_string()), true ); - let without_bwrap = create_linux_sandbox_command_args(command, &policy, cwd, false, false); + let without_bwrap = create_linux_sandbox_command_args(command, cwd, false, false); assert_eq!( without_bwrap.contains(&"--use-bwrap-sandbox".to_string()), false @@ -133,15 +169,46 @@ mod tests { fn proxy_flag_is_included_when_requested() { let command = vec!["/bin/true".to_string()]; let cwd = Path::new("/tmp"); - let policy = SandboxPolicy::new_read_only_policy(); - let args = create_linux_sandbox_command_args(command, &policy, cwd, true, true); + let args = create_linux_sandbox_command_args(command, cwd, true, true); assert_eq!( args.contains(&"--allow-network-for-proxy".to_string()), true ); } + #[test] + fn split_policy_flags_are_included() { + let command = vec!["/bin/true".to_string()]; + let cwd = Path::new("/tmp"); + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); + let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); + + let args = create_linux_sandbox_command_args_for_policies( + command, + &sandbox_policy, + &file_system_sandbox_policy, + network_sandbox_policy, + cwd, + true, + false, + ); + + assert_eq!( + args.windows(2).any(|window| { + window[0] == "--file-system-sandbox-policy" && !window[1].is_empty() + }), + true + ); + assert_eq!( + args.windows(2) + .any(|window| window[0] == "--network-sandbox-policy" + && window[1] == "\"restricted\""), + true + ); + } + #[test] fn proxy_network_requires_managed_requirements() { assert_eq!(allow_network_for_proxy(false), false); diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index eab5d0430e..eae5554848 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -14,7 +14,7 @@ use crate::exec::SandboxType; use crate::exec::StdoutStream; use crate::exec::execute_exec_request; use crate::landlock::allow_network_for_proxy; -use crate::landlock::create_linux_sandbox_command_args; +use crate::landlock::create_linux_sandbox_command_args_for_policies; use crate::protocol::FileSystemAccessMode; use crate::protocol::FileSystemPath; use crate::protocol::FileSystemSandboxEntry; @@ -514,9 +514,11 @@ impl SandboxManager { let exe = codex_linux_sandbox_exe .ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?; let allow_proxy_network = allow_network_for_proxy(enforce_managed_network); - let mut args = create_linux_sandbox_command_args( + let mut args = create_linux_sandbox_command_args_for_policies( command.clone(), &effective_policy, + &effective_file_system_policy, + effective_network_policy, sandbox_policy_cwd, use_linux_sandbox_bwrap, allow_proxy_network, diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index f9477a1264..a120ab7054 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -8,6 +8,7 @@ use std::path::Path; use codex_core::error::CodexErr; use codex_core::error::Result; use codex_core::error::SandboxErr; +use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -40,13 +41,14 @@ use seccompiler::apply_filter; /// Filesystem restrictions are intentionally handled by bubblewrap. pub(crate) fn apply_sandbox_policy_to_current_thread( sandbox_policy: &SandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, cwd: &Path, apply_landlock_fs: bool, allow_network_for_proxy: bool, proxy_routed_network: bool, ) -> Result<()> { let network_seccomp_mode = network_seccomp_mode( - sandbox_policy, + network_sandbox_policy, allow_network_for_proxy, proxy_routed_network, ); @@ -91,20 +93,20 @@ enum NetworkSeccompMode { } fn should_install_network_seccomp( - sandbox_policy: &SandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, allow_network_for_proxy: bool, ) -> bool { // Managed-network sessions should remain fail-closed even for policies that // would normally grant full network access (for example, DangerFullAccess). - !sandbox_policy.has_full_network_access() || allow_network_for_proxy + !network_sandbox_policy.is_enabled() || allow_network_for_proxy } fn network_seccomp_mode( - sandbox_policy: &SandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, allow_network_for_proxy: bool, proxy_routed_network: bool, ) -> Option { - if !should_install_network_seccomp(sandbox_policy, allow_network_for_proxy) { + if !should_install_network_seccomp(network_sandbox_policy, allow_network_for_proxy) { None } else if proxy_routed_network { Some(NetworkSeccompMode::ProxyRouted) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index c9fdd06a32..6a624ac1dd 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -14,6 +14,9 @@ use crate::proxy_routing::activate_proxy_routes_in_netns; use crate::proxy_routing::prepare_host_proxy_route_spec; use crate::vendored_bwrap::exec_vendored_bwrap; use crate::vendored_bwrap::run_vendored_bwrap_main; +use codex_protocol::protocol::FileSystemSandboxPolicy; +use codex_protocol::protocol::NetworkSandboxPolicy; +use codex_protocol::protocol::SandboxPolicy; #[derive(Debug, Parser)] /// CLI surface for the Linux sandbox helper. @@ -26,8 +29,18 @@ pub struct LandlockCommand { #[arg(long = "sandbox-policy-cwd")] pub sandbox_policy_cwd: PathBuf, - #[arg(long = "sandbox-policy")] - pub sandbox_policy: codex_protocol::protocol::SandboxPolicy, + /// Legacy compatibility policy. + /// + /// Newer callers pass split filesystem/network policies as well so the + /// helper can migrate incrementally without breaking older invocations. + #[arg(long = "sandbox-policy", hide = true)] + pub sandbox_policy: Option, + + #[arg(long = "file-system-sandbox-policy", hide = true)] + pub file_system_sandbox_policy: Option, + + #[arg(long = "network-sandbox-policy", hide = true)] + pub network_sandbox_policy: Option, /// Opt-in: use the bubblewrap-based Linux sandbox pipeline. /// @@ -77,6 +90,8 @@ pub fn run_main() -> ! { let LandlockCommand { sandbox_policy_cwd, sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, use_bwrap_sandbox, apply_seccomp_then_exec, allow_network_for_proxy, @@ -89,6 +104,16 @@ pub fn run_main() -> ! { panic!("No command specified to execute."); } ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_bwrap_sandbox); + let EffectiveSandboxPolicies { + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + } = resolve_sandbox_policies( + sandbox_policy_cwd.as_path(), + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + ); // Inner stage: apply seccomp/no_new_privs after bubblewrap has already // established the filesystem view. @@ -104,6 +129,7 @@ pub fn run_main() -> ! { let proxy_routing_active = allow_network_for_proxy; if let Err(e) = apply_sandbox_policy_to_current_thread( &sandbox_policy, + network_sandbox_policy, &sandbox_policy_cwd, false, allow_network_for_proxy, @@ -114,9 +140,10 @@ pub fn run_main() -> ! { exec_or_panic(command); } - if sandbox_policy.has_full_disk_write_access() && !allow_network_for_proxy { + if file_system_sandbox_policy.has_full_disk_write_access() && !allow_network_for_proxy { if let Err(e) = apply_sandbox_policy_to_current_thread( &sandbox_policy, + network_sandbox_policy, &sandbox_policy_cwd, false, allow_network_for_proxy, @@ -142,6 +169,8 @@ pub fn run_main() -> ! { let inner = build_inner_seccomp_command( &sandbox_policy_cwd, &sandbox_policy, + &file_system_sandbox_policy, + network_sandbox_policy, use_bwrap_sandbox, allow_network_for_proxy, proxy_route_spec, @@ -150,6 +179,7 @@ pub fn run_main() -> ! { run_bwrap_with_proc_fallback( &sandbox_policy_cwd, &sandbox_policy, + network_sandbox_policy, inner, !no_proc, allow_network_for_proxy, @@ -159,6 +189,7 @@ pub fn run_main() -> ! { // Legacy path: Landlock enforcement only, when bwrap sandboxing is not enabled. if let Err(e) = apply_sandbox_policy_to_current_thread( &sandbox_policy, + network_sandbox_policy, &sandbox_policy_cwd, true, allow_network_for_proxy, @@ -169,6 +200,53 @@ pub fn run_main() -> ! { exec_or_panic(command); } +#[derive(Debug, Clone)] +struct EffectiveSandboxPolicies { + sandbox_policy: SandboxPolicy, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, +} + +fn resolve_sandbox_policies( + sandbox_policy_cwd: &Path, + sandbox_policy: Option, + file_system_sandbox_policy: Option, + network_sandbox_policy: Option, +) -> EffectiveSandboxPolicies { + match ( + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + ) { + (Some(sandbox_policy), Some(file_system_sandbox_policy), Some(network_sandbox_policy)) => { + EffectiveSandboxPolicies { + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + } + } + (Some(sandbox_policy), None, None) => EffectiveSandboxPolicies { + file_system_sandbox_policy: FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::from(&sandbox_policy), + sandbox_policy, + }, + (None, Some(file_system_sandbox_policy), Some(network_sandbox_policy)) => { + let sandbox_policy = file_system_sandbox_policy + .to_legacy_sandbox_policy(network_sandbox_policy, sandbox_policy_cwd) + .unwrap_or_else(|err| { + panic!("failed to derive legacy sandbox policy from split policies: {err}") + }); + EffectiveSandboxPolicies { + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + } + } + (None, None, None) => panic!("missing sandbox policy configuration"), + _ => panic!("file-system and network sandbox policies must be provided together"), + } +} + fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_bwrap_sandbox: bool) { if apply_seccomp_then_exec && !use_bwrap_sandbox { panic!("--apply-seccomp-then-exec requires --use-bwrap-sandbox"); @@ -177,12 +255,13 @@ fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_bwrap_san fn run_bwrap_with_proc_fallback( sandbox_policy_cwd: &Path, - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + sandbox_policy: &SandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, inner: Vec, mount_proc: bool, allow_network_for_proxy: bool, ) -> ! { - let network_mode = bwrap_network_mode(sandbox_policy, allow_network_for_proxy); + let network_mode = bwrap_network_mode(network_sandbox_policy, allow_network_for_proxy); let mut mount_proc = mount_proc; if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy, network_mode) @@ -200,12 +279,12 @@ fn run_bwrap_with_proc_fallback( } fn bwrap_network_mode( - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, allow_network_for_proxy: bool, ) -> BwrapNetworkMode { if allow_network_for_proxy { BwrapNetworkMode::ProxyOnly - } else if sandbox_policy.has_full_network_access() { + } else if network_sandbox_policy.is_enabled() { BwrapNetworkMode::FullAccess } else { BwrapNetworkMode::Isolated @@ -214,7 +293,7 @@ fn bwrap_network_mode( fn build_bwrap_argv( inner: Vec, - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + sandbox_policy: &SandboxPolicy, sandbox_policy_cwd: &Path, options: BwrapOptions, ) -> Vec { @@ -237,7 +316,7 @@ fn build_bwrap_argv( fn preflight_proc_mount_support( sandbox_policy_cwd: &Path, - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + sandbox_policy: &SandboxPolicy, network_mode: BwrapNetworkMode, ) -> bool { let preflight_argv = @@ -248,7 +327,7 @@ fn preflight_proc_mount_support( fn build_preflight_bwrap_argv( sandbox_policy_cwd: &Path, - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + sandbox_policy: &SandboxPolicy, network_mode: BwrapNetworkMode, ) -> Vec { let preflight_command = vec![resolve_true_command()]; @@ -361,7 +440,9 @@ fn is_proc_mount_failure(stderr: &str) -> bool { /// Build the inner command that applies seccomp after bubblewrap. fn build_inner_seccomp_command( sandbox_policy_cwd: &Path, - sandbox_policy: &codex_protocol::protocol::SandboxPolicy, + sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, use_bwrap_sandbox: bool, allow_network_for_proxy: bool, proxy_route_spec: Option, @@ -375,6 +456,14 @@ fn build_inner_seccomp_command( Ok(json) => json, Err(err) => panic!("failed to serialize sandbox policy: {err}"), }; + let file_system_policy_json = match serde_json::to_string(file_system_sandbox_policy) { + Ok(json) => json, + Err(err) => panic!("failed to serialize filesystem sandbox policy: {err}"), + }; + let network_policy_json = match serde_json::to_string(&network_sandbox_policy) { + Ok(json) => json, + Err(err) => panic!("failed to serialize network sandbox policy: {err}"), + }; let mut inner = vec![ current_exe.to_string_lossy().to_string(), @@ -382,6 +471,10 @@ fn build_inner_seccomp_command( sandbox_policy_cwd.to_string_lossy().to_string(), "--sandbox-policy".to_string(), policy_json, + "--file-system-sandbox-policy".to_string(), + file_system_policy_json, + "--network-sandbox-policy".to_string(), + network_policy_json, ]; if use_bwrap_sandbox { inner.push("--use-bwrap-sandbox".to_string()); diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index cda5090306..c466de5e27 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -1,7 +1,13 @@ #[cfg(test)] use super::*; #[cfg(test)] +use codex_protocol::protocol::FileSystemSandboxPolicy; +#[cfg(test)] +use codex_protocol::protocol::NetworkSandboxPolicy; +#[cfg(test)] use codex_protocol::protocol::SandboxPolicy; +#[cfg(test)] +use pretty_assertions::assert_eq; #[test] fn detects_proc_mount_invalid_argument_failure() { @@ -91,22 +97,25 @@ fn inserts_unshare_net_when_proxy_only_network_mode_requested() { #[test] fn proxy_only_mode_takes_precedence_over_full_network_policy() { - let mode = bwrap_network_mode(&SandboxPolicy::DangerFullAccess, true); + let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true); assert_eq!(mode, BwrapNetworkMode::ProxyOnly); } #[test] fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { - let mode = bwrap_network_mode(&SandboxPolicy::DangerFullAccess, true); + let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true); let argv = build_preflight_bwrap_argv(Path::new("/"), &SandboxPolicy::DangerFullAccess, mode); assert!(argv.iter().any(|arg| arg == "--")); } #[test] fn managed_proxy_inner_command_includes_route_spec() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let args = build_inner_seccomp_command( Path::new("/tmp"), - &SandboxPolicy::new_read_only_policy(), + &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::Restricted, true, true, Some("{\"routes\":[]}".to_string()), @@ -118,10 +127,31 @@ fn managed_proxy_inner_command_includes_route_spec() { } #[test] -fn non_managed_inner_command_omits_route_spec() { +fn inner_command_includes_split_policy_flags() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let args = build_inner_seccomp_command( Path::new("/tmp"), - &SandboxPolicy::new_read_only_policy(), + &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::Restricted, + true, + false, + None, + vec!["/bin/true".to_string()], + ); + + assert!(args.iter().any(|arg| arg == "--file-system-sandbox-policy")); + assert!(args.iter().any(|arg| arg == "--network-sandbox-policy")); +} + +#[test] +fn non_managed_inner_command_omits_route_spec() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let args = build_inner_seccomp_command( + Path::new("/tmp"), + &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::Restricted, true, false, None, @@ -134,9 +164,12 @@ fn non_managed_inner_command_omits_route_spec() { #[test] fn managed_proxy_inner_command_requires_route_spec() { let result = std::panic::catch_unwind(|| { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); build_inner_seccomp_command( Path::new("/tmp"), - &SandboxPolicy::new_read_only_policy(), + &sandbox_policy, + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::Restricted, true, true, None, @@ -146,6 +179,59 @@ fn managed_proxy_inner_command_requires_route_spec() { assert!(result.is_err()); } +#[test] +fn resolve_sandbox_policies_derives_split_policies_from_legacy_policy() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + + let resolved = + resolve_sandbox_policies(Path::new("/tmp"), Some(sandbox_policy.clone()), None, None); + + assert_eq!(resolved.sandbox_policy, sandbox_policy.clone()); + assert_eq!( + resolved.file_system_sandbox_policy, + FileSystemSandboxPolicy::from(&sandbox_policy) + ); + assert_eq!( + resolved.network_sandbox_policy, + NetworkSandboxPolicy::from(&sandbox_policy) + ); +} + +#[test] +fn resolve_sandbox_policies_derives_legacy_policy_from_split_policies() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); + let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); + + let resolved = resolve_sandbox_policies( + Path::new("/tmp"), + None, + Some(file_system_sandbox_policy.clone()), + Some(network_sandbox_policy), + ); + + assert_eq!(resolved.sandbox_policy, sandbox_policy); + assert_eq!( + resolved.file_system_sandbox_policy, + file_system_sandbox_policy + ); + assert_eq!(resolved.network_sandbox_policy, network_sandbox_policy); +} + +#[test] +fn resolve_sandbox_policies_rejects_partial_split_policies() { + let result = std::panic::catch_unwind(|| { + resolve_sandbox_policies( + Path::new("/tmp"), + Some(SandboxPolicy::new_read_only_policy()), + Some(FileSystemSandboxPolicy::default()), + None, + ) + }); + + assert!(result.is_err()); +} + #[test] fn apply_seccomp_then_exec_without_bwrap_panics() { let result = std::panic::catch_unwind(|| ensure_inner_stage_mode_is_valid(true, false)); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2ccd8d82f1..4f33d7f631 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1102,6 +1102,22 @@ impl FromStr for SandboxPolicy { } } +impl FromStr for FileSystemSandboxPolicy { + type Err = serde_json::Error; + + fn from_str(s: &str) -> Result { + serde_json::from_str(s) + } +} + +impl FromStr for NetworkSandboxPolicy { + type Err = serde_json::Error; + + fn from_str(s: &str) -> Result { + serde_json::from_str(s) + } +} + impl SandboxPolicy { /// Returns a policy with read-only disk access and no network. pub fn new_read_only_policy() -> Self { From 7c35d14add31ef8fefe6bef9a2c88c29e0aaf0d0 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 6/8] sandboxing: preserve denied paths when widening permissions --- codex-rs/core/src/exec.rs | 24 ++++++++++-------------- codex-rs/core/src/sandboxing/mod.rs | 26 ++++++-------------------- 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index cd4da20616..487c653de1 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -32,6 +32,7 @@ use crate::sandboxing::CommandSpec; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxManager; use crate::sandboxing::SandboxPermissions; +use crate::sandboxing::should_require_platform_sandbox; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; @@ -165,22 +166,17 @@ pub async fn process_exec_tool_call( ) -> Result { let windows_sandbox_level = params.windows_sandbox_level; let enforce_managed_network = params.network.is_some(); - let sandbox_type = match file_system_sandbox_policy.kind { - FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => { - if enforce_managed_network { - get_platform_sandbox( - windows_sandbox_level - != codex_protocol::config_types::WindowsSandboxLevel::Disabled, - ) - .unwrap_or(SandboxType::None) - } else { - SandboxType::None - } - } - _ => get_platform_sandbox( + let sandbox_type = if should_require_platform_sandbox( + file_system_sandbox_policy, + network_sandbox_policy, + enforce_managed_network, + ) { + get_platform_sandbox( windows_sandbox_level != codex_protocol::config_types::WindowsSandboxLevel::Disabled, ) - .unwrap_or(SandboxType::None), + .unwrap_or(SandboxType::None) + } else { + SandboxType::None }; tracing::debug!("Sandbox type: {sandbox_type:?}"); diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index eae5554848..218cbbac63 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -20,7 +20,6 @@ use crate::protocol::FileSystemPath; use crate::protocol::FileSystemSandboxEntry; use crate::protocol::FileSystemSandboxKind; use crate::protocol::FileSystemSandboxPolicy; -use crate::protocol::FileSystemSpecialPathKind; use crate::protocol::NetworkSandboxPolicy; use crate::protocol::SandboxPolicy; #[cfg(target_os = "macos")] @@ -215,7 +214,6 @@ fn additional_permission_roots( ) } -#[cfg_attr(not(test), allow(dead_code))] fn merge_file_system_policy_with_additional_permissions( file_system_policy: &FileSystemSandboxPolicy, extra_reads: Vec, @@ -249,7 +247,6 @@ fn merge_file_system_policy_with_additional_permissions( } } } - fn merge_read_only_access_with_additional_reads( read_only_access: &ReadOnlyAccess, extra_reads: Vec, @@ -369,19 +366,10 @@ pub(crate) fn should_require_platform_sandbox( } match file_system_policy.kind { - FileSystemSandboxKind::Restricted => !file_system_policy.entries.iter().any(|entry| { - entry.access == FileSystemAccessMode::Write - && matches!( - &entry.path, - FileSystemPath::Special { value } - if value.kind == FileSystemSpecialPathKind::Root - && value.subpath.is_none() - ) - }), + FileSystemSandboxKind::Restricted => !file_system_policy.has_full_disk_write_access(), FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => false, } } - #[derive(Default)] pub struct SandboxManager; @@ -463,13 +451,11 @@ impl SandboxManager { if extra_reads.is_empty() && extra_writes.is_empty() { file_system_policy.clone() } else { - match file_system_policy.kind { - FileSystemSandboxKind::Restricted => { - FileSystemSandboxPolicy::from(&effective_policy) - } - FileSystemSandboxKind::Unrestricted - | FileSystemSandboxKind::ExternalSandbox => file_system_policy.clone(), - } + merge_file_system_policy_with_additional_permissions( + file_system_policy, + extra_reads, + extra_writes, + ) }; let network_sandbox_policy = NetworkSandboxPolicy::from(&effective_policy); (file_system_sandbox_policy, network_sandbox_policy) From a8119dae5fe8278e43203dc4db460deac6966494 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 7/8] protocol: keep root carveouts sandboxed --- codex-rs/core/src/sandboxing/mod.rs | 29 ++++++ codex-rs/protocol/src/protocol.rs | 144 +++++++++++++++++++++++----- 2 files changed, 151 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 218cbbac63..2982903b12 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -673,6 +673,35 @@ mod tests { ); } + #[test] + fn root_write_policy_with_carveouts_still_uses_platform_sandbox() { + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::CurrentWorkingDirectory, + subpath: Some("blocked".into()), + }, + }, + access: FileSystemAccessMode::None, + }, + ]); + + assert_eq!( + should_require_platform_sandbox(&policy, NetworkSandboxPolicy::Enabled, false), + true + ); + } + #[test] fn full_access_restricted_policy_still_uses_platform_sandbox_for_restricted_network() { let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4f33d7f631..dffa6e95a2 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -651,6 +651,25 @@ impl Default for FileSystemSandboxPolicy { } impl FileSystemSandboxPolicy { + fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool { + matches!(self.kind, FileSystemSandboxKind::Restricted) + && self.entries.iter().any(|entry| { + matches!( + &entry.path, + FileSystemPath::Special { value } + if value.kind == FileSystemSpecialPathKind::Root && predicate(entry.access) + ) + }) + } + + fn has_explicit_deny_entries(&self) -> bool { + matches!(self.kind, FileSystemSandboxKind::Restricted) + && self + .entries + .iter() + .any(|entry| entry.access == FileSystemAccessMode::None) + } + pub fn unrestricted() -> Self { Self { kind: FileSystemSandboxKind::Unrestricted, @@ -676,13 +695,10 @@ impl FileSystemSandboxPolicy { pub fn has_full_disk_read_access(&self) -> bool { match self.kind { FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true, - FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| { - matches!( - &entry.path, - FileSystemPath::Special { value } - if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_read() - ) - }), + FileSystemSandboxKind::Restricted => { + self.has_root_access(FileSystemAccessMode::can_read) + && !self.has_explicit_deny_entries() + } } } @@ -690,13 +706,10 @@ impl FileSystemSandboxPolicy { pub fn has_full_disk_write_access(&self) -> bool { match self.kind { FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => true, - FileSystemSandboxKind::Restricted => self.entries.iter().any(|entry| { - matches!( - &entry.path, - FileSystemPath::Special { value } - if value.kind == FileSystemSpecialPathKind::Root && entry.access.can_write() - ) - }), + FileSystemSandboxKind::Restricted => { + self.has_root_access(FileSystemAccessMode::can_write) + && !self.has_explicit_deny_entries() + } } } @@ -721,11 +734,24 @@ impl FileSystemSandboxPolicy { } let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); + let mut readable_roots = Vec::new(); + if self.has_root_access(FileSystemAccessMode::can_read) + && let Some(cwd_absolute) = cwd_absolute.as_ref() + { + readable_roots.push(absolute_root_path_for_cwd(cwd_absolute)); + } + dedup_absolute_paths( - self.entries - .iter() - .filter(|entry| entry.access.can_read()) - .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref())) + readable_roots + .into_iter() + .chain( + self.entries + .iter() + .filter(|entry| entry.access.can_read()) + .filter_map(|entry| { + resolve_file_system_path(&entry.path, cwd_absolute.as_ref()) + }), + ) .collect(), ) } @@ -739,11 +765,24 @@ impl FileSystemSandboxPolicy { let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); let unreadable_roots = self.get_unreadable_roots_with_cwd(cwd); + let mut writable_roots = Vec::new(); + if self.has_root_access(FileSystemAccessMode::can_write) + && let Some(cwd_absolute) = cwd_absolute.as_ref() + { + writable_roots.push(absolute_root_path_for_cwd(cwd_absolute)); + } + dedup_absolute_paths( - self.entries - .iter() - .filter(|entry| entry.access.can_write()) - .filter_map(|entry| resolve_file_system_path(&entry.path, cwd_absolute.as_ref())) + writable_roots + .into_iter() + .chain( + self.entries + .iter() + .filter(|entry| entry.access.can_write()) + .filter_map(|entry| { + resolve_file_system_path(&entry.path, cwd_absolute.as_ref()) + }), + ) .collect(), ) .into_iter() @@ -1441,6 +1480,16 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { } } +fn absolute_root_path_for_cwd(cwd: &AbsolutePathBuf) -> AbsolutePathBuf { + let root = cwd + .as_path() + .ancestors() + .last() + .unwrap_or_else(|| panic!("cwd must have a filesystem root")); + AbsolutePathBuf::from_absolute_path(root) + .unwrap_or_else(|err| panic!("cwd root must be an absolute path: {err}")) +} + fn resolve_file_system_path( path: &FileSystemPath, cwd: Option<&AbsolutePathBuf>, @@ -3880,6 +3929,57 @@ mod tests { assert!(writable.has_full_disk_write_access()); } + #[test] + fn restricted_file_system_policy_treats_root_with_carveouts_as_scoped_access() { + let cwd = TempDir::new().expect("tempdir"); + let cwd_absolute = + AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute tempdir"); + let root = absolute_root_path_for_cwd(&cwd_absolute); + let blocked = AbsolutePathBuf::resolve_path_against_base("blocked", cwd.path()) + .expect("resolve blocked"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::Root, + subpath: None, + }, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath { + kind: FileSystemSpecialPathKind::CurrentWorkingDirectory, + subpath: Some(PathBuf::from("blocked")), + }, + }, + access: FileSystemAccessMode::None, + }, + ]); + + assert!(!policy.has_full_disk_read_access()); + assert!(!policy.has_full_disk_write_access()); + assert_eq!( + policy.get_readable_roots_with_cwd(cwd.path()), + vec![root.clone()] + ); + assert_eq!( + policy.get_unreadable_roots_with_cwd(cwd.path()), + vec![blocked.clone()] + ); + + let writable_roots = policy.get_writable_roots_with_cwd(cwd.path()); + assert_eq!(writable_roots.len(), 1); + assert_eq!(writable_roots[0].root, root); + assert!( + writable_roots[0] + .read_only_subpaths + .iter() + .any(|path| path.as_path() == blocked.as_path()) + ); + } + #[test] fn restricted_file_system_policy_derives_effective_paths() { let cwd = TempDir::new().expect("tempdir"); From bfc71971a889696a7905ffc26b85748169fed44d Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 12:26:59 -0800 Subject: [PATCH 8/8] linux-sandbox: honor split filesystem policies in bwrap --- codex-rs/linux-sandbox/src/bwrap.rs | 83 +++++++++++-- codex-rs/linux-sandbox/src/linux_run_main.rs | 39 +++++-- .../linux-sandbox/src/linux_run_main_tests.rs | 15 ++- .../linux-sandbox/tests/suite/landlock.rs | 109 +++++++++++++++--- 4 files changed, 204 insertions(+), 42 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 783c3a428c..c6969a7d0c 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; use codex_core::error::CodexErr; use codex_core::error::Result; +use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::WritableRoot; @@ -89,7 +90,17 @@ pub(crate) fn create_bwrap_command_args( cwd: &Path, options: BwrapOptions, ) -> Result> { - if sandbox_policy.has_full_disk_write_access() { + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(sandbox_policy); + create_bwrap_command_args_for_policy(command, &file_system_sandbox_policy, cwd, options) +} + +pub(crate) fn create_bwrap_command_args_for_policy( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, + options: BwrapOptions, +) -> Result> { + if file_system_sandbox_policy.has_full_disk_write_access() { return if options.network_mode == BwrapNetworkMode::FullAccess { Ok(command) } else { @@ -97,7 +108,7 @@ pub(crate) fn create_bwrap_command_args( }; } - create_bwrap_flags(command, sandbox_policy, cwd, options) + create_bwrap_flags(command, file_system_sandbox_policy, cwd, options) } fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOptions) -> Vec { @@ -127,14 +138,14 @@ fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOption /// Build the bubblewrap flags (everything after `argv[0]`). fn create_bwrap_flags( command: Vec, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, options: BwrapOptions, ) -> Result> { let mut args = Vec::new(); args.push("--new-session".to_string()); args.push("--die-with-parent".to_string()); - args.extend(create_filesystem_args(sandbox_policy, cwd)?); + args.extend(create_filesystem_args(file_system_sandbox_policy, cwd)?); // Request a user namespace explicitly rather than relying on bubblewrap's // auto-enable behavior, which is skipped when the caller runs as uid 0. args.push("--unshare-user".to_string()); @@ -153,7 +164,7 @@ fn create_bwrap_flags( Ok(args) } -/// Build the bubblewrap filesystem mounts for a given sandbox policy. +/// Build the bubblewrap filesystem mounts for a given filesystem policy. /// /// The mount order is important: /// 1. Full-read policies use `--ro-bind / /`; restricted-read policies start @@ -164,11 +175,14 @@ fn create_bwrap_flags( /// writable subpaths under `/dev` (for example, `/dev/shm`). /// 4. `--ro-bind ` re-applies read-only protections under /// those writable roots so protected subpaths win. -fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result> { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); +fn create_filesystem_args( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, +) -> Result> { + let writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(cwd); ensure_mount_targets_exist(&writable_roots)?; - let mut args = if sandbox_policy.has_full_disk_read_access() { + let mut args = if file_system_sandbox_policy.has_full_disk_read_access() { // Read-only root, then mount a minimal device tree. // In bubblewrap (`bubblewrap.c`, `SETUP_MOUNT_DEV`), `--dev /dev` // creates the standard minimal nodes: null, zero, full, random, @@ -191,12 +205,12 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result< "/dev".to_string(), ]; - let mut readable_roots: BTreeSet = sandbox_policy + let mut readable_roots: BTreeSet = file_system_sandbox_policy .get_readable_roots_with_cwd(cwd) .into_iter() .map(PathBuf::from) .collect(); - if sandbox_policy.include_platform_defaults() { + if file_system_sandbox_policy.include_platform_defaults() { readable_roots.extend( LINUX_PLATFORM_DEFAULT_READ_ROOTS .iter() @@ -386,6 +400,12 @@ fn find_first_non_existent_component(target_path: &Path) -> Option { #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::FileSystemAccessMode; + use codex_protocol::protocol::FileSystemPath; + use codex_protocol::protocol::FileSystemSandboxEntry; + use codex_protocol::protocol::FileSystemSandboxPolicy; + use codex_protocol::protocol::FileSystemSpecialPath; + use codex_protocol::protocol::FileSystemSpecialPathKind; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -528,4 +548,47 @@ mod tests { ); } } + + #[test] + fn split_policy_reapplies_unreadable_carveouts_after_writable_binds() { + let temp_dir = TempDir::new().expect("temp dir"); + let writable_root = temp_dir.path().join("workspace"); + let blocked = writable_root.join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked dir"); + let writable_root = + AbsolutePathBuf::from_absolute_path(&writable_root).expect("absolute writable root"); + let blocked = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked dir"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: writable_root.clone(), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: blocked.clone(), + }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let writable_root_str = path_to_string(writable_root.as_path()); + let blocked_str = path_to_string(blocked.as_path()); + + assert!(args.windows(3).any(|window| { + window + == [ + "--bind", + writable_root_str.as_str(), + writable_root_str.as_str(), + ] + })); + assert!( + args.windows(3).any(|window| { + window == ["--ro-bind", blocked_str.as_str(), blocked_str.as_str()] + }) + ); + } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 6a624ac1dd..b33f933e6b 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; -use crate::bwrap::create_bwrap_command_args; +use crate::bwrap::create_bwrap_command_args_for_policy; use crate::landlock::apply_sandbox_policy_to_current_thread; use crate::proxy_routing::activate_proxy_routes_in_netns; use crate::proxy_routing::prepare_host_proxy_route_spec; @@ -178,7 +178,7 @@ pub fn run_main() -> ! { ); run_bwrap_with_proc_fallback( &sandbox_policy_cwd, - &sandbox_policy, + &file_system_sandbox_policy, network_sandbox_policy, inner, !no_proc, @@ -255,7 +255,7 @@ fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_bwrap_san fn run_bwrap_with_proc_fallback( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, inner: Vec, mount_proc: bool, @@ -264,7 +264,12 @@ fn run_bwrap_with_proc_fallback( let network_mode = bwrap_network_mode(network_sandbox_policy, allow_network_for_proxy); let mut mount_proc = mount_proc; - if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy, network_mode) + if mount_proc + && !preflight_proc_mount_support( + sandbox_policy_cwd, + file_system_sandbox_policy, + network_mode, + ) { eprintln!("codex-linux-sandbox: bwrap could not mount /proc; retrying with --no-proc"); mount_proc = false; @@ -274,7 +279,12 @@ fn run_bwrap_with_proc_fallback( mount_proc, network_mode, }; - let argv = build_bwrap_argv(inner, sandbox_policy, sandbox_policy_cwd, options); + let argv = build_bwrap_argv( + inner, + file_system_sandbox_policy, + sandbox_policy_cwd, + options, + ); exec_vendored_bwrap(argv); } @@ -293,12 +303,17 @@ fn bwrap_network_mode( fn build_bwrap_argv( inner: Vec, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, sandbox_policy_cwd: &Path, options: BwrapOptions, ) -> Vec { - let mut args = create_bwrap_command_args(inner, sandbox_policy, sandbox_policy_cwd, options) - .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); + let mut args = create_bwrap_command_args_for_policy( + inner, + file_system_sandbox_policy, + sandbox_policy_cwd, + options, + ) + .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); let command_separator_index = args .iter() @@ -316,24 +331,24 @@ fn build_bwrap_argv( fn preflight_proc_mount_support( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_mode: BwrapNetworkMode, ) -> bool { let preflight_argv = - build_preflight_bwrap_argv(sandbox_policy_cwd, sandbox_policy, network_mode); + build_preflight_bwrap_argv(sandbox_policy_cwd, file_system_sandbox_policy, network_mode); let stderr = run_bwrap_in_child_capture_stderr(preflight_argv); !is_proc_mount_failure(stderr.as_str()) } fn build_preflight_bwrap_argv( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_mode: BwrapNetworkMode, ) -> Vec { let preflight_command = vec![resolve_true_command()]; build_bwrap_argv( preflight_command, - sandbox_policy, + file_system_sandbox_policy, sandbox_policy_cwd, BwrapOptions { mount_proc: true, diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index c466de5e27..6be9d35c03 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -35,9 +35,10 @@ fn ignores_non_proc_mount_errors() { #[test] fn inserts_bwrap_argv0_before_command_separator() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -69,9 +70,10 @@ fn inserts_bwrap_argv0_before_command_separator() { #[test] fn inserts_unshare_net_when_network_isolation_requested() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -83,9 +85,10 @@ fn inserts_unshare_net_when_network_isolation_requested() { #[test] fn inserts_unshare_net_when_proxy_only_network_mode_requested() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -104,7 +107,11 @@ fn proxy_only_mode_takes_precedence_over_full_network_policy() { #[test] fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true); - let argv = build_preflight_bwrap_argv(Path::new("/"), &SandboxPolicy::DangerFullAccess, mode); + let argv = build_preflight_bwrap_argv( + Path::new("/"), + &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + mode, + ); assert!(argv.iter().any(|arg| arg == "--")); } diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index b12170584c..d303cbd6d3 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -9,6 +9,9 @@ use codex_core::exec::process_exec_tool_call; use codex_core::exec_env::create_env; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::protocol::FileSystemAccessMode; +use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSandboxEntry; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; @@ -71,20 +74,6 @@ async fn run_cmd_result_with_writable_roots( use_bwrap_sandbox: bool, network_access: bool, ) -> Result { - let cwd = std::env::current_dir().expect("cwd should exist"); - let sandbox_cwd = cwd.clone(); - let params = ExecParams { - command: cmd.iter().copied().map(str::to_owned).collect(), - cwd, - expiration: timeout_ms.into(), - env: create_env_from_core_vars(), - network: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - justification: None, - arg0: None, - }; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots .iter() @@ -98,14 +87,49 @@ async fn run_cmd_result_with_writable_roots( exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); + let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); + run_cmd_result_with_policies( + cmd, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + timeout_ms, + use_bwrap_sandbox, + ) + .await +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_policies( + cmd: &[&str], + sandbox_policy: SandboxPolicy, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + timeout_ms: u64, + use_bwrap_sandbox: bool, +) -> Result { + let cwd = std::env::current_dir().expect("cwd should exist"); + let sandbox_cwd = cwd.clone(); + let params = ExecParams { + command: cmd.iter().copied().map(str::to_owned).collect(), + cwd, + expiration: timeout_ms.into(), + env: create_env_from_core_vars(), + network: None, + sandbox_permissions: SandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + justification: None, + arg0: None, + }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); process_exec_tool_call( params, &sandbox_policy, - &FileSystemSandboxPolicy::from(&sandbox_policy), - NetworkSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, + network_sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, use_bwrap_sandbox, @@ -479,6 +503,59 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { assert_ne!(codex_output.exit_code, 0); } +#[tokio::test] +async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let blocked = tmpdir.path().join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked dir"); + let blocked_target = blocked.join("secret.txt"); + + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], + read_only_access: Default::default(), + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + }, + access: FileSystemAccessMode::None, + }, + ]); + let output = expect_denied( + run_cmd_result_with_policies( + &[ + "bash", + "-lc", + &format!("echo denied > {}", blocked_target.to_string_lossy()), + ], + sandbox_policy, + file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + LONG_TIMEOUT_MS, + true, + ) + .await, + "explicit split-policy carveout should be denied under bubblewrap", + ); + + assert_ne!(output.exit_code, 0); +} + #[tokio::test] async fn sandbox_blocks_ssh() { // Force ssh to attempt a real TCP connection but fail quickly. `BatchMode`