From 8b95d5467e6d86effba402a2c9b2b628ec0895e8 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Tue, 5 May 2026 13:34:08 -0700 Subject: [PATCH] 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 --- codex-rs/linux-sandbox/src/linux_run_main.rs | 27 ++++++---- .../linux-sandbox/src/linux_run_main_tests.rs | 5 ++ .../linux-sandbox/tests/suite/landlock.rs | 53 +++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index c88a28b324..27cdb5032b 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -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 { 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) { @@ -439,15 +446,15 @@ fn preflight_proc_mount_support( command_cwd: &Path, file_system_sandbox_policy: &FileSystemSandboxPolicy, network_mode: BwrapNetworkMode, -) -> bool { +) -> CodexResult { 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 { let preflight_command = vec![resolve_true_command()]; build_bwrap_argv( preflight_command, diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index 228cea6e5d..95db9d9de1 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -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 == "--")); } diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index efbcd0b486..31021fb9ce 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -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 {