fix: MCP OAuth endpoints broken on Harper 5.1.x (well-known 404 + response envelope) by jcohen-hdb · Pull Request #133 · HarperFast/oauth

jcohen-hdb · 2026-06-24T01:04:40Z

Both bugs make the v2 MCP OAuth flow unusable on current Harper (5.1.x) — discovery 404s and DCR responses are malformed — so an MCP client (e.g. a Claude.ai custom connector) can't complete the flow. Found wiring up a real /mcp connector against @harperfast/oauth@2.0.0 on Harper 5.1.10.

Bug 1 — well-known endpoints 404 (`src/lib/mcp/wellKnown.ts`)

Handlers mount via server.http(handler, { urlPath }) and reject sub-paths by comparing req.pathname to the full exactPath. Harper 5.1.x passes the path relative to the mount (/ for an exact match), so the compare never matches → PRM / AS-metadata / JWKS all 404. Fix: accept the relative / and the absolute exactPath forms (req.pathname ?? req.url); sub-paths still fall through to 404.

Bug 2 — `{ status, body }` responses mis-serialized (`src/lib/resource.ts`)

OAuthResource returns { status, body } envelopes for non-200 results. Harper 5.1.x only honors status/headers when the returned object carries a headers field; a bare { status, body } is serialized whole with a default 200. Net effect: POST /oauth/mcp/register returned HTTP 200 with {"status":201,"body":{…}} (client_id not at top level) — non-conformant for RFC 7591 DCR clients. Fix: a toHttpResponse() boundary helper on get()/post() that normalizes { status, body } → { status, headers, body:<JSON string> }; plain bodies and redirects pass through untouched.

Validation

Unit: 667 tests, 0 failures (+9 new covering both paths); eslint + prettier clean.
End-to-end on Harper 5.1.10 (@harperfast/oauth serving a real /mcp connector): well-known PRM / AS-metadata / JWKS all 200; POST /oauth/mcp/register → 201 with top-level client_id; allowedRedirectUriHosts correctly rejects non-allowlisted hosts (400); existing /oauth/<provider>/login still 302 (no regression).

🤖 Generated with Claude Code

The MCP discovery documents are mounted with `server.http(handler, { urlPath })`, and the handler rejects sub-paths by comparing `req.pathname` to the full `exactPath`. On current Harper (5.1.x), `server.http({ urlPath })` passes the path RELATIVE to the mounted prefix — `/` for an exact match, `/sub` for a sub-path — so the full-path comparison never matches and every well-known endpoint (oauth-protected-resource, oauth-authorization-server, jwks.json) returns 404. MCP clients (e.g. Claude.ai) then can't discover the AS. Accept both forms: the relative `/` (current Harper) and the absolute `exactPath` (older builds / existing unit tests), falling back to `req.url` when `pathname` is absent. Sub-paths still fall through to 404. Verified end-to-end on Harper 5.1.10: all three well-known endpoints now 200. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

OAuthResource handlers return a `{ status, body }` envelope to signal non-200 responses (DCR 201, token/registration errors, 401/403/404). On current Harper (5.1.x), the REST layer only honors `status`/`headers` when the returned object carries a `headers` field; a bare `{ status, body }` is otherwise serialized *whole* as the response body with a default 200. So e.g. dynamic client registration returned HTTP 200 with `{"status":201,"body":{...client_id...}}` — the status was dropped and the envelope leaked into the payload, which breaks RFC 7591 / 8414 clients (the client_id isn't at the top level). Redirect responses worked only incidentally because they already carry `headers`. Add `toHttpResponse()` and apply it at the get()/post() boundary: `{ status, body }` envelopes become `{ status, headers, body: <JSON> }` (mirroring the well-known `jsonResponse` helper, and serializing explicitly so these OAuth/DCR responses are always JSON regardless of the client's Accept header). Plain bodies (no `status`) and header-bearing redirects (no `body`) pass through unchanged. The per-handler logic and its unit tests are untouched — only the HTTP boundary is normalized. Verified end-to-end on Harper 5.1.10: DCR now returns a clean 201 with `client_id` at the top level, and the `allowedRedirectUriHosts` allowlist correctly rejects disallowed hosts with a 400. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

gemini-code-assist · 2026-06-24T01:04:43Z

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

DCR validation failures returned a 400 with no log entry, making client registration failures (e.g. from Claude.ai's connector) impossible to debug. Log each registration attempt at info and each rejection at warn with the error/description and the requested redirect_uris. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The previous DCR logging passed an object as the second arg, which routed the entries to stdout/system.log instead of the structured application log. Match this file's existing convention (single template-literal message) so the logs appear via read_log / the app log like everything else. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The scope-provided logger routes to system.log; Harper's global logger (the one apps import from 'harper') writes to the structured app log read by read_log. Use it for DCR request/rejection observability. Test mocks updated to export logger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

heskew · 2026-06-27T00:20:07Z

I walked through the Harper 5.1 behavior this targets. The functional fixes look right to me: server.http({ urlPath }) does pass the path relative to the mount, so accepting pathname === "/" fixes the .well-known 404 while still falling through on subpaths; and Harper REST only honors returned status as an HTTP response when headers is present, so normalizing { status, body } fixes DCR/token/error responses without changing redirects or HTML responses.

Only blocker I found is formatting: .bun/preload.js fails npm run format:check, and that is what is making both Node CI jobs red. npm run format:write should clear it.

I also ran npm test locally on the PR branch: 667 tests, 665 pass, 2 skipped.

The sole red CI job on this PR was prettier --check failing on .bun/preload.js; format:write expands the logger-stub one-liner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The relative-urlPath fix makes the BARE PRM (/.well-known/oauth-protected-resource) work on Harper 5.1.x, but MCP clients (Claude.ai) build the PRM URL by RFC 9728 §3.1 path-insertion and fetch the resource-path-appended form (/.well-known/oauth-protected-resource/<resource-path>, e.g. .../mcp). The handler mounts at the bare urlPath, so that arrives as relative "/mcp" and previously fell through to 404 — leaving discovery broken for the real client even after the bare fix. Make the PRM handler resource-path-aware: accept the appended form (relative "<resource-path>" or absolute "exactPath + <resource-path>"), deriving the path from resolveResource(). Tight + additive — PRM only (AS-metadata/JWKS stay exact-match), and any other sub-path still 404s. The PRM document is unauthenticated public discovery (RFC 9728); this changes routing only, not the auth boundary. Tests cover relative/absolute appended forms, a configured resource path, and non-resource sub-path + AS-metadata fall-through. Matches the path-aware PRM jcohen validated end-to-end against live Claude.ai. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

heskew · 2026-06-29T12:17:07Z

Picking this up to get it landed (it's the prerequisite for Stage 5 / #134 — withMCPAuth's discovery loop sits on these fixes). Pushed two commits on top of @jcohen-hdb's work:

e66268a — format .bun/preload.js to clear the sole red CI job (prettier --check).
b269c59 — serve the RFC 9728 §3.1 path-appended PRM. The relative-urlPath fix makes the bare /.well-known/oauth-protected-resource work on 5.1.x, but MCP clients (Claude.ai) build the PRM URL by path-insertion and fetch the resource-path-appended form (/.well-known/oauth-protected-resource/mcp) — which arrives as relative /mcp and was still 404ing. The PRM handler is now resource-path-aware (PRM only; AS-metadata/JWKS stay exact-match; everything else still 404s). This matches the path-aware PRM @jcohen-hdb validated end-to-end against live Claude.ai. Routing-only change to an unauthenticated public discovery doc — the auth boundary is untouched.

Cross-model review (Codex + Harper-domain pass; Gemini skipped — exfiltration-blocked, the CI bot covers it): no blockers. One significant finding — a trailing-slash configured mcp.resource (https://host/mcp/) would 404 the appended PRM — is fixed (both-sides trailing-slash normalization, exact-after-normalize) with a regression test.

Tests: 672 total, 670 pass, 2 skipped. Build/lint/format clean.

The remaining open item is the challenge-emit side (withMCPAuth pointing resource_metadata at the appended form) — that's Stage 5 / #134's territory and lands there once this merges.

The DCR-request info log hoisted JSON.stringify(body?.redirect_uris) into a const, so it ran on every registration even when the info level is suppressed (e.g. log level warn). Harper's logger omits a level method when it's below the configured level, and an optional call ?.() short-circuits without evaluating its arguments — so formatting kept INSIDE the logger call only runs when the message is emitted. Inline the stringify into the info and warn messages (output unchanged) and add a comment so it isn't re-hoisted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

github-actions · 2026-06-29T13:22:28Z

Reviewed; no blockers found. The PR successfully addresses several critical integration and observability gaps for MCP support, specifically regarding RFC 9728 compliance and Harper-specific response handling.

Suggestions (non-blocking)

src/lib/resource.ts:25 — The toHttpResponse helper is a robust fix for status-code loss in Harper; consider promoting this pattern to a shared utility if other plugins in the ecosystem face similar "envelope leak" issues.
src/lib/mcp/dcr.ts:245 — The use of lazy log formatting is excellent for performance; ensure this "don't-hoist-stringify" rule is captured in CLAUDE.md to prevent future regressions during refactoring.

claude · 2026-06-29T13:32:49Z

Reviewed; no blockers found.

Now that #133 serves the RFC 9728 §3.1 path-appended PRM (/.well-known/oauth-protected-resource/<resource-path>), close the emit half: withMCPAuth's WWW-Authenticate challenge must point at that same URL, not the bare host-root form, so a client honoring resource_metadata verbatim fetches a document the server actually answers. - wellKnown.ts: add+export protectedResourceMetadataUrl(req, cfg) — the canonical PRM URL for the configured resource (origin + well-known + resource path; bare for a root resource). Single source of truth shared with the serving side. - withMCPAuth.ts: build the challenge from protectedResourceMetadataUrl (guarded so the deny path never throws). Drop the PRM_PATH import. - Tests: expected challenge is now the resource-derived path-appended URL; add a root-resource (bare) case. Integration test asserts the appended form. - Docs: challenge-URL description updated to the path-aware form; add a "using withMCPAuth from a different component than the plugin" section (inject getConfig; keys come from the shared oauth DB) per jcohen's field report. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* MCP OAuth Stage 5: withMCPAuth bearer-token guard (#95) Plugin-provided guard for app-owned MCP routes. Validates the `Authorization: Bearer` access token (RS256 signature against the published JWKS selected by `kid`, `exp`/`nbf`, and audience binding to `mcp.resource` per RFC 8707) and, on any failure, returns the spec-mandated `401 + WWW-Authenticate: Bearer resource_metadata="..."` (RFC 9728) that closes the MCP discovery loop. On success it attaches `request.mcp = { sub, client_id, aud, scope }` and invokes the wrapped handler verbatim. Fails closed (MCP disabled / no token / no keys / malformed). Header-only token extraction (RFC 6750); query-string tokens are ignored. Registration model (the load-bearing detail): Harper core auth is a default-group middleware that 401s a non-Harper bearer token with `WWW-Authenticate: Basic`, breaking discovery. The wrapper supports both registrations, documented with urlPath as primary: - urlPath subroute (recommended) — routed dispatch isolates the chain, so core auth never runs for the route (same as /.well-known/*). - default group with `{ path }` + `before: 'authentication'` — the wrapper scopes to its path and runs ahead of core auth (static.ts precedent). Note: the #95 example registration is wrong for this Harper version (server.http takes (listener, options)); docs correct it. - tokenIssuer.ts: add verifyAccessTokenWithKeySet (kid selection; unknown kid fails; no-kid uses the sole key; RS256-pinned; aud+iss enforced). - wellKnown.ts: export PRM_PATH so the challenge URL stays in sync. - types.ts: add MCPRequestClaims + Request.mcp. - Exports from src/lib/mcp/index.ts and the top-level src/index.ts. - Unit tests (withMCPAuth + verifyAccessTokenWithKeySet) and an integration test asserting the Bearer challenge survives core auth for both registration models. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(mcp): point withMCPAuth challenge at the path-appended PRM Now that #133 serves the RFC 9728 §3.1 path-appended PRM (/.well-known/oauth-protected-resource/<resource-path>), close the emit half: withMCPAuth's WWW-Authenticate challenge must point at that same URL, not the bare host-root form, so a client honoring resource_metadata verbatim fetches a document the server actually answers. - wellKnown.ts: add+export protectedResourceMetadataUrl(req, cfg) — the canonical PRM URL for the configured resource (origin + well-known + resource path; bare for a root resource). Single source of truth shared with the serving side. - withMCPAuth.ts: build the challenge from protectedResourceMetadataUrl (guarded so the deny path never throws). Drop the PRM_PATH import. - Tests: expected challenge is now the resource-derived path-appended URL; add a root-resource (bare) case. Integration test asserts the appended form. - Docs: challenge-URL description updated to the path-aware form; add a "using withMCPAuth from a different component than the plugin" section (inject getConfig; keys come from the shared oauth DB) per jcohen's field report. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(mcp): drop now-redundant (request as any) casts in withMCPAuth `request.pathname` is declared on the Request interface as of #138 (merged), and `request.mcp` is declared in this PR — so the `(request as any)` casts reading/writing them are no longer needed. Use the typed fields directly. (The remaining `request as any` casts at the resolveResource/resolveIssuer/ protectedResourceMetadataUrl call sites bridge to wellKnown's HarperRequest shape — a separate type-unification concern, left as-is.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(mcp): address gemini review — header-safe challenge, path limit, CORS Resolves the two blockers + two suggestions from the gemini-review bot on this PR: - Header-parameter injection (blocker): `protectedResourceMetadataUrl`'s fallback could interpolate a raw, client-controlled Host into the quoted `resource_metadata="..."` challenge param. Every branch now normalizes through `new URL().origin` (which rejects/encodes a `"`/control char), with a host-less relative `PRM_PATH` as the final always-safe fallback. - Path-length limit (blocker): enforce the repo's ≤2048-char path invariant (CLAUDE.md / OAuthResource.parseRoute) in withMCPAuth too, since it can be registered outermost and bypasses parseRoute. Fail closed before token work. - CORS on the deny 401 (suggestion): add Access-Control-Allow-Origin + expose WWW-Authenticate so browser MCP clients can read the challenge cross-origin (parity with the well-known discovery docs). - Array Host header (suggestion): resolveIssuer takes the first value when `headers.host` is an array. Tests added for each. Full suite 703 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(mcp): clear OAuthResource.mcpConfig when a live reload drops all providers withMCPAuth's default config getter (and the well-known handlers) read the OAuthResource.mcpConfig static. That static is written only by configure(), which runs only in updateConfiguration()'s >=1-provider branch. The zero-provider branch swapped in the 503 error resource but left the previous mcpConfig in place — so a live reload that removed all providers left the MCP surface verifying tokens / serving discovery against stale, no-longer-valid config. Fail closed by clearing the static in the zero-provider branch. Adds a regression test that reproduces the repro (enabled + provider, then reload to providers: {}) and asserts mcpConfig is cleared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

jcohen-hdb and others added 2 commits June 23, 2026 18:44

jcohen-hdb requested a review from heskew June 24, 2026 01:06

jcohen-hdb and others added 3 commits June 23, 2026 19:45

jcohen-hdb mentioned this pull request Jun 24, 2026

MCP OAuth Stage 5: withMCPAuth bearer-token guard (#95) #134

Merged

heskew and others added 2 commits June 28, 2026 22:52

chore: format .bun/preload.js to satisfy format:check

e66268a

The sole red CI job on this PR was prettier --check failing on .bun/preload.js; format:write expands the logger-stub one-liner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

heskew added claude-review Trusted-member gesture: spawn Claude review on a bot-authored PR. gemini-review Trusted-member gesture: spawn Gemini review on a bot-authored PR. labels Jun 29, 2026

heskew added claude-review Trusted-member gesture: spawn Claude review on a bot-authored PR. and removed claude-review Trusted-member gesture: spawn Claude review on a bot-authored PR. labels Jun 29, 2026

heskew merged commit 58d5f4b into main Jun 29, 2026
20 of 22 checks passed

heskew deleted the fix/harper-5.1-mcp-response-contract branch June 29, 2026 13:54

heskew mentioned this pull request Jun 30, 2026

Validate mcp.issuer is a full origin (with scheme) at config load #139

Closed

This was referenced Jul 2, 2026

MCP OAuth v1.1: remaining 2026-07-28 spec forward-work (SEP-2350 scope step-up) #156

Open

MCP OAuth v1.x: 2026-07-28 RC spec changes #100

Closed

Navigation Menu

Uh oh!

fix: MCP OAuth endpoints broken on Harper 5.1.x (well-known 404 + response envelope) - #133

fix: MCP OAuth endpoints broken on Harper 5.1.x (well-known 404 + response envelope)#133
heskew merged 8 commits into
mainfrom
fix/harper-5.1-mcp-response-contract

jcohen-hdb commented Jun 24, 2026

Uh oh!

gemini-code-assist Bot commented Jun 24, 2026

Uh oh!

heskew commented Jun 27, 2026

Uh oh!

heskew commented Jun 29, 2026

Uh oh!

github-actions Bot commented Jun 29, 2026

Uh oh!

claude Bot commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jcohen-hdb commented Jun 24, 2026

Bug 1 — well-known endpoints 404 (src/lib/mcp/wellKnown.ts)

Bug 2 — { status, body } responses mis-serialized (src/lib/resource.ts)

Validation

Uh oh!

gemini-code-assist Bot commented Jun 24, 2026

Uh oh!

heskew commented Jun 27, 2026

Uh oh!

heskew commented Jun 29, 2026

Uh oh!

github-actions Bot commented Jun 29, 2026

Suggestions (non-blocking)

Uh oh!

claude Bot commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bug 1 — well-known endpoints 404 (`src/lib/mcp/wellKnown.ts`)

Bug 2 — `{ status, body }` responses mis-serialized (`src/lib/resource.ts`)