diff --git a/changelog.d/fixes/11811-cliproxy-health-model-auth.md b/changelog.d/fixes/11811-cliproxy-health-model-auth.md new file mode 100644 index 0000000000..e566462f2c --- /dev/null +++ b/changelog.d/fixes/11811-cliproxy-health-model-auth.md @@ -0,0 +1 @@ +- **fix(services):** embedded CLIProxyAPI lifecycle checks now use public `/healthz`, while model discovery uses the configured dedicated data-plane API key instead of the management password ([#11811](https://github.com/diegosouzapw/OmniRoute/pull/11811)) diff --git a/src/app/api/services/cliproxy/_lib.ts b/src/app/api/services/cliproxy/_lib.ts index 1009c316b8..e43a0976c0 100644 --- a/src/app/api/services/cliproxy/_lib.ts +++ b/src/app/api/services/cliproxy/_lib.ts @@ -20,7 +20,7 @@ export async function getOrInitSupervisor(): Promise { tool: TOOL, port: PORT, spawnArgs: () => resolveSpawnArgs(PORT, managementKey), - healthUrl: () => `http://127.0.0.1:${PORT}/v1/models`, + healthUrl: () => `http://127.0.0.1:${PORT}/healthz`, healthIntervalMs: 5_000, stopTimeoutMs: 15_000, logsBufferBytes: 5_242_880, diff --git a/src/lib/services/bootstrap.ts b/src/lib/services/bootstrap.ts index 559afeae7e..df58e66d26 100644 --- a/src/lib/services/bootstrap.ts +++ b/src/lib/services/bootstrap.ts @@ -1,5 +1,7 @@ import { getVersionManagerTool } from "@/lib/db/versionManager"; +import { getSettings } from "@/lib/db/settings"; import { markAllUnavailable } from "@/lib/db/serviceModels"; +import { resolveDedicatedCliproxyapiApiKey } from "@omniroute/open-sse/handlers/chatCore/cliproxyapiCredentials"; import { registerSupervisor, getSupervisor } from "./registry"; import { ServiceSupervisor } from "./ServiceSupervisor"; import { resolveSpawnArgs as nineRouterSpawnArgs } from "./installers/ninerouter"; @@ -8,10 +10,7 @@ import { CLIPROXY_DEFAULT_PORT, } from "./installers/cliproxy"; import { resolveSpawnArgs as muxSpawnArgs, MUX_DEFAULT_PORT } from "./installers/mux"; -import { - resolveSpawnArgs as bifrostSpawnArgs, - BIFROST_DEFAULT_PORT, -} from "./installers/bifrost"; +import { resolveSpawnArgs as bifrostSpawnArgs, BIFROST_DEFAULT_PORT } from "./installers/bifrost"; import { resolveSpawnArgs as darioSpawnArgs, DARIO_DEFAULT_PORT } from "./installers/dario"; import { getOrCreateApiKey } from "./apiKey"; import { scheduleServiceModelSync, stopServiceModelSync } from "./modelSync"; @@ -59,7 +58,7 @@ const SERVICES: ServiceEntry[] = [ { tool: "cliproxy", port: CLIPROXY_PORT, - healthPath: "/v1/models", + healthPath: "/healthz", healthIntervalMs: 5_000, stopTimeoutMs: 15_000, logsBufferBytes: 5_242_880, @@ -128,6 +127,11 @@ export async function bootstrapEmbeddedServices(): Promise { const apiKey = cfg.needsApiKey ? await getOrCreateApiKey(cfg.tool).catch(() => "placeholder") : ""; + // CLIProxyAPI's generated key is management-only; /v1/models uses its dedicated data-plane key. + const modelSyncApiKey = + cfg.tool === "cliproxy" + ? (resolveDedicatedCliproxyapiApiKey(await getSettings()) ?? "") + : apiKey; const supervisor = new ServiceSupervisor({ tool: cfg.tool, @@ -148,7 +152,7 @@ export async function bootstrapEmbeddedServices(): Promise { const baseUrl = `http://127.0.0.1:${cfg.port}`; supervisor.on("stateChange", (status: ServiceStatus) => { if (status.state === "running") { - scheduleServiceModelSync(cfg.tool, baseUrl, apiKey); + scheduleServiceModelSync(cfg.tool, baseUrl, modelSyncApiKey); } else if (status.state === "stopped" || status.state === "error") { stopServiceModelSync(cfg.tool); markAllUnavailable(cfg.tool); diff --git a/tests/unit/services/cliproxy-health-model-auth.test.ts b/tests/unit/services/cliproxy-health-model-auth.test.ts new file mode 100644 index 0000000000..3c5c6e2d52 --- /dev/null +++ b/tests/unit/services/cliproxy-health-model-auth.test.ts @@ -0,0 +1,150 @@ +/** + * Regression for #11803: embedded CLIProxyAPI health and model-discovery credentials. + * + * The fake service deliberately separates its public liveness endpoint from + * its authenticated data plane: + * - GET /healthz is public. + * - GET /v1/models accepts only the operator-configured dedicated API key. + * + * CLIProxyAPI's MANAGEMENT_PASSWORD is a control-plane credential and must + * never be reused for /v1/models. + */ + +import { after, test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cliproxy-auth-")); +const DEDICATED_API_KEY = "cpa-dedicated-data-plane-key"; + +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.NODE_ENV = "test"; +process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; +process.env.STORAGE_ENCRYPTION_KEY = "cliproxy-health-model-auth-test-key"; +process.env.OMNIROUTE_ADOPT_EXISTING_SERVICE = "1"; + +const seenPaths: string[] = []; +const modelAuthorizationHeaders: Array = []; + +const fakeCliproxy = http.createServer((req, res) => { + const requestPath = req.url ?? "/"; + seenPaths.push(requestPath); + + if (requestPath === "/healthz") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ status: "ok" })); + return; + } + + if (requestPath === "/v1/models") { + const authorization = req.headers.authorization ?? null; + modelAuthorizationHeaders.push(authorization); + if (authorization !== `Bearer ${DEDICATED_API_KEY}`) { + res.writeHead(401, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ error: "invalid API key" })); + return; + } + + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ data: [{ id: "fake-cpa-model", object: "model" }] })); + return; + } + + res.writeHead(404).end(); +}); + +await new Promise((resolve, reject) => { + fakeCliproxy.once("error", reject); + fakeCliproxy.listen(0, "127.0.0.1", () => resolve()); +}); + +const address = fakeCliproxy.address(); +assert.ok(address && typeof address === "object"); +process.env.CLIPROXYAPI_PORT = String(address.port); + +const core = await import("../../../src/lib/db/core.ts"); +const settingsDb = await import("../../../src/lib/db/settings.ts"); +const versionManager = await import("../../../src/lib/db/versionManager.ts"); +const { decrypt } = await import("../../../src/lib/db/encryption.ts"); +const { bootstrapEmbeddedServices } = await import("../../../src/lib/services/bootstrap.ts"); +const { getSupervisor, unregisterSupervisor } = + await import("../../../src/lib/services/registry.ts"); +const { getOrInitSupervisor } = await import("../../../src/app/api/services/cliproxy/_lib.ts"); +const { getServiceModels } = await import("../../../src/lib/db/serviceModels.ts"); +const { stopServiceModelSync } = await import("../../../src/lib/services/modelSync.ts"); + +await versionManager.upsertVersionManagerTool({ + tool: "cliproxy", + installedVersion: "test", + status: "stopped", + port: address.port, +}); +await settingsDb.updateSettings({ cliproxyapi_api_key: DEDICATED_API_KEY }); + +after(async () => { + stopServiceModelSync("cliproxy"); + const supervisor = getSupervisor("cliproxy"); + if (supervisor) await supervisor.stop(); + unregisterSupervisor("cliproxy"); + await new Promise((resolve) => fakeCliproxy.close(() => resolve())); + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("embedded CLIProxyAPI uses public health and dedicated model credentials", async () => { + await bootstrapEmbeddedServices(); + + const supervisor = getSupervisor("cliproxy"); + assert.ok(supervisor, "bootstrap must register the installed CLIProxyAPI service"); + + const serviceRow = await versionManager.getServiceRow("cliproxy"); + const managementKey = decrypt(serviceRow?.apiKey); + assert.ok(managementKey, "bootstrap must create the separate management credential"); + assert.notEqual( + managementKey, + DEDICATED_API_KEY, + "the management and data-plane credentials must remain distinct" + ); + + const status = await supervisor.start(); + assert.equal(status.state, "running", "the public /healthz probe must accept the fake service"); + + const deadline = Date.now() + 3_000; + while (modelAuthorizationHeaders.length === 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + + assert.ok(seenPaths.includes("/healthz"), "embedded health checks must use public /healthz"); + assert.deepEqual( + modelAuthorizationHeaders, + [`Bearer ${DEDICATED_API_KEY}`], + "/v1/models must receive only settings.cliproxyapi_api_key" + ); + assert.ok( + getServiceModels("cliproxy").some((model) => model.id === "cliproxy/fake-cpa-model"), + "the authenticated discovery response must be persisted" + ); + + stopServiceModelSync("cliproxy"); + await supervisor.stop(); + unregisterSupervisor("cliproxy"); + seenPaths.length = 0; + modelAuthorizationHeaders.length = 0; + + const onDemandSupervisor = await getOrInitSupervisor(); + const onDemandStatus = await onDemandSupervisor.start(); + assert.equal( + onDemandStatus.state, + "running", + "the on-demand route supervisor must also use public /healthz" + ); + assert.ok(seenPaths.includes("/healthz")); + assert.equal( + modelAuthorizationHeaders.length, + 0, + "the on-demand health probe must not call authenticated /v1/models" + ); +});