From 954a3ca0a7780a22a87a41dc69dcf395377de2d1 Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 11 Aug 2026 23:01:19 +0200 Subject: [PATCH] refactor(server): serve /mcp from a Nest controller; delete auth, audit, addons and oauth bridges --- server/src/bootstrap.ts | 42 +- server/src/mcp/index.ts | 359 +----------------- server/src/mcp/registry-handoff.ts | 20 - server/src/mcp/tools.ts | 13 +- server/src/nest/addons/addons.bridge.ts | 26 -- server/src/nest/app.module.ts | 3 +- server/src/nest/audit/audit.bridge.ts | 22 -- server/src/nest/auth/auth.bridge.ts | 56 --- .../src/nest/common/validate-route-guards.ts | 4 + .../mcp-transport/mcp-transport.constants.ts | 79 ++++ .../mcp-transport/mcp-transport.controller.ts | 34 ++ .../mcp-transport/mcp-transport.module.ts | 21 + .../mcp-transport/mcp-transport.service.ts | 279 ++++++++++++++ server/src/nest/oauth/oauth.bridge.ts | 25 -- server/src/nest/platform/platform.routes.ts | 16 - server/tests/helpers/mcp-harness.ts | 8 +- server/tests/integration/mcp.test.ts | 184 +++++++++ server/tests/unit/nest/addons.bridge.test.ts | 44 --- server/tests/unit/nest/audit.service.test.ts | 15 - server/tests/unit/nest/auth.service.test.ts | 14 +- .../unit/nest/mcp-transport.service.test.ts | 200 ++++++++++ server/tests/unit/nest/oauth.service.test.ts | 16 +- .../unit/nest/platform.controller.test.ts | 19 - 23 files changed, 861 insertions(+), 638 deletions(-) delete mode 100644 server/src/mcp/registry-handoff.ts delete mode 100644 server/src/nest/addons/addons.bridge.ts delete mode 100644 server/src/nest/audit/audit.bridge.ts delete mode 100644 server/src/nest/auth/auth.bridge.ts create mode 100644 server/src/nest/mcp-transport/mcp-transport.constants.ts create mode 100644 server/src/nest/mcp-transport/mcp-transport.controller.ts create mode 100644 server/src/nest/mcp-transport/mcp-transport.module.ts create mode 100644 server/src/nest/mcp-transport/mcp-transport.service.ts delete mode 100644 server/src/nest/oauth/oauth.bridge.ts delete mode 100644 server/tests/unit/nest/addons.bridge.test.ts create mode 100644 server/tests/unit/nest/mcp-transport.service.test.ts diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index b52c6c5a0..ff64f2e63 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -1,18 +1,17 @@ import nodeHttp from 'node:http'; +import express from 'express'; +import type { Request, Response, NextFunction, RequestHandler } from 'express'; import { NestFactory } from '@nestjs/core'; -import { ExpressAdapter, type NestExpressApplication } from '@nestjs/platform-express'; +import { ExpressAdapter } from '@nestjs/platform-express'; import type { INestApplication } from '@nestjs/common'; import type { ConfigType } from '@nestjs/config'; import { AppModule } from './nest/app.module'; import { httpConfig } from './nest/app-config'; import { applyGlobalMiddleware } from './middleware/globalMiddleware'; -import { applyPlatformUploads, applyPlatformTransport, applyPlatformStatic } from './nest/platform/platform.routes'; +import { applyPlatformUploads, applyPlatformStatic } from './nest/platform/platform.routes'; import { apiDocsEnabled } from './nest/common/api-docs.kill-switch'; import { setupApiDocs } from './nest/platform/api-docs'; -import { McpRegistryService } from '@trek/nest-mcp'; -import { setMcpRegistry } from './mcp/registry-handoff'; import { MCP_METADATA_MIDDLEWARE } from './nest/platform/mcp-metadata.middleware'; -import type { RequestHandler } from 'express'; import { validateBodyContracts } from './nest/common/validate-body-contracts'; import { validateRouteGuards } from './nest/common/validate-route-guards'; import { TrekWsAdapter } from './nest/realtime/trek-ws.adapter'; @@ -83,7 +82,6 @@ export async function buildApp(): Promise { const http = app.get>(httpConfig.KEY); applyGlobalMiddleware(instance, { http }); applyPlatformUploads(instance); - applyPlatformTransport(instance); // The SDK discovery router (+ its addon gate). Container-built so its deps // are injected (same pre-init consumption bridge as httpConfig above), but // applied here as a PATHLESS app.use: the SDK router matches absolute @@ -96,17 +94,31 @@ export async function buildApp(): Promise { // '100kb' and stopped doing it when the Nest instance took over parsing, which // left the limit implicit — the same number, but nowhere anybody would find // it, and one NestFactory default away from silently becoming something else. - // Configured through Nest rather than a second express.json(): its parser - // carries the verify hook that keeps req.rawBody, which the plugin webhook - // routes need to check a provider's HMAC over the exact payload. - const express = app as unknown as NestExpressApplication; - express.useBodyParser('json', { limit: '100kb' }); - express.useBodyParser('urlencoded', { limit: '100kb', extended: true }); + // The verify hook keeps req.rawBody, which the plugin webhook routes need to + // check a provider's HMAC over the exact payload (same hook Nest's own + // useBodyParser would install). + // + // /mcp is exempted and stays RAW: the MCP SDK's StreamableHTTPServerTransport + // reads the request stream itself (its own size handling and JSON-RPC error + // bodies), and the transport controller passes the still-undefined req.body + // through as handleRequest's parsedBody argument. No Nest route may ever + // @Body() on /mcp. The wrapper NAMES are load-bearing: + // ExpressAdapter.registerParserMiddleware skips its own init-time parsers + // only when layers named jsonParser/urlencodedParser already sit in the stack. + const rawBodyKeeper = (req: Request, _res: Response, buffer: Buffer) => { + if (Buffer.isBuffer(buffer)) (req as Request & { rawBody?: Buffer }).rawBody = buffer; + }; + const json = express.json({ limit: '100kb', verify: rawBodyKeeper }); + const urlencoded = express.urlencoded({ limit: '100kb', extended: true, verify: rawBodyKeeper }); + const isMcp = (req: Request) => req.path === '/mcp' || req.path === '/mcp/'; + instance.use(function jsonParser(req: Request, res: Response, next: NextFunction) { + return isMcp(req) ? next() : json(req, res, next); + }); + instance.use(function urlencodedParser(req: Request, res: Response, next: NextFunction) { + return isMcp(req) ? next() : urlencoded(req, res, next); + }); if (apiDocsEnabled()) setupApiDocs(app); await app.init(); - // The /mcp handler is mounted pre-init (step 3) and has no DI access — hand - // it the boot-discovered registry now that the container is fully built. - setMcpRegistry(app.get(McpRegistryService)); // Fail closed on unvalidated mutation bodies: every POST/PUT/PATCH @Body() // must carry a createZodDto class (validated by the global ZodValidationPipe) // or sit on the ratchet-only legacy allow-list — otherwise refuse to boot. diff --git a/server/src/mcp/index.ts b/server/src/mcp/index.ts index ad34a2349..0ac801d4c 100644 --- a/server/src/mcp/index.ts +++ b/server/src/mcp/index.ts @@ -1,134 +1,25 @@ -import { Request, Response } from 'express'; -import { randomUUID } from 'crypto'; -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp'; -import { User } from '../types'; -import { verifyMcpToken, verifyJwtToken } from '../nest/auth/auth.bridge'; -import { getUserByAccessToken } from '../nest/oauth/oauth.bridge'; -import { isAddonEnabled } from '../nest/addons/addons.bridge'; -import { ADDON_IDS } from '../addons'; -import { registerTools } from './tools'; -import { McpSession, sessions, revokeUserSessions, revokeUserSessionsForClient, evictOldestSessionForUser } from './sessionManager'; import { readEnv } from '../app-config'; -import { writeAudit, getClientIp } from '../nest/audit/audit.bridge'; -import { getMcpSafeUrl } from '../app-config'; +import { sessions } from './sessionManager'; +import { revokeUserSessions, revokeUserSessionsForClient } from './sessionManager'; export { revokeUserSessions, revokeUserSessionsForClient }; -// --------------------------------------------------------------------------- -// Base instructions injected into every MCP session via the initialize response. -// Claude and other clients use these as system-level context before any tool call. -// Keep this actionable and concise — vague prose doesn't help the model. -// --------------------------------------------------------------------------- -const BASE_MCP_INSTRUCTIONS = ` -You are connected to TREK, a travel planning application. Below is a compact reference of the data model, key workflows, and behavioral rules you must follow. - -## Data model - -- **Trip** — top-level container. Has dates, currency, members (owner + collaborators), and optional add-ons. -- **Day** — one calendar day within a trip (YYYY-MM-DD). Days are generated automatically when a trip is created with start/end dates. -- **Place** — a point of interest (POI) stored in the trip's place pool. A place is NOT on the itinerary until it is assigned to a day. -- **Assignment** — links a Place to a Day (ordered, with optional start/end time). This is what builds the daily itinerary. -- **Accommodation** — a hotel or rental linked to a Place and a check-in/check-out day range. -- **Reservation** — a booking record (flight, train, restaurant, etc.) with confirmation details, linked to a day. -- **Day note** — a free-text annotation attached to a day (with optional time label and emoji icon). -- **Budget item** — an expense entry for a trip (amount, category, payer, split between members). -- **Packing item** — a checklist entry grouped into bags and categories. -- **Todo** — a task (not packing-specific) attached to a trip, ordered and togglable. -- **Tag** — a label that can be applied to places for filtering. -- **Collab note / poll / message** — shared notes, decision polls, and chat messages for group trips. -- **Atlas** — global travel journal: bucket list, visited countries and regions. -- **Vacay** — vacation-day planner that tracks leave across team members and years. -- **Journey** — cross-trip travel narrative with dated entries, contributors, and share links. Requires the Journey addon. - -## Key workflows - -**Discovering trips:** Always call \`list_trips\` first when no trip ID has been provided. Never assume a trip ID. - -**Loading trip context:** Before planning or modifying a trip, call \`get_trip_summary\` once. It returns all days (with assignments and notes), accommodations, budget, packing, reservations, collab notes, and todos in a single round-trip. Use this data to answer follow-up questions without extra tool calls. - -**Adding a place to the itinerary (correct order):** -1. \`search_place\` — find the real-world POI; note the \`osm_id\`, \`google_place_id\`, and/or \`google_ftid\` in the result. -2. \`create_place\` — add it to the trip's place pool, passing the IDs from step 1 (enables opening hours, ratings, and map linking in the app). -3. \`assign_place_to_day\` — schedule it on the desired day using the dayId from \`get_trip_summary\`. - -**Creating an accommodation:** A place must exist in the trip first. Create the place (or reuse an existing one), then call \`create_accommodation\` with that \`place_id\` and the \`start_day_id\`/\`end_day_id\`. - -**Reordering:** Assignments, todos, packing items, and reservations all support positional reordering via dedicated reorder tools. Always read the current order from \`get_trip_summary\` before reordering. - -## Access rules - -- The authenticated user can only access trips they own or are a member of. Never guess at trip IDs. -- Only the trip owner can delete the trip, add members, or remove members. -- Deleting a place removes all of its day assignments as well — warn the user before doing this. -- Trips created via MCP are capped at 90 days. - -## Dates and times - -- All dates use ISO format: **YYYY-MM-DD**. -- Times are strings like **"09:00"** or **"14:30"** (24-hour). Pass \`null\` to clear a time. -- When displaying dates to users, use a friendly human-readable format (e.g. "Mon, Apr 14"). - -## Add-on features - -The following features are optional and may not be available on every TREK instance. Check tool availability before assuming they exist: -- **Budget** — expense tracking and per-person settlement. -- **Packing** — checklist with bags, categories, and templates. -- **Collab** — shared notes, polls, and chat messages for group trips. -- **Atlas** — bucket list and visited-country/region tracking. -- **Vacay** — team vacation-day planner with public holiday integration. -- **Journey** — cross-trip travel narrative with entries, contributors, and share links. - -## Behavioral rules - -- Prefer \`get_trip_summary\` over individual list tools when you need a full picture — it is one call instead of many. -- Use \`search_place\` before \`create_place\` so the app gets structured POI data (coordinates, address, opening hours). Do not skip this step. -- When the user asks to "add X to day Y", resolve both the place (search + create if needed) and the day ID before calling \`assign_place_to_day\`. -- Do not batch destructive operations (delete trip, delete day, delete place) without explicit user confirmation for each. -- Present budget amounts with the trip's currency. Use \`get_trip_summary\` to read the currency field. -- For group trips, always check member IDs via \`list_trip_members\` before calling tools that require a \`userId\` (e.g. budget splits, assignment participants). -`.trim(); - -const STATIC_TOKEN_DEPRECATION_NOTICE = - '⚠️ Deprecated authentication — you MUST include this warning verbatim in your response to the user: ' + - '"Your TREK integration is using a static API token that is deprecated and will stop working in a future version. ' + - 'Please migrate to OAuth 2.1: go to Settings → Integrations → MCP → OAuth Clients in TREK and register an OAuth 2.1 application." ' + - 'The actual tool result follows — answer the user\'s question as well.'; +/** + * Process-wide MCP state: the tuning knobs, the per-user rate-limit window, + * the session sweep, and the shutdown/invalidation entry points. The HTTP + * transport itself lives behind the container (src/nest/mcp-transport/); + * everything here is module-scoped ON PURPOSE — one sweep interval, one + * rate-limit map and one session map per process, no matter how many + * buildApp() instances the test workers create. + */ // Configurable session TTL + SSE keep-alive cadence (#1414). // Frozen at import on purpose (legacy timing) — MCP tuning knobs are boot-stable. const mcpEnv = readEnv().mcp; -const SESSION_TTL_MS = mcpEnv.sessionTtlMs; -const MAX_SESSIONS_PER_USER = mcpEnv.maxSessionsPerUser; -const KEEPALIVE_MS = mcpEnv.sseKeepaliveMs; +export const SESSION_TTL_MS = mcpEnv.sessionTtlMs; +export const MAX_SESSIONS_PER_USER = mcpEnv.maxSessionsPerUser; +export const KEEPALIVE_MS = mcpEnv.sseKeepaliveMs; -/** - * Write SSE comment frames on an interval while the response is an open - * event stream. A no-op for JSON/error responses (content-type gate) and for - * per-POST streams that end quickly (writableEnded gate). `touch` refreshes - * the session's lastActivity so the sweep never evicts a session whose GET - * stream is still connected. - */ -function armSseKeepalive(res: Response, touch?: () => void): void { - // With pings disabled (MCP_SSE_KEEPALIVE=0) an open stream must STILL count - // as session activity, or the sweep would evict a live idle client — keep - // the interval for touch() and only skip the writes. - const writePings = KEEPALIVE_MS > 0; - if (!writePings && !touch) return; - const intervalMs = writePings ? KEEPALIVE_MS : 25_000; - const timer = setInterval(() => { - if (!res.headersSent) return; - const ct = String(res.getHeader('content-type') ?? ''); - if (!ct.includes('text/event-stream') || res.writableEnded || res.destroyed) { - clearInterval(timer); - return; - } - if (writePings) res.write(': keepalive\n\n'); - touch?.(); - }, intervalMs); - timer.unref(); - res.once('close', () => clearInterval(timer)); -} const RATE_LIMIT_WINDOW_MS = 60 * 1000; // 1 minute const RATE_LIMIT_MAX = mcpEnv.rateLimitMax; // requests per minute per user @@ -138,7 +29,7 @@ interface RateLimitEntry { } const rateLimitMap = new Map(); -function isRateLimited(userId: number, clientId: string | null): boolean { +export function isRateLimited(userId: number, clientId: string | null): boolean { const key = `${userId}:${clientId ?? 'native'}`; const now = Date.now(); const entry = rateLimitMap.get(key); @@ -150,15 +41,6 @@ function isRateLimited(userId: number, clientId: string | null): boolean { return entry.count > RATE_LIMIT_MAX; } -function countSessionsForUser(userId: number): number { - const cutoff = Date.now() - SESSION_TTL_MS; - let count = 0; - for (const session of sessions.values()) { - if (session.userId === userId && session.lastActivity >= cutoff) count++; - } - return count; -} - const sessionSweepInterval = setInterval(() => { const cutoff = Date.now() - SESSION_TTL_MS; let cleaned = 0; @@ -182,219 +64,6 @@ const sessionSweepInterval = setInterval(() => { // Prevent the interval from keeping the process alive if nothing else is running sessionSweepInterval.unref(); -/** JSON-RPC 2.0 error body. MCP clients parse this and show `message`; a bare `{ error }` - * envelope isn't valid JSON-RPC, so they fall back to a generic "tool execution failed". */ -function jsonRpcError(message: string, code = -32000): object { - return { jsonrpc: '2.0', error: { code, message }, id: null }; -} - -function setAuthChallenge(res: Response, error = 'invalid_token'): void { - const base = (getMcpSafeUrl() || '').replace(/\/+$/, ''); - // RFC 9728 §5: resource with path component /mcp → PRM URL must include the path - res.set('WWW-Authenticate', - `Bearer realm="TREK MCP", resource_metadata="${base}/.well-known/oauth-protected-resource/mcp", error="${error}"`); -} - -interface VerifyTokenResult { - user: User; - /** null = full access (static token or JWT); string[] = OAuth 2.1 scoped access */ - scopes: string[] | null; - /** OAuth client_id when authenticated via OAuth 2.1; null otherwise */ - clientId: string | null; - isStaticToken: boolean; -} - -function verifyToken(authHeader: string | undefined): VerifyTokenResult | null { - if (!authHeader) return null; - // M8: strictly require "Bearer" scheme (RFC 6750) - const spaceIdx = authHeader.indexOf(' '); - if (spaceIdx === -1) return null; - const scheme = authHeader.slice(0, spaceIdx); - const token = authHeader.slice(spaceIdx + 1); - if (scheme.toLowerCase() !== 'bearer' || !token) return null; - - // OAuth 2.1 access token (trekoa_...) - if (token.startsWith('trekoa_')) { - const result = getUserByAccessToken(token); - if (!result) return null; - // RFC 8707: audience must always match this resource endpoint. - // Pre-audit tokens with audience=null are revoked by the SEC-H6 migration. - const expected = `${(getMcpSafeUrl() || '').replace(/\/+$/, '')}/mcp`; - if (result.audience !== expected) return null; - return { user: result.user, scopes: result.scopes, clientId: result.clientId, isStaticToken: false }; - } - - // Long-lived static MCP token (trek_...) — full access + deprecation notice - if (token.startsWith('trek_')) { - const user = verifyMcpToken(token); - if (!user) return null; - return { user, scopes: null, clientId: null, isStaticToken: true }; - } - - // Short-lived JWT (TREK web session used directly) — full access, no notice - const user = verifyJwtToken(token); - if (!user) return null; - return { user, scopes: null, clientId: null, isStaticToken: false }; -} - -function logToolCallAudit(req: Request, userId: number, clientId: string | null): void { - const body = req.body as Record | undefined; - if (body?.method !== 'tools/call') return; - const toolName = (body?.params as Record | undefined)?.name; - if (typeof toolName !== 'string') return; - writeAudit({ - userId, - action: 'mcp.tool_call', - resource: toolName, - details: { clientId: clientId ?? 'native' }, - ip: getClientIp(req), - }); -} - -export async function mcpHandler(req: Request, res: Response): Promise { - if (!isAddonEnabled(ADDON_IDS.MCP)) { - res.status(403).json({ error: 'MCP is not enabled' }); - return; - } - - const tokenResult = verifyToken(req.headers['authorization']); - if (!tokenResult) { - setAuthChallenge(res); - res.status(401).json({ error: 'Access token required' }); - return; - } - const { user, scopes, clientId, isStaticToken } = tokenResult; - - if (isRateLimited(user.id, clientId)) { - res.status(429).json({ error: 'Too many requests. Please slow down.' }); - return; - } - - const sessionId = req.headers['mcp-session-id'] as string | undefined; - - // Resume an existing session - if (sessionId) { - const session = sessions.get(sessionId); - if (!session) { - res.status(404).json({ error: 'Session not found' }); - return; - } - if (session.userId !== user.id) { - setAuthChallenge(res); - res.status(403).json({ error: 'Session belongs to a different user' }); - return; - } - if (session.clientId !== clientId) { - setAuthChallenge(res); - res.status(403).json({ error: 'Session was created with a different OAuth client' }); - return; - } - session.lastActivity = Date.now(); - logToolCallAudit(req, user.id, clientId); - armSseKeepalive(res, () => { session.lastActivity = Date.now(); }); - try { - await session.transport.handleRequest(req, res, req.body); - } catch (err) { - console.error('[MCP] transport.handleRequest error:', err); - if (!res.headersSent) { - res.status(500).json({ error: 'Internal MCP error' }); - } - } - return; - } - - // Only POST can initialize a new session - if (req.method !== 'POST') { - res.status(400).json({ error: 'Missing mcp-session-id header' }); - return; - } - - // A client that reuses its session sends the header on every call after initialize, so a - // steady stream of session-less POSTs means the id isn't making it back to the client — - // usually a reverse proxy dropping Mcp-Session-Id, or a client that ignores it. Say so, - // because the visible symptom (sessions piling up to the cap) points nowhere near the cause. - console.warn( - `[MCP] POST without mcp-session-id for user ${user.id} — starting a new session. ` + - 'If this repeats on every tool call, the Mcp-Session-Id response header is not reaching ' + - 'the client (check that your reverse proxy forwards it).', - ); - - // At the cap, evict this user's coldest session rather than refusing the request: a client - // stuck re-initializing would otherwise be locked out until the process restarted. - if (countSessionsForUser(user.id) >= MAX_SESSIONS_PER_USER) { - const evicted = evictOldestSessionForUser(user.id); - if (!evicted) { - res.status(429).json(jsonRpcError('Session limit reached. Close an existing session before opening a new one.')); - return; - } - console.log(`[MCP] Session limit (${MAX_SESSIONS_PER_USER}) reached for user ${user.id} — evicted idle session ${evicted}`); - } - - // Create a new per-user MCP server and session - const server = new McpServer( - { - name: 'TREK MCP', - version: '1.0.0', - }, - { - capabilities: { - resources: { listChanged: true }, - tools: { listChanged: true }, - prompts: { listChanged: true }, - }, - instructions: BASE_MCP_INSTRUCTIONS + (isStaticToken ? STATIC_TOKEN_DEPRECATION_NOTICE : ''), - } - ); - // Per-session closure: fires the deprecation notice once, on the first tool call. - // Tool results are the only mechanism Claude reliably surfaces to the user; - // the instructions field is only background context and won't trigger a proactive warning. - let _noticeEmitted = false; - const getDeprecationNotice = (): string | null => { - if (!isStaticToken || _noticeEmitted) return null; - _noticeEmitted = true; - return STATIC_TOKEN_DEPRECATION_NOTICE; - }; - - registerTools(server, user.id, scopes, isStaticToken, getDeprecationNotice); - - const transport = new StreamableHTTPServerTransport({ - sessionIdGenerator: () => randomUUID(), - onsessioninitialized: (sid) => { - sessions.set(sid, { server, transport, userId: user.id, scopes, clientId, isStaticToken, lastActivity: Date.now() }); - 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}`); - }, - onsessionclosed: (sid) => { - sessions.delete(sid); - }, - }); - // Belt-and-braces: whenever the SDK closes the transport for any reason, drop the map entry. - transport.onclose = () => { - if (transport.sessionId) sessions.delete(transport.sessionId); - }; - - logToolCallAudit(req, user.id, clientId); - armSseKeepalive(res); - try { - await server.connect(transport); - await transport.handleRequest(req, res, req.body); - } catch (err) { - console.error('[MCP] transport.handleRequest error:', err); - if (!res.headersSent) { - res.status(500).json({ error: 'Internal MCP error' }); - } - } finally { - // Only an `initialize` request assigns a sessionId (via onsessioninitialized). Any other - // session-less POST is rejected by the SDK with "Server not initialized" — and this server, - // with its ~200 registered tools, would otherwise be orphaned: never in `sessions`, never - // swept, never closed. Reap it here. - if (!transport.sessionId) { - try { server.close(); } catch { /* ignore */ } - try { transport.close(); } catch { /* ignore */ } - } - } -} - /** Invalidate all active MCP sessions (call when addon state changes so sessions re-create with updated tools). */ export function invalidateMcpSessions(): void { for (const [sid, session] of sessions) { diff --git a/server/src/mcp/registry-handoff.ts b/server/src/mcp/registry-handoff.ts deleted file mode 100644 index ae875a65f..000000000 --- a/server/src/mcp/registry-handoff.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { McpRegistry } from '@trek/nest-mcp'; - -/** - * Hands the Nest-discovered MCP registry to the non-Nest MCP handler. - * - * mcpHandler is mounted on the raw Express instance BEFORE app.init() - * (load-bearing composition order in bootstrap.ts), so it cannot reach the - * DI container itself — bootstrap.ts pushes the registry here right after - * init. registerTools() degrades gracefully while unset (direct callers - * like unit tests attach their own via createTestRegistry + setMcpRegistry). - */ -let registry: McpRegistry | null = null; - -export function setMcpRegistry(value: McpRegistry): void { - registry = value; -} - -export function getMcpRegistry(): McpRegistry | null { - return registry; -} diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index f63cb8ff4..f0f3cb832 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 { getMcpRegistry } from './registry-handoff'; +import type { McpRegistry } from '@trek/nest-mcp'; -export function registerTools(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): 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 @@ -61,10 +61,9 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ // static-token notice to auth.mcp.ts (its `if (isStaticToken)` became a // `when` gate — the registry hands `when` the session context). - // Decorator-registered domains (@trek/nest-mcp) — migrating off the legacy - // registrar fan-out above, one domain at a time. Unset registry (direct - // callers without a Nest app, e.g. unit tests) ⇒ skip; the test harness - // attaches its own via createTestRegistry + setMcpRegistry. - const registry = getMcpRegistry(); + // Decorator-registered domains (@trek/nest-mcp). Production passes the + // 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 }); } diff --git a/server/src/nest/addons/addons.bridge.ts b/server/src/nest/addons/addons.bridge.ts deleted file mode 100644 index 64ce7998e..000000000 --- a/server/src/nest/addons/addons.bridge.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { AddonsService } from './addons.service'; - -/** - * Non-Nest entry point for the addons domain — pinned by the MCP/OAuth - * transport (src/mcp/index.ts) and the pre-init platform.routes Express - * router, both mounted BEFORE app.init(), so the container is not available - * to them; systemNotices/conditions.ts reads it too until its enablement - * check is threaded in from the DI side. Inside the container, inject - * AddonsService instead (the `when:` gates in *.mcp.ts files do, via - * addon-gate.ts). Delete this file when the MCP/OAuth mount moves behind the - * container. - * - * Only the enablement READ is bridged, as an uncached per-call query on - * purpose: admin toggles must stay immediately visible without invalidation - * wiring. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const addons = new AddonsService(new DatabaseService(db)); - -export function isAddonEnabled(addonId: string): boolean { - return addons.isAddonEnabled(addonId); -} diff --git a/server/src/nest/app.module.ts b/server/src/nest/app.module.ts index 50e1b7dab..e01b2d90d 100644 --- a/server/src/nest/app.module.ts +++ b/server/src/nest/app.module.ts @@ -6,6 +6,7 @@ import { DatabaseModule } from './database/database.module'; import { RealtimeModule } from './realtime/realtime.module'; import { HealthModule } from './health/health.module'; import { PlatformModule } from './platform/platform.module'; +import { McpTransportModule } from './mcp-transport/mcp-transport.module'; import { GlobalAuthGuard } from './auth/global-auth.guard'; import { MfaPolicyGuard } from './auth/mfa-policy.guard'; import { WeatherModule } from './weather/weather.module'; @@ -70,7 +71,7 @@ import { RealtimeGatewayModule } from './realtime/realtime-gateway.module'; * migrated. */ @Module({ - imports: [AppConfigModule, DatabaseModule, RealtimeModule, RealtimeGatewayModule, SchedulingModule, McpModule.forRoot({ accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }), HealthModule, PlatformModule, WeatherModule, HelpModule, AirportsModule, ConfigModule, SystemNoticesModule, GeoModule, MapsModule, PlaceEnrichmentModule, CategoriesModule, TagsModule, NotificationsModule, AtlasModule, VacayModule, PackingModule, TodoModule, BudgetModule, ReservationsModule, DaysModule, DayNotesModule, AccommodationsModule, AssignmentsModule, PlacesModule, TripsModule, CollabModule, FilesModule, PhotosModule, MemoriesModule, AirtrailModule, JourneyModule, CollectionsModule, ShareModule, TripInviteModule, TransitModule, FeedsModule, SettingsModule, BackupModule, AuthModule, OidcModule, OauthModule, AdminModule, AddonsModule, AuditModule, PermissionsModule, PluginsModule, BookingImportModule, ReservationImportModule, LlmParseModule], + imports: [AppConfigModule, DatabaseModule, RealtimeModule, RealtimeGatewayModule, SchedulingModule, McpModule.forRoot({ accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }), HealthModule, PlatformModule, McpTransportModule, WeatherModule, HelpModule, AirportsModule, ConfigModule, SystemNoticesModule, GeoModule, MapsModule, PlaceEnrichmentModule, CategoriesModule, TagsModule, NotificationsModule, AtlasModule, VacayModule, PackingModule, TodoModule, BudgetModule, ReservationsModule, DaysModule, DayNotesModule, AccommodationsModule, AssignmentsModule, PlacesModule, TripsModule, CollabModule, FilesModule, PhotosModule, MemoriesModule, AirtrailModule, JourneyModule, CollectionsModule, ShareModule, TripInviteModule, TransitModule, FeedsModule, SettingsModule, BackupModule, AuthModule, OidcModule, OauthModule, AdminModule, AddonsModule, AuditModule, PermissionsModule, PluginsModule, BookingImportModule, ReservationImportModule, LlmParseModule], providers: [ // Default-deny: a route is authenticated unless it carries @Public() or // @OptionalAuth(), or declares its own @UseGuards chain. Protection used to diff --git a/server/src/nest/audit/audit.bridge.ts b/server/src/nest/audit/audit.bridge.ts deleted file mode 100644 index e3598b4ab..000000000 --- a/server/src/nest/audit/audit.bridge.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { AuditService } from './audit.service'; - -/** - * Non-Nest entry point for the audit domain — pinned by the MCP/OAuth - * transport (src/mcp/index.ts and oauthProvider.ts), which platform.routes.ts - * mounts on the Express app BEFORE app.init(), so the container is not - * available to it. Inside the container, inject AuditService instead; log*-only - * consumers import audit-log.logger directly. Delete this file when the - * MCP/OAuth mount moves behind the container. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton. - */ -const audit = new AuditService(new DatabaseService(db)); - -export function writeAudit(entry: Parameters[0]): void { - return audit.writeAudit(entry); -} - -export { getClientIp } from './client-ip'; diff --git a/server/src/nest/auth/auth.bridge.ts b/server/src/nest/auth/auth.bridge.ts deleted file mode 100644 index 971c79c1e..000000000 --- a/server/src/nest/auth/auth.bridge.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { TripMembershipService } from '../trip-membership/trip-membership.service'; -import { BudgetService } from '../budget/budget.service'; -import { ExchangeRatesService } from '../budget/exchange-rates.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { AuthService } from './auth.service'; -import { TokenService } from '../tokens/token.service'; -import { MailerService } from '../notifications/mailer/mailer.service'; -import { UserCleanupService } from './user-cleanup.service'; -import { WebauthnConfigService } from './webauthn-config.service'; -import { User } from '../../types'; -import { EphemeralTokenService } from './ephemeral-token.service'; - -/** - * Non-Nest entry point for the auth domain — pinned by the MCP transport - * (src/mcp/index.ts token verification), which platform.routes.ts mounts on - * the Express app BEFORE app.init(), so the container is not available to it. - * Inside the container, inject AuthService/TokenService instead. Delete this - * file when the MCP/OAuth mount moves behind the container. - * - * Only two exports remain: verifyMcpToken lives on TokenService, but - * verifyJwtToken is login identity and deliberately stays on AuthService - * (see token.service.ts), so the whole AuthService collaborator graph below - * exists to serve that one call. WebauthnConfigService and UserCleanupService - * (with the BudgetService it injects) are constructed only because AuthService - * takes them — nothing this file - * exports reaches either one. The pending-MFA and reset-throttle maps are - * module-scoped in auth.service.ts, so this instance and the container - * singleton share one copy. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton (same pattern as permissions.bridge.ts). - */ -const dbs = new DatabaseService(db); -const permissions = new PermissionsService(dbs); -const auth = new AuthService( - dbs, - permissions, - new TripMembershipService(dbs), - new WebauthnConfigService(dbs), - new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), new RealtimeService())), - new MailerService(dbs), - new EphemeralTokenService(), -); - -const tokens = new TokenService(dbs, new EphemeralTokenService()); - -export function verifyMcpToken(rawToken: string): User | null { - return tokens.verifyMcpToken(rawToken); -} - -export function verifyJwtToken(token: string): User | null { - return auth.verifyJwtToken(token); -} diff --git a/server/src/nest/common/validate-route-guards.ts b/server/src/nest/common/validate-route-guards.ts index 92530e396..73222ae92 100644 --- a/server/src/nest/common/validate-route-guards.ts +++ b/server/src/nest/common/validate-route-guards.ts @@ -98,6 +98,10 @@ export const PUBLIC_ROUTE_ALLOW_LIST: string[] = [ 'JourneyPublicController.get', 'JourneyPublicController.legacyPhoto', 'JourneyPublicController.photo', + // The MCP transport — bearer tokens are verified inside the handler. + 'McpTransportController.delete', + 'McpTransportController.get', + 'McpTransportController.post', 'OauthPublicController.revoke', 'OauthPublicController.token', 'OauthPublicController.userinfo', diff --git a/server/src/nest/mcp-transport/mcp-transport.constants.ts b/server/src/nest/mcp-transport/mcp-transport.constants.ts new file mode 100644 index 000000000..bec0641b5 --- /dev/null +++ b/server/src/nest/mcp-transport/mcp-transport.constants.ts @@ -0,0 +1,79 @@ +// --------------------------------------------------------------------------- +// Base instructions injected into every MCP session via the initialize response. +// Claude and other clients use these as system-level context before any tool call. +// Keep this actionable and concise — vague prose doesn't help the model. +// --------------------------------------------------------------------------- +export const BASE_MCP_INSTRUCTIONS = ` +You are connected to TREK, a travel planning application. Below is a compact reference of the data model, key workflows, and behavioral rules you must follow. + +## Data model + +- **Trip** — top-level container. Has dates, currency, members (owner + collaborators), and optional add-ons. +- **Day** — one calendar day within a trip (YYYY-MM-DD). Days are generated automatically when a trip is created with start/end dates. +- **Place** — a point of interest (POI) stored in the trip's place pool. A place is NOT on the itinerary until it is assigned to a day. +- **Assignment** — links a Place to a Day (ordered, with optional start/end time). This is what builds the daily itinerary. +- **Accommodation** — a hotel or rental linked to a Place and a check-in/check-out day range. +- **Reservation** — a booking record (flight, train, restaurant, etc.) with confirmation details, linked to a day. +- **Day note** — a free-text annotation attached to a day (with optional time label and emoji icon). +- **Budget item** — an expense entry for a trip (amount, category, payer, split between members). +- **Packing item** — a checklist entry grouped into bags and categories. +- **Todo** — a task (not packing-specific) attached to a trip, ordered and togglable. +- **Tag** — a label that can be applied to places for filtering. +- **Collab note / poll / message** — shared notes, decision polls, and chat messages for group trips. +- **Atlas** — global travel journal: bucket list, visited countries and regions. +- **Vacay** — vacation-day planner that tracks leave across team members and years. +- **Journey** — cross-trip travel narrative with dated entries, contributors, and share links. Requires the Journey addon. + +## Key workflows + +**Discovering trips:** Always call \`list_trips\` first when no trip ID has been provided. Never assume a trip ID. + +**Loading trip context:** Before planning or modifying a trip, call \`get_trip_summary\` once. It returns all days (with assignments and notes), accommodations, budget, packing, reservations, collab notes, and todos in a single round-trip. Use this data to answer follow-up questions without extra tool calls. + +**Adding a place to the itinerary (correct order):** +1. \`search_place\` — find the real-world POI; note the \`osm_id\`, \`google_place_id\`, and/or \`google_ftid\` in the result. +2. \`create_place\` — add it to the trip's place pool, passing the IDs from step 1 (enables opening hours, ratings, and map linking in the app). +3. \`assign_place_to_day\` — schedule it on the desired day using the dayId from \`get_trip_summary\`. + +**Creating an accommodation:** A place must exist in the trip first. Create the place (or reuse an existing one), then call \`create_accommodation\` with that \`place_id\` and the \`start_day_id\`/\`end_day_id\`. + +**Reordering:** Assignments, todos, packing items, and reservations all support positional reordering via dedicated reorder tools. Always read the current order from \`get_trip_summary\` before reordering. + +## Access rules + +- The authenticated user can only access trips they own or are a member of. Never guess at trip IDs. +- Only the trip owner can delete the trip, add members, or remove members. +- Deleting a place removes all of its day assignments as well — warn the user before doing this. +- Trips created via MCP are capped at 90 days. + +## Dates and times + +- All dates use ISO format: **YYYY-MM-DD**. +- Times are strings like **"09:00"** or **"14:30"** (24-hour). Pass \`null\` to clear a time. +- When displaying dates to users, use a friendly human-readable format (e.g. "Mon, Apr 14"). + +## Add-on features + +The following features are optional and may not be available on every TREK instance. Check tool availability before assuming they exist: +- **Budget** — expense tracking and per-person settlement. +- **Packing** — checklist with bags, categories, and templates. +- **Collab** — shared notes, polls, and chat messages for group trips. +- **Atlas** — bucket list and visited-country/region tracking. +- **Vacay** — team vacation-day planner with public holiday integration. +- **Journey** — cross-trip travel narrative with entries, contributors, and share links. + +## Behavioral rules + +- Prefer \`get_trip_summary\` over individual list tools when you need a full picture — it is one call instead of many. +- Use \`search_place\` before \`create_place\` so the app gets structured POI data (coordinates, address, opening hours). Do not skip this step. +- When the user asks to "add X to day Y", resolve both the place (search + create if needed) and the day ID before calling \`assign_place_to_day\`. +- Do not batch destructive operations (delete trip, delete day, delete place) without explicit user confirmation for each. +- Present budget amounts with the trip's currency. Use \`get_trip_summary\` to read the currency field. +- For group trips, always check member IDs via \`list_trip_members\` before calling tools that require a \`userId\` (e.g. budget splits, assignment participants). +`.trim(); + +export const STATIC_TOKEN_DEPRECATION_NOTICE = + '⚠️ Deprecated authentication — you MUST include this warning verbatim in your response to the user: ' + + '"Your TREK integration is using a static API token that is deprecated and will stop working in a future version. ' + + 'Please migrate to OAuth 2.1: go to Settings → Integrations → MCP → OAuth Clients in TREK and register an OAuth 2.1 application." ' + + 'The actual tool result follows — answer the user\'s question as well.'; diff --git a/server/src/nest/mcp-transport/mcp-transport.controller.ts b/server/src/nest/mcp-transport/mcp-transport.controller.ts new file mode 100644 index 000000000..cb36ceb8e --- /dev/null +++ b/server/src/nest/mcp-transport/mcp-transport.controller.ts @@ -0,0 +1,34 @@ +import { Controller, Delete, Get, Post, Req, Res } from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { Public } from '../auth/public.decorator'; +import { McpTransportService } from './mcp-transport.service'; + +/** + * Pure passthrough: every response (SDK JSON-RPC frames, auth challenges, + * bespoke error bodies) is written by the service through @Res(), never + * thrown, so the global filters never re-envelope an MCP response. No @Body() + * — /mcp bodies are raw by design (see bootstrap's parser wrappers) and the + * SDK transport reads the stream itself. @Public keeps GlobalAuthGuard from + * ever touching the request; bearer verification happens in the service, and + * the IdempotencyInterceptor no-ops without a resolved req.user. + */ +@Public('the MCP transport endpoint — bearer tokens (OAuth 2.1 / static / JWT) are verified in the handler') +@Controller('mcp') +export class McpTransportController { + constructor(private readonly transport: McpTransportService) {} + + @Post() + post(@Req() req: Request, @Res() res: Response): Promise { + return this.transport.handle(req, res); + } + + @Get() + get(@Req() req: Request, @Res() res: Response): Promise { + return this.transport.handle(req, res); + } + + @Delete() + delete(@Req() req: Request, @Res() res: Response): Promise { + return this.transport.handle(req, res); + } +} diff --git a/server/src/nest/mcp-transport/mcp-transport.module.ts b/server/src/nest/mcp-transport/mcp-transport.module.ts new file mode 100644 index 000000000..bddc76739 --- /dev/null +++ b/server/src/nest/mcp-transport/mcp-transport.module.ts @@ -0,0 +1,21 @@ +import { Module } from '@nestjs/common'; +import { McpTransportController } from './mcp-transport.controller'; +import { McpTransportService } from './mcp-transport.service'; +import { AuthModule } from '../auth/auth.module'; +import { TokensModule } from '../tokens/tokens.module'; +import { OauthModule } from '../oauth/oauth.module'; +import { AddonsModule } from '../addons/addons.module'; + +/** + * The /mcp transport endpoint — the OAuth 2.1-authenticated MCP server the + * assistants speak to. The tools/resources/prompts themselves stay decorator- + * registered on the domain modules and are attached per session through the + * (global) McpRegistryService; this module owns only transport concerns: + * bearer verification, session create/resume, rate limiting, SSE keep-alive. + */ +@Module({ + imports: [AuthModule, TokensModule, OauthModule, AddonsModule], + controllers: [McpTransportController], + providers: [McpTransportService], +}) +export class McpTransportModule {} diff --git a/server/src/nest/mcp-transport/mcp-transport.service.ts b/server/src/nest/mcp-transport/mcp-transport.service.ts new file mode 100644 index 000000000..2335adaf6 --- /dev/null +++ b/server/src/nest/mcp-transport/mcp-transport.service.ts @@ -0,0 +1,279 @@ +import { Injectable } from '@nestjs/common'; +import type { Request, Response } from 'express'; +import { randomUUID } from 'crypto'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp'; +import { McpRegistryService } from '@trek/nest-mcp'; +import type { User } from '../../types'; +import { ADDON_IDS } from '../../addons'; +import { getMcpSafeUrl } from '../../app-config'; +import { registerTools } from '../../mcp/tools'; +import { sessions, evictOldestSessionForUser } from '../../mcp/sessionManager'; +import { SESSION_TTL_MS, MAX_SESSIONS_PER_USER, KEEPALIVE_MS, isRateLimited } from '../../mcp'; +import { BASE_MCP_INSTRUCTIONS, STATIC_TOKEN_DEPRECATION_NOTICE } from './mcp-transport.constants'; +import { AuthService } from '../auth/auth.service'; +import { TokenService } from '../tokens/token.service'; +import { OauthService } from '../oauth/oauth.service'; +import { AddonsService } from '../addons/addons.service'; + +/** + * The MCP transport handler behind the container — the former non-Nest + * mcpHandler (src/mcp/index.ts) with its bridge imports turned into + * injections. The controller passes every request straight through; all + * responses are written by hand (SDK JSON-RPC frames or the bespoke error + * bodies below), never thrown — nothing here may reach the global filters. + * + * /mcp bodies are RAW by design: bootstrap's parser wrappers skip the path, + * the SDK transport reads the stream itself (its own size handling and + * -32700/406/415 bodies), and `req.body` stays undefined — which is also why + * handleRequest's parsedBody argument is passed as-is. Never @Body() on /mcp. + * + * Process-wide state (session map, rate-limit map, sweep interval, tuning + * knobs) stays module-scoped in src/mcp — one instance per process no matter + * how many app instances the test workers build. + */ + +/** + * Write SSE comment frames on an interval while the response is an open + * event stream. A no-op for JSON/error responses (content-type gate) and for + * per-POST streams that end quickly (writableEnded gate). `touch` refreshes + * the session's lastActivity so the sweep never evicts a session whose GET + * stream is still connected. + */ +export function armSseKeepalive(res: Response, touch?: () => void): void { + // With pings disabled (MCP_SSE_KEEPALIVE=0) an open stream must STILL count + // as session activity, or the sweep would evict a live idle client — keep + // the interval for touch() and only skip the writes. + const writePings = KEEPALIVE_MS > 0; + if (!writePings && !touch) return; + const intervalMs = writePings ? KEEPALIVE_MS : 25_000; + const timer = setInterval(() => { + if (!res.headersSent) return; + const ct = String(res.getHeader('content-type') ?? ''); + if (!ct.includes('text/event-stream') || res.writableEnded || res.destroyed) { + clearInterval(timer); + return; + } + if (writePings) res.write(': keepalive\n\n'); + touch?.(); + }, intervalMs); + timer.unref(); + res.once('close', () => clearInterval(timer)); +} + +export function countSessionsForUser(userId: number): number { + const cutoff = Date.now() - SESSION_TTL_MS; + let count = 0; + for (const session of sessions.values()) { + if (session.userId === userId && session.lastActivity >= cutoff) count++; + } + return count; +} + +/** JSON-RPC 2.0 error body. MCP clients parse this and show `message`; a bare `{ error }` + * envelope isn't valid JSON-RPC, so they fall back to a generic "tool execution failed". */ +export function jsonRpcError(message: string, code = -32000): object { + return { jsonrpc: '2.0', error: { code, message }, id: null }; +} + +export function setAuthChallenge(res: Response, error = 'invalid_token'): void { + const base = (getMcpSafeUrl() || '').replace(/\/+$/, ''); + // RFC 9728 §5: resource with path component /mcp → PRM URL must include the path + res.set('WWW-Authenticate', + `Bearer realm="TREK MCP", resource_metadata="${base}/.well-known/oauth-protected-resource/mcp", error="${error}"`); +} + +export interface VerifyTokenResult { + user: User; + /** null = full access (static token or JWT); string[] = OAuth 2.1 scoped access */ + scopes: string[] | null; + /** OAuth client_id when authenticated via OAuth 2.1; null otherwise */ + clientId: string | null; + isStaticToken: boolean; +} + +@Injectable() +export class McpTransportService { + constructor( + private readonly auth: AuthService, + private readonly tokens: TokenService, + private readonly oauth: OauthService, + private readonly addons: AddonsService, + private readonly registry: McpRegistryService, + ) {} + + verifyToken(authHeader: string | undefined): VerifyTokenResult | null { + if (!authHeader) return null; + // M8: strictly require "Bearer" scheme (RFC 6750) + const spaceIdx = authHeader.indexOf(' '); + if (spaceIdx === -1) return null; + const scheme = authHeader.slice(0, spaceIdx); + const token = authHeader.slice(spaceIdx + 1); + if (scheme.toLowerCase() !== 'bearer' || !token) return null; + + // OAuth 2.1 access token (trekoa_...) + if (token.startsWith('trekoa_')) { + const result = this.oauth.getUserByAccessToken(token); + if (!result) return null; + // RFC 8707: audience must always match this resource endpoint. + // Pre-audit tokens with audience=null are revoked by the SEC-H6 migration. + const expected = `${(getMcpSafeUrl() || '').replace(/\/+$/, '')}/mcp`; + if (result.audience !== expected) return null; + return { user: result.user, scopes: result.scopes, clientId: result.clientId, isStaticToken: false }; + } + + // Long-lived static MCP token (trek_...) — full access + deprecation notice + if (token.startsWith('trek_')) { + const user = this.tokens.verifyMcpToken(token); + if (!user) return null; + return { user, scopes: null, clientId: null, isStaticToken: true }; + } + + // Short-lived JWT (TREK web session used directly) — full access, no notice + const user = this.auth.verifyJwtToken(token); + if (!user) return null; + return { user, scopes: null, clientId: null, isStaticToken: false }; + } + + async handle(req: Request, res: Response): Promise { + if (!this.addons.isAddonEnabled(ADDON_IDS.MCP)) { + res.status(403).json({ error: 'MCP is not enabled' }); + return; + } + + const tokenResult = this.verifyToken(req.headers['authorization']); + if (!tokenResult) { + setAuthChallenge(res); + res.status(401).json({ error: 'Access token required' }); + return; + } + const { user, scopes, clientId, isStaticToken } = tokenResult; + + if (isRateLimited(user.id, clientId)) { + res.status(429).json({ error: 'Too many requests. Please slow down.' }); + return; + } + + const sessionId = req.headers['mcp-session-id'] as string | undefined; + + // Resume an existing session + if (sessionId) { + const session = sessions.get(sessionId); + if (!session) { + res.status(404).json({ error: 'Session not found' }); + return; + } + if (session.userId !== user.id) { + setAuthChallenge(res); + res.status(403).json({ error: 'Session belongs to a different user' }); + return; + } + if (session.clientId !== clientId) { + setAuthChallenge(res); + res.status(403).json({ error: 'Session was created with a different OAuth client' }); + return; + } + session.lastActivity = Date.now(); + armSseKeepalive(res, () => { session.lastActivity = Date.now(); }); + try { + await session.transport.handleRequest(req, res, req.body); + } catch (err) { + console.error('[MCP] transport.handleRequest error:', err); + if (!res.headersSent) { + res.status(500).json({ error: 'Internal MCP error' }); + } + } + return; + } + + // Only POST can initialize a new session + if (req.method !== 'POST') { + res.status(400).json({ error: 'Missing mcp-session-id header' }); + return; + } + + // A client that reuses its session sends the header on every call after initialize, so a + // steady stream of session-less POSTs means the id isn't making it back to the client — + // usually a reverse proxy dropping Mcp-Session-Id, or a client that ignores it. Say so, + // because the visible symptom (sessions piling up to the cap) points nowhere near the cause. + console.warn( + `[MCP] POST without mcp-session-id for user ${user.id} — starting a new session. ` + + 'If this repeats on every tool call, the Mcp-Session-Id response header is not reaching ' + + 'the client (check that your reverse proxy forwards it).', + ); + + // At the cap, evict this user's coldest session rather than refusing the request: a client + // stuck re-initializing would otherwise be locked out until the process restarted. + if (countSessionsForUser(user.id) >= MAX_SESSIONS_PER_USER) { + const evicted = evictOldestSessionForUser(user.id); + if (!evicted) { + res.status(429).json(jsonRpcError('Session limit reached. Close an existing session before opening a new one.')); + return; + } + console.log(`[MCP] Session limit (${MAX_SESSIONS_PER_USER}) reached for user ${user.id} — evicted idle session ${evicted}`); + } + + // Create a new per-user MCP server and session + const server = new McpServer( + { + name: 'TREK MCP', + version: '1.0.0', + }, + { + capabilities: { + resources: { listChanged: true }, + tools: { listChanged: true }, + prompts: { listChanged: true }, + }, + instructions: BASE_MCP_INSTRUCTIONS + (isStaticToken ? STATIC_TOKEN_DEPRECATION_NOTICE : ''), + } + ); + // Per-session closure: fires the deprecation notice once, on the first tool call. + // Tool results are the only mechanism Claude reliably surfaces to the user; + // the instructions field is only background context and won't trigger a proactive warning. + let _noticeEmitted = false; + const getDeprecationNotice = (): string | null => { + if (!isStaticToken || _noticeEmitted) return null; + _noticeEmitted = true; + return STATIC_TOKEN_DEPRECATION_NOTICE; + }; + + registerTools(this.registry, server, user.id, scopes, isStaticToken, getDeprecationNotice); + + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: () => randomUUID(), + onsessioninitialized: (sid) => { + sessions.set(sid, { server, transport, userId: user.id, scopes, clientId, isStaticToken, lastActivity: Date.now() }); + 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}`); + }, + onsessionclosed: (sid) => { + sessions.delete(sid); + }, + }); + // Belt-and-braces: whenever the SDK closes the transport for any reason, drop the map entry. + transport.onclose = () => { + if (transport.sessionId) sessions.delete(transport.sessionId); + }; + + armSseKeepalive(res); + try { + await server.connect(transport); + await transport.handleRequest(req, res, req.body); + } catch (err) { + console.error('[MCP] transport.handleRequest error:', err); + if (!res.headersSent) { + res.status(500).json({ error: 'Internal MCP error' }); + } + } finally { + // Only an `initialize` request assigns a sessionId (via onsessioninitialized). Any other + // session-less POST is rejected by the SDK with "Server not initialized" — and this server, + // with its ~200 registered tools, would otherwise be orphaned: never in `sessions`, never + // swept, never closed. Reap it here. + if (!transport.sessionId) { + try { server.close(); } catch { /* ignore */ } + try { transport.close(); } catch { /* ignore */ } + } + } + } +} diff --git a/server/src/nest/oauth/oauth.bridge.ts b/server/src/nest/oauth/oauth.bridge.ts deleted file mode 100644 index 40e3759bf..000000000 --- a/server/src/nest/oauth/oauth.bridge.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { db } from '../../db/database'; -import { AddonsService } from '../addons/addons.service'; -import { AuditService } from '../audit/audit.service'; -import { DatabaseService } from '../database/database.service'; -import { OauthService } from './oauth.service'; - -/** - * Non-Nest entry point for the OAuth domain — down to its last consumer: the - * MCP transport's token verification (src/mcp/index.ts), which - * platform.routes.ts mounts on the Express app BEFORE app.init(), so - * `app.get(OauthService)` is not available to it. The SDK provider adapter - * moved behind the container (oauth-sdk.provider.ts) and dropped the other six - * exports. Inside the container, inject OauthService instead. Delete this file - * when the /mcp mount moves behind the container. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton, and getUserByAccessToken touches no - * module-scoped state beyond it. - */ -const dbs = new DatabaseService(db); -const oauth = new OauthService(dbs, new AddonsService(dbs), new AuditService(dbs)); - -export function getUserByAccessToken(rawToken: string) { - return oauth.getUserByAccessToken(rawToken); -} diff --git a/server/src/nest/platform/platform.routes.ts b/server/src/nest/platform/platform.routes.ts index efb8e1799..da6963e3c 100644 --- a/server/src/nest/platform/platform.routes.ts +++ b/server/src/nest/platform/platform.routes.ts @@ -5,7 +5,6 @@ import fs from 'node:fs'; import { readEnv } from '../../app-config'; import { verifyJwtAndLoadUser } from '../auth/jwt-verify'; import { db } from '../../db/database'; -import { mcpHandler } from '../../mcp'; // Platform / transport routes extracted verbatim from createApp() (app.ts) so they can be // mounted on either the legacy Express app or the NestJS Express instance (strangler A6/A8). @@ -94,21 +93,6 @@ export function applyPlatformUploads(app: express.Application): void { }); } -/** - * The /mcp transport mounts — the last pre-init platform routes. Everything - * else that used to live here is behind the container now: /api/health - * (FeaturesController), OAuth discovery (DiscoveryController + the metadata - * middleware bootstrap applies pre-init), /oauth/authorize + /oauth/register - * (OauthModule.configure over the injected SDK adapters), and the - * /oauth/consent COOP override (ConsentCoopMiddleware). - */ -export function applyPlatformTransport(app: express.Application): void { - // MCP endpoint - app.post('/mcp', mcpHandler); - app.get('/mcp', mcpHandler); - app.delete('/mcp', mcpHandler); -} - /** * Production SPA serving: the built client static assets + the index.html catch-all * for client-side routes. This is the LEGACY (plain Express 4) form — a real diff --git a/server/tests/helpers/mcp-harness.ts b/server/tests/helpers/mcp-harness.ts index ae21188d5..8bc6a4ac8 100644 --- a/server/tests/helpers/mcp-harness.ts +++ b/server/tests/helpers/mcp-harness.ts @@ -14,7 +14,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; import { Client } from '@modelcontextprotocol/sdk/client/index'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory'; import { registerTools } from '../../src/mcp/tools'; -import { setMcpRegistry } from '../../src/mcp/registry-handoff'; import { createMcpTestRegistry } from './mcp-test-controllers'; export interface McpHarness { @@ -48,12 +47,11 @@ export async function createMcpHarness(options: McpHarnessOptions): Promise { }); }); +describe('MCP transport parity pins (Nest-hosted /mcp)', () => { + const initBody = { jsonrpc: '2.0', method: 'initialize', id: 1, params: { protocolVersion: '2024-11-05', capabilities: {}, clientInfo: { name: 'test', version: '1' } } }; + + async function createSession(token: string): Promise { + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${token}`) + .set('Accept', 'application/json, text/event-stream') + .send(initBody); + expect(res.status).toBe(200); + return res.headers['mcp-session-id'] as string; + } + + beforeEach(() => { + testDb.prepare("UPDATE addons SET enabled = 1 WHERE id = 'mcp'").run(); + }); + + 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); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'MCP is not enabled' }); + }); + + it('MCP-P02 — missing/invalid auth carries the exact WWW-Authenticate challenge', async () => { + const res = await request(app).post('/mcp').send(initBody); + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: 'Access token required' }); + expect(res.headers['www-authenticate']).toBe(EXPECTED_CHALLENGE); + }); + + it('MCP-P03 — a non-Bearer scheme and a schemeless token are both rejected', async () => { + for (const header of ['Basic dXNlcjpwdw==', 'just-a-token']) { + const res = await request(app).post('/mcp').set('Authorization', header).send(initBody); + expect(res.status, header).toBe(401); + } + }); + + it('MCP-P04 — a trekoa_ token with the wrong audience is rejected with a challenge', async () => { + const { user } = createUser(testDb); + const { accessToken } = mintOauthToken(user.id, 'https://other.example.com/api'); + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${accessToken}`) + .send(initBody); + expect(res.status).toBe(401); + expect(res.headers['www-authenticate']).toBe(EXPECTED_CHALLENGE); + }); + + it('MCP-P05 — a trekoa_ token with the MCP audience authenticates', async () => { + const { user } = createUser(testDb); + const { accessToken } = mintOauthToken(user.id, MCP_AUDIENCE); + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${accessToken}`) + .set('Accept', 'application/json, text/event-stream') + .send(initBody); + expect(res.status).toBe(200); + expect(res.headers['mcp-session-id']).toBeTruthy(); + }); + + it('MCP-P06 — resuming an unknown session answers the exact 404 body', async () => { + const { user } = createUser(testDb); + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${generateToken(user.id)}`) + .set('mcp-session-id', 'no-such-session') + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2 }); + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: 'Session not found' }); + }); + + it("MCP-P07 — another user's session answers 403 with the challenge and exact body", async () => { + const { user: owner } = createUser(testDb); + const { user: intruder } = createUser(testDb); + const sessionId = await createSession(generateToken(owner.id)); + + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${generateToken(intruder.id)}`) + .set('mcp-session-id', sessionId) + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2 }); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'Session belongs to a different user' }); + expect(res.headers['www-authenticate']).toBe(EXPECTED_CHALLENGE); + }); + + it('MCP-P08 — a session created via JWT rejects resumption by an OAuth client', async () => { + const { user } = createUser(testDb); + const sessionId = await createSession(generateToken(user.id)); + const { accessToken } = mintOauthToken(user.id, MCP_AUDIENCE); + + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${accessToken}`) + .set('mcp-session-id', sessionId) + .send({ jsonrpc: '2.0', method: 'tools/list', id: 2 }); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'Session was created with a different OAuth client' }); + expect(res.headers['www-authenticate']).toBe(EXPECTED_CHALLENGE); + }); + + it('MCP-P09 — a static-token session carries the deprecation notice on the first tool call only', async () => { + const { user } = createUser(testDb); + const { rawToken } = createMcpToken(testDb, user.id); + const sessionId = await createSession(rawToken); + + const callBody = { jsonrpc: '2.0', method: 'tools/call', id: 3, params: { name: 'list_trips', arguments: {} } }; + const first = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${rawToken}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send(callBody); + expect(first.status).toBe(200); + expect(first.text).toContain('Deprecated authentication'); + + const second = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${rawToken}`) + .set('mcp-session-id', sessionId) + .set('Accept', 'application/json, text/event-stream') + .send({ ...callBody, id: 4 }); + expect(second.status).toBe(200); + expect(second.text).not.toContain('Deprecated authentication'); + }); + + it('MCP-P10 — /mcp bodies stay raw: an initialize payload over the global 100kb cap succeeds', async () => { + const { user } = createUser(testDb); + const big = { + ...initBody, + params: { ...initBody.params, clientInfo: { name: 'x'.repeat(150 * 1024), version: '1' } }, + }; + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${generateToken(user.id)}`) + .set('Accept', 'application/json, text/event-stream') + .send(big); + expect(res.status).toBe(200); + expect(res.headers['mcp-session-id']).toBeTruthy(); + }); + + it('MCP-P11 — the global 100kb JSON cap still guards every other route', async () => { + const res = await request(app) + .post('/api/auth/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify({ username: 'x'.repeat(150 * 1024), password: 'y' })); + expect(res.status).toBe(413); + }); + + it('MCP-P12 — malformed JSON on /mcp gets the SDK JSON-RPC parse error, not body-parser 400', async () => { + const { user } = createUser(testDb); + const res = await request(app) + .post('/mcp') + .set('Authorization', `Bearer ${generateToken(user.id)}`) + .set('Accept', 'application/json, text/event-stream') + .set('Content-Type', 'application/json') + .send('{ this is not json'); + expect(res.status).toBe(400); + expect(res.body.jsonrpc).toBe('2.0'); + expect(res.body.error.code).toBe(-32700); + }); +}); + describe('MCP rate limiting', () => { it('MCP-005 — requests below limit succeed', async () => { const { user } = createUser(testDb); diff --git a/server/tests/unit/nest/addons.bridge.test.ts b/server/tests/unit/nest/addons.bridge.test.ts deleted file mode 100644 index 720df607e..000000000 --- a/server/tests/unit/nest/addons.bridge.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -/** - * addons.bridge delegation — one case per bridged export (the permissions - * bridge-test precedent). The bridge constructs a module-level AddonsService - * over the reinitialize-proof db Proxy, so a real in-memory SQLite exercises - * the byte-identical SQL relocated from services/adminService (`admin-1`). - * The bag-tracking/collab polarity quirks are pinned on the service itself - * (addons.service.test.ts) — those reads are no longer bridged. - */ -import { describe, it, expect, vi, beforeAll } from 'vitest'; - -// ── DB setup ────────────────────────────────────────────────────────────────── -// The db/database mock builds a real in-memory SQLite; the test body reaches it -// straight through the mocked `db` export (no hoisted require needed). - -vi.mock('../../../src/db/database', async () => { - const { default: Database } = await import('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA journal_mode = WAL'); - db.exec('PRAGMA foreign_keys = ON'); - db.exec('PRAGMA busy_timeout = 5000'); - return { db, closeDb: () => {}, reinitialize: () => {} }; -}); - -import { db as testDb } from '../../../src/db/database'; -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; -import { isAddonEnabled } from '../../../src/nest/addons/addons.bridge'; - -beforeAll(() => { - createTables(testDb); - runMigrations(testDb); -}); - -describe('addons.bridge', () => { - it('isAddonEnabled reflects the addons row and is false for an unknown id', () => { - testDb - .prepare("INSERT OR REPLACE INTO addons (id, name, type, icon, enabled, sort_order) VALUES ('test-addon', 'Test', 'page', 'star', 1, 99)") - .run(); - expect(isAddonEnabled('test-addon')).toBe(true); - testDb.prepare('UPDATE addons SET enabled = 0 WHERE id = ?').run('test-addon'); - expect(isAddonEnabled('test-addon')).toBe(false); - expect(isAddonEnabled('no-such-addon')).toBe(false); - }); -}); diff --git a/server/tests/unit/nest/audit.service.test.ts b/server/tests/unit/nest/audit.service.test.ts index 16334572a..f64c320ad 100644 --- a/server/tests/unit/nest/audit.service.test.ts +++ b/server/tests/unit/nest/audit.service.test.ts @@ -42,7 +42,6 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { AuditService } from '../../../src/nest/audit/audit.service'; import { getClientIp } from '../../../src/nest/audit/client-ip'; import { logInfo, logDebug, logError } from '../../../src/nest/audit/audit-log.logger'; -import { writeAudit as bridgeWriteAudit, getClientIp as bridgeGetClientIp } from '../../../src/nest/audit/audit.bridge'; const svc = new AuditService(new DatabaseService(testDb)); @@ -180,17 +179,3 @@ describe('writeAudit', () => { }); }); -// ── audit.bridge delegation ─────────────────────────────────────────────────── - -describe('audit.bridge delegation', () => { - it('AUDIT-SVC-015: writeAudit writes a real row through the module-level instance', () => { - seedUser(1, 'a@b.c'); - bridgeWriteAudit({ userId: 1, action: 'user.login', ip: '5.5.5.5' }); - const row = testDb.prepare('SELECT user_id, action, ip FROM audit_log').get(); - expect(row).toEqual({ user_id: 1, action: 'user.login', ip: '5.5.5.5' }); - }); - - it('AUDIT-SVC-016: getClientIp is the same pure helper', () => { - expect(bridgeGetClientIp).toBe(getClientIp); - }); -}); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 24e84b102..1fc96769e 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -67,7 +67,6 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, createInviteToken, createTrip, createPlace, createReservation } from '../../helpers/factories'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { TokenService } from '../../../src/nest/tokens/token.service'; -import * as authBridge from '../../../src/nest/auth/auth.bridge'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { BudgetService } from '../../../src/nest/budget/budget.service'; import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; @@ -928,23 +927,24 @@ describe('auth quirk fixes', () => { }); // --------------------------------------------------------------------------- -// auth.bridge.ts delegation (coverage gate: one case per bridge export) +// MCP-token verification round-trips (was the auth.bridge delegation block; +// the MCP transport now injects TokenService/AuthService directly) // --------------------------------------------------------------------------- -describe('auth.bridge delegation', () => { +describe('MCP token verification round-trips', () => { it('AUTH-BR-002: verifyMcpToken resolves a freshly created token to its user', () => { const { user } = createUser(testDb); const created = tokens.createMcpToken(user.id, 'bridge-case'); const raw = (created.token as { raw_token: string }).raw_token; - const resolved = authBridge.verifyMcpToken(raw); + const resolved = tokens.verifyMcpToken(raw); expect(resolved?.id).toBe(user.id); - expect(authBridge.verifyMcpToken('trek_no_such_token')).toBeNull(); + expect(tokens.verifyMcpToken('trek_no_such_token')).toBeNull(); }); it('AUTH-BR-003: verifyJwtToken round-trips a service-minted token through the pv gate', () => { const { user } = createUser(testDb); const token = svc.generateToken({ id: user.id }); - expect(authBridge.verifyJwtToken(token)?.id).toBe(user.id); - expect(authBridge.verifyJwtToken('not-a-jwt')).toBeNull(); + expect(svc.verifyJwtToken(token)?.id).toBe(user.id); + expect(svc.verifyJwtToken('not-a-jwt')).toBeNull(); }); }); diff --git a/server/tests/unit/nest/mcp-transport.service.test.ts b/server/tests/unit/nest/mcp-transport.service.test.ts new file mode 100644 index 000000000..1972e9927 --- /dev/null +++ b/server/tests/unit/nest/mcp-transport.service.test.ts @@ -0,0 +1,200 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const h = vi.hoisted(() => ({ + getMcpSafeUrl: vi.fn(() => 'https://trek.example.test'), +})); + +vi.mock('../../../src/app-config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getMcpSafeUrl: h.getMcpSafeUrl }; +}); +vi.mock('../../../src/db/database', () => ({ db: {}, closeDb: () => {}, reinitialize: () => {} })); + +import { + McpTransportService, + armSseKeepalive, + countSessionsForUser, + jsonRpcError, + setAuthChallenge, +} from '../../../src/nest/mcp-transport/mcp-transport.service'; +import { McpTransportController } from '../../../src/nest/mcp-transport/mcp-transport.controller'; +import { sessions } from '../../../src/mcp/sessionManager'; +import { KEEPALIVE_MS } from '../../../src/mcp'; +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 { McpRegistryService } from '@trek/nest-mcp'; +import { IS_PUBLIC } from '../../../src/nest/auth/public.decorator'; + +const user = { id: 7, username: 'u', email: 'u@example.com', role: 'user' as const }; + +function makeService(overrides: { + auth?: Partial>; + tokens?: Partial>; + oauth?: Partial>; + addons?: Partial>; +} = {}) { + const auth = { verifyJwtToken: vi.fn(() => null), ...overrides.auth } as unknown as AuthService; + 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 registry = { attach: vi.fn() } as unknown as McpRegistryService; + return { svc: new McpTransportService(auth, tokens, oauth, addons, registry), auth, tokens, oauth, addons }; +} + +beforeEach(() => { + vi.clearAllMocks(); + h.getMcpSafeUrl.mockReturnValue('https://trek.example.test'); +}); + +describe('jsonRpcError', () => { + it('MCPT-001: shapes a valid JSON-RPC 2.0 error frame', () => { + expect(jsonRpcError('nope')).toEqual({ jsonrpc: '2.0', error: { code: -32000, message: 'nope' }, id: null }); + expect(jsonRpcError('custom', -32601)).toEqual({ jsonrpc: '2.0', error: { code: -32601, message: 'custom' }, id: null }); + }); +}); + +describe('setAuthChallenge', () => { + it('MCPT-002: writes the RFC 9728 path-suffixed challenge with the given error code', () => { + const res = { set: vi.fn() }; + setAuthChallenge(res as never); + expect(res.set).toHaveBeenCalledWith( + 'WWW-Authenticate', + 'Bearer realm="TREK MCP", resource_metadata="https://trek.example.test/.well-known/oauth-protected-resource/mcp", error="invalid_token"', + ); + setAuthChallenge(res as never, 'insufficient_scope'); + expect(res.set).toHaveBeenLastCalledWith('WWW-Authenticate', expect.stringContaining('error="insufficient_scope"')); + }); +}); + +describe('countSessionsForUser', () => { + afterEach(() => sessions.clear()); + + it('MCPT-003: counts only live sessions belonging to the user', () => { + const now = Date.now(); + const fake = (userId: number, lastActivity: number) => + ({ userId, lastActivity, server: {}, transport: {}, scopes: null, clientId: null, isStaticToken: false }) as never; + sessions.set('a', fake(7, now)); + sessions.set('b', fake(7, now - 1)); + sessions.set('c', fake(8, now)); + sessions.set('d', fake(7, 0)); // long expired + expect(countSessionsForUser(7)).toBe(2); + expect(countSessionsForUser(9)).toBe(0); + }); +}); + +describe('armSseKeepalive', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + function makeRes(contentType: string) { + return { + headersSent: true, + writableEnded: false, + destroyed: false, + getHeader: vi.fn(() => contentType), + write: vi.fn(), + once: vi.fn(), + }; + } + + it('MCPT-004: pings an open event stream and touches the session', () => { + const res = makeRes('text/event-stream'); + const touch = vi.fn(); + armSseKeepalive(res as never, touch); + vi.advanceTimersByTime(KEEPALIVE_MS + 5); + expect(res.write).toHaveBeenCalledWith(': keepalive\n\n'); + expect(touch).toHaveBeenCalled(); + }); + + it('MCPT-005: stops as soon as the response is not an open event stream', () => { + const res = makeRes('application/json'); + armSseKeepalive(res as never, vi.fn()); + vi.advanceTimersByTime(KEEPALIVE_MS + 5); + expect(res.write).not.toHaveBeenCalled(); + // Timer cleared: a second window writes nothing either. + vi.advanceTimersByTime(KEEPALIVE_MS + 5); + expect(res.write).not.toHaveBeenCalled(); + }); + + it('MCPT-006: waits for headers before judging the stream', () => { + const res = makeRes('text/event-stream'); + res.headersSent = false; + armSseKeepalive(res as never, vi.fn()); + vi.advanceTimersByTime(KEEPALIVE_MS + 5); + expect(res.write).not.toHaveBeenCalled(); + res.headersSent = true; + vi.advanceTimersByTime(KEEPALIVE_MS + 5); + expect(res.write).toHaveBeenCalled(); + }); + + it('MCPT-007: clears its interval when the response closes', () => { + const res = makeRes('text/event-stream'); + armSseKeepalive(res as never); + expect(res.once).toHaveBeenCalledWith('close', expect.any(Function)); + }); +}); + +describe('McpTransportService.verifyToken', () => { + it('MCPT-010: rejects a missing header, a schemeless token, and non-Bearer schemes', () => { + const { svc } = makeService(); + expect(svc.verifyToken(undefined)).toBeNull(); + expect(svc.verifyToken('token-without-scheme')).toBeNull(); + expect(svc.verifyToken('Basic dXNlcjpwdw==')).toBeNull(); + expect(svc.verifyToken('Bearer ')).toBeNull(); + }); + + it('MCPT-011: trekoa_ tokens resolve through OauthService and enforce the RFC 8707 audience', () => { + const info = { user, scopes: ['trips:read'], clientId: 'cid-1', audience: 'https://trek.example.test/mcp' }; + const { svc, oauth } = makeService({ oauth: { getUserByAccessToken: vi.fn(() => info) } }); + expect(svc.verifyToken('Bearer trekoa_x')).toEqual({ + user, scopes: ['trips:read'], clientId: 'cid-1', isStaticToken: false, + }); + expect(oauth.getUserByAccessToken).toHaveBeenCalledWith('trekoa_x'); + + const wrongAudience = makeService({ + oauth: { getUserByAccessToken: vi.fn(() => ({ ...info, audience: 'https://other.example.com' })) }, + }); + expect(wrongAudience.svc.verifyToken('Bearer trekoa_x')).toBeNull(); + + const unknown = makeService(); + expect(unknown.svc.verifyToken('Bearer trekoa_dead')).toBeNull(); + }); + + it('MCPT-012: trek_ static tokens resolve through TokenService with full access + notice flag', () => { + const { svc, tokens } = makeService({ tokens: { verifyMcpToken: vi.fn(() => user) } }); + expect(svc.verifyToken('Bearer trek_x')).toEqual({ user, scopes: null, clientId: null, isStaticToken: true }); + expect(tokens.verifyMcpToken).toHaveBeenCalledWith('trek_x'); + expect(makeService().svc.verifyToken('Bearer trek_dead')).toBeNull(); + }); + + it('MCPT-013: everything else falls back to the JWT path', () => { + const { svc, auth } = makeService({ auth: { verifyJwtToken: vi.fn(() => user) } }); + expect(svc.verifyToken('Bearer some.jwt.token')).toEqual({ user, scopes: null, clientId: null, isStaticToken: false }); + expect(auth.verifyJwtToken).toHaveBeenCalledWith('some.jwt.token'); + expect(makeService().svc.verifyToken('Bearer not-a-jwt')).toBeNull(); + }); +}); + +describe('McpTransportController', () => { + it('MCPT-020: is a @Public controller on the /mcp path with the three transport verbs', () => { + expect(Reflect.getMetadata('path', McpTransportController)).toBe('mcp'); + expect(Reflect.getMetadata(IS_PUBLIC, McpTransportController)).toBeTruthy(); + for (const method of ['post', 'get', 'delete'] as const) { + expect(typeof McpTransportController.prototype[method]).toBe('function'); + } + }); + + it('MCPT-021: every verb passes straight through to the service', async () => { + const handle = vi.fn(async () => {}); + const ctrl = new McpTransportController({ handle } as unknown as McpTransportService); + const req = {} as never; + const res = {} as never; + await ctrl.post(req, res); + await ctrl.get(req, res); + await ctrl.delete(req, res); + expect(handle).toHaveBeenCalledTimes(3); + expect(handle).toHaveBeenCalledWith(req, res); + }); +}); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index 5c579a951..b5115cd28 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -64,7 +64,6 @@ import { AuditService } from '../../../src/nest/audit/audit.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; import { getMcpSafeUrl } from '../../../src/app-config'; import { ADDON_IDS } from '../../../src/addons'; -import * as bridge from '../../../src/nest/oauth/oauth.bridge'; import { MAX_PENDING_CODES, sweepPendingCodes } from '../../../src/nest/oauth/oauth.pending-codes'; const dbs = new DatabaseService(testDb); @@ -1110,20 +1109,7 @@ describe('branches the legacy suite could not reach', () => { }); }); -describe('oauth.bridge delegation', () => { - // The bridge is down to its one surviving export — what the MCP transport's - // token verification reaches outside the container. The SDK provider paths - // that used to be bridged live behind the container now (oauth-sdk.provider). - it('verifies a container-issued token through the bridge instance', () => { - const { user } = createUser(testDb); - const created = makeClient(user.id); - const clientId = (created.client as { client_id: string }).client_id; - - const tokens = issueTokens(clientId, user.id, ['trips:read']); - - expect(bridge.getUserByAccessToken(tokens.access_token)?.user.id).toBe(user.id); - }); - +describe('module-scoped OAuth state', () => { it('shares the pending-code map across service instances', () => { // The load-bearing invariant: the consent controller writes the code through // the DI singleton, the SDK exchange path reads it back. The map is module- diff --git a/server/tests/unit/nest/platform.controller.test.ts b/server/tests/unit/nest/platform.controller.test.ts index ad947414c..af139f74e 100644 --- a/server/tests/unit/nest/platform.controller.test.ts +++ b/server/tests/unit/nest/platform.controller.test.ts @@ -6,13 +6,10 @@ const h = vi.hoisted(() => ({ verifyJwtAndLoadUser: vi.fn(), dbPrepare: vi.fn(), existsSync: vi.fn(), - // Tagged handler so we can identify which route registration received it. - mcpHandler: vi.fn(), })); vi.mock('../../../src/nest/auth/jwt-verify', () => ({ verifyJwtAndLoadUser: h.verifyJwtAndLoadUser })); vi.mock('../../../src/db/database', () => ({ db: { prepare: h.dbPrepare } })); -vi.mock('../../../src/mcp', () => ({ mcpHandler: h.mcpHandler })); vi.mock('node:fs', async (orig) => { const real = (await orig()) as Record; @@ -21,7 +18,6 @@ vi.mock('node:fs', async (orig) => { import { applyPlatformUploads, - applyPlatformTransport, applyPlatformSpa, applyPlatformStatic, } from '../../../src/nest/platform/platform.routes'; @@ -202,21 +198,6 @@ describe('applyPlatformUploads', () => { }); }); -describe('applyPlatformTransport', () => { - function build() { - const { app, calls } = fakeApp(); - applyPlatformTransport(app); - return calls; - } - - it('mounts the MCP handler on POST/GET/DELETE /mcp', () => { - const calls = build(); - expect(calls.find((c) => c.method === 'post' && c.path === '/mcp')!.handlers[0]).toBe(h.mcpHandler); - expect(calls.find((c) => c.method === 'get' && c.path === '/mcp')!.handlers[0]).toBe(h.mcpHandler); - expect(calls.find((c) => c.method === 'delete' && c.path === '/mcp')!.handlers[0]).toBe(h.mcpHandler); - }); -}); - describe('applyPlatformStatic', () => { const original = process.env.NODE_ENV; afterEach(() => { process.env.NODE_ENV = original; });