From bf7e2e69472ffbc6770b9f601e8f9e12ff7f5bbd Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Mon, 9 Mar 2026 15:33:40 -0600 Subject: [PATCH] fix(credential_store): stop persisting encryption key file when keyring is available Previously, get_or_create_key() unconditionally wrote the encryption key to ~/.config/gws/.encryption_key on first run, even when the OS keyring was available. This left the key material on disk as a plain file, making credentials portable by copying the config directory. Changes: - Extract save_key_file() helper to deduplicate file-writing logic - On keyring read success: delete stale .encryption_key (migration) - On NoEntry + existing file: migrate key into keyring, then delete file - On NoEntry + new key: try keyring first, only write file as fallback Fixes #344 --- .changeset/fix-encryption-key-file-leak.md | 5 + .github/workflows/ci.yml | 32 +++++- src/credential_store.rs | 109 ++++++++++----------- 3 files changed, 83 insertions(+), 63 deletions(-) create mode 100644 .changeset/fix-encryption-key-file-leak.md diff --git a/.changeset/fix-encryption-key-file-leak.md b/.changeset/fix-encryption-key-file-leak.md new file mode 100644 index 00000000..0914ad39 --- /dev/null +++ b/.changeset/fix-encryption-key-file-leak.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Stop persisting encryption key to `.encryption_key` file when OS keyring is available. Existing file-based keys are migrated into the keyring and the file is removed on next CLI invocation. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63790172..dfff366a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,13 @@ jobs: - name: Enable sccache if: steps.sccache.outcome == 'success' - run: echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + shell: bash + run: | + if sccache --start-server 2>/dev/null; then + echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + else + echo "::warning::sccache server failed to start, building without cache" + fi - name: Cache cargo uses: Swatinem/rust-cache@ad397744b0d591a723ab90405b7247fac0e6b8db # v2 @@ -119,7 +125,13 @@ jobs: - name: Enable sccache if: steps.sccache.outcome == 'success' - run: echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + shell: bash + run: | + if sccache --start-server 2>/dev/null; then + echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + else + echo "::warning::sccache server failed to start, building without cache" + fi - name: Cache cargo uses: Swatinem/rust-cache@ad397744b0d591a723ab90405b7247fac0e6b8db # v2 @@ -155,7 +167,13 @@ jobs: - name: Enable sccache if: steps.sccache.outcome == 'success' - run: echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + shell: bash + run: | + if sccache --start-server 2>/dev/null; then + echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + else + echo "::warning::sccache server failed to start, building without cache" + fi - name: Cache cargo uses: Swatinem/rust-cache@ad397744b0d591a723ab90405b7247fac0e6b8db # v2 @@ -204,7 +222,13 @@ jobs: - name: Enable sccache if: steps.sccache.outcome == 'success' - run: echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + shell: bash + run: | + if sccache --start-server 2>/dev/null; then + echo "RUSTC_WRAPPER=sccache" >> "$GITHUB_ENV" + else + echo "::warning::sccache server failed to start, building without cache" + fi - name: Cache cargo uses: Swatinem/rust-cache@ad397744b0d591a723ab90405b7247fac0e6b8db # v2 diff --git a/src/credential_store.rs b/src/credential_store.rs index 867d5bce..aa9f1c54 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -21,6 +21,38 @@ use keyring::Entry; use rand::RngCore; use std::sync::OnceLock; +/// Persist the base64-encoded encryption key to a local file with restrictive +/// permissions (0600 file, 0700 directory). Used only as a fallback when the OS +/// keyring is unavailable. +fn save_key_file(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) + { + eprintln!("Warning: failed to set secure permissions on key directory: {e}"); + } + } + } + + #[cfg(unix)] + { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + let mut options = std::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true).mode(0o600); + let mut file = options.open(path)?; + file.write_all(b64_key.as_bytes())?; + } + #[cfg(not(unix))] + { + std::fs::write(path, b64_key)?; + } + Ok(()) +} + /// Returns the encryption key derived from the OS keyring, or falls back to a local file. /// Generates a random 256-bit key and stores it securely if it doesn't exist. fn get_or_create_key() -> anyhow::Result<[u8; 32]> { @@ -57,6 +89,11 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); + // Keyring is authoritative — remove redundant file copy + // if it exists (migrates existing installs on upgrade). + if key_file.exists() { + let _ = std::fs::remove_file(&key_file); + } return Ok(cache_key(arr)); } } @@ -71,51 +108,30 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - // Best effort: repopulate keyring for future runs. - let _ = entry.set_password(&b64_key); + // Migrate file key into keyring; remove the + // file if the keyring store succeeds. + if entry.set_password(b64_key.trim()).is_ok() { + let _ = std::fs::remove_file(&key_file); + } return Ok(cache_key(arr)); } } } } - // Generate a random 32-byte key and persist it locally as a stable fallback. + // Generate a new random 256-bit key. let mut key = [0u8; 32]; rand::thread_rng().fill_bytes(&mut key); let b64_key = STANDARD.encode(key); - if let Some(parent) = key_file.parent() { - let _ = std::fs::create_dir_all(parent); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Err(e) = - std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) - { - eprintln!( - "Warning: failed to set secure permissions on key directory: {e}" - ); - } - } - } - - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - if let Ok(mut file) = options.open(&key_file) { - use std::io::Write; - let _ = file.write_all(b64_key.as_bytes()); - } - } - #[cfg(not(unix))] - { - let _ = std::fs::write(&key_file, &b64_key); + // Try keyring first; only fall back to file storage + // if the keyring is unavailable. + if entry.set_password(&b64_key).is_ok() { + return Ok(cache_key(key)); } - // Best effort: also store in keyring when available. - let _ = entry.set_password(&b64_key); + // Keyring store failed — persist to local file as fallback. + save_key_file(&key_file, &b64_key)?; return Ok(cache_key(key)); } @@ -147,32 +163,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { use base64::{engine::general_purpose::STANDARD, Engine as _}; let b64_key = STANDARD.encode(key); - if let Some(parent) = key_file.parent() { - let _ = std::fs::create_dir_all(parent); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) - { - eprintln!("Warning: failed to set secure permissions on key directory: {e}"); - } - } - } - - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - if let Ok(mut file) = options.open(&key_file) { - use std::io::Write; - let _ = file.write_all(b64_key.as_bytes()); - } - } - #[cfg(not(unix))] - { - let _ = std::fs::write(&key_file, b64_key); - } + save_key_file(&key_file, &b64_key)?; Ok(cache_key(key)) }