mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-31 08:46:58 +02:00
fix: Gate MCP browser visual tools on visible secrets (no-changelog) (#30514)
This commit is contained in:
Bernhard Wittmann
GitHub
parent
05c25da010
commit
613feff275
|
|
@ -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: `<p>Your key is ${secret}</p>`,
|
||||
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: `<p>${secret}</p>`,
|
||||
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: `<div role="dialog"><p>${secret}</p></div>`,
|
||||
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: '<html><body></body></html>',
|
||||
path: ['document'],
|
||||
children: [
|
||||
{
|
||||
kind: 'document',
|
||||
html: '<html><body></body></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: '<main><p>Dialog dismissed.</p></main>',
|
||||
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: `<p>${secret}</p>`,
|
||||
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: `<p>${secret}</p>`,
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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<SensitivityRefusal | undefined> {
|
||||
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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
7
packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts
Normal file
7
packages/@n8n/mcp-browser/src/tools/sensitivity-types.ts
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
export type RefusalReason = 'sensitive_context' | 'probe_failed';
|
||||
|
||||
export interface SensitivityRefusal {
|
||||
ok: false;
|
||||
reason: RefusalReason;
|
||||
hint: string;
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user