mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-31 15:48:33 +00:00
automation: a confined agent can't queue a render output outside the roots (#2479)
`renderQueue.add` and `render.preRender` without an `output` take the default from where the project sits, which for a project that has never been saved is the home folder — outside every `--automation-read-root` / `--automation-write-root`. The item was queued anyway, and only `renderQueue.render` refused it, so an agent learned about it one command too late and with a queue it had to clean up. Both commands now look the project up first (`Lookup::DefaultOutput`) and are refused on an unsaved project, naming `output` as the way through. A project saved inside the roots keeps its old behaviour: the default goes beside its file and what it writes is checked when the queue is rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
bc4b48f67d
commit
a2c455091f
3 files changed
+73
-15
No files matched your search
@@ -4,7 +4,7 @@ use effectcraft_engine::Session;
|
||||
use effectcraft_engine::time::Tick;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::confine::Confinement;
|
||||
use crate::confine::{Confinement, Lookup};
|
||||
use crate::{BridgeClient, Error, Result, base64, encode_png, recompress_png};
|
||||
|
||||
pub struct Backend {
|
||||
@@ -63,12 +63,11 @@ impl Backend {
|
||||
pub fn exec(&mut self, id: &str, params: Value) -> Result<Value> {
|
||||
let params = if params.is_null() { json!({}) } else { params };
|
||||
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 {
|
||||
for lookup in c.check_command(id, ¶ms)? {
|
||||
let found = match lookup {
|
||||
Lookup::ProjectFile | Lookup::DefaultOutput => self.run("project.summary", json!({}))?,
|
||||
Lookup::RenderOutputs => self.run("renderQueue.list", json!({}))?,
|
||||
};
|
||||
c.check_lookup(&lookup, &found)?;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,8 +6,10 @@
|
||||
//! `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.
|
||||
//! 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, Execute File, Reveal in
|
||||
//! Finder…) are refused while confined.
|
||||
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
|
||||
@@ -89,11 +91,17 @@ const WRITES: &[(&str, &[&str])] = &[
|
||||
/// 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"];
|
||||
|
||||
/// Commands that queue a render, whose output defaults to the project's folder when not given.
|
||||
const QUEUES_AN_OUTPUT: &[&str] = &["renderQueue.add", "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,
|
||||
/// That the project has a file at all (`project.summary` → `path`), for a queued render whose
|
||||
/// output the agent did not name: that default is derived from where the project sits.
|
||||
DefaultOutput,
|
||||
/// Every output of the queued Render Queue items (`renderQueue.list`).
|
||||
RenderOutputs,
|
||||
}
|
||||
@@ -134,7 +142,7 @@ impl Confinement {
|
||||
|
||||
/// 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>> {
|
||||
pub fn check_command(&self, id: &str, params: &Value) -> Result<Vec<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)")));
|
||||
}
|
||||
@@ -147,11 +155,11 @@ impl Confinement {
|
||||
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() {
|
||||
if !self.check_command(inner, &inner_params)?.is_empty() {
|
||||
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);
|
||||
return Ok(vec![]);
|
||||
}
|
||||
let (named, access) = match (keys_of(WRITES, id), keys_of(READS, id)) {
|
||||
(Some(k), _) => (k, Access::Write),
|
||||
@@ -188,10 +196,19 @@ impl Confinement {
|
||||
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 }));
|
||||
let mut lookups = vec![];
|
||||
// An output the agent did not name is derived from where the project sits: the Default
|
||||
// Output Folder preference when that is set, beside the project file otherwise — and the
|
||||
// home folder when the project has never been saved, outside every root (#2479). Only
|
||||
// that last case is certainly outside, and reading the preference here would duplicate
|
||||
// the engine's precedence, so an unsaved project is told to name its output instead.
|
||||
if QUEUES_AN_OUTPUT.contains(&id) && !given {
|
||||
lookups.push(Lookup::DefaultOutput);
|
||||
}
|
||||
Ok(None)
|
||||
if IMPLICIT_WRITES.contains(&id) && !(given && id == "file.save") {
|
||||
lookups.push(if id == "file.save" || id == "file.incrementAndSave" { Lookup::ProjectFile } else { Lookup::RenderOutputs });
|
||||
}
|
||||
Ok(lookups)
|
||||
}
|
||||
|
||||
/// Check what a [`Lookup`] found: `project.summary` or `renderQueue.list`'s result.
|
||||
@@ -202,6 +219,14 @@ impl Confinement {
|
||||
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::DefaultOutput => match found.get("path").and_then(Value::as_str) {
|
||||
// Saved: the default goes beside the project file, inside the roots, and what it
|
||||
// writes is checked when the queue is rendered.
|
||||
Some(_) => Ok(()),
|
||||
None => Err(Error::BadArgs(
|
||||
"the project has no file yet, so this would queue its default output (the home folder) outside the automation write roots: give `output`, or save the project inside a write root first".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)) {
|
||||
|
||||
@@ -1380,3 +1380,37 @@ fn automation_roots_confine_file_access() {
|
||||
assert_eq!(std::fs::read_dir(&outside).unwrap().count(), 1, "only the unconfined save");
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
|
||||
/// #2479: a queued output the agent didn't name is derived from where the project sits, so on a
|
||||
/// project that has never been saved it lands in the home folder, outside every automation root.
|
||||
/// Queueing it there is refused, rather than leaving an item that can never be rendered.
|
||||
#[test]
|
||||
fn automation_roots_refuse_a_queued_output_from_an_unsaved_project() {
|
||||
use crate::confine::Confinement;
|
||||
let base = std::env::temp_dir().join(format!("ec-mcp-default-out-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
let write = base.join("out");
|
||||
std::fs::create_dir_all(&write).unwrap();
|
||||
let p = |f: &str| write.join(f).to_string_lossy().into_owned();
|
||||
let mut s = server();
|
||||
s.backend().confine(Confinement::new(&[], std::slice::from_ref(&write)).unwrap());
|
||||
call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}}));
|
||||
// Never saved: neither way of queueing a render may fall back to the home folder.
|
||||
for command in ["renderQueue.add", "render.preRender"] {
|
||||
let (c, err) = call(&mut s, "execute_command", json!({"command": command, "params": {"comp": "A"}}));
|
||||
assert!(err, "`{command}` without an output should be refused: {c:?}");
|
||||
let msg = c[0]["text"].as_str().unwrap_or_default();
|
||||
assert!(msg.contains("the project has no file yet"), "`{command}`: {msg}");
|
||||
}
|
||||
let queue = call_json(&mut s, "execute_command", json!({"command": "renderQueue.list"}));
|
||||
assert_eq!(queue["items"].as_array().map(Vec::len), Some(0), "nothing was queued: {queue}");
|
||||
// Naming an output inside a write root is how it is done.
|
||||
call_json(&mut s, "execute_command", json!({"command": "renderQueue.add", "params": {"comp": "A", "output": p("named.mp4")}}));
|
||||
// Once the project is saved there, the default output (beside it) is queued as before.
|
||||
call_json(&mut s, "save_project", json!({"path": p("proj.ecproj")}));
|
||||
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"}));
|
||||
let out = queue["items"][1]["outputPath"].as_str().unwrap_or_default();
|
||||
assert!(out.starts_with(&*write.to_string_lossy()), "{queue}");
|
||||
let _ = std::fs::remove_dir_all(&base);
|
||||
}
|
||||
Reference in new issue
Block a user