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.
This commit is contained in:
Charlie Marsh 2024-04-11 13:53:34 -04:00 committed by GitHub
parent 9d5467dc2f
commit 0d62e62fb7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 79 additions and 27 deletions

View file

@ -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,
}
}
}
}
}

View file

@ -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(),
),
}
}