mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 02:42:24 +03:00
fix(services): separate CLIProxyAPI health and model auth (#11811)
Obrigado! Validado em lote combinado (8 PRs, release/v3.8.51):
- Separação correta de responsabilidades: probe de saúde via `/healthz` público vs. autenticação de `/v1/models` com `settings.cliproxyapi_api_key` dedicada; `MANAGEMENT_PASSWORD` mantida estritamente no plano de gestão do CLIProxyAPI.
- Evidência RED→GREEN documentada e reproduzida: `tests/unit/services/cliproxy-health-model-auth.test.ts` — verde no lote.
- `⚠️ base-red inherited: #11449` reconhecido — não é causado por esta PR.
- Gates estáticos do lote OK.
This commit is contained in:
1
changelog.d/fixes/11811-cliproxy-health-model-auth.md
Normal file
1
changelog.d/fixes/11811-cliproxy-health-model-auth.md
Normal file
@@ -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))
|
||||
@@ -20,7 +20,7 @@ export async function getOrInitSupervisor(): Promise<ServiceSupervisor> {
|
||||
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,
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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);
|
||||
|
||||
150
tests/unit/services/cliproxy-health-model-auth.test.ts
Normal file
150
tests/unit/services/cliproxy-health-model-auth.test.ts
Normal file
@@ -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<string | null> = [];
|
||||
|
||||
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<void>((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<void>((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"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user