mirror of
https://github.com/openai/codex.git
synced 2026-05-18 02:02:30 +00:00
fix(linux-sandbox): avoid panic on bwrap build failures (#21127)
## Summary - Propagate Linux bubblewrap argument-construction failures instead of panicking in the helper - Keep mutable-symlink carveouts fail-closed while reporting them as ordinary sandbox build failures - Add regression coverage for a protected `.codex` symlink inside a writable workspace root ## Root cause Linux bubblewrap intentionally rejects read-only carveouts that cross a symlink the sandboxed process can still rewrite. That is the correct security behavior for protected metadata paths such as `.codex`. The bug was one layer higher: `linux_run_main` treated the expected build failure as impossible and panicked while constructing the bubblewrap argv. For issue #20716, that turned a normal fail-closed sandbox outcome into a noisy panic in the transcript. ## User impact Users with a project-local `.codex` symlink inside a writable workspace still get the conservative sandbox decision, but they no longer see a Rust panic for that condition. The helper now exits with the concise sandbox-build error so the normal denial / escalation path can handle it. Fixes #20716
This commit is contained in:
@@ -25,6 +25,7 @@ use crate::launcher::exec_bwrap;
|
||||
use crate::launcher::preferred_bwrap_supports_argv0;
|
||||
use crate::proxy_routing::activate_proxy_routes_in_netns;
|
||||
use crate::proxy_routing::prepare_host_proxy_route_spec;
|
||||
use codex_protocol::error::Result as CodexResult;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::NetworkSandboxPolicy;
|
||||
@@ -333,6 +334,7 @@ fn run_bwrap_with_proc_fallback(
|
||||
file_system_sandbox_policy,
|
||||
network_mode,
|
||||
)
|
||||
.unwrap_or_else(|err| exit_with_bwrap_build_error(err))
|
||||
{
|
||||
// Keep the retry silent so sandbox-internal diagnostics do not leak into the
|
||||
// child process stderr stream.
|
||||
@@ -350,7 +352,8 @@ fn run_bwrap_with_proc_fallback(
|
||||
sandbox_policy_cwd,
|
||||
command_cwd,
|
||||
options,
|
||||
);
|
||||
)
|
||||
.unwrap_or_else(|err| exit_with_bwrap_build_error(err));
|
||||
apply_inner_command_argv0(&mut bwrap_args.args);
|
||||
run_or_exec_bwrap(bwrap_args);
|
||||
}
|
||||
@@ -374,24 +377,28 @@ fn build_bwrap_argv(
|
||||
sandbox_policy_cwd: &Path,
|
||||
command_cwd: &Path,
|
||||
options: BwrapOptions,
|
||||
) -> crate::bwrap::BwrapArgs {
|
||||
) -> CodexResult<crate::bwrap::BwrapArgs> {
|
||||
let bwrap_args = create_bwrap_command_args(
|
||||
inner,
|
||||
file_system_sandbox_policy,
|
||||
sandbox_policy_cwd,
|
||||
command_cwd,
|
||||
options,
|
||||
)
|
||||
.unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}"));
|
||||
)?;
|
||||
|
||||
let mut argv = vec!["bwrap".to_string()];
|
||||
argv.extend(bwrap_args.args);
|
||||
crate::bwrap::BwrapArgs {
|
||||
Ok(crate::bwrap::BwrapArgs {
|
||||
args: argv,
|
||||
preserved_files: bwrap_args.preserved_files,
|
||||
synthetic_mount_targets: bwrap_args.synthetic_mount_targets,
|
||||
protected_create_targets: bwrap_args.protected_create_targets,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn exit_with_bwrap_build_error(err: codex_protocol::error::CodexErr) -> ! {
|
||||
eprintln!("error building bubblewrap command: {err}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
fn apply_inner_command_argv0(argv: &mut Vec<String>) {
|
||||
@@ -439,15 +446,15 @@ fn preflight_proc_mount_support(
|
||||
command_cwd: &Path,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
network_mode: BwrapNetworkMode,
|
||||
) -> bool {
|
||||
) -> CodexResult<bool> {
|
||||
let preflight_argv = build_preflight_bwrap_argv(
|
||||
sandbox_policy_cwd,
|
||||
command_cwd,
|
||||
file_system_sandbox_policy,
|
||||
network_mode,
|
||||
);
|
||||
)?;
|
||||
let stderr = run_bwrap_in_child_capture_stderr(preflight_argv);
|
||||
!is_proc_mount_failure(stderr.as_str())
|
||||
Ok(!is_proc_mount_failure(stderr.as_str()))
|
||||
}
|
||||
|
||||
fn build_preflight_bwrap_argv(
|
||||
@@ -455,7 +462,7 @@ fn build_preflight_bwrap_argv(
|
||||
command_cwd: &Path,
|
||||
file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
network_mode: BwrapNetworkMode,
|
||||
) -> crate::bwrap::BwrapArgs {
|
||||
) -> CodexResult<crate::bwrap::BwrapArgs> {
|
||||
let preflight_command = vec![resolve_true_command()];
|
||||
build_bwrap_argv(
|
||||
preflight_command,
|
||||
|
||||
@@ -61,6 +61,7 @@ fn inserts_bwrap_argv0_before_command_separator() {
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("build bwrap argv")
|
||||
.args;
|
||||
apply_inner_command_argv0_for_launcher(
|
||||
&mut argv,
|
||||
@@ -104,6 +105,7 @@ fn rewrites_inner_command_path_when_bwrap_lacks_argv0() {
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("build bwrap argv")
|
||||
.args;
|
||||
apply_inner_command_argv0_for_launcher(
|
||||
&mut argv,
|
||||
@@ -172,6 +174,7 @@ fn inserts_unshare_net_when_network_isolation_requested() {
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("build bwrap argv")
|
||||
.args;
|
||||
assert!(argv.contains(&"--unshare-net".to_string()));
|
||||
}
|
||||
@@ -190,6 +193,7 @@ fn inserts_unshare_net_when_proxy_only_network_mode_requested() {
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.expect("build bwrap argv")
|
||||
.args;
|
||||
assert!(argv.contains(&"--unshare-net".to_string()));
|
||||
}
|
||||
@@ -265,6 +269,7 @@ fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() {
|
||||
&FileSystemSandboxPolicy::unrestricted(),
|
||||
mode,
|
||||
)
|
||||
.expect("build preflight argv")
|
||||
.args;
|
||||
assert!(argv.iter().any(|arg| arg == "--"));
|
||||
}
|
||||
|
||||
@@ -587,6 +587,59 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() {
|
||||
assert_ne!(codex_output.exit_code, 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sandbox_reports_codex_symlink_build_failure_without_panicking() {
|
||||
if should_skip_bwrap_tests().await {
|
||||
eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable");
|
||||
return;
|
||||
}
|
||||
|
||||
use std::os::unix::fs::symlink;
|
||||
|
||||
let tmpdir = tempfile::tempdir().expect("tempdir");
|
||||
let decoy = tmpdir.path().join("decoy-codex");
|
||||
std::fs::create_dir_all(&decoy).expect("create decoy dir");
|
||||
|
||||
let dot_codex = tmpdir.path().join(".codex");
|
||||
symlink(&decoy, &dot_codex).expect("create .codex symlink");
|
||||
|
||||
let output = match run_cmd_result_with_writable_roots(
|
||||
&["bash", "-lc", "true"],
|
||||
&[tmpdir.path().to_path_buf()],
|
||||
LONG_TIMEOUT_MS,
|
||||
/*use_legacy_landlock*/ false,
|
||||
/*network_access*/ true,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output,
|
||||
result => panic!(".codex symlink build failure should deny: {result:?}"),
|
||||
};
|
||||
|
||||
assert_eq!(output.exit_code, 1);
|
||||
assert!(
|
||||
output
|
||||
.stderr
|
||||
.text
|
||||
.contains("error building bubblewrap command:"),
|
||||
"stderr: {}",
|
||||
output.stderr.text
|
||||
);
|
||||
assert!(
|
||||
output
|
||||
.stderr
|
||||
.text
|
||||
.contains("cannot enforce sandbox read-only path"),
|
||||
"stderr: {}",
|
||||
output.stderr.text
|
||||
);
|
||||
assert!(
|
||||
!output.stderr.text.contains("panicked at"),
|
||||
"stderr: {}",
|
||||
output.stderr.text
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sandbox_keeps_parent_repo_discovery_while_blocking_child_metadata() {
|
||||
if should_skip_bwrap_tests().await {
|
||||
|
||||
Reference in New Issue
Block a user