From 67db863ce5d7d74e10c44de7d23c35e82482abe5 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Tue, 25 Aug 2026 18:15:56 +0200 Subject: [PATCH 1/2] orchestrator: Add the Recovery restore capability trait One managed device's restore mechanism: rewrite the active image from the board-configured recovery source. Ok means the mechanism completed, not that the image is good. The verifier judges the restored image on the re-walk, so a restore must not check it here. Errors are actuation faults only and are treated fail-closed. restore takes the attempt count from Effect::RecoverComponent so an implementor holding several sources can pick a different one each try. A count kept by the device would drift, because it never sees which attempt succeeded. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/BUILD.bazel | 1 + services/orchestrator/capabilities/src/lib.rs | 6 + .../orchestrator/capabilities/src/recovery.rs | 153 ++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 services/orchestrator/capabilities/src/recovery.rs diff --git a/services/orchestrator/capabilities/BUILD.bazel b/services/orchestrator/capabilities/BUILD.bazel index cf554a67..7b2eda57 100644 --- a/services/orchestrator/capabilities/BUILD.bazel +++ b/services/orchestrator/capabilities/BUILD.bazel @@ -11,6 +11,7 @@ rust_library( "src/evidence.rs", "src/lib.rs", "src/lockdown_latch.rs", + "src/recovery.rs", "src/svn_floor.rs", "src/trial_boot.rs", "src/updatable.rs", diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index 2f2ab009..8190cb3f 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -30,6 +30,10 @@ //! `BootWatch` is the seam the orchestrator polls: one device's boot walk, //! erased of every device-specific type, answering with a `WalkVerdict`. //! +//! `Recovery` is the restore capability: rewrite one device's active image +//! from its board-configured recovery source, mechanism unnamed, source +//! chosen per attempt. +//! //! `LockdownLatch` is the terminal capability: latch the platform into its safe //! state, one-way, at the top of the escalation ladder. //! @@ -47,6 +51,7 @@ mod boot_control; mod boot_watch; mod evidence; mod lockdown_latch; +mod recovery; mod svn_floor; mod trial_boot; mod updatable; @@ -55,6 +60,7 @@ pub use boot_control::BootControl; pub use boot_watch::{BootWatch, FailureCause, WalkVerdict}; pub use evidence::{BootStatus, EvidenceReader}; pub use lockdown_latch::LockdownLatch; +pub use recovery::Recovery; pub use svn_floor::{Svn, SvnFloor}; pub use trial_boot::TrialBoot; pub use updatable::{PayloadReadError, PayloadSource, StageProgress, Updatable, UpdateError}; diff --git a/services/orchestrator/capabilities/src/recovery.rs b/services/orchestrator/capabilities/src/recovery.rs new file mode 100644 index 00000000..116a5a88 --- /dev/null +++ b/services/orchestrator/capabilities/src/recovery.rs @@ -0,0 +1,153 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! The [`Recovery`] restore capability contract. + +/// Restore capability: rewrite one managed device's active image from its +/// configured recovery source. +/// +/// What the recovery source is (a golden image in protected flash, a mux +/// flip to a known-good part, a fetch over a sideband) is board wiring and +/// never leaks through this seam. The orchestrator only asks for the +/// mechanism to run; it holds the device in reset before asking and +/// re-verifies the image on the walk that follows. +/// +/// # Contract +/// +/// - **`Ok` means the mechanism completed, not that the image is good.** +/// Judging the restored image belongs to the verifier on the re-walk; a +/// restore must not forge a verdict by checking it here. +/// - **Errors are actuation faults only** (source unreachable, write +/// failed). The orchestrator treats them fail-closed. +/// - **Repeatable.** Each recovery attempt calls `restore` again with the +/// next `attempt`; a partial earlier restore must not stop a later call +/// from producing a complete image. +/// - **The caller counts the attempts.** `attempt` comes from the core's own +/// retry counter, the same value its retry cap is measured against, so an +/// implementor that keeps a count of its own would drift: it never sees +/// which attempt succeeded. Running out of sources is an actuation error +/// like any other. +pub trait Recovery { + /// The error type of this device's restore mechanism. + /// + /// Bounded by [`core::error::Error`] so the orchestrator gets `Display` + /// and a `source()` cause chain, not just a `Debug` dump. Error + /// categories are implementation-defined. + type Error: core::error::Error; + + /// Rewrites the device's active image from the recovery source. + /// + /// `attempt` is this device's consecutive-recovery count, `0` on the + /// first try of a recovery cycle. Implementors that hold more + /// than one source pick per attempt (slot A on `0`, slot B on `1`, + /// golden on `2`); implementors with a single source ignore it. + fn restore(&mut self, attempt: u8) -> Result<(), Self::Error>; +} + +#[cfg(test)] +mod tests { + use super::*; + + // Implements Recovery with no HAL dependency — the contract must be + // satisfiable from any stack (mock, IPC proxy, simulator). A HAL-bound + // `Error` type would stop this compiling. + struct MockRecovery { + attempts: [u8; 4], + restores: usize, + /// Attempt number the source faults on, so a test can make one + /// restore fail and the next one succeed. + fail_on: Option, + } + + impl MockRecovery { + fn healthy() -> Self { + MockRecovery { + attempts: [0; 4], + restores: 0, + fail_on: None, + } + } + + fn faulting_on(attempt: u8) -> Self { + MockRecovery { + attempts: [0; 4], + restores: 0, + fail_on: Some(attempt), + } + } + } + + #[derive(Debug, PartialEq, Eq)] + struct MockFault; + + impl core::fmt::Display for MockFault { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("mock restore fault") + } + } + + impl core::error::Error for MockFault {} + + impl Recovery for MockRecovery { + type Error = MockFault; + + fn restore(&mut self, attempt: u8) -> Result<(), MockFault> { + if self.fail_on == Some(attempt) { + return Err(MockFault); + } + self.attempts[self.restores] = attempt; + self.restores += 1; + Ok(()) + } + } + + /// The orchestrator's shape: run the mechanism, judge nothing here. + /// `attempt` rides in from `Effect::RecoverComponent`, never from a + /// count the device keeps. + fn recover(dev: &mut R, attempt: u8) -> Result<(), R::Error> { + dev.restore(attempt) + } + + #[test] + fn contract_is_implementable_without_the_hal() { + let mut dev = MockRecovery::healthy(); + + recover(&mut dev, 0).expect("restore failed"); + recover(&mut dev, 1).expect("repeated restore failed"); + + assert_eq!(dev.restores, 2); + } + + #[test] + fn each_attempt_reaches_the_implementor() { + let mut dev = MockRecovery::healthy(); + + // Out of order and with a gap, so a device that recorded its own + // call count instead of the argument fails here. + for attempt in [2, 0, 7] { + recover(&mut dev, attempt).expect("restore failed"); + } + + assert_eq!(&dev.attempts[..3], &[2, 0, 7]); + } + + #[test] + fn a_failed_restore_does_not_block_the_next_attempt() { + let mut dev = MockRecovery::faulting_on(0); + + recover(&mut dev, 0).expect_err("expected the first attempt to fault"); + recover(&mut dev, 1).expect("the next attempt must still restore"); + + assert_eq!(dev.restores, 1); + } + + #[test] + fn errors_surface_through_the_generic_seam() { + let mut dev = MockRecovery::faulting_on(0); + + let err = recover(&mut dev, 0).expect_err("expected the restore fault"); + + // Display comes from the core::error::Error bound, not a Debug dump. + assert_eq!(err.to_string(), "mock restore fault"); + } +} From 6fe779813d480353fde002ed2be6369519f41755 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Mon, 21 Sep 2026 14:19:41 +0200 Subject: [PATCH 2/2] orchestrator: Return RestoreOutcome from Recovery::restore Return RestoreOutcome instead of a bare Ok, so an implementor can report source exhaustion without it looking like an actuation fault. SourceExhausted travels on the Ok side, the platform driver reports Event::RecoveryUnavailable, and the component is gated per policy instead of the whole platform locking. Matches the error contract in #473. Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Christina Quast --- MODULE.bazel.lock | 2 +- services/orchestrator/capabilities/src/lib.rs | 2 +- .../orchestrator/capabilities/src/recovery.rs | 111 +++++++++++++----- 3 files changed, 83 insertions(+), 32 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index d9d97045..888481c7 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2039,7 +2039,7 @@ "CARGO_BAZEL_DEBUG": null, "CARGO_BAZEL_GENERATOR_SHA256": null, "CARGO_BAZEL_GENERATOR_URL": null, - "CARGO_BAZEL_ISOLATED": "0", + "CARGO_BAZEL_ISOLATED": null, "CARGO_BAZEL_REPIN": null, "CARGO_BAZEL_REPIN_ONLY": null, "CARGO_BAZEL_TIMEOUT": null, diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index 8190cb3f..6f41a351 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -60,7 +60,7 @@ pub use boot_control::BootControl; pub use boot_watch::{BootWatch, FailureCause, WalkVerdict}; pub use evidence::{BootStatus, EvidenceReader}; pub use lockdown_latch::LockdownLatch; -pub use recovery::Recovery; +pub use recovery::{Recovery, RestoreOutcome}; pub use svn_floor::{Svn, SvnFloor}; pub use trial_boot::TrialBoot; pub use updatable::{PayloadReadError, PayloadSource, StageProgress, Updatable, UpdateError}; diff --git a/services/orchestrator/capabilities/src/recovery.rs b/services/orchestrator/capabilities/src/recovery.rs index 116a5a88..a943fcca 100644 --- a/services/orchestrator/capabilities/src/recovery.rs +++ b/services/orchestrator/capabilities/src/recovery.rs @@ -3,6 +3,22 @@ //! The [`Recovery`] restore capability contract. +/// The outcome of a single restore attempt. +/// +/// Carried on the `Ok` side of [`Recovery::restore`] so the orchestrator can +/// distinguish "the mechanism ran" from "there is nothing left to try" +/// without pattern-matching an opaque error type it cannot inspect. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RestoreOutcome { + /// The recovery source was written to the device's active region. + /// Whether the image is good is not judged here; the re-walk verifies. + Restored, + /// The device's configured recovery sources are exhausted (no untried + /// image/slot remains). The orchestrator gates the component per its + /// failure policy immediately, without waiting for the retry cap. + SourceExhausted, +} + /// Restore capability: rewrite one managed device's active image from its /// configured recovery source. /// @@ -12,21 +28,21 @@ /// mechanism to run; it holds the device in reset before asking and /// re-verifies the image on the walk that follows. /// -/// # Contract +/// `Ok(Restored)` says the mechanism completed, not that the image is good. +/// The verifier judges the restored image on the re-walk, so a restore must +/// not check it here. `Ok(SourceExhausted)` says no untried source remains, +/// and the orchestrator gates the component per its failure policy right +/// away instead of waiting for the retry cap. Errors are actuation faults +/// only, such as an unreachable source or a failed write, and the +/// orchestrator treats them fail-closed. Source exhaustion travels on the +/// `Ok` side because it is a known condition: the orchestrator applies +/// per-component policy to it instead of locking unconditionally. /// -/// - **`Ok` means the mechanism completed, not that the image is good.** -/// Judging the restored image belongs to the verifier on the re-walk; a -/// restore must not forge a verdict by checking it here. -/// - **Errors are actuation faults only** (source unreachable, write -/// failed). The orchestrator treats them fail-closed. -/// - **Repeatable.** Each recovery attempt calls `restore` again with the -/// next `attempt`; a partial earlier restore must not stop a later call -/// from producing a complete image. -/// - **The caller counts the attempts.** `attempt` comes from the core's own -/// retry counter, the same value its retry cap is measured against, so an -/// implementor that keeps a count of its own would drift: it never sees -/// which attempt succeeded. Running out of sources is an actuation error -/// like any other. +/// Every recovery attempt calls `restore` again with the next `attempt`, so +/// a partial earlier restore must not stop a later call from producing a +/// complete image. The attempt count comes from the core's retry counter, +/// the same value the retry cap is measured against. A count kept by the +/// device would drift, because it never sees which attempt succeeded. pub trait Recovery { /// The error type of this device's restore mechanism. /// @@ -38,22 +54,26 @@ pub trait Recovery { /// Rewrites the device's active image from the recovery source. /// /// `attempt` is this device's consecutive-recovery count, `0` on the - /// first try of a recovery cycle. Implementors that hold more - /// than one source pick per attempt (slot A on `0`, slot B on `1`, - /// golden on `2`); implementors with a single source ignore it. - fn restore(&mut self, attempt: u8) -> Result<(), Self::Error>; + /// first try of a recovery cycle. Implementors that hold more than one + /// source pick per attempt (slot A on `0`, slot B on `1`, golden on + /// `2`); implementors with a single source ignore it and return + /// [`RestoreOutcome::SourceExhausted`] once their only source has been + /// tried. + fn restore(&mut self, attempt: u8) -> Result; } #[cfg(test)] mod tests { use super::*; - // Implements Recovery with no HAL dependency — the contract must be - // satisfiable from any stack (mock, IPC proxy, simulator). A HAL-bound + // Implements Recovery with no HAL dependency, because the contract must + // be satisfiable from any stack (mock, IPC proxy, simulator). A HAL-bound // `Error` type would stop this compiling. struct MockRecovery { attempts: [u8; 4], restores: usize, + /// Number of distinct sources this device holds. + sources: u8, /// Attempt number the source faults on, so a test can make one /// restore fail and the next one succeed. fail_on: Option, @@ -64,6 +84,16 @@ mod tests { MockRecovery { attempts: [0; 4], restores: 0, + sources: u8::MAX, + fail_on: None, + } + } + + fn with_sources(sources: u8) -> Self { + MockRecovery { + attempts: [0; 4], + restores: 0, + sources, fail_on: None, } } @@ -72,6 +102,7 @@ mod tests { MockRecovery { attempts: [0; 4], restores: 0, + sources: u8::MAX, fail_on: Some(attempt), } } @@ -91,20 +122,23 @@ mod tests { impl Recovery for MockRecovery { type Error = MockFault; - fn restore(&mut self, attempt: u8) -> Result<(), MockFault> { + fn restore(&mut self, attempt: u8) -> Result { if self.fail_on == Some(attempt) { return Err(MockFault); } + if attempt >= self.sources { + return Ok(RestoreOutcome::SourceExhausted); + } self.attempts[self.restores] = attempt; self.restores += 1; - Ok(()) + Ok(RestoreOutcome::Restored) } } - /// The orchestrator's shape: run the mechanism, judge nothing here. - /// `attempt` rides in from `Effect::RecoverComponent`, never from a - /// count the device keeps. - fn recover(dev: &mut R, attempt: u8) -> Result<(), R::Error> { + /// Calls the mechanism the way the orchestrator does: run it, judge + /// nothing here. `attempt` comes from `Effect::RecoverComponent`, never + /// from a count the device keeps. + fn recover(dev: &mut R, attempt: u8) -> Result { dev.restore(attempt) } @@ -112,8 +146,8 @@ mod tests { fn contract_is_implementable_without_the_hal() { let mut dev = MockRecovery::healthy(); - recover(&mut dev, 0).expect("restore failed"); - recover(&mut dev, 1).expect("repeated restore failed"); + assert_eq!(recover(&mut dev, 0).unwrap(), RestoreOutcome::Restored); + assert_eq!(recover(&mut dev, 1).unwrap(), RestoreOutcome::Restored); assert_eq!(dev.restores, 2); } @@ -125,7 +159,10 @@ mod tests { // Out of order and with a gap, so a device that recorded its own // call count instead of the argument fails here. for attempt in [2, 0, 7] { - recover(&mut dev, attempt).expect("restore failed"); + assert_eq!( + recover(&mut dev, attempt).unwrap(), + RestoreOutcome::Restored + ); } assert_eq!(&dev.attempts[..3], &[2, 0, 7]); @@ -136,7 +173,7 @@ mod tests { let mut dev = MockRecovery::faulting_on(0); recover(&mut dev, 0).expect_err("expected the first attempt to fault"); - recover(&mut dev, 1).expect("the next attempt must still restore"); + assert_eq!(recover(&mut dev, 1).unwrap(), RestoreOutcome::Restored); assert_eq!(dev.restores, 1); } @@ -150,4 +187,18 @@ mod tests { // Display comes from the core::error::Error bound, not a Debug dump. assert_eq!(err.to_string(), "mock restore fault"); } + + #[test] + fn source_exhaustion_is_a_verdict_not_an_error() { + let mut dev = MockRecovery::with_sources(2); + + assert_eq!(recover(&mut dev, 0).unwrap(), RestoreOutcome::Restored); + assert_eq!(recover(&mut dev, 1).unwrap(), RestoreOutcome::Restored); + assert_eq!( + recover(&mut dev, 2).unwrap(), + RestoreOutcome::SourceExhausted + ); + + assert_eq!(dev.restores, 2, "exhaustion does not count as a restore"); + } }