diff --git a/server/src/nest-mcp/index.ts b/server/src/nest-mcp/index.ts index 916eac8a7..f772f2782 100644 --- a/server/src/nest-mcp/index.ts +++ b/server/src/nest-mcp/index.ts @@ -28,10 +28,13 @@ export { type McpAttachOptions, type McpContext, type McpDeclarativeAccess, + type McpDynamicTool, + type McpDynamicToolSource, type McpEntry, type McpEntryKind, type McpModuleOptions, type McpRegistryListing, + type McpZodSchema, type PromptOptions, type ResourceOptions, type ResourceTemplateOptions, diff --git a/server/src/nest-mcp/registry.ts b/server/src/nest-mcp/registry.ts index eb789d12d..d48b7a802 100644 --- a/server/src/nest-mcp/registry.ts +++ b/server/src/nest-mcp/registry.ts @@ -4,6 +4,8 @@ import type { McpAccessValidator, McpAttachOptions, McpContext, + McpDynamicTool, + McpDynamicToolSource, McpEntry, McpRegistryListing, PromptOptions, @@ -14,8 +16,6 @@ import type { import { ResourceTemplate as SdkResourceTemplate } from '@modelcontextprotocol/sdk/server/mcp'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import type { ZodRawShape } from 'zod'; - interface BoundEntry { entry: McpEntry; instance: object; @@ -45,6 +45,16 @@ interface LooseRegistrar { registerPrompt(name: string, config: Record, cb: (...cbArgs: unknown[]) => unknown): unknown; } +/** `this` for a dynamic tool that declares no owner. */ +const NO_OWNER: object = Object.freeze({}); + +/** Stands in for the method name a decorated entry would carry, in diagnostics. */ +const DYNAMIC_METHOD_NAME = '(dynamic)'; + +function describeError(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + function describeBound({ entry, instance }: BoundEntry): string { return `${(instance as { constructor: { name: string } }).constructor.name}.${entry.methodName}`; } @@ -74,6 +84,8 @@ export class McpRegistry { private readonly bound: BoundEntry[] = []; private readonly accessPolicy?: McpAccessPolicy; private readonly validateAccess?: McpAccessValidator; + /** Memoised `reservedNames()`; dropped by register() so it can never go stale. */ + private reserved?: ReadonlySet; constructor(options: McpRegistryOptions = {}) { this.accessPolicy = options.accessPolicy; @@ -95,6 +107,7 @@ export class McpRegistry { const entry = getEntry(ctor, name); if (entry) this.bound.push({ entry, instance }); } + this.reserved = undefined; } /** @@ -122,6 +135,74 @@ export class McpRegistry { break; } } + // After the registered entries, never before. Four reasons, in order of how + // much they cost if ignored: a reservation bug then throws on the dynamic + // registration, inside the catch below, instead of on a registered one + // outside it; a throwing source cannot stop a built-in from attaching; + // tools/list is insertion-ordered, so host-contributed tools sort last, + // which is the right priority signal in a long list; and it reads the way + // it works — the registry, then whatever this session added on top. + if (opts?.dynamicTools) this.attachDynamicTools(registrar, ctx, opts, opts.dynamicTools); + } + + /** + * Every name a REGISTERED entry occupies — deliberately not every name this + * session can see. + * + * Reserving against what the session was granted would mean a token scoped to + * `trips:read` has no `create_trip` attached, so a dynamic tool could take + * that name for a caller holding no scope for it. The set is also + * kind-agnostic: the SDK namespaces tools and prompts separately, but a + * contributor owning a built-in's string in any namespace is a + * name-confusion surface, and the wider set costs one Set lookup. + */ + private reservedNames(): ReadonlySet { + if (!this.reserved) this.reserved = new Set(this.bound.map((b) => b.entry.options.name)); + return this.reserved; + } + + private attachDynamicTools( + registrar: LooseRegistrar, + ctx: McpContext, + opts: McpAttachOptions, + source: McpDynamicToolSource, + ): void { + let tools: readonly McpDynamicTool[]; + try { + tools = source(ctx) ?? []; + } catch (err) { + // A session with no dynamic tools is degraded; a session that throws here + // is a 500 on initialize, because hosts call attach() outside their try. + console.warn(`[nest-mcp] dynamic tool source failed, no dynamic tools this session: ${describeError(err)}`); + return; + } + const reserved = this.reservedNames(); + const claimed = new Set(); + for (const tool of tools) { + const name = tool?.options?.name; + try { + if (typeof tool?.handler !== 'function') throw new Error('handler is not a function'); + if (typeof name !== 'string' || !name) throw new Error('name is missing'); + // The type says required, but this is a trust boundary: an absent + // marker takes allowed()'s "always registered" branch. + if (tool.options.access === undefined) throw new Error('access is required for a dynamic tool'); + if (reserved.has(name)) throw new Error('name is reserved by a registered entry'); + if (claimed.has(name)) throw new Error('duplicate name in this source'); + // Claimed before the gate, so a denied entry still owns its name and a + // later duplicate cannot slip in behind it. + claimed.add(name); + const owner = tool.owner ?? NO_OWNER; + // allowed() is inside the try because it is the second live throw site: + // declarative access with no configured accessPolicy. Registered + // entries only avoid it because validate() pre-checks them at boot, and + // a per-session source has no boot to be checked at. + const entry: McpEntry = { kind: 'tool', methodName: DYNAMIC_METHOD_NAME, options: tool.options }; + if (!this.allowed(entry, ctx, owner)) continue; + this.attachTool(registrar, tool.options, owner, tool.handler as AnyHandler, ctx, opts); + } catch (err) { + console.warn(`[nest-mcp] skipped dynamic tool "${String(name)}": ${describeError(err)}`); + } + } } /** @@ -213,7 +294,7 @@ export class McpRegistry { const config: Record = { title: options.title, description: options.description, - inputSchema: options.inputSchema as ZodRawShape | undefined, + inputSchema: options.inputSchema, outputSchema: options.outputSchema, annotations: options.annotations, _meta: options._meta, diff --git a/server/src/nest-mcp/types.ts b/server/src/nest-mcp/types.ts index 31b6c0485..6cad03e3b 100644 --- a/server/src/nest-mcp/types.ts +++ b/server/src/nest-mcp/types.ts @@ -109,13 +109,30 @@ interface McpEntryOptionsBase { } /** - * Mirrors the SDK's `registerTool` config. `inputSchema` is a ZodRawShape - * passed straight through — the SDK does the parsing/validation. + * A whole Zod schema, as opposed to the shape its properties are spelled out + * in. Structural for the same reason `annotations`/`_meta` are: the package's + * public d.ts never imports a deep zod subpath. + * + * The SDK accepts either form (`registerTool`, and `normalizeObjectSchema` narrows them to + * one object schema), so passing a whole schema costs nothing here. What it + * buys is `.meta()`: Zod merges registry metadata into the `toJSONSchema` + * output that `tools/list` is rendered from, which is how a host advertises a + * schema it did not author without converting it into Zod first. + */ +export interface McpZodSchema { + readonly _zod: unknown; + parse(value: unknown): unknown; +} + +/** + * Mirrors the SDK's `registerTool` config. `inputSchema` is passed straight + * through — the SDK does the parsing/validation. * `annotations`/`_meta` are kept structural so the package's public d.ts * never imports deep SDK subpaths. */ export interface ToolOptions extends McpEntryOptionsBase { - inputSchema?: ZodRawShape; + inputSchema?: ZodRawShape | McpZodSchema; outputSchema?: ZodRawShape; annotations?: Record; _meta?: Record; @@ -153,8 +170,49 @@ export interface McpAttachOptions { * failure handling. */ onInvoke?: (info: { kind: McpEntryKind; name: string }) => void; + /** + * Extra tools contributed for THIS session only, on top of the decorated + * ones. Consulted once per `attach()`, after every registered entry. + * + * They go through the same `attachTool()` as a decorated entry, so `onInvoke` + * fires for them and the host's audit trail sees them. That is the whole + * reason this lives here rather than the host calling `server.registerTool()` + * itself: anything registered outside `attach()` is invisible to the seam. + * + * Contract, and the inverse of `onInvoke`'s: this source and its entries MAY + * throw. `attach()` contains both, per entry, because hosts call it outside + * their request try block — an escape there is a 500 on every `initialize`, + * so one bad contributor would take MCP down for everyone. A failure costs + * that one tool (or, for the source itself, this session's dynamic tools); + * it never costs a registered entry. + */ + dynamicTools?: McpDynamicToolSource; } +/** One host-contributed tool, supplied per session by an `McpDynamicToolSource`. */ +export interface McpDynamicTool { + /** + * `access` is required here, unlike on a decorated entry: `allowed()` reads + * an absent marker as "always registered", so omitting it would put the tool + * on every session ungated. `attach()` re-checks it at runtime — this is a + * trust boundary, and the type alone is not the enforcement. + */ + options: ToolOptions & { access: McpAccess }; + handler: (args: unknown, ctx: McpContext) => unknown; + /** Bound as `this` on `handler`, and handed to `when(ctx, self)`. */ + owner?: object; +} + +/** + * Consulted once per `attach()`, with that session's ctx. + * + * Synchronous by design: `attach()` runs inside the `initialize` request, so an + * awaited source would put a third party's round trip in front of every session + * creation. A source that needs to ask something slow what tools exist should + * answer from state it already holds. + */ +export type McpDynamicToolSource = (ctx: McpContext) => readonly McpDynamicTool[]; + export type McpEntry = | { kind: 'tool'; methodName: string; options: ToolOptions } | { kind: 'resource'; methodName: string; options: ResourceOptions } diff --git a/server/tests/unit/nest-mcp/decorators.test.ts b/server/tests/unit/nest-mcp/decorators.test.ts index e9f9b1a16..c378f6181 100644 --- a/server/tests/unit/nest-mcp/decorators.test.ts +++ b/server/tests/unit/nest-mcp/decorators.test.ts @@ -2,7 +2,7 @@ import { McpController, Prompt, Resource, ResourceTemplate, Tool } from '../../. import { getEntry, isMcpController, type ClassRef } from '../../../src/nest-mcp/metadata'; import { describe, expect, it } from 'vitest'; -import { z } from 'zod'; +import { z, type ZodRawShape } from 'zod'; @McpController() class Decorated { @@ -83,7 +83,9 @@ describe('nest-mcp decorators', () => { const tool = getEntry(ctor(Decorated), 'doThing'); expect(tool?.kind).toBe('tool'); if (tool?.kind === 'tool') { - expect(tool.options.inputSchema?.what).toBeInstanceOf(z.ZodString); + // inputSchema is `ZodRawShape | McpZodSchema` since dynamic tools started + // passing whole schemas; this decorator declared the raw-shape arm. + expect((tool.options.inputSchema as ZodRawShape).what).toBeInstanceOf(z.ZodString); } }); diff --git a/server/tests/unit/nest-mcp/dynamic-tools.test.ts b/server/tests/unit/nest-mcp/dynamic-tools.test.ts new file mode 100644 index 000000000..06f077982 --- /dev/null +++ b/server/tests/unit/nest-mcp/dynamic-tools.test.ts @@ -0,0 +1,297 @@ +/** + * McpAttachOptions.dynamicTools — DYNTOOL-001 to DYNTOOL-012. + * + * The per-session seam that lets a host contribute tools it did not decorate. + * Two properties carry the weight here and both are security properties, not + * conveniences: a contributed tool can never take a registered entry's name, + * and nothing a contributor does can make attach() throw. Hosts call attach() + * outside their request try block, so an escape is a 500 on every initialize. + */ +import { McpController, Tool, createTestRegistry, type McpAccessPolicy, type McpDynamicTool, type McpContext } from '../../../src/nest-mcp'; +import { createAttachHarness, type TestCtx } from './harness'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { z } from 'zod'; + +@McpController() +class Builtins { + @Tool({ + name: 'read_thing', + description: 'A registered read tool.', + inputSchema: { id: z.number() }, + access: { group: 'thing', mode: 'read' } as never, + }) + readThing(): { content: Array<{ type: 'text'; text: string }> } { + return { content: [{ type: 'text', text: 'builtin read' }] }; + } + + @Tool({ + name: 'write_thing', + description: 'A registered write tool.', + inputSchema: { id: z.number() }, + access: { group: 'thing', mode: 'write' } as never, + }) + writeThing(): { content: Array<{ type: 'text'; text: string }> } { + return { content: [{ type: 'text', text: 'builtin write' }] }; + } +} + +/** No access marker, so it registers on a registry with no policy configured. */ +@McpController() +class Ungated { + @Tool({ name: 'ungated_thing', description: 'Needs no policy.' }) + ungatedThing(): { content: Array<{ type: 'text'; text: string }> } { + return { content: [{ type: 'text', text: 'ungated' }] }; + } +} + +/** Grants read to everyone, write only to a ctx that asked for it. */ +const policy: McpAccessPolicy = ({ mode }, ctx) => + mode === 'read' ? true : (ctx as TestCtx).canWrite === true; + +function makeRegistry() { + return createTestRegistry([new Builtins()], { accessPolicy: policy }); +} + +function dynamicTool(over: Partial = {}): McpDynamicTool { + return { + options: { + name: 'plugin_demo_echo', + description: 'Echoes.', + inputSchema: { value: z.string() }, + access: { group: 'thing', mode: 'read' } as never, + ...(over.options ?? {}), + }, + handler: () => ({ content: [{ type: 'text', text: 'dynamic' }] }), + ...(over.handler ? { handler: over.handler } : {}), + ...(over.owner ? { owner: over.owner } : {}), + } as McpDynamicTool; +} + +const listNames = async (client: { listTools: () => Promise<{ tools: Array<{ name: string }> }> }) => + (await client.listTools()).tools.map((t) => t.name); + +let warn: ReturnType; + +beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); +afterEach(() => { + warn.mockRestore(); +}); + +describe('dynamic tools: the happy path', () => { + it('DYNTOOL-001: registers a contributed tool and calls it', async () => { + const h = await createAttachHarness(makeRegistry(), { canWrite: true }, { + dynamicTools: () => [dynamicTool()], + }); + + expect(await listNames(h.client)).toContain('plugin_demo_echo'); + const res = await h.client.callTool({ name: 'plugin_demo_echo', arguments: { value: 'x' } }); + expect((res.content as Array<{ text: string }>)[0].text).toBe('dynamic'); + + await h.cleanup(); + }); + + it('DYNTOOL-002: binds owner as `this` on the handler', async () => { + const owner = { marker: 'the-owner' }; + const h = await createAttachHarness(makeRegistry(), {}, { + dynamicTools: () => [ + dynamicTool({ + owner, + handler: function (this: typeof owner) { + return { content: [{ type: 'text', text: this.marker }] }; + }, + }), + ], + }); + + const res = await h.client.callTool({ name: 'plugin_demo_echo', arguments: { value: 'x' } }); + expect((res.content as Array<{ text: string }>)[0].text).toBe('the-owner'); + + await h.cleanup(); + }); + + it('DYNTOOL-003: hands the owner to when(ctx, self)', async () => { + const owner = { enabled: false }; + const seen: object[] = []; + const h = await createAttachHarness(makeRegistry(), {}, { + dynamicTools: () => [ + dynamicTool({ + owner, + options: { + name: 'plugin_demo_gated', + description: 'Gated.', + access: { group: 'thing', mode: 'read' }, + when: (_ctx, self) => { + seen.push(self); + return (self as typeof owner).enabled; + }, + } as unknown as McpDynamicTool['options'], + }), + ], + }); + + expect(seen).toEqual([owner]); + expect(await listNames(h.client)).not.toContain('plugin_demo_gated'); + + await h.cleanup(); + }); + + it('DYNTOOL-004: fires onInvoke, so the host audit trail sees the call', async () => { + const onInvoke = vi.fn(); + const h = await createAttachHarness(makeRegistry(), {}, { + onInvoke, + dynamicTools: () => [dynamicTool()], + }); + + await h.client.callTool({ name: 'plugin_demo_echo', arguments: { value: 'x' } }); + + expect(onInvoke).toHaveBeenCalledWith({ kind: 'tool', name: 'plugin_demo_echo' }); + + await h.cleanup(); + }); + + it('DYNTOOL-005: registers contributed tools after every registered entry', async () => { + const h = await createAttachHarness(makeRegistry(), { canWrite: true }, { + dynamicTools: () => [dynamicTool()], + }); + + const names = await listNames(h.client); + expect(names.indexOf('plugin_demo_echo')).toBeGreaterThan(names.indexOf('write_thing')); + + await h.cleanup(); + }); +}); + +describe('dynamic tools: name reservation', () => { + it('DYNTOOL-006: cannot take the name of a registered entry', async () => { + const h = await createAttachHarness(makeRegistry(), { canWrite: true }, { + dynamicTools: () => [dynamicTool({ options: { name: 'read_thing' } as McpDynamicTool['options'] })], + }); + + const names = await listNames(h.client); + expect(names.filter((n) => n === 'read_thing')).toHaveLength(1); + const res = await h.client.callTool({ name: 'read_thing', arguments: { id: 1 } }); + expect((res.content as Array<{ text: string }>)[0].text).toBe('builtin read'); + + await h.cleanup(); + }); + + it('DYNTOOL-007: reserves against every registered name, not the ones this session got', async () => { + // canWrite is false, so write_thing is NOT attached for this session. The + // name is still reserved: otherwise a contributor could occupy a built-in + // for exactly the callers who hold no scope for it. + const h = await createAttachHarness(makeRegistry(), { canWrite: false }, { + dynamicTools: () => [dynamicTool({ options: { name: 'write_thing' } as McpDynamicTool['options'] })], + }); + + expect(await listNames(h.client)).not.toContain('write_thing'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('reserved by a registered entry')); + + await h.cleanup(); + }); + + it('DYNTOOL-008: first wins on a duplicate within one source, and a denied entry still holds its name', async () => { + const h = await createAttachHarness(makeRegistry(), {}, { + dynamicTools: () => [ + // Denied by `when`, but it has claimed the name. + dynamicTool({ + options: { + name: 'plugin_demo_echo', + description: 'First, denied.', + access: { group: 'thing', mode: 'read' }, + when: () => false, + } as unknown as McpDynamicTool['options'], + }), + dynamicTool({ options: { name: 'plugin_demo_echo', description: 'Second.' } as McpDynamicTool['options'] }), + ], + }); + + expect(await listNames(h.client)).not.toContain('plugin_demo_echo'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('duplicate name in this source')); + + await h.cleanup(); + }); +}); + +describe('dynamic tools: containment', () => { + it('DYNTOOL-009: a throwing source costs the dynamic tools, not the session', async () => { + const h = await createAttachHarness(makeRegistry(), { canWrite: true }, { + dynamicTools: () => { + throw new Error('the runtime is down'); + }, + }); + + expect(await listNames(h.client)).toEqual(['read_thing', 'write_thing']); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('the runtime is down')); + + await h.cleanup(); + }); + + it('DYNTOOL-010: one bad entry is skipped and the rest still register', async () => { + const h = await createAttachHarness(makeRegistry(), {}, { + dynamicTools: () => [ + { options: { name: 'plugin_demo_broken', description: 'No handler.', access: { group: 'thing', mode: 'read' } }, handler: undefined } as unknown as McpDynamicTool, + dynamicTool(), + ], + }); + + const names = await listNames(h.client); + expect(names).not.toContain('plugin_demo_broken'); + expect(names).toContain('plugin_demo_echo'); + + await h.cleanup(); + }); + + it('DYNTOOL-011: an entry with no access marker is skipped, never registered ungated', async () => { + const h = await createAttachHarness(makeRegistry(), {}, { + dynamicTools: () => [ + { options: { name: 'plugin_demo_ungated', description: 'No access.' }, handler: () => ({ content: [] }) } as unknown as McpDynamicTool, + ], + }); + + expect(await listNames(h.client)).not.toContain('plugin_demo_ungated'); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('access is required')); + + await h.cleanup(); + }); + + it('DYNTOOL-012: declarative access with no configured policy is skipped, not thrown', async () => { + // The second live throw site. A registered entry only avoids it because + // validate() pre-checks it at boot; a per-session source has no boot. + // The registry holds one marker-free entry so the server still advertises + // a tools capability, which is what makes the skip observable. + const noPolicy = createTestRegistry([new Ungated()]); + const h = await createAttachHarness(noPolicy, {}, { dynamicTools: () => [dynamicTool()] }); + + expect(await listNames(h.client)).toEqual(['ungated_thing']); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessPolicy was configured')); + + await h.cleanup(); + }); +}); + +describe('dynamic tools: absent', () => { + it('DYNTOOL-013: attach without a source behaves exactly as before', async () => { + const h = await createAttachHarness(makeRegistry(), { canWrite: true }); + + expect(await listNames(h.client)).toEqual(['read_thing', 'write_thing']); + expect(warn).not.toHaveBeenCalled(); + + await h.cleanup(); + }); +}); + +/** Kept honest: the ctx handed to the source is the session's own. */ +describe('dynamic tools: context', () => { + it('DYNTOOL-014: the source is consulted once, with this session ctx', async () => { + const source = vi.fn((_ctx: McpContext) => [dynamicTool()]); + const h = await createAttachHarness(makeRegistry(), { userId: 42, canWrite: true }, { dynamicTools: source }); + + expect(source).toHaveBeenCalledTimes(1); + expect(source.mock.calls[0][0]).toMatchObject({ userId: 42 }); + + await h.cleanup(); + }); +});