security(proxies): reserve outputs atomically (#2922)

* security(proxies): reserve outputs atomically

* proxies: ReservedOutput::file returns an Option instead of expect()

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <brandon@storyteller.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/proxies.rs

@@ -96,6 +96,19 @@ pub fn transcode(
     out: &str,
     preset: &Preset,
     progress: &Progress,
+) -> std::result::Result<Report, String> {
+    transcode_with_file(clip, name, src, range, out, None, preset, progress)
+}
+
+fn transcode_with_file(
+    clip: &MediaClip,
+    name: &str,
+    src: SharedSource,
+    range: Option<TimeRange>,
+    out: &str,
+    output_file: Option<Arc<std::fs::File>>,
+    preset: &Preset,
+    progress: &Progress,
 ) -> std::result::Result<Report, String> {
     let info = &clip.info;
     let rate = clip.frame_rate();
@@ -140,6 +153,7 @@ pub fn transcode(
             as u32,
         part_of_batch: true,
         prores_profile: preset.prores.to_string(),
+        output_file,
         ..Default::default()
     };
     let project = Arc::new(p);
@@ -147,6 +161,59 @@ pub fn transcode(
     filmcraft_export::export(&project, seq, &settings, &provider, progress).map_err(|e| e.to_string())
 }
 
+/// An output created exclusively while choosing its name. Unless kept after a successful
+/// transcode, dropping it removes only that file (including on a cancelled or rejected job).
+struct ReservedOutput {
+    path: PathBuf,
+    file: Option<Arc<std::fs::File>>,
+    keep: bool,
+}
+
+impl ReservedOutput {
+    fn path(&self) -> &Path {
+        &self.path
+    }
+
+    fn file(&self) -> Option<Arc<std::fs::File>> {
+        self.file.clone()
+    }
+
+    fn preserve(&mut self) {
+        self.keep = true;
+    }
+}
+
+impl Drop for ReservedOutput {
+    fn drop(&mut self) {
+        if !self.keep {
+            // Release our handle before removal, which is required on Windows.
+            self.file.take();
+            let _ = std::fs::remove_file(&self.path);
+        }
+    }
+}
+
+/// Reserve a numbered output name without following a later replacement of its pathname.
+fn reserve_unique_path(dir: &Path, stem: &str, suffix: &str, ext: &str, taken: &[String]) -> Result<ReservedOutput> {
+    std::fs::create_dir_all(dir).map_err(|e| EngineError::Other(format!("{}: {e}", dir.display())))?;
+    let mut n = 1;
+    loop {
+        let name = if n == 1 { format!("{stem}{suffix}.{ext}") } else { format!("{stem}{suffix} {n}.{ext}") };
+        let path = dir.join(name);
+        if taken.contains(&path.to_string_lossy().into_owned()) {
+            n += 1;
+            continue;
+        }
+        match std::fs::OpenOptions::new().write(true).create_new(true).open(&path) {
+            Ok(file) => {
+                return Ok(ReservedOutput { path, file: Some(Arc::new(file)), keep: false });
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => n += 1,
+            Err(e) => return Err(EngineError::Other(format!("{}: {e}", path.display()))),
+        }
+    }
+}
+
 /// Frames a transcode of `clip` will write (for progress totals).
 pub fn frame_count(clip: &MediaClip, range: Option<TimeRange>) -> u64 {
     let rate = clip.frame_rate();
@@ -184,17 +251,17 @@ fn items_param(s: &Session, p: &Value) -> Vec<ItemId> {
 }
 
 /// Start a background transcode job over `work` (item, output path, preset). Returns the job id.
-pub fn start_job(s: &mut Session, label: String, work: Vec<(ItemId, String, &'static Preset)>, on_done: OnDone, wait: bool) -> Result<u64> {
+fn start_job(s: &mut Session, label: String, work: Vec<(ItemId, ReservedOutput, &'static Preset)>, on_done: OnDone, wait: bool) -> Result<u64> {
     let mut tasks = Vec::new();
     let mut total = 0;
-    for (item, out, pr) in work {
+    for (item, output, pr) in work {
         let (clip, _, name) = media_path(&s.project, item).ok_or_else(|| bad("media.createProxies", format!("item {} is not file media", item.0)))?;
         let src = s.media.full_res_source(&s.project, item, &*s.services).ok_or_else(|| EngineError::Other(format!("{name}: no source")))?;
         if s.media.offline_status(item).is_some() {
             return Err(EngineError::Other(format!("{name} is offline; link it first")));
         }
         total += frame_count(&clip, None);
-        tasks.push((item, clip, name, src, out, pr));
+        tasks.push((item, clip, name, src, output, pr));
     }
     let id = s.jobs.iter().map(|j| j.id).max().unwrap_or(0) + 1;
     let job = crate::Job { id, label, progress: Default::default(), result: Default::default() };
@@ -206,22 +273,20 @@ pub fn start_job(s: &mut Session, label: String, work: Vec<(ItemId, String, &'st
         let mut bytes = 0;
         let mut err = None;
         let n = tasks.len();
-        for (k, (item, clip, name, src, out, pr)) in tasks.into_iter().enumerate() {
+        for (k, (item, clip, name, src, mut output, pr)) in tasks.into_iter().enumerate() {
             if prog.cancel.load(std::sync::atomic::Ordering::Relaxed) {
                 err = Some("cancelled".to_string());
                 break;
             }
             *prog.status.lock().unwrap_or_else(|e| e.into_inner()) = format!("{name} ({} of {n})", k + 1);
-            if let Some(d) = Path::new(&out).parent() {
-                let _ = std::fs::create_dir_all(d);
-            }
-            match transcode(&clip, &name, src, None, &out, pr, &prog) {
+            let out = output.path().to_string_lossy().into_owned();
+            match transcode_with_file(&clip, &name, src, None, &out, output.file(), pr, &prog) {
                 Ok(r) => {
                     bytes += r.bytes;
+                    output.preserve();
                     outs.lock().unwrap_or_else(|e| e.into_inner()).push((item, out));
                 }
                 Err(e) => {
-                    let _ = std::fs::remove_file(&out);
                     err = Some(format!("{name}: {e}"));
                     break;
                 }
@@ -371,14 +436,15 @@ pub fn create(s: &mut Session, p: &Value) -> Result<Value> {
             }
         };
         let stem = Path::new(&path).file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or(name);
-        let out = unique_path(&proxy_dir(&path, dest.as_deref()), &stem, "_Proxy", pr.extension, &taken).to_string_lossy().into_owned();
+        let output = reserve_unique_path(&proxy_dir(&path, dest.as_deref()), &stem, "_Proxy", pr.extension, &taken)?;
+        let out = output.path().to_string_lossy().into_owned();
         taken.push(out.clone());
-        work.push((i, out, pr));
+        work.push((i, output, pr));
     }
     if work.is_empty() {
         return Err(EngineError::Other(format!("nothing to proxy: {}", skipped.iter().filter_map(|v| v["reason"].as_str()).collect::<Vec<_>>().join("; "))));
     }
-    let outputs: Vec<Value> = work.iter().map(|(i, o, _)| json!({"item": i.0, "path": o})).collect();
+    let outputs: Vec<Value> = work.iter().map(|(i, o, _)| json!({"item": i.0, "path": o.path()})).collect();
     let n = work.len();
     let label = format!("Create Proxies ({n} clip{})", if n == 1 { "" } else { "s" });
     let attach = bool_p(p, "attach").unwrap_or(true);
@@ -481,9 +547,10 @@ pub fn ingest(s: &mut Session, items: &[ItemId]) -> Result<Value> {
                     _ => Path::new(&path).parent().unwrap_or(Path::new(".")).join("Ingested Media"),
                 };
                 let stem = Path::new(&path).file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or(name);
-                let out = unique_path(&dir, &stem, "", pr.extension, &taken).to_string_lossy().into_owned();
+                let output = reserve_unique_path(&dir, &stem, "", pr.extension, &taken)?;
+                let out = output.path().to_string_lossy().into_owned();
                 taken.push(out.clone());
-                work.push((i, out, pr));
+                work.push((i, output, pr));
             }
             if !work.is_empty() {
                 let id = start_job(s, format!("Ingest: transcode {} clip(s)", work.len()), work, OnDone::ReplaceMedia, false)?;

crates/export/src/lib.rs

@@ -406,6 +406,10 @@ pub struct ExportSettings {
     /// writing `path` (hosts without a filesystem: the web app offers the file as a download).
     #[serde(skip)]
     pub sink: Option<OutputSink>,
+    /// An already-created primary output file. Internal callers use this to retain an exclusive
+    /// reservation made while choosing an output name, rather than reopening that path later.
+    #[serde(skip)]
+    pub output_file: Option<Arc<std::fs::File>>,
 }
 
 /// Receives in-memory export output: `(path, bytes)` per finished file.
@@ -432,6 +436,10 @@ enum Out {
 
 impl Out {
     fn create(settings: &ExportSettings) -> Result<Out> {
+        if let Some(file) = &settings.output_file {
+            let file = file.try_clone().map_err(|e| ExportError::Io(e.to_string()))?;
+            return Ok(Out::File(std::io::BufWriter::new(file)));
+        }
         Out::create_path(settings, &settings.path)
     }
 
@@ -440,6 +448,12 @@ impl Out {
         if settings.sink.is_some() {
             return Ok(Out::Mem(std::io::Cursor::new(Vec::new())));
         }
+        if path == settings.path
+            && let Some(file) = &settings.output_file
+        {
+            let file = file.try_clone().map_err(|e| ExportError::Io(e.to_string()))?;
+            return Ok(Out::File(std::io::BufWriter::new(file)));
+        }
         let f = std::fs::File::create(path).map_err(|e| ExportError::Io(format!("{path}: {e}")))?;
         Ok(Out::File(std::io::BufWriter::new(f)))
     }
@@ -588,6 +602,7 @@ impl Default for ExportSettings {
             h264_pass: H264Pass::Single,
             signal: ColorSignal::default(),
             sink: None,
+            output_file: None,
         }
     }
 }