orchestrator-sm: fold awaiting/failed into State variants Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
diff --git a/services/orchestrator/sm/src/lib.rs b/services/orchestrator/sm/src/lib.rs index 4c52a9d..b6b19ce 100644 --- a/services/orchestrator/sm/src/lib.rs +++ b/services/orchestrator/sm/src/lib.rs
@@ -131,7 +131,6 @@ /// trust-boundary gate: it persists across a return to `Ready` and is only /// cleared by a fresh `Rot` on `PowerOnReset`. gated: heapless::Vec<ComponentId, N>, - failed: Option<ComponentId>, /// Per-component consecutive failed-restore counts. An entry is present only /// for a component with at least one recorded attempt; absent means zero. /// Keyed by `ComponentId` so interleaved recoveries of different components @@ -141,9 +140,6 @@ /// cleared on a clean return to `Ready`. retries: heapless::Vec<(ComponentId, u8), N>, max_retry: u8, - /// The `Active` component whose iRoT readiness is outstanding. `Some` only - /// while in `AwaitingReady` (INV9). - awaiting: Option<ComponentId>, /// Ties the effect-buffer size `E` to this type (zero-sized). _effect_cap: PhantomData<[u8; E]>, } @@ -160,10 +156,8 @@ chain: chain.into_entries(), cursor: 0, gated: heapless::Vec::new(), - failed: None, retries: heapless::Vec::new(), max_retry, - awaiting: None, _effect_cap: PhantomData, } } @@ -271,10 +265,7 @@ fn handle_corruption(&mut self, id: ComponentId, ctx: &mut Sink<E>) -> Outcome { match self.gate_by_policy(ctx, id) { Gating::Gated => Outcome::Handled, - Gating::NotGated => { - self.failed = Some(id); - Outcome::Transition(State::Recovering) - } + Gating::NotGated => Outcome::Transition(State::Recovering(id)), } } @@ -341,8 +332,7 @@ if self.advance_to_next_ungated(ctx, next_idx) { match current_kind { Some(ComponentKind::Active) => { - self.awaiting = Some(*id); - Outcome::Transition(State::AwaitingReady) + Outcome::Transition(State::AwaitingReady(Some(*id))) } _ => Outcome::Handled, } @@ -354,8 +344,7 @@ // Recovery is attempted first for every failure, regardless // of the component's recovery-failure policy (CSA: recover // first, classify only once retries are exhausted). - self.failed = Some(*id); - Outcome::Transition(State::Recovering) + Outcome::Transition(State::Recovering(*id)) } // Minimal fix: react to a corruption report if one arrives, // even though `PreSupervision` isn't linked to @@ -370,19 +359,24 @@ _ => Outcome::Super, }, - State::AwaitingReady => match event { + // `awaiting` is the state's own payload, so changing it means + // re-entering the variant: `Outcome::Handled` leaves the payload + // untouched. That is behavior-identical to the old field write only + // because `AwaitingReady` has no entry action — do not add one. + State::AwaitingReady(awaiting) => match event { Event::ComponentReady(id) => { - if self.awaiting != Some(*id) { + if awaiting != Some(*id) { return Outcome::Handled; // spurious / stale (INV9) } - self.awaiting = None; // If cursor is past the end, the eRoT side of the walk has // already finished (chain done, or the remainder is held) — // nothing left to verify, we're done. if (self.cursor as usize) >= self.chain.len() { Outcome::Transition(State::Ready) } else { - Outcome::Handled + // Readiness satisfied, chain not done: stay supervised, + // now awaiting nothing. + Outcome::Transition(State::AwaitingReady(None)) } } Event::VerificationPassed(id) => { @@ -392,6 +386,9 @@ ctx.emit(Effect::ReleaseReset(*id)); let next_idx = (self.cursor as usize).saturating_add(1); if self.advance_to_next_ungated(ctx, next_idx) { + // `Handled` preserves the current payload, matching the + // old behavior: `awaiting` was only ever cleared by + // `ComponentReady` or `VerificationFailed`. Outcome::Handled } else { Outcome::Transition(State::Ready) @@ -400,9 +397,7 @@ Event::VerificationFailed(id) => { // Recovery is attempted first for every failure, regardless // of the component's recovery-failure policy. - self.failed = Some(*id); - self.awaiting = None; - Outcome::Transition(State::Recovering) + Outcome::Transition(State::Recovering(*id)) } _ => Outcome::Super, }, @@ -424,16 +419,13 @@ _ => Outcome::Super, }, - State::Recovering => match event { + State::Recovering(failed) => match event { Event::Restored(_) => { // Count this attempt against the specific component in // recovery, not a global budget (CSA: exhaustion is - // per-device). `failed` is always `Some` while in - // `Recovering`; treat a missing id as exhausted defensively. - let attempts = self - .failed - .map(|id| self.bump_retry(id)) - .unwrap_or(self.max_retry); + // per-device). The recovery target is the state's payload, + // so it is always present — no defensive fallback needed. + let attempts = self.bump_retry(failed); if attempts < self.max_retry { Outcome::Transition(State::PreSupervision) } else { @@ -441,14 +433,13 @@ // the runtime-corruption path uses, so the two can never // disagree. Gated → continue the walk; NotGated // (Required/unknown) → lock down. - match self.failed.map(|id| (id, self.gate_by_policy(ctx, id))) { - Some((id, Gating::Gated)) => { - self.clear_retry(id); - self.failed = None; + match self.gate_by_policy(ctx, failed) { + Gating::Gated => { + self.clear_retry(failed); Outcome::Transition(State::PreSupervision) } // `Required`, or an unknown/missing id: lock down. - _ => { + Gating::NotGated => { ctx.emit(Effect::Emit(Event::RecoveryFailed)); Outcome::Handled } @@ -510,17 +501,14 @@ fn entry_action(&mut self, state: State, ctx: &mut Sink<E>) { match state { State::PreSupervision => { - self.awaiting = None; let _ = self.advance_to_next_ungated(ctx, 0); } State::Updating => { ctx.emit(Effect::AuthenticateUpdate); ctx.emit(Effect::StageUpdate); } - State::Recovering => { - if let Some(failed) = self.failed { - ctx.emit(Effect::RestoreGoldenImage(failed)); - } + State::Recovering(failed) => { + ctx.emit(Effect::RestoreGoldenImage(failed)); } State::Locked => { ctx.emit(Effect::LatchLockdown); @@ -536,7 +524,6 @@ // recovery episode is in flight, so every per-component streak // resets to zero. self.retries.clear(); - self.failed = None; } _ => {} } @@ -600,7 +587,7 @@ const fn is_supervised(state: State) -> bool { matches!( state, - State::AwaitingReady | State::Ready | State::Updating | State::Recovering + State::AwaitingReady(_) | State::Ready | State::Updating | State::Recovering(_) ) }
diff --git a/services/orchestrator/sm/src/model.rs b/services/orchestrator/sm/src/model.rs index abb1a9f..bd51ee4 100644 --- a/services/orchestrator/sm/src/model.rs +++ b/services/orchestrator/sm/src/model.rs
@@ -236,19 +236,32 @@ Emit(Event), } -/// The states the machine can be in. None carry data; all mutable state lives -/// in [`Rot`](crate::Rot) shared storage. +/// The states the machine can be in. A variant carries exactly the data that is +/// meaningful only while in that state; everything that spans states (the +/// cursor, the gate set, retry counts) lives in [`Rot`](crate::Rot) shared +/// storage. #[derive(Clone, Copy, PartialEq, Eq, Debug)] #[non_exhaustive] pub enum State { PowerOnReset, PreSupervision, - /// eRoT has released an `Active` component; waiting for its iRoT to finish - /// local verification and signal [`Event::ComponentReady`]. - AwaitingReady, + /// eRoT has released an `Active` component; the payload is the component + /// whose iRoT readiness is outstanding (INV9). + /// + /// - `AwaitingReady(Some(id))` — waiting for `id`'s + /// [`Event::ComponentReady`]. + /// - `AwaitingReady(None)` — that readiness has been satisfied but the chain + /// walk is not finished; the machine stays supervised while draining the + /// remaining speculative verifications, and any further `ComponentReady` + /// is spurious. + AwaitingReady(Option<ComponentId>), Ready, Updating, - Recovering, + /// A component failed verification (or was found corrupt under a + /// non-gating policy) and its golden image is being restored. The payload is + /// that component — always present, since the machine only enters this state + /// with a recovery target in hand. + Recovering(ComponentId), Locked, }
diff --git a/services/orchestrator/sm/src/tests.rs b/services/orchestrator/sm/src/tests.rs index 1c0dd70..b4c1d46 100644 --- a/services/orchestrator/sm/src/tests.rs +++ b/services/orchestrator/sm/src/tests.rs
@@ -209,7 +209,7 @@ Event::CorruptionDetected(C0), // C0 already released, but caught anyway ], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C0)); assert!(effects.contains(&Effect::RestoreGoldenImage(C0))); } @@ -368,7 +368,7 @@ ]), &[BOOT, Event::VerificationPassed(C0)], ); - assert_eq!(state, State::AwaitingReady); + assert_eq!(state, State::AwaitingReady(Some(C0))); assert!(effects.contains(&Effect::ReleaseReset(C0))); assert!(effects.contains(&Effect::ReadFirmware(C1))); @@ -402,7 +402,7 @@ Event::ComponentReady(C1), // wrong id ], ); - assert_eq!(state, State::AwaitingReady); + assert_eq!(state, State::AwaitingReady(Some(C0))); assert!(!effects.contains(&Effect::ReleaseReset(C1))); } @@ -420,7 +420,7 @@ Event::AttestationChallenge, ], ); - assert_eq!(state, State::AwaitingReady); + assert_eq!(state, State::AwaitingReady(Some(C0))); assert_eq!(effects.last(), Some(&Effect::SignAttestation)); } @@ -593,7 +593,7 @@ Event::CorruptionDetected(C0), // required → Recovering ], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C0)); assert!(effects.contains(&Effect::RestoreGoldenImage(C0))); } @@ -636,7 +636,7 @@ passive_required(&[C0, C1]), &[BOOT, Event::VerificationFailed(C0)], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C0)); assert!(effects.contains(&Effect::RestoreGoldenImage(C0))); // Component must never be released when its eRoT check failed. assert!(!effects.contains(&Effect::ReleaseReset(C0))); @@ -676,7 +676,7 @@ Event::VerificationFailed(C1), // required → Recovering ], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C1)); assert!(effects.contains(&Effect::RestoreGoldenImage(C1))); assert!(!effects.contains(&Effect::ReleaseReset(C1))); } @@ -696,7 +696,7 @@ Event::CorruptionDetected(C0), ], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C0)); assert!(effects.contains(&Effect::RestoreGoldenImage(C0))); } @@ -713,7 +713,7 @@ Event::CorruptionDetected(C0), ], ); - assert_eq!(state, State::Recovering); + assert_eq!(state, State::Recovering(C0)); assert!(effects.contains(&Effect::RestoreGoldenImage(C0))); } @@ -839,7 +839,7 @@ Effect::VerifyFirmware(C1), ], ); - assert_eq!(orch.state(), State::AwaitingReady); + assert_eq!(orch.state(), State::AwaitingReady(Some(C0))); } /// A chain with a single Active component goes directly to Ready on