mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(mcp): audit MCP tool calls at the registry seam
This commit is contained in:
@@ -25,6 +25,7 @@ export {
|
||||
type McpAccessPolicy,
|
||||
type McpAccessPredicate,
|
||||
type McpAccessValidator,
|
||||
type McpAttachOptions,
|
||||
type McpContext,
|
||||
type McpDeclarativeAccess,
|
||||
type McpEntry,
|
||||
|
||||
+36
-14
@@ -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<string, AnyHandler>)[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<string, unknown> = {
|
||||
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<string, unknown> = {
|
||||
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<string, unknown> = {
|
||||
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<string, unknown> = {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<AttachHarness> {
|
||||
export async function createAttachHarness(
|
||||
registry: McpRegistry,
|
||||
ctx: TestCtx,
|
||||
opts?: McpAttachOptions,
|
||||
): Promise<AttachHarness> {
|
||||
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' });
|
||||
|
||||
@@ -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<string, McpSession>();
|
||||
|
||||
@@ -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 });
|
||||
}
|
||||
|
||||
@@ -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],
|
||||
})
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -14,6 +14,7 @@ function makeSession(overrides: Partial<McpSession> = {}): McpSession {
|
||||
clientId: null,
|
||||
isStaticToken: false,
|
||||
lastActivity: Date.now(),
|
||||
lastClientIp: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
Reference in New Issue
Block a user