From 70d4255207cff6252cb09bdbfac5e700cf0b54e9 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Mon, 6 Jul 2026 16:42:07 -0300 Subject: [PATCH] feat(terminal-browser): support managed proxy sandbox --- codex-rs/exec-server/src/fs_sandbox.rs | 2 + codex-rs/exec-server/src/process_sandbox.rs | 2 + codex-rs/linux-sandbox/src/lib.rs | 2 + codex-rs/linux-sandbox/src/linux_run_main.rs | 52 ++++++++ .../linux-sandbox/src/linux_run_main_tests.rs | 43 +++++++ .../src/runtime_proxy_argument.rs | 80 ++++++++++++ .../src/runtime_proxy_argument_tests.rs | 59 +++++++++ codex-rs/sandboxing/src/lib.rs | 5 + codex-rs/sandboxing/src/manager.rs | 71 ++++++++++- codex-rs/sandboxing/src/manager_tests.rs | 40 ++++++ codex-rs/terminal-browser/src/sandbox.rs | 80 +++++++++--- .../terminal-browser/src/sandbox_tests.rs | 116 ++++++++++++++++-- 12 files changed, 524 insertions(+), 28 deletions(-) create mode 100644 codex-rs/linux-sandbox/src/runtime_proxy_argument.rs create mode 100644 codex-rs/linux-sandbox/src/runtime_proxy_argument_tests.rs diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 812d76751c..519a0ac8bc 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -9,6 +9,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxDirectSpawnRuntime; use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxManager; @@ -133,6 +134,7 @@ impl FileSystemSandboxRunner { workspace_roots, windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode::Preserve, + runtime: SandboxDirectSpawnRuntime::default(), transform: SandboxTransformRequest { command, permissions: permission_profile, diff --git a/codex-rs/exec-server/src/process_sandbox.rs b/codex-rs/exec-server/src/process_sandbox.rs index b5dd54e0b2..865c66d5e5 100644 --- a/codex-rs/exec-server/src/process_sandbox.rs +++ b/codex-rs/exec-server/src/process_sandbox.rs @@ -5,6 +5,7 @@ use codex_network_proxy::CUSTOM_CA_ENV_KEYS; use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path; use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxDirectSpawnRuntime; use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; @@ -109,6 +110,7 @@ pub(crate) fn prepare_exec_request( workspace_roots, windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile, + runtime: SandboxDirectSpawnRuntime::default(), transform: SandboxTransformRequest { // TODO(jif): Preserve params.arg0 for the inner command across the sandbox // wrapper, or reject sandboxed requests with a custom arg0. diff --git a/codex-rs/linux-sandbox/src/lib.rs b/codex-rs/linux-sandbox/src/lib.rs index 478cd6c379..84fee757db 100644 --- a/codex-rs/linux-sandbox/src/lib.rs +++ b/codex-rs/linux-sandbox/src/lib.rs @@ -19,6 +19,8 @@ mod launcher; mod linux_run_main; #[cfg(target_os = "linux")] mod proxy_routing; +#[cfg(target_os = "linux")] +mod runtime_proxy_argument; #[cfg(target_os = "linux")] pub fn run_main() -> ! { diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 346b1f14c0..e81cba8981 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -25,10 +25,13 @@ use crate::launcher::exec_bwrap; use crate::launcher::preferred_bwrap_supports_argv0; use crate::proxy_routing::activate_proxy_routes_in_netns; use crate::proxy_routing::prepare_host_proxy_route_spec; +use crate::runtime_proxy_argument::rewrite_http_proxy_argument_from_env; use codex_protocol::error::Result as CodexResult; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; +use codex_sandboxing::SandboxDirectSpawnRuntime; +use codex_sandboxing::SandboxRuntimeProxyArgument; use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; static BWRAP_CHILD_PID: AtomicI32 = AtomicI32::new(0); @@ -126,6 +129,14 @@ pub struct LandlockCommand { #[arg(long = "proxy-route-spec", hide = true)] pub proxy_route_spec: Option, + /// Internal: rewrite one command argument from the effective in-namespace HTTP proxy. + #[arg( + long = "rewrite-http-proxy-argument-prefix", + hide = true, + allow_hyphen_values = true + )] + pub rewrite_http_proxy_argument_prefix: Option, + /// When set, skip mounting a fresh `/proc` even though PID isolation is /// still enabled. This is primarily intended for restrictive container /// environments that deny `--proc /proc`. @@ -153,6 +164,7 @@ pub fn run_main() -> ! { apply_seccomp_then_exec, allow_network_for_proxy, proxy_route_spec, + rewrite_http_proxy_argument_prefix, no_proc, command, } = LandlockCommand::parse(); @@ -161,6 +173,15 @@ pub fn run_main() -> ! { panic!("No command specified to execute."); } ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec, use_legacy_landlock); + let runtime = SandboxDirectSpawnRuntime { + proxy_argument: match rewrite_http_proxy_argument_prefix { + Some(argument_prefix) => { + SandboxRuntimeProxyArgument::RewriteFromHttpProxy { argument_prefix } + } + None => SandboxRuntimeProxyArgument::Unchanged, + }, + }; + ensure_direct_spawn_runtime_is_valid(&runtime, allow_network_for_proxy); let EffectivePermissions { permission_profile, file_system_sandbox_policy, @@ -176,6 +197,7 @@ pub fn run_main() -> ! { // Inner stage: apply seccomp/no_new_privs after bubblewrap has already // established the filesystem view. if apply_seccomp_then_exec { + let mut command = command; if allow_network_for_proxy { let spec = proxy_route_spec .as_deref() @@ -183,6 +205,13 @@ pub fn run_main() -> ! { if let Err(err) = activate_proxy_routes_in_netns(spec) { panic!("error activating Linux proxy routing bridge: {err}"); } + if let SandboxRuntimeProxyArgument::RewriteFromHttpProxy { argument_prefix } = + &runtime.proxy_argument + && let Err(err) = + rewrite_http_proxy_argument_from_env(&mut command, argument_prefix) + { + panic!("error applying effective proxy endpoint to command: {err}"); + } } let proxy_routing_active = allow_network_for_proxy; if let Err(e) = apply_permission_profile_to_current_thread( @@ -228,6 +257,7 @@ pub fn run_main() -> ! { permission_profile: &permission_profile, allow_network_for_proxy, proxy_route_spec, + runtime: runtime.clone(), command, }); run_bwrap_with_proc_fallback( @@ -254,6 +284,20 @@ pub fn run_main() -> ! { exec_or_panic(command); } +fn ensure_direct_spawn_runtime_is_valid( + runtime: &SandboxDirectSpawnRuntime, + allow_network_for_proxy: bool, +) { + if !allow_network_for_proxy + && !matches!( + &runtime.proxy_argument, + SandboxRuntimeProxyArgument::Unchanged + ) + { + panic!("runtime proxy argument rewrite requires managed proxy routing"); + } +} + #[derive(Debug, Clone)] struct EffectivePermissions { permission_profile: PermissionProfile, @@ -1394,6 +1438,7 @@ struct InnerSeccompCommandArgs<'a> { permission_profile: &'a PermissionProfile, allow_network_for_proxy: bool, proxy_route_spec: Option, + runtime: SandboxDirectSpawnRuntime, command: Vec, } @@ -1405,6 +1450,7 @@ fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec permission_profile, allow_network_for_proxy, proxy_route_spec, + runtime, command, } = args; let current_exe = match std::env::current_exe() { @@ -1437,6 +1483,12 @@ fn build_inner_seccomp_command(args: InnerSeccompCommandArgs<'_>) -> Vec inner.push("--proxy-route-spec".to_string()); inner.push(proxy_route_spec); } + if let SandboxRuntimeProxyArgument::RewriteFromHttpProxy { argument_prefix } = + runtime.proxy_argument + { + inner.push("--rewrite-http-proxy-argument-prefix".to_string()); + inner.push(argument_prefix); + } inner.push("--".to_string()); inner.extend(command); inner diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index 4441af7809..b9008db8fc 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -5,6 +5,8 @@ use crate::linux_run_main::install_bwrap_signal_forwarders; #[cfg(test)] use crate::linux_run_main::wait_for_bwrap_child; #[cfg(test)] +use clap::Parser as _; +#[cfg(test)] use codex_protocol::models::PermissionProfile; #[cfg(test)] use codex_protocol::protocol::FileSystemSandboxPolicy; @@ -468,11 +470,49 @@ fn managed_proxy_inner_command_includes_route_spec() { permission_profile: &permission_profile, allow_network_for_proxy: true, proxy_route_spec: Some("{\"routes\":[]}".to_string()), + runtime: SandboxDirectSpawnRuntime { + proxy_argument: SandboxRuntimeProxyArgument::RewriteFromHttpProxy { + argument_prefix: "--proxy-server=".to_string(), + }, + }, command: vec!["/bin/true".to_string()], }); assert!(args.iter().any(|arg| arg == "--proxy-route-spec")); assert!(args.iter().any(|arg| arg == "{\"routes\":[]}")); + assert!( + args.windows(2).any(|window| { + window == ["--rewrite-http-proxy-argument-prefix", "--proxy-server="] + }) + ); +} + +#[test] +fn helper_cli_accepts_hyphen_prefixed_runtime_proxy_argument_prefix() { + let parsed = LandlockCommand::try_parse_from([ + "codex-linux-sandbox", + "--sandbox-policy-cwd", + "/tmp", + "--allow-network-for-proxy", + "--rewrite-http-proxy-argument-prefix", + "--proxy-server=", + "--", + "carbonyl", + "--proxy-server=http://127.0.0.1:43128", + ]) + .expect("parse Linux sandbox helper arguments"); + + assert_eq!( + parsed.rewrite_http_proxy_argument_prefix.as_deref(), + Some("--proxy-server=") + ); + assert_eq!( + parsed.command, + vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:43128".to_string(), + ] + ); } #[test] @@ -484,6 +524,7 @@ fn inner_command_includes_permission_profile_flag() { permission_profile: &permission_profile, allow_network_for_proxy: false, proxy_route_spec: None, + runtime: SandboxDirectSpawnRuntime::default(), command: vec!["/bin/true".to_string()], }); @@ -503,6 +544,7 @@ fn non_managed_inner_command_omits_route_spec() { permission_profile: &permission_profile, allow_network_for_proxy: false, proxy_route_spec: None, + runtime: SandboxDirectSpawnRuntime::default(), command: vec!["/bin/true".to_string()], }); @@ -519,6 +561,7 @@ fn managed_proxy_inner_command_requires_route_spec() { permission_profile: &permission_profile, allow_network_for_proxy: true, proxy_route_spec: None, + runtime: SandboxDirectSpawnRuntime::default(), command: vec!["/bin/true".to_string()], }) }); diff --git a/codex-rs/linux-sandbox/src/runtime_proxy_argument.rs b/codex-rs/linux-sandbox/src/runtime_proxy_argument.rs new file mode 100644 index 0000000000..5154f8fa56 --- /dev/null +++ b/codex-rs/linux-sandbox/src/runtime_proxy_argument.rs @@ -0,0 +1,80 @@ +use std::io; +use std::net::IpAddr; +use std::net::Ipv4Addr; +use std::net::SocketAddr; + +use url::Url; + +/// Replaces exactly one non-program argument with the effective loopback HTTP proxy endpoint. +/// +/// This runs only after the network-namespace bridge rewrites `HTTP_PROXY`. The argument prefix +/// comes from trusted direct-spawn orchestration metadata; no shell interpolation occurs. +pub(crate) fn rewrite_http_proxy_argument_from_env( + command: &mut [String], + argument_prefix: &str, +) -> io::Result<()> { + let proxy_url = std::env::var("HTTP_PROXY").map_err(|_| { + io::Error::new( + io::ErrorKind::NotFound, + "runtime proxy argument rewrite requires HTTP_PROXY", + ) + })?; + rewrite_http_proxy_argument(command, argument_prefix, &proxy_url) +} + +fn rewrite_http_proxy_argument( + command: &mut [String], + argument_prefix: &str, + proxy_url: &str, +) -> io::Result<()> { + if argument_prefix.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "runtime proxy argument prefix must not be empty", + )); + } + let endpoint = parse_loopback_http_proxy_endpoint(proxy_url).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "runtime HTTP proxy must be a parseable nonzero loopback endpoint", + ) + })?; + let matching_indices = command + .iter() + .enumerate() + .skip(1) + .filter_map(|(index, argument)| argument.starts_with(argument_prefix).then_some(index)) + .collect::>(); + let [argument_index] = matching_indices.as_slice() else { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "runtime proxy argument rewrite expected exactly one `{argument_prefix}` argument" + ), + )); + }; + command[*argument_index] = format!("{argument_prefix}http://{endpoint}"); + Ok(()) +} + +fn parse_loopback_http_proxy_endpoint(proxy_url: &str) -> Option { + let parsed = Url::parse(proxy_url).ok()?; + if parsed.scheme() != "http" { + return None; + } + let host = parsed.host_str()?; + let port = parsed.port_or_known_default()?; + if port == 0 { + return None; + } + let ip = if host.eq_ignore_ascii_case("localhost") { + IpAddr::V4(Ipv4Addr::LOCALHOST) + } else { + host.parse::().ok()? + }; + ip.is_loopback().then_some(SocketAddr::new(ip, port)) +} + +#[cfg(test)] +#[path = "runtime_proxy_argument_tests.rs"] +mod tests; diff --git a/codex-rs/linux-sandbox/src/runtime_proxy_argument_tests.rs b/codex-rs/linux-sandbox/src/runtime_proxy_argument_tests.rs new file mode 100644 index 0000000000..f3ccae4c9c --- /dev/null +++ b/codex-rs/linux-sandbox/src/runtime_proxy_argument_tests.rs @@ -0,0 +1,59 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn rewrites_runtime_proxy_argument_to_effective_loopback_endpoint() { + let mut command = vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:43128".to_string(), + "--remote-debugging-pipe".to_string(), + ]; + + rewrite_http_proxy_argument(&mut command, "--proxy-server=", "http://127.0.0.1:45219") + .expect("rewrite proxy argument"); + + assert_eq!( + command, + vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:45219".to_string(), + "--remote-debugging-pipe".to_string(), + ] + ); +} + +#[test] +fn runtime_proxy_argument_rewrite_rejects_ambiguous_or_non_loopback_inputs() { + let mut duplicate = vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:1".to_string(), + "--proxy-server=http://127.0.0.1:2".to_string(), + ]; + let duplicate_error = + rewrite_http_proxy_argument(&mut duplicate, "--proxy-server=", "http://127.0.0.1:45219") + .expect_err("duplicate proxy arguments must be rejected"); + assert_eq!(duplicate_error.kind(), std::io::ErrorKind::InvalidInput); + + let mut command = vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:1".to_string(), + ]; + let endpoint_error = + rewrite_http_proxy_argument(&mut command, "--proxy-server=", "http://192.0.2.10:45219") + .expect_err("non-loopback effective proxy must be rejected"); + assert_eq!(endpoint_error.kind(), std::io::ErrorKind::InvalidInput); +} + +#[test] +fn runtime_proxy_argument_rewrite_rejects_non_http_proxy_schemes() { + let mut command = vec![ + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:1".to_string(), + ]; + + let error = + rewrite_http_proxy_argument(&mut command, "--proxy-server=", "socks5h://127.0.0.1:45219") + .expect_err("non-HTTP effective proxy must be rejected"); + + assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput); +} diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index c9109146bd..cbe7e6826e 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -15,9 +15,11 @@ pub use bwrap::system_bwrap_warning; pub use codex_windows_sandbox::WindowsSandboxProxySettingsMode; pub use denial::is_likely_sandbox_denied; pub use manager::SandboxCommand; +pub use manager::SandboxDirectSpawnRuntime; pub use manager::SandboxDirectSpawnTransformRequest; pub use manager::SandboxExecRequest; pub use manager::SandboxManager; +pub use manager::SandboxRuntimeProxyArgument; pub use manager::SandboxTransformError; pub use manager::SandboxTransformRequest; pub use manager::SandboxType; @@ -54,6 +56,9 @@ impl From for CodexErr { SandboxTransformError::EnvironmentNetworkProxy(message) => { CodexErr::UnsupportedOperation(message) } + SandboxTransformError::InvalidDirectSpawnRuntime(message) => { + CodexErr::UnsupportedOperation(message) + } #[cfg(target_os = "linux")] SandboxTransformError::Wsl1UnsupportedForBubblewrap => { CodexErr::UnsupportedOperation(crate::bwrap::WSL1_BWRAP_WARNING.to_string()) diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 51d2d6c98e..b3cb7bb1eb 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -153,6 +153,26 @@ pub struct SandboxDirectSpawnTransformRequest<'a> { pub transform: SandboxTransformRequest<'a>, pub workspace_roots: &'a [AbsolutePathBuf], pub windows_sandbox_proxy_settings_mode: codex_windows_sandbox::WindowsSandboxProxySettingsMode, + /// Runtime-only process requirements that platform wrappers must preserve. + pub runtime: SandboxDirectSpawnRuntime, +} + +/// Runtime requirements for commands spawned directly through a platform sandbox wrapper. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct SandboxDirectSpawnRuntime { + pub proxy_argument: SandboxRuntimeProxyArgument, +} + +/// A command argument whose endpoint must follow a sandbox runtime proxy rewrite. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub enum SandboxRuntimeProxyArgument { + #[default] + Unchanged, + /// Replace one existing argument with `argument_prefix` using the effective `HTTP_PROXY`. + /// + /// The Linux helper validates that the rewritten endpoint remains loopback-only and replaces + /// exactly one non-program argument. It never evaluates the argument through a shell. + RewriteFromHttpProxy { argument_prefix: String }, } // TODO(anp): Revisit this preparation type once this module's PathUri migration is complete. @@ -213,6 +233,7 @@ pub enum SandboxTransformError { }, MissingLinuxSandboxExecutable, EnvironmentNetworkProxy(String), + InvalidDirectSpawnRuntime(String), #[cfg(target_os = "linux")] Wsl1UnsupportedForBubblewrap, #[cfg(not(target_os = "macos"))] @@ -240,6 +261,9 @@ impl std::fmt::Display for SandboxTransformError { Self::EnvironmentNetworkProxy(err) => { write!(f, "failed to prepare environment network proxy: {err}") } + Self::InvalidDirectSpawnRuntime(err) => { + write!(f, "invalid direct-spawn runtime requirements: {err}") + } #[cfg(target_os = "linux")] Self::Wsl1UnsupportedForBubblewrap => write!(f, "{WSL1_BWRAP_WARNING}"), #[cfg(not(target_os = "macos"))] @@ -259,6 +283,7 @@ impl std::error::Error for SandboxTransformError { | Self::InvalidSandboxPolicyCwd { source, .. } => Some(source), Self::MissingLinuxSandboxExecutable => None, Self::EnvironmentNetworkProxy(_) => None, + Self::InvalidDirectSpawnRuntime(_) => None, #[cfg(target_os = "linux")] Self::Wsl1UnsupportedForBubblewrap => None, #[cfg(not(target_os = "macos"))] @@ -474,7 +499,20 @@ impl SandboxManager { #[cfg(not(target_os = "windows"))] { - self.transform(request.transform) + let transformed = self.transform(request.transform)?; + #[cfg(target_os = "linux")] + { + let mut transformed = transformed; + if transformed.sandbox == SandboxType::LinuxSeccomp { + encode_linux_direct_spawn_runtime(&mut transformed.command, request.runtime)?; + } + Ok(transformed) + } + #[cfg(not(target_os = "linux"))] + { + let _ = request.runtime; + Ok(transformed) + } } } @@ -486,6 +524,7 @@ impl SandboxManager { ) -> Result { let workspace_roots = request.workspace_roots; let proxy_settings_mode = request.windows_sandbox_proxy_settings_mode; + let _runtime = request.runtime; let mut request = self.transform(request.transform)?; if request.sandbox == SandboxType::WindowsRestrictedToken { wrap_windows_sandbox_exec_request_for_direct_spawn( @@ -499,6 +538,36 @@ impl SandboxManager { } } +#[cfg(any(target_os = "linux", test))] +fn encode_linux_direct_spawn_runtime( + command: &mut Vec, + runtime: SandboxDirectSpawnRuntime, +) -> Result<(), SandboxTransformError> { + if runtime == SandboxDirectSpawnRuntime::default() { + return Ok(()); + } + let separator_index = command.iter().position(|arg| arg == "--").ok_or_else(|| { + SandboxTransformError::InvalidDirectSpawnRuntime( + "Linux sandbox command is missing its argument separator".to_string(), + ) + })?; + let mut runtime_args = Vec::new(); + match runtime.proxy_argument { + SandboxRuntimeProxyArgument::Unchanged => {} + SandboxRuntimeProxyArgument::RewriteFromHttpProxy { argument_prefix } => { + if argument_prefix.is_empty() { + return Err(SandboxTransformError::InvalidDirectSpawnRuntime( + "runtime proxy argument prefix must not be empty".to_string(), + )); + } + runtime_args.push("--rewrite-http-proxy-argument-prefix".to_string()); + runtime_args.push(argument_prefix); + } + } + command.splice(separator_index..separator_index, runtime_args); + Ok(()) +} + #[cfg(target_os = "windows")] fn wrap_windows_sandbox_exec_request_for_direct_spawn( request: &mut SandboxExecRequest, diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index c1a52182d7..562bb1e55f 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -1,10 +1,13 @@ use super::SandboxCommand; +use super::SandboxDirectSpawnRuntime; #[cfg(target_os = "windows")] use super::SandboxDirectSpawnTransformRequest; use super::SandboxManager; +use super::SandboxRuntimeProxyArgument; use super::SandboxTransformRequest; use super::SandboxType; use super::SandboxablePreference; +use super::encode_linux_direct_spawn_runtime; use super::get_platform_sandbox; use super::with_managed_mitm_ca_readable_root; use codex_protocol::config_types::WindowsSandboxLevel; @@ -73,6 +76,42 @@ fn restricted_file_system_uses_platform_sandbox_without_managed_network() { assert_eq!(sandbox, expected); } +#[test] +fn linux_direct_spawn_runtime_is_encoded_before_the_inner_command() { + let mut command = vec![ + "codex-linux-sandbox".to_string(), + "--sandbox-policy-cwd".to_string(), + "/workspace".to_string(), + "--".to_string(), + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:43128".to_string(), + ]; + + encode_linux_direct_spawn_runtime( + &mut command, + SandboxDirectSpawnRuntime { + proxy_argument: SandboxRuntimeProxyArgument::RewriteFromHttpProxy { + argument_prefix: "--proxy-server=".to_string(), + }, + }, + ) + .expect("encode runtime requirements"); + + assert_eq!( + command, + vec![ + "codex-linux-sandbox".to_string(), + "--sandbox-policy-cwd".to_string(), + "/workspace".to_string(), + "--rewrite-http-proxy-argument-prefix".to_string(), + "--proxy-server=".to_string(), + "--".to_string(), + "carbonyl".to_string(), + "--proxy-server=http://127.0.0.1:43128".to_string(), + ] + ); +} + #[test] fn unsandboxed_transform_preserves_foreign_cwd_and_unrestricted_file_system_policy() { let manager = SandboxManager::new(); @@ -501,6 +540,7 @@ fn transform_for_direct_spawn_windows_materializes_inner_helper() { workspace_roots: workspace_roots.as_slice(), windows_sandbox_proxy_settings_mode: codex_windows_sandbox::WindowsSandboxProxySettingsMode::Preserve, + runtime: SandboxDirectSpawnRuntime::default(), transform: SandboxTransformRequest { command: SandboxCommand { program: configured_helper.as_os_str().to_owned(), diff --git a/codex-rs/terminal-browser/src/sandbox.rs b/codex-rs/terminal-browser/src/sandbox.rs index 10a19ee281..423e91acf1 100644 --- a/codex-rs/terminal-browser/src/sandbox.rs +++ b/codex-rs/terminal-browser/src/sandbox.rs @@ -4,6 +4,7 @@ use std::path::Path; use anyhow::Context; use anyhow::Result; +use codex_network_proxy::ManagedNetworkSandboxContext; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; @@ -13,8 +14,10 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_sandboxing::SandboxCommand; +use codex_sandboxing::SandboxDirectSpawnRuntime; use codex_sandboxing::SandboxDirectSpawnTransformRequest; use codex_sandboxing::SandboxManager; +use codex_sandboxing::SandboxRuntimeProxyArgument; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; @@ -45,6 +48,13 @@ pub(crate) struct PreparedBrowserLaunch { pub(crate) arg0: Option, } +#[derive(Debug, PartialEq, Eq)] +struct BrowserNetworkSandbox { + policy: NetworkSandboxPolicy, + enforce_managed_network: bool, + managed_network: Option, +} + pub(crate) fn prepare_browser_launch( binary: &Path, args: Vec, @@ -54,12 +64,6 @@ pub(crate) fn prepare_browser_launch( network_policy: &BrowserNetworkPolicy, context: &BrowserLaunchContext, ) -> Result { - if matches!(network_policy, BrowserNetworkPolicy::ManagedProxy { .. }) { - anyhow::bail!( - "managed terminal-browser networking is not yet supported because Carbonyl needs a loopback CDP listener that the current sandbox cannot permit without bypassing the managed proxy" - ); - } - let binary = AbsolutePathBuf::from_absolute_path(binary) .context("resolve Carbonyl executable path")? .canonicalize() @@ -70,15 +74,13 @@ pub(crate) fn prepare_browser_launch( ensure_isolated_binary_root(&binary_root, browser_root, profile_root, context)?; let file_system_policy = browser_file_system_policy(browser_root, profile_root, &binary_root, &env)?; - let network_sandbox_policy = match network_policy { - BrowserNetworkPolicy::Disabled | BrowserNetworkPolicy::ManagedProxy { .. } => { - NetworkSandboxPolicy::Restricted - } - BrowserNetworkPolicy::Direct => NetworkSandboxPolicy::Enabled, - }; + let BrowserNetworkSandbox { + policy: network_sandbox_policy, + enforce_managed_network, + managed_network, + } = browser_network_sandbox(network_policy)?; let permissions = PermissionProfile::from_runtime_permissions(&file_system_policy, network_sandbox_policy); - let enforce_managed_network = false; let manager = SandboxManager::new(); let sandbox = manager.select_initial( &file_system_policy, @@ -100,7 +102,7 @@ pub(crate) fn prepare_browser_launch( args, cwd: browser_root_uri.clone(), env, - managed_network: None, + managed_network, additional_permissions: None, }, permissions: &permissions, @@ -119,6 +121,7 @@ pub(crate) fn prepare_browser_launch( }, workspace_roots: &[], windows_sandbox_proxy_settings_mode: WindowsSandboxProxySettingsMode::Reconcile, + runtime: browser_direct_spawn_runtime(network_policy), }) .context("prepare Carbonyl sandbox")?; #[cfg(target_os = "macos")] @@ -157,6 +160,55 @@ pub(crate) fn prepare_browser_launch( }) } +fn browser_direct_spawn_runtime( + network_policy: &BrowserNetworkPolicy, +) -> SandboxDirectSpawnRuntime { + let proxy_argument = match network_policy { + BrowserNetworkPolicy::ManagedProxy { .. } => { + SandboxRuntimeProxyArgument::RewriteFromHttpProxy { + argument_prefix: "--proxy-server=".to_string(), + } + } + BrowserNetworkPolicy::Disabled | BrowserNetworkPolicy::Direct => { + SandboxRuntimeProxyArgument::Unchanged + } + }; + SandboxDirectSpawnRuntime { proxy_argument } +} + +fn browser_network_sandbox(network_policy: &BrowserNetworkPolicy) -> Result { + match network_policy { + BrowserNetworkPolicy::Disabled => Ok(BrowserNetworkSandbox { + policy: NetworkSandboxPolicy::Restricted, + enforce_managed_network: false, + managed_network: None, + }), + BrowserNetworkPolicy::Direct => Ok(BrowserNetworkSandbox { + policy: NetworkSandboxPolicy::Enabled, + enforce_managed_network: false, + managed_network: None, + }), + BrowserNetworkPolicy::ManagedProxy { http_addr } => { + anyhow::ensure!( + http_addr.ip().is_loopback(), + "managed terminal-browser proxy must use a loopback address" + ); + anyhow::ensure!( + http_addr.port() != 0, + "managed terminal-browser proxy must use a nonzero port" + ); + Ok(BrowserNetworkSandbox { + policy: NetworkSandboxPolicy::Restricted, + enforce_managed_network: true, + managed_network: Some(ManagedNetworkSandboxContext { + loopback_ports: vec![http_addr.port()], + allow_local_binding: false, + }), + }) + } + } +} + fn ensure_isolated_binary_root( binary_root: &AbsolutePathBuf, browser_root: &AbsolutePathBuf, diff --git a/codex-rs/terminal-browser/src/sandbox_tests.rs b/codex-rs/terminal-browser/src/sandbox_tests.rs index 8caa4fa4a9..1a57eca287 100644 --- a/codex-rs/terminal-browser/src/sandbox_tests.rs +++ b/codex-rs/terminal-browser/src/sandbox_tests.rs @@ -1,13 +1,21 @@ use std::collections::HashMap; +use codex_network_proxy::ManagedNetworkSandboxContext; use codex_protocol::permissions::FileSystemAccessMode; +use codex_protocol::permissions::NetworkSandboxPolicy; use pretty_assertions::assert_eq; +use super::BrowserNetworkSandbox; +use super::browser_direct_spawn_runtime; use super::browser_file_system_policy; +use super::browser_network_sandbox; use super::ensure_isolated_binary_root; +#[cfg(target_os = "macos")] use super::prepare_browser_launch; use crate::network::BrowserNetworkPolicy; use crate::sandbox::BrowserLaunchContext; +use codex_sandboxing::SandboxDirectSpawnRuntime; +use codex_sandboxing::SandboxRuntimeProxyArgument; use codex_utils_absolute_path::AbsolutePathBuf; #[test] @@ -72,28 +80,110 @@ fn browser_policy_can_write_only_its_runtime_root() { } #[test] -fn managed_proxy_launches_fail_closed() { +fn managed_proxy_enforces_only_its_exact_loopback_port() { + let http_addr = "127.0.0.1:8080".parse().expect("proxy address"); + + assert_eq!( + browser_network_sandbox(&BrowserNetworkPolicy::ManagedProxy { http_addr }) + .expect("managed proxy sandbox"), + BrowserNetworkSandbox { + policy: NetworkSandboxPolicy::Restricted, + enforce_managed_network: true, + managed_network: Some(ManagedNetworkSandboxContext { + loopback_ports: vec![8_080], + allow_local_binding: false, + }), + }, + ); +} + +#[test] +fn carbonyl_direct_spawn_runtime_rewrites_only_managed_proxy() { + assert_eq!( + browser_direct_spawn_runtime(&BrowserNetworkPolicy::Direct), + SandboxDirectSpawnRuntime { + proxy_argument: SandboxRuntimeProxyArgument::Unchanged, + } + ); + assert_eq!( + browser_direct_spawn_runtime(&BrowserNetworkPolicy::ManagedProxy { + http_addr: "127.0.0.1:43128".parse().expect("proxy address"), + }), + SandboxDirectSpawnRuntime { + proxy_argument: SandboxRuntimeProxyArgument::RewriteFromHttpProxy { + argument_prefix: "--proxy-server=".to_string(), + }, + } + ); +} + +#[test] +fn direct_network_preserves_the_unrestricted_network_policy() { + assert_eq!( + browser_network_sandbox(&BrowserNetworkPolicy::Direct).expect("direct network sandbox"), + BrowserNetworkSandbox { + policy: NetworkSandboxPolicy::Enabled, + enforce_managed_network: false, + managed_network: None, + }, + ); +} + +#[test] +fn managed_proxy_rejects_non_loopback_addresses() { + let http_addr = "192.0.2.10:8080".parse().expect("proxy address"); + + let error = browser_network_sandbox(&BrowserNetworkPolicy::ManagedProxy { http_addr }) + .expect_err("non-loopback proxy must be rejected"); + + assert_eq!( + error.to_string(), + "managed terminal-browser proxy must use a loopback address" + ); +} + +#[test] +fn managed_proxy_rejects_port_zero() { + let http_addr = "127.0.0.1:0".parse().expect("proxy address"); + + let error = browser_network_sandbox(&BrowserNetworkPolicy::ManagedProxy { http_addr }) + .expect_err("port-zero proxy must be rejected"); + + assert_eq!( + error.to_string(), + "managed terminal-browser proxy must use a nonzero port" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn prepared_managed_proxy_seatbelt_policy_grants_only_the_proxy_port() { let root = tempfile::tempdir().expect("test root"); let root = AbsolutePathBuf::from_absolute_path(root.path()).expect("absolute test root"); - let Err(error) = prepare_browser_launch( - root.join("carbonyl").as_path(), + let binary_root = root.join("carbonyl-bundle"); + std::fs::create_dir_all(binary_root.as_path()).expect("create Carbonyl bundle"); + let binary = binary_root.join("carbonyl"); + std::fs::write(binary.as_path(), "test").expect("create Carbonyl binary"); + let browser_root = root.join("runtime"); + let launch = prepare_browser_launch( + binary.as_path(), Vec::new(), - &root.join("runtime"), - &root.join("runtime/profile"), + &browser_root, + &browser_root.join("profile"), HashMap::new(), &BrowserNetworkPolicy::ManagedProxy { http_addr: "127.0.0.1:8080".parse().expect("proxy address"), }, &BrowserLaunchContext::default(), - ) else { - panic!("managed proxy must be rejected"); - }; + ) + .expect("prepare managed Carbonyl launch"); - assert!( - error - .to_string() - .contains("without bypassing the managed proxy") - ); + assert_eq!(launch.program, "/usr/bin/sandbox-exec"); + assert_eq!(launch.args.first().map(String::as_str), Some("-p")); + let policy = launch.args.get(/*index*/ 1).expect("Seatbelt policy"); + assert!(policy.contains("(allow network-outbound (remote ip \"localhost:8080\"))")); + assert!(!policy.contains("(allow network-bind (local ip \"*:*\"))")); + assert!(!policy.contains("(allow network-outbound)\n")); } #[test]