diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 8b4afc23d..a4994d34b 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -479,7 +479,7 @@ pub async fn run_main_with_transport( range: None, }); } - if let Some(warning) = codex_core::config::missing_system_bwrap_warning() { + if let Some(warning) = codex_core::config::system_bwrap_warning() { config_warnings.push(ConfigWarningNotification { summary: warning, details: None, diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 3558fd9f6..c27ec5460 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -61,9 +61,11 @@ 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 -falls back to the vendored bubblewrap path otherwise. When `/usr/bin/bwrap` is -missing, Codex also surfaces a startup warning through its normal notification -path instead of printing directly from the sandbox helper. +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 +warning through its normal notification path instead of printing directly from +the sandbox helper. ### Windows diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 65c05468e..7786b3087 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -5582,16 +5582,69 @@ shell_tool = true Ok(()) } +#[cfg(target_os = "linux")] #[test] -fn missing_system_bwrap_warning_matches_system_bwrap_presence() { - #[cfg(target_os = "linux")] - assert_eq!( - missing_system_bwrap_warning().is_some(), - !Path::new("/usr/bin/bwrap").is_file() - ); +fn system_bwrap_warning_reports_missing_system_bwrap() { + let warning = system_bwrap_warning_for_path(Path::new("/definitely/not/a/bwrap")) + .expect("missing system bwrap should emit a warning"); - #[cfg(not(target_os = "linux"))] - assert!(missing_system_bwrap_warning().is_none()); + assert!(warning.contains("could not find system bubblewrap")); +} + +#[cfg(target_os = "linux")] +#[test] +fn system_bwrap_warning_reports_too_old_system_bwrap() { + 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(); + 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 +"#, + ); + 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"))] +#[test] +fn system_bwrap_warning_is_disabled_off_linux() { + assert!(system_bwrap_warning().is_none()); +} + +#[cfg(target_os = "linux")] +fn write_fake_bwrap(contents: &str) -> tempfile::TempPath { + use std::fs; + use std::os::unix::fs::PermissionsExt; + use tempfile::NamedTempFile; + + // 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 } #[tokio::test] diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6df989db1..cbbd02390 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -96,6 +96,8 @@ 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; @@ -153,21 +155,46 @@ const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [ ]; #[cfg(target_os = "linux")] -pub fn missing_system_bwrap_warning() -> Option { - if Path::new(SYSTEM_BWRAP_PATH).is_file() { - None - } else { - Some(format!( - "Codex could not find system bubblewrap at {SYSTEM_BWRAP_PATH}. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime." - )) - } +pub fn system_bwrap_warning() -> Option { + system_bwrap_warning_for_path(Path::new(SYSTEM_BWRAP_PATH)) } #[cfg(not(target_os = "linux"))] -pub fn missing_system_bwrap_warning() -> Option { +pub fn system_bwrap_warning() -> Option { None } +#[cfg(target_os = "linux")] +fn system_bwrap_warning_for_path(system_bwrap_path: &Path) -> Option { + if !system_bwrap_path.is_file() { + return Some(format!( + "Codex could not find system bubblewrap at {}. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime.", + 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") +} + fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option { let raw = std::env::var(codex_state::SQLITE_HOME_ENV).ok()?; let trimmed = raw.trim(); diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index f648a6395..c10b86f69 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -669,7 +669,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { // Print the effective configuration and initial request so users can see what Codex // is using. event_processor.print_config_summary(&config, &prompt_summary, &session_configured); - if !json_mode && let Some(message) = codex_core::config::missing_system_bwrap_warning() { + if !json_mode && let Some(message) = codex_core::config::system_bwrap_warning() { let _ = event_processor.process_event(Event { id: String::new(), msg: EventMsg::Warning(codex_protocol::protocol::WarningEvent { message }), diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 3fde7d973..ac34fb4ed 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -8,19 +8,23 @@ 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. If `/usr/bin/bwrap` is missing, the helper still falls back to +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 so users -know it is falling back to the vendored helper. +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. **Current Behavior** - Legacy `SandboxPolicy` / `sandbox_mode` configs remain supported. - Bubblewrap is the default filesystem sandbox pipeline. -- If `/usr/bin/bwrap` is present, the helper uses it. -- 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. +- 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. - 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 37a860e08..7d7e04084 100644 --- a/codex-rs/linux-sandbox/src/launcher.rs +++ b/codex-rs/linux-sandbox/src/launcher.rs @@ -4,6 +4,8 @@ use std::os::fd::AsRawFd; use std::os::raw::c_char; use std::os::unix::ffi::OsStrExt; use std::path::Path; +use std::process::Command; +use std::sync::OnceLock; use crate::vendored_bwrap::exec_vendored_bwrap; use codex_utils_absolute_path::AbsolutePathBuf; @@ -24,17 +26,41 @@ pub(crate) fn exec_bwrap(argv: Vec, preserved_files: Vec) -> ! { } fn preferred_bwrap_launcher() -> BubblewrapLauncher { - if !Path::new(SYSTEM_BWRAP_PATH).is_file() { + static LAUNCHER: OnceLock = OnceLock::new(); + LAUNCHER + .get_or_init(|| preferred_bwrap_launcher_for_path(Path::new(SYSTEM_BWRAP_PATH))) + .clone() +} + +fn preferred_bwrap_launcher_for_path(system_bwrap_path: &Path) -> BubblewrapLauncher { + if !system_bwrap_supports_argv0(system_bwrap_path) { return BubblewrapLauncher::Vendored; } - let system_bwrap_path = match AbsolutePathBuf::from_absolute_path(SYSTEM_BWRAP_PATH) { + let system_bwrap_path = match AbsolutePathBuf::from_absolute_path(system_bwrap_path) { Ok(path) => path, - Err(err) => panic!("failed to normalize system bubblewrap path {SYSTEM_BWRAP_PATH}: {err}"), + Err(err) => panic!( + "failed to normalize system bubblewrap path {}: {err}", + system_bwrap_path.display() + ), }; BubblewrapLauncher::System(system_bwrap_path) } +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. + 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") +} + fn exec_system_bwrap( program: &AbsolutePathBuf, argv: Vec, @@ -100,7 +126,57 @@ 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 expected = AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"); + + assert_eq!( + preferred_bwrap_launcher_for_path(fake_bwrap_path), + BubblewrapLauncher::System(expected) + ); + } + + #[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(); + + assert_eq!( + preferred_bwrap_launcher_for_path(fake_bwrap_path), + BubblewrapLauncher::Vendored + ); + } + + #[test] + fn falls_back_to_vendored_when_system_bwrap_is_missing() { + assert_eq!( + preferred_bwrap_launcher_for_path(Path::new("/definitely/not/a/bwrap")), + BubblewrapLauncher::Vendored + ); + } #[test] fn preserved_files_are_made_inheritable_for_system_exec() { @@ -131,4 +207,13 @@ mod tests { } 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/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4aa51df21..358b1fe5b 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -398,8 +398,8 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) ))); } -fn emit_missing_system_bwrap_warning(app_event_tx: &AppEventSender) { - let Some(message) = codex_core::config::missing_system_bwrap_warning() else { +fn emit_system_bwrap_warning(app_event_tx: &AppEventSender) { + let Some(message) = codex_core::config::system_bwrap_warning() else { return; }; @@ -2197,7 +2197,7 @@ impl App { let (app_event_tx, mut app_event_rx) = unbounded_channel(); let app_event_tx = AppEventSender::new(app_event_tx); emit_project_config_warnings(&app_event_tx, &config); - emit_missing_system_bwrap_warning(&app_event_tx); + emit_system_bwrap_warning(&app_event_tx); emit_custom_prompt_deprecation_notice(&app_event_tx, &config.codex_home).await; tui.set_notification_method(config.tui_notification_method); diff --git a/codex-rs/tui_app_server/src/app.rs b/codex-rs/tui_app_server/src/app.rs index d4569ae7a..79c15c29a 100644 --- a/codex-rs/tui_app_server/src/app.rs +++ b/codex-rs/tui_app_server/src/app.rs @@ -435,8 +435,8 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) ))); } -fn emit_missing_system_bwrap_warning(app_event_tx: &AppEventSender) { - let Some(message) = codex_core::config::missing_system_bwrap_warning() else { +fn emit_system_bwrap_warning(app_event_tx: &AppEventSender) { + let Some(message) = codex_core::config::system_bwrap_warning() else { return; }; @@ -2794,7 +2794,7 @@ impl App { let (app_event_tx, mut app_event_rx) = unbounded_channel(); let app_event_tx = AppEventSender::new(app_event_tx); emit_project_config_warnings(&app_event_tx, &config); - emit_missing_system_bwrap_warning(&app_event_tx); + emit_system_bwrap_warning(&app_event_tx); tui.set_notification_method(config.tui_notification_method); let harness_overrides =