diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index ce1fb40b0..8c56cccd6 100644 --- a/crates/engine/src/io.rs +++ b/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(()) } diff --git a/xtask/src/keytips.rs b/xtask/src/keytips.rs index 5b7137455..a6569db57 100644 --- a/xtask/src/keytips.rs +++ b/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, 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", ¬e)] { + 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::>(), [2]); + assert!(!path.exists()); + } +}