diff --git a/Cargo.lock b/Cargo.lock index 6275afd7b..3fea157ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6422,6 +6422,7 @@ dependencies = [ "uv-fs", "uv-git", "uv-git-types", + "uv-install-wheel", "uv-metadata", "uv-normalize", "uv-pep440", diff --git a/crates/uv-build-backend/src/wheel.rs b/crates/uv-build-backend/src/wheel.rs index 2f0d12fe9..9fde50a5b 100644 --- a/crates/uv-build-backend/src/wheel.rs +++ b/crates/uv-build-backend/src/wheel.rs @@ -397,7 +397,7 @@ struct RecordEntry { /// The urlsafe-base64-nopad encoded SHA256 of the files. hash: String, /// The size of the file in bytes. - size: usize, + size: u64, } /// Read the input file and write it both to the hasher and the target file. @@ -410,7 +410,7 @@ fn write_hashed( writer: &mut dyn Write, ) -> Result { let mut hasher = Sha256::new(); - let mut size = 0; + let mut size: u64 = 0; // 8KB is the default defined in `std::sys_common::io`. let mut buffer = vec![0; 8 * 1024]; loop { @@ -425,7 +425,7 @@ fn write_hashed( } hasher.update(&buffer[..read]); writer.write_all(&buffer[..read])?; - size += read; + size += read as u64; } Ok(RecordEntry { path: path.to_string(), @@ -736,7 +736,7 @@ impl DirectoryWriter for ZipDirectoryWriter { self.record.push(RecordEntry { path: path.to_string(), hash, - size: bytes.len(), + size: bytes.len() as u64, }); Ok(()) @@ -816,7 +816,7 @@ impl DirectoryWriter for FilesystemWriter { self.record.push(RecordEntry { path: path.to_string(), hash, - size: bytes.len(), + size: bytes.len() as u64, }); Ok(fs_err::write(self.root.join(path), bytes)?) diff --git a/crates/uv-distribution-types/src/lib.rs b/crates/uv-distribution-types/src/lib.rs index b2ee398a6..f50cc5b3e 100644 --- a/crates/uv-distribution-types/src/lib.rs +++ b/crates/uv-distribution-types/src/lib.rs @@ -33,6 +33,7 @@ //! //! Since we read this information from [`direct_url.json`](https://packaging.python.org/en/latest/specifications/direct-url-data-structure/), it doesn't match the information [`Dist`] exactly. use std::borrow::Cow; +use std::fmt::Display; use std::path; use std::path::Path; use std::str::FromStr; @@ -204,6 +205,15 @@ pub enum DistRef<'a> { Source(&'a SourceDist), } +impl Display for DistRef<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Built(built_dist) => Display::fmt(&built_dist, f), + Self::Source(source_dist) => Display::fmt(&source_dist, f), + } + } +} + /// A wheel, with its three possible origins (index, url, path) #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum BuiltDist { diff --git a/crates/uv-distribution/Cargo.toml b/crates/uv-distribution/Cargo.toml index 2d55c42fb..ec2ff8d53 100644 --- a/crates/uv-distribution/Cargo.toml +++ b/crates/uv-distribution/Cargo.toml @@ -28,6 +28,7 @@ uv-flags = { workspace = true } uv-fs = { workspace = true, features = ["tokio"] } uv-git = { workspace = true } uv-git-types = { workspace = true } +uv-install-wheel = { workspace = true } uv-metadata = { workspace = true } uv-normalize = { workspace = true } uv-pep440 = { workspace = true } diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index 528a2cc78..cff84342f 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -6,7 +6,6 @@ use std::sync::Arc; use std::task::{Context, Poll}; use futures::{FutureExt, TryStreamExt}; -use tempfile::TempDir; use tokio::io::{AsyncRead, AsyncSeekExt, ReadBuf}; use tokio::sync::Semaphore; use tokio_util::compat::FuturesAsyncReadCompatExt; @@ -20,11 +19,12 @@ use uv_client::{ }; use uv_distribution_filename::{SourceDistExtension, WheelFilename}; use uv_distribution_types::{ - BuildInfo, BuildableSource, BuiltDist, Dist, File, HashPolicy, Hashed, IndexUrl, InstalledDist, - Name, SourceDist, ToUrlError, + BuildInfo, BuildableSource, BuiltDist, Dist, DistRef, File, HashPolicy, Hashed, IndexUrl, + InstalledDist, Name, SourceDist, ToUrlError, }; use uv_extract::hash::Hasher; use uv_fs::write_atomic; +use uv_install_wheel::validate_and_heal_record; use uv_platform_tags::Tags; use uv_pypi_types::{HashDigest, HashDigests, PyProjectToml}; use uv_redacted::DisplaySafeUrl; @@ -491,7 +491,11 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { // Otherwise, unzip the wheel. let id = self - .unzip_wheel(&built_wheel.path, &built_wheel.target) + .unzip_wheel( + &built_wheel.path, + &built_wheel.target, + DistRef::Source(dist), + ) .await?; Ok(LocalWheel { @@ -683,19 +687,19 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let temp_dir = tempfile::tempdir_in(self.build_context.cache().root()) .map_err(Error::CacheWrite)?; - match progress { + let files = match progress { Some((reporter, progress)) => { let mut reader = ProgressReader::new(&mut hasher, progress, &**reporter); match extension { WheelExtension::Whl => { uv_extract::stream::unzip(query_url, &mut reader, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut reader, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } } } @@ -703,21 +707,25 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { WheelExtension::Whl => { uv_extract::stream::unzip(query_url, &mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } }, - } - + }; // If necessary, exhaust the reader to compute the hash. if !hashes.is_none() { hasher.finish().await.map_err(Error::HashExhaustion)?; } + // Before we make the wheel accessible by persisting it, ensure that the RECORD is + // valid. + validate_and_heal_record(temp_dir.path(), files.iter(), dist) + .map_err(Error::InstallWheelError)?; + // Persist the temporary directory to the directory store. let id = self .build_context @@ -883,52 +891,49 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { .map_err(Error::CacheWrite)?; // If no hashes are required, parallelize the unzip operation. - let hashes = if hashes.is_none() { + let (files, hashes) = if hashes.is_none() { + // Unzip the wheel into a temporary directory. let file = file.into_std().await; - tokio::task::spawn_blocking({ - let target = temp_dir.path().to_owned(); - move || -> Result<(), uv_extract::Error> { - // Unzip the wheel into a temporary directory. - match extension { - WheelExtension::Whl => { - uv_extract::unzip(file, &target)?; - } - WheelExtension::WhlZst => { - uv_extract::stream::untar_zst_file(file, &target)?; - } - } - Ok(()) - } + let target = temp_dir.path().to_owned(); + let files = tokio::task::spawn_blocking(move || match extension { + WheelExtension::Whl => uv_extract::unzip(file, &target), + WheelExtension::WhlZst => uv_extract::stream::untar_zst_file(file, &target), }) .await? .map_err(|err| Error::Extract(filename.to_string(), err))?; - HashDigests::empty() + (files, HashDigests::empty()) } else { // Create a hasher for each hash algorithm. let algorithms = hashes.algorithms(); let mut hashers = algorithms.into_iter().map(Hasher::from).collect::>(); let mut hasher = uv_extract::hash::HashReader::new(file, &mut hashers); - match extension { + let files = match extension { WheelExtension::Whl => { uv_extract::stream::unzip(query_url, &mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } - } + }; // If necessary, exhaust the reader to compute the hash. hasher.finish().await.map_err(Error::HashExhaustion)?; + let hashes = hashers.into_iter().map(HashDigest::from).collect(); - hashers.into_iter().map(HashDigest::from).collect() + (files, hashes) }; + // Before we make the wheel accessible by persisting it, ensure that the RECORD is + // valid. + validate_and_heal_record(temp_dir.path(), files.iter(), dist) + .map_err(Error::InstallWheelError)?; + // Persist the temporary directory to the directory store. let id = self .build_context @@ -1062,7 +1067,8 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { } else if hashes.is_none() { // Otherwise, unzip the wheel. let archive = Archive::new( - self.unzip_wheel(path, wheel_entry.path()).await?, + self.unzip_wheel(path, wheel_entry.path(), DistRef::Built(dist)) + .await?, HashDigests::empty(), filename.clone(), ); @@ -1100,24 +1106,29 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { let mut hasher = uv_extract::hash::HashReader::new(file, &mut hashers); // Unzip the wheel to a temporary directory. - match extension { + let files = match extension { WheelExtension::Whl => { uv_extract::stream::unzip(path.display(), &mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } WheelExtension::WhlZst => { uv_extract::stream::untar_zst(&mut hasher, temp_dir.path()) .await - .map_err(|err| Error::Extract(filename.to_string(), err))?; + .map_err(|err| Error::Extract(filename.to_string(), err))? } - } + }; // Exhaust the reader to compute the hash. hasher.finish().await.map_err(Error::HashExhaustion)?; let hashes = hashers.into_iter().map(HashDigest::from).collect(); + // Before we make the wheel accessible by persisting it, ensure that the RECORD is + // valid. + validate_and_heal_record(temp_dir.path(), files.iter(), dist) + .map_err(Error::InstallWheelError)?; + // Persist the temporary directory to the directory store. let id = self .build_context @@ -1152,21 +1163,30 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { } /// Unzip a wheel into the cache, returning the path to the unzipped directory. - async fn unzip_wheel(&self, path: &Path, target: &Path) -> Result { - let temp_dir = tokio::task::spawn_blocking({ + async fn unzip_wheel( + &self, + path: &Path, + target: &Path, + dist: DistRef<'_>, + ) -> Result { + let (temp_dir, files) = tokio::task::spawn_blocking({ let path = path.to_owned(); let root = self.build_context.cache().root().to_path_buf(); - move || -> Result { + move || -> Result<_, Error> { // Unzip the wheel into a temporary directory. let temp_dir = tempfile::tempdir_in(root).map_err(Error::CacheWrite)?; let reader = fs_err::File::open(&path).map_err(Error::CacheWrite)?; - uv_extract::unzip(reader, temp_dir.path()) + let files = uv_extract::unzip(reader, temp_dir.path()) .map_err(|err| Error::Extract(path.to_string_lossy().into_owned(), err))?; - Ok(temp_dir) + Ok((temp_dir, files)) } }) .await??; + // Before we make the wheel accessible by persisting it, ensure that the RECORD is valid. + validate_and_heal_record(temp_dir.path(), files.iter(), dist) + .map_err(Error::InstallWheelError)?; + // Persist the temporary directory to the directory store. let id = self .build_context diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs index b885fc1ef..b9f85c989 100644 --- a/crates/uv-distribution/src/error.rs +++ b/crates/uv-distribution/src/error.rs @@ -196,6 +196,9 @@ pub enum Error { #[error("Hash-checking is not supported for Git repositories: `{0}`")] HashesNotSupportedGit(String), + + #[error(transparent)] + InstallWheelError(uv_install_wheel::Error), } impl From for Error { diff --git a/crates/uv-extract/src/stream.rs b/crates/uv-extract/src/stream.rs index cc824380e..9a5c91f81 100644 --- a/crates/uv-extract/src/stream.rs +++ b/crates/uv-extract/src/stream.rs @@ -48,11 +48,13 @@ struct ComputedEntry { /// /// `source_hint` is used for warning messages, to identify the source of the ZIP archive /// beneath the reader. It might be a URL, a file path, or something else. +/// +/// Returns the list of unpacked files and their sizes. pub async fn unzip( source_hint: D, reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { /// Ensure the file path is safe to use as a [`Path`]. /// /// See: @@ -82,6 +84,7 @@ pub async fn unzip( let mut directories = FxHashSet::default(); let mut local_headers = FxHashMap::default(); + let mut files = Vec::new(); let mut offset = 0; while let Some(mut entry) = zip.next_with_entry().await? { @@ -272,6 +275,9 @@ pub async fn unzip( } } + // Collect file paths (excluding directories). + files.push((relpath.clone(), actual_uncompressed_size)); + ComputedEntry { crc32: actual_crc32, uncompressed_size: actual_uncompressed_size, @@ -577,22 +583,26 @@ pub async fn unzip( } } - Ok(()) + Ok(files) } /// Unpack the given tar archive into the destination directory. /// /// This is equivalent to `archive.unpack_in(dst)`, but it also preserves the executable bit. +/// +/// Returns the list of unpacked files and their sizes. async fn untar_in( mut archive: tokio_tar::Archive<&'_ mut (dyn tokio::io::AsyncRead + Unpin)>, dst: &Path, -) -> std::io::Result<()> { +) -> std::io::Result> { // Like `tokio-tar`, canonicalize the destination prior to unpacking. let dst = fs_err::tokio::canonicalize(dst).await?; // Memoize filesystem calls to canonicalize paths. let mut memo = FxHashSet::default(); + let mut files = Vec::new(); + let mut entries = archive.entries()?; let mut pinned = Pin::new(&mut entries); while let Some(entry) = pinned.next().await { @@ -609,6 +619,14 @@ async fn untar_in( continue; } + // Collect file paths (excluding directories). + let entry_type = file.header().entry_type(); + if entry_type.is_file() || entry_type.is_hard_link() { + let relpath = file.path()?.into_owned(); + let size = file.header().size()?; + files.push((relpath, size)); + } + // Unpack the file into the destination directory. #[cfg_attr(not(unix), allow(unused_variables))] let unpacked_at = file.unpack_in_raw(&dst, &mut memo).await?; @@ -619,7 +637,6 @@ async fn untar_in( use std::fs::Permissions; use std::os::unix::fs::PermissionsExt; - let entry_type = file.header().entry_type(); if entry_type.is_file() || entry_type.is_hard_link() { let mode = file.header().mode()?; let has_any_executable_bit = mode & 0o111; @@ -639,16 +656,18 @@ async fn untar_in( } } - Ok(()) + Ok(files) } /// Unpack a `.tar.gz` archive into the target directory, without requiring `Seek`. /// /// This is useful for unpacking files as they're being downloaded. +/// +/// Returns the list of unpacked files and their sizes. pub async fn untar_gz( reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { let reader = tokio::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let mut decompressed_bytes = async_compression::tokio::bufread::GzipDecoder::new(reader); @@ -667,10 +686,12 @@ pub async fn untar_gz( /// Unpack a `.tar.bz2` archive into the target directory, without requiring `Seek`. /// /// This is useful for unpacking files as they're being downloaded. +/// +/// Returns the list of unpacked files and their sizes. pub async fn untar_bz2( reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { let reader = tokio::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let mut decompressed_bytes = async_compression::tokio::bufread::BzDecoder::new(reader); @@ -689,10 +710,12 @@ pub async fn untar_bz2( /// Unpack a `.tar.zst` archive into the target directory, without requiring `Seek`. /// /// This is useful for unpacking files as they're being downloaded. +/// +/// Returns the list of unpacked files and their sizes. pub async fn untar_zst( reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { let reader = tokio::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let mut decompressed_bytes = async_compression::tokio::bufread::ZstdDecoder::new(reader); @@ -709,22 +732,70 @@ pub async fn untar_zst( } /// Unpack a `.tar.zst` archive from a file on disk into the target directory. -pub fn untar_zst_file(reader: R, target: impl AsRef) -> Result<(), Error> { +/// +/// Returns the list of unpacked files and their sizes. +pub fn untar_zst_file( + reader: R, + target: impl AsRef, +) -> Result, Error> { let reader = std::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let decompressed = zstd::Decoder::new(reader).map_err(Error::Io)?; let mut archive = tar::Archive::new(decompressed); archive.set_preserve_mtime(false); - archive.unpack(target).map_err(Error::io_or_compression)?; - Ok(()) + + // The logic below is `Archive::unpack`, with slight simplifications as we know the target is + // a real directory, using our error handling and adding file recording. + let mut files = Vec::new(); + + // Canonicalizing the dst directory will prepend the path with '\\?\' + // on windows which will allow windows APIs to treat the path as an + // extended-length path with a 32,767 character limit. Otherwise all + // unpacked paths over 260 characters will fail on creation with a + // NotFound exception. + let dst = fs_err::canonicalize(&target).unwrap_or(target.as_ref().to_path_buf()); + + // Delay any directory entries until the end (they will be created if needed by + // descendants), to ensure that directory permissions do not interfere with descendant + // extraction. + let mut directories = Vec::new(); + for entry in archive.entries().map_err(Error::io_or_compression)? { + let mut file = entry.map_err(Error::io_or_compression)?; + if file.header().entry_type() == tar::EntryType::Directory { + directories.push(file); + } else { + let entry_type = file.header().entry_type(); + let path = file.path().map_err(Error::io_or_compression)?.into_owned(); + let size = file.header().size().map_err(Error::io_or_compression)?; + if entry_type.is_file() || entry_type.is_hard_link() { + files.push((path, size)); + } + file.unpack_in(&dst).map_err(Error::io_or_compression)?; + } + } + + // Apply the directories. + // + // Note: the order of application is important to permissions. That is, we must traverse + // the filesystem graph in topological ordering or else we risk not being able to create + // child directories within those of more restrictive permissions. See [0] for details. + // + // [0]: + directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes())); + for mut dir in directories { + dir.unpack_in(&dst).map_err(Error::io_or_compression)?; + } + Ok(files) } /// Unpack a `.tar.xz` archive into the target directory, without requiring `Seek`. /// /// This is useful for unpacking files as they're being downloaded. +/// +/// Returns the list of unpacked files and their sizes. pub async fn untar_xz( reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { let reader = tokio::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let mut decompressed_bytes = async_compression::tokio::bufread::XzDecoder::new(reader); @@ -737,17 +808,18 @@ pub async fn untar_xz( .build(); untar_in(archive, target.as_ref()) .await - .map_err(Error::io_or_compression)?; - Ok(()) + .map_err(Error::io_or_compression) } /// Unpack a `.tar` archive into the target directory, without requiring `Seek`. /// /// This is useful for unpacking files as they're being downloaded. +/// +/// Returns the list of unpacked files and their sizes. pub async fn untar( reader: R, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { let mut reader = tokio::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader); let archive = @@ -758,8 +830,7 @@ pub async fn untar( .build(); untar_in(archive, target.as_ref()) .await - .map_err(Error::io_or_compression)?; - Ok(()) + .map_err(Error::io_or_compression) } /// Unpack a `.zip`, `.tar.gz`, `.tar.bz2`, `.tar.zst`, or `.tar.xz` archive into the target directory, @@ -767,35 +838,24 @@ pub async fn untar( /// /// `source_hint` is used for warning messages, to identify the source of the archive /// beneath the reader. It might be a URL, a file path, or something else. +/// +/// Returns the list of unpacked files and their sizes. pub async fn archive( source_hint: D, reader: R, ext: SourceDistExtension, target: impl AsRef, -) -> Result<(), Error> { +) -> Result, Error> { match ext { - SourceDistExtension::Zip => { - unzip(source_hint, reader, target).await?; - } - SourceDistExtension::Tar => { - untar(reader, target).await?; - } - SourceDistExtension::Tgz | SourceDistExtension::TarGz => { - untar_gz(reader, target).await?; - } - SourceDistExtension::Tbz | SourceDistExtension::TarBz2 => { - untar_bz2(reader, target).await?; - } + SourceDistExtension::Zip => unzip(source_hint, reader, target).await, + SourceDistExtension::Tar => untar(reader, target).await, + SourceDistExtension::Tgz | SourceDistExtension::TarGz => untar_gz(reader, target).await, + SourceDistExtension::Tbz | SourceDistExtension::TarBz2 => untar_bz2(reader, target).await, SourceDistExtension::Txz | SourceDistExtension::TarXz | SourceDistExtension::Tlz | SourceDistExtension::TarLz - | SourceDistExtension::TarLzma => { - untar_xz(reader, target).await?; - } - SourceDistExtension::TarZst => { - untar_zst(reader, target).await?; - } + | SourceDistExtension::TarLzma => untar_xz(reader, target).await, + SourceDistExtension::TarZst => untar_zst(reader, target).await, } - Ok(()) } diff --git a/crates/uv-extract/src/sync.rs b/crates/uv-extract/src/sync.rs index c84ed65f0..225c0c23f 100644 --- a/crates/uv-extract/src/sync.rs +++ b/crates/uv-extract/src/sync.rs @@ -11,7 +11,9 @@ use uv_warnings::warn_user_once; use zip::ZipArchive; /// Unzip a `.zip` archive into the target directory. -pub fn unzip(reader: fs_err::File, target: &Path) -> Result<(), Error> { +/// +/// Returns the list of unpacked files and their sizes. +pub fn unzip(reader: fs_err::File, target: &Path) -> Result, Error> { let (reader, filename) = reader.into_parts(); // Unzip in parallel. @@ -47,17 +49,17 @@ pub fn unzip(reader: fs_err::File, target: &Path) -> Result<(), Error> { // Determine the path of the file within the wheel. let Some(enclosed_name) = file.enclosed_name() else { warn!("Skipping unsafe file name: {}", file.name()); - return Ok(()); + return Ok(None); }; // Create necessary parent directories. - let path = target.join(enclosed_name); + let path = target.join(&enclosed_name); if file.is_dir() { let mut directories = directories.lock().unwrap(); if directories.insert(path.clone()) { fs_err::create_dir_all(path).map_err(Error::Io)?; } - return Ok(()); + return Ok(None); } if let Some(parent) = path.parent() { @@ -102,8 +104,10 @@ pub fn unzip(reader: fs_err::File, target: &Path) -> Result<(), Error> { } } - Ok(()) + Ok(Some((enclosed_name, size))) }) + // Filter out directories and skipped dangerous paths, we only want to collect the files. + .filter_map(Result::transpose) .collect::>() } diff --git a/crates/uv-install-wheel/src/install.rs b/crates/uv-install-wheel/src/install.rs index e1d05c939..e14031afb 100644 --- a/crates/uv-install-wheel/src/install.rs +++ b/crates/uv-install-wheel/src/install.rs @@ -14,7 +14,7 @@ use uv_pypi_types::{DirectUrl, Metadata10}; use crate::linker::{InstallState, LinkMode, link_wheel_files}; use crate::wheel::{ LibKind, WheelFile, dist_info_metadata, find_dist_info, install_data, parse_scripts, - read_record_file, write_installer_metadata, write_script_entrypoints, + read_record, write_installer_metadata, write_record, write_script_entrypoints, }; use crate::{Error, Layout}; @@ -83,7 +83,7 @@ pub fn install_wheel( .as_ref() .join(format!("{dist_info_prefix}.dist-info/RECORD")), )?; - let mut record = read_record_file(&mut record_file)?; + let mut record = read_record(&mut record_file)?; let (console_scripts, gui_scripts) = parse_scripts(&wheel, &dist_info_prefix, None, layout.python_version.1)?; @@ -149,14 +149,7 @@ pub fn install_wheel( } trace!(?name, "Writing record"); - let mut record_writer = csv::WriterBuilder::new() - .has_headers(false) - .escape(b'"') - .from_path(site_packages.join(format!("{dist_info_prefix}.dist-info/RECORD")))?; - record.sort(); - for entry in record { - record_writer.serialize(entry)?; - } + write_record(site_packages, &dist_info_prefix, record)?; Ok(()) } diff --git a/crates/uv-install-wheel/src/lib.rs b/crates/uv-install-wheel/src/lib.rs index 3ba45b1e9..0632d1d5f 100644 --- a/crates/uv-install-wheel/src/lib.rs +++ b/crates/uv-install-wheel/src/lib.rs @@ -13,8 +13,9 @@ use uv_pypi_types::Scheme; pub use install::install_wheel; pub use linker::{InstallState, LinkMode, link_wheel_files}; +pub use record::RecordEntry; pub use uninstall::{Uninstall, uninstall_egg, uninstall_legacy_editable, uninstall_wheel}; -pub use wheel::{LibKind, WheelFile, read_record_file}; +pub use wheel::{LibKind, WheelFile, read_record, validate_and_heal_record}; mod install; mod linker; @@ -47,10 +48,20 @@ pub enum Error { /// Doesn't follow file name schema #[error("Failed to move data files")] WalkDir(#[from] walkdir::Error), - #[error("RECORD file doesn't match wheel contents: {0}")] - RecordFile(String), + // This shouldn't be possible anymore, we keep it for better error reporting. + #[error( + "RECORD file doesn't match wheel contents, could not find entry for: {} ({})", + relative.simplified_display(), + absolute.simplified_display() + )] + RecordFile { + relative: PathBuf, + absolute: PathBuf, + }, #[error("RECORD file is invalid")] RecordCsv(#[from] csv::Error), + #[error("Non-UTF8 path in {0}: {1:?}")] + NonUtf8WheelPath(String, PathBuf), #[error("Broken virtual environment: {0}")] BrokenVenv(String), #[error( diff --git a/crates/uv-install-wheel/src/uninstall.rs b/crates/uv-install-wheel/src/uninstall.rs index 8ff29f9df..d3e5aa90c 100644 --- a/crates/uv-install-wheel/src/uninstall.rs +++ b/crates/uv-install-wheel/src/uninstall.rs @@ -8,7 +8,7 @@ use tracing::trace; use uv_fs::write_atomic_sync; use uv_warnings::warn_user; -use crate::wheel::read_record_file; +use crate::wheel::read_record; use crate::{Error, Layout}; /// Uninstall the wheel represented by the given `.dist-info` directory. @@ -33,7 +33,7 @@ pub fn uninstall_wheel( } Err(err) => return Err(err.into()), }; - read_record_file(&mut record_file)? + read_record(&mut record_file)? }; let mut file_count = 0usize; @@ -161,11 +161,12 @@ pub fn uninstall_wheel( static WARNED_FOR_PACKAGE: OnceLock>> = OnceLock::new(); -/// Warn and reject paths that are not part of the venv or the system interpreter. +/// Check if the path is inside the venv or a system interpreter path, and warn if it isn't. /// -/// Reject RECORD entries that escape site-packages via path traversal (e.g., -/// `../../../etc/passwd`). A malicious wheel could include such entries to cause -/// deletion of arbitrary files on uninstall. +/// Returns `false` is a path is outside the paths that files from a wheel can be installed into, +/// so that the caller can reject RECORD entries that escape site-packages via path traversal (e.g., +/// `../../../etc/passwd`). A malicious wheel could otherwise include such entries to cause deletion +/// of arbitrary files on uninstall. fn is_path_in_scheme( path: &str, site_packages: &Path, diff --git a/crates/uv-install-wheel/src/wheel.rs b/crates/uv-install-wheel/src/wheel.rs index 4037567e2..ed008340e 100644 --- a/crates/uv-install-wheel/src/wheel.rs +++ b/crates/uv-install-wheel/src/wheel.rs @@ -1,4 +1,5 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; +use std::fmt::Display; use std::io; use std::io::{BufReader, Read, Write}; use std::path::{Path, PathBuf}; @@ -6,13 +7,14 @@ use std::path::{Path, PathBuf}; use data_encoding::BASE64URL_NOPAD; use fs_err as fs; use fs_err::{DirEntry, File}; +use itertools::Itertools; use mailparse::parse_headers; use rustc_hash::FxHashMap; use sha2::{Digest, Sha256}; use tracing::{debug, instrument, trace, warn}; use walkdir::WalkDir; -use uv_fs::{Simplified, persist_with_retry_sync, relative_to}; +use uv_fs::{PortablePath, Simplified, persist_with_retry_sync, relative_to}; use uv_normalize::PackageName; use uv_pypi_types::DirectUrl; use uv_shell::escape_posix_for_single_quotes; @@ -366,12 +368,9 @@ pub(crate) fn move_folder_recorded( let entry = record .iter_mut() .find(|entry| Path::new(&entry.path) == relative_to_site_packages) - .ok_or_else(|| { - Error::RecordFile(format!( - "Could not find entry for {} ({})", - relative_to_site_packages.simplified_display(), - src.simplified_display() - )) + .ok_or_else(|| Error::RecordFile { + relative: relative_to_site_packages.to_path_buf(), + absolute: src.to_path_buf(), })?; entry.path = relative_to(&target, site_packages)? .portable_display() @@ -569,12 +568,11 @@ fn install_script( .iter_mut() .find(|entry| Path::new(&entry.path) == relative_to_site_packages) .ok_or_else(|| { - // This should be possible to occur at this point, but filesystems and such - Error::RecordFile(format!( - "Could not find entry for {} ({})", - relative_to_site_packages.simplified_display(), - path.simplified_display() - )) + // It should not be possible to error at this point, but filesystems and such. + Error::RecordFile { + relative: relative_to_site_packages.to_path_buf(), + absolute: path.clone(), + } })?; // Update the entry in the `RECORD`. @@ -793,7 +791,7 @@ pub(crate) fn get_relocatable_executable( /// Reads the record file /// -pub fn read_record_file(record: &mut impl Read) -> Result, Error> { +pub fn read_record(record: impl Read) -> Result, Error> { csv::ReaderBuilder::new() .has_headers(false) .escape(Some(b'"')) @@ -810,6 +808,113 @@ pub fn read_record_file(record: &mut impl Read) -> Result, Erro .collect() } +pub(crate) fn write_record( + site_packages: &Path, + dist_info_prefix: &str, + mut record: Vec, +) -> Result<(), Error> { + let record_file = site_packages.join(format!("{dist_info_prefix}.dist-info/RECORD")); + let mut record_writer = csv::WriterBuilder::new() + .has_headers(false) + .escape(b'"') + .from_path(record_file)?; + record.sort(); + for entry in record { + record_writer.serialize(entry)?; + } + Ok(()) +} + +/// Validate the RECORD and heal invalid RECORD files. +/// +/// This ensures that all unpacked wheels have record that matches the contents, and uninstall can't +/// remove files that don't belong to the wheel. +/// +/// This function is given both the location of the unpacked wheel and the list of files from the +/// wheel that were unpacked to avoid a walkdir for this check. +pub fn validate_and_heal_record<'a>( + wheel_dir: &Path, + unpacked_wheel: impl IntoIterator, + dist: impl Display, +) -> Result<(), Error> { + // On the filesystem: The unpacked files of the wheel. + let mut files: BTreeMap<&Path, u64> = unpacked_wheel + .into_iter() + .map(|(path, size)| (path.as_path(), *size)) + .collect(); + + // In the record: The files we expect in the wheel. + let dist_info_prefix = find_dist_info(wheel_dir)?; + let dist_info_dir = format!("{dist_info_prefix}.dist-info"); + let record_path = wheel_dir.join(&dist_info_dir).join("RECORD"); + let mut record_file = File::open(&record_path)?; + let mut record = read_record(&mut record_file)?; + + // Remove matching files from both collections. + let mut extra_record_entries = Vec::new(); + record.retain(|entry| { + let path = Path::new(&entry.path); + if files.remove(path).is_some() { + return true; + } + // Allow non-canonical spellings such as `./foo`. + if files.remove(uv_fs::normalize_path(path).as_ref()).is_some() { + return true; + } + extra_record_entries.push(path.to_path_buf()); + false + }); + + if !files.is_empty() { + // Deprecated, but not listed in RECORD if used. + files.remove(Path::new(&dist_info_dir).join("RECORD.jws").as_path()); + files.remove(Path::new(&dist_info_dir).join("RECORD.p7s").as_path()); + } + + // If the RECORD was correct, there were no extra entries in the record and no missing entries + // that weren't removed from files. + if !extra_record_entries.is_empty() { + debug!( + "RECORD contains files not in wheel archive for {}: `{}`", + dist, + extra_record_entries + .iter() + .map(Simplified::simplified_display) + .join("`, `") + ); + } + if !files.is_empty() { + debug!( + "Wheel archive contains files not in RECORD for {}: `{}`", + dist, + files + .keys() + .map(Simplified::simplified_display) + .join("`, `") + ); + } + if !extra_record_entries.is_empty() || !files.is_empty() { + debug!("Rewriting RECORD to match actual wheel contents for {dist}"); + // We already removed RECORD entries with no matching unpacked file, now add files that + // were unpacked but not listed in the archive. + for (path, size) in files { + record.push(RecordEntry { + // RECORD entries always use forward slashes, even on Windows. + path: PortablePath::from(path).to_string(), + // We don't heal the hash. It's not validated anyway (pip doesn't), and by rules of + // the spec the wheel would have been rejected anyway (if the spec would have been + // enforced). + hash: None, + size: Some(size), + }); + } + + write_record(wheel_dir, &dist_info_prefix, record)?; + } + + Ok(()) +} + /// Parse a file with email message format such as WHEEL and METADATA fn parse_email_message_file( file: impl Read, @@ -842,7 +947,7 @@ fn parse_email_message_file( Ok(data) } -/// Find the `dist-info` directory in an unzipped wheel. +/// Find the prefix of the `dist-info` directory in an unzipped wheel. /// /// See: /// @@ -953,7 +1058,7 @@ mod test { use super::{ Error, RecordEntry, Script, WheelFile, format_shebang, get_script_executable, - parse_email_message_file, read_record_file, write_installer_metadata, + parse_email_message_file, read_record, write_installer_metadata, }; #[test] @@ -1040,7 +1145,7 @@ mod test { selenium-4.1.0.dist-info/RECORD,, "}; - let entries = read_record_file(&mut record.as_bytes()).unwrap(); + let entries = read_record(&mut record.as_bytes()).unwrap(); let expected = [ "selenium/__init__.py", "selenium/common/exceptions.py", diff --git a/crates/uv-tool/src/lib.rs b/crates/uv-tool/src/lib.rs index 336f3e9a3..3e0a11bae 100644 --- a/crates/uv-tool/src/lib.rs +++ b/crates/uv-tool/src/lib.rs @@ -11,7 +11,7 @@ use tracing::{debug, warn}; use uv_cache::Cache; use uv_dirs::user_executable_directory; use uv_fs::{LockedFile, LockedFileError, LockedFileMode, Simplified}; -use uv_install_wheel::read_record_file; +use uv_install_wheel::read_record; use uv_installer::SitePackages; use uv_normalize::{InvalidNameError, PackageName}; use uv_pep440::Version; @@ -459,7 +459,7 @@ pub fn entrypoint_paths( ); // Read the RECORD file. - let record = read_record_file(&mut File::open(dist_info_path.join("RECORD"))?)?; + let record = read_record(File::open(dist_info_path.join("RECORD"))?)?; // The RECORD file uses relative paths, so we're looking for the relative path to be a prefix. let layout = site_packages.interpreter().layout(); diff --git a/crates/uv/src/commands/pip/show.rs b/crates/uv/src/commands/pip/show.rs index a3d5186e8..1dd5d8403 100644 --- a/crates/uv/src/commands/pip/show.rs +++ b/crates/uv/src/commands/pip/show.rs @@ -10,7 +10,7 @@ use tracing::debug; use uv_cache::Cache; use uv_distribution_types::{DependencyMetadata, Diagnostic, Name}; use uv_fs::Simplified; -use uv_install_wheel::read_record_file; +use uv_install_wheel::read_record; use uv_installer::SitePackages; use uv_normalize::PackageName; use uv_preview::Preview; @@ -218,7 +218,7 @@ pub(crate) fn pip_show( // If requests, show the list of installed files. if files { let path = distribution.install_path().join("RECORD"); - let record = read_record_file(&mut File::open(path)?)?; + let record = read_record(&mut File::open(path)?)?; writeln!(printer.stdout(), "Files:")?; for entry in record { writeln!(printer.stdout(), " {}", entry.path)?; diff --git a/crates/uv/tests/it/extract.rs b/crates/uv/tests/it/extract.rs index bfdd4b2ff..1bf347944 100644 --- a/crates/uv/tests/it/extract.rs +++ b/crates/uv/tests/it/extract.rs @@ -23,7 +23,8 @@ async fn unzip(url: &str) -> anyhow::Result<(), uv_extract::Error> { .into_async_read(); let target = tempfile::TempDir::new().map_err(uv_extract::Error::Io)?; - uv_extract::stream::unzip(url, reader.compat(), target.path()).await + uv_extract::stream::unzip(url, reader.compat(), target.path()).await?; + Ok(()) } #[tokio::test] diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index e7d84758c..a6878ac8a 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -1,3 +1,4 @@ +use std::io; use std::io::Cursor; use std::path::PathBuf; use std::process::Command; @@ -9,20 +10,24 @@ use flate2::write::GzEncoder; use fs_err as fs; use fs_err::File; use indoc::{formatdoc, indoc}; +use insta::assert_snapshot; use predicates::prelude::predicate; use url::Url; +use walkdir::WalkDir; use wiremock::{ Mock, MockServer, ResponseTemplate, matchers::{basic_auth, method, path}, }; +use zip::write::SimpleFileOptions; +use zip::{ZipArchive, ZipWriter}; -use uv_fs::Simplified; +use uv_fs::{PortablePath, Simplified}; use uv_static::EnvVars; #[cfg(feature = "test-git")] use uv_test::decode_token; use uv_test::{ - DEFAULT_PYTHON_VERSION, TestContext, build_vendor_links_url, download_to_disk, get_bin, - packse_index_url, uv_snapshot, venv_bin_path, + DEFAULT_PYTHON_VERSION, TestContext, apply_filters, build_vendor_links_url, download_to_disk, + get_bin, packse_index_url, uv_snapshot, venv_bin_path, }; #[test] @@ -14918,3 +14923,106 @@ fn upgrade_group_not_supported() { error: `--upgrade-group` is not supported in `uv pip` commands "); } + +/// Check that the RECORD in a wheel with that doesn't match its contents gets fixed before +/// installation. +#[test] +fn handle_record_mismatches() -> Result<()> { + let context = uv_test::test_context!("3.12") + .with_filter(( + regex::escape(r"foo-0.1.0.dist-info/uv_cache.json,sha256=") + ".*", + r"foo-0.1.0.dist-info/uv_cache.json,sha256=[SHA256],[SIZE]", + )) + .with_filter(( + regex::escape(r"foo-0.1.0.dist-info/WHEEL,sha256=") + ".*", + r"foo-0.1.0.dist-info/WHEEL,sha256=[SHA256],[SIZE]", + )); + + // Build a small wheel and unpack it for modification. + context.init().arg("--lib").arg("foo").assert().success(); + context.build().arg("--wheel").arg("foo").assert().success(); + let built_wheel = context.temp_dir.join("foo/dist/foo-0.1.0-py3-none-any.whl"); + let unpacked = context.temp_dir.join("foo-unpacked"); + ZipArchive::new(File::open(built_wheel)?)?.extract(&unpacked)?; + + // Snapshot the current (correct) RECORD. + let record = unpacked.join("foo-0.1.0.dist-info/RECORD"); + let correct_record = fs_err::read_to_string(&record)?; + let correct_record = apply_filters(correct_record, context.filters()); + assert_snapshot!(correct_record, @" + foo/__init__.py,sha256=jv2QBpHSNajIRNeADSmtqOWL9QcdUddyMK277kbp06o,49 + foo/py.typed,sha256=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU,0 + foo-0.1.0.dist-info/WHEEL,sha256=[SHA256],[SIZE] + foo-0.1.0.dist-info/METADATA,sha256=d-PGjuBKXweF5NgWUW5yDnAjBY0lg4uFZBqcnmGtNgY,147 + foo-0.1.0.dist-info/RECORD,, + "); + + // Create a broken RECORD: Remove 2 files, and add a bogus one. + fs_err::write( + &record, + indoc! {" + foo/py.typed,sha256=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU,0 + foo-0.1.0.dist-info/WHEEL,sha256=hbX8mDThv1n7VEIpQRy6c2yAFTw4iAQlEC53gDAhHSo,80 + foo-0.1.0.dist-info/RECORD,, + ../../../../etc/passwd,,0 + "}, + )?; + + // Repack the wheel. + let repacked_wheel = context.temp_dir.join("foo-0.1.0-py3-none-any.whl"); + let mut writer = ZipWriter::new(File::create(&repacked_wheel)?); + let options = SimpleFileOptions::default(); + for entry in WalkDir::new(&unpacked) { + let entry = entry?; + let path = entry.path(); + let name = path.strip_prefix(&unpacked)?; + if name.as_os_str().is_empty() { + continue; + } + // Zip entries must use forward slashes, even on Windows. + let name = PortablePath::from(name).to_string(); + if path.is_dir() { + writer.add_directory(&name, options)?; + } else { + writer.start_file(&name, options)?; + io::copy(&mut File::open(path)?, &mut writer)?; + } + } + writer.finish()?; + + uv_snapshot!(context.filters(), context.pip_install() + .arg("--find-links") + .arg(context.temp_dir.as_ref()) + .arg("--offline") + .arg("foo"), @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + foo==0.1.0 + " + ); + + // Read the healed RECORD. + let installed_record = + fs_err::read_to_string(context.site_packages().join("foo-0.1.0.dist-info/RECORD"))?; + let snapshot = apply_filters(installed_record, context.filters()); + + // Ensure that all expected files are present. + assert_snapshot!(&snapshot, @" + foo-0.1.0.dist-info/INSTALLER,sha256=5hhM4Q4mYTT9z6QB6PGpUAW81PGNFrYrdXMj4oM_6ak,2 + foo-0.1.0.dist-info/METADATA,,147 + foo-0.1.0.dist-info/RECORD,, + foo-0.1.0.dist-info/REQUESTED,sha256=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU,0 + foo-0.1.0.dist-info/WHEEL,sha256=[SHA256],[SIZE] + foo-0.1.0.dist-info/uv_cache.json,sha256=[SHA256],[SIZE] + foo/__init__.py,,49 + foo/py.typed,sha256=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU,0 + "); + + Ok(()) +}