From 1aea62dde04c96912b690f6ed481e166870f81b6 Mon Sep 17 00:00:00 2001 From: Michael Anderson <1427851+endthestart@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:23:37 -0500 Subject: [PATCH 1/2] LC-SAFETY: Refuse to create a library photo's XMP sidecar --- crates/engine/src/originals.rs | 58 +++++++++++++++++++++++++++---- crates/engine/src/tests_export.rs | 20 +++++++++++ docs/whats-new.md | 7 ++++ 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/originals.rs b/crates/engine/src/originals.rs index b3a140a2b..a66c4ec3f 100644 --- a/crates/engine/src/originals.rs +++ b/crates/engine/src/originals.rs @@ -42,10 +42,11 @@ impl OriginalGuard { put(format!("{name}.xmp"), PathBuf::from(format!("{}.xmp", path.to_string_lossy())), format!("the XMP sidecar of {label}")); } - /// `Err` (a message for the user) when writing `target` would replace a protected file. + /// `Err` (a message for the user) when writing `target` would replace a protected file, or + /// create a protected sidecar that isn't there yet. pub fn check(&self, target: &Path) -> Result<(), String> { if std::fs::symlink_metadata(target).is_err() { - return Ok(()); // nothing there to replace + return self.check_new(target); // nothing there to replace } let mut names: Vec = target.file_name().map(|n| n.to_string_lossy().to_lowercase()).into_iter().collect(); if let Some(n) = std::fs::canonicalize(target).ok().and_then(|c| c.file_name().map(|n| n.to_string_lossy().to_lowercase())) { @@ -76,6 +77,30 @@ impl OriginalGuard { } Ok(()) } + + /// A new file with a sidecar's name in that sidecar's folder would be read back as the + /// photo's edits. The folders are compared by identity, so `sub/../` or a link counts. + fn check_new(&self, target: &Path) -> Result<(), String> { + if !is_xmp(target) { + return Ok(()); + } + let Some(name) = target.file_name().map(|n| n.to_string_lossy().to_lowercase()) else { return Ok(()) }; + for (p, what) in self.by_name.get(&name).into_iter().flatten() { + if same_file(folder_of(p), folder_of(target)) { + return Err(format!("{} would become {what} in the library (choose another folder or file name)", target.display())); + } + } + Ok(()) + } +} + +fn is_xmp(path: &Path) -> bool { + path.extension().is_some_and(|e| e.eq_ignore_ascii_case("xmp")) +} + +/// The folder `path` is in (`.` for a bare file name). +fn folder_of(path: &Path) -> &Path { + path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")) } impl crate::Session { @@ -84,12 +109,13 @@ impl crate::Session { OriginalGuard::new(&self.catalog) } - /// `Err` when writing `path` would replace a catalogued photo's original or sidecar. A free - /// path (the usual case) is answered without building the guard over the whole library. + /// `Err` when writing `path` would replace a catalogued photo's original or sidecar, or + /// create one of its sidecars. A free path that isn't an `.xmp` (the usual case) is answered + /// without building the guard over the whole library. pub fn check_write_target(&self, path: &str) -> Result<(), String> { let path = Path::new(path); - if std::fs::symlink_metadata(path).is_err() { - return Ok(()); // nothing there to replace + if std::fs::symlink_metadata(path).is_err() && !is_xmp(path) { + return Ok(()); // nothing there to replace, and no sidecar to create } self.original_guard().check(path) } @@ -120,4 +146,24 @@ mod tests { assert!(g.check(&d.join("new.jpg")).is_ok(), "a free name"); let _ = std::fs::remove_dir_all(&d); } + + #[test] + fn a_sidecar_that_isnt_there_yet_is_protected_too() { + let d = std::env::temp_dir().join(format!("lc-originals-new-sidecar-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(d.join("sub")).unwrap(); + let orig = d.join("IMG_1.jpg"); + std::fs::write(&orig, b"original").unwrap(); + let mut c = Catalog::default(); + let p = Photo::new(PhotoId(1), Source::File { path: orig.to_string_lossy().to_string() }, "IMG_1.jpg", "JPEG", 10, 10, "2026-01-01"); + c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + let g = OriginalGuard::new(&c); + assert!(g.check(&d.join("IMG_1.xmp")).unwrap_err().contains("XMP sidecar of IMG_1.jpg")); + assert!(g.check(&d.join("IMG_1.jpg.xmp")).is_err(), "the appended spelling"); + assert!(g.check(&d.join("sub/../IMG_1.xmp")).is_err(), "another spelling of the folder"); + assert!(g.check(&d.join("sub/IMG_1.xmp")).is_ok(), "the same name in another folder"); + assert!(g.check(&d.join("other.xmp")).is_ok(), "no photo of that name"); + assert!(g.check(&d.join("missing/IMG_1.xmp")).is_ok(), "a folder that isn't there"); + let _ = std::fs::remove_dir_all(&d); + } } diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 4830490b5..08d9266f8 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -313,6 +313,26 @@ fn curve_preset_export_never_overwrites_originals_or_sidecars() { preset_export_preserves_originals("curve.exportPresets", json!({"names": ["Linear"]}), "curve-export-guard"); } +/// A new file named like a photo's sidecar, in the photo's folder, would be read back as that +/// photo's edits: refused even though nothing is there yet. +#[test] +fn preset_export_never_creates_a_photos_sidecar() { + let (mut s, _, dir, _, _) = library_with_jpeg("preset-export-new-sidecar"); + std::fs::create_dir_all(dir.join("presets")).unwrap(); + for name in ["IMG_1.xmp", "IMG_1.jpg.xmp", "presets/../IMG_1.xmp"] { + let path = dir.join(name); + assert!(!path.exists(), "{name} isn't there before the export"); + let error = s.execute("preset.export", &json!({"ids": ["lc.warm-glow"], "path": path.to_string_lossy()})).unwrap_err().to_string(); + assert!(error.contains("XMP sidecar of IMG_1.jpg"), "{error}"); + assert!(!path.exists(), "{name} was not created"); + } + // the same name in another folder is an ordinary preset file + let elsewhere = dir.join("presets/IMG_1.xmp"); + s.execute("preset.export", &json!({"ids": ["lc.warm-glow"], "path": elsewhere.to_string_lossy()})).unwrap(); + assert!(elsewhere.exists()); + let _ = std::fs::remove_dir_all(dir); +} + #[test] fn preset_exports_preserve_previous_files_on_write_failure() { let dir = temp_dir("preset-export-failure"); diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..f6745c6c2 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-22 + +### Exports don't create a photo's sidecar +- Saving an export, a preset or any other file as `IMG_1.xmp` or `IMG_1.jpg.xmp` in the folder of the library photo + `IMG_1.jpg` is now refused when no sidecar is there yet, as writing over an existing one already was. The library + would have read the new file back as that photo's edits. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges From 4c8a7484c6955bdae1137b11dbd346b27bb60bb4 Mon Sep 17 00:00:00 2001 From: Michael Anderson <1427851+endthestart@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:16:55 -0500 Subject: [PATCH 2/2] LC-SAFETY: Refuse a pinned write target that would become a photo's sidecar --- crates/engine/src/originals.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/engine/src/originals.rs b/crates/engine/src/originals.rs index f16d57c71..d7bda0370 100644 --- a/crates/engine/src/originals.rs +++ b/crates/engine/src/originals.rs @@ -229,6 +229,10 @@ impl crate::Session { let parent = p.parent().filter(|d| !d.as_os_str().is_empty()).and_then(|d| std::fs::canonicalize(d).ok()); match std::fs::symlink_metadata(p) { Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + // nothing there yet, but a new file with a photo's sidecar name would be read as its edits + if is_xmp(p) { + self.original_guard().check(p)?; + } Ok(WriteTarget { path: path.to_string(), state: WriteTargetState::Absent, parent }) } Err(error) => Err(format!("{path}: {error}")), @@ -326,6 +330,12 @@ mod tests { assert!(g.check(&d.join("sub/IMG_1.xmp")).is_ok(), "the same name in another folder"); assert!(g.check(&d.join("other.xmp")).is_ok(), "no photo of that name"); assert!(g.check(&d.join("missing/IMG_1.xmp")).is_ok(), "a folder that isn't there"); + // a write pinned ahead of time (a queued screenshot) is refused the same way + let mut s = crate::Session::new(); + s.catalog = c; + let pinned = s.pin_write_target(&d.join("IMG_1.xmp").to_string_lossy()); + assert!(pinned.unwrap_err().contains("XMP sidecar of IMG_1.jpg")); + assert!(s.pin_write_target(&d.join("sub/IMG_1.xmp").to_string_lossy()).is_ok(), "the same name in another folder"); let _ = std::fs::remove_dir_all(&d); }