refactor(core): standardize MCP tool naming to mcp_ FQN format#21425
refactor(core): standardize MCP tool naming to mcp_ FQN format#21425abhipatel12 merged 4 commits intomainfrom
Conversation
gemini-cli
bot
added
the
status/need-issue
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 significantly refactors the naming convention and policy enforcement for Managed Code Platform (MCP) tools. The primary goal is to enhance the robustness and security of how MCP tools are identified and governed within the system. By introducing a standardized 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
|
|
Size Change: -147 B (0%) Total Size: 26 MB ℹ️ View Unchanged
|
![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 is a significant and valuable refactoring that standardizes MCP tool naming to a fully qualified name (FQN) format, mcp_{server_name}_{tool_name}. This change improves the robustness and security of the policy engine by moving away from ambiguous string splitting and relying on explicit serverName metadata in policy rules. The changes are consistent across the codebase, including updates to tests, configuration, and core policy logic. However, I've identified a critical security vulnerability in the fallback logic for inferring a server name from an FQN string, which could lead to policy bypass. My review includes a detailed explanation of the vulnerability and a suggestion for how to resolve it.
Note: Security Review did not run due to the size of the PR.
| let serverName = | ||
| typeof rawServerName === 'string' ? rawServerName : undefined; | ||
|
|
||
| if (!serverName && toolName.startsWith(MCP_TOOL_PREFIX)) { |
There was a problem hiding this comment.
Can we use the proposed parseMcpToolName() function here?
| // Check if over-ruled by an mcpName mismatch prior | ||
| let serverName: string | undefined; | ||
| // Best-effort extraction since we lack runtime metadata for processed tools list | ||
| if (toolName.startsWith(MCP_TOOL_PREFIX)) { |
gundermanc
left a comment
gundermanc
left a comment
There was a problem hiding this comment.
Approved with some suggestions.
abhipatel12
force-pushed
the
abhi/mcp-prefix
branch
from
f03ae20 to
a5b52de
Compare
abhipatel12
left a comment
abhipatel12
left a comment
There was a problem hiding this comment.
thanks for the reveiw!
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
# Conflicts: # integration-tests/policy-headless.test.ts # packages/core/src/tools/mcp-tool.test.ts # packages/core/src/tools/mcp-tool.ts # packages/core/src/tools/tool-registry.test.ts
|
🚀 Patch PR Created! 📋 Patch Details:
📝 Next Steps:
🔗 Track Progress: |
|
🚀 Patch Release Started! 📋 Release Details:
⏳ Status: The patch release is now running. You'll receive another update when it completes. 🔗 Track Progress: |
|
✅ Patch Release Complete! 📦 Release Details:
🎉 Status: Your patch has been successfully released and published to npm! 📝 What's Available:
🔗 Links: |
2 participants
Summary
Standardize MCP tool names to the
mcp_{server_name}_{tool_name}format,strictly enforcing the Gemini API regex
^[a-zA-Z_][a-zA-Z0-9_\\-.:]{0,63}$.This moves away from arbitrary string splitting based on
__in the PolicyEngine and instead relies on explicit
serverNamemetadata to match rules,making policy enforcement robust and preventing naming collisions or silent
bypasses due to tool name truncation.
Details
generateValidNameto automatically add themcp_prefix and middle-truncate names longer than 63 characters with...to prevent silent bypasses.
mcpName?: stringtoPolicyRuleandSafetyCheckerRuleexplicit server identity. ModifiedmatchesCompositePatternto depend on explicitserverNamemetadata.matchesWildcardto natively parse explicitwildcard matches like
mcp_*andmcp_serverName_*.(
settings.mcp.allowed/settings.mcp.excluded) to produce valid FQN matchesseamlessly.
tools.excludeconfiguration active insettings.jsonbecause the Policy Engine'sdecision = "deny"was confirmedto hide tools at the API schema level just like
tools.exclude; deprecationwarnings for
tools.excluderemain intact as intended.Related Issues
N/A
How to Validate
devtoolsor terminal output that registered tools begin withthe
mcp_prefix.mcpName = "my-server"with
toolName = "*"correctly allow/deny tool executions.exceeds 63 characters) result in an
ASK_USERinteraction regardless ofbroad allow policies.
npm run preflightto confirm the full suite completessuccessfully.
Pre-Merge Checklist