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

A CIMD allowlist, so Claude can identify itself #169

Merged
lz merged 43 commits from feat/cimd-allowlist into main 2026-09-16 23:47:29 +02:00
Conversation 0 Commits 43 Files changed 30 +3530 -135
lz commented 2026-09-16 21:13:16 +02:00
Owner
Copy link

Closes #152.

The operator allowlists a Client ID Metadata Document URL; Claude can then identify itself to the OAuth server #168 added. Verified end to end against the built server with Claude Code's real document — including a callback on a loopback port the document does not register.

The design differs from the issue, and the issue should be updated

#152 specifies a resolver we write: fetch at allowlist time, register a static client row, never enable features.clientIdMetadataDocument. This ships the opposite — the library's own CIMD feature, with both gating hooks bound to the allowlist. Two things found while planning changed the conclusion.

The library already implements every piece, more thoroughly. client_id_metadata_document.js does URL validation, redirect: 'manual', a body cap, client_id equality, shared-secret rejection and an LRU honouring Cache-Control. fetch_request.js carries a special-use address table for both families and checks socket.remoteAddress on undici's connect event — i.e. the peer actually connected to, leaving no TOCTOU window. An earlier draft of this plan hand-wrote that guard and would have passed 2002:7f00:1:: (a 6to4 address embedding 127.0.0.1) as public.

The issue's objection is removable by configuration. It rejects the feature because allowFetch/allowClient default permissive next to a host-networked Docker API (fact #17). Bound to the allowlist, the fetch target is operator-controlled — the same end the resolve-at-add-time design was reaching for.

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

Three library facts the feature rests on

Each was verified against the installed oidc-provider@9.12.2, and each fails silently if got wrong.

application_type: 'native' is required, and it is the whole ballgame. Claude Code's live document declares no application_type and registers portless loopback redirects (http://localhost/callback, http://127.0.0.1/callback) — the RFC 8252 §7.3 "any port" idiom. #redirectAllowed returns false before stripping the port unless the client is native, and the default is web. Registered as-is, the client validates fine and then refuses every callback Claude Code can make. clientDefaults is the only lever: resolveClientByMetadataDocument builds the client verbatim and allowClient runs after construction.

enabled is gated on the allowlist being non-empty. discovery.js sets client_id_metadata_document_supported when the feature is on, and the defaults() after it only fills absent keys — so the discovery entry is consulted only in the off state. Gating enabled gives the issue's required behaviour and means a fresh instance has no CIMD fetch path in existence.

Both hooks are bound. allowFetch runs only on a cache miss; allowClient is what refuses a client whose entry was removed while its document sat in the LRU.

Two consequences worth knowing

Native clients re-prompt for consent on every authorization (native_client_prompt) rather than silently reusing a Grant. Defensible for a control-plane credential; observed in the live run, not inferred.

ttl.RefreshToken is now ours. application_type: 'native' disables the library's refresh-chain cap, which is gated on applicationType === 'web' — measured: web 42s (inherits the rotated token's remaining life), native a fresh 14 days on every rotation, i.e. an unbounded chain. The override is the library's own logic minus that clause. ttl-parity.test.ts pins the vendor branch so we learn if upstream fixes it.

Defects this branch fixes in already-merged #168

The consent page never worked. _loadConsentPage resolved interactions with findByUid, but Interaction.uid is a getter over jti and absent from IN_PAYLOAD, so the adapter writes NULL and the lookup can never match. Every render returned 404 "This sign-in request has expired" — with 2008 tests green. The fixture populated a uid column the library never writes; one helper is why it survived review, a /simplify, and a staging pass. The replacement test drives a real authorization and parses the uid from the redirect, so no fixture chooses the payload.

A Provider constructor throw crashed the control plane. getProvider is called synchronously inside the createServer listener. Now returns null, which the caller already turns into a 503.

getProvider's memo was bundle-local. A module-level let, while SvelteKit and src/server.ts evaluate the module separately (fact #25). Once the consent page began calling it, that meant a second Provider with a cold LRU. Now behind a globalThis slot.

Also: an inert claims line whose comment claimed it pinned us against a library default change — it cannot, merge({}, defaults, input) — and a false AGENTS.md citation.

Failure handling

Both mutators refresh the cache after committing, so a refresh failure means the row is written and the provider has not picked it up. POST and DELETE both distinguish that from "the write never took" and return 207 with the URL named (matching artifacts/…/submit's existing use for durable-write-ok/propagation-failed). DELETE matters most: a stale cache there keeps a revoked client authenticating while the UI shows it gone.

Consent-page client resolution catches OIDCProviderError and separates "not allowlisted / not resolvable" (400) from "could not reach the document" (503, retryable). provider.Client.find throws on every CIMD failure path rather than returning undefined, so the previous if (!client) 400 was unreachable for exactly these clients and every transient blip rendered as a 500.

Construction failures are retried but backed off 5s per memo key: the failure is persistent, the paths are unauthenticated, and the library emits four console.info lines before throwing on an ack mismatch — so suppressing only our own line silenced the useful one and left four times the volume.

Verification

Live, against the built server: flag false→true without a restart (the server.ts wiring line no unit test can reach), Claude Code's real document fetched from claude.ai, PKCE flow accepted on port 54321 which the document does not register, token carrying nexus:read nexus:write plus a refresh token, consent page rendering 200 with the client name and loopback warning, un-allowlisted client refused (with an allowlisted positive control in the same run), removal restoring false. Zero server errors.

182 test files, 2061 tests, typecheck 5102/0, lint clean.

cimd-ssrf-gate.test.ts asserts zero fetch calls when a non-allowlisted client is resolved — the ordering is the security property, and a refusal after the socket opens would still throw and still pass a naive test.

Known and deliberate

  • Claude still cannot connect after this merges. /api/mcp is #154.
  • Consent renders are unreachable while an instance is pre-ready (the phase gate bounces them). That is why the findByUid bug survived two staging passes; worth a look but out of scope.
  • b616c3e contains two agents' work under one message — git add writes to a shared index and a concurrent commit swept up staged files. Both diffs are correct; the message covers half. Not rewritten, since untangling a shared branch with live writers risks more than it fixes.
Closes #152. The operator allowlists a Client ID Metadata Document URL; Claude can then identify itself to the OAuth server #168 added. Verified end to end against the built server with Claude Code's real document — including a callback on a loopback port the document does not register. ## The design differs from the issue, and the issue should be updated #152 specifies a resolver we write: fetch at allowlist time, register a static client row, never enable `features.clientIdMetadataDocument`. This ships the opposite — the library's own CIMD feature, with both gating hooks bound to the allowlist. Two things found while planning changed the conclusion. **The library already implements every piece, more thoroughly.** `client_id_metadata_document.js` does URL validation, `redirect: 'manual'`, a body cap, `client_id` equality, shared-secret rejection and an LRU honouring `Cache-Control`. `fetch_request.js` carries a special-use address table for both families and checks `socket.remoteAddress` on undici's `connect` event — i.e. the peer actually connected to, leaving no TOCTOU window. An earlier draft of this plan hand-wrote that guard and would have passed `2002:7f00:1::` (a 6to4 address embedding 127.0.0.1) as public. **The issue's objection is removable by configuration.** It rejects the feature because `allowFetch`/`allowClient` default permissive next to a host-networked Docker API (fact #17). Bound to the allowlist, the fetch target is operator-controlled — the same end the resolve-at-add-time design was reaching for. What genuinely changes is *when* the fetch happens. The issue says "at request time there is no fetch at all"; here there is one, on a cold cache, against a URL the operator allowlisted. Accepted knowingly: if `claude.ai` is unreachable, Claude is not connecting anyway. ## Three library facts the feature rests on Each was verified against the installed `oidc-provider@9.12.2`, and each fails silently if got wrong. **`application_type: 'native'` is required, and it is the whole ballgame.** Claude Code's live document declares no `application_type` and registers **portless** loopback redirects (`http://localhost/callback`, `http://127.0.0.1/callback`) — the RFC 8252 §7.3 "any port" idiom. `#redirectAllowed` returns `false` *before* stripping the port unless the client is native, and the default is `web`. Registered as-is, the client validates fine and then refuses every callback Claude Code can make. `clientDefaults` is the only lever: `resolveClientByMetadataDocument` builds the client verbatim and `allowClient` runs after construction. **`enabled` is gated on the allowlist being non-empty.** `discovery.js` *sets* `client_id_metadata_document_supported` when the feature is on, and the `defaults()` after it only fills absent keys — so the `discovery` entry is consulted only in the off state. Gating `enabled` gives the issue's required behaviour and means a fresh instance has no CIMD fetch path in existence. **Both hooks are bound.** `allowFetch` runs only on a cache miss; `allowClient` is what refuses a client whose entry was removed while its document sat in the LRU. ## Two consequences worth knowing **Native clients re-prompt for consent on every authorization** (`native_client_prompt`) rather than silently reusing a Grant. Defensible for a control-plane credential; observed in the live run, not inferred. **`ttl.RefreshToken` is now ours.** `application_type: 'native'` disables the library's refresh-chain cap, which is gated on `applicationType === 'web'` — measured: web 42s (inherits the rotated token's remaining life), native a fresh 14 days on every rotation, i.e. an unbounded chain. The override is the library's own logic minus that clause. `ttl-parity.test.ts` pins the vendor branch so we learn if upstream fixes it. ## Defects this branch fixes in already-merged #168 **The consent page never worked.** `_loadConsentPage` resolved interactions with `findByUid`, but `Interaction.uid` is a getter over `jti` and absent from `IN_PAYLOAD`, so the adapter writes NULL and the lookup can never match. Every render returned 404 "This sign-in request has expired" — with 2008 tests green. The fixture populated a `uid` column the library never writes; one helper is why it survived review, a `/simplify`, and a staging pass. The replacement test drives a real authorization and parses the uid from the redirect, so no fixture chooses the payload. **A `Provider` constructor throw crashed the control plane.** `getProvider` is called synchronously inside the `createServer` listener. Now returns `null`, which the caller already turns into a 503. **`getProvider`'s memo was bundle-local.** A module-level `let`, while SvelteKit and `src/server.ts` evaluate the module separately (fact #25). Once the consent page began calling it, that meant a second Provider with a cold LRU. Now behind a `globalThis` slot. Also: an inert `claims` line whose comment claimed it pinned us against a library default change — it cannot, `merge({}, defaults, input)` — and a false `AGENTS.md` citation. ## Failure handling Both mutators refresh the cache **after** committing, so a refresh failure means the row is written and the provider has not picked it up. `POST` and `DELETE` both distinguish that from "the write never took" and return **207** with the URL named (matching `artifacts/…/submit`'s existing use for durable-write-ok/propagation-failed). `DELETE` matters most: a stale cache there keeps a **revoked client authenticating** while the UI shows it gone. Consent-page client resolution catches `OIDCProviderError` and separates "not allowlisted / not resolvable" (400) from "could not reach the document" (503, retryable). `provider.Client.find` throws on every CIMD failure path rather than returning `undefined`, so the previous `if (!client) 400` was unreachable for exactly these clients and every transient blip rendered as a 500. Construction failures are retried but backed off 5s per memo key: the failure is persistent, the paths are unauthenticated, and the library emits four `console.info` lines *before* throwing on an ack mismatch — so suppressing only our own line silenced the useful one and left four times the volume. ## Verification Live, against the built server: flag `false`→`true` **without a restart** (the `server.ts` wiring line no unit test can reach), Claude Code's real document fetched from `claude.ai`, PKCE flow accepted on **port 54321** which the document does not register, token carrying `nexus:read nexus:write` plus a refresh token, consent page rendering 200 with the client name and loopback warning, un-allowlisted client refused (with an allowlisted positive control in the same run), removal restoring `false`. Zero server errors. 182 test files, 2061 tests, typecheck 5102/0, lint clean. `cimd-ssrf-gate.test.ts` asserts **zero `fetch` calls** when a non-allowlisted client is resolved — the ordering is the security property, and a refusal *after* the socket opens would still throw and still pass a naive test. ## Known and deliberate - **Claude still cannot connect after this merges.** `/api/mcp` is #154. - Consent renders are unreachable while an instance is pre-`ready` (the phase gate bounces them). That is why the `findByUid` bug survived two staging passes; worth a look but out of scope. - `b616c3e` contains two agents' work under one message — `git add` writes to a shared index and a concurrent commit swept up staged files. Both diffs are correct; the message covers half. Not rewritten, since untangling a shared branch with live writers risks more than it fixes.
lz added this to the MCP support (#140) milestone 2026-09-16 21:13:16 +02:00
lz added 43 commits 2026-09-16 21:13:17 +02:00 
url was PRIMARY KEY without NOT NULL; SQLite doesn't imply the latter for a
non-INTEGER PK and doesn't dedupe NULLs, so unlimited NULL-url rows slipped
past the allowlist's exact-match invariant. Adds the null-url rejection test
and switches the missing-added_at cast to the exported CimdClientsTable
instead of hand-declaring the row shape.
The Task 2 examples were written to disk as literal 0x01, NUL and DEL bytes
instead of escape text. The NUL made GNU grep treat the whole plan as binary,
so every plain grep against it returned nothing and silently 'confirmed'
whatever was asked of it.
Mirrors isValidClientIdUrl's authority.includes('@') exactly. url.username/
url.password read as empty strings for a bare https://@a.example/m (empty
userinfo on both sides of the @), so that shape fell through to the
canonical-form backstop instead of the credentials check.

Also corrects a comment that overstated how the empty-authority case is
caught: only the triple-slash shape goes through the canonical-form check;
https://, https://?x and https://#x all throw at new URL() itself.
Adds url-parity.test.ts asserting the one-directional property that matters:
everything the vendor rejects, assertAllowlistUrl also rejects. Deliberately
not equality -- we're stricter (the canonical-form check the vendor lacks).

Also: cites the real lib/-avoidance precedent (oauth/consent.ts) instead of a
nonexistent AGENTS.md rule, drops two references to an ephemeral local
session, and corrects the dot-segment/control-character comments to say what
was actually measured -- both checks are message-UX in this file (the
canonical-form check refuses those inputs regardless), unlike the credentials
check, which is the one check here that alone decides accept/reject and
unlike upstream where dot-segments are the vendor's only defense.
oidc-provider resolves the Client Identifier URL itself; both of its
gating hooks are bound to the operator's allowlist. allowFetch alone is
insufficient — a cached document skips it and reaches allowClient only,
which is what refuses a de-allowlisted client still held in the LRU.

clientDefaults also gains token_endpoint_auth_method 'none' alongside
application_type 'native'. Claude Code's document declares neither, and
without the former the library default 'client_secret_basic' makes
client_secret mandatory, so the client fails to construct outright.

Documents the consequence for refresh tokens: the default RefreshTokenTTL
caps a rotating chain only for a 'web' client, so native clients now mint
a fresh 14 days on every rotation.
application_type 'native' skips the library's own RefreshTokenTTL cap,
which gates the inherit-remainingTTL branch on applicationType 'web'
(defaults.js:382-395). That silently falsified the stated reason #168
wrote no ttl block, leaving a chain that extends itself by the elapsed
time on every rotation. ttl.RefreshToken restores the property with the
library's condition minus that clause.

flow.test.ts asserts the property rather than the constant: it rotates a
native client across 600s of injected clock and requires the replacement
not to outlive the token it replaced. Without the ttl block it fails by
exactly that 600s.

Also corrects the token_endpoint_auth_method rationale. Claude Code's
document does declare it; the default earns its place because this server
accepts only 'none', so a document omitting the field constructs instead
of dying on a client_secret requirement it could never satisfy.
getProvider runs synchronously inside src/server.ts's createServer request
listener, so a Provider constructor throw is an uncaught exception that
takes the control plane down rather than one request — the crash class the
consent handler is already guarded against there. It now catches, logs and
returns null, which the caller already renders as a 503 via
serveUnavailable. The memo is written only after a success, so a failure
never poisons it; provider.test.ts pins both that and the case where an
unrelated failure must leave an existing instance intact.

With that guard, ack: 'draft-02' is safe to pin. The CIMD draft defines
what allowFetch/allowClient gate, so a version bump should fail loudly
rather than silently change those semantics: mismatched, it throws at
construction and degrades OAuth to a logged 503. The ack must be removed
when the feature goes stable, which configuration.js also throws on.
config.test.ts still carried the claim 7dc981d removed from config.ts:
Claude Code's document does declare token_endpoint_auth_method 'none', so
that default is what lets a document OMITTING the field construct, not
what rescues Claude Code. Only the application_type half was ever true.

getProvider has had two null returns since 81293f3, but its JSDoc named
one and server.ts rendered every null as 'public_url is not configured' —
telling the operator the wrong cause for a construction failure whose real
reason only reaches the log. Both now distinguish the two.

Every cimdAllowlist in the suite was [], so the feature was never enabled
at construction and ack: 'draft-02' was never evaluated: a wrong ack, or a
library bump past draft-02, passed the whole suite. provider.test.ts now
builds a real Provider with the feature on, covering the ack and the
construction guard in one assertion.
Every cause of a construction failure is persistent — a bad keystore, a
stale ack after a dependency bump, a config typo — so the catch added in
81293f3 was taken on every request, at a rate nobody authenticates:
routeOwner hands this path /oauth/* and both root well-known documents.
A ~1.4 KB stack per request is a disk-fill vector that also buries the one
line worth reading, the same defect e373697 fixed for the unlock refusal.

Construction is still retried every request, so the operator recovers
without a restart; only the reporting is de-duplicated. The first failure
logs at error, repeats of the same (issuer, message) drop to debug, and a
successful construction clears the memory so a recurrence is loud again.
_resetProvider clears it too, or one test inherits another's silence.

The ack test asserted only that construction succeeded, which stays true
if enabled regresses to a constant false — logDraftNotice reads ack only
while the feature is on, so it would have kept passing while exercising
nothing. Verified: with enabled:false and the assertion removed, all ten
cases pass. It now asserts that precondition.
metadata.test.ts covered only the OFF state. It now asserts the served
document says true once a URL is allowlisted. It does NOT prove
features.enabled reaches the library, and says so: discovery.js and our own
discovery entry both compute that flag from the same allowlist length, so
no input distinguishes them — measured, the case still passes with enabled
forced false. provider.test.ts's ack case is what carries that proof, since
logDraftNotice reads ack only while the feature is enabled.

ttl-parity.test.ts reads the vendor's own default ttl table through the
deep-import shim url-parity.test.ts established, pinning that native is
still uncapped upstream and that the constant config.ts restates is still
theirs. Labelled a maintenance signal, not a guard: drift there mostly
fails safe and flow.test.ts covers the unsafe direction.

That flow assertion is now equality. Verified empirically before tightening
rather than by reasoning: returning 1 in place of remainingTTL survives
toBeLessThanOrEqual and fails toBe.

Also: point the registration flag at the application_type blast radius a
DCR client inherits, soften the ack maintenance note to a convention rather
than a guarantee, and drop the inert claims line — measured byte-identical
with and without it, and findAccount is what actually limits claims to sub.
The page looked the client up with `adapterFor(Client).find()`, but a CIMD
client is never written to the adapter: resolveClientByMetadataDocument
builds it from the fetched document into an in-memory LRU, and addClient()
skips the upsert a DCR client gets (`if (!cimd && store)`). So consent 400d
"Unknown client" for exactly the clients #152 exists to support.

provider.Client.find runs the library three-step resolution — staticClients,
adapter, then CIMD — covering that case and every existing one. It is taken
as a parameter, like `database` and `locals`, so a test can inject a stub.

Client.find returns a Client INSTANCE, whose constructor camelCases every
recognised metadata key, so the reads move to clientName/redirectUris;
`client_name` does not exist on it and would have shown the client_id as the
name for every client. The adapter-backed tests now resolve through a real
Provider so they keep proving that mapping.

A null provider (public_url unset, or a construction throw) 503s rather than
400ing: neither is the client fault, and it matches what src/server.ts
already serves for the same two nulls.
Task 6: cache the allowlist in singletons.ts (cachedCimdAllowlist/
refreshCimdAllowlist) and wire it into src/server.ts's getProvider call,
replacing the hard-coded empty array. addCimdClient/removeCimdClient
refresh the cache themselves after committing, since there is no other
reconciliation path for it (unlike repo-env's best-effort live push).
GET/POST/DELETE /api/settings/mcp/clients wrap the cimd service's
list/add/remove functions behind the masterKey guard every settings
route uses. DELETE takes ?url= rather than a body or path segment
(the identity is a URL, and proxies are known to strip DELETE
bodies), and mirrors /api/tokens/[id]'s 404-vs-204 split so the
operator can tell "removed" from "was never there" -- the reason
removeCimdClient returns a boolean instead of void.

The route is excluded from TOKEN_ROUTES: it mints an OAuth client
identity, a credential-granting surface scoped tokens must not reach.
scopes.test.ts pins the exclusion.
Extract POST's core into a DI-testable _addCimdClient(db, url), same
shape as the preferences route's _applyPreferenceChange, so a test can
drive the real addCimdClient/assertAllowlistUrl against a real
in-memory db instead of only ever asserting against a mock told what
to throw. Add that one real-db test for the invalid-URL path.

Also drop the GET call tacked onto the POST happy-path test: it read
from a canned mock, not from what the POST actually did, so it would
pass whether or not the POST worked.
addCimdClient validates the URL, then -- after committing the row --
calls refreshCimdAllowlist, which reads the db and can fail for
reasons that have nothing to do with the URL. The route's catch block
couldn't tell the two apart and reported every failure as a 400
"invalid URL", so an operator whose URL was accepted and written
could be told it was rejected.

cimd/url.ts now throws a typed InvalidAllowlistUrl for a bad URL
(message text and validation rules unchanged; url.test.ts and
url-parity.test.ts still pass as-is). The route checks `instanceof`
instead of assuming every throw came from validation: a real
InvalidAllowlistUrl still yields 400 with its message, anything else
yields 500 with a generic body and the real cause logged.
Every consent render 404d "This sign-in request has expired". The page
called adapterFor(Interaction).findByUid(uid), which queries WHERE uid = ?,
but an Interaction has no stored uid: `uid` is a getter over `this.jti`
(models/interaction.js) and is absent from IN_PAYLOAD, so the persisted
payload keys are iat, exp, returnTo, prompt, params, cid, kind, jti — and the
adapter `uid: payload.uid ?? null` writes NULL for every interaction row.
findByUid is for Session, the only model the library calls it on.

The uid in the consent URL is the jti, which is the adapter row id, so find()
is the correct call. Measured against a live authorization: findByUid NOT
FOUND, find FOUND; and end to end on a built server the same consent URL goes
404 -> 200.

The fixtures are the reason this survived review and every gate: the tests
seeded interactions with a `uid` key, populating a column the library never
populates, so findByUid worked in tests and only in tests. seedInteraction
now writes `jti` and no `uid`, matching the real payload, and one case drives
a real /oauth/auth round trip so oidc-provider writes the row itself. With
findByUid restored, 7 of 10 cases now fail; before this change, none did.
Pass loadAllowlist as a parameter to refreshCimdAllowlist instead of
singletons.ts importing it from cimd/service.ts. The cycle was safe only
while every binding crossing it stayed a hoisted function declaration and
nothing read a singletons binding at cimd/service.ts's module top level --
an invisible invariant, and esbuild silently binds undefined across it
while Rollup crashes the SvelteKit bundle at boot, with neither build
warning about the cycle.

Also: wrap the boot-time refresh in .catch() so a failed read degrades
CIMD to disabled rather than blocking startup, matching the setOauthJwks
pattern three lines above it; return cachedCimdAllowlist() as readonly;
correct two comments that overclaimed (removeCimdClient's unconditional
refresh self-heals drift, it isn't "no cheaper check"; the cache is also
read by the SvelteKit bundle's consent page load, not just server.ts); and
strengthen two tests whose final assertion was `[]`, indistinguishable
from a cache hard-coded to always return empty.
Follow-up to 99f9b6b (could not amend: 2d1ede4 landed on top).

The uid bug survived review because the fixtures were more convenient than
reality. Three more diverged the same way:

- The interaction seeded `session: {accountId}`, but a brand-new interaction
  carries the `login` prompt before `consent`, so at consent-render time
  there is no accountId. Dropped, and the rest of the measured key set
  (iat, exp, returnTo, prompt, cid) added.
- No fixture set a ttl, so every row stored expires_at NULL — the
  never-expires branch. The expiry path that produces the real "This sign-in
  request has expired" message had no coverage at all; it does now, and
  removing the adapter filter fails it.
- Client rows were three hand-written keys where addClient upserts
  client.metadata(), thirteen keys after schema defaults. They are now built
  through the provider own Client, so they cannot drift from the schema.

Also strengthens two cases that passed vacuously under the snake_case
regression: multi-uri now asserts loopback (only the resolved array feeds
it) and the nameless case asserts redirectHost (its interaction carries no
redirect_uri, so that value comes from the resolved array too). Restoring
the snake_case reads now fails 6 of 12 rather than 2 of 8.

Adds the missing guard for the empty redirect_uri branch, which survived
every test before, and corrects two comments: the null lookup has three
causes not two, and the bundles DO share one Provider since 3f0b399.
Test-side duplication:
- Add configDeps() to oauth/fixtures.ts; three files spelled the same
  buildConfiguration deps literal by hand (config.test.ts, ttl-parity.test.ts,
  provider.test.ts's ack case).
- page.server.test.ts: 12 tests each opened their own db behind an inline
  sqliteAvailable() guard. Hoisted to describe.skipIf + beforeEach/afterEach,
  the dominant pattern in this repo (36 files vs 9), which also destroys the
  db it was leaking. Inlined providerFor(), used once.
- flow.test.ts: registerClient/registerNativeClient differed only in
  application_type. Extracted the shared metadata so that difference - the
  whole point of the pair - is the only thing visible between them.
- provider.test.ts: folded the describe-local `base` into the file's
  `baseDeps` instead of respelling four fields.

Comments that had accreted a second copy of the same explanation:
- provider.ts stated the retry/suppression rationale in three places; each
  fact now lives once, beside the line that implements it.
- provider.test.ts restated memoKey's docstring verbatim; now points at it.
- config.ts's registration note restated the clientDefaults application_type
  note it already tells you to read; keeps only the DCR-specific warning
  (loopback port-stripping) that clientDefaults does not make.
- singletons.ts's cachedCimdAllowlist accessor restated its own field doc.
- The mcp/clients route's in-try comment restated its catch comment.

No behaviour change and no test removed: 2047 tests before and after.
provider.Client.find is not the DB read the adapter lookup was. For a
client_id that is a valid https URL it tail-calls
resolveClientByMetadataDocument, which does I/O and never returns undefined
— it throws on every failure: fetch error, the 2.5s timeout, a non-200, an
unreadable or mismatched document, a forbidden auth method, or either
allowlist hook refusing. Those are not SvelteKit HttpErrors, so their status
and description were discarded and the operator got a bare 500, while the
400 beside them was unreachable for exactly the CIMD clients #152 adds.

The lookup is now wrapped and the outcomes split by what the operator has to
do about them: a reachability failure 503s with a retry message, anything
else 400s pointing at the CIMD allowlist. The cause is logged, never shown —
it can carry the document body and the upstream URL. A throw that is not an
OIDCProviderError is rethrown, so a genuine bug still surfaces as a fault
rather than as advice to edit a setting.

Both arrive as InvalidClient with status 400, so the only discriminator is
error_description. That is a version coupling, so a test drives the REAL
library down both branches — a closed loopback port for the unreachable
side, a non-allowlisted URL for the refused side, neither touching the
network — and fails if a release rewords either. A reword meanwhile degrades
to the configuration message, which never invites a futile retry.

The undefined branch stays and is now tested: it is still reachable for
staticClients and adapter misses, i.e. a client deleted between authorize
and consent. A CIMD client_id never reaches it.
WHATWG resolves a percent-encoded dot segment during parsing -- the segment
is fully removed, not left encoded/hidden. Verified directly: pathname of
'/x/%2e%2e/m' is '/m', not '/x/%2e%2e/m'; %2e only survives when it isn't
itself a whole dot segment (e.g. '/a%2e/m' -> '/a%2e/m' unchanged). The
conclusion (check the raw string, not url.pathname) was already correct, for
the opposite reason: the traversal has already happened and the evidence is
gone by the time you have a pathname, not hidden under encoding a naive
./.. check would miss.

Also drops two stale exact-count claims ('the one place ...', 'the one deep
lib/ import ...') now that ttl-parity.test.ts does the same thing for a
different oidc-provider internal.

No behaviour or assertion changes -- comments only.
`s in SCOPE_LABELS` asks whether the prototype chain has the key, not
whether we have a label for the scope. Measured: `constructor` and
`toString` both pass, so either one reached `data.granted` as
{ scope, label: <function> } — a function where ConsentPageData promises a
string, which the Record<..., string> type hides rather than catches.

Object.hasOwn asks the question the filter means. The scope fixture gains
`constructor`, the case the old check let through; the existing
`not-a-real-scope` is a safe unknown that could never have caught it.

Low consequence — Svelte escapes output, and oidc-provider filters against
scopes_supported upstream, so it is unconfirmed such a scope reaches here at
all. Worth it for the code saying what it means.

Also merges two adjacent comment blocks above the client lookup that had
accumulated into overlapping explanations of the same call.
src/server.ts's request listener and the consent page load each built the
ProviderDeps object independently. Those fields are getProvider's memo
key, so the two literals drifting (one hardcoded, one live-read) silently
builds a second Provider from the same cache -- cold CIMD LRU on every
consent render at best, an "Unknown client" 400 for a just-approved client
at worst. providerDepsFromSingletons(jwks) is now the one place that
composes them; server.ts calls it. jwks stays a parameter since each
caller's null-check response differs.
config.ts: clientDefaults is not the ONLY lever that reaches a CIMD
client's metadata -- extraClientMetadata.validator does too, with no CIMD
gate (client_schema.js's Schema constructor runs processCustomMetadata
whenever extraClientMetadata.properties.length is non-zero, independent
of the cimd flag, and hands the validator the in-construction instance to
mutate). Dormant today since extraClientMetadata is unset, but the old
comment told a future reader there was nowhere else to normalise CIMD
metadata, which is false.

mcp/clients/+server.ts: "per the task brief" cited an ephemeral planning
doc with no evidence of its own. The technical claim -- proxies drop
DELETE bodies -- stands on its own; state it directly instead.
Second call site for providerDepsFromSingletons (fe462fd). The consent page
still built the ProviderDeps literal itself, and those fields are
getProvider memo key: the analyser mutated cimdAllowlist to [] there and
199/199 passed, because with CIMD disabled the page builds a SECOND Provider,
Client.find returns undefined for a cimd client_id, and consent renders
"Unknown client" moments after the authorization succeeded under the other
Provider. Dead in production, suite green -- the findByUid shape again.

The page now composes nothing, so that mutation has nowhere to live. Its
local null-jwks early return stays: unlike server.ts, which serves its own
503, this returns null and lets _loadConsentPage check the interaction first,
so a stale link still reads as expired rather than as OAuth being down.

Adds this file to singletons.test.ts CALL_SITES, which had been asserting
only half of what it claimed, and drops the note saying so.
- New cimd-ssrf-gate.test.ts: proves oidc-provider calls allowFetch (and
  refuses) BEFORE ever touching the network for a non-allowlisted client_id
  -- the ordering config.ts's SSRF-primitive comment depends on but nothing
  asserted. Also pins the CIMD experiment at draft-02 against the vendor's
  own EXPERIMENTS map, so a version bump is loud here instead of silent
  until a construction failure in production.
- flow.test.ts: the native-refresh-cap test took its client's applicationType
  on faith. Resolves it and asserts 'native' before rotating -- removing
  clientDefaults.application_type now fails at that assertion instead of
  silently re-measuring the library's own web cap under a false title,
  fixture name, and comment.
- provider.test.ts: two reuse/no-rebuild assertions could pass on
  null === null if construction ever failed. Added not-null guards.
fix(oauth): prefix the consent route's message constants so the build accepts them
All checks were successful
ci / nexus (pull_request) Successful in 9m2s
Details
ci / images (pull_request) Successful in 10m55s
Details
pr-image-cleanup / delete-pr-images (pull_request) Successful in 13s
Details
76c19eae8d 
SvelteKit validates route-module exports at build time and rejects any name
outside its allowlist unless it starts with an underscore. CLIENT_REJECTED_MESSAGE
and CLIENT_UNREACHABLE_MESSAGE broke `pnpm build` while typecheck, lint and all
2061 tests passed — none of those gates reads that rule. _loadConsentPage in the
same file was already following the convention.
lz merged commit 27a306a621 into main 2026-09-16 23:47:29 +02:00
lz deleted branch feat/cimd-allowlist 2026-09-16 23:47:30 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
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!169
No description provided.