Skip to content

fix(auth): preserve root protected resource URI - #3259

Closed
daleselaji-dev wants to merge 2 commits into
modelcontextprotocol:mainfrom
daleselaji-dev:agent/canonical-resource-url
Closed

fix(auth): preserve root protected resource URI#3259
daleselaji-dev wants to merge 2 commits into
modelcontextprotocol:mainfrom
daleselaji-dev:agent/canonical-resource-url

Conversation

@daleselaji-dev

Copy link
Copy Markdown

Problem

ProtectedResourceMetadata.resource can be serialized with a synthetic trailing slash when a root resource URL is supplied as an already-validated AnyHttpUrl. This changes https://example.com to https://example.com/ and can break strict RFC 9728 resource matching.

Root Cause

AnyHttpUrl normalizes an empty URL path to / before ProtectedResourceMetadata's url_preserve_empty_path model setting can preserve the original spelling. The server auth settings pass an already-validated URL object into the metadata model.

Solution

Normalize only the root path in a ProtectedResourceMetadata pre-validator, removing the synthetic slash before AnyHttpUrl validation. Non-root paths and URL validation remain unchanged.

Changes

  • Preserve an empty root path for ProtectedResourceMetadata.resource.
  • Add regression coverage for raw metadata input and an already-normalized URL object.

Testing

  • py -m pytest tests/shared/test_auth.py -q — 43 passed.
  • py -m pytest tests/shared/test_auth_utils.py tests/shared/test_auth.py -q — 58 passed.
  • py -m pytest tests/client/test_auth.py -q — 140 passed, 1 xfailed.
  • py -m ruff format --check src/mcp/shared/auth.py tests/shared/test_auth.py — passed.
  • py -m ruff check src/mcp/shared/auth.py tests/shared/test_auth.py — passed.
  • git diff --check — passed.

Compatibility/Risk

The public field type remains AnyHttpUrl. Only a root-path slash synthesized by URL normalization is removed; non-root paths, query strings, and validation behavior are unchanged.

Notes for Reviewer

Pyright could not be run with the repository's configured .venv because this incremental worktree does not contain that environment. A separate available environment reported no source-file diagnostics but reported unrelated baseline test typing diagnostics; this is explicitly unverified locally.

Linked Issue

Closes #2883

@keeltrace

This comment was marked as spam.

@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR has been closed automatically. It's still a draft, but we close those early so you don't put in more time only to have it closed the moment you mark it ready.

This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and this PR doesn't link an open issue yet.

  • If you're already assigned to an issue for this, add Fixes #<n> to the description and the PR will reopen on its own.
  • If there's no issue yet, please open one instead: what you ran into, why it matters for your use case, and a minimal reproduction. That context is super important to us and is what we use to decide what to prioritise.
  • If there's an issue but you're not assigned, add Fixes #<n> anyway so they're linked, then engage on the issue itself by confirming the repro or describing the approach you'd take. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. If you are assigned, this PR reopens automatically.

You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.

CONTRIBUTING.md has the full reasoning, but in short:

  • We're a small team with very little capacity to review community PRs right now.
  • Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.

Maintainers: reopen, remove missing-issue-link, or add bypass-issue-check to override.

@github-actions github-actions Bot closed this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AnyHttpUrl normalization adds trailing slash → breaks RFC 9728 canonical resource URL in ProtectedResourceMetadata

2 participants