diff --git a/CHANGELOG.md b/CHANGELOG.md index 771ecbda96..03123eb488 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/open-sse/mcp-server/mcpCallerIdentity.ts b/open-sse/mcp-server/mcpCallerIdentity.ts new file mode 100644 index 0000000000..7d24e370b9 --- /dev/null +++ b/open-sse/mcp-server/mcpCallerIdentity.ts @@ -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, + lookup: ApiKeyLookup = getApiKeyMetadata +): Promise { + // 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 { + return resolvePrincipalFromHeaders(getMcpHttpAuthHeadersForInternalFetch()); +} diff --git a/open-sse/mcp-server/tools/compressionTools.ts b/open-sse/mcp-server/tools/compressionTools.ts index 66c67499ba..4d1df0f459 100644 --- a/open-sse/mcp-server/tools/compressionTools.ts +++ b/open-sse/mcp-server/tools/compressionTools.ts @@ -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, 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, 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); }, diff --git a/tests/unit/compression/ccr-mcp-principal-5649.test.ts b/tests/unit/compression/ccr-mcp-principal-5649.test.ts new file mode 100644 index 0000000000..fc84b5949a --- /dev/null +++ b/tests/unit/compression/ccr-mcp-principal-5649.test.ts @@ -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) => 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"); +});