Skip to content

fix: allow trailing slashes for OAuth Protected Metadata - #3062

Open
bh-wise wants to merge 1 commit into
modelcontextprotocol:mainfrom
bh-wise:fix/oauth-protected-metadata-allow-trailing-slash
Open

bh-wise wants to merge 1 commit into
modelcontextprotocol:mainfrom
bh-wise:fix/oauth-protected-metadata-allow-trailing-slash

Conversation

@bh-wise

@bh-wise bh-wise commented Oct 21, 2026 •

Copy link
Copy Markdown

Motivation and Context

When requesting server.com/resource/, the SDK uses server.com/.well-known/oauth-protected-resource/resource for the OAuth Protected Resource metadata. This is a bug -- RFC 9728 §3.1 requires removal of a "terminating slash (/) following the host component" but not following the path. This PR fixes the bug.

How Has This Been Tested?

I created new unit tests to test this.

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@bh-wise
bh-wise requested a review from a team as a code owner October 21, 2026 14:11
@changeset-bot

changeset-bot Bot commented Oct 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 50e59af

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/client Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/core-internal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Oct 21, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3062

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3062

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3062

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3062

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3062

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3062

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3062

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3062

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3062

commit: 50e59af

@claude claude Bot added the v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes label Oct 22, 2026
@arhancanli

Copy link
Copy Markdown

Read the diff (didn't run the suite). Keeping the trailing slash on non-root paths matches RFC 9728 section 3.1 as I read it: only the slash right after the host is dropped before /.well-known/oauth-protected-resource is inserted, so /mcp/ -> .../oauth-protected-resource/mcp/ is the spec form.

The risk is the one the changeset already names: a server that only serves the stripped /mcp path now gets a 404 on the first probe and the new test shows the next probe is the root URL, never .../mcp. Servers that registered metadata at the old URL then fall through to the root document, which may describe a different resource. Would it be worth one extra fallback to the stripped form between the two (and a test pinning the order), or is the intent to be strict? Either is defensible, but the order should be explicit.

@bh-wise

bh-wise commented Oct 23, 2026

Copy link
Copy Markdown
Author

The risk is the one the changeset already names: a server that only serves the stripped /mcp path now gets a 404 on the first probe and the new test shows the next probe is the root URL, never .../mcp. Servers that registered metadata at the old URL then fall through to the root document, which may describe a different resource. Would it be worth one extra fallback to the stripped form between the two (and a test pinning the order), or is the intent to be strict? Either is defensible, but the order should be explicit.

What's the concern here -- if a server has a resource of /mcp/ (with trailing slash) but only publishes metadata at /.well-known/oauth-protected-resource/mcp (without trailing slash)? I'd say that's a bug in the server so I'm not inclined to do anything to accommodate -- that said, I can see an argument for including some sort of transitional support until v3 (especially if we were to say that this is a breaking change). What do others think?

@arhancanli

Copy link
Copy Markdown

Yes, that is the case I meant, and I agree with you that strict is right. RFC 9728 §3.1 builds the metadata URL by inserting the well-known segment between host and path and keeping the path as written, so https://host/mcp/ maps to /.well-known/oauth-protected-resource/mcp/. §3.3 then requires the resource value in the document to be identical to the URL the client used, so a server that answers /mcp/ from the stripped-path document is already non-conformant.

If a transitional fallback is added anyway, I would put it after the exact URL and before the root document, and pin that order in a test. Falling through to root is the worse outcome because root may describe a different resource. The fallback result could also be checked against the §3.3 resource rule, which would reject a stripped-path document that names /mcp instead of /mcp/.

Do you know whether the existing OAuth conformance servers publish under a trailing-slash path, or only the stripped form?

@bh-wise

bh-wise commented Oct 23, 2026

Copy link
Copy Markdown
Author

Let's see what the maintainer(s) say, but I'm glad we're agreed that transitional support for buggy MCP servers is a waste of everyone's time.

FYI: §3.3 validation is mandatory -- it's "must be checked" rather than "could be checked". The SDK is currently more permissive than this but IMO that should be a separate change (and probably requires some discussion / alignment first!).

The current conformance metadata-discovery scenarios use /mcp without a trailing slash so this case doesn't apply.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants