Key hash policy on version, rather than package (#5169)

## Summary

First part of: https://github.com/astral-sh/uv/issues/5168.
This commit is contained in:
Charlie Marsh 2024-07-17 19:01:49 -04:00 committed by GitHub
parent 82d94838cb
commit 218ae2c13e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 97 additions and 62 deletions

View file

@ -114,7 +114,10 @@ impl<'a> RegistryWheelIndex<'a> {
CachedWheel::from_http_pointer(wheel_dir.join(file), cache)
{
// Enforce hash-checking based on the built distribution.
if wheel.satisfies(hasher.get_package(package)) {
if wheel.satisfies(
hasher
.get_package(&wheel.filename.name, &wheel.filename.version),
) {
Self::add_wheel(wheel, tags, &mut versions);
}
}
@ -130,7 +133,10 @@ impl<'a> RegistryWheelIndex<'a> {
CachedWheel::from_local_pointer(wheel_dir.join(file), cache)
{
// Enforce hash-checking based on the built distribution.
if wheel.satisfies(hasher.get_package(package)) {
if wheel.satisfies(
hasher
.get_package(&wheel.filename.name, &wheel.filename.version),
) {
Self::add_wheel(wheel, tags, &mut versions);
}
}
@ -174,10 +180,12 @@ impl<'a> RegistryWheelIndex<'a> {
};
if let Some(revision) = revision {
// Enforce hash-checking based on the source distribution.
if revision.satisfies(hasher.get_package(package)) {
for wheel_dir in symlinks(cache_shard.join(revision.id())) {
if let Some(wheel) = CachedWheel::from_built_source(wheel_dir) {
for wheel_dir in symlinks(cache_shard.join(revision.id())) {
if let Some(wheel) = CachedWheel::from_built_source(wheel_dir) {
// Enforce hash-checking based on the source distribution.
if revision.satisfies(
hasher.get_package(&wheel.filename.name, &wheel.filename.version),
) {
Self::add_wheel(wheel, tags, &mut versions);
}
}

View file

@ -124,7 +124,9 @@ impl FlatIndex {
}
// Check if hashes line up
let hash = if let HashPolicy::Validate(required) = hasher.get_package(&filename.name) {
let hash = if let HashPolicy::Validate(required) =
hasher.get_package(&filename.name, &filename.version)
{
if hashes.is_empty() {
HashComparison::Missing
} else if required.iter().any(|hash| hashes.contains(hash)) {
@ -163,7 +165,9 @@ impl FlatIndex {
};
// Check if hashes line up.
let hash = if let HashPolicy::Validate(required) = hasher.get_package(&filename.name) {
let hash = if let HashPolicy::Validate(required) =
hasher.get_package(&filename.name, &filename.version)
{
if hashes.is_empty() {
HashComparison::Missing
} else if required.iter().any(|hash| hashes.contains(hash)) {

View file

@ -722,11 +722,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
request_sink.blocking_send(Request::Dist(dist))?;
}
} else {
// Verify that the package is allowed under the hash-checking policy.
if !self.hasher.allows_package(name) {
return Err(ResolveError::UnhashedPackage(name.clone()));
}
// Emit a request to fetch the metadata for this package.
if self.index.packages().register(name.clone()) {
request_sink.blocking_send(Request::Package(name.clone()))?;
@ -1077,6 +1072,14 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// Emit a request to fetch the metadata for this version.
if matches!(&**package, PubGrubPackageInner::Package { .. }) {
if self.index.distributions().register(candidate.version_id()) {
// Verify that the package is allowed under the hash-checking policy.
if !self
.hasher
.allows_package(candidate.name(), candidate.version())
{
return Err(ResolveError::UnhashedPackage(candidate.name().clone()));
}
let request = Request::from(dist.for_resolution());
request_sink.blocking_send(request)?;
}
@ -1801,6 +1804,14 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// Emit a request to fetch the metadata for this version.
if self.index.distributions().register(candidate.version_id()) {
// Verify that the package is allowed under the hash-checking policy.
if !self
.hasher
.allows_package(candidate.name(), candidate.version())
{
return Err(ResolveError::UnhashedPackage(candidate.name().clone()));
}
let dist = dist.for_resolution().to_owned();
let response = match dist {

View file

@ -2,7 +2,6 @@ use std::collections::btree_map::{BTreeMap, Entry};
use std::sync::OnceLock;
use rkyv::{de::deserializers::SharedDeserializeMap, Deserialize};
use rustc_hash::FxHashSet;
use tracing::instrument;
use distribution_filename::{DistFilename, WheelFilename};
@ -94,11 +93,6 @@ impl VersionMap {
},
}
}
let allowed_yanks = allowed_yanks
.allowed_versions(package_name)
.cloned()
.unwrap_or_default();
let required_hashes = hasher.get_package(package_name).digests().to_vec();
Self {
inner: VersionMapInner::Lazy(VersionMapLazy {
map,
@ -107,10 +101,10 @@ impl VersionMap {
no_build: build_options.no_build_package(package_name),
index: index.clone(),
tags: tags.cloned(),
allowed_yanks: allowed_yanks.clone(),
hasher: hasher.clone(),
requires_python: requires_python.cloned(),
exclude_newer: exclude_newer.copied(),
allowed_yanks,
required_hashes,
}),
}
}
@ -288,9 +282,9 @@ struct VersionMapLazy {
/// Whether files newer than this timestamp should be excluded or not.
exclude_newer: Option<ExcludeNewer>,
/// Which yanked versions are allowed
allowed_yanks: FxHashSet<Version>,
allowed_yanks: AllowedYanks,
/// The hashes of allowed distributions.
required_hashes: Vec<HashDigest>,
hasher: HashStrategy,
/// The `requires-python` constraint for the resolution.
requires_python: Option<RequiresPython>,
}
@ -366,14 +360,14 @@ impl VersionMapLazy {
};
// Prioritize amongst all available files.
let version = filename.version().clone();
let yanked = file.yanked.clone();
let hashes = file.hashes.clone();
match filename {
DistFilename::WheelFilename(filename) => {
let compatibility = self.wheel_compatibility(
&filename,
&version,
&filename.name,
&filename.version,
&hashes,
yanked,
excluded,
@ -388,7 +382,8 @@ impl VersionMapLazy {
}
DistFilename::SourceDistFilename(filename) => {
let compatibility = self.source_dist_compatibility(
&version,
&filename.name,
&filename.version,
&hashes,
yanked,
excluded,
@ -416,6 +411,7 @@ impl VersionMapLazy {
fn source_dist_compatibility(
&self,
name: &PackageName,
version: &Version,
hashes: &[HashDigest],
yanked: Option<Yanked>,
@ -436,21 +432,20 @@ impl VersionMapLazy {
// Check if yanked
if let Some(yanked) = yanked {
if yanked.is_yanked() && !self.allowed_yanks.contains(version) {
if yanked.is_yanked() && !self.allowed_yanks.contains(name, version) {
return SourceDistCompatibility::Incompatible(IncompatibleSource::Yanked(yanked));
}
}
// Check if hashes line up. If hashes aren't required, they're considered matching.
let hash = if self.required_hashes.is_empty() {
let hash_policy = self.hasher.get_package(name, version);
let required_hashes = hash_policy.digests();
let hash = if required_hashes.is_empty() {
HashComparison::Matched
} else {
if hashes.is_empty() {
HashComparison::Missing
} else if hashes
.iter()
.any(|hash| self.required_hashes.contains(hash))
{
} else if hashes.iter().any(|hash| required_hashes.contains(hash)) {
HashComparison::Matched
} else {
HashComparison::Mismatched
@ -463,6 +458,7 @@ impl VersionMapLazy {
fn wheel_compatibility(
&self,
filename: &WheelFilename,
name: &PackageName,
version: &Version,
hashes: &[HashDigest],
yanked: Option<Yanked>,
@ -481,7 +477,7 @@ impl VersionMapLazy {
// Check if yanked
if let Some(yanked) = yanked {
if yanked.is_yanked() && !self.allowed_yanks.contains(version) {
if yanked.is_yanked() && !self.allowed_yanks.contains(name, version) {
return WheelCompatibility::Incompatible(IncompatibleWheel::Yanked(yanked));
}
}
@ -498,15 +494,14 @@ impl VersionMapLazy {
};
// Check if hashes line up. If hashes aren't required, they're considered matching.
let hash = if self.required_hashes.is_empty() {
let hash_policy = self.hasher.get_package(name, version);
let required_hashes = hash_policy.digests();
let hash = if required_hashes.is_empty() {
HashComparison::Matched
} else {
if hashes.is_empty() {
HashComparison::Missing
} else if hashes
.iter()
.any(|hash| self.required_hashes.contains(hash))
{
} else if hashes.iter().any(|hash| required_hashes.contains(hash)) {
HashComparison::Matched
} else {
HashComparison::Mismatched

View file

@ -1,5 +1,6 @@
use pypi_types::RequirementSource;
use rustc_hash::{FxHashMap, FxHashSet};
use std::sync::Arc;
use pep440_rs::Version;
use pep508_rs::MarkerEnvironment;
@ -10,7 +11,7 @@ use crate::{DependencyMode, Manifest};
/// A set of package versions that are permitted, even if they're marked as yanked by the
/// relevant index.
#[derive(Debug, Default, Clone)]
pub struct AllowedYanks(FxHashMap<PackageName, FxHashSet<Version>>);
pub struct AllowedYanks(Arc<FxHashMap<PackageName, FxHashSet<Version>>>);
impl AllowedYanks {
pub fn from_manifest(
@ -47,7 +48,14 @@ impl AllowedYanks {
.insert(version.clone());
}
Self(allowed_yanks)
Self(Arc::new(allowed_yanks))
}
/// Returns `true` if the package-version is allowed, even if it's marked as yanked.
pub fn contains(&self, package_name: &PackageName, version: &Version) -> bool {
self.0
.get(package_name)
.map_or(false, |versions| versions.contains(version))
}
/// Returns versions for the given package which are allowed even if marked as yanked by the

View file

@ -1,9 +1,10 @@
use std::str::FromStr;
use rustc_hash::FxHashMap;
use std::str::FromStr;
use std::sync::Arc;
use url::Url;
use distribution_types::{DistributionMetadata, HashPolicy, PackageId, UnresolvedRequirement};
use distribution_types::{DistributionMetadata, HashPolicy, UnresolvedRequirement, VersionId};
use pep440_rs::Version;
use pep508_rs::MarkerEnvironment;
use pypi_types::{HashDigest, HashError, Requirement, RequirementSource};
use uv_configuration::HashCheckingMode;
@ -19,11 +20,11 @@ pub enum HashStrategy {
/// Hashes should be validated, if present, but ignored if absent.
///
/// If necessary, hashes should be generated to ensure that the archive is valid.
Verify(FxHashMap<PackageId, Vec<HashDigest>>),
Verify(Arc<FxHashMap<VersionId, Vec<HashDigest>>>),
/// Hashes should be validated against a pre-defined list of hashes.
///
/// If necessary, hashes should be generated to ensure that the archive is valid.
Require(FxHashMap<PackageId, Vec<HashDigest>>),
Require(Arc<FxHashMap<VersionId, Vec<HashDigest>>>),
}
impl HashStrategy {
@ -33,7 +34,7 @@ impl HashStrategy {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&distribution.package_id()) {
if let Some(hashes) = hashes.get(&distribution.version_id()) {
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
@ -41,7 +42,7 @@ impl HashStrategy {
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&distribution.package_id())
.get(&distribution.version_id())
.map(Vec::as_slice)
.unwrap_or_default(),
),
@ -49,12 +50,14 @@ impl HashStrategy {
}
/// Return the [`HashPolicy`] for the given registry-based package.
pub fn get_package(&self, name: &PackageName) -> HashPolicy {
pub fn get_package(&self, name: &PackageName, version: &Version) -> HashPolicy {
match self {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&PackageId::from_registry(name.clone())) {
if let Some(hashes) =
hashes.get(&VersionId::from_registry(name.clone(), version.clone()))
{
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
@ -62,7 +65,7 @@ impl HashStrategy {
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&PackageId::from_registry(name.clone()))
.get(&VersionId::from_registry(name.clone(), version.clone()))
.map(Vec::as_slice)
.unwrap_or_default(),
),
@ -75,7 +78,7 @@ impl HashStrategy {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&PackageId::from_url(url)) {
if let Some(hashes) = hashes.get(&VersionId::from_url(url)) {
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
@ -83,7 +86,7 @@ impl HashStrategy {
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&PackageId::from_url(url))
.get(&VersionId::from_url(url))
.map(Vec::as_slice)
.unwrap_or_default(),
),
@ -91,12 +94,14 @@ impl HashStrategy {
}
/// Returns `true` if the given registry-based package is allowed.
pub fn allows_package(&self, name: &PackageName) -> bool {
pub fn allows_package(&self, name: &PackageName, version: &Version) -> bool {
match self {
Self::None => true,
Self::Generate => true,
Self::Verify(_) => true,
Self::Require(hashes) => hashes.contains_key(&PackageId::from_registry(name.clone())),
Self::Require(hashes) => {
hashes.contains_key(&VersionId::from_registry(name.clone(), version.clone()))
}
}
}
@ -106,7 +111,7 @@ impl HashStrategy {
Self::None => true,
Self::Generate => true,
Self::Verify(_) => true,
Self::Require(hashes) => hashes.contains_key(&PackageId::from_url(url)),
Self::Require(hashes) => hashes.contains_key(&VersionId::from_url(url)),
}
}
@ -121,7 +126,7 @@ impl HashStrategy {
markers: Option<&MarkerEnvironment>,
mode: HashCheckingMode,
) -> Result<Self, HashStrategyError> {
let mut hashes = FxHashMap::<PackageId, Vec<HashDigest>>::default();
let mut hashes = FxHashMap::<VersionId, Vec<HashDigest>>::default();
// For each requirement, map from name to allowed hashes. We use the last entry for each
// package.
@ -139,7 +144,7 @@ impl HashStrategy {
}
UnresolvedRequirement::Unnamed(requirement) => {
// Direct URLs are always allowed.
PackageId::from_url(&requirement.url.verbatim)
VersionId::from_url(&requirement.url.verbatim)
}
};
@ -163,6 +168,7 @@ impl HashStrategy {
hashes.insert(id, digests);
}
let hashes = Arc::new(hashes);
match mode {
HashCheckingMode::Verify => Ok(Self::Verify(hashes)),
HashCheckingMode::Require => Ok(Self::Require(hashes)),
@ -170,7 +176,7 @@ impl HashStrategy {
}
/// Pin a [`Requirement`] to a [`PackageId`], if possible.
fn pin(requirement: &Requirement) -> Option<PackageId> {
fn pin(requirement: &Requirement) -> Option<VersionId> {
match &requirement.source {
RequirementSource::Registry { specifier, .. } => {
// Must be a single specifier.
@ -183,12 +189,15 @@ impl HashStrategy {
return None;
}
Some(PackageId::from_registry(requirement.name.clone()))
Some(VersionId::from_registry(
requirement.name.clone(),
specifier.version().clone(),
))
}
RequirementSource::Url { url, .. }
| RequirementSource::Git { url, .. }
| RequirementSource::Path { url, .. }
| RequirementSource::Directory { url, .. } => Some(PackageId::from_url(url)),
| RequirementSource::Directory { url, .. } => Some(VersionId::from_url(url)),
}
}
}

View file

@ -5162,7 +5162,7 @@ fn require_hashes_missing_dependency() -> Result<()> {
// Write to a requirements file.
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str(
"anyio==4.0.0 --hash=sha256:cfdb2b588b9fc25ede96d8db56ed50848b0b649dca3dd1df0b11f683bb9e0b5f",
"werkzeug==3.0.0 --hash=sha256:cbb2600f7eabe51dbc0502f58be0b3e1b96b893b05695ea2b35b43d4de2d9962",
)?;
// Install without error when `--require-hashes` is omitted.
@ -5175,7 +5175,7 @@ fn require_hashes_missing_dependency() -> Result<()> {
----- stdout -----
----- stderr -----
error: In `--require-hashes` mode, all requirements must be pinned upfront with `==`, but found: `idna`
error: In `--require-hashes` mode, all requirements must be pinned upfront with `==`, but found: `markupsafe`
"###
);