max / audiofiles
16 files changed,
+1017 insertions,
-97 deletions
| @@ -526,10 +526,34 @@ fn init_browser(data_dir: &Path, shared: Arc<SharedState>, vault_name: &str) -> | |||
| 526 | 526 | let sample_rate = shared.device_sample_rate.load(std::sync::atomic::Ordering::Relaxed) as f32; | |
| 527 | 527 | match BrowserState::new(data_dir, shared, sample_rate, vault_name) { | |
| 528 | 528 | Ok(mut browser) => { | |
| 529 | - | for arg in std::env::args().skip(1) { | |
| 530 | - | let path = PathBuf::from(&arg); | |
| 531 | - | if path.exists() { | |
| 532 | - | browser.import_path(&path); | |
| 529 | + | // CLI args / OS "Open with": route through the background import | |
| 530 | + | // worker, never the synchronous per-file path — the sync walk here | |
| 531 | + | // ran before the eframe loop started, so the window didn't appear | |
| 532 | + | // until the whole tree was hashed (fuzz-2026-07-06 B2). | |
| 533 | + | if let Some(vfs_id) = browser.current_vfs_id() { | |
| 534 | + | let parent_id = browser.nav.current_dir; | |
| 535 | + | let mut files = Vec::new(); | |
| 536 | + | for arg in std::env::args().skip(1) { | |
| 537 | + | let path = PathBuf::from(&arg); | |
| 538 | + | if !path.exists() { | |
| 539 | + | continue; | |
| 540 | + | } | |
| 541 | + | if path.is_dir() { | |
| 542 | + | let strategy = audiofiles_browser::import::ImportStrategy::MergeIntoVfs { | |
| 543 | + | vfs_id, | |
| 544 | + | parent_id, | |
| 545 | + | }; | |
| 546 | + | browser.start_folder_import(path, strategy); | |
| 547 | + | } else { | |
| 548 | + | files.push(path); | |
| 549 | + | } | |
| 550 | + | } | |
| 551 | + | if !files.is_empty() { | |
| 552 | + | let strategy = audiofiles_browser::import::ImportStrategy::MergeIntoVfs { | |
| 553 | + | vfs_id, | |
| 554 | + | parent_id, | |
| 555 | + | }; | |
| 556 | + | browser.start_files_import(files, strategy); | |
| 533 | 557 | } | |
| 534 | 558 | } | |
| 535 | 559 | (Some(browser), None) | |
| @@ -819,17 +843,30 @@ impl AudioFilesApp { | |||
| 819 | 843 | ||
| 820 | 844 | if let Some(ref mut browser) = self.browser { | |
| 821 | 845 | if let Some(vfs_id) = browser.current_vfs_id() { | |
| 846 | + | let parent_id = browser.nav.current_dir; | |
| 847 | + | // Loose files go to the worker as one batch — dropping hundreds | |
| 848 | + | // of files used to hash them synchronously on the frame thread | |
| 849 | + | // and freeze the window (fuzz-2026-07-06 B3). Directories keep | |
| 850 | + | // their structured (folder-preserving) import path. | |
| 851 | + | let mut files = Vec::new(); | |
| 822 | 852 | for path in dropped { | |
| 823 | 853 | if path.is_dir() { | |
| 824 | 854 | let strategy = audiofiles_browser::import::ImportStrategy::MergeIntoVfs { | |
| 825 | 855 | vfs_id, | |
| 826 | - | parent_id: browser.nav.current_dir, | |
| 856 | + | parent_id, | |
| 827 | 857 | }; | |
| 828 | 858 | browser.start_folder_import(path, strategy); | |
| 829 | 859 | } else { | |
| 830 | - | browser.import_path(&path); | |
| 860 | + | files.push(path); | |
| 831 | 861 | } | |
| 832 | 862 | } | |
| 863 | + | if !files.is_empty() { | |
| 864 | + | let strategy = audiofiles_browser::import::ImportStrategy::MergeIntoVfs { | |
| 865 | + | vfs_id, | |
| 866 | + | parent_id, | |
| 867 | + | }; | |
| 868 | + | browser.start_files_import(files, strategy); | |
| 869 | + | } | |
| 833 | 870 | } | |
| 834 | 871 | audiofiles_browser::editor::draw_browser(ui, browser, self.sync_manager.as_ref()); | |
| 835 | 872 |
| @@ -21,6 +21,17 @@ const CHECK_INTERVAL_SECS: u64 = 6 * 60 * 60; | |||
| 21 | 21 | /// Current app version (from Cargo.toml at compile time). | |
| 22 | 22 | const CURRENT_VERSION: &str = env!("CARGO_PKG_VERSION"); | |
| 23 | 23 | ||
| 24 | + | /// Hard cap on the OTA manifest body. The manifest is a tiny JSON object | |
| 25 | + | /// (version + url + notes); anything larger is a misbehaving or compromised | |
| 26 | + | /// endpoint. Without this, a 200 from an allowlisted host (or an auto-followed | |
| 27 | + | /// redirect) could stream unbounded bytes into memory — the 15 s request | |
| 28 | + | /// timeout bounds *time*, not *size*. | |
| 29 | + | const MAX_UPDATE_BODY: usize = 64 * 1024; | |
| 30 | + | ||
| 31 | + | /// Cap on the release-notes string we retain and later render. Bounds what a | |
| 32 | + | /// compromised endpoint can push into the UI even within a well-formed manifest. | |
| 33 | + | const MAX_NOTES_LEN: usize = 4096; | |
| 34 | + | ||
| 24 | 35 | /// The response format from the MNW OTA updater endpoint. | |
| 25 | 36 | #[derive(serde::Deserialize)] | |
| 26 | 37 | struct UpdateResponse { | |
| @@ -29,6 +40,36 @@ struct UpdateResponse { | |||
| 29 | 40 | notes: String, | |
| 30 | 41 | } | |
| 31 | 42 | ||
| 43 | + | /// Read a response body into memory, refusing to buffer more than `cap` bytes. | |
| 44 | + | /// Rejects up front on an advertised `Content-Length` over the cap, then streams | |
| 45 | + | /// chunk-by-chunk and aborts the moment the accumulated size would exceed it — | |
| 46 | + | /// so a chunked/streamed response with no (or a lying) length header can't grow | |
| 47 | + | /// the buffer without bound. | |
| 48 | + | async fn read_capped_body(resp: reqwest::Response, cap: usize) -> Result<Vec<u8>, String> { | |
| 49 | + | if let Some(len) = resp.content_length() | |
| 50 | + | && len > cap as u64 | |
| 51 | + | { | |
| 52 | + | return Err(format!("body advertises {len} bytes (cap {cap})")); | |
| 53 | + | } | |
| 54 | + | let mut resp = resp; | |
| 55 | + | let mut buf = Vec::new(); | |
| 56 | + | while let Some(chunk) = resp.chunk().await.map_err(|e| e.to_string())? { | |
| 57 | + | if buf.len() + chunk.len() > cap { | |
| 58 | + | return Err(format!("body exceeded {cap} bytes")); | |
| 59 | + | } | |
| 60 | + | buf.extend_from_slice(&chunk); | |
| 61 | + | } | |
| 62 | + | Ok(buf) | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | /// Truncate `s` in place to at most `max` characters, respecting UTF-8 | |
| 66 | + | /// boundaries (never mid-code-point). | |
| 67 | + | fn truncate_chars(s: &mut String, max: usize) { | |
| 68 | + | if let Some((idx, _)) = s.char_indices().nth(max) { | |
| 69 | + | s.truncate(idx); | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | ||
| 32 | 73 | /// Shared update status, polled by the UI each frame. | |
| 33 | 74 | #[derive(Clone, Default)] | |
| 34 | 75 | pub struct UpdateStatus { | |
| @@ -194,20 +235,26 @@ async fn check_once(status: &Arc<Mutex<UpdateStatus>>) { | |||
| 194 | 235 | tracing::info!("audiofiles is up to date (v{CURRENT_VERSION})"); | |
| 195 | 236 | } | |
| 196 | 237 | Ok(resp) if resp.status().is_success() => { | |
| 197 | - | match resp.json::<UpdateResponse>().await { | |
| 198 | - | Ok(update) => { | |
| 199 | - | if let Ok(remote) = Version::parse(&update.version) | |
| 200 | - | && remote > current && is_trusted_download_url(&update.url) { | |
| 201 | - | tracing::info!("Update available: v{}", update.version); | |
| 202 | - | let mut s = status.lock(); | |
| 203 | - | s.available = true; | |
| 204 | - | s.version = update.version; | |
| 205 | - | s.notes = update.notes; | |
| 206 | - | s.download_url = update.url; | |
| 207 | - | } | |
| 208 | - | } | |
| 238 | + | match read_capped_body(resp, MAX_UPDATE_BODY).await { | |
| 239 | + | Ok(body) => match serde_json::from_slice::<UpdateResponse>(&body) { | |
| 240 | + | Ok(mut update) => { | |
| 241 | + | if let Ok(remote) = Version::parse(&update.version) | |
| 242 | + | && remote > current && is_trusted_download_url(&update.url) { | |
| 243 | + | tracing::info!("Update available: v{}", update.version); | |
| 244 | + | truncate_chars(&mut update.notes, MAX_NOTES_LEN); | |
| 245 | + | let mut s = status.lock(); | |
| 246 | + | s.available = true; | |
| 247 | + | s.version = update.version; | |
| 248 | + | s.notes = update.notes; | |
| 249 | + | s.download_url = update.url; | |
| 250 | + | } | |
| 251 | + | } | |
| 252 | + | Err(e) => { | |
| 253 | + | tracing::warn!("Failed to parse update response: {e}"); | |
| 254 | + | } | |
| 255 | + | }, | |
| 209 | 256 | Err(e) => { | |
| 210 | - | tracing::warn!("Failed to parse update response: {e}"); | |
| 257 | + | tracing::warn!("Update manifest rejected: {e}"); | |
| 211 | 258 | } | |
| 212 | 259 | } | |
| 213 | 260 | } | |
| @@ -363,6 +410,24 @@ mod tests { | |||
| 363 | 410 | } | |
| 364 | 411 | ||
| 365 | 412 | #[test] | |
| 413 | + | fn truncate_chars_caps_length_on_char_boundary() { | |
| 414 | + | let mut s = "hello world".to_string(); | |
| 415 | + | truncate_chars(&mut s, 5); | |
| 416 | + | assert_eq!(s, "hello"); | |
| 417 | + | ||
| 418 | + | // Under the cap: unchanged. | |
| 419 | + | let mut short = "hi".to_string(); | |
| 420 | + | truncate_chars(&mut short, 5); | |
| 421 | + | assert_eq!(short, "hi"); | |
| 422 | + | ||
| 423 | + | // Multi-byte code points: never split mid-char. | |
| 424 | + | let mut emoji = "a\u{1f600}b\u{1f600}c".to_string(); // a😀b😀c | |
| 425 | + | truncate_chars(&mut emoji, 2); | |
| 426 | + | assert_eq!(emoji, "a\u{1f600}"); | |
| 427 | + | assert!(emoji.is_char_boundary(emoji.len())); | |
| 428 | + | } | |
| 429 | + | ||
| 430 | + | #[test] | |
| 366 | 431 | fn current_version_is_valid_semver() { | |
| 367 | 432 | Version::parse(CURRENT_VERSION) | |
| 368 | 433 | .expect("CURRENT_VERSION should be valid semver"); |
| @@ -92,6 +92,20 @@ fn forge_import_chop( | |||
| 92 | 92 | } | |
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | + | /// Convert a GUI-facing [`ImportStrategyDesc`] into the worker's [`ImportStrategy`]. | |
| 96 | + | /// Shared by the directory and explicit-file import entry points. | |
| 97 | + | fn resolve_import_strategy(strategy: ImportStrategyDesc) -> ImportStrategy { | |
| 98 | + | match strategy { | |
| 99 | + | ImportStrategyDesc::Flat { vfs_id, parent_id } => { | |
| 100 | + | ImportStrategy::Flat { vfs_id, parent_id } | |
| 101 | + | } | |
| 102 | + | ImportStrategyDesc::NewVfs { vfs_name } => ImportStrategy::NewVfs { vfs_name }, | |
| 103 | + | ImportStrategyDesc::MergeIntoVfs { vfs_id, parent_id } => { | |
| 104 | + | ImportStrategy::MergeIntoVfs { vfs_id, parent_id } | |
| 105 | + | } | |
| 106 | + | } | |
| 107 | + | } | |
| 108 | + | ||
| 95 | 109 | /// Off-thread counterpart for conform import (see [`forge_import_chop`]). Maps the | |
| 96 | 110 | /// core overshoot result into the GUI-facing `ForgeOvershoot` here so the outcome | |
| 97 | 111 | /// is ready to emit. | |
| @@ -216,9 +230,13 @@ impl DirectBackend { | |||
| 216 | 230 | let mut guard = self.loose_files_worker.lock(); | |
| 217 | 231 | if guard.is_none() { | |
| 218 | 232 | let db_path = self.data_dir.join("audiofiles.db"); | |
| 219 | - | let handle = crate::loose_files_worker::spawn_loose_files_worker(db_path).map_err( | |
| 220 | - | |e| BackendError::Other(format!("failed to spawn loose-files worker: {e}")), | |
| 221 | - | )?; | |
| 233 | + | let handle = crate::loose_files_worker::spawn_loose_files_worker( | |
| 234 | + | db_path, | |
| 235 | + | self.store.root().to_path_buf(), | |
| 236 | + | ) | |
| 237 | + | .map_err(|e| { | |
| 238 | + | BackendError::Other(format!("failed to spawn loose-files worker: {e}")) | |
| 239 | + | })?; | |
| 222 | 240 | *guard = Some(handle); | |
| 223 | 241 | } | |
| 224 | 242 | // If the cached worker died (e.g. its DB open failed inside the thread), | |
| @@ -234,6 +252,20 @@ impl DirectBackend { | |||
| 234 | 252 | Ok(()) | |
| 235 | 253 | } | |
| 236 | 254 | ||
| 255 | + | /// Cancel any in-flight import worker and spawn a fresh one, returning its | |
| 256 | + | /// handle (not yet stored). Shared by [`start_import`](Self::start_import) and | |
| 257 | + | /// [`start_file_import`](Self::start_file_import). | |
| 258 | + | fn respawn_import_worker(&self) -> BackendResult<ImportHandle> { | |
| 259 | + | let db_path = self.data_dir.join("audiofiles.db"); | |
| 260 | + | let store_root = self.store.root().to_path_buf(); | |
| 261 | + | if let Some(old) = self.import_worker.lock().take() { | |
| 262 | + | old.send(ImportCommand::Cancel); | |
| 263 | + | drop(old); // joins the thread | |
| 264 | + | } | |
| 265 | + | crate::import::spawn_import_worker(db_path, store_root) | |
| 266 | + | .map_err(|e| BackendError::Other(format!("failed to spawn import worker: {e}"))) | |
| 267 | + | } | |
| 268 | + | ||
| 237 | 269 | /// Lazily spawn the search worker on first use. It opens its own read-only | |
| 238 | 270 | /// connection to the database file (WAL allows concurrent reads). | |
| 239 | 271 | fn ensure_search_worker(&self) -> BackendResult<()> { | |
| @@ -1140,6 +1172,10 @@ impl Backend for DirectBackend { | |||
| 1140 | 1172 | self.loose_files_send(crate::loose_files_worker::LooseFilesCommand::Purge) | |
| 1141 | 1173 | } | |
| 1142 | 1174 | ||
| 1175 | + | fn start_store_verify(&self) -> BackendResult<()> { | |
| 1176 | + | self.loose_files_send(crate::loose_files_worker::LooseFilesCommand::VerifyStore) | |
| 1177 | + | } | |
| 1178 | + | ||
| 1143 | 1179 | // --- Export --- | |
| 1144 | 1180 | ||
| 1145 | 1181 | fn collect_export_items( | |
| @@ -1452,32 +1488,27 @@ impl Backend for DirectBackend { | |||
| 1452 | 1488 | source: &Path, | |
| 1453 | 1489 | strategy: ImportStrategyDesc, | |
| 1454 | 1490 | ) -> BackendResult<()> { | |
| 1455 | - | let db_path = self.data_dir.join("audiofiles.db"); | |
| 1456 | - | let store_root = self.store.root().to_path_buf(); | |
| 1457 | - | ||
| 1458 | - | let import_strategy = match strategy { | |
| 1459 | - | ImportStrategyDesc::Flat { vfs_id, parent_id } => { | |
| 1460 | - | ImportStrategy::Flat { vfs_id, parent_id } | |
| 1461 | - | } | |
| 1462 | - | ImportStrategyDesc::NewVfs { vfs_name } => ImportStrategy::NewVfs { vfs_name }, | |
| 1463 | - | ImportStrategyDesc::MergeIntoVfs { vfs_id, parent_id } => { | |
| 1464 | - | ImportStrategy::MergeIntoVfs { vfs_id, parent_id } | |
| 1465 | - | } | |
| 1466 | - | }; | |
| 1467 | - | ||
| 1468 | - | // Cancel any existing import worker before starting a new one | |
| 1469 | - | if let Some(old) = self.import_worker.lock().take() { | |
| 1470 | - | old.send(ImportCommand::Cancel); | |
| 1471 | - | drop(old); // joins the thread | |
| 1472 | - | } | |
| 1473 | - | ||
| 1474 | - | let handle = crate::import::spawn_import_worker(db_path, store_root) | |
| 1475 | - | .map_err(|e| BackendError::Other(format!("failed to spawn import worker: {e}")))?; | |
| 1491 | + | let import_strategy = resolve_import_strategy(strategy); | |
| 1492 | + | let handle = self.respawn_import_worker()?; | |
| 1476 | 1493 | handle.send(ImportCommand::ImportDirectory { | |
| 1477 | 1494 | source: source.to_path_buf(), | |
| 1478 | 1495 | strategy: import_strategy, | |
| 1479 | 1496 | }); | |
| 1497 | + | *self.import_worker.lock() = Some(handle); | |
| 1498 | + | Ok(()) | |
| 1499 | + | } | |
| 1480 | 1500 | ||
| 1501 | + | fn start_file_import( | |
| 1502 | + | &self, | |
| 1503 | + | paths: &[PathBuf], | |
| 1504 | + | strategy: ImportStrategyDesc, | |
| 1505 | + | ) -> BackendResult<()> { | |
| 1506 | + | let import_strategy = resolve_import_strategy(strategy); | |
| 1507 | + | let handle = self.respawn_import_worker()?; | |
| 1508 | + | handle.send(ImportCommand::ImportFiles { | |
| 1509 | + | paths: paths.to_vec(), | |
| 1510 | + | strategy: import_strategy, | |
| 1511 | + | }); | |
| 1481 | 1512 | *self.import_worker.lock() = Some(handle); | |
| 1482 | 1513 | Ok(()) | |
| 1483 | 1514 | } | |
| @@ -1824,6 +1855,9 @@ impl Backend for DirectBackend { | |||
| 1824 | 1855 | still_missing, | |
| 1825 | 1856 | }, | |
| 1826 | 1857 | Lfe::PurgeResult { purged } => BackendEvent::LooseFilesPurged { purged }, | |
| 1858 | + | Lfe::VerifyResult { checked, corrupt } => { | |
| 1859 | + | BackendEvent::StoreIntegrity { checked, corrupt } | |
| 1860 | + | } | |
| 1827 | 1861 | Lfe::Failed { message } => BackendEvent::LooseFilesFailed { message }, | |
| 1828 | 1862 | }); | |
| 1829 | 1863 | } |
| @@ -122,6 +122,12 @@ pub enum BackendEvent { | |||
| 122 | 122 | LooseFilesPurged { | |
| 123 | 123 | purged: usize, | |
| 124 | 124 | }, | |
| 125 | + | /// Managed-store CAS scrub finished: `checked` blobs re-hashed, `corrupt` of | |
| 126 | + | /// them failed to match their content address. | |
| 127 | + | StoreIntegrity { | |
| 128 | + | checked: usize, | |
| 129 | + | corrupt: usize, | |
| 130 | + | }, | |
| 125 | 131 | LooseFilesFailed { | |
| 126 | 132 | message: String, | |
| 127 | 133 | }, | |
| @@ -669,6 +675,12 @@ pub trait Backend: Send + Sync { | |||
| 669 | 675 | /// Result via `BackendEvent::LooseFilesPurged`. | |
| 670 | 676 | fn start_loose_files_purge(&self) -> BackendResult<()>; | |
| 671 | 677 | ||
| 678 | + | /// Start a background CAS scrub: re-hash every managed blob and count those | |
| 679 | + | /// whose bytes no longer match their content address. Result via | |
| 680 | + | /// `BackendEvent::StoreIntegrity`. Off the GUI thread because it re-hashes | |
| 681 | + | /// the whole managed library. | |
| 682 | + | fn start_store_verify(&self) -> BackendResult<()>; | |
| 683 | + | ||
| 672 | 684 | // --- Export --- | |
| 673 | 685 | ||
| 674 | 686 | /// Collect export items from a VFS subtree. | |
| @@ -788,6 +800,15 @@ pub trait Backend: Send + Sync { | |||
| 788 | 800 | strategy: ImportStrategyDesc, | |
| 789 | 801 | ) -> BackendResult<()>; | |
| 790 | 802 | ||
| 803 | + | /// Start a background import of an explicit list of files (loose drops, CLI | |
| 804 | + | /// args, OS "Open with"). Runs on the same worker as [`start_import`] so the | |
| 805 | + | /// hashing never blocks the GUI thread. | |
| 806 | + | fn start_file_import( | |
| 807 | + | &self, | |
| 808 | + | paths: &[PathBuf], | |
| 809 | + | strategy: ImportStrategyDesc, | |
| 810 | + | ) -> BackendResult<()>; | |
| 811 | + | ||
| 791 | 812 | /// Start analysis on a batch of samples. | |
| 792 | 813 | fn start_analysis( | |
| 793 | 814 | &self, |
| @@ -68,6 +68,14 @@ pub enum ImportCommand { | |||
| 68 | 68 | source: PathBuf, | |
| 69 | 69 | strategy: ImportStrategy, | |
| 70 | 70 | }, | |
| 71 | + | /// Import an explicit list of files (loose drops, CLI args, OS "Open with") | |
| 72 | + | /// using the given strategy. Non-audio and directory entries are ignored. | |
| 73 | + | /// Same worker path as [`ImportDirectory`] so the hashing never runs on the | |
| 74 | + | /// GUI thread — the sync per-file loop used to freeze the window. | |
| 75 | + | ImportFiles { | |
| 76 | + | paths: Vec<PathBuf>, | |
| 77 | + | strategy: ImportStrategy, | |
| 78 | + | }, | |
| 71 | 79 | /// Cancel the current import (sets the worker's cancel flag synchronously). | |
| 72 | 80 | Cancel, | |
| 73 | 81 | } | |
| @@ -483,6 +491,29 @@ fn import_directory_flat( | |||
| 483 | 491 | false | |
| 484 | 492 | } | |
| 485 | 493 | ||
| 494 | + | /// Import an explicit list of files at one flat level. Directory and non-audio | |
| 495 | + | /// entries are ignored (the caller may pass a mixed drop). Pre-hashes the audio | |
| 496 | + | /// files in parallel, then records each serially with progress — the same shape | |
| 497 | + | /// as [`import_directory_flat`], minus the recursive walk. Returns `true` if | |
| 498 | + | /// cancelled mid-way. | |
| 499 | + | fn import_file_list( | |
| 500 | + | paths: &[PathBuf], | |
| 501 | + | vfs_id: VfsId, | |
| 502 | + | parent_id: Option<NodeId>, | |
| 503 | + | ctx: &mut ImportContext<'_>, | |
| 504 | + | ) -> bool { | |
| 505 | + | let mut hashes = prehash_level(paths); | |
| 506 | + | for path in paths { | |
| 507 | + | if ctx.is_cancelled() { | |
| 508 | + | return true; | |
| 509 | + | } | |
| 510 | + | if path.is_file() && is_audio_file(path) { | |
| 511 | + | ctx.process_file(path, vfs_id, parent_id, hashes.remove(path)); | |
| 512 | + | } | |
| 513 | + | } | |
| 514 | + | false | |
| 515 | + | } | |
| 516 | + | ||
| 486 | 517 | /// Structured import: iterate source dir's immediate children, import each top-level | |
| 487 | 518 | /// subdirectory via `import_directory_recursive`, tracking `ImportedFolder` per top-level dir. | |
| 488 | 519 | /// Files directly in the source root are imported without a folder grouping. | |
| @@ -572,10 +603,20 @@ fn import_structured( | |||
| 572 | 603 | (false, folders) | |
| 573 | 604 | } | |
| 574 | 605 | ||
| 606 | + | /// A resolved import request: either a directory to walk or an explicit list of | |
| 607 | + | /// files. Unifies the two worker commands so strategy resolution, the | |
| 608 | + | /// loose-files read, Phase 2, and the Complete emission are shared. | |
| 609 | + | enum ImportSource { | |
| 610 | + | Dir(PathBuf), | |
| 611 | + | Files(Vec<PathBuf>), | |
| 612 | + | } | |
| 613 | + | ||
| 575 | 614 | fn import_step(worker: &mut ImportWorker, cmd: ImportCommand, ctx: &WorkerCtx<ImportEvent>) { | |
| 576 | - | let ImportCommand::ImportDirectory { source, strategy } = cmd else { | |
| 615 | + | let (import_source, strategy) = match cmd { | |
| 616 | + | ImportCommand::ImportDirectory { source, strategy } => (ImportSource::Dir(source), strategy), | |
| 617 | + | ImportCommand::ImportFiles { paths, strategy } => (ImportSource::Files(paths), strategy), | |
| 577 | 618 | // Cancel: the flag was already set synchronously by the handle. | |
| 578 | - | return; | |
| 619 | + | ImportCommand::Cancel => return, | |
| 579 | 620 | }; | |
| 580 | 621 | ||
| 581 | 622 | ctx.reset_cancel(); | |
| @@ -604,18 +645,32 @@ fn import_step(worker: &mut ImportWorker, cmd: ImportCommand, ctx: &WorkerCtx<Im | |||
| 604 | 645 | ImportStrategy::MergeIntoVfs { vfs_id, parent_id } => (vfs_id, parent_id, false), | |
| 605 | 646 | }; | |
| 606 | 647 | ||
| 607 | - | // Phase 1: pre-walk to count audio files and sum sizes | |
| 608 | - | let (total, total_bytes) = match count_audio_files(&source, cancel, sink) { | |
| 609 | - | Some(result) => result, | |
| 610 | - | None => { | |
| 611 | - | sink.emit(ImportEvent::Complete { | |
| 612 | - | imported: Vec::new(), | |
| 613 | - | total_files: 0, | |
| 614 | - | errors: 0, | |
| 615 | - | duplicates: 0, | |
| 616 | - | folders: Vec::new(), | |
| 617 | - | }); | |
| 618 | - | return; | |
| 648 | + | // Phase 1: determine the total audio-file count and byte size. A directory | |
| 649 | + | // is walked (cancellable); an explicit file list is filtered in place. | |
| 650 | + | let (total, total_bytes) = match &import_source { | |
| 651 | + | ImportSource::Dir(source) => match count_audio_files(source, cancel, sink) { | |
| 652 | + | Some(result) => result, | |
| 653 | + | None => { | |
| 654 | + | sink.emit(ImportEvent::Complete { | |
| 655 | + | imported: Vec::new(), | |
| 656 | + | total_files: 0, | |
| 657 | + | errors: 0, | |
| 658 | + | duplicates: 0, | |
| 659 | + | folders: Vec::new(), | |
| 660 | + | }); | |
| 661 | + | return; | |
| 662 | + | } | |
| 663 | + | }, | |
| 664 | + | ImportSource::Files(paths) => { | |
| 665 | + | let mut count = 0; | |
| 666 | + | let mut bytes = 0u64; | |
| 667 | + | for p in paths { | |
| 668 | + | if p.is_file() && is_audio_file(p) { | |
| 669 | + | count += 1; | |
| 670 | + | bytes += fs::metadata(p).map(|m| m.len()).unwrap_or(0); | |
| 671 | + | } | |
| 672 | + | } | |
| 673 | + | (count, bytes) | |
| 619 | 674 | } | |
| 620 | 675 | }; | |
| 621 | 676 | ||
| @@ -656,11 +711,16 @@ fn import_step(worker: &mut ImportWorker, cmd: ImportCommand, ctx: &WorkerCtx<Im | |||
| 656 | 711 | loose_files, | |
| 657 | 712 | }; | |
| 658 | 713 | ||
| 659 | - | let (cancelled, folders) = if flat { | |
| 660 | - | let c = import_directory_flat(&source, vfs_id, parent_id, &mut import_ctx); | |
| 661 | - | (c, Vec::new()) | |
| 662 | - | } else { | |
| 663 | - | import_structured(&source, vfs_id, parent_id, &mut import_ctx) | |
| 714 | + | let (cancelled, folders) = match &import_source { | |
| 715 | + | ImportSource::Dir(source) if flat => { | |
| 716 | + | (import_directory_flat(source, vfs_id, parent_id, &mut import_ctx), Vec::new()) | |
| 717 | + | } | |
| 718 | + | ImportSource::Dir(source) => import_structured(source, vfs_id, parent_id, &mut import_ctx), | |
| 719 | + | // An explicit file list has no directory structure to preserve, so it | |
| 720 | + | // always imports flat (folders empty), regardless of strategy. | |
| 721 | + | ImportSource::Files(paths) => { | |
| 722 | + | (import_file_list(paths, vfs_id, parent_id, &mut import_ctx), Vec::new()) | |
| 723 | + | } | |
| 664 | 724 | }; | |
| 665 | 725 | ||
| 666 | 726 | let total_files = if cancelled { completed } else { total }; | |
| @@ -685,6 +745,46 @@ mod tests { | |||
| 685 | 745 | use super::*; | |
| 686 | 746 | ||
| 687 | 747 | #[test] | |
| 748 | + | fn import_files_command_imports_explicit_list_off_thread() { | |
| 749 | + | // B2/B3: an explicit file list imports through the worker (not the GUI | |
| 750 | + | // thread). Two audio files + one non-audio; only the two audio files | |
| 751 | + | // land, and the non-audio is ignored. | |
| 752 | + | let dir = tempfile::TempDir::new().unwrap(); | |
| 753 | + | let db_path = dir.path().join("audiofiles.db"); | |
| 754 | + | let store_root = dir.path().join("store"); | |
| 755 | + | let db = Database::open(&db_path).unwrap(); | |
| 756 | + | let vfs_id = vfs::create_vfs(&db, "Library").unwrap(); | |
| 757 | + | ||
| 758 | + | let a = dir.path().join("a.wav"); | |
| 759 | + | let b = dir.path().join("b.wav"); | |
| 760 | + | let c = dir.path().join("notes.txt"); | |
| 761 | + | fs::write(&a, b"AAAA").unwrap(); | |
| 762 | + | fs::write(&b, b"BBBB").unwrap(); | |
| 763 | + | fs::write(&c, b"not audio").unwrap(); | |
| 764 | + | // Release the GUI-side connection so the worker's own connection can open | |
| 765 | + | // the same DB file without contention. | |
| 766 | + | drop(db); | |
| 767 | + | ||
| 768 | + | let handle = spawn_import_worker(db_path, store_root).unwrap(); | |
| 769 | + | assert!(handle.send(ImportCommand::ImportFiles { | |
| 770 | + | paths: vec![a, b, c], | |
| 771 | + | strategy: ImportStrategy::MergeIntoVfs { vfs_id, parent_id: None }, | |
| 772 | + | })); | |
| 773 | + | ||
| 774 | + | let mut done = None; | |
| 775 | + | for _ in 0..100 { | |
| 776 | + | if let Some(ImportEvent::Complete { imported, total_files, .. }) = handle.try_recv() { | |
| 777 | + | done = Some((imported.len(), total_files)); | |
| 778 | + | break; | |
| 779 | + | } | |
| 780 | + | std::thread::sleep(std::time::Duration::from_millis(20)); | |
| 781 | + | } | |
| 782 | + | let (imported, total) = done.expect("expected a Complete event"); | |
| 783 | + | assert_eq!(total, 2, "only the two audio files count"); | |
| 784 | + | assert_eq!(imported, 2, "both audio files imported"); | |
| 785 | + | } | |
| 786 | + | ||
| 787 | + | #[test] | |
| 688 | 788 | fn is_audio_file_recognises_extensions() { | |
| 689 | 789 | assert!(is_audio_file(Path::new("kick.wav"))); | |
| 690 | 790 | assert!(is_audio_file(Path::new("pad.FLAC"))); |
| @@ -8,6 +8,7 @@ use std::path::PathBuf; | |||
| 8 | 8 | use tracing::instrument; | |
| 9 | 9 | ||
| 10 | 10 | use audiofiles_core::db::Database; | |
| 11 | + | use audiofiles_core::store::SampleStore; | |
| 11 | 12 | use audiofiles_core::worker_runtime::{spawn_worker, WorkerCtx, WorkerHandle}; | |
| 12 | 13 | ||
| 13 | 14 | /// Command from the GUI thread to the loose-files worker. | |
| @@ -18,6 +19,10 @@ pub enum LooseFilesCommand { | |||
| 18 | 19 | Relocate(PathBuf), | |
| 19 | 20 | /// Remove loose-files samples whose source file is missing. | |
| 20 | 21 | Purge, | |
| 22 | + | /// Re-hash every managed blob and report any whose bytes no longer match | |
| 23 | + | /// their content address (the CAS scrub). Managed-mode counterpart to | |
| 24 | + | /// `CheckIntegrity`, which only checks loose-files source presence. | |
| 25 | + | VerifyStore, | |
| 21 | 26 | } | |
| 22 | 27 | ||
| 23 | 28 | /// Event from the loose-files worker back to the GUI thread. | |
| @@ -28,6 +33,9 @@ pub enum LooseFilesEvent { | |||
| 28 | 33 | RelocateResult { relocated: usize, still_missing: usize }, | |
| 29 | 34 | /// Purge finished. | |
| 30 | 35 | PurgeResult { purged: usize }, | |
| 36 | + | /// Store scrub finished: `checked` managed blobs re-hashed, `corrupt` of | |
| 37 | + | /// them failed to match their content address. | |
| 38 | + | VerifyResult { checked: usize, corrupt: usize }, | |
| 31 | 39 | /// An operation failed. | |
| 32 | 40 | Failed { message: String }, | |
| 33 | 41 | } | |
| @@ -46,7 +54,10 @@ pub type LooseFilesHandle = WorkerHandle<LooseFilesCommand, LooseFilesEvent>; | |||
| 46 | 54 | /// Spawn the loose-files worker. It opens its own `Database` to avoid contending | |
| 47 | 55 | /// with the GUI connection. | |
| 48 | 56 | #[instrument(skip_all)] | |
| 49 | - | pub fn spawn_loose_files_worker(db_path: PathBuf) -> std::io::Result<LooseFilesHandle> { | |
| 57 | + | pub fn spawn_loose_files_worker( | |
| 58 | + | db_path: PathBuf, | |
| 59 | + | store_root: PathBuf, | |
| 60 | + | ) -> std::io::Result<LooseFilesHandle> { | |
| 50 | 61 | spawn_worker( | |
| 51 | 62 | "loose-files-worker", | |
| 52 | 63 | move || Database::open(&db_path), | |
| @@ -56,11 +67,16 @@ pub fn spawn_loose_files_worker(db_path: PathBuf) -> std::io::Result<LooseFilesH | |||
| 56 | 67 | |_state| LooseFilesEvent::Failed { | |
| 57 | 68 | message: "loose-files maintenance panicked (internal error)".to_string(), | |
| 58 | 69 | }, | |
| 59 | - | loose_files_step, | |
| 70 | + | move |db, cmd, ctx| loose_files_step(db, &store_root, cmd, ctx), | |
| 60 | 71 | ) | |
| 61 | 72 | } | |
| 62 | 73 | ||
| 63 | - | fn loose_files_step(db: &mut Database, cmd: LooseFilesCommand, ctx: &WorkerCtx<LooseFilesEvent>) { | |
| 74 | + | fn loose_files_step( | |
| 75 | + | db: &mut Database, | |
| 76 | + | store_root: &std::path::Path, | |
| 77 | + | cmd: LooseFilesCommand, | |
| 78 | + | ctx: &WorkerCtx<LooseFilesEvent>, | |
| 79 | + | ) { | |
| 64 | 80 | let event = match cmd { | |
| 65 | 81 | LooseFilesCommand::CheckIntegrity => { | |
| 66 | 82 | match audiofiles_core::store::check_loose_files_integrity(db) { | |
| @@ -87,6 +103,17 @@ fn loose_files_step(db: &mut Database, cmd: LooseFilesCommand, ctx: &WorkerCtx<L | |||
| 87 | 103 | message: e.to_string(), | |
| 88 | 104 | }, | |
| 89 | 105 | }, | |
| 106 | + | LooseFilesCommand::VerifyStore => { | |
| 107 | + | match SampleStore::new(store_root).and_then(|store| store.scrub(db)) { | |
| 108 | + | Ok((checked, corrupt)) => LooseFilesEvent::VerifyResult { | |
| 109 | + | checked, | |
| 110 | + | corrupt: corrupt.len(), | |
| 111 | + | }, | |
| 112 | + | Err(e) => LooseFilesEvent::Failed { | |
| 113 | + | message: e.to_string(), | |
| 114 | + | }, | |
| 115 | + | } | |
| 116 | + | } | |
| 90 | 117 | }; | |
| 91 | 118 | ctx.emit(event); | |
| 92 | 119 | } | |
| @@ -101,7 +128,7 @@ mod tests { | |||
| 101 | 128 | let db_path = dir.path().join("audiofiles.db"); | |
| 102 | 129 | let _db = Database::open(&db_path).unwrap(); | |
| 103 | 130 | ||
| 104 | - | let handle = spawn_loose_files_worker(db_path).unwrap(); | |
| 131 | + | let handle = spawn_loose_files_worker(db_path, dir.path().join("samples")).unwrap(); | |
| 105 | 132 | assert!(handle.try_recv().is_none()); | |
| 106 | 133 | drop(handle); | |
| 107 | 134 | } | |
| @@ -112,7 +139,7 @@ mod tests { | |||
| 112 | 139 | let db_path = dir.path().join("audiofiles.db"); | |
| 113 | 140 | let _db = Database::open(&db_path).unwrap(); | |
| 114 | 141 | ||
| 115 | - | let handle = spawn_loose_files_worker(db_path).unwrap(); | |
| 142 | + | let handle = spawn_loose_files_worker(db_path, dir.path().join("samples")).unwrap(); | |
| 116 | 143 | assert!(handle.send(LooseFilesCommand::CheckIntegrity)); | |
| 117 | 144 | ||
| 118 | 145 | let mut got = false; |
| @@ -260,6 +260,16 @@ impl BrowserState { | |||
| 260 | 260 | ||
| 261 | 261 | /// Spawn the background analysis worker and start processing samples. | |
| 262 | 262 | pub fn run_analysis(&mut self, sample_hashes: Vec<(String, String)>, config: AnalysisConfig) { | |
| 263 | + | // This is a foreground, user-initiated analysis (it drives the Analyzing | |
| 264 | + | // screen and populates pending_review_items). If a background Settings | |
| 265 | + | // "Backfill audio features" run was in flight, its flag must not survive | |
| 266 | + | // into this batch: the completion handler routes results by | |
| 267 | + | // `backfill_in_progress`, so a leaked flag would make this import's | |
| 268 | + | // Analyzing screen freeze at 0/total and silently skip tag review. The | |
| 269 | + | // foreground analysis supersedes the backfill. (Mirrors cancel_import's | |
| 270 | + | // quick_import reset — the flag-leak class from fuzz-2026-07-06 B1.) | |
| 271 | + | self.import_wf.backfill_in_progress = false; | |
| 272 | + | ||
| 263 | 273 | // Stash parameters so the retry button can restart analysis. | |
| 264 | 274 | self.import_wf.last_analysis_hashes = sample_hashes.clone(); | |
| 265 | 275 | self.import_wf.last_analysis_config = Some(config.clone()); | |
| @@ -445,6 +455,39 @@ impl BrowserState { | |||
| 445 | 455 | }; | |
| 446 | 456 | } | |
| 447 | 457 | ||
| 458 | + | /// Start a background import of an explicit list of files (loose drops, CLI | |
| 459 | + | /// args, OS "Open with"). Mirrors [`start_folder_import`](Self::start_folder_import) | |
| 460 | + | /// but hands the worker a file list instead of a directory to walk — the | |
| 461 | + | /// hashing runs off the GUI thread instead of freezing the window with a | |
| 462 | + | /// synchronous per-file loop (fuzz-2026-07-06 B2/B3). | |
| 463 | + | pub fn start_files_import(&mut self, paths: Vec<PathBuf>, strategy: ImportStrategy) { | |
| 464 | + | let strategy_desc = match strategy { | |
| 465 | + | ImportStrategy::Flat { vfs_id, parent_id } => { | |
| 466 | + | ImportStrategyDesc::Flat { vfs_id, parent_id } | |
| 467 | + | } | |
| 468 | + | ImportStrategy::NewVfs { vfs_name } => ImportStrategyDesc::NewVfs { vfs_name }, | |
| 469 | + | ImportStrategy::MergeIntoVfs { vfs_id, parent_id } => { | |
| 470 | + | ImportStrategyDesc::MergeIntoVfs { vfs_id, parent_id } | |
| 471 | + | } | |
| 472 | + | }; | |
| 473 | + | ||
| 474 | + | super::log_backend_err("start_file_import", self.backend.start_file_import(&paths, strategy_desc)); | |
| 475 | + | ||
| 476 | + | self.import_wf.import_file_errors.clear(); | |
| 477 | + | self.import_wf.analysis_errors.clear(); | |
| 478 | + | self.import_wf.operation_progress = Some(crate::state::OperationProgress::new()); | |
| 479 | + | let loose_files = self.settings.is_loose_files; | |
| 480 | + | self.import_wf.import_mode = ImportMode::Importing { | |
| 481 | + | total: 0, | |
| 482 | + | completed: 0, | |
| 483 | + | current_name: String::new(), | |
| 484 | + | walking: true, | |
| 485 | + | walking_count: 0, | |
| 486 | + | total_bytes: 0, | |
| 487 | + | loose_files, | |
| 488 | + | }; | |
| 489 | + | } | |
| 490 | + | ||
| 448 | 491 | /// Whether any background worker that reports a single terminal event is in | |
| 449 | 492 | /// flight. The single source of truth for the repaint guard in `draw_browser`: | |
| 450 | 493 | /// egui sleeps after input settles, so while a worker runs the frame loop must | |
| @@ -788,6 +831,17 @@ impl BrowserState { | |||
| 788 | 831 | self.loose_files.show_loose_files_warning = false; | |
| 789 | 832 | self.refresh_contents(); | |
| 790 | 833 | } | |
| 834 | + | BackendEvent::StoreIntegrity { checked, corrupt } => { | |
| 835 | + | self.loose_files.loose_files_busy = false; | |
| 836 | + | self.status = if corrupt == 0 { | |
| 837 | + | format!("Library integrity verified: {checked} samples, all intact.") | |
| 838 | + | } else { | |
| 839 | + | format!( | |
| 840 | + | "Library integrity check: {corrupt} of {checked} samples are corrupt \ | |
| 841 | + | or missing their blob." | |
| 842 | + | ) | |
| 843 | + | }; | |
| 844 | + | } | |
| 791 | 845 | BackendEvent::LooseFilesFailed { message } => { | |
| 792 | 846 | self.loose_files.loose_files_busy = false; | |
| 793 | 847 | self.status = format!("Loose-files operation failed: {message}"); | |
| @@ -1298,8 +1352,19 @@ impl BrowserState { | |||
| 1298 | 1352 | Some(h) => h.clone(), | |
| 1299 | 1353 | None => return, | |
| 1300 | 1354 | }; | |
| 1355 | + | // total_frames falls back to 0 when analysis is absent; a 0-length span | |
| 1356 | + | // (or an inverted one) would dispatch Trim{0,0} and produce a zero-length | |
| 1357 | + | // sample. Guard it like apply_edit_remove_range does (fuzz-2026-07-06 B4). | |
| 1358 | + | if self.edit.total_frames == 0 { | |
| 1359 | + | self.status = "Cannot trim: sample length is unknown.".to_string(); | |
| 1360 | + | return; | |
| 1361 | + | } | |
| 1301 | 1362 | let start = (self.edit.trim_start * self.edit.total_frames as f32) as usize; | |
| 1302 | 1363 | let end = (self.edit.trim_end * self.edit.total_frames as f32) as usize; | |
| 1364 | + | if start >= end { | |
| 1365 | + | self.status = "Trim range: start must be before end".to_string(); | |
| 1366 | + | return; | |
| 1367 | + | } | |
| 1303 | 1368 | let op = audiofiles_core::edit::EditOperation::Trim { start_frame: start, end_frame: end }; | |
| 1304 | 1369 | let ext = self.backend.sample_extension(&hash).unwrap_or_default(); | |
| 1305 | 1370 | if let Err(e) = self.backend.start_edit(&hash, &ext, op) { |
| @@ -65,6 +65,22 @@ impl BrowserState { | |||
| 65 | 65 | self.loose_files.show_loose_files_warning = false; | |
| 66 | 66 | } | |
| 67 | 67 | ||
| 68 | + | /// User-initiated managed-store CAS scrub: re-hash every managed blob and | |
| 69 | + | /// report how many no longer match their content address. Runs off the GUI | |
| 70 | + | /// thread (it re-hashes the whole library); the result arrives as | |
| 71 | + | /// `BackendEvent::StoreIntegrity`. This is the runtime home for the store's | |
| 72 | + | /// self-check — the "the filename is the content hash" invariant is only as | |
| 73 | + | /// good as something that actually re-reads the bytes. | |
| 74 | + | pub fn verify_store_integrity_now(&mut self) { | |
| 75 | + | match self.backend.start_store_verify() { | |
| 76 | + | Ok(()) => { | |
| 77 | + | self.loose_files.loose_files_busy = true; | |
| 78 | + | self.status = "Verifying library integrity\u{2026}".to_string(); | |
| 79 | + | } | |
| 80 | + | Err(e) => self.status = format!("Could not start integrity check \u{2014} {e}"), | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | ||
| 68 | 84 | /// Locate missing loose-files mode samples by walking `search_root` and | |
| 69 | 85 | /// hash-verifying candidates. The dialog stays open with an updated | |
| 70 | 86 | /// `loose_files_missing_count` so the user can run Locate again against a |
| @@ -416,8 +416,14 @@ pub fn draw_background_context_menu(ui: &mut egui::Ui, state: &mut BrowserState) | |||
| 416 | 416 | "Import files", | |
| 417 | 417 | &[("Audio", audiofiles_core::util::AUDIO_EXTENSIONS)], | |
| 418 | 418 | |s, paths| { | |
| 419 | - | for path in paths { | |
| 420 | - | s.import_path(&path); | |
| 419 | + | // Batch through the background worker so a large multi-select | |
| 420 | + | // doesn't hash on the GUI thread (fuzz B3). | |
| 421 | + | if let Some(vfs_id) = s.current_vfs_id() { | |
| 422 | + | let strategy = crate::import::ImportStrategy::MergeIntoVfs { | |
| 423 | + | vfs_id, | |
| 424 | + | parent_id: s.nav.current_dir, | |
| 425 | + | }; | |
| 426 | + | s.start_files_import(paths, strategy); | |
| 421 | 427 | } | |
| 422 | 428 | }, | |
| 423 | 429 | ); |
| @@ -333,6 +333,30 @@ fn draw_storage_section(ui: &mut egui::Ui, state: &mut BrowserState) { | |||
| 333 | 333 | } | |
| 334 | 334 | }); | |
| 335 | 335 | ||
| 336 | + | // Verify library integrity: re-hash every managed blob and report | |
| 337 | + | // any whose bytes no longer match their content address (bit-rot, an | |
| 338 | + | // out-of-band edit through the VFS mirror, a truncated blob). Runs on | |
| 339 | + | // the maintenance worker; the busy flag is shared with the loose-files | |
| 340 | + | // ops, so the button disables while any of them is in flight. | |
| 341 | + | ui.add_space(theme::space::SM); | |
| 342 | + | ui.horizontal(|ui| { | |
| 343 | + | let busy = state.loose_files.loose_files_busy; | |
| 344 | + | if ui | |
| 345 | + | .add_enabled(!busy, egui::Button::new("Verify library integrity")) | |
| 346 | + | .on_hover_text( | |
| 347 | + | "Re-hash every stored sample and confirm its bytes still match its \ | |
| 348 | + | content address. Catches silent on-disk corruption. Runs in the \ | |
| 349 | + | background \u{2014} the result appears in the status line.", | |
| 350 | + | ) | |
| 351 | + | .clicked() | |
| 352 | + | { | |
| 353 | + | state.verify_store_integrity_now(); | |
| 354 | + | } | |
| 355 | + | if busy { | |
| 356 | + | ui.spinner(); | |
| 357 | + | } | |
| 358 | + | }); | |
| 359 | + | ||
| 336 | 360 | ui.add_space(theme::space::MD); | |
| 337 | 361 | ui.separator(); | |
| 338 | 362 | ui.add_space(theme::space::SM); |
| @@ -438,8 +438,14 @@ fn draw_breadcrumb( | |||
| 438 | 438 | "Import files", | |
| 439 | 439 | &[("Audio", audiofiles_core::util::AUDIO_EXTENSIONS)], | |
| 440 | 440 | |s, paths| { | |
| 441 | - | for path in paths { | |
| 442 | - | s.import_path(&path); | |
| 441 | + | // Batch through the background worker so a large multi- | |
| 442 | + | // select doesn't hash on the GUI thread (fuzz B3). | |
| 443 | + | if let Some(vfs_id) = s.current_vfs_id() { | |
| 444 | + | let strategy = crate::import::ImportStrategy::MergeIntoVfs { | |
| 445 | + | vfs_id, | |
| 446 | + | parent_id: s.nav.current_dir, | |
| 447 | + | }; | |
| 448 | + | s.start_files_import(paths, strategy); | |
| 443 | 449 | } | |
| 444 | 450 | }, | |
| 445 | 451 | ); |
| @@ -63,6 +63,14 @@ pub enum CoreError { | |||
| 63 | 63 | #[error("invalid hash: {0}")] | |
| 64 | 64 | HashInvalid(String), | |
| 65 | 65 | ||
| 66 | + | /// The bytes actually copied into the store did not hash to the content | |
| 67 | + | /// address they were to be stored under — the source file changed between | |
| 68 | + | /// the pre-hash pass and the serial copy (a DAW re-render, a cloud-sync | |
| 69 | + | /// client, a network share). The copy is discarded rather than committed to | |
| 70 | + | /// a wrong `{hash}.ext` path, which would silently corrupt the CAS invariant. | |
| 71 | + | #[error("content hash mismatch: {0}")] | |
| 72 | + | HashMismatch(String), | |
| 73 | + | ||
| 66 | 74 | /// Audio analysis error (decode failure, unsupported format, etc.). | |
| 67 | 75 | #[error("analysis error: {0}")] | |
| 68 | 76 | Analysis(#[from] AnalysisError), |
| @@ -18,21 +18,27 @@ pub fn resolve_output_names( | |||
| 18 | 18 | config: &ExportConfig, | |
| 19 | 19 | pattern: Option<&RenamePattern>, | |
| 20 | 20 | ) -> Vec<String> { | |
| 21 | - | // If the backend pre-computed names (e.g. via transform_filename hook), use them. | |
| 22 | - | // These come from an untrusted Rhai plugin, so they get the SAME traversal | |
| 23 | - | // sanitization as computed names — never trust the caller to have done it. | |
| 24 | - | if let Some(ref overrides) = config.name_overrides | |
| 25 | - | && overrides.len() == items.len() { | |
| 26 | - | return overrides.iter().map(|n| strip_traversal(n)).collect(); | |
| 27 | - | } | |
| 28 | - | ||
| 29 | 21 | let output_ext = match config.format { | |
| 30 | 22 | ExportFormat::Wav => "wav", | |
| 31 | 23 | ExportFormat::Aiff => "aiff", | |
| 32 | 24 | ExportFormat::Original => "", | |
| 33 | 25 | }; | |
| 34 | 26 | ||
| 35 | - | let mut names: Vec<String> = if let Some(pat) = pattern { | |
| 27 | + | // If the backend pre-computed names (e.g. via a `transform_filename` hook), | |
| 28 | + | // seed from those — but they are NOT the final names. They come from an | |
| 29 | + | // untrusted Rhai plugin and, just as important, are not yet device-legal: | |
| 30 | + | // they fall through to the naming_rules sanitization, traversal-stripping, | |
| 31 | + | // and dedup below exactly like computed names. Returning them early (the | |
| 32 | + | // prior behavior) let a hook name with spaces / mixed case / over-length | |
| 33 | + | // reach an M8/MPC profile verbatim, so the hardware sampler rejected or | |
| 34 | + | // truncated the headline export. | |
| 35 | + | let mut names: Vec<String> = if let Some(overrides) = config | |
| 36 | + | .name_overrides | |
| 37 | + | .as_ref() | |
| 38 | + | .filter(|o| o.len() == items.len()) | |
| 39 | + | { | |
| 40 | + | overrides.iter().map(|n| n.to_string()).collect() | |
| 41 | + | } else if let Some(pat) = pattern { | |
| 36 | 42 | let contexts: Vec<RenameContext> = items | |
| 37 | 43 | .iter() | |
| 38 | 44 | .enumerate() | |
| @@ -197,6 +203,43 @@ mod tests { | |||
| 197 | 203 | } | |
| 198 | 204 | ||
| 199 | 205 | #[test] | |
| 206 | + | fn override_names_still_pass_through_device_naming_rules() { | |
| 207 | + | // F1 (fuzz-2026-07-06): a plugin's transform_filename returning a | |
| 208 | + | // human-friendly name ("My Cool Kick (v2).wav") must still be conformed | |
| 209 | + | // to the target device's naming rules (uppercase, no spaces/specials, | |
| 210 | + | // length cap) — otherwise the hardware sampler rejects the export. | |
| 211 | + | use crate::export::profile::{NamingCase, NamingRules}; | |
| 212 | + | let items = vec![item("orig.wav")]; | |
| 213 | + | let mut config = config_with_overrides(vec!["My Cool Kick (v2).wav".to_string()]); | |
| 214 | + | config.naming_rules = Some(NamingRules { | |
| 215 | + | case: NamingCase::Upper, | |
| 216 | + | separator: '_', | |
| 217 | + | max_length: 32, | |
| 218 | + | strip_special: true, | |
| 219 | + | }); | |
| 220 | + | ||
| 221 | + | let out = resolve_output_names(&items, &config, None); | |
| 222 | + | assert_eq!(out.len(), 1); | |
| 223 | + | let name = &out[0]; | |
| 224 | + | // Uppercased, no spaces, no parens — device-legal. | |
| 225 | + | assert!(!name.contains(' '), "spaces must be gone: {name}"); | |
| 226 | + | assert!(!name.contains('('), "specials must be stripped: {name}"); | |
| 227 | + | let (stem, _ext) = split_name_ext(name); | |
| 228 | + | assert_eq!(stem, stem.to_uppercase(), "stem must be uppercased: {name}"); | |
| 229 | + | } | |
| 230 | + | ||
| 231 | + | #[test] | |
| 232 | + | fn override_names_are_deduplicated() { | |
| 233 | + | // Two plugin overrides that collide must not silently overwrite in a flat | |
| 234 | + | // export — the dedup pass (previously skipped for overrides) still runs. | |
| 235 | + | let items = vec![item("a.wav"), item("b.wav")]; | |
| 236 | + | let overrides = vec!["kick.wav".to_string(), "kick.wav".to_string()]; | |
| 237 | + | let out = resolve_output_names(&items, &config_with_overrides(overrides), None); | |
| 238 | + | assert_eq!(out.len(), 2); | |
| 239 | + | assert_ne!(out[0], out[1], "colliding overrides must be de-duplicated"); | |
| 240 | + | } | |
| 241 | + | ||
| 242 | + | #[test] | |
| 200 | 243 | fn strip_traversal_defuses_payloads() { | |
| 201 | 244 | assert_eq!(strip_traversal("a/b\\c\0d.wav"), "a_b_c_d.wav"); | |
| 202 | 245 | assert_eq!(strip_traversal(".."), "__"); |
| @@ -148,6 +148,32 @@ pub fn run_export( | |||
| 148 | 148 | // Avoid silently overwriting existing files in the user's export dir. | |
| 149 | 149 | let dest = resolve_collision(&dest); | |
| 150 | 150 | ||
| 151 | + | // Reject an over-limit export *before* writing it, not after. For an | |
| 152 | + | // Original (byte-for-byte copy) export the output size equals the source | |
| 153 | + | // size exactly, so the source metadata is an exact up-front check — this | |
| 154 | + | // stops a 20 GB source from filling a 4 GB-cap SD card only to be deleted | |
| 155 | + | // by the post-write check below. Conversions (whose encoded size isn't | |
| 156 | + | // known until the encoder runs) still rely on that post-write backstop. | |
| 157 | + | if let Some(limit) = config.max_file_size_bytes | |
| 158 | + | && config.format == ExportFormat::Original | |
| 159 | + | { | |
| 160 | + | match fs::metadata(&source) { | |
| 161 | + | Ok(meta) if meta.len() > limit => { | |
| 162 | + | let actual = meta.len(); | |
| 163 | + | errors.push(( | |
| 164 | + | item.name.clone(), | |
| 165 | + | format!("exceeds device file size limit ({actual} bytes > {limit} bytes)"), | |
| 166 | + | )); | |
| 167 | + | continue; | |
| 168 | + | } | |
| 169 | + | Ok(_) => {} | |
| 170 | + | Err(e) => { | |
| 171 | + | errors.push((item.name.clone(), format!("size check: {e}"))); | |
| 172 | + | continue; | |
| 173 | + | } | |
| 174 | + | } | |
| 175 | + | } | |
| 176 | + | ||
| 151 | 177 | if let Err(e) = export_single_item(&source, &dest, item, config, cancel) { | |
| 152 | 178 | // A mid-file streaming cancel surfaces as `Cancelled`; stop the batch | |
| 153 | 179 | // cleanly rather than recording it as a per-item export failure. | |
| @@ -396,6 +422,33 @@ mod tests { | |||
| 396 | 422 | } | |
| 397 | 423 | ||
| 398 | 424 | #[test] | |
| 425 | + | fn run_export_rejects_oversize_original_before_writing() { | |
| 426 | + | // F2 (fuzz-2026-07-06): an Original export over the device size cap must | |
| 427 | + | // be rejected up front (source metadata is exact for a byte-copy), never | |
| 428 | + | // written-then-deleted — so a huge source can't fill the destination. | |
| 429 | + | let dir = tempfile::tempdir().unwrap(); | |
| 430 | + | let src = write_source(dir.path(), "big.wav", &vec![0u8; 4096]); | |
| 431 | + | let dest = dir.path().join("out"); | |
| 432 | + | let store = SampleStore::new(dir.path().join("store")).unwrap(); | |
| 433 | + | let items = vec![item_from(&src, "big.wav")]; | |
| 434 | + | let cancel = AtomicBool::new(false); | |
| 435 | + | ||
| 436 | + | let mut config = original_config(&dest, false); | |
| 437 | + | config.max_file_size_bytes = Some(1024); // source is 4096 bytes | |
| 438 | + | ||
| 439 | + | let summary = | |
| 440 | + | run_export(&items, &config, &store, &cancel, |_, _, _| true).unwrap(); | |
| 441 | + | assert_eq!(summary.errors.len(), 1); | |
| 442 | + | assert!( | |
| 443 | + | summary.errors[0].1.contains("device file size limit"), | |
| 444 | + | "unexpected error: {:?}", | |
| 445 | + | summary.errors | |
| 446 | + | ); | |
| 447 | + | // Nothing was written to the destination (not even transiently left). | |
| 448 | + | assert!(!dest.join("big.wav").exists(), "no oversize file must be written"); | |
| 449 | + | } | |
| 450 | + | ||
| 451 | + | #[test] | |
| 399 | 452 | fn run_export_precancelled_flag_produces_nothing() { | |
| 400 | 453 | let dir = tempfile::tempdir().unwrap(); | |
| 401 | 454 | let src = write_source(dir.path(), "src.wav", b"data"); |
| @@ -10,7 +10,7 @@ | |||
| 10 | 10 | //! metadata row. The hash lets SyncKit re-download the exact file from blob storage later. | |
| 11 | 11 | ||
| 12 | 12 | use std::fs; | |
| 13 | - | use std::io::Read; | |
| 13 | + | use std::io::{Read, Write}; | |
| 14 | 14 | use std::path::{Path, PathBuf}; | |
| 15 | 15 | ||
| 16 | 16 | use sha2::{Digest, Sha256}; | |
| @@ -181,13 +181,25 @@ impl SampleStore { | |||
| 181 | 181 | }; | |
| 182 | 182 | if needs_write { | |
| 183 | 183 | let tmp = dest.with_file_name(format!("{hash}.{ext}.{}.tmp", std::process::id())); | |
| 184 | - | fs::copy(path, &tmp).map_err(|e| io_err(&tmp, e))?; | |
| 185 | - | // fsync the blob bytes to disk before the rename. Without this the | |
| 186 | - | // rename can be durable while the file contents are not, leaving a | |
| 187 | - | // canonical-path blob with garbage after a power loss — which a | |
| 188 | - | // content-addressed store would then trust forever. | |
| 189 | - | if let Ok(f) = fs::File::open(&tmp) { | |
| 190 | - | let _ = f.sync_all(); | |
| 184 | + | // Copy *and* hash the bytes in a single pass, fsyncing the temp | |
| 185 | + | // before the rename. `hash` was computed by an earlier pass (batch | |
| 186 | + | // import pre-hashes the whole set, then copies one-by-one much | |
| 187 | + | // later); the source can mutate in that window (DAW re-render, | |
| 188 | + | // cloud-sync client, network share). Copying blindly would land | |
| 189 | + | // wrong bytes at `{hash}.ext` and — because dedup is size-only and | |
| 190 | + | // no read path re-hashes — trust them forever. Verifying the bytes | |
| 191 | + | // we actually wrote against the content address closes that TOCTOU: | |
| 192 | + | // a mismatch discards the temp and fails the import loudly rather | |
| 193 | + | // than corrupting the store. | |
| 194 | + | let copied = copy_hashing(path, &tmp).inspect_err(|_| { | |
| 195 | + | let _ = fs::remove_file(&tmp); | |
| 196 | + | })?; | |
| 197 | + | if copied != hash { | |
| 198 | + | let _ = fs::remove_file(&tmp); | |
| 199 | + | return Err(CoreError::HashMismatch(format!( | |
| 200 | + | "{} changed during import: expected {hash}, copied bytes hash to {copied}", | |
| 201 | + | path.display() | |
| 202 | + | ))); | |
| 191 | 203 | } | |
| 192 | 204 | if let Err(e) = fs::rename(&tmp, &dest) { | |
| 193 | 205 | let _ = fs::remove_file(&tmp); | |
| @@ -423,6 +435,45 @@ impl SampleStore { | |||
| 423 | 435 | ||
| 424 | 436 | Ok(computed == hash) | |
| 425 | 437 | } | |
| 438 | + | ||
| 439 | + | /// Scrub the managed store: re-hash every locally-present blob and report the | |
| 440 | + | /// hashes whose bytes no longer match their content address. | |
| 441 | + | /// | |
| 442 | + | /// This is the runtime home for [`verify_sample`](Self::verify_sample) — the | |
| 443 | + | /// store's headline invariant is "the filename IS the content hash," and a | |
| 444 | + | /// scrub is what confirms that invariant still holds against silent on-disk | |
| 445 | + | /// corruption (bit-rot, an out-of-band edit through the VFS mirror, a | |
| 446 | + | /// truncated blob from a pre-atomic-fix crash). Only managed blobs are | |
| 447 | + | /// checked: loose-files samples live at their `source_path` (covered by | |
| 448 | + | /// [`check_loose_files_integrity`]) and `cloud_only` rows have no local blob | |
| 449 | + | /// to verify. A missing or unreadable blob counts as corrupt (its hash is | |
| 450 | + | /// returned) rather than aborting the whole sweep. | |
| 451 | + | /// | |
| 452 | + | /// Returns `(checked, corrupt_hashes)`. | |
| 453 | + | #[instrument(skip_all)] | |
| 454 | + | pub fn scrub(&self, db: &Database) -> Result<(usize, Vec<String>)> { | |
| 455 | + | let mut stmt = db.conn().prepare( | |
| 456 | + | "SELECT hash, file_extension FROM live_samples \ | |
| 457 | + | WHERE source_path IS NULL AND cloud_only = 0", | |
| 458 | + | )?; | |
| 459 | + | let blobs: Vec<(String, String)> = stmt | |
| 460 | + | .query_map([], |row| Ok((row.get(0)?, row.get(1)?)))? | |
| 461 | + | .collect::<std::result::Result<Vec<_>, _>>()?; | |
| 462 | + | ||
| 463 | + | let mut checked = 0; | |
| 464 | + | let mut corrupt = Vec::new(); | |
| 465 | + | for (hash, ext) in blobs { | |
| 466 | + | checked += 1; | |
| 467 | + | // A read error (blob missing/unreadable) is itself an integrity | |
| 468 | + | // failure for a managed sample — record it as corrupt rather than | |
| 469 | + | // failing the sweep, so one bad blob doesn't hide the rest. | |
| 470 | + | match self.verify_sample(&hash, &ext) { | |
| 471 | + | Ok(true) => {} | |
| 472 | + | Ok(false) | Err(_) => corrupt.push(hash), | |
| 473 | + | } | |
| 474 | + | } | |
| 475 | + | Ok((checked, corrupt)) | |
| 476 | + | } | |
| 426 | 477 | } | |
| 427 | 478 | ||
| 428 | 479 | // --- Sample metadata queries --- | |
| @@ -983,6 +1034,29 @@ pub fn hash_file(path: &Path) -> Result<(String, i64)> { | |||
| 983 | 1034 | Ok((format!("{:x}", hasher.finalize()), file_size)) | |
| 984 | 1035 | } | |
| 985 | 1036 | ||
| 1037 | + | /// Copy `src` to `dst` while computing the SHA-256 of the bytes actually | |
| 1038 | + | /// written, returning the hex digest. Used by the store write path so the copy | |
| 1039 | + | /// can be verified against the content address it will be stored under — the | |
| 1040 | + | /// hash is computed over the same bytes that land on disk, not over an earlier | |
| 1041 | + | /// read of a source that may since have changed. Fsyncs `dst` before returning | |
| 1042 | + | /// so the rename that follows can't be durable ahead of the contents. | |
| 1043 | + | fn copy_hashing(src: &Path, dst: &Path) -> Result<String> { | |
| 1044 | + | let mut input = fs::File::open(src).map_err(|e| io_err(src, e))?; | |
| 1045 | + | let mut output = fs::File::create(dst).map_err(|e| io_err(dst, e))?; | |
| 1046 | + | let mut hasher = Sha256::new(); | |
| 1047 | + | let mut buf = [0u8; 8192]; | |
| 1048 | + | loop { | |
| 1049 | + | let n = input.read(&mut buf).map_err(|e| io_err(src, e))?; | |
| 1050 | + | if n == 0 { | |
| 1051 | + | break; | |
| 1052 | + | } | |
| 1053 | + | hasher.update(&buf[..n]); | |
| 1054 | + | output.write_all(&buf[..n]).map_err(|e| io_err(dst, e))?; | |
| 1055 | + | } | |
| 1056 | + | output.sync_all().map_err(|e| io_err(dst, e))?; | |
| 1057 | + | Ok(format!("{:x}", hasher.finalize())) | |
| 1058 | + | } | |
| 1059 | + | ||
| 986 | 1060 | /// Hash a batch of files in parallel (rayon). Each result is aligned to the | |
| 987 | 1061 | /// corresponding entry in `paths`. Hashing is the dominant per-file import cost | |
| 988 | 1062 | /// (SHA-256 over the whole file) and is embarrassingly parallel — every side | |
| @@ -1471,6 +1545,105 @@ mod tests { | |||
| 1471 | 1545 | } | |
| 1472 | 1546 | ||
| 1473 | 1547 | #[test] | |
| 1548 | + | fn import_hashed_rejects_hash_that_does_not_match_bytes() { | |
| 1549 | + | // Simulates the source file mutating between the parallel pre-hash pass | |
| 1550 | + | // and the serial copy: import_hashed is handed a hash that does not | |
| 1551 | + | // describe the bytes now on disk. The copy must be rejected, never | |
| 1552 | + | // committed to `{wrong_hash}.ext`. | |
| 1553 | + | let (dir, db, store) = setup(); | |
| 1554 | + | let src = create_test_file(&dir, "kick.wav", b"the actual bytes on disk"); | |
| 1555 | + | let wrong_hash = format!("{:x}", Sha256::digest(b"what we hashed earlier")); | |
| 1556 | + | ||
| 1557 | + | let result = store.import_hashed(&src, &wrong_hash, 24, &db); | |
| 1558 | + | assert!( | |
| 1559 | + | matches!(result, Err(CoreError::HashMismatch(_))), | |
| 1560 | + | "expected HashMismatch, got: {result:?}" | |
| 1561 | + | ); | |
| 1562 | + | ||
| 1563 | + | // No blob may exist at the wrong content address, and no row inserted. | |
| 1564 | + | assert!(!store.sample_path(&wrong_hash, "wav").unwrap().exists()); | |
| 1565 | + | let count: i64 = db | |
| 1566 | + | .conn() | |
| 1567 | + | .query_row("SELECT COUNT(*) FROM samples", [], |row| row.get(0)) | |
| 1568 | + | .unwrap(); | |
| 1569 | + | assert_eq!(count, 0); | |
| 1570 | + | // And no temp file leaked in the store directory. | |
| 1571 | + | let leaked = fs::read_dir(store.root()) | |
| 1572 | + | .into_iter() | |
| 1573 | + | .flatten() | |
| 1574 | + | .flatten() | |
| 1575 | + | .any(|e| e.file_name().to_string_lossy().contains(".tmp")); | |
| 1576 | + | assert!(!leaked, "a .tmp file leaked after the rejected import"); | |
| 1577 | + | } | |
| 1578 | + | ||
| 1579 | + | #[test] | |
| 1580 | + | fn import_hashed_accepts_matching_hash() { | |
| 1581 | + | let (dir, db, store) = setup(); | |
| 1582 | + | let src = create_test_file(&dir, "clap.wav", b"clap bytes"); | |
| 1583 | + | let (hash, size) = hash_file(&src).unwrap(); | |
| 1584 | + | store.import_hashed(&src, &hash, size, &db).unwrap(); | |
| 1585 | + | assert!(store.verify_sample(&hash, "wav").unwrap()); | |
| 1586 | + | } | |
| 1587 | + | ||
| 1588 | + | #[test] | |
| 1589 | + | fn scrub_passes_a_clean_store() { | |
| 1590 | + | let (dir, db, store) = setup(); | |
| 1591 | + | let a = store | |
| 1592 | + | .import(&create_test_file(&dir, "a.wav", b"aaaa"), &db) | |
| 1593 | + | .unwrap(); | |
| 1594 | + | let b = store | |
| 1595 | + | .import(&create_test_file(&dir, "b.wav", b"bbbb"), &db) | |
| 1596 | + | .unwrap(); | |
| 1597 | + | assert_ne!(a, b); | |
| 1598 | + | ||
| 1599 | + | let (checked, corrupt) = store.scrub(&db).unwrap(); | |
| 1600 | + | assert_eq!(checked, 2); | |
| 1601 | + | assert!(corrupt.is_empty()); | |
| 1602 | + | } | |
| 1603 | + | ||
| 1604 | + | #[test] | |
| 1605 | + | fn scrub_reports_a_corrupt_blob() { | |
| 1606 | + | let (dir, db, store) = setup(); | |
| 1607 | + | store | |
| 1608 | + | .import(&create_test_file(&dir, "good.wav", b"good"), &db) | |
| 1609 | + | .unwrap(); | |
| 1610 | + | let bad = store | |
| 1611 | + | .import(&create_test_file(&dir, "bad.wav", b"original"), &db) | |
| 1612 | + | .unwrap(); | |
| 1613 | + | ||
| 1614 | + | // Corrupt the stored blob in place (clear read-only first, as the store | |
| 1615 | + | // marks canonical blobs read-only). | |
| 1616 | + | let path = store.sample_path(&bad, "wav").unwrap(); | |
| 1617 | + | let mut perms = fs::metadata(&path).unwrap().permissions(); | |
| 1618 | + | #[allow(clippy::permissions_set_readonly_false)] | |
| 1619 | + | perms.set_readonly(false); | |
| 1620 | + | fs::set_permissions(&path, perms).unwrap(); | |
| 1621 | + | fs::write(&path, b"tampered").unwrap(); | |
| 1622 | + | ||
| 1623 | + | let (checked, corrupt) = store.scrub(&db).unwrap(); | |
| 1624 | + | assert_eq!(checked, 2); | |
| 1625 | + | assert_eq!(corrupt, vec![bad]); | |
| 1626 | + | } | |
| 1627 | + | ||
| 1628 | + | #[test] | |
| 1629 | + | fn scrub_flags_a_missing_blob_as_corrupt() { | |
| 1630 | + | let (dir, db, store) = setup(); | |
| 1631 | + | let hash = store | |
| 1632 | + | .import(&create_test_file(&dir, "gone.wav", b"here now"), &db) | |
| 1633 | + | .unwrap(); | |
| 1634 | + | let path = store.sample_path(&hash, "wav").unwrap(); | |
| 1635 | + | let mut perms = fs::metadata(&path).unwrap().permissions(); | |
| 1636 | + | #[allow(clippy::permissions_set_readonly_false)] | |
| 1637 | + | perms.set_readonly(false); | |
| 1638 | + | fs::set_permissions(&path, perms).unwrap(); | |
| 1639 | + | fs::remove_file(&path).unwrap(); | |
| 1640 | + | ||
| 1641 | + | let (checked, corrupt) = store.scrub(&db).unwrap(); | |
| 1642 | + | assert_eq!(checked, 1); | |
| 1643 | + | assert_eq!(corrupt, vec![hash]); | |
| 1644 | + | } | |
| 1645 | + | ||
| 1646 | + | #[test] | |
| 1474 | 1647 | fn import_rejects_zero_byte_file() { | |
| 1475 | 1648 | let (dir, db, store) = setup(); | |
| 1476 | 1649 | let src = create_test_file(&dir, "empty.wav", b""); |
| @@ -6,7 +6,7 @@ use synckit_client::{ChangeEntry, ChangeOp, DeviceId, PulledChange}; | |||
| 6 | 6 | ||
| 7 | 7 | use tracing::instrument; | |
| 8 | 8 | ||
| 9 | - | use crate::error::Result; | |
| 9 | + | use crate::error::{Result, SyncError}; | |
| 10 | 10 | ||
| 11 | 11 | use super::{json_to_sql, pk_columns, table_columns, UPSERT_ORDER, DELETE_ORDER}; | |
| 12 | 12 | ||
| @@ -152,11 +152,32 @@ pub(crate) fn apply_remote_changes(conn: &Connection, changes: &[ChangeEntry]) - | |||
| 152 | 152 | for change in &upserts { | |
| 153 | 153 | if change.table == *table | |
| 154 | 154 | && let Some(data) = &change.data { | |
| 155 | - | apply_upsert(&tx, table, data)?; | |
| 156 | - | // Record the applied remote's HLC as this row's committed clock, | |
| 157 | - | // in the same transaction, so a later stale remote is gated out. | |
| 158 | - | super::hlc::set_committed(&tx, table, &change.row_id, &change.hlc)?; | |
| 159 | - | count += 1; | |
| 155 | + | // A single constraint-violating remote row (e.g. an orphan | |
| 156 | + | // whose FK parent this device never received) must not fail | |
| 157 | + | // the whole apply: that would leave `pull_changes` erroring | |
| 158 | + | // before it advances `pull_cursor`, so the next pull refetches | |
| 159 | + | // the same bad batch forever — a permanent non-convergence | |
| 160 | + | // wedge a hostile or buggy server could trigger. Skip-and-log | |
| 161 | + | // the bad row and keep applying the rest. Genuine infra errors | |
| 162 | + | // (I/O, corruption) still propagate and roll the tx back. | |
| 163 | + | match apply_upsert(&tx, table, data) { | |
| 164 | + | Ok(()) => { | |
| 165 | + | // Record the applied remote's HLC as this row's committed | |
| 166 | + | // clock, in the same transaction, so a later stale remote | |
| 167 | + | // is gated out. | |
| 168 | + | super::hlc::set_committed(&tx, table, &change.row_id, &change.hlc)?; | |
| 169 | + | count += 1; | |
| 170 | + | } | |
| 171 | + | Err(e) if is_constraint_violation(&e) => { | |
| 172 | + | tracing::warn!( | |
| 173 | + | table = *table, | |
| 174 | + | row_id = %change.row_id, | |
| 175 | + | "skipping remote upsert that violates a local constraint \ | |
| 176 | + | (e.g. FK-orphan row); advancing so sync converges: {e}" | |
| 177 | + | ); | |
| 178 | + | } | |
| 179 | + | Err(e) => return Err(e), | |
| 180 | + | } | |
| 160 | 181 | } | |
| 161 | 182 | } | |
| 162 | 183 | } | |
| @@ -168,9 +189,21 @@ pub(crate) fn apply_remote_changes(conn: &Connection, changes: &[ChangeEntry]) - | |||
| 168 | 189 | for table in DELETE_ORDER { | |
| 169 | 190 | for change in &deletes { | |
| 170 | 191 | if change.table == *table { | |
| 171 | - | apply_delete(&tx, table, &change.row_id, change.data.as_ref())?; | |
| 172 | - | super::hlc::set_committed(&tx, table, &change.row_id, &change.hlc)?; | |
| 173 | - | count += 1; | |
| 192 | + | match apply_delete(&tx, table, &change.row_id, change.data.as_ref()) { | |
| 193 | + | Ok(()) => { | |
| 194 | + | super::hlc::set_committed(&tx, table, &change.row_id, &change.hlc)?; | |
| 195 | + | count += 1; | |
| 196 | + | } | |
| 197 | + | Err(e) if is_constraint_violation(&e) => { | |
| 198 | + | tracing::warn!( | |
| 199 | + | table = *table, | |
| 200 | + | row_id = %change.row_id, | |
| 201 | + | "skipping remote delete that violates a local constraint; \ | |
| 202 | + | advancing so sync converges: {e}" | |
| 203 | + | ); | |
| 204 | + | } | |
| 205 | + | Err(e) => return Err(e), | |
| 206 | + | } | |
| 174 | 207 | } | |
| 175 | 208 | } | |
| 176 | 209 | } | |
| @@ -186,6 +219,30 @@ pub(crate) fn apply_remote_changes(conn: &Connection, changes: &[ChangeEntry]) - | |||
| 186 | 219 | Ok(count) | |
| 187 | 220 | } | |
| 188 | 221 | ||
| 222 | + | /// True when `e` is a SQLite constraint violation (FK, NOT NULL, CHECK, UNIQUE, | |
| 223 | + | /// PK). These are attributable to the *data* in a single remote row — a bad row | |
| 224 | + | /// is skipped so it can't wedge convergence — whereas any other error (I/O, | |
| 225 | + | /// corruption, locking) indicates a real problem and is propagated to roll the | |
| 226 | + | /// apply back. | |
| 227 | + | fn is_constraint_violation(e: &SyncError) -> bool { | |
| 228 | + | matches!( | |
| 229 | + | e, | |
| 230 | + | SyncError::Db(rusqlite::Error::SqliteFailure(err, _)) | |
| 231 | + | if err.code == rusqlite::ErrorCode::ConstraintViolation | |
| 232 | + | ) | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | /// A `user_config` key the sync boundary must never carry — the exact set the | |
| 236 | + | /// *export* side refuses to emit (`db.rs` triggers + `state.rs` snapshot query: | |
| 237 | + | /// `key NOT LIKE 'sync_%' AND key != 'loose_files'`). The import path must be | |
| 238 | + | /// symmetric: a compromised or malicious server can put any `{key, value}` into | |
| 239 | + | /// a pull batch, and `loose_files` flips the vault into reference-in-place mode | |
| 240 | + | /// while the `sync_*` keys are sync-internal bookkeeping (cursors, tokens). If | |
| 241 | + | /// export won't send them, import won't accept them. See fuzz-2026-07-06 #2. | |
| 242 | + | pub(crate) fn config_key_excluded_from_sync(key: &str) -> bool { | |
| 243 | + | key.starts_with("sync_") || key == "loose_files" | |
| 244 | + | } | |
| 245 | + | ||
| 189 | 246 | /// Apply an upsert (INSERT OR REPLACE) for a single row. | |
| 190 | 247 | pub(crate) fn apply_upsert( | |
| 191 | 248 | conn: &Connection, | |
| @@ -202,6 +259,19 @@ pub(crate) fn apply_upsert( | |||
| 202 | 259 | None => return Ok(()), | |
| 203 | 260 | }; | |
| 204 | 261 | ||
| 262 | + | // Symmetric with the export denylist: never let a remote batch write a | |
| 263 | + | // sync-excluded config key into user_config. Dropping the row (rather than | |
| 264 | + | // erroring the whole apply) matches how an unknown table is handled above — | |
| 265 | + | // the batch continues, the sensitive key just never lands. | |
| 266 | + | if table == "user_config" | |
| 267 | + | && obj | |
| 268 | + | .get("key") | |
| 269 | + | .and_then(|k| k.as_str()) | |
| 270 | + | .is_some_and(config_key_excluded_from_sync) | |
| 271 | + | { | |
| 272 | + | return Ok(()); | |
| 273 | + | } | |
| 274 | + | ||
| 205 | 275 | let mut col_names = Vec::new(); | |
| 206 | 276 | let mut placeholders = Vec::new(); | |
| 207 | 277 | let mut values: Vec<Box<dyn rusqlite::types::ToSql>> = Vec::new(); | |
| @@ -303,6 +373,21 @@ pub(crate) fn apply_delete( | |||
| 303 | 373 | return Ok(()); | |
| 304 | 374 | } | |
| 305 | 375 | ||
| 376 | + | // Symmetric with the export denylist (see `config_key_excluded_from_sync`): | |
| 377 | + | // a remote batch must not *delete* a sync-excluded config key either. The | |
| 378 | + | // canonical key rides in `data` (M018+ DELETE triggers emit | |
| 379 | + | // `json_object('key', OLD.key)`); a pre-M018 client can't reference these | |
| 380 | + | // keys anyway since the export side never emitted them. | |
| 381 | + | if table == "user_config" | |
| 382 | + | && data | |
| 383 | + | .and_then(|v| v.as_object()) | |
| 384 | + | .and_then(|o| o.get("key")) | |
| 385 | + | .and_then(|k| k.as_str()) | |
| 386 | + | .is_some_and(config_key_excluded_from_sync) | |
| 387 | + | { | |
| 388 | + | return Ok(()); | |
| 389 | + | } | |
| 390 | + | ||
| 306 | 391 | let pks = pk_columns(table); | |
| 307 | 392 | ||
| 308 | 393 | // Prefer the `data` JSON: M018+ DELETE triggers always emit it, and it | |
| @@ -506,4 +591,161 @@ mod tests { | |||
| 506 | 591 | assert_eq!(onset, 0.5); | |
| 507 | 592 | assert_eq!(attack, 0.02); | |
| 508 | 593 | } | |
| 594 | + | ||
| 595 | + | fn user_config_conn() -> Connection { | |
| 596 | + | let conn = Connection::open_in_memory().unwrap(); | |
| 597 | + | conn.execute_batch( | |
| 598 | + | "CREATE TABLE user_config (key TEXT PRIMARY KEY, value TEXT NOT NULL);", | |
| 599 | + | ) | |
| 600 | + | .unwrap(); | |
| 601 | + | conn | |
| 602 | + | } | |
| 603 | + | ||
| 604 | + | fn config_value(conn: &Connection, key: &str) -> Option<String> { | |
| 605 | + | conn.query_row( | |
| 606 | + | "SELECT value FROM user_config WHERE key = ?1", | |
| 607 | + | [key], | |
| 608 | + | |r| r.get(0), | |
| 609 | + | ) | |
| 610 | + | .ok() | |
| 611 | + | } | |
| 612 | + | ||
| 613 | + | #[test] | |
| 614 | + | fn config_key_exclusion_matches_export_denylist() { | |
| 615 | + | assert!(config_key_excluded_from_sync("loose_files")); | |
| 616 | + | assert!(config_key_excluded_from_sync("sync_cursor")); | |
| 617 | + | assert!(config_key_excluded_from_sync("sync_")); | |
| 618 | + | assert!(!config_key_excluded_from_sync("sample_tombstone_retain_days")); | |
| 619 | + | assert!(!config_key_excluded_from_sync("theme")); | |
| 620 | + | // Not a prefix match beyond the literal — a benign key that merely | |
| 621 | + | // contains "sync" mid-string is allowed. | |
| 622 | + | assert!(!config_key_excluded_from_sync("autosync_hint")); | |
| 623 | + | } | |
| 624 | + | ||
| 625 | + | #[test] | |
| 626 | + | fn apply_upsert_drops_excluded_config_keys_from_remote() { | |
| 627 | + | let conn = user_config_conn(); | |
| 628 | + | ||
| 629 | + | // A hostile server tries to flip the vault into loose-files mode and to | |
| 630 | + | // inject a sync-internal key. Both must be silently dropped on import. | |
| 631 | + | apply_upsert( | |
| 632 | + | &conn, | |
| 633 | + | "user_config", | |
| 634 | + | &serde_json::json!({"key": "loose_files", "value": "1"}), | |
| 635 | + | ) | |
| 636 | + | .unwrap(); | |
| 637 | + | apply_upsert( | |
| 638 | + | &conn, | |
| 639 | + | "user_config", | |
| 640 | + | &serde_json::json!({"key": "sync_pull_cursor", "value": "999"}), | |
| 641 | + | ) | |
| 642 | + | .unwrap(); | |
| 643 | + | assert_eq!(config_value(&conn, "loose_files"), None); | |
| 644 | + | assert_eq!(config_value(&conn, "sync_pull_cursor"), None); | |
| 645 | + | ||
| 646 | + | // A benign, syncable key still applies. | |
| 647 | + | apply_upsert( | |
| 648 | + | &conn, | |
| 649 | + | "user_config", | |
| 650 | + | &serde_json::json!({"key": "sample_tombstone_retain_days", "value": "45"}), | |
| 651 | + | ) | |
| 652 | + | .unwrap(); | |
| 653 | + | assert_eq!( | |
| 654 | + | config_value(&conn, "sample_tombstone_retain_days").as_deref(), | |
| 655 | + | Some("45") | |
| 656 | + | ); | |
| 657 | + | } | |
| 658 | + | ||
| 659 | + | #[test] | |
| 660 | + | fn apply_upsert_cannot_overwrite_existing_excluded_key() { | |
| 661 | + | let conn = user_config_conn(); | |
| 662 | + | // The user has loose-files mode off locally; a remote UPSERT must not be | |
| 663 | + | // able to turn it on. | |
| 664 | + | conn.execute( | |
| 665 | + | "INSERT INTO user_config (key, value) VALUES ('loose_files', '0')", | |
| 666 | + | [], | |
| 667 | + | ) | |
| 668 | + | .unwrap(); | |
| 669 | + | apply_upsert( | |
| 670 | + | &conn, | |
| 671 | + | "user_config", | |
| 672 | + | &serde_json::json!({"key": "loose_files", "value": "1"}), | |
| 673 | + | ) | |
| 674 | + | .unwrap(); | |
| 675 | + | assert_eq!(config_value(&conn, "loose_files").as_deref(), Some("0")); | |
| 676 | + | } | |
| 677 | + | ||
| 678 | + | #[test] | |
| 679 | + | fn apply_skips_fk_violating_row_and_keeps_applying() { | |
| 680 | + | // A hostile or buggy server sends a `tags` row referencing a sample this | |
| 681 | + | // device never received (FK-orphan) alongside a valid one. The orphan | |
| 682 | + | // must be skipped — not abort the whole apply — so `pull_changes` can | |
| 683 | + | // advance its cursor and sync converges instead of wedging forever. | |
| 684 | + | use audiofiles_core::db::Database; | |
| 685 | + | let db = Database::open_in_memory().unwrap(); | |
| 686 | + | let conn = db.conn(); | |
| 687 | + | conn.pragma_update(None, "foreign_keys", true).unwrap(); | |
| 688 | + | conn.execute( | |
| 689 | + | "INSERT INTO samples (hash, original_name, file_extension, file_size, import_date, last_modified) \ | |
| 690 | + | VALUES ('goodhash', 'a.wav', 'wav', 1, 0, 0)", | |
| 691 | + | [], | |
| 692 | + | ) | |
| 693 | + | .unwrap(); | |
| 694 | + | ||
| 695 | + | let hlc = Hlc { wall_ms: 100, counter: 0, node: Uuid::from_u128(1) }; | |
| 696 | + | let mk = |sample_hash: &str, tag: &str| ChangeEntry { | |
| 697 | + | table: "tags".into(), | |
| 698 | + | op: ChangeOp::Insert, | |
| 699 | + | row_id: format!("{sample_hash}:{tag}"), | |
| 700 | + | timestamp: chrono::Utc::now(), | |
| 701 | + | hlc, | |
| 702 | + | data: Some(serde_json::json!({"sample_hash": sample_hash, "tag": tag})), | |
| 703 | + | extra: Default::default(), | |
| 704 | + | }; | |
| 705 | + | ||
| 706 | + | let applied = | |
| 707 | + | apply_remote_changes(conn, &[mk("missinghash", "drums"), mk("goodhash", "kick")]) | |
| 708 | + | .unwrap(); | |
| 709 | + | assert_eq!(applied, 1, "only the valid row should apply"); | |
| 710 | + | ||
| 711 | + | let good: i64 = conn | |
| 712 | + | .query_row("SELECT COUNT(*) FROM tags WHERE tag = 'kick'", [], |r| r.get(0)) | |
| 713 | + | .unwrap(); | |
| 714 | + | assert_eq!(good, 1); | |
| 715 | + | let orphan: i64 = conn | |
| 716 | + | .query_row("SELECT COUNT(*) FROM tags WHERE tag = 'drums'", [], |r| r.get(0)) | |
| 717 | + | .unwrap(); | |
| 718 | + | assert_eq!(orphan, 0); | |
| 719 | + | } | |
| 720 | + | ||
| 721 | + | #[test] | |
| 722 | + | fn apply_delete_drops_excluded_config_keys_from_remote() { | |
| 723 | + | let conn = user_config_conn(); | |
| 724 | + | conn.execute( | |
| 725 | + | "INSERT INTO user_config (key, value) VALUES ('loose_files', '0'), \ | |
| 726 | + | ('sample_tombstone_retain_days', '30')", | |
| 727 | + | [], | |
| 728 | + | ) | |
| 729 | + | .unwrap(); | |
| 730 | + | ||
| 731 | + | // Remote tries to delete the local loose_files guard row — must be ignored. | |
| 732 | + | apply_delete( | |
| 733 | + | &conn, | |
| 734 | + | "user_config", | |
| 735 | + | "hashed-row-id", | |
| 736 | + | Some(&serde_json::json!({"key": "loose_files"})), | |
| 737 | + | ) | |
| 738 | + | .unwrap(); | |
| 739 | + | assert_eq!(config_value(&conn, "loose_files").as_deref(), Some("0")); | |
| 740 | + | ||
| 741 | + | // A benign key delete still applies. | |
| 742 | + | apply_delete( | |
| 743 | + | &conn, | |
| 744 | + | "user_config", | |
| 745 | + | "hashed-row-id", | |
| 746 | + | Some(&serde_json::json!({"key": "sample_tombstone_retain_days"})), | |
| 747 | + | ) | |
| 748 | + | .unwrap(); | |
| 749 | + | assert_eq!(config_value(&conn, "sample_tombstone_retain_days"), None); | |
| 750 | + | } | |
| 509 | 751 | } |