arcup: download_file's error handling is silently defeated on curl 8.14.0/8.14.1

[batuhankocyigit]

Summary

Split off from review discussion on #304.

download_file in arcup/arcup relies on curl's exit code to detect
failed downloads:

if ! curl_with_headers -#fL --max-time 900 -o "$output_path" "$url"; then
    rm -f "$output_path"
    download_error "$tag" "$filename" "$url"
fi

curl_with_headers includes CURL_RETRY_ARGS=(--retry 3 --retry-delay 2 --connect-timeout 15). On curl 8.14.0 and 8.14.1, combining --retry with
--fail causes curl to exit 0 even on a 404 (upstream bug, see
curl/curl#17554 — fixed in later releases; --retry 0 or omitting
--retry exits 22 correctly on those versions).

Reproduced by running the arcup test suite's e2e test against main on a
box with curl 8.14.1: instead of download_error's intended message, the
script fails later with Checksum file is empty: … — a much less useful
error for whoever hits it.

Impact

The install still fails closed (checksum verification catches the empty/
missing file downstream), so this isn't a security issue — just a
misleading-error-message issue on a curl line version that ships in
current distros and nixpkgs.

Suggested fix

Add an explicit non-empty-file check after the curl call in download_file
(and download_file_with_github_api, which has the same pattern),
independent of curl's exit code:

if ! curl_with_headers -#fL --max-time 900 -o "$output_path" "$url" || [[ ! -s "$output_path" ]]; then
    rm -f "$output_path"
    download_error "$tag" "$filename" "$url"
fi

This immunizes the check against the curl version-specific behavior
rather than depending on the exit code alone.

[osr21]

Reproduced this on curl 8.14.1 and confirmed the diagnosis. Three things I found while verifying it that I think change the scope of the fix — the most important being that arcup/install does not fail closed.

Version boundaries, confirmed

The fix is commit b42776b (upstream PR curl/curl#17559). Checking containment against release tags: it is not in curl-8_14_1 (that tag is 18 commits behind the fix) and is in curl-8_15_0 (released 2025-07-16). So the affected set is exactly 8.14.0 and 8.14.1, fixed in 8.15.0 — matching the report.

Side-by-side on the same 404, same box:

curl	-fL	-fL --retry 3	-fL --retry 0
8.14.1 (nixpkgs)	22	0	22
8.5.0 (Ubuntu)	22	22	22

The trigger is narrower than "a 404" — and it's the worst possible subset

The upstream PR title is "fix return code when --retry is used but not triggered", and that's exactly what the behavior shows. Only non-retryable statuses lose the exit code:

HTTP	shell $?
400, 401, 403, 404	0 (wrong)
408, 429, 500, 503	22 (correct)

So transient failures — the ones --retry exists for — still report correctly, and it is precisely the permanent failures that go silent. Worth noting 403 is in that set: raw.githubusercontent.com serves 403 under rate limiting, so this is reachable without anyone publishing a bad URL.

One trap for anyone implementing a fix: -w '%{exitcode}' is clobbered too (reports 0 alongside the shell's 0). %{http_code} stays truthful (404). Any exit-code-independent check has to key on http_code, not exitcode.

arcup/install is the real bug — it silently installs a 0-byte arcup

The issue's impact assessment ("fails closed, checksum verification catches it downstream") holds for arcup itself. I walked all six CURL_RETRY_ARGS call sites, and every one in arcup happens to be backstopped by a content check that accidentally covers for the exit code:

Site	Backstop	Result on 8.14.1
download_file (327)	none	fails later at Checksum file is empty — the reported symptom
download_file_with_github_api (352)	[[ -f "$output_path" ]]	fails closed (curl creates no file under --fail, verified)
resolve_github_asset_download_url (135)	[[ -n "$location" ]]	fails closed (headers are still dumped — 1168 bytes incl. HTTP/2 404 — but a 404 carries no Location)
fetch_release_json / _latest (216–228)	empty-string checks	fails closed
update_arcup (441)	remote_version grep	fails closed as Failed to determine remote version; cp is after the grep, so no clobber

arcup/install line 75 is the exception, and it has no checksum step anywhere:

_INSTALL_TMP_FILE="$(mktemp 2>/dev/null || mktemp -t arcup)"
if ! curl_with_headers -#fL --max-time 300 -o "$_INSTALL_TMP_FILE" "$ARCUP_URL" ; then
    error "failed to download arcup from '$ARCUP_URL'"
fi
install_bin "$_INSTALL_TMP_FILE" "$BIN_DIR/arcup"

mktemp creates the file, so unlike download_file there is no "missing file" to trip over — the empty temp file is copied straight to $BIN_DIR/arcup and chmod +x'd. Running that fragment verbatim against a 404:

info: Downloading arcup script...
curl: (22) The requested URL returned error: 404      <- message printed, exit code 0
info: Arcup installed to /tmp/.../bin/arcup
-rwx--x--x 1 runner runner 0 ... arcup                <- 0 bytes
$ arcup --version                                     <- empty executable
exit=0

main() then runs "$BIN_DIR/arcup", an empty script that exits 0, and the installer prints its full success epilogue. Net effect on 8.14.x: the one-liner install reports success, installs nothing, and returns 0 — not a misleading message, a silent success. That's also the path most likely to meet an affected curl, since it runs on whatever the user's distro ships before arcup exists.

The test suite is structurally unable to catch this

test_download_error_lists_assets simulates failure with curl_with_headers() { return 22; }, and the fakebin/curl stubs follow the same convention. The tests assert against the exact exit-code contract that real 8.14.x violates, so they will stay green forever. A regression test has to stub return 0 while leaving the output path empty or absent — that's the shape the bug actually produces.

On the fix

The proposed || [[ ! -s "$output_path" ]] is right for download_file, and I'd extend it:

• download_file_with_github_api: upgrade the existing [[ -f ]] to [[ -s ]] — one character, closes the stale-file gap too.
• arcup/install: same [[ ! -s ]] guard. This is the one that changes behavior from "silently installs nothing" to an actual error.
• Since install carries its own copy of CURL_RETRY_ARGS, a per-call-site fix has to be applied in both files.

Worth considering alongside it, as a single central mitigation:

case "$(curl --version | awk 'NR==1{print $2}')" in
    8.14.0|8.14.1) CURL_RETRY_ARGS=(--retry-delay 2 --connect-timeout 15) ;;
esac

That trades retries for correct exit codes on exactly the two broken versions and needs no per-site changes. Belt-and-braces would be both: the version gate restores the contract, and the -s guards are version-independent and cost nothing.