diff --git a/changelog.d/fixes/12899-combo-restricted-key-target-policy.md b/changelog.d/fixes/12899-combo-restricted-key-target-policy.md new file mode 100644 index 0000000000..05da91404e --- /dev/null +++ b/changelog.d/fixes/12899-combo-restricted-key-target-policy.md @@ -0,0 +1 @@ +- Restricted API keys whose `allowedModels` lists a combo name no longer skip every combo member at pre-dispatch (`ALL_TARGETS_SKIPPED`). Inner-target filtering still applies when the allow-list is a provider prefix or `disableNonPublicModels` is on ([#12899](https://github.com/diegosouzapw/OmniRoute/pull/12899)). diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 6c03803ba1..08a793aa98 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -79,6 +79,7 @@ import { import { dispatchChatWithAffinityEviction } from "./chatDispatch"; import { getCachedSettings, getCombosCacheVersion } from "@/lib/db/readCache"; import { comboCheckProvider, ghComboGate } from "./chat/githubLiveCatalogFilter.ts"; +import { comboTargetPassesKeyModelPolicy } from "./chat/comboTargetKeyPolicy.ts"; import { getCombos } from "@/lib/db/combos"; import { resolveModelLockoutSettings } from "@/lib/resilience/modelLockoutSettings"; import { @@ -1006,19 +1007,10 @@ async function handleChatImplementation( } ) => { if (isComboLiveTest) return true; - - // #9057: for keys with model restrictions (allowedModels or disableNonPublicModels), - // run isModelAllowedForKey even for auto/* models. The API-key policy gate - // (validateModelAccess in apiKeyPolicy.ts) treats auto/* as a virtual combo and - // skips isModelAllowedForKey, so the per-candidate check here is the only - // enforcement point during combo routing. Without it, a key with - // disableNonPublicModels=true can reach free/prohibited models through auto/*. - const hasModelRestrictions = - apiKeyInfo && - (Boolean(apiKeyInfo.allowedModels?.length) || apiKeyInfo.disableNonPublicModels === true); - if (hasModelRestrictions && apiKey) { - const modelAllowed = await isModelAllowedForKey(apiKey, modelString); - if (!modelAllowed) return false; + // #12886: combo-name allow-list must not skip inner targets (#9057 still + // checks auto/* / disableNonPublic via comboTargetPassesKeyModelPolicy). + if (!(await comboTargetPassesKeyModelPolicy({ apiKey, apiKeyInfo, requestedModelStr: resolvedModelStr, targetModelStr: modelString, isModelAllowedForKey }))) { + return false; } // Use getModelInfo to resolve custom prefixes, but prefer the combo diff --git a/src/sse/handlers/chat/comboTargetKeyPolicy.ts b/src/sse/handlers/chat/comboTargetKeyPolicy.ts new file mode 100644 index 0000000000..7bc441056c --- /dev/null +++ b/src/sse/handlers/chat/comboTargetKeyPolicy.ts @@ -0,0 +1,48 @@ +/** + * Combo pre-dispatch API-key model policy (#9057 / #12886). + * + * Policy already admitted the requested combo. Per-target + * `isModelAllowedForKey` must not then skip every member just because the + * allow-list is the combo name. auto/* / disableNonPublic still check the + * inner target so #9057 holds. + */ + +export type ComboTargetKeyPolicyInfo = { + allowedModels?: string[] | null; + disableNonPublicModels?: boolean | null; + modelAccessMode?: string | null; +}; + +function modelMatchesAllowPattern(pattern: string, model: string): boolean { + if (pattern.endsWith("/*")) return model.startsWith(pattern.slice(0, -1)); + return pattern === model; +} + +function allowListCoversRequestedCombo( + allowedModels: string[] | null | undefined, + requestedModelStr: string +): boolean { + if (!allowedModels?.length || !requestedModelStr) return false; + return allowedModels.some((pattern) => modelMatchesAllowPattern(pattern, requestedModelStr)); +} + +export async function comboTargetPassesKeyModelPolicy(opts: { + apiKey: string | null | undefined; + apiKeyInfo: ComboTargetKeyPolicyInfo | null | undefined; + requestedModelStr: string; + targetModelStr: string; + isModelAllowedForKey: (key: string, model: string) => Promise; +}): Promise { + const { apiKey, apiKeyInfo, requestedModelStr, targetModelStr, isModelAllowedForKey } = opts; + if (!apiKey || !apiKeyInfo) return true; + + const hasModelRestrictions = + Boolean(apiKeyInfo.allowedModels?.length) || apiKeyInfo.disableNonPublicModels === true; + if (!hasModelRestrictions) return true; + + if (allowListCoversRequestedCombo(apiKeyInfo.allowedModels, requestedModelStr)) { + return true; + } + + return isModelAllowedForKey(apiKey, targetModelStr); +} diff --git a/tests/unit/combo-restricted-key-target-policy-12886.test.ts b/tests/unit/combo-restricted-key-target-policy-12886.test.ts new file mode 100644 index 0000000000..2c6635e05c --- /dev/null +++ b/tests/unit/combo-restricted-key-target-policy-12886.test.ts @@ -0,0 +1,79 @@ +/** + * #12886 — restricted API key whose allowedModels is the combo name must not + * skip every combo target at pre-dispatch (#9057 per-target check). + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { comboTargetPassesKeyModelPolicy } from "../../src/sse/handlers/chat/comboTargetKeyPolicy.ts"; + +const KEY = "sk-test-12886"; +const COMBO = "combo-deepseek-v4-flash"; +const INNER = "deepseek/deepseek-v4-flash"; +const OTHER = "anthropic/claude-sonnet-5"; + +function allowListChecker(patterns: string[]) { + return async (_key: string, model: string) => + patterns.some((pattern) => { + if (pattern.endsWith("/*")) return model.startsWith(pattern.slice(0, -1)); + return pattern === model; + }); +} + +test("#12886: combo-name-only allow-list admits that combo's inner targets", async () => { + const ok = await comboTargetPassesKeyModelPolicy({ + apiKey: KEY, + apiKeyInfo: { modelAccessMode: "restricted", allowedModels: [COMBO] }, + requestedModelStr: COMBO, + targetModelStr: INNER, + isModelAllowedForKey: allowListChecker([COMBO]), + }); + assert.equal(ok, true, "inner target must not be skipped when the combo name is allowed"); +}); + +test("#12886: provider-prefix allow-list still filters inner targets", async () => { + const checker = allowListChecker(["deepseek/*"]); + const deepseekOk = await comboTargetPassesKeyModelPolicy({ + apiKey: KEY, + apiKeyInfo: { modelAccessMode: "restricted", allowedModels: ["deepseek/*"] }, + requestedModelStr: COMBO, + targetModelStr: INNER, + isModelAllowedForKey: checker, + }); + const otherOk = await comboTargetPassesKeyModelPolicy({ + apiKey: KEY, + apiKeyInfo: { modelAccessMode: "restricted", allowedModels: ["deepseek/*"] }, + requestedModelStr: COMBO, + targetModelStr: OTHER, + isModelAllowedForKey: checker, + }); + assert.equal(deepseekOk, true); + assert.equal(otherOk, false, "non-matching inner target stays blocked"); +}); + +test("#9057: disableNonPublicModels still rejects a keyless inner target", async () => { + const ok = await comboTargetPassesKeyModelPolicy({ + apiKey: KEY, + apiKeyInfo: { disableNonPublicModels: true }, + requestedModelStr: "auto/best", + targetModelStr: "big-pickle", + isModelAllowedForKey: async () => false, + }); + assert.equal(ok, false); +}); + +test("#12886: unrestricted key skips the gate", async () => { + let called = 0; + const ok = await comboTargetPassesKeyModelPolicy({ + apiKey: KEY, + apiKeyInfo: { modelAccessMode: "all", allowedModels: [] }, + requestedModelStr: COMBO, + targetModelStr: INNER, + isModelAllowedForKey: async () => { + called += 1; + return false; + }, + }); + assert.equal(ok, true); + assert.equal(called, 0); +});