diff --git a/README.md b/README.md index 964c936..d88fee8 100644 --- a/README.md +++ b/README.md @@ -204,6 +204,9 @@ soundcraft-cli app --port 7801 ui.screenshot '{"path":"shot.png"}' # Give Claude a DAW. claude mcp add soundcraft -- soundcraft-cli mcp --demo + +# Or keep the agent to some folders (reads and writes are separate roots). +soundcraft-cli mcp --automation-read-root DIR --automation-write-root DIR ``` See [`docs/control-protocol.md`](docs/control-protocol.md) and [`docs/mcp.md`](docs/mcp.md). diff --git a/apps/soundcraft-cli/src/automation_args.rs b/apps/soundcraft-cli/src/automation_args.rs new file mode 100644 index 0000000..f874226 --- /dev/null +++ b/apps/soundcraft-cli/src/automation_args.rs @@ -0,0 +1,90 @@ +//! Parse `--automation-read-root` / `--automation-write-root` before the server starts. A malformed +//! flag must not silently remove a root and leave the server unconfined. + +use std::path::PathBuf; + +pub struct AutomationArgs { + pub read_root: Option, + pub write_root: Option, + read_given: bool, + write_given: bool, +} + +impl AutomationArgs { + pub fn new(read_root: Option, write_root: Option) -> Self { + Self { read_root, write_root, read_given: false, write_given: false } + } + + /// Returns whether this argument is a root flag. A second occurrence of a flag is an error, + /// including when the two spellings (`--flag DIR`, `--flag=DIR`) are mixed. + pub fn consume(&mut self, arg: &str, args: &mut impl Iterator) -> Result { + let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value))); + let (root, given) = match flag { + "--automation-read-root" => (&mut self.read_root, &mut self.read_given), + "--automation-write-root" => (&mut self.write_root, &mut self.write_given), + _ => return Ok(false), + }; + if *given { + return Err(format!("{flag} given twice")); + } + let value = match inline { + Some(value) => value.to_owned(), + None => { + let value = args.next().ok_or_else(|| format!("{flag} needs a folder"))?; + if value.starts_with('-') { + return Err(format!("{flag} needs a folder; use {flag}= for a folder starting with '-'")); + } + value + } + }; + if value.is_empty() { + return Err(format!("{flag} needs a non-empty folder")); + } + *root = Some(value.into()); + *given = true; + Ok(true) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn roots_are_taken_once_and_other_arguments_are_left() { + let mut roots = AutomationArgs::new(None, None); + let mut args = ["a folder".to_owned(), "song.scraft".to_owned()].into_iter(); + assert!(roots.consume("--automation-read-root", &mut args).unwrap()); + assert!(roots.consume("--automation-write-root=out", &mut args).unwrap()); + assert!(!roots.consume("--demo", &mut args).unwrap()); + assert_eq!(roots.read_root, Some("a folder".into())); + assert_eq!(roots.write_root, Some("out".into())); + assert_eq!(args.next().as_deref(), Some("song.scraft")); + assert!(roots.consume("--automation-read-root=other", &mut args).is_err()); + assert_eq!(roots.read_root, Some("a folder".into())); + } + + #[test] + fn invalid_operands_leave_roots_unchanged() { + for flag in ["--automation-read-root", "--automation-write-root"] { + for operand in [None, Some(""), Some("--version"), Some("--connect"), Some("-folder")] { + let mut roots = AutomationArgs::new(Some("read".into()), Some("write".into())); + let mut args = operand.map(str::to_owned).into_iter(); + assert!(roots.consume(flag, &mut args).is_err(), "{flag} {operand:?}"); + assert_eq!(roots.read_root, Some("read".into())); + assert_eq!(roots.write_root, Some("write".into())); + } + let mut roots = AutomationArgs::new(None, None); + assert!(roots.consume(&format!("{flag}="), &mut std::iter::empty()).is_err()); + } + } + + #[test] + fn explicit_dash_folder_and_relative_dash_folder_are_valid() { + let mut roots = AutomationArgs::new(None, None); + assert!(roots.consume("--automation-read-root=-folder", &mut std::iter::empty()).unwrap()); + assert!(roots.consume("--automation-write-root", &mut ["./-folder".to_owned()].into_iter()).unwrap()); + assert_eq!(roots.read_root, Some("-folder".into())); + assert_eq!(roots.write_root, Some("./-folder".into())); + } +} diff --git a/apps/soundcraft-cli/src/main.rs b/apps/soundcraft-cli/src/main.rs index 028b06c..9a51cc5 100644 --- a/apps/soundcraft-cli/src/main.rs +++ b/apps/soundcraft-cli/src/main.rs @@ -8,15 +8,23 @@ //! soundcraft-cli commands [FILTER] list commands //! soundcraft-cli describe ID one command's details //! soundcraft-cli app [--port P] METHOD [JSON] call a running app's control channel -//! soundcraft-cli mcp [--connect PORT] [--demo] MCP server on stdio +//! soundcraft-cli mcp [--connect PORT] [--demo] [--automation-read-root DIR] [--automation-write-root DIR] +//! MCP server on stdio. The two roots confine every file the +//! agent's commands read (open, import, presets) and write +//! (save, bounce, export) to those folders, links followed; +//! a root left out grants none of its access. They run the +//! in-process server, so they conflict with --connect. //! soundcraft-cli parity [--write PATH] parity report (markdown) //! soundcraft-cli plugins list built-in plugins //! ``` #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +mod automation_args; + use serde_json::{Value, json}; use soundcraft_automation::{Backend, Headless, Remote, mcp::Server}; use soundcraft_engine::Engine; +use soundcraft_engine::file_access::{self, AutomationRoots}; use std::io::Write; use std::process::ExitCode; @@ -378,15 +386,62 @@ fn app(args: &[String]) -> ExitCode { } } +/// What `mcp` was asked for. +struct McpArgs { + connect: Option, + demo: bool, + roots: Option>, +} + +/// Parse the `mcp` options up front: a malformed or misspelled root flag must not leave the +/// server running unconfined. +fn parse_mcp_args(args: &[String]) -> Result { + let mut connect = None; + let mut demo = false; + let mut roots_args = automation_args::AutomationArgs::new(None, None); + let mut it = args.iter().cloned(); + while let Some(a) = it.next() { + if roots_args.consume(&a, &mut it)? { + continue; + } + match a.as_str() { + "--connect" => connect = Some(it.next().ok_or("--connect needs a port or host:port")?), + "--demo" => demo = true, + other if other.starts_with("--automation-") => return Err(format!("unknown mcp option `{other}`")), + _ => {} + } + } + let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?; + if roots.is_some() && connect.is_some() { + return Err("--automation-read-root and --automation-write-root confine the in-process server; a running app is not confined by them, \ +so drop --connect (or use --connect without the roots)" + .into()); + } + Ok(McpArgs { connect, demo, roots }) +} + fn mcp(args: &[String]) -> ExitCode { + let McpArgs { connect, demo, roots } = match parse_mcp_args(args) { + Ok(a) => a, + Err(err) => return fail(err), + }; let stdin = std::io::stdin(); let stdout = std::io::stdout(); - let r = if let Some(port) = arg_value(args, "--connect") { + let r = if let Some(port) = connect { let mut s = Server::new(Remote::new(&port)); s.serve(stdin.lock(), stdout.lock()) } else { - let e = if args.iter().any(|a| a == "--demo") { soundcraft_engine::demo::demo_engine() } else { Engine::default() }; - let mut s = Server::new(Headless::new(e)); + // `--demo` is the operator's own choice, made before the roots come into force. + let e = if demo { soundcraft_engine::demo::demo_engine() } else { Engine::default() }; + let mut h = Headless::new(e); + if let Some(roots) = roots { + // Every thread of this process, not only the server's: nothing reaches past the roots. + file_access::confine_process(roots.clone()); + let show = |root: Option<&std::path::Path>| root.map_or_else(|| "none".to_string(), |r| r.display().to_string()); + eprintln!("soundcraft-cli: files confined: read root {}, write root {}", show(roots.read_root()), show(roots.write_root())); + h = h.with_automation_roots(Some(roots)); + } + let mut s = Server::new(h); s.serve(stdin.lock(), stdout.lock()) }; match r { @@ -480,6 +535,7 @@ mod tests { let help = help_text(); assert!(help.starts_with("soundcraft-cli info FILE")); assert!(help.ends_with("soundcraft-cli plugins list built-in plugins")); + assert!(help.contains("--automation-read-root DIR") && help.contains("--automation-write-root DIR")); assert!(!help.contains("```")); assert!(!help.contains("#![") && !help.contains("use serde_json")); } diff --git a/apps/soundcraft-cli/tests/cli.rs b/apps/soundcraft-cli/tests/cli.rs new file mode 100644 index 0000000..ef2fb88 --- /dev/null +++ b/apps/soundcraft-cli/tests/cli.rs @@ -0,0 +1,140 @@ +//! `soundcraft-cli mcp` with the automation root flags, run as the real binary. +// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::io::Write; +use std::path::PathBuf; +use std::process::{Command, Output, Stdio}; + +use serde_json::Value; + +fn bin() -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_soundcraft-cli")); + cmd.env("SOUNDCRAFT_NO_PREFS", "1"); + cmd +} + +fn run(args: &[&str]) -> Output { + bin().args(args).output().unwrap() +} + +fn tmpdir(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("sc-cli-roots-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +/// Send `calls` (tool name, arguments) to `soundcraft-cli mcp `; returns whether it exited +/// cleanly, its stderr, and the stdout lines after `initialize`, one per call. +fn mcp_session(args: &[&str], calls: &[(&str, Value)]) -> (bool, String, Vec) { + let mut child = bin().arg("mcp").args(args).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + { + let stdin = child.stdin.as_mut().unwrap(); + writeln!(stdin, r#"{{"jsonrpc":"2.0","id":1,"method":"initialize","params":{{"protocolVersion":"2025-06-18"}}}}"#).unwrap(); + for (i, (name, arguments)) in calls.iter().enumerate() { + let call = serde_json::json!({"jsonrpc": "2.0", "id": i + 2, "method": "tools/call", "params": {"name": name, "arguments": arguments}}); + writeln!(stdin, "{call}").unwrap(); + } + } + let out = child.wait_with_output().unwrap(); + let stdout = String::from_utf8(out.stdout).unwrap(); + let replies = stdout.lines().skip(1).map(|l| serde_json::from_str(l).unwrap()).collect(); + (out.status.success(), String::from_utf8_lossy(&out.stderr).into_owned(), replies) +} + +fn text(reply: &Value) -> &str { + reply["result"]["content"][0]["text"].as_str().unwrap() +} + +/// A short WAV made by the binary itself. +fn make_wav(path: &std::path::Path) { + let o = run(&["run", "--demo", "--bounce", path.to_str().unwrap(), "--start", "0", "--end", "0.1"]); + assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr)); +} + +#[test] +fn mcp_automation_roots_confine_the_server() { + let base = tmpdir("roots"); + let (work, elsewhere) = (base.join("work"), base.join("elsewhere")); + std::fs::create_dir_all(&work).unwrap(); + std::fs::create_dir_all(&elsewhere).unwrap(); + make_wav(&work.join("in.wav")); + make_wav(&elsewhere.join("secret.wav")); + let work_s = work.to_str().unwrap(); + let secret = elsewhere.join("secret.wav").to_str().unwrap().to_string(); + + // Both spellings of the flags; inside works, outside is refused with a readable message. + let read_flag = format!("--automation-read-root={work_s}"); + let calls = [ + ("import_audio", serde_json::json!({"path": work.join("in.wav").to_str().unwrap()})), + ("import_audio", serde_json::json!({"path": secret})), + ("save_session", serde_json::json!({"path": work.join("saved.scraft").to_str().unwrap()})), + ("save_session", serde_json::json!({"path": elsewhere.join("saved.scraft").to_str().unwrap()})), + ("bounce_mix", serde_json::json!({"path": elsewhere.join("mix.wav").to_str().unwrap()})), + ]; + let (success, stderr, replies) = mcp_session(&[&read_flag, "--automation-write-root", work_s], &calls); + assert!(success, "{stderr}"); + assert!(stderr.contains("files confined"), "{stderr}"); + assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]); + assert_eq!(replies[1]["result"]["isError"], true); + assert!(text(&replies[1]).contains("outside the read root"), "{}", text(&replies[1])); + assert_eq!(replies[2]["result"]["isError"], false, "{}", replies[2]); + assert!(work.join("saved.scraft").exists()); + assert_eq!(replies[3]["result"]["isError"], true); + assert!(text(&replies[3]).contains("outside the write root"), "{}", text(&replies[3])); + assert_eq!(replies[4]["result"]["isError"], true); + assert!(text(&replies[4]).contains("outside the write root"), "{}", text(&replies[4])); + assert!(!elsewhere.join("saved.scraft").exists() && !elsewhere.join("mix.wav").exists()); + + // A root left out grants none of its access. + let (_, _, replies) = mcp_session(&[&read_flag], &calls[2..3]); + assert_eq!(replies[0]["result"]["isError"], true); + assert!(text(&replies[0]).contains("write authority is absent"), "{}", text(&replies[0])); + let (_, _, replies) = mcp_session(&["--automation-write-root", work_s], &calls[0..1]); + assert_eq!(replies[0]["result"]["isError"], true); + assert!(text(&replies[0]).contains("read authority is absent"), "{}", text(&replies[0])); + + // No flags: nothing is confined. + let (success, _, replies) = mcp_session(&[], &calls[1..2]); + assert!(success); + assert_eq!(replies[0]["result"]["isError"], false, "{}", replies[0]); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn mcp_automation_root_flags_are_checked_up_front() { + let base = tmpdir("roots-args"); + let dir = base.to_str().unwrap(); + let missing = base.join("missing"); + let fails = |args: &[&str], needle: &str| { + let mut full = vec!["mcp"]; + full.extend_from_slice(args); + let o = run(&full); + assert!(!o.status.success(), "{args:?}"); + let err = String::from_utf8_lossy(&o.stderr).into_owned(); + assert!(err.contains(needle), "{args:?}: {err}"); + }; + // The flags confine the in-process server, so they cannot be combined with --connect. + fails(&["--connect", "7979", "--automation-read-root", dir], "--connect"); + fails(&["--automation-write-root", dir, "--connect", "7979"], "--connect"); + fails(&["--automation-read-root", dir, "--automation-read-root", dir], "given twice"); + fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice"); + fails(&["--automation-read-root"], "needs a folder"); + fails(&["--automation-write-root", "--demo"], "needs a folder"); + fails(&["--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root"); + fails(&["--automation-write-root="], "needs a non-empty folder"); + // A misspelled root flag must not start an unconfined server. + fails(&["--automation-read-roots", dir], "unknown mcp option"); + // A file is not a folder. + std::fs::write(base.join("f.txt"), "x").unwrap(); + fails(&["--automation-write-root", base.join("f.txt").to_str().unwrap()], "cannot open the automation write root"); + let _ = std::fs::remove_dir_all(base); +} + +#[test] +fn usage_lists_the_root_flags() { + let o = run(&["--help"]); + let help = String::from_utf8_lossy(&o.stdout).into_owned(); + assert!(help.contains("--automation-read-root DIR") && help.contains("--automation-write-root DIR"), "{help}"); +} diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 84d4e23..17c3a5b 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -11,8 +11,10 @@ pub mod mcp; use serde_json::{Value, json}; use soundcraft_engine::Engine; +use soundcraft_engine::file_access::{self, AutomationRoots}; use std::io::{BufRead, BufReader, Write}; use std::net::TcpStream; +use std::sync::Arc; use std::time::Duration; /// Answers control-protocol methods. @@ -26,16 +28,22 @@ pub trait Backend { /// An in-process engine (no UI). pub struct Headless { pub engine: Engine, + roots: Option>, } impl Headless { pub fn new(engine: Engine) -> Self { - Headless { engine } + Headless { engine, roots: None } } -} -impl Backend for Headless { - fn call(&mut self, method: &str, params: Value) -> Result { + /// Confine every file the session's commands read and write to `roots` + /// (`soundcraft-cli mcp --automation-read-root … --automation-write-root …`). + pub fn with_automation_roots(mut self, roots: Option>) -> Self { + self.roots = roots; + self + } + + fn answer(&mut self, method: &str, params: Value) -> Result { match method { "engine.execute" | "command" => { let id = params.get("command").or_else(|| params.get("id")).and_then(Value::as_str).ok_or("`command` required")?; @@ -56,6 +64,13 @@ impl Backend for Headless { other => Err(format!("unknown method `{other}`")), } } +} + +impl Backend for Headless { + fn call(&mut self, method: &str, params: Value) -> Result { + let roots = self.roots.clone(); + file_access::confine(roots.as_ref(), || self.answer(method, params)) + } fn has_ui(&self) -> bool { false } diff --git a/crates/automation/tests/automation_roots.rs b/crates/automation/tests/automation_roots.rs new file mode 100644 index 0000000..2c4fa16 --- /dev/null +++ b/crates/automation/tests/automation_roots.rs @@ -0,0 +1,352 @@ +//! `Headless::with_automation_roots` through the real MCP protocol: every file a command reads +//! stays inside the read root, every file it writes inside the write root. +// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use serde_json::{Value, json}; +use soundcraft_automation::{Headless, mcp::Server}; +use soundcraft_engine::Engine; +use soundcraft_engine::file_access::AutomationRoots; +use std::path::{Path, PathBuf}; + +/// A marker that must never come back from a file outside the read root. +const SECRET: &str = "TOP-SECRET-MARKER-5f3a"; + +struct Fixture { + base: PathBuf, + read: PathBuf, + write: PathBuf, + outside: PathBuf, +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.base); + } +} + +fn s(p: impl AsRef) -> String { + p.as_ref().to_string_lossy().into_owned() +} + +/// `in/` (read root), `out/` (write root) and `outside/`, each holding the same real files: +/// a session with audio, a WAV, a MIDI file, a track preset and a clip group file. `outside/` +/// also holds text files holding [`SECRET`] under the names a command might open. +fn fixture(name: &str) -> Fixture { + let base = std::env::temp_dir().join(format!("sc-roots-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (read, write, outside) = (base.join("in"), base.join("out"), base.join("outside")); + for d in [&read, &write, &outside] { + std::fs::create_dir_all(d).unwrap(); + } + // Real files, made by an unconfined engine. + let mut e = soundcraft_engine::demo::demo_engine(); + for dir in [&read, &outside] { + let run = |e: &mut Engine, id: &str, p: Value| e.execute(id, &p).unwrap_or_else(|err| panic!("{id}: {err}")); + run(&mut e, "file.bounce_mix", json!({"path": s(dir.join("tone.wav")), "start": 0, "end": 4800})); + run(&mut e, "file.export_midi", json!({"path": s(dir.join("keys.mid")), "tracks": ["Keys"]})); + run(&mut e, "edit.select", json!({"tracks": ["Bass"]})); + run(&mut e, "track.save_preset", json!({"path": s(dir.join("bass.json"))})); + run(&mut e, "session.save_as", json!({"path": s(dir.join("song.scraft")), "replace": true})); + let clips = e.session().track_by_name("Kick").unwrap().clips().iter().map(|c| c.id.0).collect::>(); + run(&mut e, "file.export_clip_groups", json!({"path": s(dir.join("group.scgrp")), "clips": clips})); + } + for f in ["secret.wav", "secret.mid", "secret.scraft", "secret.json", "secret.scgrp", "secret.mp4"] { + std::fs::write(outside.join(f), SECRET).unwrap(); + } + Fixture { base, read, write, outside } +} + +fn server(read: Option<&Path>, write: Option<&Path>) -> Server { + let roots = AutomationRoots::new(read, write).unwrap(); + Server::new(Headless::new(Engine::default()).with_automation_roots(roots)) +} + +/// One `execute` call: whether it failed and the text of the result. +fn run(s: &mut Server, command: &str, params: Value) -> (bool, String) { + let r = s + .handle(&json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "execute", "arguments": {"command": command, "params": params}}})) + .unwrap(); + let res = &r["result"]; + (res["isError"].as_bool().unwrap(), res["content"][0]["text"].as_str().unwrap().to_string()) +} + +fn ok(s: &mut Server, command: &str, params: Value) -> String { + let (failed, text) = run(s, command, params); + assert!(!failed, "{command} failed: {text}"); + text +} + +fn refused(s: &mut Server, command: &str, params: Value, needle: &str) { + let (failed, text) = run(s, command, params.clone()); + assert!(failed, "{command} {params} must be refused, got {text}"); + assert!(text.contains(needle), "{command}: {text}"); + assert!(!text.contains(SECRET), "{command} leaked file contents: {text}"); +} + +/// Every file under `dir`, to show nothing was written. +fn listing(dir: &Path) -> Vec { + let mut out = vec![]; + for entry in std::fs::read_dir(dir).unwrap().flatten() { + let p = entry.path(); + if p.is_dir() { + out.extend(listing(&p)); + } + out.push(s(&p)); + } + out.sort(); + out +} + +fn inspect(s: &mut Server) -> Value { + let r = s + .handle(&json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "inspect_session", "arguments": {"detail": "full"}}})) + .unwrap(); + serde_json::from_str(r["result"]["content"][0]["text"].as_str().unwrap()).unwrap() +} + +/// Every command that reads a user-named file, with params for a file in `dir`. +fn reads(dir: &Path, name: &dyn Fn(&str) -> String) -> Vec<(&'static str, Value)> { + let p = |f: &str| s(dir.join(name(f))); + vec![ + ("session.open", json!({"path": p("song.scraft")})), + ("file.import_audio", json!({"path": p("tone.wav")})), + ("file.import_audio", json!({"paths": [p("tone.wav")]})), + ("file.import_midi", json!({"path": p("keys.mid")})), + ("file.import_video", json!({"path": p("movie.mp4")})), + ("file.import_clip_groups", json!({"path": p("group.scgrp")})), + ("file.import_session_data", json!({"path": p("song.scraft")})), + ("track.load_preset", json!({"path": p("bass.json"), "tracks": ["Bass"]})), + ] +} + +#[test] +fn reads_outside_the_read_root_are_refused_and_nothing_reaches_the_result() { + let f = fixture("reads"); + let mut srv = server(Some(&f.read), Some(&f.write)); + ok(&mut srv, "session.new", json!({"name": "Work"})); + ok(&mut srv, "track.new", json!({"name": "Bass", "count": 1})); + ok(&mut srv, "track.new", json!({"name": "Pads", "count": 1, "kind": "instrument"})); + let before = inspect(&mut srv); + // The secret files hold the marker under the names the commands read, so a read would show. + let secret_name = |f: &str| format!("secret.{}", f.rsplit('.').next().unwrap()); + for (command, params) in reads(&f.outside, &secret_name) { + refused(&mut srv, command, params, "outside the read root"); + } + refused(&mut srv, "mix.sampler_load", json!({"track": "Pads", "path": s(f.outside.join("secret.wav"))}), "outside the read root"); + // The open session is exactly as it was: nothing was imported or opened. + assert_eq!(inspect(&mut srv), before); + // The same commands work on files inside the root. + ok(&mut srv, "file.import_audio", json!({"path": s(f.read.join("tone.wav"))})); + ok(&mut srv, "file.import_midi", json!({"path": s(f.read.join("keys.mid"))})); + ok(&mut srv, "file.import_clip_groups", json!({"path": s(f.read.join("group.scgrp"))})); + ok(&mut srv, "file.import_session_data", json!({"path": s(f.read.join("song.scraft")), "tracks": ["Kick"]})); + ok(&mut srv, "track.load_preset", json!({"path": s(f.read.join("bass.json")), "tracks": ["Bass"]})); + ok(&mut srv, "mix.sampler_load", json!({"track": "Pads", "path": s(f.read.join("tone.wav"))})); + // A missing movie inside the root is an ordinary error, not a refusal. + let (failed, text) = run(&mut srv, "file.import_video", json!({"path": s(f.read.join("movie.mp4"))})); + assert!(failed && !text.contains("read root"), "{text}"); + ok(&mut srv, "session.open", json!({"path": s(f.read.join("song.scraft"))})); + // The typed tools take the same path through the same checks. + let r = srv + .handle(&json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "open_session", "arguments": {"path": s(f.outside.join("secret.scraft"))}}})) + .unwrap(); + assert_eq!(r["result"]["isError"], true); + assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("outside the read root")); +} + +/// A clip of the demo session. +fn kick_clip() -> u64 { + soundcraft_engine::demo::demo_engine().session().track_by_name("Kick").unwrap().clips()[0].id.0 +} + +/// Every command that writes a user-named file or folder, with params for `dir`. +fn writes(dir: &Path) -> Vec<(&'static str, Value)> { + let p = |f: &str| s(dir.join(f)); + let clip = kick_clip(); + vec![ + ("session.save", json!({"path": p("a.scraft")})), + ("session.save_as", json!({"path": p("b.scraft")})), + ("session.save_copy", json!({"path": p("c.scraft")})), + ("session.save_template", json!({"path": p("d.scraft")})), + ("file.bounce_mix", json!({"path": p("e.wav"), "start": 0, "end": 4800})), + ("file.bounce_stems", json!({"dir": p("stems"), "start": 0, "end": 4800})), + ("file.export_midi", json!({"path": p("f.mid")})), + ("file.export_session_text", json!({"path": p("g.txt")})), + ("file.export_clips", json!({"dir": p("clips"), "clips": [clip]})), + ("file.export_selected_tracks_as_session", json!({"path": p("h.scraft"), "tracks": ["Kick"]})), + ("file.export_sibelius", json!({"path": p("i.musicxml")})), + ("file.send_to_sibelius", json!({"path": p("j.musicxml")})), + ("file.print_score", json!({"path": p("k.svg")})), + ("file.export_clip_groups", json!({"path": p("l.scgrp"), "clips": [clip]})), + ("track.save_preset", json!({"path": p("m.json"), "tracks": ["Kick"]})), + ] +} + +fn demo_server(read: Option<&Path>, write: Option<&Path>) -> Server { + let roots = AutomationRoots::new(read, write).unwrap(); + Server::new(Headless::new(soundcraft_engine::demo::demo_engine()).with_automation_roots(roots)) +} + +#[test] +fn every_writing_command_is_refused_outside_the_write_root_and_writes_nothing() { + let f = fixture("writes"); + let mut srv = demo_server(Some(&f.read), Some(&f.write)); + let before = listing(&f.base); + let outside_before = listing(&f.outside); + for (command, params) in writes(&f.outside) { + refused(&mut srv, command, params, "outside the write root"); + } + // Not even the temporary file a save goes through, nor a folder. + assert_eq!(listing(&f.base), before, "nothing may appear anywhere"); + // With no path, the score goes to the temp folder: outside the write root, so refused too. + refused(&mut srv, "file.send_to_sibelius", json!({}), "outside the write root"); + refused(&mut srv, "file.print_score", json!({}), "outside the write root"); + assert_eq!(listing(&f.base), before); + // Inside the write root every one of them works. + for (command, params) in writes(&f.write) { + let text = ok(&mut srv, command, params); + assert!(!text.is_empty(), "{command}"); + } + assert!(f.write.join("e.wav").is_file() && f.write.join("stems").is_dir() && f.write.join("l.scgrp").is_file()); + // A save puts the audio in `Audio Files/` beside it, inside the root as well. + assert!(f.write.join("Audio Files").is_dir()); + assert_eq!(listing(&f.outside), outside_before, "outside is as it was"); +} + +#[test] +fn saving_over_an_existing_file_outside_does_not_report_that_it_exists() { + let f = fixture("exists"); + let mut srv = demo_server(Some(&f.read), Some(&f.write)); + // Without `replace`, an existing target is reported as existing: not for a path outside the root. + refused(&mut srv, "session.save_as", json!({"path": s(f.outside.join("song.scraft"))}), "outside the write root"); +} + +#[test] +fn the_typed_bounce_and_save_tools_are_confined() { + let f = fixture("typed"); + let mut srv = demo_server(Some(&f.read), Some(&f.write)); + for (tool, args) in [ + ("bounce_mix", json!({"path": s(f.outside.join("x.wav")), "start": 0, "end": 4800})), + ("save_session", json!({"path": s(f.outside.join("x.scraft"))})), + ] { + let r = srv.handle(&json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": tool, "arguments": args}})).unwrap(); + assert_eq!(r["result"]["isError"], true, "{tool}"); + assert!(r["result"]["content"][0]["text"].as_str().unwrap().contains("outside the write root"), "{tool}"); + } + assert!(!f.outside.join("x.wav").exists() && !f.outside.join("x.scraft").exists()); + let r = srv + .handle(&json!({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "bounce_mix", "arguments": {"path": s(f.write.join("x.wav")), "start": 0, "end": 4800}}})) + .unwrap(); + assert_eq!(r["result"]["isError"], false); +} + +#[test] +fn one_root_grants_only_its_own_access() { + let f = fixture("one-root"); + // Read root only: reads work, nothing can be written anywhere. + let mut srv = demo_server(Some(&f.read), None); + ok(&mut srv, "file.import_audio", json!({"path": s(f.read.join("tone.wav"))})); + let before = listing(&f.base); + for (command, params) in writes(&f.write) { + refused(&mut srv, command, params, "write authority is absent"); + } + for (command, params) in writes(&f.read) { + refused(&mut srv, command, params, "write authority is absent"); + } + assert_eq!(listing(&f.base), before); + // Write root only: writes work, nothing can be read anywhere. + let mut srv = demo_server(None, Some(&f.write)); + for (command, params) in writes(&f.write) { + ok(&mut srv, command, params); + } + let name = |f: &str| f.to_string(); + for (command, params) in reads(&f.read, &name) { + refused(&mut srv, command, params, "read authority is absent"); + } + refused(&mut srv, "file.import_audio", json!({"path": s(f.write.join("e.wav"))}), "read authority is absent"); +} + +#[test] +fn the_roots_may_be_the_same_folder_and_are_compared_by_whole_names() { + let f = fixture("same"); + let mut srv = demo_server(Some(&f.read), Some(&f.read)); + ok(&mut srv, "file.import_audio", json!({"path": s(f.read.join("tone.wav"))})); + ok(&mut srv, "file.bounce_mix", json!({"path": s(f.read.join("again.wav")), "start": 0, "end": 4800})); + // `in2` shares the prefix of `in` and is a different folder. + let sibling = f.base.join("in2"); + std::fs::create_dir_all(&sibling).unwrap(); + std::fs::copy(f.read.join("tone.wav"), sibling.join("tone.wav")).unwrap(); + refused(&mut srv, "file.import_audio", json!({"path": s(sibling.join("tone.wav"))}), "outside the read root"); + refused(&mut srv, "file.bounce_mix", json!({"path": s(sibling.join("x.wav"))}), "outside the write root"); + // `..` out of the root is outside it. + let up = format!("{}/../outside/tone.wav", s(&f.read)); + refused(&mut srv, "file.import_audio", json!({"path": up}), "outside the read root"); +} + +#[cfg(unix)] +#[test] +fn links_out_of_the_roots_are_refused() { + use std::os::unix::fs::symlink; + let f = fixture("links"); + symlink(f.outside.join("tone.wav"), f.read.join("leak.wav")).unwrap(); + symlink(&f.outside, f.write.join("away")).unwrap(); + symlink(f.outside.join("not-yet.wav"), f.write.join("dangling.wav")).unwrap(); + let mut srv = demo_server(Some(&f.read), Some(&f.write)); + refused(&mut srv, "file.import_audio", json!({"path": s(f.read.join("leak.wav"))}), "outside the read root"); + let before = listing(&f.outside); + refused(&mut srv, "file.bounce_mix", json!({"path": s(f.write.join("away/x.wav"))}), "outside the write root"); + refused(&mut srv, "file.bounce_stems", json!({"dir": s(f.write.join("away/stems"))}), "outside the write root"); + refused(&mut srv, "session.save_as", json!({"path": s(f.write.join("away/x.scraft"))}), "outside the write root"); + refused(&mut srv, "file.bounce_mix", json!({"path": s(f.write.join("dangling.wav"))}), "a link that leads nowhere"); + assert_eq!(listing(&f.outside), before, "nothing was written through a link"); + assert!(!f.outside.join("not-yet.wav").exists()); + // A link to a folder inside the root stays inside it. + symlink(f.read.join("."), f.base.join("in-alias")).unwrap(); + let mut via_alias = demo_server(Some(&f.base.join("in-alias")), None); + ok(&mut via_alias, "file.import_audio", json!({"path": s(f.read.join("tone.wav"))})); +} + +/// A session file is hostile input too: media it names outside the read root is not loaded, +/// and the refusal is listed with the other missing media. +#[test] +fn media_a_session_names_outside_the_read_root_is_not_loaded() { + let f = fixture("media"); + let session = f.read.join("song.scraft"); + let text = std::fs::read_to_string(&session).unwrap(); + let outside_wav = s(f.outside.join("tone.wav")); + let swapped = { + let v: Value = serde_json::from_str(&text).unwrap(); + let mut rel = vec![]; + for src in v["sources"].as_array().unwrap() { + rel.push(src["path"].as_str().unwrap().to_string()); + } + assert!(!rel.is_empty()); + rel.iter().fold(text.clone(), |t, r| t.replace(&format!("\"{r}\""), &format!("{}", json!(outside_wav)))) + }; + std::fs::write(&session, swapped).unwrap(); + let mut srv = server(Some(&f.read), None); + let opened = ok(&mut srv, "session.open", json!({"path": s(&session)})); + let opened: Value = serde_json::from_str(&opened).unwrap(); + let missing = opened["missing"].as_array().unwrap(); + assert!(!missing.is_empty() && missing.iter().all(|m| m.as_str().unwrap().contains("outside the read root")), "{opened}"); + let sources = inspect(&mut srv)["sources"].clone(); + assert!(sources.as_array().unwrap().iter().all(|x| x["loaded"] == false), "{sources}"); + // The same session opened without roots does load the file it names. + let mut free = server(None, None); + ok(&mut free, "session.open", json!({"path": s(&session)})); + assert!(inspect(&mut free)["sources"].as_array().unwrap().iter().all(|x| x["loaded"] == true)); +} + +#[test] +fn without_roots_nothing_is_confined() { + let f = fixture("free"); + let mut srv = server(None, None); + ok(&mut srv, "file.import_audio", json!({"path": s(f.outside.join("tone.wav"))})); + ok(&mut srv, "file.bounce_mix", json!({"path": s(f.outside.join("free.wav")), "start": 0, "end": 4800})); + assert!(f.outside.join("free.wav").is_file()); +} diff --git a/crates/engine/src/clip_group_file.rs b/crates/engine/src/clip_group_file.rs index c889c99..607d5b5 100644 --- a/crates/engine/src/clip_group_file.rs +++ b/crates/engine/src/clip_group_file.rs @@ -31,6 +31,9 @@ struct GroupFile { /// Write the clips `ids` (grouped by track) to `path`. Returns the number of clips written. pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { + let p = with_extension(path); + // First, so a refused group file leaves nothing behind and is refused whatever else is wrong. + crate::file_access::check_write(&p).map_err(EngineError::Io)?; let s = e.session(); let mut lanes: Vec<(usize, Vec)> = Vec::new(); for id in ids { @@ -47,7 +50,6 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { }; let n = clips.len(); lanes.sort_by_key(|(i, _)| *i); - let p = with_extension(path); let stem = p.file_stem().and_then(|x| x.to_str()).unwrap_or("Clip Group").to_string(); let audio_dir_name = format!("{} Audio", crate::io::sanitize_name(&stem)); let dir = p.parent().map(Path::to_path_buf).unwrap_or_default(); @@ -62,10 +64,10 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { && let (Some(buf), Some(src)) = (s.pool.get(source), s.sources.iter().find(|x| x.id == source)) { let rel = format!("{audio_dir_name}/{}_{}.wav", crate::io::sanitize_name(&src.name), source.0); - std::fs::create_dir_all(dir.join(&audio_dir_name)).map_err(|err| EngineError::Io(format!("{audio_dir_name}: {err}")))?; + crate::io::make_dir(dir.join(&audio_dir_name))?; let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: soundcraft_audio_io::BitDepth::Float32, dither: false, bwf: None }; let bytes = soundcraft_audio_io::encode(&buf.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; - std::fs::write(dir.join(&rel), bytes).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?; + crate::io::write_path(dir.join(&rel), bytes)?; slot.insert(rel); } lane.push(c); @@ -74,7 +76,7 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { } let file = GroupFile { format: FORMAT.into(), version: 1, sample_rate: s.sample_rate.hz(), length: end - start, lanes: out_lanes, audio }; let text = serde_json::to_string_pretty(&file).map_err(|err| EngineError::Io(err.to_string()))?; - std::fs::write(&p, text).map_err(|err| EngineError::Io(format!("{}: {err}", p.display())))?; + crate::io::write_path(&p, text)?; Ok(n) } @@ -82,7 +84,7 @@ pub fn export(e: &Engine, ids: &[ClipId], path: &str) -> Result { /// are made when the session runs out). Returns the ids of the placed clips. pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) -> Result> { let id = "file.import_clip_groups"; - let text = std::fs::read_to_string(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = crate::io::read_text(path)?; let file: GroupFile = serde_json::from_str(&text).map_err(|err| EngineError::BadParams(id.into(), format!("not a clip group file: {err}")))?; if file.format != FORMAT { return Err(EngineError::BadParams(id.into(), "not a SoundCraft clip group file".into())); @@ -98,7 +100,7 @@ pub fn import(e: &mut Engine, path: &str, first: Option, at: Samples) - if !full.starts_with(&dir) || rel.contains("..") { return Err(EngineError::BadParams(id.into(), format!("audio path escapes the group folder: {rel}"))); } - let bytes = std::fs::read(&full).map_err(|err| EngineError::Io(format!("{}: {err}", full.display())))?; + let bytes = crate::io::read_file(&full)?; let (_, buf) = soundcraft_audio_io::decode(&bytes, Some("wav")).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?; decoded.insert(*old, (rel.clone(), buf)); } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 1a90f51..2acf60b 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -66,7 +66,7 @@ pub fn specs() -> Vec { cmd!("file.import_midi", "MIDI...", ["File", "Import"], Some("Cmd+Alt+I"), "{path, at?}", always, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.import_midi", "`path` required"))?.to_string(); let at = position_param(e, "file.import_midi", p, "at")?.unwrap_or(0); - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; let tracks = crate::io::import_midi_bytes(e, &bytes, at, bool_or(p, "tempo_map", true))?; Ok(json!({"tracks": tracks})) }), @@ -80,7 +80,7 @@ pub fn specs() -> Vec { |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.import_video", "`path` required"))?.to_string(); let at = position_param(e, "file.import_video", p, "at")?.unwrap_or(0).max(0); - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; crate::io::import_video_bytes(e, &path, bytes, at) } ), @@ -198,6 +198,8 @@ pub fn specs() -> Vec { /// Writing a session over an existing file needs `replace: true` (see `io::replaces`). fn refuse_replacing(e: &Engine, id: &str, file: &std::path::Path, p: &Value) -> Result<()> { + // First, so a path outside the write root is refused as such and never reported as existing. + crate::file_access::check_write(file).map_err(EngineError::Io)?; if crate::io::replaces(e, id, file) && !bool_or(p, "replace", false) { return Err(bad(id, format!("{} already exists: pass `replace: true` to replace it", file.display()))); } @@ -235,7 +237,7 @@ fn import_audio(e: &mut Engine, p: &Value) -> Result { let target = import_audio_target(e, p)?; let mut out = Vec::new(); for path in &paths { - let bytes = std::fs::read(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(path)?; let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or("Audio").to_string(); let r = crate::io::import_audio_bytes(e, &name, &bytes, Some(path.as_str()), target, at)?; out.push(r); diff --git a/crates/engine/src/cmd/sampler.rs b/crates/engine/src/cmd/sampler.rs index ea659a8..7bac7e8 100644 --- a/crates/engine/src/cmd/sampler.rs +++ b/crates/engine/src/cmd/sampler.rs @@ -55,7 +55,7 @@ fn load(e: &mut Engine, p: &Value) -> Result { values.push((k.clone(), pi.clamp(x as f32))); } } - let bytes = std::fs::read(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let bytes = crate::io::read_file(&path)?; let name = Path::new(&path).file_name().and_then(|n| n.to_str()).unwrap_or("Sample").to_string(); let ext = Path::new(&name).extension().and_then(|x| x.to_str()).map(str::to_ascii_lowercase); let (format, buf) = soundcraft_audio_io::decode(&bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?; diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index cc8ea33..8dd1640 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -406,7 +406,7 @@ fn midi_inputs(e: &mut Engine, p: &Value) -> Result { /// Decode a source file of another session (paths relative to that session's folder). fn load_source(dir: &Path, src: &Source, sr: u32) -> Option { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - let bytes = std::fs::read(&p).ok()?; + let bytes = crate::io::read_file(&p).ok()?; let (_, mut buf) = soundcraft_audio_io::decode(&bytes, p.extension().and_then(|x| x.to_str())).ok()?; if buf.sample_rate != sr && buf.sample_rate > 0 { buf.channels = soundcraft_dsp::offline::resample(&buf.channels, buf.sample_rate, sr); @@ -418,7 +418,7 @@ fn load_source(dir: &Path, src: &Source, sr: u32) -> Option Result { let id = "file.import_session_data"; let path = str_param(p, "path").filter(|x| !x.trim().is_empty()).ok_or_else(|| bad(id, "`path` required"))?.to_string(); - let text_v = std::fs::read_to_string(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text_v = crate::io::read_text(&path)?; let other = Session::from_json(&text_v).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; let dir = Path::new(&path).parent().map(Path::to_path_buf).unwrap_or_default(); let names: Option> = diff --git a/crates/engine/src/cmd/track_more.rs b/crates/engine/src/cmd/track_more.rs index 3793108..290a8d5 100644 --- a/crates/engine/src/cmd/track_more.rs +++ b/crates/engine/src/cmd/track_more.rs @@ -271,7 +271,7 @@ fn load_preset(e: &mut Engine, p: &Value) -> Result { let id = "track.load_preset"; let path = str_param(p, "path").ok_or_else(|| bad(id, "`path` required"))?.to_string(); let tracks = tracks_required(e, id, p)?; - let text = std::fs::read_to_string(&path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = crate::io::read_text(&path)?; let v: Value = serde_json::from_str(&text).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; if v.get("format").and_then(Value::as_str) != Some(PRESET_FORMAT) { return Err(EngineError::Io(format!("{path}: not a SoundCraft track preset"))); diff --git a/crates/engine/src/cmd/video.rs b/crates/engine/src/cmd/video.rs index 5277ca5..f361e0d 100644 --- a/crates/engine/src/cmd/video.rs +++ b/crates/engine/src/cmd/video.rs @@ -36,7 +36,7 @@ pub fn specs() -> Vec { .map(|v| { let clips = s.tracks.iter().flat_map(|t| t.clips()).filter(|c| c.video_source() == Some(v.id)).count(); json!({"id": v.id, "name": v.name, "path": v.path, "width": v.width, "height": v.height, "frame_rate": v.frame_rate, - "duration": v.duration, "codec": v.codec, "clips": clips, "file_found": std::path::Path::new(&v.path).is_file()}) + "duration": v.duration, "codec": v.codec, "clips": clips, "file_found": crate::io::file_found(&v.path)}) }) .collect(); Ok(json!({"videos": rows, "online": !s.edit.flag(OFFLINE_FLAG)})) diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs new file mode 100644 index 0000000..be4ea9d --- /dev/null +++ b/crates/engine/src/file_access.rs @@ -0,0 +1,487 @@ +//! The folders automation may read and write. +//! +//! `soundcraft-cli mcp --automation-read-root --automation-write-root ` confines the +//! files an agent's commands reach. [`AutomationRoots`] holds the two folders. While they are in +//! force, every path a command reads goes through [`check_read`] and every path it writes through [`check_write`]: +//! +//! - The path is made absolute against the working directory, as the file system reads it. Its +//! links are followed: for a file that doesn't exist yet, the links of its nearest existing +//! folder, the rest of the path being plain names (`..` below a missing folder is refused, and so +//! is a link that leads nowhere). +//! - The result must lie inside the root, compared by whole components (`/work2` isn't inside +//! `/work`), without regard to case on Windows. +//! - Read and write access are separate: a root given alone grants only its own kind of access. +//! +//! Roots are in force on a thread inside [`confine`], and in the whole process after +//! [`confine_process`] (`soundcraft-cli mcp`), except inside [`unconfined`]. Without roots +//! nothing is checked. +//! +//! A path is checked, then opened: a process that can change the folders in between (swapping in +//! a link) can still win that race. The roots keep an agent to the files it was given; they don't +//! guard against other programs on the computer. + +use std::cell::RefCell; +use std::ffi::OsString; +use std::path::{Component, Path, PathBuf}; +use std::sync::{Arc, OnceLock}; + +/// Read access or write access. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Access { + Read, + Write, +} + +/// The folders automation may read and write; a root left out grants no access of its kind. +#[derive(Debug)] +pub struct AutomationRoots { + read: Option, + write: Option, +} + +#[derive(Debug)] +struct Root { + /// The folder made absolute, links followed: what paths are compared with. + resolved: PathBuf, + /// The same without Windows' `\\?\` prefix, for people. + shown: PathBuf, +} + +impl Root { + /// The folder at `path`, which must exist. + fn open(path: &Path, access: Access) -> Result { + let kind = match access { + Access::Read => "read", + Access::Write => "write", + }; + let resolved = std::fs::canonicalize(path) + .and_then(|p| if p.is_dir() { Ok(p) } else { Err(std::io::ErrorKind::NotADirectory.into()) }) + .map_err(|e| format!("cannot open the automation {kind} root `{}`: {e}", path.display()))?; + Ok(Self { shown: plain(resolved.clone()), resolved }) + } +} + +impl AutomationRoots { + /// The roots `read` and `write`, which must be existing folders; `None` when neither is given + /// (nothing is confined then). + pub fn new(read: Option<&Path>, write: Option<&Path>) -> Result>, String> { + if read.is_none() && write.is_none() { + return Ok(None); + } + let read = read.map(|p| Root::open(p, Access::Read)).transpose()?; + let write = write.map(|p| Root::open(p, Access::Write)).transpose()?; + Ok(Some(Arc::new(Self { read, write }))) + } + + /// The read root, absolute with its links followed. + pub fn read_root(&self) -> Option<&Path> { + self.read.as_ref().map(|r| r.shown.as_path()) + } + + /// The write root, absolute with its links followed. + pub fn write_root(&self) -> Option<&Path> { + self.write.as_ref().map(|r| r.shown.as_path()) + } + + /// May the file or folder at `path` be read (or written)? `Err` says why not. + pub fn check(&self, path: impl AsRef, access: Access) -> Result<(), String> { + let path = path.as_ref(); + let shown_path = path.display(); + let root = match access { + Access::Read => self.read.as_ref(), + Access::Write => self.write.as_ref(), + }; + let Some(Root { resolved: folder, shown }) = root else { + return match access { + Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {shown_path}")), + Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {shown_path}")), + }; + }; + if inside(&resolve(path)?, folder) { + return Ok(()); + } + let root = shown.display(); + match access { + Access::Read => Err(format!("automation path rejected: outside the read root {root}: {shown_path}")), + Access::Write => Err(format!("automation path rejected: outside the write root {root}: {shown_path}")), + } + } +} + +/// Where the roots are in force on a thread ([`confine`], [`unconfined`]). +#[derive(Clone, Debug)] +enum Scope { + Confined(Arc), + Unconfined, +} + +thread_local! { + /// This thread's scope; `None`: the process's roots ([`confine_process`]). + static SCOPE: RefCell> = const { RefCell::new(None) }; +} + +/// The roots in force in every thread outside [`confine`] and [`unconfined`]. +static PROCESS: OnceLock> = OnceLock::new(); + +/// Put `roots` in force in the whole process, for good. `false` when roots were in force already +/// (they stay as they were). +pub fn confine_process(roots: Arc) -> bool { + PROCESS.set(roots).is_ok() +} + +/// Puts the scope it replaced back when dropped (also when `f` panics). +struct Restore(Option); + +impl Drop for Restore { + fn drop(&mut self) { + let previous = self.0.take(); + SCOPE.with(|s| *s.borrow_mut() = previous); + } +} + +fn with_scope(scope: Option, f: impl FnOnce() -> T) -> T { + let _restore = Restore(SCOPE.with(|s| s.replace(scope))); + f() +} + +/// Run `f` with `roots` in force on this thread (`None`: as things stand). +pub fn confine(roots: Option<&Arc>, f: impl FnOnce() -> T) -> T { + match roots { + Some(r) => with_scope(Some(Scope::Confined(r.clone())), f), + None => f(), + } +} + +/// Run `f` with no roots in force on this thread: what the person at the keyboard does, and the +/// app's own folders. +pub fn unconfined(f: impl FnOnce() -> T) -> T { + with_scope(Some(Scope::Unconfined), f) +} + +/// The roots in force on this thread, if any. +pub fn current() -> Option> { + match SCOPE.with(|s| s.borrow().clone()) { + Some(Scope::Confined(r)) => Some(r), + Some(Scope::Unconfined) => None, + None => PROCESS.get().cloned(), + } +} + +/// `path` without Windows' `\\?\` prefix (which `canonicalize` adds), for people to read. +fn plain(path: PathBuf) -> PathBuf { + if !cfg!(windows) { + return path; + } + let text = path.to_string_lossy(); + if let Some(rest) = text.strip_prefix(r"\\?\UNC\") { + return PathBuf::from(format!(r"\\{rest}")); + } + match text.strip_prefix(r"\\?\") { + Some(rest) => PathBuf::from(rest), + None => path, + } +} + +/// May `path` be read here? Always, without roots in force. +pub fn check_read(path: impl AsRef) -> Result<(), String> { + current().map_or(Ok(()), |r| r.check(path.as_ref(), Access::Read)) +} + +/// May `path` be written here? Always, without roots in force. +pub fn check_write(path: impl AsRef) -> Result<(), String> { + current().map_or(Ok(()), |r| r.check(path.as_ref(), Access::Write)) +} + +/// `path` made absolute with its links followed; for a path that doesn't exist (yet), its nearest +/// existing folder's links followed and the rest appended. +fn resolve(path: &Path) -> Result { + if path.as_os_str().is_empty() { + return Err("automation path rejected: path is empty".into()); + } + // Checked as given: making `…\NUL` absolute gives the device path `\\.\NUL` on Windows. + if cfg!(windows) && path.components().any(|c| matches!(c, Component::Normal(n) if is_device_name(&n.to_string_lossy()))) { + return Err(format!("automation path rejected: reserved device names are not allowed: {}", path.display())); + } + let absolute = std::path::absolute(path).map_err(|e| format!("automation path rejected: cannot resolve {}: {e}", path.display()))?; + // The names below the nearest existing folder, last first. + let mut missing: Vec = vec![]; + let mut at = absolute.as_path(); + loop { + match std::fs::canonicalize(at) { + Ok(mut found) => { + found.extend(missing.iter().rev()); + return Ok(found); + } + Err(e) => match std::fs::symlink_metadata(at) { + Ok(m) if m.file_type().is_symlink() => { + return Err(format!("automation path rejected: a link that leads nowhere: {}", path.display())); + } + // There, yet its real path can't be had (no permission): nothing to compare. + Ok(_) => return Err(format!("automation path rejected: cannot resolve {}: {e}", path.display())), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(format!("automation path rejected: cannot resolve {}: {e}", path.display())), + }, + } + let parent = at.parent(); + match (at.components().next_back(), parent) { + (Some(Component::Normal(name)), Some(parent)) => { + missing.push(name.to_os_string()); + at = parent; + } + (Some(Component::ParentDir), _) => { + return Err(format!("automation path rejected: parent traversal below a folder that doesn't exist: {}", path.display())); + } + // Down to the root (or drive) and it doesn't exist either. + _ => { + let e = std::io::Error::from(std::io::ErrorKind::NotFound); + return Err(format!("automation path rejected: cannot resolve {}: {e}", path.display())); + } + } + } +} + +/// Is `path` (resolved) `root` or inside it, component by component (without regard to case on +/// Windows, whose file systems ignore it)? +fn inside(path: &Path, root: &Path) -> bool { + let mut path = path.components(); + root.components().all(|r| path.next().is_some_and(|p| same_component(p, r))) +} + +fn same_component(a: Component, b: Component) -> bool { + if a == b { + return true; + } + if !cfg!(windows) { + return false; + } + match (a.as_os_str().to_str(), b.as_os_str().to_str()) { + (Some(a), Some(b)) => a.to_lowercase() == b.to_lowercase(), + _ => false, + } +} + +/// A Windows device name (`CON`, `NUL`, `COM1`… also with an extension), which names no file. +fn is_device_name(component: &str) -> bool { + let stem = component.split('.').next().unwrap_or(component).trim_end_matches(' ').to_ascii_uppercase(); + let numbered = |prefix: &str| stem.strip_prefix(prefix).is_some_and(|n| matches!(n, "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9")); + matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CLOCK$" | "CONIN$" | "CONOUT$") || numbered("COM") || numbered("LPT") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A fresh folder holding `in` (read root), `out` (write root) and `outside`. + fn folders(name: &str) -> (PathBuf, Arc) { + let base = std::env::temp_dir().join(format!("sc-file-access-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + for d in ["in/sub", "out", "outside", "in2"] { + std::fs::create_dir_all(base.join(d)).unwrap(); + } + std::fs::write(base.join("in/a.scraft"), "x").unwrap(); + std::fs::write(base.join("outside/secret.scraft"), "x").unwrap(); + let roots = AutomationRoots::new(Some(&base.join("in")), Some(&base.join("out"))).unwrap().unwrap(); + (base, roots) + } + + fn s(p: PathBuf) -> String { + p.to_string_lossy().into_owned() + } + + #[test] + fn paths_inside_the_roots_pass_and_others_dont() { + let (base, roots) = folders("inside"); + assert_eq!(roots.check(s(base.join("in/a.scraft")), Access::Read), Ok(())); + assert_eq!(roots.check(s(base.join("in")), Access::Read), Ok(()), "the root itself"); + assert_eq!(roots.check(s(base.join("in/sub/new.scraft")), Access::Read), Ok(()), "missing files are checked by their folder"); + assert_eq!(roots.check(s(base.join("out/new/deeper/x.wav")), Access::Write), Ok(()), "and by their nearest folder"); + let e = roots.check(s(base.join("outside/secret.scraft")), Access::Read).unwrap_err(); + assert!(e.starts_with("automation path rejected: outside the read root"), "{e}"); + // Read and write access are separate. + let e = roots.check(s(base.join("in/a.scraft")), Access::Write).unwrap_err(); + assert!(e.contains("outside the write root"), "{e}"); + assert!(roots.check(s(base.join("out/x.wav")), Access::Read).unwrap_err().contains("outside the read root")); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn a_sibling_with_the_same_prefix_is_outside() { + let (base, roots) = folders("prefix"); + std::fs::write(base.join("in2/b.scraft"), "x").unwrap(); + assert!(roots.check(s(base.join("in2/b.scraft")), Access::Read).is_err(), "in2 is not inside in"); + assert!(roots.check(format!("{}2/c.scraft", s(base.join("in"))), Access::Read).is_err()); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn parent_traversal_is_resolved_or_refused() { + let (base, roots) = folders("dotdot"); + let sep = std::path::MAIN_SEPARATOR; + let up = format!("{}{sep}..{sep}outside{sep}secret.scraft", s(base.join("in"))); + assert!(roots.check(up, Access::Read).unwrap_err().contains("outside the read root"), "`..` out of the root"); + let back = format!("{}{sep}..{sep}a.scraft", s(base.join("in/sub"))); + assert_eq!(roots.check(back, Access::Read), Ok(()), "`..` that stays inside"); + let missing = format!("{}{sep}nope{sep}..{sep}..{sep}outside{sep}x.scraft", s(base.join("out"))); + let e = roots.check(missing, Access::Write).unwrap_err(); + // Windows resolves `..` by name first (so does opening the file there): then it's outside. + assert!(e.contains("parent traversal below a folder that doesn't exist") || e.contains("outside the write root"), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn relative_paths_resolve_against_the_working_directory() { + let (base, roots) = folders("relative"); + // The tests' working directory is the crate's folder: not inside the roots. + assert!(roots.check("a.scraft", Access::Read).unwrap_err().contains("outside the read root")); + assert!(roots.check("", Access::Read).unwrap_err().contains("path is empty")); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn a_missing_root_is_an_error_and_none_is_no_confinement() { + let (base, _) = folders("missing"); + let e = AutomationRoots::new(Some(&base.join("nope")), None).unwrap_err(); + assert!(e.starts_with("cannot open the automation read root"), "{e}"); + let e = AutomationRoots::new(None, Some(&base.join("in/a.scraft"))).unwrap_err(); + assert!( + e.starts_with("cannot open the automation write root") + && e.ends_with(&std::io::Error::from(std::io::ErrorKind::NotADirectory).to_string()), + "{e}" + ); + assert!(AutomationRoots::new(Some(Path::new("")), None).unwrap_err().starts_with("cannot open the automation read root ``")); + assert!(AutomationRoots::new(None, None).unwrap().is_none()); + // One root alone grants only its own access. + let read_only = AutomationRoots::new(Some(&base.join("in")), None).unwrap().unwrap(); + let e = read_only.check(s(base.join("in/x.scraft")), Access::Write).unwrap_err(); + assert!(e.contains("write authority is absent"), "{e}"); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn links_are_followed_before_comparing() { + let (base, roots) = folders("links"); + #[cfg(unix)] + let made = |target: &Path, link: &Path| std::os::unix::fs::symlink(target, link).is_ok(); + #[cfg(windows)] + let made = |target: &Path, link: &Path| { + let made = + if target.is_dir() { std::os::windows::fs::symlink_dir(target, link) } else { std::os::windows::fs::symlink_file(target, link) }; + made.is_ok() + }; + #[cfg(not(any(unix, windows)))] + let made = |_: &Path, _: &Path| false; + // Windows needs Developer Mode or elevation to make links: nothing to test without them. + if !made(&base.join("outside/secret.scraft"), &base.join("in/leak.scraft")) { + eprintln!("skipped: symbolic links can't be made here"); + let _ = std::fs::remove_dir_all(base); + return; + } + assert!(roots.check(s(base.join("in/leak.scraft")), Access::Read).unwrap_err().contains("outside the read root"), "a file link out"); + assert!(made(&base.join("outside"), &base.join("out/away"))); + let e = roots.check(s(base.join("out/away/new.wav")), Access::Write).unwrap_err(); + assert!(e.contains("outside the write root"), "a folder link out: {e}"); + assert!(made(&base.join("outside/gone.scraft"), &base.join("out/dangling.wav"))); + let e = roots.check(s(base.join("out/dangling.wav")), Access::Write).unwrap_err(); + assert!(e.contains("a link that leads nowhere"), "writing would create the file outside: {e}"); + // A link to a folder inside the root stays inside. + assert!(made(&base.join("in/sub"), &base.join("in/alias"))); + assert_eq!(roots.check(s(base.join("in/alias/x.scraft")), Access::Read), Ok(())); + // A root given through a link is the folder it leads to. + assert!(made(&base.join("in"), &base.join("in-link"))); + let linked = AutomationRoots::new(Some(&base.join("in-link")), None).unwrap().unwrap(); + assert_eq!(linked.check(s(base.join("in/a.scraft")), Access::Read), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[cfg(windows)] + #[test] + fn windows_ignores_case_and_refuses_devices() { + let (base, roots) = folders("case"); + let upper = s(base.join("IN/a.scraft")); + assert_eq!(roots.check(upper, Access::Read), Ok(()), "{upper}"); + assert_eq!(roots.check(s(base.join("In/Sub/New.scraft")), Access::Read), Ok(())); + let e = roots.check(s(base.join("out/NUL")), Access::Write).unwrap_err(); + assert!(e.contains("reserved device names"), "{e}"); + assert!(roots.check(s(base.join("out/com1.txt")), Access::Write).is_err()); + assert_eq!(roots.check(s(base.join("out/console.txt")), Access::Write), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn scopes_nest_and_stay_on_their_thread() { + let (base, roots) = folders("scopes"); + let outside = s(base.join("outside/secret.scraft")); + assert_eq!(check_read(&outside), Ok(()), "nothing is confined by default"); + confine(Some(&roots), || { + assert!(check_read(&outside).is_err()); + assert_eq!(unconfined(|| check_read(&outside)), Ok(())); + assert!(check_read(&outside).is_err(), "restored after unconfined"); + let o = outside.clone(); + let there = std::thread::spawn(move || check_read(&o).is_ok()).join().unwrap(); + assert!(there, "another thread is not confined by this thread's scope"); + }); + assert_eq!(check_read(&outside), Ok(()), "restored after confine"); + let r = std::panic::catch_unwind(|| confine(Some(&roots), || panic!("boom"))); + assert!(r.is_err()); + assert_eq!(check_read(&outside), Ok(()), "restored after a panic"); + assert_eq!(confine(None, || check_read(&outside)), Ok(())); + let _ = std::fs::remove_dir_all(base); + } + + #[test] + fn device_names() { + for d in ["CON", "nul", "Com1", "LPT9.txt", "aux.scraft", "NUL "] { + assert!(is_device_name(d), "{d}"); + } + for f in ["console", "com10", "COM0", "lpt", "a.scraft", "nul-x"] { + assert!(!is_device_name(f), "{f}"); + } + } + + /// Every file a command reads or writes by path goes through `io` (`read_file`, `write_file` + /// and the other checked helpers there), which check the roots. Engine code that reaches for + /// the file system itself would bypass them, so none may: every file under `src/` except + /// `io.rs`, this module and the test files. Audio is decoded into the session when it enters + /// it (import, open, sampler load), so playback and bounce read nothing from disk and need no + /// check of their own. No engine module reads a fixed folder (plug-in scans live in the host + /// crates), so nothing else is skipped. + #[test] + fn commands_do_not_touch_the_file_system_themselves() { + fn walk(dir: &Path, files: &mut Vec) { + for entry in std::fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + walk(&path, files); + } else if path.extension().is_some_and(|e| e == "rs") { + files.push(path); + } + } + } + let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = vec![]; + walk(&src, &mut files); + assert!(files.iter().any(|f| f.ends_with("lib.rs")) && files.iter().any(|f| f.ends_with("cmd/file.rs")), "{files:?}"); + let mut offenders = vec![]; + for path in files { + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + // `io` and this module are the checked gateway; `tests*.rs` are test-only modules. + if path.parent() == Some(src.as_path()) && matches!(name, "io.rs" | "file_access.rs") || name.starts_with("tests") { + continue; + } + let text = std::fs::read_to_string(&path).unwrap(); + // Test modules (which build their own fixtures) come last in each file. + let code = text.split("#[cfg(test)]").next().unwrap_or(""); + for (n, line) in code.lines().enumerate() { + let line = line.trim_start(); + let touches = + ["std::fs", "fs::", "File::open", "File::create", "OpenOptions", ".is_file()", ".exists()", ".is_dir()", "canonicalize"] + .iter() + .any(|w| line.contains(w)); + if !line.starts_with("//") && touches { + offenders.push(format!("{}:{}: {line}", path.display(), n + 1)); + } + } + } + assert!(offenders.is_empty(), "use the checked helpers in crate::io (read_file, write_file, ...): {offenders:#?}"); + } +} diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index ddf328a..ca5d7fe 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -11,6 +11,50 @@ use std::sync::Arc; /// Largest import we accept (samples per channel × channels), ~4 GiB of f32. const MAX_IMPORT_SAMPLES: usize = 1 << 30; +// ─── Checked file access ────────────────────────────────────────────────────────────────────── +// +// Every file a command names by path is read and written through the helpers below, so the +// automation roots (`crate::file_access`, `soundcraft-cli mcp --automation-read-root` and +// `--automation-write-root`) are checked in one place. A test in `file_access` keeps the rest of +// the engine from calling `std::fs` itself. Audio is decoded into the session's pool the moment it +// enters it (import, open, sampler load), so playback and bounce read nothing from disk and +// need no check of their own. + +/// The bytes of the file at `path`, which must be inside the automation read root when there is one. +pub fn read_file(path: impl AsRef) -> Result> { + let path = path.as_ref(); + crate::file_access::check_read(path).map_err(EngineError::Io)?; + std::fs::read(path).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + +/// [`read_file`] for a text file. +pub fn read_text(path: impl AsRef) -> Result { + let path = path.as_ref(); + crate::file_access::check_read(path).map_err(EngineError::Io)?; + std::fs::read_to_string(path).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + +/// Is there a file at `path` the automation read root lets us see? A path outside the root is +/// reported as not found, so a command can't probe for files beyond it. +pub fn file_found(path: impl AsRef) -> bool { + let path = path.as_ref(); + crate::file_access::check_read(path).is_ok() && path.is_file() +} + +/// Create `dir` and its parents, inside the automation write root when there is one. +pub(crate) fn make_dir(dir: impl AsRef) -> Result<()> { + let dir = dir.as_ref(); + crate::file_access::check_write(dir).map_err(EngineError::Io)?; + std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.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(); + crate::file_access::check_write(path).map_err(EngineError::Io)?; + std::fs::write(path, bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + /// A relative `Audio Files/` path no other source in `s` already uses. fn unique_audio_rel(s: &Session, stem: &str) -> String { let stem = sanitize_name(stem); @@ -152,12 +196,13 @@ pub fn relink_videos(s: &mut Session, session_dir: &Path) -> Vec { let mut missing = Vec::new(); for v in &mut s.videos { let p = if Path::new(&v.path).is_absolute() { PathBuf::from(&v.path) } else { session_dir.join(&v.path) }; - if p.is_file() { + if file_found(&p) { continue; } let name = Path::new(&v.path).file_name().map(PathBuf::from).unwrap_or_default(); - let near = - [session_dir.join(&name), session_dir.join("Video Files").join(&name)].into_iter().find(|c| !name.as_os_str().is_empty() && c.is_file()); + let near = [session_dir.join(&name), session_dir.join("Video Files").join(&name)] + .into_iter() + .find(|c| !name.as_os_str().is_empty() && file_found(c)); match near { Some(c) => v.path = c.to_string_lossy().into_owned(), None => missing.push(format!("{} (video)", p.display())), @@ -303,7 +348,7 @@ pub fn bounce_stems(e: &Engine, dir: &str, r: Range, opts: &EncodeOptions) -> Re /// [`bounce_stems`] in the main format, or folded down to stereo when `fold_stereo`. pub fn bounce_stems_with(e: &Engine, dir: &str, r: Range, opts: &EncodeOptions, fold_stereo: bool) -> Result> { let s = e.session(); - std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{dir}: {err}")))?; + make_dir(dir)?; let ext = match opts.format { FileFormat::Aiff => "aif", FileFormat::Flac => "flac", @@ -325,7 +370,7 @@ pub fn bounce_stems_with(e: &Engine, dir: &str, r: Range, opts: &EncodeOptions, let buf = AudioBuffer { sample_rate: s.sample_rate.hz(), channels: ch }; let bytes = soundcraft_audio_io::encode_with_channel_mask(&buf, opts, fmt.channel_mask()).map_err(|err| EngineError::Io(err.to_string()))?; let path = Path::new(dir).join(format!("{} - {}.{ext}", sanitize_name(&s.name), sanitize_name(&name))); - std::fs::write(&path, bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))?; + write_path(&path, bytes)?; out.push(path.to_string_lossy().into_owned()); } Ok(out) @@ -534,7 +579,7 @@ pub fn commit_track(e: &mut Engine, t: TrackId) -> Result> { pub fn export_clips(e: &Engine, ids: &[soundcraft_model::ClipId], dir: &str) -> Result { let s = e.session(); - std::fs::create_dir_all(dir).map_err(|err| EngineError::Io(format!("{dir}: {err}")))?; + make_dir(dir)?; let mut n = 0; for id in ids { let Some((t, c)) = s.find_clip(*id) else { continue }; @@ -551,7 +596,7 @@ pub fn export_clips(e: &Engine, ids: &[soundcraft_model::ClipId], dir: &str) -> return Err(EngineError::Io(format!("{dir}: too many files named {stem}"))); } } - std::fs::write(&path, bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display())))?; + write_path(&path, bytes)?; n += 1; } Ok(n) @@ -596,6 +641,8 @@ pub fn save_session_copy(e: &Engine, path: &str) -> Result { /// files written and the final path. fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Session, usize, PathBuf)> { let p = session_file(path); + // Before any audio is written: a session saved outside the write root changes nothing. + crate::file_access::check_write(&p).map_err(EngineError::Io)?; let dir = p.parent().map(Path::to_path_buf).unwrap_or_default(); let audio_dir = dir.join("Audio Files"); let mut s = session.clone(); @@ -623,7 +670,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess continue; } let Some(audio) = s.pool.get(src.id) else { continue }; - std::fs::create_dir_all(&audio_dir).map_err(|err| EngineError::Io(format!("{}: {err}", audio_dir.display())))?; + make_dir(&audio_dir)?; let mut stem = sanitize_name(&src.name); if overwrite { // One file per source id so autosaves never collide or pile up. @@ -650,7 +697,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess s.name = stem.to_string(); } let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; - std::fs::create_dir_all(&dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))?; + make_dir(&dir)?; // Atomic-ish write: temp file then rename. let tmp = p.with_extension("scraft.tmp"); if !overwrite && std::fs::read(&p).is_ok_and(|old| old != text.as_bytes()) { @@ -663,6 +710,8 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess /// Write `bytes` to `tmp`, flush it to disk and rename it over `file`, so `file` holds either the /// old or the new contents, never a mix. pub fn replace_file(tmp: &Path, file: &Path, bytes: &[u8]) -> Result<()> { + crate::file_access::check_write(file).map_err(EngineError::Io)?; + crate::file_access::check_write(tmp).map_err(EngineError::Io)?; if let Err(err) = write_synced(tmp, bytes).and_then(|()| std::fs::rename(tmp, file)) { let _ = std::fs::remove_file(tmp); return Err(EngineError::Io(format!("{}: {err}", file.display()))); @@ -691,6 +740,10 @@ fn back_up(file: &Path) { return; }; let backups = dir.join("Backups"); + if let Err(err) = crate::file_access::check_write(&backups) { + log::warn!("no backup of {}: {err}", file.display()); + return; + } let prefix = format!("{stem} backup "); let suffix = format!(".{ext}"); let numbers: Vec = std::fs::read_dir(&backups) @@ -714,7 +767,9 @@ fn back_up(file: &Path) { /// extension and isn't an existing file. pub fn session_file(path: &str) -> PathBuf { let p = PathBuf::from(path); - if p.extension().is_none() && !p.is_file() { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p } + // Only a path the write root allows is probed, so a refused one can't be used to test for files. + let is_file = crate::file_access::check_write(&p).is_ok() && p.is_file(); + if p.extension().is_none() && !is_file { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p } } /// True when `command` would write a session over the existing `file`. Save, Save As and Save @@ -763,12 +818,17 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { /// Open a `.scraft` session and decode its audio files (missing files are reported, not fatal). pub fn open_session(e: &mut Engine, path: &str) -> Result> { - let text = std::fs::read_to_string(path).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; + let text = read_text(path)?; let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); for src in s.sources.clone() { let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; + // A session file is hostile input too: a path in it that leaves the read root is not read. + if let Err(why) = crate::file_access::check_read(&p) { + missing.push(why); + continue; + } match std::fs::read(&p) { Ok(bytes) => { if let Err(err) = load_source_audio(&mut s, src.id, &bytes, p.extension().and_then(|x| x.to_str())) { @@ -829,6 +889,7 @@ pub fn write_file(path: &str, bytes: &[u8]) -> Result<()> { let name = Path::new(path).file_name().and_then(|n| n.to_str()).filter(|n| !n.is_empty()).unwrap_or("Untitled"); return deliver(name, bytes).map_err(EngineError::Io); } + crate::file_access::check_write(path).map_err(EngineError::Io)?; std::fs::write(path, bytes).map_err(|err| EngineError::Io(format!("{path}: {err}"))) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c4e2f54..b73384c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -11,6 +11,7 @@ pub mod clip_group_file; pub mod cmd; pub mod demo; pub mod edit; +pub mod file_access; pub mod inspect; pub mod io; pub mod score; diff --git a/docs/mcp.md b/docs/mcp.md index 7102a23..d31712b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -6,6 +6,7 @@ Claude and other agents can make music with it. ```sh soundcraft-cli mcp # headless engine (no window, no audio device) soundcraft-cli mcp --demo # headless, starting from the demo session +soundcraft-cli mcp --automation-read-root DIR --automation-write-root DIR # headless, confined to folders soundcraft --control 7801 & # the desktop app… soundcraft-cli mcp --connect 7801 # …driven over MCP: same tools plus screenshots and clicks ``` @@ -16,6 +17,73 @@ Claude Code: claude mcp add soundcraft -- soundcraft-cli mcp --demo ``` +## Confining file access + +```sh +soundcraft-cli mcp --automation-read-root /work/project --automation-write-root /work/project/out +``` + +```json +{"mcpServers": {"soundcraft": {"command": "soundcraft-cli", + "args": ["mcp", "--automation-read-root", "/work/project", "--automation-write-root", "/work/project/out"]}}} +``` + +The two flags (VectorCraft and the other Craft apps take the same ones, so one client config +works across the suite) keep the agent to the folders you give it: + +- Every file a command reads must lie inside the **read root**: `open_session`, `session.open`, + `import_audio` and `file.import_audio`, `file.import_midi`, `file.import_video`, + `file.import_clip_groups`, `file.import_session_data`, `track.load_preset` and + `mix.sampler_load`. The media a session file names is read too, so an opened session whose + audio paths lead outside the read root loads without that audio and lists it under `missing`. +- Every file a command writes must lie inside the **write root**: `save_session`, `session.save`, + `session.save_as`, `session.save_copy`, `session.save_template`, `bounce_mix` and + `file.bounce_mix`, `file.bounce_stems`, `file.export_clips`, `file.export_midi`, + `file.export_session_text`, `file.export_selected_tracks_as_session`, `file.export_sibelius`, + `file.send_to_sibelius`, `file.print_score`, `file.export_clip_groups` and + `track.save_preset`, together with what a session save writes beside the session file + (`Audio Files/`, `Backups/`) and its temporary file. A score command given no `path` writes to + the temp folder, which is outside the write root unless you made it the root, so it is refused. +- Read and write access are separate. Give the same folder twice for both; a root left out grants + none of its access (with only `--automation-read-root`, nothing can be written to disk). Without + either flag nothing changes. +- A path is made absolute against the server's working directory, then its links are followed: a + file (or folder) link that leads out of a root is outside it. For a file that doesn't exist yet, + its nearest existing folder's links are followed and the rest must be plain names, so `..` below a + folder that doesn't exist is refused, as is a link that leads nowhere (writing through it would + create its target). The result is compared with the root by whole folder names: `/work2/x` is not + inside `/work`. On Windows case is ignored, UNC paths (`\\server\share\...`) are resolved like + drive-letter ones, and device names (`CON`, `NUL`, `COM1`...) are refused. +- A refused path is an ordinary tool error the agent can read and correct, for example + `automation path rejected: outside the write root /work/project/out: /etc/x.wav` or + `automation filesystem access is not granted: write authority is absent: /work/project/out/x.wav`. + A refused save path is reported as refused, never as "already exists". +- The roots must be existing folders; a missing one stops the server with an error. Each flag is + given once (`--flag ` or `--flag=`), and a misspelled `--automation-...` flag is an + error rather than an unconfined server. + +The flags confine the in-process server, so `--connect` together with them is an error: the server +would otherwise talk to an app that isn't confined. SoundCraft's desktop app has no equivalent flags +yet, so a running app (`soundcraft --control`) is not confined. `--demo` is the operator's own +choice and is built before the roots apply. + +Playback and bounce need no check of their own. SoundCraft decodes audio into the session when it +enters it (import, open, sampler load), so rendering reads nothing from disk afterwards. Movies are +the exception: a session only references them by path and the desktop app decodes pictures from +that file, but the headless server never does, and the movie's path is checked when it is imported +and when a session naming it is opened. + +What the roots do not cover: commands that take no path still read and change the open session, +and `inspect_session` returns it. Plug-in discovery scans the system's plug-in folders and loads +plug-in binaries, which are fixed locations and native code, not agent paths. Recording is only +reachable in the desktop app. Anything that reads or writes a path goes through one place in the +engine (`io::read_file`, `io::write_file` and the helpers beside them in `crates/engine/src/io.rs`); +a test keeps the rest of the engine from touching the file system on its own. + +The checks hold for every thread of the server process. A path is checked, then opened: another +program that swaps a link in between can still win that race, so the roots keep an agent to the +files it was given rather than guarding against other software on the computer. + ## Tools | Tool | Purpose |