From df399edff8afedffac40a2115ba108f4466923e9 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 29 Oct 2026 15:26:20 +0000 Subject: [PATCH] craft-update: a Windows MSI copy updates itself in its own context, wherever it is installed (#419) * craft-update: a Windows MSI copy updates itself in its own context, wherever it is installed Self-update only recognised an MSI copy by the launcher's per-user folder (the HKCU InstallDir key, or %LOCALAPPDATA%\Programs\). Every app installed for everyone, in Program Files\ArtCraft\ or the older Program Files\, was treated as a loose folder and updated by unpacking the portable zip over Program Files without permission. - msi_context moves here from the launcher (craft-launcher crates/native/src/msi_context.rs at b827fed, unchanged apart from visibility, MsiScope and scope_arguments). Windows Installer's own registration decides a copy's context, for the launcher and the apps alike, so the launcher can drop its copy and both routes follow one rule. - New msi_copy module: a copy is an MSI copy when an App Paths entry names its exe (a registry read, cheap enough for status checks). Its update asks Windows Installer for the context; the App Paths hive is only the fallback when that lookup cannot run. - The update stops, before the app exits, when the copy is registered in both hives, the lookup is unclear, or Windows lists the app more than once. - After the update the helper starts the exe the registrations name now, since the package may have moved an exact old default folder. - A failed self-update now names the Windows Installer log, as the launcher's installs do. - The launcher's own per-user package is recognised and updated exactly as before. Co-Authored-By: Claude Opus 5.5 * craft-update: a copy only takes the Windows Installer context of its own hive Review follow-up (CodeRabbit on #419). The product code for the context lookup came from the app's only uninstall record, which was not proven to own the running exe. A package writes App Paths under HKMU, always the hive of its own context, so a registration whose Windows Installer context differs from the hive that named this exe belongs to another copy. The update now stops then, instead of taking the other copy's context. Also documents the remaining helpers the diff touches. Co-Authored-By: Claude Opus 5.5 * craft-update: an app in the old per-user folder is judged by its registration first Review follow-up (CodeRabbit on #419). The folder rule for the launcher's per-user package also matched an app installed in %LOCALAPPDATA%\Programs\, and it ran before the App Paths check, so such a copy skipped the product-count, conflict and context checks. Now an app is judged by its registration first; the folder rule only covers an app copy no registration names. The launcher in its own folder still keeps its old path whatever is registered: on a real PC it has an App Paths entry and a stale second uninstall record, and registration-first would have stopped its self-update. Co-Authored-By: Claude Opus 5.5 * craft-update: when the Windows Installer lookup cannot run, the uninstall records decide Review follow-up (CodeRabbit on #419). The fallback trusted the App Paths hive, so a stale entry or a product registered in both scopes could run the update in the wrong context. The fallback now follows the launcher's rule: the context every uninstall record of the product agrees on (msi_context::single_scope over HKLM in both views and HKCU). Missing or mixed records stop the update, and an agreed context that differs from the App Paths hive still stops it as another copy's registration. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/update/src/install.rs | 43 +++- crates/update/src/lib.rs | 2 + crates/update/src/msi_context.ps1 | 23 ++ crates/update/src/msi_context.rs | 288 +++++++++++++++++++++++++ crates/update/src/msi_copy.rs | 338 ++++++++++++++++++++++++++++++ crates/update/src/self_update.rs | 46 ++-- 6 files changed, 717 insertions(+), 23 deletions(-) create mode 100644 crates/update/src/msi_context.ps1 create mode 100644 crates/update/src/msi_context.rs create mode 100644 crates/update/src/msi_copy.rs diff --git a/crates/update/src/install.rs b/crates/update/src/install.rs index 09de4e0c..7c9e773e 100644 --- a/crates/update/src/install.rs +++ b/crates/update/src/install.rs @@ -1645,6 +1645,15 @@ fn msi_command(program: &Path, msi: &Path) -> Result Result { + let mut command = msi_command(program, msi)?; + command.args(context); + Ok(command) +} + /// Where `msi_command` asks Windows Installer to write its log. #[cfg_attr(not(windows), allow(dead_code))] fn msi_log(msi: &Path) -> PathBuf { @@ -1667,7 +1676,9 @@ pub(crate) fn msi_path(msi: &Path) -> Result { pub mod windows { use crate::protocol::CANCELLED; - use super::{App, INSTALLER, JobPhase, Path, PathBuf, Stop, cancelled, msi_command, msi_folder_argument, msi_log, process, turn}; + use super::{ + App, INSTALLER, JobPhase, Path, PathBuf, Stop, cancelled, msi_command, msi_folder_argument, msi_log, process, reinstall_command, turn, + }; fn msiexec() -> PathBuf { std::env::var_os("SystemRoot").map_or_else(|| PathBuf::from(r"C:\Windows"), PathBuf::from).join(r"System32\msiexec.exe") @@ -1700,11 +1711,15 @@ pub mod windows { }) } - /// Runs the launcher's own new MSI for a self-update, with the same progress window and no - /// restart as [`run_msi`], and waits for it. - pub fn reinstall(msi: &Path) -> Result<(), String> { - let status = msi_command(&msiexec(), msi)?.status().map_err(|e| format!("couldn't start the Windows Installer ({e})"))?; - explain(status.code()) + /// Runs an app's own new MSI for a self-update, with the same progress window and no restart + /// as [`run_msi`], and waits for it. `context` keeps the installed copy's Windows Installer + /// context. A failure names the installer's log, which is the only place 1603 is explained. + pub fn reinstall(msi: &Path, context: &[&str]) -> Result<(), String> { + let status = reinstall_command(&msiexec(), msi, context)?.status().map_err(|e| format!("couldn't start the Windows Installer ({e})"))?; + explain(status.code()).map_err(|e| match status.code() { + Some(1602) | None => e, + _ => format!("{e}. Its log is at {}", msi_log(msi).display()), + }) } /// Runs `work` once no other Windows Installer job of ours is running (a second one would @@ -1760,6 +1775,22 @@ mod tests { ); } + #[test] + fn a_self_update_keeps_the_installed_copys_context_after_the_package_options() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("GridCraft.msi"); + std::fs::write(&file, b"test package").unwrap(); + let resolved = msi_path(&file).unwrap(); + for context in [&["ALLUSERS=1"][..], &["ALLUSERS=2", "MSIINSTALLPERUSER=1"], &[]] { + let command = reinstall_command(Path::new("msiexec.exe"), &file, context).unwrap(); + let args: Vec<_> = command.get_args().map(|a| a.to_string_lossy().into_owned()).collect(); + let mut expected: Vec = ["/i", &resolved.to_string_lossy(), "/passive", "/norestart", "/l*v"].map(String::from).to_vec(); + expected.push(resolved.with_extension("log").to_string_lossy().into_owned()); + expected.extend(context.iter().map(|p| p.to_string())); + assert_eq!(args, expected); + } + } + #[test] fn an_unavailable_msi_is_refused_before_starting_the_installer() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/update/src/lib.rs b/crates/update/src/lib.rs index d0fec496..58bb0e94 100644 --- a/crates/update/src/lib.rs +++ b/crates/update/src/lib.rs @@ -10,6 +10,8 @@ mod folders; pub mod http; pub mod install; pub mod manifest; +pub mod msi_context; +mod msi_copy; pub mod packages; pub mod paths; pub mod platform; diff --git a/crates/update/src/msi_context.ps1 b/crates/update/src/msi_context.ps1 new file mode 100644 index 00000000..d0e0e0eb --- /dev/null +++ b/crates/update/src/msi_context.ps1 @@ -0,0 +1,23 @@ +# Read-only MSI registration, limited to the current account and machine contexts. +# $code is supplied by the caller only after strict braced-GUID validation. +$ErrorActionPreference = 'Stop' +$installer = $null +$products = $null +try { + $installer = New-Object -ComObject WindowsInstaller.Installer + $products = $installer.ProductsEx($code, '', 7) + foreach ($product in $products) { + try { + $context = $product.GetType().InvokeMember('Context', 'GetProperty', $null, $product, $null) + Write-Output ([int]$context) + } finally { + [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($product) + } + } +} finally { + foreach ($com in @($products, $installer)) { + if ($null -ne $com) { + [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($com) + } + } +} diff --git a/crates/update/src/msi_context.rs b/crates/update/src/msi_context.rs new file mode 100644 index 00000000..b51c2eed --- /dev/null +++ b/crates/update/src/msi_context.rs @@ -0,0 +1,288 @@ +//! The Windows Installer context of an installed copy, from Windows Installer's own registration, +//! never inferred from a display registry entry. An existing copy must never fall back to a fresh +//! installation when discovery fails. +//! +//! Shared by the launcher's installs and the apps' self-updates, so both keep a copy in the same +//! context by the same rule. + +#![cfg_attr(not(windows), allow(dead_code))] + +/// The Windows Installer context a copy is installed in. A per-machine copy elevates and writes +/// HKLM registrations; a per-user one must not silently become a second, per-machine copy (or the +/// other way round). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum MsiScope { + PerUser, + PerMachine, +} + +/// The `msiexec` properties that ask a dual-scope package for `scope`. `ALLUSERS=1` installs for +/// everyone (Windows asks for permission); `ALLUSERS=2` with `MSIINSTALLPERUSER=1` is the documented +/// single-package way to ask for a per-user install. +pub fn scope_arguments(scope: MsiScope) -> &'static [&'static str] { + match scope { + MsiScope::PerUser => &["ALLUSERS=2", "MSIINSTALLPERUSER=1"], + MsiScope::PerMachine => &["ALLUSERS=1"], + } +} + +const UNKNOWN_CONTEXT: &str = "Windows could not confirm who this app is installed for. No changes were made. Try the app's installer instead."; +const CONFLICTING_CONTEXT: &str = "This app is installed for both your account and everyone. Use its installer to choose which copy to update."; + +/// Why the Windows Installer lookup gave no context. +#[derive(Debug, PartialEq, Eq)] +pub enum ProbeError { + /// The lookup ran but gave no usable answer. The context is never guessed from that. + Unknown, + /// The same product is registered for this account and for everyone. + Conflicting, + /// The lookup could not run (a blocked shell, a timeout, a slow first start). Carries the real + /// error so it can be logged. + Unavailable(String), +} + +/// The context an update must keep. Windows Installer's own registration decides. Only when that +/// lookup cannot run at all does the context come from the uninstall record the copy was found by +/// (`record_scope`): the machine's records are per-machine and the account's own are per-user. A +/// lookup that ran and gave no answer, or a copy registered twice, still stops the update. +pub fn update_scope( + is_msi: bool, + product_code: Option<&str>, + probe: impl FnOnce(&str) -> Result, + record_scope: impl FnOnce(&str) -> Option, +) -> Result, String> { + if !is_msi { + return Ok(None); + } + let code = product_code.filter(|code| valid_product_code(code)).ok_or(UNKNOWN_CONTEXT)?; + match probe(code) { + Ok(scope) => Ok(Some(scope)), + Err(ProbeError::Conflicting) => Err(CONFLICTING_CONTEXT.into()), + Err(ProbeError::Unknown) => Err(UNKNOWN_CONTEXT.into()), + Err(ProbeError::Unavailable(reason)) => { + log::warn!("Windows Installer context lookup for {code} could not run: {reason}"); + let scope = record_scope(code).ok_or(UNKNOWN_CONTEXT)?; + log::info!("{code}: keeping the {scope:?} context of its uninstall record"); + Ok(Some(scope)) + } + } +} + +/// The one context every uninstall record of a product agrees on. Records that disagree (the copy is +/// registered for this account and for everyone) or no records at all give `None`, so the update stops. +pub fn single_scope(scopes: impl IntoIterator) -> Option { + let mut scopes = scopes.into_iter(); + let first = scopes.next()?; + scopes.all(|scope| scope == first).then_some(first) +} + +/// The context to install in. An update keeps `updated`, the context of the copy it replaces. A fresh +/// install (no copy at all) is machine-wide, so every app lands in `C:\Program Files\ArtCraft\` +/// (or the folder the person chose) and not in one account's profile; a dual-scope package would +/// otherwise take its own default, which is per-user. Windows asks for permission once, for that +/// install. Anything else (a copy that is not an MSI) takes the package's own default. +pub fn install_scope(updated: Option, fresh: bool) -> Option { + updated.or_else(|| fresh.then_some(MsiScope::PerMachine)) +} + +/// Whether `code` is a braced Windows Installer product GUID, the only text ever placed in the +/// lookup script. +pub fn valid_product_code(code: &str) -> bool { + code.len() == 38 + && code.bytes().enumerate().all(|(index, byte)| match index { + 0 => byte == b'{', + 37 => byte == b'}', + 9 | 14 | 19 | 24 => byte == b'-', + _ => byte.is_ascii_hexdigit(), + }) +} + +/// The one context in the lookup's output (one `MSIINSTALLCONTEXT` number per line): 1 and 2 are +/// an account (managed or not), 4 the machine. Anything else is unclear, and two contexts conflict. +fn parse_contexts(output: &str) -> Result { + let mut scope = None; + for line in output.lines().map(str::trim).filter(|line| !line.is_empty()) { + let context = match line { + "1" | "2" => MsiScope::PerUser, + "4" => MsiScope::PerMachine, + _ => return Err(ProbeError::Unknown), + }; + if scope.is_some_and(|previous| previous != context) { + return Err(ProbeError::Conflicting); + } + scope = Some(context); + } + scope.ok_or(ProbeError::Unknown) +} + +/// Asks Windows Installer, read-only, which context `product_code` is installed in, for this +/// account and the machine. Runs PowerShell with a 10 second limit. +#[cfg(windows)] +pub fn probe(product_code: &str) -> Result { + use std::path::PathBuf; + use std::time::Duration; + + if !valid_product_code(product_code) { + return Err(ProbeError::Unknown); + } + let root = std::env::var_os("SystemRoot") + .map(PathBuf::from) + .filter(|root| root.is_absolute()) + .ok_or_else(|| ProbeError::Unavailable("SystemRoot is not set to an absolute path".into()))?; + let powershell = root.join(r"System32\WindowsPowerShell\v1.0\powershell.exe"); + // Only a validated GUID is interpolated. ProductsEx reads registration for the current + // account and the machine, never another account and never Win32_Product (which repairs). + let script = format!("$code='{product_code}';\n{}", include_str!("msi_context.ps1")); + let mut command = crate::process::command(powershell); + command.args(["-NoProfile", "-NonInteractive", "-Command", &script]); + let output = crate::process::run_command(command, "Windows installation lookup", Duration::from_secs(10)).map_err(ProbeError::Unavailable)?; + parse_contexts(&output) +} + +#[cfg(test)] +mod tests { + use super::*; + + const CODE: &str = "{12345678-1234-1234-1234-123456789ABC}"; + + #[test] + fn authoritative_machine_context_is_preserved() { + assert_eq!(parse_contexts("4\r\n"), Ok(MsiScope::PerMachine)); + } + + #[test] + fn managed_and_unmanaged_accounts_are_both_per_user() { + assert_eq!(parse_contexts("1\n"), Ok(MsiScope::PerUser)); + assert_eq!(parse_contexts("2\n"), Ok(MsiScope::PerUser)); + assert_eq!(parse_contexts("1\n2\n"), Ok(MsiScope::PerUser)); + } + + #[test] + fn missing_unknown_or_conflicting_contexts_are_not_guessed() { + for output in ["", " \r\n", "0", "7", "user", "2\n4", "4\n1", "4\nnoise"] { + assert!(parse_contexts(output).is_err(), "{output:?}"); + } + assert_eq!(parse_contexts("4\n1"), Err(ProbeError::Conflicting)); + assert_eq!(parse_contexts("7"), Err(ProbeError::Unknown)); + } + + #[test] + fn a_fresh_install_is_not_looked_up() { + assert_eq!(update_scope(false, None, |_| Err(ProbeError::Unknown), |_| None), Ok(None)); + } + + #[test] + fn known_updates_keep_the_authoritative_context() { + for scope in [MsiScope::PerUser, MsiScope::PerMachine] { + assert_eq!(update_scope(true, Some(CODE), |_| Ok(scope), |_| None), Ok(Some(scope))); + } + } + + #[test] + fn an_update_without_its_product_code_stops_before_probing() { + let called = std::cell::Cell::new(false); + assert!( + update_scope( + true, + None, + |_| { + called.set(true); + Ok(MsiScope::PerMachine) + }, + |_| None + ) + .is_err() + ); + assert!(!called.get()); + } + + #[test] + fn malformed_product_codes_stop_before_any_shell_call() { + for code in ["", "not-a-guid", "{12345678-1234-1234-1234-123456789ABZ}", "';exit 0;#", "{12345678-1234-1234-1234-123456789ABC}\n"] { + let called = std::cell::Cell::new(false); + assert!( + update_scope( + true, + Some(code), + |_| { + called.set(true); + Ok(MsiScope::PerMachine) + }, + |_| None + ) + .is_err() + ); + assert!(!called.get(), "{code:?}"); + } + } + + #[test] + fn unavailable_context_never_becomes_a_fresh_install() { + let unavailable = || Err(ProbeError::Unavailable("query failed".into())); + assert!(update_scope(true, Some(CODE), |_| unavailable(), |_| None).is_err()); + } + + #[test] + fn a_lookup_that_cannot_run_falls_back_to_the_uninstall_record() { + for scope in [MsiScope::PerUser, MsiScope::PerMachine] { + let seen = std::cell::RefCell::new(String::new()); + let result = update_scope( + true, + Some(CODE), + |_| Err(ProbeError::Unavailable("PowerShell is blocked".into())), + |code| { + *seen.borrow_mut() = code.to_owned(); + Some(scope) + }, + ); + assert_eq!(result, Ok(Some(scope))); + assert_eq!(*seen.borrow(), CODE); + } + } + + #[test] + fn an_answer_that_is_unclear_or_doubled_never_uses_the_uninstall_record() { + for failure in [ProbeError::Unknown, ProbeError::Conflicting] { + let result = update_scope(true, Some(CODE), |_| Err(failure), |_| panic!("must not consult the record")); + assert!(result.is_err()); + } + } + + #[test] + fn a_blocked_lookup_without_a_record_stops_the_update() { + let result = update_scope(true, Some(CODE), |_| Err(ProbeError::Unavailable("timed out".into())), |_| None); + assert_eq!(result, Err(UNKNOWN_CONTEXT.to_string())); + } + + #[test] + fn records_in_two_contexts_give_no_fallback_scope() { + assert_eq!(single_scope([]), None); + assert_eq!(single_scope([MsiScope::PerUser]), Some(MsiScope::PerUser)); + assert_eq!(single_scope([MsiScope::PerMachine, MsiScope::PerMachine]), Some(MsiScope::PerMachine)); + assert_eq!(single_scope([MsiScope::PerMachine, MsiScope::PerUser]), None); + assert_eq!(single_scope([MsiScope::PerUser, MsiScope::PerMachine, MsiScope::PerMachine]), None); + } + + #[test] + fn each_context_asks_the_package_for_itself() { + assert_eq!(scope_arguments(MsiScope::PerMachine), ["ALLUSERS=1"]); + assert_eq!(scope_arguments(MsiScope::PerUser), ["ALLUSERS=2", "MSIINSTALLPERUSER=1"]); + } + + #[test] + fn a_fresh_install_is_machine_wide() { + assert_eq!(install_scope(None, true), Some(MsiScope::PerMachine)); + } + + #[test] + fn an_update_keeps_the_context_of_the_copy_it_replaces() { + for scope in [MsiScope::PerUser, MsiScope::PerMachine] { + assert_eq!(install_scope(Some(scope), false), Some(scope)); + } + } + + #[test] + fn a_copy_that_is_not_an_msi_takes_the_package_default() { + assert_eq!(install_scope(None, false), None); + } +} diff --git a/crates/update/src/msi_copy.rs b/crates/update/src/msi_copy.rs new file mode 100644 index 00000000..05e1ca3f --- /dev/null +++ b/crates/update/src/msi_copy.rs @@ -0,0 +1,338 @@ +//! Whether a running Windows copy was installed by its MSI, and the `msiexec` properties its +//! self-update needs to keep that copy's context. +//! +//! Every Craft App MSI registers its exe under `App Paths` (HKLM when installed for everyone, +//! HKCU for one account). A copy whose exe one of those entries names is an MSI copy, wherever its +//! folder is: `Program Files\ArtCraft\`, the older `Program Files\`, a folder the person +//! chose. Recognising it only reads the registry, so it is cheap enough for any status check. +//! +//! Which context to update in is a separate, slower question that only an update asks. Windows +//! Installer's own registration answers it ([`crate::msi_context`]), as it does for the launcher; +//! only when that lookup cannot run at all do the product's own uninstall records answer, and +//! only when they all agree. The answer must +//! match the hive that named this exe: a package writes `App Paths` under `HKMU`, which is always +//! the hive of its own context, so a registration in the other context belongs to another copy and +//! the update stops. So a copy installed for everyone is never unpacked over without permission, +//! and a per-user copy never gains a second, per-machine twin or takes another copy's context. +//! +//! The launcher's own per-user MSI is still recognised by the folder it records in HKCU (or its +//! default folder) and updated exactly as before, with the package's own default context. An app +//! copy in that kind of folder is judged by its registration first ([`classify`]). + +#![cfg_attr(not(windows), allow(dead_code))] + +use std::path::{Path, PathBuf}; + +use crate::msi_context::MsiScope; + +/// How an MSI copy was recognised. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum MsiCopy { + /// An `App Paths` entry in the hive of `hive` names this exe. + Registered { hive: MsiScope }, + /// Entries in both hives name it: the update stops rather than guess. + Conflicting, + /// The launcher's own per-user package, found by its recorded or default folder. + Legacy, +} + +const CONFLICTING: &str = + "this app is registered both for your account and for everyone, so it can't tell which copy to update. Use the app's installer instead."; +const ANOTHER_COPY: &str = "Windows lists this app for a different account setting than this copy, so it can't be sure which copy it would update. Use the app's installer instead."; +const NOT_ONE_PRODUCT: &str = + "Windows lists this app's installation more than once or not at all, so it can't tell which copy to update. Use the app's installer instead."; + +/// The `msiexec` properties for `copy`'s update. `product_codes` are the app's Windows Installer +/// registrations; `probe` asks Windows Installer for one product's context, and `record_scopes` +/// lists the context of every uninstall record of that product, read only when the lookup cannot +/// run. Exactly one product must be registered: with two, either could be the copy that runs. The +/// records must agree on one context. Whichever answers, it must be the context of the hive that +/// named this exe, or the registration is another copy's. +pub(crate) fn update_arguments( + copy: MsiCopy, + product_codes: &[String], + probe: impl FnOnce(&str) -> Result, + record_scopes: impl FnOnce(&str) -> Vec, +) -> Result<&'static [&'static str], String> { + let hive = match copy { + MsiCopy::Legacy => return Ok(&[]), + MsiCopy::Conflicting => return Err(CONFLICTING.into()), + MsiCopy::Registered { hive } => hive, + }; + let [code] = product_codes else { return Err(NOT_ONE_PRODUCT.into()) }; + let fallback = |code: &str| crate::msi_context::single_scope(record_scopes(code)); + let scope = crate::msi_context::update_scope(true, Some(code), probe, fallback)?.ok_or(NOT_ONE_PRODUCT)?; + if scope != hive { + return Err(ANOTHER_COPY.into()); + } + Ok(crate::msi_context::scope_arguments(scope)) +} + +/// How a copy was installed, from what is known about it. The launcher's own per-user package in +/// its recorded or default folder keeps its update exactly as before, whatever else is registered +/// (a stale second record must not stop it). An app is judged by its `App Paths` registration +/// first, so a registered copy always passes the context checks; the folder rule only covers an +/// app copy that no registration names. `registration` is only read when it can decide. +pub(crate) fn classify(is_launcher: bool, in_legacy_folder: bool, registration: impl FnOnce() -> Option) -> Option { + if is_launcher && in_legacy_folder { + return Some(MsiCopy::Legacy); + } + registration().or_else(|| in_legacy_folder.then_some(MsiCopy::Legacy)) +} + +/// Which registrations name `exe`. `registered` holds every `App Paths` target with the context +/// of the hive it came from; both sides are expected in one canonical form. +pub(crate) fn match_registration(exe: &Path, registered: &[(MsiScope, PathBuf)]) -> Option { + let mut hives = registered.iter().filter(|(_, target)| same_path(target, exe)).map(|(hive, _)| *hive); + let first = hives.next()?; + Some(if hives.all(|hive| hive == first) { MsiCopy::Registered { hive: first } } else { MsiCopy::Conflicting }) +} + +/// The one exe the app's registrations name after an update. The package may have moved an exact +/// old default folder, so the helper starts this rather than the path it was given. `None` unless +/// every registration names the same file. +pub(crate) fn registered_exe(registered: &[(MsiScope, PathBuf)]) -> Option { + let mut targets = registered.iter().map(|(_, target)| target); + let first = targets.next()?; + targets.all(|t| same_path(t, first)).then(|| first.clone()) +} + +/// Windows paths compare without regard to case. +fn same_path(a: &Path, b: &Path) -> bool { + a.as_os_str().to_string_lossy().eq_ignore_ascii_case(&b.as_os_str().to_string_lossy()) +} + +/// An `App Paths` default value as a path: Windows allows it in quotes. +pub(crate) fn app_path_value(value: &str) -> Option { + let value = value.trim().trim_matches('"').trim(); + (!value.is_empty()).then(|| PathBuf::from(value)) +} + +#[cfg(windows)] +pub(crate) mod windows { + use std::path::{Path, PathBuf}; + + use winreg::RegKey; + use winreg::enums::{HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE, KEY_READ, KEY_WOW64_32KEY, KEY_WOW64_64KEY}; + + use super::{MsiCopy, MsiScope, app_path_value, match_registration}; + use crate::app::App; + + /// `path` with links and `..` resolved in ordinary drive syntax, or as given when it cannot be + /// resolved, so registry values and the running exe compare in one form. + fn canonical(path: &Path) -> PathBuf { + dunce::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) + } + + /// Every `App Paths` target registered for one of `app`'s exe names, with the context of its + /// hive. Entries naming a missing file are left out. + pub(crate) fn registered_exes(app: &App) -> Vec<(MsiScope, PathBuf)> { + let hives = [ + (HKEY_LOCAL_MACHINE, KEY_WOW64_64KEY, MsiScope::PerMachine), + (HKEY_LOCAL_MACHINE, KEY_WOW64_32KEY, MsiScope::PerMachine), + (HKEY_CURRENT_USER, 0, MsiScope::PerUser), + ]; + let mut out: Vec<(MsiScope, PathBuf)> = Vec::new(); + for stem in app.stems() { + let key = format!(r"SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{stem}.exe"); + for (hive, flags, scope) in hives { + let Ok(entry) = RegKey::predef(hive).open_subkey_with_flags(&key, KEY_READ | flags) else { continue }; + let Some(target) = entry.get_value::("").ok().as_deref().and_then(app_path_value) else { continue }; + if target.is_file() { + let target = canonical(&target); + if !out.iter().any(|(s, t)| *s == scope && *t == target) { + out.push((scope, target)); + } + } + } + } + out + } + + /// How `exe` was installed by an MSI, or `None` when it wasn't. Reads the registry only. + pub(crate) fn find(app: &App, exe: &Path) -> Option { + let exe = canonical(exe); + let is_launcher = app.id == crate::suite::LAUNCHER.id; + super::classify(is_launcher, is_legacy(app, &exe), || match_registration(&exe, ®istered_exes(app))) + } + + /// The `msiexec` properties for `exe`'s update, asking Windows Installer for its context. + pub(crate) fn update_arguments(app: &App, exe: &Path) -> Result<&'static [&'static str], String> { + let copy = find(app, exe).ok_or_else(|| format!("{} isn't registered with the Windows Installer", app.name))?; + let codes: Vec = crate::detect::windows::msi_installs(app) + .into_iter() + .map(|m| m.product_code) + .filter(|code| crate::msi_context::valid_product_code(code)) + .collect(); + super::update_arguments(copy, &codes, crate::msi_context::probe, record_scopes) + } + + /// The context of every uninstall record of `code`: the machine's (both registry views) are + /// per-machine, the account's own per-user. Two views of one machine record count once. + fn record_scopes(code: &str) -> Vec { + const UNINSTALL: &str = r"SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall"; + let roots = [ + (HKEY_LOCAL_MACHINE, KEY_WOW64_64KEY, MsiScope::PerMachine), + (HKEY_LOCAL_MACHINE, KEY_WOW64_32KEY, MsiScope::PerMachine), + (HKEY_CURRENT_USER, 0, MsiScope::PerUser), + ]; + let mut scopes = Vec::new(); + for (hive, flags, scope) in roots { + let found = RegKey::predef(hive).open_subkey_with_flags(format!(r"{UNINSTALL}\{code}"), KEY_READ | flags).is_ok(); + if found && !scopes.contains(&scope) { + scopes.push(scope); + } + } + scopes + } + + /// The launcher's own per-user MSI: the folder its wizard records in HKCU, or its default + /// `%LOCALAPPDATA%\Programs\`. + fn is_legacy(app: &App, exe: &Path) -> bool { + let Some(dir) = exe.parent() else { return false }; + let recorded = recorded_dir(app).map(|d| canonical(&d)); + let default = crate::folders::data_local_dir().map(|d| canonical(&d.join("Programs").join(app.name))); + [recorded, default].into_iter().flatten().any(|d| d == dir) + } + + /// The install folder the launcher's per-user wizard records in HKCU, when it recorded one. + fn recorded_dir(app: &App) -> Option { + let key = RegKey::predef(HKEY_CURRENT_USER).open_subkey(format!(r"Software\Learning Machines LLC\{}", app.name)).ok()?; + let dir: String = key.get_value("InstallDir").ok()?; + let dir = dir.trim(); + (!dir.is_empty()).then(|| PathBuf::from(dir)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::msi_context::ProbeError; + + const CODE: &str = "{500F5254-26F4-4AAA-B90D-6F3FD9947D14}"; + + fn p(s: &str) -> PathBuf { + PathBuf::from(s) + } + + fn codes(list: &[&str]) -> Vec { + list.iter().map(|c| c.to_string()).collect() + } + + #[test] + fn a_registration_makes_an_msi_copy_wherever_its_folder_is() { + let registered = [(MsiScope::PerMachine, p(r"C:\Program Files\GridCraft\gridcraft.exe"))]; + let copy = Some(MsiCopy::Registered { hive: MsiScope::PerMachine }); + assert_eq!(match_registration(&p(r"C:\Program Files\GridCraft\gridcraft.exe"), ®istered), copy); + assert_eq!(match_registration(&p(r"c:\program files\gridcraft\GRIDCRAFT.EXE"), ®istered), copy); + let user = [(MsiScope::PerUser, p(r"D:\Chosen\GridCraft\gridcraft.exe"))]; + assert_eq!(match_registration(&p(r"D:\Chosen\GridCraft\gridcraft.exe"), &user), Some(MsiCopy::Registered { hive: MsiScope::PerUser })); + } + + #[test] + fn an_exe_no_registration_names_is_not_an_msi_copy() { + let registered = [(MsiScope::PerMachine, p(r"C:\Program Files\ArtCraft\GridCraft\gridcraft.exe"))]; + assert_eq!(match_registration(&p(r"D:\Tools\GridCraft\gridcraft.exe"), ®istered), None); + assert_eq!(match_registration(&p(r"D:\Tools\GridCraft\gridcraft.exe"), &[]), None); + } + + #[test] + fn a_copy_registered_in_both_hives_stops_before_any_lookup() { + let exe = p(r"C:\Program Files\ArtCraft\GridCraft\gridcraft.exe"); + let copy = match_registration(&exe, &[(MsiScope::PerMachine, exe.clone()), (MsiScope::PerUser, exe.clone())]).unwrap(); + assert_eq!(copy, MsiCopy::Conflicting); + assert!(update_arguments(copy, &codes(&[CODE]), |_| panic!("must not look up"), |_| panic!("must not read the records")).is_err()); + } + + #[test] + fn windows_installer_confirms_the_context_of_the_hive_that_named_the_exe() { + let copy = MsiCopy::Registered { hive: MsiScope::PerMachine }; + assert_eq!( + update_arguments(copy, &codes(&[CODE]), |_| Ok(MsiScope::PerMachine), |_| panic!("must not read the records")).unwrap(), + ["ALLUSERS=1"] + ); + let copy = MsiCopy::Registered { hive: MsiScope::PerUser }; + assert_eq!( + update_arguments(copy, &codes(&[CODE]), |_| Ok(MsiScope::PerUser), |_| panic!("must not read the records")).unwrap(), + ["ALLUSERS=2", "MSIINSTALLPERUSER=1"] + ); + } + + #[test] + fn a_registration_in_the_other_context_is_another_copys_and_stops_the_update() { + for (hive, other) in [(MsiScope::PerUser, MsiScope::PerMachine), (MsiScope::PerMachine, MsiScope::PerUser)] { + let error = + update_arguments(MsiCopy::Registered { hive }, &codes(&[CODE]), |_| Ok(other), |_| panic!("must not read the records")).unwrap_err(); + assert_eq!(error, ANOTHER_COPY); + } + } + + #[test] + fn the_uninstall_records_answer_only_when_the_lookup_cannot_run() { + let copy = MsiCopy::Registered { hive: MsiScope::PerMachine }; + let blocked = |_: &str| Err(ProbeError::Unavailable("PowerShell is blocked".into())); + assert_eq!(update_arguments(copy, &codes(&[CODE]), blocked, |_| vec![MsiScope::PerMachine]).unwrap(), ["ALLUSERS=1"]); + for unclear in [ProbeError::Unknown, ProbeError::Conflicting] { + assert!(update_arguments(copy, &codes(&[CODE]), |_| Err(unclear), |_| panic!("must not read the records")).is_err()); + } + } + + #[test] + fn missing_mixed_or_mismatched_records_stop_the_fallback() { + let copy = MsiCopy::Registered { hive: MsiScope::PerMachine }; + let blocked = |_: &str| Err(ProbeError::Unavailable("PowerShell is blocked".into())); + assert!(update_arguments(copy, &codes(&[CODE]), blocked, |_| vec![]).is_err()); + assert!(update_arguments(copy, &codes(&[CODE]), blocked, |_| vec![MsiScope::PerMachine, MsiScope::PerUser]).is_err()); + let error = update_arguments(copy, &codes(&[CODE]), blocked, |_| vec![MsiScope::PerUser]).unwrap_err(); + assert_eq!(error, ANOTHER_COPY); + } + + #[test] + fn the_update_stops_unless_exactly_one_product_is_registered() { + let copy = MsiCopy::Registered { hive: MsiScope::PerMachine }; + assert!(update_arguments(copy, &[], |_| panic!("must not look up"), |_| panic!("must not read the records")).is_err()); + let two = codes(&[CODE, "{0BE5923F-1AAB-4240-8174-2F0CCC05E14C}"]); + assert!(update_arguments(copy, &two, |_| panic!("must not look up"), |_| panic!("must not read the records")).is_err()); + } + + #[test] + fn the_launcher_in_its_own_folder_keeps_its_old_path_whatever_is_registered() { + let registered = Some(MsiCopy::Registered { hive: MsiScope::PerUser }); + assert_eq!(classify(true, true, || registered), Some(MsiCopy::Legacy)); + assert_eq!(classify(true, false, || registered), registered); + assert_eq!(classify(true, false, || None), None); + } + + #[test] + fn an_app_in_the_old_per_user_folder_is_judged_by_its_registration_first() { + for registered in [MsiCopy::Registered { hive: MsiScope::PerUser }, MsiCopy::Conflicting] { + assert_eq!(classify(false, true, || Some(registered)), Some(registered)); + } + assert_eq!(classify(false, true, || None), Some(MsiCopy::Legacy)); + assert_eq!(classify(false, false, || None), None); + } + + #[test] + fn the_launchers_own_package_keeps_its_default() { + assert!(update_arguments(MsiCopy::Legacy, &[], |_| panic!("must not look up"), |_| panic!("must not read the records")).unwrap().is_empty()); + } + + #[test] + fn the_helper_restarts_the_one_exe_the_registrations_name() { + let moved = p(r"C:\Program Files\ArtCraft\GridCraft\gridcraft.exe"); + assert_eq!(registered_exe(&[(MsiScope::PerMachine, moved.clone())]), Some(moved.clone())); + assert_eq!(registered_exe(&[(MsiScope::PerMachine, moved.clone()), (MsiScope::PerUser, moved.clone())]), Some(moved)); + assert_eq!(registered_exe(&[]), None); + let two = [(MsiScope::PerMachine, p(r"C:\a\gridcraft.exe")), (MsiScope::PerUser, p(r"C:\b\gridcraft.exe"))]; + assert_eq!(registered_exe(&two), None); + } + + #[test] + fn app_paths_values_may_be_quoted() { + assert_eq!( + app_path_value(r#""C:\Program Files\ArtCraft\GridCraft\gridcraft.exe""#), + Some(p(r"C:\Program Files\ArtCraft\GridCraft\gridcraft.exe")) + ); + assert_eq!(app_path_value(" "), None); + } +} diff --git a/crates/update/src/self_update.rs b/crates/update/src/self_update.rs index f22dfb35..a3b3bc4c 100644 --- a/crates/update/src/self_update.rs +++ b/crates/update/src/self_update.rs @@ -9,7 +9,7 @@ //! | Installed as | Swap | //! |---|---| //! | macOS `.app` | copy the new bundle from the DMG over this one (fine while running), reopen it | -//! | Windows MSI (per-user, any folder) | after exit, run the new MSI (it reinstalls into the same folder), then start the exe | +//! | Windows MSI (any folder, for one account or everyone) | after exit, run the new MSI in the copy's own context (everyone asks for permission), then start the exe it registers | //! | Windows portable zip | after exit, unzip over this folder (settings in `ArtCraftLauncherData` stay) | //! | Linux AppImage | rename the new AppImage over this one (fine while running), start it | //! | Linux `.deb` / `.rpm` | the system package manager, with the password prompt; then restart | @@ -182,18 +182,17 @@ fn detect(app: &App, exe: &Path) -> Option { None } +/// How the Windows copy at `exe` was installed: portable (a `portable.txt` beside it), by its MSI +/// (registered in either context, or the launcher's own per-user package), else a loose folder. #[cfg(windows)] fn detect(app: &App, exe: &Path) -> Option { let dir = exe.parent()?; if dir.join("portable.txt").is_file() { return Some(Install::Portable(exe.to_path_buf())); } - // The MSI installs per user, into the folder chosen in its wizard (which it records in the - // registry) or by default %LOCALAPPDATA%\Programs\. - let canonical = |p: PathBuf| p.canonicalize().unwrap_or(p); - let recorded = msi_install_dir(app).map(canonical); - let default = crate::folders::data_local_dir().map(|d| canonical(d.join("Programs").join(app.name))); - if [recorded, default].into_iter().flatten().any(|d| d == dir) { + // Any folder, either context: the MSI's own registration says so (see `msi_copy`). A copy + // installed for everyone must never be unpacked over as if it were a loose folder. + if crate::msi_copy::windows::find(app, exe).is_some() { return Some(Install::Msi(exe.to_path_buf())); } if exe.is_file() { @@ -202,16 +201,22 @@ fn detect(app: &App, exe: &Path) -> Option { None } -/// The folder the MSI installed into, as it records it. +/// The `msiexec` properties that keep `exe`'s Windows Installer context, or why its update stops. +/// Asked while the app still runs (so it can show the error) and again by the helper. #[cfg(windows)] -fn msi_install_dir(app: &App) -> Option { - use winreg::RegKey; - use winreg::enums::HKEY_CURRENT_USER; - let subkey = format!(r"Software\Learning Machines LLC\{}", app.name); - let key = RegKey::predef(HKEY_CURRENT_USER).open_subkey(&subkey).ok()?; - let dir: String = key.get_value("InstallDir").ok()?; - let dir = dir.trim(); - (!dir.is_empty()).then(|| PathBuf::from(dir)) +fn msi_arguments(app: &App, exe: &Path) -> Result<&'static [&'static str], String> { + crate::msi_copy::windows::update_arguments(app, exe) +} + +/// The exe to start after an MSI update of a registered copy: the one its registrations name now +/// (the package may have moved an exact old default folder), else `exe`. +#[cfg(windows)] +fn exe_after_msi(app: &App, registered: bool, exe: &Path) -> PathBuf { + registered + .then(|| crate::msi_copy::registered_exe(&crate::msi_copy::windows::registered_exes(app))) + .flatten() + .filter(|moved| is_own_executable(app, moved)) + .unwrap_or_else(|| exe.to_path_buf()) } #[cfg(target_os = "linux")] @@ -628,11 +633,15 @@ pub fn finish(app: &App, args: &[OsString]) -> Option> { record_failure(&paths, &e); return Some(Err(e)); } + // Read before the step: the update may move or replace the old exe. + let registered = + matches!(step, Finish::Msi(_)) && matches!(crate::msi_copy::windows::find(app, &exe), Some(crate::msi_copy::MsiCopy::Registered { .. })); let done = run_step(app, &me, pid, &exe, &step, std::time::Duration::from_secs(60)); if let Err(e) = &done { record_failure(&paths, e); } // The new app, or the old one when the step failed. + let exe = if done.is_ok() { exe_after_msi(app, registered, &exe) } else { exe }; let started = signed_like(app, &me, &exe) .and_then(|()| process::spawn_detached(std::process::Command::new(&exe)).map_err(|e| format!("couldn't start {} ({e})", exe.display()))); if let Err(e) = &started { @@ -688,6 +697,7 @@ fn windows_precheck(app: &App, install: &Install, paths: &Paths, download: &Path let me = std::env::current_exe().map_err(|e| format!("couldn't find this program ({e})"))?; match install { Install::Msi(exe) => signed_like(app, &me, exe) + .and_then(|()| msi_arguments(app, exe).map(drop)) .and_then(|()| signed_like(app, &me, download)) .and_then(|()| own_executable(app, exe)) .and_then(|()| same_cache_as_helper(app, paths, exe)), @@ -824,7 +834,9 @@ fn run_step(app: &App, reference: &Path, pid: u32, exe: &Path, step: &Finish, wa wait_watch::returned(pid, waited); let result = if waited { match step { - Finish::Msi(msi) => signed_like(app, reference, msi).and_then(|()| install::windows::reinstall(msi)), + Finish::Msi(msi) => signed_like(app, reference, msi) + .and_then(|()| msi_arguments(app, exe)) + .and_then(|arguments| install::windows::reinstall(msi, arguments)), Finish::Portable { zip, tmp } => match (exe.parent(), exe.file_name()) { (Some(dir), Some(executable)) => crate::portable::unpack_over(zip, tmp, dir, executable, &|file| { // The expected executable must be signed even when its contents aren't a PE file.