-
Notifications
You must be signed in to change notification settings - Fork 10.5k
fix: add missing type field to MCPServerConfig
#15465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @jackwotherspoon, 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 addresses a regression where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
type field to MCPServerConfig
|
Size Change: +277 B (0%) Total Size: 22 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly adds the missing "type" field to the "MCPServerConfig" schema, resolving a startup failure. However, it introduces a high-severity Command Injection vulnerability by making it easier to configure MCP servers to run local commands ("stdio" transport), potentially leading to arbitrary code execution. This also creates a critical risk of silent misconfiguration, necessitating stricter validation for robust and unambiguous configuration.
jacob314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved after one minor testing comment is addressed.
jacob314
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
/patch preview |
|
✅ Patch workflow(s) dispatched successfully! 📋 Details:
🔗 Track Progress: |
|
🚀 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: |

Summary
Fix regression where
typewas added togemini mcp addcommand but notMCPServerConfigcausing startup to fail with servers added withgemini mcp addcommand.typefield toMCPServerConfigschema with enum values['sse', 'http']urlfield description to clarify it works with SSE and HTTP transportstypefieldDetails
The
gemini mcp addcommand was updated to write atypefield (e.g.,type: 'sse'ortype: 'http') when adding remote MCP servers.However, the
MCPServerConfigschema insettingsSchema.tswas not updated to include this field.Since the schema uses
additionalProperties: false, Zod validation rejects any unknown properties, causing startup failures:Test plan
type: 'sse',type: 'http'Related Issues
Fixes #15450
Fixes #15449
How to Validate
Pre-Merge Checklist