fix(2205): assert the manifest covers what install-k8s.sh actually sources

[LukasWodka]

The gap

gen-manifest.sh already cross-checks two declarations against each other — its own FILES against install.sh's, and WINDOWS_FILES against install.ps1's. Both can agree and both be wrong, because neither is ever compared to what the installer actually sources.

So a new scripts/lib/foo.sh that install-k8s.sh sources but that reaches neither array is neither fetched by the bootstrap nor covered by the manifest. At install time it is either absent — the installer breaks on a customer machine, after CI was green — or fetched by some other path and executed unverified. That is a hole in the exact property R8 exists to provide.

Found by @saadqbal reviewing #755, filed as tracebloc/backend#2205.

Arms green

The sets agree today, 17 = 17, in both directions. Nothing was broken; what was missing is anything that keeps them agreeing.

Derived, not listed

A fourth array would be one more thing to drift. The check derives the sourced set from the installer.

Only ${LIB_DIR}/… sources count as repo libs — a naive grep source also matches . /etc/os-release (gpu-amd.sh, gpu-nvidia.sh) and source "$cred_file" (provision.sh), neither of which is in this repo. I checked those before writing the pattern rather than after.

It is non-transitive, and the assumption that makes that valid is itself a machine check: no lib sources another lib today, so walking install-k8s.sh alone sees everything. _check_no_lib_sources_lib fails if that ever stops holding, because otherwise this derivation would go quietly partial — the same claim that should be a check this whole family of guards exists for.

My own guard was unreachable at first

Worth recording, because it is this epic's defect in miniature and only the right kind of mutation caught it.

Under set -euo pipefail, a no-match grep (exit 1) aborted the script before the emptiness check could report. The run still failed — but silently, and by accident. An unreachable guard reads as coverage without being it.

I found it by asserting which message came out, not merely that the exit was non-zero. Checking only the exit code showed "fail-closed ✓" and would have shipped a dead guard. Now || rc=$?, with grep's 1 (matched nothing) routed to the guard and >1 (unreadable file) kept as its own error — did not check is never nothing is sourced.

Mutation proof

Each asserted to fail for its own reason, not just to fail (workspace canon rule 10 — a bare "it went red" can be a different code path than the test's name claims):

Mutation	Verdict	Message
a lib sourced but in neither array	RED	SOURCED BUT NOT COVERED
a lib sources another lib	RED	deriving from install-k8s.sh alone is no longer complete
the derivation matches nothing	RED	ZERO sourced libs
unmutated	GREEN	scripts/manifest.sha256 is up to date.

Each mutation asserted its anchor applied before running — an inert mutation and real coverage look identical in a log.

Where it gates

Inside gen-manifest.sh --check, so it rides the already-required Source-of-truth drift via make drift (6/6 green). No new required context, no branch-protection change — the pattern established in #755.

Test plan

• make drift 6/6 green; gen-manifest.sh --check green
• three mutations above, each verified by message
• bash -n + shellcheck --severity=error clean
• No change to the manifest itself, and no change to either bootstrap array — this only adds a comparison

Fixes tracebloc/backend#2205

🤖 Generated with Claude Code

---

Note

^{Cursor Bugbot is generating a summary for commit 242ad26. Configure here.}

[LukasWodka]

bugbot run

[saadqbal]

The gap is real and the derivation is the right answer to it — two declarations checked against each other can both be wrong, and neither was ever compared to what install-k8s.sh actually sources. Deriving instead of adding a fourth array is right, and I like that you checked . /etc/os-release and source "$cred_file"before writing the ${LIB_DIR}/… pattern rather than after. Guard is green on the tree (17 = 17).

Both Bugbot Lows are real, and I reproduced them.

1. The rc -gt 1 branch cannot fire. The comment above it draws the distinction carefully — "1 is matched nothing; >1 is operational and is its own error, never 'no libs are sourced'" — and the pipeline defeats it:

$ rc=0; sourced="$(grep … /nonexistent.sh | grep … | tr … | sed … | sort -u)" || rc=$?
rc=1 sourced=[]
-> would report: ZERO derived / derivation broken <- MISROUTED

Under pipefail the status is the rightmost non-zero, and the second grep exits 1 on empty stdin, masking the first's 2. So an unreadable installer reports "the derivation is broken", which sends someone to debug the pattern instead of the missing file.

Cleanest fix is to stop asking a pipeline for it: read the file once, then match the variable —

src="$(cat scripts/install-k8s.sh)"|| { echo"[ERROR] could not read …">&2;exit 1; }
sourced="$(grep -oE '…'<<<"$src"| …)"

— which separates "cannot read" from "no match" structurally rather than by exit-code archaeology.

2. The transitivity check fails open, and it's load-bearing.|| true swallows every failure:

$ offenders="$(grep -lE '…' scripts/lib/*.sh 2>/dev/null || true)"
offenders EMPTY -> check PASSES on an unreadable glob <- FAIL-OPEN

Low severity undersells this one, because this check is the assumption that makes the non-transitive derivation valid. If it passes on a failed read, the derivation can go quietly partial — which is the same "claim that should be a machine check" the function's own comment invokes. Worth distinguishing 1 from >1 here the way the other check tries to.

3. And the thing that would have caught both: there are no tests. This PR changes exactly one file, scripts/gen-manifest.sh. scripts/tests/gen-manifest.bats has 12 cases and none of them mentions the sourced-libs check, LIB_DIR, or transitivity — and the suite wasn't touched.

That's the part I'd push back on hardest, because the comment at :149-150 explicitly invokes the rule: "mutating the pattern and asserting WHICH message came out, not just that the exit was non-zero (backend#1729 rule 10)". That describes the practice; the PR doesn't implement it. Both findings above are precisely what asserting-which-message would have surfaced.

Cases I'd want, each asserting the specific message:

• a lib sourced by install-k8s.sh but absent from FILES → the SOURCED BUT NOT COVERED arm
• a lib in FILES that nothing sources → the over-fetched arm
• the pattern mutated so it matches nothing → "derived ZERO", not a pass
• install-k8s.sh unreadable → "could not read", not "derivation is broken" (fails today)
• a lib that sources another lib → the transitivity refusal
• the lib glob unreadable → a refusal, not a pass (fails today)

The last two are the ones that turn this from a check that happens to be right today into one that stays right. Requesting changes on that basis — the idea and the derivation are good, and Prereqs — almalinux:8 is still pending anyway.

[LukasWodka]

All three addressed. You were right on every count, and finding 3 was the fair hit — the comment invoked rule 10 while the PR asserted nothing at all.

1. The rc -gt 1 branch — it MISROUTED, not just failed to fire

Reproduced exactly as you describe. Worse than unreachable: a missing installer reported "the derivation is broken", sending the reader at the pattern instead of the file. And the comment above it claimed the distinction worked — a comment asserting a property the code lacks, which is the defect this guard family exists to remove.

Took your fix: read the file once, then match the variable, so "cannot read" and "no match" are separated structurally rather than by exit-code archaeology. Worth noting || true is safe on the new pipeline and was not on the old one — the input is now a shell variable, so grep's 1 for "matched nothing" is the only non-zero it can produce, and that is exactly what the ZERO guard is for.

2. The transitivity guard — agreed that "Low" undersells it

It is the assumption that makes the non-transitive derivation valid, so failing open there lets the derivation go quietly partial. || true gone; 1 is the clean answer, >1 is its own refusal, with grep's stderr carried into the message so the reason is visible rather than inferred.

3. Tests — all six you asked for, plus a seventh

Each asserts its specific message:

Case	Message
a lib sourced but absent from FILES	SOURCED BUT NOT COVERED
a lib in FILES that nothing sources	a different set of libs
the pattern matches nothing	ZERO sourced libs
install-k8s.sh unreadable	could not read
a lib that sources another lib	no longer complete
the lib glob unreadable	Refusing to assume none
a COMMENTED-OUT source	a different set of libs

Verified against the pre-fix script with the new suite, which is the part that matters:

not ok 16 an unreadable install-k8s.sh says 'could not read', not 'derivation is broken'
not ok 18 an unreadable lib glob is refused, not assumed clean

Both fail before, pass after — exactly the two you predicted. 19/19 now.

The seventh case is a defect the tests found in the derivation

grep -oE ignores what precedes the match, so #source "${LIB_DIR}/x.sh" counted as sourced. Two consequences: a commented-out source could never surface as over-fetched, and my first version of the over-fetched case commented a line out and therefore mutated nothing — it passed on the manifest-digest check, a different code path than its name claims. Precisely the trap you caught in _brand_rgbs on #762, one PR later.

The select is now line-anchored (source/. must be the line's first token, which also stops a mention inside a string or heredoc counting), the over-fetched case DELETES the line instead, and case 19 is the regression test for the anchoring itself.

make drift 6/6, shellcheck clean.

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[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 cdf8bd0. Configure here.}

[saadqbal]

All three of my asks are closed, and the tests are the full list I named plus two I didn't think of. Suite 12 → 20 cases.

1. The rc split is structural now, which is better than the exit-code fix I suggested:

src="$(cat scripts/install-k8s.sh)"|| {
echo"[ERROR] could not read scripts/install-k8s.sh to derive its sourced libs.">&2echo" That is 'did not check', never 'nothing is sourced'.">&2

Reading once and matching the variable means "cannot read" and "no match" can't be confused by a pipeline's rightmost status. Mutation-verified — removing that guard so an unreadable file falls through reddens exactly one case:

not ok 16 an unreadable install-k8s.sh says 'could not read', not 'derivation is broken'

2. The transitivity guard captures rc instead of swallowing it. Reverting it to || true reddens exactly an unreadable lib glob is refused, not assumed clean — the test for the bug, catching the bug.

3. And the part I'd single out: the comment at :160 distinguishing where || trueis still safe — "the input is a shell variable, so the only non-zero this can produce is grep's 1 for 'matched nothing' … No I/O can fail." That's the right response to a finding about a dangerous idiom: work out where it's actually dangerous rather than banning it everywhere. Blanket-removing it would have been easier and worse.

The two cases beyond my list — a commented-out source not counting, and a FILES array with no scripts/lib entries reporting rather than dying silently — are both the kind of thing that only shows up if you actually think about the input space.

One process note on my own work, since I've been holding others to it: my first attempt at mutation A was malformed shell, so every case failed and it told me nothing. I only caught it because bash -n and the failure pattern didn't match the property. That's the same trap the mutation notes in release-train warn about — an anchor that doesn't apply reports a confident false result — and it's worth saying out loud that it catches reviewers too.

Prereqs — almalinux:8 is the one check still in flight, so I can't lift the block yet. Nothing outstanding from me; I'll approve next pass once it lands.

[saadqbal]

Everything from me is closed — recorded on my previous comment, all three asks verified with clean mutations, suite 12 → 20 cases.

The one red check is an acknowledged registry flake, not this PR.Prereqs — opensuse/leap:15.6 failed on the image pull, and the step says so in its own words:

::error::could not pull $DISTRO from Docker Hub in 3 bounded attempts
- runner-to-registry connectivity, not this PR. Re-run this job.

followed by The operation was canceled. Nothing in this PR touches the prereq path — it's gen-manifest.sh and its bats — so a re-run of that one job should clear it. Good that the workflow diagnoses this itself rather than leaving a bare non-zero for someone to attribute.

I'm not approving over a red check even a self-identified flake, because the merge tooling reads the check state and an approval next to red says something untrue about readiness. Re-run it and I'll approve on my next pass — there's nothing else outstanding.

[LukasWodka]

Thanks — and noted on the mutation-A point; that trap is why I've started asserting the anchor applied before running, since an inert mutation and real coverage are indistinguishable in a log.

On the red check:Prereqs — opensuse/leap:15.6 failed, and it is a timeout kill, not a test failure. Diagnosed by arithmetic rather than by reading the log, because a timeout-minutes kill reports as cancelled with every completed step green:

conclusion=cancelled 12:04:51 -> 12:25:07 = 20m16s (cap: timeout-minutes: 20)
3. Pull opensuse/leap:15.6 (bounded, retried) -> success
4. Install prerequisites in opensuse/leap:15.6 -> cancelled <- killed mid-step

That job normally takes 1.4–2.1 min — surveyed the last ~40 runs of installer-tests.yaml across all branches and every single one passed under 2.1 min. So this hung roughly 10x its normal runtime on the zypper install path, which is an upstream mirror stall. Note step 3's bounded pull succeeded, so it is not the Docker Hub flake the retry logic was added for either.

Nothing in this PR can reach it: the diff is scripts/gen-manifest.sh and scripts/tests/gen-manifest.bats only, neither of which is in the prereq path.

Re-ran just the failed job. Also worth flagging separately from this PR: almalinux:8 was the one you saw in flight, and opensuse/leap:15.6 then hung — two different cells of that matrix being slow in one afternoon is worth a look at whether 20 minutes is the right cap for a job whose p100 is ~2 min, since a 10x hang currently costs the full 20.

[LukasWodka]

The re-run is green — 33 pass, 0 fail — so the block is liftable whenever you next pass.

One correction, because the mechanism matters for the follow-up. It was not the image pull. On attempt 1, job 96418515396:

concl=cancelled 12:04:51 -> 12:25:07 = 20m16s (distro-prereqs timeout-minutes: 20)
3. Pull opensuse/leap:15.6 (bounded, retried) -> success <- the pull WORKED
4. Install prerequisites in opensuse/leap:15.6 -> cancelled <- killed here

The could not pull … in 3 bounded attempts text you quoted is the step's source, echoed into the log, not runtime output — a pull that had actually failed three bounded attempts would leave step 3 failure, since it exit 1s. Step 3 is success. (I got caught by exactly that echoed-source-vs-output trap twice earlier today reading --log-failed, which is why I went to the step conclusions instead.)

So it was a timeout-minutes kill on the zypper install, which reports as cancelled with every completed step green — the signature that makes these read as "CI broke for no reason".

Why the distinction is worth a paragraph: "registry flake" points the fix at more pull retries, and the pull is already bounded and retried and already worked. The real fact is that step 4 ran ~10x its normal duration — I surveyed the last ~40 runs of installer-tests.yaml across all branches and that job's slowest success is 2.1 min. A 20-minute cap on a job whose p100 is ~2 min means any upstream mirror stall burns the full 20 before anyone learns anything.

Your conclusion still holds — nothing in this PR touches the prereq path (gen-manifest.sh + its bats only) and a re-run cleared it. I just don't want the matrix's real problem filed as a registry flake.

[saadqbal]

Approving. The Prereqs — opensuse/leap:15.6 flake has cleared — 33 passing, 3 skipped, nothing failing or pending, no open threads — and the head is still cdf8bd0f, the commit I verified, so nothing to re-check.

Recapping what closed, for the record on the PR rather than only in chat:

• The rc split is structural, which is better than the exit-code fix I'd suggested: src="$(cat scripts/install-k8s.sh)" || { … } means "cannot read" and "no match" can't be conflated by a pipeline's rightmost status. Mutation-verified — removing that guard reddens exactly an unreadable install-k8s.sh says 'could not read', not 'derivation is broken'.
• The transitivity guard captures rc instead of swallowing it. Reverting to || true reddens exactly an unreadable lib glob is refused, not assumed clean.
• Twenty cases, up from twelve — the six I named plus a commented-out source not counting and a FILES array with no scripts/lib entries reporting rather than dying silently.

The bit I'd keep is the comment at :160 working out where || trueis still safe — "the input is a shell variable, so the only non-zero this can produce is grep's 1 for 'matched nothing' … No I/O can fail." Responding to a finding about a dangerous idiom by establishing where it's actually dangerous, rather than banning it everywhere, is the harder and better answer.

The underlying gap was worth closing too: two declarations checked against each other, neither compared to what the installer actually sources, on a signed bootstrap where a missed lib is either absent at install time or fetched and executed unverified.