From 1d4372c6c3bc2050e4ab76d1e0a26a551d15ec40 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Mon, 25 May 2026 12:56:38 -0400 Subject: [PATCH] Harden state transition errors --- src/state_machine.rs | 163 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 144 insertions(+), 19 deletions(-) diff --git a/src/state_machine.rs b/src/state_machine.rs index 9dca0b4..ba86216 100644 --- a/src/state_machine.rs +++ b/src/state_machine.rs @@ -28,20 +28,42 @@ pub enum CommitmentAction { RecoverExplicitly, } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum TransitionError { PolicyRejected, - IllegalRuntimeTransition, - IllegalCommitmentTransition, + IllegalRuntimeTransition { + current: RuntimeState, + action: RuntimeAction, + }, + IllegalCommitmentTransition { + current: CommitmentState, + action: CommitmentAction, + }, + InvalidAdminOverrideExitTarget { + target: RuntimeState, + }, } impl std::fmt::Display for TransitionError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { TransitionError::PolicyRejected => write!(f, "policy was rejected"), - TransitionError::IllegalRuntimeTransition => write!(f, "illegal runtime transition"), - TransitionError::IllegalCommitmentTransition => { - write!(f, "illegal commitment transition") + TransitionError::IllegalRuntimeTransition { current, action } => { + write!( + f, + "illegal runtime transition from {:?} via {:?}", + current, action + ) + } + TransitionError::IllegalCommitmentTransition { current, action } => { + write!( + f, + "illegal commitment transition from {:?} via {:?}", + current, action + ) + } + TransitionError::InvalidAdminOverrideExitTarget { target } => { + write!(f, "invalid admin override exit target {:?}", target) } } } @@ -49,6 +71,12 @@ impl std::fmt::Display for TransitionError { impl std::error::Error for TransitionError {} +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct CompositeTransition { + pub runtime_state: RuntimeState, + pub commitment_state: CommitmentState, +} + pub fn transition_runtime( current: RuntimeState, action: RuntimeAction, @@ -58,8 +86,18 @@ pub fn transition_runtime( match (current, action) { (Locked, EnterPlanning) => Ok(Planning), - (Planning, Activate { policy_accepted: true }) => Ok(Active), - (Planning, Activate { policy_accepted: false }) => Err(TransitionError::PolicyRejected), + ( + Planning, + Activate { + policy_accepted: true, + }, + ) => Ok(Active), + ( + Planning, + Activate { + policy_accepted: false, + }, + ) => Err(TransitionError::PolicyRejected), (Planning, CancelOrTimeout) => Ok(Locked), (Active, StartTransition) => Ok(Transition), (Active, CompleteForReview) => Ok(Review), @@ -71,8 +109,13 @@ pub fn transition_runtime( (Review, ContinuePlanning) => Ok(Planning), (Review, EndWorkPeriod) => Ok(Locked), (_, EnterAdminOverride) => Ok(AdminOverride), + (AdminOverride, ExitAdminOverride(AdminOverride)) => { + Err(TransitionError::InvalidAdminOverrideExitTarget { + target: AdminOverride, + }) + } (AdminOverride, ExitAdminOverride(next)) => Ok(next), - _ => Err(TransitionError::IllegalRuntimeTransition), + _ => Err(TransitionError::IllegalRuntimeTransition { current, action }), } } @@ -83,7 +126,7 @@ pub fn transition_commitment( use CommitmentAction::*; use CommitmentState::*; - match (current, action) { + match (current.clone(), action) { (Draft, Activate) => Ok(Active), (Active, PauseForTransition) => Ok(Paused), (Paused, ReturnFromPause) => Ok(Active), @@ -93,10 +136,24 @@ pub fn transition_commitment( (Paused, Abandon) => Ok(Abandoned), (Violated, RecoverExplicitly) => Ok(Active), (Violated, Abandon) => Ok(Abandoned), - _ => Err(TransitionError::IllegalCommitmentTransition), + _ => Err(TransitionError::IllegalCommitmentTransition { current, action }), } } +pub fn start_transition( + runtime_state: RuntimeState, + commitment_state: CommitmentState, +) -> Result { + let next_runtime = transition_runtime(runtime_state, RuntimeAction::StartTransition)?; + let next_commitment = + transition_commitment(commitment_state, CommitmentAction::PauseForTransition)?; + + Ok(CompositeTransition { + runtime_state: next_runtime, + commitment_state: next_commitment, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -141,14 +198,42 @@ mod tests { #[test] fn locked_cannot_directly_become_active() { + let err = transition_runtime( + RuntimeState::Locked, + RuntimeAction::Activate { + policy_accepted: true, + }, + ) + .unwrap_err(); + assert_eq!( - transition_runtime( - RuntimeState::Locked, - RuntimeAction::Activate { + err, + TransitionError::IllegalRuntimeTransition { + current: RuntimeState::Locked, + action: RuntimeAction::Activate { policy_accepted: true, - } - ), - Err(TransitionError::IllegalRuntimeTransition) + }, + } + ); + assert_eq!( + err.to_string(), + "illegal runtime transition from Locked via Activate { policy_accepted: true }" + ); + } + + #[test] + fn admin_override_cannot_exit_to_admin_override() { + let err = transition_runtime( + RuntimeState::AdminOverride, + RuntimeAction::ExitAdminOverride(RuntimeState::AdminOverride), + ) + .unwrap_err(); + + assert_eq!( + err, + TransitionError::InvalidAdminOverrideExitTarget { + target: RuntimeState::AdminOverride, + } ); } @@ -161,9 +246,49 @@ mod tests { ), Ok(CommitmentState::Active) ); + let err = + transition_commitment(CommitmentState::Violated, CommitmentAction::ReturnFromPause) + .unwrap_err(); + assert_eq!( - transition_commitment(CommitmentState::Violated, CommitmentAction::ReturnFromPause), - Err(TransitionError::IllegalCommitmentTransition) + err, + TransitionError::IllegalCommitmentTransition { + current: CommitmentState::Violated, + action: CommitmentAction::ReturnFromPause, + } + ); + assert_eq!( + err.to_string(), + "illegal commitment transition from Violated via ReturnFromPause" + ); + } + + #[test] + fn start_transition_moves_runtime_and_commitment_together() { + assert_eq!( + start_transition(RuntimeState::Active, CommitmentState::Active), + Ok(CompositeTransition { + runtime_state: RuntimeState::Transition, + commitment_state: CommitmentState::Paused, + }) + ); + } + + #[test] + fn start_transition_fails_before_returning_partial_state() { + assert_eq!( + start_transition(RuntimeState::Active, CommitmentState::Draft), + Err(TransitionError::IllegalCommitmentTransition { + current: CommitmentState::Draft, + action: CommitmentAction::PauseForTransition, + }) + ); + assert_eq!( + start_transition(RuntimeState::Locked, CommitmentState::Active), + Err(TransitionError::IllegalRuntimeTransition { + current: RuntimeState::Locked, + action: RuntimeAction::StartTransition, + }) ); } }