From cbe5f23b683792752149f53c16e60e4872dda17f Mon Sep 17 00:00:00 2001 From: viktormarinho Date: Wed, 3 Jun 2026 14:07:07 -0300 Subject: [PATCH] refactor(api): remove dead legacy unscoped MCP route mounts Remove the legacy /mcp/:connectionId, /mcp/self, /mcp/gateway and /mcp/virtual-mcp mounts plus their now-dead mcpAuth registrations and orphaned imports. All MCP traffic goes through the org-scoped /api/:org/mcp/* routes in createOrgScopedApi; inbound traffic to the legacy paths has drained to ~0. Migrate the access-control, proxy and oauth-proxy integration suites to the scoped paths (seeding org membership so resolveOrgFromPath admits the principal), since they exercised RBAC / cross-org behavior through the now-removed unscoped surface. DO NOT MERGE until the legacy-URL emitters (@deco/runtime bindings, @deco/mesh-sdk constants + mcp-oauth fallback, decopilot dispatch, typegen) are migrated to scoped paths and older published-package versions drain. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../api/access-control.integration.test.ts | 23 ++++++++++++-- apps/mesh/src/api/app.ts | 30 ++----------------- .../routes/oauth-proxy.integration.test.ts | 18 ++++++++++- .../src/api/routes/proxy.integration.test.ts | 10 +++++-- 4 files changed, 48 insertions(+), 33 deletions(-) diff --git a/apps/mesh/src/api/access-control.integration.test.ts b/apps/mesh/src/api/access-control.integration.test.ts index ea9e741d68..0273499a50 100644 --- a/apps/mesh/src/api/access-control.integration.test.ts +++ b/apps/mesh/src/api/access-control.integration.test.ts @@ -75,6 +75,7 @@ interface TestApiKey { key: string; userId: string; permissions: Permission; + orgSlug?: string; } interface BetterAuthApiKeyResult { @@ -270,6 +271,9 @@ describe("Access Control Integration Tests", () => { key, userId, permissions, + orgSlug: organizationId + ? testOrganizations.get(organizationId)?.slug + : undefined, }; // Store for verification @@ -307,6 +311,19 @@ describe("Access Control Integration Tests", () => { } as BetterAuthApiKeyResult; }) as never); + // Org-scoped MCP routes run resolveOrgFromPath, which requires the + // authenticated principal to be a member of the org. Seed membership so + // API-key requests reach the AccessControl layer (idempotent per user+org). + if (organizationId) { + const { sql } = await import("kysely"); + const role = testUsers.get(userId)?.role === "admin" ? "admin" : "member"; + await sql` + INSERT INTO "member" (id, "organizationId", "userId", role, "createdAt") + VALUES (${`mem_${userId}_${organizationId}`}, ${organizationId}, ${userId}, ${role}, ${new Date().toISOString()}) + ON CONFLICT (id) DO NOTHING + `.execute(database.db); + } + return apiKey; } @@ -328,7 +345,8 @@ describe("Access Control Integration Tests", () => { id: 1, }; - return await app.request("/mcp", { + const orgSlug = testApiKeys.get(apiKey)?.orgSlug; + return await app.request(`/api/${orgSlug}/mcp/self`, { method: "POST", headers: { "Content-Type": "application/json", @@ -358,7 +376,8 @@ describe("Access Control Integration Tests", () => { id: 1, }; - return await app.request(`/mcp/${connectionId}`, { + const orgSlug = testApiKeys.get(apiKey)?.orgSlug; + return await app.request(`/api/${orgSlug}/mcp/${connectionId}`, { method: "POST", headers: { "Content-Type": "application/json", diff --git a/apps/mesh/src/api/app.ts b/apps/mesh/src/api/app.ts index 8bf276d63a..10fe87a074 100644 --- a/apps/mesh/src/api/app.ts +++ b/apps/mesh/src/api/app.ts @@ -53,7 +53,6 @@ import { createDecoSitesOrgRoutes, createDecoSitesUserRoutes, } from "./routes/deco-sites"; -import { createVirtualMcpRoutes } from "./routes/virtual-mcp"; import { createLegacyWellKnownProtectedResourceRoutes, createWellKnownAuthServerRoutes, @@ -62,13 +61,11 @@ import { protectedResourceMetadataHandler, } from "./routes/oauth-proxy"; import openaiCompatRoutes from "./routes/openai-compat"; -import { createProxyRoutes } from "./routes/proxy"; import { createKVRoutes } from "./routes/kv"; import { createTriggerCallbackRoutes } from "./routes/trigger-callback"; import publicConfigRoutes from "./routes/public-config"; import filesRoutes from "./routes/files"; import { createThreadOutputsRoutes } from "./routes/thread-outputs"; -import { createSelfRoutes } from "./routes/self"; import { shouldSkipMeshContext, SYSTEM_PATHS } from "./utils/paths"; import { mountPluginRoutes, @@ -1689,11 +1686,6 @@ export async function createApp(options: CreateAppOptions = {}) { } return await next(); }; - app.use("/mcp/:connectionId?", mcpAuth); - app.use("/mcp/gateway/:virtualMcpId?", mcpAuth); - app.use("/mcp/virtual-mcp/:virtualMcpId?", mcpAuth); - app.use("/mcp/self", mcpAuth); - // Local file storage MCP routes — mounted whenever DevObjectStorage is the // active object-storage backend (i.e. no S3 configured). Required so the // dev-assets pseudo-connection can satisfy the OBJECT_STORAGE binding. @@ -1705,25 +1697,9 @@ export async function createApp(options: CreateAppOptions = {}) { mountDevRoutes(app, mcpAuth); } - // Virtual MCP / Agent routes (must be before proxy to match /mcp/gateway and /mcp/virtual-mcp before /mcp/:connectionId) - // /mcp/gateway/:virtualMcpId (backward compat) or /mcp/virtual-mcp/:virtualMcpId - const legacyVirtualMcp = new Hono(); - legacyVirtualMcp.use("*", createLogDeprecatedRoute({ mountPath: "/mcp" })); - legacyVirtualMcp.route("/", createVirtualMcpRoutes()); - app.route("/mcp", legacyVirtualMcp); - - // Self MCP routes (at /mcp/self) - exposes all management tools - const legacySelf = new Hono(); - legacySelf.use("*", createLogDeprecatedRoute({ mountPath: "/mcp/self" })); - legacySelf.route("/", createSelfRoutes()); - app.route("/mcp/self", legacySelf); - - // MCP Proxy routes (connection-specific) - // Note: SELF MCP ({org}_self) is handled by proxy.ts with special case detection - const legacyProxy = new Hono(); - legacyProxy.use("*", createLogDeprecatedRoute({ mountPath: "/mcp" })); - legacyProxy.route("/", createProxyRoutes()); - app.route("/mcp", legacyProxy); + // Legacy unscoped MCP mounts (/mcp/self, /mcp/gateway, /mcp/virtual-mcp, + // /mcp/:connectionId) were removed — all MCP traffic uses the org-scoped + // /api/:org/mcp/* routes (see createOrgScopedApi in routes/org-scoped.ts). // Measure LLM models route latency app.use("/api/:org/models/*", async (c, next) => { diff --git a/apps/mesh/src/api/routes/oauth-proxy.integration.test.ts b/apps/mesh/src/api/routes/oauth-proxy.integration.test.ts index b43516a2d3..08abca6910 100644 --- a/apps/mesh/src/api/routes/oauth-proxy.integration.test.ts +++ b/apps/mesh/src/api/routes/oauth-proxy.integration.test.ts @@ -144,6 +144,22 @@ describe("MCP OAuth Proxy E2E", () => { }, } as never); + // The non-OAuth 401 test hits /api/:org/mcp/:connectionId, which runs + // resolveOrgFromPath — seed the mock principal's membership in org_test so + // it isn't rejected as a non-member before reaching the proxy. + const { sql } = await import("kysely"); + const nowIso = new Date().toISOString(); + await sql` + INSERT INTO "user" (id, email, "emailVerified", name, "createdAt", "updatedAt") + VALUES ('test-user-id', 'test-user-id@test.com', false, 'Test User', ${nowIso}, ${nowIso}) + ON CONFLICT (id) DO NOTHING + `.execute(database.db); + await sql` + INSERT INTO "member" (id, "organizationId", "userId", role, "createdAt") + VALUES ('mem_test_user_org_test', ${orgId}, 'test-user-id', 'member', ${nowIso}) + ON CONFLICT (id) DO NOTHING + `.execute(database.db); + // Create a connection for each MCP server (OAuth-supporting) for (const server of MCP_SERVERS) { const connectionId = `conn_${server.name.toLowerCase().replace(/[^a-z0-9]/g, "_")}`; @@ -334,7 +350,7 @@ describe("MCP OAuth Proxy E2E", () => { const connectionId = connectionMap.get(server.url)!; // Try to access the MCP endpoint with auth - should get 401 from origin without WWW-Authenticate - const res = await app.request(`/mcp/${connectionId}`, { + const res = await app.request(`/api/org_test/mcp/${connectionId}`, { method: "POST", headers: { "Content-Type": "application/json", diff --git a/apps/mesh/src/api/routes/proxy.integration.test.ts b/apps/mesh/src/api/routes/proxy.integration.test.ts index 5c32cb36f4..259870f11c 100644 --- a/apps/mesh/src/api/routes/proxy.integration.test.ts +++ b/apps/mesh/src/api/routes/proxy.integration.test.ts @@ -128,8 +128,12 @@ describe("MCP Proxy null-org bypass", () => { vi.restoreAllMocks(); }); - it("should reject proxy access when organization context is missing", async () => { - const response = await app.request("/mcp/conn_victim_123", { + it("should reject cross-org proxy access for a non-member", async () => { + // The legacy unscoped /mcp/:connectionId route was removed; MCP traffic now + // goes through /api/:org/mcp/:connectionId. The attacker targets the victim + // org's path directly, and resolveOrgFromPath rejects them as a non-member + // before the proxy handler ever runs. + const response = await app.request("/api/victim-org/mcp/conn_victim_123", { method: "POST", headers: { "Content-Type": "application/json", @@ -149,6 +153,6 @@ describe("MCP Proxy null-org bypass", () => { expect(response.status).toBe(403); const body = await response.json(); - expect(body.error).toContain("Organization context is required"); + expect(body.error).toContain("not a member of organization"); }); });