From d21fbc656d5701b8df0d05a741313e2407fd35aa Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 22:19:04 +0000 Subject: [PATCH] fix(tests): resolve the spawned CLI binary without guessing cargo layout The bounded-child harness took the profile directory to be the test binary's grandparent, which holds only for `/deps/`. Cargo 1.101-nightly's separate build directory puts the test binary in `/build///out/`, so the lookup resolved into a hashed build directory and the five CLI-spawning regressions failed on a path that never existed (`NotFound` before the launcher landed, exit 127 after) rather than on the bounds they assert. That is why the nightly ASAN job went red on run 252 while required CI, on the pinned stable toolchain, stayed green on the same commits. Only the export of the final binary into the profile directory is contractual, and that directory is an ancestor of the test binary under either layout, so the lookup now walks up until it finds the binary and fails closed naming every path it ruled out. `is_file` is load-bearing: the nightly layout has a package-named `build/bashkit` directory that a bare existence check would accept. The walk stops at cargo's target directory, recognised by its `CACHEDIR.TAG`, so a same-named file above the build tree is never spawned in place of the binary under test; a depth bound applies whether or not that marker is present. `--bins` is not the mechanism and is not used: on nightly it builds the binary only as a test harness inside the hashed build directory and exports nothing runnable. What makes cargo export it is selecting the bashkit-cli package, whose own integration tests get `CARGO_BIN_EXE_bashkit`; both the required CI job and the ASAN job already do that through `--tests`. Covered by `cli_lookup_resolves_known_cargo_layouts` over synthetic trees for both layouts, since the live tree only ever has its own toolchain's, including the directory collision and a decoy above the target directory. Claude-Session: https://claude.ai/code/session_01AZCDWdi9DiWzZF5VmWdqXM --- .../tests/integration/threat_model_tests.rs | 139 +++++++++++++++++- knowledge/operations/testing.md | 39 +++++ 2 files changed, 173 insertions(+), 5 deletions(-) diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 7166917d2..4724d684b 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -22,15 +22,81 @@ use std::time::{Duration, Instant}; mod resource_exhaustion { use super::*; + /// Ancestor levels searched for the CLI binary. Five are needed for the + /// deepest known layout (`out`, hash, package, `build`, profile); the + /// spare one absorbs another level without reaching above `target`. + const CLI_SEARCH_DEPTH: usize = 6; + + /// Locates the `bashkit` binary exported next to this test binary's + /// build tree, or returns the candidates it ruled out. + /// + /// Where cargo puts a *test* binary is not contractual and differs by + /// toolchain: the pinned stable builds it into `/deps/`, while + /// cargo 1.101-nightly's separate build directory uses + /// `/build///out/`. What cargo does guarantee is + /// that the final `bashkit` binary is exported to the profile directory, + /// an ancestor of the test binary under either layout — so the profile + /// directory is found by walking up until the binary turns up, never by + /// assuming a fixed depth. Taking it as `exe.parent().parent()` passed on + /// stable and resolved into the hashed build directory under nightly, + /// which is how the nightly ASAN job went red while required CI on the + /// same commits stayed green. + /// + /// `is_file` is load-bearing: the nightly layout puts a *directory* + /// named after the package at `/build/bashkit`, which a bare + /// existence check would accept. + /// + /// The walk stops at cargo's target directory, marked by the + /// `CACHEDIR.TAG` cargo writes there, so an unrelated `bashkit` file + /// above the build tree can never be spawned in place of the binary + /// under test. That marker is belt-and-braces over `CLI_SEARCH_DEPTH`, + /// which bounds the walk whether or not it is present. + fn cli_in_ancestors(exe: &Path) -> Result> { + let name = format!("bashkit{}", std::env::consts::EXE_SUFFIX); + let mut ruled_out = Vec::new(); + for dir in exe.ancestors().skip(1).take(CLI_SEARCH_DEPTH) { + let candidate = dir.join(&name); + if candidate.is_file() { + return Ok(candidate); + } + ruled_out.push(candidate); + if dir.join("CACHEDIR.TAG").is_file() { + break; + } + } + Err(ruled_out) + } + /// TM-DOS-135: child-process runner for hostile scripts. /// Hostile reproducers never run in the test process: pre-fix they - /// overflowed the stack and aborted the whole runner. The CLI binary - /// sits next to the test binary's `deps` directory. + /// overflowed the stack and aborted the whole runner. + /// + /// The binary belongs to a sibling package, so `CARGO_BIN_EXE_*` is not + /// available here; it is built because the invocation selects that + /// package's own integration tests, which do get that variable. A + /// selection narrow enough to exclude them (`-p bashkit --test + /// integration` alone) leaves nothing to find, which used to surface as a + /// bare spawn `NotFound` and, once the launcher was introduced, as an + /// unexplained exit 127 with the bound assertions failing — both read + /// like product failures. Fail on the precondition instead, naming every + /// path ruled out. fn bashkit_cli_path() -> std::path::PathBuf { let exe = std::env::current_exe().expect("current test executable"); - let deps = exe.parent().expect("deps dir"); - let profile = deps.parent().expect("profile dir"); - profile.join(format!("bashkit{}", std::env::consts::EXE_SUFFIX)) + match cli_in_ancestors(&exe) { + Ok(cli) => cli, + Err(ruled_out) => panic!( + "bashkit CLI binary not found above the test binary {}: this suite spawns \ + it to run hostile reproducers out of process, so the test selection must \ + include the bashkit-cli package (its own integration tests are what make \ + cargo export the binary). Ruled out: {}", + exe.display(), + ruled_out + .iter() + .map(|p| p.display().to_string()) + .collect::>() + .join(", ") + ), + } } struct BoundedRun { @@ -1150,6 +1216,69 @@ mod resource_exhaustion { } } + /// Layout regression for the CLI lookup, over synthetic trees rather + /// than this run's own: the live tree only ever has the one layout its + /// toolchain produces, so the other would go uncovered until a nightly + /// job went red. Both known layouts must resolve the exported binary; + /// the package-named `build/bashkit` directory must not be mistaken for + /// it; and absence must stay an error, never a path that fails later as + /// a spawn error or an exit 127. + #[test] + fn cli_lookup_resolves_known_cargo_layouts() { + let root = std::env::temp_dir().join(format!("bashkit-cli-layout-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let profile = root.join("target/x86_64-unknown-linux-gnu/debug"); + // Stable: `/deps/`. Nightly 1.101 separate build + // directory: `/build///out/`, whose + // package level collides by name with the binary being sought. + let stable_exe = profile.join("deps/integration-0a5939c1"); + let nightly_exe = profile.join("build/bashkit/f7e14526a94504a4/out/integration-f7e1"); + for exe in [&stable_exe, &nightly_exe] { + std::fs::create_dir_all(exe.parent().expect("test binary dir")) + .expect("create test binary dir"); + std::fs::write(exe, b"").expect("create test binary"); + } + + for exe in [&stable_exe, &nightly_exe] { + let ruled_out = cli_in_ancestors(exe).expect_err("no binary exported yet"); + assert!( + ruled_out.contains(&profile.join("bashkit")), + "the profile directory must be among the paths reported: {ruled_out:?}" + ); + } + + // A same-named file above the build tree is never a substitute for + // the binary under test: spawning it would be a false pass, worse + // than the failure. The target directory marker stops the walk. + std::fs::write(root.join("target/CACHEDIR.TAG"), b"Signature: 8a477f59") + .expect("mark target dir"); + std::fs::write( + root.join(format!("bashkit{}", std::env::consts::EXE_SUFFIX)), + b"", + ) + .expect("decoy above the build tree"); + for exe in [&stable_exe, &nightly_exe] { + let ruled_out = cli_in_ancestors(exe).expect_err("decoy must not be accepted"); + assert!( + !ruled_out.iter().any(|p| p.parent() == Some(root.as_path())), + "walk escaped the target directory: {ruled_out:?}" + ); + } + + let cli = profile.join(format!("bashkit{}", std::env::consts::EXE_SUFFIX)); + std::fs::write(&cli, b"").expect("export cli binary"); + for exe in [&stable_exe, &nightly_exe] { + assert_eq!( + cli_in_ancestors(exe).as_deref().ok(), + Some(cli.as_path()), + "layout not resolved for {}", + exe.display() + ); + } + + std::fs::remove_dir_all(&root).expect("clean up layout fixture"); + } + /// 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 diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index f98d9f233..5646034c2 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -126,6 +126,45 @@ Anything observable only through a subprocess belongs in `cli_oneshot.rs`; anything about how flags configure the builder stays in the `main.rs` unit tests, which are far cheaper. +### The `bashkit` suite spawns the CLI, and must not guess its path + +One exception runs the CLI from the `bashkit` crate: the bounded-child +harness in `threat_model_tests.rs` (TM-DOS-135) spawns the `bashkit` +binary so hostile reproducers never execute in the test process, where +pre-fix they overflowed the stack and aborted the whole runner. + +That binary belongs to a sibling package, so `CARGO_BIN_EXE_bashkit` is +not available there. Two rules follow. + +1. The test selection must include the `bashkit-cli` package. Its own + integration tests do get `CARGO_BIN_EXE_bashkit`, which is what makes + cargo export the binary at all; `--tests` across the workspace is + enough, while `-p bashkit --test integration` on its own is not. + `--bins` is not the mechanism: on cargo 1.101-nightly it builds the + binary only as a test harness, inside the hashed build directory, and + exports nothing runnable. +2. The harness must locate the binary by walking up from the test binary + until it finds it, never at a fixed depth. Where cargo puts a test + binary is not contractual: the pinned stable uses + `/deps/`, cargo 1.101-nightly's separate build + directory uses `/build///out/`. Only the + export into the profile directory is guaranteed, and that directory is + an ancestor either way. Reading it as `exe.parent().parent()` passed on + stable and landed in the hashed build directory under nightly, so the + nightly ASAN job went red while required CI stayed green. + +The walk stops at cargo's target directory, recognised by the +`CACHEDIR.TAG` cargo writes there, so an unrelated `bashkit` file above +the build tree can never be spawned in place of the binary under test — +that would be a false pass, worse than the failure. A depth bound applies +whether or not the marker is present. + +`cli_lookup_resolves_known_cargo_layouts` covers both layouts over +synthetic trees (the live tree only ever has its own toolchain's layout), +plus the package-named `build/bashkit` *directory* not being mistaken for +the binary and a decoy above the target directory being refused. +`bashkit_cli_path()` fails closed naming every path it ruled out. + ## Coverage Uploaded to Codecov from three sources: Rust unit/integration coverage via