fix(sandbox): fix bwrap lookup for multi-entry PATH (#15973)

## Summary
- split the joined `PATH` before running system `bwrap` lookup
- keep the existing workspace-local `bwrap` skip behavior intact
- add regression tests that exercise real multi-entry search paths

## Why
The PATH-based lookup added in #15791 still wrapped the raw `PATH`
environment value as a single `PathBuf` before passing it through
`join_paths()`. On Unix, a normal multi-entry `PATH` contains `:`, so
that wrapper path is invalid as one path element and the lookup returns
`None`.

That made Codex behave as if no system `bwrap` was installed even when
`bwrap` was available on `PATH`, which is what users in #15340 were
still hitting on `0.117.0-alpha.25`.

## Impact
System `bwrap` discovery now works with normal multi-entry `PATH` values
instead of silently falling back to the vendored binary.

Fixes #15340.

## Validation
- `just fmt`
- `cargo test -p codex-sandboxing`
- `cargo test -p codex-linux-sandbox`
- `just fix -p codex-sandboxing`
- `just argument-comment-lint`
This commit is contained in:
viyatb-oai
2026-03-27 08:41:06 -07:00
committed by GitHub
Unverified
parent 426f28ca99
commit ec089fd22a
3 changed files with 8 additions and 6 deletions
+1 -1
View File
@@ -179,7 +179,7 @@ impl ActionKind {
Ok((event, Some(command)))
}
ActionKind::RunCommand { command } => {
let event = shell_event(call_id, command, 1_000, sandbox_permissions)?;
let event = shell_event(call_id, command, 2_000, sandbox_permissions)?;
Ok((event, Some(command.to_string())))
}
ActionKind::RunUnifiedExecCommand {
+1 -1
View File
@@ -20,7 +20,7 @@ fn system_bwrap_warning_for_lookup(system_bwrap_path: Option<PathBuf>) -> Option
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)
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd)
}
fn find_system_bwrap_in_search_paths(
+6 -4
View File
@@ -33,7 +33,7 @@ exit 1
}
#[test]
fn finds_first_executable_bwrap_in_search_paths() {
fn finds_first_executable_bwrap_in_joined_search_path() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let first_dir = temp_dir.path().join("first");
@@ -43,15 +43,16 @@ fn finds_first_executable_bwrap_in_search_paths() {
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);
let search_path = std::env::join_paths([first_dir, second_dir]).expect("join search path");
assert_eq!(
find_system_bwrap_in_search_paths(vec![first_dir, second_dir], &cwd),
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd),
Some(expected_bwrap)
);
}
#[test]
fn skips_workspace_local_bwrap_in_search_paths() {
fn skips_workspace_local_bwrap_in_joined_search_path() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let trusted_dir = temp_dir.path().join("trusted");
@@ -59,9 +60,10 @@ fn skips_workspace_local_bwrap_in_search_paths() {
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);
let search_path = std::env::join_paths([cwd.clone(), trusted_dir]).expect("join search path");
assert_eq!(
find_system_bwrap_in_search_paths(vec![cwd.clone(), trusted_dir], &cwd),
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd),
Some(expected_bwrap)
);
}