fix(encode): create temp files in a private directory

crates/rustmotion/src/encode/video/ffmpeg.rs

@@ -290,6 +290,17 @@ fn ffmpeg_args(
     args
 }
 
+/// Name of the scratch directory a single audio-bearing render call writes
+/// its materialised PCM into. `pid` repeats across the machine's uptime and
+/// `seq` is a small monotonic counter starting at zero, so together they are
+/// a key an outside process could realistically pre-compute and occupy
+/// ahead of time; folding in a nanosecond timestamp neither of those two
+/// alone carries closes that gap without needing a random-number
+/// dependency this crate doesn't already have.
+fn audio_tmp_dir_name(pid: u32, seq: u32, nanos: u128) -> String {
+    format!("rustmotion_audio_{pid}_{seq}_{nanos:x}")
+}
+
 /// Scratch path ffmpeg actually writes to; promoted (renamed) onto the
 /// caller's real `output_path` only after a clean exit with no `pipe_error`.
 /// Kept as a sibling of `output_path` (same directory, same filesystem, so
@@ -461,22 +472,32 @@ fn encode_with_ffmpeg_hw_impl(
     // encodes can run concurrently *within* one process (parallel test
     // threads today; `--frames` segments rendered concurrently by a future
     // distributed worker tomorrow — the exact shape this feature exists to
-    // enable). Two calls sharing a PID-only path would each `create_dir_all`
+    // enable). Two calls sharing a PID-only path would each try to create
     // the same directory, then whichever finishes first would
     // `remove_dir_all` it out from under the other mid-write, surfacing as
     // a bare `NotFound` on `std::fs::write` below. A monotonic counter on
     // top of PID makes every call's directory distinct regardless of
-    // timing.
+    // timing; a nanosecond timestamp on top of *that* keeps the full key
+    // from being small enough for something outside this process to
+    // pre-compute and occupy ahead of time — pid space and a
+    // monotonic-from-zero counter both are. `create_dir` below (not
+    // `_all`) is what actually refuses to proceed if something is already
+    // sitting at the computed path, symlink included; the timestamp only
+    // raises the cost of ever landing on that path in the first place.
     static AUDIO_TMP_DIR_SEQ: AtomicU32 = AtomicU32::new(0);
     let audio_tmp_dir = if !merged_audio.is_empty() {
         let seq = AUDIO_TMP_DIR_SEQ.fetch_add(1, Ordering::Relaxed);
-        Some(std::env::temp_dir().join(format!("rustmotion_audio_{}_{seq}", std::process::id())))
+        let nanos = std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .map(|d| d.as_nanos())
+            .unwrap_or_default();
+        Some(std::env::temp_dir().join(audio_tmp_dir_name(std::process::id(), seq, nanos)))
     } else {
         None
     };
     let pcm_data = if !merged_audio.is_empty() {
         if let Some(ref tmp_dir) = audio_tmp_dir {
-            std::fs::create_dir_all(tmp_dir)?;
+            std::fs::create_dir(tmp_dir)?;
         }
         super::super::audio::mix_audio_tracks_segment(
             &merged_audio,
@@ -837,10 +858,63 @@ pub fn concat_mp4_segments(inputs: &[std::path::PathBuf], output_path: &str) ->
 #[cfg(test)]
 mod tests {
     use super::{
-        ffmpeg_args, ffmpeg_partial_output_path, parse_encoder_names, select_hardware_encoder,
-        HardwareSelection,
+        audio_tmp_dir_name, ffmpeg_args, ffmpeg_partial_output_path, parse_encoder_names,
+        select_hardware_encoder, HardwareSelection,
     };
 
+    // ── audio scratch directory naming: not fully predictable from outside ──
+
+    #[test]
+    fn audio_tmp_dir_name_differs_across_calls_that_share_pid_and_seq() {
+        // A pid+seq pair is small enough to pre-seed exhaustively from
+        // outside the process; folding in a nanosecond timestamp neither of
+        // those two alone carries means a name computed ahead of time from
+        // pid+seq no longer identifies the exact directory this process
+        // will actually create.
+        let a = audio_tmp_dir_name(1234, 0, 111);
+        let b = audio_tmp_dir_name(1234, 0, 222);
+        assert_ne!(
+            a, b,
+            "same pid+seq, different nanos, must differ: {a} vs {b}"
+        );
+    }
+
+    #[test]
+    fn audio_tmp_dir_name_is_stable_for_identical_inputs() {
+        assert_eq!(audio_tmp_dir_name(1, 2, 3), audio_tmp_dir_name(1, 2, 3));
+    }
+
+    /// Characterizes the exact property this fix depends on: swapping
+    /// `create_dir_all` for `create_dir` at the audio scratch directory's
+    /// creation site turns "adopt whatever is already there" into "refuse
+    /// outright" the moment something — attacker-planted symlink included —
+    /// already occupies that path.
+    #[test]
+    fn create_dir_refuses_an_already_occupied_path_that_create_dir_all_would_have_adopted() {
+        let path = std::env::temp_dir().join(format!(
+            "rustmotion_audit_ws_c_preexisting_dir_{}_{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        let _ = std::fs::remove_dir_all(&path);
+        std::fs::create_dir(&path).expect("set up a pre-existing directory at the target path");
+
+        assert!(
+            std::fs::create_dir_all(&path).is_ok(),
+            "create_dir_all silently succeeding on a pre-existing directory is exactly the \
+             behavior that let a hostile pre-planted directory (or symlink) be adopted"
+        );
+        assert!(
+            std::fs::create_dir(&path).is_err(),
+            "create_dir must refuse the same pre-existing path instead of adopting it"
+        );
+
+        let _ = std::fs::remove_dir_all(&path);
+    }
+
     // ── partial-output-path naming (pure) ────────────────────────────────────
 
     #[test]

crates/rustmotion/src/encode/video_audio.rs

@@ -255,6 +255,32 @@ pub fn build_atempo_filter(rate: f64) -> Option<String> {
 
 // ─── Cache-keyed temp WAV path ────────────────────────────────────────────────
 
+/// Base directory the extracted-audio WAV cache lives under.
+///
+/// `std::env::temp_dir()` is shared and, on most Unix systems, world-writable
+/// — combined with `wav_cache_path`'s hash being deterministic (which it has
+/// to be, for the cache to ever hit twice), a different local user could
+/// compute the exact cache path ahead of time and plant content there before
+/// this process ever ran. `dirs::cache_dir()` is per-user (`~/Library/Caches`
+/// on macOS, `~/.cache` on Linux), so the same determinism that makes
+/// caching useful stops doubling as a cross-user attack surface. Falls back
+/// to `temp_dir()` only on a platform with no notion of a user cache
+/// directory at all — still better than failing outright, and consistent
+/// with every other fallback in this codebase preferring a degraded mode
+/// over an unusable one.
+fn wav_cache_base_dir() -> PathBuf {
+    let base = dirs::cache_dir()
+        .unwrap_or_else(std::env::temp_dir)
+        .join("rustmotion");
+    let _ = std::fs::create_dir_all(&base);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        let _ = std::fs::set_permissions(&base, std::fs::Permissions::from_mode(0o700));
+    }
+    base
+}
+
 fn wav_cache_path(src: &str, trim_start: f64, trim_end: Option<f64>, rate: f64) -> PathBuf {
     let mut hasher = DefaultHasher::new();
     src.hash(&mut hasher);
@@ -280,7 +306,21 @@ fn wav_cache_path(src: &str, trim_start: f64, trim_end: Option<f64>, rate: f64)
     }
 
     let hash = hasher.finish();
-    std::env::temp_dir().join(format!("rustmotion_vidaud_{:016x}.wav", hash))
+    wav_cache_base_dir().join(format!("rustmotion_vidaud_{:016x}.wav", hash))
+}
+
+/// Whether `path` is safe to reuse as a cache hit: a genuine regular file,
+/// not a symlink. `wav_cache_path` now resolves under a per-user directory
+/// (see `wav_cache_base_dir`), which already rules out a *different* user
+/// planting one; this additionally refuses to follow a symlink planted by
+/// anything running as the *same* user (a compromised sibling process, or a
+/// leftover from before that directory existed) into wherever it points.
+/// `symlink_metadata` — unlike `Path::exists`/`std::fs::metadata` — reports
+/// on the directory entry itself rather than whatever it resolves to.
+fn cached_wav_is_trustworthy(path: &std::path::Path) -> bool {
+    std::fs::symlink_metadata(path)
+        .map(|m| m.file_type().is_file())
+        .unwrap_or(false)
 }
 
 /// Scratch path ffmpeg writes to before a successful extraction is promoted
@@ -319,8 +359,9 @@ fn extract_audio_to_wav(
 ) -> Option<PathBuf> {
     let wav_path = wav_cache_path(src, trim_start, trim_end, rate);
 
-    // Reuse cached extraction.
-    if wav_path.exists() {
+    // Reuse cached extraction — but only a genuine regular file placed here
+    // by a previous extraction; see `cached_wav_is_trustworthy`.
+    if cached_wav_is_trustworthy(&wav_path) {
         return Some(wav_path);
     }
 
@@ -488,6 +529,87 @@ mod tests {
     use super::*;
     use crate::loader::load_scenario_from_source;
 
+    // ── Cache directory: per-user, not the shared world-writable temp dir ────
+
+    #[test]
+    fn wav_cache_path_does_not_sit_directly_inside_the_bare_shared_temp_dir() {
+        let cached = wav_cache_path("foo.mp4", 0.0, None, 1.0);
+        let shared_temp = std::env::temp_dir();
+        assert_ne!(
+            cached.parent(),
+            Some(shared_temp.as_path()),
+            "the cached WAV must live under a dedicated subdirectory, not directly inside the \
+             shared temp dir a same-machine, different-user attacker can also write to: got {}",
+            cached.display()
+        );
+    }
+
+    // ── Cache entries must be verified, not merely `exists()` ────────────────
+
+    #[cfg(unix)]
+    #[test]
+    fn a_symlink_at_the_cache_path_is_never_trusted_as_a_cache_hit() {
+        let target = std::env::temp_dir().join(format!(
+            "rm_vidaud_symlink_target_{}_{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        std::fs::write(&target, b"not a wav, planted by someone else").unwrap();
+
+        let link = std::env::temp_dir().join(format!(
+            "rm_vidaud_symlink_link_{}_{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        let _ = std::fs::remove_file(&link);
+        std::os::unix::fs::symlink(&target, &link).expect("create symlink fixture");
+
+        assert!(
+            !cached_wav_is_trustworthy(&link),
+            "a symlink sitting at the cache path must never be treated as a valid cache hit, \
+             regardless of what it points to"
+        );
+
+        let mut real_file = link.with_file_name(format!(
+            "rm_vidaud_real_file_{}_{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        real_file.set_extension("wav");
+        std::fs::write(&real_file, b"RIFF....").unwrap();
+        assert!(
+            cached_wav_is_trustworthy(&real_file),
+            "a genuine regular file must still be trusted"
+        );
+
+        let _ = std::fs::remove_file(&target);
+        let _ = std::fs::remove_file(&link);
+        let _ = std::fs::remove_file(&real_file);
+    }
+
+    #[test]
+    fn a_missing_path_is_not_trustworthy() {
+        let path = std::env::temp_dir().join(format!(
+            "rm_vidaud_never_created_{}_{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        let _ = std::fs::remove_file(&path);
+        assert!(!cached_wav_is_trustworthy(&path));
+    }
+
     // ── Helpers ──────────────────────────────────────────────────────────────
 
     fn load(json: &str) -> ResolvedScenario {