Merge pull request #7705 from dotCooCoo/fix/installed-script-runs-checked-text

Scripts: File › Scripts runs the text its check read, with includes from the checked folder

crates/engine/src/file_access.rs

@@ -399,6 +399,12 @@ impl ScriptScope {
         Self { roots: std::fs::canonicalize(dir).ok().into_iter().collect() }
     }
 
+    /// The scope of a script run from a file in the folder whose real path, with its links already
+    /// followed, is `real`: that folder, which is not looked up again.
+    pub(crate) fn of_real_folder(real: PathBuf) -> Self {
+        Self { roots: vec![real] }
+    }
+
     /// Is `path` refused by its name alone: a `..` component, a NUL, a Windows device name or
     /// stream, or a computer on the network?
     fn refused_by_name(path: &Path) -> bool {

crates/engine/src/script/include.rs

@@ -295,12 +295,14 @@ fn display(path: &Path) -> Arc<str> {
 
 impl Files {
     /// The files of a run of the script at `path` (`None` for a script given as text), with the
-    /// `JSINCLUDE` folders `jsinclude`.
-    pub(crate) fn new(path: Option<&str>, jsinclude: Vec<PathBuf>) -> Self {
-        let real = path.and_then(|p| std::fs::canonicalize(p).ok());
+    /// `JSINCLUDE` folders `jsinclude`. `checked` is the script's real path when the read of its text
+    /// found it, which the run's folder then comes from; without it, `path` is looked up.
+    pub(crate) fn new(path: Option<&str>, checked: Option<PathBuf>, jsinclude: Vec<PathBuf>) -> Self {
+        let scope_of_checked = checked.as_deref().and_then(Path::parent).map(|d| ScriptScope::of_real_folder(d.to_path_buf()));
+        let real = checked.or_else(|| path.and_then(|p| std::fs::canonicalize(p).ok()));
         let dir = real.as_deref().and_then(folder_of);
         Self {
-            scope: dir.as_deref().map(ScriptScope::of_folder).unwrap_or_default(),
+            scope: scope_of_checked.or_else(|| dir.as_deref().map(ScriptScope::of_folder)).unwrap_or_default(),
             jsinclude,
             include_path: Vec::new(),
             files: 0,

crates/engine/src/script/mod.rs

@@ -163,6 +163,9 @@ pub(crate) struct Request {
     pub name_given: bool,
     /// The file the script was read from, which its included files are read beside.
     pub file: Option<String>,
+    /// The real path of `file` as the read of `source` found it, which the included files are then
+    /// read beside instead of looking `file` up again.
+    pub checked_real: Option<std::path::PathBuf>,
     pub source: String,
     pub args: Map<String, Value>,
     pub fuel: u64,
@@ -1814,7 +1817,7 @@ impl Session {
         self.script_high_water.retain(|uid, _| open.contains(uid));
         self.script_stop.clear();
         self.script_report = None;
-        let mut files = include::Files::new(request.file.as_deref(), include::Files::jsinclude_env());
+        let mut files = include::Files::new(request.file.as_deref(), request.checked_real, include::Files::jsinclude_env());
         let expanded = files.expand_script(&request.source);
         let name = match &expanded {
             Ok(include::Expanded { name: Some(name), .. }) if !request.name_given => name.clone(),

crates/engine/src/tests/script_folder.rs

@@ -300,6 +300,71 @@ fn an_installed_script_is_read_as_script_run_reads_a_file() {
     let _ = std::fs::remove_dir_all(dir);
 }
 
+#[test]
+fn an_installed_run_runs_the_text_its_own_read_checked() {
+    let dir = folder("installed-run");
+    let root = dir.join("Scripts");
+    let script = touch(&root, "draw.jsx");
+    std::fs::write(&script, "#include \"lib.jsxinc\"\nvc.log(x);").unwrap();
+    std::fs::write(root.join("lib.jsxinc"), "var x = 1;").unwrap();
+    // A folder in the Scripts folder that is a link: script.run by path alone follows it.
+    let elsewhere = dir.join("elsewhere");
+    std::fs::create_dir_all(&elsewhere).unwrap();
+    std::fs::write(elsewhere.join("other.jsx"), "vc.log(2);").unwrap();
+    link_folder(&elsewhere, &root.join("linked"));
+    let linked = root.join("linked").join("other.jsx").to_string_lossy().into_owned();
+    let mut s = Session::new();
+    s.attach_scripts_dir(root.clone());
+    let path = script.to_string_lossy().into_owned();
+    let r = s.execute("script.run", &json!({"path": path, "installed": true})).unwrap();
+    assert_eq!(r["log"], json!([1]));
+    assert_eq!(s.execute("script.run", &json!({"path": linked})).unwrap()["log"], json!([2]));
+    // With `installed`, the run's own read is the check: a script behind a link is not run.
+    let e = s.execute("script.run", &json!({"path": linked, "installed": true})).unwrap_err();
+    assert_eq!(e.to_string(), cmd::scripts::NOT_LISTED);
+    let outside = touch(&dir, "outside.js");
+    let e = s.execute("script.run", &json!({"path": outside.to_string_lossy(), "installed": true})).unwrap_err();
+    assert_eq!(e.to_string(), cmd::scripts::NOT_LISTED);
+    // `installed` is true or false, and takes a path.
+    assert!(
+        s.execute("script.run", &json!({"path": path, "installed": "yes"})).unwrap_err().to_string().contains("`installed` must be true or false")
+    );
+    assert!(s.execute("script.run", &json!({"source": "1", "installed": true})).unwrap_err().to_string().contains("`installed` takes a `path`"));
+    assert_eq!(s.execute("script.run", &json!({"source": "vc.log(3);", "installed": false})).unwrap()["log"], json!([3]));
+    let _ = std::fs::remove_dir_all(dir);
+}
+
+#[test]
+fn an_installed_run_reads_its_includes_in_the_folder_its_read_found() {
+    let dir = folder("installed-scope");
+    let checked = dir.join("Scripts").join("tools");
+    let swapped = dir.join("swapped");
+    for (folder, x) in [(&checked, 1), (&swapped, 2)] {
+        std::fs::create_dir_all(folder).unwrap();
+        std::fs::write(folder.join("draw.jsx"), "").unwrap();
+        std::fs::write(folder.join("lib.jsxinc"), format!("var x = {x};")).unwrap();
+    }
+    // The path now leads through a link to another folder, as after a link was put in place of the
+    // folder once the script's text was read.
+    let link = dir.join("link");
+    link_folder(&swapped, &link);
+    let mut s = Session::new();
+    let request = script::Request {
+        name: "draw.jsx".into(),
+        name_given: false,
+        file: Some(link.join("draw.jsx").to_string_lossy().into_owned()),
+        checked_real: Some(std::fs::canonicalize(checked.join("draw.jsx")).unwrap()),
+        source: "#include \"lib.jsxinc\"\nvc.log(x);".into(),
+        args: Default::default(),
+        fuel: script::DEFAULT_FUEL,
+        locale: "en_US".into(),
+        answers: Vec::new(),
+    };
+    s.script_start(request).unwrap();
+    assert_eq!(s.script_run_to_end().unwrap()["log"], json!([1]));
+    let _ = std::fs::remove_dir_all(dir);
+}
+
 #[test]
 fn the_automation_read_root_keeps_an_agent_out_of_the_scripts_folder() {
     let dir = folder("roots");

crates/mcp/src/ui_catalog.rs

@@ -1737,7 +1737,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[
         "ui.runInstalledScript",
         "Run Script",
         "",
-        "{path: a script's path as script.listInstalled gives it} run a script of the Scripts folder, as its File › Scripts item does (the script is checked again, then script.run runs it by its path, so its #include lines and $.evalFile read beside it), without the warning Other Script… shows; a file that is no longer a .js or .jsx file in the folder or a subfolder at most 4 levels below it, or that is or lies behind a link or is hidden, is not run, and the menu's listing is read again",
+        "{path: a script's path as script.listInstalled gives it} run a script of the Scripts folder, as its File › Scripts item does (script.run with installed reads the file once, checks it again and runs that text, so its #include lines and $.evalFile read in the folder that read found), without the warning Other Script… shows; a file that is no longer a .js or .jsx file in the folder or a subfolder at most 4 levels below it, or that is or lies behind a link or is hidden, is not run, and the menu's listing is read again",
     ),
     (
         "ui.checkScript",

crates/ui-egui/src/menus/ui_commands.rs

@@ -1738,7 +1738,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[
         crate::scripts_menu::COMMAND,
         "Run Script",
         "",
-        "{path: a script's path as script.listInstalled gives it} run a script of the Scripts folder, as its File › Scripts item does (the script is checked again, then script.run runs it by its path, so its #include lines and $.evalFile read beside it), without the warning Other Script… shows; a file that is no longer a .js or .jsx file in the folder or a subfolder at most 4 levels below it, or that is or lies behind a link or is hidden, is not run, and the menu's listing is read again",
+        "{path: a script's path as script.listInstalled gives it} run a script of the Scripts folder, as its File › Scripts item does (script.run with installed reads the file once, checks it again and runs that text, so its #include lines and $.evalFile read in the folder that read found), without the warning Other Script… shows; a file that is no longer a .js or .jsx file in the folder or a subfolder at most 4 levels below it, or that is or lies behind a link or is hidden, is not run, and the menu's listing is read again",
     ),
     (
         "ui.checkScript",

crates/ui-egui/src/scripts_menu.rs

@@ -16,6 +16,7 @@ use std::sync::mpsc::{Receiver, TryRecvError};
 use std::sync::{Arc, Mutex, MutexGuard};
 
 use serde_json::{Value, json};
+use vectorcraft_engine::cmd::scripts::NOT_LISTED;
 use vectorcraft_engine::script::folder::{Entry, Listing};
 
 use crate::VectorcraftApp;
@@ -241,15 +242,16 @@ pub fn readable(p: &Value) -> bool {
     p.get("path").and_then(Value::as_str).is_some_and(|path| vectorcraft_engine::file_access::check_read(path).is_ok())
 }
 
-/// `ui.runInstalledScript {path}`: run the listed script at `path`. A script that is no longer in
-/// the folder is not run, and the folder is read again. The script runs from its file, so its
-/// `#include` lines and `$.evalFile` read beside it.
+/// `ui.runInstalledScript {path}`: run the listed script at `path`. `script.run` reads the file
+/// once, checks it as a listed script and runs that text, so its `#include` lines and `$.evalFile`
+/// read in the folder that read found. A script that is no longer in the folder is not run, and the
+/// folder is read again.
 pub fn run(app: &mut VectorcraftApp, p: &Value) -> Result<Value, String> {
     let path = p.get("path").and_then(Value::as_str).ok_or("missing `path`")?;
-    app.session.installed_script(path).map_err(|e| {
-        let ctx = app.ui_ctx.clone();
-        refresh(app, ctx.as_ref());
-        e.to_string()
-    })?;
-    app.run("script.run", json!({ "path": path }))
+    app.run("script.run", json!({ "path": path, "installed": true })).inspect_err(|e| {
+        if e == NOT_LISTED {
+            let ctx = app.ui_ctx.clone();
+            refresh(app, ctx.as_ref());
+        }
+    })
 }

docs/mcp.md

@@ -2579,6 +2579,9 @@ The rest of this section is VectorCraft's own rules:
   `The Scripts folder cannot be read`.
 - A listed file is checked again when it is chosen. A file that is gone, is no longer a `.js` or `.jsx` file under
   the folder, or is or lies behind a link ends with `the script is no longer in the Scripts folder`, and nothing runs.
+  The file is read once for that check, and the text that read checked is the text that runs; its `#include` lines
+  and `$.evalFile` read in the folder that read found. `script.run {path, installed: true}` runs a listed script the
+  same way.
 - The web build has no Scripts folder, and neither has a run with `VECTORCRAFT_NO_PREFS` set.
 
 `script.listInstalled {}` returns the same listing: `{folder, scripts: [{path, label, folders}], truncated,