fix(core): preserve the raw authority and gate the PRM derivation on a scheme - #24
Open
RobertoIskandarani wants to merge 2 commits into
Open
fix(core): preserve the raw authority and gate the PRM derivation on a scheme#24RobertoIskandarani wants to merge 2 commits into
RobertoIskandarani wants to merge 2 commits into
Conversation
wellKnownUrl built the document URL from URI.getAuthority(), which percent-decodes. An identifier whose userinfo carries an encoded '@' — "https://u%40b@api.example.com/mcp" — derived "https://u@b@api.example.com/.well-known/oauth-protected-resource/mcp": an authority with two '@' delimiters where the identifier names one, so the published URL points at a structurally different authority. The raw-preservation rule the derived path already follows (getRawPath, so "%2F" does not collapse to "/") applies to the authority for the same reason: the derived URL must name the identifier the resource publishes, not a re-encoded variant of it. getRawAuthority() carries the operator's encoding through verbatim. Core tests: 739, up from 738 on the base.
requireDerivable tested only isOpaque() and a null authority. A scheme-relative reference such as "//api.example.com/mcp" is neither, so it cleared the gate; the derivation then read a null scheme and built "null://api.example.com/.well-known/oauth-protected-resource/mcp". That string does not stay internal: AuthplaneResource.prmUrl() hands it to the resource_metadata parameter of the 401 WWW-Authenticate challenge, so a misconfigured resource advertises a URL no client can resolve rather than failing at construction where an operator would see it. RFC 8707 §2 requires the resource indicator to be an absolute URI, and RFC 3986 §4.3 defines one as always carrying a scheme, so gating on the scheme turns no legitimate identifier away. Core tests: 740, up from 739 on the preceding commit and 738 on the base.
muralx
approved these changes
Sep 2, 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
2 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.
Two independent defects in
ProtectedResourceMetadata, both of which corrupt the resourceidentifier on its way into the derived Protected Resource Metadata URL. Both are present in
the published
v0.1.0.The derived URL is not internal:
AuthplaneResource.prmUrl()splices it intoresource_metadataon a live 401WWW-Authenticatechallenge. A corrupted value is adiscovery failure the operator never sees — the response is a well-formed 401, nothing is
logged, and the client simply cannot resolve the document.
Two commits, each self-contained and green on its own.
1. The derived URL is built from the decoded authority
getAuthority()percent-decodes. Forhttps://u%40b@api.example.com/mcpit returnsu@b@api.example.com, so the derivation emitted:— an authority structurally different from the one the identifier names. RFC 9728 §3 forms
the URL by inserting the well-known string into the identifier itself, so every component
has to survive verbatim; the raw accessor is the one that preserves it.
Now uses
getRawAuthority(). Pinned bywellKnownUrl_preservesRawAuthority, which failsagainst the previous head with the decoded value above.
2.
requireDerivablenever checks the schemeThe gate was:
//api.example.com/mcpis neither opaque nor authority-less, so it passed.getScheme()returns
nullfor it, and the derivation buildsuri.getScheme() + "://" + ..., producing:The gate becomes
isOpaque() || getScheme() == null || getAuthority() == null, and theexception message names both requirements. No behaviour changes for any valid identifier:
RFC 8707 §2 requires an absolute URI as specified by RFC 3986 §4.3, whose grammar makes the
scheme mandatory, so a scheme-relative reference was never legal here.
Pinned by
schemeRelativeResource_cannotDeriveAPrmUrl, asserting bothwellKnownPathandwellKnownUrlreject. Fails against the previous head withExpecting code to raise a throwable.Verification
mvn -q verifygreen at both commits, so the branch is bisect-clean:e8f889b— core 739, mcp 62, spring 115973fdb2— core 740, mcp 62, spring 115Spotless and checkstyle report zero violations. CHANGELOG entries under
[Unreleased].Release note
v0.1.0is published and carries both defects. Worth deciding whether this rides a patchrelease rather than the next feature cut — the PRM source is identical at the tag and at
main, so these commits cherry-pick cleanly onto a hotfix branch.