From 6ca94cbff09fe36b2ec6099928fb62d130f14ded Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 13 May 2026 14:29:28 +0100 Subject: [PATCH] =?UTF-8?q?agent:=20code-atomiser-fix=20=E2=80=94=20paths.?= =?UTF-8?q?rs=20multi-legacy-candidate=20migration=20+=20copy=5Fdir=5Frecu?= =?UTF-8?q?rsive=20symlink=20loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reversibility defects in `crates/core/src/paths.rs`: Defect A (multi-legacy-candidate orphan): `resolve_app_data_dir` and `legacy_and_target_paths` short-circuited on the first legacy candidate, allowing two reachable orphan scenarios on Linux. With both `~/.magnotia` and `~/.local/share/magnotia` the shim migrated only the dot-home variant, leaving the XDG legacy invisible forever. With a stray `~/.lumotia` alongside a freshly migrated `~/.local/share/lumotia`, the resolver kept returning the dot-home path, orphaning the XDG target. `legacy_and_target_paths` now returns `Vec<(legacy, target)>`, probing every legacy variant the platform supports. The migration driver in `src-tauri/src/lib.rs` loops over the Vec and emits per-candidate tracing. A new `resolve_app_data_dir_strict` + `check_target_ambiguity` API refuses to start when more than one target candidate exists post-migration, surfacing both paths to the user via the setup hook instead of silently picking one. Regression tests: `migrate_handles_both_dot_home_and_xdg`, `resolve_app_data_dir_refuses_on_multiple_targets`. Defect B (copy_dir_recursive symlink loop on EXDEV migration): `entry.metadata()` follows symlinks, so a directory symlink reported is_dir==true and recursed unconditionally. A self-referential or ancestor-targeting directory symlink would loop until the disk filled. Switched to `entry.file_type()` (symlink-aware), re-ordered branches so `is_symlink()` is checked first, and routed all symlinks through symlink-creation (Unix + Windows) rather than recursive copy. Regression tests: `copy_dir_recursive_does_not_loop_on_self_referential_dir_symlink`, `copy_dir_recursive_preserves_directory_symlinks`. 14/14 paths tests green. Full workspace cargo test green. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/core/src/paths.rs | 603 +++++++++++++++++++++++++++++++++------ src-tauri/src/lib.rs | 45 ++- 2 files changed, 549 insertions(+), 99 deletions(-) diff --git a/crates/core/src/paths.rs b/crates/core/src/paths.rs index ae84ddc..0f9d2a8 100644 --- a/crates/core/src/paths.rs +++ b/crates/core/src/paths.rs @@ -63,7 +63,156 @@ pub fn app_data_dir() -> PathBuf { app_paths().app_data_dir() } +/// Surfaced when two or more lumotia data-dir candidates exist on disk +/// simultaneously (e.g. both `~/.lumotia` and `~/.local/share/lumotia`). +/// Picking one silently risks pointing at the wrong copy of the user's +/// transcripts. The caller (typically the Tauri setup hook) should refuse +/// to start and surface the paths to the user for manual consolidation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TargetAmbiguityError { + pub candidates: Vec, +} + +impl std::fmt::Display for TargetAmbiguityError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "ambiguous lumotia data directory — multiple candidate paths exist: {}. \ + Please consolidate manually (move data into one path and delete the other) \ + then restart.", + self.candidates + .iter() + .map(|p| p.display().to_string()) + .collect::>() + .join(", ") + ) + } +} + +impl std::error::Error for TargetAmbiguityError {} + fn resolve_app_data_dir() -> PathBuf { + match resolve_app_data_dir_strict() { + Ok(p) => p, + Err(e) => { + // Refuse to start rather than silently picking one of several + // candidate target paths. This is intentionally a panic — the + // process must not be allowed to begin writing into the wrong + // half of a split data directory. The setup hook also calls + // `check_target_ambiguity` explicitly to surface this error + // before tracing/log subsystems are spun up. + panic!("{e}"); + } + } +} + +/// Fallible variant of [`resolve_app_data_dir`]: returns the conventional +/// target path for the current platform, or a [`TargetAmbiguityError`] if +/// more than one candidate target path currently exists on disk. +/// +/// Public so that the application setup hook can perform the check +/// explicitly (and report the ambiguity through tracing) rather than +/// relying on the panic that backs the infallible `resolve_app_data_dir`. +pub fn resolve_app_data_dir_strict() -> Result { + let candidates = target_data_dir_candidates(); + let existing: Vec = candidates + .iter() + .filter(|p| p.exists()) + .cloned() + .collect(); + if existing.len() > 1 { + return Err(TargetAmbiguityError { + candidates: existing, + }); + } + // If exactly one candidate exists, prefer it (it's where the user's + // data lives). If none exist, fall through to the platform-canonical + // path so a fresh install creates the right convention. + if existing.len() == 1 { + return Ok(existing.into_iter().next().unwrap()); + } + Ok(canonical_target_data_dir()) +} + +/// Public counterpart to [`resolve_app_data_dir_strict`] returning `Ok(())` +/// when the data dir is unambiguous and the [`TargetAmbiguityError`] +/// otherwise. Useful when the caller just wants to fail-fast at boot +/// without yet caring about the path itself. +pub fn check_target_ambiguity() -> Result<(), TargetAmbiguityError> { + resolve_app_data_dir_strict().map(|_| ()) +} + +/// All conventional lumotia data-dir target paths for the current +/// platform. Lumotia chooses one canonical path at install time, but a +/// previous magnotia install or a hand-edited XDG_DATA_HOME can leave +/// data in any of these — the migration driver probes them all and the +/// resolver refuses to start if more than one survives. +fn target_data_dir_candidates() -> Vec { + let mut out = Vec::new(); + + #[cfg(target_os = "windows")] + { + if let Ok(local_app_data) = std::env::var("LOCALAPPDATA") { + if !local_app_data.is_empty() { + out.push(PathBuf::from(local_app_data).join("lumotia")); + } + } + } + + #[cfg(target_os = "macos")] + { + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + out.push( + PathBuf::from(home) + .join("Library") + .join("Application Support") + .join("Lumotia"), + ); + } + } + } + + #[cfg(target_os = "linux")] + { + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + out.push(PathBuf::from(&home).join(".lumotia")); + if let Ok(xdg) = std::env::var("XDG_DATA_HOME") { + if !xdg.is_empty() { + out.push(PathBuf::from(xdg).join("lumotia")); + } + } + out.push( + PathBuf::from(home) + .join(".local") + .join("share") + .join("lumotia"), + ); + } + } + } + + #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))] + { + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + out.push(PathBuf::from(home).join(".lumotia")); + } + } + } + + // De-duplicate while preserving order: on Linux XDG_DATA_HOME may be + // set to `~/.local/share` explicitly, in which case the explicit XDG + // candidate and the XDG default collapse to one path. + let mut seen = std::collections::HashSet::new(); + out.retain(|p| seen.insert(p.clone())); + out +} + +/// The single canonical target path for the current platform — what a +/// fresh install would create. Used when no existing candidate is found. +fn canonical_target_data_dir() -> PathBuf { #[cfg(target_os = "windows")] { let local_app_data = std::env::var("LOCALAPPDATA").unwrap_or_else(|_| ".".to_string()); @@ -82,10 +231,6 @@ fn resolve_app_data_dir() -> PathBuf { #[cfg(target_os = "linux")] { let home = std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string()); - let legacy_dot = PathBuf::from(&home).join(".lumotia"); - if legacy_dot.exists() { - return legacy_dot; - } if let Ok(xdg) = std::env::var("XDG_DATA_HOME") { if !xdg.is_empty() { return PathBuf::from(xdg).join("lumotia"); @@ -122,98 +267,148 @@ pub enum MigrationStatus { NoLegacyFound, } -/// Probe the legacy magnotia data dir paths on the current platform. -/// Returns the matched legacy path AND its convention-preserving lumotia -/// target so the migration lands the same kind of dir it found (dot-home -/// stays dot-home, XDG stays XDG, macOS Application Support stays the -/// same). -fn legacy_and_target_paths() -> Option<(PathBuf, PathBuf)> { +/// Probe ALL legacy magnotia data dir paths on the current platform. +/// Returns one (legacy, target) pair per legacy candidate that exists on +/// disk. The target is convention-preserving so the migration lands the +/// same kind of dir it found (dot-home stays dot-home, XDG stays XDG, +/// macOS Application Support stays the same). +/// +/// Previously this returned `Option<(legacy, target)>` and short-circuited +/// on the first match. On Linux that allowed a user with both +/// `~/.magnotia` AND `~/.local/share/magnotia` to migrate only one, +/// leaving the other orphaned forever (subsequent boots prefer the new +/// `~/.lumotia` so the XDG legacy is invisible). Now every legacy variant +/// is probed and migrated independently. +fn legacy_and_target_paths() -> Vec<(PathBuf, PathBuf)> { + let mut out = Vec::new(); + #[cfg(target_os = "windows")] { - let local_app_data = std::env::var("LOCALAPPDATA").ok()?; - let legacy = PathBuf::from(&local_app_data).join("magnotia"); - let target = PathBuf::from(local_app_data).join("lumotia"); - return legacy.exists().then_some((legacy, target)); + if let Ok(local_app_data) = std::env::var("LOCALAPPDATA") { + if !local_app_data.is_empty() { + let legacy = PathBuf::from(&local_app_data).join("magnotia"); + let target = PathBuf::from(local_app_data).join("lumotia"); + if legacy.exists() { + out.push((legacy, target)); + } + } + } } #[cfg(target_os = "macos")] { - let home = std::env::var("HOME").ok()?; - let app_support = PathBuf::from(home).join("Library").join("Application Support"); - let legacy = app_support.join("Magnotia"); - let target = app_support.join("Lumotia"); - return legacy.exists().then_some((legacy, target)); + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + let app_support = PathBuf::from(home) + .join("Library") + .join("Application Support"); + let legacy = app_support.join("Magnotia"); + let target = app_support.join("Lumotia"); + if legacy.exists() { + out.push((legacy, target)); + } + } + } } #[cfg(target_os = "linux")] { - let home = std::env::var("HOME").ok()?; - let dot_legacy = PathBuf::from(&home).join(".magnotia"); - if dot_legacy.exists() { - return Some((dot_legacy, PathBuf::from(&home).join(".lumotia"))); - } - if let Ok(xdg) = std::env::var("XDG_DATA_HOME") { - if !xdg.is_empty() { - let xdg_legacy = PathBuf::from(&xdg).join("magnotia"); - if xdg_legacy.exists() { - return Some((xdg_legacy, PathBuf::from(xdg).join("lumotia"))); + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + let dot_legacy = PathBuf::from(&home).join(".magnotia"); + if dot_legacy.exists() { + out.push((dot_legacy, PathBuf::from(&home).join(".lumotia"))); + } + if let Ok(xdg) = std::env::var("XDG_DATA_HOME") { + if !xdg.is_empty() { + let xdg_legacy = PathBuf::from(&xdg).join("magnotia"); + if xdg_legacy.exists() { + out.push((xdg_legacy, PathBuf::from(&xdg).join("lumotia"))); + } + } + } + let xdg_default_legacy = PathBuf::from(&home) + .join(".local") + .join("share") + .join("magnotia"); + if xdg_default_legacy.exists() { + let xdg_default_target = PathBuf::from(&home) + .join(".local") + .join("share") + .join("lumotia"); + out.push((xdg_default_legacy, xdg_default_target)); } } } - let xdg_default_legacy = PathBuf::from(&home) - .join(".local") - .join("share") - .join("magnotia"); - if xdg_default_legacy.exists() { - let xdg_default_target = PathBuf::from(home) - .join(".local") - .join("share") - .join("lumotia"); - return Some((xdg_default_legacy, xdg_default_target)); - } - None } #[cfg(not(any(target_os = "windows", target_os = "macos", target_os = "linux")))] { - let home = std::env::var("HOME").ok()?; - let legacy = PathBuf::from(&home).join(".magnotia"); - let target = PathBuf::from(home).join(".lumotia"); - legacy.exists().then_some((legacy, target)) + if let Ok(home) = std::env::var("HOME") { + if !home.is_empty() { + let legacy = PathBuf::from(&home).join(".magnotia"); + let target = PathBuf::from(home).join(".lumotia"); + if legacy.exists() { + out.push((legacy, target)); + } + } + } } + + // De-duplicate: e.g. XDG_DATA_HOME set explicitly to `~/.local/share` + // would otherwise produce the same pair twice on Linux. + let mut seen = std::collections::HashSet::new(); + out.retain(|pair| seen.insert(pair.clone())); + out } -/// Migrate a legacy magnotia data directory to its convention-preserving -/// lumotia equivalent on first launch. Idempotent: safe to call on every -/// boot. +/// Migrate every legacy magnotia data directory to its +/// convention-preserving lumotia equivalent on first launch. Idempotent: +/// safe to call on every boot. /// -/// Rules: -/// * If the resolved target already exists, do nothing and return -/// `TargetAlreadyExists`. We do not destroy lumotia data, even if a -/// stale legacy dir is also present. -/// * If only the legacy path exists, rename it to the matching lumotia -/// target (same parent dir / same convention) and rename -/// `magnotia.db` -> `lumotia.db` inside it if found. -/// * If neither exists, return `NoLegacyFound` — the first launch on a -/// clean system will create the new path itself. -/// -/// Callers should treat `Err` as a hard startup failure; silently +/// Returns one [`MigrationStatus`] per legacy candidate probed, in +/// platform-deterministic order. An empty Vec means there are no legacy +/// directories on disk (clean install). Callers should log per-candidate +/// outcomes and treat any `Err` as a hard startup failure: silently /// continuing past a migration error orphans user data behind a fresh /// empty lumotia dir. -pub fn migrate_legacy_data_dir() -> Result { +/// +/// Per-candidate rules (same as before, applied independently to each +/// legacy path that exists): +/// * If the matching target already exists, do nothing for that +/// candidate and emit `TargetAlreadyExists`. We do not destroy +/// lumotia data, even if a stale legacy dir is also present. +/// * If only the legacy path exists, rename it to the matching lumotia +/// target (same convention) and rename `magnotia.db` -> `lumotia.db` +/// inside it if found. +pub fn migrate_legacy_data_dir() -> Result, std::io::Error> { migrate_legacy_data_dir_inner(legacy_and_target_paths()) } -/// Test-friendly inner shape: takes the (legacy, target) pair explicitly -/// so tests don't depend on platform-specific HOME / LOCALAPPDATA / XDG -/// env vars. +/// Test-friendly inner shape: takes the list of (legacy, target) pairs +/// explicitly so tests don't depend on platform-specific HOME / +/// LOCALAPPDATA / XDG env vars. +/// +/// An empty input is shorthand for "no legacy on disk" and yields a +/// single [`MigrationStatus::NoLegacyFound`] entry so callers can still +/// rely on a non-empty result to drive their logging. fn migrate_legacy_data_dir_inner( - pair: Option<(PathBuf, PathBuf)>, -) -> Result { - let Some((from, to)) = pair else { - return Ok(MigrationStatus::NoLegacyFound); - }; + pairs: Vec<(PathBuf, PathBuf)>, +) -> Result, std::io::Error> { + if pairs.is_empty() { + return Ok(vec![MigrationStatus::NoLegacyFound]); + } + let mut out = Vec::with_capacity(pairs.len()); + for (from, to) in pairs { + out.push(migrate_one(from, to)?); + } + Ok(out) +} +/// Run the single-candidate migration. Extracted so the driver can loop +/// over every legacy path discovered on disk and surface per-candidate +/// outcomes individually. +fn migrate_one(from: PathBuf, to: PathBuf) -> Result { if to.exists() { return Ok(MigrationStatus::TargetAlreadyExists { target: to }); } @@ -282,27 +477,41 @@ fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), std::io::Error> { let entry = entry?; let entry_path = entry.path(); let target_path = to.join(entry.file_name()); - let metadata = entry.metadata()?; - if metadata.is_dir() { - copy_dir_recursive(&entry_path, &target_path)?; - } else if metadata.file_type().is_symlink() { + // CRITICAL: use file_type() rather than metadata(). metadata() + // follows symlinks, so a directory symlink reports is_dir==true + // and would recurse unconditionally — a self-referential or + // ancestor-targeting directory symlink loops until the disk + // fills. file_type() is symlink-aware on both Unix and Windows. + let file_type = entry.file_type()?; + if file_type.is_symlink() { // Recreate symlink rather than dereferencing — the // transcription app stores recording paths verbatim so a - // dereferenced symlink could orphan large audio blobs. + // dereferenced symlink could orphan large audio blobs, and + // a directory symlink is the only way to terminate the + // recursion at the link boundary. + let link_target = std::fs::read_link(&entry_path)?; #[cfg(unix)] { - let target = std::fs::read_link(&entry_path)?; - std::os::unix::fs::symlink(target, &target_path)?; + std::os::unix::fs::symlink(link_target, &target_path)?; } #[cfg(windows)] { - let target = std::fs::read_link(&entry_path)?; - if metadata.is_dir() { - std::os::windows::fs::symlink_dir(target, &target_path)?; + // On Windows we have to pick file vs dir symlink at + // creation time. Probe the link target with full + // metadata (it resolves through the link) to decide. + // If the target is missing or unreadable, fall back to + // a file symlink — safer than panicking the migration. + let target_is_dir = std::fs::metadata(&entry_path) + .map(|m| m.is_dir()) + .unwrap_or(false); + if target_is_dir { + std::os::windows::fs::symlink_dir(link_target, &target_path)?; } else { - std::os::windows::fs::symlink_file(target, &target_path)?; + std::os::windows::fs::symlink_file(link_target, &target_path)?; } } + } else if file_type.is_dir() { + copy_dir_recursive(&entry_path, &target_path)?; } else { std::fs::copy(&entry_path, &target_path)?; } @@ -365,6 +574,21 @@ mod tests { std::env::temp_dir().join(format!("lumotia-paths-test-{base}-{pid}-{nanos}")) } + /// Helper: drive the migration with a single (legacy, target) pair + /// and return the (only) status it produced. Keeps existing tests + /// readable after the Option -> Vec API change. + fn migrate_one_pair_inner( + pair: (PathBuf, PathBuf), + ) -> Result { + let mut statuses = migrate_legacy_data_dir_inner(vec![pair])?; + assert_eq!( + statuses.len(), + 1, + "single-pair driver should yield exactly one status" + ); + Ok(statuses.pop().unwrap()) + } + #[test] fn migrate_with_legacy_present_renames_dir_and_db() { let root = unique_tmp("legacy-present"); @@ -374,8 +598,8 @@ mod tests { std::fs::write(legacy.join("magnotia.db"), b"sqlite-stub").unwrap(); std::fs::write(legacy.join("recordings.placeholder"), b"x").unwrap(); - let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone()))) - .expect("migrate ok"); + let result = + migrate_one_pair_inner((legacy.clone(), target.clone())).expect("migrate ok"); match result { MigrationStatus::Migrated { @@ -411,8 +635,8 @@ mod tests { std::fs::write(target.join("lumotia.db"), b"new-data").unwrap(); std::fs::write(legacy.join("magnotia.db"), b"legacy-data").unwrap(); - let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone()))) - .expect("migrate ok"); + let result = + migrate_one_pair_inner((legacy.clone(), target.clone())).expect("migrate ok"); assert_eq!( result, @@ -433,9 +657,9 @@ mod tests { #[test] fn migrate_with_neither_present_returns_no_legacy() { - let result = migrate_legacy_data_dir_inner(None).expect("migrate ok"); + let result = migrate_legacy_data_dir_inner(Vec::new()).expect("migrate ok"); - assert_eq!(result, MigrationStatus::NoLegacyFound); + assert_eq!(result, vec![MigrationStatus::NoLegacyFound]); } #[test] @@ -446,8 +670,8 @@ mod tests { std::fs::create_dir_all(&legacy).unwrap(); std::fs::write(legacy.join("recordings.placeholder"), b"x").unwrap(); - let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone()))) - .expect("migrate ok"); + let result = + migrate_one_pair_inner((legacy.clone(), target.clone())).expect("migrate ok"); match result { MigrationStatus::Migrated { renamed_db, .. } => { @@ -471,8 +695,8 @@ mod tests { std::fs::create_dir_all(&legacy).unwrap(); std::fs::write(legacy.join("magnotia.db"), b"data").unwrap(); - let result = migrate_legacy_data_dir_inner(Some((legacy.clone(), target.clone()))) - .expect("migrate ok"); + let result = + migrate_one_pair_inner((legacy.clone(), target.clone())).expect("migrate ok"); assert!(matches!(result, MigrationStatus::Migrated { .. })); assert!(target.exists()); @@ -555,4 +779,205 @@ mod tests { let err = std::io::Error::from_raw_os_error(18); assert!(is_cross_device(&err)); } + + // ------------------------------------------------------------------ + // Defect A regression tests: multi-legacy-candidate + ambiguity guard + // ------------------------------------------------------------------ + + #[test] + fn migrate_handles_both_dot_home_and_xdg() { + // Reproduces the multi-legacy orphan scenario: a Linux user with + // BOTH `~/.magnotia` and `~/.local/share/magnotia` on disk. The + // old code returned `Option<(legacy, target)>` and short-circuited + // on the dot-home variant, leaving the XDG legacy orphaned. The + // new driver loops over the Vec and migrates every candidate. + let root = unique_tmp("both-legacy"); + let dot_legacy = root.join(".magnotia"); + let dot_target = root.join(".lumotia"); + let xdg_legacy = root.join(".local/share/magnotia"); + let xdg_target = root.join(".local/share/lumotia"); + std::fs::create_dir_all(&dot_legacy).unwrap(); + std::fs::create_dir_all(&xdg_legacy).unwrap(); + std::fs::write(dot_legacy.join("marker"), b"dot-home").unwrap(); + std::fs::write(xdg_legacy.join("marker"), b"xdg").unwrap(); + + let statuses = migrate_legacy_data_dir_inner(vec![ + (dot_legacy.clone(), dot_target.clone()), + (xdg_legacy.clone(), xdg_target.clone()), + ]) + .expect("migrate ok"); + + assert_eq!( + statuses.len(), + 2, + "expected one status per legacy candidate" + ); + for s in &statuses { + assert!( + matches!(s, MigrationStatus::Migrated { .. }), + "expected Migrated, got {s:?}" + ); + } + assert!(!dot_legacy.exists(), "dot-home legacy should be gone"); + assert!(!xdg_legacy.exists(), "XDG legacy should be gone"); + assert!(dot_target.exists(), "dot-home target should exist"); + assert!(xdg_target.exists(), "XDG target should exist"); + assert_eq!( + std::fs::read(dot_target.join("marker")).unwrap(), + b"dot-home".to_vec(), + "dot-home content preserved" + ); + assert_eq!( + std::fs::read(xdg_target.join("marker")).unwrap(), + b"xdg".to_vec(), + "XDG content preserved" + ); + + std::fs::remove_dir_all(&root).ok(); + } + + #[test] + fn resolve_app_data_dir_refuses_on_multiple_targets() { + // Reproduces the stray-dot-home orphan scenario: after a partial + // migration the user may end up with BOTH `~/.lumotia` and + // `~/.local/share/lumotia` on disk. Picking one silently is + // worse than failing fast, so the strict resolver must error + // with both paths surfaced for manual consolidation. + // + // We override HOME so the strict resolver scans inside our + // tempdir, then assert it returns Err with both paths named. + let root = unique_tmp("ambiguous-target"); + let fake_home = root.join("home"); + std::fs::create_dir_all(&fake_home).unwrap(); + let dot = fake_home.join(".lumotia"); + let xdg_default = fake_home.join(".local/share/lumotia"); + std::fs::create_dir_all(&dot).unwrap(); + std::fs::create_dir_all(&xdg_default).unwrap(); + + // Serialise env mutation: HOME / XDG_DATA_HOME are process-global, + // and other tests in this module rely on them being unchanged. + // We restore the previous values before returning. + let prev_home = std::env::var_os("HOME"); + let prev_xdg = std::env::var_os("XDG_DATA_HOME"); + // SAFETY: tests in this module that read HOME serialise on this + // exact pattern (set, call, restore) and the process is otherwise + // single-threaded inside a #[test] body. + std::env::set_var("HOME", &fake_home); + std::env::remove_var("XDG_DATA_HOME"); + + let result = resolve_app_data_dir_strict(); + + // Restore env BEFORE asserting so a panic doesn't poison + // subsequent tests. + match prev_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + if let Some(v) = prev_xdg { + std::env::set_var("XDG_DATA_HOME", v); + } + + let err = result.expect_err("expected ambiguity error"); + assert!( + err.candidates.iter().any(|p| p == &dot), + "error must name dot-home candidate: {err}" + ); + assert!( + err.candidates.iter().any(|p| p == &xdg_default), + "error must name XDG default candidate: {err}" + ); + let msg = err.to_string(); + assert!( + msg.contains("ambiguous"), + "message should flag ambiguity: {msg}" + ); + + std::fs::remove_dir_all(&root).ok(); + } + + // ------------------------------------------------------------------ + // Defect B regression tests: copy_dir_recursive symlink loop + // ------------------------------------------------------------------ + + #[cfg(unix)] + #[test] + fn copy_dir_recursive_does_not_loop_on_self_referential_dir_symlink() { + // The original code used `entry.metadata()` which follows + // symlinks, so a directory symlink reported is_dir==true and + // recursed unconditionally. A self-referential dir symlink would + // then loop until the disk filled. Use file_type() (which does + // NOT follow symlinks), branch on is_symlink() FIRST, and + // recreate the link instead of recursing through it. + let root = unique_tmp("symlink-self"); + let src = root.join("src"); + let dst = root.join("dst"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::write(src.join("regular-file"), b"hello").unwrap(); + // Self-reference: src/oops -> src. + std::os::unix::fs::symlink(&src, src.join("oops")).unwrap(); + + copy_dir_recursive(&src, &dst).expect("copy must terminate, not loop"); + + // The regular file should have been copied. + assert_eq!(std::fs::read(dst.join("regular-file")).unwrap(), b"hello"); + // The self-reference should have been recreated as a symlink, + // NOT as a directory full of recursive copies. + let oops = dst.join("oops"); + let oops_meta = std::fs::symlink_metadata(&oops).expect("oops should exist"); + assert!( + oops_meta.file_type().is_symlink(), + "dst/oops must be a symlink, not a recursive directory copy" + ); + // And the link target must be preserved verbatim. + let link_target = std::fs::read_link(&oops).unwrap(); + assert_eq!(link_target, src, "symlink target should be preserved"); + + std::fs::remove_dir_all(&root).ok(); + } + + #[cfg(unix)] + #[test] + fn copy_dir_recursive_preserves_directory_symlinks() { + // A directory symlink to a real sibling dir must be recreated as + // a symlink in dst (preserving the link-shape), not dereferenced + // into a recursive copy of the sibling's contents. + let root = unique_tmp("symlink-dir"); + let src = root.join("src"); + let sibling = root.join("sibling"); + let dst = root.join("dst"); + std::fs::create_dir_all(&src).unwrap(); + std::fs::create_dir_all(&sibling).unwrap(); + std::fs::write(sibling.join("payload"), b"sibling-data").unwrap(); + // src/link -> sibling (directory symlink). + std::os::unix::fs::symlink(&sibling, src.join("link")).unwrap(); + + copy_dir_recursive(&src, &dst).expect("copy ok"); + + let dst_link = dst.join("link"); + let meta = std::fs::symlink_metadata(&dst_link).expect("dst/link should exist"); + assert!( + meta.file_type().is_symlink(), + "dst/link must remain a symlink, not be replaced with a directory copy" + ); + // Following the link should still resolve to sibling content; + // the link target must be preserved verbatim. + let link_target = std::fs::read_link(&dst_link).unwrap(); + assert_eq!(link_target, sibling, "symlink target should be preserved"); + // And we must NOT have written sibling/payload into dst/link/. + // (If link is a symlink, reading dst/link/payload would follow + // it back to sibling/payload, so check on-disk shape instead.) + let entries: Vec<_> = std::fs::read_dir(&dst).unwrap().collect(); + let dst_link_entry = entries + .iter() + .find_map(|e| e.as_ref().ok()) + .filter(|e| e.file_name() == std::ffi::OsString::from("link")); + if let Some(e) = dst_link_entry { + assert!( + e.file_type().unwrap().is_symlink(), + "directory entry for dst/link must report symlink" + ); + } + + std::fs::remove_dir_all(&root).ok(); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 8e25b35..59f580e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -234,16 +234,28 @@ pub fn run() { // settings behind a fresh empty lumotia dir. let t_migrate = Instant::now(); match migrate_legacy_data_dir() { - Ok(MigrationStatus::Migrated { from, to, renamed_db }) => tracing::info!( - target: "lumotia_startup", - elapsed_ms = t_migrate.elapsed().as_millis(), - from = %from.display(), - to = %to.display(), - renamed_db, - "migrated legacy magnotia data dir to lumotia" - ), - Ok(MigrationStatus::TargetAlreadyExists { .. }) => {} - Ok(MigrationStatus::NoLegacyFound) => {} + Ok(statuses) => { + // Drive every legacy candidate independently: on Linux a + // user may have both `~/.magnotia` and + // `~/.local/share/magnotia`, and migrating only one + // would orphan the other forever. + for status in &statuses { + match status { + MigrationStatus::Migrated { from, to, renamed_db } => { + tracing::info!( + target: "lumotia_startup", + elapsed_ms = t_migrate.elapsed().as_millis(), + from = %from.display(), + to = %to.display(), + renamed_db = *renamed_db, + "migrated legacy magnotia data dir to lumotia" + ); + } + MigrationStatus::TargetAlreadyExists { .. } => {} + MigrationStatus::NoLegacyFound => {} + } + } + } Err(e) => { tracing::error!( target: "lumotia_startup", @@ -254,6 +266,19 @@ pub fn run() { } } + // After migration, refuse to start if more than one lumotia + // target candidate exists on disk (e.g. both `~/.lumotia` AND + // `~/.local/share/lumotia`). Silently picking one would point + // the app at the wrong half of a split data directory. + if let Err(amb) = check_target_ambiguity() { + tracing::error!( + target: "lumotia_startup", + error = %amb, + "ambiguous lumotia data directory — refusing to start" + ); + return Err(Box::new(amb) as Box); + } + // Initialise database and startup settings in one runtime entry. let db_path = database_path(); let (db, init_script) = tauri::async_runtime::block_on(async {