diff --git a/src/codex/account-store.ts b/src/codex/account-store.ts index 73658a6078..93a220fedb 100644 --- a/src/codex/account-store.ts +++ b/src/codex/account-store.ts @@ -229,6 +229,90 @@ export function tombstoneCodexAccount(id: string): number { }); } +/** One alias whose stored refresh grant was replaced by a rotation it did not drive. */ +export type RotatedGrantFanoutResult = { + id: string; + fromGeneration: number; + toGeneration: number; + /** The access token that alias holds after the merge; used for plan reconciliation. */ + accessToken: string; + /** + * `full` replaced the whole credential; `grant-only` spliced just the refresh grant + * onto an access credential someone else had already made newer. + */ + mode: "full" | "grant-only"; +}; + +/** + * Propagate a rotated refresh grant to same-account aliases that still hold the OLD grant. + * + * Upstream rotates the refresh grant for the ACCOUNT, not for the alias that happened to + * drive the refresh. Only the flight owner and the aliases actively waiting on it learn + * about the rotation, so an alias that was idle keeps a grant upstream has already + * invalidated: its next refresh returns `invalid_grant` and the alias is retired even + * though the account is healthy (#2892 gap 3). + * + * Membership is `(exact non-empty chatgptAccountId, exact old grant fingerprint)`. The + * alias STRING is display-only and never establishes identity, and generations are + * per-alias: a generation captured for one alias is meaningless for another and is never + * compared or assigned across them. + * + * Everything happens under one credential mutation lock with a single persist, so a + * partial write cannot leave some aliases on a dead grant. Aliases that were deleted, + * changed identity, or already moved off the old grant are skipped rather than forced. + */ +export function fanOutRotatedRefreshGrant(options: { + excludeId: string; + chatgptAccountId: string; + previousRefreshGrantFingerprint: string; + rotated: CodexAccountCredentials; +}): RotatedGrantFanoutResult[] { + const { excludeId, chatgptAccountId, previousRefreshGrantFingerprint, rotated } = options; + // Fail closed: without an exact upstream identity there is no safe membership test, and + // a rotation that did not actually change the grant has nothing to propagate. + if (!chatgptAccountId || !previousRefreshGrantFingerprint) return []; + const rotatedFingerprint = refreshGrantFingerprintForToken(rotated.refreshToken); + if (rotatedFingerprint === previousRefreshGrantFingerprint) return []; + + return withCredentialMutationLockSync(() => { + const store = loadCodexAccountRecordStore(); + const applied: RotatedGrantFanoutResult[] = []; + for (const [candidateId, candidate] of Object.entries(store)) { + if (candidateId === excludeId) continue; + if (candidate.deletedAt != null || !candidate.credential) continue; + if (candidate.credential.chatgptAccountId !== chatgptAccountId) continue; + if (recordGrantFingerprint(candidate) !== previousRefreshGrantFingerprint) continue; + + const fromGeneration = candidate.generation; + // An alias whose access credential is already newer than the rotation must keep it: + // overwriting would retire a credential someone else just committed. Only the dead + // grant is replaced, so the alias can still refresh when its own token expires. + const keepsOwnAccess = candidate.credential.expiresAt > rotated.expiresAt; + const merged: CodexAccountCredentials = keepsOwnAccess + ? { ...candidate.credential, refreshToken: rotated.refreshToken } + : { ...rotated }; + store[candidateId] = { + credential: merged, + generation: fromGeneration + 1, + refreshGrantFingerprint: rotatedFingerprint, + replacedAt: candidate.replacedAt, + ...preservedValidationMetadata(candidate), + }; + applied.push({ + id: candidateId, + fromGeneration, + toGeneration: fromGeneration + 1, + accessToken: merged.accessToken, + mode: keepsOwnAccess ? "grant-only" : "full", + }); + } + // One write for the whole fan-out: a crash between aliases must not leave part of the + // account on an invalidated grant. + if (applied.length > 0) persist(store); + return applied; + }); +} + const CHATGPT_TOKEN_URL = "https://auth.openai.com/oauth/token"; const CHATGPT_CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann"; @@ -304,6 +388,12 @@ type CodexRefreshResult = CodexTokenResult & { * refresh of the one the caller was holding, not somebody else's replacement. */ selfRefreshed?: boolean; + /** + * Same-account aliases that were carried onto the rotated grant by this flight. + * Reported so plan reconciliation runs against each alias's own token and generation; + * never used to establish this caller's own lineage. + */ + fannedOutAliases?: RotatedGrantFanoutResult[]; }; const MAX_CODEX_REFRESH_FLIGHTS = 32; const CODEX_REFRESH_FLIGHT_STALE_MS = 120_000; @@ -393,13 +483,20 @@ export async function withCodexRefreshFileLock(lockKey: string, signal: Abort function findFreshCredentialForGrant( refreshGrantFingerprint: string, excludeId: string, + chatgptAccountId: string, rejectedAccessToken?: string, ): CodexAccountCredentials | null { const now = Date.now(); + // A grant fingerprint alone does not establish WHOSE credential this is. Adopting a + // sibling record that shares the grant but reports a different upstream identity would + // silently send one account's requests under another account's bearer, so identity is + // required and an empty identity fails closed on both sides. + if (!chatgptAccountId) return null; const records = loadCodexAccountRecordStore(); for (const [candidateId, candidate] of Object.entries(records)) { if (candidateId === excludeId || candidate.deletedAt != null || !candidate.credential) continue; if (recordGrantFingerprint(candidate) !== refreshGrantFingerprint) continue; + if (candidate.credential.chatgptAccountId !== chatgptAccountId) continue; // A sibling alias can hold a still-unexpired copy of the exact token upstream // just rejected. Reusing it would bump the generation and replay the identical // bearer — a second 401 dressed up as recovery. @@ -638,7 +735,7 @@ async function resolveCodexToken( const abort = new AbortController(); const signal = AbortSignal.any([abort.signal, AbortSignal.timeout(30_000)]); let flight!: RefreshFlight; - const refreshPromise = withCodexRefreshFileLock(refreshGrantFingerprint, signal, async (): Promise => { + const fetchPromise = withCodexRefreshFileLock(refreshGrantFingerprint, signal, async (): Promise => { const current = readCodexAccountRecord(id); const lockedRecord = readCodexAccountRecord(id); const lockedCred = lockedRecord?.deletedAt == null ? lockedRecord?.credential : undefined; @@ -678,6 +775,7 @@ async function resolveCodexToken( const sameGrantFreshCredential = findFreshCredentialForGrant( refreshGrantFingerprint, id, + lockedCred.chatgptAccountId, forced?.rejectedAccessToken, ); if (sameGrantFreshCredential) { @@ -748,6 +846,15 @@ async function resolveCodexToken( if (!saveCodexAccountCredentialIfGeneration(id, startGeneration, updated)) { throw new CodexCredentialGenerationConflictError(); } + // The grant belongs to the ACCOUNT. Aliases that were idle during this flight still + // hold the grant upstream has now invalidated, so carry the rotation to them under + // their own generations before anyone tries to refresh with a dead token. + const fannedOut = fanOutRotatedRefreshGrant({ + excludeId: id, + chatgptAccountId: updated.chatgptAccountId, + previousRefreshGrantFingerprint: refreshGrantFingerprint, + rotated: updated, + }); return { accessToken: updated.accessToken, chatgptAccountId: updated.chatgptAccountId, @@ -758,7 +865,30 @@ async function resolveCodexToken( // token — tagging the new grant would make every legitimate joiner look foreign. resolvedGrantFingerprint: refreshGrantFingerprint, selfRefreshed: true, + ...(fannedOut.length > 0 ? { fannedOutAliases: fannedOut } : {}), }; + }); + /* + * Plan reconciliation belongs to the FLIGHT, not to whichever caller opened it. + * + * The flight outlives its initiating caller by design (gap 2): an aborted owner stops + * waiting while the shared work still runs and still commits the rotated credential. + * Reconciling the plan only after the owner's caller-scoped wait therefore dropped it + * whenever that owner walked away, and a same-account joiner returning through the + * adopt-stored branch does not reconcile either — so a changed `chatgpt_plan_type` + * stayed invisible in `codexAccounts[].plan` for the life of the process and skewed + * plan-selected quota projection. Attaching it to the flight runs it exactly once per + * committed result, for every waiter, including none. + */ + const refreshPromise = fetchPromise.then(async (result): Promise => { + await notePlanFromRefreshedAccessToken(id, result.accessToken, result.generation); + // Each alias carried onto the rotated grant reconciles under ITS OWN token and its + // own generation. Generations are per-alias, so the owner's value must never be + // reused here. + for (const alias of result.fannedOutAliases ?? []) { + await notePlanFromRefreshedAccessToken(alias.id, alias.accessToken, alias.toGeneration); + } + return result; }).finally(() => { if (refreshLocks.get(refreshGrantFingerprint) === flight) refreshLocks.delete(refreshGrantFingerprint); }); @@ -769,7 +899,6 @@ async function resolveCodexToken( // registered, so a joiner that arrives after this caller walks away still receives // the committed result. const result = await awaitOwnCancellation(refreshPromise, callerSignal); - await notePlanFromRefreshedAccessToken(id, result.accessToken, result.generation); return { accessToken: result.accessToken, chatgptAccountId: result.chatgptAccountId, diff --git a/tests/codex-account-store.test.ts b/tests/codex-account-store.test.ts index 760cd610ab..bd963dadd8 100644 --- a/tests/codex-account-store.test.ts +++ b/tests/codex-account-store.test.ts @@ -16,6 +16,17 @@ function refreshLockPathForToken(refreshToken: string): string { return join(TEST_DIR, `codex-refresh-${digest}.lock`); } +/** Minimal unsigned JWT carrying the plan claim the store reconciles from. */ +function planJwt(plan: string, accountId = "acct-plan-flight"): string { + const header = Buffer.from(JSON.stringify({ alg: "none" })).toString("base64url"); + const body = Buffer.from(JSON.stringify({ + chatgpt_account_id: accountId, + chatgpt_plan_type: plan, + "https://api.openai.com/auth": { chatgpt_account_id: accountId, chatgpt_plan_type: plan }, + })).toString("base64url"); + return `${header}.${body}.sig`; +} + describe("codex-account-store CRUD", () => { beforeEach(() => { // These exercises cover credential-store contention, not Windows ACL behavior. @@ -995,3 +1006,350 @@ describe("codex-account-store CRUD", () => { } }); }); + +describe("shared refresh flight plan reconciliation (#2892 gap 2 follow-up)", () => { + beforeEach(() => { + setIcaclsRunnerForTests(() => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + process.env.OPENCODEX_HOME = TEST_DIR; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + }); + + afterEach(() => { + setIcaclsRunnerForTests(null); + delete process.env.OPENCODEX_HOME; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + }); + + test("an aborted owner still reconciles the refreshed plan for the shared flight", async () => { + // The flight deliberately outlives the caller that opened it, so plan reconciliation + // must not hang off that caller's wait: a rotated token carrying a NEW + // chatgpt_plan_type would otherwise commit while codexAccounts[].plan stayed stale + // for the rest of the process, skewing plan-selected quota projection. + const { forceRefreshCodexPoolToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + const { loadConfig, saveConfig } = await import("../src/config"); + const { resetJwtPlanNotesForTests } = await import("../src/codex/plan-from-token"); + resetJwtPlanNotesForTests(); + + saveConfig({ + port: 10199, + providers: {}, + defaultProvider: "openai", + codexAccounts: [{ id: "plan-flight", email: "flight@example.test", plan: "plus", isMain: false }], + }); + saveCodexAccountCredential("plan-flight", { + accessToken: planJwt("plus"), + refreshToken: "plan-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-plan-flight", + }); + const generation = readCodexAccountRecord("plan-flight")!.generation; + + const originalFetch = globalThis.fetch; + let releaseFetch: (() => void) | undefined; + const fetchStarted = new Promise(resolve => { + globalThis.fetch = (async () => { + resolve(); + await new Promise(release => { releaseFetch = release; }); + return Response.json({ + access_token: planJwt("pro"), + refresh_token: "plan-grant2", + expires_in: 3600, + }); + }) as typeof fetch; + }); + + try { + const owner = new AbortController(); + const ownerCall = forceRefreshCodexPoolToken("plan-flight", { + rejectedGeneration: generation, + rejectedAccessToken: planJwt("plus"), + signal: owner.signal, + }); + await fetchStarted; + owner.abort(new Error("client disconnected")); + await expect(ownerCall).rejects.toThrow("client disconnected"); + + releaseFetch?.(); + // The flight is detached from every caller now, so there is nothing to await. Poll + // for the persisted outcome under a deadline instead of a fixed delay: a fixed + // sleep can pass before the flight commits on a loaded worker and let teardown race + // unfinished work, and it never proves the reconciliation actually ran. + const deadline = Date.now() + 5_000; + let persisted = loadConfig().codexAccounts?.[0]; + while ((persisted?.plan !== "pro" || persisted?.planSource !== "jwt") && Date.now() < deadline) { + await Bun.sleep(10); + persisted = loadConfig().codexAccounts?.[0]; + } + + expect(persisted?.plan).toBe("pro"); + expect(persisted?.planSource).toBe("jwt"); + expect(readCodexAccountRecord("plan-flight")!.credential!.accessToken).toBe(planJwt("pro")); + } finally { + globalThis.fetch = originalFetch; + resetJwtPlanNotesForTests(); + } + }); +}); + +describe("rotated refresh grant fan-out (#2892 gap 3)", () => { + beforeEach(() => { + setIcaclsRunnerForTests(() => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + process.env.OPENCODEX_HOME = TEST_DIR; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + }); + + afterEach(() => { + setIcaclsRunnerForTests(null); + delete process.env.OPENCODEX_HOME; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + }); + + async function refreshOwnerWithIdleSibling(siblingExpiresAt: number): Promise<{ + sibling: ReturnType; + siblingBefore: number; + }> { + const { forceRefreshCodexPoolToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("owner-alias", { + accessToken: "rejected", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("idle-alias", { + accessToken: "idle-access", + refreshToken: "shared-grant", + expiresAt: siblingExpiresAt, + chatgptAccountId: "acct-shared", + }); + const generation = readCodexAccountRecord("owner-alias")!.generation; + const siblingBefore = readCodexAccountRecord("idle-alias")!.generation; + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => Response.json({ + access_token: "rotated-access", + refresh_token: "rotated-grant", + expires_in: 3600, + })) as typeof fetch; + try { + await forceRefreshCodexPoolToken("owner-alias", { + rejectedGeneration: generation, + rejectedAccessToken: "rejected", + }); + } finally { + globalThis.fetch = originalFetch; + } + return { sibling: readCodexAccountRecord("idle-alias"), siblingBefore }; + } + + test("an idle same-account alias is carried onto the rotated grant", async () => { + // Upstream rotates the grant for the ACCOUNT. Without fan-out the idle alias keeps a + // grant upstream has already invalidated and is retired on its next refresh even + // though the account is healthy. + const { sibling, siblingBefore } = await refreshOwnerWithIdleSibling(Date.now() + 60_000); + expect(sibling!.credential!.refreshToken).toBe("rotated-grant"); + expect(sibling!.credential!.accessToken).toBe("rotated-access"); + expect(sibling!.generation).toBe(siblingBefore + 1); + }); + + test("an alias holding a newer access credential keeps it and takes only the grant", async () => { + // Exercised directly: reaching this state through a live flight is not possible, + // because a same-identity alias holding a fresh non-rejected credential is adopted + // before any fetch. It survives only as the concurrent-writer race the merge exists + // to lose safely, so the contract is asserted against the exported helper rather + // than dressed up as an interleaving the code cannot actually produce. + const { fanOutRotatedRefreshGrant, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("owner-alias", { + accessToken: "owner-access", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("newer-alias", { + accessToken: "newer-access", + refreshToken: "shared-grant", + expiresAt: Date.now() + 30 * 3600_000, + chatgptAccountId: "acct-shared", + }); + const before = readCodexAccountRecord("newer-alias")!; + + const applied = fanOutRotatedRefreshGrant({ + excludeId: "owner-alias", + chatgptAccountId: "acct-shared", + previousRefreshGrantFingerprint: refreshGrantFingerprint("shared-grant"), + rotated: { + accessToken: "rotated-access", + refreshToken: "rotated-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }, + }); + + expect(applied).toEqual([{ + id: "newer-alias", + fromGeneration: before.generation, + toGeneration: before.generation + 1, + accessToken: "newer-access", + mode: "grant-only", + }]); + const after = readCodexAccountRecord("newer-alias")!; + expect(after.credential!.accessToken).toBe("newer-access"); + expect(after.credential!.refreshToken).toBe("rotated-grant"); + expect(after.generation).toBe(before.generation + 1); + }); + + test("a different upstream identity sharing the grant fingerprint is never touched", async () => { + const { forceRefreshCodexPoolToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("owner-alias", { + accessToken: "rejected", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("foreign-alias", { + accessToken: "foreign-access", + refreshToken: "shared-grant", + expiresAt: Date.now() + 60_000, + chatgptAccountId: "acct-other", + }); + const generation = readCodexAccountRecord("owner-alias")!.generation; + const foreignBefore = readCodexAccountRecord("foreign-alias")!; + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => Response.json({ + access_token: "rotated-access", + refresh_token: "rotated-grant", + expires_in: 3600, + })) as typeof fetch; + try { + await forceRefreshCodexPoolToken("owner-alias", { + rejectedGeneration: generation, + rejectedAccessToken: "rejected", + }); + } finally { + globalThis.fetch = originalFetch; + } + + const foreignAfter = readCodexAccountRecord("foreign-alias")!; + expect(foreignAfter.credential!.accessToken).toBe("foreign-access"); + expect(foreignAfter.credential!.refreshToken).toBe("shared-grant"); + expect(foreignAfter.generation).toBe(foreignBefore.generation); + }); + + test("a sibling that already moved to another grant is left alone", async () => { + const { forceRefreshCodexPoolToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("owner-alias", { + accessToken: "rejected", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("moved-alias", { + accessToken: "moved-access", + refreshToken: "some-other-grant", + expiresAt: Date.now() + 60_000, + chatgptAccountId: "acct-shared", + }); + const generation = readCodexAccountRecord("owner-alias")!.generation; + const movedBefore = readCodexAccountRecord("moved-alias")!; + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => Response.json({ + access_token: "rotated-access", + refresh_token: "rotated-grant", + expires_in: 3600, + })) as typeof fetch; + try { + await forceRefreshCodexPoolToken("owner-alias", { + rejectedGeneration: generation, + rejectedAccessToken: "rejected", + }); + } finally { + globalThis.fetch = originalFetch; + } + + const movedAfter = readCodexAccountRecord("moved-alias")!; + expect(movedAfter.credential!.refreshToken).toBe("some-other-grant"); + expect(movedAfter.generation).toBe(movedBefore.generation); + }); + + test("a refresh that does NOT rotate the grant writes nothing to siblings", async () => { + const { forceRefreshCodexPoolToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("owner-alias", { + accessToken: "rejected", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("idle-alias", { + accessToken: "idle-access", + refreshToken: "shared-grant", + expiresAt: Date.now() + 60_000, + chatgptAccountId: "acct-shared", + }); + const generation = readCodexAccountRecord("owner-alias")!.generation; + const idleBefore = readCodexAccountRecord("idle-alias")!; + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => Response.json({ + access_token: "rotated-access", + refresh_token: "shared-grant", + expires_in: 3600, + })) as typeof fetch; + try { + await forceRefreshCodexPoolToken("owner-alias", { + rejectedGeneration: generation, + rejectedAccessToken: "rejected", + }); + } finally { + globalThis.fetch = originalFetch; + } + + const idleAfter = readCodexAccountRecord("idle-alias")!; + expect(idleAfter.generation).toBe(idleBefore.generation); + expect(idleAfter.credential!.accessToken).toBe("idle-access"); + }); + + test("a sibling adoption requires the same upstream identity, not just the grant", async () => { + // findFreshCredentialForGrant can hand a sibling's live credential to this alias. + // Matching only the grant fingerprint would send one account's requests under + // another account's bearer. + const { getValidCodexToken, readCodexAccountRecord, saveCodexAccountCredential } = + await import("../src/codex/account-store"); + saveCodexAccountCredential("expired-alias", { + accessToken: "expired-access", + refreshToken: "shared-grant", + expiresAt: Date.now() - 1000, + chatgptAccountId: "acct-shared", + }); + saveCodexAccountCredential("foreign-fresh", { + accessToken: "foreign-fresh-access", + refreshToken: "shared-grant", + expiresAt: Date.now() + 3600_000, + chatgptAccountId: "acct-other", + }); + + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => Response.json({ + access_token: "refreshed-access", + refresh_token: "refreshed-grant", + expires_in: 3600, + })) as typeof fetch; + try { + const result = await getValidCodexToken("expired-alias"); + // It must refresh for itself rather than adopt the foreign-identity credential. + expect(result.accessToken).toBe("refreshed-access"); + expect(result.chatgptAccountId).toBe("acct-shared"); + } finally { + globalThis.fetch = originalFetch; + } + expect(readCodexAccountRecord("foreign-fresh")!.credential!.accessToken).toBe("foreign-fresh-access"); + }); +});