From 0506d16740518fe58f73078f2805f112f81fbaaf Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Thu, 18 Aug 2022 19:29:11 -0400 Subject: [PATCH] Update imports.lock less frequently This patch reduces the frequency that imports.lock updates by doing a few things: * Imports are only updated when necessary for the vet to pass. If vet would successfully pass without the new audits, only changes to notes, criteria descriptions, revoked audits, and violations will be pulled. * Some commands, such as suggest, will always run against the live copy when not --locked, providing the most accurate results possible. * Only audits for crates in your crate graph will be imported. * When an update is required, all new audits for the given crate will be fetched and locked, to batch updates as much as possible. This required doing some changes to how imports are handled under the hood, as well as a number of changes to the resolver in order to track whether fresh imports are required with sufficient accuracy. Imports are also now fetched in more APIs in order to have the most up-to-date information. The changes to the resolver involved piping through "caveats" which are determined when solving paths. This is an expansion on the existing "needs_exemption"/"fully_audited" flags which were being used to generate stats after a successful vet to also handle freshly imported audits which should only be used when necessary. Special handling was also needed to make sure that audits for unused criteria are not counted by tracking caveats on a per-criteria basis and bubbling them out. --- src/errors.rs | 65 +- src/format.rs | 8 + src/main.rs | 106 ++- src/resolver.rs | 823 ++++++++++-------- src/serialization.rs | 5 + src/storage.rs | 364 ++++++-- src/tests/import.rs | 738 ++++++++++++++++ src/tests/mod.rs | 9 + ...__import__existing_peer_add_violation.snap | 13 + ..._existing_peer_import_custom_criteria.snap | 12 + ...ort__existing_peer_import_delta_audit.snap | 22 + ...__import__existing_peer_remove_unused.snap | 13 + ...__import__existing_peer_revoked_audit.snap | 10 + ...ts__import__existing_peer_skip_import.snap | 7 + ...__new_audit_for_unused_criteria_basic.snap | 12 + ..._audit_for_unused_criteria_transitive.snap | 12 + ...o_vet__tests__import__new_peer_import.snap | 11 + ...mport__peer_audits_exemption_minimize.snap | 37 + ...rt__peer_audits_exemption_no_minimize.snap | 7 + ..._builtin-simple-audit-as-default-root.snap | 2 +- ...__builtin-simple-audit-as-weaker-root.snap | 2 +- ...-deps-unaudited-adds-uneeded-criteria.snap | 2 +- 22 files changed, 1759 insertions(+), 521 deletions(-) create mode 100644 src/tests/import.rs create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_add_violation.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_custom_criteria.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_delta_audit.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_remove_unused.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_revoked_audit.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__existing_peer_skip_import.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_basic.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_transitive.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__new_peer_import.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_minimize.snap create mode 100644 src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_no_minimize.snap diff --git a/src/errors.rs b/src/errors.rs index 56e352cd..8e524071 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -185,6 +185,12 @@ pub enum StoreAcquireError { #[error(transparent)] #[diagnostic(transparent)] Validate(#[from] StoreValidateErrors), + #[error(transparent)] + #[diagnostic(transparent)] + FetchAuditError(#[from] FetchAuditError), + #[diagnostic(transparent)] + #[error(transparent)] + CriteriaChange(#[from] CriteriaChangeErrors), } #[derive(Debug, Error, Diagnostic)] @@ -203,6 +209,26 @@ pub enum StoreCommitError { ), } +#[derive(Debug, Error, Diagnostic)] +#[error("Some of your imported audits changed their criteria descriptions")] +#[diagnostic()] +pub struct CriteriaChangeErrors { + #[related] + pub errors: Vec, +} +#[derive(Debug, Error, Diagnostic)] +// FIXME: it would be rad if this was a diff! +#[error( + "{import_name}'s '{criteria_name}' criteria changed from\n\n{old_desc}\n\nto\n\n{new_desc}\n" +)] +#[diagnostic(help("Run `cargo vet regenerate imports` to accept this new definition"))] +pub struct CriteriaChangeError { + pub import_name: ImportName, + pub criteria_name: ForeignCriteriaName, + pub old_desc: String, + pub new_desc: String, +} + //////////////////////////////////////////////////////////// // StoreValidateErrors //////////////////////////////////////////////////////////// @@ -436,38 +462,25 @@ pub enum FetchAuditError { #[source] error: url::ParseError, }, + #[error("{import_name}'s '{criteria_name}' criteria is missing a description")] + MissingCriteriaDescription { + import_name: ImportName, + criteria_name: ForeignCriteriaName, + }, + #[error("{import_name}'s '{criteria_name}' criteria description URI is invalid: '{url}'")] + InvalidCriteriaDescriptionUrl { + import_name: ImportName, + criteria_name: ForeignCriteriaName, + url: String, + #[source] + error: url::ParseError, + }, #[diagnostic(transparent)] #[error(transparent)] Download(#[from] DownloadError), #[diagnostic(transparent)] #[error(transparent)] Toml(#[from] LoadTomlError), - #[diagnostic(transparent)] - #[error(transparent)] - Validate(#[from] StoreValidateErrors), - #[diagnostic(transparent)] - #[error(transparent)] - CriteriaChange(#[from] CriteriaChangeErrors), -} - -#[derive(Debug, Error, Diagnostic)] -#[error("Some of your imported audits changed their criteria descriptions")] -#[diagnostic()] -pub struct CriteriaChangeErrors { - #[related] - pub errors: Vec, -} -#[derive(Debug, Error, Diagnostic)] -// FIXME: it would be rad if this was a diff! -#[error( - "{import_name}'s '{criteria_name}' criteria changed from\n\n{old_desc}\n\nto\n\n{new_desc}\n" -)] -#[diagnostic(help("Run `cargo vet regenerate imports` to accept this new definition"))] -pub struct CriteriaChangeError { - pub import_name: ImportName, - pub criteria_name: ForeignCriteriaName, - pub old_desc: String, - pub new_desc: String, } ////////////////////////////////////////////////////////// diff --git a/src/format.rs b/src/format.rs index 458a1102..02e03b0b 100644 --- a/src/format.rs +++ b/src/format.rs @@ -162,6 +162,14 @@ pub struct AuditEntry { pub criteria: Vec>, pub kind: AuditKind, pub notes: Option, + /// A non-serialized member which indicates whether this audit is a "fresh" + /// audit. This will be set for all audits imported found in the remote + /// audits file which aren't also found in the local `imports.lock` cache. + /// + /// This should almost always be `false`, and only set to `true` by the + /// import handling code. + #[serde(skip)] + pub is_fresh_import: bool, } /// Implement PartialOrd manually because the order we want for sorting is diff --git a/src/main.rs b/src/main.rs index 186e465d..ee00422f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -524,9 +524,9 @@ fn cmd_inspect( let package = &*sub_args.package; let fetched = { - let store = Store::acquire(cfg)?; - let cache = Cache::acquire(cfg)?; let network = Network::acquire(cfg); + let store = Store::acquire(cfg, network.as_ref(), false)?; + let cache = Cache::acquire(cfg)?; // Record this command for magic in `vet certify` cache.set_last_fetch(FetchCommand::Inspect { @@ -607,14 +607,18 @@ fn cmd_certify( sub_args: &CertifyArgs, ) -> Result<(), miette::Report> { // Certify that you have reviewed a crate's source for some version / delta - let mut store = Store::acquire(cfg)?; let network = Network::acquire(cfg); + let mut store = Store::acquire(cfg, network.as_ref(), false)?; // Grab the last fetch and immediately drop the cache let last_fetch = Cache::acquire(cfg)?.get_last_fetch(); do_cmd_certify(out, cfg, sub_args, &mut store, network.as_ref(), last_fetch)?; + // Re-run the resolver after adding the new `certify`. This will be used to + // potentially update imports. + store.maybe_update_imports_file(cfg, false); + store.commit()?; Ok(()) } @@ -707,7 +711,7 @@ fn do_cmd_certify( let criteria_mapper = CriteriaMapper::new( &store.audits.criteria, - &store.imports, + store.imported_audits(), &store.config.imports, ); @@ -914,6 +918,7 @@ fn do_cmd_certify( .collect(), who, notes, + is_fresh_import: false, }; store @@ -1087,7 +1092,7 @@ fn cmd_record_violation( sub_args: &RecordViolationArgs, ) -> Result<(), miette::Report> { // Mark a package as a violation - let mut store = Store::acquire(cfg)?; + let mut store = Store::acquire_offline(cfg)?; let kind = AuditKind::Violation { violation: sub_args.versions.clone(), @@ -1134,6 +1139,7 @@ fn cmd_record_violation( criteria, who, notes, + is_fresh_import: false, }; store @@ -1156,7 +1162,7 @@ fn cmd_add_exemption( sub_args: &AddExemptionArgs, ) -> Result<(), miette::Report> { // Add an exemption entry - let mut store = Store::acquire(cfg)?; + let mut store = Store::acquire_offline(cfg)?; let dependency_criteria = if sub_args.dependency_criteria.is_empty() { // TODO: look at the current audits to infer this? prompt? @@ -1229,8 +1235,8 @@ fn cmd_suggest( ) -> Result<(), miette::Report> { // Run the checker to validate that the current set of deps is covered by the current cargo vet store trace!("suggesting..."); - let suggest_store = Store::acquire(cfg)?.clone_for_suggest(); let network = Network::acquire(cfg); + let suggest_store = Store::acquire(cfg, network.as_ref(), false)?.clone_for_suggest(); // DO THE THING!!!! let report = resolver::resolve( @@ -1261,28 +1267,21 @@ fn cmd_regenerate_imports( ) -> Result<(), miette::Report> { trace!("regenerating imports..."); - let mut store = Store::acquire(cfg)?; - let network = Network::acquire(cfg); - - if let Some(network) = &network { - if !cfg.cli.locked { - // Literally the only difference between this command and fetch-imports - // is that we pass `accept_changes = true` - tokio::runtime::Handle::current() - .block_on(store.fetch_foreign_audits(network, true))?; - store.commit()?; - return Ok(()); - } + if cfg.cli.locked { + // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered, + // or if this should be a hard error, or if we should ignore the --locked flag and + // just do it anyway + writeln!( + out, + "warning: ran `regenerate imports` with --locked, this won't do anything!" + ); + return Ok(()); } - // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered, - // or if this should be a hard error, or if we should ignore the --locked flag and - // just do it anyway - writeln!( - out, - "warning: ran `regenerate imports` with --locked, this won't do anything!" - ); - + let network = Network::acquire(cfg); + let mut store = Store::acquire(cfg, network.as_ref(), true)?; + store.maybe_update_imports_file(cfg, true); + store.commit()?; Ok(()) } @@ -1292,7 +1291,7 @@ fn cmd_regenerate_audit_as( _sub_args: &RegenerateAuditAsCratesIoArgs, ) -> Result<(), miette::Report> { trace!("regenerating audit-as-crates-io..."); - let mut store = Store::acquire(cfg)?; + let mut store = Store::acquire_offline(cfg)?; fix_audit_as(cfg, &mut store)?; @@ -1347,8 +1346,8 @@ fn cmd_regenerate_exemptions( _sub_args: &RegenerateExemptionsArgs, ) -> Result<(), miette::Report> { trace!("regenerating exemptions..."); - let mut store = Store::acquire(cfg)?; let network = Network::acquire(cfg); + let mut store = Store::acquire(cfg, network.as_ref(), false)?; minimize_exemptions(cfg, &mut store, network.as_ref())?; @@ -1457,6 +1456,9 @@ pub fn minimize_exemptions( // Alright there's the new exemptions store.config.exemptions = new_exemptions; + // Re-vet and ensure that imports are updated after changing exemptions. + store.maybe_update_imports_file(cfg, false); + Ok(()) } @@ -1466,9 +1468,9 @@ fn cmd_diff(out: &Arc, cfg: &Config, sub_args: &DiffArgs) -> Result<(), let package = &*sub_args.package; let (fetched1, fetched2) = { - let store = Store::acquire(cfg)?; - let cache = Cache::acquire(cfg)?; let network = Network::acquire(cfg); + let store = Store::acquire(cfg, network.as_ref(), false)?; + let cache = Cache::acquire(cfg)?; // Record this command for magic in `vet certify` cache.set_last_fetch(FetchCommand::Diff { @@ -1552,16 +1554,10 @@ fn cmd_check(out: &Arc, cfg: &Config, sub_args: &CheckArgs) -> Result<( // Run the checker to validate that the current set of deps is covered by the current cargo vet store trace!("vetting..."); - let mut store = Store::acquire(cfg)?; let network = Network::acquire(cfg); + let mut store = Store::acquire(cfg, network.as_ref(), false)?; if !cfg.cli.locked { - // Try to update the foreign audits (imports) - if let Some(network) = &network { - tokio::runtime::Handle::current() - .block_on(store.fetch_foreign_audits(network, false))?; - } - // Check if any of our first-parties are in the crates.io registry check_audit_as_crates_io(cfg, &store)?; } @@ -1598,6 +1594,7 @@ fn cmd_check(out: &Arc, cfg: &Config, sub_args: &CheckArgs) -> Result<( // Err(eyre!("report contains errors"))?; panic_any(ExitPanic(-1)); } else { + store.imports = store.get_updated_imports_file(&report, false); store.commit()?; } @@ -1611,25 +1608,23 @@ fn cmd_fetch_imports( ) -> Result<(), miette::Report> { trace!("fetching imports..."); - let mut store = Store::acquire(cfg)?; - let network = Network::acquire(cfg); + if cfg.cli.locked { + // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered, + // or if this should be a hard error, or if we should ignore the --locked flag and + // just do it anyway + writeln!( + out, + "warning: ran fetch-imports with --locked, this won't do anything!" + ); - if let Some(network) = &network { - if !cfg.cli.locked { - tokio::runtime::Handle::current() - .block_on(store.fetch_foreign_audits(network, false))?; - store.commit()?; - return Ok(()); - } + return Ok(()); } - // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered, - // or if this should be a hard error, or if we should ignore the --locked flag and - // just do it anyway - writeln!( - out, - "warning: ran fetch-imports with --locked, this won't do anything!" - ); + let network = Network::acquire(cfg); + let mut store = Store::acquire(cfg, network.as_ref(), false)?; + + store.maybe_update_imports_file(cfg, true); + store.commit()?; Ok(()) } @@ -1656,7 +1651,8 @@ fn cmd_dump_graph( fn cmd_fmt(_out: &Arc, cfg: &Config, _sub_args: &FmtArgs) -> Result<(), miette::Report> { // Reformat all the files (just load and store them, formatting is implicit). trace!("formatting..."); - let store = Store::acquire(cfg)?; + // We don't need to fetch foreign audits to format files + let store = Store::acquire_offline(cfg)?; store.commit()?; Ok(()) } diff --git a/src/resolver.rs b/src/resolver.rs index ae935a6a..2dd6e20c 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -67,13 +67,15 @@ use futures_util::future::join_all; use miette::IntoDiagnostic; use serde::Serialize; use serde_json::json; +use std::cmp::Reverse; +use std::collections::BinaryHeap; use std::sync::Arc; use tracing::{error, trace, trace_span}; use crate::errors::SuggestError; use crate::format::{ - self, AuditKind, CriteriaName, CriteriaStr, Delta, DiffStat, ExemptedDependency, ImportName, - ImportsFile, PackageName, PackageStr, PolicyEntry, RemoteImport, SAFE_TO_DEPLOY, SAFE_TO_RUN, + self, AuditKind, AuditsFile, CriteriaName, CriteriaStr, Delta, DiffStat, ExemptedDependency, + ImportName, PackageName, PackageStr, PolicyEntry, RemoteImport, SAFE_TO_DEPLOY, SAFE_TO_RUN, }; use crate::format::{FastMap, FastSet, SortedMap, SortedSet}; use crate::network::Network; @@ -116,6 +118,8 @@ pub struct Success { pub vetted_partially: Vec, /// Third-party packages that were successfully vetted using only 'audits' pub vetted_fully: Vec, + /// Third-party packages that needed fresh imports to be successfully vetted + pub needed_fresh_imports: SortedSet, } #[derive(Debug, Clone)] @@ -291,14 +295,10 @@ pub struct DepGraph<'a> { pub struct ResolveResult<'a> { /// The set of criteria we validated for this package. pub validated_criteria: CriteriaSet, - /// The set of criteria we validated for this package without 'exemptions' entries. - pub fully_audited_criteria: CriteriaSet, /// Individual search results for each criteria. pub search_results: Vec>, /// Whether there was an exemption for this exact version. pub directly_exempted: bool, - /// Whether we ever needed the not-fully_audited_criteria for our reverse-deps. - pub needed_exemption: bool, } pub type PolicyFailures = SortedMap; @@ -315,8 +315,9 @@ pub struct AuditFailure { pub enum SearchResult<'a> { /// We found a path, criteria validated. Connected { - /// Whether we found a path to a fully_audited entry - fully_audited: bool, + /// Caveats which were required to build the audit chain for this + /// Criteria. + caveats: Caveats, }, /// We failed to find a *proper* path, criteria not valid, but adding in failing /// edges caused by our dependencies not meeting criteria created a connection! @@ -340,6 +341,29 @@ pub enum SearchResult<'a> { }, } +// NOTE: There's probably a more efficient representation for these in the +// general case. +/// Caveats which apply to the results of an audit. +#[derive(Debug, Clone, Default)] +pub struct Caveats { + /// The set of packages which required exemptions in order to successfully + /// audit. + pub needed_exemptions: SortedSet, + + /// The set of packages which required fresh imports in order to + /// successfully audit. + pub needed_fresh_imports: SortedSet, +} + +impl Caveats { + /// Union the given caveat set with this set of caveats, mutating `self`. + fn add(&mut self, other: &Caveats) { + self.needed_exemptions.extend(&other.needed_exemptions); + self.needed_fresh_imports + .extend(&other.needed_fresh_imports); + } +} + /// A graph of the audits for a package. /// /// The nodes of the graph are Versions and the edges are audits. @@ -367,6 +391,21 @@ pub enum SearchResult<'a> { /// used for `suggest`. pub type AuditGraph<'a> = SortedMap<&'a Version, Vec>>; +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +enum DeltaEdgeOrigin { + /// This edge represents an audit from the store, either within audits.toml + /// or within the cached imports.lock file. These edges will be tried first. + StoredAudit, + /// This edge represents an exemption. These edges will be tried after + /// stored audits, but before freshly-imported audits have been attempted. + Exemption, + /// This edge represents an imported audit from a peer which is only present + /// on the remote server, and not present in the cached imports.lock file. + /// These edges will be tried after all locally-available audits have been + /// attempted. + FreshImportedAudit, +} + /// A directed edge in the graph of audits. This may be forward or backwards, /// depending on if we're searching from "roots" (forward) or the target (backward). /// The source isn't included because that's implicit in the Node. @@ -379,9 +418,9 @@ pub struct DeltaEdge<'a> { /// Requirements that dependencies must satisfy for the edge to be valid. /// If a dependency isn't mentioned, then it defaults to `criteria`. dependency_criteria: FastMap, CriteriaSet>, - /// Whether this edge represents an exemption. These will initially - /// be ignored, and then used only if we can't find a path. - is_exemption: bool, + /// The origin of this edge. See `DeltaEdgeOrigin`'s documentation for more + /// details. + origin: DeltaEdgeOrigin, } const NUM_BUILTINS: usize = 2; @@ -405,7 +444,7 @@ fn builtin_criteria() -> [CriteriaInfo; NUM_BUILTINS] { impl CriteriaMapper { pub fn new( criteria: &SortedMap, - imports: &ImportsFile, + imports: &SortedMap, mappings: &SortedMap, ) -> CriteriaMapper { // First, build a list of all our criteria @@ -416,7 +455,7 @@ impl CriteriaMapper { implies: v.implies.iter().map(|s| s.to_string()).collect::>(), }); let builtins = builtin_criteria(); - let foreigns = imports.audits.iter().flat_map(|(import, audit_file)| { + let foreigns = imports.iter().flat_map(|(import, audit_file)| { audit_file.criteria.iter().map(move |(k, v)| CriteriaInfo { namespace: CriteriaNamespace::Foreign(import.clone()), raw_name: k.clone(), @@ -800,33 +839,9 @@ impl CriteriaFailureSet { impl ResolveResult<'_> { fn with_no_criteria(empty: CriteriaSet) -> Self { Self { - validated_criteria: empty.clone(), - fully_audited_criteria: empty, + validated_criteria: empty, search_results: vec![], directly_exempted: false, - needed_exemption: false, - } - } - - fn contains(&mut self, other: &CriteriaSet) -> bool { - if self.fully_audited_criteria.contains(other) { - true - } else if self.validated_criteria.contains(other) { - self.needed_exemption = true; - true - } else { - false - } - } - - fn _has_criteria(&mut self, criteria_idx: usize) -> bool { - if self.fully_audited_criteria.has_criteria(criteria_idx) { - true - } else if self.validated_criteria.has_criteria(criteria_idx) { - self.needed_exemption = true; - true - } else { - false } } } @@ -1294,7 +1309,6 @@ impl<'a> DepGraph<'a> { // Dummy values for corner cases pub static ROOT_VERSION: Version = Version::new(0, 0, 0); -static NO_AUDITS: Vec = Vec::new(); #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub enum ResolveDepth { @@ -1317,7 +1331,7 @@ pub fn resolve<'a>( let criteria_mapper = CriteriaMapper::new( &store.audits.criteria, - &store.imports, + store.imported_audits(), &store.config.imports, ); trace!("built CriteriaMapper!"); @@ -1327,6 +1341,7 @@ pub fn resolve<'a>( vec![ResolveResult::with_no_criteria(criteria_mapper.no_criteria()); graph.nodes.len()]; let mut root_failures = RootFailures::new(); let mut violations = SortedMap::new(); + let mut root_caveats = Caveats::default(); // Actually vet the build graph for &pkgidx in &graph.topo_index { @@ -1364,6 +1379,7 @@ pub fn resolve<'a>( &mut results, &mut violations, &mut root_failures, + &mut root_caveats, pkgidx, ); } @@ -1404,6 +1420,7 @@ pub fn resolve<'a>( &mut results, &mut violations, &mut root_failures, + &mut root_caveats, pkgidx, ); } else { @@ -1499,7 +1516,7 @@ pub fn resolve<'a>( } let result = &results[pkgidx]; - if !result.needed_exemption { + if !root_caveats.needed_exemptions.contains(&pkgidx) { vetted_fully.push(pkgidx); } else if result.directly_exempted { vetted_with_exemptions.push(pkgidx); @@ -1516,6 +1533,7 @@ pub fn resolve<'a>( vetted_with_exemptions, vetted_partially, vetted_fully, + needed_fresh_imports: root_caveats.needed_fresh_imports, }), } } @@ -1532,19 +1550,41 @@ fn resolve_third_party<'a>( let package = &graph.nodes[pkgidx]; let exemptions = store.config.exemptions.get(package.name); - let own_audits = ( - CriteriaNamespace::Local, - store.audits.audits.get(package.name).unwrap_or(&NO_AUDITS), - ); - // FIXME: ideally we wouldn't clone the CriteriaNamespace every time we iterate all_criteria - // (which happens a lot when inspecting violations). - let foreign_audits = store.imports.audits.iter().map(|(import_name, audits)| { - ( - CriteriaNamespace::Foreign(import_name.clone()), - audits.audits.get(package.name).unwrap_or(&NO_AUDITS), - ) - }); - let all_audits = Some(own_audits).into_iter().chain(foreign_audits); + // Pre-build the namespaces for each audit so that we can take a reference + // to each one as-needed rather than cloning the name each time. + let foreign_namespaces: Vec = store + .imported_audits() + .keys() + .map(|import_name| CriteriaNamespace::Foreign(import_name.clone())) + .collect(); + + // Each of our own audits should be put into the "local" criteria namespace. + let own_audits = store + .audits + .audits + .get(package.name) + .map(|v| &v[..]) + .unwrap_or(&[]) + .iter() + .map(|audit| (&CriteriaNamespace::Local, audit)); + + // Each foreign audit should be put into a "foreign" criteria namespace. + let foreign_audits = store + .imported_audits() + .values() + .enumerate() + .flat_map(|(idx, audits)| { + let namespace = &foreign_namespaces[idx]; + audits + .audits + .get(package.name) + .map(|v| &v[..]) + .unwrap_or(&[]) + .iter() + .map(move |audit| (namespace, audit)) + }); + + let all_audits = own_audits.chain(foreign_audits); // See AuditGraph's docs for details on the lowering we do here let mut forward_audits = AuditGraph::new(); @@ -1552,49 +1592,53 @@ fn resolve_third_party<'a>( let mut violation_nodes = Vec::new(); // Collect up all the deltas, their criteria, and dependency_criteria - for (namespace, entries) in all_audits.clone() { - for entry in entries { - // For uniformity, model a Full Audit as `0.0.0 -> x.y.z` - let (from_ver, to_ver, dependency_criteria) = match &entry.kind { - AuditKind::Full { - version, - dependency_criteria, - } => (&ROOT_VERSION, version, dependency_criteria), - AuditKind::Delta { - delta, - dependency_criteria, - } => (&delta.from, &delta.to, dependency_criteria), - AuditKind::Violation { .. } => { - violation_nodes.push((namespace.clone(), entry)); - continue; - } - }; + for (namespace, entry) in all_audits.clone() { + // For uniformity, model a Full Audit as `0.0.0 -> x.y.z` + let (from_ver, to_ver, dependency_criteria) = match &entry.kind { + AuditKind::Full { + version, + dependency_criteria, + } => (&ROOT_VERSION, version, dependency_criteria), + AuditKind::Delta { + delta, + dependency_criteria, + } => (&delta.from, &delta.to, dependency_criteria), + AuditKind::Violation { .. } => { + violation_nodes.push((namespace.clone(), entry)); + continue; + } + }; - let criteria = criteria_mapper.criteria_from_namespaced_entry(&namespace, entry); - // Convert all the custom criteria to CriteriaSets - let dependency_criteria: FastMap<_, _> = dependency_criteria - .iter() - .map(|(pkg_name, criteria)| { - ( - &**pkg_name, - criteria_mapper.criteria_from_namespaced_list(&namespace, criteria), - ) - }) - .collect(); + let criteria = criteria_mapper.criteria_from_namespaced_entry(namespace, entry); + // Convert all the custom criteria to CriteriaSets + let dependency_criteria: FastMap<_, _> = dependency_criteria + .iter() + .map(|(pkg_name, criteria)| { + ( + &**pkg_name, + criteria_mapper.criteria_from_namespaced_list(namespace, criteria), + ) + }) + .collect(); - forward_audits.entry(from_ver).or_default().push(DeltaEdge { - version: to_ver, - criteria: criteria.clone(), - dependency_criteria: dependency_criteria.clone(), - is_exemption: false, - }); - backward_audits.entry(to_ver).or_default().push(DeltaEdge { - version: from_ver, - criteria, - dependency_criteria, - is_exemption: false, - }); - } + let origin = if entry.is_fresh_import { + DeltaEdgeOrigin::FreshImportedAudit + } else { + DeltaEdgeOrigin::StoredAudit + }; + + forward_audits.entry(from_ver).or_default().push(DeltaEdge { + version: to_ver, + criteria: criteria.clone(), + dependency_criteria: dependency_criteria.clone(), + origin, + }); + backward_audits.entry(to_ver).or_default().push(DeltaEdge { + version: from_ver, + criteria, + dependency_criteria, + origin, + }); } // Reject forbidden packages (violations) @@ -1652,47 +1696,42 @@ fn resolve_third_party<'a>( } // Note if this entry conflicts with any audits - for (namespace, audits) in all_audits.clone() { - for audit in audits { - let audit_criteria = - criteria_mapper.criteria_from_namespaced_entry(&namespace, audit); - let has_violation = violation_criterias - .iter() - .any(|v| audit_criteria.contains(v)); - if !has_violation { - continue; - } - match &audit.kind { - AuditKind::Full { version, .. } => { - if violation_range.matches(version) { - violations.entry(pkgidx).or_default().push( - ViolationConflict::AuditConflict { - violation_source: violation_source.clone(), - violation: (*violation_entry).clone(), - audit_source: namespace.clone(), - audit: audit.clone(), - }, - ); - } - } - AuditKind::Delta { delta, .. } => { - if violation_range.matches(&delta.from) - || violation_range.matches(&delta.to) - { - violations.entry(pkgidx).or_default().push( - ViolationConflict::AuditConflict { - violation_source: violation_source.clone(), - violation: (*violation_entry).clone(), - audit_source: namespace.clone(), - audit: audit.clone(), - }, - ); - } + for (namespace, audit) in all_audits.clone() { + let audit_criteria = criteria_mapper.criteria_from_namespaced_entry(namespace, audit); + let has_violation = violation_criterias + .iter() + .any(|v| audit_criteria.contains(v)); + if !has_violation { + continue; + } + match &audit.kind { + AuditKind::Full { version, .. } => { + if violation_range.matches(version) { + violations.entry(pkgidx).or_default().push( + ViolationConflict::AuditConflict { + violation_source: violation_source.clone(), + violation: (*violation_entry).clone(), + audit_source: namespace.clone(), + audit: audit.clone(), + }, + ); } - AuditKind::Violation { .. } => { - // don't care + } + AuditKind::Delta { delta, .. } => { + if violation_range.matches(&delta.from) || violation_range.matches(&delta.to) { + violations.entry(pkgidx).or_default().push( + ViolationConflict::AuditConflict { + violation_source: violation_source.clone(), + violation: (*violation_entry).clone(), + audit_source: namespace.clone(), + audit: audit.clone(), + }, + ); } } + AuditKind::Violation { .. } => { + // don't care + } } } @@ -1743,19 +1782,18 @@ fn resolve_third_party<'a>( version: to_ver, criteria: criteria.clone(), dependency_criteria: dependency_criteria.clone(), - is_exemption: true, + origin: DeltaEdgeOrigin::Exemption, }); backward_audits.entry(to_ver).or_default().push(DeltaEdge { version: from_ver, criteria, dependency_criteria, - is_exemption: true, + origin: DeltaEdgeOrigin::Exemption, }); } } let mut validated_criteria = criteria_mapper.no_criteria(); - let mut fully_audited_criteria = criteria_mapper.no_criteria(); let mut search_results = vec![]; for criteria in criteria_mapper.all_criteria_iter() { let result = search_for_path( @@ -1765,17 +1803,15 @@ fn resolve_third_party<'a>( &forward_audits, graph, criteria_mapper, - package, results, + pkgidx, + &package.normal_and_build_deps, ); match result { - SearchResult::Connected { fully_audited } => { - // We found a path, hooray, criteria validated! - if fully_audited { - fully_audited_criteria.unioned_with(criteria); - } + SearchResult::Connected { caveats } => { + // We found a patch to satisfy this criteria. validated_criteria.unioned_with(criteria); - search_results.push(SearchResult::Connected { fully_audited }); + search_results.push(SearchResult::Connected { caveats }); } SearchResult::PossiblyConnected { failed_deps } => { // We failed but found a possible solution if our dependencies were better. @@ -1795,8 +1831,9 @@ fn resolve_third_party<'a>( &backward_audits, graph, criteria_mapper, - package, results, + pkgidx, + &package.normal_and_build_deps, ); if let SearchResult::Disconnected { reachable_from_root: reachable_from_target, @@ -1824,14 +1861,69 @@ fn resolve_third_party<'a>( // We've completed our graph analysis for this package, now record the results results[pkgidx] = ResolveResult { validated_criteria, - fully_audited_criteria, - directly_exempted, search_results, - // Only gets found out later, for now, assume not. - needed_exemption: false, + directly_exempted, }; } +/// Updates `caveats`, and `failed_deps` to include any caveats or failed +/// dependencies involved in requiring `dependencies` to satisfy the given +/// criteria. +#[allow(clippy::too_many_arguments)] +fn get_dependency_criteria_caveats( + dep_graph: &DepGraph, + criteria_mapper: &CriteriaMapper, + results: &[ResolveResult<'_>], + dependencies: &[PackageIdx], + base_criteria: &CriteriaSet, + dependency_criteria: &FastMap, CriteriaSet>, + caveats: &mut Caveats, + failed_deps: &mut SortedMap, +) { + for &depidx in dependencies { + let dep_package = &dep_graph.nodes[depidx]; + let dep_results = &results[depidx]; + + // If no custom criteria is specified, then require our dependency to match + // the base criteria that we're trying to validate. This makes audits effectively + // break down their criteria into individually verifiable components instead of + // purely "all or nothing". + // + // e.g. a safe-to-deploy audit with some deps that are only safe-to-run + // still audits for safe-to-run, but not safe-to-deploy. Similarly so for + // `[safe-to-run, some-other-criteria]` validating each criteria individually. + let dep_req = dependency_criteria + .get(dep_package.name) + .unwrap_or(base_criteria); + + if !dep_results.validated_criteria.contains(dep_req) { + // This dependency's criteria is not satisfied, so add it to the + // failed deps map. + failed_deps + .entry(depidx) + .or_insert_with(|| criteria_mapper.no_criteria()) + .unioned_with(dep_req); + continue; + } + + // Only iterate the minimal set of indices to reduce the number of + // search results we check for caveats, as implied results will have a + // subset of the caveats of the stronger criteria. + for required_criteria_idx in criteria_mapper.minimal_indices(dep_req) { + // Check if the original search results succeeded here, and if it + // did record the relevant caveats. It's OK if we don't see + // `Connected` here, as that just means a policy overwrote our + // failure. + if let SearchResult::Connected { + caveats: dep_caveats, + } = &dep_results.search_results[required_criteria_idx] + { + caveats.add(dep_caveats); + } + } + } +} + #[allow(clippy::too_many_arguments)] fn search_for_path<'a>( cur_criteria: &CriteriaSet, @@ -1840,160 +1932,143 @@ fn search_for_path<'a>( audit_graph: &AuditGraph<'a>, dep_graph: &DepGraph<'a>, criteria_mapper: &CriteriaMapper, - package: &PackageNode<'a>, - results: &mut [ResolveResult], + results: &[ResolveResult], + pkgidx: PackageIdx, + dependencies: &[PackageIdx], ) -> SearchResult<'a> { - // Search for any path through the graph with edges that satisfy cur_criteria. - // Finding any path validates that we satisfy that criteria. All we're doing is - // basic depth-first search with a manual stack. + // Search for any path through the graph with edges that satisfy + // cur_criteria. Finding any path validates that we satisfy that criteria. // - // All full-audits and exemptions have been "desugarred" to a delta from 0.0.0, - // meaning our graph now has exactly one source and one sink, significantly simplifying - // the start and end conditions. + // All full-audits and exemptions have been "desugarred" to a delta from + // 0.0.0, meaning our graph now has exactly one source and one sink, + // significantly simplifying the start and end conditions. // - // Because we want to know if the validation can be done without ever using an - // exemption, we initially "defer" using those edges. This is accomplished - // by wrapping the entire algorithm in a loop, and only taking those edges on the - // next iteration of the outer loop. So if we find a path in the first iteration, - // then that's an unambiguous proof that we didn't need those edges. + // Some edges have caveats which we want to avoid requiring, so we defer + // edges with caveats to be checked later, after all edges without caveats + // have been visited. This is done by storing work to do in a BinaryHeap, + // sorted by the caveats which apply to each node. This means that if we + // find a patch without using a node with caveats, it's unambiguous proof we + // don't need edges with caveats. // - // We apply this same "deferring" trick to edges which fail because of our dependencies. - // Once we run out of both 'exemptions' entries and still don't have a path, we start - // speculatively allowing ourselves to follow those edges. If we find a path by doing that - // then we can reliably "blame" our deps for our own failings. Otherwise we there is - // no possible path, and we are absolutely just missing reviews for ourself. - - // Conclusions - let mut found_path = false; - let mut needed_exemption = false; - let mut needed_failed_edges = false; - let mut failed_deps = SortedMap::::new(); - - // Search State - let mut search_stack = vec![from_version]; + // These caveats extend all the way from exemptions (the least-important + // caveat) to fresh imports and even failed edges (the most-important + // caveat). + struct Node<'a> { + version: &'a Version, + caveats: Caveats, + failed_deps: SortedMap, + } + + impl<'a> Node<'a> { + fn key(&self) -> Reverse<(usize, usize, usize)> { + // Nodes are compared by the number of failed dependencies, fresh + // imports, and exemptions, in that order. Fewer caveats makes the + // node sort higher, as it will be stored in a max heap. + Reverse(( + self.failed_deps.len(), + self.caveats.needed_fresh_imports.len(), + self.caveats.needed_exemptions.len(), + )) + } + } + impl<'a> PartialEq for Node<'a> { + fn eq(&self, other: &Self) -> bool { + self.key() == other.key() + } + } + impl<'a> Eq for Node<'a> {} + impl<'a> PartialOrd for Node<'a> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + impl<'a> Ord for Node<'a> { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.key().cmp(&other.key()) + } + } + + let mut queue = BinaryHeap::new(); + queue.push(Node { + version: from_version, + caveats: Caveats::default(), + failed_deps: SortedMap::new(), + }); + let mut visited = SortedSet::new(); - let mut deferred_exemptions_entries = vec![]; - let mut deferred_failed_edges = vec![]; - - // Loop until we find a path or run out of deferred edges. - loop { - // If there are any deferred edges (only possible on iteration 2+), try to follow them. - // Always prefer following 'exemptions' edges, so that we only dip into failed edges when - // we've completely run out of options. - if let Some(node) = deferred_exemptions_entries.pop() { - // Don't bother if we got to that node some other way. - if visited.contains(node) { + while let Some(Node { + version, + caveats, + failed_deps, + }) = queue.pop() + { + // If We've been to a version before, We're not going to get a better + // result revisiting it, as we visit the "best" edges first. + if !visited.insert(version) { + continue; + } + + // We found a path! Return a search result reflecting what we + // discovered. + if version == to_version { + return if failed_deps.is_empty() { + SearchResult::Connected { caveats } + } else { + SearchResult::PossiblyConnected { failed_deps } + }; + } + + // Apply deltas to move along to the next layer of the search, adding it + // to our queue. + let edges = audit_graph.get(version).map(|v| &v[..]).unwrap_or(&[]); + for edge in edges { + if !edge.criteria.contains(cur_criteria) { + // This edge never would have been useful to us. continue; } - // Ok at this point we officially "need" the exemptions edge. If the search still - // fails, then we won't mention that we used this, since the graph is just broken - // and we can't make any conclusions about whether anything is needed or not! - needed_exemption = true; - search_stack.push(node); - } else if let Some(node) = deferred_failed_edges.pop() { - // Don't bother if we got to that node some other way. - if visited.contains(node) { + if visited.contains(edge.version) { + // We've been to the target of this edge already. continue; } - // Ok at this point we officially "need" the failed edge. If the search still - // fails, then we won't mention that we used this, since the graph is just broken - // and we can't make any conclusions about whether anything is needed or not! - needed_failed_edges = true; - search_stack.push(node); - } - - // Do Depth-First-Search - while let Some(cur_version) = search_stack.pop() { - // Don't revisit nodes, there's never an advantage to doing so, and because deltas - // can go both forwards and backwards in time, cycles are a real concern! - visited.insert(cur_version); - if cur_version == to_version { - // Success! Nothing more to do. - found_path = true; - break; - } - - // Apply deltas to move along to the next "layer" of the search - if let Some(edges) = audit_graph.get(cur_version) { - for edge in edges { - if !edge.criteria.contains(cur_criteria) { - // This edge never would have been useful to us - continue; - } - if visited.contains(edge.version) { - // We've been to this node already - continue; - } - // Deltas should only apply if dependencies satisfy dep_criteria - let mut deps_satisfied = true; - for &dependency in &package.normal_and_build_deps { - let dep_package = &dep_graph.nodes[dependency]; - let dep_vet_result = &mut results[dependency]; - - // If no custom criteria is specified, then require our dependency to match - // the same criteria that we're trying to validate. This makes audits effectively - // break down their criteria into individually verifiable components instead of - // purely "all or nothing". - // - // e.g. a safe-to-deploy audit with some deps that are only safe-to-run - // still audits for safe-to-run, but not safe-to-deploy. Similarly so for - // `[safe-to-run, some-other-criteria]` validating each criteria individually. - let dep_req = edge - .dependency_criteria - .get(dep_package.name) - .unwrap_or(cur_criteria); - - if !dep_vet_result.contains(dep_req) { - failed_deps - .entry(dependency) - .or_insert_with(|| criteria_mapper.no_criteria()) - .unioned_with(dep_req); - deps_satisfied = false; - } - } + let mut edge_caveats = caveats.clone(); + let mut edge_failed_deps = failed_deps.clone(); - if deps_satisfied { - // Ok yep, this edge is usable! But defer it if it's an exemption. - if edge.is_exemption { - deferred_exemptions_entries.push(edge.version); - } else { - search_stack.push(edge.version); - } - } else { - // Remember this edge failed, if we can't find any path we'll speculatively - // re-enable it. - deferred_failed_edges.push(edge.version); - } + match edge.origin { + DeltaEdgeOrigin::Exemption => { + edge_caveats.needed_exemptions.insert(pkgidx); } - } - } + DeltaEdgeOrigin::FreshImportedAudit => { + edge_caveats.needed_fresh_imports.insert(pkgidx); + } + DeltaEdgeOrigin::StoredAudit => {} + } + + get_dependency_criteria_caveats( + dep_graph, + criteria_mapper, + results, + dependencies, + cur_criteria, + &edge.dependency_criteria, + &mut edge_caveats, + &mut edge_failed_deps, + ); - // Exit conditions - if found_path - || (deferred_exemptions_entries.is_empty() && deferred_failed_edges.is_empty()) - { - break; + queue.push(Node { + version: edge.version, + caveats: edge_caveats, + failed_deps: edge_failed_deps, + }); } } - // It's only a success if we found a path and used no 'failed' edges. - if found_path && !needed_failed_edges { - // Complete success! - SearchResult::Connected { - fully_audited: !needed_exemption, - } - } else if found_path { - // Failure, but it's clearly the fault of our deps. - SearchResult::PossiblyConnected { failed_deps } - } else { - // Complete failure, we need more audits of ourself, - // so all that matters is what nodes were reachable. - SearchResult::Disconnected { - reachable_from_root: visited, - // This will get filled in by the second pass - reachable_from_target: Default::default(), - } + // Complete failure, we need more audits for this package, so all that + // matters is what nodes were reachable. + SearchResult::Disconnected { + reachable_from_root: visited, + // This will get filled in by a second pass. + reachable_from_target: Default::default(), } } @@ -2028,28 +2103,26 @@ fn resolve_first_party<'a>( // Compute whether we have each criteria based on our dependencies let mut validated_criteria = criteria_mapper.no_criteria(); - let mut search_results = vec![]; + let mut search_results = Vec::with_capacity(criteria_mapper.len()); for criteria in criteria_mapper.all_criteria_iter() { // Find any build/normal dependencies that don't satisfy this criteria + let mut caveats = Caveats::default(); let mut failed_deps = SortedMap::new(); - for &depidx in &package.normal_and_build_deps { - // If we have an explicit policy for dependency, that's all that matters. - // Otherwise just use the current criteria to "inherit" the results of our deps. - let dep_name = graph.nodes[depidx].name; - let required_criteria = dep_criteria.get(dep_name).unwrap_or(criteria); - if !results[depidx].contains(required_criteria) { - failed_deps - .entry(depidx) - .or_insert_with(|| criteria_mapper.no_criteria()) - .unioned_with(required_criteria); - } - } + + get_dependency_criteria_caveats( + graph, + criteria_mapper, + results, + &package.normal_and_build_deps, + criteria, + &dep_criteria, + &mut caveats, + &mut failed_deps, + ); if failed_deps.is_empty() { // All our deps passed the test, so we have this criteria - search_results.push(SearchResult::Connected { - fully_audited: true, - }); + search_results.push(SearchResult::Connected { caveats }); validated_criteria.unioned_with(criteria); } else { // Some of our deps failed to satisfy this criteria, record this @@ -2064,10 +2137,48 @@ fn resolve_first_party<'a>( ); // Save the results - results[pkgidx].search_results = search_results; - results[pkgidx].validated_criteria = validated_criteria; + results[pkgidx] = ResolveResult { + validated_criteria, + search_results, + directly_exempted: false, + }; +} + +fn get_policy_caveats<'a>( + criteria_mapper: &CriteriaMapper, + search_results: &[SearchResult<'a>], + own_policy: &CriteriaSet, + pkgidx: PackageIdx, + root_caveats: &mut Caveats, + policy_failures: &mut PolicyFailures, +) { + for criteria_idx in own_policy.indices() { + match &search_results[criteria_idx] { + SearchResult::PossiblyConnected { failed_deps } => { + // Our children failed us + for (&dep, failed_criteria) in failed_deps { + policy_failures + .entry(dep) + .or_insert_with(|| criteria_mapper.no_criteria()) + .unioned_with(failed_criteria); + } + } + SearchResult::Disconnected { .. } => { + // We failed ourselves + policy_failures + .entry(pkgidx) + .or_insert_with(|| criteria_mapper.no_criteria()) + .set_criteria(criteria_idx) + } + SearchResult::Connected { caveats } => { + // A-OK + root_caveats.add(caveats); + } + } + } } +#[allow(clippy::too_many_arguments)] fn resolve_self_policy<'a>( store: &'a Store, graph: &DepGraph<'a>, @@ -2075,6 +2186,7 @@ fn resolve_self_policy<'a>( results: &mut [ResolveResult<'a>], _violations: &mut SortedMap>, root_failures: &mut RootFailures, + root_caveats: &mut Caveats, pkgidx: PackageIdx, ) { let package = &graph.nodes[pkgidx]; @@ -2094,30 +2206,19 @@ fn resolve_self_policy<'a>( }; let mut policy_failures = PolicyFailures::new(); - for criteria_idx in own_policy.indices() { - match &results[pkgidx].search_results[criteria_idx] { - SearchResult::PossiblyConnected { failed_deps } => { - // Our children failed us - for (&dep, failed_criteria) in failed_deps { - policy_failures - .entry(dep) - .or_insert_with(|| criteria_mapper.no_criteria()) - .unioned_with(failed_criteria); - } - } - SearchResult::Disconnected { .. } => { - // We failed ourselves - policy_failures - .entry(pkgidx) - .or_insert_with(|| criteria_mapper.no_criteria()) - .set_criteria(criteria_idx) - } - SearchResult::Connected { .. } => { - // A-OK - } - } - } + get_policy_caveats( + criteria_mapper, + &results[pkgidx].search_results, + &own_policy, + pkgidx, + root_caveats, + &mut policy_failures, + ); + + // NOTE: We don't update search results here, just `validated_criteria`. + // This is the only way that these two should get out of sync, but we want + // to keep around the old search results as it will be useful for blame. if policy_failures.is_empty() { // We had a policy and it passed, so now we're validated for all criteria // because our parents can never require anything else of us. No need @@ -2132,6 +2233,7 @@ fn resolve_self_policy<'a>( } } +#[allow(clippy::too_many_arguments)] fn resolve_dev<'a>( store: &'a Store, graph: &DepGraph<'a>, @@ -2139,6 +2241,7 @@ fn resolve_dev<'a>( results: &mut [ResolveResult<'a>], _violations: &mut SortedMap>, root_failures: &mut RootFailures, + root_caveats: &mut Caveats, pkgidx: PackageIdx, ) { // Check the dev-deps of this package. It is assumed to be a root in this context, @@ -2166,25 +2269,23 @@ fn resolve_dev<'a>( let mut search_results = vec![]; for criteria in criteria_mapper.all_criteria_iter() { // Find any build/normal dependencies that don't satisfy this criteria + let mut caveats = Caveats::default(); let mut failed_deps = SortedMap::new(); - for &depidx in &package.dev_deps { - // If we have an explicit policy for dependency, that's all that matters. - // Otherwise just use the current criteria to "inherit" the results of our deps. - let dep_name = graph.nodes[depidx].name; - let required_criteria = dep_criteria.get(dep_name).unwrap_or(criteria); - if !results[depidx].contains(required_criteria) { - failed_deps - .entry(depidx) - .or_insert_with(|| criteria_mapper.no_criteria()) - .unioned_with(required_criteria); - } - } + + get_dependency_criteria_caveats( + graph, + criteria_mapper, + results, + &package.dev_deps, + criteria, + &dep_criteria, + &mut caveats, + &mut failed_deps, + ); if failed_deps.is_empty() { // All our deps passed the test, so we have this criteria - search_results.push(SearchResult::Connected { - fully_audited: true, - }); + search_results.push(SearchResult::Connected { caveats }); validated_criteria.unioned_with(criteria); } else { // Some of our deps failed to satisfy this criteria, record this @@ -2197,10 +2298,8 @@ fn resolve_dev<'a>( .criteria_names(&validated_criteria) .collect::>() ); - // DON'T save the results, because we're analyzing this as a dev-node and anything that - // depends on us only cares about the "normal" results. - // results[pkgidx].search_results = search_results; - // results[pkgidx].validated_criteria = validated_criteria; + // NOTE: DON'T save the results, because we're analyzing this as a dev-node + // and anything that depends on us only cares about the "normal" results. // Now check that we pass our own policy let entry = store.config.policy.get(package.name); @@ -2213,29 +2312,15 @@ fn resolve_dev<'a>( }; let mut policy_failures = PolicyFailures::new(); - for criteria_idx in own_policy.indices() { - match &search_results[criteria_idx] { - SearchResult::PossiblyConnected { failed_deps } => { - // Our children failed us - for (&dep, failed_criteria) in failed_deps { - policy_failures - .entry(dep) - .or_insert_with(|| criteria_mapper.no_criteria()) - .unioned_with(failed_criteria); - } - } - SearchResult::Disconnected { .. } => { - // We failed ourselves - policy_failures - .entry(pkgidx) - .or_insert_with(|| criteria_mapper.no_criteria()) - .set_criteria(criteria_idx) - } - SearchResult::Connected { .. } => { - // A-OK - } - } - } + + get_policy_caveats( + criteria_mapper, + &search_results, + &own_policy, + pkgidx, + root_caveats, + &mut policy_failures, + ); if policy_failures.is_empty() { trace!(" passed dev policy"); diff --git a/src/serialization.rs b/src/serialization.rs index 64c874ec..d4c1ae7c 100644 --- a/src/serialization.rs +++ b/src/serialization.rs @@ -234,6 +234,9 @@ pub mod audit { notes: val.notes, criteria: val.criteria, kind: kind?, + // By default, always read entries as non-fresh. The import code + // will set this flag to true for imported entries. + is_fresh_import: false, }) } } @@ -605,6 +608,7 @@ mod test { dependency_criteria: dc_long, }, notes: Some("notes go here!".to_owned()), + is_fresh_import: false, // ignored }, AuditEntry { who: None, @@ -614,6 +618,7 @@ mod test { dependency_criteria: dc_short, }, notes: Some("notes go here!".to_owned()), + is_fresh_import: true, // ignored }, ], ); diff --git a/src/storage.rs b/src/storage.rs index f2e98d26..cd843e7b 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -11,7 +11,7 @@ use std::{ use cargo_metadata::Version; use crates_index::Index; use flate2::read::GzDecoder; -use futures_util::future::{join_all, try_join_all}; +use futures_util::future::try_join_all; use miette::{NamedSource, SourceOffset}; use reqwest::Url; use serde::{Deserialize, Serialize}; @@ -21,18 +21,19 @@ use tracing::{error, info, log::warn, trace}; use crate::{ errors::{ CacheAcquireError, CacheCommitError, CommandError, CriteriaChangeError, - CriteriaChangeErrors, DiffError, FetchAndDiffError, FetchAuditError, FetchError, - FlockError, InvalidCriteriaError, JsonParseError, LoadJsonError, LoadTomlError, SourceFile, - StoreAcquireError, StoreCommitError, StoreCreateError, StoreJsonError, StoreTomlError, - StoreValidateError, StoreValidateErrors, TomlParseError, UnpackError, + CriteriaChangeErrors, DiffError, DownloadError, FetchAndDiffError, FetchAuditError, + FetchError, FlockError, InvalidCriteriaError, JsonParseError, LoadJsonError, LoadTomlError, + SourceFile, StoreAcquireError, StoreCommitError, StoreCreateError, StoreJsonError, + StoreTomlError, StoreValidateError, StoreValidateErrors, TomlParseError, UnpackError, }, flock::{FileLock, Filesystem}, format::{ - AuditsFile, CommandHistory, ConfigFile, CriteriaName, Delta, DiffCache, DiffStat, FastMap, - FetchCommand, ImportsFile, MetaConfig, PackageStr, SortedMap, SAFE_TO_DEPLOY, SAFE_TO_RUN, + AuditKind, AuditsFile, CommandHistory, ConfigFile, CriteriaName, Delta, DiffCache, + DiffStat, FastMap, FetchCommand, ImportName, ImportsFile, MetaConfig, PackageStr, + SortedMap, SAFE_TO_DEPLOY, SAFE_TO_RUN, }, network::Network, - resolver, + resolver::{self, Conclusion}, serialization::{spanned::Spanned, to_formatted_toml}, Config, PartialConfig, }; @@ -123,6 +124,10 @@ pub struct Store { pub imports: ImportsFile, pub audits: AuditsFile, + // The complete live set of imports fetched from the network. Will be + // initialized to `None` if `--locked` was passed. + pub live_imports: Option, + pub config_src: SourceFile, pub imports_src: SourceFile, pub audits_src: SourceFile, @@ -151,6 +156,7 @@ impl Store { criteria: SortedMap::new(), audits: SortedMap::new(), }, + live_imports: None, config_src: Arc::new(NamedSource::new(CONFIG_TOML, "")), audits_src: Arc::new(NamedSource::new(AUDITS_TOML, "")), imports_src: Arc::new(NamedSource::new(IMPORTS_LOCK, "")), @@ -162,8 +168,19 @@ impl Store { metacfg.store_path().as_path_unlocked().exists() } + pub fn acquire_offline(cfg: &Config) -> Result { + Self::acquire(cfg, None, false) + } + /// Acquire an existing store - pub fn acquire(cfg: &Config) -> Result { + /// + /// If `network` is passed and `!cfg.cli.locked`, this will fetch remote + /// imports to use for comparison purposes. + pub fn acquire( + cfg: &Config, + network: Option<&Network>, + allow_criteria_changes: bool, + ) -> Result { let root = cfg.metacfg.store_path(); // Before we do anything else, acquire an exclusive lock on the @@ -177,11 +194,24 @@ impl Store { let (imports_src, imports): (_, ImportsFile) = load_toml(IMPORTS_LOCK, lock.read_imports()?)?; + // If this command isn't locked, and the network is available, fetch the + // live state of imported audits. + let live_imports = if let (false, Some(network)) = (cfg.cli.locked, network) { + let imported_audits = tokio::runtime::Handle::current() + .block_on(fetch_imported_audits(network, &config))?; + let live_imports = + process_imported_audits(imported_audits, &imports, allow_criteria_changes)?; + Some(live_imports) + } else { + None + }; + let store = Self { lock: Some(lock), config, audits, imports, + live_imports, config_src, audits_src, imports_src, @@ -201,12 +231,37 @@ impl Store { config, imports, audits, + live_imports: None, config_src: Arc::new(NamedSource::new(CONFIG_TOML, "")), audits_src: Arc::new(NamedSource::new(AUDITS_TOML, "")), imports_src: Arc::new(NamedSource::new(IMPORTS_LOCK, "")), } } + /// Create a mock store, also mocking out the unlocked import fetching + /// process by providing the live values of imported AuditsFiles. + #[cfg(test)] + pub fn mock_online( + config: ConfigFile, + audits: AuditsFile, + imports: ImportsFile, + fetched_audits: Vec<(ImportName, AuditsFile)>, + allow_criteria_changes: bool, + ) -> Result { + let live_imports = + process_imported_audits(fetched_audits, &imports, allow_criteria_changes)?; + Ok(Self { + lock: None, + config, + imports, + audits, + live_imports: Some(live_imports), + config_src: Arc::new(NamedSource::new(CONFIG_TOML, "")), + audits_src: Arc::new(NamedSource::new(AUDITS_TOML, "")), + imports_src: Arc::new(NamedSource::new(IMPORTS_LOCK, "")), + }) + } + #[cfg(test)] pub fn mock_acquire( config: &str, @@ -222,6 +277,7 @@ impl Store { config, imports, audits, + live_imports: None, config_src, audits_src, imports_src, @@ -247,6 +303,7 @@ impl Store { config: self.config.clone(), imports: self.imports.clone(), audits: self.audits.clone(), + live_imports: self.live_imports.clone(), config_src: self.config_src.clone(), audits_src: self.audits_src.clone(), imports_src: self.imports_src.clone(), @@ -258,6 +315,18 @@ impl Store { clone } + /// Returns the set of audits which should be operated upon. + /// + /// If the store was acquired unlocked, this will include audits which are + /// not stored in imports.lock, otherwise it will only contain imports + /// stored locally. + pub fn imported_audits(&self) -> &SortedMap { + match &self.live_imports { + Some(live_imports) => &live_imports.audits, + None => &self.imports.audits, + } + } + /// Commit the store's contents back to disk pub fn commit(self) -> Result<(), StoreCommitError> { // TODO: make this truly transactional? @@ -446,84 +515,203 @@ impl Store { Ok(()) } - /// Fetch foreign audits, only call this is we're not --locked - pub async fn fetch_foreign_audits( - &mut self, - network: &Network, - accept_changes: bool, - ) -> Result<(), FetchAuditError> { - let raw_new_imports = - try_join_all(self.config.imports.iter().map(|(name, import)| async { - let audit_file = fetch_foreign_audit(network, name, &import.url).await?; - // Fetch the descriptions to cache them and check that they haven't changed - // FIXME: this should probably treat failing to fetch as an error but eula_for_criteria - // hides errors... should we have two versions? Or make it the caller's problem? - let new_descs = join_all(audit_file.criteria.iter().map(|(criteria, _)| async { - ( - criteria.clone(), - crate::eula_for_criteria(Some(network), &audit_file.criteria, criteria) - .await, - ) - })) - .await; - Ok::<_, FetchAuditError>((name.clone(), audit_file, new_descs)) - })) - .await?; + /// Return an updated version of the `imports.lock` file taking into account + /// the result of running the resolver, to pick which audits need to be + /// vendored. + /// + /// If `force_update` is specified, audits for all crates will be updated, + /// even if the currently vendored audits would be sufficient. + #[must_use] + pub fn get_updated_imports_file( + &self, + report: &resolver::ResolveReport<'_>, + force_update: bool, + ) -> ImportsFile { + if self.live_imports.is_none() { + // We're locked, so can't update anything. + return self.imports.clone(); + } + + // Determine which packages were used. + let mut used_packages = SortedMap::new(); + for package in report.graph.nodes.iter() { + used_packages.insert(package.name, false); + } + + // If we succeeded, also record which packages need fresh imports. + if let Conclusion::Success(success) = &report.conclusion { + for &node_idx in &success.needed_fresh_imports { + let package = &report.graph.nodes[node_idx]; + used_packages.insert(package.name, true); + } + } let mut new_imports = ImportsFile { audits: SortedMap::new(), }; - let mut criteria_changes = vec![]; - for (import_name, mut audits_file, new_descs) in raw_new_imports { - for (criteria_name, new_desc) in new_descs { - if !accept_changes { - // Check that the new description doesn't modify an existing old one - if let Some(old_entry) = self - .imports - .audits - .get(&import_name) - .and_then(|file| file.criteria.get(&criteria_name)) - { + for import_name in self.config.imports.keys() { + let live_audit_file = self + .imported_audits() + .get(import_name) + .expect("Live audits missing for import?"); + + // Is this the first time we're doing this import? If it is we'll + // want to effectively force-update this peer. + let first_import = !self.imports.audits.contains_key(import_name); + + // Create the new audits file, and copy criteria into it on a + // per-package basis, only including used packages. + // We always use the live version of criteria in the new file. + let mut new_audits_file = AuditsFile { + criteria: live_audit_file.criteria.clone(), + audits: SortedMap::new(), + }; + for (&package, &need_fresh_imports) in &used_packages { + // Always pull audit entries from the live version to pull in + // revocations, violations, updated notes, etc. + // Filter out entries which are fresh unless we're updating + // entries for this package or it's a violation. + let update_package = first_import || need_fresh_imports || force_update; + let audits = live_audit_file + .audits + .get(package) + .map(|v| &v[..]) + .unwrap_or(&[]) + .iter() + .filter(|audit| { + update_package + || !audit.is_fresh_import + || matches!(audit.kind, AuditKind::Violation { .. }) + }) + .cloned() + .map(|mut audit| { + audit.is_fresh_import = false; + audit + }) + .collect::>(); + + // Record audits in the new audits file, if there are any. + if !audits.is_empty() { + new_audits_file.audits.insert(package.to_owned(), audits); + } + } + new_imports + .audits + .insert(import_name.to_owned(), new_audits_file); + } + + new_imports + } + + pub fn maybe_update_imports_file(&mut self, cfg: &Config, force_update: bool) { + let report = resolver::resolve( + &cfg.metadata, + cfg.cli.filter_graph.as_ref(), + self, + // Resolve depth only impacts error results, so we can use Shallow + // to do less work. + resolver::ResolveDepth::Shallow, + ); + self.imports = self.get_updated_imports_file(&report, force_update); + } +} + +/// Process imported audits from the network, generating a `LiveImports` +/// description of the live state of imported audits. +fn process_imported_audits( + fetched_audits: Vec<(ImportName, AuditsFile)>, + imports_lock: &ImportsFile, + allow_criteria_changes: bool, +) -> Result { + let mut new_imports = ImportsFile { + audits: SortedMap::new(), + }; + let mut changed_criteria = Vec::new(); + for (import_name, mut audits_file) in fetched_audits { + // By default all audits read from the network are fresh. + for audit_entry in audits_file.audits.values_mut().flat_map(|v| v.iter_mut()) { + audit_entry.is_fresh_import = true; + } + + // If we have an existing audits file for these imports, compare against it. + if let Some(existing_audits_file) = imports_lock.audits.get(&import_name) { + if !allow_criteria_changes { + // Compare the new criteria descriptions with existing criteria + // descriptions. If the description already exists, record a + // CriteriaChangeError. + for (criteria_name, old_entry) in &existing_audits_file.criteria { + if let Some(new_entry) = audits_file.criteria.get(criteria_name) { let old_desc = old_entry.description.as_ref().unwrap(); - if old_desc != &new_desc { - criteria_changes.push(CriteriaChangeError { + let new_desc = new_entry.description.as_ref().unwrap(); + if old_desc != new_desc { + changed_criteria.push(CriteriaChangeError { import_name: import_name.clone(), criteria_name: criteria_name.to_owned(), old_desc: old_desc.clone(), - new_desc, + new_desc: new_desc.clone(), }); - continue; } } } - - // Store the new fetched description - audits_file - .criteria - .get_mut(&criteria_name) - .unwrap() - .description = Some(new_desc); } - // Now add the new import - new_imports.audits.insert(import_name, audits_file); - } - if !criteria_changes.is_empty() { - Err(CriteriaChangeErrors { - errors: criteria_changes, - })?; + // Compare the new audits with existing audits. If an audit already + // existed in the existing audits file, mark it as non-fresh. + for (package, existing_audits) in &existing_audits_file.audits { + let new_audits = audits_file + .audits + .get_mut(package) + .map(|v| &mut v[..]) + .unwrap_or(&mut []); + for existing_audit in existing_audits { + for new_audit in &mut new_audits[..] { + // Ignore `who` and `notes` for comparison, as they + // are not relevant semantically and might have been + // updated uneventfully. + if new_audit.is_fresh_import + && new_audit.kind == existing_audit.kind + && new_audit.criteria == existing_audit.criteria + { + new_audit.is_fresh_import = false; + break; + } + } + } + } } - // Accept the new imports. These will only be committed if the current command succeeds. - self.imports = new_imports; + // Now add the new import + new_imports.audits.insert(import_name, audits_file); + } - // Now do one last validation to catch corrupt imports - self.validate()?; - Ok(()) + if !changed_criteria.is_empty() { + return Err(CriteriaChangeErrors { + errors: changed_criteria, + }); } + + // FIXME: Consider doing some additional validation on these audits + // before returning? + + Ok(new_imports) +} + +/// Fetch all declared imports from the network, filling in any criteria +/// descriptions. +async fn fetch_imported_audits( + network: &Network, + config: &ConfigFile, +) -> Result, FetchAuditError> { + try_join_all(config.imports.iter().map(|(name, import)| async { + let audit_file = fetch_imported_audit(network, name, &import.url).await?; + Ok::<_, FetchAuditError>((name.clone(), audit_file)) + })) + .await } -async fn fetch_foreign_audit( +/// Fetch a single AuditsFile from the network, filling in any criteria +/// descriptions. +async fn fetch_imported_audit( network: &Network, name: &str, url: &str, @@ -536,7 +724,7 @@ async fn fetch_foreign_audit( let audit_bytes = network.download(parsed_url).await?; let audit_string = String::from_utf8(audit_bytes).map_err(LoadTomlError::from)?; let audit_source = Arc::new(NamedSource::new(name, audit_string.clone())); - let audit_file: AuditsFile = toml::de::from_str(&audit_string) + let mut audit_file: AuditsFile = toml::de::from_str(&audit_string) .map_err(|error| { let (line, col) = error.line_col().unwrap_or((0, 0)); TomlParseError { @@ -546,6 +734,46 @@ async fn fetch_foreign_audit( } }) .map_err(LoadTomlError::from)?; + + // Eagerly fetch all descriptions for criteria in the imported audits file, + // and store them inline. We'll error out if any of these descriptions are + // unavailable. + try_join_all( + audit_file + .criteria + .iter_mut() + .map(|(criteria_name, criteria_entry)| async { + if criteria_entry.description.is_some() { + return Ok(()); + } + + let url_string = criteria_entry.description_url.as_ref().ok_or_else(|| { + FetchAuditError::MissingCriteriaDescription { + import_name: name.to_owned(), + criteria_name: criteria_name.clone(), + } + })?; + let url = Url::parse(url_string).map_err(|error| { + FetchAuditError::InvalidCriteriaDescriptionUrl { + import_name: name.to_owned(), + criteria_name: criteria_name.clone(), + url: url_string.clone(), + error, + } + })?; + let bytes = network.download(url.clone()).await?; + let description = + String::from_utf8(bytes).map_err(|error| DownloadError::InvalidText { + url: url.clone(), + error, + })?; + + criteria_entry.description = Some(description); + Ok::<(), FetchAuditError>(()) + }), + ) + .await?; + Ok(audit_file) } diff --git a/src/tests/import.rs b/src/tests/import.rs new file mode 100644 index 00000000..d53402f2 --- /dev/null +++ b/src/tests/import.rs @@ -0,0 +1,738 @@ +use super::*; + +// Helper function for imports tests. Performs a vet and updates imports based +// on it, returning a diff of the two. +fn get_imports_file_changes(metadata: &Metadata, store: &Store, force_updates: bool) -> String { + // Run the resolver before calling `get_imports_file` to compute the new + // imports file. + let report = crate::resolver::resolve(metadata, None, store, ResolveDepth::Shallow); + let new_imports = store.get_updated_imports_file(&report, force_updates); + + // Format the old and new files as TOML, and write out a diff using `similar`. + let old_imports = crate::serialization::to_formatted_toml(&store.imports) + .unwrap() + .to_string(); + let new_imports = crate::serialization::to_formatted_toml(new_imports) + .unwrap() + .to_string(); + + generate_diff(&old_imports, &new_imports) +} + +// Test cases: + +#[test] +fn new_peer_import() { + // (Pass) We import all audits for our third-party packages from a brand-new + // peer even though we are already fully audited. This won't force an import + // from existing peers. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [ + ( + "third-party2".to_owned(), + vec![delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY)], + ), + ( + "unused-package".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + ), + ] + .into_iter() + .collect(), + }; + + let old_other_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + let new_other_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![delta_audit(ver(200), ver(300), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + imports + .audits + .insert(OTHER_FOREIGN.to_owned(), old_other_foreign_audits); + + config.imports.insert( + OTHER_FOREIGN.to_owned(), + crate::format::RemoteImport { + url: OTHER_FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![ + (FOREIGN.to_owned(), new_foreign_audits), + (OTHER_FOREIGN.to_owned(), new_other_foreign_audits), + ], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_skip_import() { + // (Pass) If we've previously imported from a peer, we don't import + // audits for a package unless it's useful. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [ + ( + "third-party2".to_owned(), + vec![delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY)], + ), + ( + "unused-package".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + ), + ] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_remove_unused() { + // (Pass) We'll remove unused audits when unlocked, even if our peer hasn't + // changed. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [ + ( + "third-party2".to_owned(), + vec![delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY)], + ), + ( + "unused-package".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + ), + ] + .into_iter() + .collect(), + }; + + let new_foreign_audits = old_foreign_audits.clone(); + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_import_delta_audit() { + // (Pass) If a new delta audit from a peer is useful, we'll import it and + // all other audits for that crate, including from other peers. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, mut audits, mut imports) = builtin_files_full_audited(&metadata); + + audits.audits.remove("third-party2"); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(9), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [ + // A new audit for third-party2 should fix our audit, so we should + // import all of these. + ( + "third-party2".to_owned(), + vec![ + full_audit(ver(9), SAFE_TO_DEPLOY), + delta_audit(ver(9), ver(DEFAULT_VER), SAFE_TO_DEPLOY), + delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY), + ], + ), + // This audit won't change things for us, so we won't import it to + // avoid churn. + ( + "third-party1".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + ), + ] + .into_iter() + .collect(), + }; + + let old_other_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + let new_other_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + // We'll also import unrelated audits from other sources. + audits: [( + "third-party2".to_owned(), + vec![delta_audit(ver(200), ver(300), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + imports + .audits + .insert(OTHER_FOREIGN.to_owned(), old_other_foreign_audits); + + config.imports.insert( + OTHER_FOREIGN.to_owned(), + crate::format::RemoteImport { + url: OTHER_FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![ + (FOREIGN.to_owned(), new_foreign_audits), + (OTHER_FOREIGN.to_owned(), new_other_foreign_audits), + ], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_import_custom_criteria() { + // (Pass) We'll immediately import criteria changes wen unlocked, even if + // our peer hasn't changed or we aren't mapping them locally. This doesn't + // force an import of other crates. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let new_foreign_audits = AuditsFile { + criteria: [( + "fuzzed".to_string(), + CriteriaEntry { + implies: vec![], + description: Some("fuzzed".to_string()), + description_url: None, + }, + )] + .into_iter() + .collect(), + audits: [( + "third-party2".to_owned(), + vec![ + full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY), + delta_audit(ver(DEFAULT_VER), ver(11), SAFE_TO_DEPLOY), + ], + )] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + + insta::assert_snapshot!(output); +} + +#[test] +fn new_audit_for_unused_criteria_basic() { + // (Pass) If a peer adds an audit for an unused criteria, we shouldn't + // vendor in the changes unnecessarily. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + let old_foreign_audits = AuditsFile { + criteria: [( + "fuzzed".to_string(), + CriteriaEntry { + implies: vec![], + description: Some("fuzzed".to_string()), + description_url: None, + }, + )] + .into_iter() + .collect(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let mut new_foreign_audits = old_foreign_audits.clone(); + new_foreign_audits + .audits + .get_mut("third-party2") + .unwrap() + .push(full_audit(ver(DEFAULT_VER), "fuzzed")); + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + + insta::assert_snapshot!(output); +} + +#[test] +fn new_audit_for_unused_criteria_transitive() { + // (Pass) If a peer adds an audit for an unused criteria of a transitive + // dependency, we shouldn't vendor in the changes unnecessarily. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + let old_foreign_audits = AuditsFile { + criteria: [( + "fuzzed".to_string(), + CriteriaEntry { + implies: vec![], + description: Some("fuzzed".to_string()), + description_url: None, + }, + )] + .into_iter() + .collect(), + audits: [( + "third-party1".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let mut new_foreign_audits = old_foreign_audits.clone(); + new_foreign_audits + .audits + .get_mut("third-party1") + .unwrap() + .push(full_audit(ver(DEFAULT_VER), "fuzzed")); + new_foreign_audits.audits.insert( + "transitive-third-party1".to_owned(), + vec![full_audit(ver(DEFAULT_VER), "fuzzed")], + ); + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_revoked_audit() { + // (Pass) If a previously-imported audit is removed, we should also remove + // it locally, even if we don't use it. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn existing_peer_add_violation() { + // (Pass) If a peer adds a violation for any version of a crate we use, we + // should immediately import it. We won't immediately import other audits + // added for that crate, however. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_full_audited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![ + delta_audit(ver(100), ver(200), SAFE_TO_DEPLOY), + delta_audit(ver(200), ver(300), SAFE_TO_DEPLOY), + violation(VersionReq::parse("99.*").unwrap(), SAFE_TO_DEPLOY), + ], + )] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn peer_audits_exemption_no_minimize() { + // (Pass) We don't import audits for a package which would replace an + // exemption unless we're regenerating exemptions. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_inited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + let output = get_imports_file_changes(&metadata, &store, false); + insta::assert_snapshot!(output); +} + +#[test] +fn peer_audits_exemption_minimize() { + // (Pass) We do import audits for a package which would replace an exemption + // when we're regenerating exemptions. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, audits, mut imports) = builtin_files_inited(&metadata); + + let old_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: SortedMap::new(), + }; + + let new_foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into_iter() + .collect(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), old_foreign_audits); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: FOREIGN_URL.to_owned(), + criteria_map: Vec::new(), + }, + ); + + let mut store = Store::mock_online( + config, + audits, + imports, + vec![(FOREIGN.to_owned(), new_foreign_audits)], + true, + ) + .unwrap(); + + // Capture the old imports before minimizing exemptions + let old = store.mock_commit(); + + crate::minimize_exemptions(&mock_cfg(&metadata), &mut store, None).unwrap(); + + // Capture after minimizing exemptions, and generate a diff. + let new = store.mock_commit(); + + let output = diff_store_commits(&old, &new); + insta::assert_snapshot!(output); +} + +// Other tests worth adding: +// +// - used edges with dependency-criteria should cause criteria to be imported diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 553909f3..9d7ac8ea 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -25,6 +25,7 @@ use crate::{ mod audit_as_crates_io; mod certify; +mod import; mod regenerate_unaudited; mod store_parsing; mod vet; @@ -299,6 +300,7 @@ fn delta_audit(from: Version, to: Version, criteria: CriteriaStr) -> AuditEntry delta, dependency_criteria: DependencyCriteria::default(), }, + is_fresh_import: false, } } @@ -331,6 +333,7 @@ fn delta_audit_dep( }) .collect(), }, + is_fresh_import: false, } } @@ -343,6 +346,7 @@ fn full_audit(version: Version, criteria: CriteriaStr) -> AuditEntry { version, dependency_criteria: DependencyCriteria::default(), }, + is_fresh_import: false, } } @@ -358,6 +362,7 @@ fn full_audit_m( version, dependency_criteria: DependencyCriteria::default(), }, + is_fresh_import: false, } } @@ -387,6 +392,7 @@ fn full_audit_dep( }) .collect(), }, + is_fresh_import: false, } } @@ -396,6 +402,7 @@ fn violation_hard(version: VersionReq) -> AuditEntry { notes: None, criteria: vec![SAFE_TO_RUN.to_string().into()], kind: AuditKind::Violation { violation: version }, + is_fresh_import: false, } } #[allow(dead_code)] @@ -405,6 +412,7 @@ fn violation(version: VersionReq, criteria: CriteriaStr) -> AuditEntry { notes: None, criteria: vec![criteria.to_string().into()], kind: AuditKind::Violation { violation: version }, + is_fresh_import: false, } } #[allow(dead_code)] @@ -417,6 +425,7 @@ fn violation_m( notes: None, criteria: criteria.into_iter().map(|s| s.into().into()).collect(), kind: AuditKind::Violation { violation: version }, + is_fresh_import: false, } } diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_add_violation.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_add_violation.snap new file mode 100644 index 00000000..873f9586 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_add_violation.snap @@ -0,0 +1,13 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + delta = "100.0.0 -> 200.0.0" ++ ++[[audits.peer-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++violation = "99.*" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_custom_criteria.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_custom_criteria.snap new file mode 100644 index 00000000..f67fa8de --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_custom_criteria.snap @@ -0,0 +1,12 @@ +--- +source: src/tests/import.rs +expression: output +--- + ++[audits.peer-company.criteria.fuzzed] ++description = "fuzzed" ++ + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + version = "10.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_delta_audit.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_delta_audit.snap new file mode 100644 index 00000000..4574e737 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_import_delta_audit.snap @@ -0,0 +1,22 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + version = "9.0.0" + +-[audits.rival-company.audits] ++[[audits.peer-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++delta = "9.0.0 -> 10.0.0" ++ ++[[audits.peer-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++delta = "100.0.0 -> 200.0.0" ++ ++[[audits.rival-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++delta = "200.0.0 -> 300.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_remove_unused.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_remove_unused.snap new file mode 100644 index 00000000..ab84c697 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_remove_unused.snap @@ -0,0 +1,13 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + delta = "100.0.0 -> 200.0.0" +- +-[[audits.peer-company.audits.unused-package]] +-criteria = "safe-to-deploy" +-version = "10.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_revoked_audit.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_revoked_audit.snap new file mode 100644 index 00000000..cc9e5db4 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_revoked_audit.snap @@ -0,0 +1,10 @@ +--- +source: src/tests/import.rs +expression: output +--- + +-[[audits.peer-company.audits.third-party2]] +-criteria = "safe-to-deploy" +-delta = "100.0.0 -> 200.0.0" ++[audits.peer-company.audits] + diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_peer_skip_import.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_skip_import.snap new file mode 100644 index 00000000..c24bb390 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_peer_skip_import.snap @@ -0,0 +1,7 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [audits.peer-company.audits] + diff --git a/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_basic.snap b/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_basic.snap new file mode 100644 index 00000000..b37182c8 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_basic.snap @@ -0,0 +1,12 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [audits.peer-company.criteria.fuzzed] + description = "fuzzed" + + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + version = "10.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_transitive.snap b/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_transitive.snap new file mode 100644 index 00000000..9627fddc --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__new_audit_for_unused_criteria_transitive.snap @@ -0,0 +1,12 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [audits.peer-company.criteria.fuzzed] + description = "fuzzed" + + [[audits.peer-company.audits.third-party1]] + criteria = "safe-to-deploy" + version = "10.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__new_peer_import.snap b/src/tests/snapshots/cargo_vet__tests__import__new_peer_import.snap new file mode 100644 index 00000000..43f221eb --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__new_peer_import.snap @@ -0,0 +1,11 @@ +--- +source: src/tests/import.rs +expression: output +--- + ++[[audits.peer-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++delta = "100.0.0 -> 200.0.0" ++ + [audits.rival-company.audits] + diff --git a/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_minimize.snap b/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_minimize.snap new file mode 100644 index 00000000..f54b1187 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_minimize.snap @@ -0,0 +1,37 @@ +--- +source: src/tests/import.rs +expression: output +--- +audits.toml: (unchanged) +config.toml: + + # cargo-vet config file + + [imports.peer-company] + url = "https://peercompany.co.uk" + criteria-map = [] + + [[exemptions.third-party1]] + version = "10.0.0" + criteria = "safe-to-deploy" + +-[[exemptions.third-party2]] +-version = "10.0.0" +-criteria = "safe-to-deploy" +- + [[exemptions.transitive-third-party1]] + version = "10.0.0" + criteria = "safe-to-deploy" + + +imports.lock: + + # cargo-vet imports lock + +-[audits.peer-company.audits] ++[[audits.peer-company.audits.third-party2]] ++criteria = "safe-to-deploy" ++version = "10.0.0" + + + diff --git a/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_no_minimize.snap b/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_no_minimize.snap new file mode 100644 index 00000000..c24bb390 --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__peer_audits_exemption_no_minimize.snap @@ -0,0 +1,7 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [audits.peer-company.audits] + diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-default-root.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-default-root.snap index 2ee5699c..5eb6a7e4 100644 --- a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-default-root.snap +++ b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-default-root.snap @@ -2,5 +2,5 @@ source: src/tests/vet.rs expression: output --- -Vetting Succeeded (1 fully audited, 3 exempted) +Vetting Succeeded (4 exempted) diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-weaker-root.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-weaker-root.snap index 2ee5699c..5eb6a7e4 100644 --- a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-weaker-root.snap +++ b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-audit-as-weaker-root.snap @@ -2,5 +2,5 @@ source: src/tests/vet.rs expression: output --- -Vetting Succeeded (1 fully audited, 3 exempted) +Vetting Succeeded (4 exempted) diff --git a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-deps-unaudited-adds-uneeded-criteria.snap b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-deps-unaudited-adds-uneeded-criteria.snap index 3c57c2e3..358cbdf2 100644 --- a/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-deps-unaudited-adds-uneeded-criteria.snap +++ b/src/tests/snapshots/cargo_vet__tests__vet__builtin-simple-deps-unaudited-adds-uneeded-criteria.snap @@ -2,5 +2,5 @@ source: src/tests/vet.rs expression: output --- -Vetting Succeeded (5 fully audited, 1 partially audited) +Vetting Succeeded (6 fully audited)