mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-15 11:43:10 +03:00
fix(resilience): resolve fp-pinned combo account back to real connection id (#6696)
This commit is contained in:
@@ -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 <apiKey>` 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).
|
||||
|
||||
@@ -17,11 +17,35 @@ import type { ResolvedComboTarget } from "./types.ts";
|
||||
/** Providers whose `providerSpecificData.fingerprints` array should be expanded. */
|
||||
const FINGERPRINT_PROVIDERS: ReadonlySet<string> = 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<string, unknown> | 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);
|
||||
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
123
tests/unit/combo-fingerprint-pin-6696.test.ts
Normal file
123
tests/unit/combo-fingerprint-pin-6696.test.ts
Normal file
@@ -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 ===
|
||||
// "<rowId>|fp|<fingerprint>"` 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<string, unknown> = {}) {
|
||||
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<string, unknown>).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<string, unknown>).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");
|
||||
});
|
||||
Reference in New Issue
Block a user