diff --git a/.changeset/fix-auth-keyring-backend.md b/.changeset/fix-auth-keyring-backend.md new file mode 100644 index 00000000..4b34bcd1 --- /dev/null +++ b/.changeset/fix-auth-keyring-backend.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Fix `gws auth login` encrypted credential persistence by enabling native keyring backends for the `keyring` crate on supported desktop platforms instead of silently falling back to the in-memory mock store. diff --git a/.github/workflows/automation.yml b/.github/workflows/automation.yml index ca199f4e..5eb3de95 100644 --- a/.github/workflows/automation.yml +++ b/.github/workflows/automation.yml @@ -24,6 +24,7 @@ on: permissions: contents: write + issues: write pull-requests: write jobs: @@ -125,9 +126,17 @@ jobs: return; } - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number, - labels: ['gemini: reviewed'], - }); + try { + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.payload.pull_request.number, + labels: ['gemini: reviewed'], + }); + } catch (e) { + if (e.status === 403) { + console.log(`Token cannot add labels for this review event (${e.message}) — skipping`); + return; + } + throw e; + } diff --git a/Cargo.lock b/Cargo.lock index 68931d42..6ac34932 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -203,6 +203,12 @@ version = "1.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8efb64bd706a16a1bdde310ae86b351e4d21550d98d056f22f8a7f7a2183fec" +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "bytes" version = "1.11.1" @@ -332,6 +338,16 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "core-foundation" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -1291,7 +1307,11 @@ version = "3.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eebcc3aff044e5944a8fbaf69eb277d11986064cba30c468730e8b9909fb551c" dependencies = [ + "byteorder", "log", + "security-framework 2.11.1", + "security-framework 3.7.0", + "windows-sys 0.60.2", "zeroize", ] @@ -2103,7 +2123,7 @@ dependencies = [ "openssl-probe", "rustls-pki-types", "schannel", - "security-framework", + "security-framework 3.7.0", ] [[package]] @@ -2175,6 +2195,19 @@ version = "4.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1c107b6f4780854c8b126e228ea8869f4d7b71260f962fefb57b996b8959ba6b" +[[package]] +name = "security-framework" +version = "2.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "897b2245f0b511c87893af39b033e5ca9cce68824c4d7e7630b5a1d339658d02" +dependencies = [ + "bitflags 2.11.0", + "core-foundation 0.9.4", + "core-foundation-sys", + "libc", + "security-framework-sys", +] + [[package]] name = "security-framework" version = "3.7.0" @@ -2182,7 +2215,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" dependencies = [ "bitflags 2.11.0", - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "libc", "security-framework-sys", diff --git a/Cargo.toml b/Cargo.toml index 44bf0235..e8b158a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,12 +52,20 @@ derive_builder = "0.20.2" ratatui = "0.30.0" crossterm = "0.29.0" chrono = "0.4.44" -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"] } +[target.'cfg(target_os = "macos")'.dependencies] +keyring = { version = "3.6.3", features = ["apple-native"] } + +[target.'cfg(target_os = "windows")'.dependencies] +keyring = { version = "3.6.3", features = ["windows-native"] } + +[target.'cfg(not(any(target_os = "macos", target_os = "windows")))'.dependencies] +keyring = "3.6.3" + # The profile that 'cargo dist' will build with [profile.dist] diff --git a/src/auth_commands.rs b/src/auth_commands.rs index ade681d6..65a26439 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -1556,6 +1556,7 @@ mod tests { } #[test] + #[serial_test::serial] fn config_dir_returns_gws_subdir() { let path = config_dir(); assert!(path.ends_with("gws")); diff --git a/src/credential_store.rs b/src/credential_store.rs index a0210fc2..43e76ff8 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -436,6 +436,7 @@ mod tests { get_state: MockState, set_succeeds: bool, last_set: RefCell>, + on_set: RefCell>>, } impl MockKeyring { @@ -444,6 +445,7 @@ mod tests { get_state: MockState::Ok(b64.to_string()), set_succeeds: true, last_set: RefCell::new(None), + on_set: RefCell::new(None), } } @@ -452,6 +454,7 @@ mod tests { get_state: MockState::NoEntry, set_succeeds: true, last_set: RefCell::new(None), + on_set: RefCell::new(None), } } @@ -460,6 +463,7 @@ mod tests { get_state: MockState::PlatformError, set_succeeds: true, last_set: RefCell::new(None), + on_set: RefCell::new(None), } } @@ -467,6 +471,14 @@ mod tests { self.set_succeeds = false; self } + + fn with_on_set(self, callback: F) -> Self + where + F: FnMut(&str) + 'static, + { + *self.on_set.borrow_mut() = Some(Box::new(callback)); + self + } } impl KeyringProvider for MockKeyring { @@ -482,6 +494,9 @@ mod tests { fn set_password(&self, password: &str) -> Result<(), keyring::Error> { *self.last_set.borrow_mut() = Some(password.to_string()); + if let Some(callback) = self.on_set.borrow_mut().as_mut() { + callback(password); + } if self.set_succeeds { Ok(()) } else { @@ -831,19 +846,19 @@ mod tests { 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(); + let winner_b64 = STANDARD.encode(winner_key); + let race_key_file = key_file.clone(); + let race_winner_b64 = winner_b64.clone(); - // Use NoEntry so resolve_key goes into the generate path. - let mock = MockKeyring::no_entry(); + let mock = MockKeyring::no_entry().with_on_set(move |_| { + if !race_key_file.exists() { + std::fs::write(&race_key_file, &race_winner_b64).unwrap(); + } + }); 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); }