From 9ca64d104f18fd6ac1a939fc82cfdc3a62048c81 Mon Sep 17 00:00:00 2001 From: yetone Date: Mon, 5 Oct 2026 19:43:10 +0800 Subject: [PATCH] =?UTF-8?q?update,=20gui:=20a=20new=20version=20Windows=20?= =?UTF-8?q?won't=20start=20is=20never=20left=20in=20place=20=E2=80=94=20tr?= =?UTF-8?q?ied=20before=20it=20goes=20in,=20and=20a=20restart=20into=20one?= =?UTF-8?q?=20that=20doesn't=20say=20it=20is=20up=20puts=20the=20running?= =?UTF-8?q?=20one=20back;=20that=20version=20isn't=20downloaded=20again=20?= =?UTF-8?q?by=20the=20clock,=20and=20Settings=20=E2=80=BA=20About=20?= =?UTF-8?q?=E2=80=BA=20Version=20says=20why=20(#894,=20hisiling)=20?= =?UTF-8?q?=E2=80=94=20on=20Windows=2011=20with=20Smart=20App=20Control=20?= =?UTF-8?q?on,=20the=20update=20put=20the=20new=20unsigned=20exe=20in=20pl?= =?UTF-8?q?ace=20of=20the=20running=20one=20and=20restarted=20into=20it;?= =?UTF-8?q?=20CodeIntegrity=20(3033/3077)=20refused=20it,=20magpie=20never?= =?UTF-8?q?=20started=20again=20and=20the=20working=20version=20sat=20in?= =?UTF-8?q?=20.old.=20Now,=20on=20Windows,=20the=20downloaded=20exe=20is?= =?UTF-8?q?=20first=20started=20suspended=20with=20no=20window=20and=20end?= =?UTF-8?q?ed=20at=20once=20(Windows=20checks=20a=20program=20when=20its?= =?UTF-8?q?=20process=20is=20created),=20and=20one=20refused=20stays=20out?= =?UTF-8?q?:=20the=20running=20exe=20isn't=20moved.=20At=20the=20restart?= =?UTF-8?q?=20the=20new=20process=20is=20given=20MAGPIE=5FSTARTED,=20a=20f?= =?UTF-8?q?ile=20it=20writes=20first=20thing=20(AwaitPredecessor);=20the?= =?UTF-8?q?=20old=20one=20waits=20up=20to=2020s=20for=20it=20and,=20if=20t?= =?UTF-8?q?he=20start=20fails=20(ERROR=5FSYSTEM=5FINTEGRITY=5FPOLICY=5FVIO?= =?UTF-8?q?LATION=204551,=201260=20or=20any=20other),=20the=20new=20proces?= =?UTF-8?q?s=20quits=20before=20saying=20so,=20or=20it=20says=20nothing=20?= =?UTF-8?q?in=20time=20(then=20it=20is=20ended),=20puts=20the=20running=20?= =?UTF-8?q?exe=20back=20over=20the=20new=20one=20(the=20new=20one=20rename?= =?UTF-8?q?d=20.blocked=20if=20it=20can't=20go)=20and=20keeps=20running=20?= =?UTF-8?q?instead=20of=20quitting;=20nothing=20notes=20it=20as=20updated?= =?UTF-8?q?=20in=20the=20app.=20The=20version=20is=20noted=20in=20the=20co?= =?UTF-8?q?nfig=20folder=20(update-blocked.json,=20this=20machine's=20only?= =?UTF-8?q?,=20never=20synced;=20gone=20once=20a=20version=20at=20least=20?= =?UTF-8?q?as=20new=20runs);=20the=20clock's=20checks=20leave=20it=20alone?= =?UTF-8?q?=20and=20show=20why,=20a=20Check=20clicked=20downloads=20and=20?= =?UTF-8?q?tries=20it=20once=20more.=20Settings=20=E2=80=BA=20About=20?= =?UTF-8?q?=E2=80=BA=20Version:=20for=20an=20application=20control=20polic?= =?UTF-8?q?y,=20"0.1.x=20didn't=20start=20on=20this=20computer:=20Windows'?= =?UTF-8?q?=20Smart=20App=20Control=20(or=20another=20app=20control=20poli?= =?UTF-8?q?cy)=20blocked=20the=20new=20version,=20which=20isn't=20signed.?= =?UTF-8?q?=20magpie=20stays=20on=200.1.y.=20Download=20it=20and=20run=20i?= =?UTF-8?q?t=20yourself,=20or=20turn=20Smart=20App=20Control=20off=20and?= =?UTF-8?q?=20check=20again",=20else=20the=20reason,=20with=20Download=20a?= =?UTF-8?q?nd=20Check;=20the=20error=20in=20full=20on=20hover.=20en/zh/ja/?= =?UTF-8?q?de.=20A=20user=20who=20double-clicks=20the=20refused=20exe=20is?= =?UTF-8?q?=20refused=20by=20Windows=20before=20magpie=20runs;=20only=20si?= =?UTF-8?q?gning=20helps=20there.=20TestRelaunchRefusedPutsBack=20(4551=20?= =?UTF-8?q?at=20the=20start),=20TestRelaunchSilentPutsBack=20(no=20word=20?= =?UTF-8?q?in=20time,=20killed;=20quit=20at=20once),=20TestRelaunchReporte?= =?UTF-8?q?dStays,=20TestReportStarted,=20TestBlockedNote=20and=20TestUpda?= =?UTF-8?q?teThatWontStart=20(the=20restart=20doesn't=20quit,=20the=20note?= =?UTF-8?q?,=20the=20clock=20not=20downloading=20it=20again,=20a=20Check?= =?UTF-8?q?=20that=20does)=20are=20new=20and=20fail=20without=20the=20chan?= =?UTF-8?q?ge;=20on=20Windows=20TestInstallBinaryRefusedStaysOut,=20TestTr?= =?UTF-8?q?ialStart=20and=20TestStartCheckedPutsBackForReal=20(real=20proc?= =?UTF-8?q?esses:=20not=20a=20program,=20quits=20at=20once,=20runs=20on=20?= =?UTF-8?q?silent)=20pass=20on=20the=20Windows=2011=20test=20box=20with=20?= =?UTF-8?q?the=20whole=20internal/update=20package=20(Smart=20App=20Contro?= =?UTF-8?q?l=20itself=20isn't=20on=20there,=20so=20the=20refusal=20is=20si?= =?UTF-8?q?mulated).=20update-blocked.test.cjs=20(Chromium=20and=20WebKit,?= =?UTF-8?q?=20en=20and=20zh:=20the=20reason=20and=20both=20versions,=20ful?= =?UTF-8?q?l=20error=20on=20hover,=20Download=20opens=20the=20release,=20C?= =?UTF-8?q?heck=20downloads,=20the=20other=20reason,=20no=20select,=20no?= =?UTF-8?q?=20scroll,=20no=20left=20border)=20fails=20without=20the=20chan?= =?UTF-8?q?ge;=20update-check,=20update-mirror,=20update-auto,=20update-wa?= =?UTF-8?q?it=20and=20update-download-percent=20pass=20in=20WebKit;=20clic?= =?UTF-8?q?k-scroll's=20request-list=20case=20fails=20now=20and=20then=20t?= =?UTF-8?q?he=20same=20on=20main.=20go=20vet,=20go=20build=20(darwin,=20GO?= =?UTF-8?q?OS=3Dlinux=20nogui,=20GOOS=3Dwindows)=20and=20go=20test=20-tags?= =?UTF-8?q?=20nogui=20./...=20under=20a=20temp=20HOME=20pass.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/gui/assets/app.js | 14 ++ internal/gui/assets/i18n.js | 6 + internal/gui/tests/update-blocked.test.cjs | 116 ++++++++++ internal/gui/update.go | 98 ++++++-- internal/gui/update_blocked_test.go | 105 +++++++++ internal/update/blocked.go | 218 ++++++++++++++++++ internal/update/blocked_test.go | 149 ++++++++++++ .../update/install_blocked_windows_test.go | 98 ++++++++ internal/update/proc_unix.go | 3 + internal/update/proc_windows.go | 20 ++ internal/update/relaunch_args_test.go | 2 +- internal/update/update.go | 43 ++-- 12 files changed, 840 insertions(+), 32 deletions(-) create mode 100644 internal/gui/tests/update-blocked.test.cjs create mode 100644 internal/gui/update_blocked_test.go create mode 100644 internal/update/blocked.go create mode 100644 internal/update/blocked_test.go create mode 100644 internal/update/install_blocked_windows_test.go diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index efe8cd97..9cf9e3c1 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -17483,6 +17483,20 @@ async function renderUpdate(r, u) { } btn(t("Check"), check); break; + case "blocked": { + // the new version didn't start on this computer (#894) and this one + // runs on: why, and what the user can do. The clock doesn't download + // it again; Check tries it once more. + const b = u.blocked || {}; + sub.textContent = b.policy + ? t("{v} didn't start on this computer: Windows' Smart App Control (or another app control policy) blocked the new version, which isn't signed. magpie stays on {cur}. Download it and run it yourself, or turn Smart App Control off and check again", { v: u.latest || b.version, cur: u.current }) + : t("{v} didn't start on this computer, so magpie stays on {cur}", { v: u.latest || b.version, cur: u.current }) + (b.error ? " · " + b.error : ""); + sub.title = b.error || u.error || ""; + sub.classList.add("wraps"); + if (u.url) btn(t("Download"), () => (web ? window.open(u.url, "_blank", "noopener") : api("open", { url: u.url }))); + btn(t("Check"), check); + break; + } case "": // not asked yet: with automatic updates off, only this asks sub.textContent = prefs && prefs.noAutoUpdate ? t("Automatic updates are off") : ""; btn(t("Check"), check); diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index 83e77bea..eb7e5c86 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -2125,6 +2125,8 @@ const I18N = { "Check": "检查", "Couldn't check for updates": "检查更新失败", "Couldn't download {v}": "{v} 下载失败", + "{v} didn't start on this computer: Windows' Smart App Control (or another app control policy) blocked the new version, which isn't signed. magpie stays on {cur}. Download it and run it yourself, or turn Smart App Control off and check again": "{v} 没能在这台电脑上启动：Windows 的「智能应用控制」（或其他应用控制策略）拦下了未签名的新版本，magpie 仍是 {cur}。可以下载新版本自己运行，或关掉智能应用控制后再检查更新", + "{v} didn't start on this computer, so magpie stays on {cur}": "{v} 没能在这台电脑上启动，magpie 仍是 {cur}", "{done} of {total} MB": "{done} / {total} MB", "{done} MB": "{done} MB", "Downloading…": "正在下载…", @@ -5415,6 +5417,8 @@ const I18N = { "Check": "確認", "Couldn't check for updates": "アップデートを確認できませんでした", "Couldn't download {v}": "{v} をダウンロードできませんでした", + "{v} didn't start on this computer: Windows' Smart App Control (or another app control policy) blocked the new version, which isn't signed. magpie stays on {cur}. Download it and run it yourself, or turn Smart App Control off and check again": "{v} はこのコンピューターで起動できませんでした：Windows の「スマート アプリ コントロール」（または別のアプリ制御ポリシー）が署名のない新しいバージョンをブロックしました。magpie は {cur} のままです。ダウンロードして自分で実行するか、スマート アプリ コントロールをオフにしてから再度確認してください", + "{v} didn't start on this computer, so magpie stays on {cur}": "{v} はこのコンピューターで起動できなかったため、magpie は {cur} のままです", "{done} of {total} MB": "{done} / {total} MB", "{done} MB": "{done} MB", "Downloading…": "ダウンロード中…", @@ -8695,6 +8699,8 @@ const I18N = { "Check": "Prüfen", "Couldn't check for updates": "Suche nach Updates fehlgeschlagen", "Couldn't download {v}": "{v} konnte nicht heruntergeladen werden", + "{v} didn't start on this computer: Windows' Smart App Control (or another app control policy) blocked the new version, which isn't signed. magpie stays on {cur}. Download it and run it yourself, or turn Smart App Control off and check again": "{v} ließ sich auf diesem Computer nicht starten: Die intelligente App-Steuerung von Windows (oder eine andere App-Steuerungsrichtlinie) hat die neue, nicht signierte Version blockiert. magpie bleibt bei {cur}. Lade sie herunter und starte sie selbst, oder schalte die intelligente App-Steuerung aus und prüfe erneut", + "{v} didn't start on this computer, so magpie stays on {cur}": "{v} ließ sich auf diesem Computer nicht starten, daher bleibt magpie bei {cur}", "{done} of {total} MB": "{done} von {total} MB", "{done} MB": "{done} MB", "Downloading…": "Wird heruntergeladen…", diff --git a/internal/gui/tests/update-blocked.test.cjs b/internal/gui/tests/update-blocked.test.cjs new file mode 100644 index 00000000..e4eb2f40 --- /dev/null +++ b/internal/gui/tests/update-blocked.test.cjs @@ -0,0 +1,116 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// #894: on Windows with Smart App Control on, the new unsigned version was +// refused when magpie restarted into it. magpie now puts itself back and +// runs on, and Settings › About › Version says the version didn't start +// here and why, offering Download and Check (which tries it once more). +// No click moves the page, nothing is a native select, no left border. +// English and Chinese; no backend, the API is faked. +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = path.resolve(__dirname, "../assets"); + +const words = { + en: { policy: "Smart App Control", stays: "magpie stays on 0.1.900", download: "Download", check: "Check", other: "didn't start on this computer" }, + zh: { policy: "智能应用控制", stays: "magpie 仍是 0.1.900", download: "下载", check: "检查", other: "没能在这台电脑上启动" }, +}; + +const blocked = (policy) => ({ state: "blocked", current: "0.1.900", latest: "0.1.901", url: "https://github.com/yetone/magpie-releases/releases/tag/v0.1.901", + blocked: { version: "0.1.901", policy, error: policy ? "fork/exec magpie.exe: An Application Control policy has blocked this file." : "it quit as soon as it started" } }); + +function server(lang, ctl) { + return async (route) => { + const req = route.request(), url = new URL(req.url()); + const json = (data, status) => route.fulfill({ json: data, status: status || 200 }); + const settings = () => ({ theme: "light", lang, version: "0.1.900", dir: "~/.config/magpie", gateway: "http://127.0.0.1:3425", lanURLs: [], fx: { rate: 7.2 } }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:false};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return json({ agents: [], profiles: [], settings: settings() }); + if (url.pathname === "/api/update/check") { + ctl.posts.push("check"); + ctl.update = { state: "downloading", current: "0.1.900", latest: "0.1.901", done: 1e6, total: 3e7 }; + return json(ctl.update); + } + if (url.pathname === "/api/open") { ctl.posts.push(["open", req.postDataJSON()]); return json({}); } + if (url.pathname === "/api/update") return json(ctl.update); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/groups") return json({ groups: [], models: [] }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + try { await route.fulfill({ body: await fs.readFile(file), contentType }); } catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +const scrolls = (page) => page.evaluate(() => [window.scrollY, document.scrollingElement.scrollTop, ...[...document.querySelectorAll(".view")].map((v) => v.scrollTop)].join(",")); + +async function open(browser, lang, ctl, errors) { + const page = await (await browser.newContext({ viewport: { width: 900, height: 760 }, reducedMotion: "reduce" })).newPage(); + page.setDefaultTimeout(5000); + page.on("pageerror", (e) => errors.push(e.message)); + await page.route("**/*", server(lang, ctl)); + await page.goto("http://magpie.test/?view=settings"); + await page.locator("#setTab-about").click(); + await page.locator("#setPage-about").waitFor({ state: "visible" }); + return page; +} + +async function click(page, loc) { + const before = await scrolls(page); + await loc.click(); + await page.waitForTimeout(150); + assert.equal(await scrolls(page), before, "the click must not scroll the page"); +} + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + test(engine + ": a version this computer didn't start says why", async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + for (const lang of ["en", "zh"]) { + const w = words[lang]; + await t.test(lang + ": Smart App Control", async () => { + const ctl = { posts: [], update: blocked(true) }; + const errors = []; + const page = await open(browser, lang, ctl, errors); + const ver = page.locator("#about .row.pref", { has: page.locator(".sub", { hasText: w.policy }) }); + await ver.waitFor(); + const sub = await ver.locator(".sub").innerText(); + assert.ok(sub.includes("0.1.901") && sub.includes(w.stays), sub); + assert.equal(await ver.locator(".sub").getAttribute("title"), ctl.update.blocked.error, "the error in full on hover"); + assert.equal(await page.locator("#about select").count(), 0, "no native select"); + const widths = await ver.evaluate((e) => [e, ...e.querySelectorAll("*")].map((x) => parseFloat(getComputedStyle(x).borderLeftWidth) || 0)); + assert.ok(widths.every((x) => x === 0), "no left border"); + // the text wraps inside the row, not out of it + const fits = await ver.evaluate((e) => { const r = e.getBoundingClientRect(), s = e.querySelector(".sub").getBoundingClientRect(); return s.right <= r.right + 1 && s.left >= r.left - 1; }); + assert.ok(fits, "the reason stays in the row"); + + await click(page, ver.locator("button", { hasText: w.download })); + assert.deepEqual(ctl.posts.at(-1), ["open", { url: ctl.update.url }]); + // Check tries it again: downloading + await click(page, ver.locator("button", { hasText: w.check })); + await page.locator("#about .sub", { hasText: "0.1.901" }).filter({ hasText: "%" }).waitFor(); + assert.ok(ctl.posts.includes("check")); + assert.deepEqual(errors, []); + await page.context().close(); + }); + + await t.test(lang + ": another reason", async () => { + const ctl = { posts: [], update: blocked(false) }; + const errors = []; + const page = await open(browser, lang, ctl, errors); + const ver = page.locator("#about .row.pref", { has: page.locator(".sub", { hasText: w.other }) }); + await ver.waitFor(); + const sub = await ver.locator(".sub").innerText(); + assert.ok(!sub.includes(w.policy), sub); + assert.ok(sub.includes("it quit as soon as it started") && sub.includes(w.stays), sub); + assert.equal(await ver.locator("button", { hasText: w.check }).count(), 1); + assert.deepEqual(errors, []); + await page.context().close(); + }); + } + }); +} diff --git a/internal/gui/update.go b/internal/gui/update.go index 163a8555..57604660 100644 --- a/internal/gui/update.go +++ b/internal/gui/update.go @@ -26,17 +26,23 @@ import ( // restart asks for the administrator's password; only an app that can't be // replaced at all (run from its disk image, say) is sent to the release page. type updater struct { - mu sync.Mutex - state string // checking | latest | downloading | ready | available | source | error - latest *update.Release - err string - bundle string // the .app to replace, "" when not in one or stuck - stuck string // why the .app can't be replaced where it is (update.Stuck) - exe string // off the Mac: the binary to replace, "" when not writable - self os.FileInfo // exe as this process started from it - retry bool // error: the download failed, and may be tried again - mirror string // error: the mirror the failed download came through (#893) - staged string + mu sync.Mutex + state string // checking | latest | downloading | ready | available | source | error | blocked + latest *update.Release + err string + bundle string // the .app to replace, "" when not in one or stuck + stuck string // why the .app can't be replaced where it is (update.Stuck) + exe string // off the Mac: the binary to replace, "" when not writable + self os.FileInfo // exe as this process started from it + retry bool // error: the download failed, and may be tried again + mirror string // error: the mirror the failed download came through (#893) + staged string + aside string // Windows: where install moved the running exe, to put back + // a version this computer wouldn't start (Smart App Control, #894), + // which the clock's checks don't download again; byClock, the check + // under way is the clock's + blocked *update.Blocked + byClock bool asked time.Time // when the feed was last asked, by the clock or a click done int64 // downloading: bytes so far, of total (0 when unknown) total int64 @@ -69,9 +75,11 @@ type updateJSON struct { // error: the download failed through this mirror (Settings' UpdateMirror), // and GitHub itself may do (#893) Mirror string `json:"mirror,omitempty"` - Error string `json:"error,omitempty"` - Done int64 `json:"done,omitempty"` // downloading: bytes so far - Total int64 `json:"total,omitempty"` + Error string `json:"error,omitempty"` + Done int64 `json:"done,omitempty"` // downloading: bytes so far + // blocked: the version this computer wouldn't start, and why (#894) + Blocked *update.Blocked `json:"blocked,omitempty"` + Total int64 `json:"total,omitempty"` // ready: what the gateway this process serves has in flight, which a // restart would cut short; Waiting, the restart waits for it to end; // GaveUp, the last wait ran out with it still busy @@ -101,6 +109,7 @@ func (u *updater) start() { u.self, _ = os.Stat(exe) update.RemoveOld(exe) // what the last updates on Windows moved aside update.RemoveStaleNew(exe) // what a magpie left running downloaded again + u.blocked = update.ReadBlocked(Version) } } go func() { @@ -110,7 +119,7 @@ func (u *updater) start() { last := u.asked u.mu.Unlock() if updateDue(settings.Load(), last, time.Now()) { - u.check() + u.checkByClock() } time.Sleep(time.Minute) } @@ -124,6 +133,17 @@ func (u *updater) check() { } } +// checkByClock is check, by the clock rather than a click: a version this +// computer wouldn't start is left as it is. +func (u *updater) checkByClock() { + if u.begin() { + u.mu.Lock() + u.byClock = true + u.mu.Unlock() + u.run() + } +} + // recheck asks the feed again before a restart into what was downloaded, // so a newer version out since is the one restarted into. It waits for // the feed a little while; a newer version is then downloading. @@ -151,7 +171,7 @@ func (u *updater) begin() bool { if u.state == "checking" || u.state == "downloading" { return false } - u.state, u.err, u.retry, u.mirror, u.asked = "checking", "", false, "", time.Now() + u.state, u.err, u.retry, u.mirror, u.asked, u.byClock = "checking", "", false, "", time.Now(), false return true } @@ -186,6 +206,11 @@ func (u *updater) run() { case u.bundle == "" && u.exe == "": u.state = "available" // the user fetches it from the release page return + case u.byClock && u.blocked != nil && u.blocked.Version == rel.Version: + // this computer didn't start it (#894): not downloaded again by + // itself; a check asked for tries it once more + u.state = "blocked" + return case u.replaced(): // another magpie put the update in already: a restart is all u.state = "ready" @@ -245,12 +270,17 @@ func (u *updater) install(ask bool) bool { err = update.InstallAsAdmin(u.staged, u.bundle) } } else { - err = update.InstallBinary(u.staged, u.exe) + u.aside, err = update.InstallBinaryAside(u.staged, u.exe) if ask && update.NeedsAdmin(err) && update.CanElevate() { err = update.InstallBinaryAsAdmin(u.staged, u.exe) } } u.err = "" + if errors.As(err, new(*update.BlockedError)) { + u.staged = "" // removed: this computer won't run it + u.wontStart(err) + return false + } if err != nil { if !errors.Is(err, update.ErrCanceled) { log.Println("update:", err) @@ -262,6 +292,20 @@ func (u *updater) install(ask bool) bool { return true } +// wontStart notes that the version downloaded didn't start here: it is +// not downloaded again by the clock, and the version row says why. Called +// with u.mu held. +func (u *updater) wontStart(err error) { + log.Println("update:", err) + v := "" + if u.latest != nil { + v = u.latest.Version + } + b := update.NoteBlocked(v, err) + u.blocked, u.state, u.aside = &b, "blocked", "" + u.err = err.Error() +} + // replaced is whether the binary was updated by another magpie since this // one started, which runs on from where it was moved aside. func (u *updater) replaced() bool { @@ -297,6 +341,9 @@ func (u *updater) jsonIn(lang string) updateJSON { if u.state == "downloading" { j.Done, j.Total = u.done, u.total } + if u.state == "blocked" { + j.Blocked = u.blocked + } if j.State == "ready" || u.waiting { j.Waiting, j.GaveUp = u.waiting, u.gaveUp && !u.waiting if busy.Any() { @@ -421,6 +468,9 @@ func updateRoutes(mux *http.ServeMux, w Windows) { }) } +// relaunchBinary is update.RelaunchBinary; tests stand in for it. +var relaunchBinary = update.RelaunchBinary + // restartToUpdate installs the staged version and arranges for it to open // once this process is gone; the caller then quits. magpie web runs on as // the new version in its own place (Web.Wait) instead of opening the app. @@ -446,7 +496,19 @@ func restartToUpdate(web, window bool, view string) bool { if bundle != "" { err = update.Relaunch(bundle) } else { - err = update.RelaunchBinary(exe, window, view) + updates.mu.Lock() + aside := updates.aside + updates.mu.Unlock() + err = relaunchBinary(exe, aside, window, view) + } + if errors.As(err, new(*update.BlockedError)) { + // the new version didn't start and this one is back in its place + // (#894): it runs on, and says why + os.Remove(filepath.Join(settings.Dir(), updatedInAppFile)) + updates.mu.Lock() + updates.wontStart(err) + updates.mu.Unlock() + return false } if err != nil { log.Println("update:", err) diff --git a/internal/gui/update_blocked_test.go b/internal/gui/update_blocked_test.go new file mode 100644 index 00000000..e8ab9ec9 --- /dev/null +++ b/internal/gui/update_blocked_test.go @@ -0,0 +1,105 @@ +package gui + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync/atomic" + "syscall" + "testing" + + "github.com/yetone/magpie/internal/settings" + "github.com/yetone/magpie/internal/update" +) + +// #894: on Windows with Smart App Control on, the new unsigned exe was +// put in and refused at the restart, and magpie never started again. A +// restart into a version that doesn't start leaves this one running and +// says why; the clock's checks don't download that version again, and +// a check asked for tries it once more. +func TestUpdateThatWontStart(t *testing.T) { + body := []byte("new magpie") + sum := sha256.Sum256(body) + var downloads atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/asset" { + downloads.Add(1) + w.Write(body) + return + } + json.NewEncoder(w).Encode(update.Release{Version: "0.1.11", Assets: map[string]update.Asset{ + update.BinaryAsset(): {URL: "http://" + r.Host + "/asset", SHA256: hex.EncodeToString(sum[:])}, + }}) + })) + defer srv.Close() + t.Setenv("MAGPIE_UPDATE_FEED", srv.URL) + oldV := Version + Version = "0.1.10" + defer func() { Version = oldV }() + t.Cleanup(update.ClearBlocked) + + // the running binary, in a folder of its own + exe := filepath.Join(t.TempDir(), "magpie") + os.WriteFile(exe, []byte("running magpie"), 0o755) + self, _ := os.Stat(exe) + staged := exe + ".new" + os.WriteFile(staged, body, 0o755) + + o := updates + u := &updater{exe: exe, self: self, staged: staged, state: "ready", latest: &update.Release{Version: "0.1.11"}} + updates = u + defer func() { updates = o }() + var relaunched atomic.Int32 + wasRelaunch := relaunchBinary + relaunchBinary = func(exe, old string, window bool, view string) error { + relaunched.Add(1) + // what update.RelaunchBinary does when Windows refuses the new exe: + // the running one put back, and says so + os.WriteFile(exe, []byte("running magpie"), 0o755) + return &update.BlockedError{Err: &os.PathError{Op: "fork/exec", Path: exe, Err: syscall.Errno(4551)}} + } + defer func() { relaunchBinary = wasRelaunch }() + + if restartToUpdate(false, true, "settings") { + t.Fatal("restartToUpdate said to quit, with nothing left that starts") + } + if relaunched.Load() != 1 { + t.Fatalf("relaunched %d times", relaunched.Load()) + } + j := u.json() + if j.State != "blocked" || j.Blocked == nil || j.Blocked.Version != "0.1.11" || !j.Blocked.Policy { + t.Fatalf("after the refused restart: %+v", j) + } + if _, err := os.Stat(filepath.Join(settings.Dir(), updatedInAppFile)); !os.IsNotExist(err) { + t.Errorf("noted as updated in the app: %v", err) + } + + // magpie starts again later (the note is on disk): the clock's check + // leaves the version alone + u2 := &updater{exe: exe} + u2.self, _ = os.Stat(exe) + u2.blocked = update.ReadBlocked(Version) + if u2.blocked == nil { + t.Fatal("the refused version isn't noted") + } + u2.checkByClock() + if j := u2.json(); j.State != "blocked" || j.Blocked == nil || downloads.Load() != 0 { + t.Fatalf("clock's check: %+v, %d downloads", j, downloads.Load()) + } + // Check, clicked: it is downloaded to be tried again + if self, err := update.Executable(); err == nil { + defer os.Remove(self + ".new") // where StageBinary puts it + } + u2.check() + if j := u2.json(); j.State != "ready" || downloads.Load() != 1 { + t.Fatalf("clicked check: %+v, %d downloads", j, downloads.Load()) + } + // once this version is the one running, the note goes + if update.ReadBlocked("0.1.11") != nil { + t.Error("the note outlived the version") + } +} diff --git a/internal/update/blocked.go b/internal/update/blocked.go new file mode 100644 index 00000000..345570bb --- /dev/null +++ b/internal/update/blocked.go @@ -0,0 +1,218 @@ +package update + +// A new version that this computer won't run (#894): Windows 11's Smart +// App Control, or another application control policy, refuses magpie's +// unsigned builds it doesn't know yet, and an update that put one in place +// left nothing that starts. The download is tried before it goes in +// (canStart), the restart into it waits for it to say it is up +// (AwaitPredecessor writes MAGPIE_STARTED's file), and a version refused +// either way is put back and noted (NoteBlocked), so the automatic checks +// don't download it again; a check asked for tries it once more. + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "syscall" + "time" + + "github.com/yetone/magpie/internal/appdir" + "github.com/yetone/magpie/internal/proc" +) + +// BlockedError is a new version that couldn't be started on this computer. +// Nothing of it is left in place: the version running is still the one +// installed. +type BlockedError struct { + Err error +} + +func (e *BlockedError) Error() string { + if e.Policy() { + return "Windows didn't let the new version start (" + e.Err.Error() + "): Smart App Control or another application control policy blocks magpie's unsigned builds; download it from " + Site + " and run it yourself, or turn Smart App Control off, then check for updates again" + } + return "the new version couldn't start: " + e.Err.Error() + "; download it from " + Site + " and run it yourself" +} + +func (e *BlockedError) Unwrap() error { return e.Err } + +// Policy is whether Windows' code integrity refused the program: Smart App +// Control, App Control for Business (WDAC), AppLocker or a software +// restriction policy, which the user can't let through from magpie. +func (e *BlockedError) Policy() bool { return policyRefused(e.Err) } + +// errAccessDisabledByPolicy is AppLocker's and software restriction +// policies' refusal; ERROR_SYSTEM_INTEGRITY_* (4550…4599) are code +// integrity's, Smart App Control's among them ("An Application Control +// policy has blocked this file", ERROR_SYSTEM_INTEGRITY_POLICY_VIOLATION). +const ( + errAccessDisabledByPolicy syscall.Errno = 1260 + errIntegrityPolicy syscall.Errno = 4551 +) + +func policyRefused(err error) bool { + var n syscall.Errno + if !errors.As(err, &n) { + return false + } + return n == errAccessDisabledByPolicy || n >= 4550 && n < 4600 +} + +// canStart is whether the program at path can be started here, which is +// tried before it is put in place: Windows starts it suspended and ends +// it at once, so none of it runs; elsewhere nothing refuses a build that +// downloaded whole. A var for tests. +var canStart = trialStart + +// startWait is how long a restarted magpie has to say it is up. +var startWait = 20 * time.Second + +// startedEnv names the file a relaunched magpie writes once it runs +// (AwaitPredecessor), for the one that started it to know. +const startedEnv = "MAGPIE_STARTED" + +// launch starts a program with args and env, detached from this one; the +// channel is closed when it exits, and kill ends it. A var for tests. +var launch = func(exe string, args, env []string) (<-chan struct{}, func(), error) { + cmd := proc.Command(exe, args...) + cmd.Env = env + cmd.Stdin, cmd.Stdout, cmd.Stderr = nil, nil, nil + detach(cmd) + if err := cmd.Start(); err != nil { + return nil, nil, err + } + exited := make(chan struct{}) + go func() { cmd.Wait(); close(exited) }() + return exited, func() { cmd.Process.Kill() }, nil +} + +// startChecked starts the new version at exe and waits for it to say it +// is up. When it can't be started, quits first, or says nothing in time, +// it is ended and old — this running version, moved aside for it — goes +// back in its place. +func startChecked(exe, old string, args, env []string) error { + marker := filepath.Join(os.TempDir(), "magpie-started-"+strconv.Itoa(os.Getpid())) + os.Remove(marker) + defer os.Remove(marker) + exited, kill, err := launch(exe, args, append(env, startedEnv+"="+marker)) + if err == nil { + err = awaitStarted(marker, exited, kill) + } + if err == nil { + return nil + } + blocked := &BlockedError{Err: err} + if rerr := putBack(exe, old); rerr != nil { + return errors.Join(blocked, rerr) + } + return blocked +} + +// awaitStarted waits for marker to be written, the started program's +// exit (exited closing) or startWait, whichever comes first. +func awaitStarted(marker string, exited <-chan struct{}, kill func()) error { + tick := time.NewTicker(50 * time.Millisecond) + defer tick.Stop() + deadline := time.After(startWait) + for { + if _, err := os.Stat(marker); err == nil { + return nil + } + select { + case <-exited: + if _, err := os.Stat(marker); err == nil { + return nil // it said so and then went on as another + } + return errors.New("it quit as soon as it started") + case <-deadline: + kill() + // its file let go of, to be put back over + select { + case <-exited: + case <-time.After(5 * time.Second): + } + return fmt.Errorf("it didn't start within %s", startWait) + case <-tick.C: + } + } +} + +// putBack puts old in exe's place, the new version that was there out of +// the way (removed, else kept as exe.blocked). +func putBack(exe, old string) error { + var err error + for i := 0; ; i++ { + if err = os.Remove(exe); err == nil || os.IsNotExist(err) { + break + } + if err = os.Rename(exe, exe+".blocked"); err == nil { + break + } + if i == len(asideWaits) { + return fmt.Errorf("couldn't take the new version out of %s: %w", filepath.Base(exe), err) + } + time.Sleep(asideWaits[i]) + } + if err := os.Rename(old, exe); err != nil { + return fmt.Errorf("couldn't put %s back from %s: %w", filepath.Base(exe), old, err) + } + return nil +} + +// reportStarted writes the file the magpie that started this one waits +// for, once. +func reportStarted() { + p := os.Getenv(startedEnv) + if p == "" { + return + } + os.Unsetenv(startedEnv) + os.WriteFile(p, []byte(strconv.Itoa(os.Getpid())), 0o644) +} + +// Blocked is a version this computer refused to start, and why. +type Blocked struct { + Version string `json:"version"` + Error string `json:"error"` + Policy bool `json:"policy,omitempty"` // application control (Smart App Control) + At time.Time `json:"at"` +} + +// blockedFile is this computer's own note, never synced. +func blockedFile() string { return filepath.Join(appdir.Config(), "update-blocked.json") } + +// NoteBlocked notes that version couldn't start here. +func NoteBlocked(version string, err error) Blocked { + b := Blocked{Version: version, Error: err.Error(), At: time.Now()} + var be *BlockedError + if errors.As(err, &be) { + b.Error, b.Policy = be.Err.Error(), be.Policy() + } + if data, merr := json.Marshal(b); merr == nil { + os.MkdirAll(filepath.Dir(blockedFile()), 0o755) + os.WriteFile(blockedFile(), data, 0o644) + } + return b +} + +// ReadBlocked is the version noted as refused here, while it is newer +// than current; once current has caught up (put in by hand), the note +// goes. +func ReadBlocked(current string) *Blocked { + data, err := os.ReadFile(blockedFile()) + if err != nil { + return nil + } + var b Blocked + if json.Unmarshal(data, &b) != nil || b.Version == "" || !Newer(b.Version, current) { + os.Remove(blockedFile()) + return nil + } + return &b +} + +// ClearBlocked forgets the note: the version started after all. +func ClearBlocked() { os.Remove(blockedFile()) } diff --git a/internal/update/blocked_test.go b/internal/update/blocked_test.go new file mode 100644 index 00000000..7895977f --- /dev/null +++ b/internal/update/blocked_test.go @@ -0,0 +1,149 @@ +package update + +import ( + "errors" + "os" + "path/filepath" + "strings" + "sync/atomic" + "syscall" + "testing" + "time" +) + +// relaunchFiles is an update just put in: exe the new version, old the +// running one moved aside. +func relaunchFiles(t *testing.T) (exe, old string) { + t.Helper() + dir := t.TempDir() + exe, old = filepath.Join(dir, "magpie.exe"), filepath.Join(dir, "magpie.exe.old") + os.WriteFile(exe, []byte("new version"), 0o755) + os.WriteFile(old, []byte("running version"), 0o755) + return exe, old +} + +func stubLaunch(t *testing.T, f func(exe string, args, env []string) (<-chan struct{}, func(), error)) { + t.Helper() + was := launch + launch = f + t.Cleanup(func() { launch = was }) +} + +func wantPutBack(t *testing.T, exe, old string) { + t.Helper() + if b, err := os.ReadFile(exe); err != nil || string(b) != "running version" { + t.Errorf("%s = %q (%v), want the running version back", filepath.Base(exe), b, err) + } + if _, err := os.Stat(old); !os.IsNotExist(err) { + t.Errorf("%s still there: %v", filepath.Base(old), err) + } +} + +// #894: Smart App Control refuses the new, unsigned exe when it is +// started (ERROR_SYSTEM_INTEGRITY_POLICY_VIOLATION): the running version +// goes back in its place, and the error says it was Windows' policy. +func TestRelaunchRefusedPutsBack(t *testing.T) { + exe, old := relaunchFiles(t) + stubLaunch(t, func(string, []string, []string) (<-chan struct{}, func(), error) { + return nil, nil, &os.PathError{Op: "fork/exec", Path: exe, Err: errIntegrityPolicy} + }) + err := RelaunchBinary(exe, old, true, "settings") + var be *BlockedError + if !errors.As(err, &be) || !be.Policy() { + t.Fatalf("err = %v, want a policy *BlockedError", err) + } + if !strings.Contains(err.Error(), "Smart App Control") { + t.Errorf("err doesn't say why: %v", err) + } + wantPutBack(t, exe, old) +} + +// A new version that starts and says nothing in time is ended and the +// running one put back; one that quits at once likewise. +func TestRelaunchSilentPutsBack(t *testing.T) { + was := startWait + startWait = 200 * time.Millisecond + t.Cleanup(func() { startWait = was }) + for _, quits := range []bool{false, true} { + exe, old := relaunchFiles(t) + var killed atomic.Bool + stubLaunch(t, func(string, []string, []string) (<-chan struct{}, func(), error) { + exited := make(chan struct{}) + if quits { + close(exited) + return exited, func() {}, nil + } + return exited, func() { killed.Store(true); close(exited) }, nil + }) + err := RelaunchBinary(exe, old, false, "") + var be *BlockedError + if !errors.As(err, &be) || be.Policy() { + t.Fatalf("quits=%v: err = %v, want a *BlockedError not of policy", quits, err) + } + if !quits && !killed.Load() { + t.Error("the silent new version was left running") + } + wantPutBack(t, exe, old) + } +} + +// A new version that says it is up stays; what was moved aside is left for +// it to remove. +func TestRelaunchReportedStays(t *testing.T) { + exe, old := relaunchFiles(t) + var args []string + stubLaunch(t, func(_ string, a, env []string) (<-chan struct{}, func(), error) { + args = a + for _, kv := range env { + if p, ok := strings.CutPrefix(kv, startedEnv+"="); ok { + go func() { time.Sleep(50 * time.Millisecond); os.WriteFile(p, []byte("1"), 0o644) }() + } + } + return make(chan struct{}), func() { t.Error("killed") }, nil + }) + if err := RelaunchBinary(exe, old, false, ""); err != nil { + t.Fatal(err) + } + if len(args) != 1 || args[0] != "tray" { + t.Errorf("args = %q", args) + } + if b, _ := os.ReadFile(exe); string(b) != "new version" { + t.Errorf("exe = %q", b) + } + if _, err := os.Stat(old); err != nil { + t.Errorf("old: %v", err) + } +} + +// What the started magpie writes is what the one starting it waits for. +func TestReportStarted(t *testing.T) { + p := filepath.Join(t.TempDir(), "started") + t.Setenv(startedEnv, p) + reportStarted() + if _, err := os.Stat(p); err != nil { + t.Fatal(err) + } + if os.Getenv(startedEnv) != "" { + t.Error("left in the environment, for what it starts") + } +} + +// The note of a refused version holds while it is newer than the one +// running, and goes once that has caught up. +func TestBlockedNote(t *testing.T) { + t.Cleanup(ClearBlocked) + NoteBlocked("0.1.20", &BlockedError{Err: errIntegrityPolicy}) + b := ReadBlocked("0.1.19") + if b == nil || b.Version != "0.1.20" || !b.Policy || b.Error == "" { + t.Fatalf("ReadBlocked = %+v", b) + } + if b := ReadBlocked("0.1.20"); b != nil { + t.Fatalf("caught up: %+v", b) + } + if b := ReadBlocked("0.1.19"); b != nil { + t.Fatalf("not forgotten: %+v", b) + } + if policyRefused(syscall.Errno(2)) || !policyRefused(&os.PathError{Err: syscall.Errno(1260)}) { + t.Error("policyRefused") + } +} diff --git a/internal/update/install_blocked_windows_test.go b/internal/update/install_blocked_windows_test.go new file mode 100644 index 00000000..dd34fad5 --- /dev/null +++ b/internal/update/install_blocked_windows_test.go @@ -0,0 +1,98 @@ +package update + +import ( + "errors" + "os" + "path/filepath" + "testing" + "time" +) + +// The other install tests put text in for the new version, which Windows +// won't start: they are about moving files, so the try is left out. +func init() { canStart = func(string) error { return nil } } + +// #894: a download Windows won't start isn't put in; the running exe +// stays where it is and the download goes. +func TestInstallBinaryRefusedStaysOut(t *testing.T) { + was := canStart + canStart = func(string) error { return &os.PathError{Op: "fork/exec", Err: errIntegrityPolicy} } + t.Cleanup(func() { canStart = was }) + exe, staged := rollbackFiles(t) + old, err := InstallBinaryAside(staged, exe) + var be *BlockedError + if !errors.As(err, &be) || !be.Policy() || old != "" { + t.Fatalf("InstallBinaryAside = %q, %v", old, err) + } + wantInstallFile(t, exe, "old version") + for _, p := range []string{staged, exe + ".old"} { + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Errorf("%s: %v", filepath.Base(p), err) + } + } +} + +// trialStart starts a real program, under the .new name a download has, +// without running it, and fails on what isn't one. +func TestTrialStart(t *testing.T) { + ping := filepath.Join(os.Getenv("SystemRoot"), "System32", "PING.EXE") + b, err := os.ReadFile(ping) + if err != nil { + t.Skip("no PING.EXE:", err) + } + dir := t.TempDir() + good := filepath.Join(dir, "magpie.exe.new") + os.WriteFile(good, b, 0o755) + if err := trialStart(good); err != nil { + t.Fatalf("a program: %v", err) + } + bad := filepath.Join(dir, "text.exe.new") + os.WriteFile(bad, []byte("not a program"), 0o755) + if err := trialStart(bad); err == nil { + t.Fatal("text started") + } + // ended, not left running: its file can go + if err := os.Remove(good); err != nil { + t.Fatalf("still held: %v", err) + } +} + +// The restart for real: a new version that can't be started, one that +// quits at once and one that runs on without saying it is up are each +// ended, their file let go of, and the running exe put back. +func TestStartCheckedPutsBackForReal(t *testing.T) { + ping := filepath.Join(os.Getenv("SystemRoot"), "System32", "PING.EXE") + b, err := os.ReadFile(ping) + if err != nil { + t.Skip("no PING.EXE:", err) + } + was := startWait + startWait = 2 * time.Second + t.Cleanup(func() { startWait = was }) + for _, c := range []struct { + name string + prog []byte + args []string + }{ + {"not a program", []byte("not a program"), nil}, + {"quits at once", b, []string{"-n", "1", "127.0.0.1"}}, + {"never says it is up", b, []string{"-n", "60", "127.0.0.1"}}, + } { + t.Run(c.name, func(t *testing.T) { + dir := t.TempDir() + exe, old := filepath.Join(dir, "magpie.exe"), filepath.Join(dir, "magpie.exe.old") + os.WriteFile(exe, c.prog, 0o755) + os.WriteFile(old, []byte("running version"), 0o755) + err := startChecked(exe, old, c.args, os.Environ()) + var be *BlockedError + if !errors.As(err, &be) { + t.Fatalf("err = %v", err) + } + t.Log(err) + wantInstallFile(t, exe, "running version") + if _, err := os.Stat(old); !os.IsNotExist(err) { + t.Errorf("old left: %v", err) + } + }) + } +} diff --git a/internal/update/proc_unix.go b/internal/update/proc_unix.go index 4c275e92..3e08ec87 100644 --- a/internal/update/proc_unix.go +++ b/internal/update/proc_unix.go @@ -36,3 +36,6 @@ func Reexec(exe string, args, env []string) error { proc.EndProbes() // the new version doesn't know them, nor waits on them return syscall.Exec(exe, append([]string{exe}, args...), env) } + +// trialStart: nothing here refuses a build that downloaded whole. +func trialStart(string) error { return nil } diff --git a/internal/update/proc_windows.go b/internal/update/proc_windows.go index 5b73ebc6..97592366 100644 --- a/internal/update/proc_windows.go +++ b/internal/update/proc_windows.go @@ -13,6 +13,8 @@ const ( waitTimeout = 0x00000102 detachedProc = 0x00000008 newProcGroup = 0x00000200 + suspended = 0x00000004 + noWindow = 0x08000000 ) // alive reports whether pid is still running. @@ -48,3 +50,21 @@ func Reexec(exe string, args, env []string) error { } return cmd.Process.Release() } + +// trialStart starts path suspended and ends it at once: Windows checks a +// program when it creates its process, so one Smart App Control or +// another application control policy won't run fails here, and none of +// one it will run runs. +func trialStart(path string) error { + cmd := proc.Command(path) + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.CreationFlags |= suspended | noWindow + if err := cmd.Start(); err != nil { + return err + } + cmd.Process.Kill() + cmd.Wait() + return nil +} diff --git a/internal/update/relaunch_args_test.go b/internal/update/relaunch_args_test.go index 16434f53..c18457fd 100644 --- a/internal/update/relaunch_args_test.go +++ b/internal/update/relaunch_args_test.go @@ -54,7 +54,7 @@ func TestRelaunchBinaryArgs(t *testing.T) { if err := os.WriteFile(exe, []byte(script), 0o755); err != nil { t.Fatal(err) } - if err := RelaunchBinary(exe, c.window, c.view); err != nil { + if err := RelaunchBinary(exe, "", c.window, c.view); err != nil { t.Fatal(err) } want := c.want + " " + strconv.Itoa(os.Getpid()) diff --git a/internal/update/update.go b/internal/update/update.go index 9de175c4..cd7e58c3 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -376,8 +376,22 @@ func StageBinary(ctx context.Context, rel *Release) (string, error) { // InstallBinary swaps a staged binary in for exe, the running one, which // keeps going until it exits. func InstallBinary(staged, exe string) error { + _, err := InstallBinaryAside(staged, exe) + return err +} + +// InstallBinaryAside is InstallBinary, saying where the running exe was +// moved to on Windows ("" elsewhere), for RelaunchBinary to put it back +// should the new version not start. A download this computer won't start +// (Smart App Control, #894) isn't put in: a *BlockedError, and the +// download removed. +func InstallBinaryAside(staged, exe string) (string, error) { var old string if runtime.GOOS == "windows" { + if err := canStart(staged); err != nil { + os.Remove(staged) + return "", &BlockedError{Err: err} + } // A running .exe cannot be overwritten, but it can be moved aside. // What the last update moved aside may still be running too (a // `magpie serve` started before it), and can't be removed or @@ -386,20 +400,20 @@ func InstallBinary(staged, exe string) error { RemoveOld(exe) var err error if old, err = moveAside(exe); err != nil { - return err + return "", err } } if err := os.Rename(staged, exe); err != nil { if old != "" { if restoreErr := os.Rename(old, exe); restoreErr != nil { - return errors.Join(err, fmt.Errorf("couldn't restore %s from %s: %w", filepath.Base(exe), old, restoreErr)) + return "", errors.Join(err, fmt.Errorf("couldn't restore %s from %s: %w", filepath.Base(exe), old, restoreErr)) } } else if !NeedsAdmin(err) { // kept for InstallBinaryAsAdmin os.Remove(staged) } - return err + return "", err } - return nil + return old, nil } // renameFile is os.Rename; tests make it fail. @@ -523,16 +537,18 @@ func InstallBinaryAsAdmin(staged, exe string) error { // RelaunchBinary starts exe again: with its window on view when window is // set (the window was open), else as the tray app alone, as autostart // starts it. The new process waits for this one to exit before it takes -// the gateway's port; see AwaitPredecessor. -func RelaunchBinary(exe string, window bool, view string) error { - cmd := proc.Command(exe, RelaunchArgs(window, view)...) - cmd.Env = append(os.Environ(), "MAGPIE_REPLACES="+strconv.Itoa(os.Getpid())) - cmd.Stdin, cmd.Stdout, cmd.Stderr = nil, nil, nil - detach(cmd) - if err := cmd.Start(); err != nil { - return err +// the gateway's port; see AwaitPredecessor. With old, where this one was +// moved aside to (Windows), it waits for the new one to say it is up, and +// when it can't start puts old back and says so (a *BlockedError): this +// one then runs on rather than quitting. +func RelaunchBinary(exe, old string, window bool, view string) error { + args := RelaunchArgs(window, view) + env := append(os.Environ(), "MAGPIE_REPLACES="+strconv.Itoa(os.Getpid())) + if old != "" { + return startChecked(exe, old, args, env) } - return cmd.Process.Release() + _, _, err := launch(exe, args, env) + return err } // RelaunchArgs is what RelaunchBinary starts magpie with: `gui [view]` for @@ -550,6 +566,7 @@ func RelaunchArgs(window bool, view string) []string { // AwaitPredecessor blocks, for a while at most, until the magpie that // relaunched this one has exited. func AwaitPredecessor() { + reportStarted() // first: the one that started this waits for it pid, err := strconv.Atoi(os.Getenv("MAGPIE_REPLACES")) os.Unsetenv("MAGPIE_REPLACES") if err != nil || pid <= 0 {