fix(core): scope tools.core wildcard deny to built-in tools#28388
fix(core): scope tools.core wildcard deny to built-in tools#28388vedhakoushik wants to merge 1 commit into
Conversation
Setting settings.tools.core to any value, including [], added a wildcard '*' DENY rule with no way to exclude MCP tools from matching it. That DENY (priority 4.24) outranked every MCP-specific allow mechanism — mcpServers.<name>.trust (4.2), mcp.allowed, and mcp.autoAllowInHeadless (4.1) — so all MCP tools were silently denied regardless of trust settings. Because PolicyEngine.getExcludedTools() (which drives which tools are removed from the model's declarations) reuses the same rule matching as call-time checks, the tools weren't just denied at call time — their declarations never reached the model, so it blind-guessed tool names until exhausting maxSessionTurns. This also contradicted tools.core's documented scope: "Restrict the set of built-in tools." Add an opt-in builtinOnly field to PolicyRule. When set, the rule never matches MCP tools, regardless of its toolName pattern — including the wildcard '*'. All MCP tools are structurally guaranteed to carry the mcp_ prefix (MCP_TOOL_PREFIX / isMcpToolName()), independent of any metadata being correctly populated, so builtinOnly checks serverName OR isMcpToolName(toolCall.name) to stay reliable even when metadata is missing or incomplete. Set builtinOnly: true on the Core Tools Allowlist Enforcement rule in config.ts, so it stays scoped to built-in tools as documented while leaving MCP tools governed by their own trust/allow mechanisms. Because getExcludedTools() and the real-time check() share the same ruleMatches() function, this one change fixes both the declaration-level exclusion and the call-time deny. Added regression tests: builtinOnly matching in ruleMatches() and getExcludedTools(), a priority-ordering test reproducing the exact issue scenario, and end-to-end tests through createPolicyEngineConfig + PolicyEngine reproducing the issue's minimal repro. Also clarified the tools.core docs to state explicitly that it does not affect MCP tools. Fixes google-gemini#28361 Signed-off-by: Bellam vedha kousik <bellam.vedhakoushik@gmail.com>
|
📊 PR Size: size/L
|
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 a bug where the Highlights
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 the 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 counterproductive. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a bug where configuring the core tools allowlist (tools.core) would inadvertently block Model Context Protocol (MCP) tools. To fix this, a new builtinOnly flag has been introduced to the PolicyRule interface, allowing wildcard DENY rules to be scoped exclusively to built-in tools. The policy engine has been updated to respect this flag, and comprehensive unit tests have been added to prevent regressions. Additionally, the configuration documentation has been updated to clarify the scope of tools.core. I have no further feedback to provide as the implementation is robust and well-tested.
gemini-cli
Bot
added
priority/p1
1 participant
Summary
Fixes a bug where setting
tools.coreto any value — including[]— silently disabled every MCP tool, regardless of trust settings, because a wildcard DENY rule had no way to exclude MCP tools from matching it.Details
builtinOnlyfield toPolicyRule(types.ts). When set, the rule never matches MCP tool calls, regardless of itstoolNamepattern.ruleMatches()now short-circuitsbuiltinOnlyrules for any tool call with aserverName(i.e. an MCP tool), falling back toisMcpToolName(toolCall.name)(themcp_prefix) whenserverNamemetadata is missing or incomplete.config.tsnow setsbuiltinOnly: true, sotools.corematches its documented scope ("Restrict the set of built-in tools") without also silently excluding MCP tools frommcpServers.<name>.trust,mcp.allowed, andmcp.autoAllowInHeadless.PolicyEngine.getExcludedTools()(which drives which tool declarations reach the model) shares the sameruleMatches()used for call-time checks, this one change fixes both the declaration-level exclusion and the call-time deny.tools.coreindocs/reference/configuration.mdto state explicitly that it does not affect MCP tools.Related Issues
Fixes #28361
How to Validate
settings.json:{ "mcpServers": { "github": { "command": "github-mcp-server", "args": ["stdio"], "trust": true } }, "tools": { "core": [] } }gemini --approval-mode yolo -p "Call any tool from the github MCP server"Tool "<name>" not found. After this fix: the MCP tool is discoverable and callable.npm test -w @google/gemini-cli-core -- src/policy/config.test.ts src/policy/policy-engine.test.tsPre-Merge Checklist
Supersedes #28365, opened as a clean re-submission — the original PR's commits carried a
Co-Authored-Bytrailer that blocked the CLA check from resolving. Same fix, same tests, no code changes; only the commit metadata differs.