From 605b7c3a25f265778dd27f0be9c3ff7a558f86e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 18:50:57 +0000 Subject: [PATCH] Scripting: scripts can't grant themselves file and network access (#590) A running script reached `prefs.set` through `app.run` and could turn on Settings > Scripting & Expressions > Allow Scripts to Write Files and Access Network (or turn off Warn User When Executing Files), which the File, Socket and system.callSystem gates then read: the permission the user withheld was the script's to take. Commands a script issues now go through `Session::execute_for_script`, which marks the session as running for a script for the command and everything it runs (engine.batch, ScriptUI callbacks, scheduled tasks). While it is, `prefs.set` refuses any change that would leave the Application Scripting settings different (compared on the resulting settings, so dotted keys, whole-page objects and batches are all covered), and a script's `prefs.reset` keeps them as the user set them. Command scripts read from a file (File > Scripts > Run Script File... with .json/.jsonl) count as scripts too. The UI, CLI, control channel and MCP still change them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f --- crates/engine/src/commands/file_more.rs | 5 ++- crates/engine/src/commands/settings.rs | 12 ++++++++ crates/engine/src/lib.rs | 25 +++++++++++++++ crates/engine/src/prefs.rs | 19 ++++++++++++ crates/script/src/runtime.rs | 2 +- crates/script/src/tests.rs | 41 +++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/commands/file_more.rs b/crates/engine/src/commands/file_more.rs index aa9e1be9..9aecd8d6 100644 --- a/crates/engine/src/commands/file_more.rs +++ b/crates/engine/src/commands/file_more.rs @@ -465,6 +465,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { } _ => return Err(bad("file.runScript", "missing `path` (or inline `steps`)")), }; + // A command script read from a file is a script: it can't change what scripts may do. + let from_file = p.get("steps").is_none(); let mut results = vec![]; for (i, step) in steps.iter().enumerate() { let (id, params) = match (step.get("command").or(step.get("id")).and_then(Value::as_str), step.get("method").and_then(Value::as_str)) { @@ -478,7 +480,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { if id == "file.runScript" || id == "script.run" { return Err(bad("file.runScript", "scripts can't run scripts")); } - let r = s.execute(&id, params).map_err(|e| EngineError::Other(format!("step {} ({id}): {e}", i + 1)))?; + let r = if from_file { s.execute_for_script(&id, params) } else { s.execute(&id, params) }; + let r = r.map_err(|e| EngineError::Other(format!("step {} ({id}): {e}", i + 1)))?; results.push(r); } Ok(json!({"steps": results.len(), "results": results})) diff --git a/crates/engine/src/commands/settings.rs b/crates/engine/src/commands/settings.rs index 320a6c18..1a5789e2 100644 --- a/crates/engine/src/commands/settings.rs +++ b/crates/engine/src/commands/settings.rs @@ -9,6 +9,10 @@ use crate::{EngineError, Result, Session, cmd, query}; // ---------------------------------------------------------------- prefs +/// Why a script can't change Application Scripting settings. +const SCRIPT_PERMISSIONS: &str = + "Scripts can't change what scripts are allowed to do. Change it yourself in Settings ▸ Scripting & Expressions ▸ Application Scripting."; + fn prefs_get(s: &mut Session, p: &Value) -> Result { let key = str_p(p, "key").unwrap_or(""); s.prefs.get(key).ok_or_else(|| bad("prefs.get", format!("unknown setting `{key}`"))) @@ -30,6 +34,9 @@ fn prefs_set(s: &mut Session, p: &Value) -> Result { for (k, v) in &pairs { next.set(k, v.clone()).map_err(|e| bad("prefs.set", e))?; } + if s.running_for_script() && !next.scripting.same_permissions(&s.prefs.scripting) { + return Err(EngineError::Other(SCRIPT_PERMISSIONS.into())); + } s.prefs = next; s.prefs_changed(); s.save_prefs(); @@ -39,7 +46,12 @@ fn prefs_set(s: &mut Session, p: &Value) -> Result { fn prefs_reset(s: &mut Session, p: &Value) -> Result { let page = str_p(p, "page"); + let before = s.prefs.scripting.clone(); s.prefs.reset(page).map_err(|e| bad("prefs.reset", e))?; + // A script's reset leaves what scripts may do as the user set it. + if s.running_for_script() { + s.prefs.scripting.keep_permissions(&before); + } s.prefs_changed(); s.save_prefs(); s.toast(match page { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a9ee6b89..d09cbf2b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -472,8 +472,16 @@ pub struct Session { pub footage_stamps: std::collections::HashMap, /// The stamps being taken on a worker thread ([`footage_reload::start_scan`]). pub footage_scan: Option, + /// How many script-issued commands are running ([`Session::execute_for_script`]): while one + /// is, settings that guard what scripts may do can't change (see + /// [`prefs::Scripting::same_permissions`]). + pub script_calls: ScriptCalls, } +/// The count behind [`Session::running_for_script`]; only the engine changes it. +#[derive(Clone, Copy, Debug, Default)] +pub struct ScriptCalls(u32); + /// A thumbnail render ([`Session::thumbnail_job`]): (width, height, RGBA8). pub type ThumbnailJob = Box Option<(u32, u32, Vec)> + Send>; @@ -559,6 +567,7 @@ impl Default for Session { check_footage_on_open: false, footage_stamps: Default::default(), footage_scan: None, + script_calls: ScriptCalls::default(), } } } @@ -601,6 +610,22 @@ impl Session { self.execute(id, params) } + /// [`Session::execute`] for a command a script asked for (the JavaScript engine's + /// `app.run`, ScriptUI callbacks…). The command and everything it runs count as the script's: + /// it may not change the settings that limit scripts (Settings ▸ Scripting & Expressions ▸ + /// Application Scripting), which only the user can, from the UI, CLI or control channel. + pub fn execute_for_script(&mut self, id: &str, params: Value) -> Result { + self.script_calls.0 = self.script_calls.0.saturating_add(1); + let r = self.execute(id, params); + self.script_calls.0 = self.script_calls.0.saturating_sub(1); + r + } + + /// Whether the command running was issued by a script ([`Session::execute_for_script`]). + pub fn running_for_script(&self) -> bool { + self.script_calls.0 > 0 + } + pub fn is_enabled(&self, id: &str) -> bool { commands::find(id).is_some_and(|c| (c.enabled)(self).is_ok()) } diff --git a/crates/engine/src/prefs.rs b/crates/engine/src/prefs.rs index 0bfa04a5..0ed7145e 100644 --- a/crates/engine/src/prefs.rs +++ b/crates/engine/src/prefs.rs @@ -309,6 +309,25 @@ page!(Scripting { error_banner: bool = true, }); +impl Scripting { + /// Whether `other` grants scripts the same as this: Application Scripting (file and network + /// access, the warning before executing files, the debugger). Scripts can't change these + /// ([`crate::Session::execute_for_script`]): a script could otherwise give itself the + /// access the user withheld. + pub fn same_permissions(&self, other: &Scripting) -> bool { + self.allow_scripts_write_files == other.allow_scripts_write_files + && self.warn_executing_files == other.warn_executing_files + && self.enable_js_debugger == other.enable_js_debugger + } + + /// Take `from`'s Application Scripting settings (see [`Scripting::same_permissions`]). + pub fn keep_permissions(&mut self, from: &Scripting) { + self.allow_scripts_write_files = from.allow_scripts_write_files; + self.warn_executing_files = from.warn_executing_files; + self.enable_js_debugger = from.enable_js_debugger; + } +} + /// One editable label: name and `#rrggbb` colour. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct LabelPref { diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index 62479c14..712999ee 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -152,7 +152,7 @@ pub(crate) fn exec(s: &mut Session, id: &str, params: J) -> Result { if CREATES_LAYER.contains(&id) { s.state.selected_layers.clear(); } - let r = s.execute(id, params); + let r = s.execute_for_script(id, params); if let Some(c) = target && prev != Some(c) && s.state.active_comp == Some(c) diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 946db49d..b50a7bfb 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -362,6 +362,47 @@ fn security_gate_for_files() { let _ = std::fs::remove_dir_all(&dir); } +/// #590: a script can't give itself file and network access (or turn off the warning before +/// executing files) by changing Settings ▸ Scripting & Expressions; the user still can. +#[test] +fn scripts_cant_change_what_scripts_may_do() { + let mut s = session(); + let attempts = [ + "app.run('prefs.set', {key: 'scripting.allowScriptsWriteFiles', value: true})", + "app.run('prefs.set', {values: {'scripting.warnExecutingFiles': false}})", + "app.run('prefs.set', {key: 'scripting', value: {allowScriptsWriteFiles: true}})", + "app.run('engine.batch', {steps: [{command: 'prefs.set', params: {key: 'scripting.enableJsDebugger', value: true}}]})", + ]; + for code in attempts { + let o = run_code(&mut s, code, "grant.jsx"); + let e = o.error.unwrap_or_else(|| panic!("`{code}` was allowed")); + assert!(e.message.contains("Scripts can't change what scripts are allowed to do"), "{code}: {e:?}"); + let sc = &s.prefs.scripting; + assert!(!sc.allow_scripts_write_files && sc.warn_executing_files && !sc.enable_js_debugger, "{code}"); + } + assert!(run_code(&mut s, "new Socket()", "net.jsx").error.is_some()); + // Other settings, and the expression editor's, are still the script's to change. + ok(&mut s, "app.run('prefs.set', {values: {'general.undoLevels': 40, 'scripting.editorFontSize': 16}})"); + assert_eq!((s.prefs.general.undo_levels, s.prefs.scripting.editor_font_size), (40, 16)); + // The user grants access; a script's Reset Settings doesn't take it back or change it. + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + ok(&mut s, "app.run('prefs.reset', {})"); + assert!(s.prefs.scripting.allow_scripts_write_files); + assert_eq!(s.prefs.general.undo_levels, effectcraft_engine::prefs::Prefs::default().general.undo_levels); + // A command script file is a script too. + let dir = std::env::temp_dir().join(format!("ec-script-grant-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let f = dir.join("grant.json"); + std::fs::write(&f, r#"[{"command": "prefs.set", "params": {"key": "scripting.allowScriptsWriteFiles", "value": false}}]"#).unwrap(); + let e = s.execute("file.runScript", json!({"path": f.to_string_lossy()})).unwrap_err().to_string(); + assert!(e.contains("Scripts can't change"), "{e}"); + assert!(s.prefs.scripting.allow_scripts_write_files); + // The user (UI, CLI, control channel) can take it back. + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": false})).unwrap(); + assert!(!s.prefs.scripting.allow_scripts_write_files); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn engine_entry_points() { let mut s = session();