diff --git a/nest-mcp/src/index.ts b/nest-mcp/src/index.ts index 678d609c3..916eac8a7 100644 --- a/nest-mcp/src/index.ts +++ b/nest-mcp/src/index.ts @@ -25,6 +25,7 @@ export { type McpAccessPolicy, type McpAccessPredicate, type McpAccessValidator, + type McpAttachOptions, type McpContext, type McpDeclarativeAccess, type McpEntry, diff --git a/nest-mcp/src/registry.ts b/nest-mcp/src/registry.ts index ba8d09271..eb789d12d 100644 --- a/nest-mcp/src/registry.ts +++ b/nest-mcp/src/registry.ts @@ -2,6 +2,7 @@ import { getEntry, isMcpController, type ClassRef } from './metadata'; import type { McpAccessPolicy, McpAccessValidator, + McpAttachOptions, McpContext, McpEntry, McpRegistryListing, @@ -98,25 +99,26 @@ export class McpRegistry { /** * Registers every entry passing its access check onto `server`, binding - * `ctx` as the handler's last argument (in the SDK `extra` slot). + * `ctx` as the handler's last argument (in the SDK `extra` slot). `opts` + * carries per-session hooks — see `McpAttachOptions`. */ - attach(server: McpServer, ctx: McpContext): void { + attach(server: McpServer, ctx: McpContext, opts?: McpAttachOptions): void { const registrar = server as unknown as LooseRegistrar; for (const { entry, instance } of this.bound) { if (!this.allowed(entry, ctx, instance)) continue; const handler = (instance as unknown as Record)[entry.methodName]; switch (entry.kind) { case 'tool': - this.attachTool(registrar, entry.options, instance, handler, ctx); + this.attachTool(registrar, entry.options, instance, handler, ctx, opts); break; case 'resource': - this.attachResource(registrar, entry.options, instance, handler, ctx); + this.attachResource(registrar, entry.options, instance, handler, ctx, opts); break; case 'resourceTemplate': - this.attachResourceTemplate(registrar, entry.options, instance, handler, ctx); + this.attachResourceTemplate(registrar, entry.options, instance, handler, ctx, opts); break; case 'prompt': - this.attachPrompt(registrar, entry.options, instance, handler, ctx); + this.attachPrompt(registrar, entry.options, instance, handler, ctx, opts); break; } } @@ -206,6 +208,7 @@ export class McpRegistry { instance: object, handler: AnyHandler, ctx: McpContext, + opts?: McpAttachOptions, ): void { const config: Record = { title: options.title, @@ -220,8 +223,14 @@ export class McpRegistry { // the decorated method always receives (args, ctx). const cb = options.inputSchema !== undefined - ? (args: unknown, _extra: unknown) => handler.call(instance, args, ctx) - : (_extra: unknown) => handler.call(instance, {}, ctx); + ? (args: unknown, _extra: unknown) => { + opts?.onInvoke?.({ kind: 'tool', name: options.name }); + return handler.call(instance, args, ctx); + } + : (_extra: unknown) => { + opts?.onInvoke?.({ kind: 'tool', name: options.name }); + return handler.call(instance, {}, ctx); + }; registrar.registerTool(options.name, config, cb); } @@ -231,6 +240,7 @@ export class McpRegistry { instance: object, handler: AnyHandler, ctx: McpContext, + opts?: McpAttachOptions, ): void { const metadata: Record = { title: options.title, @@ -238,9 +248,10 @@ export class McpRegistry { mimeType: options.mimeType, _meta: options._meta, }; - registrar.registerResource(options.name, options.uri, metadata, (uri: unknown, _extra: unknown) => - handler.call(instance, uri, ctx), - ); + registrar.registerResource(options.name, options.uri, metadata, (uri: unknown, _extra: unknown) => { + opts?.onInvoke?.({ kind: 'resource', name: options.name }); + return handler.call(instance, uri, ctx); + }); } private attachResourceTemplate( @@ -249,6 +260,7 @@ export class McpRegistry { instance: object, handler: AnyHandler, ctx: McpContext, + opts?: McpAttachOptions, ): void { const metadata: Record = { title: options.title, @@ -260,7 +272,10 @@ export class McpRegistry { options.name, new SdkResourceTemplate(options.uriTemplate, { list: undefined }), metadata, - (uri: unknown, variables: unknown, _extra: unknown) => handler.call(instance, uri, variables, ctx), + (uri: unknown, variables: unknown, _extra: unknown) => { + opts?.onInvoke?.({ kind: 'resourceTemplate', name: options.name }); + return handler.call(instance, uri, variables, ctx); + }, ); } @@ -270,6 +285,7 @@ export class McpRegistry { instance: object, handler: AnyHandler, ctx: McpContext, + opts?: McpAttachOptions, ): void { const config: Record = { title: options.title, @@ -278,8 +294,14 @@ export class McpRegistry { }; const cb = options.argsSchema !== undefined - ? (args: unknown, _extra: unknown) => handler.call(instance, args, ctx) - : (_extra: unknown) => handler.call(instance, {}, ctx); + ? (args: unknown, _extra: unknown) => { + opts?.onInvoke?.({ kind: 'prompt', name: options.name }); + return handler.call(instance, args, ctx); + } + : (_extra: unknown) => { + opts?.onInvoke?.({ kind: 'prompt', name: options.name }); + return handler.call(instance, {}, ctx); + }; registrar.registerPrompt(options.name, config, cb); } } diff --git a/nest-mcp/src/types.ts b/nest-mcp/src/types.ts index 7ebd2fb6e..bf16e05ff 100644 --- a/nest-mcp/src/types.ts +++ b/nest-mcp/src/types.ts @@ -133,6 +133,19 @@ export interface PromptOptions extends McpEntryOptionsBase { export type McpEntryKind = 'tool' | 'resource' | 'resourceTemplate' | 'prompt'; +/** Per-session options for `McpRegistry.attach()`. */ +export interface McpAttachOptions { + /** + * Called immediately before every attached handler runs, with the entry's + * kind and registered name. This is the host's observability seam (e.g. a + * tool-call audit trail) — nest-mcp itself attaches no semantics to it. + * Contract: the hook MUST NOT throw; nest-mcp does not catch, so a throwing + * hook fails the very invocation it is observing. Hosts wrap their own + * failure handling. + */ + onInvoke?: (info: { kind: McpEntryKind; name: string }) => void; +} + export type McpEntry = | { kind: 'tool'; methodName: string; options: ToolOptions } | { kind: 'resource'; methodName: string; options: ResourceOptions } diff --git a/nest-mcp/tests/attach.test.ts b/nest-mcp/tests/attach.test.ts index 553e73d0c..74d966030 100644 --- a/nest-mcp/tests/attach.test.ts +++ b/nest-mcp/tests/attach.test.ts @@ -250,3 +250,50 @@ describe('McpRegistry.attach', () => { ).toThrow(/declares declarative access but no accessPolicy was configured/); }); }); + +describe('McpAttachOptions.onInvoke', () => { + let harness: AttachHarness | undefined; + afterEach(async () => { + await harness?.cleanup(); + harness = undefined; + }); + + const fullCtx: TestCtx = { userId: 7, canRead: true, canWrite: true, allow: true }; + + it('fires once per invocation with the entry kind and name — attach and listing stay silent', async () => { + const seen: Array<{ kind: string; name: string }> = []; + harness = await createAttachHarness(buildRegistry(), fullCtx, { onInvoke: (info) => seen.push(info) }); + // Neither attach() nor the initialize handshake is an invocation. + expect(seen).toEqual([]); + + await harness.client.callTool({ name: 'open_tool', arguments: {} }); + expect(seen).toEqual([{ kind: 'tool', name: 'open_tool' }]); + + // The inputSchema'd tool goes through the other normalization branch. + await harness.client.callTool({ name: 'read_tool', arguments: { name: 'x' } }); + expect(seen[1]).toEqual({ kind: 'tool', name: 'read_tool' }); + + await harness.client.readResource({ uri: 'test://doc' }); + expect(seen[2]).toEqual({ kind: 'resource', name: 'fixture_doc' }); + + await harness.client.readResource({ uri: 'test://item/5' }); + expect(seen[3]).toEqual({ kind: 'resourceTemplate', name: 'fixture_item' }); + + await harness.client.getPrompt({ name: 'fixture_prompt', arguments: { topic: 't' } }); + expect(seen[4]).toEqual({ kind: 'prompt', name: 'fixture_prompt' }); + + // Listing is not an invocation. + await harness.client.listTools(); + expect(seen).toHaveLength(5); + }); + + it('runs before the handler, so a throwing hook fails the invocation it observes', async () => { + harness = await createAttachHarness(buildRegistry(), fullCtx, { + onInvoke: () => { + throw new Error('audit exploded'); + }, + }); + const result = await harness.client.callTool({ name: 'open_tool', arguments: {} }); + expect((result as { isError?: boolean }).isError).toBe(true); + }); +}); diff --git a/nest-mcp/tests/harness.ts b/nest-mcp/tests/harness.ts index a66b3ccbd..68aee898e 100644 --- a/nest-mcp/tests/harness.ts +++ b/nest-mcp/tests/harness.ts @@ -1,4 +1,4 @@ -import type { McpContext, McpRegistry } from '../src'; +import type { McpAttachOptions, McpContext, McpRegistry } from '../src'; import { Client } from '@modelcontextprotocol/sdk/client/index'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; @@ -20,9 +20,13 @@ export interface AttachHarness { } /** Bare McpServer + Client over an in-memory transport with the registry attached. */ -export async function createAttachHarness(registry: McpRegistry, ctx: TestCtx): Promise { +export async function createAttachHarness( + registry: McpRegistry, + ctx: TestCtx, + opts?: McpAttachOptions, +): Promise { const server = new McpServer({ name: 'nest-mcp-test', version: '1.0.0' }); - registry.attach(server, asCtx(ctx)); + registry.attach(server, asCtx(ctx), opts); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); const client = new Client({ name: 'test-client', version: '1.0.0' }); diff --git a/server/src/mcp/sessionManager.ts b/server/src/mcp/sessionManager.ts index cad993b5e..f71b15f39 100644 --- a/server/src/mcp/sessionManager.ts +++ b/server/src/mcp/sessionManager.ts @@ -12,6 +12,8 @@ export interface McpSession { /** true when authenticated via static trek_ token — triggers deprecation prompt */ isStaticToken: boolean; lastActivity: number; + /** IP of the most recent authorized request — what mcp.tool_call audit rows record */ + lastClientIp: string | null; } export const sessions = new Map(); diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index f0f3cb832..e2d05f453 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -1,7 +1,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import type { McpRegistry } from '@trek/nest-mcp'; +import type { McpAttachOptions, McpRegistry } from '@trek/nest-mcp'; -export function registerTools(registry: McpRegistry | null, server: McpServer, userId: number, scopes: string[] | null, isStaticToken = false, getDeprecationNotice: () => string | null = () => null): void { +export function registerTools(registry: McpRegistry | null, server: McpServer, userId: number, scopes: string[] | null, isStaticToken = false, getDeprecationNotice: () => string | null = () => null, onInvoke?: McpAttachOptions['onInvoke']): void { // The trip tools moved to the DI-discovered src/nest/trips/trips.mcp.ts and // the share-link tools to src/nest/share/share.mcp.ts (@McpController, // attached via the nest-mcp registry below — getDeprecationNotice rides the @@ -65,5 +65,5 @@ export function registerTools(registry: McpRegistry | null, server: McpServer, u // container-discovered McpRegistryService (injected into the transport // service); the no-Nest harness hands in createTestRegistry()'s build. // A null registry (direct callers without either) ⇒ skip. - if (registry) registry.attach(server, { userId, scopes, isStaticToken, getDeprecationNotice }); + if (registry) registry.attach(server, { userId, scopes, isStaticToken, getDeprecationNotice }, { onInvoke }); } diff --git a/server/src/nest/mcp-transport/mcp-transport.module.ts b/server/src/nest/mcp-transport/mcp-transport.module.ts index bddc76739..554b673a9 100644 --- a/server/src/nest/mcp-transport/mcp-transport.module.ts +++ b/server/src/nest/mcp-transport/mcp-transport.module.ts @@ -5,6 +5,7 @@ import { AuthModule } from '../auth/auth.module'; import { TokensModule } from '../tokens/tokens.module'; import { OauthModule } from '../oauth/oauth.module'; import { AddonsModule } from '../addons/addons.module'; +import { AuditModule } from '../audit/audit.module'; /** * The /mcp transport endpoint — the OAuth 2.1-authenticated MCP server the @@ -14,7 +15,7 @@ import { AddonsModule } from '../addons/addons.module'; * bearer verification, session create/resume, rate limiting, SSE keep-alive. */ @Module({ - imports: [AuthModule, TokensModule, OauthModule, AddonsModule], + imports: [AuthModule, TokensModule, OauthModule, AddonsModule, AuditModule], controllers: [McpTransportController], providers: [McpTransportService], }) diff --git a/server/src/nest/mcp-transport/mcp-transport.service.ts b/server/src/nest/mcp-transport/mcp-transport.service.ts index 2335adaf6..d919616b1 100644 --- a/server/src/nest/mcp-transport/mcp-transport.service.ts +++ b/server/src/nest/mcp-transport/mcp-transport.service.ts @@ -15,6 +15,8 @@ import { AuthService } from '../auth/auth.service'; import { TokenService } from '../tokens/token.service'; import { OauthService } from '../oauth/oauth.service'; import { AddonsService } from '../addons/addons.service'; +import { AuditService } from '../audit/audit.service'; +import { getClientIp } from '../audit/client-ip'; /** * The MCP transport handler behind the container — the former non-Nest @@ -99,6 +101,7 @@ export class McpTransportService { private readonly tokens: TokenService, private readonly oauth: OauthService, private readonly addons: AddonsService, + private readonly audit: AuditService, private readonly registry: McpRegistryService, ) {} @@ -174,6 +177,7 @@ export class McpTransportService { return; } session.lastActivity = Date.now(); + session.lastClientIp = getClientIp(req); armSseKeepalive(res, () => { session.lastActivity = Date.now(); }); try { await session.transport.handleRequest(req, res, req.body); @@ -238,12 +242,37 @@ export class McpTransportService { return STATIC_TOKEN_DEPRECATION_NOTICE; }; - registerTools(this.registry, server, user.id, scopes, isStaticToken, getDeprecationNotice); + // Tool-call audit trail: who called which tool, when, from where. Fired by + // the registry immediately before each tool handler runs (the nest-mcp + // onInvoke seam) — transport-level body sniffing can't see tool names + // because /mcp bodies are raw. The IP is the session's most recent + // authorized request (tool calls arrive on resumed requests); an audit + // failure must never fail the tool call, so the closure catches its own. + const createIp = getClientIp(req); + const transportHolder: { current: StreamableHTTPServerTransport | null } = { current: null }; + const onInvoke = (info: { kind: string; name: string }): void => { + if (info.kind !== 'tool') return; + try { + const sid = transportHolder.current?.sessionId; + const ip = (sid ? sessions.get(sid)?.lastClientIp : null) ?? createIp; + this.audit.writeAudit({ + userId: user.id, + action: 'mcp.tool_call', + resource: info.name, + details: { clientId: clientId ?? 'native' }, + ip, + }); + } catch (err) { + console.error('[MCP] tool-call audit failed:', (err as Error | undefined)?.message ?? err); + } + }; + + registerTools(this.registry, server, user.id, scopes, isStaticToken, getDeprecationNotice, onInvoke); const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID(), onsessioninitialized: (sid) => { - sessions.set(sid, { server, transport, userId: user.id, scopes, clientId, isStaticToken, lastActivity: Date.now() }); + sessions.set(sid, { server, transport, userId: user.id, scopes, clientId, isStaticToken, lastActivity: Date.now(), lastClientIp: createIp }); const authMethod = isStaticToken ? 'static-token' : scopes ? `oauth(${scopes.join(',')})` : 'jwt'; console.log(`[MCP] Session ${sid} created for user ${user.id} [${authMethod}]. Active sessions: ${sessions.size}`); }, @@ -255,6 +284,7 @@ export class McpTransportService { transport.onclose = () => { if (transport.sessionId) sessions.delete(transport.sessionId); }; + transportHolder.current = transport; armSseKeepalive(res); try { diff --git a/server/tests/integration/mcp.test.ts b/server/tests/integration/mcp.test.ts index 06bd0b0fd..906086795 100644 --- a/server/tests/integration/mcp.test.ts +++ b/server/tests/integration/mcp.test.ts @@ -458,6 +458,55 @@ describe('MCP transport parity pins (Nest-hosted /mcp)', () => { expect(second.text).not.toContain('Deprecated authentication'); }); + it('MCP-P13 — an authorized tools/call writes exactly one mcp.tool_call audit row', async () => { + const { user } = createUser(testDb); + const { accessToken, clientId } = mintOauthToken(user.id, MCP_AUDIENCE); + 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: 'list_trips', arguments: {} } }); + expect(call.status).toBe(200); + + const rows = testDb.prepare( + "SELECT user_id, action, resource, details FROM audit_log WHERE action = 'mcp.tool_call'", + ).all() as Array<{ user_id: number; action: string; resource: string; details: string }>; + expect(rows).toHaveLength(1); + expect(rows[0].user_id).toBe(user.id); + expect(rows[0].resource).toBe('list_trips'); + expect(JSON.parse(rows[0].details)).toEqual({ clientId }); + }); + + it('MCP-P14 — tools/list and resource reads write no mcp.tool_call rows', async () => { + const { user } = createUser(testDb); + const token = generateToken(user.id); + const sessionId = await createSession(token); + testDb.prepare("DELETE FROM audit_log WHERE action = 'mcp.tool_call'").run(); + + 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 read = 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: 'resources/list', id: 3, params: {} }); + expect(read.status).toBe(200); + + const rows = testDb.prepare("SELECT id FROM audit_log WHERE action = 'mcp.tool_call'").all(); + expect(rows).toHaveLength(0); + }); + it('MCP-P10 — /mcp bodies stay raw: an initialize payload over the global 100kb cap succeeds', async () => { const { user } = createUser(testDb); const big = { diff --git a/server/tests/unit/mcp/sessionManager.test.ts b/server/tests/unit/mcp/sessionManager.test.ts index eebd3e39d..b451aaaf4 100644 --- a/server/tests/unit/mcp/sessionManager.test.ts +++ b/server/tests/unit/mcp/sessionManager.test.ts @@ -14,6 +14,7 @@ function makeSession(overrides: Partial = {}): McpSession { clientId: null, isStaticToken: false, lastActivity: Date.now(), + lastClientIp: null, ...overrides, }; } diff --git a/server/tests/unit/nest/mcp-transport.service.test.ts b/server/tests/unit/nest/mcp-transport.service.test.ts index 1972e9927..58014a27c 100644 --- a/server/tests/unit/nest/mcp-transport.service.test.ts +++ b/server/tests/unit/nest/mcp-transport.service.test.ts @@ -24,6 +24,7 @@ import type { AuthService } from '../../../src/nest/auth/auth.service'; import type { TokenService } from '../../../src/nest/tokens/token.service'; import type { OauthService } from '../../../src/nest/oauth/oauth.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { AuditService } from '../../../src/nest/audit/audit.service'; import type { McpRegistryService } from '@trek/nest-mcp'; import { IS_PUBLIC } from '../../../src/nest/auth/public.decorator'; @@ -39,8 +40,9 @@ function makeService(overrides: { const tokens = { verifyMcpToken: vi.fn(() => null), ...overrides.tokens } as unknown as TokenService; const oauth = { getUserByAccessToken: vi.fn(() => null), ...overrides.oauth } as unknown as OauthService; const addons = { isAddonEnabled: vi.fn(() => true), ...overrides.addons } as unknown as AddonsService; + const audit = { writeAudit: vi.fn() } as unknown as AuditService; const registry = { attach: vi.fn() } as unknown as McpRegistryService; - return { svc: new McpTransportService(auth, tokens, oauth, addons, registry), auth, tokens, oauth, addons }; + return { svc: new McpTransportService(auth, tokens, oauth, addons, audit, registry), auth, tokens, oauth, addons, audit }; } beforeEach(() => {