fix: delete every cached item of a profile, including zsh-op leftovers

README.md

@@ -218,6 +218,7 @@ keysafe picks up where the zsh-op plugin left off:
 - With no config at the new location, `~/.config/op/config.yml` is used, with a hint to move it.
 - Profiles recorded in `~/.cache/op` count as loaded until you load them again.
 - Secrets cached under `op-secrets-<profile>` move to `keysafe.<profile>` the first time they are read; the old items are deleted.
+- Items zsh-op cached that keysafe never reads, such as secrets no longer in your config, stay behind. `keysafe doctor` warns about them and `keysafe profile clear <profile>` deletes them.
 
 ## Troubleshooting
 

src/app/exec.rs

@@ -1012,7 +1012,25 @@ impl DoctorCommand {
 
         // The keychain: looking up a missing item needs the store, but never prompts
         match self.cache.store.get("keysafe.doctor", "check") {
-            Ok(_) => self.report(Check::Ok, "Keychain", "reachable")?,
+            Ok(_) => {
+                self.report(Check::Ok, "Keychain", "reachable")?;
+
+                // Secrets zsh-op cached under its own service names
+                for profile in profiles {
+                    let Ok(items) = self.cache.legacy_items(&profile.name) else {
+                        continue;
+                    };
+                    if !items.is_empty() {
+                        let detail = format!(
+                            "{} left from zsh-op under {} (remove them with `keysafe profile clear {}`)",
+                            count(items.len(), "item"),
+                            Cache::legacy_service(&profile.name),
+                            profile.name
+                        );
+                        self.report(Check::Warning, "Keychain", detail)?;
+                    }
+                }
+            }
             Err(err) => {
                 failures += 1;
                 self.report(Check::Failure, "Keychain", format!("{err:#}"))?;
@@ -2572,6 +2590,25 @@ mod tests {
         assert!(output.ends_with("✓ Shell integration: active (bash)\n"));
     }
 
+    #[test]
+    fn doctor_warns_about_items_left_from_zsh_op() -> Result<()> {
+        let fixture = Fixture::new();
+        fixture.store.set("op-secrets-work", "OLD_TOKEN", "x")?;
+        let mut client = MockSecretClient::new();
+        client.expect_check().returning(|_| Ok("fine".into()));
+        let (mut cmd, writer) = doctor(&fixture, client, agent(vec![]));
+
+        cmd.execute(&DoctorCommandArgs {
+            parent: fixture.parent(),
+            shell: Some(Shell::Zsh),
+        })?;
+
+        assert!(writer.contents().contains(
+            "! Keychain: 1 item left from zsh-op under op-secrets-work (remove them with `keysafe profile clear work`)\n"
+        ));
+        Ok(())
+    }
+
     #[test]
     fn doctor_skips_the_agent_without_ssh_keys() -> Result<()> {
         let fixture = Fixture::new();

src/vault/cache.rs

@@ -1,5 +1,6 @@
 use anyhow::{anyhow, Context, Result};
 use std::{
+    collections::HashMap,
     io::ErrorKind,
     path::{Path, PathBuf},
     sync::OnceLock,
@@ -16,6 +17,8 @@ pub trait SecretStore {
     fn set(&self, service: &str, account: &str, value: &str) -> Result<()>;
     /// Deletes the value stored for `service` / `account`. Returns false if there was none.
     fn delete(&self, service: &str, account: &str) -> Result<bool>;
+    /// Returns the accounts that have a value stored for `service`, without reading values.
+    fn accounts(&self, service: &str) -> Result<Vec<String>>;
 }
 
 /// Keychain stores secrets in the platform credential store: the login keychain on macOS
@@ -29,14 +32,19 @@ impl Keychain {
         Self
     }
 
-    /// Returns the keyring entry for `service` / `account`.
-    fn entry(service: &str, account: &str) -> Result<keyring_core::Entry> {
+    /// Opens the platform credential store, once.
+    fn open() -> Result<()> {
         static STORE: OnceLock<Result<(), String>> = OnceLock::new();
         STORE
             .get_or_init(|| open_store().map_err(|e| e.to_string()))
             .as_ref()
-            .map_err(|e| anyhow!("failed to open the credential store: {e}"))?;
+            .map(|_| ())
+            .map_err(|e| anyhow!("failed to open the credential store: {e}"))
+    }
 
+    /// Returns the keyring entry for `service` / `account`.
+    fn entry(service: &str, account: &str) -> Result<keyring_core::Entry> {
+        Self::open()?;
         Ok(keyring_core::Entry::new(service, account)?)
     }
 }
@@ -59,6 +67,28 @@ fn open_store() -> keyring_core::Result<()> {
 }
 
 impl SecretStore for Keychain {
+    fn accounts(&self, service: &str) -> Result<Vec<String>> {
+        Self::open()?;
+        // A search returns the items' attributes only, so it never prompts for access
+        let spec = HashMap::from([("service", service)]);
+        let entries = keyring_core::Entry::search(&spec)
+            .with_context(|| format!("failed to search the keychain for {service}"))?;
+        let mut accounts: Vec<String> = entries
+            .iter()
+            .filter_map(|entry| match entry.get_specifiers() {
+                Some((_, account)) => Some(account),
+                None => entry.get_attributes().ok().and_then(|attributes| {
+                    ["username", "user", "acct"]
+                        .iter()
+                        .find_map(|key| attributes.get(*key).cloned())
+                }),
+            })
+            .collect();
+        accounts.sort();
+        accounts.dedup();
+        Ok(accounts)
+    }
+
     fn get(&self, service: &str, account: &str) -> Result<Option<String>> {
         match Self::entry(service, account)?.get_password() {
             Ok(value) => Ok(Some(value)),
@@ -131,6 +161,11 @@ impl Cache {
         format!("op-secrets-{profile}")
     }
 
+    /// Returns the secrets of `profile` still stored under the service zsh-op used.
+    pub fn legacy_items(&self, profile: &str) -> Result<Vec<String>> {
+        self.store.accounts(&Self::legacy_service(profile))
+    }
+
     /// Returns the cached value of secret `name` in `profile`.
     pub fn get(&self, profile: &str, name: &str) -> Result<Option<String>> {
         let service = Self::service(profile);
@@ -216,17 +251,23 @@ impl Cache {
     /// its metadata or stored under the legacy service, and forgets that it was loaded.
     /// Returns the number of deleted secrets.
     pub fn clear(&self, account: &Profile) -> Result<usize> {
-        let mut names: Vec<String> = account.secrets.iter().map(|s| s.name.clone()).collect();
-        for name in self.loaded(&account.name)?.unwrap_or_default() {
-            if !names.contains(&name) {
-                names.push(name);
-            }
-        }
-
         let services = [
             Self::service(&account.name),
             Self::legacy_service(&account.name),
         ];
+
+        // Configured and recorded secrets, plus anything else stored under either service,
+        // like secrets since removed from the config
+        let mut names: Vec<String> = account.secrets.iter().map(|s| s.name.clone()).collect();
+        names.extend(self.loaded(&account.name)?.unwrap_or_default());
+        for service in &services {
+            match self.store.accounts(service) {
+                Ok(found) => names.extend(found),
+                Err(err) => debug(format!("{err:#}")),
+            }
+        }
+        names.sort();
+        names.dedup();
         let mut count = 0;
         for name in &names {
             let mut deleted = false;
@@ -361,6 +402,16 @@ impl SecretStore for MemoryStore {
         let key = (service.to_string(), account.to_string());
         Ok(self.0.borrow_mut().remove(&key).is_some())
     }
+
+    fn accounts(&self, service: &str) -> Result<Vec<String>> {
+        Ok(self
+            .0
+            .borrow()
+            .keys()
+            .filter(|(s, _)| s == service)
+            .map(|(_, account)| account.clone())
+            .collect())
+    }
 }
 
 #[cfg(test)]
@@ -462,6 +513,31 @@ mod tests {
         );
     }
 
+    #[test]
+    fn clear_deletes_items_the_config_no_longer_names() {
+        let dir = tempfile::tempdir().unwrap();
+        let store = MemoryStore::with(&[
+            ("op-secrets-personal", "REMOVED_LONG_AGO", "a"),
+            ("keysafe.personal", "ALSO_GONE", "b"),
+            ("op-secrets-work", "OTHER_PROFILE", "c"),
+        ]);
+        let cache = Cache::new(Box::new(store.clone()), dir.path());
+        assert_eq!(
+            cache.legacy_items("personal").unwrap(),
+            ["REMOVED_LONG_AGO"]
+        );
+
+        assert_eq!(cache.clear(&account()).unwrap(), 2);
+
+        assert_eq!(store.value("op-secrets-personal", "REMOVED_LONG_AGO"), None);
+        assert_eq!(store.value("keysafe.personal", "ALSO_GONE"), None);
+        assert_eq!(
+            store.value("op-secrets-work", "OTHER_PROFILE").as_deref(),
+            Some("c")
+        );
+        assert!(cache.legacy_items("personal").unwrap().is_empty());
+    }
+
     #[test]
     fn service_uses_profile_prefix() {
         assert_eq!(Cache::service("work"), "keysafe.work");