diff --git a/crates/engine/src/cmd/controls.rs b/crates/engine/src/cmd/controls.rs index 94a4ef7ec..4a9ade919 100644 --- a/crates/engine/src/cmd/controls.rs +++ b/crates/engine/src/cmd/controls.rs @@ -484,7 +484,7 @@ fn press(s: &mut Session, p: &Json) -> Result { let key = (doc, approval_key(&id, ¶ms)); if !s.macro_approvals.contains(&key) { if bool_param(p, "approve") != Some(true) { - return Err(EngineError::Approve(format!("Run {id} from '{name}'? It {why}."))); + return Err(EngineError::Approve(format!("Run {id} from '{name}'? It {why}{}.", path_note(¶ms)))); } s.macro_approvals.insert(key); } @@ -497,6 +497,27 @@ fn press(s: &mut Session, p: &Json) -> Result { } } +/// The file a button's command names in its `path`, as ` (path)` for its approval question, so +/// the person sees what it reads or writes before saying yes. Empty without a `path`; a long one +/// is shortened in the middle. +fn path_note(params: &Json) -> String { + let text = match params.get("path") { + None | Some(Json::Null) => return String::new(), + Some(Json::String(t)) => t.clone(), + Some(other) => other.to_string(), + }; + let text: String = text.chars().map(|c| if c.is_control() { ' ' } else { c }).collect(); + let n = text.chars().count(); + let shown = if n > 200 { + let head: String = text.chars().take(100).collect(); + let tail: String = text.chars().skip(n - 99).collect(); + format!("{head}…{tail}") + } else { + text + }; + format!(" ({shown})") +} + /// The approval key of a button's command: its id and parameters, with a prefix that keeps it /// apart from a macro's fingerprint (canonical JSON, which starts with `{`). fn approval_key(id: &str, params: &Json) -> String { diff --git a/crates/engine/src/tests/controls.rs b/crates/engine/src/tests/controls.rs index 4ea2461a0..61de5caa2 100644 --- a/crates/engine/src/tests/controls.rs +++ b/crates/engine/src/tests/controls.rs @@ -246,6 +246,35 @@ fn a_command_button_goes_through_the_policy_with_the_documents_trust() { assert_eq!(v(&s, "B1"), Value::Number(1.0)); } +#[test] +fn a_file_commands_approval_question_names_the_file() { + let mut s = s(); + let b = id(&s.execute("insert.button", json!({"command": "file.exportCsv", "params": {"path": "/tmp/reports/q3 totals.csv"}})).unwrap()); + match s.execute("control.press", json!({"id": b})) { + Err(EngineError::Approve(q)) => assert!(q.contains("writes a file (/tmp/reports/q3 totals.csv)"), "{q}"), + other => panic!("expected a question, got {other:?}"), + } + // A long path is shortened in the middle, and line breaks can't hide part of it. + let long = format!("/tmp/{}\n{}.csv", "a".repeat(300), "b".repeat(300)); + let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap(); + if let Some(c) = sh.controls.iter_mut().find(|c| c.id as u64 == b) { + c.action = Some(ControlAction::Command { id: "file.exportCsv".into(), params: json!({"path": long}) }); + } + match s.execute("control.press", json!({"id": b})) { + Err(EngineError::Approve(q)) => { + assert!(q.contains("(/tmp/aaa") && q.contains("…") && q.contains("bbb.csv)") && !q.contains('\n') && q.len() < 600, "{q}") + } + other => panic!("expected a question, got {other:?}"), + } + // Without a path the question is unchanged. + let plain = id(&s.execute("insert.button", json!({"command": "cell.set", "params": {"cell": "A1", "input": "1"}})).unwrap()); + s.doc_mut().unwrap().trust = Trust::FromFile; + match s.execute("control.press", json!({"id": plain})) { + Err(EngineError::Approve(q)) => assert!(q.ends_with("came with a workbook you opened."), "{q}"), + other => panic!("expected a question, got {other:?}"), + } +} + #[test] fn hostile_params_are_refused() { let mut s = s();