diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index ad130587a..d4ebc6f94 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -26,6 +26,10 @@ const BIN: &str = env!("CARGO_BIN_EXE_bashkit"); fn cli() -> Command { let mut cmd = Command::new(BIN); cmd.env_remove("RUST_BACKTRACE"); + // This binary is instrumented under coverage and several tests below kill + // it mid-run; a SIGKILLed child leaves a truncated profraw, which fails + // tarpaulin's merge step. See `bashkit::testing::child_profile_path`. + cmd.env("LLVM_PROFILE_FILE", bashkit::testing::child_profile_path()); cmd } @@ -314,6 +318,9 @@ fn run_bounded_child(argv: &[&str], limits: &ProbeLimits) -> ProbeRun { cmd.args(&argv[1..]); let mut child = cmd .env_remove("RUST_BACKTRACE") + // Killed by the bound below when it overruns; keep its truncated + // profraw out of tarpaulin's merge set. + .env("LLVM_PROFILE_FILE", bashkit::testing::child_profile_path()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() @@ -1508,7 +1515,9 @@ fn streamed_output_is_not_printed_twice() { fn output_without_a_newline_arrives_before_the_script_is_released() { let dir = tempfile::tempdir().unwrap(); let mount = format!("{}:/gate", dir.path().display()); - let mut child = Command::new(BIN) + // Through `cli()`: it carries the backtrace scrub and the coverage + // profile redirect every spawn of this binary needs. + let mut child = cli() .args([ "--no-stdin", "--timeout", @@ -1518,7 +1527,6 @@ fn output_without_a_newline_arrives_before_the_script_is_released() { "-c", "printf first; while [ ! -e /gate/release ]; do sleep 0.01; done; printf last", ]) - .env_remove("RUST_BACKTRACE") .stdout(Stdio::piped()) .stderr(Stdio::null()) .spawn() diff --git a/crates/bashkit/src/testing.rs b/crates/bashkit/src/testing.rs index 341ff890f..3d30c1d3d 100644 --- a/crates/bashkit/src/testing.rs +++ b/crates/bashkit/src/testing.rs @@ -81,6 +81,48 @@ pub fn fuzz_init() { }); } +/// A throwaway `LLVM_PROFILE_FILE` path for a spawned child, outside the +/// directory `cargo tarpaulin` merges. +/// +/// Any test that spawns the instrumented `bashkit` binary must set this on +/// the child. A child inherits the *parent's* profile-path template, so it +/// writes a profraw named after the parent test binary into +/// `target/tarpaulin/profraws/`, which tarpaulin then merges. Supervised +/// children are killed by design, and a SIGKILLed process never flushes its +/// profraw, so the merge step panicked inside `llvm_profparser` +/// (`unreachable!("llvm_profparsers works on complete data")`) and failed the +/// Coverage job at random — main runs 4588 and 4600, two days apart on +/// unrelated commits. Redirecting the child keeps truncated writes out of the +/// merge set, and stops the child's counters being misattributed to the +/// parent binary. +/// +/// One directory per process, a unique file per call, removal left to the OS. +/// Deliberately not a cleanup-guarded temp dir: a killed child's foreign +/// descendant can still be writing here, so a per-call remove could fail for +/// reasons that have nothing to do with the test using it. +/// +/// A test that needs to *assert* on the child's profraw (as a bounded-artifact +/// receipt, say) should set `LLVM_PROFILE_FILE` to its own path instead; the +/// point is only that the child never writes into tarpaulin's merge set. +pub fn child_profile_path() -> std::path::PathBuf { + use std::sync::OnceLock; + use std::sync::atomic::{AtomicU64, Ordering}; + static DIR: OnceLock = OnceLock::new(); + static NEXT: AtomicU64 = AtomicU64::new(0); + let dir = DIR.get_or_init(|| { + let dir = + std::env::temp_dir().join(format!("bashkit-child-profraws-{}", std::process::id())); + // Best-effort: if the directory cannot be created the child's write + // simply fails, which is the outcome this redirect wants anyway. + let _ = std::fs::create_dir_all(&dir); + dir + }); + dir.join(format!( + "child-{}.profraw", + NEXT.fetch_add(1, Ordering::Relaxed) + )) +} + /// Execute a script under a fresh `Bash` and capture the result. /// Both `Ok(ExecResult)` and the hard `Err(execution error)` paths are /// normalized into `ExecResult` so callers don't have to branch. @@ -401,6 +443,27 @@ pub fn check_cpython_http_request(data: &[u8]) { mod tests { use super::*; + #[test] + fn child_profile_paths_are_unique_and_outside_the_merge_set() { + let a = child_profile_path(); + let b = child_profile_path(); + assert_ne!(a, b, "each child needs its own profraw path"); + for p in [&a, &b] { + let text = p.to_string_lossy(); + // The whole point: never inside the directory tarpaulin merges. + assert!( + !text.contains("tarpaulin"), + "child profraw must not land in tarpaulin's merge set: {text}" + ); + assert!(text.ends_with(".profraw"), "{text}"); + } + assert_eq!(a.parent(), b.parent(), "one directory per process"); + assert!( + a.parent().expect("parent dir").is_dir(), + "the sink directory is created up front" + ); + } + #[test] fn canary_in_the_input_is_counted_as_an_echo() { // Run 272: libFuzzer learned the canary from the detector's own diff --git a/crates/bashkit/tests/integration/bounded_child_launcher_scan_tests.rs b/crates/bashkit/tests/integration/bounded_child_launcher_scan_tests.rs index a663904f0..58f940889 100644 --- a/crates/bashkit/tests/integration/bounded_child_launcher_scan_tests.rs +++ b/crates/bashkit/tests/integration/bounded_child_launcher_scan_tests.rs @@ -87,3 +87,70 @@ fn bounded_child_launchers_never_cap_address_space() { offenders.join("\n") ); } + +/// Every test file that spawns the instrumented `bashkit` binary redirects +/// the child's coverage profile away from tarpaulin's merge set. +/// +/// A child inherits the parent's `LLVM_PROFILE_FILE` template, so it writes a +/// profraw named after the *parent* test binary into +/// `target/tarpaulin/profraws/`. Supervised children are killed by design and +/// a SIGKILLed process never flushes its profraw, so tarpaulin's merge step +/// panics inside `llvm_profparser` (`unreachable!("llvm_profparsers works on +/// complete data")`). That failed the Coverage job on main runs 4588 and 4600, +/// two days apart on unrelated commits, while required CI — which is not +/// instrumented — stayed green on the same trees. +/// +/// Satisfied either by `bashkit::testing::child_profile_path` or by setting +/// `LLVM_PROFILE_FILE` directly, which a test asserting on the child's +/// profraw as a bounded-artifact receipt needs to do. +/// +/// See knowledge/operations/testing.md, "Spawned CLI children get their own +/// coverage profile path". +#[test] +fn cli_spawning_tests_redirect_the_child_coverage_profile() { + let mut files = Vec::new(); + rust_files(&crates_dir(), &mut files); + + // How a file gets hold of the instrumented binary. + const SPAWN_MARKERS: &[&str] = &["bashkit_cli_path()", "CARGO_BIN_EXE_bashkit"]; + // How it redirects the child: the shared helper, or its own path. + const REDIRECT_MARKERS: &[&str] = &["child_profile_path", "LLVM_PROFILE_FILE"]; + + let mut spawners = Vec::new(); + let mut offenders = Vec::new(); + for file in &files { + if file.ends_with("integration/bounded_child_launcher_scan_tests.rs") { + continue; + } + let source = std::fs::read_to_string(file) + .unwrap_or_else(|e| panic!("read {}: {e}", file.display())); + // Only code spawns; prose about the rule names the markers too. + let code: String = source + .lines() + .filter(|l| !l.trim_start().starts_with("//")) + .collect::>() + .join("\n"); + if !SPAWN_MARKERS.iter().any(|m| code.contains(m)) { + continue; + } + spawners.push(file.display().to_string()); + if !REDIRECT_MARKERS.iter().any(|m| code.contains(m)) { + offenders.push(file.display().to_string()); + } + } + + // Fail closed: a scan that found no spawners proves nothing. + assert!( + spawners.len() >= 3, + "scan did not reach the known CLI-spawning harnesses, found: {spawners:?}" + ); + assert!( + offenders.is_empty(), + "these spawn the instrumented bashkit binary without redirecting the \ + child's coverage profile; a killed child then leaves a truncated \ + profraw in tarpaulin's merge set and the Coverage job fails at random. \ + Set `LLVM_PROFILE_FILE` from `bashkit::testing::child_profile_path()` \ + (knowledge/operations/testing.md):\n{}", + offenders.join("\n") + ); +} diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index a59aa807f..7b01707b1 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -255,17 +255,29 @@ pub(crate) mod resource_exhaustion { }; } - fn run_cli_argv_bounded(script: &str, extra_args: &[&str], limit: Duration) -> BoundedRun { + /// The launcher-wrapped CLI command a bounded run executes. Separated so + /// the harness wiring itself is assertable without running a child: the + /// sandbox does not expose host env to the script, so a bounded run + /// cannot report back what its own process received. + fn bounded_cli_command(script: &str, extra_args: &[&str]) -> Command { let launcher = limit_launcher(); let mut cmd = Command::new("bash"); cmd.args(["-c", &launcher, "_"]) .arg(bashkit_cli_path()) .args(["-c", script]) .args(extra_args) + // The CLI is instrumented under coverage and these children are + // killed by design; without this the truncated profraw lands in + // tarpaulin's merge set. See `testing::child_profile_path`. + .env("LLVM_PROFILE_FILE", bashkit::testing::child_profile_path()) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); - let mut run = run_command_bounded(cmd, limit); + cmd + } + + fn run_cli_argv_bounded(script: &str, extra_args: &[&str], limit: Duration) -> BoundedRun { + let mut run = run_command_bounded(bounded_cli_command(script, extra_args), limit); strip_launcher_note(&mut run); run } @@ -917,6 +929,10 @@ pub(crate) mod resource_exhaustion { cmd.args(["-c", &launcher, "_"]) .arg(program) .args(args) + // `program` is a parameter, so an instrumented binary can reach + // here; a non-instrumented helper just ignores this. Same reason + // as `run_cli_argv_bounded`. + .env("LLVM_PROFILE_FILE", bashkit::testing::child_profile_path()) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); @@ -931,6 +947,39 @@ pub(crate) mod resource_exhaustion { run_cli_argv_bounded(script, &[], limit) } + /// Every bounded CLI child is handed a coverage profile path outside the + /// directory tarpaulin merges, and a distinct one per child. + /// + /// These children are killed by design, and a SIGKILLed instrumented + /// process leaves a truncated profraw that fails tarpaulin's merge step + /// (main Coverage runs 4588 and 4600). Asserted on the built `Command` + /// rather than from inside the child: the sandbox deliberately withholds + /// host env from the script (TM-INF-013), so a bounded run cannot echo + /// back what its own process received. This pins the actual value, which + /// the static scan in `bounded_child_launcher_scan_tests` cannot. + #[test] + fn bounded_cli_children_get_a_profile_path_outside_the_merge_set() { + fn profile_of(cmd: &Command) -> std::ffi::OsString { + cmd.get_envs() + .find(|(key, _)| *key == std::ffi::OsStr::new("LLVM_PROFILE_FILE")) + .map(|(_, value)| value.expect("profile path is set, not removed")) + .expect("bounded CLI children must get LLVM_PROFILE_FILE") + .to_owned() + } + + let first = profile_of(&bounded_cli_command("echo hi", &[])); + let text = first.to_string_lossy(); + assert!(text.ends_with(".profraw"), "{text}"); + assert!( + !text.contains("tarpaulin"), + "child would write into tarpaulin's merge set: {text}" + ); + + // Distinct per child: two victims must not race on one file. + let second = profile_of(&bounded_cli_command("echo hi", &[])); + assert_ne!(first, second); + } + /// Reaps a victim inside a finite grace via caller-supplied polling. /// Returns the observed status, or `None` when the victim cannot be /// reaped inside the grace (or polling itself errors): the safe seam diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 5442f712e..f48f285d0 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -369,6 +369,42 @@ overran it under ASAN, which is how nightly #247 surfaced it. Two rules follow: - Make the linear path fast rather than the budget generous. Raising the budget to fit a slow linear run also lets a quadratic run through. +## Spawned CLI children get their own coverage profile path + +A test that spawns the instrumented `bashkit` binary must point the child's +`LLVM_PROFILE_FILE` somewhere `cargo tarpaulin` does not merge. + +A child inherits the *parent's* profile-path template, so it writes a profraw +named after the parent test binary into `target/tarpaulin/profraws/` -- the +directory tarpaulin merges. Supervised children are killed by design, and a +SIGKILLed process never flushes its profraw, so the merge step panics inside +`llvm_profparser` (`unreachable!("llvm_profparsers works on complete data")`) +and the Coverage job fails. It is a race on whether the victim flushed, so it +is intermittent: main Coverage runs 4588 and 4600 failed two days apart on +unrelated commits, each after all 2,441 tests had passed, while required CI -- +which is not instrumented -- stayed green on the same trees. The same +divergence shape as the ASAN address-space bug above. + +`bashkit::testing::child_profile_path()` supplies the path: one directory per +process, a unique file per call, removal left to the OS. Cleanup is +deliberately not guarded, because a killed child's foreign descendant can +still be writing there and a per-call remove could fail for reasons unrelated +to the test. A test that instead *asserts* on the child's profraw, as +`run_input_shape_bounded` does for its bounded-artifact receipt, sets +`LLVM_PROFILE_FILE` to its own path; the rule is only that the child never +writes into the merge set. + +Two layers enforce it. `bounded_child_launcher_scan_tests` scans every crate +and fails any file that reaches the binary (`bashkit_cli_path()`, +`CARGO_BIN_EXE_bashkit`) without setting a profile path, and it fails closed +when the scan finds no spawners at all. The scan cannot prove the variable +survives the `bash -c` launcher and its `exec`, so +`bounded_cli_child_profile_points_outside_the_merge_set` has a real bounded +child print its own `LLVM_PROFILE_FILE` back. + +Redirecting also fixes attribution: without it the CLI's counters land in a +file tarpaulin credits to the parent test binary. + ## Public capability parity contract [`contracts/capability-parity.json`](../../contracts/capability-parity.json) is the