Merge PR #2658: LC-SAFETY: Refuse to create a library photo's XMP sidecar

crates/engine/src/originals.rs

@@ -127,7 +127,8 @@ 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> {
         self.check_target(target, false)
     }
@@ -141,7 +142,7 @@ impl OriginalGuard {
 
     fn check_target(&self, target: &Path, hard_links: bool) -> 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<String> = 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())) {
@@ -186,6 +187,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 {
@@ -194,12 +219,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)
     }
@@ -214,6 +240,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}")),
@@ -294,6 +324,32 @@ mod tests {
         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");
+        // 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);
+    }
+
     #[test]
     fn write_targets_pin_absent_paths_and_recheck_the_current_library() {
         let d = std::env::temp_dir().join(format!("lc-write-targets-{}", std::process::id()));

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);
+}
+
 /// #1944: Export Keywords went straight to `std::fs::write`, replacing a library original.
 #[test]
 fn keyword_export_never_overwrites_originals_or_sidecars() {

docs/whats-new.md

@@ -95,6 +95,11 @@
 
 ## 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.
+
 ### Reliability
 - Opening a library no longer writes through a link left at the lock's owner note (`catalog.lock.owner`): the link
   is removed and a new note made in its place, so the file it pointed to is never changed (as for the catalog's