mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Reap orphaned processes in Linux sandboxes (#38396)
## Why Sandboxed descendants can outlive their immediate parent and must be collected by PID 1 in the Bubblewrap namespace. ## What changed - Launch `codex-linux-sandbox` with Bubblewrap's `--as-pid-1` option, and fall back to the bundled Bubblewrap when the system version does not support it. - Run the sandboxed command as a child, forward signals to it, reap other exited descendants, and preserve the command's exit status. - Verify proxy bridge parent identity when arming its parent-death signal. ## Testing Added Linux sandbox coverage for the filtered namespace reaper, orphan collection, and fallback from an incompatible system Bubblewrap. GitOrigin-RevId: 379f08d6c2732ea0a4caeb61f93ae302e16d2458
This commit is contained in:
@@ -117,7 +117,7 @@ set -euo pipefail
|
||||
|
||||
for arg in "$@"; do
|
||||
if [[ "${arg}" == "--help" ]]; then
|
||||
echo "Usage: bwrap --argv0 --perms"
|
||||
echo "Usage: bwrap --argv0 --perms --as-pid-1"
|
||||
exit 0
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -33,7 +33,9 @@ struct SystemBwrapCapabilities {
|
||||
supports_perms: bool,
|
||||
}
|
||||
|
||||
pub(crate) fn exec_bwrap(argv: Vec<String>, preserved_files: Vec<File>) -> ! {
|
||||
pub(crate) fn exec_bwrap(mut argv: Vec<String>, preserved_files: Vec<File>) -> ! {
|
||||
argv.insert(1, "--as-pid-1".to_string());
|
||||
|
||||
match preferred_bwrap_launcher() {
|
||||
BubblewrapLauncher::System(launcher) => {
|
||||
exec_system_bwrap(&launcher.program, argv, preserved_files)
|
||||
@@ -117,6 +119,9 @@ fn system_bwrap_capabilities(system_bwrap_path: &Path) -> Option<SystemBwrapCapa
|
||||
};
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
if !stdout.contains("--as-pid-1") && !stderr.contains("--as-pid-1") {
|
||||
return None;
|
||||
}
|
||||
Some(SystemBwrapCapabilities {
|
||||
supports_argv0: stdout.contains("--argv0") || stderr.contains("--argv0"),
|
||||
supports_perms: stdout.contains("--perms") || stderr.contains("--perms"),
|
||||
|
||||
@@ -198,7 +198,40 @@ pub fn run_main() -> ! {
|
||||
) {
|
||||
panic!("error applying Linux sandbox restrictions: {e:?}");
|
||||
}
|
||||
exec_or_panic(command);
|
||||
|
||||
let signal_mask = ForwardedSignalMask::block();
|
||||
let command_pid = unsafe { libc::fork() };
|
||||
if command_pid < 0 {
|
||||
let err = std::io::Error::last_os_error();
|
||||
panic!("failed to fork sandboxed command: {err}");
|
||||
}
|
||||
|
||||
if command_pid == 0 {
|
||||
reset_forwarded_signal_handlers_to_default();
|
||||
signal_mask.restore();
|
||||
exec_or_panic(command);
|
||||
}
|
||||
|
||||
let signal_forwarders = install_bwrap_signal_forwarders(command_pid);
|
||||
signal_mask.restore();
|
||||
loop {
|
||||
let mut status = 0;
|
||||
let reaped_pid = unsafe { libc::waitpid(-1, &mut status, 0) };
|
||||
if reaped_pid == command_pid {
|
||||
let exit_signal_mask = ForwardedSignalMask::block();
|
||||
signal_forwarders.restore();
|
||||
exit_signal_mask.restore();
|
||||
exit_with_wait_status(status);
|
||||
}
|
||||
if reaped_pid >= 0 {
|
||||
continue;
|
||||
}
|
||||
|
||||
let err = std::io::Error::last_os_error();
|
||||
if err.raw_os_error() != Some(libc::EINTR) {
|
||||
panic!("failed to reap sandboxed child: {err}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if file_system_sandbox_policy.has_full_disk_write_access() && !allow_network_for_proxy {
|
||||
|
||||
@@ -118,17 +118,17 @@ fn cleanup_proxy_socket_dir(socket_dir: &Path) -> io::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn harden_bridge_process() -> io::Result<()> {
|
||||
pub(crate) fn harden_bridge_process(expected_parent_pid: libc::pid_t) -> io::Result<()> {
|
||||
detach_bridge_stdio()?;
|
||||
set_parent_death_signal()?;
|
||||
set_parent_death_signal(expected_parent_pid)?;
|
||||
codex_process_hardening::disable_process_dumping()
|
||||
}
|
||||
|
||||
fn set_parent_death_signal() -> io::Result<()> {
|
||||
fn set_parent_death_signal(expected_parent_pid: libc::pid_t) -> io::Result<()> {
|
||||
let res = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) };
|
||||
if res != 0 {
|
||||
Err(io::Error::last_os_error())
|
||||
} else if unsafe { libc::getppid() } == 1 {
|
||||
} else if unsafe { libc::getppid() } != expected_parent_pid {
|
||||
Err(io::Error::other("parent process already exited"))
|
||||
} else {
|
||||
Ok(())
|
||||
|
||||
@@ -377,6 +377,7 @@ fn spawn_host_bridge(
|
||||
attribution_token: Option<&str>,
|
||||
) -> io::Result<libc::pid_t> {
|
||||
let (read_fd, write_fd) = create_ready_pipe()?;
|
||||
let parent_pid = unsafe { libc::getpid() };
|
||||
let pid = unsafe { libc::fork() };
|
||||
if pid < 0 {
|
||||
let err = io::Error::last_os_error();
|
||||
@@ -389,7 +390,7 @@ fn spawn_host_bridge(
|
||||
if close_fd(read_fd).is_err() {
|
||||
unsafe { libc::_exit(1) };
|
||||
}
|
||||
let result = run_host_bridge(endpoint, uds_path, write_fd, attribution_token);
|
||||
let result = run_host_bridge(endpoint, uds_path, write_fd, attribution_token, parent_pid);
|
||||
if result.is_err() {
|
||||
unsafe { libc::_exit(1) };
|
||||
}
|
||||
@@ -413,8 +414,9 @@ fn run_host_bridge(
|
||||
uds_path: &Path,
|
||||
ready_fd: libc::c_int,
|
||||
attribution_token: Option<&str>,
|
||||
parent_pid: libc::pid_t,
|
||||
) -> io::Result<()> {
|
||||
harden_bridge_process()?;
|
||||
harden_bridge_process(parent_pid)?;
|
||||
if uds_path.exists() {
|
||||
std::fs::remove_file(uds_path)?;
|
||||
}
|
||||
@@ -447,6 +449,7 @@ fn run_host_bridge(
|
||||
|
||||
fn spawn_local_bridge(uds_path: &Path) -> io::Result<u16> {
|
||||
let (read_fd, write_fd) = create_ready_pipe()?;
|
||||
let parent_pid = unsafe { libc::getpid() };
|
||||
let pid = unsafe { libc::fork() };
|
||||
if pid < 0 {
|
||||
let err = io::Error::last_os_error();
|
||||
@@ -459,7 +462,7 @@ fn spawn_local_bridge(uds_path: &Path) -> io::Result<u16> {
|
||||
if close_fd(read_fd).is_err() {
|
||||
unsafe { libc::_exit(1) };
|
||||
}
|
||||
let result = run_local_bridge(uds_path, write_fd);
|
||||
let result = run_local_bridge(uds_path, write_fd, parent_pid);
|
||||
if result.is_err() {
|
||||
unsafe { libc::_exit(1) };
|
||||
}
|
||||
@@ -473,8 +476,12 @@ fn spawn_local_bridge(uds_path: &Path) -> io::Result<u16> {
|
||||
Ok(u16::from_be_bytes(port_bytes))
|
||||
}
|
||||
|
||||
fn run_local_bridge(uds_path: &Path, ready_fd: libc::c_int) -> io::Result<()> {
|
||||
harden_bridge_process()?;
|
||||
fn run_local_bridge(
|
||||
uds_path: &Path,
|
||||
ready_fd: libc::c_int,
|
||||
parent_pid: libc::pid_t,
|
||||
) -> io::Result<()> {
|
||||
harden_bridge_process(parent_pid)?;
|
||||
let listener = bind_local_loopback_listener()?;
|
||||
let port = listener.local_addr()?.port();
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ use std::io::Write;
|
||||
use std::net::Ipv4Addr;
|
||||
use std::net::TcpListener;
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::process::Output;
|
||||
use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
@@ -82,7 +83,9 @@ async fn should_skip_bwrap_tests() -> bool {
|
||||
NETWORK_TIMEOUT_MS,
|
||||
)
|
||||
.await;
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
is_bwrap_unavailable_output(&output)
|
||||
|| (!output.status.success() && is_managed_proxy_permission_error(stderr.as_ref()))
|
||||
}
|
||||
|
||||
fn is_managed_proxy_permission_error(stderr: &str) -> bool {
|
||||
@@ -171,6 +174,207 @@ fn linux_sandbox_command(
|
||||
cmd
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sandboxed_commands_have_a_seccomp_filtered_namespace_reaper() {
|
||||
if should_skip_bwrap_tests().await {
|
||||
eprintln!("skipping bwrap test: bubblewrap is unavailable");
|
||||
return;
|
||||
}
|
||||
|
||||
let mut env = create_env_from_core_vars();
|
||||
strip_proxy_env(&mut env);
|
||||
|
||||
assert_seccomp_filtered_namespace_reaper(
|
||||
&PermissionProfile::read_only(),
|
||||
/*allow_network_for_proxy*/ false,
|
||||
env,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_proxy_commands_have_a_seccomp_filtered_namespace_reaper() {
|
||||
if let Some(skip_reason) = managed_proxy_skip_reason().await {
|
||||
eprintln!("skipping managed proxy test: {skip_reason}");
|
||||
return;
|
||||
}
|
||||
|
||||
let mut env = create_env_from_core_vars();
|
||||
strip_proxy_env(&mut env);
|
||||
env.insert("HTTP_PROXY".to_string(), "http://127.0.0.1:9".to_string());
|
||||
|
||||
for permission_profile in [PermissionProfile::read_only(), PermissionProfile::Disabled] {
|
||||
assert_seccomp_filtered_namespace_reaper(
|
||||
&permission_profile,
|
||||
/*allow_network_for_proxy*/ true,
|
||||
env.clone(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
async fn assert_seccomp_filtered_namespace_reaper(
|
||||
permission_profile: &PermissionProfile,
|
||||
allow_network_for_proxy: bool,
|
||||
env: HashMap<String, String>,
|
||||
) {
|
||||
let inherited_seccomp_filters = std::fs::read_to_string("/proc/self/status")
|
||||
.expect("test process status should be readable")
|
||||
.lines()
|
||||
.find_map(|line| line.strip_prefix("Seccomp_filters:"))
|
||||
.and_then(|count| count.trim().parse::<usize>().ok())
|
||||
.expect("test process status should expose its seccomp filter count");
|
||||
|
||||
let output = run_linux_sandbox_direct(
|
||||
&[
|
||||
"bash",
|
||||
"-c",
|
||||
"if [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; printf '%s\\n' \"$PPID\"; awk '$1 == \"Seccomp:\" && FNR == NR { print $2 } $1 == \"Seccomp_filters:\" { print $2 }' /proc/1/status /proc/self/status",
|
||||
],
|
||||
permission_profile,
|
||||
allow_network_for_proxy,
|
||||
env,
|
||||
NETWORK_TIMEOUT_MS,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
output.status.success(),
|
||||
true,
|
||||
"sandboxed command should execute; stderr={}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
if output.stdout == b"namespace proc unavailable\n" {
|
||||
eprintln!("skipping namespace reaper check: a namespaced proc mount is unavailable");
|
||||
return;
|
||||
}
|
||||
let expected_filter_count = inherited_seccomp_filters + 1;
|
||||
assert_eq!(
|
||||
output.stdout,
|
||||
format!("1\n2\n{expected_filter_count}\n{expected_filter_count}\n").as_bytes()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn namespace_reaper_collects_orphaned_descendants() {
|
||||
if should_skip_bwrap_tests().await {
|
||||
eprintln!("skipping bwrap test: bubblewrap is unavailable");
|
||||
return;
|
||||
}
|
||||
|
||||
let mut env = create_env_from_core_vars();
|
||||
strip_proxy_env(&mut env);
|
||||
|
||||
let output = run_linux_sandbox_direct(
|
||||
&[
|
||||
"bash",
|
||||
"-c",
|
||||
"if [ \"$(readlink /proc/1/ns/pid 2>/dev/null)\" != \"$(readlink /proc/self/ns/pid 2>/dev/null)\" ]; then printf 'namespace proc unavailable\\n'; exit 0; fi; orphan=$(bash -c 'sleep 0.05 </dev/null >/dev/null 2>&1 & printf \"%s\\n\" \"$!\"'); for _ in $(seq 1 100); do if [ ! -e \"/proc/$orphan\" ]; then printf 'orphan reaped\\n'; exit 0; fi; sleep 0.01; done; exit 1",
|
||||
],
|
||||
&PermissionProfile::read_only(),
|
||||
/*allow_network_for_proxy*/ false,
|
||||
env,
|
||||
NETWORK_TIMEOUT_MS,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
output.status.success(),
|
||||
true,
|
||||
"namespace init should reap orphaned descendants; stderr={}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
if output.stdout == b"namespace proc unavailable\n" {
|
||||
eprintln!("skipping orphan reaping check: a namespaced proc mount is unavailable");
|
||||
return;
|
||||
}
|
||||
assert_eq!(output.stdout, b"orphan reaped\n");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unsupported_system_bwrap_falls_back_to_bundled_bwrap() {
|
||||
if option_env!("CODEX_BWRAP_SHA256")
|
||||
.is_some_and(|digest| digest.chars().any(|character| character != '0'))
|
||||
{
|
||||
eprintln!("skipping system bwrap fallback test: bundled binaries require a release digest");
|
||||
return;
|
||||
}
|
||||
|
||||
if should_skip_bwrap_tests().await {
|
||||
eprintln!("skipping bwrap test: bubblewrap is unavailable");
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(system_bwrap) = codex_sandboxing::find_system_bwrap_in_path() else {
|
||||
eprintln!("skipping system bwrap fallback test: no system bubblewrap is available");
|
||||
return;
|
||||
};
|
||||
|
||||
let tempdir = tempfile::tempdir().expect("create isolated sandbox installation");
|
||||
let sandbox_executable = tempdir.path().join("codex-linux-sandbox");
|
||||
let original_executable = env!("CARGO_BIN_EXE_codex-linux-sandbox");
|
||||
if std::fs::hard_link(original_executable, &sandbox_executable).is_err() {
|
||||
std::fs::copy(original_executable, &sandbox_executable).expect("copy sandbox executable");
|
||||
}
|
||||
|
||||
let resources_dir = tempdir.path().join("codex-resources");
|
||||
std::fs::create_dir(&resources_dir).expect("create bundled resource directory");
|
||||
std::os::unix::fs::symlink(&system_bwrap, resources_dir.join("bwrap"))
|
||||
.expect("install bundled bubblewrap");
|
||||
|
||||
let system_dir = tempdir.path().join("system");
|
||||
std::fs::create_dir(&system_dir).expect("create fake system binary directory");
|
||||
let unsupported_bwrap = system_dir.join("bwrap");
|
||||
std::fs::write(
|
||||
&unsupported_bwrap,
|
||||
"#!/bin/sh\nif [ \"$1\" = \"--help\" ]; then printf '%s\\n' '--perms'; exit 0; fi\nexit 91\n",
|
||||
)
|
||||
.expect("write unsupported system bubblewrap");
|
||||
std::fs::set_permissions(&unsupported_bwrap, std::fs::Permissions::from_mode(0o755))
|
||||
.expect("make unsupported system bubblewrap executable");
|
||||
|
||||
let mut env = create_env_from_core_vars();
|
||||
strip_proxy_env(&mut env);
|
||||
let original_path = env.get("PATH").cloned().unwrap_or_default();
|
||||
env.insert(
|
||||
"PATH".to_string(),
|
||||
format!("{}:{original_path}", system_dir.display()),
|
||||
);
|
||||
|
||||
let cwd = std::env::current_dir().expect("current directory should exist");
|
||||
let permission_profile =
|
||||
serde_json::to_string(&PermissionProfile::read_only()).expect("serialize profile");
|
||||
let output = tokio::time::timeout(
|
||||
Duration::from_millis(NETWORK_TIMEOUT_MS),
|
||||
Command::new(&sandbox_executable)
|
||||
.args([
|
||||
"--sandbox-policy-cwd",
|
||||
cwd.to_str().expect("UTF-8 current directory"),
|
||||
"--permission-profile",
|
||||
&permission_profile,
|
||||
"--",
|
||||
"bash",
|
||||
"-c",
|
||||
"printf 'bundled fallback\\n'",
|
||||
])
|
||||
.current_dir(&cwd)
|
||||
.env_clear()
|
||||
.envs(env)
|
||||
.output(),
|
||||
)
|
||||
.await
|
||||
.expect("bundled bubblewrap should not time out")
|
||||
.expect("sandbox command should execute");
|
||||
|
||||
assert_eq!(
|
||||
output.status.success(),
|
||||
true,
|
||||
"unsupported system bubblewrap should fall back to bundled bubblewrap; stderr={}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_eq!(output.stdout, b"bundled fallback\n");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_proxy_full_filesystem_uses_minimal_dev_nodes() {
|
||||
if let Some(skip_reason) = managed_proxy_skip_reason().await {
|
||||
|
||||
Reference in New Issue
Block a user