Conversation
When a server returns resource_metadata in its WWW-Authenticate header, Priority 3 (RFC 9728) would hard-return on any failure, preventing Priorities 4 and 5 from running. For servers like Sourcegraph and Deepsearch whose authorization_servers list contains path-based URLs (e.g. /.api/mcp/deepsearch), RFC 8414 tenant extraction generates /.well-known/oauth-authorization-server/.api/mcp/deepsearch — an endpoint those servers don't serve. Validation fails, Priority 3 errors, and auth is broken despite Priorities 4/5 being able to derive the correct root-domain issuer. Fix by capturing the error from tryDiscoverFromResourceMetadata and only returning on success; failures now log at DEBUG and fall through to well-known probing (Priority 4) and URL-derived issuer (Priority 5), restoring the behaviour that existed before 6cad0b5. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4371 +/- ##
==========================================
+ Coverage 69.52% 69.55% +0.02%
==========================================
Files 480 480
Lines 48978 48981 +3
==========================================
+ Hits 34051 34067 +16
+ Misses 12302 12289 -13
Partials 2625 2625 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aponcedeleonch
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
resource_metadatain theirWWW-Authenticateheader but list path-based URLs in theirauthorization_serversfield (e.g.https://sourcegraph.com/.api/mcp/sourcegraph). Per RFC 8414, ToolHive extracts the path as a tenant and builds a discovery URL like/.well-known/oauth-authorization-server/.api/mcp/sourcegraph— an endpoint those servers don't serve. Validation fails and auth breaks.DeriveIssuerFromURLalways stripped the path and returned the root domain issuer, which worked. That commit introduced the RFC 9728 resource metadata flow with a hardreturnat Priority 3, so when validation fails Priorities 4 and 5 (includingDeriveIssuerFromURL) are never reached.tryDiscoverFromResourceMetadataand only return on success. Failures log at DEBUG and fall through to Priority 4 (well-known probing) and Priority 5 (URL-derived issuer), restoring the previous working behaviour.Type of change
Test plan
task test)Does this introduce a user-facing change?
Yes — MCP servers that use path-based
authorization_serversURLs in their resource metadata (e.g. Sourcegraph, Deepsearch) will successfully authenticate again instead of failing with a malformed discovery URL error.Special notes for reviewers
The regression was introduced in
6cad0b59and first shipped in v0.3.0. Two test cases were updated:malformed resource metadata URLandhandles malicious resource metadata response. Both previously expected an error that can no longer be returned — the function now falls through to Priority 5 and returns a URL-derived issuer. The DoS protection property of the second test is unchanged (response body draining is still capped byMaxResponseBodyDrain; the test just now asserts the derived issuer rather than an error).Generated with Claude Code