RFC: Client ID Metadata Document (CIMD) support#71
Merged
Conversation
Proposes CIMD support on both sides of the ToolHive proxy: as an OAuth client preferring CIMD over DCR when connecting to remote MCP servers (thv run), and as an authorization server accepting HTTPS URLs as client_id from MCP clients (embedded AS). Closes stacklok/toolhive#2728 Related: stacklok/toolhive#4825, stacklok/toolhive#4826 Signed-off-by: amirejaz <amir@stacklok.com>
Signed-off-by: amirejaz <amir@stacklok.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces RFC-0071 proposing support for Client ID Metadata Documents (CIMD) in ToolHive, covering both (1) thv run acting as a CIMD-aware OAuth client when talking to remote authorization servers, and (2) the embedded authorization server accepting CIMD client_id URLs from MCP clients to avoid Dynamic Client Registration (DCR) round-trips.
Changes:
- Adds a new RFC detailing a two-phase rollout plan for CIMD support (proxy client first, embedded AS second).
- Defines the intended discovery metadata flag (
client_id_metadata_document_supported), caching/SSRF protections, and fallback behavior to DCR. - Specifies the hosted ToolHive client metadata document and operational requirements for serving it.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Fix spelling: defence → defense (two occurrences) - Align config naming: CIMDEnabled → authServer.cimd.enabled - Resolve redirect/non-200 contradiction in Input Validation - Add IPv6 ULA (fc00::/7) to SSRF blocked ranges; frame as globally routable IPs only for future-proofing Signed-off-by: amirejaz <amir@stacklok.com>
Remove inaccurate port-agnostic http://localhost/callback entry. The actual default callback port is 8666, defined as DefaultCallbackPort in pkg/auth/remote/config.go and configurable via --remote-auth-callback-port. Signed-off-by: amirejaz <amir@stacklok.com>
Blocker: - Explicitly list ClientIDMetadataDocumentSupported field addition in Phase 1 task list; note both RFC 8414 and OIDC discovery must be parsed High severity: - Add PKCE S256 requirement for CIMD flows (mandatory per MCP 2025-11-25) - Expand SSRF protection: use net.IP.IsGlobalUnicast() as allowlist baseline; add 100.64.0.0/10, IPv4-mapped IPv6, documentation ranges - Add DisableKeepAlives and dual-stack DialContext requirements to SSRF transport to prevent keep-alive and Happy Eyeballs bypasses - Change self-referential binding from strict string comparison to RFC 3986 canonical normalization (lowercase host/scheme, strip default ports) - Expand DCR fallback trigger: invalid_client, unauthorized_client, invalid_request, raw HTTP 400/401 at authorization stage only - Add cache floor (60s), ceiling (24h), and no-store handling - Document refresh token behavior: document not re-fetched on refresh - Document air-gapped limitation for Phase 2 self-fetch scenario - Add Content-Type validation before JSON parsing Low/informational: - Note SHOULD vs MUST normative strength of priority chain in Summary - Add LRU cache flooding threat to threat model - Add compromised hosting as accepted residual risk in threat model - Note IETF draft-01 version in References Signed-off-by: amirejaz <amir@stacklok.com>
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
jhrozek
reviewed
Apr 24, 2026
- Move cimd.go to pkg/oauth (protocol-level, not auth-specific) - Replace new ssrf.go with enhancement to existing pkg/networking HttpClientBuilder; CIMD fetches reuse existing SSRF infrastructure with missing ranges added (100.64.0.0/10, IPv4-mapped, doc ranges) - Clarify CIMD carries no credentials; rejection = AS compatibility issue, not expiry - Note logo_uri is a URL reference, not inline data; 10 KB cap is for the JSON document only - Note custom metadata URL override as a follow-up enhancement - Clarify opt-in applies to Phase 2 only (outbound fetch gating); Phase 1 is automatic with no config flag Signed-off-by: amirejaz <amir@stacklok.com>
jhrozek
reviewed
Apr 27, 2026
Contributor
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
Second-wave review based on a deeper cross-check against the toolhive code. Most of these are concerns about the Phase 2 decorator design and a few smaller nits. Skipping straight to inline comments below.
- Revert to strict string comparison for self-referential binding (spec-mandated; full IDN normalization required to be safe, adds complexity for a case well-implemented clients never produce) - Fix PerformOAuthFlow sketch: set config.ClientID before the DCR gate (shouldDynamicallyRegisterClient checks ClientID == empty) - Narrow DCR fallback: remove invalid_request (request-shape error, not identity rejection; keeping it could mask errors or enable forced downgrade to DCR) - Expand decorator: embed full storage.Storage (~15 methods), add singleflight.Group to deduplicate concurrent fetches on anonymous /authorize, expose Unwrap() for legacy RedisStorage type assertion - Document second GetClient call site in callback.go; wired via storage embedding, no extra work needed - Document RedisStorage type assertion landmine in server_impl.go; must run migration before wrapping or use Unwrap() - Expand Phase 2 implementation plan with all files touched by config threading (RunConfig -> Config -> handler -> decorator) - Fix AuthorizationServerMetadata field location: pkg/oauth/discovery.go (shared package), not the authserver handler - Fix SSRF threat row to match expanded pkg/networking IP ranges - Narrow Phase 1 test description to match updated fallback conditions - Update Last Updated date Signed-off-by: amirejaz <amir@stacklok.com>
jhrozek
approved these changes
Apr 27, 2026
Contributor
jhrozek
left a comment
jhrozek
left a comment
There was a problem hiding this comment.
Good work on the revisions — thanks for the thorough turnaround.
3 tasks
Contributor
|
@amirejaz wanna merge this? :-) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Proposes CIMD (Client ID Metadata Document) support across two phases:
thv runacts as a CIMD-aware OAuth client. When a remote authorization server advertisesclient_id_metadata_document_supported: true,thv runpresents its hosted metadata URL (https://toolhive.dev/oauth/client-metadata.json) as theclient_idinstead of performing a DCR round-trip.client_idand complete the full authorization flow without calling/oauth/register.Motivation
The MCP specification (2025-11-25) defines CIMD as the preferred client registration mechanism:
pre-registered credentials → CIMD → DCR → user prompt. ToolHive currently jumps straight to DCR. This is both a spec-alignment issue and a friction-reduction issue — DCR requires a registration round-trip that CIMD eliminates entirely.Related issues
Generated with Claude Code