diff --git a/crates/engine/src/commands/app_more.rs b/crates/engine/src/commands/app_more.rs index 020880e8..1bb94952 100644 --- a/crates/engine/src/commands/app_more.rs +++ b/crates/engine/src/commands/app_more.rs @@ -245,7 +245,8 @@ fn has_footage(s: &Session) -> std::result::Result<(), String> { /// `File.execute()` from scripts (and agents): open a file with its default application. /// Needs Allow Scripts to Write Files and Access Network; with Warn User When Executing Files on, -/// the frontend asks first (`confirmed: true` skips the question). +/// the frontend asks first. `confirmed: true` is the frontend's answer to that question: a +/// script can't give it for itself (#1246), so a script's request always asks. fn execute_file(s: &mut Session, p: &Value) -> Result { let path = str_p(p, "path").ok_or_else(|| bad("file.executeFile", "missing `path`"))?; if !s.prefs.scripting.allow_scripts_write_files { @@ -257,7 +258,8 @@ fn execute_file(s: &mut Session, p: &Value) -> Result { if !std::path::Path::new(path).exists() { return Err(bad("file.executeFile", format!("no file {path}"))); } - if s.prefs.scripting.warn_executing_files && !b_p(p, "confirmed").unwrap_or(false) { + let confirmed = b_p(p, "confirmed").unwrap_or(false) && !s.running_for_script(); + if s.prefs.scripting.warn_executing_files && !confirmed { s.events.push(Event::Frontend { command: "app.confirmExecute".into(), params: json!({"path": path}) }); return Ok(json!({"needsConfirmation": path})); } diff --git a/crates/engine/src/tests_m145.rs b/crates/engine/src/tests_m145.rs index 93125af1..fe0a1693 100644 --- a/crates/engine/src/tests_m145.rs +++ b/crates/engine/src/tests_m145.rs @@ -371,6 +371,34 @@ fn execute_file_needs_the_gate_and_asks_first() { assert!(s.execute("file.executeFile", p).unwrap().get("opened").is_some()); } +#[test] +fn a_script_cannot_confirm_executing_a_file_for_itself() { + // Warn User When Executing Files: only the user answers the question (#1246). + let mut s = session(); + let f = tmp("script-run.txt"); + std::fs::write(&f, "x").unwrap(); + let path = f.to_string_lossy().to_string(); + let opened = |s: &mut Session| s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(_))); + // Without the gate a script can't run files at all, confirmed or not. + assert!(s.execute_for_script("file.executeFile", json!({"path": path, "confirmed": true})).is_err()); + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + s.drain_events(); + // A script asserting `confirmed` still gets the question, directly or in a batch. + let r = s.execute_for_script("file.executeFile", json!({"path": path, "confirmed": true})).unwrap(); + assert!(r.get("needsConfirmation").is_some(), "{r}"); + assert!(!opened(&mut s)); + let steps = json!({"steps": [{"command": "file.executeFile", "params": {"path": path, "confirmed": true}}]}); + let r = s.execute_for_script("engine.batch", steps).unwrap(); + assert!(r.to_string().contains("needsConfirmation"), "{r}"); + assert!(!opened(&mut s)); + // The user's answer from the frontend's question still opens it. + assert!(s.execute("file.executeFile", json!({"path": path, "confirmed": true})).unwrap().get("opened").is_some()); + assert!(opened(&mut s)); + // With the warning off a script's request opens the file without a question, as before. + s.execute("prefs.set", json!({"key": "scripting.warnExecutingFiles", "value": false})).unwrap(); + assert!(s.execute_for_script("file.executeFile", json!({"path": path})).unwrap().get("opened").is_some()); +} + // ---------------------------------------------------------------- Edit ▸ History, recent lists #[test] diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 0b8d3a35..109b0974 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -442,6 +442,32 @@ fn save_frame_to_png_writes_behind_the_file_gate() { let _ = std::fs::remove_dir_all(&dir); } +/// #1246: with Warn User When Executing Files on, a script that asserts `confirmed` itself still +/// gets the question; only the user's answer opens the file. +#[test] +fn a_script_cannot_confirm_executing_a_file_for_itself() { + let dir = std::env::temp_dir().join(format!("ec-script-exec-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let f = dir.join("run.txt"); + std::fs::write(&f, "x").unwrap(); + let q = serde_json::to_string(&f.to_string_lossy()).unwrap(); + let mut s = session(); + // The user allows file access; the warning stays on (the default). + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + s.drain_events(); + let opened = |s: &mut Session| s.drain_events().iter().any(|e| matches!(e, effectcraft_engine::Event::OpenUrl(_))); + for code in [ + format!("JSON.stringify(app.run('file.executeFile', {{path: {q}, confirmed: true}}))"), + format!("JSON.stringify(app.run('engine.batch', {{steps: [{{command: 'file.executeFile', params: {{path: {q}, confirmed: true}}}}]}}))"), + format!("new File({q}).execute()"), + ] { + let o = ok(&mut s, &code); + assert!(!o.result.to_string().contains("opened") || o.result == json!(false), "{code}: {:?}", o.result); + assert!(!opened(&mut s), "{code} opened the file without asking"); + } + 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]