Skip to content

auth: reject userinfo, fragments, and dot segments in client ID URLs - #1362

Merged
guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
ilaigold:auth-client-id-metadata-url
Oct 9, 2026
Merged

guglielmo-san merged 2 commits into
modelcontextprotocol:mainfrom
ilaigold:auth-client-id-metadata-url

Conversation

@ilaigold

@ilaigold ilaigold commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

While reading NewAuthorizationCodeHandler I checked ClientIDMetadataDocumentConfig.URL against draft-ietf-oauth-client-id-metadata-document-00 section 3, which that field cites. The check only required HTTPS and a non-empty path, so a URL with a username, a fragment, or a . or .. path segment was accepted and later sent as the client_id. Repro is in #1361.

Those three are MUST NOT in that section. I reject them in validateClientIDMetadataDocumentURL. An empty fragment is included: url.Parse drops it, so the check looks for # in the raw URL. %2e%2e is caught after the path is decoded. A query is still accepted, because the draft says SHOULD NOT, not MUST NOT. https://example.com/ is still accepted, because / is a path.

No API change. The old "non-root HTTPS URL" error is unchanged for a missing path or a non-HTTPS scheme.

Tested with go1.27.1 on darwin/arm64, on main at 8dd5d6a:

  • TestClientIDMetadataDocumentURL fails before the fix: the userinfo, fragment, and dot-segment cases return a nil error. It passes after.
  • gofmt -l on the two auth files is clean, go vet ./auth/ is clean, and go test -count=1 -race ./auth/ passes.
  • staticcheck v0.6.1 is clean on ./auth/ (run with GOTOOLCHAIN=go1.26.0, since v0.6.1 can't read Go 1.27 export data).

Fixes #1361

AI disclosure: this PR, including the patch and the tests, was written primarily by Claude Opus 5.5 in Cursor.

NewAuthorizationCodeHandler accepted a client ID metadata document
URL with a username, a fragment, or a "." or ".." path segment.
draft-ietf-oauth-client-id-metadata-document-00 section 3, which
ClientIDMetadataDocumentConfig.URL cites, says those MUST NOT appear.
A query is still accepted, because that draft only says SHOULD NOT.

Fixes modelcontextprotocol#1361

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@guglielmo-san
guglielmo-san merged commit 3c08147 into modelcontextprotocol:main Oct 9, 2026
9 checks passed
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.

auth: NewAuthorizationCodeHandler accepts client ID metadata URLs the cited draft forbids

2 participants