From 0d62e62fb776c34a19422fd65b74606c3ba0bfd7 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Thu, 11 Apr 2024 13:53:34 -0400
Subject: [PATCH] Make hash-checking failure mode stricter and safer (#2997)

## Summary

If there are no hashes for a given package, we now return
`Validate(&[])` so that the policy is impossible to satisfy. Previously,
we returned `None`, which is always satisfied.

We don't really ever expect to hit this, because we detect this case in
the resolver and raise a different error. But if we have a bug
somewhere, it's better to fail with an error than silently let the
package through.
---
 crates/uv-distribution/src/error.rs | 76 +++++++++++++++++++++++------
 crates/uv-types/src/hash.rs         | 30 +++++++-----
 2 files changed, 79 insertions(+), 27 deletions(-)

diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs
index d5a77f81a..271ee66b0 100644
--- a/crates/uv-distribution/src/error.rs
+++ b/crates/uv-distribution/src/error.rs
@@ -88,12 +88,29 @@ pub enum Error {
     HashExhaustion(#[source] std::io::Error),
 
     #[error("Hash mismatch for {distribution}\n\nExpected:\n{expected}\n\nComputed:\n{actual}")]
-    HashMismatch {
+    MismatchedHashes {
         distribution: String,
         expected: String,
         actual: String,
     },
 
+    #[error(
+        "Hash-checking is enabled, but no hashes were provided or computed for: {distribution}"
+    )]
+    MissingHashes { distribution: String },
+
+    #[error("Hash-checking is enabled, but no hashes were computed for: {distribution}\n\nExpected:\n{expected}")]
+    MissingActualHashes {
+        distribution: String,
+        expected: String,
+    },
+
+    #[error("Hash-checking is enabled, but no hashes were provided for: {distribution}\n\nComputed:\n{actual}")]
+    MissingExpectedHashes {
+        distribution: String,
+        actual: String,
+    },
+
     #[error("Hash-checking is not supported for local directories: {0}")]
     HashesNotSupportedSourceTree(String),
 
@@ -125,22 +142,51 @@ impl Error {
         expected: &[HashDigest],
         actual: &[HashDigest],
     ) -> Error {
-        let expected = expected
-            .iter()
-            .map(|hash| format!("  {hash}"))
-            .collect::<Vec<_>>()
-            .join("\n");
+        match (expected.is_empty(), actual.is_empty()) {
+            (true, true) => Self::MissingHashes { distribution },
+            (true, false) => {
+                let actual = actual
+                    .iter()
+                    .map(|hash| format!("  {hash}"))
+                    .collect::<Vec<_>>()
+                    .join("\n");
 
-        let actual = actual
-            .iter()
-            .map(|hash| format!("  {hash}"))
-            .collect::<Vec<_>>()
-            .join("\n");
+                Self::MissingExpectedHashes {
+                    distribution,
+                    actual,
+                }
+            }
+            (false, true) => {
+                let expected = expected
+                    .iter()
+                    .map(|hash| format!("  {hash}"))
+                    .collect::<Vec<_>>()
+                    .join("\n");
 
-        Self::HashMismatch {
-            distribution,
-            expected,
-            actual,
+                Self::MissingActualHashes {
+                    distribution,
+                    expected,
+                }
+            }
+            (false, false) => {
+                let expected = expected
+                    .iter()
+                    .map(|hash| format!("  {hash}"))
+                    .collect::<Vec<_>>()
+                    .join("\n");
+
+                let actual = actual
+                    .iter()
+                    .map(|hash| format!("  {hash}"))
+                    .collect::<Vec<_>>()
+                    .join("\n");
+
+                Self::MismatchedHashes {
+                    distribution,
+                    expected,
+                    actual,
+                }
+            }
         }
     }
 }
diff --git a/crates/uv-types/src/hash.rs b/crates/uv-types/src/hash.rs
index 592682998..def34c0f2 100644
--- a/crates/uv-types/src/hash.rs
+++ b/crates/uv-types/src/hash.rs
@@ -25,10 +25,12 @@ impl HashStrategy {
         match self {
             Self::None => HashPolicy::None,
             Self::Generate => HashPolicy::Generate,
-            Self::Validate(hashes) => hashes
-                .get(&distribution.package_id())
-                .map(Vec::as_slice)
-                .map_or(HashPolicy::None, HashPolicy::Validate),
+            Self::Validate(hashes) => HashPolicy::Validate(
+                hashes
+                    .get(&distribution.package_id())
+                    .map(Vec::as_slice)
+                    .unwrap_or_default(),
+            ),
         }
     }
 
@@ -37,10 +39,12 @@ impl HashStrategy {
         match self {
             Self::None => HashPolicy::None,
             Self::Generate => HashPolicy::Generate,
-            Self::Validate(hashes) => hashes
-                .get(&PackageId::from_registry(name.clone()))
-                .map(Vec::as_slice)
-                .map_or(HashPolicy::None, HashPolicy::Validate),
+            Self::Validate(hashes) => HashPolicy::Validate(
+                hashes
+                    .get(&PackageId::from_registry(name.clone()))
+                    .map(Vec::as_slice)
+                    .unwrap_or_default(),
+            ),
         }
     }
 
@@ -49,10 +53,12 @@ impl HashStrategy {
         match self {
             Self::None => HashPolicy::None,
             Self::Generate => HashPolicy::Generate,
-            Self::Validate(hashes) => hashes
-                .get(&PackageId::from_url(url))
-                .map(Vec::as_slice)
-                .map_or(HashPolicy::None, HashPolicy::Validate),
+            Self::Validate(hashes) => HashPolicy::Validate(
+                hashes
+                    .get(&PackageId::from_url(url))
+                    .map(Vec::as_slice)
+                    .unwrap_or_default(),
+            ),
         }
     }