fix(undo): restore full profile with secret key on undo-delete

This commit is contained in:
Avi 2026-09-10 12:50:07 -05:00
commit d101b8e236
4 changed files with 114 additions and 80 deletions

View file

@ -22,8 +22,10 @@ pub struct App {
pub settings: Settings, pub settings: Settings,
/// Derived vault key, present only while the encrypted vault is unlocked. /// Derived vault key, present only while the encrypted vault is unlocked.
unlock_key: Option<VaultKey>, unlock_key: Option<VaultKey>,
/// Stack of deleted profiles for undo functionality. /// Stack of deleted profiles for undo functionality. Holds the full
pub undo_history: Vec<ProfileSummary>, /// 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<crate::profiles::DeletedProfile>,
/// The most recent publish report, persisted across restarts. /// The most recent publish report, persisted across restarts.
pub last_publish: Option<StoredPublishReport>, pub last_publish: Option<StoredPublishReport>,
/// Active signer mode. /// Active signer mode.
@ -60,7 +62,8 @@ pub struct AppStateView {
pub active_profile: Option<ProfileSummary>, pub active_profile: Option<ProfileSummary>,
pub profiles: Vec<ProfileSummary>, pub profiles: Vec<ProfileSummary>,
pub settings: Settings, 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")] #[serde(skip_serializing_if = "Vec::is_empty")]
pub undo_history: Vec<ProfileSummary>, pub undo_history: Vec<ProfileSummary>,
/// The most recent publish report, persisted across restarts. /// The most recent publish report, persisted across restarts.
@ -215,36 +218,45 @@ impl App {
Ok(revealed) 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. /// Returns the restored profile summary, or an error if there is no undo history.
pub fn undo_delete(&mut self) -> Result<ProfileSummary, AppError> { pub fn undo_delete(&mut self) -> Result<ProfileSummary, AppError> {
if self.undo_history.is_empty() { let Some(deleted) = self.undo_history.pop() else {
return Err(AppError::config("No profile deletions to undo.")); return Err(AppError::config("No profile deletions to undo."));
} };
let restored = self.undo_history.pop().unwrap(); let restored = deleted.stored.clone();
// Re-add the profile to the vault // Re-add the profile to the vault unless it is somehow already there.
if !self if !self
.vault .vault
.profiles .profiles
.iter() .iter()
.any(|p| p.public_key == restored.npub) .any(|p| p.public_key == restored.public_key)
{ {
let stored = StoredProfile { // 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());
}
let is_active = self.vault.active_profile.as_deref() == Some(restored.public_key.as_str());
Ok(ProfileSummary {
label: restored.label.clone(), label: restored.label.clone(),
public_key: restored.npub.clone(), npub: restored.public_key.clone(),
secret_key: "".to_string(),
created_at: restored.created_at, created_at: restored.created_at,
is_active,
picture: restored.picture.clone(), picture: restored.picture.clone(),
nip05: restored.nip05.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());
}
}
Ok(restored)
} }
/// Protect the vault with `new_password`, re-encrypting every stored key. /// Protect the vault with `new_password`, re-encrypting every stored key.
@ -361,7 +373,11 @@ impl App {
active_profile: profiles::active_summary(&self.vault), active_profile: profiles::active_summary(&self.vault),
profiles: profiles::summaries(&self.vault), profiles: profiles::summaries(&self.vault),
settings: self.settings.clone(), 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(), last_publish: self.last_publish.clone(),
signer_mode: self.signer_mode, signer_mode: self.signer_mode,
} }
@ -622,4 +638,32 @@ mod tests {
assert_eq!(view.profiles.len(), 2); assert_eq!(view.profiles.len(), 2);
assert!(view.profiles.iter().all(|p| p.npub.starts_with("npub1"))); 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());
}
} }

View file

@ -793,9 +793,9 @@ async fn run_with_app(app: &mut App, request: Request) -> Result<serde_json::Val
Ok(json!(app.settings)) Ok(json!(app.settings))
} }
Request::DeleteProfile { npub } => { Request::DeleteProfile { npub } => {
let deleted = profiles::delete_profile(&mut app.vault, &npub)?; let deleted = profiles::delete_profile_record(&mut app.vault, &npub)?;
app.save_vault()?; app.save_vault()?;
app.undo_history.push(deleted.clone()); app.undo_history.push(deleted);
Ok(json!(app.state_view())) Ok(json!(app.state_view()))
} }
Request::UndoDelete => { Request::UndoDelete => {

View file

@ -5,11 +5,11 @@ use keynectr::app::App;
use keynectr::bunker::Signer; use keynectr::bunker::Signer;
use keynectr::errors::{AppError, ErrorKind}; use keynectr::errors::{AppError, ErrorKind};
use keynectr::ipc; use keynectr::ipc;
use keynectr::profiles::{self, ProfileSummary}; use keynectr::profiles;
use keynectr::publish; use keynectr::publish;
use keynectr::relays; use keynectr::relays;
use keynectr::settings::Theme; use keynectr::settings::Theme;
use keynectr::vault::{self, StoredProfile, Vault}; use keynectr::vault::{self, Vault};
const USAGE: &str = "\ const USAGE: &str = "\
keynectr <command> [args...] keynectr <command> [args...]
@ -629,26 +629,13 @@ fn cli_info() -> Result<String, AppError> {
} }
/// Delete a profile by npub, moving it to the undo stack. /// Delete a profile by npub, moving it to the undo stack.
/// Returns the deleted profile summary, or an error if not found. /// Returns the full deleted record so the CLI can report it and push the
fn delete_profile_direct(vault: &mut Vault, npub: &str) -> Result<ProfileSummary, AppError> { /// same entry the IPC path uses.
let pos = vault fn delete_profile_direct(
.profiles vault: &mut Vault,
.iter() npub: &str,
.position(|p| p.public_key == npub) ) -> Result<profiles::DeletedProfile, AppError> {
.ok_or_else(|| AppError::profile_not_found(npub))?; profiles::delete_profile_record(vault, 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,
})
} }
fn cli_delete_profile(args: &[String]) -> Result<String, AppError> { fn cli_delete_profile(args: &[String]) -> Result<String, AppError> {
@ -657,37 +644,22 @@ fn cli_delete_profile(args: &[String]) -> Result<String, AppError> {
} }
let npub = args[2].clone(); let npub = args[2].clone();
let mut app = App::load()?; 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)?; let deleted = delete_profile_direct(&mut app.vault, &npub)?;
app.save_vault()?; app.save_vault()?;
// Add to undo history // Add to undo history (full record: undo restores a working profile).
app.undo_history.push(deleted.clone()); app.undo_history.push(deleted);
Ok(format!( Ok(format!(
"Profile '{}' deleted (npub: {}). Use 'undo-delete' to restore.", "Profile '{label}' deleted (npub: {npub}). Use 'undo-delete' to restore."
profiles::profile_label(&app.vault, &npub).unwrap_or(&npub),
npub
)) ))
} }
fn cli_undo_delete() -> Result<String, AppError> { fn cli_undo_delete() -> Result<String, AppError> {
let mut app = App::load()?; let mut app = App::load()?;
if app.undo_history.is_empty() { let restored = app.undo_delete()?;
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());
}
app.save_vault()?; app.save_vault()?;
Ok(format!( Ok(format!(
"Profile '{}' restored from undo stack.", "Profile '{}' restored from undo stack.",

View file

@ -745,9 +745,19 @@ pub fn parse_secret_key(hex_str: &str) -> Result<SecretKey, AppError> {
SecretKey::from_slice(&bytes).map_err(|e| AppError::invalid_secret(format!("{e}"))) SecretKey::from_slice(&bytes).map_err(|e| AppError::invalid_secret(format!("{e}")))
} }
/// Delete a profile by npub, returning the deleted profile for undo. /// A profile removed from the vault together with everything needed to put it
/// The vault must not be encrypted, or the key must be provided. /// back: the safe summary for the UI *and* the full `StoredProfile` including
pub fn delete_profile(vault: &mut Vault, npub: &str) -> Result<ProfileSummary, AppError> { /// 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<DeletedProfile, AppError> {
let pos = vault let pos = vault
.profiles .profiles
.iter() .iter()
@ -757,14 +767,22 @@ pub fn delete_profile(vault: &mut Vault, npub: &str) -> Result<ProfileSummary, A
if vault.active_profile.as_deref() == Some(npub) { if vault.active_profile.as_deref() == Some(npub) {
vault.active_profile = None; vault.active_profile = None;
} }
Ok(ProfileSummary { let summary = ProfileSummary {
label: stored.label, label: stored.label.clone(),
npub: stored.public_key, npub: stored.public_key.clone(),
created_at: stored.created_at, created_at: stored.created_at,
is_active: false, is_active: false,
picture: stored.picture, picture: stored.picture.clone(),
nip05: stored.nip05, nip05: stored.nip05.clone(),
}) };
Ok(DeletedProfile { summary, stored })
}
/// Delete a profile by npub, returning only the safe summary. The secret key
/// is still recoverable in the returned value's vault removal only via
/// [`delete_profile_record`]; prefer that wherever an undo entry is kept.
pub fn delete_profile(vault: &mut Vault, npub: &str) -> Result<ProfileSummary, AppError> {
delete_profile_record(vault, npub).map(|deleted| deleted.summary)
} }
#[cfg(test)] #[cfg(test)]