fix: monitoring.md permissions note and arcup checksum filename validation

[Sertug17]

Summary

Two independent correctness fixes in a single PR.

1. docs/monitoring.md troubleshooting section contradicts user: "0" (#315)

The "Prometheus is restarting with a permissions error" section instructs operators to chown -R 65534:65534 the data directory. However, the compose.yaml in the same guide sets user: "0" on the Prometheus service (root), so the described UID 65534 write-permission error cannot occur when following the guide as written. Updated the section to clarify that user: "0" prevents this error and that the chown step only applies when Prometheus runs as a non-root user.

2. arcup/arcup verify_checksum_file silently skips filename check for bare-hash inputs (#316)

When a .sha256 file contains only a bare hash (no filename field), expected_name is empty and the if [[ -n "$expected_name" ]] guard skips filename validation entirely. The archive is accepted on hash match alone, silently violating the function's contract of verifying the checksum was intended for $archive_name. Added an elif [[ -n "$archive_name" ]] branch that errors explicitly in this case.

Test plan

• Docs-only change for monitoring.md
• verify_checksum_file with a bare-hash .sha256 now returns an error instead of silently passing

Fixes #315, #316

[osr21]

Both fixes do what they claim, and the existing suite still passes at ffc1bf3 (15/15 ok). I verified the behaviour change directly rather than by reading, and have one docs regression plus a two-line test gap I'd want addressed before merge.

The arcup change works, verified before/after

Sourcing arcup with ARCUP_SKIP_MAIN=1 and calling verify_checksum_file against each input shape, on main vs this branch:

.sha256 content	main	PR #317
hash name\n (valid)	accepted	accepted
hash *name\n (binary mode)	accepted	accepted
hash\n (target of this fix)	accepted	rejected, clear message
hash name (valid, no trailing newline)	rejected: "Checksum file is empty"	unchanged
hash name\r\n (CRLF)	rejected, mangled message	unchanged

So the intended case is fixed, the *name binary-mode strip still works, and nothing regressed.

One note on the guard: verify_checksum_file has a single caller (arcup:785) which always passes a non-empty $ARCHIVE_NAME, so elif [[ -n "$archive_name" ]] is effectively unconditional. That's fine as defensive style, but it does mean the new rejection applies to every bare-hash input in practice — a real behaviour change for anyone installing from a fork or self-hosted release via ARC_REPO, where hand-rolled .sha256 files are common. Worth a release-note line; the PR body currently reads as a pure bug fix.

Two-line test gap

arcup/test_arcup.sh:108 already has test_checksum_validation covering this exact function, with a passing case and a filename-mismatch case. The test plan says the bare-hash behaviour was verified, but no case was added, so the regression isn't locked in. It fits the existing harness in two lines:

printf '%s\n' "$checksum" > "$checksum_file"
expect_fail "checksum file without filename fails" verify_checksum_file "$archive" "$checksum_file" "$archive_name"

(Worth knowing: nothing in .github/workflows/ invokes test_arcup.sh, so it only runs locally today. That's pre-existing, not this PR's problem — I raised it on #314 — but it does mean the added case is only as good as whoever remembers to run it.)

The two bugs sitting in the six lines being edited

I raised these on #316; repeating them here only because this PR already touches this function, so they're cheap to fold in. Both reproduce on this branch:

A valid checksum file with no trailing newline is reported as empty. read returns nonzero at EOF-without-newline even though it populated both variables correctly, and the check keys off read's exit status rather than the content:

error: Checksum file is empty: /tmp/.../nonl

The file is not empty. This sends an operator hunting a truncated download that never happened.

CRLF corrupts the error message itself. The trailing CR stays in $expected_name, so the comparison fails and the CR then rewinds the cursor mid-message. Observed output:

error: Checksum file is for 'arc-node-v0.8', expected 'arc-node-v0.8.0-x86_64-unknown-linux-gnu.tar.gz'

The name is not truncated — the CR is overwriting the line as it renders, so the diagnostic actively lies about what it compared.

Normalising the line first fixes both, and makes this PR's elif the natural last piece:

local line
line=$(tr -d '\r' < "$checksum_path" | head -n1)
if [[ -z "$line" ]]; then
    error "Checksum file is empty: $checksum_path"
fi
read -r expected_checksum expected_name <<< "$line"

Entirely reasonable to decline as out of scope — but these fail closed with misleading diagnostics on valid input, where the bug being fixed here fails open silently, so they're arguably the higher-value halves of the same six lines.

Docs: this drops the searchable symptom string

This is the one I'd ask to change before merge. The removed sentence was the only occurrence of queries.active in the repo:

main:      docs/monitoring.md:279  "...an error about `queries.active` or write permission"
PR #317:   (zero occurrences)

That literal string is what an operator pastes into search after reading it in the Prometheus logs. The section heading says "a permissions error", which won't match. The clarification and the symptom text aren't in conflict — keeping both costs one sentence.

Docs: "cannot occur" is now asserted rather than implied

The new text states the error cannot occur when following the guide exactly. As I noted on #315, there are two configurations where it occurs anyway with user: "0", and in neither does the documented chown 65534 help:

• --userns-remap — container root maps to a subordinate host UID with no write access to the operator-owned directory.
• SELinux (RHEL/Fedora/CentOS) — the bind mount carries no :z/:Z label, so access is denied regardless of UID.

Previously the guide was merely silent on this. Now it tells an operator staring at a genuine permissions error that their error cannot exist, which is a worse failure mode than the original ambiguity. Softening to something like "this is usually prevented by user: \"0\" above" plus a line naming those two exceptions would keep the fix and lose the overclaim.

I won't re-litigate the direction here — I argued on #315 for dropping user: "0" and moving the chown beside the mkdir at monitoring.md:69 rather than documenting root-run, but that's a maintainer call and this PR does resolve the stated contradiction either way.

Minor process point

These are two unrelated changes with quite different risk profiles — a docs clarification, and a behaviour change to install-time verification that can break fork installs. Fixes #315, #316 means one merge closes both. Splitting would let the docs fix land immediately while the arcup change gets its test case and release note, and would make either one revertible on its own.