fix: restrict backup paths

src-tauri/src/backup.rs

@@ -184,14 +184,28 @@ pub fn list_backups(backup_dir: &Path) -> Result<Vec<BackupEntry>, String> {
     Ok(entries)
 }
 
-/// Restore a backup by extracting the zip over the vault directory
+fn validated_backup_file(backup_dir: &Path, backup_path: &str) -> Result<PathBuf, String> {
-pub fn restore_backup(vault_path: &str, backup_path: &str) -> Result<(), String> {
+    let backup_dir = fs::canonicalize(backup_dir).map_err(|error| error.to_string())?;
-    let vault = Path::new(vault_path);
+    let backup = fs::canonicalize(backup_path).map_err(|error| error.to_string())?;
-    let backup = Path::new(backup_path);
+    if backup.parent() != Some(backup_dir.as_path())
-
+        || backup.extension().and_then(|extension| extension.to_str()) != Some("zip")
-    if !backup.exists() {
+        || !backup.is_file()
-        return Err("Backup file does not exist".to_string());
+    {
+        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> {
+pub fn delete_backup(backup_dir: &Path, backup_path: &str) -> Result<(), String> {
-    let path = Path::new(backup_path);
+    let path = validated_backup_file(backup_dir, backup_path)?;
-    if path.exists() {
+    fs::remove_file(path).map_err(|error| error.to_string())
-        fs::remove_file(path).map_err(|e| e.to_string())?;
-    }
-    Ok(())
 }
 
 /// 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();
+    }
+}

src-tauri/src/commands.rs

@@ -1945,15 +1945,18 @@ pub fn list_backups(state: State<'_, AppState>) -> Result<Vec<BackupEntry>, 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::<AppState>();
         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> {
+pub fn delete_backup(state: State<'_, AppState>, backup_path: String) -> Result<(), String> {
-    crate::backup::delete_backup(&backup_path)
+    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]