Engine: macro policy checks a step's parameters after tokens resolve; any path counts as file access

A path given as a token (`{"$ref": "Input"}` or `{{Input}}`) became a file name only when the
step ran, after the policy had passed it, so a macro without permissions or approval could write
a file, even from macro.check. Exports also wrote to blank paths the policy ignored. Any `path`
other than null now needs the permission and approval, and the runner checks the policy again on
the resolved parameters before the command runs.

crates/engine/src/macros/policy.rs

@@ -43,8 +43,12 @@ const READS_FILES: &[&str] = &["data.getData", "data.fromTextCsv", "insert.pictu
 const EXPORTS: &[&str] = &["file.exportPdf", "file.exportCsv", "file.exportHtml", "file.print", "file.printPreview", "file.saveBytes"];
 
 /// The policy for running command `id` with `params` as a macro step in a document with `trust`.
+///
+/// Any `path` other than `null` counts as touching a file: a token such as `{"$ref": "Input"}`
+/// becomes a path when the step runs, and exports write to `""` and `" "` as file names. The
+/// runner checks the parameters both as written and as resolved.
 pub fn macro_policy(id: &str, params: &Json, trust: Trust) -> Policy {
-    let has_path = params.get("path").and_then(Json::as_str).is_some_and(|p| !p.trim().is_empty());
+    let has_path = params.get("path").is_some_and(|p| !p.is_null());
     let base = if EXPORTS.contains(&id) {
         if has_path { Policy::NeedsPermission("writes a file".into()) } else { Policy::Allow }
     } else if ALLOWED.contains(&id) {
@@ -104,7 +108,14 @@ mod tests {
         // Without a path they touch no file.
         for id in ["data.getData", "insert.picture", "file.exportPdf", "file.saveBytes", "file.printPreview"] {
             assert_eq!(macro_policy(id, &none, Trust::Local), Policy::Allow, "{id}");
-            assert_eq!(macro_policy(id, &json!({"path": " "}), Trust::Local), Policy::Allow, "{id}");
+            assert_eq!(macro_policy(id, &json!({"path": null}), Trust::Local), Policy::Allow, "{id}");
+        }
+        // A path that is blank or a token still touches a file once the step runs.
+        for path in [json!(""), json!(" "), json!({"$ref": "Input"}), json!("{{Input}}.csv"), json!(1)] {
+            for id in ["file.exportCsv", "data.fromTextCsv"] {
+                let p = json!({ "path": path });
+                assert!(matches!(macro_policy(id, &p, Trust::Local), Policy::NeedsPermission(_)), "{id} {p}");
+            }
         }
         for id in ["cell.set", "home.bold", "view.macros", "edit.copy", "file.properties"] {
             assert_eq!(macro_policy(id, &none, Trust::Local), Policy::Allow, "{id}");

crates/engine/src/macros/run.rs

@@ -576,29 +576,34 @@ impl Machine {
 
     fn command(&mut self, s: &mut Session, path: &str, id: &str, params: &Json) -> std::result::Result<(), StepError> {
         let fail = |msg: String| StepError::new(path, Some(id), msg);
-        match macro_policy(id, params, self.trust) {
-            Policy::Deny(why) => return Err(fail(format!("{id} can't run in a macro: {why}."))),
-            Policy::NeedsPermission(what) => {
+        // The policy sees the parameters the command will get, after tokens became A1 text.
+        let resolved = {
+            let wb = &s.doc().map_err(|e| fail(e.to_string()))?.wb;
+            resolve::resolve_params(params, &self.env(wb)).map_err(|e| StepError::resolve(path, Some(id), e))?
+        };
+        match (macro_policy(id, params, self.trust), macro_policy(id, &resolved, self.trust)) {
+            (_, Policy::Deny(why)) | (Policy::Deny(why), _) => return Err(fail(format!("{id} can't run in a macro: {why}."))),
+            (written, Policy::NeedsPermission(what)) => {
                 if !self.permissions.iter().any(|p| p == id) {
                     return Err(fail(format!("{id} {what}. Add it to the macro's permissions to allow it.")));
                 }
+                // The gate asked the person about this step only if it saw the file access.
+                if !matches!(written, Policy::NeedsPermission(_)) {
+                    return Err(fail(format!("{id} {what} only once its parameters are filled in, so it wasn't approved.")));
+                }
                 if self.dry_run {
                     self.partial = Some((path.to_string(), format!("{id} {what}")));
                     self.frames.clear();
                     return Ok(());
                 }
             }
-            Policy::Allow | Policy::Ask(_) => {}
+            (_, Policy::Allow | Policy::Ask(_)) => {}
         }
         if self.dry_run && !previewable(id) {
             self.partial = Some((path.to_string(), format!("{id} can't be previewed")));
             self.frames.clear();
             return Ok(());
         }
-        let resolved = {
-            let wb = &s.doc().map_err(|e| fail(e.to_string()))?.wb;
-            resolve::resolve_params(params, &self.env(wb)).map_err(|e| StepError::resolve(path, Some(id), e))?
-        };
         self.last_params = Some(resolved.clone());
         let queued = s.ui_requests.len();
         let r = s.execute(id, resolved);

crates/engine/src/tests/macros.rs

@@ -358,6 +358,46 @@ fn macros_are_bounded() {
     assert!(err.contains("ran longer than"), "{err}");
 }
 
+#[test]
+fn a_token_in_a_path_needs_the_permission_and_approval() {
+    let mut s = s();
+    let dir = std::env::temp_dir().join(format!("gc-macro-token-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let files = |dir: &std::path::Path| std::fs::read_dir(dir).unwrap().count();
+    let in_dir = format!("{}/{{{{Input}}}}", dir.to_string_lossy());
+    for path in [json!({"$ref": "Input"}), json!(in_dir), json!(" ")] {
+        let step = json!({"command": "file.exportCsv", "params": {"path": path}});
+        put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step]}));
+        // Without the permission nothing runs, not even the check's dry run.
+        let err = s.execute("macro.run", json!({"name": "Export"})).unwrap_err().to_string();
+        assert!(err.contains("Step 1 (file.exportCsv) writes a file. Add file.exportCsv to the macro's permissions"), "{path}: {err}");
+        let c = s.execute("macro.check", json!({"name": "Export"})).unwrap();
+        assert!(c["errors"][0].as_str().unwrap().contains("writes a file"), "{path}: {c}");
+        assert!(c.get("footprint").is_none());
+        // With it, the person approves first; the check doesn't run it before then.
+        put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step], "permissions": ["file.exportCsv"]}));
+        let r = s.execute("macro.run", json!({"name": "Export"}));
+        let Err(EngineError::Approve(q)) = &r else { panic!("{path}: {r:?}") };
+        assert!(q.contains("file.exportCsv, which writes a file"), "{q}");
+        let c = s.execute("macro.check", json!({"name": "Export"})).unwrap();
+        assert_eq!(c["needsApproval"], json!(["runs file.exportCsv, which writes a file"]), "{path}");
+        assert!(c.get("footprint").is_none());
+        assert_eq!(files(&dir), 0, "{path}");
+        assert!(!std::path::Path::new("A1.csv").exists() && !std::path::Path::new(" .csv").exists());
+    }
+    // Once approved, the check's dry run stops before the step, and only the run writes the file.
+    let step = json!({"command": "file.exportCsv", "params": {"path": in_dir}});
+    put(&mut s, json!({"name": "Export", "inputs": [{"name": "Input"}], "steps": [step], "permissions": ["file.exportCsv"]}));
+    s.execute("macro.run", json!({"name": "Export", "approve": true, "inputs": "B2"})).unwrap();
+    assert_eq!(files(&dir), 1);
+    std::fs::remove_dir_all(&dir).unwrap();
+    std::fs::create_dir_all(&dir).unwrap();
+    let c = s.execute("macro.check", json!({"name": "Export", "inputs": "B2"})).unwrap();
+    assert_eq!(c["footprint"]["partial"]["step"], "1", "{c}");
+    assert_eq!(files(&dir), 0, "the check writes nothing");
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn the_policy_keeps_macros_away_from_files_and_the_app() {
     let mut s = s();