Make the CIMD switch a McpConfig field, on by default; OAUTH_CIMD_ENABLED becomes a kill switch - #203
Merged
Merged
Conversation
…ABLED to it The library read OAUTH_CIMD_ENABLED itself, once per process through a OnceLock, which gave an embedding host no way to decide in code and left the setting out of the server's configuration. It is now McpConfig::cimd_enabled, handed to each AuthStore at construction. The imcp2 binary reads the variable (same on-values) and passes it to every instance, so the deploy path is unchanged; the parser and its test move there. Docs say which switch applies where. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
The move of the parser's test cut the function one brace short, so the module closed early and the remaining tests landed at the top level; the commit went out on a chain that did not gate on the test run. Redone from the passing tree, and checked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The configuration is consistently propagated and tested; only minor example-comment corrections remain.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Moves CIMD enablement into McpConfig while preserving environment-based configuration for the standalone binary.
Changes:
- Adds and propagates
McpConfig::cimd_enabled. - Moves environment parsing and tests into the binary.
- Updates examples, tests, and deployment documentation.
| File | Description |
|---|---|
src/lib.rs |
Adds the public configuration field. |
src/auth.rs |
Uses injected CIMD configuration. |
src/main.rs |
Maps the environment variable into configuration. |
src/e2e_handshake.rs |
Updates test configuration. |
src/metrics.rs |
Updates metrics test configuration. |
tests/routers.rs |
Updates router test configuration. |
README.md |
Documents library and binary switches. |
docs/anthropic-directory-submission.md |
Clarifies Anthropic enablement. |
docs/openai-directory-submission.md |
Clarifies OpenAI enablement. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The inline comment on the example's cimd_enabled: false read as if false advertised the mechanism. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
…switch Client ID Metadata Documents are the registration mode Claude and ChatGPT prefer, and staging has run them end to end, so the binary no longer waits for an opt-in: `cimd_enabled()` is true unless `$OAUTH_CIMD_ENABLED` is `0`/`false`/`no`/`off`, mirroring `OAUTH_REQUIRE_RESOURCE`. The variable is kept only as a roll-out kill switch and marked for removal (TODO in `main.rs`); the start-up log now notes the off state instead of the on one, and the parser test asserts the inverted table. `McpConfig::cimd_enabled` stays a required field for an embedding host; its doc and the crate/README examples now show `true`. The auth.rs section comment, the unit template, deploy.sh, the reusable workflow, the deploy README and the two directory-submission docs say the same, and the README's list of the binary's variables gains the switch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
The status dashboard's description of the AS-metadata check still said Client ID Metadata Documents are on only with OAUTH_CIMD_ENABLED=1; it now says on by default, off only where the variable is falsey, matching the binary. Found by Copilot's review. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj
MRmarioruci
approved these changes
Sep 24, 2026
aterga
added a commit
that referenced
this pull request
Oct 2, 2026
CIMD shipped in #191 (opt-in) and #203 (on by default, with OAUTH_CIMD_ENABLED as a kill switch), so the scoping plan now records what was built instead of proposing it: - the implemented gate, flow, fetcher, validation, caching, single-flight and in-flight bounds, configuration and rollback, by symbol rather than by line number; - answers to the plan's open questions, and where the build departed from the plan: a domain-and-subdomain gate instead of exact origins, no cache floor or ETag revalidation, the same-origin redirect rule, discovery's SSRF guard tightened rather than left unchanged, and the rate cap dropped on purpose; - branding moved to #103/#200 and keyed on the validated redirect. A client_id-domain key would have let any local program pose as a loopback CIMD client of a vetted vendor; - the DoS residuals as built, the tests that pin the behaviour, and an index of where it lives. Section numbers 3.4 and 3.5, which the code cites, keep their meaning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aterga
added a commit
that referenced
this pull request
Oct 5, 2026
… and #203) (#143) * docs: scope CIMD (Client ID Metadata Documents) to replace open DCR 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> * CIMD doc: origin-exact gating, strict reader, string match, mandatory 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> * CIMD doc: disable cross-origin redirects (discovery policy leaks the 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> * CIMD doc: never negative-cache per-request redirect failures (poisoning) 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> * docs: turn the CIMD scoping into a design record of what shipped CIMD shipped in #191 (opt-in) and #203 (on by default, with OAUTH_CIMD_ENABLED as a kill switch), so the scoping plan now records what was built instead of proposing it: - the implemented gate, flow, fetcher, validation, caching, single-flight and in-flight bounds, configuration and rollback, by symbol rather than by line number; - answers to the plan's open questions, and where the build departed from the plan: a domain-and-subdomain gate instead of exact origins, no cache floor or ETag revalidation, the same-origin redirect rule, discovery's SSRF guard tightened rather than left unchanged, and the rate cap dropped on purpose; - branding moved to #103/#200 and keyed on the validated redirect. A client_id-domain key would have let any local program pose as a loopback CIMD client of a vetted vendor; - the DoS residuals as built, the tests that pin the behaviour, and an index of where it lives. Section numbers 3.4 and 3.5, which the code cites, keep their meaning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * CIMD record: untrusted URLs are refused, eviction is by expiry, withdrawals wait for expiry Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * CIMD record: log sampling protects the default (info) level, not debug Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
The library read
OAUTH_CIMD_ENABLEDitself, once per process through aOnceLockinauth.rs. An embedding host, the shape production runs in, had no way to decide in code, and the setting was the one piece of behaviourMcpConfigdid not carry. It is now a field,McpConfig::cimd_enabled, handed to eachAuthStoreat construction likerequire_resource.Client ID Metadata Documents are also on by default now. They are the registration mode Claude and ChatGPT prefer, staging has run them end to end, and the trust policy limits fetches to the vetted vendor origins, so the roll-out guard that kept them off until each environment opted in has done its job. The
imcp2binary passestrueto every instance unlessOAUTH_CIMD_ENABLEDis a falsey value (0/false/no/off), mirroringOAUTH_REQUIRE_RESOURCE; the variable stays only as a kill switch for the roll-out and carries a TODO to remove it once CIMD has run in production for a while. Staging's Environment variable is1, so nothing changes there; it can be deleted whenever convenient.For an embedding host the switch is one line:
Related issues
Follows #191, which introduced the switch as an environment variable read by the library.
Changes
src/lib.rs—McpConfig::cimd_enabled: bool, documented; threaded intoAuthStore::new; the crate-level example showstrue.src/auth.rs—AuthStore::newtakescimd_enabled;cimd_enabled_by_envandcimd_enabled_byremoved; the CIMD section comment, the field's doc, the metadata comment and two test helpers reworded for the new source of truth and the new default. Tests build the store with CIMD off and switch it on per test as before.src/main.rs—cimd_enabled()istrueunlessOAUTH_CIMD_ENABLEDis falsey (cimd_enabled_byparses it, with a TODO to drop the variable); both instances get the value; the start-up log now notes the off state; the parser's test (cimd_opt_out_values) lives here.McpConfigliteral (e2e_handshake.rs,metrics.rs,tests/routers.rs, the lib tests) setscimd_enabled: false, as before.deploy/native/imcp2.service,deploy/native/deploy.sh,.github/workflows/deploy-native.yml,deploy/native/README.md— the same plumbing, described as the on-by-default kill switch it now is.README.md— the binary's variable list gainsOAUTH_CIMD_ENABLED; the example and the CIMD paragraph say on by default.docs/anthropic-directory-submission.md,docs/openai-directory-submission.md— likewise.monitoring/mcp-status/checks.js— the AS-metadata check's description says on by default; the detail line (CIMD=on|off, read from the metadata) is unchanged.History note: e17a25c, a tidy-up that moved the parser's test below the module's imports, cut the function one brace short and did not compile; 93431e8 restores the module from the passing tree. The branch head builds and passes.
Testing
On 751ebde (the Rust and deploy changes; ebe902a touches only the dashboard's description string):
cargo build --locked --workspace --all-targetscargo test --locked --workspace --all-targets— 297 tests, 0 failurescargo test --locked -p imcp2 --doc— the crate-level example compilescargo fmt --all -- --checkcargo clippy --locked -p imcp2 --all-targets— no warnings in the changed crate; the 6 remaining are the pre-existingimcp2-coreones.github/scripts/scan-internal-identifiers.sh origin/main...HEAD— clean (re-run on ebe902a)npm testinmonitoring/mcp-statuson ebe902a — 71 tests, 0 failuresReview rounds (Copilot): round 1, on 93431e8, found the examples' inline comment on
cimd_enabled: falsereading as iffalseadvertised the mechanism (fixed in 7655c0b). 751ebde then flipped the default on at the maintainer's request; round 3, on 751ebde, found the dashboard's description still describing the opt-in (fixed in ebe902a).Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01Xd7VT72Qt16qiAJynu9EKj