feat: support Client ID Metadata Documents (CIMD) alongside DCR - #62
Draft
nonchan7720 wants to merge 3 commits into
Draft
feat: support Client ID Metadata Documents (CIMD) alongside DCR#62nonchan7720 wants to merge 3 commits into
nonchan7720 wants to merge 3 commits into
Conversation
Implements CIMD (SEP-991) from the MCP 2025-11-25 spec update in both
directions while keeping existing Dynamic Client Registration (RFC 7591):
Authorization server side:
- Accept HTTPS URLs as client_id: fetch, validate (client_id match,
redirect_uris, public client only) and cache the metadata document
- Advertise client_id_metadata_document_supported in AS metadata
- Resolve the target MCP server from the RFC 8707 resource parameter
for CIMD clients that have no pre-registration
Client side (upstream MCP servers):
- Serve manifold's own CIMD document at
GET /{server}/auth/client-metadata.json
- Prefer CIMD during OAuth discovery when the upstream AS advertises
support and the gateway is served over HTTPS; fall back to DCR
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019mm3HwL3YGD2uoGYUhkKft
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CIMD requires fetching the client-supplied client_id URL by design. SSRF is mitigated by enforcing the https scheme, blocking private IPs via SafeHTTPClient, and applying size/content-type limits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019mm3HwL3YGD2uoGYUhkKft
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pkg/interfaces/http/cimd.go (1)
184-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCIMD ドキュメント URL の生成を 1 か所に集約してください。
clientMetadataDocumentURL（189 行）とClientMetadataDocument（206 行）が同じ書式を重複して組み立てています。ベース URL に末尾スラッシュが付く場合、両方が二重スラッシュを生成します。url.JoinPathを使うヘルパーへ統一すると、client_idの不一致を防げます。Also applies to: 205-207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/interfaces/http/cimd.go` around lines 184 - 190, Consolidate CIMD URL construction in clientMetadataDocumentURL and ClientMetadataDocument by introducing or reusing one helper based on url.JoinPath. Preserve the existing HTTPS validation and ensure gatewayBaseURL values with or without a trailing slash produce the same URL and client_id.pkg/interfaces/http/cimd_test.go (1)
191-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueサイズ上限のテストを追加してください。
fetchCIMDDocumentはcimdMaxBodySizeを超えるレスポンスを拒否します。この境界は現在テストされていません。1 MB を超えるボディを返すハンドラでエラーを確認するテストを追加してください。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/interfaces/http/cimd_test.go` around lines 191 - 213, Add a test alongside TestFetchClientIDMetadata_WrongContentType and TestFetchClientIDMetadata_NotFound that makes the test server return a response body larger than 1 MB, invokes fetchClientIDMetadata, and asserts an error is returned, covering the cimdMaxBodySize enforcement in fetchCIMDDocument.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/interfaces/http/auth_handler.go`:
- Around line 554-560: Before resolving the server in the resource lookup block,
validate that the resource URL’s host matches the gateway base URL, rejecting
resources from arbitrary hosts such as evil.example.com. Update the flow around
serverNameFromResource so path-based server resolution and the existing
h.servers lookup occur only after this host validation.
In `@pkg/interfaces/http/cimd.go`:
- Around line 82-89: Update the httpClient fallback in the AuthHandler flow
around fetchCIMDDocument to use the project’s SSRF-protected client package
instead of http.DefaultClient, and configure a request timeout for CIMD
retrieval. Preserve the existing h.httpClient path while ensuring nil-client
construction remains protected against private IP access and unbounded waits.
- Around line 110-121: Update fetchCIMDDocument to prevent unsafe redirects:
configure the HTTP request/client flow to reject redirects, or validate every
redirect target before following it, ensuring each CIMD URL remains HTTPS and
meets the existing host restrictions. Preserve the current request and response
handling for valid non-redirected URLs.
---
Nitpick comments:
In `@pkg/interfaces/http/cimd_test.go`:
- Around line 191-213: Add a test alongside
TestFetchClientIDMetadata_WrongContentType and
TestFetchClientIDMetadata_NotFound that makes the test server return a response
body larger than 1 MB, invokes fetchClientIDMetadata, and asserts an error is
returned, covering the cimdMaxBodySize enforcement in fetchCIMDDocument.
In `@pkg/interfaces/http/cimd.go`:
- Around line 184-190: Consolidate CIMD URL construction in
clientMetadataDocumentURL and ClientMetadataDocument by introducing or reusing
one helper based on url.JoinPath. Preserve the existing HTTPS validation and
ensure gatewayBaseURL values with or without a trailing slash produce the same
URL and client_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb53cd21-081f-4a0f-9429-b111fd202672
📒 Files selected for processing (5)
README.ja.mdREADME.mdpkg/interfaces/http/auth_handler.gopkg/interfaces/http/cimd.gopkg/interfaces/http/cimd_test.go
Address review findings: - Fall back to SafeHTTPClient instead of http.DefaultClient so the SSRF protection holds on every construction path, and bound the CIMD fetch with a 10s timeout - Stop following redirects when fetching CIMD documents so the https-only validation cannot be bypassed via a redirect hop - Require the RFC 8707 resource host to match the gateway host before resolving an MCP server from it - Consolidate CIMD document URL construction into a single helper - Add tests for the body size limit and redirect rejection Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019mm3HwL3YGD2uoGYUhkKft
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
2 participants
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.
What does this PR do?
MCP 2025-11-25 仕様アップデートの CIMD (Client ID Metadata Documents, SEP-991) に、既存の Dynamic Client Registration (RFC 7591) を維持したまま双方向で対応します。
内蔵認可サーバー側（MCP クライアント → Manifold）
client_idとして HTTPS URL を受け付け、その URL から CIMD ドキュメントを取得・検証（client_idと取得元 URL の一致、redirect_urisの検証、パブリッククライアントのみ許可）。検証済みドキュメントは store に 5 分間キャッシュSafeHTTPClient（プライベート IP 拒否）+ Content-Type 検証 + 1MB サイズ上限client_id_metadata_document_supported: trueを追加resourceパラメータ（https://host/mcp/{name}）から対象 MCP サーバーを解決するフォールバックを追加/auth/clients,/register）は変更なしMCP クライアント側（Manifold → 上流 MCP サーバー）
GET /{server}/auth/client-metadata.jsonで Manifold 自身の CIMD ドキュメントを配信client_id_metadata_document_supportedを広告し、かつゲートウェイが HTTPS で公開されている場合は CIMD を優先し、それ以外は従来どおり DCR にフォールバックその他
LoginEndpointのクライアント解決／サーバー解決をresolveLoginClient/resolveLoginServerに抽出（gocyclo 閾値対応）Related issues
なし
Checklist
make testpasses locally（CI=true go test ./pkg/...全パッケージ成功。pkg/internal/clientの一部テストはCI環境変数なしでは main でも失敗する既存事象）make lintpasses locally（ローカルの golangci-lint バイナリが Go 1.26 非対応のため実行不可。gocyclo は手動確認済み）🤖 Generated with Claude Code
https://claude.ai/code/session_019mm3HwL3YGD2uoGYUhkKft
Generated by Claude Code
Summary by CodeRabbit
新機能
client_idからクライアント情報を取得・検証できるようになりました。ドキュメント