From d2efdf19861bf52782ff04e5fff83f5ef9f7ebf5 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 10 Nov 2026 03:50:45 +0000 Subject: [PATCH] Route sandbox CLI launches through the shared sandbox manager (#60517) ## What changed - Use `SandboxManager` to prepare `codex sandbox` commands, including permission profiles, workspace roots, and managed network proxy settings. - Return a `WrappedSandboxCommand` that can produce an execution request or a configured process command, preserving native executable paths and Linux helper `argv[0]`. Require the session launcher for native Windows token sandboxes. - Move macOS inherited-terminal policy handling into the shared manager, retaining Unix socket grants and the terminal input injection denial. - Prepare managed network context alongside proxy environment variables and pass it through sandbox transformation. ## Testing - Extend sandbox network proxy tests to macOS, covering blocked direct loopback access and explicitly allowed requests. - Add coverage for Linux helper executable paths and dispatch names, including non-UTF-8 paths. GitOrigin-RevId: 8b7396d21b2f34122699388f16047329a3f5e5c6 --- codex-rs/cli/src/debug_sandbox.rs | 352 +++++++------------- codex-rs/cli/tests/sandbox_network_proxy.rs | 51 ++- codex-rs/core/src/exec.rs | 19 +- codex-rs/core/src/shell_snapshot_sandbox.rs | 3 +- codex-rs/core/src/tools/sandboxing.rs | 8 +- codex-rs/exec-server/src/process_sandbox.rs | 19 +- codex-rs/sandboxing/src/landlock.rs | 2 +- codex-rs/sandboxing/src/launch.rs | 51 +++ codex-rs/sandboxing/src/lib.rs | 1 + codex-rs/sandboxing/src/manager.rs | 64 +++- codex-rs/sandboxing/src/manager_tests.rs | 73 +++- codex-rs/sandboxing/src/seatbelt.rs | 18 +- 12 files changed, 353 insertions(+), 308 deletions(-) diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index 04ebcf160f..a0a6ea719d 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -22,18 +22,18 @@ use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use codex_core::windows_sandbox::local_binding_policy_for_sandbox; use codex_core::windows_sandbox::managed_proxy_routing_for_windows_sandbox; use codex_protocol::config_types::SandboxMode; +use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_protocol::models::SandboxEnforcement; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxTransformRequest; +use codex_sandboxing::SandboxType as SandboxBackend; use codex_sandboxing::SandboxType::WindowsMxc; -use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_permission_profile; -#[cfg(target_os = "macos")] -use codex_sandboxing::seatbelt::CreateSeatbeltCommandArgsParams; -#[cfg(target_os = "macos")] -use codex_sandboxing::seatbelt::create_seatbelt_command_args; -use codex_sandboxing::with_managed_mitm_ca_readable_root; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_cli::CliConfigOverrides; +use codex_utils_path_uri::PathUri; use tokio::process::Child; use tokio::process::Command as TokioCommand; use toml::Value as TomlValue; @@ -208,7 +208,10 @@ async fn run_command_under_sandbox( codex_linux_sandbox_exe: Option, sandbox_type: SandboxType, log_denials: bool, - #[cfg_attr(not(target_os = "macos"), allow(unused_variables))] + #[cfg_attr( + not(any(target_os = "linux", target_os = "macos")), + allow(unused_variables) + )] allow_unix_sockets: &[AbsolutePathBuf], ) -> anyhow::Result<()> { let sandbox_state = config_options @@ -249,18 +252,12 @@ async fn run_command_under_sandbox( ) .await?; - // In practice, this should be `std::env::current_dir()` because this CLI - // does not support `--cwd`, but let's use the config value for consistency. - let cwd = config.cwd.clone(); - // Non-Windows sandbox launchers still use `sandbox_policy_cwd` for any - // remaining cwd-dependent policy resolution. `:workspace_roots` entries in - // the effective profile have already been materialized from config roots. - let sandbox_policy_cwd = cwd.clone(); + let cwd = &config.cwd; let env = create_env( &config.permissions.shell_environment_policy, /*thread_id*/ None, ); - let mut permission_profile = match sandbox_state.as_ref() { + let base_profile = match sandbox_state.as_ref() { Some(state) => match &state.permission_profile { PermissionProfile::External { .. } => { // `External` only says that the producer relies on an outer sandbox; it does not @@ -271,29 +268,28 @@ async fn run_command_under_sandbox( } permission_profile => permission_profile.clone(), }, - None if matches!(sandbox_type, SandboxType::Windows) => { - config.permissions.permission_profile().clone() - } - None => config.permissions.effective_permission_profile(), + None => config.permissions.permission_profile().clone(), }; - if matches!(permission_profile, PermissionProfile::Disabled) && sandbox_state_disable_network { + if matches!(base_profile, PermissionProfile::Disabled) && sandbox_state_disable_network { anyhow::bail!( "--sandbox-state-disable-network cannot be applied to a disabled permission profile" ); } - if !matches!(permission_profile, PermissionProfile::Disabled) + let permission_profile = if !matches!(base_profile, PermissionProfile::Disabled) && (!sandbox_state_readable_root.is_empty() || sandbox_state_disable_network) { - let file_system = permission_profile + let file_system = base_profile .file_system_sandbox_policy() - .with_additional_readable_roots(&cwd, &sandbox_state_readable_root); + .with_additional_readable_roots(cwd, &sandbox_state_readable_root); let network = if sandbox_state_disable_network { NetworkSandboxPolicy::Restricted } else { - permission_profile.network_sandbox_policy() + base_profile.network_sandbox_policy() }; - permission_profile = PermissionProfile::from_runtime_permissions(&file_system, network); - } + PermissionProfile::from_runtime_permissions(&file_system, network) + } else { + base_profile + }; let use_legacy_landlock = sandbox_state.as_ref().map_or_else( || config.features.use_legacy_landlock(), |state| state.use_legacy_landlock, @@ -307,237 +303,130 @@ async fn run_command_under_sandbox( config.effective_local_windows_sandbox_type() }; - match permission_profile.enforcement() { - SandboxEnforcement::Managed => {} - SandboxEnforcement::Disabled | SandboxEnforcement::External => { - let (program, args) = command - .split_first() - .context("sandbox command must not be empty")?; - let mut child = spawn_debug_sandbox_child( - PathBuf::from(program), - args.to_vec(), - /*arg0*/ None, - cwd.to_path_buf(), - permission_profile.network_sandbox_policy(), - env, - |_| {}, - ) - .await?; - handle_exit_status(child.wait().await?); - } - } - - // Legacy Windows sandbox sessions emulate inherited stdio and exit the process. - if let SandboxType::Windows = sandbox_type { - #[cfg(target_os = "windows")] - { - if windows_sandbox_type != WindowsMxc { - if sandbox_state.is_none() { - permission_profile = config.permissions.effective_permission_profile(); - } + let sandbox = match permission_profile.enforcement() { + SandboxEnforcement::Disabled | SandboxEnforcement::External => SandboxBackend::None, + SandboxEnforcement::Managed => match sandbox_type { + #[cfg(target_os = "macos")] + SandboxType::Seatbelt => SandboxBackend::MacosSeatbelt, + SandboxType::Landlock => SandboxBackend::LinuxSeccomp, + #[cfg(windows)] + SandboxType::Windows if windows_sandbox_type == WindowsMxc => WindowsMxc, + #[cfg(windows)] + SandboxType::Windows => { + let session_profile = if sandbox_state.is_none() { + config.permissions.effective_permission_profile() + } else { + permission_profile + }; let workspace_roots = config .effective_workspace_roots() .iter() - .map(codex_utils_path_uri::PathUri::to_abs_path) + .map(PathUri::to_abs_path) .collect::>>() .context("invalid Windows sandbox workspace roots")?; run_command_under_windows_session( &config, - &permission_profile, + &session_profile, command, - cwd, + cwd.clone(), workspace_roots, env, ) .await; } - } - #[cfg(not(target_os = "windows"))] - { - anyhow::bail!("Windows sandbox is only available on Windows"); - } - } + #[cfg(not(windows))] + SandboxType::Windows => anyhow::bail!("Windows sandbox is only available on Windows"), + }, + }; #[cfg(target_os = "macos")] - let mut denial_logger = log_denials.then(DenialLogger::new).flatten(); + let mut denial_logger = (log_denials && sandbox == SandboxBackend::MacosSeatbelt) + .then(DenialLogger::new) + .flatten(); #[cfg(not(target_os = "macos"))] let _ = log_denials; - let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); - - // This proxy should only live for the lifetime of the child process. - let network_proxy = match config.permissions.network.as_ref() { - Some(spec) => Some( + // Keep the proxy alive until the child exits. + let network_proxy = if sandbox != SandboxBackend::None + && let Some(spec) = &config.permissions.network + { + Some( spec.start_proxy( &permission_profile, managed_proxy_routing_for_windows_sandbox(windows_sandbox_type), local_binding_policy_for_sandbox(windows_sandbox_type, Some(std::env::consts::OS)), /*policy_decider*/ None, /*blocked_request_observer*/ None, - managed_network_requirements_enabled, + config.managed_network_requirements_enabled(), NetworkProxyAuditMetadata::default(), ) .await .map_err(|err| anyhow::anyhow!("failed to start managed network proxy: {err}"))?, - ), - None => None, + ) + } else { + None }; let network = network_proxy .as_ref() .map(codex_core::config::StartedNetworkProxy::proxy); - // Proxy containment depends on whether a proxy is active, not whether its - // policy came from managed requirements. - #[cfg(any(target_os = "macos", target_os = "windows"))] - let enforce_managed_network = network.is_some(); - // MXC adds the CA read grant in transform after binding the workspace roots. - let managed_mitm_ca_trust_bundle_path = match (network.as_ref(), &sandbox_type) { - (None, _) | (_, SandboxType::Windows) => None, - (Some(network), _) => network.managed_mitm_ca_trust_bundle_path(), + let sandbox_exe = match sandbox { + SandboxBackend::LinuxSeccomp => config.codex_linux_sandbox_exe.clone(), + WindowsMxc => Some(std::env::current_exe()?), + SandboxBackend::None + | SandboxBackend::MacosSeatbelt + | SandboxBackend::WindowsRestrictedToken => None, }; - let runtime_permission_profile = with_managed_mitm_ca_readable_root( - permission_profile, - managed_mitm_ca_trust_bundle_path.as_ref(), - sandbox_policy_cwd.as_path(), - ); - - let mut child = match sandbox_type { - #[cfg(target_os = "macos")] - SandboxType::Seatbelt => { - let (file_system_sandbox_policy, network_sandbox_policy) = - runtime_permission_profile.to_runtime_permissions(); - let mut args = create_seatbelt_command_args(CreateSeatbeltCommandArgsParams { - command, - file_system_sandbox_policy: &file_system_sandbox_policy, - network_sandbox_policy, - sandbox_policy_cwd: sandbox_policy_cwd.as_path(), - enforce_managed_network, - managed_network: None, - environment_id: None, - network: network.as_ref(), - extra_allow_unix_sockets: allow_unix_sockets, - }) - .map_err(|err| anyhow::anyhow!(err))?; - // This CLI inherits the user's controlling terminal. Keep this deny - // after every shared policy allowance so the child cannot queue input - // for the unsandboxed shell that resumes when Codex exits. - match args.as_mut_slice() { - [flag, policy, ..] if flag.as_str() == "-p" => { - // Older macOS policy compilers do not define the TIOCSTI symbol. - policy.push_str(&format!( - "\n(deny file-ioctl (ioctl-command {}))", - libc::TIOCSTI - )); - } - _ => anyhow::bail!("Seatbelt command is missing its generated policy"), - } - spawn_debug_sandbox_child( - PathBuf::from("/usr/bin/sandbox-exec"), - args, - /*arg0*/ None, - cwd.to_path_buf(), - network_sandbox_policy, - env, - |env_map| { - env_map.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); - if let Some(network) = network.as_ref() { - network.apply_to_env(env_map); - } - }, - ) - .await? - } - SandboxType::Landlock => { - #[expect(clippy::expect_used)] - let codex_linux_sandbox_exe = config - .codex_linux_sandbox_exe - .expect("codex-linux-sandbox executable not found"); - let network_sandbox_policy = runtime_permission_profile.network_sandbox_policy(); - let (env, managed_network) = if let Some(network) = network.as_ref() { - let prepared = - network.prepare_for_optional_environment(env, /*environment_id*/ None)?; - (prepared.env, Some(prepared.sandbox_context)) - } else { - (env, None) - }; - let args = create_linux_sandbox_command_args_for_permission_profile( - command, - cwd.as_path(), - &runtime_permission_profile, - sandbox_policy_cwd.as_path(), - use_legacy_landlock, - managed_network.as_ref(), - ); - spawn_debug_sandbox_child( - codex_linux_sandbox_exe, - args, - Some("codex-linux-sandbox"), - cwd.to_path_buf(), - network_sandbox_policy, - env, - |_| {}, - ) - .await? - } - #[cfg(not(target_os = "windows"))] - SandboxType::Windows => { - unreachable!("Windows sandbox should have been handled above"); - } - #[cfg(target_os = "windows")] - SandboxType::Windows => { - use codex_sandboxing::SandboxCommand; - use codex_sandboxing::SandboxManager; - use codex_sandboxing::SandboxTransformRequest; - use codex_utils_path_uri::PathUri; - - let (program, args) = command - .split_first() - .context("sandbox command must not be empty")?; - let sandbox_policy_cwd = PathUri::from_abs_path(&sandbox_policy_cwd); - let codex_self_exe = std::env::current_exe()?; - let workspace_roots = sandbox_state.is_none().then(|| { - config - .permissions - .workspace_roots() - .iter() - .map(PathUri::from_abs_path) - .collect::>() - }); - let request = SandboxManager::new().transform(SandboxTransformRequest { - command: SandboxCommand { - program: program.into(), - args: args.to_vec(), - cwd: PathUri::from_abs_path(&cwd), - env, - managed_network: None, - additional_permissions: None, - }, - permissions: &runtime_permission_profile, - workspace_roots: workspace_roots.as_deref(), - sandbox: WindowsMxc, - enforce_managed_network, - environment_id: None, - network: network.as_ref(), - sandbox_policy_cwd: &sandbox_policy_cwd, - sandbox_exe: Some(codex_self_exe.as_path()), - use_legacy_landlock: false, - windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, - })?; - let (program, args) = request - .command - .split_first() - .context("missing MXC wrapper")?; - spawn_debug_sandbox_child( - PathBuf::from(program), - args.to_vec(), - /*arg0*/ None, - cwd.to_path_buf(), - runtime_permission_profile.network_sandbox_policy(), - request.env, - |_| {}, - ) - .await? + let mut command = command.into_iter(); + let program = command + .next() + .context("sandbox command must not be empty")?; + let cwd = PathUri::from_abs_path(cwd); + #[cfg(any(target_os = "linux", target_os = "macos"))] + let manager = SandboxManager::for_inherited_terminal(allow_unix_sockets); + #[cfg(not(any(target_os = "linux", target_os = "macos")))] + let manager = SandboxManager::new(); + let workspace_roots = sandbox_state.is_none().then(|| { + config + .permissions + .workspace_roots() + .iter() + .map(PathUri::from_abs_path) + .collect::>() + }); + #[cfg(target_os = "macos")] + let env = { + let mut env_map = env; + if sandbox == SandboxBackend::MacosSeatbelt { + env_map.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); } + env_map }; + let wrapped = manager.transform(SandboxTransformRequest { + command: SandboxCommand { + program: program.into(), + args: command.collect(), + cwd: cwd.clone(), + env, + managed_network: None, + additional_permissions: None, + }, + permissions: &permission_profile, + workspace_roots: workspace_roots.as_deref(), + sandbox, + enforce_managed_network: network.is_some(), + network: network.as_ref(), + environment_id: None, + sandbox_policy_cwd: &cwd, + sandbox_exe: sandbox_exe.as_deref(), + use_legacy_landlock, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + })?; + + let mut child = spawn_debug_sandbox_child( + wrapped.into_direct_command()?, + permission_profile.network_sandbox_policy(), + ) + .await?; #[cfg(target_os = "macos")] if let Some(denial_logger) = &mut denial_logger { @@ -627,25 +516,10 @@ async fn run_command_under_windows_session( } async fn spawn_debug_sandbox_child( - program: PathBuf, - args: Vec, - arg0: Option<&str>, - cwd: PathBuf, + command: std::process::Command, network_sandbox_policy: NetworkSandboxPolicy, - mut env: std::collections::HashMap, - apply_env: impl FnOnce(&mut std::collections::HashMap), ) -> std::io::Result { - let mut cmd = TokioCommand::new(&program); - #[cfg(unix)] - cmd.arg0(arg0.map_or_else(|| program.to_string_lossy().to_string(), String::from)); - #[cfg(not(unix))] - let _ = arg0; - cmd.args(args); - cmd.current_dir(cwd); - apply_env(&mut env); - cmd.env_clear(); - cmd.envs(env); - + let mut cmd = TokioCommand::from(command); if !network_sandbox_policy.is_enabled() { cmd.env(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR, "1"); } diff --git a/codex-rs/cli/tests/sandbox_network_proxy.rs b/codex-rs/cli/tests/sandbox_network_proxy.rs index a21b5b4ee8..177ec33741 100644 --- a/codex-rs/cli/tests/sandbox_network_proxy.rs +++ b/codex-rs/cli/tests/sandbox_network_proxy.rs @@ -1,4 +1,4 @@ -#![cfg(target_os = "linux")] +#![cfg(any(target_os = "linux", target_os = "macos"))] use std::net::TcpListener; @@ -10,12 +10,36 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; -const BWRAP_UNAVAILABLE_ERR: &str = "bubblewrap is unavailable"; +#[cfg(target_os = "macos")] +const LOOPBACK: &str = "127.0.0.1"; +#[cfg(target_os = "linux")] +const LOOPBACK: &str = "127.0.0.2"; + +fn sandbox_unavailable(stderr: &str) -> Result { + #[cfg(target_os = "linux")] + if stderr.contains("bubblewrap is unavailable") { + eprintln!("skipping network proxy sandbox test: bubblewrap is unavailable"); + return Ok(true); + } + #[cfg(target_os = "macos")] + if stderr.contains("sandbox-exec: sandbox_apply: Operation not permitted") { + let control = std::process::Command::new("/usr/bin/sandbox-exec") + .args(["-p", "(version 1)(allow default)", "/usr/bin/true"]) + .output()?; + if String::from_utf8_lossy(&control.stderr) + .contains("sandbox_apply: Operation not permitted") + { + eprintln!("skipping network proxy sandbox test: nested Seatbelt is unavailable"); + return Ok(true); + } + } + Ok(false) +} #[test] fn sandbox_with_network_proxy_blocks_direct_loopback_access() -> Result<()> { let codex_home = TempDir::new()?; - let listener = TcpListener::bind("127.0.0.2:0")?; + let listener = TcpListener::bind((LOOPBACK, 0))?; let port = listener.local_addr()?.port(); std::fs::write( codex_home.path().join("config.toml"), @@ -35,7 +59,7 @@ mode = "full" "#, )?; - let url = format!("http://127.0.0.2:{port}/"); + let url = format!("http://{LOOPBACK}:{port}/"); let output = std::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) .env("CODEX_HOME", codex_home.path()) .args([ @@ -57,8 +81,7 @@ mode = "full" .output()?; let stderr = String::from_utf8_lossy(&output.stderr); - if stderr.contains(BWRAP_UNAVAILABLE_ERR) { - eprintln!("skipping network proxy sandbox test: bubblewrap is unavailable"); + if sandbox_unavailable(&stderr)? { return Ok(()); } @@ -77,7 +100,7 @@ mode = "full" #[tokio::test] async fn sandbox_with_network_proxy_allows_explicit_loopback_access() -> Result<()> { let codex_home = TempDir::new()?; - let listener = TcpListener::bind("127.0.0.2:0")?; + let listener = TcpListener::bind((LOOPBACK, 0))?; let port = listener.local_addr()?.port(); let server = MockServer::builder().listener(listener).start().await; Mock::given(method("GET")) @@ -87,7 +110,8 @@ async fn sandbox_with_network_proxy_allows_explicit_loopback_access() -> Result< .await; std::fs::write( codex_home.path().join("config.toml"), - r#" + format!( + r#" default_permissions = "network-test" [features] @@ -103,11 +127,12 @@ mode = "full" allow_local_binding = false [permissions.network-test.network.domains] -"127.0.0.2" = "allow" -"#, +"{LOOPBACK}" = "allow" +"# + ), )?; - let url = format!("http://127.0.0.2:{port}/"); + let url = format!("http://{LOOPBACK}:{port}/"); let output = tokio::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?) .env("CODEX_HOME", codex_home.path()) .args([ @@ -129,8 +154,7 @@ allow_local_binding = false .await?; let stderr = String::from_utf8_lossy(&output.stderr); - if stderr.contains(BWRAP_UNAVAILABLE_ERR) { - eprintln!("skipping network proxy sandbox test: bubblewrap is unavailable"); + if sandbox_unavailable(&stderr)? { return Ok(()); } @@ -141,5 +165,6 @@ allow_local_binding = false String::from_utf8_lossy(&output.stdout), stderr, ); + assert_eq!(server.received_requests().await.unwrap().len(), 1); Ok(()) } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 0d82a8d55d..9faf75afff 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -350,7 +350,7 @@ pub async fn build_exec_request( let ExecParams { command, cwd, - mut env, + env, expiration, capture_policy, network, @@ -372,13 +372,16 @@ pub async fn build_exec_request( ); tracing::debug!("Sandbox type: {sandbox_type:?}"); - if let Some(network) = network.as_ref() { - network - .apply_to_env_for_optional_environment(&mut env, network_environment_id.as_deref()) + let (env, managed_network) = if let Some(network) = network.as_ref() { + let prepared = network + .prepare_for_optional_environment(env, network_environment_id.as_deref()) .map_err(|err| { network_proxy_environment_error(network_environment_id.as_deref(), err) })?; - } + (prepared.env, Some(prepared.sandbox_context)) + } else { + (env, None) + }; let (program, args) = command.split_first().ok_or_else(|| { CodexErr::Io(io::Error::new( io::ErrorKind::InvalidInput, @@ -394,7 +397,7 @@ pub async fn build_exec_request( args: args.to_vec(), cwd, env, - managed_network: None, + managed_network, additional_permissions: None, }; let options = ExecOptions { @@ -418,8 +421,8 @@ pub async fn build_exec_request( }, use_legacy_landlock, windows_sandbox_level, - }) - .map_err(CodexErr::from)?; + })? + .into_command_request(); // These hints belong to the native Windows backend. Other backends use // the materialized profile and must not project executor paths onto this host. let windows_sandbox_workspace_roots = if sandbox_type == SandboxType::WindowsRestrictedToken { diff --git a/codex-rs/core/src/shell_snapshot_sandbox.rs b/codex-rs/core/src/shell_snapshot_sandbox.rs index 03e8542d52..5ed7fb792e 100644 --- a/codex-rs/core/src/shell_snapshot_sandbox.rs +++ b/codex-rs/core/src/shell_snapshot_sandbox.rs @@ -174,7 +174,8 @@ impl ShellSnapshotSandbox { use_legacy_landlock: self.use_legacy_landlock, windows_sandbox_level: self.windows_sandbox_level, }) - .context("failed to prepare shell snapshot sandbox")?; + .context("failed to prepare shell snapshot sandbox")? + .into_command_request(); let workspace_roots = self .workspace_roots .iter() diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index c1bd44e750..2df6466d52 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -513,8 +513,8 @@ impl<'a> SandboxAttempt<'a> { sandbox_exe: self.sandbox_exe.map(std::path::PathBuf::as_path), use_legacy_landlock: self.use_legacy_landlock, windows_sandbox_level: self.windows_sandbox_level, - }) - .map_err(CodexErr::from)?; + })? + .into_command_request(); request.sandbox_override = self.sandbox_override; let workspace_roots = self .workspace_roots @@ -550,8 +550,8 @@ impl<'a> SandboxAttempt<'a> { sandbox_exe: None, use_legacy_landlock: self.use_legacy_landlock, windows_sandbox_level: self.windows_sandbox_level, - }) - .map_err(CodexErr::from)?; + })? + .into_command_request(); request.sandbox_override = self.sandbox_override; let mut exec_request = crate::sandboxing::ExecRequest::from_sandbox_exec_request(request, options, Vec::new()) diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index 6e17d023cb..f6f8ba09a3 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -19,7 +19,6 @@ use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxCommand; -use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; @@ -267,10 +266,8 @@ pub(crate) async fn prepare_exec_request_with_telemetry( } else { program }; - let transform_request = SandboxDirectSpawnTransformRequest { - workspace_roots, - windows_sandbox_proxy_settings_mode, - transform: SandboxTransformRequest { + let mut request = sandbox_manager + .transform(SandboxTransformRequest { command: SandboxCommand { program, args, @@ -293,15 +290,9 @@ pub(crate) async fn prepare_exec_request_with_telemetry( }, use_legacy_landlock: sandbox_context.use_legacy_landlock, windows_sandbox_level: windows_sandbox_level.unwrap_or(WindowsSandboxLevel::Disabled), - }, - }; - let mut request = if sandbox == SandboxType::WindowsRestrictedToken { - // The shared launcher invokes the native Windows session spawner directly. - sandbox_manager.transform(transform_request.transform) - } else { - sandbox_manager.transform_for_direct_spawn(transform_request) - } - .map_err(|err| invalid_params(format!("failed to prepare process sandbox: {err}")))?; + }) + .map_err(|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 { diff --git a/codex-rs/sandboxing/src/landlock.rs b/codex-rs/sandboxing/src/landlock.rs index dad16e238c..9d8eef0d01 100644 --- a/codex-rs/sandboxing/src/landlock.rs +++ b/codex-rs/sandboxing/src/landlock.rs @@ -23,7 +23,7 @@ pub fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { /// after parsing these arguments. The profile JSON flag is emitted before /// helper feature flags so the argv order matches the helper's CLI shape. See /// `docs/linux_sandbox.md` for the Linux semantics. -pub fn create_linux_sandbox_command_args_for_permission_profile( +pub(crate) fn create_linux_sandbox_command_args_for_permission_profile( command: Vec, command_cwd: &Path, permission_profile: &PermissionProfile, diff --git a/codex-rs/sandboxing/src/launch.rs b/codex-rs/sandboxing/src/launch.rs index d56f70de95..b126c0a3f1 100644 --- a/codex-rs/sandboxing/src/launch.rs +++ b/codex-rs/sandboxing/src/launch.rs @@ -1,12 +1,63 @@ //! Bind filesystem and network inputs for sandbox preparation on the execution host. +use crate::SandboxExecRequest; +use crate::SandboxType; +use std::ffi::OsStr; +use std::ffi::OsString; use std::io; +use std::process::Command; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemPath; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +/// A command prepared for its selected sandbox. Native Windows token sessions +/// must use the Windows session launcher. +#[must_use] +pub struct WrappedSandboxCommand { + pub(super) request: SandboxExecRequest, + // UTF-8 argv cannot represent every native executable path. + pub(super) native_program: Option, +} + +impl WrappedSandboxCommand { + /// Return the request with UTF-8 argv for callers that spawn the command themselves. + pub fn into_command_request(self) -> SandboxExecRequest { + self.request + } + + /// Return a configured process command when argv carries all required sandbox enforcement. + pub fn into_direct_command(self) -> io::Result { + let request = self.request; + if request.sandbox == SandboxType::WindowsRestrictedToken { + return Err(io::Error::new( + io::ErrorKind::Unsupported, + "a native Windows sandbox requires its session launcher", + )); + } + let (program, args) = request.command.split_first().ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidInput, "sandbox command is empty") + })?; + let mut command = Command::new( + self.native_program + .as_deref() + .unwrap_or_else(|| OsStr::new(program)), + ); + #[cfg(unix)] + if let Some(arg0) = request.arg0 { + use std::os::unix::process::CommandExt; + command.arg0(arg0); + } + command + .args(args) + .current_dir(request.cwd.to_abs_path()?) + .env_clear() + .envs(request.env); + Ok(command) + } +} + /// The authorized profile, resolved on the host that will enforce it. #[derive(Debug)] pub(super) struct PreparedLaunchPolicy { diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index 70b3a812bb..b9ce18baf9 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -40,6 +40,7 @@ pub use manager::SandboxType; pub use manager::SandboxablePreference; pub use manager::compatibility_sandbox_policy_for_permission_profile; pub use manager::get_platform_sandbox; +pub use manager::launch::WrappedSandboxCommand; pub use manager::with_managed_mitm_ca_readable_root; pub use sandbox_integrity::FileContentsChecker; pub use sandbox_integrity::IntegrityFinding; diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index fa4d252d3b..a192aa524e 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -1,4 +1,5 @@ use self::launch::PreparedLaunchPolicy; +use self::launch::WrappedSandboxCommand; use crate::LinuxSandboxPidNamespace; #[cfg(windows)] use crate::WindowsSandboxFilesystemOverrides; @@ -277,6 +278,8 @@ pub struct SandboxManager { #[cfg(target_os = "macos")] seatbelt_profile: MacosSeatbeltProfile, #[cfg(target_os = "macos")] + extra_allow_unix_sockets: Vec, + #[cfg(target_os = "macos")] allowed_symlinked_codex_home: Option, } @@ -288,11 +291,23 @@ impl SandboxManager { /// Creates a manager that applies the narrower runtime profile required by filesystem helpers. pub fn for_file_system_helpers() -> Self { Self { - linux_sandbox_pid_namespace: LinuxSandboxPidNamespace::default(), #[cfg(target_os = "macos")] seatbelt_profile: MacosSeatbeltProfile::FileSystemHelper, + ..Self::default() + } + } + + /// Prepare commands that inherit the controlling terminal. On macOS this also + /// grants access to the named Unix sockets and denies terminal input injection. + pub fn for_inherited_terminal(allow_unix_sockets: &[AbsolutePathBuf]) -> Self { + #[cfg(not(target_os = "macos"))] + let _ = allow_unix_sockets; + Self { #[cfg(target_os = "macos")] - allowed_symlinked_codex_home: None, + seatbelt_profile: MacosSeatbeltProfile::ProcessWithInheritedTerminal, + #[cfg(target_os = "macos")] + extra_allow_unix_sockets: allow_unix_sockets.to_vec(), + ..Self::default() } } @@ -359,7 +374,7 @@ impl SandboxManager { pub fn transform( &self, request: SandboxTransformRequest<'_>, - ) -> Result { + ) -> Result { #[cfg(windows)] if request.sandbox == SandboxType::WindowsRestrictedToken && request.enforce_managed_network @@ -371,6 +386,19 @@ impl SandboxManager { } 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 => { + request.sandbox_exe.map(Path::as_os_str) + } + SandboxType::MacosSeatbelt | SandboxType::WindowsRestrictedToken => None, + } + .filter(|path| path.to_str().is_none()) + .map(OsStr::to_owned); + let wrapped = |request| WrappedSandboxCommand { + request, + native_program, + }; let managed_network = command .managed_network .filter(|_| request.enforce_managed_network || backend == SandboxType::MacosSeatbelt); @@ -404,7 +432,7 @@ impl SandboxManager { arg0: None, }; if backend == SandboxType::None { - return Ok(exec); + return Ok(wrapped(exec)); } let pending = PendingSandboxedExecRequest::new( &exec.cwd, @@ -469,6 +497,18 @@ impl SandboxManager { exec.permission_profile = pending.effective_permission_profile; let (file_system_sandbox_policy, network_sandbox_policy) = exec.permission_profile.to_runtime_permissions(); + let managed_network = match (managed_network, request.network) { + (None, Some(proxy)) => { + let prepared = proxy + .prepare_for_optional_environment(exec.env, request.environment_id) + .map_err(|err| { + SandboxTransformError::EnvironmentNetworkProxy(err.to_string()) + })?; + exec.env = prepared.env; + Some(prepared.sandbox_context) + } + (context, _) => context, + }; let args = create_seatbelt_command_args_with_profile( CreateSeatbeltCommandArgsParams { command: std::mem::take(&mut exec.command), @@ -479,7 +519,7 @@ impl SandboxManager { managed_network: managed_network.as_ref(), environment_id: request.environment_id, network: request.network, - extra_allow_unix_sockets: &[], + extra_allow_unix_sockets: &self.extra_allow_unix_sockets, }, self.seatbelt_profile, self.allowed_symlinked_codex_home.as_ref(), @@ -536,7 +576,7 @@ impl SandboxManager { exec.permission_profile = pending?.effective_permission_profile; } } - Ok(exec) + Ok(wrapped(exec)) } fn wrap_linux( @@ -567,9 +607,12 @@ impl SandboxManager { 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 + exec.command = std::iter::once(program).chain(args).collect(); + // Without bwrap --argv0, the Linux helper uses its own argv[0] as the inner executable. + let arg0 = if let Some(path) = exe.to_str() + && exe.file_name() == Some(OsStr::new(CODEX_LINUX_SANDBOX_ARG0)) + { + path } else { CODEX_LINUX_SANDBOX_ARG0 }; @@ -588,6 +631,7 @@ impl SandboxManager { #[cfg(not(windows))] { self.transform(request.transform) + .map(WrappedSandboxCommand::into_command_request) } } @@ -601,7 +645,7 @@ impl SandboxManager { let fallback_roots = request.workspace_roots; let proxy_settings_mode = request.windows_sandbox_proxy_settings_mode; let sandbox_exe = request.transform.sandbox_exe; - let mut request = self.transform(request.transform)?; + let mut request = self.transform(request.transform)?.into_command_request(); if request.sandbox != SandboxType::WindowsRestrictedToken { return Ok(request); } diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index b5486d4223..990b7c2e7b 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -125,7 +125,8 @@ fn unsandboxed_transform_preserves_foreign_cwd_and_unrestricted_file_system_poli use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, }) - .expect("transform"); + .expect("transform") + .into_command_request(); assert_eq!(exec_request.cwd, cwd_uri); assert_eq!(exec_request.sandbox_policy_cwd, cwd_uri); @@ -186,7 +187,8 @@ fn symlinked_workspace_reports_seatbelt_preparation_error() { use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, }) - .expect_err("symlinked workspace should be rejected"); + .err() + .expect("symlinked workspace should be rejected"); assert!(matches!( &error, @@ -241,7 +243,8 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, }) - .expect("transform"); + .expect("transform") + .into_command_request(); assert_eq!( exec_request.permission_profile, @@ -329,7 +332,10 @@ fn transform_additional_permissions_preserves_denied_entries() { use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::RestrictedToken, }; - let exec_request = manager.transform(request).expect("transform"); + let exec_request = manager + .transform(request) + .expect("transform") + .into_command_request(); assert_eq!( exec_request.permission_profile, PermissionProfile::from_runtime_permissions( @@ -383,12 +389,11 @@ fn managed_mitm_ca_bundle_becomes_readable_for_restricted_sandbox() { ); } -#[cfg(target_os = "linux")] +#[cfg(unix)] fn transform_linux_seccomp_request( codex_linux_sandbox_exe: &std::path::Path, - mode: crate::LinuxSandboxPidNamespace, -) -> super::SandboxExecRequest { - let manager = SandboxManager::new().with_linux_sandbox_pid_namespace(mode); + manager: SandboxManager, +) -> super::WrappedSandboxCommand { let cwd = AbsolutePathBuf::current_dir().expect("current dir"); let cwd_uri = PathUri::from_abs_path(&cwd); let permissions = PermissionProfile::Disabled; @@ -491,10 +496,8 @@ fn wsl1_allows_non_bubblewrap_linux_paths() { #[test] fn transform_linux_seccomp_preserves_helper_path_in_arg0_when_available() { let codex_linux_sandbox_exe = std::path::PathBuf::from("/tmp/codex-linux-sandbox"); - let exec_request = transform_linux_seccomp_request( - &codex_linux_sandbox_exe, - crate::LinuxSandboxPidNamespace::default(), - ); + let exec_request = + transform_linux_seccomp_request(&codex_linux_sandbox_exe, SandboxManager::new()).request; assert_eq!( exec_request.arg0, @@ -516,12 +519,51 @@ fn transform_linux_seccomp_uses_helper_alias_when_launcher_is_not_helper_path() "--inherit-pid-namespace", ), ] { - let exec_request = transform_linux_seccomp_request(&codex_linux_sandbox_exe, mode); + let exec_request = transform_linux_seccomp_request( + &codex_linux_sandbox_exe, + SandboxManager::new().with_linux_sandbox_pid_namespace(mode), + ) + .request; assert_eq!(exec_request.arg0, Some("codex-linux-sandbox".to_string())); assert_eq!(exec_request.command[1], first_helper_arg); } } +#[cfg(unix)] +#[test] +fn inherited_terminal_preserves_native_linux_helper_and_uses_its_dispatch_name() +-> anyhow::Result<()> { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir()?; + let alias = OsStr::new(crate::landlock::CODEX_LINUX_SANDBOX_ARG0); + let alias_path = dir.path().join(alias); + for (helper, expected_arg0) in [ + (alias_path.clone(), alias_path.as_os_str()), + (dir.path().join("codex"), alias), + #[cfg(target_os = "linux")] + ( + dir.path().join(OsStr::from_bytes(b"cli-\xff")).join(alias), + alias, + ), + ] { + std::fs::create_dir_all(helper.parent().expect("helper directory"))?; + symlink("/bin/sh", &helper)?; + let mut wrapped = + transform_linux_seccomp_request(&helper, SandboxManager::for_inherited_terminal(&[])); + // Run a shell probe with the executable and argv[0] produced by the Linux backend. + wrapped.request.command.truncate(1); + let mut child = wrapped.into_direct_command()?; + assert_eq!(child.get_program(), helper.as_os_str()); + let output = child.args(["-c", r#"printf '%s' "$0""#]).output()?; + assert!(output.status.success(), "{output:?}"); + assert_eq!(output.stdout, expected_arg0.as_bytes()); + } + Ok(()) +} + #[cfg(unix)] #[tokio::test] async fn linux_unix_socket_grant_uses_effective_managed_policy() -> anyhow::Result<()> { @@ -617,7 +659,7 @@ async fn linux_unix_socket_grant_uses_effective_managed_policy() -> anyhow::Resu ), ("no managed network", Some(allow_all), true, false, None), ] { - let request = manager.transform(SandboxTransformRequest { + let prepared = manager.transform(SandboxTransformRequest { command: SandboxCommand { program: "true".into(), args: Vec::new(), @@ -637,7 +679,8 @@ async fn linux_unix_socket_grant_uses_effective_managed_policy() -> anyhow::Resu use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, })?; - let transported_context = request + let transported_context = prepared + .into_command_request() .command .windows(2) .find(|args| args[0] == "--managed-network") diff --git a/codex-rs/sandboxing/src/seatbelt.rs b/codex-rs/sandboxing/src/seatbelt.rs index bca03f4f4c..4b56255456 100644 --- a/codex-rs/sandboxing/src/seatbelt.rs +++ b/codex-rs/sandboxing/src/seatbelt.rs @@ -34,6 +34,7 @@ mod scratch; pub(crate) enum MacosSeatbeltProfile { #[default] Process, + ProcessWithInheritedTerminal, FileSystemHelper, } @@ -790,7 +791,7 @@ fn seatbelt_regex_for_glob(pattern: &str, glob_match: GlobMatch) -> Option { +pub(crate) struct CreateSeatbeltCommandArgsParams<'a> { pub command: Vec, pub file_system_sandbox_policy: &'a FileSystemSandboxPolicy, pub network_sandbox_policy: NetworkSandboxPolicy, @@ -802,7 +803,8 @@ pub struct CreateSeatbeltCommandArgsParams<'a> { pub extra_allow_unix_sockets: &'a [AbsolutePathBuf], } -pub fn create_seatbelt_command_args( +#[cfg(test)] +fn create_seatbelt_command_args( args: CreateSeatbeltCommandArgsParams<'_>, ) -> Result, String> { create_seatbelt_command_args_with_profile( @@ -837,7 +839,10 @@ pub(crate) fn create_seatbelt_command_args_with_profile( ) .map_err(|err| SeatbeltPreparationError::FileSystem(err.to_string()))?; let include_process_defaults = resolved_filesystem_permissions.include_platform_defaults - && profile == MacosSeatbeltProfile::Process; + && matches!( + profile, + MacosSeatbeltProfile::Process | MacosSeatbeltProfile::ProcessWithInheritedTerminal + ); let scratch_reads = if include_process_defaults { let (reads, writes) = scratch::scratch_access_roots( file_system_sandbox_policy, @@ -1023,6 +1028,13 @@ pub(crate) fn create_seatbelt_command_args_with_profile( policy_sections.push("(deny system-fcntl (fcntl-command 80 110))".to_string()); } + // Deny injecting input into the shell that resumes after this child. Keep the deny last. + if profile == MacosSeatbeltProfile::ProcessWithInheritedTerminal { + policy_sections.push(format!( + "(deny file-ioctl (ioctl-command {}))", + libc::TIOCSTI + )); + } let full_policy = policy_sections.join("\n"); let dir_params = [