diff --git a/PLANS.md b/PLANS.md index 8d747a4..ac38439 100644 --- a/PLANS.md +++ b/PLANS.md @@ -124,6 +124,7 @@ Each row is a delivery milestone, not a completion claim. Expand its record befo | 12 | Aligned, responsive client table | 11 | accepted | | 13 | Compact terminal QR disclosure | 12 | accepted | | 14 | Restart verification for valid legacy peers | 13 | accepted | +| 15 | Diagnosable systemd restart and unloaded-unit resolution | 14 | accepted | ### Milestone 1 — Rust foundation, settings, and domain states @@ -3645,8 +3646,36 @@ and all integrated gates passed. [M14 evidence](docs/evidence/milestone-14.md) records source identity and tests. No disposable-VM systemd acceptance or production access is claimed; M9 remains retired and unaccepted. +### Milestone 15 — Diagnosable systemd restart and unloaded-unit resolution + +Status: accepted (2026-10-27T22:19:58+02:00) for synthetic adapter and application behavior. M14 is accepted; M9 real systemd acceptance remains retired and unaccepted. + +#### Outcome and scope + +Preserve the service adapter's typed failure through application reporting and readable TUI status, including operation phase and configured unit without raw D-Bus/service text. Resolve existing unloaded units through the documented systemd loading API while preserving alias identity, deadlines, signal ordering, conservative submission status, mandatory backup, explicit confirmation and full runtime verification before durable applied recording. + +#### Tasks and acceptance + +| Task | Owner | State | Evidence required | +| --- | --- | --- | --- | +| M15.1 | implementer | complete for implementation scope | `src/service.rs`, `src/service/dbus.rs`, `src/app.rs`, `src/tui/terminal.rs`, mechanical `src/container_driver.rs`; synthetic tests | +| M15.2 | independent reviewer; repairer | complete | R1 uncertainty and R2 unloaded-unit test gap repaired and independently re-reviewed | +| M15.3 | coordinator | complete | Integrated diff, pinned Rust gates and [M15 evidence](docs/evidence/milestone-15.md) | + +- [x] M15-A1: every service failure category survives into application report and readable unit/phase TUI status; raw bus/service text is excluded. +- [x] M15-A2: synthetic API tests exercise the production `LoadUnit` resolution flow for unloaded and aliased units; missing units and access denial stop before restart, with shared deadline and subscription-before-request preserved. Real systemd remains unverified. +- [x] M15-A3: failures retain conservative submitted/uncertainty flags and never mark applied; backup, confirmation, generation and runtime gates remain intact. +- [x] M15-A4: independent review closes actionable findings; focused tests and required pinned Rust gates pass. Synthetic evidence and disposable-VM acceptance are reported separately. + +Checks: focused service/application/TUI tests; `cargo fmt --all -- --check`; `cargo clippy --locked --all-targets -- -D warnings`; `cargo test --locked`; `cargo build --locked --release`; `git diff --check`. All passed on the repaired Rust inputs. Tests used synthetic fixtures only; no host service or production configuration. Detailed outcomes and identities: [M15 evidence](docs/evidence/milestone-15.md). + +#### Outcome and handoff + +Accepted for the synthetic adapter and application scope after R1 (owner-change uncertainty) and R2 (unit-resolution test gap) were repaired and independently re-reviewed. Official systemd D-Bus documentation confirms `GetUnit` fails for an unloaded unit and `LoadUnit` loads it from disk if possible; the prior call was a confirmed API mismatch. The cause of the reported host failure and actual systemd acceptance remain unproven without a disposable-VM run. [M15 evidence](docs/evidence/milestone-15.md) records the reviewed source, tests and release artifact. + ## Current progress and resume point +- Milestone 15 is accepted (2026-10-27) for synthetic service-adapter and application behavior: typed failure/phase/unit reporting, documented `LoadUnit` resolution, conservative uncertainty, independent R1/R2 closure and pinned Rust gates. [M15 evidence](docs/evidence/milestone-15.md) records source/artifact identity. Disposable-VM systemd acceptance remains incomplete and unclaimed. - Milestone 14 is accepted (2026-10-27) for fixture restart orchestration: complete saved-peer projection, legacy/IPv4-only/broader-route eligibility, safe blockers and durable verified history passed independent review and pinned Rust gates. [M14 evidence](docs/evidence/milestone-14.md) records source/artifact identity. No disposable-VM systemd acceptance or production result is claimed. - Milestone 13 is accepted (2026-10-27): compact, explicitly colored QR disclosure, complete quiet zone and dimensioned fallback passed synthetic rendering tests, independent R5 closure and pinned Rust gates. [M13 evidence](docs/evidence/milestone-13.md) records the reviewed source and release artifact. No manual scan, Alpine rerun, VM or production validation is claimed. - Bounded TUI maintenance TUI-TS-001 (2026-10-27): new-client form creation timestamps now have whole-second UTC precision before strict metadata rendering. A fixed fractional-second instant and a synthetic blank-IP form-to-draft workflow passed; saved bytes stayed unchanged. Independent review found no actionable issue, and pinned-toolchain Rust completion gates passed. [Timestamp repair evidence](docs/evidence/tui-creation-timestamp.md) identifies the source and release artifact. This repairs M7 behavior without claiming new Alpine or VM validation; earlier artifact identities remain historical after this Rust change. @@ -3667,7 +3696,7 @@ production access is claimed; M9 remains retired and unaccepted. - Milestone 8 delivered reviewed opt-in Alpine Testcontainers coverage for real IPv4 WireGuard peers, application lifecycle, saved/runtime separation, restoration, import/reissue, backup failures and the real Rustix reader. Host gates and three fresh real-container runs passed on one stable source fingerprint; M8-A1–A10 accepted. Real IPv6 behavior is unverified by user-authorized scope. - Milestone 9 is retired and unaccepted: the reviewed disposable VM preparation and guarded first start did not establish Debian/systemd acceptance. The exact VM was powered off after the user stopped work. M9.2–M9.3 were not started; M9-A1–A9 are unchecked. -- Next action: await user direction after M14. Do not restart the VM, access production, commit, or deploy. +- Next action: use a disposable systemd VM if actual D-Bus acceptance is requested; no VM result is claimed here. Do not access production, commit, or deploy. ## Plan maintenance diff --git a/docs/evidence/milestone-15.md b/docs/evidence/milestone-15.md new file mode 100644 index 0000000..c0c4c4d --- /dev/null +++ b/docs/evidence/milestone-15.md @@ -0,0 +1,69 @@ +# Milestone 15 evidence: diagnosable systemd restart + +Validated 2026-10-27T22:19:58+02:00 on the native +`x86_64-unknown-linux-gnu` development host. All behavioral checks below use +synthetic fixtures or fake service/unit APIs. No system D-Bus or host service +was invoked. + +## Cause, change, and limits + +The source-level diagnostic loss was confirmed: `Application` mapped every +unsuccessful service outcome to `ServiceFailed`. The report now retains the +typed `RestartFailure`, an explicit operation phase, and the configured unit; +the terminal renders a readable category and phase with the unit escaped for +terminal safety. Raw D-Bus error messages and service output are not included. + +The [official systemd Manager D-Bus API](https://freedesktop.org/software/systemd/man/latest/org.freedesktop.systemd1.html) +confirms `GetUnit` fails if the named unit has not yet been loaded and `LoadUnit` +loads it from disk if possible. The previous adapter called `GetUnit`; it now +calls `LoadUnit`, requires `LoadState == loaded`, and reads canonical `Id` for +job-event matching while requesting restart with the configured name. This is +a confirmed API mismatch for existing unloaded units, but its role in the +user's specific host failure is a hypothesis until a disposable-VM run provides +real systemd/D-Bus evidence. Missing or nonloaded units stop at resolution. + +The preexisting service adapter could understate uncertainty if the manager +owner changed during a pending restart request. That path now reports possible +submission, possible later completion and manual recovery. Synthetic tests +cover connection, subscription, permission, resolution, request and job +failures; alias identity; unloaded and missing unit load flows; and readable +TUI status for every typed failure. They also assert failure does not mark the +state applied. Existing tests cover mandatory backup, confirmation, generation, +complete runtime match, and durable applied-state boundaries. No shell restart +path was introduced. + +Independent GPT-6-Luna/high review found M15-R1 (P2, request owner-change +uncertainty) and M15-R2 (P2, unloaded-unit adapter test gap). A separate +GPT-6-Luna/high repairer addressed both. The reviewer rechecked the actual +repair and closed both findings with no further scoped issue. + +## Commands and source identity + +Pinned `rustc`/Cargo 1.97.1, rustfmt 1.9.0 and Clippy 0.1.97 were verified. +Commands ran from the repository root on the repaired revision: + +- `cargo test --locked service::dbus::tests::`: 13 passed. +- `cargo test --locked app::tests::application_report_preserves_every_service_failure_and_operation_phase`: 1 passed. +- `cargo fmt --all -- --check`: exit 0. +- `cargo clippy --locked --all-targets -- -D warnings`: exit 0. +- `cargo test --locked`: 364 passed, 3 explicitly ignored, 0 failed, including 2 documentation tests. The selected Docker daemon test was not run. +- `cargo build --locked --release`: exit 0. +- `git diff --check`: exit 0. + +Base commit: `86c28a35b559f70cb08365a7cdf0d5961e5570ea`. Dirty Rust inputs +at validation and their SHA256 values: + +- `src/service.rs`: `83a6089a4db019918178d7d97346e104180d63eb3ccc8a025bda91939720e02e` +- `src/service/dbus.rs`: `79ebc8c91db6dd08163ee0a6484359c2a8e7623e33081c9088c4d97d507d1816` +- `src/app.rs`: `2c3f6822a48dd5ec92faad9fbfd05a5be8723057d23aaba7f92e7899891cbefb` +- `src/tui/terminal.rs`: `48835132a30b1d5243a6076abfe06f6e98fb9419b0c4433c540406767715bae3` +- `src/container_driver.rs`: `9f6b27bff93d4face72ef8980a5d8e2db418bdb079c501a885d2c419fcfd6964` + +Native release artifact `target/release/wireguard-admin` SHA256: +`18dd2ce2dcc9f93272440d45491b5f83da82a7b9ed392986a8eaf059b76bf593`. + +No Alpine rerun, disposable systemd VM, or production operation was performed. +The actual systemd/D-Bus acceptance required by M9 remains incomplete and +unclaimed. An operator can now use the specific safe category and phase to +distinguish a unit-resolution problem from connection, subscription, access, +request, or job failure without exposing raw service details. diff --git a/src/app.rs b/src/app.rs index af2dbb9..3bb0797 100644 --- a/src/app.rs +++ b/src/app.rs @@ -315,6 +315,9 @@ pub struct RestartDiagnostic { pub struct RestartReport { pub stage: RestartStage, pub failure: Option, + pub service_failure: Option, + pub service_phase: Option, + pub systemd_unit: Option, pub backup_path: Option, pub restart_submitted: bool, pub restart_may_still_complete: bool, @@ -1713,6 +1716,7 @@ impl Application { let execution = service(&settings.systemd_unit, deadline); let job = execution.outcome; let submitted = execution.submitted; + let service_phase = execution.phase; if job != RestartOutcome::Succeeded { self.active_restart_attempt = None; let may_complete = submitted @@ -1727,7 +1731,7 @@ impl Application { | service::RestartFailure::JobResultUnknown ) ); - return restart_report( + let mut report = restart_report( RestartStage::Service, Some(RestartFailureKind::ServiceFailed), Some(backup_path), @@ -1735,6 +1739,13 @@ impl Application { may_complete, submitted, ); + report.service_failure = match job { + RestartOutcome::Failed(failure) => Some(failure), + RestartOutcome::Waiting | RestartOutcome::Succeeded => None, + }; + report.service_phase = Some(service_phase); + report.systemd_unit = Some(settings.systemd_unit); + return report; } let remaining = deadline.remaining_at(now()); if remaining.is_zero() { @@ -2411,6 +2422,9 @@ fn restart_report( RestartReport { stage, failure, + service_failure: None, + service_phase: None, + systemd_unit: None, backup_path, restart_submitted, restart_may_still_complete, @@ -4172,6 +4186,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, _| { @@ -4210,6 +4225,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, _| Err(TransportError::Transport), @@ -4250,6 +4266,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |interface, remaining| { @@ -4327,6 +4344,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, _| Ok(observation), @@ -4445,6 +4463,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| Ok(incomplete), |store, saved, backup| store.install(saved, backup), @@ -4513,6 +4532,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete), submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| { runtime_called.set(true); @@ -4548,6 +4568,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, _| { @@ -4595,6 +4616,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, remaining| { @@ -4660,6 +4682,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| Err(error), |store, saved, backup| { @@ -4698,6 +4721,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Failed(failure), submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| unreachable!("terminal service failure must stop before runtime read"), |_, _, _| unreachable!("terminal service failure must stop before persistence"), @@ -4710,6 +4734,120 @@ restart_timeout_seconds = 30 } } + #[test] + fn application_report_preserves_every_service_failure_and_operation_phase() { + use crate::service::RestartPhase; + + let cases = [ + (RestartFailure::JobFailed, RestartPhase::WaitForJob, true), + ( + RestartFailure::JobDependency, + RestartPhase::WaitForJob, + true, + ), + (RestartFailure::JobTimedOut, RestartPhase::WaitForJob, true), + (RestartFailure::JobCanceled, RestartPhase::WaitForJob, true), + (RestartFailure::JobSkipped, RestartPhase::WaitForJob, true), + ( + RestartFailure::JobResultUnknown, + RestartPhase::WaitForJob, + true, + ), + ( + RestartFailure::BusDisconnected, + RestartPhase::Connect, + false, + ), + ( + RestartFailure::ManagerOwnerChanged, + RestartPhase::WaitForJob, + true, + ), + ( + RestartFailure::ManagerOwnerChanged, + RestartPhase::RequestRestart, + true, + ), + ( + RestartFailure::SubscriptionFailed, + RestartPhase::Subscribe, + false, + ), + ( + RestartFailure::UnitResolutionFailed, + RestartPhase::ResolveUnit, + false, + ), + ( + RestartFailure::PermissionDenied, + RestartPhase::ResolveUnit, + false, + ), + ( + RestartFailure::RequestFailed, + RestartPhase::RequestRestart, + false, + ), + ( + RestartFailure::WaitTimedOutMayStillComplete, + RestartPhase::WaitForJob, + true, + ), + ( + RestartFailure::EarlyEventBufferFull, + RestartPhase::WaitForJob, + true, + ), + ( + RestartFailure::InvalidJobPath, + RestartPhase::WaitForJob, + true, + ), + ]; + + let root = tempfile::tempdir().expect("temp directory"); + let (mut app, _) = prepare_restartable(root.path()); + for (failure, phase, submitted) in cases { + let preview = app.preview_restart().expect("preview saved generation"); + let report = app.restart_with_adapters( + preview, + RestartChoice::RestartSavedState, + |_, _| RestartExecution { + outcome: RestartOutcome::Failed(failure), + submitted, + phase, + }, + |_, _| unreachable!("service failure must stop before runtime read"), + |_, _, _| unreachable!("service failure must stop before persistence"), + ); + assert_eq!(report.failure, Some(RestartFailureKind::ServiceFailed)); + assert_eq!(report.service_failure, Some(failure)); + assert_eq!(report.service_phase, Some(phase)); + assert_eq!(report.systemd_unit.as_deref(), Some("wg-quick@wg0.service")); + assert_eq!(report.restart_submitted, submitted); + if failure == RestartFailure::ManagerOwnerChanged + && phase == RestartPhase::RequestRestart + { + assert!(report.restart_may_still_complete); + assert!(report.manual_recovery_required); + } + assert_eq!( + report.restart_may_still_complete, + submitted + && !matches!( + failure, + RestartFailure::JobFailed + | RestartFailure::JobDependency + | RestartFailure::JobTimedOut + | RestartFailure::JobCanceled + | RestartFailure::JobSkipped + | RestartFailure::JobResultUnknown + ) + ); + assert_eq!(app.states().applied(), &RuntimeObservation::Unknown); + } + } + #[test] fn restart_mismatch_and_post_restart_write_failure_never_release_addresses() { let root = tempfile::tempdir().expect("temp directory"); @@ -4724,6 +4862,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| { Ok(ObservedRuntime { @@ -4744,6 +4883,7 @@ restart_timeout_seconds = 30 |_, _| RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, }, |_, _| Ok(observation), |_, _, _| Err(PersistenceError::RecoveryRequired), @@ -4803,6 +4943,7 @@ restart_timeout_seconds = 30 RestartExecution { outcome: RestartOutcome::Succeeded, submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, |_, _| Err(TransportError::Transport), diff --git a/src/container_driver.rs b/src/container_driver.rs index 5c6e076..ae9e07d 100644 --- a/src/container_driver.rs +++ b/src/container_driver.rs @@ -1062,6 +1062,7 @@ fn run_failure_prevents_application_scenario( RestartExecution { outcome: RestartOutcome::Failed(RestartFailure::RequestFailed), submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } }, ) @@ -1663,6 +1664,7 @@ fn run_fixture_apply_with_cleanup( RestartOutcome::Failed(RestartFailure::RequestFailed) }, submitted: applied, + phase: crate::service::RestartPhase::WaitForJob, } } @@ -1900,6 +1902,7 @@ mod tests { RestartExecution { outcome: RestartOutcome::Failed(RestartFailure::RequestFailed), submitted: true, + phase: crate::service::RestartPhase::WaitForJob, } ); } diff --git a/src/service.rs b/src/service.rs index f9f0211..cca07e3 100644 --- a/src/service.rs +++ b/src/service.rs @@ -101,6 +101,16 @@ pub enum RestartFailure { InvalidJobPath, } +/// Secret-free operation boundary where a restart attempt failed. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum RestartPhase { + Connect, + Subscribe, + ResolveUnit, + RequestRestart, + WaitForJob, +} + /// Current state of a single restart attempt. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum RestartOutcome { @@ -115,6 +125,7 @@ pub enum RestartOutcome { pub struct RestartExecution { pub outcome: RestartOutcome, pub submitted: bool, + pub phase: RestartPhase, } /// Pure event tracker for the primary systemd unit resolved from the configured diff --git a/src/service/dbus.rs b/src/service/dbus.rs index c8c00fd..79d4cfd 100644 --- a/src/service/dbus.rs +++ b/src/service/dbus.rs @@ -18,7 +18,7 @@ use zbus::{Connection, MatchRule, MessageStream, Proxy}; use super::{ JobResult, RestartDeadline, RestartExecution, RestartFailure, RestartJobTracker, - RestartOutcome, ServiceEvent, + RestartOutcome, RestartPhase, ServiceEvent, }; const SYSTEMD_NAME: &str = "org.freedesktop.systemd1"; @@ -70,25 +70,39 @@ fn restart_with_transport_report( deadline: RestartDeadline, ) -> RestartExecution { if configured_unit.is_empty() || configured_unit.as_bytes().contains(&0) { - return execution(RestartOutcome::Failed(RestartFailure::RequestFailed), false); + return execution( + RestartOutcome::Failed(RestartFailure::RequestFailed), + false, + RestartPhase::RequestRestart, + ); } if let Err(failure) = transport.connect(deadline) { - return execution(RestartOutcome::Failed(failure), false); + return execution( + RestartOutcome::Failed(failure), + false, + RestartPhase::Connect, + ); } if deadline.is_expired_at(Instant::now()) { return execution( RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete), false, + RestartPhase::Connect, ); } if let Err(failure) = transport.subscribe(deadline) { - return execution(RestartOutcome::Failed(failure), false); + return execution( + RestartOutcome::Failed(failure), + false, + RestartPhase::Subscribe, + ); } if deadline.is_expired_at(Instant::now()) { return execution( RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete), false, + RestartPhase::Subscribe, ); } let primary_unit = match transport.resolve_primary_unit(configured_unit, deadline) { @@ -97,14 +111,22 @@ fn restart_with_transport_report( return execution( RestartOutcome::Failed(RestartFailure::UnitResolutionFailed), false, + RestartPhase::ResolveUnit, + ); + } + Err(failure) => { + return execution( + RestartOutcome::Failed(failure), + false, + RestartPhase::ResolveUnit, ); } - Err(failure) => return execution(RestartOutcome::Failed(failure), false), }; if deadline.is_expired_at(Instant::now()) { return execution( RestartOutcome::Failed(RestartFailure::WaitTimedOutMayStillComplete), false, + RestartPhase::ResolveUnit, ); } @@ -113,9 +135,15 @@ fn restart_with_transport_report( Err(failure) => { let submitted = matches!( failure, - RestartFailure::WaitTimedOutMayStillComplete | RestartFailure::BusDisconnected + RestartFailure::WaitTimedOutMayStillComplete + | RestartFailure::BusDisconnected + | RestartFailure::ManagerOwnerChanged + ); + return execution( + RestartOutcome::Failed(failure), + submitted, + RestartPhase::RequestRestart, ); - return execution(RestartOutcome::Failed(failure), submitted); } }; let mut tracker = RestartJobTracker::new(primary_unit, deadline); @@ -123,15 +151,25 @@ fn restart_with_transport_report( while outcome == RestartOutcome::Waiting { let event = match transport.next_event(deadline) { Ok(event) => event, - Err(failure) => return execution(RestartOutcome::Failed(failure), true), + Err(failure) => { + return execution( + RestartOutcome::Failed(failure), + true, + RestartPhase::WaitForJob, + ); + } }; outcome = tracker.observe(event, Instant::now()); } - execution(outcome, true) + execution(outcome, true, RestartPhase::WaitForJob) } -fn execution(outcome: RestartOutcome, submitted: bool) -> RestartExecution { - RestartExecution { outcome, submitted } +fn execution(outcome: RestartOutcome, submitted: bool, phase: RestartPhase) -> RestartExecution { + RestartExecution { + outcome, + submitted, + phase, + } } #[derive(Default)] @@ -142,6 +180,100 @@ struct SystemdBus { owner_events: Option, } +/// The small systemd unit API used by the shared resolver. Keeping method +/// selection and canonical-ID lookup here lets synthetic tests exercise the +/// same resolution flow as the production adapter. +trait UnitLoadApi { + type UnitPath; + + fn load_unit( + &mut self, + configured_unit: &str, + deadline: RestartDeadline, + ) -> Result; + fn load_state( + &mut self, + unit_path: &Self::UnitPath, + deadline: RestartDeadline, + ) -> Result; + fn canonical_id( + &mut self, + unit_path: &Self::UnitPath, + deadline: RestartDeadline, + ) -> Result; +} + +fn resolve_loaded_unit( + api: &mut impl UnitLoadApi, + configured_unit: &str, + deadline: RestartDeadline, +) -> Result { + let unit_path = api.load_unit(configured_unit, deadline)?; + if !unit_is_loaded(&api.load_state(&unit_path, deadline)?) { + return Err(RestartFailure::UnitResolutionFailed); + } + api.canonical_id(&unit_path, deadline) +} + +impl UnitLoadApi for SystemdBus { + type UnitPath = zbus::zvariant::OwnedObjectPath; + + fn load_unit( + &mut self, + configured_unit: &str, + deadline: RestartDeadline, + ) -> Result { + let manager = self + .manager + .as_ref() + .ok_or(RestartFailure::UnitResolutionFailed)?; + drive(manager.call("LoadUnit", &(configured_unit,)), deadline)? + .map_err(map_unit_resolution_error) + } + + fn load_state( + &mut self, + unit_path: &Self::UnitPath, + deadline: RestartDeadline, + ) -> Result { + let unit = self.unit_proxy(unit_path, deadline)?; + drive(unit.get_property::("LoadState"), deadline)? + .map_err(map_unit_resolution_error) + } + + fn canonical_id( + &mut self, + unit_path: &Self::UnitPath, + deadline: RestartDeadline, + ) -> Result { + let unit = self.unit_proxy(unit_path, deadline)?; + drive(unit.get_property::("Id"), deadline)?.map_err(map_unit_resolution_error) + } +} + +impl SystemdBus { + fn unit_proxy( + &self, + unit_path: &zbus::zvariant::OwnedObjectPath, + deadline: RestartDeadline, + ) -> Result, RestartFailure> { + let connection = self + .connection + .as_ref() + .ok_or(RestartFailure::BusDisconnected)?; + drive( + Proxy::new_owned( + connection.clone(), + SYSTEMD_NAME, + unit_path.clone(), + "org.freedesktop.systemd1.Unit", + ), + deadline, + )? + .map_err(map_unit_resolution_error) + } +} + impl RestartTransport for SystemdBus { fn connect(&mut self, deadline: RestartDeadline) -> Result<(), RestartFailure> { let connection = drive(Connection::system(), deadline)?.map_err(map_bus_error)?; @@ -202,28 +334,7 @@ impl RestartTransport for SystemdBus { configured_unit: &str, deadline: RestartDeadline, ) -> Result { - let manager = self - .manager - .as_ref() - .ok_or(RestartFailure::UnitResolutionFailed)?; - let connection = self - .connection - .as_ref() - .ok_or(RestartFailure::BusDisconnected)?; - let unit_path: zbus::zvariant::OwnedObjectPath = - drive(manager.call("GetUnit", &(configured_unit,)), deadline)? - .map_err(map_unit_resolution_error)?; - let unit = drive( - Proxy::new_owned( - connection.clone(), - SYSTEMD_NAME, - unit_path, - "org.freedesktop.systemd1.Unit", - ), - deadline, - )? - .map_err(map_unit_resolution_error)?; - drive(unit.get_property::("Id"), deadline)?.map_err(map_unit_resolution_error) + resolve_loaded_unit(self, configured_unit, deadline) } fn restart_unit( @@ -348,6 +459,10 @@ impl RestartTransport for SystemdBus { } } +fn unit_is_loaded(load_state: &str) -> bool { + load_state == "loaded" +} + fn map_job_result(result: &str) -> JobResult { match result { "done" => JobResult::Done, @@ -624,6 +739,172 @@ mod tests { assert!(fake.deadlines.iter().all(|(_, passed)| *passed == deadline)); } + #[test] + fn systemd_load_states_allow_only_loaded_units_to_reach_restart() { + assert!(unit_is_loaded("loaded")); + for state in ["not-found", "bad-setting", "error", "masked", "stub"] { + assert!(!unit_is_loaded(state), "unexpected load state: {state}"); + } + } + + struct FakeUnitLoadApi { + calls: Vec, + configured_unit_seen: Option, + load_state: Result, + canonical_id: Result, + load_unit_result: Result, + } + + impl UnitLoadApi for FakeUnitLoadApi { + type UnitPath = String; + + fn load_unit( + &mut self, + configured_unit: &str, + _deadline: RestartDeadline, + ) -> Result { + self.calls.push("LoadUnit".to_owned()); + self.configured_unit_seen = Some(configured_unit.to_owned()); + self.load_unit_result.clone() + } + + fn load_state( + &mut self, + unit_path: &Self::UnitPath, + _deadline: RestartDeadline, + ) -> Result { + self.calls.push(format!("LoadState:{unit_path}")); + self.load_state.clone() + } + + fn canonical_id( + &mut self, + unit_path: &Self::UnitPath, + _deadline: RestartDeadline, + ) -> Result { + self.calls.push(format!("Id:{unit_path}")); + self.canonical_id.clone() + } + } + + impl FakeUnitLoadApi { + fn loaded(unit_path: &str, canonical_id: &str) -> Self { + Self { + calls: Vec::new(), + configured_unit_seen: None, + load_state: Ok("loaded".to_owned()), + canonical_id: Ok(canonical_id.to_owned()), + load_unit_result: Ok(unit_path.to_owned()), + } + } + } + + #[test] + fn production_unit_resolver_loads_unloaded_alias_and_returns_canonical_id() { + let mut resolver = FakeUnitLoadApi::loaded( + "/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice", + "wg-quick@wg0.service", + ); + + let canonical = resolve_loaded_unit(&mut resolver, "wireguard-vpn.service", deadline()) + .expect("LoadUnit loads existing unloaded alias"); + + assert_eq!( + resolver.configured_unit_seen.as_deref(), + Some("wireguard-vpn.service") + ); + assert_eq!(canonical, "wg-quick@wg0.service"); + assert_eq!( + resolver.calls, + [ + "LoadUnit", + "LoadState:/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice", + "Id:/org/freedesktop/systemd1/unit/wg_2dquick_40wg0_2eservice", + ] + ); + } + + #[test] + fn production_unit_resolver_stops_for_missing_units_and_access_denial() { + let mut missing = FakeUnitLoadApi::loaded("/unit/missing", "unused"); + missing.load_state = Ok("not-found".to_owned()); + assert_eq!( + resolve_loaded_unit(&mut missing, "missing.service", deadline()), + Err(RestartFailure::UnitResolutionFailed) + ); + assert_eq!(missing.calls, ["LoadUnit", "LoadState:/unit/missing"]); + + let mut load_error = FakeUnitLoadApi::loaded("/unit/unused", "unused"); + load_error.load_unit_result = Err(RestartFailure::UnitResolutionFailed); + assert_eq!( + resolve_loaded_unit(&mut load_error, "absent.service", deadline()), + Err(RestartFailure::UnitResolutionFailed) + ); + assert_eq!(load_error.calls, ["LoadUnit"]); + + let mut denied = FakeUnitLoadApi::loaded("/unit/denied", "unused"); + denied.load_unit_result = Err(RestartFailure::PermissionDenied); + assert_eq!( + resolve_loaded_unit(&mut denied, "restricted.service", deadline()), + Err(RestartFailure::PermissionDenied) + ); + assert_eq!(denied.calls, ["LoadUnit"]); + } + + #[test] + fn alias_resolution_and_access_denial_stop_before_restart() { + let mut alias = FakeBus { + configured_unit: "wireguard-vpn.service", + primary_unit: "wg-quick@wg0.service", + job_path: "/job/alias", + events: VecDeque::from([removed( + "/job/alias", + "wg-quick@wg0.service", + JobResult::Done, + )]), + ..FakeBus::default() + }; + let execution = + restart_with_transport_report(&mut alias, "wireguard-vpn.service", deadline()); + assert_eq!(execution.outcome, RestartOutcome::Succeeded); + assert_eq!(execution.phase, RestartPhase::WaitForJob); + assert_eq!( + alias.calls[0..4], + ["connect", "subscribe", "resolve", "restart"] + ); + + let mut denied = FakeBus { + failure: Some(("resolve", RestartFailure::PermissionDenied)), + ..FakeBus::default() + }; + let denied_unit = denied.configured_unit; + let execution = restart_with_transport_report(&mut denied, denied_unit, deadline()); + assert_eq!( + execution.outcome, + RestartOutcome::Failed(RestartFailure::PermissionDenied) + ); + assert_eq!(execution.phase, RestartPhase::ResolveUnit); + assert!(!denied.calls.contains(&"restart")); + } + + #[test] + fn owner_change_while_restart_reply_is_pending_is_reported_as_submitted() { + let mut fake = FakeBus { + failure: Some(("restart", RestartFailure::ManagerOwnerChanged)), + ..FakeBus::default() + }; + + let execution = + restart_with_transport_report(&mut fake, "wg-quick@wg0.service", deadline()); + + assert_eq!( + execution.outcome, + RestartOutcome::Failed(RestartFailure::ManagerOwnerChanged) + ); + assert_eq!(execution.phase, RestartPhase::RequestRestart); + assert!(execution.submitted); + } + #[test] fn expired_shared_deadline_stops_between_setup_and_submission() { let mut fake = FakeBus { diff --git a/src/tui/terminal.rs b/src/tui/terminal.rs index bcff639..6ac1e15 100644 --- a/src/tui/terminal.rs +++ b/src/tui/terminal.rs @@ -508,14 +508,32 @@ fn result_message(result: &CommandResult) -> String { CommandResult::RestartPreview(_) => { "Restart preview ready; confirm the exact saved-state action.".to_owned() } - CommandResult::Restarted(report) => format!( - "Restart result: {:?}; failure: {:?}; submitted: {}; may still complete: {}; manual recovery required: {}.", - report.stage, - report.failure, - report.restart_submitted, - report.restart_may_still_complete, - report.manual_recovery_required - ), + CommandResult::Restarted(report) => { + let mut message = format!( + "Restart result: {:?}; failure: {:?}; submitted: {}; may still complete: {}; manual recovery required: {}.", + report.stage, + report.failure, + report.restart_submitted, + report.restart_may_still_complete, + report.manual_recovery_required + ); + if let Some(failure) = report.service_failure { + let phase = report + .service_phase + .map(restart_phase_label) + .unwrap_or("service operation"); + let unit = report + .systemd_unit + .as_deref() + .map(render::safe_text) + .unwrap_or_else(|| "configured systemd unit".to_owned()); + message.push_str(&format!( + " Systemd {phase} failed for {unit}: {}.", + restart_failure_label(failure) + )); + } + message + } CommandResult::DeliveryDestination(destination) => format!( "Prepared destination {}. Overwrite confirmation required: {}.", render::safe_text(&destination.path.display().to_string()), @@ -566,6 +584,39 @@ fn result_message(result: &CommandResult) -> String { } } +fn restart_phase_label(phase: crate::service::RestartPhase) -> &'static str { + match phase { + crate::service::RestartPhase::Connect => "bus connection", + crate::service::RestartPhase::Subscribe => "job subscription", + crate::service::RestartPhase::ResolveUnit => "unit resolution", + crate::service::RestartPhase::RequestRestart => "restart request", + crate::service::RestartPhase::WaitForJob => "job wait", + } +} + +fn restart_failure_label(failure: crate::service::RestartFailure) -> &'static str { + use crate::service::RestartFailure; + match failure { + RestartFailure::JobFailed => "job failed", + RestartFailure::JobDependency => "job dependency failed", + RestartFailure::JobTimedOut => "job timed out", + RestartFailure::JobCanceled => "job was canceled", + RestartFailure::JobSkipped => "job was skipped", + RestartFailure::JobResultUnknown => "job returned an unknown result", + RestartFailure::BusDisconnected => "D-Bus connection was lost", + RestartFailure::ManagerOwnerChanged => "systemd manager owner changed", + RestartFailure::SubscriptionFailed => "job subscription failed", + RestartFailure::UnitResolutionFailed => "unit could not be loaded", + RestartFailure::PermissionDenied => "permission denied", + RestartFailure::RequestFailed => "systemd request failed", + RestartFailure::WaitTimedOutMayStillComplete => { + "operation timed out and may still complete" + } + RestartFailure::EarlyEventBufferFull => "job event buffer was full", + RestartFailure::InvalidJobPath => "systemd returned an invalid job path", + } +} + fn process_command_result( result: CommandResult, state: &mut UiState, @@ -1048,11 +1099,15 @@ mod tests { #[test] fn restart_results_render_cancel_success_and_uncertain_failure_honestly() { use crate::app::{RestartFailureKind, RestartReport, RestartStage}; + use crate::service::{RestartFailure, RestartPhase}; let report = |stage, failure, restart_submitted, restart_may_still_complete| { CommandResult::Restarted(RestartReport { stage, failure, + service_failure: None, + service_phase: None, + systemd_unit: None, backup_path: None, restart_submitted, restart_may_still_complete, @@ -1076,6 +1131,137 @@ mod tests { assert!(uncertain.contains("ServiceFailed")); assert!(uncertain.contains("may still complete: true")); assert!(uncertain.contains("manual recovery required: true")); + + let detailed = CommandResult::Restarted(RestartReport { + stage: RestartStage::Service, + failure: Some(RestartFailureKind::ServiceFailed), + service_failure: Some(RestartFailure::PermissionDenied), + service_phase: Some(RestartPhase::ResolveUnit), + systemd_unit: Some("wireguard-vpn.service".to_owned()), + backup_path: None, + restart_submitted: false, + restart_may_still_complete: false, + manual_recovery_required: false, + }); + let detailed = result_message(&detailed); + assert!(detailed.contains("unit resolution")); + assert!(detailed.contains("wireguard-vpn.service")); + assert!(detailed.contains("permission denied")); + assert!(!detailed.contains("org.freedesktop.DBus.Error")); + } + + #[test] + fn every_redacted_service_failure_has_a_readable_tui_category_and_phase() { + use crate::app::{RestartFailureKind, RestartReport, RestartStage}; + use crate::service::{RestartFailure, RestartPhase}; + + for (failure, phase, label) in [ + ( + RestartFailure::JobFailed, + RestartPhase::WaitForJob, + "job failed", + ), + ( + RestartFailure::JobDependency, + RestartPhase::WaitForJob, + "job dependency failed", + ), + ( + RestartFailure::JobTimedOut, + RestartPhase::WaitForJob, + "job timed out", + ), + ( + RestartFailure::JobCanceled, + RestartPhase::WaitForJob, + "job was canceled", + ), + ( + RestartFailure::JobSkipped, + RestartPhase::WaitForJob, + "job was skipped", + ), + ( + RestartFailure::JobResultUnknown, + RestartPhase::WaitForJob, + "job returned an unknown result", + ), + ( + RestartFailure::BusDisconnected, + RestartPhase::Connect, + "D-Bus connection was lost", + ), + ( + RestartFailure::ManagerOwnerChanged, + RestartPhase::WaitForJob, + "systemd manager owner changed", + ), + ( + RestartFailure::SubscriptionFailed, + RestartPhase::Subscribe, + "job subscription failed", + ), + ( + RestartFailure::UnitResolutionFailed, + RestartPhase::ResolveUnit, + "unit could not be loaded", + ), + ( + RestartFailure::PermissionDenied, + RestartPhase::RequestRestart, + "permission denied", + ), + ( + RestartFailure::RequestFailed, + RestartPhase::RequestRestart, + "systemd request failed", + ), + ( + RestartFailure::WaitTimedOutMayStillComplete, + RestartPhase::WaitForJob, + "operation timed out and may still complete", + ), + ( + RestartFailure::EarlyEventBufferFull, + RestartPhase::WaitForJob, + "job event buffer was full", + ), + ( + RestartFailure::InvalidJobPath, + RestartPhase::WaitForJob, + "systemd returned an invalid job path", + ), + ] { + let report = CommandResult::Restarted(RestartReport { + stage: RestartStage::Service, + failure: Some(RestartFailureKind::ServiceFailed), + service_failure: Some(failure), + service_phase: Some(phase), + systemd_unit: Some("wg-quick@wg0.service".to_owned()), + backup_path: None, + restart_submitted: true, + restart_may_still_complete: true, + manual_recovery_required: true, + }); + let message = result_message(&report); + assert!(message.contains(label), "missing {failure:?}: {message}"); + assert!(message.contains("wg-quick@wg0.service")); + assert!(message.contains(restart_phase_label(phase))); + assert!(!message.contains("org.freedesktop.DBus.Error")); + } + + let hostile_unit = CommandResult::Restarted(RestartReport { + stage: RestartStage::Service, + failure: Some(RestartFailureKind::ServiceFailed), + service_failure: Some(RestartFailure::PermissionDenied), + service_phase: Some(RestartPhase::ResolveUnit), + systemd_unit: Some("wg0\u{1b}[31m.service".to_owned()), + backup_path: None, + restart_submitted: false, + restart_may_still_complete: false, + manual_recovery_required: false, + }); + assert!(!result_message(&hostile_unit).contains('\u{1b}')); } #[test]