Repository navigation
Conversation
astral-automations-bot
Bot
added
enhancement
c32776f to
a01085e
Compare
a01085e to
a8c1184
Compare
zaniebot
added
priority:3
841dc59 to
53e2890
Compare
Merging this PR will not alter performance
|
04dcf26 to
688a689
Compare
zaniebot
added
the
build:skip-release
688a689 to
7cbbadf
Compare
![astral-automations-bot[bot]](https://avatars.githubusercontent.com/in/4307167?s=60&v=4){kind=small}
| fn native_lock_directory() -> Result<PathBuf, Error> { | ||
| Ok(StateStore::from_settings(None) | ||
| .map_err(Error::NativeLockDirectory)? | ||
| .bucket(StateBucket::Credentials) | ||
| .join("native")) |
There was a problem hiding this comment.
[P2] Keep native lock paths independent of XDG overrides
On Unix, processes with different XDG_DATA_HOME values can share the same native keyring, but StateStore::from_settings(None) gives them different lock directories. Both can then read the same realm JSON, add different accounts, and overwrite each other's updates. Ignoring UV_CREDENTIALS_DIR does not prevent this. Derive the lock location from the native store's user scope independently of configurable state directories, and cover this with a cross-process test.
| let json = Zeroizing::new( | ||
| serde_json::to_string(&credentials).map_err(Error::SerializeStoredCredentials)?, | ||
| ); | ||
| entry.set_password(&json).await?; |
There was a problem hiding this comment.
[P2] Retain realm locks until native I/O completes
On macOS, Entry::set_password performs its write through spawn_blocking, which continues if the awaiting future is cancelled. Cancelling a legacy-migration request here drops its realm guard while the native write can still finish. Another process can acquire the lock, update the collection, and then have that update overwritten by the detached write. Keep lock ownership alive until the platform operation completes, including cancellation paths; the same requirement applies to collection removal.
| (Some(cli), None) => Some(cli), | ||
| (None, Some(url)) => Some(url.to_string()), | ||
| (None, None) if matches!(&backend, AuthBackend::System(_)) => None, | ||
| (None, None) => Some("__token__".to_string()), |
There was a problem hiding this comment.
[P2] Preserve lookup of legacy token accounts
A token saved by an older uv version resides under the legacy service with username __token__. Passing None here makes native::fetch skip its entire legacy lookup loop, so the previously working uv auth token example.com now fails until the user supplies --username __token__ or logs in again. Preserve the legacy token fallback when username inference finds no credential, while retaining errors for ambiguous new-format accounts.
| match self.cache().get_stored(url, username) { | ||
| Ok(credentials) => Ok(credentials), | ||
| Err(_) if self.preview.is_enabled(PreviewFeature::NativeAuth) => Err( | ||
| Self::native_store_error(crate::keyring::Error::AmbiguousUsername(url.clone())), | ||
| ), |
There was a problem hiding this comment.
[P2] Defer cached ambiguity errors until authentication is needed
A successful authenticated request caches every credential in its realm. If another path has multiple saved accounts, this branch then rejects an authenticate = auto request before trying anonymous access—even when that endpoint is public. The same request succeeds when issued before the unrelated authenticated request. Treat ambiguity during eager cache lookup as a cache miss, and surface it when a challenge or authenticate = always requires credential selection.
| exit_code: 2 (failure) | ||
| ----- stderr ----- | ||
| error: Failed to fetch credentials for native-prefix-user@https://native-prefix.example.com/apiv1 |
There was a problem hiding this comment.
[P2] Match the token diagnostic in the new snapshots
This expectation omits the backticks that commands/auth/token.rs includes around display_url; the default snapshot filters do not remove them. The same mismatch appears in native_auth_multiple_users and native_auth_logout_is_service_scoped, so these assertions cannot match the emitted diagnostic. Regenerate or correct these expectations while retaining the existing diagnostic formatting.
4d7cdb0 to
a426b84
Compare
| cache_scope: if path_sensitive_store_enabled { | ||
| CredentialsCacheScope::FetchOnly | ||
| } else { | ||
| CredentialsCacheScope::Realm |
There was a problem hiding this comment.
[P2] Retain subprocess credentials as a realm fallback
When a plaintext store exists—even an empty file after logout—this disables realm caching for every subprocess result. For an authenticate = always index at /private/simple, a successful username-inferred lookup is cached under FetchUrl::Index. A protected wheel at /packages/... uses FetchUrl::Realm, finds no propagated credentials, and skips subprocess lookup without a username. This previously succeeded through the realm cache. Retain successfully authenticated subprocess credentials as a fallback consulted after path-scoped stores.
| @@ -0,0 +1,113 @@ | |||
| #![cfg(any(target_os = "macos", target_os = "windows"))] | |||
There was a problem hiding this comment.
[P2] Feature-gate the native keyring integration tests
These platform gates make ordinary cargo test -p uv-auth access the real OS keyring on macOS and Windows. On machines without an unlocked keychain or suitable logon session, the tests can prompt or fail. Existing native tests in uv-keyring and uv/tests/it/auth.rs require the opt-in native-auth feature. Apply that convention to native_cache.rs, native_macos.rs, and native_windows.rs as well.
| let store_credentials = if let Some(text_store) = text_store { | ||
| debug!("Checking text store for credentials for `{url}`"); | ||
| 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}"); | ||
| let snapshot = StoredCredentials::from(text_store.realm_credentials(&realm)); | ||
| match CredentialsCache::select_stored(&snapshot, url, &username) { |
There was a problem hiding this comment.
[P3] Reuse plaintext realm snapshots across lookups
realm_credentials scans the entire plaintext store and clones every service and credential in the matching realm before selection. Username-bearing requests repeatedly reach this branch because complete_request_with_request_credentials never consults the stored snapshot cache, even after successful authentication populated it. The plaintext store is immutable after loading, so these requests unnecessarily rebuild the same snapshot. Reuse the shared realm snapshot and perform per-request matching against it.
| for (url, credentials) in &entries { | ||
| let actual = provider.fetch(url, credentials.username()).await?; | ||
| if actual != Some(credentials.clone()) { | ||
| return Err(std::io::Error::other(format!( | ||
| "unexpected credentials returned for {url}" |
There was a problem hiding this comment.
[P3] Separate independent Windows credential scenarios
The shared entries list combines bulk enumeration, account-name collisions, signed URL identities, and corrupt-entry tolerance, followed by a deletion check. Running these independent scenarios through one fetch loop couples their fixtures and prevents later checks from running when an earlier case fails. Keep the bulk population in the enumeration test, and move the identity and corruption regressions into separately named tests with explicit store/fetch assertions, following the repository's integration-test guidance.
710b6c5 to
5d65ecd
Compare
This branch was successfully deployed
3 participants
Store native credentials for a realm in a JSON entry that retains each account’s service URL. Share realm snapshots across requests and migrate legacy entries into this representation, so credentials remain scoped to the matching service and account.
Depends on #2591 for authentication URL prefix precedence.