-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat: support allowedUrlPattern & blockedUrlPattern Options #2037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
df09ac6
790b708
6b21527
e17cf83
82fc71a
688644c
1bc3ce2
c6392c1
2250c76
e6a761e
6f1034f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,8 @@ interface McpContextOptions { | |
| experimentalIncludeAllPages?: boolean; | ||
| // Whether CrUX data should be fetched. | ||
| performanceCrux: boolean; | ||
| // Whether allowlist/blocklist is configured. | ||
| hasNetworkBlockOrAllowlist?: boolean; | ||
| } | ||
|
|
||
| const DEFAULT_TIMEOUT = 5_000; | ||
|
|
@@ -345,24 +347,27 @@ export class McpContext implements Context { | |
| const mcpPage = this.#getMcpPage(page); | ||
| const newSettings: EmulationSettings = {...mcpPage.emulationSettings}; | ||
|
|
||
| if (!options.networkConditions) { | ||
| await page.emulateNetworkConditions(null); | ||
| delete newSettings.networkConditions; | ||
| } else if (options.networkConditions === 'Offline') { | ||
| await page.emulateNetworkConditions({ | ||
| offline: true, | ||
| download: 0, | ||
| upload: 0, | ||
| latency: 0, | ||
| }); | ||
| newSettings.networkConditions = 'Offline'; | ||
| } else if (options.networkConditions in PredefinedNetworkConditions) { | ||
| const networkCondition = | ||
| PredefinedNetworkConditions[ | ||
| options.networkConditions as keyof typeof PredefinedNetworkConditions | ||
| ]; | ||
| await page.emulateNetworkConditions(networkCondition); | ||
| newSettings.networkConditions = options.networkConditions; | ||
| // Skip network emulation if blocklist/allowlist is configured, as it is rejected by Puppeteer. | ||
| if (!this.#options.hasNetworkBlockOrAllowlist) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we throw if we try to set emulation? Reason is I try to emulate network conditions and this just silently ignores it, an LLM most likely will be confused. |
||
| if (!options.networkConditions) { | ||
| await page.emulateNetworkConditions(null); | ||
| delete newSettings.networkConditions; | ||
| } else if (options.networkConditions === 'Offline') { | ||
| await page.emulateNetworkConditions({ | ||
| offline: true, | ||
| download: 0, | ||
| upload: 0, | ||
| latency: 0, | ||
| }); | ||
| newSettings.networkConditions = 'Offline'; | ||
| } else if (options.networkConditions in PredefinedNetworkConditions) { | ||
| const networkCondition = | ||
| PredefinedNetworkConditions[ | ||
| options.networkConditions as keyof typeof PredefinedNetworkConditions | ||
| ]; | ||
| await page.emulateNetworkConditions(networkCondition); | ||
| newSettings.networkConditions = options.networkConditions; | ||
| } | ||
| } | ||
|
|
||
| const secondarySession = this.getDevToolsUniverse(mcpPage)?.session; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ import {executablePath} from 'puppeteer'; | |
|
|
||
| import {detectDisplay, ensureBrowserConnected, launch} from '../src/browser.js'; | ||
|
|
||
| import {serverHooks} from './server.js'; | ||
|
|
||
| describe('browser', () => { | ||
| it('detects display does not crash', () => { | ||
| detectDisplay(); | ||
|
|
@@ -100,4 +102,88 @@ describe('browser', () => { | |
| await browser.close(); | ||
| } | ||
| }); | ||
|
|
||
| describe('Blocking', () => { | ||
| const server = serverHooks(); | ||
|
|
||
| it('blocks URLs in blocklist', async () => { | ||
| server.addHtmlRoute('/allowed.html', '<html><body>Allowed</body></html>'); | ||
| server.addHtmlRoute('/blocked.html', '<html><body>Blocked</body></html>'); | ||
|
|
||
| const browser = await launch({ | ||
| headless: true, | ||
| isolated: true, | ||
| executablePath: await executablePath(), | ||
| devtools: false, | ||
| blocklist: ['*://*:*/blocked.html'], | ||
| }); | ||
| try { | ||
| const page = await browser.newPage(); | ||
|
|
||
| // Access allowed URL | ||
| await page.goto(server.getRoute('/allowed.html')); | ||
| const content = await page.evaluate(() => document.body.textContent); | ||
| assert.strictEqual(content, 'Allowed'); | ||
|
|
||
| // Fetch of blocked URL from the page | ||
| const fetchResult = await page.evaluate(async url => { | ||
| try { | ||
| await fetch(url); | ||
| return 'SUCCESS'; | ||
| } catch (err) { | ||
| return err instanceof Error ? err.message : String(err); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: We can just return true or false as the message may change. |
||
| } | ||
| }, server.getRoute('/blocked.html')); | ||
|
|
||
| assert.strictEqual(fetchResult, 'Failed to fetch'); | ||
| } finally { | ||
| await browser.close(); | ||
| } | ||
| }); | ||
|
|
||
| it( | ||
| 'blocks URLs not in allowlist', | ||
| {skip: 'Requires Chrome 149 or greater'}, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this also still needed? |
||
| async () => { | ||
| server.addHtmlRoute( | ||
| '/allowed.html', | ||
| '<html><body>Allowed</body></html>', | ||
| ); | ||
| server.addHtmlRoute( | ||
| '/blocked.html', | ||
| '<html><body>Blocked</body></html>', | ||
| ); | ||
|
|
||
| const browser = await launch({ | ||
| headless: true, | ||
| isolated: true, | ||
| executablePath: await executablePath(), | ||
| devtools: false, | ||
| allowlist: ['*://*/allowed.html'], | ||
| }); | ||
| try { | ||
| const page = await browser.newPage(); | ||
|
|
||
| // Access allowed URL | ||
| await page.goto(server.getRoute('/allowed.html')); | ||
| const content = await page.evaluate(() => document.body.textContent); | ||
| assert.strictEqual(content, 'Allowed'); | ||
|
|
||
| // Fetch of blocked URL from the page | ||
| const fetchResult = await page.evaluate(async url => { | ||
| try { | ||
| await fetch(url); | ||
| return 'SUCCESS'; | ||
| } catch (err) { | ||
| return err instanceof Error ? err.message : String(err); | ||
| } | ||
| }, server.getRoute('/blocked.html')); | ||
|
|
||
| assert.strictEqual(fetchResult, 'Failed to fetch'); | ||
| } finally { | ||
| await browser.close(); | ||
| } | ||
| }, | ||
| ); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this this is already stable https://chromestatus.com/roadmap
And that is the version pinned by Puppeteer.