fix(security): validate filesystem paths that parse as http/ws URLs (#3186)

## Summary

`upload_file` (and the `memory` heap tools) run their path parameters
through `validateAndResolvePathOrUrl()` in `src/ToolHandler.ts`. That
helper returns any value that parses as an `http:`/`https:`/`ws:`/`wss:`
URL **unchanged**, skipping `context.validatePath()`. But these
parameters are filesystem-path sinks: `handle.uploadFile()` /
`fileChooser.accept()` and the heap-snapshot readers pass the string to
a filesystem API.

Puppeteer's `uploadFile` calls `path.resolve()` on a non-absolute
string, and an `http://` string is not absolute, so a value such as:

```
http://x/../../../../../../../../etc/passwd
```

resolves to `/etc/passwd`. On a default setup (server-launched local
Chrome, no extra flags) this reads an arbitrary file from the host and
loads it into a page's file input, bypassing both the configured MCP
roots and the default `os.tmpdir()` restriction. It reaches the same
`validatePath` boundary that was previously hardened for symlink/prefix
escapes, through a caller that skips it.

## Root cause

The URL pass-through is only correct for parameters whose sink actually
fetches the URL (`evaluate_script`'s `sourcePath`, read via
`loadResource`) or hands it to the browser (`install_pwa`'s
`installUrlOrBundleUrl`, documented as a "file:// or http(s):// signed
web bundle"). For every other parameter the value is a filesystem path,
so treating a string that merely looks like a URL as pre-validated is
wrong.

## Fix

Make the URL pass-through opt-in:

- `FileVerificationOption` gains an `allowUrl?: boolean` (default off),
documented to be set only on parameters whose sink fetches the URL or
hands it to the browser.
- `validateAndResolvePathOrUrl()` passes an `http/ws` URL through only
when `allowUrl` is set; otherwise the value goes through
`validatePath()` like any other path. `file:` URLs and
plain/relative/absolute paths are unchanged.
- `allowUrl: true` is set on `evaluate_script`'s `sourcePath` and
`install_pwa`'s `installUrlOrBundleUrl`. Nothing else changes behaviour
for valid inputs.

## Tests

- Two existing tests that relied on the global pass-through move to the
`allowUrl` form.
- Adds a regression test asserting that a traversal `http` URL passed to
a non-`allowUrl` sink is routed through `validatePath()` (and rejected)
rather than passed through.
- `npm run build` is clean; `npm run test tests/ToolHandler.test.ts`
passes. The browser-backed e2e suites require a local Chrome download
and were not run as part of this change.

## Disclosure

Reported to the Google OSS VRP (issue 574102042); it was closed as out
of scope because the OSS VRP now accepts supply-chain reports only, with
a note that the underlying issue looked valid and that a patch
submission would be welcome. This PR is that patch.

src/ToolHandler.ts

@@ -101,20 +101,35 @@ function getToolStatusInfo(
 async function validateAndResolvePathOrUrl(
   filePathOrUrl: string,
   context: McpContext,
+  allowUrl: boolean,
 ): Promise<string | undefined> {
   if (filePathOrUrl.trim().length === 0) {
     return undefined;
   }
+  let url: URL | undefined;
   try {
-    const url = new URL(filePathOrUrl);
+    url = new URL(filePathOrUrl);
+  } catch {
+    // Not a URL; fall through and validate it as a regular file path.
+  }
+  if (url) {
     if (url.protocol === 'file:') {
       return pathToFileURL(await context.validatePath(fileURLToPath(url))).href;
     }
     if (['http:', 'https:', 'ws:', 'wss:'].includes(url.protocol)) {
-      return filePathOrUrl;
+      // Only pass a remote URL through unchanged for parameters whose sink
+      // actually fetches the URL or hands it to the browser (allowUrl). For a
+      // filesystem-path parameter, a value that merely looks like an http/ws
+      // URL must still be rejected outright: `new URL()` accepts
+      // `http://x/../../etc/passwd`, and the downstream filesystem API (e.g.
+      // Puppeteer's uploadFile) path.resolve()s that non-absolute string into
+      // an absolute host path, which would bypass validatePath (workspace
+      // roots and the temp-dir fallback) if it fell through to it unchanged.
+      if (allowUrl) {
+        return filePathOrUrl;
+      }
+      throw new Error('This parameter expects a file path, not a URL.');
     }
-  } catch {
-    // Suppress parsing errors for regular file paths.
   }
   return await context.validatePath(filePathOrUrl);
 }
@@ -133,7 +148,7 @@ function isLocalBrowser(browser: Browser): boolean {
 function shouldValidateFile(
   option: FileVerificationOption | undefined,
   isLocal: boolean,
-): boolean {
+): option is FileVerificationOption {
   if (option === true) {
     return true;
   }
@@ -158,14 +173,19 @@ async function validateToolFiles(
 ): Promise<void> {
   for (const [key, option] of Object.entries(tool.verifyFilesSchema)) {
     if (shouldValidateFile(option, isLocal)) {
+      const allowUrl = option !== true && option.allowUrl === true;
       const val = params[key];
       if (typeof val === 'string') {
-        params[key] = await validateAndResolvePathOrUrl(val, context);
+        params[key] = await validateAndResolvePathOrUrl(val, context, allowUrl);
       } else if (Array.isArray(val)) {
         const updated: unknown[] = [];
         for (const item of val) {
           if (typeof item === 'string') {
-            const resolved = await validateAndResolvePathOrUrl(item, context);
+            const resolved = await validateAndResolvePathOrUrl(
+              item,
+              context,
+              allowUrl,
+            );
             if (resolved !== undefined) {
               updated.push(resolved);
             }

src/tools/ToolDefinition.ts

@@ -62,6 +62,17 @@ export type FileVerificationOption =
   | {
       local?: boolean;
       remote?: boolean;
+      /**
+       * When true, a value that parses as an `http(s):`/`ws(s):` URL is passed
+       * through unchanged instead of being resolved as a filesystem path. Only
+       * set this for parameters whose sink fetches the URL (for example a
+       * source map or script source the server downloads) or hands it to the
+       * browser (for example a PWA install URL). For filesystem-path
+       * parameters it MUST stay unset: otherwise a value such as
+       * `http://x/../../etc/passwd` skips `validatePath` and is resolved to an
+       * absolute host path by the downstream filesystem API.
+       */
+      allowUrl?: boolean;
     };
 
 type AllKeys<T> = T extends unknown ? keyof T : never;

src/tools/pwa.ts

@@ -56,6 +56,9 @@ export const installPwa = defineTool(() => ({
     installUrlOrBundleUrl: {
       local: true,
       remote: false,
+      // May be an http(s):// or file:// signed web bundle URL handed to the
+      // browser, so allow URLs through; a plain path is still validated.
+      allowUrl: true,
     },
   },
   handler: async (request, response, context) => {

src/tools/script.ts

@@ -103,7 +103,9 @@ Example with arguments: \`(el) => el.innerText\`
     blockedByDialog: true,
     verifyFilesSchema: {
       filePath: true,
-      sourcePath: true,
+      // sourcePath may be an http(s): URL the server fetches, so allow URLs
+      // through; a file: path or plain path is still validated.
+      sourcePath: {local: true, remote: true, allowUrl: true},
     },
     handler: async (request, response, context) => {
       const {

tests/ToolHandler.test.ts

@@ -612,7 +612,9 @@ describe('ToolHandler', () => {
       blockedByDialog: false,
       verifyFilesSchema: {
         filePath: true,
-        fileList: true,
+        // allowUrl lets a remote URL pass through unchanged (file/plain paths
+        // are still validated); this is what source-map/PWA-style params use.
+        fileList: {local: true, remote: true, allowUrl: true},
       },
       handler: async request => {
         handlerCalled = true;
@@ -1011,6 +1013,7 @@ describe('ToolHandler', () => {
       verifyFilesSchema: {
         installUrlOrBundleUrl: {
           local: true,
+          allowUrl: true,
         },
       },
       handler: async () => {
@@ -1044,6 +1047,60 @@ describe('ToolHandler', () => {
     assert.strictEqual(mockContext.validatePath.called, false);
   });
 
+  it('rejects an http/ws URL for a filesystem-path parameter when allowUrl is not set, even on a local browser', async () => {
+    // Regression test for the allowUrl TSDoc in ToolDefinition.ts: a filesystem-path parameter must reject a value that merely parses as an http/ws URL instead of passing it through.
+    let handlerCalled = false;
+    const tool: ToolDefinition = {
+      name: 'upload_tool',
+      description: 'A local-only file sink without allowUrl',
+      annotations: {
+        category: ToolCategory.INPUT,
+        readOnlyHint: false,
+      },
+      schema: {
+        filePaths: zod.array(zod.string()),
+      },
+      blockedByDialog: false,
+      verifyFilesSchema: {
+        filePaths: {
+          local: true,
+          remote: false,
+        },
+      },
+      handler: async () => {
+        handlerCalled = true;
+      },
+    };
+
+    const mockContext = sinon.createStubInstance(McpContext);
+    const mockProcess = sinon.createStubInstance(ChildProcess);
+    mockContext.ensureBrowser.resolves(getMockBrowser({process: mockProcess}));
+
+    const toolMutex = new Mutex();
+    const serverArgs = new ConfigParser('1.0.0', ['node', 'script.js'], {
+      CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
+    }).parse();
+
+    const toolHandler = new ToolHandler(
+      tool,
+      serverArgs,
+      async () => mockContext,
+      toolMutex,
+      sinon.spy(),
+    );
+
+    const traversalUrl = 'http://x/' + '../'.repeat(12) + 'etc/passwd';
+    const result = await toolHandler.handle({filePaths: [traversalUrl]});
+
+    assert.strictEqual(result.isError, true);
+    assert.strictEqual(handlerCalled, false);
+    sinon.assert.notCalled(mockContext.validatePath);
+    assert.strictEqual(
+      result.content[0].type === 'text' ? result.content[0].text : '',
+      'Error: This parameter expects a file path, not a URL.',
+    );
+  });
+
   it('validates verifyFilesSchema with true but skips local: true on remote browser', async () => {
     let handlerCalled = false;
     let receivedParams: Record<string, unknown> | undefined;