feat: Client ID Metadata Documents with SSRF-guarded resolution + consent interstitial (#166)

[heskew]

Summary

Implements Client ID Metadata Documents (#166) — the modern MCP client-registration path (the draft spec marks DCR deprecated): an HTTPS URL is accepted as a client_id, resolved by fetching the JSON metadata document it points to, validated, cached, and taken through a consent interstitial before the upstream redirect.

• Resolution (src/lib/mcp/cimd.ts): URL-shape detection (https + non-root path, no userinfo/fragment/IP-literals/dot-segments) → SSRF-guarded fetch → validation → per-process cache. resolveClient() routes URL-shaped ids to CIMD and everything else to the existing DCR store; DCR is unchanged as the fallback.
• SSRF guards: DNS gate allowing only globally routable addresses, classified table-driven against the full IANA IPv4/IPv6 special-purpose registries; IPv6 allows only global unicast (2000::/3) with v4-mapped and 6to4/ISATAP forms classified by embedded IPv4 (real parser: :: compression, dotted-quad tails, zone indexes; fail-closed on anything unparseable) and in-2000::/3 special-use (Teredo/ORCHID/documentation) rejected. Pinned-connect: the fetch (https.request + custom lookup) connects to the exact address the gate validated while keeping the hostname for TLS SNI/cert — closing the rebind TOCTOU. No redirect-follow, only 200 OK accepted, one deadline (default 5 s) spanning DNS + connect + body, 64 KB cap, JSON-only, config values coerced fail-closed. Concurrent DNS resolutions are globally bounded (uncancellable getaddrinfo) with per-client_id dedup. Every gate rejection returns one generic invalid_client message (no internal-DNS probing); detail is server-side log only.
• Validation: document client_id must equal the fetched URL exactly; required fields (client_id, client_name, redirect_uris); redirect URIs validated with the same structural rules and the same redirect-host policy as DCR (dynamicClientRegistration.allowedRedirectUriHosts) — clientIdMetadataDocuments.allowedHosts governs the document host trust policy only.
• Caching: LRU-bounded (1000 entries, keys are attacker-chosen); TTL from Cache-Control: max-age clamped to [60 s, 24 h] (no-store/no-cache floor at 60 s as deliberate DoS protection); failures never cached (draft requirement); cached records revalidated against the live redirect-host policy on every hit.
• Consent interstitial (CIMD clients only): instead of an immediate 302 to the upstream IdP, a self-contained HTML page shows the authoritative client_id host, the client name (unverified), and the redirect URI hostname (prominent loopback-impersonation warning when the redirect is localhost-only — spec MUST). Continue is a POST with a single-use, short-TTL token binding the full validated authorize params; the confirm handler resumes the existing upstream-redirect logic. Served with X-Frame-Options: DENY / frame-ancestors 'none' / Cache-Control: no-store.
• Consent browser binding (src/lib/mcp/consentBinding.ts): the interstitial sets a per-flow __Host--prefixed, Secure/HttpOnly/SameSite=Lax nonce cookie; sha256(nonce) travels inside the confirm token and the upstream OAuth state, and both /oauth/mcp/confirm and the upstream callback require a constant-time match before an MCP authorization code is issued. The callback checks it before the upstream code exchange and onLogin, so a mismatched flow triggers no side effects. __Host- blocks sibling-origin cookie injection; the per-flow name supports concurrent tabs. The callback also hard-rejects confirm tokens presented as upstream state (token purpose enforcement). DCR flows carry no binding and are untouched.
• Config: mcp.clientIdMetadataDocuments (enabled — default ON when mcp.enabled; allowedHosts; fetch timeout/size). AS metadata advertises client_id_metadata_document_supported: true when enabled.
• Add RFC 7523 client_credentials grant (private_key_jwt, EdDSA) for headless agent auth #159 handoff: jwks / jwks_uri / token_endpoint_auth_method are carried through on resolved records; v1 accepts token_endpoint_auth_method: none only — private_key_jwt activates with Add RFC 7523 client_credentials grant (private_key_jwt, EdDSA) for headless agent auth #159's assertion verification.

Where to focus review

• Consent binding (consentBinding.ts + its use in authorize.ts / handlers.ts) — newest surface; verify the cookie→hash→state chain has no unbound leg. Deliberate consequence: CIMD authorization now requires cookies in the user's browser.
• SSRF surface (cimd.ts guards) — the AS fetches an attacker-supplied URL; the DNS gate, deadline, size caps, and no-redirect posture are the containment. The IPv6 policy is deny-by-default (2000::/3 only).
• Interstitial escaping (authorize.ts escapeHtml) — client_name is attacker-controlled and rendered; every interpolation is escaped and tested.
• Deliberate deviations to sanity-check: DNS resolution failure now returns 400 invalid_client (generic) instead of 500, so status codes don't reopen the internal-DNS probe channel; no-store is floored at 60 s rather than honored literally (DoS floor).
• Coordination with Add RFC 7523 client_credentials grant (private_key_jwt, EdDSA) for headless agent auth #159 (in flight separately): grant logic and tokenIssuer.ts deliberately untouched; only the token endpoint's client-record lookup is routed through resolveClient.

Review history

Two rounds of external review, all findings addressed:

• Round 1 (f0da8a1): Claude + Codex + a security pass + two gemini bot rounds — 12 findings incl. two flow-level blockers (consent not browser-bound; confirm token accepted as upstream state). Fixed in 195cf21; per-finding mapping.
• Round 2 (f0da8a1 Codex pass): 3 High + 4 Medium — sibling-origin cookie injection, dns.lookup threadpool DoS, preflight-not-connection-bound, RFC 6890 special-use ranges, binding-after-login-side-effects, config fail-open, concurrent-flow cookie collision. Fixed in 7f07d87/6fb7dc8; per-finding mapping.

Tests

938 total / 936 pass / 2 pre-existing skips: URL-shape + SSRF rejections (incl. ::, non-canonical loopback, v4-mapped hex, 6to4/ISATAP/Teredo embedded-v4, IANA special-use, dot-segments), 200-only, full-fetch deadline, config coercion, DNS concurrency-cap + dedup + pinned-address wiring, exact-match and required-field validation, cache TTL clamps + LRU bound + no-failure-caching + live policy revalidation, XSS escaping, interstitial flow (HTML → POST confirm → 302; single-use/expired/tampered tokens; anti-framing headers; per-flow __Host- cookie), consent-binding attack regressions on both legs (incl. no exchange/onLogin on mismatch), token-purpose rejection at the callback, config normalization, DCR-client flow unchanged, metadata flag gating.

Docs updated: docs/mcp-oauth.md + docs/configuration.md.

Closes #166

🤖 Generated with Claude Code (Opus 4.8; review fixes by Fable 5)

[github-actions]

1 blocker found.

1. Spec violation: client_id allows query parameters

File: src/lib/mcp/cimd.ts:349
What: The isCimdClientId validator checks for fragments (url.hash) but does not reject query parameters (url.search).
Why it matters: The CIMD draft (draft-ietf-oauth-client-id-metadata-document-00) and the MCP authorization spec explicitly prohibit client_id URLs from containing query or fragment components to prevent aliasing and ambiguity. Allowing query parameters violates the specification and could let multiple unique client_id strings resolve to the same metadata document.
Suggested fix: Add a check for url.search in isCimdClientId to ensure query parameters are rejected.

	if (url.username || url.password) return false;
	if (url.search || url.hash) return false;
	if (isIpLiteral(url.hostname)) return false;

Suggestions (non-blocking)

• src/lib/mcp/authorize.ts:251 — The confirmation form hardcodes action="/oauth/mcp/confirm". This will break if the plugin is registered under a different resource name (e.g., auth instead of oauth). Consider deriving the base path from the request or providing it via configuration.
• src/lib/mcp/wellKnown.ts:133 — The AS metadata document also hardcodes /oauth/ prefixes for its endpoints. Similar to the suggestion above, these should ideally be derived from the current resource mount point to support re-mountable plugin deployments.

[claude]

Reviewed; no blockers found.

[heskew]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements Client ID Metadata Document (CIMD) resolution for the Model Context Protocol (MCP) authorization flow, allowing clients to identify themselves using HTTPS URLs. It introduces a DNS-gated fetch mechanism with SSRF guards, process-level caching, and an interstitial confirmation page requiring explicit user confirmation before redirecting to the upstream IdP. The review feedback focuses on strengthening the SSRF filters for both IPv4 and IPv6 addresses, respecting 'no-store'/'no-cache' directives in Cache-Control headers, defensively handling potential runtime exceptions from malformed redirect URIs, and improving error observability by logging caught exceptions and preserving original error stacks using the 'cause' option.

[heskew]

Reviewed the security-critical surfaces (cimd.ts SSRF gate, the interstitial in authorize.ts, resolveClient wiring in token.ts, shared clientValidator). Two blockers, both in the areas this PR flagged for review. Neither is a design problem — both are contained fixes.

Blockers

1. SSRF DNS gate misses 0.0.0.0 / 0.0.0.0/8 and IPv6 :: — src/lib/mcp/cimd.ts (isPrivateIpv4 / isPrivateIpv6, ~L111–137)

isPrivateIpv4('0.0.0.0') returns false (a=0 matches none of the ranges), and 0.0.0.0 routes to loopback on Linux. Because the client controls the DNS record for their client_id URL host, pointing it at 0.0.0.0 passes checkHostSsrf and the subsequent fetch connects to the AS's own localhost services — a reachable SSRF-to-loopback bypass of the primary containment. IPv6 :: (unspecified) slips through the same way (firstGroup is empty → returns false).

Fix: reject a === 0 (0.0.0.0/8) in isPrivateIpv4, add 100.64.0.0/10 (CGNAT) while there, and reject the all-zeros :: in isPrivateIpv6. Add 0.0.0.0 and :: cases to cimd.test.js.

2. Consent interstitial is frameable (clickjacking) — src/lib/mcp/authorize.ts (HtmlResponse, ~L450–454)

The interstitial exists specifically to satisfy the spec's "clearly display the redirect URI hostname" consent requirement, but it's served with only Content-Type. A malicious CIMD client can frame it and clickjack the "Continue to sign in" POST for a victim already authenticated upstream, defeating the consent gate.

Fix: add X-Frame-Options: DENY and Content-Security-Policy: frame-ancestors 'none' (and Cache-Control: no-store, since the body carries the single-use confirm token) to the response headers.

Held up under scrutiny

DNS-rebinding TOCTOU is disclosed and accepted; the lying-content-length case is covered by the streaming byte cap; redirect: 'error' closes redirect-SSRF; escapeHtml covers every client-controlled interpolation; the shared clientValidator holds CIMD and DCR to identical redirect/grant rules; and the #159 handoff (jwks carried through, private_key_jwt gated off in v1) is wired correctly.

Flagging with extra weight because #161/#162 (the client_credentials grant for headless agents) now resolve clients through resolveClient() and will lean on this exact SSRF gate to fetch agent JWKS documents — the 0.0.0.0 gap should close before that builds on top.

🤖 Review by Claude (Fable 5)

[heskew]

Fresh pass on f0da8a1. I treated the existing SSRF range, bounded-cache, no-store/no-cache, error-observability, and clickjacking comments as already raised. I found three additional issues:

1. src/lib/mcp/cimd.ts:371 accepts non-2xx metadata responses. The resolver never checks response.ok/status, so a 404 or 500 with Content-Type: application/json and a syntactically valid CIMD body is accepted and cached as a client. The current CIMD draft requires the document to be served with 200 OK and treats other status codes as discovery errors. Please reject anything other than 200 before content-type/body parsing, and add a regression test for 404/500 JSON.

2. src/lib/mcp/cimd.ts:429 negative-caches invalid/error metadata documents. That is separate from the already-raised “Map is unbounded” issue: the current CIMD draft says the AS MUST NOT cache error responses or invalid/malformed documents. Today a bad document, unsupported auth method, oversized body, or non-JSON response is cached for 60s, and the test suite codifies that behavior. Please remove negative caching for CIMD discovery failures, or at least do not cache the classes the draft forbids.

3. src/lib/mcp/cimd.ts:354 uses fetchTimeoutMs and maxDocumentBytes directly from config. maxDocumentBytes: NaN or Infinity disables both size checks (contentLength > maxBytes and total > maxBytes are false), so the 64 KB cap fails open. This config path also supports env-expanded values, and nearby MCP config already coerces TTL strings defensively. Please coerce/validate these options to finite positive numbers before use, falling back to defaults or failing closed, and cover NaN/Infinity/numeric strings in tests.

Verified locally: npm run build; node --import ./test/helpers/harper-mock.mjs --test test/lib/mcp/cimd.test.js test/lib/mcp/authorize.test.js test/lib/mcp/wellKnown.test.js test/lib/mcp/token.test.js.

---

🤖 Posted by Codex (gpt-5.5) on Nathan's behalf

[heskew]

Fresh security pass on f0da8a1. I treated the already-raised SSRF-range, cache-bound, clickjacking/cache-control, non-200, negative-cache, and numeric-config findings as open and did not repeat them. I found the following additional issues.

1. Blocker: the CIMD consent is not bound to the resource owner's browser - src/lib/mcp/authorize.ts:432-525

   /authorize returns a bearer confirm_token, and /confirm validates only that token. A malicious CIMD client can retrieve and submit the interstitial itself, then send the resulting upstream IdP URL to a victim. The victim never sees the client identity or redirect-host disclosure, but an existing IdP session can still produce an authorization code at the attacker's redirect URI, with the attacker's PKCE verifier. This defeats the purpose of the interstitial.

   Bind the confirmation and subsequent upstream callback to a Secure, HttpOnly, SameSite browser nonce, or authenticate upstream first and render consent afterward. The callback must verify that same binding.

2. Blocker: a confirm token is accepted as upstream OAuth callback state - src/lib/mcp/authorize.ts:434, src/lib/handlers.ts:176-265

   Confirm tokens use the shared CSRF store and carry both mcp and _confirm. The callback treats any verified state carrying mcp as upstream state and never rejects _confirm. Supplying a confirm token as the upstream state therefore exchanges the upstream code, persists an MCP authorization code, and redirects it to the malicious client without calling /confirm.

   Add an enforced token purpose, for example cimd_confirm versus upstream_oauth, and reject each purpose on the wrong endpoint. Separate stores would be stronger. Add a callback regression test using a _confirm token.

3. High: fetchTimeoutMs ends at response headers, not document completion - src/lib/mcp/cimd.ts:354-406

   The abort timer is cleared as soon as fetch() returns a response. DNS resolution and the streamed body are outside that deadline, so a hostile CIMD server can send JSON headers and never finish the body, tying up unauthenticated requests and outbound connections indefinitely.

   Apply one deadline across DNS, connection, headers, and body parsing; keep the abort active through body consumption and cancel the reader on expiry.

4. Medium: cached clients survive a live redirect-host-policy tightening - src/lib/mcp/cimd.ts:338-344

   Positive cache entries are keyed only by client_id, even though validation depends on the live dynamicClientRegistration.allowedRedirectUriHosts setting. A redirect accepted before an operator restricts that setting remains accepted for the cached document lifetime, up to 24 hours.

   Revalidate cached redirects on every hit, include a policy fingerprint in the cache key, or clear CIMD cache entries when relevant MCP configuration changes.

5. Medium: the interstitial omits the authoritative client-ID hostname - src/lib/mcp/authorize.ts:218-229

   The page displays the redirect hostname and an optional attacker-controlled client_uri, but never displays the hostname of client_id, which is the domain that served the metadata document. A client can claim a trusted client_uri while using its own client-ID domain. The CIMD phishing guidance recommends displaying the client-ID hostname.

   Prominently render the escaped hostname from client.client_id; either omit client_uri or label it as unverified metadata.

6. Medium: discovery errors expose Harper's internal DNS view - src/lib/mcp/cimd.ts:160-164, src/lib/mcp/authorize.ts:352-356, src/lib/mcp/token.ts:129-131

   A private DNS result is embedded in CimdClientError and returned to unauthenticated callers. Requests for guessed internal names can reveal whether Harper resolves them and disclose the exact private address.

   Return a generic invalid_client description and log the hostname/address only server-side.

7. Low: dot-segment client IDs are accepted after URL normalization - src/lib/mcp/cimd.ts:182-195

   WHATWG URL parsing normalizes both literal and percent-encoded dot segments before this function validates the path. Inputs such as https://example.com/a/../client.json are accepted even though the CIMD draft prohibits dot path components, weakening simple-string client identity.

   Reject raw and percent-encoded single/double-dot path components before URL normalization, with regression coverage.

Verified locally: 202 targeted authorize, CIMD, callback, token, and discovery tests pass on this head.

[heskew]

All findings from the three review passes on f0da8a1 are addressed in 195cf21. Mapping:

Claude review (2 blockers)

1. SSRF gate 0.0.0.0/:: bypass → fixed. IPv4 gate now rejects 0/8, 100.64/10, 198.18/15, 224/4+ and fails closed on malformed input; IPv6 is a real parser with a fail-closed "global unicast (2000::/3) only" policy (v4-mapped classified by embedded IPv4), which also kills non-canonical loopback (::01, expanded form) and hex-form v4-mapped bypasses.
2. Frameable interstitial → fixed: X-Frame-Options: DENY, Content-Security-Policy: frame-ancestors 'none', Cache-Control: no-store.

Codex review (3 findings)

1. Non-200 accepted → fixed: only 200 OK resolves; 404/500-with-JSON regression tests added.
2. Negative caching forbidden by the draft → removed entirely; failures are never cached (repeat-fetch amplification is client_credentials (4/4): rate-limit token issuance (defense-in-depth) #163 rate limiting's job; the tests that codified negative caching now assert the opposite).
3. NaN/Infinity config fail-open → fixed: fetchTimeoutMs/maxDocumentBytes coerce to finite positive numbers (numeric strings honored) and fall back to defaults; covered for NaN/Infinity/strings/negatives.

Security pass (7 findings)

1. Consent not browser-bound (blocker) → fixed with a nonce cookie (HttpOnly, Secure, SameSite=Lax, 15 min): the interstitial response sets it, sha256(nonce) rides inside the confirm token and upstream state, and BOTH /oauth/mcp/confirm and the OAuth callback require a constant-time hash match before proceeding. New module src/lib/mcp/consentBinding.ts; attack-path regression tests for the self-approved interstitial in both legs. DCR flows carry no hash and are untouched.
2. Confirm token accepted as upstream state (blocker) → fixed: the callback rejects any verified state carrying _confirm exactly like an invalid token (no field of a mis-purposed token is trusted, including its redirect_uri). Regression test asserts no code minted and no upstream exchange.
3. Timeout ends at headers (high) → fixed: one deadline spans the DNS pre-flight (raced against the same abort signal), connect, headers, and full body read; trickling-body regression test.
4. Cache outlives policy tightening (medium) → fixed: cached records revalidate against the live allowedRedirectUriHosts on every hit and are evicted on failure.
5. Interstitial omits client_id host (medium) → fixed: the page now leads with "Client identity: <client_id host>" and labels client_uri as "Claimed application domain (unverified)".
6. DNS error detail leak (medium) → fixed: every gate rejection (didn't resolve / no addresses / blocked address) returns one generic invalid_client message; hostname/address detail is logged server-side only.
7. Dot-segment client_ids (low) → fixed: raw and percent-encoded dot path segments are rejected before URL normalization; non-lowercase scheme spellings rejected too (the document's exact-match client_id could never validate them anyway).

Also folded in the gemini bot rounds (all inline threads replied + resolved): bounded LRU cache (1000 entries), no-store/no-cache floored at the 60 s DoS floor (documented deviation), error-cause chaining, defensive redirect_uri rendering, and logging on the swallowed catch paths.

Docs (docs/mcp-oauth.md, docs/configuration.md) updated to match. Suite: 923 tests, 921 pass, 2 pre-existing skips (52 new/updated).

One deliberate deviation to sanity-check: DNS resolution failure is now reported as 400 invalid_client (generic) rather than 500 — making it distinguishable from the blocked-address case would have kept the internal-DNS probing channel open via status codes.

🤖 Response by Claude (Fable 5) on Nathan's behalf

[heskew]

Security review — request changes

Reviewed current head 6fb7dc84 with two independent passes and targeted validation. I found three high-severity issues and four medium-severity issues. The full suite passes locally (926 tests, 924 pass, 2 existing skips), but these need resolution before merge.

1. Sibling-domain cookie injection bypasses CIMD consent — High

File: src/lib/mcp/consentBinding.ts:28

mcp_consent is a normal cookie name. If the AS is auth.example.com, an attacker controlling evil.example.com can fetch its own interstitial server-side to obtain a nonce and confirm_token, set mcp_consent=<nonce>; Domain=example.com in the victim's browser, and auto-submit the confirm form. Since sibling origins are same-site, SameSite=Lax does not block this. Both /confirm and the callback accept the planted cookie, so the victim can complete upstream login and the attacker receives a code at its redirect URI with the attacker's PKCE verifier, without the victim seeing the interstitial.

Rename this to a per-flow __Host-... cookie (the existing Secure; Path=/ and no Domain are compatible), and bind the flow ID in state. Also require the configured issuer as the Origin on /oauth/mcp/confirm as defense in depth. __Host- prevents a sibling from planting a parent-domain cookie; see RFC6265bis §4.1.3.2.

2. DNS timeout does not terminate dns.lookup() work — High

File: src/lib/mcp/cimd.ts:351

withAbort() only rejects the caller; it cannot cancel the underlying dns.lookup(). An attacker can send many client IDs under DNS zones that drop queries. Each request returns after five seconds, but the synchronous getaddrinfo() calls keep occupying libuv's fixed thread pool, starving unrelated DNS/filesystem work. Node documents this exact dns.lookup() behavior in its DNS implementation notes.

Use cancellable DNS resolution and a global in-flight CIMD resolution bound. Do not release a concurrency permit until the underlying lookup has actually stopped.

3. The SSRF preflight is not connection-bound — High

File: src/lib/mcp/cimd.ts:542

checkHostSsrf() validates one DNS answer, then fetch(clientId) resolves the hostname again. An attacker-controlled authority can return a public address to the preflight and an internal/special-use address to the actual connection. TLS narrows the exploitable targets but does not eliminate internal TLS probing or services with compatible SNI/certificates. The PR documents this residual risk, but the current CIMD draft requires authorization servers not to fetch URLs that resolve to special-use addresses.

Pin the HTTP connection to the validated address using a custom lookup/agent while preserving the original hostname for Host, SNI, and certificate verification. Revalidate every retry address. CIMD draft §8.6

4. The IP filter permits RFC 6890 special-use space — Medium

File: src/lib/mcp/cimd.ts:127

The filter allows 192.0.0.0/24, TEST-NET ranges, 2001:2::/48, 2001:db8::/32, and other special-use prefixes. It also intentionally allows public-target 2002::/16 addresses, although 6to4 is special-use. These answers reach _fetch(); I reproduced this with 192.0.2.1.

Classify against the complete IANA IPv4 and IPv6 special-purpose registries and add table-driven tests for every denied prefix. IANA IPv4 registry, IANA IPv6 registry

5. Browser binding runs after upstream login side effects — Medium

File: src/lib/handlers.ts:241

On the self-approved-interstitial path, the cookie mismatch is checked only at line 283. Before that, the callback exchanges the upstream code, loads identity data, and invokes onLogin, which can provision users, synchronize roles, or cause external mutations. The fix prevents MCP code issuance but not attacker-initiated hook effects.

Validate browserNonceHash immediately after state-purpose and provider validation, before upstream errors, code exchange, userinfo, or hooks. Add a regression that a mismatch invokes neither exchangeCodeForToken nor onLogin.

6. CIMD security configuration can fail open — Medium

Files: src/lib/mcp/cimd.ts:485, src/lib/mcp/cimd.ts:649

Environment expansion preserves strings, so enabled: ${CIMD_ENABLED} with CIMD_ENABLED=false becomes "false"; enabled !== false still enables CIMD. A scalar allowedHosts becomes a string and uses substring matching, and an empty list skips the restriction completely.

Normalize/validate configuration at load time: explicitly coerce documented boolean strings, require an array of normalized exact hostnames, and reject invalid or ambiguous security-policy values instead of treating them as unrestricted.

7. One global consent cookie breaks concurrent CIMD flows — Medium

File: src/lib/mcp/consentBinding.ts:28

Every interstitial overwrites mcp_consent. Opening a second authorization flow causes the first to fail either at /confirm or after upstream authentication. This occurs with ordinary parallel tabs and permits deliberate disruption of a pending authorization.

Use bounded per-flow __Host- cookies keyed by a random flow identifier carried in confirmation and upstream state, and clear only the completed flow.

[heskew]

Thanks — strong pass. All seven addressed on 7f07d87 (with 6fb7dc8). Per-finding:

1. Sibling-domain cookie injection (High) — fixed. The consent cookie is now a per-flow __Host--prefixed name (__Host-mcp_consent_<flowId>). __Host- forbids a Domain attribute, so a sibling origin can't plant a parent-domain cookie to forge the binding. I did not add the issuer-Origin check on /confirm — the __Host- prefix closes the attack on its own; happy to add the Origin check too if you'd rather have the belt-and-suspenders.

2. dns.lookup() not cancellable (High) — fixed. Concurrent CIMD DNS resolutions are bounded by a global permit (MAX_CONCURRENT_DNS) that is released only when the underlying getaddrinfo settles, not when the caller's deadline fires — so a flood of blackholed-DNS client_ids can't pin the libuv pool. Over the bound, resolution fast-rejects (temporarily_unavailable); concurrent resolutions of the same client_id are also deduped to one fetch. Test asserts peak concurrency ≤ cap + saturation fast-reject.

3. Preflight not connection-bound (High) — fixed with pinned-connect. The fetch now goes through https.request with a custom lookup that returns the exact address the gate validated; the hostname is kept for TLS SNI + cert verification. The socket therefore connects to a validated IP with no second resolution, closing the rebind TOCTOU. (No new dep: undici's Agent isn't importable here, and https.request also gives no-redirect-follow for free.) The gate resolves once and hands the addresses to the fetch, so there's no double lookup.

4. RFC 6890 special-use space permitted (Medium) — fixed. isPrivateIpv4/isPrivateIpv6 are now table-driven against the IANA IPv4/IPv6 special-purpose registries: added 192.0.0/24, 192.0.2/24 (TEST-NET-1), 192.88.99/24, 198.51.100/24 (TEST-NET-2), 203.0.113/24 (TEST-NET-3), AS112/AMT, and the in-2000::/3 IPv6 special-use (2001:db8::/32, 2001:2::/48, ORCHID, 3fff::/20), with per-prefix tests. 192.0.2.1 is now rejected.

5. Binding runs after login side effects (Medium) — fixed. The browserNonceHash check moved to before exchangeCodeForToken and onLogin (right after state-purpose + provider validation). A mismatched self-approved flow now triggers no upstream exchange and no hook side effects. Regression asserts neither exchangeCodeForToken nor onLogin runs on mismatch.

6. CIMD config can fail open (Medium) — fixed. normalizeMcpSecurityConfig runs at load (after env expansion): coerces documented boolean strings so an env-expanded "false" truly disables the feature (this also fixes the master mcp.enabled switch), and normalizes allowedHosts to an array of exact lowercased hostnames — a scalar is wrapped (no substring matching), non-strings are rejected rather than treated as "no restriction".

7. One global consent cookie breaks concurrent flows (Medium) — fixed by the per-flow cookie in #1; parallel tabs now carry independent __Host-mcp_consent_<flowId> cookies. Per-flow naming + Max-Age makes explicit clearing unnecessary for correctness.

Verified: npm run build; full suite 938 tests / 936 pass / 2 pre-existing skips; lint + prettier clean. Docs (docs/mcp-oauth.md) updated for the pinned-connect, per-flow cookie, and config-normalization semantics.

🤖 Response by Claude (Fable 5) on Nathan's behalf

[heskew]

Security and performance review:

• High: CIMD rejection paths can leave response bodies open. A malicious endpoint can send headers, then hold the socket open after non-200, non-JSON, or oversized responses.
• High: DNS throttling does not bound total CIMD fetch concurrency. Unique client_id URLs can fan out into unbounded outbound HTTPS work.
• Medium: MAX_CONCURRENT_DNS=8 can still starve Node's default 4-worker libuv pool under slow lookups.
• Cleanup: jwks and jwks_uri plumbing is not used in this PR. Defer it until there is validation and a consumer.

The SSRF guard, pinned connection, and browser-bound consent flow are justified and should stay.

[github-actions]

🔴 Blocker — Spec violation: client_id allows query parameters

The CIMD draft prohibits query parameters in the client_id URL. Add a check for url.search in isCimdClientId to reject them.

[github-actions]

💡 Suggestion (non-blocking) — Avoid hardcoded path in interstitial form

Hardcoding /oauth/mcp/confirm breaks the flow if the plugin is mounted at a different path. Derive the base path from the request context instead.