fix: reject --blockedUrlPattern/--allowedUrlPattern hostname regexp groups (#2796)

## Problem

`--blockedUrlPattern`/`--allowedUrlPattern` accept any URLPattern
string, including hostnames that use a regexp group (for example
`*://(127\.\d+\.\d+\.\d+):*/*`, meant to cover a whole IP range). That
pattern is enforced correctly by the target-attach check
(`TargetManager#isUrlAllowed`, using the real `URLPattern.test()`), but
the actual network-level blocking runs through the CDP command
`Network.emulateNetworkConditionsByRule`, passing the raw pattern string
as `NetworkConditions.urlPattern`. That command's native matching does
not apply the pattern consistently on redirects, so a page allowed to
load can redirect straight through a blocked host range while an
exact-hostname pattern stays blocked in both cases.

Root-caused and written up in more detail on #2777.

This isn't something fixable from this repo's side (the mismatch is
between the documented CDP `urlPattern` semantics and the underlying
Chromium implementation for `emulateNetworkConditionsByRule`, not in
`puppeteer-core` or `chrome-devtools-mcp`), so instead of leaving the
gap silent, this rejects patterns this repo can't currently guarantee
are enforced.

## Fix

- Added `findUnenforceableHostnamePattern` (`src/utils/url.ts`), which
parses each pattern with `URLPattern` and flags one whose canonicalized
`hostname` contains a regexp group (`(`). Wildcard (`*`) and named-group
(`:name`) hostnames are unaffected and continue to work as before; a
regexp group outside the hostname (e.g. in the pathname) is also left
alone, since only the hostname case is demonstrated as broken.
- Wired it into the `blockedUrlPattern`/`allowedUrlPattern` CLI option
`coerce`, so an offending pattern fails fast at startup with a clear
error instead of silently only half-working.
- Updated both options' `describe` text and regenerated
`docs/configuration.md` via `npm run gen`.

## Testing

- Added unit tests in `tests/utils/url.test.ts` covering: a hostname
regexp group (flagged), multiple patterns (first offender returned), an
exact hostname, a wildcard hostname, a named-group hostname, a regexp
group outside the hostname, and a pattern that fails to construct.
- `npx tsc` and `npx eslint` both pass clean on the changed files.
- Verified the new function's behavior against Puppeteer's own vendored
`URLPattern` polyfill
(`node_modules/puppeteer-core/lib/third_party/urlpattern-polyfill`),
matching every case in the new test suite - my local Node (22.22)
predates Node's global `URLPattern` support that this repo's `.nvmrc`
(v24) assumes, so I couldn't run the built test file directly against
the global here, but the polyfill is the same spec implementation and
gave identical results for all cases.

Fixes #2777

---------

Co-authored-by: Natasha Gorshunova <47688881+nattallius@users.noreply.github.com>

docs/configuration.md

@@ -168,11 +168,11 @@ The Chrome DevTools MCP server supports the following configuration option:
   - **Type:** number
 
 - **`--blockedUrlPattern`/ `--blocked-url-pattern`**
-  Restricts browser's network access by blocking specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Silently detaches from targets with blocked URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns.
+  Restricts browser's network access by blocking specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Silently detaches from targets with blocked URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns. A pattern that uses a regexp group in any component (for example `(127\.\d+\.\d+\.\d+)` in the hostname) is rejected, because it is not enforced on redirects or subresources; use an exact value or a `*`/`:name` wildcard instead.
   - **Type:** array
 
 - **`--allowedUrlPattern`/ `--allowed-url-pattern`**
-  Restricts browser's network access by allowing only specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Requires Chrome 149+. Silently detaches from targets with unallowed URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns.
+  Restricts browser's network access by allowing only specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Requires Chrome 149+. Silently detaches from targets with unallowed URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns. A pattern that uses a regexp group in any component (for example `(127\.\d+\.\d+\.\d+)` in the hostname) is rejected, because it is not enforced on redirects or subresources; use an exact value or a `*`/`:name` wildcard instead.
   - **Type:** array
 
 - **`--performanceCrux`/ `--performance-crux`**

src/config/mcp-options.ts

@@ -12,6 +12,8 @@ import path from 'node:path';
 
 export const DEFAULT_FILESYSTEM_ROOT = [os.tmpdir()];
 
+import {findUnenforceablePattern} from '../utils/url.js';
+
 import {getCategoryOptions} from './category-options.js';
 import {getBrowserOptions} from './browser-options.js';
 
@@ -132,15 +134,39 @@ export const mcpOptions = {
     type: 'array',
     string: true,
     describe:
-      "Restricts browser's network access by blocking specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Silently detaches from targets with blocked URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns.",
+      "Restricts browser's network access by blocking specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Silently detaches from targets with blocked URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns. A pattern that uses a regexp group in any component (for example `(127\\.\\d+\\.\\d+\\.\\d+)` in the hostname) is rejected, because it is not enforced on redirects or subresources; use an exact value or a `*`/`:name` wildcard instead.",
     conflicts: ['allowedUrlPattern'],
+    coerce: (arg: string[] | undefined) => {
+      if (arg === undefined) {
+        return undefined;
+      }
+      const pattern = findUnenforceablePattern(arg);
+      if (pattern) {
+        throw new Error(
+          `Invalid --blockedUrlPattern "${pattern}": a regexp group is not enforced on redirects or subresources. Use an exact value or a "*"/":name" wildcard instead.`,
+        );
+      }
+      return arg;
+    },
   },
   allowedUrlPattern: {
     type: 'array',
     string: true,
     describe:
-      "Restricts browser's network access by allowing only specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Requires Chrome 149+. Silently detaches from targets with unallowed URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns.",
+      "Restricts browser's network access by allowing only specified URL patterns (uses https://urlpattern.spec.whatwg.org/). Requires Chrome 149+. Silently detaches from targets with unallowed URLs upon connection, and blocks runtime requests (including navigations and subresources). Accepts an array of patterns. A pattern that uses a regexp group in any component (for example `(127\\.\\d+\\.\\d+\\.\\d+)` in the hostname) is rejected, because it is not enforced on redirects or subresources; use an exact value or a `*`/`:name` wildcard instead.",
     conflicts: ['blockedUrlPattern'],
+    coerce: (arg: string[] | undefined) => {
+      if (arg === undefined) {
+        return undefined;
+      }
+      const pattern = findUnenforceablePattern(arg);
+      if (pattern) {
+        throw new Error(
+          `Invalid --allowedUrlPattern "${pattern}": a regexp group is not enforced on redirects or subresources. Use an exact value or a "*"/":name" wildcard instead.`,
+        );
+      }
+      return arg;
+    },
   },
   performanceCrux: {
     type: 'boolean',

src/utils/url.ts

@@ -112,6 +112,33 @@ export function isAllowedUrl(
 
 const DISALLOWED_PROTOCOLS = new Set(['javascript:', 'data:', 'vbscript:']);
 
+/**
+ * Finds the first pattern that uses a URLPattern regexp group (for example
+ * `(127\.\d+\.\d+\.\d+)`) in any component -- protocol, username, password,
+ * hostname, port, pathname, search, or hash. Chromium's
+ * `SimpleUrlPatternMatcher::Component::Create` rejects any component whose
+ * `HasRegexGroups()` is true and silently drops the rule, so
+ * `Network.emulateNetworkConditionsByRule` does not enforce these patterns
+ * on redirects or subresources, unlike the initial navigation check. A plain
+ * wildcard (`*`) or named group (`:name`) has no regexp group and is
+ * unaffected.
+ *
+ * @param patterns The `--blockedUrlPattern`/`--allowedUrlPattern` values to check.
+ * @returns The first unenforceable pattern, or undefined if all are safe.
+ * @throws Error if a pattern's syntax is invalid (via `new URLPattern`).
+ */
+export function findUnenforceablePattern(
+  patterns: string[],
+): string | undefined {
+  for (const raw of patterns) {
+    const parsed = new URLPattern(raw);
+    if (parsed.hasRegExpGroups) {
+      return raw;
+    }
+  }
+  return undefined;
+}
+
 /**
  * Validates a URL string by parsing it with `new URL` and checking for disallowed protocols and restricted schemes.
  *

tests/cli.test.ts

@@ -425,6 +425,50 @@ describe('cli args parsing', () => {
     ]);
   });
 
+  it('rejects a blocked-url-pattern with a regexp group', async () => {
+    assert.throws(
+      () =>
+        parseArguments([
+          String.raw`--blocked-url-pattern=*://(127\.\d+\.\d+\.\d+):*/*`,
+        ]),
+      /Invalid --blockedUrlPattern .*a regexp group is not enforced/,
+    );
+
+    assert.throws(
+      () =>
+        parseArguments([
+          '--blocked-url-pattern=https://a.com/*',
+          String.raw`--blocked-url-pattern=*://example.com/(foo|bar)`,
+        ]),
+      /Invalid --blockedUrlPattern .*a regexp group is not enforced/,
+    );
+  });
+
+  it('rejects an allowed-url-pattern with a regexp group', async () => {
+    assert.throws(
+      () =>
+        parseArguments([
+          String.raw`--allowed-url-pattern=*://(127\.\d+\.\d+\.\d+):*/*`,
+        ]),
+      /Invalid --allowedUrlPattern .*a regexp group is not enforced/,
+    );
+
+    assert.throws(
+      () =>
+        parseArguments([
+          '--allowed-url-pattern=https://a.com/*',
+          String.raw`--allowed-url-pattern=(http|https)://example.com/*`,
+        ]),
+      /Invalid --allowedUrlPattern .*a regexp group is not enforced/,
+    );
+  });
+
+  it('rejects a blocked-url-pattern with invalid syntax', async () => {
+    assert.throws(() =>
+      parseArguments(['--blocked-url-pattern=*://example.com/(unterminated']),
+    );
+  });
+
   it('parses source-maps flag', async () => {
     const defaultParsed = parseArguments(['main.js']);
     assert.strictEqual(defaultParsed.sourceMaps, true);

tests/utils/url.test.ts

@@ -7,7 +7,12 @@
 import assert from 'node:assert';
 import {describe, it} from 'node:test';
 
-import {isAllowedUrl, isLocalhost, validateUrl} from '../../src/utils/url.js';
+import {
+  findUnenforceablePattern,
+  isAllowedUrl,
+  isLocalhost,
+  validateUrl,
+} from '../../src/utils/url.js';
 
 describe('isLocalhost', () => {
   it('should return true for valid localhost and loopback URLs', () => {
@@ -429,3 +434,86 @@ describe('isAllowedUrl', () => {
     );
   });
 });
+
+describe('findUnenforceablePattern', () => {
+  it('flags a hostname regexp group', () => {
+    assert.strictEqual(
+      findUnenforceablePattern([String.raw`*://(127\.\d+\.\d+\.\d+):*/*`]),
+      String.raw`*://(127\.\d+\.\d+\.\d+):*/*`,
+    );
+  });
+
+  it('returns the first offending pattern among several', () => {
+    assert.strictEqual(
+      findUnenforceablePattern([
+        '*://127.0.0.1:*/*',
+        '*://*.example.com/*',
+        String.raw`*://(localhost|127\.0\.0\.1):*/*`,
+      ]),
+      String.raw`*://(localhost|127\.0\.0\.1):*/*`,
+    );
+  });
+
+  it('allows an exact hostname', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://127.0.0.1:*/*']),
+      undefined,
+    );
+  });
+
+  it('allows a wildcard hostname', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://*.example.com/*']),
+      undefined,
+    );
+  });
+
+  it('allows a named group hostname', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://:sub.example.com/*']),
+      undefined,
+    );
+  });
+
+  it('flags a regexp group in the pathname', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://example.com/(foo|bar)']),
+      '*://example.com/(foo|bar)',
+    );
+  });
+
+  it('flags a regexp group in the protocol', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['(http|https)://example.com/*']),
+      '(http|https)://example.com/*',
+    );
+  });
+
+  it('flags a regexp group in the port', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://example.com:(80|443)/*']),
+      '*://example.com:(80|443)/*',
+    );
+  });
+
+  it('flags a regexp group in the search or hash', () => {
+    assert.strictEqual(
+      findUnenforceablePattern(['*://example.com/*?(foo|bar)']),
+      '*://example.com/*?(foo|bar)',
+    );
+    assert.strictEqual(
+      findUnenforceablePattern(['*://example.com/*#(foo|bar)']),
+      '*://example.com/*#(foo|bar)',
+    );
+  });
+
+  it('throws for a pattern that fails to construct', () => {
+    assert.throws(() =>
+      findUnenforceablePattern(['*://example.com/(unterminated']),
+    );
+  });
+
+  it('returns undefined for an empty list', () => {
+    assert.strictEqual(findUnenforceablePattern([]), undefined);
+  });
+});