collective
Browse / github.com/storytold/photocraft / patch
Keep automation's gate on actions automation wrote, whoever plays them
Automation: an action a confined client loads or edits runs with the user's file access when the user plays it **Version:** PhotoCraft 0.16.0 (b1794c6e0, 2026-10-25), Windows 11. Desktop app started with `--control`, and `--automation-read-root` / `--automation-write-root` set to one folder. **Problem** A control or MCP client confined to its automation roots can write an action without running it: - `actions.load` with inline `data`. - `actions.insert` of any step into an action. It can also give the action a key with `edit.keyboardShortcuts` (`actions.play:<name>`). All of these are allowed, since none of them touches a file. The steps are only judged when they play, and playback is gated by `Se
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Higgs32584 (imported from pull request #9783, open)
- Base
d42f9da94a421980d7130c63ca2953e47802cfbe- Size
- +127 −2 in 8 files
- Review
- pending
- Shared
- 2026-10-28
- Id
0ccb56433def50090b200d2d
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/actions_cmds.rs crates/engine/src/cmd/actions_cmds/tests.rs crates/engine/src/cmd/actions_edit_cmds.rs crates/engine/src/cmd/atn_cmds/files.rs crates/engine/src/lib.rs crates/ui-egui/src/app.rs crates/ui-egui/src/panels/actions.rs crates/ui-egui/src/panels/actions/tests.rs
@@ -48,6 +48,29 @@ pub struct Action { /// Button Mode colour. Empty is none; otherwise one of [`BUTTON_COLORS`]. #[serde(default, skip_serializing_if = "String::is_empty")] pub color: String,+ /// Written by automation: loaded, recorded, or had its steps edited while+ /// [`Session::authorize`] was installed (a control, MCP or synthetic-input request).+ /// Playback keeps automation's gate ([`Session::automation_gate`]) for it even when the user+ /// starts it (a key, the Actions panel), so a client confined to its automation roots+ /// can't plant steps that later run with the user's file access. Batch and droplets copy+ /// steps instead of the action, so they do not keep the gate yet.+ #[serde(default, skip_serializing_if = "std::ops::Not::not")]+ pub automation: bool,+}++/// Mark action `idx` as written by automation when an automation gate is installed.+pub(crate) fn note_automation_edit(s: &mut Session, idx: usize) {+ if s.authorize.is_some()+ && let Some(action) = s.actions.list.get_mut(idx)+ {+ action.automation = true;+ }+}++/// The gate to install while `action` plays: automation's, for an action automation wrote that+/// plays with none installed. `None` when the session's own gate (or none) applies.+pub fn kept_gate(s: &Session, action: &Action) -> Option<fn(&str, &Value) -> Result<()>> {+ if action.automation && s.authorize.is_none() { s.automation_gate } else { None } } /// Button Mode colours, in Photoshop's order. Empty is no colour.@@ -55,7 +78,7 @@ pub const BUTTON_COLORS: [&str; 8] = ["", "red", "orange", "yellow", "green", "b impl Action { pub fn new(name: impl Into<String>, steps: Vec<(String, Value)>) -> Self {- Self { name: name.into(), steps, disabled: BTreeSet::new(), dialogs: BTreeSet::new(), set: None, color: String::new() }+ Self { name: name.into(), steps, disabled: BTreeSet::new(), dialogs: BTreeSet::new(), set: None, color: String::new(), automation: false } } /// Apply `f` to every per-step index set (`disabled`, `dialogs`) after steps moved.@@ -330,6 +353,7 @@ fn record(s: &mut Session, p: &Value) -> Result<Value> { s.actions.list.len() - 1 }; s.actions.recording = Some((s.journal.len(), idx));+ note_automation_edit(s, idx); s.actions.take = s.actions.take.wrapping_add(1); let name = s.actions.list.get(idx).map(|a| a.name.clone()).unwrap_or_default(); Ok(json!({"action": name, "index": idx}))@@ -454,7 +478,15 @@ pub fn nested_failure(id: &str, value: &Value) -> Option<String> { fn play(s: &mut Session, p: &Value) -> Result<Value> { let (action, from) = playback_plan(s, p)?; let recording = begin_playback(s, &action)?;+ // Nested calls and commands the steps run on their own behalf see the kept gate too.+ let kept = kept_gate(s, &action);+ if kept.is_some() {+ s.authorize = kept;+ } let (ran, failed) = run_recorded(s, &action, from);+ if kept.is_some() {+ s.authorize = None;+ } finish_playback(s, recording, &action, from); Ok(match failed { Some(f) => json!({"action": action.name, "ran": ran, "failed": f}),@@ -491,6 +523,7 @@ fn move_item(s: &mut Session, p: &Value) -> Result<Value> { action.steps.insert(to, moved); action.remap_steps(|i| Some(moved_index(i, step, to))); let result = json!({"action": action.name, "step": to});+ note_automation_edit(s, idx); s.actions.touch(); Ok(result) } else {@@ -547,6 +580,7 @@ fn delete(s: &mut Session, p: &Value) -> Result<Value> { } }); let result = json!({"action": action.name, "deletedStep": step, "steps": action.steps.len()});+ note_automation_edit(s, idx); s.actions.touch(); return Ok(result); }@@ -600,6 +634,7 @@ fn toggle(s: &mut Session, p: &Value) -> Result<Value> { None => action.disabled = (0..action.steps.len()).collect(), } let result = json!({"action": action.name, "disabled": action.disabled});+ note_automation_edit(s, idx); s.actions.touch(); Ok(result) }@@ -729,6 +729,48 @@ fn play_checks_each_step_with_the_authorize_hook() { assert!(s.documents().is_empty(), "the refused step does not open a document"); } +/// An action automation loads or edits keeps automation's gate when the user plays it (its+/// key, the panel), so a client confined to its roots can't plant file steps that later run+/// with the user's access. The user's own actions, and sets the user loads, run as before.+#[test]+fn actions_automation_wrote_keep_its_gate_when_the_user_plays_them() {+ fn deny_file(id: &str, _: &Value) -> Result<()> {+ if id.starts_with("file.") { Err(EngineError::Other(format!("automation command `{id}` is disabled"))) } else { Ok(()) }+ }+ let open = || ("file.openAs".to_string(), json!({"path": "/outside/secret.png"}));+ let mut s = Session::new();+ s.automation_gate = Some(deny_file);+ s.actions.list.push(Action::new("Mine", vec![open()]));+ s.actions.list.push(Action::new("Edited", vec![("layer.new.layer".into(), json!({}))]));+ let (atn, _) = crate::atn_cmds::export("Set", &[&Action::new("Planted", vec![open()])], &[], false).unwrap();+ let data = photocraft_paint::tile::b64_encode(&atn);++ // An automation client loads a set and inserts a step into an existing action.+ s.authorize = Some(deny_file);+ s.execute("actions.load", json!({"data": data})).unwrap();+ s.execute("actions.insert", json!({"action": "Edited", "id": "file.openAs", "params": {"path": "/outside/secret.png"}, "at": 0})).unwrap();+ s.authorize = None;+ assert!(!s.actions.list[0].automation, "the user's own action is not marked");++ for name in ["Planted", "Edited"] {+ let r = s.execute("actions.play", json!({"action": name})).unwrap();+ assert_eq!(r["failed"]["id"], "file.openAs", "{name}: {r}");+ assert!(r["failed"]["error"].as_str().unwrap().contains("disabled"), "{name}: {r}");+ assert!(s.authorize.is_none(), "{name}: the kept gate is removed after playback");+ }+ // Nested: the user's action calling a planted one still meets the gate.+ s.actions.list.push(Action::new("Caller", vec![("actions.play".into(), json!({"action": "Planted"}))]));+ let r = s.execute("actions.play", json!({"action": "Caller"})).unwrap();+ assert!(r["failed"]["error"].as_str().unwrap().contains("disabled"), "{r}");++ // The user's own action runs with the user's access (the file is missing, not refused).+ let r = s.execute("actions.play", json!({"action": "Mine"})).unwrap();+ assert!(!r["failed"]["error"].as_str().unwrap().contains("disabled"), "{r}");+ // A set the user loads is theirs.+ s.execute("actions.load", json!({"data": data, "set": "Loaded by the user"})).unwrap();+ assert!(!s.actions.list.last().unwrap().automation);+}+ #[test] fn append_delete_and_from_step() { let mut s = Session::new();@@ -8,7 +8,7 @@ use serde_json::{Value, json}; -use crate::actions_cmds::{flush_recording, not_playing, pending_steps, resolve};+use crate::actions_cmds::{flush_recording, not_playing, note_automation_edit, pending_steps, resolve}; use crate::commands::CommandSpec; use crate::{EngineError, Result, Session}; @@ -69,6 +69,7 @@ fn insert(s: &mut Session, p: &Value) -> Result<Value> { action.steps.insert(at, (id.clone(), params)); action.remap_steps(|i| Some(if i >= at { i + 1 } else { i })); let result = json!({"action": action.name, "step": at, "id": id, "steps": action.steps.len()});+ note_automation_edit(s, idx); s.actions.touch(); Ok(result) }@@ -58,6 +58,11 @@ pub(super) fn load(s: &mut Session, p: &Value) -> Result<Value> { } } }+ // Actions an automation client loads keep its gate when the user plays them.+ let automation = s.authorize.is_some();+ for a in &mut added {+ a.automation = automation;+ } s.actions.list.extend(added); let keys: Vec<(String, String)> = imp.function_keys.into_iter().map(|(n, k)| (renamed.get(&n).cloned().unwrap_or(n), k)).collect(); if !keys.is_empty() {@@ -238,6 +238,12 @@ pub struct Session { /// the same check a top-level request sees. `None` runs everything, which is what a local /// UI and `photocraft-cli run` do. pub authorize: Option<fn(&str, &serde_json::Value) -> Result<()>>,+ /// The gate automation runs under in this process (the desktop's policy). An action+ /// automation wrote ([`actions_cmds::Action::automation`]) plays under it when no gate is+ /// installed, as when the user presses its key, so it never gets the user's file access.+ /// `None` when there is no automation policy to keep (headless sessions gate everything+ /// with `authorize`).+ pub automation_gate: Option<fn(&str, &serde_json::Value) -> Result<()>>, /// A further route for File › Print when neither `lp` nor a CUPS server over IPP is /// reachable (the XDG Print portal in a Flatpak); see `print_cmds::Spooler`. pub print_spooler: Option<print_cmds::Spooler>,@@ -26,6 +26,8 @@ pub use thumbs::surface_fingerprint; impl PhotocraftApp { pub fn new(mut session: Session, mut services: Services) -> Self { session.updates.check = services.check_updates.take();+ // Actions automation wrote keep this gate when the user plays them (Batch included).+ session.automation_gate = session.automation_gate.or(services.automation_authorize); let mut app = Self { session, ui: UiState::default(),@@ -101,6 +101,21 @@ pub(crate) fn play(app: &mut PhotocraftApp, params: &Value) -> Result<Value, Str /// `suspended` is the recording a paused part of the same call took ([`PausedPlayback::record`]); it comes /// back, with that named call journaled, only when the whole call ends. fn play_owned(app: &mut PhotocraftApp, params: &Value, suspended: Option<Value>) -> Result<Value, String> {+ // An action automation wrote plays under automation's gate even when the user starts it (its+ // key, the panel, Button Mode); nested calls, a paused step's dialog and the steps' own+ // commands see that gate too.+ let kept = actions_cmds::playback_plan(&app.session, params).ok().and_then(|(action, _)| actions_cmds::kept_gate(&app.session, &action));+ if kept.is_some() {+ app.session.authorize = kept;+ }+ let played = play_steps(app, params, suspended);+ if kept.is_some() {+ app.session.authorize = None;+ }+ played+}++fn play_steps(app: &mut PhotocraftApp, params: &Value, suspended: Option<Value>) -> Result<Value, String> { let (action, from) = match actions_cmds::playback_plan(&app.session, params) { Ok(plan) => plan, Err(e) => {@@ -1067,6 +1067,25 @@ fn a_paused_playback_keeps_its_automation_gate_when_the_dialog_is_confirmed() { assert!(app.ui.status.contains("select.all denied"), "{}", app.ui.status); } +/// An action automation wrote, bound to a key, plays under the desktop's automation policy+/// when the user presses the key: its file step is refused, not run with the user's access.+#[test]+fn a_users_key_plays_an_action_automation_wrote_under_its_gate() {+ fn deny_file(id: &str, _: &Value) -> photocraft_engine::Result<()> {+ if id.starts_with("file.") { Err(photocraft_engine::EngineError::Other(format!("automation command `{id}` is disabled"))) } else { Ok(()) }+ }+ let services = crate::Services { automation_authorize: Some(deny_file), ..Default::default() };+ let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), services);+ let mut planted = Action::new("Backup", vec![("file.openAs".into(), json!({"path": "/outside/secret.png"}))]);+ planted.automation = true;+ app.session.actions.list.push(planted);+ assign_shortcut(&mut app, "Backup", "F9").unwrap();+ crate::shortcut_dispatch::dispatch(&mut app, &egui::Context::default(), &shortcut_id("Backup"));+ assert!(app.session.documents().is_empty());+ assert!(app.ui.status.contains("disabled"), "the refusal is reported: {}", app.ui.status);+ assert!(app.session.authorize.is_none(), "the kept gate is removed after playback");+}+ /// The dialog names only its pause: changing its fields can't resume another action. #[test] fn a_step_dialog_resumes_only_the_playback_that_paused_it() {