From 56e236257e8db759018103fdeea8e48611ebed45 Mon Sep 17 00:00:00 2001 From: adikshan11 Date: Sat, 31 Oct 2026 12:23:25 +0530 Subject: [PATCH] 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); +}