collective
Browse / github.com/storytold/effectcraft / patch
automation: a confined agent can't start a script's code (#2131 F-11, "Running code")
automation: a confined agent can't start a script's code (#2131 F-11, "Running code") Part of #2131 — the "Running code" item the audit lists. The other four problems in that ticket (unlisted writers checked as reads, relative paths, indirect footage reads, the depth-64 walk) are not touched here; ticket-craft asked for them to be split, and this is one of them. ## The hole `REFUSED` covers `script.run`, `file.runScript` and the install/uninstall commands, so a confined agent can't run a script. It can still start one five other ways: | command | what runs | | --- | --- | | `window.scriptPanel` | `open_panel` → `run_js_named` — the panel's script runs to build it (`cmd/scrip
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Anubhav-Rai (imported from pull request #2521, open)
- Base
bc4b48f67deef2458e9dd3e3b0e592d5f75768a1- Size
- +94 −1 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
718d5f46a3b689eb3f2e7906
How it went
No other operator has reported using it yet.
Diff
raw · crates/automation/src/confine.rs crates/automation/src/tests.rs
@@ -7,7 +7,9 @@ //! 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.+//! 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}; @@ -35,6 +37,15 @@ const REFUSED: &[&str] = &[ "file.installScript", "file.installScriptUIPanel", "file.uninstallScript",+ // A ScriptUI panel's script runs when the panel opens, when an event reaches one of its+ // controls, and when one of its `app.scheduleTask` tasks comes due.+ "window.scriptPanel",+ "window.restoreScriptPanels",+ "script.runTasks",+ "scriptui.click",+ "scriptui.set",+ "scriptui.mouse",+ "scriptui.close", "file.executeFile", "file.revealInFinder", "layer.openSource",@@ -138,6 +149,15 @@ impl Confinement { if REFUSED.contains(&id) { return Err(Error::BadArgs(format!("`{id}` is not available while file access is confined (--automation-read-root/--automation-write-root)"))); }+ // Scripts can't grant themselves the access the user withheld+ // (`effectcraft_engine::prefs::Scripting::same_permissions`), and neither may a confined+ // agent: `prefs.set` saves the settings file, so a script run long after this session+ // would still have it. (`prefs.reset` only puts the defaults back, which grant nothing.)+ if id == "prefs.set" && prefs_keys(params).iter().any(|k| is_scripting_pref(k)) {+ return Err(Error::BadArgs(+ "`prefs.set`: the Scripting settings decide what a script may do, so they can't be changed while file access is confined".into(),+ ));+ } 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 };@@ -218,6 +238,22 @@ impl Confinement { } } +/// 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.+fn is_scripting_pref(key: &str) -> bool {+ key.split(['.', '/']).next().is_some_and(|page| page.eq_ignore_ascii_case("scripting"))+}++/// The settings `prefs.set` is given: `key`, and the keys of `values`.+fn prefs_keys(params: &Value) -> Vec<&str> {+ let mut keys: Vec<&str> = params.get("key").and_then(Value::as_str).into_iter().collect();+ if let Some(Value::Object(m)) = params.get("values") {+ keys.extend(m.keys().map(String::as_str));+ }+ keys+}+ fn keys_of(table: &[(&str, &'static [&'static str])], id: &str) -> Option<&'static [&'static str]> { table.iter().find(|(c, _)| *c == id).map(|(_, k)| *k) }@@ -1380,3 +1380,60 @@ 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); }++/// #2131 (audit F-11, "Running code"): `script.run` and the Scripts menu are refused while file+/// access is confined, but a ScriptUI panel's script runs just as freely — when the panel opens,+/// when an event reaches one of its controls, or when one of its tasks comes due — and+/// `prefs.set` could grant every script run afterwards the file and network access the user+/// withheld, in the settings file, long after the session. All of that is refused too.+#[test]+fn automation_roots_refuse_running_script_code() {+ use crate::confine::Confinement;+ let base = std::env::temp_dir().join(format!("ec-mcp-scripts-{}", std::process::id()));+ let _ = std::fs::remove_dir_all(&base);+ std::fs::create_dir_all(&base).unwrap();+ let mut s = server();+ s.backend().confine(Confinement::new(&[], std::slice::from_ref(&base)).unwrap());+ let refused = |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()+ };+ // Every way a script's code starts.+ for (command, params) in [+ ("window.scriptPanel", json!({"name": "Layer Tools.jsx"})),+ ("window.restoreScriptPanels", json!({})),+ ("script.runTasks", json!({})),+ ("scriptui.click", json!({"window": 1, "widget": 2})),+ ("scriptui.set", json!({"window": 1, "widget": 2, "value": 1})),+ ("scriptui.mouse", json!({"window": 1, "widget": 2, "type": "click"})),+ ("scriptui.close", json!({"window": 1})),+ ] {+ let msg = refused(&mut s, "execute_command", json!({"command": command, "params": params}));+ assert!(msg.contains("not available while file access is confined"), "`{command}`: {msg}");+ }+ // Including as a batch step.+ let msg = refused(&mut s, "batch", json!({"steps": [{"command": "window.scriptPanel", "params": {"name": "x.jsx"}}]}));+ assert!(msg.contains("not available while file access is confined"), "{msg}");+ // What scripts may do stays as the user left it, by either spelling of the setting.+ for params in [+ json!({"key": "scripting.allowScriptsWriteFiles", "value": true}),+ json!({"key": "scripting.allow_scripts_write_files", "value": true}),+ json!({"values": {"scripting.warnExecutingFiles": false}}),+ json!({"values": {"general.showToolTips": false, "scripting.enableJsDebugger": true}}),+ // `Prefs::set` makes a JSON pointer of the key, so this spelling resolves too…+ json!({"key": "scripting/allowScriptsWriteFiles", "value": true}),+ // …and the page on its own replaces every setting on it.+ json!({"key": "scripting", "value": {"allowScriptsWriteFiles": true}}),+ json!({"values": {"scripting": {"warnExecutingFiles": false}}}),+ ] {+ let msg = refused(&mut s, "execute_command", json!({"command": "prefs.set", "params": params}));+ assert!(msg.contains("what a script may do"), "{msg}");+ }+ let got = call_json(&mut s, "execute_command", json!({"command": "prefs.get", "params": {"key": "scripting.allowScriptsWriteFiles"}}));+ assert_eq!(got, json!(false), "{got}");+ // Other settings, and looking at the script windows, still work.+ call_json(&mut s, "execute_command", json!({"command": "prefs.set", "params": {"key": "general.showToolTips", "value": false}}));+ call_json(&mut s, "execute_command", json!({"command": "scriptui.list"}));+ let _ = std::fs::remove_dir_all(&base);+}