From a69385f63fca2d2bfa794df9f7a17ba2f5847323 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Wed, 8 Apr 2026 10:53:18 -0700 Subject: [PATCH] Address exec-server sandbox review comments - preserve sandbox child environment variables from shared launch requests - dispatch codex-exec-server through codex-arg0 so helper arg0 is executable - send sandbox preference to remote servers and report the server-selected sandbox Co-authored-by: Codex --- codex-rs/core/src/exec.rs | 2 +- codex-rs/core/src/sandboxing/mod.rs | 18 ++------ codex-rs/core/src/tools/js_repl/mod.rs | 2 +- .../tools/runtimes/shell/unix_escalation.rs | 2 +- .../core/src/tools/runtimes/unified_exec.rs | 6 +-- codex-rs/core/src/tools/sandboxing.rs | 2 +- .../core/src/unified_exec/process_manager.rs | 6 +-- codex-rs/exec-server/Cargo.toml | 2 +- .../exec-server/src/bin/codex-exec-server.rs | 35 ++++----------- codex-rs/exec-server/src/lib.rs | 2 + codex-rs/exec-server/src/local_process.rs | 32 ++++++++----- codex-rs/exec-server/src/protocol.rs | 2 + codex-rs/exec-server/src/remote_process.rs | 9 ++-- codex-rs/exec-server/src/server.rs | 9 +++- codex-rs/exec-server/src/server/handler.rs | 8 +++- .../exec-server/src/server/handler/tests.rs | 8 ++-- .../exec-server/src/server/process_handler.rs | 8 +++- codex-rs/exec-server/src/server/processor.rs | 8 +++- codex-rs/exec-server/src/server/transport.rs | 17 ++++--- codex-rs/exec-server/tests/exec_process.rs | 14 +----- codex-rs/exec-server/tests/process.rs | 1 + codex-rs/sandboxing/src/manager.rs | 45 ++++++++++++++++--- codex-rs/sandboxing/src/manager_tests.rs | 12 ++--- 23 files changed, 140 insertions(+), 110 deletions(-) diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index fc12a6255a..464f9215ac 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -284,7 +284,7 @@ pub fn build_exec_request( capture_policy, }; let sandbox_launch_config = SandboxLaunchConfig { - sandbox: sandbox_type, + sandbox_preference: SandboxablePreference::from_selected_sandbox(sandbox_type), policy: sandbox_policy.clone(), file_system_policy: file_system_sandbox_policy.clone(), network_policy: network_sandbox_policy, diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index d241fa8266..76845c9f3f 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -12,9 +12,6 @@ use crate::exec::ExecExpiration; use crate::exec::StdoutStream; use crate::exec::WindowsRestrictedTokenFilesystemOverlay; use crate::exec::execute_exec_request; -#[cfg(target_os = "macos")] -use crate::spawn::CODEX_SANDBOX_ENV_VAR; -use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::exec_output::ExecToolCallOutput; @@ -88,13 +85,14 @@ impl ExecRequest { } pub(crate) fn from_sandbox_exec_request( - request: SandboxExecRequest, + mut request: SandboxExecRequest, options: ExecOptions, ) -> Self { + request.prepare_env_for_spawn(); let SandboxExecRequest { command, cwd, - mut env, + env, network, sandbox, windows_sandbox_level, @@ -108,16 +106,6 @@ impl ExecRequest { expiration, capture_policy, } = options; - if !network_sandbox_policy.is_enabled() { - env.insert( - CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR.to_string(), - "1".to_string(), - ); - } - #[cfg(target_os = "macos")] - if sandbox == SandboxType::MacosSeatbelt { - env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); - } Self { command, cwd, diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index ac1f4248b7..d4b509954b 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1059,7 +1059,7 @@ impl JsReplManager { capture_policy: ExecCapturePolicy::ShellTool, }; let sandbox_launch_config = SandboxLaunchConfig { - sandbox: sandbox_type, + sandbox_preference: SandboxablePreference::from_selected_sandbox(sandbox_type), policy: turn.sandbox_policy.get().clone(), file_system_policy: turn.file_system_sandbox_policy.clone(), network_policy: turn.network_sandbox_policy, 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 0010f093f9..5d1542362d 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -836,7 +836,7 @@ impl CoreShellCommandExecutor { capture_policy: ExecCapturePolicy::ShellTool, }; let sandbox_launch_config = SandboxLaunchConfig { - sandbox, + sandbox_preference: SandboxablePreference::from_selected_sandbox(sandbox), policy: sandbox_policy.clone(), file_system_policy: file_system_sandbox_policy.clone(), network_policy: network_sandbox_policy, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 9b6c97acff..35172976fc 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -87,12 +87,8 @@ fn build_remote_exec_sandbox_config( attempt: &SandboxAttempt<'_>, additional_permissions: Option, ) -> SandboxLaunchConfig { - if matches!(attempt.sandbox, codex_sandboxing::SandboxType::None) { - return SandboxLaunchConfig::no_sandbox(attempt.sandbox_cwd.to_path_buf()); - } - SandboxLaunchConfig { - sandbox: attempt.sandbox, + sandbox_preference: SandboxablePreference::from_selected_sandbox(attempt.sandbox), policy: attempt.policy.clone(), file_system_policy: attempt.file_system_policy.clone(), network_policy: attempt.network_policy, diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 9d86a823b1..ce992c5595 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -339,7 +339,7 @@ impl<'a> SandboxAttempt<'a> { network: Option<&NetworkProxy>, ) -> Result { let sandbox_launch_config = SandboxLaunchConfig { - sandbox: self.sandbox, + sandbox_preference: SandboxablePreference::from_selected_sandbox(self.sandbox), policy: self.policy.clone(), file_system_policy: self.file_system_policy.clone(), network_policy: self.network_policy, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 8341182e8b..9492bdb5e4 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -615,8 +615,8 @@ impl UnifiedExecProcessManager { }) .await .map_err(|err| UnifiedExecError::create_process(err.to_string()))?; - return UnifiedExecProcess::from_remote_started(started, request.sandbox).await; - } + return UnifiedExecProcess::from_remote_started(started, request.sandbox).await; + } let spawn_result = if tty { codex_utils_pty::pty::spawn_process_with_inherited_fds( @@ -651,12 +651,12 @@ impl UnifiedExecProcessManager { params: codex_exec_server::ExecParams, environment: &codex_exec_server::Environment, ) -> Result { - let sandbox_type = params.sandbox.sandbox; let started = environment .get_exec_backend() .start(params) .await .map_err(|err| UnifiedExecError::create_process(err.to_string()))?; + let sandbox_type = started.sandbox_type; UnifiedExecProcess::from_remote_started(started, sandbox_type).await } diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 3fe93cad3f..17fd939a8b 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -20,7 +20,7 @@ arc-swap = { workspace = true } async-trait = { workspace = true } base64 = { workspace = true } clap = { workspace = true, features = ["derive"] } -codex-protocol = { workspace = true } +codex-arg0 = { workspace = true } codex-sandboxing = { workspace = true } codex-app-server-protocol = { workspace = true } codex-protocol = { workspace = true } diff --git a/codex-rs/exec-server/src/bin/codex-exec-server.rs b/codex-rs/exec-server/src/bin/codex-exec-server.rs index 6ecdb67a47..e5e121e5c8 100644 --- a/codex-rs/exec-server/src/bin/codex-exec-server.rs +++ b/codex-rs/exec-server/src/bin/codex-exec-server.rs @@ -1,9 +1,6 @@ -#[cfg(target_os = "linux")] -use std::path::Path; - use clap::Parser; -#[cfg(target_os = "linux")] -use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; +use codex_arg0::Arg0DispatchPaths; +use codex_arg0::arg0_dispatch_or_else; #[derive(Debug, Parser)] struct ExecServerArgs { @@ -17,29 +14,13 @@ struct ExecServerArgs { } fn main() -> anyhow::Result<()> { - dispatch_arg0(); - - let runtime = tokio::runtime::Runtime::new()?; - runtime.block_on(async { + arg0_dispatch_or_else(|arg0_paths: Arg0DispatchPaths| async move { let args = ExecServerArgs::parse(); - codex_exec_server::run_main_with_listen_url(&args.listen) + let runtime = + codex_exec_server::ExecServerRuntimeConfig::new(arg0_paths.codex_linux_sandbox_exe); + codex_exec_server::run_main_with_runtime(&args.listen, runtime) .await - .map_err(|err| anyhow::Error::msg(err.to_string())) + .map_err(|err| anyhow::Error::msg(err.to_string()))?; + Ok(()) }) } - -#[cfg(target_os = "linux")] -fn dispatch_arg0() { - let argv0 = std::env::args_os().next().unwrap_or_default(); - let exe_name = Path::new(&argv0) - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or_default(); - - if exe_name == CODEX_LINUX_SANDBOX_ARG0 { - codex_linux_sandbox::run_main(); - } -} - -#[cfg(not(target_os = "linux"))] -fn dispatch_arg0() {} diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index eccb9c91bb..1cd38ac0ef 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -28,6 +28,7 @@ pub use file_system::FileSystemResult; pub use file_system::ReadDirectoryEntry; pub use file_system::RemoveOptions; pub use local_file_system::LOCAL_FS; +pub use local_process::ExecServerRuntimeConfig; pub use process::ExecBackend; pub use process::ExecProcess; pub use process::StartedExecProcess; @@ -66,3 +67,4 @@ pub use server::DEFAULT_LISTEN_URL; pub use server::ExecServerListenUrlParseError; pub use server::run_main; pub use server::run_main_with_listen_url; +pub use server::run_main_with_runtime; diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 09f6a695d6..b9943327a3 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -98,22 +98,20 @@ struct LocalExecProcess { } #[derive(Clone, Debug, Default)] -struct ExecServerRuntimeConfig { +pub struct ExecServerRuntimeConfig { codex_linux_sandbox_exe: Option, } impl ExecServerRuntimeConfig { - fn detect() -> Self { + pub fn new(codex_linux_sandbox_exe: Option) -> Self { Self { - // The Codex CLI and codex-exec-server both dispatch the Linux - // sandbox helper from their own executable via argv[0]. - codex_linux_sandbox_exe: if cfg!(target_os = "linux") { - std::env::current_exe().ok() - } else { - None - }, + codex_linux_sandbox_exe, } } + + pub fn detect() -> Self { + Self::default() + } } struct StartedProcess { @@ -133,13 +131,20 @@ impl Default for LocalProcess { impl LocalProcess { pub(crate) fn new(notifications: RpcNotificationSender) -> Self { + Self::new_with_runtime(notifications, ExecServerRuntimeConfig::detect()) + } + + pub(crate) fn new_with_runtime( + notifications: RpcNotificationSender, + runtime: ExecServerRuntimeConfig, + ) -> Self { Self { inner: Arc::new(Inner { notifications, processes: Mutex::new(HashMap::new()), initialize_requested: AtomicBool::new(false), initialized: AtomicBool::new(false), - runtime: ExecServerRuntimeConfig::detect(), + runtime, }), } } @@ -306,6 +311,7 @@ impl LocalProcess { .await .map(|started| ExecResponse { process_id: started.process_id, + sandbox: started.sandbox_type, }) } @@ -523,7 +529,7 @@ fn prepare_exec_launch( ¶ms.env, params.sandbox.additional_permissions.clone(), )?; - params + let mut launch = params .sandbox .transform( command, @@ -533,7 +539,9 @@ fn prepare_exec_launch( None, runtime.codex_linux_sandbox_exe.as_ref(), ) - .map_err(|err| internal_error(format!("failed to build sandbox launch: {err}"))) + .map_err(|err| internal_error(format!("failed to build sandbox launch: {err}")))?; + launch.prepare_env_for_spawn(); + Ok(launch) } impl LocalProcess { diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 9fc351817c..86aaddb51f 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxLaunchConfig; +use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; @@ -71,6 +72,7 @@ pub struct ExecParams { #[serde(rename_all = "camelCase")] pub struct ExecResponse { pub process_id: ProcessId, + pub sandbox: SandboxType, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/remote_process.rs b/codex-rs/exec-server/src/remote_process.rs index 36813d56c4..3a88911e45 100644 --- a/codex-rs/exec-server/src/remote_process.rs +++ b/codex-rs/exec-server/src/remote_process.rs @@ -34,19 +34,18 @@ impl RemoteProcess { impl ExecBackend for RemoteProcess { async fn start(&self, params: ExecParams) -> Result { let process_id = params.process_id.clone(); - let sandbox_type = params.sandbox.sandbox; let session = self.client.register_session(&process_id).await?; - match self.client.exec(params).await { - Ok(_) => {} + let response = match self.client.exec(params).await { + Ok(response) => response, Err(err) => { session.unregister().await; return Err(err); } - } + }; Ok(StartedExecProcess { process: Arc::new(RemoteExecProcess { session }), - sandbox_type, + sandbox_type: response.sandbox, }) } } diff --git a/codex-rs/exec-server/src/server.rs b/codex-rs/exec-server/src/server.rs index 46de5aa497..56a5871673 100644 --- a/codex-rs/exec-server/src/server.rs +++ b/codex-rs/exec-server/src/server.rs @@ -16,5 +16,12 @@ pub async fn run_main() -> Result<(), Box> pub async fn run_main_with_listen_url( listen_url: &str, ) -> Result<(), Box> { - transport::run_transport(listen_url).await + transport::run_transport(listen_url, crate::ExecServerRuntimeConfig::detect()).await +} + +pub async fn run_main_with_runtime( + listen_url: &str, + runtime: crate::ExecServerRuntimeConfig, +) -> Result<(), Box> { + transport::run_transport(listen_url, runtime).await } diff --git a/codex-rs/exec-server/src/server/handler.rs b/codex-rs/exec-server/src/server/handler.rs index 39e26548b3..22cb996055 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -1,5 +1,6 @@ use codex_app_server_protocol::JSONRPCErrorError; +use crate::ExecServerRuntimeConfig; use crate::protocol::ExecParams; use crate::protocol::ExecResponse; use crate::protocol::FsCopyParams; @@ -34,9 +35,12 @@ pub(crate) struct ExecServerHandler { } impl ExecServerHandler { - pub(crate) fn new(notifications: RpcNotificationSender) -> Self { + pub(crate) fn new( + notifications: RpcNotificationSender, + runtime: ExecServerRuntimeConfig, + ) -> Self { Self { - process: ProcessHandler::new(notifications), + process: ProcessHandler::new(notifications, runtime), file_system: FileSystemHandler::default(), } } diff --git a/codex-rs/exec-server/src/server/handler/tests.rs b/codex-rs/exec-server/src/server/handler/tests.rs index 42e4866cba..4e571e6c83 100644 --- a/codex-rs/exec-server/src/server/handler/tests.rs +++ b/codex-rs/exec-server/src/server/handler/tests.rs @@ -6,6 +6,7 @@ use pretty_assertions::assert_eq; use tokio::sync::mpsc; use super::ExecServerHandler; +use crate::ExecServerRuntimeConfig; use crate::ProcessId; use crate::protocol::ExecParams; use crate::protocol::InitializeResponse; @@ -36,9 +37,10 @@ fn exec_params(process_id: &str) -> ExecParams { async fn initialized_handler() -> Arc { let (outgoing_tx, _outgoing_rx) = mpsc::channel(16); - let handler = Arc::new(ExecServerHandler::new(RpcNotificationSender::new( - outgoing_tx, - ))); + let handler = Arc::new(ExecServerHandler::new( + RpcNotificationSender::new(outgoing_tx), + ExecServerRuntimeConfig::detect(), + )); assert_eq!( handler.initialize().expect("initialize"), InitializeResponse {} diff --git a/codex-rs/exec-server/src/server/process_handler.rs b/codex-rs/exec-server/src/server/process_handler.rs index 6f22890d35..c25ff32423 100644 --- a/codex-rs/exec-server/src/server/process_handler.rs +++ b/codex-rs/exec-server/src/server/process_handler.rs @@ -1,5 +1,6 @@ use codex_app_server_protocol::JSONRPCErrorError; +use crate::ExecServerRuntimeConfig; use crate::local_process::LocalProcess; use crate::protocol::ExecParams; use crate::protocol::ExecResponse; @@ -18,9 +19,12 @@ pub(crate) struct ProcessHandler { } impl ProcessHandler { - pub(crate) fn new(notifications: RpcNotificationSender) -> Self { + pub(crate) fn new( + notifications: RpcNotificationSender, + runtime: ExecServerRuntimeConfig, + ) -> Self { Self { - process: LocalProcess::new(notifications), + process: LocalProcess::new_with_runtime(notifications, runtime), } } diff --git a/codex-rs/exec-server/src/server/processor.rs b/codex-rs/exec-server/src/server/processor.rs index 518a1a78e0..4f35d56682 100644 --- a/codex-rs/exec-server/src/server/processor.rs +++ b/codex-rs/exec-server/src/server/processor.rs @@ -4,6 +4,7 @@ use tokio::sync::mpsc; use tracing::debug; use tracing::warn; +use crate::ExecServerRuntimeConfig; use crate::connection::CHANNEL_CAPACITY; use crate::connection::JsonRpcConnection; use crate::connection::JsonRpcConnectionEvent; @@ -15,13 +16,16 @@ use crate::rpc::method_not_found; use crate::server::ExecServerHandler; use crate::server::registry::build_router; -pub(crate) async fn run_connection(connection: JsonRpcConnection) { +pub(crate) async fn run_connection( + connection: JsonRpcConnection, + runtime: ExecServerRuntimeConfig, +) { let router = Arc::new(build_router()); let (json_outgoing_tx, mut incoming_rx, connection_tasks) = connection.into_parts(); let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(CHANNEL_CAPACITY); let notifications = RpcNotificationSender::new(outgoing_tx.clone()); - let handler = Arc::new(ExecServerHandler::new(notifications)); + let handler = Arc::new(ExecServerHandler::new(notifications, runtime)); let outbound_task = tokio::spawn(async move { while let Some(message) = outgoing_rx.recv().await { diff --git a/codex-rs/exec-server/src/server/transport.rs b/codex-rs/exec-server/src/server/transport.rs index 4726465cc0..caeaf4decc 100644 --- a/codex-rs/exec-server/src/server/transport.rs +++ b/codex-rs/exec-server/src/server/transport.rs @@ -4,6 +4,7 @@ use tokio::net::TcpListener; use tokio_tungstenite::accept_async; use tracing::warn; +use crate::ExecServerRuntimeConfig; use crate::connection::JsonRpcConnection; use crate::server::processor::run_connection; @@ -48,13 +49,15 @@ pub(crate) fn parse_listen_url( pub(crate) async fn run_transport( listen_url: &str, + runtime: ExecServerRuntimeConfig, ) -> Result<(), Box> { let bind_address = parse_listen_url(listen_url)?; - run_websocket_listener(bind_address).await + run_websocket_listener(bind_address, runtime).await } async fn run_websocket_listener( bind_address: SocketAddr, + runtime: ExecServerRuntimeConfig, ) -> Result<(), Box> { let listener = TcpListener::bind(bind_address).await?; let local_addr = listener.local_addr()?; @@ -63,13 +66,17 @@ async fn run_websocket_listener( loop { let (stream, peer_addr) = listener.accept().await?; + let runtime = runtime.clone(); tokio::spawn(async move { match accept_async(stream).await { Ok(websocket) => { - run_connection(JsonRpcConnection::from_websocket( - websocket, - format!("exec-server websocket {peer_addr}"), - )) + run_connection( + JsonRpcConnection::from_websocket( + websocket, + format!("exec-server websocket {peer_addr}"), + ), + runtime, + ) .await; } Err(err) => { diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index f411b7a0f8..927e191a65 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -17,7 +17,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxLaunchConfig; -use codex_sandboxing::SandboxType; +use codex_sandboxing::SandboxablePreference; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; @@ -224,16 +224,6 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe( Ok(()) } -fn platform_sandbox_type() -> SandboxType { - if cfg!(target_os = "macos") { - SandboxType::MacosSeatbelt - } else if cfg!(target_os = "linux") { - SandboxType::LinuxSeccomp - } else { - unreachable!("unix exec-server tests only run on macOS and Linux"); - } -} - fn write_outside_workspace_sandbox(workspace_root: &std::path::Path) -> SandboxLaunchConfig { let mut policy = SandboxPolicy::new_workspace_write_policy(); if let SandboxPolicy::WorkspaceWrite { @@ -246,7 +236,7 @@ fn write_outside_workspace_sandbox(workspace_root: &std::path::Path) -> SandboxL *exclude_slash_tmp = true; } SandboxLaunchConfig { - sandbox: platform_sandbox_type(), + sandbox_preference: SandboxablePreference::Require, policy: policy.clone(), file_system_policy: FileSystemSandboxPolicy::from_legacy_sandbox_policy( &policy, diff --git a/codex-rs/exec-server/tests/process.rs b/codex-rs/exec-server/tests/process.rs index 456078b279..a528aa4c9b 100644 --- a/codex-rs/exec-server/tests/process.rs +++ b/codex-rs/exec-server/tests/process.rs @@ -75,6 +75,7 @@ async fn exec_server_starts_process_over_websocket() -> anyhow::Result<()> { process_start_response, ExecResponse { process_id: ProcessId::from("proc-1"), + sandbox: codex_sandboxing::SandboxType::None, } ); diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index e3a5490fcc..08460f5c2c 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -43,17 +43,29 @@ impl SandboxType { } } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] pub enum SandboxablePreference { Auto, Require, Forbid, } +impl SandboxablePreference { + pub fn from_selected_sandbox(sandbox: SandboxType) -> Self { + match sandbox { + SandboxType::None => Self::Forbid, + SandboxType::MacosSeatbelt + | SandboxType::LinuxSeccomp + | SandboxType::WindowsRestrictedToken => Self::Require, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SandboxLaunchConfig { - pub sandbox: SandboxType, + pub sandbox_preference: SandboxablePreference, pub policy: SandboxPolicy, pub file_system_policy: FileSystemSandboxPolicy, pub network_policy: NetworkSandboxPolicy, @@ -68,7 +80,7 @@ pub struct SandboxLaunchConfig { impl SandboxLaunchConfig { pub fn no_sandbox(sandbox_policy_cwd: PathBuf) -> Self { Self { - sandbox: SandboxType::None, + sandbox_preference: SandboxablePreference::Forbid, policy: SandboxPolicy::DangerFullAccess, file_system_policy: FileSystemSandboxPolicy::unrestricted(), network_policy: NetworkSandboxPolicy::Enabled, @@ -131,6 +143,22 @@ pub struct SandboxExecRequest { pub arg0: Option, } +impl SandboxExecRequest { + pub fn prepare_env_for_spawn(&mut self) { + if !self.network_sandbox_policy.is_enabled() { + self.env.insert( + "CODEX_SANDBOX_NETWORK_DISABLED".to_string(), + "1".to_string(), + ); + } + #[cfg(target_os = "macos")] + if self.sandbox == SandboxType::MacosSeatbelt { + self.env + .insert("CODEX_SANDBOX".to_string(), "seatbelt".to_string()); + } + } +} + #[derive(Debug)] pub enum SandboxTransformError { MissingLinuxSandboxExecutable, @@ -208,11 +236,18 @@ impl SandboxManager { launch.network_policy, additional_permissions.as_ref(), ); + let sandbox = self.select_initial( + &effective_file_system_policy, + effective_network_policy, + launch.sandbox_preference, + launch.windows_sandbox_level, + launch.enforce_managed_network, + ); let mut argv = Vec::with_capacity(1 + command.args.len()); argv.push(command.program); argv.extend(command.args.into_iter().map(OsString::from)); - let (argv, arg0_override) = match launch.sandbox { + let (argv, arg0_override) = match sandbox { SandboxType::None => (os_argv_to_strings(argv), None), #[cfg(target_os = "macos")] SandboxType::MacosSeatbelt => { @@ -261,7 +296,7 @@ impl SandboxManager { cwd: command.cwd, env: command.env, network: network.cloned(), - sandbox: launch.sandbox, + sandbox, windows_sandbox_level: launch.windows_sandbox_level, windows_sandbox_private_desktop: launch.windows_sandbox_private_desktop, sandbox_policy: effective_policy, diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index ab652a0b04..60d2a8bfdf 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -29,10 +29,10 @@ fn test_launch_config( policy: SandboxPolicy, file_system_policy: FileSystemSandboxPolicy, network_policy: NetworkSandboxPolicy, - sandbox: SandboxType, + sandbox_preference: SandboxablePreference, ) -> SandboxLaunchConfig { SandboxLaunchConfig { - sandbox, + sandbox_preference, policy, file_system_policy, network_policy, @@ -113,7 +113,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() }, FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Restricted, - SandboxType::None, + SandboxablePreference::Forbid, ), /*network*/ None, /*codex_linux_sandbox_exe*/ None, @@ -163,7 +163,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { }, FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Restricted, - SandboxType::None, + SandboxablePreference::Forbid, ), /*network*/ None, /*codex_linux_sandbox_exe*/ None, @@ -229,7 +229,7 @@ fn transform_additional_permissions_preserves_denied_entries() { }, ]), NetworkSandboxPolicy::Restricted, - SandboxType::None, + SandboxablePreference::Forbid, ), /*network*/ None, /*codex_linux_sandbox_exe*/ None, @@ -281,7 +281,7 @@ fn transform_linux_seccomp_request( SandboxPolicy::DangerFullAccess, FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Enabled, - SandboxType::LinuxSeccomp, + SandboxablePreference::Require, ), /*network*/ None, Some(codex_linux_sandbox_exe),