mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-13 23:17:30 +00:00
MCP: --automation-read-root / --automation-write-root confine the agent's file access (#500)
`effectcraft-cli mcp` had no way to keep an agent's file access to a folder; the flags were unknown options. Both now take a folder (repeatable, as in PhotoCraft): with either given, files are read only under a read or write root and written only under a write root (none: no writes). Defaults are unchanged. automation::confine::Confinement checks every engine command the backend runs (execute_command, command_run, open/save_project, each engine.batch step), in both headless and bridge mode, before it runs: the file parameters of the commands that read or write files must be absolute paths inside the roots (symbolic links resolved through the longest existing ancestor, `..` refused), as must any other absolute path in a command's parameters. Commands that write where they aren't told are checked against the target they would write: Save / Increment and Save the open project's file, renderQueue.render every queued output (renderQueue.list). Scripts, Execute File, Reveal in Finder, Open Recent by index, `$N` file-path references in a batch, render_frame/screenshot paths outside a write root and the bridge's UI-driving tools are refused while confined. docs/mcp.md describes it. Test: a confined headless server opens and saves inside its roots and is refused outside them, for a relative path, `..`, a batch step, a `$N` path, Save to a read-only root's project, the Render Queue's default output there, a symbolic link out of the write root, and scripts; the unconfined server still writes anywhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f
This commit is contained in:
7 files changed
+437
-18
No files matched your search
@@ -117,6 +117,9 @@ const USAGE: &str = "usage: effectcraft-cli <info|commands|exec|run|props|get|se
|
||||
(app.project, comps, layers, properties…); prints writeLn
|
||||
output and the result; errors exit 1 with file:line:col
|
||||
mcp [--autosave | --bridge PORT] MCP server (JSON-RPC over stdio); --autosave preserves unsaved headless work
|
||||
[--automation-read-root DIR] [--automation-write-root DIR]
|
||||
confine the agent's file access: reads under a read or write root,
|
||||
writes under a write root (each repeatable); scripts are refused
|
||||
options: --project F.ecproj | --demo | --empty --save | --save-as F --bridge PORT --json --gpu
|
||||
<comp>: id or name, '-' = active comp; <layer>: id, '#n' or name; <value>: JSON or bare string";
|
||||
|
||||
@@ -163,6 +166,9 @@ const VALUED: &[&str] = &[
|
||||
"--webm-codec",
|
||||
"--audio-bitrate",
|
||||
"--opus-app",
|
||||
// mcp
|
||||
"--automation-read-root",
|
||||
"--automation-write-root",
|
||||
];
|
||||
|
||||
/// Options without a value. Any other `--option` is a usage error.
|
||||
@@ -232,6 +238,10 @@ impl Args {
|
||||
fn flag(&self, k: &str) -> bool {
|
||||
self.opts.iter().any(|(o, _)| o == k)
|
||||
}
|
||||
/// Every value of a repeatable option, in order.
|
||||
fn opts(&self, k: &str) -> Vec<&str> {
|
||||
self.opts.iter().filter(|(o, _)| o == k).filter_map(|(_, v)| v.as_deref()).collect()
|
||||
}
|
||||
fn opt(&self, k: &str) -> Option<&str> {
|
||||
self.opts.iter().rev().find(|(o, _)| o == k).and_then(|(_, v)| v.as_deref())
|
||||
}
|
||||
@@ -595,7 +605,12 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
|
||||
if args.flag("--autosave") && args.opt("--bridge").is_some() {
|
||||
return usage_err("--autosave is for headless MCP; the bridged app owns its auto-saves");
|
||||
}
|
||||
let b = backend(args, false)?;
|
||||
let mut b = backend(args, false)?;
|
||||
let (read, write) = (args.opts("--automation-read-root"), args.opts("--automation-write-root"));
|
||||
if !read.is_empty() || !write.is_empty() {
|
||||
let roots = |r: Vec<&str>| r.into_iter().map(std::path::PathBuf::from).collect::<Vec<_>>();
|
||||
b.confine(effectcraft_automation::confine::Confinement::new(&roots(read), &roots(write))?);
|
||||
}
|
||||
let server = McpServer::new(b);
|
||||
let mut server = if args.flag("--autosave") {
|
||||
let root =
|
||||
|
||||
@@ -4,9 +4,16 @@ use effectcraft_engine::Session;
|
||||
use effectcraft_engine::time::Tick;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::confine::Confinement;
|
||||
use crate::{BridgeClient, Error, Result, base64, encode_png, recompress_png};
|
||||
|
||||
pub enum Backend {
|
||||
pub struct Backend {
|
||||
target: Target,
|
||||
/// File access confined to folders (`--automation-read-root` / `--automation-write-root`).
|
||||
confinement: Option<Confinement>,
|
||||
}
|
||||
|
||||
enum Target {
|
||||
/// An in-process session (no window): engine commands, inspection and rendering.
|
||||
Headless(Box<Session>),
|
||||
/// A running desktop app reached over its control channel: everything, plus the live UI.
|
||||
@@ -24,43 +31,66 @@ pub struct Frame {
|
||||
|
||||
impl Backend {
|
||||
pub fn headless(session: Session) -> Self {
|
||||
Backend::Headless(Box::new(session))
|
||||
Backend { target: Target::Headless(Box::new(session)), confinement: None }
|
||||
}
|
||||
|
||||
/// Bridge to the app's control channel at `addr` (`9877` or `127.0.0.1:9877`).
|
||||
pub fn bridge(addr: &str) -> Result<Self> {
|
||||
Ok(Backend::Bridge(BridgeClient::new(addr)?))
|
||||
Ok(Backend { target: Target::Bridge(BridgeClient::new(addr)?), confinement: None })
|
||||
}
|
||||
|
||||
/// Confine the files the commands run here may read and write ([`Confinement`]).
|
||||
pub fn confine(&mut self, confinement: Confinement) {
|
||||
self.confinement = Some(confinement);
|
||||
}
|
||||
|
||||
pub fn confinement(&self) -> Option<&Confinement> {
|
||||
self.confinement.as_ref()
|
||||
}
|
||||
|
||||
pub fn is_bridge(&self) -> bool {
|
||||
matches!(self, Backend::Bridge(_))
|
||||
matches!(self.target, Target::Bridge(_))
|
||||
}
|
||||
|
||||
pub fn session(&mut self) -> Option<&mut Session> {
|
||||
match self {
|
||||
Backend::Headless(s) => Some(s),
|
||||
Backend::Bridge(_) => None,
|
||||
match &mut self.target {
|
||||
Target::Headless(s) => Some(s),
|
||||
Target::Bridge(_) => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Run an engine command by id.
|
||||
/// Run an engine command by id (checked against the [`Confinement`], if any).
|
||||
pub fn exec(&mut self, id: &str, params: Value) -> Result<Value> {
|
||||
let params = if params.is_null() { json!({}) } else { params };
|
||||
match self {
|
||||
Backend::Headless(s) => {
|
||||
if let Some(c) = self.confinement.clone() {
|
||||
let lookup = match c.check_command(id, ¶ms)? {
|
||||
Some(crate::confine::Lookup::ProjectFile) => Some((crate::confine::Lookup::ProjectFile, self.run("project.summary", json!({}))?)),
|
||||
Some(crate::confine::Lookup::RenderOutputs) => Some((crate::confine::Lookup::RenderOutputs, self.run("renderQueue.list", json!({}))?)),
|
||||
None => None,
|
||||
};
|
||||
if let Some((lookup, found)) = lookup {
|
||||
c.check_lookup(&lookup, &found)?;
|
||||
}
|
||||
}
|
||||
self.run(id, params)
|
||||
}
|
||||
|
||||
fn run(&mut self, id: &str, params: Value) -> Result<Value> {
|
||||
match &mut self.target {
|
||||
Target::Headless(s) => {
|
||||
let r = s.execute_checked(id, params);
|
||||
s.drain_events();
|
||||
Ok(r?)
|
||||
}
|
||||
Backend::Bridge(b) => b.call("engine.execute", json!({"command": id, "params": params})),
|
||||
Target::Bridge(b) => b.call("engine.execute", json!({"command": id, "params": params})),
|
||||
}
|
||||
}
|
||||
|
||||
/// Call a raw control-channel method (bridge only).
|
||||
pub fn control(&mut self, method: &str, params: Value) -> Result<Value> {
|
||||
match self {
|
||||
Backend::Bridge(b) => b.call(method, if params.is_null() { json!({}) } else { params }),
|
||||
Backend::Headless(_) => Err(Error::BadArgs(NEED_BRIDGE.into())),
|
||||
match &mut self.target {
|
||||
Target::Bridge(b) => b.call(method, if params.is_null() { json!({}) } else { params }),
|
||||
Target::Headless(_) => Err(Error::BadArgs(NEED_BRIDGE.into())),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,15 +103,15 @@ impl Backend {
|
||||
/// [`Backend::render`]; `transparent` keeps the frame's alpha instead of compositing it over
|
||||
/// the comp's background colour.
|
||||
pub fn render_with(&mut self, comp: Option<&Value>, time: Option<f64>, max_side: u32, transparent: bool) -> Result<Frame> {
|
||||
match self {
|
||||
Backend::Headless(s) => {
|
||||
match &mut self.target {
|
||||
Target::Headless(s) => {
|
||||
let cid = s.resolve_comp(comp).map_err(|_| no_comp(comp))?;
|
||||
let t = time.map(Tick::from_seconds_f64).unwrap_or_else(|| s.time());
|
||||
let (w, h, rgba) = s.render_rgba8_alpha(cid, t, max_side, transparent)?;
|
||||
let png = encode_png(w, h, rgba, max_side)?;
|
||||
Ok(Frame { png, comp: json!(cid.0), time: t.seconds(), width: w, height: h })
|
||||
}
|
||||
Backend::Bridge(b) => {
|
||||
Target::Bridge(b) => {
|
||||
let mut p = json!({"max_side": max_side, "base64": true});
|
||||
if transparent {
|
||||
p["transparent"] = json!(true);
|
||||
|
||||
@@ -0,0 +1,276 @@
|
||||
//! File access confinement for automation (`effectcraft-cli mcp --automation-read-root DIR
|
||||
//! --automation-write-root DIR`, as in PhotoCraft): an agent may only read files under a read
|
||||
//! root (or a write root) and only write under a write root.
|
||||
//!
|
||||
//! Every engine command an agent runs ([`crate::Backend::exec`], also each step of an
|
||||
//! `engine.batch`) is checked before it runs: its file parameters must be absolute paths inside
|
||||
//! the roots (symbolic links resolved, no `..`), any other absolute path in its parameters too,
|
||||
//! 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. Commands that run code or open
|
||||
//! other applications (scripts, Execute File, Reveal in Finder…) are refused while confined.
|
||||
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
|
||||
use serde_json::Value;
|
||||
|
||||
use crate::{Error, Result};
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum Access {
|
||||
Read,
|
||||
Write,
|
||||
}
|
||||
|
||||
/// The folders an agent may read and write.
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct Confinement {
|
||||
read: Vec<PathBuf>,
|
||||
write: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
/// Commands that run code, act outside the project or open other applications.
|
||||
const REFUSED: &[&str] = &[
|
||||
"script.run",
|
||||
"file.runScript",
|
||||
"file.installScript",
|
||||
"file.installScriptUIPanel",
|
||||
"file.uninstallScript",
|
||||
"file.executeFile",
|
||||
"file.revealInFinder",
|
||||
"layer.openSource",
|
||||
"help.revealLogFile",
|
||||
"file.autoSave",
|
||||
];
|
||||
|
||||
/// Commands that read files named by these parameters.
|
||||
const READS: &[(&str, &[&str])] = &[
|
||||
("file.open", &["path"]),
|
||||
("file.openRecent", &["path"]),
|
||||
("file.import", &["path", "paths"]),
|
||||
("file.importMultiple", &["paths"]),
|
||||
("file.importRecent", &["path"]),
|
||||
("file.importLottie", &["path"]),
|
||||
("file.importTimeline", &["path"]),
|
||||
("file.replaceFootage", &["path"]),
|
||||
("file.relinkFootage", &["folder"]),
|
||||
("file.setProxy", &["path"]),
|
||||
("file.watchFolder", &["folder"]),
|
||||
("anim.applyPreset", &["path"]),
|
||||
("anim.applyRecentPreset", &["path"]),
|
||||
("keys.rpfCameraImport", &["path"]),
|
||||
("essential.importTemplate", &["path"]),
|
||||
("view.importGuides", &["path"]),
|
||||
("shortcuts.import", &["path"]),
|
||||
("effect.plugins.load", &["path", "folder"]),
|
||||
("mediaBrowser.go", &["path"]),
|
||||
("mediaBrowser.addFavorite", &["path"]),
|
||||
];
|
||||
|
||||
/// Commands that write files named by these parameters.
|
||||
const WRITES: &[(&str, &[&str])] = &[
|
||||
("file.save", &["path"]),
|
||||
("file.saveAs", &["path"]),
|
||||
("file.saveCopy", &["path"]),
|
||||
("file.saveCopyAsXml", &["path"]),
|
||||
("file.collectFiles", &["folder"]),
|
||||
("file.exportLottie", &["path"]),
|
||||
("comp.saveFrameAs", &["path"]),
|
||||
("comp.saveFrameAsPsd", &["path"]),
|
||||
("comp.saveFrameAsExr", &["path"]),
|
||||
("anim.savePreset", &["path"]),
|
||||
("view.exportGuides", &["path"]),
|
||||
("render.saveCurrentPreview", &["path"]),
|
||||
("render.preRender", &["output"]),
|
||||
("essential.exportTemplate", &["path"]),
|
||||
("shortcuts.export", &["path"]),
|
||||
("renderQueue.add", &["output", "path"]),
|
||||
];
|
||||
|
||||
/// Commands that write where they are not told: the project's file, the Render Queue's outputs.
|
||||
const IMPLICIT_WRITES: &[&str] = &["file.save", "file.incrementAndSave", "renderQueue.render", "render.preRender"];
|
||||
|
||||
/// What must be looked up before an implicitly writing command may run.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub enum Lookup {
|
||||
/// The open project's file (`project.summary` → `path`).
|
||||
ProjectFile,
|
||||
/// Every output of the queued Render Queue items (`renderQueue.list`).
|
||||
RenderOutputs,
|
||||
}
|
||||
|
||||
impl Confinement {
|
||||
/// Roots that must exist (each resolved to its real path). With no write root nothing may be
|
||||
/// written; write roots may also be read.
|
||||
pub fn new(read: &[PathBuf], write: &[PathBuf]) -> Result<Self> {
|
||||
let real = |p: &PathBuf| p.canonicalize().map_err(|e| Error::BadArgs(format!("automation root {}: {e}", p.display())));
|
||||
let read = read.iter().map(real).collect::<Result<Vec<_>>>()?;
|
||||
let write = write.iter().map(real).collect::<Result<Vec<_>>>()?;
|
||||
Ok(Self { read, write })
|
||||
}
|
||||
|
||||
fn roots(&self, access: Access) -> impl Iterator<Item = &PathBuf> {
|
||||
let read = if access == Access::Read { &self.read[..] } else { &[] };
|
||||
read.iter().chain(&self.write)
|
||||
}
|
||||
|
||||
/// `path` resolved, if it is inside the roots for `access`.
|
||||
pub fn check(&self, path: &str, access: Access) -> Result<PathBuf> {
|
||||
let denied = |why: &str| {
|
||||
let kind = if access == Access::Read { "read" } else { "write" };
|
||||
let roots: Vec<String> = self.roots(access).map(|r| r.display().to_string()).collect();
|
||||
let roots = if roots.is_empty() { "none".to_string() } else { roots.join(", ") };
|
||||
Error::BadArgs(format!("cannot {kind} `{path}`: {why} (automation {kind} roots: {roots})"))
|
||||
};
|
||||
let p = Path::new(path);
|
||||
if !p.is_absolute() {
|
||||
return Err(denied("give an absolute path"));
|
||||
}
|
||||
if p.components().any(|c| matches!(c, Component::ParentDir)) {
|
||||
return Err(denied("`..` is not allowed"));
|
||||
}
|
||||
let real = resolve(p).ok_or_else(|| denied("it can't be resolved"))?;
|
||||
if self.roots(access).any(|r| real.starts_with(r)) { Ok(real) } else { Err(denied("it is outside the automation roots")) }
|
||||
}
|
||||
|
||||
/// Check engine command `id` with `params` before it runs. Returns what must still be looked
|
||||
/// up and passed to [`Confinement::check_lookup`] for commands that write implicitly.
|
||||
pub fn check_command(&self, id: &str, params: &Value) -> Result<Option<Lookup>> {
|
||||
if REFUSED.contains(&id) {
|
||||
return Err(Error::BadArgs(format!("`{id}` is not available while file access is confined (--automation-read-root/--automation-write-root)")));
|
||||
}
|
||||
if id == "engine.batch" {
|
||||
for step in params.get("steps").and_then(Value::as_array).into_iter().flatten() {
|
||||
let Some(inner) = step.get("command").or_else(|| step.get("id")).and_then(Value::as_str) else { continue };
|
||||
let inner_params = step.get("params").cloned().unwrap_or(Value::Null);
|
||||
let named = keys_of(READS, inner).or_else(|| keys_of(WRITES, inner)).unwrap_or(&[]);
|
||||
// A step's `$N` reference is resolved only as the batch runs: never for a file.
|
||||
if named.iter().filter_map(|k| inner_params.get(*k)).any(has_reference) {
|
||||
return Err(Error::BadArgs(format!("`{inner}` in a batch: file paths can't be `$N` references while file access is confined")));
|
||||
}
|
||||
if self.check_command(inner, &inner_params)?.is_some() {
|
||||
return Err(Error::BadArgs(format!("`{inner}` writes to its project's or queue's files: run it on its own while file access is confined")));
|
||||
}
|
||||
}
|
||||
return Ok(None);
|
||||
}
|
||||
let (named, access) = match (keys_of(WRITES, id), keys_of(READS, id)) {
|
||||
(Some(k), _) => (k, Access::Write),
|
||||
(None, Some(k)) => (k, Access::Read),
|
||||
(None, None) if id.starts_with("renderQueue.") || id == "prefs.set" => (&[][..], Access::Write),
|
||||
(None, None) => (&[][..], Access::Read),
|
||||
};
|
||||
let mut given = false;
|
||||
for key in named {
|
||||
let Some(v) = params.get(*key).filter(|v| !v.is_null()) else { continue };
|
||||
given = true;
|
||||
let paths: Vec<&Value> = match v {
|
||||
Value::Array(a) => a.iter().collect(),
|
||||
v => vec![v],
|
||||
};
|
||||
for p in paths {
|
||||
// A Render Queue output may name a template instead of a file.
|
||||
match p.as_str() {
|
||||
Some(s) if id == "render.preRender" && !looks_like_path(s) => {}
|
||||
Some(s) => {
|
||||
self.check(s, access)?;
|
||||
}
|
||||
None => return Err(Error::BadArgs(format!("`{id}`: `{key}` must be a path"))),
|
||||
}
|
||||
}
|
||||
}
|
||||
// Any other absolute path the command is given.
|
||||
let mut other = vec![];
|
||||
absolute_strings(params, named, &mut other);
|
||||
for s in other {
|
||||
self.check(&s, access)?;
|
||||
}
|
||||
// An entry of a recent list by index is a path the agent didn't give.
|
||||
if matches!(id, "file.openRecent" | "file.importRecent" | "anim.applyRecentPreset") && !given {
|
||||
return Err(Error::BadArgs(format!("`{id}`: give `path` while file access is confined")));
|
||||
}
|
||||
if IMPLICIT_WRITES.contains(&id) && !(given && id == "file.save") {
|
||||
return Ok(Some(if id == "file.save" || id == "file.incrementAndSave" { Lookup::ProjectFile } else { Lookup::RenderOutputs }));
|
||||
}
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
/// Check what a [`Lookup`] found: `project.summary` or `renderQueue.list`'s result.
|
||||
pub fn check_lookup(&self, lookup: &Lookup, found: &Value) -> Result<()> {
|
||||
match lookup {
|
||||
Lookup::ProjectFile => match found.get("path").and_then(Value::as_str) {
|
||||
// Increment and Save writes beside it.
|
||||
Some(p) => self.check(p, Access::Write).map(drop),
|
||||
None => Err(Error::BadArgs("the project has no file yet: give `path` (Save As) inside an automation write root".into())),
|
||||
},
|
||||
Lookup::RenderOutputs => {
|
||||
let items = found.get("items").or(Some(found)).and_then(Value::as_array).cloned().unwrap_or_default();
|
||||
for it in items.iter().filter(|it| it.get("render").and_then(Value::as_bool) != Some(false)) {
|
||||
let extra = it.get("extraOutputs").and_then(Value::as_array).into_iter().flatten().map(|o| o.get("outputPath"));
|
||||
for p in std::iter::once(it.get("outputPath")).chain(extra).chain([it.get("logPath")]).flatten() {
|
||||
if let Some(p) = p.as_str() {
|
||||
self.check(p, Access::Write)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn keys_of(table: &[(&str, &'static [&'static str])], id: &str) -> Option<&'static [&'static str]> {
|
||||
table.iter().find(|(c, _)| *c == id).map(|(_, k)| *k)
|
||||
}
|
||||
|
||||
fn has_reference(v: &Value) -> bool {
|
||||
match v {
|
||||
Value::String(s) => s.starts_with('$'),
|
||||
Value::Array(a) => a.iter().any(has_reference),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// A file path rather than a template name: has a separator or an extension.
|
||||
fn looks_like_path(s: &str) -> bool {
|
||||
s.contains(['/', '\\']) || Path::new(s).extension().is_some()
|
||||
}
|
||||
|
||||
/// Strings in `v` (recursively, except under the top-level `skip` keys) that are absolute paths,
|
||||
/// `~` paths or `file:` URLs.
|
||||
fn absolute_strings(v: &Value, skip: &[&str], out: &mut Vec<String>) {
|
||||
fn walk(v: &Value, out: &mut Vec<String>, depth: usize) {
|
||||
if depth > 64 {
|
||||
return;
|
||||
}
|
||||
match v {
|
||||
Value::String(s) => {
|
||||
let t = s.trim();
|
||||
let drive = t.len() > 2 && t.as_bytes().get(1) == Some(&b':') && t.as_bytes().first().is_some_and(u8::is_ascii_alphabetic);
|
||||
if t.starts_with('/') || t.starts_with('\\') || t.starts_with('~') || drive || t.to_ascii_lowercase().starts_with("file:") {
|
||||
out.push(t.to_string());
|
||||
}
|
||||
}
|
||||
Value::Array(a) => a.iter().for_each(|x| walk(x, out, depth + 1)),
|
||||
Value::Object(o) => o.values().for_each(|x| walk(x, out, depth + 1)),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
match v {
|
||||
Value::Object(o) => o.iter().filter(|(k, _)| !skip.contains(&k.as_str())).for_each(|(_, x)| walk(x, out, 0)),
|
||||
v => walk(v, out, 0),
|
||||
}
|
||||
}
|
||||
|
||||
/// `p` with symbolic links resolved: its longest existing ancestor's real path plus the rest
|
||||
/// (which doesn't exist yet, so can't be a link).
|
||||
fn resolve(p: &Path) -> Option<PathBuf> {
|
||||
let mut rest = vec![];
|
||||
let mut at = p;
|
||||
loop {
|
||||
if let Ok(real) = at.canonicalize() {
|
||||
return Some(rest.iter().rev().fold(real, |acc: PathBuf, c| acc.join(c)));
|
||||
}
|
||||
rest.push(at.file_name()?.to_os_string());
|
||||
at = at.parent()?;
|
||||
}
|
||||
}
|
||||
@@ -19,6 +19,7 @@ mod autosave;
|
||||
pub mod backend;
|
||||
pub mod base64;
|
||||
pub mod bridge;
|
||||
pub mod confine;
|
||||
mod long_job;
|
||||
pub mod server;
|
||||
pub mod tools;
|
||||
|
||||
@@ -1181,3 +1181,74 @@ fn command_tools_accept_id_as_an_alias() {
|
||||
let (_, err) = call(&mut s, "execute_command", json!({}));
|
||||
assert!(err, "neither key is an error");
|
||||
}
|
||||
|
||||
/// #500: `--automation-read-root` / `--automation-write-root` confine the files an agent reads
|
||||
/// and writes: inside the roots works, outside (or relative, `..`, through a symbolic link, by
|
||||
/// a batch step, by Save to the project's own file, from a script) is refused.
|
||||
#[test]
|
||||
fn automation_roots_confine_file_access() {
|
||||
use crate::confine::Confinement;
|
||||
let base = std::env::temp_dir().join(format!("ec-mcp-confine-{}", 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 p = |d: &std::path::Path, f: &str| d.join(f).to_string_lossy().into_owned();
|
||||
// Unconfined (the default): anywhere.
|
||||
let mut s = server();
|
||||
call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}}));
|
||||
call_json(&mut s, "save_project", json!({"path": p(&outside, "free.ecproj")}));
|
||||
call_json(&mut s, "save_project", json!({"path": p(&read, "source.ecproj")}));
|
||||
|
||||
let mut s = server();
|
||||
s.backend().confine(Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap());
|
||||
let denied = |s: &mut McpServer, name: &str, args: Value| {
|
||||
let (c, err) = call(s, name, args.clone());
|
||||
assert!(err, "{name} {args} should be refused: {c:?}");
|
||||
c[0]["text"].as_str().unwrap_or_default().to_string()
|
||||
};
|
||||
// Reading: inside a read root, not outside.
|
||||
call_json(&mut s, "open_project", json!({"path": p(&read, "source.ecproj")}));
|
||||
let msg = denied(&mut s, "open_project", json!({"path": p(&outside, "free.ecproj")}));
|
||||
assert!(msg.contains("outside the automation roots"), "{msg}");
|
||||
// Writing: inside a write root only; not into a read root, not relative, not with `..`.
|
||||
call_json(&mut s, "save_project", json!({"path": p(&write, "saved.ecproj")}));
|
||||
assert!(write.join("saved.ecproj").is_file());
|
||||
denied(&mut s, "save_project", json!({"path": p(&read, "copy.ecproj")}));
|
||||
denied(&mut s, "save_project", json!({"path": "relative.ecproj"}));
|
||||
denied(&mut s, "save_project", json!({"path": p(&write, "../elsewhere/up.ecproj")}));
|
||||
denied(&mut s, "execute_command", json!({"command": "comp.saveFrameAs", "params": {"path": p(&outside, "f.png")}}));
|
||||
// Any other absolute path a command is given.
|
||||
denied(&mut s, "execute_command", json!({"command": "renderQueue.add", "params": {"output": p(&outside, "movie.mp4")}}));
|
||||
// A batch step.
|
||||
let steps = json!([{"command": "comp.new", "params": {"name": "B"}}, {"command": "file.saveAs", "params": {"path": p(&outside, "b.ecproj")}}]);
|
||||
denied(&mut s, "batch", json!({"steps": steps}));
|
||||
let steps = json!([{"command": "file.saveAs", "params": {"path": "$1.path"}}]);
|
||||
denied(&mut s, "batch", json!({"steps": steps}));
|
||||
// Save (and Increment and Save) write the open project's own file: it must be writable.
|
||||
call_json(&mut s, "open_project", json!({"path": p(&read, "source.ecproj")}));
|
||||
denied(&mut s, "save_project", json!({}));
|
||||
denied(&mut s, "execute_command", json!({"command": "file.incrementAndSave"}));
|
||||
// Rendering the queue writes its items' outputs (by default beside the project).
|
||||
call_json(&mut s, "execute_command", json!({"command": "renderQueue.add", "params": {"comp": "A"}}));
|
||||
let queue = call_json(&mut s, "execute_command", json!({"command": "renderQueue.list"}));
|
||||
assert!(queue["items"][0]["outputPath"].as_str().is_some_and(|o| o.starts_with(&*read.to_string_lossy())), "{queue}");
|
||||
let msg = denied(&mut s, "execute_command", json!({"command": "renderQueue.render"}));
|
||||
assert!(msg.contains("cannot write"), "{msg}");
|
||||
call_json(&mut s, "open_project", json!({"path": p(&write, "saved.ecproj")}));
|
||||
call_json(&mut s, "save_project", json!({}));
|
||||
// Through a symbolic link out of a root.
|
||||
#[cfg(unix)]
|
||||
{
|
||||
std::os::unix::fs::symlink(&outside, write.join("link")).unwrap();
|
||||
denied(&mut s, "save_project", json!({"path": p(&write.join("link"), "escaped.ecproj")}));
|
||||
assert!(!outside.join("escaped.ecproj").exists());
|
||||
}
|
||||
// Scripts can read and write anything: refused.
|
||||
denied(&mut s, "run_script", json!({"code": "1 + 1"}));
|
||||
denied(&mut s, "execute_command", json!({"command": "file.runScript", "params": {"path": p(&read, "x.jsx")}}));
|
||||
// Writes to the files the agent named: none outside.
|
||||
assert_eq!(std::fs::read_dir(&outside).unwrap().count(), 1, "only the unconfined save");
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
@@ -103,12 +103,19 @@ pub fn available(bridge: bool) -> impl Iterator<Item = &'static ToolDef> {
|
||||
TOOLS.iter().filter(move |t| bridge || !t.bridge_only)
|
||||
}
|
||||
|
||||
/// Tools refused while file access is confined: they drive the live app like a person (menus,
|
||||
/// shortcuts, Open Recent…) or call control-channel methods directly, past the command checks.
|
||||
const UNCONFINED_ONLY: &[&str] = &["control", "ui_click", "ui_drag", "ui_key", "ui_type", "ui_set", "script_ui"];
|
||||
|
||||
/// Run a tool by name.
|
||||
pub fn run(b: &mut Backend, name: &str, args: &Value) -> Result<Reply> {
|
||||
let t = find(name).ok_or_else(|| Error::BadArgs(format!("unknown tool `{name}`")))?;
|
||||
if t.bridge_only && !b.is_bridge() {
|
||||
return Err(Error::BadArgs(NEED_BRIDGE.into()));
|
||||
}
|
||||
if b.confinement().is_some() && UNCONFINED_ONLY.contains(&name) {
|
||||
return Err(Error::BadArgs(format!("`{name}` is not available while file access is confined (--automation-read-root/--automation-write-root)")));
|
||||
}
|
||||
let empty = json!({});
|
||||
(t.run)(b, if args.is_null() { &empty } else { args })
|
||||
}
|
||||
@@ -389,6 +396,9 @@ fn render_frame(b: &mut Backend, a: &Value) -> Result<Reply> {
|
||||
let f = b.render_with(get(a, "comp"), get(a, "time").and_then(Value::as_f64), max_side, transparent)?;
|
||||
let mut info = json!({"comp": f.comp, "time": f.time, "width": f.width, "height": f.height});
|
||||
if let Some(p) = get(a, "path").and_then(Value::as_str) {
|
||||
if let Some(c) = b.confinement() {
|
||||
c.check(p, crate::confine::Access::Write)?;
|
||||
}
|
||||
std::fs::write(p, &f.png).map_err(|e| Error::Other(format!("cannot write {p}: {e}")))?;
|
||||
info["path"] = json!(p);
|
||||
}
|
||||
@@ -482,6 +492,9 @@ fn batch_tool(b: &mut Backend, a: &Value) -> Result<Reply> {
|
||||
fn screenshot(b: &mut Backend, a: &Value) -> Result<Reply> {
|
||||
let max_side = get(a, "max_side").and_then(Value::as_u64).unwrap_or(1600) as u32;
|
||||
let keep = get(a, "path").and_then(Value::as_str).map(str::to_string);
|
||||
if let (Some(c), Some(p)) = (b.confinement(), &keep) {
|
||||
c.check(p, crate::confine::Access::Write)?;
|
||||
}
|
||||
let path =
|
||||
keep.clone().unwrap_or_else(|| std::env::temp_dir().join(format!("effectcraft-mcp-shot-{}.png", std::process::id())).to_string_lossy().into_owned());
|
||||
b.control("ui.screenshot", obj(&[("path", Some(&json!(path))), ("panel", get(a, "panel")), ("id", get(a, "id"))]))?;
|
||||
|
||||
+13
@@ -3,6 +3,19 @@
|
||||
Run `effectcraft-cli mcp` for a headless session, or `effectcraft-cli mcp --bridge 9877`
|
||||
to use the desktop app's control channel. Existing ports are unchanged.
|
||||
|
||||
## Confining file access
|
||||
|
||||
`--automation-read-root DIR` and `--automation-write-root DIR` (each repeatable, as in
|
||||
PhotoCraft) confine what the agent can reach: files are read only under a read or write root and
|
||||
written only under a write root; with no write root nothing is written. Every engine command
|
||||
(`execute_command`, `command_run`, each `batch` step) is checked before it runs: its file
|
||||
parameters must be absolute paths inside the roots (symbolic links resolved, no `..`), and so must
|
||||
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). 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.
|
||||
|
||||
The server provides the same core conventions as FilmCraft #28:
|
||||
|
||||
| Tool | Arguments | Result |
|
||||
|
||||
Reference in new issue
Block a user