Skip to content

feat(server): host-aware serve() for one-process multi-host#897

Merged
bokelley merged 11 commits intomainfrom
bokelley/serve-multihost-issues
Apr 24, 2026
Merged

feat(server): host-aware serve() for one-process multi-host#897
bokelley merged 11 commits intomainfrom
bokelley/serve-multihost-issues

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Closes #885. Adds first-class multi-host routing to serve() so one Node process can front many hostnames (white-label publishers, multi-brand adapters) without re-owning the HTTP plumbing.

Changes

  • ServeOptions.publicUrl: string | (host) => string — function form resolves per unique host; result cached. Path-match validation runs against each resolved URL. Static string form unchanged.
  • ServeOptions.protectedResource: PRM | (host) => PRM — same pattern. Each host advertises its own RFC 9728 resource + authorization servers.
  • ServeContext.host: string — resolved canonical host (lowercased, port preserved) passed into the factory on every request so it can branch on hostname.
  • ServeOptions.trustForwardedHost: boolean — default false. Opt-in for X-Forwarded-Host when behind a proxy that sanitizes it (Fly, Cloud Run, internal ALB). Without it, an attacker-controlled header can't flip the advertised OAuth resource.
  • verifyBearer({ audience: (req) => string }) — audience check follows the per-host publicUrl so a token minted for snap.example.com fails validation at meta.example.com. Resolver errors surface as sanitized AuthError, not 500.
  • SKILL.md — §"Multi-Host and Alternative Transports" replaces the old "on the roadmap" caveat with a working multi-host recipe.

Security properties preserved

  • Function-form publicUrl still validates each resolved URL's path against the mount path — mismatch fails closed (500) rather than minting audience-mismatched tokens.
  • X-Forwarded-Host ignored by default; opt-in is explicit.
  • Resolver throws are caught and logged, response is 500 — not a silent mis-advertisement.

Test plan

  • 11 new tests in test/lib/serve-multihost.test.js — host threading, function-form publicUrl + PRM, caching, trustForwardedHost on/off, X-Forwarded-Host chain handling, 404 on missing host, 500 on invalid per-host publicUrl, backward compat for static form.
  • 2 new tests in test/server-auth.test.js — function-form audience succeeds for matching host / rejects for mismatched host; resolver throw → sanitized AuthError.
  • Full existing serve() + verifyBearer suites pass (54/54 targeted, 5627/5629 full suite — the 2 failures are a pre-existing EADDRINUSE flake on :3112 from a stale process in another Conductor worktree, unrelated).
  • tsc --noEmit clean.
  • CI green.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
Comment thread test/lib/serve-multihost.test.js Fixed
bokelley and others added 4 commits April 24, 2026 12:10
…osition

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) <noreply@anthropic.com>
CI format:check failure — pure whitespace/line-wrap, no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-code-quality bot flag.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
bokelley and others added 2 commits April 24, 2026 12:54
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@bokelley bokelley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strong overall — security posture is right, the scope stayed tight, and the Express-AS composition recipe is the right addition for adapters that run their own OAuth AS. Three things I'd push back on before merge:

1. audience: (req) => string callback doesn't go through serve()'s host resolution — footgun in the example.

SKILL.md multi-host example (skills/build-seller-agent/SKILL.md) has:

audience: req => `https://${req.headers['x-forwarded-host'] ?? req.headers.host}/mcp`

serve() itself honors trustForwardedHost when resolving host. This audience callback doesn't — it reads X-Forwarded-Host unconditionally. A user copying this example without trustForwardedHost: true advertises one audience (from publicUrl(host)) but verifies against another (whatever X-Forwarded-Host says). Attacker-controlled header flips the audience check but not the advertised PRM — a token minted for the attacker's claimed host verifies, even though the framework advertised a different resource.

Fix options:

  • Thread the already-resolved host into the audience callback: audience: (req, { host }) => ...
  • Better: expose the resolved publicUrl directly, so the common case is audience: (_req, { publicUrl }) => publicUrl (one line, can't drift from what serve() advertised).

Either way, the example should not read X-Forwarded-Host directly.

2. Missing an end-to-end test for the cryptographic backstop.

The 11 new tests in test/lib/serve-multihost.test.js cover host threading, function-form publicUrl/PRM, caching, trustForwardedHost on/off, and 404/500 paths. What I don't see (and should exist, since it's the security property of multi-host): a named test proving a token minted for host A, presented on a request landing at host B in the same process, fails audience verification at host B. The 2 audience-function tests in test/server-auth.test.js are unit-level — this wants to be end-to-end: two publicUrl(host) mappings, two OAuth audiences, one process, cross-host token, assert 401. If that's covered by one of the existing tests I missed, a rename would make it obvious.

3. throw new Error("Unknown host: X") → 500 is defensible but unkind to ops.

The factory example throws on unregistered hosts, framework catches and 500s. Fine for security (doesn't leak the adapter table) but confusing when an operator typos a DNS entry. A dedicated UnknownHostError mapping to 404 with a clean body would be kinder. Minor — don't block merge on this one.

Security properties I did verify hold: X-Forwarded-Host off by default, path-match validation per resolved URL, fail-closed on resolver throws, per-host result caching. Good shape.

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) <noreply@anthropic.com>
@bokelley
Copy link
Copy Markdown
Contributor Author

reopening to retrigger skipped CI workflows

@bokelley bokelley closed this Apr 24, 2026
@bokelley bokelley reopened this Apr 24, 2026
bokelley and others added 3 commits April 24, 2026 14:39
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) <noreply@anthropic.com>
CI format:check fail on the tests touched by c20613e (timeout-test
unref fix). Whitespace/line-wrap only, no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 7681396 into main Apr 24, 2026
9 checks passed
bokelley added a commit that referenced this pull request Apr 25, 2026
Drops the `as Record<string, unknown>` cast on the introspection response
in `verifyIntrospection` — JWTPayload's `[propName: string]: unknown` index
signature already accepts the RFC 7662 response shape structurally, so the
cast was hiding the real relationship between the two types.

Adds a JSDoc block on `AuthPrincipal.claims` documenting that the field
carries either a decoded JWT (verifyBearer) or an RFC 7662 introspection
response (verifyIntrospection), and warning adapter handlers passing
`sub`/`username`/`client_id`/`scope` into LLM contexts to narrow and
validate first — an upstream IdP that controls those fields can inject
prompt content otherwise.

Follow-up to #897 (closed #902). Reviewed by code-reviewer, security-reviewer,
ad-tech-protocol-expert in parallel; convergent ask was the `scope` callout
in the prompt-injection warning.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

serve(): first-class multi-host routing (one process → N hostnames)

1 participant