Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 49 additions & 0 deletions src/app/api/oauth/[provider]/[action]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 */
}
Expand Down Expand Up @@ -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 */
}
Expand Down
71 changes: 71 additions & 0 deletions tests/unit/oauth-keychain-import-only-6041.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
Loading