From 444359fa2d71b9313c902c6bd6fd598ed6d8f799 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 3 Nov 2026 23:15:55 +0000 Subject: [PATCH] Refactor sandbox request preparation by backend (#59258) ## What changed Build a `SandboxExecRequest` before applying backend-specific command wrapping. Extract Linux wrapping into `wrap_linux` and route Windows restricted-token requests through `prepare_legacy_windows_request`. Return unsandboxed requests before preparing native sandbox paths. GitOrigin-RevId: afe3bcb444c222e9e0af9a556fe580eee074e34c --- codex-rs/sandboxing/src/manager.rs | 346 ++++++++++++++++------------- 1 file changed, 187 insertions(+), 159 deletions(-) diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index f6c281a061..df6fcc8c6b 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -1,19 +1,16 @@ use crate::LinuxSandboxPidNamespace; #[cfg(target_os = "linux")] use crate::bwrap::WSL1_BWRAP_WARNING; -#[cfg(target_os = "linux")] -use crate::bwrap::is_wsl1; use crate::landlock::CODEX_LINUX_SANDBOX_ARG0; -use crate::landlock::create_linux_sandbox_command_args_for_permission_profile; use crate::policy_transforms::effective_permission_profile; use crate::policy_transforms::should_require_platform_sandbox; -#[cfg(target_os = "windows")] +#[cfg(windows)] use crate::resolve_windows_elevated_filesystem_overrides; -#[cfg(target_os = "windows")] +#[cfg(windows)] use crate::resolve_windows_restricted_token_filesystem_overrides; #[cfg(target_os = "macos")] use crate::seatbelt::MacosSeatbeltProfile; -#[cfg(target_os = "windows")] +#[cfg(windows)] use crate::windows_sandbox_uses_elevated_backend; use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::NetworkProxy; @@ -28,6 +25,7 @@ pub use codex_protocol::sandbox::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::collections::HashMap; +use std::ffi::OsStr; use std::ffi::OsString; use std::io; use std::path::Path; @@ -356,82 +354,91 @@ impl SandboxManager { &self, request: SandboxTransformRequest<'_>, ) -> Result { - let SandboxTransformRequest { - mut command, - permissions, - sandbox, - enforce_managed_network, - environment_id, - network, - sandbox_policy_cwd, - sandbox_exe, - use_legacy_landlock, - windows_sandbox_level, - } = request; - #[cfg(target_os = "macos")] - let managed_network = command.managed_network.as_ref(); - let additional_permissions = command.additional_permissions.take(); - let managed_mitm_ca_trust_bundle_path = - network.and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); - let base_effective_permission_profile = - effective_permission_profile(permissions, additional_permissions.as_ref()); - let pending_sandboxed_request = PendingSandboxedExecRequest::new( - &command.cwd, - sandbox_policy_cwd, - base_effective_permission_profile.clone(), + if request.sandbox == SandboxType::WindowsRestrictedToken { + return self.prepare_legacy_windows_request(request, SandboxOverride::NoOverride); + } + let command = request.command; + let managed_mitm_ca_trust_bundle_path = request + .network + .and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); + let permissions = effective_permission_profile( + request.permissions, + command.additional_permissions.as_ref(), + ); + let backend = request.sandbox; + let enforce_managed_network = request.enforce_managed_network; + let mut managed_network = command.managed_network; + let mut argv = Vec::with_capacity(1 + command.args.len()); + argv.push(command.program.to_string_lossy().into_owned()); + argv.extend(command.args); + let mut exec = SandboxExecRequest { + sandbox_override: SandboxOverride::NoOverride, + command: argv, + cwd: command.cwd, + sandbox_policy_cwd: request.sandbox_policy_cwd.clone(), + env: command.env, + network: request.network.cloned(), + network_environment_id: request.environment_id.map(str::to_owned), + sandbox: backend, + windows_sandbox_level: request.windows_sandbox_level, + permission_profile: permissions, + arg0: None, + }; + if backend == SandboxType::None { + return Ok(exec); + } + let pending = PendingSandboxedExecRequest::new( + &exec.cwd, + &exec.sandbox_policy_cwd, + exec.permission_profile, managed_mitm_ca_trust_bundle_path.as_ref(), ); - let mut argv = Vec::with_capacity(1 + command.args.len()); - argv.push(os_string_to_command_component(command.program)); - argv.extend(command.args); - - let (argv, arg0_override, pending_sandboxed_request) = match sandbox { - SandboxType::None => (argv, None, None), + match backend { + SandboxType::None => unreachable!("native launch returned above"), SandboxType::WindowsMxc => { if !codex_mxc_sandbox::is_available() { return Err(SandboxTransformError::WindowsMxcPreparation( - "native MXC is unavailable on this executor".to_string(), + "native MXC is unavailable on this executor".into(), )); } - if enforce_managed_network && command.managed_network.is_none() { - let network = network.ok_or_else(|| { + if enforce_managed_network && managed_network.is_none() { + let network = request.network.ok_or_else(|| { SandboxTransformError::WindowsMxcPreparation( "managed networking requires an executor-local proxy".to_string(), ) })?; let prepared = network .prepare_for_optional_environment( - std::mem::take(&mut command.env), - environment_id, + std::mem::take(&mut exec.env), + request.environment_id, ) .map_err(|err| { SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) })?; - command.env = prepared.env; - command.managed_network = Some(prepared.sandbox_context); + exec.env = prepared.env; + managed_network = Some(prepared.sandbox_context); } - let managed_network = command.managed_network.filter(|_| enforce_managed_network); - let pending = pending_sandboxed_request?; - let exe = sandbox_exe.ok_or_else(|| { + let pending = pending?; + exec.permission_profile = pending.effective_permission_profile; + let exe = request.sandbox_exe.ok_or_else(|| { SandboxTransformError::WindowsMxcPreparation( - "missing Codex executable path".to_string(), + "missing Codex executable path".into(), ) })?; - let mut full_command = - vec![os_string_to_command_component(exe.as_os_str().to_owned())]; - full_command.extend( - codex_mxc_sandbox::create_command_args( - codex_mxc_sandbox::CreateMxcCommandArgsParams { - command: argv, - permission_profile: &pending.effective_permission_profile, - sandbox_policy_cwd: pending.native_sandbox_policy_cwd.as_path(), - managed_network: managed_network.as_ref(), - env: &mut command.env, - }, - ) - .map_err(|err| SandboxTransformError::WindowsMxcPreparation(err.to_string()))?, - ); - (full_command, None, Some(pending)) + let route = managed_network.filter(|_| enforce_managed_network); + let args = codex_mxc_sandbox::create_command_args( + codex_mxc_sandbox::CreateMxcCommandArgsParams { + command: std::mem::take(&mut exec.command), + permission_profile: &exec.permission_profile, + sandbox_policy_cwd: &pending.native_sandbox_policy_cwd, + managed_network: route.as_ref(), + env: &mut exec.env, + }, + ) + .map_err(|err| SandboxTransformError::WindowsMxcPreparation(err.to_string()))?; + exec.command = std::iter::once(exe.to_string_lossy().into_owned()) + .chain(args) + .collect(); } #[cfg(target_os = "macos")] SandboxType::MacosSeatbelt => { @@ -440,20 +447,20 @@ impl SandboxManager { use crate::seatbelt::SeatbeltPreparationError; use crate::seatbelt::create_seatbelt_command_args_with_profile; - let pending = pending_sandboxed_request?; - let (file_system_sandbox_policy, network_sandbox_policy) = pending - .effective_permission_profile - .to_runtime_permissions(); - let mut args = create_seatbelt_command_args_with_profile( + let pending = pending?; + exec.permission_profile = pending.effective_permission_profile; + let (file_system_sandbox_policy, network_sandbox_policy) = + exec.permission_profile.to_runtime_permissions(); + let args = create_seatbelt_command_args_with_profile( CreateSeatbeltCommandArgsParams { - command: argv, + command: std::mem::take(&mut exec.command), file_system_sandbox_policy: &file_system_sandbox_policy, network_sandbox_policy, - sandbox_policy_cwd: pending.native_sandbox_policy_cwd.as_path(), + sandbox_policy_cwd: &pending.native_sandbox_policy_cwd, enforce_managed_network, - managed_network, - environment_id, - network, + managed_network: managed_network.as_ref(), + environment_id: exec.network_environment_id.as_deref(), + network: exec.network.as_ref(), extra_allow_unix_sockets: &[], }, self.seatbelt_profile, @@ -467,102 +474,137 @@ impl SandboxManager { SandboxTransformError::EnvironmentNetworkProxy(message) } })?; - let mut full_command = Vec::with_capacity(1 + args.len()); - full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); - full_command.append(&mut args); - (full_command, None, Some(pending)) + exec.command = std::iter::once(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_owned()) + .chain(args) + .collect(); } #[cfg(not(target_os = "macos"))] SandboxType::MacosSeatbelt => return Err(SandboxTransformError::SeatbeltUnavailable), SandboxType::LinuxSeccomp => { - let pending = pending_sandboxed_request?; - let exe = - sandbox_exe.ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?; + let pending = pending?; + exec.permission_profile = pending.effective_permission_profile; + let exe = request + .sandbox_exe + .ok_or(SandboxTransformError::MissingLinuxSandboxExecutable)?; if enforce_managed_network - && command.managed_network.is_none() - && let Some(network) = network + && managed_network.is_none() + && let Some(network) = request.network { let prepared = network .prepare_for_optional_environment( - std::mem::take(&mut command.env), - environment_id, + std::mem::take(&mut exec.env), + request.environment_id, ) .map_err(|err| { SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) })?; - command.env = prepared.env; - command.managed_network = Some(prepared.sandbox_context); + exec.env = prepared.env; + managed_network = Some(prepared.sandbox_context); } - let managed_network = - enforce_managed_network.then(|| command.managed_network.unwrap_or_default()); - #[cfg(target_os = "linux")] - ensure_linux_bubblewrap_is_supported( - &pending - .effective_permission_profile - .file_system_sandbox_policy(), - use_legacy_landlock, - managed_network.is_some(), - is_wsl1(), + let route = enforce_managed_network.then(|| managed_network.unwrap_or_default()); + self.wrap_linux( + &mut exec, + ( + &pending.native_command_cwd, + &pending.native_sandbox_policy_cwd, + ), + exe, + request.use_legacy_landlock, + route.as_ref(), )?; - let mut args = create_linux_sandbox_command_args_for_permission_profile( - argv, - pending.native_command_cwd.as_path(), - &pending.effective_permission_profile, - pending.native_sandbox_policy_cwd.as_path(), - use_legacy_landlock, - managed_network.as_ref(), - ); - // Keep default invocations compatible with older helpers. Only the - // startup opt-in requires a helper that understands PID inheritance. - if self.linux_sandbox_pid_namespace == LinuxSandboxPidNamespace::Inherit { - args.insert(0, "--inherit-pid-namespace".to_string()); - } - let mut full_command = Vec::with_capacity(1 + args.len()); - full_command.push(os_string_to_command_component(exe.as_os_str().to_owned())); - full_command.append(&mut args); - ( - full_command, - Some(linux_sandbox_arg0_override(exe)), - Some(pending), - ) } - #[cfg(target_os = "windows")] SandboxType::WindowsRestrictedToken => { - if enforce_managed_network && windows_sandbox_level != WindowsSandboxLevel::Elevated - { - return Err(SandboxTransformError::WindowsSandboxPreparation( - "managed networking requires the elevated Windows sandbox backend" - .to_string(), - )); - } - let pending = pending_sandboxed_request?; - (argv, None, Some(pending)) + unreachable!("native Windows uses its legacy launcher") } - #[cfg(not(target_os = "windows"))] - SandboxType::WindowsRestrictedToken => (argv, None, Some(pending_sandboxed_request?)), + } + Ok(exec) + } + + fn wrap_linux( + &self, + exec: &mut SandboxExecRequest, + (cwd, policy_cwd): (&AbsolutePathBuf, &AbsolutePathBuf), + exe: &Path, + use_legacy_landlock: bool, + managed_network: Option<&ManagedNetworkSandboxContext>, + ) -> Result<(), SandboxTransformError> { + #[cfg(target_os = "linux")] + ensure_linux_bubblewrap_is_supported( + &exec.permission_profile.file_system_sandbox_policy(), + use_legacy_landlock, + managed_network.is_some(), + crate::bwrap::is_wsl1(), + )?; + let program = exe.to_string_lossy().into_owned(); + let mut args = crate::landlock::create_linux_sandbox_command_args_for_permission_profile( + std::mem::take(&mut exec.command), + cwd, + &exec.permission_profile, + policy_cwd, + use_legacy_landlock, + managed_network, + ); + // Only the startup opt-in requires a helper that understands PID inheritance. + if self.linux_sandbox_pid_namespace == LinuxSandboxPidNamespace::Inherit { + args.insert(0, "--inherit-pid-namespace".to_owned()); + } + exec.command = std::iter::once(program.clone()).chain(args).collect(); + let arg0 = if exe.file_name() == Some(OsStr::new(CODEX_LINUX_SANDBOX_ARG0)) { + &program + } else { + CODEX_LINUX_SANDBOX_ARG0 }; + exec.arg0 = Some(arg0.to_owned()); + Ok(()) + } - // Unsandboxed exec-server requests may have foreign cwd values that cannot be prepared - // locally, but their effective permissions must still be preserved. In that case, carry - // forward the base profile. - let permission_profile = pending_sandboxed_request - .map_or(base_effective_permission_profile, |pending| { - pending.effective_permission_profile - }); - - Ok(SandboxExecRequest { - sandbox_override: SandboxOverride::NoOverride, + /// Prepare the unwrapped request for core's native Windows launcher, which binds its own + /// proxy environment and filesystem overrides. Other backends are prepared by [`Self::transform`]. + fn prepare_legacy_windows_request( + &self, + request: SandboxTransformRequest<'_>, + sandbox_override: SandboxOverride, + ) -> Result { + debug_assert_eq!(request.sandbox, SandboxType::WindowsRestrictedToken); + #[cfg(windows)] + if request.enforce_managed_network + && request.windows_sandbox_level != WindowsSandboxLevel::Elevated + { + return Err(SandboxTransformError::WindowsSandboxPreparation( + "managed networking requires the elevated Windows sandbox backend".into(), + )); + } + let command = request.command; + let mut argv = Vec::with_capacity(1 + command.args.len()); + argv.push(command.program.to_string_lossy().into_owned()); + argv.extend(command.args); + let trust_bundle = request + .network + .and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); + let mut prepared = SandboxExecRequest { + sandbox_override, command: argv, cwd: command.cwd, - sandbox_policy_cwd: sandbox_policy_cwd.clone(), + sandbox_policy_cwd: request.sandbox_policy_cwd.clone(), env: command.env, - network: network.cloned(), - network_environment_id: environment_id.map(str::to_string), - sandbox, - windows_sandbox_level, - permission_profile, - arg0: arg0_override, - }) + network: request.network.cloned(), + network_environment_id: request.environment_id.map(str::to_owned), + sandbox: SandboxType::WindowsRestrictedToken, + windows_sandbox_level: request.windows_sandbox_level, + permission_profile: effective_permission_profile( + request.permissions, + command.additional_permissions.as_ref(), + ), + arg0: None, + }; + prepared.permission_profile = PendingSandboxedExecRequest::new( + &prepared.cwd, + &prepared.sandbox_policy_cwd, + prepared.permission_profile, + trust_bundle.as_ref(), + )? + .effective_permission_profile; + Ok(prepared) } pub fn transform_for_direct_spawn( @@ -814,20 +856,6 @@ fn ensure_linux_bubblewrap_is_supported( Ok(()) } -fn os_string_to_command_component(value: OsString) -> String { - value - .into_string() - .unwrap_or_else(|value| value.to_string_lossy().into_owned()) -} - -fn linux_sandbox_arg0_override(exe: &Path) -> String { - if exe.file_name().and_then(|name| name.to_str()) == Some(CODEX_LINUX_SANDBOX_ARG0) { - os_string_to_command_component(exe.as_os_str().to_owned()) - } else { - CODEX_LINUX_SANDBOX_ARG0.to_string() - } -} - #[cfg(test)] #[path = "manager_tests.rs"] mod tests;