Compare commits

...

1 Commits

Author SHA1 Message Date
viyatb-oai
59aa4bfb72 fix: resolve bwrap from trusted PATH entry 2026-03-26 10:47:30 -07:00
5 changed files with 121 additions and 38 deletions

View File

@@ -59,13 +59,13 @@ 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. 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.
The Linux sandbox helper prefers the first `bwrap` found on `PATH` outside the
current working directory whenever it is available. If `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
`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.
### Windows

View File

@@ -5704,8 +5704,8 @@ shell_tool = true
#[cfg(target_os = "linux")]
#[test]
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");
let warning =
system_bwrap_warning_for_lookup(None).expect("missing system bwrap should emit a warning");
assert!(warning.contains("could not find system bubblewrap"));
}
@@ -5725,12 +5725,48 @@ exit 1
let fake_bwrap_path: &Path = fake_bwrap.as_ref();
assert_eq!(
system_bwrap_warning_for_path(fake_bwrap_path),
system_bwrap_warning_for_lookup(Some(fake_bwrap_path.to_path_buf())),
None,
"Do not warn even if bwrap does not support `--argv0`",
);
}
#[cfg(target_os = "linux")]
#[test]
fn finds_first_executable_bwrap_in_search_paths() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let first_dir = temp_dir.path().join("first");
let second_dir = temp_dir.path().join("second");
std::fs::create_dir_all(&cwd).expect("create cwd");
std::fs::create_dir_all(&first_dir).expect("create first dir");
std::fs::create_dir_all(&second_dir).expect("create second dir");
std::fs::write(first_dir.join("bwrap"), "not executable").expect("write non-executable bwrap");
let expected_bwrap = write_named_fake_bwrap_in(&second_dir);
assert_eq!(
find_system_bwrap_in_search_paths(vec![first_dir, second_dir], &cwd),
Some(expected_bwrap)
);
}
#[cfg(target_os = "linux")]
#[test]
fn skips_workspace_local_bwrap_in_search_paths() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let trusted_dir = temp_dir.path().join("trusted");
std::fs::create_dir_all(&cwd).expect("create cwd");
std::fs::create_dir_all(&trusted_dir).expect("create trusted dir");
let _workspace_bwrap = write_named_fake_bwrap_in(&cwd);
let expected_bwrap = write_named_fake_bwrap_in(&trusted_dir);
assert_eq!(
find_system_bwrap_in_search_paths(vec![cwd.clone(), trusted_dir], &cwd),
Some(expected_bwrap)
);
}
#[cfg(not(target_os = "linux"))]
#[test]
fn system_bwrap_warning_is_disabled_off_linux() {
@@ -5739,6 +5775,14 @@ fn system_bwrap_warning_is_disabled_off_linux() {
#[cfg(target_os = "linux")]
fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
write_fake_bwrap_in(
&std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
contents,
)
}
#[cfg(target_os = "linux")]
fn write_fake_bwrap_in(dir: &Path, contents: &str) -> tempfile::TempPath {
use std::fs;
use std::os::unix::fs::PermissionsExt;
use tempfile::NamedTempFile;
@@ -5746,9 +5790,8 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
// Bazel can mount the OS temp directory `noexec`, so prefer the current
// working directory for fake executables and fall back to the default temp
// dir outside that environment.
let temp_file = std::env::current_dir()
let temp_file = NamedTempFile::new_in(dir)
.ok()
.and_then(|dir| NamedTempFile::new_in(dir).ok())
.unwrap_or_else(|| NamedTempFile::new().expect("temp file"));
// Linux rejects exec-ing a file that is still open for writing.
let path = temp_file.into_temp_path();
@@ -5758,6 +5801,18 @@ fn write_fake_bwrap(contents: &str) -> tempfile::TempPath {
path
}
#[cfg(target_os = "linux")]
fn write_named_fake_bwrap_in(dir: &Path) -> PathBuf {
use std::fs;
use std::os::unix::fs::PermissionsExt;
let path = dir.join("bwrap");
fs::write(&path, "#!/bin/sh\n").expect("write fake bwrap");
let permissions = fs::Permissions::from_mode(0o755);
fs::set_permissions(&path, permissions).expect("chmod fake bwrap");
path
}
#[tokio::test]
async fn approvals_reviewer_defaults_to_manual_only_without_guardian_feature() -> std::io::Result<()>
{

View File

@@ -145,7 +145,7 @@ pub(crate) const DEFAULT_AGENT_JOB_MAX_RUNTIME_SECONDS: Option<u64> = None;
pub const CONFIG_TOML_FILE: &str = "config.toml";
const OPENAI_BASE_URL_ENV_VAR: &str = "OPENAI_BASE_URL";
#[cfg(target_os = "linux")]
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
const SYSTEM_BWRAP_PROGRAM: &str = "bwrap";
const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
OPENAI_PROVIDER_ID,
OLLAMA_OSS_PROVIDER_ID,
@@ -154,7 +154,7 @@ const RESERVED_MODEL_PROVIDER_IDS: [&str; 3] = [
#[cfg(target_os = "linux")]
pub fn system_bwrap_warning() -> Option<String> {
system_bwrap_warning_for_path(Path::new(SYSTEM_BWRAP_PATH))
system_bwrap_warning_for_lookup(find_system_bwrap_in_path())
}
#[cfg(not(target_os = "linux"))]
@@ -163,15 +163,40 @@ pub fn system_bwrap_warning() -> Option<String> {
}
#[cfg(target_os = "linux")]
fn system_bwrap_warning_for_path(system_bwrap_path: &Path) -> Option<String> {
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()
));
fn system_bwrap_warning_for_lookup(system_bwrap_path: Option<PathBuf>) -> Option<String> {
match system_bwrap_path {
Some(_) => None,
None => Some(
"Codex could not find system bubblewrap on PATH. Please install bubblewrap with your package manager. Codex will use the vendored bubblewrap in the meantime."
.to_string(),
),
}
}
None
#[cfg(target_os = "linux")]
pub fn find_system_bwrap_in_path() -> Option<PathBuf> {
let search_path = std::env::var_os("PATH")?;
let cwd = std::env::current_dir().ok()?;
find_system_bwrap_in_search_paths(std::iter::once(PathBuf::from(search_path)), &cwd)
}
#[cfg(target_os = "linux")]
fn find_system_bwrap_in_search_paths(
search_paths: impl IntoIterator<Item = PathBuf>,
cwd: &Path,
) -> Option<PathBuf> {
let search_path = std::env::join_paths(search_paths).ok()?;
let cwd = std::fs::canonicalize(cwd).unwrap_or_else(|_| cwd.to_path_buf());
which::which_in_all(SYSTEM_BWRAP_PROGRAM, Some(search_path), cwd)
.ok()?
.find_map(|path| {
let path = std::fs::canonicalize(path).ok()?;
if path.starts_with(&cwd) {
None
} else {
Some(path)
}
})
}
fn resolve_sqlite_home_env(resolved_cwd: &Path) -> Option<PathBuf> {

View File

@@ -7,25 +7,27 @@ This crate is responsible for producing:
- the `codex-exec` CLI can check if its arg0 is `codex-linux-sandbox` and, if so, execute as if it were `codex-linux-sandbox`
- 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 present but too old to support
On Linux, the bubblewrap pipeline prefers the first `bwrap` found on `PATH`
outside the current working directory whenever it is available. If `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.
no-`--argv0` compatibility path for the inner re-exec. If `bwrap` is missing,
the helper falls back to the vendored bubblewrap path compiled into this
binary.
Codex also surfaces a startup warning when `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, 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.
- If `bwrap` is present on `PATH` outside the current working directory, the
helper uses it.
- If `bwrap` is present but too old to support `--argv0`, the helper uses a
no-`--argv0` compatibility path for the inner re-exec.
- If `bwrap` is missing, the helper falls back to the vendored bubblewrap
path.
- If `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`)

View File

@@ -10,8 +10,6 @@ use std::sync::OnceLock;
use crate::vendored_bwrap::exec_vendored_bwrap;
use codex_utils_absolute_path::AbsolutePathBuf;
const SYSTEM_BWRAP_PATH: &str = "/usr/bin/bwrap";
#[derive(Debug, Clone, PartialEq, Eq)]
enum BubblewrapLauncher {
System(SystemBwrapLauncher),
@@ -36,7 +34,10 @@ pub(crate) fn exec_bwrap(argv: Vec<String>, preserved_files: Vec<File>) -> ! {
fn preferred_bwrap_launcher() -> BubblewrapLauncher {
static LAUNCHER: OnceLock<BubblewrapLauncher> = OnceLock::new();
LAUNCHER
.get_or_init(|| preferred_bwrap_launcher_for_path(Path::new(SYSTEM_BWRAP_PATH)))
.get_or_init(|| match codex_core::config::find_system_bwrap_in_path() {
Some(path) => preferred_bwrap_launcher_for_path(&path),
None => BubblewrapLauncher::Vendored,
})
.clone()
}