Skip to content

fix(arcup): accept checksum files without a trailing newline - #262

Open
devorun wants to merge 1 commit into
circlefin:mainfrom
devorun:fix/arcup-checksum-no-trailing-newline
Open

fix(arcup): accept checksum files without a trailing newline#262
devorun wants to merge 1 commit into
circlefin:mainfrom
devorun:fix/arcup-checksum-no-trailing-newline

Conversation

@devorun

@devorun devorun commented Aug 13, 2026

Copy link
Copy Markdown

Summary

arcup's verify_checksum_file rejects a valid .sha256 file whose final line has no trailing newline, aborting an otherwise-successful install with error: Checksum file is empty.

Root cause

Emptiness was inferred from read's exit status:

if ! read -r expected_checksum expected_name < "$checksum_path"; then
    error "Checksum file is empty: $checksum_path"
fi

read returns non-zero when it reaches EOF without seeing the line delimiter — i.e. when the last line has no trailing newline — even though it has already populated the variables. So a one-line checksum file with no trailing \n (valid, and what some tooling emits) wrongly takes the "empty" branch.

Reproduction

Sourcing the script with ARCUP_SKIP_MAIN=1 and calling verify_checksum_file against a valid checksum file written without a trailing newline:

printf '%s  %s' "$(compute_sha256 "$archive")" "$(basename "$archive")" > cksum   # no trailing \n
verify_checksum_file "$archive" cksum "$(basename "$archive")"
# -> error: Checksum file is empty

Fix

Check the parsed hash rather than read's exit status:

read -r expected_checksum expected_name < "$checksum_path" || true
if [[ -z "$expected_checksum" ]]; then
    error "Checksum file is empty: $checksum_path"
fi

A genuinely empty file still errors (the variable is empty), and the existing ^[0-9A-Fa-f]{64}$ check still guards malformed content.

Tests

Adds checksum file without trailing newline passes to test_arcup.sh — the existing checksum tests only wrote newline-terminated files, so this path was uncovered. The rest of the suite is unchanged and passes. The installer version is bumped per the in-file convention.

verify_checksum_file used `if ! read -r ... < file` to detect an empty
checksum file, but `read` returns non-zero when the final line has no
trailing newline even though it still populates the variables. A valid
`.sha256` file whose last line lacks a trailing newline was therefore
rejected as "Checksum file is empty", aborting an otherwise-good install.

Check the parsed hash instead of read's exit status, and add a regression
test for the no-trailing-newline case. Bump the installer version per the
in-file convention.
@osr21

osr21 commented Aug 13, 2026

Copy link
Copy Markdown

I executed your reproduction before commenting — the bug is real and your diagnosis of read's EOF semantics is exactly right:

$ printf 'abc  file' > ck; if ! read -r a b < ck; then echo "read FAILED, a=[$a] b=[$b]"; fi
read FAILED, a=[abc] b=[file]

Non-zero exit, variables fully populated — the classic trap.

That said, this is a duplicate of #243 (opened 2026-08-08, five days before this PR), down to fine detail: same title, same || true + [[ -z "$expected_checksum" ]] fix at the same site, near-identical explanatory comment, and the same first test case (printf '%s %s' without \n, same pass message). #243 additionally covers the two adjacent regressions this change could mask — a genuinely empty file must still fail, and a wrong hash without a trailing newline must fail the comparison rather than slip past the emptiness check — so it's the broader-coverage variant of the same fix. Per the first-come convention that's been applied consistently in this repo (see #255, #256), I'd recommend closing this in favor of #243.

Credit where due: you caught something #243 missed. The in-file convention says to bump ARCUP_INSTALLER_VERSION on any modification, and your 0.2.0 → 0.2.1 bump honored it while my branch didn't. I've now pushed that bump to #243 (363ded4) with the full suite green (28/28), and I'm noting here that the bump originated from your PR.

If you're looking for open ground in the same area: the arcup shellcheck CI gap (#247) and installer self-update verification (#223) threads both have adjacent unclaimed work, and #59's forwarder investigation could use more operators running the idle-gap repro described in PR #261. Happy to point you at specifics if any of those interest you.

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