fix(core): resolve symbolic link directory escape in memory import processor#28233
Conversation
|
📊 PR Size: size/M
|
|
/gemini review |
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 strengthens the security of the Gemini CLI's memory import processor by preventing directory traversal attacks via symbolic links. By shifting from string-based path validation to physical disk canonicalization, the system now correctly identifies and blocks attempts to access files outside of the intended workspace boundaries, regardless of how the file system structure is represented. 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 updates the validateImportPath function in memoryImportProcessor.ts to canonicalize paths using resolveToRealPath, preventing directory traversal escapes via symbolic links, and adds a corresponding test. The review feedback recommends optimizing performance by pre-resolving allowed directories outside the .some loop to avoid redundant synchronous disk I/O. Additionally, for the test suite, it suggests using fsSync.mkdtempSync() to securely generate temporary directories and handling potential permission errors on Windows when creating symbolic links.
There was a problem hiding this comment.
Code Review
This pull request enhances path validation in memoryImportProcessor.ts by canonicalizing paths using resolveToRealPath to prevent directory traversal via symbolic links, and adds a corresponding unit test. The review feedback correctly identifies potential test pollution if setup steps fail outside the try-finally block, as well as potential test failures on Windows due to symlink creation permissions, recommending wrapping the setup in the try block and gracefully skipping the test on Windows if permissions are missing.
gemini-cli
Bot
added
the
status/need-issue
luisfelipe-alt
force-pushed
the
bugfix/WT-engineer_522410449
branch
from
c0f11c4 to
e8e053d
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances path validation in memoryImportProcessor.ts by resolving both the target import path and the allowed directories to their real physical paths using resolveToRealPath. This prevents directory traversal vulnerabilities through symbolic links. Additionally, a unit test has been added to verify this behavior, with cross-platform handling for Windows symlink permissions. There are no review comments, so no feedback is provided.
2 participants
Summary
This PR resolves a high-severity Symbolic Link Directory Escape vulnerability in the JIT Memory Import Processor of Gemini CLI.
By exploiting this flaw, an attacker could craft a malicious repository containing a directory symbolic link pointing outside the workspace (e.g.,
src/sym_home->/home/username/) and reference a sensitive file through it inGEMINI.md(e.g.,@./src/sym_home/.ssh/id_rsa). When opened, the CLI would silently read and exfiltrate the sensitive host file into the LLM's prompt memory context.This fix ensures that all import paths and allowed directories are physically canonicalized on disk before boundary checks are performed, establishing a secure-by-default "Zero-Trust" workspace boundary.
Details
path.resolvewith the project's pre-existing, robustresolveToRealPathutility (frompackages/core/src/utils/paths.ts) insidevalidateImportPath.try-catchblock to ensure that if resolution fails (e.g., due to malicious recursive symlink loops or invalid paths), the import is rejected (false).packages/core/src/utils/memoryImportProcessor.test.tsthat creates a real temporary directory structure and directory symlink on disk to verify that out-of-bounds symlink escapes are successfully blocked.Related Issues
How to Validate
Run the specific unit tests:
Expected Result: All 26 tests pass cleanly, including
should reject paths that escape allowed directories via symbolic links.Run the linter:
Expected Result: Completes successfully with zero warnings or errors.
Run type-checking:
Expected Result: Completes successfully with zero errors.
Pre-Merge Checklist