fix: retry transient pinned tool downloads

docs/decisions/issue-1137-ci-release-download-retries.md

@@ -0,0 +1,153 @@
+# Issue 1137: Bounded CI Release-Download Retries
+
+Date: 2026-08-12
+
+Issue: #1137. Requirement: GOV-913.
+
+## Context And Gap
+
+OpenRAE's required verification lanes acquire pinned Conftest, Vale, Gitleaks,
+and OSV Scanner artifacts from their canonical GitHub Releases repositories.
+The owning installers validate checksums before executing or extracting those
+artifacts. When #1137 was captured during review of several independent pull
+requests, GitHub returned
+temporary disconnects and HTTP 503 responses while acquiring all four tool
+families. A single such response failed the lane before it could inspect the
+pull request's code; rerunning the unchanged commit succeeded once the release
+service recovered.
+
+While this remediation was in flight, PR #1113 merged a shared
+`tools/http_download.py` helper that makes up to five attempts for a small
+HTTP-status set and broad `OSError` failures. That change absorbs common
+single-attempt disconnects, but it does not govern automatic redirects, apply
+one wall-clock deadline across status, header, framing, and body reads, bound
+every response, or prevent chained transport exceptions from reaching
+diagnostics. This note therefore defines the hardened acquisition boundary
+that replaces that coarse retry implementation. It changes neither the
+verification graph nor the meaning of a passing gate.
+
+## Existing-Surface Review
+
+The review covered the four repository-local installers, their version pins,
+checksum sources, archive extraction, cache paths, nox callers, required CI
+jobs, and the shared `tools/http_download.py` added by #1113. Rather than leave
+two retry implementations, `tools/release_download.py` is the sole owner of
+retry, redirect, deadline, framing, and response-size policy. The earlier
+`download_bytes` entry point remains only as a compatibility facade: it
+delegates to the governed implementation and caps legacy attempt, timeout, and
+size options at the new policy. Runtime OCI/module downloads, vocabulary-source
+refreshes, Isabelle's separately governed official-mirror policy, and GitHub
+Actions artifact transfer are different trust or ownership boundaries and
+remain out of scope.
+
+PR #1121 independently hardens OSV Scanner's cache type, identity, digest,
+atomic-install, and size checks. The retry helper composes below that work: it
+only returns bounded bytes from the already approved OSV release origin, while
+the OSV installer remains the sole owner of cache and checksum validation.
+
+## Chosen Policy
+
+All four installers bind their release reads to one internal helper. The
+compatibility `download_bytes` entry point delegates to the same helper and has
+no independent retry or redirect implementation.
+
+The initial request accepts only exact HTTPS paths under these GitHub Release
+families:
+
+- `errata-ai/vale`;
+- `gitleaks/gitleaks`;
+- `google/osv-scanner`; and
+- `open-policy-agent/conftest`.
+
+Redirects are handled explicitly instead of delegated to `urllib`. A request
+may follow at most three hops, and each transition must be one of the current
+GitHub-controlled release paths: the exact `errata-ai/vale` to `vale-cli/vale`
+repository relocation with an unchanged release suffix, or an approved GitHub
+release URL to an exact `release-assets.githubusercontent.com` production-asset
+path. Only that final asset URL may contain GitHub's ephemeral signed query;
+the query is never included in a diagnostic, and the asset host cannot redirect
+again. Relative, HTTP, cross-origin, credential-bearing, ambiguous, cyclic,
+oversized, and other unapproved locations fail without retry. Redirect response
+bodies are closed without being read.
+
+The helper performs at most three attempts. Every connection hop and every
+bounded body read receives a finite 60-second socket timeout capped by the
+remaining 190-second total deadline. A caller may lower that per-operation
+timeout but cannot raise the policy cap. Exponential delays are deterministic
+and capped, and a deadline-aware socket reader reapplies the remaining bound
+to every status-line, header, chunk-framing, and body buffer fill. No read,
+redirect hop, sleep, or new attempt may continue after the deadline. Declared
+response lengths are checked before and after streaming; ambiguous framing and
+oversized bodies fail immediately, while an early EOF is classified as a
+retryable incomplete transfer. Each response is bounded to 256 MiB before it
+is returned to an installer. There is no jitter, alternate mirror, unpinned
+version, credential, or redirect-selection fallback.
+
+Only these failures are retryable:
+
+- HTTP 408 and 429;
+- HTTP 500 through 599; and
+- timeouts, remote disconnects, incomplete responses, and connection failures.
+
+`Retry-After` delta-seconds and HTTP-date values are honored, but an individual
+value is capped at ten seconds and can never extend the total deadline. All
+other HTTP and URL failures stop after the first attempt. Final diagnostics
+include only the approved URL path, attempt count, and stable failure class;
+they omit response bodies and exception text. Malformed HTTP status, header,
+and framing exceptions are normalized to one non-retryable diagnostic rather
+than reflecting upstream text.
+
+The helper buffers a successful bounded response and then returns control to
+the owning installer. Consequently, checksum mismatch, signature failure,
+archive/type rejection, cache identity failure, and extraction failure happen
+outside the retry loop and cannot cause a second download. Required lanes still
+fail closed when every transient attempt fails.
+
+## Alternatives Rejected
+
+- **Keep the #1113 helper unchanged.** Its automatic redirect behavior, broad
+  transport retry class, per-operation timeout, unbounded default reads, and
+  chained exception diagnostics do not satisfy the issue's trust and total-time
+  acceptance criteria.
+- **Retain both retry implementations.** A permissive legacy path beside the
+  governed path would leave future callers with two conflicting trust
+  boundaries. The compatibility API therefore delegates to the sole hardened
+  implementation and cannot raise its limits.
+- **Retry each installer independently.** This would create four subtly
+  different status lists, delays, deadlines, and diagnostics that could drift.
+- **Retry every `URLError` or every HTTP failure.** Certificate, DNS-policy,
+  authorization, missing-asset, and malformed-request failures are not evidence
+  of a safe transient event. Retrying them obscures configuration or trust
+  failures.
+- **Retry after checksum or extraction failure.** A successful transfer of
+  invalid bytes is an integrity event, not availability noise. Retrying could
+  conceal unstable or malicious upstream content.
+- **Use a mirror or latest-version fallback.** This would bypass the reviewed
+  origin, version, and digest boundary.
+- **Rely on manual workflow reruns.** Manual reruns provide no deterministic
+  attempt/time bound and consume reviewer attention without increasing
+  assurance.
+
+## Verification And Nonclaims
+
+Deterministic tests cover every approved origin and redirect transition,
+ambiguous URL and response-framing rejection, redirect cycles and hop bounds,
+success after transient HTTP and transport failures, incomplete declared
+responses, delta and HTTP-date `Retry-After`, read/backoff/deadline caps, exact
+exhaustion, non-retryable HTTP and URL failures, response-size rejection,
+sanitized diagnostics, and no second request after checksum or archive-type
+failure. Malformed redirect authorities, including parser-error and Unicode
+normalization cases, are rejected through the same sanitized public error
+boundary without reflecting their text. Local scripted HTTP servers verify
+that an early EOF never returns partial bytes, redirect bodies are not drained,
+and a trickling body cannot extend the total deadline. The same scripted
+transport verifies that trickled redirect headers and chunk trailers cannot
+extend that deadline and that a
+malformed status line cannot leak upstream text. A binding test ensures all
+four installers continue to share the helper, and clean-cache smoke tests
+exercise the live current GitHub redirect chain for all four pinned tools.
+
+This policy does not make GitHub availability hermetic, validate remote bytes,
+or replace the installers' pins, checksums, signatures, archive rules, cache
+hardening, or execution gates. It only absorbs a small, explicitly classified
+window of transient release-service failure.

docs/requirements/GOV-913/requirement.md

@@ -25,6 +25,16 @@ Requirement inventory expansion. Reusable ecosystem assets need explicit trust a
 - IMPLEMENTS → SPEC `contracts/schemas/asset-trust/reusable-asset-trust-policy-v1.json` (reusable-asset-trust-policy-v1 published schema)
 - IMPLEMENTS → ADR `docs/decisions/adrs/adr-071-reusable-asset-trust-and-integrity-policy.md` (ADR-071: Reusable Asset Trust and Integrity Policy)
 - TESTS → TEST `implementations/python/tests/test_reusable_asset_trust_policy.py` (Reusable-asset trust policy contract tests)
+- IMPLEMENTS → GITHUB_ISSUE `1137` (Bounded retries for pinned CI tool downloads)
+- IMPLEMENTS → CODE_FILE `tools/release_download.py` (Approved-origin, bounded transient retry policy)
+- IMPLEMENTS → CODE_FILE `tools/http_download.py` (Compatibility facade for the governed release boundary)
+- IMPLEMENTS → CODE_FILE `tools/policy/conftest_tool.py` (Conftest release acquisition adapter)
+- IMPLEMENTS → CODE_FILE `tools/vale_tool.py` (Vale release acquisition adapter)
+- IMPLEMENTS → CODE_FILE `tools/gitleaks_tool.py` (Gitleaks release acquisition adapter)
+- IMPLEMENTS → CODE_FILE `tools/osv_scanner_tool.py` (OSV Scanner release acquisition adapter)
+- TESTS → TEST `implementations/python/tests/test_release_download.py` (Retry classification, bounds, and integrity non-retry regressions)
+- TESTS → TEST `implementations/python/tests/test_http_download.py` (Compatibility delegation and policy-cap regressions)
+- DOCUMENTS → DOCUMENTATION `docs/decisions/issue-1137-ci-release-download-retries.md` (Release-download retry decision)
 - IMPLEMENTS → GITHUB_ISSUE `115` (Trust & integrity of reusable assets (GOV-913))
 - IMPLEMENTS → GITHUB_ISSUE `1098` (Upgrade vulnerable Click and cryptography locks and gate OSV findings)
 - IMPLEMENTS → CONFIG `implementations/python/pyproject.toml` (Fixed Click and cryptography dependency floors)

implementations/python/pyproject.toml

@@ -182,7 +182,6 @@ ignore = [
 
 [tool.ruff.lint.per-file-ignores]
 "tests/**" = ["S101", "S105", "S106", "S108", "E402", "SIM105", "F841"]
-"tools/http_download.py" = ["S310"]  # validates absolute HTTPS before opening pinned release assets
 "packages/raes_contracts/contracts/__init__.py" = ["F403", "F405"]  # intentional package re-exports
 "packages/raes_cli/**" = ["B008"]      # typer requires function calls in defaults
 "packages/raes_runtime/control_plane.py" = ["S112"]      # intentional exception suppression

implementations/python/tests/test_http_download.py

@@ -1,94 +1,145 @@
-"""Tests for bounded repository-tool downloads."""
+"""Compatibility tests for governed repository-tool downloads."""
 
 from __future__ import annotations
 
-from http.client import RemoteDisconnected
-from urllib.error import HTTPError
+import io
 
 import pytest
-from tools.http_download import download_bytes
+import tools.http_download as http_download
+from tools.release_download import DEFAULT_RELEASE_RETRY_POLICY, ReleaseDownloadError, ReleaseRetryPolicy
 
+RELEASE_URL = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_checksums.txt"
 
-class _Response:
-    def __init__(self, payload: bytes) -> None:
-        self.payload = payload
 
-    def __enter__(self) -> _Response:
-        return self
+def test_download_bytes_delegates_to_the_governed_release_boundary(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    observed: dict[str, object] = {}
 
-    def __exit__(self, *args: object) -> None:
-        return None
+    def governed_open(url: str, *, timeout: float, policy: ReleaseRetryPolicy) -> io.BytesIO:
+        observed.update(url=url, timeout=timeout, policy=policy)
+        return io.BytesIO(b"verified asset")
 
-    def read(self, size: int = -1) -> bytes:
-        return self.payload if size < 0 else self.payload[:size]
+    monkeypatch.setattr(http_download, "retrying_urlopen", governed_open)
 
+    assert http_download.download_bytes(RELEASE_URL, description="gitleaks checksums") == b"verified asset"
+    assert observed["url"] == RELEASE_URL
+    assert observed["timeout"] == 60
+    policy = observed["policy"]
+    assert isinstance(policy, ReleaseRetryPolicy)
+    assert policy.max_attempts == DEFAULT_RELEASE_RETRY_POLICY.max_attempts
+    assert policy.maximum_response_bytes == DEFAULT_RELEASE_RETRY_POLICY.maximum_response_bytes
 
-def test_download_retries_transient_disconnects_with_bounded_backoff() -> None:
-    calls = 0
-    delays: list[float] = []
 
-    def opener(_url: str, *, timeout: float) -> _Response:
-        nonlocal calls
-        calls += 1
-        assert timeout == 60
-        if calls < 3:
-            raise RemoteDisconnected("transient release-asset disconnect")
-        return _Response(b"verified asset")
+def test_download_bytes_preserves_explicit_attempt_timeout_and_size_options(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    observed: dict[str, object] = {}
+
+    def governed_open(url: str, *, timeout: float, policy: ReleaseRetryPolicy) -> io.BytesIO:
+        observed.update(url=url, timeout=timeout, policy=policy)
+        return io.BytesIO(b"four")
+
+    monkeypatch.setattr(http_download, "retrying_urlopen", governed_open)
 
     assert (
-        download_bytes(
-            "https://example.invalid/pinned-tool",
-            description="pinned tool",
-            _opener=opener,
-            _sleeper=delays.append,
+        http_download.download_bytes(
+            RELEASE_URL,
+            description="gitleaks checksums",
+            attempts=2,
+            timeout_seconds=12,
+            max_bytes=4,
         )
-        == b"verified asset"
+        == b"four"
     )
-    assert calls == 3
-    assert delays == [1.0, 2.0]
+    assert observed["url"] == RELEASE_URL
+    assert observed["timeout"] == 12
+    policy = observed["policy"]
+    assert isinstance(policy, ReleaseRetryPolicy)
+    assert policy.max_attempts == 2
+    assert policy.maximum_response_bytes == 4
+
 
+def test_download_bytes_caps_legacy_options_at_the_governed_policy(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    observed: dict[str, object] = {}
 
-def test_download_exhausts_a_bounded_retry_window() -> None:
-    delays: list[float] = []
+    def governed_open(url: str, *, timeout: float, policy: ReleaseRetryPolicy) -> io.BytesIO:
+        observed.update(url=url, timeout=timeout, policy=policy)
+        return io.BytesIO(b"verified asset")
 
-    def opener(_url: str, *, timeout: float) -> _Response:
-        raise RemoteDisconnected("persistent release-asset disconnect")
+    monkeypatch.setattr(http_download, "retrying_urlopen", governed_open)
 
-    with pytest.raises(RuntimeError, match=r"after 5 attempt\(s\): RemoteDisconnected"):
-        download_bytes(
-            "https://example.invalid/pinned-tool",
-            description="pinned tool",
-            _opener=opener,
-            _sleeper=delays.append,
+    assert (
+        http_download.download_bytes(
+            RELEASE_URL,
+            description="gitleaks checksums",
+            attempts=99,
+            timeout_seconds=999,
+            max_bytes=DEFAULT_RELEASE_RETRY_POLICY.maximum_response_bytes + 1,
         )
+        == b"verified asset"
+    )
+    assert observed["timeout"] == DEFAULT_RELEASE_RETRY_POLICY.request_timeout_seconds
+    policy = observed["policy"]
+    assert isinstance(policy, ReleaseRetryPolicy)
+    assert policy.max_attempts == DEFAULT_RELEASE_RETRY_POLICY.max_attempts
+    assert policy.maximum_response_bytes == DEFAULT_RELEASE_RETRY_POLICY.maximum_response_bytes
+
+
+@pytest.mark.parametrize(
+    "options",
+    [
+        {"attempts": 0},
+        {"max_bytes": -1},
+        {"timeout_seconds": 0},
+    ],
+)
+def test_download_bytes_rejects_invalid_compatibility_options_before_network(
+    monkeypatch: pytest.MonkeyPatch,
+    options: dict[str, int],
+) -> None:
+    monkeypatch.setattr(
+        http_download,
+        "retrying_urlopen",
+        lambda *_args, **_kwargs: pytest.fail("invalid options reached the governed downloader"),
+    )
 
-    assert delays == [1.0, 2.0, 4.0, 8.0]
+    with pytest.raises(ValueError):
+        http_download.download_bytes(RELEASE_URL, description="gitleaks checksums", **options)
 
 
-def test_download_does_not_retry_non_transient_http_status() -> None:
-    calls = 0
+def test_download_bytes_preserves_a_zero_byte_caller_bound(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    observed: dict[str, object] = {}
 
-    def opener(url: str, *, timeout: float) -> _Response:
-        nonlocal calls
-        calls += 1
-        raise HTTPError(url, 404, "not found", {}, None)
+    def governed_open(_url: str, *, timeout: float, policy: ReleaseRetryPolicy) -> io.BytesIO:
+        observed.update(timeout=timeout, policy=policy)
+        return io.BytesIO(b"x")
 
-    with pytest.raises(RuntimeError, match=r"after 1 attempt\(s\): HTTPError"):
-        download_bytes(
-            "https://example.invalid/missing-tool",
-            description="missing tool",
-            _opener=opener,
-            _sleeper=lambda _delay: None,
-        )
-    assert calls == 1
+    monkeypatch.setattr(http_download, "retrying_urlopen", governed_open)
 
+    with pytest.raises(RuntimeError, match="gitleaks checksums exceeds the download limit"):
+        http_download.download_bytes(RELEASE_URL, description="gitleaks checksums", max_bytes=0)
 
-def test_download_enforces_the_requested_size_bound() -> None:
-    with pytest.raises(RuntimeError, match="exceeds the download limit"):
-        download_bytes(
-            "https://example.invalid/pinned-tool",
-            description="pinned tool",
-            max_bytes=3,
-            _opener=lambda _url, **_kwargs: _Response(b"four"),
-            _sleeper=lambda _delay: None,
-        )
+    policy = observed["policy"]
+    assert isinstance(policy, ReleaseRetryPolicy)
+    assert policy.maximum_response_bytes == 1
+
+
+def test_download_bytes_wraps_only_the_stable_governed_failure(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    detail = "release download exhausted 3 bounded attempts: HTTP 503"
+
+    def fail(_url: str, **_kwargs: object) -> io.BytesIO:
+        raise ReleaseDownloadError(detail)
+
+    monkeypatch.setattr(http_download, "retrying_urlopen", fail)
+
+    with pytest.raises(RuntimeError, match="failed to download gitleaks checksums") as raised:
+        http_download.download_bytes(RELEASE_URL, description="gitleaks checksums")
+
+    assert detail in str(raised.value)