diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 7a356df8c8..c4a682ee3f 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1,10 +1,34 @@ import type { KiroOAuthMetadata, OAuthController, OAuthCredentials } from "./types"; import { parseCallbackInput } from "./callback-server"; import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types"; -import { loadConfig, resolveEnvValue, saveConfig } from "../config"; +import { ConfigMutationLockError, loadConfig, resolveEnvValue, saveConfig } from "../config"; import { maskEmail } from "../lib/privacy"; import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro"; -import { getAccountCredential, getAccountCredentialWithStatus, getAccountSet, removeAccount, saveAccountCredential, saveCredential, setActiveAccount, getCredential, credentialGeneration, createOAuthRefreshIntentLock, mergeAccountCredential, markAccountNeedsReauthIfGeneration, readOAuthRefreshIntent, writeOAuthRefreshIntent, markOAuthRefreshIntentStaleOwner, clearOAuthRefreshIntent, normalizeAuthStoreBuffer, OAuthMutationBusyError } from "./store"; +import { + OAuthMutationBusyError, + OAuthRefreshIntentIOError, + clearOAuthRefreshIntent, + clearOAuthRefreshIntentIfMatch, + createOAuthRefreshIntentLock, + credentialGeneration, + getAccountCredential, + getAccountCredentialWithStatus, + getAccountSet, + getCredential, + markAccountNeedsReauthIfGeneration, + markOAuthRefreshIntentCleanupPending, + markOAuthRefreshIntentStaleOwner, + mergeAccountCredential, + normalizeAuthStoreBuffer, + readOAuthRefreshIntent, + removeAccount, + saveAccountCredential, + saveCredential, + setActiveAccount, + writeOAuthRefreshIntent, + type OAuthRefreshIntent, + type OAuthRefreshIntentCleanupPending, +} from "./store"; import { loginXai, refreshXaiToken, XAI_LOCAL_CLI_DETACH_WARNING, XaiTokenRequestError } from "./xai"; import { ANTHROPIC_OAUTH_BETA, AnthropicTokenError, loginAnthropic, refreshAnthropicToken } from "./anthropic"; import { loginKimi, refreshKimiToken } from "./kimi"; @@ -559,9 +583,152 @@ function terminal(error:unknown):boolean{ // Local durable-write/read/cleanup failures are operational, not credential // death: the provider credential was never rejected or consumed. Never mark // the account needsReauth for broken local persistence infrastructure. - if (error instanceof RefreshIntentIOError) return false; + if (error instanceof RefreshIntentIOError || error instanceof OAuthRefreshIntentIOError) return false; return isTerminalRefreshError(error); } + +/** + * True when the token endpoint definitively answered and rejected the request. + * + * The Anthropic adapter attaches an HTTP status only to an explicit non-success response, + * which is the retryable rejection this PR handles. Everything else (timeout, dropped + * connection, a body that could not be read or parsed, or a local persistence fault) leaves + * the outcome unknown: the server may already have rotated the token, and a blind replay + * could trip refresh-token-reuse revocation. Those cases must keep the refresh intent. + * + * Deliberately narrower than `terminal()`, which asks whether the CREDENTIAL is dead. + * This asks the different question of whether the ATTEMPT is known to have failed. + */ +function definitivelyAnswered(error: unknown): boolean { + if (error instanceof AnthropicTokenError) return error.httpStatus !== undefined; + return false; +} + +/** + * Intent cleanup is secondary to the refresh outcome it protects. + * + * Once a credential is already durable, cleanup remains secondary and best-effort. A known + * failed attempt takes the stricter path below: its retry-safe marker must become durable before + * the original provider error can be returned. + */ +function clearAnthropicRefreshIntentBestEffort( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, +): boolean { + try { + return expected.attemptId + ? clearOAuthRefreshIntentIfMatch(provider, accountId, expected) + : clearOAuthRefreshIntent(provider, accountId, expected.generation); + } catch { + console.warn( + "[opencodex] Anthropic refresh intent cleanup failed; preserving the durable replay guard.", + ); + return false; + } +} + +const ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS = [10, 25, 50] as const; + +function isConfigMutationLockContention(error: unknown): boolean { + if (!(error instanceof ConfigMutationLockError)) return false; + const cause = error.cause; + const code = cause && typeof cause === "object" && "code" in cause + ? String((cause as { code?: unknown }).code) + : ""; + return code === "SQLITE_BUSY" || code === "SQLITE_LOCKED"; +} + +async function clearAnthropicRefreshIntentForKnownFailure( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + cleanupPending: OAuthRefreshIntentCleanupPending, + refreshError: unknown, +): Promise { + let marked: OAuthRefreshIntent | undefined; + for (let attempt = 0; attempt <= ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS.length; attempt += 1) { + try { + marked = markOAuthRefreshIntentCleanupPending( + provider, + accountId, + expected, + cleanupPending, + ); + break; + } catch (cause) { + const retryDelay = ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS[attempt]; + if (!isConfigMutationLockContention(cause) || retryDelay === undefined) { + throw new OAuthRefreshIntentIOError( + "mark-cleanup-pending", + cause, + refreshError, + ); + } + // The provider has definitively answered, so caller cancellation no longer changes the + // settlement obligation. Yield briefly while retaining the per-account refresh lock, then + // rerun the existing compare-and-swap marker against current disk state. + await Bun.sleep(retryDelay); + } + } + if (!marked) { + throw new OAuthRefreshIntentIOError( + "mark-cleanup-pending", + new Error("Anthropic refresh intent changed before safe cleanup"), + refreshError, + ); + } + + let cleared: boolean; + try { + cleared = clearOAuthRefreshIntentIfMatch(provider, accountId, marked); + } catch { + console.warn( + "[opencodex] Anthropic refresh intent cleanup failed; retry-safe cleanup remains pending.", + ); + return false; + } + if (!cleared) { + throw new OAuthRefreshIntentIOError( + "clear-cleanup-pending", + new Error("Anthropic refresh intent changed during safe cleanup"), + refreshError, + ); + } + return true; +} + +function resumeAnthropicRefreshIntentCleanup( + provider: string, + accountId: string, + pendingIntent: OAuthRefreshIntent, +): void { + let cleared: boolean; + try { + cleared = clearOAuthRefreshIntentIfMatch(provider, accountId, pendingIntent); + } catch (cause) { + throw new OAuthRefreshIntentIOError( + "resume-cleanup", + cause, + ); + } + if (!cleared) { + throw new OAuthRefreshIntentIOError( + "resume-cleanup", + new Error("Pending Anthropic refresh intent changed before cleanup"), + ); + } +} + +function clearObservedAnthropicRefreshIntent( + provider: string, + accountId: string, + pendingIntent: OAuthRefreshIntent, +): boolean { + return pendingIntent.attemptId + ? clearOAuthRefreshIntentIfMatch(provider, accountId, pendingIntent) + : clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); +} function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OAuthCredentials{if(stored.source!=="local-cli")return stored;const disk=detectGrokCliToken();if(!disk)return stored;const allowed=isSameGrokIdentity(stored,disk)||(active&&!hasComparableGrokIdentity(stored,disk));return allowed&&shouldAdoptGrokGeneration(stored,disk,now(),REFRESH_SKEW_MS)?disk:stored;} function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials { return { @@ -647,13 +814,19 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + // The disk credential is already durable here, so cleanup is secondary: an unlink + // failure must not mask a committed credential by throwing over the return below. + if (pendingIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, pendingIntent); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + if (pendingIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, pendingIntent); return disk.access; } + if (pendingIntent?.cleanupPending && pendingIntent.generation === generation) { + resumeAnthropicRefreshIntentCleanup(provider, accountId, pendingIntent); + pendingIntent = undefined; + } if (!pendingIntent?.uncertain && pendingIntent?.generation === generation) { if (pendingIntent.staleOwner) throw new OAuthTokenRefreshStaleError(); if (deps.replacedStaleFlight && pendingIntent.flightId === deps.replacedStaleFlight.flightId) { @@ -661,7 +834,9 @@ export async function refreshAnthropicAccountWithLock( markOAuthRefreshIntentStaleOwner(provider, accountId, generation, deps.replacedStaleFlight.flightId); throw new OAuthTokenRefreshStaleError(); } - clearOAuthRefreshIntent(provider, accountId, generation); + if (!clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent)) { + throw new OAuthTokenRefreshStaleError(); + } pendingIntent = undefined; } } @@ -669,7 +844,9 @@ export async function refreshAnthropicAccountWithLock( await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); throw new OAuthLoginRequiredError(provider); } - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + if (pendingIntent && !clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent)) { + throw new OAuthTokenRefreshStaleError(); + } if (account?.needsReauth) { throw new OAuthLoginRequiredError(provider); } @@ -677,9 +854,15 @@ export async function refreshAnthropicAccountWithLock( return stored.access; } + let refreshMayHaveReachedProvider = false; + let attemptIntent: OAuthRefreshIntent | undefined; try { - writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId); + attemptIntent = writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId); if (deps.signal?.aborted) throw deps.signal.reason; + // From this point on, even a synchronous client error is conservatively post-dispatch: + // the provider may have received and rotated the refresh token before the caller learned + // the outcome. + refreshMayHaveReachedProvider = true; if (deps.flight) deps.flight.dispatched = true; const fresh = merged(await def.refresh(stored.refresh, deps.signal), stored); const outcome = await mergeAccountCredential(provider, accountId, fresh, { @@ -687,17 +870,41 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - clearOAuthRefreshIntent(provider, accountId, generation); + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } - clearOAuthRefreshIntent(provider, accountId, generation); + // The rotated credential is durable now. A cleanup failure must not turn that committed + // success into a refresh failure; the old-generation intent remains a conservative guard. + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); return fresh.access; } catch (error) { - if (error instanceof OAuthMutationBusyError) throw error; - if (!terminal(error)) throw error; + if (error instanceof OAuthMutationBusyError || error instanceof OAuthTokenRefreshStaleError) throw error; + if (!terminal(error)) { + // A non-terminal failure tells the caller to retry, but the intent outlived it, so + // the next attempt hit the pending-intent branch above and raised + // OAuthLoginRequiredError. One 503 locked the account out of refresh until manual + // re-auth even after upstream recovered. + // + // Only clear the intent when the server DEFINITIVELY answered and rejected the + // request. The adapter attaches an HTTP status only to that explicit non-success + // response. A timeout, a dropped connection, or an unreadable/unparseable body + // carries no status: the server may already have + // rotated the token, and replaying it could trip refresh-token-reuse revocation. + // Those outcomes keep the intent so the guard still refuses a blind replay. + if ((!refreshMayHaveReachedProvider || definitivelyAnswered(error)) && attemptIntent) { + await clearAnthropicRefreshIntentForKnownFailure( + provider, + accountId, + attemptIntent, + refreshMayHaveReachedProvider ? "definitive-rejection" : "pre-dispatch", + error, + ); + } + throw error; + } await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); - clearOAuthRefreshIntent(provider, accountId, generation); + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); throw new OAuthLoginRequiredError(provider); } } finally { diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 755d86f3e2..bc9e0e11c1 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -19,7 +19,7 @@ import { createHash, randomUUID } from "node:crypto"; import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret } from "../config"; +import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret, withConfigMutationLockSync } from "../config"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { MAX_PENDING_OAUTH_MUTATIONS } from "../lib/translator-budget"; @@ -67,23 +67,93 @@ export function getAuthRefreshIntentLockPath(provider: string, accountId: string export function getAuthRefreshIntentPath(provider: string, accountId: string): string { return `${getAuthRefreshIntentLockPath(provider, accountId)}.json`; } -export interface OAuthRefreshIntent { version: 1; provider: string; accountId: string; generation: string; createdAt: number; flightId?: string; staleOwner?: true; uncertain?: true } +export type OAuthRefreshIntentCleanupPending = "pre-dispatch" | "definitive-rejection"; +export interface OAuthRefreshIntent { + version: 1; + provider: string; + accountId: string; + generation: string; + createdAt: number; + attemptId?: string; + flightId?: string; + staleOwner?: true; + uncertain?: true; + cleanupPending?: OAuthRefreshIntentCleanupPending; +} + +export class OAuthRefreshIntentIOError extends Error { + readonly code = "OAUTH_REFRESH_INTENT_IO"; + constructor( + readonly operation: "write-intent" | "mark-cleanup-pending" | "clear-cleanup-pending" | "resume-cleanup", + cause?: unknown, + readonly refreshError?: unknown, + ) { + super(`OAuth refresh intent ${operation} failed`, cause ? { cause } : undefined); + this.name = "OAuthRefreshIntentIOError"; + } +} + +// Both compare-and-swap sites in this file — the OAuth file lock and the refresh-intent +// file — must agree on what "the same file" means, so they share one snapshot shape and +// one identity check (`snapshot`/`sameSnapshot` below) rather than two copies that can drift. +type OAuthRefreshIntentFileSnapshot = LockSnapshot; + +export interface OAuthRefreshIntentMutationHooks { + beforeMarkCommit?: () => void; + beforeClearRecheck?: () => void; +} + +class OAuthRefreshIntentChangedError extends Error {} + +function uncertainOAuthRefreshIntent(provider: string, accountId: string): OAuthRefreshIntent { + return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; +} + function parseOAuthRefreshIntent( provider: string, accountId: string, raw: string, ): OAuthRefreshIntent { - const value = JSON.parse(raw) as Partial; + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return uncertainOAuthRefreshIntent(provider, accountId); + } + const value = parsed as Partial; if ( value.version !== 1 || value.provider !== provider || value.accountId !== accountId || typeof value.generation !== "string" + || !/^[0-9a-f]{64}$/.test(value.generation) || typeof value.createdAt !== "number" - || (value.flightId !== undefined && typeof value.flightId !== "string") + || !Number.isFinite(value.createdAt) + || !Number.isInteger(value.createdAt) + || value.createdAt < 0 + || ( + value.attemptId !== undefined + && (typeof value.attemptId !== "string" || value.attemptId.length === 0 || value.attemptId.length > 128) + ) + || ( + value.flightId !== undefined + && (typeof value.flightId !== "string" || value.flightId.length === 0 || value.flightId.length > 128) + ) || (value.staleOwner !== undefined && value.staleOwner !== true) + || (value.uncertain !== undefined && value.uncertain !== true) + || ( + value.cleanupPending !== undefined + && value.cleanupPending !== "pre-dispatch" + && value.cleanupPending !== "definitive-rejection" + ) + || ( + value.cleanupPending !== undefined + && ( + value.attemptId === undefined + || value.uncertain === true + || value.staleOwner === true + ) + ) ) { - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } return value as OAuthRefreshIntent; } @@ -96,7 +166,7 @@ export function readOAuthRefreshIntent(provider: string, accountId: string): OAu return parseOAuthRefreshIntent(provider, accountId, readFileSync(path, "utf8")); } catch (error) { if (errorCode(error) === "ENOENT") return undefined; - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } } @@ -107,27 +177,159 @@ export function peekOAuthRefreshIntent(provider: string, accountId: string): OAu return parseOAuthRefreshIntent(provider, accountId, readFileSync(path, "utf8")); } catch (error) { if (errorCode(error) === "ENOENT") return undefined; - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } } -export function writeOAuthRefreshIntent(provider: string, accountId: string, generation: string, createdAt = Date.now(), flightId?: string): void { +export function writeOAuthRefreshIntent(provider: string, accountId: string, generation: string, createdAt = Date.now(), flightId?: string): OAuthRefreshIntent { const dir = getConfigDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); hardenConfigDir(); - const intent: OAuthRefreshIntent = { version: 1, provider, accountId, generation, createdAt, ...(flightId ? { flightId } : {}) }; - atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify(intent)}\n`); + const intent: OAuthRefreshIntent = { + version: 1, + provider, + accountId, + generation, + createdAt, + attemptId: randomUUID(), + ...(flightId ? { flightId } : {}), + }; + return withConfigMutationLockSync(() => { + if (readOAuthRefreshIntent(provider, accountId) !== undefined) { + throw new OAuthRefreshIntentIOError( + "write-intent", + new Error("An OAuth refresh intent already exists; refusing to overwrite its replay guard"), + ); + } + atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify(intent)}\n`); + return intent; + }); +} + +function sameOAuthRefreshIntentAttempt(current: OAuthRefreshIntent, expected: OAuthRefreshIntent): boolean { + return current.provider === expected.provider + && current.accountId === expected.accountId + && current.generation === expected.generation + && current.createdAt === expected.createdAt + && current.attemptId === expected.attemptId + && current.flightId === expected.flightId; +} + +function exactOAuthRefreshIntentFile( + provider: string, + accountId: string, +): { intent: OAuthRefreshIntent; snapshot: OAuthRefreshIntentFileSnapshot } | undefined { + const path = getAuthRefreshIntentPath(provider, accountId); + try { + hardenConfigDir(); + hardenExistingSecret(path); + const file = snapshot(path); + let intent: OAuthRefreshIntent; + try { intent = parseOAuthRefreshIntent(provider, accountId, file.bytes); } + catch { intent = uncertainOAuthRefreshIntent(provider, accountId); } + return { intent, snapshot: file }; + } catch (error) { + if (errorCode(error) === "ENOENT") return undefined; + throw error; + } +} + +export function markOAuthRefreshIntentCleanupPending( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + cleanupPending: OAuthRefreshIntentCleanupPending, + hooks: OAuthRefreshIntentMutationHooks = {}, +): OAuthRefreshIntent | undefined { + return withConfigMutationLockSync(() => { + const path = getAuthRefreshIntentPath(provider, accountId); + const observed = exactOAuthRefreshIntentFile(provider, accountId); + const current = observed?.intent; + if ( + !observed + || !current + || expected.attemptId === undefined + || current.uncertain + || current.staleOwner + || !sameOAuthRefreshIntentAttempt(current, expected) + || (current.cleanupPending !== undefined && current.cleanupPending !== cleanupPending) + ) return undefined; + const marked: OAuthRefreshIntent = { ...current, cleanupPending }; + try { + atomicWriteFile(path, `${JSON.stringify(marked)}\n`, undefined, { + beforeRename: hooks.beforeMarkCommit, + validateBeforeRename: targetPath => { + let latest: OAuthRefreshIntentFileSnapshot; + try { latest = snapshot(targetPath); } + catch (error) { + throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit", { cause: error }); + } + if (!sameSnapshot(observed.snapshot, latest)) { + throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit"); + } + }, + }); + } catch (error) { + if (error instanceof OAuthRefreshIntentChangedError) return undefined; + throw error; + } + return marked; + }); } + +export function clearOAuthRefreshIntentIfMatch( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + hooks: OAuthRefreshIntentMutationHooks = {}, +): boolean { + return withConfigMutationLockSync(() => { + const path = getAuthRefreshIntentPath(provider, accountId); + const observed = exactOAuthRefreshIntentFile(provider, accountId); + if (!observed) return true; + const current = observed.intent; + if ( + current.uncertain + || expected.uncertain + || current.attemptId === undefined + || expected.attemptId === undefined + || current.uncertain !== expected.uncertain + || current.staleOwner !== expected.staleOwner + || current.cleanupPending !== expected.cleanupPending + || !sameOAuthRefreshIntentAttempt(current, expected) + ) return false; + hooks.beforeClearRecheck?.(); + let latest: OAuthRefreshIntentFileSnapshot; + try { latest = snapshot(path); } + catch (error) { + if (errorCode(error) === "ENOENT") return true; + throw error; + } + if (!sameSnapshot(observed.snapshot, latest)) return false; + try { unlinkSync(path); return true; } + catch (error) { if (errorCode(error) === "ENOENT") return true; throw error; } + }); +} + export function markOAuthRefreshIntentStaleOwner(provider: string, accountId: string, generation: string, flightId: string): boolean { - const current = readOAuthRefreshIntent(provider, accountId); - if (current?.uncertain || current?.generation !== generation || current.flightId !== flightId) return false; - atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify({ ...current, staleOwner: true })}\n`); - return true; + return withConfigMutationLockSync(() => { + const current = readOAuthRefreshIntent(provider, accountId); + if ( + current?.uncertain + || current?.cleanupPending + || current?.generation !== generation + || current.flightId !== flightId + ) return false; + atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify({ ...current, staleOwner: true })}\n`); + return true; + }); } export function clearOAuthRefreshIntent(provider: string, accountId: string, generation: string): boolean { - const current = readOAuthRefreshIntent(provider, accountId); - if (!current || current.generation !== generation) return false; - try { unlinkSync(getAuthRefreshIntentPath(provider, accountId)); return true; } - catch (error) { if (errorCode(error) === "ENOENT") return false; throw error; } + return withConfigMutationLockSync(() => { + const current = readOAuthRefreshIntent(provider, accountId); + if (!current || current.generation !== generation) return false; + try { unlinkSync(getAuthRefreshIntentPath(provider, accountId)); return true; } + catch (error) { if (errorCode(error) === "ENOENT") return false; throw error; } + }); } export function credentialGeneration(cred: OAuthCredentials): string { return createHash("sha256").update(JSON.stringify([cred.refresh, cred.access, cred.expires])).digest("hex"); diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index 0975e8285c..3ba3e57ce8 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -7,7 +7,19 @@ import { getValidAccessToken, getValidAccessTokenForAccount, OAuthLoginRequiredE import { RefreshIntentIOError, nousRefreshIntentBlocksReplay } from "../src/oauth/nous"; import * as nousModule from "../src/oauth/nous"; import { AnthropicTokenError } from "../src/oauth/anthropic"; -import { credentialGeneration, getAccountCredential, getAccountSet, getAuthRefreshIntentPath, getCredential, markAccountNeedsReauth, readOAuthRefreshIntent, saveCredential, writeOAuthRefreshIntent } from "../src/oauth/store"; +import { + OAuthRefreshIntentIOError, + credentialGeneration, + getAccountCredential, + getAccountSet, + getAuthRefreshIntentPath, + getCredential, + markAccountNeedsReauth, + markOAuthRefreshIntentCleanupPending, + readOAuthRefreshIntent, + saveCredential, + writeOAuthRefreshIntent, +} from "../src/oauth/store"; import * as storeModule from "../src/oauth/store"; import * as configModule from "../src/config"; @@ -517,6 +529,423 @@ describe("oauth refresh hardening", () => { expect(getAccountSet("anthropic")!.accounts[0]!.needsReauth).toBe(true); }); + /** + * A definitive non-terminal HTTP failure is retryable: the endpoint answered with a failure, + * so the durable intent must not turn that promised retry into OAuthLoginRequiredError. + * A timeout or unreadable response is different — the provider may already have rotated the + * token, so that post-dispatch intent remains as the replay guard. + */ + test("a definitive transient Anthropic HTTP failure leaves the account refreshable", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const transient = new AnthropicTokenError("server", 503, undefined); + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, getAccountCredential("anthropic", id)!)).rejects.toBe(transient); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + + // Upstream recovers: the retry the caller was promised must actually succeed. + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }), + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + }); + + test("an Anthropic refresh with an uncertain post-dispatch outcome preserves its intent", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const timeout = new AnthropicTokenError("timeout", undefined, undefined); + let refreshCalls = 0; + const def = { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + throw timeout; + }, + }; + + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)).rejects.toBe(timeout); + + const pending = readOAuthRefreshIntent("anthropic", id); + expect(pending).toMatchObject({ generation }); + expect(pending?.cleanupPending).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(pending); + }); + + test("a pre-dispatch Anthropic abort clears its unconsumed refresh intent", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const aborted = new Error("aborted before dispatch"); + const controller = new AbortController(); + controller.abort(aborted); + let calls = 0; + + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { calls += 1; throw new Error("must not run"); }, + }, credential, { signal: controller.signal })).rejects.toBe(aborted); + + expect(calls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + }); + + test("a failed pre-dispatch cleanup remains safely retryable", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const controller = new AbortController(); + const aborted = new Error("aborted before dispatch"); + controller.abort(aborted); + const realClear = storeModule.clearOAuthRefreshIntentIfMatch; + let clearCalls = 0; + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((...args) => { + clearCalls += 1; + if (clearCalls === 1) throw new Error("intent unlink failed"); + return realClear(...args); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw new Error("must not dispatch"); }, + }, credential, { signal: controller.signal })).rejects.toBe(aborted); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("pre-dispatch"); + + let retryCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + retryCalls += 1; + return { access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }; + }, + }, credential)).resolves.toBe("fresh"); + expect(retryCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("a cleanup-marker persistence failure surfaces a typed error with the provider outcome", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const markerFailure = new configModule.ConfigMutationLockError( + "Could not acquire config mutation transaction", + { cause: Object.assign(new Error("intent marker write failed"), { code: "EACCES" }) }, + ); + let markerCalls = 0; + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => { + markerCalls += 1; + throw markerFailure; + }); + try { + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, credential); + } catch (error) { + rejection = error; + } + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "mark-cleanup-pending", + code: "OAUTH_REFRESH_INTENT_IO", + cause: markerFailure, + refreshError: transient, + }); + expect(markerCalls).toBe(1); + const pending = readOAuthRefreshIntent("anthropic", id); + expect(pending?.cleanupPending).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + + test("transient cleanup-marker lock contention settles a retryable rejection without reauth", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const realMark = storeModule.markOAuthRefreshIntentCleanupPending; + let markerCalls = 0; + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation((...args) => { + markerCalls += 1; + if (markerCalls <= 2) { + throw new configModule.ConfigMutationLockError( + "Config mutation already in progress", + { cause: Object.assign(new Error("database is locked"), { code: "SQLITE_BUSY" }) }, + ); + } + return realMark(...args); + }); + try { + let providerCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + providerCalls += 1; + throw transient; + }, + }, credential)).rejects.toBe(transient); + + expect(providerCalls).toBe(1); + expect(markerCalls).toBe(3); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }), + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(getAccountCredential("anthropic", id)?.access).toBe("fresh"); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + + test("persistent cleanup-marker lock contention stays bounded and preserves the replay guard", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const transient = new AnthropicTokenError("server", 503, undefined); + let markerCalls = 0; + let lastBusy: configModule.ConfigMutationLockError | undefined; + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => { + markerCalls += 1; + lastBusy = new configModule.ConfigMutationLockError( + "Config mutation already in progress", + { cause: Object.assign(new Error("database table is locked"), { code: "SQLITE_LOCKED" }) }, + ); + throw lastBusy; + }); + try { + let providerCalls = 0; + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + providerCalls += 1; + throw transient; + }, + }, credential); + } catch (error) { + rejection = error; + } + + expect(providerCalls).toBe(1); + expect(markerCalls).toBe(4); + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "mark-cleanup-pending", + code: "OAUTH_REFRESH_INTENT_IO", + cause: lastBusy, + refreshError: transient, + }); + expect(getAccountCredential("anthropic", id)).toEqual(credential); + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + + test("a failed definitive-rejection cleanup is retried before the next Anthropic request", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const transient = new AnthropicTokenError("server", 503, undefined); + const clearFailure = new Error("intent unlink failed"); + const realClear = storeModule.clearOAuthRefreshIntentIfMatch; + const events: string[] = []; + let clearCalls = 0; + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((...args) => { + clearCalls += 1; + if (clearCalls === 1) { + events.push("clear-fail"); + throw clearFailure; + } + events.push(args[2].cleanupPending ? "clear-recovery" : "clear-success"); + return realClear(...args); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + events.push("provider-503"); + throw transient; + }, + }, credential)).rejects.toBe(transient); + + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ + generation, + cleanupPending: "definitive-rejection", + }); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + + let retryCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + events.push("provider-retry"); + retryCalls += 1; + return { access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(retryCalls).toBe(1); + expect(clearCalls).toBe(3); + expect(events).toEqual([ + "provider-503", + "clear-fail", + "clear-recovery", + "provider-retry", + "clear-success", + ]); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("a persistently blocked cleanup fails operationally without replay or reauth", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const cleanupFailure = new Error("intent unlink failed"); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { + throw cleanupFailure; + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, credential)).rejects.toBe(transient); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); + + let retryCalls = 0; + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + retryCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, credential); + } catch (error) { + rejection = error; + } + + expect(retryCalls).toBe(0); + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "resume-cleanup", + code: "OAUTH_REFRESH_INTENT_IO", + cause: cleanupFailure, + }); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("Anthropic persistence failure after provider success preserves the replay guard", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const persistenceFailure = new Error("credential persistence failed"); + let refreshCalls = 0; + const def = { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }; + }, + }; + const mergeSpy = spyOn(storeModule, "mergeAccountCredential").mockImplementation(async () => { + throw persistenceFailure; + }); + let pendingAfterFailure: ReturnType; + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBe(persistenceFailure); + + const pending = readOAuthRefreshIntent("anthropic", id); + pendingAfterFailure = pending; + expect(pending).toMatchObject({ generation }); + expect(pending?.cleanupPending).toBeUndefined(); + expect(getAccountCredential("anthropic", id)).toEqual(credential); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + mergeSpy.mockRestore(); + } + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(pendingAfterFailure); + }); + + test("post-persist intent cleanup failure does not turn Anthropic refresh success into failure", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const oldGeneration = credentialGeneration(credential); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { + throw new Error("intent unlink failed"); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }), + }, credential)).resolves.toBe("fresh"); + + expect(getAccountCredential("anthropic", id)?.access).toBe("fresh"); + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation: oldGeneration }); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + test("Anthropic post-dispatch stale flight replacement stays retryable without replay or reauth", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId; @@ -638,6 +1067,135 @@ describe("oauth refresh hardening", () => { expect(readOAuthRefreshIntent("anthropic", id)).toEqual(legacy); expect(readOAuthRefreshIntent("anthropic", id)?.uncertain).toBeUndefined(); + let refreshCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(legacy); + }); + + test("cleanup-pending CAS never rewrites a foreign same-generation attempt", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const intent = writeOAuthRefreshIntent("anthropic", id, generation, Date.now()); + + for (const foreign of [ + { ...intent, generation: "foreign-generation" }, + { ...intent, attemptId: "foreign-attempt" }, + { ...intent, flightId: "foreign-flight" }, + { ...intent, createdAt: intent.createdAt + 1 }, + ]) { + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + foreign, + "definitive-rejection", + )).toBeUndefined(); + expect(storeModule.clearOAuthRefreshIntentIfMatch("anthropic", id, foreign)).toBe(false); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(intent); + } + }); + + test("a replaced obsolete intent blocks Anthropic redispatch", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const obsolete = writeOAuthRefreshIntent("anthropic", id, "0".repeat(64)); + const replacement = { ...obsolete, attemptId: "foreign-attempt" }; + writeFileSync(getAuthRefreshIntentPath("anthropic", id), `${JSON.stringify(replacement)}\n`); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockReturnValue(false); + let refreshCalls = 0; + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, credential)).rejects.toBeInstanceOf(OAuthTokenRefreshStaleError); + expect(refreshCalls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + } finally { + clearSpy.mockRestore(); + } + }); + + test("cleanup-pending marker commit preserves an attempt replaced during comparison", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const path = getAuthRefreshIntentPath("anthropic", id); + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const original = writeOAuthRefreshIntent("anthropic", id, generation, Date.now(), "original-flight"); + const replacement = { + ...original, + attemptId: "replacement-attempt", + flightId: "replacement-flight", + }; + + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + original, + "definitive-rejection", + { beforeMarkCommit: () => writeFileSync(path, `${JSON.stringify(replacement)}\n`) }, + )).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + }); + + test("exact cleanup preserves an attempt replaced before unlink", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const path = getAuthRefreshIntentPath("anthropic", id); + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const original = writeOAuthRefreshIntent("anthropic", id, generation, Date.now(), "original-flight"); + const replacement = { + ...original, + attemptId: "replacement-attempt", + flightId: "replacement-flight", + }; + + expect(storeModule.clearOAuthRefreshIntentIfMatch( + "anthropic", + id, + original, + { beforeClearRecheck: () => writeFileSync(path, `${JSON.stringify(replacement)}\n`) }, + )).toBe(false); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + }); + + test("an invalid cleanup-pending marker fails closed as uncertain", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const intent = writeOAuthRefreshIntent( + "anthropic", + id, + credentialGeneration(getAccountCredential("anthropic", id)!), + ); + const { attemptId: _attemptId, ...withoutAttemptId } = intent; + for (const invalid of [ + { ...intent, cleanupPending: "unsafe-retry" }, + { ...withoutAttemptId, cleanupPending: "definitive-rejection" }, + { ...intent, cleanupPending: "definitive-rejection", uncertain: true }, + { ...intent, cleanupPending: "definitive-rejection", staleOwner: true }, + ]) { + writeFileSync(getAuthRefreshIntentPath("anthropic", id), `${JSON.stringify(invalid)}\n`); + expect(readOAuthRefreshIntent("anthropic", id)?.uncertain).toBe(true); + } + let refreshCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(0); }); test("Anthropic never replays an outstanding oauth-source generation across re-entry", async () => { @@ -745,6 +1303,47 @@ describe("oauth refresh hardening", () => { expect(getCredential("anthropic")?.refresh).toBe("rt-new"); }); + /** + * The disk credential is committed before the observed intent is cleaned up. Cleanup is a + * secondary durability concern, so an unlink failure must not throw over the committed + * credential and turn a successful adoption into a caller-visible refresh error. + */ + test("a cleanup failure after adopting a disk credential does not mask the committed token", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, source: "local-cli" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const stored = getAccountCredential("anthropic", id)!; + const pending = writeOAuthRefreshIntent("anthropic", id, credentialGeneration(stored)); + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + pending, + "definitive-rejection", + )).toMatchObject({ cleanupPending: "definitive-rejection" }); + + seedClaudeCredentials("disk", "rt-new", Date.now() + 3600_000); + const mock = mockRefreshFetch([new Response("unexpected", { status: 500 })]); + + // The intent carries an attemptId, so cleanup runs through the exact-match clear. + // Fail it the way a locked or read-only file would. + let cleanupAttempts = 0; + const cleanupSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { + cleanupAttempts += 1; + throw new OAuthRefreshIntentIOError("clear-cleanup-pending", new Error("forced cleanup failure (EROFS)")); + }); + try { + await expect(getValidAccessToken("anthropic")).resolves.toBe("disk"); + } finally { + cleanupSpy.mockRestore(); + } + + // The adoption committed and no network refresh was attempted; only the guard survives. + expect(cleanupAttempts).toBeGreaterThan(0); + expect(mock.count()).toBe(0); + expect(getCredential("anthropic")?.refresh).toBe("rt-new"); + expect(getAccountSet("anthropic")!.accounts[0]!.needsReauth).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); + }); + test("marked Anthropic local-cli account lazily recovers only from a newer disk generation", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, source: "local-cli" }); const id = getAccountSet("anthropic")!.activeAccountId;