fix(ci): make both safety guards fail loudly instead of vacuously #30
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/guards-fail-loudly"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Both guards could report success while checking nothing
Verified empirically, not inferred.
tools/check-hardening-drift.sh— run against an empty baseline, and again against a 50-line truncation, it printedhardening-drift: OK (no enabled hardening symbol regressed vs baseline)and exited 0 both times. The shape iswhile … done < <(enabled_hardening "$baseline"): no matches means the loop never iterates,regressedstays 0, and the success path runs. build-kernel.yml:356 only tests that the baseline file exists, so a truncated or zero-lengthreleases/<tag>/final.configwould retire the only guard covering the 31 inherited hardening symbols — with a green log and output indistinguishable from a real pass.build/configure-kernel.sh's fidelity assertion — same shape, same outcome. An emptyintent.configyieldsfidelity assertion: OK (0 intent lines, all landed).set -ecan't catch it: the inline$(grep -c …)in that message is a command substitution used as an argument, which never fails the shell.Fixes
Following the anti-vacuous-pass pattern
validate.ymlalready uses in itsintent-matches-policyanddep-list-parityjobs.drift — a ≥1000
CONFIG_-line floor on both inputs (real configs carry ~6,800, so this only catches truncation); the baseline symbol set is materialized before the loop and floored at 15. That second floor is a genuinely distinct check, not belt-and-braces:exclude_reis built fromintent.config, so a mangled intent can collapse the comparison set against a perfectly good baseline. The success line now reports the count, so a future vacuous pass is self-evident in the log.fidelity — parse once into a variable, then three guards: nothing parsed; a ≥60-line floor (92 today); and a candidates-vs-parsed equality check. The third catches what a floor cannot — CRLF endings, trailing whitespace after
is not set,#CONFIG_Xwithout the space — each of which silently drops that pin rather than failing. The CRLF case is not hypothetical: it parses all 69=ypins and drops all 23 off-pins. The guard reports exactly which lines fell through.ALLOW_HARDENING_REGRESSIONwas inertcheck-hardening-drift.sh:89has honoured it since it was written, and bothconfigs/POLICY.md:296anddocs/operators/config-architecture.md:138tell the operator to set it as a repo variable — but the drift step'senv:listed onlyWORKDIRandSERIES, sovars.ALLOW_HARDENING_REGRESSIONnever reached the script.That matters right now: without it, the only route past a drift failure is a commit to
main, which the stale-tag guard from3cd8e59then turns into "delete and re-cut the tag" — mid-CVE-SLA. Plumbed the same wayALLOW_STALE_TAGandREQUIRE_NET_ISOLATIONalready are.Also in the drift step: hard-fail an empty
SERIES(it would make the baseline glob match nothing and silently skip the guard), and make the no-baseline branch a::warning::— a skipped guard is a materially different outcome from a passed one, and a bareechoscrolls past in a 40-minute log.runner-setup.mdgains a repo-variables table; it previously mentioned onlyREQUIRE_NET_ISOLATION, in prose.Verification
Both changed blocks were tested by extracting them from the real files and running against real in-repo fixtures (12 released
final.configs, plus both authentic seed configs fetched and sha256-matched against their pins).drift:
v7.1.3 → v7.1.4release configs29 inherited hardening symbols checked31 … checked, none regressedCONFIG_KFENCE_DEFERRABLE)REGRESSION …ALLOW_HARDENING_REGRESSION=1fidelity:
final.config92 intent lines checked, all landed92 look like kconfig lines, but only 69 parsedis not setMISS …— unchanged behaviourValidators:
shellcheck build/*.sh tools/*.shclean,yamllint -d relaxedexit 0,py_compileclean,workflow-expressionsgreps clean.Note on the imminent v7.1.5 build
All four published 7.1.x releases were built from the 7.0.12 seed; v7.1.5 will be the first on the real 7.1.4 seed. I ran this guard between the two authentic seed configs and it passes (31 symbols, none regressed) — so the seed advance should not trip it. The
ALLOW_HARDENING_REGRESSIONfix is insurance for the case where the 7.1.4→7.1.5 tree delta drops something.🤖 Generated with Claude Code
The two checks that stand between a seed bump and a silently weakened kernel could both report success while checking nothing. Verified empirically, not inferred. tools/check-hardening-drift.sh: run against an EMPTY baseline, and against a 50-line truncation, it printed hardening-drift: OK (no enabled hardening symbol regressed vs baseline) and exited 0 both times. The shape is `while ... done < <(enabled_hardening "$baseline")`: no matches means the loop never iterates, `regressed` stays 0, and the success path runs. build-kernel.yml only tests that the baseline file EXISTS, so a truncated or zero-length releases/<tag>/final.config would retire the only guard covering the 31 inherited hardening symbols -- with a green log and no output distinguishable from a real pass. build/configure-kernel.sh's fidelity assertion: same shape, same outcome. An empty intent.config yields "fidelity assertion: OK (0 intent lines, all landed)". `set -e` cannot catch it, because the inline `$(grep -c ...)` in that message is a command substitution used as an ARGUMENT, which never fails the shell. Fixes, following the anti-vacuous-pass pattern validate.yml already uses in its intent-matches-policy and dep-list-parity jobs: - drift: a >=1000 CONFIG_-line floor on BOTH inputs (real configs carry ~6,800, so this only catches truncation); the baseline symbol set is materialized before the loop and floored at 15 (31 today) -- a second, distinct check, because exclude_re is built FROM intent.config and a mangled intent can collapse the comparison set against a perfectly good baseline; the success line now reports the count. - fidelity: parse once into a variable, then three guards -- nothing parsed, a >=60 line floor (92 today), and a candidates-vs-parsed equality check. The third catches what a floor cannot: CRLF endings, trailing whitespace after "is not set", "#CONFIG_X" without the space. Each of those silently drops that pin instead of failing. The CRLF case is not hypothetical -- it parses all 69 `=y` pins and drops all 23 off-pins, and the guard reports exactly which lines fell through. Separately: ALLOW_HARDENING_REGRESSION was inert. check-hardening-drift.sh has honoured it since it was written, and both configs/POLICY.md and docs/operators/config-architecture.md tell the operator to set it as a repo variable -- but the drift step's env: block listed only WORKDIR and SERIES, so `vars.ALLOW_HARDENING_REGRESSION` never reached the script. Without it the only route past a drift failure is a commit to main, which the stale-tag guard then turns into "delete and re-cut the tag", mid-CVE-SLA. Plumb it the same way ALLOW_STALE_TAG and REQUIRE_NET_ISOLATION already are, hard-fail an empty SERIES (which would silently skip the guard), and make the no-baseline branch a :⚠️: so a SKIPPED guard is visible in the run summary. Document all three hatches in a repo-variables table in runner-setup.md, which previously mentioned only REQUIRE_NET_ISOLATION, in prose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>