Found while fixing the SIGPIPE fail-opens in client#522, and deliberately left out of that PR — same shape, different failure mode, and outside the two findings that PR addresses.
The bug
_extract_yaml_value (scripts/lib/install-client-helm.sh:158) pipes into grep. On an absent key grep exits 1, pipefail propagates it, the assignment returns 1, and under set -e the function aborts — which makes the very next line unreachable:
line=$(... | grep ...) # exits 1 when the key is absent
[[ -z "$line" ]] && return # line 159 — never reached in the failing shape
So the "key not present" path that line 159 exists to handle cannot run.
Why it is latent today
All three current call sites use command substitution — $( ... ) — which suspends errexit for the function body, so the exit 1 never aborts. Verified: a bare call exits 1; the $( ) form survives.
It goes live the moment someone calls it as a bare statement, which is the natural next refactor since the function already returns 1 to mean "no value found". That is the same latency pattern as the two bugs fixed in #522: correct diagnosis, precondition not yet met.
Fix
Make the absent-key case explicit rather than relying on grep's exit status leaking through a pipeline — e.g. drop the pipe (bash read loop or [[ =~ ]]), or capture with an explicit || line="" so the empty-check on the next line is actually reachable. Whichever way, the "not found" path must be reachable, not incidentally survivable.
Verification to insist on
Do not accept a fix that only proves the $( ) call sites still work — they already do. The test must exercise the bare-statement call with an absent key and show it no longer aborts.
Context: client#522, and the broader class in backend#1371 (a guard that cannot verify must refuse to claim it did).
Found while fixing the SIGPIPE fail-opens in client#522, and deliberately left out of that PR — same shape, different failure mode, and outside the two findings that PR addresses.
The bug
_extract_yaml_value(scripts/lib/install-client-helm.sh:158) pipes intogrep. On an absent keygrepexits 1,pipefailpropagates it, the assignment returns 1, and underset -ethe function aborts — which makes the very next line unreachable:So the "key not present" path that line 159 exists to handle cannot run.
Why it is latent today
All three current call sites use command substitution —
$( ... )— which suspends errexit for the function body, so the exit 1 never aborts. Verified: a bare call exits 1; the$( )form survives.It goes live the moment someone calls it as a bare statement, which is the natural next refactor since the function already returns 1 to mean "no value found". That is the same latency pattern as the two bugs fixed in #522: correct diagnosis, precondition not yet met.
Fix
Make the absent-key case explicit rather than relying on
grep's exit status leaking through a pipeline — e.g. drop the pipe (bashreadloop or[[ =~ ]]), or capture with an explicit|| line=""so the empty-check on the next line is actually reachable. Whichever way, the "not found" path must be reachable, not incidentally survivable.Verification to insist on
Do not accept a fix that only proves the
$( )call sites still work — they already do. The test must exercise the bare-statement call with an absent key and show it no longer aborts.Context: client#522, and the broader class in backend#1371 (a guard that cannot verify must refuse to claim it did).