diff --git a/src/resolver.rs b/src/resolver.rs index 46b5bee7..b9e55e34 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -341,6 +341,34 @@ pub enum RequiredEntry { }, } +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +enum DeltaEdgeFreshness { + // All information requried for this delta edge is already stored within the store's + // supply-chain. Using this edge will require no changes to imports.lock. + Stale, + // This edge originates from an already-imported or local wildcard audit, however will require + // importing fresh publisher information into imports.lock. + FreshPublisher, + // This edge is fully fresh, and will require adding the audit entry to imports.lock to use. + Fresh, +} + +impl DeltaEdgeFreshness { + fn new(is_fresh_audit: bool, is_fresh_publisher: bool) -> Self { + if is_fresh_audit { + DeltaEdgeFreshness::Fresh + } else if is_fresh_publisher { + DeltaEdgeFreshness::FreshPublisher + } else { + DeltaEdgeFreshness::Stale + } + } + + fn is_fresh(&self) -> bool { + self != &DeltaEdgeFreshness::Stale + } +} + /// 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. @@ -355,7 +383,7 @@ struct DeltaEdge<'a> { origin: DeltaEdgeOrigin, /// Whether or not the edge is a "fresh import", and should be /// de-prioritized to avoid unnecessary imports.lock updates. - is_fresh_import: bool, + freshness: DeltaEdgeFreshness, } #[derive(Debug, Copy, Clone, Eq, PartialEq)] @@ -1142,12 +1170,13 @@ impl<'a> AuditGraph<'a> { }; let criteria = criteria_mapper.criteria_from_list(&entry.criteria); + let freshness = DeltaEdgeFreshness::new(entry.is_fresh_import, false); forward_audits.entry(from_ver).or_default().push(DeltaEdge { version: Some(to_ver), criteria: criteria.clone(), origin: origin.clone(), - is_fresh_import: entry.is_fresh_import, + freshness, }); backward_audits .entry(Some(to_ver)) @@ -1156,7 +1185,7 @@ impl<'a> AuditGraph<'a> { version: from_ver, criteria, origin, - is_fresh_import: entry.is_fresh_import, + freshness, }); } @@ -1177,19 +1206,20 @@ impl<'a> AuditGraph<'a> { audit_index, publisher_index, }; - let is_fresh_import = entry.is_fresh_import || publisher.is_fresh_import; + let freshness = + DeltaEdgeFreshness::new(entry.is_fresh_import, publisher.is_fresh_import); forward_audits.entry(from_ver).or_default().push(DeltaEdge { version: to_ver, criteria: criteria.clone(), origin: origin.clone(), - is_fresh_import, + freshness, }); backward_audits.entry(to_ver).or_default().push(DeltaEdge { version: from_ver, criteria, origin, - is_fresh_import, + freshness, }); } } @@ -1203,18 +1233,23 @@ impl<'a> AuditGraph<'a> { let to_ver = Some(&publisher.version); let criteria = criteria_mapper.criteria_from_list(&entry.criteria); let origin = DeltaEdgeOrigin::Trusted { publisher_index }; + // While the import freshness is technically based on the publisher being a + // fresh import, we use this as the _audit_ freshness here because a trusted + // entry should be put at the same caveat level ordering as other audits (which + // is determined from freshness later on). + let freshness = DeltaEdgeFreshness::new(publisher.is_fresh_import, false); forward_audits.entry(from_ver).or_default().push(DeltaEdge { version: to_ver, criteria: criteria.clone(), origin: origin.clone(), - is_fresh_import: publisher.is_fresh_import, + freshness, }); backward_audits.entry(to_ver).or_default().push(DeltaEdge { version: from_ver, criteria, origin, - is_fresh_import: publisher.is_fresh_import, + freshness, }); } } @@ -1226,19 +1261,19 @@ impl<'a> AuditGraph<'a> { let to_ver = Some(&unpublished.version); let criteria = criteria_mapper.all_criteria(); let origin = DeltaEdgeOrigin::Unpublished { unpublished_index }; - let is_fresh_import = unpublished.is_fresh_import; + let freshness = DeltaEdgeFreshness::new(unpublished.is_fresh_import, false); forward_audits.entry(from_ver).or_default().push(DeltaEdge { version: to_ver, criteria: criteria.clone(), origin: origin.clone(), - is_fresh_import, + freshness, }); backward_audits.entry(to_ver).or_default().push(DeltaEdge { version: from_ver, criteria, origin, - is_fresh_import, + freshness, }); } @@ -1255,13 +1290,13 @@ impl<'a> AuditGraph<'a> { version: to_ver, criteria: criteria.clone(), origin: origin.clone(), - is_fresh_import: false, + freshness: DeltaEdgeFreshness::Stale, }); backward_audits.entry(to_ver).or_default().push(DeltaEdge { version: from_ver, criteria, origin, - is_fresh_import: false, + freshness: DeltaEdgeFreshness::Stale, }); } } @@ -1449,6 +1484,7 @@ fn search_for_path( NonImportableAudit, PreferredExemption, PreferredUnpublished, + FreshPublisher, FreshImport, Exemption, Unpublished, @@ -1579,16 +1615,19 @@ fn search_for_path( DeltaEdgeOrigin::Unpublished { .. } => match mode { // When preferring exemptions, prefer existing unpublished // entries to avoid imports.lock churn. - SearchMode::PreferExemptions if !edge.is_fresh_import => { + SearchMode::PreferExemptions if !edge.freshness.is_fresh() => { CaveatLevel::PreferredUnpublished } SearchMode::PreferExemptions => CaveatLevel::Unpublished, // Otherwise, prefer fresh to avoid outdated versions. - _ if edge.is_fresh_import => CaveatLevel::PreferredUnpublished, + _ if edge.freshness.is_fresh() => CaveatLevel::PreferredUnpublished, _ => CaveatLevel::Unpublished, }, - _ if edge.is_fresh_import => CaveatLevel::FreshImport, - _ => CaveatLevel::None, + _ => match edge.freshness { + DeltaEdgeFreshness::Stale => CaveatLevel::None, + DeltaEdgeFreshness::FreshPublisher => CaveatLevel::FreshPublisher, + DeltaEdgeFreshness::Fresh => CaveatLevel::FreshImport, + }, }; queue.push(Node { diff --git a/src/tests/import.rs b/src/tests/import.rs index 26381a42..bc1de9a0 100644 --- a/src/tests/import.rs +++ b/src/tests/import.rs @@ -1934,3 +1934,123 @@ fn import_criteria_map_aggregated_error() { insta::assert_snapshot!(output); } + +#[test] +fn existing_import_kept_despite_local_wildcard_audit() { + // (Pass) An existing imported audit is still kept if a local wildcard audit accounts for a + // crate. + + 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"); + + audits.wildcard_audits.insert( + "third-party2".to_owned(), + vec![wildcard_audit(1, SAFE_TO_DEPLOY)], + ); + + let foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into(), + wildcard_audits: SortedMap::new(), + trusted: SortedMap::new(), + }; + + imports + .audits + .insert(FOREIGN.to_owned(), foreign_audits.clone()); + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: vec![FOREIGN_URL.to_owned()], + ..Default::default() + }, + ); + + let cfg = mock_cfg(&metadata); + + let mut network = Network::new_mock(); + MockRegistryBuilder::new() + .user(1, "user1", "User One") + .package( + "third-party2", + &[reg_published_by(ver(DEFAULT_VER), Some(1), "2022-12-15")], + ) + .serve(&mut network); + network.mock_serve_toml(FOREIGN_URL, &foreign_audits); + + let store = Store::mock_online(&cfg, config, audits, imports, &network, true).unwrap(); + + let output = get_imports_file_changes_prune(&metadata, &store); + insta::assert_snapshot!(output); +} + +#[test] +fn local_wildcard_audit_preferred_to_fresh_import() { + // (Pass) If a local wildcard audit accounts for a crate, a freshly imported audit should not + // be preferred. + + let _enter = TEST_RUNTIME.enter(); + let mock = MockMetadata::simple(); + + let metadata = mock.metadata(); + let (mut config, mut audits, imports) = builtin_files_full_audited(&metadata); + + audits.audits.remove("third-party2"); + + audits.wildcard_audits.insert( + "third-party2".to_owned(), + vec![wildcard_audit(1, SAFE_TO_DEPLOY)], + ); + + let foreign_audits = AuditsFile { + criteria: SortedMap::new(), + audits: [( + "third-party2".to_owned(), + vec![full_audit(ver(DEFAULT_VER), SAFE_TO_DEPLOY)], + )] + .into(), + // This isn't necessary (and overlaps with the imported audit), but we have it to also show + // that local wildcard audits are preferred to remote ones. + wildcard_audits: [( + "third-party2".to_owned(), + vec![wildcard_audit(1, SAFE_TO_DEPLOY)], + )] + .into(), + trusted: SortedMap::new(), + }; + + config.imports.insert( + FOREIGN.to_owned(), + crate::format::RemoteImport { + url: vec![FOREIGN_URL.to_owned()], + ..Default::default() + }, + ); + + let cfg = mock_cfg(&metadata); + + let mut network = Network::new_mock(); + MockRegistryBuilder::new() + .user(1, "user1", "User One") + .package( + "third-party2", + &[reg_published_by(ver(DEFAULT_VER), Some(1), "2022-12-15")], + ) + .serve(&mut network); + network.mock_serve_toml(FOREIGN_URL, &foreign_audits); + + let store = Store::mock_online(&cfg, config, audits, imports, &network, true).unwrap(); + + let output = get_imports_file_changes_noprune(&metadata, &store); + insta::assert_snapshot!(output); +} diff --git a/src/tests/snapshots/cargo_vet__tests__import__existing_import_kept_despite_local_wildcard_audit.snap b/src/tests/snapshots/cargo_vet__tests__import__existing_import_kept_despite_local_wildcard_audit.snap new file mode 100644 index 00000000..ca417f8d --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__existing_import_kept_despite_local_wildcard_audit.snap @@ -0,0 +1,9 @@ +--- +source: src/tests/import.rs +expression: output +--- + + [[audits.peer-company.audits.third-party2]] + criteria = "safe-to-deploy" + version = "10.0.0" + diff --git a/src/tests/snapshots/cargo_vet__tests__import__local_wildcard_audit_preferred_to_fresh_import.snap b/src/tests/snapshots/cargo_vet__tests__import__local_wildcard_audit_preferred_to_fresh_import.snap new file mode 100644 index 00000000..f1e7c72a --- /dev/null +++ b/src/tests/snapshots/cargo_vet__tests__import__local_wildcard_audit_preferred_to_fresh_import.snap @@ -0,0 +1,14 @@ +--- +source: src/tests/import.rs +expression: output +--- ++ ++[[publisher.third-party2]] ++version = "10.0.0" ++when = "2022-12-15" ++user-id = 1 ++user-login = "user1" ++user-name = "User One" ++ ++[audits.peer-company.audits] +