diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 004051f5b19..0bd63f39ece 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1214,6 +1214,20 @@ jobs: # Serial: windows_resolver_tests mutate process-global env # (BUZZ_SHELL/GIT_BASH/SystemRoot) that SharedState::new reads. run: cargo test -p buzz-dev-mcp --target $env:TARGET -- --test-threads=1 + - name: Test (buzz-agent auth coordinator) + # The auth coordinator single-flights on an OS advisory lock, which is + # LockFileEx on Windows; this integration suite drives real second + # processes on the same lock file, so it only exercises the Windows + # lock runtime if it runs ON Windows. Every other job compiles it but + # never executes it. Tests exercised on Windows: lock serialization + # (two coordinators race for the same key), cooldown sidecar sharing + # across processes, attempt-sidecar adoption (UserInitiated waiter + # adopts a predecessor's denial), and the in-process single-flight for + # same-key coalescing. Tests that are UNIX-ONLY and NOT executed here: + # crash-release (flock drop on SIGKILL, guarded by #[cfg(unix)]) and + # cross-process cache success/race (on-disk token handoff, also + # #[cfg(unix)]). + run: cargo test -p buzz-agent --target $env:TARGET --test databricks_auth_coordinator # Smoke-test the new host-prereq contract: Git for Windows (which provides # bash) is available on the runner, a shell command round-trips, and bash # does NOT resolve from System32 (so WSL's launcher is never picked up). diff --git a/Cargo.lock b/Cargo.lock index 9544a63b899..19f965d37bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -894,6 +894,7 @@ dependencies = [ "axum", "base64 0.22.1", "dirs", + "fs2", "getrandom 0.4.3", "hex", "nix 0.31.3", @@ -2969,6 +2970,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fs_extra" version = "1.3.0" diff --git a/Justfile b/Justfile index d7cfcdb1532..c81adb2381b 100644 --- a/Justfile +++ b/Justfile @@ -391,14 +391,21 @@ test-unit: # because nothing in CI runs `cargo test --workspace` — workspace # membership alone buys clippy/check, not a single executed test. cargo nextest run -p buzz-backend-kubernetes - # buzz-agent model-capabilities corpus: the Rust half of the - # cross-language drift guard. `model_capabilities.rs` embeds - # scripts/model-capabilities.json + scripts/normative-corpus.json via - # include_str! and replays the full locked corpus as pure in-process tests (no - # infra). Enumerated explicitly because nothing in CI runs - # `cargo test --workspace`; without this step a manifest edit that - # diverges Rust from the corpus ships green. - cargo nextest run -p buzz-agent --lib + # buzz-agent: two infra-free concerns run together by executing the + # whole crate (lib + integration tests), because nothing in CI runs + # `cargo test --workspace`, so without this stanza neither the crate's + # library tests nor its integration tests execute remotely. + # * model-capabilities corpus (lib): the Rust half of the + # cross-language drift guard. `model_capabilities.rs` embeds + # scripts/model-capabilities.json + scripts/normative-corpus.json via + # include_str! and replays the full locked corpus as pure in-process + # tests; without it a manifest edit that diverges Rust from the + # corpus ships green. + # * OAuth auth coordinator (lib concurrency matrix + databricks + # integration tests): lock single-flight, cooldown, cross-process + # crash recovery — infra-free via a stub OIDC provider and an + # injected browser opener, no network or Postgres. + cargo nextest run -p buzz-agent # Admin API auth-boundary tests (api::admin in buzz-relay): the NIP-98 # duplicate-tag rejections, the Host/Origin replay-ordering causal pair, # the admin.localhost origin/advertisement/canonical-URL pins, and the diff --git a/crates/buzz-agent/Cargo.toml b/crates/buzz-agent/Cargo.toml index fabf75754e1..b60644bb7b6 100644 --- a/crates/buzz-agent/Cargo.toml +++ b/crates/buzz-agent/Cargo.toml @@ -24,6 +24,24 @@ path = "src/main.rs" name = "fake-mcp" path = "tests/bin/fake_mcp.rs" +# Test-only lock holder: a real second process that takes the coordinator's +# cross-process advisory lock, so the auth tests can prove genuine +# inter-process single-flight and crash-release rather than same-process +# handles. Tiny; only used by the databricks auth integration tests. +[[bin]] +name = "lock-holder" +path = "tests/bin/lock_holder.rs" + +# Test-only auth worker: a real second process that runs the PUBLIC auth +# coordinator API (`acquire_with_intent`) with a scripted browser opener and a +# shared temp cache, so the auth tests can prove the cross-process single-flight +# contract end-to-end — durable cooldown sharing and one-grant/one-cache races +# across a genuine process boundary, not two in-process handles. Only used by +# the databricks auth integration tests. +[[bin]] +name = "auth-worker" +path = "tests/bin/auth_worker.rs" + [dependencies] tokio = { workspace = true, features = ["rt-multi-thread", "macros", "io-std", "io-util", "sync", "process", "time", "net"] } serde = { workspace = true } @@ -45,6 +63,11 @@ url = { workspace = true } urlencoding = "2" webbrowser = "1" dirs = "6" +# Cross-process advisory file lock (flock on Unix, LockFileEx on Windows) for +# the auth coordinator's single-flight. Kept off std's `File::try_lock` so the +# crate stays buildable on the repo's declared 1.88 MSRV (those std APIs are +# 1.89+). +fs2 = "0.4" [target.'cfg(unix)'.dependencies] nix = { version = "0.31", default-features = false, features = ["signal", "process"] } diff --git a/crates/buzz-agent/src/auth.rs b/crates/buzz-agent/src/auth.rs index 7ebabccbbbd..0ae34318c27 100644 --- a/crates/buzz-agent/src/auth.rs +++ b/crates/buzz-agent/src/auth.rs @@ -15,19 +15,21 @@ //! captures the redirect, and exchanges the code for a token. Subsequent //! calls hit the cache and silently refresh when expired. +use std::collections::HashMap; use std::fs; use std::io::{self, Write}; use std::path::{Path, PathBuf}; -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use async_trait::async_trait; use base64::Engine; +use fs2::FileExt; use reqwest::Client; use serde::{Deserialize, Serialize}; use serde_json::Value; use sha2::Digest; -use tokio::sync::Mutex; +use tokio::sync::{watch, Mutex}; use crate::types::AgentError; @@ -39,6 +41,219 @@ const TOKEN_REFRESH_LEEWAY: Duration = Duration::from_secs(60); /// We match: any longer and the user has gone to lunch. const BROWSER_AUTH_TIMEOUT: Duration = Duration::from_secs(60); +/// Per-request network timeout for every OAuth HTTP call (discovery, refresh +/// grant, code exchange). Without this, a hung provider connection would stall +/// the caller — and, worse, stall every same-key caller waiting on the +/// cross-process lock this holder owns. +const HTTP_REQUEST_TIMEOUT: Duration = Duration::from_secs(30); + +/// Longest an in-flight auth attempt can legitimately run: cold discovery +/// (`30s`) + browser wait (`60s`) + code exchange (`30s`), plus a failed +/// refresh (`30s`) ahead of the browser. Rounded to `150s`. A waiter derives +/// its lock-wait bound from this so it never times out ahead of a healthy +/// holder. +const AUTH_ATTEMPT_DEADLINE: Duration = Duration::from_secs(150); + +/// How long a same-key caller waits to acquire the cross-process lock before +/// giving up with [`AuthError::LockTimeout`]. Deliberately longer than +/// [`AUTH_ATTEMPT_DEADLINE`] so a waiter outlasts any legitimate holder rather +/// than timing out mid-flow. +const LOCK_WAIT_TIMEOUT: Duration = Duration::from_secs(165); + +/// Poll interval for deadline-aware lock acquisition. `try_lock` is +/// non-blocking, so we sleep between attempts rather than blocking a worker. +const LOCK_POLL_INTERVAL: Duration = Duration::from_millis(100); + +/// How long a failed interactive (browser) attempt suppresses automatic +/// re-launch for the same key. Long enough that a spurned dropdown does not +/// re-pop a browser on the next debounced refresh, short enough that a user +/// who fixes the problem is not locked out. +const COOLDOWN_DURATION: Duration = Duration::from_secs(300); + +/// Why an auth acquisition wants a token, which decides whether it may open a +/// browser and whether it honors a cooldown. +/// +/// - [`Auto`](Self::Auto): passive Desktop discovery (create/edit/defaults/ +/// onboarding). May open a browser, but honors an unexpired cooldown and +/// returns its recorded outcome instead of re-launching. +/// - [`UserInitiated`](Self::UserInitiated): an explicit human action — the +/// saved-agent model picker or `buzz-agent auth databricks`. May open a +/// browser and *bypasses* the cooldown (the user asked for it now). +/// - [`Headless`](Self::Headless): managed-runtime inference and provider +/// preflight. Never opens a browser; may consume another attempt's cached +/// success but never becomes the initiator. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum AuthIntent { + Auto, + UserInitiated, + Headless, +} + +impl AuthIntent { + /// `true` for the intents permitted to open a browser. + fn may_open_browser(self) -> bool { + matches!(self, Self::Auto | Self::UserInitiated) + } + + /// `true` for the one intent that honors a recorded cooldown on read. + fn honors_cooldown(self) -> bool { + matches!(self, Self::Auto) + } + + /// Stable discriminant for the cross-process attempt sidecar. A queued + /// caller adopts a completed attempt's failure only when the recorded + /// intent matches its own — the durable mirror of the in-process + /// [`INFLIGHT`] registry's `(path, intent)` keying, so a `UserInitiated` + /// caller never inherits an `Auto` attempt's suppressed result across + /// processes any more than it does within one. + fn as_str(self) -> &'static str { + match self { + Self::Auto => "auto", + Self::UserInitiated => "user_initiated", + Self::Headless => "headless", + } + } +} + +/// Typed result of an auth acquisition. `Ok` carries the bearer; the error +/// arm classifies *why* no token was produced so callers (and, in Phase 2, the +/// Tauri boundary) can branch on a stable code instead of matching display +/// text. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AuthError { + /// No cached token, no refresh grant, and the caller may not open a + /// browser (`Headless`). + NoCredential, + /// The user (or provider) rejected the browser authorization. + Denied, + /// The browser flow was not completed within [`BROWSER_AUTH_TIMEOUT`]. + TimedOut, + /// Every browser-launch strategy failed, so the flow never started. + BrowserOpenFailed, + /// An OAuth network call (discovery/refresh/exchange) could not reach the + /// provider or timed out. + NetworkUnavailable, + /// A refresh-token grant was rejected (dead/rotated refresh token) and the + /// caller may not fall back to a browser. + RefreshRejected, + /// The authorization-code exchange itself was rejected by the token + /// endpoint (distinct from a refresh rejection). + ExchangeFailed, + /// Could not acquire the cross-process auth lock within + /// [`LOCK_WAIT_TIMEOUT`]. + LockTimeout, +} + +impl AuthError { + /// Stable machine-readable code. Phase 2 serializes this across the Tauri + /// boundary (the `project_git_merge_error` `{code, message}` precedent) so + /// the Desktop formatter switches on the code, never on display text. + pub fn code(&self) -> &'static str { + match self { + Self::NoCredential => "no_credential", + Self::Denied => "denied", + Self::TimedOut => "timed_out", + Self::BrowserOpenFailed => "browser_open_failed", + Self::NetworkUnavailable => "network_unavailable", + Self::RefreshRejected => "refresh_rejected", + Self::ExchangeFailed => "exchange_failed", + Self::LockTimeout => "lock_timeout", + } + } + + /// `true` for the browser-attempt outcomes worth recording in the cooldown + /// sidecar — the failures that would otherwise re-pop a browser on the + /// next automatic attempt. Non-browser failures (no credential, refresh + /// rejection, lock timeout, network) are not recorded. + fn is_cooldown_worthy(&self) -> bool { + matches!( + self, + Self::Denied | Self::TimedOut | Self::BrowserOpenFailed | Self::ExchangeFailed + ) + } + + /// Reconstruct a recorded outcome from its [`code`](Self::code). The + /// cooldown-worthy variants always round-trip; `RefreshRejected` and + /// `NoCredential` are also reconstructed for the cross-process attempt + /// adoption path. Any other code (a forward-compat sidecar written by a + /// newer buzz-agent) yields `None`, treated as "no active record" rather + /// than a hard failure. + fn from_code(code: &str) -> Option { + match code { + "denied" => Some(Self::Denied), + "timed_out" => Some(Self::TimedOut), + "browser_open_failed" => Some(Self::BrowserOpenFailed), + "exchange_failed" => Some(Self::ExchangeFailed), + "refresh_rejected" => Some(Self::RefreshRejected), + "no_credential" => Some(Self::NoCredential), + _ => None, + } + } + + fn message(&self) -> String { + match self { + Self::NoCredential => { + "no cached Databricks token; run `buzz-agent auth databricks` first".into() + } + Self::Denied => "Databricks authorization was denied".into(), + Self::TimedOut => "Databricks browser authorization timed out".into(), + Self::BrowserOpenFailed => "could not open a browser for Databricks sign-in".into(), + Self::NetworkUnavailable => "could not reach Databricks to authenticate".into(), + Self::RefreshRejected => "Databricks rejected the refresh token; sign in again".into(), + Self::ExchangeFailed => "Databricks rejected the authorization code".into(), + Self::LockTimeout => "timed out waiting for a concurrent Databricks sign-in".into(), + } + } +} + +impl From for AgentError { + /// Map a typed auth failure onto the crate error the [`TokenSource`] trait + /// returns. Auth-decision failures become [`AgentError::LlmAuth`] so the + /// caller's retry loop stops instead of hammering a rejected credential; + /// purely infrastructural failures (network, lock contention) become + /// [`AgentError::Llm`], matching the pre-coordinator classification of a + /// discovery/network error. + fn from(e: AuthError) -> Self { + match e { + AuthError::NetworkUnavailable | AuthError::LockTimeout => AgentError::Llm(e.message()), + AuthError::NoCredential + | AuthError::Denied + | AuthError::TimedOut + | AuthError::BrowserOpenFailed + | AuthError::RefreshRejected + | AuthError::ExchangeFailed => AgentError::LlmAuth(e.message()), + } + } +} + +/// Opens a URL for the interactive browser step. Injected so the PKCE +/// continuation (callback listener, verifier, timeout) stays alive across the +/// launch: the coordinator calls this *while* the localhost listener is +/// bound, so a launch failure never leaves a returned URL pointing at a torn +/// down listener. Desktop (Phase 2) supplies the Tauri opener; the CLI uses +/// [`DefaultBrowserOpener`], which prints the URL and opens the system +/// browser. +pub trait BrowserOpener: Send + Sync { + /// Attempt to present `url` to the user. Returning `Err` means every + /// launch strategy for this opener failed; the coordinator then reports + /// [`AuthError::BrowserOpenFailed`] without waiting on a listener nobody + /// will reach. + fn open(&self, url: &str) -> Result<(), String>; +} + +/// Default opener: print the URL (so a user on a headless box can copy it) +/// and open the system browser. Printing is itself a launch strategy, so this +/// never reports failure — the URL is always visible to the waiting user. +pub struct DefaultBrowserOpener; + +impl BrowserOpener for DefaultBrowserOpener { + fn open(&self, url: &str) -> Result<(), String> { + eprintln!("Opening browser for authentication. If it doesn't open, visit:\n {url}"); + let _ = webbrowser::open(url); + Ok(()) + } +} + /// Asynchronous source of a bearer token. The [`Llm`] calls this per /// request, so impls are expected to be cheap on the cache-hit path. #[async_trait] @@ -124,6 +339,25 @@ struct OidcEndpoints { token_endpoint: String, } +/// Typed result of a refresh-token grant, so the coordinator can separate an +/// actual credential rejection from a transient fault. +/// +/// - [`Refreshed`](Self::Refreshed): a fresh token — success. +/// - [`Rejected`](Self::Rejected): the token endpoint returned an +/// `invalid_grant` error (dead/rotated refresh token). This is the only +/// outcome that becomes [`AuthError::RefreshRejected`] for `Headless` or +/// drives a browser fallback for interactive intents. +/// - [`Network`](Self::Network): transport error, timeout, 5xx, any 4xx that +/// is not `invalid_grant` (e.g. `invalid_request`, `invalid_client`, 429), +/// an unparseable error body, or an undecodable/malformed success body — +/// infrastructural or misconfiguration, never a credential decision, so it +/// surfaces as [`AuthError::NetworkUnavailable`] and never pops a browser. +enum RefreshOutcome { + Refreshed(CachedToken), + Rejected, + Network, +} + /// PKCE OAuth token source with on-disk refresh cache. /// /// First call: @@ -137,27 +371,93 @@ pub struct PkceOAuthTokenSource { cfg: PkceOAuthConfig, http: Client, cache_path: PathBuf, - /// Single-flight guard: only one refresh/browser flow at a time, even - /// if many tool calls land concurrently. + /// Injected browser launcher, called inside [`browser_pkce_flow`] while the + /// localhost listener is live. Production uses [`DefaultBrowserOpener`]; + /// Phase 2 supplies the Tauri opener. + opener: Arc, + /// In-memory single-flight *and* fast-path cache. The cross-process file + /// lock serializes slow-path work; this cell keeps the fast path off disk + /// during a turn and off the lock entirely. state: Mutex>, } impl PkceOAuthTokenSource { + /// Construct with the default browser opener (prints the URL and opens the + /// system browser). This is the signature every production call site uses. pub fn new(cfg: PkceOAuthConfig) -> Result, AgentError> { + Self::new_with(cfg, Arc::new(DefaultBrowserOpener)) + } + + /// Construct with an injected [`BrowserOpener`]. Tests substitute a + /// recording/failing opener to exercise the browser branch without a real + /// window; Phase 2 Desktop injects the Tauri opener. + pub fn new_with( + cfg: PkceOAuthConfig, + opener: Arc, + ) -> Result, AgentError> { + Self::new_with_http_timeout(cfg, opener, HTTP_REQUEST_TIMEOUT) + } + + /// Construct with an injected opener *and* an explicit per-request HTTP + /// timeout. Only the refresh-timeout integration test passes the timeout + /// argument: it drives a hung token endpoint against a short bound so the + /// per-request timeout classification (`NetworkUnavailable`, never + /// `RefreshRejected`) is exercised in real time. A paused-clock test can't + /// do this — tokio auto-advances into the timer while the real loopback + /// discovery call is still in flight, tripping the timeout on the wrong + /// request. Every production and other-test path goes through + /// [`new`](Self::new) or [`new_with`](Self::new_with) at the default + /// [`HTTP_REQUEST_TIMEOUT`]. + pub fn new_with_http_timeout( + cfg: PkceOAuthConfig, + opener: Arc, + http_timeout: Duration, + ) -> Result, AgentError> { let cache_path = cache_path_for(&cfg)?; if let Some(parent) = cache_path.parent() { fs::create_dir_all(parent) .map_err(|e| AgentError::Llm(format!("oauth cache dir {parent:?}: {e}")))?; } + // Every OAuth HTTP call inherits this timeout so a hung provider can + // never stall the caller — nor the same-key callers waiting on the + // cross-process lock this holder owns. Construction is fallible, so a + // build failure propagates rather than silently falling back to an + // untimed client — an untimed client would restore exactly the + // unbounded-HTTP-under-lock failure the timeout exists to prevent. + let http = Client::builder() + .timeout(http_timeout) + .build() + .map_err(|e| AgentError::Llm(format!("oauth http client: {e}")))?; let initial = read_cache(&cache_path); Ok(Arc::new(Self { cfg, - http: Client::new(), + http, cache_path, + opener, state: Mutex::new(initial), })) } + /// Path of the cross-process advisory lock file guarding slow-path auth + /// for this cache key. Co-located with the cache so it shares the + /// per-key directory and `$HOME` override. + fn lock_path(&self) -> PathBuf { + append_ext(&self.cache_path, "lock") + } + + /// Path of the cooldown sidecar recording the last browser-attempt + /// failure for this cache key. + fn cooldown_path(&self) -> PathBuf { + append_ext(&self.cache_path, "cooldown") + } + + /// Path of the attempt sidecar recording the generation and outcome of the + /// last completed slow-path acquisition for this cache key. Drives the + /// cross-process single-flight of *failures* (see [`AttemptRecord`]). + fn attempt_path(&self) -> PathBuf { + append_ext(&self.cache_path, "attempt") + } + /// Discover authorization + token endpoints from the well-known URL. async fn endpoints(&self) -> Result { let v: Value = self @@ -194,236 +494,848 @@ impl PkceOAuthTokenSource { /// The cache holds both the access and refresh tokens, so the on-disk /// file is written owner-only (`0o600` on Unix) via an atomic /// inode-swapping rename — see [`write_private_cache`]. + /// + /// On non-Unix platforms the token is stored in-memory only: the + /// `write_private_cache` path creates files with default ACLs, which do + /// not enforce owner-only access. Disk persistence is intentionally + /// disabled until a Windows-specific owner-only DACL is implemented (see + /// the `create_private_temp_file` non-Unix branch). The cost is that each + /// process performs its own acquisition on non-Unix — cross-process + /// *success* handoff requires the shared on-disk cache, so processes + /// serialize through the lock but the loser repeats the flow rather than + /// reading the winner's token. Cross-process *failure* adoption still works + /// because it uses the attempt sidecar (no token bytes). Correct and + /// safe until owner-only DACL persistence exists. fn save(&self, state: &mut Option, token: CachedToken) -> Result<(), AgentError> { - let body = serde_json::to_vec_pretty(&token) - .map_err(|e| AgentError::Llm(format!("oauth cache serialize: {e}")))?; - write_private_cache(&self.cache_path, &body).map_err(|e| { - AgentError::Llm(format!("oauth cache write {:?}: {e}", self.cache_path)) - })?; + self.persist(&token)?; *state = Some(token); Ok(()) } + /// Write `token` to the on-disk cache. Split out of [`save`](Self::save) so + /// the 401 neutralization path can rewrite the disk layer without clobbering + /// a distinct in-memory entry. No-op on non-Unix (see [`save`](Self::save)). + fn persist(&self, token: &CachedToken) -> Result<(), AgentError> { + #[cfg(unix)] + { + let body = serde_json::to_vec_pretty(token) + .map_err(|e| AgentError::Llm(format!("oauth cache serialize: {e}")))?; + write_private_cache(&self.cache_path, &body).map_err(|e| { + AgentError::Llm(format!("oauth cache write {:?}: {e}", self.cache_path)) + })?; + } + #[cfg(not(unix))] + { + // Disk persistence disabled on non-Unix: owner-only file + // permissions require a DACL that is not yet implemented. + let _ = token; + } + Ok(()) + } + + /// Neutralize the matching rejected credential in B's own in-memory `state` + /// only — no disk I/O. The joiner matching-failure path calls this rather + /// than `expire_rejected`: the leader already ran the durable disk + /// invalidation under the cross-process file lock, and re-running disk + /// mutations from the lockless joiner can race with a concurrent process C + /// that persisted a valid replacement under the same lock (C's rename can + /// be overwritten by B's unfenced rename). + /// + /// Contract: only the access-token identity is checked — the refresh token + /// is left intact so callers reaching the recovery disk-read path below can + /// still attempt a fresh token exchange with the un-revoked refresh secret. + /// + /// Limitation: the joiner's match arm triggers on a same-digest leader + /// error regardless of error code (see `acquire`'s `Err` match arm). A + /// pre-lock failure (e.g. `LockTimeout`) with a matching rejected digest + /// therefore also reaches this helper, even though the leader never + /// durably invalidated the disk copy. In that case B's in-memory entry is + /// neutralized and B returns the shared error; the disk copy survives + /// intact. A subsequent plain `bearer()` (`rejected = None`) can re-read + /// the disk entry. This is a known bounded limitation: in-memory + /// neutralization is applied without a guarantee that the durable copy is + /// also gone. + fn expire_rejected_memory(&self, state: &mut Option, rejected: Option<&str>) { + let Some(rej) = rejected else { return }; + if let Some(tok) = state.as_mut() { + if tok.access_token == rej { + tok.expires_at = Some(0); + } + } + } + + /// Neutralize a cached token the caller just reported 401-rejected. + /// + /// A 401 means the cached access token is dead even though its local expiry + /// clock still looks fresh. [`cached_hit`](Self::cached_hit) and + /// [`usable_from_disk`](Self::usable_from_disk) already exclude it for a + /// caller carrying `rejected`, but a *later* plain `bearer()` + /// (`rejected = None`) trusts the clock and would serve it, and a freshly + /// constructed source would restore it from disk. Force it expired in both + /// layers so [`is_expired`] excludes it for every future caller and every + /// fresh process, while the refresh token — which was *not* rejected and + /// drives this very recovery — stays intact. Each layer is neutralized only + /// when its access token byte-equals `rejected`, so a sibling's + /// concurrently-written distinct replacement is preserved. + /// + /// Disk neutralization is a bounded three-stage process: on atomic-rewrite + /// failure (e.g. non-writable parent directory), the implementation falls + /// back to an in-place truncating overwrite of the existing file (no + /// parent-dir perms required), and finally to `remove_file`. If all three + /// fail the file survives; `cached_hit`'s `rejected`-aware filter protects + /// this caller's path, but a later plain `bearer()` could re-read the + /// unexpired file. That residual corner is outside the normal threat model + /// (owner actively hardening their own cache file to 0400 against their own + /// process). + fn expire_rejected(&self, state: &mut Option, rejected: Option<&str>) { + let Some(rej) = rejected else { return }; + // Neutralize the in-memory entry: force-expire so `is_expired` excludes + // it for every subsequent in-process caller, while the refresh token + // (which was not rejected) stays intact for the recovery below. + if let Some(tok) = state.as_mut() { + if tok.access_token == rej { + tok.expires_at = Some(0); + } + } + // Neutralize the on-disk copy. Prefer atomic rewrite via `persist()` + // (temp-file + rename, owner-only permissions). If the atomic rewrite + // fails (e.g. the parent directory denies temp-file creation), fall back + // to in-place truncating overwrite: `OpenOptions::write().truncate(true)` + // on the existing file does not require parent-directory write permission, + // only that the file itself is owner-writable (0600, which our cache files + // always are). As a last resort, attempt `remove_file`. The two-stage + // fallback covers the proven hostile case: a 0600 token file under a + // 0500 parent — the atomic path cannot create the temp file (EACCES), but + // the in-place write succeeds because the file's own mode permits it. + // Residual out of threat model: if the owner explicitly chmodded their own + // cache file to 0400 before this runs, the in-place write also fails and + // we fall through to `remove_file`; if that too fails, the file survives + // with `expires_at = 0` still NOT written — `cached_hit`'s + // `rejected`-aware filter still protects the calling 401-recovery path, + // but a later plain `bearer()` could re-adopt the file. That corner is + // not in the normal threat model (a user actively hardening their own + // cache file against their own process). + if let Some(mut disk) = read_cache(&self.cache_path) { + if disk.access_token == rej { + disk.expires_at = Some(0); + if self.persist(&disk).is_err() { + // Atomic rewrite failed. Try in-place truncating overwrite — + // does not need parent-dir write permission, only the file's + // own mode. + let inplace_ok = serde_json::to_vec_pretty(&disk).ok().is_some_and(|body| { + use std::io::Write as _; + fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(&self.cache_path) + .and_then(|mut f| f.write_all(&body)) + .is_ok() + }); + if !inplace_ok { + let _ = fs::remove_file(&self.cache_path); + } + } + } + } + } + /// Exchange a refresh token for a fresh access token. - async fn refresh( - &self, - endpoints: &OidcEndpoints, - refresh_token: &str, - ) -> Result { + /// + /// The outcome is typed so the caller can tell an actual credential + /// rejection apart from a transient fault. Only a token-endpoint rejection + /// of the grant itself (a 4xx `invalid_grant`-class response) is a dead + /// refresh token; a transport failure, timeout, 5xx, or an + /// undecodable/malformed response is infrastructural and must never be + /// mistaken for a credential decision (it would otherwise pop a browser or + /// return `RefreshRejected` when nothing was actually rejected). + async fn refresh(&self, endpoints: &OidcEndpoints, refresh_token: &str) -> RefreshOutcome { let params = [ ("grant_type", "refresh_token"), ("refresh_token", refresh_token), ("client_id", &self.cfg.client_id), ]; - let resp = self + let resp = match self .http .post(&endpoints.token_endpoint) .form(¶ms) .send() .await - .map_err(|e| AgentError::Llm(format!("oauth refresh: {e}")))?; - if !resp.status().is_success() { + { + Ok(resp) => resp, + // Transport error or the per-request timeout elapsed: no verdict + // from the provider, so this is infrastructural, not a rejection. + Err(e) => { + tracing::warn!(error = %e, "oauth refresh transport failure"); + return RefreshOutcome::Network; + } + }; + let status = resp.status(); + if !status.is_success() { let body = resp.text().await.unwrap_or_default(); - return Err(AgentError::Llm(format!("oauth refresh failed: {body}"))); + // Per RFC 6749 §5.2 only `error == "invalid_grant"` means the + // refresh token itself is dead (expired/revoked) — the one failure + // a browser sign-in can repair. Every other 4xx (`invalid_request`, + // `invalid_client`, `unsupported_grant_type`, `invalid_scope`, 408, + // 429, …), an unparseable error body, and all 5xx are + // infrastructural or misconfiguration: a browser can't fix them, so + // they stay in the non-credential bucket and surface as + // `NetworkUnavailable` without ever popping a browser. + if status.is_client_error() + && serde_json::from_str::(&body) + .ok() + .and_then(|v| v.get("error").and_then(Value::as_str).map(str::to_owned)) + .as_deref() + == Some("invalid_grant") + { + tracing::warn!(status = %status, body = %body, "oauth refresh grant rejected"); + return RefreshOutcome::Rejected; + } + tracing::warn!(status = %status, body = %body, "oauth refresh not repairable by browser"); + return RefreshOutcome::Network; + } + let v: Value = match resp.json().await { + Ok(v) => v, + Err(e) => { + tracing::warn!(error = %e, "oauth refresh response decode failure"); + return RefreshOutcome::Network; + } + }; + match token_from_response(&v, Some(refresh_token)) { + Ok(token) => RefreshOutcome::Refreshed(token), + Err(e) => { + tracing::warn!(error = %e, "oauth refresh response missing access_token"); + RefreshOutcome::Network + } } - let v: Value = resp - .json() - .await - .map_err(|e| AgentError::Llm(format!("oauth refresh json: {e}")))?; - token_from_response(&v, Some(refresh_token)) } - /// Run the full browser-mediated Authorization Code + PKCE flow. - /// Caller must hold a TTY/browser: this opens a window and blocks. + /// Run the full browser-mediated Authorization Code + PKCE flow and cache + /// the result. Routes through the coordinator as a [`UserInitiated`] + /// acquisition: it may open a browser, bypasses (and clears) any cooldown, + /// and single-flights with concurrent callers on the cross-process lock. A + /// still-valid cached token short-circuits to success without re-prompting. + /// + /// This is the no-rejected convenience: it trusts the local expiry clock, + /// so a not-yet-expired cached token is accepted. When the caller already + /// knows the cached bearer was rejected by the server (a 401), it must use + /// [`acquire_with_intent`](Self::acquire_with_intent) with `rejected` set + /// so the stale-but-fresh token can't short-circuit the sign-in. + /// + /// [`UserInitiated`]: AuthIntent::UserInitiated pub async fn interactive_login(&self) -> Result<(), AgentError> { - let endpoints = self.endpoints().await?; - let token = browser_pkce_flow(&self.http, &self.cfg, &endpoints).await?; - let mut state = self.state.lock().await; - self.save(&mut state, token)?; + self.acquire(AuthIntent::UserInitiated, None).await?; Ok(()) } -} -#[async_trait] -impl TokenSource for PkceOAuthTokenSource { - async fn bearer(&self) -> Result { - let mut state = self.state.lock().await; + /// Public entry for passive Desktop discovery and the saved-model picker + /// (Phase 2): acquire a bearer under an explicit [`AuthIntent`], returning + /// the typed [`AuthError`] so the caller can branch on a stable `code` + /// rather than display text. The [`TokenSource`] trait methods wrap this + /// and flatten the error into [`AgentError`]. + /// + /// `rejected` carries the exact access token the provider just 401'd, if + /// any. With `rejected = None` a locally-fresh cached token is a hit (the + /// normal discovery path). With `rejected = Some(t)` the expiry clock is + /// untrustworthy — the rejected token looked fresh — so a cached token + /// equal to `t` is *not* a hit: the acquisition refreshes, and for `Auto` + /// or `UserInitiated` falls through to a browser when the refresh grant is + /// dead. This is what lets the saved-picker recovery path say "this + /// locally-fresh bearer was just rejected — replace it" instead of + /// re-returning the dead token, which `refresh_now`'s hardcoded + /// [`Headless`](AuthIntent::Headless) can never escalate to a browser. + pub async fn acquire_with_intent( + &self, + intent: AuthIntent, + rejected: Option<&str>, + ) -> Result { + self.acquire(intent, rejected).await + } - // 1. In-memory cache hit, still fresh. + /// Return a usable cached bearer, applying the identity rule for a + /// 401-driven acquisition. + /// + /// `rejected = None` (normal): a not-yet-expired cached token is a hit. + /// `rejected = Some(t)`: the expiry clock is untrustworthy — the rejected + /// token looked locally fresh — so a hit requires the cached token to + /// *differ* from `t` (a sibling already replaced it) **and** still be + /// unexpired. Without the expiry check an expired sibling token B could be + /// returned as A's replacement, skipping the refresh the 401 demanded. + /// Checks the in-memory cell first, then re-reads disk (a sibling process + /// may have written a newer token) and adopts it into the cell on a hit. + fn cached_hit( + &self, + state: &mut Option, + rejected: Option<&str>, + ) -> Option { + let usable = + |tok: &CachedToken| !is_expired(tok) && rejected != Some(tok.access_token.as_str()); if let Some(tok) = state.as_ref() { - if !is_expired(tok) { - return Ok(tok.access_token.clone()); + if usable(tok) { + return Some(tok.access_token.clone()); } } - - // 2. Re-read disk — another process may have refreshed already. - if let Some(disk_tok) = read_cache(&self.cache_path) { - if !is_expired(&disk_tok) { - let bearer = disk_tok.access_token.clone(); - *state = Some(disk_tok); - return Ok(bearer); + if let Some(disk) = read_cache(&self.cache_path) { + if usable(&disk) { + let bearer = disk.access_token.clone(); + *state = Some(disk); + return Some(bearer); } } + None + } - // 3. Try refresh if we have a refresh token. Discover endpoints once - // here — deliberately hoisted above the refresh-token check so the - // browser flow at step 5 (which also needs them) reuses this call. - let endpoints = self.endpoints().await?; - let refresh = state.as_ref().and_then(|t| t.refresh_token.clone()); - if let Some(rt) = refresh { - match self.refresh(&endpoints, &rt).await { - Ok(fresh) => { - let bearer = fresh.access_token.clone(); - self.save(&mut state, fresh)?; - return Ok(bearer); - } - Err(e) => { - tracing::warn!(error = %e, "oauth refresh failed; falling back to browser flow"); + /// Lock-free variant of [`cached_hit`]'s disk branch: read the on-disk + /// cache and return its bearer if a sibling wrote a usable replacement for + /// `rejected`. Used by the joiner's shared-failure recheck, where every + /// waiter wakes at once — taking `self.state` (even with `try_lock`) would + /// either drop the replacement for `try_lock` losers or serialize the read + /// behind a new leader holding `state` across its browser flow. The + /// in-memory memo is intentionally not updated; the next real acquisition + /// re-reads and adopts under the lock. + fn usable_from_disk(&self, rejected: Option<&str>) -> Option { + let disk = read_cache(&self.cache_path)?; + (!is_expired(&disk) && rejected != Some(disk.access_token.as_str())) + .then_some(disk.access_token) + } + + /// Discover OIDC endpoints once per flow, memoizing into `slot` so the + /// refresh and browser branches share a single discovery call. A discovery + /// failure (unreachable URL or malformed document) maps to + /// [`AuthError::NetworkUnavailable`] — the infrastructural bucket, so the + /// caller's retry loop treats it as transient rather than as an auth + /// decision. + async fn discover<'a>( + &self, + slot: &'a mut Option, + ) -> Result<&'a OidcEndpoints, AuthError> { + if slot.is_none() { + let eps = self + .endpoints() + .await + .map_err(|_| AuthError::NetworkUnavailable)?; + *slot = Some(eps); + } + Ok(slot.as_ref().expect("endpoints just populated")) + } + + /// The single acquisition entry point behind every [`TokenSource`] method. + /// + /// `intent` decides browser and cooldown policy; `rejected` (`Some` only on + /// a 401-driven refresh) switches cache checks from clock-based to + /// identity-based. The fast path returns a usable cached token without + /// touching the lock or the network. Otherwise the slow path serializes + /// every same-key caller — in this process *and* across processes — on the + /// cross-process advisory lock, so concurrent dialogs coalesce onto one + /// refresh/browser flow instead of racing browsers. + async fn acquire( + &self, + intent: AuthIntent, + rejected: Option<&str>, + ) -> Result { + // Fast path: no lock, no network. `try_lock` rather than `lock().await` + // so a caller arriving while a leader holds `state` across its browser + // flow does not block here — it falls through to the in-process + // registry below and joins the leader instead of waiting out the whole + // flow and then racing in as a second leader. A cache hit is still + // served without the file lock; a miss (or contention) coalesces. + { + if let Ok(mut state) = self.state.try_lock() { + if let Some(hit) = self.cached_hit(&mut state, rejected) { + return Ok(hit); } } + } - // 4. Re-read disk after refresh failure — another process may have won the race. - if let Some(disk_tok) = read_cache(&self.cache_path) { - if !is_expired(&disk_tok) { - let bearer = disk_tok.access_token.clone(); - *state = Some(disk_tok); - return Ok(bearer); + // In-process single-flight (see [`INFLIGHT`]). Keyed by (lock path, + // intent): callers with the same intent coalesce, so a caller already + // waiting when the leader's attempt is in flight shares the leader's + // result instead of taking the lock after it and launching a second + // browser. Distinct intents key separately: a `Headless` caller never + // shares a browser-capable slot, and — critically — a `UserInitiated` + // caller never inherits an `Auto` leader's cooldown-suppressed result, + // since the two disagree on cooldown and browser policy. Those cases + // still coordinate through the cross-process file lock. + let key: InflightKey = (self.lock_path(), intent); + let (slot, is_leader) = { + let mut reg = inflight_registry(); + match reg.get(&key) { + Some(existing) => (existing.clone(), false), + None => { + let slot = Arc::new(InflightSlot::new()); + reg.insert(key.clone(), slot.clone()); + (slot, true) + } + } + }; + if !is_leader { + // Pre-existing joiner: observe the leader's outcome, but do not + // adopt a result that violates *this* caller's contract. The slot + // is keyed only by (lock path, intent), so a joiner shares a leader + // that ran with a *different* `rejected` value — and the leader's + // result can be wrong for us in two ways: + // + // * It may publish a token equal to THIS caller's `rejected` + // bytes — e.g. its cache re-read adopted a sibling write we + // just reported 401-rejected. Returning it would retry the + // provider with the exact credentials it refused. We instead + // run our own acquisition: the slot is evicted before publish + // (see [`LeaderGuard::complete`]), so this is a fresh, bounded, + // leader-eligible attempt — not a re-join of the dead + // generation, and not a loop. Its cache re-read excludes our + // `rejected`, and `finish`'s persistence-boundary guard rejects + // any refresh- or browser-issued token equal to our `rejected` + // with a typed error before caching it — so the rerun never + // hands us back our `rejected` on any path. + // + // * It may publish a terminal failure from a *rejection-relative* + // cause — e.g. refresh reissued the leader's own `rejected` bytes + // and `finish()` returned `RefreshRejected`. That failure is valid + // only for the leader's specific rejected token; a joiner with a + // *different* `rejected` (or none) should rerun: its refresh may + // yield a valid token. The leader publishes its rejected-token + // SHA-256 digest so joiners can compare without inspecting the + // token bytes directly. A digest mismatch triggers an `acquire_leader` + // rerun (the slot is already evicted). A false rerun (non-rejection + // failure with digest mismatch) costs one network round-trip and + // stays headless — far better than silently adopting a wrong denial. + // + // * It may publish a terminal failure even though a sibling wrote + // a valid replacement into the cache while we waited. We + // re-check the cache cheaply before adopting the failure — a + // lock-free disk read, never a browser or refresh — so a shared + // failure can never fan out into an N-way browser storm. The + // disk read is lock-free (`usable_from_disk`, not under `state`) + // because all waiters wake together and the in-memory memo is + // not load-bearing here — the next real acquisition re-reads and + // adopts under the lock. + let (leader_rejected_digest, outcome) = slot.wait().await; + match outcome { + Ok(token) if Some(token.access_token.as_str()) != rejected => { + // Conditionally reconcile this source's own credential + // state so a subsequent plain `bearer()` on this source + // returns the newly-acquired token rather than a stale or + // absent credential. Adopt when B's state is absent, + // expired, or still pointing at B's own rejected token. + // Preserve a distinct newer usable credential — if another + // task independently installed a valid token into B's state + // between B joining and B waking, that token is better than + // the shared result and must not be overwritten. + // + // `lock().await` rather than `try_lock`: the reconciliation + // must complete before returning. The joiner holds neither + // the INFLIGHT registry mutex nor the cross-process file + // lock at this point, so awaiting `state` cannot deadlock + // and skipping the write would leave stale or empty state, + // recreating the original P1 regression on the next plain + // `bearer()` call. + { + let mut state = self.state.lock().await; + let adopt = state.as_ref().is_none_or(|cur| { + is_expired(cur) || rejected.is_some_and(|rej| cur.access_token == rej) + }); + if adopt { + *state = Some(token.clone()); + } + } + return Ok(token.access_token); + } + Ok(_) => { + return self + .acquire_leader(intent, rejected) + .await + .map(|t| t.access_token); + } + Err(shared) => { + // Reject-digest mismatch: the leader's failure was + // rejection-relative to ITS OWN `rejected` token, not ours. + // Rerun so we can pursue our own refresh/browser path. + if leader_rejected_digest != digest_of(rejected) { + return self + .acquire_leader(intent, rejected) + .await + .map(|t| t.access_token); + } + // Neutralize B's matching rejected in-memory state so a + // subsequent plain `bearer()` on this source does not + // resurface the rejected credential. + // + // `lock().await` rather than `try_lock`: expiry must + // complete before returning. The joiner holds neither the + // INFLIGHT registry mutex nor the cross-process file lock + // here, so awaiting `state` cannot deadlock. Skipping the + // expiry would leave matching rejected X live, recreating + // the original P1 regression on the next plain `bearer()`. + // + // In-memory only (`expire_rejected_memory`, not + // `expire_rejected`): the leader already ran the durable + // disk invalidation under the file lock. Re-running disk + // writes here is lockless — process C may have persisted a + // valid replacement under the same lock between A's failure + // and this rename, and B's unfenced rename would overwrite + // it. Note: a subsequent plain `bearer()` (`rejected=None`) + // calls `cached_hit` before the cross-process lock and can + // therefore re-read the disk copy without acquiring the lock. + { + let mut state = self.state.lock().await; + self.expire_rejected_memory(&mut state, rejected); + } + if let Some(hit) = self.usable_from_disk(rejected) { + return Ok(hit); + } + return Err(shared); } } } - // 5. No usable cache: full browser dance. - let fresh = browser_pkce_flow(&self.http, &self.cfg, &endpoints).await?; - let bearer = fresh.access_token.clone(); - self.save(&mut state, fresh)?; - Ok(bearer) + // Leader: run the real flow, then evict + publish. The guard makes + // eviction and joiner wake-up happen even if this future is cancelled + // or panics, so a dropped leader can never wedge its joiners or leave a + // dead slot that turns later callers into joiners of nothing. + let guard = LeaderGuard::new(key, slot); + let result = self.acquire_leader(intent, rejected).await; + guard.complete(result, digest_of(rejected)) } - async fn bearer_no_browser(&self) -> Result { - self.try_bearer_no_browser().await + /// The leader's slow-path body: take the cross-process lock, then run the + /// bounded acquisition under it. Split out so [`acquire`] can wrap it in + /// the in-process single-flight without the lock/deadline logic bleeding + /// into the joiner path. + async fn acquire_leader( + &self, + intent: AuthIntent, + rejected: Option<&str>, + ) -> Result { + // Snapshot the current attempt generation *before* queueing on the + // lock. When we acquire the lock, we compare: if the generation + // advanced, a predecessor completed while we were waiting and we can + // adopt its outcome instead of re-running the full flow. + let attempt_path = self.attempt_path(); + let snapshot_gen = read_attempt(&attempt_path) + .map(|r| r.generation) + .unwrap_or(0); + // Observability hook: cross-process tests install a tracing layer that + // watches for this event to establish deterministic ordering — it fires + // after the snapshot is taken and before the process queues on the lock. + tracing::trace!( + target: "buzz_agent::auth::acquire_leader_snapshot", + snapshot_gen, + "snapshot taken" + ); + + // Slow path: one flow at a time per cache key. The waiter's deadline + // exceeds a healthy holder's attempt deadline, so it never gives up on + // a live holder. + let deadline = std::time::Instant::now() + LOCK_WAIT_TIMEOUT; + let _guard = acquire_auth_lock(&self.lock_path(), deadline).await?; + + // Bound the whole locked attempt so a wedged flow can't hold the lock + // past the waiters' patience. The deadline is passed *into* + // `acquire_locked` rather than wrapped around it in a cancelling + // `tokio::time::timeout`: a cancel drops the future at an arbitrary + // await point, which would skip the cooldown write for a timed-out + // interactive attempt and let the next `Auto` caller re-pop a browser. + // Threading the deadline lets every interactive timeout exit through + // the common outcome writer while the lock is still held. + let attempt_deadline = std::time::Instant::now() + AUTH_ATTEMPT_DEADLINE; + self.acquire_locked( + intent, + rejected, + attempt_deadline, + &attempt_path, + snapshot_gen, + ) + .await } - /// Force-refresh after a 401, never touching the browser flow. + /// Slow-path body, run while holding the cross-process auth lock. /// - /// `rejected` is the access token the server just 401'd. Coalescing keys - /// off token *identity*, not the expiry clock: a 401 means the token was - /// rejected while it still looked locally fresh, so `is_expired()` would - /// say "keep it" and no grant would ever run. Instead, under the lock we - /// compare the current cached token to `rejected` — if they differ, a - /// concurrent caller (this process or a sibling) already refreshed, so we - /// return the new token without burning a second grant. If they still - /// match, this is the rejected token and we run the refresh-token grant - /// unconditionally. The whole check→refresh→save runs under one lock hold - /// so concurrent callers serialize. On any failure the refresh token is - /// preserved (never nulled) and the error is terminal `LlmAuth` — no - /// browser, no hang. - async fn refresh_now(&self, rejected: &str) -> Result { + /// `attempt_deadline` bounds the whole locked flow. Discovery and refresh + /// are each bounded by the HTTP client's per-request timeout; the browser + /// flow is wrapped in the *remaining* budget so a total-deadline expiry + /// during the interactive step surfaces as [`AuthError::TimedOut`] through + /// the same arm that records the cooldown — never as a cancellation that + /// drops the guard without writing it. + /// + /// `attempt_path` + `snapshot_gen` implement cross-process failure + /// single-flight: the caller snapshotted `snapshot_gen` before queueing on + /// the lock; if the generation has since advanced, a predecessor completed + /// while we waited. A caller already queued when the predecessor ran adopts + /// its same-intent terminal failure rather than re-running — including + /// `UserInitiated` callers, mirroring what [`INFLIGHT`] does within one + /// process. A `UserInitiated` caller arriving *after* the failure snapshots + /// the new generation and naturally does not adopt. + async fn acquire_locked( + &self, + intent: AuthIntent, + rejected: Option<&str>, + attempt_deadline: std::time::Instant, + attempt_path: &Path, + snapshot_gen: u64, + ) -> Result { let mut state = self.state.lock().await; - // 1. Coalesce by identity: if the cached token (in-memory, then disk) - // is no longer the one the server rejected, someone already - // refreshed it. Return that instead of grabbing another grant. - if let Some(tok) = state.as_ref() { - if tok.access_token != rejected { - return Ok(tok.access_token.clone()); - } + // A 401 (`rejected = Some`) proves the cached access token is dead even + // though its local expiry clock still looks fresh. Neutralize it now, + // under the lock, so it can never be served again: cache_hit already + // excludes it for callers carrying `rejected`, but a later plain + // `bearer()` (`rejected = None`) or a freshly constructed source would + // otherwise trust the clock and hand back the proven-dead bytes. The + // refresh token is untouched — it was not rejected and drives the + // recovery below. + self.expire_rejected(&mut state, rejected); + + // Re-check under the lock: a holder we queued behind may have already + // produced a token (this process or a sibling wrote the cache). + if self.cached_hit(&mut state, rejected).is_some() { + // `cached_hit` guarantees state is populated on a hit (memory entry + // was already there, or disk token was adopted into state). + return Ok(state.clone().expect("cached_hit confirmed token in state")); } - if let Some(disk_tok) = read_cache(&self.cache_path) { - if disk_tok.access_token != rejected { - let bearer = disk_tok.access_token.clone(); - *state = Some(disk_tok); - return Ok(bearer); + + // Cross-process failure single-flight. A predecessor completed while + // this caller was waiting on the lock: check whether its outcome was a + // terminal failure we should adopt rather than re-run. The contract is + // *temporal*, not intent-based: a caller whose pre-queue snapshot is + // older than the current generation was already queued while the + // predecessor ran and may adopt its failure, mirroring how the + // in-process [`INFLIGHT`] registry coalesces same-intent callers + // (including `UserInitiated`) within a single process. A `UserInitiated` + // caller arriving *after* a failure naturally snapshots the new + // generation and does not adopt, so "later explicit user retry bypasses" + // falls out without a special case. The conditions are: + // (a) the attempt generation advanced past our snapshot — we were + // queued while the predecessor ran, not a fresh arrival after it; + // (b) the recorded intent matches ours — cross-process adoption + // respects the same (path, intent) boundary as INFLIGHT, so a + // `UserInitiated` waiter never inherits an `Auto`/`Headless` + // failure (different intent, different promise to the user); + // (c) the recorded result is a recognized terminal failure — `"ok"` + // and unrecognized codes fall through to a normal attempt; + // (d) the recorded rejected_digest matches ours — a failure caused by + // the predecessor's specific rejected token is not valid for a + // caller with a *different* rejected token (both-`None` matches). + // A digest mismatch triggers a normal attempt; a false rerun on a + // non-rejection-relative failure costs one network round-trip and + // stays headless — preferable to silently serving a wrong denial. + // + // Adoptors do NOT write a new attempt record: adopting does not + // represent new work. Writing one would advance the generation so a + // third caller that arrives after the adoption (snapshot = new gen) sees + // no advance and tries its own attempt — but a fourth arriving while the + // third runs would inherit the adopter's re-written record, relaying the + // original failure indefinitely. The original record already has the + // correct generation; subsequent waiters with snapshot < original gen + // still adopt from it directly. + if let Some(rec) = read_attempt(attempt_path) { + if rec.generation > snapshot_gen + && rec.intent == intent.as_str() + && rec.rejected_digest == digest_of(rejected) + { + if let Some(err) = AuthError::from_code(&rec.result) { + return Err(err); + } } } - // 2. The cached token is still the rejected one. Run the refresh-token - // grant unconditionally — the expiry clock can't be trusted here, a - // locally-fresh token is exactly what got 401'd. - let refresh = state.as_ref().and_then(|t| t.refresh_token.clone()); - let Some(rt) = refresh else { - return Err(AgentError::LlmAuth( - "token rejected and no refresh token available".into(), - )); - }; - let endpoints = self.endpoints().await?; - match self.refresh(&endpoints, &rt).await { - Ok(fresh) => { - let bearer = fresh.access_token.clone(); - self.save(&mut state, fresh)?; - Ok(bearer) + // Refresh-token grant, if we have one. Endpoints are discovered lazily + // here (and reused by the browser branch) so a no-refresh headless + // failure never depends on reaching the discovery URL. + let mut endpoints: Option = None; + let mut refresh_failed = false; + if let Some(rt) = state.as_ref().and_then(|t| t.refresh_token.clone()) { + let eps = self.discover(&mut endpoints).await?; + match self.refresh(eps, &rt).await { + RefreshOutcome::Refreshed(fresh) => { + let result = self.finish(&mut state, fresh, intent, rejected); + // Record recognized terminal failures (rejected-equal reissuance) + // so a cross-process headless waiter can adopt them rather than + // re-running the same dead refresh. Successes are shared through + // the token cache — a waiter that wins the lock after us finds + // the token via `cached_hit` without reaching the adoption check. + if let Err(ref e) = result { + write_attempt(attempt_path, intent, e.code(), rejected); + } + return result; + } + // A transient fault (transport/timeout/5xx/decode) is not a + // credential decision: never fall through to a browser or + // report RefreshRejected. A sibling may have written a fresh + // token while we ran, so honor that first; otherwise this is + // infrastructural and surfaces as NetworkUnavailable. + RefreshOutcome::Network => { + if self.cached_hit(&mut state, rejected).is_some() { + return Ok(state.clone().expect("cached_hit confirmed token in state")); + } + return Err(AuthError::NetworkUnavailable); + } + // The token endpoint rejected the grant: a dead refresh token. + // A sibling may still have won the race while we ran; if not, + // fall through to a browser (interactive) or RefreshRejected + // (headless). + RefreshOutcome::Rejected => { + if self.cached_hit(&mut state, rejected).is_some() { + return Ok(state.clone().expect("cached_hit confirmed token in state")); + } + refresh_failed = true; + } } - // 3. Refresh token is itself dead. Terminal — surfacing LlmAuth - // stops the retry loop instead of falling to the browser flow, - // which would hang a headless harness. - Err(e) => Err(AgentError::LlmAuth(format!("token refresh failed: {e}"))), } - } -} - -impl PkceOAuthTokenSource { - /// Return a bearer token from cache or refresh, **never** opening a browser. - /// - /// Follows the same steps as [`bearer`](TokenSource::bearer) but stops at - /// step 4 — if no usable token is available after cache + refresh attempts, - /// returns `Err(LlmAuth(...))` instead of launching the browser PKCE flow. - /// Used by model-discovery paths that must not block on user interaction. - pub(crate) async fn try_bearer_no_browser(&self) -> Result { - let mut state = self.state.lock().await; - // 1. In-memory cache hit, still fresh. - if let Some(tok) = state.as_ref() { - if !is_expired(tok) { - return Ok(tok.access_token.clone()); - } + // No token from cache or refresh. Browser or terminal failure. + if !intent.may_open_browser() { + let err = if refresh_failed { + AuthError::RefreshRejected + } else { + AuthError::NoCredential + }; + write_attempt(attempt_path, intent, err.code(), rejected); + return Err(err); } - // 2. Re-read disk — another process may have refreshed already. - if let Some(disk_tok) = read_cache(&self.cache_path) { - if !is_expired(&disk_tok) { - let bearer = disk_tok.access_token.clone(); - *state = Some(disk_tok); - return Ok(bearer); + let cooldown_path = self.cooldown_path(); + if intent.honors_cooldown() { + // A recent browser attempt failed; surface its recorded outcome + // instead of re-popping a browser on this automatic attempt. + if let Some(recorded) = read_cooldown(&cooldown_path) { + return Err(recorded); } + } else { + // An explicit user retry clears any prior suppression. + clear_cooldown(&cooldown_path); } - // 3. Try refresh if we have a refresh token. Endpoints are discovered - // lazily here — only when a refresh token is actually present — so - // that an unreachable OIDC discovery URL cannot prevent the - // no-token/no-cache path from returning `LlmAuth` (graceful - // fallback) instead of `Llm` (hard error). - let refresh = state.as_ref().and_then(|t| t.refresh_token.clone()); - if let Some(rt) = refresh { - let endpoints = self.endpoints().await?; - match self.refresh(&endpoints, &rt).await { - Ok(fresh) => { - let bearer = fresh.access_token.clone(); - self.save(&mut state, fresh)?; - return Ok(bearer); - } - Err(e) => { - tracing::warn!(error = %e, "oauth refresh failed during model discovery"); - } + let eps = self.discover(&mut endpoints).await?; + // Wrap the browser flow in the *remaining* attempt budget so the total + // locked time never exceeds `attempt_deadline` (and thus never + // outlasts a waiter's `LOCK_WAIT_TIMEOUT`). A deadline expiry maps to + // `TimedOut`, which is cooldown-worthy, so it flows through the same + // writer arm below instead of being dropped by a cancel that would + // release the lock without recording the cooldown. + let remaining = attempt_deadline.saturating_duration_since(std::time::Instant::now()); + let flow = browser_pkce_flow(&self.http, &self.cfg, eps, self.opener.as_ref()); + let outcome = match tokio::time::timeout(remaining, flow).await { + Ok(result) => result, + Err(_) => Err(AuthError::TimedOut), + }; + match outcome { + // `finish` clears the cooldown on success and rejects a re-issued + // 401'd token before persisting it. + Ok(fresh) => { + let result = self.finish(&mut state, fresh, intent, rejected); + let code = match &result { + Ok(_) => "ok", + Err(e) => e.code(), + }; + write_attempt(attempt_path, intent, code, rejected); + result } - - // 4. Re-read disk after refresh failure. - if let Some(disk_tok) = read_cache(&self.cache_path) { - if !is_expired(&disk_tok) { - let bearer = disk_tok.access_token.clone(); - *state = Some(disk_tok); - return Ok(bearer); + Err(e) => { + if e.is_cooldown_worthy() { + write_cooldown(&cooldown_path, &e); } + write_attempt(attempt_path, intent, e.code(), rejected); + Err(e) } } + } + + /// Persist a freshly-obtained token, clear any cooldown, and return the + /// full [`CachedToken`] on success. A cache-write failure maps to + /// [`AuthError::NetworkUnavailable`] (the infrastructural bucket) — the + /// token was valid but couldn't be persisted, which the caller should treat + /// as transient, not as a credential rejection. + /// + /// The candidate-token persistence boundary for refresh and browser results. + /// Cache-hit paths bypass this function, but every refresh- or browser-issued + /// token flows through here before being written to memory or disk. This is + /// where the 401-recovery invariant is enforced: a token equal to the + /// caller's `rejected` bytes must never be committed — doing so would cache + /// the proven-dead token as fresh, so a later plain `bearer()` (`rejected = + /// None`) or a freshly constructed source reading the same cache would serve + /// it back. Validating *before* the write keeps the dead token out of the + /// cache and off disk entirely: we fail typed (`NetworkUnavailable` interactive + /// / `RefreshRejected` headless) without caching it or clearing the cooldown. + /// `cached_hit` and `usable_from_disk` already exclude `rejected`, so guarding + /// the two live-token sites (refresh and browser exchange) here covers every + /// path that can produce the rejected bytes. + /// + /// Returning the full [`CachedToken`] (rather than just the bearer string) + /// lets `acquire_locked` → `acquire_leader` propagate it all the way to + /// [`LeaderGuard::complete`], which publishes it through the [`InflightSlot`] + /// so every joiner can reconcile its own independent `state` cell. + fn finish( + &self, + state: &mut Option, + token: CachedToken, + intent: AuthIntent, + rejected: Option<&str>, + ) -> Result { + if rejected == Some(token.access_token.as_str()) { + return Err(if intent.may_open_browser() { + AuthError::NetworkUnavailable + } else { + AuthError::RefreshRejected + }); + } + self.save(state, token.clone()) + .map_err(|_| AuthError::NetworkUnavailable)?; + clear_cooldown(&self.cooldown_path()); + Ok(token) + } +} + +#[async_trait] +impl TokenSource for PkceOAuthTokenSource { + /// Acquire a bearer for a request. Routes through the coordinator as a + /// [`Headless`](AuthIntent::Headless) acquisition: it serves a cached or + /// refreshed token but never opens a browser, so a managed runtime with no + /// interactive display can never hang on inference. First-use auth is the + /// job of `buzz-agent auth databricks` ([`interactive_login`]). + /// + /// [`interactive_login`]: PkceOAuthTokenSource::interactive_login + async fn bearer(&self) -> Result { + self.acquire(AuthIntent::Headless, None) + .await + .map_err(Into::into) + } + + /// Identical to [`bearer`](Self::bearer) for this source — both are + /// headless. Retained as a distinct method so callers can state the + /// no-browser requirement at the call site (and so other [`TokenSource`] + /// impls that *would* browse in `bearer` can still expose a safe path). + async fn bearer_no_browser(&self) -> Result { + self.acquire(AuthIntent::Headless, None) + .await + .map_err(Into::into) + } - // No usable token — return error instead of opening a browser. - Err(AgentError::LlmAuth( - "no cached Databricks token; run `buzz-agent auth databricks` first".into(), - )) + /// Force a fresh bearer after the server rejected `rejected` with a 401. + /// + /// A [`Headless`](AuthIntent::Headless) acquisition keyed by token + /// *identity* rather than the expiry clock: a 401 means the cached token + /// was rejected while still locally fresh, so [`is_expired`] would wrongly + /// keep it. Passing `rejected` makes the coordinator run the refresh-token + /// grant unless a concurrent caller already replaced the token, in which + /// case that newer token is returned without a second grant. Never opens a + /// browser; a dead refresh token surfaces terminally so the retry loop + /// stops instead of hanging. + async fn refresh_now(&self, rejected: &str) -> Result { + self.acquire(AuthIntent::Headless, Some(rejected)) + .await + .map_err(Into::into) } } // ---- helpers ------------------------------------------------------------- +/// SHA-256 hex digest of `rejected` token bytes, or `None` when there is no +/// rejected token. Used to scope in-process and cross-process failure adoption +/// to the specific token that was rejected — a joiner carrying a *different* +/// rejected token (or none) must not inherit a rejection-relative failure. +fn digest_of(rejected: Option<&str>) -> Option { + rejected.map(|r| hex::encode(sha2::Sha256::digest(r.as_bytes()))) +} + /// Aborts a spawned task when dropped. Used to guarantee the localhost /// callback server doesn't outlive a failed/abandoned PKCE attempt. struct AbortOnDrop(tokio::task::JoinHandle<()>); @@ -438,11 +1350,7 @@ fn is_expired(t: &CachedToken) -> bool { let Some(exp) = t.expires_at else { return false; }; - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - now + TOKEN_REFRESH_LEEWAY.as_secs() >= exp + now_secs() + TOKEN_REFRESH_LEEWAY.as_secs() >= exp } const BUZZ_AGENT_CONFIG_DIR_ENV: &str = "BUZZ_AGENT_CONFIG_DIR"; @@ -484,6 +1392,379 @@ fn cache_path_for(cfg: &PkceOAuthConfig) -> Result { Ok(dir.join(format!("{hash}.json"))) } +/// Append `ext` as an extra extension onto `base` (e.g. `.json` → +/// `.json.lock`). Keeps the lock and cooldown sidecars in the same +/// per-key directory as the cache, so they inherit its `$HOME` override and +/// owner-only parent without a second key derivation. +fn append_ext(base: &Path, ext: &str) -> PathBuf { + let mut name = base.as_os_str().to_owned(); + name.push("."); + name.push(ext); + PathBuf::from(name) +} + +/// Durable record of the last browser-attempt failure for a cache key. Written +/// while holding the auth lock so concurrent writers can't interleave, read by +/// `Auto` callers to decide whether to suppress an automatic browser re-launch. +#[derive(Debug, Serialize, Deserialize)] +struct CooldownRecord { + /// [`AuthError::code`] of the failure being cooled down. + code: String, + /// Unix seconds after which the cooldown lapses and an `Auto` caller may + /// launch a browser again. + until: u64, +} + +/// Durable record of the generation and outcome of the most recently completed +/// slow-path acquisition attempt for a cache key. +/// +/// Cross-process single-flight for *failures*: the in-process [`INFLIGHT`] +/// registry coalesces same-key callers within one process, but two separate +/// processes both waiting on the OS file lock do NOT share the registry. When +/// process A holds the lock and fails (e.g. browser denial or dead refresh), +/// process B's queued caller acquires the lock after A releases it and — under +/// the old protocol — would re-run the full flow from scratch. This record lets +/// B detect that it was already queued while A ran and adopt A's failure +/// instead of hammering the provider again. +/// +/// Protocol: +/// - A caller **snapshots** the current generation from the sidecar *before* +/// queueing on the file lock. +/// - A caller that **acquires** the lock compares the current generation to its +/// snapshot: if it advanced, a predecessor completed while it was waiting. +/// If the recorded intent matches this caller's intent and the outcome is a +/// recognized terminal failure, adopt it rather than re-running. +/// - Completing attempts **write** a fresh record under the lock. Write +/// coverage: the headless no-browser arm (`RefreshRejected`/`NoCredential`), +/// the refresh arm when `finish()` fails typed (rejected-equal reissuance), +/// and the browser arm (all outcomes including `"ok"`). Omissions that are +/// intentionally not adoption-worthy: transient `Network` errors, discovery +/// failures (both non-terminal; next caller retries), and cache/refresh- +/// success paths (a waiting caller finds the token via `cached_hit` without +/// reaching the adoption check). +/// +/// The generation counter is read fresh from disk at write time so each +/// completed attempt strictly advances the value regardless of when the +/// caller's pre-queue snapshot was taken. +/// +/// Intent matching is same-intent only, mirroring the in-process `(path, +/// intent)` key. The temporal condition handles "later explicit retry bypasses": +/// a `UserInitiated` caller arriving after the failure snapshots the new +/// generation and sees no advance, so it always runs its own attempt and never +/// inherits a prior failure — regardless of intent. +#[derive(Debug, Serialize, Deserialize)] +struct AttemptRecord { + /// Strictly increasing counter: read from disk at write time and incremented + /// by one so each attempt advances from the actual current value regardless + /// of when the writing caller's snapshot was taken. + generation: u64, + /// Intent of the attempt that completed, as [`AuthIntent::as_str`]. + intent: String, + /// Error code of the terminal failure, or `"ok"` on success. Matches + /// [`AuthError::code`] / the `"ok"` sentinel. + result: String, + /// SHA-256 hex digest of the token bytes that the completing caller had + /// marked as `rejected`, or `None` when the caller carried no rejected + /// token. A waiter adopts only when its own digest matches: a failure caused + /// by the leader's specific rejected token is not valid for a waiter with a + /// *different* rejected token (or none) — its refresh may yield a live + /// token. Both-`None` is a match. A mismatched digest triggers a normal + /// attempt; a false rerun on a non-rejection failure costs one network round- + /// trip and stays headless — preferable to silently adopting a wrong denial. + #[serde(default)] + rejected_digest: Option, +} + +/// Read the attempt sidecar at `path`, if any. Returns `None` when absent, +/// unparseable, or the generation is 0 (no attempt has completed yet). +fn read_attempt(path: &Path) -> Option { + let body = fs::read(path).ok()?; + let record: AttemptRecord = serde_json::from_slice(&body).ok()?; + Some(record) +} + +/// Write a fresh attempt record at `path`. Called under the auth lock. +/// Best-effort — a write failure only means the next cross-process waiter +/// cannot adopt this attempt's outcome, so errors are swallowed. +/// +/// Always reads the current on-disk generation before writing so the new +/// record strictly advances from the actual last-recorded value, not from +/// any caller's pre-queue snapshot. An intervening different-intent attempt +/// that advanced the sidecar between snapshot and lock-acquire is reflected +/// correctly: the next waiter's comparison still sees a real advance. +fn write_attempt(path: &Path, intent: AuthIntent, result: &str, rejected: Option<&str>) { + let current_gen = read_attempt(path).map_or(0, |r| r.generation); + let record = AttemptRecord { + generation: current_gen.wrapping_add(1), + intent: intent.as_str().to_owned(), + result: result.to_owned(), + rejected_digest: digest_of(rejected), + }; + if let Ok(body) = serde_json::to_vec(&record) { + let _ = write_private_cache(path, &body); + } +} + +fn now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) +} + +/// Return the still-active cooldown outcome for `path`, if any. +/// +/// `None` when the sidecar is absent, unparseable, expired, or records a code +/// this build doesn't recognize — every one of those means "no active +/// cooldown", so the caller proceeds to a normal attempt. An expired record is +/// removed opportunistically so the directory doesn't accumulate stale files. +fn read_cooldown(path: &Path) -> Option { + let body = fs::read(path).ok()?; + let record: CooldownRecord = serde_json::from_slice(&body).ok()?; + if record.until > now_secs() { + AuthError::from_code(&record.code) + } else { + let _ = fs::remove_file(path); + None + } +} + +/// Record `err` as a fresh cooldown at `path`, expiring [`COOLDOWN_DURATION`] +/// from now. Best-effort: a write failure only means the next automatic +/// attempt may re-pop a browser, never a hard auth failure, so errors are +/// swallowed. Called while holding the auth lock. +fn write_cooldown(path: &Path, err: &AuthError) { + let record = CooldownRecord { + code: err.code().to_string(), + until: now_secs() + COOLDOWN_DURATION.as_secs(), + }; + if let Ok(body) = serde_json::to_vec(&record) { + let _ = write_private_cache(path, &body); + } +} + +/// Remove any cooldown sidecar at `path`. Called on a successful acquisition +/// (the problem is resolved) and by `UserInitiated` callers that bypass the +/// cooldown (an explicit retry clears the suppression). Best-effort. +fn clear_cooldown(path: &Path) { + let _ = fs::remove_file(path); +} + +/// Hold on the cross-process auth lock. Dropping it (or the owning process +/// dying) releases the OS advisory lock — no PID files, no manual break. +#[derive(Debug)] +struct AuthLockGuard(fs::File); + +impl Drop for AuthLockGuard { + fn drop(&mut self) { + // Explicit for intent; closing the fd would release it regardless. + let _ = FileExt::unlock(&self.0); + } +} + +/// Acquire the cross-process auth lock at `path`, polling until `deadline`. +/// +/// `fs2::FileExt::try_lock_exclusive` maps to `flock(LOCK_EX | LOCK_NB)` on +/// Unix and `LockFileEx` on Windows — advisory, per–open-file-description, so +/// a lock taken on one handle blocks every other handle (same process or not), +/// which is exactly the cross-process single-flight guarantee we want. The +/// try-lock is non-blocking, so we poll on [`LOCK_POLL_INTERVAL`] rather than +/// parking a worker thread in a blocking `lock_exclusive()`. Contention is +/// reported as [`fs2::lock_contended_error`] (`EWOULDBLOCK`/`EACCES` on Unix, +/// `ERROR_LOCK_VIOLATION` on Windows); we match its `raw_os_error` and retry. +/// Any other error is a real fault and returns [`AuthError::LockTimeout`]. A +/// waiter whose `deadline` lapses also returns [`AuthError::LockTimeout`]; +/// because the caller sets that deadline longer than [`AUTH_ATTEMPT_DEADLINE`], +/// a healthy holder always finishes first. +async fn acquire_auth_lock( + path: &Path, + deadline: std::time::Instant, +) -> Result { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).map_err(|_| AuthError::LockTimeout)?; + } + let file = fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(path) + .map_err(|_| AuthError::LockTimeout)?; + let contended = fs2::lock_contended_error().raw_os_error(); + loop { + match file.try_lock_exclusive() { + Ok(()) => return Ok(AuthLockGuard(file)), + Err(e) if e.raw_os_error() == contended => { + if std::time::Instant::now() >= deadline { + return Err(AuthError::LockTimeout); + } + tokio::time::sleep(LOCK_POLL_INTERVAL).await; + } + Err(_) => return Err(AuthError::LockTimeout), + } + } +} + +/// Key for the in-process single-flight registry: the cross-process lock path +/// (one per cache key) paired with the caller's [`AuthIntent`]. Keying by the +/// full intent — not merely browser capability — keeps callers with *different* +/// outcome policy from coalescing: an `Auto` leader honors a live cooldown and +/// returns its recorded `Denied`/`TimedOut`, but a `UserInitiated` caller is +/// promised a cooldown bypass and a fresh browser, so it must never inherit an +/// `Auto` leader's suppressed result. Each intent still coalesces with itself +/// (two concurrent `UserInitiated` sign-ins share one browser), and all intents +/// on the same key still serialize through the cross-process file lock. +type InflightKey = (PathBuf, AuthIntent); + +/// Process-global registry of in-flight auth attempts, the in-process +/// counterpart to [`acquire_auth_lock`]'s cross-process file lock. The file +/// lock serializes work across processes and shares *success* via a cache +/// re-read, but a queued caller that acquires the lock after a browser denial +/// would clear the sidecar and pop a second browser. This registry closes that +/// gap: a caller that arrives while a leader's attempt is in flight joins the +/// leader's [`InflightSlot`] and receives the *same* result — success or +/// failure — instead of taking the lock afterward and launching again. Guarded +/// by a `std::sync::Mutex` because every critical section is a cheap map lookup +/// with no `.await` held. +static INFLIGHT: LazyLock>>> = + LazyLock::new(|| std::sync::Mutex::new(HashMap::new())); + +/// Lock the in-flight registry, recovering from a poisoned mutex rather than +/// panicking: the only work done under this lock is map lookups that can't +/// leave inconsistent state, so a poison from an unrelated panic must not wedge +/// every future auth attempt. +fn inflight_registry() -> std::sync::MutexGuard<'static, HashMap>> { + INFLIGHT.lock().unwrap_or_else(|e| e.into_inner()) +} + +/// The value published by a leader to its joiners: the leader's rejected-token +/// SHA-256 digest (non-secret identity, `None` when the leader carried no +/// `rejected`) paired with the attempt result. Joiners use the digest to detect +/// a mismatch — the leader's rejection-relative failure is not valid for a +/// joiner that carried a *different* rejected token. +/// +/// On success the full [`CachedToken`] is published so each joiner can +/// conditionally reconcile its own independent [`PkceOAuthTokenSource::state`] +/// cell. Publishing the full credential (not just the bearer string) prevents +/// a joining source's state from remaining stale or empty after the coalesced +/// flow, which would otherwise cause a subsequent plain `bearer()` on that +/// source to resurface a rejected or absent credential rather than the +/// newly-acquired one. +type SlotPublish = (Option, Result); + +/// The shared result of one leader's auth attempt, awaited by any joiner that +/// arrived while the leader was in flight. A `watch` channel gives us +/// publish-once plus wait-for-publish in one primitive: the leader publishes +/// exactly once through [`LeaderGuard`]; joiners clone the published result. +struct InflightSlot { + tx: watch::Sender>, + rx: watch::Receiver>, +} + +impl InflightSlot { + fn new() -> Self { + let (tx, rx) = watch::channel(None); + Self { tx, rx } + } + + /// Block until the leader publishes, then clone out `(rejected_digest, result)`. + /// + /// `borrow_and_update` marks the current value seen before awaiting, so a + /// publish that lands between the read and the `changed()` await is not a + /// lost wakeup — the version has advanced, so `changed()` returns at once. + /// A closed channel (leader dropped without publishing — which + /// [`LeaderGuard`]'s `Drop` prevents) surfaces as a transient so the caller + /// retries rather than hangs. + async fn wait(&self) -> SlotPublish { + let mut rx = self.rx.clone(); + loop { + if let Some(publish) = rx.borrow_and_update().clone() { + return publish; + } + if rx.changed().await.is_err() { + return (None, Err(AuthError::NetworkUnavailable)); + } + } + } + + /// Publish `(rejected_digest, result)` to every waiting joiner. A send + /// error means no joiners remain, which is fine. + fn publish(&self, rejected_digest: Option, result: Result) { + let _ = self.tx.send(Some((rejected_digest, result))); + } +} + +/// RAII owner of a leader's in-flight slot. Guarantees the slot is evicted from +/// [`INFLIGHT`] and a result published to joiners even if the leader future is +/// cancelled or panics: a leader that skipped this would leave a dead slot that +/// turns every later caller into a joiner of an attempt that never publishes, +/// wedging them until `LOCK_WAIT_TIMEOUT`. +struct LeaderGuard { + key: InflightKey, + slot: Arc, + done: bool, +} + +impl LeaderGuard { + fn new(key: InflightKey, slot: Arc) -> Self { + Self { + key, + slot, + done: false, + } + } + + /// Normal completion: evict the slot, publish `(rejected_digest, result)` + /// to joiners, and return the bearer to the leader. The full + /// [`CachedToken`] is published so joiners can reconcile their own + /// [`PkceOAuthTokenSource::state`] before returning. Evicting *before* + /// publishing means a caller arriving after this point starts a fresh + /// attempt (a later explicit retry may launch), while joiners already + /// holding the slot still receive the result. `Drop` covers the cancel/panic + /// path. + fn complete( + mut self, + result: Result, + rejected_digest: Option, + ) -> Result { + self.done = true; + Self::evict(&self.key, &self.slot); + // Clone the error before moving `result` into the slot publish so we + // can return the original error to the leader on failure. + let leader_return = result + .as_ref() + .map(|t| t.access_token.clone()) + .map_err(|e| e.clone()); + self.slot.publish(rejected_digest, result); + leader_return + } + + /// Remove this leader's slot from the registry, but only if it is still the + /// same slot — defends against evicting a successor a later attempt may + /// have installed under the same key. + fn evict(key: &InflightKey, slot: &Arc) { + let mut reg = inflight_registry(); + if reg + .get(key) + .is_some_and(|existing| Arc::ptr_eq(existing, slot)) + { + reg.remove(key); + } + } +} + +impl Drop for LeaderGuard { + fn drop(&mut self) { + if self.done { + return; + } + // Cancelled or panicked before `complete`: evict so later callers start + // fresh, and wake joiners with a transient error so they retry rather + // than hang on a leader that will never publish. + Self::evict(&self.key, &self.slot); + self.slot.publish(None, Err(AuthError::NetworkUnavailable)); + } +} + /// Load a cached token, enforcing the owner-only invariant on load. /// /// Owner-only permissions are a cache *lifecycle* invariant, not just a @@ -536,11 +1817,23 @@ fn read_private_cache(path: &Path) -> io::Result> { Ok(body) } -/// Non-Unix fallback: read the cache as-is. Owner-only enforcement is the -/// Windows DACL work deferred behind the [`create_private_temp_file`] seam. +/// Non-Unix: token persistence and reading are both disabled until a +/// Windows-specific owner-only DACL is implemented. Any legacy token file +/// left by an older build (written with default ACLs) is deleted +/// opportunistically so the exposed artifact cannot be served by new builds. +/// Returns an error so [`read_cache`] yields `None`, giving a consistent +/// memory-only cache on non-Unix. #[cfg(not(unix))] fn read_private_cache(path: &Path) -> io::Result> { - fs::read(path) + // Best-effort removal of any legacy file. Errors are ignored — either the + // file does not exist (normal case) or it cannot be removed (no worse + // than before — the DACL story is still broken, but that is the pre-fix + // state we are trying to retire). + let _ = fs::remove_file(path); + Err(io::Error::new( + io::ErrorKind::Unsupported, + "token disk cache disabled on non-Unix (no owner-only DACL)", + )) } /// Removes a temp file on drop unless it was already renamed away. Keeps a @@ -731,21 +2024,38 @@ fn sanitize_callback_detail(raw: &str) -> String { .collect() } -/// Spin up a localhost callback server, open the authorize URL in a -/// browser, wait up to [`BROWSER_AUTH_TIMEOUT`] for the redirect, then -/// exchange the code for a token. +/// Spin up a localhost callback server, hand the authorize URL to `opener`, +/// wait up to [`BROWSER_AUTH_TIMEOUT`] for the redirect, then exchange the +/// code for a token. +/// +/// `opener` is invoked *after* the listener is bound and the abort guard is +/// armed, so a launch failure never returns a URL pointing at a torn-down +/// listener. Every failure is a typed [`AuthError`] so the coordinator can +/// record a cooldown (or not) by category: an open failure is +/// [`BrowserOpenFailed`], a redirect that never arrives is [`TimedOut`], a +/// provider-reported denial is [`Denied`], and a code exchange the provider +/// rejects with `invalid_grant` is [`ExchangeFailed`]; infrastructure faults +/// (bind/exchange transport, 429, 5xx, or a malformed success body) are +/// [`NetworkUnavailable`]. +/// +/// [`BrowserOpenFailed`]: AuthError::BrowserOpenFailed +/// [`TimedOut`]: AuthError::TimedOut +/// [`Denied`]: AuthError::Denied +/// [`ExchangeFailed`]: AuthError::ExchangeFailed +/// [`NetworkUnavailable`]: AuthError::NetworkUnavailable async fn browser_pkce_flow( http: &Client, cfg: &PkceOAuthConfig, endpoints: &OidcEndpoints, -) -> Result { + opener: &dyn BrowserOpener, +) -> Result { use axum::{extract::Query, response::Html, routing::get, Router}; use std::collections::HashMap; use std::net::SocketAddr; use tokio::sync::oneshot; - let (verifier, challenge) = pkce_pair()?; - let state = random_state()?; + let (verifier, challenge) = pkce_pair().map_err(|_| AuthError::NetworkUnavailable)?; + let state = random_state().map_err(|_| AuthError::NetworkUnavailable)?; let (tx, rx) = oneshot::channel::>(); let tx = Arc::new(Mutex::new(Some(tx))); @@ -768,10 +2078,10 @@ async fn browser_pkce_flow( let listener = tokio::net::TcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))) .await - .map_err(|e| AgentError::Llm(format!("oauth callback bind: {e}")))?; + .map_err(|_| AuthError::NetworkUnavailable)?; let port = listener .local_addr() - .map_err(|e| AgentError::Llm(format!("oauth callback addr: {e}")))? + .map_err(|_| AuthError::NetworkUnavailable)? .port(); let redirect_uri = format!("http://localhost:{port}"); @@ -793,14 +2103,25 @@ async fn browser_pkce_flow( urlencoding::encode(&challenge), ); - eprintln!("Opening browser for authentication. If it doesn't open, visit:\n {auth_url}"); - let _ = webbrowser::open(&auth_url); + // Launch the browser while the listener is live. A launch failure aborts + // before we wait on a redirect nobody can send. + opener.open(&auth_url).map_err(|e| { + tracing::warn!(error = %e, "oauth browser launch failed"); + AuthError::BrowserOpenFailed + })?; - let code = tokio::time::timeout(BROWSER_AUTH_TIMEOUT, rx) - .await - .map_err(|_| AgentError::Llm("oauth: browser auth timed out".into()))? - .map_err(|_| AgentError::Llm("oauth: callback sender dropped".into()))? - .map_err(|e| AgentError::Llm(format!("oauth callback: {e}")))?; + let code = match tokio::time::timeout(BROWSER_AUTH_TIMEOUT, rx).await { + // Timed out waiting for the redirect. + Err(_) => return Err(AuthError::TimedOut), + // Callback task dropped the sender without sending — treat as timeout. + Ok(Err(_)) => return Err(AuthError::TimedOut), + // Provider/user reported an error (denial, state mismatch, missing code). + Ok(Ok(Err(detail))) => { + tracing::warn!(detail = %detail, "oauth callback reported failure"); + return Err(AuthError::Denied); + } + Ok(Ok(Ok(code))) => code, + }; // Exchange code for token. let params = [ @@ -815,21 +2136,47 @@ async fn browser_pkce_flow( .form(¶ms) .send() .await - .map_err(|e| AgentError::Llm(format!("oauth exchange: {e}")))?; - if !resp.status().is_success() { + // Transport error or the per-request timeout elapsed: no verdict from + // the provider, so this is infrastructural, not a rejected grant. + .map_err(|_| AuthError::NetworkUnavailable)?; + let status = resp.status(); + if !status.is_success() { let body = resp.text().await.unwrap_or_default(); - return Err(AgentError::Llm(format!("oauth exchange failed: {body}"))); + // Only a 4xx `invalid_grant` (RFC 6749 §6.4.1) establishes the + // authorization code itself was rejected — the terminal, cooldown-worthy + // `ExchangeFailed`. A 429, any 5xx, and any other/unparseable 4xx are a + // transient provider fault or misconfiguration a cooldown must not + // suppress, so they surface as `NetworkUnavailable` — mirroring the + // refresh classifier, which likewise keys on the body `error`, not the + // bare status class. + if status.is_client_error() + && serde_json::from_str::(&body) + .ok() + .and_then(|v| v.get("error").and_then(Value::as_str).map(str::to_owned)) + .as_deref() + == Some("invalid_grant") + { + tracing::warn!(status = %status, body = %body, "oauth code exchange rejected"); + return Err(AuthError::ExchangeFailed); + } + tracing::warn!(status = %status, body = %body, "oauth code exchange not a grant rejection"); + return Err(AuthError::NetworkUnavailable); } + // A 2xx whose body is missing/malformed or lacks an access token is a + // provider fault, not a rejected grant: it never establishes that the code + // was refused, so it stays in the transient bucket rather than poisoning a + // 5-minute cooldown. let v: Value = resp .json() .await - .map_err(|e| AgentError::Llm(format!("oauth exchange json: {e}")))?; - token_from_response(&v, None) + .map_err(|_| AuthError::NetworkUnavailable)?; + token_from_response(&v, None).map_err(|_| AuthError::NetworkUnavailable) } #[cfg(test)] mod tests { use super::*; + use std::time::Instant; #[test] fn pkce_pair_produces_valid_challenge() { @@ -955,6 +2302,7 @@ mod tests { assert!(token_from_response(&v, None).is_err()); } + #[cfg(unix)] // Disk adoption relies on `write_private_cache`; non-Unix disables disk persistence. #[tokio::test] async fn test_bearer_reuses_disk_token_after_expiry() { let dir = tempfile::tempdir().unwrap(); @@ -996,11 +2344,31 @@ mod tests { assert_eq!(result, "fresh-from-disk"); } + /// A joiner that wakes to the leader's shared *failure* must still recover + /// a sibling's valid replacement from disk. The matching-failure path + /// neutralizes the joiner's own rejected state (under `lock().await`) and + /// then reads the disk lock-free — so a shared failure never forces an + /// N-way browser storm when a sibling already wrote a valid cache entry. + /// + /// The disk replacement is written AFTER B has deterministically joined the + /// slot (held state guard forces the joiner path; poll 1 confirms B is + /// blocked on `state.lock().await`). This ensures the test actually + /// exercises the joiner recovery branch rather than the initial fast-path + /// `cached_hit`. Removing the joiner disk-recovery branch must make the + /// test return Err(RefreshRejected) rather than Ok("sibling-replacement"). + /// + /// Disk-dependent: the replacement lives on disk, so `write_private_cache` + /// must be available (i.e. Unix only). + #[cfg(unix)] #[tokio::test] - async fn test_bearer_falls_through_to_browser_when_disk_also_expired() { + async fn test_joiner_shared_failure_recovers_disk_replacement() { + use std::future::Future as _; + use std::pin::pin; + use std::task::{Context, Poll, Waker}; + let dir = tempfile::tempdir().unwrap(); let cfg = PkceOAuthConfig { - discovery_url: "https://example.com/.well-known".into(), + discovery_url: "https://invalid.example.test/.well-known".into(), client_id: "test-client".into(), scopes: vec!["offline_access".into()], cache_namespace: "test".into(), @@ -1008,7 +2376,201 @@ mod tests { }; let source = PkceOAuthTokenSource::new(cfg).unwrap(); - // Expire the in-memory state. + let future_exp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 7200; + let replacement = CachedToken { + access_token: "sibling-replacement".into(), + refresh_token: Some("rt".into()), + expires_at: Some(future_exp), + }; + + // Pre-install a slot for this key and publish the leader's terminal + // failure — digest matches "rejected-bytes" so the joiner enters the + // in-memory neutralization branch. + let key: InflightKey = (source.lock_path(), AuthIntent::Headless); + let slot = Arc::new(InflightSlot::new()); + inflight_registry().insert(key.clone(), slot.clone()); + slot.publish( + digest_of(Some("rejected-bytes")), + Err(AuthError::RefreshRejected), + ); + + // Hold the state mutex so the fast-path `try_lock` fails and B is + // forced down the joiner path. The slot is already published, so + // `slot.wait()` returns immediately; B then calls `state.lock().await` + // and suspends while we hold the guard. + let state_guard = source.state.lock().await; + + let mut b_fut = pin!(source.acquire(AuthIntent::Headless, Some("rejected-bytes"))); + let waker = Waker::noop(); + let mut cx = Context::from_waker(waker); + + // Poll 1: B falls through fast-path (try_lock fails), joins the + // pre-published slot, enters the Err match arm, and blocks on + // `state.lock().await` — structural proof B is on the joiner path. + assert!( + matches!(b_fut.as_mut().poll(&mut cx), Poll::Pending), + "poll 1 must be Pending: B is blocked at state.lock().await after waking to Err" + ); + + // Now install the disk replacement. B is definitely past the initial + // fast-path and will only see this token via `usable_from_disk` after + // reconciliation — the recovery branch we are testing. + fs::write( + &source.cache_path, + serde_json::to_vec(&replacement).unwrap(), + ) + .unwrap(); + + // Release the mutex. B acquires the lock, calls expire_rejected_memory + // (empty state — no-op), then reads the disk replacement via + // `usable_from_disk` and returns Ok("sibling-replacement"). + // + // Mutation check: removing the `usable_from_disk` recovery branch + // makes B return Err(RefreshRejected) instead — the assertion fails. + drop(state_guard); + + let result = b_fut.await; + inflight_registry().remove(&key); + + assert_eq!( + result, + Ok("sibling-replacement".to_string()), + "the joiner must read the disk replacement and not inherit the shared failure — \ + mutation check: removing the usable_from_disk branch returns Err(RefreshRejected)" + ); + } + + /// **Joiner failure cleanup must not modify the shared disk cache.** + /// + /// The matching-failure joiner calls `expire_rejected_memory` (in-process + /// state only). It must not write, truncate, rename, or remove the disk + /// cache. An independent process C may have persisted a valid replacement + /// under the cross-process file lock between A's failure and B's + /// reconciliation; an unfenced disk write from B would overwrite it. + /// + /// This test seeds X on disk, runs B as a joiner that wakes to a matching + /// failure, and asserts the disk file is byte-for-byte unchanged afterward. + /// + /// Mutation check: reverting the joiner arm to call `expire_rejected` + /// instead of `expire_rejected_memory` makes B read the disk file, see + /// `access_token == "rejected-X"`, set `expires_at = 0`, and overwrite the + /// file via `persist` or in-place truncate. The disk bytes change, and the + /// "disk unchanged" assertion fails — proving the unfenced write is exactly + /// the race that would overwrite any concurrent C write that landed between + /// A's failure and B's reconciliation. + #[cfg(unix)] + #[tokio::test] + async fn test_joiner_failure_does_not_write_disk() { + use std::future::Future as _; + use std::pin::pin; + use std::task::{Context, Poll, Waker}; + + let dir = tempfile::tempdir().unwrap(); + let cfg = PkceOAuthConfig { + discovery_url: "https://invalid.example.test/.well-known".into(), + client_id: "test-client".into(), + scopes: vec!["offline_access".into()], + cache_namespace: "test".into(), + cache_dir_override: Some(dir.path().to_path_buf()), + }; + let b = PkceOAuthTokenSource::new(cfg).unwrap(); + + let future_exp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 7200; + + // Seed X on disk. The constructor may not create the parent directory + // without a pre-existing file, so ensure it exists first. + let token_x = CachedToken { + access_token: "rejected-X".into(), + refresh_token: Some("live-refresh".into()), + expires_at: Some(future_exp), + }; + if let Some(parent) = b.cache_path.parent() { + fs::create_dir_all(parent).unwrap(); + } + let disk_before = serde_json::to_vec(&token_x).unwrap(); + fs::write(&b.cache_path, &disk_before).unwrap(); + + // Pre-install a matching-failure slot (digest matches "rejected-X"). + let key: InflightKey = (b.lock_path(), AuthIntent::Headless); + let slot = Arc::new(InflightSlot::new()); + inflight_registry().insert(key.clone(), slot.clone()); + slot.publish( + digest_of(Some("rejected-X")), + Err(AuthError::RefreshRejected), + ); + + // Hold B's state mutex: fast-path try_lock fails → joiner path; + // state.lock().await during reconciliation blocks until we drop. + let state_guard = b.state.lock().await; + + let mut b_fut = pin!(b.acquire(AuthIntent::Headless, Some("rejected-X"))); + let waker = Waker::noop(); + let mut cx = Context::from_waker(waker); + + // Poll 1: B falls through fast-path, joins the pre-published slot, + // wakes to Err, and parks at state.lock().await. + assert!( + matches!(b_fut.as_mut().poll(&mut cx), Poll::Pending), + "poll 1 must be Pending: B is parked at state.lock().await after waking to Err" + ); + + // Release the state guard. B acquires the lock, calls + // expire_rejected_memory (in-memory neutralization only — no disk I/O), + // then checks usable_from_disk. The disk token's access_token is + // "rejected-X" which equals `rejected`, so usable_from_disk filters it + // and returns None. B returns Err(RefreshRejected). + drop(state_guard); + + let result = b_fut.await; + inflight_registry().remove(&key); + + assert_eq!( + result, + Err(AuthError::RefreshRejected), + "B must propagate the shared failure" + ); + + // The disk file must be byte-for-byte identical to what was seeded. + // expire_rejected_memory must not have touched it. + // + // Mutation check: expire_rejected reads the disk file, finds + // access_token == "rejected-X", sets expires_at = 0, and rewrites + // the file. The bytes change and this assertion fails — proving the + // unfenced write is the exact race that overwrites a concurrent C write + // landing between A's failure and B's reconciliation. + let disk_after = fs::read(&b.cache_path).unwrap(); + assert_eq!( + disk_after, disk_before, + "joiner failure cleanup must not modify the disk cache — \ + mutation check: expire_rejected rewrites the file (expires_at=0), \ + overwriting any concurrent write from process C" + ); + } + + #[tokio::test] + async fn test_bearer_headless_no_credential_is_terminal_without_browser() { + let dir = tempfile::tempdir().unwrap(); + let cfg = PkceOAuthConfig { + // Unreachable discovery URL: if bearer() ever attempts discovery or + // a browser flow, this test would hang or error differently. The + // headless path must not touch either. + discovery_url: "https://invalid.example.test/.well-known".into(), + client_id: "test-client".into(), + scopes: vec!["offline_access".into()], + cache_namespace: "test".into(), + cache_dir_override: Some(dir.path().to_path_buf()), + }; + let source = PkceOAuthTokenSource::new(cfg).unwrap(); + + // Expire the in-memory state with no refresh token. { let mut state = source.state.lock().await; *state = Some(CachedToken { @@ -1018,7 +2580,7 @@ mod tests { }); } - // Write an expired token to disk too. + // Write an expired, refresh-less token to disk too. let expired_token = CachedToken { access_token: "also-stale".into(), refresh_token: None, @@ -1027,27 +2589,25 @@ mod tests { let body = serde_json::to_vec_pretty(&expired_token).unwrap(); fs::write(&source.cache_path, &body).unwrap(); - // bearer() should fall through past the disk check. - // It will fail at the endpoints() discovery call since there's no server, - // which proves it didn't short-circuit on the expired disk token. - let result = source.bearer().await; - assert!(result.is_err()); - let err_msg = format!("{}", result.unwrap_err()); - assert!( - err_msg.contains("oauth discovery"), - "expected discovery error, got: {err_msg}" - ); + // bearer() is a Headless acquisition: past the cache checks with no + // refresh token, it returns terminally instead of opening a browser. + // With no refresh token it never even discovers endpoints, so the + // unreachable URL is never contacted — the error is a graceful + // LlmAuth, not a hard Llm/discovery error. + match source.bearer().await.unwrap_err() { + AgentError::LlmAuth(_) => {} // correct: terminal, no browser + other => panic!("expected terminal LlmAuth, got: {other:?}"), + } } - /// `try_bearer_no_browser` with an empty cache and no refresh token must + /// `bearer_no_browser` with an empty cache and no refresh token must /// return `LlmAuth` immediately — it must NOT attempt OIDC discovery even - /// when the `discovery_url` is unreachable/invalid. This guards the - /// regression where `endpoints()` was called unconditionally before the - /// refresh-token check, causing an `Llm` error (hard failure) instead of - /// the intended graceful `LlmAuth` fallback. + /// when the `discovery_url` is unreachable/invalid, and must never browse. + /// This guards the regression where `endpoints()` was called + /// unconditionally before the refresh-token check, causing an `Llm` error + /// (hard failure) instead of the intended graceful `LlmAuth` fallback. #[tokio::test] - async fn test_try_bearer_no_browser_empty_cache_no_refresh_returns_llm_auth_without_discovery() - { + async fn test_bearer_no_browser_empty_cache_no_refresh_returns_llm_auth_without_discovery() { let dir = tempfile::tempdir().unwrap(); // Intentionally invalid/unreachable discovery URL — if endpoints() is // called, the test will get an `Llm` error and the assertion below fails. @@ -1069,7 +2629,7 @@ mod tests { // No disk cache file either — dir is empty. - let result = source.try_bearer_no_browser().await; + let result = source.bearer_no_browser().await; assert!(result.is_err(), "expected Err, got Ok"); match result.unwrap_err() { AgentError::LlmAuth(_) => {} // correct: graceful fallback @@ -1395,4 +2955,311 @@ mod tests { "read_cache followed a symlinked cache path" ); } + + // ---- cross-process advisory lock primitive -------------------------- + // + // The full 165s waiter bound (`LOCK_WAIT_TIMEOUT`) is not exercisable in a + // unit test, so these drive `acquire_auth_lock` with explicit deadlines to + // pin the three properties the coordinator relies on: a contended waiter + // times out (never blocks forever), a timeout leaves the *holder* + // untouched (never cancels the in-flight attempt), and releasing the + // holder — the RAII stand-in for a crashed process — lets a successor + // proceed with no wedge and no lock-breaking. + + #[tokio::test] + async fn test_lock_wait_times_out_and_leaves_holder_untouched() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cache.json.lock"); + + // Holder takes the lock with a generous deadline. + let holder = acquire_auth_lock(&path, Instant::now() + Duration::from_secs(30)) + .await + .expect("holder should acquire the free lock"); + + // A waiter with an already-lapsed deadline must give up with + // LockTimeout rather than block — this is the deadline-aware polling + // that replaces a blocking `lock()`. + let waiter = acquire_auth_lock(&path, Instant::now()).await; + assert!( + matches!(waiter, Err(AuthError::LockTimeout)), + "contended waiter past its deadline must return LockTimeout, got {waiter:?}" + ); + + // The timeout did not cancel or steal the holder: a second immediate + // waiter still cannot acquire, proving the holder is intact. + let still_held = acquire_auth_lock(&path, Instant::now()).await; + assert!( + matches!(still_held, Err(AuthError::LockTimeout)), + "holder must remain intact after a waiter times out, got {still_held:?}" + ); + + drop(holder); + } + + #[tokio::test] + async fn test_lock_timeout_leaves_cooldown_sidecar_byte_for_byte_untouched() { + let dir = tempfile::tempdir().unwrap(); + let lock_path = dir.path().join("cache.json.lock"); + let cooldown_path = dir.path().join("cache.json.cooldown"); + + // A pre-existing cooldown sidecar written by an earlier interactive + // failure. A waiter that can't take the lock must return before any + // code that reads/clears/writes the cooldown, so these exact bytes + // survive untouched — otherwise a lock-contended caller could clear a + // live suppression and let the next Auto caller re-pop a browser. + let original = br#"{"code":"denied","until":9999999999}"#; + fs::write(&cooldown_path, original).unwrap(); + + // Holder owns the lock (RAII stand-in for another live process). + let holder = acquire_auth_lock(&lock_path, Instant::now() + Duration::from_secs(30)) + .await + .expect("holder should acquire the free lock"); + + // A waiter past its deadline gives up with LockTimeout — the `?` in + // `acquire_leader` propagates this before `acquire_locked` (which owns + // every sidecar mutation) is ever entered. + let waiter = acquire_auth_lock(&lock_path, Instant::now()).await; + assert!( + matches!(waiter, Err(AuthError::LockTimeout)), + "contended waiter past its deadline must return LockTimeout, got {waiter:?}" + ); + + let after = fs::read(&cooldown_path).unwrap(); + assert_eq!( + after.as_slice(), + original.as_slice(), + "a lock timeout must leave the cooldown sidecar byte-for-byte untouched" + ); + + drop(holder); + } + + /// **Awaited reconciliation is falsifiable — `lock().await` cannot regress to `try_lock`.** + /// + /// Deterministic direct-poll proof: the test task holds B's state mutex and + /// manually polls a pinned real `acquire()` future at each state transition, + /// without spawning a task or relying on scheduler ordering. + /// + /// Proof sequence: + /// 1. Seed B's state with stale X; register an unpublished slot. + /// 2. Hold B's state mutex — blocks the fast-path `try_lock` so B falls + /// through to the registry, and will block `lock().await` when B tries + /// to reconcile after waking. + /// 3. Poll B's `acquire()` once: no prior async suspension on the joiner + /// path, so B reaches `slot.wait()`'s inner `rx.changed().await` and + /// parks — the poll returns `Pending`. This is a structural proof, not a + /// scheduler assumption. + /// 4. Publish Y and poll the same future again while the state mutex is + /// still held. `slot.wait()` wakes and returns; B calls + /// `state.lock().await`, which must park because we hold the mutex → + /// this poll returns `Pending`. + /// Mutation check: with `try_lock()` the adopt block is skipped and B + /// returns immediately → this poll returns `Ready(Ok("token-Y"))`, + /// failing the `Pending` assertion. + /// 5. Release the state guard; poll to completion (or `await` the future) + /// and assert the result is `Ok("token-Y")`. + /// 6. Assert a subsequent plain `acquire(None)` returns Y from the + /// in-memory cache — the P1 contract. + /// Mutation check: `try_lock` leaves state == stale X, so this acquire + /// returns X — the exact P1 stale-credential regression. + #[tokio::test] + async fn test_joiner_reconciliation_blocked_until_state_lock_released() { + use std::future::Future as _; + use std::pin::pin; + use std::task::{Context, Poll, Waker}; + + let dir = tempfile::tempdir().unwrap(); + let b = PkceOAuthTokenSource::new(PkceOAuthConfig { + discovery_url: "https://invalid.example.test/.well-known".into(), + client_id: "test-client".into(), + scopes: vec!["offline_access".into()], + cache_namespace: "test".into(), + cache_dir_override: Some(dir.path().to_path_buf()), + }) + .unwrap(); + + let future_exp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 7200; + let make_token = |access: &str| CachedToken { + access_token: access.into(), + refresh_token: Some("rt".into()), + expires_at: Some(future_exp), + }; + + let token_x = make_token("token-X"); // B's stale/rejected credential. + let token_y = make_token("token-Y"); // shared leader result — must replace X. + + // Seed B's state with stale X. + { + let mut state = b.state.lock().await; + *state = Some(token_x.clone()); + } + + // Register an unpublished slot so B will join it. + let key: InflightKey = (b.lock_path(), AuthIntent::Headless); + let slot = Arc::new(InflightSlot::new()); + inflight_registry().insert(key.clone(), slot.clone()); + + // Hold B's state mutex. + // (a) The fast-path `try_lock` fails → B falls through to the joiner path. + // (b) `state.lock().await` during reconciliation will block until we drop. + let state_guard = b.state.lock().await; + + // Pin B's acquire() future in this stack frame for manual polling. + let mut b_fut = pin!(b.acquire(AuthIntent::Headless, Some("token-X"))); + let waker = Waker::noop(); + let mut cx = Context::from_waker(waker); + + // Poll 1: B has no async suspension before `slot.wait()`'s inner + // `rx.changed().await`. The slot is unpublished, so `changed()` parks. + // Result must be Pending — structural proof that B reached slot.wait(). + assert!( + matches!(b_fut.as_mut().poll(&mut cx), Poll::Pending), + "poll 1 must be Pending: B is parked at slot.wait() awaiting publication" + ); + + // Publish Y. `rx.changed()` wakes; on the next poll B exits slot.wait(), + // enters reconciliation, and calls `state.lock().await`. + slot.publish(None, Ok(token_y.clone())); + inflight_registry().remove(&key); + + // Poll 2: `slot.wait()` returns Y; B calls `state.lock().await`. + // With `lock().await`: the mutex is held → parks → Pending. + // Mutation (`try_lock`): try_lock fails → adopt skipped → B returns + // Ok("token-Y") immediately → Ready, not Pending. + // + // This poll is the exact mutation discriminator: Ready here is the + // bug (B completed without awaited reconciliation). + assert!( + matches!(b_fut.as_mut().poll(&mut cx), Poll::Pending), + "poll 2 must be Pending: B must not return while state mutex is held — \ + mutation check: `try_lock()` returns Ready here, proving early completion \ + without reconciliation (the P1 regression)" + ); + + // Release the mutex. B acquires the lock, evaluates the adoption + // predicate (state == stale X, matches the rejected token), writes Y, + // and returns Ok("token-Y"). + drop(state_guard); + + // Await completion (B now owns the mutex). + let result = b_fut.await; + assert_eq!( + result, + Ok("token-Y".to_string()), + "B must return the shared token Y after reconciliation completes" + ); + + // Subsequent plain acquire must return Y from the in-memory cache — + // the P1 contract. With the `try_lock` mutation, state still holds X + // and this acquire returns X (stale-credential regression). + let rb_next = b + .acquire(AuthIntent::Headless, None) + .await + .expect("subsequent acquire must return Y from in-memory state"); + assert_eq!( + rb_next, "token-Y", + "subsequent in-memory read must return Y, not stale X — \ + mutation check: `try_lock()` leaves state == X, returning X" + ); + } + + /// **Preserve-distinct-newer — reconciliation must not overwrite B's valid credential.** + /// + /// B already holds a valid, usable token Z (distinct from rejected X and from the + /// leader's shared result Y) in its `state` when the joiner reconciliation runs. + /// The adoption predicate must evaluate to false for Z and leave it in place. + /// + /// Deterministic setup via direct polling: register an unpublished slot; poll + /// B's `acquire()` once to park it at `slot.wait()`; write Z into B's state; + /// publish Y and await completion. No scheduler inference or `yield_now()`. + /// + /// Mutation check (unconditional adoption): if the reconciliation block writes + /// `*state = Some(token.clone())` unconditionally, Z is overwritten with Y. + /// The subsequent state assertion `state == Z` FAILS — proving the predicate + /// is load-bearing. + #[tokio::test] + async fn test_joiner_preserve_distinct_newer_credential() { + use std::future::Future as _; + use std::pin::pin; + use std::task::{Context, Poll, Waker}; + + let dir = tempfile::tempdir().unwrap(); + let b = PkceOAuthTokenSource::new(PkceOAuthConfig { + discovery_url: "https://invalid.example.test/.well-known".into(), + client_id: "test-client".into(), + scopes: vec!["offline_access".into()], + cache_namespace: "test".into(), + cache_dir_override: Some(dir.path().to_path_buf()), + }) + .unwrap(); + + let future_exp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 7200; + let make_token = |access: &str| CachedToken { + access_token: access.into(), + refresh_token: Some("rt".into()), + expires_at: Some(future_exp), + }; + + let token_z = make_token("token-Z"); // B's distinct, independently acquired credential. + let token_y = make_token("token-Y"); // leader's shared result — must NOT overwrite Z. + + // Register a not-yet-published slot so B will join it and wait. + // B starts with empty state so its fast-path cache miss is guaranteed. + let key: InflightKey = (b.lock_path(), AuthIntent::Headless); + let slot = Arc::new(InflightSlot::new()); + inflight_registry().insert(key.clone(), slot.clone()); + + // Pin B's future and poll once to park it at slot.wait(). + // No async suspension precedes slot.wait() on the joiner path, so the + // first poll is the structural proof that B is parked there. + let mut b_fut = pin!(b.acquire(AuthIntent::Headless, Some("token-X"))); + let waker = Waker::noop(); + let mut cx = Context::from_waker(waker); + assert!( + matches!(b_fut.as_mut().poll(&mut cx), Poll::Pending), + "B must park at slot.wait() on the first poll" + ); + + // B is now suspended in slot.wait(). Write Z into B's state — this is an + // intervening write that B will observe when it evaluates the + // reconciliation predicate after waking. + { + let mut state = b.state.lock().await; + *state = Some(token_z.clone()); + } + + // Publish Y to wake B. B will call lock().await, see Z (not expired, not + // matching "token-X"), evaluate the predicate as false, and preserve Z. + slot.publish(None, Ok(token_y.clone())); + inflight_registry().remove(&key); + + let result = b_fut.await; + + assert_eq!( + result, + Ok("token-Y".to_string()), + "B must still receive the shared bearer Y" + ); + + // B.state must still hold Z — the adoption predicate correctly skipped + // the write because Z is usable and distinct from the rejected token. + { + let state = b.state.lock().await; + assert_eq!( + state.as_ref().map(|t| t.access_token.as_str()), + Some("token-Z"), + "B.state must not be overwritten when it holds a distinct usable credential — \ + mutation check: fails if reconciliation is unconditional \ + (overwrites Z with Y regardless of predicate)" + ); + } + } } diff --git a/crates/buzz-agent/tests/bin/auth_worker.rs b/crates/buzz-agent/tests/bin/auth_worker.rs new file mode 100644 index 00000000000..5a4b76d2866 --- /dev/null +++ b/crates/buzz-agent/tests/bin/auth_worker.rs @@ -0,0 +1,252 @@ +//! Test-only helper: a real second process that runs the PUBLIC auth +//! coordinator (`PkceOAuthTokenSource::acquire_with_intent`) against a shared +//! temp cache, so the auth tests can prove the *cross-process* single-flight +//! contract end-to-end rather than with two in-process handles. +//! +//! The in-process `INFLIGHT` registry coalesces same-key callers within one +//! process before they ever reach the file lock, so two `PkceOAuthTokenSource` +//! instances in one test do NOT exercise the cross-process protocol (the OS +//! advisory lock and the on-disk cache re-read). This binary is a genuine +//! second process: it contends on the same `flock`/`LockFileEx` and reads/writes +//! the same private cache file the parent coordinator does. +//! +//! The browser step is scripted (no real window): the opener drives the +//! loopback callback exactly as a real browser would, and its launch count is +//! reported back so a test can assert "exactly one browser across processes". +//! +//! Env contract (all required unless noted): +//! AUTH_WORKER_DISCOVERY_URL — OIDC discovery URL (the parent stub). +//! AUTH_WORKER_CACHE_DIR — shared cache dir (`cache_dir_override`). +//! AUTH_WORKER_NAMESPACE — cache namespace. +//! AUTH_WORKER_CLIENT_ID — OAuth client id. +//! AUTH_WORKER_SCOPES — comma-separated scopes. +//! AUTH_WORKER_INTENT — auto | userinitiated | headless. +//! AUTH_WORKER_SCRIPT — approve | deny | failopen. +//! AUTH_WORKER_RESULT — path to write the JSON outcome to. +//! AUTH_WORKER_REJECTED — (optional) rejected token bytes passed to +//! `acquire_with_intent`; absent means no rejection. +//! AUTH_WORKER_READY_MARKER — (optional) written once the source is built, +//! before acquisition, so the parent can release +//! several workers into a genuine lock race. +//! AUTH_WORKER_START_MARKER — (optional) acquisition blocks until this file +//! exists, so multiple workers begin together. +//! AUTH_WORKER_LAUNCHED_MARKER — (optional) written when the browser opener +//! fires (i.e. this process holds the lock and is +//! mid-flow), so the parent can queue behind it. +//! AUTH_WORKER_PROCEED_MARKER — (optional) the scripted callback is withheld +//! until this file exists, so the parent can +//! confirm another process is already waiting on +//! the lock before this one resolves. +//! AUTH_WORKER_SNAPSHOT_MARKER — (optional) a file path; when set, a tracing +//! layer intercepts the `acquire_leader_snapshot` +//! event emitted by `auth.rs` after the attempt- +//! generation snapshot is taken (and before the +//! cross-process lock is acquired) and writes this +//! file once. Lets the parent observe that this +//! process has committed its snapshot-gen and is +//! about to queue on the lock. +//! +//! Result JSON: `{ "result": "ok"|"", "bearer": , +//! "launches": }`. + +use std::fs; +use std::io::{Read, Write}; +use std::net::TcpStream; +use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::Arc; +use std::time::Duration; + +use buzz_agent::auth::{AuthIntent, BrowserOpener, PkceOAuthConfig, PkceOAuthTokenSource}; +use tracing_subscriber::layer::SubscriberExt; +use tracing_subscriber::util::SubscriberInitExt; + +/// Tracing layer that writes a file once when it sees the +/// `buzz_agent::auth::acquire_leader_snapshot` event emitted by +/// `acquire_leader` immediately after the attempt-generation snapshot is fixed +/// and before the cross-process lock is acquired. Installed only when +/// `AUTH_WORKER_SNAPSHOT_MARKER` is set, so normal test runs incur no overhead. +struct SnapshotMarkerLayer { + path: PathBuf, + written: AtomicBool, +} + +impl tracing_subscriber::Layer for SnapshotMarkerLayer { + fn on_event( + &self, + event: &tracing::Event<'_>, + _ctx: tracing_subscriber::layer::Context<'_, S>, + ) { + if event.metadata().target() == "buzz_agent::auth::acquire_leader_snapshot" + && !self.written.swap(true, Ordering::SeqCst) + { + let _ = fs::write(&self.path, b"snapshotted"); + } + } +} + +/// What the scripted "user" does when the coordinator opens a browser. +#[derive(Clone, Copy)] +enum Script { + Approve, + Deny, + FailToOpen, +} + +/// A [`BrowserOpener`] that counts launches and drives the loopback callback on +/// a background thread — the same technique as the in-crate test opener, but +/// with two optional cross-process barriers so the parent can order events: +/// `launched_marker` announces that this process holds the lock and has opened +/// the browser, and `proceed_marker` withholds the callback until the parent +/// signals it has queued another process behind the lock. +struct WorkerOpener { + script: Script, + calls: Arc, + launched_marker: Option, + proceed_marker: Option, +} + +impl BrowserOpener for WorkerOpener { + fn open(&self, url: &str) -> Result<(), String> { + self.calls.fetch_add(1, Ordering::SeqCst); + if let Some(marker) = &self.launched_marker { + fs::write(marker, b"launched").expect("write launched marker"); + } + let query = match self.script { + Script::FailToOpen => return Err("no browser available".into()), + Script::Approve => "code=scripted-code", + Script::Deny => "error=access_denied", + }; + let parsed = url::Url::parse(url).expect("authorize URL must parse"); + let redirect = parsed + .query_pairs() + .find(|(k, _)| k == "redirect_uri") + .map(|(_, v)| v.into_owned()) + .expect("authorize URL carries redirect_uri"); + let state = parsed + .query_pairs() + .find(|(k, _)| k == "state") + .map(|(_, v)| v.into_owned()) + .expect("authorize URL carries state"); + let redirect = url::Url::parse(&redirect).expect("redirect_uri must parse"); + let port = redirect.port().expect("loopback redirect carries a port"); + let request = format!( + "GET /?{query}&state={state} HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\nConnection: close\r\n\r\n" + ); + let proceed = self.proceed_marker.clone(); + std::thread::spawn(move || { + // Hold the callback until the parent has confirmed another process + // is already queued behind the lock (bounded so a missing signal + // can't wedge the test past the browser timeout). + if let Some(marker) = proceed { + for _ in 0..6000 { + if marker.exists() { + break; + } + std::thread::sleep(Duration::from_millis(10)); + } + } + if let Ok(mut sock) = TcpStream::connect(("127.0.0.1", port)) { + let _ = sock.write_all(request.as_bytes()); + let _ = sock.flush(); + let mut discard = Vec::new(); + let _ = sock.read_to_end(&mut discard); + } + }); + Ok(()) + } +} + +fn env(key: &str) -> String { + std::env::var(key).unwrap_or_else(|_| panic!("{key} set")) +} + +#[tokio::main] +async fn main() { + // If the parent test set AUTH_WORKER_SNAPSHOT_MARKER, install a tracing + // subscriber layer that fires when the coordinator emits its pre-lock + // snapshot event and writes the marker file. + if let Ok(marker_path) = std::env::var("AUTH_WORKER_SNAPSHOT_MARKER") { + tracing_subscriber::registry() + .with(SnapshotMarkerLayer { + path: PathBuf::from(marker_path), + written: AtomicBool::new(false), + }) + .init(); + } + + let intent = match env("AUTH_WORKER_INTENT").as_str() { + "auto" => AuthIntent::Auto, + "userinitiated" => AuthIntent::UserInitiated, + "headless" => AuthIntent::Headless, + other => panic!("unknown AUTH_WORKER_INTENT: {other}"), + }; + let script = match env("AUTH_WORKER_SCRIPT").as_str() { + "approve" => Script::Approve, + "deny" => Script::Deny, + "failopen" => Script::FailToOpen, + other => panic!("unknown AUTH_WORKER_SCRIPT: {other}"), + }; + let result_path = PathBuf::from(env("AUTH_WORKER_RESULT")); + let start_marker = std::env::var("AUTH_WORKER_START_MARKER") + .ok() + .map(PathBuf::from); + let ready_marker = std::env::var("AUTH_WORKER_READY_MARKER") + .ok() + .map(PathBuf::from); + + let calls = Arc::new(AtomicU64::new(0)); + let opener = WorkerOpener { + script, + calls: calls.clone(), + launched_marker: std::env::var("AUTH_WORKER_LAUNCHED_MARKER") + .ok() + .map(PathBuf::from), + proceed_marker: std::env::var("AUTH_WORKER_PROCEED_MARKER") + .ok() + .map(PathBuf::from), + }; + + let cfg = PkceOAuthConfig { + discovery_url: env("AUTH_WORKER_DISCOVERY_URL"), + client_id: env("AUTH_WORKER_CLIENT_ID"), + scopes: env("AUTH_WORKER_SCOPES") + .split(',') + .map(str::to_owned) + .collect(), + cache_namespace: env("AUTH_WORKER_NAMESPACE"), + cache_dir_override: Some(PathBuf::from(env("AUTH_WORKER_CACHE_DIR"))), + }; + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener)).expect("build token source"); + + // Announce readiness, then wait for the parent's release so several workers + // hit the lock together — a genuine race rather than staggered spawns. + if let Some(marker) = &ready_marker { + fs::write(marker, b"ready").expect("write ready marker"); + } + if let Some(marker) = start_marker { + for _ in 0..6000 { + if marker.exists() { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + } + + let (result, bearer) = match src + .acquire_with_intent( + intent, + std::env::var("AUTH_WORKER_REJECTED").ok().as_deref(), + ) + .await + { + Ok(token) => ("ok".to_owned(), Some(token)), + Err(e) => (e.code().to_owned(), None), + }; + let body = serde_json::json!({ + "result": result, + "bearer": bearer, + "launches": calls.load(Ordering::SeqCst), + }); + fs::write(&result_path, serde_json::to_vec(&body).unwrap()).expect("write result file"); +} diff --git a/crates/buzz-agent/tests/bin/lock_holder.rs b/crates/buzz-agent/tests/bin/lock_holder.rs new file mode 100644 index 00000000000..275503a762c --- /dev/null +++ b/crates/buzz-agent/tests/bin/lock_holder.rs @@ -0,0 +1,50 @@ +//! Test-only helper: a real second process that takes the coordinator's +//! cross-process advisory lock and holds it until killed. +//! +//! The auth coordinator single-flights per cache key on an `fs2` advisory lock +//! (`flock` on Unix, `LockFileEx` on Windows). To prove the *cross-process* +//! contract — a genuine other process serializes the flow, and its death +//! releases the lock with no PID files or lock-breaking — a test needs an +//! actual separate process on the same lock file, not a second in-process +//! handle. This binary is that process. +//! +//! Driven by two env vars: +//! LOCK_HELPER_PATH — the lock file to acquire (the coordinator's +//! `.json.lock`). +//! LOCK_HELPER_READY — a marker file created *after* the lock is held, so +//! the parent test can synchronize on ownership before +//! racing the coordinator. +//! +//! After signaling readiness it blocks forever; the parent kills it to model a +//! crash mid-flow. + +use std::fs; + +use fs2::FileExt; + +fn main() { + let lock_path = std::env::var("LOCK_HELPER_PATH").expect("LOCK_HELPER_PATH set"); + let ready_path = std::env::var("LOCK_HELPER_READY").expect("LOCK_HELPER_READY set"); + + if let Some(parent) = std::path::Path::new(&lock_path).parent() { + fs::create_dir_all(parent).expect("create lock parent dir"); + } + // Open exactly as the coordinator does so we contend on the same inode. + let file = fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&lock_path) + .expect("open lock file"); + file.lock_exclusive() + .expect("hold the exclusive advisory lock"); + + // Signal ownership only once the lock is truly held. + fs::write(&ready_path, b"held").expect("write ready marker"); + + // Hold the lock until the parent kills us (crash stand-in). The kernel + // releases the advisory lock on process death. + loop { + std::thread::sleep(std::time::Duration::from_secs(3600)); + } +} diff --git a/crates/buzz-agent/tests/databricks_auth_coordinator.rs b/crates/buzz-agent/tests/databricks_auth_coordinator.rs new file mode 100644 index 00000000000..0937cc87c1d --- /dev/null +++ b/crates/buzz-agent/tests/databricks_auth_coordinator.rs @@ -0,0 +1,3410 @@ +//! Concurrency-matrix tests for the Databricks auth coordinator. +//! +//! The coordinator single-flights OAuth acquisition per cache key. Within one +//! process, same-key callers coalesce on an in-memory `INFLIGHT` registry +//! *before* the file lock; across processes, they serialize on an OS advisory +//! lock and share success through the on-disk cache, with failures coalesced +//! through a durable cooldown sidecar. These tests drive the public API +//! (`acquire_with_intent`, `interactive_login`) with an injected +//! [`BrowserOpener`] that scripts the localhost callback instead of popping a +//! real window — the browser step becomes deterministic and countable. +//! +//! Two `PkceOAuthTokenSource` instances in ONE process do not model two +//! processes: the `INFLIGHT` registry intercepts them before the file lock, so +//! same-process tests exercise the in-memory single-flight, not the +//! cross-process protocol. The genuinely cross-process claims — lock +//! contention, crash release, cooldown sharing across a process boundary, and +//! one-grant/one-cache under a real race — are proved with the `lock-holder` +//! and `auth-worker` helper binaries, each a real second process on the same +//! lock file and cache. The lock-primitive and lock-timeout edges live in the +//! in-crate `auth::tests` module where the private helpers are reachable. + +use std::io::Write; +use std::net::{SocketAddr, TcpStream}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use axum::extract::Form; +use axum::{routing::get, routing::post, Json, Router}; +use buzz_agent::auth::{ + AuthError, AuthIntent, BrowserOpener, PkceOAuthConfig, PkceOAuthTokenSource, +}; +use serde::Deserialize; +use serde_json::json; +use tempfile::TempDir; + +// ---- scripted browser opener -------------------------------------------- + +/// What the scripted "user" does when the coordinator opens a browser. +#[derive(Clone, Copy)] +enum Script { + /// Redirect with a valid `code`+`state` → the flow exchanges it for a + /// token and succeeds. + Approve, + /// Redirect with `error=access_denied` → the flow returns `Denied`. + Deny, + /// Every launch strategy fails → the flow returns `BrowserOpenFailed` + /// without waiting on a listener nobody will reach. + FailToOpen, +} + +/// A [`BrowserOpener`] that counts launches and drives the localhost callback +/// on a background thread, so the caller's callback wait observes the redirect +/// exactly as a real browser would deliver it. +#[derive(Clone)] +struct ScriptedOpener { + script: Script, + calls: Arc, +} + +impl ScriptedOpener { + fn new(script: Script) -> Self { + Self { + script, + calls: Arc::new(AtomicU64::new(0)), + } + } + + fn call_count(&self) -> u64 { + self.calls.load(Ordering::SeqCst) + } +} + +impl BrowserOpener for ScriptedOpener { + fn open(&self, url: &str) -> Result<(), String> { + self.calls.fetch_add(1, Ordering::SeqCst); + let query = match self.script { + Script::FailToOpen => return Err("no browser available".into()), + Script::Approve => "code=scripted-code", + Script::Deny => "error=access_denied", + }; + // Pull the loopback redirect target and the anti-CSRF state out of the + // authorize URL, then fire the callback from a separate thread so this + // synchronous `open()` returns and the flow proceeds to await it. + let parsed = url::Url::parse(url).expect("authorize URL must parse"); + let redirect = parsed + .query_pairs() + .find(|(k, _)| k == "redirect_uri") + .map(|(_, v)| v.into_owned()) + .expect("authorize URL carries redirect_uri"); + let state = parsed + .query_pairs() + .find(|(k, _)| k == "state") + .map(|(_, v)| v.into_owned()) + .expect("authorize URL carries state"); + let redirect = url::Url::parse(&redirect).expect("redirect_uri must parse"); + // The coordinator's listener binds 127.0.0.1; connect there directly so + // the callback can't land on an IPv6 `localhost` (::1) with no listener. + let port = redirect.port().expect("loopback redirect carries a port"); + // `state` is base64url (no reserved characters), safe to inline. + let request = format!( + "GET /?{query}&state={state} HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\nConnection: close\r\n\r\n" + ); + std::thread::spawn(move || { + // A real browser holds the connection open until the callback page + // responds; do the same so hyper dispatches the request before the + // socket closes (a bare write+drop races the server and is lost). + if let Ok(mut sock) = TcpStream::connect(("127.0.0.1", port)) { + use std::io::Read; + let _ = sock.write_all(request.as_bytes()); + let _ = sock.flush(); + let mut discard = Vec::new(); + let _ = sock.read_to_end(&mut discard); + } + }); + Ok(()) + } +} + +// ---- stub OIDC provider -------------------------------------------------- + +#[derive(Deserialize)] +struct TokenForm { + grant_type: String, +} + +struct Stub { + base: String, + /// authorization-code exchanges served (browser flows completed). + code_grants: Arc, + /// refresh-token grants served. + refresh_grants: Arc, +} + +/// How the stub's token endpoint answers a `refresh_token` grant. Lets a test +/// distinguish the three ways a refresh can fail so it can assert the +/// coordinator classifies each correctly: a `401` is a real credential +/// rejection (dead refresh token), a `500` is a transient provider fault, and +/// a hang models a slow/unreachable provider that must trip the per-request +/// HTTP timeout. Authorization-code grants are never affected. +#[derive(Clone, Copy)] +enum RefreshMode { + /// `200` with a fresh access token. + Succeed, + /// `401 invalid_grant` — the grant itself is rejected. + Reject, + /// `500` — a provider-side fault, transient rather than a credential + /// decision. + /// + /// Used only by Unix-only tests (refresh-error classification). + /// Gated to suppress dead-code warnings on Windows. + #[cfg(unix)] + ServerError, + /// A 4xx with the given OAuth `error` code in the body. Lets a test assert + /// the coordinator treats `invalid_grant` (any 4xx) as a dead grant, but + /// every other error code — and any non-`invalid_grant` status like `429` + /// — as infrastructural rather than a credential rejection. + /// + /// Used only by Unix-only tests (refresh-error classification). + /// Gated to suppress dead-code warnings on Windows. + #[cfg(unix)] + ClientError(axum::http::StatusCode, &'static str), + /// Sleep `d` before answering, so the caller's per-request HTTP timeout + /// elapses first (a transport timeout, not a verdict from the provider). + /// + /// Used only by Unix-only tests (refresh-timeout classification). + /// Gated to suppress dead-code warnings on Windows. + #[cfg(unix)] + Hang(Duration), + /// `200` returning the same fixed access token on every grant, regardless + /// of how many are served. Models a provider that re-issues an identical + /// access token, so a bounded rerun can hand back the exact bytes the + /// caller already reported 401-rejected. + /// + /// Used only by Unix-only tests (rejected-token neutralization, sticky + /// reissuance). Gated to suppress dead-code warnings on Windows. + #[cfg(unix)] + SucceedSticky(&'static str), +} + +/// How the stub's token endpoint answers an `authorization_code` grant (the +/// browser code exchange). Lets a test drive the exchange classifier: a +/// `401 invalid_grant` is a genuine rejected code (`ExchangeFailed`), while a +/// `429`, a `500`, and a malformed `200` are transient/provider faults that +/// must classify as `NetworkUnavailable` rather than poisoning the cooldown. +#[derive(Clone, Copy)] +enum ExchangeMode { + /// `200` with a fresh access token — the browser flow completes. + Succeed, + /// A failing status carrying the given OAuth `error` body. Only a 4xx + /// `invalid_grant` is a true code rejection; every other status/error is + /// infrastructural. + Fail(axum::http::StatusCode, &'static str), + /// `200` whose body lacks an `access_token` — a malformed success the + /// provider should never send, so it is a fault, not a rejected code. + MalformedSuccess, + /// Sleep `d` before answering, so the caller's per-request HTTP timeout + /// elapses first (a transport timeout, not a verdict from the provider). + Hang(Duration), + /// `200` returning the same fixed access token on every authorization-code + /// exchange. Models a provider that re-issues an identical access token, so + /// a browser sign-in (reached after a dead refresh) can hand back the exact + /// bytes the caller reported 401-rejected. + /// + /// Used only by Unix-only tests (sticky browser exchange after dead refresh). + /// Gated to suppress dead-code warnings on Windows. + #[cfg(unix)] + SucceedSticky(&'static str), +} + +/// Boot a stub provider. `reject_refresh` makes the token endpoint 401 every +/// refresh-token grant (a dead refresh token); authorization-code grants +/// always succeed with a fresh token. +async fn spawn_stub(reject_refresh: bool) -> Stub { + spawn_stub_with(if reject_refresh { + RefreshMode::Reject + } else { + RefreshMode::Succeed + }) + .await +} + +/// Boot a stub provider whose refresh-token grant follows `mode`. Discovery and +/// authorization-code grants always succeed instantly regardless of `mode`. +async fn spawn_stub_with(mode: RefreshMode) -> Stub { + spawn_stub_with_modes(mode, ExchangeMode::Succeed).await +} + +/// Boot a stub whose authorization-code exchange follows `exchange`. Refresh +/// grants succeed; used by the exchange-classifier tests. +async fn spawn_stub_with_exchange(exchange: ExchangeMode) -> Stub { + spawn_stub_with_modes(RefreshMode::Succeed, exchange).await +} + +/// Boot a stub provider whose refresh-token grant follows `refresh` and whose +/// authorization-code grant follows `exchange`. Discovery always succeeds. +async fn spawn_stub_with_modes(refresh: RefreshMode, exchange: ExchangeMode) -> Stub { + let code_grants = Arc::new(AtomicU64::new(0)); + let refresh_grants = Arc::new(AtomicU64::new(0)); + + let listener = tokio::net::TcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))) + .await + .unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let disco_base = base.clone(); + + let discovery = move || { + let base = disco_base.clone(); + async move { + Json(json!({ + "authorization_endpoint": format!("{base}/authorize"), + "token_endpoint": format!("{base}/token"), + })) + } + }; + + let code_for_token = code_grants.clone(); + let refresh_for_token = refresh_grants.clone(); + let app = Router::new() + // Two discovery paths so distinct-host tests derive distinct cache + // keys (the key hashes the discovery URL) from one stub. + .route("/disco/a", get(discovery.clone())) + .route("/disco/b", get(discovery)) + .route( + "/token", + post(move |Form(form): Form| { + let code_grants = code_for_token.clone(); + let refresh_grants = refresh_for_token.clone(); + let refresh = refresh; + let exchange = exchange; + async move { + if form.grant_type == "refresh_token" { + let n = refresh_grants.fetch_add(1, Ordering::SeqCst) + 1; + // A hang delays the answer so the caller's per-request + // HTTP timeout can elapse first (transport timeout, not + // a credential decision). + #[cfg(unix)] + if let RefreshMode::Hang(d) = refresh { + tokio::time::sleep(d).await; + } + return match refresh { + RefreshMode::Reject => ( + axum::http::StatusCode::UNAUTHORIZED, + Json(json!({ "error": "invalid_grant" })), + ), + #[cfg(unix)] + RefreshMode::ServerError => ( + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + Json(json!({ "error": "temporarily_unavailable" })), + ), + #[cfg(unix)] + RefreshMode::ClientError(status, error) => { + (status, Json(json!({ "error": error }))) + } + RefreshMode::Succeed => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": format!("refreshed-token-{n}"), + "refresh_token": "rotated-refresh", + "expires_in": 3600, + })), + ), + #[cfg(unix)] + RefreshMode::Hang(_) => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": format!("refreshed-token-{n}"), + "refresh_token": "rotated-refresh", + "expires_in": 3600, + })), + ), + #[cfg(unix)] + RefreshMode::SucceedSticky(tok) => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": tok, + "refresh_token": "rotated-refresh", + "expires_in": 3600, + })), + ), + }; + } + let n = code_grants.fetch_add(1, Ordering::SeqCst) + 1; + // A hang delays the answer so the caller's per-request HTTP + // timeout can elapse first (transport timeout, not a code + // decision), mirroring the refresh path above. + if let ExchangeMode::Hang(d) = exchange { + tokio::time::sleep(d).await; + } + match exchange { + ExchangeMode::Succeed => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": format!("browser-token-{n}"), + "refresh_token": "browser-refresh", + "expires_in": 3600, + })), + ), + ExchangeMode::Fail(status, error) => { + (status, Json(json!({ "error": error }))) + } + ExchangeMode::MalformedSuccess => ( + axum::http::StatusCode::OK, + Json(json!({ "token_type": "bearer" })), + ), + #[cfg(unix)] + ExchangeMode::SucceedSticky(tok) => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": tok, + "refresh_token": "browser-refresh", + "expires_in": 3600, + })), + ), + // Reached only after the sleep above; answer as a + // success the caller has already abandoned. + ExchangeMode::Hang(_) => ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": format!("browser-token-{n}"), + "refresh_token": "browser-refresh", + "expires_in": 3600, + })), + ), + } + } + }), + ); + + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + Stub { + base, + code_grants, + refresh_grants, + } +} + +/// Control handle for a stub whose refresh response is held until the parent +/// explicitly releases it. Used by the cross-process digest test to establish +/// deterministic ordering: the parent waits for `request_received` (proves A +/// holds the lock and is mid-refresh), then spawns B, waits for B's snapshot +/// marker, and finally calls `release()` before joining both workers. +#[cfg(unix)] +struct RefreshGate { + /// Notified by the stub once it has received the first refresh request. + request_received: Arc, + /// Parent signals this to let the stub return the response. + proceed: Arc, +} + +#[cfg(unix)] +impl RefreshGate { + /// Asynchronously wait until the stub has received A's refresh request. + async fn wait_for_request(&self) { + self.request_received.notified().await; + } + + /// Release the held refresh response so the stub replies to A. + fn release(&self) { + self.proceed.notify_one(); + } +} + +/// Shape of the refresh response returned by [`spawn_stub_with_held_refresh`]. +/// +/// - `Sticky(tok)` — every refresh returns `200 OK` with `access_token: tok`. +/// - `Reject` — every refresh returns `401 Unauthorized` with `invalid_grant`. +#[cfg(unix)] +enum HeldRefreshResponse { + Sticky(&'static str), + Reject, +} + +/// Spawn a stub that holds the FIRST refresh request until the parent calls +/// [`RefreshGate::release()`], then replies according to `response`. +/// Subsequent refresh requests skip the gate and reply immediately with the +/// same shape. Code-grant (`authorization_code`) requests are always answered +/// immediately with a fresh browser token. +/// +/// Returns the stub (for `refresh_grants` / `code_grants` assertions) and the +/// control gate. Used by the cross-process held-refresh tests. +#[cfg(unix)] +async fn spawn_stub_with_held_refresh(response: HeldRefreshResponse) -> (Stub, RefreshGate) { + let code_grants = Arc::new(AtomicU64::new(0)); + let refresh_grants = Arc::new(AtomicU64::new(0)); + let request_received = Arc::new(tokio::sync::Notify::new()); + let proceed = Arc::new(tokio::sync::Notify::new()); + + let listener = tokio::net::TcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))) + .await + .unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let disco_base = base.clone(); + + let discovery = move || { + let base = disco_base.clone(); + async move { + Json(json!({ + "authorization_endpoint": format!("{base}/authorize"), + "token_endpoint": format!("{base}/token"), + })) + } + }; + + let code_for_token = code_grants.clone(); + let refresh_for_token = refresh_grants.clone(); + let received_for_handler = request_received.clone(); + let proceed_for_handler = proceed.clone(); + // Track whether the first refresh has been released yet. Once the first + // grant is released, subsequent grants return immediately. + let first_released = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let reject = matches!(response, HeldRefreshResponse::Reject); + let sticky_tok = match response { + HeldRefreshResponse::Sticky(tok) => tok, + HeldRefreshResponse::Reject => "", + }; + + let app = Router::new() + .route("/disco/a", get(discovery.clone())) + .route("/disco/b", get(discovery)) + .route( + "/token", + post(move |Form(form): Form| { + let code_grants = code_for_token.clone(); + let refresh_grants = refresh_for_token.clone(); + let received = received_for_handler.clone(); + let proceed = proceed_for_handler.clone(); + let first_released = first_released.clone(); + async move { + if form.grant_type == "refresh_token" { + refresh_grants.fetch_add(1, Ordering::SeqCst); + // Hold only the first refresh request; once released, + // all subsequent requests return immediately. + if !first_released.swap(true, Ordering::SeqCst) { + received.notify_one(); + proceed.notified().await; + } + return if reject { + ( + axum::http::StatusCode::UNAUTHORIZED, + Json(json!({ "error": "invalid_grant" })), + ) + } else { + ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": sticky_tok, + "refresh_token": "rotated-refresh", + "expires_in": 3600, + })), + ) + }; + } + let n = code_grants.fetch_add(1, Ordering::SeqCst) + 1; + ( + axum::http::StatusCode::OK, + Json(json!({ + "access_token": format!("browser-token-{n}"), + "refresh_token": "browser-refresh", + "expires_in": 3600, + })), + ) + } + }), + ); + + tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let stub = Stub { + base, + code_grants, + refresh_grants, + }; + let gate = RefreshGate { + request_received, + proceed, + }; + (stub, gate) +} + +fn config(stub: &Stub, disco_path: &str, cache_dir: &std::path::Path) -> PkceOAuthConfig { + PkceOAuthConfig { + discovery_url: format!("{}{disco_path}", stub.base), + client_id: "test-client".into(), + scopes: vec!["offline_access".into()], + cache_namespace: "databricks".into(), + cache_dir_override: Some(cache_dir.to_path_buf()), + } +} + +fn future_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs() + + 3600 +} + +fn cache_file_path(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path) -> std::path::PathBuf { + use sha2::Digest; + let mut h = sha2::Sha256::new(); + h.update(cfg.discovery_url.as_bytes()); + h.update(b"|"); + h.update(cfg.client_id.as_bytes()); + h.update(b"|"); + h.update(cfg.scopes.join(",").as_bytes()); + let hash = hex::encode(h.finalize()); + cache_dir + .join(&cfg.cache_namespace) + .join(format!("{hash}.json")) +} + +/// The cross-process attempt sidecar path for a config, matching the +/// coordinator's `append_ext(cache_path, "attempt")`. Used by tests that +/// inspect the generation counter directly after a cross-process adoption to +/// verify the adopter did not re-write a new generation. +fn attempt_sidecar_path(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path) -> std::path::PathBuf { + let mut p = cache_file_path(cfg, cache_dir).into_os_string(); + p.push(".attempt"); + p.into() +} + +/// The cross-process advisory lock path for a config, matching the +/// coordinator's `append_ext(cache_path, "lock")`. Used to point the +/// out-of-process lock-holder helper at the exact file the coordinator +/// contends on. +#[cfg(unix)] +fn lock_file_path(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path) -> std::path::PathBuf { + let mut p = cache_file_path(cfg, cache_dir).into_os_string(); + p.push(".lock"); + p.into() +} + +fn seed_cache(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path, body: serde_json::Value) { + let path = cache_file_path(cfg, cache_dir); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, serde_json::to_vec(&body).unwrap()).unwrap(); +} + +// ---- acceptance matrix --------------------------------------------------- + +#[tokio::test] +async fn test_same_key_concurrent_callers_share_one_browser_attempt() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + + // Two independent sources on the same key in ONE process. The in-memory + // INFLIGHT registry coalesces them before the file lock, so this proves the + // in-process single-flight — one leader runs the browser flow, the other + // joins its published result. The genuine cross-process race is + // `test_crossprocess_two_coordinators_race_to_one_grant_and_cache`. + let a = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(opener.clone()), + ) + .unwrap(); + let b = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(opener.clone()), + ) + .unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Auto, None), + b.acquire_with_intent(AuthIntent::Auto, None), + ); + let ta = ra.expect("first caller authenticates"); + let tb = rb.expect("second caller authenticates"); + + // One browser launch, one code exchange, one shared token. + assert_eq!( + opener.call_count(), + 1, + "only one browser attempt for one key" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one authorization-code exchange" + ); + assert_eq!(ta, tb, "both callers observe the same token"); + assert_eq!(ta, "browser-token-1"); +} + +#[tokio::test] +async fn test_denied_then_auto_reads_cooldown_without_second_launch() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Deny); + + let src = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(opener.clone()), + ) + .unwrap(); + + let first = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + first, + Err(AuthError::Denied), + "first Auto attempt is denied" + ); + assert_eq!(opener.call_count(), 1); + + // The denial wrote a cooldown; a subsequent Auto caller reads it and + // returns the recorded outcome instead of popping a second browser. + let second = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + second, + Err(AuthError::Denied), + "queued Auto caller honors the cooldown" + ); + assert_eq!( + opener.call_count(), + 1, + "cooldown suppresses the second browser launch" + ); +} + +#[tokio::test] +async fn test_userinitiated_retry_bypasses_cooldown_and_reopens() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + + // First attempt: denied, writes a cooldown. + let deny_opener = ScriptedOpener::new(Script::Deny); + let denier = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(deny_opener.clone()), + ) + .unwrap(); + assert_eq!( + denier + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await, + Err(AuthError::Denied) + ); + + // The user explicitly retries: UserInitiated bypasses (and clears) the + // cooldown and opens a fresh browser, which now succeeds. + let approve_opener = ScriptedOpener::new(Script::Approve); + let retrier = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(approve_opener.clone()), + ) + .unwrap(); + let token = retrier + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await + .expect("explicit retry re-launches the browser and succeeds"); + assert_eq!(token, "browser-token-1"); + assert_eq!( + approve_opener.call_count(), + 1, + "UserInitiated retry launches despite the prior cooldown" + ); + + // Cooldown cleared on success: a follow-up Auto now sees a valid token, + // never the stale denial. + let auto = retrier.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!(auto, Ok("browser-token-1".to_string())); +} + +#[tokio::test] +async fn test_distinct_hosts_do_not_inherit_cooldown() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + + // Host A is denied and records a cooldown under key A. + let deny_opener = ScriptedOpener::new(Script::Deny); + let host_a = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(deny_opener.clone()), + ) + .unwrap(); + assert_eq!( + host_a.acquire_with_intent(AuthIntent::Auto, None).await, + Err(AuthError::Denied) + ); + + // Host B is a different key (different discovery URL). It must NOT inherit + // A's cooldown: an Auto caller launches its own browser and succeeds. + let approve_opener = ScriptedOpener::new(Script::Approve); + let host_b = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/b", cache.path()), + Arc::new(approve_opener.clone()), + ) + .unwrap(); + let token = host_b + .acquire_with_intent(AuthIntent::Auto, None) + .await + .expect("distinct host is unaffected by another key's cooldown"); + assert_eq!(token, "browser-token-1"); + assert_eq!(approve_opener.call_count(), 1); +} + +#[tokio::test] +async fn test_browser_open_failure_is_typed_and_retryable_by_user() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + + // Every launch strategy fails: the flow reports the typed BrowserOpenFailed + // without waiting on a listener nobody will reach. + let fail_opener = ScriptedOpener::new(Script::FailToOpen); + let failing = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(fail_opener.clone()), + ) + .unwrap(); + let result = failing + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await; + assert_eq!( + result, + Err(AuthError::BrowserOpenFailed), + "a failed launch surfaces as the typed BrowserOpenFailed" + ); + assert_eq!(fail_opener.call_count(), 1); + + // A failed launch writes a cooldown, but a UserInitiated retry bypasses it + // and reopens — a transient "no browser" (e.g. race with a display coming + // up) must never wedge an explicit user sign-in. + let approve_opener = ScriptedOpener::new(Script::Approve); + let retrier = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(approve_opener.clone()), + ) + .unwrap(); + let token = retrier + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await + .expect("explicit retry reopens despite the prior launch failure"); + assert_eq!(token, "browser-token-1"); + assert_eq!(approve_opener.call_count(), 1); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_headless_dead_refresh_returns_refresh_rejected_without_browser() { + let stub = spawn_stub(true).await; // refresh grants 401 + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Expired token WITH a refresh token, but the server rejects the refresh + // grant (dead/rotated). A Headless caller must classify this terminally as + // RefreshRejected and never open a browser. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + result, + Err(AuthError::RefreshRejected), + "Headless dead-refresh is terminal RefreshRejected" + ); + assert_eq!(opener.call_count(), 0, "Headless never opens a browser"); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "the refresh grant was attempted exactly once" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_interactive_dead_refresh_converts_to_browser() { + let stub = spawn_stub(true).await; // refresh grants 401 + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Same dead-refresh seed, but an interactive intent must fall through to a + // browser flow instead of failing terminally. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = src + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await + .expect("interactive intent recovers via the browser"); + assert_eq!(token, "browser-token-1"); + assert_eq!(opener.call_count(), 1, "interactive intent opens a browser"); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + assert_eq!(stub.code_grants.load(Ordering::SeqCst), 1); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_headless_expired_token_live_refresh_recovers_silently() { + let stub = spawn_stub(false).await; // refresh succeeds + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = src + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("live refresh recovers a Headless caller silently"); + assert_eq!(token, "refreshed-token-1"); + assert_eq!(opener.call_count(), 0, "no browser on a live refresh"); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_interactive_login_reuses_valid_cache_without_browser() { + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // A still-valid cached token short-circuits interactive_login: an explicit + // sign-in should not re-prompt when a good token is already present. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "already-valid", + "refresh_token": "rt", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + src.interactive_login() + .await + .expect("interactive_login succeeds off the valid cache"); + assert_eq!( + opener.call_count(), + 0, + "a valid cached token means no browser prompt" + ); +} + +// ---- locally-fresh rejected bearer (401) recovery ------------------------ +// +// The saved-model picker's recovery path: model discovery 401s a bearer that +// still looks locally fresh (its `expires_at` is in the future) and whose +// refresh grant is dead. Passing that exact token as `rejected` makes the +// clock untrustworthy, so the acquisition must not short-circuit on the fresh +// cache. `Auto` and `UserInitiated` then convert to a browser; `Headless` +// stays terminal with `RefreshRejected`. Seeding a *future*-expiry token is +// what distinguishes this from the expired-token refresh path. + +/// Seed a not-yet-expired access token with a (dead) refresh token and return +/// the access token so the caller can pass it as `rejected`. +#[cfg(unix)] +fn seed_fresh_rejectable(cfg: &PkceOAuthConfig, cache_dir: &std::path::Path) -> String { + let access = "fresh-but-rejected"; + seed_cache( + cfg, + cache_dir, + json!({ + "access_token": access, + "refresh_token": "dead-refresh", + "expires_at": future_secs(), + }), + ); + access.to_string() +} + +#[cfg(unix)] +#[tokio::test] +async fn test_auto_rejected_fresh_bearer_with_dead_refresh_launches_browser() { + let stub = spawn_stub(true).await; // refresh grants 401 + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + let rejected = seed_fresh_rejectable(&cfg, cache.path()); + + // The token is locally fresh, so without `rejected` it would be a cache + // hit and never reach the browser. Passing it as rejected forces the + // clock-based hit to fail, the dead refresh to be attempted, and an Auto + // caller to fall through to the browser. + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = src + .acquire_with_intent(AuthIntent::Auto, Some(&rejected)) + .await + .expect("Auto recovers a rejected-but-fresh bearer via the browser"); + assert_eq!(token, "browser-token-1"); + assert_eq!(opener.call_count(), 1, "Auto launches a browser to recover"); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + assert_eq!(stub.code_grants.load(Ordering::SeqCst), 1); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_headless_rejected_fresh_bearer_with_dead_refresh_returns_refresh_rejected() { + let stub = spawn_stub(true).await; // refresh grants 401 + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + let rejected = seed_fresh_rejectable(&cfg, cache.path()); + + // Same locally-fresh rejected seed, but a Headless caller cannot open a + // browser: a dead refresh is terminal RefreshRejected, never a launch. + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src + .acquire_with_intent(AuthIntent::Headless, Some(&rejected)) + .await; + assert_eq!( + result, + Err(AuthError::RefreshRejected), + "Headless dead-refresh on a rejected fresh bearer is terminal" + ); + assert_eq!(opener.call_count(), 0, "Headless never opens a browser"); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); +} + +// ---- refresh transport failures are not credential rejections ------------ +// +// A refresh that never gets a verdict from the token endpoint — a per-request +// timeout, or a 5xx — is infrastructural, not a dead credential. It must +// surface as `NetworkUnavailable` and never pop a browser or return +// `RefreshRejected`, which would misreport a transient fault as a rotated +// token and (for interactive intents) prompt a needless sign-in. + +#[cfg(unix)] +#[tokio::test] +async fn test_refresh_timeout_is_network_unavailable_not_rejected() { + // The token endpoint hangs far longer than the injected per-request HTTP + // timeout, so the refresh call times out at the transport layer with no + // verdict from the provider. A short real-time timeout is injected rather + // than pausing the clock: under `start_paused` tokio auto-advances into + // the timer while the real loopback discovery GET is still in flight, so + // discovery — not the refresh — would trip the timeout, and the refresh + // would never even be attempted. Real time keeps the timeout attached to + // the request that actually hangs, which the `refresh_grants == 1` guard + // below proves. + let stub = spawn_stub_with(RefreshMode::Hang(Duration::from_secs(30))).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Expired token with a refresh token: the coordinator attempts the refresh, + // which hangs past the HTTP timeout. A Headless caller must classify the + // timeout as NetworkUnavailable, not RefreshRejected. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "slow-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with_http_timeout( + cfg, + Arc::new(opener.clone()), + Duration::from_millis(300), + ) + .unwrap(); + let result = src.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "a refresh transport timeout is infrastructural, not a rejection" + ); + assert_eq!( + opener.call_count(), + 0, + "a timed-out refresh never becomes a credential decision" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "the refresh was attempted exactly once before timing out" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_refresh_server_error_is_network_unavailable_not_rejected() { + let stub = spawn_stub_with(RefreshMode::ServerError).await; // refresh 500s + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // A 5xx is a provider-side fault, not a grant rejection: an interactive + // intent must NOT pop a browser off it, and it must surface as + // NetworkUnavailable rather than RefreshRejected. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "server-error-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await; + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "a refresh 5xx is transient, not a credential rejection" + ); + assert_eq!( + opener.call_count(), + 0, + "a provider 5xx must not trigger an interactive browser fallback" + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); +} + +// ---- 4xx classification: only `invalid_grant` is a dead refresh token ----- +// +// RFC 6749 §5.2 uses 400/401 token responses for several `error` codes, but +// only `invalid_grant` means the refresh token is dead. Every other 4xx — +// `invalid_request`, `invalid_client`, `unsupported_grant_type`, +// `invalid_scope`, `408`, `429` — is a request/config/transient fault a +// browser cannot repair, so it must stay infrastructural (`NetworkUnavailable`) +// and never pop a browser. The classifier keys on the OAuth error body, not +// the bare status class. + +#[cfg(unix)] +#[tokio::test] +async fn test_refresh_400_invalid_grant_is_dead_grant_not_network() { + // A 400 (not just 401) carrying `invalid_grant` is still a dead refresh + // token, so a Headless caller must classify it terminally as + // RefreshRejected — proving the decision is the body error, not the status. + let stub = spawn_stub_with(RefreshMode::ClientError( + axum::http::StatusCode::BAD_REQUEST, + "invalid_grant", + )) + .await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + result, + Err(AuthError::RefreshRejected), + "a 400 invalid_grant is a dead refresh token, not infrastructural" + ); + assert_eq!(opener.call_count(), 0, "Headless never opens a browser"); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_refresh_non_invalid_grant_4xx_is_network_unavailable_not_rejected() { + // Every 4xx whose OAuth body is NOT `invalid_grant` is a request/config or + // transient fault a browser cannot repair, so it must surface as + // NetworkUnavailable and never pop a browser — even for an interactive + // intent that COULD. Two representative cases prove the classifier keys on + // the body `error`, not the status class: a 400 `invalid_request` + // (malformed/misconfigured) and a 429 `slow_down` (transient rate limit). + for (status, error, refresh_token) in [ + ( + axum::http::StatusCode::BAD_REQUEST, + "invalid_request", + "misconfigured-refresh", + ), + ( + axum::http::StatusCode::TOO_MANY_REQUESTS, + "slow_down", + "rate-limited-refresh", + ), + ] { + let stub = spawn_stub_with(RefreshMode::ClientError(status, error)).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": refresh_token, + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src + .acquire_with_intent(AuthIntent::UserInitiated, None) + .await; + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "a non-invalid_grant 4xx ({status} {error}) is infrastructural, not a credential rejection" + ); + assert_eq!( + opener.call_count(), + 0, + "a browser cannot repair {error}, so none is opened" + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + } +} + +#[tokio::test] +async fn test_two_concurrent_userinitiated_denials_share_one_browser() { + // Two UserInitiated callers arrive together on one key. The first is the + // leader and opens the browser; the second is a pre-existing joiner that + // must receive the leader's SAME Denied result rather than acquire the + // lock afterward, clear the cooldown, and pop a second browser. This is + // the failure-sharing that a lock-alone protocol loses. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Deny); + + let a = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(opener.clone()), + ) + .unwrap(); + let b = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(opener.clone()), + ) + .unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::UserInitiated, None), + b.acquire_with_intent(AuthIntent::UserInitiated, None), + ); + assert_eq!(ra, Err(AuthError::Denied), "leader observes the denial"); + assert_eq!( + rb, + Err(AuthError::Denied), + "the joiner shares the leader's denial, not a fresh attempt" + ); + assert_eq!( + opener.call_count(), + 1, + "one browser launch shared across both concurrent UserInitiated callers" + ); +} + +// ---- mixed-intent coalescing must not leak an Auto cooldown to a user ----- +// +// `Auto` and `UserInitiated` disagree on cooldown policy: `Auto` honors a +// recorded cooldown and returns its `Denied`/`TimedOut` without a browser, +// while `UserInitiated` bypasses the cooldown and opens a fresh sign-in. If +// both coalesced onto one in-process slot, a user's explicit action arriving +// behind an `Auto` leader would inherit the leader's suppressed result and +// silently get *nothing* — no browser, no bypass. Keying the single-flight +// slot by the full intent keeps the two from sharing a slot. + +#[tokio::test] +async fn test_userinitiated_joiner_does_not_inherit_auto_cooldown_result() { + // Race an Auto caller and a UserInitiated caller on one key. `join!` polls + // the Auto future first: it becomes the in-process leader, takes the file + // lock, and opens a browser that is DENIED — and it yields on the callback + // wait while still holding the lock and its INFLIGHT slot. The + // UserInitiated caller is then polled *while the Auto attempt is in flight*. + // + // Before the fix, both intents keyed the single-flight slot by browser + // capability alone, so the UserInitiated caller joined the Auto leader's + // slot and inherited its `Denied` — never opening its own browser, never + // getting the cooldown bypass it promises. Keying by the full intent keeps + // them apart: the UserInitiated caller runs its own flow, bypasses the + // cooldown the Auto denial recorded, and signs in on its own browser. + // + // Distinct openers make the coalescing visible: if the UserInitiated caller + // had inherited the Auto result, its `approve` opener would never fire. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let deny = ScriptedOpener::new(Script::Deny); + let approve = ScriptedOpener::new(Script::Approve); + + let auto = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(deny.clone()), + ) + .unwrap(); + let user = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(approve.clone()), + ) + .unwrap(); + + let (auto_res, user_res) = tokio::join!( + auto.acquire_with_intent(AuthIntent::Auto, None), + user.acquire_with_intent(AuthIntent::UserInitiated, None), + ); + assert_eq!( + auto_res, + Err(AuthError::Denied), + "the Auto leader observes its own browser denial" + ); + let bearer = + user_res.expect("the UserInitiated caller runs its own sign-in, not the Auto slot"); + assert!( + bearer.starts_with("browser-token-"), + "UserInitiated got a fresh browser token, not the Auto leader's Denied: {bearer}" + ); + assert_eq!( + deny.call_count(), + 1, + "the Auto leader opened exactly one (denied) browser" + ); + assert_eq!( + approve.call_count(), + 1, + "the UserInitiated caller opened its own browser instead of inheriting the Auto denial" + ); +} + +// ---- a joiner must never inherit its own rejected token ------------------- +// +// The in-process slot is keyed by (lock path, intent) only, so a 401-recovery +// joiner shares a leader that ran with a *different* `rejected` value. If the +// leader publishes a token equal to THIS caller's rejected bytes — e.g. its +// refresh produced exactly the generation the joiner just reported 401 — the +// joiner would retry the provider with the credentials it already knows are +// dead. The joiner must instead detect the collision and run its own bounded +// acquisition, obtaining a token that differs from its `rejected`. + +#[cfg(unix)] +#[tokio::test] +async fn test_joiner_never_receives_its_own_rejected_token() { + // Two concurrent `Headless` 401-recovery callers on one key, each rejecting + // a DIFFERENT bearer. The seeded cache token is expired, so neither caller + // is satisfied by the fast path (or the under-lock re-read) — both must go + // to the live refresh grant, which is what makes the leader slow enough to + // join. `join!` polls A first: it registers the INFLIGHT slot as leader, + // takes the file lock, and yields on its refresh HTTP call while holding + // the slot. B is then polled *while A is in flight* and joins A's slot. + // + // A's refresh yields `refreshed-token-1` and saves it. That is exactly the + // bearer B passed as `rejected` (B held gen-1 and was 401'd on it). Before + // the fix, B — a joiner keyed only by intent — received A's published + // `refreshed-token-1`: the precise bytes it just reported rejected. The fix + // makes B detect `published == own rejected`, fall through to its own + // acquisition, and refresh again to `refreshed-token-2`. The rerun goes + // straight to the leader body (not back through the registry), and its + // under-lock re-read rejects A's freshly-saved gen-1 (it equals B's + // `rejected`), so B can neither re-join the dead generation's slot, adopt + // its own rejected bytes from disk, nor loop. + let stub = spawn_stub(false).await; // refresh always succeeds + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Expired access token with a live refresh token: the expiry forces both + // callers past the cache into the refresh grant regardless of their + // distinct `rejected` values. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let a = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let b = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Headless, Some("rejected-by-a")), + b.acquire_with_intent(AuthIntent::Headless, Some("refreshed-token-1")), + ); + + assert_eq!( + ra, + Ok("refreshed-token-1".to_string()), + "the leader refreshes to gen-1, which differs from its own rejected value" + ); + let b_token = rb.expect("the joiner runs its own acquisition instead of inheriting gen-1"); + assert_ne!( + b_token, "refreshed-token-1", + "the joiner must never receive the exact bytes it reported 401-rejected" + ); + assert_eq!( + b_token, "refreshed-token-2", + "the joiner refreshed once more to a token that differs from its rejected value" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "exactly two refreshes: the leader's, then the joiner's single bounded rerun — no loop" + ); + assert_eq!( + opener.call_count(), + 0, + "a live refresh recovers both callers without any browser" + ); +} + +// ---- a bounded rerun that re-issues the rejected bytes must fail typed ----- +// +// The joiner-collision fix reruns its own bounded acquisition when the leader +// publishes the joiner's own rejected token. That rerun is only safe if it, +// too, refuses to hand back the rejected bytes: a provider that re-issues an +// identical access token on refresh would otherwise let the exact 401'd +// credential escape through the rerun. The coordinator guards the refresh +// success at the persistence boundary (`finish`), so both a plain leader and +// this rerun terminate with a typed auth error before caching the rejected +// token rather than returning it. + +#[cfg(unix)] +#[tokio::test] +async fn test_joiner_rerun_reissuing_rejected_token_fails_typed_not_loop() { + // A sticky provider returns ONE fixed access token on every refresh. Leader + // A rejects a different value, so its refresh to the sticky token is a + // clean success it publishes and caches. Joiner B rejected exactly the + // sticky token: it collides with A's published result, reruns its own + // bounded acquisition, and that rerun's refresh hands back the sticky token + // again — B's own rejected bytes. The persistence-boundary guard turns that + // into a terminal `RefreshRejected` (Headless, no browser) instead of + // returning the dead credential or looping. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("sticky-token")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let a = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let b = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Headless, Some("rejected-by-a")), + b.acquire_with_intent(AuthIntent::Headless, Some("sticky-token")), + ); + + assert_eq!( + ra, + Ok("sticky-token".to_string()), + "the leader's refresh yields the sticky token, which differs from its own rejected value" + ); + assert_eq!( + rb, + Err(AuthError::RefreshRejected), + "the joiner's rerun re-issued its own rejected bytes and must fail typed, not return them" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "exactly two refreshes: the leader's, then the joiner's single bounded rerun — no loop" + ); + assert_eq!( + opener.call_count(), + 0, + "a headless collision never opens a browser" + ); +} + +// ---- a joiner with a DIFFERENT rejected must not inherit a rejection-relative failure --- +// +// When a leader A rejects token X (its own `rejected`) and the refresh yields +// X again — causing `finish()` to return `RefreshRejected` — that failure is +// scoped to A's specific rejected token. A joiner B waiting on the same slot +// with a *different* rejected token Y must NOT adopt that failure: the refresh +// grant of X is a perfectly valid token for B (B only rejected Y). The slot +// publishes A's rejected-token digest; B detects the mismatch and reruns its +// own `acquire_leader` — which finds X already in the cache from A's successful +// write (X was issued but not cached because A had it as `rejected`, but in +// Carl's scenario there was NO prior good token — the refresh just minted X +// which IS good for B), and returns it. +// +// Concrete scenario: A rejected X, refresh re-issues X → A gets RefreshRejected. +// B rejected Y (different), refresh would yield X for B → B succeeds. + +#[cfg(unix)] +#[tokio::test] +async fn test_joiner_with_different_rejected_does_not_inherit_leaders_rejection_failure() { + // Sticky provider always returns "X" on every refresh grant. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("X")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + // A rejected "X" (same as what the provider always issues). The refresh + // re-issues "X", `finish()` returns RefreshRejected — the failure is + // rejection-relative to A's own rejected bytes. + // + // B rejected "Y" (different). It should NOT inherit A's RefreshRejected: + // the provider can give B "X", which is valid for B. + let a = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let b = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Headless, Some("X")), + b.acquire_with_intent(AuthIntent::Headless, Some("Y")), + ); + + assert_eq!( + ra, + Err(AuthError::RefreshRejected), + "A's refresh re-issued its own rejected token X — typed failure for A" + ); + assert_eq!( + rb, + Ok("X".to_string()), + "B's rejected was Y (not X), so B reruns and its refresh yields X — a valid token for B" + ); + assert_eq!( + opener.call_count(), + 0, + "headless callers never open a browser" + ); + // At least two refresh grants: A's, then B's rerun. + assert!( + stub.refresh_grants.load(Ordering::SeqCst) >= 2, + "B must have run its own refresh (rerun, not adoption)" + ); +} + +// ---- in-process joiner state reconciliation (P1 regressions) --------------- +// +// These tests drive two independently constructed same-key sources through real +// leader/joiner acquisition and verify that subsequent public reads on both +// sources reflect the shared outcome — not the stale or absent credential each +// source carried before joining. +// +// The coordinator's in-process single-flight coalesces callers on a shared +// `InflightSlot`. On the old bearer-only publication path the joiner's own +// `state` cell was never updated, so: +// - success: B's next plain `bearer()` served the locally-fresh-but-rejected +// token X rather than the just-acquired Y (memory won over disk). +// - failure: B's matching rejected X remained live; its next `bearer()` still +// served it. +// - no-persistence (Windows): B's state stayed empty; its next headless read +// returned `NoCredential` instead of Y and a second browser opened. +// +// All three tests exercise the full `finish()` → `acquire_locked()` → +// `acquire_leader()` → `LeaderGuard::complete()` → joiner wiring. + +// Unix-specific: the seed provides a live refresh token. The non-Unix constructor +// does not read the disk cache, so without a seed in memory A's headless path +// returns NoCredential rather than RefreshRejected. +#[cfg(unix)] +#[tokio::test] +async fn test_inprocess_joiner_reconciles_stale_state_after_shared_success() { + // Scenario: A and B both loaded a locally-fresh-but-401'd token X. A leads, + // refreshes to Y. B joins and wakes to Ok(Y). Without reconciliation B's + // state still holds unexpired X, so B's next plain bearer() serves X — the + // exact token the caller just reported 401-rejected. + // + // `join!` polls A first: A registers the INFLIGHT slot as leader, takes the + // file lock, and yields on the refresh HTTP call. B is polled while A is in + // flight, finds the slot, and joins. + // + // Mutation check (no state reconciliation): B.state stays Some(unexpired-X). + // The subsequent bearer() call on B hits the memory cache (X is not expired, + // rejected=None so identity check passes), and `a_next == b_next` FAILS + // because ra_next = Y and rb_next = X. + let stub = spawn_stub(false).await; // refresh returns fresh token + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::FailToOpen); // headless — no browser + let cfg = config(&stub, "/disco/a", cache.path()); + + // Unexpired X with a live refresh token: both A and B load it as their + // initial state via the constructor's `read_cache` call. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale-X", + "refresh_token": "live-refresh", + "expires_at": future_secs(), // NOT expired — locally fresh + }), + ); + + let a = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let b = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + // Both 401-recovery callers on the same key. A becomes leader (polled + // first), refreshes to "refreshed-token-1", B joins A's slot. + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Headless, Some("stale-X")), + b.acquire_with_intent(AuthIntent::Headless, Some("stale-X")), + ); + + assert_eq!( + ra, + Ok("refreshed-token-1".to_string()), + "leader (A) receives the refreshed token" + ); + assert_eq!( + rb, + Ok("refreshed-token-1".to_string()), + "joiner (B) receives the leader's token" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "exactly one refresh — B joined A's slot rather than running its own" + ); + + // After the join, both sources must hold the new token in state. Subsequent + // plain bearer() calls (rejected=None) on both must return Y, not stale X. + let ra_next = a + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("A subsequent read must return the refreshed token"); + let rb_next = b + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("B subsequent read must return the refreshed token, not stale X"); + + assert_eq!(ra_next, "refreshed-token-1", "A subsequent read returns Y"); + assert_eq!( + rb_next, "refreshed-token-1", + "B subsequent read returns Y, not stale X — \ + mutation check: fails if joiner state was not reconciled (bearer-only publication)" + ); + // No second refresh: both subsequent reads hit the in-memory cache. + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "subsequent reads hit the in-memory cache — no second network call" + ); +} + +// Unix-specific: refresh token is required for a headless rejection path. +#[cfg(unix)] +#[tokio::test] +async fn test_inprocess_joiner_neutralizes_rejected_on_matching_shared_failure() { + // Scenario: A and B both carry unexpired X as their rejected token. A leads, + // attempts a refresh, gets 401 (RefreshRejected). B joins and wakes to the + // shared failure. Without reconciliation B's state still holds unexpired X, + // so B's next plain bearer() serves it — the rejected credential reappears. + // + // With reconciliation, expire_rejected is called under lock, so X is + // force-expired in B's state and cannot be served again. + // + // Mutation check (no expire_rejected call on the joiner Err path): B.state + // still holds unexpired X after the join. B's next bearer() (rejected=None) + // hits the memory cache and returns X. The assertion `rb_next != Ok("stale-X")` + // FAILS — the rejected credential reappears. + let stub = spawn_stub(true).await; // reject_refresh=true → 401 on every refresh + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::FailToOpen); // headless — no browser + let cfg = config(&stub, "/disco/a", cache.path()); + + // Unexpired X with a live (but destined-to-be-rejected) refresh token. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale-X", + "refresh_token": "live-refresh", + "expires_at": future_secs(), // NOT expired — locally fresh + }), + ); + + let a = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let b = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::Headless, Some("stale-X")), + b.acquire_with_intent(AuthIntent::Headless, Some("stale-X")), + ); + + assert_eq!( + ra, + Err(AuthError::RefreshRejected), + "leader (A) gets RefreshRejected — dead refresh" + ); + assert_eq!( + rb, + Err(AuthError::RefreshRejected), + "joiner (B) shares the leader's RefreshRejected failure" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "exactly one refresh attempt — B joined the failure rather than retrying" + ); + + // After the shared failure, B must not be able to serve stale X on a + // subsequent plain bearer() call. Without reconciliation, B.state still + // holds unexpired X and the next bearer() would return it. + let rb_next = b.acquire_with_intent(AuthIntent::Headless, None).await; + assert_ne!( + rb_next, + Ok("stale-X".to_string()), + "B must not serve the rejected token after adopting a matching shared failure — \ + mutation check: fails if the joiner Err path skips expire_rejected" + ); +} + +// Non-Unix-specific: disk persistence is disabled on Windows, so the only way +// for B to retain Y after joining is in-memory state reconciliation. On Unix +// the disk can provide Y as a fallback, masking a reconciliation failure. +#[cfg(not(unix))] +#[tokio::test] +async fn test_inprocess_joiner_populates_empty_state_no_second_acquisition() { + // Scenario: A and B both start with empty state (no disk token on non-Unix). + // A leads, opens a browser, exchanges the code for Y. B joins A's slot and + // wakes to Ok(Y). Without reconciliation, B.state stays None. B's next + // headless acquire returns NoCredential instead of Y, and a second browser + // would open if UserInitiated. + // + // Mutation check (no state reconciliation): B.state stays None. The + // subsequent headless acquire on B returns Err(NoCredential) instead of + // Ok("browser-token-1") — the assertion FAILS. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let approve = ScriptedOpener::new(Script::Approve); + + let a = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(approve.clone()), + ) + .unwrap(); + let b = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(approve.clone()), + ) + .unwrap(); + + // Both start with empty state — UserInitiated falls through to a browser. + let (ra, rb) = tokio::join!( + a.acquire_with_intent(AuthIntent::UserInitiated, None), + b.acquire_with_intent(AuthIntent::UserInitiated, None), + ); + + assert_eq!( + ra, + Ok("browser-token-1".to_string()), + "leader (A) gets the browser token" + ); + assert_eq!( + rb, + Ok("browser-token-1".to_string()), + "joiner (B) shares the leader's browser token" + ); + assert_eq!( + approve.call_count(), + 1, + "exactly one browser opened — B joined rather than launching its own" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one authorization-code exchange" + ); + + // B's subsequent headless acquire must return Y from in-memory state without + // a second browser. Without reconciliation, B.state is None and headless + // returns NoCredential (no disk fallback on non-Unix). + let rb_next = b + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect( + "B subsequent headless read must return Y from in-memory state, not NoCredential — \ + mutation check: fails if joiner state was not reconciled (bearer-only publication)", + ); + assert_eq!( + rb_next, "browser-token-1", + "B retains Y in memory for subsequent headless reads" + ); + // No second browser: B's subsequent read hit the in-memory cache. + assert_eq!( + approve.call_count(), + 1, + "no second browser opened — B's subsequent headless read hit the in-memory cache" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "no second code exchange" + ); +} + +// ---- a browser success that re-issues the rejected bytes must fail typed --- +// +// The 401-recovery invariant lives at `finish`'s persistence boundary, so it +// must hold on the browser-success path too — not just refresh. An +// interactive caller whose refresh is dead falls through to a browser sign-in; +// if that exchange re-issues the exact token the caller reported 401-rejected +// (a provider reusing an access token within its validity window), the guard +// must terminate typed before caching it rather than hand back the dead +// bearer. A single interactive leader exercises the path; the colliding-joiner +// rerun routes through the same boundary. + +#[cfg(unix)] +#[tokio::test] +async fn test_interactive_browser_reissuing_rejected_token_fails_typed_not_loop() { + // Refresh 401s (dead), so an interactive intent falls through to the + // browser; the exchange stickily returns one fixed token on every grant. + let stub = spawn_stub_with_modes( + RefreshMode::Reject, + ExchangeMode::SucceedSticky("sticky-browser"), + ) + .await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Expired seed with a (dead) refresh token: the caller misses the cache, + // its refresh is rejected, and it browses. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + + // The caller reports the sticky browser token as its rejected bearer, so + // the browser exchange hands back exactly those bytes. + let result = src + .acquire_with_intent(AuthIntent::UserInitiated, Some("sticky-browser")) + .await; + + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "a browser success equal to the rejected bytes must fail typed, not return them" + ); + assert_eq!( + opener.call_count(), + 1, + "the interactive attempt browsed exactly once — no loop re-launching the browser" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one code exchange — the guard fails terminally instead of retrying" + ); +} + +// ---- a rejected re-issue must not poison the cache for later callers ------- +// +// The persistence-boundary guard's whole purpose: a rejected-aware acquisition +// that a provider answers with the exact 401'd bytes must not leave those bytes +// cached as fresh. Before the fix, `finish()` persisted first and the guard +// fired after, so the dead token survived on disk and in memory — the next +// plain `bearer()` (`rejected = None`) and any freshly constructed source would +// serve it straight from the cache with no re-validation. These two regressions +// prove the cache is untouched after the typed failure, on both the refresh and +// the browser re-issue paths. + +#[cfg(unix)] +#[tokio::test] +async fn test_sticky_refresh_rejection_does_not_poison_cache_for_later_callers() { + // A sticky provider re-issues `sticky-token` on every refresh. A caller that + // reports `sticky-token` as its rejected bearer gets a typed failure — and + // the rejected bytes must never reach the cache. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("sticky-token")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let rejected = src + .acquire_with_intent(AuthIntent::Headless, Some("sticky-token")) + .await; + assert_eq!( + rejected, + Err(AuthError::RefreshRejected), + "a refresh that re-issues the rejected bytes fails typed" + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + + // The rejected bytes were never persisted: a fresh process reading the same + // cache path finds the original expired seed, not `sticky-token`. + let on_disk = std::fs::read_to_string(cache_file_path(&cfg, cache.path())).unwrap(); + assert!( + on_disk.contains("expired-seed") && !on_disk.contains("sticky-token"), + "the failed acquisition poisoned the on-disk cache: {on_disk}" + ); + + // A freshly constructed source over the same cache must therefore refresh + // over the network to obtain the token — it cannot serve a cached poison. + // Under the bug this was a lock-free cache hit and `refresh_grants` stayed + // at 1; the fix forces a second refresh. `Headless, None` is the plain + // `bearer()` path (rejected = None) with the typed error surfaced directly. + let fresh = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = fresh + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("a rejected=None caller legitimately obtains the current token"); + assert_eq!(token, "sticky-token"); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "the fresh source re-validated over the network — it did not serve a cached poison" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_sticky_browser_rejection_does_not_poison_cache_for_later_callers() { + // Refresh is dead, so an interactive caller browses; the exchange stickily + // re-issues `sticky-browser`. A caller reporting those bytes as rejected + // gets a typed failure, and the dead token must never reach the cache. + let stub = spawn_stub_with_modes( + RefreshMode::Reject, + ExchangeMode::SucceedSticky("sticky-browser"), + ) + .await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let rejected = src + .acquire_with_intent(AuthIntent::UserInitiated, Some("sticky-browser")) + .await; + assert_eq!( + rejected, + Err(AuthError::NetworkUnavailable), + "a browser exchange that re-issues the rejected bytes fails typed" + ); + assert_eq!(opener.call_count(), 1); + assert_eq!(stub.code_grants.load(Ordering::SeqCst), 1); + + // The rejected bytes were never persisted: the on-disk cache still holds + // the expired seed, so no fresh process can restore `sticky-browser`. + let on_disk = std::fs::read_to_string(cache_file_path(&cfg, cache.path())).unwrap(); + assert!( + on_disk.contains("expired-seed") && !on_disk.contains("sticky-browser"), + "the failed browser acquisition poisoned the on-disk cache: {on_disk}" + ); + + // A subsequent plain `bearer()` (Headless, `rejected = None`) reads that + // un-poisoned cache: the seed is expired and its refresh is dead, so it + // fails `RefreshRejected` — it never serves `sticky-browser` from cache. + // Under the bug the poisoned cache made this a hit returning the dead bytes. + let fresh_opener = ScriptedOpener::new(Script::Approve); + let fresh = PkceOAuthTokenSource::new_with(cfg, Arc::new(fresh_opener.clone())).unwrap(); + let later = fresh.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + later, + Err(AuthError::RefreshRejected), + "a fresh source must not serve the rejected browser token from cache" + ); + assert_eq!( + fresh_opener.call_count(), + 0, + "a headless caller never browses" + ); +} + +// ---- a 401 on a locally-fresh token neutralizes the cached copy ----------- +// +// P1: the persistence-boundary guard refuses to *save* a re-issued rejected +// token, but the ORIGINAL cached copy — the exact bytes the provider just +// 401'd — is untouched. Because `is_expired` trusts only the clock, a later +// plain `bearer()` (`rejected = None`) or a freshly constructed source would +// serve that dead token straight from cache. `expire_rejected` force-expires +// the cached copy (memory and disk) under the lock the moment a caller reports +// it rejected, so no future caller and no fresh process can serve it, while the +// refresh token — not rejected, and the engine of recovery — stays intact. + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_fresh_token_is_neutralized_for_a_fresh_process() { + // The cached access token `A` is locally UNEXPIRED, and the provider + // stickily re-issues `A` on refresh. A caller reports `A` as rejected: the + // refresh hands back `A`, the guard fails typed without persisting it — and + // the original unexpired `A` must not survive on disk for a fresh process. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("A")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "A", + "refresh_token": "live-refresh", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let rejected = src + .acquire_with_intent(AuthIntent::Headless, Some("A")) + .await; + assert_eq!( + rejected, + Err(AuthError::RefreshRejected), + "a refresh that re-issues the rejected bytes fails typed" + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + + // The on-disk copy of `A` was force-expired in place: the refresh token is + // preserved, but the access token's expiry is neutralized so no clock-based + // read can serve it. Under the bug it stayed at its future expiry. + let on_disk: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(cache_file_path(&cfg, cache.path())).unwrap(), + ) + .unwrap(); + assert_eq!( + on_disk["access_token"], "A", + "the entry is kept, not deleted" + ); + assert_eq!( + on_disk["refresh_token"], "live-refresh", + "the refresh token — not rejected — survives for recovery" + ); + assert_eq!( + on_disk["expires_at"], 0, + "the rejected access token was force-expired on disk" + ); + + // A freshly constructed source reading that cache must NOT serve `A` from + // the clock: it sees the neutralized entry as expired and refreshes over + // the network. Under the bug this was a lock-free cache hit returning the + // dead `A` with `refresh_grants` frozen at 1. + let fresh = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = fresh + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("a rejected=None caller obtains the provider's current token"); + assert_eq!(token, "A"); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "the fresh source re-validated over the network — it did not serve the neutralized cache" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_fresh_token_is_neutralized_for_the_same_source() { + // The in-memory layer of the same neutralization: after the SAME source + // fails a 401-recovery on unexpired `A`, its next plain `bearer()` + // (`rejected = None`) must not serve `A` from the in-memory cell — it must + // re-validate. `A` is sticky, so recovery returns `A` again, but only after + // a real refresh grant (the discriminator: 1 cache hit vs. 2 grants). + let stub = spawn_stub_with(RefreshMode::SucceedSticky("A")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "A", + "refresh_token": "live-refresh", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + assert_eq!( + src.acquire_with_intent(AuthIntent::Headless, Some("A")) + .await, + Err(AuthError::RefreshRejected), + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + + // Same source, plain bearer: the in-memory `A` was neutralized, so this is + // a miss that refreshes rather than a cache hit. Under the bug the + // unexpired in-memory `A` was served directly and `refresh_grants` stayed 1. + let token = src + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("a subsequent plain bearer re-validates rather than serving the dead token"); + assert_eq!(token, "A"); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "the same source re-validated in memory — it did not serve the neutralized token" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_fresh_token_neutralized_when_recovery_browses() { + // The browser variant: `A` is unexpired but its refresh token is dead, so + // an interactive 401-recovery falls through to the browser, whose exchange + // stickily re-issues `A`. The guard fails typed without persisting it, and + // the neutralized `A` must not survive for a later headless caller. + let stub = spawn_stub_with_modes(RefreshMode::Reject, ExchangeMode::SucceedSticky("A")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "A", + "refresh_token": "dead-refresh", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + let rejected = src + .acquire_with_intent(AuthIntent::UserInitiated, Some("A")) + .await; + assert_eq!( + rejected, + Err(AuthError::NetworkUnavailable), + "a browser exchange that re-issues the rejected bytes fails typed" + ); + assert_eq!(opener.call_count(), 1); + assert_eq!(stub.code_grants.load(Ordering::SeqCst), 1); + + // The unexpired `A` was force-expired on disk, so a fresh headless source + // finds it unusable and — its refresh being dead — fails `RefreshRejected` + // rather than serving `A`. Under the bug the still-fresh `A` was a cache + // hit that returned the dead token. + let fresh_opener = ScriptedOpener::new(Script::Approve); + let fresh = PkceOAuthTokenSource::new_with(cfg, Arc::new(fresh_opener.clone())).unwrap(); + let later = fresh.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + later, + Err(AuthError::RefreshRejected), + "a fresh source must not serve the neutralized rejected token from cache" + ); + assert_eq!( + fresh_opener.call_count(), + 0, + "a headless caller never browses" + ); +} + +// ---- P1-1 bounded three-stage neutralization: disk fallback paths ----------- +// +// `expire_rejected()` neutralizes the on-disk token with three-stage fallback: +// 1. Atomic rewrite via `persist()` (temp-file + rename, owner-only perms). +// 2. In-place truncating overwrite via `OpenOptions::write().truncate(true)` — +// succeeds even when the parent directory is non-writable, because only the +// file's own mode matters for writing an existing file. +// 3. `remove_file` as a last resort. +// +// The primary case this tests: a 0600 token file under a 0500 parent directory. +// Temp-file creation (for the atomic path) fails with EACCES; the in-place +// write succeeds because the file itself is owner-writable. After the in-place +// overwrite the file still exists but carries `expires_at = 0`, so a later +// plain `bearer(None)` or a freshly constructed source reads the now-expired +// entry and re-validates over the network instead of serving the dead token. + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_token_disk_neutralization_neutralizes_in_place_when_parent_blocks_rewrite() { + use std::os::unix::fs::PermissionsExt as _; + + // Seed unexpired `A` with a live refresh. The provider stickily re-issues `A`. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("A")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Create the token file inside a dedicated subdirectory so we can chmod + // just that subdirectory non-writable without affecting the test harness. + let token_dir = cache.path().join("protected"); + std::fs::create_dir_all(&token_dir).unwrap(); + + // Override the config to use the protected subdir. + let cfg = PkceOAuthConfig { + cache_dir_override: Some(token_dir.clone()), + ..cfg + }; + let cache_file = cache_file_path(&cfg, &token_dir); + + seed_cache( + &cfg, + &token_dir, + json!({ + "access_token": "A", + "refresh_token": "live-refresh", + "expires_at": future_secs(), + }), + ); + + // Build the source: it reads `A` from disk into its in-memory cell. + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + // The token file lives at `token_dir/databricks/.json`. Its direct + // parent is `token_dir/databricks/`, not `token_dir` itself — the + // coordinator's `cache_path_for()` appends the namespace subdir. Assert + // the relationship explicitly so a future path-resolution change breaks + // loudly here instead of silently letting the atomic write succeed (which + // would make the test vacuously pass even without the in-place fallback). + let protected_dir = cache_file + .parent() + .expect("cache file must have a parent directory"); + assert_eq!( + protected_dir, + token_dir.join("databricks"), + "cache file's direct parent is token_dir/databricks, not token_dir" + ); + + // Pre-create the advisory lock file so `acquire_auth_lock` can open it + // even after the directory is made non-writable. The lock file must exist + // before the chmod, because `OpenOptions::create(true)` on an existing + // file succeeds regardless of parent-dir permissions, while creating a new + // file in a 0500 directory would EACCES. + let lock_file = { + let mut p = cache_file.as_os_str().to_owned(); + p.push(".lock"); + std::path::PathBuf::from(p) + }; + std::fs::File::create(&lock_file).expect("pre-create lock file before chmod"); + + // Make the direct parent non-writable (0500): temp-file creation for the + // atomic persist requires creating a new file in this directory → EACCES. + // The file itself remains 0600 owner-writable, so the in-place fallback + // path in `expire_rejected` can still open and truncate it. + std::fs::set_permissions(protected_dir, std::fs::Permissions::from_mode(0o500)).unwrap(); + + // Trigger 401-recovery: refresh stickily re-issues `A`, `finish()` rejects + // it typed. `expire_rejected` runs: atomic persist fails (EACCES on parent), + // in-place write succeeds (file mode 0600). + let result = src + .acquire_with_intent(AuthIntent::Headless, Some("A")) + .await; + assert_eq!( + result, + Err(AuthError::RefreshRejected), + "typed failure returned; neutralization does not disrupt the recovery path" + ); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + + // Restore write permission so the test harness can clean up. + std::fs::set_permissions(protected_dir, std::fs::Permissions::from_mode(0o700)).unwrap(); + + // The cache file still exists (in-place write, not removal), but its + // `expires_at` should now be 0 — it was overwritten in-place. + assert!( + cache_file.is_file(), + "in-place fallback: file still exists (not removed)" + ); + let raw = std::fs::read(&cache_file).expect("cache file readable after in-place write"); + let cached: serde_json::Value = + serde_json::from_slice(&raw).expect("cache file parseable after in-place write"); + assert_eq!( + cached.get("expires_at").and_then(|v| v.as_u64()), + Some(0), + "in-place write set expires_at = 0: token is now expired on disk" + ); + + // A fresh source constructed after the neutralization must not serve `A`. + let fresh_src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + // The disk token is expired; bearer() falls through to refresh, which + // stickily re-issues `A`, which `finish()` rejects again (no rejected + // identity on this plain call — the disk is now expired, so the source + // enters the refresh path, gets `A` back from the provider, and `finish()` + // sees no rejection guard and would persist it). But with no `rejected` + // passed here, a plain `bearer()` with the now-expired disk entry must + // re-validate. If the in-place write succeeded, the disk token has + // expires_at = 0 and `cached_hit` skips it, so the source goes to refresh. + // We confirm `A` is not served as a cache hit: the stub records a second + // refresh grant. + let _ = fresh_src + .acquire_with_intent(AuthIntent::Headless, None) + .await; + assert!( + stub.refresh_grants.load(Ordering::SeqCst) >= 2, + "fresh source did not serve `A` as a plain cache hit — it re-validated over the network" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_token_in_memory_neutralized_when_disk_neutralization_skipped() { + // When `expire_rejected()` cannot read a matching disk entry (e.g. the cache + // path is not a readable regular file), the disk layer is not neutralized, + // but the IN-MEMORY layer is always neutralized unconditionally. This test + // proves the in-memory safety path: even without disk neutralization, a + // subsequent plain `bearer()` on the same source cannot serve the dead token + // from the in-memory cell. + let stub = spawn_stub_with(RefreshMode::SucceedSticky("A")).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + let cache_file = cache_file_path(&cfg, cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "A", + "refresh_token": "live-refresh", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + // Replace the cache file with a directory so `read_private_cache` inside + // `expire_rejected` returns None (EISDIR on open). The disk branch is + // skipped entirely — only the in-memory layer is neutralized. + std::fs::remove_file(&cache_file).unwrap(); + std::fs::create_dir_all(&cache_file).unwrap(); + + let result = src + .acquire_with_intent(AuthIntent::Headless, Some("A")) + .await; + assert_eq!(result, Err(AuthError::RefreshRejected)); + assert_eq!(stub.refresh_grants.load(Ordering::SeqCst), 1); + + // In-memory layer: force-expired. The same source's next plain bearer() + // must not serve `A` from the in-memory cell. + let next = src.acquire_with_intent(AuthIntent::Headless, None).await; + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "in-memory `A` was force-expired; same source went to the network rather than serving the dead token" + ); + // The sticky refresh obtained `A` from the network (grant #2). The persist() + // call fails because the cache path is now a directory — save() maps the + // persist failure to NetworkUnavailable. This proves: (a) the in-memory + // neutralization worked (the source re-validated rather than serving A from + // the expired in-memory cell), and (b) the network was reached. The + // NetworkUnavailable result is an expected artifact of the directory-as- + // cache-path test setup, not a correctness gap. + assert!( + matches!(next, Err(AuthError::NetworkUnavailable)), + "save() fails with NetworkUnavailable on persist failure (expected artifact of test setup)" + ); + assert_ne!( + next, + Ok("A".to_owned()), + "A was not served from the expired in-memory cell — network was reached" + ); + + // Cleanup the directory we created. + std::fs::remove_dir(&cache_file).ok(); +} + +// ---- expired-sibling replacement must not satisfy a 401 recovery ---------- +// +// After a 401, `rejected = Some(t)` makes the expiry clock untrustworthy, so a +// cache hit requires a token that both DIFFERS from `t` and is still unexpired. +// An expired sibling token — one that merely differs from the rejected bytes — +// must NOT be served as the replacement: doing so would skip the refresh the +// 401 demanded and hand back a token the provider will also reject. + +#[cfg(unix)] +#[tokio::test] +async fn test_rejected_recovery_skips_expired_sibling_and_refreshes() { + let stub = spawn_stub(false).await; // refresh succeeds + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // The cached token is a DIFFERENT string from the rejected bytes, but it is + // expired. Under the old "differs is enough" rule it would be returned as + // the sibling replacement; the fix requires it to be unexpired too, so the + // coordinator must fall through to the live refresh instead. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-sibling", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let token = src + .acquire_with_intent(AuthIntent::Headless, Some("rejected-original")) + .await + .expect("an expired sibling forces a refresh rather than being reused"); + assert_eq!( + token, "refreshed-token-1", + "the expired sibling was not accepted; a fresh token was obtained" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "the 401 recovery refreshed instead of reusing the expired sibling" + ); + assert_eq!(opener.call_count(), 0, "a live refresh needs no browser"); +} + +// ---- code-exchange classifier: rejection vs. infrastructure -------------- +// +// The browser code exchange must mirror the refresh classifier: only a 4xx +// `invalid_grant` establishes the authorization code was rejected (terminal, +// cooldown-worthy `ExchangeFailed`). A 429, any 5xx, and a malformed 2xx are a +// transient provider fault that must surface as `NetworkUnavailable` — never +// poisoning the 5-minute cooldown against a provider outage after callback. + +#[tokio::test] +async fn test_exchange_invalid_grant_is_exchange_failed_and_cools_down() { + let stub = spawn_stub_with_exchange(ExchangeMode::Fail( + axum::http::StatusCode::UNAUTHORIZED, + "invalid_grant", + )) + .await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // A genuinely rejected code is terminal ExchangeFailed and is + // cooldown-worthy: a following Auto caller reads the cooldown without a + // second browser. + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let first = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + first, + Err(AuthError::ExchangeFailed), + "a 401 invalid_grant on the code exchange is a rejected grant" + ); + assert_eq!(opener.call_count(), 1); + + let second = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + second, + Err(AuthError::ExchangeFailed), + "the rejected exchange wrote a cooldown the next Auto caller honors" + ); + assert_eq!( + opener.call_count(), + 1, + "the cooldown suppressed a second browser launch" + ); +} + +#[tokio::test] +async fn test_exchange_transient_faults_are_network_unavailable_not_cooldown() { + // A 429, a 500, and a malformed 2xx are provider faults, not rejected + // codes: each must surface as NetworkUnavailable and leave no cooldown, so + // a subsequent Auto caller retries with a fresh browser rather than + // inheriting a suppressed outcome. + let cases = [ + ExchangeMode::Fail(axum::http::StatusCode::TOO_MANY_REQUESTS, "slow_down"), + ExchangeMode::Fail( + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + "temporarily_unavailable", + ), + ExchangeMode::MalformedSuccess, + ]; + for exchange in cases { + let stub = spawn_stub_with_exchange(exchange).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let result = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "a transient exchange fault is infrastructural, not a rejected code" + ); + assert_eq!(opener.call_count(), 1); + + // No cooldown was written, so a second Auto caller launches again + // rather than reading a suppressed outcome. + let retry = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + retry, + Err(AuthError::NetworkUnavailable), + "a transient exchange fault leaves no cooldown to suppress the retry" + ); + assert_eq!( + opener.call_count(), + 2, + "no cooldown means the next Auto caller opens a fresh browser" + ); + } +} + +#[tokio::test] +async fn test_exchange_timeout_is_network_unavailable_not_cooldown() { + // The code exchange hangs far longer than the injected per-request HTTP + // timeout, so the exchange POST times out at the transport layer with no + // verdict from the provider — the transport branch the classifier maps to + // NetworkUnavailable. Like the refresh-timeout test, a short real-time + // timeout is injected rather than pausing the clock: under `start_paused` + // tokio would auto-advance into the timer while the real loopback + // discovery/authorize round-trips are still in flight, tripping the timeout + // on the wrong request. Real time keeps the timeout attached to the + // exchange that actually hangs. + let stub = spawn_stub_with_exchange(ExchangeMode::Hang(Duration::from_secs(30))).await; + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + let src = PkceOAuthTokenSource::new_with_http_timeout( + cfg, + Arc::new(opener.clone()), + Duration::from_millis(300), + ) + .unwrap(); + let result = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + result, + Err(AuthError::NetworkUnavailable), + "an exchange transport timeout is infrastructural, not a rejected code" + ); + assert_eq!(opener.call_count(), 1); + + // The timed-out exchange wrote no cooldown, so a second Auto caller launches + // its own browser rather than inheriting a suppressed outcome. + let retry = src.acquire_with_intent(AuthIntent::Auto, None).await; + assert_eq!( + retry, + Err(AuthError::NetworkUnavailable), + "an exchange transport timeout leaves no cooldown to suppress the retry" + ); + assert_eq!( + opener.call_count(), + 2, + "no cooldown means the next Auto caller opens a fresh browser" + ); +} + +// ---- genuine cross-process lock contention and crash release ------------- +// +// The single-flight guarantee and its crash-release property are cross-process +// claims, so they need a real second process — not a second in-process handle — +// on the same lock file. The `lock-holder` helper binary takes the +// coordinator's advisory lock and holds it until killed; killing it models a +// crash mid-flow, and the kernel's release of the advisory lock is what lets +// the coordinator's successor proceed with no PID files and no lock breaking. + +#[cfg(unix)] +#[tokio::test] +async fn test_crossprocess_lock_holder_blocks_then_crash_release_lets_successor_proceed() { + let stub = spawn_stub(false).await; // refresh succeeds once the lock is free + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Expired token with a LIVE refresh: a cache miss forces the coordinator + // onto the slow path (it must take the lock), and once the lock is free the + // refresh recovers a token without any browser — so success is a clean + // signal that the successor proceeded. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let lock_path = lock_file_path(&cfg, cache.path()); + let ready_marker = cache.path().join("holder.ready"); + + // A real second process grabs the lock and holds it. + let mut holder = tokio::process::Command::new(env!("CARGO_BIN_EXE_lock-holder")) + .env("LOCK_HELPER_PATH", &lock_path) + .env("LOCK_HELPER_READY", &ready_marker) + .kill_on_drop(true) + .spawn() + .expect("spawn the lock-holder helper process"); + + // Synchronize on real lock ownership before racing the coordinator. + for _ in 0..600 { + if ready_marker.exists() { + break; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + assert!( + ready_marker.exists(), + "lock-holder never signaled that it holds the lock" + ); + + // The coordinator cannot make progress while another process holds the + // lock: it polls the advisory lock rather than stealing it. + let src = PkceOAuthTokenSource::new_with(cfg, Arc::new(opener.clone())).unwrap(); + let task = + tokio::spawn(async move { src.acquire_with_intent(AuthIntent::Headless, None).await }); + tokio::time::sleep(Duration::from_millis(400)).await; + assert!( + !task.is_finished(), + "coordinator must block while a live process holds the cross-process lock" + ); + + // Kill the holder: the kernel releases the advisory lock on process death, + // with no PID file inspection or lock breaking on our side. + holder.kill().await.expect("kill the lock holder"); + holder.wait().await.ok(); + + let token = task + .await + .expect("acquisition task joins") + .expect("successor proceeds once the crashed holder's lock is released"); + assert_eq!( + token, "refreshed-token-1", + "successor completes the refresh after acquiring the freed lock" + ); + assert_eq!( + opener.call_count(), + 0, + "Headless successor recovers via refresh without a browser" + ); +} + +// ---- genuine cross-process coordinator races ----------------------------- +// +// The `auth-worker` helper is a real second process running the PUBLIC +// coordinator API against the shared cache. Unlike two in-process handles +// (which the `INFLIGHT` registry coalesces before the file lock), these +// workers contend on the OS advisory lock and share success through the +// on-disk cache exactly as two Buzz processes on one machine would. + +/// A spawned `auth-worker`: its child handle plus the file it writes its JSON +/// outcome to. +struct Worker { + child: tokio::process::Child, + result_path: std::path::PathBuf, +} + +#[derive(Deserialize)] +struct WorkerOutcome { + result: String, + #[cfg(unix)] + bearer: Option, + launches: u64, +} + +impl Worker { + /// Block until the worker exits, then parse its outcome file. + async fn join(mut self) -> WorkerOutcome { + let status = self.child.wait().await.expect("auth-worker joins"); + assert!( + status.success(), + "auth-worker exited with failure: {status}" + ); + let body = std::fs::read(&self.result_path).expect("auth-worker wrote its outcome"); + serde_json::from_slice(&body).expect("auth-worker outcome parses") + } +} + +/// Spawn an `auth-worker` child against `cfg`'s shared cache. `extra` sets the +/// optional barrier-marker env vars ((name, path) pairs) a scenario needs to +/// order events across processes. +fn spawn_worker( + cfg: &PkceOAuthConfig, + cache_dir: &std::path::Path, + intent: &str, + script: &str, + tag: &str, + extra: &[(&str, &std::path::Path)], +) -> Worker { + let result_path = cache_dir.join(format!("{tag}.result.json")); + let mut cmd = tokio::process::Command::new(env!("CARGO_BIN_EXE_auth-worker")); + cmd.env("AUTH_WORKER_DISCOVERY_URL", &cfg.discovery_url) + .env("AUTH_WORKER_CACHE_DIR", cache_dir) + .env("AUTH_WORKER_NAMESPACE", &cfg.cache_namespace) + .env("AUTH_WORKER_CLIENT_ID", &cfg.client_id) + .env("AUTH_WORKER_SCOPES", cfg.scopes.join(",")) + .env("AUTH_WORKER_INTENT", intent) + .env("AUTH_WORKER_SCRIPT", script) + .env("AUTH_WORKER_RESULT", &result_path) + .kill_on_drop(true); + for (key, path) in extra { + cmd.env(key, path); + } + let child = cmd.spawn().expect("spawn the auth-worker helper process"); + Worker { child, result_path } +} + +async fn wait_for_marker(path: &std::path::Path, what: &str) { + for _ in 0..1000 { + if path.exists() { + return; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + panic!("timed out waiting for {what} ({})", path.display()); +} + +#[tokio::test] +async fn test_crossprocess_userinitiated_denial_shared_with_waiting_auto() { + // Two real processes on one key. The child runs a UserInitiated flow that + // is denied; while it holds the lock and its browser is open, the parent's + // Auto coordinator is already WAITING on the cross-process lock. The child + // must be released only once the parent is queued, so the denial the child + // records is what the waiting Auto observes — one launch total, durable + // Denied for both, across a genuine process boundary. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + let launched = cache.path().join("child.launched"); + let proceed = cache.path().join("child.proceed"); + let child = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "deny", + "denier", + &[ + ("AUTH_WORKER_LAUNCHED_MARKER", launched.as_path()), + ("AUTH_WORKER_PROCEED_MARKER", proceed.as_path()), + ], + ); + + // Wait until the child holds the lock and has opened its (scripted) + // browser; its callback is withheld until we create `proceed`. + wait_for_marker(&launched, "child browser launch").await; + + // The parent's Auto coordinator now contends for the same lock. It cannot + // proceed while the child holds it, so it is a genuine cross-process + // waiter. + let parent = PkceOAuthTokenSource::new_with( + config(&stub, "/disco/a", cache.path()), + Arc::new(ScriptedOpener::new(Script::Approve)), + ) + .unwrap(); + let auto = + tokio::spawn(async move { parent.acquire_with_intent(AuthIntent::Auto, None).await }); + tokio::time::sleep(Duration::from_millis(300)).await; + assert!( + !auto.is_finished(), + "parent Auto must block while the child process holds the lock" + ); + + // Release the child's callback: it finishes the denial and writes the + // cooldown sidecar, then drops the lock. + std::fs::write(&proceed, b"go").unwrap(); + + let child_outcome = child.join().await; + assert_eq!( + child_outcome.result, "denied", + "child UserInitiated is denied" + ); + assert_eq!(child_outcome.launches, 1, "child opens exactly one browser"); + + let auto_result = auto.await.expect("parent Auto task joins"); + assert_eq!( + auto_result, + Err(AuthError::Denied), + "the already-waiting Auto reads the child's durable denial" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 0, + "a denied flow never reaches the code exchange" + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn test_crossprocess_two_coordinators_race_to_one_grant_and_cache() { + // Two real coordinator processes race on one key from a cold cache. They + // are released together (via a shared start marker) so both contend for the + // lock. Exactly one wins the browser flow and performs the single code + // grant; the other serializes behind the lock and adopts the winner's token + // from the shared cache. Both must observe the same bearer, and the private + // cache must hold exactly one parseable token artifact. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + let ready_a = cache.path().join("a.ready"); + let ready_b = cache.path().join("b.ready"); + let start = cache.path().join("start"); + + let worker_a = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "approve", + "a", + &[ + ("AUTH_WORKER_READY_MARKER", ready_a.as_path()), + ("AUTH_WORKER_START_MARKER", start.as_path()), + ], + ); + let worker_b = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "approve", + "b", + &[ + ("AUTH_WORKER_READY_MARKER", ready_b.as_path()), + ("AUTH_WORKER_START_MARKER", start.as_path()), + ], + ); + + // Both processes are built and about to acquire; release them together. + wait_for_marker(&ready_a, "worker A ready").await; + wait_for_marker(&ready_b, "worker B ready").await; + std::fs::write(&start, b"go").unwrap(); + + let (out_a, out_b) = tokio::join!(worker_a.join(), worker_b.join()); + assert_eq!(out_a.result, "ok", "worker A authenticates"); + assert_eq!(out_b.result, "ok", "worker B authenticates"); + let bearer_a = out_a.bearer.expect("worker A returns a bearer"); + let bearer_b = out_b.bearer.expect("worker B returns a bearer"); + assert_eq!( + bearer_a, bearer_b, + "both processes observe the same bearer from the shared cache" + ); + + // Exactly one browser launch and one code exchange across both processes. + assert_eq!( + out_a.launches + out_b.launches, + 1, + "exactly one browser launch across the two coordinator processes" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one authorization-code exchange across both processes" + ); + + // The private cache holds exactly one parseable token artifact carrying the + // shared bearer. + let cache_path = cache_file_path(&cfg, cache.path()); + let raw = std::fs::read(&cache_path).expect("cache file exists"); + let cached: serde_json::Value = + serde_json::from_slice(&raw).expect("cache holds one parseable token artifact"); + assert_eq!( + cached.get("access_token").and_then(|v| v.as_str()), + Some(bearer_a.as_str()), + "the cached token is the shared bearer" + ); +} + +// ---- cross-process failure single-flight (attempt-record protocol) -------- +// +// `INFLIGHT` coalesces same-key callers within one process before they reach +// the file lock, so two separate processes both queued on the lock do NOT +// share the in-process registry. Without the attempt-record protocol, a +// process that acquires the lock AFTER the holder fails would re-run the +// full flow from scratch — a second browser launch on `Denied`, or a second +// dead-refresh call on `RefreshRejected`. The attempt sidecar lets the +// second process detect that the predecessor completed while it was waiting +// and adopt its failure directly. + +#[cfg(unix)] +#[tokio::test] +async fn test_crossprocess_waiting_headless_adopts_predecessor_refresh_rejected() { + // Two real headless processes on one key. The cache holds an expired + // token with a dead refresh. A wins the lock and calls the stub; the + // stub holds A's response so B can deterministically snapshot gen=0 + // and queue on the lock before A completes. Once B's snapshot marker + // fires, A is released: it gets `invalid_grant`, writes the attempt + // sidecar (gen=1), and releases the lock. B acquires the lock, sees + // gen=1 > snap=0, and adopts `RefreshRejected` — ONE refresh grant + // total across both processes. + // + // This replaces the prior simultaneous-start design, which was not + // deterministic: the instant-reject stub could complete A before B + // ever snapshotted, giving B snap=1 and causing a spurious second + // refresh grant. + let (stub, gate) = spawn_stub_with_held_refresh(HeldRefreshResponse::Reject).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Seed the shared cache: expired token with a dead refresh, so both + // workers fall through to the refresh grant rather than a cache hit. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "stale", + "refresh_token": "dead-refresh", + "expires_at": 1u64, + }), + ); + + let snapshot_b = cache.path().join("b.snapshot"); + + // ---- Phase 1: spawn A. It acquires the lock and immediately calls the + // stub's refresh endpoint; the stub holds the response. + let worker_a = spawn_worker(&cfg, cache.path(), "headless", "approve", "a", &[]); + + // ---- Phase 2: wait until the stub has received A's refresh request. + // This is an in-process await — no polling or timing. + // Once the stub is holding A's request, A owns the lock. + gate.wait_for_request().await; + + // ---- Phase 3: spawn B with SNAPSHOT_MARKER. B starts, reads the + // attempt sidecar (gen=0, absent), emits its snapshot + // event, and then blocks on the lock behind A. + let worker_b = spawn_worker( + &cfg, + cache.path(), + "headless", + "approve", + "b", + &[("AUTH_WORKER_SNAPSHOT_MARKER", snapshot_b.as_path())], + ); + + // ---- Phase 4: wait for B's snapshot marker. Proves B captured gen=0 + // before A can record gen=1; lock queueing is not required + // for the temporal-generation discriminator to hold. + wait_for_marker(&snapshot_b, "worker B snapshot").await; + + // ---- Phase 5: release A. Stub returns invalid_grant; A records + // RefreshRejected with gen=1 and releases the lock. B + // acquires the lock, sees gen=1 > snap=0, and adopts. + gate.release(); + + let (out_a, out_b) = tokio::join!(worker_a.join(), worker_b.join()); + + // Both workers must report RefreshRejected. + assert_eq!( + out_a.result, "refresh_rejected", + "worker A gets RefreshRejected on a dead refresh" + ); + assert_eq!( + out_b.result, "refresh_rejected", + "worker B adopts RefreshRejected via the attempt sidecar" + ); + assert_eq!(out_a.launches, 0, "headless never opens a browser"); + assert_eq!(out_b.launches, 0, "headless never opens a browser"); + + // One refresh grant total: under the old protocol the second worker would + // re-run the dead refresh independently; the attempt record prevents that. + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 1, + "exactly one refresh grant across both headless processes" + ); +} + +#[tokio::test] +async fn test_crossprocess_userinitiated_waiter_adopts_predecessor_denial() { + // The adoption contract is *temporal*, not intent-based. A `UserInitiated` + // caller whose pre-queue snapshot is older than the current generation was + // already queued while the predecessor ran and MUST adopt its same-intent + // failure — exactly as the in-process `INFLIGHT` registry coalesces + // same-intent `UserInitiated` callers onto one leader within a process. + // + // When process A (UserInitiated) gets `Denied` and process B + // (UserInitiated) was queued *behind* it (B's snapshot predates A's write), + // B adopts A's denial without opening a second browser. The result: + // exactly one browser launch and zero code exchanges — one browser total + // across both processes. + // + // Note: this is different from a *later* explicit user retry, which + // arrives after A completes, snapshots the new generation, sees no advance, + // and naturally runs its own attempt. That behavior is proved by + // `test_crossprocess_post_failure_userinitiated_runs_own_attempt` below. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + let launched_a = cache.path().join("a.launched"); + let proceed_a = cache.path().join("a.proceed"); + + // Worker A holds the lock and keeps its browser open until we signal it, + // so B is certain to be queued behind A before A resolves. + let worker_a = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "deny", + "a", + &[ + ("AUTH_WORKER_LAUNCHED_MARKER", launched_a.as_path()), + ("AUTH_WORKER_PROCEED_MARKER", proceed_a.as_path()), + ], + ); + + // Wait until A holds the lock and its browser is open. + wait_for_marker(&launched_a, "worker A browser launch").await; + + // Worker B (also UserInitiated, approve-scripted) queues behind A on the + // file lock. Even though B would succeed if it ran its own browser, it + // must adopt A's denial since it was queued while A held the lock. + // + // SNAPSHOT_MARKER is emitted by the tracing layer in B's process after B + // snapshots gen=0 and before it queues on the file lock — so observing it + // proves B captured generation 0 before A records generation 1. + let snapshot_b = cache.path().join("b.snapshot"); + let worker_b = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "approve", + "b", + &[("AUTH_WORKER_SNAPSHOT_MARKER", snapshot_b.as_path())], + ); + // Wait until B has snapshotted gen=0, then release A. + wait_for_marker(&snapshot_b, "worker B snapshot").await; + + // Release A: it denies, writes the cooldown + attempt sidecars, releases lock. + std::fs::write(&proceed_a, b"go").unwrap(); + let out_a = worker_a.join().await; + assert_eq!(out_a.result, "denied", "worker A is denied"); + assert_eq!(out_a.launches, 1, "worker A opens one browser"); + + // Worker B adopts A's denial — it does not open a second browser even + // though it is UserInitiated. Under the old contract B would open its own + // browser and succeed; under the correct temporal contract it adopts. + let out_b = worker_b.join().await; + assert_eq!( + out_b.result, "denied", + "queued UserInitiated worker B adopts A's denial rather than re-running" + ); + assert_eq!( + out_b.launches, 0, + "worker B adopts the denial without opening a browser" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 0, + "no code exchange — B adopted A's Denied without reaching the token endpoint" + ); +} + +#[tokio::test] +async fn test_crossprocess_post_failure_userinitiated_runs_own_attempt() { + // A `UserInitiated` caller that arrives *after* a failure — not queued + // during it — snapshots the current (advanced) generation, sees no advance + // when it acquires the lock, and runs its own attempt. "Later explicit user + // retry bypasses" falls out of the temporal snapshot comparison without any + // special case. + let stub = spawn_stub(false).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Worker A (UserInitiated, deny-scripted) runs to completion first. No + // synchronization needed — we await it fully before constructing B. + let worker_a = spawn_worker(&cfg, cache.path(), "userinitiated", "deny", "a", &[]); + let out_a = worker_a.join().await; + assert_eq!(out_a.result, "denied", "worker A is denied"); + assert_eq!(out_a.launches, 1, "worker A opens one browser"); + + // Worker B arrives after A has fully completed and the attempt record is + // already written with the new generation. B snapshots the current + // (advanced) generation, acquires the lock, sees no further advance, and + // runs its own browser flow — it should succeed. + let worker_b = spawn_worker(&cfg, cache.path(), "userinitiated", "approve", "b", &[]); + let out_b = worker_b.join().await; + assert_eq!( + out_b.result, "ok", + "post-failure UserInitiated worker B runs its own flow and succeeds" + ); + assert_eq!( + out_b.launches, 1, + "worker B opens its own browser (not inherited from A)" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one code exchange (worker B's own approval)" + ); +} + +// ---- cross-process: adopter must NOT re-write the attempt generation ------- +// +// Proves that an adopting process B does not advance the attempt-sidecar +// generation, so a third process C — which arrives AFTER A's failure but sees +// no generation advance (B didn't re-write) — correctly runs its own attempt. +// +// Protocol ordering (deterministic via markers, no timing): +// 1. A (UserInitiated, deny-scripted) holds the lock mid-browser via +// LAUNCHED_MARKER + PROCEED_MARKER. +// 2. B (UserInitiated, deny-scripted) starts while A holds the lock. +// B emits SNAPSHOT_MARKER after snapshotting gen=0 and before queueing +// on the lock. Parent observes the marker, then signals A's proceed. +// 3. A: denial recorded, writes gen=1 to the attempt sidecar, releases lock. +// 4. B: acquires lock, sees gen=1 > snap=0, intent matches → adopts A's +// denial. With the fix B does NOT re-write the sidecar. With the mutation +// (restoring the deleted write_attempt at the adoption site) B writes +// gen=2. +// 5. After A and B finish: assert sidecar generation == 1. This is the +// discriminating assertion — it FAILS when the adoption-site re-write is +// restored (gen becomes 2 instead of 1). +// 6. C (UserInitiated, approve-scripted) starts fresh. C's snapshot == gen +// on disk (1 with fix, 2 with mutation). In both cases C sees no advance +// and runs its own browser flow. code_grants increments by 1 for C. +// +// This test is cache-free (no seed_cache / disk-token assertions) so it runs +// on Windows as well as Unix. + +#[tokio::test] +async fn test_crossprocess_adopter_does_not_advance_generation() { + let stub = spawn_stub(false).await; // deny does not hit any endpoint + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + // ---- Phase 1: A holds the lock mid-browser ---------------------------- + let launched_a = cache.path().join("a.launched"); + let proceed_a = cache.path().join("a.proceed"); + + let worker_a = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "deny", + "a", + &[ + ("AUTH_WORKER_LAUNCHED_MARKER", launched_a.as_path()), + ("AUTH_WORKER_PROCEED_MARKER", proceed_a.as_path()), + ], + ); + + // Wait until A holds the lock and its browser is open. + wait_for_marker(&launched_a, "worker A browser launch").await; + + // ---- Phase 2: B queues behind A, snapshot barrier --------------------- + // B is UserInitiated + deny-scripted, but B will adopt A's denial rather + // than opening its own browser (B was queued while A held the lock). + // SNAPSHOT_MARKER is emitted by the tracing layer in B's process after B + // snapshots gen=0 and before it queues on the lock — so observing it + // proves B captured generation 0 before A records generation 1. + let snapshot_b = cache.path().join("b.snapshot"); + let worker_b = spawn_worker( + &cfg, + cache.path(), + "userinitiated", + "deny", + "b", + &[("AUTH_WORKER_SNAPSHOT_MARKER", snapshot_b.as_path())], + ); + + // Wait until B has snapshotted gen=0, then release A. + wait_for_marker(&snapshot_b, "worker B snapshot").await; + + // ---- Phase 3: release A, let A fail and write gen=1 ------------------- + std::fs::write(&proceed_a, b"go").unwrap(); + let out_a = worker_a.join().await; + assert_eq!(out_a.result, "denied", "worker A is denied"); + assert_eq!(out_a.launches, 1, "worker A opens exactly one browser"); + + // ---- Phase 4: B adopts (does NOT re-write the sidecar) ---------------- + let out_b = worker_b.join().await; + assert_eq!( + out_b.result, "denied", + "worker B adopts A's denial — it does not open a second browser" + ); + assert_eq!( + out_b.launches, 0, + "worker B adopts without opening a browser" + ); + + // ---- Phase 5: discriminating generation check ------------------------- + // With the fix: sidecar gen == 1 (B did not re-write). + // Mutation check: restore the deleted `write_attempt` at the adoption site + // → B writes gen=2 → this assertion FAILS. + let sidecar = attempt_sidecar_path(&cfg, cache.path()); + let raw = std::fs::read(&sidecar).expect("attempt sidecar written by A"); + let record: serde_json::Value = serde_json::from_slice(&raw).expect("sidecar parses as JSON"); + assert_eq!( + record.get("generation").and_then(|v| v.as_u64()), + Some(1), + "adopter B must not advance the sidecar generation (gen must stay at 1, not 2)" + ); + + // ---- Phase 6: C runs its own attempt ---------------------------------- + // C arrives after A's failure. C's snapshot equals the on-disk generation + // (1 with fix, 2 with mutation). Either way C sees no advance and runs its + // own browser flow. But the sidecar check above already catches the + // mutation; C proves the end-to-end behaviour. + let worker_c = spawn_worker(&cfg, cache.path(), "userinitiated", "approve", "c", &[]); + let out_c = worker_c.join().await; + assert_eq!( + out_c.result, "ok", + "worker C (fresh arrival after A's failure) runs its own flow and succeeds" + ); + assert_eq!( + out_c.launches, 1, + "worker C opens its own browser — not inherited from A or B" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "exactly one code exchange — C's own approval (A was denied; B adopted without exchange)" + ); +} + +// ---- cross-process: a waiter with a different rejected must not inherit ---- +// +// Cross-process mirror of the in-process test above: process A carries +// `rejected = "X"` and the refresh stickily re-issues "X" → A's attempt +// records RefreshRejected with `rejected_digest = sha256("X")`. Process B +// waits on the lock with `rejected = "Y"` (different). When B acquires the +// lock and reads the attempt record, the digest mismatch causes B to run its +// own attempt rather than adopt A's failure — B's refresh gets "X", which is +// valid for B, so B succeeds. +// +// Ordering is established with deterministic markers and the in-process stub +// gate, not timing: +// 1. A spawns (headless, rejected="X"). The stub holds A's refresh response +// until the parent calls `gate.release()`. +// 2. Parent waits for `gate.wait_for_request()` — proves A has acquired the +// lock and is mid-refresh (the request arrived at the stub). +// 3. Parent spawns B (headless, rejected="Y", SNAPSHOT_MARKER=b.snapshot). +// 4. Parent waits for B's snapshot marker — proves B has snapshotted gen=0 +// and is queued on the lock. +// 5. Parent calls `gate.release()`: stub returns "X" to A. A finishes with +// RefreshRejected(digest(X)), writes sidecar gen=1, releases lock. +// 6. B acquires: gen=1 > snap=0, digest(Y) ≠ digest(X) → B runs its own +// refresh → gets "X" → Ok("X"). +// +// Mutation check (no digest gating): B adopts A's RefreshRejected → +// refresh_grants stays at 1 → `refresh_grants == 2` assertion FAILS. + +#[cfg(unix)] +#[tokio::test] +async fn test_crossprocess_waiter_with_different_rejected_does_not_adopt_leaders_failure() { + // Stub stickily returns "X" but holds each response until released. + let (stub, gate) = spawn_stub_with_held_refresh(HeldRefreshResponse::Sticky("X")).await; + let cache = TempDir::new().unwrap(); + let cfg = config(&stub, "/disco/a", cache.path()); + + // Seed a token entry so both workers have a refresh token to exercise. + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "expired-seed", + "refresh_token": "live-refresh", + "expires_at": 1u64, + }), + ); + + let result_a = cache.path().join("a.result.json"); + let result_b = cache.path().join("b.result.json"); + let snapshot_b = cache.path().join("b.snapshot"); + + // ---- Phase 1: spawn A. A will acquire the lock and immediately call the + // stub's refresh endpoint; the stub holds the response. + let mut cmd_a = tokio::process::Command::new(env!("CARGO_BIN_EXE_auth-worker")); + cmd_a + .env("AUTH_WORKER_DISCOVERY_URL", &cfg.discovery_url) + .env("AUTH_WORKER_CACHE_DIR", cache.path()) + .env("AUTH_WORKER_NAMESPACE", &cfg.cache_namespace) + .env("AUTH_WORKER_CLIENT_ID", &cfg.client_id) + .env("AUTH_WORKER_SCOPES", cfg.scopes.join(",")) + .env("AUTH_WORKER_INTENT", "headless") + .env("AUTH_WORKER_SCRIPT", "failopen") // headless never browses + .env("AUTH_WORKER_REJECTED", "X") + .env("AUTH_WORKER_RESULT", &result_a) + .kill_on_drop(true); + + let child_a = cmd_a.spawn().expect("spawn worker A"); + + // ---- Phase 2: wait until the stub has received A's refresh request. + // This is an in-process await — no polling or timing needed. + // Once the stub is holding A's request, A owns the lock. + gate.wait_for_request().await; + + // ---- Phase 3: spawn B with SNAPSHOT_MARKER. + let mut cmd_b = tokio::process::Command::new(env!("CARGO_BIN_EXE_auth-worker")); + cmd_b + .env("AUTH_WORKER_DISCOVERY_URL", &cfg.discovery_url) + .env("AUTH_WORKER_CACHE_DIR", cache.path()) + .env("AUTH_WORKER_NAMESPACE", &cfg.cache_namespace) + .env("AUTH_WORKER_CLIENT_ID", &cfg.client_id) + .env("AUTH_WORKER_SCOPES", cfg.scopes.join(",")) + .env("AUTH_WORKER_INTENT", "headless") + .env("AUTH_WORKER_SCRIPT", "failopen") + .env("AUTH_WORKER_REJECTED", "Y") + .env("AUTH_WORKER_RESULT", &result_b) + .env("AUTH_WORKER_SNAPSHOT_MARKER", &snapshot_b) + .kill_on_drop(true); + + let child_b = cmd_b.spawn().expect("spawn worker B"); + + // ---- Phase 4: wait for B's snapshot marker. The tracing layer in B fires + // this after B snapshots gen=0 and before it waits for the + // lock — proves B holds snap=0 and is queued behind A. + wait_for_marker(&snapshot_b, "worker B snapshot").await; + + // ---- Phase 5: release A. Stub returns "X"; A records RefreshRejected + // with digest(X), advances gen to 1, releases the lock. + gate.release(); + + let worker_a = Worker { + child: child_a, + result_path: result_a, + }; + let worker_b = Worker { + child: child_b, + result_path: result_b, + }; + let (out_a, out_b) = tokio::join!(worker_a.join(), worker_b.join()); + + // A (rejected=X): refresh returns "X" → RefreshRejected. + // Sidecar: gen=1, result=refresh_rejected, rejected_digest=sha256("X"). + assert_eq!( + out_a.result, "refresh_rejected", + "worker A (rejected=X) must get RefreshRejected" + ); + // B (rejected=Y): gen=1 > snap=0, digest(Y) ≠ digest(X) → B runs its + // own refresh. B's refresh returns "X"; finish(rejected=Y, token=X) → Ok. + assert_eq!( + out_b.result, "ok", + "worker B (rejected=Y) must succeed after rerunning — not adopt A's RefreshRejected" + ); + // Mutation check (r8 shape, no digest gate): B adopts → refresh_grants + // stays 1. With the digest fix: B reruns → refresh_grants = 2. + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 2, + "both workers run their own refresh — digest mismatch prevented adoption" + ); +} + +// ---- P1-3 non-Unix read path disabled ----------------------------------- +// +// On non-Unix platforms (Windows) token files written by older builds with +// default ACLs should not be consumed by new builds. `read_private_cache` +// returns an error on non-Unix (and opportunistically removes the legacy +// file), so `read_cache` yields `None` and the source behaves as if no +// cached token exists — memory-only cache on non-Unix. +// +// This test uses a cfg-gated stub: on Unix it only exercises the Unix read +// path (as a sanity check); the Windows behavior is proved by the +// `#[cfg(not(unix))]` branch of `read_private_cache` and verified by the +// Windows CI build + manual testing on the Windows runner. The test is written +// to compile on all platforms and asserts the platform-appropriate invariant. + +#[tokio::test] +async fn test_non_unix_does_not_serve_legacy_on_disk_token() { + // Seed a token that would be served from disk on Unix (unexpired, valid). + let stub = spawn_stub(false).await; // fresh token on refresh/browser + let cache = TempDir::new().unwrap(); + let opener = ScriptedOpener::new(Script::Approve); + let cfg = config(&stub, "/disco/a", cache.path()); + + seed_cache( + &cfg, + cache.path(), + json!({ + "access_token": "legacy-windows-token", + "refresh_token": "legacy-refresh", + "expires_at": future_secs(), + }), + ); + + let src = PkceOAuthTokenSource::new_with(cfg.clone(), Arc::new(opener.clone())).unwrap(); + + #[cfg(unix)] + { + // On Unix the cache is read and served directly from disk — this is the + // expected behavior on a secured platform. + let token = src + .acquire_with_intent(AuthIntent::Headless, None) + .await + .expect("Unix serves the seeded token from disk"); + assert_eq!(token, "legacy-windows-token", "Unix: disk token served"); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 0, + "Unix: no refresh — the disk token was served directly" + ); + // The seeded file is still on disk (not removed on Unix). + assert!( + cache_file_path(&cfg, cache.path()).exists(), + "Unix: the cache file is preserved" + ); + } + + #[cfg(not(unix))] + { + // On non-Unix `read_private_cache` refuses to read the legacy file and + // attempts to remove it. Construction and bearer() behave as if no cache + // exists — the source falls through to a browser flow. + let token = src + .acquire_with_intent(AuthIntent::Auto, None) + .await + .expect("non-Unix: browser flow succeeds (no disk token served)"); + assert_ne!( + token, "legacy-windows-token", + "non-Unix: legacy token must not be served from disk" + ); + assert_eq!( + stub.code_grants.load(Ordering::SeqCst), + 1, + "non-Unix: browser flow ran — disk token was not served" + ); + assert_eq!( + stub.refresh_grants.load(Ordering::SeqCst), + 0, + "non-Unix: no refresh grant — the source went straight to the browser flow" + ); + // The legacy file should have been removed by read_private_cache. + assert!( + !cache_file_path(&cfg, cache.path()).exists(), + "non-Unix: legacy cache file is removed by read_private_cache" + ); + // No new token file was written (persist is a no-op on non-Unix). + // (The token is held in memory only.) + assert!( + !cache_file_path(&cfg, cache.path()).exists(), + "non-Unix: no new cache file created (memory-only)" + ); + } +} diff --git a/crates/buzz-agent/tests/fake_llm.rs b/crates/buzz-agent/tests/fake_llm.rs index fefd5a24c5d..9822243d5fe 100644 --- a/crates/buzz-agent/tests/fake_llm.rs +++ b/crates/buzz-agent/tests/fake_llm.rs @@ -80,19 +80,35 @@ async fn spawn_capturing_fake_llm(responses: Vec) -> (String, Arc, ) -> (String, Arc>>) { + let captures: Arc>> = Arc::new(Mutex::new(Vec::new())); + let url = spawn_capturing_fake_llm_core(responses, captures.clone(), None).await; + (url, captures) +} + +/// Shared connection loop for the capturing fake LLM: reads each request, +/// records its JSON body into `captures`, and replies with the next canned +/// response. When `gate` is `Some`, the FIRST request's response is withheld +/// until the gate fires; when `None`, every response is served immediately. +async fn spawn_capturing_fake_llm_core( + responses: Vec, + captures: Arc>>, + gate: Option>>>>, +) -> String { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let url = format!("http://{}", listener.local_addr().unwrap()); let queue = Arc::new(Mutex::new(VecDeque::from(responses))); - let captures: Arc>> = Arc::new(Mutex::new(Vec::new())); - let captures_clone = captures.clone(); tokio::spawn(async move { + let mut request_num = 0usize; loop { let (mut sock, _) = match listener.accept().await { Ok(p) => p, Err(_) => return, }; let queue = queue.clone(); - let captures = captures_clone.clone(); + let captures = captures.clone(); + let gate = gate.clone(); + request_num += 1; + let req_num = request_num; tokio::spawn(async move { // Read headers. let mut buf = Vec::new(); @@ -141,6 +157,15 @@ async fn spawn_capturing_fake_llm_with_statuses( captures.lock().await.push(parsed); } + // Hold the first request's response until the gate opens. + if req_num == 1 { + if let Some(gate) = &gate { + if let Some(rx) = gate.lock().await.take() { + let _ = rx.await; + } + } + } + // Send canned response. let response = queue.lock().await.pop_front().unwrap_or(CannedResponse { status: 500, @@ -164,6 +189,20 @@ async fn spawn_capturing_fake_llm_with_statuses( }); } }); + url +} + +/// A capturing fake LLM whose FIRST provider response is withheld until +/// `gate` fires. Later responses are served immediately. Used to make +/// round-boundary races deterministic: hold round 1 open until a client action +/// (e.g. a steer) is confirmed, so the second round observes it. Request bodies +/// are recorded into `captures` exactly as `spawn_capturing_fake_llm` does. +async fn spawn_gated_capturing_fake_llm( + responses: Vec, + captures: Arc>>, + gate: Arc>>>, +) -> (String, Arc>>) { + let url = spawn_capturing_fake_llm_core(responses, captures.clone(), Some(gate)).await; (url, captures) } @@ -774,14 +813,37 @@ async fn recv_active_run_id(h: &mut Harness) -> String { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn steer_folds_into_active_turn_without_cancelling() { + use tokio::sync::oneshot; + // A two-round turn (tool call → text). A steer sent once the run is live // must (a) be accepted with the matching runId, (b) NOT cancel the turn — // it still ends with end_turn — and (c) reach the provider as a user turn. - let (url, captures) = spawn_capturing_fake_llm(vec![ - openai_tool_call("call_steer", "fake__noop", json!({})), - openai_text("acknowledged the steer"), - ]) - .await; + // + // The steer is drained only at a round boundary (before the next provider + // request), so it must be enqueued before round 2 begins. Without + // synchronization a fast worker can complete round 1, drain an empty steer + // queue at the round-2 boundary, and dispatch round 2 before the steer is + // even sent — the steer then lands after the turn ends and never reaches + // the provider. To make this deterministic, the FIRST provider response is + // gated: it is withheld until the steer has been sent AND observed + // accepted, so round 1 cannot complete (and round 2 cannot start its drain) + // until the steer is already queued. + let (gate_tx, gate_rx) = oneshot::channel::<()>(); + let gate_rx = Arc::new(Mutex::new(Some(gate_rx))); + + let responses = vec![ + CannedResponse { + status: 200, + body: openai_tool_call("call_steer", "fake__noop", json!({})), + }, + CannedResponse { + status: 200, + body: openai_text("acknowledged the steer"), + }, + ]; + let captures: Arc>> = Arc::new(Mutex::new(Vec::new())); + let (url, _) = spawn_gated_capturing_fake_llm(responses, captures.clone(), gate_rx).await; + let mut h = Harness::spawn(&url).await; let sid = init_session(&mut h).await; @@ -795,7 +857,8 @@ async fn steer_folds_into_active_turn_without_cancelling() { ) .await; - // Learn the run id, then steer into it before the turn finishes. + // Learn the run id (advertised before the gated round-1 request), then steer + // into the live turn while round 1 is still held. let run_id = recv_active_run_id(&mut h).await; let steer_text = "STEER-CANARY: also consider the edge case"; let s_id = h @@ -809,9 +872,12 @@ async fn steer_folds_into_active_turn_without_cancelling() { ) .await; - // Steer is accepted and echoes the run id it landed in. + // Steer is accepted and echoes the run id it landed in. Only after this + // confirmation do we release the gate, so the steer is guaranteed queued + // before round 2's boundary drains it. let mut steer_ok = false; let mut end_turn = false; + let mut gate = Some(gate_tx); for _ in 0..40 { let v = h.recv().await; if v["id"] == json!(s_id) { @@ -827,6 +893,11 @@ async fn steer_folds_into_active_turn_without_cancelling() { "steer reply carries a messageId" ); steer_ok = true; + // Steer accepted — release round 1 so the turn proceeds to round 2, + // whose boundary now drains the queued steer. + if let Some(tx) = gate.take() { + let _ = tx.send(()); + } } else if v["id"] == json!(p_id) { // The turn was NOT cancelled — it completed normally. assert_eq!(v["result"]["stopReason"], "end_turn"); diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 68b702431af..7a3669f440e 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -1041,6 +1041,7 @@ dependencies = [ "axum", "base64 0.22.1", "dirs", + "fs2", "getrandom 0.4.3", "hex", "nix 0.31.3", @@ -3045,6 +3046,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "fs_extra" version = "1.3.0"