lz/agent-nexus
lz/agent-nexus
1
0
Fork 0
Code Issues 39 Pull requests 4 Projects Releases Packages 2 Wiki Activity Actions

CIMD: allowlist and resolver #152

New issue
Closed
opened 2026-09-15 18:23:54 +02:00 by lz · 2 comments
lz commented 2026-09-15 18:23:54 +02:00
Owner
Copy link

⚠️ Shipped in #169 with this design INVERTED — do not implement the body below as written.
It specifies a resolver we write and says never to enable features.clientIdMetadataDocument. What shipped uses the library's own CIMD feature with both gating hooks bound to the allowlist, because the SSRF objection turned out to be removable by configuration and the library's guard is stronger than a hand-written one.
See the closing comment for what shipped, corrections to the technical claims below, and the consequences accepted. The body is kept as the historical design record.

Client ID Metadata Documents are how Claude registers itself. The MCP 2026-07-28 spec deprecated DCR in favour of CIMD — "DCR continues to work for backward compatibility, but will be removed in a future version of the MCP spec" — and Claude Code identifies itself with its own CIMD at https://claude.ai/oauth/claude-code-client-metadata.

Depends on #151.

This is the only security-relevant code MCP adds

oidc-provider does not implement CIMD, so this piece is ours. Keep it small enough to read in full.

The naive implementation is an SSRF primitive. CIMD makes the client_id a URL the authorization server fetches. Nexus runs network_mode: host with DOCKER_HOST=tcp://127.0.0.1:2375 — the socket proxy publishes there specifically so host-mode Nexus can reach it (AGENTS.md fact #17), with containers, exec, images and networks allowlisted. "Fetch this URL for me" next to that is a shape worth no convenience.

The design removes the primitive rather than mitigating it

  1. CIMD URLs live in an exact-match allowlist, empty by default. No wildcards.
  2. The fetch happens when the operator adds an entry, not during an OAuth flow. Nexus resolves the document once and registers a normal static client in the adapter, keyed by that URL as its client_id.
  3. An authorization request naming an un-allowlisted client_id finds no client and is refused. Fail-closed.

So at request time there is no fetch at all, and the fetch target is operator-controlled rather than client-controlled.

Verified as the right shape: Pocket ID does exactly this, and a fresh instance reports client_id_metadata_document_supported: false until the allowlist is non-empty. Ours must serialize the same way.

Fetch constraints

For the one operator-initiated resolution — belt-and-braces given the allowlist, and cheap:

  • HTTPS only
  • 5s timeout
  • 64 KiB body cap
  • no redirects followed to a different host
  • refuse any URL resolving to a private, loopback or link-local address

Metadata

The authorization server metadata must advertise both client_id_metadata_document_supported: true and "none" in token_endpoint_auth_methods_supported. Claude selects CIMD only when both are present and silently falls back to DCR otherwise, so a test asserts both against the live document.

Watch for

Loopback redirect matching. Claude Code binds an ephemeral port, and Anthropic requires the authorization server accept http://localhost/callback and http://127.0.0.1/callback with the port ignored — RFC 8252 §7.3 mandates this for IP literals only, so the localhost half is an extra requirement. Pocket ID's loopbackURLWithWildcardPort is a good reference implementation: it strips the port and re-matches for localhost plus any IsLoopback() IP, with IPv6 bracketing.

Never register a client whose redirect URI pattern is *.

Done when

Claude Code completes an authorization flow against a Nexus instance with only its CIMD URL allowlisted; an un-allowlisted URL is refused; the allowlist being empty makes the advertised flag false; and the SSRF constraints are each a test proven able to fail.

> **⚠️ Shipped in #169 with this design INVERTED — do not implement the body below as written.** > It specifies a resolver we write and says never to enable `features.clientIdMetadataDocument`. What shipped uses the library's own CIMD feature with both gating hooks bound to the allowlist, because the SSRF objection turned out to be removable by configuration and the library's guard is stronger than a hand-written one. > **See [the closing comment](#issuecomment-3539)** for what shipped, corrections to the technical claims below, and the consequences accepted. The body is kept as the historical design record. --- Client ID Metadata Documents are how Claude registers itself. The MCP 2026-07-28 spec **deprecated DCR in favour of CIMD** — *"DCR continues to work for backward compatibility, but will be removed in a future version of the MCP spec"* — and Claude Code identifies itself with its own CIMD at `https://claude.ai/oauth/claude-code-client-metadata`. Depends on #151. ## This is the only security-relevant code MCP adds `oidc-provider` does not implement CIMD, so this piece is ours. Keep it small enough to read in full. **The naive implementation is an SSRF primitive.** CIMD makes the `client_id` a URL the authorization server fetches. Nexus runs `network_mode: host` with `DOCKER_HOST=tcp://127.0.0.1:2375` — the socket proxy publishes there specifically so host-mode Nexus can reach it (AGENTS.md fact #17), with `containers`, `exec`, `images` and `networks` allowlisted. "Fetch this URL for me" next to that is a shape worth no convenience. ## The design removes the primitive rather than mitigating it 1. CIMD URLs live in an **exact-match allowlist, empty by default**. No wildcards. 2. The fetch happens **when the operator adds an entry**, not during an OAuth flow. Nexus resolves the document once and registers a normal static client in the adapter, keyed by that URL as its `client_id`. 3. An authorization request naming an un-allowlisted `client_id` finds no client and is refused. Fail-closed. So at request time there is no fetch at all, and the fetch target is operator-controlled rather than client-controlled. Verified as the right shape: Pocket ID does exactly this, and a fresh instance reports `client_id_metadata_document_supported: false` until the allowlist is non-empty. Ours must serialize the same way. ## Fetch constraints For the one operator-initiated resolution — belt-and-braces given the allowlist, and cheap: - HTTPS only - 5s timeout - 64 KiB body cap - no redirects followed to a different host - refuse any URL resolving to a private, loopback or link-local address ## Metadata The authorization server metadata must advertise **both** `client_id_metadata_document_supported: true` and `"none"` in `token_endpoint_auth_methods_supported`. Claude selects CIMD only when both are present and silently falls back to DCR otherwise, so a test asserts both against the live document. ## Watch for Loopback redirect matching. Claude Code binds an ephemeral port, and Anthropic requires the authorization server accept `http://localhost/callback` and `http://127.0.0.1/callback` **with the port ignored** — RFC 8252 §7.3 mandates this for IP literals only, so the `localhost` half is an extra requirement. Pocket ID's `loopbackURLWithWildcardPort` is a good reference implementation: it strips the port and re-matches for `localhost` plus any `IsLoopback()` IP, with IPv6 bracketing. Never register a client whose redirect URI pattern is `*`. ## Done when Claude Code completes an authorization flow against a Nexus instance with only its CIMD URL allowlisted; an un-allowlisted URL is refused; the allowlist being empty makes the advertised flag `false`; and the SSRF constraints are each a test proven able to fail.
lz added this to the MCP support (#140) milestone 2026-09-15 18:23:54 +02:00
lz commented 2026-09-15 23:13:19 +02:00
Author
Owner
Copy link

Two findings from building #151, both verified against the installed oidc-provider@9.12.2 source. The first confirms this issue's design is the right one; the second is a trap that would otherwise be found the hard way.

The design here works, and it works because the library feature stays off

Client.find(id) (lib/models/client.js:572-615) resolves in three steps, in order:

  1. staticClients.get(id) — from Configuration.clients
  2. await this.adapter.find(id) — our Kysely adapter
  3. only if both miss, and features.clientIdMetadataDocument.enabled is true, and the id is a URL → resolveClientByMetadataDocument(...)

This issue's plan — resolve the document when the operator adds an allowlist entry, then register a normal client row keyed by that URL as its client_id — lands that row at step 2. Step 3 never runs. So no library feature needs enabling, and the request-time fetch never exists.

Do not enable features.clientIdMetadataDocument

It is off by default (lib/helpers/defaults.js:1498) and it must stay off. Turning it on reintroduces exactly the primitive this issue's design removes: a fetch, at request time, of a URL supplied by the client. Worse, both of its gating hooks default to unconditionally permissive:

async allowFetch(ctx, clientId) { return true; },   // defaults.js:1508
async allowClient(ctx, client)  { return true; },   // defaults.js:1519

So enabling it without binding both hooks would let Nexus fetch client metadata from any https URL. Nexus runs network_mode: host with DOCKER_HOST=tcp://127.0.0.1:2375 (AGENTS.md fact #17) — that is a server-side request forgery primitive sitting next to the Docker API, which is the shape this issue was written to avoid.

The operator-initiated, one-time fetch described above is strictly safer than anything achievable by enabling the feature and hardening its hooks, because the fetch target is operator-controlled rather than client-controlled.

What #151 ships

discovery.client_id_metadata_document_supported is set from cimdAllowlist.length > 0, so it serializes false on an empty allowlist — matching the Pocket ID behaviour this issue cites. Nothing populates the allowlist yet, so Claude Code cannot complete a flow against main until this issue lands. That is the intended state for #151 and is called out in its PR.

config.ts carries a comment recording all of the above, so the next person to read the discovery flag does not conclude the feature was left off by mistake.

One correction to the issue body's "Metadata" section while I am here: it says a test asserts both client_id_metadata_document_supported: true and "none" in token_endpoint_auth_methods_supported against the live document. #151 asserts the "none" half and asserts the CIMD flag is false (empty allowlist). The true case belongs here, since this issue is what makes it reachable.

Two findings from building #151, both verified against the installed `oidc-provider@9.12.2` source. The first confirms this issue's design is the right one; the second is a trap that would otherwise be found the hard way. ## The design here works, and it works *because* the library feature stays off `Client.find(id)` (`lib/models/client.js:572-615`) resolves in three steps, in order: 1. `staticClients.get(id)` — from `Configuration.clients` 2. `await this.adapter.find(id)` — **our Kysely adapter** 3. only if both miss, **and** `features.clientIdMetadataDocument.enabled` is true, **and** the id is a URL → `resolveClientByMetadataDocument(...)` This issue's plan — resolve the document when the operator adds an allowlist entry, then register a normal client row keyed by that URL as its `client_id` — lands that row at **step 2**. Step 3 never runs. So no library feature needs enabling, and the request-time fetch never exists. ## Do not enable `features.clientIdMetadataDocument` It is off by default (`lib/helpers/defaults.js:1498`) and it must stay off. Turning it on reintroduces exactly the primitive this issue's design removes: a fetch, at request time, of a URL supplied by the client. Worse, both of its gating hooks default to unconditionally permissive: ```js async allowFetch(ctx, clientId) { return true; }, // defaults.js:1508 async allowClient(ctx, client) { return true; }, // defaults.js:1519 ``` So enabling it without binding both hooks would let Nexus fetch client metadata from **any** https URL. Nexus runs `network_mode: host` with `DOCKER_HOST=tcp://127.0.0.1:2375` (AGENTS.md fact #17) — that is a server-side request forgery primitive sitting next to the Docker API, which is the shape this issue was written to avoid. The operator-initiated, one-time fetch described above is strictly safer than anything achievable by enabling the feature and hardening its hooks, because the fetch target is operator-controlled rather than client-controlled. ## What #151 ships `discovery.client_id_metadata_document_supported` is set from `cimdAllowlist.length > 0`, so it serializes `false` on an empty allowlist — matching the Pocket ID behaviour this issue cites. Nothing populates the allowlist yet, so Claude Code cannot complete a flow against `main` until this issue lands. That is the intended state for #151 and is called out in its PR. `config.ts` carries a comment recording all of the above, so the next person to read the discovery flag does not conclude the feature was left off by mistake. --- One correction to the issue body's "Metadata" section while I am here: it says a test asserts both `client_id_metadata_document_supported: true` and `"none"` in `token_endpoint_auth_methods_supported` against the live document. #151 asserts the `"none"` half and asserts the CIMD flag is `false` (empty allowlist). The `true` case belongs here, since this issue is what makes it reachable.
lz closed this issue 2026-09-16 23:47:30 +02:00
lz commented 2026-09-16 23:56:43 +02:00
Author
Owner
Copy link

Shipped in #169, with the design inverted

The body above specifies a resolver we write: fetch at allowlist time, register a static client row, never enable features.clientIdMetadataDocument. What shipped is the opposite — the library's own CIMD feature, with both gating hooks bound to the allowlist. Leaving this note so nobody implements the body as written.

Why it changed. The issue rejects the library feature because allowFetch/allowClient default permissive next to a host-networked Docker API. That objection is removable by configuration: bound to the allowlist, the fetch target is operator-controlled, which is the same end "resolve at add time" was reaching for. And the library's own guard is stronger than a hand-written one — fetch_request.js checks socket.remoteAddress on undici's connect event, i.e. the peer actually connected to, so DNS rebinding has no window to race. An early draft of our plan hand-wrote that guard and would have passed 2002:7f00:1:: (a 6to4 address embedding 127.0.0.1) as public.

What genuinely changed is when the fetch happens. The body says "at request time there is no fetch at all"; there is now one, on a cold cache, against a URL the operator allowlisted. Accepted knowingly — if claude.ai is unreachable, Claude is not connecting anyway.

Corrections to the body's technical claims

"Pocket ID's loopbackURLWithWildcardPort is a good reference implementation" — not needed. oidc-provider already ships stripLoopbackPort, and its LOOPBACKS set is {localhost, 127.0.0.1, [::1]}, so it already covers the localhost half the body flags as an Anthropic requirement beyond RFC 8252 §7.3.

The real work was elsewhere, and the body doesn't mention it: #redirectAllowed returns false before stripping the port unless applicationType === 'native', and the default is web. Claude Code's live document declares no application_type and registers portless loopback redirects, so registered as-is the client validates fine and then refuses every callback it can make. clientDefaults.application_type = 'native' is the only lever — resolveClientByMetadataDocument builds the client verbatim and allowClient runs after construction, so neither can transform metadata.

"a test asserts both client_id_metadata_document_supported: true and none" — done, but note discovery.js sets that flag whenever the feature is enabled and the defaults() after it only fills absent keys. So the discovery entry in config.ts is consulted only in the off state, and enabled is gated on the allowlist being non-empty instead. A fresh instance therefore has no CIMD fetch path in existence at all, not merely an advertised false.

"an un-allowlisted client_id is refused" — yes, but via both hooks. allowFetch runs only on a cache miss; allowClient is what refuses a client whose entry was removed while its document still sat in the LRU.

Consequences accepted

  • Native clients re-prompt for consent on every authorization (native_client_prompt) rather than silently reusing a Grant.
  • ttl.RefreshToken is now ours. application_type: 'native' disables the library's refresh-chain cap, which is gated on applicationType === 'web' — measured: web inherits the rotated token's remaining life, native gets a fresh 14 days every rotation, i.e. an unbounded chain. The override is the library's logic minus that clause, with ttl-parity.test.ts pinning the vendor branch.

Not done here

The Settings → MCP panel is #153. This shipped the service and GET/POST/DELETE /api/settings/mcp/clients only. Two things #153 inherits: added_at is the only stored metadata (the client name lives in the document, deliberately not copied), and removal stops future authorizations and refreshes immediately but leaves already-issued access tokens valid for up to an hour — true immediate revocation needs a client_id column on oauth_payloads.

The "done when" is not fully met: Claude Code cannot complete a flow against Nexus yet, because /api/mcp does not exist. That is #154. What was verified, against the built server with Claude Code's real document: a full PKCE authorization accepted on a loopback port the document does not register, the discovery flag flipping without a restart, and the consent screen rendering with the client name and loopback warning.

## Shipped in #169, with the design inverted The body above specifies a resolver we write: fetch at allowlist time, register a static client row, **never** enable `features.clientIdMetadataDocument`. What shipped is the opposite — the library's own CIMD feature, with both gating hooks bound to the allowlist. Leaving this note so nobody implements the body as written. **Why it changed.** The issue rejects the library feature because `allowFetch`/`allowClient` default permissive next to a host-networked Docker API. That objection is removable by configuration: bound to the allowlist, the fetch target is operator-controlled, which is the same end "resolve at add time" was reaching for. And the library's own guard is stronger than a hand-written one — `fetch_request.js` checks `socket.remoteAddress` on undici's `connect` event, i.e. the peer actually connected to, so DNS rebinding has no window to race. An early draft of our plan hand-wrote that guard and would have passed `2002:7f00:1::` (a 6to4 address embedding 127.0.0.1) as public. What genuinely changed is *when* the fetch happens. The body says "at request time there is no fetch at all"; there is now one, on a cold cache, against a URL the operator allowlisted. Accepted knowingly — if `claude.ai` is unreachable, Claude is not connecting anyway. ### Corrections to the body's technical claims **"Pocket ID's `loopbackURLWithWildcardPort` is a good reference implementation"** — not needed. `oidc-provider` already ships `stripLoopbackPort`, and its `LOOPBACKS` set is `{localhost, 127.0.0.1, [::1]}`, so it already covers the `localhost` half the body flags as an Anthropic requirement beyond RFC 8252 §7.3. The real work was elsewhere, and the body doesn't mention it: **`#redirectAllowed` returns `false` before stripping the port unless `applicationType === 'native'`**, and the default is `web`. Claude Code's live document declares no `application_type` and registers portless loopback redirects, so registered as-is the client validates fine and then refuses every callback it can make. `clientDefaults.application_type = 'native'` is the only lever — `resolveClientByMetadataDocument` builds the client verbatim and `allowClient` runs after construction, so neither can transform metadata. **"a test asserts both `client_id_metadata_document_supported: true` and `none`"** — done, but note `discovery.js` *sets* that flag whenever the feature is enabled and the `defaults()` after it only fills absent keys. So the `discovery` entry in `config.ts` is consulted only in the off state, and `enabled` is gated on the allowlist being non-empty instead. A fresh instance therefore has no CIMD fetch path in existence at all, not merely an advertised `false`. **"an un-allowlisted `client_id` is refused"** — yes, but via **both** hooks. `allowFetch` runs only on a cache miss; `allowClient` is what refuses a client whose entry was removed while its document still sat in the LRU. ### Consequences accepted - **Native clients re-prompt for consent on every authorization** (`native_client_prompt`) rather than silently reusing a Grant. - **`ttl.RefreshToken` is now ours.** `application_type: 'native'` disables the library's refresh-chain cap, which is gated on `applicationType === 'web'` — measured: web inherits the rotated token's remaining life, native gets a fresh 14 days every rotation, i.e. an unbounded chain. The override is the library's logic minus that clause, with `ttl-parity.test.ts` pinning the vendor branch. ### Not done here The **Settings → MCP panel is #153**. This shipped the service and `GET`/`POST`/`DELETE /api/settings/mcp/clients` only. Two things #153 inherits: `added_at` is the only stored metadata (the client name lives in the document, deliberately not copied), and removal stops future authorizations and refreshes immediately but leaves already-issued access tokens valid for up to an hour — true immediate revocation needs a `client_id` column on `oauth_payloads`. The **"done when" is not fully met**: Claude Code cannot complete a flow against Nexus yet, because `/api/mcp` does not exist. That is #154. What *was* verified, against the built server with Claude Code's real document: a full PKCE authorization accepted on a loopback port the document does not register, the discovery flag flipping without a restart, and the consent screen rendering with the client name and loopback warning.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
feat/mcp-resource-server
feat/mcp-server
feat/cache-warmth
fix/agent-status
feat/codegraph
feat/api-proxy
No results found.
Labels
Clear labels   
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lz/agent-nexus#152
No description provided.