docs: scope CIMD to replace open DCR (trust-policy-gated) - #143
Conversation
Expands the top-ranked alignment improvement into an implementable plan. Recommends a trust-policy-gated, additive design: fetch a Client ID Metadata Document only when the client_id URL's host is already on the curated vendor allow-list, keep open DCR for everything else, and never trust the document's display fields. This collapses the new outbound-fetch surface to a finite set of vetted hosts instead of an arbitrary-URL SSRF primitive on the unauthenticated /authorize path, while still delivering spec alignment and a DNS/TLS-authenticated domain key for branding. Folds in the alignment PR's review correction: skills.rs's markdown_url_for_base is NOT a usable SSRF guard (host-string compare only); the real building block is discover.rs's address-pinned fetcher (resolve_public_url + site_client + ssrf_redirect_policy), which the plan extracts into a shared module as its Phase 0. Covers the fetch/validate/ cache flow, allow-list re-keying, branding subsumption, a security analysis, phasing, and open questions — all cited against current code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Scopes a trust-policy-gated CIMD implementation while retaining DCR compatibility.
Changes:
- Defines CIMD fetching, validation, caching, and trust-policy design.
- Plans SSRF-safe fetcher reuse and branding integration.
- Documents security considerations and implementation phases.
Suppressed comments (2)
docs/scoping-cimd.md:146
- Keeping this redirect policy bypasses the trust-policy gate after the first request.
redirect_hop_okpermits redirects to any global IP literal (discover.rs:1145-1153), so a vetted-host URL that exposes an open redirect can make/authorizefetch an arbitrary public destination despite the document's “finite set of vetted hosts” guarantee. Disable redirects for CIMD (or re-run the exact origin policy and address pinning for every hop).
- Keep the 15 s timeout, address pinning, `https`-only, and bounded redirects.
docs/scoping-cimd.md:174
DEFAULT_ALLOWED_REDIRECTSis not itself a set of exact vetted client-ID hosts:redirect_uri_permitteddeliberately extends each entry to every subdomain (auth.rs:598-600), while the path pin supplies the remaining safety. Reusing that projection for a host-only CIMD policy would silently trust additional subdomains. Keep one vendor record if desired, but give it explicit exact CIMD origins rather than deriving them through the redirect matcher.
Introduce a **client-id-host trust policy** (the spec's "domain allowed via
trust policy"). Recommendation: derive it from the *same curated vendor set*
that backs `DEFAULT_ALLOWED_REDIRECTS` (`auth.rs:434`) so there is one source of
truth for "who is a vetted vendor," rather than a second independent list.
`redirect_uri_permitted` continues to gate the redirect leg.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… rate cap Address the review on the CIMD scoping doc: * Trust policy matches exact HTTPS ORIGINS, not bare hosts. resolve_public_url uses the caller-supplied port (discover.rs:1113), so a host-only gate would let https://claude.ai:8443/... reach an unvetted port; require the default 443 (and reject userinfo/other non-canonical authority) before any DNS/fetch. * CIMD must use a STRICT capped reader, not discovery's best-effort one (discover.rs:1207-1228 returns lossy/partial text without signaling — a truncated body whose prefix is valid JSON would be accepted as metadata at the auth boundary). Fail closed on over-limit/stream-error/invalid-UTF-8. * client_id validation is a plain STRING match against the requested URL — no normalization (hosted redirects use exact string membership, auth.rs:648, and normalizing would mint aliases that disagree with the raw-URL cache key). * Make the outbound-DoS control mandatory: the cache does NOT bound misses, because an attacker can vary the URL PATH on a vetted host to mint unlimited distinct keys and concurrent 15s fetches. A per-host + global concurrency/rate cap (plus negative-caching) is now a Phase 1 acceptance criterion, not an optional nicety. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
docs/scoping-cimd.md:59
- HTTPS does bind the returned metadata values to the origin that served them, so they are not as unauthenticated as an arbitrary open-DCR body. What TLS does not establish is the semantic truth of the claimed product name or logo. Clarify that distinction while retaining curated branding.
CIMD provides **no signing or attestation** of the document's contents — the
display fields (`client_name`, `logo_uri`) are exactly as spoofable as a DCR
body. The only cryptographically meaningful fact is the **host** of the URL.
docs/scoping-cimd.md:273
- A non-vetted URL-form identifier cannot silently fall back to the existing DCR path.
/oauth/registeralways mints an opaqueclient-<uuid>(src/auth.rs:2148), andvalidate_clientonly looks up that supplied identifier, so the URL will never be a DCR registration. The compatibility section should instead explain that such clients must register and retry with the returned opaque ID.
1. **Non-vetted URL `client_id`:** hard reject, or silently fall back to DCR? A
reject is clearer and avoids a confusing partial-support surface; a fallback
is more permissive. Recommend reject with an allow-listing contact.
…gate) Review fix: the doc said to keep discovery's redirect policy, but ssrf_redirect_policy / redirect_hop_ok (discover.rs:1145-1153) follow any global-IP literal and any same-host hop WITHOUT a port check — so a vetted client_id could redirect the fetch to an unvetted public IP or to vetted-host:8443, escaping the exact-origin gate of §3.1. A CIMD is served directly at its URL, so §3.3 now says to disable redirects entirely (or, if truly needed, require an exact same-origin hop), never the discovery policy. §5's SSRF bullet notes the control. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
docs/scoping-cimd.md:282
- This fallback option is not implementable with the existing DCR contract:
/oauth/registerconsumes a registration request and returns a newly minted opaque ID, so/oauth/authorizecannot transparently convert a URL client ID into DCR. Make rejection the decision; clients choosing DCR must register first and retry with the returned ID.
1. **Non-vetted URL `client_id`:** hard reject, or silently fall back to DCR? A
reject is clearer and avoids a confusing partial-support surface; a fallback
is more permissive. Recommend reject with an allow-listing contact.
docs/scoping-cimd.md:243
- Keep the negative-cache requirement scoped to document-intrinsic/non-CIMD failures. As written, “failed” can include the request-specific redirect validation defined in §3.4, allowing one malicious redirect request to install a URL-level negative entry for an otherwise valid client.
vetted host cannot fan out, and negative-cache failed/for-non-CIMD URLs so a
docs/scoping-cimd.md:74
OAUTH_ALLOWED_REDIRECT_PREFIXESdoes not override this list:allowed_redirects()appends parsed entries toDEFAULT_ALLOWED_REDIRECTS(src/auth.rs:460-475), and the README likewise documents it as additive (README.md:660-663). Calling it overridable can mislead operators into expecting that defaults can be removed.
is exempt (RFC 8252). `allowed_redirects()` (`auth.rs:457`) is overridable via
`OAUTH_ALLOWED_REDIRECT_PREFIXES`.
docs/scoping-cimd.md:136
- A URL-form request cannot silently fall back to DCR. DCR requires a separate registration body and always mints a different opaque
client-<uuid>(src/auth.rs:2058-2065, 2148-2159), while/authorizeonly has the presented URL and one redirect. Treating the URL as an opaque DCR ID would simply fail lookup, so this branch must reject and tell the client to register/retry instead.
This issue also appears on line 280 of the same file.
- If the URL's **origin** (scheme + host + default 443 port) is **not** on the
client-id trust policy → **reject** with a clear error naming the contact
for allow-listing (mirroring the DCR hosted-redirect rejection). *(Reject vs
silent DCR-fallback is an open question — see §8.)*
Review fix: §3.4 folded request-specific redirect checks into "validation," so negative-caching every validation failure by URL would let an attacker poison a valid client — request a real CIMD URL with a non-member redirect_uri, the URL gets cached as invalid, and legitimate redirects then hit the negative entry. Split validation into document-intrinsic (cacheable: fetch/JSON/client_id==URL) and per-request (never cached: redirect membership + redirect_uri_permitted, re-run every request against the positively-cached document). §3.5 and the §5 DoS bullet now negative-cache only document-intrinsic failures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (5)
docs/scoping-cimd.md:208
- “Didn't resolve” includes transient DNS, connection, timeout, 429, and 5xx failures. Treating all of these as normal negative entries can keep a legitimate CIMD unavailable after a brief vendor outage. Separate deterministic invalid-document failures from transient fetch failures, using only a short retry backoff for the latter.
- Also **negative-cache** URLs that fail a **document-intrinsic** check (§3.4) —
didn't resolve to a valid CIMD, bad JSON, `client_id` ≠ URL — so a repeat of
the same bogus path is cheap. **Never** negative-cache a **per-request**
docs/scoping-cimd.md:59
- This overstates the spoofing risk and contradicts the preceding TLS-authentication explanation. Unlike open DCR, an arbitrary caller cannot choose CIMD display values; HTTPS authenticates them as assertions by the URL's host. They still do not attest a product/legal identity, so curated branding is reasonable, but the document should state that narrower rationale.
CIMD provides **no signing or attestation** of the document's contents — the
display fields (`client_name`, `logo_uri`) are exactly as spoofable as a DCR
body. The only cryptographically meaningful fact is the **host** of the URL.
docs/scoping-cimd.md:190
- This membership rule omits the existing RFC 8252 loopback exception.
redirect_allowedaccepts an exact match orloopback_match(src/auth.rs:648-650), allowing a native client to bind an ephemeral port. A CIMD client listinghttp://127.0.0.1/callbackwould otherwise fail when authorizing with its runtime port.
- The request's `redirect_uri` is a member of the cached document's
`redirect_uris` **and** still passes `redirect_uri_permitted` (`auth.rs:545`).
docs/scoping-cimd.md:202
- A TTL floor does not honor the publisher's cache policy: it overrides
no-store,no-cache, ormax-age=0and can keep accepting a redirect URI after the vendor tries to revoke it. Retain the ceiling for stale-data safety, but honor immediate revalidation/eviction directives and use the required limiter to control re-fetch load.
- Cache **validated** documents keyed by the `client_id` URL, honoring
`Cache-Control` / `ETag` with a **TTL floor and ceiling** so a hostile
`max-age` can neither pin a stale doc forever nor force a re-fetch per request.
docs/scoping-cimd.md:224
- The current redirect entries cannot be reused as exact origins: they are registrable-domain rules that deliberately match every subdomain (
src/auth.rs:594-600), and some intended hosts differ from the entry (for example,cursor.comcoverswww.cursor.com). Deriving the CIMD gate directly either trusts all subdomains, violating the exact-origin boundary, or trusts only apex hosts and rejects intended clients. Use one vendor record with separate explicit client-ID origins and redirect domain/path rules.
Introduce a **client-id-ORIGIN trust policy** (the spec's "domain allowed via
trust policy") — exact `https://<host>` entries on the default 443 port, matched
as origins so the port gap in §3.1 cannot slip a non-default port past a host
check. Recommendation: derive it from the *same curated vendor set* that backs
`DEFAULT_ALLOWED_REDIRECTS` (`auth.rs:434`) so there is one source of truth for
Align the Client ID Metadata Document support with the scoping in PR #143 and the third review round. Trust policy: a URL client_id is fetched only when its origin is a vetted vendor's — a host on or under a domain of the hosted-redirect allow-list, on the default https port. Anything else is refused before any request goes out and, like a hosted redirect off the allow-list, pointed at the allow-listing contact (403 invalid_client, or the not-approved page for a browser). The one vendor list decides both where a code may land and whose document this server will GET. Opt-in: CIMD is advertised and URL client_ids accepted only where the deployment sets OAUTH_CIMD_ENABLED=1. The deploy template takes the variable from the GitHub Environment, so a routine deploy never switches the directory clients over by itself; unsetting it is the rollback. Negative cache: a failure that is about the URL itself (404, a redirect, not JSON, about another URL, too large, not UTF-8) is remembered for a minute so a repeat costs no fetch. A transient one (deadline, connection, 5xx, 429) is not, and a per-request failure (a redirect the document does not list) never is, so a probe cannot lock out a real client. The fetcher's errors are typed to make that split. Single-flight shares the outcome: concurrent misses for one document share the one fetch's result — failure and uncacheable document included — instead of re-fetching serially behind it, and the flight entry is retired only by the flight that made it. A document may list no more redirect_uris than a DCR registration. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
…cher The tenth review round, four findings: - The SSRF guard also refuses 100::/64, the IANA discard-only block. - A document's redirect_uris are kept only when a DCR registration could have registered them (redirect_uri_permitted: loopback, or on an allow-listed host and pinned path, never with query or fragment), besides being same-origin. A loopback entry with a fragment would otherwise have matched a fragment-free request the port-agnostic match ignores fragments on — a redirect DCR refuses. - Fetches are rate-limited, not only bounded in flight: a token bucket per process (60 a minute) and per vetted domain (30 a minute), since an origin answering at once returns its permit at once and distinct paths defeat the negative cache. The rate cap PR #143 §5 requires. - Every request in a flight holds a guard, and the last one out retires the flight. A fetcher cancelled mid-way therefore leaves the flight where its waiters and any newcomer find it, and a waiter takes over the one fetch, instead of waiters and newcomers fetching the same document on two flights. The fixture gained a hanging origin to test the handover. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
What
A dedicated scoping doc (
docs/scoping-cimd.md) that turns the top-ranked item from the MCP 2026-07-28 alignment doc (#126, "CIMD replacing open DCR") into an implementable plan.Recommendation: trust-policy-gated, additive CIMD
Fetch a Client ID Metadata Document only when the
client_idURL's host is already on the curated vendor trust policy; keep open DCR for everything else; never trust the document'sclient_name/logo_uri. This:/authorize" to a finite set of vetted hosts (no arbitrary-URL SSRF), andKey correction carried over from #126's review
skills.rs'smarkdown_url_for_baseis not a usable SSRF guard (host-string compare only, ignores ports, incidental metadata-IP rejection). The real building block isdiscover.rs's address-pinned fetcher —resolve_public_url+site_client+ssrf_redirect_policy(https-only, all-resolved-IPs-global, pinned to pre-validated addresses, bounded redirects, size caps). The plan's Phase 0 extracts that into a sharedpub(crate)module.Covers
client_id==URL, redirect membership + existing path pin, JSON structure) → cache flow, hooking into/oauth/authorize.DEFAULT_ALLOWED_REDIRECTS).auth.rs/discover.rsline numbers.No code changes — scoping only.
🤖 Generated with Claude Code