diff --git a/Cargo.lock b/Cargo.lock index a7794ade0..79498923d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1432,6 +1432,7 @@ version = "0.19.0" dependencies = [ "effectcraft-engine", "effectcraft-host", + "effectcraft-model", "effectcraft-script", "image", "serde_json", diff --git a/contributors/people.toml b/contributors/people.toml index ead8852d1..d551ccebc 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -114,3 +114,7 @@ real_name = "Ashok Ram" [people.dylanmtaylor] sync_github_name = true + +[people.adikshan11] +real_name = "Adithya Shankaran" +sync_github_name = true diff --git a/crates/automation/Cargo.toml b/crates/automation/Cargo.toml index b14b16321..916cc3501 100644 --- a/crates/automation/Cargo.toml +++ b/crates/automation/Cargo.toml @@ -14,6 +14,7 @@ effectcraft-engine = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } image = { workspace = true } +effectcraft-model = { workspace = true } [dev-dependencies] effectcraft-script = { workspace = true } diff --git a/crates/automation/src/backend.rs b/crates/automation/src/backend.rs index e0116403a..69f75df2e 100644 --- a/crates/automation/src/backend.rs +++ b/crates/automation/src/backend.rs @@ -4,7 +4,7 @@ use effectcraft_engine::Session; use effectcraft_engine::time::Tick; use serde_json::{Value, json}; -use crate::confine::{Confinement, Lookup}; +use crate::confine::{ConfinedFootage, ConfinedServices, Confinement, Lookup}; use crate::{BridgeClient, Error, Result, base64, encode_png, recompress_png}; pub struct Backend { @@ -39,8 +39,14 @@ impl Backend { Ok(Backend { target: Target::Bridge(BridgeClient::new(addr)?), confinement: None }) } - /// Confine the files the commands run here may read and write ([`Confinement`]). + /// Confine the files the commands run here may read and write ([`Confinement`]). In-process, + /// the session's own footage and file reads are confined too, so a project from a read root + /// can't show footage from outside it. pub fn confine(&mut self, confinement: Confinement) { + if let Target::Headless(s) = &mut self.target { + s.footage = std::sync::Arc::new(ConfinedFootage::new(s.footage.clone(), confinement.clone())); + s.services = std::sync::Arc::new(ConfinedServices::new(s.services.clone(), confinement.clone())); + } self.confinement = Some(confinement); } diff --git a/crates/automation/src/confine.rs b/crates/automation/src/confine.rs index afdb6a69d..5f0a7a51a 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -8,12 +8,21 @@ //! and commands that write to a file they are not given (Save, Increment and Save, rendering the //! Render Queue) are checked against where they would write. Queueing a render without naming //! its output needs a saved project, because the default output of an unsaved one is the home -//! folder. Commands that run code or open other applications (scripts, ScriptUI panels and the -//! events that reach them, Execute File, Reveal in Finder…) are refused while confined, as is -//! granting scripts the file and network access the user withheld. +//! folder. In a headless session the footage and other files a project or an imported file names +//! are read only inside the roots ([`ConfinedFootage`], [`ConfinedServices`]). Commands that run +//! code or open other applications (scripts, ScriptUI panels and the events that reach them, +//! Execute File, Reveal in Finder…) are refused while confined, as is granting scripts the file +//! and network access the user withheld. use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use effectcraft_engine::Services; +use effectcraft_engine::project::{Footage, ItemId}; +use effectcraft_engine::raster::{AuxChannels, Image}; +use effectcraft_engine::render::{FloatCurve, FootageSource}; +use effectcraft_engine::time::Tick; use serde_json::Value; use crate::{Error, Result}; @@ -302,6 +311,126 @@ impl Confinement { } } +/// The session's footage, read only inside the read roots (#2131 F-11, "Indirect reads"). The +/// commands an agent runs are checked by the paths they are given, but a project opened or +/// imported from a read root names its own footage, which is read when it renders: a file +/// outside the roots shows as missing footage instead. +pub struct ConfinedFootage { + inner: Arc, + confinement: Confinement, +} + +impl ConfinedFootage { + pub fn new(inner: Arc, confinement: Confinement) -> Self { + Self { inner, confinement } + } + + /// Whether every file of `f` (its file and each frame of a sequence) is inside the roots. + /// Checked on every read, not remembered: a symbolic link inside a root can be pointed + /// elsewhere after it was first read. + fn inside(&self, f: &Footage) -> bool { + std::iter::once(&f.path).chain(&f.sequence).filter(|p| !p.is_empty()).all(|p| self.confinement.check(p, Access::Read).is_ok()) + } +} + +impl FootageSource for ConfinedFootage { + fn frame(&self, item: ItemId, footage: &Footage, t: Tick) -> Option> { + self.inside(footage).then(|| self.inner.frame(item, footage, t)).flatten() + } + fn frame_at_size(&self, item: ItemId, footage: &Footage, t: Tick, width: u32, height: u32) -> Option> { + self.inside(footage).then(|| self.inner.frame_at_size(item, footage, t, width, height)).flatten() + } + fn frame_with(&self, item: ItemId, footage: &Footage, t: Tick, size: Option<(u32, u32)>, float: FloatCurve) -> Option> { + self.inside(footage).then(|| self.inner.frame_with(item, footage, t, size, float)).flatten() + } + fn audio(&self, item: ItemId, footage: &Footage, t: Tick, frames: usize, rate: u32) -> Option> { + self.inside(footage).then(|| self.inner.audio(item, footage, t, frames, rate)).flatten() + } + fn aux(&self, item: ItemId, footage: &Footage, t: Tick) -> Option> { + self.inside(footage).then(|| self.inner.aux(item, footage, t)).flatten() + } + fn model(&self, item: ItemId, footage: &Footage) -> Option> { + self.inside(footage).then(|| self.inner.model(item, footage)).flatten() + } + fn vector_frame(&self, item: ItemId, footage: &Footage, scale: f64) -> Option> { + self.inside(footage).then(|| self.inner.vector_frame(item, footage, scale)).flatten() + } + /// Says why footage outside the roots shows nothing, so the agent can act on it. + fn problem(&self, footage: &Footage) -> Option { + if !self.inside(footage) { + return Some(format!("{} is outside the automation read roots, so it is not read", footage.path)); + } + self.inner.problem(footage) + } + fn set_cache_budget(&self, bytes: usize) { + self.inner.set_cache_budget(bytes) + } + fn purge(&self) { + self.inner.purge() + } + fn forget(&self, path: &str) { + self.inner.forget(path) + } + fn set_conform_folder(&self, folder: Option) { + self.inner.set_conform_folder(folder) + } + fn cache_budget(&self) -> Option { + self.inner.cache_budget() + } + fn cancellable(&self, cancel: &Arc, f: &mut dyn FnMut()) { + self.inner.cancellable(cancel, f) + } + fn called_off(&self) -> bool { + self.inner.called_off() + } +} + +/// The session's file reads, only inside the read roots: what a project or an imported file +/// pulls in by itself (an SVG's or Lottie's images, a timeline's media) is checked like a path +/// the agent gives. Writes are left to the command checks. +pub struct ConfinedServices { + inner: Arc, + confinement: Confinement, +} + +impl ConfinedServices { + pub fn new(inner: Arc, confinement: Confinement) -> Self { + Self { inner, confinement } + } + + fn inside(&self, path: &str) -> bool { + self.confinement.check(path, Access::Read).is_ok() + } +} + +impl Services for ConfinedServices { + fn read_file(&self, path: &str) -> std::io::Result> { + self.confinement.check(path, Access::Read).map_err(|e| std::io::Error::new(std::io::ErrorKind::PermissionDenied, e.to_string()))?; + self.inner.read_file(path) + } + fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + self.inner.write_file(path, data) + } + fn exists(&self, path: &str) -> bool { + self.inside(path) && self.inner.exists(path) + } + fn stamp(&self, path: &str) -> Option<(u64, u128)> { + if self.inside(path) { self.inner.stamp(path) } else { None } + } + fn store_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + self.inner.store_file(path, data) + } + fn list_dir(&self, dir: &str) -> Vec { + if self.inside(dir) { self.inner.list_dir(dir) } else { vec![] } + } + fn is_dir(&self, path: &str) -> bool { + self.inside(path) && self.inner.is_dir(path) + } + fn full_path(&self, path: &str) -> Option { + self.inner.full_path(path) + } +} + /// Whether a `prefs.set` key reaches the Scripting page. `Prefs::set` turns the key into a JSON /// pointer (`key.replace('.', "/")`), so `scripting/allowScriptsWriteFiles` resolves as well as /// the dotted spelling, and the page on its own replaces every setting on it. diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 497e3e303..bb09e244b 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1730,3 +1730,127 @@ fn bridged_render_frame_asks_the_app_before_it_writes() { assert_eq!(&std::fs::read(&free).unwrap()[..8], b"\x89PNG\r\n\x1a\n"); let _ = std::fs::remove_dir_all(&dir); } + +/// #2131 F-11 ("Indirect reads"): a project opened or imported from inside a read root names its +/// footage, and those files are read when it renders. Footage outside the roots (an absolute +/// path, `../`, a symbolic link to a file or a folder out of the root) must not reach the agent: +/// its layer renders as missing footage, while footage inside the roots still renders. +#[test] +fn automation_roots_confine_the_footage_a_project_names() { + use crate::confine::Confinement; + let base = std::env::temp_dir().join(format!("ec-mcp-footage-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (read, write, outside) = (base.join("in"), base.join("out"), base.join("elsewhere")); + for d in [&read, &write, &outside] { + std::fs::create_dir_all(d).unwrap(); + } + let red = image::RgbaImage::from_pixel(8, 8, image::Rgba([255, 0, 0, 255])); + red.save(outside.join("secret.png")).unwrap(); + red.save(read.join("own.png")).unwrap(); + let p = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().into_owned(); + // A project in the read root showing `footage`, saved unconfined. + let project = |name: &str, footage: &str| { + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "C", "width": 8, "height": 8}})); + let imported = call_json(&mut s, "execute_command", json!({"command": "file.import", "params": {"paths": [footage]}})); + let item = imported["items"][0].as_u64().or_else(|| imported["item"].as_u64()).unwrap_or_else(|| panic!("{imported}")); + call_json(&mut s, "execute_command", json!({"command": "layer.addItem", "params": {"comp": "C", "item": item}})); + let path = p(&read, name); + call_json(&mut s, "save_project", json!({"path": path})); + path + }; + // The middle pixel of comp C: red when the footage was read. + let shows_red = |s: &mut McpServer| { + let (c, err) = call(s, "render_frame", json!({"comp": "C"})); + assert!(!err, "{c:?}"); + let png = base64::decode(c[0]["data"].as_str().unwrap()).unwrap(); + let px = image::load_from_memory(&png).unwrap().to_rgba8().get_pixel(4, 4).0; + px[0] > 200 && px[3] > 200 + }; + let mut projects = vec![("absolute", project("absolute.ecproj", &p(&outside, "secret.png")))]; + #[cfg(unix)] + { + std::os::unix::fs::symlink(outside.join("secret.png"), read.join("link.png")).unwrap(); + std::os::unix::fs::symlink(&outside, read.join("dir")).unwrap(); + projects.push(("linked file", project("linked-file.ecproj", &p(&read, "link.png")))); + projects.push(("linked folder", project("linked-folder.ecproj", &p(&read.join("dir"), "secret.png")))); + } + // Relative to the project: `../elsewhere/secret.png`, written into a copy of a project. + let own = project("own.ecproj", &p(&read, "own.png")); + let text = std::fs::read_to_string(&own).unwrap(); + assert!(text.contains(&p(&read, "own.png")) || text.contains("own.png"), "{text}"); + let relative = text.replace(&p(&read, "own.png"), "../elsewhere/secret.png").replace("\"own.png\"", "\"../elsewhere/secret.png\""); + std::fs::write(read.join("relative.ecproj"), relative).unwrap(); + projects.push(("relative", p(&read, "relative.ecproj"))); + + let confined = || { + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + s.backend().confine(Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap()); + s + }; + // Footage inside the read root renders. + let mut s = confined(); + call_json(&mut s, "open_project", json!({"path": own})); + assert!(shows_red(&mut s), "footage inside the read root"); + for (how, path) in &projects { + // Unconfined, the project shows the outside file. + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + call_json(&mut s, "open_project", json!({"path": path})); + assert!(shows_red(&mut s), "{how}: unconfined shows the footage"); + // Confined: opened, or imported into the open project. + let mut s = confined(); + call_json(&mut s, "open_project", json!({"path": path})); + assert!(!shows_red(&mut s), "{how}: opened, the footage outside the roots was read"); + let mut s = confined(); + call_json(&mut s, "execute_command", json!({"command": "file.importProject", "params": {"path": path}})); + assert!(!shows_red(&mut s), "{how}: imported, the footage outside the roots was read"); + } + let _ = std::fs::remove_dir_all(&base); +} + +/// A symbolic link inside the read root, read once while it pointed inside, is checked again +/// on every read: pointed outside afterwards, its footage must not come back once the frame +/// cache is emptied. +#[cfg(unix)] +#[test] +fn automation_roots_check_relinked_footage_again() { + use crate::confine::Confinement; + let base = std::env::temp_dir().join(format!("ec-mcp-relink-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (read, write, outside) = (base.join("in"), base.join("out"), base.join("elsewhere")); + for d in [&read, &write, &outside] { + std::fs::create_dir_all(d).unwrap(); + } + image::RgbaImage::from_pixel(8, 8, image::Rgba([0, 0, 255, 255])).save(read.join("own.png")).unwrap(); + image::RgbaImage::from_pixel(8, 8, image::Rgba([255, 0, 0, 255])).save(outside.join("secret.png")).unwrap(); + let link = read.join("link.png"); + std::os::unix::fs::symlink(read.join("own.png"), &link).unwrap(); + let link_path = link.to_string_lossy().into_owned(); + let project = read.join("relink.ecproj").to_string_lossy().into_owned(); + { + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "C", "width": 8, "height": 8}})); + let imported = call_json(&mut s, "execute_command", json!({"command": "file.import", "params": {"paths": [link_path]}})); + let item = imported["items"][0].as_u64().or_else(|| imported["item"].as_u64()).unwrap_or_else(|| panic!("{imported}")); + call_json(&mut s, "execute_command", json!({"command": "layer.addItem", "params": {"comp": "C", "item": item}})); + call_json(&mut s, "save_project", json!({"path": project})); + } + // The middle pixel of comp C. + let pixel = |s: &mut McpServer| { + let (c, err) = call(s, "render_frame", json!({"comp": "C"})); + assert!(!err, "{c:?}"); + let png = base64::decode(c[0]["data"].as_str().unwrap()).unwrap(); + image::load_from_memory(&png).unwrap().to_rgba8().get_pixel(4, 4).0 + }; + let mut s = McpServer::new(Backend::headless(effectcraft_host::session())); + s.backend().confine(Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap()); + call_json(&mut s, "open_project", json!({"path": project})); + let px = pixel(&mut s); + assert!(px[2] > 200 && px[0] < 50, "the link pointing inside renders: {px:?}"); + std::fs::remove_file(&link).unwrap(); + std::os::unix::fs::symlink(outside.join("secret.png"), &link).unwrap(); + call_json(&mut s, "execute_command", json!({"command": "edit.purge", "params": {"what": "memory"}})); + let px = pixel(&mut s); + assert!(!(px[0] > 200 && px[3] > 200), "the link pointed outside afterwards was read: {px:?}"); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/docs/mcp.md b/docs/mcp.md index fb02e79b6..ccca7d2ba 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -13,7 +13,9 @@ parameters must be absolute paths inside the roots (symbolic links resolved, no any other absolute path in its parameters. Save, Increment and Save and rendering the Render Queue are checked against the files they write (the open project's, the queued outputs); `renderQueue.add`, `render.preRender` and `file.createProxy` without an output file need a saved -project, since an unsaved one's default output would go to the home folder (#2479). Scripts +project, since an unsaved one's default output would go to the home folder (#2479). In a headless +session the footage a project names, and what an imported file pulls in, is read only inside the +roots too: footage outside them shows as missing (#2131). Scripts (`run_script`, `file.runScript`…), Execute File, Reveal in Finder, `$N` references to file paths in a batch, and the bridge's UI-driving tools (`ui_click`, `ui_key`, `ui_type`, `ui_drag`, `ui_set`, `script_ui`, `control`) are refused while confined. Without the flags nothing changes.