diff --git a/package-lock.json b/package-lock.json index 0fcb6418d..7760e8944 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,7 +46,6 @@ "sinon": "^22.0.0", "typescript": "^6.0.2", "typescript-eslint": "^8.43.0", - "urlpattern-polyfill": "^10.1.0", "yargs": "18.0.0" }, "engines": { @@ -8353,13 +8352,6 @@ "punycode": "^2.1.0" } }, - "node_modules/urlpattern-polyfill": { - "version": "10.1.0", - "resolved": "https://registry.npmjs.org/urlpattern-polyfill/-/urlpattern-polyfill-10.1.0.tgz", - "integrity": "sha512-IGjKp/o0NL3Bso1PymYURCJxMPNAf/ILOpendP9f5B6e1rTJgdgiOvgfoT8VxCAdY+Wisb9uhGaJJf3yZ2V9nw==", - "dev": true, - "license": "MIT" - }, "node_modules/vary": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/vary/-/vary-1.1.2.tgz", diff --git a/package.json b/package.json index f1c690281..18d604b2a 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,6 @@ "sinon": "^22.0.0", "typescript": "^6.0.2", "typescript-eslint": "^8.43.0", - "urlpattern-polyfill": "^10.1.0", "yargs": "18.0.0" }, "engines": { diff --git a/src/bin/chrome-devtools-mcp-cli-options.ts b/src/bin/chrome-devtools-mcp-cli-options.ts index 43d5c2360..1568d2fda 100644 --- a/src/bin/chrome-devtools-mcp-cli-options.ts +++ b/src/bin/chrome-devtools-mcp-cli-options.ts @@ -181,11 +181,6 @@ export const cliOptions = { describe: 'Whether to include all kinds of pages such as webviews or background pages as pages.', }, - experimentalNavigationAllowlist: { - type: 'boolean', - describe: 'Whether to enable navigation allowlist tool parameter.', - hidden: true, - }, experimentalInteropTools: { type: 'boolean', describe: 'Whether to enable interoperability tools', diff --git a/src/third_party/index.ts b/src/third_party/index.ts index da1907765..2642a089a 100644 --- a/src/third_party/index.ts +++ b/src/third_party/index.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import 'urlpattern-polyfill'; import 'core-js/modules/es.promise.with-resolvers.js'; import 'core-js/modules/es.set.union.v2.js'; import 'core-js/proposals/iterator-helpers.js'; diff --git a/src/tools/pages.ts b/src/tools/pages.ts index 0a74aa228..a42cfb915 100644 --- a/src/tools/pages.ts +++ b/src/tools/pages.ts @@ -5,11 +5,10 @@ */ import {logger} from '../logger.js'; -import type {CdpPage, Dialog, HTTPRequest} from '../third_party/index.js'; +import type {CdpPage, Dialog} from '../third_party/index.js'; import {zod} from '../third_party/index.js'; import {ToolCategory} from './categories.js'; -import type {ContextPage} from './ToolDefinition.js'; import { CLOSE_PAGE_ERROR, definePageTool, @@ -17,64 +16,6 @@ import { timeoutSchema, } from './ToolDefinition.js'; -async function navigateWithInterception( - page: ContextPage, - action: () => Promise, - allowListString?: string, - timeout?: number, -): Promise { - const allowList = allowListString - ? allowListString.split(',').map((p: string) => new URLPattern(p.trim())) - : undefined; - - const requestHandler = (interceptedRequest: HTTPRequest) => { - if (!interceptedRequest.isNavigationRequest()) { - void interceptedRequest.continue(); - return; - } - const requestUrl = interceptedRequest.url(); - const isAllowed = allowList!.some((pattern: URLPattern) => - pattern.test(requestUrl), - ); - - if (isAllowed) { - void interceptedRequest.continue(); - } else { - logger?.(`Blocking request to: ${requestUrl}`); - void interceptedRequest.abort('blockedbyclient'); - } - }; - - const cleanupInterception = async () => { - if (allowList) { - page.pptrPage.off('request', requestHandler); - await page.pptrPage.setRequestInterception(false).catch(error => { - logger?.(`Failed to disable request interception`, error); - }); - } - }; - - if (allowList) { - await page.pptrPage.setRequestInterception(true); - page.pptrPage.on('request', requestHandler); - } - - try { - await page.waitForEventsAfterAction( - async () => { - try { - await action(); - } finally { - await cleanupInterception(); - } - }, - {timeout}, - ); - } finally { - await cleanupInterception(); - } -} - export const listPages = defineTool(args => { return { name: 'list_pages', @@ -179,16 +120,6 @@ export const newPage = defineTool(args => { 'Pages in the same browser context share cookies and storage. ' + 'Pages in different browser contexts are fully isolated.', ), - ...(args?.experimentalNavigationAllowlist - ? { - allowList: zod - .string() - .optional() - .describe( - 'Optional comma-separated list of URL patterns to allow. If provided, all other navigations will be blocked.', - ), - } - : {}), ...timeoutSchema, }, blockedByDialog: false, @@ -199,15 +130,9 @@ export const newPage = defineTool(args => { request.params.isolatedContext, ); - await navigateWithInterception( - page, - () => - page.pptrPage.goto(request.params.url, { - timeout: request.params.timeout, - }), - request.params.allowList, - request.params.timeout, - ); + await page.pptrPage.goto(request.params.url, { + timeout: request.params.timeout, + }); response.setIncludePages(true); response.setListThirdPartyDeveloperTools(); @@ -247,16 +172,7 @@ export const navigatePage = definePageTool(args => { .describe( 'A JavaScript script to be executed on each new document before any other scripts for the next navigation.', ), - ...(args?.experimentalNavigationAllowlist - ? { - allowList: zod - .string() - .optional() - .describe( - 'Optional comma-separated list of URL patterns to allow. If provided, all other navigations will be blocked.', - ), - } - : {}), + ...timeoutSchema, }, blockedByDialog: false, @@ -301,71 +217,60 @@ export const navigatePage = definePageTool(args => { page.pptrPage.on('dialog', dialogHandler); try { - await navigateWithInterception( - page, - async () => { - switch (request.params.type) { - case 'url': - if (!request.params.url) { - throw new Error( - 'A URL is required for navigation of type=url.', - ); - } - try { - await page.pptrPage.goto(request.params.url, options); - response.appendResponseLine( - `Successfully navigated to ${request.params.url}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate in the selected page: ${error.message}.`, - ); - } - break; - case 'back': - try { - await page.pptrPage.goBack(options); - response.appendResponseLine( - `Successfully navigated back to ${page.pptrPage.url()}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate back in the selected page: ${error.message}.`, - ); - } - break; - case 'forward': - try { - await page.pptrPage.goForward(options); - response.appendResponseLine( - `Successfully navigated forward to ${page.pptrPage.url()}.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to navigate forward in the selected page: ${error.message}.`, - ); - } - break; - case 'reload': - try { - await page.pptrPage.reload({ - ...options, - ignoreCache: request.params.ignoreCache, - }); - response.appendResponseLine( - `Successfully reloaded the page.`, - ); - } catch (error) { - response.appendResponseLine( - `Unable to reload the selected page: ${error.message}.`, - ); - } - break; + switch (request.params.type) { + case 'url': + if (!request.params.url) { + throw new Error('A URL is required for navigation of type=url.'); } - }, - request.params.allowList, - request.params.timeout, - ); + try { + await page.pptrPage.goto(request.params.url, options); + response.appendResponseLine( + `Successfully navigated to ${request.params.url}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate in the selected page: ${error.message}.`, + ); + } + break; + case 'back': + try { + await page.pptrPage.goBack(options); + response.appendResponseLine( + `Successfully navigated back to ${page.pptrPage.url()}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate back in the selected page: ${error.message}.`, + ); + } + break; + case 'forward': + try { + await page.pptrPage.goForward(options); + response.appendResponseLine( + `Successfully navigated forward to ${page.pptrPage.url()}.`, + ); + } catch (error) { + response.appendResponseLine( + `Unable to navigate forward in the selected page: ${error.message}.`, + ); + } + break; + case 'reload': + try { + await page.pptrPage.reload({ + ...options, + ignoreCache: request.params.ignoreCache, + }); + response.appendResponseLine(`Successfully reloaded the page.`); + } catch (error) { + response.appendResponseLine( + `Unable to reload the selected page: ${error.message}.`, + ); + } + break; + } } finally { page.pptrPage.off('dialog', dialogHandler); if (initScriptId) { diff --git a/tests/tools/pagesNavigateAllowlist.test.ts b/tests/tools/pagesNavigateAllowlist.test.ts deleted file mode 100644 index 98da02d79..000000000 --- a/tests/tools/pagesNavigateAllowlist.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import assert from 'node:assert'; -import {describe, it} from 'node:test'; - -import type {ParsedArguments} from '../../src/bin/chrome-devtools-mcp-cli-options.js'; -import {navigatePage} from '../../src/tools/pages.js'; -import {serverHooks} from '../server.js'; -import {withMcpContext} from '../utils.js'; - -describe('pages allowList', () => { - const server = serverHooks(); - const args = {experimentalNavigationAllowlist: true} as ParsedArguments; - - it('navigates through redirects when all URLs are allowed', async () => { - server.addRoute('/a.html', (_req, res) => { - res.writeHead(302, {Location: '/b.html'}); - res.end(); - }); - server.addRoute('/b.html', (_req, res) => { - res.writeHead(302, {Location: '/c.html'}); - res.end(); - }); - server.addHtmlRoute( - '/c.html', - '

Final Destination

', - ); - - await withMcpContext(async (response, context) => { - const page = context.getSelectedMcpPage(); - const baseUrl = server.baseUrl; - const allowList = `${baseUrl}/a.html,${baseUrl}/b.html,${baseUrl}/c.html`; - - await navigatePage(args).handler( - { - params: { - url: `${baseUrl}/a.html`, - allowList, - }, - page, - }, - response, - context, - ); - - assert.strictEqual(page.pptrPage.url(), `${baseUrl}/c.html`); - const content = await page.pptrPage.evaluate( - () => document.querySelector('h1')?.textContent, - ); - assert.strictEqual(content, 'Final Destination'); - }); - }); - - it('blocks navigation when a redirect target is not allowed', async () => { - server.addRoute('/a.html', (_req, res) => { - res.writeHead(302, {Location: '/b.html'}); - res.end(); - }); - server.addRoute('/b.html', (_req, res) => { - res.writeHead(302, {Location: '/c.html'}); - res.end(); - }); - server.addHtmlRoute( - '/c.html', - '

Final Destination

', - ); - - await withMcpContext(async (response, context) => { - const page = context.getSelectedMcpPage(); - const baseUrl = server.baseUrl; - // b.html is missing from allowList - const allowList = `${baseUrl}/a.html,${baseUrl}/c.html`; - - await navigatePage(args).handler( - { - params: { - url: `${baseUrl}/a.html`, - allowList, - timeout: 2000, // Short timeout for failure - }, - page, - }, - response, - context, - ); - - // The navigation to b.html should be blocked. - // Puppeteer's goto will likely throw a timeout or net::ERR_ABORTED error. - const url = page.pptrPage.url(); - assert.notStrictEqual(url, `${baseUrl}/c.html`); - assert.ok( - response.responseLines.some(line => - line.includes('Unable to navigate'), - ), - ); - }); - }); -});