diff --git a/crates/engine/src/tests/plugins.rs b/crates/engine/src/tests/plugins.rs index 51e67766f..0e83a5365 100644 --- a/crates/engine/src/tests/plugins.rs +++ b/crates/engine/src/tests/plugins.rs @@ -1296,8 +1296,8 @@ fn a_request_cannot_grant_what_only_a_person_approves() { let e = s.execute("plugin.install", &json!({"dataBase64": data})).unwrap_err().to_string(); assert!(e.contains("approve them in the desktop app") && e.contains("--allow-plugin-grants"), "{e}"); let batch = json!({"commands": [{"command": "plugin.install", "params": {"dataBase64": data, "grant": ["doc.read"]}}]}); - let r = s.execute("command.batch", &batch); - assert!(r.as_ref().map_or(true, |v| v.to_string().contains(refused)), "a batch can't grant: {r:?}"); + let e = s.execute("command.batch", &batch).unwrap_err().to_string(); + assert!(e.contains(refused), "a batch can't grant: {e}"); assert!(vectorcraft_plugins::registry::get(id).is_none()); // Approved for other bytes, for another id or for other permissions: refused, and spent. @@ -1359,6 +1359,43 @@ fn a_request_cannot_grant_what_only_a_person_approves() { run(&mut s, "plugin.remove", json!({"id": id})); } +/// A module a person approved in the desktop app, recorded in the store's `grants.json`, installs +/// from another session with that store and no flag and no `grant`. Other bytes under its id are +/// refused there, with or without `grant`. +#[test] +fn a_module_approved_in_the_app_installs_from_a_strict_session_with_its_store() { + let folder = crate::tests_links::Folder::new("plugin-approval-store"); + let id = "test.engine.approval.store"; + let wasm = asking(id, r#"["doc.read"]"#, &json!({"objects": []})); + let data = vectorcraft_format::base64_encode(&wasm); + let mut desktop = strict_session(); + desktop.attach_plugin_store(folder.0.clone()); + desktop.approve_plugin_install(approval(id, &wasm, &["doc.read"], false)); + run(&mut desktop, "plugin.install", json!({"dataBase64": data, "grant": ["doc.read"]})); + assert!(std::fs::read_to_string(folder.0.join("grants.json")).unwrap().contains(id), "recorded in the store"); + + // Another process starts with an empty registry and the store's grants. + let forget = || { + vectorcraft_plugins::registry::remove(id); + vectorcraft_plugins::registry::revoke(id); + }; + forget(); + let mut headless = strict_session(); + headless.attach_plugin_store(folder.0.clone()); + forget(); + run(&mut headless, "plugin.install", json!({"dataBase64": data})); + assert_eq!(vectorcraft_plugins::registry::get(id).map(|p| p.hash().to_string()), Some(vectorcraft_plugins::permissions::sha256_hex(&wasm))); + + let other = asking(id, r#"["doc.read"]"#, &json!({"objects": [{"name": "x"}]})); + let other_data = vectorcraft_format::base64_encode(&other); + let e = headless.execute("plugin.install", &json!({"dataBase64": other_data, "replace": true})).unwrap_err().to_string(); + assert!(e.contains("approve them in the desktop app"), "{e}"); + let e = headless.execute("plugin.install", &json!({"dataBase64": other_data, "replace": true, "grant": ["doc.read"]})).unwrap_err().to_string(); + assert!(e.contains("take effect only in a session started with --allow-plugin-grants"), "{e}"); + assert_eq!(vectorcraft_plugins::registry::get(id).map(|p| p.hash().to_string()), Some(vectorcraft_plugins::permissions::sha256_hex(&wasm))); + run(&mut headless, "plugin.remove", json!({"id": id})); +} + /// A filter run the caller cancels (MCP's `notifications/cancelled`, the app's Cancel) stops between /// two slices of its budget and changes nothing. #[test] diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 90df3349e..9920f1af8 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -709,6 +709,11 @@ fn errors_are_tool_results_not_crashes() { /// A fake control server: answers `document.inspect`, echoes `engine.execute`, errors otherwise. fn fake_app() -> (String, std::thread::JoinHandle>) { + fake_app_refusing_confirm(None) +} + +/// [`fake_app`], answering `ui.dialog.confirm` with the error `refusal` when one is given. +fn fake_app_refusing_confirm(refusal: Option<&'static str>) -> (String, std::thread::JoinHandle>) { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap().to_string(); let h = std::thread::spawn(move || { @@ -724,6 +729,7 @@ fn fake_app() -> (String, std::thread::JoinHandle>) { "engine.execute" => json!({"ok": true, "result": {"echo": msg["params"]}}), "ui.inspect" => json!({"ok": true, "result": {"tool": "selection"}}), "ui.dialog.set" if !dialog_open => json!({"ok": false, "error": "no dialog open"}), + "ui.dialog.confirm" if refusal.is_some() => json!({"ok": false, "error": refusal}), m if m.starts_with("ui.dialog.") => { dialog_open &= m != "ui.dialog.cancel"; json!({"ok": true, "result": {"echo": msg["params"]}}) @@ -814,6 +820,23 @@ fn answer_dialog_does_not_confirm_when_the_app_refuses_the_set() { assert_eq!(seen[1]["params"], json!({"fields": set})); } +/// The app refuses to confirm a plug-in's install approval for automation: the call is an error +/// with the app's reason, and nothing else is sent that could install the module. +#[test] +fn answer_dialog_reports_that_the_app_refuses_to_approve_a_plugin_install() { + let refusal = "Only the person at the keyboard can approve installing a plug-in"; + let (addr, h) = fake_app_refusing_confirm(Some(refusal)); + let mut s = Server::new(Box::new(Remote::connect(&addr).unwrap())); + let set = json!([{"field": "__params", "value": {"dataBase64": "AAAA"}}]); + let r = call(&mut s, 1, "answer_dialog", json!({"set": set, "action": "confirm"})); + assert_eq!(r["isError"], true, "{r}"); + assert!(text_of(&r).contains(refusal), "{r}"); + drop(s); + let seen = h.join().unwrap(); + let methods: Vec<&str> = seen.iter().map(|m| m["method"].as_str().unwrap()).collect(); + assert_eq!(methods, ["ui.dialog.set", "ui.dialog.confirm"]); +} + #[test] fn answer_dialog_needs_the_desktop_app() { let mut s = server(); diff --git a/crates/ui-egui/src/dialogs/plugin.rs b/crates/ui-egui/src/dialogs/plugin.rs index f8457385d..46a1c4210 100644 --- a/crates/ui-egui/src/dialogs/plugin.rs +++ b/crates/ui-egui/src/dialogs/plugin.rs @@ -14,7 +14,7 @@ //! //! Dialog `pluginInstall` asks a person to approve a plug-in's permissions or heavy budget. What it //! shows and installs is held by the app, not in its fields; a control request or synthetic input -//! can't confirm it, and Enter doesn't. +//! can't confirm it. Enter confirms it only when OK has the keyboard focus. use std::sync::Arc; @@ -285,7 +285,8 @@ pub(crate) struct PendingInstall { pub approval: InstallApproval, } -/// No OK in the shared footer, so Enter does nothing. +/// No OK in the shared footer: the OK in its body is a button, which Enter clicks only when it has +/// the keyboard focus. pub(super) const INSTALL_SPEC: DialogSpec = DialogSpec { heading: |_| tl!("Install Plug-in…").trim_end_matches('…').to_string(), body_with_actions: Some(install_body), diff --git a/crates/ui-egui/src/dialogs/tests_plugin_fields.rs b/crates/ui-egui/src/dialogs/tests_plugin_fields.rs index 8f78b719d..f5f4e719a 100644 --- a/crates/ui-egui/src/dialogs/tests_plugin_fields.rs +++ b/crates/ui-egui/src/dialogs/tests_plugin_fields.rs @@ -13,7 +13,8 @@ use vectorcraft_plugins::wat as tpl; use super::*; -/// The install approval has no OK in the shared footer: Enter, real or injected, doesn't press it. +/// The install approval has no OK in the shared footer, so Enter presses OK only when OK has the +/// keyboard focus. #[test] fn enter_does_not_answer_the_install_approval() { assert!(plugin::INSTALL_SPEC.ok.is_none() && plugin::INSTALL_SPEC.body_with_actions.is_some()); diff --git a/crates/ui-egui/src/tests/automation.rs b/crates/ui-egui/src/tests/automation.rs index b61360dc8..86e6c6940 100644 --- a/crates/ui-egui/src/tests/automation.rs +++ b/crates/ui-egui/src/tests/automation.rs @@ -366,6 +366,12 @@ fn automation_cannot_approve_a_plugin_install() { // What the dialog installs isn't in its fields. let r = request(&mut app, &tx, "ui.dialog.set", json!({"field": "__params", "value": {"dataBase64": "AAAA"}})); assert!(r.get("error").is_none(), "{r}"); + // What MCP's `answer_dialog` sends: `ui.dialog.set {fields}`, then `ui.dialog.confirm`. + let r = request(&mut app, &tx, "ui.dialog.set", json!({"fields": [{"field": "__params", "value": {"dataBase64": "AAAA"}}]})); + assert!(r.get("error").is_none(), "{r}"); + let r = request(&mut app, &tx, "ui.dialog.confirm", json!({})); + refused(&r, "Only the person at the keyboard"); + assert!(app.ui.dialog.is_some() && vectorcraft_plugins::registry::get(id).is_none()); app.synthetic_frame = true; assert!(crate::dialogs::confirm(&mut app).unwrap_err().contains("Only the person at the keyboard")); assert!(vectorcraft_plugins::registry::get(id).is_none()); @@ -376,6 +382,103 @@ fn automation_cannot_approve_a_plugin_install() { app.run("plugin.remove", json!({"id": id})).unwrap(); } +type DialogHarness = egui_kittest::Harness<'static, VectorcraftApp>; + +/// Runs one UI pass of `h` for each batch of synthetic input the control path queued, each taken +/// through the host's raw-input hook, until none is left and the last batch is acknowledged. +fn take_injected(h: &mut DialogHarness) { + let ctx = h.ctx.clone(); + loop { + let mut raw = egui::RawInput::default(); + h.state_mut().raw_input_hook(&ctx, &mut raw); + if raw.events.is_empty() && h.state().synthetic.is_empty() { + break; + } + h.input_mut().events.extend(raw.events); + h.step(); + } +} + +/// The open install approval, with nothing installed or granted for `id`. +fn still_waiting(h: &DialogHarness, id: &str) -> bool { + h.state().ui.dialog.as_ref().is_some_and(|d| d.kind == crate::dialogs::plugin::INSTALL_KIND) + && vectorcraft_plugins::registry::get(id).is_none() + && vectorcraft_plugins::registry::granted(id).is_none() +} + +/// Keys and pointer input sent through the control path approve nothing at the open install +/// approval: Enter and Space press nothing with no button focused, and with OK focused they and a +/// `ui.click` on it press it and are refused. `ui.pointer` drives the canvas. A person's click on +/// the same button approves. +#[test] +fn injected_keys_and_pointer_input_cannot_approve_a_plugin_install() { + use egui_kittest::kittest::{NodeT, Queryable}; + let refusal = "Only the person at the keyboard"; + let (tx, rx) = std::sync::mpsc::channel(); + let mut app = VectorcraftApp::new(Session::new(), Default::default()).with_control(rx); + app.run("file.new", json!({"width": 100, "height": 100})).unwrap(); + let id = "test.ui.approval.injected"; + let manifest = format!(r#"{{"id":"{id}","name":"Approval","version":"1","kind":"filter","permissions":["doc.read"]}}"#); + let bytes = wat::parse_str(vectorcraft_plugins::wat::returning(&manifest, r#"{"objects":[]}"#)).unwrap(); + let data = vectorcraft_format::base64_encode(&bytes); + let r = request(&mut app, &tx, "ui.menu.invoke", json!({"command": "ui.installPlugin", "params": {"dataBase64": data}})); + assert_eq!(r["result"]["pending"], crate::dialogs::plugin::INSTALL_KIND, "{r}"); + let mut fonts = false; + let mut h: DialogHarness = egui_kittest::Harness::builder().with_size(egui::vec2(900.0, 800.0)).build_ui_state( + move |ui, app: &mut VectorcraftApp| { + if !std::mem::replace(&mut fonts, true) { + crate::theme::install_fonts(ui.ctx()); + return; + } + crate::dialogs::show(app, ui.ctx()); + }, + app, + ); + h.run_steps(4); + + for key in ["Enter", "Space"] { + assert!(!h.get_by_label("OK").accesskit_node().is_focused(), "no button has the focus yet"); + h.state_mut().ui.status.clear(); + let r = request(h.state_mut(), &tx, "ui.key", json!({"key": key})); + assert_eq!(r["ok"], true, "{r}"); + take_injected(&mut h); + assert!(still_waiting(&h, id), "{key}"); + assert_eq!(h.state().ui.status, "", "{key} presses nothing"); + } + + for key in ["Enter", "Space"] { + h.get_by_label("OK").focus(); + h.run_steps(2); + assert!(h.get_by_label("OK").accesskit_node().is_focused()); + h.state_mut().ui.status.clear(); + let r = request(h.state_mut(), &tx, "ui.key", json!({"key": key})); + assert_eq!(r["ok"], true, "{r}"); + take_injected(&mut h); + assert!(still_waiting(&h, id), "{key} on a focused OK"); + assert!(h.state().ui.status.contains(refusal), "{key} pressed OK and was refused: {:?}", h.state().ui.status); + } + + let at = h.get_by_label("OK").rect().center(); + h.state_mut().ui.status.clear(); + let r = request(h.state_mut(), &tx, "ui.click", json!({"x": at.x, "y": at.y})); + assert_eq!(r["ok"], true, "{r}"); + take_injected(&mut h); + assert!(still_waiting(&h, id), "ui.click"); + assert!(h.state().ui.status.contains(refusal), "ui.click pressed OK and was refused: {:?}", h.state().ui.status); + + let events = json!([{"kind": "down", "x": at.x, "y": at.y}, {"kind": "up", "x": at.x, "y": at.y}]); + let r = request(h.state_mut(), &tx, "ui.pointer", json!({"events": events})); + assert_eq!(r["ok"], true, "{r}"); + take_injected(&mut h); + assert!(still_waiting(&h, id), "ui.pointer"); + + h.get_by_label("OK").click(); + h.run_steps(2); + assert!(h.state().ui.dialog.is_none(), "{:?}", h.state().ui.status); + assert_eq!(vectorcraft_plugins::registry::granted(id).map(|g| g.hash), Some(vectorcraft_plugins::permissions::sha256_hex(&bytes))); + h.state_mut().run("plugin.remove", json!({"id": id})).unwrap(); +} + /// Edit Original and the Attributes panel's Browser button hand a file or URL to another app: a /// control request (or MCP, which comes through it) can't run them, roots or not. #[test] diff --git a/crates/ui-egui/src/tests/plugins.rs b/crates/ui-egui/src/tests/plugins.rs index d044deb17..22cd5caff 100644 --- a/crates/ui-egui/src/tests/plugins.rs +++ b/crates/ui-egui/src/tests/plugins.rs @@ -336,6 +336,63 @@ fn a_shortcut_runs_a_plugin_command() { app.run("plugin.remove", json!({"id": id})).unwrap(); } +/// The `grant` and `allowHeavy` that `ui.installPlugin` is given approve nothing: it asks all the +/// same, and what OK would approve comes from the manifest. A module with nothing to approve +/// installs without either. +#[test] +fn ui_install_plugin_takes_no_grant_or_heavy_budget_from_its_params() { + let mut app = app(); + let id = "test.ui.approval.params"; + let manifest = format!(r#"{{"id":"{id}","name":"Asks","kind":"filter","permissions":["doc.read"]}}"#); + let asks = wat::parse_str(tpl::returning(&manifest, r#"{"objects":[]}"#)).unwrap(); + let given = |wasm: &[u8]| json!({"dataBase64": vectorcraft_format::base64_encode(wasm), "grant": ["doc.read", "doc.write"], "allowHeavy": true}); + let r = app.run("ui.installPlugin", given(&asks)).unwrap(); + assert_eq!(r["pending"], dialogs::plugin::INSTALL_KIND); + assert!(vectorcraft_plugins::registry::get(id).is_none() && vectorcraft_plugins::registry::granted(id).is_none()); + let pending = app.pending_plugin_install.as_ref().unwrap(); + assert_eq!(pending.approval.permissions, ["doc.read"], "the permissions the manifest lists"); + assert_eq!(pending.params["grant"], json!(["doc.read"])); + assert!(!pending.approval.heavy && pending.params.get("allowHeavy").is_none(), "no heavy budget was asked about"); + dialogs::cancel(&mut app); + assert!(vectorcraft_plugins::registry::get(id).is_none()); + + let heavy_id = "test.ui.approval.params.heavy"; + let manifest = format!(r#"{{"id":"{heavy_id}","name":"Heavy","kind":"filter","budget":"heavy"}}"#); + let heavy = wat::parse_str(tpl::spinning(&manifest, 1, r#"{"objects":[]}"#)).unwrap(); + let r = app.run("ui.installPlugin", given(&heavy)).unwrap(); + assert_eq!(r["pending"], dialogs::plugin::INSTALL_KIND, "allowHeavy skips no question"); + assert!(vectorcraft_plugins::registry::get(heavy_id).is_none()); + dialogs::cancel(&mut app); + + let plain_id = "test.ui.approval.params.plain"; + let manifest = format!(r#"{{"id":"{plain_id}","name":"Plain","kind":"filter"}}"#); + let plain = wat::parse_str(tpl::returning(&manifest, r#"{"objects":[]}"#)).unwrap(); + let r = app.run("ui.installPlugin", given(&plain)).unwrap(); + assert!(r.get("pending").is_none() && app.ui.dialog.is_none(), "{r}"); + assert!(vectorcraft_plugins::registry::get(plain_id).is_some()); + assert!(vectorcraft_plugins::registry::granted(plain_id).is_none(), "the grant it was given isn't recorded"); + app.run("plugin.remove", json!({"id": plain_id})).unwrap(); +} + +/// An action's step can't grant what only a person approves: a played `plugin.install` with `grant` +/// is refused at Accelerated and at Step by Step, and nothing is installed. +#[test] +fn an_action_step_cannot_grant_a_plugin_its_permissions() { + use crate::panels::actions::{Action, ActionSet, PlaybackSpeed, play}; + let mut app = app(); + let id = "test.ui.approval.action"; + let manifest = format!(r#"{{"id":"{id}","name":"Asks","kind":"filter","permissions":["doc.read"]}}"#); + let wasm = wat::parse_str(tpl::returning(&manifest, r#"{"objects":[]}"#)).unwrap(); + let step = json!({"dataBase64": vectorcraft_format::base64_encode(&wasm), "grant": ["doc.read"]}); + app.ui.action_sets = vec![ActionSet::new("Test", vec![Action::new("Install", vec![("plugin.install".into(), step)])])]; + for speed in [PlaybackSpeed::Accelerated, PlaybackSpeed::Step] { + app.ui.action_playback.speed = speed; + let e = play(&mut app, 0, 0).unwrap_err(); + assert!(e.contains("take effect only in a session started with --allow-plugin-grants"), "{e}"); + assert!(vectorcraft_plugins::registry::get(id).is_none() && vectorcraft_plugins::registry::granted(id).is_none()); + } +} + /// A plug-in of API level 2 that lists `progress` names it in plain words when it asks. #[test] fn installing_a_plugin_that_reports_progress_says_so() { diff --git a/docs/plugins.md b/docs/plugins.md index 8a94546de..8df6ae39d 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -164,7 +164,8 @@ capability model proposed for PhotoCraft plug-ins SHA-256 shown; Cancel installs nothing. The names shown have control and bidirectional characters escaped (`\u{202E}`) and are cut to 64 characters. Only a person can press its OK: `ui.dialog.confirm`, MCP's `answer_dialog` and injected input (`ui.key`, `ui.click`) - are refused and the dialog stays open, and Enter does not confirm it. `ui.dialog.set` can't change what it installs. + are refused and the dialog stays open. Enter confirms it only when OK has the keyboard focus. `ui.dialog.set` can't + change what it installs. - `plugin.install` refuses the module, naming the permissions it needs, unless a person already granted this module (`plugin.inspect`'s `needsApproval` tells it beforehand). A request's `grant` is refused too: `plugin.install` takes it only with the approval the dialog gives. A `vectorcraft-cli mcp` or `run` started with `--allow-plugin-grants`