From 04483f4ce5694d471e471583d4ca286908d7c8b7 Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 19 Jun 2026 16:05:51 +0100 Subject: [PATCH] Keep remote exec commands native to the executor (#29099) ## Summary - Remote unified-exec now sends the original command argv to exec-server instead of materializing the orchestrator's sandbox wrapper first. - Local unified-exec keeps the existing sandbox path unchanged. - Add a focused regression test for a macOS-selected sandbox producing plain remote argv. Before: macOS orchestrator -> /usr/bin/sandbox-exec ... -> Linux exec-server After: macOS orchestrator -> /bin/bash -lc pwd -> Linux exec-server This is intentionally only the first cleanup step. Remote unified-exec commands are sent without a process sandbox until the targeted follow-ups below land. For the macOS-to-Linux path this is not a practical regression: the old sandboxed attempt failed before process launch because the Linux executor could not spawn macOS sandbox paths. ## Targeted follow-ups 1. Carry sandbox intent separately from argv. - Add an optional sandbox field to exec-server process params. - Reuse FileSystemSandboxContext rather than introducing a new sandbox model. - Carry managed-network enforcement as one explicit bit. - Keep argv plain. 2. Apply that intent inside exec-server. - Add a small process-start adapter before LocalProcess::exec. - Reuse the existing codex-sandboxing SandboxManager and exec-server runtime paths. - Follow the same shape already used by exec-server filesystem sandboxing. - Do not duplicate or move the sandbox implementations. 3. Report the sandbox actually used. - Return the executor-selected sandbox type from process/start. - Use that value in core for sandbox-denial detection and retry behavior. ## End state The orchestrator sends plain commands plus portable sandbox intent. The executor chooses and applies its own native sandbox: Linux executors use Linux sandboxing, macOS executors use Seatbelt, and Windows executors use Windows sandboxing. Concrete wrapper argv, helper paths, and sandbox env markers never cross the executor boundary. --- .../core/src/tools/runtimes/unified_exec.rs | 27 +++------ codex-rs/core/src/tools/sandboxing.rs | 31 ++++++++++ codex-rs/core/src/tools/sandboxing_tests.rs | 60 +++++++++++++++++++ codex-rs/core/src/unified_exec/mod_tests.rs | 8 +-- .../core/src/unified_exec/process_manager.rs | 44 ++++++++++++++ 5 files changed, 146 insertions(+), 24 deletions(-) diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 0d6c93b4d..c1d519120 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -421,7 +421,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt } return self .manager - .open_session_with_exec_env( + .open_session_with_prepared_exec_env( req.process_id, &prepared.exec_request, req.tty, @@ -459,33 +459,20 @@ impl<'a> ToolRuntime for UnifiedExecRunt error @ ToolError::Codex(_) => error, })?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); - let mut exec_env = attempt - .env_for( - command, - options, - managed_network, - Some(&req.turn_environment.environment_id), - ) - .map_err(ToolError::Codex)?; - exec_env.exec_server_env_config = req.exec_server_env_config.clone(); self.manager .open_session_with_exec_env( req.process_id, - &exec_env, + command, + options, + attempt, + managed_network, + /*environment_id*/ Some(&req.turn_environment.environment_id), + req.exec_server_env_config.clone(), req.tty, Box::new(NoopSpawnLifecycle), req.turn_environment.environment.as_ref(), ) .await - .map_err(|err| match err { - UnifiedExecError::SandboxDenied { output, .. } => { - ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output: Box::new(output), - network_policy_decision: None, - })) - } - other => ToolError::Rejected(other.to_string()), - }) } } diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 48c06413b..847a3c758 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -452,6 +452,37 @@ impl<'a> SandboxAttempt<'a> { self.workspace_roots.to_vec(), )) } + + pub fn env_for_exec_server( + &self, + command: SandboxCommand, + options: ExecOptions, + network: Option<&NetworkProxy>, + environment_id: Option<&str>, + ) -> Result { + let request = self + .manager + .transform(SandboxTransformRequest { + command, + permissions: self.permissions, + // 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, + network, + sandbox_policy_cwd: self.sandbox_cwd, + codex_linux_sandbox_exe: None, + use_legacy_landlock: self.use_legacy_landlock, + windows_sandbox_level: self.windows_sandbox_level, + windows_sandbox_private_desktop: self.windows_sandbox_private_desktop, + }) + .map_err(CodexErr::from)?; + Ok(crate::sandboxing::ExecRequest::from_sandbox_exec_request( + request, + options, + self.workspace_roots.to_vec(), + )) + } } #[cfg(test)] diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 21719e281..f4d6e6ce4 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -4,9 +4,16 @@ use crate::tools::hook_names::HookToolName; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::GranularApprovalConfig; +use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxType; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use serde_json::json; +use std::collections::HashMap; #[test] fn bash_permission_request_payload_omits_missing_description() { @@ -193,3 +200,56 @@ fn deny_read_blocks_explicit_escalation_and_policy_bypass() { "exec-policy allow rules would drop deny-read filesystem policy, so keep the first attempt sandboxed", ); } + +#[test] +fn exec_server_env_keeps_command_native() { + let cwd: AbsolutePathBuf = std::env::current_dir() + .expect("current dir") + .try_into() + .expect("absolute cwd"); + let cwd_uri = PathUri::from_abs_path(&cwd); + let permissions = codex_protocol::models::PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::default(), + NetworkSandboxPolicy::Restricted, + ); + let manager = SandboxManager::new(); + let attempt = SandboxAttempt { + sandbox: SandboxType::MacosSeatbelt, + permissions: &permissions, + enforce_managed_network: false, + manager: &manager, + sandbox_cwd: &cwd_uri, + workspace_roots: std::slice::from_ref(&cwd), + codex_linux_sandbox_exe: None, + use_legacy_landlock: false, + windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, + windows_sandbox_private_desktop: false, + network_denial_cancellation_token: None, + }; + let command = SandboxCommand { + program: "/bin/bash".into(), + args: vec!["-lc".to_string(), "pwd".to_string()], + cwd: cwd_uri.clone(), + env: HashMap::new(), + additional_permissions: None, + }; + 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, /*network*/ None, Some("remote")) + .expect("prepare remote exec request"); + + assert_eq!( + request.command, + vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "pwd".to_string() + ] + ); + assert_eq!(request.arg0, None); + assert_eq!(request.sandbox, SandboxType::None); +} diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index 8e1b03d11..d5b813cb4 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -108,7 +108,7 @@ async fn exec_command_with_tty( let process = Arc::new( manager - .open_session_with_exec_env( + .open_session_with_prepared_exec_env( process_id, &request, tty, @@ -811,7 +811,7 @@ async fn completed_pipe_commands_preserve_exit_code() -> anyhow::Result<()> { let environment = codex_exec_server::Environment::default_for_tests(); let process = UnifiedExecProcessManager::default() - .open_session_with_exec_env( + .open_session_with_prepared_exec_env( /*process_id*/ 1234, &request, /*tty*/ false, @@ -853,7 +853,7 @@ async fn unified_exec_uses_remote_exec_server_when_configured() -> anyhow::Resul let manager = UnifiedExecProcessManager::default(); let process = manager - .open_session_with_exec_env( + .open_session_with_prepared_exec_env( /*process_id*/ 1234, &request, /*tty*/ true, @@ -910,7 +910,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() let manager = UnifiedExecProcessManager::default(); let err = manager - .open_session_with_exec_env( + .open_session_with_prepared_exec_env( /*process_id*/ 1234, &request, /*tty*/ true, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 0aac49182..fc5508eb8 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -15,6 +15,7 @@ use crate::codex_thread::BackgroundTerminalInfo; use crate::exec_env::CODEX_THREAD_ID_ENV_VAR; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; +use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::ExecServerEnvConfig; use crate::tools::context::ExecCommandToolOutput; @@ -26,6 +27,7 @@ use crate::tools::network_approval::finish_deferred_network_approval; use crate::tools::orchestrator::ToolOrchestrator; 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; @@ -50,10 +52,12 @@ use crate::unified_exec::process::OutputBuffer; use crate::unified_exec::process::OutputHandles; use crate::unified_exec::process::SpawnLifecycleHandle; use crate::unified_exec::process::UnifiedExecProcess; +use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::error::CodexErr; use codex_protocol::error::SandboxErr; use codex_protocol::protocol::ExecCommandSource; +use codex_sandboxing::SandboxCommand; use codex_tools::ToolName; use codex_utils_output_truncation::approx_token_count; use codex_utils_path_uri::PathUri; @@ -886,7 +890,47 @@ impl UnifiedExecProcessManager { ); } + #[allow(clippy::too_many_arguments)] pub(crate) async fn open_session_with_exec_env( + &self, + process_id: i32, + command: SandboxCommand, + options: ExecOptions, + attempt: &SandboxAttempt<'_>, + network: Option<&NetworkProxy>, + environment_id: Option<&str>, + exec_server_env_config: Option, + tty: bool, + spawn_lifecycle: SpawnLifecycleHandle, + environment: &codex_exec_server::Environment, + ) -> Result { + let mut request = if environment.is_remote() { + attempt.env_for_exec_server(command, options, network, environment_id) + } else { + attempt.env_for(command, options, network, environment_id) + } + .map_err(ToolError::Codex)?; + request.exec_server_env_config = exec_server_env_config; + self.open_session_with_prepared_exec_env( + process_id, + &request, + tty, + spawn_lifecycle, + environment, + ) + .await + .map_err(|err| match err { + UnifiedExecError::SandboxDenied { output, .. } => { + ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + network_policy_decision: None, + })) + } + other => ToolError::Rejected(other.to_string()), + }) + } + + pub(crate) async fn open_session_with_prepared_exec_env( &self, process_id: i32, request: &ExecRequest,