🐛 fix(ci): refuse to run gate scripts under bash < 4 (BASH32-F01) - #87
Merged
Conversation
…F01) `/bin/bash hack/docs/truthlag_pins_test.sh` on stock macOS printed 20 PASS lines, died at its `declare -A`, never printed its final `OK:` banner — and EXITED 0. `task docs-gates`, and therefore `task check`, read that as a green gate. AGENTS.md rule 4 makes a green local `task check` a per-commit precondition, so this was a live LOCAL trust hole; CI is ubuntu/bash 5, which is exactly why it survived unnoticed. Mechanism, measured on bash 3.2.57: 3.2 has no associative arrays, so under `set -u` it re-reads `declare -A x=([k]=v)` as an INDEXED array assignment and evaluates the subscript arithmetically, hitting "unbound variable"; the shell terminates the script and the exit status is 0. The two sibling scripts fail CLOSED, but only incidentally: `hack/release/verify-artifacts.sh:202`'s literal is EMPTY, so 3.2 degrades it to `declare: -A: invalid option` and `set -e` turns that into exit 2 (measured with a --dist that actually reaches line 202); `hack/validate-schemas-stock.sh` loses `mapfile` to "command not found" and exits 1. Populating an empty literal or dropping `-e` converts either into the silent case, so the floor is now declared rather than inferred from how one construct happens to degrade. `hack/lib/require-bash.sh` takes the floor as a PARAMETER — 4.0 for `declare -A`, 4.4 for `mapfile -d ''` — because a blanket floor would refuse shells that can run the 4.0 gates and would hide which construct binds. It is bash-3.2-safe by construction: it is sourced by scripts whose whole job under 3.2 is to refuse cleanly. The lib is resolved script-relative, then from the enclosing git checkout, because hack/release/install_cosign_pin_test.sh executes mutated COPIES of verify-artifacts.sh out of a mktemp dir; unresolvable means refuse. D-154 records the decision and the two disproved theories (EXIT-trap status swallow, `set -e`). The enforcing gate lands next.
…H32-F01)
Without this, the next gate author rediscovers D-154 the way this one was
found — by accident, on macOS, after the gate had been silently certifying
nothing for months.
`hack/lint/bash_version_guard_test.sh` scans every `hack/**/*.sh` for
command-position bash 4+ constructs, requires each to declare a floor via
`require_bash`, requires that floor to be at least the FEATURE's own minimum
(`declare -A` 4.0, `local -n` 4.3, `mapfile -d ''` 4.4), and requires `|| exit
1` on the source and require_bash lines — the callers run `set -u` without
`-e`, so a failed source or an undefined function would otherwise continue
straight into the guarded construct.
Anti-vacuity, which is what this repo's reviews reliably find missing:
- all 10 detection patterns must COMPILE. A malformed regexp makes grep exit
2 per file, match nothing and leave the scan green; that is not
hypothetical — the first draft used `|` both as a field separator and
inside three patterns, truncating them to unbalanced parentheses.
- the scan must still SEE all four known feature users, so a typo reds.
- eight negative controls, all on REAL copies of the shipped scripts rather
than synthetic fixtures: guard block stripped => flagged; pristine copy =>
clean; floor lowered 4.4 -> 4.0 => flagged; `|| exit 1` dropped => flagged;
a COMMENT mentioning BASH_VERSINFO => still flagged, while a real
BASH_VERSINFO expression is accepted (the aud2 precedent's branch, which no
other control touches).
- the crux: under a real bash 3.2, a guard-stripped copy of
truthlag_pins_test.sh in a copied tree must exit 0, with the final `OK:`
banner ABSENT, after printing at least one PASS line, with `unbound
variable` on stderr. "It fails" would have passed against the two scripts
that fail closed and proved nothing.
- that control needs a bash < 4 and CANNOT run on CI ubuntu. It skips with a
ten-line block on stderr and the success line itself names the gap, so a
green CI run is never readable as evidence for it.
The gate is bash-3.2-clean and asserts that about itself, together with
hack/lib/require-bash.sh: guard machinery that cannot run under the shell it
protects against cannot refuse cleanly. It also carries an EXIT trap that reds
if it ever exits 0 without reaching its last line — the defect it exists to
catch, applied to itself.
Wired as `task check` stage 20 and pinned in hack/audit/exitgate_test.sh's
CHECK_STAGES (19 -> 20) plus STAGE_BODY_PINS in this same commit, which is what
that array exists to force. CHANGELOG.md is the regenerated `task
changelog-write` output for the preceding commit.
Stated limits, in the script's own comments: it sees command-position
constructs in `hack/**/*.sh` only — not `eval`, dynamically built command
names, bash 4 EXPANSIONS (`${v^^}`, globstar), non-.sh files, anything outside
`hack/`, or the numeric floor of a hand-rolled BASH_VERSINFO guard.
`MUST_BE_CLEAN` asserts ABSENCE from the scan, which passes for two indistinguishable reasons: the file really is clean, or its path no longer names a file the scan ever looks at. Renaming `hack/lib/require-bash.sh` left "hack/lib/require-bash.sh is bash-3.2-clean" printing PASS forever about a file that was not there — the mirror image of the defect class this gate exists to catch, and the one check here that had no anti-vacuity partner while the pattern-compile loop, KNOWN_FEATURE_USERS and controls (a)-(e) all had one. Reproduced before the fix and after: with `hack/lib/require-bash.sh` renamed to a path that does not exist, the gate now reds with "does not exist — its 'is bash-3.2-clean' assertion would pass forever about a file that is not there" instead of passing. Also corrects the preceding commit's message, which is published and so is not being rewritten: section (2) has SIX negative controls plus TWO positive discriminations (the pristine copy reported clean, and a real BASH_VERSINFO expression accepted), not "eight negative controls". The positives are what make the negatives mean something rather than being artefacts of grading a copy, so the distinction is worth stating rather than rounding away.
Both were the species this gate exists to catch, and both were found by building
the mutant rather than by reading the code.
F1 (P1) — has_versinfo_guard() accepted a MENTION, not a guard. It matched any
non-comment line containing the token, so
echo "note: BASH_VERSINFO is nice to have"
declare -A probe=([k]=v)
passed the whole gate at exit 0 with zero FAIL lines: the exact D-154 shape
walking past the gate built to stop it. Worse, control (e)'s own failure text
already asserted "a mention is not a guard" — a claim the code did not
implement. It now requires a COMPARISON (`-lt|-le|-gt|-ge|-eq|-ne|<|>|=` on a
non-comment BASH_VERSINFO line), and control (e) grew its missing half: a
non-comment mention must be FLAGGED, alongside the existing comment-rejected and
real-expression-accepted cases.
F2 (P1) — 108 lines of new library with no host-independent behavioural test.
require-bash.sh was exercised behaviourally only by section (3), which is
macOS-only, so on CI — the one place this runs on every PR — gutting
`require_bash()` to `return 0` left the gate at exit 0 and every guard in the
tree decorative. New section (2b) sources the library and probes both ends of
the comparison against whatever bash is running it: `require_bash 99.0` must
return non-zero AND name the floor on stderr, `require_bash 1.0` must return 0,
and a no-argument call must return non-zero. It runs before section (3), so it
runs everywhere.
F3 (P2) — `mapfile[[:space:]]+-[a-zA-Z]*d` required `-d` to close the FIRST
option cluster, so `mapfile -t -d '' arr` — identical in meaning to the in-tree
`mapfile -d '' -t` — read as plain 4.0 and a 4.0 floor over a 4.4 construct
passed green. Pattern widened to catch `-d` in any later option position, with
control (f) pinning it. The residual is now DOCUMENTED rather than silent: when
an option BEFORE `-d` takes an argument (`mapfile -u 3 -d ''`) it is still
missed, and CANNOT-SEE says so.
F5 — swept in this gate only: `cd "$ROOT" || exit 1`. Neighbouring scripts keep
the bare form and shellcheck is not wired, so this is not a repo-wide change.
Mutant matrix, every case on a throwaway tree copy with the bash-3.2 control
forced to skip (the CI shape), gate=OLD is 4884db9:
baseline clean, gate=NEW rc=0 FAIL=0
F1 mutant, gate=OLD rc=0 FAIL=0 <- the fail-open
F1 mutant, gate=NEW rc=1 FAIL=1
F2 lib->return 0, gate=OLD rc=0 FAIL=0 <- the fail-open
F2 lib->return 0, gate=NEW rc=1 FAIL=3
F2 lib->return 1, gate=NEW rc=1 FAIL=2
F3 mapfile -t -d @4.0, gate=OLD rc=0 FAIL=0
F3 mapfile -t -d @4.0, gate=NEW rc=1 FAIL=1
F4 (stale "19 stages" prose in verify.yaml:190 and aud2_exitgate_test.sh:66) is
outside the fence and tracked by the coordinator; both are inert, no gate parses
either string. F6 and F7 declined as recorded in the review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
Under stock macOS
/bin/bash3.2,hack/docs/truthlag_pins_test.shprinted about 20PASSlines, died at itsdeclare -A ex_s09_prefix_hint=(…), never printed its finalOK: all truth-lag pins greenbanner — and exited 0.Its caller
task docs-gates, and thereforetask check, read that as a green gate. AGENTS.md rule 4 makes a green localtask checka per-commit precondition, so the gate was certifying almost nothing for every contributor on a stock macOS shell. CI is ubuntu/bash 5, which is exactly why this survived: no CI lane could ever surface it.Mechanism, measured on bash 3.2.57 rather than reasoned: 3.2 has no associative arrays, so under
set -uit re-readsdeclare -A x=([k]=v)as an indexed array assignment and evaluates the subscriptkarithmetically, hitting "unbound variable". The shell terminates the script and the exit status is 0.Two theories were disproved en route and are recorded so they are not re-explored: the
trap … EXITstatus-swallow (3.2 propagates 1 through an EXIT trap correctly) andset -e(the script runsset -uo pipefail).The other two affected scripts fail closed, but only incidentally —
hack/release/verify-artifacts.sh's literal is empty so there is no subscript to evaluate and it degrades todeclare: -A: invalid option(exit 2 viaset -e);hack/validate-schemas-stock.sh'smapfiledegrades to "command not found" (exit 1). Populating an empty literal or dropping-esilently converts a closed failure into the open one, so "how this construct happens to degrade" is not a guarantee worth depending on.The fix
A shared
require_bash <major>[.<minor>] <feature>helper inhack/lib/require-bash.sh, called with each script's own correct floor —declare -A→ 4.0,mapfile→ 4.0,mapfile -d ''→ 4.4. Per-feature floors rather than one blanket repo-wide minimum, because a blanket floor both refuses shells that could run the 4.0 gates and hides which feature actually binds.Enforcement is in one place per D-128:
hack/lint/bash_version_guard_test.sh, wired astask checkstage 20 and pinned inhack/audit/exitgate_test.sh'sCHECK_STAGESin the same commit. It scans everyhack/**/*.shfor command-position bash 4+ constructs, requires a guard on each, requires the declared floor to be at least the feature's own minimum, and — the assertion that keeps the rest non-vacuous — requires the scan to still find all four known feature-using files, so a typo in a detection pattern reds instead of quietly matching nothing. Its own negative control mutates real copies of the three scripts with the guard lines stripped and asserts they are flagged.Stated limits
The gate sees only command-position constructs in
hack/**/*.sh— noteval, dynamically built command names, case-modification expansions, or scripts outsidehack/. The exit-0 control needs a real bash 3.2: present on macOS, absent on CI ubuntu, where it SKIPs loudly rather than passing silently.hack/audit/aud2_exitgate_test.shkeeps its existing inlineBASH_VERSINFOguard and is accepted by the gate as guarded; adopting the helper there is a follow-up.Accepted behaviour change: under bash 3.2,
hack/release/verify-artifacts.sh --helpnow refuses instead of printing usage.Verification
task checkgreen (all 20 stages),task changelog-verifygreen.bash hack/audit/exitgate_test.sh→ rc 0, reports 20 stages,CHECK_STAGESlockstep holds. Run explicitly because its grader lives only in the push-onlyaudit-exitgate-testtarget — no PR run exercises it, so a stage miscount would redmainafter merge.bash hack/audit/exitgate_test.sh --text-only→ rc 0.Rebased onto
mainafter the golangci-lint bump; thedecisions.mdandbacklog.mdconflicts were append-ordering only and both sides are kept verbatim (D-154 and D-158;CI-TOOLCHAIN,CI-TOOLCHAIN-F01andBASH32-F01rows).Decision row: D-154.