mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
POST /api/oauth/codex/import accepted a payload with an already-invalidated refresh_token and persisted it as an 'active' connection that could never work — the failure surfaced confusingly only on first real use, long after the import looked successful. Validate each record's refresh_token against OpenAI's OAuth endpoint before persisting, reusing refreshCodexToken() (free exchange, no quota). On an unrecoverable refresh error the record is rejected with a clear re-auth message; a valid token imports as before, with any rotated tokens applied. Bulk import still processes each record independently — one dead token no longer blocks the valid ones. Reproduced live 2026-07-16: a 2026-07-10 auth.json imported clean but its refresh_token returned 401 refresh_token_invalidated. TDD: 3 tests RED against the old route, 5/5 GREEN with the fix. Closes #7522
This commit is contained in:
committed by
GitHub
parent
7974d03b9d
commit
ab01d74306
1
changelog.d/fixes/7522-codex-import-validate-refresh.md
Normal file
1
changelog.d/fixes/7522-codex-import-validate-refresh.md
Normal file
@@ -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).
|
||||
@@ -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<string | null> {
|
||||
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<string, unknown>);
|
||||
imported += 1;
|
||||
|
||||
172
tests/unit/codex-import-refresh-validation-7522.test.ts
Normal file
172
tests/unit/codex-import-refresh-validation-7522.test.ts
Normal file
@@ -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<T>(impl: typeof fetch, fn: () => Promise<T>): Promise<T> {
|
||||
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");
|
||||
}
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user