fix(browser): stop recorder leaking sensitive input

Squash merge PR #1293 into dev.

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';

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'
     );
   });

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);
+  });
+});

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;