From 6fa8204efeb2ebf246caba436f9a240fb8d61493 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 17 Dec 2025 11:01:59 -0500 Subject: [PATCH] Avoid enforcing incorrect hash in mixed-hash settings (#17157) ## Summary Right now, when we return a `Dist` from a lockfile, we concatenate all hashes for all distributions for a given package. In the case of https://github.com/astral-sh/uv/issues/17143, I think that means we'll return the SHA256 from the sdist, plus the SHA512 from the wheel. If the wheel was previously installed (i.e., it's in the cache), and we computed the SHA256 at that point in time, then `Hashed::has_digests` would return `true` because we have _at least_ one SHA256. We now limit the hashes to the distribution that we expect to install. Closes https://github.com/astral-sh/uv/issues/17143. --- crates/uv-resolver/src/lock/installable.rs | 14 +- crates/uv-resolver/src/lock/mod.rs | 84 +++++--- crates/uv/tests/it/lock.rs | 220 ++++++++++++++++++++- 3 files changed, 283 insertions(+), 35 deletions(-) diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index b6d93904f..9c112a38b 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -16,7 +16,7 @@ use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_platform_tags::Tags; use uv_pypi_types::ResolverMarkerEnvironment; -use crate::lock::{LockErrorKind, Package, TagPolicy}; +use crate::lock::{HashedDist, LockErrorKind, Package, TagPolicy}; use crate::{Lock, LockError}; pub trait Installable<'lock> { @@ -527,18 +527,14 @@ pub trait Installable<'lock> { marker_env: &ResolverMarkerEnvironment, build_options: &BuildOptions, ) -> Result { - let dist = package.to_dist( - self.install_path(), - TagPolicy::Required(tags), - build_options, - marker_env, - )?; + let tag_policy = TagPolicy::Required(tags); + let HashedDist { dist, hashes } = + package.to_dist(self.install_path(), tag_policy, build_options, marker_env)?; let version = package.version().cloned(); let dist = ResolvedDist::Installable { dist: Arc::new(dist), version, }; - let hashes = package.hashes(); Ok(Node::Dist { dist, hashes, @@ -553,7 +549,7 @@ pub trait Installable<'lock> { tags: &Tags, marker_env: &ResolverMarkerEnvironment, ) -> Result { - let dist = package.to_dist( + let HashedDist { dist, .. } = package.to_dist( self.install_path(), TagPolicy::Preferred(tags), &BuildOptions::default(), diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index 3ef583155..370d037f5 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -171,6 +171,15 @@ static ANDROID_X86_MARKERS: LazyLock = LazyLock::new(|| { marker }); +/// A distribution with its associated hash. +/// +/// This pairs a [`Dist`] with the [`HashDigests`] for the specific wheel or +/// sdist that would be installed. +pub(crate) struct HashedDist { + pub(crate) dist: Dist, + pub(crate) hashes: HashDigests, +} + #[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] #[serde(try_from = "LockWire")] pub struct Lock { @@ -1761,7 +1770,7 @@ impl Lock { if let Some(version) = package.id.version.as_ref() { // For a non-dynamic package, fetch the metadata from the distribution database. - let dist = package.to_dist( + let HashedDist { dist, .. } = package.to_dist( root, TagPolicy::Preferred(tags), &BuildOptions::default(), @@ -1912,7 +1921,7 @@ impl Lock { // exactly. For example, `hatchling` will flatten any recursive (or self-referential) // extras, while `setuptools` will not. if !satisfied { - let dist = package.to_dist( + let HashedDist { dist, .. } = package.to_dist( root, TagPolicy::Preferred(tags), &BuildOptions::default(), @@ -2579,20 +2588,26 @@ impl Package { Ok(()) } - /// Convert the [`Package`] to a [`Dist`] that can be used in installation. + /// Convert the [`Package`] to a [`Dist`] that can be used in installation, along with its hash. fn to_dist( &self, workspace_root: &Path, tag_policy: TagPolicy<'_>, build_options: &BuildOptions, markers: &MarkerEnvironment, - ) -> Result { + ) -> Result { let no_binary = build_options.no_binary_package(&self.id.name); let no_build = build_options.no_build_package(&self.id.name); if !no_binary { if let Some(best_wheel_index) = self.find_best_wheel(tag_policy) { - return match &self.id.source { + let hashes = self.wheels[best_wheel_index] + .hash + .as_ref() + .map(|hash| HashDigests::from(vec![hash.0.clone()])) + .unwrap_or_else(|| HashDigests::from(vec![])); + + let dist = match &self.id.source { Source::Registry(source) => { let wheels = self .wheels @@ -2604,7 +2619,7 @@ impl Package { best_wheel_index, sdist: None, }; - Ok(Dist::Built(BuiltDist::Registry(reg_built_dist))) + Dist::Built(BuiltDist::Registry(reg_built_dist)) } Source::Path(path) => { let filename: WheelFilename = @@ -2616,7 +2631,7 @@ impl Package { install_path: absolute_path(workspace_root, path)?.into_boxed_path(), }; let built_dist = BuiltDist::Path(path_dist); - Ok(Dist::Built(built_dist)) + Dist::Built(built_dist) } Source::Direct(url, direct) => { let filename: WheelFilename = @@ -2632,29 +2647,39 @@ impl Package { url: VerbatimUrl::from_url(url), }; let built_dist = BuiltDist::DirectUrl(direct_dist); - Ok(Dist::Built(built_dist)) + Dist::Built(built_dist) } - Source::Git(_, _) => Err(LockErrorKind::InvalidWheelSource { - id: self.id.clone(), - source_type: "Git", + Source::Git(_, _) => { + return Err(LockErrorKind::InvalidWheelSource { + id: self.id.clone(), + source_type: "Git", + } + .into()); } - .into()), - Source::Directory(_) => Err(LockErrorKind::InvalidWheelSource { - id: self.id.clone(), - source_type: "directory", + Source::Directory(_) => { + return Err(LockErrorKind::InvalidWheelSource { + id: self.id.clone(), + source_type: "directory", + } + .into()); } - .into()), - Source::Editable(_) => Err(LockErrorKind::InvalidWheelSource { - id: self.id.clone(), - source_type: "editable", + Source::Editable(_) => { + return Err(LockErrorKind::InvalidWheelSource { + id: self.id.clone(), + source_type: "editable", + } + .into()); } - .into()), - Source::Virtual(_) => Err(LockErrorKind::InvalidWheelSource { - id: self.id.clone(), - source_type: "virtual", + Source::Virtual(_) => { + return Err(LockErrorKind::InvalidWheelSource { + id: self.id.clone(), + source_type: "virtual", + } + .into()); } - .into()), }; + + return Ok(HashedDist { dist, hashes }); } } @@ -2663,7 +2688,16 @@ impl Package { // any local source tree, or at least editable source trees, which we allow in // `uv pip`.) if !no_build || sdist.is_virtual() { - return Ok(Dist::Source(sdist)); + let hashes = self + .sdist + .as_ref() + .and_then(|s| s.hash()) + .map(|hash| HashDigests::from(vec![hash.0.clone()])) + .unwrap_or_else(|| HashDigests::from(vec![])); + return Ok(HashedDist { + dist: Dist::Source(sdist), + hashes, + }); } } diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 6c9bd7858..b3e50b79e 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -8231,7 +8231,6 @@ fn lock_invalid_hash() -> Result<()> { ╰─▶ Hash mismatch for `idna==3.6` Expected: - sha256:aecdbbd083b06798ae1e86adcbfe8ab1479cf864e4ee30fe4e46a003d12491ca sha256:d05567e9c24a6b9faaa835c4821bad0590fbb9d5779e7caa6e1cc4978e7eb24f Computed: @@ -8242,6 +8241,225 @@ fn lock_invalid_hash() -> Result<()> { Ok(()) } +/// Ensure that we can install from a lockfile when the index switches hash algorithms. +/// First lock and sync with SHA256 hashes, then switch to SHA512 and lock/sync again +/// without clearing the cache. +#[test] +fn lock_mixed_hashes() -> Result<()> { + let context = TestContext::new("3.13"); + + let root = context.temp_dir.child("simple-html"); + fs_err::create_dir_all(&root)?; + + let basic_package = root.child("basic-package"); + fs_err::create_dir_all(&basic_package)?; + + // Copy the wheel and sdist from `test/links`. + let wheel = basic_package.child("basic_package-0.1.0-py3-none-any.whl"); + fs_err::copy( + context + .workspace_root + .join("test/links/basic_package-0.1.0-py3-none-any.whl"), + &wheel, + )?; + + let sdist = basic_package.child("basic_package-0.1.0.tar.gz"); + fs_err::copy( + context + .workspace_root + .join("test/links/basic_package-0.1.0.tar.gz"), + &sdist, + )?; + + // Phase 1: Create an `index.html` with SHA256 hashes for both wheel and sdist. + let index = basic_package.child("index.html"); + index.write_str(&formatdoc! {r#" + + + + + + +

Links for basic-package

+ + basic_package-0.1.0-py3-none-any.whl + + + basic_package-0.1.0.tar.gz + + + + "#, + Url::from_file_path(&wheel).unwrap().as_str(), + Url::from_file_path(&sdist).unwrap().as_str(), + })?; + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(&formatdoc! { r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.13" + dependencies = ["basic-package"] + + [tool.uv] + extra-index-url = ["{}"] + "#, + Url::from_file_path(&root).unwrap().as_str() + })?; + + let index_url = Url::from_file_path(&root).unwrap().to_string(); + let filters = [(index_url.as_str(), "file://[TMP]")] + .into_iter() + .chain(context.filters()) + .collect::>(); + + // Lock with SHA256 hashes. + uv_snapshot!(context.filters(), context.lock().env_remove(EnvVars::UV_EXCLUDE_NEWER), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + "); + + let lock = context.read("uv.lock"); + + insta::with_settings!({ + filters => filters.clone(), + }, { + assert_snapshot!( + lock, @r#" + version = 1 + revision = 3 + requires-python = ">=3.13" + + [[package]] + name = "basic-package" + version = "0.1.0" + source = { registry = "simple-html" } + sdist = { path = "basic-package/basic_package-0.1.0.tar.gz", hash = "sha256:af478ff91ec60856c99a540b8df13d756513bebb65bc301fb27e0d1f974532b4" } + wheels = [ + { path = "basic-package/basic_package-0.1.0-py3-none-any.whl", hash = "sha256:7b6229db79b5800e4e98a351b5628c1c8a944533a2d428aeeaa7275a30d4ea82" }, + ] + + [[package]] + name = "project" + version = "0.1.0" + source = { virtual = "." } + dependencies = [ + { name = "basic-package" }, + ] + + [package.metadata] + requires-dist = [{ name = "basic-package" }] + "# + ); + }); + + // Sync with SHA256 hashes to populate the cache. + uv_snapshot!(filters.clone(), context.sync().arg("--frozen"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + basic-package==0.1.0 + "); + + // Phase 2: Update the index to use a SHA512 hash for the wheel instead. + index.write_str(&formatdoc! {r#" + + + + + + +

Links for basic-package

+ + basic_package-0.1.0-py3-none-any.whl + + + basic_package-0.1.0.tar.gz + + + + "#, + Url::from_file_path(&wheel).unwrap().as_str(), + Url::from_file_path(&sdist).unwrap().as_str(), + })?; + + // Lock again with `--refresh` to pick up the SHA512 hash from the updated index. + uv_snapshot!(context.filters(), context.lock().arg("--refresh").env_remove(EnvVars::UV_EXCLUDE_NEWER), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + "); + + let lock = context.read("uv.lock"); + + insta::with_settings!({ + filters => filters.clone(), + }, { + assert_snapshot!( + lock, @r#" + version = 1 + revision = 3 + requires-python = ">=3.13" + + [[package]] + name = "basic-package" + version = "0.1.0" + source = { registry = "simple-html" } + sdist = { path = "basic-package/basic_package-0.1.0.tar.gz", hash = "sha256:af478ff91ec60856c99a540b8df13d756513bebb65bc301fb27e0d1f974532b4" } + wheels = [ + { path = "basic-package/basic_package-0.1.0-py3-none-any.whl", hash = "sha512:765bde25938af485e492e25ee0e8cde262462565122c1301213a69bf9ceb2008e3997b652a604092a238c4b1a6a334e697ff3cee3c22f9a617cb14f34e26ef17" }, + ] + + [[package]] + name = "project" + version = "0.1.0" + source = { virtual = "." } + dependencies = [ + { name = "basic-package" }, + ] + + [package.metadata] + requires-dist = [{ name = "basic-package" }] + "# + ); + }); + + // Reinstalling should re-compute the hash for the `basic-package` wheel to reflect SHA512. + uv_snapshot!(filters, context.sync().arg("--frozen").arg("--reinstall"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 1 package in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + ~ basic-package==0.1.0 + "); + + Ok(()) +} + /// Vary the `--resolution-mode`, and ensure that the lockfile is updated. #[test] fn lock_resolution_mode() -> Result<()> {