Ignore unsupported grant types in CIMD documents

[amirejaz]

Summary

VS Code's real-world client-metadata document declares urn:ietf:params:oauth:grant-type:device_code in grant_types alongside authorization_code and refresh_token, and the CIMD resolver rejected the entire document over it — surfacing to the user as an opaque invalid_client ("The requested OAuth 2.0 Client does not exist") at /oauth/authorize. Since VS Code is the client CIMD was explicitly built for (#4825), this broke the feature for its primary consumer.

The root problem is a semantic mismatch: a CIMD document describes the client's capabilities across every authorization server it talks to, and the client cannot tailor it per server — so an entry this server doesn't support must not be fatal. A DCR request, by contrast, is addressed to this server specifically, so strict rejection remains correct feedback there.

• New registration.FilterPublicGrantTypes / FilterPublicResponseTypes drop unsupported entries instead of rejecting the set, but still reject when the intersection lacks the one flow this server offers (authorization_code / code) — such a client could never complete a token exchange, and a clear error at resolution beats failing every token request
• The CIMD decorator uses the filters; the stored fosite client carries only the filtered types, threaded explicitly into buildFositeClient rather than re-read from the raw document
• Dropped entries are logged at Debug with declared vs effective sets, addressing the diagnosability complaint in the issue (nothing was logged server-side)
• DCR's strict ValidatePublicGrantTypes / ValidatePublicResponseTypes are unchanged

Fixes #6290

Type of change

• Bug fix

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

New tests: a regression test resolving VS Code's real document shape (device_code alongside authorization_code, mixed loopback/https redirect URIs), filtered-vs-rejected cases in the decorator's grant/response-type tables, and table-driven tests for the two new filter functions. Pre-existing failures in pkg/plugins/pluginsvc and a gosec finding in cmd/thv/app/upgrade.go reproduce identically on clean main and are unrelated.

Does this introduce a user-facing change?

MCP clients whose CIMD document declares grant types or response types the embedded auth server does not support (e.g. VS Code's device_code) now resolve successfully, with the unsupported entries ignored, instead of failing with invalid_client.

Special notes for reviewers

• The previously tested behavior ("device_code rejected") was deliberate at CIMD implementation time; this PR flips that policy for CIMD only, with the reasoning captured in the FilterPublicGrantTypes godoc.
• The issue also notes the /oauth/authorize error path logs nothing server-side, which made this hard to diagnose. The Debug log here covers the CIMD-rejection slice of that; the general authorize-path logging gap is left for a follow-up as the issue suggests.

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.86%. Comparing base (3c4dec3) to head (dde7878).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6297      +/-   ##
==========================================
+ Coverage   72.84%   72.86%   +0.01%     
==========================================
  Files         743      743              
  Lines       77681    77707      +26     
==========================================
+ Hits        56586    56618      +32     
+ Misses      17129    17122       -7     
- Partials     3966     3967       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[amirejaz]

@claude could you review the PR and make sure the fix follows the MCP specs about CIMD

[claude]

Claude finished @amirejaz's task in 3m 45s —— View job

---

Review: CIMD grant/response type filtering vs. the MCP spec

• Read PR diff (dcr.go, cimd_decorator.go + tests)
• Read surrounding CIMD/DCR code (pkg/oauthproto/cimd/fetch.go, handlers/dcr.go, oauthproto/redirect.go)
• Check MCP draft spec (authorization + client-registration) and RFC 7591
• Post review

---

Verdict

The fix is consistent with the MCP CIMD spec, and the reasoning in the godoc is the right one. No blocking issues. Two nits and three follow-up gaps of the same shape as #6290 that this PR (correctly) doesn't touch.

Spec check

The MCP draft's client-registration page lists exactly what an AS MUST do with a CIMD document:

• MUST validate that the fetched document's client_id matches the URL exactly
• MUST validate redirect URIs presented in an authorization request against those in the metadata document
• MUST validate the document structure is valid JSON and contains required fields

Nothing there requires rejecting a document over grant_types/response_types the AS doesn't implement — and the same page makes the PR's argument normative:

Client IDs based on Client ID Metadata Documents are portable across authorization servers, since they are self-hosted HTTPS URLs resolved by the authorization server on demand.

A portable, self-hosted document that the client cannot tailor per-AS must not be fatal on an entry one AS doesn't support. RFC 7591 §2 backs the mechanism too — "the authorization server MAY reject or replace any of the client's requested metadata values … and substitute them with suitable values" — filtering is substitution.

Keeping authorization_code/code as hard invariants is right and not over-strict: fosite enforces GetGrantTypes()/GetResponseTypes() at the authorize and token endpoints anyway, so a client without them fails every request regardless; failing at resolution just makes it legible. And storing the filtered list on the fosite client (rather than the document's declaration) is the correct call — it keeps device_code from ever appearing as server policy.

Behavior check

registration.defaultGrantTypes = {authorization_code, refresh_token} and defaultResponseTypes = {code} (dcr.go:88-101) are byte-identical to the deleted defaultCIMDGrantTypes/defaultCIMDResponseTypes, so removing the duplicated CIMD defaults leaves the omitted-field path unchanged. Debug level for the drop log matches .claude/rules/go-style.md ("DEBUG for diagnostics"), and client_id is public metadata — no secret leakage. CI is green; codecov shows the new lines covered.

Nits

1. cimd_decorator.go:161 / :170 — dropped is computed but never used. It only feeds the condition, and the log message reports declared-vs-effective instead (which is more useful). A plain length comparison says the same thing with one fewer binding:

if len(grantTypes) < len(doc.GrantTypes) {

2. dcr.go:491 / :517 — the default slice is returned unaliased into a cached client. return defaultGrantTypes, nil hands the package-level slice to buildFositeClient, which stores it in fosite.DefaultClient.GrantTypes for every CIMD client in the LRU. Nothing mutates it today (fosite only reads), and validateGrantTypes already did this, so it's not a regression — but it's inconsistent with the scope path two lines over, which does slices.Clone(registration.DefaultScopes) (cimd_decorator.go:256). Cloning both defaults would make the ownership rule uniform. Fix this →

Duplicate entries in a declared list survive the filter (["authorization_code","authorization_code"]), same as validateGrantTypes — harmless, and out of scope.

Follow-ups: same failure shape, other fields

These are all pre-existing and correctly outside this PR under the one-logical-change rule, but they're the next #6290s:

a) The DCR path rejects the same client. ValidatePublicGrantTypes stays strict, which the description defends as "a DCR request is addressed to this server specifically." True in principle — but clients generally send the same metadata on the DCR fallback path when client_id_metadata_document_supported is absent, so a client declaring device_code still gets invalid_client_metadata from /register on a ToolHive AS with CIMD disabled. RFC 7591 permits substitution there too, and unlike CIMD the DCR response echoes the registered grant_types/response_types (handlers/dcr.go:208-209), so the client actually learns the effective set — the diagnosability objection is weaker there, not stronger. Worth confirming what VS Code sends to /register before deciding this path is fine as-is.

b) redirect_uris in a CIMD document are validated with RedirectURIPolicyStrict (pkg/oauthproto/cimd/fetch.go:198), which allows only https and http-loopback — while DCR's public-client path uses RedirectURIPolicyAllowPrivateSchemes (registration/dcr.go:367), explicitly allowing vscode://, cursor:// per RFC 8252 §7.1. So a native client whose portable document lists a private-use redirect is rejected wholesale, with the identical opaque invalid_client surface this PR just fixed — for exactly the class of client CIMD targets. If that asymmetry is deliberate (a self-asserted private-scheme redirect is claimable by any local app, so there is a real argument for it), it deserves the same explanatory godoc treatment FilterPublicGrantTypes just got.

c) scope is rejected wholesale when a declared scope isn't in scopes_supported (cimd_decorator.go:190-195). Same portability argument, and intersecting is strictly safer than rejecting here since it can only narrow the client's scope set. This is the closest analogue to what you just fixed.

Not worth acting on: token_endpoint_auth_method rejection is correct as-is (the server genuinely cannot honor client_secret_* for a client that has no secret with it). Separately, the spec's "SHOULD cache metadata respecting HTTP cache headers" isn't met — the decorator applies a fixed FallbackTTL and ignores Cache-Control — also pre-existing.

Tests

Good shape. The regression test pins the real VS Code document shape, the two table tests cover pass-through / filtered / rejected, and the buildFositeClient test now asserts the document's declaration is ignored in favour of the passed-in filtered list — which is the property that actually matters. The buildFositeClientWithDefaults helper keeps the unrelated tests readable. One thing worth double-checking against the live document: https://vscode.dev/oauth/client-metadata.json — if its real redirect_uris include a vscode:// entry, point (b) above means the regression test passes while the real document still fails.
· cimd-ignore-unsupported-grant-types

[jhrozek]

Reviewed with security and code-quality passes (go-security-reviewer, code-reviewer agents) — no findings at confidence ≥8/10.

• Filter/reject paths both draw from the same allowedGrantTypes/allowedResponseTypes maps as DCR's stricter validators, so there's no drift and unsupported entries never reach the stored fosite.Client.
• authorization_code/code remain hard-required, so the leniency is bounded.
• Good regression coverage, including TestFetch_VSCodeDocumentResolves mirroring the real VS Code CIMD document from #6290.

Approving.