From 5da322b66eb68d3d3d8d59f5dad9104f34b1a2a9 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 6 Mar 2026 14:12:25 -0800 Subject: [PATCH] linux-sandbox: honor split filesystem policies in bwrap --- codex-rs/linux-sandbox/src/bwrap.rs | 83 +++++++++++-- codex-rs/linux-sandbox/src/linux_run_main.rs | 39 +++++-- .../linux-sandbox/src/linux_run_main_tests.rs | 15 ++- .../linux-sandbox/tests/suite/landlock.rs | 109 +++++++++++++++--- 4 files changed, 204 insertions(+), 42 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 783c3a428c..c6969a7d0c 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; use codex_core::error::CodexErr; use codex_core::error::Result; +use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::WritableRoot; @@ -89,7 +90,17 @@ pub(crate) fn create_bwrap_command_args( cwd: &Path, options: BwrapOptions, ) -> Result> { - if sandbox_policy.has_full_disk_write_access() { + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(sandbox_policy); + create_bwrap_command_args_for_policy(command, &file_system_sandbox_policy, cwd, options) +} + +pub(crate) fn create_bwrap_command_args_for_policy( + command: Vec, + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, + options: BwrapOptions, +) -> Result> { + if file_system_sandbox_policy.has_full_disk_write_access() { return if options.network_mode == BwrapNetworkMode::FullAccess { Ok(command) } else { @@ -97,7 +108,7 @@ pub(crate) fn create_bwrap_command_args( }; } - create_bwrap_flags(command, sandbox_policy, cwd, options) + create_bwrap_flags(command, file_system_sandbox_policy, cwd, options) } fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOptions) -> Vec { @@ -127,14 +138,14 @@ fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOption /// Build the bubblewrap flags (everything after `argv[0]`). fn create_bwrap_flags( command: Vec, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, cwd: &Path, options: BwrapOptions, ) -> Result> { let mut args = Vec::new(); args.push("--new-session".to_string()); args.push("--die-with-parent".to_string()); - args.extend(create_filesystem_args(sandbox_policy, cwd)?); + args.extend(create_filesystem_args(file_system_sandbox_policy, cwd)?); // Request a user namespace explicitly rather than relying on bubblewrap's // auto-enable behavior, which is skipped when the caller runs as uid 0. args.push("--unshare-user".to_string()); @@ -153,7 +164,7 @@ fn create_bwrap_flags( Ok(args) } -/// Build the bubblewrap filesystem mounts for a given sandbox policy. +/// Build the bubblewrap filesystem mounts for a given filesystem policy. /// /// The mount order is important: /// 1. Full-read policies use `--ro-bind / /`; restricted-read policies start @@ -164,11 +175,14 @@ fn create_bwrap_flags( /// writable subpaths under `/dev` (for example, `/dev/shm`). /// 4. `--ro-bind ` re-applies read-only protections under /// those writable roots so protected subpaths win. -fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result> { - let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd); +fn create_filesystem_args( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + cwd: &Path, +) -> Result> { + let writable_roots = file_system_sandbox_policy.get_writable_roots_with_cwd(cwd); ensure_mount_targets_exist(&writable_roots)?; - let mut args = if sandbox_policy.has_full_disk_read_access() { + let mut args = if file_system_sandbox_policy.has_full_disk_read_access() { // Read-only root, then mount a minimal device tree. // In bubblewrap (`bubblewrap.c`, `SETUP_MOUNT_DEV`), `--dev /dev` // creates the standard minimal nodes: null, zero, full, random, @@ -191,12 +205,12 @@ fn create_filesystem_args(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Result< "/dev".to_string(), ]; - let mut readable_roots: BTreeSet = sandbox_policy + let mut readable_roots: BTreeSet = file_system_sandbox_policy .get_readable_roots_with_cwd(cwd) .into_iter() .map(PathBuf::from) .collect(); - if sandbox_policy.include_platform_defaults() { + if file_system_sandbox_policy.include_platform_defaults() { readable_roots.extend( LINUX_PLATFORM_DEFAULT_READ_ROOTS .iter() @@ -386,6 +400,12 @@ fn find_first_non_existent_component(target_path: &Path) -> Option { #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::FileSystemAccessMode; + use codex_protocol::protocol::FileSystemPath; + use codex_protocol::protocol::FileSystemSandboxEntry; + use codex_protocol::protocol::FileSystemSandboxPolicy; + use codex_protocol::protocol::FileSystemSpecialPath; + use codex_protocol::protocol::FileSystemSpecialPathKind; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -528,4 +548,47 @@ mod tests { ); } } + + #[test] + fn split_policy_reapplies_unreadable_carveouts_after_writable_binds() { + let temp_dir = TempDir::new().expect("temp dir"); + let writable_root = temp_dir.path().join("workspace"); + let blocked = writable_root.join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked dir"); + let writable_root = + AbsolutePathBuf::from_absolute_path(&writable_root).expect("absolute writable root"); + let blocked = AbsolutePathBuf::from_absolute_path(&blocked).expect("absolute blocked dir"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: writable_root.clone(), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: blocked.clone(), + }, + access: FileSystemAccessMode::None, + }, + ]); + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let writable_root_str = path_to_string(writable_root.as_path()); + let blocked_str = path_to_string(blocked.as_path()); + + assert!(args.windows(3).any(|window| { + window + == [ + "--bind", + writable_root_str.as_str(), + writable_root_str.as_str(), + ] + })); + assert!( + args.windows(3).any(|window| { + window == ["--ro-bind", blocked_str.as_str(), blocked_str.as_str()] + }) + ); + } } diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 6a624ac1dd..b33f933e6b 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; -use crate::bwrap::create_bwrap_command_args; +use crate::bwrap::create_bwrap_command_args_for_policy; use crate::landlock::apply_sandbox_policy_to_current_thread; use crate::proxy_routing::activate_proxy_routes_in_netns; use crate::proxy_routing::prepare_host_proxy_route_spec; @@ -178,7 +178,7 @@ pub fn run_main() -> ! { ); run_bwrap_with_proc_fallback( &sandbox_policy_cwd, - &sandbox_policy, + &file_system_sandbox_policy, network_sandbox_policy, inner, !no_proc, @@ -255,7 +255,7 @@ fn ensure_inner_stage_mode_is_valid(apply_seccomp_then_exec: bool, use_bwrap_san fn run_bwrap_with_proc_fallback( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_sandbox_policy: NetworkSandboxPolicy, inner: Vec, mount_proc: bool, @@ -264,7 +264,12 @@ fn run_bwrap_with_proc_fallback( let network_mode = bwrap_network_mode(network_sandbox_policy, allow_network_for_proxy); let mut mount_proc = mount_proc; - if mount_proc && !preflight_proc_mount_support(sandbox_policy_cwd, sandbox_policy, network_mode) + if mount_proc + && !preflight_proc_mount_support( + sandbox_policy_cwd, + file_system_sandbox_policy, + network_mode, + ) { eprintln!("codex-linux-sandbox: bwrap could not mount /proc; retrying with --no-proc"); mount_proc = false; @@ -274,7 +279,12 @@ fn run_bwrap_with_proc_fallback( mount_proc, network_mode, }; - let argv = build_bwrap_argv(inner, sandbox_policy, sandbox_policy_cwd, options); + let argv = build_bwrap_argv( + inner, + file_system_sandbox_policy, + sandbox_policy_cwd, + options, + ); exec_vendored_bwrap(argv); } @@ -293,12 +303,17 @@ fn bwrap_network_mode( fn build_bwrap_argv( inner: Vec, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, sandbox_policy_cwd: &Path, options: BwrapOptions, ) -> Vec { - let mut args = create_bwrap_command_args(inner, sandbox_policy, sandbox_policy_cwd, options) - .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); + let mut args = create_bwrap_command_args_for_policy( + inner, + file_system_sandbox_policy, + sandbox_policy_cwd, + options, + ) + .unwrap_or_else(|err| panic!("error building bubblewrap command: {err:?}")); let command_separator_index = args .iter() @@ -316,24 +331,24 @@ fn build_bwrap_argv( fn preflight_proc_mount_support( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_mode: BwrapNetworkMode, ) -> bool { let preflight_argv = - build_preflight_bwrap_argv(sandbox_policy_cwd, sandbox_policy, network_mode); + build_preflight_bwrap_argv(sandbox_policy_cwd, file_system_sandbox_policy, network_mode); let stderr = run_bwrap_in_child_capture_stderr(preflight_argv); !is_proc_mount_failure(stderr.as_str()) } fn build_preflight_bwrap_argv( sandbox_policy_cwd: &Path, - sandbox_policy: &SandboxPolicy, + file_system_sandbox_policy: &FileSystemSandboxPolicy, network_mode: BwrapNetworkMode, ) -> Vec { let preflight_command = vec![resolve_true_command()]; build_bwrap_argv( preflight_command, - sandbox_policy, + file_system_sandbox_policy, sandbox_policy_cwd, BwrapOptions { mount_proc: true, 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 c466de5e27..6be9d35c03 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -35,9 +35,10 @@ fn ignores_non_proc_mount_errors() { #[test] fn inserts_bwrap_argv0_before_command_separator() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -69,9 +70,10 @@ fn inserts_bwrap_argv0_before_command_separator() { #[test] fn inserts_unshare_net_when_network_isolation_requested() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -83,9 +85,10 @@ fn inserts_unshare_net_when_network_isolation_requested() { #[test] fn inserts_unshare_net_when_proxy_only_network_mode_requested() { + let sandbox_policy = SandboxPolicy::new_read_only_policy(); let argv = build_bwrap_argv( vec!["/bin/true".to_string()], - &SandboxPolicy::new_read_only_policy(), + &FileSystemSandboxPolicy::from(&sandbox_policy), Path::new("/"), BwrapOptions { mount_proc: true, @@ -104,7 +107,11 @@ fn proxy_only_mode_takes_precedence_over_full_network_policy() { #[test] fn managed_proxy_preflight_argv_is_wrapped_for_full_access_policy() { let mode = bwrap_network_mode(NetworkSandboxPolicy::Enabled, true); - let argv = build_preflight_bwrap_argv(Path::new("/"), &SandboxPolicy::DangerFullAccess, mode); + let argv = build_preflight_bwrap_argv( + Path::new("/"), + &FileSystemSandboxPolicy::from(&SandboxPolicy::DangerFullAccess), + mode, + ); 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 b12170584c..d303cbd6d3 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -9,6 +9,9 @@ use codex_core::exec::process_exec_tool_call; use codex_core::exec_env::create_env; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::protocol::FileSystemAccessMode; +use codex_protocol::protocol::FileSystemPath; +use codex_protocol::protocol::FileSystemSandboxEntry; use codex_protocol::protocol::FileSystemSandboxPolicy; use codex_protocol::protocol::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; @@ -71,20 +74,6 @@ async fn run_cmd_result_with_writable_roots( use_bwrap_sandbox: bool, network_access: bool, ) -> Result { - let cwd = std::env::current_dir().expect("cwd should exist"); - let sandbox_cwd = cwd.clone(); - let params = ExecParams { - command: cmd.iter().copied().map(str::to_owned).collect(), - cwd, - expiration: timeout_ms.into(), - env: create_env_from_core_vars(), - network: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - justification: None, - arg0: None, - }; - let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: writable_roots .iter() @@ -98,14 +87,49 @@ async fn run_cmd_result_with_writable_roots( exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::from(&sandbox_policy); + let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); + run_cmd_result_with_policies( + cmd, + sandbox_policy, + file_system_sandbox_policy, + network_sandbox_policy, + timeout_ms, + use_bwrap_sandbox, + ) + .await +} + +#[expect(clippy::expect_used)] +async fn run_cmd_result_with_policies( + cmd: &[&str], + sandbox_policy: SandboxPolicy, + file_system_sandbox_policy: FileSystemSandboxPolicy, + network_sandbox_policy: NetworkSandboxPolicy, + timeout_ms: u64, + use_bwrap_sandbox: bool, +) -> Result { + let cwd = std::env::current_dir().expect("cwd should exist"); + let sandbox_cwd = cwd.clone(); + let params = ExecParams { + command: cmd.iter().copied().map(str::to_owned).collect(), + cwd, + expiration: timeout_ms.into(), + env: create_env_from_core_vars(), + network: None, + sandbox_permissions: SandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + justification: None, + arg0: None, + }; let sandbox_program = env!("CARGO_BIN_EXE_codex-linux-sandbox"); let codex_linux_sandbox_exe = Some(PathBuf::from(sandbox_program)); process_exec_tool_call( params, &sandbox_policy, - &FileSystemSandboxPolicy::from(&sandbox_policy), - NetworkSandboxPolicy::from(&sandbox_policy), + &file_system_sandbox_policy, + network_sandbox_policy, sandbox_cwd.as_path(), &codex_linux_sandbox_exe, use_bwrap_sandbox, @@ -479,6 +503,59 @@ async fn sandbox_blocks_codex_symlink_replacement_attack() { assert_ne!(codex_output.exit_code, 0); } +#[tokio::test] +async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let tmpdir = tempfile::tempdir().expect("tempdir"); + let blocked = tmpdir.path().join("blocked"); + std::fs::create_dir_all(&blocked).expect("create blocked dir"); + let blocked_target = blocked.join("secret.txt"); + + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir")], + read_only_access: Default::default(), + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + }, + access: FileSystemAccessMode::None, + }, + ]); + let output = expect_denied( + run_cmd_result_with_policies( + &[ + "bash", + "-lc", + &format!("echo denied > {}", blocked_target.to_string_lossy()), + ], + sandbox_policy, + file_system_sandbox_policy, + NetworkSandboxPolicy::Enabled, + LONG_TIMEOUT_MS, + true, + ) + .await, + "explicit split-policy carveout should be denied under bubblewrap", + ); + + assert_ne!(output.exit_code, 0); +} + #[tokio::test] async fn sandbox_blocks_ssh() { // Force ssh to attempt a real TCP connection but fail quickly. `BatchMode`