fix(internal): use explicit internal key selection for dashboard probes (#6372)

Internal probes pick a management-scoped key (pickApiKeyForInternalUse); API-manager model-editor fallback catalog. Integrated into release/v3.8.46.
This commit is contained in:
jmengit
2026-07-06 15:20:53 -05:00
committed by GitHub
parent b796f2b457
commit 049bad494b
8 changed files with 181 additions and 15 deletions

View File

@@ -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).

View File

@@ -203,6 +203,7 @@ export default function ApiManagerPageClient() {
const createKeyFormRef = useRef<HTMLDivElement | null>(null);
const [keys, setKeys] = useState<ApiKey[]>([]);
const [allModels, setAllModels] = useState<Model[]>([]);
const [modelsLoaded, setModelsLoaded] = useState(false);
const [allCombos, setAllCombos] = useState<ComboOption[]>([]);
const [allConnections, setAllConnections] = useState<ProviderConnection[]>([]);
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 })}
</p>
</div>

View File

@@ -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<string | null> {
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 = {}) {

View File

@@ -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 });

View File

@@ -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",

View File

@@ -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<string | null> {
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);

View File

@@ -107,6 +107,7 @@ export {
updateApiKeyPermissions,
regenerateApiKey,
isModelAllowedForKey,
pickApiKeyForInternalUse,
clearApiKeyCaches,
resetApiKeyState,
} from "./db/apiKeys";

View File

@@ -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");
});