Automation: an allowed command can't run a denied one on its behalf (#861)
Untrusted sessions (MCP, the control channel) checked only the top-level command and the steps of `actions.play`. Any other command that runs commands internally ran them unchecked, so the allowed `file.automate.conditionalModeChange` could run `image.mode.*`, which the desktop gate denies because it resolves the user's configured colour profiles. `Session::dispatch` now applies `Session::authorize` to every command before any side effect, nested ones included, which closes the class rather than this one command. Sessions without a gate (local UI, `photocraft-cli run`) are unchanged.
This commit is contained in:
parent
eec4af6551
commit
62b0d0bd60
|
|
@ -33,7 +33,9 @@ Automation is a privilege boundary because requests can cause filesystem access,
|
|||
are rejected before file effects.
|
||||
- Engine commands that still use ambient paths fail closed at the automation boundary. Synthetic
|
||||
UI input reaches the same command policy, and automation-triggered file hooks do not run
|
||||
user-configured script-event paths.
|
||||
user-configured script-event paths. The policy also covers every command a command runs on its
|
||||
own behalf (an action's steps, `file.automate.conditionalModeChange` running `image.mode.*`), so
|
||||
an allowed command can't reach a denied one.
|
||||
- `prefs.set` applies the engine's dotted-path normalization before checking for filesystem-bearing
|
||||
preference sections, including paths with leading or repeated dots. It refuses whole-preferences
|
||||
updates and those sections; individual safe preference keys remain available.
|
||||
|
|
|
|||
|
|
@ -460,6 +460,14 @@ mod tests {
|
|||
assert!(authorize_engine_command("prefs.set", &serde_json::json!({"path": "colorSettings.workingRgb", "value": "outside.icc"})).is_err());
|
||||
assert!(authorize_engine_step("file.open", &serde_json::json!({})).is_err());
|
||||
assert!(authorize_engine_step("actions.play", &serde_json::json!({})).is_ok());
|
||||
// An allowed command can't reach a denied one by running it on its own behalf.
|
||||
let mut session = photocraft_engine::Session::new();
|
||||
session.execute("file.new", serde_json::json!({"width": 4, "height": 4})).unwrap();
|
||||
session.authorize = Some(authorize_desktop_engine_step);
|
||||
let params = serde_json::json!({"to": "grayscale"});
|
||||
assert!(authorize_desktop_engine_command("file.automate.conditionalModeChange", ¶ms).is_ok());
|
||||
assert!(session.execute("file.automate.conditionalModeChange", params).is_err());
|
||||
assert_eq!(session.active().unwrap().doc.mode, photocraft_engine::doc::ColorMode::Rgb);
|
||||
assert!(authorize_desktop_engine_step("file.saveACopy", &serde_json::json!({})).is_err());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -239,6 +239,21 @@ fn fit_image_and_conditional_mode_change() {
|
|||
assert!(s.execute("file.automate.conditionalModeChange", json!({"to": "duotone"})).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn conditional_mode_change_cannot_run_a_mode_change_the_gate_denies() {
|
||||
fn deny_mode(id: &str, _: &Value) -> Result<()> {
|
||||
if id.starts_with("image.mode.") { Err(EngineError::Other(format!("automation command `{id}` is disabled"))) } else { Ok(()) }
|
||||
}
|
||||
let mut s = session(4, 4, 8);
|
||||
s.authorize = Some(deny_mode);
|
||||
let e = s.execute("file.automate.conditionalModeChange", json!({"to": "grayscale"})).unwrap_err();
|
||||
assert!(e.to_string().contains("image.mode.grayscale"), "{e}");
|
||||
assert_eq!(doc(&s).mode, ColorMode::Rgb, "the refused nested step leaves the document alone");
|
||||
// Commands that compose only allowed steps still run.
|
||||
s.execute("file.automate.fitImage", json!({"width": 2, "height": 2})).unwrap();
|
||||
assert_eq!(doc(&s).size.width, 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flatten_all_layer_effects_and_masks() {
|
||||
for depth in [8, 16, 32] {
|
||||
|
|
|
|||
|
|
@ -594,6 +594,12 @@ impl Session {
|
|||
if !(params.is_object() || params.is_null()) {
|
||||
return Err(EngineError::BadParams { cmd: id.to_string(), msg: "params must be a JSON object".into() });
|
||||
}
|
||||
// Untrusted sessions gate every command, including the ones a command runs on its own
|
||||
// behalf (`file.automate.conditionalModeChange` runs `image.mode.*`), before any side
|
||||
// effect.
|
||||
if let Some(gate) = self.authorize {
|
||||
gate(id, ¶ms)?;
|
||||
}
|
||||
// A floating selection drops before any other command (Undo puts it back instead).
|
||||
if let Some(v) = crate::float_cmds::before_command(self, id)? {
|
||||
return Ok(Started::Done(v));
|
||||
|
|
|
|||
|
|
@ -303,8 +303,9 @@ pub struct Session {
|
|||
pub preset_store: Option<preset_store::PresetStore>,
|
||||
/// Window › Actions. The list persists with the preset store when one is attached.
|
||||
pub actions: actions_cmds::ActionState,
|
||||
/// Per-step gate for `actions.play`. Untrusted sessions (MCP, the control channel) install
|
||||
/// the same check a top-level command sees. `None` runs every step, which is what a local
|
||||
/// Gate for every command the session runs, including the ones a command runs on its own
|
||||
/// behalf and `actions.play` steps. Untrusted sessions (MCP, the control channel) install
|
||||
/// 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<()>>,
|
||||
/// Background jobs (see [`jobs`]).
|
||||
|
|
|
|||
Loading…
Reference in New Issue