From 01ab5d1fd5e2986f6c482647136a69a4ed7b5c36 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 14 Jul 2026 13:28:51 -0300 Subject: [PATCH 1/5] =?UTF-8?q?chore(ci):=20add=20.mergify.yml=20to=20main?= =?UTF-8?q?=20=E2=80=94=20Mergify=20only=20reads=20config=20from=20the=20d?= =?UTF-8?q?efault=20branch=20(#7168)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .mergify.yml | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 .mergify.yml diff --git a/.mergify.yml b/.mergify.yml new file mode 100644 index 00000000000..131c6d71a99 --- /dev/null +++ b/.mergify.yml @@ -0,0 +1,55 @@ +# Mergify merge queue — WS3.4/D5 of the v3.8.49 quality/velocity master plan. +# +# WHY: ~85-100 active PR authors/month and 300+ PRs/week peaks, all merged by ONE +# identity. The manual merge-train validated batches by hand; this queue automates +# it with batching + automatic batch bisection (a red batch of N costs ~log2(N) +# revalidations instead of N). Mergify Open Source plan: free, unlimited, public repo. +# +# GOVERNANCE (non-negotiable, mirrors CLAUDE.md Hard Rules #21/#22 + the owner's +# pre-merge ⭐ gate): +# • A PR enters the queue ONLY via the `queue` label — applied by the owner (or a +# session acting for the owner) AFTER the pre-merge ⭐ report/decision. The label +# IS the merge approval; Mergify only executes it. +# • During a release-freeze (open issue labeled `release-freeze`), do NOT label PRs +# targeting the frozen branch — the freeze is a human-honored coordination signal +# the queue cannot see. Retarget to the active release/vX+1 first (Hard Rule #21). +# • Never label a PR another session is actively working (Hard Rule #22b). +# • Fallback path if Mergify misbehaves or the OSS plan changes: the manual +# merge-train runbook (docs/ops/MERGE_TRAIN.md) — remove labels, proceed by hand. + +queue_rules: + - name: release + # Any current or future release branch — the reason GitHub's native queue was + # rejected (no wildcard support on personal-account repos). + queue_conditions: + - base~=^release/v\d+\.\d+\.\d+$ + - label=queue + - -draft + - -conflict + # "Everything that ran is green, nothing still running, AND the always-on + # anchor check succeeded" — robust to the path-filtered fast-gates (docs-only + # PRs skip code jobs; matrix shard names vary) while never fail-open: a PR with + # zero checks cannot vacuously merge, because `Merge integrity` runs on EVERY + # non-draft PR (quality.yml) and must be an affirmative success. Review approval + # is intentionally NOT a condition here: the owner-applied `queue` label IS the + # approval in this repo's single-maintainer model (see governance header). + merge_conditions: + - "#check-failure=0" + - "#check-pending=0" + - "#check-success>=1" + - check-success=Merge integrity (changelog + generated skills) + # Batching: validate up to 10 queued PRs together (the manual train's sweet spot); + # don't hold a lone PR hostage waiting for siblings. + batch_size: 10 + batch_max_wait_time: 5 min + # Squash keeps the one-commit-per-PR history the CHANGELOG reconciliation expects. + merge_method: squash + +pull_request_rules: + - name: clean up the queue label after merge + conditions: + - merged + actions: + label: + remove: + - queue From fb24740b73443c33e5e6f7ee8b781fa92e5e4cc1 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:11:35 -0300 Subject: [PATCH 2/5] fix(ci): add the auto-enqueue pull_request_rule to the Mergify config (queue_conditions alone are eligibility-only) (#7179) --- .mergify.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.mergify.yml b/.mergify.yml index 131c6d71a99..88bc431369e 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -46,6 +46,18 @@ queue_rules: merge_method: squash pull_request_rules: + # AUTO-ENQUEUE trigger — queue_conditions alone only define ELIGIBILITY in current + # Mergify semantics (without this rule the PR sits at "use @Mergifyio queue", + # observed live on PR #7175). The conditions mirror queue_conditions on purpose. + - name: queue on owner-applied label + conditions: + - base~=^release/v\d+\.\d+\.\d+$ + - label=queue + - -draft + - -conflict + actions: + queue: + name: release - name: clean up the queue label after merge conditions: - merged From 67a7d4c996904dff8dd2d64f5330bd0f81713619 Mon Sep 17 00:00:00 2001 From: artickc Date: Wed, 15 Jul 2026 02:59:11 +0300 Subject: [PATCH 3/5] fix(guardrails/chat): stop Vision Bridge hijacking credentialed models to opencode-zen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../vision-bridge-no-credentialed-hijack.md | 1 + src/lib/guardrails/visionBridge.ts | 146 ++++++++++++++---- src/lib/guardrails/visionBridgeRouter.ts | 15 +- src/sse/handlers/chat.ts | 46 +++++- src/sse/handlers/resolveRoutingModel.ts | 28 ++++ tests/unit/guardrails/visionBridge.test.ts | 96 ++++++++++++ tests/unit/resolve-routing-model.test.ts | 35 ++++- 7 files changed, 331 insertions(+), 36 deletions(-) create mode 100644 changelog.d/fixes/vision-bridge-no-credentialed-hijack.md diff --git a/changelog.d/fixes/vision-bridge-no-credentialed-hijack.md b/changelog.d/fixes/vision-bridge-no-credentialed-hijack.md new file mode 100644 index 00000000000..637d661ea81 --- /dev/null +++ b/changelog.d/fixes/vision-bridge-no-credentialed-hijack.md @@ -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 \ No newline at end of file diff --git a/src/lib/guardrails/visionBridge.ts b/src/lib/guardrails/visionBridge.ts index 4e5b45f9665..27d0441d5b5 100644 --- a/src/lib/guardrails/visionBridge.ts +++ b/src/lib/guardrails/visionBridge.ts @@ -92,6 +92,67 @@ export interface VisionBridgeDependencies { ) => Promise; /** Override combo-target vision check — return true to force processing, false to skip. */ checkModelHasComboMapping?: (model: string) => Promise; + /** + * 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; +} + +/** + * 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"). + */ +export function isProviderConnectionUsable(connection: { + authType?: string | null; + apiKey?: string | null; + accessToken?: string | null; + refreshToken?: string | null; + idToken?: string | null; + testStatus?: string | null; +}): boolean { + const status = String(connection.testStatus || "").toLowerCase(); + if (status === "disabled" || status === "banned" || status === "expired") { + return false; + } + const auth = String(connection.authType || "").toLowerCase(); + const hasKey = typeof connection.apiKey === "string" && connection.apiKey.trim().length > 0; + if (auth === "noauth" || auth === "none" || auth === "") { + // Free/noauth only counts when a real key is still present. + return hasKey; + } + if (auth === "apikey" || auth === "cookie") { + return hasKey; + } + if (auth === "oauth" || auth === "access_token" || auth === "external_idp") { + return Boolean( + (connection.refreshToken && String(connection.refreshToken).trim()) || + (connection.accessToken && String(connection.accessToken).trim()) || + (connection.idToken && String(connection.idToken).trim()) || + 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 { + 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 +244,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), - 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), + 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 diff --git a/src/lib/guardrails/visionBridgeRouter.ts b/src/lib/guardrails/visionBridgeRouter.ts index 76b1331d050..caafda1fe03 100644 --- a/src/lib/guardrails/visionBridgeRouter.ts +++ b/src/lib/guardrails/visionBridgeRouter.ts @@ -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 } diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 0bee6caa424..a5a9a08eae9 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -1,7 +1,7 @@ import { randomUUID } from "crypto"; import { resolveChatRequestBody } from "./requestBody"; import { normalizeReasoningRequest } from "@/shared/reasoning/effortStandardization"; -import { resolveRoutingModel } from "./resolveRoutingModel"; +import { alignBodyModelWithRouting, resolveRoutingModel } from "./resolveRoutingModel"; import { getProviderCredentialsWithQuotaPreflight, markAccountUnavailable, @@ -368,6 +368,21 @@ 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. Without this, the + // post-guardrail block below treats "body.model !== modelStr" as a vision- + // bridge reroute and silently restores the original body model — undoing + // X-Route-Model (e.g. header zai/glm-5.2 + body opencode-zen/gpt-5.4 → 401 + // Missing API key while HTTP logs still show zai). + { + const aligned = alignBodyModelWithRouting(body, modelStr); + body = aligned.body; + if (aligned.aligned) { + log.info( + "ROUTING", + `Aligned body.model to routing model: ${aligned.previousModel || "(none)"} → ${modelStr}` + ); + } + } // Count messages (support both messages[] and input[] formats) const msgCount = body.messages?.length || body.input?.length || 0; @@ -470,8 +485,17 @@ export async function handleChat( preCallGuardrails.message || "Request rejected: suspicious content detected" ); } + // Snapshot model BEFORE applying the guardrail payload so we only treat a + // genuine guardrail mutation (e.g. Vision Bridge) as a reroute — not a stale + // body.model that never matched X-Route-Model / the aligned routing model. + 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) { + if ( + body?.model && + typeof body.model === "string" && + body.model !== modelBeforeGuardrails + ) { 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. @@ -485,8 +509,18 @@ export async function handleChat( ); body = { ...body, model: modelStr }; } else { + log.info("ROUTING", `Guardrail model reroute: ${modelBeforeGuardrails} → ${rerouteModel}`); modelStr = rerouteModel; } + } else if ( + // 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. + modelStr && + typeof body?.model === "string" && + body.model !== modelStr + ) { + body = { ...body, model: modelStr }; } telemetry.endPhase(); @@ -529,7 +563,15 @@ export async function handleChat( // Apply hook mutations body = hookCtx.body as any; if (hookCtx.model && hookCtx.model !== modelStr) { + log.info("ROUTING", `Hook model override: ${modelStr} → ${hookCtx.model}`); modelStr = hookCtx.model; + // Keep body.model in lockstep so later stages don't re-read a stale body model. + if (typeof body?.model !== "string" || body.model !== modelStr) { + body = { ...body, model: modelStr }; + } + } else if (modelStr && typeof body?.model === "string" && body.model !== modelStr) { + // Hook rewrote body without updating model — restore the routing model. + body = { ...body, model: modelStr }; } // Short-circuit if a hook returned a direct response diff --git a/src/sse/handlers/resolveRoutingModel.ts b/src/sse/handlers/resolveRoutingModel.ts index c4c466b88e3..1ec5c5c6e42 100644 --- a/src/sse/handlers/resolveRoutingModel.ts +++ b/src/sse/handlers/resolveRoutingModel.ts @@ -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,25 @@ 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( + 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, + }; +} diff --git a/tests/unit/guardrails/visionBridge.test.ts b/tests/unit/guardrails/visionBridge.test.ts index c59713b3717..b4a8dcbef5e 100644 --- a/tests/unit/guardrails/visionBridge.test.ts +++ b/tests/unit/guardrails/visionBridge.test.ts @@ -38,6 +38,9 @@ function createGuardrail(options?: Parameters[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 | 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 | 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 + ); +}); diff --git a/tests/unit/resolve-routing-model.test.ts b/tests/unit/resolve-routing-model.test.ts index b9a3c53b9eb..3b41a8121ad 100644 --- a/tests/unit/resolve-routing-model.test.ts +++ b/tests/unit/resolve-routing-model.test.ts @@ -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) { 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"); + }); +}); From cb65743b194d9b4e1c9ba4f7abc8efe7a97596ce Mon Sep 17 00:00:00 2001 From: artickc Date: Wed, 15 Jul 2026 10:59:16 -0300 Subject: [PATCH 4/5] fix(guardrails/chat): keep chat.ts under the file-size ratchet and update stale vision-bridge tests for the credential-aware reroute skip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- src/sse/handlers/chat.ts | 81 +++--------- src/sse/handlers/resolveRoutingModel.ts | 116 ++++++++++++++++++ .../vision-bridge-callmodel.test.ts | 22 +++- .../vision-bridge-policy-reroute-6640.test.ts | 80 ++++++++---- 4 files changed, 205 insertions(+), 94 deletions(-) diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index a5a9a08eae9..d5d3e375ba1 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -1,7 +1,7 @@ import { randomUUID } from "crypto"; import { resolveChatRequestBody } from "./requestBody"; import { normalizeReasoningRequest } from "@/shared/reasoning/effortStandardization"; -import { alignBodyModelWithRouting, resolveRoutingModel } from "./resolveRoutingModel"; +import { resolveRoutingModel, RoutingModelOps } from "./resolveRoutingModel"; import { getProviderCredentialsWithQuotaPreflight, markAccountUnavailable, @@ -368,21 +368,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. Without this, the - // post-guardrail block below treats "body.model !== modelStr" as a vision- - // bridge reroute and silently restores the original body model — undoing - // X-Route-Model (e.g. header zai/glm-5.2 + body opencode-zen/gpt-5.4 → 401 - // Missing API key while HTTP logs still show zai). - { - const aligned = alignBodyModelWithRouting(body, modelStr); - body = aligned.body; - if (aligned.aligned) { - log.info( - "ROUTING", - `Aligned body.model to routing model: ${aligned.previousModel || "(none)"} → ${modelStr}` - ); - } - } + // 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; @@ -485,43 +472,19 @@ export async function handleChat( preCallGuardrails.message || "Request rejected: suspicious content detected" ); } - // Snapshot model BEFORE applying the guardrail payload so we only treat a - // genuine guardrail mutation (e.g. Vision Bridge) as a reroute — not a stale - // body.model that never matched X-Route-Model / the aligned routing model. + // 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 !== modelBeforeGuardrails - ) { - 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 { - log.info("ROUTING", `Guardrail model reroute: ${modelBeforeGuardrails} → ${rerouteModel}`); - modelStr = rerouteModel; - } - } else if ( - // 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. - modelStr && - typeof body?.model === "string" && - body.model !== modelStr - ) { - body = { ...body, model: modelStr }; - } + ({ body, modelStr } = await RoutingModelOps.reconcileGuardrailReroute({ + body, + modelBeforeGuardrails, + modelStr, + apiKey, + apiKeyId: apiKeyInfo?.id, + isModelAllowedForKey, + log, + })); telemetry.endPhase(); // T08: per-key active session limit (0 = unlimited). @@ -562,17 +525,13 @@ export async function handleChat( // Apply hook mutations body = hookCtx.body as any; - if (hookCtx.model && hookCtx.model !== modelStr) { - log.info("ROUTING", `Hook model override: ${modelStr} → ${hookCtx.model}`); - modelStr = hookCtx.model; - // Keep body.model in lockstep so later stages don't re-read a stale body model. - if (typeof body?.model !== "string" || body.model !== modelStr) { - body = { ...body, model: modelStr }; - } - } else if (modelStr && typeof body?.model === "string" && body.model !== modelStr) { - // Hook rewrote body without updating model — restore the routing model. - body = { ...body, model: modelStr }; - } + ({ 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) { diff --git a/src/sse/handlers/resolveRoutingModel.ts b/src/sse/handlers/resolveRoutingModel.ts index 1ec5c5c6e42..347abcc08d2 100644 --- a/src/sse/handlers/resolveRoutingModel.ts +++ b/src/sse/handlers/resolveRoutingModel.ts @@ -43,3 +43,119 @@ export function alignBodyModelWithRouting( 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( + 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(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(params: { + body: T; + modelBeforeGuardrails: string; + modelStr: string; + apiKey: string | null | undefined; + apiKeyId: string | undefined; + isModelAllowedForKey: (apiKey: string | null | undefined, model: string) => Promise; + 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, +}; diff --git a/tests/unit/guardrails/vision-bridge-callmodel.test.ts b/tests/unit/guardrails/vision-bridge-callmodel.test.ts index 74b9ee65319..8408ba8be4d 100644 --- a/tests/unit/guardrails/vision-bridge-callmodel.test.ts +++ b/tests/unit/guardrails/vision-bridge-callmodel.test.ts @@ -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" ); }); diff --git a/tests/unit/vision-bridge-policy-reroute-6640.test.ts b/tests/unit/vision-bridge-policy-reroute-6640.test.ts index 62a5ae0d4ff..e710bb068bb 100644 --- a/tests/unit/vision-bridge-policy-reroute-6640.test.ts +++ b/tests/unit/vision-bridge-policy-reroute-6640.test.ts @@ -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)" ); }); From a9b1d42290472bbaec9840ef7a8130ded794a507 Mon Sep 17 00:00:00 2001 From: artickc Date: Wed, 15 Jul 2026 11:13:50 -0300 Subject: [PATCH 5/5] fix(guardrails/chat): reduce isProviderConnectionUsable cyclomatic complexity to satisfy the project-wide complexity ratchet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/lib/guardrails/visionBridge.ts | 44 +++++++++++++++++++----------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/lib/guardrails/visionBridge.ts b/src/lib/guardrails/visionBridge.ts index 27d0441d5b5..1229b418b48 100644 --- a/src/lib/guardrails/visionBridge.ts +++ b/src/lib/guardrails/visionBridge.ts @@ -104,34 +104,46 @@ export interface VisionBridgeDependencies { * `noauth` with no real API key is NOT usable (opencode-zen free tier often * surfaces as noauth and then 401 "Missing API key"). */ -export function isProviderConnectionUsable(connection: { +type ProviderConnectionLike = { authType?: string | null; apiKey?: string | null; accessToken?: string | null; refreshToken?: string | null; idToken?: string | null; testStatus?: string | null; -}): boolean { +}; + +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 (status === "disabled" || status === "banned" || status === "expired") { + if (TERMINAL_CONNECTION_STATUSES.has(status)) { return false; } + const auth = String(connection.authType || "").toLowerCase(); - const hasKey = typeof connection.apiKey === "string" && connection.apiKey.trim().length > 0; - if (auth === "noauth" || auth === "none" || auth === "") { - // Free/noauth only counts when a real key is still present. - return hasKey; - } - if (auth === "apikey" || auth === "cookie") { + const hasKey = hasNonEmptyString(connection.apiKey); + + if (KEY_ONLY_AUTH_TYPES.has(auth)) { return hasKey; } - if (auth === "oauth" || auth === "access_token" || auth === "external_idp") { - return Boolean( - (connection.refreshToken && String(connection.refreshToken).trim()) || - (connection.accessToken && String(connection.accessToken).trim()) || - (connection.idToken && String(connection.idToken).trim()) || - hasKey - ); + if (TOKEN_AUTH_TYPES.has(auth)) { + return hasOAuthCredential(connection) || hasKey; } return hasKey; }