Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
238 changes: 151 additions & 87 deletions src/assail/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,27 +949,14 @@ impl Analyzer {
weak_points: &mut Vec<WeakPoint>,
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)
Expand Down Expand Up @@ -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();
Expand All @@ -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
};
Expand Down Expand Up @@ -1138,16 +1115,16 @@ impl Analyzer {
});
}

if stats.unwrap_calls > 5 {
if effective_panic_sites > 0 || effective_unwrap_calls > 0 {
weak_points.push(WeakPoint {
file: None,
line: None,
category: WeakPointCategory::PanicPath,
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,
Expand Down Expand Up @@ -1543,14 +1520,15 @@ impl Analyzer {
out
}

/// Strip the bodies of `#[cfg(test)] mod <name> { … }` 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`.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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");
}
"#,
)
Expand All @@ -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"
);
}

Expand Down Expand Up @@ -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();
}
"#,
)
Expand All @@ -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");
}

// ---------------------------------------------------------------
Expand Down Expand Up @@ -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 = "\
Expand Down
Loading
Loading