From 5bce453671080de6ca7ed6f30fd5310aedccc7ab Mon Sep 17 00:00:00 2001 From: zackees Date: Wed, 22 Jul 2026 17:32:15 -0700 Subject: [PATCH] feat(windows): add scoped USB recovery helper foundation --- Cargo.lock | 1 + crates/fbuild-cli/Cargo.toml | 10 + crates/fbuild-cli/src/cli/args.rs | 17 + crates/fbuild-cli/src/cli/deploy.rs | 2 + crates/fbuild-cli/src/cli/dispatch.rs | 25 + crates/fbuild-cli/src/cli/mod.rs | 7 + crates/fbuild-cli/src/cli/tests.rs | 22 + crates/fbuild-cli/src/cli/usb_recovery.rs | 511 +++++++++++++++ crates/fbuild-cli/src/daemon_client/types.rs | 8 + crates/fbuild-cli/src/mcp/tools.rs | 1 + crates/fbuild-core/src/usb/mod.rs | 5 + crates/fbuild-core/src/usb/recovery.rs | 169 +++++ crates/fbuild-daemon/src/models.rs | 4 + crates/fbuild-serial/Cargo.toml | 1 + crates/fbuild-serial/src/lib.rs | 1 + crates/fbuild-serial/src/usb_recovery.rs | 650 +++++++++++++++++++ 16 files changed, 1434 insertions(+) create mode 100644 crates/fbuild-cli/src/cli/usb_recovery.rs create mode 100644 crates/fbuild-core/src/usb/recovery.rs create mode 100644 crates/fbuild-serial/src/usb_recovery.rs diff --git a/Cargo.lock b/Cargo.lock index 897ba5d5..49a2a9b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1320,6 +1320,7 @@ dependencies = [ "tracing", "tracing-subscriber", "walkdir", + "windows-sys 0.52.0", ] [[package]] diff --git a/crates/fbuild-cli/Cargo.toml b/crates/fbuild-cli/Cargo.toml index 1eddbd3e..23cf87d4 100644 --- a/crates/fbuild-cli/Cargo.toml +++ b/crates/fbuild-cli/Cargo.toml @@ -40,3 +40,13 @@ tempfile = { workspace = true } walkdir = { workspace = true } owo-colors = { workspace = true } semver = { workspace = true } + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.52", features = [ + "Win32_Foundation", + "Win32_System_Registry", + "Win32_System_Threading", + "Win32_Storage_FileSystem", + "Win32_UI_Shell", + "Win32_UI_WindowsAndMessaging", +] } diff --git a/crates/fbuild-cli/src/cli/args.rs b/crates/fbuild-cli/src/cli/args.rs index fa8a2697..178b2771 100644 --- a/crates/fbuild-cli/src/cli/args.rs +++ b/crates/fbuild-cli/src/cli/args.rs @@ -341,6 +341,23 @@ pub enum Commands { /// Disable all shrink optimizations. Equivalent to `--shrink=off`. #[arg(long = "no-shrink", conflicts_with = "shrink")] no_shrink: bool, + /// Permit one UAC-gated, one-shot USB PnP recovery only when the + /// daemon returns a typed recoverable endpoint. This never elevates + /// the daemon, build, cache, or normal deployment process. + #[arg(long, conflicts_with = "no_admin")] + admin: bool, + /// Never request UAC; print physical recovery guidance instead. + #[arg(long = "no-admin", conflicts_with = "admin")] + no_admin: bool, + }, + /// Internal one-shot elevated USB recovery entry point. It is hidden so + /// users cannot mistake it for general administrator mode. + #[command(name = "__usb-recovery-helper", hide = true)] + UsbRecoveryHelper { + #[arg(long)] + request: std::path::PathBuf, + #[arg(long)] + result: std::path::PathBuf, }, /// Monitor serial output Monitor { diff --git a/crates/fbuild-cli/src/cli/deploy.rs b/crates/fbuild-cli/src/cli/deploy.rs index c9320a4a..b93d25b5 100644 --- a/crates/fbuild-cli/src/cli/deploy.rs +++ b/crates/fbuild-cli/src/cli/deploy.rs @@ -165,6 +165,7 @@ pub async fn run_deploy( emulator: Option, target: Option, output_dir: Option, + usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy, ) -> fbuild_core::Result<()> { daemon_client::ensure_daemon_running().await?; let client = DaemonClient::new(); @@ -218,6 +219,7 @@ pub async fn run_deploy( .filter(|s| !s.is_empty()), output_dir, pio_env: daemon_client::capture_pio_env(), + usb_recovery_policy, }; let resp = client.deploy(&req).await?; diff --git a/crates/fbuild-cli/src/cli/dispatch.rs b/crates/fbuild-cli/src/cli/dispatch.rs index 6d0e35cd..de356d4d 100644 --- a/crates/fbuild-cli/src/cli/dispatch.rs +++ b/crates/fbuild-cli/src/cli/dispatch.rs @@ -30,10 +30,22 @@ use super::serial_probe::run_serial; use super::show::run_show; use super::symbols_cmd::run_symbols; use super::sync_cmd::run_sync_cmd; +use super::usb_recovery::run_hidden_helper; pub async fn async_main() { let cli = Cli::parse_from(rewrite_args()); + // This must remain before tracing, environment capture, update checks, or + // any daemon client call. The elevated helper is a one-shot PnP process, + // never an alternate daemon host. FastLED/fbuild#1148. + if let Some(Commands::UsbRecoveryHelper { request, result }) = &cli.command { + if let Err(error) = run_hidden_helper(request, result) { + eprintln!("USB recovery helper failed: {error}"); + std::process::exit(1); + } + return; + } + tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) .init(); @@ -247,6 +259,8 @@ pub async fn async_main() { output_dir, shrink: _, no_shrink: _, + admin, + no_admin, }) => { let project_dir = resolve_project_dir(project_dir, &top_level_project_dir); if platformio { @@ -288,6 +302,13 @@ pub async fn async_main() { emulator, target, output_dir, + if admin { + fbuild_core::usb::UsbRecoveryPolicy::AllowAdmin + } else if no_admin { + fbuild_core::usb::UsbRecoveryPolicy::DenyAdmin + } else { + fbuild_core::usb::UsbRecoveryPolicy::Default + }, ) .await } @@ -600,6 +621,7 @@ pub async fn async_main() { None, None, None, + fbuild_core::usb::UsbRecoveryPolicy::Default, ) .await } @@ -608,6 +630,9 @@ pub async fn async_main() { Some(Commands::Bringup(args)) => run_bringup(args), Some(Commands::Port { action }) => run_port(action), Some(Commands::Cache { action }) => run_cache(action), + // Routed before any daemon-adjacent initialization above. This arm is + // unreachable but keeps the exhaustive command match honest. + Some(Commands::UsbRecoveryHelper { .. }) => unreachable!("helper was routed early"), }; if let Err(e) = result { diff --git a/crates/fbuild-cli/src/cli/mod.rs b/crates/fbuild-cli/src/cli/mod.rs index 868e408b..8123134a 100644 --- a/crates/fbuild-cli/src/cli/mod.rs +++ b/crates/fbuild-cli/src/cli/mod.rs @@ -33,6 +33,13 @@ pub mod serial_probe; pub mod show; pub mod symbols_cmd; pub mod sync_cmd; +// #1148 supplies the helper foundation; #1147 is deliberately its first +// production consumer once RP2040 deployment can emit a typed request. +#[allow( + dead_code, + reason = "USB recovery integration belongs exclusively to #1147" +)] +pub mod usb_recovery; #[cfg(test)] mod tests; diff --git a/crates/fbuild-cli/src/cli/tests.rs b/crates/fbuild-cli/src/cli/tests.rs index 88db283f..1c29d0f4 100644 --- a/crates/fbuild-cli/src/cli/tests.rs +++ b/crates/fbuild-cli/src/cli/tests.rs @@ -4,6 +4,28 @@ use super::args::{Cli, Commands, DaemonAction}; use super::compile_many::{build_ci_pio_env, normalize_ci_sketch_entry, normalize_ci_sketches}; use clap::Parser; +#[test] +fn deploy_admin_and_no_admin_conflict() { + assert!(Cli::try_parse_from(["fbuild", "deploy", "--admin", "--no-admin"]).is_err()); +} + +#[test] +fn hidden_usb_recovery_helper_arguments_parse() { + let cli = Cli::try_parse_from([ + "fbuild", + "__usb-recovery-helper", + "--request", + "request.json", + "--result", + "result.json", + ]) + .expect("parse hidden helper"); + assert!(matches!( + cli.command, + Some(Commands::UsbRecoveryHelper { .. }) + )); +} + #[test] fn normalize_ino_path_strips_to_parent_dir() { let entry = "examples/Blink/Blink.ino"; diff --git a/crates/fbuild-cli/src/cli/usb_recovery.rs b/crates/fbuild-cli/src/cli/usb_recovery.rs new file mode 100644 index 00000000..4a19e50a --- /dev/null +++ b/crates/fbuild-cli/src/cli/usb_recovery.rs @@ -0,0 +1,511 @@ +//! One-shot elevated USB recovery helper entry point. +//! +//! This module owns only the private JSON rendezvous used by the normal CLI +//! and its elevated copy. It never discovers or starts a daemon; PnP work is +//! delegated to the narrow `fbuild-serial` allowlist. FastLED/fbuild#1148. + +use fbuild_core::usb::{UsbRecoveryPolicy, UsbRecoveryRequest, UsbRecoveryResult}; +use serde::{Deserialize, Serialize}; +use std::fs::{File, OpenOptions}; +use std::io::{Read, Write}; +use std::path::{Path, PathBuf}; + +const MAX_RENDEZVOUS_BYTES: u64 = 16 * 1024; + +#[derive(Debug, Deserialize, Serialize)] +pub struct RecoveryHelperEnvelope { + pub nonce: String, + pub request: UsbRecoveryRequest, +} + +/// Facts used to enforce the CLI-side elevation policy. They are explicit so +/// tests can prove CI and non-interactive sessions never launch UAC. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct RecoveryLaunchContext { + pub is_windows: bool, + pub is_ci: bool, + pub is_interactive: bool, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum RecoveryLaunchDecision { + ManualGuidance, + RefuseNonInteractive, + LaunchOnce, +} + +/// UAC launch result. Cancellation is expected user control flow, not a +/// daemon failure; the normal caller cleans the rendezvous in either case. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum HelperLaunchOutcome { + Completed { exit_code: u32 }, + Cancelled, +} + +/// Test seam around the only operation that can display UAC. +pub trait RecoveryHelperLauncher { + fn launch( + &mut self, + request_path: &Path, + result_path: &Path, + ) -> fbuild_core::Result; +} + +/// A nonce-bound pair of files owned by the normal process. Drop removes both +/// paths after helper completion/cancellation/error; neither is a daemon or +/// cache artifact. +#[derive(Debug)] +pub struct RecoveryRendezvous { + pub request_path: PathBuf, + pub result_path: PathBuf, + pub nonce: String, +} + +impl Drop for RecoveryRendezvous { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.request_path); + let _ = std::fs::remove_file(&self.result_path); + } +} + +/// Per-user, short-lived rendezvous root. It is intentionally outside daemon +/// and cache directories, so the elevated helper never takes ownership of any +/// persistent fbuild state. +pub fn rendezvous_dir() -> PathBuf { + fbuild_paths::temp_subdir("usb-recovery") +} + +pub fn decide_recovery_launch( + policy: UsbRecoveryPolicy, + has_typed_request: bool, + context: RecoveryLaunchContext, +) -> RecoveryLaunchDecision { + if !has_typed_request + || matches!( + policy, + UsbRecoveryPolicy::Default | UsbRecoveryPolicy::DenyAdmin + ) + { + return RecoveryLaunchDecision::ManualGuidance; + } + if !context.is_windows || context.is_ci || !context.is_interactive { + RecoveryLaunchDecision::RefuseNonInteractive + } else { + RecoveryLaunchDecision::LaunchOnce + } +} + +/// Create the private payload before the normal process launches its own +/// executable with a fixed hidden helper command line. +pub fn create_rendezvous(request: UsbRecoveryRequest) -> fbuild_core::Result { + let root = rendezvous_dir(); + let mut request_file = tempfile::Builder::new() + .prefix("request-") + .suffix(".json") + .rand_bytes(32) + .tempfile_in(&root) + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot create recovery request: {error}")) + })?; + let request_path = request_file.path().to_path_buf(); + let nonce = blake3::hash(request_path.to_string_lossy().as_bytes()) + .to_hex() + .to_string(); + let envelope = RecoveryHelperEnvelope { + nonce: nonce.clone(), + request, + }; + serde_json::to_writer(&mut request_file, &envelope).map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot encode recovery request: {error}")) + })?; + request_file.as_file_mut().sync_all().map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot sync recovery request: {error}")) + })?; + let result_path = root.join(format!("result-{nonce}.json")); + if result_path.exists() { + return Err(fbuild_core::FbuildError::Other( + "recovery result path unexpectedly already exists".to_string(), + )); + } + // `keep` preserves the original create-new file rather than replacing it + // at a caller-controlled path. + let (_file, request_path) = request_file.keep().map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot persist recovery request: {}", error.error)) + })?; + Ok(RecoveryRendezvous { + request_path, + result_path, + nonce, + }) +} + +/// Launch at most once after the policy decision was made by the normal CLI. +/// No caller can launch a helper without a typed request, and all terminal +/// paths drop the rendezvous files. +pub fn launch_once_for_typed_request( + policy: UsbRecoveryPolicy, + request: UsbRecoveryRequest, + context: RecoveryLaunchContext, + launcher: &mut L, +) -> fbuild_core::Result { + let decision = decide_recovery_launch(policy, true, context); + if decision != RecoveryLaunchDecision::LaunchOnce { + return Ok(decision); + } + let rendezvous = create_rendezvous(request)?; + let _outcome = launcher.launch(&rendezvous.request_path, &rendezvous.result_path)?; + Ok(decision) +} + +/// Windows implementation that launches only the current fbuild executable +/// with the fixed hidden helper shape. It is deliberately not a general +/// command runner and never launches a daemon. +#[cfg(windows)] +pub struct WindowsUacLauncher; + +#[cfg(windows)] +impl RecoveryHelperLauncher for WindowsUacLauncher { + fn launch( + &mut self, + request_path: &Path, + result_path: &Path, + ) -> fbuild_core::Result { + use windows_sys::Win32::Foundation::{ + CloseHandle, ERROR_CANCELLED, GetLastError, WAIT_OBJECT_0, + }; + use windows_sys::Win32::System::Threading::{ + GetExitCodeProcess, INFINITE, WaitForSingleObject, + }; + use windows_sys::Win32::UI::Shell::{ + SEE_MASK_NOCLOSEPROCESS, SHELLEXECUTEINFOW, ShellExecuteExW, + }; + use windows_sys::Win32::UI::WindowsAndMessaging::SW_HIDE; + + fn wide(value: &std::ffi::OsStr) -> Vec { + use std::os::windows::ffi::OsStrExt; + value.encode_wide().chain(Some(0)).collect() + } + fn quoted_path(path: &Path) -> fbuild_core::Result { + let value = path.to_string_lossy(); + if value.contains('"') || value.contains('\n') || value.contains('\r') { + return Err(fbuild_core::FbuildError::Other( + "recovery helper path contains an unsupported character".to_string(), + )); + } + Ok(format!("\"{value}\"")) + } + + let executable = std::env::current_exe().map_err(|error| { + fbuild_core::FbuildError::Other(format!( + "cannot locate current fbuild executable: {error}" + )) + })?; + let parameters = format!( + "__usb-recovery-helper --request {} --result {}", + quoted_path(request_path)?, + quoted_path(result_path)? + ); + let executable_wide = wide(executable.as_os_str()); + let verb_wide = wide(std::ffi::OsStr::new("runas")); + let parameters_wide = wide(std::ffi::OsStr::new(¶meters)); + // SAFETY: all pointer fields target NUL-terminated buffers that remain + // alive through ShellExecuteExW; zero is the documented initialization + // for unused structure fields. + let mut execute: SHELLEXECUTEINFOW = unsafe { std::mem::zeroed() }; + execute.cbSize = std::mem::size_of::() as u32; + execute.fMask = SEE_MASK_NOCLOSEPROCESS; + execute.lpVerb = verb_wide.as_ptr(); + execute.lpFile = executable_wide.as_ptr(); + execute.lpParameters = parameters_wide.as_ptr(); + execute.nShow = SW_HIDE; + // SAFETY: `execute` is fully initialized as described above and only + // invokes this executable with the fixed helper subcommand. + if unsafe { ShellExecuteExW(&mut execute) } == 0 { + // SAFETY: GetLastError reads thread-local state immediately after + // ShellExecuteExW's documented failure return. + let error = unsafe { GetLastError() }; + if error == ERROR_CANCELLED { + return Ok(HelperLaunchOutcome::Cancelled); + } + return Err(fbuild_core::FbuildError::Other(format!( + "UAC helper launch failed ({error})" + ))); + } + if execute.hProcess == 0 { + return Err(fbuild_core::FbuildError::Other( + "UAC helper launch returned no process handle".to_string(), + )); + } + // SAFETY: hProcess was requested through SEE_MASK_NOCLOSEPROCESS and + // belongs to this caller. The handle is closed on every path below. + let wait = unsafe { WaitForSingleObject(execute.hProcess, INFINITE) }; + if wait != WAIT_OBJECT_0 { + // SAFETY: closes the valid owned process handle before returning. + unsafe { CloseHandle(execute.hProcess) }; + return Err(fbuild_core::FbuildError::Other(format!( + "UAC helper wait failed ({wait})" + ))); + } + let mut exit_code = 1u32; + // SAFETY: valid completed process handle and writable local exit code. + let got_exit_code = unsafe { GetExitCodeProcess(execute.hProcess, &mut exit_code) }; + // SAFETY: closes the valid owned process handle exactly once. + unsafe { CloseHandle(execute.hProcess) }; + if got_exit_code == 0 { + return Err(fbuild_core::FbuildError::Other( + "cannot read UAC helper exit status".to_string(), + )); + } + Ok(HelperLaunchOutcome::Completed { exit_code }) + } +} + +/// Execute the hidden helper after `dispatch::async_main` has routed to it +/// before tracing, environment capture, update checks, or daemon discovery. +pub fn run_hidden_helper(request_path: &Path, result_path: &Path) -> fbuild_core::Result<()> { + validate_rendezvous_paths(request_path, result_path)?; + let envelope = read_envelope(request_path)?; + if !valid_nonce(&envelope.nonce) || !envelope.request.has_canonical_identity() { + return Err(fbuild_core::FbuildError::Other( + "recovery helper rejected malformed rendezvous payload".to_string(), + )); + } + + let result = + fbuild_serial::usb_recovery::recover_windows_usb_device(&envelope.request, envelope.nonce); + write_result_create_new(result_path, &result) +} + +fn read_envelope(path: &Path) -> fbuild_core::Result { + reject_reparse_point(path, "recovery request")?; + let metadata = std::fs::symlink_metadata(path).map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot inspect recovery request: {error}")) + })?; + if !metadata.file_type().is_file() || metadata.file_type().is_symlink() { + return Err(fbuild_core::FbuildError::Other( + "recovery request must be a regular file".to_string(), + )); + } + if metadata.len() > MAX_RENDEZVOUS_BYTES { + return Err(fbuild_core::FbuildError::Other( + "recovery request exceeds the fixed size limit".to_string(), + )); + } + let mut contents = String::new(); + File::open(path) + .and_then(|mut file| file.read_to_string(&mut contents)) + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot read recovery request: {error}")) + })?; + serde_json::from_str(&contents).map_err(|error| { + fbuild_core::FbuildError::Other(format!("invalid recovery request JSON: {error}")) + }) +} + +fn write_result_create_new(path: &Path, result: &UsbRecoveryResult) -> fbuild_core::Result<()> { + let encoded = serde_json::to_vec(result).map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot encode recovery result: {error}")) + })?; + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .open(path) + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot create recovery result: {error}")) + })?; + file.write_all(&encoded) + .and_then(|()| file.sync_all()) + .map_err(|error| { + fbuild_core::FbuildError::Other(format!("cannot write recovery result: {error}")) + }) +} + +fn validate_rendezvous_paths(request_path: &Path, result_path: &Path) -> fbuild_core::Result<()> { + let root = rendezvous_dir(); + reject_reparse_point(&root, "recovery rendezvous directory")?; + let request_parent = request_path.parent(); + let result_parent = result_path.parent(); + let valid_parent = + request_parent == Some(root.as_path()) && result_parent == Some(root.as_path()); + let request_name = request_path.file_name().and_then(|name| name.to_str()); + let result_name = result_path.file_name().and_then(|name| name.to_str()); + if !valid_parent + || !request_name.is_some_and(|name| name.starts_with("request-") && name.ends_with(".json")) + || !result_name.is_some_and(|name| name.starts_with("result-") && name.ends_with(".json")) + { + return Err(fbuild_core::FbuildError::Other( + "recovery helper rejected paths outside its private rendezvous directory".to_string(), + )); + } + Ok(()) +} + +#[cfg(windows)] +fn reject_reparse_point(path: &Path, description: &str) -> fbuild_core::Result<()> { + use std::os::windows::ffi::OsStrExt; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_ATTRIBUTE_REPARSE_POINT, GetFileAttributesW, INVALID_FILE_ATTRIBUTES, + }; + + let wide = path + .as_os_str() + .encode_wide() + .chain(Some(0)) + .collect::>(); + // SAFETY: `wide` is NUL-terminated and remains live for the API call. + let attributes = unsafe { GetFileAttributesW(wide.as_ptr()) }; + if attributes == INVALID_FILE_ATTRIBUTES { + return Err(fbuild_core::FbuildError::Other(format!( + "cannot inspect {description} attributes" + ))); + } + if attributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + return Err(fbuild_core::FbuildError::Other(format!( + "recovery helper rejected reparse-point {description}" + ))); + } + Ok(()) +} + +#[cfg(not(windows))] +fn reject_reparse_point(_path: &Path, _description: &str) -> fbuild_core::Result<()> { + Ok(()) +} + +fn valid_nonce(nonce: &str) -> bool { + (32..=128).contains(&nonce.len()) && nonce.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +#[cfg(test)] +mod tests { + use super::*; + use fbuild_core::usb::UsbRecoveryHealth; + + fn envelope() -> RecoveryHelperEnvelope { + RecoveryHelperEnvelope { + nonce: "0123456789abcdef0123456789abcdef".to_string(), + request: UsbRecoveryRequest { + operation_id: "deploy-1".to_string(), + instance_id: "USB\\VID_2E8A&PID_000A\\serial".to_string(), + expected_class: "Ports".to_string(), + parent_instance_id: Some("USB\\ROOT_HUB30\\parent".to_string()), + expected_vid: 0x2e8a, + expected_pid: 0x000a, + expected_serial: Some("serial".to_string()), + problem_code: Some(43), + flash_completed: true, + }, + } + } + + #[test] + fn nonce_must_be_bounded_hex() { + assert!(valid_nonce(&envelope().nonce)); + assert!(!valid_nonce("too-short")); + assert!(!valid_nonce("0123456789abcdef0123456789abcdef!")); + } + + #[test] + fn result_file_is_create_new() { + let directory = tempfile::tempdir().expect("tempdir"); + let path = directory.path().join("result-test.json"); + let result = UsbRecoveryResult { + operation_id: "deploy-1".to_string(), + nonce: envelope().nonce, + validated_instance_id: None, + operation: None, + before: UsbRecoveryHealth::Unknown, + after: UsbRecoveryHealth::Unknown, + success: false, + error_code: Some("test".to_string()), + }; + assert!(write_result_create_new(&path, &result).is_ok()); + assert!(write_result_create_new(&path, &result).is_err()); + } + + #[test] + fn default_no_admin_ci_and_noninteractive_never_launch() { + let interactive_windows = RecoveryLaunchContext { + is_windows: true, + is_ci: false, + is_interactive: true, + }; + assert_eq!( + decide_recovery_launch(UsbRecoveryPolicy::Default, true, interactive_windows), + RecoveryLaunchDecision::ManualGuidance + ); + assert_eq!( + decide_recovery_launch(UsbRecoveryPolicy::DenyAdmin, true, interactive_windows), + RecoveryLaunchDecision::ManualGuidance + ); + assert_eq!( + decide_recovery_launch( + UsbRecoveryPolicy::AllowAdmin, + true, + RecoveryLaunchContext { + is_ci: true, + ..interactive_windows + }, + ), + RecoveryLaunchDecision::RefuseNonInteractive + ); + assert_eq!( + decide_recovery_launch( + UsbRecoveryPolicy::AllowAdmin, + true, + RecoveryLaunchContext { + is_interactive: false, + ..interactive_windows + }, + ), + RecoveryLaunchDecision::RefuseNonInteractive + ); + } + + #[test] + fn admin_launches_once_only_for_a_typed_request() { + struct FakeLauncher { + calls: usize, + request_path: Option, + result_path: Option, + } + impl RecoveryHelperLauncher for FakeLauncher { + fn launch( + &mut self, + request_path: &Path, + result_path: &Path, + ) -> fbuild_core::Result { + self.calls += 1; + self.request_path = Some(request_path.to_path_buf()); + self.result_path = Some(result_path.to_path_buf()); + Ok(HelperLaunchOutcome::Cancelled) + } + } + + let context = RecoveryLaunchContext { + is_windows: true, + is_ci: false, + is_interactive: true, + }; + let mut launcher = FakeLauncher { + calls: 0, + request_path: None, + result_path: None, + }; + assert_eq!( + decide_recovery_launch(UsbRecoveryPolicy::AllowAdmin, false, context), + RecoveryLaunchDecision::ManualGuidance + ); + let decision = launch_once_for_typed_request( + UsbRecoveryPolicy::AllowAdmin, + envelope().request, + context, + &mut launcher, + ) + .expect("launch policy"); + assert_eq!(decision, RecoveryLaunchDecision::LaunchOnce); + assert_eq!(launcher.calls, 1); + assert!(!launcher.request_path.expect("request path").exists()); + assert!(!launcher.result_path.expect("result path").exists()); + } +} diff --git a/crates/fbuild-cli/src/daemon_client/types.rs b/crates/fbuild-cli/src/daemon_client/types.rs index 4f79f8ad..f931ac75 100644 --- a/crates/fbuild-cli/src/daemon_client/types.rs +++ b/crates/fbuild-cli/src/daemon_client/types.rs @@ -116,6 +116,14 @@ pub struct DeployRequest { /// Snapshot of all `PLATFORMIO_*` env vars from the caller's environment. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub pio_env: BTreeMap, + /// Explicit user policy for a typed USB recovery response. The default is + /// intentionally non-elevating; #1147 is the first daemon consumer. + #[serde(default, skip_serializing_if = "is_default_usb_recovery_policy")] + pub usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy, +} + +fn is_default_usb_recovery_policy(policy: &fbuild_core::usb::UsbRecoveryPolicy) -> bool { + *policy == fbuild_core::usb::UsbRecoveryPolicy::Default } #[derive(Debug, Serialize)] diff --git a/crates/fbuild-cli/src/mcp/tools.rs b/crates/fbuild-cli/src/mcp/tools.rs index 0c21edb9..b5a86746 100644 --- a/crates/fbuild-cli/src/mcp/tools.rs +++ b/crates/fbuild-cli/src/mcp/tools.rs @@ -187,6 +187,7 @@ pub(super) async fn execute_tool( .filter(|s| !s.is_empty()), output_dir: None, pio_env: crate::daemon_client::capture_pio_env(), + usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy::Default, }; let resp = client diff --git a/crates/fbuild-core/src/usb/mod.rs b/crates/fbuild-core/src/usb/mod.rs index 7329396d..6ba85089 100644 --- a/crates/fbuild-core/src/usb/mod.rs +++ b/crates/fbuild-core/src/usb/mod.rs @@ -20,6 +20,7 @@ pub mod data; #[cfg(test)] pub mod embedded; pub mod profiles; +pub mod recovery; pub mod resolver; pub use data::{ @@ -30,6 +31,10 @@ pub use data::{ }; #[cfg(test)] pub use embedded::vendor_name as embedded_vendor_name; +pub use recovery::{ + UsbRecoveryHealth, UsbRecoveryOperation, UsbRecoveryPolicy, UsbRecoveryRequest, + UsbRecoveryResult, +}; #[cfg(test)] pub use resolver::resolve_bundled; pub use resolver::{UsbInfo, pretty, resolve, try_resolve}; diff --git a/crates/fbuild-core/src/usb/recovery.rs b/crates/fbuild-core/src/usb/recovery.rs new file mode 100644 index 00000000..7b4b436e --- /dev/null +++ b/crates/fbuild-core/src/usb/recovery.rs @@ -0,0 +1,169 @@ +//! Narrow, typed contract for one-shot USB PnP recovery. +//! +//! This module deliberately contains no host API calls. The normal daemon +//! creates a [`UsbRecoveryRequest`] only for an exact unhealthy endpoint; the +//! elevated CLI helper revalidates that identity and then asks `fbuild-serial` +//! to perform one of the two allowlisted operations. Keeping this contract in +//! `fbuild-core` lets the daemon and CLI communicate without parsing a human +//! diagnostic or making the daemon privileged. FastLED/fbuild#1148. + +use serde::{Deserialize, Serialize}; + +/// The caller's explicit permission to request the one-shot elevated helper. +/// +/// `Default` is intentionally non-elevating: it asks the user to rerun with +/// `--admin`. `DenyAdmin` is the `--no-admin` escape hatch. CI and +/// non-interactive checks remain an additional guard in the CLI and cannot be +/// bypassed by this enum. +#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum UsbRecoveryPolicy { + #[default] + Default, + AllowAdmin, + DenyAdmin, +} + +/// The sole two Windows PnP operations a recovery helper can represent. +/// +/// There is deliberately no generic command, executable, device-class, or +/// "reset all USB" variant. A phantom endpoint may only re-enumerate its +/// verified parent; a present problematic endpoint may only restart itself. +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum UsbRecoveryOperation { + ReenumerateParent, + RestartTarget, +} + +/// Host health observed before or after a recovery operation. +/// +/// This is intentionally independent of `fbuild_serial::PortHealth` so the +/// core contract does not introduce a dependency cycle. A helper result is +/// advisory only: the normal process must later perform a fresh serial +/// enumeration and openability probe before it returns a usable port. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum UsbRecoveryHealth { + HealthyPresent, + PresentProblem { problem_code: u32 }, + Phantom { problem_code: Option }, + Unknown, +} + +/// Identity facts the elevated helper must re-query before it can act. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct UsbRecoveryRequest { + /// Opaque daemon operation ID used only to correlate the one-shot result. + pub operation_id: String, + /// Canonical Windows PnP instance ID for the unhealthy endpoint. + pub instance_id: String, + /// Windows device class expected for that exact endpoint (for example, + /// `Ports`). The helper rejects a matching-looking USB instance that has + /// moved to a different class. + pub expected_class: String, + /// Immediate parent instance ID as observed by the normal process. + pub parent_instance_id: Option, + /// USB identity expected from the same endpoint, never a runtime default. + pub expected_vid: u16, + pub expected_pid: u16, + /// Required when the board profile supplied a serial number. + pub expected_serial: Option, + /// Problem code observed by the normal process, if Windows supplied one. + pub problem_code: Option, + /// Distinguishes preflight recovery from post-flash recovery-only flow. + pub flash_completed: bool, +} + +impl UsbRecoveryRequest { + /// Reject fields that cannot be safe canonical PnP identity input. + /// + /// The helper performs a second, authoritative host re-query. This check + /// nevertheless prevents a malformed rendezvous file from reaching any + /// Windows operation or being reported as a recoverable target. + pub fn has_canonical_identity(&self) -> bool { + fn canonical_pnp_id(value: &str) -> bool { + !value.is_empty() + && value.len() <= 512 + && !value.chars().any(|character| { + character.is_control() || matches!(character, '"' | '\'' | '\n' | '\r' | '\t') + }) + } + + canonical_pnp_id(&self.operation_id) + && canonical_pnp_id(&self.instance_id) + && canonical_pnp_id(&self.expected_class) + && self + .parent_instance_id + .as_deref() + .map_or(true, canonical_pnp_id) + && self.expected_serial.as_deref().map_or(true, |serial| { + !serial.is_empty() && serial.len() <= 256 && !serial.chars().any(char::is_control) + }) + } +} + +/// Bounded, non-port-bearing response from the elevated helper. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct UsbRecoveryResult { + pub operation_id: String, + pub nonce: String, + pub validated_instance_id: Option, + pub operation: Option, + pub before: UsbRecoveryHealth, + pub after: UsbRecoveryHealth, + pub success: bool, + /// Stable internal failure category, never a shell or operating-system + /// command. The normal CLI renders the actionable user-facing guidance. + pub error_code: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + fn request() -> UsbRecoveryRequest { + UsbRecoveryRequest { + operation_id: "deploy-123".to_string(), + instance_id: "USB\\VID_2E8A&PID_000A\\5303284720C4641C".to_string(), + expected_class: "Ports".to_string(), + parent_instance_id: Some("USB\\ROOT_HUB30\\4&1".to_string()), + expected_vid: 0x2e8a, + expected_pid: 0x000a, + expected_serial: Some("5303284720C4641C".to_string()), + problem_code: Some(43), + flash_completed: true, + } + } + + #[test] + fn recovery_policy_defaults_to_non_elevating() { + assert_eq!(UsbRecoveryPolicy::default(), UsbRecoveryPolicy::Default); + } + + #[test] + fn recovery_operations_cannot_represent_a_broad_usb_reset() { + let operations = [ + UsbRecoveryOperation::ReenumerateParent, + UsbRecoveryOperation::RestartTarget, + ]; + assert_eq!(operations.len(), 2); + } + + #[test] + fn canonical_request_identity_rejects_control_and_quote_injection() { + assert!(request().has_canonical_identity()); + + let mut bad_instance = request(); + bad_instance.instance_id = "USB\\VID_2E8A\n--anything".to_string(); + assert!(!bad_instance.has_canonical_identity()); + + let mut bad_parent = request(); + bad_parent.parent_instance_id = Some("USB\\ROOT_HUB\"".to_string()); + assert!(!bad_parent.has_canonical_identity()); + + let mut bad_class = request(); + bad_class.expected_class = "Ports\nUSB".to_string(); + assert!(!bad_class.has_canonical_identity()); + } +} diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index ff07413c..7e07c1d6 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -142,6 +142,10 @@ pub struct DeployRequest { /// Drop the `` segment of the build-dir path (see [`BuildRequest::flatten_env`]). #[serde(default)] pub flatten_env: bool, + /// User permission for the CLI-side one-shot USB recovery helper. The + /// daemon remains unprivileged and merely carries this typed policy. + #[serde(default)] + pub usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy, } fn default_qemu_timeout() -> u32 { diff --git a/crates/fbuild-serial/Cargo.toml b/crates/fbuild-serial/Cargo.toml index 19692a0c..67edf9bd 100644 --- a/crates/fbuild-serial/Cargo.toml +++ b/crates/fbuild-serial/Cargo.toml @@ -32,6 +32,7 @@ regex = { workspace = true } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.52", features = [ "Win32_Devices_DeviceAndDriverInstallation", + "Win32_Devices_Properties", "Win32_Foundation", "Win32_System_Registry", ] } diff --git a/crates/fbuild-serial/src/lib.rs b/crates/fbuild-serial/src/lib.rs index 634ba03a..907c6a12 100644 --- a/crates/fbuild-serial/src/lib.rs +++ b/crates/fbuild-serial/src/lib.rs @@ -42,6 +42,7 @@ pub mod ports; pub mod preemption; pub mod rpc_validate; pub mod session; +pub mod usb_recovery; pub use manager::{PortSessionInfo, SerialClientInfo, SharedSerialManager}; pub use messages::{ diff --git a/crates/fbuild-serial/src/usb_recovery.rs b/crates/fbuild-serial/src/usb_recovery.rs new file mode 100644 index 00000000..d0746ca5 --- /dev/null +++ b/crates/fbuild-serial/src/usb_recovery.rs @@ -0,0 +1,650 @@ +//! Strict, one-shot Windows USB PnP recovery boundary. +//! +//! The elevated CLI helper is intentionally thin: it supplies a typed request +//! and calls this module. This module can express only two operations, both +//! after an authoritative identity re-query: re-enumerate the exact live +//! parent of a phantom target, or restart the exact present problematic child. +//! It never opens a serial port, touches the fbuild daemon, or returns a COM +//! port. FastLED/fbuild#1148. + +use fbuild_core::usb::{ + UsbRecoveryHealth, UsbRecoveryOperation, UsbRecoveryRequest, UsbRecoveryResult, +}; + +/// A PnP devnode observed directly by the recovery backend. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct UsbPnpDevice { + pub instance_id: String, + pub parent_instance_id: Option, + pub device_class: String, + pub vid: u16, + pub pid: u16, + pub serial: Option, + pub health: UsbRecoveryHealth, +} + +/// Narrow host boundary used by the elevated helper and deterministic tests. +/// +/// No operation accepts a command line, arbitrary program, registry path, or +/// broad device selector. The caller passes only a canonical instance ID that +/// was revalidated by [`execute_recovery`]. +pub trait UsbPnpBackend { + type Error: std::fmt::Display; + + /// Return the current node. `allow_phantom` is required only to inspect + /// the original target; verified parents are always looked up live. + fn inspect( + &mut self, + instance_id: &str, + allow_phantom: bool, + ) -> Result; + + /// Re-enumerate only the exact, verified live parent of a phantom target. + fn reenumerate_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error>; + + /// Restart only the exact, verified present target child. + fn restart_target(&mut self, instance_id: &str) -> Result<(), Self::Error>; + + /// Number of bounded post-operation observations. Fakes stay instant; + /// Windows waits briefly between observations for re-enumeration to settle. + fn post_operation_poll_attempts(&self) -> usize { + 1 + } + + fn wait_for_post_operation_poll(&mut self) {} +} + +/// Execute the bounded recovery ladder with a real or fake PnP backend. +/// +/// A successful result means the allowlisted PnP operation completed, not that +/// the device is deployable. The normal unprivileged process must still run a +/// fresh #1146 health/openability probe before it can choose a serial port. +pub fn execute_recovery( + request: &UsbRecoveryRequest, + nonce: String, + backend: &mut B, +) -> UsbRecoveryResult { + let failed = |before: UsbRecoveryHealth, error_code: &str| UsbRecoveryResult { + operation_id: request.operation_id.clone(), + nonce: nonce.clone(), + validated_instance_id: None, + operation: None, + before: before.clone(), + after: before, + success: false, + error_code: Some(error_code.to_string()), + }; + + if !request.has_canonical_identity() { + return failed(UsbRecoveryHealth::Unknown, "invalid-request-identity"); + } + + let target = match backend.inspect(&request.instance_id, true) { + Ok(device) => device, + Err(_) => return failed(UsbRecoveryHealth::Unknown, "target-not-found"), + }; + let before = target.health.clone(); + if let Err(error_code) = validate_target_identity(request, &target) { + return failed(before, error_code); + } + + let (operation, action_result) = match target.health { + UsbRecoveryHealth::Phantom { .. } => { + let Some(parent_instance_id) = request.parent_instance_id.as_deref() else { + return failed(before, "missing-verified-parent"); + }; + let parent = match backend.inspect(parent_instance_id, false) { + Ok(device) => device, + Err(_) => return failed(before, "parent-not-live"), + }; + if !matches!(parent.health, UsbRecoveryHealth::HealthyPresent) + || !same_id(&parent.instance_id, parent_instance_id) + { + return failed(before, "parent-not-live"); + } + ( + UsbRecoveryOperation::ReenumerateParent, + backend.reenumerate_parent(parent_instance_id), + ) + } + UsbRecoveryHealth::PresentProblem { .. } => ( + UsbRecoveryOperation::RestartTarget, + backend.restart_target(&target.instance_id), + ), + UsbRecoveryHealth::HealthyPresent => return failed(before, "target-already-healthy"), + UsbRecoveryHealth::Unknown => return failed(before, "target-health-unknown"), + }; + + if action_result.is_err() { + return failed(before, "pnp-operation-failed"); + } + + let mut after = UsbRecoveryHealth::Unknown; + for _ in 0..backend.post_operation_poll_attempts().max(1) { + backend.wait_for_post_operation_poll(); + after = backend + .inspect(&request.instance_id, true) + .map(|device| device.health) + .unwrap_or(UsbRecoveryHealth::Unknown); + if matches!(after, UsbRecoveryHealth::HealthyPresent) { + break; + } + } + UsbRecoveryResult { + operation_id: request.operation_id.clone(), + nonce, + validated_instance_id: Some(target.instance_id), + operation: Some(operation), + before, + after, + success: true, + error_code: None, + } +} + +fn validate_target_identity( + request: &UsbRecoveryRequest, + target: &UsbPnpDevice, +) -> Result<(), &'static str> { + if !same_id(&target.instance_id, &request.instance_id) { + return Err("instance-id-mismatch"); + } + if !target + .device_class + .eq_ignore_ascii_case(&request.expected_class) + { + return Err("device-class-mismatch"); + } + if target.vid != request.expected_vid || target.pid != request.expected_pid { + return Err("vid-pid-mismatch"); + } + if let Some(expected_serial) = request.expected_serial.as_deref() { + if target.serial.as_deref() != Some(expected_serial) { + return Err("serial-mismatch"); + } + } + if let (Some(expected_problem_code), UsbRecoveryHealth::PresentProblem { problem_code }) = + (request.problem_code, &target.health) + { + if *problem_code != expected_problem_code { + return Err("problem-code-mismatch"); + } + } + if let Some(expected_parent) = request.parent_instance_id.as_deref() { + match target.parent_instance_id.as_deref() { + Some(actual_parent) if !same_id(actual_parent, expected_parent) => { + return Err("parent-mismatch"); + } + // A phantom can be recovered only if Config Manager still proves + // its immediate parent. The parent ID from the normal unprivileged + // request alone is never authority to touch a live USB node. + None => { + return Err("parent-mismatch"); + } + _ => {} + } + } + Ok(()) +} + +fn same_id(left: &str, right: &str) -> bool { + left.eq_ignore_ascii_case(right) +} + +/// Perform real host recovery when the one-shot helper is running on Windows. +/// +/// The non-Windows result deliberately fails closed. The CLI must render the +/// physical recovery instructions instead of attempting any platform-specific +/// substitute. +pub fn recover_windows_usb_device( + request: &UsbRecoveryRequest, + nonce: String, +) -> UsbRecoveryResult { + #[cfg(windows)] + { + let mut backend = windows::WindowsPnpBackend; + execute_recovery(request, nonce, &mut backend) + } + #[cfg(not(windows))] + { + UsbRecoveryResult { + operation_id: request.operation_id.clone(), + nonce, + validated_instance_id: None, + operation: None, + before: UsbRecoveryHealth::Unknown, + after: UsbRecoveryHealth::Unknown, + success: false, + error_code: Some("windows-recovery-unavailable".to_string()), + } + } +} + +#[cfg(windows)] +mod windows { + use super::*; + use std::time::Duration; + use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{ + CM_Disable_DevNode, CM_Enable_DevNode, CM_Get_DevNode_PropertyW, CM_Get_DevNode_Status, + CM_Get_Device_IDW, CM_Get_Parent, CM_LOCATE_DEVNODE_NORMAL, CM_LOCATE_DEVNODE_PHANTOM, + CM_Locate_DevNodeW, CM_Reenumerate_DevNode, CR_NO_SUCH_DEVINST, CR_SUCCESS, + MAX_DEVICE_ID_LEN, + }; + use windows_sys::Win32::Devices::Properties::{DEVPKEY_Device_Class, DEVPROP_TYPE_STRING}; + + /// Windows implementation is added below the common security ladder so + /// tests can exercise every allowlist decision without a privileged host. + pub(super) struct WindowsPnpBackend; + + impl UsbPnpBackend for WindowsPnpBackend { + type Error = String; + + fn inspect( + &mut self, + instance_id: &str, + allow_phantom: bool, + ) -> Result { + inspect_device(instance_id, allow_phantom) + } + + fn reenumerate_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error> { + reenumerate_parent(parent_instance_id) + } + + fn restart_target(&mut self, instance_id: &str) -> Result<(), Self::Error> { + restart_target(instance_id) + } + + fn post_operation_poll_attempts(&self) -> usize { + 8 + } + + fn wait_for_post_operation_poll(&mut self) { + std::thread::sleep(Duration::from_millis(250)); + } + } + + // These Config Manager functions are intentionally the only real PnP + // writes in this module. Keeping them behind `UsbPnpBackend` makes it + // impossible for the helper to call a broader operation than the trait. + fn inspect_device(instance_id: &str, allow_phantom: bool) -> Result { + let devinst = locate(instance_id, allow_phantom)?; + let actual_instance_id = device_id(devinst)?; + let parent_instance_id = parent_id(devinst)?; + let device_class = device_class(devinst)?; + let health = device_health(devinst); + let (vid, pid, serial) = + parse_usb_identity(&actual_instance_id, parent_instance_id.as_deref()).ok_or_else( + || "device does not expose a canonical USB VID/PID identity".to_string(), + )?; + + Ok(UsbPnpDevice { + instance_id: actual_instance_id, + parent_instance_id, + device_class, + vid, + pid, + serial, + health, + }) + } + + fn reenumerate_parent(parent_instance_id: &str) -> Result<(), String> { + let parent = locate(parent_instance_id, false)?; + // SAFETY: `parent` was obtained from Config Manager for the exact + // verified live parent. Flags are zero, requesting no broad scan. + let result = unsafe { CM_Reenumerate_DevNode(parent, 0) }; + (result == CR_SUCCESS) + .then_some(()) + .ok_or_else(|| format!("CM_Reenumerate_DevNode failed ({result})")) + } + + fn restart_target(instance_id: &str) -> Result<(), String> { + let target = locate(instance_id, false)?; + // SAFETY: `target` was revalidated as the exact present problematic + // child. The helper never passes a parent/hub/controller to this call. + let disabled = unsafe { CM_Disable_DevNode(target, 0) }; + if disabled != CR_SUCCESS { + return Err(format!("CM_Disable_DevNode failed ({disabled})")); + } + // SAFETY: same exact child devinst as the immediately preceding + // disable. No other Config Manager action is performed here. + let enabled = unsafe { CM_Enable_DevNode(target, 0) }; + if enabled == CR_SUCCESS { + return Ok(()); + } + // Best-effort rollback for a transient Config Manager failure. This + // is still the same exact child and does not widen the allowlist; its + // result is retained in the diagnostic rather than silently leaving a + // potentially disabled endpoint behind. + // SAFETY: same exact validated child devinst; this is a bounded + // best-effort re-enable after the first enable reported failure. + let rollback = unsafe { CM_Enable_DevNode(target, 0) }; + Err(format!( + "CM_Enable_DevNode failed ({enabled}); rollback enable returned ({rollback})" + )) + } + + fn locate(instance_id: &str, allow_phantom: bool) -> Result { + let mut devinst = 0u32; + let utf16 = instance_id + .encode_utf16() + .chain(Some(0)) + .collect::>(); + let flags = if allow_phantom { + CM_LOCATE_DEVNODE_PHANTOM + } else { + CM_LOCATE_DEVNODE_NORMAL + }; + // SAFETY: `utf16` is NUL-terminated and remains alive for the call; + // `devinst` is writable local storage. + let result = unsafe { CM_Locate_DevNodeW(&mut devinst, utf16.as_ptr(), flags) }; + (result == CR_SUCCESS) + .then_some(devinst) + .ok_or_else(|| format!("CM_Locate_DevNodeW failed ({result})")) + } + + fn device_id(devinst: u32) -> Result { + let mut buffer = [0u16; MAX_DEVICE_ID_LEN as usize]; + // SAFETY: `buffer` is writable local UTF-16 storage sized according to + // the Config Manager API's documented maximum device ID length. + let result = unsafe { + CM_Get_Device_IDW(devinst, buffer.as_mut_ptr(), (buffer.len() - 1) as u32, 0) + }; + if result != CR_SUCCESS { + return Err(format!("CM_Get_Device_IDW failed ({result})")); + } + Ok(from_utf16(&buffer)) + } + + fn parent_id(devinst: u32) -> Result, String> { + let mut parent = 0u32; + // SAFETY: `parent` is writable local storage and `devinst` came from + // Config Manager in the same process. + let result = unsafe { CM_Get_Parent(&mut parent, devinst, 0) }; + if result == CR_NO_SUCH_DEVINST { + return Ok(None); + } + if result != CR_SUCCESS { + return Err(format!("CM_Get_Parent failed ({result})")); + } + device_id(parent).map(Some) + } + + fn device_class(devinst: u32) -> Result { + let mut property_type = 0u32; + let mut buffer = [0u16; 256]; + let mut byte_len = (buffer.len() * std::mem::size_of::()) as u32; + // SAFETY: the property key and all output pointers remain valid for + // the call; the buffer size is provided in bytes as required by CM. + let result = unsafe { + CM_Get_DevNode_PropertyW( + devinst, + &DEVPKEY_Device_Class, + &mut property_type, + buffer.as_mut_ptr().cast(), + &mut byte_len, + 0, + ) + }; + if result != CR_SUCCESS || property_type != DEVPROP_TYPE_STRING { + return Err(format!( + "CM_Get_DevNode_PropertyW(Device_Class) failed ({result})" + )); + } + Ok(from_utf16(&buffer)) + } + + fn device_health(devinst: u32) -> UsbRecoveryHealth { + let mut status = 0u32; + let mut problem_code = 0u32; + // SAFETY: both output pointers are writable local storage and the + // devinst was returned by Config Manager. + let result = unsafe { CM_Get_DevNode_Status(&mut status, &mut problem_code, devinst, 0) }; + if result == CR_SUCCESS { + if problem_code == 0 { + UsbRecoveryHealth::HealthyPresent + } else { + UsbRecoveryHealth::PresentProblem { problem_code } + } + } else if result == CR_NO_SUCH_DEVINST { + UsbRecoveryHealth::Phantom { problem_code: None } + } else { + UsbRecoveryHealth::Unknown + } + } + + fn parse_usb_identity( + instance_id: &str, + parent_instance_id: Option<&str>, + ) -> Option<(u16, u16, Option)> { + fn parse(id: &str) -> Option<(u16, u16, Option)> { + let mut parts = id.split('\\'); + if !parts.next()?.eq_ignore_ascii_case("USB") { + return None; + } + let hardware = parts.next()?.to_ascii_uppercase(); + let vid_start = hardware.find("VID_")? + 4; + let pid_start = hardware.find("PID_")? + 4; + let vid = u16::from_str_radix(hardware.get(vid_start..vid_start + 4)?, 16).ok()?; + let pid = u16::from_str_radix(hardware.get(pid_start..pid_start + 4)?, 16).ok()?; + let serial = parts + .next() + .filter(|value| !value.is_empty()) + .map(str::to_string); + Some((vid, pid, serial)) + } + + let (vid, pid, child_serial) = parse(instance_id)?; + let parent_serial = + parent_instance_id + .and_then(parse) + .and_then(|(parent_vid, parent_pid, serial)| { + (parent_vid == vid && parent_pid == pid) + .then_some(serial) + .flatten() + }); + Some((vid, pid, parent_serial.or(child_serial))) + } + + fn from_utf16(buffer: &[u16]) -> String { + let length = buffer + .iter() + .position(|unit| *unit == 0) + .unwrap_or(buffer.len()); + String::from_utf16_lossy(&buffer[..length]) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::VecDeque; + + #[derive(Default)] + struct FakePnp { + observations: VecDeque>, + calls: Vec, + } + + impl FakePnp { + fn with_observations(observations: Vec) -> Self { + Self { + observations: observations.into_iter().map(Ok).collect(), + calls: Vec::new(), + } + } + } + + impl UsbPnpBackend for FakePnp { + type Error = String; + + fn inspect( + &mut self, + instance_id: &str, + allow_phantom: bool, + ) -> Result { + self.calls + .push(format!("inspect:{instance_id}:{allow_phantom}")); + self.observations + .pop_front() + .unwrap_or_else(|| Err("unexpected inspect".to_string())) + } + + fn reenumerate_parent(&mut self, parent_instance_id: &str) -> Result<(), Self::Error> { + self.calls.push(format!("reenumerate:{parent_instance_id}")); + Ok(()) + } + + fn restart_target(&mut self, instance_id: &str) -> Result<(), Self::Error> { + self.calls.push(format!("restart:{instance_id}")); + Ok(()) + } + } + + fn request() -> UsbRecoveryRequest { + UsbRecoveryRequest { + operation_id: "deploy-1".to_string(), + instance_id: "USB\\VID_2E8A&PID_000A\\serial".to_string(), + expected_class: "Ports".to_string(), + parent_instance_id: Some("USB\\ROOT_HUB30\\parent".to_string()), + expected_vid: 0x2e8a, + expected_pid: 0x000a, + expected_serial: Some("serial".to_string()), + problem_code: Some(43), + flash_completed: true, + } + } + + fn device(health: UsbRecoveryHealth) -> UsbPnpDevice { + UsbPnpDevice { + instance_id: request().instance_id, + parent_instance_id: request().parent_instance_id, + device_class: request().expected_class, + vid: 0x2e8a, + pid: 0x000a, + serial: Some("serial".to_string()), + health, + } + } + + #[test] + fn phantom_reenumerates_only_its_verified_live_parent() { + let parent = UsbPnpDevice { + instance_id: request().parent_instance_id.unwrap(), + parent_instance_id: None, + device_class: "USB".to_string(), + vid: 0x2e8a, + pid: 0x000a, + serial: Some("serial".to_string()), + health: UsbRecoveryHealth::HealthyPresent, + }; + let mut backend = FakePnp::with_observations(vec![ + device(UsbRecoveryHealth::Phantom { + problem_code: Some(43), + }), + parent, + device(UsbRecoveryHealth::HealthyPresent), + ]); + + let result = execute_recovery(&request(), "nonce".to_string(), &mut backend); + + assert!(result.success); + assert_eq!( + result.operation, + Some(UsbRecoveryOperation::ReenumerateParent) + ); + assert_eq!( + backend.calls, + vec![ + "inspect:USB\\VID_2E8A&PID_000A\\serial:true", + "inspect:USB\\ROOT_HUB30\\parent:false", + "reenumerate:USB\\ROOT_HUB30\\parent", + "inspect:USB\\VID_2E8A&PID_000A\\serial:true", + ] + ); + } + + #[test] + fn present_problem_restarts_only_the_exact_child() { + let mut backend = FakePnp::with_observations(vec![ + device(UsbRecoveryHealth::PresentProblem { problem_code: 43 }), + device(UsbRecoveryHealth::HealthyPresent), + ]); + + let result = execute_recovery(&request(), "nonce".to_string(), &mut backend); + + assert!(result.success); + assert_eq!(result.operation, Some(UsbRecoveryOperation::RestartTarget)); + assert!( + backend + .calls + .iter() + .any(|call| call == "restart:USB\\VID_2E8A&PID_000A\\serial") + ); + assert!( + !backend + .calls + .iter() + .any(|call| call.starts_with("reenumerate:")) + ); + } + + #[test] + fn identity_mismatch_rejects_before_any_pnp_operation() { + let mut mismatched = device(UsbRecoveryHealth::PresentProblem { problem_code: 43 }); + mismatched.pid = 0x000b; + let mut backend = FakePnp::with_observations(vec![mismatched]); + + let result = execute_recovery(&request(), "nonce".to_string(), &mut backend); + + assert!(!result.success); + assert_eq!(result.error_code.as_deref(), Some("vid-pid-mismatch")); + assert_eq!(backend.calls.len(), 1); + } + + #[test] + fn class_serial_and_parent_mismatches_each_fail_closed() { + type Mutation = fn(&mut UsbPnpDevice); + let cases: [(&str, Mutation); 4] = [ + ("device-class-mismatch", |device: &mut UsbPnpDevice| { + device.device_class = "USB".to_string(); + }), + ("serial-mismatch", |device: &mut UsbPnpDevice| { + device.serial = Some("different".to_string()); + }), + ("parent-mismatch", |device: &mut UsbPnpDevice| { + device.parent_instance_id = Some("USB\\ROOT_HUB30\\different".to_string()); + }), + ("problem-code-mismatch", |device: &mut UsbPnpDevice| { + device.health = UsbRecoveryHealth::PresentProblem { problem_code: 31 }; + }), + ]; + for (expected_error, mutate) in cases { + let mut target = device(UsbRecoveryHealth::PresentProblem { problem_code: 43 }); + mutate(&mut target); + let mut backend = FakePnp::with_observations(vec![target]); + let result = execute_recovery(&request(), "nonce".to_string(), &mut backend); + assert!(!result.success, "{expected_error}"); + assert_eq!(result.error_code.as_deref(), Some(expected_error)); + assert_eq!(backend.calls.len(), 1, "{expected_error}"); + } + } + + #[test] + fn unhealthy_result_after_success_remains_advisory_not_a_port() { + let mut backend = FakePnp::with_observations(vec![ + device(UsbRecoveryHealth::PresentProblem { problem_code: 43 }), + device(UsbRecoveryHealth::PresentProblem { problem_code: 43 }), + ]); + + let result = execute_recovery(&request(), "nonce".to_string(), &mut backend); + + assert!(result.success); + assert!(matches!( + result.after, + UsbRecoveryHealth::PresentProblem { .. } + )); + assert!(result.validated_instance_id.is_some()); + } +}