Skip to content

fix(security): verify Windows updater asset digests - #1792

Merged
hckrman101 merged 9 commits into
NotRexed:mainfrom
monsterlag:fix/verify-updater-assets
Nov 10, 2026
Merged

hckrman101 merged 9 commits into
NotRexed:mainfrom
monsterlag:fix/verify-updater-assets

Conversation

@monsterlag

@monsterlag monsterlag commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

The Windows updater currently checks download length but can launch a same-length altered setup or extract a same-length altered ZIP. This verifies the selected release asset against GitHub's SHA-256 digest before either handoff.

The updater rejects missing, malformed and unsupported digests before requesting the payload, hashes streamed bytes on both the browser and API routes, and promotes a unique owned partial file only after the length and digest checks pass. Digest failures are terminal; cleanup preserves the previous cached file. Only the chosen asset requires a valid digest, so an unused legacy ZIP does not block a valid setup.

Related to #1679; signing, setup-list authentication and release-tooling lockfiles remain separate work. GitHub's digest checks bytes against GitHub's release metadata and does not authenticate a compromised publishing account.

Validation:

  • The entire actual Updater.cs is compiled in a game-free fixture with authored HTTP responses, process/UI spies and temporary directories. No executable is launched or downloaded.
  • The identical 20-case fixture against the original updater produces 11 expected integrity failures and passes 9 controls. The patched updater passes all 20 cases, covering setup and ZIP routes, both transports, invalid digests, chunked responses, preserved cache, partial cleanup, HTTP errors and cancellation.
  • Strict selected source checks pass with no failures or skips, including the existing Builder update-decision test after merging current main.
  • The digest check declares RUN_ALL_ARGS in its own file to match upstream's runner format. The runner is unchanged from main; the strict selection runs all 20 cases and passes without skips.
  • The actual Windows WPF Builder compiles offline in Release with .NET SDK 8.0.427: zero warnings and errors. This is framework-dependent compilation (SelfContained=false, UseAppHost=false), not a publish/package test.

Local gaps: the full source suite, Linux execution, self-contained publishing, a real self-update or the complete delayed-retry schedule. The full suite was not run locally because an existing owner-imposed stop excludes a shader fixture; no assertions or skip policy were changed. Hosted Windows/Linux source suites and Windows Builder packaging now pass. No installed player, game files, saves or runtime settings are changed by this PR.

Current head 848175ce normally merges main at 117eae79, including #1858's presentation definitions and #1793's shared source-check repairs. The updater, its regression fixtures and the source-check runner are unchanged by those merges. All eight hosted checks pass on this head: both OS source suites, Windows Builder packaging, Windows Vulkan, Linux AppImage, both native lowering checks and workflow lint. Release-upload jobs are correctly skipped for a pull request. No real self-update was executed.

monsterlag and others added 5 commits November 10, 2026 11:51
Record the narrow digest-verification work for NotRexed#1679. No updater code
or signing policy changes in this coordination commit.

Assisted-by: OpenAI Codex
Co-Authored-By: Codex <noreply@openai.com>
Validate the selected release asset's SHA-256 before downloading, hash
the streamed bytes on both download routes, and promote only verified
owned partial files. Reject absent/malformed digests and mismatches before
launch or extraction, preserving the previous cached file on failure.

Exercise the entire updater against authored HTTP, UI and process spies:
the original accepts corrupt payloads, while all 20 fixed cases pass.
Compile the actual Windows WPF Builder offline (no publish).

Refs: NotRexed#1679
Assisted-by: OpenAI Codex
Co-Authored-By: Codex <noreply@openai.com>
Assisted-by: OpenAI Codex

Co-Authored-By: Codex <noreply@openai.com>
@hckrman101

Copy link
Copy Markdown
Collaborator

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

monsterlag and others added 4 commits November 10, 2026 12:58
Keep the upstream runner migration intact and declare the digest test SDK arguments in its own file. No test skip or assertion change in the updater regression.

Assisted-by: OpenAI Codex

Co-Authored-By: Codex <noreply@openai.com>
# Conflicts:
#	docs/worklist.md
@monsterlag
monsterlag marked this pull request as ready for review November 10, 2026 19:30

@hckrman101 hckrman101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and the conflict I flagged earlier is gone (merge state is clean, all hosted checks pass).

Checked the fail-closed paths in Updater.cs:

  • ParseDigest throws on a missing, non-string, non-sha256:, wrong-length or non-hex digest, and it runs at the top of DownloadWithRetries, before any payload request. InvalidDataException is not HttpRequestException, so IsTransient is false and neither the retry loop nor the API-URL fallback swallows it. Both callers end up in DownloadFailed with nothing launched or extracted.
  • A mismatch is a FixedTimeEquals over the streamed bytes, checked after the length check and before File.Move, so the final path only ever holds a verified file. The hash is over exactly the bytes written, on both the browser and API routes.
  • The partial is CreateNew with a GUID name, owned is set only once the stream is open, and the finally deletes only that file, so a previously cached good file is never clobbered or deleted on failure.
  • Only the selected asset is checked (setup when the setup path is taken, otherwise the ZIP), so a legacy ZIP with no digest doesn't block a valid setup.

On where the expected digest comes from: it's the digest field of the same releases/latest API response over HTTPS, so it stops a same-length swapped or corrupted asset but not a compromised publisher account or API response. The PR description says exactly that and leaves signing to separate work, which seems the right scope.

Nit, no change needed: a hard kill mid-download leaves a .part-<guid> file in the update cache, since nothing sweeps old ones. It's small and the cache is per-user, so fine to leave.

@hckrman101
hckrman101 merged commit 6d5f277 into NotRexed:main Nov 10, 2026
10 checks passed
hckrman101 pushed a commit that referenced this pull request Nov 12, 2026
The worklist was behind the forum and the merges. Since v2.1.0, many fix
PRs have merged, several rows already shipped, and the most-reported
problems in Discord #bug-reporting had no row. This PR brings
`docs/worklist.md` back in line, following the rules in the worklist and
in `docs/workflow.md`.

## Method
- **Forum snapshot.** All 409 #bug-reporting posts as of Nov 11, ~11:10
PM ET (Nov 7-11). Each post is dated by its thread ID, so the submit
time is in ET.
- **How dates were used.**
- v2.1.0 was cut from `main` at about 8:32 PM ET on Nov 10 and published
at 9:29 PM ET. A PR merged before the cut counts as shipped; I checked
this with `git merge-base --is-ancestor <merge commit> v2.1.0`.
- A report sent before a fix shipped and not repeated after it counts as
fixed.
- A report sent after v2.1.0, or before a fix that merged later, counts
as still open or waiting for a release.
- **Score.** Severity x reach x confidence.
- Reach comes from workflow.md's 2026-11-10 reporter shares. Where the
affected subset is unknown, it is marked as an upper bound.
- Confidence is 0.7 only where the GitHub tracker has logs from several
commenters; otherwise it is 0.4.
- **Owners.** New rows have no owner. Existing owners are kept.

## Changes
- **Added 20 rows (18 Open, 2 Fixed-waiting).**
- Open: RAM/VRAM fill-up to OOM (#993, 5.6; 13 threads after v2.1.0),
shader preparation stalls (3.2), AMD Windows crashes (#996, 2.9),
CPU-bound fps in Castle Town and Hyrule Field (#995), invisible objects
and doors (#1002), build.py stopped on v2.1.0, cutscene audio desync,
shadow artifacts, coloured lighting, HDR black screen and flicker, frame
generation stutter, the block grid around lights and fog, the v2.1.0
update failing, AF outlines, wrong button glyphs, Steam Deck at 6 fps,
Linux AMD slower than Windows, and the FSR 4 tick not rebuilding.
- Fixed-waiting: the pick-up counter above 60 fps (#2006) and RADV
device lost after the intro (#2362).
- **Updated 6 rows.**
- The Link-jump row is now the "above a 60 fps cap" row (#1742),
covering the Scarecrow's Song softlock, Gohma and the item hanging above
Link's head. It goes from S2 to S1 and is re-scored from 1.2 to 3.5.
- New forum links or scores on the antivirus, Intel black screen (0.11
to 0.2, #999 has logs), fairy grotto, voices (#2506) and RX 580 (#2343)
rows.
- **Moved 10 rows to Fixed, waiting for a release.** #2545 (ground
garbling), #2451 (RTX 30 crash), #2286, #2275, #2449, #2096, #2406,
#2515, #1954 and #2367.
- **Deleted 8 rows** whose fix shipped in v2.1.0: #1792, #1922, #1819,
#1863, #1447, #1451, #1769-#1810 and the HDR10 overlay.

## Not done
- PRs listed as "candidate" fixes are matched by title and description
only. None is confirmed against the reports.
- Long forum threads were judged from their summaries.
- `tests/run_all.py` was not run (docs-only change).

Co-authored-by: Joey Magnus <joey.magnus@cxalloy.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants