diff --git a/changelog.d/fixes/7645-cliproxyapi-credential.md b/changelog.d/fixes/7645-cliproxyapi-credential.md new file mode 100644 index 0000000000..ec1ad8ecfa --- /dev/null +++ b/changelog.d/fixes/7645-cliproxyapi-credential.md @@ -0,0 +1 @@ +- fix(sse): route CLIProxyAPI fallback/passthrough legs through a dedicated `cliproxyapi_api_key` credential instead of the failed native provider's own key (#7645) diff --git a/open-sse/handlers/chatCore/cliproxyapiCredentials.ts b/open-sse/handlers/chatCore/cliproxyapiCredentials.ts new file mode 100644 index 0000000000..4789afb62c --- /dev/null +++ b/open-sse/handlers/chatCore/cliproxyapiCredentials.ts @@ -0,0 +1,76 @@ +/** + * CLIProxyAPI dedicated-credential resolution (#7645). + * + * CLIProxyAPI requires its own separately-configured `api-keys:` credential + * and rejects any other token with 401. Before this fix, both the direct + * `mode: "cliproxyapi"` passthrough leg and the `mode: "fallback"` retry leg + * (`open-sse/handlers/chatCore/executorProxy.ts::resolveExecutorWithProxy`) + * reused the resolved connection's own credentials — the native provider's + * key — as the Authorization header sent to CLIProxyAPI, making the fallback + * path a permanent no-op for every provider configured this way. + * + * This module resolves and applies the dedicated `cliproxyapi_api_key` + * setting at the executor boundary, so `CliproxyapiExecutor` itself stays + * credential-source-agnostic (it just uses whatever `credentials` it's + * handed — see `buildHeaders()`). + */ + +import type { ProviderCredentials } from "../../executors/base.ts"; + +type ExecutorInput = { + credentials: ProviderCredentials; + [key: string]: unknown; +}; + +type ExecutorLike = { + execute: (input: ExecutorInput) => Promise; + [key: string]: unknown; +}; + +/** + * Reads the dedicated CLIProxyAPI key out of a settings blob (as returned by + * `getCachedSettings()`), trimmed and normalized to `null` when absent/blank. + */ +export function resolveDedicatedCliproxyapiApiKey( + settings: Record | null | undefined +): string | null { + const raw = settings?.cliproxyapi_api_key; + return typeof raw === "string" && raw.trim() ? raw.trim() : null; +} + +/** + * Builds the credentials to use for a CLIProxyAPI-bound request. When a + * dedicated key is configured it always wins — CLIProxyAPI is a single + * shared instance serving every provider, so the resolved connection's own + * (provider-specific, and possibly already-failed) credential is never the + * right token for it. Falls back to the connection's own credentials only + * when no dedicated key is configured, preserving the pre-existing behavior + * for operators who previously worked around this by pasting a valid + * CLIProxyAPI key into the connection's own `apiKey` field. + */ +export function resolveCliproxyapiCredentials( + connectionCredentials: ProviderCredentials, + dedicatedApiKey: string | null +): ProviderCredentials { + if (!dedicatedApiKey) return connectionCredentials; + return { ...connectionCredentials, apiKey: dedicatedApiKey, accessToken: undefined }; +} + +/** + * Wraps an executor so every `execute()` call is routed with the dedicated + * CLIProxyAPI credential substituted in when one is configured. No-op + * wrapper when no dedicated key is set (returns the executor unchanged). + */ +export function wrapExecutorWithCliproxyapiCredentials( + executor: T, + dedicatedApiKey: string | null +): T { + if (!dedicatedApiKey) return executor; + const wrapped = Object.create(executor) as T; + wrapped.execute = (input: ExecutorInput) => + executor.execute({ + ...input, + credentials: resolveCliproxyapiCredentials(input.credentials, dedicatedApiKey), + }); + return wrapped; +} diff --git a/open-sse/handlers/chatCore/executorProxy.ts b/open-sse/handlers/chatCore/executorProxy.ts index 870c1bf00d..c791e9c5a4 100644 --- a/open-sse/handlers/chatCore/executorProxy.ts +++ b/open-sse/handlers/chatCore/executorProxy.ts @@ -14,6 +14,10 @@ import { isCliproxyapiDeepModeEnabled } from "../../executors/cliproxyapi.ts"; import { getCachedSettings } from "@/lib/db/readCache"; import { getUpstreamProxyConfigCached } from "./comboContextCache.ts"; import { wrapExecutorWithCliproxyapiModelMapping } from "./cliproxyModelMapping.ts"; +import { + resolveDedicatedCliproxyapiApiKey, + wrapExecutorWithCliproxyapiCredentials, +} from "./cliproxyapiCredentials.ts"; type LoggerLike = | { @@ -24,6 +28,40 @@ type LoggerLike = | null | undefined; +const DEFAULT_FALLBACK_CODES = [429, 500, 502, 503, 504]; + +function parseFallbackCodes(raw: unknown): number[] | null { + if (typeof raw !== "string" || !raw.trim()) return null; + const parsed = raw + .split(",") + .map((s) => Number.parseInt(s.trim(), 10)) + .filter((n) => !Number.isNaN(n)); + return parsed.length > 0 ? parsed : null; +} + +/** + * Reads the CLIProxyAPI-related settings shared by both the direct + * `mode: "cliproxyapi"` passthrough leg and the `mode: "fallback"` retry leg: + * the custom fallback status codes and the dedicated credential (#7645). + * Falls back to defaults / no dedicated key on any read failure. + */ +async function loadCliproxyapiSettings(): Promise<{ + fallbackCodes: number[]; + dedicatedApiKey: string | null; +}> { + try { + const allSettings = await getCachedSettings(); + return { + fallbackCodes: parseFallbackCodes(allSettings.cliproxyapi_fallback_codes) ?? [ + ...DEFAULT_FALLBACK_CODES, + ], + dedicatedApiKey: resolveDedicatedCliproxyapiApiKey(allSettings), + }; + } catch { + return { fallbackCodes: [...DEFAULT_FALLBACK_CODES], dedicatedApiKey: null }; + } +} + export async function resolveExecutorWithProxy( prov: string, log?: LoggerLike, @@ -48,9 +86,10 @@ export async function resolveExecutorWithProxy( if (cfg.mode === "cliproxyapi") { log?.info?.("UPSTREAM_PROXY", `${prov} routed through CLIProxyAPI (passthrough)`); - return wrapExecutorWithCliproxyapiModelMapping( - getExecutor("cliproxyapi"), - cfg.cliproxyapiModelMapping + const { dedicatedApiKey } = await loadCliproxyapiSettings(); + return wrapExecutorWithCliproxyapiCredentials( + wrapExecutorWithCliproxyapiModelMapping(getExecutor("cliproxyapi"), cfg.cliproxyapiModelMapping), + dedicatedApiKey ); } @@ -58,28 +97,13 @@ export async function resolveExecutorWithProxy( // The model mapping applies only to the CLIProxyAPI retry leg (proxyExec) — the // native leg must keep seeing the original, unmapped model. const nativeExec = getExecutor(prov); - const proxyExec = wrapExecutorWithCliproxyapiModelMapping( - getExecutor("cliproxyapi"), - cfg.cliproxyapiModelMapping + const { fallbackCodes, dedicatedApiKey } = await loadCliproxyapiSettings(); + // #7645: the CLIProxyAPI retry leg must authenticate with the dedicated + // key, never the native provider's own (already-failed) credential. + const proxyExec = wrapExecutorWithCliproxyapiCredentials( + wrapExecutorWithCliproxyapiModelMapping(getExecutor("cliproxyapi"), cfg.cliproxyapiModelMapping), + dedicatedApiKey ); - - // Read custom fallback codes from settings. Default: 5xx + 429 + network errors. - let fallbackCodes: number[] = [429, 500, 502, 503, 504]; - try { - const allSettings = await getCachedSettings(); - if ( - typeof allSettings.cliproxyapi_fallback_codes === "string" && - allSettings.cliproxyapi_fallback_codes.trim() - ) { - const parsed = allSettings.cliproxyapi_fallback_codes - .split(",") - .map((s: string) => Number.parseInt(s.trim(), 10)) - .filter((n: number) => !Number.isNaN(n)); - if (parsed.length > 0) fallbackCodes = parsed; - } - } catch { - /* use defaults */ - } const isRetryableStatus = (s: number) => fallbackCodes.includes(s) || s === 0; const wrapper = Object.create(nativeExec); diff --git a/src/shared/validation/settingsSchemas.ts b/src/shared/validation/settingsSchemas.ts index 542188c776..ef5fda94cf 100644 --- a/src/shared/validation/settingsSchemas.ts +++ b/src/shared/validation/settingsSchemas.ts @@ -322,6 +322,11 @@ export const updateSettingsSchema = z.object({ cliproxyapi_fallback_enabled: z.boolean().optional(), cliproxyapi_url: z.string().url().max(500).optional(), cliproxyapi_fallback_codes: z.string().max(200).optional(), + // #7645: dedicated CLIProxyAPI credential. CLIProxyAPI requires its own + // separately-configured `api-keys:` credential and rejects any other token + // with 401 — without this field, the fallback/passthrough legs had no way + // to authenticate except by reusing the (incompatible) native provider key. + cliproxyapi_api_key: z.string().max(500).optional(), // CLIProxyAPI model mapping (Record) cliproxyapi_model_mapping: z.record(z.string(), z.string()).optional(), // Model lockout settings diff --git a/tests/unit/cliproxyapi-dedicated-credential-7645.test.ts b/tests/unit/cliproxyapi-dedicated-credential-7645.test.ts new file mode 100644 index 0000000000..50c09efea2 --- /dev/null +++ b/tests/unit/cliproxyapi-dedicated-credential-7645.test.ts @@ -0,0 +1,196 @@ +/** + * Regression tests for #7645 — CLIProxyAPI fallback/passthrough legs reused + * the failed native provider's own credential as the Authorization header + * sent to CLIProxyAPI, which requires its own dedicated `api-keys:` + * credential and rejects any other token with 401 — a permanent no-op for + * every provider configured with `mode: "fallback"` or `mode: "cliproxyapi"`. + * + * All tests exercise REAL production functions end-to-end: + * - updateSettings / getSettings (src/lib/db/settings.ts) + * - upsertUpstreamProxyConfig (src/lib/db/upstreamProxy.ts) + * - resolveExecutorWithProxy (open-sse/handlers/chatCore/executorProxy.ts) + * - CliproxyapiExecutor.execute (open-sse/executors/cliproxyapi.ts) + * `globalThis.fetch` is stubbed only to capture the outbound wire headers, + * distinguishing the native-provider host from the CLIProxyAPI host + * (127.0.0.1:8317). + */ + +import { describe, it, before, after, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const testDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-7645-cpa-cred-")); +process.env.DATA_DIR = testDataDir; + +const coreDb = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const upstreamProxyDb = await import("../../src/lib/db/upstreamProxy.ts"); +const { resolveExecutorWithProxy } = await import( + "../../open-sse/handlers/chatCore/executorProxy.ts" +); +const { clearUpstreamProxyConfigCache } = await import( + "../../open-sse/handlers/chatCore/comboContextCache.ts" +); +const { updateSettingsSchema } = await import("../../src/shared/validation/settingsSchemas.ts"); + +const NATIVE_KEY = "sk-native-provider-key-cliproxyapi-must-not-see"; +const DEDICATED_KEY = "cpa-dedicated-key-configured-by-operator"; + +before(async () => { + await coreDb.ensureDbInitialized(); +}); + +afterEach(async () => { + clearUpstreamProxyConfigCache(); + const { dbCache } = await import("../../src/lib/db/readCache.ts"); + dbCache?.invalidate?.("settings"); +}); + +after(() => { + coreDb.resetDbInstance(); + if (fs.existsSync(testDataDir)) fs.rmSync(testDataDir, { recursive: true, force: true }); +}); + +type ExecuteInput = { + model: string; + body: unknown; + stream: boolean; + credentials: unknown; +}; + +type ExecutorLike = { execute: (input: ExecuteInput) => Promise }; + +/** + * Stubs fetch so calls to CLIProxyAPI's host (127.0.0.1:8317) are captured + * (headers + succeed with 200), while calls to any other host throw a + * simulated native-provider network failure — driving the "fallback" retry + * leg for real. + */ +async function withCapturedCliproxyapiRequest( + fn: () => Promise +): Promise<{ headers: Record; called: boolean }> { + let capturedHeaders: Record | null = null; + const originalFetch = globalThis.fetch; + // @ts-expect-error test stub + globalThis.fetch = async (url: string, init: RequestInit) => { + if (String(url).includes("8317")) { + capturedHeaders = init.headers as Record; + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + throw new Error("simulated native provider network failure"); + }; + try { + await fn(); + } finally { + globalThis.fetch = originalFetch; + } + return { headers: capturedHeaders ?? {}, called: capturedHeaders !== null }; +} + +describe("#7645 — settingsSchemas has a dedicated cliproxyapi_api_key field", () => { + it("updateSettingsSchema accepts cliproxyapi_api_key", () => { + const shape = (updateSettingsSchema as unknown as { shape: Record }).shape; + assert.equal( + Object.prototype.hasOwnProperty.call(shape, "cliproxyapi_api_key"), + true, + "settingsSchemas.ts must define a dedicated cliproxyapi_api_key field" + ); + }); +}); + +describe("#7645 — CLIProxyAPI fallback leg authenticates with the dedicated key", () => { + it("uses the dedicated cliproxyapi_api_key, not the failed native provider's own credential", async () => { + await settingsDb.updateSettings({ cliproxyapi_api_key: DEDICATED_KEY }); + await upstreamProxyDb.upsertUpstreamProxyConfig({ + providerId: "openai-7645-fallback", + mode: "fallback", + enabled: true, + }); + + const executor = await resolveExecutorWithProxy("openai-7645-fallback", undefined, null); + + const { headers, called } = await withCapturedCliproxyapiRequest(() => + (executor as ExecutorLike).execute({ + model: "gpt-4", + body: { model: "gpt-4", messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: { apiKey: NATIVE_KEY }, + }) + ); + + assert.equal(called, true, "the CLIProxyAPI retry leg must have been invoked"); + assert.equal( + headers.Authorization, + `Bearer ${DEDICATED_KEY}`, + "CLIProxyAPI fallback leg must authenticate with the dedicated key" + ); + assert.notEqual( + headers.Authorization, + `Bearer ${NATIVE_KEY}`, + "CLIProxyAPI fallback leg must not reuse the failed native provider's own credential" + ); + }); + + it("direct cliproxyapi passthrough mode also uses the dedicated key", async () => { + await settingsDb.updateSettings({ cliproxyapi_api_key: DEDICATED_KEY }); + await upstreamProxyDb.upsertUpstreamProxyConfig({ + providerId: "anthropic-7645-passthrough", + mode: "cliproxyapi", + enabled: true, + }); + + const executor = await resolveExecutorWithProxy("anthropic-7645-passthrough", undefined, null); + + const { headers, called } = await withCapturedCliproxyapiRequest(() => + (executor as ExecutorLike).execute({ + model: "claude-3-opus", + body: { model: "claude-3-opus", messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: { apiKey: NATIVE_KEY }, + }) + ); + + assert.equal(called, true, "the CLIProxyAPI passthrough leg must have been invoked"); + assert.equal( + headers.Authorization, + `Bearer ${DEDICATED_KEY}`, + "CLIProxyAPI passthrough mode must authenticate with the dedicated key" + ); + }); + + it("falls back to the connection's own credential when no dedicated key is configured (no regression)", async () => { + await settingsDb.updateSettings({ cliproxyapi_api_key: "" }); + await upstreamProxyDb.upsertUpstreamProxyConfig({ + providerId: "anthropic-7645-no-dedicated-key", + mode: "cliproxyapi", + enabled: true, + }); + + const executor = await resolveExecutorWithProxy( + "anthropic-7645-no-dedicated-key", + undefined, + null + ); + + const { headers, called } = await withCapturedCliproxyapiRequest(() => + (executor as ExecutorLike).execute({ + model: "claude-3-opus", + body: { model: "claude-3-opus", messages: [{ role: "user", content: "hi" }] }, + stream: false, + credentials: { apiKey: NATIVE_KEY }, + }) + ); + + assert.equal(called, true); + assert.equal( + headers.Authorization, + `Bearer ${NATIVE_KEY}`, + "with no dedicated key configured, the pre-existing (workaround) behavior must be preserved" + ); + }); +});