From b6b13d1be3b92b6cf39a64b51b8bd1842fc22c3e Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:26:34 +0100 Subject: [PATCH] fix: make production panic controls falsifiable --- src/assail/analyzer.rs | 238 ++++++++++++++++++++++++++--------------- src/kanren/core.rs | 47 ++------ tests/readiness.rs | 84 +++++++++++++++ 3 files changed, 242 insertions(+), 127 deletions(-) diff --git a/src/assail/analyzer.rs b/src/assail/analyzer.rs index 4934f59..9bc610c 100644 --- a/src/assail/analyzer.rs +++ b/src/assail/analyzer.rs @@ -949,27 +949,14 @@ impl Analyzer { weak_points: &mut Vec, file_path: &str, ) -> Result<()> { - // Detect if this is a test file - Expanded patterns - let is_test_file = file_path.contains("_tests.") - || file_path.contains("/tests/") - || file_path.ends_with("_test.rs") - || file_path.starts_with("tests/") - || file_path.contains("/test_") - || file_path.contains("_spec.") // RSpec, Jest patterns - || file_path.contains("_bench.") // Benchmark files - || file_path.contains("/benches/") // Rust bench directory - || file_path.ends_with("_bench.rs") - || file_path.contains("__tests__/") // JavaScript - || file_path.contains("/__tests__/") - || file_path.ends_with(".test.") // Generic test files - || file_path.ends_with(".spec.") - || file_path.contains("_integration.") // Integration tests - || file_path.contains("_e2e.") // End-to-end tests - || file_path.contains("_unit.") // Unit tests - || file_path.contains("/examples/") // Example code (often test-like) - || file_path.contains("/samples/") // Sample code - || file_path.contains("_mock.") // Mock files - || file_path.contains("_stub."); // Stub files + // Use the canonical cross-language classifier rather than a second, + // subtly different Rust-only heuristic. Build scripts are also + // excluded from the panic surface: a build-script panic is a build + // hard-stop, not a production runtime panic in the crate being + // scanned. + let is_test_file = crate::test_context::is_test_path(file_path); + let is_build_script = file_path == "build.rs" || file_path.ends_with("/build.rs"); + let panic_surface_excluded = is_test_file || is_build_script; // Strip string literal contents AND comments before counting so that // detection-tool source files (which embed patterns as string literals) @@ -997,7 +984,9 @@ impl Analyzer { // crypto counts too (a `#[test] fn exercises_unsafe_wrapper()` // inside a production file would otherwise count toward that // file's unsafe-block total). - let code_only = Self::strip_cfg_test_modules_rs(&without_comments); + let code_only = Self::strip_should_panic_items_rs( + &Self::strip_cfg_test_modules_rs(&without_comments), + ); stats.unsafe_blocks += code_only.matches("unsafe {").count(); stats.unsafe_blocks += code_only.matches("unsafe fn").count(); @@ -1016,24 +1005,12 @@ impl Analyzer { let unwrap_calls = raw_unwrap + code_only.matches(".expect(").count(); let safe_unwrap_calls = safe_unwrap; - // Apply test file suppression. In test files, normal - // assert-macro use pushes panic/unwrap counts high with no - // production-code signal, so we suppress the counts unless they - // exceed a "clearly excessive" threshold — only the delta above - // the threshold is attributed to the file's statistics. - // - // Panic and unwrap thresholds are evaluated independently: a - // test file with 30 panics + 5 unwraps should report 10 panics - // (30 − 20) and 0 unwraps, not suppress everything because the - // unwrap count is normal. The previous version declared both - // thresholds but only compared panics, silently dropping any - // excessive unwrap signal. - let (effective_panic_sites, effective_unwrap_calls) = if is_test_file { - let panic_threshold = 20; - let unwrap_threshold = 10; - let eff_panics = panic_sites.saturating_sub(panic_threshold); - let eff_unwraps = unwrap_calls.saturating_sub(unwrap_threshold); - (eff_panics, eff_unwraps) + // Test, benchmark, #[should_panic], and build-script code is not part + // of the production panic surface. Suppress it completely; a + // threshold-based exception makes a sufficiently large test suite + // look like production code and defeats the precision contract. + let (effective_panic_sites, effective_unwrap_calls) = if panic_surface_excluded { + (0, 0) } else { (panic_sites, unwrap_calls) // Production code: count all }; @@ -1138,7 +1115,7 @@ impl Analyzer { }); } - if stats.unwrap_calls > 5 { + if effective_panic_sites > 0 || effective_unwrap_calls > 0 { weak_points.push(WeakPoint { file: None, line: None, @@ -1146,8 +1123,8 @@ impl Analyzer { location: Some(file_path.to_string()), severity: Severity::Medium, description: format!( - "{} unwrap/expect calls in {}", - stats.unwrap_calls, file_path + "{} panic sites and {} unwrap/expect calls in {}", + effective_panic_sites, effective_unwrap_calls, file_path ), recommended_attack: vec![AttackAxis::Memory, AttackAxis::Disk], suppressed: false, @@ -1543,14 +1520,15 @@ impl Analyzer { out } - /// Strip the bodies of `#[cfg(test)] mod { … }` blocks from - /// `content`, leaving the attribute, `mod` keyword, name, and - /// enclosing braces in place but replacing everything between the + /// Strip the bodies of `#[cfg(test)]` modules and functions from + /// `content`, leaving the attribute, item signature, and enclosing + /// braces in place but replacing everything between the /// braces with whitespace. Newlines in the body are preserved so /// line numbers downstream stay stable. /// /// This is the Rust analogue of Zig's `count_unsafe_in_test_blocks` - /// and is used to treat an inline `#[cfg(test)] mod tests { … }` + /// and is used to treat an inline `#[cfg(test)] mod tests { … }` or + /// `#[cfg(test)] fn test() { … }` /// inside a production file as test context for substring-based /// dangerous-pattern checks — the same way a whole file under /// `/tests/` is already treated as test context by `is_test_file`. @@ -1628,26 +1606,42 @@ impl Analyzer { } } - if k + 4 > n || &bytes[k..k + 4] != b"mod " { + let is_mod = k + 4 <= n && &bytes[k..k + 4] == b"mod "; + let is_fn = (k + 3 <= n && &bytes[k..k + 3] == b"fn ") + || (k + 8 <= n && &bytes[k..k + 8] == b"async fn"); + if !is_mod && !is_fn { out.extend_from_slice(&bytes[attr_start..attr_end]); i = attr_end; continue; } - k += 4; - while k < n && (bytes[k] as char).is_whitespace() { - k += 1; - } - while k < n && (bytes[k].is_ascii_alphanumeric() || bytes[k] == b'_') { - k += 1; - } - while k < n && (bytes[k] as char).is_whitespace() { - k += 1; - } - - if k >= n || bytes[k] != b'{' { - out.extend_from_slice(&bytes[attr_start..attr_end]); - i = attr_end; - continue; + if is_mod { + k += 4; + while k < n && (bytes[k] as char).is_whitespace() { + k += 1; + } + while k < n && (bytes[k].is_ascii_alphanumeric() || bytes[k] == b'_') { + k += 1; + } + while k < n && (bytes[k] as char).is_whitespace() { + k += 1; + } + if k >= n || bytes[k] != b'{' { + out.extend_from_slice(&bytes[attr_start..attr_end]); + i = attr_end; + continue; + } + } else { + // A function signature has parameters and possibly a return + // type between its name and body; scan to the first body + // brace while leaving declarations without a body untouched. + while k < n && bytes[k] != b'{' && bytes[k] != b';' { + k += 1; + } + if k >= n || bytes[k] != b'{' { + out.extend_from_slice(&bytes[attr_start..attr_end]); + i = attr_end; + continue; + } } let body_start = k + 1; @@ -1678,6 +1672,78 @@ impl Analyzer { String::from_utf8_lossy(&out).into_owned() } + /// Strip the bodies of `#[should_panic]` items from Rust source while + /// preserving line numbers. `#[should_panic]` is meaningful on test + /// functions even when the function is not inside a `#[cfg(test)]` module + /// or a conventional `tests/` path. + fn strip_should_panic_items_rs(content: &str) -> String { + let bytes = content.as_bytes(); + let n = bytes.len(); + let mut out = Vec::with_capacity(n); + let mut i = 0; + + while i < n { + if i + 14 > n || &bytes[i..i + 14] != b"#[should_panic" { + out.push(bytes[i]); + i += 1; + continue; + } + + let attr_start = i; + let mut attr_end = i + 14; + while attr_end < n && bytes[attr_end] != b']' { + attr_end += 1; + } + if attr_end >= n { + out.push(bytes[i]); + i += 1; + continue; + } + attr_end += 1; + + let mut body_start = attr_end; + while body_start < n && (bytes[body_start] as char).is_whitespace() { + body_start += 1; + } + let mut brace = body_start; + while brace < n && bytes[brace] != b'{' && bytes[brace] != b';' { + brace += 1; + } + // A malformed attribute or an item without a body is left + // untouched so the detector fails open. + if brace >= n || bytes[brace] != b'{' { + out.extend_from_slice(&bytes[attr_start..attr_end]); + i = attr_end; + continue; + } + + let mut depth = 1i32; + let mut end = brace + 1; + while end < n && depth > 0 { + match bytes[end] { + b'{' => depth += 1, + b'}' => depth -= 1, + _ => {} + } + end += 1; + } + if depth != 0 { + out.extend_from_slice(&bytes[attr_start..attr_end]); + i = attr_end; + continue; + } + + out.extend_from_slice(&bytes[attr_start..brace + 1]); + for byte in &bytes[brace + 1..end - 1] { + out.push(if *byte == b'\n' { b'\n' } else { b' ' }); + } + out.push(b'}'); + i = end; + } + + String::from_utf8_lossy(&out).into_owned() + } + /// Classify the argument list of `#[cfg(...)]` as selecting for /// `test` (true) or not (false). Uses [`strip_not_test_groups`] to /// drop `not(test)` groups first, then looks for a bareword `test`. @@ -6566,26 +6632,18 @@ fn safe_wrapper() { } // --------------------------------------------------------------- - // 5. analyze() on a Rust file containing `.unwrap()` — PanicPath + // 5. analyze() on a Rust file containing one explicit panic — PanicPath // --------------------------------------------------------------- #[test] - fn analyze_rust_detects_panic_path_from_unwrap() { + fn analyze_rust_detects_panic_path_from_explicit_panic() { let tmp = TempDir::new().unwrap(); - let rust_file = tmp.path().join("unwrappy.rs"); - // The analyzer triggers PanicPath when unwrap_calls > 5, - // so we need at least 6 unwrap calls. + let rust_file = tmp.path().join("panic_control.rs"); fs::write( &rust_file, r#" -fn lots_of_unwraps() { - let a = Some(1).unwrap(); - let b = Some(2).unwrap(); - let c = Some(3).unwrap(); - let d = Some(4).unwrap(); - let e = Some(5).unwrap(); - let f = Some(6).unwrap(); - let g = Some(7).unwrap(); +fn production_positive_control() { + panic!("production positive control"); } "#, ) @@ -6602,7 +6660,7 @@ fn lots_of_unwraps() { assert!( !panic_points.is_empty(), - "Should detect PanicPath weak point when >5 unwrap() calls are present" + "Should detect a single production panic" ); } @@ -6768,21 +6826,19 @@ func main() { } // --------------------------------------------------------------- - // 11. Rust file with few unwraps should NOT trigger PanicPath + // 11. Rust file with one production unwrap should trigger PanicPath // --------------------------------------------------------------- #[test] - fn analyze_rust_few_unwraps_no_panic_path() { + fn analyze_rust_single_unwrap_triggers_panic_path() { let tmp = TempDir::new().unwrap(); let rust_file = tmp.path().join("safe.rs"); - // Only 3 unwrap calls — threshold is >5 + // A single production unwrap is sufficient for the recall contract. fs::write( &rust_file, r#" -fn few_unwraps() { +fn one_unwrap() { let a = Some(1).unwrap(); - let b = Some(2).unwrap(); - let c = Some(3).unwrap(); } "#, ) @@ -6797,10 +6853,7 @@ fn few_unwraps() { .filter(|wp| wp.category == WeakPointCategory::PanicPath) .collect(); - assert!( - panic_points.is_empty(), - "Should NOT trigger PanicPath when unwrap count is <= 5" - ); + assert!(!panic_points.is_empty(), "A production unwrap must be reported"); } // --------------------------------------------------------------- @@ -7519,6 +7572,17 @@ trailing_production_code(); assert!(out.contains("fn prod")); } + #[test] + fn strip_cfg_test_function_and_should_panic_bodies() { + let src = "#[cfg(test)]\nfn cfg_test() { panic!(\"cfg\"); }\n#[should_panic]\nfn expected() { panic!(\"expected\"); }\nfn prod() { panic!(\"prod\"); }"; + let cfg_stripped = Analyzer::strip_cfg_test_modules_rs(src); + let out = Analyzer::strip_should_panic_items_rs(&cfg_stripped); + assert!(!out.contains("panic!(\"cfg\")")); + assert!(!out.contains("panic!(\"expected\")")); + assert!(out.contains("panic!(\"prod\")")); + assert_eq!(out.lines().count(), src.lines().count()); + } + #[test] fn strip_cfg_test_modules_handles_pub_mod() { let src = "\ diff --git a/src/kanren/core.rs b/src/kanren/core.rs index 887e30b..1de1ee8 100644 --- a/src/kanren/core.rs +++ b/src/kanren/core.rs @@ -570,27 +570,6 @@ impl LogicEngine { }, )); - // Rule 2: suppress_unwrap_boundary(File) :- - // weak_point(PanicPath, File, _), - // context(File, "result_returning_fn"). - let v2 = Term::Var(202); - let v3 = Term::Var(203); - self.db.add_rule(LogicRule::with_metadata( - "suppress_unwrap_boundary".into(), - LogicFact::new("suppressed", vec![Term::atom("PanicPath"), v2.clone()]), - vec![ - LogicFact::new("weak_point", vec![Term::atom("PanicPath"), v2.clone(), v3]), - LogicFact::new( - "context", - vec![v2.clone(), Term::atom("result_returning_fn")], - ), - ], - RuleMetadata { - confidence: 0.90, - ..Default::default() - }, - )); - // Rule 3: suppress_validated_deser(File) :- // weak_point(UnsafeDeserialization, File, _), // context(File, "schema_validated"). @@ -932,7 +911,6 @@ impl LogicEngine { /// - `test_file`: path contains test/spec indicators (singular _test.rs or plural _tests.rs) /// - `mutex_guarded`: file has threading constructs /// - `raii_managed`: Rust files (RAII by default) - /// - `result_returning_fn`: Rust files with unwrap calls /// - `pest_parser`: parser file with high unwrap count + zero unsafe blocks (grammar-safe) /// - `ffi_safe_wrapper`: Rust FFI wrapper around extern-C boundary with /// documented SAFETY invariants (set by `is_rust_ffi_safe_wrapper` during @@ -988,14 +966,6 @@ impl LogicEngine { )); } - // Result-returning function detection (Rust files with unwrap) - if is_rust && fs.unwrap_calls > 0 { - self.db.assert_fact(LogicFact::new( - "context", - vec![Term::atom(path), Term::atom("result_returning_fn")], - )); - } - // Pest-parser context: Rust parser files whose unwraps are // structurally guaranteed by the grammar (pest Rule pairs always // return Some on a successfully matched production — no runtime @@ -1611,11 +1581,10 @@ mod tests { "tests/integration_test.rs", "raii_managed" )); - assert!(has_context( - &engine, - "tests/integration_test.rs", - "result_returning_fn" - )); + assert!( + !has_context(&engine, "tests/integration_test.rs", "result_returning_fn"), + "unwrap presence alone must not prove a Result-returning boundary" + ); // New description-based fact assert!(has_context( @@ -1767,11 +1736,9 @@ mod tests { unsafe_wp.suppressed, "UnsafeCode must be suppressed by Rule 13" ); - // PanicPath may or may not be suppressed by Rule 2 (depends on - // whether unwrap_calls > 0 triggered result_returning_fn), but - // Rule 13 specifically must not suppress it. Assert only that - // Rule 13's action is category-scoped by checking the rule's - // own condition: UnsafeCode got suppressed, PanicPath exists. + // Rule 13 specifically must not suppress PanicPath. Assert only that + // its action is category-scoped by checking that UnsafeCode got + // suppressed while PanicPath remains visible. let _ = panic_wp; // presence already asserted above } } diff --git a/tests/readiness.rs b/tests/readiness.rs index 02518de..e1e4df4 100644 --- a/tests/readiness.rs +++ b/tests/readiness.rs @@ -239,6 +239,90 @@ fn main() { Ok(()) } +#[test] +fn readiness_c_assail_panic_surface_has_precision_and_recall_controls( +) -> Result<(), Box> { + let dir = TempDir::new()?; + fs::create_dir_all(dir.path().join("src"))?; + fs::create_dir_all(dir.path().join("tests"))?; + fs::create_dir_all(dir.path().join("benches"))?; + + fs::write( + dir.path().join("src/positive_control.rs"), + r#" +pub fn production_positive_control() { + panic!("production panic"); +} + +#[should_panic] +fn expected_panic_control() { + panic!("test panic"); +} + +#[cfg(test)] +fn cfg_test_panic() { + None::<()>.unwrap(); +} +"#, + )?; + fs::write( + dir.path().join("tests/integration.rs"), + "#[should_panic]\nfn integration_panic() { panic!(\"expected\"); }\n", + )?; + fs::write( + dir.path().join("benches/scan.rs"), + "fn benchmark_setup() { None::<()>.unwrap(); }\n", + )?; + fs::write( + dir.path().join("build.rs"), + "fn main() { panic!(\"build stop\"); }\n", + )?; + + let report = assail::analyze(dir.path())?; + let active: Vec<_> = report + .weak_points + .iter() + .filter(|wp| wp.category == WeakPointCategory::PanicPath && !wp.suppressed) + .collect(); + assert_eq!(active.len(), 1, "only the production control should remain active"); + assert_eq!( + active[0].location.as_deref(), + Some("src/positive_control.rs") + ); + + fs::write( + dir.path().join("src/positive_control.rs"), + "pub fn production_unwrap_control() { None::<()>.unwrap(); }\n#[should_panic]\nfn expected() { panic!(\"test\"); }\n", + )?; + let unwrap_report = assail::analyze(dir.path())?; + let active_unwrap: Vec<_> = unwrap_report + .weak_points + .iter() + .filter(|wp| wp.category == WeakPointCategory::PanicPath && !wp.suppressed) + .collect(); + assert_eq!( + active_unwrap.len(), + 1, + "one production unwrap must remain visible" + ); + + fs::write( + dir.path().join("src/positive_control.rs"), + "pub fn clean_production() {}\n#[should_panic]\nfn expected() { panic!(\"test\"); }\n", + )?; + let clean_report = assail::analyze(dir.path())?; + assert!( + clean_report + .weak_points + .iter() + .filter(|wp| wp.category == WeakPointCategory::PanicPath && !wp.suppressed) + .next() + .is_none(), + "excluded panic contexts must produce zero active findings" + ); + Ok(()) +} + #[test] fn readiness_c_assail_json_output() -> Result<(), Box> { let dir = TempDir::new()?;