diff --git a/src/main/linux-package-update-recovery.test.ts b/src/main/linux-package-update-recovery.test.ts index e3c4a27533f..1d9bf8d8650 100644 --- a/src/main/linux-package-update-recovery.test.ts +++ b/src/main/linux-package-update-recovery.test.ts @@ -611,6 +611,31 @@ describePosix('validation coalescing', () => { expect(hashPasses.count).toBe(2) }) + // Why: a verdict handed to a root package manager must cover the bytes as of the click, not the + // bytes a Copy click started streaming seconds earlier. + it('never reuses an in-flight pass for a pre-install re-proof', async () => { + await writePackage('orca.deb') + capture() + const artifact = recovery.getTrackedLinuxPackageArtifact() + const copyPass = recovery.resolveLinuxPackageInstallInstructions(recoveryFor()) + const installPass = recovery.revalidateLinuxPackageForInstall(artifact!) + + await expect(installPass).resolves.toEqual({ ok: true }) + await expect(copyPass).resolves.toMatchObject({ ok: true }) + expect(hashPasses.count).toBe(2) + }) + + it('lets a later Copy click join the pre-install pass', async () => { + await writePackage('orca.deb') + capture() + const artifact = recovery.getTrackedLinuxPackageArtifact() + const installPass = recovery.revalidateLinuxPackageForInstall(artifact!) + const copyPass = recovery.resolveLinuxPackageInstallInstructions(recoveryFor()) + + await Promise.all([installPass, copyPass]) + expect(hashPasses.count).toBe(1) + }) + it('does not reuse an in-flight pass for a different artifact', async () => { await writePackage('orca.deb') await writePackage('orca-next.deb') @@ -629,6 +654,39 @@ describePosix('validation coalescing', () => { }) }) +describePosix('revalidateLinuxPackageForInstall', () => { + it('proves the retained package still matches its release digest', async () => { + await writePackage('orca.deb') + capture() + const artifact = recovery.getTrackedLinuxPackageArtifact() + await expect(recovery.revalidateLinuxPackageForInstall(artifact!)).resolves.toEqual({ + ok: true + }) + }) + + it('rejects a package swapped after the download was verified', async () => { + await writePackage('orca.deb') + capture() + const artifact = recovery.getTrackedLinuxPackageArtifact() + await writePackage('orca.deb', 'attacker supplied package') + await expect(recovery.revalidateLinuxPackageForInstall(artifact!)).resolves.toEqual({ + ok: false, + reason: 'hash-mismatch' + }) + }) + + it('reports a package deleted from the cache as missing', async () => { + const filePath = await writePackage('orca.deb') + capture() + const artifact = recovery.getTrackedLinuxPackageArtifact() + await fsp.rm(filePath) + await expect(recovery.revalidateLinuxPackageForInstall(artifact!)).resolves.toEqual({ + ok: false, + reason: 'missing' + }) + }) +}) + describePosix('revealLinuxPackage', () => { it('reveals a verified package on the machine that owns it', async () => { const filePath = await writePackage('orca.deb') diff --git a/src/main/linux-package-update-recovery.ts b/src/main/linux-package-update-recovery.ts index 83aca03cb3f..f854b63c5b8 100644 --- a/src/main/linux-package-update-recovery.ts +++ b/src/main/linux-package-update-recovery.ts @@ -241,6 +241,29 @@ async function validateArtifact(artifact: LinuxPackageArtifact): Promise { + const key = `${artifact.packageType}:${artifact.version}:${artifact.path}:${artifact.sha512}` + if (!options?.fresh && inFlightValidation?.key === key) { + return inFlightValidation.promise + } + const promise: Promise = validateArtifact(artifact).finally(() => { + // Why: identity, not key — a fresh install pass may already have replaced this entry. + if (inFlightValidation?.promise === promise) { + inFlightValidation = null + } + }) + inFlightValidation = { key, promise } + return promise +} + /** * Revalidates the retained package against the digest captured from the release metadata. This is * an integrity check against that HTTPS metadata, not a package signature and not a privilege @@ -257,17 +280,7 @@ async function validateTrackedArtifact( ) { return { ok: false, reason: 'missing' } } - const key = `${artifact.packageType}:${artifact.version}:${artifact.path}:${artifact.sha512}` - if (inFlightValidation?.key === key) { - return inFlightValidation.promise - } - const promise = validateArtifact(artifact).finally(() => { - if (inFlightValidation?.key === key) { - inFlightValidation = null - } - }) - inFlightValidation = { key, promise } - return promise + return runValidation(artifact) } export async function resolveLinuxPackageInstallInstructions( @@ -289,6 +302,23 @@ export async function resolveLinuxPackageInstallInstructions( } } +/** + * Re-proves the retained package immediately before the privileged installer consumes it. + * + * The cache path is user-writable, so a digest checked when the download finished says nothing + * about the bytes `dpkg -i` will read minutes later. Re-hashing here does not close the race — + * only an immutable handoff would — but it shrinks the window from "since the download" to + * "since this call", and it catches the artifact being swapped or deleted outright. Takes the + * artifact rather than a recovery so both the retry and the plain "Restart to Update" install + * are covered. + */ +export async function revalidateLinuxPackageForInstall( + artifact: LinuxPackageArtifact +): Promise<{ ok: true } | { ok: false; reason: LinuxPackageRecoveryUnavailableReason }> { + const validation = await runValidation(artifact, { fresh: true }) + return validation.ok ? { ok: true } : { ok: false, reason: validation.reason } +} + export async function revealLinuxPackage( recovery: LinuxPackageInstallRecovery ): Promise { diff --git a/src/main/updater-linux-package-recovery-actions.test.ts b/src/main/updater-linux-package-recovery-actions.test.ts index 91b2423c817..75f593b7f57 100644 --- a/src/main/updater-linux-package-recovery-actions.test.ts +++ b/src/main/updater-linux-package-recovery-actions.test.ts @@ -9,6 +9,7 @@ const { getTrackedLinuxPackageArtifactMock, recordUpdaterLifecycleMock, resolveLinuxPackageInstallInstructionsMock, + revalidateLinuxPackageForInstallMock, revealLinuxPackageMock, resetHandlers } = vi.hoisted(() => { @@ -42,6 +43,7 @@ const { getTrackedLinuxPackageArtifactMock: vi.fn(), recordUpdaterLifecycleMock: vi.fn(), resolveLinuxPackageInstallInstructionsMock: vi.fn(), + revalidateLinuxPackageForInstallMock: vi.fn(), revealLinuxPackageMock: vi.fn(), resetHandlers: () => updaterHandlers.clear() } @@ -82,6 +84,7 @@ vi.mock('./linux-package-update-recovery', () => ({ clearTrackedLinuxPackageArtifactForOtherVersion: vi.fn(), getTrackedLinuxPackageArtifact: getTrackedLinuxPackageArtifactMock, resolveLinuxPackageInstallInstructions: resolveLinuxPackageInstallInstructionsMock, + revalidateLinuxPackageForInstall: revalidateLinuxPackageForInstallMock, revealLinuxPackage: revealLinuxPackageMock })) @@ -113,6 +116,7 @@ describe('linux package recovery actions', () => { resolveLinuxPackageInstallInstructionsMock .mockReset() .mockResolvedValue({ ok: true, command: "sudo apt install -- ''", packageFileName: 'p' }) + revalidateLinuxPackageForInstallMock.mockReset().mockResolvedValue({ ok: true }) revealLinuxPackageMock.mockReset().mockResolvedValue({ ok: true }) }) diff --git a/src/main/updater.test.ts b/src/main/updater.test.ts index 35a8c4c45bc..a0ab479343d 100644 --- a/src/main/updater.test.ts +++ b/src/main/updater.test.ts @@ -1,6 +1,53 @@ /* eslint-disable max-lines */ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createHash } from 'node:crypto' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import type * as UpdaterModule from './updater' +import type * as RecoveryModule from './linux-package-update-recovery' +import type { UpdateStatus } from '../shared/types' + +type RevalidationVerdict = Awaited< + ReturnType +> + +// Captured before any vi.useFakeTimers() call: the only handle left that still yields to libuv. +const realSetTimeout = globalThis.setTimeout + +type StagedLinuxPackages = { + cacheRoot: string + debPath: string + debSha512: string + rpmPath: string + rpmSha512: string +} + +/** + * Stages real packages inside a real updater cache: every install re-proves the retained digest by + * streaming the file off disk, so a path that never existed would abort before reaching the native + * updater. Returns the actual digests for the download events. + */ +function stageLinuxUpdateCache(): StagedLinuxPackages { + const cacheRoot = mkdtempSync(join(tmpdir(), 'orca-updater-cache-')) + const pendingDir = join(cacheRoot, 'orca-updater', 'pending') + mkdirSync(pendingDir, { recursive: true }) + const stagePackage = (fileName: string): { path: string; sha512: string } => { + const packagePath = join(pendingDir, fileName) + const bytes = Buffer.from(`orca test package ${fileName}`) + writeFileSync(packagePath, bytes) + return { path: packagePath, sha512: createHash('sha512').update(bytes).digest('base64') } + } + const deb = stagePackage('orca-ide_1.0.61_amd64.deb') + const rpm = stagePackage('orca-ide-1.0.61.x86_64.rpm') + return { + cacheRoot, + debPath: deb.path, + debSha512: deb.sha512, + rpmPath: rpm.path, + rpmSha512: rpm.sha512 + } +} const { appMock, @@ -1780,8 +1827,12 @@ describe('updater', () => { 'updater:status', expect.objectContaining({ state: 'error', - // Why: a pre-commit install failure is not fixed by restarting, so the copy must not suggest it. - message: 'Could not start the update installer. Orca remains open.' + // Why: a pre-commit install failure is not fixed by restarting, so the copy must not + // suggest it — except on macOS, where quitting does re-stage a Squirrel update. + message: + process.platform === 'darwin' + ? 'Could not restart to install the update. Quit and reopen Orca, then try again.' + : 'Could not start the update installer. Orca remains open.' }) ) }) @@ -3794,29 +3845,93 @@ describe('updater', () => { }) describe('linux root package install recovery', () => { - // Real 64-byte SHA-512 values; capture rejects a digest that cannot decode to one. - const DEB_SHA512 = - 'LHlL7dKoqg98gS2nfQv878dK+UoktbAkm4M20/hoJ2Qr0Kqsa3MSL4VmWy/Lll/MYjQFkpvOxduQ/vswentozA==' - const RPM_SHA512 = - 'W2U3AUPfVpc0Ia1qX/VJ5+8aW+yOFhysT6ryodo7A6DTZKqL6RYLK53U6ShGx+9f4lb35qtd4tOp5jzJZZdAfQ==' + let staged: StagedLinuxPackages + let EXIT_127: string + + // Why: the pre-install digest re-proof streams the package off real disk. Fake timers never + // advance libuv, so the quit timer needs fake time while the read needs real event-loop turns. + const settleQuitAndInstall = async (): Promise => { + await vi.advanceTimersByTimeAsync(100) + for (let turn = 0; turn < 40; turn += 1) { + await new Promise((resolve) => realSetTimeout(resolve, 0)) + } + await vi.advanceTimersByTimeAsync(0) + } + + beforeEach(() => { + staged = stageLinuxUpdateCache() + vi.stubEnv('XDG_CACHE_HOME', staged.cacheRoot) + EXIT_127 = `Command failed: /usr/bin/pkexec /usr/bin/dpkg -i ${staged.debPath}, exited with code 127` + }) + + afterEach(() => { + vi.doUnmock('./linux-package-update-recovery') + vi.unstubAllEnvs() + rmSync(staged.cacheRoot, { recursive: true, force: true }) + }) + + /** + * Holds the pre-install re-proof open so a verdict can be delivered at an exact point in the + * cycle. Only that call is replaced — the artifact state stays real. Must precede startUpdater. + */ + const holdRevalidation = (): { + settle: (verdict: RevalidationVerdict) => void + fail: (error: Error) => void + } => { + let pending: { + resolve: (verdict: RevalidationVerdict) => void + reject: (error: Error) => void + } | null = null + vi.doMock('./linux-package-update-recovery', async () => { + const actual = await vi.importActual( + './linux-package-update-recovery' + ) + return { + ...actual, + revalidateLinuxPackageForInstall: vi.fn( + () => + new Promise((resolve, reject) => { + pending = { resolve, reject } + }) + ) + } + }) + return { + settle: (verdict) => { + pending?.resolve(verdict) + pending = null + }, + fail: (error) => { + pending?.reject(error) + pending = null + } + } + } + + const lastStatus = (send: ReturnType): UpdateStatus | undefined => + send.mock.calls.findLast(([channel]) => channel === 'updater:status')?.[1] + // Why: macOS keeps the restart advice because quitting does re-stage a Squirrel update. const PRE_COMMIT_FAILURE_MESSAGE = process.platform === 'darwin' ? 'Could not restart to install the update. Quit and reopen Orca, then try again.' : 'Could not start the update installer. Orca remains open.' - const DEB_PATH = '/home/tester/.cache/orca-updater/pending/orca-ide_1.0.61_amd64.deb' - const RPM_PATH = '/home/tester/.cache/orca-updater/pending/orca-ide-1.0.61.x86_64.rpm' const AGENT_STDERR = 'pkexec: Error executing command as another user: No authentication agent found.' - const EXIT_127 = `Command failed: /usr/bin/pkexec /usr/bin/dpkg -i ${DEB_PATH}, exited with code 127` const downloadedEvent = (overrides?: Record): Record => ({ version: '1.0.61', - downloadedFile: DEB_PATH, - files: [{ url: 'orca-ide_1.0.61_amd64.deb', sha512: DEB_SHA512 }], + downloadedFile: staged.debPath, + files: [{ url: 'orca-ide_1.0.61_amd64.deb', sha512: staged.debSha512 }], ...overrides }) + const rpmDownloadedEvent = (): Record => + downloadedEvent({ + downloadedFile: staged.rpmPath, + files: [{ url: 'orca-ide-1.0.61.x86_64.rpm', sha512: staged.rpmSha512 }] + }) + const startUpdater = async ( packageType: 'deb' | 'rpm' | null ): Promise<{ send: ReturnType; updater: typeof UpdaterModule }> => { @@ -3902,12 +4017,12 @@ describe('updater', () => { const { send, updater } = await startUpdater('deb') await reachDownloaded(updater, downloadedEvent()) autoUpdaterMock.quitAndInstall.mockImplementation(() => { - autoUpdaterMock.logger?.error(`${AGENT_STDERR} target ${DEB_PATH}`) + autoUpdaterMock.logger?.error(`${AGENT_STDERR} target ${staged.debPath}`) throw new Error(EXIT_127) }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() // Why: the sync throw ends capture before the catch, so the stashed text must survive. expect(send).toHaveBeenCalledWith('updater:status', { @@ -3926,19 +4041,13 @@ describe('updater', () => { const openWindow = { removeAllListeners: vi.fn() } browserWindowMock.getAllWindows.mockReturnValue([openWindow] as never) const { send, updater } = await startUpdater('rpm') - await reachDownloaded( - updater, - downloadedEvent({ - downloadedFile: RPM_PATH, - files: [{ url: 'orca-ide-1.0.61.x86_64.rpm', sha512: RPM_SHA512 }] - }) - ) + await reachDownloaded(updater, rpmDownloadedEvent()) autoUpdaterMock.quitAndInstall.mockImplementation(() => { autoUpdaterMock.emit('error', new Error('Command failed, exited with code 1')) }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() expect(send).toHaveBeenCalledWith('updater:status', { state: 'error', @@ -3968,7 +4077,7 @@ describe('updater', () => { }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() expect(send).toHaveBeenCalledWith('updater:status', { state: 'error', @@ -3992,7 +4101,7 @@ describe('updater', () => { }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() expect(send).toHaveBeenCalledWith('updater:status', { @@ -4012,7 +4121,7 @@ describe('updater', () => { send.mockClear() updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() expect(recordUpdaterLifecycleMock).toHaveBeenCalledWith( 'post_commit_cleanup_failed', @@ -4030,7 +4139,7 @@ describe('updater', () => { await reachDownloaded(updater, downloadedEvent()) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() expect(killAllPtyMock).toHaveBeenCalledTimes(1) send.mockClear() @@ -4048,9 +4157,9 @@ describe('updater', () => { }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() // Why: a retry usually fails identically; a deduped status would strand the preload restart relay. expect( @@ -4069,16 +4178,205 @@ describe('updater', () => { }) }) - it('records classification-only lifecycle data for a package install failure', async () => { - const { updater } = await startUpdater('deb') + // Why: the cache path is user-writable, so the bytes verified when the recovery card + // rendered are not necessarily the bytes a root package manager would read on retry. + it('aborts the retry when the retained package no longer matches its digest', async () => { + const { send, updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + + // The escalation fails, which is what puts the recovery card (and its retry) on screen. + autoUpdaterMock.quitAndInstall.mockImplementation(() => { + autoUpdaterMock.emit('error', new Error(EXIT_127)) + }) + updater.quitAndInstall() + await settleQuitAndInstall() + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + + // A local process swaps the verified package for its own between failure and retry. + writeFileSync(staged.debPath, Buffer.from('attacker supplied package')) + send.mockClear() + killAllPtyMock.mockClear() + + updater.quitAndInstall() + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + expect(killAllPtyMock).not.toHaveBeenCalled() + expect(updater.isQuittingForUpdate()).toBe(false) + expect(send).toHaveBeenCalledWith('updater:status', { + state: 'error', + message: + 'The downloaded package no longer matches the verified release, so Orca will not hand it to a package manager. Download the update again, or get it from the official release page.' + }) + expect(recordUpdaterLifecycleMock).toHaveBeenCalledWith( + 'linux_package_revalidation_failed', + expect.objectContaining({ action: 'retry-automatic', reason: 'hash-mismatch' }), + expect.anything() + ) + }) + + // Why: "Restart to Update" is the common path and can sit unclicked for hours, so the same + // user-writable package reaches a root installer with a far longer window than any retry. + it('aborts the first install when the downloaded package was swapped', async () => { + const { send, updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + writeFileSync(staged.debPath, Buffer.from('attacker supplied package')) + send.mockClear() + + updater.quitAndInstall() + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(killAllPtyMock).not.toHaveBeenCalled() + expect(updater.isQuittingForUpdate()).toBe(false) + expect(send).toHaveBeenCalledWith('updater:status', { + state: 'error', + message: + 'The downloaded package no longer matches the verified release, so Orca will not hand it to a package manager. Download the update again, or get it from the official release page.' + }) + expect(send).toHaveBeenCalledWith('updater:quitAndInstallAborted') + expect(recordUpdaterLifecycleMock).toHaveBeenCalledWith( + 'linux_package_revalidation_failed', + expect.objectContaining({ action: 'restart-to-install', reason: 'hash-mismatch' }), + expect.anything() + ) + }) + + it('installs normally when the retained package still matches its digest', async () => { + const { send, updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + + updater.quitAndInstall() + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + expect(killAllPtyMock).toHaveBeenCalledTimes(1) + // Why: an abort push here would clear the restart flag mid-quit and re-arm the dirty-buffer + // prompt against the install that is already committed. + expect(send).not.toHaveBeenCalledWith('updater:quitAndInstallAborted') + expect(recordUpdaterLifecycleMock).not.toHaveBeenCalledWith( + 'linux_package_revalidation_failed', + expect.anything(), + expect.anything() + ) + }) + + // Why: hashing 160 MB outlives the cycle it started in, and Check for Updates stays enabled + // while it runs — a verdict from the old cycle must not replace the card that took over. + it('drops an abort verdict once a newer check replaced the card', async () => { + const revalidation = holdRevalidation() + const { send, updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + + updater.quitAndInstall() + await vi.advanceTimersByTimeAsync(100) + // The user gives up waiting and checks again; that check owns the card from here. + updater.checkForUpdatesFromMenu() + await vi.advanceTimersByTimeAsync(0) + expect(lastStatus(send)).toMatchObject({ state: 'available', version: '1.0.61' }) + + revalidation.settle({ ok: false, reason: 'hash-mismatch' }) + await settleQuitAndInstall() + + // The install is still abandoned — only the stale status is withheld. + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(lastStatus(send)).toMatchObject({ state: 'available', version: '1.0.61' }) + // Withholding the status must not also withhold the abort: the renderer armed its restart and + // would otherwise skip its unsaved-work prompt for the rest of the session. + expect(send).toHaveBeenCalledWith('updater:quitAndInstallAborted') + expect(recordUpdaterLifecycleMock).toHaveBeenCalledWith( + 'linux_package_revalidation_failed', + expect.objectContaining({ reason: 'hash-mismatch' }), + expect.anything() + ) + }) + + // Why: EMFILE/EIO during the stream says nothing about the bytes, so the copy must not claim + // the package changed and the card must keep the actions that still work. + it('keeps the recovery card usable when the re-proof cannot read the package', async () => { + const revalidation = holdRevalidation() + const { send, updater } = await startUpdater('deb') await reachDownloaded(updater, downloadedEvent()) autoUpdaterMock.quitAndInstall.mockImplementation(() => { - autoUpdaterMock.logger?.error(`${AGENT_STDERR} target ${DEB_PATH}`) autoUpdaterMock.emit('error', new Error(EXIT_127)) }) + updater.quitAndInstall() + await vi.advanceTimersByTimeAsync(100) + revalidation.settle({ ok: true }) + await settleQuitAndInstall() + send.mockClear() + + updater.quitAndInstall() + await vi.advanceTimersByTimeAsync(100) + revalidation.settle({ ok: false, reason: 'read-failed' }) + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + expect(lastStatus(send)).toEqual({ + state: 'error', + message: + 'Orca could not read the downloaded package. Download the update again, or get it from the official release page.', + recovery: { + kind: 'linux-package-install', + packageType: 'deb', + reason: 'package-install-failed', + version: '1.0.61' + } + }) + }) + + // Why: the re-proof runs before performQuitAndInstall's own error handling, so a rejection + // there would strand the quit timer and make every later install a silent no-op. + it('stays installable after a re-proof that rejects outright', async () => { + const revalidation = holdRevalidation() + const { send, updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + + updater.quitAndInstall() + await vi.advanceTimersByTimeAsync(100) + revalidation.fail(new Error('hash worker crashed')) + await settleQuitAndInstall() + + // Fails closed: an unprovable package is not handed to a root package manager. + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(lastStatus(send)).toMatchObject({ + state: 'error', + message: + 'Orca could not read the downloaded package. Download the update again, or get it from the official release page.' + }) updater.quitAndInstall() await vi.advanceTimersByTimeAsync(100) + revalidation.settle({ ok: true }) + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + }) + + // Why: a second click during the multi-second hash must not schedule a parallel install. + it('ignores a second install request while the digest re-proof runs', async () => { + const { updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + + updater.quitAndInstall() + // Fires the quit timer, which starts the hash; the read itself is still outstanding. + await vi.advanceTimersByTimeAsync(100) + updater.quitAndInstall() + await settleQuitAndInstall() + + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + }) + + it('records classification-only lifecycle data for a package install failure', async () => { + const { updater } = await startUpdater('deb') + await reachDownloaded(updater, downloadedEvent()) + autoUpdaterMock.quitAndInstall.mockImplementation(() => { + autoUpdaterMock.logger?.error(`${AGENT_STDERR} target ${staged.debPath}`) + autoUpdaterMock.emit('error', new Error(EXIT_127)) + }) + + updater.quitAndInstall() + await settleQuitAndInstall() const failure = recordUpdaterLifecycleMock.mock.calls.find( ([event]) => event === 'linux_package_install_failed' @@ -4091,7 +4389,7 @@ describe('updater', () => { errorType: 'Error' }) const durable = JSON.stringify(recordUpdaterLifecycleMock.mock.calls) - expect(durable).not.toContain(DEB_PATH) + expect(durable).not.toContain(staged.debPath) expect(durable).not.toContain('authentication agent') }) @@ -4103,7 +4401,7 @@ describe('updater', () => { }) updater.quitAndInstall() - await vi.advanceTimersByTimeAsync(100) + await settleQuitAndInstall() const failure = recordUpdaterLifecycleMock.mock.calls.find( ([event]) => event === 'linux_package_install_failed' diff --git a/src/main/updater.ts b/src/main/updater.ts index 6bb26d84dd5..302591286f3 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -45,7 +45,9 @@ import { clearTrackedLinuxPackageArtifact, getTrackedLinuxPackageArtifact, resolveLinuxPackageInstallInstructions, + revalidateLinuxPackageForInstall, revealLinuxPackage, + type LinuxPackageArtifact, type LinuxPackageRecoveryUnavailableReason } from './linux-package-update-recovery' import { @@ -124,6 +126,9 @@ let autoUpdateCheckTimer: ReturnType | null = null let nudgeCheckTimer: ReturnType | null = null let pendingQuitAndInstallTimer: ReturnType | null = null let quitAndInstallInProgress = false +// Why: the pre-install digest re-proof streams the whole package, so a second install request can +// arrive while it runs — after the quit timer was cleared but before the handoff owns the process. +let linuxPackageRevalidationInFlight = false let updateInstallMode: UpdateInstallMode = 'interactive' let lastInstallDeferralVersion = { download: null as string | null, install: null as string | null } // Why: once install has committed, late 'error' events must not clear quittingForUpdate — that would re-enable dock activate mid-installer. @@ -693,7 +698,7 @@ function clearPrereleaseFallbackContextIfSettled(): void { } async function performQuitAndInstall(): Promise { - if (quitAndInstallInProgress) { + if (quitAndInstallInProgress || linuxPackageRevalidationInFlight) { recordUpdaterLifecycle('quit_and_install_ignored', { reason: 'already-in-progress' }) return } @@ -707,6 +712,17 @@ async function performQuitAndInstall(): Promise { if (deferHeadlessServeInstall('install', pendingVersion)) { return } + // Why: the retained .deb/.rpm sits on a user-writable path that a root package manager is about + // to read, and nothing re-checks it after download. Re-prove it here — before any teardown — so a + // swapped or vanished package aborts instead of being installed as root. The synchronous guard + // keeps every non-Linux install on its existing timing. + if (getTrackedLinuxPackageArtifact() && !(await proveRetainedLinuxPackage(pendingVersion))) { + // Why: the renderer armed its restart before invoking, and it infers the abort from the error + // status — which a stale-cycle verdict deliberately withholds. Signal the abandon here, where + // it cannot depend on that decision, or the window keeps skipping its unsaved-work prompt. + mainWindowRef?.webContents.send('updater:quitAndInstallAborted') + return + } quitAndInstallInProgress = true markMacQuitAndInstallInFlight() @@ -1801,6 +1817,108 @@ function failLinuxPackageRecovery( throw new Error(message) } +/** + * Identifies the update cycle an install belongs to, so a verdict produced by a multi-second hash + * can be dropped when a newer cycle already replaced the card it would otherwise overwrite. + */ +function getInstallCycleSignature(): string { + const recovery = getActiveLinuxPackageRecovery() + if (recovery) { + return `recovery:${recovery.packageType}:${recovery.version}` + } + return currentStatus.state === 'downloaded' + ? `downloaded:${currentStatus.version}` + : `state:${currentStatus.state}` +} + +/** + * Re-proves the retained package before the install starts. Returns false when the install must be + * abandoned; the artifact is only re-read here, so callers still own every teardown decision. + */ +async function proveRetainedLinuxPackage(pendingVersion: string): Promise { + const artifact = getTrackedLinuxPackageArtifact() + if (!artifact) { + return true + } + // Why: an artifact retained from another cycle says nothing about the file electron-updater is + // about to install, so proving it would block a legitimate install on an unrelated digest. + if (pendingVersion && pendingVersion !== artifact.version) { + return true + } + const recovery = getActiveLinuxPackageRecovery() + const cycle = getInstallCycleSignature() + const reason = await revalidateRetainedLinuxPackage(artifact) + if (!reason) { + return true + } + reportLinuxPackageRevalidationFailure({ artifact, recovery, reason, cycle }) + return false +} + +/** The failing reason, or null when the retained package still matches its release digest. */ +async function revalidateRetainedLinuxPackage( + artifact: LinuxPackageArtifact +): Promise { + linuxPackageRevalidationInFlight = true + try { + const verdict = await revalidateLinuxPackageForInstall(artifact) + return verdict.ok ? null : verdict.reason + } catch (error) { + recordUpdaterLifecycle( + 'linux_package_revalidation_errored', + { errorType: error instanceof Error ? error.name : typeof error }, + { level: 'warn', message: 'Could not re-verify the retained update package' } + ) + // Why: fail closed — bytes we could not read are bytes we cannot hand to a root installer. + return 'read-failed' + } finally { + // Why: the invariant every install path depends on — a wedged flag would make quitAndInstall + // early-return for the rest of the session. + linuxPackageRevalidationInFlight = false + } +} + +function reportLinuxPackageRevalidationFailure({ + artifact, + recovery, + reason, + cycle +}: { + artifact: LinuxPackageArtifact + recovery: LinuxPackageInstallRecovery | null + reason: LinuxPackageRecoveryUnavailableReason + cycle: string +}): void { + recordUpdaterLifecycle( + 'linux_package_revalidation_failed', + { + action: recovery ? 'retry-automatic' : 'restart-to-install', + packageType: artifact.packageType, + version: artifact.version, + reason + }, + { level: 'warn', message: 'Retained update package failed its pre-install digest check' } + ) + // Why: a package proven bad must not stay tracked, but a download that landed during the hash + // owns the slot now and destroying it would force a needless 160 MB redownload. + const clearsArtifact = RECOVERY_CLEARING_REASONS.includes(reason) + if (clearsArtifact && getTrackedLinuxPackageArtifact() === artifact) { + clearTrackedLinuxPackageArtifact() + } + // Why: same reasoning as failLinuxPackageRecovery — a verdict from a cycle that has since been + // replaced must not clobber whatever card the user is looking at now. + if (getInstallCycleSignature() !== cycle) { + return + } + sendInstallFailureStatus({ + state: 'error', + message: LINUX_PACKAGE_RECOVERY_MESSAGES[reason], + // Why: an unreadable file is not evidence the bytes changed, so the recovery card and its + // Copy/Show actions survive a transient I/O failure exactly as they do elsewhere. + ...(recovery && !clearsArtifact ? { recovery } : {}) + }) +} + export async function getLinuxPackageInstallInstructions(): Promise { const recovery = getActiveLinuxPackageRecovery() if (!recovery) { @@ -1849,7 +1967,10 @@ export function quitAndInstall(): void { localBuildSelectionInProgress || pinnedBuildSelectionInProgress || pendingQuitAndInstallTimer || - quitAndInstallInProgress + quitAndInstallInProgress || + // Why: the quit timer is already cleared while the pre-install digest re-proof streams, so + // without this a second click would schedule a parallel install of the same package. + linuxPackageRevalidationInFlight ) { return } diff --git a/src/preload/renderer-restart-wiring.test.ts b/src/preload/renderer-restart-wiring.test.ts index 56cd5d3dd9e..627d04bbd4f 100644 --- a/src/preload/renderer-restart-wiring.test.ts +++ b/src/preload/renderer-restart-wiring.test.ts @@ -7,10 +7,11 @@ import { } from './renderer-restart-wiring' describe('renderer restart wiring', () => { - it('relays updater status and prevented unload events', () => { + it('relays updater status, aborted installs, and prevented unload events', () => { const eventTarget = new EventTarget() const unloadPrevented = vi.fn() const handleStatus = vi.fn() + const abort = vi.fn() const listeners = new Map void>() const ipcRenderer = { on: vi.fn((channel: string, listener: (...args: unknown[]) => void) => { @@ -20,12 +21,15 @@ describe('renderer restart wiring', () => { } as unknown as Parameters[0] eventTarget.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, unloadPrevented) - registerRendererRestartIpcRelays(ipcRenderer, eventTarget, { handleStatus }) + registerRendererRestartIpcRelays(ipcRenderer, eventTarget, { handleStatus, abort }) listeners.get('updater:status')?.({}, { state: 'error', message: 'install failed' }) + // Why: main abandons an install without any status when its verdict outlived the cycle. + listeners.get('updater:quitAndInstallAborted')?.({}) listeners.get('window:unload-prevented')?.({}) - expect(ipcRenderer.on).toHaveBeenCalledTimes(2) + expect(ipcRenderer.on).toHaveBeenCalledTimes(3) expect(handleStatus).toHaveBeenCalledWith({ state: 'error', message: 'install failed' }) + expect(abort).toHaveBeenCalledTimes(1) expect(unloadPrevented).toHaveBeenCalledTimes(1) }) diff --git a/src/preload/renderer-restart-wiring.ts b/src/preload/renderer-restart-wiring.ts index fbd3e26a71d..e5a5de41ecb 100644 --- a/src/preload/renderer-restart-wiring.ts +++ b/src/preload/renderer-restart-wiring.ts @@ -13,11 +13,15 @@ import { export function registerRendererRestartIpcRelays( ipcRenderer: Pick, eventTarget: EventTarget, - relay: Pick + relay: Pick ): void { ipcRenderer.on('updater:status', (_event, status: UpdateStatus) => { relay.handleStatus(status) }) + // Why: main abandons some installs without an error status, and only this tells the renderer. + ipcRenderer.on('updater:quitAndInstallAborted', () => { + relay.abort() + }) ipcRenderer.on('window:unload-prevented', () => { eventTarget.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) }) diff --git a/src/renderer/src/components/LinuxPackageInstallRecoveryCard.test.tsx b/src/renderer/src/components/LinuxPackageInstallRecoveryCard.test.tsx index b42844611b4..e0685b55fbe 100644 --- a/src/renderer/src/components/LinuxPackageInstallRecoveryCard.test.tsx +++ b/src/renderer/src/components/LinuxPackageInstallRecoveryCard.test.tsx @@ -484,6 +484,9 @@ describe('LinuxPackageInstallRecoveryCard retry', () => { fireEvent.click(button('Try Automatic Install Again')) await flushActions() + // Why: the retry now re-proves the package digest before quitting, so the click must report + // progress instead of leaving three inert buttons for the length of the hash. + expect(isAriaDisabled(button('Checking package...'))).toBe(true) // Why: quitAndInstall resolves as soon as main schedules the install, so a resolved promise is // not an outcome — the slot stays held until a real status arrives. expect(isAriaDisabled(button('Copy Install Command'))).toBe(true) diff --git a/src/renderer/src/components/LinuxPackageInstallRecoveryCard.tsx b/src/renderer/src/components/LinuxPackageInstallRecoveryCard.tsx index 96d6b3cd5ee..3d89b7ee6b7 100644 --- a/src/renderer/src/components/LinuxPackageInstallRecoveryCard.tsx +++ b/src/renderer/src/components/LinuxPackageInstallRecoveryCard.tsx @@ -198,6 +198,10 @@ export function LinuxPackageInstallRecoveryCard({ 'auto.components.LinuxPackageInstallRecoveryCard.3da99454c6', 'Try Automatic Install Again' ), + // Why: the retry re-proves the package digest before it quits, so the click is no longer + // instant — without this the card would just go inert for the length of the hash. + pendingLabel: CHECKING_LABEL, + isPending: pendingAction === 'retry', disabled: pendingAction !== null, onClick: handleRetryAutomatic }