diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index 9ca732d3..bfeedf6c 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -87,20 +87,27 @@ fn decode_group_audio( cmd_id: &str, ) -> Result<(Vec<(String, soundcraft_audio_io::AudioBuffer)>, BTreeMap)> { let (mut decoded, mut index, mut file_of) = (Vec::new(), BTreeMap::new(), BTreeMap::new()); + // The folder as the file system knows it, so a link among the group's files can't lead out. + let dir = if dir.as_os_str().is_empty() { Path::new(".") } else { dir }; + let root = crate::io::canonical_path(dir)?; for (old, rel) in audio_entries { let full = dir.join(rel); + let escapes = |rel: &str| EngineError::BadParams(cmd_id.into(), format!("audio path escapes the group folder: {rel}")); if !full.starts_with(dir) || Path::new(rel) .components() .any(|c| matches!(c, std::path::Component::ParentDir | std::path::Component::RootDir | std::path::Component::Prefix(_))) { - return Err(EngineError::BadParams(cmd_id.into(), format!("audio path escapes the group folder: {rel}"))); + return Err(escapes(rel)); + } + let key = crate::io::canonical_path(&full)?; + if !key.starts_with(&root) { + return Err(escapes(rel)); } - let key = crate::io::file_key(&full); let n = match index.get(&key) { Some(n) => *n, None => { - let bytes = crate::io::read_file(&full)?; + let bytes = crate::io::read_file_inside(&full, &root)?; 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"))))?; @@ -268,9 +275,59 @@ mod tests { std::fs::write(&path, file.to_string()).unwrap(); let r = e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"})); assert!(matches!(r, Err(crate::EngineError::BadParams(..))), "absolute paths are refused: {r:?}"); + // A link inside the folder that leads out of it is refused once resolved. + #[cfg(unix)] + { + let outside = std::env::temp_dir().join(format!("sc-grp-outside-{}.wav", std::process::id())); + std::fs::copy(dir.join("Wait... what_0.wav"), &outside).unwrap(); + std::os::unix::fs::symlink(&outside, dir.join("link.wav")).unwrap(); + for rel in file["audio"].as_object_mut().unwrap().values_mut() { + *rel = json!("link.wav"); + } + std::fs::write(&path, file.to_string()).unwrap(); + let r = e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"})); + assert!(matches!(r, Err(crate::EngineError::BadParams(..))), "links out of the folder are refused: {r:?}"); + let _ = std::fs::remove_file(&outside); + } let _ = std::fs::remove_dir_all(&dir); } + /// Under a read root, a link in the group folder to a folder outside the root is refused the same way + /// whether or not the file asked for exists there, so the group cannot be used to probe for names. + #[cfg(unix)] + #[test] + fn links_out_of_the_read_root_are_refused_alike_for_existing_and_missing_files() { + let base = std::env::temp_dir().join(format!("sc-grp-root-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (dir, outside) = (base.join("group"), base.join("outside")); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + let path = dir.join("drums.scgrp").to_string_lossy().to_string(); + let mut e = crate::demo::demo_engine(); + let kick = e.session().tracks.iter().find(|t| t.name == "Kick").map(|t| t.id).unwrap(); + let ids: Vec<_> = e.session().track(kick).unwrap().clips().iter().take(1).map(|c| c.id.0).collect(); + e.execute("file.export_clip_groups", &json!({"path": path, "clips": ids})).unwrap(); + let mut file: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let exported = file["audio"].as_object().unwrap().values().next().unwrap().as_str().unwrap().to_string(); + std::fs::copy(dir.join(&exported), outside.join("real.wav")).unwrap(); + std::os::unix::fs::symlink(&outside, dir.join("out")).unwrap(); + let roots = crate::file_access::AutomationRoots::new(&[&dir], &[]).unwrap().unwrap(); + let mut refusals = Vec::new(); + for target in ["out/real.wav", "out/missing.wav"] { + for rel in file["audio"].as_object_mut().unwrap().values_mut() { + *rel = json!(target); + } + std::fs::write(&path, file.to_string()).unwrap(); + let r = crate::file_access::confine(Some(&roots), || { + e.execute("file.import_clip_groups", &json!({"path": path, "at": 1000, "track": "Bass"})) + }); + refusals.push(r.unwrap_err().to_string()); + } + assert!(refusals[0].contains("outside the read root"), "{}", refusals[0]); + assert_eq!(refusals[0].replace("real.wav", "X"), refusals[1].replace("missing.wav", "X"), "existing and missing names are refused alike"); + let _ = std::fs::remove_dir_all(&base); + } + #[test] fn imported_clips_carry_no_auto_rename_scheme() { let dir = std::env::temp_dir().join(format!("sc-grp-names-{}", std::process::id())); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 9fbf07ad..01472aa3 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -65,6 +65,69 @@ fn read_file_within(path: &Path, session: bool) -> Result> { Ok(data) } +/// `O_NONBLOCK` of the platforms that have a FIFO to wait on (std has no name for it, and this crate has no unsafe +/// code or libc); 0 elsewhere, where a FIFO open still waits for a writer. +#[cfg(unix)] +const O_NONBLOCK: i32 = if cfg!(all( + any(target_os = "linux", target_os = "android"), + not(any( + target_arch = "mips", + target_arch = "mips32r6", + target_arch = "mips64", + target_arch = "mips64r6", + target_arch = "sparc", + target_arch = "sparc64" + )) +)) { + 0x800 +} else if cfg!(any( + target_os = "macos", + target_os = "ios", + target_os = "freebsd", + target_os = "netbsd", + target_os = "openbsd", + target_os = "dragonfly" +)) { + 0x4 +} else { + 0 +}; + +/// [`read_file`] for a file that must lie inside `root` (a canonical folder): the file is opened +/// once, and the bytes come from that handle only after the handle is shown to be the file the +/// path resolves to inside `root`. A file swapped in for the path after a check by name is +/// refused instead of read. Where the handle's identity can't be read (non-Unix targets) the +/// resolved path is checked against `root` alone. +pub(crate) fn read_file_inside(path: &Path, root: &Path) -> Result> { + use std::io::Read; + crate::file_access::check_read(path).map_err(EngineError::Io)?; + let err = |err: std::io::Error| EngineError::Io(format!("{}: {err}", path.display())); + // opened without waiting for a FIFO's writer (the type is checked on the opened handle, below) + let mut options = std::fs::OpenOptions::new(); + options.read(true); + #[cfg(unix)] + std::os::unix::fs::OpenOptionsExt::custom_flags(&mut options, O_NONBLOCK); + let mut file = options.open(path).map_err(err)?; + if !file.metadata().map_err(err)?.is_file() { + return Err(EngineError::Io(format!("{}: not a regular file", path.display()))); + } + let key = canonical_path(path)?; + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let (a, b) = (file.metadata().map_err(err)?, std::fs::metadata(&key).map_err(err)?); + if (a.dev(), a.ino()) != (b.dev(), b.ino()) { + return Err(EngineError::Io(format!("{}: changed while it was opened", path.display()))); + } + } + if !key.starts_with(root) { + return Err(EngineError::Io(format!("{}: outside the folder it must stay in", path.display()))); + } + let mut bytes = Vec::new(); + file.read_to_end(&mut bytes).map_err(err)?; + Ok(bytes) +} + /// [`read_file`] as UTF-8 text. pub fn read_text(path: impl AsRef) -> Result { let path = path.as_ref(); @@ -192,6 +255,17 @@ pub(crate) fn make_dir(dir: impl AsRef) -> Result<()> { std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display()))) } +/// `path` with its links followed, inside the automation read root when there is one. The root is +/// checked on the path as named (links followed, a missing tail resolved by its nearest folder) +/// before anything is resolved, so a name that leads out of the root is refused the same way +/// whether or not it exists there. +pub(crate) fn canonical_path(path: impl AsRef) -> Result { + let path = path.as_ref(); + let checked = if path.as_os_str().is_empty() { Path::new(".") } else { path }; + crate::file_access::check_read(checked).map_err(EngineError::Io)?; + std::fs::canonicalize(path).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + /// Write `bytes` to the file at `path`, inside the automation write root when there is one. pub(crate) fn write_path(path: impl AsRef, bytes: impl AsRef<[u8]>) -> Result<()> { let path = path.as_ref(); @@ -446,7 +520,7 @@ impl Out { if cfg!(target_arch = "wasm32") { return Ok(Out::Memory(std::io::Cursor::new(Vec::new()))); } - Ok(Out::File(std::io::BufWriter::with_capacity(1 << 20, std::fs::File::create(part)?))) + Ok(Out::File(std::io::BufWriter::with_capacity(1 << 20, create_new(part)?))) } /// Puts the finished file at `path` and returns its size: the part file is synced and renamed @@ -578,8 +652,28 @@ pub fn export_clips(e: &Engine, ids: &[soundcraft_model::ClipId], dir: &str) -> let audio = soundcraft_mix::render_clips(s, t, c.range()); let buf = AudioBuffer { sample_rate: s.sample_rate.hz(), channels: audio }; let bytes = soundcraft_audio_io::encode(&buf, &EncodeOptions::default()).map_err(|err| EngineError::Io(err.to_string()))?; - let path = clip_file(dir, &sanitize_name(&c.name), &mut names, Path::exists)?; - write_file(&path.to_string_lossy(), &bytes)?; + let stem = sanitize_name(&c.name); + if cfg!(target_arch = "wasm32") { + let path = clip_file(dir, &stem, &mut names, Path::exists)?; + write_file(&path.to_string_lossy(), &bytes)?; + continue; + } + // The name is taken by creating the file, so whatever entry already has it (a link + // to elsewhere included) is left alone rather than written through. An occupied name is + // taken whatever it is (a link whose target is outside the write root too): it is only + // looked at, never resolved, so the root check does not stop the search for a free name. + // The folder itself was checked above. + let (mut file, path) = loop { + let path = clip_file(dir, &stem, &mut names, |p| std::fs::symlink_metadata(p).is_ok())?; + crate::file_access::check_write(&path).map_err(EngineError::Io)?; + match std::fs::OpenOptions::new().write(true).create_new(true).open(&path) { + Ok(f) => break (f, path), + // taken since it was looked at: `names` has it now, so the next name is tried + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(err) => return Err(EngineError::Io(format!("{}: {err}", path.display()))), + } + }; + std::io::Write::write_all(&mut file, &bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))?; } Ok(names.taken.len()) } @@ -644,13 +738,13 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess { src.path = f.join(&src.path).to_string_lossy().into_owned(); } - let exists_rel = same_dir && !Path::new(&src.path).is_absolute() && dir.join(&src.path).exists(); + let exists_rel = same_dir && !Path::new(&src.path).is_absolute() && file_found(dir.join(&src.path)); let shared = shared_paths.get(i).copied().unwrap_or(false); if exists_rel && !src.unsaved && !shared { continue; } let in_folder = Path::new(&src.path).strip_prefix(&audio_dir).ok().and_then(Path::to_str).map(str::to_string); - if let Some(f) = in_folder.filter(|_| !src.unsaved && Path::new(&src.path).is_file()) { + if let Some(f) = in_folder.filter(|_| !src.unsaved && file_found(&src.path)) { src.path = format!("Audio Files/{f}"); continue; } @@ -672,8 +766,11 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess let mut stem = sanitize_name(&src.name); if overwrite { // One file per source id so autosaves never collide or pile up. + // (With only an automation write root, `file_found` cannot read an existing file, so each save + // writes a new numbered WAV: reuse never reads a file the roots do not allow, which keeps a save + // from revealing whether an unreadable file matches the audio.) stem = format!("{stem}_{}", src.id.0); - if audio_dir.join(format!("{stem}.wav")).exists() { + if file_found(audio_dir.join(format!("{stem}.wav"))) { src.path = format!("Audio Files/{stem}.wav"); src.unsaved = false; files.insert(Arc::as_ptr(audio), src.path.clone()); @@ -756,11 +853,22 @@ fn sync_dir(dir: &Path) { fn write_synced(path: &Path, bytes: &[u8]) -> std::io::Result<()> { use std::io::Write; - let mut f = std::fs::File::create(path)?; + let mut f = create_new(path)?; f.write_all(bytes)?; f.sync_all() } +/// Creates `path` as a new file. A leftover of that name (a part file from an interrupted write) +/// is removed first, but an existing entry is never opened: a link planted at a predictable name +/// in the destination folder can't redirect the write elsewhere. +fn create_new(path: &Path) -> std::io::Result { + match std::fs::remove_file(path) { + Err(err) if err.kind() != std::io::ErrorKind::NotFound => return Err(err), + _ => {} + } + std::fs::OpenOptions::new().write(true).create_new(true).open(path) +} + /// Copies `from` to `to`, which must not exist yet; an existing entry, even a link, is never followed. fn copy_new(from: &Path, to: &Path) -> std::io::Result<()> { let mut src = std::fs::File::open(from)?; @@ -840,7 +948,8 @@ fn same_path(a: &Path, b: &Path) -> bool { /// Store `bytes` as the first of `.wav`, `_01.wav`, … in `dir` that is free or already /// holds exactly these bytes, so saving never replaces audio; returns the name and whether it was -/// written. +/// written. Where the automation roots allow writing in `dir` but not reading (only a write root), +/// an existing file is never compared, so each save writes a new numbered WAV. fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { use std::io::Write; for k in 0..10_000u32 { @@ -855,7 +964,12 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { return Ok((name, true)); } Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { - if std::fs::metadata(&path).is_ok_and(|m| m.len() == bytes.len() as u64) && std::fs::read(&path).is_ok_and(|b| b == bytes) { + // Reused only when it may be read: whether it equals the audio must not be + // learnt about a file the roots allow writing beside but not reading. + if crate::file_access::check_read(&path).is_ok() + && std::fs::metadata(&path).is_ok_and(|m| m.len() == bytes.len() as u64) + && std::fs::read(&path).is_ok_and(|b| b == bytes) + { return Ok((name, false)); } } @@ -1054,6 +1168,89 @@ pub fn plan_takes(e: &Engine, unsaved_dir: &Path, input_channels: usize) -> Vec< .collect() } +#[cfg(test)] +mod exclusive_tests { + use super::*; + + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("soundcraft-io-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + #[cfg(unix)] + #[test] + fn a_planted_link_at_the_part_name_is_not_written_through() { + let dir = scratch("part-link"); + let (target, part) = (dir.join("elsewhere"), dir.join("session.scraft.tmp")); + std::os::unix::fs::symlink(&target, &part).unwrap(); + write_synced(&part, b"new").unwrap(); + assert!(!target.exists(), "the link's target was created"); + assert_eq!(std::fs::read(&part).unwrap(), b"new", "the link itself was replaced by the file"); + // A leftover regular part file is still replaced. + write_synced(&part, b"newer").unwrap(); + assert_eq!(std::fs::read(&part).unwrap(), b"newer"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn a_file_read_within_a_root_must_resolve_inside_it() { + let dir = scratch("within"); + let (inside, outside) = (dir.join("group"), dir.join("elsewhere")); + std::fs::create_dir_all(&inside).unwrap(); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(inside.join("a.wav"), b"a").unwrap(); + std::fs::write(outside.join("b.wav"), b"b").unwrap(); + std::os::unix::fs::symlink(outside.join("b.wav"), inside.join("link.wav")).unwrap(); + let root = canonical_path(&inside).unwrap(); + assert_eq!(read_file_inside(&inside.join("a.wav"), &root).unwrap(), b"a"); + assert!(read_file_inside(&inside.join("link.wav"), &root).is_err()); + // a FIFO with no writer is refused, not waited on (Linux: the coreutils tool makes it) + #[cfg(target_os = "linux")] + if std::process::Command::new("mkfifo").arg(inside.join("pipe.wav")).status().is_ok_and(|s| s.success()) { + let err = read_file_inside(&inside.join("pipe.wav"), &root).unwrap_err().to_string(); + assert!(err.contains("not a regular file"), "{err}"); + } + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn export_skips_a_name_held_by_a_dangling_link() { + let dir = scratch("export-link"); + let e = crate::demo::demo_engine(); + let (t, c) = e.session().tracks.iter().find_map(|t| t.clips().iter().find(|c| c.is_audio()).map(|c| (t.id, c.clone()))).unwrap(); + let _ = t; + let target = dir.join("elsewhere.wav"); + std::os::unix::fs::symlink(&target, dir.join(format!("{}.wav", sanitize_name(&c.name)))).unwrap(); + assert_eq!(export_clips(&e, &[c.id], &dir.to_string_lossy()).unwrap(), 1); + assert!(!target.exists(), "the export followed the link"); + assert!(dir.join(format!("{}-2.wav", sanitize_name(&c.name))).is_file(), "the next free name was used"); + let _ = std::fs::remove_dir_all(&dir); + } + + /// With a write root, a link out of it that holds the name is still just an occupied name. + #[cfg(unix)] + #[test] + fn export_skips_a_link_out_of_the_write_root_too() { + let dir = scratch("export-root-link"); + let e = crate::demo::demo_engine(); + let c = e.session().tracks.iter().find_map(|t| t.clips().iter().find(|c| c.is_audio()).cloned()).unwrap(); + let elsewhere = scratch("export-root-elsewhere"); + let target = elsewhere.join("target.wav"); + std::os::unix::fs::symlink(&target, dir.join(format!("{}.wav", sanitize_name(&c.name)))).unwrap(); + let roots = crate::file_access::AutomationRoots::new(&[], &[&dir]).unwrap().unwrap(); + let n = crate::file_access::confine(Some(&roots), || export_clips(&e, &[c.id], &dir.to_string_lossy())).unwrap(); + assert_eq!(n, 1); + assert!(!target.exists(), "the export followed the link out of the root"); + assert!(dir.join(format!("{}-2.wav", sanitize_name(&c.name))).is_file(), "the next free name was used"); + let _ = std::fs::remove_dir_all(&dir); + let _ = std::fs::remove_dir_all(&elsewhere); + } +} + #[cfg(test)] mod stem_tests { #[test]