mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
Graceful 400 for keychain-import-only providers on OAuth route (zed, #6041). Integrated into release/v3.8.44.
This commit is contained in:
committed by
GitHub
parent
dc2efa5e63
commit
415d159c80
@@ -26,6 +26,8 @@
|
||||
|
||||
### 🔧 Bug Fixes
|
||||
|
||||
- **oauth (Zed "Unknown provider" crash):** adding **Zed** from the providers dashboard threw an unhandled `OAuth GET error: Unknown provider: zed` (500) ([#6041](https://github.com/diegosouzapw/OmniRoute/issues/6041)). Zed is a **keychain-import-only** provider — it's listed in the OAuth catalog so the UI shows it, but has no OAuth handler, so the generic `/api/oauth/[provider]/[action]` route hit `getProvider("zed")` and crashed. The route now recognizes keychain-import-only providers and returns a clear **400** pointing users at the **Import** button (for both GET and POST OAuth actions), instead of a 500. Regression guard: `tests/unit/oauth-keychain-import-only-6041.test.ts`. (thanks @imblowsnow)
|
||||
|
||||
- **fix(providers):** disable the unsupported `thinking` param for `minimax-m2.7` on NVIDIA NIM (the upstream rejects it). Regression guard: `tests/unit/nvidia-minimax-thinking-strip.test.ts`. (thanks @anki1kr)
|
||||
|
||||
- **fix(mitm):** add an in-process guard so concurrent MITM server starts no longer race — a second start while one is already in flight is short-circuited instead of double-binding the listener. Regression guard: `tests/unit/mitm-start-guard.test.ts`. (thanks @anki1kr)
|
||||
|
||||
@@ -68,6 +68,40 @@ 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.
|
||||
@@ -136,6 +170,14 @@ export async function GET(
|
||||
{ status: 410 }
|
||||
);
|
||||
}
|
||||
// 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);
|
||||
}
|
||||
} catch {
|
||||
/* fall through to normal handling */
|
||||
}
|
||||
@@ -356,6 +398,13 @@ export async function POST(
|
||||
{ status: 410 }
|
||||
);
|
||||
}
|
||||
// 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);
|
||||
}
|
||||
} catch {
|
||||
/* fall through to normal handling */
|
||||
}
|
||||
|
||||
71
tests/unit/oauth-keychain-import-only-6041.test.ts
Normal file
71
tests/unit/oauth-keychain-import-only-6041.test.ts
Normal file
@@ -0,0 +1,71 @@
|
||||
// Regression guard for #6041 — the generic OAuth route threw an unhandled
|
||||
// `Unknown provider: zed` 500 when the dashboard hit /api/oauth/zed/authorize.
|
||||
// Zed is a keychain-import-only provider (listed in the OAuth catalog so the UI
|
||||
// shows it, but with no OAuth handler), so the route now returns a clear 400
|
||||
// pointing at the Import flow instead of crashing.
|
||||
//
|
||||
// DB handles 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-oauth-6041-"));
|
||||
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 route = await import("../../src/app/api/oauth/[provider]/[action]/route.ts");
|
||||
|
||||
test.before(async () => {
|
||||
// The guard runs BEFORE the auth check, but disable login so any fall-through
|
||||
// path is exercised without a 401 masking the result.
|
||||
await settingsDb.updateSettings({ requireLogin: false });
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function get(provider: string, action: string) {
|
||||
const request = new Request(`http://localhost:20128/api/oauth/${provider}/${action}`);
|
||||
return route.GET(request, { params: Promise.resolve({ provider, action }) });
|
||||
}
|
||||
|
||||
test("#6041 GET /oauth/zed/authorize returns a graceful 400, not a 500 'Unknown provider'", async () => {
|
||||
const res = await get("zed", "authorize");
|
||||
assert.equal(res.status, 400, "must be a clean 400, not a 500 crash");
|
||||
const body = await res.json();
|
||||
assert.ok(body.error, "error message present");
|
||||
assert.match(body.error, /Import/i, "must point the user at the Import flow");
|
||||
assert.doesNotMatch(body.error, /Unknown provider/i, "must not leak the raw 'Unknown provider' error");
|
||||
// Never leak a stack trace (ERROR_SANITIZATION).
|
||||
assert.doesNotMatch(body.error, /at \//, "must not leak a stack trace");
|
||||
});
|
||||
|
||||
test("#6041 other keychain OAuth actions for zed are also handled gracefully", async () => {
|
||||
for (const action of ["device-code", "exchange", "poll"]) {
|
||||
const res = await get("zed", action);
|
||||
assert.equal(res.status, 400, `zed/${action} must be a clean 400`);
|
||||
const body = await res.json();
|
||||
assert.match(body.error, /Import/i, `zed/${action} points at Import`);
|
||||
}
|
||||
});
|
||||
|
||||
test("#6041 POST /oauth/zed/exchange is also guarded (no 500)", async () => {
|
||||
const request = new Request("http://localhost:20128/api/oauth/zed/exchange", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({}),
|
||||
});
|
||||
const res = await route.POST(request, {
|
||||
params: Promise.resolve({ provider: "zed", action: "exchange" }),
|
||||
});
|
||||
assert.equal(res.status, 400);
|
||||
const body = await res.json();
|
||||
assert.match(body.error, /Import/i);
|
||||
});
|
||||
Reference in New Issue
Block a user