fix(bans): report checksums for denied build scripts

src/bans.rs

@@ -1214,18 +1214,21 @@ pub fn check_build(
         .find_map(|(i, ae)| crate::match_krate(krate, &ae.spec).then_some((i, ae)))
         .unzip();
 
+    let build_script_path = krate.targets.iter().find_map(|t| {
+        t.kind
+            .contains(&TargetKind::CustomBuild)
+            .then_some(&t.src_path)
+    });
+
     // If the build script hashes to the same value and required features are not actually
     // set on the crate, we can skip it
     if let Some(kc) = krate_config
         && let Some(bsc) = &kc.build_script
-        && let Some(path) = krate
-            .targets
-            .iter()
-            .find_map(|t| (t.name == "build-script-build").then_some(&t.src_path))
+        && let Some(path) = build_script_path
     {
         let root = &krate.manifest_path.parent().unwrap();
-        match validate_file_checksum(path, &bsc.value) {
-            Ok(_) => {
+        match bsc.value.validate_checksum(path) {
+            Ok((true, _)) => {
                 pack.push(diags::ChecksumMatch {
                     path: diags::HomePath { path, root, home },
                     checksum: bsc,
@@ -1263,20 +1266,41 @@ pub fn check_build(
                     file_id,
                 });
             }
+            Ok((false, calculated)) => {
+                pack.push(diags::ChecksumMismatch {
+                    path: diags::HomePath { path, root, home },
+                    checksum: bsc,
+                    severity: Some(Severity::Warning),
+                    error: None,
+                    calculated: Some(calculated),
+                    file_id,
+                });
+            }
             Err(err) => {
                 pack.push(diags::ChecksumMismatch {
                     path: diags::HomePath { path, root, home },
                     checksum: bsc,
                     severity: Some(Severity::Warning),
-                    error: format!("build script failed checksum: {err:#}"),
+                    error: Some(format!("build script failed checksum: {err:#}")),
+                    calculated: None,
                     file_id,
                 });
             }
         }
     }
 
     if !build_script_allowed {
-        pack.push(diags::BuildScriptNotAllowed { krate });
+        let build_script = build_script_path.and_then(|path| {
+            let root = krate.manifest_path.parent().unwrap();
+            cfg::Checksum::checksum_file(path)
+                .ok()
+                .map(|checksum| (diags::HomePath { path, root, home }, checksum))
+        });
+
+        pack.push(diags::BuildScriptNotAllowed {
+            krate,
+            build_script,
+        });
         return kc_index;
     }
 
@@ -1447,7 +1471,7 @@ pub fn check_build(
             drop(tx);
         },
         || {
-            // Note that since we ship off the checksum validation to a threads the order is
+            // Note that since we ship off the checksum validation to threads the order is
             // not guaranteed, so we just put them in a btreemap so they are consistently
             // ordered and don't trigger test errors or cause confusing output for users
             let checksum_diags = parking_lot::Mutex::new(std::collections::BTreeMap::new());
@@ -1456,28 +1480,36 @@ pub fn check_build(
                     s.spawn(|_s| {
                         let absolute_path = path;
                         let path = &absolute_path;
-                        if let Err(err) = validate_file_checksum(&absolute_path, &checksum.value) {
-                            let diag: Diag = diags::ChecksumMismatch {
+
+                        let diag: Diag = match checksum.value.validate_checksum(&absolute_path) {
+                            Ok((true, _)) => diags::ChecksumMatch {
                                 path: diags::HomePath { path, root, home },
                                 checksum,
                                 severity: None,
-                                error: format!("{err:#}"),
                                 file_id,
                             }
-                            .into();
-
-                            checksum_diags.lock().insert(absolute_path, diag);
-                        } else {
-                            let diag: Diag = diags::ChecksumMatch {
+                            .into(),
+                            Ok((false, calculated)) => diags::ChecksumMismatch {
+                                path: diags::HomePath { path, root, home },
+                                checksum,
+                                severity: None,
+                                error: None,
+                                calculated: Some(calculated),
+                                file_id,
+                            }
+                            .into(),
+                            Err(error) => diags::ChecksumMismatch {
                                 path: diags::HomePath { path, root, home },
                                 checksum,
                                 severity: None,
+                                error: Some(format!("{error:#}")),
+                                calculated: None,
                                 file_id,
                             }
-                            .into();
+                            .into(),
+                        };
 
-                            checksum_diags.lock().insert(absolute_path, diag);
-                        }
+                        checksum_diags.lock().insert(absolute_path, diag);
                     });
                 }
             });
@@ -1574,46 +1606,36 @@ fn check_is_executable(
     }
 }
 
-/// Validates the buffer matches the expected SHA-256 checksum
-fn validate_checksum(
-    mut stream: impl std::io::Read,
-    expected: &cfg::Checksum,
-) -> anyhow::Result<()> {
-    let digest = {
-        let mut dc = ring::digest::Context::new(&ring::digest::SHA256);
-        let mut chunk = [0; 8 * 1024];
-        loop {
-            let read = stream.read(&mut chunk)?;
-            if read == 0 {
-                break;
-            }
-            dc.update(&chunk[..read]);
-        }
-        dc.finish()
-    };
+impl cfg::Checksum {
+    fn checksum_file(path: &crate::Path) -> anyhow::Result<Self> {
+        use std::io::Read;
 
-    let digest = digest.as_ref();
-    if digest != expected.0 {
-        let mut hs = [0u8; 64];
-        const CHARS: &[u8] = b"0123456789abcdef";
-        for (i, &byte) in digest.iter().enumerate() {
-            let i = i * 2;
-            hs[i] = CHARS[(byte >> 4) as usize];
-            hs[i + 1] = CHARS[(byte & 0xf) as usize];
-        }
+        let mut file = std::fs::File::open(path)?;
 
-        let digest = std::str::from_utf8(&hs).unwrap();
-        anyhow::bail!("checksum mismatch, calculated {digest}");
-    }
+        let digest = {
+            let mut dc = ring::digest::Context::new(&ring::digest::SHA256);
+            let mut chunk = [0; 8 * 1024];
+            loop {
+                let read = file.read(&mut chunk)?;
+                if read == 0 {
+                    break;
+                }
+                dc.update(&chunk[..read]);
+            }
+            dc.finish()
+        };
 
-    Ok(())
-}
+        let mut bytes = [0; 32];
+        bytes.copy_from_slice(digest.as_ref());
+        Ok(Self(bytes))
+    }
 
-#[inline]
-fn validate_file_checksum(path: &crate::Path, expected: &cfg::Checksum) -> anyhow::Result<()> {
-    let file = std::fs::File::open(path)?;
-    validate_checksum(std::io::BufReader::new(file), expected)?;
-    Ok(())
+    /// Validates the files matches the expected SHA-256 checksum
+    #[inline]
+    fn validate_checksum(&self, path: &crate::Path) -> anyhow::Result<(bool, Self)> {
+        let actual = Self::checksum_file(path)?;
+        Ok((actual == *self, actual))
+    }
 }
 
 fn check_workspace_duplicates(

src/bans/cfg.rs

@@ -103,8 +103,8 @@ impl GraphHighlight {
     }
 }
 
-#[derive(Clone)]
-#[cfg_attr(test, derive(Debug, PartialEq, Eq))]
+#[derive(Clone, PartialEq, Eq)]
+#[cfg_attr(test, derive(Debug))]
 pub struct Checksum(pub [u8; 32]);
 
 #[cfg_attr(test, derive(Debug, PartialEq, Eq))]
@@ -149,6 +149,22 @@ impl std::str::FromStr for Checksum {
     }
 }
 
+impl std::fmt::Display for Checksum {
+    #[allow(unsafe_code)]
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let mut hs = [0u8; 64];
+        const CHARS: &[u8] = b"0123456789abcdef";
+        for (i, &byte) in self.0.iter().enumerate() {
+            let i = i * 2;
+            hs[i] = CHARS[(byte >> 4) as usize];
+            hs[i + 1] = CHARS[(byte & 0xf) as usize];
+        }
+
+        // SAFETY: we only insert hex ascii characters
+        f.write_str(unsafe { std::str::from_utf8_unchecked(&hs) })
+    }
+}
+
 impl<'de> Deserialize<'de> for Checksum {
     fn deserialize(value: &mut Value<'de>) -> Result<Self, DeserError> {
         let val = value.take_string(Some("a sha-256 hex encoded string"))?;

src/bans/diags.rs

@@ -451,17 +451,24 @@ impl From<UnmatchedSkipRoot> for Diag {
 
 pub(crate) struct BuildScriptNotAllowed<'a> {
     pub(crate) krate: &'a Krate,
+    pub(crate) build_script: Option<(HomePath<'a>, super::cfg::Checksum)>,
 }
 
 impl<'a> From<BuildScriptNotAllowed<'a>> for Diag {
     fn from(bs: BuildScriptNotAllowed<'a>) -> Self {
-        diag(
-            Diagnostic::new(Severity::Error).with_message(format_args!(
-                "crate '{}' has a build script but is not allowed to have one",
-                bs.krate
-            )),
-            Code::BuildScriptNotAllowed,
-        )
+        let mut diagnostic = Diagnostic::new(Severity::Error).with_message(format_args!(
+            "crate '{}' has a build script but is not allowed to have one",
+            bs.krate
+        ));
+
+        if let Some((path, checksum)) = bs.build_script {
+            diagnostic = diagnostic.with_notes(vec![
+                format!("path = '{path}'"),
+                format!("checksum = '{checksum}'"),
+            ]);
+        }
+
+        diag(diagnostic, Code::BuildScriptNotAllowed)
     }
 }
 
@@ -775,18 +782,21 @@ pub(crate) struct ChecksumMismatch<'a> {
     pub(crate) path: HomePath<'a>,
     pub(crate) checksum: &'a Spanned<super::cfg::Checksum>,
     pub(crate) severity: Option<Severity>,
-    pub(crate) error: String,
+    pub(crate) error: Option<String>,
+    pub(crate) calculated: Option<super::cfg::Checksum>,
     pub(crate) file_id: FileId,
 }
 
 impl From<ChecksumMismatch<'_>> for Diag {
     fn from(cm: ChecksumMismatch<'_>) -> Diag {
         let mut notes = vec![format!("path = '{}'", cm.path)];
-        notes.extend(
-            format!("error = {:#}", cm.error)
-                .lines()
-                .map(|l| l.to_owned()),
-        );
+        if let Some(error) = cm.error {
+            notes.extend(format!("error = '{error:#}'").lines().map(|l| l.to_owned()));
+        } else if let Some(calculated) = cm.calculated {
+            notes.push(format!(
+                "error = 'calculated different checksum {calculated}'"
+            ));
+        }
 
         let diag = Diagnostic::new(cm.severity.unwrap_or(Severity::Error))
             .with_message("file did not match the expected checksum")

tests/bans_build.rs

@@ -1,4 +1,4 @@
-use cargo_deny::{field_eq, func_name, test_utils::*};
+use cargo_deny::{assert_field_eq, field_eq, func_name, test_utils::*};
 
 macro_rules! ci_ignore {
     () => {
@@ -147,6 +147,46 @@ build-script = "00abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef00
     insta::assert_json_snapshot!(diags);
 }
 
+/// Verifies build script diagnostics include enough information to write a bypass.
+#[test]
+fn shows_non_default_build_script_checksum() {
+    ci_ignore!();
+
+    let mut diags = gather_bans(
+        func_name!(),
+        KrateGather {
+            name: "non-default-build-script",
+            targets: &["x86_64-unknown-linux-gnu"],
+            ..Default::default()
+        },
+        Config::new(
+            r#"
+[build]
+include-workspace = true
+allow-build-scripts = []
+executables = "allow"
+"#,
+        ),
+    );
+
+    diags.retain(|d| {
+        field_eq!(d, "/fields/graphs/0/Krate/name", "non-default-build-script")
+            && field_eq!(d, "/fields/code", "build-script-not-allowed")
+    });
+
+    assert_eq!(diags.len(), 1);
+    assert_field_eq!(
+        diags[0],
+        "/fields/notes/0",
+        "path = '$crate/builder/main.rs'"
+    );
+    assert_field_eq!(
+        diags[0],
+        "/fields/notes/1",
+        "checksum = '536e506bb90914c243a12b397b9a998f85ae2cbd9ba02dfd03a9e155ca5ca0f4'"
+    );
+}
+
 /// Verifies that matching build scripts cause the rest of the build check to be
 /// skipped
 #[test]

tests/snapshots/bans_build__allows_build_scripts_or_bypass.snap

@@ -63,6 +63,10 @@ expression: diags
         }
       ],
       "message": "crate 'libc = 0.2.147' has a build script but is not allowed to have one",
+      "notes": [
+        "path = '$CARGO_HOME/registry/src/index.crates.io-1949cf8c6b5b557f/libc-0.2.147/build.rs'",
+        "checksum = '5bd78d7e4e79b183fb1dab92cd640a611330131d54c479c69adbe87cbdc95ae3'"
+      ],
       "severity": "error"
     },
     "type": "diagnostic"

tests/snapshots/bans_build__detects_build_script_mismatch.snap

@@ -33,7 +33,7 @@ expression: diags
       "message": "file did not match the expected checksum",
       "notes": [
         "path = '$CARGO_HOME/registry/src/index.crates.io-1949cf8c6b5b557f/ittapi-sys-0.3.3/build.rs'",
-        "error = build script failed checksum: checksum mismatch, calculated 474a3eb189a698475d8a6f4b358eb0790db6379aea8b8a85ac925102784cd520"
+        "error = 'calculated different checksum 474a3eb189a698475d8a6f4b358eb0790db6379aea8b8a85ac925102784cd520'"
       ],
       "severity": "warning"
     },

tests/test_data/non-default-build-script/Cargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "non-default-build-script"
+version = "0.1.0"
+edition = "2021"
+build = "builder/main.rs"
+
+[workspace]

tests/test_data/non-default-build-script/builder/main.rs

@@ -0,0 +1 @@
+fn main() {}

tests/test_data/non-default-build-script/src/lib.rs

@@ -0,0 +1,3 @@
+pub fn answer() -> u8 {
+    42
+}