fix(combo): restricted keys listing a combo name no longer skip every member (#12899)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Regressão de v3.8.50 vinda do #9057, com o sintoma mais enganoso possível: `attempted: 0`. A política já tinha admitido o combo e a checagem era refeita em cada membro interno.

Manter o filtro por prefixo de provider e o `disableNonPublicModels` intactos é o que impede o short-circuit de virar um buraco na allow-list.
This commit is contained in:
Bob.Hou
2026-09-07 07:57:41 -04:00
committed by GitHub
parent e8a91173de
commit d7721559a0
4 changed files with 133 additions and 13 deletions

View File

@@ -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)).

View File

@@ -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

View File

@@ -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<boolean>;
}): Promise<boolean> {
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);
}

View File

@@ -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);
});