mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-13 10:43:43 +03:00
fix(oauth): extract keychain-import-only guard to restore file-size freeze (base-red)
`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).
This commit is contained in:
50
src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts
Normal file
50
src/app/api/oauth/[provider]/[action]/keychainImportOnly.ts
Normal file
@@ -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: <id>` 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;
|
||||
}
|
||||
@@ -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: <id>` 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 */
|
||||
}
|
||||
|
||||
38
tests/unit/oauth-keychain-import-only-guard.test.ts
Normal file
38
tests/unit/oauth-keychain-import-only-guard.test.ts
Normal file
@@ -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"));
|
||||
});
|
||||
Reference in New Issue
Block a user