Skip to content

Commit cea7466

Browse files
fix(auth): stabilize encrypted credential key fallback (googleworkspace#28)
* fix(auth): stabilize encryption key fallback across runs * chore: add changeset for auth encryption key fix * chore: cargo fmt * 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.)
1 parent 9d72541 commit cea7466

File tree

2 files changed

+90
-14
lines changed

2 files changed

+90
-14
lines changed

.changeset/31852fab9121.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@googleworkspace/cli": patch
3+
---
4+
5+
fix(auth): stabilize encrypted credential key fallback across sessions
6+
7+
When the OS keyring returned `NoEntry`, the previous code could generate
8+
a fresh random key on each process invocation instead of reusing one.
9+
This caused `credentials.enc` written by `gws auth login` to be
10+
unreadable by subsequent commands.
11+
12+
Changes:
13+
- Always prefer an existing `.encryption_key` file before generating a new key
14+
- When generating a new key, persist it to `.encryption_key` as a stable fallback
15+
- Best-effort write new keys into the keyring as well
16+
- Fix `OnceLock` race: return the already-cached key if `set` loses a race
17+
18+
Fixes #27

src/credential_store.rs

Lines changed: 72 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,23 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
3030
return Ok(*key);
3131
}
3232

33+
let cache_key = |candidate: [u8; 32]| -> [u8; 32] {
34+
if KEY.set(candidate).is_ok() {
35+
candidate
36+
} else {
37+
// If set() fails, another thread already initialized the key. .get() is
38+
// guaranteed to return Some at this point.
39+
*KEY.get()
40+
.expect("key must be initialized if OnceLock::set() failed")
41+
}
42+
};
43+
3344
let username = std::env::var("USER")
3445
.or_else(|_| std::env::var("USERNAME"))
3546
.unwrap_or_else(|_| "unknown-user".to_string());
3647

48+
let key_file = crate::auth_commands::config_dir().join(".encryption_key");
49+
3750
let entry = Entry::new("gws-cli", &username);
3851

3952
if let Ok(entry) = entry {
@@ -44,39 +57,82 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
4457
if decoded.len() == 32 {
4558
let mut arr = [0u8; 32];
4659
arr.copy_from_slice(&decoded);
47-
let _ = KEY.set(arr);
48-
return Ok(arr);
60+
return Ok(cache_key(arr));
4961
}
5062
}
5163
}
5264
Err(keyring::Error::NoEntry) => {
53-
// Generate a random 32-byte key
65+
use base64::{engine::general_purpose::STANDARD, Engine as _};
66+
67+
// If keyring is empty, prefer a persisted local key first.
68+
if key_file.exists() {
69+
if let Ok(b64_key) = std::fs::read_to_string(&key_file) {
70+
if let Ok(decoded) = STANDARD.decode(b64_key.trim()) {
71+
if decoded.len() == 32 {
72+
let mut arr = [0u8; 32];
73+
arr.copy_from_slice(&decoded);
74+
// Best effort: repopulate keyring for future runs.
75+
let _ = entry.set_password(&b64_key);
76+
return Ok(cache_key(arr));
77+
}
78+
}
79+
}
80+
}
81+
82+
// Generate a random 32-byte key and persist it locally as a stable fallback.
5483
let mut key = [0u8; 32];
5584
rand::thread_rng().fill_bytes(&mut key);
56-
57-
use base64::{engine::general_purpose::STANDARD, Engine as _};
5885
let b64_key = STANDARD.encode(key);
5986

60-
if entry.set_password(&b64_key).is_ok() {
61-
let _ = KEY.set(key);
62-
return Ok(key);
87+
if let Some(parent) = key_file.parent() {
88+
let _ = std::fs::create_dir_all(parent);
89+
#[cfg(unix)]
90+
{
91+
use std::os::unix::fs::PermissionsExt;
92+
if let Err(e) =
93+
std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700))
94+
{
95+
eprintln!(
96+
"Warning: failed to set secure permissions on key directory: {e}"
97+
);
98+
}
99+
}
100+
}
101+
102+
#[cfg(unix)]
103+
{
104+
use std::os::unix::fs::OpenOptionsExt;
105+
let mut options = std::fs::OpenOptions::new();
106+
options.write(true).create(true).truncate(true).mode(0o600);
107+
if let Ok(mut file) = options.open(&key_file) {
108+
use std::io::Write;
109+
let _ = file.write_all(b64_key.as_bytes());
110+
}
111+
}
112+
#[cfg(not(unix))]
113+
{
114+
let _ = std::fs::write(&key_file, &b64_key);
63115
}
116+
117+
// Best effort: also store in keyring when available.
118+
let _ = entry.set_password(&b64_key);
119+
120+
return Ok(cache_key(key));
64121
}
65122
Err(_) => {} // Fallthrough to file storage
66123
}
67124
}
68125

69126
// Fallback: Local file `.encryption_key`
70-
let key_file = crate::auth_commands::config_dir().join(".encryption_key");
127+
71128
if key_file.exists() {
72129
if let Ok(b64_key) = std::fs::read_to_string(&key_file) {
73130
use base64::{engine::general_purpose::STANDARD, Engine as _};
74131
if let Ok(decoded) = STANDARD.decode(b64_key.trim()) {
75132
if decoded.len() == 32 {
76133
let mut arr = [0u8; 32];
77134
arr.copy_from_slice(&decoded);
78-
let _ = KEY.set(arr);
79-
return Ok(arr);
135+
return Ok(cache_key(arr));
80136
}
81137
}
82138
}
@@ -94,7 +150,10 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
94150
#[cfg(unix)]
95151
{
96152
use std::os::unix::fs::PermissionsExt;
97-
let _ = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700));
153+
if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700))
154+
{
155+
eprintln!("Warning: failed to set secure permissions on key directory: {e}");
156+
}
98157
}
99158
}
100159

@@ -113,8 +172,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> {
113172
let _ = std::fs::write(&key_file, b64_key);
114173
}
115174

116-
let _ = KEY.set(key);
117-
Ok(key)
175+
Ok(cache_key(key))
118176
}
119177

120178
/// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key.

0 commit comments

Comments
 (0)