Skip to content

feat(oauth): accept client ID metadata documents beside registration - #1152

Merged
mforce merged 15 commits into
mainfrom
feat/1148-oauth-client-metadata-documents
Oct 10, 2026
Merged

mforce merged 15 commits into
mainfrom
feat/1148-oauth-client-metadata-documents

Conversation

@mforce

@mforce mforce commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

An app can now connect to Cluckwork by naming the https URL of a JSON document it publishes as its client_id. This is a client ID metadata document (draft-ietf-oauth-client-id-metadata-document-02). The MCP authorization specification, revision 2026-07-28, recommends these documents and deprecates dynamic client registration (DCR). Before this change, a client that supports only metadata documents could not connect. DCR keeps working unchanged beside them.

For the person approving an app, the consent screen and the sign-in preview now show Verified domain: example.com instead of Unverified app for these clients. Details names the full document URL and says what Cluckwork checked: the details came from whoever controls that address. It names the URL, not only the domain, because some websites let anyone publish a file. The name is still the app's own choice. When the redirect goes to this computer, Details adds that the domain cannot vouch for which program receives the approval.

Closes #1148

How it works

Seam. An OpenIddict handler on the authorization request runs just before OpenIddict looks the client up. For a client_id that starts with https://, it makes sure a fresh application row exists. Every other client_id goes to the registered client as before. I rejected a custom application store that fetches inside FindByClientIdAsync. With that design the token endpoint would fetch too, and a second in-memory copy of the client would sit beside the row the foreign keys need anyway. The decision record compares both designs.

Storage. Each document URL gets one ordinary OpenIddict application row, built by the same ToDescriptor rules as a DCR client. The expiry is stored in OpenIddict's Properties column, so this PR has no migration. The row is the cache, and it lives in Postgres, so every replica reads the same copy (#271). Connected apps, audit provenance (#800), the farm switch (#1146), the #796 checks and the purge sweep read the row and the token claims, so they treat these clients like any other. A metadata client nobody approves is purged after a day like a registration, and its next use fetches it again. Only the authorization request fetches; the token endpoint redeems a code that was issued against the row.

The fetcher, which is the SSRF boundary. It follows these rules:

  • It resolves the host once and refuses it if any address is special-use. IPv4 uses the RFC 6890 blocks. IPv6 allows only 2000::/3, minus its special blocks. The fetcher then connects to the address it checked, so DNS rebinding has nothing to change.
  • The address table is checked against IANA's IPv4 and IPv6 Special-Purpose Address Registries. Review round 1 found 2620:4f:8000::/48 (AS112) missing, and it is now refused.
  • It refuses IPv4-mapped IPv6 addresses before any range check. A test found that .NET 10's IPNetwork.Contains places ::ffff:10.0.0.1 inside 2000::/3.
  • It follows no redirect and accepts only 200 and application/json.
  • Nothing is fetched for a request without response_type=code and an S256 code_challenge. OpenIddict's own parameter checks run only after its first client lookup in ValidateAuthentication, so the handler checks these itself first.
  • It sends no proxy request, no cookie and no credential. It uses no decompression, and it sends a fixed User-Agent.
  • The body may be at most 5 KB. The fetcher refuses a larger Content-Length from the headers, and it enforces the cap while a chunked body streams.
  • A connection that fails during the body read is a failed fetch and an OAuth refusal, never a 500.
  • One 5-second deadline covers resolving, connecting, the headers and the whole body.
  • Fetches spend a budget on the shared counter: 10 per URL per 5 minutes, then 60 in total per minute. A fresh row costs nothing. A client that someone approved (it holds a valid authorization) spends a second global budget of the same size, which strangers cannot drain. An outright exemption would let anyone with an account fetch outside every global bound by approving clients of their own. When a budget is spent, the request is refused, and an expired copy never stands in. A fallback was tried and removed in review round 2: failed refreshes spend the URL's budget, so ten anonymous requests against a document its publisher took down would drain it and revive the old redirect URIs. The accepted cost is that an app whose copy has expired cannot start a new connection until the window resets. Connections already approved keep working, because their tokens never fetch.
  • Copies last for the document's Cache-Control lifetime, between 5 minutes and 24 hours. Errors and invalid documents are never stored.

What a document may say. The document's client_id must equal its URL, and it must not carry a client secret. Everything else must pass the #797 rules: a public client, the authorization code grant only, and https or loopback redirects. The rules also sanitise the name. A lone surrogate in any string field is invalid metadata, not a server error.

Races. Two first requests for the same URL can both find no row. The loser meets OpenIddict's duplicate-client check, the unique index or the concurrency token, then re-reads the row in a fresh scope and uses the winner's copy. That recovery applies only when the duplicate client id is OpenIddict's sole refusal. A version the loser fetched that also breaks a rule, such as an iss parameter in a redirect URI, is still refused.

Which URLs qualify. The URL must be canonical ASCII https with a path, and it may not have a query. It is capped at 100 characters, because OpenIddictApplications.ClientId and the audit column are both 100 wide.

Discovery. Discovery advertises client_id_metadata_document_supported: true.

Changes a reviewer should check

  • API change. DELETE /me/connected-apps and DELETE /users/{id}/connected-apps now take ?clientId= instead of a path segment. A URL client id contains /, and Kestrel and proxies do not agree on how to decode an encoded slash in a path. The SPA has one caller for each route, and both are updated. I found no other callers in k6, Playwright or the docs.
  • Idempotency change. IdempotencyMiddleware now folds a write's non-empty query string into its request hash. Otherwise two Disconnects of different apps under one key would replay the first app's 204 and leave the second connected. Reusing the key now gets a 409. Writes without a query hash exactly as before, and only the two Disconnect calls send one.
  • Code placement. The shared rules, the fetcher and the handler live in the Platform namespace Cluckwork.Infrastructure.OAuth. The DCR Register endpoint and the handler share ToDescriptor. An endpoint that names an Access type outside its contract fails CW1004, and contracts can only live in Domain and Application.
  • Configuration. Two optional keys, so the sim harness and the AppHost need nothing:
    • OAuth:ClientMetadata:Enabled defaults to true. A deployment without outbound https turns it off. Discovery then stops advertising support, and an https client_id is refused before any fetch. deploy/.env.example documents this.
    • OAuth:ClientMetadata:PrivateHosts lists exact host names that may resolve to a private network, such as a test stack. Loopback, link-local and multicast stay refused for these hosts too.

Docs

Verification

  • ClientMetadataFetcherTests (no database) runs every fetcher rule over real TLS to a loopback Kestrel. The tests reach it only through a scripted resolver and dialler. They cover each refused address class, a mixed answer, the rebinding pin, PrivateHosts, a redirect, status codes, content types, the streamed and declared size caps, an interrupted body, a slow body, a slow connect, the request headers, the lifetime clamp and the address table.
  • ClientMetadataTests (no database) pins the URL shape, the length cap, the verified domain and the document rules against literal inputs, including a lone surrogate in each string field.
  • OAuthFailClosedTests.ClientMetadataDocuments (Postgres) covers the flow with a metadata client: connect, list, audit and disconnect, the farm switch, consent and preview, the cache, refresh, unusable documents stored nowhere, OpenIddict's iss refusal, a malformed request refused before any fetch, the per-URL and both global budgets, an approved app refreshed past a drained strangers' budget, a removed document that a drained budget does not revive, the column lengths behind the URL cap, the insert races (including one where the losing request fetched an invalid version), discovery, turning the feature off, and the purge. OAuthFailClosedTests.ConnectedApps pins the Disconnect fingerprint.
  • Locally, the mutation runner's suite (OAuth|ClientMetadata) passes 272 of 272, the idempotency suites pass, the architecture guards pass (383), and the web unit suite passes (3,776).
  • CI was green at 7429d88e, after review round 1, and again on the head this body was last updated for.
  • Precache: 1,955.37 KiB of 1,960 KiB at the head. That is 2.70 KiB over main, which measured 1,952.67 KiB at e4ec534.
  • Screenshots: consent and sign-in preview at 1280 and 390, before and after, are in this comment. The after frames come from a real fetch of a locally served document on a Production-config stack.

Known limits

The reviews raised these, and they are left unchanged on purpose:

  • A PrivateHosts entry admits every port on that host. Only hosts the operator lists are reachable, and the list exists for test stacks.
  • An approved app's reconnect skips the permission screen even after its document changed (draft §8.4). The consent screen still shows the redirect host, and every code still costs the password.
  • For a verified app that returns to this computer, the caveat that the domain cannot vouch for the receiving program sits inside collapsed Details.
  • Someone with an account can drain the approved clients' fetch budget by approving many clients of their own, each with a sign-in and a password. That delays refreshes for other approved apps until the window resets, and no stale copy is ever used.

Mutation proof

tools/oauth/mutation-check.sh carries 52 cimd-* mutants, and its baseline filter includes the two Docker-free classes. Each mutant is one exact edit. The runner rebuilds, runs the one named test, and reads the verdict from the TRX file. All 52 ran at a3a56fd6, and all were killed. One was killed by a different assertion than its row declared: with every client counted as approved, the strangers' drain empties the approved budget too. Its row now declares that assertion, and its rerun was killed. Baseline and restore were green at 272 of 272. Earlier rounds produced the same results and are summarised in the review replies.

Mutant File Test Verdict
cimd-mixed-answer-admitted ClientMetadataFetcher ClientMetadataFetcherTests.MixedAnswer_IsRefused_WhicheverAddressComesFirst killed
cimd-mapped-admitted ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-v6-not-allow-list ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-private-v4-admitted ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-link-local-admitted ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-cgnat-admitted ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-rebinding ClientMetadataFetcher ClientMetadataFetcherTests.Connection_IsPinnedToTheVettedAddress killed
cimd-private-hosts-any-name ClientMetadataFetcher ClientMetadataFetcherTests.PrivateHosts_AdmitsOnlyTheNamedHost_OnAPrivateNetwork killed
cimd-private-hosts-loopback ClientMetadataFetcher ClientMetadataFetcherTests.PrivateHosts_AdmitsOnlyTheNamedHost_OnAPrivateNetwork killed
cimd-redirect-followed ClientMetadataFetcher ClientMetadataFetcherTests.Redirect_IsNotFollowed killed
cimd-any-2xx ClientMetadataFetcher ClientMetadataFetcherTests.OnlyOk_IsAccepted killed
cimd-content-type-unchecked ClientMetadataFetcher ClientMetadataFetcherTests.OtherContentType_IsRefused killed
cimd-stream-uncapped ClientMetadataFetcher ClientMetadataFetcherTests.StreamedBody_IsCappedWhileReading killed
cimd-declared-length-unchecked ClientMetadataFetcher ClientMetadataFetcherTests.DeclaredLength_OverTheCap_IsRefusedBeforeReading killed
cimd-body-no-deadline ClientMetadataFetcher ClientMetadataFetcherTests.SlowBody_StopsAtTheDeadline killed
cimd-no-deadline ClientMetadataFetcher ClientMetadataFetcherTests.SlowConnect_StopsAtTheDeadline killed
cimd-cookies-kept ClientMetadataFetcher ClientMetadataFetcherTests.Request_CarriesNoCookie_AndAnHonestUserAgent killed
cimd-proxy-used ClientMetadataFetcher ClientMetadataFetcherTests.Handler_FollowsNoRedirect_UsesNoProxy_KeepsNoCookies killed
cimd-lifetime-unclamped ClientMetadataFetcher ClientMetadataFetcherTests.Lifetime_FollowsCacheControl_WithinTheBounds killed
cimd-no-cache-kept-long ClientMetadataFetcher ClientMetadataFetcherTests.Lifetime_FollowsCacheControl_WithinTheBounds killed
cimd-client-id-case ClientMetadata ClientMetadataTests.Document_NamingAnotherClientId_IsRefused killed
cimd-secret-allowed ClientMetadata ClientMetadataTests.UntrustedDocument_IsRefused killed
cimd-duplicate-keys ClientMetadata ClientMetadataTests.UntrustedDocument_IsRefused killed
cimd-url-noncanonical ClientMetadata ClientMetadataTests.OtherForms_AreRefused killed
cimd-url-query ClientMetadata ClientMetadataTests.OtherForms_AreRefused killed
cimd-url-ip-literal ClientMetadata ClientMetadataTests.OtherForms_AreRefused killed
cimd-url-length ClientMetadata ClientMetadataTests.Url_LongerThanTheColumn_IsRefused killed
cimd-cache-ignored ClientMetadataDocuments OAuthFailClosedTests.StoredCopy_IsReused_UntilItExpires killed
cimd-never-refetched ClientMetadataDocuments OAuthFailClosedTests.Refetch_AppliesTheNewDocument killed
cimd-update-skipped ClientMetadataDocuments OAuthFailClosedTests.Refetch_AppliesTheNewDocument killed
cimd-per-url-budget ClientMetadataDocuments OAuthFailClosedTests.FailingUrl_IsFetchedOnlyWithinItsBudget killed
cimd-global-budget ClientMetadataDocuments OAuthFailClosedTests.ManyUrls_ShareOneGlobalBudget killed
cimd-race-unhandled ClientMetadataDocuments OAuthFailClosedTests.ConcurrentFirstRequests_StoreOneRow killed
cimd-issuer-in-redirect-stored ClientMetadataDocuments OAuthFailClosedTests.DocumentWithAnIssuerInItsRedirect_IsRefused_AndNotStored killed
cimd-disabled-still-fetches ClientMetadataDocuments OAuthFailClosedTests.TurnedOff_NeitherAdvertisesNorFetches killed
cimd-not-advertised OAuthServerRegistration OAuthFailClosedTests.Discovery_AdvertisesMetadataDocuments killed
cimd-handler-unregistered OAuthServerRegistration OAuthFailClosedTests.MetadataClient_ConnectsListsAuditsAndDisconnects_LikeAnyOther killed
cimd-consent-domain-hidden OAuthEndpoints OAuthFailClosedTests.Consent_AndPreview_NameTheVerifiedDomain killed
cimd-preview-domain-hidden OAuthEndpoints OAuthFailClosedTests.Consent_AndPreview_NameTheVerifiedDomain killed
cimd-disconnect-by-path MeEndpoints OAuthFailClosedTests.MetadataClient_ConnectsListsAuditsAndDisconnects_LikeAnyOther killed
cimd-interrupted-body-escapes ClientMetadataFetcher ClientMetadataFetcherTests.InterruptedBody_IsUnreachable killed
cimd-as112-v6-admitted ClientMetadataFetcher ClientMetadataFetcherTests.SpecialUseAddress_IsRefused_BeforeAnyConnection killed
cimd-lone-surrogate-escapes ClientMetadata ClientMetadataTests.LoneSurrogate_InAnyStringField_IsInvalidMetadata killed
cimd-client-id-unchecked ClientMetadata ClientMetadataTests.Document_NamingAnotherClientId_IsRefused killed
cimd-duplicate-client-refused ClientMetadataDocuments OAuthFailClosedTests.SecondRequest_MeetingTheFirstsCopy_UsesIt killed
cimd-stale-fallback-restored ClientMetadataDocuments OAuthFailClosedTests.RemovedDocument_IsNotRevived_ByDrainingTheBudget killed
cimd-mixed-validation-recovered ClientMetadataDocuments OAuthFailClosedTests.SecondRequest_WithAnInvalidDocument_IsRefused_DespiteTheFirstsCopy killed
cimd-idempotency-query-ignored IdempotencyMiddleware OAuthFailClosedTests.Disconnect_ReusingAKeyForAnotherApp_IsAConflict_NotAReplay killed
cimd-response-type-fetched ClientMetadataDocuments OAuthFailClosedTests.MalformedRequest_IsRefused_BeforeAnyFetch killed
cimd-pkce-fetched ClientMetadataDocuments OAuthFailClosedTests.MalformedRequest_IsRefused_BeforeAnyFetch killed
cimd-approved-shares-strangers-budget ClientMetadataDocuments OAuthFailClosedTests.ApprovedApp_IsRefreshed_PastADrainedGlobalBudget killed
cimd-everyone-gets-approved-budget ClientMetadataDocuments OAuthFailClosedTests.ApprovedApp_IsRefreshed_PastADrainedGlobalBudget killed

A client_id that is an https URL now names the client's metadata document
(draft-ietf-oauth-client-id-metadata-document-02, MCP 2026-07-28). An
OpenIddict handler keeps one ordinary application row per URL, built by
the same rules as DCR, and refetches it after its Cache-Control lifetime.
The fetcher resolves once, refuses special-use addresses, connects to the
vetted address, and follows no redirect, under a size cap, a deadline and
a shared fetch budget. Consent and the sign-in preview show the verified
domain. The Connected apps disconnect takes the client id as a query
parameter, because a URL id cannot travel in a path segment.

Refs #1148
Three findings from the security review and the mutation pass:

- A drained global fetch budget no longer locks out apps already in use.
  An expired copy that once passed every check stands in until a fetch is
  possible again; only a client never stored is refused.
- A concurrent first request that loses OpenIddict's duplicate client_id
  check now accepts the other request's copy instead of answering 400.
- Consent Details names the full document URL, because a domain that
  hosts files for anyone verifies nothing about who published one.

Refs #1148
Review round 1 (Codex GPT-6 Astra) and the commit security review:

- A body cut short after the headers (HttpIOException) is a failed
  fetch and an OAuth refusal, not a server error.
- 2620:4f:8000::/48 (AS112) joins the refused IPv6 blocks; the table is
  checked against both IANA special-purpose registries.
- A lone surrogate in any string field of a document is invalid
  metadata, not a server error.
- A deterministic test pins the duplicate-client race: the second
  request uses the first request's stored copy.
- The stale copy used while the fetch budget is spent is bounded to 24
  hours past its expiry, so it cannot keep a removed redirect alive.
- tools/oauth/mutation-check.sh gains 47 metadata-document mutants and
  runs the two Docker-free test classes in its baseline.

Refs #1148
The iss refusal test now pins OpenIddict's reason, so letting the refused
document through unstored is caught. Two mutant rows no longer declare an
unused variable, two declare xUnit's real message, and the baseline floor
is the suite's measured 261. All 47 cimd mutants are killed.

Refs #1148
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Screenshots of the consent screen and the sign-in preview, at 1280 and 390, with Details closed and open.

Before is main at e4ec5347, with an app that registered itself through DCR. After is this branch at dbe3ac58, with an app whose client_id is https://assistant.example/oauth/claude-code.json. The commits after dbe3ac58 change no UI. Each stack was an isolated sim stack (cw1148main and cw1148) running Production config behind a local Caddy https edge. Caddy also served the metadata document on a private Docker network, which the API reached through OAuth:ClientMetadata:PrivateHosts. The after frames therefore come from a real fetch through the SSRF-checked fetcher, not from a seeded row. Both stacks were removed with down -v.

Sign-in preview, before and after (1280).
Sign-in before, 1280
Sign-in after, 1280

Sign-in preview, before and after (390).
Sign-in before, 390
Sign-in after, 390

Consent, before and after (1280). The Unverified app marker becomes Verified domain: assistant.example.
Consent before, 1280
Consent after, 1280

Consent, before and after (390).
Consent before, 390
Consent after, 390

Details open, before and after (1280). After names the full document URL and adds the loopback caveat.
Details before, 1280
Details after, 1280

Details open, before and after (390).
Details before, 390
Details after, 390

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 from Codex GPT-6 Astra (head a18faef3) is addressed in d61aacb3, dbe3ac58 and d8225fc1; 7429d88e is a comment and docs tidy. All five findings are fixed.

F1 (P2), interrupted body. Fixed in dbe3ac58. The fetcher now treats an IOException during the body read the same way as an HttpRequestException. HttpIOException is one such exception. Both return metadata.unreachable, which the handler turns into an OAuth invalid_request, never a 500. The read path can throw nothing else: an HttpRequestException before the headers, an IOException during the read, or an OperationCanceledException at the deadline, which was already handled. InterruptedBody_IsUnreachable uses the scenario from the report: Content-Length: 100, then {}, then an aborted connection. It records any exception the fetch throws, so a regression fails an assertion rather than crashing the test. Mutant cimd-interrupted-body-escapes narrows the catch back to HttpRequestException, and the test kills it.

F2 (P3), AS112 IPv6. Fixed in dbe3ac58. 2620:4f:8000::/48 joins the refused IPv6 blocks. I checked the whole table against IANA's IPv4 and IPv6 Special-Purpose Address Registries (CSV fetched 2026-10-10). Every IPv4 entry falls inside a refused block. Every IPv6 entry either is one of the refused blocks inside 2000::/3 or lies outside 2000::/3, which the allow-list already refuses. AS112 was the only gap. The code comment and the decision record cite both registries. New refused-address rows: 2620:4f:8000::1, 192.175.48.1, 5f00::1 and 100::1. Mutant cimd-as112-v6-admitted removes the block, and the test kills it.

F3 (P2), lone surrogate. Fixed in dbe3ac58. FromDocument now treats InvalidOperationException like JsonException, as invalid metadata. LoneSurrogate_InAnyStringField_IsInvalidMetadata puts \uD800 or \uDC00 into each string field the document is read for: client_id, client_name, a redirect URI, a grant type, a response type and token_endpoint_auth_method. It records any exception, as the F1 test does. Mutant cimd-lone-surrogate-escapes narrows the catch back to JsonException, and the test kills it.

F4 (P2), duplicate-client race. Fixed in d61aacb3, which was pushed after the reviewed head. On a ValidationException from the create path, the handler re-reads the row in a fresh scope and uses it when another request stored one. This request's application cache already holds "none", so it needs the fresh scope. Any other validation refusal, such as an iss parameter in a redirect URI, is still refused. SecondRequest_MeetingTheFirstsCopy_UsesIt pins the reported schedule deterministically. Both requests read no row and fetch. The fake server holds the second response until the first request's row is in the database, so the second insert meets OpenIddict's duplicate-client check. Both requests then answer 200. The simultaneous-arrival test no longer swallows its barrier timeout. Mutant cimd-duplicate-client-refused drops the re-read, and the deterministic test kills it.

F5 (P3), mutation proof. Fixed in dbe3ac58 and d8225fc1. tools/oauth/mutation-check.sh now carries 47 cimd-* mutants against the fetcher, the document rules, the resolver, discovery, consent and the disconnect route. Its baseline filter is OAuth|ClientMetadata, which takes in the two Docker-free classes, and its floor is the measured 261. The rows cover every case you listed: a private range, link-local and metadata, IPv4-mapped IPv6, the pinned-address connect, redirects, both size caps, the client_id equality, the content type, and both fetch budgets. All 47 are killed, with baseline and restore green at 261 of 261. Getting there took a correction. In the first run, two rows did not compile, two declared a message xUnit does not print, and one survived: cimd-issuer-in-redirect-stored. With OpenIddict's iss refusal turned into success, nothing is stored and the unknown client is refused anyway, so the test could not tell them apart. The test now pins OpenIddict's reason, d8225fc1 corrects the rows, and the rerun killed all five. The table is in the PR body, and the decision record now describes this run.

One more change in dbe3ac58, from the commit security review. d61aacb3 let an expired stored copy stand in while the fetch budget is spent, so that someone who drains the global budget cannot lock out apps already in use. That fallback is now bounded to 24 hours past the copy's expiry (StaleGrace). After that, the request is refused, so a drained budget cannot keep a redirect URI the publisher removed. ManyUrls_ShareOneGlobalBudget_AndARecentCopyOutlastsIt covers both sides, and two mutants pin it.

… pure duplicate

Review round 2 (Codex GPT-6 Astra):

- The stale-copy fallback is gone. Failed refreshes spend the URL's budget,
  so draining it would revive the redirect URIs of a document its publisher
  removed. While the budget is spent, an app whose copy expired waits.
- A losing concurrent insert adopts the winner's row only when OpenIddict's
  sole refusal is the duplicate client id; an iss parameter in the version
  it fetched is still refused.
- The mutation runner no longer guards or restores UserEndpoints.cs, which
  no mutant targets.

Refs #1148
Review by Claude Fable 5.1 at 7429d88:

- The idempotency fingerprint covers a write's query string, so two
  Disconnects of different apps under one key conflict instead of the
  second replaying the first's 204 and leaving its app connected. A
  write without a query hashes exactly as before.
- A metadata request without response_type=code or an S256 code
  challenge is refused before any fetch. OpenIddict's own parameter
  checks run only after its first client lookup in
  ValidateAuthentication, so the handler cannot move after them.
- A client someone approved skips the global fetch budget, which
  strangers can drain, and still spends its per-URL budget; its copy is
  refreshed, never served stale.
- The mixed DNS answer test covers both orders, and a guard ties the
  URL cap to the ClientId and audit columns in the EF model.
- The decision record states the first lookup, that Enabled=false
  refuses and never revokes, the column behind the cap, and the
  anonymous row bound.

Refs #1148
2d36aa5 exempted a client with a valid authorization from the global
fetch budget. Anyone with an account can approve clients of their own,
so the exemption let them fetch outside every global bound (commit
security review). Approved clients now spend a separate global budget of
the same size, which strangers cannot drain, so total fetches stay
bounded. The approval check goes through OpenIddict's authorization
manager rather than Access's table, which the compatibility-exception
guard refused.

Refs #1148
@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

This covers Codex GPT-6 Astra's round 2 (head 7429d88e) and Claude Fable 5.1's review (also at 7429d88e). The fixes are in 1426cacd, 309bc4b5, c2c0b7be, 2d36aa57, 058416da, a3a56fd6, 6b4192cd, cf8c5c40 and 359d24c9. All 52 cimd-* mutants ran at a3a56fd6 and all were killed, with baseline and restore green at 272 of 272. The table is in the PR body.

Codex GPT-6 Astra, round 2

R2-F1 (P1), a drained budget revived refused metadata. Fixed in 1426cacd by removing the fallback. An expired copy now never stands in, whatever the budget says. While a budget is spent, an app whose copy has expired cannot start a new connection until the window resets, and connections already approved keep working because their tokens never fetch. RemovedDocument_IsNotRevived_ByDrainingTheBudget follows your sequence exactly. A stored copy expires, the publisher takes the document down (404), and failed refreshes drain the URL's budget. The next signed-in request is refused with temporarily_unavailable. Its red mutation, cimd-stale-fallback-restored, re-adds the fallback, and the test kills it. The decision record now says why there is no fallback.

R2-F2 (P2), duplicate recovery hid other validation failures. Fixed in 1426cacd. The losing request adopts the winner's row only when OpenIddict's sole validation result is ID2111, "An application with the same client identifier already exists." SecondRequest_WithAnInvalidDocument_IsRefused_DespiteTheFirstsCopy runs your two-version race deterministically. The first request stores a valid copy. The second fetches a version with an iss parameter in one redirect URI, meets both the duplicate and the iss results, and is refused with OpenIddict's iss reason. Its red mutation, cimd-mixed-validation-recovered, drops the sole-result check, and the test kills it.

R2-F3 (P3), USERS in the runner. Removed from the guard and the restore set in 1426cacd.

Claude Fable 5.1

F1 (P2), Disconnect lost its idempotency fingerprint. Fixed in 2d36aa57. IdempotencyMiddleware now folds a non-empty query string into the request hash. One key reused for another app is then a 409, "Idempotency-Key was already used with a different request", not a replay of the first app's 204. A write without a query hashes exactly as before, so no stored row changes. I checked the callers (#394): the two SPA Disconnect calls are the only writes that send a query, and each mints a fresh key. Disconnect_ReusingAKeyForAnotherApp_IsAConflict_NotAReplay disconnects app A with key K (204), retries it (204, replayed), then disconnects app B with K (409), and B's token still answers 200. Its red mutation, cimd-idempotency-query-ignored, stops hashing the query, and the test kills it. The existing idempotency suites pass.

F2 (P3), the fetch came before the cheap checks. Fixed in 2d36aa57, though not by reordering. The record was right about where the first lookup happens. When I moved the handler to just before ValidateResponseType, metadata clients were refused with ID2052, "The specified 'client_id' is invalid". That refusal comes from the generic ValidateClientId that ValidateAuthentication dispatches. So the first lookup is in ValidateAuthentication, and every parameter check runs after it. The handler therefore stays where it was and refuses, before any fetch, what every request to this server needs: response_type=code, and a code_challenge using S256. It answers with OpenIddict's own error codes. MalformedRequest_IsRefused_BeforeAnyFetch covers a missing or wrong response_type, a missing code_challenge, and a plain or missing method, each with zero resolves. Its red mutations, cimd-response-type-fetched and cimd-pkce-fetched, drop each check, and the test kills both. The record now explains where the lookup happens and why.

F3 (P3), the cap named the wrong column. Fixed in 2d36aa57. MetadataUrlCap_FitsEveryColumnThatStoresIt reads both OpenIddict's ClientId length and the audit column's length from the EF model, and fails if either drops below MaxDocumentUrlLength. The record names OpenIddict's column as the one a longer URL hits first.

F4 (P3), Enabled=false wording. Fixed in the record and in src/AGENTS.md. Like the farm switch, it refuses new connections and revokes none.

F8 (P3), the mixed-answer test covered one order. It is now a theory over both orders.

F9 (P3), ponytail notes. Done: the duplicated size message is one field, and USERS is gone (R2-F3). I kept PrivateHosts nullable, because a positional record parameter cannot default to a new set.

F10 (P3), the junk-row bound. The record now states it: about 86,400 rows a day from three addresses, held to about one day of that rate by the purge of unapproved rows.

Forward note: the global-budget exemption. Built, but changed. As an outright exemption from the global budget (2d36aa57), the commit security review pointed out a problem. Anyone with an account can approve clients of their own, so the exemption let them fetch outside every global bound. 058416da gives approved clients a separate global budget of the same size instead. Strangers cannot drain it, a client still spends its per-URL budget, and total fetches stay at most 120 a minute. Nothing is ever served stale. The approval check goes through OpenIddict's authorization manager, because reading Access's table directly fails the compatibility-exception guard. ApprovedApp_IsRefreshed_PastADrainedGlobalBudget drains the strangers' budget, then shows that an approved app's expired copy is still fetched and refreshed, while an unapproved stored copy and an unknown URL are both refused. Its red mutations are cimd-approved-shares-strangers-budget and cimd-everyone-gets-approved-budget, and the test kills both. A second security review noted the residual risk: someone with an account could drain the approved budget by approving many clients of their own, each with a sign-in and a password. That costs availability only, because no stale copy is ever used. It is recorded as an accepted limit in the decision record and the PR body.

Left as known limits, unchanged: F7, the PrivateHosts port note; F5, the reconnect shortcut surviving a metadata change (draft §8.4); F6, the loopback caveat sitting in collapsed Details. The PR body lists them.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Final review by Claude Fable 5.1 at head 359d24c: 0 P1, 0 P2, 2 P3.

Confirmed:

  • The stale fallback is gone. A spent budget returns temporarily_unavailable and never serves an expired row.
  • The ID2111 sole-result recovery is robust.
  • IdempotencyMiddleware hashes writes without a query byte for byte as before, and the Make API write idempotency atomic across replicas #307 claim and replay protocol is unchanged.
  • The approved-client budget is on the shared counter, and strangers cannot drain it.
  • All 52 mutants are valid.

Known limits, recorded here and not fixed:

  1. The query and the body share one hash stream with a \n separator. A body that begins with ? could collide with a query, but only for the same account, the same key and the same path.
  2. The pre-fetch checks restate the code-flow and S256 settings as literals, so a future flow change has two places to update.

Also noted: a disabled user's authorizations count as approved for the fetch budget until an Owner disconnects them. This affects availability only. Disabling the user already refuses their tokens.

The review loop for this PR is complete. Codex GPT-6 Astra ran two rounds, then Claude Fable 5.1 ran an independent round and a final check.

@mforce
mforce merged commit bd4e5ec into main Oct 10, 2026
19 checks passed
@mforce
mforce deleted the feat/1148-oauth-client-metadata-documents branch October 10, 2026 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Client ID Metadata Documents alongside dynamic client registration

1 participant