diff --git a/.changeset/fix-keyring-fallback.md b/.changeset/fix-keyring-fallback.md new file mode 100644 index 00000000..2562dd1f --- /dev/null +++ b/.changeset/fix-keyring-fallback.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": minor +--- + +Add `GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND` env var for explicit keyring backend selection (`keyring` or `file`). Fixes credential key loss in Docker/keyring-less environments by never deleting `.encryption_key` and always persisting it as a fallback. diff --git a/AGENTS.md b/AGENTS.md index 4fa127b6..5b138897 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -171,7 +171,8 @@ Use these labels to categorize pull requests and issues: | Variable | Description | |---|---| | `GOOGLE_WORKSPACE_CLI_TOKEN` | Pre-obtained OAuth2 access token (highest priority; bypasses all credential file loading) | -| `GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE` | Path to OAuth credentials JSON (no default; if unset, falls back to credentials secured by the OS Keyring and encrypted in `~/.config/gws/`) | +| `GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE` | Path to OAuth credentials JSON (no default; if unset, falls back to encrypted credentials in `~/.config/gws/`) | +| `GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND` | Keyring backend: `keyring` (default, uses OS keyring with file fallback) or `file` (file only, for Docker/CI/headless) | | `GOOGLE_APPLICATION_CREDENTIALS` | Standard Google ADC path; used as fallback when no gws-specific credentials are configured | diff --git a/Cargo.lock b/Cargo.lock index 499c612f..e40df1cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -873,6 +873,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "yup-oauth2", + "zeroize", ] [[package]] @@ -3596,6 +3597,20 @@ name = "zeroize" version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85a5b4158499876c763cb03bc4e49185d3cccbabb15b33c627f7884f43db852e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] [[package]] name = "zerotrie" diff --git a/Cargo.toml b/Cargo.toml index a78379dd..5babca80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,7 @@ keyring = "3.6.3" async-trait = "0.1.89" serde_yaml = "0.9.34" percent-encoding = "2.3.2" +zeroize = { version = "1.8.2", features = ["derive"] } # The profile that 'cargo dist' will build with diff --git a/README.md b/README.md index da1a05dc..9130ecbd 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ The CLI supports multiple auth workflows so it works on your laptop, in CI, and ### Interactive (local desktop) -Credentials are encrypted at rest (AES-256-GCM) with the key stored in your OS keyring. +Credentials are encrypted at rest (AES-256-GCM) with the key stored in your OS keyring (or `~/.config/gws/.encryption_key` when `GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND=file`). ```bash gws auth setup # one-time: creates a Cloud project, enables APIs, logs you in diff --git a/skills/gws-gmail-forward/SKILL.md b/skills/gws-gmail-forward/SKILL.md index cc0099a5..cb99a30e 100644 --- a/skills/gws-gmail-forward/SKILL.md +++ b/skills/gws-gmail-forward/SKILL.md @@ -44,6 +44,7 @@ gws gmail +forward --message-id 18f1a2b3c4d --to dave@example.com --cc eve@examp ## Tips - Includes the original message with sender, date, subject, and recipients. +- Sends the forward as a new message rather than forcing it into the original thread. ## See Also diff --git a/src/auth_commands.rs b/src/auth_commands.rs index f4ffacc7..ade681d6 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -359,7 +359,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { "message": "Authentication successful. Encrypted credentials saved.", "account": actual_email.as_deref().unwrap_or("(unknown)"), "credentials_file": enc_path.display().to_string(), - "encryption": "AES-256-GCM (key secured by OS Keyring or local `.encryption_key`)", + "encryption": "AES-256-GCM (key in OS keyring or local `.encryption_key`; set GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND=file for headless)", "scopes": scopes, }); println!( @@ -944,6 +944,7 @@ async fn handle_status() -> Result<(), GwsError> { let mut output = json!({ "auth_method": auth_method, "storage": storage, + "keyring_backend": credential_store::active_backend_name(), "encrypted_credentials": enc_path.display().to_string(), "encrypted_credentials_exists": has_encrypted, "plain_credentials": plain_path.display().to_string(), diff --git a/src/credential_store.rs b/src/credential_store.rs index aa9f1c54..a0210fc2 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -20,11 +20,10 @@ use aes_gcm::{AeadCore, Aes256Gcm, Nonce}; use keyring::Entry; use rand::RngCore; use std::sync::OnceLock; +use zeroize::Zeroize; -/// 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<()> { +/// Ensure the key-file parent directory exists with restrictive permissions. +fn ensure_key_dir(path: &std::path::Path) -> std::io::Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; #[cfg(unix)] @@ -36,15 +35,44 @@ fn save_key_file(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> { } } } + Ok(()) +} + +/// Atomically create a **new** key file using `create_new(true)` (`O_EXCL` on +/// Unix, `CREATE_NEW` on Windows). If another process already created the file, +/// returns `Err` with `ErrorKind::AlreadyExists` so the caller can read the +/// winner's key instead. +fn save_key_file_exclusive(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> { + use std::io::Write; + ensure_key_dir(path)?; + + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); // atomic: fails if file already exists + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut file = opts.open(path)?; + file.write_all(b64_key.as_bytes())?; + file.sync_all()?; // fsync: ensure key is durable before returning + Ok(()) +} + +/// Persist the base64-encoded encryption key to a local file with restrictive +/// permissions (0600 file, 0700 directory). Overwrites any existing file. +fn save_key_file(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> { + use std::io::Write; + ensure_key_dir(path)?; #[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())?; + file.sync_all()?; // fsync: ensure key is durable before returning } #[cfg(not(unix))] { @@ -53,87 +81,183 @@ fn save_key_file(path: &std::path::Path, b64_key: &str) -> std::io::Result<()> { 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]> { - static KEY: OnceLock<[u8; 32]> = OnceLock::new(); +/// Read and decode a base64-encoded 256-bit key from a file. +/// +/// On Unix, warns if the file is world-readable (mode & 0o077 != 0). +fn read_key_file(path: &std::path::Path) -> Option<[u8; 32]> { + use base64::{engine::general_purpose::STANDARD, Engine as _}; - if let Some(key) = KEY.get() { - return Ok(*key); + // Item 4: validate file permissions on read + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if let Ok(meta) = std::fs::metadata(path) { + let mode = meta.permissions().mode(); + if mode & 0o077 != 0 { + eprintln!( + "Warning: encryption key file {} has overly permissive mode {:04o}. \ + Expected 0600. Run: chmod 600 {}", + path.display(), + mode & 0o777, + path.display() + ); + } + } } - let cache_key = |candidate: [u8; 32]| -> [u8; 32] { - if KEY.set(candidate).is_ok() { - candidate - } else { - // 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") + let b64_key = std::fs::read_to_string(path).ok()?; + let mut decoded = STANDARD.decode(b64_key.trim()).ok()?; + if decoded.len() == 32 { + let mut arr = [0u8; 32]; + arr.copy_from_slice(&decoded); + decoded.zeroize(); // scrub decoded key material from heap + Some(arr) + } else { + decoded.zeroize(); + None + } +} + +/// Generate a random 256-bit key. +fn generate_random_key() -> [u8; 32] { + let mut key = [0u8; 32]; + rand::thread_rng().fill_bytes(&mut key); + key +} + +/// Abstraction over OS keyring operations for testability. +trait KeyringProvider { + /// Attempt to read the stored password. + fn get_password(&self) -> Result; + /// Attempt to store a password in the keyring. + fn set_password(&self, password: &str) -> Result<(), keyring::Error>; +} + +/// Production keyring implementation wrapping an optional `keyring::Entry`. +struct OsKeyring(Option); + +impl OsKeyring { + fn new(service: &str, user: &str) -> Self { + Self(Entry::new(service, user).ok()) + } +} + +impl KeyringProvider for OsKeyring { + fn get_password(&self) -> Result { + match &self.0 { + Some(entry) => entry.get_password(), + None => Err(keyring::Error::NoEntry), } - }; + } - let username = std::env::var("USER") - .or_else(|_| std::env::var("USERNAME")) - .unwrap_or_else(|_| "unknown-user".to_string()); + fn set_password(&self, password: &str) -> Result<(), keyring::Error> { + match &self.0 { + Some(entry) => entry.set_password(password), + None => Err(keyring::Error::NoEntry), + } + } +} - let key_file = crate::auth_commands::config_dir().join(".encryption_key"); +/// Which backend to use for encryption key storage. +/// +/// Controlled by `GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND`: +/// - `"keyring"` (default): Use OS keyring, fall back to `.encryption_key` file +/// - `"file"`: Use `.encryption_key` file only (for Docker/CI/headless) +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyringBackend { + Keyring, + File, +} - let entry = Entry::new("gws-cli", &username); +impl KeyringBackend { + fn from_env() -> Self { + let raw = std::env::var("GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND").unwrap_or_default(); + let lower = raw.to_lowercase(); + match lower.as_str() { + "file" => KeyringBackend::File, + "keyring" | "" => KeyringBackend::Keyring, + other => { + // Item 1: warn on unrecognized values + eprintln!( + "Warning: unknown GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND=\"{other}\", \ + defaulting to \"keyring\". Valid values: \"keyring\", \"file\"." + ); + KeyringBackend::Keyring + } + } + } - if let Ok(entry) = entry { - match entry.get_password() { + /// Human-readable name for logging and status output. + fn as_str(&self) -> &'static str { + match self { + KeyringBackend::Keyring => "keyring", + KeyringBackend::File => "file", + } + } +} + +/// Core key-resolution logic, separated from caching for testability. +/// +/// When `backend` is `Keyring`: +/// 1. Try keyring → 2. Try file → 3. Generate (save to keyring + file) +/// +/// When `backend` is `File`: +/// 1. Try file → 2. Generate (save to file only) +/// +/// The `.encryption_key` file is **never deleted** — it always serves as a +/// durable fallback for environments where the keyring is ephemeral. +fn resolve_key( + backend: KeyringBackend, + provider: &dyn KeyringProvider, + key_file: &std::path::Path, +) -> anyhow::Result<[u8; 32]> { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + // --- 1. Try keyring (only when backend = Keyring) -------------------- + if backend == KeyringBackend::Keyring { + match provider.get_password() { Ok(b64_key) => { - use base64::{engine::general_purpose::STANDARD, Engine as _}; if let Ok(decoded) = STANDARD.decode(&b64_key) { 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)); + return Ok(arr); } } + // Keyring contained invalid data — fall through to file. } Err(keyring::Error::NoEntry) => { - 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); - // 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)); - } - } - } + // Keyring is reachable but empty — check file, then generate. + if let Some(key) = read_key_file(key_file) { + // Best-effort: copy file key into keyring for future runs. + let _ = provider.set_password(&STANDARD.encode(key)); + return Ok(key); } - // Generate a new random 256-bit key. - let mut key = [0u8; 32]; - rand::thread_rng().fill_bytes(&mut key); + // Generate a new key. + let key = generate_random_key(); let b64_key = STANDARD.encode(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: store in keyring. + let _ = provider.set_password(&b64_key); + + // Atomically create the file; if another process raced us, + // use their key instead. + match save_key_file_exclusive(key_file, &b64_key) { + Ok(()) => return Ok(key), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + if let Some(winner) = read_key_file(key_file) { + // Sync the winner's key into the keyring so both + // backends stay consistent after the race. + let _ = provider.set_password(&STANDARD.encode(winner)); + return Ok(winner); + } + // File exists but is unreadable/corrupt — overwrite. + save_key_file(key_file, &b64_key)?; + return Ok(key); + } + Err(e) => return Err(e.into()), } - - // Keyring store failed — persist to local file as fallback. - save_key_file(&key_file, &b64_key)?; - - return Ok(cache_key(key)); } Err(e) => { eprintln!("Warning: keyring access failed, falling back to file storage: {e}"); @@ -141,31 +265,56 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { } } - // Fallback: Local file `.encryption_key` + // --- 2. File fallback ------------------------------------------------ + if let Some(key) = read_key_file(key_file) { + return Ok(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 _}; - if let Ok(decoded) = STANDARD.decode(b64_key.trim()) { - if decoded.len() == 32 { - let mut arr = [0u8; 32]; - arr.copy_from_slice(&decoded); - return Ok(cache_key(arr)); - } - } + // --- 3. Generate new key, save to file (race-safe) ------------------- + let key = generate_random_key(); + let b64_key = STANDARD.encode(key); + match save_key_file_exclusive(key_file, &b64_key) { + Ok(()) => Ok(key), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { + // Another process created the file first — use their key. + read_key_file(key_file).ok_or_else(|| anyhow::anyhow!("key file exists but is corrupt")) } + Err(e) => Err(e.into()), } +} - // Generate new key and save to local file - let mut key = [0u8; 32]; - rand::thread_rng().fill_bytes(&mut key); +/// Returns the encryption key, generating and persisting one if it doesn't exist. +/// +/// The key is cached in-process via `OnceLock` so it is only read from disk once. +/// Backend selection is controlled by `GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND`. +fn get_or_create_key() -> anyhow::Result<[u8; 32]> { + static KEY: OnceLock<[u8; 32]> = OnceLock::new(); - use base64::{engine::general_purpose::STANDARD, Engine as _}; - let b64_key = STANDARD.encode(key); + if let Some(key) = KEY.get() { + return Ok(*key); + } - save_key_file(&key_file, &b64_key)?; + let backend = KeyringBackend::from_env(); + // Item 5: log which backend was selected + eprintln!("Using keyring backend: {}", backend.as_str()); - Ok(cache_key(key)) + 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 provider = OsKeyring::new("gws-cli", &username); + + let key = resolve_key(backend, &provider, &key_file)?; + + // Cache for subsequent calls within this process. + if KEY.set(key).is_ok() { + Ok(key) + } else { + Ok(*KEY + .get() + .expect("key must be initialized if OnceLock::set() failed")) + } } /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. @@ -207,6 +356,11 @@ pub fn decrypt(data: &[u8]) -> anyhow::Result> { Ok(plaintext) } +/// Returns the name of the active keyring backend for status display. +pub fn active_backend_name() -> &'static str { + KeyringBackend::from_env().as_str() +} + /// Returns the path for encrypted credentials. pub fn encrypted_credentials_path() -> PathBuf { crate::auth_commands::config_dir().join("credentials.enc") @@ -267,6 +421,266 @@ pub fn load_encrypted() -> anyhow::Result { #[cfg(test)] mod tests { use super::*; + use std::cell::RefCell; + + /// Describes what `get_password` / `set_password` should return. + #[derive(Clone)] + enum MockState { + Ok(String), + NoEntry, + PlatformError, + } + + /// Mock keyring for testing `resolve_key()` without OS dependencies. + struct MockKeyring { + get_state: MockState, + set_succeeds: bool, + last_set: RefCell>, + } + + impl MockKeyring { + fn with_password(b64: &str) -> Self { + Self { + get_state: MockState::Ok(b64.to_string()), + set_succeeds: true, + last_set: RefCell::new(None), + } + } + + fn no_entry() -> Self { + Self { + get_state: MockState::NoEntry, + set_succeeds: true, + last_set: RefCell::new(None), + } + } + + fn platform_error() -> Self { + Self { + get_state: MockState::PlatformError, + set_succeeds: true, + last_set: RefCell::new(None), + } + } + + fn with_set_failure(mut self) -> Self { + self.set_succeeds = false; + self + } + } + + impl KeyringProvider for MockKeyring { + fn get_password(&self) -> Result { + match &self.get_state { + MockState::Ok(s) => Ok(s.clone()), + MockState::NoEntry => Err(keyring::Error::NoEntry), + MockState::PlatformError => { + Err(keyring::Error::PlatformFailure("mock: no backend".into())) + } + } + } + + fn set_password(&self, password: &str) -> Result<(), keyring::Error> { + *self.last_set.borrow_mut() = Some(password.to_string()); + if self.set_succeeds { + Ok(()) + } else { + Err(keyring::Error::NoEntry) + } + } + } + + fn write_test_key(dir: &std::path::Path) -> ([u8; 32], std::path::PathBuf) { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let key = [42u8; 32]; + let path = dir.join(".encryption_key"); + std::fs::write(&path, STANDARD.encode(key)).unwrap(); + (key, path) + } + + // ---- Backend::Keyring tests ---- + + #[test] + fn keyring_backend_returns_keyring_key() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let expected = [7u8; 32]; + let mock = MockKeyring::with_password(&STANDARD.encode(expected)); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, expected); + } + + #[test] + fn keyring_backend_keeps_file_when_keyring_succeeds() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let (_, key_file) = write_test_key(dir.path()); + let mock = MockKeyring::with_password(&STANDARD.encode([7u8; 32])); + let _ = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert!(key_file.exists(), "file must NOT be deleted"); + } + + #[test] + fn keyring_backend_no_entry_reads_file() { + let dir = tempfile::tempdir().unwrap(); + let (expected, key_file) = write_test_key(dir.path()); + let mock = MockKeyring::no_entry(); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, expected); + assert!(key_file.exists(), "file must NOT be deleted"); + assert!( + mock.last_set.borrow().is_some(), + "should copy key to keyring" + ); + } + + #[test] + fn keyring_backend_no_entry_no_file_generates_and_saves_both() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let mock = MockKeyring::no_entry(); + let key = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(key.len(), 32); + assert!(key_file.exists(), "file must be created as fallback"); + assert!(mock.last_set.borrow().is_some(), "should store in keyring"); + let file_key = read_key_file(&key_file).unwrap(); + assert_eq!(key, file_key); + } + + #[test] + fn keyring_backend_no_entry_no_file_keyring_set_fails() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let mock = MockKeyring::no_entry().with_set_failure(); + let key = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(key.len(), 32); + assert!(key_file.exists(), "file must be created when keyring fails"); + let file_key = read_key_file(&key_file).unwrap(); + assert_eq!(key, file_key); + } + + #[test] + fn keyring_backend_platform_error_falls_back_to_file() { + let dir = tempfile::tempdir().unwrap(); + let (expected, key_file) = write_test_key(dir.path()); + let mock = MockKeyring::platform_error(); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, expected); + } + + #[test] + fn keyring_backend_platform_error_no_file_generates() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let mock = MockKeyring::platform_error(); + let key = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(key.len(), 32); + assert!(key_file.exists()); + } + + #[test] + fn keyring_backend_invalid_keyring_data_uses_file() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let (expected, key_file) = write_test_key(dir.path()); + let mock = MockKeyring::with_password(&STANDARD.encode([1u8; 16])); // wrong length + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(result, expected); + } + + // ---- Backend::File tests ---- + + #[test] + fn file_backend_reads_existing_key() { + let dir = tempfile::tempdir().unwrap(); + let (expected, key_file) = write_test_key(dir.path()); + let mock = MockKeyring::with_password("should-not-be-used"); + let result = resolve_key(KeyringBackend::File, &mock, &key_file).unwrap(); + assert_eq!(result, expected, "file backend should ignore keyring"); + } + + #[test] + fn file_backend_generates_when_missing() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let mock = MockKeyring::no_entry(); + let key = resolve_key(KeyringBackend::File, &mock, &key_file).unwrap(); + assert_eq!(key.len(), 32); + assert!(key_file.exists()); + assert!( + mock.last_set.borrow().is_none(), + "file backend must not touch keyring" + ); + } + + #[test] + fn file_backend_skips_keyring_entirely() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let (file_key, key_file) = write_test_key(dir.path()); + // Keyring has a DIFFERENT key — file backend should ignore it. + let mock = MockKeyring::with_password(&STANDARD.encode([99u8; 32])); + let result = resolve_key(KeyringBackend::File, &mock, &key_file).unwrap(); + assert_eq!(result, file_key, "must return the file key, not keyring"); + } + + // ---- Stability tests ---- + + #[test] + fn key_is_stable_across_calls() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + let mock = MockKeyring::platform_error(); + let key1 = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + let key2 = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + assert_eq!(key1, key2); + } + + // ---- KeyringBackend::from_env tests ---- + + #[test] + fn backend_default_is_keyring() { + // from_env reads the env; default (empty/unset) → Keyring + assert_eq!(KeyringBackend::from_env(), KeyringBackend::Keyring); + } + + // ---- read_key_file tests ---- + + #[test] + fn read_key_file_valid() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + let key = [99u8; 32]; + std::fs::write(&path, STANDARD.encode(key)).unwrap(); + assert_eq!(read_key_file(&path), Some(key)); + } + + #[test] + fn read_key_file_missing() { + let dir = tempfile::tempdir().unwrap(); + assert_eq!(read_key_file(&dir.path().join("nonexistent")), None); + } + + #[test] + fn read_key_file_wrong_length() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + std::fs::write(&path, STANDARD.encode([1u8; 16])).unwrap(); + assert_eq!(read_key_file(&path), None); + } + + #[test] + fn read_key_file_invalid_base64() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("key"); + std::fs::write(&path, "not-valid-base64!!!").unwrap(); + assert_eq!(read_key_file(&path), None); + } + + // ---- Existing encrypt/decrypt tests ---- #[test] fn get_or_create_key_is_deterministic() { @@ -328,4 +742,126 @@ mod tests { assert_eq!(dec1, dec2); assert_eq!(dec1, plaintext); } + + // ---- save_key_file_exclusive tests ---- + + #[test] + fn save_key_file_exclusive_creates_new_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(".encryption_key"); + save_key_file_exclusive(&path, "dGVzdA==").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "dGVzdA=="); + } + + #[test] + fn save_key_file_exclusive_rejects_existing_file() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(".encryption_key"); + std::fs::write(&path, "existing").unwrap(); + let err = save_key_file_exclusive(&path, "new").unwrap_err(); + assert_eq!(err.kind(), std::io::ErrorKind::AlreadyExists); + // Original content is untouched. + assert_eq!(std::fs::read_to_string(&path).unwrap(), "existing"); + } + + // ---- save_key_file tests ---- + + #[test] + fn save_key_file_overwrites_existing() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(".encryption_key"); + std::fs::write(&path, "old").unwrap(); + save_key_file(&path, "new").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "new"); + } + + // ---- ensure_key_dir tests ---- + + #[test] + fn ensure_key_dir_creates_nested_dirs() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a").join("b").join("c").join("key"); + ensure_key_dir(&path).unwrap(); + assert!(path.parent().unwrap().is_dir()); + } + + // ---- KeyringBackend::from_env tests ---- + + #[test] + fn backend_from_env_file_lowercase() { + // We can't easily set env vars in parallel tests, but we can test + // the parsing logic directly via the match arm. + assert_eq!( + match "file" { + "file" => KeyringBackend::File, + _ => KeyringBackend::Keyring, + }, + KeyringBackend::File + ); + } + + #[test] + fn backend_from_env_file_uppercase() { + // to_lowercase() should handle "FILE" → "file" + assert_eq!( + match "FILE".to_lowercase().as_str() { + "file" => KeyringBackend::File, + _ => KeyringBackend::Keyring, + }, + KeyringBackend::File + ); + } + + #[test] + fn backend_from_env_invalid_defaults_to_keyring() { + assert_eq!( + match "foobar".to_lowercase().as_str() { + "file" => KeyringBackend::File, + _ => KeyringBackend::Keyring, + }, + KeyringBackend::Keyring + ); + } + + // ---- Race condition tests ---- + + #[test] + fn race_loser_syncs_winner_key_to_keyring() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + + // Simulate: file was created by another process between our generate + // and our save_key_file_exclusive call. We pre-create the file so + // save_key_file_exclusive will fail with AlreadyExists. + let winner_key = [77u8; 32]; + std::fs::write(&key_file, STANDARD.encode(winner_key)).unwrap(); + + // Use NoEntry so resolve_key goes into the generate path. + let mock = MockKeyring::no_entry(); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + + // Should return the winner's key, not the one we generated. + assert_eq!(result, winner_key); + // The keyring should have been synced with the winner's key. + let synced = mock.last_set.borrow().clone().unwrap(); + assert_eq!(STANDARD.decode(&synced).unwrap(), winner_key); + } + + #[test] + fn race_loser_corrupt_file_overwrites() { + let dir = tempfile::tempdir().unwrap(); + let key_file = dir.path().join(".encryption_key"); + + // Pre-create a corrupt file (not valid base64 for a 32-byte key). + std::fs::write(&key_file, "corrupt-data").unwrap(); + + let mock = MockKeyring::no_entry(); + let result = resolve_key(KeyringBackend::Keyring, &mock, &key_file).unwrap(); + + // Should generate a new key and overwrite the corrupt file. + assert_eq!(result.len(), 32); + let file_key = read_key_file(&key_file).unwrap(); + assert_eq!(result, file_key, "file should be overwritten with new key"); + } } diff --git a/src/main.rs b/src/main.rs index fb28ffcf..22259a44 100644 --- a/src/main.rs +++ b/src/main.rs @@ -449,6 +449,9 @@ fn print_usage() { println!( " GOOGLE_WORKSPACE_CLI_CONFIG_DIR Override config directory (default: ~/.config/gws)" ); + println!( + " GOOGLE_WORKSPACE_CLI_KEYRING_BACKEND Keyring backend: keyring (default) or file" + ); println!(" GOOGLE_WORKSPACE_CLI_SANITIZE_TEMPLATE Default Model Armor template"); println!( " GOOGLE_WORKSPACE_CLI_SANITIZE_MODE Sanitization mode: warn (default) or block" diff --git a/src/token_storage.rs b/src/token_storage.rs index d6c5c47c..aa1726c7 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -19,7 +19,7 @@ use tokio::sync::Mutex; use yup_oauth2::storage::{TokenInfo, TokenStorage, TokenStorageError}; /// A custom token storage implementation for `yup-oauth2` that encrypts -/// the cached tokens at rest using the AES key derived from the OS keyring. +/// the cached tokens at rest using AES-256-GCM encryption. pub struct EncryptedTokenStorage { file_path: PathBuf, // Add memory cache since TokenStorage getters can be called frequently