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