From b0241748eebf392817225ba25dac43e7f8008e27 Mon Sep 17 00:00:00 2001
From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com>
Date: Wed, 14 Oct 2026 22:47:31 +0300
Subject: [PATCH] cli: refuse non-object snapshot params instead of panicking
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A script line for `ui.screenshot` without a `path` had its generated path
assigned with `params["path"] = …`: `serde_json`'s mutable index panics on
an array, a string or a number, and the CLI installs no `catch_unwind`, so
a script handed to the documented headless workflow aborted the process.
Only an object (or null, like an absent `params`) takes the path now;
anything else is a script error naming the line, like the existing handling
of a malformed line.

The regression test runs the real binary on such a script and asserts a
non-zero exit, no "panicked" in stderr and the `params` must be a JSON
object message (it fails before the fix with `cannot access key "path" in
JSON array`).
---
 apps/lightcraft-cli/src/main.rs  | 12 +++++++++++-
 apps/lightcraft-cli/tests/cli.rs | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs
index b6cad57e0..572a4e955 100644
--- a/apps/lightcraft-cli/src/main.rs
+++ b/apps/lightcraft-cli/src/main.rs
@@ -832,7 +832,17 @@ fn snapshot(args: &[String]) -> Result<(), String> {
                         match next_path(&mut shots) {
                             Some(p) => {
                                 wrote_output = true;
-                                params["path"] = json!(p);
+                                // `params` is untrusted script JSON: `params["path"] = …` panics on
+                                // an array or a string. Only an object (or null, like an absent
+                                // `params`) may take the path.
+                                if params.is_null() {
+                                    params = json!({});
+                                }
+                                if let Some(o) = params.as_object_mut() {
+                                    o.insert("path".into(), json!(p));
+                                } else {
+                                    return Err(format!("{script}:{}: ui.screenshot: `params` must be a JSON object", n + 1));
+                                }
                             }
                             None => return Err(format!("{script}:{}: ui.screenshot needs a `path` (or give -o)", n + 1)),
                         }
diff --git a/apps/lightcraft-cli/tests/cli.rs b/apps/lightcraft-cli/tests/cli.rs
index bf56b02ab..5948d8b07 100644
--- a/apps/lightcraft-cli/tests/cli.rs
+++ b/apps/lightcraft-cli/tests/cli.rs
@@ -273,6 +273,24 @@ fn snapshot_script_failure_exits_non_zero() {
     assert_eq!((d.width, d.height), (320, 200));
 }
 
+/// A script line whose `params` is not an object must be an error, never a panic: the
+/// `ui.screenshot`-without-a-path branch used to index (`params["path"] = …`) an array and abort
+/// the whole script.
+#[test]
+fn snapshot_script_with_non_object_params_is_an_error() {
+    let script = tmp("snap-bad-params.jsonl");
+    let out = tmp("snap-bad-params.png");
+    std::fs::write(&script, format!("{}\n", json!({"method": "ui.screenshot", "params": [1]}))).unwrap();
+    let o = Command::new(BIN)
+        .args(["snapshot", "--demo", "--script", script.to_str().unwrap(), "-o", out.to_str().unwrap(), "--size", "320x200"])
+        .output()
+        .unwrap();
+    let stderr = String::from_utf8_lossy(&o.stderr);
+    assert!(!o.status.success(), "{stderr}");
+    assert!(!stderr.contains("panicked"), "a bad `params` must be an error, not a panic: {stderr}");
+    assert!(stderr.contains("`params` must be a JSON object"), "{stderr}");
+}
+
 #[test]
 fn snapshot_ui_zoom_keeps_requested_pixel_dimensions() {
     let script = tmp("snap-ui-zoom.jsonl");