From 43abeb4b30b01974ec1025d6d6b2978468b59cb7 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Fri, 19 Jun 2026 17:43:06 +0200 Subject: [PATCH 1/9] Flash via udisks2 on Linux instead of requiring root --- Cargo.lock | 5 + Cargo.toml | 4 + crates/hai-core/Cargo.toml | 5 + crates/hai-core/src/disk_writer.rs | 385 ++++++++++++++++++++--------- crates/hai-core/src/error.rs | 12 + crates/hai-desktop/src/commands.rs | 8 +- 6 files changed, 297 insertions(+), 122 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5bb5b1..f92aa10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1476,6 +1476,7 @@ dependencies = [ "directories", "futures-util", "hex", + "libc", "mockito", "plist", "reqwest", @@ -1489,6 +1490,7 @@ dependencies = [ "tokio", "urlencoding", "xz2", + "zbus", ] [[package]] @@ -4408,8 +4410,10 @@ dependencies = [ "mio", "parking_lot", "pin-project-lite", + "signal-hook-registry", "socket2", "tokio-macros", + "tracing", "windows-sys 0.61.2", ] @@ -5711,6 +5715,7 @@ dependencies = [ "ordered-stream", "serde", "serde_repr", + "tokio", "tracing", "uds_windows", "uuid", diff --git a/Cargo.toml b/Cargo.toml index 778188e..f254889 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,10 @@ mockito = "1" plist = "1" security-framework = "2" +# Linux specific +zbus = { version = "5", default-features = false, features = ["tokio"] } +libc = "0.2" + # Tauri tauri = { version = "2", features = [] } tauri-build = { version = "2", features = [] } diff --git a/crates/hai-core/Cargo.toml b/crates/hai-core/Cargo.toml index 1199a66..bc6efc5 100644 --- a/crates/hai-core/Cargo.toml +++ b/crates/hai-core/Cargo.toml @@ -30,6 +30,11 @@ urlencoding = { workspace = true } plist = { workspace = true } security-framework = { workspace = true } +# Linux specific +[target.'cfg(target_os = "linux")'.dependencies] +zbus = { workspace = true } +libc = { workspace = true } + [dev-dependencies] tokio = { workspace = true } tempfile = { workspace = true } diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index d64f425..e91e93b 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -527,10 +527,14 @@ mod macos { #[cfg(target_os = "linux")] mod linux { use super::*; + use std::collections::HashMap; use std::fs::File; - use std::io::{Read, Write}; + use std::io::{Read, Seek, SeekFrom, Write}; use std::process::Command; use std::sync::mpsc; + use zbus::proxy::CacheProperties; + use zbus::zvariant::{OwnedFd, OwnedObjectPath, Value}; + use zbus::Connection; /// Progress update sent from blocking task struct ProgressUpdate { @@ -546,7 +550,12 @@ mod linux { verify: bool, progress_callback: &P, ) -> Result<()> { - unmount_device(device_id)?; + let connection = Connection::system() + .await + .map_err(|e| map_udisks_error(e, "connecting to the system bus"))?; + let block_path = resolve_block_path(&connection, device_id).await?; + + unmount_device(&connection, device_id).await; let image_size = std::fs::metadata(image_path)?.len(); @@ -558,100 +567,53 @@ mod linux { message: "Writing image to device...".to_string(), }); - // Create channel for progress updates from blocking task + // Create channel for progress updates from the blocking task. let (progress_tx, progress_rx) = mpsc::channel::(); + let device = open_device_rw(&connection, &block_path).await?; let image_path_clone = image_path.clone(); - let device_id_clone = device_id.to_string(); let write_handle = tokio::task::spawn_blocking(move || { - write_to_device(&image_path_clone, &device_id_clone, image_size, progress_tx) + write_and_verify(&image_path_clone, device, image_size, verify, progress_tx) }); - // Forward progress updates while waiting for write to complete + let forward = |update: ProgressUpdate| { + let progress = if update.total_bytes > 0 { + ((update.bytes_processed as f64 / update.total_bytes as f64) * 100.0) as u8 + } else { + 0 + }; + progress_callback.on_progress(FlashProgress { + stage: update.stage, + progress, + bytes_processed: update.bytes_processed, + total_bytes: update.total_bytes, + message: update.message, + }); + }; + loop { match progress_rx.recv_timeout(std::time::Duration::from_millis(100)) { - Ok(update) => { - let progress = if update.total_bytes > 0 { - ((update.bytes_processed as f64 / update.total_bytes as f64) * 100.0) as u8 - } else { - 0 - }; - progress_callback.on_progress(FlashProgress { - stage: update.stage, - progress, - bytes_processed: update.bytes_processed, - total_bytes: update.total_bytes, - message: update.message, - }); - } + Ok(update) => forward(update), Err(mpsc::RecvTimeoutError::Timeout) => { if write_handle.is_finished() { break; } } - Err(mpsc::RecvTimeoutError::Disconnected) => { - break; - } + Err(mpsc::RecvTimeoutError::Disconnected) => break, } } + // Drain updates the task buffered after it finished (e.g. the final + // "Write complete" / "Verification complete") so they aren't lost. + while let Ok(update) = progress_rx.try_recv() { + forward(update); + } + write_handle .await .map_err(|e| Error::Io(std::io::Error::other(e)))??; - if verify { - progress_callback.on_progress(FlashProgress { - stage: FlashStage::Verifying, - progress: 0, - bytes_processed: 0, - total_bytes: image_size, - message: "Verifying written data...".to_string(), - }); - - let (verify_tx, verify_rx) = mpsc::channel::(); - - let image_path_clone = image_path.clone(); - let device_id_clone = device_id.to_string(); - - let verify_handle = tokio::task::spawn_blocking(move || { - verify_write(&image_path_clone, &device_id_clone, image_size, verify_tx) - }); - - // Forward verify progress updates - loop { - match verify_rx.recv_timeout(std::time::Duration::from_millis(100)) { - Ok(update) => { - let progress = if update.total_bytes > 0 { - ((update.bytes_processed as f64 / update.total_bytes as f64) * 100.0) - as u8 - } else { - 0 - }; - progress_callback.on_progress(FlashProgress { - stage: update.stage, - progress, - bytes_processed: update.bytes_processed, - total_bytes: update.total_bytes, - message: update.message, - }); - } - Err(mpsc::RecvTimeoutError::Timeout) => { - if verify_handle.is_finished() { - break; - } - } - Err(mpsc::RecvTimeoutError::Disconnected) => { - break; - } - } - } - - verify_handle - .await - .map_err(|e| Error::Io(std::io::Error::other(e)))??; - } - progress_callback.on_progress(FlashProgress { stage: FlashStage::Finalizing, progress: 0, @@ -673,44 +635,219 @@ mod linux { Ok(()) } - fn unmount_device(device_id: &str) -> Result<()> { - let _ = Command::new("umount") - .args(["--all-targets", device_id]) - .output(); + fn map_udisks_error(err: zbus::Error, context: &str) -> Error { + // Remedy only; the DiskServiceUnavailable variant supplies the + // "Disk service unavailable:" prefix. + const UNAVAILABLE: &str = "install and enable udisks2 to flash drives."; + + if let zbus::Error::MethodError(name, message, _) = &err { + let name = name.as_str(); + // udisks2 isn't installed / not activatable on the bus. + if name.contains("ServiceUnknown") || name.contains("NameHasNoOwner") { + return Error::DiskServiceUnavailable(UNAVAILABLE.to_string()); + } + // User dismissed the polkit dialog. + if name.contains("NotAuthorizedDismissed") { + return Error::PermissionDenied("Authorization was canceled".to_string()); + } + if name.contains("NotAuthorized") { + return Error::PermissionDenied( + message + .clone() + .unwrap_or_else(|| "Not authorized to access the device".to_string()), + ); + } + // udisks may use the dedicated DeviceBusy name, but often reports a + // busy device as Error.Failed with "Device or resource busy" in the + // message instead, so check both. + if name.contains("DeviceBusy") + || message + .as_deref() + .is_some_and(|m| m.contains("Device or resource busy")) + { + return Error::DeviceBusy(context.to_string()); + } + return Error::PermissionDenied(format!("udisks2 error while {context}: {err}")); + } + + // Not a method error → couldn't reach the bus/service at all. + Error::DiskServiceUnavailable(format!("{UNAVAILABLE} ({err})")) + } + + #[zbus::proxy( + interface = "org.freedesktop.UDisks2.Manager", + default_service = "org.freedesktop.UDisks2", + default_path = "/org/freedesktop/UDisks2/Manager", + gen_blocking = false + )] + trait UDisks2Manager { + /// Resolve a device spec like `{"path": "/dev/sdb"}` to block-object paths. + fn resolve_device( + &self, + devspec: HashMap<&str, Value<'_>>, + options: HashMap<&str, Value<'_>>, + ) -> zbus::Result>; + } + + #[zbus::proxy( + interface = "org.freedesktop.UDisks2.Block", + default_service = "org.freedesktop.UDisks2", + gen_blocking = false + )] + trait UDisks2Block { + /// Open the whole device; `mode` is `"r"`, `"w"`, or `"rw"`. + fn open_device( + &self, + mode: &str, + options: HashMap<&str, Value<'_>>, + ) -> zbus::Result; + } + + #[zbus::proxy( + interface = "org.freedesktop.UDisks2.Filesystem", + default_service = "org.freedesktop.UDisks2", + gen_blocking = false + )] + trait UDisks2Filesystem { + fn unmount(&self, options: HashMap<&str, Value<'_>>) -> zbus::Result<()>; + } + + async fn resolve_block_path(conn: &Connection, device_id: &str) -> Result { + let manager = UDisks2ManagerProxy::new(conn) + .await + .map_err(|e| map_udisks_error(e, "connecting to udisks2"))?; + let mut devspec = HashMap::new(); + devspec.insert("path", Value::from(device_id)); + + let paths = manager + .resolve_device(devspec, HashMap::new()) + .await + .map_err(|e| map_udisks_error(e, "resolving the device"))?; + + paths + .into_iter() + .next() + .ok_or_else(|| Error::DeviceNotFound(device_id.to_string())) + } + + /// Open the device read-write via udisks2 (raises the polkit prompt). + async fn open_device_rw(conn: &Connection, path: &OwnedObjectPath) -> Result { + let block = UDisks2BlockProxy::builder(conn) + .path(path.to_string()) + .map_err(|e| map_udisks_error(e, "addressing the device"))? + .cache_properties(CacheProperties::No) + .build() + .await + .map_err(|e| map_udisks_error(e, "addressing the device"))?; + // O_SYNC so each write reaches the card before returning, instead of + // buffering into RAM and stalling on one big flush at the end — keeps + // the progress bar tracking real flash speed. + let mut options = HashMap::new(); + options.insert("flags", Value::from(libc::O_SYNC)); + let fd = block + .open_device("rw", options) + .await + .map_err(|e| map_udisks_error(e, "opening the device"))?; + Ok(File::from(std::os::fd::OwnedFd::from(fd))) + } + + fn write_and_verify( + image_path: &PathBuf, + mut device: File, + total_size: u64, + verify: bool, + progress_tx: mpsc::Sender, + ) -> Result<()> { + write_to_device(image_path, &mut device, total_size, &progress_tx)?; + + if verify { + // Evict the pages we just wrote from the cache so the read-back + // comes from the medium, not RAM + drop_device_cache(&device); + + // Tag verify-phase failures as VerificationFailed so the caller can + // label them "Verification failed" rather than "Write failed". + let verified = (|| { + device.seek(SeekFrom::Start(0)).map_err(|e| { + if is_drive_disconnected(&e) { + Error::DriveDisconnected + } else { + Error::Io(e) + } + })?; + verify_write(image_path, &mut device, total_size, &progress_tx) + })(); + + verified.map_err(|e| match e { + Error::VerificationFailed(_) | Error::DriveDisconnected => e, + other => Error::VerificationFailed(other.to_string()), + })?; + } + + Ok(()) + } + + /// Best-effort: drop the kernel page cache for the whole device so a + /// subsequent read-back hits the physical medium instead of the copy we + /// just wrote. Failures are advisory and ignored. + fn drop_device_cache(device: &File) { + use std::os::fd::AsRawFd; + // SAFETY: `device` owns the fd and keeps it open for this call. A `len` + // of 0 means "to end of file"; the return value is purely advisory. + unsafe { + libc::posix_fadvise(device.as_raw_fd(), 0, 0, libc::POSIX_FADV_DONTNEED); + } + } + + /// Block-object names to unmount before writing: the device plus its first + /// 16 partitions (`sdb`+`sdb1…`, or `mmcblk0`+`mmcblk0p1…`). + fn partition_block_names(device_id: &str) -> Vec { + let base = device_id + .strip_prefix("/dev/") + .unwrap_or(device_id) + .to_string(); + // Names ending in a digit (mmcblk0, nvme0n1, loop0) take a 'p' before the + // partition index; sd*/vd* append it directly. + let needs_p = base.chars().last().is_some_and(|c| c.is_ascii_digit()); + let mut names = Vec::with_capacity(17); + names.push(base.clone()); for i in 1..=16 { - let partition = if device_id.contains("mmcblk") || device_id.contains("nvme") { - format!("{}p{}", device_id, i) + names.push(if needs_p { + format!("{base}p{i}") } else { - format!("{}{}", device_id, i) - }; - let _ = Command::new("umount").arg(&partition).output(); + format!("{base}{i}") + }); } + names + } - Ok(()) + /// Best-effort: unmount failures (not mounted, no fs, unknown object) are + /// ignored — we only need the device free before writing. + async fn unmount_device(conn: &Connection, device_id: &str) { + for name in partition_block_names(device_id) { + let object_path = format!("/org/freedesktop/UDisks2/block_devices/{name}"); + let builder = match UDisks2FilesystemProxy::builder(conn).path(object_path) { + Ok(builder) => builder, + Err(_) => continue, + }; + let proxy = match builder.cache_properties(CacheProperties::No).build().await { + Ok(proxy) => proxy, + Err(_) => continue, + }; + let mut options = HashMap::new(); + options.insert("force", Value::from(true)); + let _ = proxy.unmount(options).await; + } } fn write_to_device( image_path: &PathBuf, - device_path: &str, + dest: &mut File, total_size: u64, - progress_tx: mpsc::Sender, + progress_tx: &mpsc::Sender, ) -> Result<()> { let mut source = File::open(image_path)?; - let mut dest = std::fs::OpenOptions::new() - .write(true) - .open(device_path) - .map_err(|e| { - if e.kind() == std::io::ErrorKind::PermissionDenied { - Error::PermissionDenied( - "Root access required. Please run with sudo.".to_string(), - ) - } else if is_drive_disconnected(&e) { - Error::DriveDisconnected - } else { - Error::Io(e) - } - })?; let mut buffer = vec![0u8; WRITE_BUFFER_SIZE]; let mut bytes_written: u64 = 0; @@ -765,18 +902,11 @@ mod linux { fn verify_write( image_path: &PathBuf, - device_path: &str, + dest: &mut File, total_size: u64, - progress_tx: mpsc::Sender, + progress_tx: &mpsc::Sender, ) -> Result<()> { let mut source = File::open(image_path)?; - let mut dest = File::open(device_path).map_err(|e| { - if is_drive_disconnected(&e) { - Error::DriveDisconnected - } else { - Error::Io(e) - } - })?; let mut source_buffer = vec![0u8; WRITE_BUFFER_SIZE]; let mut dest_buffer = vec![0u8; WRITE_BUFFER_SIZE]; @@ -834,11 +964,18 @@ mod linux { use super::*; #[test] - fn test_unmount_device_nonexistent() { - // Test unmounting a device that doesn't exist - let result = unmount_device("/dev/nonexistent999"); - // Should succeed because we ignore errors from umount - assert!(result.is_ok()); + fn test_partition_block_names_sd() { + let names = partition_block_names("/dev/sdb"); + assert_eq!(names[0], "sdb"); + assert_eq!(names[1], "sdb1"); + assert_eq!(names[16], "sdb16"); + } + + #[test] + fn test_partition_block_names_mmcblk_inserts_p() { + let names = partition_block_names("/dev/mmcblk0"); + assert_eq!(names[0], "mmcblk0"); + assert_eq!(names[1], "mmcblk0p1"); } } } @@ -1535,7 +1672,11 @@ mod tests { async fn test_write_image_nonexistent_file() { let callback = TestProgressCallback::new(); let image_path = PathBuf::from("/tmp/nonexistent_image_file.img"); - let device_id = "/dev/sdb"; + // Non-resolvable device: write_image now fails at device resolution + // before the image is ever opened, so this checks the error path + // generically (no real device touched, no polkit prompt) rather than + // the missing-file case specifically. + let device_id = "/dev/hai-test-nonexistent"; let result = write_image(&image_path, device_id, false, &callback).await; assert!(result.is_err()); @@ -1549,8 +1690,8 @@ mod tests { std::fs::write(temp_file.path(), b"test data").unwrap(); let image_path = temp_file.path().to_path_buf(); - // This will fail with permission denied unless running as root - let device_id = "/dev/null"; // Use /dev/null as a safe test target + // Non-resolvable device: fails before any privileged udisks2 call. + let device_id = "/dev/hai-test-nonexistent"; let result = write_image(&image_path, device_id, false, &callback).await; // Could be either permission denied or other error @@ -1663,8 +1804,10 @@ mod tests { #[cfg(target_os = "macos")] let device_id = "/dev/disk2"; + // Non-resolvable device so write_image fails before any privileged + // udisks2 call (mock mode isn't honoured by write_image itself). #[cfg(target_os = "linux")] - let device_id = "/dev/sdb"; + let device_id = "/dev/hai-test-nonexistent"; #[cfg(target_os = "windows")] let device_id = "\\\\.\\PhysicalDrive1"; @@ -1690,7 +1833,7 @@ mod tests { #[cfg(target_os = "macos")] let device_id = "/dev/disk99"; #[cfg(target_os = "linux")] - let device_id = "/dev/sdb99"; + let device_id = "/dev/hai-test-nonexistent"; #[cfg(target_os = "windows")] let device_id = "\\\\.\\PhysicalDrive99"; @@ -1914,7 +2057,7 @@ mod tests { #[cfg(target_os = "macos")] let device_id = "/dev/disk999"; #[cfg(target_os = "linux")] - let device_id = "/dev/sdz99"; + let device_id = "/dev/hai-test-nonexistent"; #[cfg(target_os = "windows")] let device_id = "\\\\.\\PhysicalDrive999"; diff --git a/crates/hai-core/src/error.rs b/crates/hai-core/src/error.rs index 7ace711..93c4c46 100644 --- a/crates/hai-core/src/error.rs +++ b/crates/hai-core/src/error.rs @@ -23,6 +23,9 @@ pub enum Error { #[error("Permission denied: {0}")] PermissionDenied(String), + #[error("Disk service unavailable: {0}")] + DiskServiceUnavailable(String), + #[error("Operation cancelled")] Cancelled, @@ -117,6 +120,15 @@ mod tests { assert!(msg.contains("Need root access")); } + #[test] + fn test_display_disk_service_unavailable() { + let error = Error::DiskServiceUnavailable("udisks2 is not available".to_string()); + assert_eq!( + error.to_string(), + "Disk service unavailable: udisks2 is not available" + ); + } + #[test] fn test_display_cancelled() { let error = Error::Cancelled; diff --git a/crates/hai-desktop/src/commands.rs b/crates/hai-desktop/src/commands.rs index ece5e18..88cacce 100644 --- a/crates/hai-desktop/src/commands.rs +++ b/crates/hai-desktop/src/commands.rs @@ -199,7 +199,13 @@ pub async fn flash_image( &callback, ) .await - .map_err(|e| format!("Write failed: {}", e))?; + .map_err(|e| match e { + // Verify-phase failures are tagged VerificationFailed; the rest are writes. + hai_core::Error::VerificationFailed(msg) => format!("Verification failed: {}", msg), + // Already carries its own "Disk service unavailable:" prefix. + err @ hai_core::Error::DiskServiceUnavailable(_) => err.to_string(), + other => format!("Write failed: {}", other), + })?; // Clean up extracted image let _ = tokio::fs::remove_file(&extracted_path).await; From c3251e36ffa5ccd63cb1cfac72ff231c034b5dbd Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Tue, 7 Jul 2026 10:45:11 +0200 Subject: [PATCH 2/9] test: cover udisks2 error mapping and Linux write/verify paths --- crates/hai-core/src/disk_writer.rs | 126 +++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index 4242d96..b8ad7c3 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -964,6 +964,8 @@ mod linux { #[cfg(test)] mod tests { use super::*; + use zbus::message::Message; + use zbus::names::OwnedErrorName; #[test] fn test_partition_block_names_sd() { @@ -979,6 +981,130 @@ mod linux { assert_eq!(names[0], "mmcblk0"); assert_eq!(names[1], "mmcblk0p1"); } + + fn method_error(name: &str, message: Option<&str>) -> zbus::Error { + let msg = Message::method_call("/", "Test") + .unwrap() + .build(&()) + .unwrap(); + zbus::Error::MethodError( + OwnedErrorName::try_from(name).unwrap(), + message.map(String::from), + msg, + ) + } + + #[test] + fn test_map_udisks_error_service_unknown() { + let err = method_error("org.freedesktop.DBus.Error.ServiceUnknown", None); + let mapped = map_udisks_error(err, "resolving the device"); + assert!(matches!(mapped, Error::DiskServiceUnavailable(_))); + } + + #[test] + fn test_map_udisks_error_authorization_dismissed() { + // Must match before the generic NotAuthorized branch, since the + // name contains "NotAuthorized" as a prefix. + let err = method_error( + "org.freedesktop.UDisks2.Error.NotAuthorizedDismissed", + Some("Not authorized to perform operation"), + ); + let mapped = map_udisks_error(err, "opening the device"); + assert!( + matches!(mapped, Error::PermissionDenied(msg) if msg == "Authorization was canceled") + ); + } + + #[test] + fn test_map_udisks_error_not_authorized_uses_message() { + let err = method_error( + "org.freedesktop.UDisks2.Error.NotAuthorizedCanObtain", + Some("Not authorized to open the device"), + ); + let mapped = map_udisks_error(err, "opening the device"); + assert!( + matches!(mapped, Error::PermissionDenied(msg) if msg == "Not authorized to open the device") + ); + } + + #[test] + fn test_map_udisks_error_busy_from_failed_message() { + let err = method_error( + "org.freedesktop.UDisks2.Error.Failed", + Some("Error opening device /dev/sdb: Device or resource busy"), + ); + let mapped = map_udisks_error(err, "opening the device"); + assert!(matches!(mapped, Error::DeviceBusy(_))); + } + + #[test] + fn test_map_udisks_error_bus_unreachable() { + let err = zbus::Error::Failure("could not connect".to_string()); + let mapped = map_udisks_error(err, "connecting to the system bus"); + assert!(matches!(mapped, Error::DiskServiceUnavailable(_))); + } + + #[test] + fn test_write_to_device_copies_image() { + let data: Vec = (0..123_456u32).map(|i| (i % 251) as u8).collect(); + let image = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(image.path(), &data).unwrap(); + + let mut dest = tempfile::tempfile().unwrap(); + let (tx, rx) = mpsc::channel(); + write_to_device( + &image.path().to_path_buf(), + &mut dest, + data.len() as u64, + &tx, + ) + .unwrap(); + + dest.seek(SeekFrom::Start(0)).unwrap(); + let mut written = Vec::new(); + dest.read_to_end(&mut written).unwrap(); + assert_eq!(written, data); + + let last = rx.try_iter().last().unwrap(); + assert_eq!(last.stage, FlashStage::Writing); + assert_eq!(last.bytes_processed, data.len() as u64); + } + + #[test] + fn test_verify_write_detects_mismatch() { + let image = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(image.path(), b"expected data").unwrap(); + + let mut dest = tempfile::tempfile().unwrap(); + dest.write_all(b"corrupted data").unwrap(); + dest.seek(SeekFrom::Start(0)).unwrap(); + + let (tx, _rx) = mpsc::channel(); + let result = verify_write(&image.path().to_path_buf(), &mut dest, 13, &tx); + assert!(matches!(result, Err(Error::VerificationFailed(_)))); + } + + #[test] + fn test_write_and_verify_roundtrip() { + let data: Vec = (0..65_536u32).map(|i| (i % 199) as u8).collect(); + let image = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(image.path(), &data).unwrap(); + + let device = tempfile::tempfile().unwrap(); + let (tx, rx) = mpsc::channel(); + write_and_verify( + &image.path().to_path_buf(), + device, + data.len() as u64, + true, + tx, + ) + .unwrap(); + + let stages: Vec = rx.try_iter().map(|u| u.stage).collect(); + assert!(stages.contains(&FlashStage::Writing)); + assert!(stages.contains(&FlashStage::Verifying)); + } } } From 1c1572cfe0067b7339a0936a4ab18b488b194cf9 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Tue, 7 Jul 2026 10:47:13 +0200 Subject: [PATCH 3/9] fix: correct udisks2 error labeling and polkit prompt timing --- crates/hai-core/src/disk_writer.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index b8ad7c3..0d582c8 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -561,6 +561,8 @@ mod linux { let image_size = std::fs::metadata(image_path)?.len(); + let device = open_device_rw(&connection, &block_path).await?; + progress_callback.on_progress(FlashProgress { stage: FlashStage::Writing, progress: 0, @@ -572,7 +574,6 @@ mod linux { // Create channel for progress updates from the blocking task. let (progress_tx, progress_rx) = mpsc::channel::(); - let device = open_device_rw(&connection, &block_path).await?; let image_path_clone = image_path.clone(); let write_handle = tokio::task::spawn_blocking(move || { @@ -669,7 +670,9 @@ mod linux { { return Error::DeviceBusy(context.to_string()); } - return Error::PermissionDenied(format!("udisks2 error while {context}: {err}")); + return Error::Io(std::io::Error::other(format!( + "udisks2 error while {context}: {err}" + ))); } // Not a method error → couldn't reach the bus/service at all. From d2ab87e75c206045413f2b178ea3326ab7ee4f16 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 14:33:53 +0200 Subject: [PATCH 4/9] Add O_EXCL to open options --- crates/hai-core/src/disk_writer.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index 0d582c8..58b9734 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -745,11 +745,12 @@ mod linux { .build() .await .map_err(|e| map_udisks_error(e, "addressing the device"))?; - // O_SYNC so each write reaches the card before returning, instead of - // buffering into RAM and stalling on one big flush at the end — keeps - // the progress bar tracking real flash speed. + + // O_EXCL: to make sure we have exlcusive access to the disk and error if not + // O_SYNC: so each write reaches the card before returning ot keep the + // progress bar in sync let mut options = HashMap::new(); - options.insert("flags", Value::from(libc::O_SYNC)); + options.insert("flags", Value::from(libc::O_EXCL | libc::O_SYNC)); let fd = block .open_device("rw", options) .await From bbf4579f556e71f3e85ef291f941d28e556dc9a2 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 14:34:15 +0200 Subject: [PATCH 5/9] Add prompt for password info for linux --- src/components/confirm-dialog.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/confirm-dialog.ts b/src/components/confirm-dialog.ts index 3cf0aa9..8b9ac3a 100644 --- a/src/components/confirm-dialog.ts +++ b/src/components/confirm-dialog.ts @@ -82,9 +82,9 @@ export class ConfirmDialog extends LitElement { All data on ${this.driveName} will be permanently erased. This action cannot be undone.

- ${this._isMacOS() + ${this._promptsForPassword() ? html`

- You will be prompted for your password to allow writing to the + You may be prompted for your password to allow writing to the drive. This is required because writing to external drives needs administrator privileges.

` @@ -139,8 +139,12 @@ export class ConfirmDialog extends LitElement { ); } - private _isMacOS(): boolean { - return navigator.platform.toLowerCase().includes("mac"); + // macOS asks for admin credentials via Authorization Services; Linux raises + // a polkit prompt through udisks2. Windows requires launching elevated, so + // there is no in-flow prompt to announce. + private _promptsForPassword(): boolean { + const platform = navigator.platform.toLowerCase(); + return platform.includes("mac") || platform.includes("linux"); } } From 3ed22e4a26fa244e61ceab6deab5ef5783a01fe0 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 16:22:35 +0200 Subject: [PATCH 6/9] Unmount via udisks2 partition enumeration and propagate auth/service errors --- crates/hai-core/src/disk_writer.rs | 92 +++++++++++++----------------- 1 file changed, 41 insertions(+), 51 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index 58b9734..b3b596b 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -90,6 +90,8 @@ pub async fn write_image( verify: bool, progress_callback: &P, ) -> Result<()> { + std::fs::metadata(image_path)?; + // Safety check: refuse to write to system drives validate_device_path(device_id)?; @@ -557,7 +559,7 @@ mod linux { .map_err(|e| map_udisks_error(e, "connecting to the system bus"))?; let block_path = resolve_block_path(&connection, device_id).await?; - unmount_device(&connection, device_id).await; + unmount_device(&connection, &block_path).await?; let image_size = std::fs::metadata(image_path)?.len(); @@ -717,6 +719,16 @@ mod linux { fn unmount(&self, options: HashMap<&str, Value<'_>>) -> zbus::Result<()>; } + #[zbus::proxy( + interface = "org.freedesktop.UDisks2.PartitionTable", + default_service = "org.freedesktop.UDisks2", + gen_blocking = false + )] + trait UDisks2PartitionTable { + #[zbus(property)] + fn partitions(&self) -> zbus::Result>; + } + async fn resolve_block_path(conn: &Connection, device_id: &str) -> Result { let manager = UDisks2ManagerProxy::new(conn) .await @@ -745,9 +757,9 @@ mod linux { .build() .await .map_err(|e| map_udisks_error(e, "addressing the device"))?; - - // O_EXCL: to make sure we have exlcusive access to the disk and error if not - // O_SYNC: so each write reaches the card before returning ot keep the + + // O_EXCL: to make sure we have exclusive access to the disk and error if not + // O_SYNC: so each write reaches the card before returning to keep the // progress bar in sync let mut options = HashMap::new(); options.insert("flags", Value::from(libc::O_EXCL | libc::O_SYNC)); @@ -806,34 +818,21 @@ mod linux { } } - /// Block-object names to unmount before writing: the device plus its first - /// 16 partitions (`sdb`+`sdb1…`, or `mmcblk0`+`mmcblk0p1…`). - fn partition_block_names(device_id: &str) -> Vec { - let base = device_id - .strip_prefix("/dev/") - .unwrap_or(device_id) - .to_string(); - // Names ending in a digit (mmcblk0, nvme0n1, loop0) take a 'p' before the - // partition index; sd*/vd* append it directly. - let needs_p = base.chars().last().is_some_and(|c| c.is_ascii_digit()); - let mut names = Vec::with_capacity(17); - names.push(base.clone()); - for i in 1..=16 { - names.push(if needs_p { - format!("{base}p{i}") - } else { - format!("{base}{i}") - }); + /// Unmount everything on the device before writing. + async fn unmount_device(conn: &Connection, block_path: &OwnedObjectPath) -> Result<()> { + let mut targets = vec![block_path.clone()]; + // Unpartitioned media has no PartitionTable interface; the property + // read fails and only the whole-disk filesystem is unmounted. + if let Ok(builder) = UDisks2PartitionTableProxy::builder(conn).path(block_path.clone()) { + if let Ok(table) = builder.cache_properties(CacheProperties::No).build().await { + if let Ok(partitions) = table.partitions().await { + targets.extend(partitions); + } + } } - names - } - /// Best-effort: unmount failures (not mounted, no fs, unknown object) are - /// ignored — we only need the device free before writing. - async fn unmount_device(conn: &Connection, device_id: &str) { - for name in partition_block_names(device_id) { - let object_path = format!("/org/freedesktop/UDisks2/block_devices/{name}"); - let builder = match UDisks2FilesystemProxy::builder(conn).path(object_path) { + for path in targets { + let builder = match UDisks2FilesystemProxy::builder(conn).path(path) { Ok(builder) => builder, Err(_) => continue, }; @@ -843,8 +842,16 @@ mod linux { }; let mut options = HashMap::new(); options.insert("force", Value::from(true)); - let _ = proxy.unmount(options).await; + if let Err(e) = proxy.unmount(options).await { + if let err @ (Error::PermissionDenied(_) | Error::DiskServiceUnavailable(_)) = + map_udisks_error(e, "unmounting a volume") + { + return Err(err); + } + } } + + Ok(()) } fn write_to_device( @@ -971,21 +978,6 @@ mod linux { use zbus::message::Message; use zbus::names::OwnedErrorName; - #[test] - fn test_partition_block_names_sd() { - let names = partition_block_names("/dev/sdb"); - assert_eq!(names[0], "sdb"); - assert_eq!(names[1], "sdb1"); - assert_eq!(names[16], "sdb16"); - } - - #[test] - fn test_partition_block_names_mmcblk_inserts_p() { - let names = partition_block_names("/dev/mmcblk0"); - assert_eq!(names[0], "mmcblk0"); - assert_eq!(names[1], "mmcblk0p1"); - } - fn method_error(name: &str, message: Option<&str>) -> zbus::Error { let msg = Message::method_call("/", "Test") .unwrap() @@ -1804,14 +1796,12 @@ mod tests { async fn test_write_image_nonexistent_file() { let callback = TestProgressCallback::new(); let image_path = PathBuf::from("/tmp/nonexistent_image_file.img"); - // Non-resolvable device: write_image now fails at device resolution - // before the image is ever opened, so this checks the error path - // generically (no real device touched, no polkit prompt) rather than - // the missing-file case specifically. let device_id = "/dev/hai-test-nonexistent"; + // The image check runs before device validation and any D-Bus + // call, so a missing image surfaces as Io even with a bad device. let result = write_image(&image_path, device_id, false, &callback).await; - assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::Io(_))); } #[tokio::test] From a589d2f68b701203acb8e3cc6196c842d97dc970 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 16:22:41 +0200 Subject: [PATCH 7/9] Require removable/hotplug Linux targets instead of a name deny-list --- crates/hai-core/src/disk_writer.rs | 145 +++++++++++------------------ 1 file changed, 55 insertions(+), 90 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index b3b596b..e4ecd80 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -53,20 +53,17 @@ fn validate_device_path(device_id: &str) -> Result<()> { #[cfg(target_os = "linux")] { - // On Linux, refuse to write to common system drive patterns - let dangerous_patterns = [ - "/dev/sda", // First SATA drive (often system) - "/dev/nvme0n1", // First NVMe drive (often system) - "/dev/vda", // First virtio drive (VMs) - ]; - - for pattern in dangerous_patterns { - if device_id == pattern { - return Err(Error::PermissionDenied(format!( - "{} appears to be a system drive and cannot be overwritten", - device_id - ))); - } + // Require the kernel-reported removable/hotplug signal (the same one + // device enumeration filters on) instead of a name deny-list: /dev/sda + // or /dev/nvme0n1 are legitimate USB targets on machines that boot + // from another disk. A mounted system drive is additionally caught by + // the exclusive (O_EXCL) open at write time. + let name = device_id.strip_prefix("/dev/").unwrap_or(device_id); + if name.is_empty() || !is_removable_or_hotplug(std::path::Path::new("/sys/block"), name) { + return Err(Error::PermissionDenied(format!( + "{} is not a removable drive and cannot be overwritten", + device_id + ))); } } @@ -83,6 +80,20 @@ fn validate_device_path(device_id: &str) -> Result<()> { Ok(()) } +/// Whether the kernel reports the drive as removable, or it sits on a +/// hot-pluggable bus (usb/mmc), which lsblk also treats as hotplug — +/// USB-attached disks often report `removable` as 0. +#[cfg(target_os = "linux")] +fn is_removable_or_hotplug(sys_block: &std::path::Path, name: &str) -> bool { + let dev = sys_block.join(name); + let removable = std::fs::read_to_string(dev.join("removable")).is_ok_and(|s| s.trim() == "1"); + let hotplug = std::fs::canonicalize(&dev).is_ok_and(|p| { + let p = p.to_string_lossy(); + p.contains("/usb") || p.contains("/mmc") + }); + removable || hotplug +} + /// Write an image file to a block device with progress updates pub async fn write_image( image_path: &PathBuf, @@ -1484,50 +1495,45 @@ mod tests { #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_blocks_sda_linux() { - let result = validate_device_path("/dev/sda"); - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); - } - - #[test] - #[cfg(target_os = "linux")] - fn test_validate_device_path_blocks_nvme0n1_linux() { - let result = validate_device_path("/dev/nvme0n1"); - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); + fn test_is_removable_or_hotplug_removable_flag() { + let sys = tempfile::tempdir().unwrap(); + std::fs::create_dir(sys.path().join("sdb")).unwrap(); + std::fs::write(sys.path().join("sdb/removable"), "1\n").unwrap(); + assert!(is_removable_or_hotplug(sys.path(), "sdb")); } #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_blocks_vda_linux() { - let result = validate_device_path("/dev/vda"); - assert!(result.is_err()); - assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); + fn test_is_removable_or_hotplug_blocks_internal_disk() { + let sys = tempfile::tempdir().unwrap(); + std::fs::create_dir(sys.path().join("sda")).unwrap(); + std::fs::write(sys.path().join("sda/removable"), "0\n").unwrap(); + assert!(!is_removable_or_hotplug(sys.path(), "sda")); } #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_allows_sdb_linux() { - assert!(validate_device_path("/dev/sdb").is_ok()); + fn test_is_removable_or_hotplug_usb_bus_counts_as_hotplug() { + let sys = tempfile::tempdir().unwrap(); + let real = sys.path().join("devices/pci0000:00/usb1/host0/block/sda"); + std::fs::create_dir_all(&real).unwrap(); + std::fs::write(real.join("removable"), "0\n").unwrap(); + std::os::unix::fs::symlink(&real, sys.path().join("sda")).unwrap(); + assert!(is_removable_or_hotplug(sys.path(), "sda")); } #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_allows_sdc_linux() { - assert!(validate_device_path("/dev/sdc").is_ok()); + fn test_is_removable_or_hotplug_blocks_unknown_device() { + let sys = tempfile::tempdir().unwrap(); + assert!(!is_removable_or_hotplug(sys.path(), "sdz")); } #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_allows_mmcblk0_linux() { - assert!(validate_device_path("/dev/mmcblk0").is_ok()); - } - - #[test] - #[cfg(target_os = "linux")] - fn test_validate_device_path_allows_nvme1n1_linux() { - assert!(validate_device_path("/dev/nvme1n1").is_ok()); + fn test_validate_device_path_blocks_device_absent_from_sysfs() { + let result = validate_device_path("/dev/hai-test-nonexistent"); + assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); } #[test] @@ -1567,9 +1573,10 @@ mod tests { #[test] fn test_validate_device_path_empty_string() { - // Empty string should be allowed (validation doesn't check for empty) - // This tests the current behavior let result = validate_device_path(""); + #[cfg(target_os = "linux")] + assert!(result.is_err()); + #[cfg(not(target_os = "linux"))] assert!(result.is_ok()); } @@ -1707,8 +1714,10 @@ mod tests { #[cfg(target_os = "macos")] let result = write_image(&image_path, "/dev/disk0", false, &callback).await; + // Not in /sys/block, so validation treats it as non-removable — + // host-independent, unlike asserting on the machine's real /dev/sda. #[cfg(target_os = "linux")] - let result = write_image(&image_path, "/dev/sda", false, &callback).await; + let result = write_image(&image_path, "/dev/hai-test-nonexistent", false, &callback).await; #[cfg(target_os = "windows")] let result = write_image(&image_path, "\\\\.\\PhysicalDrive0", false, &callback).await; @@ -1772,25 +1781,6 @@ mod tests { mod linux_tests { use super::*; - #[test] - fn test_validate_all_dangerous_patterns() { - assert!(validate_device_path("/dev/sda").is_err()); - assert!(validate_device_path("/dev/nvme0n1").is_err()); - assert!(validate_device_path("/dev/vda").is_err()); - } - - #[test] - fn test_validate_safe_devices() { - assert!(validate_device_path("/dev/sdb").is_ok()); - assert!(validate_device_path("/dev/sdc").is_ok()); - assert!(validate_device_path("/dev/sdd").is_ok()); - assert!(validate_device_path("/dev/nvme1n1").is_ok()); - assert!(validate_device_path("/dev/nvme2n1").is_ok()); - assert!(validate_device_path("/dev/vdb").is_ok()); - assert!(validate_device_path("/dev/mmcblk0").is_ok()); - assert!(validate_device_path("/dev/mmcblk1").is_ok()); - } - #[tokio::test] #[serial] async fn test_write_image_nonexistent_file() { @@ -1819,15 +1809,6 @@ mod tests { // Could be either permission denied or other error assert!(result.is_ok() || result.is_err()); } - - #[test] - fn test_validate_mmcblk_and_nvme_partitions() { - // These should all pass - they're not in the dangerous list - assert!(validate_device_path("/dev/mmcblk0p1").is_ok()); - assert!(validate_device_path("/dev/nvme0n1p1").is_ok()); - // But nvme0n1 itself should be blocked - assert!(validate_device_path("/dev/nvme0n1").is_err()); - } } // Windows-specific tests @@ -1984,21 +1965,6 @@ mod tests { } } - #[test] - #[cfg(target_os = "linux")] - fn test_validate_linux_all_dangerous_devices() { - // Test all dangerous patterns - assert!(validate_device_path("/dev/sda").is_err()); - assert!(validate_device_path("/dev/nvme0n1").is_err()); - assert!(validate_device_path("/dev/vda").is_err()); - - // Ensure partitions of these devices are OK - assert!(validate_device_path("/dev/sda1").is_ok()); - assert!(validate_device_path("/dev/sda2").is_ok()); - assert!(validate_device_path("/dev/nvme0n1p1").is_ok()); - assert!(validate_device_path("/dev/vda1").is_ok()); - } - #[test] #[cfg(target_os = "windows")] fn test_validate_windows_all_physical_drives() { @@ -2070,12 +2036,11 @@ mod tests { // Test the full validation path for various device IDs #[test] + #[cfg(not(target_os = "linux"))] fn test_validate_multiple_safe_devices() { let safe_devices = vec![ #[cfg(target_os = "macos")] "/dev/disk5", - #[cfg(target_os = "linux")] - "/dev/sde", #[cfg(target_os = "windows")] "\\\\.\\PhysicalDrive5", ]; From d4453a2174bb3361d8d89924a828419b22cd2fcb Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 17:23:28 +0200 Subject: [PATCH 8/9] merge linux validate_device_path tests --- crates/hai-core/src/disk_writer.rs | 54 ++++++++++-------------------- 1 file changed, 17 insertions(+), 37 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index e4ecd80..3483cac 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -1532,8 +1532,20 @@ mod tests { #[test] #[cfg(target_os = "linux")] fn test_validate_device_path_blocks_device_absent_from_sysfs() { - let result = validate_device_path("/dev/hai-test-nonexistent"); - assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); + // The /dev/ prefix and trailing slashes are normalized away, so all + // spellings hit the same sysfs lookup and get the same verdict. + for id in [ + "/dev/hai-test-nonexistent", + "hai-test-nonexistent", + "/dev/hai-test-nonexistent/", + ] { + match validate_device_path(id) { + Err(Error::PermissionDenied(msg)) => { + assert!(msg.contains("not a removable drive"), "{id}: {msg}"); + } + other => panic!("expected PermissionDenied for {id}, got {other:?}"), + } + } } #[test] @@ -2005,22 +2017,6 @@ mod tests { } } - #[test] - #[cfg(target_os = "linux")] - fn test_validation_error_messages_linux() { - let devices = vec!["/dev/sda", "/dev/nvme0n1", "/dev/vda"]; - for device in devices { - let result = validate_device_path(device); - assert!(result.is_err()); - match result { - Err(Error::PermissionDenied(msg)) => { - assert!(msg.contains(device) || msg.contains("system drive")); - } - _ => panic!("Expected PermissionDenied error for {}", device), - } - } - } - #[test] #[cfg(target_os = "windows")] fn test_validation_error_message_windows() { @@ -2094,18 +2090,10 @@ mod tests { // Test with path that doesn't have /dev/ prefix #[test] - #[cfg(any(target_os = "macos", target_os = "linux"))] + #[cfg(target_os = "macos")] fn test_validate_without_dev_prefix() { - #[cfg(target_os = "macos")] - { - assert!(validate_device_path("disk5").is_ok()); - assert!(validate_device_path("disk0").is_err()); - } - - #[cfg(target_os = "linux")] - { - assert!(validate_device_path("sdb").is_ok()); - } + assert!(validate_device_path("disk5").is_ok()); + assert!(validate_device_path("disk0").is_err()); } // Test case sensitivity @@ -2202,14 +2190,6 @@ mod tests { } } - // Test validation with slash variations - #[test] - #[cfg(target_os = "linux")] - fn test_validate_linux_with_trailing_slash() { - assert!(validate_device_path("/dev/sdb/").is_ok()); - assert!(validate_device_path("/dev/sda/").is_err()); - } - // Test multiple consecutive calls to progress callback #[test] fn test_progress_callback_multiple_calls() { From a3076fcd7bd5a1a8cbc9eb92a8f1221a7899ce09 Mon Sep 17 00:00:00 2001 From: Ariel Ebersberger Date: Wed, 2 Sep 2026 22:13:09 +0200 Subject: [PATCH 9/9] address review comments --- crates/hai-core/src/disk_writer.rs | 108 ++++++++++++-------- crates/hai-desktop/src/commands.rs | 2 + test/unit/components/confirm-dialog.test.ts | 37 +++++++ 3 files changed, 102 insertions(+), 45 deletions(-) diff --git a/crates/hai-core/src/disk_writer.rs b/crates/hai-core/src/disk_writer.rs index 3483cac..8b97608 100644 --- a/crates/hai-core/src/disk_writer.rs +++ b/crates/hai-core/src/disk_writer.rs @@ -53,13 +53,17 @@ fn validate_device_path(device_id: &str) -> Result<()> { #[cfg(target_os = "linux")] { - // Require the kernel-reported removable/hotplug signal (the same one - // device enumeration filters on) instead of a name deny-list: /dev/sda - // or /dev/nvme0n1 are legitimate USB targets on machines that boot - // from another disk. A mounted system drive is additionally caught by - // the exclusive (O_EXCL) open at write time. - let name = device_id.strip_prefix("/dev/").unwrap_or(device_id); - if name.is_empty() || !is_removable_or_hotplug(std::path::Path::new("/sys/block"), name) { + // Require the removable/hotplug signal device enumeration filters on + // instead of a name deny-list: /dev/sda or /dev/nvme0n1 are legitimate + // USB targets on machines that boot from another disk. A mounted + // system drive is additionally caught by the exclusive (O_EXCL) open + // at write time. + let device_path = if device_id.starts_with("/dev/") { + device_id.to_string() + } else { + format!("/dev/{}", device_id) + }; + if device_id.is_empty() || !is_removable_or_hotplug(&device_path) { return Err(Error::PermissionDenied(format!( "{} is not a removable drive and cannot be overwritten", device_id @@ -80,18 +84,40 @@ fn validate_device_path(device_id: &str) -> Result<()> { Ok(()) } -/// Whether the kernel reports the drive as removable, or it sits on a -/// hot-pluggable bus (usb/mmc), which lsblk also treats as hotplug — -/// USB-attached disks often report `removable` as 0. +/// Whether lsblk reports the drive as removable or hot-plugged — the same +/// signal (and tool) device enumeration filters on, so the two layers cannot +/// drift, and lsblk's bus-chain hotplug derivation (usb, mmc, thunderbolt, …) +/// is not reimplemented here. +#[cfg(target_os = "linux")] +fn is_removable_or_hotplug(device_path: &str) -> bool { + let output = match std::process::Command::new("lsblk") + .args(["--nodeps", "--json", "--output", "RM,HOTPLUG", device_path]) + .output() + { + Ok(output) if output.status.success() => output.stdout, + // Unknown device (or no lsblk at all): not a valid target. + _ => return false, + }; + parse_lsblk_removable(&output) +} + #[cfg(target_os = "linux")] -fn is_removable_or_hotplug(sys_block: &std::path::Path, name: &str) -> bool { - let dev = sys_block.join(name); - let removable = std::fs::read_to_string(dev.join("removable")).is_ok_and(|s| s.trim() == "1"); - let hotplug = std::fs::canonicalize(&dev).is_ok_and(|p| { - let p = p.to_string_lossy(); - p.contains("/usb") || p.contains("/mmc") - }); - removable || hotplug +fn parse_lsblk_removable(json: &[u8]) -> bool { + #[derive(serde::Deserialize)] + struct LsblkOutput { + blockdevices: Vec, + } + #[derive(serde::Deserialize)] + struct LsblkFlags { + #[serde(default)] + rm: Option, + #[serde(default)] + hotplug: Option, + } + serde_json::from_slice::(json) + .ok() + .and_then(|out| out.blockdevices.into_iter().next()) + .is_some_and(|dev| dev.rm == Some(true) || dev.hotplug == Some(true)) } /// Write an image file to a block device with progress updates @@ -1495,45 +1521,37 @@ mod tests { #[test] #[cfg(target_os = "linux")] - fn test_is_removable_or_hotplug_removable_flag() { - let sys = tempfile::tempdir().unwrap(); - std::fs::create_dir(sys.path().join("sdb")).unwrap(); - std::fs::write(sys.path().join("sdb/removable"), "1\n").unwrap(); - assert!(is_removable_or_hotplug(sys.path(), "sdb")); - } + fn test_parse_lsblk_removable_flags() { + let removable = br#"{"blockdevices": [{"rm": true, "hotplug": false}]}"#; + assert!(parse_lsblk_removable(removable)); - #[test] - #[cfg(target_os = "linux")] - fn test_is_removable_or_hotplug_blocks_internal_disk() { - let sys = tempfile::tempdir().unwrap(); - std::fs::create_dir(sys.path().join("sda")).unwrap(); - std::fs::write(sys.path().join("sda/removable"), "0\n").unwrap(); - assert!(!is_removable_or_hotplug(sys.path(), "sda")); + // USB/MMC disks often report rm=false but hotplug=true. + let hotplug = br#"{"blockdevices": [{"rm": false, "hotplug": true}]}"#; + assert!(parse_lsblk_removable(hotplug)); + + let internal = br#"{"blockdevices": [{"rm": false, "hotplug": false}]}"#; + assert!(!parse_lsblk_removable(internal)); } #[test] #[cfg(target_os = "linux")] - fn test_is_removable_or_hotplug_usb_bus_counts_as_hotplug() { - let sys = tempfile::tempdir().unwrap(); - let real = sys.path().join("devices/pci0000:00/usb1/host0/block/sda"); - std::fs::create_dir_all(&real).unwrap(); - std::fs::write(real.join("removable"), "0\n").unwrap(); - std::os::unix::fs::symlink(&real, sys.path().join("sda")).unwrap(); - assert!(is_removable_or_hotplug(sys.path(), "sda")); + fn test_parse_lsblk_removable_degenerate_output() { + assert!(!parse_lsblk_removable(br#"{"blockdevices": []}"#)); + assert!(!parse_lsblk_removable(br#"{"blockdevices": [{}]}"#)); + assert!(!parse_lsblk_removable(b"not json")); } #[test] #[cfg(target_os = "linux")] fn test_is_removable_or_hotplug_blocks_unknown_device() { - let sys = tempfile::tempdir().unwrap(); - assert!(!is_removable_or_hotplug(sys.path(), "sdz")); + assert!(!is_removable_or_hotplug("/dev/hai-test-nonexistent")); } #[test] #[cfg(target_os = "linux")] - fn test_validate_device_path_blocks_device_absent_from_sysfs() { + fn test_validate_device_path_blocks_unknown_device() { // The /dev/ prefix and trailing slashes are normalized away, so all - // spellings hit the same sysfs lookup and get the same verdict. + // spellings hit the same lsblk lookup and get the same verdict. for id in [ "/dev/hai-test-nonexistent", "hai-test-nonexistent", @@ -1814,12 +1832,12 @@ mod tests { std::fs::write(temp_file.path(), b"test data").unwrap(); let image_path = temp_file.path().to_path_buf(); - // Non-resolvable device: fails before any privileged udisks2 call. + // Unknown to lsblk, so validation rejects it before any + // privileged udisks2 call. let device_id = "/dev/hai-test-nonexistent"; let result = write_image(&image_path, device_id, false, &callback).await; - // Could be either permission denied or other error - assert!(result.is_ok() || result.is_err()); + assert!(matches!(result.unwrap_err(), Error::PermissionDenied(_))); } } diff --git a/crates/hai-desktop/src/commands.rs b/crates/hai-desktop/src/commands.rs index 95a3705..b0435bc 100644 --- a/crates/hai-desktop/src/commands.rs +++ b/crates/hai-desktop/src/commands.rs @@ -204,6 +204,8 @@ pub async fn flash_image( hai_core::Error::VerificationFailed(msg) => format!("Verification failed: {}", msg), // Already carries its own "Disk service unavailable:" prefix. err @ hai_core::Error::DiskServiceUnavailable(_) => err.to_string(), + // A disconnect doesn't require a prefix + err @ hai_core::Error::DriveDisconnected => err.to_string(), other => format!("Write failed: {}", other), })?; diff --git a/test/unit/components/confirm-dialog.test.ts b/test/unit/components/confirm-dialog.test.ts index e2aac3a..e99cc0b 100644 --- a/test/unit/components/confirm-dialog.test.ts +++ b/test/unit/components/confirm-dialog.test.ts @@ -281,4 +281,41 @@ describe("confirm-dialog", () => { expect(event.bubbles).to.be.true; expect(event.composed).to.be.true; }); + + describe("password note", () => { + const originalPlatform = navigator.platform; + + const setPlatform = (value: string) => { + Object.defineProperty(window.navigator, "platform", { + value, + configurable: true, + }); + }; + + afterEach(() => setPlatform(originalPlatform)); + + const cases: Array<{ label: string; platform: string; shown: boolean }> = [ + { label: "macOS", platform: "MacIntel", shown: true }, + { label: "Linux", platform: "Linux x86_64", shown: true }, + // Windows has no in-flow prompt: the app must already run elevated. + { label: "Windows", platform: "Win32", shown: false }, + ]; + + for (const { label, platform, shown } of cases) { + it(`${shown ? "shows" : "hides"} the note on ${label}`, async () => { + setPlatform(platform); + const el = await fixture(html` + + `); + + const note = el.shadowRoot!.querySelector(".password-note"); + if (shown) { + expect(note).to.exist; + expect(note!.textContent).to.contain("prompted for your password"); + } else { + expect(note).to.not.exist; + } + }); + } + }); });