From d0a32d8a03d0c67b5a1eabc73029ed84c05fb2ee Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Sat, 4 Jul 2026 03:24:30 -0300 Subject: [PATCH] fix(oauth): extract keychain-import-only guard to restore file-size freeze (base-red) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `src/app/api/oauth/[provider]/[action]/route.ts` grew to 959 lines, past its frozen cap of 924 (`check:file-size` → Fast Quality Gates red on release/v3.8.44). The growth came from #6054 (graceful 400 for keychain-import-only providers / zed): a doc block, two Sets (KEYCHAIN_IMPORT_ONLY_PROVIDERS, OAUTH_FLOW_ACTIONS) and a keychainImportOnlyResponse() helper, plus two duplicated guard blocks in GET/POST. That is a cohesive, self-contained leaf, so extract it to a new `keychainImportOnly.ts` exposing `keychainImportOnlyGuard(provider, action)` (returns the 400 NextResponse or null). The two route callsites collapse to a 2-line guard each. route.ts: 959 -> 918 (< 924, freeze restored). No behavior change. Tests (Rule #8/#18): - Existing tests/unit/oauth-keychain-import-only-6041.test.ts (route-level GET/POST zed 400) still pass unchanged — behavior preserved. - New tests/unit/oauth-keychain-import-only-guard.test.ts pins the extracted guard in isolation (zed+flow -> 400, normal provider -> null, zed+non-flow -> null). --- .../[provider]/[action]/keychainImportOnly.ts | 50 ++++++++++++++++++ .../api/oauth/[provider]/[action]/route.ts | 51 ++----------------- .../oauth-keychain-import-only-guard.test.ts | 38 ++++++++++++++ 3 files changed, 93 insertions(+), 46 deletions(-) create mode 100644 src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts create mode 100644 tests/unit/oauth-keychain-import-only-guard.test.ts diff --git a/src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts b/src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts new file mode 100644 index 0000000000..0ee954f54a --- /dev/null +++ b/src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts @@ -0,0 +1,50 @@ +import { NextResponse } from "next/server"; + +/** + * Providers that have NO browser OAuth flow at all — their credentials are read + * from the OS keychain via a dedicated Import button, not an OAuth + * authorize/exchange. They are listed in the OAuth provider *catalog* + * (so the dashboard shows them) but have no entry in the OAuth provider + * *handler* registry, so hitting the generic OAuth route for them threw an + * unhandled `Unknown provider: ` 500 (#6041). Return a clear, actionable + * response pointing at the Import flow instead. + * + * Extracted from the OAuth route handler into this leaf module so the route + * stays under its frozen file-size cap (#6155 base-red follow-up). + */ +export const KEYCHAIN_IMPORT_ONLY_PROVIDERS = new Set(["zed"]); + +/** GET/POST OAuth actions that don't apply to keychain-import-only providers. */ +export const OAUTH_FLOW_ACTIONS = new Set([ + "authorize", + "device-code", + "start-callback-server", + "poll-callback", + "exchange", + "poll", + "device-complete", +]); + +function keychainImportOnlyResponse(provider: string) { + return NextResponse.json( + { + error: + `${provider} has no browser OAuth flow — it imports LLM credentials from the ` + + `OS keychain. Use the "Import" button on the ${provider} provider card in the ` + + `dashboard to discover and import them automatically.`, + }, + { status: 400 } + ); +} + +/** + * If `provider` is keychain-import-only and `action` is an OAuth-flow action, + * return the graceful 400 response; otherwise return null so the caller falls + * through to normal OAuth handling. + */ +export function keychainImportOnlyGuard(provider: string, action: string): NextResponse | null { + if (KEYCHAIN_IMPORT_ONLY_PROVIDERS.has(provider) && OAUTH_FLOW_ACTIONS.has(action)) { + return keychainImportOnlyResponse(provider); + } + return null; +} diff --git a/src/app/api/oauth/[provider]/[action]/route.ts b/src/app/api/oauth/[provider]/[action]/route.ts index 373f572b01..270372eb0e 100755 --- a/src/app/api/oauth/[provider]/[action]/route.ts +++ b/src/app/api/oauth/[provider]/[action]/route.ts @@ -35,6 +35,7 @@ import { import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth"; import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error"; +import { keychainImportOnlyGuard } from "./keychainImportOnly"; // Use globalThis to persist callback server state across Next.js HMR reloads if (!globalThis.__codexCallbackState) { @@ -68,40 +69,6 @@ const RETIRED_PKCE_PROVIDERS = new Set(["windsurf", "devin-cli"]); /** Providers that allow direct import of a raw API token (no OAuth exchange). */ const IMPORT_TOKEN_PROVIDERS = new Set(["windsurf", "devin-cli", "grok-cli"]); -/** - * Providers that have NO browser OAuth flow at all — their credentials are read - * from the OS keychain via a dedicated Import button, not an OAuth - * authorize/exchange. They are listed in the OAuth provider *catalog* - * (so the dashboard shows them) but have no entry in the OAuth provider - * *handler* registry, so hitting the generic OAuth route for them threw an - * unhandled `Unknown provider: ` 500 (#6041). Return a clear, actionable - * response pointing at the Import flow instead. - */ -const KEYCHAIN_IMPORT_ONLY_PROVIDERS = new Set(["zed"]); - -/** GET/POST OAuth actions that don't apply to keychain-import-only providers. */ -const OAUTH_FLOW_ACTIONS = new Set([ - "authorize", - "device-code", - "start-callback-server", - "poll-callback", - "exchange", - "poll", - "device-complete", -]); - -function keychainImportOnlyResponse(provider: string) { - return NextResponse.json( - { - error: - `${provider} has no browser OAuth flow — it imports LLM credentials from the ` + - `OS keychain. Use the "Import" button on the ${provider} provider card in the ` + - `dashboard to discover and import them automatically.`, - }, - { status: 400 } - ); -} - /** * Constant-time string comparison to prevent timing-oracle attacks (CWE-208). * Handles null/undefined safely and different-length strings. @@ -172,12 +139,8 @@ export async function GET( } // Keychain-import-only providers (e.g. zed) have no OAuth flow — return a // clear 400 pointing at the Import button instead of a 500 (#6041). - if ( - KEYCHAIN_IMPORT_ONLY_PROVIDERS.has(earlyParams.provider) && - OAUTH_FLOW_ACTIONS.has(earlyParams.action) - ) { - return keychainImportOnlyResponse(earlyParams.provider); - } + const kio = keychainImportOnlyGuard(earlyParams.provider, earlyParams.action); + if (kio) return kio; } catch { /* fall through to normal handling */ } @@ -399,12 +362,8 @@ export async function POST( ); } // Keychain-import-only providers (e.g. zed) have no OAuth flow (#6041). - if ( - KEYCHAIN_IMPORT_ONLY_PROVIDERS.has(earlyParams.provider) && - OAUTH_FLOW_ACTIONS.has(earlyParams.action) - ) { - return keychainImportOnlyResponse(earlyParams.provider); - } + const kio = keychainImportOnlyGuard(earlyParams.provider, earlyParams.action); + if (kio) return kio; } catch { /* fall through to normal handling */ } diff --git a/tests/unit/oauth-keychain-import-only-guard.test.ts b/tests/unit/oauth-keychain-import-only-guard.test.ts new file mode 100644 index 0000000000..4160987917 --- /dev/null +++ b/tests/unit/oauth-keychain-import-only-guard.test.ts @@ -0,0 +1,38 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + keychainImportOnlyGuard, + KEYCHAIN_IMPORT_ONLY_PROVIDERS, + OAUTH_FLOW_ACTIONS, +} from "../../src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts"; + +// Unit coverage for the leaf extracted from the OAuth route (#6155 file-size +// base-red follow-up). The route-level behavior is guarded by +// oauth-keychain-import-only-6041.test.ts; this pins the guard in isolation. + +test("keychainImportOnlyGuard returns a 400 for a keychain-import-only provider on an OAuth-flow action", async () => { + const res = keychainImportOnlyGuard("zed", "authorize"); + assert.ok(res, "expected a response, not null"); + assert.equal(res!.status, 400); + const body = await res!.json(); + assert.match(body.error, /no browser OAuth flow/i); + assert.match(body.error, /Import/); +}); + +test("keychainImportOnlyGuard returns null for a normal OAuth provider", () => { + assert.equal(keychainImportOnlyGuard("openai", "authorize"), null); + assert.equal(keychainImportOnlyGuard("anthropic", "exchange"), null); +}); + +test("keychainImportOnlyGuard returns null for a keychain provider on a non-flow action", () => { + // e.g. a callback/status action that is not in OAUTH_FLOW_ACTIONS + assert.equal(keychainImportOnlyGuard("zed", "status"), null); +}); + +test("the sets stay in sync with the guard's expectations", () => { + assert.ok(KEYCHAIN_IMPORT_ONLY_PROVIDERS.has("zed")); + assert.ok(OAUTH_FLOW_ACTIONS.has("authorize")); + assert.ok(OAUTH_FLOW_ACTIONS.has("exchange")); + assert.ok(!OAUTH_FLOW_ACTIONS.has("status")); +});