fix(browser-agent): enable "Allow all server tools" session policy#22343
fix(browser-agent): enable "Allow all server tools" session policy#22343gsquared94 merged 33 commits intomainfrom
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively centralizes MCP policy updates and enhances tool server attribution. However, critical security vulnerabilities have been identified: a flaw in the policy priority hierarchy allows dynamic 'Always allow' rules to override explicit security exclusions, potentially leading to policy bypass, and a function signature mismatch between the scheduler and the policy update logic breaks the policy narrowing mechanism for standard tools, resulting in overly broad permission rules. Additionally, there's a regression in the logic for persisting MCP tool policies, which will cause incorrect rules to be saved. These issues need to be addressed to ensure the integrity of the security framework and correct policy persistence.
…card format
- Centralized MCP policy management by moving logic to the scheduler and removing redundant assignments in agent-scheduler.ts.
- Standardized the MCP server wildcard format to '{server}__*' across the policy engine and extraction logic.
- Improved robustness of tool name extraction in config.ts.
- Updated unit tests (agent-scheduler.test.ts, policy.test.ts) to match the new function signatures and wildcard formats.
|
Size Change: +385 B (0%) Total Size: 26.2 MB
ℹ️ View Unchanged
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where the 'Allow all server tools for this session' policy was not effectively applied to the browser agent. It achieves this by standardizing the identification and naming conventions for browser agent tools, ensuring they are properly recognized and managed by the PolicyEngine, thereby enabling correct application of server-wide policies. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where session-wide policies for browser agent tools were not working by using fully qualified tool names and ensuring the policy engine can identify the server. A security audit found no high or critical vulnerabilities, and the changes enhance the security posture by enabling more precise policy enforcement. However, the newly added toolAnnotations are not being propagated to the tool invocation objects, making that part of the change ineffective.
|
I think this should also fix #20594 |
Summary
This PR fixes an issue where selecting
Allow all server tools for this sessionhad no effect for the browser agent. It ensures browser agent tools are correctly identified as MCP-style tools and qualify for server-wide policy matching.Details
The browser agent uses tools that lacked the standard metadata and naming conventions required by the PolicyEngine for wildcard matching. This PR addresses this through the following changes:
BROWSER_AGENT_SERVER_NAMEand replace hard codedbrowser-agentto make sure it uses the same value across all settingsMcpDeclarativeToolinclude _serverName in the toolAnnotations.mcp_browser-agent_*wildcard rules while maintaining strict security checks.packages/core/src/policy/config.tsto correctly pass themcpNamewhen adding dynamic rules, ensuring approvals are securely scoped to the specific server.Related Issues
fixes #22342
How to Validate
to tools from the same server are auto-approved.
Pre-Merge Checklist