Merge PR #2455: Web: lightcraft.command refuses params that aren't a JSON ob

apps/lightcraft-web/src/automation.rs

@@ -0,0 +1,48 @@
+//! The page's automation entry point, `lightcraft.command(id, params)` (see `web.rs`): what the
+//! params text means. Plain Rust, so it is tested natively.
+
+use serde_json::Value;
+
+/// The command parameters in `text`: a JSON object, or `{}` when the text is empty or blank (or
+/// `null`). Anything else is an error, never `{}`: a command run without its params acts on the
+/// selection or with defaults, so a typo such as `{ids: [3]}` must not delete or reset the
+/// selected photos. The control channel and MCP refuse such params the same way.
+pub fn params(text: &str) -> Result<Value, String> {
+    let text = text.trim();
+    if text.is_empty() {
+        return Ok(Value::Object(Default::default()));
+    }
+    match serde_json::from_str::<Value>(text) {
+        Ok(Value::Null) => Ok(Value::Object(Default::default())),
+        Ok(v @ Value::Object(_)) => Ok(v),
+        Ok(_) => Err("params must be a JSON object, e.g. {\"ids\": [3]}".into()),
+        Err(e) => Err(format!("params are not valid JSON ({e}); nothing was run")),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::json;
+
+    #[test]
+    fn blank_or_null_params_are_an_empty_object() {
+        for text in ["", "   ", "\n", "null", " null "] {
+            assert_eq!(params(text), Ok(json!({})), "{text:?}");
+        }
+    }
+
+    #[test]
+    fn an_object_is_passed_through() {
+        assert_eq!(params(r#"{"ids": [3], "rating": 5}"#), Ok(json!({"ids": [3], "rating": 5})));
+    }
+
+    /// Before, any of these ran the command with `{}`: `photo.delete` then removed the selection.
+    #[test]
+    fn invalid_json_or_a_non_object_is_refused() {
+        for text in ["{ids: [3]}", "{\"ids\": [3]", "ids=3", "[3]", "3", "\"{}\"", "true"] {
+            assert!(params(text).is_err(), "{text:?} must not run the command");
+        }
+        assert!(params("{ids: [3]}").unwrap_err().contains("nothing was run"));
+    }
+}

apps/lightcraft-web/src/lib.rs

@@ -16,6 +16,7 @@
 #![forbid(unsafe_code)]
 #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
+pub mod automation;
 pub mod backup;
 pub mod bench;
 pub mod files;

apps/lightcraft-web/src/web.rs

@@ -643,8 +643,11 @@ impl WebApp {
     fn run_inbox(&mut self) {
         let inbox = INBOX.with(|q| std::mem::take(&mut *q.borrow_mut()));
         for (id, params, (resolve, reject)) in inbox {
-            let params: serde_json::Value = serde_json::from_str(&params).unwrap_or(json!({}));
-            let r = if id == "web.stats" { Ok(self.web_stats()) } else { self.app.run(&id, params) };
+            let r = match crate::automation::params(&params) {
+                Err(e) => Err(format!("{id}: {e}")),
+                Ok(_) if id == "web.stats" => Ok(self.web_stats()),
+                Ok(params) => self.app.run(&id, params),
+            };
             let _ = match r {
                 Ok(v) => resolve.call1(&JsValue::NULL, &v.to_string().into()),
                 Err(e) => reject.call1(&JsValue::NULL, &e.into()),

docs/web.md

@@ -158,7 +158,10 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not
   `write` service hands each file to the browser as a download instead of writing it to disk.
 - **Automation.** `await lightcraft.command("library.info", "{}")` runs any engine or UI command
   by id on the next frame and resolves to the JSON result (`web.stats` reports storage, workers
-  and the render queue). This is how the headless-Chrome checks drive the page.
+  and the render queue). The params are a JSON object; empty text and JSON `null` mean `{}`
+  (no params: a command that takes photo ids then acts on the selection). Any other text that
+  isn't an object rejects the promise and runs nothing, as the control channel and MCP do, so a
+  typo can't run a command on the selection. This is how the headless-Chrome checks drive the page.
 
 ## Not yet