From a014c104122ee15abbcdea2cfa101cfb11e5e7d2 Mon Sep 17 00:00:00 2001 From: Bob Pankratz Date: Sat, 11 Jul 2026 19:08:20 -0500 Subject: [PATCH] fix(upgrade): authenticate release checksums --- .../MootInstallerCore/ReleaseDownloader.swift | 85 +++++++++++++++++-- .../mootx01/Commands/UpgradeCommand.swift | 52 ++++++++++-- .../ReleaseDownloaderTests.swift | 77 +++++++++++++++++ 3 files changed, 201 insertions(+), 13 deletions(-) diff --git a/apps/mootx01/Sources/MootInstallerCore/ReleaseDownloader.swift b/apps/mootx01/Sources/MootInstallerCore/ReleaseDownloader.swift index 602ccf1d..7a09ccd3 100644 --- a/apps/mootx01/Sources/MootInstallerCore/ReleaseDownloader.swift +++ b/apps/mootx01/Sources/MootInstallerCore/ReleaseDownloader.swift @@ -2,8 +2,9 @@ // // Online upgrade path for `mootx01 upgrade`. Mirrors scripts/install.sh: // fetch the latest GitHub release tag, download the platform asset, -// verify SHA-256 via CryptoKit, extract the binary, and delegate -// placement to Installer.placeBinary. +// verify SHA-256 via CryptoKit, authenticate checksums.txt with minisign on +// Linux/POSIX (macOS relies on Developer ID + Gatekeeper), extract the binary, +// and delegate placement to Installer.placeBinary. // // Repo slug is "codedaptive/mootx01-ee", matching install.sh:15. // Asset naming follows install.sh:136: mootx01-{tag}-{os}-{arch}.tar.gz @@ -13,6 +14,11 @@ import CryptoKit import Foundation +private let minisignPublicKey = """ +untrusted comment: minisign public key BC4D1E6ABCB5B788 +RWSIt7W8ah5NvMXMLQ3+T2flXrQ+J6xoDxDrL62I+8iEkR04YIAlXa12 +""" + // MARK: - ReleaseDownloader /// Downloads, verifies, and places a new mootx01 binary from a GitHub release. @@ -81,13 +87,16 @@ public struct ReleaseDownloader: Sendable { return tagName } - /// Downloads the platform asset for `tag`, verifies SHA-256, extracts the - /// binary, and returns a URL pointing to the extracted `mootx01` binary. + /// Downloads the platform asset for `tag`, verifies SHA-256, verifies the + /// detached minisign signature for `checksums.txt` on non-macOS platforms, + /// extracts the binary, and returns a URL pointing to the extracted + /// `mootx01` binary. /// /// The asset name mirrors install.sh:136: `mootx01-{tag}-{os}-{arch}.tar.gz`. - /// The checksum file `checksums.txt` is downloaded from the same release base - /// and verified against the tarball using CryptoKit.SHA256. Extraction uses - /// `/usr/bin/tar -xzf`. + /// The checksum file `checksums.txt` is downloaded from the same release base, + /// authenticated against the embedded Ed25519 minisign public key on + /// Linux/POSIX, and verified against the tarball using CryptoKit.SHA256. + /// Extraction uses `/usr/bin/tar -xzf`. /// /// - Parameter tag: raw GitHub tag string (e.g. "v1.0.0") from `latestTag()`. /// - Returns: URL of the extracted binary inside a temporary directory. @@ -106,6 +115,7 @@ public struct ReleaseDownloader: Sendable { let tarballURL = tmpDir.appendingPathComponent(tarball) let checksumsURL = tmpDir.appendingPathComponent("checksums.txt") + let minisigURL = tmpDir.appendingPathComponent("checksums.txt.minisig") // Download asset and checksums file. let assetURL = URL(string: "\(base)/\(tarball)")! @@ -120,6 +130,17 @@ public struct ReleaseDownloader: Sendable { // the install path (mirrors verify_checksum in scripts/install.sh). try verifySHA256(tarballURL: tarballURL, checksumsURL: checksumsURL, tarball: tarball) + #if !os(macOS) + // Authenticate checksums.txt against the bundled Ed25519 trust root before + // extracting or installing anything. Without this, an attacker who can + // tamper with release assets can ship a malicious tarball plus a matching + // unauthenticated checksums.txt and satisfy the SHA-256 check. + let sigURL = URL(string: "\(base)/checksums.txt.minisig")! + let (sigData, _) = try await fetchData(sigURL) + try sigData.write(to: minisigURL) + try verifyMinisignSignature(checksumsURL: checksumsURL, signatureURL: minisigURL) + #endif + // Validate all archive members before extraction to prevent zip-slip: // an archive member with an absolute path or .. component could escape // tmpDir and overwrite arbitrary files (planned hardening — fails CLOSED). @@ -182,6 +203,54 @@ public struct ReleaseDownloader: Sendable { } } + /// Verify the detached minisign signature over checksums.txt using the + /// repository's embedded public key. Mirrors install.sh and the Rust vertical: + /// fail closed if the key is a placeholder, minisign is unavailable, or the + /// signature does not validate. + private func verifyMinisignSignature(checksumsURL: URL, signatureURL: URL) throws { + if minisignPublicKey.contains("PLACEHOLDER") { + throw UpgradeError.signatureVerificationFailed( + "minisign public key is a PLACEHOLDER — signature verification not yet active" + ) + } + + let keyURL = FileManager.default.temporaryDirectory + .appendingPathComponent("mootx01-minisign-pub-\(UUID().uuidString).pub") + try minisignPublicKey.write(to: keyURL, atomically: true, encoding: .utf8) + defer { try? FileManager.default.removeItem(at: keyURL) } + + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/env") + process.arguments = [ + "minisign", "-V", + "-p", keyURL.path, + "-m", checksumsURL.path, + "-x", signatureURL.path, + ] + let errPipe = Pipe() + process.standardError = errPipe + + do { + try process.run() + } catch { + throw UpgradeError.signatureVerificationFailed( + "minisign is required for release signature verification but was not found. " + + "Install minisign and retry; do not bypass this check." + ) + } + let stderr = errPipe.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + + guard process.terminationStatus == 0 else { + let detail = String(decoding: stderr, as: UTF8.self) + .trimmingCharacters(in: .whitespacesAndNewlines) + throw UpgradeError.signatureVerificationFailed( + "minisign signature verification FAILED for checksums.txt" + + (detail.isEmpty ? "" : ": \(detail)") + ) + } + } + /// Check whether a single archive member path (one line from `tar -tf`) is safe /// to extract into a destination directory. Returns a non-nil reason string when /// the member is unsafe — an absolute/rooted path or any `..` component — and @@ -347,6 +416,8 @@ public enum UpgradeError: Error, Sendable { case invalidAPIResponse(String) /// SHA-256 of the downloaded asset does not match checksums.txt. case checksumMismatch(String) + /// Detached minisign verification of checksums.txt failed or could not run. + case signatureVerificationFailed(String) /// tar extraction exited non-zero or the expected binary was absent. case extractionFailed(String) /// Binary write failed due to insufficient permissions. diff --git a/apps/mootx01/Sources/mootx01/Commands/UpgradeCommand.swift b/apps/mootx01/Sources/mootx01/Commands/UpgradeCommand.swift index a0d57f8b..5a0ddd69 100644 --- a/apps/mootx01/Sources/mootx01/Commands/UpgradeCommand.swift +++ b/apps/mootx01/Sources/mootx01/Commands/UpgradeCommand.swift @@ -5,9 +5,9 @@ // (rust/src/commands/upgrade.rs): // // Remote (default): fetch the latest GitHub release via ReleaseDownloader -// (SHA-256 + tarball-member validation), confirm unless --yes, place, and -// run the convergence steps (plugin rematerialization, permission-tier -// migration, service restart). +// (SHA-256 + minisign on Linux/POSIX + tarball-member validation), confirm +// unless --yes, place, and run the convergence steps (plugin rematerialization, +// permission-tier migration, service restart). // // Local (--from ): the developer workflow — copies a freshly built // binary from an explicit path (e.g. --from .build/release/mootx01). @@ -25,7 +25,8 @@ struct UpgradeCommand: AsyncParsableCommand { abstract: "Upgrade mootx01 to the latest release (or from a local build).", discussion: """ Without flags, upgrade downloads the latest release (SHA-256 - verified), installs it, converges plugin packages and tool + verified, with checksums.txt authenticated by minisign on + Linux/POSIX), installs it, converges plugin packages and tool permissions, and restarts the background services. Use --from to install a local build instead of downloading: @@ -84,12 +85,15 @@ struct UpgradeCommand: AsyncParsableCommand { // Source resolution, mirroring the Rust vertical: --from is the // local developer path; the default is the verified remote download - // (MOOT-INSTALL-E fix 3a — ReleaseDownloader's SHA-256 + tarball - // member validation, the machinery ReleaseDownloaderTests covers). + // (MOOT-INSTALL-E fix 3a — ReleaseDownloader's SHA-256 + independent + // checksums.txt authentication + tarball member validation, the + // machinery ReleaseDownloaderTests covers). let sourcePath: String var downloadTmpDir: URL? + let isRemoteDownload: Bool if from != nil { sourcePath = try resolveSource(cwd: cwd) + isRemoteDownload = false } else { let tag: String? do { @@ -122,6 +126,7 @@ struct UpgradeCommand: AsyncParsableCommand { // the remote path unchanged. downloadTmpDir = binaryURL.deletingLastPathComponent() sourcePath = binaryURL.path + isRemoteDownload = true } defer { if let downloadTmpDir { @@ -150,6 +155,11 @@ struct UpgradeCommand: AsyncParsableCommand { print("Updated: \(mgrPath)") } } + #if os(macOS) + if isRemoteDownload { + applyGatekeeperQuarantine(paths: [binaryPath, MootPaths.installedMgrBinaryURL(homeDirectory: home).path]) + } + #endif // ADR-024 Wave 3, Defect 1: an upgrade alone never touches // ~/.claude/mootx01-plugin or Claude Code's plugin cache — without @@ -204,6 +214,36 @@ struct UpgradeCommand: AsyncParsableCommand { } } + #if os(macOS) + /// The Swift remote upgrade path downloads and extracts with URLSession/tar, + /// which does not mark files as internet downloads. Restore the shell + /// installer's trust split by setting com.apple.quarantine on remotely + /// installed binaries so Gatekeeper assesses Developer ID/notarization on + /// first launch. This is best-effort, matching install.sh's non-fatal xattr + /// behavior. + private func applyGatekeeperQuarantine(paths: [String]) { + let qts = String(Int(Date().timeIntervalSince1970), radix: 16) + let qval = "0083;\(qts);mootx01-upgrade;" + for path in paths where FileManager.default.fileExists(atPath: path) { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/xattr") + process.arguments = ["-w", "com.apple.quarantine", qval, path] + do { + try process.run() + process.waitUntilExit() + if process.terminationStatus == 0 { + print("Quarantine xattr set on \(path) (Gatekeeper will assess on first run)") + } else { + print("Note: could not set quarantine xattr on \(path) — Gatekeeper assessment skipped (non-fatal)") + } + } catch { + print("Note: xattr not found — skipping Gatekeeper quarantine tagging (non-fatal)") + return + } + } + } + #endif + /// See the call site's doc comment. Only touches /// `~/.claude/settings.json` when it already carries at least one of /// our permission entries (`PermissionsWriter.hasAnyMootEntries`) — an diff --git a/apps/mootx01/Tests/MootInstallerCoreTests/ReleaseDownloaderTests.swift b/apps/mootx01/Tests/MootInstallerCoreTests/ReleaseDownloaderTests.swift index 6ff0a6ff..f529504e 100644 --- a/apps/mootx01/Tests/MootInstallerCoreTests/ReleaseDownloaderTests.swift +++ b/apps/mootx01/Tests/MootInstallerCoreTests/ReleaseDownloaderTests.swift @@ -115,6 +115,83 @@ struct ReleaseDownloaderTests { } } + /// A matching SHA-256 line is not enough on Linux/POSIX: checksums.txt must + /// also have a detached minisign signature. This prevents a tampered release + /// endpoint from supplying a malicious tarball plus a matching checksum file. + @Test func downloadRequiresMinisignSignatureWhenChecksumMatches() async throws { + #if !os(macOS) + let tag = "v1.1.0" + + #if arch(arm64) + let arch = "arm64" + #else + let arch = "x86_64" + #endif + let os = "linux" + let tarball = "mootx01-\(tag)-\(os)-\(arch).tar.gz" + let base = "https://github.com/codedaptive/mootx01-ce/releases/download/\(tag)" + + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("mootx01-minisig-required-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let contentDir = tmpDir.appendingPathComponent("contents", isDirectory: true) + try FileManager.default.createDirectory(at: contentDir, withIntermediateDirectories: true) + let fakeBinary = contentDir.appendingPathComponent("mootx01") + try Data("#!/bin/sh\necho tampered\n".utf8).write(to: fakeBinary) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fakeBinary.path) + + let tarballURL = tmpDir.appendingPathComponent(tarball) + let pack = Process() + pack.executableURL = URL(fileURLWithPath: "/usr/bin/tar") + pack.arguments = ["-czf", tarballURL.path, "-C", contentDir.path, "."] + pack.standardError = Pipe() + try pack.run() + pack.waitUntilExit() + guard pack.terminationStatus == 0 else { + Issue.record("Test setup: tar -czf exited \(pack.terminationStatus) — cannot continue") + return + } + + let tarballData = try Data(contentsOf: tarballURL) + let digestProcess = Process() + digestProcess.executableURL = URL(fileURLWithPath: "/usr/bin/shasum") + digestProcess.arguments = ["-a", "256", tarballURL.path] + let digestPipe = Pipe() + digestProcess.standardOutput = digestPipe + try digestProcess.run() + let digestOutput = digestPipe.fileHandleForReading.readDataToEndOfFile() + digestProcess.waitUntilExit() + guard digestProcess.terminationStatus == 0, + let digest = String(decoding: digestOutput, as: UTF8.self).split(separator: " ").first + else { + Issue.record("Test setup: shasum -a 256 exited \(digestProcess.terminationStatus)") + return + } + let checksum = "\(digest) \(tarball)\n" + + let downloader = ReleaseDownloader( + repo: testRepo, + currentVersion: "1.0.0", + fetchData: mockFetch([ + ok("\(base)/\(tarball)", tarballData), + ok("\(base)/checksums.txt", checksum), + // No checksums.txt.minisig fixture: the downloader must request + // it and fail closed rather than extracting the matching-checksum tarball. + ])) + + do { + _ = try await downloader.download(tag: tag) + Issue.record("Expected failure before extraction without checksums.txt.minisig") + } catch let e as URLError { + #expect(e.code == .badURL, "mockFetch should fail on the required minisig URL") + } catch { + Issue.record("Unexpected error type when minisig is absent: \(error)") + } + #endif + } + // MARK: unsafeMemberReason — zip-slip path safety predicate /// Absolute paths (starting with /) in a tar archive member allow overwriting