From 04abf264a0889712f20c07203be6d307afcc2f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krist=C3=B3f=20T=C3=B3th?= Date: Wed, 4 Nov 2026 01:40:37 +0100 Subject: [PATCH] Forward signals to the child processes of agent-sandbox --- src/lib.rs | 9 +- src/process_tree.rs | 2 +- src/sandbox.rs | 24 ++++-- src/signals.rs | 177 +++++++++++++++++++++++++++++++++++++++- tests/e2e/forwarding.rs | 150 ++++++++++++++++++++++++++++++++++ tests/e2e/main.rs | 1 + 6 files changed, 352 insertions(+), 11 deletions(-) create mode 100644 tests/e2e/forwarding.rs diff --git a/src/lib.rs b/src/lib.rs index 8dfee5f..b8ea26c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ use std::process::ExitStatus; use std::time::Duration; use crate::sandbox::Bwrap; +use crate::signals::ForwardedSignals; const CLEANUP_GRACE_PERIOD: Duration = Duration::from_secs(5); @@ -100,7 +101,8 @@ fn resolve_run_user_from_proc() -> Option { pub fn run(config: SandboxConfig) -> Result { preflight::check(&config)?; - let bwrap = Bwrap::build(&config)?; + let forwarded_signals = ForwardedSignals::detect(); + let mut bwrap = Bwrap::build(&config, &forwarded_signals.build_signal_reset_prefix())?; if config.dry_run { println!("{}", bwrap.shell_quoted()); @@ -108,7 +110,10 @@ pub fn run(config: SandboxConfig) -> Result { } process_tree::become_subreaper()?; - let status = bwrap.run()?; + let signal_forwarder = forwarded_signals.capture()?; + // bwrap ignores them: its death would SIGKILL the entrypoint through --die-with-parent. + let bwrap_process = bwrap.spawn(signal_forwarder.ignore_forwarded_signals())?; + let status = signal_forwarder.forward_until_exit(&bwrap_process)?; process_tree::terminate_and_reap_all(CLEANUP_GRACE_PERIOD)?; Ok(status) } diff --git a/src/process_tree.rs b/src/process_tree.rs index 2764394..ebf68f9 100644 --- a/src/process_tree.rs +++ b/src/process_tree.rs @@ -28,7 +28,7 @@ pub fn terminate_and_reap_all(grace_period: Duration) -> io::Result<()> { // Not exact: a process that forks during the last pass can miss the signals. // That is good enough, because every survivor receives SIGKILL after the grace period. -fn signal_all_children(signals: &[Signal]) { +pub fn signal_all_children(signals: &[Signal]) { let mut signaled = HashSet::new(); for _ in 0..MAX_WALK_PASSES { let root = process::getpid(); diff --git a/src/sandbox.rs b/src/sandbox.rs index b93cb67..770d103 100644 --- a/src/sandbox.rs +++ b/src/sandbox.rs @@ -1,6 +1,7 @@ use std::io; +use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; -use std::process::{Command, ExitStatus}; +use std::process::{Child, Command}; use crate::agents; use crate::blacklist; @@ -16,14 +17,21 @@ pub struct Bwrap { } impl Bwrap { - pub fn build(config: &SandboxConfig) -> Result { - let (bwrap, command_line) = build_command(config)?; + pub fn build( + config: &SandboxConfig, + entrypoint_prefix: &[String], + ) -> Result { + let (bwrap, command_line) = build_command(config, entrypoint_prefix)?; command_line.fill(&bwrap.shell_quoted())?; Ok(bwrap) } - pub fn run(mut self) -> io::Result { - self.command.status() + pub fn spawn( + &mut self, + before_exec: impl FnMut() -> io::Result<()> + Send + Sync + 'static, + ) -> io::Result { + unsafe { self.command.pre_exec(before_exec) }; + self.command.spawn() } pub fn shell_quoted(&self) -> String { @@ -34,7 +42,10 @@ impl Bwrap { } } -fn build_command(config: &SandboxConfig) -> Result<(Bwrap, BwrapCommandLine), SandboxError> { +fn build_command( + config: &SandboxConfig, + entrypoint_prefix: &[String], +) -> Result<(Bwrap, BwrapCommandLine), SandboxError> { let mut cmd = Command::new("bwrap"); let hardened = config.hardened || matches!(config.mode, SandboxMode::Whitelist); @@ -85,6 +96,7 @@ fn build_command(config: &SandboxConfig) -> Result<(Bwrap, BwrapCommandLine), Sa cmd.args(&config.bwrap_args); cmd.arg("--") + .args(entrypoint_prefix) .arg(&config.command) .args(&config.command_args); diff --git a/src/signals.rs b/src/signals.rs index 51820ba..dea3e13 100644 --- a/src/signals.rs +++ b/src/signals.rs @@ -1,8 +1,181 @@ +use std::io; +use std::mem::MaybeUninit; use std::os::unix::process::ExitStatusExt; -use std::process::{self, ExitStatus}; +use std::process::{self, Child, ExitStatus}; +use std::ptr; use nix::sys::prctl; -use nix::sys::signal::{self, SigHandler, SigSet, Signal}; +use nix::sys::signal::{self, SigHandler, SigSet, SigmaskHow, Signal}; +use nix::sys::signalfd::{SfdFlags, SignalFd}; +use rustix::io::Errno; +use rustix::process::{Pid, WaitOptions}; + +use crate::process_tree; + +const STANDARD_FORWARDED: [i32; 15] = [ + libc::SIGHUP, + libc::SIGINT, + libc::SIGQUIT, + libc::SIGTERM, + libc::SIGUSR1, + libc::SIGUSR2, + libc::SIGALRM, + libc::SIGPIPE, + libc::SIGVTALRM, + libc::SIGPROF, + libc::SIGIO, + libc::SIGPWR, + libc::SIGSTKFLT, + libc::SIGXCPU, + libc::SIGXFSZ, +]; + +pub struct ForwardedSignals { + numbers: Vec, +} + +impl ForwardedSignals { + pub fn detect() -> Self { + let numbers = STANDARD_FORWARDED + .into_iter() + .chain(libc::SIGRTMIN()..=libc::SIGRTMAX()) + .filter(|&number| !is_ignored_by_parent(number)) + .collect(); + Self { numbers } + } + + pub fn build_signal_reset_prefix(&self) -> Vec { + let numbers: Vec = self.numbers.iter().map(i32::to_string).collect(); + vec![ + "env".to_string(), + format!("--default-signal={}", numbers.join(",")), + "--".to_string(), + ] + } + + pub fn capture(self) -> io::Result { + unsafe { signal::signal(Signal::SIGCHLD, SigHandler::SigDfl) }?; + + let mask = to_sigset(self.numbers.iter().copied().chain([libc::SIGCHLD])); + let original_mask = mask.thread_swap_mask(SigmaskHow::SIG_BLOCK)?; + let fd = SignalFd::with_flags(&mask, SfdFlags::SFD_CLOEXEC)?; + + Ok(SignalForwarder { + forwarded_signals: self.numbers, + original_mask, + fd, + received_signals: Vec::new(), + }) + } +} + +// The Rust runtime ignores SIGPIPE itself, so the inherited disposition is unknown. +fn is_ignored_by_parent(number: i32) -> bool { + number != libc::SIGPIPE && is_ignored(number) +} + +fn is_ignored(number: i32) -> bool { + let mut action = MaybeUninit::::zeroed(); + let result = unsafe { libc::sigaction(number, ptr::null(), action.as_mut_ptr()) }; + result == 0 && unsafe { action.assume_init() }.sa_sigaction == libc::SIG_IGN +} + +fn to_sigset(numbers: impl IntoIterator) -> SigSet { + let mut raw = MaybeUninit::::uninit(); + unsafe { + libc::sigemptyset(raw.as_mut_ptr()); + for number in numbers { + libc::sigaddset(raw.as_mut_ptr(), number); + } + SigSet::from_sigset_t_unchecked(raw.assume_init()) + } +} + +pub struct SignalForwarder { + forwarded_signals: Vec, + original_mask: SigSet, + fd: SignalFd, + received_signals: Vec, +} + +impl SignalForwarder { + pub fn ignore_forwarded_signals( + &self, + ) -> impl FnMut() -> io::Result<()> + Send + Sync + 'static { + let forwarded_signals = self.forwarded_signals.clone(); + let original_mask = self.original_mask; + move || { + for &number in &forwarded_signals { + unsafe { libc::signal(number, libc::SIG_IGN) }; + } + Ok(signal::sigprocmask( + SigmaskHow::SIG_SETMASK, + Some(&original_mask), + None, + )?) + } + } + + pub fn forward_until_exit(mut self, child: &Child) -> io::Result { + let child = Pid::from_child(child); + loop { + let Some(info) = self.fd.read_signal()? else { + continue; + }; + + let number = info.ssi_signo as i32; + if number != libc::SIGCHLD { + self.forward(number, info.ssi_code); + } else if let Some(status) = reap_until_exit_of(child)? { + return Ok(self.mirror(status)); + } + } + } + + fn forward(&mut self, number: i32, code: i32) { + self.received_signals.push(number); + + if matches!(code, libc::SI_USER | libc::SI_QUEUE | libc::SI_TKILL) { + process_tree::signal_all_children(&[to_process_signal(number)]); + } else if number == libc::SIGHUP && is_session_leader() { + process_tree::signal_all_children(&[ + rustix::process::Signal::HUP, + rustix::process::Signal::CONT, + ]); + } + } + + fn mirror(&self, status: ExitStatus) -> ExitStatus { + match status.code() { + Some(code) if self.received_signals.contains(&(code - 128)) => { + ExitStatus::from_raw(code - 128) + } + _ => status, + } + } +} + +fn to_process_signal(number: i32) -> rustix::process::Signal { + unsafe { rustix::process::Signal::from_raw_unchecked(number) } +} + +fn is_session_leader() -> bool { + rustix::process::getsid(None).is_ok_and(|sid| sid == rustix::process::getpid()) +} + +fn reap_until_exit_of(child: Pid) -> io::Result> { + let mut child_status = None; + loop { + match rustix::process::wait(WaitOptions::NOHANG) { + Ok(Some((pid, status))) if pid == child => { + child_status = Some(ExitStatus::from_raw(status.as_raw())); + } + Ok(Some(_)) => {} + Ok(None) | Err(Errno::CHILD) => return Ok(child_status), + Err(e) => return Err(e.into()), + } + } +} pub fn exit_with_status(status: ExitStatus) -> ! { match status.signal() { diff --git a/tests/e2e/forwarding.rs b/tests/e2e/forwarding.rs new file mode 100644 index 0000000..5c4651a --- /dev/null +++ b/tests/e2e/forwarding.rs @@ -0,0 +1,150 @@ +use std::os::unix::process::{CommandExt, ExitStatusExt}; +use std::path::Path; +use std::process::{Child, Stdio}; +use std::thread; +use std::time::{Duration, Instant}; + +use tempfile::TempDir; + +use crate::common::*; + +#[test] +fn sigterm_lets_entrypoint_clean_up_in_blacklist_mode() { + assert_sigterm_lets_entrypoint_clean_up("--blacklist"); +} + +#[test] +fn sigterm_lets_entrypoint_clean_up_in_whitelist_mode() { + assert_sigterm_lets_entrypoint_clean_up("--whitelist"); +} + +fn assert_sigterm_lets_entrypoint_clean_up(mode: &str) { + let dir = TempDir::new().expect("failed to create temp dir"); + let script = "trap 'sleep 0.5; touch cleaned; exit 0' TERM + touch ready + sleep 60 & wait"; + + let (_sandbox, mut child) = spawn_sandbox(&[mode], dir.path(), script); + wait_for_file(&dir.path().join("ready")); + send_signal(&child, libc::SIGTERM); + let status = child.wait().unwrap(); + + assert!( + dir.path().join("cleaned").exists(), + "cleanup did not finish" + ); + assert_eq!(status.code(), Some(0)); +} + +#[test] +fn forwarded_signal_death_of_entrypoint_is_mirrored() { + let dir = TempDir::new().expect("failed to create temp dir"); + let script = "touch ready + sleep 60 & wait"; + + let (_sandbox, mut child) = spawn_sandbox(&["--blacklist"], dir.path(), script); + wait_for_file(&dir.path().join("ready")); + send_signal(&child, libc::SIGTERM); + let status = child.wait().unwrap(); + + assert_eq!(status.signal(), Some(libc::SIGTERM)); +} + +#[test] +fn user_and_realtime_signals_reach_entrypoint() { + let dir = TempDir::new().expect("failed to create temp dir"); + let realtime = libc::SIGRTMIN() + 1; + let script = format!( + "trap 'touch usr1' USR1 + trap 'touch realtime' {realtime} + trap 'exit 0' TERM + touch ready + while :; do sleep 0.05; done" + ); + + let (_sandbox, mut child) = spawn_sandbox(&["--blacklist"], dir.path(), &script); + wait_for_file(&dir.path().join("ready")); + send_signal(&child, libc::SIGUSR1); + wait_for_file(&dir.path().join("usr1")); + send_signal(&child, realtime); + wait_for_file(&dir.path().join("realtime")); + send_signal(&child, libc::SIGTERM); + + assert_eq!(child.wait().unwrap().code(), Some(0)); +} + +#[test] +fn entrypoint_starts_with_default_dispositions_except_inherited_ignores() { + let mut sandbox = Sandbox::new(&["--blacklist"]); + sandbox.args(["--", "cat", "/proc/self/status"]); + unsafe { + sandbox.pre_exec(|| { + libc::signal(libc::SIGHUP, libc::SIG_IGN); + Ok(()) + }); + } + + let output = sandbox + .output() + .expect("agent-sandbox binary failed to execute"); + let status = String::from_utf8_lossy(&output.stdout); + let ignored = read_signal_mask(&status, "SigIgn:"); + let blocked = read_signal_mask(&status, "SigBlk:"); + + assert!( + has_signal(ignored, libc::SIGHUP), + "inherited SIGHUP ignore was lost" + ); + for signal in [libc::SIGINT, libc::SIGTERM, libc::SIGUSR1, libc::SIGPIPE] { + assert!(!has_signal(ignored, signal), "signal {signal} is ignored"); + } + assert_eq!(blocked, 0, "entrypoint starts with blocked signals"); +} + +#[test] +fn entrypoint_inherits_no_signalfd() { + let output = Sandbox::new(&["--blacklist"]) + .args(["--", "ls", "-l", "/proc/self/fd"]) + .output() + .expect("agent-sandbox binary failed to execute"); + let fds = String::from_utf8_lossy(&output.stdout); + + assert!( + !fds.contains("signalfd"), + "entrypoint inherits a signalfd: {fds}" + ); +} + +fn spawn_sandbox(args: &[&str], dir: &Path, script: &str) -> (Sandbox, Child) { + let mut sandbox = Sandbox::new(args); + let child = sandbox + .args(["--chdir", dir.to_str().unwrap(), "--", "sh", "-c", script]) + .stdout(Stdio::null()) + .spawn() + .expect("agent-sandbox binary failed to execute"); + (sandbox, child) +} + +fn wait_for_file(path: &Path) { + let deadline = Instant::now() + Duration::from_secs(10); + while !path.exists() { + assert!(Instant::now() < deadline, "timed out waiting for {path:?}"); + thread::sleep(Duration::from_millis(10)); + } +} + +fn send_signal(child: &Child, signal: i32) { + unsafe { libc::kill(child.id() as i32, signal) }; +} + +fn read_signal_mask(proc_status: &str, field: &str) -> u64 { + let line = proc_status + .lines() + .find_map(|line| line.strip_prefix(field)) + .unwrap_or_else(|| panic!("{field} missing in {proc_status}")); + u64::from_str_radix(line.trim(), 16).unwrap() +} + +fn has_signal(mask: u64, signal: i32) -> bool { + mask & (1 << (signal - 1)) != 0 +} diff --git a/tests/e2e/main.rs b/tests/e2e/main.rs index d656e76..765e6bb 100644 --- a/tests/e2e/main.rs +++ b/tests/e2e/main.rs @@ -2,6 +2,7 @@ mod common; mod cli; mod env; +mod forwarding; mod modes; mod mounts; mod namespaces;