From bbf5c2fe95d67a93c6fa64cbb11a7383296a8bea Mon Sep 17 00:00:00 2001 From: tony-shi Date: Wed, 25 Mar 2026 23:26:00 +0800 Subject: [PATCH] fix(browser): detect embedded URLs in query params to prevent allowedDomains bypass (#23225) Co-authored-by: cynthialong0-0 <82900738+cynthialong0-0@users.noreply.github.com> --- .../src/agents/browser/browserManager.test.ts | 70 +++++++++++++++++++ .../core/src/agents/browser/browserManager.ts | 60 ++++++++++++---- 2 files changed, 118 insertions(+), 12 deletions(-) diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts index 303c07288d..c38457e4aa 100644 --- a/packages/core/src/agents/browser/browserManager.test.ts +++ b/packages/core/src/agents/browser/browserManager.test.ts @@ -272,6 +272,76 @@ describe('BrowserManager', () => { expect(result.isError).toBe(true); expect((result.content || [])[0]?.text).toContain('not permitted'); }); + + it('should block proxy URL with embedded disallowed domain in query params', async () => { + const restrictedConfig = makeFakeConfig({ + agents: { + browser: { + allowedDomains: ['*.google.com'], + }, + }, + }); + const manager = new BrowserManager(restrictedConfig); + const result = await manager.callTool('new_page', { + url: 'https://translate.google.com/translate?sl=en&tl=en&u=https://blocked.org/page', + }); + + expect(result.isError).toBe(true); + expect((result.content || [])[0]?.text).toContain( + 'an embedded URL targets a disallowed domain', + ); + }); + + it('should block proxy URL with embedded disallowed domain in URL fragment (hash)', async () => { + const restrictedConfig = makeFakeConfig({ + agents: { + browser: { + allowedDomains: ['*.google.com'], + }, + }, + }); + const manager = new BrowserManager(restrictedConfig); + const result = await manager.callTool('new_page', { + url: 'https://translate.google.com/#view=home&op=translate&sl=en&tl=zh-CN&u=https://blocked.org', + }); + + expect(result.isError).toBe(true); + expect((result.content || [])[0]?.text).toContain( + 'an embedded URL targets a disallowed domain', + ); + }); + + it('should allow proxy URL when embedded domain is also allowed', async () => { + const restrictedConfig = makeFakeConfig({ + agents: { + browser: { + allowedDomains: ['*.google.com', 'github.com'], + }, + }, + }); + const manager = new BrowserManager(restrictedConfig); + const result = await manager.callTool('new_page', { + url: 'https://translate.google.com/translate?u=https://github.com/repo', + }); + + expect(result.isError).toBe(false); + }); + + it('should allow navigation to allowed domain without proxy params', async () => { + const restrictedConfig = makeFakeConfig({ + agents: { + browser: { + allowedDomains: ['*.google.com'], + }, + }, + }); + const manager = new BrowserManager(restrictedConfig); + const result = await manager.callTool('new_page', { + url: 'https://translate.google.com/?sl=en&tl=zh', + }); + + expect(result.isError).toBe(false); + }); }); describe('MCP connection', () => { diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts index cc059feea3..4eb9c2b19c 100644 --- a/packages/core/src/agents/browser/browserManager.ts +++ b/packages/core/src/agents/browser/browserManager.ts @@ -610,29 +610,65 @@ export class BrowserManager { try { const parsedUrl = new URL(url); - const urlHostname = parsedUrl.hostname.replace(/\.$/, ''); + const urlHostname = parsedUrl.hostname; - for (const domainPattern of allowedDomains) { - if (domainPattern.startsWith('*.')) { - const baseDomain = domainPattern.substring(2); + if (!this.isDomainAllowed(urlHostname, allowedDomains)) { + // If none matched, then deny + return `Tool '${toolName}' is not permitted for the requested URL/domain based on your current browser settings.`; + } + + // Check query parameters for embedded URLs that could bypass domain + // restrictions via proxy services (e.g. translate.google.com/translate?u=BLOCKED). + const paramsToCheck = [ + ...parsedUrl.searchParams.values(), + // Also check fragments which might contain query-like params + ...new URLSearchParams(parsedUrl.hash.replace(/^#/, '')).values(), + ]; + for (const paramValue of paramsToCheck) { + try { + const embeddedUrl = new URL(paramValue); if ( - urlHostname === baseDomain || - urlHostname.endsWith(`.${baseDomain}`) + embeddedUrl.protocol === 'http:' || + embeddedUrl.protocol === 'https:' ) { - return undefined; - } - } else { - if (urlHostname === domainPattern) { - return undefined; + const embeddedHostname = embeddedUrl.hostname.replace(/\.$/, ''); + if (!this.isDomainAllowed(embeddedHostname, allowedDomains)) { + return `Tool '${toolName}' is not permitted: an embedded URL targets a disallowed domain.`; + } } + } catch { + // Not a valid URL, skip. } } + + return undefined; } catch { return `Invalid URL: Malformed URL string.`; } + } + /** + * Checks whether a hostname matches any pattern in the allowed domains list. + */ + private isDomainAllowed(hostname: string, allowedDomains: string[]): boolean { + const normalized = hostname.replace(/\.$/, ''); + for (const domainPattern of allowedDomains) { + if (domainPattern.startsWith('*.')) { + const baseDomain = domainPattern.substring(2); + if ( + normalized === baseDomain || + normalized.endsWith(`.${baseDomain}`) + ) { + return true; + } + } else { + if (normalized === domainPattern) { + return true; + } + } + } // If none matched, then deny - return `Tool '${toolName}' is not permitted for the requested URL/domain based on your current browser settings.`; + return false; } /**