diff --git a/crates/uv-auth/src/cache.rs b/crates/uv-auth/src/cache.rs index 0968877e621..1253957c4a0 100644 --- a/crates/uv-auth/src/cache.rs +++ b/crates/uv-auth/src/cache.rs @@ -11,7 +11,7 @@ use url::Url; use uv_once_map::OnceMap; use uv_redacted::DisplaySafeUrl; -use crate::credentials::{Authentication, Username}; +use crate::credentials::{Authentication, CredentialsFromUrlError, Username}; use crate::{Credentials, Realm}; type FxOnceMap = OnceMap>; @@ -62,13 +62,16 @@ impl CredentialsCache { /// Populate the global authentication store with credentials on a URL, if there are any. /// /// Returns `true` if the store was updated. - pub fn store_credentials_from_url(&self, url: &DisplaySafeUrl) -> bool { - if let Some(credentials) = Credentials::from_url(url) { + pub fn store_credentials_from_url( + &self, + url: &DisplaySafeUrl, + ) -> Result { + if let Some(credentials) = Credentials::from_url(url)? { trace!("Caching credentials for {url}"); self.insert(url, Arc::new(Authentication::from(credentials))); - true + Ok(true) } else { - false + Ok(false) } } diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs index a9221db4be8..39fca3e2e5c 100644 --- a/crates/uv-auth/src/credentials.rs +++ b/crates/uv-auth/src/credentials.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use std::fmt; use std::io::Read; use std::io::Write; -use std::str::FromStr; +use std::str::{FromStr, Utf8Error}; use base64::prelude::BASE64_STANDARD; use base64::read::DecoderReader; @@ -39,6 +39,14 @@ pub enum Credentials { }, } +#[derive(Debug, Error)] +pub enum CredentialsFromUrlError { + #[error("URL username contains invalid UTF-8")] + InvalidUsernameUtf8(#[source] Utf8Error), + #[error("URL password contains invalid UTF-8")] + InvalidPasswordUtf8(#[source] Utf8Error), +} + #[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Default, Serialize, Deserialize)] #[serde(transparent)] pub struct Username(Option); @@ -223,31 +231,37 @@ impl Credentials { /// Parse [`Credentials`] from a URL, if any. /// /// Returns [`None`] if both [`Url::username`] and [`Url::password`] are not populated. - pub fn from_url(url: &Url) -> Option { + pub fn from_url(url: &Url) -> Result, CredentialsFromUrlError> { if url.username().is_empty() && url.password().is_none() { - return None; + return Ok(None); } - Some(Self::Basic { - // Remove percent-encoding from URL credentials - // See - username: if url.username().is_empty() { - None - } else { - Some( - percent_encoding::percent_decode_str(url.username()) - .decode_utf8_lossy() - .into_owned(), - ) - } - .into(), - password: url.password().map(|password| { - Password( - percent_encoding::percent_decode_str(password) - .decode_utf8_lossy() - .into_owned(), - ) - }), - }) + + // Remove percent-encoding from URL credentials. + // See + let username = if url.username().is_empty() { + None + } else { + Some( + percent_encoding::percent_decode_str(url.username()) + .decode_utf8() + .map_err(CredentialsFromUrlError::InvalidUsernameUtf8)? + .into_owned(), + ) + }; + let password = url + .password() + .map(|password| { + percent_encoding::percent_decode_str(password) + .decode_utf8() + .map(|password| Password(password.into_owned())) + .map_err(CredentialsFromUrlError::InvalidPasswordUtf8) + }) + .transpose()?; + + Ok(Some(Self::Basic { + username: username.into(), + password, + })) } /// Extract the [`Credentials`] from the environment, given a named source. @@ -267,15 +281,17 @@ impl Credentials { /// Parse [`Credentials`] from an HTTP request, if any. /// /// Only HTTP Basic Authentication is supported. - pub(crate) fn from_request(request: &Request) -> Option { + pub(crate) fn from_request(request: &Request) -> Result, CredentialsFromUrlError> { // First, attempt to retrieve the credentials from the URL - Self::from_url(request.url()).or( - // Then, attempt to pull the credentials from the headers - request - .headers() - .get(reqwest::header::AUTHORIZATION) - .map(Self::from_header_value)?, - ) + if let Some(credentials) = Self::from_url(request.url())? { + return Ok(Some(credentials)); + } + + // Then, attempt to pull the credentials from the headers + Ok(request + .headers() + .get(reqwest::header::AUTHORIZATION) + .and_then(Self::from_header_value)) } /// Parse [`Credentials`] from an authorization header, if any. @@ -619,7 +635,7 @@ impl Authentication { #[cfg(test)] mod tests { - use insta::assert_debug_snapshot; + use insta::{assert_debug_snapshot, assert_snapshot}; use reqsign::aws::Credential as AwsCredential; use reqsign::azure::Credential as AzureCredential; use reqsign::{Context, ProvideCredential}; @@ -657,7 +673,7 @@ mod tests { #[test] fn from_url_no_credentials() { let url = &Url::parse("https://example.com/simple/first/").unwrap(); - assert_eq!(Credentials::from_url(url), None); + assert!(matches!(Credentials::from_url(url), Ok(None))); } #[test] @@ -666,7 +682,7 @@ mod tests { let mut auth_url = url.clone(); auth_url.set_username("user").unwrap(); auth_url.set_password(Some("password")).unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); assert_eq!(credentials.username(), Some("user")); assert_eq!(credentials.password(), Some("password")); } @@ -674,17 +690,15 @@ mod tests { #[test] fn from_url_invalid_utf8_username() { let url = Url::parse("https://%FF:password@example.com/simple/first/").unwrap(); - let credentials = Credentials::from_url(&url).unwrap(); - assert_eq!(credentials.username(), Some("\u{fffd}")); - assert_eq!(credentials.password(), Some("password")); + let error = Credentials::from_url(&url).unwrap_err(); + assert_snapshot!(error, @"URL username contains invalid UTF-8"); } #[test] fn from_url_invalid_utf8_password() { let url = Url::parse("https://user:%FF@example.com/simple/first/").unwrap(); - let credentials = Credentials::from_url(&url).unwrap(); - assert_eq!(credentials.username(), Some("user")); - assert_eq!(credentials.password(), Some("\u{fffd}")); + let error = Credentials::from_url(&url).unwrap_err(); + assert_snapshot!(error, @"URL password contains invalid UTF-8"); } #[test] @@ -692,7 +706,7 @@ mod tests { let url = &Url::parse("https://example.com/simple/first/").unwrap(); let mut auth_url = url.clone(); auth_url.set_password(Some("password")).unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); assert_eq!(credentials.username(), None); assert_eq!(credentials.password(), Some("password")); } @@ -705,7 +719,7 @@ mod tests { fn from_url_empty_username_with_password() { // Parse a URL with the format `:password@host` directly let url = Url::parse("https://:token@example.com/simple/first/").unwrap(); - let credentials = Credentials::from_url(&url).unwrap(); + let credentials = Credentials::from_url(&url).unwrap().unwrap(); assert_eq!(credentials.username(), None); assert_eq!(credentials.password(), Some("token")); assert!( @@ -719,7 +733,7 @@ mod tests { let url = &Url::parse("https://example.com/simple/first/").unwrap(); let mut auth_url = url.clone(); auth_url.set_username("user").unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); assert_eq!(credentials.username(), Some("user")); assert_eq!(credentials.password(), None); } @@ -730,7 +744,7 @@ mod tests { let mut auth_url = url.clone(); auth_url.set_username("user").unwrap(); auth_url.set_password(Some("password")).unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); let mut request = Request::new(reqwest::Method::GET, url); request = credentials.authenticate(request); @@ -752,7 +766,7 @@ mod tests { let mut auth_url = url.clone(); auth_url.set_username("user@domain").unwrap(); auth_url.set_password(Some("password")).unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); let mut request = Request::new(reqwest::Method::GET, url); request = credentials.authenticate(request); @@ -774,7 +788,7 @@ mod tests { let mut auth_url = url.clone(); auth_url.set_username("user").unwrap(); auth_url.set_password(Some("password==")).unwrap(); - let credentials = Credentials::from_url(&auth_url).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap().unwrap(); let mut request = Request::new(reqwest::Method::GET, url); request = credentials.authenticate(request); diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 1748402f974..2a23627e751 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -1,6 +1,6 @@ pub use access_token::AccessToken; pub use cache::CredentialsCache; -pub use credentials::{Credentials, Username}; +pub use credentials::{Credentials, CredentialsFromUrlError, Username}; pub use index::{AuthPolicy, Index, Indexes}; pub use keyring::KeyringProvider; pub use middleware::AuthMiddleware; diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 44a82b7e5d0..c0e5f43c9d0 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -20,7 +20,9 @@ use crate::pyx::{DEFAULT_TOLERANCE_SECS, PyxTokenStore}; use crate::{ AccessToken, CredentialsCache, KeyringProvider, cache::FetchUrl, - credentials::{Authentication, AuthenticationError, Credentials, Username}, + credentials::{ + Authentication, AuthenticationError, Credentials, CredentialsFromUrlError, Username, + }, index::{AuthPolicy, Indexes}, realm::Realm, }; @@ -36,6 +38,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: CredentialsFromUrlError) -> Self { + Self::middleware(err) + } +} + /// Strategy for loading netrc files. enum NetrcMode { Automatic(LazyLock>), @@ -363,7 +371,7 @@ impl Middleware for AuthMiddleware { next: Next<'_>, ) -> reqwest_middleware::Result { // Check for credentials attached to the request already - let request_credentials = Credentials::from_request(&request).map(Authentication::from); + let request_credentials = Credentials::from_request(&request)?.map(Authentication::from); // In the middleware, existing credentials are already moved from the URL // to the headers so for display purposes we restore some information diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index dec2dfd24b9..a81f801b69e 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -21,9 +21,12 @@ use tracing::{debug, warn}; use url::ParseError; use url::Url; -use uv_auth::{AuthMiddleware, Credentials, CredentialsCache, Indexes, PyxTokenStore}; +use uv_auth::{ + AuthMiddleware, Credentials, CredentialsCache, CredentialsFromUrlError, Indexes, PyxTokenStore, +}; use uv_configuration::ProxyUrlKind; use uv_configuration::{KeyringProviderType, ProxyUrl, TrustedHost}; +use uv_distribution_types::IndexCredentialsError; use uv_git::GitHttpSettings; use uv_pep508::MarkerEnvironment; use uv_platform_tags::Platform; @@ -61,13 +64,13 @@ pub const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); pub const DEFAULT_READ_TIMEOUT_UPLOAD: Duration = Duration::from_mins(15); #[derive(Debug, Error)] -#[error("failed to build HTTP client")] -pub struct ClientBuildError(#[source] reqwest::Error); - -impl From for ClientBuildError { - fn from(error: reqwest::Error) -> Self { - Self(error) - } +pub enum ClientBuildError { + #[error("failed to build HTTP client")] + Reqwest(#[from] reqwest::Error), + #[error(transparent)] + Credentials(#[from] CredentialsFromUrlError), + #[error(transparent)] + IndexCredentials(#[from] IndexCredentialsError), } /// Selectively skip parts or the entire auth middleware. @@ -361,7 +364,10 @@ impl<'a> BaseClientBuilder<'a> { } /// See [`CredentialsCache::store_credentials_from_url`]. - pub fn store_credentials_from_url(&self, url: &DisplaySafeUrl) -> bool { + pub fn store_credentials_from_url( + &self, + url: &DisplaySafeUrl, + ) -> Result { self.credentials_cache.store_credentials_from_url(url) } @@ -962,7 +968,9 @@ fn request_into_redirect( // Check if there are credentials on the redirect location itself. // If so, move them to Authorization header. if !redirect_url.username().is_empty() { - if let Some(credentials) = Credentials::from_url(&redirect_url) { + if let Some(credentials) = + Credentials::from_url(&redirect_url).map_err(reqwest_middleware::Error::middleware)? + { let _ = redirect_url.set_username(""); let _ = redirect_url.set_password(None); headers.insert(AUTHORIZATION, credentials.to_header_value()); diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 28f4b11fb04..ff039acaa3b 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -142,9 +142,9 @@ impl<'a> RegistryClientBuilder<'a> { } /// Add all authenticated sources to the cache. - fn cache_index_credentials(&mut self) { + fn cache_index_credentials(&mut self) -> Result<(), ClientBuildError> { for index in self.index_locations.known_indexes() { - if let Some(credentials) = index.credentials() { + if let Some(credentials) = index.credentials()? { trace!( "Read credentials for index {}", index @@ -161,10 +161,11 @@ impl<'a> RegistryClientBuilder<'a> { .store_credentials(index.raw_url(), credentials); } } + Ok(()) } pub fn build(mut self) -> Result { - self.cache_index_credentials(); + self.cache_index_credentials()?; let index_urls = self.index_locations.index_urls(); // Build a base client @@ -194,8 +195,11 @@ impl<'a> RegistryClientBuilder<'a> { } /// Share the underlying client between two different middleware configurations. - pub fn wrap_existing(mut self, existing: &BaseClient) -> RegistryClient { - self.cache_index_credentials(); + pub fn wrap_existing( + mut self, + existing: &BaseClient, + ) -> Result { + self.cache_index_credentials()?; let index_urls = self.index_locations.index_urls(); // Wrap in any relevant middleware and handle connectivity. @@ -210,7 +214,7 @@ impl<'a> RegistryClientBuilder<'a> { // Wrap in the cache middleware. let client = CachedClient::new(client); - RegistryClient { + Ok(RegistryClient { index_urls, index_strategy: self.index_strategy, torch_backend: self.torch_backend, @@ -220,7 +224,7 @@ impl<'a> RegistryClientBuilder<'a> { read_timeout, flat_indexes: Arc::default(), pyx_token_store: PyxTokenStore::from_settings().ok(), - } + }) } } diff --git a/crates/uv-distribution-types/src/index.rs b/crates/uv-distribution-types/src/index.rs index 36c6fe39a33..32345cbf767 100644 --- a/crates/uv-distribution-types/src/index.rs +++ b/crates/uv-distribution-types/src/index.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize, Serializer}; use thiserror::Error; use url::Url; -use uv_auth::{AuthPolicy, Credentials}; +use uv_auth::{AuthPolicy, Credentials, CredentialsFromUrlError}; use uv_redacted::DisplaySafeUrl; use uv_small_str::SmallString; @@ -255,6 +255,14 @@ pub struct Index { pub exclude_newer: Option, } +#[derive(Debug, Error)] +#[error("Failed to parse credentials in index URL: {url}")] +pub struct IndexCredentialsError { + url: DisplaySafeUrl, + #[source] + source: CredentialsFromUrlError, +} + impl PartialEq for Index { fn eq(&self, other: &Self) -> bool { let Self { @@ -453,23 +461,44 @@ impl Index { /// are stripped from the stored URL. #[must_use] pub fn with_promoted_auth_policy(mut self) -> Self { - if matches!(self.authenticate, AuthPolicy::Auto) && self.credentials().is_some() { + if matches!(self.authenticate, AuthPolicy::Auto) && self.has_credentials() { self.authenticate = AuthPolicy::Always; } self } + /// Return whether credentials are configured for the index. + /// + /// This only checks for the presence of credentials. It intentionally avoids decoding URL + /// credentials, since this is used to preserve the authentication policy after credentials are + /// removed from the stored URL; parsing errors are reported when the credentials are retrieved. + fn has_credentials(&self) -> bool { + if self + .name + .as_ref() + .is_some_and(|name| Credentials::from_env(name.to_env_var()).is_some()) + { + return true; + } + + let url = self.url.url(); + !url.username().is_empty() || url.password().is_some() + } + /// Retrieve the credentials for the index, either from the environment, or from the URL itself. - pub fn credentials(&self) -> Option { + pub fn credentials(&self) -> Result, IndexCredentialsError> { // If the index is named, and credentials are provided via the environment, prefer those. if let Some(name) = self.name.as_ref() { if let Some(credentials) = Credentials::from_env(name.to_env_var()) { - return Some(credentials); + return Ok(Some(credentials)); } } // Otherwise, extract the credentials from the URL. - Credentials::from_url(self.url.url()) + Credentials::from_url(self.url.url()).map_err(|source| IndexCredentialsError { + url: self.url.url().clone(), + source, + }) } /// Resolve the index relative to the given root directory. diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs index 08e736da732..210478105c0 100644 --- a/crates/uv-distribution/src/metadata/lowering.rs +++ b/crates/uv-distribution/src/metadata/lowering.rs @@ -8,7 +8,8 @@ use thiserror::Error; use uv_auth::CredentialsCache; use uv_distribution_filename::DistExtension; use uv_distribution_types::{ - Index, IndexLocations, IndexMetadata, IndexName, Origin, Requirement, RequirementSource, + Index, IndexCredentialsError, IndexLocations, IndexMetadata, IndexName, Origin, Requirement, + RequirementSource, }; use uv_fs::{Simplified, normalize_absolute_path, normalize_path}; use uv_git_types::{GitLfs, GitReference, GitUrl, GitUrlParseError}; @@ -244,7 +245,7 @@ impl LoweredRequirement { hint, }); }; - if let Some(credentials) = index.credentials() { + if let Some(credentials) = index.credentials()? { credentials_cache.store_credentials(index.raw_url(), credentials); } let index = IndexMetadata { @@ -484,7 +485,7 @@ impl LoweredRequirement { hint, }); }; - if let Some(credentials) = index.credentials() { + if let Some(credentials) = index.credentials()? { credentials_cache.store_credentials(index.raw_url(), credentials); } let index = IndexMetadata { @@ -596,6 +597,8 @@ pub enum LoweringError { #[error(transparent)] InvalidUrl(#[from] DisplaySafeUrlError), #[error(transparent)] + IndexCredentials(#[from] IndexCredentialsError), + #[error(transparent)] InvalidVerbatimUrl(#[from] uv_pep508::VerbatimUrlError), #[error("Fragments are not allowed in URLs: `{0}`")] ForbiddenFragment(DisplaySafeUrl), diff --git a/crates/uv-git/src/credentials.rs b/crates/uv-git/src/credentials.rs index a7885eeb9b0..326dd54cab4 100644 --- a/crates/uv-git/src/credentials.rs +++ b/crates/uv-git/src/credentials.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::sync::{Arc, LazyLock, RwLock}; use tracing::trace; -use uv_auth::Credentials; +use uv_auth::{Credentials, CredentialsFromUrlError}; use uv_cache_key::RepositoryUrl; use uv_redacted::DisplaySafeUrl; @@ -34,12 +34,12 @@ pub fn store_credentials(url: RepositoryUrl, credentials: Credentials) { /// Populate the global authentication store with credentials on a Git URL, if there are any. /// /// Returns `true` if the store was updated. -pub fn store_credentials_from_url(url: &DisplaySafeUrl) -> bool { - if let Some(credentials) = Credentials::from_url(url) { +pub fn store_credentials_from_url(url: &DisplaySafeUrl) -> Result { + if let Some(credentials) = Credentials::from_url(url)? { trace!("Caching credentials for {url}"); store_credentials(RepositoryUrl::new(url), credentials); - true + Ok(true) } else { - false + Ok(false) } } diff --git a/crates/uv-publish/src/lib.rs b/crates/uv-publish/src/lib.rs index 53ad1a64bcb..56b8097ea4b 100644 --- a/crates/uv-publish/src/lib.rs +++ b/crates/uv-publish/src/lib.rs @@ -26,8 +26,8 @@ use url::Url; use uv_auth::{Credentials, PyxTokenStore, Realm}; use uv_cache::{Cache, Refresh}; use uv_client::{ - BaseClient, DEFAULT_MAX_REDIRECTS, MetadataFormat, OwnedArchive, RegistryClientBuilder, - RequestBuilder, RetryParsingError, RetryState, + BaseClient, ClientBuildError, DEFAULT_MAX_REDIRECTS, MetadataFormat, OwnedArchive, + RegistryClientBuilder, RequestBuilder, RetryParsingError, RetryState, }; use uv_configuration::{KeyringProviderType, TrustedPublishing}; use uv_distribution_filename::{DistFilename, SourceDistExtension, SourceDistFilename}; @@ -78,6 +78,8 @@ pub enum PublishError { MixedCredentials(String), #[error("Failed to query check URL")] CheckUrlIndex(#[source] uv_client::Error), + #[error(transparent)] + ClientBuild(#[from] ClientBuildError), #[error( "Local file and index file do not match for {filename}. \ Local: {hash_algorithm}={local}, Remote: {hash_algorithm}={remote}" @@ -962,7 +964,7 @@ pub async fn check_url( let registry_client = registry_client_builder .clone() .cache(cache_refresh) - .wrap_existing(client); + .wrap_existing(client)?; debug!("Checking for {filename} in the registry"); let response = match registry_client diff --git a/crates/uv/src/commands/auth/helper.rs b/crates/uv/src/commands/auth/helper.rs index ba16a044883..d8481f8dd99 100644 --- a/crates/uv/src/commands/auth/helper.rs +++ b/crates/uv/src/commands/auth/helper.rs @@ -68,7 +68,7 @@ async fn credentials_for_url( let pyx_store = PyxTokenStore::from_settings()?; // Use only the username from the URL, if present - discarding the password - let url_credentials = Credentials::from_url(url); + let url_credentials = Credentials::from_url(url)?; let username = url_credentials.as_ref().and_then(|c| c.username()); if url_credentials .as_ref() diff --git a/crates/uv/src/commands/auth/login.rs b/crates/uv/src/commands/auth/login.rs index c71f2a3b6f8..a5037a967ad 100644 --- a/crates/uv/src/commands/auth/login.rs +++ b/crates/uv/src/commands/auth/login.rs @@ -71,7 +71,7 @@ pub(crate) async fn login( }; // Extract credentials from URL if present - let url_credentials = Credentials::from_url(&url); + let url_credentials = Credentials::from_url(&url)?; let url_username = url_credentials.as_ref().and_then(|c| c.username()); let url_password = url_credentials.as_ref().and_then(|c| c.password()); diff --git a/crates/uv/src/commands/auth/logout.rs b/crates/uv/src/commands/auth/logout.rs index 599f511ecb4..65851bd68ef 100644 --- a/crates/uv/src/commands/auth/logout.rs +++ b/crates/uv/src/commands/auth/logout.rs @@ -39,7 +39,7 @@ pub(crate) async fn logout( }; // Extract credentials from URL if present - let url_credentials = Credentials::from_url(&url); + let url_credentials = Credentials::from_url(&url)?; let url_username = url_credentials.as_ref().and_then(|c| c.username()); let username = match (username, url_username) { diff --git a/crates/uv/src/commands/auth/token.rs b/crates/uv/src/commands/auth/token.rs index 4c460b7bd26..ad3f4ab497e 100644 --- a/crates/uv/src/commands/auth/token.rs +++ b/crates/uv/src/commands/auth/token.rs @@ -37,7 +37,7 @@ pub(crate) async fn token( let url = service.url(); // Extract credentials from URL if present - let url_credentials = Credentials::from_url(url); + let url_credentials = Credentials::from_url(url)?; let url_username = url_credentials.as_ref().and_then(|c| c.username()); let username = match (username, url_username) { diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 1ebcb879d84..fe194a23c4f 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -922,7 +922,7 @@ fn edits( extra, group, }) => { - let credentials = uv_auth::Credentials::from_url(&git); + let credentials = uv_auth::Credentials::from_url(&git)?; if let Some(credentials) = credentials { debug!("Caching credentials for: {git}"); store_credentials(RepositoryUrl::new(&git), credentials); diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 265bdce551a..607bf71f49a 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -697,7 +697,7 @@ async fn do_lock( let client_builder = client_builder.clone().keyring(*keyring_provider); for index in target.indexes() { - if let Some(credentials) = index.credentials() { + if let Some(credentials) = index.credentials()? { if let Some(root_url) = index.root_url() { client_builder.store_credentials(&root_url, credentials.clone()); } diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 7855063c377..c179c082ba0 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -8,7 +8,7 @@ use owo_colors::OwoColorize; use tracing::{debug, trace, warn}; use uv_audit::osv; use uv_audit::{Dependency, VulnerabilityID}; -use uv_auth::CredentialsCache; +use uv_auth::{CredentialsCache, CredentialsFromUrlError}; use uv_cache::{Cache, CacheBucket}; use uv_cache_key::{cache_digest, cache_name}; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; @@ -19,8 +19,8 @@ use uv_configuration::{ use uv_dispatch::{BuildDispatch, SharedState}; use uv_distribution::{DistributionDatabase, LoweredExtraBuildDependencies, LoweredRequirement}; use uv_distribution_types::{ - ExtraBuildRequirement, ExtraBuildRequires, Index, Requirement, RequiresPython, Resolution, - UnresolvedRequirement, UnresolvedRequirementSpecification, + ExtraBuildRequirement, ExtraBuildRequires, Index, IndexCredentialsError, Requirement, + RequiresPython, Resolution, UnresolvedRequirement, UnresolvedRequirementSpecification, }; use uv_fs::{CWD, LockedFile, LockedFileError, LockedFileMode, Simplified}; use uv_git::ResolvedRepositoryReference; @@ -295,6 +295,12 @@ pub(crate) enum ProjectError { #[error(transparent)] ClientBuild(#[from] uv_client::ClientBuildError), + #[error(transparent)] + Credentials(#[from] CredentialsFromUrlError), + + #[error(transparent)] + IndexCredentials(#[from] IndexCredentialsError), + #[error(transparent)] Python(#[from] uv_python::Error), diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index ef4c25f5d3d..777a93bc177 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -796,7 +796,7 @@ pub(crate) async fn do_sync( let extra_build_requires = extra_build_requires.match_runtime(&resolution)?; // Populate credentials from the target. - store_credentials_from_target(target, &client_builder); + store_credentials_from_target(target, &client_builder)?; // Initialize the registry client. let client = RegistryClientBuilder::new(client_builder, cache.clone()) @@ -1025,10 +1025,13 @@ fn apply_no_virtual_project(resolution: Resolution) -> Resolution { /// /// These credentials can come from any of `tool.uv.sources`, `tool.uv.dev-dependencies`, /// `project.dependencies`, and `project.optional-dependencies`. -fn store_credentials_from_target(target: InstallTarget<'_>, client_builder: &BaseClientBuilder) { +fn store_credentials_from_target( + target: InstallTarget<'_>, + client_builder: &BaseClientBuilder, +) -> Result<()> { // Iterate over any indexes in the target. for index in target.indexes() { - if let Some(credentials) = index.credentials() { + if let Some(credentials) = index.credentials()? { if let Some(root_url) = index.root_url() { client_builder.store_credentials(&root_url, credentials.clone()); } @@ -1040,10 +1043,10 @@ fn store_credentials_from_target(target: InstallTarget<'_>, client_builder: &Bas for source in target.sources() { match source { Source::Git { git, .. } => { - uv_git::store_credentials_from_url(git); + uv_git::store_credentials_from_url(git)?; } Source::Url { url, .. } => { - client_builder.store_credentials_from_url(url); + client_builder.store_credentials_from_url(url)?; } _ => {} } @@ -1057,14 +1060,15 @@ fn store_credentials_from_target(target: InstallTarget<'_>, client_builder: &Bas match &url.parsed_url { ParsedUrl::GitDirectory(ParsedGitDirectoryUrl { url, .. }) | ParsedUrl::GitPath(ParsedGitPathUrl { url, .. }) => { - uv_git::store_credentials_from_url(url.url()); + uv_git::store_credentials_from_url(url.url())?; } ParsedUrl::Archive(ParsedArchiveUrl { url, .. }) => { - client_builder.store_credentials_from_url(url); + client_builder.store_credentials_from_url(url)?; } _ => {} } } + Ok(()) } #[derive(Debug, Serialize)] diff --git a/crates/uv/tests/pip_install/pip_install.rs b/crates/uv/tests/pip_install/pip_install.rs index ae08384d021..8148d4bad92 100644 --- a/crates/uv/tests/pip_install/pip_install.rs +++ b/crates/uv/tests/pip_install/pip_install.rs @@ -6338,6 +6338,27 @@ async fn install_package_basic_auth_from_url() { context.assert_command("import anyio").success(); } +/// Reject credentials that are not valid UTF-8. +#[test] +fn install_package_basic_auth_invalid_utf8() { + let context = uv_test::test_context!("3.12"); + + uv_snapshot!(context.filters(), context.pip_install() + .arg("anyio") + .arg("--index-url") + .arg("https://user:%FF@example.com/simple") + .arg("--strict"), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse credentials in index URL: https://user:****@example.com/simple + Caused by: URL password contains invalid UTF-8 + Caused by: invalid utf-8 sequence of 1 bytes from index 0 + "); +} + /// Install a package from an index that requires authentication #[tokio::test] async fn install_package_basic_auth_from_netrc_default() -> Result<()> {