Bail when trying to attach ambiguous credentials instead of picking an arbitrary one (#16983)

This PR makes `TextCredentialStore.get_credentials` fail on ambiguous
matches, instead of returning the first "best" match while iterating
over all credentials.

Added a unit test, plus tested via:

```
❯ jq -n '{uri: "https://foo"}' | cargo -q run auth helper --protocol=bazel get
error: Multiple credentials found for URL 'https://foo/', specify which username to use
```
(the above needed two username/password combos registered for
`https://foo` using `uv auth login https://foo`)

crates/uv-auth/src/middleware.rs

@@ -803,14 +803,18 @@ impl AuthMiddleware {
         // Text credential store support.
         } else if let Some(credentials) = self.text_store.get().await.and_then(|text_store| {
             debug!("Checking text store for credentials for {url}");
-            text_store
-                .get_credentials(
-                    url,
-                    credentials
-                        .as_ref()
-                        .and_then(|credentials| credentials.username()),
-                )
-                .cloned()
+            match text_store.get_credentials(
+                url,
+                credentials
+                    .as_ref()
+                    .and_then(|credentials| credentials.username()),
+            ) {
+                Ok(credentials) => credentials.cloned(),
+                Err(err) => {
+                    debug!("Failed to get credentials from text store: {err}");
+                    None
+                }
+            }
         }) {
             debug!("Found credentials in plaintext store for {url}");
             Some(credentials)

crates/uv-auth/src/store.rs

@@ -122,6 +122,12 @@ pub enum BearerAuthError {
     UnexpectedPassword,
 }
 
+#[derive(Debug, Error, PartialEq)]
+pub enum LookupError {
+    #[error("Multiple credentials found for URL '{0}', specify which username to use")]
+    AmbiguousUsername(DisplaySafeUrl),
+}
+
 /// A single credential entry in a TOML credentials file.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(try_from = "TomlCredentialWire", into = "TomlCredentialWire")]
@@ -334,7 +340,7 @@ impl TextCredentialStore {
         &self,
         url: &DisplaySafeUrl,
         username: Option<&str>,
-    ) -> Option<&Credentials> {
+    ) -> Result<Option<&Credentials>, LookupError> {
         let request_realm = Realm::from(url);
 
         // Perform an exact lookup first
@@ -345,7 +351,7 @@ impl TextCredentialStore {
                 url_service.clone(),
                 Username::from(username.map(str::to_string)),
             )) {
-                return Some(credential);
+                return Ok(Some(credential));
             }
         }
 
@@ -376,15 +382,17 @@ impl TextCredentialStore {
             let specificity = service.url().path().len();
             if best.is_none_or(|(best_specificity, _, _)| specificity > best_specificity) {
                 best = Some((specificity, service, credential));
+            } else if best.is_some_and(|(best_specificity, _, _)| specificity == best_specificity) {
+                return Err(LookupError::AmbiguousUsername(url.clone()));
             }
         }
 
         // Return the most specific match
         if let Some((_, _, credential)) = best {
-            return Some(credential);
+            return Ok(Some(credential));
         }
 
-        None
+        Ok(None)
     }
 
     /// Store credentials for a given service.
@@ -455,10 +463,10 @@ mod tests {
         let service = Service::from_str("https://example.com").unwrap();
         store.insert(service.clone(), credentials.clone());
         let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
-        assert!(store.get_credentials(&url, None).is_some());
+        assert!(store.get_credentials(&url, None).unwrap().is_some());
 
         let url = DisplaySafeUrl::parse("https://example.com/path").unwrap();
-        let retrieved = store.get_credentials(&url, None).unwrap();
+        let retrieved = store.get_credentials(&url, None).unwrap().unwrap();
         assert_eq!(retrieved.username(), Some("user"));
         assert_eq!(retrieved.password(), Some("pass"));
 
@@ -468,7 +476,7 @@ mod tests {
                 .is_some()
         );
         let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
-        assert!(store.get_credentials(&url, None).is_none());
+        assert!(store.get_credentials(&url, None).unwrap().is_none());
     }
 
     #[tokio::test]
@@ -494,12 +502,12 @@ password = "pass2"
         let store = TextCredentialStore::from_file(temp_file.path()).unwrap();
 
         let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
-        assert!(store.get_credentials(&url, None).is_some());
+        assert!(store.get_credentials(&url, None).unwrap().is_some());
         let url = DisplaySafeUrl::parse("https://test.org/").unwrap();
-        assert!(store.get_credentials(&url, None).is_some());
+        assert!(store.get_credentials(&url, None).unwrap().is_some());
 
         let url = DisplaySafeUrl::parse("https://example.com").unwrap();
-        let cred = store.get_credentials(&url, None).unwrap();
+        let cred = store.get_credentials(&url, None).unwrap().unwrap();
         assert_eq!(cred.username(), Some("testuser"));
         assert_eq!(cred.password(), Some("testpass"));
 
@@ -535,7 +543,7 @@ password = "pass2"
 
         for url_str in matching_urls {
             let url = DisplaySafeUrl::parse(url_str).unwrap();
-            let cred = store.get_credentials(&url, None);
+            let cred = store.get_credentials(&url, None).unwrap();
             assert!(cred.is_some(), "Failed to match URL with prefix: {url_str}");
         }
 
@@ -548,7 +556,7 @@ password = "pass2"
 
         for url_str in non_matching_urls {
             let url = DisplaySafeUrl::parse(url_str).unwrap();
-            let cred = store.get_credentials(&url, None);
+            let cred = store.get_credentials(&url, None).unwrap();
             assert!(cred.is_none(), "Should not match non-prefix URL: {url_str}");
         }
     }
@@ -572,7 +580,7 @@ password = "pass2"
 
         for url_str in matching_urls {
             let url = DisplaySafeUrl::parse(url_str).unwrap();
-            let cred = store.get_credentials(&url, None);
+            let cred = store.get_credentials(&url, None).unwrap();
             assert!(
                 cred.is_some(),
                 "Failed to match URL in same realm: {url_str}"
@@ -588,7 +596,7 @@ password = "pass2"
 
         for url_str in non_matching_urls {
             let url = DisplaySafeUrl::parse(url_str).unwrap();
-            let cred = store.get_credentials(&url, None);
+            let cred = store.get_credentials(&url, None).unwrap();
             assert!(
                 cred.is_none(),
                 "Should not match URL in different realm: {url_str}"
@@ -612,12 +620,12 @@ password = "pass2"
 
         // Should match the most specific prefix
         let url = DisplaySafeUrl::parse("https://example.com/api/v1/users").unwrap();
-        let cred = store.get_credentials(&url, None).unwrap();
+        let cred = store.get_credentials(&url, None).unwrap().unwrap();
         assert_eq!(cred.username(), Some("specific"));
 
         // Should match the general prefix for non-specific paths
         let url = DisplaySafeUrl::parse("https://example.com/api/v2").unwrap();
-        let cred = store.get_credentials(&url, None).unwrap();
+        let cred = store.get_credentials(&url, None).unwrap().unwrap();
         assert_eq!(cred.username(), Some("general"));
     }
 
@@ -630,17 +638,17 @@ password = "pass2"
         store.insert(service.clone(), user1_creds.clone());
 
         // Should return credentials when username matches
-        let result = store.get_credentials(&url, Some("user1"));
+        let result = store.get_credentials(&url, Some("user1")).unwrap();
         assert!(result.is_some());
         assert_eq!(result.unwrap().username(), Some("user1"));
         assert_eq!(result.unwrap().password(), Some("pass1"));
 
         // Should not return credentials when username doesn't match
-        let result = store.get_credentials(&url, Some("user2"));
+        let result = store.get_credentials(&url, Some("user2")).unwrap();
         assert!(result.is_none());
 
         // Should return credentials when no username is specified
-        let result = store.get_credentials(&url, None);
+        let result = store.get_credentials(&url, None).unwrap();
         assert!(result.is_some());
         assert_eq!(result.unwrap().username(), Some("user1"));
     }
@@ -668,12 +676,12 @@ password = "pass2"
         let url = DisplaySafeUrl::parse("https://example.com/api/v1/users").unwrap();
 
         // Should match specific credentials when username matches
-        let result = store.get_credentials(&url, Some("specific_user"));
+        let result = store.get_credentials(&url, Some("specific_user")).unwrap();
         assert!(result.is_some());
         assert_eq!(result.unwrap().username(), Some("specific_user"));
 
         // Should match the general credentials when requesting general_user (falls back to less specific prefix)
-        let result = store.get_credentials(&url, Some("general_user"));
+        let result = store.get_credentials(&url, Some("general_user")).unwrap();
         assert!(
             result.is_some(),
             "Should match general_user from less specific prefix"
@@ -681,8 +689,37 @@ password = "pass2"
         assert_eq!(result.unwrap().username(), Some("general_user"));
 
         // Should match most specific when no username specified
-        let result = store.get_credentials(&url, None);
+        let result = store.get_credentials(&url, None).unwrap();
         assert!(result.is_some());
         assert_eq!(result.unwrap().username(), Some("specific_user"));
     }
+
+    #[test]
+    fn test_ambiguous_username_error() {
+        let mut store = TextCredentialStore::default();
+
+        // Add two credentials for the same service with different usernames
+        let service = Service::from_str("https://example.com/api").unwrap();
+        let user1_creds = Credentials::basic(Some("user1".to_string()), Some("pass1".to_string()));
+        let user2_creds = Credentials::basic(Some("user2".to_string()), Some("pass2".to_string()));
+
+        store.insert(service.clone(), user1_creds);
+        store.insert(service.clone(), user2_creds);
+
+        let url = DisplaySafeUrl::parse("https://example.com/api/v1").unwrap();
+
+        // When no username is specified, should return an error because there are multiple matches with same specificity
+        let result = store.get_credentials(&url, None);
+        assert!(result.is_err());
+        assert_eq!(result, Err(LookupError::AmbiguousUsername(url.clone())));
+
+        // When a specific username is provided, should return the correct credentials
+        let result = store.get_credentials(&url, Some("user1")).unwrap();
+        assert!(result.is_some());
+        assert_eq!(result.unwrap().username(), Some("user1"));
+
+        let result = store.get_credentials(&url, Some("user2")).unwrap();
+        assert!(result.is_some());
+        assert_eq!(result.unwrap().username(), Some("user2"));
+    }
 }

crates/uv/src/commands/auth/helper.rs

@@ -103,7 +103,7 @@ async fn credentials_for_url(
     let backend = AuthBackend::from_settings(preview).await?;
     let credentials = match &backend {
         AuthBackend::System(provider) => provider.fetch(url, username).await,
-        AuthBackend::TextStore(store, _lock) => store.get_credentials(url, username).cloned(),
+        AuthBackend::TextStore(store, _lock) => store.get_credentials(url, username)?.cloned(),
     };
     Ok(credentials)
 }

crates/uv/src/commands/auth/token.rs

@@ -66,7 +66,7 @@ pub(crate) async fn token(
             .await
             .ok_or_else(|| anyhow::anyhow!("Failed to fetch credentials for {display_url}"))?,
         AuthBackend::TextStore(store, _lock) => store
-            .get_credentials(url, Some(&username))
+            .get_credentials(url, Some(&username))?
             .cloned()
             .ok_or_else(|| anyhow::anyhow!("Failed to fetch credentials for {display_url}"))?,
     };