From a065a40c382673910436483dd70966c14a6bddc4 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 15:39:20 -0800 Subject: [PATCH 1/9] 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 | 142 ++++- codex-rs/core/src/landlock.rs | 3 +- codex-rs/core/src/sandboxing/mod.rs | 500 ++++++++++++++++-- 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 | 15 +- 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, 768 insertions(+), 80 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 7898e2ffbbdf..366eb1ced50f 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::permissions::FileSystemSandboxPolicy::from(&policy); + let network_sandbox_policy = + codex_protocol::permissions::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 bd32dfe02965..8d7e7b72e939 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -87,6 +87,8 @@ use codex_protocol::models::BaseInstructions; use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; use codex_protocol::openai_models::ModelInfo; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::HasLegacyEvent; use codex_protocol::protocol::ItemCompletedEvent; @@ -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 cb44d1cada36..31d9c7552008 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 8779b2e1c302..7319b7740ba6 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -19,7 +19,6 @@ use tokio_util::sync::CancellationToken; use crate::error::CodexErr; use crate::error::Result; use crate::error::SandboxErr; -use crate::get_platform_sandbox; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecCommandOutputDeltaEvent; @@ -33,7 +32,11 @@ use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use crate::text_encoding::bytes_to_string_smart; +use crate::tools::sandboxing::SandboxablePreference; use codex_network_proxy::NetworkProxy; +use codex_protocol::permissions::FileSystemSandboxKind; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_pty::process_group::kill_child_process_group; pub const DEFAULT_EXEC_COMMAND_TIMEOUT_MS: u64 = 10_000; @@ -72,6 +75,21 @@ pub struct ExecParams { pub arg0: Option, } +fn select_process_exec_tool_sandbox_type( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, + enforce_managed_network: bool, +) -> SandboxType { + SandboxManager::new().select_initial( + file_system_sandbox_policy, + network_sandbox_policy, + SandboxablePreference::Auto, + windows_sandbox_level, + enforce_managed_network, + ) +} + /// Mechanism to terminate an exec invocation before it finishes naturally. #[derive(Clone, Debug)] pub enum ExecExpiration { @@ -149,9 +167,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,23 +180,12 @@ 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 { .. } => { - 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( - windows_sandbox_level != codex_protocol::config_types::WindowsSandboxLevel::Disabled, - ) - .unwrap_or(SandboxType::None), - }; + let sandbox_type = select_process_exec_tool_sandbox_type( + file_system_sandbox_policy, + network_sandbox_policy, + windows_sandbox_level, + enforce_managed_network, + ); tracing::debug!("Sandbox type: {sandbox_type:?}"); let ExecParams { @@ -215,6 +225,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 +259,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 +279,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 +717,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 +756,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 +771,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 +1137,53 @@ 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 + ); + } + + #[test] + fn process_exec_tool_call_uses_platform_sandbox_for_network_only_restrictions() { + let expected = crate::get_platform_sandbox(false).unwrap_or(SandboxType::None); + + assert_eq!( + select_process_exec_tool_sandbox_type( + &FileSystemSandboxPolicy::unrestricted(), + NetworkSandboxPolicy::Restricted, + codex_protocol::config_types::WindowsSandboxLevel::Disabled, + false, + ), + expected + ); + } + #[cfg(unix)] #[test] fn sandbox_detection_flags_sigsys_exit_code() { @@ -1140,6 +1226,8 @@ mod tests { params, SandboxType::None, &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&SandboxPolicy::new_read_only_policy()), + NetworkSandboxPolicy::Restricted, None, None, ) @@ -1196,6 +1284,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 65b2a68073de..fdb8a52af1d6 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -3,6 +3,7 @@ use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; +use codex_protocol::permissions::NetworkSandboxPolicy; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -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 9258889c79dd..7c204f4362cd 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -30,6 +30,14 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; +use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::FileSystemSandboxKind; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::FileSystemSpecialPath; +use codex_protocol::permissions::NetworkSandboxPolicy; +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,35 @@ 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 matches!(value, FileSystemSpecialPath::Root) + ) + }), + FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => false, + } +} + #[derive(Default)] pub struct SandboxManager; @@ -307,7 +391,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 +407,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 +431,8 @@ impl SandboxManager { let SandboxTransformRequest { mut spec, policy, + file_system_policy, + network_policy, sandbox, enforce_managed_network, network, @@ -360,16 +445,41 @@ 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 { + merge_file_system_policy_with_additional_permissions( + file_system_policy, + extra_reads, + extra_writes, + ) + }; + let network_sandbox_policy = + if merge_network_access(network_policy.is_enabled(), &additional_permissions) { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }; + (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 +498,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 +517,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 +552,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 +587,12 @@ 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::NetworkAccess; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::tools::sandboxing::SandboxablePreference; @@ -493,16 +606,24 @@ mod tests { use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; + use codex_protocol::permissions::FileSystemSpecialPath; + use codex_protocol::permissions::NetworkSandboxPolicy; 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 +636,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 +645,98 @@ 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::Root, + }, + 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::Root, + }, + 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::Root, + }, + 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 +838,6 @@ mod tests { } ); } - #[cfg(target_os = "macos")] #[test] fn effective_permissions_merge_macos_extensions_with_additional_permissions() { @@ -679,4 +892,223 @@ 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 transform_additional_permissions_preserves_denied_entries() { + let manager = SandboxManager::new(); + let cwd = std::env::current_dir().expect("current dir"); + let temp_dir = TempDir::new().expect("create temp dir"); + let workspace_root = AbsolutePathBuf::from_absolute_path( + canonicalize(temp_dir.path()).expect("canonicalize temp dir"), + ) + .expect("absolute temp dir"); + let allowed_path = workspace_root.join("allowed").expect("allowed path"); + let denied_path = workspace_root.join("denied").expect("denied path"); + 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 { + file_system: Some(FileSystemPermissions { + read: None, + write: Some(vec![allowed_path.clone()]), + }), + ..Default::default() + }), + justification: None, + }, + policy: &SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + network_access: false, + }, + file_system_policy: &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: denied_path.clone(), + }, + access: FileSystemAccessMode::None, + }, + ]), + 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::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: denied_path }, + access: FileSystemAccessMode::None, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { path: allowed_path }, + access: FileSystemAccessMode::Write, + }, + ]) + ); + assert_eq!( + exec_request.network_sandbox_policy, + NetworkSandboxPolicy::Restricted + ); + } + + #[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::Root, + }, + 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 8d556a8eec53..760c6bd8ae95 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -22,6 +22,7 @@ use crate::spawn::CODEX_SANDBOX_ENV_VAR; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; +use codex_protocol::permissions::NetworkSandboxPolicy; const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); const MACOS_SEATBELT_NETWORK_POLICY: &str = include_str!("seatbelt_network_policy.sbpl"); @@ -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 67e6ace04472..575a2f777cd7 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 codex_protocol::permissions::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 2f77d9fceed4..39c89e36e5de 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -36,6 +36,8 @@ use super::SessionTaskContext; use crate::codex::Session; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; const USER_SHELL_TIMEOUT_MS: u64 = 60 * 60 * 1000; // 1 hour @@ -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 42016ff9bf9a..f9cc3d854b03 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 f66c79bbb02c..7bed13ce4fcc 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 e4f7c80ab743..0a0ae50007ce 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -24,6 +24,8 @@ use codex_execpolicy::RuleMatch; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::RejectConfig; @@ -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 5d7b0f5e04fa..4d9a65d5ddaf 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 @@ -31,6 +31,10 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsPreferencesPermission; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::PermissionProfile; +#[cfg(target_os = "macos")] +use codex_protocol::permissions::FileSystemSandboxPolicy; +#[cfg(target_os = "macos")] +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SkillScope; use codex_shell_escalation::EscalationExecution; use codex_shell_escalation::EscalationPermissions; @@ -474,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, @@ -524,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, @@ -588,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 28d87b5bf3c9..be8a0363e750 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -17,6 +17,8 @@ use crate::tools::network_approval::NetworkApprovalSpec; use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use futures::Future; @@ -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 6323857150ef..2e46d6a94321 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 = + codex_protocol::permissions::FileSystemSandboxPolicy::from(turn.sandbox_policy.get()); + turn.network_sandbox_policy = + codex_protocol::permissions::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 654660068172..e00ec963df14 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::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::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 362fcaf35fd5..76021081454b 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::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::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 8a70b181dca0506aab14e6ad0e420fe1c8c9c800 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 16:23:58 -0800 Subject: [PATCH 2/9] protocol: derive effective file access from filesystem policies --- codex-rs/protocol/src/permissions.rs | 222 +++++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 188 ++++++++++++++++++----- 2 files changed, 370 insertions(+), 40 deletions(-) diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index baf17e0ce2ee..c5aabea6ba53 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -1,4 +1,5 @@ use std::collections::HashSet; +use std::ffi::OsStr; use std::io; use std::path::Path; use std::path::PathBuf; @@ -8,11 +9,13 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; use strum_macros::Display; +use tracing::error; use ts_rs::TS; use crate::protocol::NetworkAccess; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; +use crate::protocol::WritableRoot; #[derive( Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS, @@ -141,6 +144,114 @@ 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 matches!(value, FileSystemSpecialPath::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 matches!(value, FileSystemSpecialPath::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 matches!(value, FileSystemSpecialPath::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, @@ -422,6 +533,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>, @@ -471,3 +592,104 @@ 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")) +} + +fn resolve_gitdir_from_file(dot_git: &AbsolutePathBuf) -> Option { + let contents = match std::fs::read_to_string(dot_git.as_path()) { + Ok(contents) => contents, + Err(err) => { + error!( + "Failed to read {path} for gitdir pointer: {err}", + path = dot_git.as_path().display() + ); + return None; + } + }; + + let trimmed = contents.trim(); + let (_, gitdir_raw) = match trimmed.split_once(':') { + Some(parts) => parts, + None => { + error!( + "Expected {path} to contain a gitdir pointer, but it did not match `gitdir: `.", + path = dot_git.as_path().display() + ); + return None; + } + }; + let gitdir_raw = gitdir_raw.trim(); + if gitdir_raw.is_empty() { + error!( + "Expected {path} to contain a gitdir pointer, but it was empty.", + path = dot_git.as_path().display() + ); + return None; + } + let base = match dot_git.as_path().parent() { + Some(base) => base, + None => { + error!( + "Unable to resolve parent directory for {path}.", + path = dot_git.as_path().display() + ); + return None; + } + }; + let gitdir_path = match AbsolutePathBuf::resolve_path_against_base(gitdir_raw, base) { + Ok(path) => path, + Err(err) => { + error!( + "Failed to resolve gitdir path {gitdir_raw} from {path}: {err}", + path = dot_git.as_path().display() + ); + return None; + } + }; + if !gitdir_path.as_path().exists() { + error!( + "Resolved gitdir path {path} does not exist.", + path = gitdir_path.as_path().display() + ); + return None; + } + Some(gitdir_path) +} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 02776ff24018..32d62677360a 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -61,6 +61,13 @@ pub use crate::approvals::NetworkApprovalContext; pub use crate::approvals::NetworkApprovalProtocol; pub use crate::approvals::NetworkPolicyAmendment; pub use crate::approvals::NetworkPolicyRuleAction; +pub use crate::permissions::FileSystemAccessMode; +pub use crate::permissions::FileSystemPath; +pub use crate::permissions::FileSystemSandboxEntry; +pub use crate::permissions::FileSystemSandboxKind; +pub use crate::permissions::FileSystemSandboxPolicy; +pub use crate::permissions::FileSystemSpecialPath; +pub use crate::permissions::NetworkSandboxPolicy; pub use crate::request_user_input::RequestUserInputEvent; /// Open/close tags for special user-input blocks. Used across crates to avoid @@ -542,7 +549,6 @@ impl NetworkAccess { matches!(self, NetworkAccess::Enabled) } } - fn default_include_platform_defaults() -> bool { true } @@ -883,45 +889,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() } @@ -929,6 +901,49 @@ impl SandboxPolicy { } } +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); + } + } + + let mut deduped = Vec::with_capacity(subpaths.len()); + let mut seen = HashSet::new(); + for path in subpaths { + if seen.insert(path.to_path_buf()) { + deduped.push(path); + } + } + deduped +} + fn is_git_pointer_file(path: &AbsolutePathBuf) -> bool { path.as_path().is_file() && path.as_path().file_name() == Some(OsStr::new(".git")) } @@ -3156,11 +3171,13 @@ mod tests { use crate::permissions::FileSystemPath; use crate::permissions::FileSystemSandboxEntry; use crate::permissions::FileSystemSandboxPolicy; + use crate::permissions::FileSystemSpecialPath; use crate::permissions::NetworkSandboxPolicy; use anyhow::Result; use pretty_assertions::assert_eq; use serde_json::json; use tempfile::NamedTempFile; + use tempfile::TempDir; #[test] fn external_sandbox_reports_full_access_flags() { @@ -3241,6 +3258,97 @@ 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::Root, + }, + 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::Root, + }, + 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::Minimal, + }, + access: FileSystemAccessMode::Read, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: secret.clone(), + }, + 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 fd4299609acbf5b7b21c2f03260f60f86d5f07fc Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 16:23:58 -0800 Subject: [PATCH 3/9] safety: honor filesystem policy carveouts in apply_patch --- codex-rs/core/src/apply_patch.rs | 1 + codex-rs/core/src/safety.rs | 88 ++++++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 0b9cca1c9d66..1928a956215a 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 350e7dad0f2f..4c674de08db5 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,10 @@ 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::RejectConfig; use codex_utils_absolute_path::AbsolutePathBuf; use tempfile::TempDir; @@ -223,13 +232,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 +253,7 @@ mod tests { }; assert!(is_write_patch_constrained_to_writable_paths( &add_outside, - &policy_with_parent, + &FileSystemSandboxPolicy::from(&policy_with_parent), &cwd, )); } @@ -264,6 +273,7 @@ mod tests { &add_inside, AskForApproval::OnRequest, &policy, + &FileSystemSandboxPolicy::from(&policy), &cwd, WindowsSandboxLevel::Disabled ), @@ -294,6 +304,7 @@ mod tests { &add_outside, AskForApproval::OnRequest, &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -308,6 +319,7 @@ mod tests { mcp_elicitations: false, }), &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -339,6 +351,7 @@ mod tests { mcp_elicitations: false, }), &policy_workspace_only, + &FileSystemSandboxPolicy::from(&policy_workspace_only), &cwd, WindowsSandboxLevel::Disabled, ), @@ -348,4 +361,47 @@ 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 blocked_absolute = AbsolutePathBuf::from_absolute_path(blocked_path.clone()).unwrap(); + 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::Root, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: blocked_absolute, + }, + 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 e24e36ec77517d67c6ea1d45f19fc443d066b7b2 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 16:23:58 -0800 Subject: [PATCH 4/9] seatbelt: honor split filesystem sandbox policies --- codex-rs/core/src/sandboxing/mod.rs | 7 +- codex-rs/core/src/seatbelt.rs | 468 +++++++++++++++++++------ codex-rs/core/src/tools/js_repl/mod.rs | 22 +- 3 files changed, 373 insertions(+), 124 deletions(-) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 7c204f4362cd..49ecef923ad7 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -19,7 +19,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; @@ -496,9 +496,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 760c6bd8ae95..29f73a1c2df8 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -22,6 +22,7 @@ use crate::spawn::CODEX_SANDBOX_ENV_VAR; use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; +use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; const MACOS_SEATBELT_BASE_POLICY: &str = include_str!("seatbelt_base_policy.sbpl"); @@ -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, + &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, - sandbox_policy, + 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); - } +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 { + build_seatbelt_access_policy( + "file-write*", + "WRITABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots.clone(), + }], + ) } + } else { + 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(), + ) + }; - if writable_folder_policies.is_empty() { - ("".to_string(), Vec::new()) + 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 file_write_policy = format!( - "(allow file-write*\n{}\n)", - writable_folder_policies.join(" ") + let (policy, params) = build_seatbelt_access_policy( + "file-read*", + "READABLE_ROOT", + vec![SeatbeltAccessRoot { + root: root_absolute_path(), + excluded_subpaths: unreadable_roots, + }], ); - (file_write_policy, file_write_params) + ( + format!("; allow read-only file operations\n{policy}"), + params, + ) } - } - }; - - 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, - ) - } - }; + 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,6 +627,7 @@ 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; @@ -504,6 +639,11 @@ mod tests { use crate::seatbelt_permissions::MacOsAutomationPermission; use crate::seatbelt_permissions::MacOsPreferencesPermission; use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; + use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; @@ -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,93 @@ 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::Root, + }, + 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 +1227,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 +1240,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={}", + cwd.canonicalize() + .expect("canonicalize cwd") + .to_string_lossy() + ), + format!( + "-DWRITABLE_ROOT_1={}", vulnerable_root_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_0_RO_0={}", + "-DWRITABLE_ROOT_1_RO_0={}", dot_git_canonical.to_string_lossy() ), format!( - "-DWRITABLE_ROOT_0_RO_1={}", + "-DWRITABLE_ROOT_1_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() + 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 f9cc3d854b03..bc2a5342cea1 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 30aae07ce3a292e1ab83a233a0798706c4a40fee Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 16:23:58 -0800 Subject: [PATCH 5/9] 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 | 96 ++++++++++++++- codex-rs/protocol/src/protocol.rs | 16 +++ 6 files changed, 312 insertions(+), 46 deletions(-) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index fdb8a52af1d6..ff67c057c8a1 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -3,6 +3,7 @@ use crate::spawn::SpawnChildRequest; use crate::spawn::StdioPolicy; use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; +use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use std::collections::HashMap; use std::path::Path; @@ -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 49ecef923ad7..69ab438e1771 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::SandboxPolicy; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; @@ -516,9 +516,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 f9477a1264fd..a120ab70546a 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 c9fdd06a326a..6a624ac1dd30 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 cda509030604..c466de5e2783 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()), @@ -117,11 +126,32 @@ fn managed_proxy_inner_command_includes_route_spec() { assert!(args.iter().any(|arg| arg == "{\"routes\":[]}")); } +#[test] +fn inner_command_includes_split_policy_flags() { + 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, + 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"), - &SandboxPolicy::new_read_only_policy(), + &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 32d62677360a..bf85f30bf4f7 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -727,6 +727,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 5b5f1a313c4c65fc2e37aa3c1aac45f54e7c6b73 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 7 Mar 2026 17:06:34 -0800 Subject: [PATCH 6/9] fix(linux-sandbox): update split network test expectations --- codex-rs/linux-sandbox/src/landlock.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/codex-rs/linux-sandbox/src/landlock.rs b/codex-rs/linux-sandbox/src/landlock.rs index a120ab70546a..307f956a4d4e 100644 --- a/codex-rs/linux-sandbox/src/landlock.rs +++ b/codex-rs/linux-sandbox/src/landlock.rs @@ -268,13 +268,13 @@ mod tests { use super::NetworkSeccompMode; use super::network_seccomp_mode; use super::should_install_network_seccomp; - use codex_protocol::protocol::SandboxPolicy; + use codex_protocol::protocol::NetworkSandboxPolicy; use pretty_assertions::assert_eq; #[test] fn managed_network_enforces_seccomp_even_for_full_network_policy() { assert_eq!( - should_install_network_seccomp(&SandboxPolicy::DangerFullAccess, true), + should_install_network_seccomp(NetworkSandboxPolicy::Enabled, true), true ); } @@ -282,7 +282,7 @@ mod tests { #[test] fn full_network_policy_without_managed_network_skips_seccomp() { assert_eq!( - should_install_network_seccomp(&SandboxPolicy::DangerFullAccess, false), + should_install_network_seccomp(NetworkSandboxPolicy::Enabled, false), false ); } @@ -290,11 +290,11 @@ mod tests { #[test] fn restricted_network_policy_always_installs_seccomp() { assert!(should_install_network_seccomp( - &SandboxPolicy::new_read_only_policy(), + NetworkSandboxPolicy::Restricted, false )); assert!(should_install_network_seccomp( - &SandboxPolicy::new_read_only_policy(), + NetworkSandboxPolicy::Restricted, true )); } @@ -302,7 +302,7 @@ mod tests { #[test] fn managed_proxy_routes_use_proxy_routed_seccomp_mode() { assert_eq!( - network_seccomp_mode(&SandboxPolicy::DangerFullAccess, true, true), + network_seccomp_mode(NetworkSandboxPolicy::Enabled, true, true), Some(NetworkSeccompMode::ProxyRouted) ); } @@ -310,7 +310,7 @@ mod tests { #[test] fn restricted_network_without_proxy_routing_uses_restricted_mode() { assert_eq!( - network_seccomp_mode(&SandboxPolicy::new_read_only_policy(), false, false), + network_seccomp_mode(NetworkSandboxPolicy::Restricted, false, false), Some(NetworkSeccompMode::Restricted) ); } @@ -318,7 +318,7 @@ mod tests { #[test] fn full_network_without_managed_proxy_skips_network_seccomp_mode() { assert_eq!( - network_seccomp_mode(&SandboxPolicy::DangerFullAccess, false, false), + network_seccomp_mode(NetworkSandboxPolicy::Enabled, false, false), None ); } From 99d7568bd04396e399e9145c067fa10f3273685d Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 7 Mar 2026 17:20:41 -0800 Subject: [PATCH 7/9] refactor(linux-sandbox): simplify inner seccomp args --- codex-rs/linux-sandbox/src/linux_run_main.rs | 34 +++++--- .../linux-sandbox/src/linux_run_main_tests.rs | 82 +++++++++---------- 2 files changed, 64 insertions(+), 52 deletions(-) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 6a624ac1dd30..54a302a06a04 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -166,16 +166,16 @@ pub fn run_main() -> ! { } else { None }; - let inner = build_inner_seccomp_command( - &sandbox_policy_cwd, - &sandbox_policy, - &file_system_sandbox_policy, + let inner = build_inner_seccomp_command(InnerSeccompCommandArgs { + sandbox_policy_cwd: &sandbox_policy_cwd, + sandbox_policy: &sandbox_policy, + file_system_sandbox_policy: &file_system_sandbox_policy, network_sandbox_policy, use_bwrap_sandbox, allow_network_for_proxy, proxy_route_spec, command, - ); + }); run_bwrap_with_proc_fallback( &sandbox_policy_cwd, &sandbox_policy, @@ -437,17 +437,29 @@ fn is_proc_mount_failure(stderr: &str) -> bool { || stderr.contains("Permission denied")) } -/// Build the inner command that applies seccomp after bubblewrap. -fn build_inner_seccomp_command( - sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, - file_system_sandbox_policy: &FileSystemSandboxPolicy, +struct InnerSeccompCommandArgs<'a> { + sandbox_policy_cwd: &'a Path, + sandbox_policy: &'a SandboxPolicy, + file_system_sandbox_policy: &'a FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, use_bwrap_sandbox: bool, allow_network_for_proxy: bool, proxy_route_spec: Option, command: Vec, -) -> Vec { +} + +/// Build the inner command that applies seccomp after bubblewrap. +fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec { + let InnerSeccompCommandArgs { + sandbox_policy_cwd, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + use_bwrap_sandbox, + allow_network_for_proxy, + proxy_route_spec, + command, + } = args; let current_exe = match std::env::current_exe() { Ok(path) => path, Err(err) => panic!("failed to resolve current executable path: {err}"), 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 c466de5e2783..35c60f364f06 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -111,16 +111,16 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { #[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"), - &sandbox_policy, - &FileSystemSandboxPolicy::from(&sandbox_policy), - NetworkSandboxPolicy::Restricted, - true, - true, - Some("{\"routes\":[]}".to_string()), - vec!["/bin/true".to_string()], - ); + let args = build_inner_seccomp_command(InnerSeccompCommandArgs { + sandbox_policy_cwd: Path::new("/tmp"), + sandbox_policy: &sandbox_policy, + file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + use_bwrap_sandbox: true, + allow_network_for_proxy: true, + proxy_route_spec: Some("{\"routes\":[]}".to_string()), + command: vec!["/bin/true".to_string()], + }); assert!(args.iter().any(|arg| arg == "--proxy-route-spec")); assert!(args.iter().any(|arg| arg == "{\"routes\":[]}")); @@ -129,16 +129,16 @@ fn managed_proxy_inner_command_includes_route_spec() { #[test] fn inner_command_includes_split_policy_flags() { 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, - vec!["/bin/true".to_string()], - ); + let args = build_inner_seccomp_command(InnerSeccompCommandArgs { + sandbox_policy_cwd: Path::new("/tmp"), + sandbox_policy: &sandbox_policy, + file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + use_bwrap_sandbox: true, + allow_network_for_proxy: false, + proxy_route_spec: None, + command: vec!["/bin/true".to_string()], + }); assert!(args.iter().any(|arg| arg == "--file-system-sandbox-policy")); assert!(args.iter().any(|arg| arg == "--network-sandbox-policy")); @@ -147,16 +147,16 @@ fn inner_command_includes_split_policy_flags() { #[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, - vec!["/bin/true".to_string()], - ); + let args = build_inner_seccomp_command(InnerSeccompCommandArgs { + sandbox_policy_cwd: Path::new("/tmp"), + sandbox_policy: &sandbox_policy, + file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + use_bwrap_sandbox: true, + allow_network_for_proxy: false, + proxy_route_spec: None, + command: vec!["/bin/true".to_string()], + }); assert!(!args.iter().any(|arg| arg == "--proxy-route-spec")); } @@ -165,16 +165,16 @@ fn non_managed_inner_command_omits_route_spec() { 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"), - &sandbox_policy, - &FileSystemSandboxPolicy::from(&sandbox_policy), - NetworkSandboxPolicy::Restricted, - true, - true, - None, - vec!["/bin/true".to_string()], - ) + build_inner_seccomp_command(InnerSeccompCommandArgs { + sandbox_policy_cwd: Path::new("/tmp"), + sandbox_policy: &sandbox_policy, + file_system_sandbox_policy: &FileSystemSandboxPolicy::from(&sandbox_policy), + network_sandbox_policy: NetworkSandboxPolicy::Restricted, + use_bwrap_sandbox: true, + allow_network_for_proxy: true, + proxy_route_spec: None, + command: vec!["/bin/true".to_string()], + }) }); assert!(result.is_err()); } @@ -186,7 +186,7 @@ fn resolve_sandbox_policies_derives_split_policies_from_legacy_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.sandbox_policy, sandbox_policy); assert_eq!( resolved.file_system_sandbox_policy, FileSystemSandboxPolicy::from(&sandbox_policy) From a5583d4acd23d8ed973bac3ff4bd4b1f21fbbbea Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 7 Mar 2026 17:46:40 -0800 Subject: [PATCH 8/9] refactor(core): simplify linux sandbox panic paths --- codex-rs/core/src/landlock.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index ff67c057c8a1..736392efa2a6 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -80,15 +80,12 @@ pub(crate) fn create_linux_sandbox_command_args_for_policies( 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 sandbox_policy_json = serde_json::to_string(sandbox_policy) + .unwrap_or_else(|err| panic!("failed to serialize sandbox policy: {err}")); let file_system_policy_json = serde_json::to_string(file_system_sandbox_policy) - .expect("Failed to serialize FileSystemSandboxPolicy to JSON"); - #[expect(clippy::expect_used)] + .unwrap_or_else(|err| panic!("failed to serialize filesystem sandbox policy: {err}")); let network_policy_json = serde_json::to_string(&network_sandbox_policy) - .expect("Failed to serialize NetworkSandboxPolicy to JSON"); + .unwrap_or_else(|err| panic!("failed to serialize network sandbox policy: {err}")); let mut linux_cmd = create_linux_sandbox_command_args( command, @@ -118,10 +115,9 @@ pub(crate) fn create_linux_sandbox_command_args( use_bwrap_sandbox: bool, allow_network_for_proxy: bool, ) -> Vec { - #[expect(clippy::expect_used)] let sandbox_policy_cwd = sandbox_policy_cwd .to_str() - .expect("cwd must be valid UTF-8") + .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) .to_string(); let mut linux_cmd: Vec = vec!["--sandbox-policy-cwd".to_string(), sandbox_policy_cwd]; From f8bbe765a2047cc7961010d9d20ad937ac30305b Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Sat, 7 Mar 2026 18:46:15 -0800 Subject: [PATCH 9/9] refactor(linux-sandbox): simplify policy argument handling --- codex-rs/core/src/landlock.rs | 42 ++++++++++++-------- codex-rs/linux-sandbox/src/linux_run_main.rs | 26 +++++++----- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index 736392efa2a6..d022adbc789b 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -69,7 +69,9 @@ pub(crate) fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { /// `codex-linux-sandbox`. /// /// The helper performs the actual sandboxing (bubblewrap + seccomp) after -/// parsing these arguments. See `docs/linux_sandbox.md` for the Linux semantics. +/// parsing these arguments. Policy JSON flags are emitted before helper feature +/// flags so the argv order matches the helper's CLI shape. See +/// `docs/linux_sandbox.md` for the Linux semantics. #[allow(clippy::too_many_arguments)] pub(crate) fn create_linux_sandbox_command_args_for_policies( command: Vec, @@ -86,29 +88,35 @@ pub(crate) fn create_linux_sandbox_command_args_for_policies( .unwrap_or_else(|err| panic!("failed to serialize filesystem sandbox policy: {err}")); let network_policy_json = serde_json::to_string(&network_sandbox_policy) .unwrap_or_else(|err| panic!("failed to serialize network sandbox policy: {err}")); + let sandbox_policy_cwd = sandbox_policy_cwd + .to_str() + .unwrap_or_else(|| panic!("cwd must be valid UTF-8")) + .to_string(); - let mut linux_cmd = create_linux_sandbox_command_args( - command, + let mut linux_cmd: Vec = vec![ + "--sandbox-policy-cwd".to_string(), 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, - ], - ); + "--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, + ]; + if use_bwrap_sandbox { + linux_cmd.push("--use-bwrap-sandbox".to_string()); + } + if allow_network_for_proxy { + linux_cmd.push("--allow-network-for-proxy".to_string()); + } + linux_cmd.push("--".to_string()); + linux_cmd.extend(command); linux_cmd } /// Converts the sandbox cwd and execution options into the CLI invocation for /// `codex-linux-sandbox`. +#[cfg(test)] pub(crate) fn create_linux_sandbox_command_args( command: Vec, sandbox_policy_cwd: &Path, diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 54a302a06a04..dad4a6d3ac1e 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -213,24 +213,31 @@ fn resolve_sandbox_policies( 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)) => { + // Accept either a fully legacy policy, a fully split policy pair, or all + // three views together. Reject partial split-policy input so the helper + // never runs with mismatched filesystem/network state. + let split_policies = match (file_system_sandbox_policy, network_sandbox_policy) { + (Some(file_system_sandbox_policy), Some(network_sandbox_policy)) => { + Some((file_system_sandbox_policy, network_sandbox_policy)) + } + (None, None) => None, + _ => panic!("file-system and network sandbox policies must be provided together"), + }; + + match (sandbox_policy, split_policies) { + (Some(sandbox_policy), Some((file_system_sandbox_policy, network_sandbox_policy))) => { EffectiveSandboxPolicies { sandbox_policy, file_system_sandbox_policy, network_sandbox_policy, } } - (Some(sandbox_policy), None, None) => EffectiveSandboxPolicies { + (Some(sandbox_policy), 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)) => { + (None, Some((file_system_sandbox_policy, network_sandbox_policy))) => { let sandbox_policy = file_system_sandbox_policy .to_legacy_sandbox_policy(network_sandbox_policy, sandbox_policy_cwd) .unwrap_or_else(|err| { @@ -242,8 +249,7 @@ fn resolve_sandbox_policies( network_sandbox_policy, } } - (None, None, None) => panic!("missing sandbox policy configuration"), - _ => panic!("file-system and network sandbox policies must be provided together"), + (None, None) => panic!("missing sandbox policy configuration"), } }