From 45b737455a90b69f179efde3f5b0c7d2579c89c7 Mon Sep 17 00:00:00 2001 From: adikshan11 Date: Sat, 31 Oct 2026 11:16:36 +0530 Subject: [PATCH 1/3] automation: a confined project can't show footage from outside the roots (#2131 F-11, "Indirect reads") The command checks look at the paths an agent gives, but a project it opens or imports from a read root names its own footage, which was read wherever it is when the project rendered: render_frame returned its pixels. Backend::confine now wraps a headless session's footage and services: ConfinedFootage reads frames, audio, aux channels, models and vector frames only for footage inside the read roots (problem() says why the rest shows nothing), and ConfinedServices checks read_file, exists, stamp, list_dir and is_dir the same way. Writes are left to the command checks. Test: automation_roots_confine_the_footage_a_project_names covers an absolute path, `../`, a linked file and a linked folder, each opened and imported. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + crates/automation/Cargo.toml | 1 + crates/automation/src/backend.rs | 10 ++- crates/automation/src/confine.rs | 141 ++++++++++++++++++++++++++++++- crates/automation/src/tests.rs | 77 +++++++++++++++++ docs/mcp.md | 4 +- 6 files changed, 228 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 896d08dc2..81b512d06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1432,6 +1432,7 @@ version = "0.18.0" dependencies = [ "effectcraft-engine", "effectcraft-host", + "effectcraft-model", "effectcraft-script", "image", "serde_json", 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..609213fa4 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -8,12 +8,22 @@ //! 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::collections::HashMap; use std::path::{Component, Path, PathBuf}; +use std::sync::atomic::AtomicBool; +use std::sync::{Arc, Mutex, PoisonError}; +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 +312,131 @@ 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, + /// Whether each footage path checked so far is inside: frames are read many times. + checked: Mutex>, +} + +impl ConfinedFootage { + pub fn new(inner: Arc, confinement: Confinement) -> Self { + Self { inner, confinement, checked: Mutex::default() } + } + + /// Whether every file of `f` (its file and each frame of a sequence) is inside the roots. + fn inside(&self, f: &Footage) -> bool { + let mut checked = self.checked.lock().unwrap_or_else(PoisonError::into_inner); + std::iter::once(&f.path) + .chain(&f.sequence) + .filter(|p| !p.is_empty()) + .all(|p| *checked.entry(p.clone()).or_insert_with(|| 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.checked.lock().unwrap_or_else(PoisonError::into_inner).remove(path); + 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..3d0eacc2b 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1730,3 +1730,80 @@ 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); +} 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. From 9ad970f9ff8af9c34b60c8208da9d0434fa88ee8 Mon Sep 17 00:00:00 2001 From: adikshan11 Date: Sat, 31 Oct 2026 11:16:36 +0530 Subject: [PATCH 2/3] Contributors: add adikshan11's name Co-Authored-By: Claude Opus 5.5 --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index e0915f09c..1dcf7e7df 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -117,3 +117,7 @@ real_name = "Ashok Ram" [people.dylanmtaylor] sync_github_name = true + +[people.adikshan11] +real_name = "Adithya Shankaran" +sync_github_name = true From 56e236257e8db759018103fdeea8e48611ebed45 Mon Sep 17 00:00:00 2001 From: adikshan11 Date: Sat, 31 Oct 2026 12:23:25 +0530 Subject: [PATCH 3/3] automation: check confined footage on every read, not once per path ConfinedFootage remembered the first answer for each footage path. A symbolic link inside a read root that pointed inside when first read could be pointed outside afterwards, and the next frame-cache miss read the outside file. Each read now runs the confinement check again. automation_roots_check_relinked_footage_again renders a link to a file inside the root, points it outside, empties the memory cache and renders again: the outside pixels came back before this change and don't now. Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/confine.rs | 16 ++++------- crates/automation/src/tests.rs | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/crates/automation/src/confine.rs b/crates/automation/src/confine.rs index 609213fa4..5f0a7a51a 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -14,10 +14,9 @@ //! Execute File, Reveal in Finder…) are refused while confined, as is granting scripts the file //! and network access the user withheld. -use std::collections::HashMap; use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; use std::sync::atomic::AtomicBool; -use std::sync::{Arc, Mutex, PoisonError}; use effectcraft_engine::Services; use effectcraft_engine::project::{Footage, ItemId}; @@ -319,22 +318,18 @@ impl Confinement { pub struct ConfinedFootage { inner: Arc, confinement: Confinement, - /// Whether each footage path checked so far is inside: frames are read many times. - checked: Mutex>, } impl ConfinedFootage { pub fn new(inner: Arc, confinement: Confinement) -> Self { - Self { inner, confinement, checked: Mutex::default() } + 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 { - let mut checked = self.checked.lock().unwrap_or_else(PoisonError::into_inner); - std::iter::once(&f.path) - .chain(&f.sequence) - .filter(|p| !p.is_empty()) - .all(|p| *checked.entry(p.clone()).or_insert_with(|| self.confinement.check(p, Access::Read).is_ok())) + std::iter::once(&f.path).chain(&f.sequence).filter(|p| !p.is_empty()).all(|p| self.confinement.check(p, Access::Read).is_ok()) } } @@ -374,7 +369,6 @@ impl FootageSource for ConfinedFootage { self.inner.purge() } fn forget(&self, path: &str) { - self.checked.lock().unwrap_or_else(PoisonError::into_inner).remove(path); self.inner.forget(path) } fn set_conform_folder(&self, folder: Option) { diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 3d0eacc2b..bb09e244b 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1807,3 +1807,50 @@ fn automation_roots_confine_the_footage_a_project_names() { } 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); +}