Repository: mozilla/cargo-vet
Pull request #406 (2023-02-15 02:08:19)

Relax crate policy checks to only require versions for all crates in the
graph for a particular package if `dependency-criteria` is used by any policy of that package.


Patch:
diff --git a/src/errors.rs b/src/errors.rs
index 45ea63b..d0c9078 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -193,7 +193,10 @@ pub enum CratePolicyError {
 }
 
 #[derive(Debug, Error, Diagnostic, PartialEq, Eq)]
-#[diagnostic(help("Add a `policy.\"<crate>:<version>\"` entry for them"))]
+#[diagnostic(help(
+    "Specifing `dependency-criteria` requires explicit policies for each version of \
+     a crate. Add a `policy.\"<crate>:<version>\"` entry for them."
+))]
 pub struct NeedsPolicyVersionErrors {
     pub errors: Vec<PackageError>,
 }
diff --git a/src/main.rs b/src/main.rs
index d96b42d..1a0bd1c 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2183,8 +2183,9 @@ fn check_audit_as_crates_io(
 /// Check crate policies for correctness.
 ///
 /// This verifies two rules:
-/// 1. Policies relating to third-party crates must have associated version(s). If a crate exists
-///    as a third-party dependency anywhere in the dependency graph, all versions must be specified.
+/// 1. Policies using `dependency-criteria` which relate to third-party crates must have associated
+///    version(s). If a crate has any `dependency-criteria` specified and exists as a third-party
+///    dependency anywhere in the dependency graph, all versions must be specified.
 /// 2. Any versioned policies must correspond to a crate in the graph.
 fn check_crate_policies(cfg: &Config, store: &Store) -> Result<(), CratePolicyErrors> {
     // All defined policy package names (to be removed).
@@ -2198,12 +2199,20 @@ fn check_crate_policies(cfg: &Config, store: &Store) -> Result<(), CratePolicyEr
         .filter_map(|(name, version, _)| version.map(|version| (name.clone(), version.clone())))
         .collect();
 
-    // A set of all third-party packages (for lookup of whether a crate has any third-party
+    // The set of all third-party packages (for lookup of whether a crate has any third-party
     // versions in use).
     let third_party_packages = foreign_packages_strict(&cfg.metadata, &store.config)
         .map(|p| &p.name)
         .collect::<SortedSet<_>>();
 
+    // The set of all packages which have a `dependency-criteria` specified in a policy.
+    let dependency_criteria_packages = store
+        .config
+        .policy
+        .iter()
+        .filter_map(|(name, _, entry)| (!entry.dependency_criteria.is_empty()).then_some(name))
+        .collect::<SortedSet<_>>();
+
     let mut needs_policy_version_errors = Vec::new();
 
     for package in &cfg.metadata.packages {
@@ -2211,12 +2220,11 @@ fn check_crate_policies(cfg: &Config, store: &Store) -> Result<(), CratePolicyEr
 
         let versioned_policy_exists =
             versioned_policy_crates.remove(&(package.name.clone(), package.vet_version()));
-        let crate_policy_exists = store.config.policy.package.contains_key(&package.name);
 
-        // If a crate has at least one third-party package and a crate policy exists, a versioned
-        // policy for all used versions must exist.
+        // If a crate has at least one third-party package and some crate policy specifies a
+        // `dependency-criteria`, a versioned policy for all used versions must exist.
         if third_party_packages.contains(&package.name)
-            && crate_policy_exists
+            && dependency_criteria_packages.contains(&package.name)
             && !versioned_policy_exists
         {
             needs_policy_version_errors.push(PackageError {


Test patch:
diff --git a/src/tests/crate_policies.rs b/src/tests/crate_policies.rs
index 659d660..7d4fe2b 100644
--- a/src/tests/crate_policies.rs
+++ b/src/tests/crate_policies.rs
@@ -1,9 +1,29 @@
+use crate::errors::CratePolicyErrors;
+
 use super::*;
 
 struct CratePolicyTest(pub MockMetadata);
 
 impl CratePolicyTest {
+    pub fn no_errors<F>(&self, alter_config: F)
+    where
+        F: FnOnce(&mut ConfigFile),
+    {
+        self.check_crate_policies(alter_config)
+            .expect("crate policy check should succeed");
+    }
+
     pub fn insta_crate_policy_errors<N: AsRef<str>, F>(&self, name: N, alter_config: F)
+    where
+        F: FnOnce(&mut ConfigFile),
+    {
+        let e = self
+            .check_crate_policies(alter_config)
+            .expect_err("crate policy check should have failed");
+        insta::assert_snapshot!(name.as_ref(), format!("{:?}", miette::Report::new(e)));
+    }
+
+    fn check_crate_policies<F>(&self, alter_config: F) -> Result<(), CratePolicyErrors>
     where
         F: FnOnce(&mut ConfigFile),
     {
@@ -13,14 +33,33 @@ impl CratePolicyTest {
         let store = Store::mock(config, audits, imports);
         let cfg = mock_cfg(&metadata);
 
-        let e = crate::check_crate_policies(&cfg, &store)
-            .expect_err("crate policy check should have failed");
-        insta::assert_snapshot!(name.as_ref(), format!("{:?}", miette::Report::new(e)));
+        crate::check_crate_policies(&cfg, &store)
+    }
+}
+
+/// Checks that if a third-party crate is present, and an unversioned policy is used _without_
+/// `dependency-criteria`, no error occurs.
+#[test]
+fn simple_crate_policies_third_party_crates_dont_need_versions() {
+    let _enter = TEST_RUNTIME.enter();
+
+    CratePolicyTest(MockMetadata::overlapping()).no_errors(|config| {
+        config.policy.insert(
+            "third-party".into(),
+            PackagePolicyEntry::Unversioned(Default::default()),
+        );
+    });
+}
+
+fn dep_criteria_policy_entry() -> PolicyEntry {
+    PolicyEntry {
+        dependency_criteria: [("foo".to_owned(), vec!["bar".to_owned().into()])].into(),
+        ..Default::default()
     }
 }
 
-/// Checks that if a third-party crate is present, and an unversioned policy is used, an error
-/// occurs indicating that versions need to be specified.
+/// Checks that if a third-party crate is present, and an unversioned policy is used with
+/// `dependency-criteria`, an error occurs indicating that versions need to be specified.
 #[test]
 fn simple_crate_policies_third_party_crates_imply_versions() {
     let _enter = TEST_RUNTIME.enter();
@@ -30,15 +69,15 @@ fn simple_crate_policies_third_party_crates_imply_versions() {
         |config| {
             config.policy.insert(
                 "third-party".into(),
-                PackagePolicyEntry::Unversioned(Default::default()),
+                PackagePolicyEntry::Unversioned(dep_criteria_policy_entry()),
             );
         },
     );
 }
 
-/// Checks that if a third-party crate is present and a versioned policy is used for one version,
-/// an error occurs indicating that versions are needed for other versions (regardless of whether
-/// they are considered first- or third-party).
+/// Checks that if a third-party crate is present and a versioned policy with `dependency-criteria`
+/// is used for one version, an error occurs indicating that versions are needed for other versions
+/// (regardless of whether they are considered first- or third-party).
 #[test]
 fn simple_crate_policies_third_party_crates_need_all_versions() {
     let _enter = TEST_RUNTIME.enter();
@@ -52,7 +91,7 @@ fn simple_crate_policies_third_party_crates_need_all_versions() {
                 config.policy.insert(
                     "third-party".into(),
                     PackagePolicyEntry::Versioned {
-                        version: [(ver(which), Default::default())].into(),
+                        version: [(ver(which), dep_criteria_policy_entry())].into(),
                     },
                 );
             },
diff --git a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_imply_versions.snap b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_imply_versions.snap
index 7c958e3..92478d1 100644
--- a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_imply_versions.snap
+++ b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_imply_versions.snap
@@ -9,5 +9,7 @@ Error:
   × some crates have policies that are missing an associated version
   │   third-party:1.0.0
   │   third-party:2.0.0
-  help: Add a `policy."<crate>:<version>"` entry for them
+  help: Specifing `dependency-criteria` requires explicit policies for each
+        version of a crate. Add a `policy."<crate>:<version>"` entry for
+        them.
 
diff --git a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_1.snap b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_1.snap
index 4daecc1..6380877 100644
--- a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_1.snap
+++ b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_1.snap
@@ -8,5 +8,7 @@ expression: "format!(\"{:?}\", miette :: Report :: new(e))"
 Error: 
   × some crates have policies that are missing an associated version
   │   third-party:2.0.0
-  help: Add a `policy."<crate>:<version>"` entry for them
+  help: Specifing `dependency-criteria` requires explicit policies for each
+        version of a crate. Add a `policy."<crate>:<version>"` entry for
+        them.
 
diff --git a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_2.snap b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_2.snap
index a69d532..4939dc2 100644
--- a/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_2.snap
+++ b/src/tests/snapshots/cargo_vet__tests__crate_policies__third_party_crates_need_all_versions_2.snap
@@ -8,5 +8,7 @@ expression: "format!(\"{:?}\", miette :: Report :: new(e))"
 Error: 
   × some crates have policies that are missing an associated version
   │   third-party:1.0.0
-  help: Add a `policy."<crate>:<version>"` entry for them
+  help: Specifing `dependency-criteria` requires explicit policies for each
+        version of a crate. Add a `policy."<crate>:<version>"` entry for
+        them.