From ac0a065b3d86574237b554c9a4a6794893fa47ee Mon Sep 17 00:00:00 2001 From: Sayo Date: Sun, 27 Sep 2026 20:22:16 +0530 Subject: [PATCH 1/9] fix(devin): route a revoked key to needsReauth and follow CLI key rotation A Devin key that Cognition rejects with 401 never reached needsReauth. The OAuth 401 replay only ran on HTTP responses in adapter-dispatch, and Devin is a runTurn adapter, so its forced refresh never ran; with a never-expiring stored expiry and refresh disabled, a dead account looked healthy and every turn 401'd. - run the same forced-refresh replay on the runTurn first-event preflight for isOAuth401ReplayProvider routes, and add devin to that set and to FORCE_REFRESH_PROVIDERS. A terminal refresh marks the account needsReauth; the turn then moves to a surviving stored account, or carries the login instruction. Only a structured 401 triggers it, so Devin 429 and quota-worded permission_denied stay rate limits. - generalize the terminal-refresh alternate picker from Kiro to any generic-failover provider, counting stored logins so the just-flagged account still counts toward consent. - refreshDevinToken re-reads the Devin CLI credential file for a local-cli account and adopts a different key when the host passes the allowlist, no other stored account owns it, and identities agree. The refreshed credential keeps its local-cli source. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit 1b0604cb55e82985f38677dbdc9bf9af153fdeb8) --- scripts/test-layout/layout.json | 1 + src/oauth/devin.ts | 30 ++- src/oauth/index.ts | 5 +- src/oauth/kiro-terminal-failover.ts | 32 ++- src/server/responses/request-transport.ts | 2 + src/server/responses/run-turn-execution.ts | 137 ++++++++---- structure/transports/responses-failover.md | 13 ++ tests/fixtures/test-layout-expected.json | 1 + .../responses-devin-401-replay.test.ts | 195 ++++++++++++++++++ 9 files changed, 361 insertions(+), 55 deletions(-) create mode 100644 tests/responses/responses-devin-401-replay.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index da0966e7b43..1afa60781b3 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -34,6 +34,7 @@ "start-args.test.ts": "cli", "start-ownership-publication.test.ts": "cli", "responses-core-modules.test.ts": "responses", + "responses-devin-401-replay.test.ts": "responses", "responses-grok-devin-preflight.test.ts": "responses", "responses-passthrough-transient-policy.test.ts": "responses", "responses-spend-ledger-wiring.test.ts": "responses", diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 75d9c68ce0f..3d2fdf0b3b4 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -263,14 +263,38 @@ export async function loginDevin( return loginDevinBrowser(ctrl, DEFAULT_REGION); } +/** + * A CLI-imported account follows the CLI: after `devin auth login` rewrites the + * credential file, the copy stored at import time is stale while the file holds + * a live key. The session JWT carries no expiry, so the upstream 401 is the only + * signal, and this re-read runs only on that forced refresh. + * + * Nothing is adopted that could belong to someone else: the host must pass the + * allowlist before the key is paired with it, a key another stored account + * already owns stays with that account, and a key whose identity contradicts the + * stored one is refused. + */ +function rereadDevinCliCredential(stored: OAuthCredentials): OAuthCredentials | undefined { + const outcome = readDevinCliCredentialOutcome(); + if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; + const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); + if (apiBaseUrl === undefined) return undefined; + if (findDevinCredentialOwner("devin", outcome.file.apiKey) !== undefined) return undefined; + const fresh = credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"); + if (stored.accountId && fresh.accountId && stored.accountId !== fresh.accountId) return undefined; + return fresh; +} + export async function refreshDevinToken( _refreshToken: string, _signal?: AbortSignal, - _credential?: OAuthCredentials, + credential?: OAuthCredentials, ): Promise { + const reread = credential?.source === "local-cli" ? rereadDevinCliCredential(credential) : undefined; + if (reread) return reread; // Cognition has no refresh endpoint. Extending the stored expiry here is what // the carried implementation did, and it makes a revoked key look valid - // forever. Throwing lets the request path mark the account needsReauth the - // first time a forced refresh happens. + // forever. Throwing on the upstream-401 forced refresh is what marks the + // account needsReauth. throw new Error("invalid_grant: Devin API keys do not refresh. Run ocx login devin again."); } diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 196283b0953..c294be1d0a8 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -640,6 +640,7 @@ const FORCE_REFRESH_PROVIDERS = new Set([ "kiro", "google-antigravity", "orcarouter-oauth", + "devin", ]); export async function forceRefreshOAuthAccessSnapshot( @@ -848,7 +849,9 @@ function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OA function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials { return { ...fresh, - source: previous.source === "local-cli" ? "oauth" : fresh.source ?? previous.source ?? "oauth", + // A refresh that re-read the CLI's own file is still that CLI's session (Devin). + source: fresh.source === "local-cli" ? "local-cli" + : previous.source === "local-cli" ? "oauth" : fresh.source ?? previous.source ?? "oauth", ...(fresh.projectId === undefined && previous.projectId ? { projectId: previous.projectId } : {}), ...(fresh.apiBaseUrl === undefined && previous.apiBaseUrl ? { apiBaseUrl: previous.apiBaseUrl } : {}), ...(fresh.email === undefined && previous.email ? { email: previous.email } : {}), diff --git a/src/oauth/kiro-terminal-failover.ts b/src/oauth/kiro-terminal-failover.ts index 968ac1e8b4d..e2a2dcafbb6 100644 --- a/src/oauth/kiro-terminal-failover.ts +++ b/src/oauth/kiro-terminal-failover.ts @@ -2,27 +2,41 @@ import type { OAuthAccessSnapshot } from "./index"; import type { OcxConfig } from "../types"; import { getValidAccessSnapshotForAccount } from "./index"; import { credentialGeneration, getAccountCredentialWithStatus, getAccountSet } from "./store"; -import { eligibleFailoverAccounts, isGenericOAuthFailoverEnabled, +import { eligibleFailoverAccounts, isGenericFailoverProvider, GENERIC_OAUTH_MAX_ACCOUNTS_PER_REQUEST } from "./generic-account-failover"; -/** Only an unchanged, allowlist-classified dead credential permits this alternate. */ -export async function tryKiroAlternateAfterTerminalRefresh( - config: OcxConfig, failedAccountId: string, failedGeneration: string, +/** + * Only an unchanged, allowlist-classified dead credential permits this alternate. + * + * Consent is the stored-login count, needsReauth rows included: the refused account was + * marked needsReauth a moment ago, and counting only healthy rows would make a two-account + * setup look like one exactly when the second account is needed. + */ +export async function tryAlternateAfterTerminalRefresh( + config: OcxConfig, providerName: string, failedAccountId: string, failedGeneration: string, ): Promise { - if (!isGenericOAuthFailoverEnabled(config, "kiro")) return null; - const failed = getAccountCredentialWithStatus("kiro", failedAccountId); + const provider = config.providers?.[providerName]; + if (!provider || !isGenericFailoverProvider(providerName, provider)) return null; + const order = getAccountSet(providerName)?.accounts.map(row => row.id) ?? []; + if (order.length < 2) return null; + const failed = getAccountCredentialWithStatus(providerName, failedAccountId); if (!failed?.needsReauth || credentialGeneration(failed.credential) !== failedGeneration) return null; - const order = getAccountSet("kiro")?.accounts.map(row => row.id) ?? []; const after = order.indexOf(failedAccountId); if (after < 0) return null; const ring = [...order.slice(after + 1), ...order.slice(0, after)]; - const eligible = new Set(eligibleFailoverAccounts("kiro")); + const eligible = new Set(eligibleFailoverAccounts(providerName)); let attempted = 0; for (const id of ring) { if (id === failedAccountId || !eligible.has(id)) continue; if (++attempted >= GENERIC_OAUTH_MAX_ACCOUNTS_PER_REQUEST) break; - try { return await getValidAccessSnapshotForAccount("kiro", id, { requireUsableAccount: true }); } + try { return await getValidAccessSnapshotForAccount(providerName, id, { requireUsableAccount: true }); } catch { /* Keep the original login-required result if every alternate is stale. */ } } return null; } + +export function tryKiroAlternateAfterTerminalRefresh( + config: OcxConfig, failedAccountId: string, failedGeneration: string, +): Promise { + return tryAlternateAfterTerminalRefresh(config, "kiro", failedAccountId, failedGeneration); +} diff --git a/src/server/responses/request-transport.ts b/src/server/responses/request-transport.ts index cbba112ba47..8bcd81bbf58 100644 --- a/src/server/responses/request-transport.ts +++ b/src/server/responses/request-transport.ts @@ -118,6 +118,8 @@ export async function prepareResponsesTransport( || route.providerName === "kiro" || route.providerName === "google-antigravity" || route.providerName === "orcarouter-oauth" + // runTurn transport: the replay runs on the first-event preflight in run-turn-execution. + || route.providerName === "devin" ) && route.provider.authMode === "oauth"; let sentOAuthSnapshot: OAuthAccessSnapshot | undefined; let replayOAuthCredentialSnapshot: Pick | undefined; diff --git a/src/server/responses/run-turn-execution.ts b/src/server/responses/run-turn-execution.ts index 736be252156..d042589aa0a 100644 --- a/src/server/responses/run-turn-execution.ts +++ b/src/server/responses/run-turn-execution.ts @@ -27,6 +27,8 @@ import { rotateGenericOAuthAccountOn429, failoverAccountSnapshot, } from "../../oauth/generic-account-failover"; +import { OAuthLoginRequiredError, publicOAuthAuthenticationErrorMessage, type OAuthAccessSnapshot } from "../../oauth/index"; +import { tryAlternateAfterTerminalRefresh } from "../../oauth/kiro-terminal-failover"; import { resolveWireProtocolOverride } from "../adapter-resolve"; import { formatErrorResponse, bridgeToResponsesSSE, buildResponseJSON } from "../../bridge"; import { redactSecretString } from "../../lib/redact"; @@ -93,6 +95,9 @@ export async function executeResponsesRunTurn( | "genericFailovers" | "genericFailoverLimit" | "applyFailoverSnapshot" + | "refreshResolvedOAuthSelection" + | "isOAuth401ReplayProvider" + | "sentOAuthSnapshot" | "resolveSelectionAdapter" | "adapter" | "noteRoutedAttemptSend" @@ -122,6 +127,7 @@ export async function executeResponsesRunTurn( adapterBindings, refreshRunTurnAdapter, applyFailoverSnapshot, + refreshResolvedOAuthSelection, resolveSelectionAdapter, } = transportState; const { @@ -348,6 +354,86 @@ export async function executeResponsesRunTurn( yield* await preflightRunTurnFailover(stream, iterParsed); })(); }; + // Rebind the turn to an admitted account. The failed attempt emitted no client-visible bytes, + // so replay is safe, but a Cursor conversation/checkpoint is credential-scoped: carrying its + // account identity into the next account would not be. The rotated adapter derives its own. + const adoptRunTurnAccount = (admittedSnapshot: Pick): boolean => { + parsed._cursorIdentityScope = undefined; + parsed._cursorConversationId = undefined; + if (parsed._providerContinuation?.cursor) { + const { cursor: _discardedCursor, ...otherProviderState } = parsed._providerContinuation; + parsed._providerContinuation = otherProviderState; + } + const rotatedProvider = resolveWireProtocolOverride( + route.providerName, + route.modelId, + route.provider, + inboundWire, + route.staticPolicy, + ); + const rotatedAdapter = resolveSelectionAdapter(rotatedProvider, config.cacheRetention); + if (!rotatedAdapter.runTurn) return false; + transportState.runTurnAdapter = rotatedAdapter; + bindRouteReasoningReplayScope({ + parsed, + providerName: route.providerName, + provider: rotatedProvider, + adapterName: rotatedAdapter.name, + oauthCredentialSnapshot: { + accountId: admittedSnapshot.accountId, + generation: admittedSnapshot.generation, + }, + codexAuthContext: admissionState.authCtx, + forwardHeaders: requestState.selectedForwardHeaders, + }); + sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, rotatedAdapter.name, logCtx.accountLogLabel); + recordAttemptCredentialSource(logCtx.activeAttempt, route.providerName, route.provider, rotatedAdapter.name); + return true; + }; + // The runTurn half of the upstream-401 replay adapter-dispatch runs for HTTP transports: + // force-refresh the credential that was sent, once per request. A refresh that cannot + // succeed marks the account needsReauth inside the OAuth owner, so the account stops being + // selected; the turn then moves to a surviving stored account, or, with none, the client + // gets the login instruction instead of an opaque upstream 401. + let oauth401ReplayAttempted = false; + const recoverRunTurnAdapterOnPreflight401 = async ( + error: Extract, + ): Promise => { + const sent = transportState.sentOAuthSnapshot; + if (error.status !== 401 || !transportState.isOAuth401ReplayProvider || !sent || oauth401ReplayAttempted) return false; + oauth401ReplayAttempted = true; + const hop = reserveCredentialHop("auth-recovery", `${route.providerName}|${route.modelId}|runturn-oauth-401`); + if (!hop.allowed) return false; + try { + let admitted: OAuthAccessSnapshot | null; + try { + admitted = await applyFailoverSnapshot(await refreshResolvedOAuthSelection(sent)); + } catch (err) { + const alternate = err instanceof OAuthLoginRequiredError + ? await tryAlternateAfterTerminalRefresh(config, route.providerName, sent.accountId, sent.generation) + : null; + admitted = alternate ? await applyFailoverSnapshot(alternate) : null; + if (admitted) transportState.genericFailovers += 1; + else Object.assign(error, { errorType: "authentication_error", message: publicOAuthAuthenticationErrorMessage(err) }); + } + if (!admitted || !adoptRunTurnAccount(admitted)) { + hop.permit?.release(); + return false; + } + sendBudgetState.pendingHopPermit = hop.permit; + return true; + } catch { + hop.permit?.release(); + return false; + } + }; + const recoverRunTurnAdapterOnPreflightError = async ( + error: Extract, + ): Promise => { + if (await recoverRunTurnAdapterOnPreflight401(error)) return "oauth-401"; + if (await rotateRunTurnAdapterOnPreflight429(error)) return "oauth-account-429"; + return undefined; + }; const rotateRunTurnAdapterOnPreflight429 = async ( error: Extract, ): Promise => { @@ -400,42 +486,10 @@ export async function executeResponsesRunTurn( hop.permit?.release(); return false; } - // A Cursor conversation/checkpoint is credential-scoped. The failed attempt emitted no - // client-visible bytes, so replay is safe, but carrying its account identity into the next - // account would not be. Let the rotated adapter derive a fresh identity and conversation. - parsed._cursorIdentityScope = undefined; - parsed._cursorConversationId = undefined; - if (parsed._providerContinuation?.cursor) { - const { cursor: _discardedCursor, ...otherProviderState } = parsed._providerContinuation; - parsed._providerContinuation = otherProviderState; - } - const rotatedProvider = resolveWireProtocolOverride( - route.providerName, - route.modelId, - route.provider, - inboundWire, - route.staticPolicy, - ); - const rotatedAdapter = resolveSelectionAdapter(rotatedProvider, config.cacheRetention); - if (!rotatedAdapter.runTurn) { + if (!adoptRunTurnAccount(admittedSnapshot)) { hop.permit?.release(); return false; } - transportState.runTurnAdapter = rotatedAdapter; - bindRouteReasoningReplayScope({ - parsed, - providerName: route.providerName, - provider: rotatedProvider, - adapterName: rotatedAdapter.name, - oauthCredentialSnapshot: { - accountId: admittedSnapshot.accountId, - generation: admittedSnapshot.generation, - }, - codexAuthContext: admissionState.authCtx, - forwardHeaders: requestState.selectedForwardHeaders, - }); - sealRequestAttemptIdentity(logCtx.activeAttempt, logCtx.provider, rotatedAdapter.name, logCtx.accountLogLabel); - recordAttemptCredentialSource(logCtx.activeAttempt, route.providerName, route.provider, rotatedAdapter.name); // The caller replays the turn on this rotation, and a runTurn adapter dispatches through // its own reservation ladder -- Cursor reserves once per physical send. Confirming here // would leave that ladder to charge the same replay a second time (#4709), so hand the @@ -463,13 +517,14 @@ export async function executeResponsesRunTurn( yield event; continue; } - if (!firstMeaningfulSeen && !replayUnsafe && event.type === "error" - && await rotateRunTurnAdapterOnPreflight429(event)) { + const recovery = !firstMeaningfulSeen && !replayUnsafe && event.type === "error" + ? await recoverRunTurnAdapterOnPreflightError(event) : undefined; + if (recovery) { const retryQueue = createAdapterEventQueue({ onBacklogExceeded: () => runTurnAbort.abort(), }); const pendingPermit = sendBudgetState.pendingHopPermit; - const retryAttempt = runTurnAttempt(retryQueue, "oauth-account-429", false, replayParsed); + const retryAttempt = runTurnAttempt(retryQueue, recovery, false, replayParsed); if (pendingPermit) { const releaseIfUnclaimed = () => { if (sendBudgetState.pendingHopPermit !== pendingPermit) return; @@ -522,15 +577,13 @@ export async function executeResponsesRunTurn( return streamAfterPreflight(preflight.stream, replayParsed, preflight.replayUnsafe); } if (preflight.ready) return streamAfterPreflight(preflight.stream, replayParsed, preflight.replayUnsafe); - if (preflight.replayUnsafe - || !preflight.error - || !(await rotateRunTurnAdapterOnPreflight429(preflight.error))) { - return preflight.stream; - } + const recovery = preflight.replayUnsafe || !preflight.error + ? undefined : await recoverRunTurnAdapterOnPreflightError(preflight.error); + if (!recovery) return preflight.stream; const retryQueue = createAdapterEventQueue({ onBacklogExceeded: () => runTurnAbort.abort(), }); - latestRetryAttempt = runTurnAttempt(retryQueue, "oauth-account-429", false, replayParsed); + latestRetryAttempt = runTurnAttempt(retryQueue, recovery, false, replayParsed); void latestRetryAttempt; source = retryQueue.stream(); } diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 76f0cfb246c..539f97881e3 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -246,6 +246,19 @@ surface the pre-output 429 immediately, because the outer response cannot forwar heartbeats while it is choosing a target. An earlier replay-unsafe heartbeat or meaningful output keeps the failure on the current target. +## runTurn pre-output 401 replay + +`src/server/responses/run-turn-execution.ts` runs the `adapter-dispatch.ts` OAuth 401 replay on the +runTurn first-event preflight for `isOAuth401ReplayProvider` routes (Devin is the runTurn member): a +structured 401 before output or a replay-unsafe heartbeat force-refreshes the sent credential once +per request under an `auth-recovery` hop and replays the turn. A terminal refresh has already marked +the account needsReauth; the turn moves to a surviving stored account via +`tryAlternateAfterTerminalRefresh`, else the 401 carries the login instruction. Devin quota +`permission_denied` maps to 429 and plain `permission_denied` to 403, so neither refreshes. +`refreshDevinToken` throws `invalid_grant` except for a `local-cli` account, which adopts a different +key from the Devin CLI file only when its host passes `validateDevinApiBaseUrl`, no other stored +account owns the key, and identities agree. Test: `tests/responses/responses-devin-401-replay.test.ts`. + ## Optional client transport hints `dropCodexSafetyBuffering` defaults to false. Canonical OpenAI forward Responses can remove only diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index a02bf39bd4a..4269c2596ed 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -46,6 +46,7 @@ "start-args.test.ts": "cli", "start-ownership-publication.test.ts": "cli", "responses-core-modules.test.ts": "responses", + "responses-devin-401-replay.test.ts": "responses", "responses-grok-devin-preflight.test.ts": "responses", "responses-passthrough-transient-policy.test.ts": "responses", "responses-spend-ledger-wiring.test.ts": "responses", diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts new file mode 100644 index 00000000000..6bd878798dc --- /dev/null +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -0,0 +1,195 @@ +import { afterAll, afterEach, beforeEach, expect, mock, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import type { ProviderAdapter } from "../../src/adapters/base"; +import type { AdapterEvent, OcxConfig, OcxProviderConfig } from "../../src/types"; +import { getAccountSet, saveCredential } from "../../src/oauth/store"; +import { clearGenericFailoverHealth } from "../../src/oauth/generic-account-failover"; +import { DEVIN_CLI_CREDENTIALS_ENV } from "../../src/oauth/devin/cli-import"; +import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +import { createTempHome } from "../helpers/temp-home"; + +const DEAD = "devin-session-token$synthetic-dead"; +const LIVE = "devin-session-token$synthetic-live"; +const ROTATED = "devin-session-token$synthetic-rotated"; + +const resolver = await import("../../src/server/adapter-resolve"); +const originalResolve = resolver.resolveAdapter; +const originalResolverModule = { ...resolver }; +let sentKeys: string[] = []; +let rateLimited = false; +mock.module("../../src/server/adapter-resolve", () => ({ ...resolver, + resolveAdapter(provider: OcxProviderConfig, cache?: "none" | "short" | "long") { + if (provider.adapter !== "devin") return originalResolve(provider, cache); + return { + name: "devin", + buildRequest: () => ({ url: provider.baseUrl, method: "POST", headers: {}, body: "" }), + async *parseStream() { yield { type: "done" } as AdapterEvent; }, + async runTurn(_parsed, _incoming, emit) { + const key = String(provider.apiKey); + sentKeys.push(key); + if (rateLimited) { + emit({ type: "error", status: 429, errorType: "rate_limit_error", code: "resource_exhausted", + retryable: true, message: "Cognition chat failed (resource_exhausted)" }); + return; + } + if (key === DEAD) { + emit({ type: "error", status: 401, errorType: "authentication_error", code: "unauthenticated", + retryable: false, message: "Devin cloud error unauthenticated: invalid api key" }); + return; + } + emit({ type: "text_delta", text: `served by ${key === ROTATED ? "rotated" : "live"}` }); + emit({ type: "done" }); + }, + } satisfies ProviderAdapter; + }, +})); +const { handleResponses } = await import("../../src/server/responses"); + +let home: ReturnType; +let release: (() => void) | undefined; +let previousCliPath: string | undefined; + +beforeEach(() => { + home = createTempHome("ocx-devin-401-replay-"); + release = acquireOwnedSpendHome(); + clearGenericFailoverHealth(); + sentKeys = []; + rateLimited = false; + previousCliPath = process.env[DEVIN_CLI_CREDENTIALS_ENV]; + // Never let a test read the developer's real CLI credential. + process.env[DEVIN_CLI_CREDENTIALS_ENV] = home.path("devin-credentials.toml"); +}); +afterAll(() => { + mock.module("../../src/server/adapter-resolve", () => originalResolverModule); +}); +afterEach(() => { + try { + release?.(); + } finally { + if (previousCliPath === undefined) delete process.env[DEVIN_CLI_CREDENTIALS_ENV]; + else process.env[DEVIN_CLI_CREDENTIALS_ENV] = previousCliPath; + clearGenericFailoverHealth(); + home.remove(); + } +}); + +function writeCliFile(apiKey: string, apiServerUrl = "https://server.codeium.com"): void { + writeFileSync(home.path("devin-credentials.toml"), + `windsurf_api_key = "${apiKey}"\napi_server_url = "${apiServerUrl}"\n`); +} + +async function saveDevin(access: string, accountId: string, source: "oauth" | "local-cli" = "oauth") { + await saveCredential("devin", { + access, refresh: access, expires: Number.MAX_SAFE_INTEGER, accountId, source, + apiBaseUrl: "https://server.codeium.com", + }); +} + +function run(stream = false) { + const config = { + port: 0, defaultProvider: "devin", + providers: { devin: { adapter: "devin", authMode: "oauth", baseUrl: "https://server.codeium.com", models: ["swe-1-6"] } }, + } as OcxConfig; + return handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "devin/swe-1-6", input: "answer", stream }), + }), config, { model: "", provider: "", surface: "codex" }); +} + +function account(id: string) { + return getAccountSet("devin")?.accounts.find(row => row.credential.accountId === id); +} + +test.each([false, true])("a revoked key is marked needsReauth and the turn fails over (stream=%s)", async stream => { + await saveDevin(LIVE, "spare"); + await saveDevin(DEAD, "revoked"); + expect(getAccountSet("devin")?.activeAccountId).toBe(account("revoked")?.id); + + const response = await run(stream); + const body = await response.text(); + + expect(response.status).toBe(200); + expect(body).toContain("served by live"); + expect(sentKeys).toEqual([DEAD, LIVE]); + expect(account("revoked")?.needsReauth).toBe(true); + expect(getAccountSet("devin")?.activeAccountId).toBe(account("spare")?.id); + + // The dead account is not reselected by the next request. + sentKeys = []; + expect((await run()).status).toBe(200); + expect(sentKeys).toEqual([LIVE]); +}); + +test("a lone revoked account surfaces the login instruction", async () => { + await saveDevin(DEAD, "revoked"); + + // A buffered runTurn failure is a `status: "failed"` Response object, not an HTTP error. + const body = await (await run()).json() as { status: string; error: { type: string; message: string } }; + + expect(body.status).toBe("failed"); + expect(body.error.type).toBe("authentication_error"); + expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); + expect(sentKeys).toEqual([DEAD]); + expect(account("revoked")?.needsReauth).toBe(true); + + // Later turns fail fast on the flagged account instead of re-sending a dead key. + sentKeys = []; + const next = await run(); + expect(next.status).toBe(401); + expect(await next.text()).toContain("ocx login devin"); + expect(sentKeys).toEqual([]); +}); + +test("a CLI-imported account adopts the key a later `devin auth login` wrote", async () => { + await saveDevin(DEAD, "cli", "local-cli"); + writeCliFile(ROTATED); + + const response = await run(); + + expect(response.status).toBe(200); + expect(await response.text()).toContain("served by rotated"); + expect(sentKeys).toEqual([DEAD, ROTATED]); + const row = account("cli"); + expect(row?.needsReauth).not.toBe(true); + expect(row?.credential.access).toBe(ROTATED); + expect(row?.credential.source).toBe("local-cli"); + expect(row?.credential.expires).toBe(Number.MAX_SAFE_INTEGER); +}); + +test.each([ + ["unchanged", () => writeCliFile(DEAD)], + ["missing", () => {}], + ["off-allowlist host", () => writeCliFile(ROTATED, "https://attacker.example")], +])("a CLI-imported account with a %s credential file needs reauth", async (_label, arrange) => { + await saveDevin(DEAD, "cli", "local-cli"); + arrange(); + + const body = await (await run()).json() as { error: { message: string } }; + + expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); + expect(sentKeys).toEqual([DEAD]); + expect(account("cli")?.needsReauth).toBe(true); + expect(account("cli")?.credential.access).toBe(DEAD); +}); + +test("a CLI key another stored account already owns is not adopted", async () => { + await saveDevin(ROTATED, "other"); + await saveDevin(DEAD, "cli", "local-cli"); + writeCliFile(ROTATED); + + await (await run()).text(); + + expect(account("cli")?.needsReauth).toBe(true); + expect(account("cli")?.credential.access).toBe(DEAD); +}); + +test("a 429 is not treated as an authentication failure", async () => { + await saveDevin(LIVE, "limited"); + rateLimited = true; + + const body = await (await run()).json() as { error: { type: string } }; + + expect(body.error.type).toBe("rate_limit_error"); + expect(sentKeys).toEqual([LIVE]); + expect(account("limited")?.needsReauth).not.toBe(true); +}); From 8e821efa301380a667787fb0db01c2f43df0ba12 Mon Sep 17 00:00:00 2001 From: Sayo Date: Sun, 27 Sep 2026 20:32:22 +0530 Subject: [PATCH 2/9] fix(devin): fail closed on CLI key identity and harden 401 failover Review follow-up to the Devin 401 replay. - CLI re-read adopts a rotated key only when its identity is established: the session token carries only a session_id, so the key must mint a user_jwt whose auth_uid/email do not contradict the slot's, and no other stored account may own the key or that identity. The adopted credential records the minted identity so later adoptions are strict. - runTurn 401 recovery tries the terminal-refresh alternate on any refresh failure, not only OAuthLoginRequiredError; the helper still requires the sent generation to be flagged needsReauth. Same one-line change on the Kiro HTTP path. The alternate now respects genericFailoverLimit. - merged(): note the local-cli preservation is shared with Meta Muse and why it is safe. - structure: a single upstream 401 on an oauth-source Devin account marks needsReauth by design; document the identity rule. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit a42f349c767e956d4df7e6ad47caf6b0f601f386) --- src/oauth/devin.ts | 49 ++++++-- src/oauth/index.ts | 4 +- src/server/responses/run-turn-execution.ts | 7 +- structure/transports/responses-failover.md | 9 +- .../responses-devin-401-replay.test.ts | 117 ++++++++++++++++-- 5 files changed, 162 insertions(+), 24 deletions(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 3d2fdf0b3b4..fc9100535cc 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -19,6 +19,7 @@ import { DEFAULT_REGION, type WindsurfRegion } from "./devin/types"; import { registerUser } from "./devin/register-user"; import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseUrl } from "./devin/api-base"; import { readDevinCliCredentialOutcome } from "./devin/cli-import"; +import { mintUserJwt } from "../adapters/devin/cloud-direct/auth"; import { getCredential, listAccounts } from "./store"; import { DEPRECATED_OAUTH_PROVIDER_ALIASES } from "./index"; @@ -269,28 +270,56 @@ export async function loginDevin( * a live key. The session JWT carries no expiry, so the upstream 401 is the only * signal, and this re-read runs only on that forced refresh. * - * Nothing is adopted that could belong to someone else: the host must pass the - * allowlist before the key is paired with it, a key another stored account - * already owns stays with that account, and a key whose identity contradicts the - * stored one is refused. + * Adoption fails closed on identity. The session token carries only a + * session_id, so the file key's account is established by minting a user_jwt + * with it (GetUserJwt answers with auth_uid and email); a key that cannot mint + * one is dead or unidentifiable and is refused. The minted identity must then + * not contradict the slot's recorded accountId or email, and no other stored + * account may own the key or that identity. + * + * A slot imported from the CLI records no identity (its token has none to + * give), so its first adoption rests on the last two rules alone: it is by + * definition "whatever the CLI is signed into", and the adopted credential + * records the minted identity, which makes every later adoption strict. */ -function rereadDevinCliCredential(stored: OAuthCredentials): OAuthCredentials | undefined { +async function rereadDevinCliCredential(stored: OAuthCredentials, signal?: AbortSignal): Promise { const outcome = readDevinCliCredentialOutcome(); if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); if (apiBaseUrl === undefined) return undefined; if (findDevinCredentialOwner("devin", outcome.file.apiKey) !== undefined) return undefined; - const fresh = credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"); - if (stored.accountId && fresh.accountId && stored.accountId !== fresh.accountId) return undefined; - return fresh; + let minted: Record | undefined; + try { + minted = decodeJwtPayload((await mintUserJwt(outcome.file.apiKey, apiBaseUrl, signal)).jwt); + } catch { + return undefined; + } + const accountId = typeof minted?.auth_uid === "string" && minted.auth_uid ? minted.auth_uid : undefined; + const email = typeof minted?.email === "string" && minted.email ? minted.email : undefined; + if (accountId === undefined) return undefined; + if (stored.accountId && stored.accountId !== accountId) return undefined; + if (stored.email?.includes("@") && stored.email.toLowerCase() !== email?.toLowerCase()) return undefined; + if (devinIdentityOwnedElsewhere(stored, accountId, email)) return undefined; + return { ...credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"), accountId, ...(email ? { email } : {}) }; +} + +function devinIdentityOwnedElsewhere(stored: OAuthCredentials, accountId: string, email: string | undefined): boolean { + for (const slot of ["devin", ...devinAliasCredentialSlots("devin")]) { + for (const { credential } of listAccounts(slot)) { + if (credential.access === stored.access) continue; + if (credential.accountId === accountId) return true; + if (email && credential.email?.toLowerCase() === email.toLowerCase()) return true; + } + } + return false; } export async function refreshDevinToken( _refreshToken: string, - _signal?: AbortSignal, + signal?: AbortSignal, credential?: OAuthCredentials, ): Promise { - const reread = credential?.source === "local-cli" ? rereadDevinCliCredential(credential) : undefined; + const reread = credential?.source === "local-cli" ? await rereadDevinCliCredential(credential, signal) : undefined; if (reread) return reread; // Cognition has no refresh endpoint. Extending the stored expiry here is what // the carried implementation did, and it makes a revoked key look valid diff --git a/src/oauth/index.ts b/src/oauth/index.ts index c294be1d0a8..6e61b63399f 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -849,7 +849,9 @@ function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OA function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials { return { ...fresh, - // A refresh that re-read the CLI's own file is still that CLI's session (Devin). + // Shared: a refresh function returns "local-cli" only when the credential it hands back + // still is the local CLI's (Devin re-reading the CLI file, Meta Muse echoing its durable + // CLI key). Relabelling that "oauth" would stop the next forced refresh from re-reading it. source: fresh.source === "local-cli" ? "local-cli" : previous.source === "local-cli" ? "oauth" : fresh.source ?? previous.source ?? "oauth", ...(fresh.projectId === undefined && previous.projectId ? { projectId: previous.projectId } : {}), diff --git a/src/server/responses/run-turn-execution.ts b/src/server/responses/run-turn-execution.ts index d042589aa0a..cdb52c72fc6 100644 --- a/src/server/responses/run-turn-execution.ts +++ b/src/server/responses/run-turn-execution.ts @@ -27,7 +27,7 @@ import { rotateGenericOAuthAccountOn429, failoverAccountSnapshot, } from "../../oauth/generic-account-failover"; -import { OAuthLoginRequiredError, publicOAuthAuthenticationErrorMessage, type OAuthAccessSnapshot } from "../../oauth/index"; +import { publicOAuthAuthenticationErrorMessage, type OAuthAccessSnapshot } from "../../oauth/index"; import { tryAlternateAfterTerminalRefresh } from "../../oauth/kiro-terminal-failover"; import { resolveWireProtocolOverride } from "../adapter-resolve"; import { formatErrorResponse, bridgeToResponsesSSE, buildResponseJSON } from "../../bridge"; @@ -409,7 +409,10 @@ export async function executeResponsesRunTurn( try { admitted = await applyFailoverSnapshot(await refreshResolvedOAuthSelection(sent)); } catch (err) { - const alternate = err instanceof OAuthLoginRequiredError + // Not only OAuthLoginRequiredError: a concurrent request that already flagged this + // account and moved the selection makes this refresh fail as "selection changed". The + // helper still requires the sent generation to be flagged needsReauth, so it is safe. + const alternate = transportState.genericFailovers < transportState.genericFailoverLimit ? await tryAlternateAfterTerminalRefresh(config, route.providerName, sent.accountId, sent.generation) : null; admitted = alternate ? await applyFailoverSnapshot(alternate) : null; diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 539f97881e3..7b0bf96836c 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -255,9 +255,12 @@ per request under an `auth-recovery` hop and replays the turn. A terminal refres the account needsReauth; the turn moves to a surviving stored account via `tryAlternateAfterTerminalRefresh`, else the 401 carries the login instruction. Devin quota `permission_denied` maps to 429 and plain `permission_denied` to 403, so neither refreshes. -`refreshDevinToken` throws `invalid_grant` except for a `local-cli` account, which adopts a different -key from the Devin CLI file only when its host passes `validateDevinApiBaseUrl`, no other stored -account owns the key, and identities agree. Test: `tests/responses/responses-devin-401-replay.test.ts`. +By design a single upstream 401 on an `oauth`-source Devin account marks it needsReauth: Cognition has +no refresh endpoint and there is no confirming probe. A `local-cli` account instead re-reads the CLI +file and adopts a different key only if its host passes `validateDevinApiBaseUrl`, the key mints a +user_jwt (its auth_uid/email are the identity; the session token has none), that identity does not +contradict the slot's, and no other slot owns the key or identity; the adopted identity is recorded. +Test: `tests/responses/responses-devin-401-replay.test.ts`. ## Optional client transport hints diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index 6bd878798dc..d7dc6b3e66c 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -11,12 +11,34 @@ import { createTempHome } from "../helpers/temp-home"; const DEAD = "devin-session-token$synthetic-dead"; const LIVE = "devin-session-token$synthetic-live"; const ROTATED = "devin-session-token$synthetic-rotated"; +const originalFetch = globalThis.fetch; + +// GetUserJwt stand-in: the identity a key mints, keyed by the key inside the protobuf body. +let mintedIdentity: Record = {}; +let mintCalls = 0; +function fakeUserJwt(payload: object): string { + const part = (value: object) => Buffer.from(JSON.stringify(value)).toString("base64url"); + return `${part({ alg: "HS256", typ: "JWT" })}.${part({ ...payload, exp: 9_999_999_999 })}.c2lnbmF0dXJl`; +} +const mintFetch = (async (input: Parameters[0], init?: RequestInit) => { + const url = String(input instanceof Request ? input.url : input); + if (!url.endsWith("/exa.auth_pb.AuthService/GetUserJwt")) return originalFetch(input, init); + mintCalls++; + const body = Buffer.from(init?.body as Uint8Array).toString("latin1"); + const key = Object.keys(mintedIdentity).find(candidate => body.includes(candidate)); + const identity = key ? mintedIdentity[key] : undefined; + if (!identity) return new Response("", { status: 401 }); + const jwt = Buffer.from(fakeUserJwt(identity)); + return new Response(Buffer.concat([Buffer.from([0x0a, jwt.length & 0x7f | 0x80, jwt.length >> 7]), jwt]), + { status: 200, headers: { "content-type": "application/proto" } }); +}) as typeof fetch; const resolver = await import("../../src/server/adapter-resolve"); const originalResolve = resolver.resolveAdapter; const originalResolverModule = { ...resolver }; let sentKeys: string[] = []; let rateLimited = false; +let holdDeadSend: (() => Promise) | undefined; mock.module("../../src/server/adapter-resolve", () => ({ ...resolver, resolveAdapter(provider: OcxProviderConfig, cache?: "none" | "short" | "long") { if (provider.adapter !== "devin") return originalResolve(provider, cache); @@ -33,6 +55,7 @@ mock.module("../../src/server/adapter-resolve", () => ({ ...resolver, return; } if (key === DEAD) { + await holdDeadSend?.(); emit({ type: "error", status: 401, errorType: "authentication_error", code: "unauthenticated", retryable: false, message: "Devin cloud error unauthenticated: invalid api key" }); return; @@ -55,6 +78,10 @@ beforeEach(() => { clearGenericFailoverHealth(); sentKeys = []; rateLimited = false; + holdDeadSend = undefined; + mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", email: "rotated@example.com" } }; + mintCalls = 0; + globalThis.fetch = mintFetch; previousCliPath = process.env[DEVIN_CLI_CREDENTIALS_ENV]; // Never let a test read the developer's real CLI credential. process.env[DEVIN_CLI_CREDENTIALS_ENV] = home.path("devin-credentials.toml"); @@ -68,6 +95,7 @@ afterEach(() => { } finally { if (previousCliPath === undefined) delete process.env[DEVIN_CLI_CREDENTIALS_ENV]; else process.env[DEVIN_CLI_CREDENTIALS_ENV] = previousCliPath; + globalThis.fetch = originalFetch; clearGenericFailoverHealth(); home.remove(); } @@ -85,6 +113,18 @@ async function saveDevin(access: string, accountId: string, source: "oauth" | "l }); } +// A CLI import records no identity: the session token it copies carries only a session_id. +async function saveCliImport(access: string, extra: { accountId?: string; email?: string } = {}) { + await saveCredential("devin", { + access, refresh: access, expires: Number.MAX_SAFE_INTEGER, source: "local-cli", + apiBaseUrl: "https://server.codeium.com", ...extra, + }, { preserveIdentityless: true }); +} + +function cliAccount() { + return getAccountSet("devin")?.accounts.find(row => row.credential.source === "local-cli"); +} + function run(stream = false) { const config = { port: 0, defaultProvider: "devin", @@ -141,7 +181,7 @@ test("a lone revoked account surfaces the login instruction", async () => { }); test("a CLI-imported account adopts the key a later `devin auth login` wrote", async () => { - await saveDevin(DEAD, "cli", "local-cli"); + await saveCliImport(DEAD); writeCliFile(ROTATED); const response = await run(); @@ -149,9 +189,12 @@ test("a CLI-imported account adopts the key a later `devin auth login` wrote", a expect(response.status).toBe(200); expect(await response.text()).toContain("served by rotated"); expect(sentKeys).toEqual([DEAD, ROTATED]); - const row = account("cli"); + const row = cliAccount(); expect(row?.needsReauth).not.toBe(true); expect(row?.credential.access).toBe(ROTATED); + // The minted identity is recorded, so the next adoption for this slot is strict. + expect(row?.credential.accountId).toBe("uid-rotated"); + expect(row?.credential.email).toBe("rotated@example.com"); expect(row?.credential.source).toBe("local-cli"); expect(row?.credential.expires).toBe(Number.MAX_SAFE_INTEGER); }); @@ -161,26 +204,84 @@ test.each([ ["missing", () => {}], ["off-allowlist host", () => writeCliFile(ROTATED, "https://attacker.example")], ])("a CLI-imported account with a %s credential file needs reauth", async (_label, arrange) => { - await saveDevin(DEAD, "cli", "local-cli"); + await saveCliImport(DEAD); arrange(); const body = await (await run()).json() as { error: { message: string } }; expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); expect(sentKeys).toEqual([DEAD]); - expect(account("cli")?.needsReauth).toBe(true); - expect(account("cli")?.credential.access).toBe(DEAD); + expect(cliAccount()?.needsReauth).toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); }); test("a CLI key another stored account already owns is not adopted", async () => { await saveDevin(ROTATED, "other"); - await saveDevin(DEAD, "cli", "local-cli"); + await saveCliImport(DEAD); + writeCliFile(ROTATED); + + await (await run()).text(); + + expect(cliAccount()?.needsReauth).toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); +}); + +test.each([ + ["a key that cannot mint a user_jwt", async () => { mintedIdentity = {}; await saveCliImport(DEAD); }], + ["a slot whose recorded accountId differs", async () => { await saveCliImport(DEAD, { accountId: "uid-before" }); }], + ["a slot whose recorded email differs", async () => { await saveCliImport(DEAD, { email: "before@example.com" }); }], + ["an identity another stored account owns", async () => { + await saveDevin(LIVE, "uid-rotated"); + await saveCliImport(DEAD); + }], +])("the CLI key is not adopted for %s", async (_label, arrange) => { + await arrange(); writeCliFile(ROTATED); await (await run()).text(); - expect(account("cli")?.needsReauth).toBe(true); - expect(account("cli")?.credential.access).toBe(DEAD); + expect(sentKeys).not.toContain(ROTATED); + expect(cliAccount()?.needsReauth).toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); +}); + +test("a slot whose recorded identity matches adopts the rotated key", async () => { + await saveCliImport(DEAD, { accountId: "uid-rotated", email: "Rotated@example.com" }); + writeCliFile(ROTATED); + + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.access).toBe(ROTATED); + expect(mintCalls).toBe(1); +}); + +test("a turn whose 401 lands after another turn already failed the account over still fails over", async () => { + await saveDevin(LIVE, "spare"); + await saveDevin(DEAD, "revoked"); + // Both turns send on the revoked account; the second 401 only arrives once the first turn has + // flagged it and moved the selection, so the second refresh sees a changed selection. + const bothSent = Promise.withResolvers(); + const firstDone = Promise.withResolvers(); + let deadSends = 0; + holdDeadSend = async () => { + const order = ++deadSends; + if (order === 2) bothSent.resolve(); + await bothSent.promise; + if (order === 2) await firstDone.promise; + }; + + const first = run().then(async response => { + const text = await response.text(); + firstDone.resolve(); + return text; + }); + const second = run().then(response => response.text()); + const [firstBody, secondBody] = await Promise.all([first, second]); + + expect(deadSends).toBe(2); + expect(firstBody).toContain("served by live"); + expect(secondBody).toContain("served by live"); + expect(account("revoked")?.needsReauth).toBe(true); + expect(getAccountSet("devin")?.activeAccountId).toBe(account("spare")?.id); }); test("a 429 is not treated as an authentication failure", async () => { From 120b5e34f1feaa29611caf369258d1db50cf6da2 Mon Sep 17 00:00:00 2001 From: Sayo Date: Sun, 27 Sep 2026 20:38:22 +0530 Subject: [PATCH 3/9] fix(devin): only a definitive refusal from the identity probe flags the account Re-review follow-up. - A timeout, network failure, 5xx or 429 during the CLI key's identity mint now throws a non-terminal error, so the account is not marked needsReauth (which would strand it) and the next 401 retries. Only a 401/403 from GetUserJwt or a minted token without auth_uid refuses. - The probe mint has a 5s timeout so it cannot hold the per-account refresh lock for the mint's full 30s. - Identity compares like with like: a stored id may be the key's sub or auth_uid, so both minted claims are accepted; emails are trimmed and case-folded. The ownership scan skips the refreshed row by id, which the generic refresh lock now passes to the provider refresh. - Test that an off-allowlist host never reaches the mint. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit 40379257c61f83f765da63cd327b7d81d6ab5e8b) --- src/oauth/devin.ts | 82 ++++++++++++++----- src/oauth/index.ts | 4 +- .../responses-devin-401-replay.test.ts | 50 ++++++++++- 3 files changed, 112 insertions(+), 24 deletions(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index fc9100535cc..42607464cb0 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -19,7 +19,7 @@ import { DEFAULT_REGION, type WindsurfRegion } from "./devin/types"; import { registerUser } from "./devin/register-user"; import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseUrl } from "./devin/api-base"; import { readDevinCliCredentialOutcome } from "./devin/cli-import"; -import { mintUserJwt } from "../adapters/devin/cloud-direct/auth"; +import { CloudAuthError, mintUserJwt } from "../adapters/devin/cloud-direct/auth"; import { getCredential, listAccounts } from "./store"; import { DEPRECATED_OAUTH_PROVIDER_ALIASES } from "./index"; @@ -272,8 +272,10 @@ export async function loginDevin( * * Adoption fails closed on identity. The session token carries only a * session_id, so the file key's account is established by minting a user_jwt - * with it (GetUserJwt answers with auth_uid and email); a key that cannot mint - * one is dead or unidentifiable and is refused. The minted identity must then + * with it (GetUserJwt answers with auth_uid and email). A key Cognition refuses + * (401/403) or whose token lacks auth_uid is refused; a mint that fails for any + * other reason throws a non-terminal error, so the account is not flagged and + * the next 401 retries. The minted identity must then * not contradict the slot's recorded accountId or email, and no other stored * account may own the key or that identity. * @@ -282,7 +284,25 @@ export async function loginDevin( * definition "whatever the CLI is signed into", and the adopted credential * records the minted identity, which makes every later adoption strict. */ -async function rereadDevinCliCredential(stored: OAuthCredentials, signal?: AbortSignal): Promise { +/** An identity probe must not hold the per-account refresh lock for the mint's full 30s. */ +const DEVIN_IDENTITY_MINT_TIMEOUT_MS = 5_000; + +/** Neither a live key nor a dead one: the refresh must fail without flagging the account. */ +class DevinIdentityProbeUnavailableError extends Error { + constructor() { + super("Could not confirm the Devin CLI session identity right now; retry shortly."); + this.name = "DevinIdentityProbeUnavailableError"; + } +} + +const normalizedEmail = (value: string | undefined): string | undefined => + value?.trim().toLowerCase() || undefined; + +async function rereadDevinCliCredential( + stored: OAuthCredentials, + signal: AbortSignal | undefined, + currentAccountId: string | undefined, +): Promise { const outcome = readDevinCliCredentialOutcome(); if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); @@ -290,25 +310,45 @@ async function rereadDevinCliCredential(stored: OAuthCredentials, signal?: Abort if (findDevinCredentialOwner("devin", outcome.file.apiKey) !== undefined) return undefined; let minted: Record | undefined; try { - minted = decodeJwtPayload((await mintUserJwt(outcome.file.apiKey, apiBaseUrl, signal)).jwt); - } catch { - return undefined; + const timeout = AbortSignal.timeout(DEVIN_IDENTITY_MINT_TIMEOUT_MS); + const probeSignal = signal ? AbortSignal.any([signal, timeout]) : timeout; + minted = decodeJwtPayload((await mintUserJwt(outcome.file.apiKey, apiBaseUrl, probeSignal)).jwt); + } catch (error) { + // Only Cognition refusing the key is evidence the file holds no live session. A timeout, + // DNS failure, 5xx or 429 says nothing about the key; flagging the account on one would + // strand it, because a needsReauth slot is never refreshed again. + if (error instanceof CloudAuthError && (error.status === 401 || error.status === 403)) return undefined; + throw new DevinIdentityProbeUnavailableError(); } - const accountId = typeof minted?.auth_uid === "string" && minted.auth_uid ? minted.auth_uid : undefined; - const email = typeof minted?.email === "string" && minted.email ? minted.email : undefined; - if (accountId === undefined) return undefined; - if (stored.accountId && stored.accountId !== accountId) return undefined; - if (stored.email?.includes("@") && stored.email.toLowerCase() !== email?.toLowerCase()) return undefined; - if (devinIdentityOwnedElsewhere(stored, accountId, email)) return undefined; - return { ...credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"), accountId, ...(email ? { email } : {}) }; + const authUid = typeof minted?.auth_uid === "string" && minted.auth_uid ? minted.auth_uid : undefined; + if (authUid === undefined) return undefined; + // identityFromApiKey records `sub ?? auth_uid`, so a stored id may be either claim. + const mintedIds = new Set([authUid, ...(typeof minted?.sub === "string" && minted.sub ? [minted.sub] : [])]); + const rawEmail = typeof minted?.email === "string" ? minted.email.trim() : ""; + const email = rawEmail || undefined; + if (stored.accountId && !mintedIds.has(stored.accountId)) return undefined; + const storedEmail = normalizedEmail(stored.email); + if (storedEmail?.includes("@") && storedEmail !== normalizedEmail(email)) return undefined; + if (devinIdentityOwnedElsewhere(stored, currentAccountId, mintedIds, normalizedEmail(email))) return undefined; + return { ...credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"), accountId: authUid, ...(email ? { email } : {}) }; } -function devinIdentityOwnedElsewhere(stored: OAuthCredentials, accountId: string, email: string | undefined): boolean { +/** + * Every stored Devin row except the one being refreshed, across the alias-linked slots + * (the active row `getCredential` reads is one of these). Rows are skipped by id; only a + * caller that cannot name the row falls back to matching its key. + */ +function devinIdentityOwnedElsewhere( + stored: OAuthCredentials, + currentAccountId: string | undefined, + mintedIds: ReadonlySet, + email: string | undefined, +): boolean { for (const slot of ["devin", ...devinAliasCredentialSlots("devin")]) { - for (const { credential } of listAccounts(slot)) { - if (credential.access === stored.access) continue; - if (credential.accountId === accountId) return true; - if (email && credential.email?.toLowerCase() === email.toLowerCase()) return true; + for (const { id, credential } of listAccounts(slot)) { + if (currentAccountId !== undefined ? id === currentAccountId : credential.access === stored.access) continue; + if (credential.accountId !== undefined && mintedIds.has(credential.accountId)) return true; + if (email && normalizedEmail(credential.email) === email) return true; } } return false; @@ -318,8 +358,10 @@ export async function refreshDevinToken( _refreshToken: string, signal?: AbortSignal, credential?: OAuthCredentials, + accountId?: string, ): Promise { - const reread = credential?.source === "local-cli" ? await rereadDevinCliCredential(credential, signal) : undefined; + const reread = credential?.source === "local-cli" + ? await rereadDevinCliCredential(credential, signal, accountId) : undefined; if (reread) return reread; // Cognition has no refresh endpoint. Extending the stored expiry here is what // the carried implementation did, and it makes a revoked key look valid diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 6e61b63399f..927480c95fa 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -192,6 +192,8 @@ interface OAuthProviderDef { refreshToken: string, signal?: AbortSignal, credential?: OAuthCredentials, + /** Store row being refreshed; passed by the generic lock only. */ + accountId?: string, ): Promise; /** provider entry written into config.json on first login. */ providerConfig: OcxProviderConfig; @@ -1057,7 +1059,7 @@ export async function refreshGenericAccountWithLock( } const generation = credentialGeneration(stored); try { - const fresh = merged(await def.refresh(stored.refresh, deps.signal, stored), stored); + const fresh = merged(await def.refresh(stored.refresh, deps.signal, stored, accountId), stored); const outcome = await mergeAccountCredential(provider, accountId, fresh, { expectedGeneration: generation, afterPrePersistRead: deps.afterPrePersistRead, diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index d7dc6b3e66c..46958f41f70 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -14,8 +14,9 @@ const ROTATED = "devin-session-token$synthetic-rotated"; const originalFetch = globalThis.fetch; // GetUserJwt stand-in: the identity a key mints, keyed by the key inside the protobuf body. -let mintedIdentity: Record = {}; +let mintedIdentity: Record = {}; let mintCalls = 0; +let mintFailure: (() => Response) | undefined; function fakeUserJwt(payload: object): string { const part = (value: object) => Buffer.from(JSON.stringify(value)).toString("base64url"); return `${part({ alg: "HS256", typ: "JWT" })}.${part({ ...payload, exp: 9_999_999_999 })}.c2lnbmF0dXJl`; @@ -24,6 +25,7 @@ const mintFetch = (async (input: Parameters[0], init?: RequestInit const url = String(input instanceof Request ? input.url : input); if (!url.endsWith("/exa.auth_pb.AuthService/GetUserJwt")) return originalFetch(input, init); mintCalls++; + if (mintFailure) return mintFailure(); const body = Buffer.from(init?.body as Uint8Array).toString("latin1"); const key = Object.keys(mintedIdentity).find(candidate => body.includes(candidate)); const identity = key ? mintedIdentity[key] : undefined; @@ -81,6 +83,7 @@ beforeEach(() => { holdDeadSend = undefined; mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", email: "rotated@example.com" } }; mintCalls = 0; + mintFailure = undefined; globalThis.fetch = mintFetch; previousCliPath = process.env[DEVIN_CLI_CREDENTIALS_ENV]; // Never let a test read the developer's real CLI credential. @@ -213,6 +216,8 @@ test.each([ expect(sentKeys).toEqual([DEAD]); expect(cliAccount()?.needsReauth).toBe(true); expect(cliAccount()?.credential.access).toBe(DEAD); + // The key is only ever sent to an allowlisted host, including the identity probe. + expect(mintCalls).toBe(0); }); test("a CLI key another stored account already owns is not adopted", async () => { @@ -227,7 +232,15 @@ test("a CLI key another stored account already owns is not adopted", async () => }); test.each([ - ["a key that cannot mint a user_jwt", async () => { mintedIdentity = {}; await saveCliImport(DEAD); }], + ["a key Cognition refuses with 401", async () => { mintedIdentity = {}; await saveCliImport(DEAD); }], + ["a key Cognition refuses with 403", async () => { + mintFailure = () => new Response("", { status: 403 }); + await saveCliImport(DEAD); + }], + ["a minted token without auth_uid", async () => { + mintedIdentity = { [ROTATED]: { email: "rotated@example.com" } }; + await saveCliImport(DEAD); + }], ["a slot whose recorded accountId differs", async () => { await saveCliImport(DEAD, { accountId: "uid-before" }); }], ["a slot whose recorded email differs", async () => { await saveCliImport(DEAD, { email: "before@example.com" }); }], ["an identity another stored account owns", async () => { @@ -245,8 +258,39 @@ test.each([ expect(cliAccount()?.credential.access).toBe(DEAD); }); +test.each([ + ["a transient 503", () => new Response("", { status: 503 })], + ["a 429", () => new Response("", { status: 429 })], + ["a network failure", () => { throw new TypeError("fetch failed"); }], +])("an identity probe that fails with %s does not flag the account", async (_label, failure) => { + await saveCliImport(DEAD); + writeCliFile(ROTATED); + mintFailure = failure; + + const body = await (await run()).json() as { error: { type: string; message: string } }; + + expect(body.error.type).toBe("authentication_error"); + expect(body.error.message).not.toContain("ocx login devin"); + expect(cliAccount()?.needsReauth).not.toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); + + // Once the probe recovers, the next 401 adopts the rotated key. + mintFailure = undefined; + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.access).toBe(ROTATED); +}); + +test("a slot recorded from the key's `sub` claim still matches the minted identity", async () => { + mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", sub: "sub-rotated", email: "rotated@example.com" } }; + await saveCliImport(DEAD, { accountId: "sub-rotated" }); + writeCliFile(ROTATED); + + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.accountId).toBe("uid-rotated"); +}); + test("a slot whose recorded identity matches adopts the rotated key", async () => { - await saveCliImport(DEAD, { accountId: "uid-rotated", email: "Rotated@example.com" }); + await saveCliImport(DEAD, { accountId: "uid-rotated", email: " Rotated@Example.com " }); writeCliFile(ROTATED); expect(await (await run()).text()).toContain("served by rotated"); From 9496aba5113b49e8f75f75eb61c861648562db82 Mon Sep 17 00:00:00 2001 From: Sayo Date: Sun, 27 Sep 2026 20:48:31 +0530 Subject: [PATCH 4/9] fix(devin): an unreadable CLI credential file does not flag the account A credentials.toml that exists but cannot be read or parsed (locked, or mid-write by `devin auth login`) fell through to invalid_grant and marked the account needsReauth, stranding a live key. It now fails the refresh with the same non-terminal error as a transient identity-probe failure; only a missing file or a key Cognition refuses flags the account. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit aa3e0727c6d830a80602fd98a43cbd925b697a2a) --- src/oauth/devin.ts | 3 +++ structure/transports/responses-failover.md | 2 +- .../responses-devin-401-replay.test.ts | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 42607464cb0..33c222ac3d6 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -304,6 +304,9 @@ async function rereadDevinCliCredential( currentAccountId: string | undefined, ): Promise { const outcome = readDevinCliCredentialOutcome(); + // A file that exists but cannot be read or parsed may be mid-write by `devin auth login` + // or briefly locked; like a failed identity mint, that says nothing about the key. + if (outcome.kind === "unreadable" || outcome.kind === "incomplete") throw new DevinIdentityProbeUnavailableError(); if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); if (apiBaseUrl === undefined) return undefined; diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 7b0bf96836c..ffcf7e3263a 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -259,7 +259,7 @@ By design a single upstream 401 on an `oauth`-source Devin account marks it need no refresh endpoint and there is no confirming probe. A `local-cli` account instead re-reads the CLI file and adopts a different key only if its host passes `validateDevinApiBaseUrl`, the key mints a user_jwt (its auth_uid/email are the identity; the session token has none), that identity does not -contradict the slot's, and no other slot owns the key or identity; the adopted identity is recorded. +contradict the slot's, and no other slot owns the key or identity; the adopted identity is recorded. An unreadable or half-written file, or a mint failing other than 401/403, fails the refresh unflagged. Test: `tests/responses/responses-devin-401-replay.test.ts`. ## Optional client transport hints diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index 46958f41f70..d5809d6ba4f 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -280,6 +280,24 @@ test.each([ expect(cliAccount()?.credential.access).toBe(ROTATED); }); +test("a credential file caught mid-write does not flag the account", async () => { + await saveCliImport(DEAD); + // Half-written by `devin auth login`: the key line is there, the server line is not yet. + writeFileSync(home.path("devin-credentials.toml"), `windsurf_api_key = "${ROTATED}"\n`); + + const body = await (await run()).json() as { error: { type: string; message: string } }; + + expect(body.error.type).toBe("authentication_error"); + expect(body.error.message).not.toContain("ocx login devin"); + expect(cliAccount()?.needsReauth).not.toBe(true); + expect(mintCalls).toBe(0); + + // Once the write completes, the next 401 adopts the rotated key. + writeCliFile(ROTATED); + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.access).toBe(ROTATED); +}); + test("a slot recorded from the key's `sub` claim still matches the minted identity", async () => { mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", sub: "sub-rotated", email: "rotated@example.com" } }; await saveCliImport(DEAD, { accountId: "sub-rotated" }); From 9eab47ca3ec3310b24f93f4868dc5b4423fc933c Mon Sep 17 00:00:00 2001 From: Sayo Date: Sun, 27 Sep 2026 20:56:20 +0530 Subject: [PATCH 5/9] fix(responses): refresh the sent account after an A->B->A selection refreshResolvedOAuthSelection skipped the forced refresh whenever the selection revision had moved, even when the selection had come back to the account that was sent. The rejected credential was then replayed and the one recovery attempt spent on it. Refresh whenever the current selection is the sent account. When applying the alternate still fails (a newer manual selection names the flagged account), project the login instruction instead of the raw upstream 401. Co-Authored-By: Claude Opus 5.5 (1M context) (cherry picked from commit 91bf2517843c187fa27d30ae18ac221d877477f9) --- src/server/responses/request-transport.ts | 7 +++--- src/server/responses/run-turn-execution.ts | 5 +++- .../responses-devin-401-replay.test.ts | 23 ++++++++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/server/responses/request-transport.ts b/src/server/responses/request-transport.ts index 8bcd81bbf58..a884edc5c7a 100644 --- a/src/server/responses/request-transport.ts +++ b/src/server/responses/request-transport.ts @@ -196,9 +196,10 @@ export async function prepareResponsesTransport( }; const refreshResolvedOAuthSelection = async (sent: OAuthAccessSnapshot): Promise => { const current = captureOAuthAccountSelection(route.providerName); - const unchanged = current?.accountId === oauthSelection?.accountId - && current?.revision === oauthSelection?.revision; - const candidate = unchanged ? await forceRefreshOAuthAccessSnapshot(sent) : sent; + // Keyed on the account, not the selection revision: a selection that moved away and back + // (A -> B -> A) before the 401 landed still serves the rejected credential, and skipping + // the refresh would replay it and spend the one recovery attempt. + const candidate = current?.accountId === sent.accountId ? await forceRefreshOAuthAccessSnapshot(sent) : sent; const admitted = await commitResolvedOAuthSelection(candidate); if (!admitted) throw new Error("OAuth selection changed during credential recovery"); if (kiroLoadEnabled && options.accountLoad?.lease?.accountId !== admitted.accountId) { diff --git a/src/server/responses/run-turn-execution.ts b/src/server/responses/run-turn-execution.ts index cdb52c72fc6..2d84f3fca16 100644 --- a/src/server/responses/run-turn-execution.ts +++ b/src/server/responses/run-turn-execution.ts @@ -425,7 +425,10 @@ export async function executeResponsesRunTurn( } sendBudgetState.pendingHopPermit = hop.permit; return true; - } catch { + } catch (err) { + // Applying the alternate can still fail, e.g. when a newer manual selection points back + // at the flagged account; the client then needs the login instruction, not the raw 401. + Object.assign(error, { errorType: "authentication_error", message: publicOAuthAuthenticationErrorMessage(err) }); hop.permit?.release(); return false; } diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index d5809d6ba4f..a35b47fbea0 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -2,7 +2,7 @@ import { afterAll, afterEach, beforeEach, expect, mock, test } from "bun:test"; import { writeFileSync } from "node:fs"; import type { ProviderAdapter } from "../../src/adapters/base"; import type { AdapterEvent, OcxConfig, OcxProviderConfig } from "../../src/types"; -import { getAccountSet, saveCredential } from "../../src/oauth/store"; +import { getAccountSet, saveCredential, setActiveAccount } from "../../src/oauth/store"; import { clearGenericFailoverHealth } from "../../src/oauth/generic-account-failover"; import { DEVIN_CLI_CREDENTIALS_ENV } from "../../src/oauth/devin/cli-import"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; @@ -356,3 +356,24 @@ test("a 429 is not treated as an authentication failure", async () => { expect(sentKeys).toEqual([LIVE]); expect(account("limited")?.needsReauth).not.toBe(true); }); + +test("a selection that leaves the revoked account and returns to it before the 401 still refreshes it", async () => { + await saveDevin(LIVE, "spare"); + await saveDevin(DEAD, "revoked"); + const revoked = account("revoked")!.id; + // Same account id at 401 time, newer selection revision: the refresh must still run, or the + // rejected key is replayed and the one allowed recovery is spent on it. + holdDeadSend = async () => { + holdDeadSend = undefined; + await setActiveAccount("devin", account("spare")!.id); + await setActiveAccount("devin", revoked); + }; + + const body = await (await run()).json() as { error: { message: string } }; + + expect(sentKeys.filter(key => key === DEAD)).toHaveLength(1); + expect(account("revoked")?.needsReauth).toBe(true); + // The operator's newer manual selection names the revoked account, so no automatic move + // overrides it; the client is told to log in rather than shown the raw upstream 401. + expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); +}); From c3f7162235c5a4351f9de5f26f4a72bd70ade529 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 16:57:44 +0900 Subject: [PATCH 6/9] fix(devin): make CLI adoption atomic and preserve paused recovery Co-authored-by: Sayo --- .../src/content/docs/reference/adapters.md | 8 ++- src/oauth/devin.ts | 30 ++++++++++- src/oauth/index.ts | 8 ++- src/oauth/store.ts | 2 +- src/server/responses/run-turn-execution.ts | 25 ++++++++- structure/providers-and-adapters.md | 5 ++ structure/transports/responses-failover.md | 29 ++--------- .../responses-devin-401-replay.test.ts | 52 ++++++++++++++++++- tests/server/server-kiro-refusal-e2e.test.ts | 22 ++++++++ 9 files changed, 148 insertions(+), 33 deletions(-) diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 5f15be73148..71c282d62d6 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -556,8 +556,12 @@ configuration that names the old id is rewritten at startup. stream. Cognition enforces a per-tool-description length limit (6,998 chars) and an exact-phrase blocklist; the adapter sanitizes known triggers and truncates over-long descriptions before encoding. -- Devin/Cognition API keys do not refresh. Run `ocx login devin` again when the key expires or is - revoked. +- Devin/Cognition API keys have no refresh endpoint. If Cognition rejects a stored key with 401, + OpenCodex marks that account for reauthentication and can use another signed-in account for + the turn. Run `ocx login devin` again for a revoked browser-login key. A CLI-imported account + can follow a later `devin auth login` key rotation when the CLI host and account identity + validate; if the CLI file is temporarily unreadable or the identity check is unavailable, + retry after it recovers. A paused account stays paused during this recovery and returns 403. - Only the credential is local when the CLI import path is used. The turn itself goes to Cognition either way, so the import and browser login paths differ in nothing but where the credential came from. Install the CLI with diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 33c222ac3d6..a5ba0e20dd8 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -20,7 +20,7 @@ import { registerUser } from "./devin/register-user"; import { DEVIN_DEFAULT_API_SERVER, resolveDevinApiBaseUrl, validateDevinApiBaseUrl } from "./devin/api-base"; import { readDevinCliCredentialOutcome } from "./devin/cli-import"; import { CloudAuthError, mintUserJwt } from "../adapters/devin/cloud-direct/auth"; -import { getCredential, listAccounts } from "./store"; +import { getCredential, listAccounts, type AuthStore } from "./store"; import { DEPRECATED_OAUTH_PROVIDER_ALIASES } from "./index"; export { DEVIN_DEFAULT_API_SERVER } from "./devin/api-base"; @@ -298,6 +298,30 @@ class DevinIdentityProbeUnavailableError extends Error { const normalizedEmail = (value: string | undefined): string | undefined => value?.trim().toLowerCase() || undefined; +const devinMintedIdentities = new WeakMap>(); + +/** Recheck the minted identity and key against the locked, freshly read store. */ +export function assertDevinCliAdoptionOwnership( + store: AuthStore, + provider: string, + accountId: string, + credential: OAuthCredentials, +): void { + const mintedIds = devinMintedIdentities.get(credential); + if (!mintedIds) return; + const email = normalizedEmail(credential.email); + for (const slot of [provider, ...devinAliasCredentialSlots(provider)]) { + for (const row of store[slot]?.accounts ?? []) { + if (slot === provider && row.id === accountId) continue; + if (row.credential.access === credential.access + || (row.credential.accountId !== undefined && mintedIds.has(row.credential.accountId)) + || (email !== undefined && normalizedEmail(row.credential.email) === email)) { + throw new DevinIdentityProbeUnavailableError(); + } + } + } +} + async function rereadDevinCliCredential( stored: OAuthCredentials, signal: AbortSignal | undefined, @@ -333,7 +357,9 @@ async function rereadDevinCliCredential( const storedEmail = normalizedEmail(stored.email); if (storedEmail?.includes("@") && storedEmail !== normalizedEmail(email)) return undefined; if (devinIdentityOwnedElsewhere(stored, currentAccountId, mintedIds, normalizedEmail(email))) return undefined; - return { ...credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"), accountId: authUid, ...(email ? { email } : {}) }; + const adopted = { ...credentialsFromApiKey(outcome.file.apiKey, apiBaseUrl, "local-cli"), accountId: authUid, ...(email ? { email } : {}) }; + devinMintedIdentities.set(adopted, mintedIds); + return adopted; } /** diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 927480c95fa..7f0b537fd70 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -30,6 +30,7 @@ import { type OAuthCredentialWriteReceipt, type OAuthRefreshIntent, type OAuthRefreshIntentCleanupPending, + type AuthStore, } from "./store"; import { loginXai, refreshXaiToken, XAI_LOCAL_CLI_DETACH_WARNING, XaiTokenRequestError } from "./xai"; import { ANTHROPIC_OAUTH_BETA, AnthropicTokenError, loginAnthropic, refreshAnthropicToken } from "./anthropic"; @@ -38,7 +39,7 @@ import { loginNous, NousTokenError, refreshNousToken, clearNousRefreshIntent, Re import { loginChatGPT, refreshChatGPTToken, type ChatGPTLoginFlow } from "./chatgpt"; import { loginAntigravity, refreshAntigravityToken } from "./google-antigravity"; import { loginCursor, refreshCursorToken } from "./cursor"; -import { loginDevin, refreshDevinToken } from "./devin"; +import { assertDevinCliAdoptionOwnership, loginDevin, refreshDevinToken } from "./devin"; import { validateDevinApiBaseUrl } from "./devin/api-base"; import { loginGithubCopilot, refreshGithubCopilotToken, validateCopilotApiBaseUrl } from "./github-copilot"; import { loginCommandCode, refreshCommandCodeToken } from "./command-code"; @@ -1059,10 +1060,13 @@ export async function refreshGenericAccountWithLock( } const generation = credentialGeneration(stored); try { - const fresh = merged(await def.refresh(stored.refresh, deps.signal, stored, accountId), stored); + const refreshed = await def.refresh(stored.refresh, deps.signal, stored, accountId); + const fresh = merged(refreshed, stored); const outcome = await mergeAccountCredential(provider, accountId, fresh, { expectedGeneration: generation, afterPrePersistRead: deps.afterPrePersistRead, + ...(provider === "devin" ? { assertOwnership: (store: AuthStore) => + assertDevinCliAdoptionOwnership(store, provider, accountId, refreshed) } : {}), }); if (outcome.superseded) { if (outcome.stored.expires > Date.now() + REFRESH_SKEW_MS) return outcome.stored.access; diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 71b5b485974..5c9fff453fe 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -1379,5 +1379,5 @@ export async function markAccountNeedsReauth( }, [provider, accountId]); } -export async function mergeAccountCredential(provider:string,accountId:string,credential:OAuthCredentials,opts:{expectedGeneration?:string;afterPrePersistRead?:()=>void|Promise}={}):Promise<{superseded:false}|{superseded:true;stored:OAuthCredentials}>{const safe=normalizeCredential(credential);if(!safe)throw new Error("Refusing to persist invalid OAuth credential");return await mutateStore(async store=>{await opts.afterPrePersistRead?.();const account=store[provider]?.accounts.find(x=>x.id===accountId);if(!account)throw new Error(`OAuth account disappeared before persist: ${provider}`);if(opts.expectedGeneration!==undefined&&credentialGeneration(account.credential)!==opts.expectedGeneration)return{superseded:true,stored:account.credential};account.credential=safe;delete account.needsReauth;return{superseded:false};},[provider,accountId,safe,opts.expectedGeneration]);} +export async function mergeAccountCredential(provider:string,accountId:string,credential:OAuthCredentials,opts:{expectedGeneration?:string;afterPrePersistRead?:()=>void|Promise;assertOwnership?: (store: AuthStore) => void}={}):Promise<{superseded:false}|{superseded:true;stored:OAuthCredentials}>{const safe=normalizeCredential(credential);if(!safe)throw new Error("Refusing to persist invalid OAuth credential");return await mutateStore(async store=>{await opts.afterPrePersistRead?.();const account=store[provider]?.accounts.find(x=>x.id===accountId);if(!account)throw new Error(`OAuth account disappeared before persist: ${provider}`);if(opts.expectedGeneration!==undefined&&credentialGeneration(account.credential)!==opts.expectedGeneration)return{superseded:true,stored:account.credential};opts.assertOwnership?.(store);account.credential=safe;delete account.needsReauth;return{superseded:false};},[provider,accountId,safe,opts.expectedGeneration]);} export async function markAccountNeedsReauthIfGeneration(provider:string,accountId:string,generation:string,writerGeneration=captureConfigGeneration()):Promise{const key=oauthAccountKey(provider,accountId);if(writerGeneration{const account=store[provider]?.accounts.find(x=>x.id===accountId);if(!account?.credential||credentialGeneration(account.credential)!==generation)return false;if(writerGeneration, ): Promise => { @@ -409,6 +410,12 @@ export async function executeResponsesRunTurn( try { admitted = await applyFailoverSnapshot(await refreshResolvedOAuthSelection(sent)); } catch (err) { + if (err instanceof OAuthAccountPausedError) { + pausedAuthRecovery = true; + Object.assign(error, { status: 403, errorType: "permission_error", message: publicOAuthAuthenticationErrorMessage(err) }); + hop.permit?.release(); + return false; + } // Not only OAuthLoginRequiredError: a concurrent request that already flagged this // account and moved the selection makes this refresh fail as "selection changed". The // helper still requires the sent generation to be flagged needsReauth, so it is safe. @@ -658,6 +665,14 @@ export async function executeResponsesRunTurn( // Preflight holds only heartbeats and the first meaningful event. A first-event 429 can be // replayed transparently; after any output reaches the bridge, a later error stays terminal. eventSource = await preflightRunTurnFailover(eventSource, wsFirstParsed, preflightDeadlineAt); + if (pausedAuthRecovery) { + cancelResponseCompletion(); + runTurnAbort.abort(); + queue.close(); + cleanupRunTurnAbort(); + releaseSearchProbeLease(); + return formatErrorResponse(403, "permission_error", publicOAuthAuthenticationErrorMessage(new OAuthAccountPausedError())); + } } if (grokDevinPreflight) { const preflight = await preflightAdapterEvents(eventSource, undefined, { @@ -788,6 +803,14 @@ export async function executeResponsesRunTurn( for await (const event of await preflightRunTurnFailover( (async function* () { yield* firstAttemptEvents; })(), )) runTurnEvents.push(event); + if (pausedAuthRecovery) { + cancelResponseCompletion(); + runTurnAbort.abort(); + queue.close(); + cleanupRunTurnAbort(); + releaseSearchProbeLease(); + return formatErrorResponse(403, "permission_error", publicOAuthAuthenticationErrorMessage(new OAuthAccountPausedError())); + } } if (grokDevinPreflight) { const preflight = await preflightAdapterEvents( diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 15b2c77556a..1c3961971ae 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -91,6 +91,11 @@ proactive refresh, per-account quota probes (`accountQuotaProbeSkip` in Muse key-mint quota read, and xAI/Gemini web-search sidecar eligibility. The stored credential remains available for resume, while requests with no unpaused account fail with 403 rather than as a login failure. +Devin's local-CLI forced refresh validates the CLI tenant host before probing its key with a +bounded `GetUserJwt` call. It adopts a changed key only when the minted identity matches the +stored slot and the key and identity are unowned across Devin and alias accounts at the locked +store write. The generation check still protects concurrent edits. A losing adoption or +unreadable CLI file leaves the account unflagged; a paused account returns 403. The account actually sent supplies the generation fence; a rotated bearer always travels with its own profile ARN and region. Reactive rotation follows the stored two-account quorum, while refusal-aware first admission follows the proactive preference setting. diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index ffcf7e3263a..5bcb67cacef 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -248,19 +248,9 @@ the failure on the current target. ## runTurn pre-output 401 replay -`src/server/responses/run-turn-execution.ts` runs the `adapter-dispatch.ts` OAuth 401 replay on the -runTurn first-event preflight for `isOAuth401ReplayProvider` routes (Devin is the runTurn member): a -structured 401 before output or a replay-unsafe heartbeat force-refreshes the sent credential once -per request under an `auth-recovery` hop and replays the turn. A terminal refresh has already marked -the account needsReauth; the turn moves to a surviving stored account via -`tryAlternateAfterTerminalRefresh`, else the 401 carries the login instruction. Devin quota -`permission_denied` maps to 429 and plain `permission_denied` to 403, so neither refreshes. -By design a single upstream 401 on an `oauth`-source Devin account marks it needsReauth: Cognition has -no refresh endpoint and there is no confirming probe. A `local-cli` account instead re-reads the CLI -file and adopts a different key only if its host passes `validateDevinApiBaseUrl`, the key mints a -user_jwt (its auth_uid/email are the identity; the session token has none), that identity does not -contradict the slot's, and no other slot owns the key or identity; the adopted identity is recorded. An unreadable or half-written file, or a mint failing other than 401/403, fails the refresh unflagged. -Test: `tests/responses/responses-devin-401-replay.test.ts`. +`src/server/responses/run-turn-execution.ts` handles a structured pre-output 401 for `isOAuth401ReplayProvider` runTurn routes with one generation-fenced forced refresh and an `auth-recovery` hop. A terminal refresh may admit a surviving account; otherwise the client gets the login instruction. Devin quota `permission_denied` (429) and plain permission denial (403) do not trigger this path. +An `oauth`-source Devin key rejected with 401 needs reauthentication because Cognition has no refresh endpoint. A `local-cli` slot can adopt a changed CLI key after host and bounded identity validation; [OAuth/Devin ownership](../providers-and-adapters.md) defines the locked store check across aliases. Unreadable files and transient probes leave the account unflagged. A pause during refresh returns 403 without retry or reauth. Kiro's terminal alternate requires `OAuthLoginRequiredError`; transient refresh failures do not enter it. +Tests: `tests/responses/responses-devin-401-replay.test.ts` and `tests/server/server-kiro-refusal-e2e.test.ts`. ## Optional client transport hints @@ -602,14 +592,5 @@ provider cannot establish the original serving identity and remains portable. ## Anthropic Fast downgrade recovery -The `anthropic` OAuth and `anthropic-apikey` registry entries use native `anthropic-speed` FastWire -only for `claude-opus-5-5`, `claude-opus-5` and `claude-opus-4-8`; there is no provider-wide Fast -fallback. In the main adapter dispatch loop, a fast refusal naming fast mode or the `speed` parameter -(400 or 429), or a 429 with a fast-pool remaining header of zero, may use one shared-budget repair -permit for a standard-speed resend. The resend charges the root workflow send counter once without a -second request-budget charge. The request retains the drop decision through later rebuilds and records -`anthropic-fast-downgrade`, cause `parameter-rejected`, and a `downgraded` / `response-declined` tier -outcome. A spent budget leaves the original refusal intact; generic 429 and 529 responses keep their -ordinary handling. This repair precedes same-target 429 waiting and credential rotation. Continuation -and sidecar owners do not use this repair. Coverage: `tests/routing/fastwire-policy.test.ts` and -`tests/responses/responses-anthropic-fast-downgrade.test.ts`. +The `anthropic` OAuth and `anthropic-apikey` registry entries use native `anthropic-speed` FastWire only for `claude-opus-5-5`, `claude-opus-5` and `claude-opus-4-8`; there is no provider-wide Fast fallback. In the main adapter dispatch loop, a fast refusal naming fast mode or the `speed` parameter (400 or 429), or a 429 with a fast-pool remaining header of zero, may use one shared-budget repair permit for a standard-speed resend. +The resend charges the root workflow send counter once without a second request-budget charge. The request retains the drop decision through later rebuilds and records `anthropic-fast-downgrade`, cause `parameter-rejected`, and a `downgraded` / `response-declined` tier outcome. A spent budget leaves the original refusal intact; generic 429 and 529 responses keep their ordinary handling. This repair precedes same-target 429 waiting and credential rotation. Continuation and sidecar owners do not use this repair. Coverage: `tests/routing/fastwire-policy.test.ts` and `tests/responses/responses-anthropic-fast-downgrade.test.ts`. diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index a35b47fbea0..d872f1831a8 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -2,7 +2,8 @@ import { afterAll, afterEach, beforeEach, expect, mock, test } from "bun:test"; import { writeFileSync } from "node:fs"; import type { ProviderAdapter } from "../../src/adapters/base"; import type { AdapterEvent, OcxConfig, OcxProviderConfig } from "../../src/types"; -import { getAccountSet, saveCredential, setActiveAccount } from "../../src/oauth/store"; +import { getAccountSet, saveCredential, setAccountPaused, setActiveAccount } from "../../src/oauth/store"; +import { forceRefreshOAuthAccessSnapshot, getValidAccessTokenSnapshot } from "../../src/oauth"; import { clearGenericFailoverHealth } from "../../src/oauth/generic-account-failover"; import { DEVIN_CLI_CREDENTIALS_ENV } from "../../src/oauth/devin/cli-import"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; @@ -11,12 +12,14 @@ import { createTempHome } from "../helpers/temp-home"; const DEAD = "devin-session-token$synthetic-dead"; const LIVE = "devin-session-token$synthetic-live"; const ROTATED = "devin-session-token$synthetic-rotated"; +const DEAD_OTHER = "devin-session-token$synthetic-dead-other"; const originalFetch = globalThis.fetch; // GetUserJwt stand-in: the identity a key mints, keyed by the key inside the protobuf body. let mintedIdentity: Record = {}; let mintCalls = 0; let mintFailure: (() => Response) | undefined; +let holdMint: (() => Promise) | undefined; function fakeUserJwt(payload: object): string { const part = (value: object) => Buffer.from(JSON.stringify(value)).toString("base64url"); return `${part({ alg: "HS256", typ: "JWT" })}.${part({ ...payload, exp: 9_999_999_999 })}.c2lnbmF0dXJl`; @@ -25,6 +28,7 @@ const mintFetch = (async (input: Parameters[0], init?: RequestInit const url = String(input instanceof Request ? input.url : input); if (!url.endsWith("/exa.auth_pb.AuthService/GetUserJwt")) return originalFetch(input, init); mintCalls++; + await holdMint?.(); if (mintFailure) return mintFailure(); const body = Buffer.from(init?.body as Uint8Array).toString("latin1"); const key = Object.keys(mintedIdentity).find(candidate => body.includes(candidate)); @@ -84,6 +88,7 @@ beforeEach(() => { mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", email: "rotated@example.com" } }; mintCalls = 0; mintFailure = undefined; + holdMint = undefined; globalThis.fetch = mintFetch; previousCliPath = process.env[DEVIN_CLI_CREDENTIALS_ENV]; // Never let a test read the developer's real CLI credential. @@ -298,6 +303,51 @@ test("a credential file caught mid-write does not flag the account", async () => expect(cliAccount()?.credential.access).toBe(ROTATED); }); +test("concurrent identity-less CLI slots cannot both adopt one rotated key", async () => { + await saveCliImport(DEAD); + const first = await getValidAccessTokenSnapshot("devin"); + await saveCliImport(DEAD_OTHER); + const second = await getValidAccessTokenSnapshot("devin"); + expect(first.accountId).not.toBe(second.accountId); + writeCliFile(ROTATED); + const bothMinted = Promise.withResolvers(); + const releaseMint = Promise.withResolvers(); + holdMint = async () => { + if (mintCalls === 2) bothMinted.resolve(); + await releaseMint.promise; + }; + const refreshes = Promise.allSettled([ + forceRefreshOAuthAccessSnapshot(first), forceRefreshOAuthAccessSnapshot(second), + ]); + await bothMinted.promise; + releaseMint.resolve(); + const outcomes = await refreshes; + const rows = getAccountSet("devin")!.accounts; + expect(outcomes.filter(outcome => outcome.status === "fulfilled")).toHaveLength(1); + expect(rows.filter(row => row.credential.access === ROTATED)).toHaveLength(1); + expect(rows.every(row => row.needsReauth !== true)).toBe(true); +}); + +test.each([false, true])("an account paused during 401 refresh returns 403 (stream=%s)", async stream => { + await saveCliImport(DEAD); + const accountId = cliAccount()!.id; + writeCliFile(ROTATED); + const mintStarted = Promise.withResolvers(); + const releaseMint = Promise.withResolvers(); + holdMint = async () => { mintStarted.resolve(); await releaseMint.promise; }; + const responsePromise = run(stream); + await mintStarted.promise; + await setAccountPaused("devin", accountId, true); + releaseMint.resolve(); + const response = await responsePromise; + const body = await response.json() as { error: { type: string; message: string } }; + expect(response.status).toBe(403); + expect(body.error.type).toBe("permission_error"); + expect(body.error.message).toContain("OAuth account is paused"); + expect(sentKeys).toEqual([DEAD]); + expect(cliAccount()?.needsReauth).not.toBe(true); +}); + test("a slot recorded from the key's `sub` claim still matches the minted identity", async () => { mintedIdentity = { [ROTATED]: { auth_uid: "uid-rotated", sub: "sub-rotated", email: "rotated@example.com" } }; await saveCliImport(DEAD, { accountId: "sub-rotated" }); diff --git a/tests/server/server-kiro-refusal-e2e.test.ts b/tests/server/server-kiro-refusal-e2e.test.ts index abd4624cbc4..74d17cc9514 100644 --- a/tests/server/server-kiro-refusal-e2e.test.ts +++ b/tests/server/server-kiro-refusal-e2e.test.ts @@ -474,6 +474,28 @@ describe("Kiro refusal recovery through Responses", () => { } finally { await server.stop(true); } }); + test("a transient Kiro refresh failure does not enter terminal failover", async () => { + const [a] = await seed(); saveConfig(config()); + const sends: string[] = []; + globalThis.fetch = (async (input, init) => { + const url = input instanceof Request ? input.url : String(input); + if (url.endsWith("/refreshToken")) return new Response("", { status: 503 }); + if (url.startsWith("https://runtime.") && url.endsWith(".kiro.dev/")) { + const auth = new Headers(init?.headers).get("authorization") ?? ""; + sends.push(auth); + return auth === "Bearer access-a" ? new Response("expired", { status: 401 }) : answer("served by b"); + } + return realFetch(input, init); + }) as typeof fetch; + const server = startServer(0); + try { + const response = await post(server); + expect(response.status).toBe(401); + expect(sends).toEqual(["Bearer access-a"]); + expect(getAccountSet("kiro")!.accounts.find(row => row.id === a!.id)?.needsReauth).not.toBe(true); + } finally { await server.stop(true); } + }); + for (const [status, reason] of [[400, "MONTHLY_REQUEST_COUNT"], [403, "TEMPORARILY_SUSPENDED"]] as const) { test(`failed alternate resolution returns original ${status} with normalized Kiro message`, async () => { const [, b] = await seed(); saveConfig(config()); From 0d688a97dc20eb70156009a89004143d4968e68f Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 17:26:57 +0900 Subject: [PATCH 7/9] fix(devin): preserve alias identity and paused key during adoption Co-authored-by: Sayo --- src/oauth/devin.ts | 3 +- src/oauth/index.ts | 6 ++-- structure/providers-and-adapters.md | 5 +-- structure/transports/responses-failover.md | 2 +- .../responses-devin-401-replay.test.ts | 33 ++++++++++++++++++- 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index a5ba0e20dd8..a99523adac2 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -312,7 +312,8 @@ export function assertDevinCliAdoptionOwnership( const email = normalizedEmail(credential.email); for (const slot of [provider, ...devinAliasCredentialSlots(provider)]) { for (const row of store[slot]?.accounts ?? []) { - if (slot === provider && row.id === accountId) continue; + // A legacy alias can still hold the same account id during rekey. + if (row.id === accountId) continue; if (row.credential.access === credential.access || (row.credential.accountId !== undefined && mintedIds.has(row.credential.accountId)) || (email !== undefined && normalizedEmail(row.credential.email) === email)) { diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 7f0b537fd70..18249758a53 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1065,8 +1065,10 @@ export async function refreshGenericAccountWithLock( const outcome = await mergeAccountCredential(provider, accountId, fresh, { expectedGeneration: generation, afterPrePersistRead: deps.afterPrePersistRead, - ...(provider === "devin" ? { assertOwnership: (store: AuthStore) => - assertDevinCliAdoptionOwnership(store, provider, accountId, refreshed) } : {}), + ...(provider === "devin" ? { assertOwnership: (store: AuthStore) => { + if (store[provider]?.accounts.find(row => row.id === accountId)?.paused) throw new OAuthAccountPausedError(); + assertDevinCliAdoptionOwnership(store, provider, accountId, refreshed); + } } : {}), }); if (outcome.superseded) { if (outcome.stored.expires > Date.now() + REFRESH_SKEW_MS) return outcome.stored.access; diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 1c3961971ae..1ce8ff61623 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -94,8 +94,9 @@ as a login failure. Devin's local-CLI forced refresh validates the CLI tenant host before probing its key with a bounded `GetUserJwt` call. It adopts a changed key only when the minted identity matches the stored slot and the key and identity are unowned across Devin and alias accounts at the locked -store write. The generation check still protects concurrent edits. A losing adoption or -unreadable CLI file leaves the account unflagged; a paused account returns 403. +store write. The same account id in a legacy alias slot is not a competing owner. The +generation check still protects concurrent edits. A losing adoption or unreadable CLI +file leaves the account unflagged; a paused account returns 403 with its stored key intact. The account actually sent supplies the generation fence; a rotated bearer always travels with its own profile ARN and region. Reactive rotation follows the stored two-account quorum, while refusal-aware first admission follows the proactive preference setting. diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 5bcb67cacef..20ccc74f797 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -249,7 +249,7 @@ the failure on the current target. ## runTurn pre-output 401 replay `src/server/responses/run-turn-execution.ts` handles a structured pre-output 401 for `isOAuth401ReplayProvider` runTurn routes with one generation-fenced forced refresh and an `auth-recovery` hop. A terminal refresh may admit a surviving account; otherwise the client gets the login instruction. Devin quota `permission_denied` (429) and plain permission denial (403) do not trigger this path. -An `oauth`-source Devin key rejected with 401 needs reauthentication because Cognition has no refresh endpoint. A `local-cli` slot can adopt a changed CLI key after host and bounded identity validation; [OAuth/Devin ownership](../providers-and-adapters.md) defines the locked store check across aliases. Unreadable files and transient probes leave the account unflagged. A pause during refresh returns 403 without retry or reauth. Kiro's terminal alternate requires `OAuthLoginRequiredError`; transient refresh failures do not enter it. +An `oauth`-source Devin key rejected with 401 needs reauthentication because Cognition has no refresh endpoint. A `local-cli` slot can adopt a changed CLI key after host and bounded identity validation; [OAuth/Devin ownership](../providers-and-adapters.md) defines the locked store check across aliases. Unreadable files and transient probes leave the account unflagged. A pause during refresh returns 403 without retry, reauth, or replacing the stored key. Kiro's terminal alternate requires `OAuthLoginRequiredError`; transient refresh failures do not enter it. Tests: `tests/responses/responses-devin-401-replay.test.ts` and `tests/server/server-kiro-refusal-e2e.test.ts`. ## Optional client transport hints diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index d872f1831a8..69bb66dfc3f 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -2,7 +2,7 @@ import { afterAll, afterEach, beforeEach, expect, mock, test } from "bun:test"; import { writeFileSync } from "node:fs"; import type { ProviderAdapter } from "../../src/adapters/base"; import type { AdapterEvent, OcxConfig, OcxProviderConfig } from "../../src/types"; -import { getAccountSet, saveCredential, setAccountPaused, setActiveAccount } from "../../src/oauth/store"; +import { getAccountSet, replaceProviderAccountSet, saveCredential, setAccountPaused, setActiveAccount } from "../../src/oauth/store"; import { forceRefreshOAuthAccessSnapshot, getValidAccessTokenSnapshot } from "../../src/oauth"; import { clearGenericFailoverHealth } from "../../src/oauth/generic-account-failover"; import { DEVIN_CLI_CREDENTIALS_ENV } from "../../src/oauth/devin/cli-import"; @@ -207,6 +207,36 @@ test("a CLI-imported account adopts the key a later `devin auth login` wrote", a expect(row?.credential.expires).toBe(Number.MAX_SAFE_INTEGER); }); +test("a legacy alias copy of the same account does not block CLI key rotation", async () => { + await saveCliImport(DEAD); + const current = cliAccount()!; + await replaceProviderAccountSet("devin-cli", { + activeAccountId: current.id, + accounts: [{ ...current, credential: { + ...current.credential, accountId: "uid-rotated", email: "rotated@example.com", + } }], + }); + writeCliFile(ROTATED); + + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.access).toBe(ROTATED); + expect(getAccountSet("devin-cli")?.accounts[0]?.credential.access).toBe(DEAD); + expect(cliAccount()?.needsReauth).not.toBe(true); +}); + +test("a distinct legacy alias account still blocks an owned CLI identity", async () => { + await saveCredential("devin-cli", { + access: LIVE, refresh: LIVE, expires: Number.MAX_SAFE_INTEGER, + accountId: "uid-rotated", source: "oauth", apiBaseUrl: "https://server.codeium.com", + }); + await saveCliImport(DEAD); + writeCliFile(ROTATED); + + await (await run()).text(); + expect(cliAccount()?.credential.access).toBe(DEAD); + expect(cliAccount()?.needsReauth).toBe(true); +}); + test.each([ ["unchanged", () => writeCliFile(DEAD)], ["missing", () => {}], @@ -346,6 +376,7 @@ test.each([false, true])("an account paused during 401 refresh returns 403 (stre expect(body.error.message).toContain("OAuth account is paused"); expect(sentKeys).toEqual([DEAD]); expect(cliAccount()?.needsReauth).not.toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); }); test("a slot recorded from the key's `sub` claim still matches the minted identity", async () => { From 5d1bd649560779e4a7f6e517ba75799d974ba19c Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 17:45:30 +0900 Subject: [PATCH 8/9] fix(auth): allow same-account Devin alias key adoption Co-authored-by: Sayo --- .../src/content/docs/reference/adapters.md | 6 +++-- src/oauth/devin.ts | 7 +++++- structure/providers-and-adapters.md | 7 +++--- .../responses-devin-401-replay.test.ts | 23 ++++++++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 71c282d62d6..78c347fe73d 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -560,8 +560,10 @@ configuration that names the old id is rewritten at startup. OpenCodex marks that account for reauthentication and can use another signed-in account for the turn. Run `ocx login devin` again for a revoked browser-login key. A CLI-imported account can follow a later `devin auth login` key rotation when the CLI host and account identity - validate; if the CLI file is temporarily unreadable or the identity check is unavailable, - retry after it recovers. A paused account stays paused during this recovery and returns 403. + validate. A legacy `devin-cli` slot for that same account may already hold the new key; + a different account holding it blocks adoption. If the CLI file is temporarily unreadable or + the identity check is unavailable, retry after it recovers. A paused account stays paused + during this recovery and returns 403. - Only the credential is local when the CLI import path is used. The turn itself goes to Cognition either way, so the import and browser login paths differ in nothing but where the credential came from. Install the CLI with diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index a99523adac2..7343b50b34d 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -335,7 +335,12 @@ async function rereadDevinCliCredential( if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); if (apiBaseUrl === undefined) return undefined; - if (findDevinCredentialOwner("devin", outcome.file.apiKey) !== undefined) return undefined; + // A detached alias rekey can already hold this account's new key. Only a + // different account's key is a conflict; the locked write checks again. + for (const slot of ["devin", ...devinAliasCredentialSlots("devin")]) { + if (listAccounts(slot).some(({ id, credential }) => + id !== currentAccountId && credential.access === outcome.file.apiKey)) return undefined; + } let minted: Record | undefined; try { const timeout = AbortSignal.timeout(DEVIN_IDENTITY_MINT_TIMEOUT_MS); diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 1ce8ff61623..4afe190ed93 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -94,9 +94,10 @@ as a login failure. Devin's local-CLI forced refresh validates the CLI tenant host before probing its key with a bounded `GetUserJwt` call. It adopts a changed key only when the minted identity matches the stored slot and the key and identity are unowned across Devin and alias accounts at the locked -store write. The same account id in a legacy alias slot is not a competing owner. The -generation check still protects concurrent edits. A losing adoption or unreadable CLI -file leaves the account unflagged; a paused account returns 403 with its stored key intact. +store write. The same account id in a legacy alias slot is not a competing owner, +even when that alias already holds the rotated key; another account holding the key +still blocks adoption. The generation check still protects concurrent edits. A losing adoption or +unreadable CLI file leaves the account unflagged; a paused account returns 403 with its stored key intact. The account actually sent supplies the generation fence; a rotated bearer always travels with its own profile ARN and region. Reactive rotation follows the stored two-account quorum, while refusal-aware first admission follows the proactive preference setting. diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index 69bb66dfc3f..b7b3e42d8fe 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -207,23 +207,40 @@ test("a CLI-imported account adopts the key a later `devin auth login` wrote", a expect(row?.credential.expires).toBe(Number.MAX_SAFE_INTEGER); }); -test("a legacy alias copy of the same account does not block CLI key rotation", async () => { +test("a legacy alias copy of the same account already holding the rotated key does not block adoption", async () => { await saveCliImport(DEAD); const current = cliAccount()!; await replaceProviderAccountSet("devin-cli", { activeAccountId: current.id, accounts: [{ ...current, credential: { - ...current.credential, accountId: "uid-rotated", email: "rotated@example.com", + ...current.credential, access: ROTATED, refresh: ROTATED, + accountId: "uid-rotated", email: "rotated@example.com", } }], }); writeCliFile(ROTATED); expect(await (await run()).text()).toContain("served by rotated"); + expect(sentKeys).toEqual([DEAD, ROTATED]); expect(cliAccount()?.credential.access).toBe(ROTATED); - expect(getAccountSet("devin-cli")?.accounts[0]?.credential.access).toBe(DEAD); + expect(getAccountSet("devin-cli")?.accounts[0]?.credential.access).toBe(ROTATED); expect(cliAccount()?.needsReauth).not.toBe(true); }); +test("a distinct legacy alias account holding the rotated key blocks adoption", async () => { + await saveCliImport(DEAD); + await saveCredential("devin-cli", { + access: ROTATED, refresh: ROTATED, expires: Number.MAX_SAFE_INTEGER, + source: "oauth", apiBaseUrl: "https://server.codeium.com", + }); + writeCliFile(ROTATED); + + await (await run()).text(); + expect(sentKeys).not.toContain(ROTATED); + expect(mintCalls).toBe(0); + expect(cliAccount()?.credential.access).toBe(DEAD); + expect(cliAccount()?.needsReauth).toBe(true); +}); + test("a distinct legacy alias account still blocks an owned CLI identity", async () => { await saveCredential("devin-cli", { access: LIVE, refresh: LIVE, expires: Number.MAX_SAFE_INTEGER, From 6b42a3b366827ac4c9a693220dfd5ff280fd83f2 Mon Sep 17 00:00:00 2001 From: JUN Date: Mon, 28 Sep 2026 19:35:11 +0900 Subject: [PATCH 9/9] fix(auth): reject identity-less Devin CLI key adoption Require a bound account ID or email before adopting a rotated CLI key. Route identity-less imports through terminal reauthentication and cover cross-user rotation. Co-authored-by: Sayo --- .../src/content/docs/reference/adapters.md | 9 ++- src/oauth/devin.ts | 8 +- structure/providers-and-adapters.md | 12 +-- structure/transports/responses-failover.md | 2 +- .../responses-devin-401-replay.test.ts | 81 ++++++++++--------- 5 files changed, 61 insertions(+), 51 deletions(-) diff --git a/docs-site/src/content/docs/reference/adapters.md b/docs-site/src/content/docs/reference/adapters.md index 78c347fe73d..5045e6624fa 100644 --- a/docs-site/src/content/docs/reference/adapters.md +++ b/docs-site/src/content/docs/reference/adapters.md @@ -559,10 +559,11 @@ configuration that names the old id is rewritten at startup. - Devin/Cognition API keys have no refresh endpoint. If Cognition rejects a stored key with 401, OpenCodex marks that account for reauthentication and can use another signed-in account for the turn. Run `ocx login devin` again for a revoked browser-login key. A CLI-imported account - can follow a later `devin auth login` key rotation when the CLI host and account identity - validate. A legacy `devin-cli` slot for that same account may already hold the new key; - a different account holding it blocks adoption. If the CLI file is temporarily unreadable or - the identity check is unavailable, retry after it recovers. A paused account stays paused + can follow a later `devin auth login` key rotation only when its stored account ID or email + matches the minted CLI identity. Imports without a stored identity require an explicit + `ocx login devin` after rotation. A legacy `devin-cli` slot for that same account may already + hold the new key; a different account holding it blocks adoption. If the CLI file is + temporarily unreadable or the identity check is unavailable, retry after it recovers. A paused account stays paused during this recovery and returns 403. - Only the credential is local when the CLI import path is used. The turn itself goes to Cognition either way, so the import and browser login paths differ in nothing but where the diff --git a/src/oauth/devin.ts b/src/oauth/devin.ts index 7343b50b34d..53c82d7da3a 100644 --- a/src/oauth/devin.ts +++ b/src/oauth/devin.ts @@ -279,10 +279,8 @@ export async function loginDevin( * not contradict the slot's recorded accountId or email, and no other stored * account may own the key or that identity. * - * A slot imported from the CLI records no identity (its token has none to - * give), so its first adoption rests on the last two rules alone: it is by - * definition "whatever the CLI is signed into", and the adopted credential - * records the minted identity, which makes every later adoption strict. + * A CLI-imported slot has no bound identity. It cannot adopt a changed key; + * explicit `ocx login devin` is required after its first rotation. */ /** An identity probe must not hold the per-account refresh lock for the mint's full 30s. */ const DEVIN_IDENTITY_MINT_TIMEOUT_MS = 5_000; @@ -333,6 +331,8 @@ async function rereadDevinCliCredential( // or briefly locked; like a failed identity mint, that says nothing about the key. if (outcome.kind === "unreadable" || outcome.kind === "incomplete") throw new DevinIdentityProbeUnavailableError(); if (outcome.kind !== "ok" || outcome.file.apiKey === stored.access) return undefined; + // Without a stored identity, the CLI file may now belong to another user. + if (!stored.accountId && !normalizedEmail(stored.email)?.includes("@")) return undefined; const apiBaseUrl = validateDevinApiBaseUrl(outcome.file.apiServerUrl); if (apiBaseUrl === undefined) return undefined; // A detached alias rekey can already hold this account's new key. Only a diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 4afe190ed93..8d4ab2b2f1d 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -91,11 +91,13 @@ proactive refresh, per-account quota probes (`accountQuotaProbeSkip` in Muse key-mint quota read, and xAI/Gemini web-search sidecar eligibility. The stored credential remains available for resume, while requests with no unpaused account fail with 403 rather than as a login failure. -Devin's local-CLI forced refresh validates the CLI tenant host before probing its key with a -bounded `GetUserJwt` call. It adopts a changed key only when the minted identity matches the -stored slot and the key and identity are unowned across Devin and alias accounts at the locked -store write. The same account id in a legacy alias slot is not a competing owner, -even when that alias already holds the rotated key; another account holding the key +Devin's local-CLI forced refresh requires a stored account ID or email before it can adopt a +changed CLI key. Identity-less imports take the terminal reauthentication path; they require +an explicit `ocx login devin` after rotation. For bound slots, it validates the CLI tenant host +and probes the key with a bounded `GetUserJwt` call. It adopts a changed key only when the minted +identity matches the stored slot and the key and identity are unowned across Devin and alias +accounts at the locked store write. The same account id in a legacy alias slot is not a +competing owner, even when that alias already holds the rotated key; another account holding the key still blocks adoption. The generation check still protects concurrent edits. A losing adoption or unreadable CLI file leaves the account unflagged; a paused account returns 403 with its stored key intact. The account actually sent supplies the generation fence; a rotated bearer always travels diff --git a/structure/transports/responses-failover.md b/structure/transports/responses-failover.md index 20ccc74f797..af32c1e9f93 100644 --- a/structure/transports/responses-failover.md +++ b/structure/transports/responses-failover.md @@ -249,7 +249,7 @@ the failure on the current target. ## runTurn pre-output 401 replay `src/server/responses/run-turn-execution.ts` handles a structured pre-output 401 for `isOAuth401ReplayProvider` runTurn routes with one generation-fenced forced refresh and an `auth-recovery` hop. A terminal refresh may admit a surviving account; otherwise the client gets the login instruction. Devin quota `permission_denied` (429) and plain permission denial (403) do not trigger this path. -An `oauth`-source Devin key rejected with 401 needs reauthentication because Cognition has no refresh endpoint. A `local-cli` slot can adopt a changed CLI key after host and bounded identity validation; [OAuth/Devin ownership](../providers-and-adapters.md) defines the locked store check across aliases. Unreadable files and transient probes leave the account unflagged. A pause during refresh returns 403 without retry, reauth, or replacing the stored key. Kiro's terminal alternate requires `OAuthLoginRequiredError`; transient refresh failures do not enter it. +An `oauth`-source Devin key rejected with 401 needs reauthentication because Cognition has no refresh endpoint. A `local-cli` slot with a stored account ID or email can adopt a changed CLI key after host and bounded identity validation; [OAuth/Devin ownership](../providers-and-adapters.md) defines the locked store check across aliases. An identity-less CLI import instead needs an explicit `ocx login devin` after rotation. Unreadable files and transient probes leave the account unflagged. A pause during refresh returns 403 without retry, reauth, or replacing the stored key. Kiro's terminal alternate requires `OAuthLoginRequiredError`; transient refresh failures do not enter it. Tests: `tests/responses/responses-devin-401-replay.test.ts` and `tests/server/server-kiro-refusal-e2e.test.ts`. ## Optional client transport hints diff --git a/tests/responses/responses-devin-401-replay.test.ts b/tests/responses/responses-devin-401-replay.test.ts index b7b3e42d8fe..6bbe4bd8877 100644 --- a/tests/responses/responses-devin-401-replay.test.ts +++ b/tests/responses/responses-devin-401-replay.test.ts @@ -188,27 +188,33 @@ test("a lone revoked account surfaces the login instruction", async () => { expect(sentKeys).toEqual([]); }); -test("a CLI-imported account adopts the key a later `devin auth login` wrote", async () => { +test("an identity-less CLI import rejects a rotated key and needs reauth", async () => { await saveCliImport(DEAD); writeCliFile(ROTATED); - const response = await run(); - - expect(response.status).toBe(200); - expect(await response.text()).toContain("served by rotated"); - expect(sentKeys).toEqual([DEAD, ROTATED]); + const body = await (await run()).json() as { error: { message: string } }; + expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); + expect(sentKeys).toEqual([DEAD]); + expect(mintCalls).toBe(0); const row = cliAccount(); - expect(row?.needsReauth).not.toBe(true); - expect(row?.credential.access).toBe(ROTATED); - // The minted identity is recorded, so the next adoption for this slot is strict. - expect(row?.credential.accountId).toBe("uid-rotated"); - expect(row?.credential.email).toBe("rotated@example.com"); - expect(row?.credential.source).toBe("local-cli"); - expect(row?.credential.expires).toBe(Number.MAX_SAFE_INTEGER); + expect(row?.needsReauth).toBe(true); + expect(row?.credential.access).toBe(DEAD); +}); + +test("a CLI file belonging to another user cannot replace a bound account", async () => { + await saveCliImport(DEAD, { accountId: "uid-original", email: "original@example.com" }); + writeCliFile(ROTATED); // Mints uid-rotated, a different Devin user. + + const body = await (await run()).json() as { error: { message: string } }; + expect(body.error.message).toBe("Not logged in to devin. Run: ocx login devin"); + expect(sentKeys).toEqual([DEAD]); + expect(mintCalls).toBe(1); + expect(cliAccount()?.needsReauth).toBe(true); + expect(cliAccount()?.credential.access).toBe(DEAD); }); test("a legacy alias copy of the same account already holding the rotated key does not block adoption", async () => { - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); const current = cliAccount()!; await replaceProviderAccountSet("devin-cli", { activeAccountId: current.id, @@ -246,7 +252,7 @@ test("a distinct legacy alias account still blocks an owned CLI identity", async access: LIVE, refresh: LIVE, expires: Number.MAX_SAFE_INTEGER, accountId: "uid-rotated", source: "oauth", apiBaseUrl: "https://server.codeium.com", }); - await saveCliImport(DEAD); + await saveCliImport(DEAD, { email: "rotated@example.com" }); writeCliFile(ROTATED); await (await run()).text(); @@ -274,7 +280,7 @@ test.each([ test("a CLI key another stored account already owns is not adopted", async () => { await saveDevin(ROTATED, "other"); - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); writeCliFile(ROTATED); await (await run()).text(); @@ -284,20 +290,20 @@ test("a CLI key another stored account already owns is not adopted", async () => }); test.each([ - ["a key Cognition refuses with 401", async () => { mintedIdentity = {}; await saveCliImport(DEAD); }], + ["a key Cognition refuses with 401", async () => { mintedIdentity = {}; await saveCliImport(DEAD, { accountId: "uid-rotated" }); }], ["a key Cognition refuses with 403", async () => { mintFailure = () => new Response("", { status: 403 }); - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); }], ["a minted token without auth_uid", async () => { mintedIdentity = { [ROTATED]: { email: "rotated@example.com" } }; - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); }], ["a slot whose recorded accountId differs", async () => { await saveCliImport(DEAD, { accountId: "uid-before" }); }], ["a slot whose recorded email differs", async () => { await saveCliImport(DEAD, { email: "before@example.com" }); }], ["an identity another stored account owns", async () => { await saveDevin(LIVE, "uid-rotated"); - await saveCliImport(DEAD); + await saveCliImport(DEAD, { email: "rotated@example.com" }); }], ])("the CLI key is not adopted for %s", async (_label, arrange) => { await arrange(); @@ -315,7 +321,7 @@ test.each([ ["a 429", () => new Response("", { status: 429 })], ["a network failure", () => { throw new TypeError("fetch failed"); }], ])("an identity probe that fails with %s does not flag the account", async (_label, failure) => { - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); writeCliFile(ROTATED); mintFailure = failure; @@ -333,7 +339,7 @@ test.each([ }); test("a credential file caught mid-write does not flag the account", async () => { - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); // Half-written by `devin auth login`: the key line is there, the server line is not yet. writeFileSync(home.path("devin-credentials.toml"), `windsurf_api_key = "${ROTATED}"\n`); @@ -350,33 +356,25 @@ test("a credential file caught mid-write does not flag the account", async () => expect(cliAccount()?.credential.access).toBe(ROTATED); }); -test("concurrent identity-less CLI slots cannot both adopt one rotated key", async () => { +test("concurrent identity-less CLI slots both require explicit reauth", async () => { await saveCliImport(DEAD); const first = await getValidAccessTokenSnapshot("devin"); await saveCliImport(DEAD_OTHER); const second = await getValidAccessTokenSnapshot("devin"); expect(first.accountId).not.toBe(second.accountId); writeCliFile(ROTATED); - const bothMinted = Promise.withResolvers(); - const releaseMint = Promise.withResolvers(); - holdMint = async () => { - if (mintCalls === 2) bothMinted.resolve(); - await releaseMint.promise; - }; - const refreshes = Promise.allSettled([ + const outcomes = await Promise.allSettled([ forceRefreshOAuthAccessSnapshot(first), forceRefreshOAuthAccessSnapshot(second), ]); - await bothMinted.promise; - releaseMint.resolve(); - const outcomes = await refreshes; const rows = getAccountSet("devin")!.accounts; - expect(outcomes.filter(outcome => outcome.status === "fulfilled")).toHaveLength(1); - expect(rows.filter(row => row.credential.access === ROTATED)).toHaveLength(1); - expect(rows.every(row => row.needsReauth !== true)).toBe(true); + expect(outcomes.every(outcome => outcome.status === "rejected")).toBe(true); + expect(rows.filter(row => row.credential.access === ROTATED)).toHaveLength(0); + expect(rows.every(row => row.needsReauth === true)).toBe(true); + expect(mintCalls).toBe(0); }); test.each([false, true])("an account paused during 401 refresh returns 403 (stream=%s)", async stream => { - await saveCliImport(DEAD); + await saveCliImport(DEAD, { accountId: "uid-rotated" }); const accountId = cliAccount()!.id; writeCliFile(ROTATED); const mintStarted = Promise.withResolvers(); @@ -414,6 +412,15 @@ test("a slot whose recorded identity matches adopts the rotated key", async () = expect(mintCalls).toBe(1); }); +test("a slot bound only by matching email adopts the rotated key", async () => { + await saveCliImport(DEAD, { email: " Rotated@Example.com " }); + writeCliFile(ROTATED); + + expect(await (await run()).text()).toContain("served by rotated"); + expect(cliAccount()?.credential.access).toBe(ROTATED); + expect(cliAccount()?.credential.accountId).toBe("uid-rotated"); +}); + test("a turn whose 401 lands after another turn already failed the account over still fails over", async () => { await saveDevin(LIVE, "spare"); await saveDevin(DEAD, "revoked");