From bf37e1c853bcc19e518bd0c61eb5827f1164edd4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 03:15:25 +0000 Subject: [PATCH] address review: make the fallback take folder exclusively under an unguessable name; qualify the owner-only claim to Unix --- crates/engine/src/autosave.rs | 37 ++++++++++++++++++++++++++++++ crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/app/playback.rs | 26 +++++++++++++++++---- crates/ui-egui/src/lib.rs | 2 ++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 516664da..4d4701cf 100644 --- a/crates/engine/src/autosave.rs +++ b/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 { + 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}; diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 0be771ba..d3171ce8 100644 --- a/crates/ui-egui/src/app.rs +++ b/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(), diff --git a/crates/ui-egui/src/app/playback.rs b/crates/ui-egui/src/app/playback.rs index a23518c0..b570a647 100644 --- a/crates/ui-egui/src/app/playback.rs +++ b/crates/ui-egui/src/app/playback.rs @@ -156,16 +156,19 @@ impl SoundApp { fn arm_recorder(&mut self) -> Result, String> { let name = soundcraft_engine::io::sanitize_name(&self.engine.session().name); // Without a settings folder the takes go under the temporary folder, in a folder of this - // process's own rather than beside every other account's files. - let unsaved_dir = - self.autosave_dir.as_ref().map_or_else(|| std::env::temp_dir().join(format!("SoundCraft-{}", std::process::id())), |d| d.join(name)); + // 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 is owner-only, like the - // autosaves, before the recorder makes anything in it. + // 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()))?; @@ -179,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 { + 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 { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1618b315..d70da7e5 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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, + /// 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, /// Writes the open session's autosaves in the background, once there is something to write. autosaver: Option, /// The document revision last handed to the autosaver (0 once that autosave is stale).