diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c00de5..45d7572b 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Ceiling] Unreleased ### Fixed +- **Signing out of StepFun no longer leaves a live Oasis token in the keyring.** A successful refresh wrote the new token to the OS keyring (`codexbar-stepfun` / `api_key`), and Revoke stored credentials only cleared Preferences, cookies, and token-accounts. The next fetch then read the leftover and stayed signed in. Revoke now asks the provider to delete that copy. A keyring error fails the revoke instead of reporting success while the token remains. - **Reset countdowns agree at a remaining day and in the last minute.** The CLI statusline used `hours > 24`, so 24h 10m stayed "24h 10m" while the tray and the TypeScript hooks already said "1d 0h". The native tooltip and taskbar strip floored a still-future 30s remainder to "0m", the stuck-timer leftover SBS-621 fixed in the hooks. Every reset surface now floors one total of minutes, clamps a sub-minute remainder to 1, and cuts a day at 1440 minutes. A user looking at the tray, the CLI, and the tooltip at 24h 1s sees "1d 0h" (or "Resets in 1d" in the locale sentence); at 30s they see "1m", never "0m". - **A failed update check no longer tells you that you are current.** Checking for updates treated a GitHub outage, a rate-limit, or an unreadable release payload the same as "no newer release", so About said you were up to date. Only a successful "latest is not newer" is Idle now. Failures, including the existing 15s timeout, are Error, and About shows that the check could not run. A second check after a download is ready no longer clears Install & Restart. - **Charts opens in a couple of seconds instead of half a minute.** On a machine holding gigabytes of Codex and Claude transcripts, opening Charts started three separate walks of the same logs at once, and each one read every file from the top: Estimated API value scanned ninety days when the furthest period it shows reaches back sixty, the activity heatmap scanned thirty, and the provider charts scanned again on top. Nothing was kept, so switching tabs paid for all of it again, and clicking Yesterday or 30 days re-ran a full scan for numbers the card already had in hand. Each transcript is now parsed once and its records are kept in a small index beside your settings; a file that grew since is resumed from where the last read stopped rather than re-read from the start, and a file that was replaced rather than appended to is read again in full. Providers are scanned at the same time instead of one after another, both cards keep their last result on disk so a restart is not a cold start, and the work runs in the background shortly after launch for anyone who opens these cards, so the wait lands where nobody is watching it. The numbers are unchanged: an indexed scan is checked against a full re-parse, and the index is discarded outright if model prices move, since it stores the dollars they produced. diff --git a/rust/src/providers/mod.rs b/rust/src/providers/mod.rs index 295e4ef1..ef79bbfd 100755 --- a/rust/src/providers/mod.rs +++ b/rust/src/providers/mod.rs @@ -158,6 +158,18 @@ pub(crate) fn resolve_api_key( ))) } +/// Delete provider-owned copies of live tokens that survive the shared +/// credential files. `revoke_managed_credentials` calls this so settings does +/// not hardcode a vendor. Today only StepFun writes a refreshed token to the +/// OS keyring (SBS-920). Other providers only *read* keyring entries the user +/// or another app placed there; those are left alone. +pub(crate) fn clear_persisted_credentials(provider: crate::core::ProviderId) -> anyhow::Result<()> { + match provider { + crate::core::ProviderId::StepFun => stepfun::clear_persisted_credentials(), + _ => Ok(()), + } +} + /// Whether a parsed URL points at the local machine. /// /// Host-based, never prefix-based: `http://localhost@evil.example` and @@ -229,4 +241,10 @@ mod browser_cookie_policy_tests { Err(crate::core::ProviderError::NoCookies) )); } + + #[test] + fn clear_persisted_credentials_is_noop_for_providers_without_a_shadow_store() { + assert!(clear_persisted_credentials(crate::core::ProviderId::Claude).is_ok()); + assert!(clear_persisted_credentials(crate::core::ProviderId::Codex).is_ok()); + } } diff --git a/rust/src/providers/stepfun/mod.rs b/rust/src/providers/stepfun/mod.rs index 2262ff31..2a668b4f 100644 --- a/rust/src/providers/stepfun/mod.rs +++ b/rust/src/providers/stepfun/mod.rs @@ -162,11 +162,7 @@ impl StepFunProvider { } fn persist_refreshed_token(&self, token: &str) { - if let Ok(entry) = keyring::Entry::new(STEPFUN_CREDENTIAL_TARGET, "api_key") - && let Err(error) = entry.set_password(token) - { - tracing::debug!("Could not persist refreshed StepFun token: {error}"); - } + persist_refreshed_token_in(&OsTokenSecretStore, token); } async fn post_json Deserialize<'de>>( @@ -403,21 +399,155 @@ impl Provider for StepFunProvider { } } +/// Write the refreshed Oasis token to the keyring, unless the credential it +/// belongs to was revoked while the refresh was in flight. +/// +/// A refresh only runs after an auth failure, which is also the moment someone +/// is most likely to be signing out. Revoke takes the state write lock, deletes +/// the keyring copy, confirms it is gone, and reports success; an unguarded +/// write landing after that put a live token back and the session stayed signed +/// in (SBS-920). Taking the same lock and re-reading Preferences under it is +/// what makes the two orders decide, rather than race. +fn persist_refreshed_token_in(store: &impl TokenSecretStore, token: &str) { + persist_refreshed_token_checked(store, token, stepfun_credential_configured) +} + +/// Persist under the state lock, asking `configured` while holding it. +/// +/// The predicate is a parameter so a test can drive the real locked path +/// rather than only the decision it reaches. +fn persist_refreshed_token_checked( + store: &impl TokenSecretStore, + token: &str, + configured: impl FnOnce() -> bool, +) { + let locked = crate::secure_file::with_state_write_lock(|| { + Ok(persist_refreshed_token_when(store, token, configured())) + }); + if let Err(error) = locked { + tracing::debug!( + "Could not take the state lock to persist refreshed StepFun token: {error}" + ); + } +} + +/// Whether StepFun still has a credential this refreshed token could belong to. +/// +/// Preferences is what revoke clears. An environment variable is not Ceiling's +/// to remove and authenticates on its own, so a machine configured that way +/// keeps refreshing normally. +fn stepfun_credential_configured() -> bool { + if crate::settings::provider_credential_present(crate::core::ProviderId::StepFun) { + return true; + } + ["STEPFUN_OASIS_TOKEN", "STEPFUN_TOKEN"] + .iter() + .any(|name| std::env::var(name).is_ok_and(|value| !value.trim().is_empty())) +} + +/// The decision itself, separated from the lock and the disk read so a test can +/// state the revoked case directly. +fn persist_refreshed_token_when( + store: &impl TokenSecretStore, + token: &str, + still_configured: bool, +) -> bool { + if !still_configured { + tracing::debug!("StepFun credential was revoked mid-refresh; refreshed token not stored"); + return false; + } + if let Err(error) = store.set(STEPFUN_CREDENTIAL_TARGET, "api_key", token) { + tracing::debug!("Could not persist refreshed StepFun token: {error}"); + return false; + } + true +} + +/// Delete the refreshed Oasis token Ceiling wrote to the OS keyring. +/// +/// `revoke_managed_credentials` clears Preferences / cookies / token-accounts +/// only. StepFun's live refresh path also writes `codexbar-stepfun` / `api_key`, +/// and `resolve_token` reads that copy after the Preferences key is gone +/// (SBS-920). Missing is success; any other keyring error fails closed so +/// revoke cannot report success while the token remains. +pub(crate) fn clear_persisted_credentials() -> anyhow::Result<()> { + clear_token_secret(&OsTokenSecretStore) +} + +fn clear_token_secret(store: &impl TokenSecretStore) -> anyhow::Result<()> { + store + .delete(STEPFUN_CREDENTIAL_TARGET, "api_key") + .map_err(|error| anyhow::anyhow!("Could not delete StepFun keyring token: {error}"))?; + match store.get(STEPFUN_CREDENTIAL_TARGET, "api_key") { + Ok(None) => Ok(()), + Ok(Some(_)) => Err(anyhow::anyhow!( + "StepFun keyring token was still present after delete" + )), + Err(error) => Err(anyhow::anyhow!( + "Could not confirm StepFun keyring token was deleted: {error}" + )), + } +} + +trait TokenSecretStore { + fn get(&self, service: &str, user: &str) -> Result, String>; + fn set(&self, service: &str, user: &str, value: &str) -> Result<(), String>; + fn delete(&self, service: &str, user: &str) -> Result<(), String>; +} + +struct OsTokenSecretStore; + +impl TokenSecretStore for OsTokenSecretStore { + fn get(&self, service: &str, user: &str) -> Result, String> { + let entry = keyring::Entry::new(service, user).map_err(|error| error.to_string())?; + match entry.get_password() { + Ok(value) if !value.trim().is_empty() => Ok(Some(value)), + Ok(_) => Ok(None), + Err(keyring::Error::NoEntry) => Ok(None), + Err(error) => Err(error.to_string()), + } + } + + fn set(&self, service: &str, user: &str, value: &str) -> Result<(), String> { + let entry = keyring::Entry::new(service, user).map_err(|error| error.to_string())?; + entry.set_password(value).map_err(|error| error.to_string()) + } + + fn delete(&self, service: &str, user: &str) -> Result<(), String> { + let entry = keyring::Entry::new(service, user).map_err(|error| error.to_string())?; + match entry.delete_credential() { + Ok(()) => Ok(()), + Err(keyring::Error::NoEntry) => Ok(()), + Err(error) => Err(error.to_string()), + } + } +} + fn resolve_token( explicit: Option<&str>, credential_target: &str, env_names: &[&str], +) -> Result { + resolve_token_in(&OsTokenSecretStore, explicit, credential_target, env_names) +} + +fn resolve_token_in( + store: &impl TokenSecretStore, + explicit: Option<&str>, + credential_target: &str, + env_names: &[&str], ) -> Result { if let Some(key) = explicit && !key.trim().is_empty() { return Ok(key.trim().to_string()); } - if let Ok(entry) = keyring::Entry::new(credential_target, "api_key") - && let Ok(key) = entry.get_password() - && !key.trim().is_empty() - { - return Ok(key); + match store.get(credential_target, "api_key") { + Ok(Some(key)) if !key.trim().is_empty() => return Ok(key), + // Empty, missing, or unreadable: fall through to env, matching the + // previous resolver. Revoke does not use this path — + // `clear_token_secret` fails closed on the same unknown. + Ok(Some(_)) | Ok(None) | Err(_) => {} } for env in env_names { if let Ok(key) = std::env::var(env) @@ -435,6 +565,91 @@ fn resolve_token( #[cfg(test)] mod tests { use super::*; + use std::collections::HashMap; + use std::sync::Mutex; + + struct MemoryTokenSecretStore { + inner: Mutex>, + } + + impl MemoryTokenSecretStore { + fn new() -> Self { + Self { + inner: Mutex::new(HashMap::new()), + } + } + } + + impl TokenSecretStore for MemoryTokenSecretStore { + fn get(&self, service: &str, user: &str) -> Result, String> { + Ok(self + .inner + .lock() + .expect("memory token store lock") + .get(&(service.to_string(), user.to_string())) + .cloned()) + } + + fn set(&self, service: &str, user: &str, value: &str) -> Result<(), String> { + self.inner + .lock() + .expect("memory token store lock") + .insert((service.to_string(), user.to_string()), value.to_string()); + Ok(()) + } + + fn delete(&self, service: &str, user: &str) -> Result<(), String> { + self.inner + .lock() + .expect("memory token store lock") + .remove(&(service.to_string(), user.to_string())); + Ok(()) + } + } + + struct FailingDeleteStore; + + impl TokenSecretStore for FailingDeleteStore { + fn get(&self, _service: &str, _user: &str) -> Result, String> { + Ok(Some("leftover-oasis-token".to_string())) + } + + fn set(&self, _service: &str, _user: &str, _value: &str) -> Result<(), String> { + Ok(()) + } + + fn delete(&self, _service: &str, _user: &str) -> Result<(), String> { + Err("keyring locked".to_string()) + } + } + + /// Delete reports success but leaves the secret in place — the revoke + /// fail-open that SBS-920 is. Confirmation must reject this. + struct LyingDeleteStore { + inner: MemoryTokenSecretStore, + } + + impl LyingDeleteStore { + fn new() -> Self { + Self { + inner: MemoryTokenSecretStore::new(), + } + } + } + + impl TokenSecretStore for LyingDeleteStore { + fn get(&self, service: &str, user: &str) -> Result, String> { + self.inner.get(service, user) + } + + fn set(&self, service: &str, user: &str, value: &str) -> Result<(), String> { + self.inner.set(service, user, value) + } + + fn delete(&self, _service: &str, _user: &str) -> Result<(), String> { + Ok(()) + } + } #[test] fn stepfun_snapshot_converts_left_rates_to_used_percent() { @@ -470,4 +685,137 @@ mod tests { assert!(is_authentication_message("HTTP 401")); assert!(!is_authentication_message("rate limit")); } + + /// SBS-920: after revoke, `resolve_token` must not revive the session from + /// the leftover keyring copy `persist_refreshed_token` wrote. + #[test] + fn revoke_clears_refreshed_keyring_token_so_resolve_cannot_revive_session() { + let store = MemoryTokenSecretStore::new(); + persist_refreshed_token_when(&store, "access...refresh", true); + assert_eq!( + resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]).unwrap(), + "access...refresh" + ); + + clear_token_secret(&store).expect("revoke must delete the leftover token"); + + let error = resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]) + .expect_err("leftover keyring token must not authenticate after revoke"); + assert!( + matches!(error, ProviderError::NotInstalled(_)), + "expected NotInstalled after revoke, got {error:?}" + ); + } + + /// SBS-920: a refresh that lands after revoke must not put the session + /// back. Revoke deletes the keyring copy, confirms it, and reports + /// success; the write that follows has to see a revoked credential and + /// leave the store empty. + #[test] + fn a_refresh_landing_after_revoke_does_not_restore_the_token() { + let store = MemoryTokenSecretStore::new(); + persist_refreshed_token_when(&store, "access...refresh", true); + clear_token_secret(&store).expect("revoke must delete the leftover token"); + + let persisted = persist_refreshed_token_when(&store, "refreshed-after-revoke", false); + + assert!(!persisted, "a revoked credential must not be written back"); + let error = resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]) + .expect_err("a refresh after revoke must not authenticate"); + assert!( + matches!(error, ProviderError::NotInstalled(_)), + "expected NotInstalled after revoke, got {error:?}" + ); + } + + /// The locked path itself, not just the decision it reaches. + /// + /// Without this, dropping the lock or hard-coding the check to true would + /// leave every other test here green while the race this closes came back. + #[test] + fn the_locked_persist_path_writes_nothing_for_a_revoked_credential() { + let store = MemoryTokenSecretStore::new(); + + persist_refreshed_token_checked(&store, "refreshed-after-revoke", || false); + + let error = resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]) + .expect_err("a revoked credential must leave the keyring empty"); + assert!(matches!(error, ProviderError::NotInstalled(_))); + } + + #[test] + fn the_locked_persist_path_writes_for_a_live_credential() { + let store = MemoryTokenSecretStore::new(); + + persist_refreshed_token_checked(&store, "fresh-token", || true); + + assert_eq!( + resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]).unwrap(), + "fresh-token" + ); + } + + #[test] + fn a_refresh_for_a_live_credential_is_still_persisted() { + let store = MemoryTokenSecretStore::new(); + + assert!(persist_refreshed_token_when(&store, "fresh-token", true)); + + assert_eq!( + resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]).unwrap(), + "fresh-token" + ); + } + #[test] + fn clear_persisted_credentials_is_idempotent_when_keyring_has_no_entry() { + let store = MemoryTokenSecretStore::new(); + clear_token_secret(&store).expect("missing keyring entry is already revoked"); + let error = resolve_token_in(&store, None, STEPFUN_CREDENTIAL_TARGET, &[]) + .expect_err("empty store must not resolve a token"); + assert!(matches!(error, ProviderError::NotInstalled(_))); + } + + #[test] + fn clear_persisted_credentials_fails_closed_when_keyring_delete_errors() { + let error = + clear_token_secret(&FailingDeleteStore).expect_err("a locked keyring must fail revoke"); + assert!( + error + .to_string() + .contains("Could not delete StepFun keyring token"), + "got {error}" + ); + assert_eq!( + resolve_token_in(&FailingDeleteStore, None, STEPFUN_CREDENTIAL_TARGET, &[]).unwrap(), + "leftover-oasis-token" + ); + } + + #[test] + fn clear_persisted_credentials_fails_closed_when_delete_lies() { + let store = LyingDeleteStore::new(); + persist_refreshed_token_when(&store, "still-here", true); + let error = clear_token_secret(&store) + .expect_err("reporting success while the token remains is fail-open"); + assert!( + error.to_string().contains("still present after delete"), + "got {error}" + ); + } + + #[test] + fn resolve_token_prefers_explicit_preferences_key_over_keyring() { + let store = MemoryTokenSecretStore::new(); + persist_refreshed_token_when(&store, "keyring-copy", true); + assert_eq!( + resolve_token_in( + &store, + Some(" preferences-copy "), + STEPFUN_CREDENTIAL_TARGET, + &[] + ) + .unwrap(), + "preferences-copy" + ); + } } diff --git a/rust/src/settings.rs b/rust/src/settings.rs index b51929ec..9f630712 100755 --- a/rust/src/settings.rs +++ b/rust/src/settings.rs @@ -59,6 +59,11 @@ pub use types::*; /// idempotent. An I/O failure between atomic file replacements can leave a /// partial revocation, but retrying safely completes it; it can never restore /// a credential or lose another provider's concurrent update. +/// +/// After the shared files are written, the provider-owned hook runs so a +/// vendor-specific shadow copy (StepFun's refreshed Oasis keyring token, +/// SBS-920) cannot outlive Sign out. A hook error fails the revoke; retrying +/// is safe because the file removals and the hook are both idempotent. pub fn revoke_managed_credentials(provider: ProviderId) -> anyhow::Result<()> { crate::secure_file::with_state_write_lock(|| { let keys_path = ApiKeys::keys_path() @@ -67,26 +72,73 @@ pub fn revoke_managed_credentials(provider: ProviderId) -> anyhow::Result<()> { .ok_or_else(|| std::io::Error::other("Could not determine cookies path"))?; let token_store = crate::core::TokenAccountStore::new(); - let mut keys = ApiKeys::try_load_from(&keys_path).map_err(std::io::Error::other)?; - let mut cookies = - ManualCookies::try_load_from(&cookies_path).map_err(std::io::Error::other)?; - let mut token_accounts = token_store.load().map_err(std::io::Error::other)?; - - keys.remove(provider.cli_name()); - cookies.remove(provider.cli_name()); - token_accounts.remove(&provider); - - keys.save_to(&keys_path).map_err(std::io::Error::other)?; - cookies - .save_to(&cookies_path) - .map_err(std::io::Error::other)?; - token_store - .save_unlocked(&token_accounts) - .map_err(std::io::Error::other) + revoke_managed_credentials_in(provider, &keys_path, &cookies_path, &token_store, || { + crate::providers::clear_persisted_credentials(provider) + }) }) .map_err(Into::into) } +/// Whether `provider` still has a credential in Preferences. +/// +/// An unreadable store answers `true`. This exists for the background refresh +/// paths, which ask "was this revoked while I was working" before writing a +/// renewed token, and [`ApiKeys::load`] cannot tell an empty store from one +/// that failed to decode. Reading a decode failure as a revoke would drop a +/// renewed token and leave the session on a credential the provider may have +/// already rotated away from. +pub(crate) fn provider_credential_present(provider: ProviderId) -> bool { + match ApiKeys::try_load() { + Ok(keys) => keys.has_key(provider.cli_name()), + Err(error) => { + tracing::warn!("Could not read stored API keys ({error}); treating as still signed in"); + true + } + } +} + +/// The body of [`revoke_managed_credentials`], with the paths and the +/// provider-specific hook passed in so a test can fail the hook on purpose. +/// +/// The caller holds the state write lock. +fn revoke_managed_credentials_in( + provider: ProviderId, + keys_path: &std::path::Path, + cookies_path: &std::path::Path, + token_store: &crate::core::TokenAccountStore, + clear_persisted: impl FnOnce() -> anyhow::Result<()>, +) -> std::io::Result<()> { + let mut keys = ApiKeys::try_load_from(keys_path).map_err(std::io::Error::other)?; + let mut cookies = ManualCookies::try_load_from(cookies_path).map_err(std::io::Error::other)?; + let mut token_accounts = token_store.load().map_err(std::io::Error::other)?; + + keys.remove(provider.cli_name()); + cookies.remove(provider.cli_name()); + token_accounts.remove(&provider); + + // The keyring copy goes first, and the file stores only after it is + // confirmed gone. Both of the orders' failure modes are real: + // + // * A hook that errors after the files were already emptied returns + // Err with every file store reporting "no credential", which is + // exactly the state that hides the Revoke control. The user is left + // unable to retry a revoke that did not finish, while the leftover + // keyring token still authenticates. + // * A crash in the gap between the two leaves whichever half ran. With + // the keyring first that is a live-looking file store and no token, + // which fails closed; the other way round it is a signed-out UI over + // a token that still works. + clear_persisted().map_err(std::io::Error::other)?; + + keys.save_to(keys_path).map_err(std::io::Error::other)?; + cookies + .save_to(cookies_path) + .map_err(std::io::Error::other)?; + token_store + .save_unlocked(&token_accounts) + .map_err(std::io::Error::other) +} + #[cfg(test)] mod tests; diff --git a/rust/src/settings/api_keys.rs b/rust/src/settings/api_keys.rs index b1a3e9cc..eead90bc 100644 --- a/rust/src/settings/api_keys.rs +++ b/rust/src/settings/api_keys.rs @@ -88,7 +88,7 @@ impl ApiKeys { }) } - pub(super) fn try_load() -> anyhow::Result { + pub(crate) fn try_load() -> anyhow::Result { let Some(path) = Self::keys_path() else { return Ok(Self::default()); }; diff --git a/rust/src/settings/tests.rs b/rust/src/settings/tests.rs index f4c6ce4c..82d0cade 100644 --- a/rust/src/settings/tests.rs +++ b/rust/src/settings/tests.rs @@ -1548,3 +1548,71 @@ fn taskbar_account_map_is_sanitized_on_load() { ); assert_eq!(map.len(), 2, "blank provider keys must be dropped too"); } + +/// SBS-920: the keyring copy is cleared before the file stores, so a hook that +/// fails leaves the credential visible in Preferences. +/// +/// The other order emptied every file store first, and the Revoke control is +/// hidden exactly when they all report "no credential". A user whose revoke +/// failed halfway would be left with no way to retry it and a keyring token +/// that still authenticates. +#[test] +fn a_failing_revoke_hook_leaves_the_credential_in_preferences() { + let dir = tempfile::tempdir().expect("tempdir"); + let keys_path = dir.path().join("api_keys.json"); + let cookies_path = dir.path().join("cookies.json"); + let token_store = crate::core::TokenAccountStore::with_path(dir.path().join("tokens.json")); + + let mut keys = ApiKeys::default(); + keys.set(ProviderId::StepFun.cli_name(), "oasis-token", None); + keys.save_to(&keys_path).expect("seed api keys"); + + let error = revoke_managed_credentials_in( + ProviderId::StepFun, + &keys_path, + &cookies_path, + &token_store, + || Err(anyhow::anyhow!("keyring is locked")), + ) + .expect_err("a hook that cannot clear the keyring must fail the revoke"); + assert!( + error.to_string().contains("keyring is locked"), + "got {error}" + ); + + let after = ApiKeys::try_load_from(&keys_path).expect("reload api keys"); + assert!( + after.has_key(ProviderId::StepFun.cli_name()), + "the credential must survive so Revoke stays available to retry" + ); +} + +/// The successful path still clears every store it owns. +#[test] +fn a_successful_revoke_clears_preferences_after_the_hook() { + let dir = tempfile::tempdir().expect("tempdir"); + let keys_path = dir.path().join("api_keys.json"); + let cookies_path = dir.path().join("cookies.json"); + let token_store = crate::core::TokenAccountStore::with_path(dir.path().join("tokens.json")); + + let mut keys = ApiKeys::default(); + keys.set(ProviderId::StepFun.cli_name(), "oasis-token", None); + keys.save_to(&keys_path).expect("seed api keys"); + + let mut hook_ran = false; + revoke_managed_credentials_in( + ProviderId::StepFun, + &keys_path, + &cookies_path, + &token_store, + || { + hook_ran = true; + Ok(()) + }, + ) + .expect("revoke"); + + assert!(hook_ran, "the keyring hook must run"); + let after = ApiKeys::try_load_from(&keys_path).expect("reload api keys"); + assert!(!after.has_key(ProviderId::StepFun.cli_name())); +}