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

crates/rustmotion/src/encode/audio.rs

@@ -64,12 +64,8 @@ pub(crate) fn decode_audio_file(path: &str) -> Result<(Vec<f32>, u32, u32)> {
         })?;
 
     let track_id = track.id;
-    let sample_rate = track.codec_params.sample_rate.unwrap_or(44100);
-    let channels = track
-        .codec_params
-        .channels
-        .map(|c| c.count() as u32)
-        .unwrap_or(2);
+    let header_sample_rate = track.codec_params.sample_rate;
+    let header_channels = track.codec_params.channels.map(|c| c.count() as u32);
 
     let mut decoder = symphonia::default::get_codecs()
         .make(&track.codec_params, &DecoderOptions::default())
@@ -79,6 +75,7 @@ pub(crate) fn decode_audio_file(path: &str) -> Result<(Vec<f32>, u32, u32)> {
         })?;
 
     let mut all_samples: Vec<f32> = Vec::new();
+    let mut decoded_spec: Option<(u32, u32)> = None;
 
     loop {
         let packet = match format.next_packet() {
@@ -107,11 +104,40 @@ pub(crate) fn decode_audio_file(path: &str) -> Result<(Vec<f32>, u32, u32)> {
         sample_buf.copy_interleaved_ref(decoded);
 
         all_samples.extend_from_slice(sample_buf.samples());
+
+        if decoded_spec.is_none() {
+            decoded_spec = Some((spec.rate, spec.channels.count() as u32));
+        }
     }
 
+    let (sample_rate, channels) =
+        resolve_decoded_spec(decoded_spec, header_sample_rate, header_channels);
+
     Ok((all_samples, sample_rate, channels))
 }
 
+/// `decode_audio_file`'s `(sample_rate, channels)` return value: the actual
+/// spec of the first packet that decoded, when there was one — that's what
+/// `all_samples` was interleaved from — falling back to the container
+/// header's own declaration only when nothing ever decoded at all. Some
+/// demuxers leave the header fields absent or, for formats whose per-frame
+/// syntax can encode a channel count the container-level probe cannot see
+/// (e.g. an ADTS/AAC stream with an implicit `channel_configuration`),
+/// disagreeing with what the codec itself produces — trusting the header
+/// unconditionally there silently mis-sizes every downstream stereo/mono
+/// interpretation of `all_samples`. `44100`/`2` is the last-resort default,
+/// unchanged from before this function tracked a decoded spec at all.
+fn resolve_decoded_spec(
+    decoded_spec: Option<(u32, u32)>,
+    header_sample_rate: Option<u32>,
+    header_channels: Option<u32>,
+) -> (u32, u32) {
+    decoded_spec.unwrap_or((
+        header_sample_rate.unwrap_or(44100),
+        header_channels.unwrap_or(2),
+    ))
+}
+
 /// Duration, sample rate and channel count of a local audio file — the
 /// `rustmotion info` answer to "how long is this audio track?".
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -479,6 +505,49 @@ fn resample_linear(samples: &[f32], src_rate: u32, dst_rate: u32) -> Vec<f32> {
 mod tests {
     use super::*;
 
+    // ── decode_audio_file must trust the decoded spec, not the header ────────
+    //
+    // Reproducing the exact divergence through a real file needs a decoder
+    // whose *decoded* output can genuinely disagree with what the container
+    // probed ahead of time. Tracing one concrete case where this happens in
+    // the wild — `symphonia-codec-aac`'s ADTS reader leaving `codec_params.channels`
+    // `None` when `channel_configuration == 0` — into `AacDecoder::try_new`
+    // (symphonia-codec-aac 0.5.5, `aac/mod.rs`) shows that without an
+    // `extra_data`/channel-layout fallback it returns a hard
+    // `unsupported_error` instead of ever reaching a decoded packet: this
+    // crate's enabled `symphonia` features (`mp3, wav, ogg, flac, aac`, no
+    // `isomp4`) never populate that fallback for a bare ADTS stream. So this
+    // exact repro surfaces as a loud decode failure, not silent corruption —
+    // a real fixture can't exercise the discard/trust distinction at all.
+    // `resolve_decoded_spec` is exactly that distinction pulled out as a
+    // pure decision, tested directly with the divergence a real file could
+    // produce for a different codec/container pairing.
+    #[test]
+    fn resolve_decoded_spec_prefers_the_decoded_packets_own_spec_over_the_header() {
+        let decoded = Some((48_000, 1));
+        let header_rate = Some(44_100);
+        let header_channels = Some(2);
+        assert_eq!(
+            resolve_decoded_spec(decoded, header_rate, header_channels),
+            (48_000, 1),
+            "a packet actually decoded — all_samples came from its spec, so the \
+             return value must match it, not the header's own guess"
+        );
+    }
+
+    #[test]
+    fn resolve_decoded_spec_falls_back_to_the_header_when_nothing_ever_decoded() {
+        assert_eq!(
+            resolve_decoded_spec(None, Some(22_050), Some(1)),
+            (22_050, 1)
+        );
+    }
+
+    #[test]
+    fn resolve_decoded_spec_falls_back_to_the_hardcoded_default_as_a_last_resort() {
+        assert_eq!(resolve_decoded_spec(None, None, None), (44_100, 2));
+    }
+
     /// Write a minimal, hand-rolled canonical PCM WAV file (16-bit, mono) —
     /// no ffmpeg and no extra crate needed, `symphonia`'s built-in WAV demuxer
     /// decodes this directly.

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 {