mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-18 21:02:50 +03:00
Compare commits
1 Commits
fix/12745-
...
fix/12111-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d1fd3a6f40 |
@@ -2721,10 +2721,6 @@ PLAYGROUND_COMPARE_MAX_COLUMNS=4
|
||||
# MEMORY_VEC_TOP_K=20 # default top-K for vector search
|
||||
# MEMORY_RRF_K=60 # RRF k constant (sqlite-vec hybrid recipe)
|
||||
# HF_HUB_ENDPOINT=https://huggingface.co # override Hugging Face Hub base URL for static potion downloads
|
||||
# Test/diagnostic seam (src/lib/memory/vectorStore.ts) — forces getVectorStore() to
|
||||
# return null (simulates a cloud/WASM environment without sqlite-vec), degrading
|
||||
# memory retrieval to FTS5 keyword search. Default off; leave unset in production.
|
||||
# VECTOR_STORE_DISABLE_VEC=false
|
||||
# TV6 typed memory decay (OPT-IN, default off — the sweep DELETES decayed memories)
|
||||
# MEMORY_TYPED_DECAY_ENABLED=false # master switch for the destructive sweep (default off)
|
||||
# MEMORY_TYPED_DECAY_EPISODIC_DAYS=30 # episodic TTL in days; 0 = episodic immune too
|
||||
|
||||
1
changelog.d/fixes/12111-vision-bridge-model-lockout.md
Normal file
1
changelog.d/fixes/12111-vision-bridge-model-lockout.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(guardrails): stop Vision Bridge from re-selecting a model locked after a 404 (#12111)
|
||||
@@ -1 +0,0 @@
|
||||
- fix(memory): authenticate the internal /v1/rerank loopback call so memory reranking no longer silently degrades to unranked order when REQUIRE_API_KEY=true (#12745)
|
||||
@@ -923,7 +923,6 @@ Embedding layer, vector store and reranking knobs for the persistent memory subs
|
||||
| `HF_HUB_ENDPOINT` | `https://huggingface.co` | Override Hugging Face Hub base URL used by `staticPotion.ts` (e.g. mirror endpoint for air-gapped setups). |
|
||||
| `MEMORY_VEC_TOP_K` | `20` | Default top-K used by the `sqlite-vec` brute-force vector search inside `src/lib/memory/vectorStore.ts`. |
|
||||
| `MEMORY_RRF_K` | `60` | Reciprocal Rank Fusion constant `k` for hybrid FTS5 + vector retrieval (sqlite-vec recipe). |
|
||||
| `VECTOR_STORE_DISABLE_VEC` | `false` | Test/diagnostic seam in `getVectorStore()` (`src/lib/memory/vectorStore.ts`): when `true`, forces the vector store to `null` (simulates a cloud/WASM environment without `sqlite-vec`), degrading memory retrieval to FTS5 keyword search. Leave unset in production. |
|
||||
| `NOTION_API_KEY` | _(unset)_ | API key for Notion backend (used by `genericBackend.ts` known backend preset). |
|
||||
| `NOTION_API_URL` | `https://api.notion.com/v1`| Base URL for Notion API (can override for self-hosted Notion alternatives). |
|
||||
| `OBSIDIAN_API_KEY` | _(unset)_ | API key for Obsidian Vault backend (used by `genericBackend.ts` known backend preset). |
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
import { resolveProviderId } from "@/shared/constants/providers";
|
||||
import { isNoAuthProviderKey } from "@/shared/utils/noAuthProviders";
|
||||
import { SYNTHETIC_NOAUTH_CONNECTION_ID } from "@omniroute/open-sse/services/autoCombo/resilienceCandidateFilter.ts";
|
||||
|
||||
/**
|
||||
* True when a provider connection can actually authenticate upstream.
|
||||
@@ -119,3 +120,51 @@ export async function hasUsableCredentialsForModel(model: string): Promise<boole
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/** A minimal reference to a usable provider connection, for per-connection lockout checks. */
|
||||
export interface UsableConnectionRef {
|
||||
id: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the individual usable connections for `model`'s provider (#12111).
|
||||
*
|
||||
* `hasUsableCredentialsForModel` collapses this same data to a single
|
||||
* boolean, which is enough to know a provider is reachable at all but not
|
||||
* enough to know whether one *specific* model is servable: `isModelLocked`
|
||||
* (open-sse/services/accountFallback.ts) is scoped per provider+connection+
|
||||
* model, so callers that need to exclude a locked model must check it
|
||||
* against each connection that could actually serve it — dropping the model
|
||||
* only when every one of those connections has it locked (mirrors
|
||||
* `isConnectionEligibleForModel` in
|
||||
* open-sse/services/autoCombo/resilienceCandidateFilter.ts).
|
||||
*
|
||||
* Returns `null` on the same indeterminate cases as
|
||||
* `hasUsableCredentialsForModel` (credential store unavailable) so callers
|
||||
* can fail open identically. No-auth providers with no stored connection row
|
||||
* resolve to the synthetic "noauth" connection id, matching the id
|
||||
* `lockModel`/`isModelLocked` use for those providers elsewhere in the
|
||||
* resilience layer.
|
||||
*/
|
||||
export async function getUsableConnectionsForModel(
|
||||
model: string
|
||||
): Promise<UsableConnectionRef[] | null> {
|
||||
const rawProvider = typeof model === "string" ? model.split("/")[0]?.trim() : "";
|
||||
if (!rawProvider) return null;
|
||||
const provider = resolveProviderId(rawProvider);
|
||||
const isNoAuth = isNoAuthProviderKey(rawProvider, provider);
|
||||
try {
|
||||
const { getProviderConnections } = await loadProvidersModule();
|
||||
const connections = await getProviderConnections({ provider, isActive: true });
|
||||
if (!Array.isArray(connections)) return null;
|
||||
if (connections.length === 0) {
|
||||
return isNoAuth ? [{ id: SYNTHETIC_NOAUTH_CONNECTION_ID }] : [];
|
||||
}
|
||||
const usable = isNoAuth
|
||||
? connections.filter((c: any) => !hasTerminalConnectionStatus(c))
|
||||
: connections.filter((c: any) => isProviderConnectionUsable(c));
|
||||
return usable.map((c: any) => ({ id: String(c.id) }));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,8 +7,16 @@ import { getResolvedModelCapabilities } from "@/lib/modelCapabilities";
|
||||
import { getActiveSyncedCatalog } from "@/lib/db/models/activeSyncedCatalog";
|
||||
import { PROVIDER_MODELS } from "@omniroute/open-sse/config/providerModels";
|
||||
import { getRegisteredProviderEffortBaseModelId } from "@omniroute/open-sse/utils/registeredEffortVariants.ts";
|
||||
import { hasUsableCredentialsForModel } from "./visionBridgeCredentials";
|
||||
import {
|
||||
hasUsableCredentialsForModel,
|
||||
getUsableConnectionsForModel,
|
||||
} from "./visionBridgeCredentials";
|
||||
import { isVisionBridgeForcedModel } from "@/shared/constants/visionBridgeDefaults";
|
||||
import { resolveProviderId } from "@/shared/constants/providers";
|
||||
import {
|
||||
isModelLocked,
|
||||
getAllModelLockouts,
|
||||
} from "@omniroute/open-sse/services/accountFallback.ts";
|
||||
|
||||
export interface VisionModelCandidate {
|
||||
modelId: string;
|
||||
@@ -109,6 +117,12 @@ function calculateSuccessRate(modelId: string): number {
|
||||
export interface VisionBridgeRouterDeps {
|
||||
hasUsableCredentials?: (model: string) => Promise<boolean | null>;
|
||||
getActiveSyncedCatalog?: (provider: string) => Promise<VisionModelCatalog>;
|
||||
/**
|
||||
* (#12111) Per-connection model-lockout check, defaulting to the real
|
||||
* `accountFallback.isModelLocked`. Injectable for the same reason as
|
||||
* `hasUsableCredentials`: `node:test` has no supported ESM module-mocking.
|
||||
*/
|
||||
isModelLocked?: (provider: string, connectionId: string, model: string) => boolean;
|
||||
}
|
||||
|
||||
export interface VisionModelCatalog {
|
||||
@@ -150,6 +164,53 @@ function createCatalogModelPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* connectionIds worth probing for a `(providerAlias, modelId)` lockout check:
|
||||
* the provider's DB-known usable connections, plus any connectionId that
|
||||
* already has an active lockout entry for this provider (#12111) — a 404
|
||||
* lock (`accountFallback.lockModel`) can target a connectionId the DB-backed
|
||||
* lookup does not surface (e.g. it predates a reconnect, or the credential
|
||||
* check path a caller injected does not go through the same DB rows), and
|
||||
* missing it would silently fail the exclusion open.
|
||||
*/
|
||||
function collectLockoutConnectionIds(providerAlias: string): string[] {
|
||||
const canonicalProvider = resolveProviderId(providerAlias);
|
||||
return getAllModelLockouts()
|
||||
.filter((entry) => entry.provider === canonicalProvider)
|
||||
.map((entry) => entry.connectionId);
|
||||
}
|
||||
|
||||
/**
|
||||
* (#12111) True unless `modelId` is locked (a post-404 model lockout, see
|
||||
* `accountFallback.lockModel`) on every connection that could actually serve
|
||||
* it. `isModelLocked` is scoped per provider+connection+model, so a single
|
||||
* locked connection must not exclude a model that's still reachable through
|
||||
* another connection on the same provider — mirrors
|
||||
* `isConnectionEligibleForModel` in
|
||||
* open-sse/services/autoCombo/resilienceCandidateFilter.ts. Fails open (never
|
||||
* excludes) when nothing is known about the provider's connections, matching
|
||||
* `hasUsableCredentialsForModel`'s existing fail-open contract — this check
|
||||
* only narrows an already-credentialed candidate, it never widens the pool.
|
||||
*/
|
||||
async function isModelUsableGivenLockouts(
|
||||
providerAlias: string,
|
||||
modelId: string,
|
||||
deps: VisionBridgeRouterDeps
|
||||
): Promise<boolean> {
|
||||
const checkLocked = deps.isModelLocked ?? isModelLocked;
|
||||
const dbConnections = await getUsableConnectionsForModel(`${providerAlias}/${modelId}`);
|
||||
if (dbConnections === null) return true; // indeterminate credential store — fail open
|
||||
|
||||
const candidateIds = new Set(dbConnections.map((conn) => conn.id));
|
||||
for (const id of collectLockoutConnectionIds(providerAlias)) candidateIds.add(id);
|
||||
if (candidateIds.size === 0) return true; // nothing known about this provider's connections
|
||||
|
||||
for (const id of candidateIds) {
|
||||
if (!checkLocked(providerAlias, id, modelId)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
async function cachedModelRemainsAvailable(
|
||||
fullModelId: string,
|
||||
deps: VisionBridgeRouterDeps
|
||||
@@ -162,6 +223,8 @@ async function cachedModelRemainsAvailable(
|
||||
const registryModel = PROVIDER_MODELS[providerAlias]?.find((model) => model.id === modelId);
|
||||
if (!registryModel) return false;
|
||||
|
||||
if (!(await isModelUsableGivenLockouts(providerAlias, modelId, deps))) return false;
|
||||
|
||||
const catalog = await readActiveCatalog(providerAlias, deps);
|
||||
return createCatalogModelPredicate(providerAlias, catalog)(registryModel);
|
||||
}
|
||||
@@ -193,13 +256,27 @@ async function getVisionCapableModels(
|
||||
});
|
||||
if (visionModels.length === 0) return [];
|
||||
|
||||
const usableModels = (
|
||||
const credentialedModels = (
|
||||
await Promise.all(
|
||||
visionModels.map(async (model) =>
|
||||
(await checkCreds(`${providerAlias}/${model.id}`)) === false ? null : model
|
||||
)
|
||||
)
|
||||
).filter((model): model is (typeof visionModels)[number] => model !== null);
|
||||
if (credentialedModels.length === 0) return [];
|
||||
|
||||
// (#12111) A healthy provider connection does not mean every model on
|
||||
// it is servable: chatCore.ts locks one specific model for 120s on a
|
||||
// 404 while leaving the connection active, so the credential check
|
||||
// above never sees it. Drop only the models locked on every usable
|
||||
// connection for this provider.
|
||||
const usableModels = (
|
||||
await Promise.all(
|
||||
credentialedModels.map(async (model) =>
|
||||
(await isModelUsableGivenLockouts(providerAlias, model.id, deps)) ? model : null
|
||||
)
|
||||
)
|
||||
).filter((model): model is (typeof credentialedModels)[number] => model !== null);
|
||||
if (usableModels.length === 0) return [];
|
||||
|
||||
const catalog = await readActiveCatalog(providerAlias, deps);
|
||||
|
||||
@@ -1,214 +0,0 @@
|
||||
/**
|
||||
* src/lib/memory/__tests__/rerank-loopback-auth-12745.test.ts
|
||||
*
|
||||
* Regression guard for #12745 — applyRerank()'s internal loopback call to
|
||||
* /v1/rerank used to carry no credential, so with REQUIRE_API_KEY=true the
|
||||
* global authz proxy's clientApiPolicy would 401 it and rerank silently
|
||||
* degraded to unranked order (fail-open by design, so nothing ever surfaced
|
||||
* the failure).
|
||||
*
|
||||
* This file proves two things:
|
||||
* 1. The loopback fetch retrieval.ts's applyRerank() issues now carries a
|
||||
* real Authorization: Bearer <internal key> header (fixed by attaching
|
||||
* pickApiKeyForInternalUse() — the same internal-probe selector already
|
||||
* used by combo-health-check / cloud-sync-verify).
|
||||
* 2. That fix was NOT done by exempting /v1/rerank from auth: an
|
||||
* unauthenticated *external* request to /api/v1/rerank is still
|
||||
* rejected by clientApiPolicy when REQUIRE_API_KEY=true.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
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(), "omr-rerank-auth-12745-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.DISABLE_SQLITE_AUTO_BACKUP = "true";
|
||||
process.env.VECTOR_STORE_DISABLE_VEC = "true";
|
||||
|
||||
const INTERNAL_KEY = "sk-internal-test-key-12745";
|
||||
|
||||
vi.mock("../settings", () => ({
|
||||
getMemorySettings: async () => ({
|
||||
enabled: true,
|
||||
maxTokens: 2000,
|
||||
retentionDays: 30,
|
||||
strategy: "semantic",
|
||||
skillsEnabled: true,
|
||||
embeddingSource: "static",
|
||||
embeddingProviderModel: null,
|
||||
customBaseUrl: null,
|
||||
customModelId: null,
|
||||
transformersEnabled: false,
|
||||
staticEnabled: true,
|
||||
rerankEnabled: true,
|
||||
rerankProviderModel: "test-provider/test-rerank-model",
|
||||
vectorStore: "sqlite-vec",
|
||||
primaryBackend: "sqlite",
|
||||
fallbackBackends: [],
|
||||
backendConfigs: {},
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../embedding", () => ({
|
||||
resolveEmbeddingSource: () => ({
|
||||
source: "static",
|
||||
model: "static-hash-8",
|
||||
dimensions: 8,
|
||||
identity: "static",
|
||||
signature: "static-8",
|
||||
reason: "test: static embedding, no network",
|
||||
}),
|
||||
embed: async () => ({
|
||||
vector: new Float32Array([1, 0, 0, 0, 0, 0, 0, 0]),
|
||||
source: "static",
|
||||
model: "static-hash-8",
|
||||
dimensions: 8,
|
||||
latencyMs: 0,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../vectorStore", () => ({
|
||||
getVectorStore: () => ({
|
||||
ensureReady: async () => ({ ready: true, reason: "test" }),
|
||||
upsertVector: async () => undefined,
|
||||
deleteVector: async () => undefined,
|
||||
searchVector: async () => [
|
||||
{ memoryId: "rrk-auth-1", score: 0.91 },
|
||||
{ memoryId: "rrk-auth-2", score: 0.82 },
|
||||
],
|
||||
searchHybrid: async () => [],
|
||||
stats: async () => ({ rowCount: 2, needsReindex: 0, activeDim: 8 }),
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("../../db/apiKeys", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../../db/apiKeys")>();
|
||||
return {
|
||||
...actual,
|
||||
pickApiKeyForInternalUse: vi.fn(async () => INTERNAL_KEY),
|
||||
};
|
||||
});
|
||||
|
||||
const core = await import("../../db/core");
|
||||
const { retrievePreview } = await import("../retrieval");
|
||||
const { pickApiKeyForInternalUse } = await import("../../db/apiKeys");
|
||||
|
||||
function cleanupDb() {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
function insertMemory(apiKeyId: string, id: string, content: string) {
|
||||
const db = core.getDbInstance();
|
||||
db.prepare(
|
||||
`INSERT INTO memories (id, api_key_id, session_id, type, key, content, metadata, created_at, updated_at, expires_at)
|
||||
VALUES (?, ?, ?, 'factual', ?, ?, '{}', datetime('now'), datetime('now'), NULL)`
|
||||
).run(id, apiKeyId, "", `key-${id}`, content);
|
||||
}
|
||||
|
||||
let originalFetch: typeof globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
cleanupDb();
|
||||
originalFetch = globalThis.fetch;
|
||||
vi.mocked(pickApiKeyForInternalUse).mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
});
|
||||
|
||||
describe("#12745 — memory rerank loopback call authentication", () => {
|
||||
test("applyRerank()'s loopback fetch to /v1/rerank carries an internal Authorization bearer", async () => {
|
||||
insertMemory("api-rrk-auth", "rrk-auth-1", "The capital of France is Paris.");
|
||||
insertMemory("api-rrk-auth", "rrk-auth-2", "TypeScript is a superset of JavaScript.");
|
||||
|
||||
const calls: Array<{ url: string; headers: Record<string, string> }> = [];
|
||||
|
||||
globalThis.fetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url = typeof input === "string" ? input : input.toString();
|
||||
const headers: Record<string, string> = {};
|
||||
new Headers(init?.headers).forEach((value, key) => {
|
||||
headers[key.toLowerCase()] = value;
|
||||
});
|
||||
calls.push({ url, headers });
|
||||
|
||||
// Emulate the REAL clientApiPolicy behavior this loopback call hits in
|
||||
// production: reject without a bearer/x-api-key, accept a valid one.
|
||||
const hasCredential = Boolean(headers["authorization"] || headers["x-api-key"]);
|
||||
if (!hasCredential) {
|
||||
return new Response(JSON.stringify({ error: { message: "Authentication required" } }), {
|
||||
status: 401,
|
||||
});
|
||||
}
|
||||
return new Response(
|
||||
JSON.stringify({
|
||||
results: [
|
||||
{ index: 1, relevance_score: 0.95 },
|
||||
{ index: 0, relevance_score: 0.4 },
|
||||
],
|
||||
}),
|
||||
{ status: 200 }
|
||||
);
|
||||
}) as unknown as typeof globalThis.fetch;
|
||||
|
||||
const bundle = await retrievePreview("api-rrk-auth", "capital of France", {
|
||||
strategy: "semantic",
|
||||
maxTokens: 2000,
|
||||
limit: 5,
|
||||
});
|
||||
|
||||
expect(calls.length).toBeGreaterThan(0);
|
||||
const rerankCall = calls.find((c) => c.url.includes("/v1/rerank"));
|
||||
expect(rerankCall).toBeDefined();
|
||||
|
||||
const hasCredential = Boolean(
|
||||
rerankCall?.headers["authorization"] || rerankCall?.headers["x-api-key"]
|
||||
);
|
||||
expect(hasCredential).toBe(true);
|
||||
expect(rerankCall?.headers["authorization"]).toBe(`Bearer ${INTERNAL_KEY}`);
|
||||
|
||||
// Functional consequence: with a valid credential the rerank response is
|
||||
// actually honored (item order follows relevance_score) instead of
|
||||
// silently keeping pre-rerank vector-search order.
|
||||
expect(bundle.items[0]?.memory.id).toBe("rrk-auth-2");
|
||||
});
|
||||
|
||||
test("without a credential the same loopback call would still be 401'd (no auth bypass introduced)", async () => {
|
||||
insertMemory("api-rrk-noauth", "rrk-auth-1", "The capital of France is Paris.");
|
||||
insertMemory("api-rrk-noauth", "rrk-auth-2", "TypeScript is a superset of JavaScript.");
|
||||
|
||||
// Simulate the pre-fix condition: internal key selector finds nothing.
|
||||
vi.mocked(pickApiKeyForInternalUse).mockResolvedValueOnce(null);
|
||||
|
||||
let sawUnauthenticatedRerankCall = false;
|
||||
globalThis.fetch = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
const url = typeof input === "string" ? input : input.toString();
|
||||
const headers: Record<string, string> = {};
|
||||
new Headers(init?.headers).forEach((value, key) => {
|
||||
headers[key.toLowerCase()] = value;
|
||||
});
|
||||
const hasCredential = Boolean(headers["authorization"] || headers["x-api-key"]);
|
||||
if (url.includes("/v1/rerank") && !hasCredential) {
|
||||
sawUnauthenticatedRerankCall = true;
|
||||
return new Response(JSON.stringify({ error: { message: "Authentication required" } }), {
|
||||
status: 401,
|
||||
});
|
||||
}
|
||||
return new Response(JSON.stringify({ results: [] }), { status: 200 });
|
||||
}) as unknown as typeof globalThis.fetch;
|
||||
|
||||
const bundle = await retrievePreview("api-rrk-noauth", "capital of France", {
|
||||
strategy: "semantic",
|
||||
maxTokens: 2000,
|
||||
limit: 5,
|
||||
});
|
||||
|
||||
expect(sawUnauthenticatedRerankCall).toBe(true);
|
||||
// Fail-open by design: retrieval keeps working (unranked) rather than throwing.
|
||||
expect(bundle.items.length).toBe(2);
|
||||
});
|
||||
});
|
||||
@@ -12,8 +12,6 @@ import { getQdrantConfig, checkQdrantHealth, searchSemanticMemory } from "./qdra
|
||||
import type { MemoryEngineStatus } from "@/shared/schemas/memory";
|
||||
import { supportsFts5 } from "../db/migrationRunner";
|
||||
import type { SqliteAdapter } from "../db/adapters/types";
|
||||
import { pickApiKeyForInternalUse } from "../db/apiKeys";
|
||||
import { getRuntimePorts } from "../runtime/ports";
|
||||
import {
|
||||
estimateTokens,
|
||||
parseMetadata,
|
||||
@@ -145,29 +143,16 @@ function buildFtsRows(apiKeyId: string, config: FtsColConfig): MemoryRow[] {
|
||||
}
|
||||
}
|
||||
|
||||
// Loopback rerank URL — localhost only, never routed over the network. The port is
|
||||
// derived from the same runtime source every other internal self-call uses
|
||||
// (getRuntimePorts()/process.env.PORT — see src/lib/runtime/ports.ts), never hardcoded,
|
||||
// so this keeps working when an operator overrides PORT/API_PORT (#12745).
|
||||
function getRerankLoopbackUrl(): string {
|
||||
const { apiPort } = getRuntimePorts();
|
||||
// nosemgrep: javascript.lang.security.audit.non-literal-regexp.non-literal-regexp
|
||||
return `http://127.0.0.1:${apiPort}/v1/rerank`;
|
||||
}
|
||||
// Loopback rerank URL — localhost only, never routed over the network.
|
||||
// nosemgrep: javascript.lang.security.audit.non-literal-regexp.non-literal-regexp
|
||||
const RERANK_LOOPBACK_URL = "http://127.0.0.1:20128/v1/rerank";
|
||||
|
||||
/**
|
||||
* Apply reranking via /v1/rerank (loopback-only) if rerankEnabled + rerankProviderModel is set.
|
||||
* Returns reordered array (or original order on any error — rerank failure never fails retrieval).
|
||||
*
|
||||
* Auth note (#12745): /v1/rerank is a CLIENT_API route gated by clientApiPolicy — with
|
||||
* REQUIRE_API_KEY=true an unauthenticated loopback call gets 401'd by the same policy
|
||||
* that protects it from the outside, and this call used to send no credential at all,
|
||||
* silently degrading retrieval to unranked order. Attach a real, DB-backed API key
|
||||
* (the same internal-probe selector already used by combo-health-check / cloud-sync-verify,
|
||||
* see pickApiKeyForInternalUse()) as a Bearer token instead of exempting the route.
|
||||
*
|
||||
* Security note: the URL is a loopback address (127.0.0.1) — it never carries sensitive
|
||||
* data over a network link. HTTP is safe for loopback-only IPC.
|
||||
* Security note: the URL is a hardcoded loopback address (127.0.0.1:20128) — it never
|
||||
* carries sensitive data over a network link. HTTP is safe for loopback-only IPC.
|
||||
* nosemgrep: javascript.lang.security.detect-non-literal-url
|
||||
*/
|
||||
async function applyRerank<T extends { memory: Memory; score: number }>(
|
||||
@@ -186,14 +171,10 @@ async function applyRerank<T extends { memory: Memory; score: number }>(
|
||||
top_n: items.length,
|
||||
};
|
||||
|
||||
const internalKey = await pickApiKeyForInternalUse("internal-probe");
|
||||
const headers: Record<string, string> = { "content-type": "application/json" };
|
||||
if (internalKey) headers.authorization = `Bearer ${internalKey}`;
|
||||
|
||||
const res = await fetch(getRerankLoopbackUrl(), {
|
||||
const res = await fetch(RERANK_LOOPBACK_URL, {
|
||||
// nosemgrep: typescript.react.security.react-insecure-request.react-insecure-request
|
||||
method: "POST",
|
||||
headers,
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
signal: AbortSignal.timeout(5000),
|
||||
});
|
||||
|
||||
64
tests/unit/guardrails/visionBridge12111Repro.test.ts
Normal file
64
tests/unit/guardrails/visionBridge12111Repro.test.ts
Normal file
@@ -0,0 +1,64 @@
|
||||
/**
|
||||
* TDD repro for issue #12111: Vision Bridge auto-router can select a model
|
||||
* already locked after a 404.
|
||||
*
|
||||
* getVisionCapableModels() (src/lib/guardrails/visionBridgeRouter.ts) filters
|
||||
* candidates only on the registry vision flag and hasUsableCredentialsForModel
|
||||
* (connection-scoped). It never consults isModelLocked
|
||||
* (open-sse/services/accountFallback.ts), which the provider layer sets on a
|
||||
* 404 "model not found" (open-sse/handlers/chatCore.ts). This test locks a
|
||||
* vision-capable model exactly as chatCore.ts would after a 404, then asks
|
||||
* getBestVisionModel() for a pick while forcing every other vision-capable
|
||||
* provider to look uncredentialed (mirroring the reporter's setup: only one
|
||||
* provider connection is actually usable) -- the locked model must not win.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { getBestVisionModel, clearSelectionCache } =
|
||||
await import("../../../src/lib/guardrails/visionBridgeRouter.ts");
|
||||
const { lockModel, clearAllModelLockouts, isModelLocked } =
|
||||
await import("../../../open-sse/services/accountFallback.ts");
|
||||
|
||||
test.beforeEach(() => {
|
||||
clearSelectionCache();
|
||||
clearAllModelLockouts();
|
||||
});
|
||||
|
||||
test("getBestVisionModel must not select a model locked after a 404 (#12111)", async () => {
|
||||
const provider = "nvidia";
|
||||
const connectionId = "conn-nvidia-1";
|
||||
// The issue's original log line named "moonshotai/kimi-k2.6"; the registry
|
||||
// has since renamed that entry to "kimi-k3" (open-sse/config/providers/
|
||||
// registry/nvidia/index.ts) but it resolves the same way: it is the first
|
||||
// vision-capable nvidia model in registry order, so it is still the model
|
||||
// getBestVisionModel picks first when only nvidia is credentialed.
|
||||
const modelId = "moonshotai/kimi-k3";
|
||||
const fullModelId = `${provider}/${modelId}`;
|
||||
|
||||
// Reproduce the exact runtime event from the issue log line:
|
||||
// "[provider] Node <redacted> model not found (404) for <model>
|
||||
// - locking model for 120s (connection stays active)"
|
||||
lockModel(provider, connectionId, modelId, "not_found", 120_000);
|
||||
assert.equal(
|
||||
isModelLocked(provider, connectionId, modelId),
|
||||
true,
|
||||
"sanity check: accountFallback must report the model as locked"
|
||||
);
|
||||
|
||||
// Only the nvidia provider looks credentialed -- mirrors the reporter's
|
||||
// setup where the NVIDIA connection tests 200 all day (connection-scoped
|
||||
// credential check passes) but the specific model 404s for the account.
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{ hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
|
||||
);
|
||||
|
||||
assert.notEqual(
|
||||
model,
|
||||
fullModelId,
|
||||
"getBestVisionModel selected a model that accountFallback has locked after " +
|
||||
"a 404 -- getVisionCapableModels() never consults isModelLocked " +
|
||||
"(src/lib/guardrails/visionBridgeRouter.ts)"
|
||||
);
|
||||
});
|
||||
@@ -25,6 +25,10 @@ const {
|
||||
getLatencyStats,
|
||||
} = await import("../../../src/lib/guardrails/visionBridgeRouter.ts");
|
||||
const { PROVIDER_MODELS } = await import("../../../open-sse/config/providerModels.ts");
|
||||
const { lockModel, clearAllModelLockouts, isModelLocked } =
|
||||
await import("../../../open-sse/services/accountFallback.ts");
|
||||
const { createProviderConnection, deleteProviderConnectionsByProvider } =
|
||||
await import("../../../src/lib/db/providers.ts");
|
||||
type VisionBridgeRouterDepsT =
|
||||
import("../../../src/lib/guardrails/visionBridgeRouter.ts").VisionBridgeRouterDeps;
|
||||
|
||||
@@ -294,3 +298,103 @@ test("getLatencyStats — should return latency statistics", () => {
|
||||
assert.equal(stats["model-a"].avg, 110);
|
||||
assert.equal(stats["model-a"].successRate, 1);
|
||||
});
|
||||
|
||||
// ── model-lockout exclusion (#12111) ────────────────────────────────────────
|
||||
// getVisionCapableModels() must consult accountFallback's per-connection
|
||||
// model lockout (set by chatCore.ts on a 404) in addition to the credential
|
||||
// check, and drop a model only when every usable connection has it locked —
|
||||
// see tests/unit/guardrails/visionBridge12111Repro.test.ts for the original
|
||||
// end-to-end reproduction against the exact reporter setup. These cases
|
||||
// exercise the same production code path (getBestVisionModel →
|
||||
// getVisionCapableModels → isModelUsableGivenLockouts) with a synthetic
|
||||
// registry entry, following the pattern in "accepts a registry model whose
|
||||
// liveCatalogIds match upstream" above.
|
||||
|
||||
test("getBestVisionModel — excludes a model locked on its only usable connection (#12111)", async () => {
|
||||
const provider = "__vision-bridge-lockout-test-1__";
|
||||
const connectionId = "conn-1";
|
||||
const modelId = "synthetic-vision-model";
|
||||
PROVIDER_MODELS[provider] = [
|
||||
{ id: modelId, name: "Synthetic Vision Model", supportsVision: true },
|
||||
];
|
||||
clearAllModelLockouts();
|
||||
lockModel(provider, connectionId, modelId, "not_found", 120_000);
|
||||
|
||||
try {
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{ hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
|
||||
);
|
||||
assert.notEqual(model, `${provider}/${modelId}`);
|
||||
} finally {
|
||||
delete PROVIDER_MODELS[provider];
|
||||
clearAllModelLockouts();
|
||||
}
|
||||
});
|
||||
|
||||
test("getBestVisionModel — keeps a model locked on one connection while a second connection stays usable (#12111)", async () => {
|
||||
const provider = "__vision-bridge-lockout-test-2__";
|
||||
const modelId = "synthetic-vision-model";
|
||||
PROVIDER_MODELS[provider] = [
|
||||
{ id: modelId, name: "Synthetic Vision Model", supportsVision: true },
|
||||
];
|
||||
clearAllModelLockouts();
|
||||
|
||||
const lockedConn = await createProviderConnection({
|
||||
provider,
|
||||
authType: "apikey",
|
||||
apiKey: "sk-test-locked",
|
||||
});
|
||||
const openConn = await createProviderConnection({
|
||||
provider,
|
||||
authType: "apikey",
|
||||
apiKey: "sk-test-open",
|
||||
});
|
||||
lockModel(provider, (lockedConn as { id: string }).id, modelId, "not_found", 120_000);
|
||||
// Sanity: the OTHER connection must not itself be locked.
|
||||
assert.equal(isModelLocked(provider, (openConn as { id: string }).id, modelId), false);
|
||||
|
||||
try {
|
||||
const model = await getBestVisionModel(
|
||||
{},
|
||||
{ hasUsableCredentials: async (id) => id.startsWith(`${provider}/`) }
|
||||
);
|
||||
assert.equal(
|
||||
model,
|
||||
`${provider}/${modelId}`,
|
||||
"a model locked on only ONE of two usable connections must stay selectable"
|
||||
);
|
||||
} finally {
|
||||
delete PROVIDER_MODELS[provider];
|
||||
clearAllModelLockouts();
|
||||
await deleteProviderConnectionsByProvider(provider);
|
||||
}
|
||||
});
|
||||
|
||||
test("getBestVisionModel — drops a cached selection once it becomes locked mid-window (#12111)", async () => {
|
||||
const provider = "__vision-bridge-lockout-test-3__";
|
||||
const connectionId = "conn-1";
|
||||
const modelId = "synthetic-vision-model";
|
||||
PROVIDER_MODELS[provider] = [
|
||||
{ id: modelId, name: "Synthetic Vision Model", supportsVision: true },
|
||||
];
|
||||
clearAllModelLockouts();
|
||||
const deps = { hasUsableCredentials: async (id: string) => id.startsWith(`${provider}/`) };
|
||||
|
||||
try {
|
||||
// First call populates the 60s selection cache with the only candidate.
|
||||
assert.equal(await getBestVisionModel({}, deps), `${provider}/${modelId}`);
|
||||
|
||||
// The model 404s and gets locked mid-cache-window, exactly like chatCore.ts.
|
||||
lockModel(provider, connectionId, modelId, "not_found", 120_000);
|
||||
|
||||
// A cache hit that never re-validates lockouts would keep returning the
|
||||
// now-locked model for up to 60s of further failing requests (the
|
||||
// reporter's complaint); it must fall through to "no usable candidate".
|
||||
assert.equal(await getBestVisionModel({}, deps), null);
|
||||
} finally {
|
||||
delete PROVIDER_MODELS[provider];
|
||||
clearAllModelLockouts();
|
||||
clearSelectionCache();
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user