diff --git a/docs/browser-automation.md b/docs/browser-automation.md index 4c230d23..aeb5e617 100644 --- a/docs/browser-automation.md +++ b/docs/browser-automation.md @@ -269,3 +269,5 @@ If CCS reports `unsupported_build`, upgrade Codex and rerun `ccs browser status` - Prefer a dedicated automation user-data dir instead of your everyday browser profile - Do not commit browser paths, secrets, or generated session state to version control - Treat `~/.ccs/config.yaml`, `~/.claude.json`, and the browser user-data directory as local machine state +- `browser_wait_for_event` requires explicit scoping for network request events (`urlIncludes`) and download events (`urlIncludes` or `suggestedFilenameIncludes`) +- Event details redact observed navigation, request, and download URLs before returning them to the MCP caller so observed browser metadata does not expose query strings, fragments, or path-scoped bearer values diff --git a/lib/mcp/ccs-browser-server.cjs b/lib/mcp/ccs-browser-server.cjs index 8032197e..b847cbe3 100755 --- a/lib/mcp/ccs-browser-server.cjs +++ b/lib/mcp/ccs-browser-server.cjs @@ -1209,7 +1209,18 @@ function getTools() { }, event: { type: 'object', - description: 'Required event selector.', + description: + 'Required event selector. request events require urlIncludes; download events require urlIncludes or suggestedFilenameIncludes. URLs in returned details are redacted.', + properties: { + kind: { type: 'string', enum: ['dialog', 'navigation', 'request', 'download'] }, + dialogType: { type: 'string' }, + messageIncludes: { type: 'string' }, + urlIncludes: { type: 'string' }, + method: { type: 'string' }, + suggestedFilenameIncludes: { type: 'string' }, + }, + required: ['kind'], + additionalProperties: false, }, }, required: ['event'], @@ -3348,24 +3359,80 @@ function parseEventCondition(value) { }; } if (value.kind === 'request') { + const urlIncludes = value.urlIncludes ? String(value.urlIncludes) : undefined; + if (!urlIncludes) { + throw new Error('request events require urlIncludes to limit network metadata exposure'); + } return { kind: 'request', - urlIncludes: value.urlIncludes ? String(value.urlIncludes) : undefined, + urlIncludes, method: value.method ? String(value.method) : undefined, }; } if (value.kind === 'download') { + const urlIncludes = value.urlIncludes ? String(value.urlIncludes) : undefined; + const suggestedFilenameIncludes = value.suggestedFilenameIncludes + ? String(value.suggestedFilenameIncludes) + : undefined; + if (!urlIncludes && !suggestedFilenameIncludes) { + throw new Error( + 'download events require urlIncludes or suggestedFilenameIncludes to limit metadata exposure' + ); + } return { kind: 'download', - urlIncludes: value.urlIncludes ? String(value.urlIncludes) : undefined, - suggestedFilenameIncludes: value.suggestedFilenameIncludes - ? String(value.suggestedFilenameIncludes) - : undefined, + urlIncludes, + suggestedFilenameIncludes, }; } throw new Error(`unknown event kind: ${String(value.kind || '')}`); } +function redactUrlForModel(value, options = {}) { + const rawUrl = String(value || ''); + if (!rawUrl) { + return ''; + } + try { + const url = new URL(rawUrl); + if (options.originOnly) { + return url.origin; + } + url.username = ''; + url.password = ''; + url.search = ''; + url.hash = ''; + return url.toString(); + } catch { + return '[redacted-url]'; + } +} + +function redactObservedEvent(event, observed) { + if (!observed || typeof observed !== 'object') { + return observed; + } + if (event.kind === 'request') { + return { + url: redactUrlForModel(observed.url, { originOnly: true }), + method: observed.method || '', + }; + } + if (event.kind === 'download') { + return { + url: redactUrlForModel(observed.url, { originOnly: true }), + suggestedFilename: observed.suggestedFilename || '', + }; + } + if (event.kind === 'navigation' && Object.prototype.hasOwnProperty.call(observed, 'url')) { + return { + ...observed, + url: redactUrlForModel(observed.url, { originOnly: true }), + }; + } + return observed; +} + function matchesObservedEvent(event, observed) { if (event.kind === 'dialog') { return ( @@ -4819,7 +4886,8 @@ async function handleWaitForEvent(toolArgs) { ); const event = parseEventCondition(toolArgs.event); const observed = await waitForMatchingEvent({ page, pageIndex, timeoutMs, event }); - return `pageIndex: ${pageIndex}\nevent: ${event.kind}\nstatus: observed\ndetail: ${JSON.stringify(observed)}`; + const safeObserved = redactObservedEvent(event, observed); + return `pageIndex: ${pageIndex}\nevent: ${event.kind}\nstatus: observed\ndetail: ${JSON.stringify(safeObserved)}`; } function findInterceptRuleIndex(ruleId) { diff --git a/tests/unit/hooks/browser-mcp-advanced-interactions.test.ts b/tests/unit/hooks/browser-mcp-advanced-interactions.test.ts index a06c3198..00306f6b 100644 --- a/tests/unit/hooks/browser-mcp-advanced-interactions.test.ts +++ b/tests/unit/hooks/browser-mcp-advanced-interactions.test.ts @@ -750,7 +750,7 @@ describe('ccs-browser MCP server - advanced interactions', () => { ); }); - it('ignores child-frame navigations when waiting for a page navigation event', async () => { + it('ignores child-frame navigations and redacts matched navigation URLs', async () => { const responses = await runMcpRequests( [ { @@ -759,8 +759,13 @@ describe('ccs-browser MCP server - advanced interactions', () => { currentUrl: 'https://example.com/', events: { navigations: [ - { url: 'https://example.com/embedded-checkout', parentId: 'frame-1' }, - { url: 'https://example.com/checkout' }, + { + url: 'https://example.com/embedded-checkout?code=child-secret', + parentId: 'frame-1', + }, + { + url: 'https://example.com/checkout/session-token-123?code=secret-state#done', + }, ], }, }, @@ -774,7 +779,7 @@ describe('ccs-browser MCP server - advanced interactions', () => { name: 'browser_wait_for_event', arguments: { timeoutMs: 1000, - event: { kind: 'navigation', urlIncludes: '/checkout' }, + event: { kind: 'navigation', urlIncludes: '/checkout/session-token-123' }, }, }, }, @@ -783,8 +788,69 @@ describe('ccs-browser MCP server - advanced interactions', () => { const text = getResponseText(responses.find((message) => message.id === 58)); expect(text).toContain('status: observed'); - expect(text).toContain('"url":"https://example.com/checkout"'); + expect(text).toContain('"url":"https://example.com"'); expect(text).not.toContain('embedded-checkout'); + expect(text).not.toContain('session-token-123'); + expect(text).not.toContain('secret-state'); + }); + + it('requires request event URL scoping and redacts observed request URLs', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Request Page', + currentUrl: 'https://example.com/', + events: { + requests: [ + { + url: 'https://api.example.com/oauth/callback?code=secret-code&state=secret-state', + method: 'GET', + }, + ], + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 59, + method: 'tools/call', + params: { + name: 'browser_wait_for_event', + arguments: { + timeoutMs: 1000, + event: { kind: 'request' }, + }, + }, + }, + { + jsonrpc: '2.0', + id: 60, + method: 'tools/call', + params: { + name: 'browser_wait_for_event', + arguments: { + timeoutMs: 1000, + event: { kind: 'request', urlIncludes: '/oauth/callback' }, + }, + }, + }, + ] + ); + + const unscopedText = getResponseText(responses.find((message) => message.id === 59)); + expect(unscopedText).toContain( + 'Browser MCP failed: request events require urlIncludes to limit network metadata exposure' + ); + + const scopedText = getResponseText(responses.find((message) => message.id === 60)); + expect(scopedText).toContain('status: observed'); + expect(scopedText).toContain('"url":"https://api.example.com"'); + expect(scopedText).toContain('"method":"GET"'); + expect(scopedText).not.toContain('secret-code'); + expect(scopedText).not.toContain('secret-state'); + expect(scopedText).not.toContain('/oauth/callback'); }); it('filters download events by the selected page frame when pageIndex is provided', async () => { diff --git a/tests/unit/hooks/browser-mcp-downloads-and-files.test.ts b/tests/unit/hooks/browser-mcp-downloads-and-files.test.ts index ebf42139..fbb175c9 100644 --- a/tests/unit/hooks/browser-mcp-downloads-and-files.test.ts +++ b/tests/unit/hooks/browser-mcp-downloads-and-files.test.ts @@ -262,6 +262,52 @@ describe('ccs-browser MCP server - downloads and file inputs', () => { expect(getResponseText(responses.find((message) => message.id === 62))).toContain( 'status: observed' ); + const text = getResponseText(responses.find((message) => message.id === 62)); + expect(text).toContain('status: observed'); + expect(text).toContain('"url":"https://example.com"'); + expect(text).not.toContain('/files/export.zip'); + }); + + it('requires download event scoping before observing browser-level download URLs', async () => { + const responses = await runMcpRequests( + [ + { + id: 'page-1', + title: 'Event Page', + currentUrl: 'https://example.com/', + events: { + downloads: [ + { + guid: 'download-guid-3', + url: 'https://example.com/files/private.zip?signature=secret', + suggestedFilename: 'private.zip', + }, + ], + }, + }, + ], + [ + { + jsonrpc: '2.0', + id: 63, + method: 'tools/call', + params: { + name: 'browser_wait_for_event', + arguments: { + timeoutMs: 1000, + event: { kind: 'download' }, + }, + }, + }, + ], + { responseTimeoutMs: 12000 } + ); + + const text = getResponseText(responses.find((message) => message.id === 63)); + expect(text).toContain( + 'Browser MCP failed: download events require urlIncludes or suggestedFilenameIncludes to limit metadata exposure' + ); + expect(text).not.toContain('signature=secret'); }); it('sets files on selected-page, frameSelector, and pierceShadow file inputs', async () => {