test: report bwrap payload failures during teardown

bazel/rules/testing/bwrap/src/lib.rs

@@ -11,9 +11,11 @@ use std::os::fd::AsRawFd;
 use std::os::unix::fs::PermissionsExt;
 use std::os::unix::net::UnixStream;
 use std::os::unix::process::CommandExt;
+use std::os::unix::process::ExitStatusExt;
 use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command as StdCommand;
+use std::process::ExitStatus;
 use std::process::Stdio;
 use std::time::Duration;
 use std::time::Instant;
@@ -61,6 +63,12 @@ struct BwrapRuntimePaths {
     bwrap: PathBuf,
 }
 
+#[derive(Clone, Copy)]
+enum LauncherExitPolicy {
+    RequireSuccess,
+    AllowTeardownKill,
+}
+
 impl BwrapTestCommand {
     /// Creates an isolated bwrap command for `executable`.
     pub fn new(executable: impl Into<PathBuf>) -> Self {
@@ -337,14 +345,16 @@ impl BwrapProcesses {
             .sandbox_init
             .start_kill()
             .context("kill bwrap PID namespace init");
-        let (mut check_exit_status, status_check_result) = match child_status {
-            Ok(Some(_)) => (true, Ok(())),
-            Ok(None) => (false, Ok(())),
-            Err(error) => (false, Err(error).context("check bwrap process status")),
+        let (exit_policy, status_check_result) = match child_status {
+            Ok(Some(_)) => (LauncherExitPolicy::RequireSuccess, Ok(())),
+            Ok(None) => (LauncherExitPolicy::AllowTeardownKill, Ok(())),
+            Err(error) => (
+                LauncherExitPolicy::RequireSuccess,
+                Err(error).context("check bwrap process status"),
+            ),
         };
         let mut fallback_started = false;
         let early_fallback_result = if sandbox_kill_result.is_err() {
-            check_exit_status = false;
             fallback_started = true;
             self.start_kill_launcher()
         } else {
@@ -356,7 +366,6 @@ impl BwrapProcesses {
             .await
             .context("wait for bwrap PID namespace init");
         let late_fallback_result = if sandbox_wait_result.is_err() && !fallback_started {
-            check_exit_status = false;
             self.start_kill_launcher()
         } else {
             Ok(())
@@ -373,7 +382,6 @@ impl BwrapProcesses {
                     launcher_timeout_result = Err(anyhow::anyhow!(
                         "timed out waiting for bwrap launcher after {BWRAP_CLEANUP_TIMEOUT:?}"
                     ));
-                    check_exit_status = false;
                     timeout_kill_result = self.start_kill_launcher();
                     match tokio::time::timeout(BWRAP_CLEANUP_TIMEOUT, self.child.wait()).await {
                         Ok(result) => result.context("wait for killed bwrap launcher"),
@@ -383,13 +391,7 @@ impl BwrapProcesses {
                     }
                 }
             }
-            .and_then(|status| {
-                anyhow::ensure!(
-                    !check_exit_status || status.success(),
-                    "bwrap process exited with {status}"
-                );
-                Ok(())
-            });
+            .and_then(|status| ensure_launcher_exit_status(status, exit_policy));
 
         // Every cleanup action has been attempted, so an individual error
         // should not cause the blocking fallback to repeat them.
@@ -482,6 +484,18 @@ impl Drop for BwrapProcesses {
     }
 }
 
+fn ensure_launcher_exit_status(status: ExitStatus, policy: LauncherExitPolicy) -> Result<()> {
+    // bwrap normalizes a SIGKILL of its PID-namespace init to 128 + SIGKILL.
+    // Killing the launcher directly leaves it signal-terminated instead.
+    let expected_teardown_kill = matches!(policy, LauncherExitPolicy::AllowTeardownKill)
+        && (status.code() == Some(128 + libc::SIGKILL) || status.signal() == Some(libc::SIGKILL));
+    anyhow::ensure!(
+        status.success() || expected_teardown_kill,
+        "bwrap process exited with {status}"
+    );
+    Ok(())
+}
+
 fn log_panic_cleanup(args: std::fmt::Arguments<'_>) {
     let _ = writeln!(std::io::stderr().lock(), "{args}");
 }

bazel/rules/testing/bwrap/src/lib_tests.rs

@@ -5,9 +5,11 @@ use std::future::Future;
 use std::os::fd::AsRawFd;
 use std::os::fd::FromRawFd;
 use std::os::fd::OwnedFd;
+use std::os::unix::process::ExitStatusExt;
 use std::panic::AssertUnwindSafe;
 use std::path::Path;
 use std::path::PathBuf;
+use std::process::ExitStatus;
 
 use anyhow::Context;
 use anyhow::Result;
@@ -23,6 +25,8 @@ use tokio::time::timeout;
 
 use super::BwrapTestCommand;
 use super::BwrapTestProcess;
+use super::LauncherExitPolicy;
+use super::ensure_launcher_exit_status;
 
 fn smoke_fixture() -> Result<PathBuf> {
     Ok(codex_utils_cargo_bin::cargo_bin("bwrap-smoke")?)
@@ -103,6 +107,53 @@ fn namespace_identity(namespace: &str) -> Result<String> {
         .into_owned())
 }
 
+fn exit_status(code: i32) -> ExitStatus {
+    ExitStatus::from_raw(code << 8)
+}
+
+fn signal_status(signal: i32) -> ExitStatus {
+    ExitStatus::from_raw(signal)
+}
+
+#[test]
+fn launcher_exit_status_distinguishes_payload_failures_from_teardown() {
+    let actual = [
+        ensure_launcher_exit_status(exit_status(0), LauncherExitPolicy::RequireSuccess).is_ok(),
+        ensure_launcher_exit_status(exit_status(73), LauncherExitPolicy::RequireSuccess).is_ok(),
+        ensure_launcher_exit_status(
+            exit_status(128 + libc::SIGKILL),
+            LauncherExitPolicy::RequireSuccess,
+        )
+        .is_ok(),
+        ensure_launcher_exit_status(
+            signal_status(libc::SIGKILL),
+            LauncherExitPolicy::RequireSuccess,
+        )
+        .is_ok(),
+        ensure_launcher_exit_status(exit_status(0), LauncherExitPolicy::AllowTeardownKill).is_ok(),
+        ensure_launcher_exit_status(exit_status(73), LauncherExitPolicy::AllowTeardownKill).is_ok(),
+        ensure_launcher_exit_status(
+            exit_status(128 + libc::SIGKILL),
+            LauncherExitPolicy::AllowTeardownKill,
+        )
+        .is_ok(),
+        ensure_launcher_exit_status(
+            signal_status(libc::SIGKILL),
+            LauncherExitPolicy::AllowTeardownKill,
+        )
+        .is_ok(),
+        ensure_launcher_exit_status(
+            signal_status(libc::SIGTERM),
+            LauncherExitPolicy::AllowTeardownKill,
+        )
+        .is_ok(),
+    ];
+    assert_eq!(
+        actual,
+        [true, false, false, false, true, false, true, true, false]
+    );
+}
+
 #[tokio::test]
 async fn dropping_without_teardown_panics_and_cleans_up() -> Result<()> {
     let process = waiting_smoke_process().await?;