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/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/panels/midi_editor.rs

@@ -265,42 +265,20 @@ fn draw_roll_background(painter: &egui::Painter, g: &Roll<'_>) {
         }
     }
     // Grid.
-    let step = grid_stride(grid_ticks, px_per_tick);
     let mut tick = 0;
     while tick <= len_ticks {
         let bar = tick % (TICKS_PER_QUARTER * 4) == 0;
         let x = g.x_of(tick);
-        painter.line_segment(
-            [pos2(x, roll.min.y), pos2(x, roll.max.y)],
-            Stroke::new(1.0, if bar { Color32::from_rgb(80, 80, 86) } else { Color32::from_rgb(50, 50, 54) }),
-        );
-        tick = tick.saturating_add(step);
+        if grid_ticks as f32 * px_per_tick > 4.0 || bar {
+            painter.line_segment(
+                [pos2(x, roll.min.y), pos2(x, roll.max.y)],
+                Stroke::new(1.0, if bar { Color32::from_rgb(80, 80, 86) } else { Color32::from_rgb(50, 50, 54) }),
+            );
+        }
+        tick += grid_ticks;
     }
 }
 
-/// Closest lines the grid draws, in points.
-const MIN_GRID_PX: f32 = 4.0;
-
-/// Ticks between grid lines: the grid when its lines are more than [`MIN_GRID_PX`] apart, else
-/// as few common multiples of the grid and a bar as keep that spacing (so every line is a snap
-/// position and falls on a bar). The lines drawn are then bounded by the roll's width, however long
-/// the clip, instead of by its length.
-fn grid_stride(grid_ticks: i64, px_per_tick: f32) -> i64 {
-    let bar_ticks = TICKS_PER_QUARTER * 4;
-    if grid_ticks as f32 * px_per_tick > MIN_GRID_PX {
-        return grid_ticks.max(1);
-    }
-    let grid = grid_ticks.max(1);
-    let (mut a, mut b) = (grid, bar_ticks);
-    while b != 0 {
-        (a, b) = (b, a % b);
-    }
-    let common = (grid / a).saturating_mul(bar_ticks);
-    // `as` saturates a huge or NaN quotient; a zero `px_per_tick` gives one common multiple.
-    let n = (MIN_GRID_PX / (common as f32 * px_per_tick)).ceil().max(1.0) as i64;
-    common.saturating_mul(n)
-}
-
 /// The notes, with a drag in progress applied.
 fn draw_notes(app: &SoundApp, painter: &egui::Painter, g: &Roll<'_>, base: Color32) {
     let Roll { sequence, grid_ticks, .. } = *g;
@@ -500,24 +478,6 @@ fn velocity_lane(app: &mut SoundApp, ui: &mut Ui, g: &Roll<'_>, cid: ClipId, bas
 mod tests {
     use super::*;
 
-    #[test]
-    fn grid_lines_are_bounded_by_the_roll_width_not_the_clip_length() {
-        let sixteenth = TICKS_PER_QUARTER / 4;
-        // A 1000 pt roll showing a 4-bar clip: every sixteenth is 16 pt apart and drawn.
-        assert_eq!(grid_stride(sixteenth, 1000.0 / (16.0 * TICKS_PER_QUARTER as f32)), sixteenth);
-        // The same roll over a clip a billion ticks long: at most a line every 4 pt.
-        let (len, px_per_tick) = (1_000_000_000i64, 1000.0 / 1_000_000_000.0f32);
-        let step = grid_stride(sixteenth, px_per_tick);
-        assert_eq!(step % (TICKS_PER_QUARTER * 4), 0, "lines fall on bars");
-        assert!((len / step) <= 251, "{} lines", len / step);
-        // a dotted quarter grid (1440 ticks): every line is also a grid position
-        let dotted = TICKS_PER_QUARTER * 3 / 2;
-        let step = grid_stride(dotted, px_per_tick);
-        assert_eq!((step % dotted, step % (TICKS_PER_QUARTER * 4)), (0, 0), "lines are snap positions on bars: {step}");
-        assert!((len / step) <= 251, "{} lines", len / step);
-        assert!(grid_stride(0, 0.0) >= TICKS_PER_QUARTER * 4, "a zero grid or width still advances");
-    }
-
     #[test]
     fn switching_clip_drops_selection_and_gestures() {
         let mut st = MidiEditorState { selected: vec![0, 2], clip: Some(ClipId(1)), band: Some(pos2(1.0, 2.0)), ..Default::default() };

crates/ui-egui/src/panels/video_window.rs

@@ -115,20 +115,6 @@ fn first_movie_aspect(s: &soundcraft_model::Session) -> Option<f32> {
     s.videos.first().map(|v| v.aspect() as f32)
 }
 
-/// Most thumbnails one clip draws in the Video Universe strip: 8 pt each, so more than the
-/// widest strip holds.
-const MAX_UNIVERSE_THUMBS: usize = 1 << 12;
-
-/// Left edges of the thumbnails of a clip starting at `clip_x` (each `tw` wide) that lie in
-/// `shown_min..shown_max`. The index and position are worked out in `f64`: an `f32` this far from
-/// the origin (a clip starting a billion points left) cannot step by a thumbnail's width.
-fn thumb_xs(clip_x: f32, shown_min: f32, shown_max: f32, tw: f32) -> impl Iterator<Item = f32> {
-    let (origin, tw) = (f64::from(clip_x), f64::from(tw));
-    let first = ((f64::from(shown_min) - origin) / tw).floor().max(0.0);
-    let count = ((f64::from(shown_max) - origin) / tw - first).ceil().max(0.0) as usize;
-    (0..count.min(MAX_UNIVERSE_THUMBS)).map(move |i| (origin + (first + i as f64) * tw) as f32)
-}
-
 fn universe(app: &mut SoundApp, ctx: &egui::Context) {
     app.video.poll(ctx);
     let s = app.engine.session_arc();
@@ -159,10 +145,8 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
             }
             let tw = (cr.height() * v.aspect() as f32).max(8.0);
             let hpx = ((cr.height() * ppp / 16.0).ceil() * 16.0).clamp(16.0, 256.0) as u32;
-            // Only the thumbnails inside the strip, counted rather than stepped: a clip starting
-            // far left of it has an edge so large that adding a thumbnail's width wouldn't move it.
-            let shown = cr.intersect(r);
-            for x in thumb_xs(cr.min.x, shown.min.x, shown.max.x, tw) {
+            let mut x = cr.min.x;
+            while x < cr.max.x {
                 let secs = movie_secs(&s, c.start, offset, strip_position(x, r.min.x, r.width(), end, c.start)).max(0.0);
                 if let Some(tex) = app.video.thumb(ctx, key, &v.path, secs, hpx) {
                     let tr = Rect::from_min_size(pos2(x, cr.min.y), vec2(tw, cr.height()));
@@ -170,6 +154,7 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
                     let uv = Rect::from_min_max(pos2(0.0, 0.0), pos2((vis.width() / tw).clamp(0.0, 1.0), 1.0));
                     painter.image(tex.id(), vis, uv, Color32::WHITE);
                 }
+                x += tw;
             }
         }
         // The Edit window's visible range and the playhead.
@@ -197,18 +182,7 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
 
 #[cfg(test)]
 mod tests {
-    use super::{strip_position, thumb_xs};
-
-    #[test]
-    fn thumbnails_of_a_clip_far_left_of_the_strip_are_evenly_spaced() {
-        // 2^30 points left of the strip with 8 pt thumbnails: f32 steps of `first + i` here are 16 apart
-        let xs: Vec<f32> = thumb_xs(-1_073_741_824.0, 0.0, 80.0, 8.0).collect();
-        assert_eq!(xs.len(), 10);
-        assert!(xs.windows(2).all(|w| w[1] - w[0] == 8.0), "{xs:?}");
-        assert_eq!(xs[0], 0.0);
-        // a clip inside the strip starts at its own edge
-        assert_eq!(thumb_xs(10.0, 10.0, 34.0, 8.0).collect::<Vec<_>>(), vec![10.0, 18.0, 26.0]);
-    }
+    use super::strip_position;
 
     // A 1024 px strip over 1_024_000 samples: 1000 samples per point, exact in f32.
     const LEFT: f32 = 100.0;

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}"))?;