fix(cloud): gate MCP proxy invoke on owner-or-public (cross-org IDOR) (#11838)

[NubsCarson]

Closes #11838. [cloud-money] — cross-tenant IDOR found by a Fable-5 IDOR hunt (hand-verified inline; the workflow was Opus-tainted so I traced the route myself).

Bug (MED): GET /api/mcp/proxy/[mcpId] gates on resolveMcpProxyView (owner-or-public), but the POST/invoke handler fetched getById(mcpId) unscoped and checked only existence + status, so org B could invoke org A's private live MCP — hitting A's backend/credentials while B is billed.

Fix: apply the same resolveMcpProxyView gate in POST before billing. Public MCPs stay open (monetization model); private MCPs owner-only.

Proof: non-owner+private → 404, no billing (red without the gate: request proceeds, 500); non-owner+public → 200, billed. 10/0 green, typecheck + biome clean. Security path — do not self-merge.

[greptile-apps]

Your trial has ended. Reactivate Greptile to resume code reviews.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 83e41d80-a423-4b8e-a6cc-5e7a961f2467

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/mcp-proxy-invoke-idor

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[NubsCarson]

[cloud-security] LGTM — verified against PR head 4245c59e. Adversarial pass on the gate (auth-bypass / gate-placement / billing-integrity / over-tightening), all checks hold:

1. Gate placement is right. resolveMcpProxyView runs after auth (401) + existence/status checks and before the affiliate read, before reserveAndDeductCredits (route.ts:208), and before any upstream fetch — a rejected caller produces zero billing and zero contact with the owner's backend, so no refund path is needed for the denied case.
2. Same helper as GET, fail-closed: isOwner requires a truthy viewerOrganizationId (route.ts:70-73), and a null/undefined is_public denies. 404 body mirrors the missing-MCP response, so live private MCPs stay existence-hidden.
3. Monetization preserved (no over-tightening): non-owner + public → 200 and billed exactly once — the new test asserts reserveAndDeductCredits called once.
4. Tests are load-bearing. Ran the suite at head: 10/10 green. Reverted only the route hunk and re-ran: the private-IDOR test goes red (request proceeds → 500 instead of 404), public-invoke test still green. Restored → green.
5. No sibling bypass left in this route family: mcp/registry lists via listPublic only; mcp/stream is a 410 tombstone; v1/mcps/[mcpId] GET already gates owner-or-public (route.ts:89-90) and mutations are org-scoped; mcp/info + mcp/list are static platform definitions. The proxy POST was the one ungated invoke path and this closes it.

Non-blocking nit (pre-existing, NOT introduced here): in both GET and POST the status !== "live" check ("MCP is not available") runs before the ownership gate, so a non-owner probing a private MCP id can distinguish exists-but-not-live from missing/live-private. LOW (UUIDs make enumeration impractical, both branches are 404s) — if anyone touches this file again, swapping the gate ahead of the status check erases the oracle in one move.

No changes requested. Money+security path — ready for maintainer merge, do not self-merge.

[NubsCarson]

[cloud-audit] LGTM — the cross-org invoke IDOR (#11838) is genuinely closed.

Verified on head 4245c59:

1. Gate applied before any action. The check sits at packages/cloud/api/mcp/proxy/[mcpId]/route.ts:169-176, immediately after the existence/status checks and BEFORE the affiliate lookup (:182), the upfront debit reserveAndDeductCredits (:208), endpoint resolution (:277-317), and the upstream fetch (:349/:356). A blocked caller triggers no billing and no outbound request — matching the new test's reserveAndDeductCredits / safeFetch not-called assertions (mcp-proxy-refund.test.ts:109-121).

2. Correct check, matching the read path. POST reuses the identical pure resolveMcpProxyView (route.ts:65-74, allowed = isOwner || is_public, owner = same organization_id) that GET already enforces at :109-116. Same predicate, same 404 body ("MCP not found") as a missing id, so the gate itself is not an oracle.

3. No bypass via other verbs/paths. This route exposes only GET (gated, :90), POST (now gated), and OPTIONS (204 no-op, :433). Swept every other cloud API route at head for unscoped user-MCP access: /api/v1/mcps/[mcpId] GET gates owner-or-public (v1 route.ts:89-90) and PUT/DELETE pass authResult.user.organization_id into the service (:153, :176); /api/mcp/{stream,info,list} and /api/mcps/* never touch userMcpsService/mcpId (platform first-party MCPs). The proxy POST was the only invoke path, and it is now gated.

4. Fail-closed. POST hard-requires auth (:148-152, 401), and resolveMcpProxyView treats a null/undefined/mismatched viewerOrganizationId as non-owner (:70-72), so a missing org can never grant access — only is_public=true opens the resource, which is the intended monetization model.

LOW nit (non-blocking, pre-existing, identical in GET): the status !== "live" check at :160 runs before the gate and returns a different 404 body ("MCP is not available" vs "MCP not found"), letting a non-owner distinguish a paused private MCP from a nonexistent id. Fine to leave or fold the gate ahead of the status check in a follow-up.

[cloud-audit]

[NubsCarson]

[cloud-security] LGTM — adversarially verified (Fable-5 + independent test run in a throwaway worktree at the PR head). POST/invoke now applies the exact same resolveMcpProxyView owner-or-public gate as GET, BEFORE the affiliate lookup + reserveAndDeductCredits — so a non-owner hitting a private MCP is blocked with zero billing/upstream side effects, same 404 as GET. Whole bug class closed: the sub-app is only GET(gated)/POST(now gated)/OPTIONS(204), no user-MCP SSE/stream path exists, v1/mcps/[mcpId] GET already enforces the identical check, discovery uses listPublic. Reverting just the route file makes the new private-MCP test go red (404 + no-billing + no-fetch) and the public-MCP path stays billed-once/200 (no regression). Nit (pre-existing, follow-up): both handlers check status !== 'live' before the ownership gate → a private-but-not-live MCP leaks existence/status via the 404 body string ('not available' vs 'not found'); swap the two checks in a follow-up. Good catch on the invoke path — this was the gap my earlier proxy sweep missed (I checked GET/view, not POST/invoke).

[lalalune]

Reviewed against #11838 and the existing GET access policy. The POST path now uses the same owner-or-public gate before credit reservation, so private cross-org invoke returns 404 without billing or upstream access while public MCP monetization remains intact. The added tests cover both sides of that boundary.

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.