Skip to content

Commit

Permalink
Add --vaidate-hashes
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jun 4, 2024
1 parent 40e0ddd commit 6e9ed7e
Show file tree
Hide file tree
Showing 11 changed files with 438 additions and 44 deletions.
2 changes: 1 addition & 1 deletion crates/uv-requirements/src/source_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {
let hashes = match self.hasher {
HashStrategy::None => HashPolicy::None,
HashStrategy::Generate => HashPolicy::Generate,
HashStrategy::Validate { .. } => {
HashStrategy::Verify(_) | HashStrategy::Require(_) => {
return Err(anyhow::anyhow!(
"Hash-checking is not supported for local directories: {}",
path.user_display()
Expand Down
167 changes: 132 additions & 35 deletions crates/uv-types/src/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ pub enum HashStrategy {
None,
/// Hashes should be generated (specifically, a SHA-256 hash), but not validated.
Generate,
/// Hashes should be validated against a pre-defined list of hashes. If necessary, hashes should
/// be generated so as to ensure that the archive is valid.
Validate(FxHashMap<PackageId, Vec<HashDigest>>),
/// 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>>),
/// 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>>),
}

impl HashStrategy {
Expand All @@ -26,7 +31,14 @@ impl HashStrategy {
match self {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Validate(hashes) => HashPolicy::Validate(
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&distribution.package_id()) {
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
}
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&distribution.package_id())
.map(Vec::as_slice)
Expand All @@ -40,7 +52,14 @@ impl HashStrategy {
match self {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Validate(hashes) => HashPolicy::Validate(
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&PackageId::from_registry(name.clone())) {
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
}
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&PackageId::from_registry(name.clone()))
.map(Vec::as_slice)
Expand All @@ -54,7 +73,14 @@ impl HashStrategy {
match self {
Self::None => HashPolicy::None,
Self::Generate => HashPolicy::Generate,
Self::Validate(hashes) => HashPolicy::Validate(
Self::Verify(hashes) => {
if let Some(hashes) = hashes.get(&PackageId::from_url(url)) {
HashPolicy::Validate(hashes.as_slice())
} else {
HashPolicy::None
}
}
Self::Require(hashes) => HashPolicy::Validate(
hashes
.get(&PackageId::from_url(url))
.map(Vec::as_slice)
Expand All @@ -68,7 +94,8 @@ impl HashStrategy {
match self {
Self::None => true,
Self::Generate => true,
Self::Validate(hashes) => hashes.contains_key(&PackageId::from_registry(name.clone())),
Self::Verify(_) => true,
Self::Require(hashes) => hashes.contains_key(&PackageId::from_registry(name.clone())),
}
}

Expand All @@ -77,7 +104,8 @@ impl HashStrategy {
match self {
Self::None => true,
Self::Generate => true,
Self::Validate(hashes) => hashes.contains_key(&PackageId::from_url(url)),
Self::Verify(_) => true,
Self::Require(hashes) => hashes.contains_key(&PackageId::from_url(url)),
}
}

Expand All @@ -87,7 +115,7 @@ impl HashStrategy {
/// that reference the environment as true. In other words, it does
/// environment independent expression evaluation. (Which in turn devolves
/// to "only evaluate marker expressions that reference an extra name.")
pub fn from_requirements<'a>(
pub fn require<'a>(
requirements: impl Iterator<Item = (&'a UnresolvedRequirement, &'a [String])>,
markers: Option<&MarkerEnvironment>,
) -> Result<Self, HashStrategyError> {
Expand All @@ -103,7 +131,12 @@ impl HashStrategy {
// Every requirement must be either a pinned version or a direct URL.
let id = match &requirement {
UnresolvedRequirement::Named(requirement) => {
uv_requirement_to_package_id(requirement)?
Self::pin(requirement).ok_or_else(|| {
HashStrategyError::UnpinnedRequirement(
requirement.to_string(),
HashCheckingMode::Require,
)
})?
}
UnresolvedRequirement::Unnamed(requirement) => {
// Direct URLs are always allowed.
Expand All @@ -113,7 +146,10 @@ impl HashStrategy {

// Every requirement must include a hash.
if digests.is_empty() {
return Err(HashStrategyError::MissingHashes(requirement.to_string()));
return Err(HashStrategyError::MissingHashes(
requirement.to_string(),
HashCheckingMode::Require,
));
}

// Parse the hashes.
Expand All @@ -125,41 +161,102 @@ impl HashStrategy {
hashes.insert(id, digests);
}

Ok(Self::Validate(hashes))
Ok(Self::Require(hashes))
}
}

fn uv_requirement_to_package_id(requirement: &Requirement) -> Result<PackageId, HashStrategyError> {
Ok(match &requirement.source {
RequirementSource::Registry { specifier, .. } => {
// Must be a single specifier.
let [specifier] = specifier.as_ref() else {
return Err(HashStrategyError::UnpinnedRequirement(
requirement.to_string(),
));
/// Generate the hashes to verify from a set of [`UnresolvedRequirement`] entries.
pub fn verify<'a>(
requirements: impl Iterator<Item = (&'a UnresolvedRequirement, &'a [String])>,
markers: Option<&MarkerEnvironment>,
) -> Result<Self, HashStrategyError> {
let mut hashes = FxHashMap::<PackageId, Vec<HashDigest>>::default();

// For each requirement, map from name to allowed hashes. We use the last entry for each
// package.
for (requirement, digests) in requirements {
if !requirement.evaluate_markers(markers, &[]) {
continue;
}

// Hashes are optional in this mode.
if digests.is_empty() {
continue;
}

// Parse the hashes.
let digests = digests
.iter()
.map(|digest| HashDigest::from_str(digest))
.collect::<Result<Vec<_>, _>>()?;

// Every requirement must be either a pinned version or a direct URL.
let id = match &requirement {
UnresolvedRequirement::Named(requirement) => {
Self::pin(requirement).ok_or_else(|| {
HashStrategyError::UnpinnedRequirement(
requirement.to_string(),
HashCheckingMode::Verify,
)
})?
}
UnresolvedRequirement::Unnamed(requirement) => {
// Direct URLs are always allowed.
PackageId::from_url(&requirement.url.verbatim)
}
};

// Must be pinned to a specific version.
if *specifier.operator() != pep440_rs::Operator::Equal {
return Err(HashStrategyError::UnpinnedRequirement(
requirement.to_string(),
));
hashes.insert(id, digests);
}

Ok(Self::Verify(hashes))
}

/// Pin a [`Requirement`] to a [`PackageId`], if possible.
fn pin(requirement: &Requirement) -> Option<PackageId> {
match &requirement.source {
RequirementSource::Registry { specifier, .. } => {
// Must be a single specifier.
let [specifier] = specifier.as_ref() else {
return None;
};

// Must be pinned to a specific version.
if *specifier.operator() != pep440_rs::Operator::Equal {
return None;
}

Some(PackageId::from_registry(requirement.name.clone()))
}
RequirementSource::Url { url, .. }
| RequirementSource::Git { url, .. }
| RequirementSource::Path { url, .. } => Some(PackageId::from_url(url)),
}
}
}

PackageId::from_registry(requirement.name.clone())
#[derive(Debug, Copy, Clone)]
pub enum HashCheckingMode {
Require,
Verify,
}

impl std::fmt::Display for HashCheckingMode {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Require => write!(f, "--require-hashes"),
Self::Verify => write!(f, "--verify-hashes"),
}
RequirementSource::Url { url, .. }
| RequirementSource::Git { url, .. }
| RequirementSource::Path { url, .. } => PackageId::from_url(url),
})
}
}

#[derive(thiserror::Error, Debug)]
pub enum HashStrategyError {
#[error(transparent)]
Hash(#[from] HashError),
#[error("In `--require-hashes` mode, all requirement must have their versions pinned with `==`, but found: {0}")]
UnpinnedRequirement(String),
#[error("In `--require-hashes` mode, all requirement must have a hash, but none were provided for: {0}")]
MissingHashes(String),
#[error(
"In `{1}` mode, all requirement must have their versions pinned with `==`, but found: {0}"
)]
UnpinnedRequirement(String, HashCheckingMode),
#[error("In `{1}` mode, all requirement must have a hash, but none were provided for: {0}")]
MissingHashes(String, HashCheckingMode),
}
1 change: 1 addition & 0 deletions crates/uv-workspace/src/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ impl Combine for PipOptions {
link_mode: self.link_mode.combine(other.link_mode),
compile_bytecode: self.compile_bytecode.combine(other.compile_bytecode),
require_hashes: self.require_hashes.combine(other.require_hashes),
verify_hashes: self.verify_hashes.combine(other.verify_hashes),
concurrent_downloads: self
.concurrent_downloads
.combine(other.concurrent_downloads),
Expand Down
1 change: 1 addition & 0 deletions crates/uv-workspace/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub struct PipOptions {
pub link_mode: Option<LinkMode>,
pub compile_bytecode: Option<bool>,
pub require_hashes: Option<bool>,
pub verify_hashes: Option<bool>,
pub concurrent_downloads: Option<NonZeroUsize>,
pub concurrent_builds: Option<NonZeroUsize>,
pub concurrent_installs: Option<NonZeroUsize>,
Expand Down
40 changes: 38 additions & 2 deletions crates/uv/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,13 +663,33 @@ pub(crate) struct PipSyncArgs {
/// - Editable installs are not supported.
/// - Local dependencies are not supported, unless they point to a specific wheel (`.whl`) or
/// source archive (`.zip`, `.tar.gz`), as opposed to a directory.
#[arg(long, env = "UV_REQUIRE_HASHES",
value_parser = clap::builder::BoolishValueParser::new(), overrides_with("no_require_hashes"))]
#[arg(
long,
env = "UV_REQUIRE_HASHES",
value_parser = clap::builder::BoolishValueParser::new(),
overrides_with("no_require_hashes"),
)]
pub(crate) require_hashes: bool,

#[arg(long, overrides_with("require_hashes"), hide = true)]
pub(crate) no_require_hashes: bool,

/// Validate any hashes provided in the requirements file.
///
/// Unlike `--require-hashes`, `--verify-hashes` does not require that all requirements have
/// hashes; instead, it will limit itself to verifying the hashes of those requirements that do
/// include them.
#[arg(
long,
env = "UV_VERIFY_HASHES",
value_parser = clap::builder::BoolishValueParser::new(),
overrides_with("no_verify_hashes"),
)]
pub(crate) verify_hashes: bool,

#[arg(long, overrides_with("verify_hashes"), hide = true)]
pub(crate) no_verify_hashes: bool,

/// Attempt to use `keyring` for authentication for index URLs.
///
/// Function's similar to `pip`'s `--keyring-provider subprocess` argument,
Expand Down Expand Up @@ -1026,6 +1046,22 @@ pub(crate) struct PipInstallArgs {
#[arg(long, overrides_with("require_hashes"), hide = true)]
pub(crate) no_require_hashes: bool,

/// Validate any hashes provided in the requirements file.
///
/// Unlike `--require-hashes`, `--verify-hashes` does not require that all requirements have
/// hashes; instead, it will limit itself to verifying the hashes of those requirements that do
/// include them.
#[arg(
long,
env = "UV_VERIFY_HASHES",
value_parser = clap::builder::BoolishValueParser::new(),
overrides_with("no_verify_hashes"),
)]
pub(crate) verify_hashes: bool,

#[arg(long, overrides_with("verify_hashes"), hide = true)]
pub(crate) no_verify_hashes: bool,

/// Attempt to use `keyring` for authentication for index URLs.
///
/// Due to not having Python imports, only `--keyring-provider subprocess` argument is currently
Expand Down
11 changes: 10 additions & 1 deletion crates/uv/src/commands/pip/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub(crate) async fn pip_install(
link_mode: LinkMode,
compile: bool,
require_hashes: bool,
verify_hashes: bool,
setup_py: SetupPyStrategy,
connectivity: Connectivity,
config_settings: &ConfigSettings,
Expand Down Expand Up @@ -247,7 +248,15 @@ pub(crate) async fn pip_install(

// Collect the set of required hashes.
let hasher = if require_hashes {
HashStrategy::from_requirements(
HashStrategy::require(
requirements
.iter()
.chain(overrides.iter())
.map(|entry| (&entry.requirement, entry.hashes.as_slice())),
Some(&markers),
)?
} else if verify_hashes {
HashStrategy::verify(
requirements
.iter()
.chain(overrides.iter())
Expand Down
12 changes: 11 additions & 1 deletion crates/uv/src/commands/pip/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub(crate) async fn pip_sync(
link_mode: LinkMode,
compile: bool,
require_hashes: bool,
verify_hashes: bool,
index_locations: IndexLocations,
index_strategy: IndexStrategy,
keyring_provider: KeyringProviderType,
Expand Down Expand Up @@ -198,9 +199,18 @@ pub(crate) async fn pip_sync(

// Collect the set of required hashes.
let hasher = if require_hashes {
HashStrategy::from_requirements(
HashStrategy::require(
requirements
.iter()
.chain(overrides.iter())
.map(|entry| (&entry.requirement, entry.hashes.as_slice())),
Some(&markers),
)?
} else if verify_hashes {
HashStrategy::verify(
requirements
.iter()
.chain(overrides.iter())
.map(|entry| (&entry.requirement, entry.hashes.as_slice())),
Some(&markers),
)?
Expand Down
Loading

0 comments on commit 6e9ed7e

Please sign in to comment.