mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-04 22:32:12 +03:00
This commit is contained in:
committed by
GitHub
parent
a219ab5ee3
commit
6f3d1d5216
@@ -30,6 +30,8 @@
|
||||
|
||||
### 🔧 Bug Fixes
|
||||
|
||||
- **compression (CCR retrieve via MCP HTTP):** the `omniroute_ccr_retrieve` MCP tool returned `"CCR block not found"` for blocks stored earlier in the **same** session when called over the MCP HTTP transports (SSE / Streamable HTTP), e.g. from OpenCode in a Docker deployment. Compression stores each block keyed by the API-key principal (`String(apiKeyInfo.id)`), but the tool resolved the caller via `extra.authInfo.clientId` — which the MCP SDK never populates for API-key auth — so it fell back to `"anonymous"` and the compound store-key never matched. The retrieve tool now resolves the caller's API-key id from the MCP HTTP auth context (`httpAuthContext`) using the **same** `getApiKeyMetadata` lookup used at storage time, so retrieval matches storage. Cross-tenant IDOR isolation is preserved: a different key resolves to a different id → miss; no key → the anonymous bucket only. Regression guard: `tests/unit/compression/ccr-mcp-principal-5649.test.ts` (extraction, distinct-principal isolation, fail-closed, end-to-end store→retrieve). ([#5649](https://github.com/diegosouzapw/OmniRoute/issues/5649))
|
||||
|
||||
- **compression (context-editing telemetry):** streaming responses now record Context Editing savings. Anthropic surfaces `context_management.applied_edits[]` on the final `message_delta` snapshot of an SSE stream, but the streaming reconstruction (`buildStreamSummaryFromEvents` → Claude branch) dropped `context_management` entirely **and** no telemetry hook was wired into the streaming finalizer — so the delegated server-side context-clear savings (`cleared_input_tokens` / `cleared_tool_uses`) surfaced under engine `context-editing` in compression analytics **only for non-streaming responses**. The collector now preserves `context_management` from the final snapshot (last-writer-wins), and `onStreamComplete` mirrors the non-streaming `recordContextEditingTelemetryHook` (best-effort, Claude-only, HTTP 200 only). Purely additive telemetry — no payload mutation, no new env flag, no behavior change when the stream carries no `context_management`. Regression guard: `tests/unit/context-editing-streaming-telemetry.test.ts` (3). gaps v3.8.42 — T01 (5.1).
|
||||
|
||||
- **proxy (relay test diagnostics):** the Proxy Pool "Test" button showed a bare "failed" with **nothing in the server logs** when a **relay** (Vercel / Deno / Cloudflare) *responded* with a non-200 — e.g. a `401` from an auth-token mismatch after a `STORAGE_ENCRYPTION_KEY` rotation. The relay success-path response set `success: false` but carried no `error` field, so the dashboard had no reason to show and the server logged nothing. The test now returns an actionable `error` (the HTTP status, plus an auth/encryption-key hint on `401`/`403`) and logs the failure server-side; the SOCKS5/HTTP proxy path now logs its failures too. Shaping extracted to `buildRelayTestResult` with a regression guard (`tests/unit/proxy-relay-test-error-5716.test.ts`). Note: this surfaces *why* a relay fails — it does not repair a genuinely broken/misconfigured relay. ([#5716](https://github.com/diegosouzapw/OmniRoute/issues/5716))
|
||||
|
||||
53
open-sse/mcp-server/mcpCallerIdentity.ts
Normal file
53
open-sse/mcp-server/mcpCallerIdentity.ts
Normal file
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* #5649 — resolve the MCP caller's API-key **principal id** for content stores
|
||||
* (CCR) that are keyed by principal.
|
||||
*
|
||||
* The CCR store keys blocks by `String(apiKeyInfo.id)` at compression time
|
||||
* (`chatCore` → `apiKeyInfo = getApiKeyMetadata(rawKey)`). MCP tool retrieval must
|
||||
* resolve the SAME id or the block is not found. On the MCP HTTP transports
|
||||
* (SSE / Streamable HTTP) the raw key lives in `httpAuthContext`'s
|
||||
* AsyncLocalStorage (set by `withMcpHttpAuthContext`), NOT in the tool handler's
|
||||
* `extra.authInfo` (OmniRoute authenticates with API keys, not OAuth client ids —
|
||||
* so `extra.authInfo.clientId` is never populated and the caller resolved to
|
||||
* "anonymous", producing a cross-principal store-key miss).
|
||||
*
|
||||
* Resolving through the same `getApiKeyMetadata` lookup keeps cross-tenant IDOR
|
||||
* isolation intact: a different key → a different id → a miss; no key → undefined
|
||||
* → the anonymous (`__anon__`) bucket, which only matches unauthenticated stores.
|
||||
*/
|
||||
import { getMcpHttpAuthHeadersForInternalFetch } from "./httpAuthContext.ts";
|
||||
import { extractApiKey } from "../../src/sse/services/auth.ts";
|
||||
import { getApiKeyMetadata } from "../../src/lib/db/apiKeys.ts";
|
||||
|
||||
type ApiKeyLookup = (rawKey: string) => Promise<{ id?: string | number | null } | null>;
|
||||
|
||||
/**
|
||||
* Pure resolver: given the request auth headers and a key→metadata lookup, return
|
||||
* the principal id (as a string) or `undefined`. Separated from the AsyncLocalStorage
|
||||
* read so it is unit-testable without a live transport or DB.
|
||||
*/
|
||||
export async function resolvePrincipalFromHeaders(
|
||||
headers: Record<string, string>,
|
||||
lookup: ApiKeyLookup = getApiKeyMetadata
|
||||
): Promise<string | undefined> {
|
||||
// Nothing to resolve without an Authorization / x-api-key header.
|
||||
if (!headers.Authorization && !headers["x-api-key"]) return undefined;
|
||||
const rawKey = extractApiKey({ headers: new Headers(headers) }, { allowUrl: false });
|
||||
if (!rawKey) return undefined;
|
||||
try {
|
||||
const meta = await lookup(rawKey);
|
||||
return meta?.id != null && meta.id !== "" ? String(meta.id) : undefined;
|
||||
} catch {
|
||||
// Fail closed: an unresolved principal can only reach the anonymous bucket.
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the current MCP HTTP caller's API-key principal id from the ambient
|
||||
* `httpAuthContext`. Returns `undefined` off the HTTP transport (stdio) or when the
|
||||
* request carries no API key.
|
||||
*/
|
||||
export function resolveMcpCallerApiKeyId(): Promise<string | undefined> {
|
||||
return resolvePrincipalFromHeaders(getMcpHttpAuthHeadersForInternalFetch());
|
||||
}
|
||||
@@ -250,6 +250,7 @@ import {
|
||||
commandToId,
|
||||
} from "../../services/compression/engines/rtk/index.ts";
|
||||
import { resolveCallerScopeContext } from "../scopeEnforcement.ts";
|
||||
import { resolveMcpCallerApiKeyId } from "../mcpCallerIdentity.ts";
|
||||
|
||||
const ccrRetrieveInput = z.object({
|
||||
hash: z
|
||||
@@ -410,9 +411,19 @@ export const compressionTools = {
|
||||
"Scope: read:compression. Always available (sticky-on).",
|
||||
scopes: ["read:compression"],
|
||||
inputSchema: ccrRetrieveInput,
|
||||
handler: (args: z.infer<typeof ccrRetrieveInput>, extra?: McpToolExtraLike) => {
|
||||
// Derive caller identity from MCP auth context so the retrieve is scoped to the
|
||||
// same principal that stored the block. This closes the cross-tenant IDOR (HIGH).
|
||||
handler: async (args: z.infer<typeof ccrRetrieveInput>, extra?: McpToolExtraLike) => {
|
||||
// Retrieve must use the SAME principal the CCR store used at compression time:
|
||||
// `String(apiKeyInfo.id)` (chatCore → getApiKeyMetadata(rawKey)). On MCP HTTP
|
||||
// transports the raw key lives in httpAuthContext (not in extra.authInfo, since
|
||||
// OmniRoute auth is API-key not OAuth-clientId) — resolve it to the same key id
|
||||
// so the block is found. Without this the caller resolved to "anonymous" and the
|
||||
// store-key never matched (#5649). Cross-tenant IDOR stays closed: a different
|
||||
// key → different id → miss; no key → undefined → anonymous bucket only.
|
||||
const apiKeyPrincipal = await resolveMcpCallerApiKeyId();
|
||||
if (apiKeyPrincipal) {
|
||||
return handleCcrRetrieve(args, apiKeyPrincipal);
|
||||
}
|
||||
// Fallback (unchanged): OAuth clientId / session scope context, then anonymous.
|
||||
const { callerId } = resolveCallerScopeContext(extra, ["read:compression"]);
|
||||
return handleCcrRetrieve(args, callerId === "anonymous" ? undefined : callerId);
|
||||
},
|
||||
|
||||
90
tests/unit/compression/ccr-mcp-principal-5649.test.ts
Normal file
90
tests/unit/compression/ccr-mcp-principal-5649.test.ts
Normal file
@@ -0,0 +1,90 @@
|
||||
/**
|
||||
* #5649 — CCR MCP retrieve principal resolution.
|
||||
*
|
||||
* CCR stores blocks keyed by `String(apiKeyInfo.id)` at compression time. The MCP
|
||||
* `omniroute_ccr_retrieve` tool used to resolve the caller via `extra.authInfo.clientId`
|
||||
* (never populated for API-key auth) → "anonymous" → a store-key miss ("block not
|
||||
* found"). The fix resolves the caller's API-key id from the auth headers via the SAME
|
||||
* `getApiKeyMetadata` lookup, so retrieval matches storage — without weakening
|
||||
* cross-tenant IDOR isolation.
|
||||
*/
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { resolvePrincipalFromHeaders } from "../../../open-sse/mcp-server/mcpCallerIdentity.ts";
|
||||
import {
|
||||
storeBlock,
|
||||
retrieveBlock,
|
||||
handleCcrRetrieve,
|
||||
resetCcrStore,
|
||||
} from "../../../open-sse/services/compression/engines/ccr/index.ts";
|
||||
|
||||
// A fake key→metadata lookup: maps a raw key to a DB-row id, exactly like getApiKeyMetadata.
|
||||
const fakeLookup = (map: Record<string, string>) => async (rawKey: string) =>
|
||||
map[rawKey] ? { id: map[rawKey] } : null;
|
||||
|
||||
test("#5649 resolves a Bearer API key to its principal id (not anonymous)", async () => {
|
||||
const id = await resolvePrincipalFromHeaders(
|
||||
{ Authorization: "Bearer sk-tenant-A" },
|
||||
fakeLookup({ "sk-tenant-A": "42" })
|
||||
);
|
||||
assert.equal(id, "42", "a valid Bearer key must resolve to its api-key id, not undefined/anonymous");
|
||||
});
|
||||
|
||||
test("#5649 resolves x-api-key (with anthropic-version gate) to its principal id", async () => {
|
||||
const id = await resolvePrincipalFromHeaders(
|
||||
{ "x-api-key": "sk-anthropic", "anthropic-version": "2023-06-01" },
|
||||
fakeLookup({ "sk-anthropic": "99" })
|
||||
);
|
||||
assert.equal(id, "99");
|
||||
});
|
||||
|
||||
test("#5649 distinct keys resolve to distinct principals (IDOR isolation preserved)", async () => {
|
||||
const lookup = fakeLookup({ "sk-A": "42", "sk-B": "77" });
|
||||
const a = await resolvePrincipalFromHeaders({ Authorization: "Bearer sk-A" }, lookup);
|
||||
const b = await resolvePrincipalFromHeaders({ Authorization: "Bearer sk-B" }, lookup);
|
||||
assert.equal(a, "42");
|
||||
assert.equal(b, "77");
|
||||
assert.notEqual(a, b);
|
||||
});
|
||||
|
||||
test("#5649 no auth headers → undefined (never calls the lookup)", async () => {
|
||||
let called = false;
|
||||
const id = await resolvePrincipalFromHeaders({}, async () => {
|
||||
called = true;
|
||||
return { id: "x" };
|
||||
});
|
||||
assert.equal(id, undefined);
|
||||
assert.equal(called, false, "must not hit the DB when there is no key");
|
||||
});
|
||||
|
||||
test("#5649 unknown / unresolvable key → undefined (fail closed to anonymous bucket)", async () => {
|
||||
const id = await resolvePrincipalFromHeaders(
|
||||
{ Authorization: "Bearer sk-unknown" },
|
||||
fakeLookup({ "sk-A": "42" })
|
||||
);
|
||||
assert.equal(id, undefined);
|
||||
});
|
||||
|
||||
test("#5649 end-to-end: a block stored under the api-key id is retrievable by the resolved principal, not by another tenant", async () => {
|
||||
resetCcrStore();
|
||||
const lookup = fakeLookup({ "sk-A": "42", "sk-B": "77" });
|
||||
const bigText = "confidential block for tenant 42 ".repeat(40);
|
||||
|
||||
// Storage side (mirrors chatCore: principal = String(apiKeyInfo.id)).
|
||||
const hash = storeBlock(bigText, "42");
|
||||
|
||||
// Retrieval side: resolve the SAME key's headers → "42" → block found.
|
||||
const owner = await resolvePrincipalFromHeaders({ Authorization: "Bearer sk-A" }, lookup);
|
||||
assert.equal(owner, "42");
|
||||
assert.equal(retrieveBlock(hash, owner), bigText, "owner key must retrieve its own block");
|
||||
const ownerResult = handleCcrRetrieve({ hash }, owner);
|
||||
assert.ok("content" in ownerResult, "owner retrieve returns content");
|
||||
|
||||
// A different tenant's key resolves to a different principal → blocked.
|
||||
const other = await resolvePrincipalFromHeaders({ Authorization: "Bearer sk-B" }, lookup);
|
||||
assert.equal(other, "77");
|
||||
assert.equal(retrieveBlock(hash, other), null, "[HIGH IDOR] other tenant must not retrieve the block");
|
||||
const otherResult = handleCcrRetrieve({ hash }, other);
|
||||
assert.ok("error" in otherResult, "[HIGH IDOR] cross-tenant retrieve returns error");
|
||||
});
|
||||
Reference in New Issue
Block a user