#2135: only file types LightCraft reads reach the system opener

Photo ▸ Edit In ▸ Open Original in Other Editor handed a photo's stored path
straight to `open` / `xdg-open` / `explorer.exe`, with `Path::is_file()` as the
only check, and Lightroom catalog import kept original paths exactly as the
catalog had them. A crafted `.lrcat` could therefore put `payload.exe`,
`start.lnk`, `run.bat`, `page.hta`, `Evil.app` or `go.command` into the library
and have it run on one user click.

- `lightcraft_engine::open_path`: one gate for every path that leaves
  LightCraft for the OS. The extension must be one LightCraft reads
  (`import::EXTENSIONS`, which already covers the Edit In copy formats
  `tif`/`tiff`/`psd`), it must be a regular file (so a `.app` bundle and any
  other folder are out), what a link resolves to must be a read type too, and
  what is handed over is the absolute path. A refusal is the sentence shown to
  the user.
- Open Original and Edit In go through it; a refused photo is listed in
  `skipped` with the reason, and the menu item is greyed out for a record that
  can never be opened (extension only, so no disk or share is touched while a
  menu is drawn).
- `lightroom_catalog::refuse_non_media` drops such images where the catalog is
  read and again where its photos become records, with a warning each, so a
  refused entry never becomes an openable library record. A virtual copy of a
  refused original goes with it.

Tests: a catalog naming a program (and a virtual copy of it) imports neither
while the catalog's real photos do; a library record naming a program is never
handed to the stub launcher, which records no invocation, and a program
selected beside a photo leaves only the photo, by absolute path. Both fail
before this change.

Co-Authored-By: Claude <noreply@anthropic.com>

crates/engine/src/lib.rs

@@ -56,6 +56,7 @@ pub mod media;
 pub mod memory;
 pub mod merge;
 mod model_download;
+pub mod open_path;
 pub mod originals;
 mod pdf;
 pub mod preset_import;

crates/engine/src/lightroom_catalog.rs

@@ -513,10 +513,54 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize,
             snapshots: snapshots.remove(&id).unwrap_or_default(),
         });
     }
+    refuse_non_media(&mut out);
     ensure_read_active(cancel)?;
     Ok(out)
 }
 
+/// Drop the catalog's images whose original is not a file type LightCraft reads, saying so in the
+/// import's warnings.
+///
+/// The catalog decides these paths, and a record it leaves behind is one click from the system
+/// opener (Photo ▸ Edit In ▸ Open Original in Other Editor). A record for `payload.exe`,
+/// `start.lnk` or `Evil.app` would make a `.lrcat` someone was sent a way to have a program run
+/// (issue #2135), so such an image never becomes a photo: not imported, not relinkable, not
+/// openable. A virtual copy of a refused original goes with it — it has no original of its own.
+///
+/// Called where the catalog is read and again where its photos become records, so neither route
+/// can keep one; the second pass finds nothing left to say.
+pub(crate) fn refuse_non_media(data: &mut CatalogImport) {
+    // masters first: a virtual copy may come before its master in the table
+    let refused: HashSet<i64> = data
+        .photos
+        .iter()
+        .filter(|p| number(&p.image, "masterImage") == 0 && !crate::open_path::is_media(Path::new(&p.path)))
+        .map(|p| p.source_id)
+        .collect();
+    if refused.is_empty() {
+        return;
+    }
+    let mut warnings = Vec::new();
+    data.photos.retain(|p| {
+        let master = number(&p.image, "masterImage");
+        // `masterImage` is 0 for a master, so a refused id of 0 (a malformed catalog) must not
+        // take every master of the catalog with it
+        if master != 0 {
+            if !refused.contains(&master) {
+                return true;
+            }
+            warnings.push(format!("image {}: not imported; its original was not imported", p.source_id));
+            return false;
+        }
+        if !refused.contains(&p.source_id) {
+            return true;
+        }
+        warnings.push(format!("image {}: not imported; {} isn't one of the file types LightCraft reads", p.source_id, p.path));
+        false
+    });
+    data.warnings.extend(warnings);
+}
+
 fn ensure_read_active(cancel: &AtomicBool) -> Result<(), String> {
     if cancel.load(Ordering::Relaxed) { Err("Lightroom import cancelled".into()) } else { Ok(()) }
 }
@@ -692,8 +736,10 @@ pub(crate) struct ApplyPrepared<'a> {
     pub(crate) undo0: usize,
 }
 
-pub(crate) fn apply_prepared(s: &mut crate::Session, data: CatalogImport, context: ApplyPrepared<'_>) -> crate::Result<AppliedImport> {
+pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, context: ApplyPrepared<'_>) -> crate::Result<AppliedImport> {
     let ApplyPrepared { update_existing, files, index, archive_path, missing_sources, undo0 } = context;
+    // nothing but a file type LightCraft reads becomes a record, whichever route got here (#2135)
+    refuse_non_media(&mut data);
     validate(&data).map_err(error)?;
     let mut warnings = data.warnings.clone();
     let duplicates: HashMap<_, _> = files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), PhotoId(id)))).collect();
@@ -962,6 +1008,48 @@ mod tests {
             warnings: Vec::new(),
         }
     }
+    /// A crafted `.lrcat` that names a program as a photo's original gets no record for it, so
+    /// Photo ▸ Edit In ▸ Open Original has nothing to hand the system opener (#2135). Its virtual
+    /// copies go with it; the catalog's real photos still import.
+    #[test]
+    fn a_catalog_original_that_is_not_a_photo_file_is_not_imported() {
+        let mut data = sample();
+        let mut exe = data.photos[1].clone();
+        exe.source_id = 3;
+        exe.uuid = "payload".into();
+        exe.path = "/Users/sky/Downloads/payload.exe".into();
+        exe.image = row(json!({"id_local":3,"fileFormat":"ARW","fileWidth":20,"fileHeight":10}));
+        let mut exe_copy = exe.clone();
+        exe_copy.source_id = 4;
+        exe_copy.uuid = "payload-copy".into();
+        exe_copy.path = "/Users/sky/Downloads/payload.exe".into();
+        exe_copy.image = row(json!({"id_local":4,"masterImage":3,"copyName":"Mono"}));
+        // the copy comes before its master, as a catalog's table order may have it
+        data.photos.insert(0, exe_copy);
+        data.photos.push(exe);
+        for bad in [r"C:\Users\sky\start.lnk", "/Applications/Evil.app", "/Users/sky/go.command", "/Users/sky/run.bat"] {
+            let mut one = data.clone();
+            one.photos.retain(|p| p.source_id < 3);
+            let mut planted = one.photos[1].clone();
+            planted.source_id = 5;
+            planted.uuid = "planted".into();
+            planted.path = bad.into();
+            planted.image = row(json!({"id_local":5,"fileFormat":"ARW"}));
+            one.photos.push(planted);
+            let mut s = crate::Session::new();
+            let r = apply(&mut s, one, false).unwrap();
+            assert_eq!(r["photos"], 2, "{bad} is not catalogued: {r}");
+            assert!(s.catalog.photos().all(|p| !matches!(&p.source, Source::File { path } if path == bad)), "{bad} became a library record");
+        }
+        let mut s = crate::Session::new();
+        let r = apply(&mut s, data, false).unwrap();
+        assert_eq!(r["photos"], 2, "only the real master and its copy: {r}");
+        assert!(s.catalog.photos().all(|p| !matches!(&p.source, Source::File { path } if path.ends_with(".exe"))), "the program was catalogued");
+        let warnings = r["warnings"].to_string();
+        assert!(warnings.contains("payload.exe") && warnings.contains("file types LightCraft reads"), "{warnings}");
+        assert!(warnings.contains("image 4: not imported"), "the virtual copy of the refused original goes with it: {warnings}");
+    }
+
     #[test]
     fn offline_local_master_and_copy_join_library_with_one_undo_step() {
         let mut s = crate::Session::new();

crates/engine/src/open_path.rs

@@ -0,0 +1,117 @@
+//! Whether a path the library holds may be handed to the operating system.
+//!
+//! A photo's `Source::File` path is not always something a user typed: it can come from an
+//! imported Lightroom catalog, an `.lrcat` someone was sent, a relink or a synchronised folder.
+//! Photo ▸ Edit In ▸ *Open Original in Other Editor* and the system default opener run whatever
+//! the path names, so a record pointing at `payload.exe`, `thing.lnk`, `run.bat`, `page.hta`
+//! (Windows) or `Evil.app`, `go.command` (macOS) would be executed on one click (issue #2135).
+//!
+//! [`openable`] is the one gate every such hand-over goes through: the path must be a regular
+//! file whose extension is one LightCraft itself reads ([`crate::import::EXTENSIONS`], which
+//! already covers the Edit In copy formats `tif`/`tiff`/`psd`), and what a link resolves to must
+//! be one too, so a `photo.jpg` pointing at `/bin/sh` is refused as well. What comes back is the
+//! absolute path to hand over; what comes back on refusal is the sentence to show the user.
+//!
+//! Nothing here opens, stats-for-content or reads the file: it only decides whether the path may
+//! leave LightCraft. Catalog import applies the same test ([`crate::lightroom_catalog`]), so a
+//! refused original never becomes a library record in the first place.
+
+use std::path::Path;
+
+/// The absolute path to hand to an opener or external editor, or the reason it is refused.
+///
+/// `label` names the thing in the error (a file name, a photo's name); an empty `label` uses
+/// the path's own file name.
+pub fn openable(path: &str, label: &str) -> Result<String, String> {
+    let p = Path::new(path);
+    let name = match label.trim() {
+        "" => p.file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().into_owned()),
+        l => l.to_string(),
+    };
+    if !is_media(p) {
+        return Err(format!("{name} isn't one of the file types LightCraft reads, so it isn't handed to another application"));
+    }
+    match std::fs::metadata(p) {
+        Ok(m) if m.is_file() => {}
+        Ok(_) => return Err(format!("{name} isn't a file, so it isn't handed to another application")),
+        Err(e) => return Err(format!("the original is missing: {path} ({e})")),
+    }
+    // a link may name a photo and point at a program; what runs is what it points at
+    if let Ok(target) = p.canonicalize()
+        && !is_media(&target)
+    {
+        return Err(format!("{name} points at {}, which isn't a file type LightCraft reads", target.display()));
+    }
+    let absolute = std::path::absolute(p).map_err(|e| format!("{name}: {e}"))?;
+    Ok(absolute.to_string_lossy().into_owned())
+}
+
+/// Whether `path`'s extension is one LightCraft reads (the Edit In copy formats included).
+pub fn is_media(path: &Path) -> bool {
+    crate::import::is_supported(path)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn write(dir: &Path, name: &str) -> String {
+        let p = dir.join(name);
+        std::fs::write(&p, b"x").unwrap();
+        p.to_string_lossy().into_owned()
+    }
+
+    #[test]
+    fn only_file_types_lightcraft_reads_are_handed_over() {
+        let dir = std::env::temp_dir().join(format!("lc-openable-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        for good in ["DSC_1.NEF", "a.jpg", "copy-Edit.tif", "copy-Edit.psd"] {
+            let path = write(&dir, good);
+            assert_eq!(openable(&path, "").unwrap(), path, "{good} is a photo LightCraft reads");
+        }
+        for bad in ["payload.exe", "shortcut.lnk", "run.bat", "page.hta", "go.command", "no-extension"] {
+            let path = write(&dir, bad);
+            let e = openable(&path, "").unwrap_err();
+            assert!(e.contains(bad) && e.contains("file types LightCraft reads"), "{bad}: {e}");
+        }
+        // a bundle (macOS `.app`) and any other folder are not files
+        let bundle = dir.join("Evil.app");
+        std::fs::create_dir_all(&bundle).unwrap();
+        let e = openable(&bundle.to_string_lossy(), "").unwrap_err();
+        assert!(e.contains("isn't one of the file types") || e.contains("isn't a file"), "{e}");
+        let e = openable(&dir.join("gone.jpg").to_string_lossy(), "").unwrap_err();
+        assert!(e.contains("missing"), "{e}");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// A relative path is made absolute: the opener runs with its own working directory.
+    #[test]
+    fn the_path_handed_over_is_absolute() {
+        let dir = std::env::temp_dir().join(format!("lc-openable-abs-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        let path = write(&dir, "a.jpg");
+        let dotted = Path::new(&path).parent().unwrap().join(".").join("a.jpg");
+        let out = openable(&dotted.to_string_lossy(), "").unwrap();
+        assert!(Path::new(&out).is_absolute(), "{out}");
+        assert!(out.ends_with("a.jpg"), "{out}");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// A link named like a photo that points at a program is refused: what the opener runs is
+    /// the target, not the name.
+    #[cfg(unix)]
+    #[test]
+    fn a_link_to_a_program_is_refused() {
+        let dir = std::env::temp_dir().join(format!("lc-openable-link-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        let target = write(&dir, "payload.command");
+        let link = dir.join("holiday.jpg");
+        std::os::unix::fs::symlink(&target, &link).unwrap();
+        let e = openable(&link.to_string_lossy(), "").unwrap_err();
+        assert!(e.contains("points at") && e.contains("payload.command"), "{e}");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}