From fb778030c9d633d322ce111feabe57602421dff4 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:13:35 +0000 Subject: [PATCH] Reject invalid shape ids in `ids` instead of using the selection Found while testing crates/engine/src/cmd/mod.rs:ids_param / targets (~line 335-350) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/mod.rs | 5 +++++ crates/engine/src/tests.rs | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index fa3f6459..ad2c86ab 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -355,6 +355,11 @@ pub(crate) fn with_param(p: &Value, key: &str, v: Value) -> Value { /// Targets: `ids` / `id` params, else the selection. pub(crate) fn targets(s: &Session, p: &Value) -> Result> { + if let Some(a) = p.get("ids").and_then(Value::as_array) + && let Some(v) = a.iter().find(|v| v.as_u64().and_then(|v| u32::try_from(v).ok()).is_none()) + { + return Err(bad("shape", format!("`ids` has an invalid shape id: {v}"))); + } if let Some(ids) = ids_param(p, "ids") && !ids.is_empty() { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 35b0ab39..3a589212 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4403,3 +4403,14 @@ fn duplicating_a_moved_copy_repeats_the_move() { assert_eq!(boxed(&mut s, f), (340.0 + off, 100.0 + off)); assert_eq!(s.doc().unwrap().doc.validate(), Vec::::new()); } + +#[test] +fn invalid_ids_array_never_falls_back_to_the_selection() { + let mut s = session(); + let a = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 100, 100]})).unwrap()["id"].as_u64().unwrap(); + for ids in [json!([-1]), json!([1.5]), json!([4294967296u64]), json!([-1, a])] { + let r = s.execute("edit.delete", &json!({ "ids": ids })); + assert!(matches!(r, Err(crate::EngineError::BadParams { .. })), "invalid ids {ids} gave {r:?}"); + assert!(s.doc().unwrap().shape(deckcraft_model::ShapeId(a as u32)).is_some(), "shape deleted for {ids}"); + } +}