fix(plugins): the install error names the format the file actually is (#10362) (#10633)

* fix(plugins): the install error names the format the file actually is (#10362)

Plug-ins > Install Plug-in pointed at an old Photoshop plug-in (.8bt) answered
with "not a valid plug-in module: not a WebAssembly binary (missing \0asm
header)". That was one fixed message for every file that is not wasm: it said
what we wanted and nothing about the file the person is holding.

format_hint in runtime.rs now reads the magic number and the error says what the
file looks like. For a Windows module the message is (one line in the code,
wrapped here):

  not a valid plug-in module: not a WebAssembly binary (missing \0asm header);
  this file looks like a Windows native executable or library (a .dll, or a
  Photoshop plug-in such as .8bf/.8bt) - PhotoCraft plug-ins are WebAssembly
  modules (.wasm)

7F 45 4C 46 says "a Linux native library (ELF)", CE FA ED FE / CF FA ED FE /
CA FE BA BE / BE BA FE CA say "a macOS native binary (Mach-O, thin or fat)",
50 4B 03 04 / 50 4B 05 06 say "a zip archive". Any other header keeps today's
message byte for byte, so nothing keyed on it changes. The signatures are public
format facts, read with starts_with/get so a short or hostile file cannot panic.
The error variant, the enum, the ABI check, check_code_section and the wasmi
setup are untouched, and native-code loading stays out of scope (PR #7907).

Fail-first, before the production change:

  cargo test -p photocraft-plugins --lib --tests
    lib:  test result: ok. 10 passed; 0 failed
    abi:  test result: FAILED. 19 passed; 1 failed; 0 ignored; 0 measured
          assertion line: "windows dll or .8bt: not a valid plug-in module:
          not a WebAssembly binary (missing \0asm header)" (the MZ fixture that
          was already in the file)
  cargo test -p photocraft-engine --lib -- plugin
    test result: FAILED. 7 passed; 1 failed; 0 ignored; 0 measured; 2518 filtered out

After:

  cargo test -p photocraft-plugins
    lib 10 passed; abi 20 passed; doc-tests 0
  cargo test -p photocraft-engine --lib -- plugin
    test result: ok. 8 passed; 0 failed; 2518 filtered out

The new engine test drives the real plugin.install command with a `path` whose
file starts with MZ bytes, so it covers the menu path (registry::load_file) end
to end, not only the library check.

Gates: cargo clippy -p photocraft-plugins --all-targets -- -D warnings and
cargo clippy -p photocraft-engine --all-targets -- -D warnings (clean; the only
stderr noise is the pre-existing "patch `egui_*` was not used in the crate
graph" cargo warnings); cargo xtask layers ("OK: 31 crates, no layering
violations."); cargo xtask wasm ("  ok     24 crates", "  ok     photocraft-
codecs --features heif", no FAIL line); cargo fmt -p photocraft-plugins --
--check and cargo fmt -p photocraft-engine -- --check (silent, exit 0). The
whole engine --lib suite is not run here: it fails on clean main in this
checkout for font/i18n reasons (no CRAFT_FONTS_DIR). No doc quoted the old
message (git grep "WebAssembly binary" finds only this crate), so nothing to
update there.

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

* test(plugins): one install-error test per recognised format, tempfile in the engine test (#10362)

Review feedback on #10633. The single looped test in abi.rs is now one test
per signature (PE/COFF MZ, ELF, Mach-O in all four byte orders, zip), one for
an empty or unrecognised file (including signatures cut short, which keep the
old message byte for byte) and one showing a valid WebAssembly module still
loads. The engine test uses tempfile::tempdir (already a dependency of
photocraft-engine) so the directory is removed on a panic, and asserts the
message does not contain the temp path: the hint is static text, so the
message adds no path.

Gates: cargo test -p photocraft-plugins --lib --tests (10 lib, 26 tests);
cargo test -p photocraft-engine --lib -- plugin (8 passed); clippy -D warnings
on both crates; cargo fmt --all -- --check; cargo xtask layers (OK).

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

* test(engine): keep the non-wasm install-error test off wasm32, as tempfile is (#10362)

tempfile is a non-wasm32 dependency of photocraft-engine, so the test that
uses it is gated like the crate's other file-system tests.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/plugin_cmds/tests.rs

@@ -133,6 +133,23 @@ fn example_invert_matches_builtin_invert_and_round_trips_the_registry() {
     std::fs::remove_dir_all(&dir).ok();
 }
 
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn installing_a_file_that_is_not_wasm_names_what_it_is() {
+    // #10362: the Plug-ins > Install Plug-in dialog pointed at an old Photoshop plug-in (.8bt, a
+    // native Windows module) failed with a message that said nothing about that file. `path` goes
+    // through `registry::load_file`, so this is the exact user path.
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("Old Plug-in.8bt");
+    std::fs::write(&path, b"MZ\x90\x00\x03\0").unwrap();
+    let e = Session::new().execute("plugin.install", json!({"path": path.to_str().unwrap()})).unwrap_err();
+    let msg = e.to_string();
+    assert!(msg.contains("Windows") && msg.contains("8bf"), "{msg}");
+    assert!(msg.contains("WebAssembly"), "{msg}");
+    // The hint is static text: no path the person did not choose is added to the message.
+    assert!(!msg.contains(dir.path().to_str().unwrap()), "{msg}");
+}
+
 #[test]
 fn params_are_validated_and_passed() {
     let bytes = wat_plugin("test.cmds.grey", r#"{"amount":{"type":"number","min":0,"max":1,"default":1}}"#, GREY);

crates/plugins/src/runtime.rs

@@ -167,6 +167,26 @@ fn check_code_section(bytes: &[u8]) -> Result<()> {
     Ok(())
 }
 
+/// What a file that is not WebAssembly actually is, judged from its magic number, so the install
+/// error can name the format the person picked instead of only the one we wanted (public format
+/// facts: the PE/COFF, ELF, Mach-O and zip signatures). `None` for anything else. Reads with
+/// `starts_with`/`get`, so a file shorter than the signature is fine.
+fn format_hint(bytes: &[u8]) -> Option<&'static str> {
+    if bytes.starts_with(b"MZ") {
+        return Some("a Windows native executable or library (a .dll, or a Photoshop plug-in such as .8bf/.8bt)");
+    }
+    if bytes.get(..4) == Some(&[0x7f, b'E', b'L', b'F']) {
+        return Some("a Linux native library (ELF)");
+    }
+    if matches!(bytes.get(..4), Some(&[0xce, 0xfa, 0xed, 0xfe] | &[0xcf, 0xfa, 0xed, 0xfe] | &[0xca, 0xfe, 0xba, 0xbe] | &[0xbe, 0xba, 0xfe, 0xca])) {
+        return Some("a macOS native binary (Mach-O, thin or fat)");
+    }
+    if matches!(bytes.get(..4), Some(&[0x50, 0x4b, 0x03, 0x04] | &[0x50, 0x4b, 0x05, 0x06])) {
+        return Some("a zip archive");
+    }
+    None
+}
+
 impl Plugin {
     /// Validates and loads a module: checks the ABI version and reads the manifest.
     pub fn load(bytes: &[u8], limits: Limits) -> Result<Plugin> {
@@ -174,7 +194,11 @@ impl Plugin {
             return Err(Error::Module(format!("module is {} bytes (limit {})", bytes.len(), limits.max_module_bytes)));
         }
         if !bytes.starts_with(b"\0asm") {
-            return Err(Error::Module("not a WebAssembly binary (missing \\0asm header)".into()));
+            let msg = "not a WebAssembly binary (missing \\0asm header)";
+            return Err(Error::Module(match format_hint(bytes) {
+                Some(hint) => format!("{msg}; this file looks like {hint} - PhotoCraft plug-ins are WebAssembly modules (.wasm)"),
+                None => msg.to_string(),
+            }));
         }
         check_code_section(bytes)?;
         let mut config = Config::default();

crates/plugins/tests/abi.rs

@@ -219,6 +219,64 @@ fn malformed_and_oversized_modules_are_refused() {
     assert!(matches!(e, Error::Abi(_)), "{e}");
 }
 
+/// The message `Plugin::load` gives a file that is not WebAssembly and not recognised (#10362).
+const NOT_WASM: &str = "not a valid plug-in module: not a WebAssembly binary (missing \\0asm header)";
+
+/// Loads `bytes`, which must be refused as a module, and returns the message.
+fn refusal(bytes: &[u8]) -> String {
+    let Err(e) = Plugin::load(bytes, Limits::default()) else { panic!("{bytes:?} was accepted") };
+    assert!(matches!(e, Error::Module(_)), "{e}");
+    e.to_string()
+}
+
+/// The message names the format (`format`), says plug-ins are WebAssembly, and stays on one line
+/// (`Plugin::load` has only bytes, so the hint cannot carry a path).
+fn assert_names(bytes: &[u8], format: &str) {
+    let m = refusal(bytes);
+    assert!(m.starts_with(NOT_WASM), "{m}");
+    assert!(m.contains(format), "{m}");
+    assert!(m.contains("PhotoCraft plug-ins are WebAssembly modules"), "{m}");
+    assert!(!m.contains('\n'), "{m}");
+}
+
+#[test]
+fn a_pe_coff_file_is_named_as_a_windows_binary() {
+    // #10362: an old Photoshop plug-in (.8bt) is a native Windows module.
+    assert_names(b"MZ\x90\x00\x03\0", "Windows");
+    assert!(refusal(b"MZ").contains("8bf"));
+}
+
+#[test]
+fn an_elf_file_is_named_as_a_linux_library() {
+    assert_names(&[0x7f, b'E', b'L', b'F', 2, 1, 1], "Linux");
+}
+
+#[test]
+fn a_mach_o_file_is_named_as_a_macos_binary() {
+    for head in [[0xce, 0xfa, 0xed, 0xfe], [0xcf, 0xfa, 0xed, 0xfe], [0xca, 0xfe, 0xba, 0xbe], [0xbe, 0xba, 0xfe, 0xca]] {
+        assert_names(&[&head[..], &[7, 1][..]].concat(), "macOS");
+    }
+}
+
+#[test]
+fn a_zip_file_is_named_as_a_zip_archive() {
+    assert_names(b"PK\x03\x04\x14\0", "zip");
+    assert_names(b"PK\x05\x06\0\0", "zip");
+}
+
+#[test]
+fn an_empty_or_unrecognised_file_keeps_the_plain_message() {
+    // Nothing recognised (including a signature cut short): today's message, exactly.
+    for bytes in [&b""[..], &b"x"[..], &b"junkjunk"[..], &[0x7f, b'E'][..], &b"PK"[..]] {
+        assert_eq!(refusal(bytes), NOT_WASM, "{bytes:?}");
+    }
+}
+
+#[test]
+fn a_valid_wasm_module_still_loads_unchanged() {
+    assert!(Plugin::load(INVERT, Limits::default()).is_ok());
+}
+
 #[test]
 fn a_code_section_claiming_no_functions_but_many_bytes_is_refused() {
     // Header, then a code section (id 10) declaring 2000 bytes whose function count is 0, cut off