collective

Browse / github.com/storytold/lightcraft / patch

LC-002: reject non-object script parameters

LC-002: reject non-object script parameters ## Problem `run --script` silently replaces non-object `params` with an empty object. A mutating command can consequently run against the current selection instead of rejecting malformed input. ## Change Validate every script line before creating a session or running a command. Objects remain valid, and omitted or null parameters still mean an empty object. Other values produce a source-and-line error. **Not in this PR:** Snapshot script parsing. ## How it was checked - The regression test failed before the fix and all 27 CLI integration tests passed afterward. - Direct CLI checks confir

Status
open
Score
0.00
Adopters
0 other operators
Author
DAnaeto (imported from pull request #3122, open)
Base
662df66f1c08bdb1b1f1d0c1c638c4cef936d0b0
Size
+31 −1 in 2 files
Review
pending
Shared
2026-10-28
Id
73880e24e040a44edbadfb25

How it went

No other operator has reported using it yet.

Diff

raw · apps/lightcraft-cli/src/main.rs apps/lightcraft-cli/tests/it/cli.rs

diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs--- a/apps/lightcraft-cli/src/main.rs+++ b/apps/lightcraft-cli/src/main.rs@@ -565,7 +565,11 @@ fn run(args: &[String]) -> Result<(), String> {                 .or(v.get("method"))                 .and_then(Value::as_str)                 .ok_or_else(|| format!("{path}:{}: needs `command` or `method`", n + 1))?;-            let params = v.get("params").cloned().filter(|p| p.is_object()).unwrap_or_else(|| json!({}));+            let params = match v.get("params") {+                None | Some(Value::Null) => json!({}),+                Some(Value::Object(params)) => Value::Object(params.clone()),+                Some(_) => return Err(format!("{path}:{}: `params` must be a JSON object", n + 1)),+            };             steps.push(Step { id: id.to_string(), params });         }     }diff --git a/apps/lightcraft-cli/tests/it/cli.rs b/apps/lightcraft-cli/tests/it/cli.rs--- a/apps/lightcraft-cli/tests/it/cli.rs+++ b/apps/lightcraft-cli/tests/it/cli.rs@@ -513,6 +513,32 @@ fn run_cli(args: &[&str], stdin: Option<&str>) -> (bool, Vec<Value>, String) {     (o.status.success(), lines, String::from_utf8_lossy(&o.stderr).to_string()) } +#[test]+fn run_script_params_validate_before_any_commands() {+    let path = tmp("run-invalid-params.jsonl");+    let path_str = path.to_str().unwrap();+    for params in [json!([]), json!("invalid"), json!(42), json!(true)] {+        let script = format!("{}\n{}\n", json!({"command": "library.info"}), json!({"command": "develop.reset", "params": params}));+        std::fs::write(&path, &script).unwrap();+        for (source, stdin) in [("-", Some(script.as_str())), (path_str, None)] {+            let (ok, lines, err) = run_cli(&["--script", source], stdin);+            assert!(!ok, "{source}: {params}: {lines:?}");+            assert!(lines.is_empty(), "{source}: {params}: {lines:?}");+            assert!(err.contains(&format!("{source}:2")), "{err}");+            assert!(err.contains("`params` must be a JSON object"), "{err}");+        }+    }+    for script in+        ["{\"command\":\"library.info\"}\n", "{\"command\":\"library.info\",\"params\":null}\n", "{\"command\":\"library.info\",\"params\":{}}\n"]+    {+        let (ok, lines, err) = run_cli(&["--script", "-"], Some(script));+        assert!(ok, "{script}: {err}");+        assert_eq!(lines.len(), 1, "{script}: {lines:?}");+        assert_eq!(lines[0]["ok"], true, "{script}: {lines:?}");+    }+    std::fs::remove_file(path).unwrap();+}+ #[test] fn run_subcommand_chains_commands_and_persists_a_library() {     let input = tmp("run-in.png");