From 312dd04830debcd2129d60c7e6efc1aefb975ec6 Mon Sep 17 00:00:00 2001
From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com>
Date: Fri, 16 Oct 2026 16:21:24 +0200
Subject: [PATCH] Reject out-of-range start in photo.rename and
 photo.renamePreview

Found while testing crates/engine/src/rename.rs:919 (start parsed at crates/engine/src/cmd/manage.rs:12)

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---
 crates/engine/src/cmd/manage.rs     |  9 +++++++--
 crates/engine/src/rename.rs         |  3 ++-
 crates/engine/src/tests_organize.rs | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/crates/engine/src/cmd/manage.rs b/crates/engine/src/cmd/manage.rs
index 66ddfe9fb..339b20b90 100644
--- a/crates/engine/src/cmd/manage.rs
+++ b/crates/engine/src/cmd/manage.rs
@@ -9,8 +9,13 @@ use crate::Result;
 
 fn rename_args(s: &crate::Session, p: &Value, c: &str) -> Result<(Vec<lightcraft_catalog::PhotoId>, String, usize)> {
     let template = str_param(p, "template").ok_or_else(|| bad(c, "missing `template`"))?.to_string();
-    let start = p.get("start").and_then(Value::as_u64).unwrap_or(1) as usize;
-    Ok((s.targets(p)?, template, start))
+    let start = p.get("start").and_then(Value::as_u64).unwrap_or(1);
+    let ids = s.targets(p)?;
+    // every number of the sequence `start..start + n` must fit
+    let last = start.checked_add((ids.len() as u64).saturating_sub(1));
+    let start = last.and_then(|l| usize::try_from(l).ok()).and_then(|_| usize::try_from(start).ok());
+    let start = start.ok_or_else(|| bad(c, "start out of range"))?;
+    Ok((ids, template, start))
 }
 
 /// A named set of colour-label names (red, yellow, green, blue, purple; empty = the colour's name).
diff --git a/crates/engine/src/rename.rs b/crates/engine/src/rename.rs
index 16003cd61..f0d725793 100644
--- a/crates/engine/src/rename.rs
+++ b/crates/engine/src/rename.rs
@@ -916,7 +916,8 @@ fn plan_rename_core(
 ) -> Vec<RenamePlan> {
     let mut taken: HashSet<String> = HashSet::new(); // lower-case target paths/names claimed in this batch
     let mut out = Vec::new();
-    for (seq, p) in (start..).zip(photos) {
+    for (i, p) in photos.iter().enumerate() {
+        let seq = start.saturating_add(i);
         let want = expand(template, p, seq);
         let (stem, ext) = split_ext(&want);
         let (stem, ext) = (stem.to_string(), ext.to_string());
diff --git a/crates/engine/src/tests_organize.rs b/crates/engine/src/tests_organize.rs
index 8990acb4c..344838432 100644
--- a/crates/engine/src/tests_organize.rs
+++ b/crates/engine/src/tests_organize.rs
@@ -336,3 +336,23 @@ fn deleting_an_album_folder_preserves_photos_and_restores_its_subtree() {
     drop(reopened);
     let _ = std::fs::remove_dir_all(dir);
 }
+
+/// A huge `start` is a validation error, not an arithmetic overflow in the numbering.
+#[test]
+fn rename_huge_start_is_a_bad_param() {
+    let mut s = Session::with_demo();
+    let ids: Vec<u64> = s.catalog.photos().take(3).map(|p| p.id.0).collect();
+    let names_before: Vec<String> = s.catalog.photos().map(|p| p.file_name.clone()).collect();
+    for cmd in ["photo.renamePreview", "photo.rename"] {
+        let e = s.execute(cmd, &json!({"template": "{seq}", "start": u64::MAX, "ids": ids})).unwrap_err();
+        assert!(e.to_string().contains("start out of range"), "{cmd}: {e}");
+    }
+    let names_after: Vec<String> = s.catalog.photos().map(|p| p.file_name.clone()).collect();
+    assert_eq!(names_before, names_after, "a rejected rename changes nothing");
+    // the last start whose whole sequence fits is accepted, one more is not
+    let fits = usize::MAX as u64 - 2;
+    let r = s.execute("photo.renamePreview", &json!({"template": "{seq}", "start": fits, "ids": ids}));
+    assert!(r.is_ok(), "{r:?}");
+    let r = s.execute("photo.renamePreview", &json!({"template": "{seq}", "start": fits + 1, "ids": ids}));
+    assert!(r.is_err(), "{r:?}");
+}