Forward signals to the child processes of agent-sandbox

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<String> {
 pub fn run(config: SandboxConfig) -> Result<ExitStatus, SandboxError> {
     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<ExitStatus, SandboxError> {
     }
 
     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)
 }

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();

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<Self, SandboxError> {
+    pub fn build(
-        let (bwrap, command_line) = build_command(config)?;
+        config: &SandboxConfig,
+        entrypoint_prefix: &[String],
+    ) -> Result<Self, SandboxError> {
+        let (bwrap, command_line) = build_command(config, entrypoint_prefix)?;
         command_line.fill(&bwrap.shell_quoted())?;
         Ok(bwrap)
     }
 
-    pub fn run(mut self) -> io::Result<ExitStatus> {
+    pub fn spawn(
-        self.command.status()
+        &mut self,
+        before_exec: impl FnMut() -> io::Result<()> + Send + Sync + 'static,
+    ) -> io::Result<Child> {
+        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);
 

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<i32>,
+}
+
+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<String> {
+        let numbers: Vec<String> = 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<SignalForwarder> {
+        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::<libc::sigaction>::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<Item = i32>) -> SigSet {
+    let mut raw = MaybeUninit::<libc::sigset_t>::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<i32>,
+    original_mask: SigSet,
+    fd: SignalFd,
+    received_signals: Vec<i32>,
+}
+
+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<ExitStatus> {
+        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<Option<ExitStatus>> {
+    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() {

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
+}

tests/e2e/main.rs

@@ -2,6 +2,7 @@ mod common;
 
 mod cli;
 mod env;
+mod forwarding;
 mod modes;
 mod mounts;
 mod namespaces;