fix(auth): verify keyring roundtrip before deleting .encryption_key#379
fix(auth): verify keyring roundtrip before deleting .encryption_key#379alexriftagentic wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
On macOS (and some Linux environments), the OS keyring `set_password()` returns Ok(()) even when the entry is not actually persisted — e.g. for ad-hoc signed npm-installed binaries that lack Keychain entitlements. The previous code deleted the `.encryption_key` file after a seemingly successful keyring write. On the next process launch, `get_password()` would fail to find the entry, the file was gone, so a new random key was generated — permanently losing access to existing credentials.enc. This commit makes three changes: 1. Migration path: after writing to keyring, verify the roundtrip by reading back immediately. Only delete the file if readback matches. 2. New key generation: always write to the local file first, then optionally store in keyring. The file is the source of truth. 3. Keyring-read-success path: stop deleting the file copy when the keyring read succeeds — keep it as a durable backup. Fixes googleworkspace#360, googleworkspace#364, googleworkspace#367, googleworkspace#375, googleworkspace#274
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 addresses a critical credential decryption bug that rendered 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
|
![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
The pull request modifies the get_or_create_key function in src/credential_store.rs to enhance the robustness and reliability of encryption key storage. The changes prioritize file-based storage as a primary backup mechanism and introduce more resilient interactions with the OS keyring. Specifically, when a key is successfully read from the keyring, the local key file is now retained as a backup instead of being removed. During key migration from a file to the keyring, a roundtrip verification step has been added: the file is only removed if the key can be successfully written to and read back from the keyring, preventing potential data loss. Furthermore, when a new key is generated, it is now always persisted to a local file first, with keyring storage becoming an optional, secondary step for convenience, ensuring the key is never lost even if keyring operations fail.
4 participants
Summary
Fixes the credential decryption bug that makes
gwsunusable on macOS (and some Linux environments) aftergws auth login.get_or_create_key()incredential_store.rsdeletes the.encryption_keyfile afterentry.set_password()returnsOk(()). On macOS with ad-hoc signed binaries (e.g. npm-installed), the Keychain write silently fails to persist — so the next process invocation can't read the key back, generates a new random key, and existingcredentials.encbecomes permanently undecryptable."encryption_error": "Could not decrypt. May have been created on a different machine."immediately after a successfulgws auth login, on the same machine.Changes
set_password(), verify the roundtrip withget_password(). Only delete.encryption_keyif readback matches..encryption_keyfile first (source of truth), then optionally store in keyring. Previously it wrote to keyring only and skipped the file ifset_password()returnedOk..encryption_keyfile when keyring read succeeds — keep it as a durable backup for environments where keyring access is intermittent.Test plan
cargo checkpassesgws auth login→gws auth statusshowsencryption_valid: true.encryption_keyfile is preserved after loginencrypt_decrypt_round_trip, etc.)Fixes
Closes #360, #364, #367, #375, #274
🤖 Generated with Claude Code