sm: Walk the platform again after an update activates

docs/src/design/orchestrator/orchestrator-machine.md

@@ -25,7 +25,7 @@ stateDiagram-v2
 
     state SupervisingPlatform {
         Ready         --> Updating      : UpdateRequest(id)<br/>/ AuthenticateStageUpdate
-        Updating      --> Ready         : UpdateVerified / ActivateUpdate
+        Updating      --> PreSupervision : UpdateVerified / ActivateUpdate
         Updating      --> Ready         : UpdateRejected / DiscardStaged
         Ready         --> Recovering    : CorruptionDetected<br/>/ RestoreGoldenImage
         Updating      --> Recovering    : CorruptionDetected<br/>/ RestoreGoldenImage
@@ -116,6 +116,8 @@ here — they persist across re-walks so exhausted components are not re-verifie
 | `VerificationFailed(id)` | — | — | `Recovering(id)` — recovery is attempted first, regardless of the component's recovery-failure policy |
 | `CorruptionDetected(id)` | `Required`/unknown | `RestoreGoldenImage` | `Recovering(id)` |
 | `CorruptionDetected(id)` | `Isolable`/`Cascading` | `AssertReset` · `ReportIsolated` | `Handled` (component gated; walk continues) |
+| `CommitTimeout` | `pending_commit` set | — | `Locked` (commit window spans the post-activation walk; `LatchLockdown` on entry) |
+| `CommitTimeout` | `pending_commit` clear | — | `Handled` (no window open, nothing to enforce) |
 | anything else | — | — | `Outcome::Super` (top level — discarded) |
 
 When advancing the cursor, any component marked `Isolated` is skipped without
@@ -203,7 +205,7 @@ state's payload.
 
 | Event | Guard | Effects | Next state |
 |---|---|---|---|
-| `UpdateVerified` | — | `ActivateUpdate` | `Ready` (commit window opens on the payload) |
+| `UpdateVerified` | — | `ActivateUpdate` | `PreSupervision` (re-walk; commit window spans the walk) |
 | `UpdateRejected` | — | `DiscardStaged` | `Ready` (INV4) |
 | `CorruptionDetected(id)` | `Required`/unknown | `DiscardStaged` (then `RestoreGoldenImage` on entry) | `Recovering(id)` (update preempted; staged image discarded) |
 | `CorruptionDetected(id)` | `Isolable`/`Cascading` | `AssertReset(id)` · `ReportIsolated(id)` | `Handled` (component gated; update continues, staged image kept) |
@@ -333,6 +335,8 @@ handler (`handle_supervising`).
 | `AttestationChallenge` | — | `SignAttestation` | `Handled` (no transition — INV6) |
 | `CorruptionDetected(id)` | `attrs.failure_policy == Required` | — | `Recovering(id)` (INV5) |
 | `CorruptionDetected(id)` | `attrs.failure_policy != Required` | `AssertReset(id)` · `ReportIsolated(id)` | `Handled` (component gated; machine stays in current state) |
+| `CommitTimeout` | `pending_commit` set | — | `Locked` (the post-activation walk passes through supervised states too; `LatchLockdown` on entry) |
+| `CommitTimeout` | `pending_commit` clear | — | `Handled` |
 | anything else | — | — | `Outcome::Super` (discarded) |
 
 ---

services/orchestrator/driver/src/tests.rs

@@ -170,13 +170,17 @@ impl core::error::Error for ResetFault {}
 /// line after the control moves into the driver.
 struct MockReset {
     held: std::rc::Rc<core::cell::Cell<bool>>,
+    /// Times the line was asserted, so a test can tell one boot from a
+    /// second one.
+    holds: std::rc::Rc<core::cell::Cell<usize>>,
     fail: bool,
 }
 
 impl MockReset {
     fn new() -> Self {
         Self {
             held: std::rc::Rc::new(core::cell::Cell::new(true)),
+            holds: std::rc::Rc::new(core::cell::Cell::new(0)),
             fail: false,
         }
     }
@@ -189,6 +193,7 @@ impl orchestrator_capabilities::BootControl for MockReset {
         if self.fail {
             return Err(ResetFault);
         }
+        self.holds.set(self.holds.get() + 1);
         self.held.set(true);
         Ok(())
     }
@@ -1972,30 +1977,43 @@ fn activate_update_ends_the_job() {
     assert_eq!(driver.pending_update(), None);
 }
 
-// The path through the SM up to Staged: a request, the pump, staging
-// completes, and the pump parks. The full path through activation
-// requires the crypto verify-client (emitting UpdateVerified).
+// The path through the SM end to end: a request, the pump, staging
+// completes, the verdict activates, and the walk that follows resets the
+// component into what it just activated.
+//
+// The verdict is dispatched here rather than read off the pump, which
+// parks at Staged until the crypto verify client is wired.
 #[test]
-fn an_update_stages_through_the_sm() {
+fn an_update_runs_through_the_sm_and_rewalks() {
     let mut orch = orchestrator();
     let mut driver = update_driver(MockUpdatable::stepping(2));
     orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned));
     assert_eq!(orch.state(), State::Ready);
 
+    let resets_before = driver.board().boot_controls[0].holds.get();
+
     request_update(&mut orch, &mut driver, C0, CANDIDATE_LEN).unwrap();
     assert_eq!(orch.state(), State::Updating(C0));
 
     for tick in 0..16 {
-        if let Some(event) = driver.pump_update(tick).event {
-            orch.dispatch(&mut driver, event);
+        driver.pump_update(tick);
+        if driver.board().updatables[0].ready {
             break;
         }
     }
+    assert!(driver.board().updatables[0].ready, "never staged");
 
-    // Pump parked at Staged, no event emitted. SM stays in Updating.
-    assert_eq!(orch.state(), State::Updating(C0));
-    assert!(driver.board().updatables[0].ready);
-    assert!(driver.pending_update().is_some());
+    orch.dispatch(&mut driver, Event::UpdateVerified);
+
+    assert_eq!(orch.state(), State::Ready);
+    assert!(driver.board().updatables[0].active);
+    assert_eq!(driver.pending_update(), None);
+    // The activation only proposed the image; the walk that followed is
+    // what reset the device into it.
+    assert!(
+        driver.board().boot_controls[0].holds.get() > resets_before,
+        "the updated component was never reset"
+    );
 }
 
 // The rejection path through the SM: DiscardStaged runs and the platform

services/orchestrator/sm/src/rot.rs

@@ -43,10 +43,21 @@ pub struct Rot<const N: usize, const E: usize> {
     /// [`Event::CommitTimeout`] while it is still set goes to
     /// [`State::Locked`].
     ///
-    /// Also cleared on entry to [`State::Updating`] (a newer update replaces
-    /// this one) and [`State::Recovering`] (the running image is now suspect),
-    /// the two ways to leave `Ready` while still running. Not cleared on
-    /// `Ready` entry, because activation sets it on the way in.
+    /// The window spans the walk that follows an activation: the device only
+    /// boots the candidate once it is reset, so the window cannot close before
+    /// that walk finishes. Every state it passes through answers
+    /// [`Event::CommitTimeout`], including the unsupervised
+    /// [`State::PreSupervision`], or a fire during the walk would be dropped
+    /// and never come again. In a multi-component chain, the updated device
+    /// can boot (and confirm) while the walk is still verifying later
+    /// neighbours; `BootConfirmed` is only handled in `Ready`, so the
+    /// emitter must re-raise or hold it until the walk finishes.
+    ///
+    /// Cleared on the matching [`Event::BootConfirmed`] (the window closes
+    /// normally) and on entry to the two states that end the window by leaving
+    /// `Ready` while still running, [`State::Updating`] (a superseding update)
+    /// and [`State::Recovering`] (the running image is now suspect). Not cleared
+    /// on `Ready` entry, because activation sets it on the way in.
     pending_commit: Option<ComponentId>,
     /// Ties the effect-buffer size `E` to this type (zero-sized).
     _effect_cap: PhantomData<[u8; E]>,
@@ -394,6 +405,18 @@ impl<const N: usize, const E: usize> Rot<N, E> {
 
             // Cursor walk via Outcome::Handled — a self-transition would reset cursor.
             State::PreSupervision => match event {
+                // The commit window can span this walk: an activation enters
+                // `PreSupervision` with the window open. `PreSupervision` is
+                // unsupervised, so without this match arm the watchdog fire would be
+                // dropped and never come again, leaving commit-or-lock
+                // unenforced for the length of a boot.
+                Event::CommitTimeout => {
+                    if self.pending_commit.is_some() {
+                        Outcome::Transition(State::Locked)
+                    } else {
+                        Outcome::Handled
+                    }
+                }
                 Event::VerificationPassed(id) => {
                     // Only the component currently under verification
                     // (`chain[cursor]`, whose `VerifyFirmware` was just emitted)
@@ -617,7 +640,15 @@ impl<const N: usize, const E: usize> Rot<N, E> {
                     // driver arms its commit watchdog on `ActivateUpdate`, and
                     // `CommitTimeout` bounds this window (commit-or-lock).
                     self.pending_commit = Some(target);
-                    Outcome::Transition(State::Ready)
+                    // Re-walk rather than returning to `Ready`. Activation only
+                    // proposes the image; the device runs the old one until it
+                    // is reset, and `PreSupervision` entry quiesces every live
+                    // component before verifying at rest. That reset is what
+                    // boots the candidate, and the walk's `VerifyFirmware` is
+                    // what records its SVN, which the floor commit then takes.
+                    // Without it a `BootConfirmed` would commit the previous
+                    // image's SVN and leave the downgrade window open.
+                    Outcome::Transition(State::PreSupervision)
                 }
                 Event::UpdateRejected => {
                     ctx.emit(Effect::DiscardStaged);
@@ -769,6 +800,16 @@ impl<const N: usize, const E: usize> Rot<N, E> {
                 ctx.emit(Effect::ReportUpdateDeferred);
                 Outcome::Handled
             }
+            // Same window, same reason as `PreSupervision`'s match arm: the walk
+            // that follows an activation passes through the supervised
+            // states too, and a dropped watchdog fire never returns.
+            Event::CommitTimeout => {
+                if self.pending_commit.is_some() {
+                    Outcome::Transition(State::Locked)
+                } else {
+                    Outcome::Handled
+                }
+            }
             Event::EffectFailed => Outcome::Transition(State::Locked),
             _ => Outcome::Super,
         }

services/orchestrator/sm/src/tests.rs

@@ -1979,6 +1979,9 @@ fn an_unrelated_boot_confirmed_leaves_the_commit_window_open() {
             Event::VerificationPassed(C1),
             Event::UpdateRequest(C0),
             Event::UpdateVerified,
+            // The re-walk has to finish before the confirm can land.
+            Event::VerificationPassed(C0),
+            Event::VerificationPassed(C1),
             Event::BootConfirmed(C1),
             Event::CommitTimeout,
         ],
@@ -2010,6 +2013,9 @@ fn a_sibling_boot_confirmed_does_not_consume_the_window() {
             Event::VerificationPassed(C1),
             Event::UpdateRequest(C0),
             Event::UpdateVerified,
+            // The re-walk has to finish before the confirm can land.
+            Event::VerificationPassed(C0),
+            Event::VerificationPassed(C1),
             Event::BootConfirmed(C1),
             Event::BootConfirmed(C0),
         ],
@@ -2062,12 +2068,98 @@ fn update_verified_activates_update() {
             Event::UpdateVerified,
         ],
     );
-    assert_eq!(state, State::Ready);
+    // Activation only proposes the image. The device runs the old one
+    // until the walk resets it, so the machine walks rather than
+    // returning to Ready.
+    assert_eq!(state, State::PreSupervision);
     assert!(effects.contains(&Effect::ActivateUpdate));
     assert!(!effects.contains(&Effect::DiscardStaged));
     assert!(!effects.contains(&Effect::RecoverComponent { id: C0, attempt: 0 }));
 }
 
+/// The re-walk is what boots the candidate: the activated component is
+/// quiesced, verified at rest, and released again. Its `VerifyFirmware`
+/// is also what records the new image's SVN for a later floor commit.
+#[test]
+fn an_activation_resets_and_re_verifies_the_component() {
+    let (effects, state) = drive(
+        passive_required(&[C0]),
+        &[
+            BOOT,
+            Event::VerificationPassed(C0),
+            Event::UpdateRequest(C0),
+            Event::UpdateVerified,
+            Event::VerificationPassed(C0),
+        ],
+    );
+
+    assert_eq!(state, State::Ready);
+    let activated = effects
+        .iter()
+        .position(|e| *e == Effect::ActivateUpdate)
+        .expect("never activated");
+    let after = &effects[activated..];
+    assert!(after.contains(&Effect::AssertReset(C0)), "never reset");
+    assert!(
+        after.contains(&Effect::VerifyFirmware(C0)),
+        "never re-verified"
+    );
+    assert!(after.contains(&Effect::ReleaseReset(C0)), "never released");
+}
+
+/// The walk that follows an activation passes through the supervised
+/// states too, and the watchdog has to be answered there as well. An
+/// active component parks the walk in `AwaitingReady` until its iRoT
+/// reports, which is where this fire lands.
+#[test]
+fn a_commit_timeout_while_the_post_update_walk_awaits_readiness_latches_locked() {
+    // Two components, so the walk moves on to C1 and parks in
+    // AwaitingReady(C0) instead of finishing in Ready.
+    let (effects, state) = drive(
+        chain(&[
+            (C0, ComponentAttrs::active_required()),
+            (C1, ComponentAttrs::passive_required()),
+        ]),
+        &[
+            BOOT,
+            Event::VerificationPassed(C0),
+            Event::ComponentReady(C0),
+            Event::VerificationPassed(C1),
+            Event::UpdateRequest(C0),
+            Event::UpdateVerified,
+            // The re-walk releases C0 and moves to C1, so the machine is
+            // in AwaitingReady when the watchdog fires: the supervising
+            // handler is what has to answer it.
+            Event::VerificationPassed(C0),
+            Event::CommitTimeout,
+        ],
+    );
+
+    assert_eq!(state, State::Locked);
+    assert!(effects.contains(&Effect::LatchLockdown));
+    assert!(!effects.contains(&Effect::CommitSvnFloor(C0)));
+}
+
+/// A spurious watchdog fire during an ordinary boot walk, with no update
+/// activated, must not brick the boot: there is no window to fail closed
+/// on, so the walk carries on.
+#[test]
+fn a_commit_timeout_during_a_plain_boot_walk_is_ignored() {
+    let (effects, state) = drive(
+        passive_required(&[C0]),
+        &[
+            BOOT,
+            // Mid-walk: C0 is verified but the machine has not left
+            // PreSupervision yet.
+            Event::CommitTimeout,
+            Event::VerificationPassed(C0),
+        ],
+    );
+
+    assert_eq!(state, State::Ready);
+    assert!(!effects.contains(&Effect::LatchLockdown));
+}
+
 /// The anti-rollback floor is committed only on a proven-healthy boot, never
 /// at activation. `UpdateVerified` activates the image (authentication) but
 /// must NOT emit `CommitSvnFloor`; a later `BootConfirmed` (the runtime health
@@ -2085,7 +2177,7 @@ fn svn_floor_commits_on_boot_confirmed_not_on_activation() {
             Event::UpdateVerified,
         ],
     );
-    assert_eq!(activated_state, State::Ready);
+    assert_eq!(activated_state, State::PreSupervision);
     assert!(activated.contains(&Effect::ActivateUpdate));
     assert!(!activated.contains(&Effect::CommitSvnFloor(C0)));
 
@@ -2097,6 +2189,9 @@ fn svn_floor_commits_on_boot_confirmed_not_on_activation() {
             Event::VerificationPassed(C0),
             Event::UpdateRequest(C0),
             Event::UpdateVerified,
+            // The walk the activation started has to finish: the floor
+            // commit takes the SVN that walk verified.
+            Event::VerificationPassed(C0),
             Event::BootConfirmed(C0),
         ],
     );
@@ -2117,7 +2212,9 @@ fn commit_timeout_while_pending_latches_locked() {
             Event::VerificationPassed(C0),
             Event::UpdateRequest(C0),
             Event::UpdateVerified,
-            // Window open: activated, awaiting BootConfirmed. Watchdog fires.
+            // Window open and the re-walk still running: the watchdog
+            // fires in PreSupervision, which is unsupervised, so the match arm
+            // has to be there or the fire is dropped for good.
             Event::CommitTimeout,
         ],
     );
@@ -2140,6 +2237,7 @@ fn commit_timeout_after_confirm_is_stale_noop() {
             Event::VerificationPassed(C0),
             Event::UpdateRequest(C0),
             Event::UpdateVerified,
+            Event::VerificationPassed(C0),
             Event::BootConfirmed(C0),
             // Window already closed by the commit above.
             Event::CommitTimeout,