diff --git a/src/app.rs b/src/app.rs index b6f9372..f58875e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -22,8 +22,10 @@ pub struct App { pub settings: Settings, /// Derived vault key, present only while the encrypted vault is unlocked. unlock_key: Option, - /// Stack of deleted profiles for undo functionality. - pub undo_history: Vec, + /// Stack of deleted profiles for undo functionality. Holds the full + /// stored record (including secret key material, exactly as it was on + /// disk) so undo restores a working profile, not an empty shell. + pub undo_history: Vec, /// The most recent publish report, persisted across restarts. pub last_publish: Option, /// Active signer mode. @@ -60,7 +62,8 @@ pub struct AppStateView { pub active_profile: Option, pub profiles: Vec, pub settings: Settings, - /// Recently deleted profiles, newest last, for undo. + /// Recently deleted profiles (safe summaries only — never secret material), + /// newest last, for undo. #[serde(skip_serializing_if = "Vec::is_empty")] pub undo_history: Vec, /// The most recent publish report, persisted across restarts. @@ -215,36 +218,45 @@ impl App { Ok(revealed) } - /// Undo the last profile deletion, restoring the profile to the vault. + /// Undo the last profile deletion, restoring the profile — with its real + /// stored secret key — to the vault. /// Returns the restored profile summary, or an error if there is no undo history. pub fn undo_delete(&mut self) -> Result { - if self.undo_history.is_empty() { + let Some(deleted) = self.undo_history.pop() else { return Err(AppError::config("No profile deletions to undo.")); - } - let restored = self.undo_history.pop().unwrap(); - // Re-add the profile to the vault + }; + let restored = deleted.stored.clone(); + // Re-add the profile to the vault unless it is somehow already there. if !self .vault .profiles .iter() - .any(|p| p.public_key == restored.npub) + .any(|p| p.public_key == restored.public_key) { - let stored = StoredProfile { - label: restored.label.clone(), - public_key: restored.npub.clone(), - secret_key: "".to_string(), - created_at: restored.created_at, - picture: restored.picture.clone(), - nip05: restored.nip05.clone(), - signer_mode: SignerMode::Embedded, - }; - self.vault.profiles.push(stored); - // If no active profile, this restored one becomes active - if self.vault.active_profile.is_none() { - self.vault.active_profile = Some(restored.npub.clone()); + // A secret-less record (should not happen for records created by + // delete_profile_record) must not silently create a hollow + // profile: refuse and hand the entry back rather than corrupt the + // vault. + if restored.secret_key.trim().is_empty() { + self.undo_history.push(deleted); + return Err(AppError::internal( + "The undo entry is missing its secret key; the profile was not restored.", + )); } + self.vault + .active_profile + .get_or_insert(restored.public_key.clone()); + self.vault.profiles.push(restored.clone()); } - Ok(restored) + let is_active = self.vault.active_profile.as_deref() == Some(restored.public_key.as_str()); + Ok(ProfileSummary { + label: restored.label.clone(), + npub: restored.public_key.clone(), + created_at: restored.created_at, + is_active, + picture: restored.picture.clone(), + nip05: restored.nip05.clone(), + }) } /// Protect the vault with `new_password`, re-encrypting every stored key. @@ -361,7 +373,11 @@ impl App { active_profile: profiles::active_summary(&self.vault), profiles: profiles::summaries(&self.vault), settings: self.settings.clone(), - undo_history: self.undo_history.clone(), + undo_history: self + .undo_history + .iter() + .map(|deleted| deleted.summary.clone()) + .collect(), last_publish: self.last_publish.clone(), signer_mode: self.signer_mode, } @@ -622,4 +638,32 @@ mod tests { assert_eq!(view.profiles.len(), 2); assert!(view.profiles.iter().all(|p| p.npub.starts_with("npub1"))); } + + #[test] + fn undo_delete_restores_working_profile_without_leaking_secret() { + let mut app = sample_app(); + let target = app.vault.profiles[0].clone(); + let secret = target.secret_key.clone(); + + let deleted = profiles::delete_profile_record(&mut app.vault, &target.public_key).unwrap(); + app.undo_history.push(deleted); + assert_eq!(app.vault.profiles.len(), 1); + + let restored = app.undo_delete().unwrap(); + assert_eq!(restored.npub, target.public_key); + assert_eq!(app.vault.profiles.len(), 2); + let stored = app + .vault + .profiles + .iter() + .find(|p| p.public_key == target.public_key) + .unwrap(); + assert_eq!(stored.secret_key, secret, "undo must restore the real key"); + assert!(!stored.secret_key.is_empty()); + + // The UI-facing view carries summaries only — never secret material. + let view_json = serde_json::to_string(&app.state_view()).unwrap(); + assert!(!view_json.contains(&secret)); + assert!(app.undo_history.is_empty()); + } } diff --git a/src/ipc.rs b/src/ipc.rs index cfa1bed..3edaa24 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -793,9 +793,9 @@ async fn run_with_app(app: &mut App, request: Request) -> Result { - let deleted = profiles::delete_profile(&mut app.vault, &npub)?; + let deleted = profiles::delete_profile_record(&mut app.vault, &npub)?; app.save_vault()?; - app.undo_history.push(deleted.clone()); + app.undo_history.push(deleted); Ok(json!(app.state_view())) } Request::UndoDelete => { diff --git a/src/main.rs b/src/main.rs index c578b63..525b731 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,11 +5,11 @@ use keynectr::app::App; use keynectr::bunker::Signer; use keynectr::errors::{AppError, ErrorKind}; use keynectr::ipc; -use keynectr::profiles::{self, ProfileSummary}; +use keynectr::profiles; use keynectr::publish; use keynectr::relays; use keynectr::settings::Theme; -use keynectr::vault::{self, StoredProfile, Vault}; +use keynectr::vault::{self, Vault}; const USAGE: &str = "\ keynectr [args...] @@ -629,26 +629,13 @@ fn cli_info() -> Result { } /// Delete a profile by npub, moving it to the undo stack. -/// Returns the deleted profile summary, or an error if not found. -fn delete_profile_direct(vault: &mut Vault, npub: &str) -> Result { - let pos = vault - .profiles - .iter() - .position(|p| p.public_key == npub) - .ok_or_else(|| AppError::profile_not_found(npub))?; - let stored = vault.profiles.remove(pos); - // Clear the active_profile if it was the one deleted - if vault.active_profile.as_deref() == Some(npub) { - vault.active_profile = None; - } - Ok(ProfileSummary { - label: stored.label, - npub: stored.public_key, - created_at: stored.created_at, - is_active: false, - picture: stored.picture, - nip05: stored.nip05, - }) +/// Returns the full deleted record so the CLI can report it and push the +/// same entry the IPC path uses. +fn delete_profile_direct( + vault: &mut Vault, + npub: &str, +) -> Result { + profiles::delete_profile_record(vault, npub) } fn cli_delete_profile(args: &[String]) -> Result { @@ -657,37 +644,22 @@ fn cli_delete_profile(args: &[String]) -> Result { } let npub = args[2].clone(); let mut app = App::load()?; + // Capture the label BEFORE removal; the profile is gone afterwards. + let label = profiles::profile_label(&app.vault, &npub) + .unwrap_or(&npub) + .to_string(); let deleted = delete_profile_direct(&mut app.vault, &npub)?; app.save_vault()?; - // Add to undo history - app.undo_history.push(deleted.clone()); + // Add to undo history (full record: undo restores a working profile). + app.undo_history.push(deleted); Ok(format!( - "Profile '{}' deleted (npub: {}). Use 'undo-delete' to restore.", - profiles::profile_label(&app.vault, &npub).unwrap_or(&npub), - npub + "Profile '{label}' deleted (npub: {npub}). Use 'undo-delete' to restore." )) } fn cli_undo_delete() -> Result { let mut app = App::load()?; - if app.undo_history.is_empty() { - return Err(AppError::config("No profile deletions to undo.")); - } - let restored = app.undo_history.pop().unwrap(); - // Re-add the profile to the vault - let stored = StoredProfile { - label: restored.label.clone(), - public_key: restored.npub.clone(), - secret_key: "".to_string(), - created_at: restored.created_at, - picture: restored.picture, - nip05: restored.nip05, - signer_mode: keynectr::vault::SignerMode::Embedded, - }; - app.vault.profiles.push(stored); - if app.vault.active_profile.is_none() { - app.vault.active_profile = Some(restored.npub.clone()); - } + let restored = app.undo_delete()?; app.save_vault()?; Ok(format!( "Profile '{}' restored from undo stack.", diff --git a/src/profiles.rs b/src/profiles.rs index ec6ad14..218f229 100644 --- a/src/profiles.rs +++ b/src/profiles.rs @@ -745,9 +745,19 @@ pub fn parse_secret_key(hex_str: &str) -> Result { SecretKey::from_slice(&bytes).map_err(|e| AppError::invalid_secret(format!("{e}"))) } -/// Delete a profile by npub, returning the deleted profile for undo. -/// The vault must not be encrypted, or the key must be provided. -pub fn delete_profile(vault: &mut Vault, npub: &str) -> Result { +/// A profile removed from the vault together with everything needed to put it +/// back: the safe summary for the UI *and* the full `StoredProfile` including +/// its secret key material (plaintext or encrypted blob, exactly as stored). +#[derive(Debug, Clone)] +pub struct DeletedProfile { + pub summary: ProfileSummary, + pub stored: StoredProfile, +} + +/// Delete a profile by npub, returning the deleted record for undo. The +/// returned `DeletedProfile` carries the real stored secret so undo can +/// restore a fully functional profile. Never serialize it to the UI. +pub fn delete_profile_record(vault: &mut Vault, npub: &str) -> Result { let pos = vault .profiles .iter() @@ -757,14 +767,22 @@ pub fn delete_profile(vault: &mut Vault, npub: &str) -> Result Result { + delete_profile_record(vault, npub).map(|deleted| deleted.summary) } #[cfg(test)]