From e1b1e33021cc9c2ac427bdb2463897759c46bec1 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 20 Mar 2026 10:57:59 -0700 Subject: [PATCH] fix: fall back to vendored bubblewrap when system bwrap lacks --argv0 --- codex-rs/core/README.md | 7 +-- codex-rs/linux-sandbox/README.md | 10 ++-- codex-rs/linux-sandbox/src/launcher.rs | 73 ++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/README.md b/codex-rs/core/README.md index 3558fd9f60..aa4c7db7e8 100644 --- a/codex-rs/core/README.md +++ b/codex-rs/core/README.md @@ -61,9 +61,10 @@ 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 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. ### Windows diff --git a/codex-rs/linux-sandbox/README.md b/codex-rs/linux-sandbox/README.md index 3fde7d9738..8994aa5201 100644 --- a/codex-rs/linux-sandbox/README.md +++ b/codex-rs/linux-sandbox/README.md @@ -8,7 +8,8 @@ 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. @@ -16,9 +17,10 @@ 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 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, Codex also surfaces a startup warning instead of printing directly from the sandbox helper. - Legacy Landlock + mount protections remain available as an explicit legacy diff --git a/codex-rs/linux-sandbox/src/launcher.rs b/codex-rs/linux-sandbox/src/launcher.rs index 37a860e085..77da399fb8 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,39 @@ 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_path.is_file() || !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 { + // Older distro packages (for example Ubuntu 20.04/22.04) ship bubblewrap + // 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,8 +124,43 @@ 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; + #[test] + fn prefers_system_bwrap_when_help_lists_argv0() { + let fake_bwrap = write_fake_bwrap( + "#!/bin/sh\nif [ \"$1\" = \"--help\" ]; then\n echo ' --argv0 PROGRAM'\n exit 0\nfi\nexit 1\n", + ); + 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( + "#!/bin/sh\nif [ \"$1\" = \"--help\" ]; then\n echo 'usage: bwrap [OPTION...] COMMAND'\n exit 0\nfi\nexit 1\n", + ); + + 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() { let file = NamedTempFile::new().expect("temp file"); @@ -131,4 +190,12 @@ mod tests { } flags } + + fn write_fake_bwrap(contents: &str) -> NamedTempFile { + let file = NamedTempFile::new().expect("temp file"); + fs::write(file.path(), contents).expect("write fake bwrap"); + let permissions = fs::Permissions::from_mode(0o755); + fs::set_permissions(file.path(), permissions).expect("chmod fake bwrap"); + file + } }