fix(updater): restore the current app when a macOS install fails (fix #3505, #3506) (#3578)

* fix(updater): restore the current app when a macOS install fails

On macOS `install_inner` moved the installed app into a `TempDir` backup and
then renamed the new bundle into place. Neither of the two steps after that
move restored the app on failure, and the `TempDir` deleted the backup on
every exit path, so a failed final rename left the user with no app at the
install path and no way back. The privileged fallback had the same shape:
`rm -rf` the app, then `mv` the new one in.

Both moves are now `replace_bundle`, which renames the current app to the
backup, renames the staged bundle into place, and renames the backup back if
that second step fails. The privileged script moves the current app aside,
moves the new one in, and restores on failure; it deletes the previous bundle
only after the new one is in place. The temp and install locations are
compared with `st_dev` before anything moves, returning the existing
`TempDirNotOnSameMountPoint` rather than failing after the app has already
been moved, and the staged bundle root is set to 0755 because `tempfile`
creates it 0700 and that mode followed it into `/Applications`.

Two unit tests cover `replace_bundle`: the staged bundle ends up in place with
the previous one in the backup, and a failed second rename leaves the current
bundle where it was and returns that error.

Closes #3505, closes #3506.

* fix(updater): exchange the current and new bundles atomically on APFS

Even with the restore in place there was a moment between the two renames
where the install path held nothing, and a process killed in that moment
left the previous app in the temp dir rather than where it belonged.

`swap_bundle` calls `renamex_np` with `RENAME_SWAP`, which exchanges the two
directories in one step so the install path always holds one of the two. The
previous app ends up in the extraction temp dir and is removed with it. Where
the file system does not support the swap (`ENOTSUP`, or `EINVAL` on older
systems) the two-rename path with the restore is used as before, and a
`PermissionDenied` from either still goes to the privileged fallback.

`libc` is added for the macOS target only; it was already in the lockfile
through tauri. One macOS test checks the exchange and skips on a file system
that reports `ENOTSUP`.

* fix(updater): keep the previous app when it cannot be restored

If the restore rename also failed, the previous app was left only in the backup TempDir and deleted with it. Keep the backup in that case and return Error::PreviousAppNotRestored with its path.

* fix(updater): quote the paths in the privileged macOS install script

The paths went into single-quoted sh words inside an AppleScript string unescaped, so a quote in the app path broke the command (run as root) or panicked on the compile expect. Quote them for sh and AppleScript, and report script failures instead of panicking.

* fix(updater): fall back to moving the bundles on any swap error

Only ENOTSUP and EINVAL got the two-rename fallback, so volumes reporting EOPNOTSUPP or ENOSYS for RENAME_SWAP failed every update. Fall back on anything but a permission error, and have the swap test skip only off APFS.

* fix(updater): stage the macOS update on the app's volume

When the system temp dir was on another volume than the app, every update was refused. Create the temp dirs next to the app in that case, and compare the install path itself rather than what a symlink there points to.

* test(updater): update a macOS app on another volume than the temp dir

Runs the app from APFS and HFS+ disk images, covering the atomic swap and the two-rename fallback with the update staged next to the app.

* fix(updater): swap the bundles atomically in the privileged macOS install

The admin path ran two mv calls, leaving the install path empty between them. Run renamex_np with RENAME_SWAP as root through osascript's JavaScript bridge, and only fall back to the moves where the swap fails.

* add test

* cleanup

---------

Co-authored-by: Lucas Nogueira <lucas@tauri.app>

plugins/updater/src/error.rs

@@ -65,9 +65,14 @@ pub enum Error {
     /// `zip` errors.
     #[error(transparent)]
     Extract(#[from] zip::result::ZipError),
-    /// Temp dir is not on same mount mount. This prevents our updater to rename the AppImage to a temp file.
-    #[error("temp directory is not on the same mount point as the AppImage")]
+    /// No temp dir could be created on the same mount point as the app, which the updater needs
+    /// to move the app aside and the update into place with renames.
+    #[error("temp directory is not on the same mount point as the app")]
     TempDirNotOnSameMountPoint,
+    /// Installing the update failed, and the app it was replacing could not be moved back to its
+    /// install path either. The previous app was kept at the given path instead.
+    #[error("failed to install the update and to restore the previous app, which was left at {0}")]
+    PreviousAppNotRestored(std::path::PathBuf),
     /// The downloaded archive does not contain a binary for the current target.
     #[error("binary for the current target not found in the archive")]
     BinaryNotFoundInArchive,

plugins/updater/src/updater.rs

@@ -1389,18 +1389,33 @@ impl Update {
     /// └── ...
     fn install_inner(&self, bytes: &[u8]) -> Result<()> {
         use flate2::read::GzDecoder;
+        use std::os::unix::fs::{MetadataExt, PermissionsExt};
 
         let cursor = Cursor::new(bytes);
         let mut extracted_files: Vec<PathBuf> = Vec::new();
 
-        // Create temp directories for backup and extraction
+        // The app is moved with renames, which only work within one file system, so the
+        // temp dirs for backup and extraction go on the app's volume: in the system temp
+        // dir when it is there, and next to the app otherwise. If neither is, refuse now
+        // rather than fail after the current app has been moved away. The renames act on
+        // the install path itself, so a symlink there is not followed.
+        let app_dev = std::fs::symlink_metadata(&self.extract_path)?.dev();
+        let tmp_root = [
+            Some(std::env::temp_dir()),
+            self.extract_path.parent().map(Path::to_path_buf),
+        ]
+        .into_iter()
+        .flatten()
+        .find(|dir| dir.metadata().is_ok_and(|m| m.dev() == app_dev))
+        .ok_or(Error::TempDirNotOnSameMountPoint)?;
+
         let tmp_backup_dir = tempfile::Builder::new()
             .prefix("tauri_current_app")
-            .tempdir()?;
+            .tempdir_in(&tmp_root)?;
 
         let tmp_extract_dir = tempfile::Builder::new()
             .prefix("tauri_updated_app")
-            .tempdir()?;
+            .tempdir_in(&tmp_root)?;
 
         let decoder = GzDecoder::new(cursor);
         let mut archive = tar::Archive::new(decoder);
@@ -1424,55 +1439,92 @@ impl Update {
             extracted_files.push(extraction_path);
         }
 
-        // Try to move the current app to backup
-        let move_result = std::fs::rename(
-            &self.extract_path,
-            tmp_backup_dir.path().join("current_app"),
-        );
-        let need_authorization = if let Err(err) = move_result {
-            if err.kind() == std::io::ErrorKind::PermissionDenied {
-                true
-            } else {
+        // The temp dir becomes the installed bundle's root, and tempfile creates it
+        // with mode 0700, which stops every other user of the machine from launching
+        // the updated app.
+        std::fs::set_permissions(
+            tmp_extract_dir.path(),
+            std::fs::Permissions::from_mode(0o755),
+        )?;
+
+        // Exchange the current app and the new one in a single step where the file
+        // system supports it, so the install path is never empty; the previous app
+        // then sits in the extraction temp dir, which is removed on drop. Where the
+        // swap is not supported, fall back to two renames with a restore on failure.
+        // File systems report that with different errors (`ENOTSUP` on HFS+,
+        // `EOPNOTSUPP`, `ENOSYS` or `EINVAL` elsewhere), so anything but a permission
+        // error, which needs the privileged install instead, gets the fallback.
+        let backup = tmp_backup_dir.path().join("current_app");
+        let moved = match swap_bundle(&self.extract_path, tmp_extract_dir.path()) {
+            Err(err) if err.kind() != std::io::ErrorKind::PermissionDenied => {
+                log::debug!("cannot swap the app bundles ({err}), moving them instead");
+                replace_bundle(&self.extract_path, tmp_extract_dir.path(), &backup)
+            }
+            other => other,
+        };
+        let need_authorization = match moved {
+            Ok(()) => false,
+            Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => true,
+            Err(err) => {
+                if let Some(kept) = keep_backup_if_not_restored(
+                    &self.extract_path,
+                    tmp_backup_dir,
+                    &backup,
+                    std::env::home_dir().as_deref(),
+                ) {
+                    log::error!("failed to install the update: {err}");
+                    return Err(Error::PreviousAppNotRestored(kept));
+                }
                 std::fs::remove_dir_all(tmp_extract_dir.path()).ok();
                 return Err(err.into());
             }
-        } else {
-            false
         };
 
         if need_authorization {
             log::debug!("app installation needs admin privileges");
-            // Use AppleScript to perform moves with admin privileges
+            // Use AppleScript to swap the bundles, or move them where the file system
+            // cannot swap, with admin privileges: the current app is only deleted once
+            // the new one is in place.
             let apple_script = format!(
-                "do shell script \"rm -rf '{src}' && mv -f '{new}' '{src}'\" with administrator privileges",
-                src = self.extract_path.display(),
-                new = tmp_extract_dir.path().display()
+                "do shell script {} with administrator privileges",
+                applescript_string(&privileged_install_command(
+                    &self.extract_path,
+                    tmp_extract_dir.path(),
+                    &backup
+                ))
             );
 
             let (tx, rx) = std::sync::mpsc::channel();
             let res = (self.context.run_on_main_thread)(Box::new(move || {
                 let mut script =
                     osakit::Script::new_from_source(osakit::Language::AppleScript, &apple_script);
-                script.compile().expect("invalid AppleScript");
-                let r = script.execute();
-                tx.send(r).unwrap();
+                let result = match script.compile() {
+                    Ok(()) => script.execute().map(|_| ()).map_err(|e| e.to_string()),
+                    Err(e) => Err(e.to_string()),
+                };
+                let _ = tx.send(result);
             }));
-            let result = rx.recv().unwrap();
+            // `recv` only fails when the closure never ran, which `res` then reports
+            let result = res
+                .map_err(|e| e.to_string())
+                .and_then(|()| rx.recv().map_err(|e| e.to_string())?);
 
-            if res.is_err() || result.is_err() {
+            if let Err(err) = result {
+                log::error!("failed to move the new app into place: {err}");
+                if let Some(kept) = keep_backup_if_not_restored(
+                    &self.extract_path,
+                    tmp_backup_dir,
+                    &backup,
+                    std::env::home_dir().as_deref(),
+                ) {
+                    return Err(Error::PreviousAppNotRestored(kept));
+                }
                 std::fs::remove_dir_all(tmp_extract_dir.path()).ok();
                 return Err(Error::Io(std::io::Error::new(
                     std::io::ErrorKind::PermissionDenied,
                     "Failed to move the new app into place",
                 )));
             }
-        } else {
-            // Remove existing directory if it exists
-            if self.extract_path.exists() {
-                std::fs::remove_dir_all(&self.extract_path)?;
-            }
-            // Move the new app to the target path
-            std::fs::rename(tmp_extract_dir.path(), &self.extract_path)?;
         }
 
         let _ = std::process::Command::new("touch")
@@ -1483,6 +1535,117 @@ impl Update {
     }
 }
 
+/// Exchange `target` and `staged` atomically, so there is no instant at which
+/// `target` does not exist. APFS supports the swap; other file systems return an
+/// error, and the caller falls back to [`replace_bundle`].
+#[cfg(target_os = "macos")]
+fn swap_bundle(target: &Path, staged: &Path) -> std::io::Result<()> {
+    use std::ffi::CString;
+    use std::os::unix::ffi::OsStrExt;
+
+    let from = CString::new(staged.as_os_str().as_bytes())?;
+    let to = CString::new(target.as_os_str().as_bytes())?;
+    // SAFETY: both pointers are valid NUL-terminated strings that outlive the call,
+    // and `renamex_np` only reads them.
+    let result = unsafe { libc::renamex_np(from.as_ptr(), to.as_ptr(), libc::RENAME_SWAP) };
+    if result == 0 {
+        Ok(())
+    } else {
+        Err(std::io::Error::last_os_error())
+    }
+}
+
+/// Move `staged` into `target`, keeping what was at `target` in `backup` until the
+/// move has succeeded. If the second rename fails, the first is undone so `target`
+/// is left exactly as it was. Both renames must be within one file system.
+#[cfg(any(target_os = "macos", test))]
+fn replace_bundle(target: &Path, staged: &Path, backup: &Path) -> std::io::Result<()> {
+    std::fs::rename(target, backup)?;
+    if let Err(err) = std::fs::rename(staged, target) {
+        // Best effort: the error worth reporting is the one from the failed move.
+        let _ = std::fs::rename(backup, target);
+        return Err(err);
+    }
+    Ok(())
+}
+
+/// Exchanges the two paths it is given like [`swap_bundle`], for the privileged install:
+/// the shell has no command for the swap, but `osascript` running this JavaScript for
+/// Automation can call `renamex_np` (`2` is `RENAME_SWAP`), and ships with every macOS.
+/// It prints `swapped` or `failed`, so the caller can tell a swap that did not happen
+/// from an `osascript` failure that leaves unknown whether it did.
+#[cfg(target_os = "macos")]
+const SWAP_BUNDLE_JXA: &str = r#"function run(argv) {
+  ObjC.import("stdio");
+  return $.renamex_np(argv[0], argv[1], 2) === 0 ? "swapped" : "failed";
+}"#;
+
+/// The shell command the privileged install runs: the same swap as [`swap_bundle`], and
+/// where the file system cannot swap, the same moves as [`replace_bundle`], deleting the
+/// previous app only once the new one is in place. The moves only run once the swap is
+/// known not to have happened, since after a swap they would put the previous app back.
+#[cfg(target_os = "macos")]
+fn privileged_install_command(target: &Path, staged: &Path, backup: &Path) -> String {
+    let swap = sh_quote(Path::new(SWAP_BUNDLE_JXA));
+    let target = sh_quote(target);
+    let staged = sh_quote(staged);
+    let backup = sh_quote(backup);
+    format!(
+        "case \"$(/usr/bin/osascript -l JavaScript -e {swap} {staged} {target} 2>/dev/null)\" in swapped) rm -rf {staged};; failed) mv -f {target} {backup} && {{ mv -f {staged} {target} || {{ mv -f {backup} {target}; exit 1; }}; }} && rm -rf {backup};; *) exit 1;; esac"
+    )
+}
+
+/// Quotes `path` as a single `sh` word, whatever characters it holds.
+#[cfg(target_os = "macos")]
+fn sh_quote(path: &Path) -> String {
+    format!("'{}'", path.to_string_lossy().replace('\'', r"'\''"))
+}
+
+/// Quotes `s` as an AppleScript string literal.
+#[cfg(target_os = "macos")]
+fn applescript_string(s: &str) -> String {
+    format!("\"{}\"", s.replace('\\', r"\\").replace('"', "\\\""))
+}
+
+/// After a failed install, returns where the previous app is if it never made it back to
+/// `target`. `backup` is then the only copy left, and usually sits in the system temp dir,
+/// which macOS empties on its own, so it is moved next to `target`, or to `fallback_dir`
+/// (the home folder) when that is not writable, as `<name> (previous version).app`. If
+/// neither works, `backup_dir` is kept rather than deleted on drop as it otherwise is.
+#[cfg(any(target_os = "macos", test))]
+fn keep_backup_if_not_restored(
+    target: &Path,
+    backup_dir: tempfile::TempDir,
+    backup: &Path,
+    fallback_dir: Option<&Path>,
+) -> Option<PathBuf> {
+    if std::fs::symlink_metadata(target).is_ok() || std::fs::symlink_metadata(backup).is_err() {
+        return None;
+    }
+
+    let mut name = target.file_stem().unwrap_or_default().to_os_string();
+    name.push(" (previous version)");
+    if let Some(extension) = target.extension() {
+        name.push(".");
+        name.push(extension);
+    }
+    let kept = [target.parent(), fallback_dir]
+        .into_iter()
+        .flatten()
+        .map(|dir| dir.join(&name))
+        // `rename` replaces an empty directory, so never move onto anything already there
+        .filter(|kept| std::fs::symlink_metadata(kept).is_err())
+        .find(|kept| std::fs::rename(backup, kept).is_ok());
+
+    match kept {
+        Some(kept) => Some(kept),
+        None => {
+            let _ = backup_dir.keep();
+            Some(backup.to_path_buf())
+        }
+    }
+}
+
 /// Gets the base target string used by the updater. If bundle type is available it
 /// will be added to this string when selecting the download URL and signature.
 /// `tauri::utils::platform::bundle_type` method is used to obtain current bundle type.
@@ -1864,6 +2027,279 @@ mod tests {
         assert!(verify_signed_version(comment, "2024-01-02", true).is_err());
     }
 
+    #[test]
+    fn replace_bundle_moves_the_staged_bundle_into_place() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        let staged = dir.path().join("staged");
+        let backup = dir.path().join("backup");
+        std::fs::create_dir(&target).unwrap();
+        std::fs::write(target.join("version"), "old").unwrap();
+        std::fs::create_dir(&staged).unwrap();
+        std::fs::write(staged.join("version"), "new").unwrap();
+
+        super::replace_bundle(&target, &staged, &backup).unwrap();
+
+        assert_eq!(
+            std::fs::read_to_string(target.join("version")).unwrap(),
+            "new"
+        );
+        assert_eq!(
+            std::fs::read_to_string(backup.join("version")).unwrap(),
+            "old"
+        );
+        assert!(!staged.exists());
+    }
+
+    #[cfg(target_os = "macos")]
+    fn is_apfs(path: &std::path::Path) -> bool {
+        use std::os::unix::ffi::OsStrExt;
+
+        let path = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap();
+        let mut stat: libc::statfs = unsafe { std::mem::zeroed() };
+        // SAFETY: `path` is NUL-terminated and `stat` is a valid `statfs` to write to.
+        assert_eq!(unsafe { libc::statfs(path.as_ptr(), &mut stat) }, 0);
+        // SAFETY: `f_fstypename` is a NUL-terminated string filled in by `statfs`.
+        let name = unsafe { std::ffi::CStr::from_ptr(stat.f_fstypename.as_ptr()) };
+        name.to_bytes() == b"apfs"
+    }
+
+    #[test]
+    #[cfg(target_os = "macos")]
+    fn swap_bundle_exchanges_the_two_bundles_in_place() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        let staged = dir.path().join("staged");
+        std::fs::create_dir(&target).unwrap();
+        std::fs::write(target.join("version"), "old").unwrap();
+        std::fs::create_dir(&staged).unwrap();
+        std::fs::write(staged.join("version"), "new").unwrap();
+
+        // A temp dir on a file system without swap support proves nothing here, but on
+        // APFS the swap has to work: the install would otherwise always fall back.
+        if !is_apfs(dir.path()) {
+            return;
+        }
+        super::swap_bundle(&target, &staged).unwrap();
+
+        assert_eq!(
+            std::fs::read_to_string(target.join("version")).unwrap(),
+            "new"
+        );
+        assert_eq!(
+            std::fs::read_to_string(staged.join("version")).unwrap(),
+            "old"
+        );
+    }
+
+    #[test]
+    fn replace_bundle_restores_the_current_bundle_when_the_move_fails() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        let backup = dir.path().join("backup");
+        std::fs::create_dir(&target).unwrap();
+        std::fs::write(target.join("version"), "old").unwrap();
+        let missing = dir.path().join("missing");
+
+        let err = super::replace_bundle(&target, &missing, &backup).unwrap_err();
+
+        assert_eq!(err.kind(), std::io::ErrorKind::NotFound);
+        assert_eq!(
+            std::fs::read_to_string(target.join("version")).unwrap(),
+            "old"
+        );
+        assert!(!backup.exists());
+    }
+
+    /// A directory whose name `sh` and AppleScript would both misread unquoted.
+    #[cfg(target_os = "macos")]
+    fn hostile_dir() -> tempfile::TempDir {
+        tempfile::Builder::new()
+            .prefix("it's \"a\" \\ $(touch pwned) `dir`\n")
+            .tempdir()
+            .unwrap()
+    }
+
+    #[cfg(target_os = "macos")]
+    fn assert_privileged_install_command_moved(dir: &std::path::Path) {
+        let target = dir.join("Bob's \"App\".app");
+        assert_eq!(
+            std::fs::read_to_string(target.join("version")).unwrap(),
+            "new"
+        );
+        assert!(!dir.join("staged").exists());
+        assert!(!dir.join("backup").exists());
+        assert!(!dir.join("pwned").exists());
+    }
+
+    #[cfg(target_os = "macos")]
+    fn stage_privileged_install(dir: &std::path::Path) -> String {
+        let target = dir.join("Bob's \"App\".app");
+        let staged = dir.join("staged");
+        std::fs::create_dir(&target).unwrap();
+        std::fs::write(target.join("version"), "old").unwrap();
+        std::fs::create_dir(&staged).unwrap();
+        std::fs::write(staged.join("version"), "new").unwrap();
+        super::privileged_install_command(&target, &staged, &dir.join("backup"))
+    }
+
+    #[test]
+    #[cfg(target_os = "macos")]
+    fn privileged_install_command_quotes_every_path() {
+        let dir = hostile_dir();
+        let command = stage_privileged_install(dir.path());
+
+        let status = std::process::Command::new("sh")
+            .arg("-c")
+            .arg(&command)
+            .current_dir(dir.path())
+            .status()
+            .unwrap();
+
+        assert!(status.success());
+        assert_privileged_install_command_moved(dir.path());
+    }
+
+    /// The command the install runs, wrapped the way it is handed to AppleScript, only without
+    /// `with administrator privileges` so it runs without a prompt.
+    #[test]
+    #[cfg(target_os = "macos")]
+    fn privileged_install_script_quotes_every_path() {
+        let dir = hostile_dir();
+        let command = stage_privileged_install(dir.path());
+
+        let output = std::process::Command::new("osascript")
+            .arg("-e")
+            .arg(format!(
+                "do shell script {}",
+                super::applescript_string(&command)
+            ))
+            .current_dir(dir.path())
+            .output()
+            .unwrap();
+
+        assert!(
+            output.status.success(),
+            "{}",
+            String::from_utf8_lossy(&output.stderr)
+        );
+        assert_privileged_install_command_moved(dir.path());
+    }
+
+    /// The privileged install swaps the bundles where the file system can, so on APFS it
+    /// never touches the backup: one in a directory that does not exist must not matter.
+    #[test]
+    #[cfg(target_os = "macos")]
+    fn privileged_install_command_swaps_the_bundles() {
+        let dir = tempfile::tempdir().unwrap();
+        if !is_apfs(dir.path()) {
+            return;
+        }
+        let target = dir.path().join("App.app");
+        let staged = dir.path().join("staged");
+        std::fs::create_dir(&target).unwrap();
+        std::fs::write(target.join("version"), "old").unwrap();
+        std::fs::create_dir(&staged).unwrap();
+        std::fs::write(staged.join("version"), "new").unwrap();
+        let backup = dir.path().join("missing").join("backup");
+
+        let status = std::process::Command::new("sh")
+            .arg("-c")
+            .arg(super::privileged_install_command(&target, &staged, &backup))
+            .status()
+            .unwrap();
+
+        assert!(status.success());
+        assert_eq!(
+            std::fs::read_to_string(target.join("version")).unwrap(),
+            "new"
+        );
+        assert!(!staged.exists());
+    }
+
+    #[test]
+    fn keeps_the_backup_when_the_previous_app_was_not_restored() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        let backup_dir = tempfile::tempdir_in(dir.path()).unwrap();
+        let backup_dir_path = backup_dir.path().to_path_buf();
+        let backup = backup_dir_path.join("current_app");
+        std::fs::create_dir(&backup).unwrap();
+        std::fs::write(backup.join("version"), "old").unwrap();
+
+        let kept = super::keep_backup_if_not_restored(&target, backup_dir, &backup, None);
+
+        let expected = dir.path().join("App (previous version).app");
+        assert_eq!(kept.as_deref(), Some(expected.as_path()));
+        assert_eq!(
+            std::fs::read_to_string(expected.join("version")).unwrap(),
+            "old"
+        );
+        assert!(!backup_dir_path.exists());
+    }
+
+    #[test]
+    fn moves_the_backup_to_the_fallback_dir_when_it_cannot_go_next_to_the_app() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        std::fs::write(dir.path().join("App (previous version).app"), "other").unwrap();
+        let backup_dir = tempfile::tempdir_in(dir.path()).unwrap();
+        let backup = backup_dir.path().join("current_app");
+        std::fs::create_dir(&backup).unwrap();
+        std::fs::write(backup.join("version"), "old").unwrap();
+        let fallback = tempfile::tempdir_in(dir.path()).unwrap();
+
+        let kept =
+            super::keep_backup_if_not_restored(&target, backup_dir, &backup, Some(fallback.path()));
+
+        let expected = fallback.path().join("App (previous version).app");
+        assert_eq!(kept.as_deref(), Some(expected.as_path()));
+        assert_eq!(
+            std::fs::read_to_string(expected.join("version")).unwrap(),
+            "old"
+        );
+    }
+
+    #[test]
+    fn keeps_the_backup_in_place_when_it_cannot_be_moved() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        // A file where the backup would go next to the app, which must not be replaced
+        std::fs::write(dir.path().join("App (previous version).app"), "other").unwrap();
+        let backup_dir = tempfile::tempdir_in(dir.path()).unwrap();
+        let backup_dir_path = backup_dir.path().to_path_buf();
+        let backup = backup_dir_path.join("current_app");
+        std::fs::create_dir(&backup).unwrap();
+        std::fs::write(backup.join("version"), "old").unwrap();
+        let missing = dir.path().join("missing");
+
+        let kept = super::keep_backup_if_not_restored(&target, backup_dir, &backup, Some(&missing));
+
+        assert_eq!(kept.as_deref(), Some(backup.as_path()));
+        assert_eq!(
+            std::fs::read_to_string(backup.join("version")).unwrap(),
+            "old"
+        );
+        assert_eq!(
+            std::fs::read_to_string(dir.path().join("App (previous version).app")).unwrap(),
+            "other"
+        );
+    }
+
+    #[test]
+    fn drops_the_backup_when_the_previous_app_is_in_place() {
+        let dir = tempfile::tempdir().unwrap();
+        let target = dir.path().join("App.app");
+        std::fs::create_dir(&target).unwrap();
+        let backup_dir = tempfile::tempdir_in(dir.path()).unwrap();
+        let backup_dir_path = backup_dir.path().to_path_buf();
+        let backup = backup_dir_path.join("current_app");
+        std::fs::create_dir(&backup).unwrap();
+
+        assert!(super::keep_backup_if_not_restored(&target, backup_dir, &backup, None).is_none());
+        assert!(!backup_dir_path.exists());
+    }
+
     #[test]
     #[cfg(windows)]
     fn it_wraps_correctly() {