forked from heavy-duty/rig
fix: the --version read cannot die ahead of its own refusal
Self-review of the whole head after the panel converged. One real defect,
reproduced verbatim before it was touched.
runner_version_of is a pipeline, and every forgejo-runner-* command runs under
set -euo pipefail. When the binary exits non-zero the pipeline fails, and
`PRESENT_VER="$(runner_version_of "$BIN")"` is an ASSIGNMENT — so install died
at that line, exit 1, printing nothing at all. The lib documented the opposite
("empty when it cannot answer"), and the refusal written for exactly this case
twenty lines later — "the download landed but cannot run" — was unreachable
code that could never have fired.
The shape is not hypothetical. `[ -x ]` yes, runnable no is what a truncated or
wrong-arch download leaves behind, and a ci-box's template preinstalls the
binary at mint — so the silent death sat on the one path this command family
exists to serve.
json_field, two files away, already carries `|| true` with this reasoning
written above it: "callers run under set -e with pipefail, where a grep that
matches nothing would otherwise kill the script with no message". The same
guard, for the same reason, at both sites: the lib reader and install's own
read of --version.
The test could not have caught it. vparse drove the lib through `bash -c` with
NO shell options, and its "bad" stub exits 0 — it proved garbage-in-empty-out
in a permissive shell, which is not where the function is used. vparse now runs
under set -euo pipefail, and a stub that exits non-zero is a third row.
Mutation-checked: that row exits 1 against the unpatched lib and 0 against the
fixed one. Install's site is grep-pinned, since reaching it for real needs root
and a downloaded binary.
748 passed (was 746), release 31, drill 43, shellcheck clean.
This commit is contained in:
parent
6c3b08117f
commit
ab29ee78fb
3 changed files with 33 additions and 3 deletions
|
|
@ -347,7 +347,10 @@ if [ "$NEED_DOWNLOAD" -eq 1 ]; then
|
|||
mv -f "$BIN.rig-new" "$BIN"
|
||||
log "installed ${BIN}"
|
||||
fi
|
||||
INSTALLED_VER="$("$BIN" --version 2>/dev/null | head -n1)"
|
||||
# `|| true` so the refusal BELOW is the one that fires. Under set -euo pipefail
|
||||
# a bare `VAR="$(cmd | head)"` dies at the assignment when cmd exits non-zero,
|
||||
# which is precisely the case this line exists to diagnose — see the lib.
|
||||
INSTALLED_VER="$("$BIN" --version 2>/dev/null | head -n1 || true)"
|
||||
[ -n "$INSTALLED_VER" ] || die "${BIN} does not answer --version — the download landed but cannot run"
|
||||
# The converge actually took — asserted, not assumed. A pin that silently did
|
||||
# not land is exactly the failure --version exists to make impossible.
|
||||
|
|
|
|||
|
|
@ -59,9 +59,21 @@ forgejo_runner_secure() {
|
|||
# reports, empty when it cannot answer. `forgejo-runner --version` prints
|
||||
# "forgejo-runner version v12.13.2"; the leading v is stripped so this compares
|
||||
# against a --version argument, which has its own v stripped at parse.
|
||||
#
|
||||
# `|| true` for json_field's reason, which bites harder here. Callers run under
|
||||
# `set -euo pipefail`, where a pipeline whose FIRST stage exits non-zero fails
|
||||
# the whole pipeline — and `PRESENT_VER="$(runner_version_of "$BIN")"` is an
|
||||
# assignment, so the script dies AT THAT LINE, with no message. "Empty when it
|
||||
# cannot answer" is only true if this says so out loud.
|
||||
#
|
||||
# That is not a hypothetical shape: the binary is `[ -x ]` but unrunnable
|
||||
# exactly when a ci-box's template preinstall landed a truncated or wrong-arch
|
||||
# download — the one path this command family exists for. Without this, install
|
||||
# exits 1 in silence and the refusal written for that case ("the download
|
||||
# landed but cannot run") is unreachable code.
|
||||
runner_version_of() {
|
||||
"$1" --version 2>/dev/null | head -n1 \
|
||||
| sed -nE 's/.*[Vv]ersion[[:space:]]+v?([0-9][0-9A-Za-z.+-]*).*/\1/p'
|
||||
| sed -nE 's/.*[Vv]ersion[[:space:]]+v?([0-9][0-9A-Za-z.+-]*).*/\1/p' || true
|
||||
}
|
||||
|
||||
# runner_download_decision <have-binary yes|no> <present-ver> <wanted-ver>
|
||||
|
|
|
|||
17
test/cli.sh
17
test/cli.sh
|
|
@ -3389,13 +3389,28 @@ check "forgejo-runner: install routes through the shared checksum policy" 0 "fet
|
|||
# binary at mint — so a bare presence check would make --version dead on the
|
||||
# exact path this command exists for.
|
||||
FRLIB="$ROOT/commands/lib/forgejo-runner-config.sh"
|
||||
vparse() { bash -c ". '$FRLIB'; runner_version_of '$1'"; }
|
||||
# Driven under the CALLER'S shell options, and that is half the test. Every
|
||||
# forgejo-runner-* command is `set -euo pipefail`; a reader that only returns
|
||||
# empty in a permissive shell has not kept its contract where it is used.
|
||||
vparse() { bash -c "set -euo pipefail; . '$FRLIB'; runner_version_of \"\$1\"" _ "$1"; }
|
||||
VSTUB="$(mktemp -d)"
|
||||
printf '#!/bin/sh\necho "forgejo-runner version v12.13.2"\n' > "$VSTUB/fr"; chmod +x "$VSTUB/fr"
|
||||
printf '#!/bin/sh\necho "garbage"\n' > "$VSTUB/bad"; chmod +x "$VSTUB/bad"
|
||||
# `[ -x ]` yes, runnable no: a truncated or wrong-arch download, which is what a
|
||||
# ci-box template preinstall leaves behind when it half-lands. Distinct from
|
||||
# `bad` — that one EXITS 0 and merely says nothing parseable, so it never
|
||||
# exercised the pipeline-failure path at all.
|
||||
printf '#!/bin/sh\nexit 1\n' > "$VSTUB/dead"; chmod +x "$VSTUB/dead"
|
||||
check "forgejo-runner: the version reader strips the leading v" 0 "12.13.2" vparse "$VSTUB/fr"
|
||||
check "forgejo-runner: an unreadable version yields empty, not garbage" 0 "" vparse "$VSTUB/bad"
|
||||
check "forgejo-runner: a binary that cannot RUN yields empty, not a silent set -e death" 0 "" \
|
||||
vparse "$VSTUB/dead"
|
||||
rm -rf "$VSTUB"
|
||||
# The install site's own read of --version needs the same guard, or its refusal
|
||||
# is unreachable: the assignment dies before the test below it can fire. Pinned
|
||||
# by grep because reaching that line for real needs root and a downloaded binary.
|
||||
check "forgejo-runner: the --version read cannot die ahead of its own refusal" 0 "|| true" \
|
||||
grep -o 'head -n1 || true' "$FR"
|
||||
|
||||
# The decision itself, driven — this is the case review !110 caught, and a
|
||||
# grep could not have caught it. Each row is a real lifecycle situation.
|
||||
|
|
|
|||
Loading…
Reference in a new issue