fix(auth): enable native keyring backends on top of #359

.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.

.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;
+            }

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]

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"));

src/credential_store.rs

@@ -436,6 +436,7 @@ mod tests {
         get_state: MockState,
         set_succeeds: bool,
         last_set: RefCell<Option<String>>,
+        on_set: RefCell<Option<Box<dyn FnMut(&str)>>>,
     }
 
     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,13 +463,22 @@ mod tests {
                 get_state: MockState::PlatformError,
                 set_succeeds: true,
                 last_set: RefCell::new(None),
+                on_set: RefCell::new(None),
             }
         }
 
         fn with_set_failure(mut self) -> Self {
             self.set_succeeds = false;
             self
         }
+
+        fn with_on_set<F>(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);
     }