|
| 1 | +Open fix: improve path validation in memory discovery and resolve IDE errors |
| 2 | +#16055 sontoriyama wants to merge 5 commits into google-gemini:main from |
| 3 | +sontoriyama:fix-clean-history +261 −102 Conversation 3 Commits 5 Checks 2 Files |
| 4 | +changed 7 Conversation @sontoriyama sontoriyama commented 23 minutes ago • This |
| 5 | +PR improves the robustness of the memory discovery logic and resolves systemic |
| 6 | +IDE diagnostic errors. |
| 7 | + |
| 8 | +Key changes: |
| 9 | + |
| 10 | +Enhanced path validation and boundary checks in memory discovery. Resolved |
| 11 | +constructor parameter mismatches in DeclarativeTool. Verified stability with |
| 12 | +existing test suites. General cleanup of temporary diagnostic files. sontoriyama |
| 13 | +added 5 commits 1 hour ago @sontoriyama fix(core): add 'antigravity' as fallback |
| 14 | +binary name for Linux b727cd2 @sontoriyama fix(core): skip directories when |
| 15 | +reading GEMINI.md files 30619af @sontoriyama fix(core): improve path validation |
| 16 | +for imports and resolve test logic… d53c401 @sontoriyama security: implement |
| 17 | +path traversal protection and fix linter diagnostics 88cced0 @sontoriyama fix: |
| 18 | +resolve IDE parameter mismatches and ensure memory tool security 2c3c861 |
| 19 | +@sontoriyama sontoriyama requested a review from a team as a code owner 23 |
| 20 | +minutes ago @gemini-code-assist Contributor gemini-code-assist bot commented 23 |
| 21 | +minutes ago Summary of Changes Hello @sontoriyama, I'm Gemini Code Assist1! I'm |
| 22 | +currently reviewing this pull request and will post my feedback shortly. In the |
| 23 | +meantime, here's a summary to help you and other reviewers quickly get up to |
| 24 | +speed! |
| 25 | + |
| 26 | +This pull request significantly enhances the security and reliability of the |
| 27 | +memory management system by introducing robust path traversal prevention for |
| 28 | +GEMINI.md files and imports. It also improves the memory discovery process by |
| 29 | +gracefully handling misnamed directories. Additionally, the PR includes |
| 30 | +important dependency updates and minor fixes for IDE compatibility and editor |
| 31 | +command aliases, contributing to a more stable and secure codebase. |
| 32 | + |
| 33 | +Highlights Enhanced Security for Memory Tools: Implemented robust path traversal |
| 34 | +prevention for GEMINI.md filenames and imports, ensuring that memory files |
| 35 | +cannot access sensitive data outside designated project or global directories. |
| 36 | +Improved Memory Discovery Robustness: Added logic to silently skip directories |
| 37 | +that are mistakenly named GEMINI.md during memory discovery, preventing errors |
| 38 | +and improving user experience. Dependency Updates and Cleanup: Updated several |
| 39 | +core dependencies, including express, qs, and type-is, and refined the |
| 40 | +NOTICES.txt file to reflect these changes and remove outdated entries. IDE |
| 41 | +Compatibility Fixes: Addressed a TypeScript type assertion issue in MemoryTool |
| 42 | +tests to resolve potential IDE false positives. Expanded Editor Support: Added |
| 43 | +'antigravity' as an alias for the 'agy' command in the editor utility. Using |
| 44 | +Gemini Code Assist Footnotes Review the Privacy Notices, Generative AI |
| 45 | +Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code |
| 46 | +Assist in GitHub here. Gemini can make mistakes, so double check it and use code |
| 47 | +with caution. ↩ |
| 48 | + |
| 49 | +@sontoriyama sontoriyama changed the title fix: resolve IDE false positives and |
| 50 | +validate memory tool security security: prevent path traversal in GEMINI.md |
| 51 | +imports and fix IDE errors 20 minutes ago @sontoriyama sontoriyama changed the |
| 52 | +title security: prevent path traversal in GEMINI.md imports and fix IDE errors |
| 53 | +fix: improve path validation in memory discovery and resolve IDE errors 17 |
| 54 | +minutes ago gemini-code-assist[bot] gemini-code-assist bot reviewed 15 minutes |
| 55 | +ago Contributor gemini-code-assist bot left a comment Code Review This pull |
| 56 | +request introduces important security enhancements and bug fixes, primarily |
| 57 | +focused on the MemoryTool. The changes effectively add path traversal validation |
| 58 | +for memory file names and memory imports, which is a significant improvement. |
| 59 | +The accompanying tests are thorough and cover critical security scenarios, such |
| 60 | +as preventing imports from outside the project root. The handling of directories |
| 61 | +named like memory files also adds to the robustness of the memory discovery |
| 62 | +process. |
| 63 | + |
| 64 | +I have found one critical security vulnerability that remains in the |
| 65 | +setGeminiMdFilename function, where the validation is incomplete and still |
| 66 | +allows for path traversal using . or .. as filenames. My review includes a |
| 67 | +specific suggestion to address this. |
| 68 | + |
| 69 | +Overall, this is a valuable contribution to the security and stability of the |
| 70 | +tool. Once the identified issue is resolved, this PR will be in great shape. |
| 71 | + |
| 72 | +packages/core/src/tools/memoryTool.ts Comment on lines +75 to +79 if |
| 73 | +(trimmed.includes('/') || trimmed.includes('\\')) { throw new Error( |
| 74 | +`Invalid GEMINI.md filename: ${trimmed}. Filenames cannot contain path separators.`, |
| 75 | +); } Contributor @gemini-code-assist gemini-code-assist bot 15 minutes ago |
| 76 | +critical |
| 77 | + |
| 78 | +The validation for newFilename is a good security measure, but it's incomplete. |
| 79 | +It currently checks for path separators (/ and \\) but fails to prevent the use |
| 80 | +of . or .. as a filename. This allows for a path traversal vulnerability. For |
| 81 | +instance, setting newFilename to '..' would cause functions like |
| 82 | +getGlobalMemoryFilePath to construct a path that traverses up one directory, |
| 83 | +enabling file writes outside the intended .gemini directory. To mitigate this, |
| 84 | +the validation should also explicitly disallow . and .. as filenames. |
| 85 | + |
| 86 | +Suggested change if (trimmed.includes('/') || trimmed.includes('\\')) { throw |
| 87 | +new Error( |
| 88 | +`Invalid GEMINI.md filename: ${trimmed}. Filenames cannot contain path separators.`, |
| 89 | +); } if (trimmed.includes('/') || trimmed.includes('\\') || trimmed === '.' || |
| 90 | +trimmed === '..') { throw new Error( |
| 91 | +`Invalid GEMINI.md filename: ${trimmed}. Filenames cannot contain path separators or be '.' or '..'.`, |
| 92 | +); } @sontoriyama Reply... Merge info Review required Code owner review required |
| 93 | +by reviewers with write access. |
| 94 | + |
| 95 | +3 workflows awaiting approval This workflow requires approval from a maintainer. |
| 96 | +Learn more about approving workflows. |
| 97 | + |
| 98 | +pending checks CI CIExpected — Waiting for status to be reported Required E2E |
| 99 | +(Chained) E2E (Chained)Expected — Waiting for status to be reported Required |
| 100 | +successful checks cla/google cla/googleSuccessful in 1s — ✅ All contributors |
| 101 | +are covered under a CLA with Google Required Gemini Automated PR Labeler / |
| 102 | +label-pr (pull_request_target) Gemini Automated PR Labeler / label-pr |
| 103 | +(pull_request_target)Successful in 1m Merging is blocked Waiting on code owner |
| 104 | +review from google-gemini/gemini-cli-maintainers. |
0 commit comments