address review: make the fallback take folder exclusively under an unguessable name; qualify the owner-only claim to Unix

crates/engine/src/autosave.rs

@@ -425,6 +425,27 @@ pub fn create_folder(dir: &Path) -> std::io::Result<()> {
     builder.create(dir)
 }
 
+/// A new folder directly in `parent`, named `{prefix}-` and 32 random hex digits. It is made
+/// exclusively (it is created by this call or the call fails), so a folder or link another account
+/// put there first is never used, and the name cannot be guessed ahead. On Unix it is owner-only
+/// (`0o700`); on other systems it has the mode `parent` gives it.
+pub fn create_unique_folder(parent: &Path, prefix: &str) -> std::io::Result<PathBuf> {
+    use std::hash::{BuildHasher, Hasher};
+    for _ in 0..16 {
+        let random = |_| std::collections::hash_map::RandomState::new().build_hasher().finish();
+        let dir = parent.join(format!("{prefix}-{:016x}{:016x}", random(0), random(1)));
+        let mut builder = std::fs::DirBuilder::new();
+        #[cfg(unix)]
+        std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700);
+        match builder.create(&dir) {
+            Ok(()) => return Ok(dir),
+            Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {}
+            Err(err) => return Err(err),
+        }
+    }
+    Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "no unused folder name"))
+}
+
 fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
     let mut f = std::fs::File::create(path)?;
     f.write_all(bytes)?;
@@ -447,6 +468,22 @@ fn io_error(path: &Path, err: std::io::Error) -> EngineError {
 mod tests {
     use super::*;
     use crate::io::open_session;
+
+    #[test]
+    fn unique_folders_are_new_and_owner_only() {
+        let parent = std::env::temp_dir().join(format!("soundcraft-unique-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&parent);
+        std::fs::create_dir_all(&parent).unwrap();
+        let (a, b) = (create_unique_folder(&parent, "T").unwrap(), create_unique_folder(&parent, "T").unwrap());
+        assert_ne!(a, b);
+        assert!(a.is_dir() && a.parent() == Some(parent.as_path()));
+        assert_eq!(a.file_name().unwrap().len(), "T-".len() + 32);
+        #[cfg(unix)]
+        assert_eq!(std::os::unix::fs::PermissionsExt::mode(&std::fs::metadata(&a).unwrap().permissions()) & 0o777, 0o700);
+        assert!(create_unique_folder(&parent.join("missing"), "T").is_err(), "no parents are made");
+        let _ = std::fs::remove_dir_all(&parent);
+    }
+
     use serde_json::json;
     use soundcraft_audio_io::AudioBuffer;
     use soundcraft_model::{ChannelFormat, Clip, TrackKind};

crates/engine/src/clip_group_file.rs

@@ -103,7 +103,7 @@ fn decode_group_audio(
                 let bytes = crate::io::read_file(&full)?;
                 let (_, buf) = soundcraft_audio_io::decode(&bytes, Some("wav")).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?;
                 let buf = crate::io::at_rate(buf, rate, crate::io::MAX_IMPORT_SAMPLES)
-                    .map_err(|r| EngineError::Io(format!("{rel}: {}", r.message("too large to import"))))?;
+                    .ok_or_else(|| EngineError::Io(format!("{rel}: too large to import")))?;
                 decoded.push((rel.clone(), buf));
                 index.insert(key, decoded.len() - 1);
                 decoded.len() - 1

crates/engine/src/cmd/sampler.rs

@@ -75,7 +75,7 @@ pub(crate) fn attach(e: &mut Engine, t: TrackId, name: &str, bytes: &[u8], path:
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
     let buf = crate::io::at_rate(buf, e.session().sample_rate.hz(), MAX_SAMPLE_VALUES)
-        .map_err(|r| EngineError::Io(format!("{name}: {}", r.message("too long for a sample (ten minutes of 192 kHz stereo at most)"))))?;
+        .ok_or_else(|| EngineError::Io(format!("{name}: too long for a sample (ten minutes of 192 kHz stereo at most)")))?;
     let channels = buf.num_channels();
     let s = e.session_mut();
     let src = crate::io::add_source(s, name, buf, path, format.format);

crates/engine/src/io/import.rs

@@ -7,48 +7,18 @@ use super::{
 /// Largest import we accept (samples per channel × channels), ~4 GiB of f32.
 pub(crate) const MAX_IMPORT_SAMPLES: usize = 1 << 30;
 
-/// The most a file's audio may grow when converted to the session rate. Sessions run at 8 kHz to
-/// 768 kHz (96×); past this a file is lying about its rate, and converting a few kilobytes would
-/// otherwise allocate and filter hundreds of megabytes.
-const MAX_RATE_EXPANSION: u32 = 128;
-
-/// Why [`at_rate`] refused a conversion.
-#[derive(Debug, PartialEq, Eq)]
-pub(crate) enum RateError {
-    /// The converted audio would hold more samples than the limit.
-    TooLarge,
-    /// The file's rate is so far below the session's that converting would grow it past [`MAX_RATE_EXPANSION`].
-    Ratio { from: u32, to: u32 },
-}
-
-impl RateError {
-    /// The refusal as a sentence: `size` says what a size refusal means to the caller.
-    pub(crate) fn message(&self, size: &str) -> String {
-        match self {
-            RateError::TooLarge => size.to_string(),
-            RateError::Ratio { from, to } => {
-                format!("its {from} Hz sample rate is too low to convert to the session's {to} Hz (at most {MAX_RATE_EXPANSION}-fold)")
-            }
-        }
-    }
-}
-
-/// `buf` converted to `rate`, unless it would then hold more than `limit` samples, or grow more
-/// than [`MAX_RATE_EXPANSION`]-fold: a file with a tiny sample rate grows by orders of magnitude.
-pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> std::result::Result<AudioBuffer, RateError> {
+/// `buf` converted to `rate`, unless it would then hold more than `limit` samples: a file with a
+/// tiny sample rate grows by orders of magnitude.
+pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> Option<AudioBuffer> {
     let from = buf.sample_rate.max(1);
-    // widened integers: dividing first would truncate 128.86-fold down to 128
-    if u64::from(rate) > u64::from(from) * u64::from(MAX_RATE_EXPANSION) {
-        return Err(RateError::Ratio { from, to: rate });
-    }
     if buf.frames() as f64 * buf.num_channels() as f64 * f64::from(rate) / f64::from(from) > limit as f64 {
-        return Err(RateError::TooLarge);
+        return None;
     }
     if from != rate {
         buf.channels = soundcraft_dsp::offline::resample(&buf.channels, from, rate);
         buf.sample_rate = rate;
     }
-    Ok(buf)
+    Some(buf)
 }
 
 /// A relative `Audio Files/` path no other source in `s` already uses.
@@ -145,7 +115,7 @@ pub fn decode_import(path: &str, rate: u32) -> Result<DecodedImport> {
 fn decode_import_bytes(name: &str, bytes: &[u8], rate: u32) -> Result<(soundcraft_audio_io::AudioInfo, Arc<SourceAudio>)> {
     let ext = Path::new(name).extension().and_then(OsStr::to_str).map(str::to_ascii_lowercase);
     let (info, buf) = soundcraft_audio_io::decode(bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| EngineError::Io(format!("{name}: {}", e.message("file is too large to import"))))?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or_else(|| EngineError::Io(format!("{name}: file is too large to import")))?;
     if buf.frames() == 0 {
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
@@ -245,22 +215,8 @@ mod rate_tests {
     #[test]
     fn a_tiny_sample_rate_is_refused_before_it_is_converted() {
         let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
-        assert!(at_rate(buf(1), 48_000, 1_000_000).is_err());
+        assert!(at_rate(buf(1), 48_000, 1_000_000).is_none());
         let ok = at_rate(buf(8_000), 48_000, 1_000_000).unwrap();
         assert_eq!((ok.sample_rate, ok.frames()), (48_000, 600));
     }
-
-    #[test]
-    fn an_extreme_rate_expansion_is_refused_whatever_the_sample_budget() {
-        let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
-        // 100 frames at 300 Hz would fit the budget as 128 000 samples, but grow 640-fold.
-        assert_eq!(at_rate(buf(300), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 300, to: 192_000 });
-        assert!(at_rate(buf(1_500), 192_000, usize::MAX).is_ok(), "128-fold is the supported extreme");
-        assert!(at_rate(buf(8_000), 768_000, usize::MAX).is_ok(), "the slowest file at the fastest session");
-        // 128.86-fold: integer division would truncate it to 128 and let it through
-        assert_eq!(at_rate(buf(1_490), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 1_490, to: 192_000 });
-        // a short file that is only over the sample budget is a size refusal, not a rate one
-        assert_eq!(at_rate(buf(48_000), 48_000, 10).unwrap_err(), RateError::TooLarge);
-        assert!(RateError::Ratio { from: 300, to: 192_000 }.message("big").contains("300 Hz"));
-    }
 }

crates/engine/src/io/session.rs

@@ -72,7 +72,7 @@ pub(crate) fn open_session_linked(e: &mut Engine, path: &Path, links: &Path) ->
 /// Decode one source's audio file at the session rate.
 pub(super) fn decode_source(bytes: &[u8], ext: Option<&str>, rate: u32) -> std::result::Result<Arc<SourceAudio>, String> {
     let (_, buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| e.message("too large to load"))?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or("too large to load")?;
     Ok(Arc::new(SourceAudio::new(buf)))
 }
 

crates/ui-egui/src/app.rs

@@ -46,6 +46,7 @@ impl SoundApp {
             services,
             native_menu_bar: false,
             autosave_dir: None,
+            fallback_take_dir: None,
             autosaver: None,
             autosaved_rev: 0,
             leftovers: Vec::new(),

crates/ui-egui/src/app/playback.rs

@@ -155,15 +155,21 @@ impl SoundApp {
     /// autosave folder. Returns the takes it records.
     fn arm_recorder(&mut self) -> Result<Vec<soundcraft_engine::io::Take>, String> {
         let name = soundcraft_engine::io::sanitize_name(&self.engine.session().name);
-        let unsaved_dir = self.autosave_dir.as_ref().map_or_else(std::env::temp_dir, |d| d.join(name));
+        // Without a settings folder the takes go under the temporary folder, in a folder of this
+        // run's own that nobody else could have made first.
+        let unsaved_dir = match &self.autosave_dir {
+            Some(d) => d.join(name),
+            None => self.fallback_take_dir()?,
+        };
         let Some(r) = &mut self.recorder else { return Err("no audio input device".into()) };
         let plan = soundcraft_engine::io::plan_takes(&self.engine, &unsaved_dir, r.channels);
         if plan.is_empty() {
             return Err("no audio track is record-enabled".into());
         }
-        // The takes are unsaved work: the folder they go into under the autosave root is
-        // owner-only, like the autosaves, before the recorder makes anything in it.
-        if self.engine.path.is_none() && self.autosave_dir.is_some() {
+        // The takes are unsaved work: on Unix the folder they go into is owner-only, like the
+        // autosaves, before the recorder makes anything in it (other systems get the folder's
+        // inherited access: no ACLs are set, so the temporary folder's own per-user access applies).
+        if self.engine.path.is_none() {
             for dir in plan.iter().filter_map(|(_, _, path)| path.parent()) {
                 soundcraft_engine::autosave::create_folder(dir).map_err(|e| format!("{}: {e}", dir.display()))?;
             }
@@ -176,6 +182,19 @@ impl SoundApp {
         Ok(takes)
     }
 
+    /// The folder for the takes of a session never saved when there is no autosave folder, made on
+    /// first use: exclusively and under a name nobody can guess, so another account cannot have
+    /// made it first, with open permissions or as a link.
+    fn fallback_take_dir(&mut self) -> Result<std::path::PathBuf, String> {
+        if let Some(dir) = &self.fallback_take_dir {
+            return Ok(dir.clone());
+        }
+        let dir = soundcraft_engine::autosave::create_unique_folder(&std::env::temp_dir(), "SoundCraft")
+            .map_err(|e| format!("temporary take folder: {e}"))?;
+        self.fallback_take_dir = Some(dir.clone());
+        Ok(dir)
+    }
+
     fn finish_recording(&mut self) {
         self.engine.transport.recording = false;
         if let Some(p) = &self.player {

crates/ui-egui/src/lib.rs

@@ -278,6 +278,8 @@ pub struct SoundApp {
     pub native_menu_bar: bool,
     /// Folder for crash-recovery autosaves (native apps set it).
     pub autosave_dir: Option<std::path::PathBuf>,
+    /// Where the takes of a session never saved go without an autosave folder: made once per run, under the temporary folder.
+    fallback_take_dir: Option<std::path::PathBuf>,
     /// Writes the open session's autosaves in the background, once there is something to write.
     autosaver: Option<soundcraft_engine::autosave::Autosaver>,
     /// The document revision last handed to the autosaver (0 once that autosave is stale).

crates/ui-egui/src/video_track.rs

@@ -90,6 +90,13 @@ impl Worker {
 }
 
 fn open_movie(path: &str) -> Result<Movie, String> {
+    // The last check before the file system: the path came from a session file, so the read
+    // roots apply, and only a regular file is opened (a pipe would hold the video worker
+    // forever).
+    soundcraft_engine::file_access::check_read(path)?;
+    if !std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?.is_file() {
+        return Err(format!("{path}: not a regular file"));
+    }
     #[cfg(not(target_arch = "wasm32"))]
     {
         let f = std::fs::File::open(path).map_err(|e| format!("{path}: {e}"))?;