Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0315833
trying the fix
cynthialong0-0 Mar 13, 2026
24c5d9f
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 13, 2026
32ada00
refactor(core): centralize MCP policy management and standardize wild…
cynthialong0-0 Mar 13, 2026
44cffcc
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 13, 2026
2c68d20
fix build
cynthialong0-0 Mar 13, 2026
07abf8e
fix config
cynthialong0-0 Mar 15, 2026
4e24293
refactor the fix
cynthialong0-0 Mar 16, 2026
0978c76
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 16, 2026
b79a3ff
fix merge conflict
cynthialong0-0 Mar 16, 2026
6076f7d
add config change
cynthialong0-0 Mar 16, 2026
9993383
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 16, 2026
9b58e3c
update comment
cynthialong0-0 Mar 16, 2026
19b8e34
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 16, 2026
24a8b8b
add unit test and e2e test to prevent regression
cynthialong0-0 Mar 17, 2026
8224e66
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 17, 2026
630b390
remove unncessary wait
cynthialong0-0 Mar 17, 2026
ff8e507
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 17, 2026
44ced17
test(core): fix lint issues in policy-updater.test
cynthialong0-0 Mar 17, 2026
8caa23d
fix format
cynthialong0-0 Mar 17, 2026
6fb836b
fix server name in test
cynthialong0-0 Mar 17, 2026
2d8d178
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 17, 2026
e2c39d5
try to fix the sandbox:none test case
cynthialong0-0 Mar 18, 2026
0c63661
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 18, 2026
127e1ca
rever test change
cynthialong0-0 Mar 18, 2026
770861a
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 18, 2026
526a19d
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 18, 2026
00f1367
include privacy notice update in the test
cynthialong0-0 Mar 19, 2026
eeaa118
Merge branch 'main' into refactor/mcp-policy-centralization
cynthialong0-0 Mar 19, 2026
cba984d
try to make sandbox none & macos to work
cynthialong0-0 Mar 19, 2026
accef78
update test
cynthialong0-0 Mar 19, 2026
773489b
try defaultApprovalMode in setting
cynthialong0-0 Mar 19, 2026
fb38c4d
fix test failure in macos env
cynthialong0-0 Mar 19, 2026
b43bb92
try removing new_page from final output
cynthialong0-0 Mar 19, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions integration-tests/browser-policy.responses
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"I'll help you with that."},{"functionCall":{"name":"browser_agent","args":{"task":"Open https://example.com and check if there is a heading"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"new_page","args":{"url":"https://example.com"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"take_snapshot","args":{}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"functionCall":{"name":"complete_task","args":{"success":true,"summary":"SUCCESS_POLICY_TEST_COMPLETED"}}}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":100,"candidatesTokenCount":50,"totalTokenCount":150}}]}
{"method":"generateContentStream","response":[{"candidates":[{"content":{"parts":[{"text":"Task completed successfully. The page has the heading \"Example Domain\"."}],"role":"model"},"finishReason":"STOP","index":0}],"usageMetadata":{"promptTokenCount":200,"candidatesTokenCount":50,"totalTokenCount":250}}]}
178 changes: 178 additions & 0 deletions integration-tests/browser-policy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { TestRig, poll } from './test-helper.js';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { execSync } from 'node:child_process';
import { existsSync, writeFileSync, readFileSync, mkdirSync } from 'node:fs';
import stripAnsi from 'strip-ansi';

const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);

const chromeAvailable = (() => {
try {
if (process.platform === 'darwin') {
execSync(
'test -d "/Applications/Google Chrome.app" || test -d "/Applications/Chromium.app"',
{
stdio: 'ignore',
},
);
} else if (process.platform === 'linux') {
execSync(
'which google-chrome || which chromium-browser || which chromium',
{ stdio: 'ignore' },
);
} else if (process.platform === 'win32') {
const chromePaths = [
'C:\\Program Files\\Google\\Chrome\\Application\\chrome.exe',
'C:\\Program Files (x86)\\Google\\Chrome\\Application\\chrome.exe',
`${process.env['LOCALAPPDATA'] ?? ''}\\Google\\Chrome\\Application\\chrome.exe`,
];
const found = chromePaths.some((p) => existsSync(p));
if (!found) {
execSync('where chrome || where chromium', { stdio: 'ignore' });
}
} else {
return false;
}
return true;
} catch {
return false;
}
})();

describe.skipIf(!chromeAvailable)('browser-policy', () => {
let rig: TestRig;

beforeEach(() => {
rig = new TestRig();
});

afterEach(async () => {
await rig.cleanup();
});

it('should skip confirmation when "Allow all server tools for this session" is chosen', async () => {
rig.setup('browser-policy-skip-confirmation', {
fakeResponsesPath: join(__dirname, 'browser-policy.responses'),
settings: {
agents: {
overrides: {
browser_agent: {
enabled: true,
},
},
browser: {
headless: true,
sessionMode: 'isolated',
allowedDomains: ['example.com'],
},
},
},
});

// Manually trust the folder to avoid the dialog and enable option 3
const geminiDir = join(rig.homeDir!, '.gemini');
mkdirSync(geminiDir, { recursive: true });

// Write to trustedFolders.json
const trustedFoldersPath = join(geminiDir, 'trustedFolders.json');
const trustedFolders = {
[rig.testDir!]: 'TRUST_FOLDER',
};
writeFileSync(trustedFoldersPath, JSON.stringify(trustedFolders, null, 2));

// Force confirmation for browser agent.
// NOTE: We don't force confirm browser tools here because "Allow all server tools"
// adds a rule with ALWAYS_ALLOW_PRIORITY (3.9x) which would be overshadowed by
// a rule in the user tier (4.x) like the one from this TOML.
// By removing the explicit mcp rule, the first MCP tool will still prompt
// due to default approvalMode = 'default', and then "Allow all" will correctly
// bypass subsequent tools.
const policyFile = join(rig.testDir!, 'force-confirm.toml');
writeFileSync(
policyFile,
`
[[rule]]
name = "Force confirm browser_agent"
toolName = "browser_agent"
decision = "ask_user"
priority = 200
`,
);

// Update settings.json in both project and home directories to point to the policy file
for (const baseDir of [rig.testDir!, rig.homeDir!]) {
const settingsPath = join(baseDir, '.gemini', 'settings.json');
if (existsSync(settingsPath)) {
const settings = JSON.parse(readFileSync(settingsPath, 'utf-8'));
settings.policyPaths = [policyFile];
// Ensure folder trust is enabled
settings.security = settings.security || {};
settings.security.folderTrust = settings.security.folderTrust || {};
settings.security.folderTrust.enabled = true;
writeFileSync(settingsPath, JSON.stringify(settings, null, 2));
}
}

const run = await rig.runInteractive({
approvalMode: 'default',
env: {
GEMINI_CLI_INTEGRATION_TEST: 'true',
},
});

await run.sendKeys(
'Open https://example.com and check if there is a heading\r',
);
await run.sendKeys('\r');

// Handle confirmations.
// 1. Initial browser_agent delegation (likely only 3 options, so use option 1: Allow once)
await poll(
() => stripAnsi(run.output).toLowerCase().includes('action required'),
60000,
1000,
);
await run.sendKeys('1\r');
await new Promise((r) => setTimeout(r, 2000));

// Handle privacy notice
await poll(
() => stripAnsi(run.output).toLowerCase().includes('privacy notice'),
5000,
100,
);
await run.sendKeys('1\r');
await new Promise((r) => setTimeout(r, 5000));

// new_page (MCP tool, should have 4 options, use option 3: Allow all server tools)
await poll(
() => {
const stripped = stripAnsi(run.output).toLowerCase();
return (
stripped.includes('new_page') &&
stripped.includes('allow all server tools for this session')
);
},
60000,
1000,
);

// Select "Allow all server tools for this session" (option 3)
await run.sendKeys('3\r');
await new Promise((r) => setTimeout(r, 30000));

const output = stripAnsi(run.output).toLowerCase();

expect(output).toContain('browser_agent');
expect(output).toContain('completed successfully');
});
});
22 changes: 19 additions & 3 deletions packages/core/src/agents/browser/mcpToolWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import type { MessageBus } from '../../confirmation-bus/message-bus.js';
import type { BrowserManager, McpToolCallResult } from './browserManager.js';
import { debugLogger } from '../../utils/debugLogger.js';
import { suspendInputBlocker, resumeInputBlocker } from './inputBlocker.js';
import { MCP_TOOL_PREFIX } from '../../tools/mcp-tool.js';
import { BROWSER_AGENT_NAME } from './browserAgentDefinition.js';

/**
* Tools that interact with page elements and require the input blocker
Expand Down Expand Up @@ -62,7 +64,13 @@ class McpToolInvocation extends BaseToolInvocation<
messageBus: MessageBus,
private readonly shouldDisableInput: boolean,
) {
super(params, messageBus, toolName, toolName);
super(
params,
messageBus,
`${MCP_TOOL_PREFIX}${BROWSER_AGENT_NAME}_${toolName}`,
toolName,
BROWSER_AGENT_NAME,
);
}

getDescription(): string {
Expand All @@ -79,7 +87,7 @@ class McpToolInvocation extends BaseToolInvocation<
return {
type: 'mcp',
title: `Confirm MCP Tool: ${this.toolName}`,
serverName: 'browser-agent',
serverName: BROWSER_AGENT_NAME,
toolName: this.toolName,
toolDisplayName: this.toolName,
onConfirm: async (outcome: ToolConfirmationOutcome) => {
Expand All @@ -92,7 +100,7 @@ class McpToolInvocation extends BaseToolInvocation<
_outcome: ToolConfirmationOutcome,
): PolicyUpdateOptions | undefined {
return {
mcpName: 'browser-agent',
mcpName: BROWSER_AGENT_NAME,
};
}

Expand Down Expand Up @@ -202,6 +210,14 @@ class McpDeclarativeTool extends DeclarativeTool<
);
}

// Used for determining tool identity in the policy engine to check if a tool
// call is allowed based on policy.
override get toolAnnotations(): Record<string, unknown> {
return {
_serverName: BROWSER_AGENT_NAME,
};
}
Comment thread
cynthialong0-0 marked this conversation as resolved.

build(
params: Record<string, unknown>,
): ToolInvocation<Record<string, unknown>, ToolResult> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('mcpToolWrapper Confirmation', () => {
expect(details).toEqual(
expect.objectContaining({
type: 'mcp',
serverName: 'browser-agent',
serverName: 'browser_agent',
toolName: 'test_tool',
}),
);
Expand All @@ -76,7 +76,7 @@ describe('mcpToolWrapper Confirmation', () => {
expect(mockMessageBus.publish).toHaveBeenCalledWith(
expect.objectContaining({
type: MessageBusType.UPDATE_POLICY,
mcpName: 'browser-agent',
mcpName: 'browser_agent',
persist: false,
}),
);
Expand All @@ -94,7 +94,7 @@ describe('mcpToolWrapper Confirmation', () => {
);

expect(options).toEqual({
mcpName: 'browser-agent',
mcpName: 'browser_agent',
});
});
});
28 changes: 28 additions & 0 deletions packages/core/src/policy/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,34 @@ name = "invalid-name"
).toBeUndefined();
});

it('should support mcpName in policy rules from TOML', async () => {
mockPolicyFile(
nodePath.join(MOCK_DEFAULT_DIR, 'mcp.toml'),
`
[[rule]]
toolName = "my-tool"
mcpName = "my-server"
decision = "allow"
priority = 150
`,
);

const config = await createPolicyEngineConfig(
{},
ApprovalMode.DEFAULT,
MOCK_DEFAULT_DIR,
);

const rule = config.rules?.find(
(r) =>
r.toolName === 'mcp_my-server_my-tool' &&
r.mcpName === 'my-server' &&
r.decision === PolicyDecision.ALLOW,
);
expect(rule).toBeDefined();
expect(rule?.priority).toBeCloseTo(1.15, 5);
});

it('should have default ASK_USER rule for discovered tools', async () => {
const config = await createPolicyEngineConfig({}, ApprovalMode.DEFAULT);
const discoveredRule = config.rules?.find(
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/policy/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ export function createPolicyUpdater(
decision: PolicyDecision.ALLOW,
priority,
argsPattern: new RegExp(pattern),
mcpName: message.mcpName,
source: 'Dynamic (Confirmed)',
});
}
Expand Down Expand Up @@ -611,6 +612,7 @@ export function createPolicyUpdater(
decision: PolicyDecision.ALLOW,
priority,
argsPattern,
mcpName: message.mcpName,
Comment thread
cynthialong0-0 marked this conversation as resolved.
source: 'Dynamic (Confirmed)',
});
}
Expand Down
Loading
Loading