modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(auth): address review — reject a query or fragment in the metadata path

Browse source 
`isOriginRelativePath` compared the resolved href against the literal, which
`URL` preserves a query and fragment in — so `/doc?version=1` and `/doc#section`
passed. Express matches on the path alone (and treats `?` as a pattern
character), and a fragment never reaches the server, so the advertised URL
could not resolve to the registered route. Reject both, with cases added to
the table-driven test.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-24 20:36:39 -04:00
commit 8beba25b58
2 changed files with 13 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

5
clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts
View file

@ -116,6 +116,11 @@ describe("OAuth challenge resource_metadata (RFC 9728)", () => {
["https://other-host/doc", false], ["https://other-host/doc", false],
["custom/protected-resource", false], ["custom/protected-resource", false],
["/has a space", false], ["/has a space", false],
// `href` preserves both, so these pass an origin+href comparison — but
// Express matches on the path alone and a fragment never reaches the
// server, so the advertised URL could not resolve to the route.
["/doc?version=1", false],
["/doc#section", false],
["", false], ["", false],
])("origin-relative path check: %j -> %s", (value, expected) => { ])("origin-relative path check: %j -> %s", (value, expected) => {
expect(isOriginRelativePath(value)).toBe(expected); expect(isOriginRelativePath(value)).toBe(expected);

8
test-servers/src/test-server-oauth.ts
View file

@ -40,6 +40,12 @@ const PATH_VALIDATION_BASE = "http://config.invalid";
* it does not serve (Copilot). Comparing the resolved href against the literal * it does not serve (Copilot). Comparing the resolved href against the literal
* also rejects anything the parser would rewrite (spaces, unescaped * also rejects anything the parser would rewrite (spaces, unescaped
* characters), which an Express route would not match either. * characters), which an Express route would not match either.
*
* A query or fragment is rejected for the same reason from the other
* direction: `href` preserves both, so `/doc?v=1` and `/doc#s` would pass the
* comparison above, yet Express matches on the path alone (and treats `?` as a
* pattern character) and a fragment is never sent on the wire at all — so the
* advertised URL could not reach the registered route (Copilot).
*/ */
export function isOriginRelativePath(value: unknown): value is string { export function isOriginRelativePath(value: unknown): value is string {
if (typeof value !== "string" || !value.startsWith("/")) { if (typeof value !== "string" || !value.startsWith("/")) {
@ -49,6 +55,8 @@ export function isOriginRelativePath(value: unknown): value is string {
const resolved = new URL(value, PATH_VALIDATION_BASE); const resolved = new URL(value, PATH_VALIDATION_BASE);
return ( return (
resolved.origin === PATH_VALIDATION_BASE && resolved.origin === PATH_VALIDATION_BASE &&
resolved.search === "" &&
resolved.hash === "" &&
resolved.href === `${PATH_VALIDATION_BASE}${value}` resolved.href === `${PATH_VALIDATION_BASE}${value}`
); );
} catch { } catch {