fix(arcup): accept a valid checksum written in uppercase hex - #302
fix(arcup): accept a valid checksum written in uppercase hex#302Bornoz wants to merge 2 commits into
Conversation
verify_checksum_file accepts a checksum whose hash matches ^[0-9A-Fa-f]{64}$ —
either case — but then compares it against compute_sha256, which always returns
lowercase (sha256sum and shasum both do). The comparison is case-sensitive, so a
correct digest written in uppercase fails as "Checksum verification failed" and
the install is refused for a genuine release.
Fold the expected checksum to lowercase before comparing, the same normalization
detect_platform already applies to uname output. Adds an uppercase case to
test_checksum_validation, which fails on main and passes with the fix.
|
Verified this independently at Confirmed on
Severity, honestly framed: I pulled a real The gap: no |
The script header requires incrementing ARCUP_INSTALLER_VERSION for any modification to arcup, and the self-update check compares it against the copy on main to tell a user their installer is stale. This change edits arcup, so it owes the bump. Test suite stays at 26/26: the self-update fixture serves 0.2.0 as the remote version, so a local 0.2.1 is simply not older and nothing is advertised as an update.
|
Thanks for the merge test, that is more than I checked myself. Bump added in 3a07773: On renumbering: #300, #243 and #262 all claim 0.2.1, so whichever lands first takes it and the rest have to move. I have no stake in the ordering, so treat 0.2.1 here as a placeholder and I will rebase onto whatever number is free when this is ready to land. Suite is still 26/26, exit 0, on Linux with GNU coreutils. The self-update fixture serves 0.2.0 as the remote version, so a local 0.2.1 reads as not-older and nothing is advertised as an update. Agreed on the severity framing too. Every |
|
Re-ran the merge test at the new head ( All six pairwise merges are clean, in both orders. Tested at current heads (#300 Adding the bump did not cost this branch the conflict-free property. I claimed the four PRs "will conflict pairwise on that line" — they do not, because all four set the identical value The real hazard is the opposite of the one I described: silence, not conflict. Merging #243 + #302 produces a tree whose header reads That has a concrete consequence in the self-update path. Both So a user holding the first-landed That makes your "treat 0.2.1 as a placeholder and rebase onto whatever is free" plan the right call, with one caveat worth flagging to whoever lands these: nothing will remind them. No conflict fires, no test asserts the bump (the suite exercises Your other claims check out at I'm an external community contributor, not affiliated with Circle, with no write access to this repository. Advisory only. |
verify_checksum_fileaccepts a checksum whose hash matches^[0-9A-Fa-f]{64}$— either case — but then compares it againstcompute_sha256, which always returns lowercase (sha256sumandshasumboth do). The comparison is case-sensitive, so a correct digest written in uppercase fails asChecksum verification failedand the install is refused for a genuine release.Fold the expected checksum to lowercase before comparing, the same normalization
detect_platformalready applies tounameoutput. Adds an uppercase case totest_checksum_validation; it fails onmainand passes with the fix.shellcheck arcup/arcupis clean and the suite is 26/26.This is a separate defect from the open trailing-newline fixes (#243, #262) and the self-update verification work (#204/#223) — same function, different failure. It applies after the format check and touches only the comparison, so it composes with those rather than competing.