From bb8f018da75583ed15d66ff4052074364175cd76 Mon Sep 17 00:00:00 2001 From: bordumb Date: Wed, 18 Mar 2026 01:28:35 +0000 Subject: [PATCH] fix: reduce false positives, improve discovery and CI ergonomics --- crates/cargo-capsec/src/cli.rs | 4 ++++ crates/cargo-capsec/src/detector.rs | 24 ++++++++++++++++++++-- crates/cargo-capsec/src/discovery.rs | 13 ++++++++---- crates/cargo-capsec/src/main.rs | 30 ++++++++++++++++++---------- 4 files changed, 54 insertions(+), 17 deletions(-) diff --git a/crates/cargo-capsec/src/cli.rs b/crates/cargo-capsec/src/cli.rs index 2672398..e8f7ebd 100644 --- a/crates/cargo-capsec/src/cli.rs +++ b/crates/cargo-capsec/src/cli.rs @@ -75,4 +75,8 @@ pub struct AuditArgs { /// Skip these crates (comma-separated) #[arg(long)] pub skip: Option, + + /// Suppress output (exit code only, for CI) + #[arg(short, long)] + pub quiet: bool, } diff --git a/crates/cargo-capsec/src/detector.rs b/crates/cargo-capsec/src/detector.rs index 915b7e2..c024da6 100644 --- a/crates/cargo-capsec/src/detector.rs +++ b/crates/cargo-capsec/src/detector.rs @@ -56,8 +56,12 @@ impl Detector { .map(|call| expand_call(&call.segments, &import_map)) .collect(); - // Pass 1: collect path-based findings and build a set of matched paths - // (needed for MethodWithContext co-occurrence checks in pass 2) + // Pass 1: collect path-based findings and build a set of matched patterns. + // We store the *pattern* (e.g. ["Command", "new"]), not the expanded call path. + // This is correct: if someone writes `use std::process::Command; Command::new("sh")`, + // import expansion produces `std::process::Command::new`, which suffix-matches + // the pattern ["Command", "new"]. Pass 2 then checks for pattern co-occurrence, + // so `.output()` fires only when the Command::new *pattern* was matched in pass 1. let mut matched_paths: HashSet> = HashSet::new(); for (call, expanded) in func.calls.iter().zip(expanded_calls.iter()) { @@ -356,6 +360,22 @@ mod tests { } } + #[test] + fn detect_aliased_import() { + let source = r#" + use std::fs::read as load; + fn fetch() { + let _ = load("data.bin"); + } + "#; + let parsed = parse_source(source, "test.rs").unwrap(); + let detector = Detector::new(); + let findings = detector.analyse(&parsed, "test-crate", "0.1.0"); + assert!(!findings.is_empty(), "Should detect aliased import: use std::fs::read as load"); + assert_eq!(findings[0].category, Category::Fs); + assert!(findings[0].call_text.contains("std::fs::read")); + } + #[test] fn detect_impl_block_method() { let source = r#" diff --git a/crates/cargo-capsec/src/discovery.rs b/crates/cargo-capsec/src/discovery.rs index 0532e3e..3658c1e 100644 --- a/crates/cargo-capsec/src/discovery.rs +++ b/crates/cargo-capsec/src/discovery.rs @@ -23,11 +23,16 @@ struct Package { source: Option, } -pub fn discover_crates(workspace_root: &Path) -> Result, String> { - // Don't use --no-deps: we need path dependencies to appear. - // We distinguish local vs registry crates by source == None (local) vs Some (registry). +pub fn discover_crates(workspace_root: &Path, include_deps: bool) -> Result, String> { + // Use --no-deps by default for speed (avoids resolving 300+ transitive deps). + // Drop it when --include-deps is set so path dependencies and registry crates appear. + let mut args = vec!["metadata", "--format-version=1"]; + if !include_deps { + args.push("--no-deps"); + } + let output = Command::new("cargo") - .args(["metadata", "--format-version=1"]) + .args(&args) .current_dir(workspace_root) .output() .map_err(|e| format!("Failed to run cargo metadata: {e}"))?; diff --git a/crates/cargo-capsec/src/main.rs b/crates/cargo-capsec/src/main.rs index 2331223..8345148 100644 --- a/crates/cargo-capsec/src/main.rs +++ b/crates/cargo-capsec/src/main.rs @@ -33,7 +33,7 @@ fn run_audit(args: AuditArgs) { }; // Discover crates - let crates = match discovery::discover_crates(&workspace_root) { + let crates = match discovery::discover_crates(&workspace_root, args.include_deps) { Ok(c) => c, Err(e) => { eprintln!("Error: {e}"); @@ -96,21 +96,30 @@ fn run_audit(args: AuditArgs) { // Apply allow rules all_findings.retain(|f| !config::should_allow(f, &cfg)); + // Load baseline once if needed for diff or fail-on + let baseline_data = if args.diff || args.fail_on.is_some() { + baseline::load_baseline(&workspace_root) + } else { + None + }; + // Diff against baseline if args.diff { - if let Some(bl) = baseline::load_baseline(&workspace_root) { - let diff_result = baseline::diff(&all_findings, &bl); + if let Some(ref bl) = baseline_data { + let diff_result = baseline::diff(&all_findings, bl); baseline::print_diff(&diff_result); } else { eprintln!("No baseline found. Run with --baseline first."); } } - // Report - match args.format.as_str() { - "json" => println!("{}", reporter::report_json(&all_findings)), - "sarif" => println!("{}", reporter::report_sarif(&all_findings)), - _ => reporter::report_text(&all_findings), + // Report (suppress with --quiet) + if !args.quiet { + match args.format.as_str() { + "json" => println!("{}", reporter::report_json(&all_findings)), + "sarif" => println!("{}", reporter::report_sarif(&all_findings)), + _ => reporter::report_text(&all_findings), + } } // Save baseline @@ -126,9 +135,8 @@ fn run_audit(args: AuditArgs) { let threshold = Risk::from_str(fail_level); if args.diff { - // When diffing, only fail on NEW findings above threshold - if let Some(bl) = baseline::load_baseline(&workspace_root) { - let diff_result = baseline::diff(&all_findings, &bl); + if let Some(ref bl) = baseline_data { + let diff_result = baseline::diff(&all_findings, bl); let new_set: std::collections::HashSet<_> = diff_result.new_findings.into_iter().collect(); let has_new_high = all_findings.iter().any(|f| {