fix(browser): preserve URL paths when removing credentials (#4731)

* fix(browser): preserve URL paths when removing credentials

* test(browser): verify URL userinfo through real V8 captures

scripts/lib/browser-url-userinfo-e2e.mjs

@@ -0,0 +1,186 @@
+import assert from "node:assert/strict";
+import { createHash } from "node:crypto";
+import { fileURLToPath } from "node:url";
+import { Client } from "@modelcontextprotocol/client";
+import { StdioClientTransport } from "@modelcontextprotocol/client/stdio";
+
+import { CdpConnection } from "../../dist/browser/CdpConnection.js";
+import { parseEvidence } from "../../dist/domain/evidence.js";
+import { artifactCliEvidence } from "./artifact-e2e.mjs";
+import { closeVerifierResources } from "./verifier-cleanup.mjs";
+
+/** Verify real V8 source labels, which retain spelling that navigation normalizes. */
+export async function verifyBrowserUrlUserInfo(endpoint, origin) {
+  const rawUrl = `${origin.replace("://", "://user:pass@")}\\path@tail/../keep.js?x=1#frag`;
+  const expectedUrl = `${origin}\\path@tail/../keep.js?x=1#frag`;
+  const sources = [rawUrl, expectedUrl].map(
+    (url, index) =>
+      `globalThis.reaUserinfoUrlProbe${index} = () => 4731;\n//# sourceURL=${url}`,
+  );
+  const response = await fetch(`${endpoint}/json/new?about:blank`, {
+    method: "PUT",
+  });
+  assert.equal(
+    response.ok,
+    true,
+    "Browser refused the owned URL fixture target",
+  );
+  const target = await response.json();
+  assert.equal(typeof target.id, "string");
+  let connection;
+  let transport;
+  let client;
+  let verificationError;
+  try {
+    connection = await CdpConnection.connect(
+      target.webSocketDebuggerUrl,
+      "inspect_web_page",
+    );
+    await loadFixturePage(connection, `${origin}/app?userinfo-probe`);
+    const producerUrls = [];
+    connection.onEvent((event) => {
+      if (event.method === "Debugger.scriptParsed")
+        producerUrls.push(event.params.url);
+    });
+    await connection.send("Debugger.enable");
+    const browserUrl = await connection.send("Runtime.evaluate", {
+      expression: `new URL(${JSON.stringify(rawUrl)}).href`,
+      returnByValue: true,
+    });
+    assert.equal(browserUrl.exceptionDetails, undefined);
+    assert.equal(browserUrl.result.value, new URL(rawUrl).href);
+    assert.notEqual(browserUrl.result.value, rawUrl);
+    assert.equal(new URL(browserUrl.result.value).pathname, "/keep.js");
+    for (const source of sources) {
+      const evaluated = await connection.send("Runtime.evaluate", {
+        expression: source,
+        returnByValue: true,
+      });
+      assert.equal(evaluated.exceptionDetails, undefined);
+    }
+    assert.ok(
+      [rawUrl, expectedUrl].every((url) => producerUrls.includes(url)),
+      "Real V8 did not retain the exact sourceURL spelling on the CDP wire",
+    );
+    const assertScripts = (evidence) => {
+      const result = parseEvidence(evidence).normalized_result;
+      return sources.map((source) => {
+        const script = result.scripts.items.find(
+          (item) =>
+            item.source.included && item.source.artifact.text === source,
+        );
+        assert.ok(
+          script,
+          "Public capture omitted the retained V8 fixture script",
+        );
+        assert.equal(script.url, expectedUrl);
+        assert.equal(new URL(script.url).origin, origin);
+        assert.equal(
+          script.source.artifact.sha256,
+          createHash("sha256").update(source).digest("hex"),
+        );
+        return script.url;
+      });
+    };
+    const cliUrl = assertScripts(
+      await artifactCliEvidence("inspect-web-page", endpoint, [
+        target.id,
+        "--allowed-origins",
+        origin,
+        "--include-script-sources",
+        "--observation-ms",
+        "0",
+      ]),
+    );
+    transport = new StdioClientTransport({
+      command: process.execPath,
+      args: [fileURLToPath(new URL("../rea.mjs", import.meta.url)), "mcp"],
+      env: { ...process.env, REA_LOG_LEVEL: "silent" },
+      stderr: "pipe",
+    });
+    client = new Client({ name: "browser-url-userinfo-e2e", version: "1" });
+    await client.connect(transport);
+    const mcp = await client.callTool({
+      name: "inspect_web_page",
+      arguments: {
+        cdp_endpoint: endpoint,
+        allowed_origins: [origin],
+        target_id: target.id,
+        include_script_sources: true,
+        observation_ms: 0,
+      },
+    });
+    assert.notEqual(mcp.isError, true, JSON.stringify(mcp));
+    assert.deepEqual(assertScripts(mcp.structuredContent), cliUrl);
+    return {
+      cli: true,
+      stdio_mcp: true,
+      raw_v8_source_url: true,
+      credential_free_control: true,
+      original_source_digest: true,
+      browser_url_parser_normalizes_backslash_and_dot_segments: true,
+    };
+  } catch (cause) {
+    verificationError = cause;
+    throw cause;
+  } finally {
+    await closeVerifierResources(
+      [
+        async () => client?.close(),
+        async () => transport?.close(),
+        async () => connection?.close(),
+        async () => {
+          const closed = await fetch(`${endpoint}/json/close/${target.id}`);
+          assert.equal(
+            closed.ok,
+            true,
+            "Browser retained the owned URL fixture target",
+          );
+        },
+      ],
+      verificationError,
+    );
+  }
+}
+
+async function loadFixturePage(connection, url) {
+  const loaded = new Set();
+  let loaderId;
+  let resolveLoaded;
+  let timer;
+  const load = new Promise((resolve) => {
+    resolveLoaded = resolve;
+  });
+  const unsubscribe = connection.onEvent((event) => {
+    if (event.method !== "Page.lifecycleEvent" || event.params.name !== "load")
+      return;
+    loaded.add(event.params.loaderId);
+    if (event.params.loaderId === loaderId) resolveLoaded();
+  });
+  try {
+    await connection.send("Page.enable");
+    await connection.send("Page.setLifecycleEventsEnabled", { enabled: true });
+    const navigation = await connection.send("Page.navigate", { url });
+    assert.equal(navigation.errorText, undefined);
+    assert.equal(typeof navigation.loaderId, "string");
+    loaderId = navigation.loaderId;
+    if (!loaded.has(loaderId))
+      await Promise.race([
+        load,
+        new Promise((_, reject) => {
+          timer = setTimeout(
+            () => reject(new Error("Owned URL fixture did not finish loading")),
+            10_000,
+          );
+        }),
+      ]);
+    assert.equal(
+      (await connection.send("Page.getFrameTree")).frameTree.frame.url,
+      url,
+    );
+  } finally {
+    clearTimeout(timer);
+    unsubscribe();
+    await connection.send("Page.setLifecycleEventsEnabled", { enabled: false });
+  }
+}

scripts/verify-real-browser.mjs

@@ -53,6 +53,7 @@ import { artifactCliEvidence, artifactMcpResult } from "./lib/artifact-e2e.mjs";
 import { verifyScenarioEnvironment } from "./lib/browser-scenario-environment-e2e.mjs";
 import { verifyScenarioStorage } from "./lib/browser-scenario-storage-e2e.mjs";
 import { verifyScenarioServiceWorkers } from "./lib/browser-scenario-service-workers-e2e.mjs";
+import { verifyBrowserUrlUserInfo } from "./lib/browser-url-userinfo-e2e.mjs";
 
 const SCENARIO_SECRET_VALUE = "rea-browser-verifier-secret";
 const SCENARIO_URL_SECRET_VALUE = "rea-browser-url-verifier-secret";
@@ -378,6 +379,7 @@ try {
   });
   const moduleTrace = await verifyBrowserModules(executable);
   const captureMetadata = await verifyBrowserCaptureMetadataBudget(endpoint);
+  const urlUserInfo = await verifyBrowserUrlUserInfo(endpoint, site.origin);
   report = {
     browser: observed.value.browser.product,
     endpoint,
@@ -405,6 +407,7 @@ try {
     scriptExport,
     moduleTrace,
     captureMetadata,
+    urlUserInfo,
     domDestinations,
     browserScenarioCli: true,
     browserScenarioAttachCleanup: "disconnected-external",

src/domain/browserObservation.test.ts

@@ -123,6 +123,42 @@ describe("browser observation sensitive surfaces and retention", () => {
     expect(sanitized.redacted).toBe(true);
   });
 
+  it("preserves path userinfo-like text after a special-scheme backslash", () => {
+    const suffix = String.raw`\path@tail/../keep?x=1#frag`;
+    expect(
+      sanitizeBrowserUrl(`https://user:pass@app.example.test${suffix}`),
+    ).toEqual({
+      url: `https://app.example.test${suffix}`,
+      origin: "https://app.example.test",
+      query_parameter_names: ["x"],
+      redacted: true,
+    });
+    expect(sanitizeBrowserUrl(`https://app.example.test${suffix}`)).toEqual({
+      url: `https://app.example.test${suffix}`,
+      origin: "https://app.example.test",
+      query_parameter_names: ["x"],
+      redacted: false,
+    });
+    expect(
+      sanitizeBrowserUrl(
+        String.raw`custom://user:pass@host\name@tail/path?x=1#frag`,
+      ),
+    ).toEqual({
+      url: "custom://tail/path?x=1#frag",
+      origin: null,
+      query_parameter_names: ["x"],
+      redacted: true,
+    });
+    expect(
+      sanitizeBrowserUrl("https://us\ter:pass@app.example.test/path?x=1#frag"),
+    ).toEqual({
+      url: "https://app.example.test/path?x=1#frag",
+      origin: "https://app.example.test",
+      query_parameter_names: ["x"],
+      redacted: true,
+    });
+  });
+
   it("retains ordinary query values and malformed URL diagnostics", () => {
     const longName = `a${"x".repeat(400)}`;
     const parameters = [

src/domain/browserObservation.ts

@@ -1,7 +1,10 @@
 import { z } from "zod";
 
 import { urlDeclaresQueryOrFragment } from "./urlComponents.js";
-import { parseUrlQueryParameterNames } from "./recordedTransportUrl.js";
+import {
+  parseRecordedTransportUrl,
+  parseUrlQueryParameterNames,
+} from "./recordedTransportUrl.js";
 
 const parseExactOrigin = (value: string): string | undefined => {
   let url: URL;
@@ -268,32 +271,10 @@ export const sanitizeBrowserUrl = (value: string): SanitizedBrowserUrl => {
 };
 
 const removeUrlUserInfo = (value: string, parsed: URL): string => {
-  const schemeSeparator = value.indexOf("://");
-  const authorityStart =
-    schemeSeparator < 0
-      ? value.startsWith("//")
-        ? 2
-        : -1
-      : schemeSeparator + 3;
-  if (authorityStart < 0) return credentialFreeHref(parsed);
-  const remaining = value.slice(authorityStart);
-  const delimiter = remaining.search(/[/?#]/u);
-  const authorityEnd =
-    delimiter < 0 ? value.length : authorityStart + delimiter;
-  const authority = value.slice(authorityStart, authorityEnd);
-  const userInfoEnd = authority.lastIndexOf("@");
-  if (userInfoEnd < 0) return credentialFreeHref(parsed);
-  const candidate = `${value.slice(0, authorityStart)}${authority.slice(userInfoEnd + 1)}${value.slice(authorityEnd)}`;
-  try {
-    const candidateUrl = new URL(candidate);
-    return candidateUrl.username === "" && candidateUrl.password === ""
-      ? candidate
-      : credentialFreeHref(parsed);
-  } catch (cause: unknown) {
-    // Fallback to the already-sanitized href when the candidate is invalid.
-    void cause;
-    return credentialFreeHref(parsed);
-  }
+  const { credentials } = parseRecordedTransportUrl(value);
+  return credentials === null || credentials === "ambiguous"
+    ? credentialFreeHref(parsed)
+    : `${value.slice(0, credentials.start)}${value.slice(credentials.end)}`;
 };
 
 const credentialFreeHref = (parsed: URL): string => {

tests/boundary/browser/cdpBrowserProvider.targetInventory.test.ts

@@ -2,11 +2,12 @@ import { createServer } from "node:http";
 
 import { describe, expect, it } from "vitest";
 
-import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js";
-import { listBrowserTargetsInputSchema } from "../../../src/domain/browserObservation.js";
+import { parseEvidence } from "../../../src/domain/evidence.js";
+import { browserTargetListSchema } from "../../../src/domain/browserObservationSchemas.js";
+import { runCliJson } from "../../fixtures/cliJsonProcess.js";
 
 describe("CDP target inventory", () => {
-  it("returns every allowed target within the bounded discovery payload", async () => {
+  it("returns complete target inventory and original URL paths through the CLI", async () => {
     let port = 0;
     const targets = Array.from({ length: 1_101 }, (_, index) => {
       const id = `target-${String(index).padStart(4, "0")}`;
@@ -14,7 +15,10 @@ describe("CDP target inventory", () => {
         id,
         type: "page",
         title: `Page ${String(index)}`,
-        url: `https://allowed.example/page/${String(index)}`,
+        url:
+          index === 0
+            ? String.raw`https://user:pass@allowed.example\path@tail/../keep?x=1#frag`
+            : `https://allowed.example/page/${String(index)}`,
         ...(index === 0 ? {} : { attached: index === 1 ? false : true }),
         webSocketDebuggerUrl: `ws://127.0.0.1:${String(port)}/devtools/page/${id}`,
       };
@@ -52,24 +56,30 @@ describe("CDP target inventory", () => {
       port = address.port;
       const endpoint = `http://127.0.0.1:${String(port)}`;
 
-      const result = await new CdpBrowserProvider().listTargets(
-        listBrowserTargetsInputSchema.parse({
-          cdp_endpoint: endpoint,
-          allowed_origins: ["https://allowed.example"],
-        }),
+      const evidence = parseEvidence(
+        await runCliJson([
+          "list-browser-targets",
+          endpoint,
+          "--allowed-origins",
+          "https://allowed.example",
+          "--json",
+        ]),
       );
+      const result = browserTargetListSchema.parse(evidence.normalized_result);
 
-      if (!result.ok) throw result.error;
-      expect(result.value.targets).toHaveLength(targets.length);
-      expect(result.value.targets[0]?.target_id).toBe("target-0000");
-      expect(result.value.targets.at(-1)?.target_id).toBe("target-1100");
+      expect(result.targets).toHaveLength(targets.length);
+      expect(result.targets[0]?.target_id).toBe("target-0000");
+      expect(result.targets[0]?.url).toBe(
+        String.raw`https://allowed.example\path@tail/../keep?x=1#frag`,
+      );
+      expect(result.targets[0]?.origin).toBe("https://allowed.example");
+      expect(JSON.stringify(evidence)).not.toContain("user:pass");
+      expect(result.targets.at(-1)?.target_id).toBe("target-1100");
       expect(
-        result.value.targets.slice(0, 3).map(({ attached }) => attached),
+        result.targets.slice(0, 3).map(({ attached }) => attached),
       ).toEqual([null, false, true]);
       expect(
-        result.value.limitations.some((item) =>
-          item.includes("attachment state"),
-        ),
+        result.limitations.some((item) => item.includes("attachment state")),
       ).toBe(true);
     } finally {
       if (server.listening)