fix(patch): cherry-pick 24adacd to release/v0.34.0-preview.2-pr-22332 to patch version v0.34.0-preview.2 and create version 0.34.0-preview.3#22391
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 refactors the token storage mechanism to enhance robustness and simplify the logic for handling secure credentials. The primary change involves centralizing the fallback from native OS keychain to encrypted file storage within the Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request refactors the credential storage by consolidating file-based and keychain-based storage into a single KeychainService with a file-based fallback. However, the encryption key derivation in the new FileKeychain is weak, relying on a hardcoded password and non-secret system information, which effectively reduces the encryption to obfuscation. Additionally, this refactoring introduces a critical data loss issue as it lacks logic to migrate existing tokens from the old mcp-oauth-tokens-v2.json file to the new gemini-credentials.json format, impacting users who relied on file-based storage. A more secure key derivation method and a migration path are needed to address these issues.
| constructor() { | ||
| const configDir = path.join(homedir(), GEMINI_DIR); | ||
| this.tokenFilePath = path.join(configDir, 'gemini-credentials.json'); | ||
| this.encryptionKey = this.deriveEncryptionKey(); | ||
| } |
There was a problem hiding this comment.
This new FileKeychain implementation introduces a critical data loss issue for users who were previously using the file-based token storage. The old implementation (FileTokenStorage) used mcp-oauth-tokens-v2.json, but this new implementation uses gemini-credentials.json with a different internal structure and does not attempt to migrate existing data.
This will cause users who relied on the file-based fallback to lose all their saved OAuth tokens upon updating.
A migration path should be implemented. For example, in the constructor or loadData, you could check for the existence of the old file, read and convert the data to the new format, save it to the new file, and then delete or rename the old file to prevent re-migration.
| const salt = `${os.hostname()}-${os.userInfo().username}-gemini-cli`; | ||
| return crypto.scryptSync('gemini-cli-oauth', salt, 32); |
There was a problem hiding this comment.
The FileKeychain class, which serves as the fallback mechanism for secure credential storage when a native OS keychain is unavailable, uses a hardcoded password ('gemini-cli-oauth') and non-secret system information (hostname and username) to derive its encryption key via scryptSync. Because both the password and the salt are either hardcoded in the source code or easily discoverable on the system, the encryption provides very little security beyond the file system permissions. An attacker who obtains the encrypted credentials file (e.g., through backups, unauthorized disk access, or a separate vulnerability) can easily derive the encryption key and decrypt the sensitive OAuth tokens stored within.
|
Size Change: -656 B (0%) Total Size: 26.5 MB ℹ️ View Unchanged
|
This PR automatically cherry-picks commit 24adacd to patch version v0.34.0-preview.2 in the preview release to create version 0.34.0-preview.3.