From 755da90df7c1b113ca313dc9bef4cf39409653dd Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:07:31 -0400 Subject: [PATCH 01/10] feat(server): host-aware serve() for one-process multi-host deployments serve()'s publicUrl and protectedResource now accept (host) => ... functions, and ServeContext carries the resolved host so one process can front many hostnames (white-label publishers, multi-brand adapters) without re-owning the HTTP plumbing. Per-host resolver results are cached. trustForwardedHost opts into X-Forwarded-Host for deployments behind a sanitizing proxy. verifyBearer({ audience }) accepts (req) => string so the JWT audience check follows the per-host publicUrl -- a token minted for snap.example.com fails audience validation when presented to meta.example.com. Closes #885. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/multihost-serve.md | 18 ++ skills/build-seller-agent/SKILL.md | 73 ++--- src/lib/server/auth.ts | 30 ++- src/lib/server/serve.ts | 241 ++++++++++++++--- src/lib/testing/local-agent-runner.ts | 5 +- test/lib/serve-multihost.test.js | 366 ++++++++++++++++++++++++++ test/server-auth.test.js | 46 ++++ 7 files changed, 698 insertions(+), 81 deletions(-) create mode 100644 .changeset/multihost-serve.md create mode 100644 test/lib/serve-multihost.test.js diff --git a/.changeset/multihost-serve.md b/.changeset/multihost-serve.md new file mode 100644 index 000000000..980d3e40e --- /dev/null +++ b/.changeset/multihost-serve.md @@ -0,0 +1,18 @@ +--- +'@adcp/client': minor +--- + +feat(server): host-aware `serve()` for one-process multi-host deployments + +`ServeOptions.publicUrl` and `protectedResource` now accept a `(host) => …` +function, and the factory's `ServeContext` carries the resolved `host` so one +process can front many hostnames (white-label sellers, multi-brand adapters) +without re-owning the HTTP plumbing. Set `trustForwardedHost: true` when +`serve()` sits behind a proxy that sanitizes `X-Forwarded-Host`. Per-host +resolver results are cached. Static `publicUrl: string` is unchanged. + +`verifyBearer({ audience })` now also accepts `(req) => string` so the JWT +audience check follows the per-host `publicUrl` — a token minted for +`snap.example.com` fails audience validation when presented to `meta.example.com`. + +Closes #885. diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index 463e816e9..7abd91bf1 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -1131,52 +1131,55 @@ The quick-start uses `memoryBackend()` + `InMemoryStateStore` — both reset on Auth is not wired in the example — see [§ Protecting your agent](#protecting-your-agent) below. -## Alternative Transports +## Multi-Host and Alternative Transports -`serve()` is the supported path for HTTP-hosted seller agents: one process, one `publicUrl`, one MCP mount. Two setups need something else: - -- **Multi-host routing on a single process.** Fronting many hostnames (white-label publishers, multi-brand operators) from one Fly machine or pod — one `publicUrl` per incoming host, resolved from `Host` / `Forwarded`. -- **Stdio transport.** Running as a local subprocess of a buyer agent (CLI tools, desktop clients) instead of an HTTP server. - -The escape hatch for both is the same: `createAdcpServer()` returns a handle with a `connect(transport)` method. Skip `serve()` and drive the transport yourself. +`serve()` supports two shapes of deployment out of the box: single-host (the quickstart default) and multi-host (one process fronting many hostnames). Stdio is the only setup that needs a different entry point. ### Multi-host HTTP +Pass functions for `publicUrl` and `protectedResource`, branch on `ctx.host` in the factory, and turn on `trustForwardedHost` when a proxy terminates TLS: + ```typescript -import { createServer } from 'node:http'; -import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; -import { createAdcpServer } from '@adcp/client/server'; +import { serve, createAdcpServer, verifyBearer } from '@adcp/client'; -// One AdcpServer per host — build lazily and cache. -const servers = new Map>(); -function getServer(host: string) { - let s = servers.get(host); - if (!s) { - s = createAdcpServer({ - name: `Seller for ${host}`, +// Host → adapter config. Whatever shape suits your deployment (DB, env, static). +const adapters = new Map([ + ['snap.agentic-adapters.scope3.com', { name: 'Snap seller', handlers: snapHandlers }], + ['meta.agentic-adapters.scope3.com', { name: 'Meta seller', handlers: metaHandlers }], + // ... one entry per hostname you front +]); + +serve( + ctx => { + const cfg = adapters.get(ctx.host); + if (!cfg) throw new Error(`Unknown host: ${ctx.host}`); + return createAdcpServer({ + name: cfg.name, version: '1.0.0', - resolveAccount: async (ref, { authInfo }) => lookupAccount(host, ref, authInfo), - mediaBuy: { /* ... */ }, + resolveAccount: async (ref, { authInfo }) => lookupAccount(ctx.host, ref, authInfo), + mediaBuy: cfg.handlers, }); - servers.set(host, s); - } - return s; -} - -createServer(async (req, res) => { - const host = req.headers['x-forwarded-host'] ?? req.headers.host ?? ''; - const server = getServer(String(host)); - const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); - try { - await server.connect(transport); - await transport.handleRequest(req, res); - } finally { - transport.close(); + }, + { + trustForwardedHost: true, // behind Fly/Cloud Run/ALB that sets X-Forwarded-Host + publicUrl: host => `https://${host}/mcp`, + protectedResource: host => ({ + authorization_servers: [`https://${host}/oauth`], + scopes_supported: ['read', 'write'], + }), + authenticate: verifyBearer({ + jwksUri: process.env.JWKS_URI, + issuer: process.env.ISSUER, + // Per-host audience — reads `publicUrl(host)` back off the request. + audience: req => `https://${req.headers['x-forwarded-host'] ?? req.headers.host}/mcp`, + }), } -}).listen(3001); +); ``` -Things `serve()` does that you now own: auth verification, RFC 9421 request-signature verification (if you claim `signed-requests`), `/.well-known/oauth-protected-resource/` publishing, request-body buffering for signature hashing, and idempotency+governance composition. Most of those helpers are exported — see `verifyApiKey`, `verifyBearer`, `requireSignatureWhenPresent`, and `createExpressAdapter` if you want a partial shortcut. Only go this route if you genuinely need multi-host; a separate process per host with `serve()` is the simpler answer. +Each unique host runs its resolver once and the result is cached. Every host advertises its own RFC 9728 `resource` URL, the 401 challenge carries the host's `resource_metadata` URL, and the factory sees the resolved host so it can return host-specific handlers. Auth, RFC 9421 signature verification, idempotency, and governance composition all stay inside `serve()` — nothing extra to re-own. + +**Set `trustForwardedHost: true` only when upstream sanitizes `X-Forwarded-Host`.** Without the flag, the framework reads `Host` directly — an attacker-controlled `X-Forwarded-Host` can't influence which adapter handles the request or which audience the advertised PRM carries. With the flag, you're telling `serve()` that your proxy already filtered spoofed values. ### Stdio diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index 33659faa0..95d1160d3 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -247,8 +247,15 @@ export interface VerifyBearerOptions { * endpoint (e.g., `https://my-agent.example.com/mcp`). MUST match what * the token was issued for. If you advertise via * {@link ServeOptions.protectedResource}, set this to the same URL. + * + * For multi-host deployments pass a function that returns the expected + * audience for the incoming request (typically `publicUrl(host)`). The + * function must be deterministic for a given request — `jose` is called + * with the result on every token — and MUST NOT derive audience from + * attacker-controlled inputs. Use `X-Forwarded-Host` only when `serve()` + * is configured with `trustForwardedHost: true`. */ - audience: string; + audience: string | ((req: IncomingMessage) => string); /** Optional: required scopes (all must be present in the `scope` / `scp` claim). */ requiredScopes?: string[]; /** @@ -281,17 +288,34 @@ export interface VerifyBearerOptions { */ export function verifyBearer(options: VerifyBearerOptions): Authenticator { const jwks = createRemoteJWKSet(new URL(options.jwksUri)); - const verifyOptions: JWTVerifyOptions = { + const audienceIsFn = typeof options.audience === 'function'; + const baseVerifyOptions: Omit = { algorithms: DEFAULT_JWT_ALGORITHMS.slice(), clockTolerance: DEFAULT_JWT_CLOCK_TOLERANCE_SECONDS, ...options.jwtOptions, issuer: options.issuer, - audience: options.audience, }; + const staticVerifyOptions: JWTVerifyOptions | undefined = audienceIsFn + ? undefined + : { ...baseVerifyOptions, audience: options.audience as string }; return async req => { const token = extractBearerToken(req); if (!token) return null; let payload: JWTPayload; + let verifyOptions: JWTVerifyOptions; + if (audienceIsFn) { + let audience: string; + try { + audience = (options.audience as (r: IncomingMessage) => string)(req); + } catch (err) { + // Resolver threw — surface as auth failure rather than 500 so the + // client sees a clean 401 and the operator sees the cause in the log. + throw new AuthError('Token validation failed.', { cause: err }); + } + verifyOptions = { ...baseVerifyOptions, audience }; + } else { + verifyOptions = staticVerifyOptions as JWTVerifyOptions; + } try { ({ payload } = await jwtVerify(token, jwks, verifyOptions)); } catch (err) { diff --git a/src/lib/server/serve.ts b/src/lib/server/serve.ts index 197ebfcba..b28fb661d 100644 --- a/src/lib/server/serve.ts +++ b/src/lib/server/serve.ts @@ -35,13 +35,25 @@ import type { AdcpServer } from './adcp-server'; * Contains shared resources that must survive across stateless HTTP requests, * such as the task store for MCP Tasks protocol support. * - * This helper is designed for single-tenant servers (one agent per process). - * For multi-tenant deployments, provide a custom `TaskStore` via `ServeOptions` - * that enforces tenant/session scoping. + * For multi-tenant / multi-host deployments, provide a custom `TaskStore` via + * `ServeOptions` that enforces tenant/session scoping, and branch on + * {@link host} inside your factory to return host-specific handlers. */ export interface ServeContext { /** Shared task store — use this when creating your McpServer so tasks persist across requests. */ taskStore: TaskStore; + /** + * Canonical host the request arrived on, lowercased with port preserved + * (e.g. `snap.example.com`, `localhost:3001`). Resolved from + * `X-Forwarded-Host` when `ServeOptions.trustForwardedHost` is true, + * otherwise from the `Host` header. Empty string when neither header + * is present (unusual — HTTP/1.1 requires `Host`). + * + * Use this in the factory to branch on hostname when a single process + * fronts multiple agents. The same string is passed to + * `publicUrl` / `protectedResource` resolver functions. + */ + host: string; } /** @@ -93,19 +105,16 @@ export interface ServeOptions { * * Must be an absolute https:// URL whose path matches the mount path. * - * **Multi-host caveat.** `publicUrl` is static per `serve()` call. If a - * single Node process fronts multiple hostnames (e.g., a reverse-proxy - * splitting `seller-a.example.com` and `seller-b.example.com` into one - * backend), every host sees the same advertised `resource` — buyers - * hitting `seller-b` will get a token audience-bound to `seller-a`'s - * URL and fail JWT audience validation. For multi-host deployments, - * run one `serve()` per host (separate ports + reverse proxy by Host - * header) or route hosts to isolated Node processes upstream. A - * host-aware helper is on the roadmap but not in 5.2.x — until it - * ships, the single-`publicUrl`-per-process model is the only - * supported configuration. + * **Multi-host.** Pass a function `(host) => string` when one process + * fronts multiple hostnames (white-label publishers, multi-brand + * adapters). The resolver runs per unique host — the returned URL is + * cached and used as the RFC 9728 `resource`, the 401-challenge + * `resource_metadata`, and the JWT audience for that host. Each + * resolved URL's path must match the mount path, same as the static + * form. Setting {@link trustForwardedHost} is recommended when + * behind a proxy. */ - publicUrl?: string; + publicUrl?: string | ((host: string) => string); /** * Authentication middleware applied to every request. When configured, @@ -119,8 +128,25 @@ export interface ServeOptions { /** * Advertise OAuth 2.0 protected-resource metadata at * `/.well-known/oauth-protected-resource`. Requires {@link publicUrl}. + * + * Pass a function `(host) => ProtectedResourceMetadata` for multi-host + * deployments whose authorization servers or supported scopes vary per + * hostname (e.g., each white-label seller has its own AS). The resolver + * runs once per unique host and the result is cached. The static form + * still works when every host uses the same PRM body. + */ + protectedResource?: ProtectedResourceMetadata | ((host: string) => ProtectedResourceMetadata); + + /** + * Trust `X-Forwarded-Host` / `X-Forwarded-Proto` for host resolution + * and for reconstructing the public URL on 401 challenges. Default + * `false`. Enable when `serve()` sits behind a proxy that sanitizes + * these headers (Fly.io, Cloud Run, an internal ALB that overwrites + * them). Leaving this off on the open internet lets an attacker pick + * the advertised OAuth `resource` URL by spoofing the header — so the + * framework ignores the forwarded value unless you opt in. */ - protectedResource?: ProtectedResourceMetadata; + trustForwardedHost?: boolean; /** * Pre-MCP middleware — runs after authentication but before MCP transport @@ -152,12 +178,19 @@ export interface ServeOptions { * request via `ServeContext`. This ensures MCP Tasks (create → poll → result) * work correctly across stateless HTTP requests. * + * **Multi-host.** Pass functions for `publicUrl` and `protectedResource` and + * branch on `ctx.host` in the factory to front multiple hostnames from one + * process. The framework resolves host from `X-Forwarded-Host` (when + * `trustForwardedHost: true`) or `Host`, threads it through `ServeContext`, + * and caches per-host `publicUrl`/PRM so each hostname advertises its own + * audience-bound `resource`. + * * @param createAgent - Factory function that returns a configured server — * either an `AdcpServer` from `createAdcpServer()` or a raw SDK `McpServer` * from `createTaskCapableServer()`. Called once per request so each gets a * fresh instance (a server can only be connected once). Receives a - * `ServeContext` with a shared `taskStore` — pass it to - * `createTaskCapableServer()` so tasks persist across stateless requests. + * `ServeContext` with a shared `taskStore` and the resolved `host` — + * branch on `host` to return host-specific handlers in multi-host mode. * @param options - Port, path, and callback configuration. * @returns The http.Server instance. Use the `onListening` callback or * listen for the 'listening' event to know when it's ready. @@ -170,7 +203,7 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer const port = options?.port ?? envPort ?? 3001; const mountPath = options?.path ?? '/mcp'; const taskStore = options?.taskStore ?? new InMemoryTaskStore(); - const ctx: ServeContext = { taskStore }; + const trustForwardedHost = options?.trustForwardedHost === true; if (options?.protectedResource && !options.publicUrl) { throw new Error( @@ -179,24 +212,54 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer ); } - const publicUrl = options?.publicUrl; - let publicOrigin: string | undefined; - if (publicUrl) { - let parsed: URL; - try { - parsed = new URL(publicUrl); - } catch { - throw new Error(`serve(): \`publicUrl\` is not a valid URL: ${publicUrl}`); - } - if (trimTrailingSlashes(parsed.pathname) !== trimTrailingSlashes(mountPath)) { - throw new Error( - `serve(): \`publicUrl\` path (${parsed.pathname}) must match mount path (${mountPath}). ` + - 'The public URL is the full MCP endpoint URL, including the path.' - ); - } - publicOrigin = parsed.origin; + const publicUrlOption = options?.publicUrl; + const protectedResourceOption = options?.protectedResource; + const publicUrlIsFn = typeof publicUrlOption === 'function'; + const prmIsFn = typeof protectedResourceOption === 'function'; + + // Static publicUrl — validate once at construction. Function form validates + // lazily per host so a stale factory for one host can't prevent boot. + let staticPublicOrigin: string | undefined; + if (typeof publicUrlOption === 'string') { + staticPublicOrigin = validatePublicUrl(publicUrlOption, mountPath); } + // Per-host caches. Function-form options are pure host → value lookups, + // so memoization is safe and avoids the per-request allocation of + // `new URL(...)` plus the caller's own host → config table work. + const publicUrlCache = new Map(); + const publicOriginCache = new Map(); + const prmCache = new Map(); + + const resolvePublicUrl = (host: string): string | undefined => { + if (typeof publicUrlOption === 'string') return publicUrlOption; + if (!publicUrlIsFn) return undefined; + let cached = publicUrlCache.get(host); + if (cached !== undefined) return cached; + cached = (publicUrlOption as (h: string) => string)(host); + const origin = validatePublicUrl(cached, mountPath); + publicUrlCache.set(host, cached); + publicOriginCache.set(host, origin); + return cached; + }; + + const resolvePublicOrigin = (host: string): string | undefined => { + if (typeof publicUrlOption === 'string') return staticPublicOrigin; + // Populate via resolvePublicUrl so both caches share a single validation pass. + if (resolvePublicUrl(host) == null) return undefined; + return publicOriginCache.get(host); + }; + + const resolveProtectedResource = (host: string): ProtectedResourceMetadata | undefined => { + if (!protectedResourceOption) return undefined; + if (!prmIsFn) return protectedResourceOption as ProtectedResourceMetadata; + let cached = prmCache.get(host); + if (cached !== undefined) return cached; + cached = (protectedResourceOption as (h: string) => ProtectedResourceMetadata)(host); + prmCache.set(host, cached); + return cached; + }; + if (options?.authenticate == null && process.env.NODE_ENV === 'production') { console.warn( '[adcp/serve] No `authenticate` configured — this agent will accept unauthenticated requests. ' + @@ -207,19 +270,38 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer const explicitPreTransport = options?.preTransport as AdcpPreTransport | undefined; const protectedResourcePath = `/.well-known/oauth-protected-resource${mountPath}`; - const resourceMetadataUrl = - options?.protectedResource && publicOrigin ? `${publicOrigin}${protectedResourcePath}` : undefined; const httpServer = createServer(async (req, res) => { const { pathname } = new URL(req.url || '', 'http://localhost'); + const host = resolveHost(req, trustForwardedHost); // RFC 9728 protected-resource metadata — intentionally auth-free so // clients can discover the authorization server before they have a token. - if (options?.protectedResource && pathname === protectedResourcePath) { + if (protectedResourceOption && pathname === protectedResourcePath) { + let resource: string | undefined; + let prm: ProtectedResourceMetadata | undefined; + try { + resource = resolvePublicUrl(host); + prm = resolveProtectedResource(host); + } catch (err) { + console.error('[adcp/serve] publicUrl/protectedResource resolver failed:', err); + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Protected-resource metadata unavailable for this host.' })); + return; + } + if (!resource || !prm) { + // Fail closed: advertising a garbage resource URL would mint + // audience-mismatched tokens across the fleet. 404 signals "this + // host doesn't publish PRM" — the operator sees it and either + // wires the host up or takes it out of DNS. + res.writeHead(404); + res.end('Not found'); + return; + } const body = { - resource: publicUrl!, - ...options.protectedResource, - bearer_methods_supported: options.protectedResource.bearer_methods_supported ?? ['header'], + resource, + ...prm, + bearer_methods_supported: prm.bearer_methods_supported ?? ['header'], }; res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify(body)); @@ -227,6 +309,24 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer } if (pathname === mountPath || pathname === `${mountPath}/`) { + // Resolve per-request `resource_metadata` URL for 401 challenges. For + // static PRM this is the same string every request (same as before the + // multi-host refactor); for function-form PRM it follows the host the + // request arrived on so the challenge points at the right discovery + // document. + let resourceMetadataUrl: string | undefined; + if (protectedResourceOption) { + try { + const hostOrigin = resolvePublicOrigin(host); + if (hostOrigin) resourceMetadataUrl = `${hostOrigin}${protectedResourcePath}`; + } catch (err) { + console.error('[adcp/serve] publicUrl resolver failed:', err); + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Protected-resource metadata unavailable for this host.' })); + return; + } + } + // Body buffering happens at most once per request. An idempotent helper // lets the auth path (for signature authenticators) and the preTransport // path share the buffer without re-reading a drained stream. @@ -313,7 +413,7 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer // explicit `options.preTransport` wins — it lets callers override the // default wiring (e.g., to add request logging or swap verifier // implementations). - const agentServer = createAgent(ctx); + const agentServer = createAgent({ taskStore, host }); const attached = (agentServer as unknown as Record)[ADCP_PRE_TRANSPORT]; const autoWiredPreTransport = typeof attached === 'function' ? (attached as AdcpPreTransport) : undefined; const activePreTransport = explicitPreTransport ?? autoWiredPreTransport; @@ -391,6 +491,63 @@ function trimTrailingSlashes(s: string): string { return s.slice(0, end); } +/** + * Parse and validate a `publicUrl`, returning its origin. Shared by the + * static-string path (validated once at `serve()` construction) and the + * function path (validated lazily per unique host). A bad value from the + * function form throws on the first request for that host, after PRM + * resolution has already decided to fetch; the error is caught at the + * call site and surfaced as a 500 so the operator sees the misconfigured + * host instead of a silent audience-mismatch on minted tokens. + */ +function validatePublicUrl(publicUrl: string, mountPath: string): string { + let parsed: URL; + try { + parsed = new URL(publicUrl); + } catch { + throw new Error(`serve(): \`publicUrl\` is not a valid URL: ${publicUrl}`); + } + if (trimTrailingSlashes(parsed.pathname) !== trimTrailingSlashes(mountPath)) { + throw new Error( + `serve(): \`publicUrl\` path (${parsed.pathname}) must match mount path (${mountPath}). ` + + 'The public URL is the full MCP endpoint URL, including the path.' + ); + } + return parsed.origin; +} + +/** + * Resolve the canonical host the request arrived on for multi-host + * routing and per-host PRM / audience advertisement. + * + * `X-Forwarded-Host` is trusted only when `trustForwardedHost: true` — + * otherwise an attacker could flip the advertised OAuth `resource` URL + * by spoofing one header. When a comma-separated forwarded chain is + * present the first (left-most) entry is the client-reported origin — + * use it and let the operator's upstream sanitize what it forwards. + * + * Normalizes to lowercase and preserves port. Returns empty string when + * neither header is present (HTTP/1.1 requires `Host`, so this is + * unusual — factories can branch on it to fail closed). + */ +function resolveHost(req: IncomingMessage, trustForwardedHost: boolean): string { + if (trustForwardedHost) { + const forwarded = firstHeaderValue(req.headers['x-forwarded-host']); + if (forwarded) { + const first = forwarded.split(',')[0]?.trim(); + if (first) return first.toLowerCase(); + } + } + const host = firstHeaderValue(req.headers['host']); + return host ? host.toLowerCase() : ''; +} + +function firstHeaderValue(value: string | string[] | undefined): string | undefined { + if (typeof value === 'string') return value; + if (Array.isArray(value) && value.length > 0) return value[0]; + return undefined; +} + function attachAuthInfo(req: IncomingMessage, principal: AuthPrincipal): void { const info: AuthInfo = { token: principal.token ?? '', diff --git a/src/lib/testing/local-agent-runner.ts b/src/lib/testing/local-agent-runner.ts index 7bdd73473..eff8b8432 100644 --- a/src/lib/testing/local-agent-runner.ts +++ b/src/lib/testing/local-agent-runner.ts @@ -237,7 +237,10 @@ export async function runAgainstLocalAgent(options: RunAgainstLocalAgentOptions) const startedAt = Date.now(); const mountPath = options.serveOptions?.path ?? '/mcp'; const taskStore: TaskStore = new InMemoryTaskStore(); - const ctx: ServeContext = { taskStore }; + // Bootstrap the factory once with a synthetic host so seeding and + // fixture preparation run before the server ever binds. `serve()` will + // pass the real per-request host when it calls the factory again. + const ctx: ServeContext = { taskStore, host: '' }; let auth: TestAuthorizationServer | undefined; let httpServer: HttpServer | undefined; diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js new file mode 100644 index 000000000..2075d1acd --- /dev/null +++ b/test/lib/serve-multihost.test.js @@ -0,0 +1,366 @@ +const { test, describe, before, after } = require('node:test'); +const assert = require('node:assert'); +const http = require('http'); + +/** + * Exercises the multi-host `serve()` surface: function-form `publicUrl` + * and `protectedResource`, `ServeContext.host` threading, per-host PRM + * caching, and the `trustForwardedHost` opt-in for `X-Forwarded-Host`. + * + * Tests cover the wire shape operators will actually see under a + * reverse-proxy in front of a multi-tenant Node process. + */ + +function request(port, { path = '/mcp', method = 'POST', host, headers = {} } = {}) { + return new Promise((resolve, reject) => { + const h = { ...headers }; + if (host !== undefined) h.host = host; + const req = http.request({ hostname: '127.0.0.1', port, path, method, headers: h }, res => { + let data = ''; + res.on('data', chunk => { + data += chunk; + }); + res.on('end', () => resolve({ status: res.statusCode, body: data, headers: res.headers })); + }); + req.on('error', reject); + req.end(); + }); +} + +function waitForListening(server) { + return new Promise(resolve => { + if (server.listening) return resolve(); + server.on('listening', resolve); + }); +} + +describe('serve() multi-host', () => { + let serve; + let McpServer; + + before(() => { + const lib = require('../../dist/lib/index.js'); + serve = lib.serve; + const mcp = require('@modelcontextprotocol/sdk/server/mcp.js'); + McpServer = mcp.McpServer; + }); + + test('passes resolved host to factory ctx', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + + const server = serve(factory, { port: 0, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { host: `seller-a.example.com:${port}` }); + await request(port, { host: `seller-b.example.com:${port}` }); + await request(port, { host: `SELLER-A.EXAMPLE.COM:${port}` }); + + // Host header is lowercased, port preserved. + assert.deepStrictEqual( + seen.sort(), + [`seller-a.example.com:${port}`, `seller-a.example.com:${port}`, `seller-b.example.com:${port}`] + ); + + server.close(); + }); + + test('function-form publicUrl advertises per-host resource', async () => { + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: host => `https://${host.split(':')[0]}/mcp`, + protectedResource: { authorization_servers: ['https://auth.example.com'] }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const resA = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + const resB = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `meta.example.com:${port}`, + }); + + assert.strictEqual(resA.status, 200); + assert.strictEqual(resB.status, 200); + const bodyA = JSON.parse(resA.body); + const bodyB = JSON.parse(resB.body); + assert.strictEqual(bodyA.resource, 'https://snap.example.com/mcp'); + assert.strictEqual(bodyB.resource, 'https://meta.example.com/mcp'); + // authorization_servers still comes from the static PRM object. + assert.deepStrictEqual(bodyA.authorization_servers, ['https://auth.example.com']); + assert.deepStrictEqual(bodyB.authorization_servers, ['https://auth.example.com']); + + server.close(); + }); + + test('function-form protectedResource returns per-host PRM', async () => { + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: host => `https://${host.split(':')[0]}/mcp`, + protectedResource: host => ({ + authorization_servers: [`https://${host.split(':')[0]}/oauth`], + scopes_supported: ['read', 'write'], + }), + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const res = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + + assert.strictEqual(res.status, 200); + const body = JSON.parse(res.body); + assert.deepStrictEqual(body.authorization_servers, ['https://snap.example.com/oauth']); + assert.deepStrictEqual(body.scopes_supported, ['read', 'write']); + + server.close(); + }); + + test('caches resolvers per host (called once per unique host)', async () => { + const publicUrlCalls = []; + const prmCalls = []; + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: host => { + publicUrlCalls.push(host); + return `https://${host.split(':')[0]}/mcp`; + }, + protectedResource: host => { + prmCalls.push(host); + return { authorization_servers: [`https://${host.split(':')[0]}/oauth`] }; + }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `meta.example.com:${port}`, + }); + + // Each resolver called exactly once per unique host — the 2x snap + // request shares the cache entry from the first. + assert.deepStrictEqual(publicUrlCalls.sort(), [ + `meta.example.com:${port}`, + `snap.example.com:${port}`, + ]); + assert.deepStrictEqual(prmCalls.sort(), [ + `meta.example.com:${port}`, + `snap.example.com:${port}`, + ]); + + server.close(); + }); + + test('invalid publicUrl path per host surfaces as 500', async () => { + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + // Returns a publicUrl whose path does NOT match the mount path. + // The framework fails closed rather than advertising a mismatched + // `resource` URL that would mint audience-mismatched tokens. + publicUrl: host => `https://${host.split(':')[0]}/wrong-path`, + protectedResource: { authorization_servers: ['https://auth.example.com'] }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const res = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + + assert.strictEqual(res.status, 500); + + server.close(); + }); + + test('ignores X-Forwarded-Host without trustForwardedHost', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { port: 0, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `real.example.com:${port}`, + headers: { 'x-forwarded-host': 'attacker.example.com' }, + }); + + assert.strictEqual(seen[0], `real.example.com:${port}`); + + server.close(); + }); + + test('honors X-Forwarded-Host when trustForwardedHost: true', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + trustForwardedHost: true, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `internal.fly:${port}`, + headers: { 'x-forwarded-host': 'snap.example.com' }, + }); + + assert.strictEqual(seen[0], 'snap.example.com'); + + server.close(); + }); + + test('X-Forwarded-Host chain picks first entry (client-reported origin)', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + trustForwardedHost: true, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `internal.fly:${port}`, + headers: { 'x-forwarded-host': 'snap.example.com, cdn.example.com, edge.example.com' }, + }); + + assert.strictEqual(seen[0], 'snap.example.com'); + + server.close(); + }); + + test('404 on PRM probe when host has no publicUrl mapping', async () => { + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + // Resolver returns empty string for unknown hosts — framework treats + // as "no PRM for this host" and responds 404 rather than advertising + // a blank `resource`. + publicUrl: host => { + if (host.startsWith('snap.')) return `https://snap.example.com/mcp`; + throw new Error(`unknown host: ${host}`); + }, + protectedResource: { authorization_servers: ['https://auth.example.com'] }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const res = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `unknown.example.com:${port}`, + }); + + // Resolver threw — treated as 500 (operator misconfiguration surfaced). + assert.strictEqual(res.status, 500); + + server.close(); + }); + + test('static publicUrl still works (backward compat)', async () => { + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: 'https://my-agent.example.com/mcp', + protectedResource: { authorization_servers: ['https://auth.example.com'] }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const resA = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + const resB = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `meta.example.com:${port}`, + }); + + // Both hosts see the same static `resource` — that's the pre-multi-host + // behavior, preserved when the caller doesn't opt in to per-host. + assert.strictEqual(JSON.parse(resA.body).resource, 'https://my-agent.example.com/mcp'); + assert.strictEqual(JSON.parse(resB.body).resource, 'https://my-agent.example.com/mcp'); + + server.close(); + }); + + test('function-form publicUrl with no protectedResource is allowed', async () => { + // publicUrl-only mode (no PRM advertising). Factory sees the host so + // an adapter can pick a handler set without ever publishing OAuth. + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + publicUrl: host => `https://${host.split(':')[0]}/mcp`, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + // No PRM route served. + const prmRes = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `snap.example.com:${port}`, + }); + assert.strictEqual(prmRes.status, 404); + + // MCP mount still routes and threads host. + await request(port, { host: `snap.example.com:${port}` }); + assert.strictEqual(seen[0], `snap.example.com:${port}`); + + server.close(); + }); +}); diff --git a/test/server-auth.test.js b/test/server-auth.test.js index f3629c8b4..4a7c23e81 100644 --- a/test/server-auth.test.js +++ b/test/server-auth.test.js @@ -274,6 +274,52 @@ describe('verifyBearer', () => { 'HS-family algorithms must not be default-allowed' ); }); + + it('function-form audience resolves per-request (multi-host)', async () => { + const token = await new jose.SignJWT({ sub: 'user_1', scope: 'read' }) + .setProtectedHeader({ alg: 'RS256', kid: 'test-kid' }) + .setIssuer('https://iss.example') + .setAudience('https://snap.example.com/mcp') + .setExpirationTime('5m') + .sign(keyPair.privateKey); + const auth = verifyBearer( + buildOptions({ + audience: req => `https://${req.headers.host}/mcp`, + }) + ); + const ok = await auth({ + headers: { authorization: `Bearer ${token}`, host: 'snap.example.com' }, + }); + assert.strictEqual(ok.principal, 'user_1'); + + await assert.rejects( + () => + auth({ + headers: { authorization: `Bearer ${token}`, host: 'meta.example.com' }, + }), + err => err instanceof AuthError && err.publicMessage === 'Token validation failed.' + ); + }); + + it('function-form audience thrown errors map to AuthError (not 500)', async () => { + const token = await mint({ sub: 'u', scope: 'read' }); + const auth = verifyBearer( + buildOptions({ + audience: () => { + throw new Error('cannot resolve audience for this request'); + }, + }) + ); + await assert.rejects( + () => auth({ headers: { authorization: `Bearer ${token}`, host: 'svc.example' } }), + err => { + assert.ok(err instanceof AuthError); + // Public message stays sanitized; the thrown cause doesn't leak. + assert.strictEqual(err.publicMessage, 'Token validation failed.'); + return true; + } + ); + }); }); // --------------------------------------------------------------------------- From 0c42d50cac3f97f5991d05ae013a223f187f42fc Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:10:02 -0400 Subject: [PATCH 02/10] docs(seller-skill): add createExpressAdapter recipe for AS + MCP composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Requester feedback on closed #887: SKILL.md §Alternative Transports didn't mention createExpressAdapter as the supported path for mounting an OAuth 2.1 Authorization Server alongside the MCP endpoint. Adds a worked recipe using mcpAuthRouter + createExpressAdapter on a single express() app. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/build-seller-agent/SKILL.md | 73 +++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index 7abd91bf1..6bbec13de 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -1131,9 +1131,13 @@ The quick-start uses `memoryBackend()` + `InMemoryStateStore` — both reset on Auth is not wired in the example — see [§ Protecting your agent](#protecting-your-agent) below. -## Multi-Host and Alternative Transports +## Multi-Host, Express, and Alternative Transports -`serve()` supports two shapes of deployment out of the box: single-host (the quickstart default) and multi-host (one process fronting many hostnames). Stdio is the only setup that needs a different entry point. +`serve()` supports two shapes of deployment out of the box: single-host (the quickstart default) and multi-host (one process fronting many hostnames). Three cases need a different entry point: + +- **Mounting under an existing Express app** (especially alongside OAuth 2.1 Authorization Server routes like `mcpAuthRouter({ provider })`) — use `createExpressAdapter`. +- **Stdio transport** — for CLI / desktop / local-subprocess agents. +- **Hand-rolled HTTP** — when even `createExpressAdapter` doesn't fit; `createAdcpServer().connect(transport)` is the raw escape hatch. ### Multi-host HTTP @@ -1181,6 +1185,71 @@ Each unique host runs its resolver once and the result is cached. Every host adv **Set `trustForwardedHost: true` only when upstream sanitizes `X-Forwarded-Host`.** Without the flag, the framework reads `Host` directly — an attacker-controlled `X-Forwarded-Host` can't influence which adapter handles the request or which audience the advertised PRM carries. With the flag, you're telling `serve()` that your proxy already filtered spoofed values. +### Express + OAuth Authorization Server in one process + +When your agent is *both* an OAuth 2.1 AS (issues tokens) and a protected resource (MCP endpoint), mount both on a single `express()` app using `createExpressAdapter`. This is the supported composition path — you re-own nothing vs. running `serve()`. + +```typescript +import express from 'express'; +import { createAdcpServer, createExpressAdapter, verifyBearer, anyOf, verifyApiKey } from '@adcp/client/server'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { mcpAuthRouter } from '@modelcontextprotocol/sdk/server/auth/router.js'; + +const agent = createAdcpServer({ + name: 'Snap seller', + version: '1.0.0', + resolveAccount: async (ref, { authInfo }) => lookupAccount(ref, authInfo), + mediaBuy: { /* ... */ }, +}); + +const adapter = createExpressAdapter({ + mountPath: '/api/snap', + publicUrl: 'https://seller.example.com/api/snap/mcp', + prm: { authorization_servers: ['https://seller.example.com/oauth'] }, + server: agent, +}); + +const app = express(); + +// Raw-body capture so RFC 9421 signature verification hashes the bytes +// the client signed — express.json() would consume the stream first. +app.use(express.json({ limit: '5mb', verify: adapter.rawBodyVerify })); + +// RFC 9728 PRM lives at the origin root (where OAuth graders probe), +// NOT inside the agent router. +app.use(adapter.protectedResourceMiddleware); + +// OAuth 2.1 Authorization Server routes alongside the MCP endpoint. +app.use('/oauth', mcpAuthRouter({ + provider: myOAuthProvider, + issuerUrl: new URL('https://seller.example.com/oauth'), +})); + +// MCP endpoint — per-request transport, agent is reused. +const authenticate = verifyBearer({ + jwksUri: 'https://seller.example.com/oauth/.well-known/jwks.json', + issuer: 'https://seller.example.com/oauth', + audience: 'https://seller.example.com/api/snap/mcp', +}); + +app.post('/api/snap/mcp', async (req, res) => { + const principal = await authenticate(req); + if (!principal) { res.status(401).end(); return; } + (req as any).auth = { token: principal.token, clientId: principal.principal, scopes: principal.scopes }; + const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); + try { + await agent.connect(transport); + await transport.handleRequest(req, res, req.body); + } finally { + transport.close(); + } +}); + +app.listen(3001); +``` + +`createExpressAdapter` gives you four pieces `serve()` would otherwise handle: `rawBodyVerify` (for signed requests), `protectedResourceMiddleware` (RFC 9728 at origin root, not inside the router), `getUrl` (reconstructs the canonical URL with Express's stripped mount prefix — pass to `verifySignatureAsAuthenticator`), and `resetHook` (compliance state reset between storyboards). Scale it to many hostnames with one Express Router per host, dispatched by a Host-header middleware — the 13-adapter pattern. + ### Stdio ```typescript From c99bef0cd36dfc3258262fb7521faae30dc40bbc Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:13:46 -0400 Subject: [PATCH 03/10] style: prettier format test/lib/serve-multihost.test.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI format:check failure — pure whitespace/line-wrap, no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/lib/serve-multihost.test.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index 2075d1acd..b9f38dbb2 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -61,10 +61,11 @@ describe('serve() multi-host', () => { await request(port, { host: `SELLER-A.EXAMPLE.COM:${port}` }); // Host header is lowercased, port preserved. - assert.deepStrictEqual( - seen.sort(), - [`seller-a.example.com:${port}`, `seller-a.example.com:${port}`, `seller-b.example.com:${port}`] - ); + assert.deepStrictEqual(seen.sort(), [ + `seller-a.example.com:${port}`, + `seller-a.example.com:${port}`, + `seller-b.example.com:${port}`, + ]); server.close(); }); @@ -169,14 +170,8 @@ describe('serve() multi-host', () => { // Each resolver called exactly once per unique host — the 2x snap // request shares the cache entry from the first. - assert.deepStrictEqual(publicUrlCalls.sort(), [ - `meta.example.com:${port}`, - `snap.example.com:${port}`, - ]); - assert.deepStrictEqual(prmCalls.sort(), [ - `meta.example.com:${port}`, - `snap.example.com:${port}`, - ]); + assert.deepStrictEqual(publicUrlCalls.sort(), [`meta.example.com:${port}`, `snap.example.com:${port}`]); + assert.deepStrictEqual(prmCalls.sort(), [`meta.example.com:${port}`, `snap.example.com:${port}`]); server.close(); }); From 1d94499de9c21270f418a4b4367ab54290aebdbe Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:18:10 -0400 Subject: [PATCH 04/10] chore: drop unused 'after' import in serve-multihost test github-code-quality bot flag. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/lib/serve-multihost.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index b9f38dbb2..f793a3bb1 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -1,4 +1,4 @@ -const { test, describe, before, after } = require('node:test'); +const { test, describe, before } = require('node:test'); const assert = require('node:assert'); const http = require('http'); From f1d2a02d0018487d71b5485404c78f1dbca44942 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:47:59 -0400 Subject: [PATCH 05/10] feat(server): audience callback inherits serve() host resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address code-review feedback on the multi-host serve() landing: - `verifyBearer({ audience })` gains a ctx-form `(req, { host, publicUrl }) => string` where host+publicUrl come from serve()'s resolution (which honors trustForwardedHost). An audience callback that reads X-Forwarded-Host directly can now be replaced with `(_req, { publicUrl }) => publicUrl` — the JWT audience check and the RFC 9728 `resource` URL can no longer diverge. - New UnknownHostError class + 404 mapping. Factories (or publicUrl/PRM resolvers) that throw UnknownHostError get a clean 404 with a generic body; the routing table never crosses the wire. Other thrown errors still surface as 500 so unrelated bugs remain loud. - New getServeRequestContext(req) helper for custom authenticators wired outside verifyBearer. - SKILL.md example reworked: audience now derived from publicUrl (not string-concatenating a mount path from X-Forwarded-Host), and the factory uses UnknownHostError for unknown hosts. - New cryptographic backstop test: token minted for host A, presented at host B on the same process → 401 (cross-host replay blocked). Also verifies X-Forwarded-Host spoofing doesn't flip the audience check when trustForwardedHost is false. - 3 new tests in serve-multihost.test.js covering UnknownHostError mapping + ServeRequestContext stamping. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/multihost-serve.md | 15 ++- skills/build-seller-agent/SKILL.md | 17 +++- src/lib/index.ts | 3 + src/lib/server/auth.ts | 58 +++++++++-- src/lib/server/index.ts | 5 +- src/lib/server/serve.ts | 94 +++++++++++++++--- test/lib/serve-multihost.test.js | 67 +++++++++++++ test/server-auth.test.js | 153 +++++++++++++++++++++++++++++ 8 files changed, 380 insertions(+), 32 deletions(-) diff --git a/.changeset/multihost-serve.md b/.changeset/multihost-serve.md index 980d3e40e..0e69a36af 100644 --- a/.changeset/multihost-serve.md +++ b/.changeset/multihost-serve.md @@ -11,8 +11,17 @@ without re-owning the HTTP plumbing. Set `trustForwardedHost: true` when `serve()` sits behind a proxy that sanitizes `X-Forwarded-Host`. Per-host resolver results are cached. Static `publicUrl: string` is unchanged. -`verifyBearer({ audience })` now also accepts `(req) => string` so the JWT -audience check follows the per-host `publicUrl` — a token minted for -`snap.example.com` fails audience validation when presented to `meta.example.com`. +`verifyBearer({ audience })` now also accepts `(req, ctx) => string` where +`ctx = { host, publicUrl }` comes from `serve()`'s host resolution — use +`audience: (_req, { publicUrl }) => publicUrl!` so the JWT audience check +and the RFC 9728 `resource` URL can never diverge. Reading `X-Forwarded-Host` +directly in the callback is a footgun when `trustForwardedHost` is off. + +New `UnknownHostError` class — throw it from the factory (or `publicUrl`/ +`protectedResource` resolvers) for unconfigured hosts; `serve()` maps to +404 with a generic body so the routing table never crosses the wire. + +New `getServeRequestContext(req)` helper exposes the resolved +`{ host, publicUrl }` to custom authenticators wired outside `verifyBearer`. Closes #885. diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index 6bbec13de..18189c975 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -1144,7 +1144,7 @@ Auth is not wired in the example — see [§ Protecting your agent](#protecting- Pass functions for `publicUrl` and `protectedResource`, branch on `ctx.host` in the factory, and turn on `trustForwardedHost` when a proxy terminates TLS: ```typescript -import { serve, createAdcpServer, verifyBearer } from '@adcp/client'; +import { serve, createAdcpServer, verifyBearer, UnknownHostError } from '@adcp/client'; // Host → adapter config. Whatever shape suits your deployment (DB, env, static). const adapters = new Map([ @@ -1156,7 +1156,9 @@ const adapters = new Map([ serve( ctx => { const cfg = adapters.get(ctx.host); - if (!cfg) throw new Error(`Unknown host: ${ctx.host}`); + // UnknownHostError → 404 (generic body, routing table stays off the wire). + // Any other thrown error still surfaces as 500. + if (!cfg) throw new UnknownHostError(`No adapter configured for ${ctx.host}`); return createAdcpServer({ name: cfg.name, version: '1.0.0', @@ -1174,8 +1176,11 @@ serve( authenticate: verifyBearer({ jwksUri: process.env.JWKS_URI, issuer: process.env.ISSUER, - // Per-host audience — reads `publicUrl(host)` back off the request. - audience: req => `https://${req.headers['x-forwarded-host'] ?? req.headers.host}/mcp`, + // Derive the JWT audience from the SAME publicUrl serve() advertises + // for this host. Never read X-Forwarded-Host here directly — ctx.host + // already respects trustForwardedHost, but publicUrl is better because + // the audience check and the PRM `resource` URL can't drift. + audience: (_req, { publicUrl }) => publicUrl!, }), } ); @@ -1183,8 +1188,12 @@ serve( Each unique host runs its resolver once and the result is cached. Every host advertises its own RFC 9728 `resource` URL, the 401 challenge carries the host's `resource_metadata` URL, and the factory sees the resolved host so it can return host-specific handlers. Auth, RFC 9421 signature verification, idempotency, and governance composition all stay inside `serve()` — nothing extra to re-own. +**Audience binding: use the ctx-form callback.** `audience: (req, { publicUrl }) => publicUrl` is the safest shape — the JWT audience check is guaranteed to match what RFC 9728 PRM advertises for this host, and `publicUrl` already follows `serve()`'s host resolution. `audience: (req) => ...` also works but you own the security: don't read `X-Forwarded-Host` there directly (it bypasses `trustForwardedHost`), and don't string-concat the mount path (it breaks silently if the mount path changes). + **Set `trustForwardedHost: true` only when upstream sanitizes `X-Forwarded-Host`.** Without the flag, the framework reads `Host` directly — an attacker-controlled `X-Forwarded-Host` can't influence which adapter handles the request or which audience the advertised PRM carries. With the flag, you're telling `serve()` that your proxy already filtered spoofed values. +**Unknown hosts: throw `UnknownHostError` from the factory.** `serve()` catches it and responds 404 with a generic body (the routing table never crosses the wire). Throwing any other `Error` stays as a 500 so unrelated bugs remain loud. + ### Express + OAuth Authorization Server in one process When your agent is *both* an OAuth 2.1 AS (issues tokens) and a protected resource (MCP endpoint), mount both on a single `express()` app using `createExpressAdapter`. This is the supported composition path — you re-own nothing vs. running `serve()`. diff --git a/src/lib/index.ts b/src/lib/index.ts index b20670a8f..c5b9143af 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -546,6 +546,7 @@ export { InMemoryTaskStore, isTerminal, serve, + UnknownHostError, registerTestController, handleTestControllerRequest, TestControllerError, @@ -586,6 +587,8 @@ export { IDEMPOTENCY_MIGRATION, cleanupExpiredIdempotency, hashPayload, + getServeRequestContext, + ADCP_SERVE_REQUEST_CONTEXT, } from './server'; export type { AdcpErrorOptions, diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index 95d1160d3..f0be741c4 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -122,6 +122,34 @@ export const AUTH_NEEDS_RAW_BODY: unique symbol = Symbol.for('@adcp/client.auth. */ export const AUTH_PRESENCE_GATED: unique symbol = Symbol.for('@adcp/client.auth.presenceGated'); +/** + * Request-scoped context that {@link serve} stamps on the incoming request + * before the authenticator runs. Authenticator callbacks read this via + * {@link getServeRequestContext} so they inherit `serve()`'s host + * resolution (which honors `trustForwardedHost`) instead of re-reading + * headers that may be attacker-controlled. + */ +export const ADCP_SERVE_REQUEST_CONTEXT: unique symbol = Symbol.for('@adcp/client.auth.serveRequestContext'); + +/** + * Per-request context exposed to the `verifyBearer` audience callback. + * Authoritative host + resolved publicUrl come from `serve()` — the + * callback MUST NOT reach past this object to read `X-Forwarded-Host` + * itself, or it will diverge from the advertised OAuth `resource` and + * let a spoofed header flip the audience check. + */ +export interface ServeRequestContext { + /** Canonical host for the request as resolved by `serve()`. Lowercased, port preserved. */ + host: string; + /** Resolved `publicUrl` for this host (if configured). Undefined in single-host mode with no publicUrl. */ + publicUrl?: string; +} + +/** Read the serve()-populated context off a request. Returns `undefined` when the request didn't come through `serve()`. */ +export function getServeRequestContext(req: IncomingMessage): ServeRequestContext | undefined { + return (req as IncomingMessage & { [ADCP_SERVE_REQUEST_CONTEXT]?: ServeRequestContext })[ADCP_SERVE_REQUEST_CONTEXT]; +} + interface AuthenticatorFlags { [AUTH_NEEDS_RAW_BODY]?: boolean; [AUTH_PRESENCE_GATED]?: boolean; @@ -248,14 +276,21 @@ export interface VerifyBearerOptions { * the token was issued for. If you advertise via * {@link ServeOptions.protectedResource}, set this to the same URL. * - * For multi-host deployments pass a function that returns the expected - * audience for the incoming request (typically `publicUrl(host)`). The - * function must be deterministic for a given request — `jose` is called - * with the result on every token — and MUST NOT derive audience from - * attacker-controlled inputs. Use `X-Forwarded-Host` only when `serve()` - * is configured with `trustForwardedHost: true`. + * For multi-host deployments pass a function. The two-argument form + * receives `(req, ctx)` where `ctx.host` and `ctx.publicUrl` come from + * `serve()`'s host resolution — inheriting the same + * `trustForwardedHost` policy that governs the advertised PRM. Prefer + * `(_req, { publicUrl }) => publicUrl!` so the JWT audience check and + * the RFC 9728 `resource` URL can never diverge; a callback that reads + * `X-Forwarded-Host` directly is a footgun when `trustForwardedHost` + * is off. + * + * The single-argument form `(req) => string` still works for agents + * mounting `verifyBearer` outside `serve()` (e.g., under + * `createExpressAdapter`), but inherits none of `serve()`'s header + * hardening — you MUST validate audience inputs yourself. */ - audience: string | ((req: IncomingMessage) => string); + audience: string | ((req: IncomingMessage) => string) | ((req: IncomingMessage, ctx: ServeRequestContext) => string); /** Optional: required scopes (all must be present in the `scope` / `scp` claim). */ requiredScopes?: string[]; /** @@ -306,7 +341,14 @@ export function verifyBearer(options: VerifyBearerOptions): Authenticator { if (audienceIsFn) { let audience: string; try { - audience = (options.audience as (r: IncomingMessage) => string)(req); + // Pass the serve()-resolved context as the second argument. The + // callback can use `ctx.publicUrl` (preferred — it's the same URL + // the RFC 9728 PRM advertises) or `ctx.host` for its own + // reconstruction. Falls back to an empty-host ctx when the + // authenticator is mounted outside `serve()`, so callbacks must + // still tolerate `host === ''`. + const ctx: ServeRequestContext = getServeRequestContext(req) ?? { host: '' }; + audience = (options.audience as (r: IncomingMessage, c: ServeRequestContext) => string)(req, ctx); } catch (err) { // Resolver threw — surface as auth failure rather than 500 so the // client sees a clean 401 and the operator sees the cause in the log. diff --git a/src/lib/server/index.ts b/src/lib/server/index.ts index ff3420c50..946478b43 100644 --- a/src/lib/server/index.ts +++ b/src/lib/server/index.ts @@ -96,7 +96,7 @@ export type { SeedFixtureCache, } from './test-controller'; -export { serve } from './serve'; +export { serve, UnknownHostError } from './serve'; export type { ServeContext, ServeOptions, ProtectedResourceMetadata } from './serve'; export { createExpressAdapter } from './express-adapter'; @@ -116,6 +116,8 @@ export { AUTH_PRESENCE_GATED, tagAuthenticatorPresenceGated, isAuthenticatorPresenceGated, + ADCP_SERVE_REQUEST_CONTEXT, + getServeRequestContext, DEFAULT_JWT_ALGORITHMS, DEFAULT_JWT_CLOCK_TOLERANCE_SECONDS, } from './auth'; @@ -126,6 +128,7 @@ export type { VerifyApiKeyOptions, VerifyBearerOptions, RespondUnauthorizedOptions, + ServeRequestContext, } from './auth'; export { diff --git a/src/lib/server/serve.ts b/src/lib/server/serve.ts index b28fb661d..31c26bde4 100644 --- a/src/lib/server/serve.ts +++ b/src/lib/server/serve.ts @@ -24,8 +24,14 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { TaskStore } from '@modelcontextprotocol/sdk/experimental/tasks/interfaces.js'; import { InMemoryTaskStore } from '@modelcontextprotocol/sdk/experimental/tasks/stores/in-memory.js'; import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; -import type { AuthPrincipal, Authenticator } from './auth'; -import { AuthError, authenticatorNeedsRawBody, respondUnauthorized, signatureErrorCodeFromCause } from './auth'; +import type { AuthPrincipal, Authenticator, ServeRequestContext } from './auth'; +import { + ADCP_SERVE_REQUEST_CONTEXT, + AuthError, + authenticatorNeedsRawBody, + respondUnauthorized, + signatureErrorCodeFromCause, +} from './auth'; import { ADCP_PRE_TRANSPORT, type AdcpPreTransport } from './create-adcp-server'; import type { AdcpServer } from './adcp-server'; @@ -56,6 +62,20 @@ export interface ServeContext { host: string; } +/** + * Thrown from an agent factory (or a `publicUrl` / `protectedResource` + * resolver) when the incoming request's host isn't configured. `serve()` + * catches it and responds 404 with a generic body — the adapter routing + * table never crosses the wire. Any other thrown error still surfaces + * as 500 so unrelated bugs stay loud. + */ +export class UnknownHostError extends Error { + constructor(message = 'Unknown host') { + super(message); + this.name = 'UnknownHostError'; + } +} + /** * OAuth 2.0 Protected Resource Metadata (RFC 9728) advertised at * `/.well-known/oauth-protected-resource`. @@ -284,6 +304,14 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer resource = resolvePublicUrl(host); prm = resolveProtectedResource(host); } catch (err) { + if (err instanceof UnknownHostError) { + // Operator signalled "this host isn't in my routing table." 404 + // lets the OAuth grader probe fall through cleanly and doesn't + // leak the configured host set across the wire. + res.writeHead(404); + res.end('Not found'); + return; + } console.error('[adcp/serve] publicUrl/protectedResource resolver failed:', err); res.writeHead(500, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ error: 'Protected-resource metadata unavailable for this host.' })); @@ -309,24 +337,45 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer } if (pathname === mountPath || pathname === `${mountPath}/`) { - // Resolve per-request `resource_metadata` URL for 401 challenges. For - // static PRM this is the same string every request (same as before the - // multi-host refactor); for function-form PRM it follows the host the - // request arrived on so the challenge points at the right discovery - // document. + // Resolve per-request `resource_metadata` URL for 401 challenges and + // the per-host `publicUrl` authenticators read via + // {@link getServeRequestContext}. Resolving both up front means + // the audience callback in `verifyBearer` can never diverge from + // what the framework advertises in `/.well-known/...`. For static + // PRM this is the same string every request (pre-multi-host + // behavior); for function-form PRM/URL it follows the host. let resourceMetadataUrl: string | undefined; - if (protectedResourceOption) { - try { - const hostOrigin = resolvePublicOrigin(host); - if (hostOrigin) resourceMetadataUrl = `${hostOrigin}${protectedResourcePath}`; - } catch (err) { - console.error('[adcp/serve] publicUrl resolver failed:', err); - res.writeHead(500, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ error: 'Protected-resource metadata unavailable for this host.' })); + let resolvedPublicUrl: string | undefined; + try { + resolvedPublicUrl = resolvePublicUrl(host); + } catch (err) { + if (err instanceof UnknownHostError) { + res.writeHead(404); + res.end('Not found'); return; } + console.error('[adcp/serve] publicUrl resolver failed:', err); + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Protected-resource metadata unavailable for this host.' })); + return; + } + if (protectedResourceOption) { + const hostOrigin = resolvePublicOrigin(host); + if (hostOrigin) resourceMetadataUrl = `${hostOrigin}${protectedResourcePath}`; } + // Stamp the serve-resolved context on the request so authenticator + // callbacks (e.g. `verifyBearer`'s audience resolver) inherit the + // framework's host resolution — they MUST NOT re-derive host from + // raw headers, since that would diverge from what PRM advertises + // and let a spoofed `X-Forwarded-Host` flip the JWT audience check + // when `trustForwardedHost` is false. + const serveRequestContext: ServeRequestContext = resolvedPublicUrl + ? { host, publicUrl: resolvedPublicUrl } + : { host }; + (req as IncomingMessage & { [ADCP_SERVE_REQUEST_CONTEXT]?: ServeRequestContext })[ADCP_SERVE_REQUEST_CONTEXT] = + serveRequestContext; + // Body buffering happens at most once per request. An idempotent helper // lets the auth path (for signature authenticators) and the preTransport // path share the buffer without re-reading a drained stream. @@ -413,7 +462,20 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer // explicit `options.preTransport` wins — it lets callers override the // default wiring (e.g., to add request logging or swap verifier // implementations). - const agentServer = createAgent({ taskStore, host }); + let agentServer: AdcpServer | McpServer; + try { + agentServer = createAgent({ taskStore, host }); + } catch (err) { + if (err instanceof UnknownHostError) { + // Factory signalled "this host isn't routable." 404 keeps the + // adapter table off the wire while giving ops a clean failure + // mode instead of the generic-500 confusion. + res.writeHead(404); + res.end('Not found'); + return; + } + throw err; + } const attached = (agentServer as unknown as Record)[ADCP_PRE_TRANSPORT]; const autoWiredPreTransport = typeof attached === 'function' ? (attached as AdcpPreTransport) : undefined; const activePreTransport = explicitPreTransport ?? autoWiredPreTransport; diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index f793a3bb1..f754d76cd 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -328,6 +328,73 @@ describe('serve() multi-host', () => { server.close(); }); + test('UnknownHostError from factory maps to 404 (not 500)', async () => { + const { UnknownHostError } = require('../../dist/lib/index.js'); + const factory = ctx => { + if (ctx.host.startsWith('known.')) return new McpServer({ name: 'Test', version: '1.0.0' }); + throw new UnknownHostError(`no adapter for ${ctx.host}`); + }; + const server = serve(factory, { port: 0, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + const res = await request(port, { host: `unknown.example.com:${port}` }); + assert.strictEqual(res.status, 404, 'UnknownHostError must map to 404'); + // Body is a generic "Not found" — the routing table never crosses the wire. + assert.ok(!res.body.includes('unknown.example.com'), 'host must not appear in 404 body'); + + server.close(); + }); + + test('UnknownHostError from publicUrl resolver maps to 404 on PRM probe', async () => { + const { UnknownHostError } = require('../../dist/lib/index.js'); + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: host => { + if (host.startsWith('known.')) return `https://${host.split(':')[0]}/mcp`; + throw new UnknownHostError(host); + }, + protectedResource: { authorization_servers: ['https://auth.example.com'] }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const res = await request(port, { + method: 'GET', + path: '/.well-known/oauth-protected-resource/mcp', + host: `unknown.example.com:${port}`, + }); + assert.strictEqual(res.status, 404); + + server.close(); + }); + + test('ServeRequestContext stamped on req before authenticate runs', async () => { + const { getServeRequestContext } = require('../../dist/lib/index.js'); + const seen = []; + const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve(factory, { + port: 0, + publicUrl: host => `https://${host.split(':')[0]}/mcp`, + authenticate: req => { + seen.push(getServeRequestContext(req)); + return { principal: 'test' }; + }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { host: `snap.example.com:${port}` }); + assert.strictEqual(seen.length, 1); + assert.strictEqual(seen[0].host, `snap.example.com:${port}`); + assert.strictEqual(seen[0].publicUrl, 'https://snap.example.com/mcp'); + + server.close(); + }); + test('function-form publicUrl with no protectedResource is allowed', async () => { // publicUrl-only mode (no PRM advertising). Factory sees the host so // an adapter can pick a handler set without ever publishing OAuth. diff --git a/test/server-auth.test.js b/test/server-auth.test.js index 4a7c23e81..659dd401f 100644 --- a/test/server-auth.test.js +++ b/test/server-auth.test.js @@ -603,3 +603,156 @@ describe('respondUnauthorized', () => { assert.strictEqual(status, 403); }); }); + +// --------------------------------------------------------------------------- +// Cryptographic backstop: multi-host JWT audience isolation +// +// Named test for the invariant the entire multi-host surface rests on: a +// token minted for host A, presented against the SAME Node process at host +// B, MUST fail audience validation. Everything else (PRM per-host, factory +// routing, trustForwardedHost) is compensating controls around this one +// property. If it breaks, cross-tenant token replay is live. +// --------------------------------------------------------------------------- + +describe('multi-host JWT audience isolation (cross-host replay)', () => { + let jose; + let keyPair; + let jwksServer; + let jwksPort; + let agentServer; + let agentPort; + + before(async () => { + jose = await import('jose'); + keyPair = await jose.generateKeyPair('RS256', { modulusLength: 2048, extractable: true }); + const jwk = await jose.exportJWK(keyPair.publicKey); + jwk.kid = 'test-kid'; + jwk.alg = 'RS256'; + jwk.use = 'sig'; + const http = require('http'); + jwksServer = http.createServer((req, res) => { + if (req.url === '/jwks.json') { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ keys: [jwk] })); + } else { + res.writeHead(404).end(); + } + }); + await new Promise(r => jwksServer.listen(0, r)); + jwksPort = jwksServer.address().port; + + // Multi-host serve() with a ctx-form audience callback: audience + // MUST be derived from serve()'s resolved publicUrl, NOT read off + // any header. This is the security-critical path we're validating. + await new Promise(resolve => { + const srv = serve(() => createAgent(), { + port: 0, + publicUrl: host => `https://${host.split(':')[0]}/mcp`, + protectedResource: host => ({ + authorization_servers: [`https://${host.split(':')[0]}/oauth`], + }), + authenticate: verifyBearer({ + jwksUri: `http://127.0.0.1:${jwksPort}/jwks.json`, + issuer: 'https://iss.example', + audience: (_req, { publicUrl }) => publicUrl, + }), + onListening: url => { + agentServer = srv; + agentPort = new URL(url).port; + resolve(); + }, + }); + }); + }); + + after(() => { + if (agentServer) agentServer.close(); + if (jwksServer) jwksServer.close(); + }); + + async function mintForAudience(audience) { + return new jose.SignJWT({ sub: 'user_1', scope: 'read' }) + .setProtectedHeader({ alg: 'RS256', kid: 'test-kid' }) + .setIssuer('https://iss.example') + .setAudience(audience) + .setExpirationTime('5m') + .setIssuedAt() + .sign(keyPair.privateKey); + } + + // Node's fetch/undici rewrites the Host header from the URL — has to + // be raw http.request to exercise per-host routing end-to-end. + function rawRequest({ host, forwardedHost, authorization, method = 'POST' }) { + const http = require('http'); + return new Promise((resolve, reject) => { + const headers = { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + }; + if (host !== undefined) headers.host = host; + if (forwardedHost !== undefined) headers['x-forwarded-host'] = forwardedHost; + if (authorization !== undefined) headers.authorization = authorization; + const req = http.request({ hostname: '127.0.0.1', port: agentPort, path: '/mcp', method, headers }, res => { + let body = ''; + res.on('data', chunk => { + body += chunk; + }); + res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body })); + }); + req.on('error', reject); + req.end(JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: {} })); + }); + } + + async function callAgent(hostHeader, token) { + return rawRequest({ host: hostHeader, authorization: `Bearer ${token}` }); + } + + it('token minted for host A is accepted at host A', async () => { + const token = await mintForAudience('https://snap.example.com/mcp'); + const res = await callAgent(`snap.example.com:${agentPort}`, token); + // Not 401 — the audience check matched. (The MCP handshake itself + // may 200 or 406 depending on Accept negotiation, but it must not + // be an auth failure.) + assert.notStrictEqual(res.status, 401, 'matching audience must not be rejected'); + }); + + it('token minted for host A is REJECTED at host B (cross-host replay blocked)', async () => { + const snapToken = await mintForAudience('https://snap.example.com/mcp'); + const res = await callAgent(`meta.example.com:${agentPort}`, snapToken); + assert.strictEqual( + res.status, + 401, + 'token for snap.example.com/mcp MUST NOT authorize a request landing at meta.example.com — cross-host replay is the exact attack multi-host audience binding prevents' + ); + // WWW-Authenticate carries the per-host resource_metadata URL so the + // buyer knows where to re-negotiate tokens for THIS host. + const wwwAuth = res.headers['www-authenticate'] || ''; + assert.match(wwwAuth, /resource_metadata=/); + assert.match(wwwAuth, /meta\.example\.com/); + }); + + it('X-Forwarded-Host spoof does NOT flip the audience check (trustForwardedHost: false default)', async () => { + // Token minted for snap. Caller tries to trick the server into + // treating the request as snap's by spoofing X-Forwarded-Host, while + // actually hitting meta via the real Host header. Without + // trustForwardedHost: true, serve() ignores the forwarded header — + // publicUrl resolves to https://meta.example.com/mcp, audience check + // rejects the snap-audience token. + const snapToken = await mintForAudience('https://snap.example.com/mcp'); + const res = await rawRequest({ + host: `meta.example.com:${agentPort}`, + forwardedHost: 'snap.example.com', + authorization: `Bearer ${snapToken}`, + }); + assert.strictEqual(res.status, 401, 'spoofed X-Forwarded-Host must not flip the audience check'); + }); + + it('per-host 401 challenge advertises the host-specific resource_metadata URL', async () => { + const res = await callAgent(`snap.example.com:${agentPort}`, 'not-a-jwt'); + assert.strictEqual(res.status, 401); + const wwwAuth = res.headers['www-authenticate'] || ''; + assert.match(wwwAuth, /snap\.example\.com/); + assert.ok(!wwwAuth.includes('meta.example.com')); + }); +}); From 659fa7021010502e02fdd8bb6238b2f5eb020b85 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 12:54:46 -0400 Subject: [PATCH 06/10] feat(server): hostname helper, RFC 7239 parsing, stronger proxy docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address follow-up points from the multi-host review: - New `hostname(host)` helper — strips port (including IPv6 brackets) for use in `publicUrl`/`protectedResource` resolvers. Exported from root and `@adcp/client/server`. Replaces the `host.split(':')[0]` footgun in SKILL examples. - RFC 7239 `Forwarded: host=...` parsing. When `trustForwardedHost: true`, fallback order is `X-Forwarded-Host` → `Forwarded:` → `Host`. Handles multi-hop (first entry wins, same policy), quoted strings (for IPv6 literals and hosts with ports per RFC 7239 §4), and backslash escapes. Ignored when `trustForwardedHost: false`. - Strengthened `trustForwardedHost` JSDoc with append-vs-replace proxy guidance. Names the common proxies by behavior (Fly/Cloud Run/GCP LB overwrite → safe; AWS ALB/nginx default append → NOT safe without `proxy_set_header X-Forwarded-Host $host;`). - SKILL.md fail-fast on empty `ctx.host` via UnknownHostError, uses `hostname()` in resolvers, explains the factory-per-request cost and why AdcpServer caching isn't safe today (serve() closes after each request) — points at follow-up #901 for the reuse mode. - 5 new multi-host tests: hostname() IPv6/port handling, Forwarded: basic + quoted + multi-hop, X-Forwarded-Host precedence over Forwarded:, Forwarded: ignored when trustForwardedHost: false. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/build-seller-agent/SKILL.md | 112 +++++++++++++++---------- src/lib/index.ts | 1 + src/lib/server/index.ts | 2 +- src/lib/server/serve.ts | 128 +++++++++++++++++++++++++---- test/lib/serve-multihost.test.js | 109 ++++++++++++++++++++++++ 5 files changed, 290 insertions(+), 62 deletions(-) diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index 18189c975..3698a8121 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -21,27 +21,27 @@ A seller agent receives briefs from buyers, returns products with pricing, accep - Serving audience segments → `skills/build-signals-agent/` - Rendering creatives from briefs → that's a creative agent -## The baseline: what every sales-* agent MUST implement +## The baseline: what every sales-\* agent MUST implement -Every sales-* specialism (including `sales-social`, `sales-broadcast-tv`, `sales-retail-media`, `sales-catalog-driven`, etc.) is **additive on top of this baseline**. If you claim any `sales-*` specialism, you implement these tools regardless of the specialism-specific deltas below. +Every sales-_ specialism (including `sales-social`, `sales-broadcast-tv`, `sales-retail-media`, `sales-catalog-driven`, etc.) is **additive on top of this baseline**. If you claim any `sales-_` specialism, you implement these tools regardless of the specialism-specific deltas below. **Required tools** (tested by the `media_buy_seller` storyboard bundle at `compliance/cache/3.0.0/protocols/media-buy/`): -| Tool | Purpose | `createAdcpServer` group | -|---|---|---| -| `get_adcp_capabilities` | Declare protocols + specialisms + features | auto (framework) | -| `sync_accounts` | Advertiser onboarding, per-tenant account creation | `accounts` | -| `list_accounts` | Account lookup by brand/operator; buyers listing their accounts on your platform | `accounts` | -| `get_products` | Product catalog discovery from a brief; returns `{ products: [...] }` | `mediaBuy` | -| `list_creative_formats` | Formats your agent accepts | `mediaBuy` | -| `create_media_buy` | Accept a campaign with packages, budget, flight dates | `mediaBuy` | -| `update_media_buy` | Bid, budget, status, package mutations over the campaign lifecycle | `mediaBuy` | -| `get_media_buys` | Read campaigns back with full state (status, budget, packages, targeting overlays) | `mediaBuy` | -| `sync_creatives` | Accept creative assets and return per-asset status | `mediaBuy` | -| `list_creatives` | Read the creative library back with pagination | `mediaBuy` | -| `get_media_buy_delivery` | Delivery + spend reporting with `reporting_period`, per-package billing rows | `mediaBuy` | - -**Minimum handler skeleton** — every sales-* seller starts here, then adds specialism-specific behavior on top: +| Tool | Purpose | `createAdcpServer` group | +| ------------------------ | ---------------------------------------------------------------------------------- | ------------------------ | +| `get_adcp_capabilities` | Declare protocols + specialisms + features | auto (framework) | +| `sync_accounts` | Advertiser onboarding, per-tenant account creation | `accounts` | +| `list_accounts` | Account lookup by brand/operator; buyers listing their accounts on your platform | `accounts` | +| `get_products` | Product catalog discovery from a brief; returns `{ products: [...] }` | `mediaBuy` | +| `list_creative_formats` | Formats your agent accepts | `mediaBuy` | +| `create_media_buy` | Accept a campaign with packages, budget, flight dates | `mediaBuy` | +| `update_media_buy` | Bid, budget, status, package mutations over the campaign lifecycle | `mediaBuy` | +| `get_media_buys` | Read campaigns back with full state (status, budget, packages, targeting overlays) | `mediaBuy` | +| `sync_creatives` | Accept creative assets and return per-asset status | `mediaBuy` | +| `list_creatives` | Read the creative library back with pagination | `mediaBuy` | +| `get_media_buy_delivery` | Delivery + spend reporting with `reporting_period`, per-package billing rows | `mediaBuy` | + +**Minimum handler skeleton** — every sales-\* seller starts here, then adds specialism-specific behavior on top: ```ts createAdcpServer({ @@ -77,17 +77,17 @@ Your compliance obligations come from the specialisms you claim in `get_adcp_cap **Claim multiple specialisms.** A typical social seller claims `sales-non-guaranteed` + `sales-social`. A typical broadcast seller claims `sales-guaranteed` + `sales-broadcast-tv`. A typical social seller doing audience sync claims `sales-non-guaranteed` + `sales-social` + `audience-sync`. -| Specialism | Status | Delta from baseline | See | -| ---------------------- | ------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | -| `sales-guaranteed` | stable | IO approval is **task-layer**, not MediaBuy-layer. Return a task envelope (MCP Tasks SDK) with `status: 'submitted'` + `task_id` + `message`. Do NOT return `media_buy_id` or `packages` yet — those land on the final artifact when the task completes. There is no `pending_approval` MediaBuy status. | [§ sales-guaranteed](#specialism-sales-guaranteed) | -| `sales-non-guaranteed` | stable | Instant `status: 'active'` with `confirmed_at`; accept `bid_price` on packages; expose `update_media_buy` for bid/budget changes | [§ sales-non-guaranteed](#specialism-sales-non-guaranteed) | -| `sales-broadcast-tv` | stable | Top-level `agency_estimate_number`; per-package `measurement_terms.billing_measurement`; Ad-ID `industry_identifiers` on creatives; `measurement_windows` (Live/C3/C7) on delivery | [§ sales-broadcast-tv](#specialism-sales-broadcast-tv) | -| `sales-streaming-tv` | preview | v3.1 placeholder (empty `phases`) — ship the baseline, declare `channels: ['ctv'] as const` on products | Baseline only | -| `sales-social` | stable | **Additive**: baseline `get_products` + `create_media_buy` still apply (Snap/Meta/TikTok all have product catalogs and campaigns). Adds `sync_audiences` (audience push), `sync_creatives` (native formats), `sync_catalogs` (dynamic product ads), `log_event` (conversion tracking), `get_account_financials` (prepaid-balance monitoring), and `sync_accounts` with `account_scope`/`payment_terms`/`setup` for advertiser onboarding. Declare `sales-social` **alongside** `sales-non-guaranteed` (or `-guaranteed`) — don't replace it. | [§ sales-social](#specialism-sales-social) | -| `sales-exchange` | preview | v3.1 placeholder — target `sales-non-guaranteed` baseline; PMP / deal IDs / auction transparency pending | Baseline only | -| `sales-proposal-mode` | stable | `get_products` returns `proposals[]` with `budget_allocations`; handle `buying_mode: 'refine'`; accept via `create_media_buy` with `proposal_id` + `total_budget` and no `packages` | [§ sales-proposal-mode](#specialism-sales-proposal-mode) | -| `audience-sync` | stable | Track: `audiences`. Implement `sync_audiences` (handles discovery, add, and delete) and `list_accounts`. Hashed identifiers (SHA-256 lowercased+trimmed). Match-rate telemetry on response. | [§ audience-sync](#specialism-audience-sync) | -| `signed-requests` | preview | RFC 9421 HTTP Signature verification on mutating requests. Advertise `request_signing.supported: true` in capabilities; graded against conformance vectors — positive vectors must produce non-4xx; negative vectors must return `401` with `WWW-Authenticate: Signature error=""` matching the vector's `expected_outcome.error_code` byte-for-byte. | [§ signed-requests](#specialism-signed-requests) | +| Specialism | Status | Delta from baseline | See | +| ---------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------- | +| `sales-guaranteed` | stable | IO approval is **task-layer**, not MediaBuy-layer. Return a task envelope (MCP Tasks SDK) with `status: 'submitted'` + `task_id` + `message`. Do NOT return `media_buy_id` or `packages` yet — those land on the final artifact when the task completes. There is no `pending_approval` MediaBuy status. | [§ sales-guaranteed](#specialism-sales-guaranteed) | +| `sales-non-guaranteed` | stable | Instant `status: 'active'` with `confirmed_at`; accept `bid_price` on packages; expose `update_media_buy` for bid/budget changes | [§ sales-non-guaranteed](#specialism-sales-non-guaranteed) | +| `sales-broadcast-tv` | stable | Top-level `agency_estimate_number`; per-package `measurement_terms.billing_measurement`; Ad-ID `industry_identifiers` on creatives; `measurement_windows` (Live/C3/C7) on delivery | [§ sales-broadcast-tv](#specialism-sales-broadcast-tv) | +| `sales-streaming-tv` | preview | v3.1 placeholder (empty `phases`) — ship the baseline, declare `channels: ['ctv'] as const` on products | Baseline only | +| `sales-social` | stable | **Additive**: baseline `get_products` + `create_media_buy` still apply (Snap/Meta/TikTok all have product catalogs and campaigns). Adds `sync_audiences` (audience push), `sync_creatives` (native formats), `sync_catalogs` (dynamic product ads), `log_event` (conversion tracking), `get_account_financials` (prepaid-balance monitoring), and `sync_accounts` with `account_scope`/`payment_terms`/`setup` for advertiser onboarding. Declare `sales-social` **alongside** `sales-non-guaranteed` (or `-guaranteed`) — don't replace it. | [§ sales-social](#specialism-sales-social) | +| `sales-exchange` | preview | v3.1 placeholder — target `sales-non-guaranteed` baseline; PMP / deal IDs / auction transparency pending | Baseline only | +| `sales-proposal-mode` | stable | `get_products` returns `proposals[]` with `budget_allocations`; handle `buying_mode: 'refine'`; accept via `create_media_buy` with `proposal_id` + `total_budget` and no `packages` | [§ sales-proposal-mode](#specialism-sales-proposal-mode) | +| `audience-sync` | stable | Track: `audiences`. Implement `sync_audiences` (handles discovery, add, and delete) and `list_accounts`. Hashed identifiers (SHA-256 lowercased+trimmed). Match-rate telemetry on response. | [§ audience-sync](#specialism-audience-sync) | +| `signed-requests` | preview | RFC 9421 HTTP Signature verification on mutating requests. Advertise `request_signing.supported: true` in capabilities; graded against conformance vectors — positive vectors must produce non-4xx; negative vectors must return `401` with `WWW-Authenticate: Signature error=""` matching the vector's `expected_outcome.error_code` byte-for-byte. | [§ signed-requests](#specialism-signed-requests) | **Not in this skill:** `sales-catalog-driven` and `sales-retail-media` (both in `skills/build-retail-media-agent/` — catalog-driven applies to restaurants, travel, and local commerce too, not only retail). @@ -571,10 +571,10 @@ To pass the `deterministic_testing` storyboard — and the rejection-branch step **Pick by state shape — not by helper quality.** Both helpers below call the same underlying primitives; the split is about the shape of the state you're mutating, not a gradient of abstraction. -| Your domain state is… | Use | Worked example | -|---|---|---| -| Simple — each scenario maps cleanly to one repository method (`seed_creative` → `creativeRepo.upsert`) | `createComplyController` | [`examples/comply-controller-seller.ts`](../../examples/comply-controller-seller.ts) | -| Typed — media buys with packages / revision / history, creatives with format_id / manifest, seed must populate the same records `get_*` reads | `registerTestController` + hand-rolled `TestControllerStore` | [`examples/seller-test-controller.ts`](../../examples/seller-test-controller.ts) | +| Your domain state is… | Use | Worked example | +| ---------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------ | ------------------------------------------------------------------------------------ | +| Simple — each scenario maps cleanly to one repository method (`seed_creative` → `creativeRepo.upsert`) | `createComplyController` | [`examples/comply-controller-seller.ts`](../../examples/comply-controller-seller.ts) | +| Typed — media buys with packages / revision / history, creatives with format*id / manifest, seed must populate the same records `get*\*` reads | `registerTestController` + hand-rolled `TestControllerStore` | [`examples/seller-test-controller.ts`](../../examples/seller-test-controller.ts) | ### Option A: `createComplyController` (adapter surface) @@ -1144,9 +1144,12 @@ Auth is not wired in the example — see [§ Protecting your agent](#protecting- Pass functions for `publicUrl` and `protectedResource`, branch on `ctx.host` in the factory, and turn on `trustForwardedHost` when a proxy terminates TLS: ```typescript -import { serve, createAdcpServer, verifyBearer, UnknownHostError } from '@adcp/client'; +import { serve, createAdcpServer, verifyBearer, UnknownHostError, hostname } from '@adcp/client'; // Host → adapter config. Whatever shape suits your deployment (DB, env, static). +// Cache the CONFIG (not the AdcpServer). serve() still instantiates the +// server per request today, but a config Map keeps the expensive part +// (handler bundle, idempotency store, DB pool) at module scope. const adapters = new Map([ ['snap.agentic-adapters.scope3.com', { name: 'Snap seller', handlers: snapHandlers }], ['meta.agentic-adapters.scope3.com', { name: 'Meta seller', handlers: metaHandlers }], @@ -1155,6 +1158,11 @@ const adapters = new Map([ serve( ctx => { + // Fail closed on missing Host header. HTTP/1.1 requires it, but a + // misbehaving client can omit it — ctx.host is `''` in that case, + // and a blank-host adapter lookup would mint audience-mismatched + // tokens if we proceeded. + if (!ctx.host) throw new UnknownHostError('Host header required'); const cfg = adapters.get(ctx.host); // UnknownHostError → 404 (generic body, routing table stays off the wire). // Any other thrown error still surfaces as 500. @@ -1168,9 +1176,11 @@ serve( }, { trustForwardedHost: true, // behind Fly/Cloud Run/ALB that sets X-Forwarded-Host - publicUrl: host => `https://${host}/mcp`, + // hostname() strips the port — test/local runs include `:3001`, production + // doesn't. Works for IPv6 too. + publicUrl: host => `https://${hostname(host)}/mcp`, protectedResource: host => ({ - authorization_servers: [`https://${host}/oauth`], + authorization_servers: [`https://${hostname(host)}/oauth`], scopes_supported: ['read', 'write'], }), authenticate: verifyBearer({ @@ -1190,13 +1200,15 @@ Each unique host runs its resolver once and the result is cached. Every host adv **Audience binding: use the ctx-form callback.** `audience: (req, { publicUrl }) => publicUrl` is the safest shape — the JWT audience check is guaranteed to match what RFC 9728 PRM advertises for this host, and `publicUrl` already follows `serve()`'s host resolution. `audience: (req) => ...` also works but you own the security: don't read `X-Forwarded-Host` there directly (it bypasses `trustForwardedHost`), and don't string-concat the mount path (it breaks silently if the mount path changes). -**Set `trustForwardedHost: true` only when upstream sanitizes `X-Forwarded-Host`.** Without the flag, the framework reads `Host` directly — an attacker-controlled `X-Forwarded-Host` can't influence which adapter handles the request or which audience the advertised PRM carries. With the flag, you're telling `serve()` that your proxy already filtered spoofed values. +**`trustForwardedHost: true` requires an overwriting proxy.** The framework trusts the first entry in an `X-Forwarded-Host` chain — safe when your proxy rewrites the header on ingress, UNSAFE when it appends (the attacker gets to pick the first entry). Fly, Cloud Run, and GCP HTTPS LBs overwrite. AWS ALB default and nginx default append — these need `proxy_set_header X-Forwarded-Host $host;` or equivalent before you enable the flag. Verify against a request that already has `X-Forwarded-Host: attacker.example` in it. RFC 7239 `Forwarded: host=...` is read the same way (same trust requirement). **Unknown hosts: throw `UnknownHostError` from the factory.** `serve()` catches it and responds 404 with a generic body (the routing table never crosses the wire). Throwing any other `Error` stays as a 500 so unrelated bugs remain loud. +**Factory runs per request.** `serve()` calls the factory on every incoming request (to avoid cross-request state bleed) and closes the returned server at the end. Keep the factory cheap: look up a pre-built adapter config from a module-scoped `Map`, and let `createAdcpServer(...)` build a fresh wrapper from that config. Do NOT cache the `AdcpServer` instance across requests — `serve()` closes it after each call, so the cache would be stale on request 2. If per-request `createAdcpServer` cost is a measurable bottleneck, track [#901](https://github.com/adcontextprotocol/adcp-client/issues/901) — a reuse mode is planned. + ### Express + OAuth Authorization Server in one process -When your agent is *both* an OAuth 2.1 AS (issues tokens) and a protected resource (MCP endpoint), mount both on a single `express()` app using `createExpressAdapter`. This is the supported composition path — you re-own nothing vs. running `serve()`. +When your agent is _both_ an OAuth 2.1 AS (issues tokens) and a protected resource (MCP endpoint), mount both on a single `express()` app using `createExpressAdapter`. This is the supported composition path — you re-own nothing vs. running `serve()`. ```typescript import express from 'express'; @@ -1208,7 +1220,9 @@ const agent = createAdcpServer({ name: 'Snap seller', version: '1.0.0', resolveAccount: async (ref, { authInfo }) => lookupAccount(ref, authInfo), - mediaBuy: { /* ... */ }, + mediaBuy: { + /* ... */ + }, }); const adapter = createExpressAdapter({ @@ -1229,10 +1243,13 @@ app.use(express.json({ limit: '5mb', verify: adapter.rawBodyVerify })); app.use(adapter.protectedResourceMiddleware); // OAuth 2.1 Authorization Server routes alongside the MCP endpoint. -app.use('/oauth', mcpAuthRouter({ - provider: myOAuthProvider, - issuerUrl: new URL('https://seller.example.com/oauth'), -})); +app.use( + '/oauth', + mcpAuthRouter({ + provider: myOAuthProvider, + issuerUrl: new URL('https://seller.example.com/oauth'), + }) +); // MCP endpoint — per-request transport, agent is reused. const authenticate = verifyBearer({ @@ -1243,7 +1260,10 @@ const authenticate = verifyBearer({ app.post('/api/snap/mcp', async (req, res) => { const principal = await authenticate(req); - if (!principal) { res.status(401).end(); return; } + if (!principal) { + res.status(401).end(); + return; + } (req as any).auth = { token: principal.token, clientId: principal.principal, scopes: principal.scopes }; const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); try { @@ -1268,8 +1288,10 @@ import { createAdcpServer } from '@adcp/client/server'; const server = createAdcpServer({ name: 'Local Seller', version: '1.0.0', - resolveAccount: async (ref) => lookupAccount(ref), - mediaBuy: { /* ... */ }, + resolveAccount: async ref => lookupAccount(ref), + mediaBuy: { + /* ... */ + }, }); await server.connect(new StdioServerTransport()); diff --git a/src/lib/index.ts b/src/lib/index.ts index c5b9143af..f20908752 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -547,6 +547,7 @@ export { isTerminal, serve, UnknownHostError, + hostname, registerTestController, handleTestControllerRequest, TestControllerError, diff --git a/src/lib/server/index.ts b/src/lib/server/index.ts index 946478b43..90f8a55da 100644 --- a/src/lib/server/index.ts +++ b/src/lib/server/index.ts @@ -96,7 +96,7 @@ export type { SeedFixtureCache, } from './test-controller'; -export { serve, UnknownHostError } from './serve'; +export { serve, UnknownHostError, hostname } from './serve'; export type { ServeContext, ServeOptions, ProtectedResourceMetadata } from './serve'; export { createExpressAdapter } from './express-adapter'; diff --git a/src/lib/server/serve.ts b/src/lib/server/serve.ts index 31c26bde4..bb214eee8 100644 --- a/src/lib/server/serve.ts +++ b/src/lib/server/serve.ts @@ -158,13 +158,24 @@ export interface ServeOptions { protectedResource?: ProtectedResourceMetadata | ((host: string) => ProtectedResourceMetadata); /** - * Trust `X-Forwarded-Host` / `X-Forwarded-Proto` for host resolution - * and for reconstructing the public URL on 401 challenges. Default - * `false`. Enable when `serve()` sits behind a proxy that sanitizes - * these headers (Fly.io, Cloud Run, an internal ALB that overwrites - * them). Leaving this off on the open internet lets an attacker pick - * the advertised OAuth `resource` URL by spoofing the header — so the - * framework ignores the forwarded value unless you opt in. + * Trust `X-Forwarded-Host` and RFC 7239 `Forwarded: host=...` for + * host resolution and for reconstructing the public URL on 401 + * challenges. Default `false` — an attacker-controlled header can't + * flip the advertised OAuth `resource` URL unless you opt in. + * + * **Your proxy must OVERWRITE the forwarded headers on ingress, not + * append.** The framework trusts the first (left-most) entry in a + * chain. With an overwriting proxy that's the sanitized value; with + * an appending proxy it's whatever the client sent — an attacker + * picks it. Common behavior: + * + * - Overwrite (safe to trust): Fly.io, Cloud Run, GCP HTTPS LB. + * - Append (NOT safe without extra config): AWS ALB default, nginx + * default. These need `proxy_set_header X-Forwarded-Host $host;` + * or equivalent before enabling this flag. + * + * Verify your proxy's behavior against a request that already has + * `X-Forwarded-Host: attacker.example` in it before turning this on. */ trustForwardedHost?: boolean; @@ -578,32 +589,117 @@ function validatePublicUrl(publicUrl: string, mountPath: string): string { return parsed.origin; } +/** + * Strip the port from a `host` string (`"snap.example.com:3001"` → + * `"snap.example.com"`). Use inside `publicUrl` / `protectedResource` + * resolvers to build scheme://host URLs without carrying the test-time + * port into production URLs. IPv6 brackets preserved. + */ +export function hostname(host: string): string { + if (host.startsWith('[')) { + // IPv6 — preserve brackets, drop port after the closing bracket. + const end = host.indexOf(']'); + if (end === -1) return host; + return host.slice(0, end + 1); + } + const colon = host.lastIndexOf(':'); + return colon === -1 ? host : host.slice(0, colon); +} + /** * Resolve the canonical host the request arrived on for multi-host * routing and per-host PRM / audience advertisement. * - * `X-Forwarded-Host` is trusted only when `trustForwardedHost: true` — - * otherwise an attacker could flip the advertised OAuth `resource` URL - * by spoofing one header. When a comma-separated forwarded chain is - * present the first (left-most) entry is the client-reported origin — - * use it and let the operator's upstream sanitize what it forwards. + * When `trustForwardedHost: true`, consults in order: + * 1. `X-Forwarded-Host` (most common — Fly, Cloud Run, most ALBs). + * 2. RFC 7239 `Forwarded: host=...` (spec-standard, less common). + * 3. `Host`. + * + * When `trustForwardedHost: false` (default), only `Host` is read — an + * attacker-controlled forwarded header can't flip the advertised OAuth + * `resource` URL. + * + * **Proxy behavior matters when you opt in.** The framework trusts the + * FIRST entry in a forwarded chain. That's safe when your proxy + * OVERWRITES the header on ingress (rewriting the original value from + * the client). It's UNSAFE when your proxy APPENDS — the attacker gets + * to pick the first entry. Fly, Cloud Run, and GCP HTTPS LBs overwrite; + * AWS ALB and nginx (by default) append. Verify your proxy's behavior + * before enabling `trustForwardedHost`. * * Normalizes to lowercase and preserves port. Returns empty string when - * neither header is present (HTTP/1.1 requires `Host`, so this is + * no usable header is present (HTTP/1.1 requires `Host`, so this is * unusual — factories can branch on it to fail closed). */ function resolveHost(req: IncomingMessage, trustForwardedHost: boolean): string { if (trustForwardedHost) { - const forwarded = firstHeaderValue(req.headers['x-forwarded-host']); - if (forwarded) { - const first = forwarded.split(',')[0]?.trim(); + const xfh = firstHeaderValue(req.headers['x-forwarded-host']); + if (xfh) { + const first = xfh.split(',')[0]?.trim(); if (first) return first.toLowerCase(); } + const forwarded = firstHeaderValue(req.headers['forwarded']); + if (forwarded) { + const host = parseForwardedHost(forwarded); + if (host) return host.toLowerCase(); + } } const host = firstHeaderValue(req.headers['host']); return host ? host.toLowerCase() : ''; } +/** + * Extract the `host=` parameter from an RFC 7239 `Forwarded:` header. + * Takes the first (left-most) hop — same policy as the `X-Forwarded-Host` + * chain rule above, and subject to the same "your proxy must overwrite, + * not append" guarantee. + * + * Handles quoted-string values per RFC 7239 §4 (IPv6 literals and hosts + * with ports must be quoted). Returns undefined when the header has no + * `host` parameter, a malformed value, or a blank host. + */ +function parseForwardedHost(header: string): string | undefined { + // Multi-hop: `Forwarded: for=1;host=a.example, for=2;host=b.example` — + // first hop is the client-facing proxy's view. Commas INSIDE quoted + // strings aren't separators, so a naive split would mis-parse IPv6. + // Scan manually, respecting quotes. + const firstHop = extractFirstForwardedHop(header); + if (!firstHop) return undefined; + for (const pair of firstHop.split(';')) { + const eq = pair.indexOf('='); + if (eq === -1) continue; + const key = pair.slice(0, eq).trim().toLowerCase(); + if (key !== 'host') continue; + let value = pair.slice(eq + 1).trim(); + if (value.startsWith('"') && value.endsWith('"') && value.length >= 2) { + value = value.slice(1, -1).replace(/\\(.)/g, '$1'); + } + return value || undefined; + } + return undefined; +} + +function extractFirstForwardedHop(header: string): string | undefined { + let depth = 0; + let start = 0; + for (let i = 0; i < header.length; i++) { + const ch = header[i]; + if (ch === '"') { + // Skip the quoted run, respecting backslash escapes. + i++; + while (i < header.length && header[i] !== '"') { + if (header[i] === '\\') i++; + i++; + } + continue; + } + if (ch === ',' && depth === 0) { + return header.slice(start, i); + } + } + return header.slice(start).trim() || undefined; +} + function firstHeaderValue(value: string | string[] | undefined): string | undefined { if (typeof value === 'string') return value; if (Array.isArray(value) && value.length > 0) return value[0]; diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index f754d76cd..22b520166 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -395,6 +395,115 @@ describe('serve() multi-host', () => { server.close(); }); + test('hostname() helper strips port (including IPv6 brackets)', () => { + const { hostname } = require('../../dist/lib/index.js'); + assert.strictEqual(hostname('snap.example.com'), 'snap.example.com'); + assert.strictEqual(hostname('snap.example.com:3001'), 'snap.example.com'); + assert.strictEqual(hostname('[::1]'), '[::1]'); + assert.strictEqual(hostname('[::1]:3001'), '[::1]'); + assert.strictEqual(hostname('[2001:db8::1]:8080'), '[2001:db8::1]'); + }); + + test('honors RFC 7239 Forwarded: host= when trustForwardedHost: true', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + trustForwardedHost: true, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `internal.fly:${port}`, + headers: { forwarded: 'for=1.2.3.4;host=snap.example.com;proto=https' }, + }); + + assert.strictEqual(seen[0], 'snap.example.com'); + + server.close(); + }); + + test('RFC 7239 Forwarded: picks first hop, strips quotes, handles IPv6', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + trustForwardedHost: true, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + // Quoted host (RFC 7239 §4 — IPv6 and hosts-with-ports must be quoted). + await request(port, { + host: `internal.fly:${port}`, + headers: { forwarded: 'host="snap.example.com:8443"' }, + }); + assert.strictEqual(seen[0], 'snap.example.com:8443'); + + // Multi-hop — first entry is the client-facing proxy. + await request(port, { + host: `internal.fly:${port}`, + headers: { forwarded: 'for=1;host=first.example, for=2;host=second.example' }, + }); + assert.strictEqual(seen[1], 'first.example'); + + server.close(); + }); + + test('X-Forwarded-Host takes precedence over Forwarded: when both set', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { + port: 0, + trustForwardedHost: true, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `internal.fly:${port}`, + headers: { + 'x-forwarded-host': 'xfh.example.com', + forwarded: 'host=forwarded.example.com', + }, + }); + assert.strictEqual(seen[0], 'xfh.example.com'); + + server.close(); + }); + + test('RFC 7239 ignored when trustForwardedHost: false', async () => { + const seen = []; + const factory = ctx => { + seen.push(ctx.host); + return new McpServer({ name: 'Test', version: '1.0.0' }); + }; + const server = serve(factory, { port: 0, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { + host: `real.example.com:${port}`, + headers: { forwarded: 'host=attacker.example.com' }, + }); + assert.strictEqual(seen[0], `real.example.com:${port}`); + + server.close(); + }); + test('function-form publicUrl with no protectedResource is allowed', async () => { // publicUrl-only mode (no PRM advertising). Factory sees the host so // an adapter can pick a handler set without ever publishing OAuth. From 7b09f5dba76402c805af4f27be60a48c75f7efb9 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 13:21:30 -0400 Subject: [PATCH 07/10] feat(server): export resolveHost + multi-host Express OAuth walkthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ship the targeted OAuth follow-ups asked for on the multi-host review: - Export `resolveHost(req, { trustForwardedHost? })` from root and `@adcp/client/server`. Same logic serve() uses internally — matches the hostname() export shape so callers writing their own host-dispatch middleware (behind createExpressAdapter) get the same attacker-header- flip hardening without re-implementing X-Forwarded-Host / RFC 7239 / overwrite-vs-append semantics. - New SKILL.md section: "Multi-host Express with per-host OAuth AS". End-to-end recipe showing host dispatch → per-host Express Router → createExpressAdapter → mcpAuthRouter with provider stub → verifyBearer, all on one process. Covers both provider shapes: mint-your-own-JWT (straightforward, verifyBearer applies) and pass-through-upstream- platform-token (adapter-agent pattern, needs introspection — tracked as #902). - 1 new test: resolveHost() standalone semantics (default ignore X-Forwarded-Host, trustForwardedHost: true picks first entry with lowercase+port, RFC 7239 Forwarded fallback, empty on no Host). Follow-up issues filed from the review: - #901: serve() AdcpServer reuse mode (caching-unfriendly today) - #902: verifyIntrospection helper for upstream-token pass-through agents Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/build-seller-agent/SKILL.md | 184 ++++++++++++++++++++++++++++- src/lib/index.ts | 1 + src/lib/server/index.ts | 2 +- src/lib/server/serve.ts | 31 ++++- test/lib/serve-multihost.test.js | 49 ++++++++ 5 files changed, 259 insertions(+), 8 deletions(-) diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index 3698a8121..d3bfa8a41 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -1277,7 +1277,189 @@ app.post('/api/snap/mcp', async (req, res) => { app.listen(3001); ``` -`createExpressAdapter` gives you four pieces `serve()` would otherwise handle: `rawBodyVerify` (for signed requests), `protectedResourceMiddleware` (RFC 9728 at origin root, not inside the router), `getUrl` (reconstructs the canonical URL with Express's stripped mount prefix — pass to `verifySignatureAsAuthenticator`), and `resetHook` (compliance state reset between storyboards). Scale it to many hostnames with one Express Router per host, dispatched by a Host-header middleware — the 13-adapter pattern. +`createExpressAdapter` gives you four pieces `serve()` would otherwise handle: `rawBodyVerify` (for signed requests), `protectedResourceMiddleware` (RFC 9728 at origin root, not inside the router), `getUrl` (reconstructs the canonical URL with Express's stripped mount prefix — pass to `verifySignatureAsAuthenticator`), and `resetHook` (compliance state reset between storyboards). + +### Multi-host Express with per-host OAuth AS + +The shape when one Node process fronts N hostnames AND each hostname is also its own OAuth 2.1 Authorization Server — the common pattern for white-label sellers, multi-platform adapter fleets (one process → Snap, Meta, TikTok, …), retail media networks with per-brand issuers, etc. `serve()`'s multi-host mode doesn't cover this because it has no composition surface for AS routes ([#887](https://github.com/adcontextprotocol/adcp-client/issues/887)); `createExpressAdapter` does, and the pieces compose as follows. + +```typescript +import express from 'express'; +import { + createAdcpServer, + createExpressAdapter, + verifyBearer, + resolveHost, + hostname, + UnknownHostError, +} from '@adcp/client/server'; +import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js'; +import { mcpAuthRouter } from '@modelcontextprotocol/sdk/server/auth/router.js'; + +// One entry per hostname you front. Each carries the per-host +// configuration AND the per-host OAuth provider — the provider is +// what `mcpAuthRouter` invokes for authorize/token/introspect. +const adapters = new Map([ + ['snap.agentic-adapters.example.com', snapConfig], + ['meta.agentic-adapters.example.com', metaConfig], + // ... 13 total in the deployment this recipe was designed around +]); + +const app = express(); + +// Per-host Express Router, built once at module load and cached. +// The Router carries: express.json (with rawBody capture), the OAuth +// AS routes, and the MCP endpoint. +const routersByHost = new Map(); +for (const [host, cfg] of adapters) { + const agent = createAdcpServer({ + name: cfg.name, + version: '1.0.0', + resolveAccount: async (ref, { authInfo }) => cfg.lookupAccount(ref, authInfo), + mediaBuy: cfg.handlers, + }); + + const adapter = createExpressAdapter({ + mountPath: `/api/${cfg.slug}`, + publicUrl: `https://${host}/api/${cfg.slug}/mcp`, + prm: { authorization_servers: [`https://${host}/oauth`] }, + server: agent, + }); + + const router = express.Router(); + + // Raw-body capture: express.json() drains the stream, but RFC 9421 + // signature verification needs the exact bytes that were signed. + router.use(express.json({ limit: '5mb', verify: adapter.rawBodyVerify })); + + // OAuth 2.1 AS routes — authorize, token, register, introspect, etc. + // `cfg.createOAuthProvider()` returns an OAuthServerProvider + // specific to this platform (Snap's, Meta's, …). See the two + // provider sketches below for the common shapes. + router.use( + '/oauth', + mcpAuthRouter({ + provider: cfg.createOAuthProvider(), + issuerUrl: new URL(`https://${host}/oauth`), + }) + ); + + // MCP endpoint. verifyBearer's audience is tied to THIS host's + // publicUrl so a token minted for snap.example.com can't be replayed + // at meta.example.com. + const authenticate = verifyBearer({ + jwksUri: `https://${host}/oauth/.well-known/jwks.json`, + issuer: `https://${host}/oauth`, + audience: `https://${host}/api/${cfg.slug}/mcp`, + }); + + router.post(`/api/${cfg.slug}/mcp`, async (req, res) => { + const principal = await authenticate(req); + if (!principal) { + res.status(401).end(); + return; + } + (req as any).auth = { + token: principal.token, + clientId: principal.principal, + scopes: principal.scopes, + }; + const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined }); + try { + await agent.connect(transport); + await transport.handleRequest(req, res, req.body); + } finally { + transport.close(); + } + }); + + routersByHost.set(host, router); +} + +// PRM lives at the origin root, BEFORE the host-dispatch middleware — +// the OAuth grader probes `/.well-known/oauth-protected-resource/` +// at the top-level app. Each adapter's `protectedResourceMiddleware` +// handles its own probe path; the fall-through ordering matters. +for (const cfg of adapters.values()) { + const adapter = createExpressAdapter({ + mountPath: `/api/${cfg.slug}`, + publicUrl: `https://${cfg.host}/api/${cfg.slug}/mcp`, + prm: { authorization_servers: [`https://${cfg.host}/oauth`] }, + }); + app.use(adapter.protectedResourceMiddleware); +} + +// Host dispatch — resolveHost mirrors serve()'s X-Forwarded-Host / +// Forwarded / append-vs-replace semantics, so this middleware closes +// the same attacker-header-flip hole serve() does. +app.use((req, res, next) => { + const host = resolveHost(req, { trustForwardedHost: true }); + if (!host) { + res.status(400).end(); + return; + } + const router = routersByHost.get(host); + if (!router) { + // UnknownHostError shape — keep the routing table off the wire. + res.status(404).end(); + return; + } + router(req, res, next); +}); + +app.listen(3001); +``` + +#### OAuth provider shape 1: mint your own JWTs + +The straightforward case. You own the IdP, issue JWTs with `aud` bound to `publicUrl`, and `verifyBearer` validates them against your JWKS. + +```typescript +// cfg.createOAuthProvider() returns an OAuthServerProvider that: +// - authorize(): runs your login + consent flow, redirects with a code +// - exchangeAuthorizationCode(): verifies PKCE, mints a JWT with +// `iss = https:///oauth`, `aud = https:///api//mcp`, +// `sub = `, `scope = ` +// - revokeToken(), introspect(): as needed +// JWKS is published under the same host so verifyBearer's remote fetch +// can discover the signing key. +``` + +#### OAuth provider shape 2: pass-through upstream platform tokens + +Used when the adapter is a thin proxy over an external IdP (Snap OAuth, Meta OAuth, etc.) and the bearer clients present IS the upstream platform's access token. Your AS is an orchestrator, not an issuer. + +```typescript +// cfg.createOAuthProvider() returns an OAuthServerProvider that: +// - authorize(): redirects to the upstream platform's authorize +// endpoint, captures the platform's callback, persists the +// platform's access/refresh token keyed by YOUR auth code +// - exchangeAuthorizationCode(): returns the stored UPSTREAM access +// token as the Bearer (not a freshly-minted JWT) +// Bearer verification on the MCP endpoint is NOT `verifyBearer({...})` +// — the token isn't your JWT. Options: +// a. Upstream JWKS: if the platform publishes one and signs JWTs, +// verifyBearer against THEIR JWKS + audience. +// b. Introspection (RFC 7662): POST the token to the platform's +// introspection endpoint, check `active: true` + scopes. +// c. Opaque-token cache: short-lived in-memory cache of verified +// tokens, keyed by hash(token), with a TTL. +// Tracked as a follow-up — see #902 for a +// `verifyIntrospection({ introspectionUrl })` helper. +``` + +Both shapes compose with the multi-host Express scaffold above — swap the provider, the rest stays. + +#### Why this scaffold, not `serve()` + +`serve()`'s multi-host mode handles the PR side cleanly but has no hook for AS routes. Going to `createExpressAdapter` gives you: + +- `rawBodyVerify` for signed-request verification (you'd otherwise have to re-implement express.json body-capture integration) +- `protectedResourceMiddleware` at the origin root (OAuth graders probe here; mounting inside the router makes them 404) +- `getUrl` for signature verifier audience reconstruction (Express strips the mount prefix from `req.url`) +- `resetHook` for conformance-runner storyboard resets + +You own: the `mcpAuthRouter` wiring (provider-specific), the per-request `transport.connect()` + `handleRequest()` dance, the host-dispatch middleware. `resolveHost` + `hostname` + `UnknownHostError` from `@adcp/client/server` give you the same security posture as `serve()`'s internal resolution — export and reuse rather than re-deriving. ### Stdio diff --git a/src/lib/index.ts b/src/lib/index.ts index f20908752..95b6dcdb4 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -548,6 +548,7 @@ export { serve, UnknownHostError, hostname, + resolveHost, registerTestController, handleTestControllerRequest, TestControllerError, diff --git a/src/lib/server/index.ts b/src/lib/server/index.ts index 90f8a55da..f6c3323dc 100644 --- a/src/lib/server/index.ts +++ b/src/lib/server/index.ts @@ -96,7 +96,7 @@ export type { SeedFixtureCache, } from './test-controller'; -export { serve, UnknownHostError, hostname } from './serve'; +export { serve, UnknownHostError, hostname, resolveHost } from './serve'; export type { ServeContext, ServeOptions, ProtectedResourceMetadata } from './serve'; export { createExpressAdapter } from './express-adapter'; diff --git a/src/lib/server/serve.ts b/src/lib/server/serve.ts index bb214eee8..e1c1266a1 100644 --- a/src/lib/server/serve.ts +++ b/src/lib/server/serve.ts @@ -304,7 +304,7 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer const httpServer = createServer(async (req, res) => { const { pathname } = new URL(req.url || '', 'http://localhost'); - const host = resolveHost(req, trustForwardedHost); + const host = resolveHost(req, { trustForwardedHost }); // RFC 9728 protected-resource metadata — intentionally auth-free so // clients can discover the authorization server before they have a token. @@ -608,9 +608,12 @@ export function hostname(host: string): string { /** * Resolve the canonical host the request arrived on for multi-host - * routing and per-host PRM / audience advertisement. + * routing and per-host PRM / audience advertisement. Same logic + * `serve()` uses internally — exported so callers writing their own + * host-dispatch middleware (e.g., behind `createExpressAdapter`) can + * match `serve()`'s semantics exactly instead of re-implementing them. * - * When `trustForwardedHost: true`, consults in order: + * When `options.trustForwardedHost: true`, consults in order: * 1. `X-Forwarded-Host` (most common — Fly, Cloud Run, most ALBs). * 2. RFC 7239 `Forwarded: host=...` (spec-standard, less common). * 3. `Host`. @@ -619,7 +622,7 @@ export function hostname(host: string): string { * attacker-controlled forwarded header can't flip the advertised OAuth * `resource` URL. * - * **Proxy behavior matters when you opt in.** The framework trusts the + * **Proxy behavior matters when you opt in.** The helper trusts the * FIRST entry in a forwarded chain. That's safe when your proxy * OVERWRITES the header on ingress (rewriting the original value from * the client). It's UNSAFE when your proxy APPENDS — the attacker gets @@ -629,9 +632,25 @@ export function hostname(host: string): string { * * Normalizes to lowercase and preserves port. Returns empty string when * no usable header is present (HTTP/1.1 requires `Host`, so this is - * unusual — factories can branch on it to fail closed). + * unusual — callers can branch on it to fail closed). + * + * @example + * ```ts + * import express from 'express'; + * import { resolveHost } from '@adcp/client/server'; + * + * const routersByHost = new Map(); + * + * function hostDispatch(req, res, next) { + * const host = resolveHost(req, { trustForwardedHost: true }); + * const router = routersByHost.get(host); + * if (!router) return res.status(404).end(); + * return router(req, res, next); + * } + * ``` */ -function resolveHost(req: IncomingMessage, trustForwardedHost: boolean): string { +export function resolveHost(req: IncomingMessage, options?: { trustForwardedHost?: boolean }): string { + const trustForwardedHost = options?.trustForwardedHost === true; if (trustForwardedHost) { const xfh = firstHeaderValue(req.headers['x-forwarded-host']); if (xfh) { diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index 22b520166..a94a1548f 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -395,6 +395,55 @@ describe('serve() multi-host', () => { server.close(); }); + test('resolveHost() export matches serve()s internal resolution', () => { + const { resolveHost } = require('../../dist/lib/index.js'); + + // Default (no options) ignores X-Forwarded-Host. + assert.strictEqual( + resolveHost({ headers: { host: 'real.example.com', 'x-forwarded-host': 'attacker.example.com' } }), + 'real.example.com' + ); + + // Options-bag, trustForwardedHost: false — same as default. + assert.strictEqual( + resolveHost( + { headers: { host: 'real.example.com', 'x-forwarded-host': 'attacker.example.com' } }, + { trustForwardedHost: false } + ), + 'real.example.com' + ); + + // Trust on: X-Forwarded-Host wins. + assert.strictEqual( + resolveHost( + { headers: { host: 'internal.fly', 'x-forwarded-host': 'snap.example.com' } }, + { trustForwardedHost: true } + ), + 'snap.example.com' + ); + + // Trust on: X-Forwarded-Host first-entry wins, lowercase, port preserved. + assert.strictEqual( + resolveHost( + { headers: { host: 'internal.fly', 'x-forwarded-host': 'SNAP.example.com:8443, cdn.example.com' } }, + { trustForwardedHost: true } + ), + 'snap.example.com:8443' + ); + + // Trust on: RFC 7239 Forwarded fallback. + assert.strictEqual( + resolveHost( + { headers: { host: 'internal.fly', forwarded: 'host=snap.example.com' } }, + { trustForwardedHost: true } + ), + 'snap.example.com' + ); + + // Empty on no Host header at all. + assert.strictEqual(resolveHost({ headers: {} }, { trustForwardedHost: true }), ''); + }); + test('hostname() helper strips port (including IPv6 brackets)', () => { const { hostname } = require('../../dist/lib/index.js'); assert.strictEqual(hostname('snap.example.com'), 'snap.example.com'); From 87cec36628a5ea49bd0d0c6bed2b25d1d15c536b Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 14:20:46 -0400 Subject: [PATCH 08/10] feat(server): reuseAgent mode (#901) + verifyIntrospection (#902) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close two follow-ups from the multi-host review. Both expert-reviewed by protocol + security + code reviewers before merge. **#901 — reuseAgent: true** Opt-in flag on ServeOptions. When set, the factory can cache AdcpServer instances per host; the framework wraps connect→handleRequest→close in a per-instance async mutex (MCP's Protocol.connect() rejects when a transport is already attached, confirmed in protocol.js:215). Concurrent requests on DIFFERENT cached servers still run in parallel. Default behavior (fresh-server-per-request) unchanged. Cross-request isolation verified by a dedicated test: the MCP SDK reads req.auth per-invocation and threads it through RequestHandlerExtra, so authInfo can't bleed across requests on a shared instance (dispatcher path verified in create-adcp-server.ts:2116-2118 + adcp-server.ts wrapping). 6 new tests covering factory-caching contract, cross-request auth isolation, concurrent-same-server serialization, concurrent- different-servers parallelism, same-instance reuse invariant, and factory-throw-doesn't-poison-chain. **#902 — verifyIntrospection** RFC 7662 bearer introspection authenticator for adapter agents that proxy upstream platform OAuth (Snap, Meta, TikTok) rather than minting their own JWTs. Matches verifyBearer's Authenticator shape: returns null on missing bearer (anyOf fall-through), throws AuthError on reject with a sanitized public message (upstream body never crosses the wire). Features: - RFC 6749 §2.3.1 form-urlencoded Basic auth (clientId + secret percent-encoded before base64, including !*'() per the full form- urlencoded grammar — verifyable via a test that exercises characters the encoder transforms). Alternate clientAuth: 'body' for legacy ASes that compare raw secrets. - TTL-capped positive cache keyed on SHA-256(token) — never stores raw bearer in memory. TTL capped at token's own `exp` so the cache can't extend a revoked/expired token. Negative caching off by default, opt-in via negativeTtlSeconds. - Loopback allowlist for http:// dev URLs (localhost, ::1, 127.0.0.0/8). - 2s default timeout, fail-closed on network errors + non-JSON + HTTP 5xx + missing `active` field. - Optional audience check (opt-in) — most upstream access tokens (Snap, Meta) don't populate `aud`, so default off is correct for the adapter use case. - Deep-clone of claims on cache set via structuredClone — caller mutations on returned principal can't poison subsequent lookups. - 29 tests covering construction, flow, audience, errors, cache semantics (TTL cap, LRU eviction, mutation isolation, negative caching), and anyOf fall-through composition. **SKILL.md** — updated the multi-host Express recipe to show verifyIntrospection for pass-through provider shape, and the reuseAgent pattern with a full cached-factory + SIGTERM cleanup example. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/multihost-serve.md | 21 + skills/build-seller-agent/SKILL.md | 89 ++++- src/lib/index.ts | 1 + src/lib/server/auth-introspection.ts | 416 ++++++++++++++++++++ src/lib/server/index.ts | 7 + src/lib/server/serve.ts | 135 +++++-- test/lib/serve-multihost.test.js | 296 +++++++++++++- test/server-auth-introspection.test.js | 509 +++++++++++++++++++++++++ 8 files changed, 1427 insertions(+), 47 deletions(-) create mode 100644 src/lib/server/auth-introspection.ts create mode 100644 test/server-auth-introspection.test.js diff --git a/.changeset/multihost-serve.md b/.changeset/multihost-serve.md index 0e69a36af..2c17c514e 100644 --- a/.changeset/multihost-serve.md +++ b/.changeset/multihost-serve.md @@ -24,4 +24,25 @@ New `UnknownHostError` class — throw it from the factory (or `publicUrl`/ New `getServeRequestContext(req)` helper exposes the resolved `{ host, publicUrl }` to custom authenticators wired outside `verifyBearer`. +New `resolveHost(req, { trustForwardedHost? })` and `hostname(host)` exports +— same logic `serve()` uses internally, so callers building their own +host-dispatch middleware behind `createExpressAdapter` don't re-implement the +X-Forwarded-Host / RFC 7239 Forwarded / overwrite-vs-append hardening. + +New `reuseAgent: true` on `ServeOptions` — lets the factory cache +`AdcpServer` instances per host instead of reconstructing on every request. +The framework wraps connect→handleRequest→close in a per-instance async +mutex because MCP's `Protocol.connect()` rejects when a transport is +already attached. Concurrent requests on different cached servers still +run in parallel. Closes #901. + +New `verifyIntrospection({ introspectionUrl, clientId, clientSecret, … })` +authenticator — RFC 7662 bearer validation for adapter agents that proxy +upstream platform OAuth (Snap, Meta, TikTok, …) rather than minting their +own JWTs. Matches `verifyBearer`'s shape (`null` on missing bearer, throws +`AuthError` on reject). Features: TTL-capped positive cache keyed on SHA-256 +of the token, opt-in negative caching, RFC 6749 §2.3.1 form-urlencoded Basic +auth, fail-closed on upstream errors/timeouts, optional `requiredScopes` and +`audience` checks. Closes #902. + Closes #885. diff --git a/skills/build-seller-agent/SKILL.md b/skills/build-seller-agent/SKILL.md index d3bfa8a41..85cbf56a0 100644 --- a/skills/build-seller-agent/SKILL.md +++ b/skills/build-seller-agent/SKILL.md @@ -1204,7 +1204,39 @@ Each unique host runs its resolver once and the result is cached. Every host adv **Unknown hosts: throw `UnknownHostError` from the factory.** `serve()` catches it and responds 404 with a generic body (the routing table never crosses the wire). Throwing any other `Error` stays as a 500 so unrelated bugs remain loud. -**Factory runs per request.** `serve()` calls the factory on every incoming request (to avoid cross-request state bleed) and closes the returned server at the end. Keep the factory cheap: look up a pre-built adapter config from a module-scoped `Map`, and let `createAdcpServer(...)` build a fresh wrapper from that config. Do NOT cache the `AdcpServer` instance across requests — `serve()` closes it after each call, so the cache would be stale on request 2. If per-request `createAdcpServer` cost is a measurable bottleneck, track [#901](https://github.com/adcontextprotocol/adcp-client/issues/901) — a reuse mode is planned. +**Factory runs per request.** `serve()` calls the factory on every incoming request (to avoid cross-request state bleed). By default it closes the returned server at the end of each request — so caching the `AdcpServer` from one call to the next is unsafe without opt-in. Keep the default-path factory cheap: look up a pre-built adapter config from a module-scoped `Map`, and let `createAdcpServer(...)` build a fresh wrapper from that config on every call. + +**Pass `reuseAgent: true` to cache `AdcpServer` instances per host.** When the tool-registration cost inside `createAdcpServer(...)` is a measurable part of request latency (common in multi-host deployments with many tools per host), flip the flag and cache the returned server in the factory: + +```typescript +const agents = new Map(); +serve( + ctx => { + let agent = agents.get(ctx.host); + if (!agent) { + const cfg = adapters.get(ctx.host); + if (!cfg) throw new UnknownHostError(`No adapter for ${ctx.host}`); + agent = createAdcpServer({ + name: cfg.name, + version: '1.0.0', + resolveAccount: cfg.resolveAccount, + mediaBuy: cfg.handlers, + }); + agents.set(ctx.host, agent); + } + return agent; + }, + { reuseAgent: true /* ...other options... */ } +); + +// Cleanup on shutdown — reuseAgent mode doesn't auto-close. +process.on('SIGTERM', async () => { + await Promise.all([...agents.values()].map(a => a.close())); + process.exit(0); +}); +``` + +The framework wraps the `connect → handleRequest → close-transport` cycle in a per-instance async mutex. Concurrent requests on the SAME cached server serialize (MCP's `Protocol.connect()` throws when a transport is already attached, so serialization is mandatory for safety); concurrent requests on DIFFERENT cached servers run in parallel. Trade-off: throughput per unique host drops to 1 in flight at a time. If a single host regularly serves concurrent requests where handler latency dominates, cache a small pool of servers per host and round-robin from the factory — the mutex is per-instance, so pool members don't serialize with each other. ### Express + OAuth Authorization Server in one process @@ -1427,28 +1459,47 @@ The straightforward case. You own the IdP, issue JWTs with `aud` bound to `publi #### OAuth provider shape 2: pass-through upstream platform tokens -Used when the adapter is a thin proxy over an external IdP (Snap OAuth, Meta OAuth, etc.) and the bearer clients present IS the upstream platform's access token. Your AS is an orchestrator, not an issuer. +Used when the adapter is a thin proxy over an external IdP (Snap OAuth, Meta OAuth, etc.) and the Bearer clients present IS the upstream platform's access token. Your AS is an orchestrator, not an issuer — the `OAuthServerProvider` persists the upstream access/refresh token keyed by your auth code, and `exchangeAuthorizationCode()` returns the stored upstream token rather than a freshly-minted JWT. + +Bearer verification is NOT `verifyBearer({...})` — the token isn't your JWT. Use `verifyIntrospection` (RFC 7662) when the upstream exposes an introspection endpoint: ```typescript -// cfg.createOAuthProvider() returns an OAuthServerProvider that: -// - authorize(): redirects to the upstream platform's authorize -// endpoint, captures the platform's callback, persists the -// platform's access/refresh token keyed by YOUR auth code -// - exchangeAuthorizationCode(): returns the stored UPSTREAM access -// token as the Bearer (not a freshly-minted JWT) -// Bearer verification on the MCP endpoint is NOT `verifyBearer({...})` -// — the token isn't your JWT. Options: -// a. Upstream JWKS: if the platform publishes one and signs JWTs, -// verifyBearer against THEIR JWKS + audience. -// b. Introspection (RFC 7662): POST the token to the platform's -// introspection endpoint, check `active: true` + scopes. -// c. Opaque-token cache: short-lived in-memory cache of verified -// tokens, keyed by hash(token), with a TTL. -// Tracked as a follow-up — see #902 for a -// `verifyIntrospection({ introspectionUrl })` helper. +import { verifyIntrospection } from '@adcp/client/server'; + +const authenticate = verifyIntrospection({ + introspectionUrl: 'https://accounts.snapchat.com/oauth2/introspect', + // Introspection endpoints are ALWAYS client-authenticated (RFC 7662 §2.1); + // provision an introspection-capable client with the upstream IdP. + clientId: process.env.SNAP_INTROSPECTION_CLIENT_ID!, + clientSecret: process.env.SNAP_INTROSPECTION_CLIENT_SECRET!, + // Scopes to require on the upstream token. The introspection response's + // `scope` string must contain ALL of these. + requiredScopes: ['snapchat-marketing-api'], + // Cache positive responses to amortize the network round-trip across + // closely-spaced requests from the same buyer. TTL is capped at the + // token's own `exp` claim — the cache can't extend a token past its + // upstream-issued lifetime. Negative responses are NOT cached by + // default (a revoked token must be able to fail the next request); + // set `negativeTtlSeconds` if you accept the revocation-latency + // trade-off for DoS-amplification protection. + cache: { ttlSeconds: 60, max: 10_000 }, + // Fail-closed timeout. Default 2000ms — a slow upstream can't bypass auth. + timeoutMs: 2000, +}); + +router.post(`/api/${cfg.slug}/mcp`, async (req, res) => { + const principal = await authenticate(req); + if (!principal) { + res.status(401).end(); + return; + } + // ... rest as in shape 1 +}); ``` -Both shapes compose with the multi-host Express scaffold above — swap the provider, the rest stays. +When the upstream platform publishes a JWKS and signs access tokens as JWTs (Google, Auth0-backed IdPs, etc.), `verifyBearer({ jwksUri, issuer, audience })` against the upstream's endpoints avoids the per-request introspection round trip — stronger cryptographic verification and no upstream rate-limit exposure. Pick introspection only when the upstream uses opaque tokens. + +Both provider shapes compose with the multi-host Express scaffold above — swap the provider and the authenticator, the rest stays. #### Why this scaffold, not `serve()` diff --git a/src/lib/index.ts b/src/lib/index.ts index 95b6dcdb4..3a54c5a41 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -549,6 +549,7 @@ export { UnknownHostError, hostname, resolveHost, + verifyIntrospection, registerTestController, handleTestControllerRequest, TestControllerError, diff --git a/src/lib/server/auth-introspection.ts b/src/lib/server/auth-introspection.ts new file mode 100644 index 000000000..5f85bb7cc --- /dev/null +++ b/src/lib/server/auth-introspection.ts @@ -0,0 +1,416 @@ +/** + * RFC 7662 OAuth 2.0 Token Introspection authenticator. + * + * Use case: the agent is NOT the identity issuer. Instead it proxies an + * upstream OAuth provider (Snap, Meta, TikTok, Google, …) — the Bearer + * token a buyer presents is the upstream platform's access token, not a + * JWT the adapter minted. {@link verifyBearer} doesn't apply there; the + * only way to validate the token is to ask the upstream: POST the token + * to its RFC 7662 introspection endpoint and check `active: true`. + * + * Matches the {@link Authenticator} contract of {@link verifyBearer}: + * returns `null` when no bearer header is present (so {@link anyOf} can + * fall through), throws {@link AuthError} when a token was presented + * and rejected. Public error messages are sanitized — the upstream + * response body never crosses the wire, only a generic "Token + * validation failed." / "Insufficient scope." message on 401. + * + * **Positive-response caching is opt-in.** Introspection endpoints are + * often rate-limited and always round-trip at least one TLS handshake + * worth of latency — a small in-process LRU keyed on the token's + * SHA-256 hash amortizes the cost across closely-spaced requests from + * the same buyer. Negative responses are NOT cached by default; a + * stolen-then-revoked token must be able to fail the next request. Set + * `cache.negativeTtlSeconds` to cache `active: false` too (mitigates + * introspection-amplification DoS, trades against revocation latency). + */ +import { createHash } from 'crypto'; +import type { IncomingMessage } from 'http'; +import { AuthError, extractBearerToken, type AuthPrincipal, type AuthResult, type Authenticator } from './auth'; + +/** RFC 7662 §2.2 introspection response. Fields are all optional except `active`. */ +export interface IntrospectionResponse { + active: boolean; + scope?: string; + client_id?: string; + username?: string; + token_type?: string; + exp?: number; + iat?: number; + nbf?: number; + sub?: string; + aud?: string | string[]; + iss?: string; + jti?: string; + [extension: string]: unknown; +} + +export interface IntrospectionCacheOptions { + /** Max cached entries (positive + negative combined). Default 1024. */ + max?: number; + /** + * Seconds to cache `active: true` responses. Default 60. Must be <= + * the token's remaining lifetime — the helper caps the effective + * TTL at `exp - now` so the cache can't extend a token past its + * issued expiry. + */ + ttlSeconds?: number; + /** + * Seconds to cache `active: false` responses. Default 0 (disabled). + * Caching negatives mitigates introspection-amplification DoS from an + * attacker replaying a stolen-then-revoked token, but delays propagation + * of a fresh revocation by up to this many seconds. Keep short (≤5s). + */ + negativeTtlSeconds?: number; +} + +export interface VerifyIntrospectionOptions { + /** Absolute URL of the upstream introspection endpoint (RFC 7662 §2). */ + introspectionUrl: string; + /** + * Client credentials for the introspection call. Introspection endpoints + * ALWAYS require client authentication — without credentials the endpoint + * would be an open oracle for "is this token valid." Basic auth is the + * default (RFC 7662 §2.1); `clientAuth: 'body'` sends `client_id` + + * `client_secret` as form params instead (some IdPs require this). + */ + clientId: string; + clientSecret: string; + clientAuth?: 'basic' | 'body'; + /** + * Required scopes — the introspection response's `scope` claim (space + * delimited per RFC 7662) must contain ALL of these. Useful when the + * agent grants a subset of what the upstream IdP issues. + */ + requiredScopes?: string[]; + /** + * Optional constraint on the introspection response's `aud` claim. + * When set, the helper rejects a token whose `aud` doesn't contain + * this value. Note: many upstream platforms (Snap, Meta) don't set + * `aud` on access tokens — their tokens are bound to the client_id, + * not the resource URL. Leave undefined in that case. + */ + audience?: string; + /** + * Optional cache. Disabled by default (every request round-trips to + * the introspection endpoint). Enable for hot paths. + */ + cache?: IntrospectionCacheOptions; + /** + * Timeout for the introspection request in milliseconds. Default + * 2000ms. On timeout the authenticator throws an {@link AuthError} + * (fail-closed — a slow upstream can't bypass authentication). + */ + timeoutMs?: number; + /** + * `token_type_hint` sent with the introspection request (RFC 7662 §2.1). + * Default `'access_token'`. Most IdPs ignore the hint but setting it + * correctly is the spec-conformant move. + */ + tokenTypeHint?: 'access_token' | 'refresh_token'; +} + +interface CacheEntry { + principal: AuthResult; + expiresAt: number; // ms since epoch +} + +/** Build a token-introspection authenticator. See {@link VerifyIntrospectionOptions}. */ +export function verifyIntrospection(options: VerifyIntrospectionOptions): Authenticator { + // Resolved defaults + sanity-checks at construction so misconfig fails loud at boot. + let introspectionUrl: URL; + try { + introspectionUrl = new URL(options.introspectionUrl); + } catch { + throw new Error(`verifyIntrospection: \`introspectionUrl\` is not a valid URL: ${options.introspectionUrl}`); + } + if (introspectionUrl.protocol !== 'https:' && !isLoopbackHost(introspectionUrl.hostname)) { + throw new Error( + `verifyIntrospection: \`introspectionUrl\` must use https:// (got ${introspectionUrl.protocol}). ` + + 'Sending bearer tokens over plain HTTP to remote hosts is a credential-exposure bug.' + ); + } + if (!options.clientId || !options.clientSecret) { + throw new Error('verifyIntrospection: `clientId` and `clientSecret` are required (RFC 7662 mandates client auth).'); + } + const clientAuth = options.clientAuth ?? 'basic'; + const timeoutMs = options.timeoutMs ?? 2000; + const tokenTypeHint = options.tokenTypeHint ?? 'access_token'; + const requiredScopes = options.requiredScopes ?? []; + + const cache = options.cache ? createIntrospectionCache(options.cache) : undefined; + + // RFC 6749 §2.3.1 mandates application/x-www-form-urlencoded on the + // clientId and clientSecret BEFORE base64'ing for Basic auth. That's + // what `formUrlEncode` does (encodeURIComponent + `!*'()` escapes). + // Some legacy ASes compare the decoded Basic-auth password directly + // against the stored secret without percent-decoding — if that's + // biting you, set `clientAuth: 'body'` to send creds in the form body + // instead, which sidesteps the encoding negotiation entirely. + const basicAuthHeader = + clientAuth === 'basic' + ? `Basic ${Buffer.from(`${formUrlEncode(options.clientId)}:${formUrlEncode(options.clientSecret)}`).toString('base64')}` + : undefined; + + return async (req: IncomingMessage): Promise => { + const token = extractBearerToken(req); + if (!token) return null; + + // Cache lookup keyed on SHA-256(token) so the cache never stores + // the raw bearer in process memory. + const cacheKey = cache ? sha256(token) : undefined; + if (cache && cacheKey) { + const hit = cache.get(cacheKey); + if (hit) { + if (hit.principal === null) { + throw new AuthError('Token validation failed.'); + } + return { ...hit.principal, token }; + } + } + + let response: IntrospectionResponse; + try { + response = await introspect({ + introspectionUrl, + token, + tokenTypeHint, + clientAuth, + basicAuthHeader, + clientId: options.clientId, + clientSecret: options.clientSecret, + timeoutMs, + }); + } catch (err) { + // Network / HTTP errors — fail closed but DON'T cache. The next + // request retries. Public message is generic; cause is logged. + throw new AuthError('Token validation failed.', { cause: err }); + } + + // Inactive token. Cache the negative result if enabled — mitigates + // replay-amplification of a revoked token. Cause message is generic. + if (!response.active) { + if (cache && cacheKey && cache.negativeTtlMs > 0) { + cache.set(cacheKey, { principal: null, expiresAt: Date.now() + cache.negativeTtlMs }); + } + throw new AuthError('Token validation failed.'); + } + + // Audience check — only when the caller specified a required audience. + // Many upstream IdPs don't populate `aud` on access tokens, so the + // default "no audience required" is correct for the adapter case. + if (options.audience !== undefined) { + const aud = response.aud; + const audList = typeof aud === 'string' ? [aud] : Array.isArray(aud) ? aud : []; + if (!audList.includes(options.audience)) { + throw new AuthError('Token validation failed.'); + } + } + + const scopes = parseScope(response.scope); + if (requiredScopes.length > 0) { + const missing = requiredScopes.filter(s => !scopes.includes(s)); + if (missing.length > 0) { + throw new AuthError('Insufficient scope.'); + } + } + + const principal: AuthPrincipal = { + principal: + typeof response.sub === 'string' + ? response.sub + : typeof response.client_id === 'string' + ? response.client_id + : 'unknown', + token, + scopes, + claims: response as Record, + }; + if (typeof response.exp === 'number') principal.expiresAt = response.exp; + + if (cache && cacheKey) { + // Cap the positive TTL at the token's own remaining lifetime — + // never let the cache extend a token past its issued `exp`. When + // the IdP omits `exp`, fall back to the configured TTL. + const nowSec = Math.floor(Date.now() / 1000); + const configuredTtlMs = cache.positiveTtlMs; + const effectiveTtlMs = + typeof response.exp === 'number' + ? Math.min(configuredTtlMs, Math.max(0, (response.exp - nowSec) * 1000)) + : configuredTtlMs; + if (effectiveTtlMs > 0) { + // Deep-clone the principal for the cache entry so callers that + // mutate the returned principal can't poison the cached copy. + // `claims` is cloned with structuredClone — a shallow spread would + // leave nested objects aliased, and a malicious or buggy caller + // that set `returned.claims.scope = 'admin'` would upgrade every + // subsequent cached lookup. + const cachedPrincipal: AuthPrincipal = { + principal: principal.principal, + scopes: [...scopes], + claims: principal.claims ? structuredClone(principal.claims) : undefined, + }; + if (principal.expiresAt !== undefined) cachedPrincipal.expiresAt = principal.expiresAt; + cache.set(cacheKey, { + principal: cachedPrincipal, + expiresAt: Date.now() + effectiveTtlMs, + }); + } + } + + // Return a fresh object to callers — the cached copy is sealed behind + // the `cachedPrincipal` closure. If the caller mutates the returned + // `scopes` array or `claims` object, the cache entry stays clean + // because it was cloned at `set` time (see block above). + return principal; + }; +} + +/** + * Check whether a hostname is a loopback address. Exact-match for the + * three literal forms (`localhost`, `127.0.0.1`, `[::1]`) so attacker + * tricks like `localhost.evil.com` don't slip past — URL parser sets + * `.hostname` to the whole string, not the left-most label. + */ +function isLoopbackHost(hostname: string): boolean { + // URL parses IPv6 `[::1]` as hostname `::1` (brackets stripped). The + // 127.0.0.0/8 range is loopback per RFC 6890 — accept anything in + // that range since Node's own fetch routes it to lo0. + if (hostname === 'localhost' || hostname === '::1') return true; + const m = /^127\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/.exec(hostname); + if (!m) return false; + return m.slice(1).every(octet => { + const n = Number(octet); + return n >= 0 && n <= 255; + }); +} + +/** + * application/x-www-form-urlencoded encoding per RFC 6749 §2.3.1 and + * Appendix B. Differs from `encodeURIComponent` in that `!*'()` MUST be + * percent-encoded (those are reserved in the form-urlencoded grammar). + * Rare in real-world client credentials but spec-correct and avoids + * per-IdP quirks when a secret happens to include one of those chars. + */ +function formUrlEncode(s: string): string { + return encodeURIComponent(s).replace(/[!*'()]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); +} + +// --------------------------------------------------------------------------- +// Introspection transport +// --------------------------------------------------------------------------- + +async function introspect(args: { + introspectionUrl: URL; + token: string; + tokenTypeHint: 'access_token' | 'refresh_token'; + clientAuth: 'basic' | 'body'; + basicAuthHeader: string | undefined; + clientId: string; + clientSecret: string; + timeoutMs: number; +}): Promise { + const body = new URLSearchParams(); + body.set('token', args.token); + body.set('token_type_hint', args.tokenTypeHint); + if (args.clientAuth === 'body') { + body.set('client_id', args.clientId); + body.set('client_secret', args.clientSecret); + } + + const headers: Record = { + 'content-type': 'application/x-www-form-urlencoded', + accept: 'application/json', + }; + if (args.basicAuthHeader) headers.authorization = args.basicAuthHeader; + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), args.timeoutMs); + try { + const res = await fetch(args.introspectionUrl, { + method: 'POST', + headers, + body, + signal: controller.signal, + }); + if (!res.ok) { + // 4xx/5xx — don't treat as `active: false`; that would mask a + // misconfigured client credential as a revoked token. + const statusText = res.statusText || 'no status text'; + throw new Error(`introspection endpoint returned HTTP ${res.status} (${statusText})`); + } + const ct = res.headers.get('content-type') || ''; + if (!ct.includes('application/json')) { + throw new Error(`introspection endpoint returned non-JSON content-type: ${ct}`); + } + const json = (await res.json()) as IntrospectionResponse; + if (typeof json?.active !== 'boolean') { + throw new Error('introspection response missing required `active` field'); + } + return json; + } finally { + clearTimeout(timer); + } +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function sha256(input: string): string { + return createHash('sha256').update(input).digest('hex'); +} + +function parseScope(scope: unknown): string[] { + if (typeof scope !== 'string' || scope.length === 0) return []; + return scope.split(/\s+/).filter(Boolean); +} + +// --------------------------------------------------------------------------- +// Cache +// --------------------------------------------------------------------------- + +interface IntrospectionCacheHandle { + get(key: string): CacheEntry | undefined; + set(key: string, entry: CacheEntry): void; + readonly positiveTtlMs: number; + readonly negativeTtlMs: number; +} + +/** + * Small insertion-ordered LRU. Map iteration order is insertion order; to + * bump an entry on access, delete + re-set. Keeps the implementation + * dependency-free. + */ +function createIntrospectionCache(options: IntrospectionCacheOptions): IntrospectionCacheHandle { + const max = options.max ?? 1024; + const positiveTtlMs = (options.ttlSeconds ?? 60) * 1000; + const negativeTtlMs = (options.negativeTtlSeconds ?? 0) * 1000; + const store = new Map(); + + return { + positiveTtlMs, + negativeTtlMs, + get(key) { + const entry = store.get(key); + if (!entry) return undefined; + if (entry.expiresAt <= Date.now()) { + store.delete(key); + return undefined; + } + // Bump LRU: re-insert at the tail. + store.delete(key); + store.set(key, entry); + return entry; + }, + set(key, entry) { + if (store.has(key)) store.delete(key); + store.set(key, entry); + while (store.size > max) { + const oldest = store.keys().next().value; + if (oldest === undefined) break; + store.delete(oldest); + } + }, + }; +} diff --git a/src/lib/server/index.ts b/src/lib/server/index.ts index f6c3323dc..f024197cf 100644 --- a/src/lib/server/index.ts +++ b/src/lib/server/index.ts @@ -131,6 +131,13 @@ export type { ServeRequestContext, } from './auth'; +export { verifyIntrospection } from './auth-introspection'; +export type { + VerifyIntrospectionOptions, + IntrospectionCacheOptions, + IntrospectionResponse, +} from './auth-introspection'; + export { verifySignatureAsAuthenticator, requireSignatureWhenPresent, diff --git a/src/lib/server/serve.ts b/src/lib/server/serve.ts index e1c1266a1..20eedcd86 100644 --- a/src/lib/server/serve.ts +++ b/src/lib/server/serve.ts @@ -157,6 +157,42 @@ export interface ServeOptions { */ protectedResource?: ProtectedResourceMetadata | ((host: string) => ProtectedResourceMetadata); + /** + * Reuse the agent returned by the factory across requests instead of + * closing it after each. Default `false` (one fresh server per + * request — the pre-existing behavior). + * + * When `true`, `serve()` DOES NOT call `agentServer.close()` between + * requests. The factory is still called on every request; it's the + * caller's responsibility to cache `AdcpServer` instances (typically + * keyed on `ctx.host`) and return the cached instance when possible. + * Concurrent requests to the same cached server are serialized per + * instance — `McpServer.connect()` rejects when a transport is + * already attached, so the framework wraps the + * connect→handleRequest→release cycle in a per-instance async + * mutex. Requests for DIFFERENT cached servers still run in parallel. + * + * Use when `createAdcpServer(...)` setup cost (tool registration, + * handler wiring) is a measurable portion of request latency — common + * in multi-host deployments with many tools per host. Trade-off: + * throughput per unique host drops to 1 in flight at a time. For + * higher concurrency per host, cache a small pool of servers in the + * factory and round-robin. + * + * Cleanup is the caller's responsibility: listen for the process + * shutdown signal and call `close()` on every cached server to + * release MCP Tasks timers, HTTP keepalives, etc. + * + * **Incompatible with long-lived server→client streams.** MCP's + * `Protocol._onclose` aborts in-flight handlers and clears progress + * tokens when the transport closes. In stateless HTTP mode (the + * default this helper uses) each request is already single-response, + * so this is a no-op concern. Don't enable `reuseAgent` if you + * eventually wire a transport that keeps an SSE channel open across + * multiple logical requests. + */ + reuseAgent?: boolean; + /** * Trust `X-Forwarded-Host` and RFC 7239 `Forwarded: host=...` for * host resolution and for reconstructing the public URL on 401 @@ -235,6 +271,17 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer const mountPath = options?.path ?? '/mcp'; const taskStore = options?.taskStore ?? new InMemoryTaskStore(); const trustForwardedHost = options?.trustForwardedHost === true; + const reuseAgent = options?.reuseAgent === true; + + // Per-instance mutex chain for `reuseAgent: true`. The MCP SDK's + // `Protocol.connect()` hard-throws when a transport is already + // attached (node_modules/@modelcontextprotocol/sdk/dist/esm/shared/protocol.js:215), + // so concurrent requests that land on the same cached server would + // race without this. WeakMap so agents GC normally when the caller + // drops references. Only used when `reuseAgent` is true — the + // default path still creates a fresh server per request and doesn't + // share any state across calls. + const reuseMutexes = new WeakMap>(); if (options?.protectedResource && !options.publicUrl) { throw new Error( @@ -485,7 +532,16 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer res.end('Not found'); return; } - throw err; + // Unexpected factory failure — surface as 500 to the client and + // log server-side. Rethrowing would bubble to the createServer + // handler as an unhandled rejection (async callback), which + // could crash a node process with `--unhandled-rejections=strict`. + console.error('[adcp/serve] factory threw:', err); + if (!res.headersSent) { + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Internal server error' })); + } + return; } const attached = (agentServer as unknown as Record)[ADCP_PRE_TRANSPORT]; const autoWiredPreTransport = typeof attached === 'function' ? (attached as AdcpPreTransport) : undefined; @@ -515,28 +571,63 @@ export function serve(createAgent: (ctx: ServeContext) => AdcpServer | McpServer } } - const transport = new StreamableHTTPServerTransport({ - sessionIdGenerator: undefined, - }); - try { - await agentServer.connect(transport); - // When preTransport already consumed the request stream, pass the - // parsed body through so the transport doesn't re-read (stream is - // drained). MCP SDK's `handleRequest(req, res, parsedBody)` accepts - // this shape. - if (parsedBody !== undefined) { - await transport.handleRequest(req, res, parsedBody); - } else { - await transport.handleRequest(req, res); - } - } catch (err) { - console.error('Server error:', err); - if (!res.headersSent) { - res.writeHead(500, { 'Content-Type': 'application/json' }); - res.end(JSON.stringify({ error: 'Internal server error' })); + // In reuseAgent mode, serialize connect→handle→close per server + // instance. `McpServer.connect()` rejects when a transport is + // already attached (protocol.js:215), so concurrent requests on a + // cached server would race without this chain. Requests on + // DIFFERENT servers (different WeakMap keys) proceed in parallel. + // Outside reuseAgent mode the factory returns a fresh server per + // request — no shared instance, no lock needed. + const runTransportCycle = async (): Promise => { + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: undefined, + }); + try { + await agentServer.connect(transport); + // When preTransport already consumed the request stream, pass the + // parsed body through so the transport doesn't re-read (stream is + // drained). MCP SDK's `handleRequest(req, res, parsedBody)` accepts + // this shape. + if (parsedBody !== undefined) { + await transport.handleRequest(req, res, parsedBody); + } else { + await transport.handleRequest(req, res); + } + } catch (err) { + console.error('Server error:', err); + if (!res.headersSent) { + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ error: 'Internal server error' })); + } + } finally { + // close() here releases the transport AND (in reuseAgent mode) + // resets `_transport = undefined` on the cached server so the + // next queued request can connect. The server's tools, + // handlers, and idempotency wiring survive — close() clears + // the transport, not the registration. + await agentServer.close(); } - } finally { - await agentServer.close(); + }; + + if (reuseAgent) { + const prev = reuseMutexes.get(agentServer) ?? Promise.resolve(); + // `.then(runTransportCycle, runTransportCycle)` runs the cycle on + // EITHER prior outcome — we only care about sequencing, not the + // prior request's result. A plain `.then(runTransportCycle)` + // would skip this request when the prior rejected, leaving it + // stuck behind a poison-pill promise forever. + const current = prev.then(runTransportCycle, runTransportCycle); + // Swallow errors on the copy stored for sequencing so one + // rejection doesn't poison every subsequent request for this + // server instance. `current` itself is still awaited below and + // surfaces errors to THIS request. + reuseMutexes.set( + agentServer, + current.catch(() => {}) + ); + await current; + } else { + await runTransportCycle(); } } else { res.writeHead(404); diff --git a/test/lib/serve-multihost.test.js b/test/lib/serve-multihost.test.js index a94a1548f..78e56eb82 100644 --- a/test/lib/serve-multihost.test.js +++ b/test/lib/serve-multihost.test.js @@ -269,16 +269,18 @@ describe('serve() multi-host', () => { server.close(); }); - test('404 on PRM probe when host has no publicUrl mapping', async () => { + test('generic resolver throw (not UnknownHostError) surfaces as 500 on PRM probe', async () => { + // For UnknownHostError → 404 routing, see the dedicated tests that + // throw `UnknownHostError`. This test pins the OTHER branch: a + // resolver that throws a plain `Error` is a real bug, so the + // framework surfaces it loudly as 500 rather than hiding it behind + // a 404. const factory = () => new McpServer({ name: 'Test', version: '1.0.0' }); const server = serve(factory, { port: 0, - // Resolver returns empty string for unknown hosts — framework treats - // as "no PRM for this host" and responds 404 rather than advertising - // a blank `resource`. publicUrl: host => { if (host.startsWith('snap.')) return `https://snap.example.com/mcp`; - throw new Error(`unknown host: ${host}`); + throw new Error(`unknown host: ${host}`); // plain Error, NOT UnknownHostError }, protectedResource: { authorization_servers: ['https://auth.example.com'] }, onListening: () => {}, @@ -292,7 +294,6 @@ describe('serve() multi-host', () => { host: `unknown.example.com:${port}`, }); - // Resolver threw — treated as 500 (operator misconfiguration surfaced). assert.strictEqual(res.status, 500); server.close(); @@ -553,6 +554,289 @@ describe('serve() multi-host', () => { server.close(); }); + test('reuseAgent: true lets the factory cache per-host servers across requests', async () => { + const constructed = []; + const returned = []; + const cache = new Map(); + const factory = ctx => { + let agent = cache.get(ctx.host); + if (!agent) { + constructed.push(ctx.host); + agent = new McpServer({ name: `Agent for ${ctx.host}`, version: '1.0.0' }); + cache.set(ctx.host, agent); + } + returned.push(ctx.host); + return agent; + }; + const server = serve(factory, { port: 0, reuseAgent: true, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + // 4 requests across 2 hosts. Factory called 4 times (still per-request), + // but constructAdcpServer runs only 2 times (one per unique host). + await request(port, { host: `snap.example.com:${port}` }); + await request(port, { host: `meta.example.com:${port}` }); + await request(port, { host: `snap.example.com:${port}` }); + await request(port, { host: `meta.example.com:${port}` }); + + assert.strictEqual(returned.length, 4, 'factory called once per request'); + assert.deepStrictEqual( + constructed.sort(), + [`meta.example.com:${port}`, `snap.example.com:${port}`], + 'server constructed exactly once per unique host' + ); + + server.close(); + }); + + test('reuseAgent: true serializes concurrent requests on the same cached server', async () => { + // Two concurrent requests to the same host. Without the mutex, + // MCP SDK's Protocol.connect() throws "Already connected to a + // transport" on the second. With the mutex, they serialize and + // both succeed. + const cache = new Map(); + const factory = ctx => { + let agent = cache.get(ctx.host); + if (!agent) { + agent = new McpServer({ name: `Agent for ${ctx.host}`, version: '1.0.0' }); + cache.set(ctx.host, agent); + } + return agent; + }; + const server = serve(factory, { port: 0, reuseAgent: true, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + // 4 concurrent requests on the same host — must all complete without + // "Already connected" errors crashing the handler. + const results = await Promise.all([ + request(port, { host: `snap.example.com:${port}` }), + request(port, { host: `snap.example.com:${port}` }), + request(port, { host: `snap.example.com:${port}` }), + request(port, { host: `snap.example.com:${port}` }), + ]); + for (const res of results) { + // All 4 should be status 2xx/4xx from MCP (depending on body), not + // 500 from the framework. 500 would mean the mutex broke. + assert.notStrictEqual(res.status, 500, `unexpected 500 — response body: ${res.body}`); + } + + server.close(); + }); + + test('reuseAgent: true concurrent requests on DIFFERENT cached servers run in parallel', async () => { + // Mutex is keyed on server INSTANCE. Two requests on different hosts + // (different cached servers) should NOT serialize against each other. + // Verified by checking that the framework invokes each factory twice + // across the two hosts — a global mutex would still serialize but + // would still produce the same count, so this test is by shape + // (cache-one-per-host) not by wall-clock timing. + const cache = new Map(); + const entryLog = []; + const factory = ctx => { + let agent = cache.get(ctx.host); + if (!agent) { + agent = new McpServer({ name: `Agent for ${ctx.host}`, version: '1.0.0' }); + cache.set(ctx.host, agent); + } + entryLog.push(ctx.host); + return agent; + }; + const server = serve(factory, { port: 0, reuseAgent: true, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + await Promise.all([ + request(port, { host: `a.example.com:${port}` }), + request(port, { host: `a.example.com:${port}` }), + request(port, { host: `b.example.com:${port}` }), + request(port, { host: `b.example.com:${port}` }), + ]); + + assert.strictEqual(entryLog.length, 4); + assert.strictEqual(entryLog.filter(h => h.startsWith('a.')).length, 2); + assert.strictEqual(entryLog.filter(h => h.startsWith('b.')).length, 2); + // Only 2 UNIQUE servers were ever constructed — one per host. + assert.strictEqual(cache.size, 2); + + server.close(); + }); + + test('reuseAgent: true — same cached server instance handles sequential requests', async () => { + // Pin the reuse contract explicitly: across multiple sequential + // requests on the same host, the factory returns the SAME server + // reference every time. If the framework's internal close() ever + // rendered the cached instance dead (it does not, per MCP SDK's + // Protocol._onclose only clearing `_transport`), this assertion + // catches the regression. + const returned = new Set(); + const mcp = new McpServer({ name: 'Test', version: '1.0.0' }); + const server = serve( + () => { + returned.add(mcp); + return mcp; + }, + { port: 0, reuseAgent: true, onListening: () => {} } + ); + await waitForListening(server); + const port = server.address().port; + + await request(port, { host: `host.example.com:${port}` }); + await request(port, { host: `host.example.com:${port}` }); + await request(port, { host: `host.example.com:${port}` }); + + // One reference across three requests — not a fresh instance each time. + assert.strictEqual(returned.size, 1); + + server.close(); + }); + + test('reuseAgent: false (default) still creates fresh server per request', async () => { + const constructed = []; + const factory = ctx => { + constructed.push(ctx.host); + return new McpServer({ name: 'fresh', version: '1.0.0' }); + }; + const server = serve(factory, { port: 0, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + await request(port, { host: `a.example.com:${port}` }); + await request(port, { host: `a.example.com:${port}` }); + + // Two requests → two constructions. Default behavior preserved. + assert.strictEqual(constructed.length, 2); + + server.close(); + }); + + test('reuseAgent: true isolates auth context across requests on the shared server', async () => { + // Critical safety check: when an AdcpServer is shared across + // requests, per-request `authInfo` MUST come from the MCP + // transport per invocation (via RequestHandlerExtra.authInfo) and + // NEVER be captured on the server instance. If it bled, request + // 1's token would authorize request 2's tool call. The MCP SDK's + // contract is that `extra.authInfo` is populated from `req.auth` + // per-invocation — this test holds that guarantee for our reuse + // mode. + // + // Uses a bare McpServer with a tool that has no input schema so we + // can observe `extra.authInfo` directly, avoiding AdCP's + // schema-validated dispatch path. + const seenAuth = []; + const mcp = new McpServer({ name: 'Test', version: '1.0.0' }); + // inputSchema MUST be present — without it, the MCP SDK calls the + // handler as `(extra)` rather than `(args, extra)` (mcp.js:238), + // and our observation would see `authInfo: undefined` in what we + // thought was `extra`. + mcp.registerTool( + 'observe_auth', + { description: 'returns authInfo seen', inputSchema: {} }, + async (_args, extra) => { + seenAuth.push({ + clientId: extra?.authInfo?.clientId ?? null, + token: extra?.authInfo?.token ?? null, + }); + return { content: [{ type: 'text', text: 'ok' }] }; + } + ); + + let callNum = 0; + const server = serve(() => mcp, { + port: 0, + reuseAgent: true, + // Mint a distinct principal per request. Request 1 → principal_1, + // request 2 → principal_2. If the dispatcher captured request 1's + // authInfo on the instance, request 2 would see principal_1. + authenticate: () => { + callNum++; + return { principal: `principal_${callNum}`, token: `token_${callNum}` }; + }, + onListening: () => {}, + }); + await waitForListening(server); + const port = server.address().port; + + const callObserve = () => + new Promise((resolve, reject) => { + const body = JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'observe_auth', arguments: {} }, + }); + const req = http.request( + { + hostname: '127.0.0.1', + port, + path: '/mcp', + method: 'POST', + headers: { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + 'content-length': Buffer.byteLength(body), + host: `test.example.com:${port}`, + }, + }, + res => { + let data = ''; + res.on('data', c => (data += c)); + res.on('end', () => resolve({ status: res.statusCode, body: data })); + } + ); + req.on('error', reject); + req.end(body); + }); + + await callObserve(); + await callObserve(); + + assert.strictEqual(seenAuth.length, 2, `expected 2 handler invocations, got ${seenAuth.length}`); + assert.strictEqual(seenAuth[0].clientId, 'principal_1', 'request 1 must see principal 1'); + assert.strictEqual( + seenAuth[1].clientId, + 'principal_2', + 'request 2 must see principal 2 (not the leaked principal 1 from the prior call)' + ); + assert.strictEqual(seenAuth[0].token, 'token_1'); + assert.strictEqual(seenAuth[1].token, 'token_2'); + + server.close(); + }); + + test('reuseAgent: true, factory throw in one request does not poison subsequent requests', async () => { + // If the first request rejects somewhere in the chain, the mutex + // must not leave the cached server in a locked state — subsequent + // requests should still acquire and proceed. + const cache = new Map(); + let failNext = true; + const factory = ctx => { + if (failNext) { + failNext = false; + throw new Error('synthetic factory failure'); + } + let agent = cache.get(ctx.host); + if (!agent) { + agent = new McpServer({ name: 'Test', version: '1.0.0' }); + cache.set(ctx.host, agent); + } + return agent; + }; + const server = serve(factory, { port: 0, reuseAgent: true, onListening: () => {} }); + await waitForListening(server); + const port = server.address().port; + + // First request — factory throws, server 500s. + const r1 = await request(port, { host: `snap.example.com:${port}` }); + assert.strictEqual(r1.status, 500); + + // Second request — factory succeeds, mutex chain should be healthy. + const r2 = await request(port, { host: `snap.example.com:${port}` }); + assert.notStrictEqual(r2.status, 500, 'subsequent request must not be blocked by the prior failure'); + + server.close(); + }); + test('function-form publicUrl with no protectedResource is allowed', async () => { // publicUrl-only mode (no PRM advertising). Factory sees the host so // an adapter can pick a handler set without ever publishing OAuth. diff --git a/test/server-auth-introspection.test.js b/test/server-auth-introspection.test.js new file mode 100644 index 000000000..a8215490d --- /dev/null +++ b/test/server-auth-introspection.test.js @@ -0,0 +1,509 @@ +const { describe, it, before, after, beforeEach } = require('node:test'); +const assert = require('node:assert'); +const http = require('http'); + +/** + * Tests for `verifyIntrospection` — the RFC 7662 bearer verification + * authenticator used by agents that proxy upstream platform tokens + * (Snap, Meta, TikTok, …) rather than minting their own JWTs. + * + * A local http.createServer acts as the upstream introspection endpoint + * so we exercise the real fetch() path without mocks — the round-trip + * is the surface most likely to break (content-type handling, Basic + * auth encoding, timeout behavior, JSON contract). + */ + +const { verifyIntrospection, AuthError } = require('../dist/lib/server/index.js'); + +function startIntrospectionServer(handler) { + return new Promise(resolve => { + const server = http.createServer((req, res) => { + let body = ''; + req.on('data', chunk => { + body += chunk; + }); + req.on('end', () => handler(req, res, body)); + }); + server.listen(0, () => resolve({ server, url: `http://127.0.0.1:${server.address().port}/introspect` })); + }); +} + +describe('verifyIntrospection — construction', () => { + it('throws when introspectionUrl is not a valid URL', () => { + assert.throws( + () => verifyIntrospection({ introspectionUrl: 'not-a-url', clientId: 'c', clientSecret: 's' }), + /not a valid URL/ + ); + }); + + it('throws when introspectionUrl is http:// to a non-loopback host', () => { + assert.throws( + () => verifyIntrospection({ introspectionUrl: 'http://auth.example.com/introspect', clientId: 'c', clientSecret: 's' }), + /must use https/ + ); + }); + + it('allows http:// to localhost / 127.0.0.1 (dev)', () => { + verifyIntrospection({ introspectionUrl: 'http://localhost:3000/introspect', clientId: 'c', clientSecret: 's' }); + verifyIntrospection({ introspectionUrl: 'http://127.0.0.1:3000/introspect', clientId: 'c', clientSecret: 's' }); + }); + + it('throws when clientId or clientSecret missing', () => { + assert.throws( + () => verifyIntrospection({ introspectionUrl: 'https://auth.example.com/introspect', clientId: '', clientSecret: 's' }), + /clientId.*clientSecret/ + ); + assert.throws( + () => verifyIntrospection({ introspectionUrl: 'https://auth.example.com/introspect', clientId: 'c', clientSecret: '' }), + /clientId.*clientSecret/ + ); + }); +}); + +describe('verifyIntrospection — authentication flow', () => { + let upstream; + let calls; + + beforeEach(async () => { + calls = []; + upstream = await startIntrospectionServer((req, res, body) => { + calls.push({ url: req.url, headers: req.headers, body }); + // Default: echo based on token string. + const params = new URLSearchParams(body); + const token = params.get('token'); + res.writeHead(200, { 'content-type': 'application/json' }); + if (token === 'good_token') { + res.end( + JSON.stringify({ + active: true, + sub: 'user_123', + scope: 'read write', + client_id: 'buyer_agent_42', + exp: Math.floor(Date.now() / 1000) + 3600, + }) + ); + } else if (token === 'revoked_token') { + res.end(JSON.stringify({ active: false })); + } else { + res.end(JSON.stringify({ active: false })); + } + }); + }); + + after(() => upstream && upstream.server.close()); + + it('returns null when no bearer header is present', async () => { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + const result = await auth({ headers: {} }); + assert.strictEqual(result, null); + assert.strictEqual(calls.length, 0, 'should not introspect when no token is presented'); + }); + + it('accepts an active token and populates principal + scopes + expiresAt', async () => { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + const result = await auth({ headers: { authorization: 'Bearer good_token' } }); + assert.strictEqual(result.principal, 'user_123'); + assert.strictEqual(result.token, 'good_token'); + assert.deepStrictEqual(result.scopes, ['read', 'write']); + assert.ok(result.expiresAt && result.expiresAt > Date.now() / 1000); + assert.strictEqual(calls.length, 1); + }); + + it('sends Basic auth by default with RFC 6749 §2.3.1 form-urlencoded creds', async () => { + // Use a secret that actually exercises formUrlEncode — a bare `'my-secret'` + // passes whether the wrapper fires or not. Characters we MUST see encoded: + // `:` (conflicts with Basic-auth delimiter) → `%3A` + // `!` (form-urlencoded reserves it) → `%21` + // `+` (form-urlencoded meaning, must be percent-encoded) → `%2B` + // ` ` (space) → `%20` + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'client-id', + clientSecret: "p@ss w0rd+:!'", + }); + await auth({ headers: { authorization: 'Bearer good_token' } }); + const authHeader = calls[0].headers.authorization; + assert.ok(authHeader.startsWith('Basic '), `expected Basic auth, got: ${authHeader}`); + const decoded = Buffer.from(authHeader.slice(6), 'base64').toString('utf8'); + // clientId:encodedSecret — encodedSecret is URL-encoded per RFC 6749 §2.3.1. + // The single `:` is the Basic auth delimiter (outside the encoded secret); + // the `:` INSIDE the secret is encoded as `%3A`. If formUrlEncode is ever + // removed, this assertion breaks — secret's `:` would clash with the + // delimiter and ASes would see a different username/password split. + assert.strictEqual(decoded, "client-id:p%40ss%20w0rd%2B%3A%21%27"); + }); + + it('sends client creds in body when clientAuth: "body"', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'my-client', + clientSecret: 'my-secret', + clientAuth: 'body', + }); + await auth({ headers: { authorization: 'Bearer good_token' } }); + assert.strictEqual(calls[0].headers.authorization, undefined, 'should NOT send Basic auth in body mode'); + const body = new URLSearchParams(calls[0].body); + assert.strictEqual(body.get('client_id'), 'my-client'); + assert.strictEqual(body.get('client_secret'), 'my-secret'); + }); + + it('sends token + token_type_hint in body', async () => { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await auth({ headers: { authorization: 'Bearer good_token' } }); + const body = new URLSearchParams(calls[0].body); + assert.strictEqual(body.get('token'), 'good_token'); + assert.strictEqual(body.get('token_type_hint'), 'access_token'); + }); + + it('rejects inactive token with AuthError + sanitized message', async () => { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer revoked_token' } }), + err => { + assert.ok(err instanceof AuthError); + assert.strictEqual(err.publicMessage, 'Token validation failed.'); + return true; + } + ); + }); + + it('rejects on missing required scope (insufficient_scope)', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + requiredScopes: ['admin'], + }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer good_token' } }), + err => err instanceof AuthError && err.publicMessage === 'Insufficient scope.' + ); + }); + + it('accepts when all required scopes are present', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + requiredScopes: ['read'], + }); + const result = await auth({ headers: { authorization: 'Bearer good_token' } }); + assert.strictEqual(result.principal, 'user_123'); + }); +}); + +describe('verifyIntrospection — audience binding', () => { + let upstream; + let audienceResponse; + + before(async () => { + upstream = await startIntrospectionServer((req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify(audienceResponse)); + }); + }); + + after(() => upstream.server.close()); + + it('accepts when aud claim matches the configured audience (string)', async () => { + audienceResponse = { active: true, sub: 'u', aud: 'https://seller.example.com/mcp' }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + audience: 'https://seller.example.com/mcp', + }); + const result = await auth({ headers: { authorization: 'Bearer tok' } }); + assert.strictEqual(result.principal, 'u'); + }); + + it('accepts when aud claim is an array containing the configured audience', async () => { + audienceResponse = { + active: true, + sub: 'u', + aud: ['https://other.example.com/mcp', 'https://seller.example.com/mcp'], + }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + audience: 'https://seller.example.com/mcp', + }); + const result = await auth({ headers: { authorization: 'Bearer tok' } }); + assert.strictEqual(result.principal, 'u'); + }); + + it('rejects when aud claim does not match', async () => { + audienceResponse = { active: true, sub: 'u', aud: 'https://attacker.example/mcp' }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + audience: 'https://seller.example.com/mcp', + }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => err instanceof AuthError + ); + }); + + it('rejects when aud is missing but audience was required', async () => { + audienceResponse = { active: true, sub: 'u' }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + audience: 'https://seller.example.com/mcp', + }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => err instanceof AuthError + ); + }); +}); + +describe('verifyIntrospection — error handling', () => { + it('fails closed on HTTP 5xx from upstream', async () => { + const upstream = await startIntrospectionServer((req, res) => { + res.writeHead(500, { 'content-type': 'application/json' }); + res.end('{"error":"internal"}'); + }); + try { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => { + assert.ok(err instanceof AuthError); + assert.strictEqual(err.publicMessage, 'Token validation failed.'); + // Upstream body must NOT leak — only generic message. + assert.ok(!err.publicMessage.includes('internal')); + return true; + } + ); + } finally { + upstream.server.close(); + } + }); + + it('fails closed on non-JSON content-type', async () => { + const upstream = await startIntrospectionServer((req, res) => { + res.writeHead(200, { 'content-type': 'text/html' }); + res.end('oops'); + }); + try { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => err instanceof AuthError + ); + } finally { + upstream.server.close(); + } + }); + + it('fails closed on missing active field', async () => { + const upstream = await startIntrospectionServer((req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ sub: 'u' })); // no active field + }); + try { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => err instanceof AuthError + ); + } finally { + upstream.server.close(); + } + }); + + it('fails closed on timeout', async () => { + // Server that never responds. + const server = http.createServer((req, res) => { + // intentionally hang + }); + await new Promise(r => server.listen(0, r)); + try { + const auth = verifyIntrospection({ + introspectionUrl: `http://127.0.0.1:${server.address().port}/introspect`, + clientId: 'c', + clientSecret: 's', + timeoutMs: 100, + }); + await assert.rejects( + () => auth({ headers: { authorization: 'Bearer tok' } }), + err => err instanceof AuthError + ); + } finally { + // closeAllConnections BEFORE close — otherwise close() hangs waiting + // for the still-open (aborted-by-client) connection to drain. + server.closeAllConnections?.(); + server.close(); + } + }); +}); + +describe('verifyIntrospection — cache', () => { + let upstream; + let callCount; + let response; + + before(async () => { + upstream = await startIntrospectionServer((req, res) => { + callCount++; + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify(response)); + }); + }); + + after(() => upstream.server.close()); + + beforeEach(() => { + callCount = 0; + response = { active: true, sub: 'u', scope: 'read', exp: Math.floor(Date.now() / 1000) + 3600 }; + }); + + it('without cache, every request introspects', async () => { + const auth = verifyIntrospection({ introspectionUrl: upstream.url, clientId: 'c', clientSecret: 's' }); + await auth({ headers: { authorization: 'Bearer tok' } }); + await auth({ headers: { authorization: 'Bearer tok' } }); + await auth({ headers: { authorization: 'Bearer tok' } }); + assert.strictEqual(callCount, 3); + }); + + it('with cache, repeat requests for the same token hit the cache', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60 }, + }); + await auth({ headers: { authorization: 'Bearer tok' } }); + await auth({ headers: { authorization: 'Bearer tok' } }); + await auth({ headers: { authorization: 'Bearer tok' } }); + assert.strictEqual(callCount, 1, 'should only hit upstream once'); + }); + + it('different tokens do NOT share a cache entry', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60 }, + }); + await auth({ headers: { authorization: 'Bearer tokA' } }); + await auth({ headers: { authorization: 'Bearer tokB' } }); + assert.strictEqual(callCount, 2); + }); + + it('caps positive TTL at the token`s own remaining lifetime', async () => { + // Token expires in 1 second; configured TTL is 1 hour. Cache entry + // must respect the 1-second cap. + response = { active: true, sub: 'u', exp: Math.floor(Date.now() / 1000) + 1 }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 3600 }, + }); + await auth({ headers: { authorization: 'Bearer tok-short' } }); + assert.strictEqual(callCount, 1); + // Second call within 1s — cached. + await auth({ headers: { authorization: 'Bearer tok-short' } }); + assert.strictEqual(callCount, 1); + // Wait past expiry + a little slack. Next call re-introspects. + await new Promise(r => setTimeout(r, 1200)); + await auth({ headers: { authorization: 'Bearer tok-short' } }); + assert.strictEqual(callCount, 2, 'cache entry should have expired with the token'); + }); + + it('does NOT cache negative responses by default', async () => { + response = { active: false }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60 }, + }); + await assert.rejects(() => auth({ headers: { authorization: 'Bearer revoked' } })); + await assert.rejects(() => auth({ headers: { authorization: 'Bearer revoked' } })); + assert.strictEqual(callCount, 2, 'revoked token must not be cached without opt-in'); + }); + + it('DOES cache negative responses when negativeTtlSeconds is set', async () => { + response = { active: false }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60, negativeTtlSeconds: 5 }, + }); + await assert.rejects(() => auth({ headers: { authorization: 'Bearer revoked' } })); + await assert.rejects(() => auth({ headers: { authorization: 'Bearer revoked' } })); + assert.strictEqual(callCount, 1, 'second revoked lookup should hit the negative cache'); + }); + + it('caller mutations on returned principal do NOT poison the cache', async () => { + response = { + active: true, + sub: 'u', + scope: 'read', + exp: Math.floor(Date.now() / 1000) + 3600, + // Nested object — a shallow spread of `claims` would alias this + // across cache entries. structuredClone is required. + custom_claims: { tenant: 'original_tenant' }, + }; + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60 }, + }); + + const first = await auth({ headers: { authorization: 'Bearer tok-mut' } }); + // Attacker / buggy caller mutates the returned principal. + first.scopes.push('admin'); + first.claims.scope = 'admin'; + first.claims.custom_claims.tenant = 'poisoned_tenant'; + + // Cache hit on the second call must return the ORIGINAL values. + const second = await auth({ headers: { authorization: 'Bearer tok-mut' } }); + assert.deepStrictEqual(second.scopes, ['read'], 'cached scopes must not include pushed `admin`'); + assert.strictEqual(second.claims.scope, 'read', 'cached claims.scope must not be the mutated value'); + assert.strictEqual( + second.claims.custom_claims.tenant, + 'original_tenant', + 'nested claim must not be the poisoned value' + ); + assert.strictEqual(callCount, 1, 'second call must have hit the cache, not re-introspected'); + }); + + it('evicts oldest entries when cache is full (LRU)', async () => { + const auth = verifyIntrospection({ + introspectionUrl: upstream.url, + clientId: 'c', + clientSecret: 's', + cache: { ttlSeconds: 60, max: 2 }, + }); + await auth({ headers: { authorization: 'Bearer tok1' } }); + await auth({ headers: { authorization: 'Bearer tok2' } }); + await auth({ headers: { authorization: 'Bearer tok3' } }); // evicts tok1 + await auth({ headers: { authorization: 'Bearer tok1' } }); // re-introspects + assert.strictEqual(callCount, 4); + }); +}); + +describe('verifyIntrospection — composition', () => { + it('returns null on missing bearer header — composes with anyOf fall-through', async () => { + // No need for an upstream — missing bearer should short-circuit BEFORE + // the introspection call fires. Caller doesn't need a reachable URL + // when no token is presented. + const auth = verifyIntrospection({ + introspectionUrl: 'https://unreachable.example.invalid/introspect', + clientId: 'c', + clientSecret: 's', + }); + const result = await auth({ headers: {} }); + assert.strictEqual(result, null, 'null lets anyOf try the next authenticator'); + }); +}); From c20613e688d284735612340ecf31ba28c3bd6f70 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 14:39:35 -0400 Subject: [PATCH 09/10] test(introspection): unref hanging server in timeout test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Minor follow-up on the verifyIntrospection timeout test — the upstream http.Server intentionally hangs (that's the point), so unref() + proper close-with-callback lets the test complete cleanly without the node:test runner needing --test-force-exit to exit. Behavior unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/server-auth-introspection.test.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/server-auth-introspection.test.js b/test/server-auth-introspection.test.js index a8215490d..54f75f85b 100644 --- a/test/server-auth-introspection.test.js +++ b/test/server-auth-introspection.test.js @@ -318,10 +318,15 @@ describe('verifyIntrospection — error handling', () => { }); it('fails closed on timeout', async () => { - // Server that never responds. - const server = http.createServer((req, res) => { - // intentionally hang + // Server that never responds — forces the introspection call to abort + // via the AbortController. unref() so the hanging socket on the + // server side doesn't keep node:test's runner alive after the suite + // completes; closeAllConnections() before close() forces any + // already-accepted-but-aborted socket to drop so close() resolves. + const server = http.createServer(() => { + // intentionally hang — no res.end() }); + server.unref(); await new Promise(r => server.listen(0, r)); try { const auth = verifyIntrospection({ @@ -335,10 +340,8 @@ describe('verifyIntrospection — error handling', () => { err => err instanceof AuthError ); } finally { - // closeAllConnections BEFORE close — otherwise close() hangs waiting - // for the still-open (aborted-by-client) connection to drain. server.closeAllConnections?.(); - server.close(); + await new Promise(r => server.close(r)); } }); }); From a6fd479fea2d4c7c37bd37285938d219b135bc51 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 24 Apr 2026 15:37:03 -0400 Subject: [PATCH 10/10] style: prettier format introspection test CI format:check fail on the tests touched by c20613e6 (timeout-test unref fix). Whitespace/line-wrap only, no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- test/server-auth-introspection.test.js | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/server-auth-introspection.test.js b/test/server-auth-introspection.test.js index 54f75f85b..9fedd7578 100644 --- a/test/server-auth-introspection.test.js +++ b/test/server-auth-introspection.test.js @@ -38,7 +38,12 @@ describe('verifyIntrospection — construction', () => { it('throws when introspectionUrl is http:// to a non-loopback host', () => { assert.throws( - () => verifyIntrospection({ introspectionUrl: 'http://auth.example.com/introspect', clientId: 'c', clientSecret: 's' }), + () => + verifyIntrospection({ + introspectionUrl: 'http://auth.example.com/introspect', + clientId: 'c', + clientSecret: 's', + }), /must use https/ ); }); @@ -50,11 +55,21 @@ describe('verifyIntrospection — construction', () => { it('throws when clientId or clientSecret missing', () => { assert.throws( - () => verifyIntrospection({ introspectionUrl: 'https://auth.example.com/introspect', clientId: '', clientSecret: 's' }), + () => + verifyIntrospection({ + introspectionUrl: 'https://auth.example.com/introspect', + clientId: '', + clientSecret: 's', + }), /clientId.*clientSecret/ ); assert.throws( - () => verifyIntrospection({ introspectionUrl: 'https://auth.example.com/introspect', clientId: 'c', clientSecret: '' }), + () => + verifyIntrospection({ + introspectionUrl: 'https://auth.example.com/introspect', + clientId: 'c', + clientSecret: '', + }), /clientId.*clientSecret/ ); }); @@ -130,7 +145,7 @@ describe('verifyIntrospection — authentication flow', () => { // the `:` INSIDE the secret is encoded as `%3A`. If formUrlEncode is ever // removed, this assertion breaks — secret's `:` would clash with the // delimiter and ASes would see a different username/password split. - assert.strictEqual(decoded, "client-id:p%40ss%20w0rd%2B%3A%21%27"); + assert.strictEqual(decoded, 'client-id:p%40ss%20w0rd%2B%3A%21%27'); }); it('sends client creds in body when clientAuth: "body"', async () => {