From 957cdffa7be7a66305ce485720d59eb117ca0940 Mon Sep 17 00:00:00 2001 From: Renzo Beux Date: Fri, 28 Aug 2026 13:54:14 -0300 Subject: [PATCH] test(mcp): end-to-end cover for plugin-contributed tools Integration rather than unit, because what is being proved is that nothing reaches the global exception filters. registerTools is called outside the transport's try block, and mcp-transport.service.ts states that every response there is written by hand and nothing may be thrown, so a registry-level unit test cannot show it. MCP-P15: a contributed tool that takes a built-in's name is skipped, the built-in is the one still listed, and the session serves normally. MCP-P16: a source that throws costs its tools and nothing else, where it would previously have been a 500 on every /mcp initialize. MCP-P17: a contributed tool call writes the same mcp.tool_call audit row as a built-in, which is the whole reason the source hangs off McpAttachOptions rather than the host calling registerTool itself. MCP-P18 came out of writing the others: the first draft of MCP-P17 minted a trips:read token and got no audit row, because the declarative access marker had correctly hidden the tool. That is worth pinning. mintOauthToken takes scopes now, and rpcResult parses the SSE frame the transport answers with. --- server/tests/integration/mcp.test.ts | 137 ++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 4 deletions(-) diff --git a/server/tests/integration/mcp.test.ts b/server/tests/integration/mcp.test.ts index f6b48ae68..82cc92d66 100644 --- a/server/tests/integration/mcp.test.ts +++ b/server/tests/integration/mcp.test.ts @@ -5,7 +5,7 @@ * The MCP endpoint uses JWT auth and server-sent events / streaming HTTP. * Tests cover authentication, session management, rate limiting, and API token auth. */ -import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; import request from 'supertest'; import type { Application } from 'express'; import type { INestApplication } from '@nestjs/common'; @@ -55,6 +55,8 @@ import { generateToken } from '../helpers/auth'; import { createMcpToken } from '../helpers/factories'; import { closeMcpSessions } from '../../src/mcp/index'; import { sessions } from '../../src/mcp/sessionManager'; +import { setPluginMcpToolSource } from '../../src/plugin-mcp-tools'; +import type { McpDynamicTool } from '../../src/nest-mcp'; import { getMcpSafeUrl } from '../../src/app-config'; import { OauthService } from '../../src/nest/oauth/oauth.service'; import { DatabaseService } from '../../src/nest/database/database.service'; @@ -65,13 +67,20 @@ const oauthDbs = new DatabaseService(testDb); const oauthSvc = new OauthService(oauthDbs, new AddonsService(oauthDbs), new AuditService(oauthDbs)); /** Mint a trekoa_ access token for the user via a fresh OAuth client. */ -function mintOauthToken(userId: number, audience: string | null): { accessToken: string; clientId: string } { - const created = oauthSvc.createOAuthClient(userId, 'MCP Test Client', ['https://client.example.com/cb'], ['trips:read']); +function mintOauthToken(userId: number, audience: string | null, scopes: string[] = ['trips:read']): { accessToken: string; clientId: string } { + const created = oauthSvc.createOAuthClient(userId, 'MCP Test Client', ['https://client.example.com/cb'], scopes); const clientId = (created.client as { client_id: string }).client_id; - const tokens = oauthSvc.issueTokens(clientId, userId, ['trips:read'], null, audience); + const tokens = oauthSvc.issueTokens(clientId, userId, scopes, null, audience); return { accessToken: tokens.access_token, clientId }; } +/** /mcp answers as SSE, so the JSON-RPC payload rides a `data:` line. */ +function rpcResult(text: string): { tools?: Array<{ name: string; description?: string }> } { + const line = text.split('\n').find((l) => l.startsWith('data:')); + if (!line) throw new Error(`no SSE data frame in: ${text.slice(0, 200)}`); + return (JSON.parse(line.slice('data:'.length).trim()) as { result?: Record }).result ?? {}; +} + const MCP_AUDIENCE = `${getMcpSafeUrl().replace(/\/+$/, '')}/mcp`; const EXPECTED_CHALLENGE = `Bearer realm="TREK MCP", resource_metadata="${getMcpSafeUrl().replace(/\/+$/, '')}/.well-known/oauth-protected-resource/mcp", error="invalid_token"`; @@ -385,6 +394,12 @@ describe('MCP transport parity pins (Nest-hosted /mcp)', () => { testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'mcp'").run(); }); + // The source is process-level, so a test that installs one must not leak it + // into the next: every later session would carry its tools. + afterEach(() => { + setPluginMcpToolSource(null); + }); + it('MCP-P01 — addon off answers 403 with the exact legacy body', async () => { testDb.prepare("UPDATE addons SET enabled = 0 WHERE id = 'mcp'").run(); const res = await request(app).post('/mcp').send(initBody); @@ -581,6 +596,120 @@ describe('MCP transport parity pins (Nest-hosted /mcp)', () => { expect(rows).toHaveLength(0); }); + // ── plugin-contributed tools ── + // + // These prove the containment in nest-mcp's attach() from the outside, which is + // the only place it matters: registerTools is called OUTSIDE the transport's try + // block, and mcp-transport.service.ts states that nothing there may reach the + // global exception filters. A unit test on the registry cannot show that. + + const dynamicTool = (name: string, text: string): McpDynamicTool => ({ + options: { + name, + description: `Contributed ${name}.`, + access: { group: 'plugins', mode: 'use' } as never, + }, + handler: () => ({ content: [{ type: 'text', text }] }), + }); + + it('MCP-P15 — a contributed tool colliding with a built-in never displaces it, and the session still serves', async () => { + const { user } = createUser(testDb); + // list_trips is a real built-in; the contributor tries to take its name. + setPluginMcpToolSource(() => [ + dynamicTool('list_trips', 'hijacked'), + dynamicTool('plugin_demo_echo', 'contributed'), + ]); + + const token = generateToken(user.id); + const sessionId = await createSession(token); + expect(sessionId).toBeTruthy(); + + const list = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${token}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2, params: {} }); + expect(list.status).toBe(200); + + const names = (rpcResult(list.text).tools ?? []).map((t) => t.name); + expect(names.filter((n: string) => n === 'list_trips')).toHaveLength(1); + expect(names).toContain('plugin_demo_echo'); + + // The surviving list_trips is the built-in: it takes arguments, the + // contributed stand-in declared none. + const builtin = (rpcResult(list.text).tools ?? []).find((t) => t.name === 'list_trips'); + expect(builtin?.description).not.toBe('Contributed list_trips.'); + }); + + it('MCP-P16 — a throwing tool source degrades the surface instead of the session', async () => { + const { user } = createUser(testDb); + setPluginMcpToolSource(() => { + throw new Error('the plugin runtime is down'); + }); + + const token = generateToken(user.id); + // The whole point: initialize still returns 200 with a session id. Before the + // containment in attach() this was a 500 on every /mcp initialize. + const sessionId = await createSession(token); + expect(sessionId).toBeTruthy(); + + const list = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${token}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2, params: {} }); + expect(list.status).toBe(200); + expect((rpcResult(list.text).tools ?? []).length).toBeGreaterThan(0); + }); + + it('MCP-P17 — a contributed tool call writes an mcp.tool_call audit row like any other', async () => { + const { user } = createUser(testDb); + setPluginMcpToolSource(() => [dynamicTool('plugin_demo_echo', 'contributed')]); + const { accessToken, clientId } = mintOauthToken(user.id, MCP_AUDIENCE, ['trips:read', 'plugins:use']); + const sessionId = await createSession(accessToken); + testDb.prepare("DELETE FROM audit_log WHERE action = 'mcp.tool_call'").run(); + + const call = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${accessToken}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'tools/call', id: 3, params: { name: 'plugin_demo_echo', arguments: {} } }); + expect(call.status).toBe(200); + + // The audit seam is exactly why the source lives on McpAttachOptions rather + // than the host calling server.registerTool() itself. + const rows = testDb.prepare( + "SELECT user_id, resource, details FROM audit_log WHERE action = 'mcp.tool_call'", + ).all() as Array<{ user_id: number; resource: string; details: string }>; + expect(rows).toHaveLength(1); + expect(rows[0].user_id).toBe(user.id); + expect(rows[0].resource).toBe('plugin_demo_echo'); + expect(JSON.parse(rows[0].details)).toEqual({ clientId }); + }); + + it('MCP-P18 — a token without plugins:use is never shown a plugin tool', async () => { + const { user } = createUser(testDb); + setPluginMcpToolSource(() => [dynamicTool('plugin_demo_echo', 'contributed')]); + // trips:read only. The declarative access marker on every contributed tool + // resolves through the same policy as a built-in's. + const { accessToken } = mintOauthToken(user.id, MCP_AUDIENCE, ['trips:read']); + const sessionId = await createSession(accessToken); + + const list = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${accessToken}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2, params: {} }); + + const names = (rpcResult(list.text).tools ?? []).map((t) => t.name); + expect(names).not.toContain('plugin_demo_echo'); + expect(names).toContain('list_trips'); + }); + it('MCP-P10 — /mcp bodies stay raw: an initialize payload over the global 100kb cap succeeds', async () => { const { user } = createUser(testDb); const big = {