From 59ea420f1dcae118c62ad14fc0f1bdd065fe4533 Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 28 Jul 2026 20:56:29 +0200 Subject: [PATCH] feat(nest-mcp): host-typed access groups + entry validation hook --- nest-mcp/README.md | 27 +++++++- nest-mcp/src/index.ts | 6 +- nest-mcp/src/registry.ts | 38 +++++----- nest-mcp/src/types.ts | 37 +++++++++- nest-mcp/tests/create-test-registry.test.ts | 9 +++ nest-mcp/tests/registry.test.ts | 77 ++++++++++++++++++++- 6 files changed, 172 insertions(+), 22 deletions(-) diff --git a/nest-mcp/README.md b/nest-mcp/README.md index 86c0ce929..3f2d47dc0 100644 --- a/nest-mcp/README.md +++ b/nest-mcp/README.md @@ -40,7 +40,7 @@ export class ThingsMcp { Every decorator accepts `access` and `when`: -- **Declarative** — `access: { group: 'things', mode: 'read' | 'write' }`, resolved by the `accessPolicy` given once at `McpModule.forRoot(...)`. +- **Declarative** — `access: { group: 'things', mode: 'read' | 'write' }`, resolved by the `accessPolicy` given once at `McpModule.forRoot(...)`. `group` is typed as `McpAccessGroup` — plain `string` until the host augments `McpAccessGroupRegistry` (see below), after which only the host's registered groups compile. - **Predicate** — `access: (ctx) => boolean`, bypasses the policy. - **Availability gate** — `when: (ctx) => boolean`, evaluated *before* `access` (both must pass). Use it for feature/addon toggles so scope markers stay declarative: `when: () => isAddonEnabled(...)`, `access: { group: 'packing', mode: 'read' }`. - **Omitted `access`** — the entry is always registered (subject to `when`). @@ -64,7 +64,20 @@ Six `TOOL_ANNOTATIONS_*` presets cover the read/write/delete/idempotency matrix `McpRegistryService` runs `registry.validate()` at boot (`onModuleInit`), and `createTestRegistry` runs it on construction, so misconfiguration breaks startup instead of MCP session creation: - duplicate names per kind (fixed resources: duplicate URIs); -- declarative `access` without a configured `accessPolicy`. +- declarative `access` without a configured `accessPolicy`; +- declarative `access` rejected by the optional host-supplied `validateAccess` hook. + +All problems are aggregated into one `Invalid MCP registry: ...` error, so a single boot failure reports every misconfiguration. + +`validateAccess?: (access, entry) => string | null | undefined` (on both `McpModule.forRoot(...)` and `createTestRegistry(...)` options) is called once per entry with declarative access — predicates are opaque to the host and skipped. Return a problem description to fail boot, null/undefined to accept: + +```ts +McpModule.forRoot({ + accessPolicy, + validateAccess: ({ group, mode }) => + hostKnowsGroupMode(group, mode) ? null : `no '${group}:${mode}' scope`, +}) +``` ### Context typing @@ -80,6 +93,16 @@ declare module '@trek/nest-mcp' { } ``` +### Access-group typing + +Same trick for `access.group`: the package exports an empty `McpAccessGroupRegistry` interface, and `McpAccessGroup` (the type of `group`) is `string` until the host augments it — after which every decorator's `group` is checked against the host's union at compile time: + +```ts +declare module '@trek/nest-mcp' { + interface McpAccessGroupRegistry extends Record {} +} +``` + ### Wiring ```ts diff --git a/nest-mcp/src/index.ts b/nest-mcp/src/index.ts index 2f4bec360..c7d2f9cc0 100644 --- a/nest-mcp/src/index.ts +++ b/nest-mcp/src/index.ts @@ -13,18 +13,22 @@ export { } from './helpers'; export { McpRegistryService } from './mcp-registry.service'; export { McpModule } from './mcp.module'; -export { McpRegistry, type McpRegistryListing, type McpRegistryOptions } from './registry'; +export { McpRegistry, type McpRegistryOptions } from './registry'; export { createTestRegistry } from './testing'; export { MCP_MODULE_OPTIONS, type McpAccess, + type McpAccessGroup, + type McpAccessGroupRegistry, type McpAccessPolicy, type McpAccessPredicate, + type McpAccessValidator, type McpContext, type McpDeclarativeAccess, type McpEntry, type McpEntryKind, type McpModuleOptions, + type McpRegistryListing, type PromptOptions, type ResourceOptions, type ResourceTemplateOptions, diff --git a/nest-mcp/src/registry.ts b/nest-mcp/src/registry.ts index ff396d3bc..03a69f7e1 100644 --- a/nest-mcp/src/registry.ts +++ b/nest-mcp/src/registry.ts @@ -1,10 +1,10 @@ import { getEntry, isMcpController, type ClassRef } from './metadata'; import type { - McpAccess, McpAccessPolicy, + McpAccessValidator, McpContext, McpEntry, - McpEntryKind, + McpRegistryListing, PromptOptions, ResourceOptions, ResourceTemplateOptions, @@ -20,16 +20,9 @@ interface BoundEntry { instance: object; } -export interface McpRegistryListing { - kind: McpEntryKind; - name: string; - className: string; - methodName: string; - access?: McpAccess; -} - export interface McpRegistryOptions { accessPolicy?: McpAccessPolicy; + validateAccess?: McpAccessValidator; } type AnyHandler = (this: unknown, ...handlerArgs: unknown[]) => unknown; @@ -79,9 +72,11 @@ function enumerateMethodNames(instance: object): string[] { export class McpRegistry { private readonly bound: BoundEntry[] = []; private readonly accessPolicy?: McpAccessPolicy; + private readonly validateAccess?: McpAccessValidator; constructor(options: McpRegistryOptions = {}) { this.accessPolicy = options.accessPolicy; + this.validateAccess = options.validateAccess; } /** @@ -134,12 +129,17 @@ export class McpRegistry { * session creation: * - duplicate names per kind (fixed resources: duplicate URIs), which the * SDK would otherwise reject per-session at attach(); - * - declarative access without a configured accessPolicy. + * - declarative access without a configured accessPolicy; + * - declarative access the host-supplied `validateAccess` hook rejects + * (predicates are opaque to the host and never passed to it). + * All problems are aggregated into one error so a single boot failure + * reports every misconfiguration. */ validate(): void { const seen = new Map(); const duplicates: string[] = []; const unresolvable: string[] = []; + const invalidAccess: string[] = []; for (const bound of this.bound) { const { entry } = bound; const key = @@ -148,9 +148,10 @@ export class McpRegistry { if (prior) duplicates.push(`${key} (${describeBound(prior)} and ${describeBound(bound)})`); else seen.set(key, bound); const access = entry.options.access; - if (!this.accessPolicy && access !== undefined && typeof access !== 'function') { - unresolvable.push(`${key} (${describeBound(bound)})`); - } + if (access === undefined || typeof access === 'function') continue; + if (!this.accessPolicy) unresolvable.push(`${key} (${describeBound(bound)})`); + const problem = this.validateAccess?.(access, this.toListing(bound)); + if (problem) invalidAccess.push(`${key} (${describeBound(bound)}): ${problem}`); } const problems: string[] = []; if (duplicates.length) problems.push(`duplicate MCP registrations: ${duplicates.join(', ')}`); @@ -160,18 +161,23 @@ export class McpRegistry { `(McpModule.forRoot({ accessPolicy })): ${unresolvable.join(', ')}`, ); } + if (invalidAccess.length) problems.push(`invalid access declarations: ${invalidAccess.join(', ')}`); if (problems.length) throw new Error(`Invalid MCP registry: ${problems.join('; ')}`); } /** Introspection: every recorded entry, regardless of access. */ list(): McpRegistryListing[] { - return this.bound.map(({ entry, instance }) => ({ + return this.bound.map((bound) => this.toListing(bound)); + } + + private toListing({ entry, instance }: BoundEntry): McpRegistryListing { + return { kind: entry.kind, name: entry.options.name, className: (instance as { constructor: { name: string } }).constructor.name, methodName: entry.methodName, access: entry.options.access, - })); + }; } private allowed(entry: McpEntry, ctx: McpContext): boolean { diff --git a/nest-mcp/src/types.ts b/nest-mcp/src/types.ts index 3c051dbd1..6af473953 100644 --- a/nest-mcp/src/types.ts +++ b/nest-mcp/src/types.ts @@ -14,12 +14,30 @@ import type { ZodRawShape } from 'zod'; // eslint-disable-next-line @typescript-eslint/no-empty-object-type export interface McpContext {} +/** + * Host-augmentable registry of valid `access.group` values — the `McpContext` + * augmentation trick applied to groups. Augment once: + * + * declare module '@trek/nest-mcp' { + * interface McpAccessGroupRegistry extends Record {} + * } + * + * Unaugmented, `McpAccessGroup` stays `string` — the package attaches no + * semantics to groups, same contract as `McpContext`. + */ +// eslint-disable-next-line @typescript-eslint/no-empty-object-type +export interface McpAccessGroupRegistry {} + +export type McpAccessGroup = keyof McpAccessGroupRegistry extends never + ? string + : keyof McpAccessGroupRegistry & string; + /** * Declarative access marker resolved by the host-supplied `accessPolicy`. * The package attaches no semantics to `group`/`mode` — the policy does. */ export interface McpDeclarativeAccess { - group: string; + group: McpAccessGroup; mode: 'read' | 'write'; } @@ -90,8 +108,25 @@ export type McpEntry = | { kind: 'resourceTemplate'; methodName: string; options: ResourceTemplateOptions } | { kind: 'prompt'; methodName: string; options: PromptOptions }; +/** Introspection view of one recorded entry (see `McpRegistry.list()`). */ +export interface McpRegistryListing { + kind: McpEntryKind; + name: string; + className: string; + methodName: string; + access?: McpAccess; +} + +/** + * Host-supplied per-entry check run by `validate()`. Return a problem + * description to fail boot, null/undefined to accept. Only called for + * entries with DECLARATIVE access (predicates are opaque to the host). + */ +export type McpAccessValidator = (access: McpDeclarativeAccess, entry: McpRegistryListing) => string | null | undefined; + export interface McpModuleOptions { accessPolicy?: McpAccessPolicy; + validateAccess?: McpAccessValidator; } /** Injection token for the options object given to `McpModule.forRoot()`. */ diff --git a/nest-mcp/tests/create-test-registry.test.ts b/nest-mcp/tests/create-test-registry.test.ts index d4e483c11..bb4e27de9 100644 --- a/nest-mcp/tests/create-test-registry.test.ts +++ b/nest-mcp/tests/create-test-registry.test.ts @@ -26,6 +26,15 @@ describe('createTestRegistry', () => { expect(() => createTestRegistry([new One(), new Two()])).toThrow(/no accessPolicy was configured/); }); + it('surfaces a validateAccess rejection on construction', () => { + expect(() => + createTestRegistry([new Two()], { + accessPolicy: () => true, + validateAccess: ({ group, mode }) => `unknown group '${group}' (mode '${mode}')`, + }), + ).toThrow(/invalid access declarations: tool "two_tool" \(Two\.two\): unknown group 't' \(mode 'write'\)/); + }); + it('forwards the accessPolicy option', () => { const calls: string[] = []; const policy: McpAccessPolicy = (access, ctx) => { diff --git a/nest-mcp/tests/registry.test.ts b/nest-mcp/tests/registry.test.ts index 4935608cc..352221096 100644 --- a/nest-mcp/tests/registry.test.ts +++ b/nest-mcp/tests/registry.test.ts @@ -1,6 +1,6 @@ -import { McpController, McpRegistry, Resource, Tool } from '../src'; +import { McpController, McpRegistry, Resource, Tool, type McpAccessValidator } from '../src'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; @McpController() class Alpha { @@ -106,3 +106,76 @@ describe('McpRegistry.validate', () => { ); }); }); + +@McpController() +class MixedAccess { + @Tool({ name: 'open_tool' }) + open() {} + + @Tool({ name: 'predicate_tool', access: () => true }) + predicated() {} + + @Tool({ name: 'declared_read', access: { group: 'a', mode: 'read' } }) + declaredRead() {} + + @Tool({ name: 'declared_write', access: { group: 'b', mode: 'write' } }) + declaredWrite() {} +} + +describe('McpRegistry.validate — validateAccess hook', () => { + it('invokes the hook once per declarative entry, never for predicate/omitted access', () => { + const validateAccess = vi.fn(() => null); + const registry = new McpRegistry({ accessPolicy: () => true, validateAccess }); + registry.register(new MixedAccess()); + expect(() => registry.validate()).not.toThrow(); + expect(validateAccess).toHaveBeenCalledTimes(2); + expect(validateAccess).toHaveBeenCalledWith( + { group: 'a', mode: 'read' }, + expect.objectContaining({ + kind: 'tool', + name: 'declared_read', + className: 'MixedAccess', + methodName: 'declaredRead', + }), + ); + expect(validateAccess).toHaveBeenCalledWith( + { group: 'b', mode: 'write' }, + expect.objectContaining({ + kind: 'tool', + name: 'declared_write', + className: 'MixedAccess', + methodName: 'declaredWrite', + }), + ); + }); + + it('accepts null and undefined returns', () => { + const returns: (string | null | undefined)[] = [null, undefined]; + const registry = new McpRegistry({ accessPolicy: () => true, validateAccess: () => returns.shift() }); + registry.register(new MixedAccess()); + expect(() => registry.validate()).not.toThrow(); + }); + + it('aggregates every rejected entry into one error naming each origin', () => { + const registry = new McpRegistry({ + accessPolicy: () => true, + validateAccess: ({ group, mode }) => `no '${group}:${mode}' scope`, + }); + registry.register(new MixedAccess()); + expect(() => registry.validate()).toThrow( + /Invalid MCP registry: invalid access declarations: tool "declared_read" \(MixedAccess\.declaredRead\): no 'a:read' scope, tool "declared_write" \(MixedAccess\.declaredWrite\): no 'b:write' scope/, + ); + }); + + it('composes with duplicate-name problems in the same aggregated error', () => { + const registry = new McpRegistry({ + accessPolicy: () => true, + validateAccess: ({ mode }) => (mode === 'write' ? 'bad group' : null), + }); + registry.register(new Alpha()); + registry.register(new AlphaClone()); + expect(() => registry.validate()).toThrow( + /duplicate MCP registrations: tool "alpha_one".*; invalid access declarations: tool "alpha_two" \(Alpha\.two\): bad group/, + ); + }); +});