From 68f1bff93bdca47eae6e1e38edcd9f0de8b3e9eb Mon Sep 17 00:00:00 2001 From: Tristan Michael Date: Thu, 2 Apr 2026 08:23:07 -0700 Subject: [PATCH 01/11] fix: prevent path traversal, enable CSP, and harden URL domain extraction Validate that resolved list paths stay within the workspace root to prevent directory traversal via malicious list names. Enable Content Security Policy in Tauri config instead of leaving it null. Fix CLI domain extraction to strip userinfo (user:pass@) from URLs before using as keyring service name. --- apps/tauri/src-tauri/tauri.conf.json | 2 +- crates/onyx-cli/src/commands/sync.rs | 24 ++++++++++++---------- crates/onyx-core/src/storage.rs | 30 ++++++++++++++++++++++++---- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/apps/tauri/src-tauri/tauri.conf.json b/apps/tauri/src-tauri/tauri.conf.json index 1613341..83a6a4b 100644 --- a/apps/tauri/src-tauri/tauri.conf.json +++ b/apps/tauri/src-tauri/tauri.conf.json @@ -23,7 +23,7 @@ } ], "security": { - "csp": null + "csp": "default-src 'self'; style-src 'self' 'unsafe-inline'; font-src 'self' https://fonts.gstatic.com; connect-src ipc: http://ipc.localhost" } }, "bundle": { diff --git a/crates/onyx-cli/src/commands/sync.rs b/crates/onyx-cli/src/commands/sync.rs index 234062d..a20d0c2 100644 --- a/crates/onyx-cli/src/commands/sync.rs +++ b/crates/onyx-cli/src/commands/sync.rs @@ -204,18 +204,20 @@ fn print_workspace_status(name: &str, path: &std::path::Path, webdav_url: Option Ok(()) } -/// Extract domain from a URL for credential storage. +/// Extract host from a URL for credential storage. fn extract_domain(url: &str) -> String { - url.split("://") - .nth(1) - .unwrap_or(url) - .split('/') - .next() - .unwrap_or(url) - .split(':') - .next() - .unwrap_or(url) - .to_string() + // Strip scheme + let after_scheme = url.split("://").nth(1).unwrap_or(url); + // Strip path + let authority = after_scheme.split('/').next().unwrap_or(after_scheme); + // Strip userinfo (user:pass@host) + let host_port = if let Some(at_pos) = authority.rfind('@') { + &authority[at_pos + 1..] + } else { + authority + }; + // Strip port + host_port.split(':').next().unwrap_or(host_port).to_string() } /// Prompt the user for text input. diff --git a/crates/onyx-core/src/storage.rs b/crates/onyx-core/src/storage.rs index 62bb4a2..9a41991 100644 --- a/crates/onyx-core/src/storage.rs +++ b/crates/onyx-core/src/storage.rs @@ -149,8 +149,30 @@ impl FileSystemStorage { Err(Error::ListNotFound(list_id.to_string())) } - fn list_dir_path_by_name(&self, name: &str) -> PathBuf { - self.root_path.join(name) + fn list_dir_path_by_name(&self, name: &str) -> Result { + let path = self.root_path.join(name); + // Prevent path traversal: resolved path must stay within root + let canonical_root = self.root_path.canonicalize() + .unwrap_or_else(|_| self.root_path.clone()); + let canonical_path = if path.exists() { + path.canonicalize().unwrap_or_else(|_| path.clone()) + } else { + // For non-existent paths, normalize by resolving the parent + if let Some(parent) = path.parent() { + let canonical_parent = if parent.exists() { + parent.canonicalize().unwrap_or_else(|_| parent.to_path_buf()) + } else { + parent.to_path_buf() + }; + canonical_parent.join(path.file_name().unwrap_or_default()) + } else { + path.clone() + } + }; + if !canonical_path.starts_with(&canonical_root) { + return Err(Error::InvalidData(format!("Invalid list name: path escapes workspace"))); + } + Ok(path) } fn sanitize_filename(name: &str) -> String { @@ -371,7 +393,7 @@ impl Storage for FileSystemStorage { } fn create_list(&mut self, name: String) -> Result { - let list_dir = self.list_dir_path_by_name(&name); + let list_dir = self.list_dir_path_by_name(&name)?; if list_dir.exists() { return Err(Error::InvalidData(format!("List '{}' already exists", name))); @@ -473,7 +495,7 @@ impl Storage for FileSystemStorage { fn rename_list(&mut self, list_id: Uuid, new_name: String) -> Result<()> { let old_dir = self.list_dir_path(list_id)?; - let new_dir = self.list_dir_path_by_name(&new_name); + let new_dir = self.list_dir_path_by_name(&new_name)?; if new_dir.exists() { return Err(Error::InvalidData(format!("A list named '{}' already exists", new_name))); From 3b2cb12272f1593aba9c1964fd41157cd0053bc5 Mon Sep 17 00:00:00 2001 From: Tristan Michael Date: Thu, 2 Apr 2026 08:23:20 -0700 Subject: [PATCH 02/11] fix: replace panicking unwrap/expect calls with graceful error handling Replace .expect() in config path resolution with a fallback default. Replace all .lock().unwrap() on Tauri AppState mutex with a helper that converts poisoned locks into error strings. Replace .expect() on Android app data dir with proper error propagation. Handle LAST_WRITE and WATCHER global mutex locks gracefully. Propagate subtask cascade delete errors instead of silently ignoring them. --- apps/tauri/src-tauri/src/lib.rs | 69 ++++++++++++++++++++------------- crates/onyx-core/src/config.rs | 6 +-- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/apps/tauri/src-tauri/src/lib.rs b/apps/tauri/src-tauri/src/lib.rs index dc02145..b94fd14 100644 --- a/apps/tauri/src-tauri/src/lib.rs +++ b/apps/tauri/src-tauri/src/lib.rs @@ -34,6 +34,11 @@ struct AppState { repo: Option, } +/// Lock the AppState mutex, converting poisoned locks into an error string. +fn lock_state(state: &Mutex) -> Result, String> { + state.lock().map_err(|e| format!("State lock poisoned: {}", e)) +} + /// Serializable sync result for the frontend. #[derive(Debug, Serialize, Deserialize, Clone)] struct SyncResult { @@ -61,7 +66,9 @@ impl From for SyncResult { /// Suppress file watcher events for the next second (call before writes). #[cfg(not(target_os = "android"))] fn mute_watcher(_state: &mut AppState) { - *LAST_WRITE.lock().unwrap() = Some(Instant::now()); + if let Ok(mut t) = LAST_WRITE.lock() { + *t = Some(Instant::now()); + } } #[cfg(target_os = "android")] @@ -85,13 +92,13 @@ fn ensure_repo(state: &mut AppState) -> Result<(), String> { #[tauri::command] fn get_config(state: State<'_, Mutex>) -> Result { - let s = state.lock().unwrap(); + let s = lock_state(&state)?; Ok(s.config.clone()) } #[tauri::command] fn save_config(state: State<'_, Mutex>) -> Result<(), String> { - let s = state.lock().unwrap(); + let s = lock_state(&state)?; s.config.save_to_file(&s.config_path).map_err(|e| e.to_string()) } @@ -101,7 +108,7 @@ fn add_workspace( path: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; let ws = WorkspaceConfig::new(PathBuf::from(&path)); s.config.add_workspace(name.clone(), ws); s.config @@ -119,7 +126,7 @@ fn set_current_workspace( name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; s.config .set_current_workspace(name) .map_err(|e| e.to_string())?; @@ -134,7 +141,7 @@ fn remove_workspace( name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; s.config.remove_workspace(&name); s.repo = None; s.config @@ -155,7 +162,7 @@ fn init_workspace(path: String) -> Result<(), String> { #[tauri::command] fn get_lists(state: State<'_, Mutex>) -> Result, String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; s.repo .as_ref() @@ -169,7 +176,7 @@ fn create_list( name: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); s.repo @@ -184,7 +191,7 @@ fn delete_list( list_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -202,7 +209,7 @@ fn list_tasks( list_id: String, state: State<'_, Mutex>, ) -> Result, String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; s.repo @@ -220,7 +227,7 @@ fn create_task( parent_id: Option, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -245,7 +252,7 @@ fn update_task( task: Task, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -262,7 +269,7 @@ fn delete_task( task_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -276,7 +283,7 @@ fn delete_task( .map(|t| t.id) .collect(); for child_id in child_ids { - let _ = repo.delete_task(lid, child_id); + repo.delete_task(lid, child_id).map_err(|e| format!("Failed to delete subtask {}: {}", child_id, e))?; } repo.delete_task(lid, tid) .map_err(|e| e.to_string()) @@ -288,7 +295,7 @@ fn toggle_task( task_id: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -322,7 +329,7 @@ fn reorder_task( new_position: usize, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let lid = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -343,7 +350,7 @@ fn move_task( task_id: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let from = Uuid::parse_str(&from_list_id).map_err(|e| e.to_string())?; @@ -362,7 +369,7 @@ fn rename_list( new_name: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -379,7 +386,7 @@ fn set_group_by_due_date( enabled: bool, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; mute_watcher(&mut s); let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; @@ -395,7 +402,7 @@ fn get_group_by_due_date( list_id: String, state: State<'_, Mutex>, ) -> Result { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; ensure_repo(&mut s)?; let id = Uuid::parse_str(&list_id).map_err(|e| e.to_string())?; s.repo @@ -413,7 +420,7 @@ fn set_webdav_config( webdav_url: String, state: State<'_, Mutex>, ) -> Result<(), String> { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) { ws.webdav_url = Some(webdav_url); } @@ -477,7 +484,7 @@ async fn sync_workspace( // Persist last_sync timestamp to config { - let mut s = state.lock().unwrap(); + let mut s = lock_state(&state)?; if let Some(ws) = s.config.workspaces.get_mut(&workspace_name) { ws.last_sync = Some(Utc::now()); } @@ -504,16 +511,22 @@ fn start_watcher(handle: tauri::AppHandle, path: PathBuf) { }); if !has_data_change { return; } // Skip if we wrote recently (self-change suppression) - if let Some(t) = *LAST_WRITE.lock().unwrap() { - if t.elapsed() < std::time::Duration::from_secs(1) { return; } + if let Ok(guard) = LAST_WRITE.lock() { + if let Some(t) = *guard { + if t.elapsed() < std::time::Duration::from_secs(1) { return; } + } } let _ = handle.emit("fs-changed", ()); }, ); match debouncer { Ok(mut d) => { - let _ = d.watcher().watch(&path, notify::RecursiveMode::Recursive); - *WATCHER.lock().unwrap() = Some(d); + if let Err(e) = d.watcher().watch(&path, notify::RecursiveMode::Recursive) { + eprintln!("Failed to watch path {}: {e}", path.display()); + } + if let Ok(mut w) = WATCHER.lock() { + *w = Some(d); + } } Err(e) => eprintln!("Failed to start file watcher: {e}"), } @@ -545,7 +558,9 @@ pub fn run() { #[cfg(target_os = "android")] { use tauri::Manager; - app.path().app_data_dir().expect("Failed to get app data dir").join("config.json") + app.path().app_data_dir() + .map_err(|e| format!("Failed to get app data dir: {}", e))? + .join("config.json") } #[cfg(not(target_os = "android"))] { diff --git a/crates/onyx-core/src/config.rs b/crates/onyx-core/src/config.rs index 9667fe3..b13a2bd 100644 --- a/crates/onyx-core/src/config.rs +++ b/crates/onyx-core/src/config.rs @@ -82,9 +82,9 @@ impl AppConfig { } pub fn get_config_path() -> PathBuf { - let config_dir = directories::ProjectDirs::from("", "", "onyx") - .expect("Failed to determine config directory"); - config_dir.config_dir().join("config.json") + directories::ProjectDirs::from("", "", "onyx") + .map(|dirs| dirs.config_dir().join("config.json")) + .unwrap_or_else(|| PathBuf::from("onyx-config.json")) } } From 056cd8ee49bad285e9d272be722de7f551ee2182 Mon Sep 17 00:00:00 2001 From: Tristan Michael Date: Thu, 2 Apr 2026 08:23:28 -0700 Subject: [PATCH 03/11] fix: harden sync file validation and offline queue corruption handling Restrict is_syncable() to validate path depth: .md files and .listdata.json must be at depth 2 (inside list dirs), .metadata.json only at depth 1 (root). Prevents syncing arbitrary files at unexpected depths. Back up corrupted sync queue files before resetting, and log warnings on parse failures instead of silently dropping queued operations. --- crates/onyx-core/src/sync.rs | 46 +++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/crates/onyx-core/src/sync.rs b/crates/onyx-core/src/sync.rs index 1ccbf38..167854c 100644 --- a/crates/onyx-core/src/sync.rs +++ b/crates/onyx-core/src/sync.rs @@ -258,8 +258,19 @@ impl OfflineQueue { return Self::default(); } match std::fs::read_to_string(&queue_path) { - Ok(content) => serde_json::from_str(&content).unwrap_or_default(), - Err(_) => Self::default(), + Ok(content) => match serde_json::from_str(&content) { + Ok(queue) => queue, + Err(e) => { + eprintln!("Warning: corrupt sync queue, backing up and resetting: {}", e); + let backup = workspace_path.join(".syncqueue.json.bak"); + let _ = std::fs::copy(&queue_path, &backup); + Self::default() + } + }, + Err(e) => { + eprintln!("Warning: failed to read sync queue: {}", e); + Self::default() + } } } @@ -338,12 +349,23 @@ pub fn compute_checksum(data: &[u8]) -> String { format!("{:x}", hasher.finalize()) } -/// Check if a filename is a syncable file (*.md, .listdata.json, .metadata.json). +/// Check if a file is syncable: *.md files and metadata files at expected depths. fn is_syncable(path: &str) -> bool { - let filename = path.rsplit('/').next().unwrap_or(path); - filename.ends_with(".md") - || filename == ".listdata.json" - || filename == ".metadata.json" + let parts: Vec<&str> = path.split('/').collect(); + let filename = parts.last().copied().unwrap_or(path); + // .metadata.json only at workspace root (depth 1) + if filename == ".metadata.json" { + return parts.len() == 1; + } + // .listdata.json only inside a list directory (depth 2) + if filename == ".listdata.json" { + return parts.len() == 2; + } + // .md files inside a list directory (depth 2) + if filename.ends_with(".md") { + return parts.len() == 2; + } + false } /// Scan local workspace files and compute checksums. @@ -1009,14 +1031,20 @@ mod tests { #[test] fn test_is_syncable() { - assert!(is_syncable("file.md")); + // .md files must be inside a list dir (depth 2) assert!(is_syncable("My Tasks/Buy groceries.md")); - assert!(is_syncable(".listdata.json")); + assert!(!is_syncable("file.md")); // root-level md not valid + // .listdata.json inside a list dir (depth 2) assert!(is_syncable("My Tasks/.listdata.json")); + assert!(!is_syncable(".listdata.json")); // root-level not valid + // .metadata.json only at root (depth 1) assert!(is_syncable(".metadata.json")); + assert!(!is_syncable("My Tasks/.metadata.json")); // nested not valid + // Non-syncable assert!(!is_syncable(".syncstate.json")); assert!(!is_syncable("random.txt")); assert!(!is_syncable("image.png")); + assert!(!is_syncable("a/b/c/deep.md")); // too deep } #[test] From 54836f14e76061b1b4c6cab74bc349c11186ef09 Mon Sep 17 00:00:00 2001 From: Tristan Michael Date: Thu, 2 Apr 2026 08:23:38 -0700 Subject: [PATCH 04/11] fix: improve frontend accessibility, consistency, and resource cleanup Fix nested interactive elements in TaskItem (button inside button) by restructuring as div + button with aria-label. Replace native confirm() with ConfirmDialog for workspace removal. Store fs-changed event unlisten function for cleanup. Log file watcher errors instead of swallowing them. Fix var usage to const. Add Firefox scrollbar-width support. --- apps/tauri/src/app.css | 7 +++++++ .../src/lib/components/TaskDetailView.svelte | 2 +- apps/tauri/src/lib/components/TaskItem.svelte | 13 +++++++------ apps/tauri/src/lib/screens/TasksScreen.svelte | 17 +++++++++++++++-- apps/tauri/src/lib/stores/app.svelte.ts | 8 ++++++-- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/apps/tauri/src/app.css b/apps/tauri/src/app.css index 3871f70..ec6380a 100644 --- a/apps/tauri/src/app.css +++ b/apps/tauri/src/app.css @@ -45,6 +45,13 @@ body { } /* Scrollbar styling */ +* { + scrollbar-width: thin; + scrollbar-color: #d1d5db transparent; +} +.dark * { + scrollbar-color: #4b5563 transparent; +} ::-webkit-scrollbar { width: 4px; } diff --git a/apps/tauri/src/lib/components/TaskDetailView.svelte b/apps/tauri/src/lib/components/TaskDetailView.svelte index 824b9ab..8392600 100644 --- a/apps/tauri/src/lib/components/TaskDetailView.svelte +++ b/apps/tauri/src/lib/components/TaskDetailView.svelte @@ -34,7 +34,7 @@ function debouncedSave(fields: Partial) { clearTimeout(saveTimer); - var snapshot = { ...task }; + const snapshot = { ...task }; saveTimer = setTimeout(() => { app.updateTask({ ...snapshot, ...fields, updated_at: new Date().toISOString() }); }, 400); diff --git a/apps/tauri/src/lib/components/TaskItem.svelte b/apps/tauri/src/lib/components/TaskItem.svelte index 520a2c7..bf922a7 100644 --- a/apps/tauri/src/lib/components/TaskItem.svelte +++ b/apps/tauri/src/lib/components/TaskItem.svelte @@ -106,15 +106,16 @@ {/if} -
@@ -160,7 +161,7 @@ - +
diff --git a/apps/tauri/src/lib/screens/TasksScreen.svelte b/apps/tauri/src/lib/screens/TasksScreen.svelte index 3e09e5d..6ed0665 100644 --- a/apps/tauri/src/lib/screens/TasksScreen.svelte +++ b/apps/tauri/src/lib/screens/TasksScreen.svelte @@ -44,7 +44,7 @@ showWorkspacePicker = false; if (showListMenu && listMenuEl && !listMenuEl.contains(e.target as Node)) showListMenu = false; - var target = e.target as HTMLElement; + const target = e.target as HTMLElement; if (wsMenuName && !target.closest("[data-ws-menu]")) wsMenuName = null; } @@ -58,6 +58,7 @@ let listMenuEl = $state(null); let confirmDeleteList = $state(false); let confirmDeleteCompleted = $state(false); + let confirmRemoveWorkspace = $state(null); let dragId = $state(null); let dragOverId = $state(null); let resizing = $state(false); @@ -270,7 +271,7 @@ {#if wsMenuName === name}