fix(648): the version parser was fail-OPEN on a jq that cannot start

Round 3, and it found that round 2's fix was a REGRESSION on the case that matters most.

`raw=$(jq --version 2>&1 || true)` did two wrong things at once: folded stderr into the
parse input and discarded the exit status. Combined with a pattern that matched the first
<digits>.<digits> ANYWHERE, a jq broken by a glibc mismatch — which exits 127 and writes
"version `GLIBC_2.34' not found" to stderr — parsed as version 2.34 and PASSED the floor.
The strip-based parse this replaced failed CLOSED there. So the fix for a fail-open bug
introduced a worse fail-open bug, in the one script whose entire purpose is to refuse to
certify a version it did not parse.

Same mechanism, second symptom: an unanchored match let a prefix outrank the real version.
`2026.07.26 jq-1.6` parsed as 2026.07; a leading warning line carrying any number won too.

Now: jq's exit status is captured explicitly (`$?` inside `if ! cmd` is the NEGATED status,
so that needed care too), stderr is kept out of the parse, and the pattern is anchored to
the leading `jq` token. Every legitimate form still parses — `jq-1.6`, `jq version 1.6`,
`jq-1.7.1`, `jq-1.6-dirty`, `jq-1.6 (Debian 1.6-2.1)`, `jq-1.10` (numeric compare, so the
two-digit minor is not read lexically).

The tests could not have caught any of this: the shim always exited 0 and never wrote to
stderr, so every case it could express was clean. It now takes stderr and an exit code, and
the four new cases turn red under the exact mutation.

Also: the drift guard now strips comment lines before matching. A future comment citing
`pulls/$pr/files?limit=100` as an example of what not to do would otherwise have reddened
script-tests — which, per this branch's own correction, blocks merges.

And the record no longer over-corrects: the combined-status read is guarded by
`if [ "$mwcs" != "true" ]`, so a red script-tests blocks the hook-mediated merge path, not
literally every merge.

Decisions-Edit: yes

docs/decisions/records/ci/jq-version-contract.md

@@ -58,8 +58,13 @@ assumed. So the fix has to hold at 1.6, in every gate, regardless of which lane
   twice. `.claude/hooks/pretooluse-merge-consent.sh` reads the **combined** commit status and denies
   on anything that is not `success`/`skipped` — see `ci.advisory-red-blocks-the-merge-gate` (#598).
   `script-tests` is a Gitea Actions job, so its red is a context folded into that combined state.
-  A jq bump therefore reddens `script-tests` and blocks every non-docs-only merge in the repo until
-  someone re-pins.
+  A jq bump therefore reddens `script-tests` and blocks non-docs-only merges until someone re-pins.
+
+  One qualification, so this does not over-correct in the other direction: that combined-status read
+  is guarded by `if [ "$mwcs" != "true" ]`. On the `merge_when_checks_succeed` path the hook does not
+  read the combined status at all and defers to Gitea, which gates on *required* checks only — and
+  `script-tests` is not one. So the blast radius is the hook-mediated merge path, not literally every
+  merge.
 
   The pin is kept anyway, deliberately: the fix is a one-line edit to the `--expect` value in
   `pr-checks.yml`, the failure message spells that out, and the alternative — silently losing the

scripts/jq-preflight.sh

@@ -71,7 +71,24 @@ if ! command -v jq >/dev/null 2>&1; then
   exit 1
 fi
 
-raw=$(jq --version 2>&1 || true)
+# Take jq's EXIT STATUS seriously, and keep stderr OUT of the parse input.
+#
+# This was `raw=$(jq --version 2>&1 || true)`, which did neither — and that combination turned the
+# guard fail-OPEN on the case it most needs to catch. A jq that cannot start (the canonical one is a
+# glibc mismatch after a base-image change) exits 127 and writes something like
+# `jq: /lib/x86_64-linux-gnu/libc.so.6: version 'GLIBC_2.34' not found` to stderr. Folded into `raw`,
+# that string contains `2.34`, which the version pattern happily matched — so the preflight printed
+# "parsed 2.34", certified the floor, and exited 0 on a jq that cannot run at all. The strip-based
+# parse this replaced failed CLOSED there, so it was a regression introduced by the fix.
+# `$?` inside an `if ! cmd; then` block is the NEGATED status (0), not jq's, so capture it explicitly.
+set +e
+raw=$(jq --version 2>/dev/null)
+jq_rc=$?
+set -e
+if [ "$jq_rc" -ne 0 ]; then
+  echo "jq-preflight: 'jq --version' failed (exit ${jq_rc}). jq is on PATH but cannot run — a broken build or a missing shared library. Failing closed rather than certifying a version it did not report." >&2
+  exit 1
+fi
 # `jq --version` prints e.g. `jq-1.6`, `jq-1.7.1`, or on some builds `jq-1.8.2-dirty`.
 # Parse with an explicit regex rather than by stripping around the first `-` and `.`.
 #
@@ -86,9 +103,15 @@ raw=$(jq --version 2>&1 || true)
 #
 # That is the silently-untested-axis failure this script was written to eliminate, reproduced inside
 # the script itself. Require a real `<digits>.<digits>` match, and fail closed when there isn't one.
-if [[ "$raw" =~ ([0-9]+)\.([0-9]+) ]]; then
-  major="${BASH_REMATCH[1]}"
-  minor="${BASH_REMATCH[2]}"
+# ANCHORED to the leading `jq` token, not "first digits.digits anywhere in the string".
+#
+# An unanchored match takes whatever number comes first, wherever it is. That accepted a leading
+# warning line or a date prefix as the version — `2026.07.26 jq-1.6` parsed as 2026.07, which sails
+# over the floor. Anchoring keeps every legitimate form (`jq-1.6`, `jq version 1.6`, `jq-1.7.1`,
+# `jq-1.6-dirty`, `jq-1.6 (Debian 1.6-2.1)`) and rejects the rest, which then fails closed below.
+if [[ "$raw" =~ ^[[:space:]]*[Jj][Qq][[:space:]-]+(version[[:space:]]+)?v?([0-9]+)\.([0-9]+) ]]; then
+  major="${BASH_REMATCH[2]}"
+  minor="${BASH_REMATCH[3]}"
 else
   echo "jq-preflight: could not parse a major.minor version out of '${raw}'. Refusing to assert a floor against an unparsed version — that would silently pass." >&2
   exit 1

scripts/tests/test_jq_preflight.py

@@ -29,18 +29,35 @@ WORKFLOWS = REPO_ROOT / ".gitea" / "workflows"
 BASH = shutil.which("bash") or "/bin/bash"
 
 
+def _shq(s):
+    """Single-quote a string for /bin/sh."""
+    return "'" + s.replace("'", "'\\''") + "'"
+
+
 @pytest.fixture
 def preflight(tmp_path):
     bindir = tmp_path / "bin"
     bindir.mkdir()
 
     class Handle:
-        def with_jq(self, version_line):
-            """Install a fake `jq` reporting `version_line` for --version."""
+        def with_jq(self, version_line, stderr="", exit_code=0):
+            """Install a fake `jq` reporting `version_line` for --version.
+
+            `stderr` and `exit_code` exist because an earlier version of this shim ALWAYS exited 0
+            and never wrote to stderr — so it structurally could not observe the worst failure this
+            script has: a jq that cannot start. The preflight was folding stderr into the parse via
+            `2>&1` and discarding the exit status, so a glibc-mismatch message containing `2.34`
+            parsed as version 2.34 and PASSED the floor. Every case the shim could express was clean,
+            so every test passed.
+            """
             shim = bindir / "jq"
-            shim.write_text("#!/bin/sh\n"
-                            'if [ "$1" = "--version" ]; then echo "%s"; exit 0; fi\nexit 0\n'
-                            % version_line)
+            body = "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then\n"
+            if version_line:
+                body += '  printf "%%s\\n" %s\n' % _shq(version_line)
+            if stderr:
+                body += '  printf "%%s\\n" %s >&2\n' % _shq(stderr)
+            body += "  exit %d\nfi\nexit 0\n" % exit_code
+            shim.write_text(body)
             shim.chmod(0o755)
 
         def without_jq(self):
@@ -163,6 +180,50 @@ def test_unparseable_version_fails_CLOSED_rather_than_asserting_nothing(prefligh
     assert "could not parse" in r.stderr
 
 
+def test_a_jq_that_cannot_START_fails_closed(preflight):
+    """THE case the previous shim could not express, and the guard therefore got wrong.
+
+    A jq broken by a glibc mismatch (the canonical post-base-image-bump failure) exits 127 and writes
+    `... version 'GLIBC_2.34' not found` to STDERR. The preflight was reading `jq --version 2>&1` and
+    discarding the exit status, so that message became the parse input, `2.34` matched, and the floor
+    was certified green on a jq that cannot run at all.
+    """
+    preflight.with_jq(
+        "", stderr="jq: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found",
+        exit_code=127)
+    r = preflight.run()
+    assert r.returncode == 1
+    assert "cannot run" in r.stderr
+    assert "parsed 2.34" not in r.stdout, "stderr must never be parsed as a version"
+
+
+@pytest.mark.parametrize("version_line", [
+    "warning: something 3.14",      # a noise line carrying a plausible number
+    "2026.07.26 jq-1.6",            # a date prefix, which outranks the real version if unanchored
+    "jq-master-v0.0.0-1.6",
+])
+def test_a_number_that_is_not_the_VERSION_is_not_accepted_as_one(preflight, version_line):
+    """Matching the first `<digits>.<digits>` ANYWHERE let a prefix win over the real version.
+    `2026.07.26 jq-1.6` parsed as 2026.07 and sailed over the floor. The pattern is anchored to the
+    leading `jq` token, so these fail closed instead."""
+    preflight.with_jq(version_line)
+    r = preflight.run()
+    assert r.returncode == 1, f"{version_line!r} was accepted as a version"
+    assert "could not parse" in r.stderr
+
+
+@pytest.mark.parametrize("version_line,expected", [
+    ("jq-1.6 (Debian 1.6-2.1)", "1.6"),   # distro packaging suffix
+    ("jq-1.10", "1.10"),                  # two-digit minor: must compare numerically, not lexically
+    ("jq-1.7.1", "1.7"),
+])
+def test_legitimate_forms_still_parse_to_the_right_version(preflight, version_line, expected):
+    preflight.with_jq(version_line)
+    r = preflight.run()
+    assert r.returncode == 0, f"{version_line!r}: {r.stderr}"
+    assert f"parsed {expected}" in r.stdout
+
+
 # --- Wiring guards: the preflight is worthless if a caller silently stops running it ------------
 
 def test_script_tests_pins_the_jq_version():

scripts/tests/test_pr_changed_files.py

@@ -361,11 +361,16 @@ def test_the_hook_uses_the_shared_script_and_does_not_reimplement_it():
     text = HOOK.read_text()
     assert "scripts/pr-changed-files.sh" in text, (
         f"{HOOK.relative_to(REPO_ROOT)} no longer calls the shared enumeration")
+    # Search CODE only. The hook's comments legitimately discuss this endpoint, and a future comment
+    # writing `pulls/$pr/files?limit=100` as an example of what NOT to do would redden this test —
+    # which, since `script-tests` red blocks merges via the combined status, would block the repo over
+    # a piece of prose.
+    code = "\n".join(ln for ln in text.splitlines() if not ln.lstrip().startswith("#"))
     # An inline `pulls/<n>/files?` fetch is the signature of a re-inlined copy. Match on the endpoint
     # alone, NOT on `?limit=` — an earlier version anchored the query string, so a copy written as
     # `files?page=1&limit=50` would have walked straight past a guard that exists to stop exactly
-    # that. A drift guard one refactor away from decorative is worse than none, because it reads as
-    # coverage.
-    assert not re.search(r"pulls/\$?\{?\w+\}?/files\?", text), (
+    # that. Still evadable by a copy that builds the URL without a literal `?`, so this narrows the
+    # gap rather than closing it.
+    assert not re.search(r"pulls/\$?\{?\w+\}?/files\?", code), (
         f"{HOOK.relative_to(REPO_ROOT)} appears to enumerate PR files inline again — "
         "that is the duplication ersatztv#649 removed")