test(indirect): enforce real bounds in the hostile-cycle harness

crates/bashkit/tests/integration/threat_model_tests.rs

@@ -9,6 +9,7 @@ use bashkit::{
     Bash, ExecutionLimits, FileSystem, FileSystemExt, FsLimits, InMemoryFs, MemoryLimits,
     OverlayFs, SessionLimits, TraceEventDetails, TraceEventKind, TraceMode,
 };
+use std::io::Read;
 use std::path::Path;
 use std::process::{Command, Stdio};
 use std::sync::Arc;
@@ -34,45 +35,184 @@ mod resource_exhaustion {
 
     struct BoundedRun {
         timed_out: bool,
+        oversized: bool,
+        /// True when the deadline passed with readers unfinished: a foreign
+        /// descendant retains the pipes. Detection, not reaping: without
+        /// platform process-group APIs (no new dependencies) a foreign
+        /// descendant cannot be reaped portably, so the run fails visibly.
+        incomplete_readers: bool,
+        /// First stderr line from the launcher (`harness-limits: ...`),
+        /// proving the CPU/stack caps were applied to this run. Empty when
+        /// the launcher itself refused (fail-closed exit 99).
+        limits_note: String,
         exit_code: Option<i32>,
         stdout: String,
         stderr: String,
     }
 
-    /// Runs a script in a child CLI process under a wall-clock watchdog.
-    /// Darwin note: Rust std exposes no portable per-child CPU, stack, or
-    /// memory caps, so the watchdog kill plus the 8 KiB output cap are the
-    /// enforced bounds here. Records the real producer exit status.
+    /// Fail-closed launcher limits: 10 CPU seconds, 4 MiB main-thread
+    /// stack. Any refusal exits 99 before the producer starts, so a run
+    /// without limits can never be mistaken for a bounded one.
+    const BOUNDED_CLI_LIMITS: &str = "ulimit -s 4096 || exit 99; ulimit -t 10 || exit 99";
+
+    /// Per-pipe sample cap. Past the cap the reader stops draining and the
+    /// run is marked `oversized`, which fails the harness: oversize output
+    /// is never silently truncated.
+    const BOUNDED_SAMPLE_CAP: usize = 8192;
+
+    /// Reads one pipe to the sample cap on its own thread (concurrent with
+    /// the sibling pipe, so a chatty child can never deadlock the runner),
+    /// then reports its bytes. Past the cap it stops draining: the child
+    /// blocks on the full pipe until the poll loop kills it.
+    fn spawn_capped_reader(
+        id: u8,
+        pipe: Box<dyn std::io::Read + Send>,
+        tx: std::sync::mpsc::Sender<(u8, Vec<u8>, bool)>,
+    ) -> std::thread::JoinHandle<()> {
+        std::thread::spawn(move || {
+            let mut pipe = pipe;
+            let mut buf = Vec::new();
+            let mut chunk = [0u8; 4096];
+            let mut oversized = false;
+            loop {
+                match pipe.read(&mut chunk) {
+                    Ok(0) => break,
+                    Ok(n) => {
+                        buf.extend_from_slice(&chunk[..n]);
+                        if buf.len() > BOUNDED_SAMPLE_CAP {
+                            oversized = true;
+                            break;
+                        }
+                    }
+                    Err(_) => break,
+                }
+            }
+            let _ = tx.send((id, buf, oversized));
+        })
+    }
+
+    /// Runs a script in the built CLI under `run_command_bounded`.
+    /// The launcher applies the fail-closed CPU/stack caps, reports the
+    /// effective values as the first stderr line, then `exec`s the CLI so
+    /// exit codes and diagnostics stay the real producer's own.
     fn run_cli_bounded(script: &str, limit: Duration) -> BoundedRun {
-        let mut child = Command::new(bashkit_cli_path())
+        let launcher = format!(
+            "{BOUNDED_CLI_LIMITS}; echo \"harness-limits: stack=$(ulimit -s) cpu=$(ulimit -t)\" >&2; exec \"$@\""
+        );
+        let mut cmd = Command::new("bash");
+        cmd.args(["-c", &launcher, "_"])
+            .arg(bashkit_cli_path())
             .args(["-c", script])
             .stdin(Stdio::null())
             .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
-            .expect("spawn bashkit CLI");
-        let start = Instant::now();
-        let timed_out = loop {
-            match child.try_wait().expect("poll child") {
-                Some(_) => break false,
-                None => {
-                    if start.elapsed() >= limit {
-                        let _ = child.kill();
-                        break true;
+            .stderr(Stdio::piped());
+        let mut run = run_command_bounded(cmd, limit);
+        (run.limits_note, run.stderr) = match run.stderr.split_once('\n') {
+            Some((first, rest)) if first.starts_with("harness-limits:") => {
+                (first.to_string(), rest.to_string())
+            }
+            _ => (String::new(), run.stderr),
+        };
+        run
+    }
+
+    /// Runs an already-built command with one shared wall deadline for
+    /// child exit, pipe drain, and reaping:
+    /// - wall clock: past `limit` the child is killed; if it had already
+    ///   exited, the run is not marked timed out;
+    /// - output: one reader thread per pipe stops past the sample cap, the
+    ///   child is killed, and the run is marked `oversized` (a harness
+    ///   failure, never a silent cut). Readers run concurrently, so a
+    ///   chatty child can never deadlock the runner on a full pipe;
+    /// - drain: after the break, missing readers get a short grace period.
+    ///   Anything still missing means a foreign descendant retains the
+    ///   pipe: the run is marked `incomplete_readers` and fails visibly
+    ///   instead of waiting forever. Reader threads for retained pipes are
+    ///   dropped, not joined without a deadline (one documented thread
+    ///   leak per such run, only in the retention self-test).
+    /// - CPU/stack caps are the launcher's job (see above); Rust std
+    ///   exposes no portable per-child CPU/stack/memory caps, and Darwin
+    ///   does not enforce address-space caps: documented here, not claimed.
+    fn run_command_bounded(mut cmd: Command, limit: Duration) -> BoundedRun {
+        let mut child = cmd.spawn().expect("spawn bounded child");
+        let (tx, rx) = std::sync::mpsc::channel();
+        let mut readers = Vec::new();
+        if let Some(pipe) = child.stdout.take() {
+            readers.push(spawn_capped_reader(0, Box::new(pipe), tx.clone()));
+        }
+        if let Some(pipe) = child.stderr.take() {
+            readers.push(spawn_capped_reader(1, Box::new(pipe), tx.clone()));
+        }
+        drop(tx);
+        let mut bufs: [Option<Vec<u8>>; 2] = [None, None];
+        let mut oversized = false;
+        let mut status = None;
+        let mut timed_out = false;
+        let deadline = Instant::now() + limit;
+        // Pipe EOF alone never ends the run: only an observed exit does,
+        // otherwise a child that closes output and keeps running would
+        // bypass the watchdog into an unbounded wait. The deadline verdict
+        // is recorded from pre-kill observed liveness and never recomputed
+        // after killing: reaping the SIGKILL victim must not flip a genuine
+        // timeout to false on scheduling luck.
+        loop {
+            while let Ok((id, buf, over)) = rx.try_recv() {
+                bufs[id as usize] = Some(buf);
+                oversized |= over;
+            }
+            if bufs[0].is_some() && bufs[1].is_some() && status.is_some() {
+                break;
+            }
+            if oversized {
+                // `kill` is idempotent; errors (already exited) are ignored.
+                let _ = child.kill();
+            }
+            if status.is_none() {
+                status = child.try_wait().expect("poll child");
+            }
+            if Instant::now() >= deadline {
+                if status.is_none() && !oversized {
+                    let _ = child.kill();
+                    timed_out = true;
+                }
+                break;
+            }
+            std::thread::sleep(Duration::from_millis(10));
+        }
+        // Bounded drain grace: an exited child with unretained pipes hits
+        // EOF promptly; anything still missing is a retained pipe.
+        let grace_end = Instant::now() + Duration::from_secs(2);
+        while (bufs[0].is_none() || bufs[1].is_none()) && Instant::now() < grace_end {
+            match rx.recv_timeout(Duration::from_millis(50)) {
+                Ok((id, buf, over)) => {
+                    bufs[id as usize] = Some(buf);
+                    oversized |= over;
+                }
+                Err(_) => {
+                    if bufs[0].is_some() && bufs[1].is_some() {
+                        break;
                     }
-                    std::thread::sleep(Duration::from_millis(10));
                 }
             }
-        };
-        let output = child.wait_with_output().expect("collect child output");
-        const CAP: usize = 8192;
-        let mut stdout = String::from_utf8_lossy(&output.stdout).into_owned();
-        let mut stderr = String::from_utf8_lossy(&output.stderr).into_owned();
-        stdout.truncate(CAP);
-        stderr.truncate(CAP);
+        }
+        let incomplete_readers = bufs[0].is_none() || bufs[1].is_none();
+        if !incomplete_readers {
+            for reader in readers {
+                let _ = reader.join();
+            }
+        }
+        // Always reaped explicitly: `try_wait` may already have seen the
+        // exit, in which case this returns immediately with the same status.
+        let reaped = child.wait().expect("reap child");
+        let status = Some(status.unwrap_or(reaped));
+        let stdout = String::from_utf8_lossy(bufs[0].as_deref().unwrap_or(&[])).into_owned();
+        let stderr = String::from_utf8_lossy(bufs[1].as_deref().unwrap_or(&[])).into_owned();
         BoundedRun {
             timed_out,
-            exit_code: output.status.code(),
+            oversized,
+            incomplete_readers,
+            limits_note: String::new(),
+            exit_code: status.and_then(|s| s.code()),
             stdout,
             stderr,
         }
@@ -92,6 +232,16 @@ mod resource_exhaustion {
         ] {
             let run = run_cli_bounded(script, Duration::from_secs(20));
             assert!(!run.timed_out, "hostile script hung: {script}");
+            assert!(!run.oversized, "hostile script oversized: {script}");
+            assert!(
+                !run.incomplete_readers,
+                "hostile script readers incomplete: {script}"
+            );
+            assert!(
+                run.limits_note.contains("stack=4096") && run.limits_note.contains("cpu=10"),
+                "limits not applied: {}",
+                run.limits_note
+            );
             assert_eq!(
                 run.exit_code,
                 Some(1),
@@ -109,6 +259,119 @@ mod resource_exhaustion {
         }
     }
 
+    /// The bounded runner itself is tested: a hung child must be
+    /// reported as a timeout, and an overproducer as oversize. Both fail
+    /// visibly (a boolean the harness asserts on), so a helper regression
+    /// cannot silently pass hostile coverage. The short-deadline case runs
+    /// repeatedly: the timeout verdict must be deterministic, never a
+    /// scheduling coin flip.
+    #[test]
+    fn bounded_helper_kills_hung_child() {
+        let run = run_cli_bounded("sleep 30", Duration::from_secs(3));
+        assert!(
+            run.timed_out,
+            "hung child not reported: {:?}",
+            run.exit_code
+        );
+        assert!(!run.oversized, "hung child misreported as oversize");
+        assert!(
+            !run.incomplete_readers,
+            "hung child misreported as incomplete"
+        );
+        // Repeated short deadlines: every run must report the timeout.
+        for _ in 0..3 {
+            let run = run_cli_bounded("sleep 30", Duration::from_secs(2));
+            assert!(run.timed_out, "short deadline flaked");
+            assert!(!run.oversized, "short deadline misreported as oversize");
+        }
+    }
+
+    /// The launcher is fail-closed: if `ulimit` refuses, the producer never
+    /// starts (exit 99) instead of running unbounded. A shadowed failing
+    /// `ulimit` exercises the refusal with a harmless instant child.
+    #[test]
+    fn bounded_launcher_failure_is_fail_closed() {
+        let probe = [
+            "ulimit() { return 1; }; ",
+            BOUNDED_CLI_LIMITS,
+            "; echo SURVIVED",
+        ]
+        .concat();
+        let out = Command::new("bash")
+            .args(["-c", &probe])
+            .stdin(Stdio::null())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()
+            .expect("spawn launcher probe")
+            .wait_with_output()
+            .expect("reap launcher probe");
+        assert_eq!(
+            out.status.code(),
+            Some(99),
+            "stderr: {}",
+            String::from_utf8_lossy(&out.stderr)
+        );
+        assert!(
+            !String::from_utf8_lossy(&out.stdout).contains("SURVIVED"),
+            "launcher ran the producer without limits"
+        );
+    }
+
+    /// Both pipes hit EOF while the producer lives: only the wall deadline
+    /// may end the run. The previous loop broke on complete buffers and
+    /// then waited without a deadline; this case hung it forever.
+    #[test]
+    fn bounded_helper_covers_early_pipe_close() {
+        let run = run_cli_bounded("echo hi; exec 1>&- 2>&-; sleep 30", Duration::from_secs(5));
+        assert!(run.timed_out, "live producer with closed pipes not killed");
+        assert!(!run.oversized, "early close misreported as oversize");
+        assert!(
+            !run.incomplete_readers,
+            "closed pipes misreported as incomplete"
+        );
+        assert!(run.stdout.contains("hi"), "stdout: {}", run.stdout);
+        assert_eq!(run.exit_code, None, "expected SIGKILL, no exit code");
+    }
+
+    /// An exited child with descendant-retained pipes: the exit is observed
+    /// but readers never finish. The helper must report incomplete readers
+    /// within bounds, not wait forever. Real bash (not the CLI) is the
+    /// pipe holder here; the orphaned `sleep` is harmless and bounded.
+    #[test]
+    fn bounded_helper_flags_retained_pipes() {
+        let mut cmd = Command::new("bash");
+        cmd.args(["-c", "sleep 30 & exit 0"])
+            .stdin(Stdio::null())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped());
+        let run = run_command_bounded(cmd, Duration::from_secs(5));
+        assert!(!run.timed_out, "exited child misreported as timeout");
+        assert!(
+            run.incomplete_readers,
+            "retained pipes not reported as incomplete"
+        );
+        assert_eq!(run.exit_code, Some(0), "stdout: {}", run.stdout);
+    }
+
+    #[test]
+    fn bounded_helper_flags_oversize_output() {
+        let script =
+            "i=0; while ((i < 4000)); do echo \"padding line $i padding\"; i=$((i + 1)); done";
+        let run = run_cli_bounded(script, Duration::from_secs(20));
+        assert!(!run.timed_out, "overproducer hung");
+        assert!(run.oversized, "overproducer not flagged as oversize");
+        assert!(
+            !run.incomplete_readers,
+            "overproducer misreported as incomplete"
+        );
+        assert!(
+            run.stdout.len() <= BOUNDED_SAMPLE_CAP + 4096,
+            "sample not capped: {}",
+            run.stdout.len()
+        );
+    }
+
     /// TM-DOS-135: a bracket indirect target is refused unless the first
     /// complete `[...]` reference consumes the whole value. Modern bash
     /// (5.x) rejects trailing operators with `invalid variable name`;