Reject invalid UTF-8 URL credentials

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<K, V> = OnceMap<K, V, BuildHasherDefault<FxHasher>>;
@@ -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<bool, CredentialsFromUrlError> {
+        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)
         }
     }
 

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<String>);
@@ -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<Self> {
+    pub fn from_url(url: &Url) -> Result<Option<Self>, 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 <https://github.com/pypa/pip/blob/06d21db4ff1ab69665c22a88718a4ea9757ca293/src/pip/_internal/utils/misc.py#L497-L499>
-            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 <https://github.com/pypa/pip/blob/06d21db4ff1ab69665c22a88718a4ea9757ca293/src/pip/_internal/utils/misc.py#L497-L499>
+        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<Self> {
+    pub(crate) fn from_request(request: &Request) -> Result<Option<Self>, 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,33 +682,31 @@ 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"));
     }
 
     #[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]
     fn from_url_no_username() {
         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);

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;

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<AuthenticationError> for Error {
     }
 }
 
+impl From<CredentialsFromUrlError> for Error {
+    fn from(err: CredentialsFromUrlError) -> Self {
+        Self::middleware(err)
+    }
+}
+
 /// Strategy for loading netrc files.
 enum NetrcMode {
     Automatic(LazyLock<Option<Netrc>>),
@@ -363,7 +371,7 @@ impl Middleware for AuthMiddleware {
         next: Next<'_>,
     ) -> reqwest_middleware::Result<Response> {
         // 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

crates/uv-client/src/base_client.rs

@@ -21,7 +21,9 @@ 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_git::GitHttpSettings;
@@ -61,13 +63,11 @@ 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<reqwest::Error> 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),
 }
 
 /// Selectively skip parts or the entire auth middleware.
@@ -361,7 +361,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<bool, CredentialsFromUrlError> {
         self.credentials_cache.store_credentials_from_url(url)
     }
 
@@ -962,7 +965,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());

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<RegistryClient, ClientBuildError> {
-        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<RegistryClient, ClientBuildError> {
+        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(),
-        }
+        })
     }
 }