Skip to content

fix(auth): preserve Linux encryption key fallback#371

Closed
mkrueger12 wants to merge 1 commit intogoogleworkspace:mainfrom
mkrueger12:bugfix-364
Closed

fix(auth): preserve Linux encryption key fallback#371
mkrueger12 wants to merge 1 commit intogoogleworkspace:mainfrom
mkrueger12:bugfix-364

Conversation

@mkrueger12
Copy link

@mkrueger12 mkrueger12 commented Mar 10, 2026

Description

Fix a Linux credential storage regression in v0.9.1 where encrypted OAuth credentials can become undecryptable on the same machine after the key-storage change in 5872dbe.

This PR keeps a stable local .encryption_key fallback on Linux even when OS keyring operations appear to succeed.

Fixes #364.

Dry Run Output:

// Not applicable: this PR does not add a new feature or command.

Problem

Issue #364 reports a Linux environment where:

  • credentials.enc exists
  • gws auth status shows encrypted_credentials_exists: true
  • encryption_valid: false
  • authenticated commands fail with:
    • Failed to decrypt credentials: Decryption failed. Credentials may have been created on a different machine

The environment is a Debian/headless-ish setup with DBus present and libsecret installed, but without a clearly stable desktop keyring session.

Root cause

Commit 5872dbe changed the key persistence behavior to stop keeping .encryption_key once the OS keyring was available.

That is fine when the keyring is reliably available across fresh processes, but it is too optimistic for some Linux/Secret Service setups:

  1. one process successfully writes the encryption key into the keyring
  2. the CLI removes or does not keep the local .encryption_key fallback
  3. a later process cannot retrieve the same key from Secret Service
  4. credentials.enc is still present, but it can no longer be decrypted

In other words, the write path can succeed once while the read path is not stable across invocations.

What this PR changes

Linux behavior

On Linux, after a successful keyring read/write, keep .encryption_key synchronized as a stable fallback instead of removing it.

This applies to:

  • reading an existing key from the keyring
  • migrating a file-based key into the keyring
  • generating a new key and successfully storing it in the keyring

Non-Linux behavior

Keep the current behavior:

  • if keyring access succeeds, the local fallback file is removed
  • the keyring remains authoritative

Why this approach

The regression appears to be Linux-specific and tied to environments where Secret Service is partially available but not reliably retrievable across processes.

Keeping the fallback on Linux is a conservative durability fix:

  • it restores stable decryption across invocations
  • it avoids making credentials.enc unreadable on the same machine
  • it preserves the newer keyring integration instead of reverting it entirely

Security tradeoff

This does slightly reduce defense-in-depth on Linux compared to keyring-only storage, because the AES key may also exist on disk in .encryption_key.

That said:

  • the file is written with restrictive permissions (0600, parent dir 0700)
  • this does not introduce remote code execution or privilege escalation
  • it is a local at-rest secret storage tradeoff in exchange for restoring reliable credential decryption

A stricter future approach could try to prove keyring stability across fresh processes before deleting the fallback. For this regression fix, preserving the fallback on Linux seemed like the least surprising and safest behavior.

Tests and validation

Added coverage for the platform strategy in credential_store:

  • sync_key_file_after_keyring_success_matches_platform_strategy
    • verifies that Linux persists the fallback key file
    • verifies that other platforms remove it
  • linux_strategy_persists_fallback_key_material
    • verifies the Linux fallback file contains the expected key material

Also added the required changeset file:

  • .changeset/polite-eggs-burn.md

Commands run locally:

cargo fmt --all
cargo test credential_store -- --nocapture
cargo clippy -- -D warnings

Reproduction path validated during investigation

Checked out v0.9.1 and traced the regression to the current logic in src/credential_store.rs:

  • successful keyring access removes .encryption_key
  • successful key generation + keyring store does not retain a file fallback

That matches the failure mode reported in #364.

Checklist

  • My code follows the AGENTS.md guidelines (no generated google-* crates).
  • I have run cargo fmt --all to format the code perfectly.
  • I have run cargo clippy -- -D warnings and resolved all warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have provided a Changeset file (e.g. via pnpx changeset) to document my changes.

@changeset-bot
Copy link

changeset-bot bot commented Mar 10, 2026

🦋 Changeset detected

Latest commit: de50550

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@googleworkspace/cli Patch

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

@google-cla
Copy link

google-cla bot commented Mar 10, 2026

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.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, 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 resolves a critical regression affecting Linux users, specifically those in headless or unstable keyring environments, where previously stored encrypted credentials could become inaccessible. By implementing a conservative strategy to always maintain a local fallback encryption key on Linux, the change ensures the durability and reliability of credential decryption without fully reverting the benefits of OS keyring integration.

Highlights

  • Linux Credential Storage Regression Fix: Addressed a regression in v0.9.1 where encrypted OAuth credentials on Linux could become undecryptable due to changes in key storage, specifically when the OS keyring is unreliable.
  • Encryption Key Fallback Persistence: Modified the behavior on Linux to always keep a local .encryption_key fallback file synchronized, even when OS keyring operations succeed, to ensure stable decryption.
  • Platform-Specific Keyring Integration: Introduced a platform-specific strategy where Linux persists the fallback key, while other operating systems continue to remove the local fallback if keyring access is successful.
  • Test Coverage Expanded: Added new tests (sync_key_file_after_keyring_success_matches_platform_strategy and linux_strategy_persists_fallback_key_material) to verify the platform-specific key persistence logic.
Changelog
  • src/credential_store.rs
    • Introduced should_persist_fallback_key function to determine if the local fallback key should be kept based on the operating system.
    • Added persist_fallback_key function to handle saving the encryption key to a file with error logging.
    • Implemented sync_key_file_after_keyring_success to conditionally persist or remove the local fallback key based on the platform strategy.
    • Modified get_or_create_key to use sync_key_file_after_keyring_success when a key is retrieved from the keyring, migrated to the keyring, or newly generated and stored in the keyring.
    • Removed unconditional file deletion logic in favor of the new platform-aware synchronization function.
    • Added two new test cases to validate the platform-specific key persistence behavior.
Activity
  • The author identified and investigated a regression in v0.9.1 related to Linux credential storage.
  • A reproduction path for the issue was validated during the investigation.
  • New test cases were added to cover the platform-specific key persistence strategy.
  • The credential_store test suite was run locally and passed.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Generative AI Prohibited Use Policy, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly addresses a critical regression on Linux where encrypted credentials could become unreadable. The approach of maintaining a fallback encryption key file on Linux, even when the OS keyring is accessible, is a sound and conservative fix for the reported issue. The changes are well-contained within credential_store.rs and are accompanied by good unit tests.

I have one high-severity comment regarding a potential race condition in the key generation logic that could affect concurrent executions of the CLI. While this issue pre-dates this PR, it is relevant to the modified code and could lead to a similar data loss scenario.

Comment on lines +113 to +116
// Keep a local fallback on Linux because Secret
// Service availability can be transient across fresh
// processes in headless environments.
sync_key_file_after_keyring_success(&key_file, &b64_key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The get_or_create_key function appears to have a race condition when multiple gws processes run concurrently and the encryption key does not yet exist. The OnceLock only prevents races within a single process, but two separate processes could still race to create the key.

For example:

  1. Process A starts, finds no key.
  2. Process B starts, finds no key.
  3. Process A generates key K_A, saves it to the keyring and/or file.
  4. Process B generates key K_B, saves it, overwriting K_A.

If Process A has already used K_A to encrypt credentials, those credentials will become undecryptable because the key has been replaced by K_B.

To resolve this, consider using a file lock (e.g., with the fs2 crate) around the key creation logic to ensure that only one process can create the key at a time. This would make the key generation atomic across processes.

@github-actions
Copy link
Contributor

/gemini review

Signed-off-by: Max <max@backlandlabs.io>
@mkrueger12
Copy link
Author

Addressed the race condition comment.

What changed:

  • added an inter-process lock around key creation/migration in get_or_create_key() using a config-dir lock file (.encryption_key.lock)
  • re-check key resolution only after the lock is held, so concurrent fresh gws processes cannot generate different keys and overwrite each other
  • kept the Linux fallback-file behavior from the original fix

Validation run locally after the update:

  • cargo fmt --all
  • cargo test credential_store -- --nocapture
  • cargo clippy -- -D warnings

Latest commit on the branch: de50550

@github-actions
Copy link
Contributor

/gemini review

@jpoehnelt
Copy link
Member

Superseded by #359 which takes a different approach: instead of verifying the keyring roundtrip, it always preserves .encryption_key as a durable fallback and adds GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND=file for environments without a keyring. Thank you for the contribution! See #359

@jpoehnelt jpoehnelt closed this Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linux 0.9.1: encrypted credentials become undecryptable on same machine after key-storage change

3 participants