mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 18:13:26 +00:00
xarmian
69e471db8f
* fix(oauth): default to canonical audience when client omits RFC 8707 resource= (TASK-951)
Real MCP clients (Claude Desktop, Cursor as of 2026-05) don't send the
RFC 8707 `resource` parameter on /oauth/authorize at all. Before this
fix, translateResourceToAudience only translated resource→audience
when resource= was present, so empty-resource requests reached
fosite's audienceMatchingStrategy with an empty needle and got
rejected with "resource parameter is required (RFC 8707)". fosite
then redirected to the client's redirect_uri with
?error=invalid_request&error_description=..., and Claude's
backend callback failed with the pydantic envelope "code: Field required"
(because no `code` parameter was in the redirect query).
RFC 8707 §2 marks the resource parameter OPTIONAL; servers with a
single canonical audience are expected to default to it. pad's OAuth
server has exactly one canonical audience by construction
(cfg.MCPPublicURL + "/mcp"), so the right policy is to inject
canonical when the client sends neither resource= nor audience=.
Now translateResourceToAudience handles three cases in priority order:
1. audience= already set — leave both keys untouched.
2. resource= present — copy to audience= (existing path).
3. Neither present — inject canonical into both. The token gets
bound to canonical exactly as if the client had sent it.
audienceMatchingStrategy's strict empty-needle reject stays as
defense in depth: case 3 only fires when canonical is configured
(main.go won't construct the OAuth server otherwise), but if some
future code path bypasses the translation helper, the matching
strategy still fails loudly rather than minting an unbound token.
Adds TestOAuth_Authorize_AcceptsNoResource_DefaultsToCanonical
pinning Claude Desktop's exact request shape (no resource=, no
audience=). Pairs with the existing AcceptsResourceOnly and
audience-mismatch tests to lock in the full /authorize matrix.
* docs(oauth): document RFC 8707 cross-server replay trade-off + audit log
Per Codex review #383 round 1: defaulting to canonical when the
client omits resource= weakens the cross-server replay defense
RFC 8707 was designed to provide. Threat is the confused-deputy
attack — malicious MCP server lies that pad's AS is its AS,
client (which doesn't send resource=) drives a flow against pad's
AS, pad mints a token bound to canonical, client returns it to
the attacker, attacker replays at pad's /mcp.
We're shipping with the default-to-canonical path because every
real-world MCP client (Claude Desktop / Cursor / ChatGPT as of
2026-05) omits resource= and the alternative is "remote MCP
doesn't work for any client until the entire ecosystem adopts
RFC 8707."
Mitigations now documented in the comment + active in the code:
- Consent screen (TASK-952) is the trust anchor. Every grant
requires a click-through that identifies the resource as
"your Pad workspaces" and lists the user's actual workspace
names. A user attempting to connect to a non-pad MCP server
who lands on pad's consent screen sees the mismatch.
- Matches industry practice (GitHub / Google / Atlassian all
rely on consent-as-trust-anchor since RFC 8707 is barely
deployed).
- audienceMatchingStrategy's strict empty-needle reject stays
as defense in depth — fires when canonical is unset and on
any future code path that bypasses the helper.
- Audit log (slog.Warn) on every default-fire gives ops a
signal to detect anomalies — a spike of defaulted requests
from a previously-unseen client_id is the earliest detectable
shape of a confused-deputy attempt.
Future task tracks restoring the strict reject once Claude /
Cursor / ChatGPT all send resource=.