From b46aee09fe552045c5f5cab8e204396d49c84908 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:55:39 +0000 Subject: [PATCH] ci(tools): retry pinned downloads and name a TLS failure as one Main went red at 2026-09-22T07:09Z on commit 4df7881 -- which touched nothing in this area -- with `curl: (60) SSL certificate problem: certificate has expired` fetching the pinned FastQC zip. That host is a single university web server; its certificate had lapsed, and the log said only "##[error]Process completed with exit code 60", which reads like a code failure until someone opens the log and knows what 60 means. curl does not retry a certificate failure without --retry-all-errors, so --retry alone would not have covered the class that bit. Skipping the tool is not available either: issue #30 exists because fastqc was silently absent from CI, and a skip is not a pass. So the three pinned archive installs now share one sourced helper that: - retries the transient class (4 attempts, 5s apart), covering TLS handshake and receive failures as well as connection errors; - passes --fail, so an HTTP error status is a failure now rather than a body whose checksum mismatches later -- which points at the artifact instead of at the URL being wrong; - refuses an empty body explicitly, rather than leaving it to the checksum to explain; - annotates WHICH failure it was: a TLS failure prints the cause, the openssl command that confirms it, and the pin file that repoints it; a network failure says so; an HTTP error says the pinned URL is wrong; - returns curl's exit code, so the step still fails, loudly. The checksum check stays hard and stays in the caller, unchanged: a retry must never turn "the artifact changed" into "the artifact was eventually accepted". Verified against real failure modes, not by inspection: a self-signed TLS server (exit 60 -> the TLS diagnosis, exit 60), an HTTP 404 (exit 22 -> the URL diagnosis), an empty body (refused, exit 1), a good download against the matching checksum (passes), and a mismatched checksum (refused by the caller's check). test_install_pins.jl guards the wiring, and that guard was mutation tested rather than assumed. Deleting the helper, reverting one install step to a bare curl, and removing --retry-all-errors from the command each fail it. The first version did NOT fail on the third mutation, because the flag's name also appears in the comment explaining it -- the same sink this file's own $FASTQC_URL note warns about -- so it now asserts against the script with its comments stripped. 123/123 pass. Refs #30 --- .github/workflows/ci.yml | 15 ++++- scripts/ci/fetch_pinned.sh | 111 +++++++++++++++++++++++++++++++++ test/unit/test_install_pins.jl | 43 +++++++++++++ 3 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 scripts/ci/fetch_pinned.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9aac12f..a9b3f1f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -367,9 +367,16 @@ jobs: # Downloaded against the pinned URL and refused unless it hashes to the pinned # SHA256, so CI runs the same bytes install.jl puts on a developer's machine. + # + # fetch_pinned adds retries that cover the TLS class (curl does not retry a + # certificate failure on its own) and, when a download still fails, annotates + # WHICH failure it was. On 2026-09-22T07:09Z the FastQC host's expired + # certificate reddened main on a commit that had touched nothing in this area; + # the log said only "exit code 60". See scripts/ci/fetch_pinned.sh. - name: Install vsearch run: | - curl -sSL -o vsearch.tar.gz "$VSEARCH_URL" + source scripts/ci/fetch_pinned.sh + fetch_pinned "$VSEARCH_URL" vsearch.tar.gz echo "$VSEARCH_SHA256 vsearch.tar.gz" | sha256sum -c - tar xzf vsearch.tar.gz --warning=no-unknown-keyword sudo mv "vsearch-$VSEARCH_VERSION-linux-x86_64/bin/vsearch" /usr/local/bin/vsearch @@ -378,7 +385,8 @@ jobs: - name: Install swarm run: | - curl -sSL -o swarm.tar.gz "$SWARM_URL" + source scripts/ci/fetch_pinned.sh + fetch_pinned "$SWARM_URL" swarm.tar.gz echo "$SWARM_SHA256 swarm.tar.gz" | sha256sum -c - tar xzf swarm.tar.gz --warning=no-unknown-keyword sudo mv "swarm-$SWARM_VERSION-linux-x86_64/bin/swarm" /usr/local/bin/swarm @@ -396,7 +404,8 @@ jobs: # install landed and that a JRE is present on the runner. - name: Install fastqc run: | - curl -sSL -o fastqc.zip "$FASTQC_URL" + source scripts/ci/fetch_pinned.sh + fetch_pinned "$FASTQC_URL" fastqc.zip echo "$FASTQC_SHA256 fastqc.zip" | sha256sum -c - sudo unzip -q -d /opt fastqc.zip rm -f fastqc.zip diff --git a/scripts/ci/fetch_pinned.sh b/scripts/ci/fetch_pinned.sh new file mode 100644 index 00000000..fb94994a --- /dev/null +++ b/scripts/ci/fetch_pinned.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MPL-2.0 +# SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +# +# fetch_pinned.sh — download a pinned artifact, absorbing transient failures and +# naming a TLS failure as a TLS failure. +# +# SOURCED, not executed: it defines one function and returns. Callers keep the +# checksum check and the unpacking in their own step, because those differ per +# archive (zip vs tar.gz) and the pin test asserts the checksum line is visible in +# the workflow's own run block. +# +# Usage, from a step in .github/workflows/ci.yml: +# +# source scripts/ci/fetch_pinned.sh +# fetch_pinned "$VSEARCH_URL" vsearch.tar.gz +# echo "$VSEARCH_SHA256 vsearch.tar.gz" | sha256sum -c - +# +# Why this exists. On 2026-09-22T07:09Z the "Install fastqc" step reddened main on +# commit 4df7881 with: +# +# curl: (60) SSL certificate problem: certificate has expired +# +# Nothing in that commit could cause it: the pinned FastQC URL is hosted on one +# university web server, and that server's certificate had lapsed. The gate went red +# for a third party's TLS maintenance, and the run's log said only "Process completed +# with exit code 60", which reads like a code failure until someone opens the log and +# knows what 60 means. +# +# Two things are wrong with that, and they need different fixes: +# +# 1. A transient handshake failure (a reset mid-TLS, a slow CA fetch, a proxy) +# should not fail the build at all. curl does NOT retry those by default, and +# `--retry` alone still excludes them; `--retry-all-errors` is what covers the +# TLS class. That is the retry below. +# +# 2. A genuine, sustained certificate expiry cannot be retried away, and must not +# be papered over — skipping the tool is exactly the failure issue #30 was +# about, and the repository's standing rule is that a skip is not a pass. So it +# still fails, but it fails SAYING WHAT IT IS, with the command that confirms it +# and the file that repoints it. The next person then spends a minute on it +# instead of reading a diff for a certificate they did not touch. +# +# The checksum check stays hard, and stays in the caller: a retry must never turn +# "the artifact changed" into "the artifact was eventually accepted". + +# fetch_pinned +# +# Downloads to , retrying transient failures. Returns 0 on a +# non-empty download. On failure, annotates the cause and returns curl's own exit +# code, so the step fails with the code that explains it. +fetch_pinned() { + local url="$1" out="$2" rc=0 + + # --retry-all-errors is the load-bearing flag: without it curl refuses to retry + # exit 60 (certificate), 35 (handshake) and 56 (recv), which are precisely the + # transient cases that used to fail a run on the first attempt. + # -f/--fail as well: without it curl treats an HTTP 404 as success and writes the + # error page to $out, so the failure would surface later as a checksum mismatch -- + # true, but it points at the artifact rather than at the URL being wrong. + curl -fsSL --retry 4 --retry-delay 5 --retry-all-errors \ + --connect-timeout 20 --max-time 600 \ + -o "$out" "$url" || rc=$? + + if [ "$rc" -eq 0 ]; then + # A 200 with an empty body is a success to curl and a broken artifact to + # everyone else. The checksum check would catch it, but naming it here says + # which of the two happened. + if [ -s "$out" ]; then + return 0 + fi + echo "::error::fetched $url but it is empty; the checksum check would fail next" + return 1 + fi + + local host="${url#https://}" + host="${host%%/*}" + # The port is only appended when the URL did not already carry one, or the + # command printed below reads `host:8443:443` and cannot be pasted anywhere. + case "$host" in + *:*) ;; + *) host="$host:443" ;; + esac + + case "$rc" in + 35|51|58|60|77|83|90) + echo "::error::TLS verification failed (curl exit $rc) fetching $url" + echo "::error::The certificate is the problem, not this commit. Confirm with:" + echo "::error:: openssl s_client -connect $host:443 /dev/null | openssl x509 -noout -dates -subject" + echo "::error::If it has expired, the host has to renew it: re-run then, or repoint" + echo "::error::the URL and sha256 in config/defaults/tool_versions.yml if the" + echo "::error::artifact has moved. Do NOT skip the tool — CI installs it because" + echo "::error::the pipeline shells out to it (issue #30)." + ;; + 22) + echo "::error::the server answered with an HTTP error status fetching $url" + echo "::error::(curl --fail, exit 22). The pinned URL is wrong or the artifact" + echo "::error::was withdrawn; check config/defaults/tool_versions.yml against" + echo "::error::whatever the project is serving now." + ;; + 6|7|28|56) + echo "::error::network failure (curl exit $rc) fetching $url after 4 retries;" + echo "::error::host unreachable or slow rather than refusing TLS. Re-run, or check" + echo "::error::the host is still the right place for this pin." + ;; + *) + echo "::error::download failed (curl exit $rc) fetching $url" + ;; + esac + return "$rc" +} diff --git a/test/unit/test_install_pins.jl b/test/unit/test_install_pins.jl index fe6eadc1..9d99e107 100644 --- a/test/unit/test_install_pins.jl +++ b/test/unit/test_install_pins.jl @@ -231,6 +231,49 @@ end # cd-hit is the one tool CI does not pin, so it is matched on the apt line. @test occursin("apt-get install -y cd-hit", runs) + + # The archive downloads retry, and say what failed when they still do not. + # + # MEASURED 2026-09-22T07:09Z: main went red on commit 4df7881 -- a commit that + # touched nothing in this area -- because the FastQC host's TLS certificate had + # expired. The log read "curl: (60) SSL certificate problem: certificate has + # expired" and then "##[error]Process completed with exit code 60", which reads + # like a code failure until someone opens the log and knows what 60 means. + # + # curl does not retry a certificate failure unless --retry-all-errors is given, + # so `--retry` alone would not have covered the class that bit. And skipping the + # tool is not available either: issue #30 exists precisely because fastqc was + # silently absent. So the installs share one helper that retries the transient + # class and annotates the cause when the failure is real. + helper = joinpath(REPO_ROOT, "scripts", "ci", "fetch_pinned.sh") + @test isfile(helper) + + # Comments stripped before asserting, and that is not incidental. The obvious + # version of this guard -- does "--retry-all-errors" appear in the file -- + # PASSES with the flag deleted from the command, because the paragraph + # explaining the flag names it. MUTATION-TESTED 2026-09-22: removing the flag + # from the curl invocation left this testset green until the assertion read the + # code alone. The same trap, in the same file, for the same reason as the + # `$FASTQC_URL` note above: a guard that can be satisfied by prose guards prose. + fetch_code = join([line for line in eachline(helper) + if !startswith(strip(line), "#")], "\n") + + # The load-bearing flag: this is the fix, not a detail. + @test occursin("--retry-all-errors", fetch_code) + # A diagnosis rather than a bare exit code, and a lever to pull: the next + # reader is told it is the certificate and where the pin lives. + @test occursin("::error::TLS verification failed", fetch_code) + @test occursin("tool_versions.yml", fetch_code) + # It reports and then fails; it never converts a failure into a pass. + @test !occursin("|| true", fetch_code) + @test !occursin("set +e", fetch_code) + + # Every pinned archive goes through it, so a tool added later cannot arrive + # with a bare curl that dies on the first TLS hiccup. + @test occursin("source scripts/ci/fetch_pinned.sh", runs) + for ref in (raw"$VSEARCH_URL", raw"$SWARM_URL", raw"$FASTQC_URL") + @test occursin("fetch_pinned \"$ref\"", runs) + end end @testset "a required check name is a stable identifier" begin