diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 45b78cc..9bca859 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -2026,9 +2026,8 @@ pub fn create_version( let config = state.config.lock().map_err(|e| e.to_string())?; let vault_path = config.active_vault.as_ref().ok_or("No active vault")?; let max_versions = config.max_versions_per_note; - let raw = std::fs::read_to_string(&path).map_err(|e| e.to_string())?; - crate::history::force_snapshot(vault_path, ¬e_id, &raw, max_versions); - Ok(()) + let raw = operations::read_vault_note(vault_path, &path)?.raw; + crate::history::force_snapshot(vault_path, ¬e_id, &raw, max_versions) } #[tauri::command] diff --git a/src-tauri/src/history.rs b/src-tauri/src/history.rs index 8ba518b..84c6985 100644 --- a/src-tauri/src/history.rs +++ b/src-tauri/src/history.rs @@ -1,22 +1,36 @@ use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use chrono::{DateTime, Utc}; use crate::types::VersionEntry; +fn safe_path_component<'a>(value: &'a str, label: &str) -> Result<&'a str, String> { + let mut components = Path::new(value).components(); + if value.is_empty() + || !matches!(components.next(), Some(Component::Normal(_))) + || components.next().is_some() + { + return Err(format!("Invalid {label}")); + } + Ok(value) +} + /// Directory: .helixnotes/history// -fn history_dir(vault_path: &str, note_id: &str) -> PathBuf { - Path::new(vault_path) +fn history_dir(vault_path: &str, note_id: &str) -> Result { + Ok(Path::new(vault_path) .join(".helixnotes") .join("history") - .join(note_id) + .join(safe_path_component(note_id, "note ID")?)) } /// Save a version snapshot if enough time has passed since the last one. /// Minimum interval: 5 minutes between snapshots. pub fn maybe_snapshot(vault_path: &str, note_id: &str, raw_content: &str, max_versions: u32) { - let dir = history_dir(vault_path, note_id); + let Ok(dir) = history_dir(vault_path, note_id) else { + log::warn!("Skipping history snapshot with an invalid note ID"); + return; + }; // Check if we should create a snapshot (5 min cooldown) if let Ok(entries) = fs::read_dir(&dir) { @@ -68,31 +82,30 @@ pub fn maybe_snapshot(vault_path: &str, note_id: &str, raw_content: &str, max_ve } /// Force-create a version snapshot, bypassing the cooldown. -pub fn force_snapshot(vault_path: &str, note_id: &str, raw_content: &str, max_versions: u32) { - let dir = history_dir(vault_path, note_id); - - if let Err(e) = fs::create_dir_all(&dir) { - eprintln!("Failed to create history dir: {}", e); - return; - } +pub fn force_snapshot( + vault_path: &str, + note_id: &str, + raw_content: &str, + max_versions: u32, +) -> Result<(), String> { + let dir = history_dir(vault_path, note_id)?; + fs::create_dir_all(&dir).map_err(|error| error.to_string())?; let timestamp = Utc::now().format("%Y-%m-%dT%H-%M-%S").to_string(); let filename = format!("{}.md", timestamp); let path = dir.join(&filename); - if let Err(e) = fs::write(&path, raw_content) { - eprintln!("Failed to write version snapshot: {}", e); - return; - } + fs::write(&path, raw_content).map_err(|error| error.to_string())?; if max_versions > 0 { - let _ = prune_versions(&dir, max_versions); + prune_versions(&dir, max_versions)?; } + Ok(()) } /// List all version snapshots for a note, newest first. pub fn list_versions(vault_path: &str, note_id: &str) -> Result, String> { - let dir = history_dir(vault_path, note_id); + let dir = history_dir(vault_path, note_id)?; if !dir.exists() { return Ok(Vec::new()); } @@ -134,6 +147,7 @@ pub fn list_versions(vault_path: &str, note_id: &str) -> Result Result { + safe_path_component(timestamp, "version timestamp")?; // Convert ISO timestamp back to filename: 2026-02-08T18:30:00Z → 2026-02-08T18-30-00.md let filename = if let Some(t_pos) = timestamp.find('T') { let date_part = ×tamp[..t_pos]; @@ -144,7 +158,7 @@ pub fn get_version(vault_path: &str, note_id: &str, timestamp: &str) -> Result Result<(), String> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::{get_version, list_versions}; + use std::fs; + use uuid::Uuid; + + #[test] + fn rejects_history_path_traversal() { + let vault = + std::env::temp_dir().join(format!("helixnotes-history-test-{}", Uuid::new_v4())); + let metadata = vault.join(".helixnotes"); + let escaped_history = metadata.join("escaped"); + fs::create_dir_all(&escaped_history).unwrap(); + fs::write(escaped_history.join("2026-01-01T00-00-00.md"), "escaped").unwrap(); + fs::create_dir_all(metadata.join("history").join("safe")).unwrap(); + fs::write(metadata.join("secret.md"), "secret").unwrap(); + + assert!(list_versions(&vault.to_string_lossy(), "../escaped").is_err()); + assert!(get_version(&vault.to_string_lossy(), "safe", "../../secret").is_err()); + + fs::remove_dir_all(vault).unwrap(); + } +}