diff --git a/docs/sso.md b/docs/sso.md index 540ec2e8..b5d1491b 100644 --- a/docs/sso.md +++ b/docs/sso.md @@ -205,17 +205,17 @@ on** — or turn enforcement off first. Role sync decides which **MCP roles** a user holds; those roles decide which tools they may use. Where that restriction is applied depends on the endpoint: +Both endpoints filter `tools/list` to the tools the caller's roles allow, and +refuse `tools/call` for anything else: + | Endpoint | `tools/list` | `tools/call` | |---|---|---| | `/mcp/` (per server) | Filtered to the user's tools | Denied if not allowed | -| `/mcp` (global) | **Lists every tool of the workspace** | Denied if not allowed | - -A restricted user can never *invoke* a tool their roles do not grant on either -endpoint. But on the global `/mcp` endpoint they still *see* the whole -inventory, and an AI client will plan with tools it cannot use. +| `/mcp` (global) | Filtered to the user's tools, within their organization | Denied if not allowed | -**Prefer a per-server endpoint** (`Settings → MCP Servers`) when you rely on -role-based restriction. +A tool that has **no** role assigned to it at all is visible only to users whose +access is unrestricted — an admin, or someone holding no MCP role. Once a user +holds any MCP role, they see exactly what their roles grant. --- diff --git a/packages/backend/src/mcp-server/global-tool-visibility.spec.ts b/packages/backend/src/mcp-server/global-tool-visibility.spec.ts new file mode 100644 index 00000000..9509b9e2 --- /dev/null +++ b/packages/backend/src/mcp-server/global-tool-visibility.spec.ts @@ -0,0 +1,47 @@ +import { toolVisibilityRole } from './mcp-server.service'; + +/** + * The global `/mcp` registry holds one entry per tool name for the WHOLE + * deployment, and the transport's `tools/list` handler is synchronous — it + * cannot ask the database who is calling. It filters on `user.roles` against + * each tool's `requiredRoles`, so the controller resolves visibility + * asynchronously first and plants the answer on the request. + * + * These tests pin that contract. If `attachVisibleTools` stops running, or the + * registration stops declaring a visibility role, one tenant's tool names, + * descriptions and input schemas become readable by every other tenant. + */ +describe('global /mcp tool visibility', () => { + // Mirrors @rekog/mcp-nest's ToolAuthorizationService.canAccessTool for the + // 'any' match mode, which is what the transport applies to tools/list. + const canSee = (userRoles: string[] | undefined, toolName: string) => { + const required = [toolVisibilityRole(toolName)]; + if (!userRoles) return false; + return required.some((r) => userRoles.includes(r)); + }; + + it('namespaces the synthetic role so it cannot collide with a real one', () => { + expect(toolVisibilityRole('reports')).toBe('tool:reports'); + expect(toolVisibilityRole('ADMIN')).not.toBe('ADMIN'); + }); + + it('hides a tool whose visibility role the caller does not hold', () => { + const roles = [toolVisibilityRole('mine')]; + expect(canSee(roles, 'mine')).toBe(true); + expect(canSee(roles, 'othertenant_confidential_report')).toBe(false); + }); + + it('shows nothing to a caller with no visibility roles at all', () => { + expect(canSee([], 'anything')).toBe(false); + }); + + // Two tenants can name a tool the same way; the registry keeps one entry, so + // gating on the NAME is what lets both tenants see their own copy. Gating on + // the tool id would show it only to whichever tenant registered first. + it('is keyed on the tool name, not a per-tenant id', () => { + const orgA = [toolVisibilityRole('shared_name')]; + const orgB = [toolVisibilityRole('shared_name')]; + expect(canSee(orgA, 'shared_name')).toBe(true); + expect(canSee(orgB, 'shared_name')).toBe(true); + }); +}); diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index 31aea657..553e1c42 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -21,6 +21,7 @@ import { } from '@modelcontextprotocol/node'; import { McpCombinedAuthGuard } from '../auth/mcp-combined-auth.guard'; import { mcpHttpTransport } from './mcp-strategy'; +import { toolVisibilityRole } from './mcp-server.service'; import { McpServersService } from '../mcp-servers/mcp-servers.service'; import { McpSessionManager } from '../mcp-servers/mcp-session.manager'; import { ToolRegistry, RegisteredTool } from './tool-registry'; @@ -124,11 +125,14 @@ export class McpEndpointController { @Post() async handleGlobalPost(@Req() req: Request, @Res() res: Response) { + const visible = await this.attachVisibleTools(req); + if (this.refuseHiddenToolCall(req, res, visible)) return; await mcpHttpTransport.httpHandlers.handlePost(req, res); } @Get() async handleGlobalGet(@Req() req: Request, @Res() res: Response) { + await this.attachVisibleTools(req); await mcpHttpTransport.httpHandlers.handleGet(req, res); } @@ -137,6 +141,105 @@ export class McpEndpointController { await mcpHttpTransport.httpHandlers.handleDelete(req, res); } + /** + * Resolves which tools the caller may SEE and records them on the request. + * + * The global registry is shared by every organization — it holds one entry + * per tool name across the whole deployment — and the transport's + * `tools/list` handler is synchronous, so it can neither query the database + * nor know which tenant is asking. Left alone it therefore returns EVERY + * registered tool to any authenticated caller, which leaks one tenant's tool + * names, descriptions and input schemas to every other tenant, and hands a + * role-restricted user the full inventory of their own workspace. + * + * Calls were never affected: `tools/call` resolves the tool by name AND + * organization and refuses a mismatch. This closes the listing side. + * + * Two scopes are applied, in this order: + * 1. ORGANIZATION — only tools owned by the caller's active org. + * 2. ROLE — of those, only the ones the caller's MCP roles allow. + */ + private async attachVisibleTools(req: Request): Promise | null> { + const user = (req as any).user; + + // No identified principal: a static MCP_API_KEY / MCP_BEARER_TOKEN or an + // explicitly enabled anonymous mode. Both are operator credentials on a + // single-tenant self-hosted box, so the pre-existing "everything" answer + // is the correct one and narrowing it here would break those deployments. + if (!user?.sub || !user.organizationId) return null; + + const orgTools = this.toolRegistry + .getAllTools() + .filter((t) => t.organizationId === user.organizationId); + + const allowedToolIds = await this.rolesService.getAllowedToolIds( + user.sub, + user.organizationId, + ); + + // `null` means unrestricted — an ADMIN, or a user holding no MCP role at + // all. The organization scope still applies. + const visible = + allowedToolIds === null + ? orgTools + : orgTools.filter((t) => allowedToolIds.includes(t.id)); + + const visibleNames = new Set(visible.map((t) => t.name)); + user.roles = [ + ...[...visibleNames].map(toolVisibilityRole), + // Tools declared statically in code (the Knowledge Graph helper, the + // demo tools) carry no visibility role and stay visible to everyone — + // they hold no tenant data. + ...(Array.isArray(user.roles) ? user.roles : []), + ]; + return visibleNames; + } + + /** + * Answers a call for a tool the caller cannot see, before the transport does. + * + * The transport would refuse it anyway — the same visibility role gates + * `tools/call` — but its message names the synthetic role + * (`requires any of roles: tool:foo`), which exposes an implementation + * detail and tells an operator nothing about what to do next. + * + * The wording is deliberately ambiguous between "another workspace's tool" + * and "your role does not grant it": distinguishing them would confirm to a + * caller that a tool of that name exists somewhere else in the deployment, + * which is the disclosure this whole path exists to prevent. + */ + private refuseHiddenToolCall( + req: Request, + res: Response, + visible: Set | null, + ): boolean { + // `null` means visibility was not narrowed for this caller — an operator + // credential on a single-tenant box. Nothing to refuse. + if (visible === null) return false; + + const body = (req as any).body; + // Only single calls. A batch is handled by the transport, which still + // refuses each hidden tool — with the less friendly message. + if (!body || Array.isArray(body) || body.method !== 'tools/call') return false; + + const name = body?.params?.name; + if (typeof name !== 'string' || visible.has(name)) return false; + + // Static, code-declared tools carry no visibility role and are open to + // everyone, so they must not be refused here. + if (this.toolRegistry.countByName(name) === 0) return false; + + res.status(200).json({ + jsonrpc: '2.0', + id: body.id ?? null, + error: { + code: -32600, + message: `Tool '${name}' is not available to you. It belongs to another workspace, or your MCP role does not grant it.`, + }, + }); + return true; + } + // ─── Public, anonymous, static demo MCP server ────────────────────────── // // A self-describing MCP endpoint at the EXACT path /mcp/demo. It exposes only diff --git a/packages/backend/src/mcp-server/mcp-server.service.ts b/packages/backend/src/mcp-server/mcp-server.service.ts index 6e1ada66..54509a60 100644 --- a/packages/backend/src/mcp-server/mcp-server.service.ts +++ b/packages/backend/src/mcp-server/mcp-server.service.ts @@ -17,6 +17,14 @@ import { deriveToolAnnotations, } from './tool-annotations'; +/** + * The synthetic role that makes one tool visible in the GLOBAL `/mcp` + * `tools/list`. Prefixed so it can never collide with a real role name. + */ +export function toolVisibilityRole(toolName: string): string { + return `tool:${toolName}`; +} + @Injectable() export class McpServerService implements OnModuleInit { private readonly logger = new Logger(McpServerService.name); @@ -232,9 +240,28 @@ export class McpServerService implements OnModuleInit { name, description, parameters: zodParams, + // Gate this entry on a synthetic "role" naming the tool itself, matched + // with 'any'. The transport's `tools/list` handler is SYNCHRONOUS, so it + // cannot ask the database who the caller is — but it does compare + // `user.roles` against this list. The global endpoint therefore resolves + // the caller's visible tools asynchronously BEFORE delegating and plants + // the answer on `req.user.roles`. See `visibleToolRoles` in + // mcp-endpoint.controller.ts. + // + // Keyed on NAME, not tool id: this registry holds one entry per name + // across every organization, so the id belongs to whichever tenant + // registered it first. Name is also what the call handler resolves by + // (`getToolForOrg(name, org)`), so the two stay consistent. + requiredRoles: [toolVisibilityRole(name)], + requiredRolesMatch: 'any', ...(annotations ? { annotations } : {}), handler: async (args: Record, _context: any, request: any) => { - // Check role-based tool access if user is identified + // Role check, kept as the SECOND layer. The transport now refuses a + // disallowed call before the handler runs, because `requiredRoles` + // above gates `tools/call` as well as `tools/list`. This stays so that + // a tool registered without a visibility role — a future code path, a + // merge that drops the option — is still not freely callable. Defence + // in depth, not the only gate. const user = request?.user; if (user?.sub) { // Global /mcp registry: there is no server-scoped org here, so the