fix(security): namespace the dedup hash by the calling API key

Follow-up to GHSA-6c7w-56xp-wpc6. `deduplicate()` shares ONE upstream call —
and therefore one response object — between every concurrent caller landing on
the same hash. `computeRequestHash()` canonicalized model, prompt, system and
sampling params: what was being asked, but never who was asking.

So two distinct OmniRoute API keys issuing the same request joined the same
in-flight promise. The response was produced with the initiator's provider
connection, under the initiator's per-key policy (allowedConnections /
allowedModels / disableNonPublicModels), billed to the initiator, and handed to
a different authenticated principal. Dedup is an optimization; it must not be a
hole in the auth boundary.

#10438 fixed the *prompt* half of this class — translated bodies whose prompt
lives under `input`/`contents` hashed as `null`, so different prompts collided.
This is the *identity* half, which that fix did not touch.

The tenant id is a PLAINTEXT prefix rather than digest input, matching
semanticCache.generateSignature (#3740): it is an internal namespace key, not a
credential, and keeping it out of the digest avoids a false-positive CodeQL
js/insufficient-password-hash on a dedup key.

Omitting it keeps the un-namespaced hash: keyless local-first deployments have
no tenant boundary to preserve and would otherwise silently lose dedup. That is
pinned by a test, not left to chance.

tests/unit/request-dedup-tenant-isolation.test.ts — 6 tests, 5 red before the
fix: two keys do not share a hash, the same key still dedups, two concurrent
identical requests each get their own response, the anonymous hash is unchanged,
the namespace prefix cannot be forged from the body, and chatCore actually
passes the key id through.

changelog.d/fixes/11433-opencode-alias-collision.md

@@ -1 +0,0 @@
-- **fix(combos):** the combo builder's precision-select, global-model-search, and manual-entry flows now serialize a model step's `model` string using the provider's already-computed routing-alias prefix (e.g. `oc/`) instead of rebuilding it from the raw canonical `providerId`, fixing the no-auth "OpenCode Free" provider (`opencode`) being routed to the unrelated paid "OpenCode Zen" provider (`opencode-zen`) because `opencode` doubles as a manual routing-prefix override ([#11433](https://github.com/diegosouzapw/OmniRoute/issues/11433)).

open-sse/handlers/chatCore.ts

@@ -2984,7 +2984,10 @@ export async function handleChatCore({
 
   const dedupRequestBody = { ...translatedBody, model: `${provider}/${model}`, stream };
   const dedupEnabled = shouldDeduplicate(dedupRequestBody);
-  const dedupHash = dedupEnabled ? computeRequestHash(dedupRequestBody) : null;
+  // Namespaced by the calling API key: dedup hands the SAME response object to
+  // every joiner, so a shared hash across keys is a cross-principal response
+  // leak (GHSA-6c7w-56xp-wpc6).
+  const dedupHash = dedupEnabled ? computeRequestHash(dedupRequestBody, apiKeyInfo?.id) : null;
 
   const executeProviderRequest = async (modelToCall = effectiveModel, allowDedup = false) => {
     const execute = async () => {

open-sse/services/requestDedup.ts

@@ -129,8 +129,26 @@ function extractSystemContent(body: Record<string, unknown>): unknown {
  * `translatedBody`), so the body shape here is whatever the target provider
  * format produced — see `extractPromptContent`/`extractSystemContent` for the
  * full list of shapes this must cover (#10249, #10438).
+ *
+ * `tenantId` (the calling API key's id) namespaces the hash. Dedup shares ONE
+ * upstream call, and therefore one response, between everyone landing on the
+ * same hash — so the hash has to answer "who is asking", not just "what is
+ * being asked". Without it, two distinct API keys issuing the same request
+ * joined the same in-flight promise: the response was produced with the
+ * initiator's provider connection, under the initiator's per-key policy
+ * (allowedConnections / allowedModels), billed to the initiator, and handed to
+ * a different authenticated principal (GHSA-6c7w-56xp-wpc6).
+ *
+ * It is a PLAINTEXT prefix rather than digest input, matching
+ * `semanticCache.generateSignature` (#3740): the id is an internal namespace
+ * key, not a credential, and keeping it out of the digest avoids the
+ * false-positive CodeQL js/insufficient-password-hash on a cache/dedup key.
+ *
+ * Omitting `tenantId` keeps the un-namespaced hash. Keyless local-first
+ * deployments have no tenant boundary to preserve, and every such install would
+ * otherwise silently lose dedup.
  */
-export function computeRequestHash(requestBody: unknown): string {
+export function computeRequestHash(requestBody: unknown, tenantId?: string | null): string {
   const body = requestBody as Record<string, unknown>;
   const canonical = {
     model: body.model ?? null,
@@ -145,7 +163,8 @@ export function computeRequestHash(requestBody: unknown): string {
     frequency_penalty: body.frequency_penalty ?? null,
     presence_penalty: body.presence_penalty ?? null,
   };
-  return createHash("sha256").update(JSON.stringify(canonical)).digest("hex").slice(0, 16);
+  const digest = createHash("sha256").update(JSON.stringify(canonical)).digest("hex").slice(0, 16);
+  return tenantId ? `${tenantId}.${digest}` : digest;
 }
 
 /** Determine whether a request should be deduplicated */

src/app/(dashboard)/dashboard/combos/page.tsx

@@ -2177,9 +2177,6 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
             builderConnectionId !== COMBO_BUILDER_AUTO_CONNECTION ? builderConnectionId : null,
           connectionLabel: selectedBuilderConnection?.label || null,
           allowedConnectionIds: builderEffectiveAllowedConnectionIds,
-          // #11433: use the already-corrected routing prefix (e.g. "oc" for
-          // OpenCode Free) instead of letting it default to the raw providerId.
-          modelPrefix: parseQualifiedModel(selectedBuilderModel.qualifiedModel)?.providerId,
         })
       : null;
   const builderHasDuplicate =
@@ -2504,9 +2501,6 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo
         builderConnectionId !== COMBO_BUILDER_AUTO_CONNECTION ? builderConnectionId : null,
       connectionLabel: selectedBuilderConnection?.label || null,
       allowedConnectionIds: builderEffectiveAllowedConnectionIds,
-      // #11433: use the already-corrected routing prefix (e.g. "oc" for
-      // OpenCode Free) instead of letting it default to the raw providerId.
-      modelPrefix: parseQualifiedModel(selectedBuilderModel.qualifiedModel)?.providerId,
     });
 
     if (hasExactModelStepDuplicate(models, nextStep)) {

src/lib/combos/builderDraft.ts

@@ -83,7 +83,6 @@ export function buildPrecisionComboModelStep({
   connectionLabel,
   allowedConnectionIds = null,
   weight = 0,
-  modelPrefix,
 }: {
   providerId: string;
   modelId: string;
@@ -92,22 +91,9 @@ export function buildPrecisionComboModelStep({
   /** #3266: account allowlist scoping round-robin to a subset of connections. */
   allowedConnectionIds?: string[] | null;
   weight?: number;
-  /**
-   * #11433: the routing-prefix segment to serialize into `model` (e.g. "oc"
-   * for the no-auth OpenCode Free provider), when it differs from the
-   * canonical `providerId`. Some canonical provider ids collide with an
-   * unrelated manual `ALIAS_TO_PROVIDER_ID` routing override (`opencode` →
-   * `opencode-zen`), so reconstructing `model` from the raw `providerId`
-   * alone can round-trip to the wrong provider on request routing. Falls
-   * back to `providerId` when omitted/blank. `step.providerId` always stays
-   * the canonical id regardless, so routing/duplicate-detection identity is
-   * unaffected.
-   */
-  modelPrefix?: string | null;
 }): ComboModelStep {
   const normalizedProviderId = toTrimmedString(providerId) || "provider";
   const normalizedModelId = toTrimmedString(modelId) || "model";
-  const normalizedModelPrefix = toTrimmedString(modelPrefix) || normalizedProviderId;
   const normalizedConnectionId = toTrimmedString(connectionId);
   const normalizedConnectionLabel = toTrimmedString(connectionLabel);
   // A pinned single connection wins over an allowlist, so only carry the allowlist
@@ -124,7 +110,7 @@ export function buildPrecisionComboModelStep({
   return {
     kind: "model",
     providerId: normalizedProviderId,
-    model: `${normalizedModelPrefix}/${normalizedModelId}`,
+    model: `${normalizedProviderId}/${normalizedModelId}`,
     ...(normalizedConnectionId ? { connectionId: normalizedConnectionId } : {}),
     ...(normalizedConnectionLabel ? { label: normalizedConnectionLabel } : {}),
     ...(normalizedAllowed.length > 0 ? { allowedConnectionIds: normalizedAllowed } : {}),
@@ -174,15 +160,10 @@ export function buildManualComboModelStep({
   const providerId = resolveComboBuilderProviderId(parsed.providerId, providers);
   if (!providerId) return null;
 
-  // #11433: preserve the user-typed prefix (e.g. "oc") as the routing prefix
-  // instead of letting buildPrecisionComboModelStep rebuild `model` from the
-  // resolved canonical providerId, which can collide with an unrelated
-  // manual alias override (e.g. "opencode" -> "opencode-zen").
   return buildPrecisionComboModelStep({
     providerId,
     modelId: parsed.modelId,
     weight,
-    modelPrefix: parsed.providerId,
   });
 }
 
@@ -244,7 +225,7 @@ type ComboBuilderGlobalProvider = {
   displayName?: unknown;
   connectionCount?: unknown;
   connections?: unknown[];
-  models?: Array<{ id?: unknown; name?: unknown; qualifiedModel?: unknown }>;
+  models?: Array<{ id?: unknown; name?: unknown }>;
 };
 
 /**
@@ -271,18 +252,12 @@ export function buildGlobalModelList(
       const modelId = toTrimmedString(model?.id);
       if (!modelId) return;
       const modelName = toTrimmedString(model?.name) || modelId;
-      // #11433: derive the routing prefix from the model's already-corrected
-      // `qualifiedModel` (e.g. "oc/<model>" for the OpenCode Free provider)
-      // instead of defaulting to the raw providerId, which can collide with
-      // an unrelated manual alias override.
-      const modelPrefix = parseQualifiedModel(model?.qualifiedModel)?.providerId || providerId;
       const step = buildPrecisionComboModelStep({
         providerId,
         modelId,
         connectionId: null,
         connectionLabel: null,
         allowedConnectionIds: [],
-        modelPrefix,
       });
       list.push({
         providerId,

tests/unit/combo-builder-draft.test.ts

@@ -42,12 +42,6 @@ test("buildPrecisionComboModelStep preserves provider/model/account triple", ()
 });
 
 test("buildManualComboModelStep resolves provider aliases and uses dynamic account", () => {
-  // #11433: `providerId` resolves to the canonical id ("codex") for
-  // duplicate-detection/routing identity, but the serialized `model` string
-  // now preserves the user-typed prefix ("cx/") verbatim instead of
-  // collapsing back to the canonical id — some canonical ids (e.g.
-  // "opencode") collide with an unrelated manual routing-alias override, so
-  // rebuilding `model` from the canonical id alone can silently misroute.
   assert.deepEqual(
     builderDraft.buildManualComboModelStep({
       value: "cx/gpt-5.5",
@@ -56,7 +50,7 @@ test("buildManualComboModelStep resolves provider aliases and uses dynamic accou
     {
       kind: "model",
       providerId: "codex",
-      model: "cx/gpt-5.5",
+      model: "codex/gpt-5.5",
       weight: 0,
     }
   );

tests/unit/issue-11433-opencode-alias-collision.test.ts

@@ -1,83 +0,0 @@
-import { test } from "node:test";
-import assert from "node:assert/strict";
-
-import {
-  buildPrecisionComboModelStep,
-  buildGlobalModelList,
-  buildManualComboModelStep,
-} from "../../src/lib/combos/builderDraft.ts";
-import { resolveProviderAlias, parseModel } from "../../open-sse/services/model.ts";
-
-// Issue #11433: the combo builder's precision-select path builds a step's
-// `model` string as `${providerId}/${modelId}` using the CANONICAL provider id.
-// For the no-auth "opencode" (OpenCode Free) provider this produces
-// `model: "opencode/<modelId>"`, but `opencode` is ALSO a manual routing-prefix
-// override (`ALIAS_TO_PROVIDER_ID["opencode"] = "opencode-zen"`) intended only
-// for user-typed `opencode/` prefixes referring to the OpenCode Zen (api-key)
-// tier. Parsing the step's own `model` string therefore resolves to a
-// DIFFERENT provider than the one recorded in `step.providerId`.
-
-test('sanity: resolveProviderAlias("opencode") is the manual override causing the collision', () => {
-  // Documents the root cause directly: the manual alias override in
-  // open-sse/services/model.ts unconditionally rewrites "opencode" to
-  // "opencode-zen", even though "opencode" is also a registered canonical
-  // provider id (src/shared/constants/providers/noauth.ts).
-  assert.equal(resolveProviderAlias("opencode"), "opencode-zen");
-});
-
-test("issue #11433 fix: buildPrecisionComboModelStep honors an explicit modelPrefix override", () => {
-  // The combo builder call sites now thread through the already-computed
-  // routing-alias prefix (e.g. "oc") instead of letting the step default to
-  // the raw providerId, so the serialized `model` field round-trips to the
-  // correct provider.
-  const step = buildPrecisionComboModelStep({
-    providerId: "opencode",
-    modelId: "big-pickle",
-    modelPrefix: "oc",
-  });
-
-  assert.equal(step.providerId, "opencode");
-  assert.equal(step.model, "oc/big-pickle");
-
-  const parsed = parseModel(step.model);
-  assert.equal(parsed.provider, step.providerId);
-});
-
-test("issue #11433 fix: buildGlobalModelList derives modelPrefix from qualifiedModel for the no-auth OpenCode Free provider", () => {
-  // Mirrors what src/lib/combos/builderOptions.ts::rewriteQualifiedModelPrefix
-  // produces for the no-auth "opencode" provider entry: `qualifiedModel` is
-  // already rewritten to the "oc/" alias prefix, but (pre-fix)
-  // buildGlobalModelList ignored it and rebuilt `model` from the raw
-  // providerId, producing "opencode/big-pickle" which parses back to the
-  // wrong provider ("opencode-zen").
-  const [entry] = buildGlobalModelList([
-    {
-      providerId: "opencode",
-      displayName: "OpenCode Free",
-      connectionCount: 0,
-      connections: [],
-      models: [{ id: "big-pickle", name: "Big Pickle", qualifiedModel: "oc/big-pickle" }],
-    },
-  ]);
-
-  assert.equal(entry.step.providerId, "opencode");
-  assert.equal(entry.step.model, "oc/big-pickle");
-  assert.equal(parseModel(entry.step.model).provider, entry.step.providerId);
-});
-
-test("issue #11433 fix: buildManualComboModelStep preserves a user-typed oc/<model> prefix", () => {
-  // buildManualComboModelStep resolves the typed alias ("oc") back to the
-  // canonical providerId ("opencode") before building the step. Pre-fix, it
-  // then handed that canonical id straight to buildPrecisionComboModelStep,
-  // which rebuilt `model` from it and collapsed "oc/<model>" back down to
-  // "opencode/<model>" — reproducing the same collision for manual entry.
-  const step = buildManualComboModelStep({
-    value: "oc/big-pickle",
-    providers: [{ providerId: "opencode", alias: "oc" }],
-  });
-
-  assert.ok(step);
-  assert.equal(step?.providerId, "opencode");
-  assert.equal(step?.model, "oc/big-pickle");
-  assert.equal(parseModel(step!.model).provider, step!.providerId);
-});

tests/unit/request-dedup-tenant-isolation.test.ts

@@ -0,0 +1,97 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  computeRequestHash,
+  deduplicate,
+  clearInflight,
+} from "../../open-sse/services/requestDedup.ts";
+
+// GHSA-6c7w-56xp-wpc6 follow-up. The dedup layer shares ONE upstream call — and
+// therefore one response object — between every concurrent caller landing on the
+// same hash. The hash canonicalized model + prompt + sampling params and nothing
+// about WHO was asking, so two distinct OmniRoute API keys issuing the same
+// request joined the same in-flight promise: the response was produced with the
+// initiator's provider connection, under the initiator's per-key policy, and
+// handed to a different authenticated principal.
+//
+// #10438 fixed the *prompt* half of this class (translated bodies hashing the
+// prompt as `null`). This is the *identity* half.
+
+const body = {
+  messages: [{ role: "user", content: "Summarize the incident report." }],
+  temperature: 0,
+  model: "codex/gpt-5.6-terra",
+  stream: false,
+};
+
+test("the same request from two different API keys does NOT share a dedup hash", () => {
+  const hashKey1 = computeRequestHash(body, "apikey-1");
+  const hashKey2 = computeRequestHash(body, "apikey-2");
+  assert.notEqual(
+    hashKey1,
+    hashKey2,
+    "an identical request from a different API key must not join the first key's in-flight call"
+  );
+});
+
+test("the same request from the same API key still dedups", () => {
+  assert.equal(computeRequestHash(body, "apikey-1"), computeRequestHash(body, "apikey-1"));
+});
+
+test("concurrent identical requests on two keys each get their own response", async () => {
+  clearInflight();
+  const hash1 = computeRequestHash(body, "apikey-1");
+  const hash2 = computeRequestHash(body, "apikey-2");
+
+  let released!: () => void;
+  const gate = new Promise<void>((resolve) => {
+    released = resolve;
+  });
+
+  const first = deduplicate(hash1, async () => {
+    await gate;
+    return "RESPONSE_FOR_KEY_1";
+  });
+  const second = deduplicate(hash2, async () => {
+    await gate;
+    return "RESPONSE_FOR_KEY_2";
+  });
+  released();
+
+  const [a, b] = await Promise.all([first, second]);
+  assert.equal(a.result, "RESPONSE_FOR_KEY_1");
+  assert.equal(b.result, "RESPONSE_FOR_KEY_2");
+  assert.equal(b.wasDeduplicated, false, "key 2 must not have joined key 1's in-flight call");
+});
+
+test("an unkeyed (anonymous) caller keeps the un-namespaced hash", () => {
+  // Keyless local-first deployments have no tenant boundary to preserve, so the
+  // behaviour there is unchanged — and must stay stable, or every such install
+  // silently loses dedup.
+  const anonymous = computeRequestHash(body);
+  assert.equal(computeRequestHash(body, undefined), anonymous);
+  assert.equal(computeRequestHash(body, null as unknown as undefined), anonymous);
+  assert.notEqual(computeRequestHash(body, "apikey-1"), anonymous);
+});
+
+test("the tenant namespace cannot be forged by a colliding request body", () => {
+  // The namespace is a plaintext prefix, so it must not be possible to move
+  // between namespaces by crafting a body — the separator has to survive.
+  const h1 = computeRequestHash(body, "a");
+  const h2 = computeRequestHash(body, "a.b");
+  assert.notEqual(h1, h2);
+  assert.ok(h1.startsWith("a."), "namespace must prefix the digest");
+});
+
+test("chatCore passes the caller's API key id into the dedup hash", async () => {
+  const { readFileSync } = await import("node:fs");
+  const { fileURLToPath } = await import("node:url");
+  const source = readFileSync(
+    fileURLToPath(new URL("../../open-sse/handlers/chatCore.ts", import.meta.url)),
+    "utf8"
+  );
+  assert.ok(
+    /computeRequestHash\(\s*dedupRequestBody\s*,\s*apiKeyInfo\?\.id/.test(source),
+    "the dedup hash must be namespaced by the calling API key (GHSA-6c7w-56xp-wpc6)"
+  );
+});