feat(core): add resolveWorkspaceRelativePath and getExtensionSetting utilities#25383
feat(core): add resolveWorkspaceRelativePath and getExtensionSetting utilities#25383mahimashanware wants to merge 3 commits intogoogle-gemini:mainfrom
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 implements the second phase of the dynamic extension directory resolution plan. It focuses on hardening the core infrastructure by providing robust utilities for secure file system path resolution and centralized configuration access for extensions. These changes ensure that extension operations remain isolated within the designated workspace boundaries while improving the developer experience for 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 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new getExtensionSetting method to the Config class for retrieving extension-specific settings and adds a resolveWorkspaceRelativePath utility to the Storage class. The latter provides secure path resolution relative to the project root, including support for tilde expansion and validation against path traversal. Existing logic for getPlansDir was refactored to use this new utility, and corresponding unit tests were added or updated. I have no feedback to provide.
75c1a40 to
6a313b6
Compare
b143c40 to
c64d139
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a new getExtensionSetting method to the Config class for retrieving extension-specific settings and a resolveWorkspaceRelativePath utility in the Storage class to securely handle path resolution with tilde expansion and traversal protection. Feedback suggests refining getExtensionSetting to improve type safety by removing the generic cast, adding a check for extension activation, and returning a raw string. Additionally, for resolveWorkspaceRelativePath, it is recommended to remove the try-catch fallback to normalizePath to ensure that all paths are strictly validated using resolveToRealPath, thereby preventing potential symlink-based path traversal vulnerabilities.
6a313b6 to
092e7b8
Compare
c64d139 to
627d2a5
Compare
092e7b8 to
b288542
Compare
627d2a5 to
fe5fade
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new getExtensionSetting method to the Config class for retrieving extension-specific settings and adds a secure resolveWorkspaceRelativePath utility to the Storage class. The latter includes tilde expansion and strict path validation to prevent workspace escape vulnerabilities. Corresponding unit tests have been added to verify these new functionalities and ensure robust error handling for invalid paths. I have no feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request introduces getExtensionSetting to the Config class and a secure resolveWorkspaceRelativePath method to the Storage class, which includes tilde expansion and path traversal protection. Corresponding unit tests were added and existing tests updated. Feedback suggests refactoring getExtensionSetting to use a unique extension ID instead of a name to ensure more reliable lookups.
a8418aa to
e32ef3c
Compare
23977a3 to
023fe08
Compare
e32ef3c to
eee6466
Compare
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383). We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'. This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community! |
Summary
Phase 2 of the dynamic extension plan directory resolution implementation. This PR introduces essential utility functions for secure path resolution and extension-specific configuration retrieval.
Details
resolveWorkspaceRelativePathtopackages/core/src/config/storage.ts. This utility ensures that relative paths provided by extensions are strictly resolved within the workspace root, preventing path traversal vulnerabilities.getExtensionSettingtopackages/core/src/config/config.ts. This allows the agent to fetch configuration values (likeplan.directory) contributed by extensions while falling back to user globals or system defaults if the extension is not active or hasn't provided a value.Related Issues
Depends on Phase 1 (PR #25382).
How to Validate
Run unit tests for storage and config utilities:
npm test -w @google/gemini-cli-core -- src/config/storage.test.ts src/config/config.test.tsPre-Merge Checklist
Related to #24572.