From 7cb5cf73eb090cd9cf30c0842a26351070bb1ac1 Mon Sep 17 00:00:00 2001 From: migsilva89 <85745515+migsilva89@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:09:42 +0100 Subject: [PATCH 1/4] Let the app install its own updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loadout 0.3.2 could tell you a new version existed and then send you to a download page. That is the whole of what it did: every update ended in somebody dragging an app over another one in Applications, and plenty of people simply never did it. Sparkle replaces that. It finds the release, checks it was signed with the key this copy was built with, verifies it before unpacking, installs it and reopens. Pinned to 2.9.6 rather than a range, because this is the one dependency that installs executable code on somebody's disk. The old checker is deleted rather than left beside it. Two mechanisms mean two schedules, two preferences and two answers to "am I current?" — and the first time they disagree, one of the places you looked was lying. Its tests go with it: they described a GitHub API call and a version comparison that Sparkle now does itself. The preference survives the swap. Somebody who turned checks off in 0.3.2 asked not to be checked, so that answer is moved into Sparkle's own preference before the updater starts, and the dead keys are cleared. Settings › Updates stays, and now reads Sparkle instead of keeping its own copy of the same facts. --- Package.resolved | 15 ++ Package.swift | 21 ++- Sources/LoadoutApp/LoadoutApp.swift | 9 +- Sources/LoadoutApp/SettingsView.swift | 108 ++++++-------- Sources/LoadoutApp/UpdateNotice.swift | 113 --------------- Sources/LoadoutApp/Updates.swift | 83 +++++++++++ Sources/LoadoutCore/UpdateCheck.swift | 136 ------------------ Tests/LoadoutCoreTests/UpdateCheckTests.swift | 117 --------------- 8 files changed, 167 insertions(+), 435 deletions(-) create mode 100644 Package.resolved delete mode 100644 Sources/LoadoutApp/UpdateNotice.swift create mode 100644 Sources/LoadoutApp/Updates.swift delete mode 100644 Sources/LoadoutCore/UpdateCheck.swift delete mode 100644 Tests/LoadoutCoreTests/UpdateCheckTests.swift diff --git a/Package.resolved b/Package.resolved new file mode 100644 index 0000000..43ce0dc --- /dev/null +++ b/Package.resolved @@ -0,0 +1,15 @@ +{ + "originHash" : "f31cfb8fadb6bc974b9c4476ea08ba27eb7ab498ff3eb8c29ab3925c27d747fd", + "pins" : [ + { + "identity" : "sparkle", + "kind" : "remoteSourceControl", + "location" : "https://github.com/sparkle-project/Sparkle", + "state" : { + "revision" : "ac2def288cbff5cfc7df3ffef6abdf45b72bcb0a", + "version" : "2.9.6" + } + } + ], + "version" : 3 +} diff --git a/Package.swift b/Package.swift index 99385f7..05677b2 100644 --- a/Package.swift +++ b/Package.swift @@ -4,9 +4,28 @@ import PackageDescription let package = Package( name: "Loadout", platforms: [.macOS(.v15)], + dependencies: [ + // Pinned because this framework installs executable code: it is what replaces Loadout.app + // on somebody's disk. Moving off this version is a deliberate review, not something a + // release build picks up on its own. + .package(url: "https://github.com/sparkle-project/Sparkle", exact: "2.9.6"), + ], targets: [ .target(name: "LoadoutCore"), - .executableTarget(name: "LoadoutApp", dependencies: ["LoadoutCore"]), + .executableTarget( + name: "LoadoutApp", + dependencies: [ + "LoadoutCore", + .product(name: "Sparkle", package: "Sparkle"), + ], + // The executable sits in Contents/MacOS and the framework is embedded in the standard + // sibling Frameworks directory by Scripts/build-app.sh. Without this rpath the app + // builds and then refuses to launch out of the bundle, because dyld has nowhere to + // look for Sparkle. + linkerSettings: [.unsafeFlags([ + "-Xlinker", "-rpath", "-Xlinker", "@loader_path/../Frameworks", + ])] + ), .testTarget(name: "LoadoutCoreTests", dependencies: ["LoadoutCore"]), ] ) diff --git a/Sources/LoadoutApp/LoadoutApp.swift b/Sources/LoadoutApp/LoadoutApp.swift index 35538eb..33dbd46 100644 --- a/Sources/LoadoutApp/LoadoutApp.swift +++ b/Sources/LoadoutApp/LoadoutApp.swift @@ -69,9 +69,10 @@ struct LoadoutApp: App { Window("Loadout", id: "main") { ContentView(model: model) .frame(minWidth: 824, minHeight: 640) - // After the window is up and the inventory has been read, so a version check can - // never be the reason a launch feels slow. - .task { UpdateNotice.checkOnLaunch() } + // After the window is up and the inventory has been read, so starting the + // updater can never be the reason a launch feels slow. Sparkle owns the schedule + // from here: it decides when the next check is due, not this line. + .task { Updates.start() } // After the inventory has been read, so the welcome can state what was found // rather than open on zeroes. .task { model.showWelcomeIfNeeded() } @@ -87,7 +88,7 @@ struct LoadoutApp: App { // Directly under "About Loadout", where every Mac app puts it and where a hand // looking for it goes first. CommandGroup(after: .appInfo) { - Button("Check for Updates…") { UpdateNotice.checkNow() } + Button("Check for Updates…") { Updates.checkNow() } Divider() Button(model.showsSettings ? "Hide Settings" : "Settings…") { model.showsSettings.toggle() diff --git a/Sources/LoadoutApp/SettingsView.swift b/Sources/LoadoutApp/SettingsView.swift index 2e86be2..5adeb28 100644 --- a/Sources/LoadoutApp/SettingsView.swift +++ b/Sources/LoadoutApp/SettingsView.swift @@ -387,92 +387,72 @@ private struct UsageSourceRow: View { // MARK: - Updates -/// The visible half of the update check: which version is running, whether the daily check is on, -/// and a button that answers now. +/// The visible half of Sparkle: which version is running, whether the daily check is on, when it +/// last got an answer, and a button that asks now. /// -/// The answer lands in the pane rather than in a window, because somebody who pressed a button in -/// Settings is already looking at the place the answer belongs — and a pane cannot block the app -/// the way an alert can. The question itself is the same one the menu item asks: UpdateCheck. +/// Everything here reads and writes Sparkle's own state rather than keeping a copy. Loadout 0.3.2 +/// had a pane that stored its own switch and its own "last checked" date, which is how a Settings +/// screen ends up disagreeing with the app it belongs to. Press "Check now" and Sparkle puts up +/// its standard window — the one that shows the release notes and does the installing — so the +/// answer arrives in the place that can act on it. struct UpdatesTab: View { - @AppStorage(UpdateNotice.automaticKey) private var checksAutomatically = true - @State private var isChecking = false - @State private var answer: Answer? - - /// What the pane can say: UpdateCheck's three outcomes, plus the one this side decides — a - /// build run from source, which has no version to compare in the first place. - private enum Answer { - case outcome(UpdateCheck.Outcome, running: String) - case unreleasedBuild - } + /// Mirrors of Sparkle's preference, because SwiftUI needs something it can observe. `set` on + /// the binding writes through to the updater; nothing else ever writes this. + @State private var checksAutomatically = Updates.automaticallyChecksForUpdates + @State private var lastCheck: Date? = Updates.lastCheck var body: some View { Form { - LabeledContent("Version", value: UpdateCheck.runningVersion() ?? "Unreleased build") + LabeledContent("Version", value: Updates.current ?? "Unreleased build") - Toggle("Check for a new version once a day", isOn: $checksAutomatically) - .help( - "Asks github.com for the latest release number, at most once a day. " - + "No files, no identifiers — and off means Loadout makes no network calls at all." - ) + Toggle("Check for updates automatically", isOn: Binding( + get: { checksAutomatically }, + set: { newValue in + checksAutomatically = newValue + Updates.automaticallyChecksForUpdates = newValue + } + )) + .help( + "Asks the release feed for a new version about once a day, and offers to install " + + "it. No files and no identifiers are sent — and off means Loadout makes no " + + "network call at all." + ) + + LabeledContent("Last checked", value: lastCheckLine) HStack { - if isChecking { - ProgressView().controlSize(.small) - Text("Checking…") - .font(.caption) - .foregroundStyle(.secondary) - } Spacer() Button("Check now") { check() } - .disabled(isChecking) - .help("Ask GitHub right now, whether or not the daily check is on") + .help("Ask right now, whether or not the automatic check is on") .pointingHand() } - if let answer, !isChecking { - VStack(alignment: .leading, spacing: 6) { - Text(message(for: answer)) - .font(.caption) - .foregroundStyle(.secondary) - if case .outcome(.available(let update), _) = answer { - Button("Open release page") { NSWorkspace.shared.open(update.page) } - .pointingHand() - } - } - } + Text( + "An update is downloaded and installed by Loadout itself. It is only accepted if " + + "it is signed with the key this copy was built with, so a tampered download " + + "is refused rather than installed." + ) + .font(.caption) + .foregroundStyle(.secondary) } .formStyle(.grouped) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) } - /// Loadout never installs anything for you, so the new-version line says what to do next - /// rather than pretending a button here could do it — and a check that never reached GitHub - /// says so, instead of passing itself off as good news. - private func message(for answer: Answer) -> String { - switch answer { - case .outcome(.available(let update), _): - "Loadout \(update.version) is out. Download it and drag it over the copy in Applications." - case .outcome(.upToDate, let running): - "Loadout \(running) is the latest version." - case .outcome(.unreachable, _): - "Couldn't check for updates — GitHub could not be reached. Try again in a moment." - case .unreleasedBuild: - "This build has no version number, so there is nothing to compare — version checks only work on a released build." - } + /// Sparkle has no date until the first check completes, and "Never" is a truer answer for a + /// fresh install than a date invented to fill the row. + private var lastCheckLine: String { + guard let lastCheck else { return "Never" } + return lastCheck.formatted(date: .abbreviated, time: .shortened) } + /// Sparkle owns the window that follows, so there is nothing to show in the pane afterwards — + /// only the date to catch up with, once the check has had a moment to land. private func check() { - isChecking = true + Updates.checkNow() Task { - if let running = UpdateCheck.runningVersion() { - let outcome = await UpdateCheck.check(against: running) - // The launch notice must not raise this same version later: it has been shown. - if case .available(let update) = outcome { UpdateNotice.noteShown(update.version) } - answer = .outcome(outcome, running: running) - } else { - answer = .unreleasedBuild - } - isChecking = false + try? await Task.sleep(for: .seconds(2)) + lastCheck = Updates.lastCheck } } } diff --git a/Sources/LoadoutApp/UpdateNotice.swift b/Sources/LoadoutApp/UpdateNotice.swift deleted file mode 100644 index 17001e4..0000000 --- a/Sources/LoadoutApp/UpdateNotice.swift +++ /dev/null @@ -1,113 +0,0 @@ -import AppKit -import Foundation -import LoadoutCore - -/// Tells the owner when a newer Loadout is out — the launch check that speaks only when there is -/// something to say, and the menu item that answers whenever it is asked. -/// -/// Two different manners on purpose. The launch check is uninvited, so it stays silent unless -/// there is a new version, and it mentions any one version only once: being told the same thing -/// every morning is how a person learns to dismiss the box without reading it. The menu item was -/// asked a question, so it always answers — including "you are up to date", which is the answer -/// somebody who clicked it actually wants. -@MainActor -enum UpdateNotice { - /// The last version the launch check mentioned, so it does not mention it again. - private static let announcedKey = "lastAnnouncedUpdate" - /// The switch in Settings › Updates. On by default, and read before the request is built, so - /// off means Loadout makes no network call at all. - static let automaticKey = "checksForUpdates" - /// When the launch check last ran. Opening the app ten times in a morning is still one request. - private static let lastCheckKey = "lastUpdateCheck" - - /// Roughly a day, and short of it on purpose: exactly 24h means somebody who opens Loadout - /// each morning at the same time never gets a second check. - private static let interval: TimeInterval = 20 * 60 * 60 - - static var checksAutomatically: Bool { - get { UserDefaults.standard.object(forKey: automaticKey) as? Bool ?? true } - set { UserDefaults.standard.set(newValue, forKey: automaticKey) } - } - - /// Runs shortly after launch, and only ever opens a window for a version it has not raised - /// before. Never blocks the launch: it is a detached task, and a failed check says nothing. - static func checkOnLaunch() { - guard checksAutomatically else { return } - let last = UserDefaults.standard.object(forKey: lastCheckKey) as? Date ?? .distantPast - guard Date().timeIntervalSince(last) > interval else { return } - guard let running = UpdateCheck.runningVersion() else { return } - Task { - let outcome = await UpdateCheck.check(against: running) - // Only an answer starts the clock. A check that never reached GitHub taught nobody - // anything, so the next launch asks again rather than waiting out the day. - guard outcome != .unreachable else { return } - UserDefaults.standard.set(Date(), forKey: lastCheckKey) - guard case .available(let update) = outcome, - UserDefaults.standard.string(forKey: announcedKey) != update.version - else { return } - UserDefaults.standard.set(update.version, forKey: announcedKey) - present(update) - } - } - - /// The Loadout menu's "Check for Updates…", which answers either way. Somebody asked, so it - /// ignores both the once-a-day throttle and the switch: those govern the uninvited check. - static func checkNow() { - UserDefaults.standard.set(Date(), forKey: lastCheckKey) - Task { - guard let running = UpdateCheck.runningVersion() else { - // A build that was never packaged has no version to compare — say that rather - // than claim it is current. - return say( - "This build has no version", - "Version checks only work on a released build of Loadout, not one run from source.", - link: true - ) - } - switch await UpdateCheck.check(against: running) { - case .available(let update): - UserDefaults.standard.set(update.version, forKey: announcedKey) - present(update) - case .upToDate: - say("Loadout \(running) is the latest version", "You're up to date.", link: false) - case .unreachable: - say( - "Couldn't check for updates", - "GitHub could not be reached. Try again in a moment.", - link: false - ) - } - } - } - - /// Settings › Updates asks the same question but shows the answer in the pane instead of a - /// window, so it records what it found here — otherwise the launch notice would later raise a - /// version somebody has already been shown. - static func noteShown(_ version: String) { - UserDefaults.standard.set(version, forKey: announcedKey) - } - - /// The one that matters: a new version exists, here is what it is, and here is the way to it. - private static func present(_ update: UpdateCheck.Available) { - let alert = NSAlert() - alert.messageText = "Loadout \(update.version) is available" - alert.informativeText = "You're running \(UpdateCheck.runningVersion() ?? "an older version"). " - + "Download the new one and drag it to Applications, replacing this copy." - alert.addButton(withTitle: "Download") - alert.addButton(withTitle: "Later") - if alert.runModal() == .alertFirstButtonReturn { - NSWorkspace.shared.open(update.page) - } - } - - private static func say(_ title: String, _ detail: String, link: Bool) { - let alert = NSAlert() - alert.messageText = title - alert.informativeText = detail - alert.addButton(withTitle: "OK") - if link { alert.addButton(withTitle: "Open Releases") } - if alert.runModal() == .alertSecondButtonReturn { - NSWorkspace.shared.open(UpdateCheck.releasesPage) - } - } -} diff --git a/Sources/LoadoutApp/Updates.swift b/Sources/LoadoutApp/Updates.swift new file mode 100644 index 0000000..7ae5909 --- /dev/null +++ b/Sources/LoadoutApp/Updates.swift @@ -0,0 +1,83 @@ +import AppKit +import Sparkle + +/// The one thing in Loadout that knows about new versions: Sparkle finds the release, checks the +/// signature against the public key baked into the bundle, and replaces the app in place. +/// +/// Loadout 0.3.2 shipped a checker of its own that asked GitHub for a version number and opened +/// the release page — it never installed anything, so every update was still a manual drag into +/// Applications. This replaces it outright rather than sitting beside it. Two update mechanisms +/// mean two schedules, two preferences and two answers to "am I current?", and the moment they +/// disagree the app is lying to somebody in at least one of the places they looked. +/// +/// Sparkle owns the schedule and the single preference behind it, so the menu item, the Settings +/// pane and the daily background check are three doors into one state. +@MainActor +enum Updates { + /// `startingUpdater: false` because the updater must not start while the app is still + /// assembling itself — `start()` below runs it once the preference migration has happened, so + /// somebody who turned checks off in 0.3.2 is not checked on the way past. + private static let controller = SPUStandardUpdaterController( + startingUpdater: false, + updaterDelegate: nil, + userDriverDelegate: nil + ) + private static var started = false + + /// The running app's version, from the same `CFBundleShortVersionString` the build script + /// writes out of the git tag. A build run with `swift run` has no bundle and no version, which + /// is worth saying out loud in the pane rather than showing as a zero. + nonisolated static var current: String? { + guard let raw = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String, + !raw.isEmpty, raw != "dev" + else { return nil } + return raw + } + + static var automaticallyChecksForUpdates: Bool { + get { + start() + return controller.updater.automaticallyChecksForUpdates + } + set { + start() + controller.updater.automaticallyChecksForUpdates = newValue + } + } + + /// When Sparkle last got an answer, for the pane to show. Nil until the first check lands. + static var lastCheck: Date? { + start() + return controller.updater.lastUpdateCheckDate + } + + /// Starts the updater, once, and carries the old preference across first. + /// + /// The 0.3.2 checker stored its switch under `checksForUpdates`. Somebody who turned that off + /// asked not to be checked, and letting Sparkle start on its own default would quietly turn it + /// back on — so the answer is moved into Sparkle's own preference and the dead key removed, + /// which also means the migration cannot run twice. + static func start() { + guard !started else { return } + started = true + let defaults = UserDefaults.standard + if let previous = defaults.object(forKey: "checksForUpdates") as? Bool { + controller.updater.automaticallyChecksForUpdates = previous + defaults.removeObject(forKey: "checksForUpdates") + } + // Dead keys from the 0.3.2 checker's own bookkeeping. Harmless, but leaving them behind + // means the next person to read `defaults read com.migsilva.loadout` finds state that + // nothing writes and nothing reads. + defaults.removeObject(forKey: "lastAnnouncedUpdate") + defaults.removeObject(forKey: "lastUpdateCheck") + controller.startUpdater() + } + + /// The Loadout menu's "Check for Updates…" and the Settings button, which are the same + /// question. Sparkle answers either way — including "you're up to date", which is the answer + /// somebody who pressed the button actually wants. + static func checkNow() { + start() + controller.checkForUpdates(nil) + } +} diff --git a/Sources/LoadoutCore/UpdateCheck.swift b/Sources/LoadoutCore/UpdateCheck.swift deleted file mode 100644 index 9862f4b..0000000 --- a/Sources/LoadoutCore/UpdateCheck.swift +++ /dev/null @@ -1,136 +0,0 @@ -import Foundation - -/// Whether a newer Loadout has been published, and where to get it. -/// -/// The app ships as a DMG people drag to Applications, so nothing tells them a new version exists -/// — the first release went out with no way at all to reach the people who had already downloaded -/// it. This is that way: one call to the GitHub releases API, a version comparison, and a link. -/// -/// Deliberately not an auto-updater. It never downloads and never replaces anything: it says a -/// newer version is out and opens the release page. Replacing a signed app in place is the part -/// that goes wrong silently, and a link cannot. -public enum UpdateCheck { - /// Where the releases live. The API endpoint answers with the newest non-draft, non-prerelease - /// release, which is exactly the one a person should be offered. - public static let latestReleaseAPI = URL(string: "https://api.github.com/repos/migsilva89/loadout/releases/latest")! - public static let releasesPage = URL(string: "https://github.com/migsilva89/loadout/releases/latest")! - - public struct Available: Equatable, Sendable { - /// The published version, without the tag's leading "v" — "0.1.1", not "v0.1.1". - public let version: String - /// The release's own page, for the person to download from. - public let page: URL - - public init(version: String, page: URL) { - self.version = version - self.page = page - } - } - - /// The running app's version, from the same `CFBundleShortVersionString` the build script - /// writes from the git tag. A build run straight from Xcode or `swift run` has no bundle - /// version; that returns nil and the check is skipped rather than comparing against garbage. - public static func runningVersion(bundle: Bundle = .main) -> String? { - guard let raw = bundle.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String, - !raw.isEmpty, raw != "dev" - else { return nil } - return raw - } - - /// What asking GitHub can end in. "Up to date" and "could not ask" are different facts and are - /// kept apart here, because telling somebody they are on the latest version when the question - /// never left the machine is the one lie this whole file exists to avoid. - public enum Outcome: Equatable, Sendable { - case available(Available) - case upToDate - /// Offline, rate-limited, non-200, a payload that changed shape — every way of not knowing. - case unreachable - } - - /// Asks GitHub what the newest release is and says which of the three it is. Nothing throws its - /// way out to the person using the app: a failure is a state to report, not an error to raise. - /// - /// - Parameter current: the running version, "0.3.1" or "v0.3.1". - public static func check( - against current: String, - session: URLSession = .shared - ) async -> Outcome { - var request = URLRequest(url: latestReleaseAPI) - // GitHub asks for these two by name and answers unversioned requests less predictably. - request.setValue("application/vnd.github+json", forHTTPHeaderField: "Accept") - request.setValue("Loadout/\(current)", forHTTPHeaderField: "User-Agent") - request.timeoutInterval = 10 - - guard let (data, response) = try? await session.data(for: request), - let http = response as? HTTPURLResponse, http.statusCode == 200, - let release = try? JSONDecoder().decode(Release.self, from: data) - else { return .unreachable } - - return available(from: release, current: current).map(Outcome.available) ?? .upToDate - } - - /// The older shape of the same question, for callers that only act when there is something to - /// download. A failed check and an up-to-date app both answer nil here, which is why anything - /// that has to tell a person what happened should ask `check(against:)` instead. - /// - /// - Parameter current: defaults to the running bundle's version. - public static func newerRelease( - than current: String? = runningVersion(), - session: URLSession = .shared - ) async -> Available? { - guard let current else { return nil } - guard case .available(let update) = await check(against: current, session: session) else { - return nil - } - return update - } - - /// The decision itself, split out from the network so it can be tested without one. - static func available(from release: Release, current: String) -> Available? { - let latest = normalise(release.tagName) - guard isNewer(latest, than: normalise(current)) else { return nil } - let page = URL(string: release.htmlURL) ?? releasesPage - return Available(version: latest, page: page) - } - - /// "v0.1.1" and "0.1.1" are the same version written two ways — the tag carries the "v" and - /// the bundle does not. - static func normalise(_ version: String) -> String { - var trimmed = version.trimmingCharacters(in: .whitespacesAndNewlines) - if trimmed.first == "v" || trimmed.first == "V" { trimmed.removeFirst() } - return trimmed - } - - /// Compares dotted numeric versions field by field, shorter one padded with zeros, so 0.2 beats - /// 0.1.9 and 0.1.10 beats 0.1.9 — which a string comparison gets backwards. - /// - /// Anything after the numbers (a "-beta" suffix) is ignored rather than ranked: the API only - /// hands back full releases, so a prerelease should never arrive here, and guessing an ordering - /// for one would be inventing a rule nothing follows. - static func isNewer(_ candidate: String, than current: String) -> Bool { - let left = fields(candidate), right = fields(current) - for index in 0.. b } - } - return false - } - - private static func fields(_ version: String) -> [Int] { - version.split(separator: ".").map { field in - Int(field.prefix { $0.isNumber }) ?? 0 - } - } - - /// Only the two fields this needs, so an unrelated change to GitHub's payload cannot break it. - struct Release: Decodable { - let tagName: String - let htmlURL: String - - enum CodingKeys: String, CodingKey { - case tagName = "tag_name" - case htmlURL = "html_url" - } - } -} diff --git a/Tests/LoadoutCoreTests/UpdateCheckTests.swift b/Tests/LoadoutCoreTests/UpdateCheckTests.swift deleted file mode 100644 index e8abb0c..0000000 --- a/Tests/LoadoutCoreTests/UpdateCheckTests.swift +++ /dev/null @@ -1,117 +0,0 @@ -import XCTest -@testable import LoadoutCore - -/// The version comparison behind "a newer Loadout is available". Tested without a network: the -/// decision is the part that can be wrong quietly, and a wrong one either nags people who are -/// already current or leaves them on a broken build forever. -final class UpdateCheckTests: XCTestCase { - private func release(_ tag: String, page: String = "https://github.com/migsilva89/loadout/releases/tag/x") -> UpdateCheck.Release { - UpdateCheck.Release(tagName: tag, htmlURL: page) - } - - func testATagIsNewerThanTheRunningVersion() { - let found = UpdateCheck.available(from: release("v0.1.1"), current: "0.1.0") - XCTAssertEqual(found?.version, "0.1.1") - } - - func testTheSameVersionIsNotAnUpdate() { - XCTAssertNil(UpdateCheck.available(from: release("v0.1.0"), current: "0.1.0")) - } - - func testAnOlderPublishedVersionIsNotAnUpdate() { - XCTAssertNil(UpdateCheck.available(from: release("v0.0.9"), current: "0.1.0")) - } - - /// The one a string comparison gets backwards: "0.1.10" sorts before "0.1.9" as text. - func testTenIsNewerThanNine() { - XCTAssertTrue(UpdateCheck.isNewer("0.1.10", than: "0.1.9")) - XCTAssertFalse(UpdateCheck.isNewer("0.1.9", than: "0.1.10")) - } - - /// A shorter version is padded rather than treated as smaller: 0.2 is 0.2.0, which beats 0.1.9. - func testAShorterVersionIsPaddedWithZeros() { - XCTAssertTrue(UpdateCheck.isNewer("0.2", than: "0.1.9")) - XCTAssertFalse(UpdateCheck.isNewer("0.2", than: "0.2.0")) - XCTAssertTrue(UpdateCheck.isNewer("1.0", than: "0.9.9")) - } - - /// The tag carries a "v" and the bundle does not; they are the same version written two ways. - func testTheTagsLeadingVIsIgnored() { - XCTAssertEqual(UpdateCheck.normalise("v0.1.1"), "0.1.1") - XCTAssertEqual(UpdateCheck.normalise(" 0.1.1 "), "0.1.1") - XCTAssertNil(UpdateCheck.available(from: release("v0.1.0"), current: "v0.1.0")) - } - - func testTheReleasesOwnPageIsWhereItSends() { - let page = "https://github.com/migsilva89/loadout/releases/tag/v0.2.0" - XCTAssertEqual(UpdateCheck.available(from: release("v0.2.0", page: page), current: "0.1.0")?.page.absoluteString, page) - } - - // MARK: - The three outcomes, over a stubbed network - - /// "Up to date" and "couldn't ask" used to be the same nil. They are different facts — one is - /// GitHub answering, the other is never having reached it — and a pane that says "you're on - /// the latest version" when the request failed is lying to somebody who asked a plain question. - func testGitHubAnsweringWithANewerTagIsAnUpdate() async { - let outcome = await check(status: 200, body: #"{"tag_name":"v0.2.0","html_url":"https://example.com/r"}"#) - XCTAssertEqual(outcome, .available(UpdateCheck.Available(version: "0.2.0", page: URL(string: "https://example.com/r")!))) - } - - func testGitHubAnsweringWithTheSameTagIsUpToDate() async { - let outcome = await check(status: 200, body: #"{"tag_name":"v0.1.0","html_url":"https://example.com/r"}"#) - XCTAssertEqual(outcome, .upToDate) - } - - /// Offline, and the case that matters most: silence is not agreement. - func testANetworkErrorIsUnreachableRatherThanUpToDate() async { - let outcome = await check(error: URLError(.notConnectedToInternet)) - XCTAssertEqual(outcome, .unreachable) - } - - /// Rate limiting is GitHub's usual way of saying no, and it arrives as a perfectly valid body. - func testANon200IsUnreachable() async { - let outcome = await check(status: 403, body: #"{"message":"API rate limit exceeded"}"#) - XCTAssertEqual(outcome, .unreachable) - } - - func testAPayloadThatIsNotTheExpectedShapeIsUnreachable() async { - let notJSON = await check(status: 200, body: "502 Bad Gateway") - XCTAssertEqual(notJSON, .unreachable) - let missingTheTag = await check(status: 200, body: #"{"name":"0.2.0"}"#) - XCTAssertEqual(missingTheTag, .unreachable) - } - - /// Runs the real `check(against:)` against a stubbed URLSession, so every branch above is the - /// shipping code path and not a rehearsal of it. - private func check(status: Int = 200, body: String = "", error: URLError? = nil) async -> UpdateCheck.Outcome { - StubProtocol.answer = (status, Data(body.utf8), error) - let configuration = URLSessionConfiguration.ephemeral - configuration.protocolClasses = [StubProtocol.self] - return await UpdateCheck.check(against: "0.1.0", session: URLSession(configuration: configuration)) - } -} - -/// Answers whatever the test last set, so no test here touches the network. -private final class StubProtocol: URLProtocol { - /// One test runs at a time and each sets this before asking; the unchecked annotation is that - /// fact written down, not a claim that this would be safe under concurrency. - nonisolated(unsafe) static var answer: (status: Int, body: Data, error: URLError?) = (200, Data(), nil) - - override class func canInit(with request: URLRequest) -> Bool { true } - override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } - override func stopLoading() {} - - override func startLoading() { - let answer = Self.answer - if let error = answer.error { - client?.urlProtocol(self, didFailWithError: error) - return - } - let response = HTTPURLResponse( - url: request.url!, statusCode: answer.status, httpVersion: nil, headerFields: nil - )! - client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) - client?.urlProtocol(self, didLoad: answer.body) - client?.urlProtocolDidFinishLoading(self) - } -} From 7fe5af90e1d1bbebfcd8c8e9079eb5194d74636e Mon Sep 17 00:00:00 2001 From: migsilva89 <85745515+migsilva89@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:09:52 +0100 Subject: [PATCH 2/4] Ship Sparkle inside the app, signed the way it has to be MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The framework is embedded in Contents/Frameworks with ditto, because a framework is a tree of symlinks and cp -R flattens it into something that looks right and will not load. Sparkle's XPC services are dropped: they only exist to carry the installer across a sandbox boundary, and Loadout is not sandboxed, so keeping them ships two executables that can never run. Signing is inside out and never with --deep. --deep is deprecated and hands the outer bundle's options to nested code, which is exactly wrong here: Autoupdate and Updater.app are separate programs that each need their own signature and the hardened runtime. Signing the app before the framework would be pointless anyway — changing anything inside a bundle breaks the signature wrapped around it. None of this fails loudly. A wrongly signed Sparkle builds, notarises and installs happily, and only shows up months later when somebody accepts an update and nothing happens. Hence Scripts/test-update.sh, which asks the assembled bundle the questions swift test cannot: is the framework there, is it signed, can the app find it, is the feed address right, is the public key in place. CI runs it after the build. --- .github/workflows/tests.yml | 5 ++++ Scripts/build-app.sh | 56 +++++++++++++++++++++++++++++++++---- Scripts/test-update.sh | 53 +++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) create mode 100755 Scripts/test-update.sh diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a4bc3c9..c82db00 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,3 +14,8 @@ jobs: run: swift test - name: Build the app bundle run: ./Scripts/build-app.sh + # Whether Sparkle is embedded, signed and pointed at the right feed cannot be answered by + # swift test, because none of it exists until the app is assembled — and getting it wrong + # fails silently, months later, on the first update somebody accepts. + - name: Check the app can update itself + run: ./Scripts/test-update.sh diff --git a/Scripts/build-app.sh b/Scripts/build-app.sh index 4b89c2e..5069a38 100755 --- a/Scripts/build-app.sh +++ b/Scripts/build-app.sh @@ -31,9 +31,21 @@ BINARY="$(swift build -c "$CONFIG" --product LoadoutApp --show-bin-path)/Loadout echo "→ Assembling the bundle" rm -rf "$APP" -mkdir -p "$APP/Contents/MacOS" "$APP/Contents/Resources" +mkdir -p "$APP/Contents/MacOS" "$APP/Contents/Resources" "$APP/Contents/Frameworks" cp "$BINARY" "$APP/Contents/MacOS/Loadout" +# Sparkle is what installs the next version, so it travels inside the app. `ditto` rather than +# `cp -R` because a framework is a bundle of symlinks and cp flattens them, which produces a +# framework that looks right and will not load. +SPARKLE="$APP/Contents/Frameworks/Sparkle.framework" +BUILT_SPARKLE="$(dirname "$BINARY")/Sparkle.framework" +[[ -d "$BUILT_SPARKLE" ]] || { echo "no Sparkle.framework beside $BINARY — run 'swift package resolve'"; exit 1; } +rm -rf "$SPARKLE" +ditto "$BUILT_SPARKLE" "$SPARKLE" +# Loadout is not sandboxed. Sparkle's XPC services exist only to carry the installer across a +# sandbox boundary, so keeping them ships two executables and two signatures that can never run. +rm -rf "$SPARKLE/Versions/B/XPCServices" "$SPARKLE/XPCServices" + if [[ -f "$ROOT/Resources/Loadout.icns" ]]; then cp "$ROOT/Resources/Loadout.icns" "$APP/Contents/Resources/Loadout.icns" else @@ -57,23 +69,55 @@ cat > "$APP/Contents/Info.plist" <LSApplicationCategoryTypepublic.app-category.developer-tools NSHighResolutionCapable NSHumanReadableCopyrightMiguel Silva + + + SUFeedURLhttps://github.com/migsilva89/loadout/releases/latest/download/appcast.xml + SUPublicEDKeylUaE3YVkBVqKzXHSQ5Kuex3WtTnffdZtNfHTFbA85ts= + SURequireSignedFeed + SUVerifyUpdateBeforeExtraction + SUEnableAutomaticChecks + SUAllowsAutomaticUpdates + SUScheduledCheckInterval86400 + SUSendProfileInfo PLIST plutil -lint "$APP/Contents/Info.plist" > /dev/null +# Inside out, and never with --deep. --deep is deprecated and applies the outer bundle's options +# to nested code, which is exactly wrong here: Autoupdate and Updater.app are separate programs +# that must each carry their own signature and the hardened runtime. Signing the app first and the +# framework after would also be pointless — changing anything inside a bundle invalidates the +# signature wrapped around it. +# +# This is the part that fails quietly. A wrongly signed Sparkle still notarises and still ships; +# it only breaks when somebody accepts an update, and then the installer cannot launch and the app +# just never updates. Scripts/test-update.sh checks the assembled bundle for that. if [[ -n "$SIGN_IDENTITY" ]]; then echo "→ Signing with Developer ID" - # No --deep: it is deprecated and signs nested code with the wrong options. There is nothing - # nested here anyway — one binary in one bundle. - codesign --force --sign "$SIGN_IDENTITY" --options runtime --timestamp "$APP" - codesign --verify --strict --verbose=1 "$APP" 2>&1 | tail -1 + SIGN=("$SIGN_IDENTITY" --options runtime --timestamp) else echo "→ Signing (ad hoc — only runs on this machine)" - codesign --force --sign - "$APP" 2>/dev/null + # No hardened runtime and no timestamp: an ad hoc signature cannot carry either, and asking for + # them makes codesign refuse rather than warn. + SIGN=(- --timestamp=none) fi +codesign --force --sign "${SIGN[@]}" "$SPARKLE/Versions/B/Autoupdate" 2>/dev/null +codesign --force --sign "${SIGN[@]}" "$SPARKLE/Versions/B/Updater.app" 2>/dev/null +codesign --force --sign "${SIGN[@]}" "$SPARKLE" 2>/dev/null +codesign --force --sign "${SIGN[@]}" "$APP" 2>/dev/null +codesign --verify --deep --strict --verbose=1 "$APP" 2>&1 | tail -1 + # Make sure Finder and the Dock pick the new icon up rather than a cached one. touch "$APP" diff --git a/Scripts/test-update.sh b/Scripts/test-update.sh new file mode 100755 index 0000000..a913dcd --- /dev/null +++ b/Scripts/test-update.sh @@ -0,0 +1,53 @@ +#!/bin/bash +# The assembled app really can update itself: a loadable, correctly signed Sparkle inside the +# bundle, and the immutable half of the key that verifies every release. +# +# This suite exists because none of it fails loudly. A Sparkle signed with the wrong options, or a +# framework the app cannot find, still builds, still notarises and still installs — and then the +# first update somebody accepts goes nowhere, with no error anybody sees. +# +# ./Scripts/test-update.sh checks dist/Loadout.app +# LOADOUT_APP=/path/to/Loadout.app ./Scripts/test-update.sh + +set -uo pipefail +cd "$(dirname "$0")/.." + +APP="${LOADOUT_APP:-dist/Loadout.app}" +INFO="$APP/Contents/Info.plist" +FRAMEWORK="$APP/Contents/Frameworks/Sparkle.framework" +failures=0 + +check() { + local name="$1" + shift + if "$@" >/dev/null 2>&1; then echo "OK $name"; else failures=$((failures + 1)); echo "FAIL $name"; fi +} + +check "there is an app to check" test -d "$APP" +check "Sparkle is embedded" test -f "$FRAMEWORK/Versions/B/Sparkle" +check "the installer is embedded" test -f "$FRAMEWORK/Versions/B/Autoupdate" +check "the update window is embedded" test -d "$FRAMEWORK/Versions/B/Updater.app" +check "unused sandbox services are absent" test ! -e "$FRAMEWORK/XPCServices" +check "the framework and its helpers are signed" codesign --verify --deep --strict "$FRAMEWORK" +check "the app and everything in it are signed" codesign --verify --deep --strict "$APP" +check "the app can find the embedded framework" sh -c \ + "otool -l '$APP/Contents/MacOS/Loadout' | grep -q '@loader_path/../Frameworks'" +check "the feed has one stable address" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SUFeedURL' '$INFO')\" = \ + 'https://github.com/migsilva89/loadout/releases/latest/download/appcast.xml'" +check "updates require the public signing key" sh -c \ + "test -n \"\$(/usr/libexec/PlistBuddy -c 'Print :SUPublicEDKey' '$INFO')\"" +check "the download is verified before it is unpacked" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SUVerifyUpdateBeforeExtraction' '$INFO')\" = true" +check "the feed itself must also be signed" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SURequireSignedFeed' '$INFO')\" = true" +check "the automatic check is on" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SUEnableAutomaticChecks' '$INFO')\" = true" +check "it asks before replacing the app" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SUAllowsAutomaticUpdates' '$INFO')\" = false" +check "no system profile is sent" sh -c \ + "test \"\$(/usr/libexec/PlistBuddy -c 'Print :SUSendProfileInfo' '$INFO')\" = false" + +echo +if [ "$failures" -eq 0 ]; then echo "all good"; else echo "$failures failing"; fi +[ "$failures" -eq 0 ] From 39f9417512ea542efd57d263677b0410c9c350ea Mon Sep 17 00:00:00 2001 From: migsilva89 <85745515+migsilva89@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:10:04 +0100 Subject: [PATCH 3/4] Publish a signed feed, or refuse to publish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An update feed is only worth anything signed: the app is built with SURequireSignedFeed, so an unsigned one is treated as no feed at all and every installed copy quietly stops finding updates. Scripts/appcast.sh signs the image and the feed with the private key in this machine's login keychain — never in this repository — and refuses to run if that key and the public half baked into the app have drifted apart, which would make every installed copy reject the release as forged. The feed points at a second, byte-identical copy of the image published as Loadout--update.dmg. GitHub counts downloads per asset and nothing else, so this is the only way to tell an update from a first install. Both have to go up or the updater follows the feed to a 404. release.sh writes the feed after stapling rather than before — stapling changes the bytes the signature covers — clears the previous release's feed and update copy before building, and only prints the publish command when a signed feed is actually sitting in dist/. Otherwise it says so, because a release published without one breaks the updater for everybody who already has the app. --- Scripts/appcast.sh | 83 ++++++++++++++++++++++++++++++++++++++++++++++ Scripts/release.sh | 53 ++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100755 Scripts/appcast.sh diff --git a/Scripts/appcast.sh b/Scripts/appcast.sh new file mode 100755 index 0000000..83563cd --- /dev/null +++ b/Scripts/appcast.sh @@ -0,0 +1,83 @@ +#!/bin/bash +# Signs one finished disk image and writes the Sparkle feed that is published beside it. +# +# The private EdDSA key stays in this machine's login keychain under the `loadout` account and is +# never written to a file here; only its public half is in the app's Info.plist, put there by +# Scripts/build-app.sh. Nothing in this script prints or exports the private key. +# +# The feed points at a second, identically-built copy of the image, published under the `-update` +# name. Same bytes, same signature, different GitHub asset — which is the only way to tell an +# update apart from a first install, because GitHub counts downloads per asset and nothing else. +# Both must be published or installed copies follow the feed to a 404. +# +# ./Scripts/appcast.sh signs dist/Loadout-.dmg +# ./Scripts/appcast.sh path/to/Some.dmg signs that one instead + +set -euo pipefail +cd "$(dirname "$0")/.." +ROOT="$PWD" + +# The same question build-app.sh asks, so the feed can never describe a version the app does not +# claim: only version tags count. +VERSION="$(git -C "$ROOT" describe --tags --match 'v[0-9]*' --always 2>/dev/null || echo "0.0.0")" +VERSION="${VERSION#v}" + +DMG="${1:-$ROOT/dist/Loadout-$VERSION.dmg}" +APP="$ROOT/dist/Loadout.app" +TOOLS="$ROOT/.build/artifacts/sparkle/Sparkle/bin" +GENERATE="$TOOLS/generate_appcast" +KEYS="$TOOLS/generate_keys" +OUTPUT="$ROOT/dist/appcast.xml" +UPDATE_DMG="$ROOT/dist/Loadout-$VERSION-update.dmg" + +[ -f "$DMG" ] || { echo "error: no disk image at $DMG" >&2; exit 1; } +[ -x "$GENERATE" ] || { + echo "error: Sparkle's release tools are missing — run swift package resolve" >&2 + exit 1 +} + +# The public key in the built app and the private key in the keychain are two halves of one thing. +# If they have drifted apart — a rebuilt key, a different machine — every installed copy would +# reject this update as forged, and it would look like the updater is broken rather than the key. +[ -f "$APP/Contents/Info.plist" ] || { + echo "error: no built app at $APP — run Scripts/build-app.sh first" >&2 + exit 1 +} +EXPECTED="$(/usr/libexec/PlistBuddy -c 'Print :SUPublicEDKey' "$APP/Contents/Info.plist")" +ACTUAL="$("$KEYS" --account loadout -p)" +[ "$ACTUAL" = "$EXPECTED" ] || { + echo "error: the Sparkle key in the keychain does not match the app" >&2 + exit 1 +} + +# generate_appcast reads a directory and describes everything in it, so it gets a directory holding +# exactly one image — the update copy, under the name the feed should point at. +STAGE="$(mktemp -d)" +trap 'rm -rf "$STAGE"' EXIT +ditto "$DMG" "$STAGE/$(basename "$UPDATE_DMG")" + +"$GENERATE" \ + --account loadout \ + --download-url-prefix "https://github.com/migsilva89/loadout/releases/download/v$VERSION/" \ + --full-release-notes-url "https://github.com/migsilva89/loadout/blob/main/CHANGELOG.md" \ + --link "https://loadout.migsilva.dev" \ + --maximum-versions 1 \ + --maximum-deltas 0 \ + -o "$STAGE/appcast.xml" \ + "$STAGE" + +cp "$STAGE/appcast.xml" "$OUTPUT" +cp "$DMG" "$UPDATE_DMG" + +# An unsigned feed is worse than no feed: SURequireSignedFeed means installed copies would silently +# reject it, so the app would look like it had simply stopped finding updates. +xmllint --noout "$OUTPUT" +grep -q 'sparkle:edSignature=' "$OUTPUT" \ + || { echo "error: the update in appcast.xml is not signed" >&2; exit 1; } +grep -q '