From 358d0c6847a93260d21697a31ab2dfb390dde0b0 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 28 Oct 2026 13:22:38 +1100 Subject: [PATCH 01/10] feat(macos): prompt to move Buzz to /Applications when run from installer media MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users who open Buzz straight from the downloaded DMG (or from a Gatekeeper translocated copy, another read-only volume, or ~/Downloads) hit confusing failures later: automatic updates can't write to the bundle, the image stays mounted, and state lands on an ephemeral copy. Detect those locations and offer to move Buzz into /Applications with a native prompt. On macOS release builds, once the Tauri event loop reports `Ready`, `installer_media` resolves the running `.app` bundle, classifies its location (installed in /Applications or ~/Applications, disk image, translocated, read-only volume, other), and — unless a `suppress-move-to-applications` marker exists in the app data dir — shows an app-modal NSAlert whose wording reflects whether the move is a fresh install, upgrade, downgrade, reinstall, or replacement of an unreadable version (compared as semver from both bundles' Info.plist). Accepting stages a `ditto` copy under a hidden name in /Applications, trashes any installed copy via NSFileManager, renames the staged copy into place, clears quarantine, spawns a detached shell that relaunches the installed copy after this process exits (detaching the disk image when running from one), and exits. All shell-interpolated paths are single-quote escaped. Failures keep the current copy running and explain how to move it by hand. Declining continues normally; the suppression checkbox is honoured regardless of which button was clicked. Detection is fail-open and the flow never escalates privileges. The prompt runs from `RunEvent::Ready` rather than `setup` because the synchronous `runModal()` would otherwise block setup and race the webview. Pure classification, scenario, copy, and shell-quoting helpers build on every platform and are covered by unit tests; only the AppKit/statfs/ditto code is macOS-only. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- Cargo.lock | 2 + README.md | 2 +- src-tauri/Cargo.toml | 6 +- src-tauri/src/installer_media.rs | 994 +++++++++++++++++++++++++++++++ src-tauri/src/lib.rs | 14 + 5 files changed, 1015 insertions(+), 3 deletions(-) create mode 100644 src-tauri/src/installer_media.rs diff --git a/Cargo.lock b/Cargo.lock index cddd7380e1..4364c68e99 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -41,6 +41,7 @@ dependencies = [ "objc2-user-notifications 0.3.2", "objc2-web-kit 0.3.2", "percent-encoding", + "plist", "portable-pty", "qrcode", "rcgen", @@ -52,6 +53,7 @@ dependencies = [ "rustls-native-certs", "secp256k1 0.31.1", "security-framework", + "semver", "serde", "serde_json", "sha2", diff --git a/README.md b/README.md index ac26e4f18e..a61b669273 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ Get the newest build from the [releases page](https://github.com/block/buzz-app/ | Platform | Download | Notes | | --- | --- | --- | -| macOS (Apple Silicon) | `Buzz_…_aarch64.dmg` | Signed and notarized. Intel Macs aren't supported. | +| macOS (Apple Silicon) | `Buzz_…_aarch64.dmg` | Signed and notarized. Drag Buzz to Applications; if you open it from the disk image instead, Buzz offers to move itself there. Intel Macs aren't supported. | | Windows (x64) | `Buzz_…_x64_unsigned.exe` | Not code-signed, so Windows SmartScreen warns you before it runs. | | Linux (x64) | `Buzz_…_amd64.deb` or `Buzz_…_x86_64.AppImage` | Built on Ubuntu 24.04. | diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index c2765913e2..3585689bdd 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -65,6 +65,7 @@ getrandom = "0.3" zeroize = "1.9" nostr = { version = "=0.45.5", default-features = false, features = ["std", "nip44", "os-rng"] } dirs = "6" +semver = "1" rfd = { version = "0.16", default-features = false } tempfile = "3" fs2 = "0.4" @@ -90,10 +91,11 @@ libc = "0.2" # Pin the small upstream Origin rejection fix; released versions accept browser clients. tauri-plugin-mcp-bridge = { git = "https://github.com/hypothesi/mcp-server-tauri", rev = "1d5105e72835b28ca05a75bcd612b9127c12fe94", optional = true } security-framework = "3.7" +plist = "1" block2 = "=0.6.2" objc2 = "=0.6.4" -objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["NSScreen", "NSWindow", "NSEvent", "block2"] } -objc2-foundation = { version = "=0.3.2", default-features = false, features = ["std", "NSObject", "NSThread", "NSBundle", "NSString", "NSError", "NSUserDefaults", "NSProcessInfo"] } +objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["NSScreen", "NSWindow", "NSEvent", "NSAlert", "NSButton", "NSControl", "NSCell", "NSResponder", "NSView", "block2"] } +objc2-foundation = { version = "=0.3.2", default-features = false, features = ["std", "NSObject", "NSThread", "NSBundle", "NSString", "NSError", "NSUserDefaults", "NSProcessInfo", "NSFileManager", "NSURL"] } objc2-user-notifications = { version = "=0.3.2", default-features = false, features = ["UNUserNotificationCenter", "UNNotificationSettings", "UNNotification", "UNNotificationContent", "UNNotificationRequest", "UNNotificationResponse", "UNNotificationTrigger", "block2"] } objc2-web-kit = { version = "=0.3.2", default-features = false, features = ["std", "objc2-app-kit", "block2", "WKWebView", "WKUIDelegate", "WKFrameInfo", "WKSecurityOrigin"] } diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs new file mode 100644 index 0000000000..fd608fa556 --- /dev/null +++ b/src-tauri/src/installer_media.rs @@ -0,0 +1,994 @@ +//! macOS-only detection for when Buzz is launched from installer media (a +//! mounted disk image, a read-only or App Translocation location, or some +//! other non-installed download location) rather than from an Applications +//! folder. +//! +//! When that happens we surface a native first-launch prompt offering to move +//! Buzz into `/Applications` and relaunch from there. This mirrors the +//! widely-copied Potion Factory `LetsMove` flow: never prompt when the app is +//! already installed, treat read-only/disk-image/translocated locations as +//! "running from installer media", and on acceptance copy the bundle, clear +//! quarantine metadata, relaunch the installed copy, and detach the disk image +//! if we were running from one. +//! +//! Ported from berd (squareup/berd#561). + +use std::path::{Path, PathBuf}; + +/// Where the running `.app` bundle lives, classified into the buckets that +/// decide whether we prompt the user to move into `/Applications`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RunLocation { + /// Already inside an Applications directory — never prompt. + InstalledInApplications, + /// Running from a mounted disk image (read-only volume under `/Volumes`). + DiskImage, + /// Running from a Gatekeeper App Translocation location (a randomized + /// read-only path under `/private/var/folders/.../AppTranslocation/...`). + Translocated, + /// Running from some other read-only volume. + ReadOnlyVolume, + /// Running from a writable, non-installed location (e.g. `~/Downloads`). + OtherNonInstalled, +} + +impl RunLocation { + /// Whether this location should trigger the move-to-Applications prompt. + /// Anything other than an installed copy is treated as installer media. + pub fn should_prompt_move(self) -> bool { + !matches!(self, RunLocation::InstalledInApplications) + } +} + +/// Filesystem facts about the volume containing the running bundle, gathered +/// via `statfs`. Kept as a plain struct so the classification logic can be +/// unit-tested without a real mount. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VolumeInfo { + pub mount_point: PathBuf, + pub read_only: bool, + pub is_root: bool, +} + +/// Classify a bundle path + its volume into a [`RunLocation`]. +/// +/// Conservative by design: a bundle inside any Applications directory is +/// always treated as installed (no prompt), and only unambiguous installer +/// media signals (translocation, read-only volumes) escalate above +/// `OtherNonInstalled`. +pub fn classify(bundle_path: &Path, home: Option<&Path>, volume: &VolumeInfo) -> RunLocation { + if is_in_applications_folder(bundle_path, home) { + return RunLocation::InstalledInApplications; + } + if is_translocated_path(bundle_path) { + return RunLocation::Translocated; + } + if volume.read_only && !volume.is_root { + return if volume.mount_point.starts_with("/Volumes") { + RunLocation::DiskImage + } else { + RunLocation::ReadOnlyVolume + }; + } + RunLocation::OtherNonInstalled +} + +/// Whether `path` lives inside the system `/Applications` or the per-user +/// `~/Applications` directory. Uses component-aware prefix matching so e.g. +/// `/ApplicationsOther/Foo.app` is not mistaken for an install. +fn is_in_applications_folder(path: &Path, home: Option<&Path>) -> bool { + if path.starts_with("/Applications") { + return true; + } + if let Some(home) = home { + if path.starts_with(home.join("Applications")) { + return true; + } + } + false +} + +/// Whether `path` is a Gatekeeper App Translocation mount. Translocated +/// bundles live under `/private/var/folders/.../AppTranslocation//...`, +/// so the `AppTranslocation` path component is a reliable, self-contained +/// signal that a simple `/Volumes/...` check would miss. +fn is_translocated_path(path: &Path) -> bool { + path.components() + .any(|component| component.as_os_str() == "AppTranslocation") +} + +/// Resolve the running `.app` bundle directory by walking up from the current +/// executable to the nearest ancestor with a `.app` extension. +fn bundle_path_from_executable(executable: &Path) -> Option { + executable + .ancestors() + .find(|ancestor| ancestor.extension().is_some_and(|ext| ext == "app")) + .map(Path::to_path_buf) +} + +/// Version identity read from a bundle's `Contents/Info.plist`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BundleVersion { + /// `CFBundleShortVersionString`, e.g. `"0.7.0"`. + pub short: String, + /// `CFBundleVersion`, the build identifier. + pub build: String, +} + +/// How the running copy's version relates to a copy already in `/Applications`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VersionRelation { + /// The running copy is a newer version than the installed one. + RunningNewer, + /// The running copy is an older version than the installed one. + RunningOlder, + /// Both copies report the same version. + Same, + /// At least one version string could not be parsed. + Unknown, +} + +/// Compare two bundle versions by their `CFBundleShortVersionString`, parsed as +/// semver. If either side fails to parse we return [`VersionRelation::Unknown`] +/// rather than guessing — the caller degrades to neutral "replace" wording. +fn compare_versions(running: &BundleVersion, installed: &BundleVersion) -> VersionRelation { + use semver::Version; + + let (Ok(running), Ok(installed)) = ( + Version::parse(&running.short), + Version::parse(&installed.short), + ) else { + return VersionRelation::Unknown; + }; + + match running.cmp(&installed) { + std::cmp::Ordering::Greater => VersionRelation::RunningNewer, + std::cmp::Ordering::Less => VersionRelation::RunningOlder, + std::cmp::Ordering::Equal => VersionRelation::Same, + } +} + +/// What moving into `/Applications` actually means given what's already there. +/// Folding "does an install exist" and the version relation into one value +/// keeps the dialog copy and the action that follows it from ever disagreeing. +#[derive(Debug, Clone, PartialEq, Eq)] +enum MoveScenario { + /// Nothing in `/Applications` — a plain first install. + FreshInstall, + /// An older copy is installed; this copy is newer. + Upgrade { from: String, to: String }, + /// A newer copy is installed; this copy is older. + Downgrade { from: String, to: String }, + /// The same version is already installed. + Reinstall { version: String }, + /// A copy is installed but its version couldn't be determined. + ExistingUnknown, +} + +/// Derive the [`MoveScenario`] from whether `/Applications/Buzz.app` exists and +/// the two versions. Pure so it can be unit-tested over the full truth table. +fn scenario_for( + target_exists: bool, + running: Option<&BundleVersion>, + installed: Option<&BundleVersion>, +) -> MoveScenario { + if !target_exists { + return MoveScenario::FreshInstall; + } + let (Some(running), Some(installed)) = (running, installed) else { + return MoveScenario::ExistingUnknown; + }; + match compare_versions(running, installed) { + VersionRelation::RunningNewer => MoveScenario::Upgrade { + from: installed.short.clone(), + to: running.short.clone(), + }, + VersionRelation::RunningOlder => MoveScenario::Downgrade { + from: installed.short.clone(), + to: running.short.clone(), + }, + VersionRelation::Same => MoveScenario::Reinstall { + version: running.short.clone(), + }, + VersionRelation::Unknown => MoveScenario::ExistingUnknown, + } +} + +/// The three text fields a native `NSAlert` needs. +struct AlertCopy { + message: String, + informative: String, + primary_button: String, +} + +/// A trailing note describing how the app is currently being run, appended to +/// every scenario's informative text so the user understands the prompt's +/// trigger regardless of the version situation. +fn location_note(location: RunLocation) -> &'static str { + match location { + RunLocation::DiskImage => "\n\nYou're currently running Buzz from a disk image.", + RunLocation::Translocated => { + "\n\nmacOS is running this copy from a temporary, read-only location." + } + RunLocation::ReadOnlyVolume => "\n\nYou're currently running Buzz from a read-only volume.", + RunLocation::OtherNonInstalled => { + "\n\nYou're currently running Buzz from outside your Applications folder." + } + RunLocation::InstalledInApplications => "", + } +} + +/// Produce the alert's message, informative text, and primary-button label for +/// a given scenario and run location. Pure and unit-tested so the wording can +/// never drift from the action it describes. +fn alert_copy(scenario: &MoveScenario, location: RunLocation) -> AlertCopy { + let note = location_note(location); + match scenario { + MoveScenario::FreshInstall => AlertCopy { + message: "Move to Applications folder?".to_string(), + informative: format!( + "Buzz can move itself to your Applications folder and relaunch from there. \ + This keeps automatic updates working and avoids problems with running from \ + installer media.{note}" + ), + primary_button: "Move to Applications Folder".to_string(), + }, + MoveScenario::Upgrade { from, to } => AlertCopy { + message: "Upgrade Buzz in your Applications folder?".to_string(), + informative: format!( + "You have Buzz {from} installed. This copy is the newer version {to}. \ + Buzz can replace the installed copy and relaunch from it.{note}" + ), + primary_button: "Upgrade and Relaunch".to_string(), + }, + MoveScenario::Downgrade { from, to } => AlertCopy { + message: "Replace the newer version in your Applications folder?".to_string(), + informative: format!( + "You have Buzz {from} installed, which is newer than this copy ({to}). \ + Buzz can replace the installed copy and relaunch from it.{note}" + ), + primary_button: "Replace and Relaunch".to_string(), + }, + MoveScenario::Reinstall { version } => AlertCopy { + message: format!("Reinstall Buzz {version} in your Applications folder?"), + informative: format!( + "Buzz {version} is already installed in your Applications folder. Buzz can \ + replace the installed copy and relaunch from it.{note}" + ), + primary_button: "Reinstall and Relaunch".to_string(), + }, + MoveScenario::ExistingUnknown => AlertCopy { + message: "Replace the copy in your Applications folder?".to_string(), + informative: format!( + "Buzz is already installed in your Applications folder. Buzz can replace the \ + installed copy and relaunch from it.{note}" + ), + primary_button: "Replace and Relaunch".to_string(), + }, + } +} + +/// Message and informative text for the alert shown when a move the user +/// confirmed fails partway through. Without privilege escalation a standard +/// user on a locked-down `/Applications` will reliably land here, so the +/// failure must be visible rather than swallowed into a log line. Pure so the +/// wording stays testable alongside the rest of the dialog copy. +fn move_failed_alert_copy(error: &str) -> (String, String) { + ( + "Couldn't move Buzz to your Applications folder".to_string(), + format!( + "Buzz will keep running from its current location. You can move it yourself by \ + dragging Buzz into your Applications folder, then opening it from there.\n\n\ + Details: {error}" + ), + ) +} + +/// Quote `value` for safe interpolation into a `/bin/sh -c` command string. +/// Wraps it in single quotes and rewrites any embedded single quote as the +/// classic `'\''` sequence (close quote, escaped literal quote, reopen quote), +/// so a path containing shell metacharacters — or a bundle deliberately renamed +/// to contain a quote — can't break out of the quoting and run arbitrary +/// commands. Pure so the escaping is unit-testable. +fn shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', "'\\''")) +} + +#[cfg(target_os = "macos")] +mod macos { + use super::*; + use std::os::unix::ffi::OsStrExt; + use std::process::Command; + use tauri::{AppHandle, Manager}; + + const SUPPRESSION_MARKER: &str = "suppress-move-to-applications"; + + /// Detect whether Buzz is running from installer media and, if so, offer + /// to move it into `/Applications`. Runs once per launch from + /// `RunEvent::Ready`, on the main thread. On acceptance this relaunches + /// from the installed copy and exits the current process. + pub(crate) fn maybe_prompt_move_to_applications(app: &AppHandle) { + // Never interrupt development builds, which legitimately run from + // `target/` and similar non-installed locations. + if cfg!(debug_assertions) { + return; + } + + let Some(bundle_path) = running_app_bundle_path() else { + return; + }; + let Some(app_name) = bundle_path.file_name() else { + return; + }; + let Some(volume) = volume_info(&bundle_path) else { + return; + }; + let home = dirs::home_dir(); + let location = classify(&bundle_path, home.as_deref(), &volume); + if !location.should_prompt_move() { + return; + } + + // Respect a prior "Do not show this message again" choice. + let marker = suppression_marker_path(app); + if marker.as_ref().is_some_and(|path| path.exists()) { + return; + } + + // Read both versions from disk (rather than trusting the compiled-in + // version) so the running and installed copies are always compared + // apples-to-apples. + let target = Path::new("/Applications").join(app_name); + let running = read_bundle_version(&bundle_path); + let installed = target + .exists() + .then(|| read_bundle_version(&target)) + .flatten(); + let scenario = scenario_for(target.exists(), running.as_ref(), installed.as_ref()); + + eprintln!( + "Buzz appears to be running from installer media ({location:?}, {scenario:?}); \ + prompting to move to /Applications" + ); + + let Some(decision) = show_move_alert(&scenario, location) else { + return; + }; + + if decision.suppress { + if let Some(marker) = marker { + if let Some(parent) = marker.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(&marker, b""); + } + } + + if !decision.move_requested { + return; + } + + match move_to_applications_and_relaunch(&bundle_path, &volume, location) { + Ok(()) => { + eprintln!("Relaunching Buzz from /Applications"); + std::process::exit(0); + } + Err(error) => { + eprintln!("Failed to move Buzz to /Applications: {error}"); + show_move_failed_alert(&error.to_string()); + } + } + } + + fn suppression_marker_path(app: &AppHandle) -> Option { + app.path() + .app_data_dir() + .ok() + .map(|dir| dir.join(SUPPRESSION_MARKER)) + } + + fn running_app_bundle_path() -> Option { + let executable = std::env::current_exe().ok()?; + bundle_path_from_executable(&executable) + } + + /// Read `CFBundleShortVersionString` and `CFBundleVersion` from a bundle's + /// `Contents/Info.plist`. Returns `None` if the plist is missing/unreadable + /// or lacks a short version string; the build identifier is best-effort and + /// falls back to the short version when absent. + fn read_bundle_version(bundle_path: &Path) -> Option { + let info_plist = bundle_path.join("Contents/Info.plist"); + let value = plist::Value::from_file(&info_plist).ok()?; + let dict = value.as_dictionary()?; + let short = dict.get("CFBundleShortVersionString")?.as_string()?; + let build = dict + .get("CFBundleVersion") + .and_then(plist::Value::as_string) + .unwrap_or(short); + Some(BundleVersion { + short: short.to_string(), + build: build.to_string(), + }) + } + + /// Gather volume facts for `path` via `statfs`. + fn volume_info(path: &Path) -> Option { + let c_path = std::ffi::CString::new(path.as_os_str().as_bytes()).ok()?; + // SAFETY: `stat` is fully initialized by `statfs` on success; we only + // read it when the call returns 0. + let mut stat: libc::statfs = unsafe { std::mem::zeroed() }; + if unsafe { libc::statfs(c_path.as_ptr(), &mut stat) } != 0 { + return None; + } + let mount_point = c_string_field_to_path(&stat.f_mntonname); + let read_only = (stat.f_flags & libc::MNT_RDONLY as u32) != 0; + let is_root = mount_point == Path::new("/"); + Some(VolumeInfo { + mount_point, + read_only, + is_root, + }) + } + + fn c_string_field_to_path(field: &[libc::c_char]) -> PathBuf { + let bytes: Vec = field + .iter() + .take_while(|&&byte| byte != 0) + .map(|&byte| byte as u8) + .collect(); + PathBuf::from(std::ffi::OsStr::from_bytes(&bytes)) + } + + struct MoveDecision { + move_requested: bool, + suppress: bool, + } + + fn show_move_alert(scenario: &MoveScenario, location: RunLocation) -> Option { + use objc2_app_kit::{NSAlert, NSAlertFirstButtonReturn, NSControlStateValueOn}; + use objc2_foundation::{MainThreadMarker, NSString}; + + let copy = alert_copy(scenario, location); + + // AppKit calls here are only valid on the main thread, proven by the + // `mtm` marker we just obtained. + let mtm = MainThreadMarker::new()?; + let alert = NSAlert::new(mtm); + alert.setMessageText(&NSString::from_str(©.message)); + alert.setInformativeText(&NSString::from_str(©.informative)); + let _ = alert.addButtonWithTitle(&NSString::from_str(©.primary_button)); + let _ = alert.addButtonWithTitle(&NSString::from_str("Do Not Move")); + alert.setShowsSuppressionButton(true); + + let move_requested = alert.runModal() == NSAlertFirstButtonReturn; + let suppress = alert + .suppressionButton() + .is_some_and(|button| button.state() == NSControlStateValueOn); + + Some(MoveDecision { + move_requested, + suppress, + }) + } + + /// Tell the user a confirmed move failed, so the app silently continuing + /// from installer media isn't the only signal. Best-effort: if we can't get + /// onto the main thread the warning log is the fallback. + fn show_move_failed_alert(error: &str) { + use objc2_app_kit::NSAlert; + use objc2_foundation::{MainThreadMarker, NSString}; + + let (message, informative) = move_failed_alert_copy(error); + + // AppKit calls here are only valid on the main thread, proven by the + // `mtm` marker. + let Some(mtm) = MainThreadMarker::new() else { + return; + }; + let alert = NSAlert::new(mtm); + alert.setMessageText(&NSString::from_str(&message)); + alert.setInformativeText(&NSString::from_str(&informative)); + let _ = alert.addButtonWithTitle(&NSString::from_str("OK")); + let _ = alert.runModal(); + } + + fn move_to_applications_and_relaunch( + bundle_path: &Path, + volume: &VolumeInfo, + location: RunLocation, + ) -> std::io::Result<()> { + let app_name = bundle_path + .file_name() + .ok_or_else(|| std::io::Error::other("app bundle path has no file name"))?; + let target = Path::new("/Applications").join(app_name); + + // Stage the copy under a hidden temporary name on the same volume as + // the target, so a failed or interrupted `ditto` (disk full, killed + // mid-copy) can never leave a working installed copy trashed and the + // target a half-written bundle. Only once the copy fully succeeds do + // we trash the old copy and rename the staged one into place — an + // atomic, same-volume rename. The move the dialog promised therefore + // either happens completely or not at all. + let staging = Path::new("/Applications").join(format!( + ".{}.{}.staging", + app_name.to_string_lossy(), + std::process::id() + )); + // Clear any leftover staging bundle from a previously interrupted run + // so `ditto` writes into a clean location rather than merging into it. + if staging.exists() { + let _ = std::fs::remove_dir_all(&staging); + } + + // `ditto` is the macOS-blessed way to copy an app bundle while + // preserving its structure and metadata. + let status = Command::new("/usr/bin/ditto") + .arg(bundle_path) + .arg(&staging) + .status()?; + if !status.success() { + let _ = std::fs::remove_dir_all(&staging); + return Err(std::io::Error::other(format!("ditto exited with {status}"))); + } + + // The copy is complete and on the target's volume, so the remaining + // steps are fast and the rename is atomic. An installed copy, if one + // exists, goes to the Trash (recoverable and Finder-undoable) rather + // than being overwritten in place. If trashing fails (e.g. a + // read-only /Applications) we bail before the rename and discard the + // staged copy, leaving the existing install untouched. + if target.exists() { + if let Err(error) = move_to_trash(&target) { + let _ = std::fs::remove_dir_all(&staging); + return Err(error); + } + } + if let Err(error) = std::fs::rename(&staging, &target) { + let _ = std::fs::remove_dir_all(&staging); + return Err(error); + } + + // Clear quarantine so Gatekeeper doesn't re-prompt or translocate the + // freshly installed copy. + let _ = Command::new("/usr/bin/xattr") + .args(["-r", "-d", "com.apple.quarantine"]) + .arg(&target) + .status(); + + let detach_mount = + matches!(location, RunLocation::DiskImage).then(|| volume.mount_point.clone()); + relaunch_after_exit(&target, detach_mount) + } + + /// Move `path` to the user's Trash via `NSFileManager`. Preferred over + /// `rm -rf` because it's recoverable and Finder-undoable if the user changes + /// their mind about replacing the installed copy. + fn move_to_trash(path: &Path) -> std::io::Result<()> { + use objc2_foundation::{NSFileManager, NSString, NSURL}; + + let url = NSURL::fileURLWithPath(&NSString::from_str(&path.to_string_lossy())); + NSFileManager::defaultManager() + .trashItemAtURL_resultingItemURL_error(&url, None) + .map_err(|error| { + std::io::Error::other(format!( + "failed to move {} to Trash: {error:?}", + path.display() + )) + }) + } + + /// Spawn a detached shell that waits for this process to exit, launches the + /// installed copy, and (optionally) detaches the disk image we ran from. + fn relaunch_after_exit(target: &Path, detach_mount: Option) -> std::io::Result<()> { + let pid = std::process::id(); + // The paths are single-quote escaped before interpolation: the only + // reason a shell is involved at all is the wait-for-pid loop, and a + // bundle whose name contains shell metacharacters must not be able to + // turn that loop into arbitrary command execution. + let target = shell_single_quote(&target.to_string_lossy()); + let mut script = format!( + "while /bin/kill -0 {pid} >/dev/null 2>&1; do /bin/sleep 0.2; done; \ + /usr/bin/open {target}" + ); + if let Some(mount) = detach_mount { + let mount = shell_single_quote(&mount.to_string_lossy()); + script.push_str(&format!( + "; /bin/sleep 1; /usr/bin/hdiutil detach {mount} >/dev/null 2>&1" + )); + } + Command::new("/bin/sh").arg("-c").arg(script).spawn()?; + Ok(()) + } +} + +#[cfg(target_os = "macos")] +pub(crate) use macos::maybe_prompt_move_to_applications; + +#[cfg(test)] +mod tests { + use super::*; + + fn volume(mount_point: &str, read_only: bool) -> VolumeInfo { + VolumeInfo { + mount_point: PathBuf::from(mount_point), + read_only, + is_root: mount_point == "/", + } + } + + #[test] + fn bundle_path_resolves_to_dot_app_ancestor() { + let exe = Path::new("/Applications/Buzz.app/Contents/MacOS/Buzz"); + assert_eq!( + bundle_path_from_executable(exe), + Some(PathBuf::from("/Applications/Buzz.app")) + ); + } + + #[test] + fn bundle_path_is_none_without_dot_app() { + let exe = Path::new("/Users/someone/code/buzz-app/target/release/Buzz"); + assert_eq!(bundle_path_from_executable(exe), None); + } + + #[test] + fn applications_folder_detection_is_component_aware() { + let home = Path::new("/Users/someone"); + assert!(is_in_applications_folder( + Path::new("/Applications/Buzz.app"), + Some(home) + )); + assert!(is_in_applications_folder( + Path::new("/Users/someone/Applications/Buzz.app"), + Some(home) + )); + // A directory that merely starts with the same characters is not a match. + assert!(!is_in_applications_folder( + Path::new("/ApplicationsOther/Buzz.app"), + Some(home) + )); + assert!(!is_in_applications_folder( + Path::new("/Volumes/Buzz/Buzz.app"), + Some(home) + )); + } + + #[test] + fn translocation_detected_by_path_component() { + let translocated = + Path::new("/private/var/folders/ab/cd/T/AppTranslocation/UUID/d/Buzz.app"); + assert!(is_translocated_path(translocated)); + assert!(!is_translocated_path(Path::new("/Volumes/Buzz/Buzz.app"))); + } + + #[test] + fn classify_installed_app_does_not_prompt() { + let location = classify( + Path::new("/Applications/Buzz.app"), + Some(Path::new("/Users/someone")), + &volume("/", false), + ); + assert_eq!(location, RunLocation::InstalledInApplications); + assert!(!location.should_prompt_move()); + } + + #[test] + fn classify_user_applications_counts_as_installed() { + let location = classify( + Path::new("/Users/someone/Applications/Buzz.app"), + Some(Path::new("/Users/someone")), + &volume("/", false), + ); + assert_eq!(location, RunLocation::InstalledInApplications); + // Without a known home, ~/Applications is just another writable folder. + let location = classify( + Path::new("/Users/someone/Applications/Buzz.app"), + None, + &volume("/", false), + ); + assert_eq!(location, RunLocation::OtherNonInstalled); + } + + #[test] + fn classify_disk_image() { + let location = classify( + Path::new("/Volumes/Buzz/Buzz.app"), + None, + &volume("/Volumes/Buzz", true), + ); + assert_eq!(location, RunLocation::DiskImage); + assert!(location.should_prompt_move()); + } + + #[test] + fn classify_translocation_takes_priority_over_volume() { + let location = classify( + Path::new("/private/var/folders/ab/cd/T/AppTranslocation/UUID/d/Buzz.app"), + None, + &volume("/private/var/folders/ab/cd/T/AppTranslocation/UUID/d", true), + ); + assert_eq!(location, RunLocation::Translocated); + } + + #[test] + fn classify_read_only_non_volume() { + let location = classify( + Path::new("/mnt/ro/Buzz.app"), + None, + &volume("/mnt/ro", true), + ); + assert_eq!(location, RunLocation::ReadOnlyVolume); + } + + #[test] + fn classify_read_only_root_is_not_installer_media() { + // A sealed system volume mounted at `/` is read-only, but a bundle on + // it is just a non-installed copy, not a disk image. + let location = classify( + Path::new("/ApplicationsOther/Buzz.app"), + Some(Path::new("/Users/someone")), + &volume("/", true), + ); + assert_eq!(location, RunLocation::OtherNonInstalled); + } + + #[test] + fn classify_writable_download_is_other_non_installed() { + let location = classify( + Path::new("/Users/someone/Downloads/Buzz.app"), + Some(Path::new("/Users/someone")), + &volume("/", false), + ); + assert_eq!(location, RunLocation::OtherNonInstalled); + assert!(location.should_prompt_move()); + } + + fn version(short: &str, build: &str) -> BundleVersion { + BundleVersion { + short: short.to_string(), + build: build.to_string(), + } + } + + #[test] + fn compare_versions_orders_by_short_semver() { + assert_eq!( + compare_versions(&version("0.8.0", "8"), &version("0.7.12", "712")), + VersionRelation::RunningNewer + ); + assert_eq!( + compare_versions(&version("0.7.12", "712"), &version("0.8.0", "8")), + VersionRelation::RunningOlder + ); + assert_eq!( + compare_versions(&version("1.2.3", "10"), &version("1.2.3", "20")), + VersionRelation::Same + ); + } + + #[test] + fn compare_versions_unparseable_is_unknown() { + assert_eq!( + compare_versions(&version("not-a-version", "1"), &version("0.7.12", "712")), + VersionRelation::Unknown + ); + assert_eq!( + compare_versions(&version("0.8.0", "8"), &version("", "0")), + VersionRelation::Unknown + ); + } + + #[test] + fn scenario_for_no_install_is_fresh() { + assert_eq!( + scenario_for(false, Some(&version("0.8.0", "8")), None), + MoveScenario::FreshInstall + ); + } + + #[test] + fn scenario_for_existing_install_maps_relation() { + assert_eq!( + scenario_for( + true, + Some(&version("0.8.0", "8")), + Some(&version("0.7.12", "712")) + ), + MoveScenario::Upgrade { + from: "0.7.12".to_string(), + to: "0.8.0".to_string(), + } + ); + assert_eq!( + scenario_for( + true, + Some(&version("0.7.12", "712")), + Some(&version("0.8.0", "8")) + ), + MoveScenario::Downgrade { + from: "0.8.0".to_string(), + to: "0.7.12".to_string(), + } + ); + assert_eq!( + scenario_for( + true, + Some(&version("0.8.0", "8")), + Some(&version("0.8.0", "8")) + ), + MoveScenario::Reinstall { + version: "0.8.0".to_string(), + } + ); + } + + #[test] + fn scenario_for_unreadable_existing_is_existing_unknown() { + // Target exists but we couldn't read one (or both) versions. + assert_eq!( + scenario_for(true, Some(&version("0.8.0", "8")), None), + MoveScenario::ExistingUnknown + ); + assert_eq!( + scenario_for(true, None, Some(&version("0.7.12", "712"))), + MoveScenario::ExistingUnknown + ); + // Both readable but at least one short string isn't semver. + assert_eq!( + scenario_for( + true, + Some(&version("nightly", "8")), + Some(&version("0.7.12", "712")) + ), + MoveScenario::ExistingUnknown + ); + } + + #[test] + fn alert_copy_reflects_scenario() { + let fresh = alert_copy(&MoveScenario::FreshInstall, RunLocation::DiskImage); + assert_eq!(fresh.message, "Move to Applications folder?"); + assert_eq!(fresh.primary_button, "Move to Applications Folder"); + assert!(fresh.informative.starts_with("Buzz can move itself")); + + let upgrade = alert_copy( + &MoveScenario::Upgrade { + from: "0.7.12".to_string(), + to: "0.8.0".to_string(), + }, + RunLocation::DiskImage, + ); + assert_eq!(upgrade.message, "Upgrade Buzz in your Applications folder?"); + assert_eq!(upgrade.primary_button, "Upgrade and Relaunch"); + assert!(upgrade + .informative + .contains("You have Buzz 0.7.12 installed")); + assert!(upgrade.informative.contains("newer version 0.8.0")); + + let downgrade = alert_copy( + &MoveScenario::Downgrade { + from: "0.8.0".to_string(), + to: "0.7.12".to_string(), + }, + RunLocation::DiskImage, + ); + assert_eq!( + downgrade.message, + "Replace the newer version in your Applications folder?" + ); + assert_eq!(downgrade.primary_button, "Replace and Relaunch"); + assert!(downgrade + .informative + .contains("0.8.0 installed, which is newer than this copy (0.7.12)")); + + let reinstall = alert_copy( + &MoveScenario::Reinstall { + version: "0.8.0".to_string(), + }, + RunLocation::DiskImage, + ); + assert_eq!( + reinstall.message, + "Reinstall Buzz 0.8.0 in your Applications folder?" + ); + assert_eq!(reinstall.primary_button, "Reinstall and Relaunch"); + assert!(reinstall + .informative + .starts_with("Buzz 0.8.0 is already installed")); + + let unknown = alert_copy(&MoveScenario::ExistingUnknown, RunLocation::DiskImage); + assert_eq!( + unknown.message, + "Replace the copy in your Applications folder?" + ); + assert_eq!(unknown.primary_button, "Replace and Relaunch"); + assert!(unknown + .informative + .starts_with("Buzz is already installed in your Applications folder.")); + } + + #[test] + fn alert_copy_appends_location_note() { + // The per-location note is appended regardless of scenario. + for scenario in [ + MoveScenario::FreshInstall, + MoveScenario::Upgrade { + from: "0.7.12".to_string(), + to: "0.8.0".to_string(), + }, + MoveScenario::Downgrade { + from: "0.8.0".to_string(), + to: "0.7.12".to_string(), + }, + MoveScenario::Reinstall { + version: "0.8.0".to_string(), + }, + MoveScenario::ExistingUnknown, + ] { + assert!(alert_copy(&scenario, RunLocation::DiskImage) + .informative + .ends_with("\n\nYou're currently running Buzz from a disk image.")); + assert!(alert_copy(&scenario, RunLocation::Translocated) + .informative + .ends_with("\n\nmacOS is running this copy from a temporary, read-only location.")); + assert!(alert_copy(&scenario, RunLocation::ReadOnlyVolume) + .informative + .ends_with("\n\nYou're currently running Buzz from a read-only volume.")); + assert!(alert_copy(&scenario, RunLocation::OtherNonInstalled) + .informative + .ends_with( + "\n\nYou're currently running Buzz from outside your Applications folder." + )); + assert!(!alert_copy(&scenario, RunLocation::InstalledInApplications) + .informative + .ends_with('\n')); + } + } + + #[test] + fn move_failed_alert_copy_surfaces_error_and_manual_path() { + let (message, informative) = move_failed_alert_copy("ditto exited with exit status: 1"); + assert_eq!(message, "Couldn't move Buzz to your Applications folder"); + // The user is told the app keeps working and how to move it by hand. + assert!(informative.starts_with("Buzz will keep running from its current location.")); + assert!(informative.contains("dragging Buzz into your Applications folder")); + // The underlying error is included for support/debugging. + assert!(informative.ends_with("\n\nDetails: ditto exited with exit status: 1")); + } + + #[test] + fn shell_single_quote_wraps_plain_paths() { + assert_eq!( + shell_single_quote("/Applications/Buzz.app"), + "'/Applications/Buzz.app'" + ); + // Spaces and shell metacharacters are inert inside single quotes. + assert_eq!( + shell_single_quote("/Volumes/My Buzz & Co/Buzz.app"), + "'/Volumes/My Buzz & Co/Buzz.app'" + ); + } + + #[cfg(unix)] + #[test] + fn shell_single_quote_neutralizes_embedded_quotes() { + // A bundle renamed to break out of double quotes stays a single inert + // argument under single-quote escaping. + let malicious = r#"/Volumes/x/Buzz";rm -rf ~;".app"#; + let quoted = shell_single_quote(malicious); + assert_eq!(quoted, r#"'/Volumes/x/Buzz";rm -rf ~;".app'"#); + // A literal single quote becomes the classic close/escape/reopen + // sequence rather than ending the quoted string early. + assert_eq!(shell_single_quote("a'b"), r#"'a'\''b'"#); + + // The escaping round-trips through a real shell: the quoted string, + // echoed back, must equal the original path verbatim with no splitting + // or command execution. + let output = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(format!("printf %s {}", shell_single_quote(malicious))) + .output() + .expect("spawn /bin/sh"); + assert_eq!(String::from_utf8_lossy(&output.stdout), malicious); + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 5c21f32524..5706d74ddf 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -50,6 +50,10 @@ use nsec_backup::{ }; use nsec_backup_choice::{nsec_backup_choice, nsec_backup_set_choice}; mod image_clipboard; +// Pure classification and copy build everywhere so their tests run on every CI +// platform; only macOS has a caller. +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +mod installer_media; mod jev; mod keep_awake; mod legacy_communities; @@ -917,6 +921,16 @@ pub fn run() { .build(context) .expect("failed to build Buzz Foundation") .run(|app, event| { + // Offer to move into /Applications when launched from installer media + // (DMG, a read-only or translocated location, or another non-installed + // download). Deferred out of `setup` to here so the synchronous + // `NSAlert.runModal()` runs on the main thread only once setup has + // returned and the event loop is running. Accepting copies the bundle, + // relaunches the installed copy, and exits this process. + #[cfg(target_os = "macos")] + if matches!(event, tauri::RunEvent::Ready) { + installer_media::maybe_prompt_move_to_applications(app); + } #[cfg(target_os = "macos")] if matches!(event, tauri::RunEvent::Reopen { .. }) { deep_links::focus_main(app); From fe6757b146e011e7adb998cf2ac64fd9e156d4a8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 28 Oct 2026 16:17:30 +1100 Subject: [PATCH 02/10] fix(macos): defer the move-to-/Applications prompt off tao's Ready callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The installer-media prompt from 8e7e7eb5c never appeared in a release build launched from a DMG: the process logged "running from installer media … prompting to move to /Applications" and then hung at 0 % CPU with zero-size windows. `NSAlert.runModal()` was called inline from the `RunEvent::Ready` arm of the Tauri `run` callback. tao delivers that callback from `did_finish_launching` while holding its non-reentrant handler `Mutex`. The modal's first `CA::Transaction::flush` displays the already-visible main window, whose `drawRect` calls back into tao (`view::draw_rect` → `handle_redraw` → `Handler::handle_nonuser_event`) and blocks on the same lock on the same thread — a deadlock, reproducible 2/2 (tao 0.37.1, tauri 2.12.4). `app.run_on_main_thread` would not help because it is delivered through the same locked handler. The hang needs a visible window: an app whose main window is still hidden at `Ready` has nothing to draw during the modal, so the re-entrant redraw never happens. Buzz shows its main window before `Ready`. Fix: schedule `maybe_prompt_move_to_applications` with `dispatch2::DispatchQueue::main().exec_async` so it runs on the next main-run-loop turn, after the callback has returned and the lock is released. `dispatch2` was already in Cargo.lock transitively; it is now a direct macOS-only dependency pinned like the other objc2 crates. The comments that claimed running from `Ready` (rather than `setup`) avoided the blocking problem now state the actual constraint. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- Cargo.lock | 1 + src-tauri/Cargo.toml | 1 + src-tauri/src/installer_media.rs | 9 ++++++--- src-tauri/src/lib.rs | 21 ++++++++++++++++----- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4364c68e99..a7b2aa7159 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19,6 +19,7 @@ dependencies = [ "bytes", "chrono", "dirs 6.0.0", + "dispatch2", "dunce", "fs2", "futures-lite", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 3585689bdd..d5881d0142 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -93,6 +93,7 @@ tauri-plugin-mcp-bridge = { git = "https://github.com/hypothesi/mcp-server-tauri security-framework = "3.7" plist = "1" block2 = "=0.6.2" +dispatch2 = "=0.3.1" objc2 = "=0.6.4" objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["NSScreen", "NSWindow", "NSEvent", "NSAlert", "NSButton", "NSControl", "NSCell", "NSResponder", "NSView", "block2"] } objc2-foundation = { version = "=0.3.2", default-features = false, features = ["std", "NSObject", "NSThread", "NSBundle", "NSString", "NSError", "NSUserDefaults", "NSProcessInfo", "NSFileManager", "NSURL"] } diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index fd608fa556..6f05b6fb72 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -304,9 +304,12 @@ mod macos { const SUPPRESSION_MARKER: &str = "suppress-move-to-applications"; /// Detect whether Buzz is running from installer media and, if so, offer - /// to move it into `/Applications`. Runs once per launch from - /// `RunEvent::Ready`, on the main thread. On acceptance this relaunches - /// from the installed copy and exits the current process. + /// to move it into `/Applications`. Runs once per launch, on the main + /// thread, from a main-queue block scheduled at `RunEvent::Ready`: the + /// modal alert redraws the visible main window, which re-enters tao, so it + /// must not be called from inside a tao event-loop callback (see the call + /// site in `lib.rs`). On acceptance this relaunches from the installed + /// copy and exits the current process. pub(crate) fn maybe_prompt_move_to_applications(app: &AppHandle) { // Never interrupt development builds, which legitimately run from // `target/` and similar non-installed locations. diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 5706d74ddf..afba54a261 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -923,13 +923,24 @@ pub fn run() { .run(|app, event| { // Offer to move into /Applications when launched from installer media // (DMG, a read-only or translocated location, or another non-installed - // download). Deferred out of `setup` to here so the synchronous - // `NSAlert.runModal()` runs on the main thread only once setup has - // returned and the event loop is running. Accepting copies the bundle, - // relaunches the installed copy, and exits this process. + // download). Accepting copies the bundle, relaunches the installed + // copy, and exits this process. + // + // The synchronous `NSAlert.runModal()` must not run inside any tao + // event-loop callback, `Ready` included: tao holds a non-reentrant + // `Mutex` around its handler while delivering the callback, and the + // modal's first `CA::Transaction::flush` redraws the already-visible + // main window, whose `drawRect` re-enters tao and blocks on that + // same lock forever. Hopping onto the GCD main queue runs the prompt + // on the next main-run-loop turn, after the callback has returned + // and the lock is released. (`run_on_main_thread` would not help: + // it is delivered through the same locked handler.) #[cfg(target_os = "macos")] if matches!(event, tauri::RunEvent::Ready) { - installer_media::maybe_prompt_move_to_applications(app); + let app = app.clone(); + dispatch2::DispatchQueue::main().exec_async(move || { + installer_media::maybe_prompt_move_to_applications(&app); + }); } #[cfg(target_os = "macos")] if matches!(event, tauri::RunEvent::Reopen { .. }) { From 177b76ceeefd1876e09671b8648d9646c2f68763 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 28 Oct 2026 17:17:23 +1100 Subject: [PATCH 03/10] fix(macos): replace an installed old Buzz and a previous Buzz build the same way MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An installed old Buzz from block/buzz (`Buzz.app`, `xyz.block.buzz.app`, executable `buzz-desktop`, `CFBundleShortVersionString` ≤ 0.5.28, `CFBundleVersion` `0.5.28-block`) and an installed previous build of this app (`Buzz.app`, `xyz.block.buzz.desktop`, executable `Buzz`) now go through exactly the same Upgrade/Reinstall/Downgrade flow in the move-to-/Applications prompt, and the flow cannot tell which of the two it is replacing. Why that holds: - Same file name, path-keyed lookup. The installed copy is `/Applications/`, now a single `installed_copy_path` helper shared by the prompt and the move, with a doc comment stating the identity indifference is deliberate. The audit confirmed nothing in the module reads `CFBundleIdentifier`, `CFBundleExecutable`, `CFBundleName`, the signing identity or the data dir to locate, compare, trash or replace the installed copy; the suppression marker lives in the running app's own data dir and is not an installed-copy lookup. - Semver on the short version only. `compare_versions` parses `CFBundleShortVersionString`, which both apps ship as clean semver; `CFBundleVersion` is read into `BundleVersion::build` but never parsed, so the Block-signed `-block` suffix cannot degrade the scenario to `ExistingUnknown`. A `-test.N` / `-intel-test.N` canary suffix on the short string parses as a semver prerelease and still sorts below every 0.7.x build. Every version block/buzz has shipped is < 0.6.0, so an installed old Buzz is always an Upgrade. - Path-keyed terminate. `tauri_plugin_single_instance` keeps a previous build of this app from running alongside the prompt, but the old Buzz has a different identifier, keeps running after its window closes (tray icon, self-updater that auto-downloads) and is not covered by that guard. Before trashing the installed bundle, the accept path now quits every running application whose `NSRunningApplication.bundleURL` normalises (via `canonicalize`) to the installed bundle path: polite `terminate()`, up to 2 s of polling, then `forceTerminate()`. The outcome is logged and the move continues either way, since trashing a running bundle succeeds. Liveness is checked through `isTerminated` plus `kill(pid, 0)`, because `NSRunningApplication` properties only update when the main run loop spins, which it does not while we poll on the main thread. Enabled `NSWorkspace`, `NSRunningApplication` and `libc` on the macOS-only `objc2-app-kit` dependency. A leftover `Sprout.app` is not handled. Verified: - New unit tests: `old_buzz_from_block_buzz_is_always_an_upgrade` (0.5.28 with `0.5.28` and `0.5.28-block` builds, 0.5.9-block, 0.3.18, `0.5.29-test.123`, `0.5.28-intel-test.45`, `0.0.1-test.3`, each against the version in `tauri.conf.json`), `old_buzz_prerelease_suffix_never_degrades_to_replace`, `old_buzz_and_previous_buzz_app_build_get_the_same_upgrade_dialog` (0.5.28 and 0.7.0 → 0.7.1 yield the same scenario variant, message and button, and informative text identical apart from the numbers), `installed_copy_path_uses_only_the_bundle_file_name`, and `normalized_bundle_path_matches_through_symlinks`. - `cargo test -p Buzz installer_media`: 26 passed. `cargo fmt --check`, `cargo clippy -p Buzz --all-targets -- -D warnings` and `cargo build --release -p Buzz` on macOS: clean. - The pure helpers were run (via a throwaway test, not committed) against the real `/Applications/Buzz.app/Contents/Info.plist` on this machine without launching or moving anything: identifier `xyz.block.buzz.app`, executable `buzz-desktop`, installed `0.5.28` / `0.5.28-block`, running `0.7.0` → `Upgrade { from: "0.5.28", to: "0.7.0" }`; dialog "Upgrade Buzz in your Applications folder?" / "You have Buzz 0.5.28 installed. This copy is the newer version 0.7.0. Buzz can replace the installed copy and relaunch from it." / button "Upgrade and Relaunch". Not exercised live: the terminate step against a running old Buzz, and the full DMG → replace flow over an installed old Buzz. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- Cargo.lock | 1 + src-tauri/Cargo.toml | 2 +- src-tauri/src/installer_media.rs | 270 +++++++++++++++++++++++++++++-- 3 files changed, 263 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a7b2aa7159..d9a08f8d2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3573,6 +3573,7 @@ checksum = "d49e936b501e5c5bf01fda3a9452ff86dc3ea98ad5f283e1455153142d97518c" dependencies = [ "bitflags 2.13.1", "block2 0.6.2", + "libc", "objc2 0.6.4", "objc2-core-foundation", "objc2-core-graphics", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index d5881d0142..901c723e6a 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -95,7 +95,7 @@ plist = "1" block2 = "=0.6.2" dispatch2 = "=0.3.1" objc2 = "=0.6.4" -objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["NSScreen", "NSWindow", "NSEvent", "NSAlert", "NSButton", "NSControl", "NSCell", "NSResponder", "NSView", "block2"] } +objc2-app-kit = { version = "=0.3.2", default-features = false, features = ["NSScreen", "NSWindow", "NSEvent", "NSAlert", "NSButton", "NSControl", "NSCell", "NSResponder", "NSView", "NSWorkspace", "NSRunningApplication", "libc", "block2"] } objc2-foundation = { version = "=0.3.2", default-features = false, features = ["std", "NSObject", "NSThread", "NSBundle", "NSString", "NSError", "NSUserDefaults", "NSProcessInfo", "NSFileManager", "NSURL"] } objc2-user-notifications = { version = "=0.3.2", default-features = false, features = ["UNUserNotificationCenter", "UNNotificationSettings", "UNNotification", "UNNotificationContent", "UNNotificationRequest", "UNNotificationResponse", "UNNotificationTrigger", "block2"] } diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index 6f05b6fb72..a597e9f2af 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -12,6 +12,13 @@ //! if we were running from one. //! //! Ported from berd (squareup/berd#561). +//! +//! The copy already installed in `/Applications` is located, compared, quit +//! and replaced purely by bundle path. Nothing here reads a bundle identifier, +//! executable name, signing identity or data directory, so the old Buzz from +//! block/buzz (`xyz.block.buzz.app`, executable `buzz-desktop`) and a previous +//! build of this app (`xyz.block.buzz.desktop`, executable `Buzz`), which share +//! the `Buzz.app` file name, go through exactly the same flow. use std::path::{Path, PathBuf}; @@ -106,6 +113,27 @@ fn bundle_path_from_executable(executable: &Path) -> Option { .map(Path::to_path_buf) } +/// Where a copy of the running bundle lives once installed: `/Applications/` +/// plus the running bundle's file name. +/// +/// Deliberately keyed on the file name alone, never on `CFBundleIdentifier`, +/// `CFBundleExecutable`, `CFBundleName`, the signing identity or the data +/// directory. The old Buzz from block/buzz (`xyz.block.buzz.app`, executable +/// `buzz-desktop`) and this app (`xyz.block.buzz.desktop`, executable `Buzz`) +/// both install as `Buzz.app`, and replacing either must look and behave the +/// same, so the lookup must not be able to tell them apart. +fn installed_copy_path(bundle_path: &Path) -> Option { + let app_name = bundle_path.file_name()?; + Some(Path::new("/Applications").join(app_name)) +} + +/// Resolve symlinks and `/private` aliasing so two spellings of the same +/// bundle directory compare equal. A path that can't be resolved (e.g. it no +/// longer exists) is compared as written. +fn normalized_bundle_path(path: &Path) -> PathBuf { + std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) +} + /// Version identity read from a bundle's `Contents/Info.plist`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BundleVersion { @@ -320,7 +348,7 @@ mod macos { let Some(bundle_path) = running_app_bundle_path() else { return; }; - let Some(app_name) = bundle_path.file_name() else { + let Some(target) = installed_copy_path(&bundle_path) else { return; }; let Some(volume) = volume_info(&bundle_path) else { @@ -340,8 +368,8 @@ mod macos { // Read both versions from disk (rather than trusting the compiled-in // version) so the running and installed copies are always compared - // apples-to-apples. - let target = Path::new("/Applications").join(app_name); + // apples-to-apples. Only `CFBundleShortVersionString` is compared; + // whatever is installed under this file name is treated the same. let running = read_bundle_version(&bundle_path); let installed = target .exists() @@ -500,10 +528,8 @@ mod macos { volume: &VolumeInfo, location: RunLocation, ) -> std::io::Result<()> { - let app_name = bundle_path - .file_name() + let target = installed_copy_path(bundle_path) .ok_or_else(|| std::io::Error::other("app bundle path has no file name"))?; - let target = Path::new("/Applications").join(app_name); // Stage the copy under a hidden temporary name on the same volume as // the target, so a failed or interrupted `ditto` (disk full, killed @@ -512,9 +538,9 @@ mod macos { // we trash the old copy and rename the staged one into place — an // atomic, same-volume rename. The move the dialog promised therefore // either happens completely or not at all. - let staging = Path::new("/Applications").join(format!( + let staging = target.with_file_name(format!( ".{}.{}.staging", - app_name.to_string_lossy(), + target.file_name().unwrap_or_default().to_string_lossy(), std::process::id() )); // Clear any leftover staging bundle from a previously interrupted run @@ -539,8 +565,12 @@ mod macos { // exists, goes to the Trash (recoverable and Finder-undoable) rather // than being overwritten in place. If trashing fails (e.g. a // read-only /Applications) we bail before the rename and discard the - // staged copy, leaving the existing install untouched. + // staged copy, leaving the existing install untouched. Any running + // instance of the installed bundle is quit first; trashing a running + // bundle still succeeds, so a copy that won't quit never blocks the + // move. if target.exists() { + terminate_running_copies(&target); if let Err(error) = move_to_trash(&target) { let _ = std::fs::remove_dir_all(&staging); return Err(error); @@ -580,6 +610,65 @@ mod macos { }) } + /// How long a running copy of the installed bundle gets to quit politely + /// before it is force-quit so the move can proceed. + const TERMINATE_GRACE: std::time::Duration = std::time::Duration::from_secs(2); + + /// Quit every running application whose bundle is `target`, so the copy + /// about to be trashed doesn't linger with a stale tray icon or write a + /// later self-update into the Trash. Matched by bundle *path*, not bundle + /// identifier: `tauri_plugin_single_instance` already guarantees a previous + /// build of this app isn't running, but the old block/buzz Buzz uses a + /// different identifier, keeps running after its window is closed, and + /// must be quit the same way without naming it. Best-effort: the outcome + /// is logged and the move continues regardless, because trashing a running + /// bundle succeeds on macOS. + fn terminate_running_copies(target: &Path) { + use objc2_app_kit::NSWorkspace; + + let target = normalized_bundle_path(target); + let running = NSWorkspace::sharedWorkspace().runningApplications(); + for app in running.iter() { + let Some(path) = app.bundleURL().and_then(|url| url.path()) else { + continue; + }; + if normalized_bundle_path(Path::new(&path.to_string())) != target { + continue; + } + let pid = app.processIdentifier(); + let asked = app.terminate(); + let deadline = std::time::Instant::now() + TERMINATE_GRACE; + while asked && !has_exited(&app, pid) && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(50)); + } + let outcome = if has_exited(&app, pid) { + "quit before replacement" + } else if app.forceTerminate() { + "force-quit before replacement" + } else { + "would not quit; replacing it anyway" + }; + eprintln!( + "Running copy of {} (pid {pid}): {outcome}", + target.display() + ); + } + } + + /// Whether a running application has exited. `isTerminated` only updates + /// when the main run loop spins, which it doesn't while we sleep-poll on + /// the main thread, so the kernel's view of the pid is checked as well. + fn has_exited(app: &objc2_app_kit::NSRunningApplication, pid: libc::pid_t) -> bool { + if app.isTerminated() { + return true; + } + // `kill(-1, 0)` would probe every process, so only a real pid counts. + // SAFETY: signal 0 performs no action beyond the existence check. + pid > 0 + && unsafe { libc::kill(pid, 0) } == -1 + && std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH) + } + /// Spawn a detached shell that waits for this process to exit, launches the /// installed copy, and (optionally) detaches the disk image we ran from. fn relaunch_after_exit(target: &Path, detach_mount: Option) -> std::io::Result<()> { @@ -746,6 +835,49 @@ mod tests { assert!(location.should_prompt_move()); } + #[test] + fn installed_copy_path_uses_only_the_bundle_file_name() { + // Whatever the running copy's identifier or executable, the installed + // copy is `/Applications/`. + assert_eq!( + installed_copy_path(Path::new("/Volumes/Buzz/Buzz.app")), + Some(PathBuf::from("/Applications/Buzz.app")) + ); + assert_eq!( + installed_copy_path(Path::new( + "/private/var/folders/ab/cd/T/AppTranslocation/UUID/d/Buzz.app" + )), + Some(PathBuf::from("/Applications/Buzz.app")) + ); + assert_eq!(installed_copy_path(Path::new("/")), None); + } + + #[cfg(unix)] + #[test] + fn normalized_bundle_path_matches_through_symlinks() { + let dir = tempfile::tempdir().expect("tempdir"); + let bundle = dir.path().join("Buzz.app"); + std::fs::create_dir(&bundle).expect("create bundle"); + let link = dir.path().join("Linked.app"); + std::os::unix::fs::symlink(&bundle, &link).expect("symlink"); + // `NSRunningApplication` reports a resolved bundle URL while the target + // is built from the file name; both must normalise to one path (on + // macOS this also folds `/var` onto `/private/var`). + assert_eq!( + normalized_bundle_path(&link), + normalized_bundle_path(&bundle) + ); + assert_ne!( + normalized_bundle_path(&link), + normalized_bundle_path(&dir.path().join("Other.app")) + ); + // A path that doesn't exist is compared as written rather than dropped. + assert_eq!( + normalized_bundle_path(Path::new("/Applications/Missing.app")), + PathBuf::from("/Applications/Missing.app") + ); + } + fn version(short: &str, build: &str) -> BundleVersion { BundleVersion { short: short.to_string(), @@ -781,6 +913,126 @@ mod tests { ); } + /// The `CFBundleShortVersionString` this build ships, read from the same + /// `tauri.conf.json` the bundler reads so the old-Buzz cases track the real + /// current version instead of a copy of it. + fn current_version() -> String { + let conf: serde_json::Value = + serde_json::from_str(include_str!("../tauri.conf.json")).expect("tauri.conf.json"); + conf["version"].as_str().expect("version").to_string() + } + + /// Every version family the old Buzz from block/buzz has shipped, as the + /// (`CFBundleShortVersionString`, `CFBundleVersion`) pairs found in its + /// bundles: the OSS and Block-signed (`-block`) 0.5.x releases, canary and + /// Intel-canary builds, and the Sprout-era test builds. + const OLD_BUZZ_VERSIONS: &[(&str, &str)] = &[ + ("0.5.28", "0.5.28"), + ("0.5.28", "0.5.28-block"), + ("0.5.9", "0.5.9-block"), + ("0.3.18", "0.3.18"), + ("0.5.29-test.123", "0.5.29-test.123"), + ("0.5.28-intel-test.45", "0.5.28-intel-test.45"), + ("0.0.1-test.3", "0.0.1-test.3"), + ]; + + #[test] + fn old_buzz_from_block_buzz_is_always_an_upgrade() { + let current = current_version(); + let running = version(¤t, ¤t); + for (short, build) in OLD_BUZZ_VERSIONS { + let installed = version(short, build); + assert_eq!( + compare_versions(&running, &installed), + VersionRelation::RunningNewer, + "{short} / {build}" + ); + assert_eq!( + scenario_for(true, Some(&running), Some(&installed)), + MoveScenario::Upgrade { + from: short.to_string(), + to: current.clone(), + }, + "{short} / {build}" + ); + } + } + + #[test] + fn old_buzz_prerelease_suffix_never_degrades_to_replace() { + // `CFBundleVersion` is read but never compared, so a `-block` or + // `-test.N` suffix there changes nothing; the same suffix on the short + // string parses as a semver prerelease rather than falling into + // `ExistingUnknown`. + let running = version("0.7.1", "0.7.1"); + let clean = scenario_for(true, Some(&running), Some(&version("0.5.28", "0.5.28"))); + assert_eq!( + scenario_for( + true, + Some(&running), + Some(&version("0.5.28", "0.5.28-block")) + ), + clean + ); + assert_eq!( + scenario_for( + true, + Some(&running), + Some(&version("0.5.28", "0.5.29-test.123")) + ), + clean + ); + for short in ["0.5.28-block", "0.5.29-test.123", "0.5.28-intel-test.45"] { + assert_eq!( + scenario_for(true, Some(&running), Some(&version(short, short))), + MoveScenario::Upgrade { + from: short.to_string(), + to: "0.7.1".to_string(), + }, + "{short}" + ); + } + } + + #[test] + fn old_buzz_and_previous_buzz_app_build_get_the_same_upgrade_dialog() { + // Same scenario variant, same message and button, and informative text + // identical apart from the version numbers: the dialog can't tell + // whether `/Applications/Buzz.app` is block/buzz's Buzz or an earlier + // build of this app. + let running = version("0.7.1", "0.7.1"); + let old_buzz = scenario_for( + true, + Some(&running), + Some(&version("0.5.28", "0.5.28-block")), + ); + let previous_buzz_app = + scenario_for(true, Some(&running), Some(&version("0.7.0", "0.7.0"))); + assert_eq!( + old_buzz, + MoveScenario::Upgrade { + from: "0.5.28".to_string(), + to: "0.7.1".to_string(), + } + ); + assert_eq!( + previous_buzz_app, + MoveScenario::Upgrade { + from: "0.7.0".to_string(), + to: "0.7.1".to_string(), + } + ); + + let old = alert_copy(&old_buzz, RunLocation::DiskImage); + let previous = alert_copy(&previous_buzz_app, RunLocation::DiskImage); + assert_eq!(old.message, previous.message); + assert_eq!(old.primary_button, previous.primary_button); + assert_eq!( + old.informative.replace("0.5.28", "0.7.0"), + previous.informative + ); + } + #[test] fn scenario_for_no_install_is_fresh() { assert_eq!( From 2a192849d764cd3c9b861e056d67c717b79ee3c8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 00:53:01 +1100 Subject: [PATCH 04/10] fix(macos): tear down as Quit does before relaunching from /Applications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accepting the move-to-/Applications prompt ended with `std::process::exit(0)` and skipped `crate::shut_down`, which Quit (`RunEvent::Exit`) and the fenced sign-out (`exit_fenced`) both run before exiting. Anything Buzz had started in its own process group outlived the DMG copy and kept running beside the relaunched installed copy: the login-shell probe that setup always spawns (its 10 s deadline is enforced by a tokio task that dies with the process), plugin host processes and the bundled agent runtimes, which run from `/Volumes/…`, terminal sessions and MCP hosts. The `Ok` arm now calls `crate::shut_down(app)` once the move and the relaunch shell have succeeded, then exits. Not earlier: the `Err` arm keeps the app running and still needs all of that. It is safe here: the block runs on the main thread from the main dispatch queue with tao's handler lock free, and Quit already calls the same function from inside that lock. It cannot stop the relaunch shell, which stays in Buzz's own process group; `shut_down` signals only child groups and the probe's group. `app.exit(0)` was rejected because it returns to the run loop and leaves the window interactive after `/Applications` has been replaced. Verified live with ad-hoc-signed release builds on hand-made DMGs, the bundle renamed `BuzzCheck.app` and the identifier overridden so neither `/Applications/Buzz.app` nor real app data was touched. A `ZDOTDIR` `.zshenv` held the login-shell probe in `/bin/sleep 4711`: - Before (pre-fix 0.7.0 build): after accepting, the held probe was still running, reparented to launchd (ppid 1). - After (0.7.1 build of this change, upgrading an installed 0.7.0): the held probe was gone once the DMG copy exited; the installed copy relaunched 0.54 s after the press and the DMG detached at 1.77 s. `cargo fmt --check` and `cargo clippy -p Buzz --all-targets -- -D warnings` are clean. No unit test: the change is one call in AppKit-only code, and the teardown pieces have their own tests. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index a597e9f2af..1ac8131bba 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -336,8 +336,8 @@ mod macos { /// thread, from a main-queue block scheduled at `RunEvent::Ready`: the /// modal alert redraws the visible main window, which re-enters tao, so it /// must not be called from inside a tao event-loop callback (see the call - /// site in `lib.rs`). On acceptance this relaunches from the installed - /// copy and exits the current process. + /// site in `lib.rs`). On acceptance this tears down as Quit does, + /// relaunches from the installed copy and exits the current process. pub(crate) fn maybe_prompt_move_to_applications(app: &AppHandle) { // Never interrupt development builds, which legitimately run from // `target/` and similar non-installed locations. @@ -402,6 +402,11 @@ mod macos { match move_to_applications_and_relaunch(&bundle_path, &volume, location) { Ok(()) => { eprintln!("Relaunching Buzz from /Applications"); + // Tear down as Quit does, so terminals, plugin processes and + // agent runtimes don't outlive us. Only now: had the move or + // the relaunch failed, the app would keep running and still + // need them. + crate::shut_down(app); std::process::exit(0); } Err(error) => { From fa11853d7ac4e46be3d73568ce444c6c0b728f5b Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 00:53:04 +1100 Subject: [PATCH 05/10] fix(macos): reclaim an interrupted move's staging copy, or stop The move staged its copy at `/Applications/.Buzz.app..staging`, so the `staging.exists()` cleanup only matched on pid reuse. A copy left by an interrupted earlier move (most likely a force-quit during the wait for a running installed copy to quit) was never removed, and each interruption added another hidden 66 MB bundle. The cleanup also dropped its error, and `ditto` merges into an existing destination, so a leftover that couldn't be removed would leave stale files in the new copy and break its signature. The staging copy now has one fixed name, `/Applications/.Buzz.app.staging` (`staging_path`), and `clear_staging` removes whatever is there before `ditto` runs. Nothing there is fine; any other removal error stops the move and reaches the existing failure alert, naming the path. A fixed name is safe because the single-instance socket is keyed by identifier only, so a DMG copy and the installed copy never move at the same time for one user; another user's staging tree can't be removed, which now fails closed. The name still ends in `.staging`, not `.app`, so Launch Services never registers it. Globbing pid-suffixed names was rejected: those only exist on machines that ran this unreleased branch. Verified: - New unit tests `staging_path_is_hidden_fixed_and_beside_the_target`, `clear_staging_removes_a_leftover_copy`, `clear_staging_accepts_nothing_to_clear` and `clear_staging_fails_when_a_leftover_cannot_be_removed`. With the old behaviour restored (pid in the name, removal errors ignored) the first and last fail. `cargo test -p Buzz installer_media`: 30 passed. - Live, same harness as the previous commit (`BuzzCheck.app`, overridden identifier): - Interrupted move: with a stand-in installed at `/Applications/BuzzCheck.app` and held under SIGSTOP, `kill -9` of the DMG copy after `ditto` finished left `.BuzzCheck.app.staging`. With a stale file planted in it and the stand-in quit, the next accepted move removed it: the installed copy has no stale file, `codesign --verify --deep --strict` passes and no `.staging` remains. Before (pre-fix build), an interrupted move left `.BuzzCheck.app.8740.staging`, which was still there after the next move completed. - Leftover that can't be removed (`Contents` at 0555): accepting showed "Couldn't move Buzz to your Applications folder", the error logged and passed to it was "failed to remove leftover staging copy /Applications/.BuzzCheck.app.staging: Permission denied (os error 13)", the installed copy was untouched and the DMG copy kept running after OK. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 107 +++++++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 12 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index 1ac8131bba..7b9f997527 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -134,6 +134,37 @@ fn normalized_bundle_path(path: &Path) -> PathBuf { std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) } +/// Where a move stages its copy before renaming it over `target`: +/// `/Applications/.Buzz.app.staging`. Beside the target, so the final rename +/// is same-volume and atomic; hidden, and not an `.app`, so Launch Services +/// never registers it. The name is fixed because the single-instance guard +/// lets only one move run at a time, so anything already there was left by +/// an earlier move that was interrupted. +fn staging_path(target: &Path) -> PathBuf { + let mut name = std::ffi::OsString::from("."); + name.push(target.file_name().unwrap_or_default()); + name.push(".staging"); + target.with_file_name(name) +} + +/// Remove a leftover staging copy so the next copy starts from nothing. +/// Nothing to remove is fine. A leftover that can't be removed is an error: +/// `ditto` would merge into it, and the stale files would break the new +/// copy's code signature. +fn clear_staging(staging: &Path) -> std::io::Result<()> { + match std::fs::remove_dir_all(staging) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(std::io::Error::new( + error.kind(), + format!( + "failed to remove leftover staging copy {}: {error}", + staging.display() + ), + )), + } +} + /// Version identity read from a bundle's `Contents/Info.plist`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BundleVersion { @@ -536,23 +567,18 @@ mod macos { let target = installed_copy_path(bundle_path) .ok_or_else(|| std::io::Error::other("app bundle path has no file name"))?; - // Stage the copy under a hidden temporary name on the same volume as - // the target, so a failed or interrupted `ditto` (disk full, killed + // Stage the copy under a hidden name on the same volume as the + // target, so a failed or interrupted `ditto` (disk full, killed // mid-copy) can never leave a working installed copy trashed and the // target a half-written bundle. Only once the copy fully succeeds do // we trash the old copy and rename the staged one into place — an // atomic, same-volume rename. The move the dialog promised therefore // either happens completely or not at all. - let staging = target.with_file_name(format!( - ".{}.{}.staging", - target.file_name().unwrap_or_default().to_string_lossy(), - std::process::id() - )); - // Clear any leftover staging bundle from a previously interrupted run - // so `ditto` writes into a clean location rather than merging into it. - if staging.exists() { - let _ = std::fs::remove_dir_all(&staging); - } + let staging = staging_path(&target); + // At most one staging copy exists. One left by an interrupted earlier + // move is removed first; if it can't be, the move stops here rather + // than copying into it. + clear_staging(&staging)?; // `ditto` is the macOS-blessed way to copy an app bundle while // preserving its structure and metadata. @@ -883,6 +909,63 @@ mod tests { ); } + #[test] + fn staging_path_is_hidden_fixed_and_beside_the_target() { + // No pid or other per-run part, so a later move finds an earlier + // move's leftover; never an `.app`, so Launch Services ignores it. + assert_eq!( + staging_path(Path::new("/Applications/Buzz.app")), + PathBuf::from("/Applications/.Buzz.app.staging") + ); + assert_eq!( + staging_path(Path::new("/Applications/Buzz Preview.app")), + PathBuf::from("/Applications/.Buzz Preview.app.staging") + ); + } + + #[test] + fn clear_staging_removes_a_leftover_copy() { + let dir = tempfile::tempdir().expect("tempdir"); + let staging = dir.path().join(".Buzz.app.staging"); + std::fs::create_dir_all(staging.join("Contents/MacOS")).expect("create leftover"); + std::fs::write(staging.join("Contents/MacOS/Buzz"), b"stale").expect("write leftover"); + + clear_staging(&staging).expect("clear leftover"); + + assert!(!staging.exists()); + } + + #[test] + fn clear_staging_accepts_nothing_to_clear() { + let dir = tempfile::tempdir().expect("tempdir"); + clear_staging(&dir.path().join(".Buzz.app.staging")).expect("nothing to clear"); + } + + #[cfg(unix)] + #[test] + fn clear_staging_fails_when_a_leftover_cannot_be_removed() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("tempdir"); + let staging = dir.path().join(".Buzz.app.staging"); + let contents = staging.join("Contents"); + std::fs::create_dir_all(&contents).expect("create leftover"); + std::fs::write(contents.join("x"), b"stale").expect("write leftover"); + // Entries of a read-only directory can't be unlinked, even by its owner. + std::fs::set_permissions(&contents, std::fs::Permissions::from_mode(0o555)) + .expect("lock leftover"); + let result = clear_staging(&staging); + std::fs::set_permissions(&contents, std::fs::Permissions::from_mode(0o755)) + .expect("unlock leftover"); + + let error = result.expect_err("a leftover that can't be removed stops the move"); + assert!( + error.to_string().contains(&staging.display().to_string()), + "{error}" + ); + assert!(contents.join("x").exists()); + } + fn version(short: &str, build: &str) -> BundleVersion { BundleVersion { short: short.to_string(), From 08c27f1930a366227a118b86635aa1353735d636 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 00:53:06 +1100 Subject: [PATCH 06/10] fix(macos): give a running old Buzz time to stop its agents before force-quitting Before trashing the installed copy, the accept path asks any running copy of it to quit and force-quits it after `TERMINATE_GRACE`. The old Buzz from block/buzz never refuses or delays the quit (tao implements no `applicationShouldTerminate:` and its `ExitRequested` arm never prevents exit), but its quit stops its managed agents first: SIGTERM to each agent's process group, up to 2 s, then SIGKILL for any still running. Our 2 s cap starts slightly earlier, so `forceTerminate()` landed just before that SIGKILL fallback. Agents still alive then were orphaned, kept running from `~/.Trash/Buzz.app`, and nothing would ever sweep them. `TERMINATE_GRACE` is now 3 s, with a doc comment tying it to that teardown. The wait ends as soon as the app exits, so the longer cap only costs time while the old copy is still stopping. The wait stays synchronous on the main thread: a shorter cap or force-quitting first would orphan more agents, and waiting without blocking would let tao and Tauri callbacks, IPC and Quit run while /Applications is half replaced. Verified live with the same harness, using a stand-in buzz-app build with a different identifier (`xyz.block.buzz.desktop.dmgdemo.standin`) installed as `/Applications/BuzzCheck.app`, launched, and its window closed so it keeps running hidden like old Buzz: - Running normally: "Running copy of /Applications/BuzzCheck.app (pid 71631): quit before replacement" 0.21 s after the press; the move completed. - Held under SIGSTOP: "force-quit before replacement" 3.12 s after the press (the 3 s grace after `ditto`); the move completed and the new copy relaunched. `cargo fmt --check`, `cargo clippy -p Buzz --all-targets -- -D warnings` and `cargo test -p Buzz installer_media` (30 passed) are clean on this commit. Not exercised: old Buzz's own quit under `terminate()`, and an agent that ignores SIGTERM. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index 7b9f997527..16b8f0716b 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -642,8 +642,13 @@ mod macos { } /// How long a running copy of the installed bundle gets to quit politely - /// before it is force-quit so the move can proceed. - const TERMINATE_GRACE: std::time::Duration = std::time::Duration::from_secs(2); + /// before it is force-quit so the move can proceed. The old block/buzz + /// Buzz never refuses to quit, but its quit stops its agents first: + /// SIGTERM, up to 2 s, then SIGKILL for any still running. Force-quitting + /// it inside that window would orphan those agents, still running from + /// the Trash, so the cap leaves room for it. The wait ends as soon as the + /// app exits, so the cap only costs time while it is still stopping. + const TERMINATE_GRACE: std::time::Duration = std::time::Duration::from_secs(3); /// Quit every running application whose bundle is `target`, so the copy /// about to be trashed doesn't linger with a stale tray icon or write a From 17c7077448b2a8f732c64d442a5802f405dbe90c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 14:51:01 +1100 Subject: [PATCH 07/10] fix(macos): don't force-quit an installed copy that accepted terminate() Before trashing the installed copy, the accept path asked any running copy of it to quit, polled for up to `TERMINATE_GRACE` (3 s), then force-quit it. No fixed cap fits the old Buzz from block/buzz, which is the copy this realistically hits: its quit closes terminal sessions one at a time (up to 250 ms each, and an interactive zsh ignores SIGTERM, so each takes the full 250 ms), then gives its agents up to 2 s before SIGKILL, and only its later orphan/system sweeps kill agent workers in their own process groups. With a busy agent and a few terminal tabs, `forceTerminate()` landed mid-teardown and orphaned agents that kept running from `~/.Trash/Buzz.app`, connected to their relays, with nothing in buzz-app to ever clean them up. The move doesn't need the old copy to have exited, because trashing a running bundle succeeds on macOS. So a copy that accepts `terminate()` is now left to finish its own shutdown, from the Trash if need be, and the move goes ahead at once. `forceTerminate()` is used only for a copy that didn't accept the request. `TERMINATE_GRACE`, the polling loop and `has_exited` are gone, and with them the synchronous main-thread wait. The doc comment no longer claims old Buzz stops its agents first. Tradeoffs: old Buzz's tray icon can stay visible for a few seconds after the new copy relaunches, and a hung old Buzz that accepted the request keeps running from the Trash instead of being killed (force-quitting it would orphan its agents anyway). `cargo fmt --check`, `cargo clippy -p Buzz --all-targets -- -D warnings` and `cargo test -p Buzz installer_media` (30 passed) are clean. Not exercised live: the accept path against a running installed copy. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 62 +++++++++++--------------------- 1 file changed, 20 insertions(+), 42 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index 16b8f0716b..8da8658c05 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -597,9 +597,9 @@ mod macos { // than being overwritten in place. If trashing fails (e.g. a // read-only /Applications) we bail before the rename and discard the // staged copy, leaving the existing install untouched. Any running - // instance of the installed bundle is quit first; trashing a running - // bundle still succeeds, so a copy that won't quit never blocks the - // move. + // instance of the installed bundle is asked to quit first, without + // waiting for it: trashing a running bundle still succeeds, so a copy + // that is still quitting, or won't quit, never blocks the move. if target.exists() { terminate_running_copies(&target); if let Err(error) = move_to_trash(&target) { @@ -641,24 +641,21 @@ mod macos { }) } - /// How long a running copy of the installed bundle gets to quit politely - /// before it is force-quit so the move can proceed. The old block/buzz - /// Buzz never refuses to quit, but its quit stops its agents first: - /// SIGTERM, up to 2 s, then SIGKILL for any still running. Force-quitting - /// it inside that window would orphan those agents, still running from - /// the Trash, so the cap leaves room for it. The wait ends as soon as the - /// app exits, so the cap only costs time while it is still stopping. - const TERMINATE_GRACE: std::time::Duration = std::time::Duration::from_secs(3); - - /// Quit every running application whose bundle is `target`, so the copy - /// about to be trashed doesn't linger with a stale tray icon or write a - /// later self-update into the Trash. Matched by bundle *path*, not bundle + /// Ask every running application whose bundle is `target` to quit, so the + /// copy about to be trashed doesn't linger with a stale tray icon or write + /// a later self-update into the Trash. Matched by bundle *path*, not bundle /// identifier: `tauri_plugin_single_instance` already guarantees a previous /// build of this app isn't running, but the old block/buzz Buzz uses a /// different identifier, keeps running after its window is closed, and - /// must be quit the same way without naming it. Best-effort: the outcome - /// is logged and the move continues regardless, because trashing a running - /// bundle succeeds on macOS. + /// must be quit the same way without naming it. + /// + /// A copy that accepts `terminate()` is left to finish quitting on its own, + /// from the Trash if need be, because trashing a running bundle succeeds on + /// macOS. Old Buzz closes its terminals and stops its agents and their + /// workers before it exits, and that takes as long as it takes, so + /// force-quitting it after any fixed wait could orphan agents that nothing + /// would ever clean up. Only a copy that doesn't accept the request is + /// force-quit. The outcome is logged and the move continues regardless. fn terminate_running_copies(target: &Path) { use objc2_app_kit::NSWorkspace; @@ -671,40 +668,21 @@ mod macos { if normalized_bundle_path(Path::new(&path.to_string())) != target { continue; } - let pid = app.processIdentifier(); - let asked = app.terminate(); - let deadline = std::time::Instant::now() + TERMINATE_GRACE; - while asked && !has_exited(&app, pid) && std::time::Instant::now() < deadline { - std::thread::sleep(std::time::Duration::from_millis(50)); - } - let outcome = if has_exited(&app, pid) { - "quit before replacement" + let outcome = if app.terminate() { + "asked to quit before replacement" } else if app.forceTerminate() { "force-quit before replacement" } else { "would not quit; replacing it anyway" }; eprintln!( - "Running copy of {} (pid {pid}): {outcome}", - target.display() + "Running copy of {} (pid {}): {outcome}", + target.display(), + app.processIdentifier() ); } } - /// Whether a running application has exited. `isTerminated` only updates - /// when the main run loop spins, which it doesn't while we sleep-poll on - /// the main thread, so the kernel's view of the pid is checked as well. - fn has_exited(app: &objc2_app_kit::NSRunningApplication, pid: libc::pid_t) -> bool { - if app.isTerminated() { - return true; - } - // `kill(-1, 0)` would probe every process, so only a real pid counts. - // SAFETY: signal 0 performs no action beyond the existence check. - pid > 0 - && unsafe { libc::kill(pid, 0) } == -1 - && std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH) - } - /// Spawn a detached shell that waits for this process to exit, launches the /// installed copy, and (optionally) detaches the disk image we ran from. fn relaunch_after_exit(target: &Path, detach_mount: Option) -> std::io::Result<()> { From af927628c55dcb773a0b9b848ec2f104ec866ae8 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 17:24:18 +1100 Subject: [PATCH 08/10] docs(macos): drop the private repository reference from installer_media Review on #1792 flagged that the `installer_media` module comment pointed to a private repository and issue, which public contributors can't open and which exposes internal coordination details. That line is removed. The comment still names its public provenance, the Potion Factory `LetsMove` flow it mirrors. No license or authorship attribution is lost: the referenced code is Block's own, and none of it was copied from a third party. The same reference and other discussion of that internal implementation were also removed from the branch's earlier commit messages and from the PR description. Comment-only change; not built or tested. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index 8da8658c05..ab9cc9790b 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -11,8 +11,6 @@ //! quarantine metadata, relaunch the installed copy, and detach the disk image //! if we were running from one. //! -//! Ported from berd (squareup/berd#561). -//! //! The copy already installed in `/Applications` is located, compared, quit //! and replaced purely by bundle path. Nothing here reads a bundle identifier, //! executable name, signing identity or data directory, so the old Buzz from From e777fa409205c9068290772bcbb886b27ea4a7c9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 19:04:40 +1100 Subject: [PATCH 09/10] fix(macos): put the installed copy back if replacing it fails Replacing an installed copy trashed it, then renamed the staged copy into /Applications. If the rename failed after the Trash had succeeded, the error path only discarded the staged copy, so /Applications/Buzz.app was left absent, and `move_to_trash` had dropped the URL the Trash returned, so nothing could put it back. The user was then told "Buzz will keep running from its current location" with no copy installed at all. A failed relaunch after a successful rename had a related gap: the same alert said the move failed while the new copy stayed installed and the old one sat in the Trash. The sequencing now lives in `replace_installed_copy`, which takes the trash step, and the steps after the rename (clearing quarantine and starting the relaunch shell), as functions. If the rename or those steps fail, it takes the new copy back out and renames the trashed copy back to /Applications, so a failed move leaves /Applications as it found it and the existing failure alert stays true. If the old copy can't be put back, the error, which is logged and shown in that alert, names where in the Trash it is. `move_to_trash` now asks `trashItemAtURL:resultingItemURL:` for the resulting URL and returns it, rather than guessing a location the Trash may have renamed on collision. Restoring from that URL was chosen over swapping the staged copy in with `renamex_np(RENAME_SWAP)` and trashing the old copy afterwards: the swap would put the old copy in the Trash as `.Buzz.app.staging`, hidden by Finder, and Put Back would restore it under that name. Tradeoffs and behaviour changes: - A process killed between the Trash and the rename still leaves the installed path empty with the old copy in the Trash. The window is two file operations wide; the swap would have closed it. - If macOS reported a successful Trash without a resulting URL (its API documents that it always returns one), the move stops with an error saying the copy was moved to the Trash, rather than carrying on with no way back. - A fresh install whose relaunch shell fails to start now removes the new copy too, so the failure alert's "keep running from its current location" holds there as well. Verified: new unit tests drive `replace_installed_copy` in temp directories with real renames and a stand-in Trash: `replace_installed_copy_trashes_the_old_copy_and_installs_the_new_one`, `..._puts_the_old_copy_back_when_the_rename_fails` (the staged copy disappears after the Trash), `..._puts_the_old_copy_back_when_finishing_fails` and `..._names_the_trashed_copy_when_it_cannot_be_put_back` (the folder made read-only after the Trash, as in the review's probe). `cargo fmt --check`, `cargo clippy -p Buzz --all-targets -- -D warnings` and `cargo test -p Buzz installer_media` (35 passed) are clean; they were run once, on the tree with the following commit applied. Not exercised: the real Trash URL from `NSFileManager`, or any move into /Applications. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 246 +++++++++++++++++++++++++++---- 1 file changed, 216 insertions(+), 30 deletions(-) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index ab9cc9790b..bcb7666fe8 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -163,6 +163,56 @@ fn clear_staging(staging: &Path) -> std::io::Result<()> { } } +/// Rename the staged copy to `target`, then run `finish`. A copy already at +/// `target` is passed to `trash` first, which moves it to the Trash and returns +/// where it went. If the rename or `finish` fails, the new copy is taken back +/// out and the trashed copy renamed back to `target`, so a failed move leaves +/// `target` as it found it. If the trashed copy can't be put back, the error +/// says where in the Trash it is. `trash` is a parameter so tests can stand in +/// for the real Trash. +fn replace_installed_copy( + staging: &Path, + target: &Path, + trash: impl FnOnce(&Path) -> std::io::Result, + finish: impl FnOnce() -> std::io::Result<()>, +) -> std::io::Result<()> { + let trashed = if target.exists() { + match trash(target) { + Ok(trashed) => Some(trashed), + Err(error) => { + let _ = std::fs::remove_dir_all(staging); + return Err(error); + } + } + } else { + None + }; + let result = std::fs::rename(staging, target).and_then(|()| { + finish().inspect_err(|_| { + let _ = std::fs::rename(target, staging); + }) + }); + let Err(error) = result else { + return Ok(()); + }; + let _ = std::fs::remove_dir_all(staging); + let Some(trashed) = trashed else { + return Err(error); + }; + match std::fs::rename(&trashed, target) { + Ok(()) => Err(error), + Err(restore_error) => Err(std::io::Error::new( + error.kind(), + format!( + "{error}; the previous copy couldn't be put back at {} ({restore_error}) \ + and is in the Trash at {}", + target.display(), + trashed.display() + ), + )), + } +} + /// Version identity read from a bundle's `Contents/Info.plist`. #[derive(Debug, Clone, PartialEq, Eq)] pub struct BundleVersion { @@ -593,49 +643,59 @@ mod macos { // steps are fast and the rename is atomic. An installed copy, if one // exists, goes to the Trash (recoverable and Finder-undoable) rather // than being overwritten in place. If trashing fails (e.g. a - // read-only /Applications) we bail before the rename and discard the - // staged copy, leaving the existing install untouched. Any running - // instance of the installed bundle is asked to quit first, without - // waiting for it: trashing a running bundle still succeeds, so a copy - // that is still quitting, or won't quit, never blocks the move. - if target.exists() { - terminate_running_copies(&target); - if let Err(error) = move_to_trash(&target) { - let _ = std::fs::remove_dir_all(&staging); - return Err(error); - } - } - if let Err(error) = std::fs::rename(&staging, &target) { - let _ = std::fs::remove_dir_all(&staging); - return Err(error); - } - - // Clear quarantine so Gatekeeper doesn't re-prompt or translocate the - // freshly installed copy. - let _ = Command::new("/usr/bin/xattr") - .args(["-r", "-d", "com.apple.quarantine"]) - .arg(&target) - .status(); - + // read-only /Applications) we stop before the rename and discard the + // staged copy, leaving the existing install untouched; if a later + // step fails, the trashed copy is put back. Any running instance of + // the installed bundle is asked to quit first, without waiting for + // it: trashing a running bundle still succeeds, so a copy that is + // still quitting, or won't quit, never blocks the move. let detach_mount = matches!(location, RunLocation::DiskImage).then(|| volume.mount_point.clone()); - relaunch_after_exit(&target, detach_mount) + replace_installed_copy( + &staging, + &target, + |installed| { + terminate_running_copies(installed); + move_to_trash(installed) + }, + || { + // Clear quarantine so Gatekeeper doesn't re-prompt or + // translocate the freshly installed copy. + let _ = Command::new("/usr/bin/xattr") + .args(["-r", "-d", "com.apple.quarantine"]) + .arg(&target) + .status(); + relaunch_after_exit(&target, detach_mount) + }, + ) } - /// Move `path` to the user's Trash via `NSFileManager`. Preferred over - /// `rm -rf` because it's recoverable and Finder-undoable if the user changes - /// their mind about replacing the installed copy. - fn move_to_trash(path: &Path) -> std::io::Result<()> { + /// Move `path` to the user's Trash via `NSFileManager` and return where it + /// went, which can differ from its name when the Trash already holds one. + /// Preferred over `rm -rf` because it's recoverable and Finder-undoable if + /// the user changes their mind about replacing the installed copy. + fn move_to_trash(path: &Path) -> std::io::Result { use objc2_foundation::{NSFileManager, NSString, NSURL}; let url = NSURL::fileURLWithPath(&NSString::from_str(&path.to_string_lossy())); + let mut trashed = None; NSFileManager::defaultManager() - .trashItemAtURL_resultingItemURL_error(&url, None) + .trashItemAtURL_resultingItemURL_error(&url, Some(&mut trashed)) .map_err(|error| { std::io::Error::other(format!( "failed to move {} to Trash: {error:?}", path.display() )) + })?; + // macOS reports the new location whenever the move succeeds. + trashed + .and_then(|url| url.path()) + .map(|trashed| PathBuf::from(trashed.to_string())) + .ok_or_else(|| { + std::io::Error::other(format!( + "moved {} to the Trash, but macOS didn't say where", + path.display() + )) }) } @@ -947,6 +1007,132 @@ mod tests { assert!(contents.join("x").exists()); } + /// An installed `Applications/Buzz.app` holding `old`, its staged + /// replacement holding `new`, and an empty stand-in Trash. + fn installed_and_staged() -> (tempfile::TempDir, PathBuf, PathBuf, PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + let target = dir.path().join("Applications/Buzz.app"); + let staging = staging_path(&target); + let trash = dir.path().join("Trash"); + std::fs::create_dir_all(&target).expect("create installed copy"); + std::fs::write(target.join("old"), b"").expect("write installed copy"); + std::fs::create_dir_all(&staging).expect("create staged copy"); + std::fs::write(staging.join("new"), b"").expect("write staged copy"); + std::fs::create_dir(&trash).expect("create trash"); + (dir, target, staging, trash) + } + + /// Stand-in for the Trash: moves the installed copy into `trash`, like + /// `NSFileManager` does, and returns where it went. + fn move_into(trash: &Path, installed: &Path) -> std::io::Result { + let trashed = trash.join(installed.file_name().expect("file name")); + std::fs::rename(installed, &trashed)?; + Ok(trashed) + } + + #[test] + fn replace_installed_copy_trashes_the_old_copy_and_installs_the_new_one() { + let (_dir, target, staging, trash) = installed_and_staged(); + let mut finished = false; + + replace_installed_copy( + &staging, + &target, + |installed| move_into(&trash, installed), + || { + finished = true; + Ok(()) + }, + ) + .expect("replace"); + + assert!(finished); + assert!(target.join("new").exists()); + assert!(!target.join("old").exists()); + assert!(trash.join("Buzz.app/old").exists()); + assert!(!staging.exists()); + } + + #[test] + fn replace_installed_copy_puts_the_old_copy_back_when_the_rename_fails() { + let (_dir, target, staging, trash) = installed_and_staged(); + + let error = replace_installed_copy( + &staging, + &target, + |installed| { + let trashed = move_into(&trash, installed)?; + // The staged copy is gone by the time it's renamed. + std::fs::remove_dir_all(&staging)?; + Ok(trashed) + }, + || panic!("finish runs only after the rename"), + ) + .expect_err("the rename fails"); + + assert_eq!(error.kind(), std::io::ErrorKind::NotFound, "{error}"); + assert!(target.join("old").exists()); + assert!(!trash.join("Buzz.app").exists()); + } + + #[test] + fn replace_installed_copy_puts_the_old_copy_back_when_finishing_fails() { + let (_dir, target, staging, trash) = installed_and_staged(); + + let error = replace_installed_copy( + &staging, + &target, + |installed| move_into(&trash, installed), + || Err(std::io::Error::other("relaunch failed")), + ) + .expect_err("finishing fails"); + + assert_eq!(error.to_string(), "relaunch failed"); + assert!(target.join("old").exists()); + assert!(!target.join("new").exists()); + assert!(!trash.join("Buzz.app").exists()); + assert!(!staging.exists()); + } + + #[cfg(unix)] + #[test] + fn replace_installed_copy_names_the_trashed_copy_when_it_cannot_be_put_back() { + use std::os::unix::fs::PermissionsExt; + + let (_dir, target, staging, trash) = installed_and_staged(); + let applications = target.parent().expect("parent").to_path_buf(); + + let result = replace_installed_copy( + &staging, + &target, + |installed| { + let trashed = move_into(&trash, installed)?; + // Nothing more can be renamed into or out of the folder. + std::fs::set_permissions(&applications, std::fs::Permissions::from_mode(0o555))?; + Ok(trashed) + }, + || panic!("finish runs only after the rename"), + ); + std::fs::set_permissions(&applications, std::fs::Permissions::from_mode(0o755)) + .expect("unlock folder"); + + let error = result.expect_err("the rename and the restore both fail"); + let trashed = trash.join("Buzz.app"); + assert_eq!( + error.kind(), + std::io::ErrorKind::PermissionDenied, + "{error}" + ); + assert!( + error + .to_string() + .contains(&format!("is in the Trash at {}", trashed.display())), + "{error}" + ); + assert!(trashed.join("old").exists()); + assert!(!target.exists()); + } + fn version(short: &str, build: &str) -> BundleVersion { BundleVersion { short: short.to_string(), From 400ce9369356ee97ce9369ba0255f40860d51308 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 29 Oct 2026 19:04:55 +1100 Subject: [PATCH 10/10] fix(macos): don't offer to move, or ask to quit, the copy that is running Two gaps let the move-to-/Applications flow mistake the running bundle for the installed copy it replaces. Classification looks only at the running bundle's own path, so a copy in ~/Downloads still prompted when /Applications/Buzz.app was a symlink to it, offering to "reinstall" that copy over itself. On accept, `terminate_running_copies` canonicalises the installed path, which then matches this process, and it had no own-PID exclusion, so it would have asked the installer itself to quit (or force-quit it) before the replacement and relaunch had finished. Two fixes: - `terminate_running_copies` skips the application whose `processIdentifier` is `std::process::id()`, whatever its bundle path resolves to. - Before prompting, `is_running_bundle` compares the running bundle and the installed copy path through the existing `normalized_bundle_path` (`canonicalize`). When they are the same bundle, Buzz logs it and doesn't prompt: opening /Applications/Buzz.app already opens this copy, so there is nothing to move it over, and not prompting fits the module's fail-open stance. Prompting anyway was rejected: the dialog could only offer to reinstall a copy over itself. The check runs after classification, so an ordinary launch from /Applications neither canonicalises anything nor logs. Tradeoffs: the identity check runs once, before the modal alert, so a symlink created while the alert is up isn't caught; the PID exclusion still keeps this process from being asked to quit. Someone who deliberately symlinked /Applications/Buzz.app to a copy on a disk image is no longer prompted and keeps running from it. Verified: new unit test `installed_copy_symlinked_to_the_running_bundle_is_the_running_bundle` (temp dir: a symlink at Applications/Buzz.app to Downloads/Buzz.app matches; a separate real copy and a missing path don't). `cargo fmt --check`, `cargo clippy -p Buzz --all-targets -- -D warnings` and `cargo test -p Buzz installer_media` (35 passed) are clean on this commit. Not exercised: the PID exclusion against a real `NSRunningApplication` list (it's one comparison in AppKit-only code and has no unit test), and no prompt was launched. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- src-tauri/src/installer_media.rs | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src-tauri/src/installer_media.rs b/src-tauri/src/installer_media.rs index bcb7666fe8..c2efbd072c 100644 --- a/src-tauri/src/installer_media.rs +++ b/src-tauri/src/installer_media.rs @@ -132,6 +132,13 @@ fn normalized_bundle_path(path: &Path) -> PathBuf { std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) } +/// Whether the installed copy at `installed` is the running bundle itself, as +/// when `/Applications/Buzz.app` is a symlink to it. Launching the installed +/// copy then already launches this one, so there is nothing to move it over. +fn is_running_bundle(bundle_path: &Path, installed: &Path) -> bool { + normalized_bundle_path(bundle_path) == normalized_bundle_path(installed) +} + /// Where a move stages its copy before renaming it over `target`: /// `/Applications/.Buzz.app.staging`. Beside the target, so the final rename /// is same-volume and atomic; hidden, and not an `.app`, so Launch Services @@ -438,6 +445,17 @@ mod macos { if !location.should_prompt_move() { return; } + // The classification only looks at this copy's path; if the installed + // copy resolves to this same bundle, this copy is the installed one, + // and a move would replace it with a copy of itself. + if is_running_bundle(&bundle_path, &target) { + eprintln!( + "{} resolves to this copy of Buzz ({}); not prompting to move it", + target.display(), + bundle_path.display() + ); + return; + } // Respect a prior "Do not show this message again" choice. let marker = suppression_marker_path(app); @@ -714,12 +732,19 @@ mod macos { /// force-quitting it after any fixed wait could orphan agents that nothing /// would ever clean up. Only a copy that doesn't accept the request is /// force-quit. The outcome is logged and the move continues regardless. + /// + /// This process is never asked to quit, even if `target` resolves to its + /// own bundle. fn terminate_running_copies(target: &Path) { use objc2_app_kit::NSWorkspace; let target = normalized_bundle_path(target); + let own_pid = std::process::id(); let running = NSWorkspace::sharedWorkspace().runningApplications(); for app in running.iter() { + if u32::try_from(app.processIdentifier()) == Ok(own_pid) { + continue; + } let Some(path) = app.bundleURL().and_then(|url| url.path()) else { continue; }; @@ -950,6 +975,27 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn installed_copy_symlinked_to_the_running_bundle_is_the_running_bundle() { + let dir = tempfile::tempdir().expect("tempdir"); + let running = dir.path().join("Downloads/Buzz.app"); + std::fs::create_dir_all(&running).expect("create running bundle"); + let installed = dir.path().join("Applications/Buzz.app"); + std::fs::create_dir(installed.parent().expect("parent")).expect("create Applications"); + std::os::unix::fs::symlink(&running, &installed).expect("symlink"); + assert!(is_running_bundle(&running, &installed)); + + // A real installed copy, or none at all, is a different bundle. + let other = dir.path().join("Other/Buzz.app"); + std::fs::create_dir_all(&other).expect("create other copy"); + assert!(!is_running_bundle(&running, &other)); + assert!(!is_running_bundle( + &running, + &dir.path().join("Missing/Buzz.app") + )); + } + #[test] fn staging_path_is_hidden_fixed_and_beside_the_target() { // No pid or other per-run part, so a later move finds an earlier