xtask: probe keytip commands without file access, and refuse text that could escape generated code

cargo xtask keytips ran every command keytips.json names, with its
params, under the user's file permissions (file.exportPdf could replace
any file, also during --check). It also wrote target, excel and note
strings into generated Rust without escaping. Probes now run inside the
engine's ScriptSandbox, and those fields may not hold quotes,
backslashes, braces or control characters.

crates/engine/src/io.rs

@@ -755,10 +755,10 @@ thread_local! {
 /// While one is alive, [`read_file`] and [`write_file`] on this thread refuse every path: a
 /// stored script's steps run inside one (#2178), so no command reaches the file system from a
 /// script whatever its parameters are called. Dropping it (also on unwind) lifts the refusal.
-pub(crate) struct ScriptSandbox(());
+pub struct ScriptSandbox(());
 
 impl ScriptSandbox {
-    pub(crate) fn enter() -> Self {
+    pub fn enter() -> Self {
         let _ = SCRIPT_SANDBOX.try_with(|c| c.set(c.get().saturating_add(1)));
         ScriptSandbox(())
     }

crates/xlsx/src/tests/xls_tests.rs

@@ -652,19 +652,3 @@ fn xls_short_window2_is_ignored() {
         assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0));
     }
 }
-
-/// Shared-formula followers each keep their own text, charged to the workbook's text allowance:
-/// with no allowance left they keep their cached values, and the import still succeeds.
-#[test]
-fn shared_formula_text_counts_against_the_text_allowance() {
-    let file = compound("Workbook", &sample(false));
-    // The smallest allowance that reads every string: nothing is left for expanded formulas.
-    let left = (0..1000).find(|n| crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
-    let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).unwrap();
-    assert_eq!(ftext(&wb, "B6"), None);
-    assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0));
-    assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings);
-    // With room, the followers keep their formulas.
-    let (wb, _) = crate::xls::read_xls_with(&file, crate::xls::Budget::default()).unwrap();
-    assert!(ftext(&wb, "B6").is_some());
-}

crates/xlsx/src/xls.rs

@@ -89,11 +89,6 @@ impl Budget {
     pub(crate) fn with_max_records(max_records: usize) -> Self {
         Budget { max_records, ..Budget::default() }
     }
-    /// A budget with only `left` bytes of text allowance (tests).
-    #[cfg(test)]
-    pub(crate) fn with_text_left(left: usize) -> Self {
-        Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() }
-    }
     fn record(&mut self) -> Result<(), IoError> {
         self.records = self.records.saturating_add(1);
         if self.records > self.max_records {
@@ -108,16 +103,6 @@ impl Budget {
         }
         Ok(())
     }
-    /// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise:
-    /// for text the import can do without (expanded shared formulas keep their cached values).
-    fn try_text(&mut self, len: usize) -> bool {
-        let total = self.text.saturating_add(len);
-        let fits = total <= MAX_TEXT;
-        if fits {
-            self.text = total;
-        }
-        fits
-    }
     fn text(&mut self, len: usize) -> Result<(), IoError> {
         self.text = self.text.saturating_add(len);
         if self.text > MAX_TEXT {
@@ -502,7 +487,6 @@ fn rk(v: u32) -> Value {
 /// keep their cached values).
 fn resolve_pending(
     st: &mut SheetState<'_>,
-    budget: &mut Budget,
     arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
     shared: &HashMap<CellRef, Parsed>,
     pending: &[Pending],
@@ -520,18 +504,6 @@ fn resolve_pending(
             }
             shared_attaches += 1;
             st.attach(p.pos, f, Mode::Shared, None);
-            let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len());
-            // Every follower keeps its own text: charge it to the workbook's text allowance, and
-            // past it drop the formula again so the cell keeps its cached value.
-            if let Some(len) = len
-                && !budget.try_text(len)
-            {
-                if let Some(cell) = st.sheet.cells.get_mut(p.pos) {
-                    cell.formula = None;
-                }
-                st.unsupported("more shared-formula text than the import allows");
-                break;
-            }
             let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE);
             if !imported {
                 failed.insert(p.anchor);
@@ -614,7 +586,7 @@ fn worksheet(
     // Shared and array formulas: their definitions follow the first cell that uses them.
     let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared));
     let pending = std::mem::take(&mut st.pending);
-    resolve_pending(&mut st, budget, &arrays, &shared, &pending);
+    resolve_pending(&mut st, &arrays, &shared, &pending);
     blank_array_followers(&mut st, &pending);
     if st.dropped > 0 {
         report.warn(format!("{} cells outside the grid were skipped.", st.dropped));

xtask/src/keytips.rs

@@ -77,6 +77,8 @@ fn probe_live_commands(calls: &[(String, String)]) -> Probe {
         let params = if params.is_empty() { serde_json::json!({}) } else { serde_json::from_str(params).unwrap_or_default() };
         let mut s = Session::new();
         s.new_workbook();
+        // The JSON picks the commands and their params: none of them may touch the file system.
+        let _sandbox = gridcraft_engine::io::ScriptSandbox::enter();
         let status = match s.execute(id, params.clone()) {
             Ok(_) => {
                 if s.journal.iter().any(|(jid, _)| jid == id) {
@@ -181,15 +183,16 @@ fn parse_and_probe(source: &str) -> Result<(Vec<Entry>, Probe), String> {
                 }
             }
         }
-        entries.push(Entry {
-            seq,
-            kind,
-            target,
-            pending,
-            params,
-            excel: e.get("excel").and_then(|v| v.as_str()).unwrap_or("").to_string(),
-            note: e.get("note").and_then(|v| v.as_str()).unwrap_or("").to_string(),
-        });
+        let excel = e.get("excel").and_then(|v| v.as_str()).unwrap_or("").to_string();
+        let note = e.get("note").and_then(|v| v.as_str()).unwrap_or("").to_string();
+        // These are written into generated Rust: a target inside string literals and format
+        // strings, the annotations after `//`. Refuse anything that could end either.
+        for (field, text) in [("target", &target), ("excel", &excel), ("note", &note)] {
+            if text.chars().any(|c| c.is_control() || matches!(c, '"' | '\\' | '{' | '}')) {
+                return Err(format!("{at}: `{field}` must not hold quotes, backslashes, braces or control characters: {text:?}"));
+            }
+        }
+        entries.push(Entry { seq, kind, target, pending, params, excel, note });
     }
     // JSON order is authoritative and curated: the first entry for a tab is its canonical
     // badge (legacy aliases come later), and groups stay together. Do not sort.
@@ -434,3 +437,34 @@ fn reset(h: &mut Harness) {
     key(h, Key::Escape); // leave keytip mode entirely
 }
 "#;
+
+#[cfg(all(test, feature = "engine"))]
+mod tests {
+    use super::*;
+
+    /// Text that would end a comment or string literal in the generated Rust is refused.
+    #[test]
+    fn annotations_and_targets_cannot_escape_generated_code() {
+        for entry in [
+            r#"{"seq": "Q", "kind": "command", "target": "edit.copy", "note": "x\nfn evil() {}"}"#,
+            r#"{"seq": "Q", "kind": "command", "target": "edit.copy", "excel": "a\rb"}"#,
+            r#"{"seq": "Q", "kind": "dialog", "target": "x\"); evil(); (\""}"#,
+            r#"{"seq": "Q", "kind": "control", "target": "{0}"}"#,
+        ] {
+            let doc = format!(r#"{{"sequences": [{entry}]}}"#);
+            assert!(parse_and_probe(&doc).is_err(), "{entry}");
+        }
+        assert!(parse_and_probe(r#"{"sequences": [{"seq": "Q", "kind": "control", "target": "fontSize", "note": "Font Size box"}]}"#).is_ok());
+    }
+
+    /// A probed command may not write files, whatever its params say.
+    #[test]
+    fn probes_cannot_touch_the_file_system() {
+        let path = std::env::temp_dir().join(format!("keytips-probe-{}.pdf", std::process::id()));
+        let _ = fs::remove_file(&path);
+        let calls = vec![("file.exportPdf".to_string(), serde_json::json!({ "path": path.to_string_lossy() }).to_string())];
+        let probe = probe_live_commands(&calls);
+        assert_eq!(probe.values().copied().collect::<Vec<_>>(), [2]);
+        assert!(!path.exists());
+    }
+}