diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 02625d90c..9fc695795 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1595,6 +1595,7 @@ dependencies = [ "anyhow", "codex-apply-patch", "codex-linux-sandbox", + "codex-sandboxing", "codex-shell-escalation", "codex-utils-home-dir", "dotenvy", @@ -2252,6 +2253,7 @@ dependencies = [ "clap", "codex-core", "codex-protocol", + "codex-sandboxing", "codex-utils-absolute-path", "landlock", "libc", diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 4687ec5ba..620a85a09 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -537,6 +537,7 @@ impl CodexMessageProcessor { data: None, })?; apply_runtime_feature_enablement(&mut config, &self.current_runtime_feature_enablement()); + config.codex_self_exe = self.arg0_paths.codex_self_exe.clone(); config.codex_linux_sandbox_exe = self.arg0_paths.codex_linux_sandbox_exe.clone(); config.main_execve_wrapper_exe = self.arg0_paths.main_execve_wrapper_exe.clone(); Ok(config) diff --git a/codex-rs/arg0/Cargo.toml b/codex-rs/arg0/Cargo.toml index abe7277d9..cd409fedd 100644 --- a/codex-rs/arg0/Cargo.toml +++ b/codex-rs/arg0/Cargo.toml @@ -15,6 +15,7 @@ workspace = true anyhow = { workspace = true } codex-apply-patch = { workspace = true } codex-linux-sandbox = { workspace = true } +codex-sandboxing = { workspace = true } codex-shell-escalation = { workspace = true } codex-utils-home-dir = { workspace = true } dotenvy = { workspace = true } diff --git a/codex-rs/arg0/src/lib.rs b/codex-rs/arg0/src/lib.rs index e210783a3..efad6c248 100644 --- a/codex-rs/arg0/src/lib.rs +++ b/codex-rs/arg0/src/lib.rs @@ -4,12 +4,12 @@ use std::path::Path; use std::path::PathBuf; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; +use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_utils_home_dir::find_codex_home; #[cfg(unix)] use std::os::unix::fs::symlink; use tempfile::TempDir; -const LINUX_SANDBOX_ARG0: &str = "codex-linux-sandbox"; const APPLY_PATCH_ARG0: &str = "apply_patch"; const MISSPELLED_APPLY_PATCH_ARG0: &str = "applypatch"; #[cfg(unix)] @@ -19,6 +19,12 @@ const TOKIO_WORKER_STACK_SIZE_BYTES: usize = 16 * 1024 * 1024; #[derive(Clone, Debug, Default, Eq, PartialEq)] pub struct Arg0DispatchPaths { + /// Stable path to the current Codex executable for child re-execs. + /// + /// Prefer this over [`std::env::current_exe()`] in code that may run under + /// a test harness, where `current_exe()` can point at the harness binary + /// instead of the real Codex CLI. + pub codex_self_exe: Option, pub codex_linux_sandbox_exe: Option, pub main_execve_wrapper_exe: Option, } @@ -79,7 +85,7 @@ pub fn arg0_dispatch() -> Option { } } - if exe_name == LINUX_SANDBOX_ARG0 { + if exe_name == CODEX_LINUX_SANDBOX_ARG0 { // Safety: [`run_main`] never returns. codex_linux_sandbox::run_main(); } else if exe_name == APPLY_PATCH_ARG0 || exe_name == MISSPELLED_APPLY_PATCH_ARG0 { @@ -133,8 +139,10 @@ pub fn arg0_dispatch() -> Option { /// /// 1. Load `.env` values from `~/.codex/.env` before creating any threads. /// 2. Construct a Tokio multi-thread runtime. -/// 3. Derive the path to the current executable (so children can re-invoke the -/// sandbox) when running on Linux. +/// 3. Capture the current executable path and derive the +/// `codex-linux-sandbox` helper path (falling back to the current +/// executable if needed) so children can re-invoke the sandbox when running +/// on Linux. /// 4. Execute the provided async `main_fn` inside that runtime, forwarding any /// error. Note that `main_fn` receives [`Arg0DispatchPaths`], which /// contains the helper executable paths needed to construct @@ -150,7 +158,7 @@ where // Retain the TempDir so it exists for the lifetime of the invocation of // this executable. Admittedly, we could invoke `keep()` on it, but it // would be nice to avoid leaving temporary directories behind, if possible. - let path_entry = arg0_dispatch(); + let path_entry_guard = arg0_dispatch(); // Regular invocation – create a Tokio runtime and execute the provided // async entry-point. @@ -158,16 +166,13 @@ where runtime.block_on(async move { let current_exe = std::env::current_exe().ok(); let paths = Arg0DispatchPaths { + codex_self_exe: current_exe.clone(), codex_linux_sandbox_exe: if cfg!(target_os = "linux") { - current_exe.or_else(|| { - path_entry - .as_ref() - .and_then(|path_entry| path_entry.paths().codex_linux_sandbox_exe.clone()) - }) + linux_sandbox_exe_path(path_entry_guard.as_ref(), current_exe) } else { None }, - main_execve_wrapper_exe: path_entry + main_execve_wrapper_exe: path_entry_guard .as_ref() .and_then(|path_entry| path_entry.paths().main_execve_wrapper_exe.clone()), }; @@ -176,6 +181,18 @@ where }) } +fn linux_sandbox_exe_path( + path_entry_guard: Option<&Arg0PathEntryGuard>, + current_exe: Option, +) -> Option { + // Prefer the `codex-linux-sandbox` alias when available so callers can + // re-exec through a path whose basename still triggers arg0 dispatch on + // bubblewrap builds that do not support `--argv0`. + path_entry_guard + .and_then(|path_entry| path_entry.paths().codex_linux_sandbox_exe.clone()) + .or(current_exe) +} + fn build_runtime() -> anyhow::Result { let mut builder = tokio::runtime::Builder::new_multi_thread(); builder.enable_all(); @@ -276,7 +293,7 @@ pub fn prepend_path_entry_for_codex_aliases() -> std::io::Result std::io::Result std::io::Result> { #[cfg(test)] mod tests { + use super::Arg0DispatchPaths; + use super::Arg0PathEntryGuard; use super::LOCK_FILENAME; use super::janitor_cleanup; + use super::linux_sandbox_exe_path; use std::fs; use std::fs::File; use std::path::Path; + use std::path::PathBuf; + use tempfile::TempDir; fn create_lock(dir: &Path) -> std::io::Result { let lock_path = dir.join(LOCK_FILENAME); @@ -413,6 +436,28 @@ mod tests { .open(lock_path) } + #[test] + fn linux_sandbox_exe_path_prefers_codex_linux_sandbox_alias() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let lock_file = create_lock(temp_dir.path())?; + let alias_path = temp_dir.path().join("codex-linux-sandbox"); + let path_entry = Arg0PathEntryGuard::new( + temp_dir, + lock_file, + Arg0DispatchPaths { + codex_self_exe: Some(PathBuf::from("/usr/bin/codex")), + codex_linux_sandbox_exe: Some(alias_path.clone()), + main_execve_wrapper_exe: None, + }, + ); + + assert_eq!( + linux_sandbox_exe_path(Some(&path_entry), Some(PathBuf::from("/usr/bin/codex"))), + Some(alias_path), + ); + Ok(()) + } + #[test] fn janitor_skips_dirs_without_lock_file() -> std::io::Result<()> { let root = tempfile::tempdir()?; diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index c154ef463..4b8cb793b 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -59,10 +59,11 @@ only when the split filesystem policy round-trips through the legacy cases like `/repo = write`, `/repo/a = none`, `/repo/a/b = write`, where the more specific writable child must reopen under a denied parent. -The Linux sandbox helper prefers `/usr/bin/bwrap` whenever it is available and -supports the required argv-rewrite flags, and falls back to the vendored -bubblewrap path compiled into the binary otherwise. When `/usr/bin/bwrap` is -missing or too old to support the required flags, Codex also surfaces a startup +The Linux sandbox helper prefers `/usr/bin/bwrap` whenever it is available. If +`/usr/bin/bwrap` is present but too old to support `--argv0`, the helper keeps +using system bubblewrap and switches to a no-`--argv0` compatibility path for +the inner re-exec. If `/usr/bin/bwrap` is missing, it falls back to the +vendored bubblewrap path compiled into the binary and Codex surfaces a startup warning through its normal notification path instead of printing directly from the sandbox helper. diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index a4bdb2c94..f29d37fe2 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -867,6 +867,7 @@ pub(crate) struct TurnContext { pub(crate) features: ManagedFeatures, pub(crate) ghost_snapshot: GhostSnapshotConfig, pub(crate) final_output_json_schema: Option, + pub(crate) codex_self_exe: Option, pub(crate) codex_linux_sandbox_exe: Option, pub(crate) tool_call_gate: Arc, pub(crate) truncation_policy: TruncationPolicy, @@ -974,6 +975,7 @@ impl TurnContext { features, ghost_snapshot: self.ghost_snapshot.clone(), final_output_json_schema: self.final_output_json_schema.clone(), + codex_self_exe: self.codex_self_exe.clone(), codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(), tool_call_gate: Arc::new(ReadinessFlag::new()), truncation_policy, @@ -1431,6 +1433,7 @@ impl Session { features: per_turn_config.features.clone(), ghost_snapshot: per_turn_config.ghost_snapshot.clone(), final_output_json_schema: None, + codex_self_exe: per_turn_config.codex_self_exe.clone(), codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(), tool_call_gate: Arc::new(ReadinessFlag::new()), truncation_policy: model_info.truncation_policy.into(), @@ -5477,6 +5480,7 @@ async fn spawn_review_thread( shell_environment_policy: parent_turn_context.shell_environment_policy.clone(), cwd: parent_turn_context.cwd.clone(), final_output_json_schema: None, + codex_self_exe: parent_turn_context.codex_self_exe.clone(), codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(), tool_call_gate: Arc::new(ReadinessFlag::new()), js_repl: Arc::clone(&sess.js_repl), diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 84ac6d71b..7d2f6c573 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -4417,6 +4417,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, + codex_self_exe: None, codex_linux_sandbox_exe: None, main_execve_wrapper_exe: None, js_repl_node_path: None, @@ -4559,6 +4560,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, + codex_self_exe: None, codex_linux_sandbox_exe: None, main_execve_wrapper_exe: None, js_repl_node_path: None, @@ -4699,6 +4701,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, + codex_self_exe: None, codex_linux_sandbox_exe: None, main_execve_wrapper_exe: None, js_repl_node_path: None, @@ -4825,6 +4828,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { history: History::default(), ephemeral: false, file_opener: UriBasedFileOpener::VsCode, + codex_self_exe: None, codex_linux_sandbox_exe: None, main_execve_wrapper_exe: None, js_repl_node_path: None, @@ -5708,7 +5712,7 @@ fn system_bwrap_warning_reports_missing_system_bwrap() { #[cfg(target_os = "linux")] #[test] -fn system_bwrap_warning_reports_too_old_system_bwrap() { +fn system_bwrap_warning_skips_too_old_system_bwrap() { let fake_bwrap = write_fake_bwrap( r#"#!/bin/sh if [ "$1" = "--help" ]; then @@ -5719,27 +5723,12 @@ exit 1 "#, ); let fake_bwrap_path: &Path = fake_bwrap.as_ref(); - let warning = system_bwrap_warning_for_path(fake_bwrap_path) - .expect("old system bwrap should emit a warning"); - assert!(warning.contains("too old to support `--argv0`")); -} - -#[cfg(target_os = "linux")] -#[test] -fn system_bwrap_warning_skips_supported_system_bwrap() { - let fake_bwrap = write_fake_bwrap( - r#"#!/bin/sh -if [ "$1" = "--help" ]; then - echo ' --argv0 PROGRAM' - exit 0 -fi -exit 1 -"#, + assert_eq!( + system_bwrap_warning_for_path(fake_bwrap_path), + None, + "Do not warn even if bwrap does not support `--argv0`", ); - let fake_bwrap_path: &Path = fake_bwrap.as_ref(); - - assert_eq!(system_bwrap_warning_for_path(fake_bwrap_path), None); } #[cfg(not(target_os = "linux"))] diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 761e43b93..b2b55feec 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -96,8 +96,6 @@ use std::collections::HashMap; use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; -#[cfg(target_os = "linux")] -use std::process::Command; use crate::config::permissions::compile_permission_profile; use crate::config::permissions::get_readable_roots_required_for_codex_runtime; @@ -172,27 +170,8 @@ fn system_bwrap_warning_for_path(system_bwrap_path: &Path) -> Option { system_bwrap_path.display() )); } - if system_bwrap_supports_argv0(system_bwrap_path) { - return None; - } - Some(format!( - "Codex found system bubblewrap at {}, but it is too old to support `--argv0`. Please upgrade bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime.", - system_bwrap_path.display() - )) -} - -#[cfg(target_os = "linux")] -fn system_bwrap_supports_argv0(system_bwrap_path: &Path) -> bool { - // bubblewrap added `--argv0` in v0.9.0: - // https://github.com/containers/bubblewrap/releases/tag/v0.9.0 - let output = match Command::new(system_bwrap_path).arg("--help").output() { - Ok(output) => output, - Err(_) => return false, - }; - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - stdout.contains("--argv0") || stderr.contains("--argv0") + None } fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option { @@ -476,6 +455,10 @@ pub struct Config { /// output will be hyperlinked using the specified URI scheme. pub file_opener: UriBasedFileOpener, + /// Path to the current Codex executable. This cannot be set in the config + /// file: it must be set in code via [`ConfigOverrides`]. + pub codex_self_exe: Option, + /// Path to the `codex-linux-sandbox` executable. This must be set if /// [`codex_sandboxing::SandboxType::LinuxSeccomp`] is used. Note that this /// cannot be set in the config file: it must be set in code via @@ -760,7 +743,7 @@ impl Config { /// designed to use [AskForApproval::Never] exclusively. /// /// Further, [ConfigOverrides] contains some options that are not supported - /// in [ConfigToml], such as `cwd`, `codex_linux_sandbox_exe`, and + /// in [ConfigToml], such as `cwd`, `codex_self_exe`, `codex_linux_sandbox_exe`, and /// `main_execve_wrapper_exe`. pub async fn load_with_cli_overrides_and_harness_overrides( cli_overrides: Vec<(String, TomlValue)>, @@ -1838,6 +1821,7 @@ pub struct ConfigOverrides { pub model_provider: Option, pub service_tier: Option>, pub config_profile: Option, + pub codex_self_exe: Option, pub codex_linux_sandbox_exe: Option, pub main_execve_wrapper_exe: Option, pub js_repl_node_path: Option, @@ -2036,6 +2020,7 @@ impl Config { model_provider, service_tier: service_tier_override, config_profile: config_profile_key, + codex_self_exe, codex_linux_sandbox_exe, main_execve_wrapper_exe, js_repl_node_path: js_repl_node_path_override, @@ -2642,6 +2627,7 @@ impl Config { history, ephemeral: ephemeral.unwrap_or_default(), file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), + codex_self_exe, codex_linux_sandbox_exe, main_execve_wrapper_exe, js_repl_node_path, diff --git a/codex-rs/core/src/landlock.rs b/codex-rs/core/src/landlock.rs index c1dc4ba54..ef55a207e 100644 --- a/codex-rs/core/src/landlock.rs +++ b/codex-rs/core/src/landlock.rs @@ -5,6 +5,7 @@ use crate::spawn::spawn_child_async; use codex_network_proxy::NetworkProxy; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_sandboxing::landlock::allow_network_for_proxy; use codex_sandboxing::landlock::create_linux_sandbox_command_args_for_policies; use std::collections::HashMap; @@ -48,11 +49,24 @@ where use_legacy_landlock, allow_network_for_proxy(/*enforce_managed_network*/ false), ); - let arg0 = Some("codex-linux-sandbox"); + let codex_linux_sandbox_exe = codex_linux_sandbox_exe.as_ref(); + // Preserve the helper alias when we already have it; otherwise force argv0 + // so arg0 dispatch still reaches the Linux sandbox path. + let arg0 = if codex_linux_sandbox_exe + .file_name() + .and_then(|name| name.to_str()) + == Some(CODEX_LINUX_SANDBOX_ARG0) + { + // Old bubblewrap builds without `--argv0` need a real helper path whose + // basename still dispatches to the Linux sandbox entrypoint. + codex_linux_sandbox_exe.to_string_lossy().into_owned() + } else { + CODEX_LINUX_SANDBOX_ARG0.to_string() + }; spawn_child_async(SpawnChildRequest { - program: codex_linux_sandbox_exe.as_ref().to_path_buf(), + program: codex_linux_sandbox_exe.to_path_buf(), args, - arg0, + arg0: Some(&arg0), cwd: command_cwd, network_sandbox_policy, network, diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 62ebde48b..edb94fb4f 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -205,7 +205,6 @@ impl ToolHandler for ApplyPatchHandler { permissions_preapproved: effective_additional_permissions .permissions_preapproved, timeout_ms: None, - codex_exe: turn.codex_linux_sandbox_exe.clone(), }; let mut orchestrator = ToolOrchestrator::new(); @@ -308,7 +307,6 @@ pub(crate) async fn intercept_apply_patch( permissions_preapproved: effective_additional_permissions .permissions_preapproved, timeout_ms, - codex_exe: turn.codex_linux_sandbox_exe.clone(), }; let mut orchestrator = ToolOrchestrator::new(); diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 7f5686e47..c476d4773 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -42,7 +42,6 @@ pub struct ApplyPatchRequest { pub additional_permissions: Option, pub permissions_preapproved: bool, pub timeout_ms: Option, - pub codex_exe: Option, } #[derive(Default)] @@ -66,25 +65,38 @@ impl ApplyPatchRuntime { } } + #[cfg(target_os = "windows")] fn build_sandbox_command( req: &ApplyPatchRequest, - _codex_home: &std::path::Path, + codex_home: &std::path::Path, ) -> Result { - let exe = if let Some(path) = &req.codex_exe { - path.clone() - } else { - #[cfg(target_os = "windows")] - { - codex_windows_sandbox::resolve_current_exe_for_launch(_codex_home, "codex.exe") - } - #[cfg(not(target_os = "windows"))] - { - std::env::current_exe().map_err(|e| { - ToolError::Rejected(format!("failed to determine codex exe: {e}")) - })? - } - }; - Ok(SandboxCommand { + Ok(Self::build_sandbox_command_with_program( + req, + codex_windows_sandbox::resolve_current_exe_for_launch(codex_home, "codex.exe"), + )) + } + + #[cfg(not(target_os = "windows"))] + fn build_sandbox_command( + req: &ApplyPatchRequest, + codex_self_exe: Option<&PathBuf>, + ) -> Result { + let exe = Self::resolve_apply_patch_program(codex_self_exe)?; + Ok(Self::build_sandbox_command_with_program(req, exe)) + } + + #[cfg(not(target_os = "windows"))] + fn resolve_apply_patch_program(codex_self_exe: Option<&PathBuf>) -> Result { + if let Some(path) = codex_self_exe { + return Ok(path.clone()); + } + + std::env::current_exe() + .map_err(|e| ToolError::Rejected(format!("failed to determine codex exe: {e}"))) + } + + fn build_sandbox_command_with_program(req: &ApplyPatchRequest, exe: PathBuf) -> SandboxCommand { + SandboxCommand { program: exe.to_string_lossy().to_string(), args: vec![ CODEX_CORE_APPLY_PATCH_ARG1.to_string(), @@ -94,7 +106,7 @@ impl ApplyPatchRuntime { // Run apply_patch with a minimal environment for determinism and to avoid leaks. env: HashMap::new(), additional_permissions: req.additional_permissions.clone(), - }) + } } fn stdout_stream(ctx: &ToolCtx) -> Option { @@ -134,13 +146,13 @@ impl Approvable for ApplyPatchRuntime { let approval_keys = self.approval_keys(req); let changes = req.changes.clone(); Box::pin(async move { + if req.permissions_preapproved && retry_reason.is_none() { + return ReviewDecision::Approved; + } if routes_approval_to_guardian(turn) { let action = ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id); return review_approval_request(session, turn, action, retry_reason).await; } - if req.permissions_preapproved && retry_reason.is_none() { - return ReviewDecision::Approved; - } if let Some(reason) = retry_reason { let rx_approve = session .request_patch_approval( @@ -200,7 +212,10 @@ impl ToolRuntime for ApplyPatchRuntime { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, ) -> Result { + #[cfg(target_os = "windows")] let command = Self::build_sandbox_command(req, &ctx.turn.config.codex_home)?; + #[cfg(not(target_os = "windows"))] + let command = Self::build_sandbox_command(req, ctx.turn.codex_self_exe.as_ref())?; let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 79e35a9c2..e38bf5bb0 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -2,6 +2,8 @@ use super::*; use codex_protocol::protocol::GranularApprovalConfig; use pretty_assertions::assert_eq; use std::collections::HashMap; +#[cfg(not(target_os = "windows"))] +use std::path::PathBuf; #[test] fn wants_no_sandbox_approval_granular_respects_sandbox_flag() { @@ -51,7 +53,6 @@ fn guardian_review_request_includes_patch_context() { additional_permissions: None, permissions_preapproved: false, timeout_ms: None, - codex_exe: None, }; let guardian_request = ApplyPatchRuntime::build_guardian_review_request(&request, "call-1"); @@ -67,3 +68,75 @@ fn guardian_review_request_includes_patch_context() { } ); } + +#[cfg(not(target_os = "windows"))] +#[test] +fn build_sandbox_command_prefers_configured_codex_self_exe_for_apply_patch() { + let path = std::env::temp_dir().join("apply-patch-current-exe-test.txt"); + let action = ApplyPatchAction::new_add_for_test(&path, "hello".to_string()); + let request = ApplyPatchRequest { + action, + file_paths: vec![ + AbsolutePathBuf::from_absolute_path(&path).expect("temp path should be absolute"), + ], + changes: HashMap::from([( + path, + FileChange::Add { + content: "hello".to_string(), + }, + )]), + exec_approval_requirement: ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + additional_permissions: None, + permissions_preapproved: false, + timeout_ms: None, + }; + let codex_self_exe = PathBuf::from("/tmp/codex"); + + let command = ApplyPatchRuntime::build_sandbox_command(&request, Some(&codex_self_exe)) + .expect("build sandbox command"); + + assert_eq!( + command.program, + codex_self_exe.to_string_lossy().to_string() + ); +} + +#[cfg(not(target_os = "windows"))] +#[test] +fn build_sandbox_command_falls_back_to_current_exe_for_apply_patch() { + let path = std::env::temp_dir().join("apply-patch-current-exe-test.txt"); + let action = ApplyPatchAction::new_add_for_test(&path, "hello".to_string()); + let request = ApplyPatchRequest { + action, + file_paths: vec![ + AbsolutePathBuf::from_absolute_path(&path).expect("temp path should be absolute"), + ], + changes: HashMap::from([( + path, + FileChange::Add { + content: "hello".to_string(), + }, + )]), + exec_approval_requirement: ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + }, + additional_permissions: None, + permissions_preapproved: false, + timeout_ms: None, + }; + + let command = + ApplyPatchRuntime::build_sandbox_command(&request, None).expect("build sandbox command"); + + assert_eq!( + command.program, + std::env::current_exe() + .expect("current exe") + .to_string_lossy() + .to_string() + ); +} diff --git a/codex-rs/core/tests/common/lib.rs b/codex-rs/core/tests/common/lib.rs index 484bd189e..8c03971ad 100644 --- a/codex-rs/core/tests/common/lib.rs +++ b/codex-rs/core/tests/common/lib.rs @@ -208,10 +208,6 @@ fn default_test_overrides() -> ConfigOverrides { #[cfg(target_os = "linux")] pub fn find_codex_linux_sandbox_exe() -> Result { - if let Ok(path) = std::env::current_exe() { - return Ok(path); - } - if let Some(path) = TEST_ARG0_PATH_ENTRY .get() .and_then(Option::as_ref) @@ -220,6 +216,10 @@ pub fn find_codex_linux_sandbox_exe() -> Result { return Ok(path); } + if let Ok(path) = std::env::current_exe() { + return Ok(path); + } + codex_utils_cargo_bin::cargo_bin("codex-linux-sandbox") } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 5ed5e8a73..efbcf8d8c 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -568,15 +568,21 @@ impl TestCodexBuilder { hook(home.path()); } if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex") { - config.codex_linux_sandbox_exe = Some(path); + config.codex_self_exe = Some(path); + } else if let Ok(path) = codex_utils_cargo_bin::cargo_bin("codex-exec") { + // `codex-exec` also supports `--codex-run-as-apply-patch`, so use it + // when the multitool binary is not available in test builds. + config.codex_self_exe = Some(path); } else if let Ok(exe) = std::env::current_exe() - && let Some(path) = exe - .parent() - .and_then(|parent| parent.parent()) - .map(|parent| parent.join("codex")) - && path.is_file() + && let Some(bin_dir) = exe.parent().and_then(|parent| parent.parent()) { - config.codex_linux_sandbox_exe = Some(path); + let codex = bin_dir.join("codex"); + let codex_exec = bin_dir.join("codex-exec"); + if codex.is_file() { + config.codex_self_exe = Some(codex); + } else if codex_exec.is_file() { + config.codex_self_exe = Some(codex_exec); + } } let mut mutators = vec![]; diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index d0cefcb61..15fd3b7d3 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -19,6 +19,8 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +#[cfg(target_os = "linux")] +use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use core_test_support::assert_regex_match; use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; @@ -87,6 +89,53 @@ fn apply_patch_responses( ] } +#[cfg(target_os = "linux")] +#[large_stack_test] +async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let codex_linux_sandbox_exe = harness + .test() + .config + .codex_linux_sandbox_exe + .as_ref() + .expect("linux test config should include codex-linux-sandbox helper"); + assert_eq!( + codex_linux_sandbox_exe + .file_name() + .and_then(|name| name.to_str()), + Some(CODEX_LINUX_SANDBOX_ARG0), + ); + + let patch = "*** Begin Patch\n*** Add File: helper-alias.txt\n+hello\n*** End Patch"; + let call_id = "apply-helper-alias"; + mount_apply_patch( + &harness, + call_id, + patch, + "done", + ApplyPatchModelOutput::Function, + ) + .await; + + harness.submit("please apply helper alias patch").await?; + + let out = harness + .apply_patch_output(call_id, ApplyPatchModelOutput::Function) + .await; + assert_regex_match( + r"(?s)^Exit code: 0.*Success\. Updated the following files:\nA helper-alias\.txt\n?$", + &out, + ); + assert_eq!( + fs::read_to_string(harness.path("helper-alias.txt"))?, + "hello\n" + ); + + Ok(()) +} + #[large_stack_test] #[test_case(ApplyPatchModelOutput::Freeform)] #[test_case(ApplyPatchModelOutput::Function)] diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index d540b01bb..5bbf233f1 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -1,8 +1,11 @@ // Aggregates all former standalone integration tests as modules. use std::ffi::OsString; +use std::path::Path; +use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; use codex_arg0::Arg0PathEntryGuard; use codex_arg0::arg0_dispatch; +use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use ctor::ctor; use tempfile::TempDir; @@ -19,7 +22,20 @@ const CODEX_HOME_ENV_VAR: &str = "CODEX_HOME"; // based on the arg0. // NOTE: this doesn't work on ARM #[ctor] -pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { +pub static CODEX_ALIASES_TEMP_DIR: Option = { + let mut args = std::env::args_os(); + let argv0 = args.next().unwrap_or_default(); + let exe_name = Path::new(&argv0) + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or(""); + let argv1 = args.next().unwrap_or_default(); + // Helper re-execs inherit this ctor too, but they may run inside a sandbox + // where creating another CODEX_HOME tempdir under /tmp is not allowed. + if exe_name == CODEX_LINUX_SANDBOX_ARG0 || argv1 == CODEX_CORE_APPLY_PATCH_ARG1 { + return None; + } + #[allow(clippy::unwrap_used)] let codex_home = tempfile::Builder::new() .prefix("codex-core-tests") @@ -47,11 +63,11 @@ pub static CODEX_ALIASES_TEMP_DIR: TestCodexAliasesGuard = unsafe { }, } - TestCodexAliasesGuard { + Some(TestCodexAliasesGuard { _codex_home: codex_home, _arg0: arg0, _previous_codex_home: previous_codex_home, - } + }) }; #[cfg(not(target_os = "windows"))] diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index c4f106b46..0cc5ad28c 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -327,6 +327,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cwd: resolved_cwd, model_provider: model_provider.clone(), service_tier: None, + codex_self_exe: arg0_paths.codex_self_exe.clone(), codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(), js_repl_node_path: None, diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index e6dd37927..744a9998f 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -43,7 +43,7 @@ async fn spawn_command_under_sandbox( env: HashMap, ) -> std::io::Result { use codex_core::landlock::spawn_command_under_linux_sandbox; - let codex_linux_sandbox_exe = codex_utils_cargo_bin::cargo_bin("codex-exec") + let codex_linux_sandbox_exe = core_test_support::find_codex_linux_sandbox_exe() .map_err(|err| io::Error::new(io::ErrorKind::NotFound, err))?; spawn_command_under_linux_sandbox( codex_linux_sandbox_exe, diff --git a/codex-rs/linux-sandbox/Cargo.toml b/codex-rs/linux-sandbox/Cargo.toml index 88da2d747..c0bfc7aca 100644 --- a/codex-rs/linux-sandbox/Cargo.toml +++ b/codex-rs/linux-sandbox/Cargo.toml @@ -19,6 +19,7 @@ workspace = true clap = { workspace = true, features = ["derive"] } codex-core = { workspace = true } codex-protocol = { workspace = true } +codex-sandboxing = { workspace = true } codex-utils-absolute-path = { workspace = true } landlock = { workspace = true } libc = { workspace = true } diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index ac34fb4ed..5898a9c68 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -8,23 +8,24 @@ This crate is responsible for producing: - this should also be true of the `codex` multitool CLI On Linux, the bubblewrap pipeline prefers the system `/usr/bin/bwrap` whenever -it is available and supports the required argv-rewrite flags. If `/usr/bin/bwrap` -is missing or too old to support the required flags, the helper falls back to -the vendored bubblewrap path compiled into this binary. -Codex also surfaces a startup warning when `/usr/bin/bwrap` is missing or too -old to support the required flags so users know it is falling back to the -vendored helper. +it is available. If `/usr/bin/bwrap` is present but too old to support +`--argv0`, the helper keeps using system bubblewrap and switches to a +no-`--argv0` compatibility path for the inner re-exec. If `/usr/bin/bwrap` is +missing, the helper falls back to the vendored bubblewrap path compiled into +this binary. +Codex also surfaces a startup warning when `/usr/bin/bwrap` is missing so users +know it is falling back to the vendored helper. **Current Behavior** - Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported. - Bubblewrap is the default filesystem sandbox pipeline. -- If `/usr/bin/bwrap` is present and supports the required argv-rewrite flags, - the helper uses it. -- If `/usr/bin/bwrap` is missing or too old to support the required flags, the - helper falls back to the vendored bubblewrap path. -- If `/usr/bin/bwrap` is missing or too old to support the required flags, - Codex also surfaces a startup warning instead of printing directly from the - sandbox helper. +- If `/usr/bin/bwrap` is present, the helper uses it. +- If `/usr/bin/bwrap` is present but too old to support `--argv0`, the helper + uses a no-`--argv0` compatibility path for the inner re-exec. +- If `/usr/bin/bwrap` is missing, the helper falls back to the vendored + bubblewrap path. +- If `/usr/bin/bwrap` is missing, Codex also surfaces a startup warning instead + of printing directly from the sandbox helper. - Legacy Landlock + mount protections remain available as an explicit legacy fallback path. - Set `features.use_legacy_landlock = true` (or CLI `-c use_legacy_landlock=true`) diff --git a/codex-rs/linux-sandbox/src/launcher.rs b/codex-rs/linux-sandbox/src/launcher.rs index 7d7e04084..e68eea272 100644 --- a/codex-rs/linux-sandbox/src/launcher.rs +++ b/codex-rs/linux-sandbox/src/launcher.rs @@ -14,13 +14,21 @@ const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap"; #[derive(Debug, Clone, PartialEq, Eq)] enum BubblewrapLauncher { - System(AbsolutePathBuf), + System(SystemBwrapLauncher), Vendored, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct SystemBwrapLauncher { + program: AbsolutePathBuf, + supports_argv0: bool, +} + pub(crate) fn exec_bwrap(argv: Vec, preserved_files: Vec) -> ! { match preferred_bwrap_launcher() { - BubblewrapLauncher::System(program) => exec_system_bwrap(&program, argv, preserved_files), + BubblewrapLauncher::System(launcher) => { + exec_system_bwrap(&launcher.program, argv, preserved_files) + } BubblewrapLauncher::Vendored => exec_vendored_bwrap(argv, preserved_files), } } @@ -33,10 +41,18 @@ fn preferred_bwrap_launcher() -> BubblewrapLauncher { } fn preferred_bwrap_launcher_for_path(system_bwrap_path: &Path) -> BubblewrapLauncher { - if !system_bwrap_supports_argv0(system_bwrap_path) { + preferred_bwrap_launcher_for_path_with_probe(system_bwrap_path, system_bwrap_supports_argv0) +} + +fn preferred_bwrap_launcher_for_path_with_probe( + system_bwrap_path: &Path, + system_bwrap_supports_argv0: impl FnOnce(&Path) -> bool, +) -> BubblewrapLauncher { + if !system_bwrap_path.is_file() { return BubblewrapLauncher::Vendored; } + let supports_argv0 = system_bwrap_supports_argv0(system_bwrap_path); let system_bwrap_path = match AbsolutePathBuf::from_absolute_path(system_bwrap_path) { Ok(path) => path, Err(err) => panic!( @@ -44,14 +60,25 @@ fn preferred_bwrap_launcher_for_path(system_bwrap_path: &Path) -> BubblewrapLaun system_bwrap_path.display() ), }; - BubblewrapLauncher::System(system_bwrap_path) + BubblewrapLauncher::System(SystemBwrapLauncher { + program: system_bwrap_path, + supports_argv0, + }) +} + +pub(crate) fn preferred_bwrap_supports_argv0() -> bool { + match preferred_bwrap_launcher() { + BubblewrapLauncher::System(launcher) => launcher.supports_argv0, + BubblewrapLauncher::Vendored => true, + } } fn system_bwrap_supports_argv0(system_bwrap_path: &Path) -> bool { // bubblewrap added `--argv0` in v0.9.0: // https://github.com/containers/bubblewrap/releases/tag/v0.9.0 // Older distro packages (for example Ubuntu 20.04/22.04) ship builds that - // reject `--argv0`, so prefer the vendored build in that case. + // reject `--argv0`, so use the system binary's no-argv0 compatibility path + // in that case. let output = match Command::new(system_bwrap_path).arg("--help").output() { Ok(output) => output, Err(_) => return false, @@ -126,47 +153,34 @@ fn clear_cloexec(fd: libc::c_int) { mod tests { use super::*; use pretty_assertions::assert_eq; - use std::fs; - use std::os::unix::fs::PermissionsExt; use tempfile::NamedTempFile; - use tempfile::TempPath; #[test] fn prefers_system_bwrap_when_help_lists_argv0() { - let fake_bwrap = write_fake_bwrap( - r#"#!/bin/sh -if [ "$1" = "--help" ]; then - echo ' --argv0 PROGRAM' - exit 0 -fi -exit 1 -"#, - ); - let fake_bwrap_path: &Path = fake_bwrap.as_ref(); + let fake_bwrap = NamedTempFile::new().expect("temp file"); + let fake_bwrap_path = fake_bwrap.path(); let expected = AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"); assert_eq!( - preferred_bwrap_launcher_for_path(fake_bwrap_path), - BubblewrapLauncher::System(expected) + preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| true), + BubblewrapLauncher::System(SystemBwrapLauncher { + program: expected, + supports_argv0: true, + }) ); } #[test] - fn falls_back_to_vendored_when_system_bwrap_lacks_argv0() { - let fake_bwrap = write_fake_bwrap( - r#"#!/bin/sh -if [ "$1" = "--help" ]; then - echo 'usage: bwrap [OPTION...] COMMAND' - exit 0 -fi -exit 1 -"#, - ); - let fake_bwrap_path: &Path = fake_bwrap.as_ref(); + fn prefers_system_bwrap_when_system_bwrap_lacks_argv0() { + let fake_bwrap = NamedTempFile::new().expect("temp file"); + let fake_bwrap_path = fake_bwrap.path(); assert_eq!( - preferred_bwrap_launcher_for_path(fake_bwrap_path), - BubblewrapLauncher::Vendored + preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| false), + BubblewrapLauncher::System(SystemBwrapLauncher { + program: AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"), + supports_argv0: false, + }) ); } @@ -207,13 +221,4 @@ exit 1 } flags } - - fn write_fake_bwrap(contents: &str) -> TempPath { - // Linux rejects exec-ing a file that is still open for writing. - let path = NamedTempFile::new().expect("temp file").into_temp_path(); - fs::write(&path, contents).expect("write fake bwrap"); - let permissions = fs::Permissions::from_mode(0o755); - fs::set_permissions(&path, permissions).expect("chmod fake bwrap"); - path - } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index b753460dc..8f39837de 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -12,11 +12,13 @@ use crate::bwrap::BwrapOptions; use crate::bwrap::create_bwrap_command_args; use crate::landlock::apply_sandbox_policy_to_current_thread; 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 codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; +use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; #[derive(Debug, Parser)] /// CLI surface for the Linux sandbox helper. @@ -426,13 +428,14 @@ fn run_bwrap_with_proc_fallback( mount_proc, network_mode, }; - let bwrap_args = build_bwrap_argv( + let mut bwrap_args = build_bwrap_argv( inner, file_system_sandbox_policy, sandbox_policy_cwd, command_cwd, options, ); + apply_inner_command_argv0(&mut bwrap_args.args); exec_bwrap(bwrap_args.args, bwrap_args.preserved_files); } @@ -456,7 +459,7 @@ fn build_bwrap_argv( command_cwd: &Path, options: BwrapOptions, ) -> crate::bwrap::BwrapArgs { - let mut bwrap_args = create_bwrap_command_args( + let bwrap_args = create_bwrap_command_args( inner, file_system_sandbox_policy, sandbox_policy_cwd, @@ -465,16 +468,6 @@ fn build_bwrap_argv( ) .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); - let command_separator_index = bwrap_args - .args - .iter() - .position(|arg| arg == "--") - .unwrap_or_else(|| panic!("bubblewrap argv is missing command separator '--'")); - bwrap_args.args.splice( - command_separator_index..command_separator_index, - ["--argv0".to_string(), "codex-linux-sandbox".to_string()], - ); - let mut argv = vec!["bwrap".to_string()]; argv.extend(bwrap_args.args); crate::bwrap::BwrapArgs { @@ -483,6 +476,46 @@ fn build_bwrap_argv( } } +fn apply_inner_command_argv0(argv: &mut Vec) { + apply_inner_command_argv0_for_launcher( + argv, + preferred_bwrap_supports_argv0(), + current_process_argv0(), + ); +} + +fn apply_inner_command_argv0_for_launcher( + argv: &mut Vec, + supports_argv0: bool, + argv0_fallback_command: String, +) { + let command_separator_index = argv + .iter() + .position(|arg| arg == "--") + .unwrap_or_else(|| panic!("bubblewrap argv is missing command separator '--'")); + + if supports_argv0 { + argv.splice( + command_separator_index..command_separator_index, + ["--argv0".to_string(), CODEX_LINUX_SANDBOX_ARG0.to_string()], + ); + return; + } + + let command_index = command_separator_index + 1; + let Some(command) = argv.get_mut(command_index) else { + panic!("bubblewrap argv is missing inner command after '--'"); + }; + *command = argv0_fallback_command; +} + +fn current_process_argv0() -> String { + match std::env::args_os().next() { + Some(argv0) => argv0.to_string_lossy().into_owned(), + None => panic!("failed to resolve current process argv[0]"), + } +} + fn preflight_proc_mount_support( sandbox_policy_cwd: &Path, command_cwd: &Path, 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 b42d7e552..d8b84ffba 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -40,7 +40,7 @@ fn ignores_non_proc_mount_errors() { #[test] fn inserts_bwrap_argv0_before_command_separator() { let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let argv = build_bwrap_argv( + let mut argv = build_bwrap_argv( vec!["/bin/true".to_string()], &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), @@ -51,6 +51,11 @@ fn inserts_bwrap_argv0_before_command_separator() { }, ) .args; + apply_inner_command_argv0_for_launcher( + &mut argv, + true, + "/tmp/codex-arg0-session/codex-linux-sandbox".to_string(), + ); assert_eq!( argv, vec![ @@ -74,6 +79,73 @@ fn inserts_bwrap_argv0_before_command_separator() { ); } +#[test] +fn rewrites_inner_command_path_when_bwrap_lacks_argv0() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); + let mut argv = build_bwrap_argv( + vec!["/bin/true".to_string()], + &FileSystemSandboxPolicy::from(&sandbox_policy), + Path::new("/"), + Path::new("/"), + BwrapOptions { + mount_proc: true, + network_mode: BwrapNetworkMode::FullAccess, + }, + ) + .args; + apply_inner_command_argv0_for_launcher( + &mut argv, + false, + "/tmp/codex-arg0-session/codex-linux-sandbox".to_string(), + ); + + assert!(!argv.iter().any(|arg| arg == "--argv0")); + assert!( + argv.windows(2) + .any(|window| { window == ["--", "/tmp/codex-arg0-session/codex-linux-sandbox"] }) + ); +} + +#[test] +fn rewrites_bwrap_helper_command_not_nested_user_command_when_current_exe_appears_later() { + let nested_current_exe = std::env::current_exe() + .expect("current exe") + .to_string_lossy() + .into_owned(); + let mut argv = vec![ + "bwrap".to_string(), + "--".to_string(), + "/tmp/helper-symlink".to_string(), + "--sandbox-policy-cwd".to_string(), + "/tmp/cwd".to_string(), + "--".to_string(), + nested_current_exe.clone(), + "--codex-run-as-apply-patch".to_string(), + "patch".to_string(), + ]; + + apply_inner_command_argv0_for_launcher( + &mut argv, + false, + "/tmp/argv0-fallback-helper".to_string(), + ); + + assert_eq!( + argv, + vec![ + "bwrap".to_string(), + "--".to_string(), + "/tmp/argv0-fallback-helper".to_string(), + "--sandbox-policy-cwd".to_string(), + "/tmp/cwd".to_string(), + "--".to_string(), + nested_current_exe, + "--codex-run-as-apply-patch".to_string(), + "patch".to_string(), + ] + ); +} + #[test] fn inserts_unshare_net_when_network_isolation_requested() { let sandbox_policy = SandboxPolicy::new_read_only_policy(); @@ -313,6 +385,7 @@ fn resolve_sandbox_policies_rejects_mismatched_legacy_and_split_inputs() { Some(NetworkSandboxPolicy::Enabled), ) .expect_err("mismatched legacy and split policies should fail"); + assert!( matches!( err, diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index f3f66b555..f83fd4fd5 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -176,6 +176,7 @@ impl CodexToolCallParam { cwd: cwd.map(PathBuf::from), approval_policy: approval_policy.map(Into::into), sandbox_mode: sandbox.map(Into::into), + codex_self_exe: arg0_paths.codex_self_exe.clone(), codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(), base_instructions, diff --git a/codex-rs/sandboxing/src/landlock.rs b/codex-rs/sandboxing/src/landlock.rs index 8636f9ed0..d948d23d1 100644 --- a/codex-rs/sandboxing/src/landlock.rs +++ b/codex-rs/sandboxing/src/landlock.rs @@ -2,6 +2,11 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use std::path::Path; + +/// Basename used when the Codex executable self-invokes as the Linux sandbox +/// helper. +pub const CODEX_LINUX_SANDBOX_ARG0: &str = "codex-linux-sandbox"; + pub fn allow_network_for_proxy(enforce_managed_network: bool) -> bool { // When managed network requirements are active, request proxy-only // networking from the Linux sandbox helper. Without managed requirements, diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index f55f53cb1..3d286eb64 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -1,3 +1,4 @@ +use crate::landlock::CODEX_LINUX_SANDBOX_ARG0; use crate::landlock::allow_network_for_proxy; use crate::landlock::create_linux_sandbox_command_args_for_policies; use crate::policy_transforms::EffectiveSandboxPermissions; @@ -247,7 +248,10 @@ impl SandboxManager { let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(exe.to_string_lossy().to_string()); full_command.append(&mut args); - (full_command, Some("codex-linux-sandbox".to_string())) + ( + full_command, + Some(linux_sandbox_arg0_override(exe.as_path())), + ) } #[cfg(target_os = "windows")] SandboxType::WindowsRestrictedToken => (argv, None), @@ -271,6 +275,14 @@ impl SandboxManager { } } +fn linux_sandbox_arg0_override(exe: &Path) -> String { + if exe.file_name().and_then(|name| name.to_str()) == Some(CODEX_LINUX_SANDBOX_ARG0) { + exe.to_string_lossy().into_owned() + } else { + CODEX_LINUX_SANDBOX_ARG0.to_string() + } +} + #[cfg(test)] #[path = "manager_tests.rs"] mod tests; diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 17921b191..690aec89d 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -249,3 +249,54 @@ fn transform_additional_permissions_preserves_denied_entries() { NetworkSandboxPolicy::Restricted ); } + +#[cfg(target_os = "linux")] +fn transform_linux_seccomp_request( + codex_linux_sandbox_exe: &std::path::PathBuf, +) -> super::SandboxExecRequest { + let manager = SandboxManager::new(); + let cwd = std::env::current_dir().expect("current dir"); + manager + .transform(SandboxTransformRequest { + command: SandboxCommand { + program: "true".to_string(), + args: Vec::new(), + cwd: cwd.clone(), + env: HashMap::new(), + additional_permissions: None, + }, + policy: &SandboxPolicy::DangerFullAccess, + file_system_policy: &FileSystemSandboxPolicy::unrestricted(), + network_policy: NetworkSandboxPolicy::Enabled, + sandbox: SandboxType::LinuxSeccomp, + enforce_managed_network: false, + network: None, + sandbox_policy_cwd: cwd.as_path(), + codex_linux_sandbox_exe: Some(codex_linux_sandbox_exe), + use_legacy_landlock: false, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + windows_sandbox_private_desktop: false, + }) + .expect("transform") +} + +#[cfg(target_os = "linux")] +#[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); + + assert_eq!( + exec_request.arg0, + Some(codex_linux_sandbox_exe.to_string_lossy().into_owned()) + ); +} + +#[cfg(target_os = "linux")] +#[test] +fn transform_linux_seccomp_uses_helper_alias_when_launcher_is_not_helper_path() { + let codex_linux_sandbox_exe = std::path::PathBuf::from("/tmp/codex"); + let exec_request = transform_linux_seccomp_request(&codex_linux_sandbox_exe); + + assert_eq!(exec_request.arg0, Some("codex-linux-sandbox".to_string())); +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 43182b524..e291480d0 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -418,6 +418,7 @@ pub async fn run_main( cwd, model_provider: model_provider_override.clone(), config_profile: cli.config_profile.clone(), + codex_self_exe: arg0_paths.codex_self_exe.clone(), codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(), show_raw_agent_reasoning: cli.oss.then_some(true), diff --git a/codex-rs/tui_app_server/src/lib.rs b/codex-rs/tui_app_server/src/lib.rs index c5e2479bf..1e4086685 100644 --- a/codex-rs/tui_app_server/src/lib.rs +++ b/codex-rs/tui_app_server/src/lib.rs @@ -776,6 +776,7 @@ pub async fn run_main( cwd, model_provider: model_provider_override.clone(), config_profile: cli.config_profile.clone(), + codex_self_exe: arg0_paths.codex_self_exe.clone(), codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(), show_raw_agent_reasoning: cli.oss.then_some(true),