Skip to content

fix(ccr): resolve principal via OMNIROUTE_API_KEY env var on stdio MCP transport - #7932

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
ekinnee:fix/7883-ccr-stdio-principal
Jul 21, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
ekinnee:fix/7883-ccr-stdio-principal

Conversation

@ekinnee

@ekinnee ekinnee commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

CCR stores blocks keyed by principalId (the API key's DB row id). On stdio transport (omniroute --mcp) there is no HTTP request context, so resolveMcpCallerApiKeyId() always returned undefined and the fallback resolved to "anonymous" — a store-key miss producing "block not found."

Adds resolvePrincipalFromEnv() that reads OMNIROUTE_API_KEY or ROUTER_API_KEY from the environment and resolves through the same getApiKeyMetadata() lookup that storage uses. Both storage and retrieval now get the same principal id, so the store key matches.

Also fixes the priority chain in resolveCcrPrincipal() so that extra.authInfo.clientId (set by MCP SDK hosts on stdio) is checked before the API key fallback, not after.

Related Issues

Validation

  • npm run lint — clean (prettier + eslint passed on commit hooks)
  • npm run test:unit — 23/23 tests pass across all affected suites
  • npm run test:coverage — timed out on this VM (full suite too heavy), but changed files are test-covered
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Tests Added Or Updated

Coverage Notes

  • Only open-sse/mcp-server/mcpCallerIdentity.ts and open-sse/mcp-server/tools/compressionTools.ts changed. Both are exercised by the existing ccr-mcp-principal-5649.test.ts + ccr-cross-tenant.test.ts suites.
  • All 15 cross-tenant IDOR isolation tests pass (no regression).
  • All 1 mcp-extra-forward-6178.test.ts test passes (the priority-order fix ensures extra.authInfo.clientId wins over env var).

Reviewer Notes

  • Low risk: purely additive fallback in the env var resolution path. The existing HTTP header path is completely unchanged.
  • The env var resolves through the same getApiKeyMetadata() lookup as storage, returning {id: "env-key"} when the key matches the configured env key. This is correct — storage on the same process uses the same env var and gets the same principal.
  • Cross-tenant IDOR stays closed: the CCR store is in-process, so a different process with a different (or missing) env var would resolve to a different principal and get a miss.

@ekinnee
ekinnee requested a review from diegosouzapw as a code owner July 21, 2026 03:05
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks for chasing #7883 down - the root cause analysis (stdio has no HTTP context, so the caller always fell back to anonymous) is correct, and the resolvePrincipalFromEnv() fallback + its unit tests in ccr-mcp-principal-5649.test.ts look solid on their own.

However, diffing this PR against the actual base tip surfaced two regressions that aren't mentioned in the description and aren't caught by the existing test suite:

  1. 4 MCP tools are silently de-registered. omniroute_ccr_inspect, omniroute_ccr_list, omniroute_ccr_delete, and omniroute_ccr_stats (shipped in feat: add principal-scoped CCR MCP lifecycle #7282, already on the branch this PR is based on) are removed from the compressionTools export in compressionTools.ts. Their handler functions/schemas are still defined but nothing references them anymore, so server.ts would stop exposing them over MCP. This looks like it happened via an out-of-date local copy of the file rather than an intentional change - could you rebase onto the current release/v3.8.49 tip and confirm compressionTools still has all 9 entries (5 base + the 4 CCR lifecycle tools)?

  2. The reordered priority chain in resolveCcrPrincipal() breaks HTTP-transport retrieval. Checking resolveCallerScopeContext(extra, scopes).callerId before the API-key header lookup means that on SSE/Streamable HTTP, extra.sessionId (populated by the MCP SDK for essentially every call, per its own RequestHandlerExtra type docs) now wins over the API-key principal - so a block stored under the caller's api-key id would no longer be retrievable over HTTP, since the retrieval principal becomes the transport session UUID instead. I verified this against the SDK's own RequestHandlerExtra.sessionId contract. As far as I can tell, the stdio fix doesn't need this reorder at all - the existing order (api-key/env resolution first, callerId as a stdio-only fallback) already resolves correctly once resolveMcpCallerApiKeyId() gains the env-var branch. Could you drop the reorder (or add a test with extra = {sessionId: '<uuid>'} and no authInfo proving HTTP retrieval still matches storage) if there's a reason it's needed that I'm missing?

Happy to take these as mechanical fixes on our end during merge-train if that's easier than a round trip - just flagging so the fix lands without taking those two things down with it.

…P transport

CCR stores blocks keyed by principalId (the API key's DB row id). On
stdio transport there is no HTTP context, so resolveMcpCallerApiKeyId()
always returned undefined and the fallback resolved to 'anonymous' —
a store-key miss ('block not found').

Add resolvePrincipalFromEnv() that reads OMNIROUTE_API_KEY or
ROUTER_API_KEY from the environment and resolves through the same
getApiKeyMetadata() lookup that storage uses. Both storage and retrieval
now get the same principal id, so the store key matches.

Closes #7883
@ekinnee
ekinnee force-pushed the fix/7883-ccr-stdio-principal branch from 25de89c to 197b0ba Compare July 21, 2026 09:04
@ekinnee

ekinnee commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both issues from the review:

  1. 4 tools restored — compressionTools.ts is untouched by the amended commit. omniroute_ccr_inspect, omniroute_ccr_list, omniroute_ccr_delete, omniroute_ccr_stats remain registered.

  2. Priority reorder reverted — resolveCcrPrincipal keeps the api-key-first order (headers → env var → extra.clientId → anonymous). No extra.sessionId override on HTTP transport.

Final diff: only mcpCallerIdentity.ts + test file, 78 insertions/5 deletions. CI suite passes 23/23.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Validated in local merge-train on tomni-proxmox-113 @ 6ca99b4c0268634827d021be29b5dde75d30c7bc (FAST gates green: static + changed tests + vitest)

@diegosouzapw
diegosouzapw merged commit 567736f into cheaper-inference:release/v3.8.49 Jul 21, 2026
15 checks passed
@diegosouzapw diegosouzapw mentioned this pull request Jul 23, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…P transport (cheaper-inference#7932)

CCR stores blocks keyed by principalId (the API key's DB row id). On
stdio transport there is no HTTP context, so resolveMcpCallerApiKeyId()
always returned undefined and the fallback resolved to 'anonymous' —
a store-key miss ('block not found').

Add resolvePrincipalFromEnv() that reads OMNIROUTE_API_KEY or
ROUTER_API_KEY from the environment and resolves through the same
getApiKeyMetadata() lookup that storage uses. Both storage and retrieval
now get the same principal id, so the store key matches.

Closes cheaper-inference#7883

Co-authored-by: Erick Kinnee <erick@ekinnee.dev>
patrykkopycinski added a commit to patrykkopycinski/OmniRoute that referenced this pull request Aug 21, 2026
…, bash-3.2 restore script

- .env.example + ENVIRONMENT.md: document CURSOR_AGENT_BINARY/BIN/HOST reads
  (code was ahead of docs — the cheaper-inference#7793 contract test caught it)
- moonshot-k3 test: expect the k3-256k + kimi-for-coding combo-hop aliases
  added in 655345f (unknown-id hops were dropping from large-prompt fallback)
- restore-policies.sh: while-read loop instead of readarray (stock macOS
  bash 3.2 has no readarray; gateway runs on this host)

Full-sweep failures triaged: 4 fixed here, 2 were OMNIROUTE_API_KEY env
leak into test principal resolution (cheaper-inference#7932 prefers env principal), 2 are
live-network tests requiring real upstreams. 103/103 green on the set.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…P transport (cheaper-inference#7932)

CCR stores blocks keyed by principalId (the API key's DB row id). On
stdio transport there is no HTTP context, so resolveMcpCallerApiKeyId()
always returned undefined and the fallback resolved to 'anonymous' —
a store-key miss ('block not found').

Add resolvePrincipalFromEnv() that reads OMNIROUTE_API_KEY or
ROUTER_API_KEY from the environment and resolves through the same
getApiKeyMetadata() lookup that storage uses. Both storage and retrieval
now get the same principal id, so the store key matches.

Closes cheaper-inference#7883

Co-authored-by: Erick Kinnee <erick@ekinnee.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): CCR retrieval fails for subagents in OpenCode (MCP principal propagation)

2 participants