fix: SIGPIPE under pipefail made the chart version guard skip a real chart change

[LukasWodka]

What

Three producer | early-exiting-consumer pipelines ran under set -o pipefail. When the consumer exits on its first match/line it closes the pipe; once the producer still has more than the ~64KB pipe buffer to write it takes SIGPIPE, and the pipeline exits 141 — a success case reported as a failure.

Fixes two Bugbot findings from the #519 review. Restructured to remove the pipes rather than sprinkling || true, which would only trade one fail-open for another.

Site	Change
.github/workflows/chart-version-guard.yml	Classify the changed-file list with a bash read loop + case — no pipe, no subprocess. Plus explicit fail-closed::error:: paths for a missing base SHA / failed git diff.
scripts/lib/install-client-helm.sh · _pull_failure_detail	printf … | head -n 3 → head -n 3 <<< "$bad"
scripts/lib/gpu-nvidia.sh · _gpu_stack_signature	… | head -1 || true → capture whole, first line via ${var%%$'\n'*}, || var="" for the absent/timeout contract

scripts/manifest.sha256 regenerated with scripts/gen-manifest.sh (R8) — never hand-edited.

Why finding 1 is the serious one: the guard failed OPEN

In the guard the 141 lands on an if ! branch, so a genuine client/templates/** edit was reported as "guard N/A" and the version-bump check was skipped — waving through exactly the dark ship the gate was added to stop (PR #472 / the perIngestionTables flag block).

Evidence

Threshold — measured on the runner's own toolchain (ubuntu:24.04, bash 5.2.21, GNU grep 3.11, coreutils 9.4)

Pipeline exit status under set -o pipefail, producer = printf, match on line 1:

changed-file list	grep -q (match)	head -n 3	head -1
7,222 B	0	0	0
65,622 B	141	141	141
220,922 B	141	141	141
1,488,922 B	141	141	141

Finding 1 — end-to-end, real git repos, the workflow's run: block extracted verbatim from the YAML

Chart template changed, version NOT bumped ⇒ the gate must exit 1. Padding placed in docs/ so it sorts afterclient/templates/** (the match lands on line 1, with the rest of the stream still to write):

F1 21 paths / 903 B match line 1/21 OLD exit=1 NEW exit=1
F2 1301 paths / 58,723 B match line 1/1301 OLD exit=0 ⇐ FAIL-OPEN NEW exit=1 ✓
F3 4001 paths / 182,923 B match line 1/4001 OLD exit=0 ⇐ FAIL-OPEN NEW exit=1 ✓

OLD exit=0 printed No chart template/values change in this PR — guard N/A. — with a template edit sitting in the diff and no version bump.

The mirror direction is broken too. A SIGPIPE on the grep -qx 'client/Chart.yaml'match short-circuits the &&, so a PR that did bump the version gets failed with a message asserting it did not (client/Chart.yaml sorts first in byte order, so the match is on line 1):

E template + bump + 1300 pad / 95,141 B OLD exit=1 ⇐ false positive NEW exit=0 ✓

Gate semantics are otherwise preserved — all five control cases agree:

A template changed, NO bump OLD=1 NEW=1 (blocks)
B template changed + bump OLD=0 NEW=0 (passes)
C no chart change OLD=0 NEW=0 ("guard N/A")
D tmpl + NO bump + 1300 pad OLD=1 NEW=1 (blocks)

Finding 2 — _pull_failure_detail

The real function, extracted from the lib, set -euo pipefail, mocked kubectl emitting 20,000 ImagePullBackOff pods (1,188,889 B). One process per shape so no outer || can suspend errexit:

body=pipe (old) call=$(f) x509 reason emitted ✓
body=pipe (old) call=$(f || true) x509 reason emitted ✓
body=pipe (old) call=f > file *** ABORTED exit=141 — x509 reason LOST ***
body=here (new) call=$(f) x509 reason emitted ✓
body=here (new) call=$(f || true) x509 reason emitted ✓
body=here (new) call=f > file x509 reason emitted ✓

Isolating the inner pipeline confirms the mechanism in every redirection shape:

bad= 335 B pipe>/dev/null=0 pipe>file=0 pipe-in-$()=0 herestring=0
bad= 116,889 B pipe>/dev/null=141 pipe>file=141 pipe-in-$()=141 herestring=0
bad=1,188,889 B pipe>/dev/null=141 pipe>file=141 pipe-in-$()=141 herestring=0

Finding 2 — _gpu_stack_signature (behaviour-preserving hardening)

Identical output old vs new across five probe shapes, including 20,000 lines (1.1 MB) of chatty nvidia-ctk output:

mode=normal old=[…1.15.0|550.54.14] new=[…1.15.0|550.54.14]
mode=multi old=[…1.15.0|550.54.14] new=[…1.15.0|550.54.14]
mode=absent old=[] new=[]
mode=timeout old=[] new=[]
mode=chatty old=[1.15.0|550.54.14] new=[1.15.0|550.54.14]

Gates

• bats scripts/tests/*.bats — 669/669 pass, 0 failures (incl. the three _gpu_stack_signature tests and set -e safe with no versions (#431 Bugbot))
• shellcheck --severity=warning on both touched libs — clean; the only hit is the pre-existing cross-file SC2034 K3D_GPU_FLAGS, byte-identical on develop
• shellcheck --severity=error on the full CI file set — clean
• actionlint .github/workflows/chart-version-guard.yml — clean
• bash -n both libs — parse
• bash scripts/check-style.sh — clean
• scripts/gen-manifest.sh --check — up to date (failed before regeneration, as expected)
• New constructs re-verified on bash 3.2.57 (the macOS installer floor)

Two premises that did not hold up

1. "especially on large release-train diffs" overstates the reachability for this repo. The threshold is ~65 KB of changed paths (~900 files). tracebloc/client has 175 tracked files total, 5,756 bytes of full path list — so even a diff touching every file is ~11× under the threshold, and the largest --name-only list in the last 400 merges to main is 3,819 bytes. The fail-open is real and correctly diagnosed, but latent today, not an active dark-ship. It goes live if client/charts/ ever gains vendored subcharts or the repo grows. Fixing it because a guard must fail closed by construction, not because the repo happens to be small.

2. _pull_failure_detail does not currently lose the x509 reason. The finding says the function "exits before emitting the scoped pull events". At the only call site — install-client-helm.sh:575, fail_detail="$(_pull_failure_detail "$ns" || true)" — the || true suspends errexit for the entire function body, so the 141 never aborts anything and the x509 line is still emitted (proven above: call=$(f || true) emits it even at 1.19 MB). The abort is only reachable from a bare-statement call. It also needs ~1,300+ pods in ImagePullBackOff in one namespace. Worth fixing because returning 1 for "no pull failure" makes a bare call the natural next refactor — which would then abort the install mid-step with a bare 141.

Also spotted, deliberately not touched (out of scope)

scripts/lib/install-client-helm.sh:158, _extract_yaml_value:

line=$(grep -E "^${key}:""$file"2>/dev/null | head -1)

Same shape, but the latent hazard is a different one and unrelated to SIGPIPE: when the key is absent, grep exits 1, pipefail makes the pipeline 1, and the assignment returns 1 — yet line 159 ([[ -z "$line" ]] && return) shows "not found" is an expected path, so that line is unreachable in the shape that fails.

Measured, same method as above:

bare call, key absent, errexit live -> OUTER EXIT=1 (aborts; line 159 never runs)
v="$(probe … )", key absent <- real shape -> OUTER EXIT=0 (survives, returns "")

All three live call sites (lines 209, 651, 652) use the $( ) form, so this is latent, not live — exactly the same masking as finding 2. Flagged for its own ticket rather than folded in here, since it needs a different fix (|| line="", not a herestring) and is outside both findings' scope.

Notes

• Targets develop. Does not touch the open promotion PR release-train: develop -> staging #519, and no Bugbot review thread was resolved — a later release-train run picks this up.

Test plan

• bats scripts/tests/*.bats — 669/669
• shellcheck --severity=error (CI set) + --severity=warning (touched files)
• actionlint on the changed workflow
• bash scripts/check-style.sh
• scripts/gen-manifest.sh --check
• Old-vs-new differential on real git repos for the guard (5 cases + 3 fail-open cases)
• Old-vs-new differential for both installer functions on ubuntu:24.04 and bash 3.2

🤖 Generated with Claude Code

[LukasWodka]

bugbot run

[LukasWodka]

👋 Heads-up — Code review queue is at 33 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

• averaging-service#263 — docs(changelog): add missing fragment for bug: dataset rm cannot delete staging files — ingestor (uid 65534) vs jobs-manager uid mismatch, no shared fsGroup #259 · author: @divyasinghds · no reviewer assigned
• averaging-service#270 — release-train: develop -> staging · author: @LukasWodka · no reviewer assigned
• backend#1146 — docs(rfc): platform cost & autoscaling architecture · author: @saadqbal · reviewer: @LukasWodka, @saqlainsyed007
• backend#1201 — docs(rfc): Data spaces & federated dataset composition (draft) · author: @LukasWodka · reviewer: @saadqbal
• backend#1217 — docs(rfc): Dataset integrity & score reproducibility (draft) · author: @LukasWodka · reviewer: @saadqbal
• backend#1261 — RFC-BACKEND-0008: board & process reset (decision RFC — for markup) · author: @LukasWodka · reviewer: @saadqbal
• backend#1284 — docs(deploy): backend deploy runbook skeleton (#1263) · author: @LukasWodka · no reviewer assigned
• backend#1332 — feat(check-model): allow KNN/GaussianProcess on a single-edge owned dataset (#923 follow-up) · author: @divyasinghds · no reviewer assigned
• backend#1389 — docs(platform): land RFC-0002 phase runbooks & plans (#1145) · author: @saadqbal · no reviewer assigned
• backend#1398 — release-train: develop -> staging · author: @LukasWodka · reviewer: @saadqbal

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e023105. Configure here.}

[LukasWodka]

/fr-pass