security: budget nested action playback and preserve batch auth

This commit is contained in:
Dylan M. Taylor
2026-10-22 07:12:21 -04:00
parent 81c694a3f9
commit 5338f1de46
3 changed files with 41 additions and 1 deletions
+34
View File
@@ -81,6 +81,8 @@ pub struct ActionsFile {
pub actions: Vec<Action>,
}
pub const MAX_PLAYBACK_STEPS: u64 = 100_000;
/// Actions list and the recording / playback flags. Only `list` is persisted.
#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)]
#[serde(default)]
@@ -95,6 +97,9 @@ pub struct ActionState {
/// Active action names, for cycle detection in engine and desktop playback.
#[serde(skip)]
pub playback_stack: Vec<String>,
/// Number of steps executed by the current top-level playback tree.
#[serde(skip)]
pub playback_steps: u64,
/// Bumped whenever `list` changes, so the preset store can skip unchanged writes.
#[serde(skip)]
pub rev: u64,
@@ -143,6 +148,15 @@ pub fn save_target_closed(s: &Session, id: &str, target: Option<photocraft_doc::
})
}
/// Charge one step against the complete nested playback tree.
pub fn consume_playback_step(s: &mut Session) -> bool {
if s.actions.playback_steps >= MAX_PLAYBACK_STEPS {
false
} else {
s.actions.playback_steps += 1;
true
}
}
/// View-only menu commands recorded by the desktop shell. Headless playback reports
/// them as unsupported instead of silently dropping them; Batch and droplets skip them.
pub fn shell_view_command(id: &str) -> bool {
@@ -348,6 +362,10 @@ fn run_recorded(s: &mut Session, action: &Action, from: usize) -> (u64, Option<V
let mut ran = 0u64;
let mut failed = None;
for (i, (id, params)) in action.enabled_steps().skip_while(|(i, _)| *i < from) {
if !consume_playback_step(s) {
failed = Some(json!({"step": i, "id": id, "error": format!("action playback exceeded the {}-step limit", MAX_PLAYBACK_STEPS)}));
break;
}
if let Some(auth) = s.authorize
&& let Err(e) = auth(id, params)
{
@@ -379,6 +397,9 @@ fn run_recorded(s: &mut Session, action: &Action, from: usize) -> (u64, Option<V
/// for diagnostics, but record just the named call in the parent action.
pub fn begin_playback(s: &mut Session, action: &Action) -> Result<Option<(usize, usize)>> {
flush_recording(s)?;
if s.actions.playing == 0 {
s.actions.playback_steps = 0;
}
let recording = s.actions.recording.take();
s.actions.playing = s.actions.playing.saturating_add(1);
s.actions.playback_stack.push(action.name.clone());
@@ -1583,3 +1604,16 @@ mod tests {
assert!(s.actions.list[0].color.is_empty(), "rejected colours change nothing");
}
}
#[cfg(test)]
mod security_tests {
use super::*;
#[test]
fn playback_budget_is_shared_by_nested_calls() {
let mut session = Session::new();
session.actions.playback_steps = MAX_PLAYBACK_STEPS - 1;
assert!(consume_playback_step(&mut session));
assert!(!consume_playback_step(&mut session));
}
}
+3 -1
View File
@@ -1259,7 +1259,9 @@ fn batch(s: &mut Session, p: &Value) -> Result<Value> {
let actions = crate::actions_cmds::ActionState { list, playing: s.actions.playing, playback_stack: s.actions.playback_stack.clone(), ..Default::default() };
let names = batch_naming::plan(&inputs, &output, &format, p)?;
let save = SaveOpts::from_params(s, p, cmd)?;
let run = |scratch: &mut Session| {
let authorize = s.authorize;
let run = move |scratch: &mut Session| {
scratch.authorize = authorize;
scratch.actions = actions.clone();
for (id, params) in &steps {
let r = scratch.execute(id, params.clone())?;
+4
View File
@@ -101,6 +101,10 @@ pub(crate) fn play(app: &mut PhotocraftApp, params: &Value) -> Result<Value, Str
let mut failed = None;
let mut paused = None;
for (i, (id, p)) in action.enabled_steps().skip_while(|(i, _)| *i < from) {
if !actions_cmds::consume_playback_step(&mut app.session) {
failed = Some(json!({"step": i, "id": id, "error": format!("action playback exceeded the {}-step limit", actions_cmds::MAX_PLAYBACK_STEPS)}));
break;
}
// The dialog toggle: show the step's dialog with its recorded values and continue
// from the next step when it is confirmed (`resume`). Agents play headlessly.
if action.dialogs.contains(&i) && !app.automation_input && (crate::filter_dialog::has_dialog(id) || crate::adjust_dialog::has_dialog(id)) {