Merge pull request #979 from marcoarnulfo/feat/repeatable-automation-roots

MCP: --automation-read-root and --automation-write-root may each be given more than once

apps/soundcraft-cli/src/automation_args.rs

@@ -3,30 +3,22 @@
 
 use std::path::PathBuf;
 
+#[derive(Default)]
 pub struct AutomationArgs {
-    pub read_root: Option<PathBuf>,
+    pub read_roots: Vec<PathBuf>,
-    pub write_root: Option<PathBuf>,
+    pub write_roots: Vec<PathBuf>,
-    read_given: bool,
-    write_given: bool,
 }
 
 impl AutomationArgs {
-    pub fn new(read_root: Option<PathBuf>, write_root: Option<PathBuf>) -> Self {
+    /// Returns whether this argument is a root flag. Each flag may be given more than once (#874),
-        Self { read_root, write_root, read_given: false, write_given: false }
+    /// in either spelling (`--flag DIR`, `--flag=DIR`): every use adds a folder.
-    }
-
-    /// 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<Item = String>) -> Result<bool, String> {
         let (flag, inline) = arg.split_once('=').map_or((arg, None), |(flag, value)| (flag, Some(value)));
-        let (root, given) = match flag {
+        let roots = match flag {
-            "--automation-read-root" => (&mut self.read_root, &mut self.read_given),
+            "--automation-read-root" => &mut self.read_roots,
-            "--automation-write-root" => (&mut self.write_root, &mut self.write_given),
+            "--automation-write-root" => &mut self.write_roots,
             _ => return Ok(false),
         };
-        if *given {
-            return Err(format!("{flag} given twice"));
-        }
         let value = match inline {
             Some(value) => value.to_owned(),
             None => {
@@ -40,8 +32,7 @@ impl AutomationArgs {
         if value.is_empty() {
             return Err(format!("{flag} needs a non-empty folder"));
         }
-        *root = Some(value.into());
+        roots.push(value.into());
-        *given = true;
         Ok(true)
     }
 }
@@ -51,40 +42,43 @@ mod tests {
     use super::*;
 
     #[test]
-    fn roots_are_taken_once_and_other_arguments_are_left() {
+    fn roots_add_up_and_other_arguments_are_left() {
-        let mut roots = AutomationArgs::new(None, None);
+        let mut roots = AutomationArgs::default();
         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.read_roots, [PathBuf::from("a folder")]);
-        assert_eq!(roots.write_root, Some("out".into()));
+        assert_eq!(roots.write_roots, [PathBuf::from("out")]);
         assert_eq!(args.next().as_deref(), Some("song.scraft"));
-        assert!(roots.consume("--automation-read-root=other", &mut args).is_err());
+        // #874: a flag given again adds a folder, in either spelling.
-        assert_eq!(roots.read_root, Some("a folder".into()));
+        assert!(roots.consume("--automation-read-root=other", &mut args).unwrap());
+        assert!(roots.consume("--automation-write-root", &mut ["out2".to_owned()].into_iter()).unwrap());
+        assert_eq!(roots.read_roots, [PathBuf::from("a folder"), PathBuf::from("other")]);
+        assert_eq!(roots.write_roots, [PathBuf::from("out"), PathBuf::from("out2")]);
     }
 
     #[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 roots = AutomationArgs { read_roots: vec!["read".into()], write_roots: vec!["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.read_roots, [PathBuf::from("read")]);
-                assert_eq!(roots.write_root, Some("write".into()));
+                assert_eq!(roots.write_roots, [PathBuf::from("write")]);
             }
-            let mut roots = AutomationArgs::new(None, None);
+            let mut roots = AutomationArgs::default();
             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);
+        let mut roots = AutomationArgs::default();
         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.read_roots, [PathBuf::from("-folder")]);
-        assert_eq!(roots.write_root, Some("./-folder".into()));
+        assert_eq!(roots.write_roots, [PathBuf::from("./-folder")]);
     }
 }

apps/soundcraft-cli/src/main.rs

@@ -8,12 +8,14 @@
 //! 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] [--automation-read-root DIR] [--automation-write-root DIR]
+//! 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
+//!                                                 MCP server on stdio. The 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
+//!                                                 each flag may be given more than once, a path inside any
-//!                                                 in-process server, so they conflict with --connect.
+//!                                                 of its folders being allowed, and a kind 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
 //! ```
@@ -398,7 +400,7 @@ struct McpArgs {
 fn parse_mcp_args(args: &[String]) -> Result<McpArgs, String> {
     let mut connect = None;
     let mut demo = false;
-    let mut roots_args = automation_args::AutomationArgs::new(None, None);
+    let mut roots_args = automation_args::AutomationArgs::default();
     let mut it = args.iter().cloned();
     while let Some(a) = it.next() {
         if roots_args.consume(&a, &mut it)? {
@@ -411,7 +413,9 @@ fn parse_mcp_args(args: &[String]) -> Result<McpArgs, String> {
             _ => {}
         }
     }
-    let roots = AutomationRoots::new(roots_args.read_root.as_deref(), roots_args.write_root.as_deref())?;
+    let read: Vec<&std::path::Path> = roots_args.read_roots.iter().map(std::path::PathBuf::as_path).collect();
+    let write: Vec<&std::path::Path> = roots_args.write_roots.iter().map(std::path::PathBuf::as_path).collect();
+    let roots = AutomationRoots::new(&read, &write)?;
     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)"
@@ -437,8 +441,7 @@ fn mcp(args: &[String]) -> ExitCode {
         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: {}", roots.describe());
-            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);

apps/soundcraft-cli/tests/it/cli.rs

@@ -110,6 +110,56 @@ fn mcp_automation_roots_confine_the_server() {
     let _ = std::fs::remove_dir_all(base);
 }
 
+/// #874: each root flag may be given more than once. A path inside any root of its kind passes;
+/// the two roots' common parent, `..` out of a root and a link out of one stay refused.
+#[test]
+fn mcp_automation_roots_take_several_folders() {
+    // Resolved, as the server shows the roots (`/var` is `/private/var` on macOS).
+    let base = std::fs::canonicalize(tmpdir("several")).unwrap();
+    for d in ["in", "in2", "out", "out2", "outside"] {
+        std::fs::create_dir_all(base.join(d)).unwrap();
+    }
+    for f in ["in/a.wav", "in2/b.wav", "outside/secret.wav", "parent.wav"] {
+        make_wav(&base.join(f));
+    }
+    let p = |f: &str| base.join(f).to_str().unwrap().to_string();
+    let sep = std::path::MAIN_SEPARATOR;
+    let calls = [
+        ("import_audio", serde_json::json!({"path": p("in/a.wav")})),
+        ("import_audio", serde_json::json!({"path": p("in2/b.wav")})),
+        // The roots' common parent is not a root.
+        ("import_audio", serde_json::json!({"path": p("parent.wav")})),
+        ("import_audio", serde_json::json!({"path": format!("{}{sep}..{sep}outside{sep}secret.wav", p("in2"))})),
+        ("bounce_mix", serde_json::json!({"path": p("out/a.wav")})),
+        ("bounce_mix", serde_json::json!({"path": p("out2/b.wav")})),
+        ("bounce_mix", serde_json::json!({"path": p("x.wav")})),
+        // Read and write stay separate: a read root is not a write root, nor the other way round.
+        ("bounce_mix", serde_json::json!({"path": p("in2/c.wav")})),
+        ("import_audio", serde_json::json!({"path": p("out2/b.wav")})),
+    ];
+    let read2 = format!("--automation-read-root={}", p("in2"));
+    let args = ["--automation-read-root", &p("in"), &read2, "--automation-write-root", &p("out"), "--automation-write-root", &p("out2")];
+    let (success, stderr, replies) = mcp_session(&args, &calls);
+    assert!(success, "{stderr}");
+    assert!(stderr.contains(&format!("read {}, {}", p("in"), p("in2"))), "{stderr}");
+    let errors: Vec<bool> = replies.iter().map(|r| r["result"]["isError"] == true).collect();
+    assert_eq!(errors, [false, false, true, true, false, false, true, true, true], "{replies:?}");
+    for (i, kind) in [(2, "read"), (3, "read"), (6, "write"), (7, "write"), (8, "read")] {
+        assert!(text(&replies[i]).contains(&format!("outside the {kind} roots ")), "{}", text(&replies[i]));
+    }
+    assert!(base.join("out/a.wav").is_file() && base.join("out2/b.wav").is_file());
+    assert!(!base.join("x.wav").exists() && !base.join("in2/c.wav").exists());
+
+    // A link in one root that leads out of all of them is outside.
+    #[cfg(unix)]
+    {
+        std::os::unix::fs::symlink(base.join("outside/secret.wav"), base.join("in2/leak.wav")).unwrap();
+        let (_, _, replies) = mcp_session(&args, &[("import_audio", serde_json::json!({"path": p("in2/leak.wav")}))]);
+        assert!(text(&replies[0]).contains("outside the read roots"), "{}", text(&replies[0]));
+    }
+    let _ = std::fs::remove_dir_all(base);
+}
+
 #[test]
 fn a_bare_save_session_name_lands_in_the_working_directory_inside_the_roots() {
     let work = tmpdir("bare-name");
@@ -144,8 +194,9 @@ fn mcp_automation_root_flags_are_checked_up_front() {
     // 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");
+    // #874: a flag may be given again, but every folder it names must exist.
-    fails(&["--automation-write-root", dir, "--automation-write-root", dir], "given twice");
+    fails(&["--automation-read-root", dir, "--automation-read-root", missing.to_str().unwrap()], "cannot open the automation read root");
+    fails(&["--automation-write-root", dir, "--automation-write-root"], "needs a folder");
     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");

crates/automation/tests/it/automation_roots.rs

@@ -57,7 +57,7 @@ fn fixture(name: &str) -> Fixture {
 }
 
 fn server(read: Option<&Path>, write: Option<&Path>) -> Server<Headless> {
-    let roots = AutomationRoots::new(read, write).unwrap();
+    let roots = AutomationRoots::new(read.as_slice(), write.as_slice()).unwrap();
     Server::new(Headless::new(Engine::default()).with_automation_roots(roots))
 }
 
@@ -186,7 +186,7 @@ fn writes(dir: &Path) -> Vec<(&'static str, Value)> {
 }
 
 fn demo_server(read: Option<&Path>, write: Option<&Path>) -> Server<Headless> {
-    let roots = AutomationRoots::new(read, write).unwrap();
+    let roots = AutomationRoots::new(read.as_slice(), write.as_slice()).unwrap();
     Server::new(Headless::new(soundcraft_engine::demo::demo_engine()).with_automation_roots(roots))
 }
 

crates/engine/src/file_access.rs

@@ -1,15 +1,16 @@
 //! The folders automation may read and write.
 //!
 //! `soundcraft-cli mcp --automation-read-root <dir> --automation-write-root <dir>` confines the
-//! files an agent's commands reach. [`AutomationRoots`] holds the two folders. While they are in
+//! files an agent's commands reach; each flag may be given more than once (#874). [`AutomationRoots`]
-//! force, every path a command reads goes through [`check_read`] and every path it writes through [`check_write`]:
+//! holds the folders of each kind. 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
+//! - The result must lie inside one of the roots of its kind, compared by whole components
-//!   `/work`), without regard to case on Windows.
+//!   (`/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
@@ -32,11 +33,11 @@ pub enum Access {
     Write,
 }
 
-/// The folders automation may read and write; a root left out grants no access of its kind.
+/// The folders automation may read and write; a kind with no root grants no access of its kind.
 #[derive(Debug)]
 pub struct AutomationRoots {
-    read: Option<Root>,
+    read: Vec<Root>,
-    write: Option<Root>,
+    write: Vec<Root>,
 }
 
 #[derive(Debug)]
@@ -62,48 +63,61 @@ impl Root {
 }
 
 impl AutomationRoots {
-    /// The roots `read` and `write`, which must be existing folders; `None` when neither is given
+    /// The roots `read` and `write`, which must be existing folders (one given twice counts once);
-    /// (nothing is confined then).
+    /// `None` when neither kind has any (nothing is confined then).
-    pub fn new(read: Option<&Path>, write: Option<&Path>) -> Result<Option<Arc<Self>>, String> {
+    pub fn new(read: &[&Path], write: &[&Path]) -> Result<Option<Arc<Self>>, String> {
-        if read.is_none() && write.is_none() {
+        if read.is_empty() && write.is_empty() {
             return Ok(None);
         }
-        let read = read.map(|p| Root::open(p, Access::Read)).transpose()?;
+        let open = |paths: &[&Path], access: Access| -> Result<Vec<Root>, String> {
-        let write = write.map(|p| Root::open(p, Access::Write)).transpose()?;
+            let mut roots: Vec<Root> = Vec::with_capacity(paths.len());
-        Ok(Some(Arc::new(Self { read, write })))
+            for path in paths {
+                let root = Root::open(path, access)?;
+                if !roots.iter().any(|r| r.resolved == root.resolved) {
+                    roots.push(root);
+                }
+            }
+            Ok(roots)
+        };
+        Ok(Some(Arc::new(Self { read: open(read, Access::Read)?, write: open(write, Access::Write)? })))
     }
 
-    /// The read root, absolute with its links followed.
+    /// The roots for people: `read A, B; write C` (`none` for a kind without any).
-    pub fn read_root(&self) -> Option<&Path> {
+    pub fn describe(&self) -> String {
-        self.read.as_ref().map(|r| r.shown.as_path())
+        let list = |roots: &[Root]| {
+            if roots.is_empty() {
+                return "none".to_string();
+            }
+            roots.iter().map(|r| r.shown.display().to_string()).collect::<Vec<_>>().join(", ")
+        };
+        format!("read {}; write {}", list(&self.read), list(&self.write))
     }
 
-    /// The write root, absolute with its links followed.
+    /// May the file or folder at `path` be read (or written)? It must lie inside one of the roots
-    pub fn write_root(&self) -> Option<&Path> {
+    /// of that kind. `Err` says why not.
-        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<Path>, access: Access) -> Result<(), String> {
         let path = path.as_ref();
         let shown_path = path.display();
-        let root = match access {
+        let roots = match access {
-            Access::Read => self.read.as_ref(),
+            Access::Read => &self.read,
-            Access::Write => self.write.as_ref(),
+            Access::Write => &self.write,
         };
-        let Some(Root { resolved: folder, shown }) = root else {
+        if roots.is_empty() {
             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) {
+        let resolved = resolve(path)?;
+        if roots.iter().any(|r| inside(&resolved, &r.resolved)) {
             return Ok(());
         }
-        let root = shown.display();
+        let shown: Vec<String> = roots.iter().map(|r| r.shown.display().to_string()).collect();
-        match access {
+        match (shown.as_slice(), access) {
-            Access::Read => Err(format!("automation path rejected: outside the read root {root}: {shown_path}")),
+            ([root], 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}")),
+            ([root], Access::Write) => Err(format!("automation path rejected: outside the write root {root}: {shown_path}")),
+            (_, Access::Read) => Err(format!("automation path rejected: outside the read roots {}: {shown_path}", shown.join(", "))),
+            (_, Access::Write) => Err(format!("automation path rejected: outside the write roots {}: {shown_path}", shown.join(", "))),
         }
     }
 }
@@ -280,7 +294,7 @@ mod tests {
         }
         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();
+        let roots = AutomationRoots::new(&[&base.join("in")], &[&base.join("out")]).unwrap().unwrap();
         (base, roots)
     }
 
@@ -340,18 +354,18 @@ mod tests {
     #[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();
+        let e = AutomationRoots::new(&[&base.join("nope")], &[]).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();
+        let e = AutomationRoots::new(&[], &[&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(&[Path::new("")], &[]).unwrap_err().starts_with("cannot open the automation read root ``"));
-        assert!(AutomationRoots::new(None, None).unwrap().is_none());
+        assert!(AutomationRoots::new(&[], &[]).unwrap().is_none());
         // One root alone grants only its own access.
-        let read_only = AutomationRoots::new(Some(&base.join("in")), None).unwrap().unwrap();
+        let read_only = AutomationRoots::new(&[&base.join("in")], &[]).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);
@@ -388,11 +402,66 @@ mod tests {
         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();
+        let linked = AutomationRoots::new(&[&base.join("in-link")], &[]).unwrap().unwrap();
         assert_eq!(linked.check(s(base.join("in/a.scraft")), Access::Read), Ok(()));
         let _ = std::fs::remove_dir_all(base);
     }
 
+    /// #874: several roots of a kind: a path inside any of them passes, one inside none is refused
+    /// naming them all, and read and write stay separate.
+    #[test]
+    fn several_roots_of_a_kind_each_grant_their_access() {
+        let (base, _) = folders("several");
+        std::fs::create_dir_all(base.join("out2")).unwrap();
+        std::fs::write(base.join("in2/b.scraft"), "x").unwrap();
+        std::fs::write(base.join("parent.scraft"), "x").unwrap();
+        let roots = AutomationRoots::new(&[&base.join("in"), &base.join("in2")], &[&base.join("out"), &base.join("out2")]).unwrap().unwrap();
+        assert_eq!(roots.check(s(base.join("in/a.scraft")), Access::Read), Ok(()));
+        assert_eq!(roots.check(s(base.join("in2/b.scraft")), Access::Read), Ok(()));
+        let e = roots.check(s(base.join("outside/secret.scraft")), Access::Read).unwrap_err();
+        assert!(e.starts_with("automation path rejected: outside the read roots ") && e.contains(", "), "{e}");
+        // Two sibling roots don't make their parent a root.
+        assert!(roots.check(s(base.join("parent.scraft")), Access::Read).unwrap_err().contains("outside the read roots"));
+        assert!(roots.check(s(base.clone()), Access::Read).is_err());
+        assert_eq!(roots.check(s(base.join("out/x.wav")), Access::Write), Ok(()));
+        assert_eq!(roots.check(s(base.join("out2/new/x.wav")), Access::Write), Ok(()));
+        assert!(roots.check(s(base.join("outside/x.wav")), Access::Write).unwrap_err().contains("outside the write roots"));
+        assert!(roots.check(s(base.join("x.wav")), Access::Write).is_err());
+        assert!(roots.check(s(base.join("in2/b.scraft")), Access::Write).is_err());
+        assert!(roots.check(s(base.join("out2/x.wav")), Access::Read).is_err());
+        // `..` out of one root is outside, also when it lands next to another root.
+        let sep = std::path::MAIN_SEPARATOR;
+        let up = format!("{}{sep}..{sep}outside{sep}secret.scraft", s(base.join("in2")));
+        assert!(roots.check(up, Access::Read).is_err());
+        // `..` from one root into another stays inside.
+        let across = format!("{}{sep}..{sep}in{sep}a.scraft", s(base.join("in2")));
+        assert_eq!(roots.check(across, Access::Read), Ok(()));
+        // A folder given twice (here once through `..`) counts once.
+        let twice = AutomationRoots::new(&[&base.join("in"), &base.join("in/sub/..")], &[]).unwrap().unwrap();
+        assert!(!twice.describe().contains(", ") && twice.describe().ends_with("; write none"), "{}", twice.describe());
+        // A missing folder among several is an error.
+        let e = AutomationRoots::new(&[&base.join("in"), &base.join("nope")], &[]).unwrap_err();
+        assert!(e.starts_with("cannot open the automation read root"), "{e}");
+        let _ = std::fs::remove_dir_all(base);
+    }
+
+    /// A link inside one root that leads into another root of the same kind stays inside; one that
+    /// leads out of all of them is outside.
+    #[cfg(unix)]
+    #[test]
+    fn links_between_several_roots() {
+        let (base, _) = folders("several-links");
+        std::fs::write(base.join("in2/b.scraft"), "x").unwrap();
+        std::os::unix::fs::symlink(base.join("in2/b.scraft"), base.join("in/to-in2.scraft")).unwrap();
+        std::os::unix::fs::symlink(base.join("outside/secret.scraft"), base.join("in2/leak.scraft")).unwrap();
+        std::os::unix::fs::symlink(&base, base.join("in2/up")).unwrap();
+        let roots = AutomationRoots::new(&[&base.join("in"), &base.join("in2")], &[]).unwrap().unwrap();
+        assert_eq!(roots.check(s(base.join("in/to-in2.scraft")), Access::Read), Ok(()));
+        assert!(roots.check(s(base.join("in2/leak.scraft")), Access::Read).unwrap_err().contains("outside the read roots"));
+        assert!(roots.check(s(base.join("in2/up/outside/secret.scraft")), Access::Read).unwrap_err().contains("outside the read roots"));
+        let _ = std::fs::remove_dir_all(base);
+    }
+
     #[cfg(windows)]
     #[test]
     fn windows_ignores_case_and_refuses_devices() {

docs/mcp.md

@@ -21,6 +21,8 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo
 
 ```sh
 soundcraft-cli mcp --automation-read-root /work/project --automation-write-root /work/project/out
+soundcraft-cli mcp --automation-read-root /work/project --automation-read-root /work/shared-samples \
+  --automation-write-root /work/project/out --automation-write-root /tmp/scratch
 ```
 
 ```json
@@ -31,20 +33,24 @@ soundcraft-cli mcp --automation-read-root /work/project --automation-write-root
 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`,
+- Every file a command reads must lie inside a **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`.
+  audio paths lead outside the read roots loads without that audio and lists it under `missing`.
-- Every file a command writes must lie inside the **write root**: `save_session`, `session.save`,
+- Every file a command writes must lie inside a **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.
+  the temp folder, which is outside the write roots unless you made it one, so it is refused.
-- Read and write access are separate. Give the same folder twice for both; a root left out grants
+- Each flag may be given more than once (#874): a path inside any of the read roots may be read,
+  one inside any of the write roots written, so one session can read a project folder and a shared
+  samples folder, or write to an output folder and a scratch folder, without widening a root to
+  their common parent. A folder given twice counts once.
+- Read and write access are separate. Give the same folder to both flags for both; a kind 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
@@ -55,12 +61,13 @@ works across the suite) keep the agent to the folders you give it:
   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 path rejected: outside the write root /work/project/out: /etc/x.wav` (with several,
+  `outside the write roots /work/project/out, /tmp/scratch: ...`) 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
+- The roots must be existing folders; a missing one stops the server with an error. Each flag takes
-  given once (`--flag <dir>` or `--flag=<dir>`), and a misspelled `--automation-...` flag is an
+  one folder per use (`--flag <dir>` or `--flag=<dir>`) and may be used again for more, and a
-  error rather than an unconfined server.
+  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