From e943b2b0491f55db8a7ac34396f4ac9d8ea0ecfc Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 3 Mar 2026 21:17:12 -0700 Subject: [PATCH 1/4] fix(auth): stabilize encryption key fallback across runs --- src/credential_store.rs | 72 +++++++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 13 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index d146be92..2b0f33d9 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -30,10 +30,20 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { return Ok(*key); } + let cache_key = |candidate: [u8; 32]| -> [u8; 32] { + if KEY.set(candidate).is_ok() { + candidate + } else { + KEY.get().copied().unwrap_or(candidate) + } + }; + let username = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) .unwrap_or_else(|_| "unknown-user".to_string()); + let key_file = crate::auth_commands::config_dir().join(".encryption_key"); + let entry = Entry::new("gws-cli", &username); if let Ok(entry) = entry { @@ -44,30 +54,68 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - let _ = KEY.set(arr); - return Ok(arr); + return Ok(cache_key(arr)); } } } Err(keyring::Error::NoEntry) => { - // Generate a random 32-byte key + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + // If keyring is empty, prefer a persisted local key first. + if key_file.exists() { + if let Ok(b64_key) = std::fs::read_to_string(&key_file) { + if let Ok(decoded) = STANDARD.decode(b64_key.trim()) { + 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); + return Ok(cache_key(arr)); + } + } + } + } + + // Generate a random 32-byte key and persist it locally as a stable fallback. let mut key = [0u8; 32]; rand::thread_rng().fill_bytes(&mut key); - - use base64::{engine::general_purpose::STANDARD, Engine as _}; let b64_key = STANDARD.encode(key); - if entry.set_password(&b64_key).is_ok() { - let _ = KEY.set(key); - return Ok(key); + if let Some(parent) = key_file.parent() { + let _ = std::fs::create_dir_all(parent); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)); + } + } + + #[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); + } + + // Best effort: also store in keyring when available. + let _ = entry.set_password(&b64_key); + + return Ok(cache_key(key)); } Err(_) => {} // Fallthrough to file storage } } // Fallback: Local file `.encryption_key` - let key_file = crate::auth_commands::config_dir().join(".encryption_key"); + if key_file.exists() { if let Ok(b64_key) = std::fs::read_to_string(&key_file) { use base64::{engine::general_purpose::STANDARD, Engine as _}; @@ -75,8 +123,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - let _ = KEY.set(arr); - return Ok(arr); + return Ok(cache_key(arr)); } } } @@ -113,8 +160,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let _ = std::fs::write(&key_file, b64_key); } - let _ = KEY.set(key); - Ok(key) + Ok(cache_key(key)) } /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. From 6740575898de39650f2106772524a7298f8387cc Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 3 Mar 2026 21:19:39 -0700 Subject: [PATCH 2/4] chore: add changeset for auth encryption key fix --- .changeset/31852fab9121.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/31852fab9121.md diff --git a/.changeset/31852fab9121.md b/.changeset/31852fab9121.md new file mode 100644 index 00000000..001607e3 --- /dev/null +++ b/.changeset/31852fab9121.md @@ -0,0 +1,18 @@ +--- +"@googleworkspace/cli": patch +--- + +fix(auth): stabilize encrypted credential key fallback across sessions + +When the OS keyring returned `NoEntry`, the previous code could generate +a fresh random key on each process invocation instead of reusing one. +This caused `credentials.enc` written by `gws auth login` to be +unreadable by subsequent commands. + +Changes: +- Always prefer an existing `.encryption_key` file before generating a new key +- When generating a new key, persist it to `.encryption_key` as a stable fallback +- Best-effort write new keys into the keyring as well +- Fix `OnceLock` race: return the already-cached key if `set` loses a race + +Fixes #27 From c820b435cb2970c94a4ad114b7c6ddc5186f79e2 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 3 Mar 2026 21:24:20 -0700 Subject: [PATCH 3/4] chore: cargo fmt --- src/credential_store.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index 2b0f33d9..b44e4987 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -86,7 +86,10 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)); + let _ = std::fs::set_permissions( + parent, + std::fs::Permissions::from_mode(0o700), + ); } } From 79266e94964e3f697d68505bea28e92215e08ac8 Mon Sep 17 00:00:00 2001 From: jpoehnelt-bot Date: Tue, 3 Mar 2026 21:29:34 -0700 Subject: [PATCH 4/4] fix(auth): address Gemini review comments - OnceLock expect + permission warnings - Replace unwrap_or(candidate) with expect() in cache_key closure for clearer OnceLock race invariant: if set() fails, get() is guaranteed to return Some - Emit eprintln! warnings (rather than silently ignoring) when set_permissions fails on the encryption key directory, matching the warning pattern used throughout the codebase (src/auth_commands.rs, helpers/workflows.rs, etc.) --- src/credential_store.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index b44e4987..6b4eb9cc 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -34,7 +34,10 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if KEY.set(candidate).is_ok() { candidate } else { - KEY.get().copied().unwrap_or(candidate) + // If set() fails, another thread already initialized the key. .get() is + // guaranteed to return Some at this point. + *KEY.get() + .expect("key must be initialized if OnceLock::set() failed") } }; @@ -86,10 +89,13 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions( - parent, - std::fs::Permissions::from_mode(0o700), - ); + 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}" + ); + } } } @@ -144,7 +150,10 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)); + 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}"); + } } }