Skip to content

Commit e8fe43b

Browse files
cynthialong0-0cynthialong0-0
authored
feat(browser): add sensitive action controls and read-only noise reduction (#22867)
1 parent 11ec4ac commit e8fe43b

11 files changed

Lines changed: 342 additions & 1 deletion

File tree

‎docs/cli/settings.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,13 @@ they appear in the UI.
101101
| Disable Loop Detection | `model.disableLoopDetection` | Disable automatic detection and prevention of infinite loops. | `false` |
102102
| Skip Next Speaker Check | `model.skipNextSpeakerCheck` | Skip the next speaker check. | `true` |
103103

104+
### Agents
105+
106+
| UI Label | Setting | Description | Default |
107+
| ------------------------- | ---------------------------------------- | --------------------------------------------------------------------------------------------- | ------- |
108+
| Confirm Sensitive Actions | `agents.browser.confirmSensitiveActions` | Require manual confirmation for sensitive browser actions (e.g., fill_form, evaluate_script). | `false` |
109+
| Block File Uploads | `agents.browser.blockFileUploads` | Hard-block file upload requests from the browser agent. | `false` |
110+
104111
### Context
105112

106113
| UI Label | Setting | Description | Default |

‎docs/reference/configuration.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,17 @@ their corresponding top-level category object in your `settings.json` file.
12101210
- **Description:** Disable user input on browser window during automation.
12111211
- **Default:** `true`
12121212

1213+
- **`agents.browser.confirmSensitiveActions`** (boolean):
1214+
- **Description:** Require manual confirmation for sensitive browser actions
1215+
(e.g., fill_form, evaluate_script).
1216+
- **Default:** `false`
1217+
- **Requires restart:** Yes
1218+
1219+
- **`agents.browser.blockFileUploads`** (boolean):
1220+
- **Description:** Hard-block file upload requests from the browser agent.
1221+
- **Default:** `false`
1222+
- **Requires restart:** Yes
1223+
12131224
#### `context`
12141225

12151226
- **`context.fileName`** (string | string[]):

‎packages/cli/src/config/settingsSchema.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,26 @@ const SETTINGS_SCHEMA = {
11981198
'Disable user input on browser window during automation.',
11991199
showInDialog: false,
12001200
},
1201+
confirmSensitiveActions: {
1202+
type: 'boolean',
1203+
label: 'Confirm Sensitive Actions',
1204+
category: 'Advanced',
1205+
requiresRestart: true,
1206+
default: false,
1207+
description:
1208+
'Require manual confirmation for sensitive browser actions (e.g., fill_form, evaluate_script).',
1209+
showInDialog: true,
1210+
},
1211+
blockFileUploads: {
1212+
type: 'boolean',
1213+
label: 'Block File Uploads',
1214+
category: 'Advanced',
1215+
requiresRestart: true,
1216+
default: false,
1217+
description:
1218+
'Hard-block file upload requests from the browser agent.',
1219+
showInDialog: true,
1220+
},
12011221
},
12021222
},
12031223
},

‎packages/core/src/agents/browser/browserAgentFactory.test.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import {
1111
} from './browserAgentFactory.js';
1212
import { injectAutomationOverlay } from './automationOverlay.js';
1313
import { makeFakeConfig } from '../../test-utils/config.js';
14+
import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../../policy/types.js';
1415
import type { Config } from '../../config/config.js';
1516
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
17+
import type { PolicyEngine } from '../../policy/policy-engine.js';
1618
import type { BrowserManager } from './browserManager.js';
1719

1820
// Create mock browser manager
@@ -300,6 +302,116 @@ describe('browserAgentFactory', () => {
300302
});
301303
});
302304

305+
describe('Policy Registration', () => {
306+
let mockPolicyEngine: {
307+
addRule: ReturnType<typeof vi.fn>;
308+
hasRuleForTool: ReturnType<typeof vi.fn>;
309+
removeRulesForTool: ReturnType<typeof vi.fn>;
310+
getRules: ReturnType<typeof vi.fn>;
311+
};
312+
313+
beforeEach(() => {
314+
mockPolicyEngine = {
315+
addRule: vi.fn(),
316+
hasRuleForTool: vi.fn().mockReturnValue(false),
317+
removeRulesForTool: vi.fn(),
318+
getRules: vi.fn().mockReturnValue([]),
319+
};
320+
vi.spyOn(mockConfig, 'getPolicyEngine').mockReturnValue(
321+
mockPolicyEngine as unknown as PolicyEngine,
322+
);
323+
});
324+
325+
it('should register sensitive action rules', async () => {
326+
mockConfig = makeFakeConfig({
327+
agents: {
328+
browser: {
329+
confirmSensitiveActions: true,
330+
},
331+
},
332+
});
333+
vi.spyOn(mockConfig, 'getPolicyEngine').mockReturnValue(
334+
mockPolicyEngine as unknown as PolicyEngine,
335+
);
336+
337+
await createBrowserAgentDefinition(mockConfig, mockMessageBus);
338+
339+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
340+
expect.objectContaining({
341+
toolName: 'mcp_browser_agent_fill',
342+
decision: PolicyDecision.ASK_USER,
343+
priority: 999,
344+
}),
345+
);
346+
347+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
348+
expect.objectContaining({
349+
toolName: 'mcp_browser_agent_upload_file',
350+
decision: PolicyDecision.ASK_USER,
351+
priority: 999,
352+
}),
353+
);
354+
355+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
356+
expect.objectContaining({
357+
toolName: 'mcp_browser_agent_evaluate_script',
358+
decision: PolicyDecision.ASK_USER,
359+
priority: 999,
360+
}),
361+
);
362+
});
363+
364+
it('should register fill rule even when confirmSensitiveActions is disabled', async () => {
365+
await createBrowserAgentDefinition(mockConfig, mockMessageBus);
366+
367+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
368+
expect.objectContaining({
369+
toolName: 'mcp_browser_agent_fill',
370+
}),
371+
);
372+
373+
expect(mockPolicyEngine.addRule).not.toHaveBeenCalledWith(
374+
expect.objectContaining({
375+
toolName: 'mcp_browser_agent_upload_file',
376+
}),
377+
);
378+
});
379+
380+
it('should register ALLOW rules for read-only tools', async () => {
381+
mockBrowserManager.getDiscoveredTools.mockResolvedValue([
382+
{ name: 'take_snapshot', description: 'Take snapshot' },
383+
{ name: 'take_screenshot', description: 'Take screenshot' },
384+
{ name: 'list_pages', description: 'list all pages' },
385+
]);
386+
387+
await createBrowserAgentDefinition(mockConfig, mockMessageBus);
388+
389+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
390+
expect.objectContaining({
391+
toolName: 'mcp_browser_agent_take_snapshot',
392+
decision: PolicyDecision.ALLOW,
393+
priority: PRIORITY_SUBAGENT_TOOL,
394+
}),
395+
);
396+
397+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
398+
expect.objectContaining({
399+
toolName: 'mcp_browser_agent_take_screenshot',
400+
decision: PolicyDecision.ALLOW,
401+
priority: PRIORITY_SUBAGENT_TOOL,
402+
}),
403+
);
404+
405+
expect(mockPolicyEngine.addRule).toHaveBeenCalledWith(
406+
expect.objectContaining({
407+
toolName: 'mcp_browser_agent_list_pages',
408+
decision: PolicyDecision.ALLOW,
409+
priority: PRIORITY_SUBAGENT_TOOL,
410+
}),
411+
);
412+
});
413+
});
414+
303415
describe('cleanupBrowserAgent', () => {
304416
it('should call close on browser manager', async () => {
305417
await cleanupBrowserAgent(

‎packages/core/src/agents/browser/browserAgentFactory.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import type { LocalAgentDefinition } from '../types.js';
2121
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
2222
import type { AnyDeclarativeTool } from '../../tools/tools.js';
2323
import { BrowserManager } from './browserManager.js';
24+
import { BROWSER_AGENT_NAME } from './browserAgentDefinition.js';
25+
import { MCP_TOOL_PREFIX } from '../../tools/mcp-tool.js';
2426
import {
2527
BrowserAgentDefinition,
2628
type BrowserTaskResultSchema,
@@ -30,6 +32,11 @@ import { createAnalyzeScreenshotTool } from './analyzeScreenshot.js';
3032
import { injectAutomationOverlay } from './automationOverlay.js';
3133
import { injectInputBlocker } from './inputBlocker.js';
3234
import { debugLogger } from '../../utils/debugLogger.js';
35+
import {
36+
PolicyDecision,
37+
PRIORITY_SUBAGENT_TOOL,
38+
type PolicyRule,
39+
} from '../../policy/types.js';
3340

3441
/**
3542
* Creates a browser agent definition with MCP tools configured.
@@ -86,9 +93,79 @@ export async function createBrowserAgentDefinition(
8693
browserManager,
8794
messageBus,
8895
shouldDisableInput,
96+
browserConfig.customConfig.blockFileUploads,
8997
);
9098
const availableToolNames = mcpTools.map((t) => t.name);
9199

100+
// Register high-priority policy rules for sensitive actions which is not
101+
// able to be overwrite by YOLO mode.
102+
const policyEngine = config.getPolicyEngine();
103+
104+
if (policyEngine) {
105+
const existingRules = policyEngine.getRules();
106+
107+
const restrictedTools = ['fill', 'fill_form'];
108+
109+
// ASK_USER for upload_file and evaluate_script when sensitive action
110+
// need confirmation.
111+
if (browserConfig.customConfig.confirmSensitiveActions) {
112+
restrictedTools.push('upload_file', 'evaluate_script');
113+
}
114+
115+
for (const toolName of restrictedTools) {
116+
const rule = generateAskUserRules(toolName);
117+
if (!existingRules.some((r) => isRuleEqual(r, rule))) {
118+
policyEngine.addRule(rule);
119+
}
120+
}
121+
122+
// Reduce noise for read-only tools in default mode
123+
const readOnlyTools = [
124+
'take_snapshot',
125+
'take_screenshot',
126+
'list_pages',
127+
'list_network_requests',
128+
];
129+
for (const toolName of readOnlyTools) {
130+
if (availableToolNames.includes(toolName)) {
131+
const rule = generateAllowRules(toolName);
132+
if (!existingRules.some((r) => isRuleEqual(r, rule))) {
133+
policyEngine.addRule(rule);
134+
}
135+
}
136+
}
137+
}
138+
139+
function generateAskUserRules(toolName: string): PolicyRule {
140+
return {
141+
toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`,
142+
decision: PolicyDecision.ASK_USER,
143+
priority: 999,
144+
source: 'BrowserAgent (Sensitive Actions)',
145+
mcpName: BROWSER_AGENT_NAME,
146+
};
147+
}
148+
149+
function generateAllowRules(toolName: string): PolicyRule {
150+
return {
151+
toolName: `${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`,
152+
decision: PolicyDecision.ALLOW,
153+
priority: PRIORITY_SUBAGENT_TOOL,
154+
source: 'BrowserAgent (Read-Only)',
155+
mcpName: BROWSER_AGENT_NAME,
156+
};
157+
}
158+
159+
// Check if policy rule the same in all the attributes that we care about
160+
function isRuleEqual(rule1: PolicyRule, rule2: PolicyRule) {
161+
return (
162+
rule1.toolName === rule2.toolName &&
163+
rule1.decision === rule2.decision &&
164+
rule1.priority === rule2.priority &&
165+
rule1.mcpName === rule2.mcpName
166+
);
167+
}
168+
92169
// Validate required semantic tools are available
93170
const requiredSemanticTools = [
94171
'click',

‎packages/core/src/agents/browser/mcpToolWrapper.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,4 +301,55 @@ describe('mcpToolWrapper', () => {
301301
expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(3);
302302
});
303303
});
304+
305+
describe('Hard Block: upload_file', () => {
306+
beforeEach(() => {
307+
mockMcpTools.push({
308+
name: 'upload_file',
309+
description: 'Upload a file',
310+
inputSchema: {
311+
type: 'object',
312+
properties: { path: { type: 'string' } },
313+
},
314+
});
315+
});
316+
317+
it('should block upload_file when blockFileUploads is true', async () => {
318+
const tools = await createMcpDeclarativeTools(
319+
mockBrowserManager,
320+
mockMessageBus,
321+
false,
322+
true, // blockFileUploads
323+
);
324+
325+
const uploadTool = tools.find((t) => t.name === 'upload_file')!;
326+
const invocation = uploadTool.build({ path: 'test.txt' });
327+
const result = await invocation.execute(new AbortController().signal);
328+
329+
expect(result.error).toBeDefined();
330+
expect(result.llmContent).toContain('File uploads are blocked');
331+
expect(mockBrowserManager.callTool).not.toHaveBeenCalled();
332+
});
333+
334+
it('should NOT block upload_file when blockFileUploads is false', async () => {
335+
const tools = await createMcpDeclarativeTools(
336+
mockBrowserManager,
337+
mockMessageBus,
338+
false,
339+
false, // blockFileUploads
340+
);
341+
342+
const uploadTool = tools.find((t) => t.name === 'upload_file')!;
343+
const invocation = uploadTool.build({ path: 'test.txt' });
344+
const result = await invocation.execute(new AbortController().signal);
345+
346+
expect(result.error).toBeUndefined();
347+
expect(result.llmContent).toBe('Tool result');
348+
expect(mockBrowserManager.callTool).toHaveBeenCalledWith(
349+
'upload_file',
350+
expect.anything(),
351+
expect.anything(),
352+
);
353+
});
354+
});
304355
});

0 commit comments

Comments
 (0)