From 4a36a1e0bd2fc94ec1d81a05b521d0b5242a0e6c Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:02:24 -0700 Subject: [PATCH 1/2] Make kernel and core-admin tests fail on an absent subject The auth-oidc cdylib probe panics in every run instead of skipping outside CI; the seven skip guards in plugin_chain_tests.rs are gone. pipe_tests.rs asserts /bin/cat, /usr/bin/env and a non-empty HOME instead of returning. The core-admin unreadable-catalog test replaces its chmod 000 injection (ignored by root) with a regular file at the plugins-dir path, which fails read_dir for every user, and asserts that precondition instead of skipping. A no_silent_skip scanner in each crate fails on skip prints, CI probes and early-return guards on an absent subject. --- .../src/v1/tests/service_tests.rs | 58 +-- .../busbar-core-admin/tests/no_silent_skip.rs | 414 +++++++++++++++++ .../src/auth/tests/plugin_chain_tests.rs | 85 ++-- .../src/plane_host/pipe_tests.rs | 38 +- crates/busbar-kernel/tests/no_silent_skip.rs | 417 ++++++++++++++++++ 5 files changed, 898 insertions(+), 114 deletions(-) create mode 100644 crates/busbar-core-admin/tests/no_silent_skip.rs create mode 100644 crates/busbar-kernel/tests/no_silent_skip.rs diff --git a/crates/busbar-core-admin/src/v1/tests/service_tests.rs b/crates/busbar-core-admin/src/v1/tests/service_tests.rs index b4f10afb86..051e4a19b3 100644 --- a/crates/busbar-core-admin/src/v1/tests/service_tests.rs +++ b/crates/busbar-core-admin/src/v1/tests/service_tests.rs @@ -826,16 +826,18 @@ fn catalog_cache_future_inserted_at_is_treated_as_stale() { /// The staleness scenario this guards against. /// FIRST, an empty-but-READABLE plugins dir caches fine (unchanged behavior — same as a MISSING -/// dir, both legitimately mean "no plugins"). THEN the directory becomes UNREADABLE (permission -/// denied) with its CONTENTS unchanged — the exact case the old `unwrap_or_default()` collapsed -/// to the SAME fingerprint as the empty dir, serving the stale cached `[]` forever instead of -/// the real `INVALID` row. The fixed version must never serve that stale cache and must surface -/// the real `INVALID: ...` row on every read while unreadable, not just the first. +/// dir, both legitimately mean "no plugins"). THEN the path becomes UNREADABLE as a directory — +/// the exact case the old `unwrap_or_default()` collapsed to the SAME fingerprint as the empty dir, +/// serving the stale cached `[]` forever instead of the real `INVALID` row. The fixed version must +/// never serve that stale cache and must surface the real `INVALID: ...` row on every read while +/// unreadable, not just the first. +/// +/// The injection replaces the directory with a regular FILE at the same path: `read_dir` on it +/// fails ("not a directory") whatever the privileges, so the test runs as root and non-root alike +/// (a `chmod 000` is ignored by root, and once made this test skip on a root-run container). #[cfg(unix)] #[test] fn catalog_unreadable_dir_does_not_serve_stale_cache() { - use std::os::unix::fs::PermissionsExt; - let dir = tmp_plugins_dir("unreadable"); let svc = svc_with(dir.clone(), unsigned_ok_posture()); @@ -850,39 +852,25 @@ fn catalog_unreadable_dir_does_not_serve_stale_cache() { "an empty dir's (empty) scan is cached, exactly like a missing dir would be" ); - // Restoring permissions with a bare `set_permissions` call at the - // END of the test left a window — the two `store_plugin_catalog()` reads below AND the - // `read_dir` probe above all run un-guarded, and a panic (e.g. an assertion failure inside - // `store_plugin_catalog`, or any future change to it) during that window would leave the - // temp dir at `0o000` permanently: nothing later ever restores it, potentially breaking - // this test's OWN cleanup or a later test that reuses the same `temp_dir()` infrastructure. - // An RAII guard restores the original mode on drop — including on an early return or a - // panic unwinding through this scope — so there is no code path that leaves the directory - // unreadable. - struct RestorePermsOnDrop<'a> { - dir: &'a std::path::Path, - mode: u32, - } - impl Drop for RestorePermsOnDrop<'_> { + // An RAII guard removes the injected file on drop — including on a panic unwinding through + // this scope — so no code path leaves a regular file squatting on the plugins-dir path. + struct RemoveFileOnDrop<'a>(&'a std::path::Path); + impl Drop for RemoveFileOnDrop<'_> { fn drop(&mut self) { - let _ = std::fs::set_permissions(self.dir, std::fs::Permissions::from_mode(self.mode)); + let _ = std::fs::remove_file(self.0); } } - let original_mode = std::fs::metadata(&dir).unwrap().permissions().mode(); - std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o000)).unwrap(); - let _restore = RestorePermsOnDrop { - dir: &dir, - mode: original_mode, - }; + std::fs::remove_dir(&dir).unwrap(); + std::fs::write(&dir, b"not a directory").unwrap(); + let _remove = RemoveFileOnDrop(&dir); - // Some environments (containers running as root) ignore permission bits entirely — skip - // rather than false-fail if `read_dir` still succeeds. `_restore` drops (restoring - // permissions) when this early return unwinds the scope. - if std::fs::read_dir(&dir).is_ok() { - eprintln!("skip: running with privileges that bypass directory permission bits"); - return; - } + // The precondition is checked, not assumed: a regular file is never readable as a directory, + // for root and non-root alike. + assert!( + std::fs::read_dir(&dir).is_err(), + "a regular file at the plugins-dir path must fail read_dir for every user" + ); let cat_after = svc.store_plugin_catalog(); // And every SUBSEQUENT read while STILL unreadable surfaces the SAME real row again — the diff --git a/crates/busbar-core-admin/tests/no_silent_skip.rs b/crates/busbar-core-admin/tests/no_silent_skip.rs new file mode 100644 index 0000000000..106306dac5 --- /dev/null +++ b/crates/busbar-core-admin/tests/no_silent_skip.rs @@ -0,0 +1,414 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Busbar Inc and contributors + +//! NO TEST IN THIS CRATE PASSES BY SKIPPING ITS SUBJECT (status row 114). An instrument that cannot +//! produce a NO is not a check, and when an instrument must choose, it false-fails +//! (BUSBAR-1.6.0.md): a test whose subject is absent fails, it never returns early and reports `ok`. +//! +//! A scan over this crate's own test sources — every `.rs` under a `tests/` directory (in `src/` +//! or the crate's `tests/`) and every `src/` file named `*_tests.rs` or `tests.rs`, this file +//! excluded — fails, naming `file:line`, on: +//! +//! * (a) a `println!`/`eprintln!` whose literal starts `skip:` or `SKIP`; +//! * (b) a `CI` environment probe (`var_os("CI")` / `var("CI")`): the switch that made a guard fatal +//! under CI and silent everywhere else; +//! * (c) an `if` or let-else guard on a subject probe (`auth_cdylib`, `write_auth_plugin`, an +//! absolute `Path::new("/`, `var("HOME")`, `read_dir(&dir).is_ok()`) whose body is nothing but +//! comments, prints and a bare `return;`. +//! +//! Comments are blanked before the scan, so prose that names these shapes is not a finding. + +use std::path::{Path, PathBuf}; + +/// Files the scan must reach, so a walk that silently found nothing cannot pass. +const MUST_COVER: &[&str] = &["src/v1/tests/service_tests.rs", "src/tests/tests.rs"]; + +/// The probes whose early-return guard is a skip (rule c). +const SUBJECTS: &[&str] = &[ + "auth_cdylib", + "write_auth_plugin", + "Path::new(\"/", + "var(\"HOME\")", + "read_dir(&dir).is_ok()", +]; + +const RULE_A: &str = "(a) a `skip:` print"; +const RULE_B: &str = "(b) a CI probe"; +const RULE_C: &str = "(c) an early-return guard on an absent subject"; + +fn crate_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +/// The crate's test sources, this file excluded. +fn test_sources() -> Vec { + fn walk(dir: &Path, under_tests: bool, out: &mut Vec) { + let mut entries: Vec = std::fs::read_dir(dir) + .unwrap_or_else(|e| panic!("read {}: {e}", dir.display())) + .map(|e| e.expect("a directory entry").path()) + .collect(); + entries.sort(); + for path in entries { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default() + .to_owned(); + if path.is_dir() { + walk(&path, under_tests || name == "tests", out); + } else if name.ends_with(".rs") + && (under_tests || name == "tests.rs" || name.ends_with("_tests.rs")) + { + out.push(path); + } + } + } + let root = crate_root(); + let mut out = Vec::new(); + walk(&root.join("src"), false, &mut out); + walk(&root.join("tests"), true, &mut out); + let me = root.join("tests").join("no_silent_skip.rs"); + out.retain(|p| *p != me); + out +} + +fn is_ident(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' +} + +/// If a string, raw string or char literal starts at `i`, the index just past it. +fn literal_end(b: &[u8], i: usize) -> Option { + match b[i] { + b'"' => { + let mut j = i + 1; + while j < b.len() { + match b[j] { + b'\\' => j += 2, + b'"' => return Some(j + 1), + _ => j += 1, + } + } + Some(b.len()) + } + b'r' if i == 0 + || !is_ident(b[i - 1]) + || (b[i - 1] == b'b' && (i < 2 || !is_ident(b[i - 2]))) => + { + let mut j = i + 1; + while b.get(j) == Some(&b'#') { + j += 1; + } + if b.get(j) != Some(&b'"') { + return None; + } + let hashes = j - i - 1; + j += 1; + while j < b.len() { + if b[j] == b'"' && b[j + 1..].iter().take(hashes).filter(|&&c| c == b'#').count() == hashes { + return Some(j + 1 + hashes); + } + j += 1; + } + Some(b.len()) + } + b'\'' => { + if b.get(i + 1) == Some(&b'\\') { + // Past the escaped character itself, so `'\''` ends at its own closing quote. + let mut j = i + 3; + while j < b.len() && b[j] != b'\'' { + j += 1; + } + return Some(j + 1); + } + let lead = *b.get(i + 1)?; + let width = match lead { + 0x00..=0x7f => 1, + 0xc0..=0xdf => 2, + 0xe0..=0xef => 3, + _ => 4, + }; + (b.get(i + 1 + width) == Some(&b'\'')).then_some(i + 2 + width) + } + _ => None, + } +} + +/// `src` with every comment blanked to spaces; newlines and literals are kept, so offsets and line +/// numbers hold. +fn mask_comments(src: &str) -> Vec { + let b = src.as_bytes(); + let mut out = b.to_vec(); + let mut i = 0; + while i < b.len() { + if b[i] == b'/' && b.get(i + 1) == Some(&b'/') { + while i < b.len() && b[i] != b'\n' { + out[i] = b' '; + i += 1; + } + } else if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + let mut depth = 0usize; + while i < b.len() { + if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + depth += 1; + out[i] = b' '; + out[i + 1] = b' '; + i += 2; + } else if b[i] == b'*' && b.get(i + 1) == Some(&b'/') { + depth -= 1; + out[i] = b' '; + out[i + 1] = b' '; + i += 2; + if depth == 0 { + break; + } + } else { + if b[i] != b'\n' { + out[i] = b' '; + } + i += 1; + } + } + } else if let Some(end) = literal_end(b, i) { + i = end; + } else { + i += 1; + } + } + out +} + +fn find_all(hay: &[u8], needle: &[u8]) -> Vec { + hay.windows(needle.len()) + .enumerate() + .filter(|(_, w)| *w == needle) + .map(|(i, _)| i) + .collect() +} + +fn skip_ws(m: &[u8], mut i: usize) -> usize { + while i < m.len() && m[i].is_ascii_whitespace() { + i += 1; + } + i +} + +/// The index of the bracket closing the one at `open`, skipping literals. +fn matching(m: &[u8], open: usize) -> Option { + let close = match m[open] { + b'(' => b')', + b'[' => b']', + b'{' => b'}', + _ => return None, + }; + let mut depth = 0usize; + let mut i = open; + while i < m.len() { + if let Some(end) = literal_end(m, i) { + i = end; + continue; + } + if m[i] == m[open] { + depth += 1; + } else if m[i] == close { + depth -= 1; + if depth == 0 { + return Some(i); + } + } + i += 1; + } + None +} + +/// When the subject at `pos` is the condition of an `if` or the scrutinee of a let-else, the +/// guard's body (between its braces). +fn guard_body(m: &[u8], pos: usize) -> Option> { + let start = m[..pos] + .iter() + .rposition(|&c| matches!(c, b';' | b'{' | b'}')) + .map_or(0, |i| i + 1); + let mut head = &m[skip_ws(m, start)..pos]; + if let Some(rest) = head.strip_prefix(b"else") { + head = &rest[skip_ws(rest, 0)..]; + } + let is_if = head.starts_with(b"if ") || head.starts_with(b"if!") || head.starts_with(b"if("); + let is_let = head.starts_with(b"let "); + if !is_if && !is_let { + return None; + } + let mut i = pos; + let open = loop { + if i >= m.len() { + return None; + } + if let Some(end) = literal_end(m, i) { + i = end; + continue; + } + match m[i] { + b';' => return None, + b'{' => break i, + _ => i += 1, + } + }; + if is_let && !m[pos..open].trim_ascii_end().ends_with(b"else") { + return None; + } + Some(open + 1..matching(m, open)?) +} + +/// Whether a guard body is nothing but prints and a bare `return;` (comments are already blank). +fn is_bare_skip(m: &[u8], body: std::ops::Range) -> bool { + const PRINTS: &[&[u8]] = &[b"eprintln!", b"println!", b"eprint!", b"print!"]; + let mut i = body.start; + let mut returns = false; + loop { + i = skip_ws(m, i); + if i >= body.end { + return returns; + } + let rest = &m[i..body.end]; + if let Some(print) = PRINTS.iter().find(|p| rest.starts_with(p)) { + let open = skip_ws(m, i + print.len()); + let Some(close) = (open < body.end).then(|| matching(m, open)).flatten() else { + return false; + }; + i = skip_ws(m, close + 1); + if m.get(i) == Some(&b';') { + i += 1; + } + } else if rest.starts_with(b"return") { + let after = skip_ws(m, i + b"return".len()); + if after < body.end && m[after] != b';' { + return false; + } + returns = true; + i = after + 1; + } else { + return false; + } + } +} + +/// Every finding in one source: its 1-based line and its rule. +fn scan(src: &str) -> Vec<(usize, &'static str)> { + let m = mask_comments(src); + let line_of = |pos: usize| m[..pos].iter().filter(|&&c| c == b'\n').count() + 1; + let mut found = Vec::new(); + + for pos in find_all(&m, b"println!") { + let open = skip_ws(&m, pos + b"println!".len()); + if !matches!(m.get(open), Some(b'(' | b'[' | b'{')) { + continue; + } + let lit = skip_ws(&m, open + 1); + if m.get(lit) == Some(&b'"') + && (m[lit + 1..].starts_with(b"skip:") || m[lit + 1..].starts_with(b"SKIP")) + { + found.push((line_of(pos), RULE_A)); + } + } + for probe in [&b"var_os(\"CI\")"[..], &b"var(\"CI\")"[..]] { + for pos in find_all(&m, probe) { + found.push((line_of(pos), RULE_B)); + } + } + for subject in SUBJECTS { + for pos in find_all(&m, subject.as_bytes()) { + if guard_body(&m, pos).is_some_and(|body| is_bare_skip(&m, body)) { + found.push((line_of(pos), RULE_C)); + } + } + } + found.sort(); + found.dedup(); + found +} + +#[test] +fn no_test_in_this_crate_passes_by_skipping_its_subject() { + let root = crate_root(); + let sources = test_sources(); + for must in MUST_COVER { + assert!( + sources.iter().any(|p| p.ends_with(must)), + "the scan must cover {must}; it walked {} files", + sources.len() + ); + } + let mut findings = Vec::new(); + for path in &sources { + let src = std::fs::read_to_string(path) + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + let lines: Vec<&str> = src.lines().collect(); + let rel = path.strip_prefix(&root).unwrap_or(path).display(); + for (line, rule) in scan(&src) { + let text = lines.get(line - 1).map_or("", |l| l.trim()); + findings.push(format!("{rel}:{line}: {rule}: {text}")); + } + } + assert!( + findings.is_empty(), + "{} test-source site(s) pass by skipping an absent subject; each must be a hard failure \ + (status row 114):\n{}", + findings.len(), + findings.join("\n") + ); +} + +/// The scan can say NO: every skip shape it names is found, and the hard-failure forms that +/// replace them are not. +#[test] +fn the_scan_finds_every_skip_shape_and_passes_the_hard_failures() { + let skips: &[(&str, &str)] = &[ + ("eprintln!(\"skip: not built\");", RULE_A), + ("println!(\n \"SKIP the rest\"\n);", RULE_A), + ("if std::env::var_os(\"CI\").is_some() { panic!(); }", RULE_B), + ("let ci = std::env::var(\"CI\").is_ok();", RULE_B), + ( + "let dir = d();\nlet Some(path) = auth_cdylib() else {\n eprintln!(\"why\"); // note\n return;\n};", + RULE_C, + ), + ( + "if !write_auth_plugin(&dir, \"idp.tar.gz\", \"a\", \"b\") {\n return;\n}", + RULE_C, + ), + ( + "let Ok(home) = std::env::var(\"HOME\") else {\n return; // no HOME\n};", + RULE_C, + ), + ( + "if !std::path::Path::new(\"/bin/cat\").exists() {\n return;\n}", + RULE_C, + ), + ( + "if std::fs::read_dir(&dir).is_ok() {\n eprintln!(\"bypassed\");\n return;\n}", + RULE_C, + ), + ]; + for (src, rule) in skips { + assert!( + scan(src).iter().any(|(_, r)| r == rule), + "the scan must find {rule} in:\n{src}\n(found {:?})", + scan(src) + ); + } + + let clean: &[&str] = &[ + "assert!(std::path::Path::new(\"/bin/cat\").exists(), \"/bin/cat is required by this test\");", + "let home = std::env::var(\"HOME\").expect(\"HOME must be set for this test\");", + "let lib = std::fs::read(auth_cdylib()).unwrap();", + "write_auth_plugin(&dir, \"idp.tar.gz\", \"a\", \"b\");", + "assert!(std::fs::read_dir(&dir).is_err(), \"fails for every user\");", + "if std::fs::read_dir(&dir).is_ok() {\n panic!(\"readable\");\n}", + "// eprintln!(\"skip: in prose\"); std::env::var_os(\"CI\")\n/* if !auth_cdylib() { return; } */", + "fn write_auth_plugin(dir: &Path) {\n return;\n}", + ]; + for src in clean { + assert!( + scan(src).is_empty(), + "the scan must pass:\n{src}\n(found {:?})", + scan(src) + ); + } +} diff --git a/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs b/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs index bac54ebb74..404bc0b8d6 100644 --- a/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs +++ b/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs @@ -32,15 +32,14 @@ use std::path::{Path, PathBuf}; /// The auth-oidc module's cdylib, built by the workspace build as a git dev-dependency of the /// composition root — so it lives under `deps/` WITH a metadata hash (`lib-.`) and is -/// never uplifted; the exact name is checked too. Newest wins. Under CI (`cargo test --workspace` -/// always builds it) a missing cdylib is a HARD failure, never a silent skip; locally a missing -/// cdylib skips cleanly. +/// never uplifted; the exact name is checked too. Newest wins. A missing cdylib is a HARD failure in +/// every run, never a skip. /// /// Checks BOTH the uplifted profile dir AND `target/deps`: a SCOPED build leaves the artifact in /// `target/deps` ALONE, and checking only the profile dir once made every cdylib-gated test in this /// file return early and report `ok` — `untrusted_auth_plugin_fails_closed_not_open` and /// `missing_auth_plugin_is_loud_boot_failure`, the front-door fail-closed guarantees, among them. -fn auth_cdylib() -> Option { +fn auth_cdylib() -> PathBuf { let candidate = (|| { let exe = std::env::current_exe().ok()?; let profile_dir = exe.parent()?.parent()?; @@ -69,13 +68,12 @@ fn auth_cdylib() -> Option { .max() .map(|(_, p)| p) })(); - if candidate.is_none() && std::env::var_os("CI").is_some() { + candidate.unwrap_or_else(|| { panic!( - "the auth-oidc plugin cdylib is not built under CI; refusing to silently skip the \ - full-chain auth-plugin seam coverage" - ); - } - candidate + "the busbar_auth_oidc_plugin cdylib is not built: run `cargo test -p busbar-kernel \ + --no-run` first (its dev-dependency edge builds it; this test never skips)" + ) + }) } /// The issuer every token in this file is signed by, and the audience it is bound to. @@ -126,11 +124,9 @@ fn auth_manifest(name: &str, alias: &str, publisher: &str) -> busbar_plugin_load .iter() .max() .expect("auth abi"); - m.statement = auth_cdylib().and_then(|path| { - busbar_plugin_loader::dispatch::rendering_of_library(&path) - .expect("the auth-oidc cdylib states its door") - .map(hex::encode) - }); + m.statement = busbar_plugin_loader::dispatch::rendering_of_library(&auth_cdylib()) + .expect("the auth-oidc cdylib states its door") + .map(hex::encode); m } @@ -140,11 +136,7 @@ fn auth_manifest(name: &str, alias: &str, publisher: &str) -> busbar_plugin_load #[test] fn a_v1_auth_plugin_is_refused_at_boot_and_the_current_one_builds_browser_login() { let dir = tmp_plugin_dir("auth-login-v1-gate"); - let Some(path) = auth_cdylib() else { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - }; - let lib = std::fs::read(&path).expect("read the auth-oidc cdylib"); + let lib = std::fs::read(auth_cdylib()).expect("read the auth-oidc cdylib"); let mut manifest = auth_manifest("acme-idp", "cap-idp", "acme"); manifest.abi_version = 1; std::fs::write(dir.join("idp.tar.gz"), unsigned_tarball(manifest, &lib)).unwrap(); @@ -191,17 +183,13 @@ fn a_v1_auth_plugin_is_refused_at_boot_and_the_current_one_builds_browser_login( let _ = std::fs::remove_dir_all(&dir2); } -/// Write an UNSIGNED (structurally valid) auth-oidc tarball into `dir` under `file`, returning the -/// cdylib bytes' presence. We use the unsigned+`allow_unsigned` path because the test cannot sign -/// with the embedded first-party release key; this still exercises the whole load pipeline. -fn write_auth_plugin(dir: &Path, file: &str, name: &str, alias: &str) -> bool { - let Some(path) = auth_cdylib() else { - return false; - }; - let lib = std::fs::read(&path).expect("read the auth-oidc cdylib"); +/// Write an UNSIGNED (structurally valid) auth-oidc tarball into `dir` under `file`. We use the +/// unsigned+`allow_unsigned` path because the test cannot sign with the embedded first-party release +/// key; this still exercises the whole load pipeline. +fn write_auth_plugin(dir: &Path, file: &str, name: &str, alias: &str) { + let lib = std::fs::read(auth_cdylib()).expect("read the auth-oidc cdylib"); let tarball = unsigned_tarball(auth_manifest(name, alias, "acme"), &lib); std::fs::write(dir.join(file), tarball).unwrap(); - true } /// An enabled plugins config over `dir` that permits the unsigned test cdylib. @@ -249,17 +237,13 @@ fn write_auth_plugin_with_root_secret_schema( file: &str, name: &str, alias: &str, -) -> bool { - let Some(path) = auth_cdylib() else { - return false; - }; - let lib = std::fs::read(&path).expect("read the auth-oidc cdylib"); +) { + let lib = std::fs::read(auth_cdylib()).expect("read the auth-oidc cdylib"); let tarball = unsigned_tarball( auth_manifest_with_root_secret_schema(name, alias, "acme", "audience"), &lib, ); std::fs::write(dir.join(file), tarball).unwrap(); - true } /// An `auth.chain` naming exactly the given plugin module (with an optional `settings` map). @@ -281,10 +265,7 @@ fn chain_with(module: &str, settings: serde_json::Map #[test] fn auth_plugin_setting_secret_ref_is_resolved_and_delivered() { let dir = tmp_plugin_dir("auth-plugin-secret-ref"); - if !write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "ref-auth") { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - } + write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "ref-auth"); let plugins = plugins_cfg_allow_unsigned(&dir); // The trust root delivered via an env SecretRef: the engine resolves `{ env: VAR }` to the PEM @@ -357,10 +338,7 @@ fn auth_plugin_setting_secret_ref_is_resolved_and_delivered() { #[test] fn auth_plugin_root_level_secret_marked_setting_resolves_and_authenticates() { let dir = tmp_plugin_dir("auth-plugin-root-secret-contract"); - if !write_auth_plugin_with_root_secret_schema(&dir, "idp.tar.gz", "acme-idp", "sec-auth") { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - } + write_auth_plugin_with_root_secret_schema(&dir, "idp.tar.gz", "acme-idp", "sec-auth"); let plugins = plugins_cfg_allow_unsigned(&dir); let var = format!("BUSBAR_PLUGIN_ROOT_SECRET_CONTRACT_{}", std::process::id()); @@ -434,10 +412,7 @@ fn auth_plugin_root_level_secret_marked_setting_resolves_and_authenticates() { fn auth_plugin_loads_and_identifies_through_middleware() { let dir = tmp_plugin_dir("auth-plugin-e2e"); // The config chain name is the ALIAS `my-auth`; the plugin's canonical name differs on purpose. - if !write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "my-auth") { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - } + write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "my-auth"); let plugins = plugins_cfg_allow_unsigned(&dir); let cfg = chain_with("my-auth", settings()); @@ -515,10 +490,7 @@ fn auth_plugin_role_binding_and_scope_cap_apply() { use busbar_contract::authz::Scope; let dir = tmp_plugin_dir("auth-plugin-policy"); - if !write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "idp") { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - } + write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "idp"); let plugins = plugins_cfg_allow_unsigned(&dir); let mut cfg = chain_with("idp", settings()); // The chain entry caps this module at `read-only`, below the `full` the role would otherwise @@ -592,10 +564,7 @@ fn auth_plugin_role_binding_and_scope_cap_apply() { fn admin_modules_rebuilt_on_reload() { use crate::auth::AdminAuthChain; let dir = tmp_plugin_dir("admin-modules-reload"); - if !write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "admin-oidc") { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - } + write_auth_plugin(&dir, "idp.tar.gz", "acme-idp", "admin-oidc"); let plugins = plugins_cfg_allow_unsigned(&dir); let registry = busbar_plugin_loader::scan_and_validate( Path::new(&plugins.dir), @@ -641,11 +610,7 @@ fn admin_modules_rebuilt_on_reload() { #[test] fn untrusted_auth_plugin_fails_closed_not_open() { let dir = tmp_plugin_dir("auth-plugin-untrusted"); - let Some(path) = auth_cdylib() else { - eprintln!("skip: auth-oidc plugin cdylib not built (run under --workspace)"); - return; - }; - let lib = std::fs::read(&path).unwrap(); + let lib = std::fs::read(auth_cdylib()).unwrap(); let tarball = unsigned_tarball(auth_manifest("acme-idp", "test-idp-double", "acme"), &lib); std::fs::write(dir.join("idp.tar.gz"), tarball).unwrap(); diff --git a/crates/busbar-kernel/src/plane_host/pipe_tests.rs b/crates/busbar-kernel/src/plane_host/pipe_tests.rs index bbf2350338..868d5b3dea 100644 --- a/crates/busbar-kernel/src/plane_host/pipe_tests.rs +++ b/crates/busbar-kernel/src/plane_host/pipe_tests.rs @@ -131,10 +131,11 @@ fn read_at_least(vt: &PlaneHostVtable, host: HostCtx, pipe: PipeId, want: usize) #[test] fn subprocess_pipe_echoes_bytes_through_cat() { - // `/bin/cat` exists on macOS and Linux alike; if a platform lacks it, skip rather than fail. - if !std::path::Path::new("/bin/cat").exists() { - return; - } + // `/bin/cat` exists on macOS and Linux alike; a runner without it is broken, so fail. + assert!( + std::path::Path::new("/bin/cat").exists(), + "/bin/cat is required by this test" + ); let command = pack_command(&["/bin/cat"]); let desc = subprocess_desc(&command); let allowlist = vec!["/bin/cat".to_string()]; @@ -189,9 +190,10 @@ fn subprocess_env_is_cleared_and_selective_never_leaking_the_hosts() { // `env_clear()` + selective `envs`, the child must see ONLY the records the plane named — never the // host's own environment (which holds provider keys), so the marker is present and the host's // always-present `PATH` is not. This is the CONFIRMED regression this carrier closes. - if !std::path::Path::new("/usr/bin/env").exists() { - return; - } + assert!( + std::path::Path::new("/usr/bin/env").exists(), + "/usr/bin/env is required by this test" + ); let command = pack_command(&["/usr/bin/env"]); let env = env_record("BUSBAR_ENV_MARKER", 0, b"present"); let mut desc = subprocess_desc(&command); @@ -227,15 +229,12 @@ fn subprocess_env_resolves_a_secret_reference_host_side() { // plaintext at spawn through the built-in resolver, exactly as the in-process stdio spawn does. // The `env` module reads a host environment variable — `HOME` is present in any test environment — // so the child ends up with the resolved value the plane never held. - if !std::path::Path::new("/usr/bin/env").exists() { - return; - } - let Ok(home) = std::env::var("HOME") else { - return; // no HOME to resolve against — skip rather than fail. - }; - if home.is_empty() { - return; - } + assert!( + std::path::Path::new("/usr/bin/env").exists(), + "/usr/bin/env is required by this test" + ); + let home = std::env::var("HOME").expect("HOME must be set for this test"); + assert!(!home.is_empty(), "HOME must be non-empty for this test"); let command = pack_command(&["/usr/bin/env"]); // The sugar form the config layer accepts: `{ env: HOME }` ⇒ the `env` secret module, key `HOME`. let secret_json = br#"{"env":"HOME"}"#; @@ -320,9 +319,10 @@ fn the_ffi_slot_refuses_a_plane_driven_subprocess() { #[test] fn arena_drop_reclaims_and_kills_an_unclosed_subprocess() { - if !std::path::Path::new("/bin/cat").exists() { - return; - } + assert!( + std::path::Path::new("/bin/cat").exists(), + "/bin/cat is required by this test" + ); let command = pack_command(&["/bin/cat"]); let desc = subprocess_desc(&command); let allowlist = vec!["/bin/cat".to_string()]; diff --git a/crates/busbar-kernel/tests/no_silent_skip.rs b/crates/busbar-kernel/tests/no_silent_skip.rs new file mode 100644 index 0000000000..3e92bf6654 --- /dev/null +++ b/crates/busbar-kernel/tests/no_silent_skip.rs @@ -0,0 +1,417 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Busbar Inc and contributors + +//! NO TEST IN THIS CRATE PASSES BY SKIPPING ITS SUBJECT (status row 114). An instrument that cannot +//! produce a NO is not a check, and when an instrument must choose, it false-fails +//! (BUSBAR-1.6.0.md): a test whose subject is absent fails, it never returns early and reports `ok`. +//! +//! A scan over this crate's own test sources — every `.rs` under a `tests/` directory (in `src/` +//! or the crate's `tests/`) and every `src/` file named `*_tests.rs` or `tests.rs`, this file +//! excluded — fails, naming `file:line`, on: +//! +//! * (a) a `println!`/`eprintln!` whose literal starts `skip:` or `SKIP`; +//! * (b) a `CI` environment probe (`var_os("CI")` / `var("CI")`): the switch that made a guard fatal +//! under CI and silent everywhere else; +//! * (c) an `if` or let-else guard on a subject probe (`auth_cdylib`, `write_auth_plugin`, an +//! absolute `Path::new("/`, `var("HOME")`, `read_dir(&dir).is_ok()`) whose body is nothing but +//! comments, prints and a bare `return;`. +//! +//! Comments are blanked before the scan, so prose that names these shapes is not a finding. + +use std::path::{Path, PathBuf}; + +/// Files the scan must reach, so a walk that silently found nothing cannot pass. +const MUST_COVER: &[&str] = &[ + "src/auth/tests/plugin_chain_tests.rs", + "src/plane_host/pipe_tests.rs", +]; + +/// The probes whose early-return guard is a skip (rule c). +const SUBJECTS: &[&str] = &[ + "auth_cdylib", + "write_auth_plugin", + "Path::new(\"/", + "var(\"HOME\")", + "read_dir(&dir).is_ok()", +]; + +const RULE_A: &str = "(a) a `skip:` print"; +const RULE_B: &str = "(b) a CI probe"; +const RULE_C: &str = "(c) an early-return guard on an absent subject"; + +fn crate_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +/// The crate's test sources, this file excluded. +fn test_sources() -> Vec { + fn walk(dir: &Path, under_tests: bool, out: &mut Vec) { + let mut entries: Vec = std::fs::read_dir(dir) + .unwrap_or_else(|e| panic!("read {}: {e}", dir.display())) + .map(|e| e.expect("a directory entry").path()) + .collect(); + entries.sort(); + for path in entries { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or_default() + .to_owned(); + if path.is_dir() { + walk(&path, under_tests || name == "tests", out); + } else if name.ends_with(".rs") + && (under_tests || name == "tests.rs" || name.ends_with("_tests.rs")) + { + out.push(path); + } + } + } + let root = crate_root(); + let mut out = Vec::new(); + walk(&root.join("src"), false, &mut out); + walk(&root.join("tests"), true, &mut out); + let me = root.join("tests").join("no_silent_skip.rs"); + out.retain(|p| *p != me); + out +} + +fn is_ident(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' +} + +/// If a string, raw string or char literal starts at `i`, the index just past it. +fn literal_end(b: &[u8], i: usize) -> Option { + match b[i] { + b'"' => { + let mut j = i + 1; + while j < b.len() { + match b[j] { + b'\\' => j += 2, + b'"' => return Some(j + 1), + _ => j += 1, + } + } + Some(b.len()) + } + b'r' if i == 0 + || !is_ident(b[i - 1]) + || (b[i - 1] == b'b' && (i < 2 || !is_ident(b[i - 2]))) => + { + let mut j = i + 1; + while b.get(j) == Some(&b'#') { + j += 1; + } + if b.get(j) != Some(&b'"') { + return None; + } + let hashes = j - i - 1; + j += 1; + while j < b.len() { + if b[j] == b'"' && b[j + 1..].iter().take(hashes).filter(|&&c| c == b'#').count() == hashes { + return Some(j + 1 + hashes); + } + j += 1; + } + Some(b.len()) + } + b'\'' => { + if b.get(i + 1) == Some(&b'\\') { + // Past the escaped character itself, so `'\''` ends at its own closing quote. + let mut j = i + 3; + while j < b.len() && b[j] != b'\'' { + j += 1; + } + return Some(j + 1); + } + let lead = *b.get(i + 1)?; + let width = match lead { + 0x00..=0x7f => 1, + 0xc0..=0xdf => 2, + 0xe0..=0xef => 3, + _ => 4, + }; + (b.get(i + 1 + width) == Some(&b'\'')).then_some(i + 2 + width) + } + _ => None, + } +} + +/// `src` with every comment blanked to spaces; newlines and literals are kept, so offsets and line +/// numbers hold. +fn mask_comments(src: &str) -> Vec { + let b = src.as_bytes(); + let mut out = b.to_vec(); + let mut i = 0; + while i < b.len() { + if b[i] == b'/' && b.get(i + 1) == Some(&b'/') { + while i < b.len() && b[i] != b'\n' { + out[i] = b' '; + i += 1; + } + } else if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + let mut depth = 0usize; + while i < b.len() { + if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + depth += 1; + out[i] = b' '; + out[i + 1] = b' '; + i += 2; + } else if b[i] == b'*' && b.get(i + 1) == Some(&b'/') { + depth -= 1; + out[i] = b' '; + out[i + 1] = b' '; + i += 2; + if depth == 0 { + break; + } + } else { + if b[i] != b'\n' { + out[i] = b' '; + } + i += 1; + } + } + } else if let Some(end) = literal_end(b, i) { + i = end; + } else { + i += 1; + } + } + out +} + +fn find_all(hay: &[u8], needle: &[u8]) -> Vec { + hay.windows(needle.len()) + .enumerate() + .filter(|(_, w)| *w == needle) + .map(|(i, _)| i) + .collect() +} + +fn skip_ws(m: &[u8], mut i: usize) -> usize { + while i < m.len() && m[i].is_ascii_whitespace() { + i += 1; + } + i +} + +/// The index of the bracket closing the one at `open`, skipping literals. +fn matching(m: &[u8], open: usize) -> Option { + let close = match m[open] { + b'(' => b')', + b'[' => b']', + b'{' => b'}', + _ => return None, + }; + let mut depth = 0usize; + let mut i = open; + while i < m.len() { + if let Some(end) = literal_end(m, i) { + i = end; + continue; + } + if m[i] == m[open] { + depth += 1; + } else if m[i] == close { + depth -= 1; + if depth == 0 { + return Some(i); + } + } + i += 1; + } + None +} + +/// When the subject at `pos` is the condition of an `if` or the scrutinee of a let-else, the +/// guard's body (between its braces). +fn guard_body(m: &[u8], pos: usize) -> Option> { + let start = m[..pos] + .iter() + .rposition(|&c| matches!(c, b';' | b'{' | b'}')) + .map_or(0, |i| i + 1); + let mut head = &m[skip_ws(m, start)..pos]; + if let Some(rest) = head.strip_prefix(b"else") { + head = &rest[skip_ws(rest, 0)..]; + } + let is_if = head.starts_with(b"if ") || head.starts_with(b"if!") || head.starts_with(b"if("); + let is_let = head.starts_with(b"let "); + if !is_if && !is_let { + return None; + } + let mut i = pos; + let open = loop { + if i >= m.len() { + return None; + } + if let Some(end) = literal_end(m, i) { + i = end; + continue; + } + match m[i] { + b';' => return None, + b'{' => break i, + _ => i += 1, + } + }; + if is_let && !m[pos..open].trim_ascii_end().ends_with(b"else") { + return None; + } + Some(open + 1..matching(m, open)?) +} + +/// Whether a guard body is nothing but prints and a bare `return;` (comments are already blank). +fn is_bare_skip(m: &[u8], body: std::ops::Range) -> bool { + const PRINTS: &[&[u8]] = &[b"eprintln!", b"println!", b"eprint!", b"print!"]; + let mut i = body.start; + let mut returns = false; + loop { + i = skip_ws(m, i); + if i >= body.end { + return returns; + } + let rest = &m[i..body.end]; + if let Some(print) = PRINTS.iter().find(|p| rest.starts_with(p)) { + let open = skip_ws(m, i + print.len()); + let Some(close) = (open < body.end).then(|| matching(m, open)).flatten() else { + return false; + }; + i = skip_ws(m, close + 1); + if m.get(i) == Some(&b';') { + i += 1; + } + } else if rest.starts_with(b"return") { + let after = skip_ws(m, i + b"return".len()); + if after < body.end && m[after] != b';' { + return false; + } + returns = true; + i = after + 1; + } else { + return false; + } + } +} + +/// Every finding in one source: its 1-based line and its rule. +fn scan(src: &str) -> Vec<(usize, &'static str)> { + let m = mask_comments(src); + let line_of = |pos: usize| m[..pos].iter().filter(|&&c| c == b'\n').count() + 1; + let mut found = Vec::new(); + + for pos in find_all(&m, b"println!") { + let open = skip_ws(&m, pos + b"println!".len()); + if !matches!(m.get(open), Some(b'(' | b'[' | b'{')) { + continue; + } + let lit = skip_ws(&m, open + 1); + if m.get(lit) == Some(&b'"') + && (m[lit + 1..].starts_with(b"skip:") || m[lit + 1..].starts_with(b"SKIP")) + { + found.push((line_of(pos), RULE_A)); + } + } + for probe in [&b"var_os(\"CI\")"[..], &b"var(\"CI\")"[..]] { + for pos in find_all(&m, probe) { + found.push((line_of(pos), RULE_B)); + } + } + for subject in SUBJECTS { + for pos in find_all(&m, subject.as_bytes()) { + if guard_body(&m, pos).is_some_and(|body| is_bare_skip(&m, body)) { + found.push((line_of(pos), RULE_C)); + } + } + } + found.sort(); + found.dedup(); + found +} + +#[test] +fn no_test_in_this_crate_passes_by_skipping_its_subject() { + let root = crate_root(); + let sources = test_sources(); + for must in MUST_COVER { + assert!( + sources.iter().any(|p| p.ends_with(must)), + "the scan must cover {must}; it walked {} files", + sources.len() + ); + } + let mut findings = Vec::new(); + for path in &sources { + let src = std::fs::read_to_string(path) + .unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + let lines: Vec<&str> = src.lines().collect(); + let rel = path.strip_prefix(&root).unwrap_or(path).display(); + for (line, rule) in scan(&src) { + let text = lines.get(line - 1).map_or("", |l| l.trim()); + findings.push(format!("{rel}:{line}: {rule}: {text}")); + } + } + assert!( + findings.is_empty(), + "{} test-source site(s) pass by skipping an absent subject; each must be a hard failure \ + (status row 114):\n{}", + findings.len(), + findings.join("\n") + ); +} + +/// The scan can say NO: every skip shape it names is found, and the hard-failure forms that +/// replace them are not. +#[test] +fn the_scan_finds_every_skip_shape_and_passes_the_hard_failures() { + let skips: &[(&str, &str)] = &[ + ("eprintln!(\"skip: not built\");", RULE_A), + ("println!(\n \"SKIP the rest\"\n);", RULE_A), + ("if std::env::var_os(\"CI\").is_some() { panic!(); }", RULE_B), + ("let ci = std::env::var(\"CI\").is_ok();", RULE_B), + ( + "let dir = d();\nlet Some(path) = auth_cdylib() else {\n eprintln!(\"why\"); // note\n return;\n};", + RULE_C, + ), + ( + "if !write_auth_plugin(&dir, \"idp.tar.gz\", \"a\", \"b\") {\n return;\n}", + RULE_C, + ), + ( + "let Ok(home) = std::env::var(\"HOME\") else {\n return; // no HOME\n};", + RULE_C, + ), + ( + "if !std::path::Path::new(\"/bin/cat\").exists() {\n return;\n}", + RULE_C, + ), + ( + "if std::fs::read_dir(&dir).is_ok() {\n eprintln!(\"bypassed\");\n return;\n}", + RULE_C, + ), + ]; + for (src, rule) in skips { + assert!( + scan(src).iter().any(|(_, r)| r == rule), + "the scan must find {rule} in:\n{src}\n(found {:?})", + scan(src) + ); + } + + let clean: &[&str] = &[ + "assert!(std::path::Path::new(\"/bin/cat\").exists(), \"/bin/cat is required by this test\");", + "let home = std::env::var(\"HOME\").expect(\"HOME must be set for this test\");", + "let lib = std::fs::read(auth_cdylib()).unwrap();", + "write_auth_plugin(&dir, \"idp.tar.gz\", \"a\", \"b\");", + "assert!(std::fs::read_dir(&dir).is_err(), \"fails for every user\");", + "if std::fs::read_dir(&dir).is_ok() {\n panic!(\"readable\");\n}", + "// eprintln!(\"skip: in prose\"); std::env::var_os(\"CI\")\n/* if !auth_cdylib() { return; } */", + "fn write_auth_plugin(dir: &Path) {\n return;\n}", + ]; + for src in clean { + assert!( + scan(src).is_empty(), + "the scan must pass:\n{src}\n(found {:?})", + scan(src) + ); + } +} From 37975120dfd44dc901eccb76740518e72a021ac1 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:03:12 -0700 Subject: [PATCH 2/2] pre-flight: fmt / lock / abi header --- crates/busbar-core-admin/tests/no_silent_skip.rs | 9 ++++++++- .../busbar-kernel/src/auth/tests/plugin_chain_tests.rs | 7 +------ crates/busbar-kernel/tests/no_silent_skip.rs | 9 ++++++++- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/busbar-core-admin/tests/no_silent_skip.rs b/crates/busbar-core-admin/tests/no_silent_skip.rs index 106306dac5..cc047ca3c7 100644 --- a/crates/busbar-core-admin/tests/no_silent_skip.rs +++ b/crates/busbar-core-admin/tests/no_silent_skip.rs @@ -104,7 +104,14 @@ fn literal_end(b: &[u8], i: usize) -> Option { let hashes = j - i - 1; j += 1; while j < b.len() { - if b[j] == b'"' && b[j + 1..].iter().take(hashes).filter(|&&c| c == b'#').count() == hashes { + if b[j] == b'"' + && b[j + 1..] + .iter() + .take(hashes) + .filter(|&&c| c == b'#') + .count() + == hashes + { return Some(j + 1 + hashes); } j += 1; diff --git a/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs b/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs index 404bc0b8d6..8f021f1bd7 100644 --- a/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs +++ b/crates/busbar-kernel/src/auth/tests/plugin_chain_tests.rs @@ -232,12 +232,7 @@ fn auth_manifest_with_root_secret_schema( /// Write an UNSIGNED auth-oidc tarball whose manifest declares `audience` as a ROOT-LEVEL /// `x-busbar-secret` field (see [`auth_manifest_with_root_secret_schema`]). -fn write_auth_plugin_with_root_secret_schema( - dir: &Path, - file: &str, - name: &str, - alias: &str, -) { +fn write_auth_plugin_with_root_secret_schema(dir: &Path, file: &str, name: &str, alias: &str) { let lib = std::fs::read(auth_cdylib()).expect("read the auth-oidc cdylib"); let tarball = unsigned_tarball( auth_manifest_with_root_secret_schema(name, alias, "acme", "audience"), diff --git a/crates/busbar-kernel/tests/no_silent_skip.rs b/crates/busbar-kernel/tests/no_silent_skip.rs index 3e92bf6654..ecf2b67c7a 100644 --- a/crates/busbar-kernel/tests/no_silent_skip.rs +++ b/crates/busbar-kernel/tests/no_silent_skip.rs @@ -107,7 +107,14 @@ fn literal_end(b: &[u8], i: usize) -> Option { let hashes = j - i - 1; j += 1; while j < b.len() { - if b[j] == b'"' && b[j + 1..].iter().take(hashes).filter(|&&c| c == b'#').count() == hashes { + if b[j] == b'"' + && b[j + 1..] + .iter() + .take(hashes) + .filter(|&&c| c == b'#') + .count() + == hashes + { return Some(j + 1 + hashes); } j += 1;