mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-02 09:20:42 -07:00
fix(browser): terminate subagent immediately on domain restriction violations (#24313)
This commit is contained in:
@@ -73,7 +73,7 @@ export function buildBrowserSystemPrompt(
|
||||
.map((d) => `- ${d}`)
|
||||
.join(
|
||||
'\n',
|
||||
)}\nDo NOT attempt to navigate to any other domains using new_page or navigate_page, as it will be rejected. This is a hard security constraint.\nDo NOT use proxy services (e.g. Google Translate, Google AMP, or any URL translation/caching service) to access content from domains outside this list. Embedding a blocked URL as a parameter of an allowed-domain service is a direct violation of this security restriction.`
|
||||
)}\nDo NOT attempt to navigate to any other domains using new_page or navigate_page, as it will be rejected. This is a hard security constraint.\nDo NOT use proxy services (e.g. Google Translate, Google AMP, or any URL translation/caching service) to access content from domains outside this list. Embedding a blocked URL as a parameter of an allowed-domain service is a direct violation of this security restriction.\nCRITICAL: If the user's task requires visiting a website or domain that is NOT in this allowed list, you MUST call complete_task IMMEDIATELY with success=false. Explain that the required domain is not in the allowed list and cannot be accessed. Do NOT attempt to accomplish the task by searching for the target content on allowed domains — this defeats the purpose of domain restrictions. The allowed domains list is a security policy, not a hint about which sites to use as alternatives.`
|
||||
: '';
|
||||
|
||||
return `You are an expert browser automation agent (Orchestrator). Your goal is to completely fulfill the user's request.${allowedDomainsInstruction}
|
||||
@@ -111,6 +111,7 @@ TERMINAL FAILURES — STOP IMMEDIATELY:
|
||||
Some errors are unrecoverable and retrying will never help. When you see ANY of these, call complete_task immediately with success=false and include the EXACT error message (including any remediation steps it contains) in your summary:
|
||||
- "Could not connect to Chrome" or "Failed to connect to Chrome" or "Timed out connecting to Chrome" — Include the full error message with its remediation steps in your summary verbatim. Do NOT paraphrase or omit instructions.
|
||||
- "Browser closed" or "Target closed" or "Session closed" — The browser process has terminated. Include the error and tell the user to try again.
|
||||
- "Domain not allowed:" — The target domain is blocked by the allowedDomains security policy. Do NOT retry with a different URL or try to find the content on an allowed domain.
|
||||
- "net::ERR_" network errors on the SAME URL after 2 retries — the site is unreachable. Report the URL and error.
|
||||
- "reached maximum action limit" — You have performed too many actions in this task. Stop immediately and report this limit to the user.
|
||||
- Any error that appears IDENTICALLY 3+ times in a row — it will not resolve by retrying.
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { BrowserManager } from './browserManager.js';
|
||||
import { BrowserManager, DomainNotAllowedError } from './browserManager.js';
|
||||
import { makeFakeConfig } from '../../test-utils/config.js';
|
||||
import type { Config } from '../../config/config.js';
|
||||
import { injectAutomationOverlay } from './automationOverlay.js';
|
||||
@@ -224,12 +224,9 @@ describe('BrowserManager', () => {
|
||||
},
|
||||
});
|
||||
const manager = new BrowserManager(restrictedConfig);
|
||||
const result = await manager.callTool('navigate_page', {
|
||||
url: 'https://evil.com',
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect((result.content || [])[0]?.text).toContain('not permitted');
|
||||
await expect(
|
||||
manager.callTool('navigate_page', { url: 'https://evil.com' }),
|
||||
).rejects.toThrow(DomainNotAllowedError);
|
||||
expect(Client).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -276,12 +273,9 @@ describe('BrowserManager', () => {
|
||||
},
|
||||
});
|
||||
const manager = new BrowserManager(restrictedConfig);
|
||||
const result = await manager.callTool('new_page', {
|
||||
url: 'https://evil.com',
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect((result.content || [])[0]?.text).toContain('not permitted');
|
||||
await expect(
|
||||
manager.callTool('new_page', { url: 'https://evil.com' }),
|
||||
).rejects.toThrow(DomainNotAllowedError);
|
||||
});
|
||||
|
||||
it('should block proxy URL with embedded disallowed domain in query params', async () => {
|
||||
@@ -293,14 +287,11 @@ describe('BrowserManager', () => {
|
||||
},
|
||||
});
|
||||
const manager = new BrowserManager(restrictedConfig);
|
||||
const result = await manager.callTool('new_page', {
|
||||
url: 'https://translate.google.com/translate?sl=en&tl=en&u=https://blocked.org/page',
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect((result.content || [])[0]?.text).toContain(
|
||||
'an embedded URL targets a disallowed domain',
|
||||
);
|
||||
await expect(
|
||||
manager.callTool('new_page', {
|
||||
url: 'https://translate.google.com/translate?sl=en&tl=en&u=https://blocked.org/page',
|
||||
}),
|
||||
).rejects.toThrow(DomainNotAllowedError);
|
||||
});
|
||||
|
||||
it('should block proxy URL with embedded disallowed domain in URL fragment (hash)', async () => {
|
||||
@@ -312,14 +303,11 @@ describe('BrowserManager', () => {
|
||||
},
|
||||
});
|
||||
const manager = new BrowserManager(restrictedConfig);
|
||||
const result = await manager.callTool('new_page', {
|
||||
url: 'https://translate.google.com/#view=home&op=translate&sl=en&tl=zh-CN&u=https://blocked.org',
|
||||
});
|
||||
|
||||
expect(result.isError).toBe(true);
|
||||
expect((result.content || [])[0]?.text).toContain(
|
||||
'an embedded URL targets a disallowed domain',
|
||||
);
|
||||
await expect(
|
||||
manager.callTool('new_page', {
|
||||
url: 'https://translate.google.com/#view=home&op=translate&sl=en&tl=zh-CN&u=https://blocked.org',
|
||||
}),
|
||||
).rejects.toThrow(DomainNotAllowedError);
|
||||
});
|
||||
|
||||
it('should allow proxy URL when embedded domain is also allowed', async () => {
|
||||
@@ -397,7 +385,7 @@ describe('BrowserManager', () => {
|
||||
const args = vi.mocked(StdioClientTransport).mock.calls[0]?.[0]
|
||||
?.args as string[];
|
||||
expect(args).toContain(
|
||||
'--chromeArg="--host-rules=MAP * 127.0.0.1, EXCLUDE google.com, EXCLUDE *.openai.com, EXCLUDE 127.0.0.1"',
|
||||
'--chromeArg="--host-rules=MAP * ~NOTFOUND, EXCLUDE google.com, EXCLUDE *.openai.com"',
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -37,6 +37,18 @@ const __dirname = path.dirname(__filename);
|
||||
// Default browser profile directory name within ~/.gemini/
|
||||
const BROWSER_PROFILE_DIR = 'cli-browser-profile';
|
||||
|
||||
/**
|
||||
* Typed error for domain restriction violations.
|
||||
* Thrown when a navigation tool targets a domain not in allowedDomains.
|
||||
* Caught by mcpToolWrapper to terminate the agent immediately.
|
||||
*/
|
||||
export class DomainNotAllowedError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'DomainNotAllowedError';
|
||||
}
|
||||
}
|
||||
|
||||
// Default timeout for MCP operations
|
||||
const MCP_TIMEOUT_MS = 60_000;
|
||||
|
||||
@@ -242,15 +254,7 @@ export class BrowserManager {
|
||||
|
||||
const errorMessage = this.checkNavigationRestrictions(toolName, args);
|
||||
if (errorMessage) {
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: 'text',
|
||||
text: errorMessage,
|
||||
},
|
||||
],
|
||||
isError: true,
|
||||
};
|
||||
throw new DomainNotAllowedError(errorMessage);
|
||||
}
|
||||
|
||||
const client = await this.getRawMcpClient();
|
||||
@@ -526,7 +530,7 @@ export class BrowserManager {
|
||||
})
|
||||
.join(', ');
|
||||
mcpArgs.push(
|
||||
`--chromeArg="--host-rules=MAP * 127.0.0.1, ${exclusionRules}, EXCLUDE 127.0.0.1"`,
|
||||
`--chromeArg="--host-rules=MAP * ~NOTFOUND, ${exclusionRules}"`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -739,8 +743,7 @@ export class BrowserManager {
|
||||
const urlHostname = parsedUrl.hostname;
|
||||
|
||||
if (!this.isDomainAllowed(urlHostname, allowedDomains)) {
|
||||
// If none matched, then deny
|
||||
return `Tool '${toolName}' is not permitted for the requested URL/domain based on your current browser settings.`;
|
||||
return 'Domain not allowed: The requested domain is not in the allowed list.';
|
||||
}
|
||||
|
||||
// Check query parameters for embedded URLs that could bypass domain
|
||||
@@ -759,7 +762,7 @@ export class BrowserManager {
|
||||
) {
|
||||
const embeddedHostname = embeddedUrl.hostname.replace(/\.$/, '');
|
||||
if (!this.isDomainAllowed(embeddedHostname, allowedDomains)) {
|
||||
return `Tool '${toolName}' is not permitted: an embedded URL targets a disallowed domain.`;
|
||||
return 'Domain not allowed: Embedded URL targets a disallowed domain.';
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
|
||||
@@ -28,7 +28,11 @@ import {
|
||||
type PolicyUpdateOptions,
|
||||
} from '../../tools/tools.js';
|
||||
import type { MessageBus } from '../../confirmation-bus/message-bus.js';
|
||||
import type { BrowserManager, McpToolCallResult } from './browserManager.js';
|
||||
import {
|
||||
type BrowserManager,
|
||||
type McpToolCallResult,
|
||||
DomainNotAllowedError,
|
||||
} from './browserManager.js';
|
||||
import { debugLogger } from '../../utils/debugLogger.js';
|
||||
import { suspendInputBlocker, resumeInputBlocker } from './inputBlocker.js';
|
||||
import { MCP_TOOL_PREFIX } from '../../tools/mcp-tool.js';
|
||||
@@ -173,9 +177,13 @@ class McpToolInvocation extends BaseToolInvocation<
|
||||
} catch (error) {
|
||||
const errorMsg = error instanceof Error ? error.message : String(error);
|
||||
|
||||
// Chrome connection errors are fatal — re-throw to terminate the agent
|
||||
// immediately instead of returning a result the LLM would retry.
|
||||
if (errorMsg.includes('Could not connect to Chrome')) {
|
||||
// Domain restriction and Chrome connection errors are fatal — re-throw
|
||||
// to terminate the agent immediately instead of returning a result
|
||||
// the LLM would retry or work around.
|
||||
if (
|
||||
error instanceof DomainNotAllowedError ||
|
||||
errorMsg.includes('Could not connect to Chrome')
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user