From 8beba25b585aa0814abf326aaf46f5e6326e6dd6 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Mon, 24 Aug 2026 20:36:39 -0400
Subject: [PATCH] =?UTF-8?q?fix(auth):=20address=20review=20=E2=80=94=20rej?=
 =?UTF-8?q?ect=20a=20query=20or=20fragment=20in=20the=20metadata=20path?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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>
---
 .../mcp/oauth-resource-metadata-challenge.test.ts         | 5 +++++
 test-servers/src/test-server-oauth.ts                     | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts b/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts
index bda34f61..2a9f9f84 100644
--- a/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts
+++ b/clients/web/src/test/integration/mcp/oauth-resource-metadata-challenge.test.ts
@@ -116,6 +116,11 @@ describe("OAuth challenge resource_metadata (RFC 9728)", () => {
     ["https://other-host/doc", false],
     ["custom/protected-resource", 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],
   ])("origin-relative path check: %j -> %s", (value, expected) => {
     expect(isOriginRelativePath(value)).toBe(expected);
diff --git a/test-servers/src/test-server-oauth.ts b/test-servers/src/test-server-oauth.ts
index 71280861..3b7a28e8 100644
--- a/test-servers/src/test-server-oauth.ts
+++ b/test-servers/src/test-server-oauth.ts
@@ -40,6 +40,12 @@ const PATH_VALIDATION_BASE = "http://config.invalid";
  * it does not serve (Copilot). Comparing the resolved href against the literal
  * also rejects anything the parser would rewrite (spaces, unescaped
  * 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 {
   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);
     return (
       resolved.origin === PATH_VALIDATION_BASE &&
+      resolved.search === "" &&
+      resolved.hash === "" &&
       resolved.href === `${PATH_VALIDATION_BASE}${value}`
     );
   } catch {