fix(linux): verify the FFmpeg download against a pinned checksum - #530
Merged
Conversation
thcp
force-pushed
the
fix/516-drain-child-pipes
branch
from
August 31, 2026 21:10
e24e0a7 to
845b5ac
Compare
thcp
marked this pull request as ready for review
August 31, 2026 21:13
download_linux_ffmpeg fetched a tarball from a rolling URL on a single host, extracted it with the system tar, marked the binaries executable and ran them, with no integrity check of any kind. Windows verifies against BtbN's published checksums.sha256 and macOS against four pinned hashes; Linux verified nothing. verify_ffmpeg only proves the binary runs and has the encoders we need, which says nothing about where it came from. Pinned rather than verified against upstream's .md5 companion, which was the obvious move but is worth less than it looks: MD5 is broken for collisions, and the companion is served by the same host as the tarball, so anyone able to replace one can replace the other. It evidences corruption, not authenticity. The pinned hash was computed from the artifact whose MD5 matched upstream's published 7fa72b652e19bf84c9461e332ea1cdf3, so this pin is anchored to what upstream currently vouches for. The URL is a rolling one, so this needs a manual bump when upstream publishes a new build; the current one is dated 2024-08-24. A stale pin fails closed with a checksum error rather than silently accepting whatever arrives. STEMDECK_FFMPEG_URL still overrides, and skips the check -- an override points somewhere we cannot have a hash for, so vouching for it is the caller's business, matching how the macOS override already behaves. Note this code cannot be compiled on macOS or by ci.yml, which does not build the Rust shell at all. It was type-checked by temporarily widening its cfg gate to #[cfg(unix)]; cargo check reported no errors. Refs #518
thcp
force-pushed
the
fix/518-linux-ffmpeg-checksum
branch
from
August 31, 2026 21:14
474b483 to
1ab8c74
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #518. Targets
fix/516-drain-child-pipes-- last of the sequential Rust changes. Merge #529 first.The problem
download_linux_ffmpegfetched a tarball from a rolling URL on a single non-CDN host, extracted it with the systemtar, marked the binaries executable and ran them -- with no integrity check of any kind.checksums.sha256verify_ffmpegonly proves the binary runs and has the encoders StemDeck needs. That says nothing about where it came from.Why a pinned SHA256 rather than upstream's .md5
The obvious move was the
.md5companion, matching the Windows shape. I checked it and it is worth less than it looks:.sha256(confirmed: 404)So this pins our own SHA256, the way the macOS path already does. The pinned value was computed from the artifact whose MD5 matched upstream's published
7fa72b652e19bf84c9461e332ea1cdf3, so the pin is anchored to what upstream currently vouches for.Trade-off: the URL is rolling, so this needs a manual bump when upstream publishes a new build. A stale pin fails closed with a checksum error rather than silently accepting whatever arrives, which is the right direction to fail.
STEMDECK_FFMPEG_URLstill overrides and skips the check -- an override points somewhere we cannot have a hash for, matching how the macOS override already behaves.Verification -- and a gap reviewers should know about
This code cannot be compiled on macOS, and
ci.ymldoes not build the Rust shell at all -- Linux Rust is compiled only bylinux-release.yml, at release time. So a type error here would first surface during a release.To check it locally I temporarily widened the cfg gate on the two constants and
download_linux_ffmpegfrom#[cfg(all(unix, not(target_os = "macos")))]to#[cfg(unix)]and rancargo check:Then restored all 7 gates (verified by count).
Worth filing separately
There is no
linux-check.yml. #421 addedmacos-check.ymlandwindows-check.ymlprecisely becauseci.ymlis 100% ubuntu and could not compile cfg-gated code for those platforms -- but nothing compiles the Linux Rust either, despite the runner being ubuntu. That is a real hole and it is what made this PR awkward to verify. Happy to open an issue.