mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-06 07:12:12 +03:00
fix(guardrails/chat): stop Vision Bridge hijacking credentialed models to opencode-zen (#7204)
* chore(ci): add .mergify.yml to main — Mergify only reads config from the default branch (#7168) * fix(ci): add the auto-enqueue pull_request_rule to the Mergify config (queue_conditions alone are eligibility-only) (#7179) * fix(guardrails/chat): stop Vision Bridge hijacking credentialed models to opencode-zen OpenCode (and similar clients) often send image parts in long sessions. Vision Bridge treated the request model as non-vision and whole-request- rerouted to getBestVisionModel(), which preferred opencode-* (priority 0). That landed on a noauth connection and returned 401 Missing API key — while proxies/combos still logged the original target (zai/glm-5.2, grok-cli, …). Also: after resolveRoutingModel(X-Route-Model), keep body.model aligned so the post-guardrail "body.model !== modelStr" path cannot undo the routing header. - visionBridge: skip whole-request reroute when original model has usable creds - visionBridge: refuse reroute to targets known unusable (noauth without key) - visionBridgeRouter: deprioritize opencode-* for auto vision pick - chat: alignBodyModelWithRouting + only adopt true guardrail model mutations - tests: VB-CRED-01/02 + alignBodyModelWithRouting coverage * fix(guardrails/chat): keep chat.ts under the file-size ratchet and update stale vision-bridge tests for the credential-aware reroute skip - Extract the routing-model reconciliation logic (X-Route-Model align, post-guardrail reroute policy re-check, hook model override) into RoutingModelOps helpers in resolveRoutingModel.ts, shrinking chat.ts back under the frozen 1796-line file-size baseline (was 1837). - Update tests/unit/guardrails/vision-bridge-callmodel.test.ts: the fallback mock must match whichever API shape the selected fallback model actually calls (OpenAI-compatible vs Anthropic), since the vision-bridge router priority fix in this PR can now legitimately select an Anthropic fallback model instead of always defaulting to an OpenAI-shaped opencode-* model. - Update tests/unit/vision-bridge-policy-reroute-6640.test.ts: per this PR's own VB-CRED-01 test, a credentialed original model is now intentionally never whole-request-rerouted (it always falls through to describe-then- forward) — so the pre-existing #6640 tests are updated to assert the final, user-facing answer always comes from the original credentialed model, matching the new intended behavior instead of the retired whole-request-reroute path. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> * fix(guardrails/chat): reduce isProviderConnectionUsable cyclomatic complexity to satisfy the project-wide complexity ratchet The new isProviderConnectionUsable helper (complexity 21) regressed the project-wide complexity ratchet from 2056 to 2057. Refactor it to use Set membership checks and small extracted helpers (hasNonEmptyString, hasOAuthCredential) instead of chained === / || comparisons — same behavior, verified by the existing "isProviderConnectionUsable rejects noauth without api key" test, with complexity back under the 15-per-function threshold and the project-wide ratchet back at the 2056 baseline. Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com> --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- fix(guardrails/chat): do not whole-request-reroute Vision Bridge away from credentialed models (e.g. combo target zai/glm-5.2 or grok-cli → opencode-zen noauth 401); align body.model with X-Route-Model so post-guardrail cannot undo the routing header
|
||||
@@ -92,6 +92,79 @@ export interface VisionBridgeDependencies {
|
||||
) => Promise<string>;
|
||||
/** Override combo-target vision check — return true to force processing, false to skip. */
|
||||
checkModelHasComboMapping?: (model: string) => Promise<boolean>;
|
||||
/**
|
||||
* Whether a model string has a usable active credential (true/false).
|
||||
* Return `null` when indeterminate (no DB / error) so callers can fail-open.
|
||||
*/
|
||||
hasUsableCredentials?: (model: string) => Promise<boolean | null>;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a provider connection can actually authenticate upstream.
|
||||
* `noauth` with no real API key is NOT usable (opencode-zen free tier often
|
||||
* surfaces as noauth and then 401 "Missing API key").
|
||||
*/
|
||||
type ProviderConnectionLike = {
|
||||
authType?: string | null;
|
||||
apiKey?: string | null;
|
||||
accessToken?: string | null;
|
||||
refreshToken?: string | null;
|
||||
idToken?: string | null;
|
||||
testStatus?: string | null;
|
||||
};
|
||||
|
||||
const TERMINAL_CONNECTION_STATUSES = new Set(["disabled", "banned", "expired"]);
|
||||
// Free/noauth only counts when a real key is still present; apikey/cookie need the same.
|
||||
const KEY_ONLY_AUTH_TYPES = new Set(["noauth", "none", "", "apikey", "cookie"]);
|
||||
const TOKEN_AUTH_TYPES = new Set(["oauth", "access_token", "external_idp"]);
|
||||
|
||||
function hasNonEmptyString(value: unknown): boolean {
|
||||
return typeof value === "string" && value.trim().length > 0;
|
||||
}
|
||||
|
||||
function hasOAuthCredential(connection: ProviderConnectionLike): boolean {
|
||||
return (
|
||||
hasNonEmptyString(connection.refreshToken) ||
|
||||
hasNonEmptyString(connection.accessToken) ||
|
||||
hasNonEmptyString(connection.idToken)
|
||||
);
|
||||
}
|
||||
|
||||
export function isProviderConnectionUsable(connection: ProviderConnectionLike): boolean {
|
||||
const status = String(connection.testStatus || "").toLowerCase();
|
||||
if (TERMINAL_CONNECTION_STATUSES.has(status)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const auth = String(connection.authType || "").toLowerCase();
|
||||
const hasKey = hasNonEmptyString(connection.apiKey);
|
||||
|
||||
if (KEY_ONLY_AUTH_TYPES.has(auth)) {
|
||||
return hasKey;
|
||||
}
|
||||
if (TOKEN_AUTH_TYPES.has(auth)) {
|
||||
return hasOAuthCredential(connection) || hasKey;
|
||||
}
|
||||
return hasKey;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve whether `provider/model` has at least one usable active connection.
|
||||
* Returns `null` when the credential store is unavailable (unit tests / early boot).
|
||||
*/
|
||||
export async function hasUsableCredentialsForModel(model: string): Promise<boolean | null> {
|
||||
const provider = typeof model === "string" ? model.split("/")[0]?.trim() : "";
|
||||
if (!provider) return null;
|
||||
try {
|
||||
const { getProviderConnections } = await import("@/lib/db/providers");
|
||||
const connections = await getProviderConnections({ provider, isActive: true });
|
||||
if (!Array.isArray(connections)) return null;
|
||||
// Empty active set is a definitive "no" only when the table is readable.
|
||||
if (connections.length === 0) return false;
|
||||
return connections.some((c: any) => isProviderConnectionUsable(c));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export class VisionBridgeGuardrail extends BaseGuardrail {
|
||||
@@ -183,37 +256,66 @@ export class VisionBridgeGuardrail extends BaseGuardrail {
|
||||
return { block: false };
|
||||
}
|
||||
|
||||
// 9. Individual non-combo model with images → REROUTE to best vision-capable model
|
||||
// 9. Individual non-combo model with images → optionally REROUTE to best vision-capable model
|
||||
// instead of describing images through an intermediate vision call.
|
||||
// This lets a downstream vision model process the image natively.
|
||||
//
|
||||
// CRITICAL (VibeProxy combo / explicit provider models):
|
||||
// When the original model already has usable credentials (e.g. combo target
|
||||
// zai/glm-5.2), NEVER whole-request-reroute to another provider. Auto-select
|
||||
// prefers opencode-* (priority 0) even when only a broken noauth connection
|
||||
// exists, which produced: HTTP log zai → Guardrail reroute → opencode-zen 401
|
||||
// "Missing API key" while the combo UI still showed body=zai. Fall through to
|
||||
// the image-describe path so the user's chosen model still answers.
|
||||
if (comboVisionBridgeDecision === "not-combo" && !forceVisionBridge) {
|
||||
// Honor an explicit operator override from the Vision Bridge settings tab
|
||||
// (settings.visionBridgeModel) as the fixed reroute target, for consistency
|
||||
// with the combo/describe path below (step 10) which always honors it via
|
||||
// getVisionBridgeConfig. When unset, auto-select the fastest available
|
||||
// vision-capable model from available providers.
|
||||
const configuredModel =
|
||||
typeof settings.visionBridgeModel === "string" && settings.visionBridgeModel.trim()
|
||||
? settings.visionBridgeModel.trim()
|
||||
: undefined;
|
||||
const bestModel = getBestVisionModel({ fixedModel: configuredModel });
|
||||
if (bestModel && bestModel !== model) {
|
||||
const modifiedBody = {
|
||||
...(body as Record<string, unknown>),
|
||||
model: bestModel,
|
||||
};
|
||||
return {
|
||||
block: false,
|
||||
modifiedPayload: modifiedBody as unknown,
|
||||
meta: {
|
||||
rerouted: true,
|
||||
fromModel: model,
|
||||
toModel: bestModel,
|
||||
imagesKept: imageParts.length,
|
||||
},
|
||||
};
|
||||
const checkCreds =
|
||||
this.deps.hasUsableCredentials ?? hasUsableCredentialsForModel;
|
||||
const originalUsable = await checkCreds(model);
|
||||
|
||||
if (originalUsable === true) {
|
||||
// Keep the credentialed model; describe images below if needed.
|
||||
context.log?.debug?.(
|
||||
"VISION_BRIDGE",
|
||||
`Skipping whole-request vision reroute; keeping credentialed model ${model}`
|
||||
);
|
||||
} else {
|
||||
// Honor an explicit operator override from the Vision Bridge settings tab
|
||||
// (settings.visionBridgeModel) as the fixed reroute target, for consistency
|
||||
// with the combo/describe path below (step 10) which always honors it via
|
||||
// getVisionBridgeConfig. When unset, auto-select the fastest available
|
||||
// vision-capable model from available providers.
|
||||
const configuredModel =
|
||||
typeof settings.visionBridgeModel === "string" && settings.visionBridgeModel.trim()
|
||||
? settings.visionBridgeModel.trim()
|
||||
: undefined;
|
||||
const bestModel = getBestVisionModel({ fixedModel: configuredModel });
|
||||
if (bestModel && bestModel !== model) {
|
||||
const bestUsable = await checkCreds(bestModel);
|
||||
// Only block the reroute when we KNOW the target is unusable (false).
|
||||
// `null` (no DB / tests) fails open so existing unit tests keep working.
|
||||
if (bestUsable === false) {
|
||||
context.log?.warn?.(
|
||||
"VISION_BRIDGE",
|
||||
`Vision reroute target ${bestModel} has no usable credentials; describing images instead of hijacking ${model}`
|
||||
);
|
||||
} else {
|
||||
const modifiedBody = {
|
||||
...(body as Record<string, unknown>),
|
||||
model: bestModel,
|
||||
};
|
||||
return {
|
||||
block: false,
|
||||
modifiedPayload: modifiedBody as unknown,
|
||||
meta: {
|
||||
rerouted: true,
|
||||
fromModel: model,
|
||||
toModel: bestModel,
|
||||
imagesKept: imageParts.length,
|
||||
},
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
// Fall through: if no vision model found, describe images as text instead
|
||||
// Fall through: describe images as text (or no-op if describe path can't run)
|
||||
}
|
||||
|
||||
// 10. Get configuration
|
||||
|
||||
@@ -110,12 +110,17 @@ function getVisionCapableModels(): VisionModelCandidate[] {
|
||||
const caps = getResolvedModelCapabilities(fullModelId);
|
||||
|
||||
if (caps.supportsVision === true) {
|
||||
// Determine priority based on provider type
|
||||
// Determine priority based on provider type (lower = better).
|
||||
// Do NOT prefer opencode-* first: those catalog entries often resolve to a
|
||||
// noauth connection and 401 "Missing API key", hijacking working providers
|
||||
// (e.g. zai/glm-5.2 combo targets) when Vision Bridge auto-reroutes.
|
||||
let priority = 100;
|
||||
if (providerAlias.startsWith("opencode-")) {
|
||||
priority = 0; // Local/free models first
|
||||
} else if (providerAlias === "openai" || providerAlias === "anthropic") {
|
||||
priority = 50; // Major providers
|
||||
if (providerAlias === "openai" || providerAlias === "anthropic") {
|
||||
priority = 50; // Major providers with real API keys
|
||||
} else if (providerAlias === "vertex" || providerAlias === "gemini") {
|
||||
priority = 55;
|
||||
} else if (providerAlias.startsWith("opencode-")) {
|
||||
priority = 95; // Free/catalog — only if nothing credentialed is available
|
||||
} else {
|
||||
priority = 75; // Other providers
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { randomUUID } from "crypto";
|
||||
import { resolveChatRequestBody } from "./requestBody";
|
||||
import { normalizeReasoningRequest } from "@/shared/reasoning/effortStandardization";
|
||||
import { resolveRoutingModel } from "./resolveRoutingModel";
|
||||
import { resolveRoutingModel, RoutingModelOps } from "./resolveRoutingModel";
|
||||
import {
|
||||
getProviderCredentialsWithQuotaPreflight,
|
||||
markAccountUnavailable,
|
||||
@@ -370,6 +370,8 @@ export async function handleChat(
|
||||
// resolveRoutingModel). The resolved model still passes through
|
||||
// enforceApiKeyPolicy below, so it cannot bypass per-key allowlists.
|
||||
let modelStr = resolveRoutingModel(request, body);
|
||||
// Align body.model with the routing model immediately (see applyRoutingModelAlignment).
|
||||
body = RoutingModelOps.align(body, modelStr, log);
|
||||
|
||||
// Count messages (support both messages[] and input[] formats)
|
||||
const msgCount = body.messages?.length || body.input?.length || 0;
|
||||
@@ -472,24 +474,19 @@ export async function handleChat(
|
||||
preCallGuardrails.message || "Request rejected: suspicious content detected"
|
||||
);
|
||||
}
|
||||
// Snapshot model BEFORE the guardrail payload (see reconcileGuardrailReroute).
|
||||
const modelBeforeGuardrails =
|
||||
typeof body?.model === "string" && body.model.length > 0 ? body.model : modelStr;
|
||||
body = preCallGuardrails.payload;
|
||||
if (body?.model && typeof body.model === "string" && body.model !== modelStr) {
|
||||
const rerouteModel = body.model;
|
||||
// A guardrail (e.g. Vision Bridge auto-reroute) can swap body.model AFTER
|
||||
// enforceApiKeyPolicy already validated modelStr's allowlist/budget above.
|
||||
// Re-check the new target against the same per-key allowlist so a
|
||||
// policy-restricted key cannot be silently routed to an unchecked model.
|
||||
const rerouteAllowed = await isModelAllowedForKey(apiKey, rerouteModel);
|
||||
if (!rerouteAllowed) {
|
||||
log.warn(
|
||||
"POLICY",
|
||||
`Guardrail reroute to "${rerouteModel}" rejected by API key policy (key=${apiKeyInfo?.id || "unknown"}); keeping original model "${modelStr}"`
|
||||
);
|
||||
body = { ...body, model: modelStr };
|
||||
} else {
|
||||
modelStr = rerouteModel;
|
||||
}
|
||||
}
|
||||
({ body, modelStr } = await RoutingModelOps.reconcileGuardrailReroute({
|
||||
body,
|
||||
modelBeforeGuardrails,
|
||||
modelStr,
|
||||
apiKey,
|
||||
apiKeyId: apiKeyInfo?.id,
|
||||
isModelAllowedForKey,
|
||||
log,
|
||||
}));
|
||||
telemetry.endPhase();
|
||||
|
||||
// T08: per-key active session limit (0 = unlimited).
|
||||
@@ -530,9 +527,13 @@ export async function handleChat(
|
||||
|
||||
// Apply hook mutations
|
||||
body = hookCtx.body as any;
|
||||
if (hookCtx.model && hookCtx.model !== modelStr) {
|
||||
modelStr = hookCtx.model;
|
||||
}
|
||||
({ body, modelStr } = RoutingModelOps.reconcileModelOverride({
|
||||
body,
|
||||
modelStr,
|
||||
overrideModel: hookCtx.model,
|
||||
logTag: "Hook model override",
|
||||
log,
|
||||
}));
|
||||
|
||||
// Short-circuit if a hook returned a direct response
|
||||
if (hookResponse) {
|
||||
|
||||
@@ -5,6 +5,12 @@
|
||||
// proxy can send `X-Route-Model` to restore routing control without mutating the
|
||||
// request body. The resolved value still flows through `enforceApiKeyPolicy`, so
|
||||
// it cannot bypass per-key model/combo allowlists. See PR #4863.
|
||||
//
|
||||
// IMPORTANT: callers MUST then align `body.model` with the resolved value via
|
||||
// `alignBodyModelWithRouting` (or equivalent). Otherwise the post-guardrail
|
||||
// "body.model !== modelStr → adopt body.model" path silently undoes the header
|
||||
// override and routes to the original body model (e.g. opencode-zen 401 while
|
||||
// logs still show the X-Route-Model target like zai/glm-5.2).
|
||||
|
||||
type HeaderCarrier = { headers: { get(name: string): string | null } };
|
||||
|
||||
@@ -15,3 +21,141 @@ export function resolveRoutingModel(
|
||||
const headerModel = request.headers.get("x-route-model")?.trim();
|
||||
return headerModel || body.model;
|
||||
}
|
||||
|
||||
/**
|
||||
* Keep body.model in sync with the routing model after resolveRoutingModel.
|
||||
* Returns the (possibly new) body object and whether body.model was rewritten.
|
||||
*/
|
||||
export function alignBodyModelWithRouting<T extends { model?: unknown }>(
|
||||
body: T,
|
||||
modelStr: string | null | undefined
|
||||
): { body: T; aligned: boolean; previousModel: string | null } {
|
||||
const previousModel = typeof body?.model === "string" ? body.model : null;
|
||||
if (!modelStr || typeof modelStr !== "string" || modelStr.length === 0) {
|
||||
return { body, aligned: false, previousModel };
|
||||
}
|
||||
if (previousModel === modelStr) {
|
||||
return { body, aligned: false, previousModel };
|
||||
}
|
||||
return {
|
||||
body: { ...body, model: modelStr },
|
||||
aligned: true,
|
||||
previousModel,
|
||||
};
|
||||
}
|
||||
|
||||
type RoutingLogger = { info: (tag: string, msg: string) => void };
|
||||
|
||||
/**
|
||||
* Thin wrapper around alignBodyModelWithRouting that also emits the ROUTING
|
||||
* log line, kept out of chat.ts so the caller stays a one-liner.
|
||||
*
|
||||
* Callers MUST run this immediately after resolveRoutingModel. Without it,
|
||||
* the post-guardrail "body.model !== modelStr → adopt body.model" reconcile
|
||||
* path (see reconcileGuardrailReroute below) treats a mismatched body.model
|
||||
* as a guardrail reroute and silently restores it — undoing an X-Route-Model
|
||||
* header override (e.g. header zai/glm-5.2 + body opencode-zen/gpt-5.4 → 401
|
||||
* Missing API key while HTTP logs still show zai).
|
||||
*/
|
||||
export function applyRoutingModelAlignment<T extends { model?: unknown }>(
|
||||
body: T,
|
||||
modelStr: string | null | undefined,
|
||||
log: RoutingLogger
|
||||
): T {
|
||||
const aligned = alignBodyModelWithRouting(body, modelStr);
|
||||
if (aligned.aligned) {
|
||||
log.info(
|
||||
"ROUTING",
|
||||
`Aligned body.model to routing model: ${aligned.previousModel || "(none)"} → ${modelStr}`
|
||||
);
|
||||
}
|
||||
return aligned.body;
|
||||
}
|
||||
|
||||
type GuardrailRerouteLogger = {
|
||||
info: (tag: string, msg: string) => void;
|
||||
warn: (tag: string, msg: string) => void;
|
||||
};
|
||||
|
||||
/**
|
||||
* Keep body.model glued to modelStr after a stage that may override modelStr
|
||||
* (e.g. a pre-request hook) without necessarily rewriting body.model itself.
|
||||
* Returns the (possibly new) body/modelStr and logs the override when the
|
||||
* hook actually changed modelStr.
|
||||
*/
|
||||
export function reconcileModelOverride<T extends { model?: unknown }>(params: {
|
||||
body: T;
|
||||
modelStr: string;
|
||||
overrideModel: string | null | undefined;
|
||||
logTag: string;
|
||||
log: GuardrailRerouteLogger;
|
||||
}): { body: T; modelStr: string } {
|
||||
const { body, overrideModel, logTag, log } = params;
|
||||
let { modelStr } = params;
|
||||
|
||||
if (overrideModel && overrideModel !== modelStr) {
|
||||
log.info("ROUTING", `${logTag}: ${modelStr} → ${overrideModel}`);
|
||||
modelStr = overrideModel;
|
||||
if (typeof body?.model !== "string" || body.model !== modelStr) {
|
||||
return { body: { ...body, model: modelStr }, modelStr };
|
||||
}
|
||||
return { body, modelStr };
|
||||
}
|
||||
if (modelStr && typeof body?.model === "string" && body.model !== modelStr) {
|
||||
// The stage rewrote body without updating model — restore the routing model.
|
||||
return { body: { ...body, model: modelStr }, modelStr };
|
||||
}
|
||||
return { body, modelStr };
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconcile body.model after the pre-call guardrail pipeline runs. A guardrail
|
||||
* (e.g. Vision Bridge auto-reroute) can swap body.model AFTER
|
||||
* enforceApiKeyPolicy already validated modelStr's allowlist/budget, so any
|
||||
* genuine change must be re-checked against the same per-key allowlist before
|
||||
* being adopted as the new routing model. `modelBeforeGuardrails` must be a
|
||||
* snapshot of body.model taken immediately before the guardrail payload was
|
||||
* applied — comparing against a stale/aligned value would misclassify a
|
||||
* legitimate X-Route-Model alignment as a guardrail reroute.
|
||||
*/
|
||||
export async function reconcileGuardrailReroute<T extends { model?: unknown }>(params: {
|
||||
body: T;
|
||||
modelBeforeGuardrails: string;
|
||||
modelStr: string;
|
||||
apiKey: string | null | undefined;
|
||||
apiKeyId: string | undefined;
|
||||
isModelAllowedForKey: (apiKey: string | null | undefined, model: string) => Promise<boolean>;
|
||||
log: GuardrailRerouteLogger;
|
||||
}): Promise<{ body: T; modelStr: string }> {
|
||||
const { body, modelBeforeGuardrails, apiKey, apiKeyId, isModelAllowedForKey, log } = params;
|
||||
let { modelStr } = params;
|
||||
|
||||
if (body?.model && typeof body.model === "string" && body.model !== modelBeforeGuardrails) {
|
||||
const rerouteModel = body.model;
|
||||
const rerouteAllowed = await isModelAllowedForKey(apiKey, rerouteModel);
|
||||
if (!rerouteAllowed) {
|
||||
log.warn(
|
||||
"POLICY",
|
||||
`Guardrail reroute to "${rerouteModel}" rejected by API key policy (key=${apiKeyId || "unknown"}); keeping original model "${modelStr}"`
|
||||
);
|
||||
return { body: { ...body, model: modelStr }, modelStr };
|
||||
}
|
||||
log.info("ROUTING", `Guardrail model reroute: ${modelBeforeGuardrails} → ${rerouteModel}`);
|
||||
return { body, modelStr: rerouteModel };
|
||||
}
|
||||
// Guardrails returned a payload whose model drifted from modelStr without
|
||||
// changing from the pre-guardrail value (should not happen after align), or
|
||||
// stripped model — keep body.model glued to modelStr.
|
||||
if (modelStr && typeof body?.model === "string" && body.model !== modelStr) {
|
||||
return { body: { ...body, model: modelStr }, modelStr };
|
||||
}
|
||||
return { body, modelStr };
|
||||
}
|
||||
|
||||
// Grouped under one namespace so chat.ts needs a single extra import name
|
||||
// alongside resolveRoutingModel — see each function's own doc comment above.
|
||||
export const RoutingModelOps = {
|
||||
align: applyRoutingModelAlignment,
|
||||
reconcileGuardrailReroute,
|
||||
reconcileModelOverride,
|
||||
};
|
||||
|
||||
@@ -41,9 +41,13 @@ const TINY_PNG = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAf
|
||||
|
||||
test("callVisionModel falls through to next model when primary fails", async () => {
|
||||
let fetchCallCount = 0;
|
||||
const FALLBACK_RESPONSE = JSON.stringify({
|
||||
choices: [{ message: { content: "fallback model description" } }],
|
||||
});
|
||||
// The fallback candidate can legitimately resolve to either an OpenAI-compatible
|
||||
// model (POST .../chat/completions, { choices: [{ message: { content } }] }) or an
|
||||
// Anthropic model (POST .../v1/messages, { content: [{ type: "text", text }] }) —
|
||||
// vision-bridge router priority (#7204) now ranks credentialed providers (openai/
|
||||
// anthropic) ahead of opencode-*, so the mock must match whichever shape the
|
||||
// fallback attempt actually requests instead of assuming OpenAI's shape.
|
||||
const FALLBACK_TEXT = "fallback model description";
|
||||
|
||||
globalThis.fetch = async (url: RequestInfo | URL, _init?: RequestInit) => {
|
||||
fetchCallCount++;
|
||||
@@ -51,8 +55,14 @@ test("callVisionModel falls through to next model when primary fails", async ()
|
||||
// First call (primary model) — simulate API error
|
||||
throw new Error("mock: primary model unavailable");
|
||||
}
|
||||
// Second call (fallback model) — return valid response
|
||||
return new Response(FALLBACK_RESPONSE, {
|
||||
// Second call (fallback model) — return a valid response shaped for whichever
|
||||
// API the fallback model actually calls.
|
||||
const urlStr = typeof url === "string" ? url : url.toString();
|
||||
const isAnthropicCall = urlStr.includes("/v1/messages");
|
||||
const body = isAnthropicCall
|
||||
? JSON.stringify({ content: [{ type: "text", text: FALLBACK_TEXT }] })
|
||||
: JSON.stringify({ choices: [{ message: { content: FALLBACK_TEXT } }] });
|
||||
return new Response(body, {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
@@ -72,7 +82,7 @@ test("callVisionModel falls through to next model when primary fails", async ()
|
||||
);
|
||||
assert.equal(
|
||||
result,
|
||||
"fallback model description",
|
||||
FALLBACK_TEXT,
|
||||
"must return the fallback model's response"
|
||||
);
|
||||
});
|
||||
|
||||
@@ -38,6 +38,9 @@ function createGuardrail(options?: Parameters<typeof VisionBridgeGuardrail>[0])
|
||||
}
|
||||
return mockVisionResponse;
|
||||
},
|
||||
// Fail-open (null) so classic VB-S01/S07/S10 reroute tests keep working without a
|
||||
// live credential DB. Credential-aware cases inject an explicit mock.
|
||||
hasUsableCredentials: async () => null,
|
||||
...(options?.deps ?? {}),
|
||||
},
|
||||
});
|
||||
@@ -695,3 +698,96 @@ test("VB-S11b: passthroughs when vision-capable model has NO combo mapping", asy
|
||||
assert.strictEqual(result.modifiedPayload, undefined);
|
||||
assert.strictEqual(visionCallCount, 0);
|
||||
});
|
||||
|
||||
// ── Credential-aware whole-request reroute (combo zai hijack fix) ───────────
|
||||
|
||||
test("VB-CRED-01: does NOT whole-request-reroute when original model has usable credentials", async () => {
|
||||
// Repro: OpenCode 94-msg body with images + combo target zai/glm-5.2 was
|
||||
// hijacked to opencode-zen/gpt-5.4 (priority 0, noauth) → 401 Missing API key.
|
||||
const guardrail = createGuardrail({
|
||||
deps: {
|
||||
hasUsableCredentials: async (m: string) =>
|
||||
m.startsWith("zai/") ? true : m.startsWith("opencode-") ? false : null,
|
||||
},
|
||||
});
|
||||
|
||||
const payload = createPayload({
|
||||
model: "zai/glm-5.2",
|
||||
messages: [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "text", text: "What is in this screenshot?" },
|
||||
{
|
||||
type: "image_url",
|
||||
image_url: { url: "https://example.com/shot.png" },
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = await guardrail.preCall(payload, createContext({ model: "zai/glm-5.2" }));
|
||||
assert.strictEqual(result.block, false);
|
||||
// Must NOT swap model to opencode-zen / openai vision target
|
||||
if (result.modifiedPayload) {
|
||||
const modified = result.modifiedPayload as { model?: string };
|
||||
assert.strictEqual(
|
||||
modified.model,
|
||||
"zai/glm-5.2",
|
||||
"credentialed original model must not be whole-request-rerouted"
|
||||
);
|
||||
}
|
||||
const meta = result.meta as Record<string, unknown> | undefined;
|
||||
assert.notStrictEqual(meta?.rerouted, true, "must not set rerouted meta for credentialed model");
|
||||
});
|
||||
|
||||
test("VB-CRED-02: does NOT reroute to a vision model known to lack credentials", async () => {
|
||||
mockSettings.visionBridgeModel = "opencode-zen/gpt-5.4";
|
||||
const guardrail = createGuardrail({
|
||||
deps: {
|
||||
// Original unusable, best vision model also unusable
|
||||
hasUsableCredentials: async () => false,
|
||||
},
|
||||
});
|
||||
|
||||
const payload = createPayload({
|
||||
model: "minimax/minimax-01",
|
||||
messages: [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "text", text: "Describe" },
|
||||
{ type: "image_url", image_url: { url: "https://example.com/a.png" } },
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = await guardrail.preCall(payload, createContext({ model: "minimax/minimax-01" }));
|
||||
assert.strictEqual(result.block, false);
|
||||
const meta = result.meta as Record<string, unknown> | undefined;
|
||||
assert.notStrictEqual(meta?.rerouted, true, "must not reroute to unusable vision model");
|
||||
});
|
||||
|
||||
test("isProviderConnectionUsable rejects noauth without api key", async () => {
|
||||
const { isProviderConnectionUsable } = await import(
|
||||
"../../../src/lib/guardrails/visionBridge.ts"
|
||||
);
|
||||
assert.strictEqual(
|
||||
isProviderConnectionUsable({ authType: "noauth", apiKey: null }),
|
||||
false
|
||||
);
|
||||
assert.strictEqual(
|
||||
isProviderConnectionUsable({ authType: "apikey", apiKey: "sk-real" }),
|
||||
true
|
||||
);
|
||||
assert.strictEqual(
|
||||
isProviderConnectionUsable({ authType: "oauth", refreshToken: "rt" }),
|
||||
true
|
||||
);
|
||||
assert.strictEqual(
|
||||
isProviderConnectionUsable({ authType: "apikey", apiKey: "x", testStatus: "banned" }),
|
||||
false
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
// Regression guard for #4863: X-Route-Model header overrides body.model for routing.
|
||||
// Also covers alignBodyModelWithRouting — without body alignment the post-guardrail
|
||||
// path silently restores body.model and undoes the header (zai header + opencode body → 401).
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { resolveRoutingModel } from "../../src/sse/handlers/resolveRoutingModel.ts";
|
||||
import {
|
||||
alignBodyModelWithRouting,
|
||||
resolveRoutingModel,
|
||||
} from "../../src/sse/handlers/resolveRoutingModel.ts";
|
||||
|
||||
function req(headers: Record<string, string>) {
|
||||
return { headers: { get: (n: string) => headers[n.toLowerCase()] ?? null } };
|
||||
@@ -30,3 +35,31 @@ describe("resolveRoutingModel (#4863)", () => {
|
||||
assert.equal(resolveRoutingModel(req({ "x-route-model": " " }), { model: "fallback" }), "fallback");
|
||||
});
|
||||
});
|
||||
|
||||
describe("alignBodyModelWithRouting (X-Route-Model body lockstep)", () => {
|
||||
it("rewrites body.model when it differs from the routing model", () => {
|
||||
const body = { model: "opencode-zen/gpt-5.4", messages: [{ role: "user", content: "hi" }] };
|
||||
const routed = resolveRoutingModel(req({ "x-route-model": "zai/glm-5.2" }), body);
|
||||
const result = alignBodyModelWithRouting(body, routed);
|
||||
assert.equal(routed, "zai/glm-5.2");
|
||||
assert.equal(result.aligned, true);
|
||||
assert.equal(result.previousModel, "opencode-zen/gpt-5.4");
|
||||
assert.equal(result.body.model, "zai/glm-5.2");
|
||||
// Original body object is not mutated
|
||||
assert.equal(body.model, "opencode-zen/gpt-5.4");
|
||||
});
|
||||
|
||||
it("is a no-op when body.model already matches", () => {
|
||||
const body = { model: "zai/glm-5.2" };
|
||||
const result = alignBodyModelWithRouting(body, "zai/glm-5.2");
|
||||
assert.equal(result.aligned, false);
|
||||
assert.equal(result.body, body);
|
||||
});
|
||||
|
||||
it("is a no-op when routing model is empty", () => {
|
||||
const body = { model: "opencode-zen/gpt-5.4" };
|
||||
const result = alignBodyModelWithRouting(body, null);
|
||||
assert.equal(result.aligned, false);
|
||||
assert.equal(result.body.model, "opencode-zen/gpt-5.4");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,15 +5,29 @@
|
||||
// against the original model. Without a re-check, a key restricted via
|
||||
// `allowedModels` could silently execute against an unvetted reroute target.
|
||||
//
|
||||
// UPDATED for PR #7204 (Vision Bridge no-credentialed-hijack fix): when the
|
||||
// ORIGINAL model already has a usable, credentialed connection (as seeded
|
||||
// below via `seedConnection("openai", ...)`), Vision Bridge now deliberately
|
||||
// never whole-request-reroutes it to another model (see
|
||||
// `VisionBridgeGuardrail.preCall` step 9 / `VB-CRED-01` in
|
||||
// tests/unit/guardrails/visionBridge.test.ts) — it always falls through to
|
||||
// describe-then-forward: an internal call describes the image via the vision
|
||||
// model, then the description is forwarded as text to the ORIGINAL,
|
||||
// already-approved model, which produces the final, user-facing answer.
|
||||
//
|
||||
// These tests exercise the real `handleChat()` pipeline end-to-end (real DB,
|
||||
// real guardrail registry, mocked upstream fetch) to prove:
|
||||
// 1. A guardrail-driven reroute to a model NOT in the key's `allowedModels`
|
||||
// is rejected — the request falls back to the original, already-approved
|
||||
// model instead of silently escaping the policy.
|
||||
// 2. A guardrail-driven reroute to a model that DOES pass the allowlist is
|
||||
// still honored (the fix must not break the legitimate reroute).
|
||||
// 3. The reroute honors an explicit `settings.visionBridgeModel` operator
|
||||
// override, consistent with the combo/describe path.
|
||||
// real guardrail registry, mocked upstream fetch) to prove the policy
|
||||
// invariant that motivated #6640 still holds under the new #7204 behavior:
|
||||
// 1. A vision-bridge target NOT in the key's `allowedModels` is only ever
|
||||
// used internally (image description) — it never becomes the final,
|
||||
// user-facing answering model. That role always stays with the
|
||||
// original, already-approved model.
|
||||
// 2. Even when the vision target IS inside `allowedModels`, a credentialed
|
||||
// original model is still never whole-request-rerouted — the original
|
||||
// model remains the final answerer (no regression from #7204's intent).
|
||||
// 3. An explicit `settings.visionBridgeModel` operator override is honored
|
||||
// as the internal describe-path model, but — consistent with #7204 —
|
||||
// does not override a credentialed original model as the final answerer.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
@@ -71,27 +85,37 @@ test("#6640: guardrail reroute to a model outside allowedModels is rejected —
|
||||
})
|
||||
);
|
||||
|
||||
// The reroute target (gpt-4o-mini) is not in allowedModels — the request
|
||||
// must NOT be silently executed against it. It must either fall back to the
|
||||
// original, already-approved model (gpt-3.5-turbo) or be rejected outright,
|
||||
// but it must never reach the upstream with the disallowed model.
|
||||
// The vision-bridge target (gpt-4o-mini) is not in allowedModels — the
|
||||
// request must NOT be silently executed against it as the final answering
|
||||
// model. Under #7204, the credentialed original model (gpt-3.5-turbo) is
|
||||
// never whole-request-rerouted in the first place, so the vision target can
|
||||
// only ever appear as an internal, non-final describe call. Either way, the
|
||||
// FINAL upstream call (the one whose response the user actually receives)
|
||||
// must never be the disallowed model.
|
||||
if (response.status === 200) {
|
||||
assert.equal(fetchCalls.length, 1, "exactly one upstream call expected");
|
||||
assert.ok(fetchCalls.length >= 1, "at least one upstream call expected");
|
||||
const finalCall = fetchCalls[fetchCalls.length - 1];
|
||||
assert.equal(
|
||||
fetchCalls[0].body?.model,
|
||||
finalCall.body?.model,
|
||||
"gpt-3.5-turbo",
|
||||
"must fall back to the original allowed model, not silently execute the disallowed reroute target"
|
||||
"the final, user-facing answer must come from the original allowed model, not the disallowed vision target"
|
||||
);
|
||||
} else {
|
||||
assert.equal(fetchCalls.length, 0, "a rejected request must never reach the upstream");
|
||||
}
|
||||
});
|
||||
|
||||
test("#6640: guardrail reroute to a model inside allowedModels is honored (no regression)", async () => {
|
||||
test("#6640: a credentialed original model is never whole-request-rerouted, even when the vision target is inside allowedModels (PR #7204)", async () => {
|
||||
await seedConnection("openai", { apiKey: "sk-openai-primary" });
|
||||
await settingsDb.updateSettings({ visionBridgeModel: "openai/gpt-4o-mini" });
|
||||
|
||||
// This key allows BOTH the original model and the reroute target.
|
||||
// This key allows BOTH the original model and the vision target — before
|
||||
// #7204 this would have made the whole-request reroute to gpt-4o-mini the
|
||||
// final answering model. #7204 deliberately changes this: a credentialed
|
||||
// original model (gpt-3.5-turbo, seeded above) is never whole-request-
|
||||
// rerouted regardless of what's allowed — it always stays the final
|
||||
// answerer, with the vision target used only for the internal image
|
||||
// description (see VB-CRED-01 in tests/unit/guardrails/visionBridge.test.ts).
|
||||
const apiKey = await seedApiKey({
|
||||
allowedModels: ["openai/gpt-3.5-turbo", "openai/gpt-4o-mini"],
|
||||
});
|
||||
@@ -110,19 +134,20 @@ test("#6640: guardrail reroute to a model inside allowedModels is honored (no re
|
||||
);
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(fetchCalls.length, 1);
|
||||
assert.ok(fetchCalls.length >= 1, "at least one upstream call expected");
|
||||
const finalCall = fetchCalls[fetchCalls.length - 1];
|
||||
assert.equal(
|
||||
fetchCalls[0].body?.model,
|
||||
"gpt-4o-mini",
|
||||
"reroute to an allowed vision model must still be honored"
|
||||
finalCall.body?.model,
|
||||
"gpt-3.5-turbo",
|
||||
"the credentialed original model must remain the final answerer — no whole-request reroute (PR #7204), even though gpt-4o-mini is allowed"
|
||||
);
|
||||
});
|
||||
|
||||
test("#6640: reroute honors an explicit settings.visionBridgeModel override (consistency with the describe path)", async () => {
|
||||
test("#6640: settings.visionBridgeModel override does not displace a credentialed original model as the final answerer (PR #7204)", async () => {
|
||||
await seedConnection("openai", { apiKey: "sk-openai-primary" });
|
||||
await settingsDb.updateSettings({ visionBridgeModel: "openai/gpt-4o-mini" });
|
||||
|
||||
// No allowlist restriction — nothing to enforce here, this proves the
|
||||
// No allowlist restriction — nothing to enforce here; this proves the
|
||||
// settings threading itself (independent of the policy re-check above).
|
||||
const apiKey = await seedApiKey();
|
||||
|
||||
@@ -140,10 +165,11 @@ test("#6640: reroute honors an explicit settings.visionBridgeModel override (con
|
||||
);
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(fetchCalls.length, 1);
|
||||
assert.ok(fetchCalls.length >= 1, "at least one upstream call expected");
|
||||
const finalCall = fetchCalls[fetchCalls.length - 1];
|
||||
assert.equal(
|
||||
fetchCalls[0].body?.model,
|
||||
"gpt-4o-mini",
|
||||
"the configured settings.visionBridgeModel must be honored as the reroute target"
|
||||
finalCall.body?.model,
|
||||
"gpt-3.5-turbo",
|
||||
"the configured settings.visionBridgeModel must not override the credentialed original model as the final answerer (PR #7204)"
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user