diff --git a/CHANGELOG.md b/CHANGELOG.md index 08207a7b16..a5505e2986 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ### 🐛 Bug Fixes +- **fix(api):** internal probes (combo-test, cloud-sync verify) now pick a **management-scoped / allow-all API key** instead of naively grabbing `getApiKeys()[0]` — a restricted `self:usage` first row made the probe fail with "Model X is not allowed for this API key" even when the combo path was healthy (`pickApiKeyForInternalUse` in `src/lib/db/apiKeys.ts`). The API-manager model editor also falls back to `/api/models?all=true` when `/v1/models` is catalog-protected. Regression guard: `tests/unit/pick-internal-api-key-6372.test.ts`. (thanks @jmengit) - **fix(live-ws):** the Live Dashboard WebSocket server now **rejects on bind failure** (e.g. `EADDRINUSE` when the API bridge already holds the port) instead of letting the error surface as an unhandled `error` event that crash-loops the process — the `error` listener is attached to `wss` (not `server`) and releases the EventBus subscription on a failed start ([#6324](https://github.com/diegosouzapw/OmniRoute/issues/6324)). Regression guard: `tests/unit/live-ws-eaddrinuse-6324.test.ts`. (thanks @vinayakkulkarni) - **fix(dashboard):** the Home provider-topology widget now trusts the live provider-metrics snapshot — it uses `topology.errorProvider` and live `activeRequests` directly instead of re-deriving state from a stale `lastErrorAt` or applying a frontend timeout filter, so the topology reflects real-time provider health. Regression guard: `tests/unit/home-provider-topology-live-state.test.ts`. (thanks @xz-dev) - **fix(sse):** strip zero-width markers from streamed **tool-call arguments** — a follow-up to [#5857](https://github.com/diegosouzapw/OmniRoute/pull/5857). That PR removed injected zero-width joiners (U+200D) from streamed assistant text/reasoning but deliberately left tool-call argument JSON byte-exact. The request-side obfuscation (`open-sse/services/claudeCodeObfuscation.ts`) injects ZWJ into agent words — including the temp path inside the Bash tool description — and Claude models copy that verbatim into generated commands, which are delivered as tool-call arguments rather than assistant text. As a result the ZWJ survived and corrupted code blocks (e.g. a temp path rendered with an invisible joiner). Now `open-sse/handlers/responseSanitizer.ts` strips zero-width code points from tool-call argument strings at every emit site (OpenAI non-stream/stream chat `tool_calls` + legacy `function_call`, native Responses `function_call` items, the OpenAI→Responses conversion, and the native Responses streaming `response.function_call_arguments.delta/.done` events). Only zero-width code points are removed; JSON structure and all other bytes stay identical (no parse/restringify), so normal arguments remain byte-exact. Regression guard: 6 new cases in `tests/unit/response-sanitizer.test.ts` (suite 50/50). diff --git a/src/app/(dashboard)/dashboard/api-manager/ApiManagerPageClient.tsx b/src/app/(dashboard)/dashboard/api-manager/ApiManagerPageClient.tsx index 3a54c5a74f..e4a0de2e2e 100644 --- a/src/app/(dashboard)/dashboard/api-manager/ApiManagerPageClient.tsx +++ b/src/app/(dashboard)/dashboard/api-manager/ApiManagerPageClient.tsx @@ -203,6 +203,7 @@ export default function ApiManagerPageClient() { const createKeyFormRef = useRef(null); const [keys, setKeys] = useState([]); const [allModels, setAllModels] = useState([]); + const [modelsLoaded, setModelsLoaded] = useState(false); const [allCombos, setAllCombos] = useState([]); const [allConnections, setAllConnections] = useState([]); const [loading, setLoading] = useState(true); @@ -320,14 +321,40 @@ export default function ApiManagerPageClient() { }, [showAddModal, nameError, scrollCreateKeyFormToTop]); const fetchModels = async () => { + setModelsLoaded(false); try { const res = await fetch("/v1/models"); if (res.ok) { const data = await res.json(); - setAllModels(data.data || []); + setAllModels(Array.isArray(data.data) ? data.data : []); + return; + } + + // Fallback for dashboard API-key editing: /v1/models can be protected by + // API-key catalog auth, but the dashboard still needs a stable catalog so + // users can edit allowedModels. /api/models?all=true returns the static + // dashboard model inventory in a slightly different shape. + const fallbackRes = await fetch("/api/models?all=true"); + if (fallbackRes.ok) { + const fallbackData = await fallbackRes.json(); + const fallbackModels = Array.isArray(fallbackData.models) ? fallbackData.models : []; + setAllModels( + fallbackModels + .map((m: any) => ({ + id: typeof m.fullModel === "string" ? m.fullModel : `${m.provider}/${m.model}`, + owned_by: typeof m.provider === "string" ? m.provider : "unknown", + name: typeof m.alias === "string" ? m.alias : m.model || m.fullModel, + })) + .filter((m: Model) => typeof m.id === "string" && m.id.length > 0) + ); + } else { + setAllModels([]); } } catch (error) { console.log("Error fetching models:", error); + setAllModels([]); + } finally { + setModelsLoaded(true); } }; @@ -1533,6 +1560,7 @@ export default function ApiManagerPageClient() { apiKey={editingKey} modelsByProvider={filteredModelsByProvider} allModels={permissionModels} + modelsLoaded={modelsLoaded} allCombos={allCombos} allConnections={allConnections} searchModel={searchModel} @@ -1552,6 +1580,7 @@ const PermissionsModal = memo(function PermissionsModal({ apiKey, modelsByProvider, allModels, + modelsLoaded, allCombos, allConnections, searchModel, @@ -1563,6 +1592,7 @@ const PermissionsModal = memo(function PermissionsModal({ apiKey: ApiKey; modelsByProvider: ProviderGroup[]; allModels: Model[]; + modelsLoaded: boolean; allCombos: ComboOption[]; allConnections: ProviderConnection[]; searchModel: string; @@ -2024,7 +2054,13 @@ const PermissionsModal = memo(function PermissionsModal({ allowAll ? "text-green-700 dark:text-green-300" : "text-amber-700 dark:text-amber-300" }`} > - {allowAll ? t("allowAllDesc") : t("restrictDesc", { selectedCount, totalModels })} + {allowAll + ? t("allowAllDesc") + : !modelsLoaded + ? t("restrictLoading") + : totalModels === 0 + ? t("restrictCatalogUnavailable", { selectedCount }) + : t("restrictDesc", { selectedCount, totalModels })}

diff --git a/src/app/api/combos/test/route.ts b/src/app/api/combos/test/route.ts index 7b37cfcd86..db8723477d 100644 --- a/src/app/api/combos/test/route.ts +++ b/src/app/api/combos/test/route.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { NextResponse } from "next/server"; import { buildComboTestRequestBody, extractComboTestResponseText } from "@/lib/combos/testHealth"; -import { getApiKeys, getComboByName, getCombos } from "@/lib/localDb"; +import { getComboByName, getCombos, pickApiKeyForInternalUse } from "@/lib/localDb"; import { getRuntimePorts } from "@/lib/runtime/ports"; import { resolveNestedComboTargets } from "@omniroute/open-sse/services/combo.ts"; import { testComboSchema } from "@/shared/validation/schemas"; @@ -9,15 +9,10 @@ import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; async function getInternalApiKey(): Promise { - try { - const keys = await getApiKeys(); - const active = ( - keys as Array<{ key: string; isActive?: boolean; revokedAt?: string | null }> - ).find((k) => k.key && k.isActive !== false && !k.revokedAt); - return active?.key ?? null; - } catch { - return null; - } + // Combo health-check probes hit /v1/chat/completions, which enforces + // per-key model allowlists (see shared/utils/apiKeyPolicy.ts). Picking + // an arbitrary active key is unsafe — see pickApiKeyForInternalUse. + return pickApiKeyForInternalUse("combo-health-check"); } function buildComboTestResult(target, partial = {}) { diff --git a/src/app/api/sync/cloud/route.ts b/src/app/api/sync/cloud/route.ts index 89fcd4c65e..2f37d15a9d 100644 --- a/src/app/api/sync/cloud/route.ts +++ b/src/app/api/sync/cloud/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from "next/server"; -import { getApiKeys, createApiKey, updateSettings } from "@/lib/localDb"; +import { getApiKeys, createApiKey, pickApiKeyForInternalUse, updateSettings } from "@/lib/localDb"; import { getConsistentMachineId } from "@/shared/utils/machineId"; import { syncToCloud, fetchWithTimeout, CLOUD_URL } from "@/lib/cloudSync"; import fs from "fs/promises"; @@ -24,8 +24,9 @@ export async function GET() { // Cloud is enabled — try to verify connection const machineId = await getConsistentMachineId(); - const keys = await getApiKeys(); - const apiKey = keys[0]?.key; + // Prefer a manage-scoped or allow-all key so the verify ping is not + // rejected upstream when keys[0] is a restricted self:usage key. + const apiKey = await pickApiKeyForInternalUse("cloud-sync-verify"); if (!apiKey || !CLOUD_URL) { return NextResponse.json({ enabled: true, connected: false }); diff --git a/src/i18n/messages/en.json b/src/i18n/messages/en.json index 3df936b09b..63f30a902d 100644 --- a/src/i18n/messages/en.json +++ b/src/i18n/messages/en.json @@ -1777,6 +1777,8 @@ "models": "{count} models", "permissionsTitle": "Permissions: {name}", "allowAllDesc": "This key can access all available models.", + "restrictLoading": "Loading model catalog…", + "restrictCatalogUnavailable": "Model catalog unavailable; this key has {selectedCount} selected model restrictions.", "restrictDesc": "This key can access {selectedCount} of {totalModels} models.", "selectedCount": "{count} selected", "maxActiveSessions": "Max Active Sessions", diff --git a/src/lib/db/apiKeys.ts b/src/lib/db/apiKeys.ts index 7d76f99296..874bae3f1b 100644 --- a/src/lib/db/apiKeys.ts +++ b/src/lib/db/apiKeys.ts @@ -454,6 +454,86 @@ export async function getApiKeys() { }); } +/** + * Select an API key for internal OmniRoute operations (combo health checks, + * cloud-sync verify pings, etc.). + * + * Naive selection of `getApiKeys()[0]` is unsafe because the first row is + * whatever happened to be inserted first — usually a regular `self:usage` + * key with a restricted model allowlist. Internal probes that reuse that + * key to call `/v1/chat/completions` then hit + * Model "X" is not allowed for this API key + * from `shared/utils/apiKeyPolicy.ts` even when the upstream combo path is + * healthy. Likewise, cloud-sync verify pings flip to "disconnected" + * because the arbitrary key is rejected upstream. + * + * Selection rules (first match wins): + * 1. Active, non-revoked key whose `scopes` includes "manage" + * (management keys are by policy not subject to model allowlists). + * 2. Active, non-revoked key with empty allowedModels (allow-all). + * 3. Active, non-revoked key with the most recent `lastUsedAt`. + * 4. First active, non-revoked key (legacy fallback — preserves prior + * behavior when no key matches the better rules above). + * + * The selector is deliberately conservative: it never promotes a revoked, + * inactive, or banned key, and it never widens a key's allowedModels. + */ +export async function pickApiKeyForInternalUse( + purpose: + | "combo-health-check" + | "cloud-sync-verify" + | "internal-probe" = "internal-probe" +): Promise { + try { + const keys = (await getApiKeys()) as Array<{ + key?: string; + isActive?: boolean; + revokedAt?: string | null; + isBanned?: boolean; + scopes?: string[]; + allowedModels?: string[]; + lastUsedAt?: string | number | null; + }>; + + const isUsable = (k: (typeof keys)[number]) => + Boolean(k.key) && k.isActive !== false && !k.revokedAt && k.isBanned !== true; + + // 1. Management-scoped key (preferred for any internal probe). + const manageKey = keys.find( + (k) => + isUsable(k) && Array.isArray(k.scopes) && k.scopes.includes("manage"), + ); + if (manageKey?.key) return manageKey.key; + + // 2. Allow-all key (empty allowedModels means no model restrictions). + const allowAllKey = keys.find( + (k) => + isUsable(k) && + Array.isArray(k.allowedModels) && + k.allowedModels.length === 0, + ); + if (allowAllKey?.key) return allowAllKey.key; + + // 3. Most recently used (proxy for "the user actually wants this one + // working right now"). + const byRecency = [...keys] + .filter(isUsable) + .sort((a, b) => { + const aT = typeof a.lastUsedAt === "number" ? a.lastUsedAt : 0; + const bT = typeof b.lastUsedAt === "number" ? b.lastUsedAt : 0; + return bT - aT; + }); + if (byRecency[0]?.key) return byRecency[0].key; + + // 4. Legacy fallback: first active key. Keeps the function working + // for setups with no managed/allow-all/recently-used key. + const firstActive = keys.find(isUsable); + return firstActive?.key ?? null; + } catch { + return null; + } +} + export async function getApiKeyById(id: string) { const db = getDbInstance() as ApiKeysDbLike; const stmt = getPreparedStatements(db); diff --git a/src/lib/localDb.ts b/src/lib/localDb.ts index fcf46560e2..9cd7be993e 100755 --- a/src/lib/localDb.ts +++ b/src/lib/localDb.ts @@ -107,6 +107,7 @@ export { updateApiKeyPermissions, regenerateApiKey, isModelAllowedForKey, + pickApiKeyForInternalUse, clearApiKeyCaches, resetApiKeyState, } from "./db/apiKeys"; diff --git a/tests/unit/pick-internal-api-key-6372.test.ts b/tests/unit/pick-internal-api-key-6372.test.ts new file mode 100644 index 0000000000..ab70247d31 --- /dev/null +++ b/tests/unit/pick-internal-api-key-6372.test.ts @@ -0,0 +1,50 @@ +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"; + +// #6372: internal probes (combo-test, cloud-sync verify) must NOT naively grab +// getApiKeys()[0] — that first row is usually a restricted self:usage key, so +// the probe hits "Model X is not allowed for this API key" even when the combo +// path is healthy. pickApiKeyForInternalUse prefers a management-scoped key. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-pick-internal-key-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const apiKeysDb = await import("../../src/lib/db/apiKeys.ts"); + +function reset() { + core.resetDbInstance(); + apiKeysDb.resetApiKeyState(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(() => reset()); +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("#6372: returns null when there are no keys", async () => { + assert.equal(await apiKeysDb.pickApiKeyForInternalUse("combo-health-check"), null); +}); + +test("#6372: prefers a management-scoped key over a plain self:usage key", async () => { + // Insert the plain (restricted-intent) key FIRST so getApiKeys()[0] would be + // the wrong one under the old naive selection. + await apiKeysDb.createApiKey("usage-key", "machine-a", ["self:usage"]); + const mgr = await apiKeysDb.createApiKey("manage-key", "machine-a", ["manage"]); + + const picked = await apiKeysDb.pickApiKeyForInternalUse("combo-health-check"); + assert.equal(picked, mgr.key, "should pick the management-scoped key, not the first row"); +}); + +test("#6372: falls back to an active key when none is management-scoped", async () => { + const only = await apiKeysDb.createApiKey("usage-key", "machine-a", ["self:usage"]); + const picked = await apiKeysDb.pickApiKeyForInternalUse("internal-probe"); + assert.equal(picked, only.key, "should still return a usable active key via fallback rules"); +});