From f7aad26f1d4e098bbd86ce8415a622fda55b0208 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 10 Nov 2026 20:57:38 +0000 Subject: [PATCH] Defer exec-server sandbox preparation to the execution host (#60663) ## Why Remote commands can use workspace paths from a different operating system. Preparing them on the client should preserve those paths and command arguments for the execution host. ## What changed - Build exec-server requests directly, carrying the native command and sandbox context without applying the client's sandbox transform. - Resolve workspace permissions and runtime read access in `SandboxManager` on the execution host, including helper binaries and validated managed-network CA bundles. - Add explicit backend selection and required platform sandbox selection, rejecting launches when the required sandbox is unavailable. ## Testing Extend request tests to cover foreign workspace paths, Unicode arguments, and empty arguments. Extend the remote process test on Unix to verify that restricted reads allow the sandbox helper and validated CA bundle while denying an untrusted certificate file. GitOrigin-RevId: 5fc02ac4208912ecefb80a212742389afc14ac05 --- codex-rs/cli/src/debug_sandbox.rs | 4 +- codex-rs/core/src/exec.rs | 4 +- codex-rs/core/src/shell_snapshot_sandbox.rs | 4 +- .../core/src/tools/runtimes/unified_exec.rs | 77 +++++++------ codex-rs/core/src/tools/sandboxing.rs | 51 ++++----- codex-rs/core/src/tools/sandboxing_tests.rs | 66 +++++------ .../core/src/unified_exec/process_manager.rs | 32 ++---- codex-rs/core/tests/windows_sandbox.rs | 4 +- codex-rs/exec-server/src/environment.rs | 2 +- codex-rs/exec-server/src/fs_sandbox.rs | 28 ++--- codex-rs/exec-server/src/process_sandbox.rs | 103 ++++++------------ codex-rs/exec-server/src/sandbox_selection.rs | 36 ++---- codex-rs/exec-server/tests/exec_process.rs | 35 ++++-- codex-rs/sandboxing/src/launch.rs | 14 ++- codex-rs/sandboxing/src/lib.rs | 4 + codex-rs/sandboxing/src/manager.rs | 54 ++++++--- codex-rs/sandboxing/src/manager_tests.rs | 22 ++-- 17 files changed, 270 insertions(+), 270 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index a0a6ea719d..4f074f28d9 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -28,6 +28,7 @@ use codex_protocol::models::SandboxEnforcement; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType as SandboxBackend; use codex_sandboxing::SandboxType::WindowsMxc; @@ -412,7 +413,8 @@ async fn run_command_under_sandbox( }, permissions: &permission_profile, workspace_roots: workspace_roots.as_deref(), - sandbox, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(sandbox), enforce_managed_network: network.is_some(), network: network.as_ref(), environment_id: None, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 9faf75afff..029eb4ae01 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -37,6 +37,7 @@ use codex_protocol::protocol::ExecCommandOutputDeltaEvent; use codex_protocol::protocol::ExecOutputStream; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; @@ -409,7 +410,8 @@ pub async fn build_exec_request( command, permissions: permission_profile, workspace_roots: None, - sandbox: sandbox_type, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(sandbox_type), enforce_managed_network, environment_id: network_environment_id.as_deref(), network: network.as_ref(), diff --git a/codex-rs/core/src/shell_snapshot_sandbox.rs b/codex-rs/core/src/shell_snapshot_sandbox.rs index 5ed7fb792e..a45412e71f 100644 --- a/codex-rs/core/src/shell_snapshot_sandbox.rs +++ b/codex-rs/core/src/shell_snapshot_sandbox.rs @@ -17,6 +17,7 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::policy_transforms::merge_permission_profiles; @@ -165,7 +166,8 @@ impl ShellSnapshotSandbox { }, permissions: &self.permissions, workspace_roots: None, - sandbox: self.sandbox, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(self.sandbox), enforce_managed_network: self.enforce_managed_network, environment_id: Some(&self.environment_id), network: self.network.as_ref(), diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index ae986a721d..849795fb50 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -44,7 +44,6 @@ use codex_core_plugins::PluginMetricsSidecar; use codex_features::Feature; use codex_file_system::WindowsSandboxSelection; use codex_network_proxy::CREDENTIAL_BROKER_ACTIVE_ENV_KEY; -use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::NetworkProxy; use codex_network_proxy::RemoteCredentialMasking; use codex_network_proxy::RemoteNetworkProxyConfig; @@ -122,26 +121,6 @@ fn unified_exec_options( } } -fn build_unified_exec_sandbox_command( - command: &[String], - cwd: &PathUri, - env: &HashMap, - managed_network: Option, - additional_permissions: Option, -) -> Result { - let (program, args) = command - .split_first() - .ok_or_else(|| ToolError::Rejected("command args are empty".to_string()))?; - Ok(SandboxCommand { - program: program.clone().into(), - args: args.to_vec(), - cwd: cwd.clone(), - env: env.clone(), - managed_network, - additional_permissions, - }) -} - impl<'a> UnifiedExecRuntime<'a> { /// Creates a runtime bound to the shared unified-exec process manager. pub fn new(manager: &'a UnifiedExecProcessManager) -> Self { @@ -596,10 +575,11 @@ impl<'a> ToolRuntime, UnifiedExecAttempt> for UnifiedExec prepared.permissions.additional_permissions.as_ref(), internal_permissions.as_ref(), ); + let executor_managed = self.uses_executor_managed_process_sandbox(req); let permissions = TerminalPermissions::for_launch( &prepared.turn_environment, &ctx.step_context.turn, - if self.uses_executor_managed_process_sandbox(req) { + if executor_managed { TerminalSandboxSource::Executor } else { TerminalSandboxSource::Native @@ -624,31 +604,50 @@ impl<'a> ToolRuntime, UnifiedExecAttempt> for UnifiedExec permissions }; - let command = build_unified_exec_sandbox_command( - &command, - &prepared.cwd, - &env, - managed_network_context, - additional_permissions, - ) - .map_err(|error| match error { - ToolError::Rejected(_) => { - ToolError::Rejected("missing command line for PTY".to_string()) - } - error @ ToolError::Codex(_) => error, - })?; + let Some((program, args)) = command.split_first() else { + return Err(ToolError::Rejected( + "missing command line for PTY".to_string(), + )); + }; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); + let request = if executor_managed { + attempt + .env_for_exec_server( + command, + prepared.cwd.clone(), + env, + managed_network_context, + additional_permissions.as_ref(), + options, + ) + .await + } else { + let command = SandboxCommand { + program: program.as_str().into(), + args: args.to_vec(), + cwd: prepared.cwd.clone(), + env, + managed_network: managed_network_context, + additional_permissions, + }; + attempt + .env_for( + command, + options, + managed_network, + Some(&prepared.turn_environment.selection.environment_id), + ) + .await + } + .map_err(ToolError::Codex)?; let process = self .manager .open_session_with_exec_env( req.process_id, ctx, - command, - options, - attempt, + request, managed_network, network_proxy_launch, - /*environment_id*/ Some(&prepared.turn_environment.selection.environment_id), req.exec_server_env_config.clone(), req.shell_snapshot.clone(), windows_sandbox_proxy_settings_mode, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 2df6466d52..9f37891109 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -14,17 +14,21 @@ use crate::tools::hook_names::HookToolName; use crate::tools::network_approval::NetworkApprovalSpec; use codex_file_system::FileSystemSandboxContext; use codex_file_system::WindowsSandboxSelection; +use codex_network_proxy::ManagedNetworkSandboxContext; use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::error::CodexErr; +use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; pub(crate) use codex_protocol::sandbox::SandboxOverride; use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; @@ -505,7 +509,8 @@ impl<'a> SandboxAttempt<'a> { command, permissions: self.permissions, workspace_roots: None, - sandbox: self.sandbox, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(self.sandbox), enforce_managed_network: self.enforce_managed_network, environment_id, network, @@ -527,32 +532,28 @@ impl<'a> SandboxAttempt<'a> { pub async fn env_for_exec_server( &self, - command: SandboxCommand, + argv: Vec, + cwd: PathUri, + env: HashMap, + managed_network: Option, + additional_permissions: Option<&AdditionalPermissionProfile>, options: ExecOptions, ) -> Result { - let managed_network = command.managed_network.clone(); - let exec_server_permissions = effective_permission_profile( - self.exec_server_permissions, - command.additional_permissions.as_ref(), - ); - let mut request = self - .manager - .transform(SandboxTransformRequest { - command, - permissions: self.permissions, - workspace_roots: None, - // The exec-server must receive the native command, not this host's wrapper. - sandbox: SandboxType::None, - enforce_managed_network: self.enforce_managed_network, - environment_id: None, - network: None, - sandbox_policy_cwd: self.sandbox_cwd, - sandbox_exe: None, - use_legacy_landlock: self.use_legacy_landlock, - windows_sandbox_level: self.windows_sandbox_level, - })? - .into_command_request(); - request.sandbox_override = self.sandbox_override; + let exec_server_permissions = + effective_permission_profile(self.exec_server_permissions, additional_permissions); + let request = SandboxExecRequest { + command: argv, + cwd, + env, + network: None, + network_environment_id: None, + sandbox: SandboxType::None, + sandbox_override: self.sandbox_override, + sandbox_policy_cwd: self.sandbox_cwd.clone(), + windows_sandbox_level: self.windows_sandbox_level, + permission_profile: exec_server_permissions.clone(), + arg0: None, + }; let mut exec_request = crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, Vec::new()) .await?; diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 3c2cf95933..8465631cb0 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -363,15 +363,16 @@ async fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backe #[tokio::test] async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { - let cwd: AbsolutePathBuf = std::env::current_dir() - .expect("current dir") - .try_into() - .expect("absolute cwd"); - let cwd_uri = PathUri::from_abs_path(&cwd); + let cwd_uri = PathUri::parse(if cfg!(windows) { + "file:///workspace" + } else { + "file:///C:/workspace" + }) + .expect("foreign workspace"); let exec_server_permissions = codex_protocol::models::PermissionProfile::workspace_write(); let permissions = exec_server_permissions .clone() - .materialize_project_roots_with_workspace_roots(std::slice::from_ref(&cwd)); + .materialize_project_roots_with_path_uris(std::slice::from_ref(&cwd_uri)); let manager = SandboxManager::new(); let mut attempt = SandboxAttempt { sandbox_override: SandboxOverride::NoOverride, @@ -395,32 +396,33 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { allow_local_binding: false, ..Default::default() }; - let command = || SandboxCommand { - program: "/bin/bash".into(), - args: vec!["-lc".to_string(), "pwd".to_string()], - cwd: cwd_uri.clone(), - env: HashMap::new(), - managed_network: Some(managed_network.clone()), - additional_permissions: None, + let argv = vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf 'café au lait'".to_string(), + String::new(), + ]; + let prepare = async |attempt: &SandboxAttempt<'_>| { + attempt + .env_for_exec_server( + argv.clone(), + cwd_uri.clone(), + HashMap::new(), + Some(managed_network.clone()), + /*additional_permissions*/ None, + crate::sandboxing::ExecOptions { + expiration: crate::exec::ExecExpiration::DefaultTimeout, + capture_policy: crate::exec::ExecCapturePolicy::ShellTool, + }, + ) + .await }; - let options = || crate::sandboxing::ExecOptions { - expiration: crate::exec::ExecExpiration::DefaultTimeout, - capture_policy: crate::exec::ExecCapturePolicy::ShellTool, - }; - let request = attempt - .env_for_exec_server(command(), options()) + let request = prepare(&attempt) .await .expect("prepare remote exec request"); assert!(!attempt.is_escalated()); - assert_eq!( - request.command, - vec![ - "/bin/bash".to_string(), - "-lc".to_string(), - "pwd".to_string() - ] - ); + assert_eq!(request.command, argv); assert_eq!(request.arg0, None); assert_eq!(request.sandbox, SandboxType::None); assert_eq!( @@ -433,9 +435,9 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { user_home_dir: None, temporary_directories: None, windows_sandbox_selection: if cfg!(windows) { - codex_file_system::WindowsSandboxSelection::RestrictedToken - } else { codex_file_system::WindowsSandboxSelection::Disabled + } else { + codex_file_system::WindowsSandboxSelection::RestrictedToken }, windows_sandbox_proxy_settings_mode: None, use_legacy_landlock: false, @@ -448,8 +450,7 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { ); attempt.sandbox_override = SandboxOverride::EscalatedSandboxWithRestrictions; - let escalated_request = attempt - .env_for_exec_server(command(), options()) + let escalated_request = prepare(&attempt) .await .expect("prepare approved escalation with retained sandbox restrictions"); let mut expected_context = request.exec_server_sandbox.expect("sandbox context"); @@ -460,8 +461,7 @@ async fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { ); attempt.sandbox_requested = false; - let request = attempt - .env_for_exec_server(command(), options()) + let request = prepare(&attempt) .await .expect("prepare unsandboxed remote exec request"); assert!(attempt.is_escalated()); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index caa45622a2..315dbee402 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -27,7 +27,6 @@ use crate::exec_env::inject_session_env; use crate::exec_env::set_tool_call_id_env_var; use crate::guardian::GuardianReviewContext; use crate::plugins::metrics::finish_and_track_measurements; -use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::ExecServerEnvConfig; use crate::tools::ApprovalContext; @@ -42,7 +41,6 @@ use crate::tools::runtimes::is_managed_proxy_env_var; use crate::tools::runtimes::unified_exec::UnifiedExecAttempt; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; -use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::unified_exec::ExecCommandRequest; @@ -84,7 +82,6 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecCommandSource; use codex_protocol::protocol::TerminalInteractionEvent; use codex_protocol::shell_environment::is_non_inheritable_env_var; -use codex_sandboxing::SandboxCommand; use codex_tools::ToolName; use codex_utils_head_tail_buffer::HeadTailBuffer; use codex_utils_output_truncation::approx_tokens_from_byte_count; @@ -1310,36 +1307,29 @@ impl UnifiedExecProcessManager { &self, process_id: i32, tool_ctx: &ToolCtx, - command: SandboxCommand, - options: ExecOptions, - attempt: &SandboxAttempt<'_>, + request: ExecRequest, network: Option<&NetworkProxy>, network_proxy_launch: Option, - environment_id: Option<&str>, exec_server_env_config: Option, shell_snapshot: Option, windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode, tty: bool, environment: &codex_exec_server::Environment, ) -> Result { - let mut request = if environment.is_remote() || shell_snapshot.is_some() { - attempt.env_for_exec_server(command, options).await - } else { - attempt - .env_for(command, options, network, environment_id) - .await - } - .map_err(ToolError::Codex)?; let network_policy_decider = network_proxy_launch .as_ref() .filter(|launch| launch.policy_decision_timeout_ms.is_some()) .and_then(|launch| network?.remote_policy_decider(launch.proxy.allow_local_binding)); - if environment.is_remote() && network.is_some() && network_proxy_launch.is_none() { - request.exec_server_enforce_managed_network = false; - } - request.exec_server_network_proxy = network_proxy_launch; - request.exec_server_env_config = exec_server_env_config; - request.exec_server_shell_snapshot = shell_snapshot; + let missing_remote_proxy = + environment.is_remote() && network.is_some() && network_proxy_launch.is_none(); + let request = ExecRequest { + exec_server_enforce_managed_network: request.exec_server_enforce_managed_network + && !missing_remote_proxy, + exec_server_network_proxy: network_proxy_launch, + exec_server_env_config, + exec_server_shell_snapshot: shell_snapshot, + ..request + }; self.open_session_with_prepared_exec_env( process_id, &request, diff --git a/codex-rs/core/tests/windows_sandbox.rs b/codex-rs/core/tests/windows_sandbox.rs index c0c0d1d693..8957895686 100644 --- a/codex-rs/core/tests/windows_sandbox.rs +++ b/codex-rs/core/tests/windows_sandbox.rs @@ -40,6 +40,7 @@ use codex_protocol::user_input::UserInput; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::WindowsSandboxProxySettingsMode; @@ -518,7 +519,8 @@ async fn windows_elevated_temp_only_core_and_direct_spawn_enforce_carveouts() -> }, permissions: &permission_profile, workspace_roots: None, - sandbox: SandboxType::WindowsRestrictedToken, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::WindowsRestrictedToken), enforce_managed_network: false, environment_id: None, network: None, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 0a83e16d3c..a5adfb7728 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -2024,7 +2024,7 @@ mod tests { assert_eq!( err.to_string(), format!( - "exec-server rejected request (-32602): sandbox cwd URI `{sandbox_cwd}` is not valid on this exec-server host: {source}" + "exec-server rejected request (-32602): failed to prepare process sandbox: invalid sandbox policy cwd: {source}" ) ); } diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 8da7523dfa..cc0dd859a6 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -3,7 +3,6 @@ use std::collections::HashMap; use std::time::Duration; use codex_exec_server_protocol::JSONRPCErrorError; -use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; @@ -16,7 +15,6 @@ use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; -use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; #[cfg(not(any(target_os = "linux", target_os = "macos")))] use codex_utils_absolute_path::canonicalize_preserving_symlinks; @@ -157,17 +155,8 @@ impl FileSystemSandboxRunner { let sandbox_manager = sandbox_manager.with_allowed_symlinked_codex_home( self.runtime_paths.allowed_symlinked_codex_home.clone(), ); - let (sandbox, windows_sandbox_level) = crate::sandbox_selection::select_sandbox( - &sandbox_manager, - permission_profile, - sandbox_context, - /*has_managed_network_requirements*/ false, - ); - if sandbox == SandboxType::None { - return Err(invalid_request( - "filesystem sandbox cannot be enforced on this executor".to_string(), - )); - } + let (selection, windows_sandbox_level) = + crate::sandbox_selection::sandbox_selection(sandbox_context); // Requests use absolute paths; the helper can start at the filesystem root even if // the policy cwd was removed. Keep its drive or share for Windows `:root` rules. let helper_cwd = cwd @@ -192,7 +181,8 @@ impl FileSystemSandboxRunner { command, permissions: permission_profile, workspace_roots: None, - sandbox, + runtime_readable_paths: &[], + selection, enforce_managed_network: false, environment_id: None, network: None, @@ -203,11 +193,15 @@ impl FileSystemSandboxRunner { self.runtime_paths.codex_linux_sandbox_exe.as_deref() }, use_legacy_landlock: sandbox_context.use_legacy_landlock, - windows_sandbox_level: windows_sandbox_level - .unwrap_or(WindowsSandboxLevel::Disabled), + windows_sandbox_level, }, }) - .map_err(|err| invalid_request(format!("failed to prepare fs sandbox: {err}")))?; + .map_err(|err| match err { + codex_sandboxing::SandboxTransformError::SandboxUnavailable => invalid_request( + "filesystem sandbox cannot be enforced on this executor".to_string(), + ), + err => invalid_request(format!("failed to prepare fs sandbox: {err}")), + })?; request.sandbox_override = sandbox_context.sandbox_override; Ok(request) } diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index f6f8ba09a3..057f4fda79 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -25,7 +25,6 @@ use codex_sandboxing::SandboxType; use codex_sandboxing::WindowsSandboxFilesystemOverrides; use codex_sandboxing::WindowsSandboxProxySettingsMode; use codex_sandboxing::WindowsSandboxSpawnRequest; -use codex_sandboxing::with_managed_mitm_ca_readable_root; #[cfg(windows)] use codex_shell_command::shell_detect::fallback_powershell_shell_for_windows_sandbox; use codex_utils_absolute_path::AbsolutePathBuf; @@ -38,7 +37,7 @@ use crate::process_telemetry::trace_process_id; use crate::protocol::ExecParams; use crate::rpc::internal_error; use crate::rpc::invalid_params; -use crate::sandbox_selection::select_sandbox; +use crate::sandbox_selection::sandbox_selection; const CREDENTIAL_MASKING_FAILURE_MODE: CredentialMaskingFailureMode = CredentialMaskingFailureMode::Warn; @@ -152,24 +151,16 @@ pub(crate) async fn prepare_exec_request_with_telemetry( }; let runtime_paths = runtime_paths .ok_or_else(|| invalid_params("sandbox runtime paths are not configured".to_string()))?; - sandbox_context - .validate_file_system_paths_for_current_host() - .map_err(|err| invalid_params(err.to_string()))?; let windows_sandbox_proxy_settings_mode = sandbox_context .windows_sandbox_proxy_settings_mode .unwrap_or_default(); - // TODO(jif): Transport permissions before orchestrator-local paths are materialized, - // then resolve executor-local helper and workspace paths here. - let permissions = sandbox_context.permissions.clone(); - let sandbox_policy_cwd = &sandbox_context.cwd; - let native_sandbox_policy_cwd = native_path(sandbox_policy_cwd, "sandbox cwd")?; - let native_workspace_roots = sandbox_context - .workspace_roots - .iter() - .map(|root| native_path(root, "sandbox workspace root")) - .collect::, _>>()?; - let workspace_roots = native_workspace_roots.as_slice(); - let permissions = permissions.materialize_project_roots_with_workspace_roots(workspace_roots); + let mut runtime_readable_paths = Vec::new(); + if cfg!(target_os = "linux") || (cfg!(unix) && params.arg0.is_some()) { + runtime_readable_paths.push(runtime_paths.codex_self_exe.clone()); + } + if cfg!(target_os = "linux") { + runtime_readable_paths.extend(runtime_paths.codex_linux_sandbox_exe.iter().cloned()); + } let managed_mitm_ca_trust_bundle_path = managed_network.as_ref().and_then(|_| { CUSTOM_CA_ENV_KEYS.iter().find_map(|key| { let path = env.get(*key)?; @@ -179,55 +170,13 @@ pub(crate) async fn prepare_exec_request_with_telemetry( AbsolutePathBuf::from_absolute_path(path).ok() }) }); - let permissions = with_managed_mitm_ca_readable_root( - permissions, - managed_mitm_ca_trust_bundle_path.as_ref(), - native_sandbox_policy_cwd.as_path(), - ); - #[cfg(unix)] - let (file_system_policy, network_policy) = permissions.to_runtime_permissions(); - #[cfg(unix)] - let sandbox_helper_paths = params - .arg0 - .iter() - .map(|_| runtime_paths.codex_self_exe.clone()) - .collect::>(); - // Bubblewrap launches the configured helper, which may re-enter this executable to apply - // seccomp, so the outer filesystem sandbox must expose both paths. - #[cfg(target_os = "linux")] - let sandbox_helper_paths = { - let mut sandbox_helper_paths = sandbox_helper_paths; - if !sandbox_helper_paths.contains(&runtime_paths.codex_self_exe) { - sandbox_helper_paths.push(runtime_paths.codex_self_exe.clone()); - } - sandbox_helper_paths.extend(runtime_paths.codex_linux_sandbox_exe.iter().cloned()); - sandbox_helper_paths - }; - #[cfg(unix)] - let file_system_policy = file_system_policy - .with_additional_readable_roots(native_sandbox_policy_cwd.as_path(), &sandbox_helper_paths); - #[cfg(unix)] - let permissions = PermissionProfile::from_runtime_permissions_with_enforcement( - permissions.enforcement(), - &file_system_policy, - network_policy, - ); + runtime_readable_paths.extend(managed_mitm_ca_trust_bundle_path); let sandbox_manager = SandboxManager::new() .with_linux_sandbox_pid_namespace(runtime_paths.linux_sandbox_pid_namespace); #[cfg(target_os = "macos")] let sandbox_manager = sandbox_manager .with_allowed_symlinked_codex_home(runtime_paths.allowed_symlinked_codex_home.clone()); - let (sandbox, windows_sandbox_level) = select_sandbox( - &sandbox_manager, - &permissions, - sandbox_context, - params.enforce_managed_network, - ); - if sandbox == SandboxType::None { - return Err(invalid_params( - "sandbox intent cannot be enforced on this executor".to_string(), - )); - } + let (selection, windows_sandbox_level) = sandbox_selection(sandbox_context); let (program, args) = params .argv .split_first() @@ -276,33 +225,43 @@ pub(crate) async fn prepare_exec_request_with_telemetry( managed_network, additional_permissions: None, }, - permissions: &permissions, - workspace_roots: None, - sandbox, + permissions: &sandbox_context.permissions, + workspace_roots: Some(&sandbox_context.workspace_roots), + runtime_readable_paths: &runtime_readable_paths, + selection, enforce_managed_network: params.enforce_managed_network, environment_id: None, network: None, - sandbox_policy_cwd, + sandbox_policy_cwd: &sandbox_context.cwd, sandbox_exe: if cfg!(windows) { Some(runtime_paths.codex_self_exe.as_path()) } else { runtime_paths.codex_linux_sandbox_exe.as_deref() }, use_legacy_landlock: sandbox_context.use_legacy_landlock, - windows_sandbox_level: windows_sandbox_level.unwrap_or(WindowsSandboxLevel::Disabled), + windows_sandbox_level, }) - .map_err(|err| invalid_params(format!("failed to prepare process sandbox: {err}")))? + .map_err(|err| match err { + codex_sandboxing::SandboxTransformError::SandboxUnavailable => { + invalid_params(err.to_string()) + } + err => invalid_params(format!("failed to prepare process sandbox: {err}")), + })? .into_command_request(); request.sandbox_override = sandbox_context.sandbox_override; crate::run_integrity_checks(&request).await; - let windows_sandbox = if sandbox == SandboxType::WindowsRestrictedToken { - let windows_sandbox_level = windows_sandbox_level.ok_or_else(|| { - invalid_params("restricted token sandbox requires a sandbox level".to_string()) - })?; + let windows_sandbox = if request.sandbox == SandboxType::WindowsRestrictedToken { + let permissions = request.permission_profile; + let native_sandbox_policy_cwd = native_path(&request.sandbox_policy_cwd, "sandbox cwd")?; + let native_workspace_roots = sandbox_context + .workspace_roots + .iter() + .map(|root| native_path(root, "sandbox workspace root")) + .collect::>()?; request.arg0 = params.arg0.clone(); let proxy_enforced = params.enforce_managed_network; let filesystem_overrides = WindowsSandboxFilesystemOverrides::resolve( - sandbox, + request.sandbox, &permissions, &native_sandbox_policy_cwd, windows_sandbox_level, diff --git a/codex-rs/exec-server/src/sandbox_selection.rs b/codex-rs/exec-server/src/sandbox_selection.rs index abecdcd967..d28d0905a0 100644 --- a/codex-rs/exec-server/src/sandbox_selection.rs +++ b/codex-rs/exec-server/src/sandbox_selection.rs @@ -1,12 +1,10 @@ -//! Resolves local sandbox implementations and checks support for an explicitly selected remote MXC. +//! Translates Windows sandbox selection and validates explicit remote MXC requests. use codex_file_system::FileSystemSandboxContext; use codex_file_system::WindowsSandboxSelection; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::models::PermissionProfile; -use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxSelection; use codex_sandboxing::SandboxType; -use codex_sandboxing::SandboxablePreference; pub(crate) fn validate_remote_mxc(info: &crate::EnvironmentInfo) -> std::io::Result<()> { if !info.capabilities.windows_mxc { @@ -18,29 +16,19 @@ pub(crate) fn validate_remote_mxc(info: &crate::EnvironmentInfo) -> std::io::Res Ok(()) } -pub(crate) fn select_sandbox( - manager: &SandboxManager, - permission_profile: &PermissionProfile, - sandbox_context: &FileSystemSandboxContext, - has_managed_network_requirements: bool, -) -> (SandboxType, Option) { - let windows_sandbox_level = match sandbox_context.windows_sandbox_selection { +pub(crate) fn sandbox_selection( + context: &FileSystemSandboxContext, +) -> (SandboxSelection, WindowsSandboxLevel) { + let level = match context.windows_sandbox_selection { WindowsSandboxSelection::Disabled => WindowsSandboxLevel::Disabled, WindowsSandboxSelection::RestrictedToken => WindowsSandboxLevel::RestrictedToken, WindowsSandboxSelection::Elevated => WindowsSandboxLevel::Elevated, - WindowsSandboxSelection::Mxc => return (SandboxType::WindowsMxc, None), - }; - let windows_sandbox_type = match windows_sandbox_level { - WindowsSandboxLevel::Disabled => SandboxType::None, - WindowsSandboxLevel::RestrictedToken | WindowsSandboxLevel::Elevated => { - SandboxType::WindowsRestrictedToken + WindowsSandboxSelection::Mxc => { + return ( + SandboxSelection::Explicit(SandboxType::WindowsMxc), + WindowsSandboxLevel::Disabled, + ); } }; - let sandbox_type = manager.select_initial( - permission_profile, - SandboxablePreference::Require, - windows_sandbox_type, - has_managed_network_requirements, - ); - (sandbox_type, Some(windows_sandbox_level)) + (SandboxSelection::RequirePlatform, level) } diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index 26f5ec6e8e..bfd8417e37 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -1272,9 +1272,9 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> { Ok(()) } -#[cfg(target_os = "linux")] +#[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn remote_process_keeps_sandbox_helper_visible_with_restricted_reads() -> Result<()> { +async fn remote_process_can_read_its_helper_and_generated_ca_with_restricted_reads() -> Result<()> { if let Some(warning) = codex_sandboxing::system_bwrap_warning( &PermissionProfile::read_only(), &std::env::current_dir()?, @@ -1283,7 +1283,16 @@ async fn remote_process_keeps_sandbox_helper_visible_with_restricted_reads() -> return Ok(()); } - let context = create_process_context(/*use_remote*/ true).await?; + let server = exec_server().await?; + let environment = Environment::create_for_tests(Some(server.websocket_url().to_string()))?; + let proxy_dir = server.codex_home().canonicalize()?.join("proxy"); + std::fs::create_dir_all(&proxy_dir)?; + // The bundle name contains the SHA-256 of its contents. + let bundle = proxy_dir + .join("ca-bundle-ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad.pem"); + std::fs::write(&bundle, b"abc")?; + let untrusted = proxy_dir.join("untrusted.pem"); + std::fs::write(&untrusted, b"untrusted")?; let workspace = TempDir::new()?; let file = workspace.path().join("allowed.txt"); std::fs::write(&file, b"allowed")?; @@ -1309,22 +1318,30 @@ async fn remote_process_keeps_sandbox_helper_visible_with_restricted_reads() -> cwd.clone(), ); - let session = context - .backend + let session = environment + .get_exec_backend() .start(ExecParams { metadata: Default::default(), process_id: ProcessId::from("proc-restricted-helper"), - argv: vec!["/bin/cat".to_string(), file.to_string_lossy().into_owned()], + argv: vec![ + "/bin/sh".to_owned(), "-c".to_owned(), + r#"cat "$1" "$CURL_CA_BUNDLE" && ! cat "$SSL_CERT_FILE" 2>/dev/null"#.to_owned(), + "sh".to_owned(), file.to_string_lossy().into_owned(), + ], cwd, shell_snapshot: None, env_policy: /*env_policy*/ None, - env: HashMap::from([("PATH".to_string(), std::env::var("PATH")?)]), + env: HashMap::from([ + ("PATH".to_owned(), std::env::var("PATH")?), + ("CURL_CA_BUNDLE".to_owned(), bundle.to_string_lossy().into_owned()), + ("SSL_CERT_FILE".to_owned(), untrusted.to_string_lossy().into_owned()), + ]), tty: false, pipe_stdin: false, arg0: None, sandbox: Some(sandbox), enforce_managed_network: false, - managed_network: None, + managed_network: Some(codex_network_proxy::ManagedNetworkSandboxContext::default()), network_proxy: None, }) .await?; @@ -1332,7 +1349,7 @@ async fn remote_process_keeps_sandbox_helper_visible_with_restricted_reads() -> assert_eq!( output, - ("allowed".to_string(), String::new(), Some(0), true) + ("allowedabc".to_string(), String::new(), Some(0), true) ); Ok(()) } diff --git a/codex-rs/sandboxing/src/launch.rs b/codex-rs/sandboxing/src/launch.rs index b126c0a3f1..6df3c45456 100644 --- a/codex-rs/sandboxing/src/launch.rs +++ b/codex-rs/sandboxing/src/launch.rs @@ -70,11 +70,18 @@ impl PreparedLaunchPolicy { permissions: PermissionProfile, policy_cwd: &PathUri, roots: Option<&[PathUri]>, + runtime_readable_paths: &[AbsolutePathBuf], ) -> io::Result { let Some(roots) = roots else { + if !runtime_readable_paths.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "runtime read access requires explicit workspace roots", + )); + } return Ok(Self { permissions }); }; - native_path(policy_cwd, "sandbox policy cwd")?; + let cwd = native_path(policy_cwd, "sandbox policy cwd")?; let workspace_roots = native_workspace_roots(roots)?; let (file_system, network) = permissions.to_runtime_permissions(); for entry in &file_system.entries { @@ -82,8 +89,9 @@ impl PreparedLaunchPolicy { native_path(path, "sandbox permission path")?; } } - let file_system = - file_system.materialize_project_roots_with_workspace_roots(&workspace_roots); + let file_system = file_system + .materialize_project_roots_with_workspace_roots(&workspace_roots) + .with_additional_readable_roots(&cwd, runtime_readable_paths); let permissions = PermissionProfile::from_runtime_permissions_with_enforcement( permissions.enforcement(), &file_system, diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index b9ce18baf9..6e3345afc2 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -34,6 +34,7 @@ pub use manager::SandboxCommand; pub use manager::SandboxDirectSpawnTransformRequest; pub use manager::SandboxExecRequest; pub use manager::SandboxManager; +pub use manager::SandboxSelection; pub use manager::SandboxTransformError; pub use manager::SandboxTransformRequest; pub use manager::SandboxType; @@ -80,6 +81,9 @@ impl From for CodexErr { | error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. } => { CodexErr::InvalidRequest(error.to_string()) } + error @ SandboxTransformError::SandboxUnavailable => { + CodexErr::UnsupportedOperation(error.to_string()) + } SandboxTransformError::MissingLinuxSandboxExecutable => { CodexErr::LandlockSandboxExecutableNotProvided } diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index a192aa524e..cfbf64aa3c 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -115,6 +115,15 @@ pub struct SandboxExecRequest { pub arg0: Option, } +/// How the execution host selects a backend for this launch. +#[derive(Clone, Copy)] +pub enum SandboxSelection { + /// Use the selected backend, including an explicitly native launch. + Explicit(SandboxType), + /// Refuse the launch if this host cannot provide its platform sandbox. + RequirePlatform, +} + /// Bundled arguments for sandbox transformation. /// /// This keeps call sites self-documenting when several fields are optional. @@ -123,7 +132,9 @@ pub struct SandboxTransformRequest<'a> { pub permissions: &'a PermissionProfile, /// Bind symbolic workspace rules to these executor roots. `None` means they are already resolved. pub workspace_roots: Option<&'a [PathUri]>, - pub sandbox: SandboxType, + /// Executor-selected helper binaries and validated trust bundles needed by the child. + pub runtime_readable_paths: &'a [AbsolutePathBuf], + pub selection: SandboxSelection, pub enforce_managed_network: bool, pub environment_id: Option<&'a str>, // TODO(viyatb): Evaluate switching this to Option> @@ -200,6 +211,7 @@ pub enum SandboxTransformError { source: io::Error, }, MissingLinuxSandboxExecutable, + SandboxUnavailable, WindowsMxcPreparation(String), EnvironmentNetworkProxy(String), #[cfg(target_os = "macos")] @@ -226,6 +238,9 @@ impl std::fmt::Display for SandboxTransformError { f, "sandbox policy cwd URI `{cwd}` is not valid on this host: {source}" ), + Self::SandboxUnavailable => { + write!(f, "sandbox intent cannot be enforced on this executor") + } Self::MissingLinuxSandboxExecutable => { write!(f, "missing codex-linux-sandbox executable path") } @@ -257,7 +272,7 @@ impl std::error::Error for SandboxTransformError { Self::InvalidPolicy(source) => Some(source), Self::InvalidCommandCwd { source, .. } | Self::InvalidSandboxPolicyCwd { source, .. } => Some(source), - Self::MissingLinuxSandboxExecutable => None, + Self::MissingLinuxSandboxExecutable | Self::SandboxUnavailable => None, Self::WindowsMxcPreparation(_) => None, Self::EnvironmentNetworkProxy(_) => None, #[cfg(target_os = "macos")] @@ -375,8 +390,29 @@ impl SandboxManager { &self, request: SandboxTransformRequest<'_>, ) -> Result { + let command = request.command; + let permissions = effective_permission_profile( + request.permissions, + command.additional_permissions.as_ref(), + ); + let policy = PreparedLaunchPolicy::resolve( + permissions, + request.sandbox_policy_cwd, + request.workspace_roots, + request.runtime_readable_paths, + ) + .map_err(SandboxTransformError::InvalidPolicy)?; + let backend = match request.selection { + SandboxSelection::Explicit(sandbox) => sandbox, + SandboxSelection::RequirePlatform => { + #[cfg(windows)] + crate::windows_mxc::record_availability_once(); + get_platform_sandbox(request.windows_sandbox_level != WindowsSandboxLevel::Disabled) + .ok_or(SandboxTransformError::SandboxUnavailable)? + } + }; #[cfg(windows)] - if request.sandbox == SandboxType::WindowsRestrictedToken + if backend == SandboxType::WindowsRestrictedToken && request.enforce_managed_network && request.windows_sandbox_level != WindowsSandboxLevel::Elevated { @@ -384,8 +420,6 @@ impl SandboxManager { "managed networking requires the elevated Windows sandbox backend".into(), )); } - let backend = request.sandbox; - let command = request.command; let native_program = match backend { SandboxType::None => Some(command.program.as_os_str()), SandboxType::LinuxSeccomp | SandboxType::WindowsMxc => { @@ -405,16 +439,6 @@ impl SandboxManager { 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 policy = PreparedLaunchPolicy::resolve( - permissions, - request.sandbox_policy_cwd, - request.workspace_roots, - ) - .map_err(SandboxTransformError::InvalidPolicy)?; let mut argv = Vec::with_capacity(1 + command.args.len()); argv.push(command.program.to_string_lossy().into_owned()); argv.extend(command.args); diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 990b7c2e7b..11162adeaf 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -2,6 +2,7 @@ use super::SandboxCommand; #[cfg(windows)] use super::SandboxDirectSpawnTransformRequest; use super::SandboxManager; +use super::SandboxSelection; use super::SandboxTransformRequest; use super::SandboxType; use super::SandboxablePreference; @@ -116,7 +117,8 @@ fn unsandboxed_transform_preserves_foreign_cwd_and_unrestricted_file_system_poli }, permissions: &permissions, workspace_roots: None, - sandbox: SandboxType::None, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::None), enforce_managed_network: false, environment_id: Some("remote"), network: None, @@ -178,7 +180,8 @@ fn symlinked_workspace_reports_seatbelt_preparation_error() { }, permissions: &permissions, workspace_roots: None, - sandbox: SandboxType::MacosSeatbelt, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::MacosSeatbelt), enforce_managed_network: false, environment_id: None, network: None, @@ -234,7 +237,8 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { }, permissions: &permissions, workspace_roots: None, - sandbox: SandboxType::None, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::None), enforce_managed_network: false, environment_id: None, network: None, @@ -323,7 +327,8 @@ fn transform_additional_permissions_preserves_denied_entries() { }, permissions: &permissions, workspace_roots: Some(roots), - sandbox: SandboxType::None, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::None), enforce_managed_network: false, network: None, environment_id: None, @@ -409,7 +414,8 @@ fn transform_linux_seccomp_request( }, permissions: &permissions, workspace_roots: None, - sandbox: SandboxType::LinuxSeccomp, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::LinuxSeccomp), enforce_managed_network: false, environment_id: None, network: None, @@ -670,7 +676,8 @@ async fn linux_unix_socket_grant_uses_effective_managed_policy() -> anyhow::Resu }, permissions: &PermissionProfile::Disabled, workspace_roots: None, - sandbox: SandboxType::LinuxSeccomp, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::LinuxSeccomp), enforce_managed_network, environment_id: None, network: live_proxy.then_some(&network), @@ -817,7 +824,8 @@ fn transform_for_direct_spawn_windows_materializes_inner_helper() { }, permissions: &permissions, workspace_roots: roots, - sandbox: SandboxType::WindowsRestrictedToken, + runtime_readable_paths: &[], + selection: SandboxSelection::Explicit(SandboxType::WindowsRestrictedToken), enforce_managed_network: false, environment_id: None, network: None,