diff --git a/changelog.d/features/11104-operator-error-rules.md b/changelog.d/features/11104-operator-error-rules.md new file mode 100644 index 00000000000..f31e78c01f3 --- /dev/null +++ b/changelog.d/features/11104-operator-error-rules.md @@ -0,0 +1 @@ +- **feat(providers):** let operators declare per-provider error rules through `settings.providerErrorRules` instead of patching the catalog — an operator-supplied rule for a provider is consulted before the built-in `providerRuleRegistry`, receives the raw error text, and has its declared scope/cooldown/reason actually honored end to end, for any provider (declaring the rule is the opt-in — no extra allowlist entry needed). Matches are plain case-insensitive substrings (never RegExp) and bounded to 50 rules to keep the hot path safe ([#11104](https://github.com/diegosouzapw/OmniRoute/pull/11104)) diff --git a/docs/architecture/RESILIENCE_GUIDE.md b/docs/architecture/RESILIENCE_GUIDE.md index 030f65dd414..0048761e609 100644 --- a/docs/architecture/RESILIENCE_GUIDE.md +++ b/docs/architecture/RESILIENCE_GUIDE.md @@ -448,14 +448,14 @@ classification rules pick the fallback `reason` and lock `scope` Classification rules only see full error **text** (needed to match body markers like `额度不足`) for providers listed in the `FULL_TEXT_RULE_PROVIDERS` allowlist in `providerErrorRules.ts` — currently only `"agentrouter"`. For -every other provider, `checkFallbackError` hands `getProviderErrorRuleMatch` -only the structured error (`{code, type}`), which is enough for -header/status/code-based rules but blind to body-text markers. The helper -`resolveRuleMatchBody()` performs this selection: full error text for -allowlisted providers, the structured error otherwise. Adding a provider to -`FULL_TEXT_RULE_PROVIDERS` is an explicit per-provider opt-in — it exists so -that the default path for every provider not on the list stays -byte-for-byte unchanged. +every other **built-in catalog** provider, `checkFallbackError` hands +`getProviderErrorRuleMatch` only the structured error (`{code, type}`), which +is enough for header/status/code-based rules but blind to body-text markers. +The helper `resolveRuleMatchBody()` performs this selection: full error text +for allowlisted providers, the structured error otherwise. Adding a +**built-in** provider to `FULL_TEXT_RULE_PROVIDERS` is an explicit per-provider +opt-in — it exists so that the default path for every provider not on the +list stays byte-for-byte unchanged. A rule's `scope` (`model` / `provider` / `connection`) is a separate opt-in from `FULL_TEXT_RULE_PROVIDERS`: `checkFallbackError` only surfaces it as @@ -466,6 +466,31 @@ honorsRuleLockScope()` — today only `"agentrouter"`). See "Restated quota errors" above for what a `scope: "connection"` match actually does once a provider is on that allowlist. +**#11104 — operator-declared rules bypass both allowlists.** An operator can +declare a per-provider rule at runtime via `settings.providerErrorRules` +(`open-sse/config/providerErrorRules.ts::setOperatorProviderErrorRules`) +without editing this file. Gating an operator rule behind +`FULL_TEXT_RULE_PROVIDERS`/`HONORS_RULE_LOCK_SCOPE_PROVIDERS` — allowlists +meant to protect the **default** behavior of built-in catalog rules — would +make the settings mechanism inert for every provider except the ones already +listed there, since declaring the rule is already the operator's explicit +opt-in. `resolveRuleMatchBody()` and `honorsRuleLockScope()` both check +`hasOperatorRuleForProvider()` first: a provider with an operator rule gets +the raw error text and has its declared `scope` honored, regardless of +whether it also appears in either allowlist. + +**Known gap — `providerRuleRegistry` is never consulted for HTTP 400.** +`checkFallbackError`'s `BAD_REQUEST` branch classifies status 400 entirely +through its own pattern arrays (`MODEL_ACCESS_DENIED_PATTERNS`, +`CONTEXT_OVERFLOW_PATTERNS`, etc. in `accountFallback.ts`) and returns before +the `configuredRule`/`getProviderErrorRuleMatch` branch above it is reached. +A built-in catalog rule (or an operator rule) with `status: 400` is +syntactically valid but will never fire. No existing rule targets 400 today, +so nothing in production is affected — but a future 400 rule needs this +branch touched first, which is a larger change than adding a rule (it +reclassifies 400 for every provider already relying on the pattern-array +behavior) and is out of scope for a single-provider rule addition. + ### Adding a new quota-misstating gateway 1. Register one rule array in `statusRestatementRegistry` diff --git a/open-sse/config/providerErrorRules.ts b/open-sse/config/providerErrorRules.ts index c910e3fb4d9..6f00d2c5f12 100644 --- a/open-sse/config/providerErrorRules.ts +++ b/open-sse/config/providerErrorRules.ts @@ -30,21 +30,63 @@ export type ProviderErrorRule = { export type ProviderErrorRuleMatch = { reason: ConfiguredErrorReason; /** - * Intended lock scope. #10334: this field is CONSUMED end-to-end only for - * providers in `HONORS_RULE_LOCK_SCOPE_PROVIDERS` (agentrouter-exclusive - * today, gated by `honorsRuleLockScope()`) — for those, `checkFallbackError` - * surfaces it as `ruleScope` on its return value for the persistence layer - * to honor instead of re-deriving scope from `hasPerModelQuota()`. For - * every other provider it remains INFORMATIONAL: `getProviderErrorRuleMatch` - * callers still read only `reason`/`cooldownMs`, and the actual lock scope - * is decided independently by each call site. Widening the allowlist is - * tracked as a follow-up — see `docs/architecture/RESILIENCE_GUIDE.md` §7. + * Intended lock scope. #10334: for a BUILT-IN catalog rule, this field is + * CONSUMED end-to-end only for providers in `HONORS_RULE_LOCK_SCOPE_PROVIDERS` + * (agentrouter-exclusive today, gated by `honorsRuleLockScope()`) — for those, + * `checkFallbackError` surfaces it as `ruleScope` on its return value for the + * persistence layer to honor instead of re-deriving scope from + * `hasPerModelQuota()`. For every other built-in-rule provider it remains + * INFORMATIONAL. #11104: an OPERATOR-declared rule (`OperatorProviderErrorRule`) + * is exempt from this allowlist — `honorsRuleLockScope()` always returns true + * when the provider has one, since the operator already opted in by declaring + * the rule. Widening `HONORS_RULE_LOCK_SCOPE_PROVIDERS` itself (for a new + * built-in catalog rule) is tracked as a follow-up — see + * `docs/architecture/RESILIENCE_GUIDE.md` §7. */ scope: "model" | "provider" | "connection"; /** Optional explicit cooldown; falls back to the existing per-reason defaults. */ cooldownMs?: number; }; +/** + * Operator-declared per-provider error rule (settings-driven). + * + * Mirrors the catalog `ProviderErrorRule` but is data-only so an operator can + * add a scope/cooldown/reason override for a provider without editing this + * file. `match` is a plain case-insensitive SUBSTRING of the error body — never + * a RegExp — so an operator-supplied pattern can never introduce a ReDoS on the + * error-classification hot path. Bounded to <= 50 rules total by the settings + * schema. An operator rule is consulted BEFORE the built-in `providerRuleRegistry` + * and wins on the first status+substring match for a provider. + */ +export type OperatorProviderErrorRule = { + status: number; + match: string; + scope: "model" | "provider" | "connection"; + reason?: ConfiguredErrorReason; + cooldownMs?: number; +}; + +let operatorProviderErrorRules: Record = {}; + +/** + * Inject operator-declared rules. Called from the runtime-settings applier + * (`applyRuntimeSettings`) once at boot and on every settings update, with the + * value validated by the settings schema. Pass `undefined`/empty/null to clear. + * Provider keys are lowercased so lookups are case-insensitive. + */ +export function setOperatorProviderErrorRules( + rules: Record | undefined | null +): void { + operatorProviderErrorRules = {}; + if (!rules) return; + for (const [provider, list] of Object.entries(rules)) { + if (Array.isArray(list) && list.length > 0) { + operatorProviderErrorRules[provider.toLowerCase()] = list; + } + } +} + // ─── Opencode ─────────────────────────────────────────────────────────────────── // Opencode Go uses an account-wide quota. The body usually says "rate limit // reached" but the presence of `x-ratelimit-remaining-requests: 0` is the @@ -272,11 +314,21 @@ export const providerRuleRegistry = new Map([ * FULL_TEXT_RULE_PROVIDERS: that set controls what body a rule matches against * (input), this one controls whether the matched scope changes caller behavior * (output). A provider could need one without the other. + * + * Providers with an operator-declared rule (`setOperatorProviderErrorRules`) + * are honored too, without being added here: the allowlist exists to gate + * BUILT-IN catalog rules, which change default behavior for every operator + * running that provider — an operator rule is already an explicit, per-operator + * opt-in, so gating it a second time behind this list would make the settings + * mechanism (#11104) silently inert for every provider except the ones listed + * below. See `hasOperatorRuleForProvider`. */ const HONORS_RULE_LOCK_SCOPE_PROVIDERS = new Set(["agentrouter"]); export function honorsRuleLockScope(provider: string | null | undefined): boolean { - return !!provider && HONORS_RULE_LOCK_SCOPE_PROVIDERS.has(provider.toLowerCase()); + if (!provider) return false; + const key = provider.toLowerCase(); + return HONORS_RULE_LOCK_SCOPE_PROVIDERS.has(key) || hasOperatorRuleForProvider(key); } /** @@ -310,28 +362,51 @@ export function egressBucketedLockProviders(): string[] { } /** - * Providers whose rules match on the FULL upstream error text. - * checkFallbackError's rule lookup normally passes only the structured + * Providers whose BUILT-IN catalog rules match on the FULL upstream error + * text. checkFallbackError's rule lookup normally passes only the structured * error ({code, type} — message stripped by the combo callers), which is * enough for header/status/code rules but blind to body-text markers like * agentrouter's "额度不足". Providers in this set get the raw error text as * the match body instead. EXCLUSIVE allowlist by owner decision (2026-08-13): * adding a provider here is an explicit opt-in — the default path for every * other provider must remain byte-for-byte unchanged. + * + * Operator-declared rules bypass this allowlist entirely (see + * `hasOperatorRuleForProvider`): the operator's `match` is a literal substring + * of the error body by construction, so a rule that never sees body text could + * never match anything, defeating the point of declaring it. */ const FULL_TEXT_RULE_PROVIDERS = new Set(["agentrouter"]); +/** + * True when an operator has declared at least one rule for this provider via + * `settings.providerErrorRules` (injected through `setOperatorProviderErrorRules`). + * Presence of the rule IS the opt-in — no separate allowlist to maintain, and + * no widening decision needed as new operators configure new providers. + */ +export function hasOperatorRuleForProvider(provider: string | null | undefined): boolean { + if (!provider) return false; + const rules = operatorProviderErrorRules[provider.toLowerCase()]; + return !!rules && rules.length > 0; +} + /** * Resolve the body handed to getProviderErrorRuleMatch inside - * checkFallbackError: full error text for FULL_TEXT_RULE_PROVIDERS, - * the structured error for everyone else. + * checkFallbackError: full error text for FULL_TEXT_RULE_PROVIDERS or any + * provider with an operator-declared rule, the structured error for everyone + * else. */ export function resolveRuleMatchBody( provider: string | null | undefined, structuredError: unknown, errorText: string | null | undefined ): unknown { - if (provider && FULL_TEXT_RULE_PROVIDERS.has(provider.toLowerCase()) && errorText) { + if ( + provider && + (FULL_TEXT_RULE_PROVIDERS.has(provider.toLowerCase()) || + hasOperatorRuleForProvider(provider)) && + errorText + ) { return errorText; } return structuredError ?? null; @@ -346,10 +421,32 @@ export function getProviderErrorRuleMatch( provider: string | null | undefined, status: number, headers: Headers | Record | null | undefined, - body?: unknown + body?: unknown, + operatorRules?: Record ): ProviderErrorRuleMatch | null { if (!provider) return null; - const rules = providerRuleRegistry.get(provider.toLowerCase()); + const key = provider.toLowerCase(); + + // Operator-declared rules win first: an operator can override any catalog + // rule for a provider without editing this file. `operatorRules` is the + // injected source (tests / direct callers); when omitted we fall back to the + // settings-backed cache populated by `setOperatorProviderErrorRules`. + const opRules = (operatorRules ?? operatorProviderErrorRules)?.[key]; + if (opRules && opRules.length > 0) { + const text = typeof body === "string" ? body : JSON.stringify(body ?? ""); + const lowered = text.toLowerCase(); + for (const r of opRules) { + if (r.status === status && lowered.includes(r.match.toLowerCase())) { + return { + reason: r.reason ?? "quota_exhausted", + scope: r.scope, + cooldownMs: r.cooldownMs, + }; + } + } + } + + const rules = providerRuleRegistry.get(key); if (!rules) return null; // Normalize headers: accept either a `Headers` object (from `fetch()`) or // a plain record. Provider rules access headers via plain object indexing. diff --git a/src/lib/config/runtimeSettings.ts b/src/lib/config/runtimeSettings.ts index dc5e9d720dc..bf615749bae 100644 --- a/src/lib/config/runtimeSettings.ts +++ b/src/lib/config/runtimeSettings.ts @@ -1,5 +1,9 @@ import { clearHealthCheckLogCache } from "@/lib/tokenHealthCheck"; import { setCustomBannedSignals } from "@omniroute/open-sse/services/accountFallback.ts"; +import { + setOperatorProviderErrorRules, + type OperatorProviderErrorRule, +} from "@omniroute/open-sse/config/providerErrorRules.ts"; import { isAutomatedTestProcess } from "@/shared/utils/testProcess"; type JsonRecord = Record; @@ -46,6 +50,7 @@ interface RuntimeSettingsSnapshot { systemTransforms: unknown; authzBypass: AuthzBypassSnapshot; customBannedSignals: string[]; + providerErrorRules: Record | null; } // Default bypass policy: kill-switch on, `/api/mcp/` bypassable. Mirrors the @@ -72,6 +77,7 @@ const DEFAULT_RUNTIME_SETTINGS_SNAPSHOT: RuntimeSettingsSnapshot = { systemTransforms: null, authzBypass: DEFAULT_AUTHZ_BYPASS_SNAPSHOT, customBannedSignals: [], + providerErrorRules: null, }; let lastAppliedSnapshot: RuntimeSettingsSnapshot | null = null; @@ -138,6 +144,34 @@ function normalizeStringArray(value: unknown): string[] { ); } +/** + * Defensive shape-check of operator-declared error rules pulled from settings. + * The settings schema already validates this on write; this guard prevents a + * malformed stored value (or an unexpected shape) from crashing the + * error-classification hot path. Returns null when the value is missing or not + * a record of non-empty rule arrays. + */ +function normalizeOperatorProviderErrorRules( + value: unknown +): Record | null { + if (value === null || typeof value !== "object") return null; + const record = value as Record; + const result: Record = {}; + for (const [provider, list] of Object.entries(record)) { + if (!Array.isArray(list) || list.length === 0) continue; + const rules = list.filter( + (entry): entry is OperatorProviderErrorRule => + !!entry && + typeof entry === "object" && + typeof (entry as OperatorProviderErrorRule).status === "number" && + typeof (entry as OperatorProviderErrorRule).match === "string" && + typeof (entry as OperatorProviderErrorRule).scope === "string" + ); + if (rules.length > 0) result[provider.toLowerCase()] = rules; + } + return Object.keys(result).length > 0 ? result : null; +} + function normalizeStringRecord(value: unknown): Record { const record = toRecord(parseStoredJson(value, "modelAliases")); const entries = Object.entries(record) @@ -244,6 +278,7 @@ export function buildRuntimeSettingsSnapshot( systemTransforms: parseStoredJson(settings.systemTransforms, "systemTransforms"), authzBypass: normalizeAuthzBypass(settings), customBannedSignals: normalizeStringArray(settings.customBannedSignals), + providerErrorRules: normalizeOperatorProviderErrorRules(settings.providerErrorRules), }; } @@ -540,6 +575,13 @@ export async function applyRuntimeSettings( markChanged("bannedSignals"); } + if ( + force || + hasChanged(currentSnapshot.providerErrorRules, previousSnapshot.providerErrorRules) + ) { + setOperatorProviderErrorRules(currentSnapshot.providerErrorRules ?? undefined); + } + lastAppliedSnapshot = currentSnapshot; return changes; } diff --git a/src/shared/validation/settingsSchemas.ts b/src/shared/validation/settingsSchemas.ts index 388e3f73c74..beefb5fc8f3 100644 --- a/src/shared/validation/settingsSchemas.ts +++ b/src/shared/validation/settingsSchemas.ts @@ -259,6 +259,48 @@ export const updateSettingsSchema = z.object({ }) ) .optional(), + /** + * Operator-declared per-provider error rules. Consulted BEFORE the built-in + * `providerRuleRegistry` in open-sse/config/providerErrorRules.ts so an + * operator can add a scope/cooldown/reason override for a provider without + * editing the catalog. Matches are plain case-insensitive SUBSTRINGS of the + * error body (never RegExp) to keep the classification hot path ReDoS-safe. + * Bounded to 50 rules total so a misconfigured setting cannot blow up the + * matcher. + */ + providerErrorRules: z + .record( + z.string().trim().min(1).max(100), + z.array( + z.object({ + status: z.number().int().min(100).max(599), + match: z.string().min(1).max(200), + scope: z.enum(["model", "provider", "connection"]), + reason: z + .enum([ + "auth_error", + "quota_exhausted", + "rate_limit_exceeded", + "model_capacity", + "server_error", + "unknown", + ]) + .optional(), + cooldownMs: z.number().int().min(0).max(86_400_000).optional(), + }) + ) + ) + .optional() + .superRefine((value, ctx) => { + if (!value) return; + const total = Object.values(value).reduce((n, rules) => n + rules.length, 0); + if (total > 50) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `providerErrorRules: at most 50 rules total, got ${total}`, + }); + } + }), // #6168: global session-stickiness opt-out (per-combo config overrides this). disableSessionStickiness: z.boolean().optional(), /** Keep eligible combo targets close to the provider-side prompt cache. */ diff --git a/tests/unit/provider-error-rules-operator.test.ts b/tests/unit/provider-error-rules-operator.test.ts new file mode 100644 index 00000000000..6f8c9e52ef8 --- /dev/null +++ b/tests/unit/provider-error-rules-operator.test.ts @@ -0,0 +1,135 @@ +import { describe, it, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { + getProviderErrorRuleMatch, + setOperatorProviderErrorRules, + resolveRuleMatchBody, + honorsRuleLockScope, + type OperatorProviderErrorRule, +} from "../../open-sse/config/providerErrorRules.ts"; + +describe("operator error rules", () => { + beforeEach(() => { + // Isolate each test from the settings-backed cache. + setOperatorProviderErrorRules(undefined); + }); + + it("operator rule overrides the catalog registry for a provider", () => { + const op: Record = { + nvidia: [{ status: 404, match: "Not found for account", scope: "model", cooldownMs: 1000 }], + }; + const m = getProviderErrorRuleMatch("nvidia", 404, null, "Not found for account id 123", op); + assert.ok(m, "operator rule should match"); + assert.equal(m.scope, "model"); + assert.equal(m.cooldownMs, 1000); + }); + + it("operator rule wins even when a catalog rule would also match", () => { + const op: Record = { + openrouter: [{ status: 402, match: "credits exhausted", scope: "model" }], + }; + const m = getProviderErrorRuleMatch("openrouter", 402, null, "credits exhausted on key", op); + assert.ok(m); + // Catalog rule for openrouter/402 uses scope "connection"; the operator + // override must take precedence. + assert.equal(m.scope, "model"); + }); + + it("operator can reclassify a 401 before the global permanent rule", () => { + const op: Record = { + acme: [ + { status: 401, match: "transient quota", scope: "connection", reason: "quota_exhausted" }, + ], + }; + const m = getProviderErrorRuleMatch("acme", 401, null, "transient quota — retry shortly", op); + assert.ok(m); + assert.equal(m.scope, "connection"); + assert.equal(m.reason, "quota_exhausted"); + }); + + it("unknown provider with no operator rule returns null (no throw)", () => { + const m = getProviderErrorRuleMatch("unknown-provider", 402, null, "anything"); + assert.equal(m, null); + }); + + it("substring match is case-insensitive", () => { + const op: Record = { + nvidia: [{ status: 404, match: "NOT FOUND", scope: "model" }], + }; + const m = getProviderErrorRuleMatch("nvidia", 404, null, "Body says Not Found Here", op); + assert.ok(m); + assert.equal(m.scope, "model"); + }); + + it("status must match before the substring is considered", () => { + const op: Record = { + nvidia: [{ status: 404, match: "not found", scope: "model" }], + }; + // 500 with the same body text must NOT match a 404 rule. + const m = getProviderErrorRuleMatch("nvidia", 500, null, "not found for account", op); + assert.equal(m, null); + }); + + it("without an operator override the catalog registry is intact", () => { + const m = getProviderErrorRuleMatch("openrouter", 402, null, "credits exhausted on key"); + assert.ok(m); + assert.equal(m.scope, "connection"); + assert.equal(m.cooldownMs, 2 * 60 * 1000); + }); + + it("reads the settings-backed cache via setOperatorProviderErrorRules", () => { + setOperatorProviderErrorRules({ + nvidia: [{ status: 404, match: "Not found", scope: "model" }], + }); + const m = getProviderErrorRuleMatch("nvidia", 404, null, "Not found for account"); + assert.ok(m); + assert.equal(m.scope, "model"); + // Provider key lookup is case-insensitive. + const m2 = getProviderErrorRuleMatch("NVIDIA", 404, null, "Not found here"); + assert.ok(m2); + assert.equal(m2.scope, "model"); + }); + + // Regression coverage for #11104's original gap: an operator rule for any + // provider outside the built-in FULL_TEXT_RULE_PROVIDERS/ + // HONORS_RULE_LOCK_SCOPE_PROVIDERS allowlists was silently text-blind (only + // {code,type} reached the matcher) and had its declared scope dropped by the + // persistence layer. Declaring an operator rule for a provider must be + // sufficient by itself — no separate allowlist entry required. + describe("operator rule bypasses the built-in allowlists", () => { + it("resolveRuleMatchBody hands the full error text once an operator rule exists for the provider", () => { + setOperatorProviderErrorRules({ + acme: [{ status: 404, match: "model withdrawn", scope: "model" }], + }); + const body = resolveRuleMatchBody("acme", { code: "not_found" }, "Model withdrawn upstream"); + assert.equal(body, "Model withdrawn upstream"); + }); + + it("resolveRuleMatchBody keeps returning the structured error for a provider with no operator rule", () => { + const body = resolveRuleMatchBody("acme", { code: "not_found" }, "Model withdrawn upstream"); + assert.deepEqual(body, { code: "not_found" }); + }); + + it("honorsRuleLockScope is true once an operator rule exists for the provider", () => { + assert.equal(honorsRuleLockScope("acme"), false); + setOperatorProviderErrorRules({ + acme: [{ status: 404, match: "model withdrawn", scope: "model" }], + }); + assert.equal(honorsRuleLockScope("acme"), true); + }); + + it("an operator rule for a non-allowlisted provider matches on raw body text end to end", () => { + setOperatorProviderErrorRules({ + acme: [{ status: 404, match: "model withdrawn", scope: "model" }], + }); + const body = resolveRuleMatchBody( + "acme", + { code: "not_found" }, + "Error: model withdrawn upstream" + ); + const m = getProviderErrorRuleMatch("acme", 404, null, body); + assert.ok(m, "operator rule should match once resolveRuleMatchBody hands it the raw text"); + assert.equal(m.scope, "model"); + }); + }); +});