mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-09 17:28:51 +00:00
fix(resilience): resolve fp-pinned combo account back to real connection id (#6696)
This commit is contained in:
parent
f4fb0d310b
commit
d54a6db1fe
4 changed files with 177 additions and 0 deletions
|
|
@ -14,6 +14,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(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).
|
||||
- **fix(security):** loopback-gate `/api/middleware/*` so a leaked JWT over a tunnel can't install or trigger a middleware hook — middleware hooks compile + run arbitrary JS via `new vm.Script` on the request hot path (`src/lib/middleware/registry.ts`), the same RCE class as the already-gated `/api/plugins/*`; `/api/middleware/` is now in `LOCAL_ONLY_API_PREFIXES` so loopback enforcement runs unconditionally before any auth check (Hard Rules #15 + #17). Regression guard: `tests/unit/route-guard-middleware-local-only.test.ts`. ([#6541](https://github.com/diegosouzapw/OmniRoute/pull/6541)) — see PR. (thanks @developerjillur)
|
||||
- **fix(startup):** AgentBridge's MITM server no longer fails to start with `ROUTER_API_KEY is required` on a normal install ([#6403](https://github.com/diegosouzapw/OmniRoute/issues/6403)) — `POST /api/tools/agent-bridge/server` resolved the spawned MITM child's router key from only an explicit `apiKey` body field (never sent by the AgentBridge UI — the schema has no such field) and the `ROUTER_API_KEY` env var (unset by default), so `startMitm()` always received `""` and the child hard-exited, even though OmniRoute already had a usable API key in its own DB. A new `resolveRouterApiKey()` now falls back to `pickApiKeyForInternalUse()` (the same DB-backed selector the combo-health-check / cloud-sync internal probes use), resolving in order: explicit key → `ROUTER_API_KEY` env → an existing DB key. Regression guard: `tests/unit/agentbridge-mitm-router-key-6403.test.ts`.
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue