diff --git a/src-tauri/src/backup.rs b/src-tauri/src/backup.rs index 53ea006..3b364c9 100644 --- a/src-tauri/src/backup.rs +++ b/src-tauri/src/backup.rs @@ -184,14 +184,28 @@ pub fn list_backups(backup_dir: &Path) -> Result, String> { Ok(entries) } -/// Restore a backup by extracting the zip over the vault directory -pub fn restore_backup(vault_path: &str, backup_path: &str) -> Result<(), String> { - let vault = Path::new(vault_path); - let backup = Path::new(backup_path); - - if !backup.exists() { - return Err("Backup file does not exist".to_string()); +fn validated_backup_file(backup_dir: &Path, backup_path: &str) -> Result { + let backup_dir = fs::canonicalize(backup_dir).map_err(|error| error.to_string())?; + let backup = fs::canonicalize(backup_path).map_err(|error| error.to_string())?; + if backup.parent() != Some(backup_dir.as_path()) + || backup.extension().and_then(|extension| extension.to_str()) != Some("zip") + || !backup.is_file() + { + return Err( + "Backup path must point to a ZIP file in the configured backup directory".to_string(), + ); } + Ok(backup) +} + +/// Restore a backup by extracting the zip over the vault directory +pub fn restore_backup( + vault_path: &str, + backup_dir: &Path, + backup_path: &str, +) -> Result<(), String> { + let vault = Path::new(vault_path); + let backup = validated_backup_file(backup_dir, backup_path)?; let file = fs::File::open(backup).map_err(|e| format!("Failed to open backup: {}", e))?; let mut archive = @@ -245,12 +259,9 @@ pub fn restore_backup(vault_path: &str, backup_path: &str) -> Result<(), String> } /// Delete a single backup file -pub fn delete_backup(backup_path: &str) -> Result<(), String> { - let path = Path::new(backup_path); - if path.exists() { - fs::remove_file(path).map_err(|e| e.to_string())?; - } - Ok(()) +pub fn delete_backup(backup_dir: &Path, backup_path: &str) -> Result<(), String> { + let path = validated_backup_file(backup_dir, backup_path)?; + fs::remove_file(path).map_err(|error| error.to_string()) } /// Remove old backups keeping only the newest `max_count` @@ -261,9 +272,31 @@ pub fn cleanup_old_backups(backup_dir: &Path, max_count: u32) -> Result<(), Stri if backups.len() as u32 > max_count { let to_remove = backups.split_off(max_count as usize); for entry in to_remove { - delete_backup(&entry.path)?; + delete_backup(backup_dir, &entry.path)?; } } Ok(()) } + +#[cfg(test)] +mod tests { + use super::delete_backup; + use std::fs; + use uuid::Uuid; + + #[test] + fn delete_backup_rejects_files_outside_backup_directory() { + let root = std::env::temp_dir().join(format!("helixnotes-backup-test-{}", Uuid::new_v4())); + let backup_dir = root.join("backups"); + let outside = root.join("outside.zip"); + fs::create_dir_all(&backup_dir).unwrap(); + fs::write(&outside, "must survive").unwrap(); + + let result = delete_backup(&backup_dir, &outside.to_string_lossy()); + + assert!(result.is_err()); + assert!(outside.exists()); + fs::remove_dir_all(root).unwrap(); + } +} diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 9d8a2eb..45b78cc 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -1945,15 +1945,18 @@ pub fn list_backups(state: State<'_, AppState>) -> Result, Stri #[tauri::command] pub fn restore_backup(app: AppHandle, backup_path: String) -> Result<(), String> { - let vault_path = { + let (vault_path, backup_dir) = { let state = app.state::(); let config = state.config.lock().map_err(|e| e.to_string())?; - config.active_vault.clone().ok_or("No active vault")? + ( + config.active_vault.clone().ok_or("No active vault")?, + crate::backup::get_backup_dir(&config.backup_location)?, + ) }; std::thread::spawn(move || { use tauri::Emitter; - match crate::backup::restore_backup(&vault_path, &backup_path) { + match crate::backup::restore_backup(&vault_path, &backup_dir, &backup_path) { Ok(()) => { let _ = app.emit( "restore-done", @@ -1977,8 +1980,10 @@ pub fn restore_backup(app: AppHandle, backup_path: String) -> Result<(), String> } #[tauri::command] -pub fn delete_backup(backup_path: String) -> Result<(), String> { - crate::backup::delete_backup(&backup_path) +pub fn delete_backup(state: State<'_, AppState>, backup_path: String) -> Result<(), String> { + let config = state.config.lock().map_err(|error| error.to_string())?; + let backup_dir = crate::backup::get_backup_dir(&config.backup_location)?; + crate::backup::delete_backup(&backup_dir, &backup_path) } #[tauri::command]