Repository: mozilla/cargo-vet (rust)

Issue:
Warn when policy.crate.audit-as-crates-io is set and the crate comes from crates.io
Currently, `cargo vet` is happy with audit-as-crates-io being set to either true or false, but I think both cases are weird when the crate does come crates.io, and `cargo vet` should warn about the situation.

Pull request:
Check for audit-as-crates-io on non-third-party crates
Fixes #366

Patch:
diff --git a/src/errors.rs b/src/errors.rs
index b802e62..e8ebb85 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -94,6 +94,9 @@ pub enum AuditAsError {
     #[error(transparent)]
     #[diagnostic(transparent)]
     ShouldntBeAuditAs(ShouldntBeAuditAsErrors),
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    UnusedAuditAs(UnusedAuditAsErrors),
 }
 
 #[derive(Debug, Error, Diagnostic)]
@@ -128,6 +131,22 @@ impl Display for ShouldntBeAuditAsErrors {
     }
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[diagnostic(help("Remove the audit-as-crates-io entries"))]
+pub struct UnusedAuditAsErrors {
+    pub errors: Vec<UnusedAuditAsError>,
+}
+
+impl Display for UnusedAuditAsErrors {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str("some audit-as-crates-io policies don't match first-party crates")?;
+        for e in &self.errors {
+            f.write_fmt(format_args!("\n  {}", e))?
+        }
+        Ok(())
+    }
+}
+
 #[derive(Debug, Error)]
 #[error("{package}:{version}")]
 pub struct NeedsAuditAsError {
@@ -142,6 +161,12 @@ pub struct ShouldntBeAuditAsError {
     pub version: Version,
 }
 
+#[derive(Debug, Error)]
+#[error("{package}")]
+pub struct UnusedAuditAsError {
+    pub package: PackageName,
+}
+
 ///////////////////////////////////////////////////////////
 // CertifyError
 ///////////////////////////////////////////////////////////
diff --git a/src/main.rs b/src/main.rs
index 22cec41..f5974d0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -14,9 +14,9 @@ use errors::{
     AggregateCriteriaImplies, AggregateError, AggregateErrors, AggregateImpliesMismatchError,
     AuditAsError, AuditAsErrors, CacheAcquireError, CertifyError, FetchAuditError, LoadTomlError,
     NeedsAuditAsError, NeedsAuditAsErrors, ShouldntBeAuditAsError, ShouldntBeAuditAsErrors,
-    TomlParseError, UserInfoError,
+    TomlParseError, UnusedAuditAsError, UnusedAuditAsErrors, UserInfoError,
 };
-use format::{CriteriaName, CriteriaStr, PackageName, PolicyEntry};
+use format::{CriteriaName, CriteriaStr, PackageName, PolicyEntry, SortedSet};
 use futures_util::future::{join_all, try_join_all};
 use indicatif::ProgressDrawTarget;
 use lazy_static::lazy_static;
@@ -1367,6 +1367,15 @@ fn fix_audit_as(cfg: &Config, store: &mut Store) -> Result<(), CacheAcquireError
                             .audit_as_crates_io = Some(false);
                     }
                 }
+                AuditAsError::UnusedAuditAs(unuseds) => {
+                    for err in unuseds.errors {
+                        // XXX: consider removing the policy completely if
+                        // there's nothing left in it anymore?
+                        if let Some(policy) = store.config.policy.get_mut(&err.package) {
+                            policy.audit_as_crates_io = None;
+                        }
+                    }
+                }
             }
         }
     }
@@ -2040,7 +2049,17 @@ fn check_audit_as_crates_io(
     let mut needs_audit_as_entry = vec![];
     let mut shouldnt_be_audit_as = vec![];
 
+    let mut unused_audit_as: SortedSet<PackageName> = store
+        .config
+        .policy
+        .iter()
+        .filter(|(_, policy)| policy.audit_as_crates_io.is_some())
+        .map(|(name, _)| name.clone())
+        .collect();
+
     for package in first_party_packages_strict(&cfg.metadata, &store.config) {
+        unused_audit_as.remove(&package.name);
+
         let audit_policy = store
             .config
             .policy
@@ -2084,7 +2103,10 @@ fn check_audit_as_crates_io(
         }
     }
 
-    if !needs_audit_as_entry.is_empty() || !shouldnt_be_audit_as.is_empty() {
+    if !needs_audit_as_entry.is_empty()
+        || !shouldnt_be_audit_as.is_empty()
+        || !unused_audit_as.is_empty()
+    {
         let mut errors = vec![];
         if !needs_audit_as_entry.is_empty() {
             errors.push(AuditAsError::NeedsAuditAs(NeedsAuditAsErrors {
@@ -2096,6 +2118,14 @@ fn check_audit_as_crates_io(
                 errors: shouldnt_be_audit_as,
             }));
         }
+        if !unused_audit_as.is_empty() {
+            errors.push(AuditAsError::UnusedAuditAs(UnusedAuditAsErrors {
+                errors: unused_audit_as
+                    .into_iter()
+                    .map(|package| UnusedAuditAsError { package })
+                    .collect(),
+            }))
+        }
         return Err(AuditAsErrors { errors });
     }
 


Test patch:
diff --git a/src/tests/audit_as_crates_io.rs b/src/tests/audit_as_crates_io.rs
index 16d2abb..3c54fa4 100644
--- a/src/tests/audit_as_crates_io.rs
+++ b/src/tests/audit_as_crates_io.rs
@@ -46,9 +46,11 @@ fn simple_audit_as_crates_io_all_true() {
     let (mut config, audits, imports) = builtin_files_full_audited(&metadata);
 
     for package in &mock.packages {
-        config
-            .policy
-            .insert(package.name.to_owned(), audit_as_policy(Some(true)));
+        if package.is_first_party {
+            config
+                .policy
+                .insert(package.name.to_owned(), audit_as_policy(Some(true)));
+        }
     }
 
     let store = Store::mock(config, audits, imports);
@@ -67,9 +69,11 @@ fn simple_audit_as_crates_io_all_false() {
     let (mut config, audits, imports) = builtin_files_full_audited(&metadata);
 
     for package in &mock.packages {
-        config
-            .policy
-            .insert(package.name.to_owned(), audit_as_policy(Some(false)));
+        if package.is_first_party {
+            config
+                .policy
+                .insert(package.name.to_owned(), audit_as_policy(Some(false)));
+        }
     }
 
     let store = Store::mock(config, audits, imports);
@@ -102,9 +106,11 @@ fn complex_audit_as_crates_io_all_true() {
     let (mut config, audits, imports) = builtin_files_full_audited(&metadata);
 
     for package in &mock.packages {
-        config
-            .policy
-            .insert(package.name.to_owned(), audit_as_policy(Some(true)));
+        if package.is_first_party {
+            config
+                .policy
+                .insert(package.name.to_owned(), audit_as_policy(Some(true)));
+        }
     }
 
     let store = Store::mock(config, audits, imports);
@@ -123,9 +129,11 @@ fn complex_audit_as_crates_io_all_false() {
     let (mut config, audits, imports) = builtin_files_full_audited(&metadata);
 
     for package in &mock.packages {
-        config
-            .policy
-            .insert(package.name.to_owned(), audit_as_policy(Some(false)));
+        if package.is_first_party {
+            config
+                .policy
+                .insert(package.name.to_owned(), audit_as_policy(Some(false)));
+        }
     }
 
     let store = Store::mock(config, audits, imports);
@@ -256,3 +264,24 @@ fn audit_as_crates_io_need_update() {
     let output = get_audit_as_crates_io(&cfg, &store);
     insta::assert_snapshot!("audit-as-crates-io-need-update", output);
 }
+
+#[test]
+fn audit_as_crates_io_non_first_party() {
+    let _enter = TEST_RUNTIME.enter();
+
+    let mock = MockMetadata::complex();
+    let metadata = mock.metadata();
+    let (mut config, audits, imports) = builtin_files_full_audited(&metadata);
+
+    for package in &mock.packages {
+        config
+            .policy
+            .insert(package.name.to_owned(), audit_as_policy(Some(false)));
+    }
+
+    let store = Store::mock(config, audits, imports);
+    let cfg = mock_cfg(&metadata);
+
+    let output = get_audit_as_crates_io(&cfg, &store);
+    insta::assert_snapshot!("audit-as-crates-io-non-first-party", output);
+}
diff --git a/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-non-first-party.snap b/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-non-first-party.snap
new file mode 100644
index 0000000..381efa2
--- /dev/null
+++ b/src/tests/snapshots/cargo_vet__tests__audit_as_crates_io__audit-as-crates-io-non-first-party.snap
@@ -0,0 +1,14 @@
+---
+source: src/tests/audit_as_crates_io.rs
+expression: output
+---
+
+  × There are some issues with your policy.audit-as-crates-io entries
+
+Error: 
+  × some audit-as-crates-io policies don't match first-party crates
+  │   third-core
+  │   thirdA
+  │   thirdAB
+  help: Remove the audit-as-crates-io entries
+