From 25bb8d8500d307c8e14f2b095ab4d94a7a7b09ac Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Thu, 9 Jul 2026 05:38:41 -0300 Subject: [PATCH] fix(resilience): resolve fp-pinned combo account back to real connection id (#6696) --- CHANGELOG.md | 1 + .../services/combo/fingerprintExpansion.ts | 45 +++++++ open-sse/services/combo/types.ts | 8 ++ tests/unit/combo-fingerprint-pin-6696.test.ts | 123 ++++++++++++++++++ 4 files changed, 177 insertions(+) create mode 100644 tests/unit/combo-fingerprint-pin-6696.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bba532ec2..fa08eaa2bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral ### 🐛 Bug Fixes +- **fix(resilience):** a combo step "pinned" to one fingerprint account (mimocode/mcode/opencode multi-account providers) never actually resolved to that account, so it couldn't fail over when the pinned account was depleted ([#6696](https://github.com/diegosouzapw/OmniRoute/issues/6696), relates #6612) — the combo builder UI encodes an account pin as a composite connectionId (`${rowId}|fp|${fingerprint}`, `src/lib/combos/builderOptions.ts`), but `expandTargetsByFingerprints()` (`open-sse/services/combo/fingerprintExpansion.ts`) looked that composite string up directly in `connectionById` (keyed by real DB row ids), got `undefined`, and passed the target through unchanged, still carrying the bogus composite id — so downstream credential resolution could never match it either. `expandTargetsByFingerprints()` now splits the `|fp|` composite id back into the real connection row id + the pinned fingerprint (new `splitFingerprintPin()` helper) before any lookup, resolving the target to the real connectionId (with the pinned fingerprint carried on the new `pinnedFingerprint` field) instead of the inert composite string. Regression guard: `tests/unit/combo-fingerprint-pin-6696.test.ts`. - **fix(cli):** per-agent AgentBridge DNS toggle was broken for 8 of the 9 supported agents, and a failed MITM startup step could orphan the spawned proxy child — `addDNSEntry`/`removeDNSEntry` (`src/mitm/dns/dnsConfig.ts`) always resolved the legacy Antigravity default hosts regardless of which agent's toggle was flipped, so enabling DNS for Cursor/Codex/Claude Code/etc. silently added only `daily-cloudcode-pa.googleapis.com` while the DB recorded `dns_enabled=true` for the selected agent. Both functions now accept an optional `agentId` and resolve hosts via `ALL_TARGETS`; `POST /api/tools/agent-bridge/agents/[id]/dns` passes the route's `id` through and now returns 404 for an id that doesn't match a known target instead of silently falling back. Separately, `startMitmInternal()` (`src/mitm/manager.ts`) now wraps `generateCert()` (log + rethrow), the `provisionDnsEntries()` call, and the PID-file write in try/catch so a mid-startup failure can't orphan the already-spawned MITM child process. On Windows, `addDNSEntries`/`removeDNSEntries` also batch every missing/present entry into a single elevated PowerShell invocation instead of one UAC prompt per host line. Regression guard: `tests/unit/dns-config-generic.test.ts` (agent-specific resolution + batching), `tests/unit/agent-bridge-dns-route-validation.test.ts` (404 for unknown agent id). ([#6338](https://github.com/diegosouzapw/OmniRoute/pull/6338) — thanks @hamsa0x7) - **fix(guardrails):** Vision Bridge's individual-model auto-reroute (route an image-bearing request straight to a vision-capable model instead of describe-then-forward) could bypass a policy-restricted API key's model allowlist/budget ([#6640](https://github.com/diegosouzapw/OmniRoute/pull/6640)) — `VisionBridgeGuardrail.preCall()` (`src/lib/guardrails/visionBridge.ts`) swaps `body.model` to the best available vision-capable model, but that swap happens in the guardrail pipeline AFTER `chat.ts` already called `enforceApiKeyPolicy()` against the ORIGINAL model, so a key scoped to a narrow `allowedModels` list could still execute against an unvetted (and possibly costlier) vision model the reroute picked. `chat.ts` now re-validates any guardrail-driven model change against the same per-key allowlist (`isModelAllowedForKey`) before honoring it, falling back to the original already-approved model when the reroute target is not allowed. The reroute path also now honors an explicit `settings.visionBridgeModel` operator override (previously ignored, unlike the combo/describe path a few lines below it, which already respects it via `getVisionBridgeConfig`). Regression guard: `tests/unit/guardrails/visionBridge.test.ts` (22 tests). (thanks @herjarsa) - **fix(auth):** an API key restricted via `allowedModels`/`allowedCombos` could bypass that restriction entirely over the Codex Responses-over-WebSocket bridge ([#6564](https://github.com/diegosouzapw/OmniRoute/issues/6564)) — `prepare()` in `src/app/api/internal/codex-responses-ws/route.ts` authenticated the WS bridge's API key (`authenticate()`/`authorizeWebSocketHandshake()`) and honored `allowedConnections`, but never called `enforceApiKeyPolicy()`, the same model/combo policy gate the HTTP `/v1/responses` path enforces via `handleChat()` — so a key scoped to e.g. `combo/model-1.0` could still reach a direct Codex model like `gpt-5.5` through this transport, as long as an eligible Codex OAuth connection existed. The bridge's WS auth token arrives via query params (`api_key`/`token`/`access_token`), not a normal `Authorization` header, so a new `enforceCodexWsApiKeyPolicy()` builds an equivalent `Request` carrying an explicit `Authorization: Bearer ` header and calls `enforceApiKeyPolicy()` against the CLIENT-requested model, before any Codex-specific model remapping or credential selection. Regression guard: `tests/unit/codex-ws-policy-enforcement-6564.test.ts` (a model-restricted key is rejected 403 before reaching credential selection; a combo-restricted key is rejected 403 requesting a disallowed combo; a key that DOES allow the requested model still proceeds past policy). diff --git a/open-sse/services/combo/fingerprintExpansion.ts b/open-sse/services/combo/fingerprintExpansion.ts index c96bb3ad05..be3d511509 100644 --- a/open-sse/services/combo/fingerprintExpansion.ts +++ b/open-sse/services/combo/fingerprintExpansion.ts @@ -17,11 +17,35 @@ import type { ResolvedComboTarget } from "./types.ts"; /** Providers whose `providerSpecificData.fingerprints` array should be expanded. */ const FINGERPRINT_PROVIDERS: ReadonlySet = new Set(["mimocode", "mcode", "opencode"]); +/** Separator the combo builder UI uses to encode an account pin (#6087). */ +const FP_PIN_SEPARATOR = "|fp|"; + /** Check whether a provider uses fingerprint-based multi-account. */ export function isFingerprintProvider(provider: string): boolean { return FINGERPRINT_PROVIDERS.has(provider); } +/** + * Split a combo builder "pinned account" connectionId (`${rowId}|fp|${fingerprint}`, + * produced by `expandConnectionOptions` in `src/lib/combos/builderOptions.ts`) back + * into the real DB connection row id and the pinned fingerprint (#6696). + * + * Returns `null` when `connectionId` does not carry the pin separator, so callers can + * fall through to the unpinned resolution path unchanged. + */ +export function splitFingerprintPin( + connectionId: string +): { realConnectionId: string; pinnedFingerprint: string } | null { + const separatorIndex = connectionId.indexOf(FP_PIN_SEPARATOR); + if (separatorIndex === -1) return null; + + const realConnectionId = connectionId.slice(0, separatorIndex); + const pinnedFingerprint = connectionId.slice(separatorIndex + FP_PIN_SEPARATOR.length); + if (!realConnectionId || !pinnedFingerprint) return null; + + return { realConnectionId, pinnedFingerprint }; +} + /** Safely extract the fingerprints array from a connection record. */ export function getConnectionFingerprints( connection: Record | undefined | null @@ -84,6 +108,27 @@ export function expandTargetsByFingerprints( continue; } + // #6696: the combo builder UI pins a specific account by encoding it as + // `${rowId}|fp|${fingerprint}`. That composite string never matches a real + // DB row id in `connectionById`, so resolve it back to the real + // connectionId + the pinned fingerprint here before any other lookup — + // otherwise the pin is silently inert and credential resolution can never + // find the connection at all. + const pin = splitFingerprintPin(connectionId); + if (pin) { + result.push({ + ...target, + connectionId: pin.realConnectionId, + pinnedFingerprint: pin.pinnedFingerprint, + executionKey: buildFingerprintExecutionKey( + target.executionKey, + pin.pinnedFingerprint, + false + ), + }); + continue; + } + const connection = connectionById.get(connectionId); const fingerprints = getConnectionFingerprints(connection); diff --git a/open-sse/services/combo/types.ts b/open-sse/services/combo/types.ts index 290b0964d7..ec3bac65b9 100644 --- a/open-sse/services/combo/types.ts +++ b/open-sse/services/combo/types.ts @@ -151,6 +151,14 @@ export type ResolvedComboTarget = { label: string | null; failoverBeforeRetry?: unknown; trafficType?: "production" | "shadow"; + /** + * Fingerprint-based account pin resolved from a combo builder composite + * connectionId (`${rowId}|fp|${fingerprint}`, see + * `expandTargetsByFingerprints` in `./fingerprintExpansion.ts`, #6696). + * Set only for fingerprint-provider targets (mimocode/mcode/opencode) that + * were pinned to one specific account. + */ + pinnedFingerprint?: string; }; export type ShadowRoutingConfig = { diff --git a/tests/unit/combo-fingerprint-pin-6696.test.ts b/tests/unit/combo-fingerprint-pin-6696.test.ts new file mode 100644 index 0000000000..6a683f49e8 --- /dev/null +++ b/tests/unit/combo-fingerprint-pin-6696.test.ts @@ -0,0 +1,123 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +// #6696 — the combo builder's "pin a specific account" feature for fingerprint +// providers (mimocode/mcode/opencode) builds a composite connectionId of the +// form `${rowId}|fp|${fingerprint}` (src/lib/combos/builderOptions.ts:251), but +// nothing in the combo execution path ever splits that composite id back into +// a real rowId + a selected fingerprint. This test proves the pin is inert: +// once a combo step is configured with the composite id produced by the +// builder, `expandTargetsByFingerprints` (the function combo.ts calls right +// before target resolution/credential lookup) cannot find the connection in +// `connectionById` (which is keyed by the real DB row id) and silently passes +// the target through UNCHANGED, still carrying the bogus composite +// connectionId. Downstream, `getProviderCredentials`'s `forcedConnectionId` +// filter (src/sse/services/auth.ts) also can never match `conn.id === +// "|fp|"` against a real row id — so the pinned target +// never resolves to real credentials for the intended (or ANY) account and +// the combo step is effectively dead weight instead of a working, fail-over- +// capable target. + +const { expandTargetsByFingerprints } = await import( + "../../open-sse/services/combo/fingerprintExpansion.ts" +); + +function makeTarget(overrides: Record = {}) { + return { + kind: "model" as const, + stepId: "step-0", + executionKey: "step-0", + modelStr: "mimocode/mimo-auto", + provider: "mimocode", + providerId: null, + connectionId: "conn-1", + weight: 0, + label: null, + ...overrides, + }; +} + +test("#6696: fp-pinned composite connectionId is never resolved to the real connection + fingerprint", () => { + const realConnectionId = "conn-1"; + const conn = { + id: realConnectionId, + provider: "mimocode", + providerSpecificData: { fingerprints: ["fp-aaa", "fp-bbb"] }, + }; + const connById = new Map([[realConnectionId, conn]]); + + // Exactly what the combo builder UI persists for a step pinned to + // "Account 1" — src/lib/combos/builderOptions.ts:251: + // id: `${connection.id}|fp|${fingerprints[i]}` + const pinnedFingerprint = "fp-aaa"; + const compositeConnectionId = `${realConnectionId}|fp|${pinnedFingerprint}`; + + const targets = [makeTarget({ connectionId: compositeConnectionId })]; + + const result = expandTargetsByFingerprints(targets, connById, (t) => t.provider); + + assert.equal(result.length, 1, "pin should resolve to exactly one target"); + + // This is what SHOULD hold once fixed: connectionId fed downstream must be + // the real DB row id, not the UI-only composite string. + assert.equal( + result[0].connectionId, + realConnectionId, + "fp-pinned target must resolve to the real connection id for credential lookup to succeed" + ); + + // The selected fingerprint must be threaded through so downstream execution + // (and future account-scoped cooldown/lockout) can still tell which account + // was pinned, instead of losing that information once the composite id is + // unwrapped. + assert.equal( + (result[0] as Record).pinnedFingerprint, + pinnedFingerprint, + "the pinned fingerprint must survive resolution so downstream execution can target that account" + ); +}); + +test("#6696: composite connectionId never matches connectionById (root cause of the inert pin)", () => { + const realConnectionId = "conn-1"; + const conn = { + id: realConnectionId, + provider: "mimocode", + providerSpecificData: { fingerprints: ["fp-aaa", "fp-bbb"] }, + }; + const connById = new Map([[realConnectionId, conn]]); + const compositeConnectionId = `${realConnectionId}|fp|fp-aaa`; + + assert.equal( + connById.get(compositeConnectionId), + undefined, + "composite fp-pin id must not resolve directly against connectionById" + ); +}); + +test("#6696: a pin to an unknown connection does not crash and leaves the target inert but not thrown away", () => { + const connById = new Map(); + const targets = [makeTarget({ connectionId: "missing-conn|fp|fp-zzz" })]; + + const result = expandTargetsByFingerprints(targets, connById, (t) => t.provider); + + assert.equal(result.length, 1); + assert.equal(result[0].connectionId, "missing-conn"); + assert.equal((result[0] as Record).pinnedFingerprint, "fp-zzz"); +}); + +test("#6696: non-fingerprint providers are unaffected by the |fp| split", () => { + const connById = new Map([ + ["conn-1", { id: "conn-1", provider: "openai", providerSpecificData: {} }], + ]); + const targets = [ + makeTarget({ provider: "openai", modelStr: "openai/gpt-4", connectionId: "conn-1|fp|fp-aaa" }), + ]; + + const result = expandTargetsByFingerprints(targets, connById, (t) => t.provider); + + assert.equal(result.length, 1); + // Non-fingerprint providers are passed through unchanged — the literal + // string (however unusual) is left alone since this provider never goes + // through the fingerprint-pin UI flow. + assert.equal(result[0].connectionId, "conn-1|fp|fp-aaa"); +});