From 913d8afeb499a1b288cc8cd1b6629f01a3596ce8 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 27 May 2026 14:28:59 -0700 Subject: [PATCH] fix(linux-sandbox): preserve detached children Co-authored-by: Codex noreply@openai.com --- codex-rs/linux-sandbox/src/bwrap.rs | 52 +++++++++++++++++-- codex-rs/linux-sandbox/src/linux_run_main.rs | 14 +++++ .../linux-sandbox/tests/suite/landlock.rs | 42 +++++++++++++++ codex-rs/sandboxing/src/landlock.rs | 1 + codex-rs/sandboxing/src/landlock_tests.rs | 1 + 5 files changed, 105 insertions(+), 5 deletions(-) diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 25a3814fb5..ac5e650cb5 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -65,6 +65,8 @@ pub(crate) struct BwrapOptions { pub mount_proc: bool, /// How networking should be configured inside the bubblewrap sandbox. pub network_mode: BwrapNetworkMode, + /// How the sandbox lifetime should relate to the initial command process. + pub process_lifetime: BwrapProcessLifetime, /// Optional maximum depth for expanding unreadable glob patterns with ripgrep. /// /// Keep this uncapped by default so existing nested deny-read matches are @@ -77,11 +79,22 @@ impl Default for BwrapOptions { Self { mount_proc: true, network_mode: BwrapNetworkMode::FullAccess, + process_lifetime: BwrapProcessLifetime::TerminateWithParent, glob_scan_max_depth: None, } } } +/// Lifetime behavior for descendants running inside bubblewrap. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub(crate) enum BwrapProcessLifetime { + /// Ask bubblewrap to kill the sandbox when its parent exits. + #[default] + TerminateWithParent, + /// Allow intentionally detached children to keep running in the sandbox. + AllowDetachedChildren, +} + /// Network policy modes for bubblewrap. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub(crate) enum BwrapNetworkMode { @@ -265,9 +278,11 @@ pub(crate) fn create_bwrap_command_args( } fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOptions) -> BwrapArgs { - let mut args = vec![ - "--new-session".to_string(), - "--die-with-parent".to_string(), + let mut args = vec!["--new-session".to_string()]; + if options.process_lifetime == BwrapProcessLifetime::TerminateWithParent { + args.push("--die-with-parent".to_string()); + } + args.extend([ "--bind".to_string(), "/".to_string(), "/".to_string(), @@ -275,7 +290,7 @@ fn create_bwrap_flags_full_filesystem(command: Vec, options: BwrapOption // not need ambient CAP_SYS_ADMIN to create the remaining namespaces. "--unshare-user".to_string(), "--unshare-pid".to_string(), - ]; + ]); if options.network_mode.should_unshare_network() { args.push("--unshare-net".to_string()); } @@ -316,7 +331,9 @@ fn create_bwrap_flags( let normalized_command_cwd = normalize_command_cwd_for_bwrap(command_cwd); let mut args = Vec::new(); args.push("--new-session".to_string()); - args.push("--die-with-parent".to_string()); + if options.process_lifetime == BwrapProcessLifetime::TerminateWithParent { + args.push("--die-with-parent".to_string()); + } args.extend(filesystem_args); // Request a user namespace explicitly rather than relying on bubblewrap's // auto-enable behavior, which is skipped when the caller runs as uid 0. @@ -1411,6 +1428,31 @@ mod tests { ); } + #[test] + fn allow_detached_children_omits_die_with_parent_but_keeps_pid_namespace() { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }]); + let args = create_bwrap_command_args( + vec!["/bin/true".to_string()], + &policy, + Path::new("/"), + Path::new("/"), + BwrapOptions { + process_lifetime: BwrapProcessLifetime::AllowDetachedChildren, + ..Default::default() + }, + ) + .expect("create bwrap args"); + + assert!(!args.args.contains(&"--die-with-parent".to_string())); + assert!(args.args.contains(&"--unshare-pid".to_string())); + assert!(args.args.contains(&"--unshare-user".to_string())); + } + #[test] fn full_disk_write_with_unreadable_glob_still_wraps_and_masks_match() { if !ripgrep_available() { diff --git a/codex-rs/linux-sandbox/src/linux_run_main.rs b/codex-rs/linux-sandbox/src/linux_run_main.rs index 346b1f14c0..b5161fa770 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main.rs @@ -19,6 +19,7 @@ use std::time::Duration; use crate::bwrap::BwrapNetworkMode; use crate::bwrap::BwrapOptions; +use crate::bwrap::BwrapProcessLifetime; use crate::bwrap::create_bwrap_command_args; use crate::landlock::apply_permission_profile_to_current_thread; use crate::launcher::exec_bwrap; @@ -122,6 +123,11 @@ pub struct LandlockCommand { #[arg(long = "allow-network-for-proxy", hide = true, default_value_t = false)] pub allow_network_for_proxy: bool, + /// Internal: permit intentionally detached command descendants to outlive + /// the one-shot sandbox helper invocation. + #[arg(long = "allow-detached-children", hide = true, default_value_t = false)] + pub allow_detached_children: bool, + /// Internal route spec used for managed proxy routing in bwrap mode. #[arg(long = "proxy-route-spec", hide = true)] pub proxy_route_spec: Option, @@ -152,6 +158,7 @@ pub fn run_main() -> ! { use_legacy_landlock, apply_seccomp_then_exec, allow_network_for_proxy, + allow_detached_children, proxy_route_spec, no_proc, command, @@ -238,6 +245,11 @@ pub fn run_main() -> ! { inner, !no_proc, allow_network_for_proxy, + if allow_detached_children { + BwrapProcessLifetime::AllowDetachedChildren + } else { + BwrapProcessLifetime::TerminateWithParent + }, ); } @@ -322,6 +334,7 @@ fn run_bwrap_with_proc_fallback( inner: Vec, mount_proc: bool, allow_network_for_proxy: bool, + process_lifetime: BwrapProcessLifetime, ) -> ! { let network_mode = bwrap_network_mode(network_sandbox_policy, allow_network_for_proxy); let mut mount_proc = mount_proc; @@ -344,6 +357,7 @@ fn run_bwrap_with_proc_fallback( let options = BwrapOptions { mount_proc, network_mode, + process_lifetime, ..Default::default() }; let mut bwrap_args = build_bwrap_argv( diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index e18f2a17b4..a5171acc04 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -21,6 +21,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::collections::HashMap; use std::path::PathBuf; +use std::time::Duration; use tempfile::NamedTempFile; // At least on GitHub CI, the arm64 tests appear to need longer timeouts. @@ -393,6 +394,47 @@ async fn sandbox_ignores_missing_writable_roots_under_bwrap() { assert_eq!(output.stdout.text, "sandbox-ok"); } +#[tokio::test] +async fn detached_children_survive_parent_exit_under_bwrap() { + if should_skip_bwrap_tests().await { + eprintln!("skipping bwrap test: bwrap sandbox prerequisites are unavailable"); + return; + } + + let tempdir = tempfile::tempdir().expect("tempdir"); + let log_path = tempdir.path().join("detached.log"); + let command = format!( + "nohup sh -c 'for i in 1 2 3 4 5 6 7 8 9 10; do printf x >> \"{log}\"; sleep 0.2; done' >/dev/null 2>&1 first_len, + "detached child should keep writing after the parent exits", + ); +} + #[tokio::test] async fn test_no_new_privs_is_enabled() { let output = run_cmd_output( diff --git a/codex-rs/sandboxing/src/landlock.rs b/codex-rs/sandboxing/src/landlock.rs index 0ff3f6977f..dcf8e4d011 100644 --- a/codex-rs/sandboxing/src/landlock.rs +++ b/codex-rs/sandboxing/src/landlock.rs @@ -53,6 +53,7 @@ pub fn create_linux_sandbox_command_args_for_permission_profile( if allow_network_for_proxy { linux_cmd.push("--allow-network-for-proxy".to_string()); } + linux_cmd.push("--allow-detached-children".to_string()); linux_cmd.push("--".to_string()); linux_cmd.extend(command); linux_cmd diff --git a/codex-rs/sandboxing/src/landlock_tests.rs b/codex-rs/sandboxing/src/landlock_tests.rs index 14b1c047eb..184a04fa0a 100644 --- a/codex-rs/sandboxing/src/landlock_tests.rs +++ b/codex-rs/sandboxing/src/landlock_tests.rs @@ -77,6 +77,7 @@ fn permission_profile_flag_is_included() { .any(|window| window[0] == "--command-cwd" && window[1] == "/tmp/link"), true ); + assert!(args.contains(&"--allow-detached-children".to_string())); } #[test]