From a89fbdab8f0d83db3c7128fb5cbb6957e6bfb6c9 Mon Sep 17 00:00:00 2001
From: Anubhav Rai <adobearakap@gmail.com>
Date: Wed, 28 Oct 2026 00:05:19 +0530
Subject: [PATCH] engine: two template footage entries can't become one file
 (#2123)

The fallback name was never checked against the names already taken, so an
entry called exactly what the fallback produces took the same file: `media/1/a:b.png`,
`media/2/a_b.png` and `media/3/2-a_b.png` extracted to five files for six
entries, the third overwriting the second, and both items' footage pointing at
it. Deduplication now runs on the name actually written and counts up until it
is free.

The collision predates making the names safe (two entries named `a_b.png` and
`2-a_b.png` collide on their own), but sanitising gives more ways in, so it
belongs with it.

Found by CodeRabbit on the pull request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 crates/engine/src/home/templates.rs | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/crates/engine/src/home/templates.rs b/crates/engine/src/home/templates.rs
index 21b4b61d8..515178730 100644
--- a/crates/engine/src/home/templates.rs
+++ b/crates/engine/src/home/templates.rs
@@ -751,7 +751,14 @@ fn extract_footage(s: &Session, proj: &mut Project, entries: &[(String, Vec<u8>)
         // The item's id, and only that: it is written as one ([`embed_footage`]), but a template
         // from elsewhere can hold anything there, and `..\` in it walks out of `dir` on Windows.
         let item = k.trim_start_matches("media/").split('/').next().filter(|i| i.bytes().all(|b| b.is_ascii_digit())).unwrap_or("0");
-        let name = if used.insert(file.clone()) { file } else { format!("{item}-{file}") };
+        // A name another entry has taken gets the item's id, and a number if that is taken too:
+        // the names an entry can ask for are not distinct, and two files must not become one.
+        let mut name = file.clone();
+        let mut n = 0usize;
+        while !used.insert(name.clone()) {
+            n += 1;
+            name = if n == 1 { format!("{item}-{file}") } else { format!("{item}-{n}-{file}") };
+        }
         let out = format!("{dir}/{name}");
         s.services.store_file(&out, v).map_err(|e| EngineError::Other(format!("cannot write {out}: {e}")))?;
         if let Some(imp) = &s.importer {
@@ -999,6 +1006,10 @@ mod tests {
             // The base name is taken, so this one's "item id" is used — and it is not one.
             entry("media/..\\..\\..\\Startup\\x/clip.mp4"),
             entry("media/1/C:evil.bat"),
+            // Three that collide once the first is made safe: each still gets its own file.
+            entry("media/1/a:b.png"),
+            entry("media/2/a_b.png"),
+            entry("media/3/2-a_b.png"),
         ];
         let written = extract_footage(&s, &mut proj, &entries, &ds).unwrap();
         assert_eq!(written.len(), entries.len());
@@ -1007,7 +1018,8 @@ mod tests {
             assert!(!name.contains(['/', '\\', ':']), "{w}");
             assert!(!name.trim_matches(['.', ' ']).is_empty(), "{w}");
         }
-        // One file each, all of them in the folder.
+        // One file each, all of them in the folder, none overwritten by the next.
+        assert_eq!(written.iter().collect::<std::collections::BTreeSet<_>>().len(), entries.len(), "{written:?}");
         assert_eq!(std::fs::read_dir(&dir).map(|d| d.count()).unwrap_or(0), entries.len());
         let _ = std::fs::remove_dir_all(&dir);
         // And the names that would leave nothing to write.