fix(core): made context files append instead of replace#26950
Conversation
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 modifies how context files are managed in the Gemini CLI, shifting from a replacement-based model to an additive one. By allowing multiple context files to be registered simultaneously, users can now include more diverse memory sources without overwriting previous configurations. The changes include robust handling for uniqueness and a new reset mechanism to maintain control over the configuration state. 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
|
|
Size Change: +1.03 kB (0%) Total Size: 34.1 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
This pull request updates the memory tool to support multiple context filenames by modifying setGeminiMdFilename to append new names and introducing a resetGeminiMdFilename function. Feedback identifies critical path traversal vulnerabilities in both functions due to a lack of input sanitization. Additionally, the reviewer suggested a logic improvement for setGeminiMdFilename to ensure that newly added filenames take precedence over existing ones.
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 modifies the memory tool configuration to support appending multiple context files rather than overwriting the previous configuration. This change improves flexibility for users who need to include multiple context sources simultaneously. A new reset function has been introduced to manage the state explicitly, ensuring backward compatibility and cleaner configuration management. 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
|
There was a problem hiding this comment.
Code Review
This pull request updates the memory tool to support multiple context filenames, modifying setGeminiMdFilename to append unique names and introducing resetGeminiMdFilename to manage the list. Feedback identifies critical path traversal vulnerabilities in both functions, suggesting the need for path normalization and sanitization. Additionally, the reviewer recommends refactoring the module-level global state to instance-scoped properties to avoid potential race conditions in concurrent environments.
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 refactors the mechanism for managing context files within the application. Previously, setting a new context filename would overwrite any existing configuration. The updated logic now allows for context files to be appended, enabling the use of multiple context files concurrently. A dedicated reset function has also been added to provide explicit control over clearing and re-establishing context file settings. 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
|
There was a problem hiding this comment.
Code Review
This pull request updates the memoryTool to support multiple context filenames by modifying setGeminiMdFilename to append unique names and introducing a resetGeminiMdFilename function. Feedback highlights critical security vulnerabilities related to path traversal in both functions due to a lack of input sanitization. Additionally, the reviewer noted that using a module-level global variable for mutable state violates project guidelines for concurrent environments and recommended refactoring to instance-scoped state.
Fix path traversal for `resetGeminiMdFilename` Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the MemoryTool to support multiple context filenames by modifying setGeminiMdFilename to append unique, sanitized filenames and introducing resetGeminiMdFilename for replacing the filename list. The CLI configuration and associated tests were updated to reflect these changes. Feedback highlights potential path traversal vulnerabilities on Windows within the manual sanitization logic, recommending the use of a centralized resolveToRealPath utility for more robust validation. Additionally, an import update was suggested in the CLI configuration to support idempotent loading logic.
cocosheng-g
left a comment
There was a problem hiding this comment.
please address bot comments otherwise lgtm
Summary
Made context files append instead of replace when
settings.context.fileNameis set.Details
Related Issues
Fixes #26728
How to Validate
Add
with a file
AGENTS.mdthat could contain any text. Start Gemini CLI and run/memory show. It should include the context fromAGENTS.md.Pre-Merge Checklist