fix(updater): prevent busy DMGs from blocking macOS updates

Prefer diskutil eject with retries and a compatibility fallback. Defer failed cleanup without blocking verified updates, preserve mounted directories, and add regression tests.
This commit is contained in:
lzt404 committed 2026-09-28 16:42:22 +08:00
1 parent c0c79b0bb6
commit 9991aea818
2 files changed
+277 -15

No files matched your search

+93 -15
View File
@@ -971,8 +971,12 @@ pub(crate) async fn download_and_stage_portable_app_update(
launch_portable_update_helper(app, &helper_path, &work_dir, &descriptor).await
}
.await;
// A busy DMG may still be mounted here. Never recursively clean its mount point.
if result.is_err() {
let _ = fs::remove_dir_all(&work_dir);
cleanup_macos_update_dmg(&work_dir);
if !work_dir.join("mount").exists() {
let _ = fs::remove_dir_all(&work_dir);
}
}
result
}
@@ -2113,7 +2117,13 @@ fn run_macos_update_command(command: &mut Command, action: &str) -> Result<(), S
return Ok(());
}
let stderr = String::from_utf8_lossy(&output.stderr);
Err(format!("Failed to {action}: {}", stderr.trim()))
let stdout = String::from_utf8_lossy(&output.stdout);
let details = [stderr.trim(), stdout.trim()]
.into_iter()
.filter(|text| !text.is_empty())
.collect::<Vec<_>>()
.join("; ");
Err(format!("Failed to {action} ({}): {details}", output.status))
}
#[cfg(target_os = "macos")]
@@ -2121,6 +2131,23 @@ pub(crate) fn stage_macos_application_from_dmg(
dmg_path: &Path,
work_dir: &Path,
staged_app: &Path,
) -> Result<(), String> {
stage_macos_application_from_dmg_with_commands(
dmg_path,
work_dir,
staged_app,
run_macos_update_command,
thread::sleep,
)
}
#[cfg(any(target_os = "macos", test))]
pub(crate) fn stage_macos_application_from_dmg_with_commands(
dmg_path: &Path,
work_dir: &Path,
staged_app: &Path,
mut run: impl FnMut(&mut Command, &str) -> Result<(), String>,
mut wait: impl FnMut(Duration),
) -> Result<(), String> {
let mount_dir = work_dir.join("mount");
fs::create_dir_all(&mount_dir).map_err(|error| format!("Failed to create DMG mount directory: {error}"))?;
@@ -2130,7 +2157,7 @@ pub(crate) fn stage_macos_application_from_dmg(
.arg(dmg_path)
.args(["-nobrowse", "-readonly", "-mountpoint"])
.arg(&mount_dir);
run_macos_update_command(&mut attach, "Mount application update DMG")?;
run(&mut attach, "Mount application update DMG")?;
let source_app = mount_dir.join("EasyCLIProxyAPI.app");
let stage_result = (|| -> Result<(), String> {
@@ -2144,27 +2171,79 @@ pub(crate) fn stage_macos_application_from_dmg(
.map_err(|error| format!("Failed to create application update staging directory: {error}"))?;
let mut ditto = Command::new("ditto");
ditto.arg(&source_app).arg(staged_app);
run_macos_update_command(&mut ditto, "Stage new macOS application")?;
run(&mut ditto, "Stage new macOS application")?;
let mut codesign = Command::new("codesign");
codesign
.args(["--verify", "--deep", "--strict"])
.arg(staged_app);
run_macos_update_command(&mut codesign, "Verify new macOS application signature")?;
run(&mut codesign, "Verify new macOS application signature")?;
let mut gatekeeper = Command::new("spctl");
gatekeeper
.args(["--assess", "--type", "execute", "--verbose=2"])
.arg(staged_app);
run_macos_update_command(&mut gatekeeper, "Verify new macOS application system trust status")
run(&mut gatekeeper, "Verify new macOS application system trust status")
})();
let mut detach = Command::new("hdiutil");
detach.arg("detach").arg(&mount_dir);
let detach_result = run_macos_update_command(&mut detach, "Unmount application update DMG");
match (stage_result, detach_result) {
(Err(error), _) => Err(error),
(Ok(()), Err(error)) => Err(error),
(Ok(()), Ok(())) => Ok(()),
match eject_macos_update_dmg_with_commands(&mount_dir, &mut run, &mut wait) {
Ok(()) => {
let _ = fs::remove_dir(&mount_dir);
}
Err(error) => {
// A successfully staged app no longer depends on the read-only DMG.
// Preserve staging errors, but never fail an update just for cleanup.
eprintln!("Application update DMG cleanup deferred: {error}");
}
}
stage_result
}
#[cfg(any(target_os = "macos", test))]
pub(crate) fn eject_macos_update_dmg_with_commands(
mount_dir: &Path,
mut run: impl FnMut(&mut Command, &str) -> Result<(), String>,
mut wait: impl FnMut(Duration),
) -> Result<(), String> {
let mut last_error = String::new();
// Spotlight or security scanning can briefly hold the newly mounted image busy.
for attempt in 0..3 {
if attempt > 0 {
wait(Duration::from_millis(500));
}
let mut eject = Command::new("diskutil");
eject.arg("eject").arg(mount_dir);
match run(&mut eject, "Eject application update DMG") {
Ok(()) => return Ok(()),
Err(error) => last_error = error,
}
}
// Compatibility fallback, limited to this updater's private read-only mount.
let mut detach = Command::new("hdiutil");
detach.args(["detach", "-force"]).arg(mount_dir);
run(&mut detach, "Force unmount application update DMG")
.map_err(|error| format!("{last_error}; {error}"))
}
#[cfg(target_os = "macos")]
fn cleanup_macos_update_dmg(work_dir: &Path) {
cleanup_macos_update_dmg_with_commands(work_dir, run_macos_update_command, thread::sleep);
}
#[cfg(any(target_os = "macos", test))]
pub(crate) fn cleanup_macos_update_dmg_with_commands(
work_dir: &Path,
run: impl FnMut(&mut Command, &str) -> Result<(), String>,
wait: impl FnMut(Duration),
) {
let mount_dir = work_dir.join("mount");
if mount_dir.exists() {
if let Err(error) = eject_macos_update_dmg_with_commands(&mount_dir, run, wait) {
eprintln!("Application update DMG cleanup deferred: {error}");
return;
}
let _ = fs::remove_dir(&mount_dir);
}
let _ = fs::remove_file(work_dir.join("update.dmg"));
}
#[cfg(target_os = "macos")]
@@ -2374,9 +2453,8 @@ pub(crate) fn replace_macos_application(descriptor: &MacosUpdateDescriptor) -> R
#[cfg(target_os = "macos")]
fn cleanup_macos_update_payload(descriptor_path: &Path, descriptor: &MacosUpdateDescriptor) {
let _ = fs::remove_file(descriptor.work_dir.join("update.dmg"));
cleanup_macos_update_dmg(&descriptor.work_dir);
let _ = fs::remove_dir_all(descriptor.work_dir.join("staging"));
let _ = fs::remove_dir_all(descriptor.work_dir.join("mount"));
let _ = fs::remove_file(&descriptor.ack_path);
let _ = fs::remove_file(portable_update_helper_ack_path(&descriptor.work_dir));
let _ = fs::remove_file(descriptor_path);
+184
View File
@@ -51,6 +51,190 @@ fn macos_update_helper_stays_inside_the_signed_app_bundle() {
assert_eq!(macos_update_helper_path(&current_exe), current_exe);
}
#[test]
fn macos_dmg_eject_retries_busy_mounts_and_stops_after_success() {
let mount_dir = Path::new("/private/tmp/update with spaces/mount");
for busy_attempts in 0..3 {
let mut attempts = 0;
let mut waits = Vec::new();
eject_macos_update_dmg_with_commands(
mount_dir,
|command, _| {
assert_eq!(command.get_program(), "diskutil");
assert_eq!(
command.get_args().collect::<Vec<_>>(),
[std::ffi::OsStr::new("eject"), mount_dir.as_os_str()]
);
attempts += 1;
if attempts <= busy_attempts {
Err("Resource busy".to_string())
} else {
Ok(())
}
},
|duration| waits.push(duration),
)
.unwrap();
assert_eq!(attempts, busy_attempts + 1);
assert_eq!(waits, vec![Duration::from_millis(500); busy_attempts]);
}
}
#[test]
fn macos_dmg_eject_falls_back_only_for_the_update_mount() {
let mount_dir = Path::new("/private/tmp/update with spaces/mount");
for force_succeeds in [true, false] {
let mut eject_attempts = 0;
let mut force_attempts = 0;
let result = eject_macos_update_dmg_with_commands(
mount_dir,
|command, _| {
if command.get_program() == "diskutil" {
eject_attempts += 1;
return Err("diskutil: Resource busy".to_string());
}
assert_eq!(command.get_program(), "hdiutil");
assert_eq!(
command.get_args().collect::<Vec<_>>(),
[
std::ffi::OsStr::new("detach"),
std::ffi::OsStr::new("-force"),
mount_dir.as_os_str(),
]
);
force_attempts += 1;
if force_succeeds {
Ok(())
} else {
Err("hdiutil: Resource busy".to_string())
}
},
|_| {},
);
assert_eq!(eject_attempts, 3);
assert_eq!(force_attempts, 1);
if force_succeeds {
result.unwrap();
} else {
let error = result.unwrap_err();
assert!(error.contains("diskutil: Resource busy"));
assert!(error.contains("hdiutil: Resource busy"));
}
}
}
#[test]
fn macos_dmg_staging_survives_cleanup_failure_without_bypassing_verification() {
// Exercise successful staging and each fatal preparation step, even when
// every eject attempt fails with the error reported in issue #301.
for failure in [None, Some("ditto"), Some("codesign"), Some("spctl")] {
let root = agent_test_home("macos-dmg-stage-busy");
let source_app = root.join("mount").join("EasyCLIProxyAPI.app");
let staged_app = root.join("staging/EasyCLIProxyAPI.app");
fs::create_dir_all(&source_app).unwrap();
fs::write(source_app.join("payload"), b"application").unwrap();
let mut preparation = Vec::new();
let mut cleanup_attempts = 0;
let result = stage_macos_application_from_dmg_with_commands(
&root.join("update.dmg"),
&root,
&staged_app,
|command, _| {
let program = command.get_program().to_str().unwrap();
let args = command.get_args().collect::<Vec<_>>();
if program == "diskutil" || args.first().is_some_and(|arg| *arg == "detach") {
cleanup_attempts += 1;
return Err("Resource busy".to_string());
}
preparation.push(program.to_string());
if failure == Some(program) {
return Err(format!("{program} failed"));
}
if program == "ditto" {
assert_eq!(args, [source_app.as_os_str(), staged_app.as_os_str()]);
fs::create_dir_all(&staged_app).unwrap();
fs::copy(source_app.join("payload"), staged_app.join("payload")).unwrap();
} else if program == "codesign" || program == "spctl" {
assert_eq!(args.last().copied(), Some(staged_app.as_os_str()));
}
Ok(())
},
|_| {},
);
assert_eq!(cleanup_attempts, 4);
if let Some(failure) = failure {
assert_eq!(result.unwrap_err(), format!("{failure} failed"));
assert_eq!(preparation.last().map(String::as_str), Some(failure));
} else {
result.unwrap();
assert_eq!(preparation, ["hdiutil", "ditto", "codesign", "spctl"]);
assert_eq!(fs::read(staged_app.join("payload")).unwrap(), b"application");
}
assert!(source_app.join("payload").is_file());
fs::remove_dir_all(root).unwrap();
}
}
#[test]
fn macos_dmg_missing_app_is_rejected_and_still_ejected() {
let root = agent_test_home("macos-dmg-missing-app");
let mut commands = Vec::new();
let error = stage_macos_application_from_dmg_with_commands(
&root.join("update.dmg"),
&root,
&root.join("staging/EasyCLIProxyAPI.app"),
|command, _| {
commands.push(command.get_program().to_string_lossy().into_owned());
Ok(())
},
|_| panic!("Successful ejection should not wait"),
)
.unwrap_err();
assert!(error.contains("missing EasyCLIProxyAPI.app"));
assert_eq!(commands, ["hdiutil", "diskutil"]);
assert!(!root.join("mount").exists());
fs::remove_dir_all(root).unwrap();
}
#[test]
fn macos_dmg_cleanup_preserves_busy_images_and_retries_later() {
let root = agent_test_home("macos-dmg-cleanup");
let mount_dir = root.join("mount");
let archive = root.join("update.dmg");
let mounted_file = mount_dir.join("mounted-file");
fs::create_dir_all(&mount_dir).unwrap();
fs::write(&archive, b"disk image").unwrap();
fs::write(&mounted_file, b"mounted contents").unwrap();
cleanup_macos_update_dmg_with_commands(
&root,
|_, _| Err("Resource busy".to_string()),
|_| {},
);
assert_eq!(fs::read(&archive).unwrap(), b"disk image");
assert_eq!(fs::read(&mounted_file).unwrap(), b"mounted contents");
cleanup_macos_update_dmg_with_commands(
&root,
|command, _| {
assert_eq!(command.get_program(), "diskutil");
// Simulate the mounted contents disappearing after a successful eject.
fs::remove_file(&mounted_file).unwrap();
Ok(())
},
|_| panic!("Successful ejection should not wait"),
);
assert!(!archive.exists());
assert!(!mount_dir.exists());
cleanup_macos_update_dmg_with_commands(
&root,
|_, _| panic!("An already cleaned image should not be ejected again"),
|_| panic!("An already cleaned image should not wait"),
);
fs::remove_dir_all(root).unwrap();
}
fn portable_update_test_asset(version: &str, arch: &str) -> PortableUpdateAsset {
let (_, display, suffix) = portable_update_asset_platform().unwrap();
let name = format!("EasyCLIProxyAPI-v{version}-{display}-{arch}.{suffix}");