From a196adfe93ad7fc5f27271a85b01fb5e9fcbbb61 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 08:19:26 -0400 Subject: [PATCH] Harden Image>Apply Image/Calculations against an empty source (Rule 9) source() could in principle return no channel planes, and apply_image/ calculations then panicked via src[0] / .remove(0). Guard source() to return Err when it yields no planes, protecting every caller. Graceful-failure test covers adversarial params. AGENTS.md: add the panic_hunt fuzz to the finish checklist and restate the no-panic rule. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 3 +++ crates/engine/src/channel_cmds.rs | 4 ++++ crates/engine/src/channel_cmds/tests.rs | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index da4c660a..7a559769 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,8 +68,11 @@ cargo clippy -p --all-targets -- -D warnings cargo xtask layers cargo xtask wasm # if you touched L0–L6 cargo xtask parity # if you added commands; commit the regenerated docs/parity.md +cargo test -p photocraft-engine --test panic_hunt -- --ignored # if you added/changed commands: no panic on adversarial input (Rule 9) ``` +Commands must **never panic** on bad input (Rule 9): every `run` closure and the code it calls returns `Err`, not a panic, for any params or document state. New commands come with a graceful-failure test (empty/out-of-range/wrong-type params → `Err`, not a crash). + Then append a terse entry to `log/devlog.md` (what landed, numbers, what's still open). Sessions can end abruptly (crashes, context limits), so the dev log plus a green tree is how the next agent picks up. Keep the tree building at every step. ## 6. Parallel agents diff --git a/crates/engine/src/channel_cmds.rs b/crates/engine/src/channel_cmds.rs index b1665a78..524f1f57 100644 --- a/crates/engine/src/channel_cmds.rs +++ b/crates/engine/src/channel_cmds.rs @@ -373,6 +373,10 @@ fn source(s: &Session, p: &Value, cmd: &str, gray: bool) -> Result> let r = p.get("channel").map_or(Some(ChanRef::Composite), |v| parse_ref(v, &st.doc)).ok_or_else(|| bad(cmd, "unknown `channel`"))?; let layer = source_layer(p); let mut planes = if gray { vec![gray_plane(&st.doc, layer, st.active_layer, r)?] } else { ref_planes(&st.doc, layer, st.active_layer, r)? }; + // Guard every caller (`.remove(0)` / `src[0]`) against an empty source. + if planes.is_empty() { + return Err(bad(cmd, "the source has no channels")); + } if p.get("invert").and_then(Value::as_bool).unwrap_or(false) { for pl in &mut planes { for v in pl.iter_mut() { diff --git a/crates/engine/src/channel_cmds/tests.rs b/crates/engine/src/channel_cmds/tests.rs index f2391c3c..399675b9 100644 --- a/crates/engine/src/channel_cmds/tests.rs +++ b/crates/engine/src/channel_cmds/tests.rs @@ -442,3 +442,23 @@ fn image_size_and_rotate_carry_channels_and_quick_mask() { assert_eq!(d.channels[0].surface.format().sample, SampleType::U16); assert_eq!(d.quick_mask.as_ref().unwrap().surface.format().sample, SampleType::U16); } + +#[test] +fn apply_image_and_calculations_fail_gracefully_never_panic() { + // Rule 9: adversarial params must return Err, not panic (guards the empty-source `src[0]` / + // `.remove(0)` paths in apply_image/calculations). + let mut s = session(); + for p in [ + json!({}), + json!({"source": {"channel": 999}}), + json!({"source": {"channel": "nope"}}), + json!({"source": {"document": 999}}), + json!({"blending": "bogus", "opacity": -50}), + ] { + // Must return a Result (Ok or Err) without panicking. + let _ = s.execute("image.applyImage", p.clone()); + let _ = s.execute("image.calculations", p); + } + // A well-formed self-apply still works. + assert!(s.execute("image.applyImage", json!({"source": {"channel": "composite"}})).is_ok()); +}