From 40b756b87e642b7a271c2cc9540cbb846d0ea503 Mon Sep 17 00:00:00 2001 From: lusoris Date: Tue, 10 Nov 2026 16:15:42 +0100 Subject: [PATCH 1/6] fix(ci): retry refused and unreachable connections on CI downloads The cuda tidy lane failed on 2026-11-10 when curl could not connect to apt.llvm.org (exit 7, "Could not connect to server") and gave up on the first attempt. curl's --retry alone does not retry exit 7, and --retry-connrefused retries it only when the socket error is ECONNREFUSED, so an unreachable host still ended the job. Every curl fetch in workflows, container recipes and shell scripts now carries --retry N with --retry-all-errors, the convention most curl sites already used. wget fetches carry --tries=N, --retry-connrefused and --retry-on-host-error, so refused, unreachable and name-resolution failures are retried. The tidy lane's APT calls retry with Acquire::Retries=3. Per-fetch time bounds are unchanged. scripts/ci/tests/test_fetch_connect_retry.py reads every tracked fetch site and fails on a curl or wget call without these flags. Its allow-list names eight fetches that already sit inside a bounded outer poll or must fail fast, each with its reason. A pre-commit hook runs the test on every commit. Signed-off-by: lusoris --- .github/workflows/build.yml | 2 +- .github/workflows/ffmpeg-integration.yml | 2 +- .github/workflows/libvmaf-build-matrix.yml | 4 +- .github/workflows/lint-and-format.yml | 6 +- .github/workflows/security-scans.yml | 2 +- .github/workflows/supply-chain.yml | 8 +- .github/workflows/tests-and-quality-gates.yml | 2 +- .github/workflows/windows-tester-bundle.yml | 2 +- .pre-commit-config.yaml | 9 + Containerfile.vmafx | 2 +- Dockerfile | 6 +- Dockerfile.ffmpeg | 2 +- changelog.d/fixed/fetch-connect-retry.md | 1 + dev/Containerfile | 18 +- dev/Containerfile.runner | 2 +- docker/Dockerfile.tester | 2 +- docs/development/ci.md | 23 + scripts/ai/fetch-tiny-blobs.sh | 2 +- scripts/ci/build-macos-tester-bundle.sh | 6 +- scripts/ci/ffmpeg-shared-series.sh | 4 +- scripts/ci/install-cuda-toolkit.ps1 | 4 +- scripts/ci/install-cuda-toolkit.sh | 4 +- scripts/ci/install-intel-oneapi.sh | 2 +- scripts/ci/install-llvm.sh | 2 +- scripts/ci/install-rocm-from-image.sh | 2 +- scripts/ci/pinned-tool.sh | 2 +- scripts/ci/tests/test_fetch_connect_retry.py | 405 ++++++++++++++++++ scripts/dev/tidy-lane.sh | 26 +- scripts/release/pypi-publish-state.sh | 2 +- scripts/setup/fedora.sh | 2 +- scripts/setup/ubuntu.sh | 4 +- 31 files changed, 499 insertions(+), 61 deletions(-) create mode 100644 changelog.d/fixed/fetch-connect-retry.md create mode 100644 scripts/ci/tests/test_fetch_connect_retry.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4d97ada008..8aeab8453f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -333,7 +333,7 @@ jobs: bash scripts/ci/install-intel-ocloc.sh sudo apt-get update sudo -E apt-get -yq install gpg-agent cmake - wget -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list diff --git a/.github/workflows/ffmpeg-integration.yml b/.github/workflows/ffmpeg-integration.yml index df3850077f..10da3d6595 100644 --- a/.github/workflows/ffmpeg-integration.yml +++ b/.github/workflows/ffmpeg-integration.yml @@ -195,7 +195,7 @@ jobs: # ocloc compiles the SYCL ahead-of-time images; the Linux compiler # does not ship it and meson setup refuses without it (ADR-1360). bash scripts/ci/install-intel-ocloc.sh - wget -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list diff --git a/.github/workflows/libvmaf-build-matrix.yml b/.github/workflows/libvmaf-build-matrix.yml index 80b24db17e..b0c9cb53c8 100644 --- a/.github/workflows/libvmaf-build-matrix.yml +++ b/.github/workflows/libvmaf-build-matrix.yml @@ -414,7 +414,7 @@ jobs: sudo -E apt-get -yq install ccache ninja-build nasm gcc g++ gpg-agent pkg-config libzimg-dev # ADR-1360 (design notes) bash scripts/ci/install-intel-ocloc.sh - wget -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list @@ -948,7 +948,7 @@ jobs: WINDOWS_BASEKIT_URL: https://registrationcenter-download.intel.com/akdlm/IRC_NAS/1f18901e-877d-469d-a41a-a10f11b39336/intel-oneapi-base-toolkit-2025.3.0.372_offline.exe WINDOWS_DPCPP_COMPONENTS: intel.oneapi.win.cpp-dpcpp-common run: | - curl.exe --output %TEMP%\webimage.exe --url %WINDOWS_BASEKIT_URL% --retry 5 --retry-delay 5 + curl.exe --output %TEMP%\webimage.exe --url %WINDOWS_BASEKIT_URL% --retry 5 --retry-all-errors --retry-delay 5 start /b /wait %TEMP%\webimage.exe -s -x -f webimage_extracted --log extract.log del %TEMP%\webimage.exe webimage_extracted\bootstrapper.exe -s --action install --components=%WINDOWS_DPCPP_COMPONENTS% --eula=accept -p=NEED_VS2017_INTEGRATION=0 -p=NEED_VS2019_INTEGRATION=0 -p=NEED_VS2022_INTEGRATION=0 --log-dir=. diff --git a/.github/workflows/lint-and-format.yml b/.github/workflows/lint-and-format.yml index 80ab36828a..0a6c3a7525 100644 --- a/.github/workflows/lint-and-format.yml +++ b/.github/workflows/lint-and-format.yml @@ -662,7 +662,7 @@ jobs: run: | printf 'Acquire::Retries "3";\nAcquire::http::Timeout "30";\nAcquire::https::Timeout "30";\nAPT::Keep-Downloaded-Packages "true";\n' \ | sudo tee /etc/apt/apt.conf.d/80ci-retries >/dev/null - wget -qO- \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- \ https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | sudo gpg --dearmor \ -o /usr/share/keyrings/oneapi-archive-keyring.gpg @@ -1132,7 +1132,7 @@ jobs: fi url="https://github.com/stoplightio/spectral/releases/download/v${version}/spectral-linux-x64" mkdir -p "${RUNNER_TEMP}/spectral" - curl -fsSL --retry 3 --max-time 300 -o "${RUNNER_TEMP}/spectral/spectral" "${url}" + curl -fsSL --retry 3 --retry-all-errors --max-time 300 -o "${RUNNER_TEMP}/spectral/spectral" "${url}" echo "${sha} ${RUNNER_TEMP}/spectral/spectral" | sha256sum -c - chmod +x "${RUNNER_TEMP}/spectral/spectral" echo "${RUNNER_TEMP}/spectral" >> "${GITHUB_PATH}" @@ -1271,7 +1271,7 @@ jobs: mkdir -p "${RUNNER_TEMP}/trivy-bin" tarball="trivy_${version}_Linux-64bit.tar.gz" url="https://github.com/aquasecurity/trivy/releases/download/v${version}/${tarball}" - curl -fsSL --retry 3 --max-time 300 -o "${RUNNER_TEMP}/${tarball}" "${url}" + curl -fsSL --retry 3 --retry-all-errors --max-time 300 -o "${RUNNER_TEMP}/${tarball}" "${url}" echo "${expected_sha} ${RUNNER_TEMP}/${tarball}" | sha256sum -c - tar -xzf "${RUNNER_TEMP}/${tarball}" -C "${RUNNER_TEMP}/trivy-bin" trivy echo "${RUNNER_TEMP}/trivy-bin" >> "${GITHUB_PATH}" diff --git a/.github/workflows/security-scans.yml b/.github/workflows/security-scans.yml index 18e1f5222d..ef025826bb 100644 --- a/.github/workflows/security-scans.yml +++ b/.github/workflows/security-scans.yml @@ -308,7 +308,7 @@ jobs: - name: Install gitleaks run: | GITLEAKS_VERSION=8.24.3 - curl -sSfL --max-time 300 \ + curl --retry 3 --retry-all-errors -sSfL --max-time 300 \ "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ | tar -xz -C /usr/local/bin gitleaks gitleaks version diff --git a/.github/workflows/supply-chain.yml b/.github/workflows/supply-chain.yml index c599196ad7..93d2986fa4 100644 --- a/.github/workflows/supply-chain.yml +++ b/.github/workflows/supply-chain.yml @@ -976,7 +976,7 @@ jobs: for crate in vmafx-sys vmafx; do version="$(python3 -c 'import sys, tomllib; print(tomllib.load(open(sys.argv[1], "rb"))["package"]["version"])' \ "bindings/rust/$crate/Cargo.toml")" - http_code="$(curl --silent --show-error --max-time 60 \ + http_code="$(curl --retry 3 --retry-all-errors --silent --show-error --max-time 60 \ --user-agent 'VMAFx/vmafx release publisher (https://github.com/VMAFx/vmafx)' \ --output "$RUNNER_TEMP/$crate-version.json" --write-out '%{http_code}' \ "https://crates.io/api/v1/crates/$crate/$version")" @@ -1071,7 +1071,7 @@ jobs: fi local_files=("${wheels[0]}" "${sdists[0]}") response="$RUNNER_TEMP/pypi-release.json" - http_code="$(curl --retry 3 --max-time 60 --silent --show-error \ + http_code="$(curl --retry 3 --retry-all-errors --max-time 60 --silent --show-error \ --output "$response" --write-out '%{http_code}' \ "https://pypi.org/pypi/vmaf-mcp/$VMAFX_PEP440_VERSION/json")" publish=false @@ -1165,7 +1165,7 @@ jobs: response="$RUNNER_TEMP/pypi-release-after.json" http_code=000 for attempt in {1..12}; do - http_code="$(curl --retry 3 --max-time 60 --silent --show-error --output "$response" \ + http_code="$(curl --retry 3 --retry-all-errors --max-time 60 --silent --show-error --output "$response" \ --write-out '%{http_code}' \ "https://pypi.org/pypi/vmaf-mcp/$VMAFX_PEP440_VERSION/json")" if [[ "$http_code" == "200" && @@ -1373,7 +1373,7 @@ jobs: run: | set -euo pipefail url="$(python3 scripts/release/distribution.py source-url --version "$VMAFX_VERSION")" - curl --fail --location --silent --show-error --retry 3 --max-time 120 \ + curl --fail --location --silent --show-error --retry 3 --retry-all-errors --max-time 120 \ --output "$RUNNER_TEMP/source.tar.gz" "$url" digest="$(sha256sum "$RUNNER_TEMP/source.tar.gz" | cut -d' ' -f1)" python3 scripts/release/distribution.py homebrew --version "$VMAFX_VERSION" \ diff --git a/.github/workflows/tests-and-quality-gates.yml b/.github/workflows/tests-and-quality-gates.yml index b77aa568ba..5a24514182 100644 --- a/.github/workflows/tests-and-quality-gates.yml +++ b/.github/workflows/tests-and-quality-gates.yml @@ -1047,7 +1047,7 @@ jobs: mkdir -p "${RUNNER_TEMP}/ort-cache" tarball="${RUNNER_TEMP}/ort-cache/onnxruntime-linux-x64-gpu-${ORT_VERSION}.tgz" if [ ! -f "$tarball" ]; then - curl -fsSL --retry 3 --retry-delay 5 --max-time 600 -o "$tarball" \ + curl -fsSL --retry 3 --retry-all-errors --retry-delay 5 --max-time 600 -o "$tarball" \ "https://github.com/microsoft/onnxruntime/releases/download/v${ORT_VERSION}/onnxruntime-linux-x64-gpu-${ORT_VERSION}.tgz" fi if ! printf '%s %s\n' "${ORT_SHA256}" "$tarball" | sha256sum --check --strict; then diff --git a/.github/workflows/windows-tester-bundle.yml b/.github/workflows/windows-tester-bundle.yml index 739dd3f8d3..81f146baad 100644 --- a/.github/workflows/windows-tester-bundle.yml +++ b/.github/workflows/windows-tester-bundle.yml @@ -312,7 +312,7 @@ jobs: $pins[$name] = $line.Matches[0].Groups[1].Value } $exe = Join-Path $env:RUNNER_TEMP 'oneapi-basekit.exe' - curl.exe --fail --location --retry 5 --retry-delay 5 --output $exe --url $pins['INTEL_BASEKIT_WINDOWS_URL'] + curl.exe --fail --location --retry 5 --retry-all-errors --retry-delay 5 --output $exe --url $pins['INTEL_BASEKIT_WINDOWS_URL'] if ($LASTEXITCODE -ne 0) { throw "download failed ($LASTEXITCODE)" } $size = (Get-Item $exe).Length if ($size -ne [int64]$pins['INTEL_BASEKIT_WINDOWS_SIZE']) { throw "installer is $size bytes, not $($pins['INTEL_BASEKIT_WINDOWS_SIZE'])" } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f7ee76802f..7b5bd4d3e3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -836,6 +836,15 @@ repos: files: '^(\.github/workflows/|scripts/ci/(pr_workflow_budget\.py|tests/test_pr_workflow_budget\.py)|\.pre-commit-config\.yaml$)' pass_filenames: false + - id: test-fetch-connect-retry + name: HTTP fetches retry connection and host errors + entry: python3 -B scripts/ci/tests/test_fetch_connect_retry.py + language: system + files: '\.(sh|bash|ksh|zsh|fish|ps1|cmd|bat)$|(^|/)([^/]*\.)?(Dockerfile|Containerfile)(\.[^/]+)?$|^\.github/workflows/[^/]+\.ya?ml$|^scripts/ci/tests/test_fetch_connect_retry\.py$|^\.pre-commit-config\.yaml$' + # Shebangs can also identify shell scripts with arbitrary filenames. + always_run: true + pass_filenames: false + - id: check-api-doc-coverage name: Every public vmafx symbol is named in docs/api (#2436) entry: python3 scripts/ci/check_api_doc_coverage.py diff --git a/Containerfile.vmafx b/Containerfile.vmafx index 8b0a5884b0..0cf3295e1a 100644 --- a/Containerfile.vmafx +++ b/Containerfile.vmafx @@ -128,7 +128,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ # umf/tcm/tbb component dirs together without missing-dependency surprises. # The ~10 GB it installs never leaves this stage — stage 3 copies out only # the specific .so's `ldd` says vmaf/ffmpeg/libvmaf actually link (below). -RUN curl -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ +RUN curl --retry 3 --retry-all-errors -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | gpg --dearmor \ | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ && printf 'deb [signed-by=/usr/share/keyrings/intel-sw-products.gpg] https://apt.repos.intel.com/oneapi all main\n' \ diff --git a/Dockerfile b/Dockerfile index a87028c0c0..a11578c472 100644 --- a/Dockerfile +++ b/Dockerfile @@ -87,7 +87,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ if [ "$ENABLE_SYCL" = "true" ]; then \ apt-get update && apt-get install -y --no-install-recommends gnupg2 ca-certificates && \ - wget -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg && \ echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ > /etc/apt/sources.list.d/oneapi.list && \ @@ -101,7 +101,7 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ # ---------- nv-codec-headers ---------- WORKDIR /tmp/nv-codec -RUN wget -q "https://github.com/FFmpeg/nv-codec-headers/archive/${NV_CODEC_TAG}.zip" && \ +RUN wget --tries=3 --retry-connrefused --retry-on-host-error -q "https://github.com/FFmpeg/nv-codec-headers/archive/${NV_CODEC_TAG}.zip" && \ unzip -q "${NV_CODEC_TAG}.zip" WORKDIR /tmp/nv-codec/nv-codec-headers-${NV_CODEC_TAG} RUN make && make install @@ -129,7 +129,7 @@ RUN --mount=type=cache,target=/root/.cache/ccache,sharing=locked \ RUN echo /usr/local/lib/x86_64-linux-gnu > /etc/ld.so.conf.d/vmaf-local.conf && ldconfig # ---------- build FFmpeg ---------- -RUN wget -q "${FFMPEG_REMOTE%.git}/archive/${FFMPEG_TAG}.zip" && \ +RUN wget --tries=3 --retry-connrefused --retry-on-host-error -q "${FFMPEG_REMOTE%.git}/archive/${FFMPEG_TAG}.zip" && \ unzip -q "${FFMPEG_TAG}.zip" && rm "${FFMPEG_TAG}.zip" COPY ffmpeg-patches /tmp/ffmpeg-patches diff --git a/Dockerfile.ffmpeg b/Dockerfile.ffmpeg index 6188ea598c..a63f743095 100644 --- a/Dockerfile.ffmpeg +++ b/Dockerfile.ffmpeg @@ -17,7 +17,7 @@ ARG FFMPEG_REMOTE=https://github.com/FFmpeg/FFmpeg.git ARG NVCC_FLAGS="-gencode arch=compute_75,code=sm_75 -gencode arch=compute_80,code=sm_80 -gencode arch=compute_90,code=sm_90 -gencode arch=compute_120,code=sm_120 -gencode arch=compute_120,code=compute_120 -O2" # get ffmpeg -RUN wget -q "${FFMPEG_REMOTE%.git}/archive/${FFMPEG_TAG}.zip" && \ +RUN wget --tries=3 --retry-connrefused --retry-on-host-error -q "${FFMPEG_REMOTE%.git}/archive/${FFMPEG_TAG}.zip" && \ unzip -q "${FFMPEG_TAG}.zip" # Apply the same cumulative patch series as every other FFmpeg image: the diff --git a/changelog.d/fixed/fetch-connect-retry.md b/changelog.d/fixed/fetch-connect-retry.md new file mode 100644 index 0000000000..58fd86fc9b --- /dev/null +++ b/changelog.d/fixed/fetch-connect-retry.md @@ -0,0 +1 @@ +- **CI:** a refused or unreachable connection no longer ends a CI download on the first attempt: curl and wget fetches in shell scripts, workflows and container recipes retry connection and host-resolution failures, the tidy lane's APT downloads retry, and a contract test checks every fetch for the retry flags. diff --git a/dev/Containerfile b/dev/Containerfile index 8209cf13da..5f7c57fa3a 100644 --- a/dev/Containerfile +++ b/dev/Containerfile @@ -275,7 +275,7 @@ RUN update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-13 100 \ # ClangBuildAnalyzer (Unlicense, ADR-3336) build-time analysis ARG CLANGBUILDANALYZER_VERSION=1.6.0 ARG CLANGBUILDANALYZER_SHA256=868a8d34ecb9b65da4e5874342062a12c081ce4385c7ddd6ce7d557a0c5c292d -RUN curl -fsSL \ +RUN curl --retry 3 --retry-all-errors -fsSL \ "https://github.com/aras-p/ClangBuildAnalyzer/archive/refs/tags/v${CLANGBUILDANALYZER_VERSION}.tar.gz" \ -o /tmp/cba.tar.gz \ && echo "${CLANGBUILDANALYZER_SHA256} /tmp/cba.tar.gz" | sha256sum -c - \ @@ -352,7 +352,7 @@ RUN for attempt in 1 2 3; do \ # AlmaLinux 8's assembler predates the AVX-512 sources. Build the maintained # assembler from the upstream release tarball, verified before extraction. -RUN curl --fail --location --show-error \ +RUN curl --retry 3 --retry-all-errors --fail --location --show-error \ --output /tmp/nasm.tar.xz \ "https://www.nasm.us/pub/nasm/releasebuilds/${NATIVE_NASM_VERSION}/nasm-${NATIVE_NASM_VERSION}.tar.xz" \ && echo "${NATIVE_NASM_SHA256} /tmp/nasm.tar.xz" | sha256sum --check --strict \ @@ -415,7 +415,7 @@ ENV LD_LIBRARY_PATH=${CUDA_HOME}/lib64 # does not publish year-quarter-versioned meta-packages (e.g., # `intel-basekit-2025.3` does not exist in the apt repo); the versioning is # tracked internally by the component packages the meta-package pulls in. -RUN curl -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ +RUN curl --retry 3 --retry-all-errors -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ | gpg --dearmor \ | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ && printf 'deb [signed-by=/usr/share/keyrings/intel-sw-products.gpg] https://apt.repos.intel.com/oneapi all main\n' \ @@ -498,8 +498,8 @@ RUN . /opt/vmafx/build-config.env \ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ . /opt/vmafx/build-config.env \ - && curl -fsSL "https://github.com/oneapi-src/level-zero/releases/download/v${LEVEL_ZERO_VERSION}/libze1_${LEVEL_ZERO_VERSION}%2Bu24.04_amd64.deb" -o /tmp/libze1.deb \ - && curl -fsSL "https://github.com/oneapi-src/level-zero/releases/download/v${LEVEL_ZERO_VERSION}/libze-dev_${LEVEL_ZERO_VERSION}%2Bu24.04_amd64.deb" -o /tmp/libze-dev.deb \ + && curl --retry 3 --retry-all-errors -fsSL "https://github.com/oneapi-src/level-zero/releases/download/v${LEVEL_ZERO_VERSION}/libze1_${LEVEL_ZERO_VERSION}%2Bu24.04_amd64.deb" -o /tmp/libze1.deb \ + && curl --retry 3 --retry-all-errors -fsSL "https://github.com/oneapi-src/level-zero/releases/download/v${LEVEL_ZERO_VERSION}/libze-dev_${LEVEL_ZERO_VERSION}%2Bu24.04_amd64.deb" -o /tmp/libze-dev.deb \ && apt-get update \ && apt-get install -y --no-install-recommends /tmp/libze1.deb /tmp/libze-dev.deb \ && find /tmp -maxdepth 1 -name 'libze*.deb' -delete @@ -516,7 +516,7 @@ ARG RUSTUP_INIT_SHA256=dda7234360b7f578ca8b0ddcb80145646fa61a67c1720a5abc7051b35 ENV RUSTUP_HOME=/opt/rust/rustup \ CARGO_HOME=/opt/rust/cargo RUN . /opt/vmafx/build-config.env \ - && curl -fsSL "https://static.rust-lang.org/rustup/archive/${RUSTUP_INIT_VERSION}/x86_64-unknown-linux-gnu/rustup-init" -o /tmp/rustup-init \ + && curl --retry 3 --retry-all-errors -fsSL "https://static.rust-lang.org/rustup/archive/${RUSTUP_INIT_VERSION}/x86_64-unknown-linux-gnu/rustup-init" -o /tmp/rustup-init \ && echo "${RUSTUP_INIT_SHA256} /tmp/rustup-init" | sha256sum -c - \ && chmod +x /tmp/rustup-init \ && /tmp/rustup-init -y --no-modify-path --profile minimal \ @@ -739,7 +739,7 @@ RUN chown vmaf:vmaf /build /build/vmaf # by ai/pyproject.toml. ARG ORT_VERSION=1.30.0 ARG ORT_SHA256=a5ed5a3cac51fbb2e90da632ae43d19212faaa20e76484e62bcb7c23ddb3b3fd -RUN curl -fsSL \ +RUN curl --retry 3 --retry-all-errors -fsSL \ "https://github.com/microsoft/onnxruntime/releases/download/v${ORT_VERSION}/onnxruntime-linux-x64-${ORT_VERSION}.tgz" \ -o /tmp/ort.tgz \ && echo "${ORT_SHA256} /tmp/ort.tgz" | sha256sum --check --strict - \ @@ -786,11 +786,11 @@ USER root # (`nv-codec-headers` from code.ffmpeg.org, `nv-codec-headers-` from # GitHub), so make targets the extracted directory rather than a fixed name. ARG NV_CODEC_HEADERS_REF=n13.1.15.0 -RUN { curl -fsSL --retry 1 --retry-delay 2 --connect-timeout 10 --max-time 60 \ +RUN { curl -fsSL --retry 1 --retry-all-errors --retry-delay 2 --connect-timeout 10 --max-time 60 \ "https://code.ffmpeg.org/FFmpeg/nv-codec-headers/archive/${NV_CODEC_HEADERS_REF}.tar.gz" \ -o /tmp/nv-codec-headers.tgz \ || { echo "nv-codec-headers: code.ffmpeg.org unreachable, falling back to the GitHub mirror" >&2; \ - curl -fsSL --retry 3 --retry-delay 2 --connect-timeout 20 --max-time 120 \ + curl -fsSL --retry 3 --retry-all-errors --retry-delay 2 --connect-timeout 20 --max-time 120 \ "https://github.com/FFmpeg/nv-codec-headers/archive/refs/tags/${NV_CODEC_HEADERS_REF}.tar.gz" \ -o /tmp/nv-codec-headers.tgz; }; } \ && tar xzf /tmp/nv-codec-headers.tgz -C /tmp \ diff --git a/dev/Containerfile.runner b/dev/Containerfile.runner index 708eb2abe8..54cc739309 100644 --- a/dev/Containerfile.runner +++ b/dev/Containerfile.runner @@ -33,7 +33,7 @@ RUN (getent group render >/dev/null || groupadd -g 988 render 2>/dev/null || gro # Download and verify official actions-runner tarball RUN mkdir -p /actions-runner \ && cd /actions-runner \ - && curl -fsSL "https://github.com/actions/runner/releases/download/v${RUNNER_VERSION}/actions-runner-linux-x64-${RUNNER_VERSION}.tar.gz" -o runner.tar.gz \ + && curl --retry 3 --retry-all-errors -fsSL "https://github.com/actions/runner/releases/download/v${RUNNER_VERSION}/actions-runner-linux-x64-${RUNNER_VERSION}.tar.gz" -o runner.tar.gz \ && echo "${RUNNER_SHA256} runner.tar.gz" | sha256sum -c - \ && tar xzf runner.tar.gz \ && rm -f runner.tar.gz \ diff --git a/docker/Dockerfile.tester b/docker/Dockerfile.tester index 717f74194c..408c3040ab 100644 --- a/docker/Dockerfile.tester +++ b/docker/Dockerfile.tester @@ -43,7 +43,7 @@ RUN apt-get update \ && rm -rf /var/lib/apt/lists/* COPY tools/rc1-tester/image/fixtures.sha256 /fixtures.sha256 WORKDIR /out -RUN awk '{print $2}' /fixtures.sha256 | xargs -P4 -I{} curl -fsSL --retry 3 --create-dirs \ +RUN awk '{print $2}' /fixtures.sha256 | xargs -P4 -I{} curl -fsSL --retry 3 --retry-all-errors --create-dirs \ -o {} "https://raw.githubusercontent.com/Netflix/vmaf_resource/${VMAF_RESOURCE_COMMIT}/python/test/resource/{}" \ && sha256sum --check --strict /fixtures.sha256 \ && chmod -R a+rX /out diff --git a/docs/development/ci.md b/docs/development/ci.md index cee697c4e0..0fb745a9f5 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -1152,6 +1152,29 @@ first earlier step that did not succeed (`shard 1/4 outcome was skipped: step Meson-unit-suite outcome was failure`), and `scripts/ci/test_fail_closed_ci.py` runs that step's script against planted outcomes. +### Connection retries on downloads + +On 2026-11-10 the `Tidy Lane (cuda)` job failed on its first attempt to fetch +the apt.llvm.org signing key: `curl: (7) Failed to connect to apt.llvm.org port +443`. The fetch had `--retry 3`, but curl's `--retry` alone does not retry a +failed connection, and `--retry-connrefused` adds only a refused one, not an +unreachable network or host. + +Every HTTP(S) fetch in a shell script, workflow or container recipe therefore +retries connection failures: + +- **curl:** `--retry N --retry-all-errors`, with N positive. +- **wget:** `--tries=N --retry-connrefused --retry-on-host-error`, with N + positive. +- **APT in `scripts/dev/tidy-lane.sh`:** `-o Acquire::Retries=3`. + +Each site keeps its own `--max-time` and `--connect-timeout` bounds. +`scripts/ci/tests/test_fetch_connect_retry.py` checks every tracked fetch for +these flags and runs as the `test-fetch-connect-retry` pre-commit hook. Fetches +that must fail fast, such as a local health probe inside its own bounded poll, +are listed with a reason in its `ALLOWLIST`; an entry whose site is gone fails +the test. + ## Apt archive cache Every job in `tests-and-quality-gates.yml` that runs `apt-get install` caches diff --git a/scripts/ai/fetch-tiny-blobs.sh b/scripts/ai/fetch-tiny-blobs.sh index 8aa7dbb4b3..326003d45b 100755 --- a/scripts/ai/fetch-tiny-blobs.sh +++ b/scripts/ai/fetch-tiny-blobs.sh @@ -111,7 +111,7 @@ fetch_one() { tmp="$(mktemp "$dest.XXXXXX")" _TINY_STAGING_FILES+=("$tmp") if ! curl --fail --silent --show-error --location \ - --max-time 600 --retry 3 --retry-connrefused \ + --max-time 600 --retry 3 --retry-all-errors \ --output "$tmp" "$url"; then rm -f "$tmp" # Pop the just-cleaned path so the EXIT trap doesn't double-rm. diff --git a/scripts/ci/build-macos-tester-bundle.sh b/scripts/ci/build-macos-tester-bundle.sh index 3cd122cabf..d7610d507f 100755 --- a/scripts/ci/build-macos-tester-bundle.sh +++ b/scripts/ci/build-macos-tester-bundle.sh @@ -105,13 +105,13 @@ step "fixtures (pinned commit, SHA-256 checked)" python3 "$image_dir/prepare_build.py" fixtures "$image_dir/fixtures.sha256" \ "$image_dir/fixtures.json" >"$out/fixtures-needed.sha256" while read -r _ file; do - curl -fsSL --max-time 600 --retry 3 --create-dirs -o "$resource/$file" \ + curl -fsSL --max-time 600 --retry 3 --retry-all-errors --create-dirs -o "$resource/$file" \ "https://raw.githubusercontent.com/Netflix/vmaf_resource/$VMAF_RESOURCE_COMMIT/python/test/resource/$file" done <"$out/fixtures-needed.sha256" (cd "$resource" && shasum -a 256 -c "$out/fixtures-needed.sha256") step "interpreter (python-build-standalone, SHA-256 checked)" -curl -fsSL --max-time 600 --retry 3 -o "$out/pbs.tar.gz" "$PBS_URL" +curl -fsSL --max-time 600 --retry 3 --retry-all-errors -o "$out/pbs.tar.gz" "$PBS_URL" echo "$PBS_SHA256 $out/pbs.tar.gz" | shasum -a 256 -c - mkdir "$out/pbs" tar -xzf "$out/pbs.tar.gz" -C "$out/pbs" @@ -141,7 +141,7 @@ step "licence texts (python-build-standalone full archive, CPython Doc/license.r # The install_only archive holds no licence texts; the full archive of the same # release has them for every library linked into the interpreter (ADR-1503). mkdir -p "$texts/python-build-standalone" -curl -fsSL --max-time 600 --retry 3 -o "$out/pbs-full.tar.zst" "$PBS_FULL_URL" +curl -fsSL --max-time 600 --retry 3 --retry-all-errors -o "$out/pbs-full.tar.zst" "$PBS_FULL_URL" echo "$PBS_FULL_SHA256 $out/pbs-full.tar.zst" | shasum -a 256 -c - zstd -dc "$out/pbs-full.tar.zst" | tar -xf - -C "$out" python/licenses python/PYTHON.json mv "$out"/python/licenses/* "$out/python/PYTHON.json" "$texts/python-build-standalone/" diff --git a/scripts/ci/ffmpeg-shared-series.sh b/scripts/ci/ffmpeg-shared-series.sh index 2179f4ea00..6d766d1d95 100755 --- a/scripts/ci/ffmpeg-shared-series.sh +++ b/scripts/ci/ffmpeg-shared-series.sh @@ -251,12 +251,12 @@ download() { # download URL DESTINATION ;; https://*) if command -v curl >/dev/null 2>&1; then - run_with_deadline "$DOWNLOAD_SECONDS" curl --fail --silent --show-error --location --retry 3 \ + run_with_deadline "$DOWNLOAD_SECONDS" curl --fail --silent --show-error --location --retry 3 --retry-all-errors \ --max-time "$DOWNLOAD_SECONDS" --output "$destination" "$url" || release_fallback "$url" "$destination" || die 1 "download failed: $url" elif command -v wget >/dev/null 2>&1; then - run_with_deadline "$DOWNLOAD_SECONDS" wget --quiet --tries=3 --timeout="$DOWNLOAD_SECONDS" --output-document="$destination" "$url" || + run_with_deadline "$DOWNLOAD_SECONDS" wget --quiet --tries=3 --retry-connrefused --retry-on-host-error --timeout="$DOWNLOAD_SECONDS" --output-document="$destination" "$url" || release_fallback "$url" "$destination" || die 1 "download failed: $url" else diff --git a/scripts/ci/install-cuda-toolkit.ps1 b/scripts/ci/install-cuda-toolkit.ps1 index 76f9f4a801..5544cd147a 100644 --- a/scripts/ci/install-cuda-toolkit.ps1 +++ b/scripts/ci/install-cuda-toolkit.ps1 @@ -60,7 +60,7 @@ $manifestUrl = "$base/redistrib_$cudaVersion.json" $manifestFile = Join-Path $env:RUNNER_TEMP "redistrib_$cudaVersion.json" Write-Host "install-cuda-toolkit: CUDA $cudaVersion from $manifestUrl" -curl.exe --fail --location --silent --show-error --retry 5 --retry-delay 10 ` +curl.exe --fail --location --silent --show-error --retry 5 --retry-all-errors --retry-delay 10 ` --output $manifestFile $manifestUrl if ($LASTEXITCODE -ne 0) { throw "install-cuda-toolkit: no redistrib manifest for CUDA $cudaVersion at $manifestUrl" @@ -94,7 +94,7 @@ foreach ($component in $components) { $relative = $platform.relative_path $archive = Join-Path $env:RUNNER_TEMP (Split-Path -Leaf $relative) Write-Host " $component $($entry.version) -> $(Split-Path -Leaf $relative)" - curl.exe --fail --location --silent --show-error --retry 5 --retry-delay 10 ` + curl.exe --fail --location --silent --show-error --retry 5 --retry-all-errors --retry-delay 10 ` --output $archive "$base/$relative" if ($LASTEXITCODE -ne 0) { throw "install-cuda-toolkit: download failed for $base/$relative" diff --git a/scripts/ci/install-cuda-toolkit.sh b/scripts/ci/install-cuda-toolkit.sh index 633485c552..e0841ff46d 100755 --- a/scripts/ci/install-cuda-toolkit.sh +++ b/scripts/ci/install-cuda-toolkit.sh @@ -181,7 +181,7 @@ if ! command -v curl >/dev/null 2>&1 || ! dpkg -s ca-certificates >/dev/null 2>& $SUDO apt-get install -y -qq --no-install-recommends curl ca-certificates fi -if ! curl -fsSI --connect-timeout 20 --max-time 60 "${base}/cuda-keyring_1.1-1_all.deb" >/dev/null; then +if ! curl --retry 3 --retry-all-errors -fsSI --connect-timeout 20 --max-time 60 "${base}/cuda-keyring_1.1-1_all.deb" >/dev/null; then echo "::error::install-cuda-toolkit: NVIDIA publishes no CUDA repository for '${distro}'." \ "Pin the runner to a release NVIDIA serves, or add the mapping here." >&2 exit 1 @@ -189,7 +189,7 @@ fi tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT -curl -fsSL --connect-timeout 20 --max-time 300 -o "$tmp/cuda-keyring.deb" "${base}/cuda-keyring_1.1-1_all.deb" +curl --retry 3 --retry-all-errors -fsSL --connect-timeout 20 --max-time 300 -o "$tmp/cuda-keyring.deb" "${base}/cuda-keyring_1.1-1_all.deb" $SUDO dpkg -i "$tmp/cuda-keyring.deb" $SUDO apt-get update -qq diff --git a/scripts/ci/install-intel-oneapi.sh b/scripts/ci/install-intel-oneapi.sh index 2aaa215105..67ce75b4e7 100755 --- a/scripts/ci/install-intel-oneapi.sh +++ b/scripts/ci/install-intel-oneapi.sh @@ -132,7 +132,7 @@ trap 'rm -rf "$tmp"' EXIT # --- repository key: keep exactly the pinned key -------------------------- key_url="https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB" keyring=/usr/share/keyrings/intel-oneapi-archive-keyring.gpg -curl -fsSL --connect-timeout 20 --max-time 120 --proto '=https' --tlsv1.2 -o "$tmp/intel.pub" "$key_url" +curl --retry 3 --retry-all-errors -fsSL --connect-timeout 20 --max-time 120 --proto '=https' --tlsv1.2 -o "$tmp/intel.pub" "$key_url" export GNUPGHOME="$tmp/gnupg" mkdir -m 0700 "$GNUPGHOME" # Without gpg-agent (a Recommends of gpg) the import still stores the public diff --git a/scripts/ci/install-llvm.sh b/scripts/ci/install-llvm.sh index 1a1f4457f8..66bb84e322 100755 --- a/scripts/ci/install-llvm.sh +++ b/scripts/ci/install-llvm.sh @@ -59,7 +59,7 @@ trap 'exit 130' INT trap 'exit 143' TERM installer=$(mktemp) tmp_files+=("$installer") -wget -qO "$installer" --tries=5 --retry-connrefused --waitretry=5 --timeout=30 \ +wget -qO "$installer" --tries=5 --retry-connrefused --retry-on-host-error --waitretry=5 --timeout=30 \ https://apt.llvm.org/llvm.sh if ! printf '%s %s\n' "$LLVM_INSTALLER_SHA256" "$installer" | sha256sum --check --status; then echo "install-llvm: llvm.sh SHA-256 mismatch; refusing to execute" >&2 diff --git a/scripts/ci/install-rocm-from-image.sh b/scripts/ci/install-rocm-from-image.sh index e09dc00715..017a76cee1 100755 --- a/scripts/ci/install-rocm-from-image.sh +++ b/scripts/ci/install-rocm-from-image.sh @@ -189,7 +189,7 @@ extract_layer() { local digest="$1" attempt=1 statuses while [ "$attempt" -le "$LAYER_ATTEMPTS" ]; do set +e - curl -fsSL --max-time "$LAYER_MAX_TIME" \ + curl --retry 3 --retry-all-errors -fsSL --max-time "$LAYER_MAX_TIME" \ -H "Authorization: Bearer ${TOKEN}" \ "https://${REGISTRY}/v2/${REPO}/blobs/${digest}" | "${decomp[@]}" | diff --git a/scripts/ci/pinned-tool.sh b/scripts/ci/pinned-tool.sh index 36afb00a26..439ed3518a 100755 --- a/scripts/ci/pinned-tool.sh +++ b/scripts/ci/pinned-tool.sh @@ -59,7 +59,7 @@ dir="${XDG_CACHE_HOME:-$HOME/.cache}/vmafx-tools/$tool/$version" tarball="$dir/$tool.tar.gz" mkdir -p "$dir" if [[ ! -f "$tarball" ]]; then - curl -sSfL --retry 3 --max-time 300 -o "$tarball.part" "$url" + curl -sSfL --retry 3 --retry-all-errors --max-time 300 -o "$tarball.part" "$url" mv "$tarball.part" "$tarball" fi if ! echo "$sha $tarball" | sha256sum --check --status; then diff --git a/scripts/ci/tests/test_fetch_connect_retry.py b/scripts/ci/tests/test_fetch_connect_retry.py new file mode 100644 index 0000000000..c52380efe3 --- /dev/null +++ b/scripts/ci/tests/test_fetch_connect_retry.py @@ -0,0 +1,405 @@ +#!/usr/bin/env python3 +# Copyright 2026 Lusoris +# SPDX-License-Identifier: EUPL-1.2 +"""Offline contract for connection retries at every tracked shell fetch site.""" + +from __future__ import annotations + +import re +import shlex +import shutil +import sys +import tempfile +import unittest +from collections import Counter +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[3] +sys.path.insert(0, str(ROOT)) + +from scripts.lib.safe_subprocess import run as run_command # noqa: E402 + +# Exact path + URL argument + reason. No implicit loopback exemption: a new +# probe must demonstrate its own bounded poll, or why repeating it is incorrect. +ALLOWLIST = ( + ( + "scripts/ci/observability-compose-smoke.sh", + "http://localhost:9090/api/v1/query?query=up%7Bjob%3D%22vmafx-node%22%7D%3D%3D1", + "node_scraped runs inside a 90-attempt outer poll; final assertion must fail fast", + ), + ( + "scripts/release/smoke-published-image.sh", + "http://$address$path", + "wait_ready repeats all endpoints at most READY_TRIES times", + ), + ( + ".github/workflows/docker-publish-production.yml", + "http://127.0.0.1:18080/healthz", + "health check already has a 30-attempt outer poll", + ), + ( + ".github/workflows/docker-publish-production.yml", + "http://127.0.0.1:18080/readyz", + "single readiness assertion after bounded health poll; failure must stop the smoke", + ), + ( + "scripts/git-hooks/pre-push-pr-body-lint.sh", + "https://github.com/${repository}/pulls", + "bounded fallback lookup returns 2 to its caller, which handles lookup failure", + ), + ( + "scripts/git-hooks/pre-push-pr-body-lint.sh", + "https://github.com/${repository}/pull/${pull_number}", + "bounded fallback page lookup returns 2 to its caller, which handles lookup failure", + ), + ( + "test/e2e/score-smoke.sh", + "http://127.0.0.1:${LOCAL_PORT}/readyz", + "readiness check already has a 60-attempt outer poll", + ), + ( + "test/e2e/score-smoke.sh", + "http://127.0.0.1:${LOCAL_PORT}/v1/score", + "single non-idempotent scoring POST through the local port-forward; fail fast", + ), +) +FETCH = re.compile(r"(? bool: + """Shell extensions/shebangs, Actions workflows, and container recipe names.""" + name = Path(path) + return ( + name.suffix in SHELL_SUFFIXES + or bool(SHELL_HEADER.match(data)) + or (path.startswith(".github/workflows/") and name.suffix in (".yml", ".yaml")) + or bool(re.search(r"(?:^|\.)(?:Dockerfile|Containerfile)(?:\.|$)", name.name)) + ) + + +def tracked_sources() -> list[tuple[str, str]]: + """Read Git's tracked files only; never launch a fetch or execute a scanned script.""" + git = shutil.which("git") + if git is None: + raise AssertionError("git is required to enumerate tracked fetch sites") + result = run_command( + [git, "ls-files", "-z"], + allowed_executables=[git], + cwd=ROOT, + capture_output=True, + text=True, + check=True, + timeout_seconds=30, + ) + sources = [] + for path in result.stdout.split("\0"): + source = ROOT / path + if not path or not source.is_file() or source.is_symlink(): + continue + # A shebang can make an extensionless file a shell script. Decode with + # replacement so binary assets need no special extension allow-list. + with source.open("rb") as handle: + header = handle.read(128) + if in_scope(path, header.decode("utf-8", errors="replace")): + sources.append((path, source.read_text(encoding="utf-8"))) + return sources + + +def logical_lines(text: str) -> list[tuple[int, str]]: + """Join shell, PowerShell, and cmd continuations without losing source locations.""" + lines = [] + pending = "" + start = 1 + for number, line in enumerate(text.splitlines(), 1): + if not pending: + start = number + if line.endswith(("\\", "`", "^")): + pending += line[:-1] + " " + else: + lines.append((start, pending + line)) + pending = "" + if pending: + lines.append((start, pending)) + return lines + + +def command_args(command: str) -> list[str]: + """Tokenize one fetch, stopping before the next shell command or enclosing $().""" + lexer = shlex.shlex(command, posix=True, punctuation_chars=";&|()") + lexer.whitespace_split = True + try: + tokens = list(lexer) + except ValueError: + # Embedded shell/YAML fragments may end inside an enclosing quote. + # Keep auditing the whole fetch rather than discarding that line. + tokens = [token.strip("\"'") for token in command.split()] + args = [] + for token in tokens: + if token in CONTROL or token.startswith("#"): + break + args.append(token) + return args + + +def fetch_commands(text: str) -> list[tuple[int, list[str]]]: + """Inventory literal and variable URL fetches on every logical source line.""" + commands = [] + for line_number, line in logical_lines(text): + if line.lstrip().startswith("#"): + continue + for match in FETCH.finditer(line): + args = command_args(line[match.start() :]) + # Include variable URLs (including Windows %URL%), not just literal + # hosts. Package lists and command -v checks have no URL arguments. + if any(re.match(r"https?://|\$|%", arg) for arg in args[1:]): + commands.append((line_number, args)) + return commands + + +def option_value(args: list[str], option: str) -> str: + """Read the last value supplied with a long option.""" + value = "" + for index, arg in enumerate(args): + if arg.startswith(option + "="): + value = arg.partition("=")[2] + elif arg == option: + value = args[index + 1] if index + 1 < len(args) else "" + return value + + +def positive_count(args: list[str], option: str) -> bool: + """Accept only literal positive integer retry counts.""" + return bool(re.fullmatch(r"[1-9][0-9]*", option_value(args, option))) + + +def enabled(args: list[str], option: str) -> bool: + """Honor the last explicit enable or disable switch.""" + switches = [arg for arg in args if arg in (option, option.replace("--", "--no-", 1))] + return bool(switches) and switches[-1] == option + + +def retry_errors(args: list[str]) -> list[str]: + """Require positive retries and the fetch tool's connection error switches.""" + if args[0] == "wget": + if ( + positive_count(args, "--tries") + and enabled(args, "--retry-connrefused") + and enabled(args, "--retry-on-host-error") + ): + return [] + return ["wget needs positive --tries, --retry-connrefused and --retry-on-host-error"] + errors = [] + if not positive_count(args, "--retry"): + errors.append("curl needs positive --retry") + if not enabled(args, "--retry-all-errors"): + errors.append("curl needs --retry-all-errors") + return errors + + +def matching_exemptions( + path: str, args: list[str], allowlist: tuple[tuple[str, str, str], ...] +) -> list[tuple[str, str, str]]: + """Match exemptions by both tracked path and complete URL argument.""" + return [entry for entry in allowlist if entry[0] == path and entry[1] in args] + + +def audit_sources( + sources: list[tuple[str, str]], allowlist: tuple[tuple[str, str, str], ...] = ALLOWLIST +) -> list[str]: + """Audit fetch arguments and reject stale or ambiguous allow-list entries.""" + errors = [] + seen: Counter[tuple[str, str, str]] = Counter() + for path, text in sources: + for line, args in fetch_commands(text): + matches = matching_exemptions(path, args, allowlist) + if matches: + seen.update(matches) + else: + errors.extend(f"{path}:{line}: {error}" for error in retry_errors(args)) + for entry in allowlist: + if not entry[2] or seen[entry] != 1: + errors.append( + f"{entry[0]}: stale/ambiguous allow-list entry ({seen[entry]}): {entry[1]}" + ) + return errors + + +class FetchConnectRetryTests(unittest.TestCase): + def check_script(self, text: str) -> list[str]: + """Audit a temporary shell source without executing its commands.""" + with tempfile.TemporaryDirectory() as temporary: + script = Path(temporary) / "fetch.sh" + script.write_text(text, encoding="utf-8") + return audit_sources([(script.name, script.read_text(encoding="utf-8"))], ()) + + def test_compliant_temporary_script(self) -> None: + """Accept curl and wget with every required retry option.""" + self.assertEqual( + self.check_script( + "curl -fsSL --retry 3 --retry-all-errors https://example.invalid/x\n" + "wget -q --tries=3 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x\n" + ), + [], + ) + + def test_plain_retry_does_not_cover_refused_connection(self) -> None: + """Reject curl retries that omit all-errors coverage.""" + self.assertTrue(self.check_script("curl -fsSL --retry 3 https://example.invalid/x\n")) + + def test_curl_retry_connrefused_alone_does_not_cover_unreachable_connection(self) -> None: + """Reject refused-only curl retries because other exit-7 causes remain fatal.""" + self.assertEqual( + self.check_script("curl --retry 3 --retry-connrefused https://example.invalid/x\n"), + ["fetch.sh:1: curl needs --retry-all-errors"], + ) + + def test_wget_without_tries_fails(self) -> None: + """Reject wget without an explicit positive attempt count.""" + self.assertTrue( + self.check_script( + "wget --retry-connrefused --retry-on-host-error https://example.invalid/x\n" + ) + ) + + def test_wget_without_retry_connrefused_fails(self) -> None: + """Reject wget without retries for refused or unreachable connections.""" + self.assertTrue( + self.check_script("wget --tries=3 --retry-on-host-error https://example.invalid/x\n") + ) + + def test_wget_without_retry_on_host_error_fails(self) -> None: + """Reject wget without host resolution retries.""" + self.assertTrue( + self.check_script("wget --tries=3 --retry-connrefused https://example.invalid/x\n") + ) + + def test_existing_all_errors_retry_is_accepted(self) -> None: + """Accept the existing curl all-errors retry convention.""" + self.assertEqual( + self.check_script("curl -f --retry 5 --retry-all-errors https://example.invalid/x\n"), + [], + ) + + def test_continuation_flags_count(self) -> None: + """Find retry flags across supported logical line continuations.""" + for continuation in ("\\", "`", "^"): + with self.subTest(continuation=continuation): + self.assertEqual( + self.check_script( + f"curl -fsSL --retry 3 {continuation}\n" + f" --retry-all-errors {continuation}\n" + " https://example.invalid/x\n" + ), + [], + ) + + def test_loopback_without_exact_allowlist_entry_fails(self) -> None: + """Require retries for loopback fetches unless explicitly exempted.""" + self.assertTrue(self.check_script("curl -fsSL http://localhost:8080/healthz\n")) + self.assertTrue(self.check_script("wget -q http://127.0.0.1:8080/healthz\n")) + + def test_allowlist_is_exact_and_stale_entries_fail(self) -> None: + """Reject missing, duplicated, or mismatched exact exemptions.""" + entry = (("fetch.sh", "http://localhost:8080/healthz", "bounded outer poll"),) + source = "curl -fsSL http://localhost:8080/healthz\n" + self.assertEqual(audit_sources([("fetch.sh", source)], entry), []) + self.assertTrue(audit_sources([], entry)) + self.assertTrue(audit_sources([("other.sh", source)], entry)) + self.assertTrue(audit_sources([("fetch.sh", source * 2)], entry)) + self.assertTrue( + audit_sources([("fetch.sh", source.replace("healthz", "healthz/new"))], entry) + ) + + def test_flags_on_next_command_or_comment_do_not_count(self) -> None: + """Keep comments and subsequent commands outside fetch arguments.""" + for suffix in (" # --retry-all-errors", " | echo --retry-all-errors"): + with self.subTest(suffix=suffix): + self.assertTrue( + self.check_script("curl --retry 3 https://example.invalid/x" + suffix) + ) + + def test_retry_must_be_enabled_and_positive(self) -> None: + """Reject missing, nonpositive, noninteger, and disabled curl retries.""" + compliant = "curl --retry 3 --retry-all-errors https://example.invalid/x" + for mutation in ( + compliant.replace("--retry 3", ""), + compliant.replace("--retry 3", "--retry 0"), + compliant.replace("--retry 3", "--retry -1"), + compliant.replace("--retry 3", "--retry 1.5"), + compliant.replace("--retry-all-errors", "--retry-all-errors --no-retry-all-errors"), + ): + with self.subTest(mutation=mutation): + self.assertTrue(self.check_script(mutation)) + + def test_wget_retry_must_be_enabled_and_positive(self) -> None: + """Reject zero attempts and disabled wget connection or host retries.""" + compliant = ( + "wget --tries=3 --retry-connrefused --retry-on-host-error https://example.invalid/x" + ) + for mutation in ( + compliant.replace("--tries=3", "--tries=0"), + compliant.replace("--retry-connrefused", "--no-retry-connrefused"), + compliant.replace("--retry-on-host-error", "--no-retry-on-host-error"), + ): + with self.subTest(mutation=mutation): + self.assertTrue(self.check_script(mutation)) + + def test_unbalanced_quote_uses_whitespace_split_without_skipping_line(self) -> None: + """Audit malformed logical lines instead of crashing or silently skipping fetches.""" + for url in ('https://example.invalid/x"', "'https://example.invalid/x"): + with self.subTest(url=url): + self.assertEqual( + self.check_script(f"curl --retry 3 {url}\n"), + ["fetch.sh:1: curl needs --retry-all-errors"], + ) + self.assertEqual( + self.check_script(f"curl --retry 3 --retry-all-errors {url}\n"), [] + ) + + def test_fetch_names_in_diagnostic_prose_are_not_commands(self) -> None: + """Exclude tool names inside diagnostics while retaining real bare-URL fetches.""" + self.assertEqual( + self.check_script('die 2 "curl or wget is required to download $url"\n'), [] + ) + self.assertTrue(self.check_script("curl https://example.invalid/x\n")) + self.assertTrue(self.check_script("wget https://example.invalid/x\n")) + + def test_variable_urls_windows_and_container_fetches(self) -> None: + """Inventory variable URLs in shell, Windows, and container commands.""" + for command in ( + 'http_code="$(curl --retry 3 "$URL")"', + 'RUN curl -fsSL "$url" && echo done', + "curl.exe --url %WINDOWS_BASEKIT_URL% --retry 5", + ): + with self.subTest(command=command): + self.assertTrue(self.check_script(command)) + + def test_scanned_file_kinds(self) -> None: + """Include supported shell and recipe paths while excluding prose files.""" + for path in ( + "scripts/fetch.sh", + "fetch.bash", + "scripts/fetch.ps1", + "fetch.cmd", + ".github/workflows/fetch.yml", + ".github/workflows/fetch.yaml", + "Dockerfile", + "dev/Containerfile.runner", + "docker/dev/alpine.Dockerfile", + ): + with self.subTest(path=path): + self.assertTrue(in_scope(path, "")) + self.assertTrue(in_scope("fetch", "#!/usr/bin/env bash\n")) + self.assertFalse(in_scope("README.md", "curl https://example.invalid/x")) + + def test_tracked_fetches_retry_connect_failures(self) -> None: + """Enforce the retry contract across the live tracked site inventory.""" + self.assertEqual(audit_sources(tracked_sources()), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/dev/tidy-lane.sh b/scripts/dev/tidy-lane.sh index eda1b7563b..c02e1943de 100755 --- a/scripts/dev/tidy-lane.sh +++ b/scripts/dev/tidy-lane.sh @@ -213,13 +213,13 @@ ensure_clang_tidy() { local key=/etc/apt/trusted.gpg.d/apt.llvm.org.asc codename # shellcheck disable=SC1091 # the image's own os-release codename="$(. /etc/os-release && printf '%s' "$VERSION_CODENAME")" - curl -fsSL --retry 3 --max-time 60 "$LLVM_APT_SIGNER_URL" -o "$key" + curl -fsSL --retry 3 --retry-all-errors --max-time 60 "$LLVM_APT_SIGNER_URL" -o "$key" printf '%s %s\n' "$LLVM_APT_SIGNER_SHA256" "$key" | sha256sum --check --status || die "apt.llvm.org key does not match the pinned SHA-256; refusing to install" printf 'deb [signed-by=%s] https://apt.llvm.org/%s/ llvm-toolchain-%s-%s main\n' \ "$key" "$codename" "$codename" "$CLANG_TIDY_MAJOR" >/etc/apt/sources.list.d/llvm-tidy.list - DEBIAN_FRONTEND=noninteractive apt-get update -qq - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ "clang-tidy-${CLANG_TIDY_MAJOR}" >/dev/null "$CLANG_TIDY_BIN_PATH" --version | grep -qF "LLVM version ${CLANG_TIDY_MAJOR}." || die "clang-tidy ${CLANG_TIDY_MAJOR} did not install" @@ -234,8 +234,8 @@ ensure_clang_compiler() { ls /usr/lib/llvm-${CLANG_TIDY_MAJOR}/lib/clang/*/lib/linux/libclang_rt.fuzzer-*.a >/dev/null 2>&1; then return 0 fi - DEBIAN_FRONTEND=noninteractive apt-get update -qq - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ "clang-${CLANG_TIDY_MAJOR}" "libclang-rt-${CLANG_TIDY_MAJOR}-dev" >/dev/null command -v "clang-${CLANG_TIDY_MAJOR}" >/dev/null || die "clang-${CLANG_TIDY_MAJOR} did not install" @@ -249,9 +249,9 @@ have_cross_toolchain() { # skips the download. ensure_cross_toolchain() { have_cross_toolchain && return 0 - DEBIAN_FRONTEND=noninteractive apt-get update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq # shellcheck disable=SC2086 # a fixed list of package names - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ $CROSS_PACKAGES >/dev/null have_cross_toolchain || die "the aarch64 cross toolchain did not install" } @@ -266,9 +266,9 @@ have_gstreamer_dev() { # image that carries them skips the download. ensure_gstreamer_dev() { have_gstreamer_dev && return 0 - DEBIAN_FRONTEND=noninteractive apt-get update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq # shellcheck disable=SC2086 # a fixed list of package names - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ $GSTREAMER_PACKAGES >/dev/null have_gstreamer_dev || die "the GStreamer development files did not install" } @@ -278,9 +278,9 @@ ensure_gstreamer_dev() { # its cross sysroot has no arm64 zimg. ensure_zimg_dev() { pkg-config --exists 'zimg >= 2.7' && return 0 - DEBIAN_FRONTEND=noninteractive apt-get update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq # shellcheck disable=SC2086 # a fixed list of package names - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ $ZIMG_PACKAGES >/dev/null pkg-config --exists 'zimg >= 2.7' || die "zimg's development files did not install" } @@ -308,8 +308,8 @@ ensure_rust_toolchain() { export PATH="$CARGO_HOME/bin:$PATH" RUSTUP_TOOLCHAIN="$version" have_rust_toolchain "$version" && return 0 if ! command -v rustup >/dev/null; then - DEBIAN_FRONTEND=noninteractive apt-get update -qq - DEBIAN_FRONTEND=noninteractive apt-get install -y -qq --no-install-recommends \ + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 update -qq + DEBIAN_FRONTEND=noninteractive apt-get -o Acquire::Retries=3 install -y -qq --no-install-recommends \ rustup >/dev/null fi rustup toolchain install "$version" --profile minimal >/dev/null diff --git a/scripts/release/pypi-publish-state.sh b/scripts/release/pypi-publish-state.sh index cb0f669081..c9e36ac907 100755 --- a/scripts/release/pypi-publish-state.sh +++ b/scripts/release/pypi-publish-state.sh @@ -42,7 +42,7 @@ local_files=("${wheels[@]}" "${sdists[0]}") response="$(mktemp)" trap 'rm -f "$response"' EXIT -http_code="$(curl --retry 3 --max-time 60 --silent --show-error \ +http_code="$(curl --retry 3 --retry-all-errors --max-time 60 --silent --show-error \ --output "$response" --write-out '%{http_code}' \ "https://pypi.org/pypi/$project/$version/json")" diff --git a/scripts/setup/fedora.sh b/scripts/setup/fedora.sh index 41606aba76..2bcb93ee8c 100755 --- a/scripts/setup/fedora.sh +++ b/scripts/setup/fedora.sh @@ -39,7 +39,7 @@ if [[ "$INSTALL_LINTERS" == "true" ]]; then # Pin the explicit version URL — /releases/latest/download/ 404s once # upstream tags a newer version since the filename embeds the version. SHFMT_VERSION="v3.13.1" - $SUDO curl -fsSL -o /usr/local/bin/shfmt \ + $SUDO curl --retry 3 --retry-all-errors -fsSL -o /usr/local/bin/shfmt \ "https://github.com/mvdan/sh/releases/download/${SHFMT_VERSION}/shfmt_${SHFMT_VERSION}_linux_amd64" $SUDO chmod +x /usr/local/bin/shfmt fi diff --git a/scripts/setup/ubuntu.sh b/scripts/setup/ubuntu.sh index c065fd29bd..c255986d5b 100755 --- a/scripts/setup/ubuntu.sh +++ b/scripts/setup/ubuntu.sh @@ -45,7 +45,7 @@ if [[ "$INSTALL_LINTERS" == "true" ]]; then # contains the version string. if ! command -v shfmt >/dev/null; then SHFMT_VERSION="v3.13.1" - $SUDO curl -fsSL -o /usr/local/bin/shfmt \ + $SUDO curl --retry 3 --retry-all-errors -fsSL -o /usr/local/bin/shfmt \ "https://github.com/mvdan/sh/releases/download/${SHFMT_VERSION}/shfmt_${SHFMT_VERSION}_linux_amd64" $SUDO chmod +x /usr/local/bin/shfmt fi @@ -63,7 +63,7 @@ fi if [[ "$ENABLE_SYCL" == "true" ]]; then echo "--- Intel oneAPI DPC++ (SYCL) ---" - wget -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB | + wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB | $SUDO gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" | $SUDO tee /etc/apt/sources.list.d/oneapi.list >/dev/null From 94a318fc648fb79363baf5b5317ad392c7095bfb Mon Sep 17 00:00:00 2001 From: lusoris Date: Tue, 10 Nov 2026 16:38:34 +0100 Subject: [PATCH 2/6] test(ci): annotate the fetch retry test error lists The pre-push mypy gate refused the branch: mypy could not infer the element type of the empty errors list in audit_sources. Both result lists in the contract test now carry list[str]. Signed-off-by: lusoris --- scripts/ci/tests/test_fetch_connect_retry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ci/tests/test_fetch_connect_retry.py b/scripts/ci/tests/test_fetch_connect_retry.py index c52380efe3..f1a2ef5f3a 100644 --- a/scripts/ci/tests/test_fetch_connect_retry.py +++ b/scripts/ci/tests/test_fetch_connect_retry.py @@ -191,7 +191,7 @@ def retry_errors(args: list[str]) -> list[str]: ): return [] return ["wget needs positive --tries, --retry-connrefused and --retry-on-host-error"] - errors = [] + errors: list[str] = [] if not positive_count(args, "--retry"): errors.append("curl needs positive --retry") if not enabled(args, "--retry-all-errors"): @@ -210,7 +210,7 @@ def audit_sources( sources: list[tuple[str, str]], allowlist: tuple[tuple[str, str, str], ...] = ALLOWLIST ) -> list[str]: """Audit fetch arguments and reject stale or ambiguous allow-list entries.""" - errors = [] + errors: list[str] = [] seen: Counter[tuple[str, str, str]] = Counter() for path, text in sources: for line, args in fetch_commands(text): From 575998cb8ca591cca6e9fe212facc4cea760ee46 Mon Sep 17 00:00:00 2001 From: lusoris Date: Tue, 10 Nov 2026 18:22:26 +0100 Subject: [PATCH 3/6] fix(ci): save retried downloads to a file and parse fetch options Reviewers found that a retried curl writing into a pipe can append a second partial body, so gpg or tar can read corrupt input even after the retry succeeds. curl clears partial data only from its own output file. Every retried download now writes to a temporary file, and its consumer runs only after the download succeeds. The ROCm layer stream keeps a single curl attempt inside the existing whole-pipeline retry loop (LAYER_ATTEMPTS) and is listed as an exact exemption. The contract test now parses curl and wget options from allow-lists taken from curl 8.23.0 src/tool_getparam.c and GNU Wget 1.25.0. A flag used as the value of another option no longer counts, --url values are inventoried, comments are stripped only where a word starts, and tidy-lane APT calls need a positive literal retry count. git clone and APT outside the tidy lane stay uncovered; issue 3392 tracks them and two known-gap fixtures pin that limit. A scripts/ci/AGENTS.d page records the invariant, and the generated gates index includes it. Signed-off-by: lusoris --- .github/workflows/build.yml | 6 +- .github/workflows/ffmpeg-integration.yml | 6 +- .github/workflows/libvmaf-build-matrix.yml | 6 +- .github/workflows/lint-and-format.yml | 8 +- .github/workflows/security-scans.yml | 7 +- Containerfile.vmafx | 7 +- Dockerfile | 6 +- changelog.d/fixed/fetch-connect-retry.md | 2 +- dev/Containerfile | 7 +- docs/development/ci.md | 36 +- docs/rebase-notes.d/fetch-connect-retry.md | 5 + scripts/ci/AGENTS-gates.md | 1 + scripts/ci/AGENTS.d/fetch-connect-retry.md | 23 + scripts/ci/install-rocm-from-image.sh | 20 +- scripts/ci/tests/test_fetch_connect_retry.py | 532 ++++++++++++++++--- scripts/setup/ubuntu.sh | 6 +- 16 files changed, 571 insertions(+), 107 deletions(-) create mode 100644 docs/rebase-notes.d/fetch-connect-retry.md create mode 100644 scripts/ci/AGENTS.d/fetch-connect-retry.md diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8aeab8453f..4ee54bc5dd 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -333,8 +333,10 @@ jobs: bash scripts/ci/install-intel-ocloc.sh sudo apt-get update sudo -E apt-get -yq install gpg-agent cmake - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$RUNNER_TEMP/oneapi-key.asc" \ + https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB + sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$RUNNER_TEMP/oneapi-key.asc" + rm -f "$RUNNER_TEMP/oneapi-key.asc" echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list sudo apt-get update diff --git a/.github/workflows/ffmpeg-integration.yml b/.github/workflows/ffmpeg-integration.yml index 10da3d6595..1db92828f6 100644 --- a/.github/workflows/ffmpeg-integration.yml +++ b/.github/workflows/ffmpeg-integration.yml @@ -195,8 +195,10 @@ jobs: # ocloc compiles the SYCL ahead-of-time images; the Linux compiler # does not ship it and meson setup refuses without it (ADR-1360). bash scripts/ci/install-intel-ocloc.sh - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$RUNNER_TEMP/oneapi-key.asc" \ + https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB + sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$RUNNER_TEMP/oneapi-key.asc" + rm -f "$RUNNER_TEMP/oneapi-key.asc" echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list sudo apt-get update diff --git a/.github/workflows/libvmaf-build-matrix.yml b/.github/workflows/libvmaf-build-matrix.yml index b0c9cb53c8..e759f9d56f 100644 --- a/.github/workflows/libvmaf-build-matrix.yml +++ b/.github/workflows/libvmaf-build-matrix.yml @@ -414,8 +414,10 @@ jobs: sudo -E apt-get -yq install ccache ninja-build nasm gcc g++ gpg-agent pkg-config libzimg-dev # ADR-1360 (design notes) bash scripts/ci/install-intel-ocloc.sh - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$RUNNER_TEMP/oneapi-key.asc" \ + https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB + sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$RUNNER_TEMP/oneapi-key.asc" + rm -f "$RUNNER_TEMP/oneapi-key.asc" echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list sudo apt-get update diff --git a/.github/workflows/lint-and-format.yml b/.github/workflows/lint-and-format.yml index 0a6c3a7525..96b6a96bd8 100644 --- a/.github/workflows/lint-and-format.yml +++ b/.github/workflows/lint-and-format.yml @@ -662,10 +662,10 @@ jobs: run: | printf 'Acquire::Retries "3";\nAcquire::http::Timeout "30";\nAcquire::https::Timeout "30";\nAPT::Keep-Downloaded-Packages "true";\n' \ | sudo tee /etc/apt/apt.conf.d/80ci-retries >/dev/null - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- \ - https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | sudo gpg --dearmor \ - -o /usr/share/keyrings/oneapi-archive-keyring.gpg + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$RUNNER_TEMP/oneapi-key.asc" \ + https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB + sudo gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$RUNNER_TEMP/oneapi-key.asc" + rm -f "$RUNNER_TEMP/oneapi-key.asc" echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] \ https://apt.repos.intel.com/oneapi all main" \ | sudo tee /etc/apt/sources.list.d/oneAPI.list diff --git a/.github/workflows/security-scans.yml b/.github/workflows/security-scans.yml index ef025826bb..6cf0ef3447 100644 --- a/.github/workflows/security-scans.yml +++ b/.github/workflows/security-scans.yml @@ -308,9 +308,10 @@ jobs: - name: Install gitleaks run: | GITLEAKS_VERSION=8.24.3 - curl --retry 3 --retry-all-errors -sSfL --max-time 300 \ - "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" \ - | tar -xz -C /usr/local/bin gitleaks + curl --retry 3 --retry-all-errors -sSfL --max-time 300 -o "$RUNNER_TEMP/gitleaks.tar.gz" \ + "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + tar -xzf "$RUNNER_TEMP/gitleaks.tar.gz" -C /usr/local/bin gitleaks + rm -f "$RUNNER_TEMP/gitleaks.tar.gz" gitleaks version # Scan the history of the checked-out commit only. Without --log-opts # gitleaks runs `git log --all`, and fetch-depth: 0 fetches every diff --git a/Containerfile.vmafx b/Containerfile.vmafx index 0cf3295e1a..fb3ff81d64 100644 --- a/Containerfile.vmafx +++ b/Containerfile.vmafx @@ -128,9 +128,10 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ # umf/tcm/tbb component dirs together without missing-dependency surprises. # The ~10 GB it installs never leaves this stage — stage 3 copies out only # the specific .so's `ldd` says vmaf/ffmpeg/libvmaf actually link (below). -RUN curl --retry 3 --retry-all-errors -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | gpg --dearmor \ - | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ +RUN key="$(mktemp)" \ + && curl --retry 3 --retry-all-errors -fsSL -o "$key" https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + && gpg --dearmor < "$key" | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ + && rm -f "$key" \ && printf 'deb [signed-by=/usr/share/keyrings/intel-sw-products.gpg] https://apt.repos.intel.com/oneapi all main\n' \ > /etc/apt/sources.list.d/intel-oneapi.list diff --git a/Dockerfile b/Dockerfile index a11578c472..5ee4db3b18 100644 --- a/Dockerfile +++ b/Dockerfile @@ -87,8 +87,10 @@ RUN --mount=type=cache,target=/var/cache/apt,sharing=locked \ --mount=type=cache,target=/var/lib/apt,sharing=locked \ if [ "$ENABLE_SYCL" = "true" ]; then \ apt-get update && apt-get install -y --no-install-recommends gnupg2 ca-certificates && \ - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg && \ + key="$(mktemp)" && \ + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$key" https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB && \ + gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$key" && \ + rm -f "$key" && \ echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \ > /etc/apt/sources.list.d/oneapi.list && \ apt-get update && apt-get install -y --no-install-recommends \ diff --git a/changelog.d/fixed/fetch-connect-retry.md b/changelog.d/fixed/fetch-connect-retry.md index 58fd86fc9b..14888097f6 100644 --- a/changelog.d/fixed/fetch-connect-retry.md +++ b/changelog.d/fixed/fetch-connect-retry.md @@ -1 +1 @@ -- **CI:** a refused or unreachable connection no longer ends a CI download on the first attempt: curl and wget fetches in shell scripts, workflows and container recipes retry connection and host-resolution failures, the tidy lane's APT downloads retry, and a contract test checks every fetch for the retry flags. +- **CI:** a refused or unreachable connection no longer ends a CI download on the first attempt: curl and wget fetches in shell scripts, workflows and container recipes retry connection and host-resolution failures, the tidy lane's APT downloads retry, and a contract test checks curl and wget fetches for the retry flags and requires retried downloads to write to a file before use. diff --git a/dev/Containerfile b/dev/Containerfile index 5f7c57fa3a..e7075e6794 100644 --- a/dev/Containerfile +++ b/dev/Containerfile @@ -415,9 +415,10 @@ ENV LD_LIBRARY_PATH=${CUDA_HOME}/lib64 # does not publish year-quarter-versioned meta-packages (e.g., # `intel-basekit-2025.3` does not exist in the apt repo); the versioning is # tracked internally by the component packages the meta-package pulls in. -RUN curl --retry 3 --retry-all-errors -fsSL https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ - | gpg --dearmor \ - | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ +RUN key="$(mktemp)" \ + && curl --retry 3 --retry-all-errors -fsSL -o "$key" https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \ + && gpg --dearmor < "$key" | tee /usr/share/keyrings/intel-sw-products.gpg > /dev/null \ + && rm -f "$key" \ && printf 'deb [signed-by=/usr/share/keyrings/intel-sw-products.gpg] https://apt.repos.intel.com/oneapi all main\n' \ > /etc/apt/sources.list.d/intel-oneapi.list diff --git a/docs/development/ci.md b/docs/development/ci.md index 0fb745a9f5..cc4dace58e 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -1160,20 +1160,40 @@ the apt.llvm.org signing key: `curl: (7) Failed to connect to apt.llvm.org port failed connection, and `--retry-connrefused` adds only a refused one, not an unreachable network or host. -Every HTTP(S) fetch in a shell script, workflow or container recipe therefore -retries connection failures: +Every curl and wget HTTP(S) fetch in a shell script, workflow or container +recipe retries connection failures: - **curl:** `--retry N --retry-all-errors`, with N positive. - **wget:** `--tries=N --retry-connrefused --retry-on-host-error`, with N positive. -- **APT in `scripts/dev/tidy-lane.sh`:** `-o Acquire::Retries=3`. +- **APT in `scripts/dev/tidy-lane.sh`:** `-o Acquire::Retries=N`, with N a + literal positive integer, on every `update` and `install` command. + +Retried downloads must write the body to a file before a consumer reads it. +In curl 8.23.0, [`docs/cmdline-opts/retry-all-errors.md`](https://github.com/curl/curl/blob/3163ec065643ba20709f190111f42c3e1fe95273/docs/cmdline-opts/retry-all-errors.md) +explains that curl clears partial output from its own output file before +retrying; pipes, command capture and shell `>` redirection can instead append +duplicate partial data. Wget follows the same file-first rule as a precaution; +its pipe behavior on retry has not been measured. Header-only requests and +explicit body disposal to `/dev/null` need no saved body. Each site keeps its own `--max-time` and `--connect-timeout` bounds. -`scripts/ci/tests/test_fetch_connect_retry.py` checks every tracked fetch for -these flags and runs as the `test-fetch-connect-retry` pre-commit hook. Fetches -that must fail fast, such as a local health probe inside its own bounded poll, -are listed with a reason in its `ALLOWLIST`; an entry whose site is gone fails -the test. +`scripts/ci/tests/test_fetch_connect_retry.py` audits tracked sources offline +and runs as the `test-fetch-connect-retry` pre-commit hook. Its option allow-list +models value consumption and final flag state. Unknown options fail: add a row +from curl 8.23.0 `src/tool_getparam.c` (the pinned source above) or the installed +GNU Wget 1.25.0 `wget --help`, rather than silently accepting a new spelling. + +Exemptions require an exact path, parsed URL and reason in `ALLOWLIST`; stale +or ambiguous entries fail. Local health checks within bounded outer polls may +need to fail fast. The ROCm layer download in +`scripts/ci/install-rocm-from-image.sh` retries the whole curl, decompressor and +tar pipeline up to `LAYER_ATTEMPTS` times; inner curl retries would append to the +pipe, so this streaming fetch is exempt from both inner retries and file output. + +**Not covered:** `git clone` and APT calls outside the tidy lane. Follow-up +issue #3392 tracks these gaps; the tests whose docstrings start +`Known gap (#3392):` replay both forms so the scanner's limits stay explicit. ## Apt archive cache diff --git a/docs/rebase-notes.d/fetch-connect-retry.md b/docs/rebase-notes.d/fetch-connect-retry.md new file mode 100644 index 0000000000..7b7f05cc7f --- /dev/null +++ b/docs/rebase-notes.d/fetch-connect-retry.md @@ -0,0 +1,5 @@ +## CI downloads retry connection failures and write to a file first (2026-11-10) + +- Netflix-derived `Dockerfile`: keep wget connection and host-resolution retry flags when updating the optional oneAPI key setup from upstream. +- Preserve the temporary-file key download, successful-fetch-before-gpg ordering and cleanup; an inner retry into a pipe can duplicate partial bytes. +- `scripts/ci/tests/test_fetch_connect_retry.py::FetchConnectRetryTests::test_tracked_fetches_retry_connect_failures` guards the live download inventory, retry flags and file-first body contract. diff --git a/scripts/ci/AGENTS-gates.md b/scripts/ci/AGENTS-gates.md index 910504e03b..46516c2025 100644 --- a/scripts/ci/AGENTS-gates.md +++ b/scripts/ci/AGENTS-gates.md @@ -27,6 +27,7 @@ file of this directory. `Touching` paths: relative to this directory, leading | `check-dco.py`, `dco-cutoff.txt`, `tests/test_check_dco.py` | [dco-sign-off](AGENTS.d/dco-sign-off.md) | Non-merge PR commits require author/committer sign-off; bot exemption two-sided; fail closed. | | `/.config/lefthook/engine.sh`, `/.config/praetor/engine.mk`, `/lefthook.yml`, `/scripts/git-hooks/hiss-audit.sh`, `/scripts/githooks/state-sync.sh`, `/.config/lint-exceptions.d/shfmt.toml`, `tests/test_shfmt_scope.py` | [engine-launcher](AGENTS.d/engine-launcher.md) | Governance hooks reach engine only through `sh .config/lefthook/engine.sh`; launcher stays praetor's unedited text. | | `test_fail_closed_ci.py`, `/scripts/git-hooks/pre-push-mypy.py`, `/scripts/git-hooks/test-pre-push-mypy.py`, `/requirements/locks/mypy.txt` | [fail-closed-ci](AGENTS.d/fail-closed-ci.md) | Required Python Lint = hash-locked mypy via merge-base hook, full history, no exit masking; test stays wired. | +| `tests/test_fetch_connect_retry.py`, `install-rocm-from-image.sh`, `/docs/development/ci.md` | [fetch-connect-retry](AGENTS.d/fetch-connect-retry.md) | Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. | | `ffmpeg-patches-surface-check.sh`, `tests/test_ffmpeg_patches_surface_check.py` | [ffmpeg-patches-surface](AGENTS.d/ffmpeg-patches-surface.md) | Tree checks run in hook (--surface-only) + CI; scan failure or zero docs fails closed; bash 3.2 safe. | | `check-issue-reference-provenance.py`, `tests/test_issue_reference_provenance.py` | [issue-reference-provenance](AGENTS.d/issue-reference-provenance.md) | Protected historical contexts keep their archived `lusoris/vmaf` identity; contracts stay context-scoped. | | `check-local-data-contract.sh`, `tests/test-check-local-data-contract.sh` | [local-data-roots](AGENTS.d/local-data-roots.md) | `.workingdir/` = private state, `.corpus/` = data, tracked files = public evidence; no tracked link into ignored data. | diff --git a/scripts/ci/AGENTS.d/fetch-connect-retry.md b/scripts/ci/AGENTS.d/fetch-connect-retry.md new file mode 100644 index 0000000000..7373b8b288 --- /dev/null +++ b/scripts/ci/AGENTS.d/fetch-connect-retry.md @@ -0,0 +1,23 @@ +--- +paths: + - scripts/ci/tests/test_fetch_connect_retry.py + - scripts/ci/install-rocm-from-image.sh + - docs/development/ci.md +invariant: Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. +area: gates +--- +# Fetch connection retry contract + +- Curl: positive `--retry` + enabled `--retry-all-errors`. +- Wget: positive `--tries` + enabled `--retry-connrefused` + `--retry-on-host-error`. +- Retried body -> fetch-owned file; consumer after success; cleanup after use. +- Stdout body forbidden through pipe, capture, shell file redirection; + `/dev/null` disposal allowed. +- Option allow-list: curl 8.23.0 `src/tool_getparam.c`; GNU Wget 1.25.0 `wget --help`. +- Unknown option -> error; value consumes next argument; final switch wins. +- Exemption shape: exact path + parsed URL + reason; stale/ambiguous entry -> failure. +- ROCm layer exemption: whole pipeline retried `LAYER_ATTEMPTS` times; inner + retry appends partial bytes. +- Tidy-lane APT `update`/`install`: `-o Acquire::Retries=N`, literal positive N. +- Known gaps #3392: `git clone`, APT outside tidy lane; `Known gap (#3392):` fixtures. +- Test: `scripts/ci/tests/test_fetch_connect_retry.py`; offline source reads only. diff --git a/scripts/ci/install-rocm-from-image.sh b/scripts/ci/install-rocm-from-image.sh index 017a76cee1..63845030d6 100755 --- a/scripts/ci/install-rocm-from-image.sh +++ b/scripts/ci/install-rocm-from-image.sh @@ -132,9 +132,13 @@ for pattern in "${EXCLUDES[@]}"; do tar_excludes+=("--exclude=$pattern") done +STAGE="$(mktemp -d)" +trap 'rm -rf "$STAGE"' EXIT + echo "==> Authenticating to $REGISTRY for $REPO" -TOKEN="$(curl -fsSL --max-time "$API_MAX_TIME" --retry 5 --retry-delay 10 --retry-all-errors \ - "${AUTH}?service=${AUTH_SERVICE}&scope=repository:${REPO}:pull" | jq -r .token)" +curl -fsSL --max-time "$API_MAX_TIME" --retry 5 --retry-delay 10 --retry-all-errors \ + -o "$STAGE/.token.json" "${AUTH}?service=${AUTH_SERVICE}&scope=repository:${REPO}:pull" +TOKEN="$(jq -r .token "$STAGE/.token.json")" if [ -z "$TOKEN" ] || [ "$TOKEN" = "null" ]; then echo "install-rocm-from-image.sh: failed to obtain a registry pull token" >&2 exit 1 @@ -145,7 +149,8 @@ ACCEPT='application/vnd.docker.distribution.manifest.v2+json,application/vnd.doc fetch_manifest() { curl -fsSL --max-time "$API_MAX_TIME" --retry 5 --retry-delay 10 --retry-all-errors \ -H "Authorization: Bearer ${TOKEN}" -H "Accept: ${ACCEPT}" \ - "https://${REGISTRY}/v2/${REPO}/manifests/$1" + -o "$STAGE/.manifest.json" "https://${REGISTRY}/v2/${REPO}/manifests/$1" && + cat "$STAGE/.manifest.json" } echo "==> Fetching manifest $DIGEST" @@ -171,8 +176,6 @@ if [ -z "$LAYER_COUNT" ] || [ "$LAYER_COUNT" = "null" ] || [ "$LAYER_COUNT" -eq fi echo "==> $LAYER_COUNT layers to scan" -STAGE="$(mktemp -d)" -trap 'rm -rf "$STAGE"' EXIT TAR_ERRORS="$STAGE/.tar-errors" # One layer, streamed from the registry into tar. A layer with no opt/rocm/ @@ -180,16 +183,15 @@ TAR_ERRORS="$STAGE/.tar-errors" # most layers; any other failure of the download, the decompression or tar # stops the install instead of leaving a partial tree. # -# If a network drop or mid-stream reset occurs, curl's in-pipe retry cannot -# recover because downstream decompressor and tar processes have already died. -# We therefore retry the entire pipeline (curl | decomp | tar) up to +# An inner curl retry would append to the pipe, duplicating partial body data. +# Retry the entire pipeline (curl | decomp | tar) up to # LAYER_ATTEMPTS times. Re-extracting a layer over a partial extraction is safe: # tar overwrites the same paths. extract_layer() { local digest="$1" attempt=1 statuses while [ "$attempt" -le "$LAYER_ATTEMPTS" ]; do set +e - curl --retry 3 --retry-all-errors -fsSL --max-time "$LAYER_MAX_TIME" \ + curl -fsSL --max-time "$LAYER_MAX_TIME" \ -H "Authorization: Bearer ${TOKEN}" \ "https://${REGISTRY}/v2/${REPO}/blobs/${digest}" | "${decomp[@]}" | diff --git a/scripts/ci/tests/test_fetch_connect_retry.py b/scripts/ci/tests/test_fetch_connect_retry.py index f1a2ef5f3a..1ac4956a0b 100644 --- a/scripts/ci/tests/test_fetch_connect_retry.py +++ b/scripts/ci/tests/test_fetch_connect_retry.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 # Copyright 2026 Lusoris # SPDX-License-Identifier: EUPL-1.2 -"""Offline contract for connection retries at every tracked shell fetch site.""" +"""Audit curl/wget fetches and tidy-lane APT offline; git/other APT gap: #3392.""" from __future__ import annotations @@ -12,6 +12,7 @@ import tempfile import unittest from collections import Counter +from dataclasses import dataclass, field from pathlib import Path ROOT = Path(__file__).resolve().parents[3] @@ -19,9 +20,15 @@ from scripts.lib.safe_subprocess import run as run_command # noqa: E402 -# Exact path + URL argument + reason. No implicit loopback exemption: a new -# probe must demonstrate its own bounded poll, or why repeating it is incorrect. +# Curl/wget fetches + tidy-lane APT only; git clone/other APT gap: #3392. +# Exact path + parsed URL + reason; exempt commands skip every check. ALLOWLIST = ( + ( + "scripts/ci/install-rocm-from-image.sh", + "https://${REGISTRY}/v2/${REPO}/blobs/${digest}", + "extract_layer retries the whole curl | decompress | tar pipeline LAYER_ATTEMPTS times; " + "an inner retry would append to the pipe", + ), ( "scripts/ci/observability-compose-smoke.sh", "http://localhost:9090/api/v1/query?query=up%7Bjob%3D%22vmafx-node%22%7D%3D%3D1", @@ -94,7 +101,7 @@ def tracked_sources() -> list[tuple[str, str]]: check=True, timeout_seconds=30, ) - sources = [] + sources: list[tuple[str, str]] = [] for path in result.stdout.split("\0"): source = ROOT / path if not path or not source.is_file() or source.is_symlink(): @@ -110,7 +117,7 @@ def tracked_sources() -> list[tuple[str, str]]: def logical_lines(text: str) -> list[tuple[int, str]]: """Join shell, PowerShell, and cmd continuations without losing source locations.""" - lines = [] + lines: list[tuple[int, str]] = [] pending = "" start = 1 for number, line in enumerate(text.splitlines(), 1): @@ -126,99 +133,313 @@ def logical_lines(text: str) -> list[tuple[int, str]]: return lines +# curl 8.23.0, commit 3163ec065643ba20709f190111f42c3e1fe95273: +# src/tool_getparam.c aliases[]; src/tool_getparam.h:321-326. +# ARG_STRG/FILE/SECS/UNUM consume values; ARG_BOOL accepts --no-. +# Rows: long name, short letter, takes value, accepts --no-. +CURL_OPTIONS = ( + ("connect-timeout", None, True, False), + ("create-dirs", None, False, True), + ("data", "d", True, False), + ("data-urlencode", None, True, False), + ("fail", "f", False, True), + ("fail-with-body", None, False, True), + ("get", "G", False, True), + ("header", "H", True, False), + ("location", "L", False, True), + ("max-time", "m", True, False), + ("oauth2-bearer", None, True, False), + ("output", "o", True, False), + ("proto", None, True, False), + ("remove-on-error", None, False, True), + ("retry", None, True, False), + ("retry-all-errors", None, False, True), + ("retry-connrefused", None, False, True), + ("retry-delay", None, True, False), + ("retry-max-time", None, True, False), + ("show-error", "S", False, True), + ("silent", "s", False, True), + ("tlsv1.2", None, False, False), + ("url", None, True, False), + ("user-agent", "A", True, False), + ("write-out", "w", True, False), + ("head", "I", False, True), + ("remote-name", "O", False, True), +) +# GNU Wget 1.25.0, LC_ALL=C wget --help; flag --no- forms measured at rc 0 +# in review; --no-tries=3 rejected at rc 2. Same row format as curl. +WGET_OPTIONS = ( + ("output-document", "O", True, False), + ("quiet", "q", False, True), + ("retry-connrefused", None, False, True), + ("retry-on-host-error", None, False, True), + ("timeout", "T", True, False), + ("tries", "t", True, False), + ("waitretry", None, True, False), + ("spider", None, False, True), +) +APT_RETRY_PATHS = ("scripts/dev/tidy-lane.sh",) + + +@dataclass +class ParsedFetch: + """Hold consumed options, URLs, redirection and parser diagnostics.""" + + tool: str + values: dict[str, str] = field(default_factory=dict) + flags: dict[str, bool] = field(default_factory=dict) + urls: list[str] = field(default_factory=list) + stdout_target: str = "" + errors: list[str] = field(default_factory=list) + + +def quoted_end(command: str, index: int) -> int: + """Skip a quoted span while honoring double-quote backslash escapes.""" + quote = command[index] + index += 1 + while index < len(command): + char = command[index] + index += 1 + if quote == '"' and char == "\\": + index += 1 + elif char == quote: + break + return index + + +def strip_comment(command: str) -> str: + """Strip shell comments while retaining quoted and in-word hash characters.""" + index = 0 + while index < len(command): + char = command[index] + if char == "\\": + index += 2 + continue + if char in ("'", '"'): + index = quoted_end(command, index) + continue + word_start = index == 0 or command[index - 1].isspace() or command[index - 1] in ";&|()" + if char == "#" and word_start: + return command[:index] + index += 1 + return command + + def command_args(command: str) -> list[str]: - """Tokenize one fetch, stopping before the next shell command or enclosing $().""" + """Tokenize one fetch up to shell control after removing actual comments.""" + command = strip_comment(command) lexer = shlex.shlex(command, posix=True, punctuation_chars=";&|()") lexer.whitespace_split = True + lexer.commenters = "" try: - tokens = list(lexer) + arguments = list(lexer) except ValueError: # Embedded shell/YAML fragments may end inside an enclosing quote. - # Keep auditing the whole fetch rather than discarding that line. - tokens = [token.strip("\"'") for token in command.split()] - args = [] - for token in tokens: - if token in CONTROL or token.startswith("#"): + arguments = [argument.strip("\"'") for argument in command.split()] + args: list[str] = [] + index = 0 + while index < len(arguments): + argument = arguments[index] + if argument == "&" and index + 1 < len(arguments) and arguments[index + 1].startswith(">"): + index += 1 + argument += arguments[index] + elif argument in CONTROL: break - args.append(token) + args.append(argument) + index += 1 return args -def fetch_commands(text: str) -> list[tuple[int, list[str]]]: - """Inventory literal and variable URL fetches on every logical source line.""" - commands = [] - for line_number, line in logical_lines(text): - if line.lstrip().startswith("#"): +def consume_value( + args: list[str], index: int, attached: str | None, parsed: ParsedFetch, name: str +) -> int: + """Consume an attached or following value without interpreting its contents.""" + if attached is not None: + value = attached + consumed = 1 + elif index + 1 < len(args): + value = args[index + 1] + consumed = 2 + else: + parsed.errors.append(f"{parsed.tool} option --{name} needs a value") + return 1 + parsed.values[name] = value + if name == "url": + parsed.urls.append(value) + return consumed + + +def parse_long(args: list[str], index: int, parsed: ParsedFetch) -> int: + """Validate a long option and consume its value or final flag state.""" + option, equals, value = args[index][2:].partition("=") + negated = option.startswith("no-") + name = option[3:] if negated else option + rows = WGET_OPTIONS if parsed.tool == "wget" else CURL_OPTIONS + row = next((row for row in rows if row[0] == name), None) + if row is None or (negated and not row[3]): + parsed.errors.append(f"unknown {parsed.tool} option --{option}") + return 1 + if row[2]: + return consume_value(args, index, value if equals else None, parsed, name) + if equals: + parsed.errors.append(f"{parsed.tool} flag --{option} does not take a value") + else: + parsed.flags[name] = not negated + return 1 + + +def parse_short(args: list[str], index: int, parsed: ParsedFetch) -> int: + """Read short bundles left to right with value options consuming the remainder.""" + rows = WGET_OPTIONS if parsed.tool == "wget" else CURL_OPTIONS + bundle = args[index][1:] + for offset, letter in enumerate(bundle): + row = next((row for row in rows if row[1] == letter), None) + if row is None: + parsed.errors.append(f"unknown {parsed.tool} option -{letter}") continue - for match in FETCH.finditer(line): - args = command_args(line[match.start() :]) - # Include variable URLs (including Windows %URL%), not just literal - # hosts. Package lists and command -v checks have no URL arguments. - if any(re.match(r"https?://|\$|%", arg) for arg in args[1:]): - commands.append((line_number, args)) - return commands - - -def option_value(args: list[str], option: str) -> str: - """Read the last value supplied with a long option.""" - value = "" - for index, arg in enumerate(args): - if arg.startswith(option + "="): - value = arg.partition("=")[2] - elif arg == option: - value = args[index + 1] if index + 1 < len(args) else "" - return value + if row[2]: + attached = bundle[offset + 1 :] or None + return consume_value(args, index, attached, parsed, row[0]) + parsed.flags[row[0]] = True + return 1 + + +def parse_redirect(args: list[str], index: int, parsed: ParsedFetch) -> int: + """Skip redirection targets and record the destination of standard output.""" + match = re.match(r"([012&]?)(>>?|<") and descriptor != "2": + parsed.stdout_target = target + return consumed + + +def parse_fetch(args: list[str]) -> ParsedFetch: + """Parse only allow-listed curl or wget options with their real value consumption.""" + parsed = ParsedFetch("curl" if args[0] == "curl.exe" else args[0]) + index = 1 + positional = False + while index < len(args): + arg = args[index] + redirected = parse_redirect(args, index, parsed) + if redirected: + index += redirected + continue + if arg == "--": + positional = True + elif not positional and arg.startswith("--"): + index += parse_long(args, index, parsed) + continue + elif not positional and arg.startswith("-") and arg != "-": + index += parse_short(args, index, parsed) + continue + else: + parsed.urls.append(arg) + index += 1 + return parsed -def positive_count(args: list[str], option: str) -> bool: - """Accept only literal positive integer retry counts.""" - return bool(re.fullmatch(r"[1-9][0-9]*", option_value(args, option))) +def fetch_commands(text: str) -> list[tuple[int, ParsedFetch]]: + """Inventory parsed literal and variable URLs, including long URL options.""" + commands: list[tuple[int, ParsedFetch]] = [] + for line_number, source_line in logical_lines(text): + line = strip_comment(source_line) + for match in FETCH.finditer(line): + parsed = parse_fetch(command_args(line[match.start() :])) + if any(re.match(r"https?://|\$|%", url) for url in parsed.urls): + commands.append((line_number, parsed)) + return commands -def enabled(args: list[str], option: str) -> bool: - """Honor the last explicit enable or disable switch.""" - switches = [arg for arg in args if arg in (option, option.replace("--", "--no-", 1))] - return bool(switches) and switches[-1] == option +def positive_count(parsed: ParsedFetch, option: str) -> bool: + """Accept only literal positive integer retry counts from consumed values.""" + return bool(re.fullmatch(r"[1-9][0-9]*", parsed.values.get(option, ""))) -def retry_errors(args: list[str]) -> list[str]: +def retry_errors(parsed: ParsedFetch) -> list[str]: """Require positive retries and the fetch tool's connection error switches.""" - if args[0] == "wget": + if parsed.tool == "wget": if ( - positive_count(args, "--tries") - and enabled(args, "--retry-connrefused") - and enabled(args, "--retry-on-host-error") + positive_count(parsed, "tries") + and parsed.flags.get("retry-connrefused") + and parsed.flags.get("retry-on-host-error") ): return [] return ["wget needs positive --tries, --retry-connrefused and --retry-on-host-error"] errors: list[str] = [] - if not positive_count(args, "--retry"): + if not positive_count(parsed, "retry"): errors.append("curl needs positive --retry") - if not enabled(args, "--retry-all-errors"): + if not parsed.flags.get("retry-all-errors"): errors.append("curl needs --retry-all-errors") return errors +def body_errors(parsed: ParsedFetch) -> list[str]: + """Reject retried bodies on stdout except explicit disposal to /dev/null.""" + if parsed.tool == "wget": + stdout = parsed.values.get("output-document") == "-" and not parsed.flags.get("spider") + else: + stdout = ( + parsed.values.get("output", "-") == "-" + and not parsed.flags.get("remote-name") + and not parsed.flags.get("head") + ) + if stdout and parsed.stdout_target != "/dev/null": + return [f"{parsed.tool} retry must write the body to a file, not stdout"] + return [] + + def matching_exemptions( - path: str, args: list[str], allowlist: tuple[tuple[str, str, str], ...] + path: str, parsed: ParsedFetch, allowlist: tuple[tuple[str, str, str], ...] ) -> list[tuple[str, str, str]]: - """Match exemptions by both tracked path and complete URL argument.""" - return [entry for entry in allowlist if entry[0] == path and entry[1] in args] + """Match exemptions by both tracked path and complete parsed URL.""" + return [entry for entry in allowlist if entry[0] == path and entry[1] in parsed.urls] + + +def apt_errors(path: str, text: str) -> list[str]: + """Require positive literal APT retry counts in the declared tidy lane only.""" + errors: list[str] = [] + if path not in APT_RETRY_PATHS: + return errors + for line, command in logical_lines(text): + command_text = strip_comment(command) + for match in re.finditer(r"(? bool: + """Validate the final explicit APT retry setting against positive literal integers.""" + retries = [ + args[index + 1] + for index, arg in enumerate(args[:-1]) + if arg == "-o" and args[index + 1].startswith("Acquire::Retries=") + ] + return bool(retries and re.fullmatch(r"Acquire::Retries=[1-9][0-9]*", retries[-1])) def audit_sources( sources: list[tuple[str, str]], allowlist: tuple[tuple[str, str, str], ...] = ALLOWLIST ) -> list[str]: - """Audit fetch arguments and reject stale or ambiguous allow-list entries.""" + """Audit parsed fetches and APT while rejecting stale or ambiguous exemptions.""" errors: list[str] = [] seen: Counter[tuple[str, str, str]] = Counter() for path, text in sources: - for line, args in fetch_commands(text): - matches = matching_exemptions(path, args, allowlist) + errors.extend(apt_errors(path, text)) + for line, parsed in fetch_commands(text): + matches = matching_exemptions(path, parsed, allowlist) if matches: seen.update(matches) else: - errors.extend(f"{path}:{line}: {error}" for error in retry_errors(args)) + findings = parsed.errors + retry_errors(parsed) + body_errors(parsed) + errors.extend(f"{path}:{line}: {error}" for error in findings) for entry in allowlist: if not entry[2] or seen[entry] != 1: errors.append( @@ -239,7 +460,7 @@ def test_compliant_temporary_script(self) -> None: """Accept curl and wget with every required retry option.""" self.assertEqual( self.check_script( - "curl -fsSL --retry 3 --retry-all-errors https://example.invalid/x\n" + "curl -o out -fsSL --retry 3 --retry-all-errors https://example.invalid/x\n" "wget -q --tries=3 --retry-connrefused --retry-on-host-error " "https://example.invalid/x\n" ), @@ -248,12 +469,16 @@ def test_compliant_temporary_script(self) -> None: def test_plain_retry_does_not_cover_refused_connection(self) -> None: """Reject curl retries that omit all-errors coverage.""" - self.assertTrue(self.check_script("curl -fsSL --retry 3 https://example.invalid/x\n")) + self.assertTrue( + self.check_script("curl -o out -fsSL --retry 3 https://example.invalid/x\n") + ) def test_curl_retry_connrefused_alone_does_not_cover_unreachable_connection(self) -> None: """Reject refused-only curl retries because other exit-7 causes remain fatal.""" self.assertEqual( - self.check_script("curl --retry 3 --retry-connrefused https://example.invalid/x\n"), + self.check_script( + "curl -o out --retry 3 --retry-connrefused https://example.invalid/x\n" + ), ["fetch.sh:1: curl needs --retry-all-errors"], ) @@ -280,7 +505,9 @@ def test_wget_without_retry_on_host_error_fails(self) -> None: def test_existing_all_errors_retry_is_accepted(self) -> None: """Accept the existing curl all-errors retry convention.""" self.assertEqual( - self.check_script("curl -f --retry 5 --retry-all-errors https://example.invalid/x\n"), + self.check_script( + "curl -o out -f --retry 5 --retry-all-errors https://example.invalid/x\n" + ), [], ) @@ -290,7 +517,7 @@ def test_continuation_flags_count(self) -> None: with self.subTest(continuation=continuation): self.assertEqual( self.check_script( - f"curl -fsSL --retry 3 {continuation}\n" + f"curl -o out -fsSL --retry 3 {continuation}\n" f" --retry-all-errors {continuation}\n" " https://example.invalid/x\n" ), @@ -319,12 +546,12 @@ def test_flags_on_next_command_or_comment_do_not_count(self) -> None: for suffix in (" # --retry-all-errors", " | echo --retry-all-errors"): with self.subTest(suffix=suffix): self.assertTrue( - self.check_script("curl --retry 3 https://example.invalid/x" + suffix) + self.check_script("curl -o out --retry 3 https://example.invalid/x" + suffix) ) def test_retry_must_be_enabled_and_positive(self) -> None: """Reject missing, nonpositive, noninteger, and disabled curl retries.""" - compliant = "curl --retry 3 --retry-all-errors https://example.invalid/x" + compliant = "curl -o out --retry 3 --retry-all-errors https://example.invalid/x" for mutation in ( compliant.replace("--retry 3", ""), compliant.replace("--retry 3", "--retry 0"), @@ -353,11 +580,11 @@ def test_unbalanced_quote_uses_whitespace_split_without_skipping_line(self) -> N for url in ('https://example.invalid/x"', "'https://example.invalid/x"): with self.subTest(url=url): self.assertEqual( - self.check_script(f"curl --retry 3 {url}\n"), + self.check_script(f"curl -o out --retry 3 {url}\n"), ["fetch.sh:1: curl needs --retry-all-errors"], ) self.assertEqual( - self.check_script(f"curl --retry 3 --retry-all-errors {url}\n"), [] + self.check_script(f"curl -o out --retry 3 --retry-all-errors {url}\n"), [] ) def test_fetch_names_in_diagnostic_prose_are_not_commands(self) -> None: @@ -396,6 +623,179 @@ def test_scanned_file_kinds(self) -> None: self.assertTrue(in_scope("fetch", "#!/usr/bin/env bash\n")) self.assertFalse(in_scope("README.md", "curl https://example.invalid/x")) + def test_url_options(self) -> None: + """Audit both long URL spellings and their consumed values.""" + for url in ("--url=https://example.invalid/x", "--url https://example.invalid/x"): + with self.subTest(url=url): + self.assertIn( + "fetch.sh:1: curl needs positive --retry", + self.check_script(f"curl {url} -o out"), + ) + self.assertEqual( + self.check_script(f"curl {url} --retry 3 --retry-all-errors -o out"), [] + ) + + def test_option_value_consumption(self) -> None: + """Keep option-looking values out of the flag state.""" + self.assertEqual( + self.check_script( + "curl --output --retry-all-errors --retry 3 https://example.invalid/x" + ), + ["fetch.sh:1: curl needs --retry-all-errors"], + ) + for output in ("-fsSLo out", "--output=out", "-oout"): + self.assertEqual( + self.check_script( + f"curl {output} --retry=3 --retry-all-errors https://example.invalid/x" + ), + [], + ) + for output in ("-qO out", "-qOout", "--output-document=out"): + self.assertEqual( + self.check_script( + f"wget {output} --tries=3 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x" + ), + [], + ) + + def test_invalid_options(self) -> None: + """Reject unknown options, flag values, and missing option values.""" + base = "curl -o out --retry 3 --retry-all-errors https://example.invalid/x " + for option, error in ( + ("--retry-all-errors=1", "curl flag --retry-all-errors does not take a value"), + ("--retry", "curl option --retry needs a value"), + ("--bogus", "unknown curl option --bogus"), + ("-Z", "unknown curl option -Z"), + ("--no-output=out", "unknown curl option --no-output"), + ("--no-tlsv1.2", "unknown curl option --no-tlsv1.2"), + ): + with self.subTest(option=option): + self.assertIn(f"fetch.sh:1: {error}", self.check_script(base + option)) + self.assertIn( + "fetch.sh:1: unknown wget option --no-tries", + self.check_script("wget --no-tries=3 https://example.invalid/x"), + ) + + def test_body_stdout(self) -> None: + """Reject pipe, capture, redirected-file and explicit stdout bodies.""" + base = "curl --retry 3 --retry-all-errors https://example.invalid/x" + for command in ( + base + " | tar", + f'body="$({base})"', + base + " > file", + base + " -o -", + base + " 1>file", + base + " &>file", + base + " >>file", + base + " -o out -o -", + base + " -I --no-head", + base + " -O --no-remote-name", + ): + with self.subTest(command=command): + self.assertEqual( + self.check_script(command), + ["fetch.sh:1: curl retry must write the body to a file, not stdout"], + ) + self.assertEqual( + self.check_script( + "wget -qO- --tries=3 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x" + ), + ["fetch.sh:1: wget retry must write the body to a file, not stdout"], + ) + + def test_body_file_or_discard(self) -> None: + """Accept files, explicit disposal, remote names and header-only fetches.""" + base = "curl --retry 3 --retry-all-errors https://example.invalid/x" + for suffix in ( + " >/dev/null", + " > /dev/null", + " 1>/dev/null", + " &>/dev/null", + " -o /dev/null", + " -I", + " -O", + " -o out 2>errors errors < input", + " --no-head -I", + " --no-remote-name -O", + " -o - -o out", + ): + with self.subTest(suffix=suffix): + self.assertEqual(self.check_script(base + suffix), []) + self.assertEqual( + self.check_script( + "wget -qO- --tries=3 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x --spider" + ), + [], + ) + + def test_comment_fragments(self) -> None: + """Preserve URL fragments while excluding flags and apostrophes in comments.""" + for url in ( + "https://example.invalid/x#frag", + "'https://example.invalid/x#frag'", + '"https://example.invalid/x#frag"', + ): + self.assertEqual( + self.check_script(f"curl --retry 3 {url} --retry-all-errors -o out"), [] + ) + self.assertEqual( + self.check_script( + "curl --retry 3 -o out https://example.invalid/x # don't --retry-all-errors" + ), + ["fetch.sh:1: curl needs --retry-all-errors"], + ) + self.assertEqual(strip_comment("a#b # end"), "a#b ") + self.assertEqual(strip_comment(r"a\#b # end"), r"a\#b ") + self.assertEqual(strip_comment(r'"a\"#b" # end'), r'"a\"#b" ') + self.assertEqual(strip_comment("echo ok;# end"), "echo ok;") + + def test_apt_retries(self) -> None: + """Require positive literal retry settings for tidy-lane update and install.""" + path = "scripts/dev/tidy-lane.sh" + for verb in ("update", "install -y package"): + self.assertEqual( + audit_sources([(path, f"apt-get -o Acquire::Retries=3 {verb}")], ()), [] + ) + for setting in ( + "", + "-o Acquire::Retries=0", + "-o Acquire::Retries=x", + "-o Acquire::Retries=3 -o Acquire::Retries=0", + ): + with self.subTest(verb=verb, setting=setting): + self.assertEqual( + audit_sources([(path, f"apt-get {setting} {verb}")], ()), + [f"{path}:1: apt-get needs -o Acquire::Retries=N with N positive"], + ) + self.assertEqual(audit_sources([(path, "apt-get --version")], ()), []) + self.assertEqual( + audit_sources([(path, "echo ready && apt-get update")], ()), + [f"{path}:1: apt-get needs -o Acquire::Retries=N with N positive"], + ) + self.assertEqual(audit_sources([(path, "# apt-get update")], ()), []) + + def test_known_gap_git_clone(self) -> None: + """Known gap (#3392): Git clone retries remain outside this scanner.""" + self.assertEqual(self.check_script("git clone https://example.invalid/r.git"), []) + + def test_known_gap_other_apt(self) -> None: + """Known gap (#3392): APT outside tidy-lane remains outside this scanner.""" + for setting in ("", "-o Acquire::Retries=0", "-o Acquire::Retries=x"): + self.assertEqual(audit_sources([("other.sh", f"apt-get {setting} install x")], ()), []) + + def test_url_exemption(self) -> None: + """Match exemptions against URLs consumed by long options.""" + entry = (("fetch.sh", "http://localhost:8080/healthz", "bounded outer poll"),) + for url in ("--url=http://localhost:8080/healthz", "--url http://localhost:8080/healthz"): + self.assertEqual(audit_sources([("fetch.sh", f"curl {url}")], entry), []) + self.assertTrue( + audit_sources([("fetch.sh", "curl --url=http://localhost:8080/other")], entry) + ) + def test_tracked_fetches_retry_connect_failures(self) -> None: """Enforce the retry contract across the live tracked site inventory.""" self.assertEqual(audit_sources(tracked_sources()), []) diff --git a/scripts/setup/ubuntu.sh b/scripts/setup/ubuntu.sh index c255986d5b..f61bb7d01e 100755 --- a/scripts/setup/ubuntu.sh +++ b/scripts/setup/ubuntu.sh @@ -63,8 +63,10 @@ fi if [[ "$ENABLE_SYCL" == "true" ]]; then echo "--- Intel oneAPI DPC++ (SYCL) ---" - wget --tries=3 --retry-connrefused --retry-on-host-error -qO- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB | - $SUDO gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg + key="$(mktemp)" + wget --tries=3 --retry-connrefused --retry-on-host-error -qO "$key" https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB + $SUDO gpg --dearmor -o /usr/share/keyrings/oneapi-archive-keyring.gpg "$key" + rm -f "$key" echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" | $SUDO tee /etc/apt/sources.list.d/oneapi.list >/dev/null $SUDO apt-get update From a0bc6957255f2b121b541241e8102bc7a7a456d4 Mon Sep 17 00:00:00 2001 From: lusoris Date: Wed, 11 Nov 2026 01:48:06 +0100 Subject: [PATCH 4/6] fix(ci): honour curl -o in the ROCm test stub; require two wget tries Commit 575998cb8 made install-rocm-from-image.sh save the token and the manifest with `curl -o`, which is right for real curl. The test stub ignored `-o` and printed every body to stdout, so three tests in test_install_rocm_from_image.py failed in Tooling Tests with a missing .token.json. The stub now writes the body to the file named by `-o`, `--output` or `--output=`, and to stdout only when none is given; the blob path keeps its partial write and exit 92. A new test covers each output spelling. GNU Wget counts `--tries=N` as total attempts, so `--tries=1` never retries. The audit accepted it. It now requires N of at least 2 for wget (WGET_MIN_TRIES, through positive_count with a minimum), keeps N of at least 1 for curl `--retry` and APT `Acquire::Retries`, and has a negative test for 1 and a boundary test for 2. docs/development/ ci.md and the agent topic page say so. The topic page front matter now lists scripts/dev/tidy-lane.sh, whose APT retries it governs; scripts/ci/AGENTS-gates.md is regenerated. Signed-off-by: lusoris --- docs/development/ci.md | 4 +- scripts/ci/AGENTS-gates.md | 2 +- scripts/ci/AGENTS.d/fetch-connect-retry.md | 4 +- scripts/ci/tests/test_fetch_connect_retry.py | 40 ++++++++-- .../ci/tests/test_install_rocm_from_image.py | 74 ++++++++++++++++--- 5 files changed, 102 insertions(+), 22 deletions(-) diff --git a/docs/development/ci.md b/docs/development/ci.md index cc4dace58e..79dd7526d9 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -1164,8 +1164,8 @@ Every curl and wget HTTP(S) fetch in a shell script, workflow or container recipe retries connection failures: - **curl:** `--retry N --retry-all-errors`, with N positive. -- **wget:** `--tries=N --retry-connrefused --retry-on-host-error`, with N - positive. +- **wget:** `--tries=N --retry-connrefused --retry-on-host-error`, with N at + least 2 because Wget counts total attempts, including the first. - **APT in `scripts/dev/tidy-lane.sh`:** `-o Acquire::Retries=N`, with N a literal positive integer, on every `update` and `install` command. diff --git a/scripts/ci/AGENTS-gates.md b/scripts/ci/AGENTS-gates.md index f7a215f593..e61e4b3e0f 100644 --- a/scripts/ci/AGENTS-gates.md +++ b/scripts/ci/AGENTS-gates.md @@ -27,7 +27,7 @@ file of this directory. `Touching` paths: relative to this directory, leading | `check-dco.py`, `dco-cutoff.txt`, `tests/test_check_dco.py` | [dco-sign-off](AGENTS.d/dco-sign-off.md) | Non-merge PR commits require author/committer sign-off; bot exemption two-sided; fail closed. | | `/.config/lefthook/engine.sh`, `/.config/praetor/engine.mk`, `/lefthook.yml`, `/scripts/git-hooks/hiss-audit.sh`, `/scripts/githooks/state-sync.sh`, `/.config/lint-exceptions.d/shfmt.toml`, `tests/test_shfmt_scope.py` | [engine-launcher](AGENTS.d/engine-launcher.md) | Governance hooks reach engine only through `sh .config/lefthook/engine.sh`; launcher stays praetor's unedited text. | | `test_fail_closed_ci.py`, `/scripts/git-hooks/pre-push-mypy.py`, `/scripts/git-hooks/test-pre-push-mypy.py`, `/requirements/locks/mypy.txt` | [fail-closed-ci](AGENTS.d/fail-closed-ci.md) | Required Python Lint = hash-locked mypy via merge-base hook, full history, no exit masking; test stays wired. | -| `tests/test_fetch_connect_retry.py`, `install-rocm-from-image.sh`, `/docs/development/ci.md` | [fetch-connect-retry](AGENTS.d/fetch-connect-retry.md) | Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. | +| `tests/test_fetch_connect_retry.py`, `install-rocm-from-image.sh`, `/scripts/dev/tidy-lane.sh`, `/docs/development/ci.md` | [fetch-connect-retry](AGENTS.d/fetch-connect-retry.md) | Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. | | `ffmpeg-patches-surface-check.sh`, `tests/test_ffmpeg_patches_surface_check.py` | [ffmpeg-patches-surface](AGENTS.d/ffmpeg-patches-surface.md) | Tree checks run in hook (--surface-only) + CI; scan failure or zero docs fails closed; bash 3.2 safe. | | `check-issue-reference-provenance.py`, `tests/test_issue_reference_provenance.py` | [issue-reference-provenance](AGENTS.d/issue-reference-provenance.md) | Protected historical contexts keep their archived `lusoris/vmaf` identity; contracts stay context-scoped. | | `check-local-data-contract.sh`, `tests/test-check-local-data-contract.sh` | [local-data-roots](AGENTS.d/local-data-roots.md) | `.workingdir/` = private state, `.corpus/` = data, tracked files = public evidence; no tracked link into ignored data. | diff --git a/scripts/ci/AGENTS.d/fetch-connect-retry.md b/scripts/ci/AGENTS.d/fetch-connect-retry.md index 7373b8b288..25a66ce694 100644 --- a/scripts/ci/AGENTS.d/fetch-connect-retry.md +++ b/scripts/ci/AGENTS.d/fetch-connect-retry.md @@ -2,6 +2,7 @@ paths: - scripts/ci/tests/test_fetch_connect_retry.py - scripts/ci/install-rocm-from-image.sh + - scripts/dev/tidy-lane.sh - docs/development/ci.md invariant: Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. area: gates @@ -9,7 +10,8 @@ area: gates # Fetch connection retry contract - Curl: positive `--retry` + enabled `--retry-all-errors`. -- Wget: positive `--tries` + enabled `--retry-connrefused` + `--retry-on-host-error`. +- Wget: `--tries=N`, literal N >= 2 (total attempts) + enabled + `--retry-connrefused` + `--retry-on-host-error`. - Retried body -> fetch-owned file; consumer after success; cleanup after use. - Stdout body forbidden through pipe, capture, shell file redirection; `/dev/null` disposal allowed. diff --git a/scripts/ci/tests/test_fetch_connect_retry.py b/scripts/ci/tests/test_fetch_connect_retry.py index 1ac4956a0b..0ffbfe32ac 100644 --- a/scripts/ci/tests/test_fetch_connect_retry.py +++ b/scripts/ci/tests/test_fetch_connect_retry.py @@ -179,6 +179,8 @@ def logical_lines(text: str) -> list[tuple[int, str]]: ("spider", None, False, True), ) APT_RETRY_PATHS = ("scripts/dev/tidy-lane.sh",) +# GNU Wget --tries counts total attempts, so one attempt never retries. +WGET_MIN_TRIES = 2 @dataclass @@ -356,21 +358,24 @@ def fetch_commands(text: str) -> list[tuple[int, ParsedFetch]]: return commands -def positive_count(parsed: ParsedFetch, option: str) -> bool: - """Accept only literal positive integer retry counts from consumed values.""" - return bool(re.fullmatch(r"[1-9][0-9]*", parsed.values.get(option, ""))) +def positive_count(parsed: ParsedFetch, option: str, minimum: int = 1) -> bool: + """Accept only literal integer retry counts of at least ``minimum`` from consumed values.""" + value = parsed.values.get(option, "") + return bool(re.fullmatch(r"[1-9][0-9]*", value)) and int(value) >= minimum def retry_errors(parsed: ParsedFetch) -> list[str]: - """Require positive retries and the fetch tool's connection error switches.""" + """Require retry counts that retry and the fetch tool's connection error switches.""" if parsed.tool == "wget": if ( - positive_count(parsed, "tries") + positive_count(parsed, "tries", WGET_MIN_TRIES) and parsed.flags.get("retry-connrefused") and parsed.flags.get("retry-on-host-error") ): return [] - return ["wget needs positive --tries, --retry-connrefused and --retry-on-host-error"] + return [ + "wget needs --tries=N with N at least 2, --retry-connrefused and --retry-on-host-error" + ] errors: list[str] = [] if not positive_count(parsed, "retry"): errors.append("curl needs positive --retry") @@ -490,6 +495,29 @@ def test_wget_without_tries_fails(self) -> None: ) ) + def test_wget_one_attempt_has_no_retry(self) -> None: + """Reject one total attempt even when both connection switches are enabled.""" + self.assertEqual( + self.check_script( + "wget --tries=1 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x\n" + ), + [ + "fetch.sh:1: wget needs --tries=N with N at least 2, " + "--retry-connrefused and --retry-on-host-error" + ], + ) + + def test_wget_two_attempts_is_boundary(self) -> None: + """Accept two total attempts as the smallest retrying wget count.""" + self.assertEqual( + self.check_script( + "wget --tries=2 --retry-connrefused --retry-on-host-error " + "https://example.invalid/x\n" + ), + [], + ) + def test_wget_without_retry_connrefused_fails(self) -> None: """Reject wget without retries for refused or unreachable connections.""" self.assertTrue( diff --git a/scripts/ci/tests/test_install_rocm_from_image.py b/scripts/ci/tests/test_install_rocm_from_image.py index ce1d0b0a22..e1d31dd09e 100644 --- a/scripts/ci/tests/test_install_rocm_from_image.py +++ b/scripts/ci/tests/test_install_rocm_from_image.py @@ -25,6 +25,7 @@ EXPECTED_RETRY_CALL_COUNT = 2 NEGATIVE_ATTEMPT_COUNT = 3 EXPECTED_BOUNDARY_CALL_COUNT = 1 +CURL_PARTIAL_EXIT = 92 CURL_STUB = """#!/usr/bin/env python3 import json @@ -33,14 +34,29 @@ from pathlib import Path state_dir = Path(os.environ["FAKE_CURL_STATE_DIR"]) +args = sys.argv[1:] +output = None +for index, arg in enumerate(args): + if arg in ("-o", "--output") and index + 1 < len(args): + output = args[index + 1] + elif arg.startswith("--output="): + output = arg.partition("=")[2] + +def write_body(data): + if output is None or output == "-": + sys.stdout.buffer.write(data) + sys.stdout.buffer.flush() + else: + Path(output).write_bytes(data) + url = None -for arg in sys.argv[1:]: +for arg in args: if arg.startswith("http://") or arg.startswith("https://"): url = arg break if not url: - for arg in sys.argv[1:]: + for arg in args: if "token" in arg or "/manifests/" in arg or "/blobs/" in arg: url = arg break @@ -49,12 +65,12 @@ sys.exit(1) if "token" in url or "auth" in url: - sys.stdout.write(json.dumps({"token": "t"})) + write_body(json.dumps({"token": "t"}).encode()) sys.exit(0) if "/manifests/" in url: manifest_file = state_dir / "manifest.json" - sys.stdout.write(manifest_file.read_text(encoding="utf-8")) + write_body(manifest_file.read_bytes()) sys.exit(0) if "/blobs/" in url: @@ -79,19 +95,15 @@ if mode == "fail_once": if count == 1: - sys.stdout.buffer.write(data[: len(data) // 2]) - sys.stdout.buffer.flush() + write_body(data[: len(data) // 2]) sys.exit(92) - sys.stdout.buffer.write(data) - sys.stdout.buffer.flush() + write_body(data) sys.exit(0) elif mode == "always_fail": - sys.stdout.buffer.write(data[: len(data) // 2]) - sys.stdout.buffer.flush() + write_body(data[: len(data) // 2]) sys.exit(92) elif mode == "succeed": - sys.stdout.buffer.write(data) - sys.stdout.buffer.flush() + write_body(data) sys.exit(0) else: sys.exit(1) @@ -217,6 +229,44 @@ def _run_installer( ) +def test_curl_stub_output_modes(tmp_path: Path) -> None: + """Send bodies to each curl output spelling, or stdout when none is set.""" + bin_dir, state_dir, _, env = _setup_test_env(tmp_path) + manifest = b'{"layers": []}' + (state_dir / "manifest.json").write_bytes(manifest) + url = "https://registry.example.invalid/v2/rocm/manifests/test" + curl = str(bin_dir / "curl") + for index, option in enumerate(("-o", "--output", "--output=")): + output = tmp_path / f"manifest-{index}.json" + arguments = [f"{option}{output}"] if option.endswith("=") else [option, str(output)] + proc = subprocess.run( # noqa: S603 - test-owned stub and arguments + [curl, *arguments, url], env=env, capture_output=True, check=False, timeout=60 + ) + assert proc.returncode == 0 + assert proc.stdout == b"" + assert output.read_bytes() == manifest + + proc = subprocess.run( # noqa: S603 - test-owned stub and arguments + [curl, url], env=env, capture_output=True, check=False, timeout=60 + ) + assert proc.returncode == 0 + assert proc.stdout == manifest + + (state_dir / "mode.txt").write_text("always_fail", encoding="utf-8") + blob = (state_dir / "rocm.tar.gz").read_bytes() + output = tmp_path / "partial.tar.gz" + proc = subprocess.run( # noqa: S603 - test-owned stub and arguments + [curl, "--output", str(output), f"https://registry.example.invalid/blobs/{ROCM_DIGEST}"], + env=env, + capture_output=True, + check=False, + timeout=60, + ) + assert proc.returncode == CURL_PARTIAL_EXIT + assert proc.stdout == b"" + assert output.read_bytes() == blob[: len(blob) // 2] + + def test_positive_midstream_retry(tmp_path: Path) -> None: """Positive case: mid-stream failure once then success -> exit 0, dest contains the file.""" _, state_dir, dest_dir, env = _setup_test_env(tmp_path) From 77ca9792101ff8e8d91980161086fa20e3b44e59 Mon Sep 17 00:00:00 2001 From: lusoris Date: Wed, 11 Nov 2026 02:19:33 +0100 Subject: [PATCH 5/6] test(ci): run the curl stub through safe_subprocess The curl stub test called subprocess.run directly, and Codacy flagged the three calls (Bandit B603 and an Opengrep subprocess rule). The noqa comment silenced ruff only. The test now runs the stub through scripts.lib.safe_subprocess.run with the stub as the only allowed executable, as the other CI tests do. The assertions are unchanged. Signed-off-by: lusoris --- .../ci/tests/test_install_rocm_from_image.py | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/scripts/ci/tests/test_install_rocm_from_image.py b/scripts/ci/tests/test_install_rocm_from_image.py index e1d31dd09e..fb23a80d03 100644 --- a/scripts/ci/tests/test_install_rocm_from_image.py +++ b/scripts/ci/tests/test_install_rocm_from_image.py @@ -10,10 +10,15 @@ import os import stat import subprocess +import sys import tarfile from pathlib import Path from typing import Any +sys.path.insert(0, str(Path(__file__).resolve().parents[3])) + +from scripts.lib.safe_subprocess import run as run_command + REPO_ROOT = Path(__file__).resolve().parents[3] DEFAULT_SCRIPT = REPO_ROOT / "scripts" / "ci" / "install-rocm-from-image.sh" SCRIPT_PATH = Path(os.environ.get("VMAFX_INSTALL_ROCM_SCRIPT", str(DEFAULT_SCRIPT))) @@ -239,15 +244,23 @@ def test_curl_stub_output_modes(tmp_path: Path) -> None: for index, option in enumerate(("-o", "--output", "--output=")): output = tmp_path / f"manifest-{index}.json" arguments = [f"{option}{output}"] if option.endswith("=") else [option, str(output)] - proc = subprocess.run( # noqa: S603 - test-owned stub and arguments - [curl, *arguments, url], env=env, capture_output=True, check=False, timeout=60 + proc = run_command( + [curl, *arguments, url], + allowed_executables=(curl,), + env=env, + capture_output=True, + timeout_seconds=60, ) assert proc.returncode == 0 assert proc.stdout == b"" assert output.read_bytes() == manifest - proc = subprocess.run( # noqa: S603 - test-owned stub and arguments - [curl, url], env=env, capture_output=True, check=False, timeout=60 + proc = run_command( + [curl, url], + allowed_executables=(curl,), + env=env, + capture_output=True, + timeout_seconds=60, ) assert proc.returncode == 0 assert proc.stdout == manifest @@ -255,12 +268,12 @@ def test_curl_stub_output_modes(tmp_path: Path) -> None: (state_dir / "mode.txt").write_text("always_fail", encoding="utf-8") blob = (state_dir / "rocm.tar.gz").read_bytes() output = tmp_path / "partial.tar.gz" - proc = subprocess.run( # noqa: S603 - test-owned stub and arguments + proc = run_command( [curl, "--output", str(output), f"https://registry.example.invalid/blobs/{ROCM_DIGEST}"], + allowed_executables=(curl,), env=env, capture_output=True, - check=False, - timeout=60, + timeout_seconds=60, ) assert proc.returncode == CURL_PARTIAL_EXIT assert proc.stdout == b"" From 71414322a6135f37fde27546eb40eb51aba562cc Mon Sep 17 00:00:00 2001 From: lusoris Date: Wed, 11 Nov 2026 05:20:13 +0100 Subject: [PATCH 6/6] fix(ci): retry the NASM download in a loop on AlmaLinux 8 curl The release-build stage runs on manylinux_2_28 (AlmaLinux 8), whose curl 7.61.1 rejects --retry-all-errors (added in curl 7.71.0), so the image build failed with exit 2 on an unknown option. The NASM download now runs curl with --retry 3 --retry-connrefused inside a three-attempt loop, so refused and unreachable hosts are still retried. The fetch audit gains OLD_CURL_STAGES: every curl call in the named stage, exempt fetches included, is refused if it passes --retry-all-errors. A stale table row fails the audit. Checked in the pinned manylinux_2_28 image: curl 7.61.1 parses the new options, the loop gives up after three refused attempts with exit 1, the real download verifies against the pinned checksum, and --retry-all-errors exits 2. Signed-off-by: lusoris --- dev/Containerfile | 14 +- docs/development/ci.md | 18 +++ scripts/ci/AGENTS-gates.md | 2 +- scripts/ci/AGENTS.d/fetch-connect-retry.md | 11 ++ scripts/ci/tests/test_fetch_connect_retry.py | 135 ++++++++++++++++++- 5 files changed, 172 insertions(+), 8 deletions(-) diff --git a/dev/Containerfile b/dev/Containerfile index e7075e6794..5212ddcf5f 100644 --- a/dev/Containerfile +++ b/dev/Containerfile @@ -352,9 +352,17 @@ RUN for attempt in 1 2 3; do \ # AlmaLinux 8's assembler predates the AVX-512 sources. Build the maintained # assembler from the upstream release tarball, verified before extraction. -RUN curl --retry 3 --retry-all-errors --fail --location --show-error \ - --output /tmp/nasm.tar.xz \ - "https://www.nasm.us/pub/nasm/releasebuilds/${NATIVE_NASM_VERSION}/nasm-${NATIVE_NASM_VERSION}.tar.xz" \ +# AlmaLinux 8 curl 7.61 lacks --retry-all-errors (7.71); loop retries unreachable host/network. +RUN for attempt in 1 2 3; do \ + curl --retry 3 --retry-connrefused --fail --location --show-error \ + --output /tmp/nasm.tar.xz \ + "https://www.nasm.us/pub/nasm/releasebuilds/${NATIVE_NASM_VERSION}/nasm-${NATIVE_NASM_VERSION}.tar.xz" \ + && break || { \ + if [ "$attempt" -eq 3 ]; then exit 1; fi; \ + echo "nasm download attempt $attempt failed; retrying..." >&2; \ + sleep 5; \ + }; \ + done \ && echo "${NATIVE_NASM_SHA256} /tmp/nasm.tar.xz" | sha256sum --check --strict \ && mkdir /tmp/nasm-src \ && tar -xJf /tmp/nasm.tar.xz -C /tmp/nasm-src --strip-components=1 \ diff --git a/docs/development/ci.md b/docs/development/ci.md index 79dd7526d9..323293117e 100644 --- a/docs/development/ci.md +++ b/docs/development/ci.md @@ -1191,6 +1191,24 @@ need to fail fast. The ROCm layer download in tar pipeline up to `LAYER_ATTEMPTS` times; inner curl retries would append to the pipe, so this streaming fetch is exempt from both inner retries and file output. +Older curl stages use a bounded shell loop around the fetch plus an exact +`ALLOWLIST` exemption instead of `--retry-all-errors`. In `dev/Containerfile`, +the `release-build` stage uses manylinux_2_28 / AlmaLinux 8 curl 7.61.1; +`--retry-all-errors` requires 7.71.0. Its NASM download uses +`--retry 3 --retry-connrefused` inside a three-attempt loop, with a five-second +delay between attempts. Each attempt writes `/tmp/nasm.tar.xz` through curl's +`--output`; checksum verification, extraction and compilation follow success. +The outer loop repeats the whole fetch for unreachable hosts or networks too. + +`OLD_CURL_STAGES` in the same test module records `(path, stage name, reason)` +for stages with older curl. The audit rejects `--retry-all-errors` on every +parsed curl fetch from that stage's `FROM ... AS ` to the next `FROM`, +including exempt fetches. Missing files or stage names fail as stale entries. +To cover another old-curl stage, add its exact path, stage name and measured +curl version/unsupported flag to the table; give each loop-protected download +an exact URL exemption with its retry bound. Keep the planted rejection, +compatible-loop acceptance and next-stage boundary tests with this check. + **Not covered:** `git clone` and APT calls outside the tidy lane. Follow-up issue #3392 tracks these gaps; the tests whose docstrings start `Known gap (#3392):` replay both forms so the scanner's limits stay explicit. diff --git a/scripts/ci/AGENTS-gates.md b/scripts/ci/AGENTS-gates.md index e61e4b3e0f..c385421328 100644 --- a/scripts/ci/AGENTS-gates.md +++ b/scripts/ci/AGENTS-gates.md @@ -27,7 +27,7 @@ file of this directory. `Touching` paths: relative to this directory, leading | `check-dco.py`, `dco-cutoff.txt`, `tests/test_check_dco.py` | [dco-sign-off](AGENTS.d/dco-sign-off.md) | Non-merge PR commits require author/committer sign-off; bot exemption two-sided; fail closed. | | `/.config/lefthook/engine.sh`, `/.config/praetor/engine.mk`, `/lefthook.yml`, `/scripts/git-hooks/hiss-audit.sh`, `/scripts/githooks/state-sync.sh`, `/.config/lint-exceptions.d/shfmt.toml`, `tests/test_shfmt_scope.py` | [engine-launcher](AGENTS.d/engine-launcher.md) | Governance hooks reach engine only through `sh .config/lefthook/engine.sh`; launcher stays praetor's unedited text. | | `test_fail_closed_ci.py`, `/scripts/git-hooks/pre-push-mypy.py`, `/scripts/git-hooks/test-pre-push-mypy.py`, `/requirements/locks/mypy.txt` | [fail-closed-ci](AGENTS.d/fail-closed-ci.md) | Required Python Lint = hash-locked mypy via merge-base hook, full history, no exit masking; test stays wired. | -| `tests/test_fetch_connect_retry.py`, `install-rocm-from-image.sh`, `/scripts/dev/tidy-lane.sh`, `/docs/development/ci.md` | [fetch-connect-retry](AGENTS.d/fetch-connect-retry.md) | Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. | +| `tests/test_fetch_connect_retry.py`, `install-rocm-from-image.sh`, `/scripts/dev/tidy-lane.sh`, `/dev/Containerfile`, `/docs/development/ci.md` | [fetch-connect-retry](AGENTS.d/fetch-connect-retry.md) | Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. | | `ffmpeg-patches-surface-check.sh`, `tests/test_ffmpeg_patches_surface_check.py` | [ffmpeg-patches-surface](AGENTS.d/ffmpeg-patches-surface.md) | Tree checks run in hook (--surface-only) + CI; scan failure or zero docs fails closed; bash 3.2 safe. | | `check-issue-reference-provenance.py`, `tests/test_issue_reference_provenance.py` | [issue-reference-provenance](AGENTS.d/issue-reference-provenance.md) | Protected historical contexts keep their archived `lusoris/vmaf` identity; contracts stay context-scoped. | | `check-local-data-contract.sh`, `tests/test-check-local-data-contract.sh` | [local-data-roots](AGENTS.d/local-data-roots.md) | `.workingdir/` = private state, `.corpus/` = data, tracked files = public evidence; no tracked link into ignored data. | diff --git a/scripts/ci/AGENTS.d/fetch-connect-retry.md b/scripts/ci/AGENTS.d/fetch-connect-retry.md index 25a66ce694..024abbee8e 100644 --- a/scripts/ci/AGENTS.d/fetch-connect-retry.md +++ b/scripts/ci/AGENTS.d/fetch-connect-retry.md @@ -3,6 +3,7 @@ paths: - scripts/ci/tests/test_fetch_connect_retry.py - scripts/ci/install-rocm-from-image.sh - scripts/dev/tidy-lane.sh + - dev/Containerfile - docs/development/ci.md invariant: Curl/wget retries consume options correctly; retried bodies saved before use; tidy-lane APT retries positive. area: gates @@ -18,6 +19,16 @@ area: gates - Option allow-list: curl 8.23.0 `src/tool_getparam.c`; GNU Wget 1.25.0 `wget --help`. - Unknown option -> error; value consumes next argument; final switch wins. - Exemption shape: exact path + parsed URL + reason; stale/ambiguous entry -> failure. +- `OLD_CURL_STAGES`: exact path + stage name + reason; + absent file/stage -> stale error. +- Old-curl stage: parsed curl flags checked through next `FROM`; + exemptions included. +- `dev/Containerfile`, `release-build`: manylinux_2_28 / AlmaLinux 8 + curl 7.61.1; + `--retry-all-errors` needs 7.71.0 -> forbidden. +- NASM exemption: exact release URL; whole-fetch loop x3, + `--retry 3 --retry-connrefused`, curl-owned `/tmp/nasm.tar.xz`; + checksum/extract/build after successful fetch. - ROCm layer exemption: whole pipeline retried `LAYER_ATTEMPTS` times; inner retry appends partial bytes. - Tidy-lane APT `update`/`install`: `-o Acquire::Retries=N`, literal positive N. diff --git a/scripts/ci/tests/test_fetch_connect_retry.py b/scripts/ci/tests/test_fetch_connect_retry.py index 0ffbfe32ac..fb9345cec8 100644 --- a/scripts/ci/tests/test_fetch_connect_retry.py +++ b/scripts/ci/tests/test_fetch_connect_retry.py @@ -21,8 +21,15 @@ from scripts.lib.safe_subprocess import run as run_command # noqa: E402 # Curl/wget fetches + tidy-lane APT only; git clone/other APT gap: #3392. -# Exact path + parsed URL + reason; exempt commands skip every check. +# Exact path + parsed URL + reason; exemptions never skip old-curl stage checks. ALLOWLIST = ( + ( + "dev/Containerfile", + "https://www.nasm.us/pub/nasm/releasebuilds/${NATIVE_NASM_VERSION}/" + "nasm-${NATIVE_NASM_VERSION}.tar.xz", + "AlmaLinux 8 curl 7.61 lacks --retry-all-errors; " + "RUN loop repeats the whole fetch 3 times", + ), ( "scripts/ci/install-rocm-from-image.sh", "https://${REGISTRY}/v2/${REPO}/blobs/${digest}", @@ -70,6 +77,13 @@ "single non-idempotent scoring POST through the local port-forward; fail fast", ), ) +OLD_CURL_STAGES = ( + ( + "dev/Containerfile", + "release-build", + "manylinux_2_28 / AlmaLinux 8 curl 7.61.1; --retry-all-errors needs 7.71.0", + ), +) FETCH = re.compile(r"(? bool: return bool(retries and re.fullmatch(r"Acquire::Retries=[1-9][0-9]*", retries[-1])) +def stage_fetches(text: str, stage: str) -> tuple[bool, list[tuple[int, ParsedFetch]]]: + """Find a named stage and its parsed fetches, stopping at every subsequent FROM.""" + found = False + active = False + commands: list[tuple[int, ParsedFetch]] = [] + for line, source_line in logical_lines(text): + command = strip_comment(source_line).strip() + if re.match(r"FROM\s+", command, re.IGNORECASE): + alias = re.search(r"\s+AS\s+(\S+)\s*$", command, re.IGNORECASE) + active = alias is not None and alias[1].casefold() == stage.casefold() + found |= active + elif active: + commands.extend((line, parsed) for _, parsed in fetch_commands(command)) + return found, commands + + +def old_curl_errors( + sources: list[tuple[str, str]], stages: tuple[tuple[str, str, str], ...] +) -> list[str]: + """Reject unsupported curl flags in declared stages, even on exempt fetches.""" + errors: list[str] = [] + by_path = dict(sources) + for path, stage, reason in stages: + found, commands = stage_fetches(by_path.get(path, ""), stage) + if not found: + errors.append(f"{path}: stale old-curl stage entry: {stage} ({reason})") + for line, parsed in commands: + if parsed.tool == "curl" and "retry-all-errors" in parsed.flags: + errors.append( + f"{path}:{line}: stage {stage}: curl cannot use --retry-all-errors ({reason})" + ) + return errors + + def audit_sources( - sources: list[tuple[str, str]], allowlist: tuple[tuple[str, str, str], ...] = ALLOWLIST + sources: list[tuple[str, str]], + allowlist: tuple[tuple[str, str, str], ...] = ALLOWLIST, + old_curl_stages: tuple[tuple[str, str, str], ...] = (), ) -> list[str]: """Audit parsed fetches and APT while rejecting stale or ambiguous exemptions.""" - errors: list[str] = [] + errors = old_curl_errors(sources, old_curl_stages) seen: Counter[tuple[str, str, str]] = Counter() for path, text in sources: errors.extend(apt_errors(path, text)) @@ -824,9 +874,86 @@ def test_url_exemption(self) -> None: audit_sources([("fetch.sh", "curl --url=http://localhost:8080/other")], entry) ) + def check_old_curl(self, text: str) -> list[str]: + """Audit an exempt fixture fetch with the live old-curl stage table enabled.""" + entry = (("dev/Containerfile", "https://example.invalid/x", "bounded outer retry"),) + return audit_sources([("dev/Containerfile", text)], entry, OLD_CURL_STAGES) + + def test_old_curl_stage_rejects_all_errors_on_exempt_fetch(self) -> None: + """Reject the planted 41b288ab0 fetch even when its URL is exempt.""" + errors = self.check_old_curl( + "FROM x AS release-build\n" + "RUN curl --retry 3 --retry-all-errors -o out https://example.invalid/x\n" + ) + self.assertEqual( + errors, + [ + "dev/Containerfile:2: stage release-build: curl cannot use --retry-all-errors " + f"({OLD_CURL_STAGES[0][2]})" + ], + ) + + def test_old_curl_stage_accepts_whole_fetch_loop(self) -> None: + """Accept the bounded loop with curl 7.61-compatible retry options.""" + self.assertEqual( + self.check_old_curl( + "FROM x AS release-build\n" + "RUN for attempt in 1 2 3; do \\\n" + " curl --retry 3 --retry-connrefused --fail --location --show-error \\\n" + " --output out https://example.invalid/x \\\n" + " && break || { \\\n" + ' if [ "$attempt" -eq 3 ]; then exit 1; fi; \\\n' + ' echo "attempt $attempt failed; retrying..." >&2; sleep 5; \\\n' + " }; \\\n" + "done\n" + ), + [], + ) + + def test_old_curl_stage_ends_at_next_from(self) -> None: + """Keep all-errors retries in later named or unnamed stages outside the check.""" + for boundary in ("FROM y AS later", "FROM y", "from --platform=$BUILDPLATFORM y as later"): + with self.subTest(boundary=boundary): + self.assertEqual( + self.check_old_curl( + "FROM x AS release-build\n" + f"{boundary}\n" + "RUN curl --retry 3 --retry-all-errors -o out https://example.invalid/x\n" + ), + [], + ) + + def test_old_curl_stage_missing_file_or_stage_is_stale(self) -> None: + """Reject stale table rows for an absent source file or absent stage.""" + for sources in ([], [("dev/Containerfile", "FROM x AS other\n")]): + with self.subTest(sources=sources): + self.assertEqual( + audit_sources(sources, (), OLD_CURL_STAGES), + [ + "dev/Containerfile: stale old-curl stage entry: release-build " + f"({OLD_CURL_STAGES[0][2]})" + ], + ) + + def test_old_curl_stage_uses_parsed_options(self) -> None: + """Ignore option-looking values while rejecting unsupported disabled flags too.""" + self.assertEqual( + self.check_old_curl( + "FROM x AS release-build\n" + "RUN curl --output --retry-all-errors https://example.invalid/x\n" + ), + [], + ) + self.assertTrue( + self.check_old_curl( + "FROM x AS release-build\n" + "RUN curl --no-retry-all-errors -o out https://example.invalid/x\n" + ) + ) + def test_tracked_fetches_retry_connect_failures(self) -> None: """Enforce the retry contract across the live tracked site inventory.""" - self.assertEqual(audit_sources(tracked_sources()), []) + self.assertEqual(audit_sources(tracked_sources(), old_curl_stages=OLD_CURL_STAGES), []) if __name__ == "__main__":