diff --git a/.github/scripts/windows-installer.iss b/.github/scripts/windows-installer.iss index 5aad047a..8c610b10 100644 --- a/.github/scripts/windows-installer.iss +++ b/.github/scripts/windows-installer.iss @@ -125,7 +125,12 @@ Name: "{autodesktop}\tty7"; Filename: "{app}\tty7-app.exe"; Tasks: desktopicon; ; reads those same keys to decide whether an existing registration still points ; at this install, and two hand-kept copies of the layout would drift. Runs ; before the launch entry below so a first start already sees the final state. -Filename: "{app}\tty7-app.exe"; Parameters: "--register-explorer-menu"; Tasks: explorermenu; Flags: runhidden waituntilterminated +; skipifsilent because a silent run *is* the elevated in-place update (#504): +; launching the app there would run it elevated, and under over-the-shoulder +; elevation the menu would land in the administrator's hive, not the user's. +; Skipping loses nothing: an upgrade keeps the install path, so the HKCU keys +; a ticked first install wrote still point at the right executable. +Filename: "{app}\tty7-app.exe"; Parameters: "--register-explorer-menu"; Tasks: explorermenu; Flags: runhidden waituntilterminated skipifsilent Filename: "{app}\tty7-app.exe"; Description: "{cm:LaunchProgram,tty7}"; Flags: nowait postinstall skipifsilent [UninstallRun] diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb2c907..14ed7ee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -630,6 +630,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **A Windows install for all users now updates itself** — previously an + in-place update was refused there (replacing files under `C:\Program Files` + takes administrator rights, and a silent installer launched unelevated would + either install a second copy per-user or raise a bare UAC prompt from a + temporary directory). The update now runs as one announced UAC prompt that + covers both privileged stages, and the app itself never runs elevated: a + helper running as the signed-in user waits the install out and relaunches + tty7 with the user's own token. The update dialog says the prompt is coming + before the app quits, and "Install on Next Launch" is not offered — nobody + would be there to answer. Declining the prompt is not an error: the staged + package simply waits in Settings. Everything the elevated half needs crosses + as command-line arguments (an elevated child does not inherit the + environment), the package's checksum travels from the release server in the + GUI's memory rather than from a file the download could have rewritten, and + the privileged stages always run the *installed* updater — a medium-integrity + process cannot swap the binary that gets elevated. The staged copy lives in + an administrator-only `%ProgramData%` directory while the chain runs (#504). + Installations updated by an updater that predates the chain still fall back + to the manual download page, so the first release carrying this updates the + way it always did; the one after it updates itself. + - **Every confirmation answers the way the platform taught you it would** — the action button is on the right and Cancel on the left, through one shared helper. Answer 0 is drawn rightmost and takes Return, so the old @@ -1247,6 +1268,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 no longer hands the cursor back to the arrow the moment the drag it advertised begins. +- **A failed update install says so instead of asking again** — the GUI hands + the install to the `tty7-updater` helper and quits, so a failure inside the + helper used to reach only `update.log`: the old version relaunched with no + word about it, and because the prompt state had already been cleared, the + next check offered the very same version again — and again. The helper now + records the terminal outcome of every attempt (`update-outcome.json` next + to `update.json`) — before relaunching the previous app, so the relaunched + GUI finds it already on disk — and the GUI folds it into the update state at + startup: Settings shows the failure with the installer's own reason until + dismissed, and the version asks again only after the same three-day + reminder "Later" uses rather than on the next launch, so one failed install + neither nags nor quietly retires the version. The same channel carries the + success case, so a failure an earlier attempt recorded is retired by the + update that did land. As part of this the helper takes the config directory + as a `--config-dir` argument rather than through the environment, which an + elevated child process would not inherit (#540). + ## [26.8.2] - 2026-08-09 ### Added diff --git a/Cargo.toml b/Cargo.toml index 8699bc69..5b7e1a8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,6 +147,11 @@ libc = "0.2" windows-sys = { version = "0.61", features = [ "Win32_Foundation", "Win32_Storage_FileSystem", + # ConvertStringSecurityDescriptorToSecurityDescriptorW + SECURITY_ATTRIBUTES: + # the elevated updater's %ProgramData% staging directory gets an explicit + # admin-only DACL (see `create_dir_admin_only` in tty7-updater). + "Win32_Security", + "Win32_Security_Authorization", "Win32_System_Registry", "Wdk_System_SystemServices", "Win32_System_SystemInformation", diff --git a/crates/tty7-core/src/daemon/install/mod.rs b/crates/tty7-core/src/daemon/install/mod.rs index d6a4d10d..c4913ece 100644 --- a/crates/tty7-core/src/daemon/install/mod.rs +++ b/crates/tty7-core/src/daemon/install/mod.rs @@ -6,6 +6,7 @@ pub mod asset; pub mod checksums; #[cfg(feature = "remote-install")] pub mod download; +pub mod outcome; #[cfg(feature = "remote-install")] pub mod proxy; pub mod ssh_ops; diff --git a/crates/tty7-core/src/daemon/install/outcome.rs b/crates/tty7-core/src/daemon/install/outcome.rs new file mode 100644 index 00000000..dbffcd6a --- /dev/null +++ b/crates/tty7-core/src/daemon/install/outcome.rs @@ -0,0 +1,99 @@ +//! The one record the auto-updater leaves behind about how an install ended. +//! +//! The updater is a separate process the GUI never waits on: the GUI quits as +//! soon as the helper is spawned, and the helper's only other channel is +//! `update.log`, which nobody reads unprompted. An install that failed +//! therefore looked exactly like one still in flight — and because launching +//! the helper had already cleared the prompt state, the same version simply +//! asked again (issue #540). This file closes that channel: the helper writes +//! it on every terminal path it can still reach, and the next GUI launch +//! merges it into the on-disk update state and deletes it. +//! +//! Both sides live in different binaries (`tty7-app` reads, `tty7-updater` +//! writes), so the schema lives here where neither can drift from the other. + +use std::path::Path; + +/// The filename under the config directory. The full path crosses to the +/// updater as a command-line argument — an elevated child does not inherit +/// the caller's `TTY7_CONFIG_DIR` — so this constant is only for the side +/// that reads and deletes the file. +pub const OUTCOME_FILE_NAME: &str = "update-outcome.json"; + +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct UpdateOutcome { + /// The version the attempt tried to install. + pub version: String, + pub ok: bool, + /// Why it failed. Always present when `ok` is false — a failure without + /// a reason is exactly the silent failure this file exists to kill. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub detail: Option, +} + +/// Atomic so a GUI that launches mid-write never parses half a record. +pub fn write_outcome(path: &Path, outcome: &UpdateOutcome) -> std::io::Result<()> { + let json = serde_json::to_vec(outcome).map_err(std::io::Error::other)?; + crate::core::config::write_atomic(path, &json) +} + +/// `None` when no updater ran (the common case — the file only exists between +/// an install attempt and the next GUI launch). A file that exists but cannot +/// be read or parsed is an error: something did run, and "the result is +/// unreadable" is itself a result worth surfacing. +pub fn read_outcome(path: &Path) -> std::io::Result> { + let bytes = match std::fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + }; + let outcome = serde_json::from_slice(&bytes) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error))?; + Ok(Some(outcome)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn outcome_round_trips_through_the_file() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join(OUTCOME_FILE_NAME); + + assert_eq!(read_outcome(&path).unwrap(), None); + + let failure = UpdateOutcome { + version: "27.0.0".to_string(), + ok: false, + detail: Some("the installer exited with exit code 5".to_string()), + }; + write_outcome(&path, &failure).unwrap(); + assert_eq!(read_outcome(&path).unwrap(), Some(failure)); + + let success = UpdateOutcome { + version: "27.0.0".to_string(), + ok: true, + detail: None, + }; + write_outcome(&path, &success).unwrap(); + assert_eq!(read_outcome(&path).unwrap(), Some(success)); + // A success stays lean: no `"detail":null` for a reader to trip on. + assert_eq!( + std::fs::read_to_string(&path).unwrap(), + r#"{"version":"27.0.0","ok":true}"# + ); + } + + #[test] + fn a_garbage_outcome_is_an_error_not_a_silent_miss() { + let root = tempfile::tempdir().unwrap(); + let path = root.path().join(OUTCOME_FILE_NAME); + std::fs::write(&path, b"not json").unwrap(); + + assert_eq!( + read_outcome(&path).unwrap_err().kind(), + std::io::ErrorKind::InvalidData + ); + } +} diff --git a/src/bin/tty7-updater.rs b/src/bin/tty7-updater.rs index ced5572c..be2a8824 100644 --- a/src/bin/tty7-updater.rs +++ b/src/bin/tty7-updater.rs @@ -46,7 +46,8 @@ mod macos { let stage = next_path(&mut args)?; let expected_version = next_string(&mut args)?; let log = next_path(&mut args)?; - reject_extra(args)?; + let options = tail_options(args)?; + options.apply(); install(InstallPlan { parent_pid, current, @@ -56,6 +57,7 @@ mod macos { stage, expected_version, log, + result_file: options.result_file, }) } _ => Err(usage()), @@ -66,7 +68,8 @@ mod macos { "usage: tty7-updater verify \ \n\ or: tty7-updater install \ - " + \ + [--config-dir ] [--result-file ]" .to_string() } @@ -88,6 +91,65 @@ mod macos { } } + /// The named options an install verb takes after its positional + /// arguments. See the Windows half of this file for why these are + /// arguments and not the environment. + #[derive(Default)] + struct TailOptions { + config_dir: Option, + result_file: Option, + } + + fn tail_options( + mut args: impl Iterator, + ) -> Result { + let mut options = TailOptions::default(); + while let Some(arg) = args.next() { + match arg.to_str() { + Some("--config-dir") => options.config_dir = Some(next_path(&mut args)?), + Some("--result-file") => options.result_file = Some(next_path(&mut args)?), + _ => return Err(usage()), + } + } + Ok(options) + } + + impl TailOptions { + fn apply(&self) { + let Some(dir) = &self.config_dir else { return }; + tty7_core::core::config::set_config_dir(dir.clone()); + // Re-exported so the relaunched app — a child of this process — + // keeps answering for the same config directory. Safe here: + // argument parsing runs before any thread exists. + unsafe { std::env::set_var("TTY7_CONFIG_DIR", dir) }; + } + } + + /// The terminal outcome of the attempt, for the next GUI launch to merge + /// into the update state (#540). Best-effort, like every log line here. + fn report_outcome( + result_file: Option<&Path>, + log: &Path, + version: &str, + result: &Result<(), String>, + ) { + let Some(path) = result_file else { return }; + let outcome = tty7_core::daemon::install::outcome::UpdateOutcome { + version: version.to_string(), + ok: result.is_ok(), + detail: result.as_ref().err().cloned(), + }; + if let Err(error) = tty7_core::daemon::install::outcome::write_outcome(path, &outcome) { + log_line( + log, + &format!( + "could not record the update outcome at {}: {error}", + path.display() + ), + ); + } + } + struct InstallPlan { parent_pid: u32, current: PathBuf, @@ -97,9 +159,14 @@ mod macos { stage: PathBuf, expected_version: String, log: PathBuf, + result_file: Option, } fn install(plan: InstallPlan) -> Result<(), String> { + install_inner(&plan) + } + + fn install_inner(plan: &InstallPlan) -> Result<(), String> { let replacement = plan.stage.join("unpacked/tty7.app"); wait_for_exit(plan.parent_pid); log_line(&plan.log, "re-verifying staged tty7 update"); @@ -108,11 +175,29 @@ mod macos { if let Err(error) = verification { log_line(&plan.log, &error); let _ = fs::remove_dir_all(&plan.stage); + let result = Err(error); + // The outcome lands before the old app does: the relaunched GUI + // merges it at startup, and a write afterward races that merge + // (#540). + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + &result, + ); let _ = launch_app(&plan.current); - return Err(error); + return result; } log_line(&plan.log, &format!("replacing {}", plan.current.display())); - replace_and_relaunch(&plan.current, &replacement, &plan.stage, launch_app) + let report = |result: &Result<(), String>| { + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + result, + ); + }; + replace_and_relaunch(&plan.current, &replacement, &plan.stage, launch_app, report) .inspect_err(|error| log_line(&plan.log, error)) } @@ -219,6 +304,7 @@ mod macos { replacement: &Path, stage: &Path, launch: impl Fn(&Path) -> Result<(), String>, + report: impl Fn(&Result<(), String>), ) -> Result<(), String> { // The staging directory is a fresh TempDir created beside the current // bundle, so a backup here stays on the same filesystem without using a @@ -227,33 +313,55 @@ mod macos { // update (or simply an unrelated user-owned path). let backup = stage.join("previous.app"); if backup.exists() { - return Err(format!( + let result = Err(format!( "the update staging backup already exists: {}", backup.display() )); + report(&result); + return result; + } + if let Err(error) = fs::rename(current, &backup) { + let result = Err(format!("moving the current app aside: {error}")); + report(&result); + return result; } - fs::rename(current, &backup) - .map_err(|error| format!("moving the current app aside: {error}"))?; if let Err(error) = fs::rename(replacement, current) { let _ = fs::rename(&backup, current); let _ = fs::remove_dir_all(stage); - return Err(format!("putting the staged app in place: {error}")); + let result = Err(format!("putting the staged app in place: {error}")); + report(&result); + return result; } match launch(current) { Ok(()) => { let _ = remove_path(&backup); let _ = fs::remove_dir_all(stage); - Ok(()) + let result = Ok(()); + report(&result); + result } Err(error) => { let _ = remove_path(current); - fs::rename(&backup, current) - .map_err(|restore| format!("{error}; restoring the previous app: {restore}"))?; - let _ = fs::remove_dir_all(stage); - let _ = launch(current); - Err(error) + let (result, relaunch) = match fs::rename(&backup, current) { + Ok(()) => { + let _ = fs::remove_dir_all(stage); + (Err(error), true) + } + Err(restore) => ( + Err(format!("{error}; restoring the previous app: {restore}")), + false, + ), + }; + // The outcome lands before the old app does: the relaunched GUI + // merges it at startup, and a write afterward races that merge + // (#540). + report(&result); + if relaunch { + let _ = launch(current); + } + result } } } @@ -360,7 +468,7 @@ mod macos { bundle(¤t, "old"); bundle(&replacement, "new"); - replace_and_relaunch(¤t, &replacement, &stage, |_| Ok(())).unwrap(); + replace_and_relaunch(¤t, &replacement, &stage, |_| Ok(()), |_| ()).unwrap(); assert_eq!(fs::read_to_string(current.join("marker")).unwrap(), "new"); assert!(!stage.exists()); @@ -376,19 +484,30 @@ mod macos { bundle(¤t, "old"); bundle(&replacement, "new"); let launches = std::cell::Cell::new(0); + let reported_after_launches = std::cell::Cell::new(usize::MAX); - let error = replace_and_relaunch(¤t, &replacement, &stage, |_| { - launches.set(launches.get() + 1); - if launches.get() == 1 { - Err("new app failed".to_string()) - } else { - Ok(()) - } - }) + let error = replace_and_relaunch( + ¤t, + &replacement, + &stage, + |_| { + launches.set(launches.get() + 1); + if launches.get() == 1 { + Err("new app failed".to_string()) + } else { + Ok(()) + } + }, + |_| reported_after_launches.set(launches.get()), + ) .unwrap_err(); assert_eq!(error, "new app failed"); assert_eq!(launches.get(), 2); + // The outcome is reported after the failed first launch but before + // the old app comes back — the relaunched GUI must find it already + // on disk at startup (#540). + assert_eq!(reported_after_launches.get(), 1); assert_eq!(fs::read_to_string(current.join("marker")).unwrap(), "old"); assert!(!stage.exists()); } @@ -404,7 +523,7 @@ mod macos { bundle(&replacement, "new"); bundle(&sibling, "keep"); - replace_and_relaunch(¤t, &replacement, &stage, |_| Ok(())).unwrap(); + replace_and_relaunch(¤t, &replacement, &stage, |_| Ok(()), |_| ()).unwrap(); assert_eq!(fs::read_to_string(current.join("marker")).unwrap(), "new"); assert_eq!(fs::read_to_string(sibling.join("marker")).unwrap(), "keep"); @@ -483,15 +602,18 @@ mod windows { use std::process::{Child, Command, ExitStatus, Stdio}; use std::ptr::null_mut; use std::thread; - use std::time::Duration; + use std::time::{Duration, Instant}; use smol::io::AsyncReadExt as _; + use tty7_core::daemon::install::outcome::UpdateOutcome; use windows_sys::Win32::Foundation::{ - CloseHandle, ERROR_INVALID_PARAMETER, GetLastError, HANDLE, WAIT_FAILED, + CloseHandle, ERROR_ACCESS_DENIED, ERROR_INVALID_PARAMETER, GetLastError, HANDLE, LocalFree, + WAIT_FAILED, WAIT_TIMEOUT, }; use windows_sys::Win32::Storage::FileSystem::{ - GetFileVersionInfoSizeW, GetFileVersionInfoW, VS_FIXEDFILEINFO, VerQueryValueW, + CreateDirectoryW, GetFileVersionInfoSizeW, GetFileVersionInfoW, VS_FIXEDFILEINFO, + VerQueryValueW, }; use windows_sys::Win32::System::Threading::{ INFINITE, OpenProcess, PROCESS_SYNCHRONIZE, WaitForSingleObject, @@ -575,7 +697,8 @@ mod windows { let expected_version = next_string(&mut args)?; let log = next_path(&mut args)?; let stage = next_path(&mut args)?; - reject_extra(args)?; + let options = tail_options(args)?; + options.apply(); install(InstallPlan { parent_pid, installer, @@ -585,6 +708,8 @@ mod windows { expected_version, log, stage, + result_file: options.result_file, + expected_sha256: None, }) } "install-portable" => { @@ -598,7 +723,8 @@ mod windows { let expected_version = next_string(&mut args)?; let log = next_path(&mut args)?; let stage = next_path(&mut args)?; - reject_extra(args)?; + let options = tail_options(args)?; + options.apply(); install_portable(PortableInstallPlan { parent_pid, archive, @@ -608,6 +734,7 @@ mod windows { expected_version, log, stage, + result_file: options.result_file, }) } "cleanup" => { @@ -620,6 +747,90 @@ mod windows { fs::remove_dir_all(&stage) .map_err(|error| format!("removing {}: {error}", stage.display())) } + "capabilities" => { + reject_extra(args)?; + // One token per line. The GUI reads this to learn whether the + // *installed* updater — the binary a UAC prompt would point + // at — speaks the elevation verbs; a build that predates them + // exits with the usage error above instead, which is the same + // answer (#504). + for capability in ELEVATION_CAPABILITIES { + println!("{capability}"); + } + Ok(()) + } + "elevated-stage" => { + let gui_pid = next_string(&mut args)? + .parse::() + .map_err(|_| "gui pid is not an unsigned integer".to_string())?; + let installer = next_path(&mut args)?; + let asset_name = next_string(&mut args)?; + let install_dir = next_path(&mut args)?; + let expected_version = next_string(&mut args)?; + let log = next_path(&mut args)?; + let stage = next_path(&mut args)?; + let options = tail_options(args)?; + options.apply(); + let (Some(expected_sha256), Some(status_file)) = + (options.expected_sha256, options.status_file) + else { + return Err(usage()); + }; + elevated_stage(ElevatedStagePlan { + gui_pid, + installer, + asset_name, + install_dir, + expected_version, + log, + stage, + expected_sha256, + status_file, + result_file: options.result_file, + config_dir: options.config_dir, + }) + } + "install-elevated" => { + let parent_pid = next_string(&mut args)? + .parse::() + .map_err(|_| "parent pid is not an unsigned integer".to_string())?; + let installer = next_path(&mut args)?; + let checksums = next_path(&mut args)?; + let asset_name = next_string(&mut args)?; + let install_dir = next_path(&mut args)?; + let expected_version = next_string(&mut args)?; + let log = next_path(&mut args)?; + let stage = next_path(&mut args)?; + let options = tail_options(args)?; + options.apply(); + let Some(expected_sha256) = options.expected_sha256 else { + return Err(usage()); + }; + install_elevated(InstallPlan { + parent_pid, + installer, + checksums, + asset_name, + install_dir, + expected_version, + log, + stage, + result_file: options.result_file, + expected_sha256: Some(expected_sha256), + }) + } + "relaunch-watcher" => { + let options = tail_options(args)?; + options.apply(); + watch(&WatcherPlan { + status_file: options.status_file.ok_or_else(usage)?, + result_file: options.result_file.ok_or_else(usage)?, + app: options.app_path.ok_or_else(usage)?, + log: options.log.ok_or_else(usage)?, + version: options.expected_version.ok_or_else(usage)?, + gui_pid: options.gui_pid, + }) + } _ => Err(usage()), } } @@ -627,12 +838,25 @@ mod windows { fn usage() -> String { "usage: tty7-updater verify \n\ or: tty7-updater install \ - \n\ + \ + [--config-dir ] [--result-file ]\n\ or: tty7-updater verify-portable \ \n\ or: tty7-updater install-portable \ - \n\ - or: tty7-updater cleanup " + \ + [--config-dir ] [--result-file ]\n\ + or: tty7-updater cleanup \n\ + or: tty7-updater capabilities\n\ + or: tty7-updater elevated-stage \ + \ + --expected-sha256 --status-file \ + [--config-dir ] [--result-file ]\n\ + or: tty7-updater install-elevated \ + \ + --expected-sha256 [--config-dir ] [--result-file ]\n\ + or: tty7-updater relaunch-watcher --status-file --result-file \ + --app-path --log --expected-version \ + [--gui-pid ] [--config-dir ]" .to_string() } @@ -654,6 +878,106 @@ mod windows { } } + /// The named options an install verb takes after its positional arguments. + /// + /// Everything the caller needs this process to know travels this way — + /// never through the environment. An elevated (UAC) child does not + /// inherit the spawning GUI's environment, so a `TTY7_CONFIG_DIR` set + /// there would silently fall back to the *administrator's* config + /// directory under an over-the-shoulder elevation (#504). + #[derive(Default)] + struct TailOptions { + config_dir: Option, + result_file: Option, + /// The staged package's digest as the release server published it. + /// Crosses the elevation boundary on the command line because the + /// checksums file beside the package cannot anchor trust there: a + /// medium-integrity process can rewrite both together. + expected_sha256: Option, + /// Where `elevated-stage` tells the watcher which pid names the + /// install chain. + status_file: Option, + /// The `tty7-app.exe` the watcher relaunches. + app_path: Option, + /// Log override for the verbs that take no positional log path (the + /// watcher). + log: Option, + /// The version being installed, for the watcher's synthesized + /// outcomes. + expected_version: Option, + /// The GUI the watcher must not relaunch over. See [`WatcherPlan`]. + gui_pid: Option, + } + + fn tail_options(mut args: impl Iterator) -> Result { + let mut options = TailOptions::default(); + while let Some(arg) = args.next() { + match arg.to_str() { + Some("--config-dir") => options.config_dir = Some(next_path(&mut args)?), + Some("--result-file") => options.result_file = Some(next_path(&mut args)?), + Some("--expected-sha256") => { + options.expected_sha256 = Some(next_string(&mut args)?) + } + Some("--status-file") => options.status_file = Some(next_path(&mut args)?), + Some("--app-path") => options.app_path = Some(next_path(&mut args)?), + Some("--log") => options.log = Some(next_path(&mut args)?), + Some("--expected-version") => { + options.expected_version = Some(next_string(&mut args)?) + } + Some("--gui-pid") => { + options.gui_pid = Some( + next_string(&mut args)? + .parse::() + .map_err(|_| "gui pid is not an unsigned integer".to_string())?, + ) + } + _ => return Err(usage()), + } + } + Ok(options) + } + + impl TailOptions { + fn apply(&self) { + let Some(dir) = &self.config_dir else { return }; + tty7_core::core::config::set_config_dir(dir.clone()); + // The override above covers this process; the variable is + // re-exported so the helper children this process spawns — the + // payload's `--stop-daemon`, the relaunched app — keep answering + // for the same config directory. Safe here: argument parsing runs + // before any thread exists. + unsafe { std::env::set_var("TTY7_CONFIG_DIR", dir) }; + } + } + + /// The terminal outcome of the attempt, for the next GUI launch to merge + /// into the update state (#540). Best-effort: a result that cannot be + /// recorded goes to the log like every other updater detail. On paths that + /// relaunch the previous app this runs *before* the relaunch, so the GUI + /// finds the outcome already on disk when it starts. + fn report_outcome( + result_file: Option<&Path>, + log: &Path, + version: &str, + result: &Result<(), String>, + ) { + let Some(path) = result_file else { return }; + let outcome = tty7_core::daemon::install::outcome::UpdateOutcome { + version: version.to_string(), + ok: result.is_ok(), + detail: result.as_ref().err().cloned(), + }; + if let Err(error) = tty7_core::daemon::install::outcome::write_outcome(path, &outcome) { + log_line( + log, + &format!( + "could not record the update outcome at {}: {error}", + path.display() + ), + ); + } + } + struct InstallPlan { parent_pid: u32, installer: PathBuf, @@ -663,6 +987,10 @@ mod windows { expected_version: String, log: PathBuf, stage: PathBuf, + result_file: Option, + /// Set on the elevated path, where the digest — not the checksums + /// file it was copied with — is the trust anchor (#504). + expected_sha256: Option, } struct PortableInstallPlan { @@ -674,21 +1002,54 @@ mod windows { expected_version: String, log: PathBuf, stage: PathBuf, + result_file: Option, + } + + /// How an install ends. The distinction exists because an elevated + /// process must never spawn `tty7-app.exe`: the app it launched would + /// inherit the elevation — and under an over-the-shoulder prompt it + /// would even be the *administrator's* app, with the wrong account's + /// config. The elevated chain reports instead, and the medium-integrity + /// watcher relaunches. + #[derive(Clone, Copy, PartialEq, Eq)] + enum Completion { + /// Today's path: this process relaunches the app itself. + RelaunchHere, + /// Elevated path: release the guard, write the outcome, leave the + /// relaunch to the watcher. + ReportToWatcher, } fn install(plan: InstallPlan) -> Result<(), String> { + install_inner(&plan, Completion::RelaunchHere) + } + + fn install_elevated(plan: InstallPlan) -> Result<(), String> { + install_inner(&plan, Completion::ReportToWatcher) + } + + fn install_inner(plan: &InstallPlan, completion: Completion) -> Result<(), String> { log_line(&plan.log, "waiting for the tty7 GUI to exit"); if let Err(error) = wait_for_exit(plan.parent_pid) { - return recover_from_failed_update(&plan, error); + return recover_from_failed_update(plan, error, completion); } log_line(&plan.log, "re-verifying the staged Windows installer"); - if let Err(error) = verify_update( - &plan.installer, - &plan.checksums, - &plan.asset_name, - &plan.expected_version, - ) { - return recover_from_failed_update(&plan, error); + let verification = match &plan.expected_sha256 { + Some(digest) => verify_update_digest( + &plan.installer, + &plan.asset_name, + &plan.expected_version, + digest, + ), + None => verify_update( + &plan.installer, + &plan.checksums, + &plan.asset_name, + &plan.expected_version, + ), + }; + if let Err(error) = verification { + return recover_from_failed_update(plan, error, completion); } // Setup's own PrepareToInstall repeats this, but doing it here first @@ -701,51 +1062,749 @@ mod windows { ); // From here until the relaunch, a `tty7` CLI call or a manual launch // must not spawn a daemon that relocks the files Setup is replacing. - // launch_app releases the guard on every path out of this function. + // Every path out of this function releases the guard — `launch_app` + // on the relaunch-here paths, an explicit clear on the elevated ones. tty7_core::daemon::update_guard::hold(); if let Err(error) = tty7_core::daemon::spawn::stop_for_update(&plan.install_dir) { - return recover_from_failed_update(&plan, error); + return recover_from_failed_update(plan, error, completion); } log_line(&plan.log, "running the tty7 Windows installer"); let status = match run_installer(&plan.installer, &plan.log) { Ok(status) => status, Err(error) => { - return recover_from_failed_update(&plan, error); + return recover_from_failed_update(plan, error, completion); } }; if !status.success() { let error = format!("the Windows installer exited with {status}"); - return recover_from_failed_update(&plan, error); + return recover_from_failed_update(plan, error, completion); } if let Err(error) = verify_installed_payload(&plan.install_dir, &plan.expected_version) { - return recover_from_failed_update(&plan, error); + return recover_from_failed_update(plan, error, completion); + } + if completion == Completion::ReportToWatcher { + log_line( + &plan.log, + "the Windows update completed; the watcher relaunches tty7", + ); + // The watcher hands the outcome to the next GUI launch, so it is + // written before the guard comes off and anything can start. + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + &Ok(()), + ); + tty7_core::daemon::update_guard::clear(); + queue_cleanup(&plan.install_dir, &plan.stage); + return Ok(()); } log_line(&plan.log, "the Windows update completed; relaunching tty7"); let result = launch_app(&plan.install_dir); if let Err(error) = &result { log_line(&plan.log, error); } + // A failed relaunch is the outcome here, and there is no running GUI + // to race it; a successful one launches the new build, whose absorb + // finds this already on disk. + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + &result, + ); queue_cleanup(&plan.install_dir, &plan.stage); result } /// Records one terminal update failure and restores the same recovery /// behavior for every step that can fail after the GUI starts shutting down. - fn recover_from_failed_update(plan: &InstallPlan, error: String) -> Result<(), String> { - recover_without_replacement(&plan.log, &plan.install_dir, &plan.stage, error) + fn recover_from_failed_update( + plan: &InstallPlan, + error: String, + completion: Completion, + ) -> Result<(), String> { + if completion == Completion::ReportToWatcher { + // The relaunch belongs to the watcher on this path — an elevated + // process spawning the app is the one thing the chain must never + // do, failure recovery included. The guard still ends here: the + // installation is no longer being replaced. + log_line(&plan.log, &error); + let result = Err(error); + // Before the guard comes off: the watcher hands this to the next + // GUI launch, and the guard is what holds that launch back. + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + &result, + ); + tty7_core::daemon::update_guard::clear(); + queue_cleanup(&plan.install_dir, &plan.stage); + return result; + } + recover_without_replacement( + &plan.log, + &plan.install_dir, + &plan.stage, + plan.result_file.as_deref(), + &plan.expected_version, + error, + ) + } + + // --------------------------------------------------------------------- + // The elevated chain (#504): one UAC prompt, two elevated stages, and a + // watcher that never elevates. The GUI points the prompt at the + // *installed* updater — the one binary a medium-integrity process cannot + // have replaced — running `elevated-stage`; that stage re-stages the + // payload under an admin-only directory and runs `install-elevated` from + // it; the install stage reports through the outcome file instead of + // relaunching the app; and the `relaunch-watcher`, spawned by the GUI + // before the prompt as the original user, relaunches it. + + /// What `capabilities` prints, one per line — the verbs the GUI requires + /// before it points a UAC prompt at the installed updater. The GUI keeps + /// its own copy of this list (see `updater_speaks_elevation` in + /// `core::update`); an updater that predates the verbs never prints them. + const ELEVATION_CAPABILITIES: [&str; 3] = + ["elevated-stage", "install-elevated", "relaunch-watcher"]; + + struct ElevatedStagePlan { + gui_pid: u32, + installer: PathBuf, + asset_name: String, + install_dir: PathBuf, + expected_version: String, + log: PathBuf, + stage: PathBuf, + expected_sha256: String, + status_file: PathBuf, + result_file: Option, + config_dir: Option, + } + + fn elevated_stage(plan: ElevatedStagePlan) -> Result<(), String> { + let result = elevated_stage_inner(&plan); + if let Err(error) = &result + && plan + .result_file + .as_deref() + .is_some_and(|path| !path.exists()) + { + // The install stage writes the outcome itself once it runs, so a + // failure before that — a digest mismatch, a staging error — has + // to be reported here, or the watcher waits out its status grace + // for a chain that never started. + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + &Err(error.clone()), + ); + } + result + } + + fn elevated_stage_inner(plan: &ElevatedStagePlan) -> Result<(), String> { + log_line(&plan.log, "preparing the elevated update staging"); + // Every path this stage trusts is derived from its own image, never + // taken from the caller: the caller sits below the integrity + // boundary, and `` is what the helper pinning below + // compares against. Believing the argument would put both halves of + // that comparison in the hands of whoever wrote the command line. + let install_dir = installed_root()?; + if !same_directory(&install_dir, &plan.install_dir) { + log_line( + &plan.log, + &format!( + "the caller named {} as the installation; using {}, which is where \ + this updater actually runs from", + plan.install_dir.display(), + install_dir.display() + ), + ); + } + // The digest arrived on the command line — the one value a + // medium-integrity process cannot have forged, because the GUI read + // it from the release server over HTTPS. Everything this chain + // executes or installs is checked against it, before use and again + // after every copy. + verify_digest( + &plan.installer, + &plan.expected_sha256, + "staged Windows installer", + )?; + // The staged helper copy runs a process tree higher than it was + // written, so it is pinned to the installed image first — the one + // file a medium-integrity process cannot have replaced. + pin_helper_to_installed(&plan.stage.join("tty7-updater.exe"), &install_dir)?; + + let staging = create_protected_staging()?; + let result = run_install_stage(plan, &install_dir, &staging); + // Whatever happened, the admin-only staging goes with this process. A + // removal failure strands it for the next elevated run to clear. + let _ = fs::remove_dir_all(&staging); + result + } + + /// The installation this process was started from. UAC pointed the prompt + /// at `{app}\tty7-updater.exe`, so this process's own image names the + /// directory a medium-integrity process cannot write — the only trust + /// root the chain has before the payload is signed. + fn installed_root() -> Result { + let exe = std::env::current_exe() + .map_err(|error| format!("locating the running updater: {error}"))?; + exe.parent() + .map(Path::to_path_buf) + .ok_or_else(|| format!("{} names no directory", exe.display())) + } + + /// Only for telling the log that the caller's `` disagreed + /// with the image path. Deliberately not a gate: the derived directory is + /// used either way, and a cosmetic difference (case, a short path) must + /// not fail an update the user is watching. + fn same_directory(left: &Path, right: &Path) -> bool { + left.as_os_str() + .to_string_lossy() + .eq_ignore_ascii_case(&right.as_os_str().to_string_lossy()) + } + + fn run_install_stage( + plan: &ElevatedStagePlan, + install_dir: &Path, + staging: &Path, + ) -> Result<(), String> { + let staged_installer = staging.join(&plan.asset_name); + let staged_checksums = staging.join("checksums.txt"); + let staged_updater = staging.join("tty7-updater.exe"); + fs::copy(&plan.installer, &staged_installer).map_err(|error| { + format!( + "copying {} into the protected staging: {error}", + plan.installer.display() + ) + })?; + fs::copy(plan.stage.join("checksums.txt"), &staged_checksums).map_err(|error| { + format!("copying the checksums into the protected staging: {error}") + })?; + fs::copy(plan.stage.join("tty7-updater.exe"), &staged_updater) + .map_err(|error| format!("copying the updater into the protected staging: {error}"))?; + // Re-verified at their new home: the copy, not just the source, is + // what stage 2 executes and installs. + verify_digest( + &staged_installer, + &plan.expected_sha256, + "re-staged Windows installer", + )?; + pin_helper_to_installed(&staged_updater, install_dir)?; + + // Name this process to the watcher as late as possible: it waits on + // the install stage below, so its one pid covers the whole chain. + if let Some(parent) = plan.status_file.parent() { + let _ = fs::create_dir_all(parent); + } + fs::write(&plan.status_file, std::process::id().to_string()) + .map_err(|error| format!("writing {}: {error}", plan.status_file.display()))?; + + log_line(&plan.log, "running the elevated install stage"); + let mut command = Command::new(&staged_updater); + command + .arg("install-elevated") + .arg(plan.gui_pid.to_string()) + .arg(&staged_installer) + .arg(&staged_checksums) + .arg(&plan.asset_name) + .arg(install_dir) + .arg(&plan.expected_version) + .arg(&plan.log) + .arg(&plan.stage) + .arg("--expected-sha256") + .arg(&plan.expected_sha256); + if let Some(result_file) = &plan.result_file { + command.arg("--result-file").arg(result_file); + } + if let Some(config_dir) = &plan.config_dir { + command.arg("--config-dir").arg(config_dir); + } + let status = command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .and_then(|mut child| child.wait()) + .map_err(|error| format!("running the elevated install stage: {error}"))?; + if !status.success() { + return Err(format!("the elevated install stage exited with {status}")); + } + Ok(()) + } + + /// `%ProgramData%\tty7\update-`, created with a DACL that lets no + /// standard user in. Between the digest check and the install the payload + /// sits in this directory, and one any medium-integrity process could + /// write would hand it a swap-in window across exactly that gap. + fn create_protected_staging() -> Result { + let program_data = std::env::var_os("ProgramData") + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from(r"C:\ProgramData")); + let root = program_data.join("tty7"); + claim_protected_root(&root)?; + let staging = root.join(format!("update-{}", std::process::id())); + create_dir_admin_only(&staging)?; + Ok(staging) + } + + /// Claims `%ProgramData%\tty7` itself with the admin-only DACL, taking + /// down whatever holds the name first. + /// + /// `%ProgramData%` lets a standard user create directories, and the + /// creator owns what it creates. A root left to exist as somebody's + /// pre-created directory would hand its owner delete-child over the + /// "admin-only" staging inside it — enough to rename the verified + /// staging aside and drop an identically named one of their own into the + /// gap between the digest check and the execute, which is the exact + /// window the protected DACL exists to close. Creating the root here + /// makes it as unwritable as the staging: `CreateDirectoryW` applies the + /// descriptor only when it is the one creating the directory, so + /// succeeding *is* the proof. + /// + /// Nothing else lives under it — it holds staging directories and + /// nothing more — so removing it costs at most a dead chain's leftovers. + /// A holder that cannot be cleared (a live chain's locked image, an + /// attacker sitting on an open handle) fails the update closed. + fn claim_protected_root(root: &Path) -> Result<(), String> { + // A standard user racing the removal can only lose the name back to + // us; three tries is more than that race needs. + let mut failure = String::new(); + for _ in 0..3 { + // `remove_path`, not `remove_dir_all`: the name may be held by a + // file or by a junction pointing somewhere it would be a + // catastrophe to recurse into, and a name that is not there at + // all is the ordinary first run. + remove_path(root)?; + match create_dir_admin_only(root) { + Ok(()) => return Ok(()), + Err(error) => failure = error, + } + } + Err(failure) + } + + fn create_dir_admin_only(dir: &Path) -> Result<(), String> { + use windows_sys::Win32::Security::Authorization::{ + ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + }; + use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; + // Owner Administrators, group SYSTEM, and a protected DACL granting + // full control to exactly those two — not even read to Users. + const SDDL: &str = "O:BAG:SYD:P(A;OICI;FA;;;BA)(A;OICI;FA;;;SY)"; + let mut descriptor: *mut c_void = null_mut(); + if unsafe { + ConvertStringSecurityDescriptorToSecurityDescriptorW( + wide_string(SDDL).as_ptr(), + SDDL_REVISION_1, + &mut descriptor, + null_mut(), + ) + } == 0 + { + return Err(format!( + "translating the staging DACL: OS error {}", + unsafe { GetLastError() } + )); + } + let attributes = SECURITY_ATTRIBUTES { + nLength: size_of::() as u32, + lpSecurityDescriptor: descriptor, + bInheritHandle: 0, + }; + let created = unsafe { CreateDirectoryW(wide_path(dir).as_ptr(), &attributes) }; + let _ = unsafe { LocalFree(descriptor) }; + if created == 0 { + return Err(format!("creating {}: OS error {}", dir.display(), unsafe { + GetLastError() + })); + } + Ok(()) + } + + /// The staged helper copy runs elevated, so it may only be the exact + /// bytes of the installed one: the installation directory is the trust + /// root a medium-integrity process cannot write, and the staged copy is + /// pinned to it before any use. + fn pin_helper_to_installed(staged: &Path, install_dir: &Path) -> Result<(), String> { + let installed = install_dir.join("tty7-updater.exe"); + if file_digest(staged)? != file_digest(&installed)? { + return Err(format!( + "the staged updater at {} does not match the installed {} — \ + refusing to run it elevated", + staged.display(), + installed.display() + )); + } + Ok(()) + } + + fn file_digest(path: &Path) -> Result { + let bytes = + fs::read(path).map_err(|error| format!("reading {}: {error}", path.display()))?; + Ok(tty7_core::daemon::install::checksums::hex( + &tty7_core::daemon::install::checksums::sha256(&bytes), + )) + } + + fn verify_digest(file: &Path, expected_hex: &str, label: &str) -> Result<(), String> { + let actual = file_digest(file)?; + if !actual.eq_ignore_ascii_case(expected_hex) { + return Err(format!( + "the {label} failed sha256 verification: expected {expected_hex}, got {actual}" + )); + } + Ok(()) + } + + /// The elevated path's replacement for `verify_update`: same filename and + /// version checks, but the digest comes from the command line — the value + /// the GUI read from the release server — not from a checksums file that + /// crossed the medium-integrity staging directory beside the installer. + fn verify_update_digest( + installer: &Path, + asset_name: &str, + expected_version: &str, + expected_sha256: &str, + ) -> Result<(), String> { + if installer.file_name() != Some(OsStr::new(asset_name)) { + return Err(format!( + "the staged installer filename does not match the release asset {asset_name:?}" + )); + } + verify_digest(installer, expected_sha256, "staged Windows installer")?; + // Same reasoning as the manifest path: corruption or replacement + // while the helper waited is caught here, after the GUI exited. + verify_file_version(installer, expected_version, "staged Windows installer") + } + + struct WatcherPlan { + status_file: PathBuf, + result_file: PathBuf, + app: PathBuf, + log: PathBuf, + version: String, + /// The GUI that raised the prompt, so a relaunch never lands beside a + /// window that is still there. Optional: a watcher told nothing about + /// it still brings the app back, which is the point of the timeouts + /// below — it just cannot tell "still up" from "already gone". + gui_pid: Option, + } + + /// How often the watcher looks at the status and outcome files. + const WATCH_POLL: Duration = Duration::from_secs(1); + /// How long the watcher waits for the chain to first report in. Covers a + /// UAC prompt left open on the secure desktop — not a slow install, which + /// `WATCH_TIMEOUT` bounds once the chain has reported. + const WATCH_STATUS_GRACE: Duration = Duration::from_secs(15 * 60); + /// Bounds the whole install once the chain reported in. An install still + /// running past it (an antivirus rescanning every replaced file) has + /// stopped being an install and started being a machine with no + /// terminal on it: the watcher gives up waiting and brings the app back. + const WATCH_TIMEOUT: Duration = Duration::from_secs(60 * 60); + /// Between the chain's pid dying and giving up on its outcome: the write + /// is the last thing the chain does, so it lands within seconds. + const WATCH_RESULT_GRACE: Duration = Duration::from_secs(10); + /// How long a GUI that is still up when the watcher is ready to relaunch + /// is given to finish quitting. Long enough for a shutdown already under + /// way, short enough that a GUI which is *staying* (a declined prompt + /// whose `kill` did not land) is recognized as staying. + const GUI_EXIT_GRACE: Duration = Duration::from_secs(30); + + /// What the watcher ended up believing about the install, and whether + /// that belief is already on disk. Anything it had to invent has to be + /// written before the app comes back, or a failure the chain never got + /// to record dies with the watcher — which is the hole #540 is about. + struct WatchOutcome { + outcome: UpdateOutcome, + recorded: bool, + } + + impl WatchOutcome { + /// The chain's own word for it, already on disk. + fn recorded(outcome: UpdateOutcome) -> Self { + Self { + outcome, + recorded: true, + } + } + + /// The watcher's word for it, and nobody else's. + fn synthesized(plan: &WatcherPlan, detail: &str) -> Self { + Self { + outcome: UpdateOutcome { + version: plan.version.clone(), + ok: false, + detail: Some(detail.to_string()), + }, + recorded: false, + } + } + } + + fn watch(plan: &WatcherPlan) -> Result<(), String> { + // Opened first thing, while the GUI is provably still alive: it is + // blocked inside `ShellExecuteExW` waiting on the prompt, which is + // why the watcher is spawned before the prompt is raised. A handle + // taken then keeps naming that same process object no matter which + // process inherits the number later. + let gui = GuiProcess::open(plan.gui_pid); + let started = Instant::now(); + let mut chain_seen = false; + let end = loop { + if let Some(end) = read_outcome_lossy(plan) { + break end; + } + match status_pid(&plan.status_file) { + Some(pid) if pid_alive(pid) => { + chain_seen = true; + if started.elapsed() > WATCH_TIMEOUT { + break WatchOutcome::synthesized( + plan, + "the elevated update was still running an hour after it \ + started and never recorded a result", + ); + } + } + // A dead pid — whether or not it was ever seen alive (a fast + // chain can complete between two polls) — or a status file + // that vanished after the chain was seen: the chain is over, + // and its outcome is the last thing it writes. + Some(_) => { + break await_final_outcome(plan); + } + None if chain_seen => { + break await_final_outcome(plan); + } + None => { + if started.elapsed() > WATCH_STATUS_GRACE { + break WatchOutcome::synthesized( + plan, + "the elevated updater never reported in; the install did \ + not run", + ); + } + } + } + thread::sleep(WATCH_POLL); + }; + finish_watch(plan, &gui, end) + } + + /// The chain is gone; its outcome should already be on its way to disk. + fn await_final_outcome(plan: &WatcherPlan) -> WatchOutcome { + let deadline = Instant::now() + WATCH_RESULT_GRACE; + loop { + if let Some(end) = read_outcome_lossy(plan) { + return end; + } + if Instant::now() >= deadline { + return WatchOutcome::synthesized( + plan, + "the elevated updater exited without recording a result", + ); + } + thread::sleep(WATCH_POLL); + } + } + + /// The watcher's read of the outcome file: a parse failure is an outcome + /// — something wrote it — not a reason to keep waiting. Unreadable counts + /// as unrecorded, so the description below replaces the garbage. + fn read_outcome_lossy(plan: &WatcherPlan) -> Option { + match tty7_core::daemon::install::outcome::read_outcome(&plan.result_file) { + Ok(outcome) => outcome.map(WatchOutcome::recorded), + Err(error) => { + let detail = format!( + "the update result at {} could not be read: {error}", + plan.result_file.display() + ); + log_line(&plan.log, &detail); + Some(WatchOutcome::synthesized(plan, &detail)) + } + } + } + + fn finish_watch(plan: &WatcherPlan, gui: &GuiProcess, end: WatchOutcome) -> Result<(), String> { + let _ = fs::remove_file(&plan.status_file); + let WatchOutcome { outcome, recorded } = end; + // Nothing relaunches over a GUI that is still on screen. Two shapes + // reach here with one running: a declined prompt whose `kill` did not + // land — that GUI is staying, and owns its own window and its own + // record — and a chain that failed fast enough to beat the quitting + // GUI out the door, which is a GUI that will be gone in a moment and + // must be relaunched once it is. Waiting tells them apart without + // having to know which it was. + if gui.alive() && !gui.wait_for_exit(GUI_EXIT_GRACE) { + log_line( + &plan.log, + "the tty7 that raised the prompt is still running; leaving the \ + relaunch to it", + ); + return Ok(()); + } + if !recorded { + // The chain never got far enough to say this itself, and the + // launch below is what reads it: an unexplained update that + // simply asks again is exactly what the outcome file is for. + log_line(&plan.log, outcome.detail.as_deref().unwrap_or_default()); + if let Err(error) = + tty7_core::daemon::install::outcome::write_outcome(&plan.result_file, &outcome) + { + log_line( + &plan.log, + &format!( + "could not record the update outcome at {}: {error}", + plan.result_file.display() + ), + ); + } + } + // Success or failure, the binary at the app path is the one to run: + // the new version after a completed install, the previous one after + // a recovery. Spawned from this never-elevated process, so the app + // comes back as the original user whatever the chain ran as. + let health = Command::new(&plan.app) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .map_err(|error| format!("launching {}: {error}", plan.app.display())) + .and_then(|mut child| healthy_after_grace(&mut child)); + if let Err(error) = health { + // "Installed but nothing came back" is a failure the user must + // see, so it replaces the outcome the chain recorded. + let detail = match &outcome.detail { + Some(previous) => format!("{previous}; and the relaunch failed: {error}"), + None => format!("the update installed but the relaunch failed: {error}"), + }; + let _ = tty7_core::daemon::install::outcome::write_outcome( + &plan.result_file, + &UpdateOutcome { + version: outcome.version.clone(), + ok: false, + detail: Some(detail.clone()), + }, + ); + log_line(&plan.log, &detail); + return Err(detail); + } + if outcome.ok { + Ok(()) + } else { + Err(outcome.detail.unwrap_or_default()) + } + } + + /// The GUI that raised the UAC prompt, as seen by the watcher it spawned. + /// + /// Only ever consulted to answer "would relaunching now put a second + /// window beside the first", and the answer is asymmetric on purpose. A + /// living GUI always answers to its own pid, so "alive" is never wrong in + /// the direction that would double-launch. The one way to be wrong the + /// other way is a pid recycled into an unrelated process, which needs the + /// GUI to have died before this watcher even started — before the prompt + /// was answered — and costs only the relaunch this watcher would not have + /// performed before either. + struct GuiProcess { + pid: Option, + /// Held from watcher startup. A handle outlives the pid: the kernel + /// keeps the process object alive as long as this is open, so a + /// recycled number cannot answer for it. + handle: Option, + } + + impl GuiProcess { + fn open(pid: Option) -> Self { + let handle = pid.and_then(|pid| { + let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, pid) }; + (!handle.is_null()).then(|| OwnedHandle(handle)) + }); + Self { pid, handle } + } + + fn alive(&self) -> bool { + if let Some(handle) = &self.handle { + return unsafe { WaitForSingleObject(handle.0, 0) } == WAIT_TIMEOUT; + } + // The handle could not be taken. Falling back to the number keeps + // the conservative answer available; a caller that named no pid + // at all has no GUI to collide with. + self.pid.is_some_and(pid_alive) + } + + /// Waits out a GUI that is quitting. `true` once it is gone, `false` + /// if it is still there when the budget runs out — which is the + /// answer "this one is staying". + fn wait_for_exit(&self, budget: Duration) -> bool { + let deadline = Instant::now() + budget; + loop { + if !self.alive() { + return true; + } + if Instant::now() >= deadline { + return false; + } + thread::sleep(WATCH_POLL); + } + } + } + + /// Whether the pid names a live process, from an account that may not be + /// allowed to open it: under an over-the-shoulder elevation the chain + /// runs as the administrator, and `ERROR_ACCESS_DENIED` is exactly the + /// "alive" answer that boundary gives. + fn pid_alive(pid: u32) -> bool { + let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, pid) }; + if handle.is_null() { + return unsafe { GetLastError() } == ERROR_ACCESS_DENIED; + } + let handle = OwnedHandle(handle); + // OpenProcess succeeds for an exited process while a handle remains; + // the zero wait tells running from merely remembered. + let wait = unsafe { WaitForSingleObject(handle.0, 0) }; + wait == WAIT_TIMEOUT + } + + fn status_pid(status_file: &Path) -> Option { + fs::read_to_string(status_file).ok()?.trim().parse().ok() } fn install_portable(plan: PortableInstallPlan) -> Result<(), String> { + install_portable_inner(&plan) + } + + fn install_portable_inner(plan: &PortableInstallPlan) -> Result<(), String> { log_line(&plan.log, "waiting for the tty7 GUI to exit"); if let Err(error) = wait_for_exit(plan.parent_pid) { - return recover_without_replacement(&plan.log, &plan.install_dir, &plan.stage, error); + return recover_without_replacement( + &plan.log, + &plan.install_dir, + &plan.stage, + plan.result_file.as_deref(), + &plan.expected_version, + error, + ); } let payload = plan.stage.join(PORTABLE_PAYLOAD_DIR); if let Err(error) = remove_path(&payload) { - return recover_without_replacement(&plan.log, &plan.install_dir, &plan.stage, error); + return recover_without_replacement( + &plan.log, + &plan.install_dir, + &plan.stage, + plan.result_file.as_deref(), + &plan.expected_version, + error, + ); } log_line( &plan.log, @@ -758,7 +1817,14 @@ mod windows { &plan.expected_version, &payload, ) { - return recover_without_replacement(&plan.log, &plan.install_dir, &plan.stage, error); + return recover_without_replacement( + &plan.log, + &plan.install_dir, + &plan.stage, + plan.result_file.as_deref(), + &plan.expected_version, + error, + ); } log_line( @@ -770,10 +1836,25 @@ mod windows { // immediately, and a guard naming a dead writer holds nothing. tty7_core::daemon::update_guard::hold(); if let Err(error) = stop_daemon_from_payload(&payload, &plan.install_dir) { - return recover_without_replacement(&plan.log, &plan.install_dir, &plan.stage, error); + return recover_without_replacement( + &plan.log, + &plan.install_dir, + &plan.stage, + plan.result_file.as_deref(), + &plan.expected_version, + error, + ); } log_line(&plan.log, "replacing the tty7 Windows portable files"); + let report = |result: &Result<(), String>| { + report_outcome( + plan.result_file.as_deref(), + &plan.log, + &plan.expected_version, + result, + ); + }; let result = replace_portable_and_relaunch( &plan.install_dir, &payload, @@ -782,6 +1863,7 @@ mod windows { launch_app(directory) }, launch_app, + &report, ); if let Err(error) = &result { log_line(&plan.log, error); @@ -796,12 +1878,19 @@ mod windows { log: &Path, install_dir: &Path, stage: &Path, + result_file: Option<&Path>, + version: &str, error: String, ) -> Result<(), String> { log_line(log, &error); + let result = Err(error); + // The outcome lands before the old app does: the relaunched GUI + // merges it into the update state at startup, and a write afterward + // races that merge (#540). + report_outcome(result_file, log, version, &result); let _ = launch_app(install_dir); queue_cleanup(install_dir, stage); - Err(error) + result } fn verify_update( @@ -1168,11 +2257,16 @@ mod windows { payload: &Path, activate_replacement: impl Fn(&Path) -> Result<(), String>, relaunch_previous: impl Fn(&Path) -> Result<(), String>, + report: &impl Fn(&Result<(), String>), ) -> Result<(), String> { // A unique backup inside the portable directory is on the same volume // as every managed path, so moving old files aside does not degrade to // a cross-volume copy. Keep it explicitly: if rollback itself fails, // dropping a TempDir must never delete the only remaining old binary. + // + // Every failure arm reports its outcome before relaunching the + // previous app: the relaunched GUI merges the outcome at startup, and + // a write afterward races that merge (#540). let backup = match tempfile::Builder::new() .prefix(".tty7-update-backup-") .tempdir_in(install_dir) @@ -1186,6 +2280,7 @@ mod windows { // The daemon has already stopped, but no installed files have // moved yet. Restore GUI availability before returning the // backup error so every post-shutdown failure recovers alike. + report(&Err(cause.clone())); return Err(with_relaunch_failure(cause, relaunch_previous(install_dir))); } }; @@ -1196,6 +2291,7 @@ mod windows { if let Err(error) = fs::write(backup.join(PORTABLE_BACKUP_INCOMPLETE), b"") { let cause = format!("marking the update backup {}: {error}", backup.display()); let _ = remove_path(&backup); + report(&Err(cause.clone())); return Err(with_relaunch_failure(cause, relaunch_previous(install_dir))); } @@ -1211,6 +2307,7 @@ mod windows { "moving {} into the update backup: {error}", current.display() ); + report(&Err(cause.clone())); let restore = restore_moved_roots(install_dir, &backup, &moved); let relaunch = relaunch_previous(install_dir); if restore.is_ok() { @@ -1227,11 +2324,23 @@ mod windows { .filter(|(source, _)| source.exists()) .try_for_each(|(source, destination)| copy_path(&source, &destination)); if let Err(error) = copy_result { - return rollback_portable_failure(install_dir, &backup, error, &relaunch_previous); + return rollback_portable_failure( + install_dir, + &backup, + error, + &relaunch_previous, + report, + ); } if let Err(error) = activate_replacement(install_dir) { - return rollback_portable_failure(install_dir, &backup, error, &relaunch_previous); + return rollback_portable_failure( + install_dir, + &backup, + error, + &relaunch_previous, + report, + ); } // The replacement survived its launch grace period. Old managed files @@ -1241,6 +2350,7 @@ mod windows { // it is finished business the next launch may discard on its own. let _ = fs::remove_file(backup.join(PORTABLE_BACKUP_INCOMPLETE)); let _ = remove_path(&backup); + report(&Ok(())); Ok(()) } @@ -1249,7 +2359,9 @@ mod windows { backup: &Path, cause: String, relaunch_previous: &impl Fn(&Path) -> Result<(), String>, + report: &impl Fn(&Result<(), String>), ) -> Result<(), String> { + report(&Err(cause.clone())); let restore = restore_portable_backup(install_dir, backup); let relaunch = relaunch_previous(install_dir); if restore.is_ok() { @@ -1415,6 +2527,12 @@ mod windows { } } + /// How long a parent that can only be *observed*, not waited on, is + /// given to finish quitting. See `wait_for_exit_across_accounts`: past + /// this, a recycled pid is indistinguishable from a process that never + /// exits, and failing closed beats installing over locked files. + const CROSS_ACCOUNT_EXIT_WAIT: Duration = Duration::from_secs(120); + fn wait_for_exit(pid: u32) -> Result<(), String> { // Opening the handle before the GUI exits makes PID reuse irrelevant: // the kernel handle continues to name the original process object. @@ -1424,6 +2542,9 @@ mod windows { if error == ERROR_INVALID_PARAMETER { return Ok(()); } + if error == ERROR_ACCESS_DENIED { + return wait_for_exit_across_accounts(pid); + } return Err(format!("opening parent process {pid}: OS error {error}")); } let handle = OwnedHandle(handle); @@ -1437,6 +2558,34 @@ mod windows { Ok(()) } + /// The wait when the parent's process object refuses this account a + /// handle: under an over-the-shoulder elevation the chain runs as the + /// administrator, and the signed-in user's GUI answers its `OpenProcess` + /// with `ERROR_ACCESS_DENIED` — the same boundary `pid_alive` documents + /// from the watcher's side. The pid is still observable across it, so + /// the wait degrades to polling the pid until it stops answering. + /// + /// Bounded where the handle wait is not: without a handle, a pid + /// recycled after the GUI exited cannot be told from a GUI that never + /// exits, and the GUI was already quitting when this process was + /// spawned. Running out the budget fails the attempt closed — the + /// recovery path reports it and the watcher brings the app back — + /// rather than letting Setup fight a window that may still hold locks. + fn wait_for_exit_across_accounts(pid: u32) -> Result<(), String> { + let deadline = Instant::now() + CROSS_ACCOUNT_EXIT_WAIT; + while pid_alive(pid) { + if Instant::now() >= deadline { + return Err(format!( + "parent process {pid} was still running {} seconds after the \ + install began", + CROSS_ACCOUNT_EXIT_WAIT.as_secs() + )); + } + thread::sleep(WATCH_POLL); + } + Ok(()) + } + struct OwnedHandle(HANDLE); impl Drop for OwnedHandle { @@ -1737,6 +2886,126 @@ mod windows { assert!(arguments.contains(&expected)); } + #[test] + fn tail_options_parse_named_arguments_in_any_order() { + let options = tail_options( + [ + OsString::from("--result-file"), + OsString::from(r"C:\config\update-outcome.json"), + OsString::from("--config-dir"), + OsString::from(r"C:\config"), + ] + .into_iter(), + ) + .unwrap(); + assert_eq!(options.config_dir.as_deref(), Some(Path::new(r"C:\config"))); + assert_eq!( + options.result_file.as_deref(), + Some(Path::new(r"C:\config\update-outcome.json")) + ); + + let none = tail_options(Vec::new().into_iter()).unwrap(); + assert!(none.config_dir.is_none() && none.result_file.is_none()); + + // Anything unrecognized — including the positionals of an old or + // new caller whose plans do not match this build — stays an error. + assert!(tail_options([OsString::from("--surprise")].into_iter()).is_err()); + assert!(tail_options([OsString::from("stray")].into_iter()).is_err()); + // A flag missing its value is an error, not an empty path. + assert!(tail_options([OsString::from("--config-dir")].into_iter()).is_err()); + } + + /// The watcher relaunches the app on every way out, so the one thing + /// it must never get wrong is "is that GUI still on screen" — the + /// answer that keeps a declined prompt from being handed a second + /// window. + #[test] + fn a_live_gui_is_never_mistaken_for_a_finished_one() { + // Nothing named: nothing to collide with, so a relaunch goes ahead. + let unnamed = GuiProcess::open(None); + assert!(!unnamed.alive()); + assert!(unnamed.wait_for_exit(Duration::from_millis(0))); + + // This process is alive and staying: "still running", which is + // what suppresses the relaunch. + let own = GuiProcess::open(Some(std::process::id())); + assert!(own.alive()); + assert!(!own.wait_for_exit(Duration::from_millis(0))); + + // The case the relaunch actually depends on: a handle taken while + // the process was alive keeps naming it, and reports the exit + // afterwards however the pid is reused. + let mut child = Command::new("ping") + .args(["-n", "30", "127.0.0.1"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("ping ships with Windows"); + let gui = GuiProcess::open(Some(child.id())); + assert!(gui.alive()); + child.kill().unwrap(); + let _ = child.wait(); + assert!(gui.wait_for_exit(Duration::from_secs(5))); + } + + /// The elevated stage pins the helper it is about to run against the + /// installed one, so the directory that comparison reads must come + /// from this process's own image. Taken from `` instead, + /// both sides of the comparison would belong to whoever wrote the + /// command line — and the caller sits below the integrity boundary. + #[test] + fn the_installed_root_is_the_running_image_not_an_argument() { + let exe = std::env::current_exe().unwrap(); + assert_eq!(installed_root().unwrap(), exe.parent().unwrap()); + + // Only the log ever asks this, so case is all it has to forgive. + assert!(same_directory( + Path::new(r"C:\Program Files\tty7"), + Path::new(r"c:\program files\TTY7") + )); + assert!(!same_directory( + Path::new(r"C:\Program Files\tty7"), + Path::new(r"C:\Users\mallory\tty7") + )); + } + + #[test] + fn report_outcome_records_the_terminal_result_for_the_next_gui() { + let root = tempfile::tempdir().unwrap(); + let outcome_path = root.path().join("update-outcome.json"); + let log = root.path().join("update.log"); + + report_outcome(None, &log, "27.0.0", &Ok(())); + assert!( + !outcome_path.exists(), + "a caller without --result-file gets today's behavior" + ); + + report_outcome(Some(&outcome_path), &log, "27.0.0", &Ok(())); + let outcome = tty7_core::daemon::install::outcome::read_outcome(&outcome_path).unwrap(); + assert_eq!( + outcome, + Some(tty7_core::daemon::install::outcome::UpdateOutcome { + version: "27.0.0".to_string(), + ok: true, + detail: None, + }) + ); + + let failure: Result<(), String> = Err("the installer exited with code 5".to_string()); + report_outcome(Some(&outcome_path), &log, "27.0.0", &failure); + let outcome = tty7_core::daemon::install::outcome::read_outcome(&outcome_path).unwrap(); + assert_eq!( + outcome, + Some(tty7_core::daemon::install::outcome::UpdateOutcome { + version: "27.0.0".to_string(), + ok: false, + detail: Some("the installer exited with code 5".to_string()), + }) + ); + } + #[test] fn archive_verification_rejects_tampered_installer_bytes() { let root = tempfile::tempdir().unwrap(); @@ -1908,6 +3177,7 @@ mod windows { Ok(()) }, |_| panic!("the previous version must not relaunch after success"), + &|_| {}, ) .unwrap(); @@ -1953,6 +3223,7 @@ mod windows { Ok(()) }, |_| panic!("the previous version must not relaunch after success"), + &|_| {}, ) .unwrap(); @@ -1978,15 +3249,21 @@ mod windows { fs::write(payload.path().join("tty7-app.exe"), b"new app").unwrap(); fs::write(payload.path().join("tty7.exe"), b"new cli").unwrap(); let relaunched = Cell::new(0usize); + let reported = Cell::new(false); let error = replace_portable_and_relaunch( install.path(), payload.path(), |_| Err("the new app exited immediately".to_string()), |_| { + // The outcome must already be on disk when the previous + // app comes back — the relaunched GUI reads it at startup + // (#540). + assert!(reported.get(), "the outcome is reported first"); relaunched.set(relaunched.get() + 1); Ok(()) }, + &|_| reported.set(true), ) .unwrap_err(); @@ -2023,6 +3300,7 @@ mod windows { relaunched.set(relaunched.get() + 1); Ok(()) }, + &|_| {}, ) .unwrap_err(); diff --git a/src/core/update.rs b/src/core/update.rs index 9db1206d..bfdcf63d 100644 --- a/src/core/update.rs +++ b/src/core/update.rs @@ -103,9 +103,29 @@ pub struct AvailableUpdate { pub version: String, pub installable: bool, pub install_hint: Option, + /// Windows, all-users Inno layout: installable, but the install raises + /// one UAC prompt (#504). The dialog says so up front — a prompt the + /// user was told about is consent; one that appears uninvited is not. + #[cfg(target_os = "windows")] + pub needs_elevation: bool, asset: Option, } +impl AvailableUpdate { + /// Whether installing this package raises a UAC prompt. A method rather + /// than the field so dialog code stays cross-platform: no other layout + /// ever elevates. + #[cfg(target_os = "windows")] + fn needs_elevation(&self) -> bool { + self.needs_elevation + } + + #[cfg(not(target_os = "windows"))] + fn needs_elevation(&self) -> bool { + false + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub enum UpdateInstallHint { #[cfg(target_os = "macos")] @@ -323,6 +343,8 @@ fn spawn_check_inner(report_failure: bool, cx: &mut App) { version: version.clone(), installable: selection.asset.is_some(), install_hint: selection.reason, + #[cfg(target_os = "windows")] + needs_elevation: selection.needs_elevation, asset: selection.asset, }; log::info!("update available: {version} (running {current})"); @@ -446,9 +468,16 @@ fn prompt_update(update: &AvailableUpdate, window: &mut Window, cx: &mut App) { ("current", current_version()), ], ); - match hint.as_deref() { - Some(note) => format!("{base} {note}"), - None => base, + // The elevation warning outranks any other note: it is the one thing + // the user must know *before* the app quits, because the next thing + // they see is a UAC prompt (#504). + if update.needs_elevation() { + format!("{base} {}", t(L10nKey::UpdateDialogNeedsElevation)) + } else { + match hint.as_deref() { + Some(note) => format!("{base} {note}"), + None => base, + } } } else { t_fmt( @@ -470,11 +499,17 @@ fn prompt_update(update: &AvailableUpdate, window: &mut Window, cx: &mut App) { // people should have to reach for — it lives in Settings, not one stray // keystroke away. let buttons: Vec = if update.installable { - vec![ - gpui::PromptButton::ok(t(L10nKey::SettingsUpdateAndRelaunch)), - gpui::PromptButton::ok(t(L10nKey::UpdateDialogNextLaunch)), - gpui::PromptButton::cancel(t(L10nKey::UpdateDialogLater)), - ] + let mut buttons = vec![gpui::PromptButton::ok(t( + L10nKey::SettingsUpdateAndRelaunch, + ))]; + // An unattended next-launch install is a promise an elevation-needing + // package cannot keep: nobody is there to answer UAC before the first + // window (#504). The only honest offer is "now". + if !update.needs_elevation() { + buttons.push(gpui::PromptButton::ok(t(L10nKey::UpdateDialogNextLaunch))); + } + buttons.push(gpui::PromptButton::cancel(t(L10nKey::UpdateDialogLater))); + buttons } else { vec![ gpui::PromptButton::ok(t(L10nKey::SettingsUpdateViewRelease)), @@ -491,12 +526,15 @@ fn prompt_update(update: &AvailableUpdate, window: &mut Window, cx: &mut App) { let update = update.clone(); cx.spawn(async move |cx| { let installable = update.installable; + // With elevation in play, button 1 *is* "Later" — only a layout that + // offered "next launch" may treat index 1 as that answer. + let offered_next_launch = installable && !update.needs_elevation(); match answer.await { Ok(0) if installable => { cx.update(install_available); } Ok(0) => open_releases_page(), - Ok(1) if installable => { + Ok(1) if offered_next_launch => { cx.update(|cx| stage_for_next_launch(update, cx)); } // "Later" — index 1 or 2 depending on the shape — plus a dropped @@ -595,6 +633,15 @@ pub fn install_available(cx: &mut App) { /// is the option that costs the user nothing: no interrupted work now, and no /// decision to make later either. pub fn stage_for_next_launch(update: AvailableUpdate, cx: &mut App) { + #[cfg(target_os = "windows")] + if update.needs_elevation { + // "Next launch" promises an unattended install, which an + // elevation-needing package cannot keep — there is nobody to answer a + // UAC prompt before the first window exists (#504). Offer the one + // path that works instead: install now, with the user present. + install_available(cx); + return; + } APPLY_ON_LAUNCH.store(true, Ordering::Relaxed); // Overrides a pending "install as soon as it lands": choosing next launch // is choosing not to be restarted now. @@ -803,6 +850,10 @@ fn spawn_progress_pump(cx: &mut App) { fn launch_pending(pending: PendingUpdate, cx: &mut App) { update_status(cx, |status| status.phase = UpdatePhase::Installing); + #[cfg(target_os = "windows")] + if pending.needs_elevation { + return launch_pending_elevated(pending, cx); + } match pending.launch() { Ok(()) => { // The updater owns the staging directory from here. Forgetting it @@ -824,6 +875,60 @@ fn launch_pending(pending: PendingUpdate, cx: &mut App) { } } +/// Starts the elevated install chain (#504): a de-elevated watcher that will +/// relaunch the app, then a single UAC prompt covering the two privileged +/// stages. +/// +/// Success looks exactly like a plain install from here — this process quits +/// and the watcher, not the GUI, owns the relaunch. Declining the UAC prompt +/// is not a failure: nothing ever ran elevated, nothing needs recording, and +/// the staged package is still usable, so the plan stays put and the app +/// stays up. Settings still shows it as ready to install. +#[cfg(target_os = "windows")] +fn launch_pending_elevated(pending: PendingUpdate, cx: &mut App) { + // ShellExecuteEx cannot run on the UI thread: raising the UAC prompt + // pumps this thread's message loop (the shell broadcasts change + // notifications through the window), which re-enters gpui while its App + // is borrowed and aborts the process on a RefCell double-borrow. The + // whole launch — watcher spawn included, so the pairing stays atomic — + // runs on a background thread; only the bookkeeping comes back. + let launch = cx.background_executor().spawn(smol::unblock(move || { + let version = pending.version.clone(); + (pending.launch_elevated(), version) + })); + cx.spawn(async move |cx| { + let (result, version) = launch.await; + cx.update(|cx| match result { + Ok(ElevationStart::Started) => { + // The watcher owns the staging directory from here. Forgetting + // it now is what stops a relaunch from trying to install it + // twice. + let mut state = UpdateState::load(); + state.pending = None; + state.last_prompted = None; + state.save(); + cx.quit(); + } + Ok(ElevationStart::Declined) => { + log::info!( + "administrator approval for the {version} update was declined; \ + the package stays staged" + ); + update_status(cx, |status| status.phase = UpdatePhase::Idle); + } + Err(error) => { + let detail = format!("{error:#}"); + log::error!("could not start the elevated install: {detail}"); + record_failure(&version, &detail, cx); + update_status(cx, |status| { + status.phase = UpdatePhase::Failed(UpdateFailure::Launch(detail)); + }); + } + }) + }) + .detach(); +} + /// Records a failure where the user can still find it tomorrow, and lets the /// version prompt again. /// @@ -906,6 +1011,19 @@ pub fn apply_pending_at_launch() -> bool { return false; } + #[cfg(target_os = "windows")] + if pending.needs_elevation { + // An unattended start has nobody to approve a UAC prompt, and raising + // one before the first window — unsigned path and all — reads as + // malware. The plan stays staged; once the app is up, Settings offers + // the install with the window behind it (#504). + log::info!( + "the staged {} update needs administrator approval; leaving it for an interactive install", + pending.version + ); + return false; + } + log::info!( "applying the staged {} update before startup", pending.version @@ -934,6 +1052,87 @@ pub fn apply_pending_at_launch() -> bool { } } +/// Folds the outcome the updater recorded for the install attempt that +/// produced this launch into the on-disk update state, then removes the file. +/// +/// This is what a failed install used to lack (#540): the GUI quits as soon +/// as the helper is spawned, so without the outcome file a failure lived only +/// in `update.log` — and because launching the helper had already cleared the +/// prompt state, the next check simply offered the same version again. Runs +/// before any window exists; `spawn_check`'s hydration carries the result +/// into Settings. +pub fn absorb_update_outcome_at_launch() { + use tty7_core::daemon::install::outcome::{UpdateOutcome, read_outcome}; + + let Some(path) = update_outcome_path() else { + return; + }; + let outcome = match read_outcome(&path) { + Ok(Some(outcome)) => outcome, + Ok(None) => return, + // A result that exists but cannot be read is itself a result: an + // updater ran, and what it left is unusable. + Err(error) => UpdateOutcome { + version: current_version().to_string(), + ok: false, + detail: Some(format!( + "the update result at {} could not be read: {error}", + path.display() + )), + }, + }; + // Consumed either way: an outcome describes the attempt that already ran, + // never the next one. + let _ = std::fs::remove_file(&path); + + let mut state = UpdateState::load(); + if outcome.ok { + if outcome.version == current_version() { + log::info!("the update to {} completed", outcome.version); + // A failure recorded by an earlier attempt at this same version + // is finished business now. + if state + .last_failure + .as_ref() + .is_some_and(|failure| failure.version == outcome.version) + { + state.last_failure = None; + state.save(); + } + } else { + // The helper said the install went in, yet this process is a + // different version — someone reinstalled by hand in between. + // Nothing to show, but the log should have it. + log::warn!( + "the updater reported installing {} but this is {}; ignoring the stale outcome", + outcome.version, + current_version() + ); + } + return; + } + + let detail = outcome + .detail + .unwrap_or_else(|| "the update failed without recording a reason".to_string()); + log::error!("the update to {} failed: {detail}", outcome.version); + state.last_failure = Some(FailureRecord { + version: outcome.version.clone(), + detail, + }); + // A failure must not retire the version — `last_prompted` set with no + // `remind_after` is "one failed install retires it for good", the + // invariant `a_failure_lets_the_version_prompt_again` pins. But this + // attempt already restarted the app once, and `spawn_check` runs at every + // launch: asking again seconds later is the nag loop #540 is about. The + // middle course is the one "Later" already uses — keep the version marked + // as asked, and push the next ask out by `REMIND_LATER`. The failure sits + // in Settings the whole while. + state.last_prompted = Some(outcome.version); + state.remind_after = Some(now_secs() + REMIND_LATER.as_secs()); + state.save(); +} + /// Removes staging directories belonging to a run that died before its updater /// could clean up — quitting mid-download is the usual cause. /// @@ -1150,6 +1349,13 @@ fn human_bytes(bytes: u64) -> String { format!("{:.1} MB", bytes as f64 / MB) } +/// The shape of a persisted [`PendingUpdate`]. A plan written by a build +/// whose updater invocation differs from this one's is discarded rather than +/// launched: its staged helper is the *old* build's updater and would not +/// understand this build's arguments. Absent from pre-parameterization +/// plans, which deserialize as 0 and never match. +const PLAN_VERSION: u32 = 2; + /// A downloaded, verified package waiting to be installed. /// /// Splitting "fetch" from "install" is the point of the whole design: it turns @@ -1166,18 +1372,33 @@ pub struct PendingUpdate { /// package that was merely fetched ahead of time: it waits in Settings. #[serde(default)] pub apply_on_launch: bool, + /// See [`PLAN_VERSION`]. + #[serde(default)] + plan_version: u32, updater: PathBuf, command: String, rest: Vec, config_dir: Option, stage: PathBuf, + /// Windows, all-users Inno layout: install through the elevated chain + /// (#504) rather than by spawning the staged helper directly. + #[serde(default)] + needs_elevation: bool, + /// The staged package's digest as the release server published it. The + /// elevated chain's trust anchor: the checksums file beside the package + /// cannot serve there, because a medium-integrity process can rewrite + /// both together. + #[serde(default)] + expected_sha256: Option, } impl PendingUpdate { - /// Whether the package is still on disk. Staging lives in a temporary - /// directory that a cleaner, an antivirus, or a reboot may have taken. + /// Whether the package is still on disk and still speaks this build's + /// updater protocol. Staging lives in a temporary directory that a + /// cleaner, an antivirus, or a reboot may have taken; the plan version + /// is the other half — see [`PLAN_VERSION`]. pub fn is_usable(&self) -> bool { - self.updater.is_file() && self.stage.is_dir() + self.plan_version == PLAN_VERSION && self.updater.is_file() && self.stage.is_dir() } fn launch(&self) -> Result<()> { @@ -1187,9 +1408,284 @@ impl PendingUpdate { rest: self.rest.clone(), config_dir: self.config_dir.clone(), stage: self.stage.clone(), + needs_elevation: self.needs_elevation, + expected_sha256: self.expected_sha256.clone(), } .launch() } + + /// The pieces of a staged Inno plan that the elevated launch needs, read + /// out of `rest` by position — the order `prepare_windows_update` pushes + /// them. A plan that cannot name every piece is not elevated. + #[cfg(target_os = "windows")] + fn elevated_parts(&self) -> Option { + if !self.needs_elevation || self.command != "install" || self.rest.len() != 7 { + return None; + } + Some(ElevatedPlanParts { + installer: self.rest[0].clone(), + asset_name: self.rest[2].to_str()?.to_string(), + install_dir: self.rest[3].clone(), + version: self.rest[4].to_str()?.to_string(), + log: self.rest[5].clone(), + stage: self.rest[6].clone(), + expected_sha256: self.expected_sha256.clone()?, + }) + } +} + +/// See [`PendingUpdate::elevated_parts`]. +#[cfg(target_os = "windows")] +struct ElevatedPlanParts { + installer: PathBuf, + asset_name: String, + install_dir: PathBuf, + version: String, + log: PathBuf, + stage: PathBuf, + expected_sha256: String, +} + +/// How asking Windows to start the privileged half ended. Only `Started` +/// hands the machine to elevated processes; `Declined` means the user +/// answered the UAC prompt with "no" and nothing ever ran elevated. +#[cfg(target_os = "windows")] +enum ElevationStart { + Started, + Declined, +} + +#[cfg(target_os = "windows")] +impl PendingUpdate { + /// Spawns the de-elevated relaunch watcher, then raises the single UAC + /// prompt that covers both privileged stages (#504). + /// + /// The watcher goes first so a declined prompt leaves exactly one + /// medium-integrity process to reap. The elevated half is always the + /// *installed* updater — the trust root a medium-integrity process cannot + /// rewrite — and everything it needs arrives as arguments, because an + /// elevated child (over-the-shoulder especially) does not inherit this + /// process's environment. + fn launch_elevated(&self) -> Result { + use windows_sys::Win32::Foundation::ERROR_CANCELLED; + + let mut parts = self + .elevated_parts() + .context("the staged package does not describe an elevated install")?; + // The prompt names the installation this process is running from, not + // the one the plan names. `update.json` lives in the user's config + // directory: a plan naming some other directory would aim a UAC + // prompt — and the elevated half's `` — at a binary of + // the plan writer's choosing. + let installed_updater = + bundled_updater().context("tty7-updater.exe is not installed beside this app")?; + let installed_dir = installed_updater + .parent() + .context("the installed updater names no directory")? + .to_path_buf(); + parts.install_dir = installed_dir.clone(); + let Some(status) = update_elevation_status_path() else { + anyhow::bail!("no config directory for the elevation status file"); + }; + // The watcher requires it: without the outcome file the install's + // result would die with the elevated chain (#540). + let Some(outcome) = update_outcome_path() else { + anyhow::bail!("no config directory for the install outcome file"); + }; + let app = std::env::current_exe().context("locating the running app")?; + // Stale markers from an earlier attempt have to be gone: the watcher + // reads both files, and would otherwise act on a dead attempt's + // answer. + let _ = std::fs::remove_file(&status); + let _ = std::fs::remove_file(&outcome); + + let mut watcher = Command::new(&self.updater); + watcher + .arg("relaunch-watcher") + .arg("--status-file") + .arg(&status) + .arg("--result-file") + .arg(&outcome) + .arg("--app-path") + .arg(&app) + .arg("--log") + .arg(&parts.log) + .arg("--expected-version") + .arg(&parts.version) + // So the watcher can tell a GUI that is quitting from one that + // stayed: a declined prompt leaves this process on screen, and + // the relaunch it would otherwise perform on its own timeout + // would put a second window beside it. + .arg("--gui-pid") + .arg(std::process::id().to_string()); + if let Some(dir) = &self.config_dir { + watcher.arg("--config-dir").arg(dir); + } + let mut watcher = tty7_core::core::proc::hide_console(&mut watcher) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .context("launching the relaunch watcher")?; + + let parameters = + elevated_stage_arguments(&parts, &status, &outcome, self.config_dir.as_deref()); + match shell_execute_elevated(&installed_updater, ¶meters, &installed_dir) { + Ok(()) => Ok(ElevationStart::Started), + Err(code) => { + // Whatever the answer was, the watcher must not outlive this + // process when no elevated half is coming: it would spend its + // whole 15-minute grace waiting for a chain that will never + // report, while the app it was to relaunch is still running. + let _ = watcher.kill(); + let _ = std::fs::remove_file(&status); + if code == ERROR_CANCELLED { + return Ok(ElevationStart::Declined); + } + Err(anyhow::anyhow!( + "asking Windows to run the install elevated failed (error {code})" + )) + } + } + } +} + +/// The command line for the privileged first stage, as a single string: +/// ShellExecuteEx hands it to the child's CRT argv splitter verbatim, so +/// every argument is quoted — the install directory typically lives under +/// `C:\Program Files`. Built as an `OsString` so a path that is not valid +/// Unicode survives the round trip. +#[cfg(target_os = "windows")] +fn elevated_stage_arguments( + parts: &ElevatedPlanParts, + status_file: &Path, + outcome: &Path, + config_dir: Option<&Path>, +) -> std::ffi::OsString { + let mut line = std::ffi::OsString::new(); + let mut push = |arg: &std::ffi::OsStr| push_quoted(&mut line, arg); + push(std::ffi::OsStr::new("elevated-stage")); + push(std::ffi::OsStr::new(&std::process::id().to_string())); + push(parts.installer.as_os_str()); + push(std::ffi::OsStr::new(&parts.asset_name)); + push(parts.install_dir.as_os_str()); + push(std::ffi::OsStr::new(&parts.version)); + push(parts.log.as_os_str()); + push(parts.stage.as_os_str()); + push(std::ffi::OsStr::new("--expected-sha256")); + push(std::ffi::OsStr::new(&parts.expected_sha256)); + push(std::ffi::OsStr::new("--status-file")); + push(status_file.as_os_str()); + if let Some(dir) = config_dir { + push(std::ffi::OsStr::new("--config-dir")); + push(dir.as_os_str()); + } + push(std::ffi::OsStr::new("--result-file")); + push(outcome.as_os_str()); + line +} + +/// Appends one argument the way `CommandLineToArgvW` — and the CRT splitter +/// the elevated helper is parsed by — reads back verbatim. +/// +/// Wrapping in quotes is not enough on its own. A backslash is an escape +/// character only in front of a quote, so a path that *ends* in one +/// (`--config-dir "D:\tty7\"`) escapes its own closing quote and swallows the +/// rest of the command line into a single argument; an embedded quote splits +/// one argument into several. Neither can be reached across the integrity +/// boundary — every value here comes from this user's own session — but the +/// receiver runs elevated, and an argument list it can misread is not a thing +/// to leave to the shape of a path. +#[cfg(target_os = "windows")] +fn push_quoted(line: &mut std::ffi::OsString, arg: &std::ffi::OsStr) { + use std::os::windows::ffi::{OsStrExt as _, OsStringExt as _}; + + const QUOTE: u16 = b'"' as u16; + const BACKSLASH: u16 = b'\\' as u16; + + let mut quoted = vec![QUOTE]; + // The run of backslashes immediately behind the cursor: doubled if a + // quote follows it, left alone otherwise. + let mut backslashes = 0usize; + for unit in arg.encode_wide() { + match unit { + BACKSLASH => { + backslashes += 1; + quoted.push(unit); + } + QUOTE => { + quoted.resize(quoted.len() + backslashes, BACKSLASH); + quoted.push(BACKSLASH); + quoted.push(QUOTE); + backslashes = 0; + } + _ => { + backslashes = 0; + quoted.push(unit); + } + } + } + // The closing quote is a quote like any other: the run in front of it has + // to be doubled too. + quoted.resize(quoted.len() + backslashes, BACKSLASH); + quoted.push(QUOTE); + + if !line.is_empty() { + line.push(" "); + } + line.push(std::ffi::OsString::from_wide("ed)); +} + +/// Starts `executable` elevated — the `runas` verb is what turns the request +/// into exactly one UAC prompt — and returns as soon as Windows answers. The +/// elevated process is deliberately not tracked by handle: the status file +/// and the watcher own liveness from here, which is what lets this process +/// quit immediately. +/// +/// The error case carries the raw Windows error code so the caller can tell +/// "the user said no" (`ERROR_CANCELLED`) apart from a genuine launch +/// failure. +#[cfg(target_os = "windows")] +fn shell_execute_elevated( + executable: &Path, + parameters: &std::ffi::OsStr, + working_dir: &Path, +) -> std::result::Result<(), u32> { + use std::os::windows::ffi::OsStrExt as _; + use windows_sys::Win32::Foundation::GetLastError; + use windows_sys::Win32::UI::Shell::{SEE_MASK_FLAG_NO_UI, SHELLEXECUTEINFOW, ShellExecuteExW}; + use windows_sys::Win32::UI::WindowsAndMessaging::SW_HIDE; + + fn wide(value: &std::ffi::OsStr) -> Vec { + value.encode_wide().chain(std::iter::once(0)).collect() + } + + let verb = wide(std::ffi::OsStr::new("runas")); + let file = wide(executable.as_os_str()); + let params = wide(parameters); + let directory = wide(working_dir.as_os_str()); + // SAFETY: zero-initializing is valid for this all-POD struct; every field + // that matters is set below. + let mut info: SHELLEXECUTEINFOW = unsafe { std::mem::zeroed() }; + info.cbSize = std::mem::size_of::() as u32; + // No error dialog from the shell itself: a decline is a normal answer the + // caller handles, and a real failure is reported in-process. + info.fMask = SEE_MASK_FLAG_NO_UI; + info.lpVerb = verb.as_ptr(); + info.lpFile = file.as_ptr(); + info.lpParameters = params.as_ptr(); + info.lpDirectory = directory.as_ptr(); + // The updater is a console program; an elevated console window flashing + // up beside the GUI reads as a crash. + info.nShow = SW_HIDE; + // SAFETY: `info` is fully initialized, and every pointer names a + // NUL-terminated buffer that outlives the call. + if unsafe { ShellExecuteExW(&mut info) } == 0 { + // SAFETY: called immediately after the failed call on the same + // thread, so the error code is the one ShellExecuteExW set. + return Err(unsafe { GetLastError() }); + } + Ok(()) } #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] @@ -1419,6 +1915,8 @@ struct ReleaseAsset { struct AssetSelection { asset: Option, reason: Option, + #[cfg(target_os = "windows")] + needs_elevation: bool, } fn select_release_asset(version: &str, assets: &[GitHubAsset]) -> AssetSelection { @@ -1426,28 +1924,35 @@ fn select_release_asset(version: &str, assets: &[GitHubAsset]) -> AssetSelection } fn select_release_asset_for( - package: Result, + package: Result, assets: &[GitHubAsset], ) -> AssetSelection { - let name = match package { - Ok(name) => name, + let offer = match package { + Ok(offer) => offer, Err(reason) => { return AssetSelection { asset: None, reason: Some(reason), + #[cfg(target_os = "windows")] + needs_elevation: false, }; } }; + let name = offer.name; let Some(asset) = assets.iter().find(|asset| asset.name == name) else { return AssetSelection { asset: None, reason: Some(UpdateInstallHint::MissingPackage(name)), + #[cfg(target_os = "windows")] + needs_elevation: false, }; }; let Some(checksums) = assets.iter().find(|asset| asset.name == "checksums.txt") else { return AssetSelection { asset: None, reason: Some(UpdateInstallHint::MissingChecksums), + #[cfg(target_os = "windows")] + needs_elevation: false, }; }; AssetSelection { @@ -1457,12 +1962,33 @@ fn select_release_asset_for( checksums_url: checksums.browser_download_url.clone(), }), reason: None, + #[cfg(target_os = "windows")] + needs_elevation: offer.needs_elevation, + } +} + +/// The release package this installation can replace itself with. Split from +/// the bare filename so the Windows Inno layout can carry "yes, but the +/// install needs a UAC prompt" alongside it (#504). +struct PackageOffer { + name: String, + #[cfg(target_os = "windows")] + needs_elevation: bool, +} + +impl PackageOffer { + fn plain(name: String) -> Self { + Self { + name, + #[cfg(target_os = "windows")] + needs_elevation: false, + } } } /// The release package this installation can replace itself with, or the /// reason it cannot. -fn package_for_current_install(version: &str) -> Result { +fn package_for_current_install(version: &str) -> Result { #[cfg(target_os = "macos")] { let Some(app) = current_macos_app_bundle() else { @@ -1478,7 +2004,9 @@ fn package_for_current_install(version: &str) -> Result Result false, + WindowsUpdatability::NeedsElevation => true, + WindowsUpdatability::Unsupported(hint) => return Err(hint), + }; if !layout.directory().join("tty7-updater.exe").is_file() { return Err(UpdateInstallHint::UnsupportedWindows); } + // UAC is pointed at the *installed* updater, so the elevated chain is + // only as real as the verbs that binary speaks: one from a release + // before the verbs existed keeps today's answer — the release page. + if needs_elevation && !updater_speaks_elevation(layout.directory()) { + return Err(UpdateInstallHint::WindowsAllUsersInstall); + } return windows_package_for_layout(version, &layout) + .map(|name| PackageOffer { + name, + needs_elevation, + }) .ok_or(UpdateInstallHint::UnsupportedWindows); } #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] @@ -1517,6 +2059,38 @@ fn package_for_current_install(version: &str) -> Result bool { + let updater = install_dir.join("tty7-updater.exe"); + let mut command = Command::new(&updater); + command + .arg("capabilities") + .stdin(Stdio::null()) + .stderr(Stdio::null()); + let output = tty7_core::core::proc::hide_console(&mut command).output(); + let Ok(output) = output else { return false }; + output.status.success() && capabilities_cover_elevation(&output.stdout) +} + +/// The pure half, so the token matching is testable without spawning a real +/// helper. Mirrors `ELEVATION_CAPABILITIES` in the updater. +#[cfg(target_os = "windows")] +fn capabilities_cover_elevation(stdout: &[u8]) -> bool { + let Ok(text) = std::str::from_utf8(stdout) else { + return false; + }; + let tokens: std::collections::HashSet<&str> = text.lines().map(str::trim).collect(); + ["elevated-stage", "install-elevated", "relaunch-watcher"] + .iter() + .all(|capability| tokens.contains(capability)) +} + fn prepare_update( version: &str, asset: &ReleaseAsset, @@ -1557,6 +2131,11 @@ struct PreparedUpdate { rest: Vec, config_dir: Option, stage: PathBuf, + /// Windows, all-users Inno layout: launch through the elevated chain + /// (#504), not by spawning the staged helper directly. + needs_elevation: bool, + /// See [`PendingUpdate::expected_sha256`]. + expected_sha256: Option, } impl PreparedUpdate { @@ -1575,19 +2154,29 @@ impl PreparedUpdate { PendingUpdate { version, apply_on_launch, + plan_version: PLAN_VERSION, updater: self.updater, command: self.command, rest: self.rest, config_dir: self.config_dir, stage: self.stage, + needs_elevation: self.needs_elevation, + expected_sha256: self.expected_sha256, } } fn launch(&self) -> Result<()> { let mut command = Command::new(&self.updater); command.args(self.args()); - if let Some(config_dir) = &self.config_dir { - command.env("TTY7_CONFIG_DIR", config_dir); + let outcome = update_outcome_path(); + for arg in updater_tail_args(self.config_dir.as_deref(), outcome.as_deref()) { + command.arg(arg); + } + if let Some(outcome) = &outcome { + // A result from an earlier attempt has to be gone before this one + // starts: a helper that dies before writing its own would + // otherwise be read as having written *that* one. + let _ = std::fs::remove_file(outcome); } tty7_core::core::proc::hide_console(&mut command) .stdin(Stdio::null()) @@ -1602,6 +2191,43 @@ impl PreparedUpdate { } } +/// Where the updater records how the install ended; the next launch merges it +/// into the update state (`absorb_update_outcome_at_launch`). +fn update_outcome_path() -> Option { + crate::core::config::config_path(tty7_core::daemon::install::outcome::OUTCOME_FILE_NAME) +} + +/// The file the privileged first stage writes its pid into so the de-elevated +/// watcher can tell the chain apart from a UAC prompt nobody has answered +/// yet. It cannot live in the privileged staging directory (the watcher's +/// token may not read there), so it sits next to the outcome file: an +/// administrator can write the user's config directory under every elevation +/// shape, including over-the-shoulder (#504). +#[cfg(target_os = "windows")] +fn update_elevation_status_path() -> Option { + crate::core::config::config_path("update-elevation.status") +} + +/// The named options after the positional arguments. Everything the updater +/// must know about this process's configuration crosses as arguments, never +/// the environment: on Windows an elevated (UAC) child does not inherit the +/// spawning process's environment, so a `TTY7_CONFIG_DIR` set there would +/// fall back to the administrator's config directory exactly in the case +/// that needs the caller's (#504). The updater re-exports the variable for +/// the children it spawns itself. +fn updater_tail_args(config_dir: Option<&Path>, outcome: Option<&Path>) -> Vec { + let mut args = Vec::new(); + if let Some(dir) = config_dir { + args.push(std::ffi::OsString::from("--config-dir")); + args.push(dir.as_os_str().to_os_string()); + } + if let Some(outcome) = outcome { + args.push(std::ffi::OsString::from("--result-file")); + args.push(outcome.as_os_str().to_os_string()); + } + args +} + #[cfg(target_os = "macos")] fn update_staging_dir(parent: &Path) -> Result { tempfile::Builder::new() @@ -1673,6 +2299,8 @@ fn prepare_macos_update( ], config_dir: crate::core::config::config_dir_path(), stage: dir, + needs_elevation: false, + expected_sha256: None, }) } @@ -1688,14 +2316,37 @@ fn prepare_windows_update( // Re-checked here rather than trusting the check that produced the offer: // an installation can be relocated, or its privileges changed, between the // update check and the user pressing the button. - if let Err(hint) = windows_layout_is_updatable(&layout) { + let updatability = windows_layout_updatability(&layout); + let needs_elevation = match updatability { + WindowsUpdatability::Updatable => false, + WindowsUpdatability::NeedsElevation => true, // This error surfaces in Settings as the update failure — the moment // the user most needs to understand it — so it is the localized hint, // not the English one meant for logs (#602). - anyhow::bail!("{}", localized_update_install_hint(&hint)); - } + WindowsUpdatability::Unsupported(hint) => { + anyhow::bail!("{}", localized_update_install_hint(&hint)) + } + }; let install_dir = layout.directory().to_path_buf(); let bundled = bundled_updater().context("tty7-updater.exe is not bundled with this app")?; + + // The digest the elevated chain trusts, taken from the downloaded + // manifest rather than the staged copy of it: the staged file lands in a + // user-writable directory, so it cannot anchor its own verification. + // Computed for both Inno modes — a plan's elevation answer is re-derived + // at launch, and the digest costs one manifest parse. + let expected_sha256 = if matches!(layout, WindowsUpdateLayout::Inno(_)) { + let manifest = + std::str::from_utf8(checksums).context("checksums.txt is not valid UTF-8")?; + Some( + tty7_core::daemon::install::checksums::expected_digest(manifest, asset_name) + .map(|digest| tty7_core::daemon::install::checksums::hex(&digest)) + .context("reading the staged package's digest from checksums.txt")?, + ) + } else { + None + }; + let staging = system_update_staging_dir()?; let dir = staging.path().to_path_buf(); let package = write_staged_asset(&dir, asset_name, package)?; @@ -1759,6 +2410,8 @@ fn prepare_windows_update( ], config_dir: crate::core::config::config_dir_path(), stage: dir, + needs_elevation, + expected_sha256, }) } @@ -1847,22 +2500,36 @@ fn windows_directory_is_writable(directory: &Path) -> bool { .is_ok() } -/// Rejects the Windows installation layouts that cannot be replaced by this -/// process, before anything is downloaded. +/// What this Windows installation layout means for an in-place update. #[cfg(target_os = "windows")] -fn windows_layout_is_updatable(layout: &WindowsUpdateLayout) -> Result<(), UpdateInstallHint> { +#[derive(Clone, Debug, PartialEq, Eq)] +enum WindowsUpdatability { + /// A per-user Inno install, or a writable portable directory: the + /// updater runs as the signed-in user, no prompt involved. + Updatable, + /// An Inno install replacing it needs administrator rights for (#504): + /// the elevated chain handles it — one announced UAC prompt, and the app + /// never runs elevated itself. + NeedsElevation, + Unsupported(UpdateInstallHint), +} + +/// Sorts the Windows installation layouts by what replacing them takes, +/// before anything is downloaded. +#[cfg(target_os = "windows")] +fn windows_layout_updatability(layout: &WindowsUpdateLayout) -> WindowsUpdatability { match layout { WindowsUpdateLayout::Inno(directory) => { if windows_inno_needs_elevation(directory) { - return Err(UpdateInstallHint::WindowsAllUsersInstall); + return WindowsUpdatability::NeedsElevation; } - Ok(()) + WindowsUpdatability::Updatable } WindowsUpdateLayout::Portable(directory) => { if !windows_directory_is_writable(directory) { - return Err(UpdateInstallHint::UnsupportedWindows); + return WindowsUpdatability::Unsupported(UpdateInstallHint::UnsupportedWindows); } - Ok(()) + WindowsUpdatability::Updatable } } } @@ -2106,7 +2773,7 @@ mod tests { fn release_asset_requires_the_platform_package_and_checksums() { let name = "tty7-27.1.0-macos-arm64.zip"; let assets = [github_asset(name), github_asset("checksums.txt")]; - let selected = select_release_asset_for(Ok(name.to_string()), &assets); + let selected = select_release_asset_for(Ok(PackageOffer::plain(name.to_string())), &assets); assert_eq!( selected.asset, Some(ReleaseAsset { @@ -2121,7 +2788,10 @@ mod tests { #[test] fn release_without_checksums_is_never_installable() { let name = "tty7-27.1.0-macos-arm64.zip"; - let selected = select_release_asset_for(Ok(name.to_string()), &[github_asset(name)]); + let selected = select_release_asset_for( + Ok(PackageOffer::plain(name.to_string())), + &[github_asset(name)], + ); assert!(selected.asset.is_none()); assert_eq!(selected.reason, Some(UpdateInstallHint::MissingChecksums)); } @@ -2129,7 +2799,9 @@ mod tests { #[test] fn release_without_the_exact_platform_package_is_never_guessed() { let selected = select_release_asset_for( - Ok("tty7-27.1.0-macos-arm64.zip".to_string()), + Ok(PackageOffer::plain( + "tty7-27.1.0-macos-arm64.zip".to_string(), + )), &[ github_asset("tty7-27.1.0-macos-x86_64.zip"), github_asset("checksums.txt"), @@ -2402,6 +3074,7 @@ mod tests { #[test] fn update_state_round_trips_and_defaults() { + let _lock = UPDATE_STATE_LOCK.lock().unwrap(); crate::core::config::pin_test_config_dir(); let path = UpdateState::path().expect("config dir pinned"); @@ -2422,11 +3095,14 @@ mod tests { pending: Some(PendingUpdate { version: "27.0.0".into(), apply_on_launch: true, + plan_version: PLAN_VERSION, updater: PathBuf::from("/tmp/tty7-updater"), command: "install".into(), rest: vec![PathBuf::from("/tmp/stage/tty7.zip")], config_dir: None, stage: PathBuf::from("/tmp/stage"), + needs_elevation: false, + expected_sha256: None, }), ..Default::default() } @@ -2489,6 +3165,165 @@ mod tests { assert!(should_prompt(&state, "27.0.0")); } + /// Serializes the tests that touch the real `update.json` under the + /// pinned per-process config dir (the pattern `update_guard`'s tests + /// document: one shared state file, parallel tests, one lock). + static UPDATE_STATE_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + #[test] + fn updater_tail_args_carry_config_and_outcome_as_arguments() { + assert_eq!( + updater_tail_args( + Some(Path::new(r"C:\cfg")), + Some(Path::new(r"C:\cfg\update-outcome.json")), + ), + [ + std::ffi::OsString::from("--config-dir"), + std::ffi::OsString::from(r"C:\cfg"), + std::ffi::OsString::from("--result-file"), + std::ffi::OsString::from(r"C:\cfg\update-outcome.json"), + ] + ); + // Each flag stands alone; a machine with no resolvable config dir + // simply passes neither, which is the updater's pre-existing default. + assert!(updater_tail_args(None, None).is_empty()); + assert_eq!( + updater_tail_args(None, Some(Path::new("/tmp/outcome.json"))), + [ + std::ffi::OsString::from("--result-file"), + std::ffi::OsString::from("/tmp/outcome.json"), + ] + ); + } + + /// The whole outcome-file lifecycle in one test: the file and + /// `update.json` are per-process global state, and parallel tests + /// writing both would race each other. + #[test] + fn an_updater_outcome_is_absorbed_exactly_once() { + use tty7_core::daemon::install::outcome::{UpdateOutcome, write_outcome}; + + let _lock = UPDATE_STATE_LOCK.lock().unwrap(); + crate::core::config::pin_test_config_dir(); + let state_path = UpdateState::path().expect("config dir pinned"); + let outcome_path = update_outcome_path().expect("config dir pinned"); + let _ = std::fs::remove_file(&state_path); + let _ = std::fs::remove_file(&outcome_path); + + // No outcome: nothing changes, nothing is invented. + absorb_update_outcome_at_launch(); + let state = UpdateState::load(); + assert!(state.last_failure.is_none() && state.last_prompted.is_none()); + + // A failure lands in the state and — the #540 half — does not set the + // version up to prompt again on its own right away: the throttle is + // `remind_after`, never a retirement. + write_outcome( + &outcome_path, + &UpdateOutcome { + version: "27.0.0".into(), + ok: false, + detail: Some("the installer exited with code 5".into()), + }, + ) + .unwrap(); + absorb_update_outcome_at_launch(); + let mut state = UpdateState::load(); + assert_eq!( + state.last_failure, + Some(FailureRecord { + version: "27.0.0".into(), + detail: "the installer exited with code 5".into(), + }) + ); + assert!(!should_prompt(&state, "27.0.0")); + // The version is throttled, not retired: once the reminder expires the + // question comes back — the same invariant + // `a_failure_lets_the_version_prompt_again` pins for the in-process + // path. + let due = state + .remind_after + .expect("a failed install defers the next prompt rather than retiring it"); + assert!(due > now_secs()); + state.remind_after = Some(now_secs().saturating_sub(1)); + assert!(should_prompt(&state, "27.0.0")); + assert!(!outcome_path.exists(), "the outcome is consumed once"); + + // A later launch finds no file and changes nothing. + absorb_update_outcome_at_launch(); + assert_eq!( + UpdateState::load() + .last_failure + .as_ref() + .map(|f| &f.version), + Some(&"27.0.0".to_string()) + ); + + // A success retires the failure recorded for that same version. The + // two name the running version here because the crate version is the + // only "current" a test can have. + write_outcome( + &outcome_path, + &UpdateOutcome { + version: current_version().to_string(), + ok: false, + detail: Some("first attempt failed".into()), + }, + ) + .unwrap(); + absorb_update_outcome_at_launch(); + assert_eq!( + UpdateState::load() + .last_failure + .as_ref() + .map(|f| &f.version), + Some(¤t_version().to_string()) + ); + write_outcome( + &outcome_path, + &UpdateOutcome { + version: current_version().to_string(), + ok: true, + detail: None, + }, + ) + .unwrap(); + absorb_update_outcome_at_launch(); + assert_eq!(UpdateState::load().last_failure, None); + assert!(!outcome_path.exists()); + + // A success naming some other version is stale (a hand-installed + // rollback in between): dropped, never shown. + write_outcome( + &outcome_path, + &UpdateOutcome { + version: "99.0.0".into(), + ok: true, + detail: None, + }, + ) + .unwrap(); + absorb_update_outcome_at_launch(); + assert_eq!(UpdateState::load().last_failure, None); + + // Garbage is an outcome too: a failure with an odd detail beats + // silence about an attempt that definitely ran. + std::fs::write(&outcome_path, b"not json").unwrap(); + absorb_update_outcome_at_launch(); + let state = UpdateState::load(); + assert!( + state + .last_failure + .as_ref() + .is_some_and(|failure| failure.detail.contains("could not be read")), + "{:?}", + state.last_failure + ); + assert!(!outcome_path.exists()); + + let _ = std::fs::remove_file(&state_path); + } + #[test] fn only_our_own_staging_directories_are_swept() { assert!(is_stage_name(".tty7-update-abc123")); @@ -2506,6 +3341,8 @@ mod tests { rest: vec![PathBuf::from("/tmp/stage/tty7.zip")], config_dir: None, stage: PathBuf::from("/tmp/stage"), + needs_elevation: false, + expected_sha256: None, }; let args = prepared.args(); assert_eq!(args[0], PathBuf::from("install")); @@ -2607,7 +3444,10 @@ mod tests { // A writable temp directory is never the all-users installation, so // this layout is offered the normal in-place update. - assert_eq!(windows_layout_is_updatable(&layout), Ok(())); + assert_eq!( + windows_layout_updatability(&layout), + WindowsUpdatability::Updatable + ); assert_eq!( select_release_asset_for(Err(UpdateInstallHint::WindowsAllUsersInstall), &[]).reason, @@ -2625,15 +3465,15 @@ mod tests { let directory = root.path().to_path_buf(); assert!(windows_directory_is_writable(&directory)); assert_eq!( - windows_layout_is_updatable(&WindowsUpdateLayout::Portable(directory)), - Ok(()) + windows_layout_updatability(&WindowsUpdateLayout::Portable(directory)), + WindowsUpdatability::Updatable ); let missing = root.path().join("gone"); assert!(!windows_directory_is_writable(&missing)); assert_eq!( - windows_layout_is_updatable(&WindowsUpdateLayout::Portable(missing)), - Err(UpdateInstallHint::UnsupportedWindows) + windows_layout_updatability(&WindowsUpdateLayout::Portable(missing)), + WindowsUpdatability::Unsupported(UpdateInstallHint::UnsupportedWindows) ); } @@ -2654,4 +3494,197 @@ mod tests { path.display() ); } + + /// A plan persisted by a build from before the tail-argument protocol + /// cannot be carried out by its own staged helper: it relied on the + /// environment, which an elevated child never inherits (#504). The plan + /// version is what quietly drops those plans at the next launch instead + /// of failing weirdly against a helper that does not understand its + /// arguments. + #[test] + fn a_plan_from_another_protocol_version_is_not_usable() { + let root = tempfile::tempdir().unwrap(); + let updater = root.path().join("tty7-updater"); + std::fs::write(&updater, b"test updater").unwrap(); + let stage = root.path().join("stage"); + std::fs::create_dir(&stage).unwrap(); + + let plan = |plan_version| PendingUpdate { + version: "27.0.0".into(), + apply_on_launch: true, + plan_version, + updater: updater.clone(), + command: "install".into(), + rest: vec![stage.join("tty7.zip")], + config_dir: None, + stage: stage.clone(), + needs_elevation: false, + expected_sha256: None, + }; + assert!(plan(PLAN_VERSION).is_usable()); + // Plans written before the field existed deserialize it as 0. + assert!(!plan(0).is_usable()); + assert!(!plan(PLAN_VERSION + 1).is_usable()); + } + + #[cfg(target_os = "windows")] + #[test] + fn an_elevation_capable_updater_is_recognised_by_its_own_answer() { + assert!(capabilities_cover_elevation( + b"elevated-stage\ninstall-elevated\nrelaunch-watcher\n" + )); + // Order and unrelated tokens are the updater's business. + assert!(capabilities_cover_elevation( + b"install\nrelaunch-watcher\nelevated-stage\ninstall-elevated\n" + )); + // An old updater exits with a usage error: no tokens, no chain, and + // the install falls back to pointing at the release page. + assert!(!capabilities_cover_elevation( + b"usage: tty7-updater [args]" + )); + assert!(!capabilities_cover_elevation(b"")); + // Two of the three verbs is not the chain. + assert!(!capabilities_cover_elevation( + b"elevated-stage\ninstall-elevated\n" + )); + } + + #[cfg(target_os = "windows")] + #[test] + fn an_elevated_plan_names_every_piece_the_chain_needs() { + let digest = "ab".repeat(32); + let stage = PathBuf::from(r"C:\Users\someone\AppData\Local\Temp\tty7-update-x"); + let plan = |needs_elevation, expected_sha256: Option<&str>| PendingUpdate { + version: "27.0.0".into(), + apply_on_launch: false, + plan_version: PLAN_VERSION, + updater: stage.join("tty7-updater.exe"), + command: "install".into(), + // The order `prepare_windows_update` pushes: installer, checksums, + // asset name, install dir, version, log, stage. + rest: vec![ + stage.join("tty7-27.0.0-windows-x86_64-setup.exe"), + stage.join("checksums.txt"), + PathBuf::from("tty7-27.0.0-windows-x86_64-setup.exe"), + PathBuf::from(r"C:\Program Files\tty7"), + PathBuf::from("27.0.0"), + stage.join("update.log"), + stage.clone(), + ], + config_dir: Some(PathBuf::from(r"C:\Users\someone\.config\tty7")), + stage: stage.clone(), + needs_elevation, + expected_sha256: expected_sha256.map(str::to_string), + }; + + let parts = plan(true, Some(&digest)) + .elevated_parts() + .expect("a complete elevated plan yields its parts"); + assert_eq!( + parts.installer, + stage.join("tty7-27.0.0-windows-x86_64-setup.exe") + ); + assert_eq!(parts.asset_name, "tty7-27.0.0-windows-x86_64-setup.exe"); + assert_eq!(parts.install_dir, PathBuf::from(r"C:\Program Files\tty7")); + assert_eq!(parts.version, "27.0.0"); + assert_eq!(parts.expected_sha256, digest); + + // Not flagged, or flagged but missing the digest that anchors the + // chain's trust, is not an elevated plan at all. + assert!(plan(false, Some(&digest)).elevated_parts().is_none()); + assert!(plan(true, None).elevated_parts().is_none()); + let mut short = plan(true, Some(&digest)); + short.rest.truncate(6); + assert!(short.elevated_parts().is_none()); + } + + #[cfg(target_os = "windows")] + #[test] + fn the_elevated_command_line_quotes_every_argument() { + let parts = ElevatedPlanParts { + installer: PathBuf::from(r"C:\Users\some one\stage\setup.exe"), + asset_name: "tty7-27.0.0-windows-x86_64-setup.exe".into(), + install_dir: PathBuf::from(r"C:\Program Files\tty7"), + version: "27.0.0".into(), + log: PathBuf::from(r"C:\Users\some one\stage\update.log"), + stage: PathBuf::from(r"C:\Users\some one\stage"), + expected_sha256: "ab".repeat(32), + }; + let line = elevated_stage_arguments( + &parts, + Path::new(r"C:\Users\some one\.config\tty7\update-elevation.status"), + Path::new(r"C:\Users\some one\.config\tty7\update-outcome.json"), + Some(Path::new(r"C:\Users\some one\.config\tty7")), + ) + .into_string() + .expect("the test paths are valid Unicode"); + + assert!(line.starts_with("\"elevated-stage\" "), "{line}"); + assert!( + line.contains(&format!("\"{}\"", std::process::id())), + "{line}" + ); + // A space in a path must never reach the CRT splitter unquoted. + assert!(line.contains(r#""C:\Program Files\tty7""#), "{line}"); + assert!( + line.contains(r#""C:\Users\some one\stage\setup.exe""#), + "{line}" + ); + assert!( + line.contains(&format!( + "\"--expected-sha256\" \"{}\"", + parts.expected_sha256 + )), + "{line}" + ); + assert!( + line.contains( + r#""--status-file" "C:\Users\some one\.config\tty7\update-elevation.status""# + ), + "{line}" + ); + assert!( + line.contains(r#""--config-dir" "C:\Users\some one\.config\tty7""#), + "{line}" + ); + assert!( + line.contains( + r#""--result-file" "C:\Users\some one\.config\tty7\update-outcome.json""# + ), + "{line}" + ); + } + + /// The CRT splitter the elevated helper is parsed by treats a backslash + /// as an escape only in front of a quote — so a config directory ending + /// in one would otherwise escape its own closing quote and eat every + /// argument after it, including `--result-file`, which is the file the + /// watcher waits on. + #[cfg(target_os = "windows")] + #[test] + fn a_trailing_backslash_does_not_escape_its_own_closing_quote() { + let quote = |value: &str| { + let mut line = std::ffi::OsString::new(); + push_quoted(&mut line, std::ffi::OsStr::new(value)); + line.into_string().expect("valid Unicode in, valid out") + }; + + assert_eq!( + quote(r"C:\Program Files\tty7"), + r#""C:\Program Files\tty7""# + ); + // Doubled only in front of the closing quote. + assert_eq!(quote(r"D:\tty7\"), r#""D:\tty7\\""#); + assert_eq!(quote(r"D:\tty7\\"), r#""D:\tty7\\\\""#); + // An embedded quote is escaped, and the run in front of it doubled. + assert_eq!(quote(r#"a"b"#), r#""a\"b""#); + assert_eq!(quote(r#"a\"b"#), r#""a\\\"b""#); + // Interior backslashes are literal: doubling them would rename paths. + assert_eq!(quote(r"a\b"), r#""a\b""#); + + let mut line = std::ffi::OsString::new(); + push_quoted(&mut line, std::ffi::OsStr::new("one")); + push_quoted(&mut line, std::ffi::OsStr::new("two")); + assert_eq!(line.into_string().unwrap(), r#""one" "two""#); + } } diff --git a/src/main.rs b/src/main.rs index 7ab093b6..f6cb9a3d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -509,6 +509,10 @@ fn main() { if crate::core::update::apply_pending_at_launch() { return; } + // If this launch *is* the relaunch an updater just performed, its outcome + // file is waiting; fold it into the update state before the first window + // reads it. Also covers the recovery relaunch after a failed install. + crate::core::update::absorb_update_outcome_at_launch(); let (config, config_outcome) = crate::core::config::Config::load_with_outcome(); let gui_language = config.gui_language.clone(); diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 5d2e3ff0..0b98fe5f 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -644,6 +644,9 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::UpdateDialogCannotSelfUpdate => "This installation cannot update itself.", L10nKey::UpdateDialogLater => "Later", L10nKey::UpdateDialogNextLaunch => "Install on Next Launch", + L10nKey::UpdateDialogNeedsElevation => { + "This copy of tty7 is installed for all users, so Windows will ask for administrator approval once before the install begins. tty7 itself never runs elevated — it comes back as you." + } L10nKey::SettingsUpdateCheckFailed => "Could not check for updates: {error}", L10nKey::SettingsUpdatePrepareFailed => "Update failed: {error}", L10nKey::SettingsUpdateLaunchFailed => "Could not start the installer: {error}", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index f1f89957..3e9b3a28 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -651,6 +651,9 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::UpdateDialogCannotSelfUpdate => "このインストールは自動更新できません。", L10nKey::UpdateDialogLater => "後で", L10nKey::UpdateDialogNextLaunch => "次回起動時にインストール", + L10nKey::UpdateDialogNeedsElevation => { + "この tty7 は全ユーザー向けにインストールされているため、インストール開始前に Windows の管理者承認が一度だけ求められます。tty7 自体が管理者権限で実行されることはなく、あなたの権限のまま再起動します。" + } L10nKey::SettingsUpdateCheckFailed => "アップデートを確認できませんでした: {error}", L10nKey::SettingsUpdatePrepareFailed => "アップデートに失敗しました: {error}", L10nKey::SettingsUpdateLaunchFailed => "インストーラーを起動できませんでした: {error}", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 2622a7ea..be7a8a45 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -529,6 +529,7 @@ l10n_keys! { UpdateDialogCannotSelfUpdate, UpdateDialogLater, UpdateDialogNextLaunch, + UpdateDialogNeedsElevation, SettingsUpdateCheckFailed, SettingsUpdatePrepareFailed, SettingsUpdateLaunchFailed, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 6f79fc95..7c190640 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -565,6 +565,9 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::UpdateDialogCannotSelfUpdate => "这份安装无法自行更新。", L10nKey::UpdateDialogLater => "以后再说", L10nKey::UpdateDialogNextLaunch => "下次启动时安装", + L10nKey::UpdateDialogNeedsElevation => { + "这份 tty7 是为所有用户安装的，开始安装前 Windows 会请求一次管理员批准。tty7 本身不会以管理员身份运行——重启后仍以你的身份回来。" + } L10nKey::SettingsUpdateCheckFailed => "无法检查更新：{error}", L10nKey::SettingsUpdatePrepareFailed => "更新失败：{error}", L10nKey::SettingsUpdateLaunchFailed => "无法启动安装程序：{error}",