feat(credential_store): add GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND env var#359
feat(credential_store): add GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND env var#359
Conversation
Add gogcli-style backend selection for encryption key storage: - keyring (default): OS keyring with file fallback - file: .encryption_key file only (Docker/CI/headless) Never delete .encryption_key — it always serves as a durable fallback for environments where the keyring is ephemeral. When generating new keys with backend=keyring, save to both keyring and file. Extracts KeyringProvider trait + resolve_key() for testability. 25 tests covering both backends and all edge cases. Fixes #344
🦋 Changeset detectedLatest commit: 80ce461 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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 introduces a new environment variable to provide explicit control over the encryption key storage backend, addressing a critical issue where keys were lost in ephemeral environments. By ensuring the Highlights
Changelog
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 introduces a robust solution for managing encryption keys by adding the GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND environment variable and ensuring the .encryption_key file is always preserved as a fallback. The refactoring of the key retrieval logic into a testable KeyringProvider trait and resolve_key function, along with the comprehensive new test suite, significantly improves the code's maintainability and correctness.
I have one high-severity concern regarding a potential race condition during initial key generation if multiple processes run simultaneously. Please see the detailed comment in src/credential_store.rs.
Note: Security Review did not run due to the size of the PR.
Use create_new(true) (O_EXCL on Unix, CREATE_NEW on Windows) when generating a new encryption key file. If another process wins the race, read their key instead. Platform-independent.
|
/gemini review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #359 +/- ##
==========================================
+ Coverage 62.17% 63.21% +1.04%
==========================================
Files 38 38
Lines 14797 15102 +305
==========================================
+ Hits 9200 9547 +347
+ Misses 5597 5555 -42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When two processes race to create the encryption key file, the loser now syncs the winner's key back into the keyring. Without this, the keyring and file could permanently diverge.
|
/gemini review |
…ing, and race paths - save_key_file_exclusive: creates new file, rejects existing - save_key_file: overwrites existing - ensure_key_dir: creates nested dirs - KeyringBackend: file/FILE/invalid parsing - Race loser: syncs winner key to keyring - Race loser: corrupt file gets overwritten
|
/gemini review |
1. Warn on unrecognized KEYRING_BACKEND values instead of silent default 2. fsync after key file writes for crash durability 3. Zeroize decoded key material from heap after copy 4. Warn if key file has overly permissive Unix permissions (mode & 077) 5. Log which keyring backend was selected to stderr 6. Expose keyring_backend in 'gws auth status' JSON output
|
/gemini review |
Summary
Adds gogcli-style backend selection for encryption key storage, inspired by gogcli.
GOOGLE_WORKSPACE_CLI_KEYRING_BACKENDkeyring(default).encryption_keyfile fallbackfile.encryption_keyfile only — for Docker/CI/headlessKey changes
.encryption_key— it always serves as a durable fallback for ephemeral keyring environmentsbackend=keyring, save to both keyring and fileKeyringProvidertrait +resolve_key()for testabilityRoot cause (#344)
The previous fix for #344 deleted
.encryption_keywhen the OS keyring succeeded. In Docker containers, the keyring is ephemeral, so the key was permanently lost on container restart. This PR preserves the file and adds an explicit env var to bypass the keyring entirely.Fixes #344