Merge pull request #1351 from DWAA1660/fix/1246-script-confirmed-flag

Scripts: a script can't confirm executing a file for itself (#1246)

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<Value> {
     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<Value> {
     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}));
     }

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]

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]