diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index dffd096..9d8a2eb 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -513,8 +513,14 @@ pub fn create_notebook( } #[tauri::command] -pub fn rename_notebook(path: String, new_name: String) -> Result { - operations::rename_notebook(&path, &new_name) +pub fn rename_notebook( + state: State<'_, AppState>, + path: String, + new_name: String, +) -> Result { + let config = state.config.lock().map_err(|error| error.to_string())?; + let vault_path = config.active_vault.as_ref().ok_or("No active vault")?; + operations::rename_notebook(vault_path, &path, &new_name) } #[tauri::command] @@ -531,7 +537,7 @@ pub fn move_notebook( .map(|p| p.to_string_lossy().to_string()) .unwrap_or_default(); - let new_full_path = operations::move_notebook(¬ebook_path, &dest_parent)?; + let new_full_path = operations::move_notebook(vault_path, ¬ebook_path, &dest_parent)?; let new_relative = Path::new(&new_full_path) .strip_prefix(vault_path.as_str()) @@ -612,8 +618,10 @@ pub fn get_notes( } #[tauri::command] -pub fn read_note(path: String) -> Result { - operations::read_note(&path) +pub fn read_note(state: State<'_, AppState>, path: String) -> Result { + let config = state.config.lock().map_err(|error| error.to_string())?; + let vault_path = config.active_vault.as_ref().ok_or("No active vault")?; + operations::read_note(vault_path, &path) } #[tauri::command] @@ -623,24 +631,23 @@ pub fn save_note( meta: NoteMeta, body: String, ) -> Result<(), String> { - // Snapshot current content before overwriting (if file exists) - let config = state.config.lock().map_err(|e| e.to_string())?; - if let Some(vault_path) = &config.active_vault { - if std::path::Path::new(&path).exists() { - if let Ok(old_raw) = std::fs::read_to_string(&path) { - let max_versions = config.max_versions_per_note; - let note_id = meta.id.clone(); - let vp = vault_path.clone(); - // Snapshot in background so save isn't slowed down - std::thread::spawn(move || { - crate::history::maybe_snapshot(&vp, ¬e_id, &old_raw, max_versions); - }); - } - } - } + let config = state.config.lock().map_err(|error| error.to_string())?; + let vault_path = config + .active_vault + .as_ref() + .ok_or("No active vault")? + .clone(); + let max_versions = config.max_versions_per_note; + let old_raw = operations::read_vault_note(&vault_path, &path)?.raw; drop(config); - operations::save_note(&path, &meta, &body)?; + let note_id = meta.id.clone(); + let snapshot_vault = vault_path.clone(); + std::thread::spawn(move || { + crate::history::maybe_snapshot(&snapshot_vault, ¬e_id, &old_raw, max_versions); + }); + + operations::save_note(&vault_path, &path, &meta, &body)?; // Re-index note so search picks up changes (background to avoid blocking on FUSE fsync) index_note_bg(&state, &path); @@ -724,7 +731,7 @@ pub fn move_note( .map(|p| p.to_string_lossy().to_string()) .unwrap_or_default(); - let new_full_path = operations::move_note(¬e_path, &dest_notebook)?; + let new_full_path = operations::move_note(vault_path, ¬e_path, &dest_notebook)?; // Update quick access if the moved note was in it if !old_relative.is_empty() { @@ -1057,6 +1064,21 @@ pub fn get_tasks(state: State<'_, AppState>) -> Result, + note_path: &str, +) -> Result<(String, NoteMeta, String), String> { + let config = state.config.lock().map_err(|error| error.to_string())?; + let vault_path = config + .active_vault + .as_ref() + .ok_or("No active vault")? + .clone(); + drop(config); + let note = operations::read_vault_note(&vault_path, note_path)?; + Ok((vault_path, note.meta, note.content)) +} + fn toggle_checkbox_line(line: &str, done: bool) -> String { let mut s = line.to_string(); if done { @@ -1082,11 +1104,7 @@ pub fn set_task_done( raw_line: String, done: bool, ) -> Result<(), String> { - let p = std::path::Path::new(¬e_path); - let raw = std::fs::read_to_string(p).map_err(|e| e.to_string())?; - let filename = p.file_name().unwrap_or_default().to_string_lossy().to_string(); - let (meta, body) = crate::vault::frontmatter::parse_note(&raw, &filename); - + let (vault_path, meta, body) = read_task_note(&state, ¬e_path)?; let mut lines: Vec = body.lines().map(|l| l.to_string()).collect(); // Verify the expected line; if the note drifted, fall back to the first exact match. let idx = if lines.get(line).map(|l| *l == raw_line).unwrap_or(false) { @@ -1107,7 +1125,7 @@ pub fn set_task_done( if body.ends_with('\n') && !new_body.ends_with('\n') { new_body.push('\n'); } - operations::save_note(¬e_path, &meta, &new_body)?; + operations::save_note(&vault_path, ¬e_path, &meta, &new_body)?; index_note_bg(&state, ¬e_path); Ok(()) @@ -1140,11 +1158,7 @@ pub fn set_task_priority( Some(_) => return Err("Invalid priority".to_string()), }; - let p = std::path::Path::new(¬e_path); - let raw = std::fs::read_to_string(p).map_err(|e| e.to_string())?; - let filename = p.file_name().unwrap_or_default().to_string_lossy().to_string(); - let (meta, body) = crate::vault::frontmatter::parse_note(&raw, &filename); - + let (vault_path, meta, body) = read_task_note(&state, ¬e_path)?; let mut lines: Vec = body.lines().map(|l| l.to_string()).collect(); let idx = if lines.get(line).map(|l| *l == raw_line).unwrap_or(false) { line @@ -1164,7 +1178,7 @@ pub fn set_task_priority( if body.ends_with('\n') && !new_body.ends_with('\n') { new_body.push('\n'); } - operations::save_note(¬e_path, &meta, &new_body)?; + operations::save_note(&vault_path, ¬e_path, &meta, &new_body)?; index_note_bg(&state, ¬e_path); Ok(()) @@ -1196,11 +1210,7 @@ pub fn set_task_due( Some(_) => return Err("Invalid due date".to_string()), }; - let p = std::path::Path::new(¬e_path); - let raw = std::fs::read_to_string(p).map_err(|e| e.to_string())?; - let filename = p.file_name().unwrap_or_default().to_string_lossy().to_string(); - let (meta, body) = crate::vault::frontmatter::parse_note(&raw, &filename); - + let (vault_path, meta, body) = read_task_note(&state, ¬e_path)?; let mut lines: Vec = body.lines().map(|l| l.to_string()).collect(); let idx = if lines.get(line).map(|l| *l == raw_line).unwrap_or(false) { line @@ -1220,7 +1230,7 @@ pub fn set_task_due( if body.ends_with('\n') && !new_body.ends_with('\n') { new_body.push('\n'); } - operations::save_note(¬e_path, &meta, &new_body)?; + operations::save_note(&vault_path, ¬e_path, &meta, &new_body)?; index_note_bg(&state, ¬e_path); Ok(()) diff --git a/src-tauri/src/vault/operations.rs b/src-tauri/src/vault/operations.rs index 0c10fa4..31bbaf1 100644 --- a/src-tauri/src/vault/operations.rs +++ b/src-tauri/src/vault/operations.rs @@ -17,6 +17,101 @@ pub fn helixnotes_dir(vault_path: &str) -> PathBuf { Path::new(vault_path).join(".helixnotes") } +fn canonicalize_path(path: &Path, label: &str) -> Result { + fs::canonicalize(path).map_err(|error| format!("Invalid {label}: {error}")) +} + +fn ensure_vault_content_path( + vault_path: &str, + requested_path: &Path, + allow_root: bool, +) -> Result { + let vault = canonicalize_path(Path::new(vault_path), "vault path")?; + let requested = canonicalize_path(requested_path, "vault item path")?; + let metadata = vault.join(".helixnotes"); + + if !requested.starts_with(&vault) + || requested.starts_with(&metadata) + || (!allow_root && requested == vault) + { + return Err("Path must stay inside the active vault".to_string()); + } + + Ok(requested_path.to_path_buf()) +} + +fn ensure_vault_content_dir(vault_path: &str, requested_path: &Path) -> Result { + let requested = ensure_vault_content_path(vault_path, requested_path, true)?; + if !requested.is_dir() { + return Err("Vault destination is not a directory".to_string()); + } + Ok(requested) +} + +fn ensure_note_path(vault_path: &str, requested_path: &Path) -> Result { + let requested = ensure_vault_content_path(vault_path, requested_path, false)?; + if !requested.is_file() + || requested.extension().and_then(|extension| extension.to_str()) != Some("md") + { + return Err("Note path must point to a Markdown file".to_string()); + } + Ok(requested) +} + +fn ensure_readable_note_path(vault_path: &str, requested_path: &Path) -> Result { + if let Ok(note) = ensure_note_path(vault_path, requested_path) { + return Ok(note); + } + + let trashed_note = ensure_trash_entry(vault_path, requested_path)?; + if !trashed_note.is_file() + || trashed_note.extension().and_then(|extension| extension.to_str()) != Some("md") + { + return Err("Note path must point to a Markdown file".to_string()); + } + Ok(trashed_note) +} + +fn ensure_notebook_path(vault_path: &str, requested_path: &Path) -> Result { + let requested = ensure_vault_content_path(vault_path, requested_path, false)?; + if !requested.is_dir() { + return Err("Notebook path must point to a directory".to_string()); + } + Ok(requested) +} + +fn ensure_trash_entry(vault_path: &str, requested_path: &Path) -> Result { + let trash = canonicalize_path(&helixnotes_dir(vault_path).join("trash"), "trash path")?; + let requested = canonicalize_path(requested_path, "trash item path")?; + if requested == trash || !requested.starts_with(&trash) { + return Err("Path must be an item inside the active vault trash".to_string()); + } + Ok(requested_path.to_path_buf()) +} + +fn safe_relative_path(path: &str) -> Result<&Path, String> { + let relative = Path::new(path); + if relative.as_os_str().is_empty() + || !relative + .components() + .all(|component| matches!(component, Component::Normal(_) | Component::CurDir)) + { + return Err("Path must be a safe vault-relative path".to_string()); + } + Ok(relative) +} + +fn safe_child_name(name: &str) -> Result<&str, String> { + let mut components = Path::new(name).components(); + if name.trim().is_empty() + || !matches!(components.next(), Some(Component::Normal(_))) + || components.next().is_some() + { + return Err("Name must not contain path separators".to_string()); + } + Ok(name) +} + pub fn ensure_vault_structure(vault_path: &str) -> Result<(), String> { let hn_dir = helixnotes_dir(vault_path); fs::create_dir_all(hn_dir.join("trash")).map_err(|e| e.to_string())?; @@ -243,6 +338,7 @@ pub fn count_root_notes(vault_path: &str) -> Result { pub fn scan_notes(vault_path: &str, notebook_path: Option<&str>) -> Result, String> { let scan_path = notebook_path.unwrap_or(vault_path); let root = Path::new(scan_path); + ensure_vault_content_dir(vault_path, root)?; let vault_root = Path::new(vault_path); log::info!("scan_notes: vault={}, scan={}, exists={}", vault_path, scan_path, root.exists()); @@ -423,8 +519,18 @@ fn read_note_entry_from_str( }) } -pub fn read_note(path: &str) -> Result { - let p = Path::new(path); +pub fn read_note(vault_path: &str, path: &str) -> Result { + let validated = ensure_readable_note_path(vault_path, Path::new(path))?; + read_note_content(&validated, path) +} + +pub fn read_vault_note(vault_path: &str, path: &str) -> Result { + let validated = ensure_note_path(vault_path, Path::new(path))?; + read_note_content(&validated, path) +} + +fn read_note_content(validated: &Path, reported_path: &str) -> Result { + let p = validated; let raw = fs::read_to_string(p).map_err(|e| e.to_string())?; let filename = p .file_name() @@ -454,14 +560,15 @@ pub fn read_note(path: &str) -> Result { } Ok(NoteContent { - path: path.to_string(), + path: reported_path.to_string(), meta, content, raw, }) } -pub fn save_note(path: &str, meta: &NoteMeta, body: &str) -> Result<(), String> { +pub fn save_note(vault_path: &str, path: &str, meta: &NoteMeta, body: &str) -> Result<(), String> { + let path = ensure_note_path(vault_path, Path::new(path))?; let mut updated_meta = meta.clone(); updated_meta.modified = Utc::now(); @@ -471,7 +578,7 @@ pub fn save_note(path: &str, meta: &NoteMeta, body: &str) -> Result<(), String> } // Read existing file to preserve unknown frontmatter fields - let existing = fs::read_to_string(path).unwrap_or_default(); + let existing = fs::read_to_string(&path).unwrap_or_default(); let raw = if existing.is_empty() { frontmatter::update_note_raw(&updated_meta, body) } else { @@ -487,14 +594,11 @@ pub fn create_note( notebook_relative: Option<&str>, title: &str, ) -> Result { - let dir = match notebook_relative { - Some(rel) => Path::new(vault_path).join(rel), + let requested_dir = match notebook_relative { + Some(rel) => Path::new(vault_path).join(safe_relative_path(rel)?), None => PathBuf::from(vault_path), }; - - if !dir.exists() { - return Err("Notebook directory does not exist".to_string()); - } + let dir = ensure_vault_content_dir(vault_path, &requested_dir)?; let filename = sanitize_filename(title); let mut file_path = dir.join(format!("{}.md", filename)); @@ -535,10 +639,8 @@ pub fn create_note( } pub fn duplicate_note(path: &str, vault_path: &str) -> Result { - let src = Path::new(path); - if !src.is_file() { - return Err("Note does not exist".to_string()); - } + let validated = ensure_note_path(vault_path, Path::new(path))?; + let src = validated.as_path(); let parent = src .parent() @@ -719,12 +821,12 @@ pub fn create_notebook( parent_relative: Option<&str>, name: &str, ) -> Result { - let parent = match parent_relative { - Some(rel) => Path::new(vault_path).join(rel), + let requested_parent = match parent_relative { + Some(rel) => Path::new(vault_path).join(safe_relative_path(rel)?), None => PathBuf::from(vault_path), }; - - let dir_path = parent.join(name); + let parent = ensure_vault_content_dir(vault_path, &requested_parent)?; + let dir_path = parent.join(safe_child_name(name)?); if dir_path.exists() { return Err("Notebook already exists".to_string()); } @@ -748,10 +850,8 @@ pub fn create_notebook( } pub fn delete_note(vault_path: &str, note_path: &str) -> Result<(), String> { - let src = Path::new(note_path); - if !src.exists() { - return Err("Note does not exist".to_string()); - } + let validated = ensure_note_path(vault_path, Path::new(note_path))?; + let src = validated.as_path(); let trash_dir = helixnotes_dir(vault_path).join("trash"); fs::create_dir_all(&trash_dir).map_err(|e| e.to_string())?; @@ -771,10 +871,8 @@ pub fn delete_note(vault_path: &str, note_path: &str) -> Result<(), String> { } pub fn delete_notebook(vault_path: &str, notebook_path: &str) -> Result<(), String> { - let src = Path::new(notebook_path); - if !src.exists() { - return Err("Notebook does not exist".to_string()); - } + let validated = ensure_notebook_path(vault_path, Path::new(notebook_path))?; + let src = validated.as_path(); let trash_dir = helixnotes_dir(vault_path).join("trash"); fs::create_dir_all(&trash_dir).map_err(|e| e.to_string())?; @@ -803,10 +901,8 @@ pub fn delete_notebook(vault_path: &str, notebook_path: &str) -> Result<(), Stri } pub fn rename_note(path: &str, new_title: &str, vault_path: &str) -> Result { - let src = Path::new(path); - if !src.exists() { - return Err("Note does not exist".to_string()); - } + let validated = ensure_note_path(vault_path, Path::new(path))?; + let src = validated.as_path(); // Read old title before renaming let raw = fs::read_to_string(src).map_err(|e| e.to_string())?; @@ -966,13 +1062,11 @@ fn update_wikilinks_after_rename( } } -pub fn rename_notebook(path: &str, new_name: &str) -> Result { - let src = Path::new(path); - if !src.exists() { - return Err("Notebook does not exist".to_string()); - } +pub fn rename_notebook(vault_path: &str, path: &str, new_name: &str) -> Result { + let validated = ensure_notebook_path(vault_path, Path::new(path))?; + let src = validated.as_path(); - let new_path = src.parent().unwrap().join(new_name); + let new_path = src.parent().unwrap().join(safe_child_name(new_name)?); if new_path.exists() { return Err("A notebook with that name already exists".to_string()); } @@ -981,13 +1075,12 @@ pub fn rename_notebook(path: &str, new_name: &str) -> Result { Ok(new_path.to_string_lossy().to_string()) } -pub fn move_note(note_path: &str, dest_notebook: &str) -> Result { - let src = Path::new(note_path); - if !src.exists() { - return Err("Note does not exist".to_string()); - } +pub fn move_note(vault_path: &str, note_path: &str, dest_notebook: &str) -> Result { + let validated = ensure_note_path(vault_path, Path::new(note_path))?; + let src = validated.as_path(); - let dest_dir = Path::new(dest_notebook); + let validated_dest = ensure_vault_content_dir(vault_path, Path::new(dest_notebook))?; + let dest_dir = validated_dest.as_path(); if !dest_dir.is_dir() { return Err("Destination notebook does not exist".to_string()); } @@ -999,13 +1092,16 @@ pub fn move_note(note_path: &str, dest_notebook: &str) -> Result Ok(dest.to_string_lossy().to_string()) } -pub fn move_notebook(notebook_path: &str, dest_parent: &str) -> Result { - let src = Path::new(notebook_path); - if !src.exists() || !src.is_dir() { - return Err("Notebook does not exist".to_string()); - } +pub fn move_notebook( + vault_path: &str, + notebook_path: &str, + dest_parent: &str, +) -> Result { + let validated = ensure_notebook_path(vault_path, Path::new(notebook_path))?; + let src = validated.as_path(); - let dest_parent_path = Path::new(dest_parent); + let validated_dest = ensure_vault_content_dir(vault_path, Path::new(dest_parent))?; + let dest_parent_path = validated_dest.as_path(); if !dest_parent_path.is_dir() { return Err("Destination does not exist".to_string()); } @@ -1101,15 +1197,17 @@ pub fn restore_note( trash_path: &str, dest_notebook: Option<&str>, ) -> Result { - let src = Path::new(trash_path); - if !src.exists() { + let validated = ensure_trash_entry(vault_path, Path::new(trash_path))?; + let src = validated.as_path(); + if !src.is_file() { return Err("Trashed note does not exist".to_string()); } - let dest_dir = match dest_notebook { + let requested_dest = match dest_notebook { Some(nb) => PathBuf::from(nb), None => PathBuf::from(vault_path), }; + let dest_dir = ensure_vault_content_dir(vault_path, &requested_dest)?; // Strip timestamp prefix from trash filename (17-char with millis or 14-char legacy) let filename = src.file_name().unwrap_or_default().to_string_lossy(); @@ -1132,8 +1230,9 @@ pub fn restore_note( } pub fn restore_notebook(vault_path: &str, trash_path: &str) -> Result { - let src = Path::new(trash_path); - if !src.exists() || !src.is_dir() { + let validated = ensure_trash_entry(vault_path, Path::new(trash_path))?; + let src = validated.as_path(); + if !src.is_dir() { return Err("Trashed notebook does not exist".to_string()); } @@ -1156,7 +1255,7 @@ pub fn restore_notebook(vault_path: &str, trash_path: &str) -> Result Result Result<(), String> { - let p = Path::new(path); + let validated = ensure_trash_entry(vault_path, Path::new(path))?; + let p = validated.as_path(); let parent = p.parent().map(|pp| pp.to_path_buf()); if p.is_dir() { fs::remove_dir_all(p).map_err(|e| e.to_string())?; @@ -1431,8 +1531,9 @@ pub fn sanitize_filename(name: &str) -> String { #[cfg(test)] mod tests { use super::{ - compare_natural_names, duplicate_note, get_note_switcher_titles, helixnotes_dir, - load_notebook_icons, scan_notebooks, set_notebook_icon, + compare_natural_names, create_notebook, duplicate_note, get_note_switcher_titles, + helixnotes_dir, load_notebook_icons, permanent_delete, read_note, restore_notebook, + scan_notebooks, set_notebook_icon, }; use std::fs; use uuid::Uuid; @@ -1592,6 +1693,75 @@ mod tests { fs::remove_file(outside).unwrap(); } + #[test] + fn reads_markdown_notes_from_trash_without_allowing_external_files() { + let test_root = + std::env::temp_dir().join(format!("helixnotes-path-security-test-{}", Uuid::new_v4())); + let vault = test_root.join("vault"); + let trash = helixnotes_dir(&vault.to_string_lossy()).join("trash"); + let trashed_note = trash.join("20240101000000000_Note.md"); + let outside = test_root.join("outside.md"); + fs::create_dir_all(&trash).unwrap(); + fs::write(&trashed_note, "---\ntitle: Note\n---\n\ntrashed").unwrap(); + fs::write(&outside, "outside").unwrap(); + + assert!(read_note(&vault.to_string_lossy(), &trashed_note.to_string_lossy()).is_ok()); + assert!(read_note(&vault.to_string_lossy(), &outside.to_string_lossy()).is_err()); + + fs::remove_dir_all(test_root).unwrap(); + } + + #[test] + fn rejects_permanent_deletion_outside_trash() { + let test_root = + std::env::temp_dir().join(format!("helixnotes-path-security-test-{}", Uuid::new_v4())); + let vault = test_root.join("vault"); + let outside = test_root.join("outside.md"); + fs::create_dir_all(helixnotes_dir(&vault.to_string_lossy()).join("trash")).unwrap(); + fs::write(&outside, "must survive").unwrap(); + + let result = permanent_delete(&vault.to_string_lossy(), &outside.to_string_lossy()); + + assert!(result.is_err()); + assert_eq!(fs::read_to_string(&outside).unwrap(), "must survive"); + fs::remove_dir_all(test_root).unwrap(); + } + + #[test] + fn rejects_notebook_creation_outside_vault() { + let test_root = + std::env::temp_dir().join(format!("helixnotes-path-security-test-{}", Uuid::new_v4())); + let vault = test_root.join("vault"); + fs::create_dir_all(&vault).unwrap(); + + let result = create_notebook(&vault.to_string_lossy(), Some(".."), "escaped"); + + assert!(result.is_err()); + assert!(!test_root.join("escaped").exists()); + fs::remove_dir_all(test_root).unwrap(); + } + + #[test] + fn rejects_traversal_in_restored_notebook_metadata() { + let test_root = + std::env::temp_dir().join(format!("helixnotes-path-security-test-{}", Uuid::new_v4())); + let vault = test_root.join("vault"); + let trash = helixnotes_dir(&vault.to_string_lossy()).join("trash"); + let trashed_notebook = trash.join("20240101000000000_Notebook"); + fs::create_dir_all(&trashed_notebook).unwrap(); + fs::write(trash.join("20240101000000000_Notebook.meta"), "../escaped").unwrap(); + + let result = restore_notebook( + &vault.to_string_lossy(), + &trashed_notebook.to_string_lossy(), + ); + + assert!(result.is_err()); + assert!(trashed_notebook.exists()); + assert!(!test_root.join("escaped").exists()); + fs::remove_dir_all(test_root).unwrap(); + } + #[test] fn duplicates_note_content_and_assigns_unique_identity_and_name() { let vault =