From 697946381d571b410810f8d107819ec4fc492ccc Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 30 May 2026 21:18:33 -0300 Subject: [PATCH] fix(auth): prevent Codex multi-account refresh_token family revocation (#2941) Integrated into release/v3.8.8 --- open-sse/handlers/chatCore.ts | 32 ++++++- open-sse/services/refreshSerializer.ts | 107 +++++++++++++++++++++++ open-sse/services/tokenRefresh.ts | 12 ++- src/app/api/providers/[id]/test/route.ts | 23 ++++- tests/unit/refresh-serializer.test.ts | 107 +++++++++++++++++++++++ 5 files changed, 275 insertions(+), 6 deletions(-) create mode 100644 open-sse/services/refreshSerializer.ts create mode 100644 tests/unit/refresh-serializer.test.ts diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 109930bf86..f01a6946e5 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -59,7 +59,8 @@ import { PROVIDER_ERROR_TYPES, isEmptyContentResponse, } from "../services/errorClassifier.ts"; -import { updateProviderConnection } from "@/lib/db/providers"; +import { updateProviderConnection, getProviderConnectionById } from "@/lib/db/providers"; +import { wasRefreshTokenRotated } from "@omniroute/open-sse/services/refreshSerializer.ts"; import { recordKeyFailure, recordKeySuccess, @@ -4323,6 +4324,11 @@ export async function handleChatCore({ // inside getAccessToken, we still need to do the credentials mutation + user // callback after refreshCredentials returns. The `persistFnRan` flag tracks // which path executed so we don't double-fire (race-prone) or skip (regression). + // Front 3: remember the refresh_token we are about to present so that, if the + // refresh fails as unrecoverable, we can tell a genuine death apart from a + // stale-token reuse that a concurrent/sibling refresh already rotated past. + const attemptedRefreshToken = + typeof credentials?.refreshToken === "string" ? credentials.refreshToken : null; let persistFnRan = false; const persistFn = onCredentialsRefreshed ? async (refreshResult: Record) => { @@ -4405,7 +4411,29 @@ export async function handleChatCore({ } else { log?.warn?.("TOKEN", `${provider.toUpperCase()} | refresh failed`); if (isUnrecoverableRefreshError(newCredentials) && onCredentialsRefreshed) { - await onCredentialsRefreshed({ testStatus: "expired", isActive: false }); + // Front 3 (reuse-race tolerance): before deactivating, re-read the DB. + // If a sibling/concurrent refresh already rotated this connection's + // refresh_token (common for Codex/OpenAI under one shared Auth0 client), + // the failure we saw was a stale-token reuse — the account is healthy + // with the newer token, so keep it active instead of killing it. + let alreadyRotated = false; + if (typeof connectionId === "string" && connectionId && attemptedRefreshToken) { + try { + const latest = await getProviderConnectionById(connectionId); + if (wasRefreshTokenRotated(attemptedRefreshToken, latest?.refreshToken)) { + alreadyRotated = true; + log?.warn?.( + "TOKEN", + `${provider.toUpperCase()} | refresh_token already rotated by a concurrent refresh — keeping connection active` + ); + } + } catch { + // DB read failed — fall through to the safe default (deactivate). + } + } + if (!alreadyRotated) { + await onCredentialsRefreshed({ testStatus: "expired", isActive: false }); + } } } } diff --git a/open-sse/services/refreshSerializer.ts b/open-sse/services/refreshSerializer.ts new file mode 100644 index 0000000000..10d044de9a --- /dev/null +++ b/open-sse/services/refreshSerializer.ts @@ -0,0 +1,107 @@ +/** + * Global OAuth refresh serialization, keyed by rotation group. + * + * Why this exists (Front 1 of the Codex multi-account cascade fix): + * Providers that share a single Auth0 client_id — notably OpenAI Codex and the + * `openai` provider — enforce "single active session per client_id". When two + * *sibling* accounts under that client refresh their `refresh_token` at nearly + * the same time, Auth0 treats it as token reuse and revokes the WHOLE + * refresh_token family, so previously-healthy accounts suddenly fail with + * `refresh_token_invalidated` / `refresh_token_reused` (openai/codex#9648). + * + * The per-connection mutex in tokenRefresh.ts does NOT help: the colliding + * refreshes happen on DIFFERENT connections. This serializer forces the actual + * network refresh to concurrency=1 across every connection in a rotation group, + * so two siblings never POST to /oauth/token concurrently. Optional spacing + * (CODEX_REFRESH_SPACING_MS) inserts a small gap between consecutive refreshes + * in a group for extra safety. Non-rotating providers (Google, etc.) are not + * serialized — their refresh_tokens are permanent and there is no cascade. + */ + +// Providers mapped to the same string share one serialized lane. Codex and the +// raw `openai` provider use the same Auth0 backend, so they MUST share a lane. +const ROTATION_LOCK_GROUP: Record = { + codex: "openai-auth0", + openai: "openai-auth0", + claude: "anthropic-oauth", + "gitlab-duo": "gitlab-duo", + kiro: "kiro", + "kimi-coding": "kimi-coding", + qwen: "qwen", +}; + +function readSpacingMs(): number { + const raw = Number(process.env.CODEX_REFRESH_SPACING_MS); + return Number.isFinite(raw) && raw >= 0 ? raw : 0; +} + +// Tail promise per group — each new refresh chains after the previous one. +const groupTail = new Map>(); + +const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +/** Returns the serialization group for a provider, or null when it is not a rotating provider. */ +export function rotationGroupFor(provider: string): string | null { + return ROTATION_LOCK_GROUP[provider] ?? null; +} + +/** + * Run `fn` (the actual network refresh) serialized against every other refresh + * in the same rotation group. Different groups run concurrently; non-rotating + * providers run immediately with no locking. + */ +export async function serializeRefresh(provider: string, fn: () => Promise): Promise { + const group = rotationGroupFor(provider); + if (!group) return fn(); + + const prevTail = groupTail.get(group) ?? Promise.resolve(); + let releaseMine!: () => void; + const mine = new Promise((resolve) => { + releaseMine = resolve; + }); + const myTail = prevTail.then(() => mine); + groupTail.set(group, myTail); + + // Wait for our turn. Ignore a predecessor's rejection — its `finally` still + // releases the lane, so the queue keeps flowing even after a failed refresh. + await prevTail.catch(() => {}); + + try { + return await fn(); + } finally { + const spacing = readSpacingMs(); + if (spacing > 0) await delay(spacing); + releaseMine(); + // Garbage-collect the lane when nobody chained after us. + if (groupTail.get(group) === myTail) groupTail.delete(group); + } +} + +/** + * Front 3 (reuse-race tolerance): decide whether an unrecoverable refresh failure + * (`refresh_token_invalidated` / `refresh_token_reused`) should be IGNORED because + * a concurrent or sibling refresh already rotated this connection's refresh_token. + * + * If the DB now holds a different, non-empty refresh_token than the one we + * presented, the failure was a stale-token reuse and the connection is actually + * healthy with the newer token — so it must stay active instead of being + * deactivated. Mirrors the health-check's `credentialsChangedSinceSweep` guard + * and codex-lb's replica race-detection. + */ +export function wasRefreshTokenRotated( + attemptedRefreshToken: string | null | undefined, + latestRefreshToken: string | null | undefined +): boolean { + return ( + typeof attemptedRefreshToken === "string" && + attemptedRefreshToken.length > 0 && + typeof latestRefreshToken === "string" && + latestRefreshToken.length > 0 && + latestRefreshToken !== attemptedRefreshToken + ); +} + +/** Test-only: clear all in-flight lanes between tests. */ +export function __resetRefreshSerializerForTest(): void { + groupTail.clear(); +} diff --git a/open-sse/services/tokenRefresh.ts b/open-sse/services/tokenRefresh.ts index ae7ea2e6a1..4abb7acbee 100755 --- a/open-sse/services/tokenRefresh.ts +++ b/open-sse/services/tokenRefresh.ts @@ -4,6 +4,7 @@ import { PROVIDERS, OAUTH_ENDPOINTS } from "../config/constants.ts"; import { getGitHubCopilotRefreshHeaders } from "../config/providerHeaderProfiles.ts"; import { pbkdf2Sync } from "node:crypto"; import { runWithProxyContext } from "../utils/proxyFetch.ts"; +import { serializeRefresh } from "./refreshSerializer.ts"; import { WINDSURF_CONFIG } from "@/lib/oauth/constants/oauth"; import { buildGitLabOAuthEndpoints, resolveGitLabOAuthBaseUrl } from "@/lib/oauth/gitlab"; @@ -1500,7 +1501,9 @@ export async function getAccessToken( // the legacy `connectionId`-less path would silently swallow the callback, // leaving DB rows out of sync with rotated tokens (Codex/OpenAI). We still // resolve the promise to all waiters with the refreshed credentials. - const refreshPromise = _getAccessTokenInternal(provider, credentials, log, proxyConfig) + const refreshPromise = serializeRefresh(provider, () => + _getAccessTokenInternal(provider, credentials, log, proxyConfig) + ) .then(async (result) => { if (result?.accessToken && effectiveOnPersist) { try { @@ -1604,7 +1607,12 @@ async function _getAccessTokenWithStalenessCheck(provider, credentials, log, pro } const oldRefreshToken = credentials.refreshToken; - const result = await _getAccessTokenInternal(provider, credentials, log, proxyConfig); + // Front 1: serialize the network refresh across all connections of the same + // rotation group (e.g. Codex+openai share one Auth0 client) so two sibling + // accounts never refresh concurrently and trip Auth0 family revocation. + const result = await serializeRefresh(provider, () => + _getAccessTokenInternal(provider, credentials, log, proxyConfig) + ); // Record the rotation so subsequent stale callers can be redirected to the // new tokens without re-hitting upstream (which would trigger Auth0 family diff --git a/src/app/api/providers/[id]/test/route.ts b/src/app/api/providers/[id]/test/route.ts index d766925886..99a49736cc 100644 --- a/src/app/api/providers/[id]/test/route.ts +++ b/src/app/api/providers/[id]/test/route.ts @@ -13,6 +13,7 @@ import { validateProviderApiKey } from "@/lib/providers/validation"; import { getCliRuntimeStatus } from "@/shared/services/cliRuntime"; // Use the shared open-sse token refresh with built-in dedup/race-condition cache import { getAccessToken } from "@omniroute/open-sse/services/tokenRefresh.ts"; +import { rotationGroupFor } from "@omniroute/open-sse/services/refreshSerializer.ts"; import { saveCallLog } from "@/lib/usageDb"; import { logProxyEvent } from "@/lib/proxyLogger"; import { runWithProxyContext } from "@omniroute/open-sse/utils/proxyFetch.ts"; @@ -420,9 +421,14 @@ async function testOAuthConnection(connection: any) { let refreshed = false; let newTokens = null; - // Auto-refresh if token is expired and provider supports refresh + // Auto-refresh if token is expired and provider supports refresh. + // Front 2: NEVER burn a rotating provider's single-use refresh_token from a + // connection test. Under a shared Auth0 client (Codex/OpenAI) a test-time + // refresh can cascade-invalidate sibling accounts' refresh_token families + // (openai/codex#9648). Leave rotation to the reactive, mutex-guarded 401 path. const tokenExpired = isTokenExpired(connection); - if (config.refreshable && tokenExpired && connection.refreshToken) { + const isRotatingProvider = rotationGroupFor(connection.provider) !== null; + if (config.refreshable && tokenExpired && connection.refreshToken && !isRotatingProvider) { const tokens = await refreshOAuthToken(connection); if (tokens) { accessToken = tokens.accessToken; @@ -454,6 +460,19 @@ async function testOAuthConnection(connection: any) { } // Check if token is expired (no refresh available) if (tokenExpired) { + // Front 2: for rotating providers we intentionally did NOT refresh above. + // An expired access_token here is recoverable on next real use via the + // reactive 401 path, so don't report the account as broken (which would + // tempt the operator to re-test and never resolve). Keep it active. + if (isRotatingProvider && connection.refreshToken) { + return { + valid: true, + error: null, + refreshed: false, + newTokens: null, + diagnosis: makeDiagnosis("ok", "oauth", null, null), + }; + } const error = "Token expired"; return { valid: false, diff --git a/tests/unit/refresh-serializer.test.ts b/tests/unit/refresh-serializer.test.ts new file mode 100644 index 0000000000..305c4e6e5d --- /dev/null +++ b/tests/unit/refresh-serializer.test.ts @@ -0,0 +1,107 @@ +/** + * Front 1 (Codex multi-account cascade) — global refresh serialization. + * + * Providers under the same Auth0 client_id (OpenAI Codex + openai) get the WHOLE + * refresh_token family revoked when two sibling accounts refresh concurrently + * (openai/codex#9648). The per-connection mutex does NOT help here — the colliding + * refreshes are on DIFFERENT connections. `serializeRefresh` forces concurrency=1 + * across every connection in a rotation group, so two siblings never hit Auth0's + * /oauth/token at the same time. Non-rotating providers are left untouched. + * + * Evidence: VM 192.168.0.15 logs showed bursts of 5-6 Codex refreshes within + * ~14s whenever a batch of accounts was added — exactly the cascade trigger. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { + serializeRefresh, + rotationGroupFor, + wasRefreshTokenRotated, + __resetRefreshSerializerForTest, +} from "../../open-sse/services/refreshSerializer.ts"; + +const delay = (ms: number) => new Promise((r) => setTimeout(r, ms)); + +test("codex and openai share one group and never refresh concurrently", async () => { + __resetRefreshSerializerForTest(); + let active = 0; + let maxActive = 0; + const order: string[] = []; + + const task = (label: string, provider: string, ms: number) => + serializeRefresh(provider, async () => { + active++; + maxActive = Math.max(maxActive, active); + await delay(ms); + order.push(label); + active--; + return label; + }); + + // codex + openai = same Auth0 client → must serialize together. + await Promise.all([task("a", "codex", 30), task("b", "openai", 10), task("c", "codex", 10)]); + + assert.equal(maxActive, 1, "no two refreshes in the openai-auth0 group may overlap"); + assert.equal(order.length, 3); +}); + +test("different rotation groups run concurrently (not serialized against each other)", async () => { + __resetRefreshSerializerForTest(); + let active = 0; + let maxActive = 0; + const run = (provider: string) => + serializeRefresh(provider, async () => { + active++; + maxActive = Math.max(maxActive, active); + await delay(20); + active--; + }); + + await Promise.all([run("codex"), run("claude"), run("kiro")]); + assert.equal(maxActive, 3, "codex / claude / kiro are independent groups"); +}); + +test("non-rotating providers are not serialized", async () => { + __resetRefreshSerializerForTest(); + let active = 0; + let maxActive = 0; + const run = (provider: string) => + serializeRefresh(provider, async () => { + active++; + maxActive = Math.max(maxActive, active); + await delay(20); + active--; + }); + + await Promise.all([run("gemini-cli"), run("gemini-cli"), run("antigravity")]); + assert.equal(maxActive, 3, "non-rotating providers must keep running in parallel"); +}); + +test("an error in one refresh does not deadlock the group queue", async () => { + __resetRefreshSerializerForTest(); + await assert.rejects( + serializeRefresh("codex", async () => { + throw new Error("boom"); + }), + /boom/ + ); + const recovered = await serializeRefresh("codex", async () => "ok"); + assert.equal(recovered, "ok", "queue must keep flowing after a failed refresh"); +}); + +test("rotationGroupFor maps the OpenAI Auth0 family together", () => { + assert.equal(rotationGroupFor("codex"), rotationGroupFor("openai")); + assert.notEqual(rotationGroupFor("codex"), rotationGroupFor("claude")); + assert.equal(rotationGroupFor("gemini-cli"), null); +}); + +// Front 3 — reuse-race tolerance: only keep a connection active after an +// unrecoverable refresh failure when the DB token actually changed under us. +test("wasRefreshTokenRotated: true only when the DB token changed to a non-empty value", () => { + assert.equal(wasRefreshTokenRotated("rt-old", "rt-new"), true, "a sibling rotated it"); + assert.equal(wasRefreshTokenRotated("rt-old", "rt-old"), false, "same token = genuinely dead"); + assert.equal(wasRefreshTokenRotated("rt-old", null), false, "no DB token = deactivate"); + assert.equal(wasRefreshTokenRotated("rt-old", ""), false, "empty DB token = deactivate"); + assert.equal(wasRefreshTokenRotated(null, "rt-new"), false, "unknown attempted = deactivate"); + assert.equal(wasRefreshTokenRotated(undefined, undefined), false); +});