diff --git a/lib/mcp/ccs-browser-server.cjs b/lib/mcp/ccs-browser-server.cjs index b847cbe3..bfb0d2f2 100755 --- a/lib/mcp/ccs-browser-server.cjs +++ b/lib/mcp/ccs-browser-server.cjs @@ -5492,7 +5492,10 @@ function buildRecordingInstallExpression(recordingPayload) { const recordingPayload = JSON.parse(${JSON.stringify(JSON.stringify(recordingPayload))}); const existing = globalThis.__CCS_BROWSER_RECORDING_RECORDER__; if (existing && existing.installed === true) { - return { installed: true }; + if (typeof existing.teardown === 'function') { + existing.teardown(); + } + delete globalThis.__CCS_BROWSER_RECORDING_RECORDER__; } const events = Array.isArray(recordingPayload.events) ? [...recordingPayload.events] : []; @@ -5524,8 +5527,42 @@ function buildRecordingInstallExpression(recordingPayload) { timestamp: Date.now(), }); }; + const sensitiveInputTypes = new Set(['password', 'hidden']); + const sensitiveAttributePattern = /(?:pass(?:word|code|phrase)?|pwd|secret|token|api[-_ ]?key|access[-_ ]?key|private[-_ ]?key|credential|otp|one[-_ ]?time[-_ ]?(?:code|password)|verif(?:ication)?[-_ ]?code|security[-_ ]?code|pin|auth(?:orization)?[-_ ]?(?:code|token)|mfa|2fa)/i; + const sensitiveAutocompleteValues = new Set([ + 'current-password', + 'new-password', + 'one-time-code', + 'cc-number', + 'cc-csc', + ]); + const isSensitiveTextTarget = (target) => { + if (!target || !(target instanceof Element)) { + return false; + } + if (target instanceof HTMLInputElement) { + const inputType = String(target.type || '').toLowerCase(); + if (sensitiveInputTypes.has(inputType)) { + return true; + } + } + const autocompleteTokens = String(target.getAttribute('autocomplete') || '') + .toLowerCase() + .split(/\s+/) + .filter(Boolean); + if (autocompleteTokens.some((token) => sensitiveAutocompleteValues.has(token))) { + return true; + } + const attributesToInspect = ['id', 'name', 'placeholder', 'aria-label', 'data-testid']; + return attributesToInspect.some((attributeName) => + sensitiveAttributePattern.test(String(target.getAttribute(attributeName) || '')) + ); + }; const onInput = (event) => { const target = event.target; + if (isSensitiveTextTarget(target)) { + return; + } let text = ''; if (target instanceof HTMLInputElement || target instanceof HTMLTextAreaElement) { text = target.value; @@ -5536,7 +5573,19 @@ function buildRecordingInstallExpression(recordingPayload) { } pushEvent({ kind: 'type', selector: getSelector(target), text, timestamp: Date.now() }); }; + const isRecordableKey = (event) => { + if (isSensitiveTextTarget(event.target)) { + return false; + } + if (typeof event.key !== 'string' || event.key.length !== 1) { + return true; + } + return event.altKey === true || event.ctrlKey === true || event.metaKey === true; + }; const onKeyDown = (event) => { + if (!isRecordableKey(event)) { + return; + } const modifiers = []; if (event.altKey) modifiers.push('Alt'); if (event.ctrlKey) modifiers.push('Control'); @@ -5604,7 +5653,19 @@ async function finalizeRecordingCapture(session) { }; const response = await sendCdpCommand(page, 'Runtime.evaluate', { - expression: `(() => globalThis.__CCS_BROWSER_RECORDING_RECORDER__ || { events: [], warnings: [] })()`, + expression: `(() => { + const recorder = globalThis.__CCS_BROWSER_RECORDING_RECORDER__; + if (!recorder) { + return { events: [], warnings: [] }; + } + const events = Array.isArray(recorder.events) ? [...recorder.events] : []; + const warnings = Array.isArray(recorder.warnings) ? [...recorder.warnings] : []; + if (typeof recorder.teardown === 'function') { + recorder.teardown(); + } + delete globalThis.__CCS_BROWSER_RECORDING_RECORDER__; + return { events, warnings }; + })()`, returnByValue: true, awaitPromise: true, }); @@ -5624,6 +5685,28 @@ async function finalizeRecordingCapture(session) { session.warnings = warnings.map((warning) => String(warning.message || warning)); } +async function teardownRecordingCapture(session) { + if (!session || !session.pageWebSocketDebuggerUrl) { + return; + } + const page = { + id: session.pageId, + webSocketDebuggerUrl: session.pageWebSocketDebuggerUrl, + }; + await sendCdpCommand(page, 'Runtime.evaluate', { + expression: `(() => { + const recorder = globalThis.__CCS_BROWSER_RECORDING_RECORDER__; + if (recorder && typeof recorder.teardown === 'function') { + recorder.teardown(); + } + delete globalThis.__CCS_BROWSER_RECORDING_RECORDER__; + return { installed: false }; + })()`, + returnByValue: true, + awaitPromise: true, + }); +} + async function handleStartRecording(toolArgs) { if (activeRecordingSession) { throw new Error('recording already active'); @@ -5674,6 +5757,11 @@ async function handleStopRecording() { } catch (error) { finalizeError = error instanceof Error ? error : new Error(String(error)); session.warnings.push(`recording capture finalization failed: ${finalizeError.message}`); + try { + await teardownRecordingCapture(session); + } catch { + // Preserve the original finalization failure for the caller. + } } session.status = 'stopped'; session.stoppedAt = new Date().toISOString(); @@ -5694,6 +5782,12 @@ async function handleClearRecording() { if (!latestRecordingSession && !activeRecordingSession) { throw new Error('no recording available'); } + const session = activeRecordingSession || latestRecordingSession; + try { + await teardownRecordingCapture(session); + } catch { + // Clearing session-local recording state should still succeed if the page is already gone. + } activeRecordingSession = null; latestRecordingSession = null; return 'status: cleared'; diff --git a/tests/unit/hooks/browser-mcp-recording-and-replay.test.ts b/tests/unit/hooks/browser-mcp-recording-and-replay.test.ts index 782108d4..82fa355a 100644 --- a/tests/unit/hooks/browser-mcp-recording-and-replay.test.ts +++ b/tests/unit/hooks/browser-mcp-recording-and-replay.test.ts @@ -96,17 +96,19 @@ describe('ccs-browser MCP server - recording and replay', () => { }); it('cleans up recording state when stop finalization fails', async () => { - const responses = await runMcpRequests( - [ - { - id: 'page-1', - title: 'Broken Stop Recording Page', - currentUrl: 'https://example.com/broken-stop-recording', - recording: { - finalizeError: 'recording finalize failed', - }, + const pages: MockPageState[] = [ + { + id: 'page-1', + title: 'Broken Stop Recording Page', + currentUrl: 'https://example.com/broken-stop-recording', + recording: { + finalizeError: 'recording finalize failed', }, - ], + }, + ]; + + const stopResponses = await runMcpRequests( + pages, [ { jsonrpc: '2.0', @@ -120,19 +122,25 @@ describe('ccs-browser MCP server - recording and replay', () => { method: 'tools/call', params: { name: 'browser_stop_recording', arguments: {} }, }, - { - jsonrpc: '2.0', - id: 1019_3, - method: 'tools/call', - params: { name: 'browser_start_recording', arguments: {} }, - }, ] ); - expect(getResponseText(responses.find((message) => message.id === 1019_2))).toContain( + expect(getResponseText(stopResponses.find((message) => message.id === 1019_2))).toContain( 'recording finalize failed' ); - expect(getResponseText(responses.find((message) => message.id === 1019_3))).toContain( + expect(pages[0].recording?.installed).toBe(false); + expect(pages[0].recording?.teardownCalls).toBe(1); + + const restartResponses = await runMcpRequests(pages, [ + { + jsonrpc: '2.0', + id: 1019_3, + method: 'tools/call', + params: { name: 'browser_start_recording', arguments: {} }, + }, + ]); + + expect(getResponseText(restartResponses.find((message) => message.id === 1019_3))).toContain( 'status: recording' ); }); diff --git a/tests/unit/hooks/browser-mcp-recording-security.test.ts b/tests/unit/hooks/browser-mcp-recording-security.test.ts new file mode 100644 index 00000000..179cab28 --- /dev/null +++ b/tests/unit/hooks/browser-mcp-recording-security.test.ts @@ -0,0 +1,82 @@ +import { readFileSync } from 'node:fs'; +import { describe, expect, it } from 'bun:test'; +import { runMcpRequests } from './browser-mcp-test-harness'; +import type { MockPageState } from './browser-mcp-test-harness'; + +describe('ccs-browser MCP server - recording security', () => { + it('filters sensitive text targets and printable key presses in injected recorder code', () => { + const source = readFileSync('lib/mcp/ccs-browser-server.cjs', 'utf8'); + const sensitiveAttributePatternMatch = source.match( + /const sensitiveAttributePattern = \/(.+)\/i;/ + ); + + expect(source).toContain("sensitiveInputTypes = new Set(['password', 'hidden'])"); + expect(source).toContain('sensitiveAutocompleteValues'); + expect(source).toContain('autocompleteTokens.some((token) => sensitiveAutocompleteValues.has(token))'); + expect(source).toContain('isSensitiveTextTarget(target)'); + expect(source).toContain("typeof event.key !== 'string' || event.key.length !== 1"); + expect(sensitiveAttributePatternMatch).not.toBeNull(); + + const sensitiveAttributePattern = new RegExp(sensitiveAttributePatternMatch![1], 'i'); + expect(sensitiveAttributePattern.test('verification_code')).toBe(true); + expect(sensitiveAttributePattern.test('security-code')).toBe(true); + expect(sensitiveAttributePattern.test('pin')).toBe(true); + expect(sensitiveAttributePattern.test('auth_token')).toBe(true); + expect(sensitiveAttributePattern.test('author')).toBe(false); + }); + + it('tears down page recording hooks when stopping or clearing', async () => { + const pages: MockPageState[] = [ + { + id: 'page-1', + title: 'Recording Page', + currentUrl: 'https://example.com/recording', + recording: { + events: [{ kind: 'click', selector: '#submit', timestamp: 1710000000000 }], + }, + }, + ]; + + await runMcpRequests(pages, [ + { + jsonrpc: '2.0', + id: 1, + method: 'tools/call', + params: { name: 'browser_start_recording', arguments: {} }, + }, + { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name: 'browser_stop_recording', arguments: {} }, + }, + { + jsonrpc: '2.0', + id: 3, + method: 'tools/call', + params: { name: 'browser_clear_recording', arguments: {} }, + }, + ]); + + expect(pages[0].recording?.installed).toBe(false); + expect(pages[0].recording?.teardownCalls).toBe(1); + + await runMcpRequests(pages, [ + { + jsonrpc: '2.0', + id: 4, + method: 'tools/call', + params: { name: 'browser_start_recording', arguments: {} }, + }, + { + jsonrpc: '2.0', + id: 5, + method: 'tools/call', + params: { name: 'browser_clear_recording', arguments: {} }, + }, + ]); + + expect(pages[0].recording?.installed).toBe(false); + expect(pages[0].recording?.teardownCalls).toBe(2); + }); +}); diff --git a/tests/unit/hooks/browser-mcp-test-harness.ts b/tests/unit/hooks/browser-mcp-test-harness.ts index 4027ac5a..52dd8cea 100644 --- a/tests/unit/hooks/browser-mcp-test-harness.ts +++ b/tests/unit/hooks/browser-mcp-test-harness.ts @@ -207,6 +207,8 @@ type MockRecordingPlan = { warnings?: MockRecordingWarning[]; injectionError?: string; finalizeError?: string; + installed?: boolean; + teardownCalls?: number; }; type MockFrameState = { @@ -1174,6 +1176,10 @@ function createMockBrowser(pagesInput: MockPageState[]) { return; } + if (plan.installed) { + plan.teardownCalls = (plan.teardownCalls || 0) + 1; + } + plan.installed = true; if ( recordingPayload && (recordingPayload.events?.length || recordingPayload.warnings?.length) @@ -1187,9 +1193,8 @@ function createMockBrowser(pagesInput: MockPageState[]) { } if ( - expression.includes( - 'globalThis.__CCS_BROWSER_RECORDING_RECORDER__ || { events: [], warnings: [] }' - ) + expression.includes('const recorder = globalThis.__CCS_BROWSER_RECORDING_RECORDER__') && + expression.includes('return { events, warnings }') ) { const plan = getMockRecordingPlan(page); if (plan.finalizeError) { @@ -1201,6 +1206,10 @@ function createMockBrowser(pagesInput: MockPageState[]) { ); return; } + if (plan.installed) { + plan.teardownCalls = (plan.teardownCalls || 0) + 1; + } + plan.installed = false; reply({ result: { type: 'object', @@ -1213,6 +1222,19 @@ function createMockBrowser(pagesInput: MockPageState[]) { return; } + if ( + expression.includes('const recorder = globalThis.__CCS_BROWSER_RECORDING_RECORDER__') && + expression.includes('return { installed: false }') + ) { + const plan = getMockRecordingPlan(page); + if (plan.installed) { + plan.teardownCalls = (plan.teardownCalls || 0) + 1; + } + plan.installed = false; + reply({ result: { type: 'object', value: { installed: false } } }); + return; + } + if (expression.includes('new DragEvent') && expression.includes('new DataTransfer()')) { const selector = parseJsonArgument(expression, 'selector') || ''; const nth = parseNumberArgument(expression, 'nth') ?? 0;