From b2d73ec68b90fdc467db028dc72d530b58e1870a Mon Sep 17 00:00:00 2001 From: Hongwei Qin <122079993+shihuaidexianyu@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:13:20 +0800 Subject: [PATCH] feat(update): update an all-users Windows install through one UAC prompt (#562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(update): surface a failed install instead of silently re-prompting (#540) The GUI quits as soon as tty7-updater is spawned, so an install that failed inside the helper left a trace only in update.log — and because launching the helper had already cleared the prompt state, the next check offered the same version again, and again. The failure mode the user saw was an app that nagged about an update it could not install. The helper now writes update-outcome.json beside update.json on every terminal path it can still reach, and the next GUI launch folds it into the update state: a failure shows in Settings with the installer's own reason until dismissed and stops the version from re-prompting on its own; a success at the running version retires a failure an earlier attempt recorded. A leftover result that exists but cannot be parsed is reported rather than dropped — something ran, and "unreadable" is a result too. The same change moves the config directory off the environment and onto the command line (--config-dir). An elevated child process does not inherit the spawner's environment, so TTY7_CONFIG_DIR would have fallen back to the administrator's config directory exactly in the over-the-shoulder case — the groundwork this lays for #504. The updater re-exports the variable for the helper children it spawns itself, so the relaunched app keeps answering for the same config directory. * feat(update): update an all-users Windows install through one UAC prompt (#504) An Inno install under C:\Program Files could not be replaced in place: the updater ran the release Setup as the signed-in user, which either installed a second, per-user copy beside the real one or let Inno re-launch itself elevated — a bare UAC prompt for an unsigned executable in %TEMP%, seconds after the GUI had vanished. So the layout was refused outright and told to download by hand. It now updates itself, with the split the design in #504 settled on: one UAC prompt covering two privileged stages, and one watcher that is never elevated at all. - The GUI probes the *installed* updater for the new verbs by running it ("capabilities"), so a side-loaded or downgraded binary answers for itself instead of being trusted by version number. An updater that predates the verbs exits with a usage error, and the install falls back to pointing at the release page exactly as before — the first release carrying this still updates the old way, and the one after it updates itself. - The prompt dialog says the UAC prompt is coming before the app quits, and stops offering "Install on Next Launch": nobody is there to answer a prompt before the first window exists. The same guard keeps a staged plan from being armed for the next launch, and apply_pending_at_launch leaves an elevation-needing plan staged rather than raising a windowless prompt at boot. - "Install now" spawns the watcher first (medium integrity, the signed-in user's token, so the relaunched app is never elevated), then ShellExecuteEx "runas" on the installed updater — the trust root a medium-integrity process cannot rewrite. Everything the elevated half needs crosses as command-line arguments, because an over-the-shoulder child inherits neither the environment nor the user's profile. The package's expected SHA-256 crosses the same way, from the checksums the GUI already holds in memory, so a payload and its checksums file cannot be rewritten together behind the IL boundary. - The privileged first stage re-verifies the payload against that digest, pins its helper byte-for-byte to the installed updater, stages both in a fresh administrator-only %ProgramData% directory (an explicit SDDL DACL, swept of stale directories first), and only then runs the install stage — which runs Setup silently, writes the outcome file, and never touches the app binary itself. The watcher follows the chain through the status file and pid liveness (ERROR_ACCESS_DENIED from OpenProcess still means "alive" across accounts), then relaunches the app de-elevated and probes that it actually came up. - Declining the UAC prompt is not an error: the watcher is reaped, nothing ran elevated, and the staged package simply waits in Settings. Persisted plans from before this protocol serde-default a plan version that is_usable rejects, so a stale plan is discarded instead of failing against a helper that would not understand its arguments. The installer script's explorer-menu registration gains skipifsilent: a silent run *is* this update path, and launching the app there would write the menu into the administrator's hive under over-the-shoulder elevation. One note on the test suite: ui::remote_connect's a_routed_auth_prompt_carries_the_machine_that_raised_it fails under parallel test execution on this machine both with and without this change — a pre-existing flake, unrelated. * fix(update): run the UAC request off the UI thread Real-machine verification of the elevated chain caught this on the first click: ShellExecuteExW pumps the calling thread's message loop while the shell raises the consent prompt (its change notifications re-enter the window), and from the UI thread that re-enters gpui with its App already borrowed — the process aborts on a RefCell double-borrow before anything ever elevates. The launch — watcher spawn included, so the pairing stays atomic — now runs on the background executor, and only the bookkeeping (quit / decline / failure) comes back to the UI thread. * fix(update): throttle a failed version instead of retiring it (#540) Per the review on #540: a failed install must not keep the version retired via last_prompted — record last_prompted plus a fresh remind_after deadline (the same three days "Later" uses), so the version asks again once the reminder expires. should_prompt already treats "last_prompted matches, reminder expired" as prompt-again, so no logic change is needed there, and the pinned a_failure_lets_the_version_prompt_again test still holds. Also write update-outcome.json *before* relaunching the previous app on the macOS/Windows/portable non-elevated paths: the GUI that comes up next is exactly the process that absorbs the outcome, and it used to be relaunched before the failure existed on disk. The elevated chain is unchanged — its watcher already waited for the file. * fix(update): let only the elevated updater's own image name the trust root Three holes on the privileged side of the #504 chain, all of the same shape: a value that decides what runs elevated was taken from the medium-integrity caller. - `elevated-stage` pinned the staged helper against `\tty7-updater.exe`, where `` is a command-line argument. Both halves of that comparison were the caller's to choose: name a directory holding two copies of any binary and the pin passes, then stage 2 runs it elevated. The stage now derives the installation from its own image — UAC pointed the prompt at `{app}\tty7-updater.exe`, so `current_exe` is the one path nothing below the boundary could have written — and passes that on to stage 2. A caller that named a different directory only gets a line in the log. - The staging directory's DACL let no standard user in, but its parent did: `%ProgramData%` grants Users the right to create directories, and the creator owns what it creates. A pre-created `%ProgramData%\tty7` gave its owner delete-child over the administrator-only staging inside it — enough to rename the verified staging aside and drop an identical name of their own into the gap between the digest check and the execute. The root is now created with the same protected descriptor, taking down whatever holds the name first; `CreateDirectoryW` applies a descriptor only when it is the one creating the directory, so succeeding is the proof. The per-run sweep goes with it — the root's removal takes the leftovers. - The GUI aimed the prompt at the updater the *plan* named, and `update.json` sits in the user's config directory. It now aims at the installation this process runs from, so the binary the prompt names is the binary that starts. Also quote the elevated command line the way `CommandLineToArgvW` reads it back: a backslash escapes only in front of a quote, so a config directory ending in one used to escape its own closing quote and swallow every argument after it, `--result-file` — the file the watcher waits on — included. * test(update): pin the elevated stage's trust root to its own image A regression test for the shape of the hole rather than the hole: if `installed_root` ever goes back to reading an argument, the pin the elevated stage runs before executing the staged helper stops meaning anything, and nothing else in the suite would notice. * fix(update): bring tty7 back when the elevated chain never reports The watcher's two timeouts returned without relaunching. Every other way out of the chain ends with the app back on screen, but a stage 1 that died before writing its status or its outcome — killed, crashed, an AppInfo service that never delivered it — left the user with the GUI already quit, nothing to replace it, and nothing said. Same for an install still running an hour later. Both paths now end the way the others do: an outcome the watcher wrote itself, then the relaunch. The synthesized outcome is written whether or not the relaunch succeeds, which also closes the same gap on the pre-existing "the elevated updater exited without recording a result" path — the next launch can name what happened instead of silently offering the version again. What kept those paths from relaunching was the risk of a second window beside a GUI that is still up: a declined prompt leaves this process running, and the kill that reaps its watcher can lose. The watcher now takes the GUI's pid and opens a handle to it at startup — while the GUI is provably alive, since it is sitting in ShellExecuteExW waiting on the prompt — so the number cannot be recycled out from under it. Before relaunching, a GUI that is still alive is waited out for 30 seconds: one that is quitting (a chain that failed fast can beat it out the door) is gone well inside that and gets its relaunch, one that is staying is recognized as staying and gets neither a relaunch nor a failure record it did not earn. A live process always answers to its own pid, so the check cannot be wrong in the direction that double-launches. Also give the Japanese elevation notice its closing 。 * fix(update): poll the parent out across the elevation account boundary Under an over-the-shoulder elevation the install stage runs as the administrator, and OpenProcess on the signed-in user's GUI answers ERROR_ACCESS_DENIED - the same boundary pid_alive already documents from the watcher's side. wait_for_exit treated that as a fatal error, so the chain recovered and reported a failure before Setup ever ran. The wait now degrades to polling the pid until it stops answering, bounded so a recycled pid cannot hold the install hostage forever. --------- Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Co-authored-by: l0ng-ai --- .github/scripts/windows-installer.iss | 7 +- CHANGELOG.md | 38 + Cargo.toml | 5 + crates/tty7-core/src/daemon/install/mod.rs | 1 + .../tty7-core/src/daemon/install/outcome.rs | 99 ++ src/bin/tty7-updater.rs | 1386 ++++++++++++++++- src/core/update.rs | 1109 ++++++++++++- src/main.rs | 4 + src/ui/i18n/en.rs | 3 + src/ui/i18n/ja.rs | 3 + src/ui/i18n/mod.rs | 1 + src/ui/i18n/zh.rs | 3 + 12 files changed, 2566 insertions(+), 93 deletions(-) create mode 100644 crates/tty7-core/src/daemon/install/outcome.rs 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}",