fix: honor empty URL query and fragment markers (#2091)

URL.search and URL.hash are empty for both a missing component and an explicit empty one, so endpoint and origin checks were accepting URLs the callers meant to reject.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Faheem AhmadandCursor authored and GitHub committed 2026-10-13 06:05:29 +09:00
1 parent d59ab6d977
commit af29247a78
10 files changed
+62 -14

No files matched your search

+2 -2
View File
@@ -7,6 +7,7 @@ import { AnalysisError } from "../domain/analysisErrorBase.js";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js";
import { sanitizeBrowserUrl } from "../domain/browserObservation.js";
import { urlDeclaresQueryOrFragment } from "../domain/urlComponents.js";
import { safeParseJson } from "../domain/safeJson.js";
const endpointVersionSchema = z.object({
@@ -230,8 +231,7 @@ const safeCdpWebSocketEndpoint = (
parsed.port !== trustedEndpoint.port ||
parsed.username !== "" ||
parsed.password !== "" ||
parsed.search !== "" ||
parsed.hash !== "" ||
urlDeclaresQueryOrFragment(parsed) ||
path === undefined
)
throw new BrowserObservationError(operation, "invalid_endpoint_response");
+4
View File
@@ -19,6 +19,8 @@ describe("browser observation contracts", () => {
"https://*.example.com",
"https://example.com/path",
"https://example.com/?token=secret",
"https://example.com/?",
"https://example.com/#",
"https://user:pass@example.com",
"file:///tmp/page.html",
])
@@ -37,6 +39,8 @@ describe("browser observation contracts", () => {
"http://192.168.1.2:9222",
"https://127.0.0.1:9222",
"http://127.0.0.1:9222/json/list",
"http://127.0.0.1:9222/?",
"http://127.0.0.1:9222/#",
"http://user:pass@127.0.0.1:9222",
])
expect(browserEndpointSchema.safeParse(value).success, value).toBe(false);
+4 -4
View File
@@ -1,5 +1,7 @@
import { z } from "zod";
import { urlDeclaresQueryOrFragment } from "./urlComponents.js";
const parseExactOrigin = (value: string): string | undefined => {
let url: URL;
try {
@@ -15,8 +17,7 @@ const parseExactOrigin = (value: string): string | undefined => {
url.username !== "" ||
url.password !== "" ||
url.pathname !== "/" ||
url.search !== "" ||
url.hash !== ""
urlDeclaresQueryOrFragment(url)
)
return undefined;
return url.origin;
@@ -63,8 +64,7 @@ export const browserEndpointSchema = z
url.username !== "" ||
url.password !== "" ||
url.pathname !== "/" ||
url.search !== "" ||
url.hash !== ""
urlDeclaresQueryOrFragment(url)
) {
context.addIssue({
code: "custom",
@@ -6,6 +6,7 @@ import { javascriptRuntimeUnresolvedLocationSchema } from "./javascriptRuntimeOb
import { javascriptApplicationGraphSchema } from "./javascriptApplicationGraph.js";
import { digestSchema } from "../digests.js";
import { prefixedDigestSchema } from "../digests.js";
import { urlDeclaresQueryOrFragment } from "../urlComponents.js";
const evidenceIdSchema = prefixedDigestSchema("ev");
const nodeIdSchema = prefixedDigestSchema("jag_node");
@@ -53,8 +54,7 @@ const normalizeRuntimeUrlPrefix = (value: string): string | null => {
!["http:", "https:"].includes(url.protocol) ||
url.username !== "" ||
url.password !== "" ||
url.search !== "" ||
url.hash !== ""
urlDeclaresQueryOrFragment(url)
)
return null;
if (!url.pathname.endsWith("/")) url.pathname += "/";
+22
View File
@@ -0,0 +1,22 @@
import { describe, expect, it } from "vitest";
import { urlDeclaresQueryOrFragment } from "./urlComponents.js";
describe("URL query and fragment spelling", () => {
it("treats an explicit empty query or fragment as present", () => {
expect(urlDeclaresQueryOrFragment(new URL("https://a.test/main.js"))).toBe(
false,
);
expect(
urlDeclaresQueryOrFragment(new URL("https://a.test/%3Ffile.js")),
).toBe(false);
for (const value of [
"https://a.test/main.js?",
"https://a.test/main.js#",
"https://a.test/main.js?#",
"https://a.test/main.js?v=1",
"https://a.test/main.js#part",
])
expect(urlDeclaresQueryOrFragment(new URL(value)), value).toBe(true);
});
});
+7
View File
@@ -0,0 +1,7 @@
/**
* True when the URL spells a query or fragment, including an empty one.
* `URL.search` and `URL.hash` are empty both when that component is absent
* and when it is present but empty, so href is the spelling that keeps them apart.
*/
export const urlDeclaresQueryOrFragment = (url: URL): boolean =>
url.href.includes("?") || url.href.includes("#");
+2 -3
View File
@@ -4,6 +4,7 @@ import type {
CapturedWebScript,
ExportedWebScript,
} from "./webScriptExport.js";
import { urlDeclaresQueryOrFragment } from "./urlComponents.js";
const hash = (text: string): string =>
createHash("sha256").update(text).digest("hex");
@@ -94,9 +95,7 @@ const candidatePath = (script: CapturedWebScript): CandidatePath => {
return {
path: `origins/${hash(url.origin)}/${parts.join("/")}`,
reason:
// URL.search/hash collapse absent and explicitly empty components.
url.href.includes("?") ||
url.href.includes("#") ||
urlDeclaresQueryOrFragment(url) ||
url.username !== "" ||
url.password !== ""
? "URL query, fragment, or credentials cannot select a canonical local module."
+2 -2
View File
@@ -6,6 +6,7 @@ import type {
} from "../domain/javascript/javascriptRuntimeObservation.js";
import { BrowserObservationError } from "../domain/browserObservationError.js";
import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js";
import { urlDeclaresQueryOrFragment } from "../domain/urlComponents.js";
import { parseCdpEndpointValue, readCdpJson } from "../browser/CdpEndpoint.js";
const versionSchema = z
@@ -98,8 +99,7 @@ const validatedInspectorWebSocket = (
socket.port !== trusted.port ||
socket.username !== "" ||
socket.password !== "" ||
socket.search !== "" ||
socket.hash !== "" ||
urlDeclaresQueryOrFragment(socket) ||
!allowedPaths.has(socket.pathname)
)
throw new BrowserObservationError(operation, "invalid_endpoint_response");
+15
View File
@@ -0,0 +1,15 @@
import { describe, expect, it } from "vitest";
import { parseJebMcpEndpoint } from "./JebMcpEndpoint.js";
describe("JEB MCP endpoint", () => {
it("rejects an explicit empty query or fragment instead of dropping it", () => {
expect(parseJebMcpEndpoint("http://127.0.0.1:8425/mcp").ok).toBe(true);
for (const value of [
"http://127.0.0.1:8425/mcp?",
"http://127.0.0.1:8425/mcp#",
"http://127.0.0.1:8425/mcp?#",
])
expect(parseJebMcpEndpoint(value).ok, value).toBe(false);
});
});
+2 -1
View File
@@ -1,6 +1,7 @@
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import type { AnalysisError } from "../domain/analysisErrorBase.js";
import { err, ok, type Result } from "../domain/result.js";
import { urlDeclaresQueryOrFragment } from "../domain/urlComponents.js";
/** Default JEB MCP endpoint documented by the engine's headless client. */
export const DEFAULT_JEB_MCP_ENDPOINT = "http://127.0.0.1:8425/mcp";
@@ -46,7 +47,7 @@ export const parseJebMcpEndpoint = (
"Embedding credentials in the JEB MCP endpoint is not supported; configure authentication with the engine",
}),
);
if (url.search !== "" || url.hash !== "")
if (urlDeclaresQueryOrFragment(url))
return err(
new AnalysisInputError("jeb_mcp_endpoint", {
cause: "JEB MCP endpoint query strings and fragments are not supported",