fix(arcup): reject empty bootstrap downloads

[huklaa]

Fixes #309.
Prevents the bootstrap installer from treating an empty download as successful when curl incorrectly returns exit code 0.
The installer now verifies that the downloaded temporary file is non-empty before installing it.
Adds a regression test reproducing the false-success behavior.
This is intentionally scoped to the bootstrap silent-success path.

[osr21]

Reviewed this end to end and reproduced the underlying curl behaviour locally rather than taking #309 at face value. The fix is correct, the predicate is the right one, and the regression test is genuine. Details and a few follow-on observations below.

The mechanism reproduces exactly

Against the real endpoint shape (raw.githubusercontent.com, missing path), using the script's own CURL_RETRY_ARGS:

curl	exit code	temp file
8.14.1 (affected)	0	0 bytes
8.5.0 (unaffected)	22	not written

The detail that matters for this PR: because main pre-creates the temp file with mktemp before curl runs, the failure mode leaves a file that exists but is zero bytes. So [[ ! -s ... ]] is exactly the right test — -e or -f would not have caught it — and I confirmed -s fires against the real 8.14.1 result, not just against the stub.

Parsing is right too: ! cmd || [[ ... ]] groups as (! cmd) || ([[ ... ]]) in bash, so both conditions reach error as intended.

The regression test genuinely fails without the fix

Reverting only the arcup/install line and re-running the new test:

FAIL: empty bootstrap download was accepted
Installing arcup...
info: Arcup installed to /tmp/.../arc/bin/arcup
info: Running arcup to install arc-node binaries...
info: Arc binaries are in your PATH for this session.

(exit 1)

That output is worth pasting into the PR description, because it is the actual user-visible bug: a zero-byte file gets chmod +x'd, installed as arcup, executed, and the installer then reports complete success.

The scoping claim holds up — I checked the other paths

"Intentionally scoped to the bootstrap silent-success path" is the right call, and it is defensible for a reason worth recording in the PR body: the other curl_with_headers callers in arcup/arcup share the identical vulnerable idiom, but all of them fail safe downstream.

• Binary download (download_file) — verify_checksum_file plus validate_archive_contents (tar -tzf) reject an empty archive, and an empty .sha256 hits the explicit "Checksum file is empty" guard.
• Self-update (update_arcup) — an empty file yields no ARCUP_INSTALLER_VERSION match, so it stops at "Failed to determine remote version".

Bootstrap is the only path that installs and executes an unverified payload with no downstream integrity check, which is precisely why it is the only one that silently succeeds. A sentence to that effect would stop the scope decision reading as arbitrary.

One place I would still change, same idiom

download_file_with_github_api already tries to do what this PR adds, but one character off:

if curl "${CURL_RETRY_ARGS[@]}" -fL --max-time 900 -o "$output_path" "$download_url" >/dev/null 2>&1; then
    [[ -f "$output_path" ]] && return 0
fi

-f means "exists", and under this same curl bug the file exists at zero bytes, so this returns success on a failed download. The checksum step catches it afterwards, so it is not a correctness hole — but that guard is clearly intended as the non-empty check, and -s makes it one. Same fix, same spirit, and it turns a confusing downstream checksum mismatch back into a clear download error.

The test currently never runs

No workflow references arcup at all:

$ grep -rln arcup .github/workflows/
(no matches)

ci.yml is Rust, contracts and proto jobs only. This is pre-existing — arcup/test_arcup.sh has the same problem and predates this PR — so it is not a fault of the change. But it does mean a regression test that cannot currently catch a regression. Given the PR exists specifically to prevent a silent recurrence, a small shell-tests job running both scripts would give it teeth.

Related and concrete: test_install.sh lands as mode 100644, while its sibling test_arcup.sh is 100755. It has a shebang but is not executable, so ./arcup/test_install.sh fails and only bash arcup/test_install.sh works — which will bite whoever wires up that CI job. Worth a chmod +x before merge.

Optional hardening

-s closes the empty case. The residual gap is a non-empty wrong payload: -f suppresses HTTP error bodies so a plain 404 stays empty, but a captive portal or intercepting proxy answering 200 with HTML would pass -s, then get chmod +x'd and executed. Since the bootstrap path has no checksum by design, a one-line sanity check closes that cheaply:

head -n1 "$_INSTALL_TMP_FILE" | grep -q '^#!' || error "downloaded arcup is not a script"

Entirely optional, and arguably out of scope for a #309 fix.

---

Very minor nit: install installs an _install_cleanup EXIT trap at source time and the test then sets its own EXIT trap, so between that replacement and bash's handling of traps across the (main) subshell boundary, the mktemp file created inside main is not cleaned up by either. Harmless, just a stray temp file.

Net: correct fix, real test, sensible scope. The -f to -s change and the chmod +x are the two I would want before merge.