From bc918ff1adaffc61add213415d7293f35b8d1fc3 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 24 Aug 2026 00:52:24 +0800 Subject: [PATCH] fix(settings): say when deleting a profile cannot forget its secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `delete_profile_confirmed` releases the keychain entries a profile was the last thing pointing at — a password, and any passphrase for a key no surviving profile still lists. Both calls threw the result away. The store returns `Ok` when there was nothing there (`NoEntry` is mapped), so an `Err` is a real refusal: a locked keychain, or a user who dismissed the authorisation prompt. When that happened the profile was deleted anyway and the secret stayed behind — permanently, because the profile that reached it is exactly what just went away, and "Forget password" lives on the menu that no longer exists. That is the stranding the two comments in this function were written to fix; they fixed never asking, not being refused. Every other caller of these two already says when the store refuses — `ssh_prompt` logs both, and the standalone "Forget password" returns a message either way. This one now does too: it logs, and hands its caller what could not be finished, which goes to the window the prompt belonged to as a notification. `SettingsCouldntForgetPassphrase` is new and names the key by path; the existing password message already had a key. Placeholders match across all three locales, which the i18n guards check. --- src/ui/i18n/en.rs | 3 +++ src/ui/i18n/ja.rs | 3 +++ src/ui/i18n/mod.rs | 1 + src/ui/i18n/zh.rs | 1 + src/ui/settings.rs | 43 +++++++++++++++++++++++++++++++++++++++---- 5 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index e4d8552d..d7f64062 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -256,6 +256,9 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SettingsCouldntForgetPassword => { "Could not forget the saved password for {endpoint}: {error}" } + L10nKey::SettingsCouldntForgetPassphrase => { + "Could not forget the saved passphrase for {path}: {error}" + } L10nKey::SettingsSecurity => "Security", L10nKey::SettingsSecurityIntro => { "A host can override either of these under its own Advanced." diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 806037ec..0be983bc 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -261,6 +261,9 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::SettingsCouldntForgetPassword => { "{endpoint} のパスワードを消去できませんでした: {error}" } + L10nKey::SettingsCouldntForgetPassphrase => { + "{path} のパスフレーズを消去できませんでした: {error}" + } L10nKey::SettingsSecurity => "セキュリティ", L10nKey::SettingsSecurityIntro => "ホストは詳細設定でこれらを上書きできます", L10nKey::SettingsVerifyHostKeys => "ホストキーを検証", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 5fa68a3d..8e6fbbc2 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -261,6 +261,7 @@ l10n_keys! { SettingsDeleteProfileBody, SettingsDeleteProfileCascade, SettingsCouldntForgetPassword, + SettingsCouldntForgetPassphrase, SettingsSecurity, SettingsSecurityIntro, SettingsVerifyHostKeys, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 6f00c914..97a72275 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -233,6 +233,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { "有 {count} 个已保存的远程工作区条目指向 {endpoint},会一并从本机清除。远端机器上的会话照常跑——新建配置连上去就能在工作区列表里找回。" } L10nKey::SettingsCouldntForgetPassword => "无法清除 {endpoint} 的已保存密码:{error}", + L10nKey::SettingsCouldntForgetPassphrase => "无法清除 {path} 的已保存密码短语:{error}", L10nKey::SettingsSecurity => "安全", L10nKey::SettingsSecurityIntro => "主机可以在自己的高级选项中覆盖这些设置。", L10nKey::SettingsVerifyHostKeys => "校验主机密钥", diff --git a/src/ui/settings.rs b/src/ui/settings.rs index b4888538..561e719c 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -3976,12 +3976,30 @@ impl Tty7App { ); cx.spawn_in(window, async move |this, cx| { let Ok(0) = answer.await else { return }; - let _ = this.update(cx, |this, cx| this.delete_profile_confirmed(id, cx)); + // The profile is the last thing that pointed at these secrets, so + // a keychain that refuses to let one go strands it for good with no + // UI left anywhere to try again. Raised on the window the prompt + // belonged to, for the same reason forgetting a password alone is. + let _ = this.update_in(cx, |this, window, cx| { + for problem in this.delete_profile_confirmed(id, cx) { + window.push_notification(problem, cx); + } + }); }) .detach(); } - fn delete_profile_confirmed(&mut self, id: Uuid, cx: &mut Context) { + /// Deletes the profile, and returns whatever it could not finish. + /// + /// The keychain work below is best-effort in the sense that it must not + /// stop the delete — but not in the sense that it can go unsaid. Every + /// caller of `delete_password` and `delete_key_passphrase` elsewhere says + /// when the store refuses; this one used to throw the answer away, which + /// is the one place it matters most, because the profile that reached + /// those secrets is on its way out. + #[must_use] + fn delete_profile_confirmed(&mut self, id: Uuid, cx: &mut Context) -> Vec { + let mut problems = Vec::new(); // "Forget password" lives on the menu that is about to stop existing, // so deleting the profile used to strand its keychain entry with no UI // left to remove it. Only let go of the secret when nothing else on the @@ -3995,7 +4013,14 @@ impl Tty7App { let shared = profiles_sharing_endpoint(cfg, id) > 0; if let Some((user, host, port)) = endpoint.filter(|_| !shared) { use crate::core::keychain::{CredentialStore, OsCredentialStore}; - let _ = OsCredentialStore.delete_password(&user, &host, port); + if let Err(e) = OsCredentialStore.delete_password(&user, &host, port) { + let endpoint = format!("{user}@{host}:{port}"); + log::warn!("could not forget password for {endpoint} in keychain: {e}"); + problems.push(t_fmt( + L10nKey::SettingsCouldntForgetPassword, + &[("endpoint", &endpoint), ("error", &e.to_string())], + )); + } } // The same argument for the key passphrases this profile taught the // app about: the comment above says "the secret", but until now only @@ -4022,7 +4047,16 @@ impl Tty7App { continue; }; let account = crate::core::keychain::key_account_from_contents(&bytes); - let _ = OsCredentialStore.delete_key_passphrase(&account); + if let Err(e) = OsCredentialStore.delete_key_passphrase(&account) { + log::warn!("could not forget passphrase for {path} in keychain: {e}"); + problems.push(t_fmt( + L10nKey::SettingsCouldntForgetPassphrase, + &[ + ("path", &crate::terminal::view::one_line(path)), + ("error", &e.to_string()), + ], + )); + } } // Forget the entries that routed through this profile (#485) — @@ -4046,6 +4080,7 @@ impl Tty7App { s.ssh_detail = SshDetail::None; } cx.notify(); + problems } /// Import `~/.ssh/config`, and say what that did.