From 405f732e1dba57248cfe204e2c5919378e83a759 Mon Sep 17 00:00:00 2001 From: Koray Taylan Davgana Date: Wed, 20 May 2026 00:28:48 +0300 Subject: [PATCH 1/4] docs: update recovery section in db/mod.rs to describe timestamped backup behavior --- backend/src/db/mod.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/backend/src/db/mod.rs b/backend/src/db/mod.rs index f800f21..f695299 100644 --- a/backend/src/db/mod.rs +++ b/backend/src/db/mod.rs @@ -15,8 +15,21 @@ //! ## Recovery from corrupt or unreadable databases //! //! If the database file exists but cannot be decrypted (wrong key or -//! corruption), it is deleted and recreated from scratch. Disk configs must be -//! re-added by the user, and bookmarks and preferences are non-critical. +//! corruption detected via failed `user_version` pragma or `Connection::open`), +//! a timestamped backup is created first: +//! +//! - `diskdeck.db.corrupt..bak` (raw byte copy of the original) +//! - `diskdeck.db.corrupt..bak.txt` (sidecar explaining the +//! incident, timestamp, and how to attempt manual restore by renaming back) +//! +//! The backup is written to the **same directory** as the original. Only after +//! the backup (and sidecar) succeeds is the corrupt `diskdeck.db` removed and a +//! fresh database created. This gives users and support staff a post-mortem +//! recovery path without changing the "start fresh on irrecoverable key/DB" +//! safety posture. +//! +//! If the backup copy fails for any reason (e.g. disk full), the original file +//! is left untouched and a clear `DiskDeckError` is returned instead of deleting. //! //! ## Schema migrations //! From 6a32eef78eb0eeef31541824fe78fd4f92401587 Mon Sep 17 00:00:00 2001 From: Koray Taylan Davgana Date: Wed, 20 May 2026 00:30:43 +0300 Subject: [PATCH 2/4] =?UTF-8?q?feat(db):=20safer=20recovery=20=E2=80=94=20?= =?UTF-8?q?backup=20corrupt=20diskdeck.db=20before=20delete=20with=20times?= =?UTF-8?q?tamped=20.bak=20+=20.txt=20sidecar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract backup_and_remove_corrupt_db helper that only deletes after successful copy + best-effort sidecar - Call from both recovery branches in DiskStore::new (user_version failure and initial open failure) - If backup fails, return Database error and leave original file untouched - Enhance test_corrupt_db_gets_replaced to assert .bak and sidecar are created - Updated module docs to document the new parachute behavior This directly addresses #5: never lose user data (disk credentials etc.) without a recoverable artifact. Closes #5 --- backend/src/db/mod.rs | 118 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 112 insertions(+), 6 deletions(-) diff --git a/backend/src/db/mod.rs b/backend/src/db/mod.rs index f695299..c43da14 100644 --- a/backend/src/db/mod.rs +++ b/backend/src/db/mod.rs @@ -180,12 +180,15 @@ impl DiskStore { match conn.pragma_query_value(None, "user_version", |row| row.get::<_, i32>(0)) { Ok(_) => Self::init(conn), Err(_) => { - // Key mismatch or corrupt DB — remove and recreate + // Key mismatch or corrupt DB — backup first, then recreate log::warn!( "Database appears corrupt or key mismatch, starting fresh" ); drop(conn); - let _ = std::fs::remove_file(&db_path); + backup_and_remove_corrupt_db( + &db_path, + "user_version pragma failed after key application (possible key mismatch or corruption)", + )?; let conn = Connection::open(&db_path)?; if !key.is_empty() { conn.pragma_update(None, "key", format!("x'{key}'"))?; @@ -195,9 +198,12 @@ impl DiskStore { } } Err(_) => { - // File cannot be opened at all — remove and recreate + // File cannot be opened at all — backup first (if exists), then recreate log::warn!("Database cannot be opened, starting fresh"); - let _ = std::fs::remove_file(&db_path); + backup_and_remove_corrupt_db( + &db_path, + "Connection::open failed (file missing, unreadable, or permissions issue)", + )?; let conn = Connection::open(&db_path)?; if !key.is_empty() { conn.pragma_update(None, "key", format!("x'{key}'"))?; @@ -857,6 +863,83 @@ fn run_migrations(conn: &Connection) -> Result<(), DiskDeckError> { Ok(()) } +/// Backs up a corrupt/unreadable `diskdeck.db` (if it exists) to a timestamped +/// `.bak` file + explanatory `.txt` sidecar in the same directory, then removes +/// the original. If the backup step fails, the original is left in place and +/// an error is returned so the caller does not blindly delete user data. +/// +/// This is the single place that implements the "never delete without a parachute" +/// safety rule for database recovery. Called from the two recovery branches in +/// [`DiskStore::new`]. +/// +/// The sidecar is best-effort (does not fail the backup if it cannot be written). +fn backup_and_remove_corrupt_db(db_path: &Path, reason: &str) -> Result<(), DiskDeckError> { + if !db_path.exists() { + // No file to back up (e.g. first-run or already-deleted). Proceed to create fresh. + return Ok(()); + } + + let timestamp = chrono::Local::now().format("%Y%m%d-%H%M%S").to_string(); + let parent = db_path.parent().unwrap_or(db_path); + let backup_name = format!("diskdeck.db.corrupt.{}.bak", timestamp); + let backup_path = parent.join(&backup_name); + + // 1. Copy the raw bytes first. This must succeed before we consider deleting. + if let Err(e) = std::fs::copy(db_path, &backup_path) { + let msg = format!( + "Could not create backup of corrupt database before recovery ({}) at {}: {}. Refusing to delete the original.", + reason, + db_path.display(), + e + ); + log::error!("{}", msg); + return Err(DiskDeckError::Database(msg)); + } + + // 2. Best-effort sidecar (optional but strongly recommended per requirements). + let sidecar_name = format!("diskdeck.db.corrupt.{}.bak.txt", timestamp); + let sidecar_path = parent.join(&sidecar_name); + let sidecar_content = format!( + "DiskDeck database recovery backup\n\ + \n\ + Reason: {}\n\ + Timestamp (local): {}\n\ + Original database path: {}\n\ + Backup file: {}\n\ + \n\ + This is a raw byte-for-byte copy of the diskdeck.db file that could not be\n\ + opened/decrypted (key mismatch, keychain issue, or file corruption).\n\ + \n\ + To attempt manual recovery with a different key or SQLCipher tooling:\n\ + 1. Rename the .bak file back to 'diskdeck.db' in the same directory.\n\ + 2. Ensure the correct 32-byte encryption key is available in the OS keychain.\n\ + 3. Restart DiskDeck.\n\ + \n\ + Power users and support staff can use this file + sidecar for post-mortem\n\ + analysis or forensic recovery. The backup is never encrypted or moved.\n", + reason, timestamp, db_path.display(), backup_path.display() + ); + if let Err(e) = std::fs::write(&sidecar_path, sidecar_content) { + log::warn!( + "Backup .bak created successfully but sidecar {} could not be written: {}", + sidecar_path.display(), + e + ); + // Do not fail the whole backup for sidecar — it is user-friendly, not mandatory for safety. + } + + // 3. Only now is it safe to delete the original. + std::fs::remove_file(db_path)?; + + log::warn!( + "Created timestamped backup {} (and sidecar) of corrupt database before recovery. Reason: {}. Original deleted; fresh DB will be created.", + backup_path.display(), + reason + ); + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -1016,8 +1099,8 @@ mod tests { // Write garbage to simulate an encrypted/corrupt database file std::fs::write(&db_path, b"not a sqlite database at all").unwrap(); - // DiskStore::new should detect the corruption, delete the file, - // and create a fresh database + // DiskStore::new should detect the corruption, *backup* the file first + // (creating .bak + .txt sidecar), delete the original, and create fresh DB. let store = DiskStore::new(dir.path(), "").unwrap(); let disks = store.load().unwrap(); assert!(disks.is_empty()); @@ -1028,6 +1111,29 @@ mod tests { .unwrap(); let loaded = store.load().unwrap(); assert_eq!(loaded.len(), 1); + + // Critical safety check: a timestamped backup + sidecar must exist next to the (now-deleted) original + let dir_entries: Vec<_> = std::fs::read_dir(dir.path()) + .unwrap() + .filter_map(Result::ok) + .collect(); + let bak_file = dir_entries.iter().find(|e| { + let name = e.file_name().to_string_lossy().to_string(); + name.contains("diskdeck.db.corrupt.") && name.ends_with(".bak") + }); + assert!( + bak_file.is_some(), + "expected a timestamped .bak backup file to have been created before deleting the corrupt DB" + ); + + let txt_file = dir_entries.iter().find(|e| { + let name = e.file_name().to_string_lossy().to_string(); + name.contains("diskdeck.db.corrupt.") && name.ends_with(".bak.txt") + }); + assert!( + txt_file.is_some(), + "expected a .bak.txt sidecar explaining the recovery to have been created" + ); } #[test] From 26a050adf68fd436923d09b81f01ca2b872159dc Mon Sep 17 00:00:00 2001 From: Koray Taylan Davgana Date: Wed, 20 May 2026 01:02:53 +0300 Subject: [PATCH 3/4] fix(db): address all 6 Copilot review comments on PR #10 (issue #5) - Docs: clarify that only .bak backup is mandatory for deletion; sidecar is best-effort (addresses thread on module docs) - Capture actual rusqlite/io errors in both recovery paths and include in reason passed to backup helper + enhanced logs (addresses pragma and Connection::open threads) - Timestamp: millisecond suffix to avoid same-second filename collisions on rapid recoveries - Parent dir: robust fallback to "." instead of treating file path as dir (prevents invalid paths) - Backup failure: log full details server-side; return Storage error (passes user-friendly message through IPC sanitization without leaking paths) All addressed per review feedback. CI + clippy + tests green. Refs #5 --- backend/src/db/mod.rs | 54 ++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/backend/src/db/mod.rs b/backend/src/db/mod.rs index c43da14..f0befcd 100644 --- a/backend/src/db/mod.rs +++ b/backend/src/db/mod.rs @@ -23,10 +23,12 @@ //! incident, timestamp, and how to attempt manual restore by renaming back) //! //! The backup is written to the **same directory** as the original. Only after -//! the backup (and sidecar) succeeds is the corrupt `diskdeck.db` removed and a -//! fresh database created. This gives users and support staff a post-mortem -//! recovery path without changing the "start fresh on irrecoverable key/DB" -//! safety posture. +//! the backup copy (`.bak` file) succeeds is the corrupt `diskdeck.db` removed +//! and a fresh database created. The accompanying `.bak.txt` sidecar is +//! best-effort (written after the critical backup copy; a sidecar failure does +//! not prevent deletion of the original or recovery). This gives users and +//! support staff a post-mortem recovery path without changing the "start fresh +//! on irrecoverable key/DB" safety posture. //! //! If the backup copy fails for any reason (e.g. disk full), the original file //! is left untouched and a clear `DiskDeckError` is returned instead of deleting. @@ -179,15 +181,19 @@ impl DiskStore { // Verify the database is readable match conn.pragma_query_value(None, "user_version", |row| row.get::<_, i32>(0)) { Ok(_) => Self::init(conn), - Err(_) => { + Err(e) => { // Key mismatch or corrupt DB — backup first, then recreate log::warn!( - "Database appears corrupt or key mismatch, starting fresh" + "Database appears corrupt or key mismatch, starting fresh: {}", + e ); drop(conn); backup_and_remove_corrupt_db( &db_path, - "user_version pragma failed after key application (possible key mismatch or corruption)", + &format!( + "user_version pragma failed after key application (possible key mismatch or corruption): {}", + e + ), )?; let conn = Connection::open(&db_path)?; if !key.is_empty() { @@ -197,12 +203,15 @@ impl DiskStore { } } } - Err(_) => { + Err(e) => { // File cannot be opened at all — backup first (if exists), then recreate - log::warn!("Database cannot be opened, starting fresh"); + log::warn!("Database cannot be opened, starting fresh: {}", e); backup_and_remove_corrupt_db( &db_path, - "Connection::open failed (file missing, unreadable, or permissions issue)", + &format!( + "Connection::open failed (file missing, unreadable, or permissions issue): {}", + e + ), )?; let conn = Connection::open(&db_path)?; if !key.is_empty() { @@ -879,21 +888,34 @@ fn backup_and_remove_corrupt_db(db_path: &Path, reason: &str) -> Result<(), Disk return Ok(()); } - let timestamp = chrono::Local::now().format("%Y%m%d-%H%M%S").to_string(); - let parent = db_path.parent().unwrap_or(db_path); + let now = chrono::Local::now(); + // Use millisecond precision to avoid filename collisions on rapid successive recoveries. + let timestamp = format!( + "{}-{:03}", + now.format("%Y%m%d-%H%M%S"), + now.timestamp_subsec_millis() + ); + let parent = db_path + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| std::path::Path::new(".")); let backup_name = format!("diskdeck.db.corrupt.{}.bak", timestamp); let backup_path = parent.join(&backup_name); // 1. Copy the raw bytes first. This must succeed before we consider deleting. if let Err(e) = std::fs::copy(db_path, &backup_path) { - let msg = format!( - "Could not create backup of corrupt database before recovery ({}) at {}: {}. Refusing to delete the original.", + let full_detail = format!( + "Could not create backup of corrupt database before recovery (reason: {}) at {}: {}. Refusing to delete the original.", reason, db_path.display(), e ); - log::error!("{}", msg); - return Err(DiskDeckError::Database(msg)); + log::error!("{}", full_detail); + // Return a user-visible error (Storage passes the message through IPC sanitization) + // without embedding internal paths in the user-facing string; full details are logged. + return Err(DiskDeckError::Storage( + "Could not create a backup of the corrupt database before recovery. The original database file has been preserved for safety. Check the application logs for details.".to_string() + )); } // 2. Best-effort sidecar (optional but strongly recommended per requirements). From 49b36466ca129bb1e1d3bef94a237ec8ec3354de Mon Sep 17 00:00:00 2001 From: Koray Taylan Davgana Date: Wed, 20 May 2026 01:07:45 +0300 Subject: [PATCH 4/4] fix(db): address latest Copilot review comments on current HEAD (PR #10, issue #5) - Docs: update example filename pattern to reflect the millisecond suffix (YYYYMMDD-HHMMSS[-mmm]) - Robustness: log backup path *immediately* after successful copy (before sidecar/remove) - Handle remove_file failure after backup: log full details incl. backup path; return clear Storage error so user knows backup exists even if original delete fails (e.g. Windows lock). Never lose the parachute reference. These address the two new comments from the fresh Copilot review. Refs #5 --- backend/src/db/mod.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/backend/src/db/mod.rs b/backend/src/db/mod.rs index f0befcd..b0f364d 100644 --- a/backend/src/db/mod.rs +++ b/backend/src/db/mod.rs @@ -18,8 +18,8 @@ //! corruption detected via failed `user_version` pragma or `Connection::open`), //! a timestamped backup is created first: //! -//! - `diskdeck.db.corrupt..bak` (raw byte copy of the original) -//! - `diskdeck.db.corrupt..bak.txt` (sidecar explaining the +//! - `diskdeck.db.corrupt.]>.bak` (raw byte copy of the original) +//! - `diskdeck.db.corrupt.]>.bak.txt` (sidecar explaining the //! incident, timestamp, and how to attempt manual restore by renaming back) //! //! The backup is written to the **same directory** as the original. Only after @@ -918,6 +918,14 @@ fn backup_and_remove_corrupt_db(db_path: &Path, reason: &str) -> Result<(), Disk )); } + // Log backup success *immediately* after copy (before any later steps that could fail). + // This ensures the backup location is always recorded in logs even if sidecar or remove_file later fail. + log::warn!( + "Successfully created timestamped backup {} of corrupt database before recovery. Reason: {}. Original will now be removed.", + backup_path.display(), + reason + ); + // 2. Best-effort sidecar (optional but strongly recommended per requirements). let sidecar_name = format!("diskdeck.db.corrupt.{}.bak.txt", timestamp); let sidecar_path = parent.join(&sidecar_name); @@ -951,10 +959,23 @@ fn backup_and_remove_corrupt_db(db_path: &Path, reason: &str) -> Result<(), Disk } // 3. Only now is it safe to delete the original. - std::fs::remove_file(db_path)?; + // Handle remove failure gracefully: the backup exists and is the important artifact for recovery. + if let Err(e) = std::fs::remove_file(db_path) { + let full_detail = format!( + "Backup {} created successfully for corrupt DB (reason: {}), but remove_file of original {} failed: {}. Backup remains available.", + backup_path.display(), + reason, + db_path.display(), + e + ); + log::error!("{}", full_detail); + return Err(DiskDeckError::Storage( + "Backup of the corrupt database succeeded, but the original file could not be removed (it may be locked or in use on this system). The backup copy is preserved in the same directory and can be used for manual recovery or inspection. Check the application logs for the exact backup path.".to_string() + )); + } log::warn!( - "Created timestamped backup {} (and sidecar) of corrupt database before recovery. Reason: {}. Original deleted; fresh DB will be created.", + "Removed original corrupt database after successful backup. Fresh DB will be created. Backup: {}. Reason: {}", backup_path.display(), reason );