mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-11 17:32:35 +03:00
Compare commits
1 Commits
fix/12745-
...
fix/12783-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a37c39ef73 |
@@ -35,16 +35,24 @@ export function resolveOpencodeTarget(opts = {}) {
|
||||
baseUrl = `http://localhost:${Number(opts.port ?? process.env.PORT ?? 20128) || 20128}`;
|
||||
}
|
||||
|
||||
// Precedence: explicit --api-key flag > OMNIROUTE_API_KEY env var > active
|
||||
// context's management token. A context's accessToken/apiKey is a CLI
|
||||
// management credential (oma_live_...) with no /v1/* inference scope — it
|
||||
// must never silently outrank a real inference key the caller supplied
|
||||
// either as a flag or via the ambient env var (mirrors the explicit >
|
||||
// ambient-env > context precedence documented in bin/cli/api.mjs's
|
||||
// buildHeaders()). Only fall back to the context token when neither an
|
||||
// explicit flag nor the env var is set.
|
||||
let apiKey = opts.apiKey ?? opts["api-key"];
|
||||
if (!apiKey) apiKey = process.env.OMNIROUTE_API_KEY || "";
|
||||
if (!apiKey) {
|
||||
try {
|
||||
const c = resolveActiveContext(opts.context ?? process.env.OMNIROUTE_CONTEXT);
|
||||
apiKey = c?.accessToken || c?.apiKey;
|
||||
apiKey = c?.accessToken || c?.apiKey || "";
|
||||
} catch {
|
||||
/* no context auth */
|
||||
}
|
||||
}
|
||||
if (!apiKey) apiKey = process.env.OMNIROUTE_API_KEY || "";
|
||||
return { baseUrl: baseUrl.replace(/\/+$/, ""), apiKey };
|
||||
}
|
||||
|
||||
@@ -177,8 +185,17 @@ export function registerSetupOpencode(program) {
|
||||
"--allow-container-write",
|
||||
"Write even when the target is inside a container and not mounted from the host"
|
||||
)
|
||||
.action(async (opts) => {
|
||||
const code = await runSetupOpencodeCommand(opts);
|
||||
.action(async (opts, cmd) => {
|
||||
// Commander parses the ancestor program's own global --api-key option
|
||||
// (bin/cli/program.mjs, bound to .env("OMNIROUTE_API_KEY")) against any
|
||||
// occurrence of the flag in argv, so it wins the value even when the
|
||||
// user typed --api-key AFTER `setup-opencode` — this local option's own
|
||||
// `opts.apiKey` never sees it. cmd.optsWithGlobals() resolves to the
|
||||
// correct value either way ("globals overwrite locals" is exactly the
|
||||
// outcome we want here, since the global option is where the value
|
||||
// always actually lands).
|
||||
const resolvedOpts = { ...opts, apiKey: cmd.optsWithGlobals().apiKey ?? opts.apiKey };
|
||||
const code = await runSetupOpencodeCommand(resolvedOpts);
|
||||
if (code !== 0) process.exit(code);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
@@ -0,0 +1 @@
|
||||
- fix(cli): setup-opencode no longer sends an active context's management token to `/v1/models` when `--api-key`/`OMNIROUTE_API_KEY` is supplied — an explicit flag or the env var now always outranks the context's token, and the flag itself is no longer swallowed by the parent program's global `--api-key` option (#12783)
|
||||
@@ -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),
|
||||
});
|
||||
|
||||
121
tests/unit/repro-12783-setup-opencode-apikey.test.ts
Normal file
121
tests/unit/repro-12783-setup-opencode-apikey.test.ts
Normal file
@@ -0,0 +1,121 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { test } from "node:test";
|
||||
|
||||
import { resolveOpencodeTarget } from "../../bin/cli/commands/setup-opencode.mjs";
|
||||
|
||||
/** Point OMNIROUTE_CONTEXT config resolution at an isolated, throwaway DATA_DIR. */
|
||||
function withIsolatedContext(contextConfig, fn) {
|
||||
const dir = mkdtempSync(join(tmpdir(), "omniroute-setup-opencode-test-"));
|
||||
const originalDataDir = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = dir;
|
||||
writeFileSync(
|
||||
join(dir, "config.json"),
|
||||
JSON.stringify({
|
||||
version: 1,
|
||||
currentContext: "remote",
|
||||
contexts: { remote: contextConfig },
|
||||
})
|
||||
);
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (originalDataDir === undefined) delete process.env.DATA_DIR;
|
||||
else process.env.DATA_DIR = originalDataDir;
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
function withEnvApiKey(value, fn) {
|
||||
const original = process.env.OMNIROUTE_API_KEY;
|
||||
if (value === undefined) delete process.env.OMNIROUTE_API_KEY;
|
||||
else process.env.OMNIROUTE_API_KEY = value;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (original === undefined) delete process.env.OMNIROUTE_API_KEY;
|
||||
else process.env.OMNIROUTE_API_KEY = original;
|
||||
}
|
||||
}
|
||||
|
||||
test("setup-opencode: --api-key typed AFTER the subcommand name is not stolen by the parent program's global option", async () => {
|
||||
const { createProgram } = await import("../../bin/cli/program.mjs");
|
||||
const program = createProgram();
|
||||
const setupOpencode = program.commands.find((c) => c.name() === "setup-opencode");
|
||||
assert.ok(setupOpencode, "setup-opencode subcommand must be registered");
|
||||
|
||||
let capturedApiKey;
|
||||
setupOpencode._actionHandler = null; // avoid the real network-calling action
|
||||
setupOpencode.action((opts, cmd) => {
|
||||
capturedApiKey = cmd.optsWithGlobals().apiKey ?? opts.apiKey;
|
||||
});
|
||||
|
||||
await program.parseAsync(
|
||||
[
|
||||
"node",
|
||||
"omniroute",
|
||||
"setup-opencode",
|
||||
"--remote",
|
||||
"http://100.64.0.1:20128",
|
||||
"--api-key",
|
||||
"sk-TESTKEY123",
|
||||
],
|
||||
{ from: "node" }
|
||||
);
|
||||
|
||||
assert.equal(
|
||||
capturedApiKey,
|
||||
"sk-TESTKEY123",
|
||||
"the CLI-supplied --api-key value must reach the setup-opencode action handler"
|
||||
);
|
||||
});
|
||||
|
||||
test("resolveOpencodeTarget: (a) explicit --api-key flag wins over an active context's management token", () => {
|
||||
withEnvApiKey(undefined, () => {
|
||||
withIsolatedContext(
|
||||
{ baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" },
|
||||
() => {
|
||||
const { apiKey } = resolveOpencodeTarget({ apiKey: "sk-FLAG", context: "remote" });
|
||||
assert.equal(apiKey, "sk-FLAG");
|
||||
}
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test("resolveOpencodeTarget: (b) OMNIROUTE_API_KEY env wins over an active context's management token when no flag is passed", () => {
|
||||
withEnvApiKey("sk-ENVKEY", () => {
|
||||
withIsolatedContext(
|
||||
{ baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" },
|
||||
() => {
|
||||
const { apiKey } = resolveOpencodeTarget({ context: "remote" });
|
||||
assert.equal(apiKey, "sk-ENVKEY");
|
||||
}
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test("resolveOpencodeTarget: (c) the context's token is used only when neither a flag nor the env var is set", () => {
|
||||
withEnvApiKey(undefined, () => {
|
||||
withIsolatedContext(
|
||||
{ baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" },
|
||||
() => {
|
||||
const { apiKey } = resolveOpencodeTarget({ context: "remote" });
|
||||
assert.equal(apiKey, "oma_live_CONTEXT_TOKEN");
|
||||
}
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test("resolveOpencodeTarget: falls back to '' when neither a flag, env var, nor a resolvable context is present", () => {
|
||||
withEnvApiKey(undefined, () => {
|
||||
withIsolatedContext({ baseUrl: "http://100.64.0.1:20128" }, () => {
|
||||
const { apiKey } = resolveOpencodeTarget({
|
||||
remote: "http://100.64.0.1:20128",
|
||||
context: "__no-such-context__",
|
||||
});
|
||||
assert.equal(apiKey, "");
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user