Skip to content

adding minimal lifecycle hooks to allow for scenarios like adding/mod… - #3

Merged
heskew merged 4 commits into
mainfrom
oauth-lifecycle-hooks
Oct 28, 2025
Merged

adding minimal lifecycle hooks to allow for scenarios like adding/mod…#3
heskew merged 4 commits into
mainfrom
oauth-lifecycle-hooks

Conversation

@heskew

@heskew heskew commented Oct 27, 2025

Copy link
Copy Markdown
Member

…ifying user records on authn

@heskew
heskew requested a review from Copilot October 28, 2025 21:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements comprehensive OAuth lifecycle management with automatic token refresh, lifecycle hooks, and improved session handling. The plugin now automatically validates and refreshes OAuth tokens on every HTTP request using middleware, with hooks for user provisioning and custom logic.

  • Adds automatic token refresh at 80% of token lifetime via HTTP middleware
  • Implements lifecycle hooks system (onLogin, onLogout, onTokenRefresh) for extensibility
  • Refactors session structure to store OAuth metadata with expiration tracking
  • Adds hot-reloading support for configuration changes with concurrency protection

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/types.ts Adds OAuthHooks interface, OAuthSessionMetadata type, and server middleware type definitions
src/lib/hookManager.ts New HookManager class for registering and executing lifecycle hooks
src/lib/sessionValidator.ts New session validation logic with automatic token refresh
src/lib/withOAuthValidation.ts New resource wrapper for OAuth validation (currently unused in tests)
src/lib/resource.ts Updates resource to use hookManager, exposes providers, changes logout endpoint
src/lib/handlers.ts Refactors handlers to use new session structure, adds clearOAuthSession utility
src/index.ts Adds middleware registration, hook management, and async config updates with concurrency control
test/* Updates all tests to use mockHookManager and new session structure
package.json Adds dist/ to eslint ignore pattern
assets/test.html Updates logout to use generic /oauth/logout endpoint
README.md Extensive documentation updates for new features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/sessionValidator.ts
Comment thread src/lib/handlers.ts
Comment thread src/lib/hookManager.ts
Comment thread src/lib/sessionValidator.ts Outdated
heskew and others added 3 commits October 28, 2025 15:00
little more defensive check on hooks

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
just updating with what's potentially changed (updated oauth session metadata)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@heskew
heskew merged commit c4aa859 into main Oct 28, 2025
@heskew heskew linked an issue Oct 28, 2025 that may be closed by this pull request
@heskew
heskew deleted the oauth-lifecycle-hooks branch October 30, 2025 15:38
heskew added a commit that referenced this pull request Jul 10, 2026
…flow consent cookie, DNS bounding, config hardening

Second external-review batch on top of the consent-binding/SSRF work.

Consent cookie (Codex #1 sibling-injection High, #7 concurrent-flows Medium):
- Switch the consent cookie to a per-flow __Host--prefixed name
  (__Host-mcp_consent_<flowId>). __Host- forbids a Domain attribute, so a
  sibling origin can no longer plant a parent-domain cookie to forge the
  binding (SameSite=Lax doesn't stop siblings — they're same-site). The
  per-flow id (carried in confirm + upstream state) lets parallel tabs run
  concurrent flows without clobbering each other's binding.

Callback ordering (Codex #5 Medium):
- Verify the browser binding BEFORE exchangeCodeForToken and the onLogin hook,
  so a mismatched (self-approved) flow triggers no upstream exchange and no
  provisioning side-effects. Regression asserts neither runs on mismatch.

SSRF (Codex #3 rebind High, #2 threadpool High, #4 special-use Medium):
- Pinned-connect: fetch via https.request with a custom lookup that connects to
  the exact address the gate validated, while keeping the hostname for TLS SNI +
  cert verification — closes the DNS-rebinding TOCTOU. (No undici dep: undici
  isn't importable here; https.request also gives no-redirect-follow for free.)
- Bound concurrent DNS resolutions with a permit released only when the raw
  (uncancellable) getaddrinfo settles, so a flood of blackholed-DNS client_ids
  can't pin the libuv pool; fast-reject when saturated; dedup concurrent
  resolutions of the same client_id into one fetch.
- Classify against the full IANA IPv4/IPv6 special-purpose registries
  (192.0.2/24, 198.51.100/24, 203.0.113/24, 192.0.0/24, 192.88.99/24, AS112/AMT,
  and in-2000::/3 Teredo/ORCHID/documentation), table-driven.

Config (Codex #6 Medium):
- Normalize the mcp block at load: coerce documented boolean strings
  (env-expanded "false" no longer leaves a security switch truthy) and require
  allowedHosts to be an array of exact lowercased hostnames (a scalar is
  wrapped, not substring-matched; non-strings rejected).

Deferred (agreed): the pinnedHttpsFetch path is production-only (tests stub the
fetch seam and exercise SSRF via the DNS seam); its lookup is a thin pass-through
of the pre-validated addresses.

Tests 938/936 pass (2 pre-existing skips); docs updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jul 10, 2026
…sent interstitial (#166) (#167)

* feat: Client ID Metadata Documents (CIMD) with SSRF-guarded resolution and consent interstitial (#166)

Implements the MCP authorization spec's CIMD support: when a client_id is an HTTPS
URL with a non-root path, the AS fetches it as a JSON metadata document instead of
doing a DCR lookup.

Core changes:
- New `src/lib/mcp/cimd.ts`: `isCimdClientId`, `resolveCimdClient`, `resolveClient`.
  SSRF guards via DNS pre-flight (all A/AAAA records checked against private/loopback
  ranges), IP-literal rejection, no-redirect fetch, 5 s timeout, 64 KB cap.
  Per-process cache with `Cache-Control: max-age`-based TTL clamped to [60 s, 86400 s];
  negative-cache on client errors (60 s), not on server errors.
- New `src/lib/mcp/clientValidator.ts`: shared validators extracted from dcr.ts
  (`validateRedirectUri`, `validateStringArray`, `validateGrantTypes`, etc.).
- `src/lib/mcp/authorize.ts`: `resolveClient` replaces direct `MCPClientStore` lookup.
  `escapeHtml` and `buildInterstitialPage` for the CIMD consent page.
  `handleAuthorize` returns 200 HTML for CIMD clients; 302 for stored/DCR clients.
  New `handleAuthorizeConfirm` for `POST /oauth/mcp/confirm`: verify + consume
  one-time token, validate `_confirm` marker, redirect to upstream IdP.
- `src/lib/mcp/wellKnown.ts`: advertises `client_id_metadata_document_supported: true`
  when CIMD is enabled (default on).
- `src/lib/mcp/token.ts`: `authenticateClient` uses `resolveClient`; handles
  `CimdClientError` as `invalid_client`.
- `src/lib/mcp/index.ts` + `src/lib/resource.ts`: route `POST /oauth/mcp/confirm`
  to `handleAuthorizeConfirm`; thread `providers` registry into `handleMCPPost`.
- `src/types.ts`: `MCPClientIdMetadataDocumentsConfig`, `MCPConfig.clientIdMetadataDocuments`,
  `MCPClientRecord._cimd`, `MCPClientMetadata.jwks/jwks_uri`.

Tests (50 new passing):
- `test/lib/mcp/cimd.test.js`: isCimdClientId shape checks, SSRF DNS gate, allowedHosts
  policy, document validation, cache TTL, resolveClient routing.
- `test/lib/mcp/authorize.test.js`: escapeHtml (XSS vectors), buildInterstitialPage
  (loopback warning, token binding, XSS), handleAuthorize CIMD path (200 HTML),
  handleAuthorizeConfirm (valid token → 302, single-use, expired, missing _confirm).
- `test/lib/mcp/wellKnown.test.js`: CIMD flag present when enabled, absent when disabled.

Docs: CIMD section in mcp-oauth.md, four config rows in configuration.md.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(cimd): separate document-host policy from redirect-host policy

clientIdMetadataDocuments.allowedHosts governs which hosts may SERVE
metadata documents; redirect URIs are a different policy and now validate
against dynamicClientRegistration.allowedRedirectUriHosts (same rules as
DCR clients). A trusted vendor whose document declares redirect targets
on another host is no longer wrongly rejected. Regression tests added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(cimd): address external security review — consent browser binding, token purpose, SSRF gate, cache and fetch hardening

Consent flow (the two flow-level blockers):
- Bind the CIMD consent to the approving browser: the interstitial sets an
  HttpOnly/Secure/SameSite=Lax nonce cookie whose SHA-256 travels inside the
  confirm token and upstream state; POST /oauth/mcp/confirm and the OAuth
  callback both require a hash match before proceeding (new
  src/lib/mcp/consentBinding.ts). A malicious client can no longer
  self-approve the interstitial and hand the victim the upstream IdP URL.
- Enforce token purpose at the callback: a confirm token presented as
  upstream OAuth state is rejected like an invalid token (it previously
  passed the mcp/providerName checks and skipped consent entirely).

SSRF DNS gate:
- IPv4: also reject 0/8, 100.64/10 (CGNAT), 198.18/15, 224/4+; malformed
  input fails closed.
- IPv6: real parser (:: compression, embedded dotted-quad, zone index);
  allow only global unicast 2000::/3, with v4-mapped addresses classified
  by their embedded IPv4 address. Closes the ::, non-canonical loopback,
  and hex-form v4-mapped bypasses.
- All gate rejections (didn't resolve / blocked address) return one generic
  invalid_client message; details are logged server-side only, so callers
  can't probe the server's internal DNS view.
- Reject dot path segments (raw or percent-encoded) and non-lowercase
  scheme spellings in CIMD client_ids before URL normalization erases them.

Fetch path:
- Only 200 OK is accepted (404/500 JSON no longer resolves as a client).
- One deadline across DNS, connect, headers, and body read — a trickling
  body can no longer hold connections open past the timeout.
- fetchTimeoutMs/maxDocumentBytes are coerced to finite positive numbers;
  NaN/Infinity/garbage fall back to defaults instead of failing open.

Cache:
- LRU-bounded to 1000 entries (keys are attacker-chosen input).
- Failures are never cached (CIMD draft forbids caching errors/invalid
  documents); negative caching removed.
- Cache-Control no-store/no-cache floor at the 60 s DoS floor.
- Cached records revalidate against the live allowedRedirectUriHosts on
  every hit, so tightening the policy takes effect immediately.

Interstitial:
- Served with X-Frame-Options: DENY, CSP frame-ancestors 'none', and
  Cache-Control: no-store (page carries the single-use confirm token).
- Displays the authoritative client_id hostname; client_uri is labelled
  unverified; unparseable redirect_uri degrades instead of throwing.

Plus error-cause chaining and server-side logging on swallowed catch paths
(confirm verify, token client lookup). 52 new/updated tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(cimd): decode IPv4-in-IPv6 transition forms and fail closed on unknown DNS family

Follow-up from the cross-model review pass on 195cf21.

- isPrivateIpv6 now decodes the IPv4 embedded in 6to4 (2002::/16) and ISATAP
  and classifies it via isPrivateIpv4, and rejects Teredo (2001:0000::/32)
  outright. These transition forms sit inside the 2000::/3 global-unicast
  allow but can target a private IPv4 — previously a 6to4/ISATAP address
  wrapping 10/8 or 127/8 was allowed.
- checkHostSsrf fails closed on any DNS address family other than 4/6 rather
  than skipping both range checks (defense-in-depth; dns.lookup only returns
  4/6 today).
- Tests for 6to4/ISATAP/Teredo private targets, a public 6to4 pass-through,
  and the unknown-family reject.

In-flight fetch dedup (thundering herd on an uncached attacker URL) was raised
as a suggestion and is deferred to rate limiting (#163), bounded meanwhile by
the 64 KB / 5 s caps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(cimd): address second Codex review — pinned-connect, __Host- per-flow consent cookie, DNS bounding, config hardening

Second external-review batch on top of the consent-binding/SSRF work.

Consent cookie (Codex #1 sibling-injection High, #7 concurrent-flows Medium):
- Switch the consent cookie to a per-flow __Host--prefixed name
  (__Host-mcp_consent_<flowId>). __Host- forbids a Domain attribute, so a
  sibling origin can no longer plant a parent-domain cookie to forge the
  binding (SameSite=Lax doesn't stop siblings — they're same-site). The
  per-flow id (carried in confirm + upstream state) lets parallel tabs run
  concurrent flows without clobbering each other's binding.

Callback ordering (Codex #5 Medium):
- Verify the browser binding BEFORE exchangeCodeForToken and the onLogin hook,
  so a mismatched (self-approved) flow triggers no upstream exchange and no
  provisioning side-effects. Regression asserts neither runs on mismatch.

SSRF (Codex #3 rebind High, #2 threadpool High, #4 special-use Medium):
- Pinned-connect: fetch via https.request with a custom lookup that connects to
  the exact address the gate validated, while keeping the hostname for TLS SNI +
  cert verification — closes the DNS-rebinding TOCTOU. (No undici dep: undici
  isn't importable here; https.request also gives no-redirect-follow for free.)
- Bound concurrent DNS resolutions with a permit released only when the raw
  (uncancellable) getaddrinfo settles, so a flood of blackholed-DNS client_ids
  can't pin the libuv pool; fast-reject when saturated; dedup concurrent
  resolutions of the same client_id into one fetch.
- Classify against the full IANA IPv4/IPv6 special-purpose registries
  (192.0.2/24, 198.51.100/24, 203.0.113/24, 192.0.0/24, 192.88.99/24, AS112/AMT,
  and in-2000::/3 Teredo/ORCHID/documentation), table-driven.

Config (Codex #6 Medium):
- Normalize the mcp block at load: coerce documented boolean strings
  (env-expanded "false" no longer leaves a security switch truthy) and require
  allowedHosts to be an array of exact lowercased hostnames (a scalar is
  wrapped, not substring-matched; non-strings rejected).

Deferred (agreed): the pinnedHttpsFetch path is production-only (tests stub the
fetch seam and exercise SSRF via the DNS seam); its lookup is a thin pass-through
of the pre-validated addresses.

Tests 938/936 pass (2 pre-existing skips); docs updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(cimd): drop redundant Buffer.from in Buffer.concat

Buffer.concat accepts Uint8Array[] directly (gemini review nit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(cimd): reject query strings in CIMD client_ids

The draft (§3) says client_id URLs SHOULD NOT include a query string;
enforce it as part of the strict URL-shape profile — a dynamic server
could otherwise mint unlimited exact-match client_id aliases of one
document by echoing query variants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* fix(cimd): tear down rejected connections, bound total resolution concurrency

Third external review batch:
- Abort the pinned connection when a response is rejected after headers
  (non-200, non-JSON, oversize) — the deadline timer is cleared on exit,
  so a hostile endpoint could otherwise hold rejected sockets open
  indefinitely.
- Bound TOTAL concurrent CIMD resolutions (DNS + connect + body) at 16,
  fast-rejecting past the cap; the DNS permit alone released too early
  to stop unique client_ids fanning out into unbounded HTTPS work.
- Drop MAX_CONCURRENT_DNS from 8 to 2 — below the default 4-thread
  libuv pool, so blackholed lookups can never starve fs/crypto users.
- Remove the jwks/jwks_uri carry-through: no consumer or validation
  until #159, which will add the plumbing alongside both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* chore(cimd): defense-in-depth minors from external agent review

- Add 224/4 multicast to the IPv4 special-use table so the table alone
  is complete (the a >= 224 early return remains as a fast path).
- Document that CIMD interactive authorization requires HTTPS — the
  __Host-/Secure consent cookie is silently dropped on plain-HTTP
  origins.
- Note readConsentNonce's base64url dependency and first-match
  semantics for future readers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* docs(cimd): spell out 224/4 multicast + 240/4 reserved in the SSRF header

Replace the cryptic 224/4+ shorthand in the module-header range list
(gemini nit).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
heskew added a commit that referenced this pull request Jul 11, 2026
Codex review (#171 request-changes):
- #1 (medium): the issuance limiter debited a bucket keyed by the
  submitted client_id BEFORE verification. Since CIMD client_ids are
  public URLs, any caller could flood a known agent's URL with a bogus
  assertion and 429 the real agent before its valid assertion was
  checked. Move the limiter to AFTER proof-of-possession, keyed by the
  verified client_id. Pre-auth work stays bounded by the per-URL CIMD
  fetch limiter + resolution/DNS concurrency caps (signature verify is
  CPU-only, no jti burn). Regression test: forged assertions for a
  victim's client_id can't drain its quota.
- #2 (medium): the LRU map retained raw keys, so maxKeys bounded entry
  COUNT not bytes. Store a SHA-256 fingerprint as the bucket key; memory
  is now maxKeys x constant regardless of key length. Long-unique-key
  flood test added.
- #3 (low): fractional rates already clamped in ad30fb2
  (capacity = max(1, rate), fractional refill preserved).

Also folds in two gemini nits: consolidated the redundant buckets.set in
tryTake, and the client_id length cap (ad30fb2) bounds pre-hash input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73
heskew added a commit that referenced this pull request Jul 11, 2026
…mpts (#163) (#171)

* feat(mcp): rate-limit client_credentials issuance and CIMD fetch attempts (#163)

Closes the last leg of #159 (req 5, defense-in-depth):

- New per-node in-memory token-bucket module (rateLimit.ts):
  continuous refill, injected clock, LRU-bounded key space
  (keys are attacker-chosen client_ids/URLs). Per-node by design —
  Harper replication makes a shared counter table a hot-write
  anti-pattern, and the assertion replay guard + 60s exp window
  already bound cross-node abuse.
- Grant limiter: mcp.clientCredentials.rateLimit requests/min per
  client_id (default 30; false/0 disables), checked BEFORE
  resolveClient so an over-limit client triggers no CIMD fetch or
  crypto work. Over-limit: 429 + error "slow_down" + Retry-After.
- CIMD fetch limiter: fixed 10 attempts/min per client_id URL at the
  resolution layer (post-cache, post-dedup — only actual fetch
  attempts consume), closing the bad-document fetch-amplification
  deferral from #167. Both #163-deferral comments updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* refactor(cimd): check concurrency cap before spending a fetch-limiter token; doc the per-thread limit ceiling

Cross-model review (domain pass) follow-ups:
- Move the MAX_CONCURRENT_RESOLUTIONS check ahead of the per-URL
  fetch-limiter take so a capacity reject no longer consumes a token
  without a fetch — makes 'only actual fetch attempts consume' literally
  true.
- Document that the per-node token buckets are in fact per worker
  thread, so N threads means an N× effective ceiling (inherited from the
  existing per-thread CIMD cache/concurrency design). Intentional for a
  defense-in-depth control; docs now say so rather than implying a hard
  node-wide cap.

Both Gemini findings (LRU-reset, per-key 'bypass') were adjudicated
by-design/noise and need no change — see PR description.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* fix(mcp): clamp sub-1 bucket capacity and cap client_id length

Gemini review follow-ups on #171:
- createRateLimiter clamps capacity to >= 1. A configured rate below
  1/min gave a burst ceiling under the 1 token a take needs, so the
  bucket could never admit anyone (worse than the reported 'first
  request blocked' — it was every request). Refill rate is untouched, so
  a sub-1/min limit still means one request then one per 60/rate seconds.
- Cap client_id at 2048 chars before it becomes a rate-limiter map key
  (attacker-chosen, retained up to maxKeys). Same defense-in-depth
  family as the repo's request-path and assertion-length caps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* fix(mcp): move issuance limiter post-auth; fingerprint limiter keys

Codex review (#171 request-changes):
- #1 (medium): the issuance limiter debited a bucket keyed by the
  submitted client_id BEFORE verification. Since CIMD client_ids are
  public URLs, any caller could flood a known agent's URL with a bogus
  assertion and 429 the real agent before its valid assertion was
  checked. Move the limiter to AFTER proof-of-possession, keyed by the
  verified client_id. Pre-auth work stays bounded by the per-URL CIMD
  fetch limiter + resolution/DNS concurrency caps (signature verify is
  CPU-only, no jti burn). Regression test: forged assertions for a
  victim's client_id can't drain its quota.
- #2 (medium): the LRU map retained raw keys, so maxKeys bounded entry
  COUNT not bytes. Store a SHA-256 fingerprint as the bucket key; memory
  is now maxKeys x constant regardless of key length. Long-unique-key
  flood test added.
- #3 (low): fractional rates already clamped in ad30fb2
  (capacity = max(1, rate), fractional refill preserved).

Also folds in two gemini nits: consolidated the redundant buckets.set in
tryTake, and the client_id length cap (ad30fb2) bounds pre-hash input.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* docs(mcp): correct the rate-limiter ordering claim after the post-auth move

The doc still said issuance was limited 'before any client resolution or
crypto work' — stale after 523ce12 moved the limiter to after assertion
verification. Rewrite to describe the actual (and safer) ordering:
debited post-verification so bogus assertions can't drain a real agent's
quota, with pre-auth work bounded by the CIMD fetch limiter + concurrency
caps. Claude review nit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* feat(mcp): surface 429 + Retry-After on CIMD throttles; centralize client_id cap

Two gemini nits on #171:
- The CIMD fetch rate limit now throws slow_down + statusCode 429 +
  retryAfterSeconds, and both the token endpoint and the authorize
  handler emit that status with a Retry-After header (via a shared
  cimdErrorResponse helper on the token side, widened ErrorJSON on the
  authorize side) — mirroring the issuance limiter instead of the old
  401 temporarily_unavailable that misleadingly read as an auth failure.
  The concurrency cap now also returns 429 (kept its temporarily_unavailable
  code — server-busy, not client-too-fast).
- MAX_CLIENT_ID_LENGTH moved to cimd.ts and exported; resolveClient now
  rejects an over-length client_id (unknown-client null) before it
  becomes a fetch-limiter key, and token.ts imports the shared constant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* fix(mcp): cap Retry-After; cover the authorize-endpoint 429 branch

Two more gemini nits on #171:
- Clamp retryAfterSeconds at 2,147,483 (int32-second max) so a tiny
  configured rate can't advertise a multi-year backoff.
- Add an authorize.test.js case that trips the CIMD fetch limiter and
  asserts handleAuthorize returns 429 slow_down + Retry-After — the 429
  status-selection and header branch added in b221570 were only exercised
  at the token endpoint.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

* fix(mcp): correct Retry-After comment; guard refill rate in the limiter

Two gemini nits on #171:
- The MAX_RETRY_AFTER_SECONDS comment said ~2.1e9 s; the value is
  2,147,483 s (~2.1e6, ≈24.8 days — int32-max ms as whole seconds).
- Guard the utility against a non-finite/non-positive refillPerMinute
  (would make the bucket never refill and divide the retry-after math by
  zero/negative): fall back to the ≥1 capacity so the limiter stays
  well-defined even if a future caller misconfigures it. Test added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SerGP6Am3xz2CKyPgbRc73

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

Need some mechanism to verify the session and refresh tokens

2 participants