From 5b80f87c972a90ea7d312d4885ed3554e92c00d3 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 4 May 2026 10:38:31 -0700 Subject: [PATCH] fix(linux-sandbox): fall back when system bwrap lacks perms (#20628) ## Why Codex `0.128` started using `--perms` in more routine Linux sandbox construction when protected workspace metadata mounts landed in #19852. Upstream bubblewrap added `--perms` in `v0.5.0`, so system `bwrap` versions older than that, including the `v0.4.0` and `v0.4.1` family, do not support the flag. The launcher still selected those binaries as long as they existed on `PATH`. That means affected hosts can fail every sandboxed command up front with: ```text bwrap: Unknown option --perms ``` The reports in #20590 and duplicate #20623 match that compatibility gap; #20623 explicitly shows system bubblewrap `0.4.0`. ## What changed - Replace the single `--argv0` probe with a small system-bwrap capability probe in `codex-rs/linux-sandbox/src/launcher.rs`. - Continue using the old-system `--argv0` compatibility path when needed, but only select a system `bwrap` if it also advertises `--perms`. - Fall back to the vendored `bwrap` when the system binary is too old for the flags Codex now requires. - Add regression coverage for the old-system-bwrap case so binaries without `--perms` stay on the vendored path. ## Verification - Added `falls_back_to_vendored_when_system_bwrap_lacks_perms` to cover the reported compatibility gap. - Ran `cargo test -p codex-linux-sandbox` and `cargo clippy -p codex-linux-sandbox --tests` locally. On macOS, the crate builds but its Linux-only tests are cfg-gated out, so the new regression test still needs Linux CI or a Linux devbox run for real execution coverage. ## Related issues - Fixes #20590 - Duplicate report: #20623 --- codex-rs/exec-server/tests/file_system.rs | 2 +- codex-rs/linux-sandbox/src/launcher.rs | 56 +++++++++++++++++++---- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index c42159a6d..0840b2a90 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -198,7 +198,7 @@ set -euo pipefail for arg in "$@"; do if [[ "${arg}" == "--help" ]]; then - echo "Usage: bwrap --argv0" + echo "Usage: bwrap --argv0 --perms" exit 0 fi done diff --git a/codex-rs/linux-sandbox/src/launcher.rs b/codex-rs/linux-sandbox/src/launcher.rs index cfaa70f93..577ef5240 100644 --- a/codex-rs/linux-sandbox/src/launcher.rs +++ b/codex-rs/linux-sandbox/src/launcher.rs @@ -23,6 +23,12 @@ struct SystemBwrapLauncher { supports_argv0: bool, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct SystemBwrapCapabilities { + supports_argv0: bool, + supports_perms: bool, +} + pub(crate) fn exec_bwrap(argv: Vec, preserved_files: Vec) -> ! { match preferred_bwrap_launcher() { BubblewrapLauncher::System(launcher) => { @@ -43,18 +49,24 @@ fn preferred_bwrap_launcher() -> BubblewrapLauncher { } fn preferred_bwrap_launcher_for_path(system_bwrap_path: &Path) -> BubblewrapLauncher { - preferred_bwrap_launcher_for_path_with_probe(system_bwrap_path, system_bwrap_supports_argv0) + preferred_bwrap_launcher_for_path_with_probe(system_bwrap_path, system_bwrap_capabilities) } fn preferred_bwrap_launcher_for_path_with_probe( system_bwrap_path: &Path, - system_bwrap_supports_argv0: impl FnOnce(&Path) -> bool, + system_bwrap_capabilities: impl FnOnce(&Path) -> Option, ) -> BubblewrapLauncher { if !system_bwrap_path.is_file() { return BubblewrapLauncher::Vendored; } - let supports_argv0 = system_bwrap_supports_argv0(system_bwrap_path); + let Some(SystemBwrapCapabilities { + supports_argv0, + supports_perms: true, + }) = system_bwrap_capabilities(system_bwrap_path) + else { + return BubblewrapLauncher::Vendored; + }; let system_bwrap_path = match AbsolutePathBuf::from_absolute_path(system_bwrap_path) { Ok(path) => path, Err(err) => panic!( @@ -75,7 +87,7 @@ pub(crate) fn preferred_bwrap_supports_argv0() -> bool { } } -fn system_bwrap_supports_argv0(system_bwrap_path: &Path) -> bool { +fn system_bwrap_capabilities(system_bwrap_path: &Path) -> Option { // 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 @@ -83,11 +95,14 @@ fn system_bwrap_supports_argv0(system_bwrap_path: &Path) -> bool { // in that case. let output = match Command::new(system_bwrap_path).arg("--help").output() { Ok(output) => output, - Err(_) => return false, + Err(_) => return None, }; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); - stdout.contains("--argv0") || stderr.contains("--argv0") + Some(SystemBwrapCapabilities { + supports_argv0: stdout.contains("--argv0") || stderr.contains("--argv0"), + supports_perms: stdout.contains("--perms") || stderr.contains("--perms"), + }) } fn exec_system_bwrap( @@ -164,7 +179,12 @@ mod tests { let expected = AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"); assert_eq!( - preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| true), + preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| { + Some(SystemBwrapCapabilities { + supports_argv0: true, + supports_perms: true, + }) + }), BubblewrapLauncher::System(SystemBwrapLauncher { program: expected, supports_argv0: true, @@ -178,7 +198,12 @@ mod tests { let fake_bwrap_path = fake_bwrap.path(); assert_eq!( - preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| false), + preferred_bwrap_launcher_for_path_with_probe(fake_bwrap_path, |_| { + Some(SystemBwrapCapabilities { + supports_argv0: false, + supports_perms: true, + }) + }), BubblewrapLauncher::System(SystemBwrapLauncher { program: AbsolutePathBuf::from_absolute_path(fake_bwrap_path).expect("absolute"), supports_argv0: false, @@ -186,6 +211,21 @@ mod tests { ); } + #[test] + fn falls_back_to_vendored_when_system_bwrap_lacks_perms() { + let fake_bwrap = NamedTempFile::new().expect("temp file"); + + assert_eq!( + preferred_bwrap_launcher_for_path_with_probe(fake_bwrap.path(), |_| { + Some(SystemBwrapCapabilities { + supports_argv0: false, + supports_perms: false, + }) + }), + BubblewrapLauncher::Vendored + ); + } + #[test] fn falls_back_to_vendored_when_system_bwrap_is_missing() { assert_eq!(