From 5b3b761404d49111c85cf7fb84cd8b73a0b6b598 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 1 Aug 2026 20:43:01 -0500 Subject: [PATCH] fix(desktop/windows): don't pre-write the update marker for stale installers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit copy_self_to_hermes_home no-ops during --update, so the hermes-setup.exe staged by a user's ORIGINAL install orchestrates every later update forever. Installers predating #74782 have no self-PID exclusion in UpdateMarkerGuard::acquire, so when the desktop pre-writes the marker naming that very updater (#59313), the updater reads its own claim as a foreign live owner and aborts: Another Hermes update is already running (PID , started 1s ago) mapped to the "Hermes is still running. Close all Hermes windows" screen. Retry relaunches the desktop, which pre-writes a fresh marker naming the next updater, which refuses itself again — an unbreakable loop. The always-live PID also defeats the staleness self-heal in readLiveUpdateMarker, and the update that would replace the stale binary is precisely the one being refused, so there is no route out. Gate the pre-write on the staged installer's mtime, which faithfully stamps the installer generation (the binary is written at install/repair time). Anything staged before the self-adopt fix skips the pre-write and lets the updater write its own claim; the hand-off itself is untouched, because that stale binary is the only updater those users have and it works fine once allowed to acquire. Unreadable mtime counts as unsupported: skipping the pre-write only loses anti-respawn hardening, while a wedged updater can never update again. --- apps/desktop/electron/updater-process.test.ts | 60 ++++++++++++++++++- apps/desktop/electron/updater-process.ts | 51 ++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/apps/desktop/electron/updater-process.test.ts b/apps/desktop/electron/updater-process.test.ts index 96f48eb9ff016..e781fe3efa3ec 100644 --- a/apps/desktop/electron/updater-process.test.ts +++ b/apps/desktop/electron/updater-process.test.ts @@ -4,7 +4,65 @@ import path from 'node:path' import { test } from 'vitest' -import { resolveStagedUpdaterBinary, spawnUpdaterProcess } from './updater-process' +import { + MARKER_SELF_ADOPT_EPOCH_MS, + resolveStagedUpdaterBinary, + spawnUpdaterProcess, + stagedUpdaterSupportsPrewrittenMarker +} from './updater-process' + +const DAY_MS = 24 * 60 * 60 * 1000 + +test('stagedUpdaterSupportsPrewrittenMarker rejects installers predating the self-adopt fix', () => { + // The real-world trap: an installer staged at first install months ago, never + // refreshed because copy_self_to_hermes_home no-ops during --update. + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS - 60 * DAY_MS + }), + false + ) +}) + +test('stagedUpdaterSupportsPrewrittenMarker accepts installers from the fix onward', () => { + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS + }), + true + ) + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS + 30 * DAY_MS + }), + true + ) +}) + +test('stagedUpdaterSupportsPrewrittenMarker treats an unreadable mtime as unsupported', () => { + // Bias toward the path that can always make progress: a skipped pre-write + // loses anti-respawn hardening, a wedged updater can never update again. + assert.equal( + stagedUpdaterSupportsPrewrittenMarker('C:\\Hermes\\hermes-setup.exe', { + stagedMtimeMs: () => null + }), + false + ) +}) + +test('resolveStagedUpdaterBinary still returns a stale staged updater on Windows', () => { + // Staleness gates only the marker PRE-WRITE, never the hand-off itself: + // the stale binary is the only updater these users have, and it works fine + // once it is allowed to write its own claim. + assert.equal( + resolveStagedUpdaterBinary('C:\\Hermes', { + fileExists: () => true, + isWindows: true, + stagedMtimeMs: () => MARKER_SELF_ADOPT_EPOCH_MS - 60 * DAY_MS + }), + path.join('C:\\Hermes', 'hermes-setup.exe') + ) +}) test('spawnUpdaterProcess hides the updater console and detaches the child on Windows', () => { const calls: Array<{ args: string[]; command: string; options: SpawnOptions }> = [] diff --git a/apps/desktop/electron/updater-process.ts b/apps/desktop/electron/updater-process.ts index 56352da3e8534..97b1d9651afa8 100644 --- a/apps/desktop/electron/updater-process.ts +++ b/apps/desktop/electron/updater-process.ts @@ -12,8 +12,20 @@ export interface UpdaterChild { export interface ResolveStagedUpdaterBinaryDeps { isWindows?: boolean fileExists?: (candidate: string) => boolean + stagedMtimeMs?: (candidate: string) => number | null } +/** + * Staged installers older than this have no self-PID exclusion in + * `UpdateMarkerGuard::acquire` and will refuse an update whose marker was + * pre-written on their behalf. + * + * The self-adopt fix landed in #74782 / 160586ff8 (2026-07-30 17:57 +0700). + * We compare against the start of 2026-07-31 UTC so the boundary is + * unambiguous for binaries staged that same day. + */ +export const MARKER_SELF_ADOPT_EPOCH_MS = Date.UTC(2026, 6, 31) + function stagedFileExists(candidate: string): boolean { try { return statSync(candidate).isFile() @@ -22,6 +34,14 @@ function stagedFileExists(candidate: string): boolean { } } +function stagedFileMtimeMs(candidate: string): number | null { + try { + return statSync(candidate).mtimeMs + } catch { + return null + } +} + /** * Decide which staged installer binary — if any — may be handed an update. * @@ -61,6 +81,37 @@ export function resolveStagedUpdaterBinary( return fileExists(candidate) ? candidate : null } +/** + * True when the staged installer is new enough to survive a pre-written marker. + * + * `copy_self_to_hermes_home` deliberately no-ops during `--update` + * (apps/bootstrap-installer/src-tauri/src/paths.rs), so the binary staged by a + * user's ORIGINAL install orchestrates every later update — forever. Installers + * predating #74782 have no self-PID exclusion in `UpdateMarkerGuard::acquire`, + * so when the desktop pre-writes the marker naming that very updater, the + * updater reads its own claim as a foreign live owner and aborts with + * "Another Hermes update is already running (PID , started 1s ago)" — + * the observed infinite "Install didn't finish" loop. Skipping the pre-write + * for those binaries lets them acquire cleanly and run `hermes update`, which + * pulls the permanent fixes. See shouldPrewriteUpdateMarker. + * + * We cannot ask the binary its version without executing it, so use its mtime: + * the installer is written to HERMES_HOME at install/repair time, making mtime + * a faithful stamp of which installer generation produced it. + * + * Unreadable mtime counts as UNSUPPORTED — the pre-write is a best-effort + * hardening, while a wedged updater is unrecoverable, so we bias toward the + * path that can always make progress. + */ +export function stagedUpdaterSupportsPrewrittenMarker( + candidate: string, + deps: ResolveStagedUpdaterBinaryDeps = {} +): boolean { + const mtimeMs = (deps.stagedMtimeMs ?? stagedFileMtimeMs)(candidate) + + return typeof mtimeMs === 'number' && Number.isFinite(mtimeMs) && mtimeMs >= MARKER_SELF_ADOPT_EPOCH_MS +} + export interface SpawnUpdaterProcessDeps { isWindows?: boolean spawnProcess?: (command: string, args: string[], options: SpawnOptions) => UpdaterChild