diff --git a/.github/scripts/postpack_signing.py b/.github/scripts/postpack_signing.py new file mode 100644 index 00000000..5198a9ac --- /dev/null +++ b/.github/scripts/postpack_signing.py @@ -0,0 +1,1084 @@ +#!/usr/bin/env python3 +"""Stage and write back the executables `vpk pack` generates, signed after packing. + +Ported unchanged in logic from PerformanceMonitor, where it was added in its #3288. + +WHY THIS RUNS AFTER PACKING RATHER THAN INSIDE IT + + Velopack generates three executables during `vpk pack` -- the portable launcher stub, + `Update.exe` (`Squirrel.exe` under its internal name) and `Setup.exe`. They do not exist when + the pre-pack signing rounds run, so those rounds cannot reach them. + + Signing them from inside packing is not available either. SignPath's open-source signing + policy requires origin verification through a trusted build system: a request is accepted only + through `signpath/github-action-submit-signing-request`, which attests repository, branch, + commit and build-job URL. That is a workflow step, and a workflow step cannot be invoked from + inside a running `vpk pack`. A raw API-token submission carries no attestation and the policy + answers 403. + + So the two standalone assets are signed by an Action step after packing. `Setup.exe` and + `Portable.zip` carry no recorded hash: `releases..json` records a SHA256 and a Size + for the two `.nupkg` files and nothing else, so rewriting the standalone assets invalidates + nothing. The copies of the stub and `Squirrel.exe` inside the `.nupkg` stay unsigned, two per + product, and they are the only executables a release publishes without a signature. They are + allowlisted by member path AND container in verify_release_signatures.py, so the same two + names anywhere else still fail its guard. + +WHAT IT SIGNS, AND HOW IT FINDS THEM + + Per product directory: the single `*-Setup.exe`, and every `.exe` at the ROOT of the single + `*Portable.zip`. The root entries are the deployed launcher and `Update.exe`; the signed app + payload sits one level down in `current/` and is not resubmitted. + + The root rule is positional, not by name, because the launcher is named after the Velopack + pack id rather than the application's own executable: Performance Studio's portable zip's root + entry is `PerformanceStudio.exe` while the application is `PlanViewer.App.exe` and lives at + `current/`. + +THE PORTABLE ZIP IS A SHIPPED ARTIFACT + + So `apply` rebuilds it member by member rather than re-creating it. Every member that is not + replaced keeps its local header, name, extra field and raw compressed bytes verbatim; the + central directory keeps every field except the local-header offsets, which shift. The two + replaced members are re-deflated with the original compression method. `apply` then reopens + the result and asserts that property before the rebuilt zip replaces the original: identical + entry names in identical order, identical per-entry metadata, byte-identical raw members + except the replaced ones, and identical decompressed content except the replaced ones. + +USAGE + + python postpack_signing.py collect --stage DIR --manifest FILE --product studio=releases/velopack + + python postpack_signing.py apply --signed DIR --manifest FILE + + python postpack_signing.py --self-test + + Exit status is 0 on success and 1 on any failure. There is no partial success: `apply` reads + and signature-checks the entire signing response, then builds and verifies every product's + rebuilt archive, and only then writes anything into the packed output. +""" + +from __future__ import annotations + +import argparse +import glob +import hashlib +import json +import os +import shutil +import struct +import sys +import zipfile +import zlib +from dataclasses import dataclass + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +import verify_release_signatures as vrs # noqa: E402 the one PE parse in the repo + +MANIFEST_VERSION = 1 + +_LFH = b"PK\x03\x04" +_CDH = b"PK\x01\x02" +_EOCD = b"PK\x05\x06" +_LFH_LEN = 30 +# Bit 3 puts the sizes in a trailing data descriptor instead of the local header, which makes a +# member's raw extent unknowable from the central directory alone. +_FLAG_DATA_DESCRIPTOR = 0x0008 +_U32_MAX = 0xFFFFFFFF +_U16_MAX = 0xFFFF + +# The fields a rebuilt zip has to reproduce exactly. `compress_size`, `CRC` and `file_size` are +# compared separately because the replaced members legitimately change all three. +_PRESERVED_FIELDS = ( + "compress_type", + "date_time", + "external_attr", + "internal_attr", + "create_system", + "create_version", + "extract_version", + "flag_bits", + "comment", + "extra", +) + + +class PostPackError(RuntimeError): + """A precondition, a postcondition or an input shape is wrong.""" + + +@dataclass +class Member: + """One zip member as the central directory describes it, plus two digests. + + `content_sha` covers the DECOMPRESSED bytes; `raw_sha` covers the local header, the name, + the extra field and the raw compressed bytes. The first says the content survived, the + second says the bytes did. + """ + + name: str + compress_type: int + date_time: tuple + external_attr: int + internal_attr: int + create_system: int + create_version: int + extract_version: int + flag_bits: int + comment: bytes + extra: bytes + crc: int + file_size: int + compress_size: int + content_sha: str + raw_sha: str + + +def _dos_datetime(date_time: tuple) -> bytes: + year, month, day, hour, minute, second = date_time + dosdate = (year - 1980) << 9 | month << 5 | day + dostime = hour << 11 | minute << 5 | (second // 2) + return struct.pack(" tuple[int, int, int, bytes]: + """(total_entries, cd_size, cd_offset, archive_comment).""" + window = min(66 * 1024, size) + handle.seek(size - window) + tail = handle.read(window) + i = tail.rfind(_EOCD) + if i < 0: + raise PostPackError("no end-of-central-directory record") + disk, cd_disk, here, total, cd_size, cd_offset, clen = struct.unpack_from(" list[Member]: + """Describe every member of `path`, in central-directory order.""" + out: list[Member] = [] + with open(path, "rb") as raw, zipfile.ZipFile(path) as zf: + size = os.path.getsize(path) + total, _, _, _ = _read_eocd(raw, size) + infos = zf.infolist() + if len(infos) != total: + raise PostPackError(f"{path}: eocd says {total} entries, central directory has {len(infos)}") + for info in infos: + if info.flag_bits & _FLAG_DATA_DESCRIPTOR: + raise PostPackError(f"{path} :: {info.filename}: data descriptor, raw extent unknown") + raw.seek(info.header_offset) + head = raw.read(_LFH_LEN) + if head[:4] != _LFH: + raise PostPackError(f"{path} :: {info.filename}: no local file header at {info.header_offset}") + nlen, elen = struct.unpack_from(" None: + """Write `source` to `destination` with `replacements` substituted by member name.""" + with open(source, "rb") as raw, zipfile.ZipFile(source) as zf: + infos = zf.infolist() + size = os.path.getsize(source) + with open(source, "rb") as raw: + total, _, _, archive_comment = _read_eocd(raw, size) + if len(infos) != total: + raise PostPackError(f"{source}: eocd says {total} entries, central directory has {len(infos)}") + unknown = sorted(set(replacements) - {i.filename for i in infos}) + if unknown: + raise PostPackError(f"{source}: no such member(s) to replace: {unknown}") + + central = bytearray() + with open(destination, "wb") as out: + for info in infos: + if info.flag_bits & _FLAG_DATA_DESCRIPTOR: + raise PostPackError(f"{source} :: {info.filename}: data descriptor, raw extent unknown") + raw.seek(info.header_offset) + head = raw.read(_LFH_LEN) + if head[:4] != _LFH: + raise PostPackError(f"{source} :: {info.filename}: no local file header") + nlen, elen = struct.unpack_from(" _U32_MAX or usize > _U32_MAX: + raise PostPackError(f"{source} :: {info.filename}: member needs zip64") + out.write(head) + out.write(name_and_extra) + out.write(body) + + central += _CDH + central += struct.pack( + " _U32_MAX or len(central) > _U32_MAX or len(infos) > _U16_MAX: + raise PostPackError(f"{source}: rebuilt archive needs zip64") + out.write(central) + out.write( + _EOCD + + struct.pack( + " None: + """Fail unless `rebuilt_path` differs from `before` in exactly the replaced members.""" + with zipfile.ZipFile(rebuilt_path) as zf: + bad = zf.testzip() + if bad is not None: + raise PostPackError(f"{rebuilt_path}: member '{bad}' fails its CRC") + + after = snapshot(rebuilt_path) + if [m.name for m in after] != [m.name for m in before]: + b, a = [m.name for m in before], [m.name for m in after] + raise PostPackError( + f"{rebuilt_path}: entry list changed " + f"({len(b)} -> {len(a)} entries; added {sorted(set(a) - set(b))[:5]}, " + f"removed {sorted(set(b) - set(a))[:5]}, order differs: {b != a})" + ) + + for old, new in zip(before, after): + for field in _PRESERVED_FIELDS: + if getattr(new, field) != getattr(old, field): + raise PostPackError( + f"{rebuilt_path} :: {old.name}: {field} changed " + f"{getattr(old, field)!r} -> {getattr(new, field)!r}" + ) + if old.name in replacements: + want = hashlib.sha256(replacements[old.name]).hexdigest() + if new.content_sha != want: + raise PostPackError(f"{rebuilt_path} :: {old.name}: content is not the signed file") + if new.raw_sha == old.raw_sha: + raise PostPackError(f"{rebuilt_path} :: {old.name}: unchanged, so it was not replaced") + else: + if new.raw_sha != old.raw_sha: + raise PostPackError(f"{rebuilt_path} :: {old.name}: raw bytes changed") + if (new.crc, new.file_size, new.compress_size, new.content_sha) != ( + old.crc, + old.file_size, + old.compress_size, + old.content_sha, + ): + raise PostPackError(f"{rebuilt_path} :: {old.name}: content changed") + + +def is_signed(path: str) -> bool: + """Whether `path` carries a certificate table. A file that is not a PE image is a refusal. + + Reading a non-PE as "unsigned" would submit it for signing and then report it as signed or + not on the strength of bytes that are not a PE header at all. + """ + try: + with open(path, "rb") as handle: + offset, length = vrs.certificate_table(handle.read(vrs.HEADER_BYTES)) + except (OSError, vrs.ReadError, struct.error) as exc: + raise PostPackError(f"{path}: not a PE image ({exc})") from exc + return offset != 0 and length != 0 + + +def exactly_one(directory: str, pattern: str, what: str) -> str: + hits = sorted(glob.glob(os.path.join(directory, pattern))) + if len(hits) != 1: + raise PostPackError( + f"{directory}: expected exactly one {what} matching '{pattern}', found " + f"{len(hits)}: {[os.path.basename(h) for h in hits]}" + ) + return hits[0] + + +def portable_root_executables(archive: str) -> list[str]: + """The `.exe` members at the archive root: the deployed launcher and `Update.exe`.""" + with zipfile.ZipFile(archive) as zf: + names = [info.filename for info in zf.infolist()] + roots = [ + name + for name in names + if name.lower().endswith(".exe") and "/" not in name and "\\" not in name + ] + if not roots: + raise PostPackError( + f"{archive}: no .exe at the archive root. The deployed launcher and Update.exe " + f"live there; {len(names)} entries were read." + ) + return roots + + +def collect(stage: str, manifest_path: str, products: list[tuple[str, str]]) -> int: + if os.path.exists(stage): + shutil.rmtree(stage) + os.makedirs(stage, exist_ok=True) + + manifest: dict = {"version": MANIFEST_VERSION, "products": []} + staged: dict[str, str] = {} + already: list[str] = [] + + for name, directory in products: + if not os.path.isdir(directory): + raise PostPackError(f"{directory}: not a directory") + setup = exactly_one(directory, "*-Setup.exe", "installer") + portable = exactly_one(directory, "*Portable.zip", "portable archive") + + entry: dict = { + "name": name, + "setup": {"path": setup, "stage": f"{name}-setup/{os.path.basename(setup)}"}, + "portable": {"path": portable, "members": []}, + } + + def stage_file(rel: str, writer) -> None: + if rel in staged: + raise PostPackError(f"two files stage to the same path '{rel}'") + target = os.path.join(stage, rel) + os.makedirs(os.path.dirname(target), exist_ok=True) + writer(target) + if is_signed(target): + already.append(rel) + staged[rel] = target + + stage_file(entry["setup"]["stage"], lambda dst: shutil.copyfile(setup, dst)) + + def write_blob(destination: str, blob: bytes) -> None: + with open(destination, "wb") as handle: + handle.write(blob) + + with zipfile.ZipFile(portable) as zf: + for member in portable_root_executables(portable): + rel = f"{name}-portable/{member}" + blob = zf.read(member) + stage_file(rel, lambda dst, blob=blob: write_blob(dst, blob)) + entry["portable"]["members"].append({"member": member, "stage": rel}) + + manifest["products"].append(entry) + print(f" {name}: {os.path.basename(setup)}") + for member in entry["portable"]["members"]: + print(f" {name}: {os.path.basename(portable)} :: {member['member']}") + + if not staged: + raise PostPackError("nothing was staged. An empty signing request signs nothing.") + + os.makedirs(os.path.dirname(os.path.abspath(manifest_path)), exist_ok=True) + with open(manifest_path, "w", encoding="utf-8", newline="\n") as handle: + json.dump(manifest, handle, indent=2) + handle.write("\n") + + print(f"\ncollect: {len(staged)} file(s) staged under {stage}") + if already: + print(f"collect: {len(already)} already carry a signature: {sorted(already)}") + return 0 + + +def apply(signed_dir: str, manifest_path: str) -> int: + with open(manifest_path, "r", encoding="utf-8") as handle: + manifest = json.load(handle) + if manifest.get("version") != MANIFEST_VERSION: + raise PostPackError(f"{manifest_path}: manifest version {manifest.get('version')!r} is not {MANIFEST_VERSION}") + + def signed_blob(rel: str) -> bytes: + path = os.path.join(signed_dir, rel) + if not os.path.isfile(path): + raise PostPackError(f"the signing response has no '{rel}' under {signed_dir}") + if not is_signed(path): + raise PostPackError(f"{rel} came back from signing without a certificate table") + with open(path, "rb") as handle: + return handle.read() + + # Every file in the response is read and checked before any artifact is written, so a bad + # response leaves the packed output exactly as `vpk pack` left it. Writing first and failing + # afterwards would still fail the release, but it would leave an unsigned installer on disk + # under a name the next step would otherwise have uploaded. + blobs: dict[str, bytes] = {} + for product in manifest["products"]: + blobs[product["setup"]["stage"]] = signed_blob(product["setup"]["stage"]) + for member in product["portable"]["members"]: + blobs[member["stage"]] = signed_blob(member["stage"]) + + # Nothing in the packed output is touched until every replacement has been built AND + # verified, for every product. Each one is written beside its target and moved into place by + # a rename, so a write that is interrupted leaves the original intact rather than truncated: + # `Setup.exe` is about 80 MB, and a half-written installer under the name the upload step reads + # is worse than a failed job. + pending: list[tuple[str, str]] = [] + scratch_paths: list[str] = [] + census: list[str] = [] + try: + for product in manifest["products"]: + setup = product["setup"]["path"] + scratch = setup + ".signed" + scratch_paths.append(scratch) + with open(scratch, "wb") as handle: + handle.write(blobs[product["setup"]["stage"]]) + pending.append((scratch, setup)) + census.append(f" {os.path.basename(setup)} signed") + + archive = product["portable"]["path"] + replacements = {m["member"]: blobs[m["stage"]] for m in product["portable"]["members"]} + if not replacements: + raise PostPackError(f"{archive}: no members to replace") + before = snapshot(archive) + rebuilt = archive + ".rebuilt" + scratch_paths.append(rebuilt) + rebuild(archive, replacements, rebuilt) + assert_faithful(before, rebuilt, replacements) + pending.append((rebuilt, archive)) + census.append( + f" {os.path.basename(archive)} rebuilt with {len(replacements)} signed " + f"member(s), {len(before)} entries preserved" + ) + + for scratch, target in pending: + os.replace(scratch, target) + except OSError as exc: + # Reported rather than raised as a traceback: the staging write and the rename are the + # two places a full disk or a killed runner lands, and the operator needs to be told + # that the packed output is unchanged rather than shown a stack. + raise PostPackError(f"the write-back could not complete: {exc}") from exc + finally: + for scratch in scratch_paths: + if os.path.isfile(scratch): + os.remove(scratch) + + written = 0 + for product in manifest["products"]: + setup = product["setup"]["path"] + if not is_signed(setup): + raise PostPackError(f"{setup}: unsigned after the write-back") + written += 1 + + archive = product["portable"]["path"] + with zipfile.ZipFile(archive) as zf: + for member in (m["member"] for m in product["portable"]["members"]): + offset, length = vrs.certificate_table(zf.read(member)[: vrs.HEADER_BYTES]) + if offset == 0 or length == 0: + raise PostPackError(f"{archive} :: {member}: unsigned after the rebuild") + written += 1 + + for line in census: + print(line) + + if not written: + raise PostPackError("the manifest named no files. An empty write-back signs nothing.") + print(f"\napply: {written} executable(s) written back and verified") + return 0 + + +# --------------------------------------------------------------------------------------------- +# Self-test. It builds portable archives and installers whose shape is the measured shape of the +# real PerformanceMonitor v3.7.0 assets (the fixture names are PerformanceMonitor's too): no directory entries, no extra fields, no comments, UTF-8 name flag on +# every entry, one stored `.portable` marker, and the launcher at the root under the pack id +# while the application payload sits at `current/`. +# --------------------------------------------------------------------------------------------- + + +def _portable_zip(path: str, root_exe: str, app_exe: str, members: dict[str, bytes]) -> None: + with zipfile.ZipFile(path, "w") as zf: + marker = zipfile.ZipInfo(".portable", date_time=(2026, 9, 12, 10, 0, 0)) + marker.compress_type = zipfile.ZIP_STORED + marker.flag_bits |= 0x800 + zf.writestr(marker, b"") + ordered = [(root_exe, vrs._synth_pe(signed=False)), ("Update.exe", vrs._synth_pe(signed=False))] + ordered += [(f"current/{app_exe}", vrs._synth_pe(signed=True))] + ordered += sorted(members.items()) + for name, blob in ordered: + info = zipfile.ZipInfo(name, date_time=(2026, 9, 12, 10, 0, 0)) + info.compress_type = zipfile.ZIP_DEFLATED + info.flag_bits |= 0x800 + zf.writestr(info, blob) + + +def _product_dir(root: str, name: str, pack_id: str, app_exe: str) -> str: + directory = os.path.join(root, f"velopack-{name}") + os.makedirs(directory, exist_ok=True) + with open(os.path.join(directory, f"{pack_id}-{name}-Setup.exe"), "wb") as handle: + handle.write(vrs._synth_pe(signed=False)) + _portable_zip( + os.path.join(directory, f"{pack_id}-{name}-Portable.zip"), + f"{pack_id}.exe", + app_exe, + {"current/Some.Library.dll": b"x" * 300, "current/sq.version": b"3.7.2"}, + ) + return directory + + +def _divergent_local_header(path: str) -> None: + """Set the UTF-8 name flag in one member's LOCAL header only, leaving the central one clear. + + Two bytes, no size or offset change. It gives a zip whose per-member metadata, CRC, sizes and + content are all readable from the central directory and all unchanged, while its raw bytes + differ from anything a rewriter that reconstructs local headers from the central directory + would produce. That is the only thing the raw-bytes comparison decides on its own. + """ + with zipfile.ZipFile(path) as zf: + target = zf.infolist()[1] + with open(path, "r+b") as handle: + handle.seek(target.header_offset + 6) + flags = struct.unpack(" str: + """Return a directory that mirrors `stage` with a certificate table appended to each file. + + A SignPath response is the same container with the same entry paths, signed. `--verify-dir` + and this script both read "is the certificate table non-empty", so appending a table is the + right stand-in: it is exactly the state the guard distinguishes. + + `only` restricts the appending to staged paths containing that substring, which produces the + response shape that matters most -- some files signed, some returned as submitted. + """ + signed = stage + suffix + if os.path.exists(signed): + shutil.rmtree(signed) + for base, _, files in os.walk(stage): + for name in files: + source = os.path.join(base, name) + rel = os.path.relpath(source, stage) + target = os.path.join(signed, rel) + os.makedirs(os.path.dirname(target), exist_ok=True) + with open(source, "rb") as handle: + blob = bytearray(handle.read()) + if only and only not in rel.replace(os.sep, "/"): + with open(target, "wb") as handle: + handle.write(bytes(blob)) + continue + table = b"\x00" * 0x1C0 + offset = len(blob) + pe_off = struct.unpack_from(" str: + """Rewrite `source` with local headers reconstructed from the central directory. + + What a `zipfile`-based round trip produces: every field the central directory carries is + preserved, and any local-header field that disagrees with it is silently normalised. + """ + destination = os.path.join(root, "from-central.zip") + with zipfile.ZipFile(source) as src, zipfile.ZipFile(destination, "w") as dst: + for info in src.infolist(): + fresh = zipfile.ZipInfo(info.filename, date_time=info.date_time) + fresh.compress_type = info.compress_type + fresh.flag_bits = info.flag_bits + fresh.external_attr = info.external_attr + fresh.internal_attr = info.internal_attr + fresh.create_system = info.create_system + fresh.create_version = info.create_version + fresh.extract_version = info.extract_version + fresh.comment = info.comment + fresh.extra = info.extra + dst.writestr(fresh, src.read(info)) + return destination + + +def self_test() -> int: + import tempfile + + failures: list[str] = [] + checks = 0 + + def expect_raises(label: str, fn) -> None: + nonlocal checks + checks += 1 + try: + fn() + except PostPackError: + return + except Exception as exc: # noqa: BLE001 any other exception is still a wrong failure + failures.append(f"{label}: raised {type(exc).__name__} instead of PostPackError: {exc}") + return + failures.append(f"{label}: succeeded instead of refusing") + + def expect_ok(label: str, fn): + nonlocal checks + checks += 1 + try: + return fn() + except Exception as exc: # noqa: BLE001 + failures.append(f"{label}: refused a correct input ({type(exc).__name__}: {exc})") + return None + + def expect(label: str, condition: bool) -> None: + nonlocal checks + checks += 1 + if not condition: + failures.append(label) + + with tempfile.TemporaryDirectory() as root: + lite = _product_dir(root, "lite", "PerformanceMonitorLite", "PerformanceMonitorLite.exe") + # The Darling Viewer case: the root launcher carries the PACK ID, and the application's + # own executable name appears only under current/. A name-based rule misses the launcher + # and submits the already-signed payload instead. + viewer = _product_dir( + root, + "darlingviewer", + "PerformanceMonitorDarlingViewer", + "PerformanceMonitor.Darling.Viewer.exe", + ) + + stage = os.path.join(root, "stage") + manifest = os.path.join(root, "manifest.json") + expect_ok( + "collect refused a correct pair of product directories", + lambda: collect(stage, manifest, [("lite", lite), ("darlingviewer", viewer)]), + ) + with open(manifest, encoding="utf-8") as handle: + recorded = json.load(handle) + rels = sorted( + [p["setup"]["stage"] for p in recorded["products"]] + + [m["stage"] for p in recorded["products"] for m in p["portable"]["members"]] + ) + expect( + f"collect staged {rels} rather than the six expected files", + rels + == [ + "darlingviewer-portable/PerformanceMonitorDarlingViewer.exe", + "darlingviewer-portable/Update.exe", + "darlingviewer-setup/PerformanceMonitorDarlingViewer-darlingviewer-Setup.exe", + "lite-portable/PerformanceMonitorLite.exe", + "lite-portable/Update.exe", + "lite-setup/PerformanceMonitorLite-lite-Setup.exe", + ], + ) + expect( + "collect staged a member from current/, which is signed pre-pack", + not any("current" in rel for rel in rels), + ) + + # Two installers, or none, means the glob is matching something other than what packing + # produced -- silently signing the wrong file, or the wrong number of them. + two = os.path.join(root, "two-setups") + os.makedirs(two, exist_ok=True) + shutil.copytree(lite, two, dirs_exist_ok=True) + shutil.copyfile( + os.path.join(lite, "PerformanceMonitorLite-lite-Setup.exe"), + os.path.join(two, "PerformanceMonitorLite-previous-Setup.exe"), + ) + expect_raises( + "collect accepted two *-Setup.exe in one product directory", + lambda: collect(os.path.join(root, "s2"), os.path.join(root, "m2.json"), [("lite", two)]), + ) + + none = os.path.join(root, "no-portable") + os.makedirs(none, exist_ok=True) + shutil.copyfile( + os.path.join(lite, "PerformanceMonitorLite-lite-Setup.exe"), + os.path.join(none, "PerformanceMonitorLite-lite-Setup.exe"), + ) + expect_raises( + "collect accepted a product directory with no portable archive", + lambda: collect(os.path.join(root, "s3"), os.path.join(root, "m3.json"), [("lite", none)]), + ) + + notpe = os.path.join(root, "not-pe") + os.makedirs(notpe, exist_ok=True) + with open(os.path.join(notpe, "Thing-lite-Setup.exe"), "wb") as handle: + handle.write(b"this is not a PE image") + _portable_zip( + os.path.join(notpe, "Thing-lite-Portable.zip"), "Thing.exe", "Thing.exe", {} + ) + expect_raises( + "collect accepted an installer that is not a PE image", + lambda: collect(os.path.join(root, "s4"), os.path.join(root, "m4.json"), [("lite", notpe)]), + ) + + # apply refuses a response that is missing a file, and one that comes back unsigned. + expect_raises( + "apply accepted a signing response with nothing in it", + lambda: apply(os.path.join(root, "empty-response"), manifest), + ) + def artifact_digests() -> dict[str, str]: + out = {} + for directory in (lite, viewer): + for name in sorted(os.listdir(directory)): + path = os.path.join(directory, name) + if not os.path.isfile(path): + continue + with open(path, "rb") as handle: + out[f"{directory}/{name}"] = hashlib.sha256(handle.read()).hexdigest() + return out + + untouched = artifact_digests() + expect_raises( + "apply accepted files that came back without a certificate table", + lambda: apply(stage, manifest), + ) + expect( + "apply modified a packed artifact before refusing the signing response", + artifact_digests() == untouched, + ) + + # The response shape that separates "refuses" from "refuses without writing": the + # installers come back signed and the portable members come back as submitted. A run that + # checks each file as it writes it leaves a signed installer beside an unbuilt zip. + partial = _sign_stage(stage, suffix="-partial", only="-setup/") + untouched = artifact_digests() + expect_raises( + "apply accepted a response in which only some files came back signed", + lambda: apply(partial, manifest), + ) + expect( + "apply wrote the signed installers before refusing a partially signed response", + artifact_digests() == untouched, + ) + + # And the mirror: the portable members come back signed and the installers come back as + # submitted. Every rebuild then succeeds, so only a check on the installers refuses it -- + # and the artifacts have to be untouched when it does. + mirror = _sign_stage(stage, suffix="-mirror", only="-portable/") + untouched = artifact_digests() + expect_raises( + "apply accepted a response whose installers came back unsigned", + lambda: apply(mirror, manifest), + ) + expect( + "apply wrote the rebuilt archives before refusing an unsigned installer", + artifact_digests() == untouched, + ) + + # A fully signed response whose SECOND product cannot be rebuilt. This is the case that + # decides whether the write-back is atomic ACROSS products: the first product's rebuild + # succeeds, and the refusal arrives while its artifacts must still be untouched. + awkward = os.path.join(root, "velopack-awkward") + os.makedirs(awkward, exist_ok=True) + with open(os.path.join(awkward, "Awkward-awkward-Setup.exe"), "wb") as handle: + handle.write(vrs._synth_pe(signed=False)) + with zipfile.ZipFile(os.path.join(awkward, "Awkward-awkward-Portable.zip"), "w") as zf: + marker = zipfile.ZipInfo(".portable", date_time=(2026, 9, 12, 10, 0, 0)) + marker.compress_type = zipfile.ZIP_STORED + zf.writestr(marker, b"") + # A compression method the rebuilder refuses rather than silently re-encoding. + odd = zipfile.ZipInfo("Awkward.exe", date_time=(2026, 9, 12, 10, 0, 0)) + odd.compress_type = zipfile.ZIP_BZIP2 + zf.writestr(odd, vrs._synth_pe(signed=False)) + + pair_stage = os.path.join(root, "pair-stage") + pair_manifest = os.path.join(root, "pair.json") + expect_ok( + "collect refused a product whose archive uses an unusual compression method", + lambda: collect(pair_stage, pair_manifest, [("lite", lite), ("awkward", awkward)]), + ) + pair_signed = _sign_stage(pair_stage, suffix="-signed") + untouched = artifact_digests() + expect_raises( + "apply accepted a response whose second product cannot be rebuilt", + lambda: apply(pair_signed, pair_manifest), + ) + expect( + "apply committed the first product before the second product's rebuild failed", + artifact_digests() == untouched, + ) + + # An installer that cannot be staged beside its target. `Setup.exe` is 124 MB in a real + # release, so it is written to a scratch path and renamed; writing it in place would + # leave a truncated installer under the name the upload step reads. Making the scratch + # path a directory is the cheapest way to fail that write without a full disk. + blocked = os.path.join(lite, "PerformanceMonitorLite-lite-Setup.exe.signed") + os.makedirs(blocked, exist_ok=True) + whole = _sign_stage(stage, suffix="-whole") + untouched = artifact_digests() + expect_raises( + "apply accepted an installer it could not stage beside its target", + lambda: apply(whole, manifest), + ) + expect( + "apply modified an artifact after failing to stage the installer", + artifact_digests() == untouched, + ) + os.rmdir(blocked) + + # A manifest naming no products. `collect` cannot write one, but `apply` reads a file + # rather than a return value, so the guard on its own census is what stops a hand-edited + # or future manifest from reporting success over an empty write-back -- the failure this + # whole issue is about, in the one place left that could still produce it. + empty_manifest = os.path.join(root, "empty.json") + with open(empty_manifest, "w", encoding="utf-8") as handle: + json.dump({"version": MANIFEST_VERSION, "products": []}, handle) + expect_raises( + "apply reported success over a manifest naming no products", + lambda: apply(whole, empty_manifest), + ) + + signed = _sign_stage(stage) + before_lite = snapshot(os.path.join(lite, "PerformanceMonitorLite-lite-Portable.zip")) + expect_ok("apply refused a correct signing response", lambda: apply(signed, manifest)) + + after_lite = snapshot(os.path.join(lite, "PerformanceMonitorLite-lite-Portable.zip")) + expect( + f"the rebuilt portable zip has {len(after_lite)} entries, not {len(before_lite)}", + len(after_lite) == len(before_lite), + ) + expect( + "the rebuilt portable zip reordered or renamed its entries", + [m.name for m in after_lite] == [m.name for m in before_lite], + ) + replaced = {"PerformanceMonitorLite.exe", "Update.exe"} + unchanged_raw_held = all( + new.raw_sha == old.raw_sha + for old, new in zip(before_lite, after_lite) + if old.name not in replaced + ) + expect("a member that was not replaced changed its raw bytes", unchanged_raw_held) + changed = { + new.name for old, new in zip(before_lite, after_lite) if new.raw_sha != old.raw_sha + } + expect(f"the rebuild changed {sorted(changed)} rather than the two root executables", changed == replaced) + with zipfile.ZipFile(os.path.join(lite, "PerformanceMonitorLite-lite-Portable.zip")) as zf: + signed_now = { + name: vrs.certificate_table(zf.read(name)[: vrs.HEADER_BYTES]) != (0, 0) + for name in replaced + } + expect(f"the rebuilt root executables are not signed: {signed_now}", all(signed_now.values())) + expect( + "the installer is unsigned after apply", + is_signed(os.path.join(lite, "PerformanceMonitorLite-lite-Setup.exe")), + ) + + # The faithfulness check has to catch a rebuild that loses structure rather than content. + # Compress-Archive and a plain zipfile round-trip both drop the stored `.portable` marker's + # method and reorder entries, which is the failure this guards. + source = os.path.join(viewer, "PerformanceMonitorDarlingViewer-darlingviewer-Portable.zip") + before_viewer = snapshot(source) + lossy = os.path.join(root, "lossy.zip") + with zipfile.ZipFile(source) as src, zipfile.ZipFile(lossy, "w", zipfile.ZIP_DEFLATED) as dst: + for name in reversed(src.namelist()): + dst.writestr(name, src.read(name)) + expect_raises( + "the faithfulness check accepted a reordered, re-created archive", + lambda: assert_faithful(before_viewer, lossy, {}), + ) + + # And a rebuild that preserves structure but does not actually replace anything. + noop = os.path.join(root, "noop.zip") + rebuild(source, {}, noop) + expect_ok("the faithfulness check rejected a byte-faithful rebuild", lambda: assert_faithful(before_viewer, noop, {})) + expect_raises( + "the faithfulness check accepted a rebuild that replaced nothing it was told to", + lambda: assert_faithful( + before_viewer, noop, {"PerformanceMonitorDarlingViewer.exe": vrs._synth_pe(signed=True)} + ), + ) + expect_raises( + "rebuild accepted a member name that is not in the archive", + lambda: rebuild(source, {"NoSuchFile.exe": b"x"}, os.path.join(root, "bad.zip")), + ) + + # The case that isolates the raw-bytes comparison from every other assertion: same + # entries, same order, same metadata, same content, re-deflated at a different level. + # Only a check on the compressed bytes can tell this apart from a faithful rebuild, and + # a rebuild that recompresses is the one that would silently rewrite a shipped artifact. + recompressed = os.path.join(root, "recompressed.zip") + with zipfile.ZipFile(source) as src, zipfile.ZipFile(recompressed, "w") as dst: + for info in src.infolist(): + fresh = zipfile.ZipInfo(info.filename, date_time=info.date_time) + fresh.compress_type = info.compress_type + fresh.flag_bits = info.flag_bits + fresh.external_attr = info.external_attr + fresh.internal_attr = info.internal_attr + fresh.create_system = info.create_system + fresh.create_version = info.create_version + fresh.extract_version = info.extract_version + fresh.comment = info.comment + fresh.extra = info.extra + fresh._compresslevel = 1 + dst.writestr(fresh, src.read(info)) + expect_raises( + "the faithfulness check accepted a recompressed archive as byte-faithful", + lambda: assert_faithful(before_viewer, recompressed, {}), + ) + + # The raw-bytes comparison on its own. This pair is identical in every field the central + # directory carries -- names, order, method, timestamps, attributes, CRC, both sizes and + # content -- and differs only in two bytes of one LOCAL header, which is what a rewriter + # that reconstructs local headers rather than copying them changes. + divergent = os.path.join(root, "divergent.zip") + with zipfile.ZipFile(divergent, "w") as zf: + for name, blob in (("a.txt", b"a" * 400), ("b.txt", b"b" * 400), ("c.txt", b"c" * 400)): + info = zipfile.ZipInfo(name, date_time=(2026, 9, 12, 10, 0, 0)) + info.compress_type = zipfile.ZIP_DEFLATED + zf.writestr(info, blob) + reconstructed = snapshot(divergent) + _divergent_local_header(divergent) + divergent_snapshot = snapshot(divergent) + expect( + "patching a local header changed a field the central directory reports", + [ + (m.name, m.compress_type, m.crc, m.file_size, m.compress_size, m.content_sha) + for m in divergent_snapshot + ] + == [ + (m.name, m.compress_type, m.crc, m.file_size, m.compress_size, m.content_sha) + for m in reconstructed + ], + ) + faithful_copy = os.path.join(root, "divergent-copy.zip") + rebuild(divergent, {}, faithful_copy) + expect_ok( + "rebuild did not copy a local header that disagrees with the central directory", + lambda: assert_faithful(divergent_snapshot, faithful_copy, {}), + ) + expect_raises( + "the faithfulness check accepted an archive whose local headers were reconstructed", + lambda: assert_faithful(divergent_snapshot, _rebuilt_from_central(divergent, root), {}), + ) + + # Order, isolated from content: two members with identical content and metadata under + # different names, swapped. Only a check on the entry ORDER, or one on raw bytes that + # includes the member name, tells the two archives apart. + twins = os.path.join(root, "twins.zip") + swapped = os.path.join(root, "twins-swapped.zip") + for target, names in ((twins, ("one.bin", "two.bin")), (swapped, ("two.bin", "one.bin"))): + with zipfile.ZipFile(target, "w") as zf: + for name in names: + info = zipfile.ZipInfo(name, date_time=(2026, 9, 12, 10, 0, 0)) + info.compress_type = zipfile.ZIP_DEFLATED + zf.writestr(info, b"identical" * 40) + expect_raises( + "the faithfulness check accepted two same-content members in swapped order", + lambda: assert_faithful(snapshot(twins), swapped, {}), + ) + + for failure in failures: + print(f"SELF-TEST FAIL: {failure}", file=sys.stderr) + if failures: + return 1 + print(f"self-test: {checks} assertions passed") + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("mode", nargs="?", choices=("collect", "apply")) + parser.add_argument("--self-test", action="store_true", dest="run_self_test") + parser.add_argument("--stage", help="collect: directory to stage the unsigned files into") + parser.add_argument("--signed", help="apply: directory holding the signing response") + parser.add_argument("--manifest", help="the file collect writes and apply reads") + parser.add_argument( + "--product", + action="append", + default=[], + metavar="NAME=DIR", + help="collect: a product name and its `vpk pack` output directory", + ) + args = parser.parse_args() + + if args.run_self_test: + return self_test() + if not args.mode: + parser.error("a mode is required unless --self-test is given") + if not args.manifest: + parser.error("--manifest is required") + + try: + if args.mode == "collect": + if not args.stage: + parser.error("--stage is required for collect") + if not args.product: + parser.error("at least one --product NAME=DIR is required for collect") + products = [] + for spec in args.product: + name, _, directory = spec.partition("=") + if not name or not directory: + parser.error(f"--product expects NAME=DIR, got {spec!r}") + products.append((name, directory)) + return collect(args.stage, args.manifest, products) + if not args.signed: + parser.error("--signed is required for apply") + return apply(args.signed, args.manifest) + except PostPackError as exc: + print(f"postpack: FAIL: {exc}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/scripts/verify_release_signatures.py b/.github/scripts/verify_release_signatures.py new file mode 100644 index 00000000..129d8e87 --- /dev/null +++ b/.github/scripts/verify_release_signatures.py @@ -0,0 +1,741 @@ +#!/usr/bin/env python3 +"""Report which published Windows executables carry an Authenticode signature. + +Ported unchanged in logic from PerformanceMonitor, where it was added in its #3288. + +Every executable `vpk pack` GENERATES shipped unsigned on every Performance Studio release +through v1.28.0 -- the portable launcher stub, `Update.exe`, and `Setup.exe` -- while the app +payload was signed. PerformanceMonitor had the same gap in v3.6.0 and v3.7.0. It goes +unnoticed because the only visible symptom is a Windows "unknown publisher" +block on someone else's machine, and because nothing asked the question. + +So this asks it, from any platform: is there a signature attached to each thing a user can +launch. `--verify-dir` asks it of artifacts still on disk in the release job, which is the step +that refuses to publish. The tag form asks it of what a release actually shipped. + +WHAT IT CHECKS, AND WHAT IT DELIBERATELY DOES NOT + + It reads the PE optional header's Security data directory (index 4) and reports whether the + certificate table is non-empty. That is "a signature is attached" and nothing more. + + It does NOT verify the signature: not the chain, not trust, not the timestamp, not revocation, + not whether the digest matches the file. Doing that properly needs a Windows trust store, and + the defect actually shipped twice was TOTAL ABSENCE -- offset 0, size 0 -- which this catches + for free. Read a pass as "signed", never as "correctly signed". + + It only understands PE files. A NuGet package signature, an MSIX catalog, or a detached + signature file are all invisible to it. + + Executables inside a published archive are checked too, because the one that matters most is + inside one: the portable zip's ROOT `PerformanceStudio.exe` is a 397 kB launcher shim, + while the signed 180 kB app sits in `current/`. A check that only looked at top-level assets + would have passed every affected release. + + Two executables are allowed to be unsigned, and only in one place each: `lib/app/*_ExecutionStub.exe` + and `lib/app/Squirrel.exe` INSIDE a `.nupkg`. `releases..json` records a SHA256 and a + Size for each `.nupkg`, the delta package patches against those bytes, and `vpk pack` is what + writes that manifest -- so re-signing a member of a `.nupkg` invalidates it. The copies of the + same two files that ship in `Portable.zip` carry no recorded hash and ARE signed, and the + allowance is keyed on the container as well as the member path so those copies, and a + same-named file anywhere else, still fail. See ALLOWED_UNSIGNED. + +COST, because it is why this is runnable rather than theoretical + + It never downloads a whole artifact. For each standalone `.exe` it fetches the first 4 kB to + locate the certificate table, then that table's byte range. For each archive it fetches the + end-of-central-directory record, the central directory, and then only the entries that are + executables. In PerformanceMonitor, checking v3.7.0's five Lite assets moved a few MB rather + than the ~370 MB those assets total. + + This relies on the asset host honouring HTTP Range. GitHub release downloads do. If a host + ignores Range and returns the whole body, the reads still succeed and it merely costs what a + download would -- it does not silently check the wrong bytes, because a short or unexpected + read raises rather than being interpreted. + +USAGE + + python .github/scripts/verify_release_signatures.py v1.28.0 + python .github/scripts/verify_release_signatures.py v1.28.0 --json + python .github/scripts/verify_release_signatures.py --verify-dir releases/velopack + python .github/scripts/verify_release_signatures.py --self-test + + Exit status is 1 if any executable is unsigned and not covered by ALLOWED_UNSIGNED, 2 if a + release, asset or directory could not be read at all, and 0 otherwise. An empty executable + list is status 2, not 0: finding nothing to check is a broken run, not a clean one. + + `--verify-dir` is the release guard: the same reader over artifacts that are still on disk + before upload, so it needs no HTTP and no published release. The tag form answers "what did we + actually publish", which is the question that goes unasked. Both apply ALLOWED_UNSIGNED. +""" + +from __future__ import annotations + +import argparse +import json +import re +import struct +import subprocess +import sys +import zlib +from dataclasses import dataclass, field + +ARCHIVE_SUFFIXES = (".zip", ".nupkg") +EXECUTABLE_SUFFIXES = (".exe", ".dll") +HEADER_BYTES = 4096 +# The end-of-central-directory record is 22 bytes plus a comment of up to 64 kB. +EOCD_SEARCH_BYTES = 66 * 1024 + + +@dataclass(frozen=True) +class AllowedUnsigned: + """One executable that is allowed to be unsigned, in one kind of container only. + + `container_suffix` is matched against the containing archive's name and `member` against the + path inside it. A standalone file has no container and therefore no allowance: that is what + keeps an unsigned `Setup.exe`, or an unsigned copy of either of these two files sitting + beside it, a failure. + """ + + container_suffix: str + member: re.Pattern[str] + reason: str + + +ALLOWED_UNSIGNED: tuple[AllowedUnsigned, ...] = ( + AllowedUnsigned( + container_suffix=".nupkg", + member=re.compile(r"\Alib/app/[^/]+_ExecutionStub\.exe\Z", re.IGNORECASE), + reason=( + "Velopack generates the launcher stub during `vpk pack`, so no pre-pack round reaches " + "it, and `releases..json` records this package's SHA256 and Size for the " + "updater and the delta to patch against, so no post-pack round may rewrite it. The " + "copy in Portable.zip is signed after packing." + ), + ), + AllowedUnsigned( + container_suffix=".nupkg", + member=re.compile(r"\Alib/app/Squirrel\.exe\Z", re.IGNORECASE), + reason=( + "Velopack's updater, generated during `vpk pack` and deployed as Update.exe. Same " + "recorded-hash constraint as the launcher stub. The copy in Portable.zip is signed " + "after packing." + ), + ), +) + + +def allowance_for(container: str, member: str) -> AllowedUnsigned | None: + """The allowance covering an unsigned `member` of `container`, or None. + + `member` is empty for a standalone file, which never has an allowance. + """ + if not member: + return None + normalised = member.replace("\\", "/") + for allowed in ALLOWED_UNSIGNED: + if not container.lower().endswith(allowed.container_suffix): + continue + if allowed.member.search(normalised): + return allowed + return None + + +class ReadError(RuntimeError): + """A byte range could not be read, or came back the wrong size.""" + + +@dataclass +class Finding: + asset: str + path: str + size: int + signed: bool + note: str = "" + + @property + def label(self) -> str: + return self.asset if self.path == "" else f"{self.asset} :: {self.path}" + + +@dataclass +class Report: + tag: str + findings: list[Finding] = field(default_factory=list) + errors: list[str] = field(default_factory=list) + + @property + def allowed(self) -> list[tuple[Finding, AllowedUnsigned]]: + pairs = ((f, allowance_for(f.asset, f.path)) for f in self.findings if not f.signed) + return [(f, a) for f, a in pairs if a is not None] + + @property + def unsigned(self) -> list[Finding]: + """Unsigned and NOT allowed. The same allowlist the release guard applies. + + Both modes read the same allowlist so that a correct release audits clean here. A mode + that reported the two allowed `.nupkg` members as failures would be red on every release + from now on, which is the state in which nobody reads it. + """ + return [ + f + for f in self.findings + if not f.signed and allowance_for(f.asset, f.path) is None + ] + + +def fetch_range(url: str, start: int, length: int) -> bytes: + """Fetch `length` bytes at `start`. Raises rather than returning a short read.""" + end = start + length - 1 + proc = subprocess.run( + ["curl", "-sSL", "-r", f"{start}-{end}", "--fail", url], + capture_output=True, + ) + if proc.returncode != 0: + raise ReadError(f"range {start}-{end} failed: {proc.stderr.decode(errors='replace')[:200]}") + data = proc.stdout + # A host ignoring Range returns the whole body, which is longer than asked for. That is + # usable (the prefix is still the right bytes for a start of 0) but a SHORT read is not. + if len(data) < length: + raise ReadError(f"range {start}-{end} returned {len(data)} of {length} bytes") + return data[:length] + + +def certificate_table(pe: bytes) -> tuple[int, int]: + """Return (offset, size) of the PE certificate table, or (0, 0) when absent.""" + if pe[:2] != b"MZ": + raise ReadError("not an MZ image") + pe_off = struct.unpack_from(" Finding: + head = fetch_range(url, 0, min(HEADER_BYTES, size)) + off, length = certificate_table(head) + return Finding(asset=name, path="", size=size, signed=off != 0 and length != 0) + + +def zip_entries(url: str, size: int) -> list[tuple[str, int, int, int, int]]: + """(name, method, compressed_size, uncompressed_size, local_header_offset) per entry.""" + window = min(EOCD_SEARCH_BYTES, size) + tail = fetch_range(url, size - window, window) + i = tail.rfind(b"PK\x05\x06") + if i < 0: + raise ReadError("no end-of-central-directory record") + cd_size, cd_off = struct.unpack_from(" bytes: + lh = fetch_range(url, lho, 30) + nlen, elen = struct.unpack_from(" tuple[list[Finding], list[str]]: + findings: list[Finding] = [] + errors: list[str] = [] + wanted = EXECUTABLE_SUFFIXES if include_dlls else (".exe",) + for member, method, csize, usize, lho in zip_entries(url, size): + if not member.lower().endswith(wanted): + continue + try: + blob = read_zip_member(url, method, csize, lho) + off, length = certificate_table(blob) + findings.append( + Finding(asset=name, path=member, size=usize, signed=off != 0 and length != 0) + ) + except ReadError as exc: + errors.append(f"{name} :: {member}: {exc}") + return findings, errors + + +def verify_local_dir(directory: str) -> int: + """Assert every executable under `directory`, including inside zips and nupkgs, is signed. + + This is the release guard. It runs on the artifacts as they sit on disk BEFORE upload, so it + needs no network and no published release -- the difference between this and the tag mode is + only where the bytes come from. + + ALLOWED_UNSIGNED exempts exactly two members of a `.nupkg`. Everything else fails, including + those same two names in any other container and as standalone files. + """ + import glob + import os + import zipfile + + findings: list[Finding] = [] + for path in sorted(glob.glob(os.path.join(directory, "**", "*"), recursive=True)): + if not os.path.isfile(path): + continue + lower = path.lower() + rel = os.path.relpath(path, directory) + if lower.endswith(".exe"): + try: + with open(path, "rb") as handle: + off, length = certificate_table(handle.read(HEADER_BYTES)) + findings.append(Finding(rel, "", os.path.getsize(path), off != 0 and length != 0)) + except (OSError, ReadError, struct.error) as exc: + print(f"GUARD: cannot read {rel}: {exc}", file=sys.stderr) + return 2 + elif lower.endswith(ARCHIVE_SUFFIXES): + try: + with zipfile.ZipFile(path) as zf: + for info in zf.infolist(): + if not info.filename.lower().endswith(".exe"): + continue + blob = zf.read(info) + off, length = certificate_table(blob) + findings.append( + Finding(rel, info.filename, info.file_size, off != 0 and length != 0) + ) + except (OSError, zipfile.BadZipFile, ReadError, struct.error) as exc: + print(f"GUARD: cannot read {rel}: {exc}", file=sys.stderr) + return 2 + + if not findings: + print(f"GUARD FAIL: no executables found under {directory}", file=sys.stderr) + return 2 + + allowed: list[tuple[Finding, AllowedUnsigned]] = [] + unsigned: list[Finding] = [] + for finding in findings: + if finding.signed: + continue + allowance = allowance_for(os.path.basename(finding.asset), finding.path) + if allowance is None: + unsigned.append(finding) + else: + allowed.append((finding, allowance)) + + allowed_ids = {id(entry) for entry, _ in allowed} + for finding in sorted(findings, key=lambda x: (x.signed, x.label)): + if finding.signed: + mark = "signed " + elif id(finding) in allowed_ids: + mark = "allowed " + else: + mark = "UNSIGNED" + print(f" {mark} {finding.label}") + + for finding, allowance in allowed: + print(f"\nallowed unsigned: {finding.label}\n {allowance.reason}") + + # An allowance that matches nothing has outlived the constraint that justifies it. It is + # reported rather than fatal: the guard's job is to refuse unsigned executables, and an + # allowance covering none of them refuses nothing. + used = {id(allowance) for _, allowance in allowed} + for allowance in ALLOWED_UNSIGNED: + if id(allowance) not in used: + print( + f"ALLOWANCE UNUSED: {allowance.container_suffix} :: " + f"{allowance.member.pattern} matched no unsigned executable" + ) + + print( + f"\n{len(findings)} executable(s) checked, {len(unsigned)} unsigned, " + f"{len(allowed)} allowed unsigned" + ) + if unsigned: + print(f"GUARD FAIL: {len(unsigned)} unsigned executable(s). See PerformanceMonitor #3288.", file=sys.stderr) + return 1 + return 0 + + +def release_assets(tag: str, repo: str, gh: str) -> list[dict]: + proc = subprocess.run( + [gh, "api", f"repos/{repo}/releases/tags/{tag}"], capture_output=True, text=True + ) + if proc.returncode != 0: + raise ReadError(f"cannot read release {tag}: {proc.stderr.strip()[:300]}") + return json.loads(proc.stdout).get("assets", []) + + +def build_report(tag: str, repo: str, gh: str, include_dlls: bool) -> Report: + report = Report(tag=tag) + for asset in release_assets(tag, repo, gh): + name, url, size = asset["name"], asset["browser_download_url"], asset["size"] + lower = name.lower() + try: + if lower.endswith(".exe"): + report.findings.append(check_standalone_exe(url, name, size)) + elif lower.endswith(ARCHIVE_SUFFIXES): + found, errs = check_archive(url, name, size, include_dlls) + report.findings.extend(found) + report.errors.extend(errs) + except ReadError as exc: + report.errors.append(f"{name}: {exc}") + return report + + +def _synth_pe(*, signed: bool, plus: bool = True) -> bytes: + """A minimal PE image with the Security directory either populated or zeroed. + + The control this whole script needs: a checker that answered "signed" unconditionally would + have reported every release clean, which is indistinguishable from the fix having landed. + """ + buf = bytearray(1024) + buf[0:2] = b"MZ" + pe_off = 128 + struct.pack_into(" str: + """Materialise a release directory. A str key is a standalone file, a dict is an archive. + + Used by the self-test so the guard is exercised through its real entry point -- a directory + of real zip and nupkg files -- rather than through a stubbed finding list. + """ + import os + import zipfile + + os.makedirs(root, exist_ok=True) + for name, content in layout.items(): + target = os.path.join(root, name) + os.makedirs(os.path.dirname(target) or root, exist_ok=True) + if isinstance(content, dict): + with zipfile.ZipFile(target, "w") as zf: + for member, signed in content.items(): + zf.writestr(member, _synth_pe(signed=bool(signed))) + else: + with open(target, "wb") as handle: + handle.write(_synth_pe(signed=bool(content))) + return root + + +def self_test() -> int: + import contextlib + import io + import os + import tempfile + + failures: list[str] = [] + checks = 0 + + def guard(label: str, layout: dict[str, object], want: int) -> None: + """Run the release guard over a synthesised release directory.""" + nonlocal checks + checks += 1 + with tempfile.TemporaryDirectory() as tmp: + directory = _release_dir(os.path.join(tmp, "releases"), layout) + sink = io.StringIO() + with contextlib.redirect_stdout(sink), contextlib.redirect_stderr(sink): + got = verify_local_dir(directory) + if got != want: + failures.append(f"{label}: guard returned {got}, expected {want}") + + def expect(label: str, condition: bool) -> None: + nonlocal checks + checks += 1 + if not condition: + failures.append(label) + + for plus in (True, False): + shape = "PE32+" if plus else "PE32" + checks += 2 + off, size = certificate_table(_synth_pe(signed=True, plus=plus)) + if (off, size) == (0, 0): + failures.append(f"{shape}: a signed image read as unsigned") + off, size = certificate_table(_synth_pe(signed=False, plus=plus)) + if (off, size) != (0, 0): + failures.append(f"{shape}: an unsigned image read as signed ({off}, {size})") + + # Both directions matter, and so does refusing garbage rather than defaulting either way. + # + # The third case is load-bearing and was added because a mutation exposed its absence: with + # the MZ check deleted, "empty", "not MZ" and "MZ without PE" were ALL still caught by the + # PE-signature check below it, so nothing actually asserted that the MZ check does anything. + # This one is an image that is not MZ but DOES carry a well-formed PE header at the offset, + # which only the MZ check can reject. + not_mz_but_valid_pe = bytearray(_synth_pe(signed=True)) + not_mz_but_valid_pe[0:2] = b"ZZ" + for label, blob in ( + ("empty", b""), + ("not MZ", b"ZM" + bytes(1022)), + ("MZ without PE", b"MZ" + bytes(1022)), + ("non-MZ carrying a valid PE header", bytes(not_mz_but_valid_pe)), + ): + checks += 1 + try: + certificate_table(blob) + except (ReadError, struct.error, IndexError): + pass + else: + failures.append(f"{label}: returned a verdict instead of raising") + + # A short range must raise, or a truncated read silently becomes "no signature". + checks += 1 + try: + fetch_range("file:///dev/null", 0, 64) + except ReadError: + pass + else: + failures.append("a short read returned data instead of raising") + + # ------------------------------------------------------------------------------------------ + # The release guard and its allowlist. + # + # A blanket skip by filename is the failure mode these cases exist to refuse: the two allowed + # names also appear as the portable zip's root launcher and as Update.exe, and `Setup.exe` + # shares nothing with them but would pass any allowlist loose enough to be written by name + # alone. So every case below pairs a member path with a container. + # ------------------------------------------------------------------------------------------ + stub = "lib/app/PerformanceMonitorLite_ExecutionStub.exe" + squirrel = "lib/app/Squirrel.exe" + app = "lib/app/PerformanceMonitorLite.exe" + nupkg = "PerformanceMonitorLite-3.7.2-lite-full.nupkg" + portable = "PerformanceMonitorLite-lite-Portable.zip" + setup = "PerformanceMonitorLite-lite-Setup.exe" + + def correct_release() -> dict[str, object]: + """What a release looks like once post-pack signing has run.""" + return { + nupkg: {app: True, stub: False, squirrel: False}, + portable: { + "PerformanceMonitorLite.exe": True, + "Update.exe": True, + "current/PerformanceMonitorLite.exe": True, + }, + setup: True, + } + + guard("a correct release", correct_release(), 0) + + # The allowance is the whole point, so it has to actually apply. + only_allowed = {nupkg: {app: True, stub: False, squirrel: False}} + guard("the two allowed nupkg members", only_allowed, 0) + guard("the allowed stub alone", {nupkg: {app: True, stub: False}}, 0) + guard("the allowed Squirrel.exe alone", {nupkg: {app: True, squirrel: False}}, 0) + + # And it has to stop applying the moment anything about the location changes. + broken = correct_release() + broken[setup] = False + guard("an unsigned Setup.exe beside allowed nupkg members", broken, 1) + + broken = correct_release() + broken[portable] = dict(broken[portable]) # type: ignore[arg-type] + broken[portable]["PerformanceMonitorLite.exe"] = False # type: ignore[index] + guard("an unsigned portable-root launcher", broken, 1) + + broken = correct_release() + broken[portable] = dict(broken[portable]) # type: ignore[arg-type] + broken[portable]["Update.exe"] = False # type: ignore[index] + guard("an unsigned portable Update.exe", broken, 1) + + guard( + "the allowed member paths inside a .zip rather than a .nupkg", + {portable.replace("Portable", "Other"): {stub: False, squirrel: False}}, + 1, + ) + guard("an unsigned standalone Squirrel.exe", {"Squirrel.exe": False}, 1) + guard( + "an unsigned standalone launcher stub", + {"PerformanceMonitorLite_ExecutionStub.exe": False}, + 1, + ) + guard( + "an unsigned nupkg member that is not on the allowlist", + {nupkg: {app: True, "lib/app/Update.exe": False}}, + 1, + ) + guard( + "an allowed name in a different directory inside the nupkg", + {nupkg: {app: True, "lib/other/Squirrel.exe": False}}, + 1, + ) + guard("an unsigned app payload inside the nupkg", {nupkg: {app: False}}, 1) + guard("a nupkg whose stub is signed and app is not", {nupkg: {app: False, stub: False}}, 1) + + # Finding nothing to check is a broken run, not a clean one. + guard("a directory with no executables", {}, 2) + + # The allowance predicate itself, at the boundary the container check defends. + expect( + "a standalone file was granted an allowance", + allowance_for("PerformanceMonitorLite-lite-Setup.exe", "") is None, + ) + expect( + "the stub inside a .nupkg was not allowed", + allowance_for(nupkg, stub) is not None, + ) + expect( + "the stub inside a .zip was allowed", + allowance_for(portable, stub) is None, + ) + expect( + "a backslash-separated nupkg member path was not normalised", + allowance_for(nupkg, squirrel.replace("/", "\\")) is not None, + ) + expect( + "a nested path under lib/app was allowed", + allowance_for(nupkg, "lib/app/sub/Squirrel.exe") is None, + ) + expect( + "a name that merely ends with the allowed name was allowed", + allowance_for(nupkg, "lib/app/NotSquirrel.exe") is None, + ) + expect( + "every allowance carries a reason", + all(a.reason.strip() for a in ALLOWED_UNSIGNED), + ) + expect( + f"the allowlist holds {len(ALLOWED_UNSIGNED)} entries rather than the two nupkg members", + len(ALLOWED_UNSIGNED) == 2, + ) + + # The published-release mode and the release guard have to agree, or the audit is red on + # every correct release and stops being read. + def as_findings(layout: dict[str, object]) -> Report: + out = Report(tag="synthetic") + for asset, content in layout.items(): + if isinstance(content, dict): + for member, signed in content.items(): + out.findings.append(Finding(asset, member, 1024, bool(signed))) + else: + out.findings.append(Finding(asset, "", 1024, bool(content))) + return out + + fixed = as_findings(correct_release()) + expect( + f"the published-release mode reports {len(fixed.unsigned)} unsigned on a correct release", + not fixed.unsigned, + ) + expect( + f"the published-release mode reports {len(fixed.allowed)} allowances on a correct release", + len(fixed.allowed) == 2, + ) + regressed = as_findings({**correct_release(), setup: False}) + expect( + "the published-release mode passed an unsigned Setup.exe", + [f.label for f in regressed.unsigned] == [setup], + ) + + for f in failures: + print(f"SELF-TEST FAIL: {f}", file=sys.stderr) + if failures: + return 1 + print(f"self-test: {checks} assertions passed") + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("tag", nargs="?", help="release tag, e.g. v1.28.0") + parser.add_argument("--self-test", action="store_true", dest="self_test") + parser.add_argument( + "--verify-dir", + metavar="DIR", + help="release guard: every .exe under DIR, including inside zips/nupkgs, must be signed", + ) + parser.add_argument("--repo", default="erikdarlingdata/PerformanceStudio") + parser.add_argument("--gh", default="gh", help="gh executable") + parser.add_argument( + "--include-dlls", + action="store_true", + help="also check .dll members inside archives (slower; the shipped defect was .exe only)", + ) + parser.add_argument("--json", action="store_true", dest="as_json") + args = parser.parse_args() + + if args.self_test: + return self_test() + if args.verify_dir: + return verify_local_dir(args.verify_dir) + if not args.tag: + parser.error("a release tag is required unless --self-test or --verify-dir is given") + + try: + report = build_report(args.tag, args.repo, args.gh, args.include_dlls) + except ReadError as exc: + print(f"FATAL: {exc}", file=sys.stderr) + return 2 + + if args.as_json: + print( + json.dumps( + { + "tag": report.tag, + "executables": [ + { + "asset": f.asset, + "path": f.path, + "size": f.size, + "signed": f.signed, + "allowed_unsigned": not f.signed + and allowance_for(f.asset, f.path) is not None, + } + for f in report.findings + ], + "unsigned_count": len(report.unsigned), + "allowed_unsigned_count": len(report.allowed), + "errors": report.errors, + }, + indent=2, + ) + ) + else: + print(f"{report.tag}: {len(report.findings)} executable(s) checked\n") + width = max((len(f.label) for f in report.findings), default=0) + allowed_labels = {f.label for f, _ in report.allowed} + for f in sorted(report.findings, key=lambda x: (x.signed, x.label)): + if f.signed: + mark = "signed " + elif f.label in allowed_labels: + mark = "allowed " + else: + mark = "UNSIGNED" + print(f" {mark} {f.label:<{width}} {f.size:>10,} bytes") + for f, allowance in report.allowed: + print(f"\nallowed unsigned: {f.label}\n {allowance.reason}") + for e in report.errors: + print(f"\n ERROR {e}", file=sys.stderr) + + if report.errors and not report.findings: + return 2 + if not report.findings: + print("\nFATAL: no executables found — a release with nothing to check is a broken run.", + file=sys.stderr) + return 2 + if report.unsigned: + print(f"\n{len(report.unsigned)} unsigned executable(s). See PerformanceMonitor #3288.", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 399827c8..8a54c28c 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -132,7 +132,11 @@ jobs: $hash = (Get-FileHash $_.FullName -Algorithm SHA256).Hash.ToLower() "$hash $($_.Name)" } - $checksums | Out-File -FilePath releases/SHA256SUMS.txt -Encoding utf8 + # LF line endings and no BOM. Out-File writes CRLF on Windows, and then + # `sha256sum -c` and `shasum -c` on Linux and macOS read the \r as part + # of each file name and report every file as missing. release.yml + # writes its SHA256SUMS.txt the same way; keep the two in step. + [IO.File]::WriteAllText("$PWD/releases/SHA256SUMS.txt", (($checksums -join "`n") + "`n"), [Text.UTF8Encoding]::new($false)) Write-Host "Checksums:" $checksums | ForEach-Object { Write-Host $_ } diff --git a/.github/workflows/release-signatures.yml b/.github/workflows/release-signatures.yml new file mode 100644 index 00000000..65c5c3c9 --- /dev/null +++ b/.github/workflows/release-signatures.yml @@ -0,0 +1,66 @@ +name: Release signatures + +# Every executable `vpk pack` GENERATES (Setup.exe, and the launcher and Update.exe in +# Portable.zip) shipped unsigned on every release through v1.28.0, while the app payload was +# signed. release.yml now signs them after packing. The post-pack signing lives in +# .github/scripts/postpack_signing.py and the reader that checks the result lives in +# .github/scripts/verify_release_signatures.py. Both come from PerformanceMonitor (its #3288). +# This workflow exists so neither can rot unnoticed, and so a published release can be audited +# on demand without a local checkout. +# +# Neither self-test needs SignPath, a release, or a Windows runner. That matters because the +# release path itself runs only when dev is merged into main, and it blocks on manual approvals, +# so these self-tests are the only part of it that a pull request can exercise. +# +# The release-time guard is not here. It is the "Assert every Velopack executable is signed" +# step in release.yml, which checks the files on disk before anything is published. This one +# answers "what did we actually publish", which needs the release to exist first. +on: + pull_request: + paths: + - '.github/scripts/verify_release_signatures.py' + - '.github/scripts/postpack_signing.py' + - '.github/workflows/release-signatures.yml' + branches: [dev, main] + workflow_dispatch: + inputs: + tag: + description: 'Release tag to audit (e.g. v1.29.0). Leave blank to self-test only.' + required: false + type: string + +concurrency: + group: release-signatures-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + signatures: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Covers the PE parse and the release guard's allowlist. The allowlist cases are the ones + # worth naming: the two allowed names also appear as the portable zip's root launcher and + # as Update.exe, so a skip written by filename alone would let an unsigned Setup.exe and an + # unsigned launcher through, which is the defect. Each case pairs a member path with a + # container, and a mutation broadening the match to the basename turns this step red. + - name: Self-test the signature reader + run: python3 .github/scripts/verify_release_signatures.py --self-test + + # Covers the staging rules and the portable-zip rebuild. The rebuild matters because the + # zip is a shipped artifact: the self-test asserts that a re-created archive, which loses + # entry order and the stored `.portable` marker, is refused. + - name: Self-test the post-pack signing script + run: python3 .github/scripts/postpack_signing.py --self-test + + # Manual only, because it requires a published release. It exits 1 for v1.28.0 and every + # earlier release, which shipped Setup.exe and the portable launcher unsigned. + - name: Audit a published release + if: github.event_name == 'workflow_dispatch' && inputs.tag != '' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TAG: ${{ inputs.tag }} + run: python3 .github/scripts/verify_release_signatures.py "$TAG" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 022caa7d..c4f8769c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -160,6 +160,84 @@ jobs: Remove-Item -Recurse -Force publish/win-x64 Copy-Item -Recurse signed/win-x64 publish/win-x64 + # ── Velopack (packs the signed Windows build) ───────────────────── + # Packing runs before the release is created, so the executables it + # generates are signed before anything is published. ── + - name: Create Velopack release (Windows) + shell: pwsh + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + VERSION: ${{ steps.version.outputs.VERSION }} + run: | + # Pin vpk to match the Velopack PackageReference (Velopack recommends the + # CLI and library versions match for compatible packages + reproducible releases). + dotnet tool install -g vpk --version 1.2.0 + New-Item -ItemType Directory -Force -Path releases/velopack + + # Download previous release for delta generation + vpk download github --repoUrl https://github.com/${{ github.repository }} --channel win -o releases/velopack --token $env:GH_TOKEN + + # Pack Windows release (now signed) + vpk pack -u PerformanceStudio -v $env:VERSION -p publish/win-x64 -e PlanViewer.App.exe -o releases/velopack --channel win + + # `vpk pack` generates three executables: Setup.exe, and the launcher + # (PerformanceStudio.exe) and Update.exe at the root of Portable.zip. They + # do not exist when the App is signed above, so they shipped unsigned + # through v1.28.0. The steps below sign them in one more SignPath request + # and write them back. PerformanceMonitor does the same (its #3288), and + # both scripts come from there. + # + # The launcher stub and Squirrel.exe inside the .nupkg stay unsigned. + # releases.win.json records the .nupkg's SHA256 and size, and the delta is + # built against those bytes, so the .nupkg must not change after packing. + # Nothing records a hash for Setup.exe or Portable.zip. + # + # Unlike the SSMS signing below, this is required, like the App signing: + # if a step fails or the approval times out, the job stops here, before + # the release is created. + - name: Collect the executables Velopack generates + shell: pwsh + run: python .github/scripts/postpack_signing.py collect --stage postpack/unsigned --manifest postpack/manifest.json --product studio=releases/velopack + + - name: Upload the Velopack-generated executables for signing + id: upload-postpack + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: Velopack-generated-unsigned + path: postpack/unsigned/ + + # Same organization, project, policy and timeout as the App request. The + # "SignTemplate" artifact configuration on signpath.io Authenticode-signs + # every .exe and .dll in the artifact, with min-matches="0". So an empty + # artifact would sign nothing and still succeed, and the collect step + # refuses to stage an empty set for that reason. + - name: Sign the executables Velopack generates + uses: signpath/github-action-submit-signing-request@f6d04783b4569d051e0c80105fe66e82819d0092 # v3.0 + with: + api-token: '${{ secrets.SIGNPATH_API_TOKEN }}' + organization-id: '7969f8b6-d946-4a74-9bac-a55856d8b8e0' + project-slug: 'PerformanceStudio' + signing-policy-slug: 'release-signing' + artifact-configuration-slug: 'SignTemplate' + github-artifact-id: '${{ steps.upload-postpack.outputs.artifact-id }}' + wait-for-completion: true + output-artifact-directory: 'postpack/signed' + wait-for-completion-timeout-in-seconds: 1800 + + # Setup.exe is overwritten. Portable.zip is rebuilt entry by entry: every + # entry that is not replaced keeps its original bytes, and the rebuilt zip + # is checked against the original before it replaces it. + - name: Write the signed Velopack executables back + shell: pwsh + run: python .github/scripts/postpack_signing.py apply --signed postpack/signed --manifest postpack/manifest.json + + # Fails the job if any .exe in the Velopack output has no signature, + # including the ones inside Portable.zip and the .nupkg files. The only + # exceptions are the two .nupkg members described above. + - name: Assert every Velopack executable is signed + shell: pwsh + run: python .github/scripts/verify_release_signatures.py --verify-dir releases/velopack + # ── SignPath code signing for the SSMS extension and its installer. # Unlike the App, this is best-effort. If a step below fails, or an # approval times out, the release still goes out with the unsigned @@ -167,8 +245,9 @@ jobs: # says so. The one exception is in the replace step: if it cannot put # the unsigned files back after a failed copy or a failed signature # check, the job stops before anything is published. Each SignPath - # request needs a manual approval, so a release run now waits for two: - # the App first, then these two files. ── + # request needs a manual approval, so a release run now waits for three: + # the App, then the executables Velopack generates, then these two + # files. ── - name: Stage SSMS files for signing if: steps.ssms.outputs.BUILT == 'true' continue-on-error: true @@ -296,24 +375,6 @@ jobs: curl -sS -f "https://ssmsgallery.azurewebsites.net/api/upload" \ -F "file=@releases/PlanViewer.Ssms.vsix" - # ── Velopack (uses signed Windows binaries) ─────────────────────── - - name: Create Velopack release (Windows) - shell: pwsh - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - VERSION: ${{ steps.version.outputs.VERSION }} - run: | - # Pin vpk to match the Velopack PackageReference (Velopack recommends the - # CLI and library versions match for compatible packages + reproducible releases). - dotnet tool install -g vpk --version 1.2.0 - New-Item -ItemType Directory -Force -Path releases/velopack - - # Download previous release for delta generation - vpk download github --repoUrl https://github.com/${{ github.repository }} --channel win -o releases/velopack --token $env:GH_TOKEN - - # Pack Windows release (now signed) - vpk pack -u PerformanceStudio -v $env:VERSION -p publish/win-x64 -e PlanViewer.App.exe -o releases/velopack --channel win - # ── Package and upload ──────────────────────────────────────────── - name: Package and upload shell: pwsh @@ -388,7 +449,11 @@ jobs: $hash = (Get-FileHash $_.FullName -Algorithm SHA256).Hash.ToLower() "$hash $($_.Name)" } - $checksums | Out-File -FilePath releases/SHA256SUMS.txt -Encoding utf8 + # LF line endings and no BOM. Out-File writes CRLF on Windows, and then + # `sha256sum -c` and `shasum -c` on Linux and macOS read the \r as part + # of each file name and report every file as missing. nightly.yml + # writes its SHA256SUMS.txt the same way; keep the two in step. + [IO.File]::WriteAllText("$PWD/releases/SHA256SUMS.txt", (($checksums -join "`n") + "`n"), [Text.UTF8Encoding]::new($false)) Write-Host "Checksums:" $checksums | ForEach-Object { Write-Host $_ }