diff --git a/README.md b/README.md index 714103e..6db085b 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ [![crates.io](https://img.shields.io/crates/v/pysentry)](https://crates.io/crates/pysentry) [![Downloads](https://static.pepy.tech/badge/pysentry-rs/week)](https://pepy.tech/projects/pysentry-rs) -[**Documentation**](https://nyudenkov.github.io/pysentry/) · [**Benchmarks**](benchmarks/results/) · [Help test & improve](https://github.com/nyudenkov/pysentry/issues/12) · [Usage survey](https://tally.so/r/mYNPNv) +[**Documentation**](https://docs.pysentry.com) · [**Benchmarks**](benchmarks/results/) · [Help test & improve](https://github.com/nyudenkov/pysentry/issues/12) · [Usage survey](https://tally.so/r/mYNPNv) @@ -43,7 +43,7 @@ pip install pysentry-rs # PyPI cargo install pysentry # crates.io ``` -Pre-built binaries are attached to [GitHub Releases](https://github.com/nyudenkov/pysentry/releases). See the [installation guide](https://nyudenkov.github.io/pysentry/getting-started/installation) for all options. +Pre-built binaries are attached to [GitHub Releases](https://github.com/nyudenkov/pysentry/releases). See the [installation guide](https://docs.pysentry.com/getting-started/installation) for all options. > **Naming:** the Python package installs the binary as `pysentry-rs`; the Rust crate and release binaries are plain `pysentry`. Examples below use `pysentry-rs` — substitute accordingly. @@ -69,7 +69,7 @@ pysentry-rs --format sarif --output results.sarif pysentry-rs --forbid-quarantined ``` -More examples in the [quickstart guide](https://nyudenkov.github.io/pysentry/getting-started/quickstart). +More examples in the [quickstart guide](https://docs.pysentry.com/getting-started/quickstart). ## Pre-commit @@ -97,7 +97,7 @@ steps: fail-on: high ``` -On any other CI system, `pysentry-rs --fail-on high` exits non-zero when findings reach the threshold. Details in the [CI guide](https://nyudenkov.github.io/pysentry/ci). +On any other CI system, `pysentry-rs --fail-on high` exits non-zero when findings reach the threshold. Details in the [CI guide](https://docs.pysentry.com/ci). ## Configuration @@ -117,17 +117,17 @@ enabled = ["pypa", "osv"] ids = ["CVE-2023-12345"] ``` -All options are covered in the [configuration guide](https://nyudenkov.github.io/pysentry/configuration/config-files). +All options are covered in the [configuration guide](https://docs.pysentry.com/configuration/config-files). ## Documentation -Full documentation lives at [https://nyudenkov.github.io/pysentry/](https://nyudenkov.github.io/pysentry): -[Installation](https://nyudenkov.github.io/pysentry/getting-started/installation) · -[Quickstart](https://nyudenkov.github.io/pysentry/getting-started/quickstart) · -[CLI options](https://nyudenkov.github.io/pysentry/configuration/cli-options) · -[Configuration files](https://nyudenkov.github.io/pysentry/configuration/config-files) · -[Environment variables](https://nyudenkov.github.io/pysentry/configuration/environment-variables) · -[Troubleshooting](https://nyudenkov.github.io/pysentry/troubleshooting) +Full documentation lives at [https://docs.pysentry.com](https://docs.pysentry.com): +[Installation](https://docs.pysentry.com/getting-started/installation) · +[Quickstart](https://docs.pysentry.com/getting-started/quickstart) · +[CLI options](https://docs.pysentry.com/configuration/cli-options) · +[Configuration files](https://docs.pysentry.com/configuration/config-files) · +[Environment variables](https://docs.pysentry.com/configuration/environment-variables) · +[Troubleshooting](https://docs.pysentry.com/troubleshooting) ## Requirements diff --git a/docs/docusaurus.config.ts b/docs/docusaurus.config.ts index b08f44a..7d6e483 100644 --- a/docs/docusaurus.config.ts +++ b/docs/docusaurus.config.ts @@ -11,8 +11,8 @@ const config: Config = { v4: true, }, - url: 'https://nyudenkov.github.io', - baseUrl: '/pysentry/', + url: 'https://docs.pysentry.com', + baseUrl: '/', organizationName: 'nyudenkov', projectName: 'pysentry', diff --git a/src/audit/merge.rs b/src/audit/merge.rs index c339246..170dbd0 100644 --- a/src/audit/merge.rs +++ b/src/audit/merge.rs @@ -125,6 +125,33 @@ impl AuditArgs { ignore_while_no_fix.extend(config.ignore.while_no_fix.clone()); merged.ignore_while_no_fix = ignore_while_no_fix; + let mut ignore_packages = self.ignore_packages.clone(); + ignore_packages.extend(config.ignore.packages.clone()); + merged.ignore_packages = ignore_packages; + + // fail_on_partial defaults to true (fail-closed); the CLI flag and config + // can only relax it, matching the "flags turn ON" idiom (cf. no_fail_on_unknown). + if !self.no_fail_on_partial && !config.sources.fail_on_partial { + merged.no_fail_on_partial = true; + } + + // Per-group fail thresholds (config-only). Normalize keys to PEP 735 form so + // they compare against graph attribution's normalized group names. Levels were + // validated at config load; the fallback keeps this infallible. + merged.group_fail_on = config + .groups + .iter() + .map(|(name, policy)| { + // invariant: levels were validated in Config::validate at load, so parse + // cannot fail here; the fallback keeps this map infallible without a panic. + let level = policy.fail_on.parse().unwrap_or(SeverityLevel::Medium); + ( + crate::parsers::manifest_reader::normalize_group_name(name), + level, + ) + }) + .collect(); + // CLI -v flag overrides config quiet. Only apply config quiet when not explicitly verbose. if config.output.quiet && !crate::logging::is_verbose(&self.verbosity) { merged.config_quiet = true; @@ -396,6 +423,60 @@ mod tests { assert!(merged.direct_only); } + #[test] + fn test_fail_on_partial_config_relaxes_default() { + let args = parse_audit_args(&["."]); + let mut config = crate::config::Config::default(); + config.sources.fail_on_partial = false; + let merged = args.merge_with_config(&config); + assert!(merged.no_fail_on_partial); + } + + #[test] + fn test_fail_on_partial_strict_by_default() { + let args = parse_audit_args(&["."]); + let config = crate::config::Config::default(); // fail_on_partial = true + let merged = args.merge_with_config(&config); + assert!(!merged.no_fail_on_partial); + } + + #[test] + fn test_cli_no_fail_on_partial_overrides_config_strict() { + let args = parse_audit_args(&["--no-fail-on-partial", "."]); + let config = crate::config::Config::default(); // fail_on_partial = true + let merged = args.merge_with_config(&config); + assert!(merged.no_fail_on_partial); + } + + #[test] + fn test_ignore_packages_merged_from_config() { + let args = parse_audit_args(&["."]); + let mut config = crate::config::Config::default(); + config.ignore.packages = vec!["internal-pkg".to_string()]; + let merged = args.merge_with_config(&config); + assert_eq!(merged.ignore_packages, vec!["internal-pkg"]); + } + + // compact-XOR-detailed must hold on the merged/effective config (#174/Q16). + // No guard exists in perform_audit: Config::validate rejects both-true at load, + // and merge's precedence clears the loser. These cases pin that invariant. + #[test] + fn test_compact_detailed_mutually_exclusive_post_merge() { + // CLI --compact + config detailed. + let args = parse_audit_args(&["--compact", "."]); + let mut config = crate::config::Config::default(); + config.defaults.detailed = true; + let merged = args.merge_with_config(&config); + assert!(!(merged.compact && merged.detailed)); + + // CLI --detailed + config compact. + let args = parse_audit_args(&["--detailed", "."]); + let mut config = crate::config::Config::default(); + config.defaults.compact = true; + let merged = args.merge_with_config(&config); + assert!(!(merged.compact && merged.detailed)); + } + #[test] fn test_empty_groups_does_not_force_direct_only() { let args = parse_audit_args(&["."]); diff --git a/src/audit/pipeline.rs b/src/audit/pipeline.rs index 6e77468..3711168 100644 --- a/src/audit/pipeline.rs +++ b/src/audit/pipeline.rs @@ -12,14 +12,15 @@ use crate::output::generate_report; use crate::parsers::manifest_reader; use crate::parsers::requirements::RequirementsParser; use crate::parsers::{ParserRegistry, ProjectParser}; -use crate::types::ResolverType; +use crate::types::{PackageName, ResolverType}; +use crate::vulnerability::database::SuppressionReason; use crate::{ AuditCache, AuditReport, DependencyScanner, MatcherConfig, Severity, VulnerabilityDatabase, VulnerabilityMatch, VulnerabilityMatcher, VulnerabilitySource, }; use anyhow::Result; -use futures::future::try_join_all; -use std::collections::HashSet; +use futures::future::join_all; +use std::collections::{BTreeMap, HashSet}; use std::path::{Path, PathBuf}; /// Findings at/above the `fail_on` threshold (or failing maintenance issues) @@ -211,6 +212,16 @@ pub async fn audit( let maintenance_config = audit_args.maintenance_check_config(); let fail_maintenance = report.should_fail_on_maintenance(&maintenance_config); + // Partial scan under strict `fail_on_partial` (the default): a source failed, so the scan + // is incomplete. Fail-closed with a system error (exit 2) — the report was already printed + // with the partial marker above, so the incompleteness is visible before we exit. + if partial_scan_should_fail( + !report.failed_sources.is_empty(), + audit_args.no_fail_on_partial, + ) { + return Ok(EXIT_ERROR); + } + if fail_vulns || fail_maintenance { Ok(EXIT_VULNERABILITIES_FOUND) } else { @@ -234,28 +245,107 @@ fn build_matcher_config(audit_args: &AuditArgs) -> MatcherConfig { ) } -/// Evaluate whether any match triggers the fail_on exit condition. -/// Returns (matches, should_fail). -pub(crate) fn evaluate_fail_condition( - matches: Vec, - fail_on: &crate::SeverityLevel, - fail_on_unknown: bool, -) -> (Vec, bool) { - let fail_on_db = match fail_on { +/// Whether a partial scan (at least one source failed to fetch) should exit with a system +/// error. Fail-closed by default: leniency (`no_fail_on_partial`) is opt-in. +fn partial_scan_should_fail(has_failed_sources: bool, no_fail_on_partial: bool) -> bool { + has_failed_sources && !no_fail_on_partial +} + +fn severity_level_to_db(level: &crate::SeverityLevel) -> Severity { + match level { crate::SeverityLevel::Low => Severity::Low, crate::SeverityLevel::Medium => Severity::Medium, crate::SeverityLevel::High => Severity::High, crate::SeverityLevel::Critical => Severity::Critical, - }; + } +} + +/// A finding's effective fail threshold under strictest-wins: the minimum (strictest) +/// threshold across every context that reaches its package. +/// +/// The contexts are the reaching dependency groups (each contributing its `[groups.*]` +/// threshold) and — only when the package is **main-reachable** — the main/prod context, +/// contributing the global `fail_on`. A package that ships to production therefore keeps +/// `global` as a floor (a permissive group can only tighten it), while a group-only +/// package takes its group threshold outright, which may be *looser* than global (closes +/// #151: "fail only on critical for dev"). With no reaching group carrying a policy, the +/// global default applies. +fn effective_threshold( + finding: &VulnerabilityMatch, + global: Severity, + group_thresholds: &BTreeMap, + main_reachable: &HashSet, +) -> Severity { + let group_min = finding + .groups + .iter() + .filter_map(|group| group_thresholds.get(group).copied()) + .min(); + + match group_min { + // Main-reachable: global is a floor the group policy can only tighten. + Some(min) if main_reachable.contains(&finding.package_name) => min.min(global), + // Group-only: the group policy stands alone and may loosen below global. + Some(min) => min, + // No reaching group carries a policy → the global default. + None => global, + } +} - let fail_vulns = matches.iter().any(|m| { +/// Evaluate whether any (non-suppressed) match triggers the fail_on exit condition. +/// +/// `group_thresholds` (normalized group name → severity) applies per-group policy via +/// strictest-wins; `main_reachable` gates whether the global threshold floors a finding +/// (see [`effective_threshold`]). Pass an empty map for global-threshold-only behavior. +/// This selects the exit condition ONLY — it never filters what is reported (the fail_on +/// invariant). +pub(crate) fn evaluate_fail_condition( + matches: &[VulnerabilityMatch], + global_fail_on: &crate::SeverityLevel, + group_thresholds: &BTreeMap, + main_reachable: &HashSet, + fail_on_unknown: bool, +) -> bool { + let global_db = severity_level_to_db(global_fail_on); + + matches.iter().any(|m| { + if m.suppressed.is_some() { + return false; + } + // Unknown severity ignores thresholds entirely (pre-policy behavior). Short-circuit + // BEFORE the min computation: `Unknown` is the lowest Severity variant, so folding it + // into the effective threshold would collapse any policied finding to fail-on-everything. if m.vulnerability.is_level_unknown() { return fail_on_unknown; } - m.vulnerability.meets_level(fail_on_db) - }); + m.vulnerability.meets_level(effective_threshold( + m, + global_db, + group_thresholds, + main_reachable, + )) + }) +} - (matches, fail_vulns) +/// Suppress findings whose package matches an `[ignore].packages` entry — marked, never +/// dropped, so they still appear in the report but don't trigger the exit condition. +/// Names compared via `PackageName` (PEP 503), never raw strings. A `packages` entry that +/// matches nothing warns, mirroring the unmatched-ignore-id warning. +fn apply_package_ignores(matches: &mut [VulnerabilityMatch], ignore_packages: &[String]) { + for raw in ignore_packages { + // invariant: entries are validated in Config::validate, so this normalizes cleanly. + let ignored = PackageName::new(raw); + let mut hit = false; + for m in matches.iter_mut() { + if m.package_name == ignored { + m.suppressed = Some(SuppressionReason::IgnoredPackage); + hit = true; + } + } + if !hit { + tracing::warn!("ignore package '{}' did not match any finding", raw); + } + } } #[cfg_attr(feature = "hotpath", hotpath::measure)] @@ -601,7 +691,10 @@ async fn perform_audit( let fetch_tasks = vuln_sources.into_iter().map(|source| { let packages = packages.clone(); - async move { source.fetch_vulnerabilities(&packages).await } + async move { + let name = source.name(); + (name, source.fetch_vulnerabilities(&packages).await) + } }); // Fetch maintenance status (PEP 792) in parallel if enabled @@ -632,11 +725,47 @@ async fn perform_audit( } }; - // Run vulnerability fetching and maintenance checks in parallel - let (vuln_result, maintenance_issues) = - tokio::join!(try_join_all(fetch_tasks), maintenance_future); + // Run vulnerability fetching and maintenance checks in parallel. Sources are collected + // per-source (not `try_join_all` + `?`) so a single source failure does not abort the + // whole audit: it becomes a "partial" scan whose handling depends on `fail_on_partial`. + let (fetch_results, maintenance_issues) = + tokio::join!(join_all(fetch_tasks), maintenance_future); + + let mut databases = Vec::new(); + let mut failures: Vec<(&'static str, anyhow::Error)> = Vec::new(); + for (name, result) in fetch_results { + match result { + Ok(db) => databases.push(db), + Err(e) => failures.push((name, e.into())), + } + } + + // Total failure (every source down, or the only source in a single-source run) is always + // a hard error regardless of `fail_on_partial` — there is nothing to report on. + if databases.is_empty() { + return match failures.into_iter().next() { + Some((name, err)) => { + Err(err.context(format!("all vulnerability sources failed (first: {name})"))) + } + // Unreachable: an empty `databases` implies >= 1 failed source, since there is + // always >= 1 configured source. A defensive error, never a panic. + None => Err(anyhow::anyhow!("no vulnerability data could be fetched")), + }; + } - let databases = vuln_result?; + // Partial scan: some sources failed but at least one succeeded. Warn loudly and carry the + // failed-source names into the report; the exit gate (`no_fail_on_partial`) is applied by + // the caller after the report — an incomplete scan is never silent. + let failed_sources: Vec = failures + .iter() + .map(|(name, _)| (*name).to_string()) + .collect(); + for (name, err) in &failures { + tracing::warn!("vulnerability source '{name}' failed to fetch: {err}"); + if !audit_args.is_quiet() { + eprintln!("Warning: vulnerability source '{name}' failed to fetch: {err}"); + } + } if databases.len() > 1 && !audit_args.is_quiet() { eprintln!( @@ -658,15 +787,61 @@ async fn perform_audit( let matcher = VulnerabilityMatcher::new(database, matcher_config); let matches = matcher.find_vulnerabilities(&dependencies)?; - let filtered_matches = matcher.filter_matches(matches); + let mut display_matches = matcher.filter_matches(matches); for ignore_id in matcher.unmatched_ignore_ids() { tracing::warn!("ignore ID '{}' did not match any finding", ignore_id); } - let (display_matches, fail_vulns) = evaluate_fail_condition( - filtered_matches, + // Per-group attribution and main-reachability are only consumed by per-group policy. + // When no `[groups.*]` threshold is configured they can never change the outcome, so + // skip the extra lock/manifest parses entirely (the common case). + let mut main_reachable: HashSet = HashSet::new(); + if !audit_args.group_fail_on.is_empty() { + // Tag each finding with the dependency groups (PEP 735 / Poetry) that reach its + // package, for per-group policy thresholds. Never filters `display_matches`. + let group_attribution = + crate::parsers::graph::build_group_attribution(&audit_args.path, &detected_parser_name) + .await; + for finding in &mut display_matches { + if let Some(groups) = group_attribution.get(&finding.package_name) { + finding.groups = groups.iter().cloned().collect(); + } + } + + // The main/prod reachability set: a finding in one of these packages keeps the + // global `fail_on` as a floor, so a permissive group threshold cannot loosen it. + main_reachable = + crate::parsers::graph::build_main_reachable(&audit_args.path, &detected_parser_name) + .await; + + // Per-group policy is set but the project has no group-aware lock, so attribution is + // empty and the thresholds can never apply. Warn loudly rather than silently ignoring. + if !crate::parsers::has_group_aware_lock(&audit_args.path) { + let msg = "per-group fail thresholds ([groups.*]) are configured but this project \ + has no group-aware lock (uv.lock, poetry.lock, pylock.toml); the \ + thresholds are ignored"; + tracing::warn!("{msg}"); + if !audit_args.is_quiet() { + eprintln!("Warning: {msg}"); + } + } + } + + // Suppress [ignore].packages matches (marked, never dropped) before evaluating the + // exit condition, so a suppressed finding is still reported but never fails the run. + apply_package_ignores(&mut display_matches, &audit_args.ignore_packages); + + let group_thresholds: BTreeMap = audit_args + .group_fail_on + .iter() + .map(|(name, level)| (name.clone(), severity_level_to_db(&level.clone().into()))) + .collect(); + let fail_vulns = evaluate_fail_condition( + &display_matches, &fail_on_level, + &group_thresholds, + &main_reachable, !audit_args.no_fail_on_unknown, ); @@ -689,7 +864,8 @@ async fn perform_audit( warnings, maintenance_issues, ) - .with_transitive_roots(transitive_roots); + .with_transitive_roots(transitive_roots) + .with_failed_sources(failed_sources); let summary = report.summary(); let maint_summary = report.maintenance_summary(); @@ -834,10 +1010,27 @@ fn should_skip_script_dir(name: &str) -> bool { #[cfg(test)] mod tests { - use super::{evaluate_fail_condition, scan_pep723_scripts}; + use super::{ + apply_package_ignores, build_matcher_config, evaluate_fail_condition, + partial_scan_should_fail, scan_pep723_scripts, + }; + use crate::types::PackageName; + use crate::vulnerability::database::SuppressionReason; use crate::{Severity, VulnerabilityMatch}; + use std::collections::{BTreeMap, HashSet}; use std::str::FromStr; + #[test] + fn test_partial_scan_strict_fails_and_lenient_continues() { + // Strict (default): a failed source is an incomplete scan → fail (exit 2). + assert!(partial_scan_should_fail(true, false)); + // Lenient (--no-fail-on-partial): continue by findings despite the failed source. + assert!(!partial_scan_should_fail(true, true)); + // No failures: the knob is irrelevant, never fail on this account. + assert!(!partial_scan_should_fail(false, false)); + assert!(!partial_scan_should_fail(false, true)); + } + // Calling --group on a project with no pyproject.toml must return a clear error // before any dependency scanning begins. #[tokio::test] @@ -1002,34 +1195,231 @@ mod tests { vulnerability: crate::vulnerability::database::Vulnerability::with_level(vuln_level), is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, } } - // HIGH meets fail_on=Medium → should fail, match is returned. + fn no_groups() -> BTreeMap { + BTreeMap::new() + } + + // The default finding package ("test-pkg") is treated as group-only (not shipped to + // prod) unless a test opts it into main-reachability. + fn no_main() -> HashSet { + HashSet::new() + } + + fn make_grouped_match(vuln_level: Severity, groups: &[&str]) -> VulnerabilityMatch { + let mut m = make_match(vuln_level); + m.groups = groups.iter().map(|g| g.to_string()).collect(); + m + } + + // HIGH meets fail_on=Medium → should fail. #[test] fn test_fail_condition_meets_threshold() { let matches = vec![make_match(Severity::High)]; - let (display, fail) = evaluate_fail_condition(matches, &crate::SeverityLevel::Medium, true); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Medium, + &no_groups(), + &no_main(), + true, + ); assert!(fail, "HIGH meets fail_on=Medium threshold"); - assert_eq!(display.len(), 1, "match is returned"); } - // LOW does not meet fail_on=High → no failure, match is returned. + // LOW does not meet fail_on=High → no failure. #[test] fn test_fail_condition_below_threshold() { let matches = vec![make_match(Severity::Low)]; - let (display, fail) = evaluate_fail_condition(matches, &crate::SeverityLevel::High, true); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::High, + &no_groups(), + &no_main(), + true, + ); assert!(!fail, "LOW does not meet fail_on=High threshold"); - assert_eq!(display.len(), 1, "match is returned"); } // UNKNOWN triggers failure when fail_on_unknown=true regardless of threshold. #[test] fn test_fail_condition_unknown_vuln() { let matches = vec![make_match(Severity::Unknown)]; - let (display, fail) = evaluate_fail_condition(matches, &crate::SeverityLevel::Medium, true); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Medium, + &no_groups(), + &no_main(), + true, + ); assert!(fail, "Unknown causes failure when fail_on_unknown=true"); - assert_eq!(display.len(), 1, "match is returned"); + } + + // strictest-wins: a dev-only finding with a stricter group threshold (low) fails even + // though the global fail_on (critical) would not, and a group threshold never loosens + // below what a second reaching group demands. + #[test] + fn test_policy_group_threshold_tightens_below_global() { + let matches = vec![make_grouped_match(Severity::Medium, &["dev"])]; + let thresholds = BTreeMap::from([("dev".to_string(), Severity::Low)]); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Critical, + &thresholds, + &no_main(), + true, + ); + assert!( + fail, + "MEDIUM meets the group's low threshold despite global=critical" + ); + } + + #[test] + fn test_policy_strictest_group_wins_across_groups() { + // Reachable from prod (medium) and dev (critical): strictest = medium. + let matches = vec![make_grouped_match(Severity::Medium, &["prod", "dev"])]; + let thresholds = BTreeMap::from([ + ("prod".to_string(), Severity::Medium), + ("dev".to_string(), Severity::Critical), + ]); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::High, + &thresholds, + &no_main(), + true, + ); + assert!( + fail, + "MEDIUM meets prod's medium (strictest reaching group)" + ); + } + + // A group with no configured threshold falls back to the global default. + #[test] + fn test_policy_unpoliced_group_uses_global() { + let matches = vec![make_grouped_match(Severity::Medium, &["docs"])]; + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::High, + &no_groups(), + &no_main(), + true, + ); + assert!(!fail, "MEDIUM below global high, no group override"); + } + + // Loosen (closes #151): a group-ONLY package (not shipped to prod) takes its group's + // permissive threshold outright — a HIGH dev finding does not fail when the dev policy + // is `critical`, even though the stricter global `medium` would have failed it. + #[test] + fn test_policy_group_only_loosens_below_global() { + let matches = vec![make_grouped_match(Severity::High, &["dev"])]; + let thresholds = BTreeMap::from([("dev".to_string(), Severity::Critical)]); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Medium, + &thresholds, + &no_main(), // dev-only: not main-reachable + true, + ); + assert!( + !fail, + "HIGH below the dev group's critical threshold; global=medium must not floor a group-only package" + ); + } + + // Floor: the same permissive group threshold cannot loosen a package that is ALSO + // main-reachable (ships to prod) — global stays a floor, so the HIGH finding fails. + #[test] + fn test_policy_main_reachable_keeps_global_floor() { + let matches = vec![make_grouped_match(Severity::High, &["dev"])]; + let thresholds = BTreeMap::from([("dev".to_string(), Severity::Critical)]); + let main_reachable: HashSet = + std::iter::once(PackageName::new("test-pkg")).collect(); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Medium, + &thresholds, + &main_reachable, + true, + ); + assert!( + fail, + "HIGH meets the global medium floor; a prod-shipped package can't be loosened by a group" + ); + } + + // Unknown short-circuits before the group-threshold min (a configured group must not + // collapse the effective level to Unknown and fail on everything). + #[test] + fn test_policy_unknown_ignores_group_thresholds() { + let matches = vec![make_grouped_match(Severity::Unknown, &["dev"])]; + let thresholds = BTreeMap::from([("dev".to_string(), Severity::Critical)]); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Critical, + &thresholds, + &no_main(), + false, + ); + assert!( + !fail, + "Unknown with fail_on_unknown=false never fails, group threshold ignored" + ); + } + + // Suppressed findings are still reported (kept in the slice) but never trigger failure. + #[test] + fn test_package_ignore_suppresses_without_dropping() { + let mut ignored = make_match(Severity::Critical); + ignored.package_name = PackageName::new("ignored-pkg"); + let mut matches = vec![ignored, make_grouped_match(Severity::High, &["dev"])]; + apply_package_ignores(&mut matches, &["ignored_pkg".to_string()]); + + // Reporting: nothing dropped, and the matched finding is marked suppressed. + assert_eq!(matches.len(), 2, "suppressed findings stay in the report"); + assert_eq!( + matches.first().and_then(|m| m.suppressed), + Some(SuppressionReason::IgnoredPackage), + "PEP 503 normalization matches ignored_pkg to ignored-pkg" + ); + assert!(matches.get(1).expect("second finding").suppressed.is_none()); + + // Exit: the suppressed critical no longer fails; the other finding still can. + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::Critical, + &no_groups(), + &no_main(), + true, + ); + assert!( + !fail, + "suppressed critical excluded; high does not meet critical" + ); + let fail = evaluate_fail_condition( + &matches, + &crate::SeverityLevel::High, + &no_groups(), + &no_main(), + true, + ); + assert!(fail, "the non-suppressed high still fails at fail_on=high"); + } + + // The matcher threshold stays Low regardless of policy — policy touches the exit + // condition only, never what reaches the report (the fail_on invariant). + #[test] + fn test_policy_never_filters_matcher() { + use clap::Parser; + let args = crate::cli::AuditArgs::try_parse_from(["pysentry", "."]).unwrap(); + let config = build_matcher_config(&args); + assert_eq!(config.min_severity, crate::SeverityLevel::Low); } // Regression guard: fail_on must never narrow the matcher. A v0.4.5 refactor wired fail_on diff --git a/src/cli.rs b/src/cli.rs index 67c10c9..413f671 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -219,6 +219,20 @@ pub struct AuditArgs { #[arg(skip)] pub config_quiet: bool, + /// Package names to suppress entirely, from `[ignore].packages` (config-only). + #[arg(skip)] + pub ignore_packages: Vec, + + /// Per-group fail thresholds from `[groups.]` (config-only), keyed by the + /// PEP 735-normalized group name so lookups match graph attribution. + #[arg(skip)] + pub group_fail_on: std::collections::BTreeMap, + + /// Continue with the sources that succeeded instead of failing when a + /// vulnerability source cannot be fetched (default: fail-closed on any error). + #[arg(long = "no-fail-on-partial")] + pub no_fail_on_partial: bool, + /// Show detailed vulnerability descriptions (full text instead of truncated) #[arg(long, conflicts_with = "compact")] pub detailed: bool, @@ -542,6 +556,25 @@ impl From for crate::SeverityLevel { } } +impl std::str::FromStr for SeverityLevel { + type Err = String; + + /// Parses the canonical level strings (the same set `Config::validate_level` + /// accepts). Single source of truth for level parsing — do not hand-roll the + /// string match elsewhere. + fn from_str(s: &str) -> Result { + match s { + "low" => Ok(Self::Low), + "medium" => Ok(Self::Medium), + "high" => Ok(Self::High), + "critical" => Ok(Self::Critical), + other => Err(format!( + "invalid severity level '{other}' (expected low, medium, high, or critical)" + )), + } + } +} + impl From for crate::VulnerabilitySourceType { fn from(source: VulnerabilitySourceType) -> Self { match source { diff --git a/src/config.rs b/src/config.rs index e29571e..e31d323 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,9 @@ use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; +use std::str::FromStr; use tracing::warn; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -45,6 +47,19 @@ pub struct Config { #[serde(default)] pub output: OutputConfig, + + /// Per-group fail thresholds, keyed by dependency-group name (`[groups.]`). + /// Consumed by the policy engine (strictest-wins). + #[serde(default)] + pub groups: BTreeMap, +} + +/// Per-group policy overriding the global `fail_on` threshold for packages +/// reachable from that dependency group. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GroupPolicy { + pub fail_on: String, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -93,6 +108,11 @@ pub struct SourcesConfig { /// Override the OSV API base URL (custom/self-hosted OSV-compatible endpoint). #[serde(default)] pub service_url: Option, + + /// Fail (exit 2) if any vulnerability source fails to fetch. Default `true` + /// (fail-closed). `false` continues with the sources that succeeded. + #[serde(default = "default_fail_on_partial")] + pub fail_on_partial: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -128,6 +148,11 @@ pub struct IgnoreConfig { #[serde(default)] pub while_no_fix: Vec, + + /// Package names to suppress entirely (all versions). PEP 503 normalized at + /// the comparison site, never compared as raw strings. + #[serde(default)] + pub packages: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -572,12 +597,35 @@ impl Config { self.validate_level(&self.defaults.fail_on, "defaults.fail_on")?; + // Group config keys are raw TOML strings, but attribution (graph.rs) tags + // packages with PEP 735-normalized group names. Reject two keys that + // normalize to the same name — otherwise one silently shadows the other in + // the policy lookup. Mirrors list_group_names' ambiguity rejection. + let mut seen_groups: BTreeMap = BTreeMap::new(); + for (name, policy) in &self.groups { + self.validate_level(&policy.fail_on, &format!("groups.{name}.fail_on"))?; + let normalized = crate::parsers::manifest_reader::normalize_group_name(name); + if let Some(existing) = seen_groups.insert(normalized.clone(), name.clone()) { + anyhow::bail!( + "ambiguous group policy: [groups.{existing}] and [groups.{name}] normalize to the same name '{normalized}'" + ); + } + } + if self.defaults.compact && self.defaults.detailed { anyhow::bail!( "Cannot set both 'compact' and 'detailed' to true. These options are mutually exclusive." ); } + // Validate ignore.packages entries up-front so a malformed name fails loudly at + // load rather than silently matching nothing and reading as a no-op warning later. + for package in &self.ignore.packages { + crate::types::PackageName::from_str(package).map_err(|e| { + anyhow::anyhow!("Invalid package name '{package}' in ignore.packages: {e}") + })?; + } + match self.defaults.display.as_str() { "text" | "table" => {} _ => anyhow::bail!( @@ -676,6 +724,7 @@ impl Default for Config { maintenance: MaintenanceConfig::default(), notifications: NotificationsConfig::default(), output: OutputConfig::default(), + groups: BTreeMap::new(), } } } @@ -703,6 +752,7 @@ impl Default for SourcesConfig { Self { enabled: default_sources(), service_url: None, + fail_on_partial: default_fail_on_partial(), } } } @@ -770,6 +820,9 @@ fn default_scope() -> String { fn default_display() -> String { "table".to_string() } +fn default_fail_on_partial() -> bool { + true +} fn default_sources() -> Vec { vec!["pypa".to_string(), "pypi".to_string(), "osv".to_string()] } @@ -1295,6 +1348,70 @@ format = "json" assert!(err.to_string().contains("Invalid display mode")); } + #[test] + fn test_policy_config_round_trips() { + let temp_dir = TempDir::new().unwrap(); + let config_path = temp_dir.path().join(".pysentry.toml"); + + let content = r#" +version = 1 + +[sources] +fail_on_partial = false + +[ignore] +packages = ["internal-pkg"] + +[groups.dev] +fail_on = "critical" +"#; + fs::write(&config_path, content).unwrap(); + let loader = ConfigLoader::load_from_file(&config_path).unwrap(); + assert!(!loader.config.sources.fail_on_partial); + assert_eq!(loader.config.ignore.packages, vec!["internal-pkg"]); + assert_eq!( + loader.config.groups.get("dev").map(|g| g.fail_on.as_str()), + Some("critical") + ); + } + + #[test] + fn test_fail_on_partial_defaults_true() { + assert!(Config::default().sources.fail_on_partial); + } + + #[test] + fn test_invalid_group_fail_on_level_rejected() { + let mut config = Config::default(); + config.groups.insert( + "dev".to_string(), + GroupPolicy { + fail_on: "bogus".to_string(), + }, + ); + let err = config.validate().unwrap_err(); + assert!(err.to_string().contains("groups.dev.fail_on")); + } + + #[test] + fn test_ambiguous_group_names_rejected() { + let mut config = Config::default(); + config.groups.insert( + "my_group".to_string(), + GroupPolicy { + fail_on: "high".to_string(), + }, + ); + config.groups.insert( + "my-group".to_string(), + GroupPolicy { + fail_on: "low".to_string(), + }, + ); + let err = config.validate().unwrap_err(); + assert!(err.to_string().contains("ambiguous group policy")); + } + #[test] fn test_config_parses_groups_field() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/output/human.rs b/src/output/human.rs index cc5d865..ba15155 100644 --- a/src/output/human.rs +++ b/src/output/human.rs @@ -33,6 +33,22 @@ fn type_column_budget( .max("transitive".len()) } +/// " (SUPPRESSED)" tag for a policy-suppressed finding, styled dim, or "" when not suppressed. +/// Its unstyled width is `SUPPRESSED_TAG_WIDTH` (needed by the grid column-width calc). +const SUPPRESSED_TAG: &str = " (SUPPRESSED)"; +const SUPPRESSED_TAG_WIDTH: usize = SUPPRESSED_TAG.len(); + +fn suppressed_tag( + m: &crate::vulnerability::database::VulnerabilityMatch, + styles: &OutputStyles, +) -> String { + if m.suppressed.is_some() { + SUPPRESSED_TAG.style(styles.dimmed).to_string() + } else { + String::new() + } +} + /// Truncate a cell's visible text to `budget` characters, appending '…' when cut. Keeps the /// variable-length Type cell from forcing the whole grid table past the terminal width. fn truncate_cell(text: &str, budget: usize) -> String { @@ -119,6 +135,16 @@ pub(crate) fn generate_human_report( writeln!(output)?; } + if !report.failed_sources.is_empty() { + writeln!( + output, + "{}: scan incomplete — source(s) failed to fetch: {}. Findings may be missing.", + "PARTIAL SCAN".style(styles.header), + report.failed_sources.join(", ") + )?; + writeln!(output)?; + } + if !report.warnings.is_empty() { writeln!(output, "{}", "WARNINGS".style(styles.header))?; for warning in &report.warnings { @@ -144,7 +170,8 @@ pub(crate) fn generate_human_report( let mut sev_w = "Severity".chars().count(); for m in &report.matches { let id_len = m.vulnerability.id.chars().count() - + usize::from(m.vulnerability.withdrawn.is_some()) * " (WITHDRAWN)".len(); + + usize::from(m.vulnerability.withdrawn.is_some()) * " (WITHDRAWN)".len() + + usize::from(m.suppressed.is_some()) * SUPPRESSED_TAG_WIDTH; id_w = id_w.max(id_len); pkg_w = pkg_w.max(m.package_name.to_string().chars().count()); ver_w = ver_w.max(format!("v{}", m.installed_version).chars().count()); @@ -156,12 +183,17 @@ pub(crate) fn generate_human_report( for m in &report.matches { let id_field = if m.vulnerability.withdrawn.is_some() { format!( - "{} {}", + "{} {}{}", m.vulnerability.id.style(styles.vuln_id), - "(WITHDRAWN)".style(styles.withdrawn_tag) + "(WITHDRAWN)".style(styles.withdrawn_tag), + suppressed_tag(m, styles) ) } else { - m.vulnerability.id.style(styles.vuln_id).to_string() + format!( + "{}{}", + m.vulnerability.id.style(styles.vuln_id), + suppressed_tag(m, styles) + ) }; let dep_type = if m.is_direct { format!( @@ -213,9 +245,10 @@ pub(crate) fn generate_human_report( }; writeln!( output, - " {}{} {} v{} [{}] {}", + " {}{}{} {} v{} [{}] {}", m.vulnerability.id.style(styles.vuln_id), withdrawn_tag, + suppressed_tag(m, styles), m.package_name.to_string().style(styles.package), m.installed_version, m.vulnerability @@ -258,10 +291,11 @@ pub(crate) fn generate_human_report( writeln!( output, - " {}. {}{} {} v{} [{}] {}{}", + " {}. {}{}{} {} v{} [{}] {}{}", i + 1, m.vulnerability.id.style(styles.vuln_id), withdrawn_tag, + suppressed_tag(m, styles), m.package_name.to_string().style(styles.package), m.installed_version, m.vulnerability @@ -705,6 +739,70 @@ mod tests { assert!(output.contains("No vulnerabilities found")); } + #[test] + fn test_partial_scan_renders_failed_sources() { + let report = crate::output::model::AuditReport::new( + DependencyStats { + total_packages: 1, + direct_packages: 1, + transitive_packages: 0, + by_source: HashMap::new(), + }, + DatabaseStats { + total_vulnerabilities: 0, + total_packages: 0, + severity_counts: HashMap::new(), + packages_with_most_vulns: vec![], + }, + vec![], + FixAnalysis { + total_matches: 0, + fixable: 0, + unfixable: 0, + fix_suggestions: vec![], + }, + vec![], + Vec::new(), + ) + .with_failed_sources(vec!["osv".to_string()]); + + let output = generate_human_report( + &report, + DetailLevel::Normal, + DisplayMode::Table, + &OutputStyles::default(), + ) + .unwrap(); + assert!(output.contains("PARTIAL SCAN"), "output: {output}"); + assert!(output.contains("osv"), "output: {output}"); + } + + #[test] + fn test_suppressed_finding_tagged_in_all_arms() { + use crate::vulnerability::database::SuppressionReason; + let mut report = create_test_report(); + report.matches.first_mut().unwrap().suppressed = Some(SuppressionReason::IgnoredPackage); + + for (detail, display) in [ + (DetailLevel::Compact, DisplayMode::Table), + (DetailLevel::Compact, DisplayMode::Text), + (DetailLevel::Normal, DisplayMode::Text), + (DetailLevel::Detailed, DisplayMode::Text), + ] { + let output = + generate_human_report(&report, detail, display, &OutputStyles::default()).unwrap(); + // Suppressed finding is still listed (never dropped) and marked. + assert!( + output.contains("SUPPRESSED"), + "missing tag for {detail:?}/{display:?}: {output}" + ); + assert!( + output.contains("GHSA-test-1234"), + "finding dropped for {detail:?}/{display:?}: {output}" + ); + } + } + #[test] fn test_compact_report_no_header_no_footer() { let report = create_test_report(); @@ -1223,6 +1321,8 @@ mod tests { vulnerability, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }]; let fix_analysis = FixAnalysis { @@ -1297,6 +1397,8 @@ mod tests { vulnerability: withdrawn_vuln, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }]; let fix_analysis = FixAnalysis { diff --git a/src/output/json.rs b/src/output/json.rs index b49915e..ca8ec79 100644 --- a/src/output/json.rs +++ b/src/output/json.rs @@ -20,6 +20,8 @@ pub(crate) fn generate_json_report( fix_suggestions: &report.fix_analysis.fix_suggestions, warnings: &report.warnings, maintenance_issues: &report.maintenance_issues, + partial: !report.failed_sources.is_empty(), + failed_sources: &report.failed_sources, }; Ok(serde_json::to_string_pretty(&view)?) } @@ -40,6 +42,10 @@ struct JsonReportView<'a> { fix_suggestions: &'a [FixSuggestion], warnings: &'a [String], maintenance_issues: &'a [MaintenanceIssue], + /// True when one or more vulnerability sources failed to fetch — findings are incomplete. + partial: bool, + /// Names of the sources that failed to fetch (empty on a full scan). + failed_sources: &'a [String], } #[cfg(test)] @@ -106,7 +112,7 @@ mod tests { #[test] fn test_json_maintenance_issue_type_lowercase() { - // Regression test: Phase 3 switched from Display (.to_string() → "DEPRECATED") + // Regression test: serialization switched from Display (.to_string() → "DEPRECATED") // to direct serde serialization (→ "deprecated"). Verifies the serde path. let dependency_stats = DependencyStats { total_packages: 3, @@ -179,6 +185,37 @@ mod tests { assert_eq!(json["vulnerabilities"][1]["is_direct"], false); } + #[test] + fn test_json_suppression_and_partial_surfaced() { + use crate::vulnerability::database::SuppressionReason; + let mut report = create_test_report(); + report.matches[0].suppressed = Some(SuppressionReason::IgnoredPackage); + let report = report.with_failed_sources(vec!["osv".to_string()]); + + let output = generate_json_report(&report).unwrap(); + let json: serde_json::Value = serde_json::from_str(&output).unwrap(); + + // Suppressed findings stay in the list (never dropped) and are tagged. + assert_eq!(json["vulnerabilities"].as_array().unwrap().len(), 1); + assert_eq!(json["total_vulnerabilities"], 1); + assert_eq!(json["vulnerabilities"][0]["suppressed"], "ignored_package"); + + // Partial-scan state is always present (false/empty on a clean scan). + assert_eq!(json["partial"], true); + assert_eq!(json["failed_sources"][0], "osv"); + } + + #[test] + fn test_json_partial_defaults_present_on_clean_scan() { + let report = create_test_report(); + let output = generate_json_report(&report).unwrap(); + let json: serde_json::Value = serde_json::from_str(&output).unwrap(); + assert_eq!(json["partial"], false); + assert_eq!(json["failed_sources"].as_array().unwrap().len(), 0); + // Not suppressed → field omitted entirely. + assert!(json["vulnerabilities"][0]["suppressed"].is_null()); + } + #[test] fn test_json_cvss_version_serialized() { let report = create_test_report_with_extras(); diff --git a/src/output/markdown.rs b/src/output/markdown.rs index 3c7dc6a..1358294 100644 --- a/src/output/markdown.rs +++ b/src/output/markdown.rs @@ -34,6 +34,15 @@ pub(crate) fn generate_markdown_report( )?; writeln!(output)?; + if !report.failed_sources.is_empty() { + writeln!( + output, + "> ⚠️ **Partial scan:** source(s) failed to fetch: {}. Findings may be incomplete.", + report.failed_sources.join(", ") + )?; + writeln!(output)?; + } + if !summary.severity_counts.is_empty() { writeln!(output, "## 🚨 Severity Breakdown")?; writeln!(output)?; @@ -88,13 +97,20 @@ pub(crate) fn generate_markdown_report( "" }; + let suppressed_tag = if m.suppressed.is_some() { + " *(suppressed)*" + } else { + "" + }; + writeln!( output, - "### {}. {} `{}`{}{}", + "### {}. {} `{}`{}{}{}", i + 1, icon, m.vulnerability.id, withdrawn_tag, + suppressed_tag, source_tag )?; writeln!(output)?; @@ -322,6 +338,23 @@ mod tests { assert!(output.contains("- **Type:** direct")); } + #[test] + fn test_markdown_suppression_and_partial() { + use crate::vulnerability::database::SuppressionReason; + let mut report = create_test_report(); + report.matches.first_mut().unwrap().suppressed = Some(SuppressionReason::IgnoredPackage); + let report = report.with_failed_sources(vec!["osv".to_string()]); + + let output = generate_markdown_report(&report).unwrap(); + + // Suppressed finding kept and tagged. + assert!(output.contains("GHSA-test-1234")); + assert!(output.contains("*(suppressed)*")); + // Partial-scan callout naming the failed source. + assert!(output.contains("Partial scan")); + assert!(output.contains("osv")); + } + #[test] fn test_markdown_fix_suggestions_table() { let report = create_test_report_with_multiple_fixes(); diff --git a/src/output/model.rs b/src/output/model.rs index c81eb5c..371122d 100644 --- a/src/output/model.rs +++ b/src/output/model.rs @@ -81,6 +81,9 @@ pub struct AuditReport { /// "transitive (via X)" display. Empty unless scanned from a lock file that records /// dependency edges (uv.lock, poetry.lock, pylock.toml). pub transitive_roots: HashMap>, + /// Vulnerability sources that failed to fetch. Non-empty means the scan is incomplete + /// (partial): findings reflect only the sources that succeeded. Empty on a full scan. + pub failed_sources: Vec, cached_summary: OnceLock, } @@ -95,6 +98,7 @@ impl Clone for AuditReport { warnings: self.warnings.clone(), maintenance_issues: self.maintenance_issues.clone(), transitive_roots: self.transitive_roots.clone(), + failed_sources: self.failed_sources.clone(), cached_summary: OnceLock::new(), } } @@ -119,10 +123,18 @@ impl AuditReport { warnings, maintenance_issues, transitive_roots: HashMap::new(), + failed_sources: Vec::new(), cached_summary: OnceLock::new(), } } + /// Record the vulnerability sources that failed to fetch (partial scan). Empty = full scan. + #[must_use] + pub fn with_failed_sources(mut self, failed_sources: Vec) -> Self { + self.failed_sources = failed_sources; + self + } + /// Attach the child → top-level-deps map used for "transitive (via X)" display. #[must_use] pub fn with_transitive_roots( @@ -290,6 +302,8 @@ pub(crate) mod test_helpers { vulnerability, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }]; let fix_analysis = FixAnalysis { @@ -375,6 +389,8 @@ pub(crate) mod test_helpers { vulnerability: direct_vulnerability, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }, VulnerabilityMatch { package_name: PackageName::from_str("transitive-package").unwrap(), @@ -382,6 +398,8 @@ pub(crate) mod test_helpers { vulnerability: transitive_vulnerability, is_direct: false, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }, ]; diff --git a/src/output/sarif.rs b/src/output/sarif.rs index 6f61ac8..768c7aa 100644 --- a/src/output/sarif.rs +++ b/src/output/sarif.rs @@ -16,9 +16,11 @@ use serde_json::{json, Value}; use serde_sarif::sarif::{ ArtifactLocation as SarifArtifactLocation, Invocation as SarifInvocation, Location as SarifLocation, LogicalLocation as SarifLogicalLocation, Message as SarifMessage, - MultiformatMessageString, PhysicalLocation as SarifPhysicalLocation, PropertyBag, - Region as SarifRegion, ReportingConfiguration, ReportingDescriptor, Result as SarifResult, - ResultLevel, Run as SarifRun, Sarif, Tool as SarifTool, ToolComponent as SarifToolComponent, + MultiformatMessageString, Notification as SarifNotification, + PhysicalLocation as SarifPhysicalLocation, PropertyBag, Region as SarifRegion, + ReportingConfiguration, ReportingDescriptor, Result as SarifResult, ResultLevel, + Run as SarifRun, Sarif, Suppression as SarifSuppression, Tool as SarifTool, + ToolComponent as SarifToolComponent, }; use sha2::{Digest, Sha256}; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -80,6 +82,7 @@ impl SarifGenerator { &report.dependency_stats, &report.database_stats, &report.warnings, + &report.failed_sources, start_time, )?; @@ -969,6 +972,15 @@ impl SarifGenerator { result.rule_index = Some(idx); } + // Policy-suppressed findings stay in the report but are marked suppressed. + // The suppression source (a config file) is external to the scanned artifact. + if let Some(reason) = m.suppressed { + result.suppressions = Some(vec![SarifSuppression::builder() + .kind(json!("external")) + .justification(reason.label().to_string()) + .build()]); + } + results.push(result); } @@ -1097,6 +1109,7 @@ impl SarifGenerator { dependency_stats: &DependencyStats, database_stats: &DatabaseStats, warnings: &[String], + failed_sources: &[String], start_time: DateTime, ) -> Result { // originalUriBaseIds enables portable path resolution across CI environments. @@ -1119,15 +1132,32 @@ impl SarifGenerator { .build(), ); + // A partial scan (one or more sources failed to fetch) is an incomplete run: + // executionSuccessful=false, with a notification naming the failed sources so the + // incompleteness is never silent in SARIF consumers (GitHub/GitLab Security). + let partial = !failed_sources.is_empty(); + let end_time = Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true); let mut invocation = SarifInvocation::builder() - .execution_successful(true) + .execution_successful(!partial) .command_line("pysentry".to_string()) .start_time_utc(start_time.to_rfc3339_opts(SecondsFormat::Secs, true)) .exit_code(i64::from(!results.is_empty())) .build(); invocation.end_time_utc = Some(end_time); + if partial { + let message = format!( + "Partial scan: {} source(s) failed to fetch ({}). Findings may be incomplete.", + failed_sources.len(), + failed_sources.join(", ") + ); + invocation.tool_execution_notifications = Some(vec![SarifNotification::builder() + .message(SarifMessage::builder().text(message).build()) + .level(json!("error")) + .build()]); + } + let mut scan_stats: BTreeMap = BTreeMap::new(); scan_stats.insert( "total_packages".to_string(), @@ -1269,6 +1299,8 @@ mod tests { vulnerability: create_test_vulnerability(), is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, } } @@ -1401,6 +1433,8 @@ mod tests { vulnerability: vuln, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }; generator.generate_rules(&[test_match]); @@ -1481,6 +1515,46 @@ mod tests { } } + #[test] + fn test_sarif_suppression_and_partial_surfaced() { + use crate::vulnerability::database::SuppressionReason; + let mut report = create_test_report(); + report.matches[0].suppressed = Some(SuppressionReason::IgnoredPackage); + let report = report.with_failed_sources(vec!["osv".to_string()]); + + let temp_dir = TempDir::new().unwrap(); + let mut generator = SarifGenerator::new(temp_dir.path()); + let sarif: serde_json::Value = + serde_json::from_str(&generator.generate_report(&report).unwrap()).unwrap(); + + let results = sarif["runs"][0]["results"].as_array().unwrap(); + // Suppressed findings stay in results (never dropped) and carry a suppressions entry. + assert_eq!(results.len(), 1); + assert_eq!(results[0]["suppressions"][0]["kind"], "external"); + + // Partial scan → executionSuccessful=false + a toolExecutionNotification. + let invocation = &sarif["runs"][0]["invocations"][0]; + assert_eq!(invocation["executionSuccessful"], false); + let notifications = invocation["toolExecutionNotifications"].as_array().unwrap(); + assert!(notifications[0]["message"]["text"] + .as_str() + .unwrap() + .contains("osv")); + } + + #[test] + fn test_sarif_clean_scan_execution_successful() { + let report = create_test_report(); + let temp_dir = TempDir::new().unwrap(); + let mut generator = SarifGenerator::new(temp_dir.path()); + let sarif: serde_json::Value = + serde_json::from_str(&generator.generate_report(&report).unwrap()).unwrap(); + let invocation = &sarif["runs"][0]["invocations"][0]; + assert_eq!(invocation["executionSuccessful"], true); + // No suppression on a non-suppressed finding. + assert!(sarif["runs"][0]["results"][0]["suppressions"].is_null()); + } + #[test] fn test_help_text_vs_markdown_differ() { let vuln = create_test_vulnerability(); @@ -1888,6 +1962,8 @@ test = [ vulnerability: vuln, is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }; generator.generate_rules(std::slice::from_ref(&test_match)); diff --git a/src/parsers/graph.rs b/src/parsers/graph.rs index e2faf0f..0d8e82a 100644 --- a/src/parsers/graph.rs +++ b/src/parsers/graph.rs @@ -11,7 +11,7 @@ //! as before. use crate::parsers::reachability::reachable_closure; -use crate::parsers::{lock, poetry_lock, pylock}; +use crate::parsers::{lock, manifest_reader, poetry_lock, pylock}; use crate::types::PackageName; use std::collections::{HashMap, HashSet}; use std::path::Path; @@ -67,6 +67,102 @@ fn transitive_roots( .collect() } +/// package → normalized dependency-group names (PEP 735 / Poetry) that reach it, for +/// per-group policy attribution. Runs `reachable_closure` once per group declared in +/// pyproject.toml, seeded from that group's own deps only (see +/// `read_group_only_deps_from_value`) — a package reachable from both main and a group +/// still carries that group, so it never loses its group attribution. Whether the global +/// threshold also floors such a shared package is decided separately, from +/// `build_main_reachable`. +/// +/// Only group-aware locks (uv.lock, poetry.lock, pylock.toml) carry group structure; +/// any other parser (or a project with no declared groups) yields an empty map, the +/// same contract as `build_transitive_roots`. +pub async fn build_group_attribution( + project_dir: &Path, + parser_name: &str, +) -> HashMap> { + let edges = match group_aware_edges(project_dir, parser_name).await { + Some(edges) => edges, + None => return HashMap::new(), + }; + + let pyproject_path = project_dir.join("pyproject.toml"); + let Some(doc) = read_pyproject_doc(&pyproject_path).await else { + return HashMap::new(); + }; + let Ok(group_names) = manifest_reader::list_group_names_from_value(&doc) else { + return HashMap::new(); + }; + + let mut attribution: HashMap> = HashMap::new(); + for group in &group_names { + let Ok(seed) = manifest_reader::read_group_only_deps_from_value(&doc, group) else { + continue; + }; + if seed.is_empty() { + continue; + } + let normalized = manifest_reader::normalize_group_name(group); + for reached in reachable_closure(&seed, &edges) { + attribution + .entry(reached) + .or_default() + .insert(normalized.clone()); + } + } + attribution +} + +/// Packages reachable from the project's **main** (unconditional) dependencies only — +/// `[project].dependencies` / `[tool.poetry.dependencies]`, excluding every group. +/// +/// The policy engine uses this to decide whether a per-group threshold may *loosen* a +/// finding: a package that ships to production (main-reachable) keeps the global `fail_on` +/// as a floor, whereas a group-only package may take its group's threshold freely (below +/// global if the group is more permissive). Same dispatch/empty-map contract as +/// [`build_group_attribution`] — non-group-aware locks yield an empty set. +pub async fn build_main_reachable(project_dir: &Path, parser_name: &str) -> HashSet { + let edges = match group_aware_edges(project_dir, parser_name).await { + Some(edges) => edges, + None => return HashSet::new(), + }; + + let pyproject_path = project_dir.join("pyproject.toml"); + // Empty group filter → main dependencies only (no group passes the filter). + let main_seed = + manifest_reader::read_direct_deps_from_pyproject(&pyproject_path, Some(&HashSet::new())) + .await + .ok() + .flatten() + .unwrap_or_default(); + + if main_seed.is_empty() { + return HashSet::new(); + } + reachable_closure(&main_seed, &edges) +} + +/// Shared dispatch for the two policy-attribution builders: the dependency edge map for a +/// group-aware lock, or `None` for any other parser (or an empty lock). +async fn group_aware_edges( + project_dir: &Path, + parser_name: &str, +) -> Option>> { + let edges = match parser_name { + "uv.lock" => lock::uv_lock_edges(project_dir).await, + "poetry.lock" => poetry_lock::poetry_lock_edges(project_dir).await, + "pylock.toml" => pylock::pylock_edges(project_dir).await, + _ => return None, + }; + (!edges.is_empty()).then_some(edges) +} + +async fn read_pyproject_doc(pyproject_path: &Path) -> Option { + let content = tokio::fs::read_to_string(pyproject_path).await.ok()?; + toml::from_str(&content).ok() +} + #[cfg(test)] mod tests { use super::*; @@ -202,4 +298,196 @@ files = [] let roots = transitive_roots(&direct, &edges); assert_eq!(roots.get(&pkg("b")), Some(&vec![pkg("a")])); } + + // Lock: root -> requests, httpx (both main). requests -> certifi. dev group declares + // {pytest, requests} — requests is declared by BOTH main and dev. + // + // requests must still tag `dev`: seeding dev's closure from dev's own deps only (not + // main ∪ dev) means the shared package isn't erased from the seed, so its transitive + // (certifi) is correctly attributed to dev too. httpx is main-only and reachable from + // no group closure, so it gets no attribution entry at all — "main" is never itself a + // group tag. + #[tokio::test] + async fn build_group_attribution_shared_transitive_and_dev_only() { + let lock_content = r#" +version = 1 +requires-python = ">=3.11" + +[[package]] +name = "root" +source = { virtual = "." } +dependencies = [{ name = "requests" }, { name = "httpx" }] + +[[package]] +name = "requests" +version = "2.31.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [{ name = "certifi" }] + +[[package]] +name = "httpx" +version = "0.27.0" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "certifi" +version = "2024.1.1" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "pytest" +version = "8.0.0" +source = { registry = "https://pypi.org/simple" } +"#; + let pyproject_content = r#" +[project] +name = "myapp" +dependencies = ["requests>=2.31", "httpx>=0.27"] + +[dependency-groups] +dev = ["pytest>=8", "requests>=2.31"] +"#; + + let dir = TempDir::new().unwrap(); + tokio::fs::write(dir.path().join("uv.lock"), lock_content) + .await + .unwrap(); + tokio::fs::write(dir.path().join("pyproject.toml"), pyproject_content) + .await + .unwrap(); + + let attribution = build_group_attribution(dir.path(), "uv.lock").await; + + let dev: HashSet = ["dev".to_string()].into_iter().collect(); + assert_eq!( + attribution.get(&pkg("pytest")), + Some(&dev), + "pytest is dev-only" + ); + assert_eq!( + attribution.get(&pkg("requests")), + Some(&dev), + "requests is declared by both main and dev; must still tag dev" + ); + assert_eq!( + attribution.get(&pkg("certifi")), + Some(&dev), + "certifi is reachable from requests via dev's closure" + ); + assert!( + attribution.get(&pkg("httpx")).is_none_or(HashSet::is_empty), + "httpx is main-only and unreachable from any group closure, got: {:?}", + attribution.get(&pkg("httpx")) + ); + } + + #[tokio::test] + async fn build_group_attribution_unknown_format_is_empty() { + let dir = TempDir::new().unwrap(); + let attribution = build_group_attribution(dir.path(), "requirements.txt").await; + assert!(attribution.is_empty()); + } + + #[tokio::test] + async fn build_group_attribution_no_declared_groups_is_empty() { + let lock_content = r#" +version = 1 +requires-python = ">=3.11" + +[[package]] +name = "requests" +version = "2.31.0" +source = { registry = "https://pypi.org/simple" } +"#; + let pyproject_content = r#" +[project] +name = "myapp" +dependencies = ["requests>=2.31"] +"#; + let dir = TempDir::new().unwrap(); + tokio::fs::write(dir.path().join("uv.lock"), lock_content) + .await + .unwrap(); + tokio::fs::write(dir.path().join("pyproject.toml"), pyproject_content) + .await + .unwrap(); + + let attribution = build_group_attribution(dir.path(), "uv.lock").await; + assert!( + attribution.is_empty(), + "no [dependency-groups]/[project.optional-dependencies]/poetry groups declared" + ); + } + + // main = [requests, httpx]; requests -> certifi. dev = [pytest]. main-reachable must be + // exactly the main closure {requests, httpx, certifi} — never the dev-only pytest. + #[tokio::test] + async fn build_main_reachable_covers_main_closure_only() { + let lock_content = r#" +version = 1 +requires-python = ">=3.11" + +[[package]] +name = "root" +source = { virtual = "." } +dependencies = [{ name = "requests" }, { name = "httpx" }] + +[[package]] +name = "requests" +version = "2.31.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [{ name = "certifi" }] + +[[package]] +name = "httpx" +version = "0.27.0" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "certifi" +version = "2024.1.1" +source = { registry = "https://pypi.org/simple" } + +[[package]] +name = "pytest" +version = "8.0.0" +source = { registry = "https://pypi.org/simple" } +"#; + let pyproject_content = r#" +[project] +name = "myapp" +dependencies = ["requests>=2.31", "httpx>=0.27"] + +[dependency-groups] +dev = ["pytest>=8"] +"#; + let dir = TempDir::new().unwrap(); + tokio::fs::write(dir.path().join("uv.lock"), lock_content) + .await + .unwrap(); + tokio::fs::write(dir.path().join("pyproject.toml"), pyproject_content) + .await + .unwrap(); + + let main_reachable = build_main_reachable(dir.path(), "uv.lock").await; + + assert!(main_reachable.contains(&pkg("requests")), "main direct dep"); + assert!(main_reachable.contains(&pkg("httpx")), "main direct dep"); + assert!( + main_reachable.contains(&pkg("certifi")), + "transitive of a main dep" + ); + assert!( + !main_reachable.contains(&pkg("pytest")), + "pytest is dev-only, must not be main-reachable" + ); + } + + #[tokio::test] + async fn build_main_reachable_unknown_format_is_empty() { + let dir = TempDir::new().unwrap(); + assert!(build_main_reachable(dir.path(), "requirements.txt") + .await + .is_empty()); + } } diff --git a/src/parsers/manifest_reader.rs b/src/parsers/manifest_reader.rs index 686cc7d..10e19f3 100644 --- a/src/parsers/manifest_reader.rs +++ b/src/parsers/manifest_reader.rs @@ -163,6 +163,108 @@ pub async fn read_direct_deps_with_extras_from_pyproject( Ok(Some((names, extras_map))) } +/// Read only the dependencies declared directly under one named group — excluding the +/// project's unconditional main dependencies ([project].dependencies / +/// [tool.poetry.dependencies]) that `read_direct_deps_from_pyproject` always folds in. +/// +/// Used to seed per-group reachability for policy attribution (see +/// `parsers::graph::build_group_attribution`). Seeding from the union of main + group +/// (as `read_direct_deps_from_pyproject(Some({group}))` returns) and then subtracting +/// the main-only seed would erase any package the group declares that is *also* a main +/// dependency, silently dropping that package's group-only transitives from attribution — +/// the same shared-transitive trap `identify_optional_packages` already guards against. +/// Returns Ok(empty set) if the file or the group section does not exist. +pub async fn read_group_only_deps_from_pyproject( + path: &Path, + group: &str, +) -> Result> { + let content = match tokio::fs::read_to_string(path).await { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(HashSet::new()), + Err(e) => return Err(e.into()), + }; + let doc: toml::Value = toml::from_str(&content)?; + read_group_only_deps_from_value(&doc, group) +} + +/// Value-level core of [`read_group_only_deps_from_pyproject`], operating on an +/// already-parsed document so a caller iterating many groups (e.g. +/// `graph::build_group_attribution`) parses `pyproject.toml` once instead of per group. +pub(crate) fn read_group_only_deps_from_value( + doc: &toml::Value, + group: &str, +) -> Result> { + let filter: HashSet = std::iter::once(group.to_string()).collect(); + + let mut names: HashSet = HashSet::new(); + let mut extras_map: HashMap> = HashMap::new(); + + // PEP 621: [project.optional-dependencies] + if let Some(project) = doc.get("project") { + if let Some(optional) = project + .get("optional-dependencies") + .and_then(|v| v.as_table()) + { + for (group_name, group_deps) in optional { + if !group_passes_filter(Some(&filter), group_name) { + continue; + } + if let Some(dep_arr) = group_deps.as_array() { + for dep in dep_arr { + if let Some(dep_str) = dep.as_str() { + record_pep508(dep_str, &mut names, &mut extras_map); + } + } + } + } + } + } + + // PEP 735: [dependency-groups], resolves include-group. + if let Some(dep_groups) = doc.get("dependency-groups") { + if let Some(table) = dep_groups.as_table() { + let mut resolved = Vec::new(); + for (group_name, entries) in table { + if !group_passes_filter(Some(&filter), group_name) { + continue; + } + if let Some(entry_arr) = entries.as_array() { + let mut current_path = Vec::new(); + collect_group_deps(entry_arr, dep_groups, &mut current_path, &mut resolved)?; + } + } + for dep_str in &resolved { + record_pep508(dep_str, &mut names, &mut extras_map); + } + } + } + + // Poetry: [tool.poetry.group..dependencies] + if let Some(tool) = doc.get("tool") { + if let Some(poetry) = tool.get("poetry") { + if let Some(poetry_groups) = poetry.get("group").and_then(|v| v.as_table()) { + for (group_name, group_val) in poetry_groups { + if !group_passes_filter(Some(&filter), group_name) { + continue; + } + if let Some(group_deps) = + group_val.get("dependencies").and_then(|v| v.as_table()) + { + collect_poetry_table_deps_with_extras( + group_deps, + false, + &mut names, + &mut extras_map, + ); + } + } + } + } + } + + Ok(names) +} + fn record_pep508( dep_str: &str, names: &mut HashSet, @@ -393,7 +495,12 @@ pub async fn list_group_names(path: &Path) -> Result> { Err(e) => return Err(e.into()), }; let doc: toml::Value = toml::from_str(&content)?; + list_group_names_from_value(&doc) +} +/// Value-level core of [`list_group_names`], operating on an already-parsed document so a +/// caller that also reads per-group deps parses `pyproject.toml` only once. +pub(crate) fn list_group_names_from_value(doc: &toml::Value) -> Result> { let mut raw_names: Vec = Vec::new(); // PEP 621: [project.optional-dependencies] keys @@ -1418,4 +1525,58 @@ black = "^24" assert!(result.contains("dev")); assert_eq!(result.len(), 1); } + + // requests is declared in BOTH main and the dev group. The group-only reader must + // still return it — dropping it here is exactly the shared-transitive bug the union + // helper (`read_direct_deps_from_pyproject`) would silently reintroduce if a caller + // tried to derive "group-only" by subtracting the main seed after the fact. + #[tokio::test] + async fn test_read_group_only_deps_keeps_shared_main_dependency() { + let file = write_toml( + r#" +[project] +name = "myproject" +dependencies = ["requests>=2.31", "httpx>=0.27"] + +[dependency-groups] +dev = ["pytest>=8", "requests>=2.31"] +"#, + ); + let result = read_group_only_deps_from_pyproject(file.path(), "dev") + .await + .unwrap(); + assert_eq!( + result, + HashSet::from([PackageName::new("pytest"), PackageName::new("requests")]), + "must contain the group's own deps, including one shared with main" + ); + assert!( + !result.contains(&PackageName::new("httpx")), + "main-only dependency must not leak into a group-only read" + ); + } + + #[tokio::test] + async fn test_read_group_only_deps_missing_group_is_empty() { + let file = write_toml( + r#" +[project] +name = "myproject" +dependencies = ["httpx>=0.27"] +"#, + ); + let result = read_group_only_deps_from_pyproject(file.path(), "dev") + .await + .unwrap(); + assert!(result.is_empty()); + } + + #[tokio::test] + async fn test_read_group_only_deps_missing_file_is_empty() { + let result = + read_group_only_deps_from_pyproject(Path::new("/nonexistent/pyproject.toml"), "dev") + .await + .unwrap(); + assert!(result.is_empty()); + } } diff --git a/src/providers/osv.rs b/src/providers/osv.rs index f64ce53..1394c3d 100644 --- a/src/providers/osv.rs +++ b/src/providers/osv.rs @@ -709,6 +709,61 @@ impl OsvSource { (vuln_id, result) } + /// Process one page of a paginated OSV batch response. + /// + /// Returns the `(vuln_id, package_name)` pairs on this page plus the + /// follow-up queries for every result carrying a `next_page_token`. OSV + /// returns results in query order, so `queries[idx]` names result `idx`. + fn collect_page( + queries: &[OsvQuery], + response: OsvBatchResponse, + ) -> (Vec<(String, String)>, Vec) { + let mut collected = Vec::new(); + let mut next_queries = Vec::new(); + + for (idx, result) in response.results.into_iter().enumerate() { + let Some(query) = queries.get(idx) else { + continue; + }; + let package_name = query + .package + .as_ref() + .map(|p| p.name.clone()) + .unwrap_or_else(|| "unknown".to_string()); + + for vuln in result.vulns { + collected.push((vuln.id, package_name.clone())); + } + + if let Some(token) = result.next_page_token { + let mut follow_up = query.clone(); + follow_up.page_token = Some(token); + next_queries.push(follow_up); + } + } + + (collected, next_queries) + } + + /// Query an OSV batch, following `next_page_token` until every query is + /// exhausted. Unpaginated fetching silently truncates results = false + /// negatives, so there is no page cap (correctness over speed). A failed + /// page propagates rather than truncating: the source is marked partial by + /// the pipeline instead of silently reporting fewer vulnerabilities. + async fn query_batch_paginated(&self, batch: &[OsvQuery]) -> Result> { + let mut collected = Vec::new(); + let mut pending: Vec = batch.to_vec(); + + while !pending.is_empty() { + let response = self.query_batch(&pending).await?; + let (mut page, next_queries) = Self::collect_page(&pending, response); + collected.append(&mut page); + pending = next_queries; + } + + Ok(collected) + } + /// Query OSV batch API with retry logic async fn query_batch(&self, batch: &[OsvQuery]) -> Result { let batch_len = batch.len(); @@ -822,6 +877,7 @@ impl VulnerabilityProvider for OsvSource { purl: None, }), version: Some(version.clone()), + page_token: None, } }) .collect(); @@ -855,41 +911,12 @@ impl VulnerabilityProvider for OsvSource { for batch in batches { debug!("Querying OSV API with {} packages", batch.len()); - match self.query_batch(batch).await { - Ok(batch_response) => { - debug!( - "Successfully parsed batch response with {} results", - batch_response.results.len() - ); - - // Collect vulnerability IDs and map them to packages - for (idx, result) in batch_response.results.into_iter().enumerate() { - let package_name = if let Some(query) = batch.get(idx) { - query - .package - .as_ref() - .map(|p| p.name.clone()) - .unwrap_or_else(|| "unknown".to_string()) - } else { - "unknown".to_string() - }; - - for vuln in result.vulns { - debug!( - "Found vulnerability {} for package {}", - vuln.id, package_name - ); - all_vulnerability_ids.push(vuln.id.clone()); - package_vuln_mapping - .entry(vuln.id) - .or_default() - .insert(package_name.clone()); - } - } - } - Err(e) => { - warn!("Failed to query OSV batch: {}", e); - } + for (vuln_id, package_name) in self.query_batch_paginated(batch).await? { + all_vulnerability_ids.push(vuln_id.clone()); + package_vuln_mapping + .entry(vuln_id) + .or_default() + .insert(package_name); } // Update batch progress bar @@ -1066,6 +1093,9 @@ struct OsvQuery { package: Option, #[serde(skip_serializing_if = "Option::is_none")] version: Option, + /// Continuation token from a prior page's `next_page_token`. + #[serde(skip_serializing_if = "Option::is_none")] + page_token: Option, } /// OSV batch API response @@ -1079,6 +1109,9 @@ struct OsvBatchResponse { struct OsvResult { #[serde(default)] vulns: Vec, + /// Present when this query has more results on a following page. + #[serde(default)] + next_page_token: Option, } /// Lightweight vulnerability data from batch API @@ -1717,6 +1750,61 @@ mod tests { .any(|range| range.contains(&Version::from_str("1.5.0").unwrap()))); } + #[test] + fn test_pagination_follows_next_page_token_across_pages() { + // Page 1: pkg-a paginates (token "t1"), pkg-b is complete. + let queries = vec![ + OsvQuery { + package: Some(OsvPackage { + ecosystem: "PyPI".to_string(), + name: "pkg-a".to_string(), + purl: None, + }), + version: Some("1.0".to_string()), + page_token: None, + }, + OsvQuery { + package: Some(OsvPackage { + ecosystem: "PyPI".to_string(), + name: "pkg-b".to_string(), + purl: None, + }), + version: Some("2.0".to_string()), + page_token: None, + }, + ]; + let page1: OsvBatchResponse = serde_json::from_str( + r#"{"results":[ + {"vulns":[{"id":"VULN-A1"}],"next_page_token":"t1"}, + {"vulns":[{"id":"VULN-B1"}]} + ]}"#, + ) + .unwrap(); + + let (collected1, next_queries) = OsvSource::collect_page(&queries, page1); + assert_eq!( + collected1, + vec![ + ("VULN-A1".to_string(), "pkg-a".to_string()), + ("VULN-B1".to_string(), "pkg-b".to_string()), + ] + ); + // Only the paginating query is followed, and it carries the token. + assert_eq!(next_queries.len(), 1); + assert_eq!(next_queries[0].page_token.as_deref(), Some("t1")); + assert_eq!(next_queries[0].package.as_ref().unwrap().name, "pkg-a"); + + // Page 2: final page for pkg-a, no further token. + let page2: OsvBatchResponse = + serde_json::from_str(r#"{"results":[{"vulns":[{"id":"VULN-A2"}]}]}"#).unwrap(); + let (collected2, done) = OsvSource::collect_page(&next_queries, page2); + assert_eq!( + collected2, + vec![("VULN-A2".to_string(), "pkg-a".to_string())] + ); + assert!(done.is_empty(), "no token means pagination stops"); + } + #[test] fn test_should_cache_empty_db_with_expected_vulns() { let db = VulnerabilityDatabase::new(); diff --git a/src/vulnerability/database.rs b/src/vulnerability/database.rs index aec4676..67b1f85 100644 --- a/src/vulnerability/database.rs +++ b/src/vulnerability/database.rs @@ -184,6 +184,37 @@ pub struct VulnerabilityMatch { /// File that contributed the dependency, when known. #[serde(skip_serializing_if = "Option::is_none")] pub source_file: Option, + /// Normalized dependency-group names (PEP 735 / Poetry) that reach this package, + /// for per-group policy attribution. Empty for main-only packages and for parsers + /// with no group concept (requirements.txt, Pipfile.lock). + /// + /// Not serialized yet: it drives the exit condition only. Remove this `skip` once + /// the output backends surface group attribution across JSON/SARIF/human formats. + #[serde(skip)] + pub groups: std::collections::BTreeSet, + /// Set when the finding is suppressed by policy (e.g. `[ignore].packages`). Suppressed + /// findings stay in the report (never dropped) but do not trigger the `fail_on` exit + /// condition. Surfaced in every output format (never omitted) so a suppressed finding + /// is always visible as suppressed rather than silently gone. + #[serde(skip_serializing_if = "Option::is_none")] + pub suppressed: Option, +} + +/// Why a finding was suppressed (reported but excluded from the exit condition). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum SuppressionReason { + /// The package matched an `[ignore].packages` entry. + IgnoredPackage, +} + +impl SuppressionReason { + /// Short human-readable label for report output. + pub fn label(self) -> &'static str { + match self { + SuppressionReason::IgnoredPackage => "ignored package", + } + } } /// A vulnerability database containing advisories and indexed lookups. diff --git a/src/vulnerability/matcher.rs b/src/vulnerability/matcher.rs index 6a1e912..92ba0c2 100644 --- a/src/vulnerability/matcher.rs +++ b/src/vulnerability/matcher.rs @@ -198,6 +198,8 @@ impl VulnerabilityMatcher { vulnerability: vulnerability.clone(), is_direct: dependency.is_direct, source_file: dependency.source_file.clone(), + groups: std::collections::BTreeSet::new(), + suppressed: None, }); } } @@ -590,6 +592,8 @@ mod tests { vulnerability: create_test_vulnerability(), is_direct: true, source_file: None, + groups: std::collections::BTreeSet::new(), + suppressed: None, }]; let analysis = matcher.analyze_fixes(&matches); diff --git a/tests/config_wiring.rs b/tests/config_wiring.rs index 0d3230f..cce213b 100644 --- a/tests/config_wiring.rs +++ b/tests/config_wiring.rs @@ -84,6 +84,11 @@ fn every_config_field_reaches_effective_settings() { // ----- Group A: [sources] ----- assert_eq!(merged.sources, vec!["osv"], "sources.enabled: {WIRING}"); + // fail_on_partial = false surfaces as no_fail_on_partial = true. + assert!( + merged.no_fail_on_partial, + "sources.fail_on_partial: {WIRING}" + ); // ----- Group A: [resolver] ----- assert_eq!( @@ -117,6 +122,11 @@ fn every_config_field_reaches_effective_settings() { vec!["GHSA-aaaa-bbbb-cccc"], "ignore.while_no_fix: {WIRING}" ); + assert_eq!( + merged.ignore_packages, + vec!["internal-pkg"], + "ignore.packages: {WIRING}" + ); // ----- Group A: [maintenance] (the AuditArgs-bound subset) ----- // enabled = false surfaces as no_maintenance_check = true. @@ -177,6 +187,14 @@ fn every_config_field_reaches_effective_settings() { !config.notifications.enabled, "notifications.enabled: {WIRING}" ); + + // [groups.*] has no CLI flag, but merge normalizes its keys into group_fail_on + // (keyed by PEP 735-normalized name) for the policy engine. + assert_eq!( + merged.group_fail_on.get("dev"), + Some(&SeverityLevel::Critical), + "groups.dev.fail_on: {WIRING}" + ); } /// `deny_unknown_fields` turns a typo'd config key into a hard error instead of diff --git a/tests/fixtures/config-wiring/full.pysentry.toml b/tests/fixtures/config-wiring/full.pysentry.toml index e29ffb9..fc21454 100644 --- a/tests/fixtures/config-wiring/full.pysentry.toml +++ b/tests/fixtures/config-wiring/full.pysentry.toml @@ -25,6 +25,7 @@ include_scripts = true # default: false [sources] enabled = ["osv"] # default: ["pypa", "pypi", "osv"] +fail_on_partial = false # default: true [resolver] type = "pip-tools" # default: uv @@ -39,6 +40,7 @@ vulnerability_ttl = 99 # default: 48 [ignore] ids = ["PYSEC-2024-0001"] # default: [] while_no_fix = ["GHSA-aaaa-bbbb-cccc"] # default: [] +packages = ["internal-pkg"] # default: [] [http] timeout = 99 # default: 120 @@ -62,3 +64,6 @@ enabled = false # default: true [output] quiet = true # default: false + +[groups.dev] # default: {} (consumed by policy engine in Phase 3) +fail_on = "critical" diff --git a/tests/per_group_integration.rs b/tests/per_group_integration.rs index 1f4af9e..26ac0b7 100644 --- a/tests/per_group_integration.rs +++ b/tests/per_group_integration.rs @@ -159,6 +159,55 @@ async fn test_exclude_extra_regression() { ); } +/// Wiring check for `build_group_attribution` (v0.5.0 policy plan, Phase 1): drives it +/// against the same real uv.lock + pyproject.toml fixture used above, through the parser's +/// real `name()` string ("uv.lock") rather than a copy of the match-arm literal — the same +/// seam `build_transitive_roots_dispatches_poetry_by_parser_name` guards for the sibling +/// transitive-roots feature. Proves the dispatch, the edge reader, and the reachability walk +/// agree end to end, independent of `build_group_attribution`'s own unit tests. +#[tokio::test] +async fn test_group_attribution_tags_prod_and_dev_from_real_fixture() { + use pysentry::parsers::graph::build_group_attribution; + + let attribution = build_group_attribution(Path::new(FIXTURE_DIR), "uv.lock").await; + + // prod = ["httpx"]; httpcore is httpx's exclusive transitive (not shared with the + // main-only `requests` dependency), so it only shows up via the prod closure. + let prod: HashSet = attribution + .get(&pysentry::types::PackageName::new("httpcore")) + .cloned() + .unwrap_or_default(); + assert!( + prod.contains("prod"), + "httpcore (httpx transitive) must be tagged prod, got: {prod:?}" + ); + + let dev: HashSet = attribution + .get(&pysentry::types::PackageName::new("pytest")) + .cloned() + .unwrap_or_default(); + assert!( + dev.contains("dev"), + "pytest must be tagged dev, got: {dev:?}" + ); + + assert!( + attribution + .get(&pysentry::types::PackageName::new("iniconfig")) + .is_some_and(|g| g.contains("dev")), + "iniconfig (pytest transitive) must be tagged dev" + ); + + // requests is main-only (not declared by any group), so its exclusive transitives + // must get no attribution entry at all. + assert!( + attribution + .get(&pysentry::types::PackageName::new("urllib3")) + .is_none_or(|g| g.is_empty()), + "urllib3 (requests-only transitive) must not be attributed to any group" + ); +} + /// (d) --group unknown_group: binary exits non-zero; stderr contains "available groups:". #[test] fn test_unknown_group_errors() {