From 613feff2755ce1b8994023f3a0be1343db4384ab Mon Sep 17 00:00:00 2001 From: Bernhard Wittmann Date: Mon, 18 May 2026 16:05:03 +0200 Subject: [PATCH] fix: Gate MCP browser visual tools on visible secrets (no-changelog) (#30514) --- .../mcp-browser/src/tools/inspection.test.ts | 191 +++++++++++++++++- .../@n8n/mcp-browser/src/tools/inspection.ts | 44 +++- .../src/tools/sensitivity-types.ts | 7 + 3 files changed, 237 insertions(+), 5 deletions(-) create mode 100644 packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts diff --git a/packages/@n8n/mcp-browser/src/tools/inspection.test.ts b/packages/@n8n/mcp-browser/src/tools/inspection.test.ts index fb19467715f..7dd2c705d43 100644 --- a/packages/@n8n/mcp-browser/src/tools/inspection.test.ts +++ b/packages/@n8n/mcp-browser/src/tools/inspection.test.ts @@ -89,6 +89,30 @@ describe('createInspectionTools', () => { expect(mockConnection.adapter.snapshot).toHaveBeenCalledWith('page1', undefined, false); }); + + it('redacts DOM-derived snapshot text with the host-side analyzer', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; + mockConnection.adapter.snapshot.mockResolvedValue({ + tree: `- text "Your key is ${secret}" [ref=e1]`, + refCount: 1, + }); + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: `

Your key is ${secret}

`, + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + const data = structuredOf(result); + + expect(data.snapshot).toBe('- text "Your key is [REDACTED:anthropic_api_key]" [ref=e1]'); + expect(textOf(result)).not.toContain(secret); + }); }); }); @@ -137,6 +161,99 @@ describe('createInspectionTools', () => { { fullPage: true }, ); }); + + it('refuses when the live HTML probe is sensitive', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: `

${secret}

`, + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'sensitive_context' }); + expect(mockConnection.adapter.screenshot).not.toHaveBeenCalled(); + }); + + it('refuses element-targeted screenshots when the page is sensitive', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: `

${secret}

`, + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({ element: { ref: 'e1' } }, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'sensitive_context' }); + expect(mockConnection.adapter.screenshot).not.toHaveBeenCalled(); + }); + + it('refuses when the live HTML probe fails', async () => { + mockConnection.adapter.probePageHtml.mockResolvedValue({ ok: false, error: 'boom' }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'probe_failed' }); + expect(mockConnection.adapter.screenshot).not.toHaveBeenCalled(); + }); + + it('refuses when sensitivity analysis throws unexpectedly', async () => { + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: '', + path: ['document'], + children: [ + { + kind: 'document', + html: '', + path: ['bad-child'], + children: undefined as unknown as [], + errors: [], + }, + ], + errors: [], + }, + }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'probe_failed' }); + expect(mockConnection.adapter.screenshot).not.toHaveBeenCalled(); + }); + + it('allows screenshots after a previously sensitive dialog is dismissed', async () => { + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: '

Dialog dismissed.

', + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(result.content[0].type).toBe('image'); + expect(mockConnection.adapter.screenshot).toHaveBeenCalledWith('page1', undefined, { + fullPage: undefined, + }); + }); }); }); @@ -239,9 +356,38 @@ describe('createInspectionTools', () => { expect(data.result).toEqual({ count: 5 }); }); - it('redacts DOM-discovered secrets returned by evaluate', async () => { - const secret = 'live-input-secret'; + it('allows clean pages and backstop-redacts literal secret return values', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; mockConnection.adapter.evaluate.mockResolvedValue(secret); + + const result = await getTool().execute({ script: '"secret"' }, TOOL_CONTEXT); + const data = structuredOf(result); + + expect(data.result).toBe('[REDACTED:anthropic_api_key]'); + expect(textOf(result)).not.toContain(secret); + }); + + it('refuses when the live HTML probe is sensitive', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: `

${secret}

`, + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({ script: 'document.title' }, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'sensitive_context' }); + expect(mockConnection.adapter.evaluate).not.toHaveBeenCalled(); + }); + + it('refuses when DOM-discovered password values are visible', async () => { + const secret = 'live-input-secret'; mockConnection.adapter.probePageHtml.mockResolvedValue({ ok: true, root: { @@ -256,12 +402,21 @@ describe('createInspectionTools', () => { { script: 'document.querySelector("input")?.value' }, TOOL_CONTEXT, ); - const data = structuredOf(result); expect(mockConnection.adapter.probePageHtml).toHaveBeenCalledWith('page1'); - expect(data.result).toBe('[REDACTED:password]'); + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'sensitive_context' }); + expect(mockConnection.adapter.evaluate).not.toHaveBeenCalled(); expect(textOf(result)).not.toContain(secret); }); + + it('refuses when the live HTML probe fails', async () => { + mockConnection.adapter.probePageHtml.mockResolvedValue({ ok: false, error: 'boom' }); + + const result = await getTool().execute({ script: 'document.title' }, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'probe_failed' }); + expect(mockConnection.adapter.evaluate).not.toHaveBeenCalled(); + }); }); }); @@ -362,6 +517,34 @@ describe('createInspectionTools', () => { expect(data.pdf).toBe('pdfbase64'); expect(data.pages).toBe(3); }); + + it('refuses when the live HTML probe is sensitive', async () => { + const secret = `sk-ant-api03-${'a'.repeat(93)}AA`; + mockConnection.adapter.probePageHtml.mockResolvedValue({ + ok: true, + root: { + kind: 'document', + html: `

${secret}

`, + path: ['document'], + children: [], + errors: [], + }, + }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'sensitive_context' }); + expect(mockConnection.adapter.pdf).not.toHaveBeenCalled(); + }); + + it('refuses when the live HTML probe fails', async () => { + mockConnection.adapter.probePageHtml.mockResolvedValue({ ok: false, error: 'boom' }); + + const result = await getTool().execute({}, TOOL_CONTEXT); + + expect(structuredOf(result)).toMatchObject({ ok: false, reason: 'probe_failed' }); + expect(mockConnection.adapter.pdf).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/@n8n/mcp-browser/src/tools/inspection.ts b/packages/@n8n/mcp-browser/src/tools/inspection.ts index 69eb39a16c2..94919a209e5 100644 --- a/packages/@n8n/mcp-browser/src/tools/inspection.ts +++ b/packages/@n8n/mcp-browser/src/tools/inspection.ts @@ -1,9 +1,11 @@ import { z } from 'zod'; import type { BrowserConnection } from '../connection'; -import type { ToolDefinition } from '../types'; +import { analyzeHtmlSensitivity, type SensitivityResult } from '../sensitivity/analyze-html'; +import type { ConnectionState, ToolDefinition } from '../types'; import { formatCallToolResult, formatImageResponse } from '../utils'; import { createConnectedTool, elementTargetSchema, pageIdField } from './helpers'; +import type { SensitivityRefusal } from './sensitivity-types'; export function createInspectionTools(connection: BrowserConnection): ToolDefinition[] { return [ @@ -75,6 +77,9 @@ function browserScreenshot(connection: BrowserConnection): ToolDefinition { 'Take a screenshot of the page or a specific element. Returns a base64-encoded PNG image. Note: Prefer browser_snapshot for most tasks — it is smaller, faster, and returns refs for element targeting. Use screenshots only when you need visual information (layout, images, charts).', browserScreenshotSchema, async (state, input, pageId) => { + const refusal = await sensitivityRefusal(state, pageId); + if (refusal) return formatCallToolResult({ ...refusal }); + const base64 = await state.adapter.screenshot(pageId, input.element, { fullPage: input.fullPage, }); @@ -170,6 +175,9 @@ function browserEvaluate(connection: BrowserConnection): ToolDefinition { 'Execute JavaScript in the page context and return the result. The script must be an expression or IIFE. The result is JSON-serialized.', browserEvaluateSchema, async (state, input, pageId) => { + const refusal = await sensitivityRefusal(state, pageId); + if (refusal) return formatCallToolResult({ ...refusal }); + const result = await state.adapter.evaluate(pageId, input.script); return formatCallToolResult({ result }); }, @@ -242,6 +250,9 @@ function browserPdf(connection: BrowserConnection): ToolDefinition { 'Generate a PDF of the current page.', browserPdfSchema, async (state, input, pageId) => { + const refusal = await sensitivityRefusal(state, pageId); + if (refusal) return formatCallToolResult({ ...refusal }); + const result = await state.adapter.pdf(pageId, { format: input.format, landscape: input.landscape, @@ -252,6 +263,37 @@ function browserPdf(connection: BrowserConnection): ToolDefinition { ); } +async function sensitivityRefusal( + state: ConnectionState, + pageId: string, +): Promise { + let sensitivity: SensitivityResult; + try { + sensitivity = analyzeHtmlSensitivity(await state.adapter.probePageHtml(pageId)); + } catch { + return { + ok: false, + reason: 'probe_failed', + hint: 'sensitivity check could not run; retry, or use browser_snapshot if you need page state', + }; + } + if (!sensitivity.ok) { + return { + ok: false, + reason: 'probe_failed', + hint: 'sensitivity check could not run; retry, or use browser_snapshot if you need page state', + }; + } + if (sensitivity.sensitive) { + return { + ok: false, + reason: 'sensitive_context', + hint: 'use browser_snapshot; screenshot, PDF, and evaluate are suppressed because secrets are visible on this page', + }; + } + return undefined; +} + // --------------------------------------------------------------------------- // browser_network // --------------------------------------------------------------------------- diff --git a/packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts b/packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts new file mode 100644 index 00000000000..3345ecba2d9 --- /dev/null +++ b/packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts @@ -0,0 +1,7 @@ +export type RefusalReason = 'sensitive_context' | 'probe_failed'; + +export interface SensitivityRefusal { + ok: false; + reason: RefusalReason; + hint: string; +}