From be51553c9eae8d0fdc8a8a6ff396644f78d3008b Mon Sep 17 00:00:00 2001 From: minhn4 Date: Thu, 3 Sep 2026 12:14:31 +0700 Subject: [PATCH] fix(desktop): install unsigned macOS updates outside Squirrel.Mac "Install and restart" did nothing on macOS. Fleet builds have no Apple Developer ID, and Squirrel.Mac validates an update against the running app's code signature, so ShipIt rejected every update with "code object is not signed at all" on an error event nothing listened to. The controller had already stopped the sidecar and then reverted to ready, leaving a broken app and a button that kept offering the same install. On macOS the updater now reads the running bundle's signature before installing. A Developer-ID-signed build still goes through Squirrel; anything else is installed by the fork-only mac-update.ts: the checksum-verified zip is extracted with ditto, staged beside the app, and a detached script swaps the bundles and relaunches once the app exits. The controller gains a prepareInstall step that runs before the sidecars stop, and install failures on every platform now surface as an error state shown in the update dialog. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Ff2GbiXs1BQGGn5txCgECX --- CHANGELOG.md | 13 ++ DIVERGENCES.md | 1 + packages/app/src/i18n/desktop-native.ts | 1 + packages/app/src/i18n/en.ts | 1 + packages/app/src/i18n/es.ts | 2 + packages/app/src/i18n/vi.ts | 1 + packages/desktop/src/main/mac-update.test.ts | 121 ++++++++++++++++ packages/desktop/src/main/mac-update.ts | 132 ++++++++++++++++++ .../src/main/updater-controller.test.ts | 63 ++++++++- .../desktop/src/main/updater-controller.ts | 27 ++-- packages/desktop/src/main/updater.ts | 43 ++++++ 11 files changed, 389 insertions(+), 16 deletions(-) create mode 100644 packages/desktop/src/main/mac-update.test.ts create mode 100644 packages/desktop/src/main/mac-update.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 591c569bc..2287ec941 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,19 @@ file is the human-readable summary of what changed and why. ### Desktop +- **"Install and restart" works on macOS** — the desktop build is not signed + with an Apple Developer ID, and Squirrel.Mac (which handles macOS updates) + refuses to install a bundle it cannot validate against the running app's + signature. The failure only reached the log, so the button did nothing, and + the local server it had already stopped left the app broken until relaunch. + On macOS the app now installs unsigned updates itself: the downloaded + archive (already checksum-verified against the feed) is unpacked and staged + next to the app, then swapped in and relaunched once the app exits. A + Developer-ID-signed build still goes through Squirrel. Install failures on + every platform now show their reason in the update dialog instead of + silently reverting to "Install and restart". Installs made before this + change need one manual reinstall from the DMG; their own button cannot + pick the fix up. - **Windows arm64 installer has a working terminal** — the arm64 build shipped only the x64 `node-pty` and `@parcel/watcher` prebuilds (bun installs optional platform packages for the build host's CPU only), so on an arm64 diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 6d0f64562..64891b5cd 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -231,6 +231,7 @@ from the squash message first, or use a plain merge. | `packages/desktop/src/main/index.ts` | Deleted upstream's `app.commandLine.appendSwitch("proxy-bypass-list", "<-loopback>")`. | Chromium's `<-loopback>` bypass rule means the opposite of what it reads as: it *subtracts* the implicit rules that keep loopback off the proxy, i.e. it opts localhost traffic *into* proxying (`net/docs/proxy.md`). Today it is inert — Electron reads `--proxy-bypass-list` only together with `--proxy-server` (`shell/browser/browser_process_impl.cc`), which the app never passes — but it becomes actively harmful the moment anything launches the app with `--proxy-server`: the renderer's sidecar traffic would be routed into the proxy. Chromium already bypasses loopback by default, so nothing replaces it. | Keep deleted. If upstream adds new proxy switches, check them against Chromium's implicit-bypass semantics before taking them. | | Startup proxy-stall gate: `packages/desktop/src/main/{index,ipc}.ts`, `packages/desktop/src/preload/{index,types}.ts`, `packages/desktop/src/renderer/index.tsx`, `packages/app/src/app.tsx`, `packages/app/src/context/platform.tsx` | Three additive pieces. (1) `main/index.ts` fires `session.defaultSession.resolveProxy(url)` for the sidecar URL as soon as the port is known (fire-and-forget, result logged). (2) A `local-server-health` IPC chain: main answers with the Node-fetch `checkHealth` against the spawned sidecar once `serverReady` resolves (`false` if initialization failed), preload exposes `localServerHealth()`, the renderer platform maps it, and `platform.tsx` gains the optional `localServerHealth?()` capability. (3) `app.tsx` `ConnectionGate`: the health-check effect is restructured around an inner `poll`; when the 10 s blocking window expires without a healthy answer and the connection is the built-in sidecar, it asks `platform.localServerHealth` — if the host process vouches for the server it keeps blocking on the splash for up to another 50 s instead of showing the unreachable error. Background mode and `type: "http"` behavior are unchanged, and without the capability (web) the gate behaves exactly as upstream. | Chromium queues every renderer request — loopback included — until proxy initialization (PAC download / WPAD discovery) resolves; the implicit loopback-bypass rules are only applied once the resolution service is READY (`net/proxy_resolution/configured_proxy_resolution_service.cc`). On corporate networks that takes tens of seconds (measured 21 s vs 64 ms with an unreachable PAC host), so users behind a proxy saw "Could not reach Local Server" at every launch while the server was healthy the whole time — main's own Node-fetch health check (loopback in `NO_PROXY`) had already passed before the window opened. The prewarm overlaps proxy init with sidecar startup; the gate fallback distinguishes "server down" from "renderer network stack not ready". | Keep all three pieces; take upstream's surrounding changes. If upstream restructures `ConnectionGate`, re-apply the vouch-then-extend fallback around whatever its blocking window becomes. Keep the `local-server-health` channel name stable. | | `packages/desktop/src/main/updater.ts` | `autoUpdater.allowDowngrade` `true` → `false`; `autoUpdater.quitAndInstall()` → `quitAndInstall(true, true)`. | Upstream's `true` is safe against immutable GitHub release tags. The fork's feed is a mutable bucket path, where re-uploading or failing to replace a stale `latest.yml` would actively downgrade every client on the next 10-minute poll. The `quitAndInstall` arguments are load-bearing against the assisted installer (see the `electron-builder.config.ts` row): the defaults are `isSilent = false, isForceRunAfter = false`, and `NsisUpdater.doInstall` appends `/S` only when `isSilent` — so with an assisted installer the default would run the full wizard on **every** update, re-asking install mode and directory for a location the user already chose. `isForceRunAfter` is honoured only when `isSilent` is set (`BaseUpdater.quitAndInstall` passes `autoRunAppAfterInstall` instead when it is not), so both arguments are needed to install quietly *and* come back up. | Keep `false`; take upstream's other updater settings. Keep `quitAndInstall(true, true)` for as long as `nsis.oneClick` is `false`; the two only make sense together. `channel = "latest"` and `allowPrerelease = false` stay as upstream has them — the first must match `publish.channel` in `electron-builder.config.ts`, the second is inert for the `generic` provider (it only steers GitHub tag resolution). | +| Unsigned macOS install path: `packages/desktop/src/main/updater.ts`, `packages/desktop/src/main/updater-controller.ts`, `packages/desktop/src/main/updater-controller.test.ts`, `packages/app/src/i18n/{en,es,vi}.ts`, `packages/app/src/i18n/desktop-native.ts` | `UpdaterBackend` gains an optional `prepareInstall()` (run before `stop()`), `quitAndInstall()` may return a promise, and `install()` transitions to `error` with the message on failure instead of silently back to `ready`. `updater.ts` remembers `update-downloaded`'s `downloadedFile`; on macOS `prepareInstall` reads the running bundle's signature with `codesign` and, unless it carries a real `Authority=` chain, stages the update through the fork-only `mac-update.ts` (`ditto` extract, then a sibling `.CoDev.app.update`); `quitAndInstall` then spawns the detached swap script and calls `app.quit()` instead of `autoUpdater.quitAndInstall`. New native key `desktop.updater.error.downloadMissing`. | Squirrel.Mac validates an update against the running app's designated requirement, which only a real signing identity provides. Fleet builds have no Apple Developer ID, so electron-builder leaves Electron's linker ad-hoc signature and ShipIt fails with `Code signature ... did not pass validation: code object is not signed at all` — on an `error` event nothing listened to, so "Install and restart" did nothing while the sidecar had already been stopped. Windows (`verifyUpdateCodeSignature: false`) and AppImage verify nothing, hence macOS-only. Preparing before `stop()` keeps a failed install from leaving a dead app. | Keep the `prepareInstall` hook and the error transition; take upstream's other controller changes and re-apply the order prepare → stop → quitAndInstall if upstream rewrites `install()`. The gate self-disables for Developer-ID-signed builds (`readCodeSignature` returns `signed` and Squirrel is used), so the path can stay even if signing arrives; drop it only if unsigned builds are no longer shipped. | | Updater dialogs: `packages/desktop/src/main/updater.ts`, `packages/app/src/i18n/{en,es}.ts`, `packages/app/src/i18n/desktop-native.ts` | `showUpdaterDialog` names versions and speaks the user's language. Every branch gains a `detail` line carrying `app.getVersion()`, and the `available` state (where a check stops once automatic downloading is off) gains a consent dialog whose Download button calls `controller.download()` fire-and-forget. Since the v1.18.19 sync the strings come from the desktop-native registry through `nativeT` (upstream's mechanism); the fork adds the keys upstream lacks — `desktop.updater.dialog.{checkFailed,upToDate,ready}.detail`, `desktop.updater.dialog.available.*`, `desktop.updater.dialog.download` — to `DESKTOP_NATIVE_ENGLISH` and the es/vi dicts. | "Check for Updates..." is asked precisely to answer "what am I on, and what would I move to"; the `available` state previously closed showing nothing. | Keep the version details, the `available` branch, and the fork registry keys; take upstream's surrounding updater changes. New dialog copy needs a registry key plus es/vi dict entries, and `{{placeholder}}`s must survive translation (`placeholders.test.ts`). | | Dual-path WSL install: `packages/desktop/src/main/wsl/{runtime,servers}.ts` | `runtime.ts`: three internals gain `export` (`runPowerShell`, `runInteractiveCommand`, `resolveSystem32Command`) — no behavior change. `servers.ts`: the `install-wsl` job calls `installWslRuntimeDualPath` and the `install-distro` job calls `installWslDistroDualPath` (both from the fork-only `install-bundle.ts`, which holds all new logic) instead of `installWslRuntimeElevated` / `installWslDistro` directly; the `install-wsl` job additionally treats exit 3010 (`WSL_INSTALL_REBOOT_REQUIRED`, re-raised by the fork's elevated script) as success with `pendingRestart: true` rather than as a failure — `wsl --version` answers from the freshly installed MSI before the just-enabled VirtualMachinePlatform is active, so no post-install probe can detect the pending reboot; otherwise both return the same `WslCommandResult` shape and the surrounding job/error handling is untouched. | The customer fleet sits behind whitelist-only proxies that cannot reach Microsoft's servers. The dual path downloads from the codev storage host — the runtime as a per-architecture MSI named by its own manifest (`/codev/installers/wsl/manifest.json`, assets under `/codev/installers/wsl//`) and staged through `stageAsset` for resume + sha256 verification, the distro from the pinned `wsl.zip` — enables VirtualMachinePlatform and Microsoft-Windows-Subsystem-Linux via offline dism + installs the MSI behind one UAC prompt (Windows 10 requires the WSL optional component even for the standalone MSI — without it every `wsl` distro command fails with Wsl/WSL_E_WSL_OPTIONAL_COMPONENT_REQUIRED; on machines that already have it dism is a no-op), and falls back to upstream's `wsl --install` / `--web-download` paths only when the download cannot be *obtained* — a completed-but-failed elevated install is returned as-is, never retried behind a second UAC prompt. | Keep the two call swaps, the 3010 handling in `install-wsl`, and the three `export` keywords; take upstream's surrounding changes. If upstream reworks `installWslRuntimeElevated` / `installWslDistro`, keep them exported from `runtime.ts` — `install-bundle.ts` calls them as the fallback path. | | `packages/desktop/package.json` | Added the `test` script (`bun test src --only-failures`). | The package had 21 test files and no way to run them: with no `test` script, `bun turbo test` skipped it and every desktop test was dead weight. Scoped to `src` (like `@opencode-ai/ui`) so the built `out/` tree is never scanned. Paired with the `turbo.json` row — the script alone does not enrol the package in CI. | Keep ours. If upstream adds its own `test` script, take theirs only if it covers `src`. | diff --git a/packages/app/src/i18n/desktop-native.ts b/packages/app/src/i18n/desktop-native.ts index 04cd18d99..ddac47567 100644 --- a/packages/app/src/i18n/desktop-native.ts +++ b/packages/app/src/i18n/desktop-native.ts @@ -103,6 +103,7 @@ export const DESKTOP_NATIVE_ENGLISH = { "desktop.updater.dialog.download": "Download", "desktop.updater.dialog.restart": "Restart", "desktop.updater.dialog.later": "Later", + "desktop.updater.error.downloadMissing": "The downloaded update could not be found. Check for updates again.", "desktop.recovery.action.relaunch": "Relaunch", "desktop.recovery.action.exportLogs": "Export Logs", diff --git a/packages/app/src/i18n/en.ts b/packages/app/src/i18n/en.ts index 9b52b6208..b5cf42e02 100644 --- a/packages/app/src/i18n/en.ts +++ b/packages/app/src/i18n/en.ts @@ -724,6 +724,7 @@ export const dict = { "desktop.updater.dialog.download": "Download", "desktop.updater.dialog.restart": "Restart", "desktop.updater.dialog.later": "Later", + "desktop.updater.error.downloadMissing": "The downloaded update could not be found. Check for updates again.", // The desktop app menu (macOS native menu bar and the Windows/Linux // titlebar menu) — see desktop-menu.ts. diff --git a/packages/app/src/i18n/es.ts b/packages/app/src/i18n/es.ts index 59dd2c2f8..e76f74348 100644 --- a/packages/app/src/i18n/es.ts +++ b/packages/app/src/i18n/es.ts @@ -55,6 +55,8 @@ export const dict = { "desktop.updater.dialog.ready.title": "Actualización lista", "desktop.updater.dialog.restart": "Reiniciar", "desktop.updater.dialog.later": "Más tarde", + "desktop.updater.error.downloadMissing": + "No se encontró la actualización descargada. Vuelve a buscar actualizaciones.", "desktop.recovery.action.relaunch": "Volver a iniciar", "desktop.recovery.action.exportLogs": "Exportar registros", diff --git a/packages/app/src/i18n/vi.ts b/packages/app/src/i18n/vi.ts index 233098db9..e0aad5786 100644 --- a/packages/app/src/i18n/vi.ts +++ b/packages/app/src/i18n/vi.ts @@ -688,6 +688,7 @@ export const dict = { "desktop.updater.dialog.download": "Tải xuống", "desktop.updater.dialog.restart": "Khởi động lại", "desktop.updater.dialog.later": "Để sau", + "desktop.updater.error.downloadMissing": "Không tìm thấy bản cập nhật đã tải xuống. Hãy kiểm tra cập nhật lại.", // The desktop app menu (macOS native menu bar and the Windows/Linux // titlebar menu) — see desktop-menu.ts. diff --git a/packages/desktop/src/main/mac-update.test.ts b/packages/desktop/src/main/mac-update.test.ts new file mode 100644 index 000000000..aba914e0a --- /dev/null +++ b/packages/desktop/src/main/mac-update.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, test } from "bun:test" +import { spawn } from "node:child_process" +import { existsSync } from "node:fs" +import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises" +import { tmpdir } from "node:os" +import { join } from "node:path" +import { appBundlePath, parseCodeSignature, stageMacUpdate, stagedBundlePath, swapMacUpdate } from "./mac-update" + +describe("parseCodeSignature", () => { + test("recognizes a Developer ID chain", () => { + const report = [ + "Executable=/Applications/CoDev.app/Contents/MacOS/CoDev", + "Identifier=ai.codev.desktop", + "Format=app bundle with Mach-O universal (x86_64 arm64)", + "Signature size=9000", + "Authority=Developer ID Application: Example Corp (ABCDE12345)", + "Authority=Developer ID Certification Authority", + "Authority=Apple Root CA", + "TeamIdentifier=ABCDE12345", + ].join("\n") + expect(parseCodeSignature(report)).toBe("signed") + }) + + test("recognizes the linker's ad-hoc signature electron-builder leaves on unsigned builds", () => { + const report = [ + "Executable=/Applications/CoDev.app/Contents/MacOS/CoDev", + "Identifier=Electron", + "CodeDirectory v=20400 size=392 flags=0x20002(adhoc,linker-signed) hashes=9+0 location=embedded", + "Signature=adhoc", + "TeamIdentifier=not set", + ].join("\n") + expect(parseCodeSignature(report)).toBe("adhoc") + }) + + test("recognizes a bundle with no signature at all", () => { + expect(parseCodeSignature("/tmp/CoDev.app: code object is not signed at all\n")).toBe("unsigned") + expect(parseCodeSignature("")).toBe("unsigned") + }) +}) + +test("appBundlePath walks up from the executable to the bundle", () => { + expect(appBundlePath("/Applications/CoDev.app/Contents/MacOS/CoDev")).toBe("/Applications/CoDev.app") +}) + +// Real ditto, mv, and /bin/sh against throwaway bundles; the fake app is a +// directory tree with a marker file, and the "running app" the swap waits on +// is a short sleep. +describe.skipIf(process.platform !== "darwin")("unsigned mac update", () => { + async function bundle(root: string, name: string, marker: string) { + const app = join(root, name) + await mkdir(join(app, "Contents", "MacOS"), { recursive: true }) + await writeFile(join(app, "Contents", "MacOS", "run"), "#!/bin/sh\n", { mode: 0o755 }) + await writeFile(join(app, "Contents", "version"), marker) + return app + } + + async function zipped(root: string, app: string) { + const zip = join(root, "update.zip") + await new Promise((resolve, reject) => { + const child = spawn("ditto", ["-c", "-k", "--keepParent", app, zip]) + child.on("error", reject) + child.on("exit", (code) => (code === 0 ? resolve() : reject(new Error(`ditto exited ${code}`)))) + }) + return zip + } + + test("stages the bundle from the archive beside the installed app", async () => { + const root = await mkdtemp(join(tmpdir(), "codev-mac-update-")) + try { + const installed = await bundle(join(root, "Applications"), "CoDev.app", "1") + const zip = await zipped(root, await bundle(join(root, "build"), "CoDev.app", "2")) + + const staged = await stageMacUpdate({ zip, app: installed }) + + expect(staged).toBe(stagedBundlePath(installed)) + expect(await readFile(join(staged, "Contents", "version"), "utf8")).toBe("2") + expect(await readFile(join(installed, "Contents", "version"), "utf8")).toBe("1") + expect((await readdir(tmpdir())).some((entry) => entry.startsWith("codev-update-"))).toBe(false) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + test("rejects an archive without a bundle and leaves the app alone", async () => { + const root = await mkdtemp(join(tmpdir(), "codev-mac-update-")) + try { + const installed = await bundle(join(root, "Applications"), "CoDev.app", "1") + await mkdir(join(root, "build", "notes"), { recursive: true }) + await writeFile(join(root, "build", "notes", "README"), "nothing here") + const zip = await zipped(root, join(root, "build", "notes")) + + await expect(stageMacUpdate({ zip, app: installed })).rejects.toThrow("No application bundle") + expect(existsSync(stagedBundlePath(installed))).toBe(false) + expect(await readFile(join(installed, "Contents", "version"), "utf8")).toBe("1") + } finally { + await rm(root, { recursive: true, force: true }) + } + }) + + test("swaps the staged bundle in once the app exits and removes the archive", async () => { + const root = await mkdtemp(join(tmpdir(), "codev-mac-update-")) + try { + const installed = await bundle(join(root, "Applications"), "CoDev.app", "1") + const zip = await zipped(root, await bundle(join(root, "build"), "CoDev.app", "2")) + const staged = await stageMacUpdate({ zip, app: installed }) + const running = spawn("sleep", ["0.5"]) + const log = join(root, "install.log") + + const swap = swapMacUpdate({ app: installed, staged, zip, pid: running.pid!, log, relaunch: false }) + await new Promise((resolve) => swap.on("exit", () => resolve())) + + expect(await readFile(log, "utf8")).toContain(`installed ${installed}`) + expect(await readFile(join(installed, "Contents", "version"), "utf8")).toBe("2") + expect(existsSync(staged)).toBe(false) + expect(existsSync(zip)).toBe(false) + expect((await readdir(join(root, "Applications"))).filter((entry) => entry.includes(".old."))).toEqual([]) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) +}) diff --git a/packages/desktop/src/main/mac-update.ts b/packages/desktop/src/main/mac-update.ts new file mode 100644 index 000000000..0ddab73e9 --- /dev/null +++ b/packages/desktop/src/main/mac-update.ts @@ -0,0 +1,132 @@ +import { execFile, spawn } from "node:child_process" +import { openSync, closeSync } from "node:fs" +import { mkdtemp, readdir, rm } from "node:fs/promises" +import { tmpdir } from "node:os" +import { basename, dirname, join, resolve } from "node:path" +import { promisify } from "node:util" + +// Fork: macOS updates for builds Squirrel.Mac refuses to install. +// +// electron-updater hands a downloaded update to Squirrel.Mac (ShipIt), which +// validates the new bundle against the running app's designated requirement +// before swapping it in. That requirement needs a real signing identity: an +// unsigned or ad-hoc signed build fails with "code object is not signed at +// all" (or a cdhash mismatch), and the failure arrives on an event nobody +// shows, so "Install and restart" silently does nothing. Windows verifies no +// signature (verifyUpdateCodeSignature: false) and AppImage just replaces the +// file, which is why only macOS is affected. +// +// Fleet builds are made without an Apple Developer ID, so this module does +// what ShipIt would have: extract the zip electron-updater already fetched +// and verified (sha512 against latest-mac.yml), stage the new bundle next to +// the installed one, and once the app has exited, rename the two and launch +// the new one. Everything that can fail — no room, no permission, no .app in +// the archive — happens while the app is still running, so a failure is an +// error the user can read rather than a dead app. + +const execFileAsync = promisify(execFile) + +export type CodeSignature = "signed" | "adhoc" | "unsigned" + +// `codesign --display --verbose=2` reports on stderr and exits non-zero for +// an unsigned bundle; both streams are folded into one report by the caller. +export function parseCodeSignature(report: string): CodeSignature { + if (/code object is not signed at all/.test(report)) return "unsigned" + if (/^Signature=adhoc$/m.test(report)) return "adhoc" + if (/^Authority=/m.test(report)) return "signed" + return "unsigned" +} + +export async function readCodeSignature(appPath: string) { + const report = await execFileAsync("codesign", ["--display", "--verbose=2", appPath]).then( + (result) => result.stderr + result.stdout, + (error: { stderr?: string; stdout?: string }) => (error.stderr ?? "") + (error.stdout ?? ""), + ) + return parseCodeSignature(report) +} + +// Contents/MacOS/ → .app +export function appBundlePath(execPath = process.execPath) { + return resolve(execPath, "../../..") +} + +export function stagedBundlePath(appPath: string) { + return join(dirname(appPath), `.${basename(appPath)}.update`) +} + +// Extracts the update and moves its bundle beside the installed one. A stale +// stage from an earlier attempt is replaced, so a crash mid-way self-heals on +// the next click. `ditto` rather than a JS unzipper: it is what Finder uses, +// so symlinks, permissions, and resource forks inside the bundle survive. +export async function stageMacUpdate(input: { zip: string; app: string }) { + const scratch = await mkdtemp(join(tmpdir(), "codev-update-")) + try { + await execFileAsync("ditto", ["-x", "-k", input.zip, scratch]) + const name = (await readdir(scratch)).find((entry) => entry.endsWith(".app")) + if (!name) throw new Error(`No application bundle found in ${basename(input.zip)}`) + const staged = stagedBundlePath(input.app) + await rm(staged, { recursive: true, force: true }) + // /bin/mv copies when the temp dir sits on another volume; fs.rename would + // fail with EXDEV. + await execFileAsync("mv", [join(scratch, name), staged]) + return staged + } finally { + await rm(scratch, { recursive: true, force: true }) + } +} + +// Detached shell that waits for the app to exit, swaps the bundles with two +// renames (the old one is kept until the new one is in place, so a failed +// second rename restores it), drops the quarantine flag the new files never +// had anyway, and relaunches. Output goes to `log`; nothing here can report +// back to the app, which is gone by then. +const swapScript = ` +set -u +app="$1"; staged="$2"; pid="$3"; zip="$4"; relaunch="$5" +i=0 +while kill -0 "$pid" 2>/dev/null; do + i=$((i + 1)) + if [ "$i" -ge 300 ]; then echo "timed out waiting for pid $pid to exit"; exit 1; fi + sleep 0.2 +done +old="$app.old.$$" +mv "$app" "$old" || { echo "could not move aside $app"; exit 1; } +if ! mv "$staged" "$app"; then + echo "could not move $staged into place; restoring" + mv "$old" "$app" + exit 1 +fi +rm -rf "$old" +rm -f "$zip" +xattr -dr com.apple.quarantine "$app" 2>/dev/null || true +echo "installed $app" +if [ "$relaunch" = "1" ]; then open "$app"; fi +` + +export function swapMacUpdate(input: { + app: string + staged: string + zip: string + pid: number + log: string + relaunch?: boolean +}) { + const fd = openSync(input.log, "a") + const child = spawn( + "/bin/sh", + [ + "-c", + swapScript, + "codev-update", + input.app, + input.staged, + String(input.pid), + input.zip, + input.relaunch === false ? "0" : "1", + ], + { detached: true, stdio: ["ignore", fd, fd] }, + ) + closeSync(fd) + child.unref() + return child +} diff --git a/packages/desktop/src/main/updater-controller.test.ts b/packages/desktop/src/main/updater-controller.test.ts index 0646db621..c4d1f88e2 100644 --- a/packages/desktop/src/main/updater-controller.test.ts +++ b/packages/desktop/src/main/updater-controller.test.ts @@ -102,10 +102,7 @@ describe("updater controller", () => { expect(app.controller.getState()).toEqual({ status: "ready", version: "2.0.0" }) }) - test("returns to ready when installation cannot start", async () => { - const app = setup() - await app.controller.start() - + test("reports an error when installation cannot start", async () => { const failed = createUpdaterController({ enabled: true, currentVersion: "1.0.0", @@ -122,7 +119,63 @@ describe("updater controller", () => { await failed.start() await expect(failed.install()).rejects.toThrow("stop failed") - expect(failed.getState()).toEqual({ status: "ready", version: "2.0.0" }) + expect(failed.getState()).toEqual({ status: "error", message: "stop failed" }) + }) + + test("prepares the install before stopping the sidecars", async () => { + const calls: string[] = [] + const controller = createUpdaterController({ + enabled: true, + currentVersion: "1.0.0", + backend: { + checkForUpdates: async () => ({ isUpdateAvailable: true, updateInfo: { version: "2.0.0" } }), + downloadUpdate: async () => {}, + prepareInstall: async () => { + calls.push("prepare") + }, + quitAndInstall: async () => { + calls.push("install") + }, + }, + persistence: { get: () => undefined, set() {}, clear() {} }, + stop: async () => { + calls.push("stop") + }, + }) + await controller.start() + + await controller.install() + + expect(calls).toEqual(["prepare", "stop", "install"]) + }) + + // The sidecars are what a failed preparation must not have touched: the + // app keeps working, and the dialog shows what went wrong. + test("a failed preparation leaves the sidecars running and reports the error", async () => { + const calls: string[] = [] + const controller = createUpdaterController({ + enabled: true, + currentVersion: "1.0.0", + backend: { + checkForUpdates: async () => ({ isUpdateAvailable: true, updateInfo: { version: "2.0.0" } }), + downloadUpdate: async () => {}, + prepareInstall: async () => { + throw new Error("No application bundle found in update.zip") + }, + quitAndInstall: () => { + calls.push("install") + }, + }, + persistence: { get: () => undefined, set() {}, clear() {} }, + stop: async () => { + calls.push("stop") + }, + }) + await controller.start() + + await expect(controller.install()).rejects.toThrow("No application bundle") + expect(calls).toEqual([]) + expect(controller.getState()).toEqual({ status: "error", message: "No application bundle found in update.zip" }) }) // Both on is what the updater did before either setting existed. diff --git a/packages/desktop/src/main/updater-controller.ts b/packages/desktop/src/main/updater-controller.ts index c3d662937..229cb022a 100644 --- a/packages/desktop/src/main/updater-controller.ts +++ b/packages/desktop/src/main/updater-controller.ts @@ -7,7 +7,10 @@ export type UpdaterReadyRecord = { version: string } export type UpdaterBackend = { checkForUpdates(): Promise<{ isUpdateAvailable?: boolean; updateInfo?: { version?: string } } | null | undefined> downloadUpdate(): Promise - quitAndInstall(): void + // Work that can fail loudly, done while the app is still whole: runs before + // the sidecars are stopped, so a failure leaves a working app. + prepareInstall?(): Promise + quitAndInstall(): void | Promise } type UpdaterPersistence = { @@ -137,16 +140,18 @@ export function createUpdaterController(input: { if (state.status !== "ready") throw new Error("Update is not ready to install") const version = state.version transition({ status: "installing", version }) - await input - .stop() - .then(() => { - input.backend.quitAndInstall() - transition({ status: "ready", version }) - }) - .catch((error) => { - transition({ status: "ready", version }) - throw error - }) + try { + await input.backend.prepareInstall?.() + await input.stop() + await input.backend.quitAndInstall() + transition({ status: "ready", version }) + } catch (error) { + // Shown in the update dialog. Reverting to ready instead left the + // button offering the same failing install with nothing to explain + // why the app was still running. + transition({ status: "error", message: error instanceof Error ? error.message : String(error) }) + throw error + } }, } } diff --git a/packages/desktop/src/main/updater.ts b/packages/desktop/src/main/updater.ts index 3034f1e09..05b6a2380 100644 --- a/packages/desktop/src/main/updater.ts +++ b/packages/desktop/src/main/updater.ts @@ -1,8 +1,10 @@ +import { join } from "node:path" import { app, dialog } from "electron" import pkg from "electron-updater" import { UPDATER_ENABLED } from "./constants" import { createUpdaterController, type UpdaterReadyRecord } from "./updater-controller" import { getLogger } from "./logging" +import { appBundlePath, readCodeSignature, stageMacUpdate, swapMacUpdate } from "./mac-update" import { getStore } from "./store" import { setAppQuitting } from "./windows" import { nativeT } from "./native-translations" @@ -40,6 +42,17 @@ export function setupAutoUpdater(stop: () => Promise) { ...(typeof record.autoDownload === "boolean" ? { autoDownload: record.autoDownload } : {}), } } + // Fork: the zip electron-updater fetched and verified, remembered for the + // unsigned-macOS install path below. Set again on every launch — a cached + // download re-emits the event once its checksum is re-verified. + let downloaded: string | undefined + autoUpdater.on("update-downloaded", (event) => { + downloaded = event.downloadedFile + }) + // Fork: the bundle staged beside the installed app by prepareInstall, + // consumed by quitAndInstall. Only ever set on macOS builds Squirrel.Mac + // cannot install (see mac-update.ts). + let staged: { app: string; path: string; zip: string } | undefined const controller = createUpdaterController({ enabled: UPDATER_ENABLED, initialPrefs: readPrefs(), @@ -48,10 +61,40 @@ export function setupAutoUpdater(stop: () => Promise) { backend: { checkForUpdates: () => autoUpdater.checkForUpdates(), downloadUpdate: () => autoUpdater.downloadUpdate(), + prepareInstall: + process.platform === "darwin" + ? async () => { + const bundle = appBundlePath() + const signature = await readCodeSignature(bundle) + // Squirrel.Mac validates the update against the running app's + // designated requirement, which only a real identity provides. + // A signed build takes the normal path; anything else would + // fail inside ShipIt with "code object is not signed at all". + if (signature === "signed") return + if (!downloaded) throw new Error(nativeT("desktop.updater.error.downloadMissing")) + logger.log("staging update outside Squirrel.Mac", { signature, bundle, zip: downloaded }) + const path = await stageMacUpdate({ zip: downloaded, app: bundle }) + staged = { app: bundle, path, zip: downloaded } + } + : undefined, quitAndInstall: () => { // quitAndInstall closes all windows before emitting before-quit, so // flag the quit first to keep window ids persisted for restore. setAppQuitting() + if (staged) { + const pending = staged + staged = undefined + swapMacUpdate({ + app: pending.app, + staged: pending.path, + zip: pending.zip, + pid: process.pid, + log: join(app.getPath("userData"), "logs", "update-install.log"), + }) + logger.log("update swap scheduled; quitting", { bundle: pending.app }) + app.quit() + return + } try { // Silent, and relaunch afterwards. The installer is assisted // (oneClick: false, so users can choose where to install), which