feat(mcp): client_credentials grant with CIMD-first private_key_jwt agents - #170
Conversation
…gents (#161, #162) Headless agents authenticate as themselves with RFC 7523 private_key_jwt (EdDSA/Ed25519) client assertions on a new client_credentials grant. Resolution/validation (#161): CIMD documents declaring client_credentials take a distinct shape — exactly that one grant, private_key_jwt, an inline JWK Set of 1..8 PUBLIC Ed25519 keys (private material rejects the document; jwks_uri rejected outright — no second SSRF surface), and NO redirect_uris/response_types (deviation from the CIMD draft's required list, per RFC 7591's redirect-based-grants-only semantics; documented). Credentials documents only materialize when the operator has pinned clientIdMetadataDocuments.allowedHosts — enforced at resolution, on cache hits, and at startup: hosting a reachable document must never suffice to mint tokens. DCR back-compat dropped: SUPPORTED_GRANT_TYPES unchanged, so registration still rejects client_credentials. Grant (#162): explicit opt-in via mcp.clientCredentials.enabled (default OFF; disabled is indistinguishable from any unsupported grant). Verifies the assertion with the #160 primitives (aud = token endpoint, exp <= 60s, jti single-use via the replay table — storage failures fail closed to 500), rejects any Basic/secret riding along, binds RFC 8707 resource by exact match (invalid_target otherwise), and mints the existing RS256 token with sub = client_id (RFC 9068 §2.2), TTL 300s default, and never a refresh token. onMCPTokenIssued gains type 'client_credentials'. Discovery advertises client_credentials/private_key_jwt/EdDSA only when enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
…etimes, case-insensitive auth-scheme guard - Check the RFC 8707 resource match BEFORE consuming the jti: a recoverable request-param mistake must not burn the single-use assertion (regression test proves the same assertion retries). - Reject exp <= iat as structurally malformed in verifyClientAssertion. - Match the Basic-auth riding-along guard case-insensitively (RFC 9110 §11.1 schemes are case-insensitive). - Drop the stale '#162 will wire' comment — the knobs stay unwired by design; defaults are the conservative 60s/5s. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Code Review
This pull request implements the RFC 7523 client_credentials grant for headless agents within the Model Context Protocol (MCP) authorization flow. It introduces configuration options to enable the grant and define access token lifetimes, enforces strict startup validation requiring a pinned host allowlist and CIMD resolution, and implements client assertion verification with replay protection. Feedback on the changes suggests strengthening the validation of the registered JWK Set during CIMD document resolution by ensuring kid uniqueness, requiring kid when multiple keys are present, and strictly validating the Ed25519 x coordinate length and alphabet.
|
Reviewed; no blockers found. |
…shape at resolution Verification already fails closed on missing/duplicate kids (selectKey requires exactly one match) — but a fleet publishing a malformed rotation set would cache fine and then fail every assertion with a confusing verify error. Rejecting at document resolution surfaces the problem once, clearly. Also pins x to the exact 43-char base64url shape of an Ed25519 public key so malformed keys never enter the cache, and fixes the unused _omit lint errors CI caught in cimd.test.js. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
The allowedHosts allowlist — the gate between 'hosts a reachable document' and 'mints tokens' — is enforced on the CIMD resolution path. Require _cimd on the resolved record so a stored (DCR) client can never mint here, even if a future DCR surface could register the credentials shape; lifting the pin requires its own registration gate (#161's optional initialAccessToken leg). Gemini review suggestion; the companion suggestion (record the jti after minting) was rejected — check-and-record before mint is the anti-replay barrier, and moving it would open a concurrent-replay window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
|
Gemini suggestions dispositioned on 1. 2. Record the jti after minting — rejected. The check-and-record is the anti-replay barrier: deferring it past Suite: 1017 tests / 1015 pass / 2 pre-existing skips; lint + prettier clean. 🤖 Response by Claude (Fable 5) on Nathan's behalf |
There was a problem hiding this comment.
Review: request changes
1. Client-credentials can advertise and use a cleartext token endpoint — high
src/index.ts:190-200 accepts an http: mcp.issuer, and the new clientCredentials gate at :216-229 does not tighten that. Discovery consequently publishes an HTTP token endpoint; an agent sends its signed assertion there and receives the bearer token there. A network observer can steal the token and race the still-valid assertion. OAuth requires TLS for the token endpoint (RFC 6749 §§1.6, 3.2).
Require an https: issuer when mcp.clientCredentials.enabled is true. If local development needs HTTP, make a narrowly-scoped loopback-only development exception rather than permitting a cleartext remote AS.
2. The asserted single-use replay property is not provided under concurrency — high
src/lib/mcp/token.ts:532 treats MCPAssertionJtiStore.checkAndRecord() as the final anti-replay barrier, but src/lib/mcp/assertionJtiStore.ts:20-29 and its test at test/lib/mcp/assertionJtiStore.test.js:108-120 document that concurrent Table.create() calls can all succeed under Harper's current pre-staging existence check. A captured, still-valid assertion can therefore be redeemed concurrently for multiple five-minute access tokens; the 120-second row TTL only blocks later requests.
Do not claim or depend on single-use jti enforcement until the underlying write is atomic across the deployments this plugin supports. Gate this grant on an atomic store/primitive (including cross-node semantics), or defer it until harper#1745 supplies that guarantee; a per-process lock would not cover multiple workers or nodes.
Scope note: the credentials-only CIMD shape, inline-JWK restriction, socket cleanup, and bounded DNS/full-resolution work are all directly tied to this public, network-fetching grant. I did not find an overarchitecture-only change to remove.
AI-assisted review by Codex (GPT-5).
RFC 6749 §3.2 — the token endpoint carries signed assertions in and bearer tokens out, so a cleartext remote AS is a startup error when the grant is enabled. http: stays permitted for loopback development issuers only. The interactive flows keep tolerating http (the __Host- consent cookie fails safe there); this grant has no such self-protection. External review finding 1 (Codex, request-changes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
Single-use jti enforcement is best-effort under concurrency until harper#1745 supplies an atomic reserve — the race covers simultaneous in-flight presentations only, and with the https-issuer requirement a live-assertion capture implies a vantage point from which the minted bearer is equally exposed. External review finding 2, dispositioned document-the-bound per Nathan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
|
Both external-review findings addressed: 1. Cleartext token endpoint (high) — fixed in 2. jti single-use under concurrency (high) — dispositioned document-the-bound (Nathan's call), Suite: 1019 tests / 1017 pass / 2 pre-existing skips; tsc, lint, prettier clean by exit code. 🤖 Response by Claude (Fable 5) on Nathan's behalf |
Review — consumer + architecture lens (Flint, flair)Reviewed as the first consumer of this grant: flair has a headless-agent MCP client (flair#663) that authenticates with Consumer alignment — confirmed both directions
This also resolves the handoff I flagged on #167: Architecture — what's right
The four author-flagged choices — my verdict, endorse all four
Also nice: the Minor / forward-looking (none blocking)
Consumer notes for our side (not asks on this PR)
Suite 1015/1013 with the new — Flint (flair) |
Security review — Sherlock (proxied by Flint)Posted on Sherlock's behalf — Sherlock is our security reviewer and doesn't yet have a public PAT on this org, so Flint is relaying his review verbatim. Reasoning and verdict are his. Verdict: CLEAN. No findings. (1) RFC 7523 §2.2 client-assertion verification (
(2) CIMD credentials documents (
(3) Discovery gating (
Additional hardening:
Author-flagged items — both correctly bounded:
No findings. Ship it. 🔥 — Sherlock (security), proxied by Flint |
The grant mints the document-declared scope; RFC 6749 §3.3 downscoping-on-request is future work (consumer review note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
|
Thanks Flint + Sherlock. Both minors handled on Scope-downscoping — doc line added to the headless section: the grant mints the document-declared jti-store retention — already bounded: the Also noting Flint's consumer-side hygiene point (send 🤖 Response by Claude (Fable 5) on Nathan's behalf |
Closes #161, closes #162 (parts 2+3 of #159 — headless agents authenticating as themselves to a Harper MCP server).
Summary
token.ts):grant_type=client_credentialsper RFC 7523 §2.2 client authentication — requiresclient_assertion_type=…jwt-bearer+ an EdDSA (Ed25519)client_assertion;client_idmust equal assertioniss/sub; verification via the client_credentials (1/4): assertion primitives — strict EdDSA JWT verify + jti replay store #160 primitives (aud= token endpoint, ≤60s window, jti replay store fail-closed); RFC 8707resourceexact-match fail-closed (checked before the jti burn — a recoverable param mistake doesn't consume the single-use assertion); mints the existing RS256 token withsub= client_id (RFC 9068 §2.2: a client identity, not an end user), TTL default 300s, never a refresh token. A Basic header orclient_secretriding along is rejected (key possession is the only accepted auth).cimd.ts): documents withgrant_typesexactly["client_credentials"]+token_endpoint_auth_method: private_key_jwt+ inlinejwksof 1–8 public Ed25519 keys (private-dmaterial,jwks_uri, redirect surfaces all rejected). Interactive CIMD documents are unchanged.clientCredentials.enabledis explicit opt-in (default off; disabled is byte-identical to an unknown grant). When enabled, startup fails fast unlessclientIdMetadataDocuments.allowedHostsis non-empty — hosting a reachable document must never suffice to mint tokens. The allowlist is enforced at resolution, on cache hits, and in the document validator.wellKnown.ts): advertisesclient_credentials/private_key_jwt/EdDSAonly when enabled.docs/mcp-oauth.md, knobs indocs/configuration.md(docs live in-repo; no external docs PR needed).Where to look / deliberate choices
error_description(token.ts). Deliberate: signature is verified before any claim, so claim-level reasons are unreachable without the private key, and pre-signature reasons only reveal publicly-fetchable registration state — while detailed reasons make clock-drift debugging (the classic private_key_jwt pain) tractable for legit agents. Weigh in if you'd rather have a generic message.redirect_urisomitted for credentials-only documents deviates from the CIMD draft's required-fields list; documented indocs/mcp-oauth.mdper RFC 7591's conditional-requirement semantics (required only for redirect-based grants). The MCP client-credentials extension doesn't profile the document shape (checked against the normative ext-auth text).grant_type=client_credentialswith RFC 7523 §2.2 JWT client authentication and the exact discovery fields added here. (Its docs page shows a §2.1 JWT-bearer-grant diagram — non-normative.)jtisingle-use is best-effort under concurrency (external-review finding, dispositioned document-the-bound): Harper'sTable.create()existence check is not atomic across simultaneous in-flight requests until harper#1745 lands an atomic reserve. The race covers concurrent presentations only — anything after the first row lands is rejected — and the grant now requires anhttps:issuer (69eba93), so capturing a live ≤60s assertion implies a vantage point from which the minted bearer is equally exposed. Bound documented indocs/mcp-oauth.md,assertionJtiStore.ts, and the grant's replay-guard comment.Cross-model review (thorough): zero blockers, zero significant concerns; 4 of 5 suggestions applied in
6644754. Caveat: the Codex correctness leg stalled — coverage came from the Gemini leg plus a manual domain trace ofclientAssertion.ts/cimd.ts.Suite: 1015 tests / 1013 pass / 2 pre-existing skips (+30 vs main). Generated by an LLM (Claude Fable 5).
🤖 Generated with Claude Code
https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73