diff --git a/changelog.d/fixes/7522-codex-import-validate-refresh.md b/changelog.d/fixes/7522-codex-import-validate-refresh.md new file mode 100644 index 0000000000..73ae7b0f20 --- /dev/null +++ b/changelog.d/fixes/7522-codex-import-validate-refresh.md @@ -0,0 +1 @@ +- The Codex account import (`POST /api/oauth/codex/import`) now validates each record's `refresh_token` against OpenAI's OAuth endpoint before persisting the connection: an already-invalidated session (`refresh_token_invalidated` / a dead `auth.json`) is rejected with a clear "run `codex login` again and re-import" message instead of importing as `active` and failing confusingly on first use. Valid tokens import as before, with any rotated tokens applied (#7522). diff --git a/src/app/api/oauth/codex/import/route.ts b/src/app/api/oauth/codex/import/route.ts index 5e37803696..a7302a3a6d 100644 --- a/src/app/api/oauth/codex/import/route.ts +++ b/src/app/api/oauth/codex/import/route.ts @@ -4,6 +4,63 @@ import { normalizeCodexImportRecord, flattenCodexImportPayload } from "@/lib/oau import { createProviderConnection } from "@/models"; import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; +import { refreshCodexToken, isUnrecoverableRefreshError } from "@omniroute/open-sse/services/tokenRefresh.ts"; + +/** + * Message returned when the imported record's refresh_token is already dead + * (rotated/consumed/expired) — see #7522. Persisting a connection whose + * refresh_token can never succeed leaves an `active` connection that fails + * confusingly on first real use, long after the import looked successful. + */ +const EXPIRED_SESSION_MESSAGE = + "This Codex session has expired — run `codex login` again and re-import. " + + "(Esta sessão do Codex expirou — rode `codex login` novamente e reimporte.)"; + +/** + * Validate a normalized Codex import record's refresh_token against OpenAI's + * OAuth token endpoint before it is persisted as a connection. Reuses + * `refreshCodexToken()` (the same rotating-refresh-token exchange used by the + * runtime token-refresh path) instead of re-implementing the POST — the + * exchange call itself is free (no model/quota usage). + * + * Returns `null` when the token is valid (or the check was inconclusive, e.g. + * a transient network error) — the import proceeds normally in that case, + * optionally with rotated tokens already applied to `payload`. Returns an + * error string when the refresh_token is confirmed dead and the import + * should be rejected. + */ +async function validateCodexRefreshToken( + payload: { accessToken: string; refreshToken: string }, +): Promise { + let refreshResult: unknown; + try { + refreshResult = await refreshCodexToken(payload.refreshToken, undefined, null); + } catch { + // Network/transport failure: inconclusive, do not block the import. + return null; + } + + if (isUnrecoverableRefreshError(refreshResult)) { + return EXPIRED_SESSION_MESSAGE; + } + + if ( + refreshResult && + typeof refreshResult === "object" && + typeof (refreshResult as { accessToken?: unknown }).accessToken === "string" + ) { + const refreshed = refreshResult as { accessToken: string; refreshToken?: string }; + payload.accessToken = refreshed.accessToken; + if (typeof refreshed.refreshToken === "string" && refreshed.refreshToken) { + payload.refreshToken = refreshed.refreshToken; + } + } + + // `refreshResult === null` (transient error already logged inside + // refreshCodexToken) is inconclusive — fall through and import the + // originally-supplied tokens rather than blocking on a network hiccup. + return null; +} /** * POST /api/oauth/codex/import @@ -78,6 +135,14 @@ export async function POST(request: Request) { results.push({ index: i, ok: false, error: norm.error }); continue; } + + const refreshError = await validateCodexRefreshToken(norm.payload); + if (refreshError) { + failed += 1; + results.push({ index: i, ok: false, error: refreshError }); + continue; + } + try { const conn = await createProviderConnection(norm.payload as Record); imported += 1; diff --git a/tests/unit/codex-import-refresh-validation-7522.test.ts b/tests/unit/codex-import-refresh-validation-7522.test.ts new file mode 100644 index 0000000000..cb22a60748 --- /dev/null +++ b/tests/unit/codex-import-refresh-validation-7522.test.ts @@ -0,0 +1,172 @@ +// Regression test for #7522: POST /api/oauth/codex/import must validate the +// imported refresh_token BEFORE persisting a connection. Previously a payload +// carrying an already-invalidated refresh_token (e.g. `refresh_token_invalidated` +// / a dead-on-arrival `auth.json`) was imported as `active` and only failed +// confusingly on first real use. +// +// This test mocks global.fetch so `refreshCodexToken()` (open-sse/services/ +// tokenRefresh.ts) talks to a fake OpenAI OAuth token endpoint instead of the +// network — the refresh exchange itself is reused, not reimplemented. +// +// DB handles are released in test.after (CLAUDE.md learning: unreleased +// SQLite handles hang node:test). + +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-codex-import-refresh-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const route = await import("../../src/app/api/oauth/codex/import/route.ts"); + +test.before(async () => { + await settingsDb.updateSettings({ requireLogin: false }); +}); + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +function jsonResponse(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); +} + +async function withMockedFetch(impl: typeof fetch, fn: () => Promise): Promise { + const original = globalThis.fetch; + globalThis.fetch = impl; + try { + return await fn(); + } finally { + globalThis.fetch = original; + } +} + +async function postImport(body: unknown) { + const request = new Request("http://localhost:20128/api/oauth/codex/import", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + const response = await route.POST(request); + return { status: response.status, body: await response.json() }; +} + +const BASE_RECORD = { + access_token: "seed-access-token", + refresh_token: "seed-refresh-token-2026-07-10", + email: "operator@example.com", +}; + +test("import: rejects a record whose refresh_token is already invalidated upstream (#7522)", async () => { + await withMockedFetch( + (async () => + jsonResponse({ error: { code: "refresh_token_invalidated" } }, 401)) as unknown as typeof fetch, + async () => { + const { status, body } = await postImport({ accounts: BASE_RECORD }); + + assert.equal(status, 200); + assert.equal(body.success, false); + assert.equal(body.imported, 0); + assert.equal(body.failed, 1); + assert.equal(body.results[0].ok, false); + assert.match(body.results[0].error, /expired|codex login/i); + + const rows = await providersDb.getProviderConnections({ provider: "codex" }); + const created = rows.find((r) => r.email === BASE_RECORD.email); + assert.equal(created, undefined, "no connection should be persisted for a dead refresh_token"); + } + ); +}); + +test("import: rejects a record whose refresh_token was already consumed (refresh_token_reused)", async () => { + await withMockedFetch( + (async () => + jsonResponse({ error: { code: "refresh_token_reused" } }, 400)) as unknown as typeof fetch, + async () => { + const { status, body } = await postImport({ + accounts: { ...BASE_RECORD, email: "reused@example.com" }, + }); + + assert.equal(status, 200); + assert.equal(body.success, false); + assert.equal(body.failed, 1); + + const rows = await providersDb.getProviderConnections({ provider: "codex" }); + const created = rows.find((r) => r.email === "reused@example.com"); + assert.equal(created, undefined); + } + ); +}); + +test("import: creates the connection (with rotated tokens) when the refresh_token is still valid", async () => { + await withMockedFetch( + (async () => + jsonResponse({ + access_token: "rotated-access-token", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + })) as unknown as typeof fetch, + async () => { + const { status, body } = await postImport({ + accounts: { ...BASE_RECORD, email: "valid@example.com" }, + }); + + assert.equal(status, 200); + assert.equal(body.success, true); + assert.equal(body.imported, 1); + assert.equal(body.failed, 0); + assert.equal(body.results[0].ok, true); + + const rows = await providersDb.getProviderConnections({ provider: "codex" }); + const created = rows.find((r) => r.email === "valid@example.com"); + assert.ok(created, "connection should be persisted for a valid refresh_token"); + assert.equal(created?.accessToken, "rotated-access-token"); + assert.equal(created?.refreshToken, "rotated-refresh-token"); + } + ); +}); + +test("import: a transient network error validating the refresh_token does not block the import", async () => { + await withMockedFetch( + (async () => { + throw new Error("ECONNRESET"); + }) as unknown as typeof fetch, + async () => { + const { status, body } = await postImport({ + accounts: { ...BASE_RECORD, email: "transient@example.com" }, + }); + + assert.equal(status, 200); + assert.equal(body.success, true); + assert.equal(body.imported, 1); + + const rows = await providersDb.getProviderConnections({ provider: "codex" }); + const created = rows.find((r) => r.email === "transient@example.com"); + assert.ok(created, "import should proceed with the original tokens on a transient failure"); + assert.equal(created?.accessToken, BASE_RECORD.access_token); + } + ); +}); + +test("import: error responses never leak a stack trace", async () => { + await withMockedFetch( + (async () => jsonResponse({ error: { code: "refresh_token_invalidated" } }, 401)) as unknown as typeof fetch, + async () => { + const { body } = await postImport({ + accounts: { ...BASE_RECORD, email: "leak-check@example.com" }, + }); + assert.ok(!JSON.stringify(body).includes("at /"), "must not leak a stack trace"); + assert.ok(!JSON.stringify(body).includes(".ts:"), "must not leak a source location"); + } + ); +});