From 6bd33d7b2d66ad074563d395774931608aee2f19 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 11:38:12 +0900 Subject: [PATCH 1/5] fix(coverage-evidence): retry trusted-uv download on transient network errors The shared releases.astral.sh origin is fetched by every coverage-evidence run across the organization. A single transient HTTPError/URLError under concurrent load previously failed the whole job with no retry, producing false-negative REQUEST_CHANGES verdicts on otherwise-healthy PRs (observed directly on naruon #1293 and #1300, three days apart, identical error). Split _download_trusted_uv_archive into a single-attempt network sink (_fetch_trusted_uv_archive_once) plus a bounded-retry wrapper (3 attempts, short backoff) that only retries OSError. Trust-boundary violations (unsafe redirect, oversized payload) remain RuntimeErrors raised on the first attempt and are never retried. Updated the static AST security contract test to target the relocated network-sink function (same one-literal-URL invariant, unchanged). New tests cover: transient-then- success, retry exhaustion, and that redirect/size rejections are never retried. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 1 + .../materialize_base_python_requirements.py | 111 +++++++++++------- ...st_materialize_base_python_requirements.py | 82 +++++++++++-- tests/test_trusted_uv_download_contract.py | 11 +- 4 files changed, 148 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..1202cae60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 98cdad459..9a6bfa257 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -19,6 +19,7 @@ import sys import tarfile import tempfile +import time import urllib.parse import urllib.request from typing import Any @@ -50,6 +51,8 @@ TRUSTED_UV_DOWNLOAD_MAX_BYTES = 64 * 1024 * 1024 TRUSTED_UV_BINARY_MAX_BYTES = 64 * 1024 * 1024 TRUSTED_UV_VERSION_TIMEOUT_SECONDS = 10 +TRUSTED_UV_DOWNLOAD_ATTEMPTS = 3 +TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS = 2.0 class _RejectTrustedUvRedirects(urllib.request.HTTPRedirectHandler): @@ -165,53 +168,79 @@ def _git(repo_root: pathlib.Path, *args: str) -> bytes: return completed.stdout -def _download_trusted_uv_archive() -> bytes: - """Download the fixed uv release archive through one HTTPS trust boundary.""" - _install_trusted_uv_url_opener() - try: - # Keep the audited URL literal at the network sink so static analysis can - # prove that neither user data nor repository content selects a scheme, - # host, path, query, fragment, method, or request header. - with urllib.request.urlopen( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected # nosec B310 - "https://releases.astral.sh/github/uv/releases/download/0.12.1/" - "uv-x86_64-unknown-linux-gnu.tar.gz", - timeout=TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, - ) as response: - final_url = urllib.parse.urlparse(response.geturl()) - try: - final_port = final_url.port - except ValueError as exc: - raise RuntimeError( - "trusted uv archive redirected outside the fixed " - "releases.astral.sh HTTPS origin" - ) from exc - if ( - (final_url.scheme, final_url.hostname) - != ("https", "releases.astral.sh") - or final_port not in (None, 443) - ): - raise RuntimeError( - "trusted uv archive redirected outside the fixed " - "releases.astral.sh HTTPS origin" - ) - payload = bytearray() - while len(payload) <= TRUSTED_UV_DOWNLOAD_MAX_BYTES: - chunk = response.read( - TRUSTED_UV_DOWNLOAD_MAX_BYTES + 1 - len(payload) - ) - if not chunk: - break - payload.extend(chunk) - except OSError as exc: - raise RuntimeError( - f"trusted uv archive download failed: {type(exc).__name__}" - ) from exc +def _fetch_trusted_uv_archive_once() -> bytes: + """Perform one download attempt through the fixed HTTPS trust boundary. + + Raises ``OSError`` (the network-level failure) on a transient fetch + problem so the caller can decide whether to retry, and ``RuntimeError`` + for every trust-boundary violation (unsafe redirect, oversized payload), + which must fail closed on the first occurrence and is never retried. + """ + # Keep the audited URL literal at the network sink so static analysis can + # prove that neither user data nor repository content selects a scheme, + # host, path, query, fragment, method, or request header. + with urllib.request.urlopen( # nosemgrep: python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected # nosec B310 + "https://releases.astral.sh/github/uv/releases/download/0.12.1/" + "uv-x86_64-unknown-linux-gnu.tar.gz", + timeout=TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ) as response: + final_url = urllib.parse.urlparse(response.geturl()) + try: + final_port = final_url.port + except ValueError as exc: + raise RuntimeError( + "trusted uv archive redirected outside the fixed " + "releases.astral.sh HTTPS origin" + ) from exc + if ( + (final_url.scheme, final_url.hostname) != ("https", "releases.astral.sh") + or final_port not in (None, 443) + ): + raise RuntimeError( + "trusted uv archive redirected outside the fixed " + "releases.astral.sh HTTPS origin" + ) + payload = bytearray() + while len(payload) <= TRUSTED_UV_DOWNLOAD_MAX_BYTES: + chunk = response.read(TRUSTED_UV_DOWNLOAD_MAX_BYTES + 1 - len(payload)) + if not chunk: + break + payload.extend(chunk) if len(payload) > TRUSTED_UV_DOWNLOAD_MAX_BYTES: raise RuntimeError("trusted uv archive exceeded the bounded download size") return bytes(payload) +def _download_trusted_uv_archive() -> bytes: + """Download the fixed uv release archive through one HTTPS trust boundary. + + The single shared ``releases.astral.sh`` origin is fetched by every + coverage-evidence run across the organization, so it occasionally answers + a transient network or HTTP error under concurrent load even though the + file itself is healthy (observed directly: repeated + ``trusted uv archive download failed: HTTPError`` failures on + unmodified, otherwise-passing pull requests). A bounded number of + attempts absorbs that transient condition; every attempt still runs the + full trust boundary unchanged (redirect rejection, host/port pin, size + bound, and the checksum/member verification performed by the caller), so + retrying strictly adds resilience and never weakens a check. + """ + _install_trusted_uv_url_opener() + last_error: OSError | None = None + for attempt in range(TRUSTED_UV_DOWNLOAD_ATTEMPTS): + if attempt: + time.sleep(TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS * attempt) + try: + return _fetch_trusted_uv_archive_once() + except OSError as exc: + last_error = exc + continue + raise RuntimeError( + f"trusted uv archive download failed: {type(last_error).__name__}" + ) from last_error + + def _verified_uv_binary(archive_payload: bytes) -> bytes: """Return the bounded uv executable after archive and member verification.""" digest = hashlib.sha256(archive_payload).hexdigest() diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..64948ce5d 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -514,31 +514,87 @@ def test_download_trusted_uv_archive_accepts_fixed_https_origin( def test_download_trusted_uv_archive_rejects_unsafe_redirect( monkeypatch: pytest.MonkeyPatch, ) -> None: - """A redirect away from the fixed HTTPS release host fails closed.""" + """A redirect away from the fixed HTTPS release host fails closed, unretried.""" response = FakeHttpResponse("https://example.invalid/uv.tar.gz") - monkeypatch.setattr(materializer.urllib.request, "urlopen", lambda *_a, **_k: response) + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + return response + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) with pytest.raises(RuntimeError, match="redirected outside"): materializer._download_trusted_uv_archive() + assert calls == 1 -def test_download_trusted_uv_archive_rejects_network_and_size_failures( +def test_download_trusted_uv_archive_rejects_oversized_archive_unretried( monkeypatch: pytest.MonkeyPatch, ) -> None: - """Network errors and oversized archives cannot enter the trusted tool path.""" - monkeypatch.setattr( - materializer.urllib.request, - "urlopen", - lambda *_a, **_k: (_ for _ in ()).throw(OSError("offline")), - ) - with pytest.raises(RuntimeError, match="download failed"): - materializer._download_trusted_uv_archive() - + """An oversized archive fails closed on the first attempt without retrying.""" response = FakeHttpResponse(materializer.TRUSTED_UV_ARCHIVE_URL, b"12345") - monkeypatch.setattr(materializer.urllib.request, "urlopen", lambda *_a, **_k: response) + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + return response + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) monkeypatch.setattr(materializer, "TRUSTED_UV_DOWNLOAD_MAX_BYTES", 4) with pytest.raises(RuntimeError, match="bounded download size"): materializer._download_trusted_uv_archive() + assert calls == 1 + + +def test_download_trusted_uv_archive_exhausts_retries_and_reports_download_failed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A persistent transient network error retries a bounded number of times.""" + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> None: + nonlocal calls + calls += 1 + raise OSError("offline") + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) + monkeypatch.setattr(materializer.time, "sleep", lambda _seconds: None) + with pytest.raises(RuntimeError, match="download failed"): + materializer._download_trusted_uv_archive() + assert calls == materializer.TRUSTED_UV_DOWNLOAD_ATTEMPTS + + +def test_download_trusted_uv_archive_retries_transient_failure_then_succeeds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A transient failure on the first attempt is absorbed by a later retry. + + The shared ``releases.astral.sh`` origin serves every coverage-evidence + run org-wide, so a lone transient error (observed directly in production + as ``trusted uv archive download failed: HTTPError``) must not fail the + whole job when a subsequent attempt would have succeeded. + """ + payload = b"archive" + response = FakeHttpResponse(materializer.TRUSTED_UV_ARCHIVE_URL, payload) + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + if calls == 1: + raise OSError("transient") + return response + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) + sleeps: list[float] = [] + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == payload + assert calls == 2 + assert sleeps == [materializer.TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS] def test_verified_uv_binary_accepts_exact_archive( diff --git a/tests/test_trusted_uv_download_contract.py b/tests/test_trusted_uv_download_contract.py index 02f3c5961..a680c794d 100644 --- a/tests/test_trusted_uv_download_contract.py +++ b/tests/test_trusted_uv_download_contract.py @@ -24,11 +24,16 @@ def _module_tree() -> ast.Module: def _download_function() -> ast.FunctionDef: - """Return the trusted-uv downloader function from the parsed module.""" + """Return the trusted-uv single-attempt network-sink function from the module. + + ``_download_trusted_uv_archive`` is a bounded-retry wrapper around this + function; the literal-URL network sink itself lives in + ``_fetch_trusted_uv_archive_once`` so it runs unchanged on every attempt. + """ for node in _module_tree().body: - if isinstance(node, ast.FunctionDef) and node.name == "_download_trusted_uv_archive": + if isinstance(node, ast.FunctionDef) and node.name == "_fetch_trusted_uv_archive_once": return node - raise AssertionError("trusted uv downloader function is missing") + raise AssertionError("trusted uv single-attempt downloader function is missing") def _assigned_literal(name: str) -> object: From d14546f004caad08f4ed441d2e505604b9c6aa22 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 11:55:54 +0900 Subject: [PATCH 2/5] test(coverage-evidence): assert trusted-uv linear retry delays Record each backoff sleep on retry exhaustion and require delay then 2*delay. Keep Darwin installer tests on the linux x86_64 path. Cite RFC 9110 for transient-only retries. --- CHANGELOG.md | 2 +- .../trusted-uv-transient-download-retry.md | 27 +++++++++++++++++++ ...st_materialize_base_python_requirements.py | 15 ++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 docs/doctoring/trusted-uv-transient-download-retry.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 1202cae60..b84e9bc75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. +- Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. Exhaustion tests now assert the linear backoff sequence. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/docs/doctoring/trusted-uv-transient-download-retry.md b/docs/doctoring/trusted-uv-transient-download-retry.md new file mode 100644 index 000000000..265a7b828 --- /dev/null +++ b/docs/doctoring/trusted-uv-transient-download-retry.md @@ -0,0 +1,27 @@ +# Trusted-uv transient download retry + +## Incident and buyer impact + +Every organization pull request runs `coverage-evidence`, which downloads one +pinned `uv` archive from `releases.astral.sh`. A single transient `HTTPError` +on that shared origin failed the gate and produced a false-negative +OpenCode `REQUEST_CHANGES` on otherwise healthy heads +(`ContextualWisdomLab/naruon#1293`, `ContextualWisdomLab/naruon#1300`). + +## Decision + +Retry only `OSError` (including `HTTPError` / `URLError`) a bounded three +times with linear backoff. Trust-boundary violations (`RuntimeError` for +redirect, host/port, or oversized payload) fail on the first attempt and are +never retried. Each attempt still pins scheme, host, port, size, checksum, +and archive member. + +This follows the HTTP retry discipline for transient server/network failures +and does not retry client-side policy failures (Fielding et al., 2022, +§15.5). + +## References + +Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* +(RFC 9110). Internet Engineering Task Force. +https://doi.org/10.17487/RFC9110 diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 64948ce5d..cb163596f 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -30,6 +30,13 @@ def _created_tool_directory(path: Path) -> str: return str(path) +def _force_linux_x86_64_installer(monkeypatch: pytest.MonkeyPatch) -> None: + """Exercise the installer path that GitHub-hosted linux x86_64 runners use.""" + monkeypatch.setattr(materializer.sys, "platform", "linux") + monkeypatch.setattr(materializer.platform, "machine", lambda: "x86_64") + materializer._install_trusted_uv.cache_clear() + + def test_materializes_only_regular_hash_locks_from_exact_base(tmp_path: Path) -> None: """A PR-modified lock cannot enter the networked coverage image build context.""" repo = tmp_path / "repo" @@ -561,10 +568,13 @@ def _urlopen(*_a: object, **_k: object) -> None: raise OSError("offline") monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) - monkeypatch.setattr(materializer.time, "sleep", lambda _seconds: None) + sleeps: list[float] = [] + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) with pytest.raises(RuntimeError, match="download failed"): materializer._download_trusted_uv_archive() assert calls == materializer.TRUSTED_UV_DOWNLOAD_ATTEMPTS + delay = materializer.TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS + assert sleeps == [delay, delay * 2] def test_download_trusted_uv_archive_retries_transient_failure_then_succeeds( @@ -700,6 +710,7 @@ def test_install_trusted_uv_verifies_version_and_caches_path( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """The installer writes one executable, verifies its version, and caches it.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -746,6 +757,7 @@ def test_install_trusted_uv_rejects_version_process_failures( failure: OSError | subprocess.TimeoutExpired, ) -> None: """A missing or hung downloaded executable is removed and rejected.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / "uv" monkeypatch.setattr( materializer.tempfile, @@ -777,6 +789,7 @@ def test_install_trusted_uv_rejects_wrong_version_or_exit_status( completed: subprocess.CompletedProcess[bytes], ) -> None: """Unexpected version output or a nonzero status cannot satisfy the pin.""" + _force_linux_x86_64_installer(monkeypatch) tool_dir = tmp_path / f"uv-{completed.returncode}-{len(completed.stdout)}" monkeypatch.setattr( materializer.tempfile, From bbbe46d8b7263c61a30cc0b7205d6f9a9e97e871 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 15:54:04 +0900 Subject: [PATCH 3/5] test(coverage-evidence): pin HTTPError 503 as a retried OSError CWE-755: the production urllib HTTPError is an OSError subclass and must enter the bounded retry; RuntimeError trust-boundary failures stay unretried. --- ARCHITECTURE.md | 85 +++++++++++++++++++ CHANGELOG.md | 2 +- CLAUDE.md | 3 + .../trusted-uv-transient-download-retry.md | 7 +- ...st_materialize_base_python_requirements.py | 36 ++++++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 ARCHITECTURE.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 000000000..d9e826096 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,85 @@ +# Architecture — ContextualWisdomLab `.github` + +This repository is the organization control plane. It is not naruon and it +does not own product data. Sibling products remain standalone modules; this +repo publishes org profile assets, reusable required workflows, and the +review/merge schedulers those products consume. + +## System context + +```mermaid +flowchart LR + Buyer["Commercial buyer / reviewer"] + Agents["Agents on AGENTS.md"] + Project["GitHub Project #1"] + Hub["This repo: org .github"] + Products["Owned products
naruon · orchestrator · engines"] + Runner["Required workflows in each repo context"] + + Buyer --> Hub + Agents --> Project + Agents --> Hub + Project --> Hub + Hub --> Runner + Runner --> Products + Products -->|"standalone or as module"| Buyer +``` + +## Trusted-uv download retry gate + +```mermaid +flowchart TD + Fetch["Fetch pinned releases.astral.sh archive"] + Kind{"Exception class?"} + Retry{"Attempts remaining?"} + Success["Checksum and member verify"] + FailClosed["Fail closed: trust-boundary or exhausted"] + + Fetch --> Kind + Kind -->|"OSError including HTTPError"| Retry + Kind -->|"RuntimeError redirect or size"| FailClosed + Retry -->|"yes"| Fetch + Retry -->|"no"| FailClosed + Fetch -->|"bytes"| Success +``` + +CWE-755: a lone origin 503 is an `OSError` and must retry. A redirect or +oversized payload is a `RuntimeError` and must not. + +## Control-plane data flow + +```mermaid +sequenceDiagram + participant PR as Pull request + participant RW as Required workflows + participant OC as OpenCode reviewer + participant SV as sandboxed_verify / web E2E + participant MS as Merge scheduler + + PR->>RW: pull_request_target on trusted base + RW->>OC: bounded evidence + NVIDIA NIM / OpenCode + OC->>SV: PoC command in isolated copy + SV-->>OC: redacted stdout/stderr + command metadata + OC-->>PR: APPROVE or request changes + MS->>PR: merge only on current-head approval + green checks +``` + +## Trust boundaries + +- Required review workflows execute **base-branch** scripts. +- Reviewer agents stay `edit: deny`. +- Logs redact credential shapes. They do not mask operational PII. +- LLM and scheduled agents bind `NVIDIA_NIM_API_KEY`. They never use + `COPILOT_GITHUB_TOKEN`. +- Rust remains the psychometric arithmetic owner. + +## Quality gates + +`scripts/ci/` ships with 100% statement/branch coverage and 100% +docstrings. + +## Related durable documents + +- [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) +- [`docs/doctoring/trusted-uv-transient-download-retry.md`](docs/doctoring/trusted-uv-transient-download-retry.md) +- [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md) diff --git a/CHANGELOG.md b/CHANGELOG.md index b84e9bc75..ee07dadf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed -- Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. Exhaustion tests now assert the linear backoff sequence. +- Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. Exhaustion tests now assert the linear backoff sequence. The contract now raises the production `HTTPError` 503 type so CWE-755 cannot recast that OSError subclass as an unretried trust-boundary failure. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/CLAUDE.md b/CLAUDE.md index 1c7bdb2f6..f64a075c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,6 +63,9 @@ Details: `README.md` and `PR_GOVERNANCE_AUDIT.md`. dependency sets (see below). - `fuzz/` + `.clusterfuzzlite/` — Atheris fuzz targets for the review-output normalizer and the ClusterFuzzLite discovery marker. +- `ARCHITECTURE.md` — control-plane mermaid (system context, trusted-uv + retry gate, review sequence, trust boundaries). Reconstruct from the repo, + not private agent memory. - `docs/` — master context, Project protocol, `org-required-workflow-rollout.md`, `scorecard-governance.md`, SBOM inventory. - `.jules/` — recorded performance (`bolt.md`) and security (`sentinel.md`) learnings from past work diff --git a/docs/doctoring/trusted-uv-transient-download-retry.md b/docs/doctoring/trusted-uv-transient-download-retry.md index 265a7b828..a42dea269 100644 --- a/docs/doctoring/trusted-uv-transient-download-retry.md +++ b/docs/doctoring/trusted-uv-transient-download-retry.md @@ -18,10 +18,15 @@ and archive member. This follows the HTTP retry discipline for transient server/network failures and does not retry client-side policy failures (Fielding et al., 2022, -§15.5). +§15.5). CWE-755 forbids mishandling exceptional conditions (MITRE, 2026): +`HTTPError` is an `OSError` subclass, so a lone 503 must enter the bounded +retry; `RuntimeError` trust-boundary violations must not. ## References +MITRE. (2026). *CWE-755: Improper handling of exceptional conditions*. +https://cwe.mitre.org/data/definitions/755.html + Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index cb163596f..5e8c3c727 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -6,6 +6,8 @@ import subprocess import sys import tarfile +import urllib.error +from email.message import EmailMessage from pathlib import Path import pytest @@ -607,6 +609,40 @@ def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: assert sleeps == [materializer.TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS] +def test_download_trusted_uv_archive_retries_httperror_then_succeeds( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The production incident type, urllib HTTPError, is an OSError subclass. + + CWE-755: treating HTTPError as a trust-boundary RuntimeError would skip + the bounded retry and fail a healthy origin on one 503. + """ + payload = b"archive" + response = FakeHttpResponse(materializer.TRUSTED_UV_ARCHIVE_URL, payload) + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + if calls == 1: + raise urllib.error.HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + 503, + "Service Unavailable", + EmailMessage(), + io.BytesIO(b""), + ) + return response + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) + sleeps: list[float] = [] + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == payload + assert calls == 2 + assert sleeps == [materializer.TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS] + + def test_verified_uv_binary_accepts_exact_archive( monkeypatch: pytest.MonkeyPatch, ) -> None: From b983bb20091933c2f604edcaee9a6b078775edd6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Thu, 13 Aug 2026 18:31:22 +0900 Subject: [PATCH 4/5] fix(coverage): do not retry trusted-uv HTTP 4xx except 429 A missing or forbidden archive is a client policy failure. Retry only 5xx, 429, and non-HTTP OSError so a 404 cannot be probed three times. --- AGENTS.md | 2 + CHANGELOG.md | 1 + CLAUDE.md | 2 + .../trusted-uv-transient-download-retry.md | 11 +++-- .../materialize_base_python_requirements.py | 16 +++++++ ...st_materialize_base_python_requirements.py | 46 +++++++++++++++++++ 6 files changed, 73 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 688b33035..51e57a70b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,3 +2,5 @@ > **Agents: read the master context FIRST.** Before any work, read [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) (mission · naruon-as-platform + inter-component UML · cross-cutting disciplines · conventions · roadmap · current state), the live **GitHub Project #1** (work/roadmap source of truth), the full spec **ContextualWisdomLab/naruon#974**, and operate the Project per [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md). The repo/Project — not any private agent memory — is the source of truth. + +Trusted-uv download retries 5xx/429 only; other 4xx fail closed on the first attempt. See [`ARCHITECTURE.md`](ARCHITECTURE.md) and [`docs/doctoring/trusted-uv-transient-download-retry.md`](docs/doctoring/trusted-uv-transient-download-retry.md). diff --git a/CHANGELOG.md b/CHANGELOG.md index ee07dadf5..e9750580d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Trusted-uv download retry now fail-closes on the first HTTP 4xx except 429, so a missing archive or forbidden origin is not probed three times. 503/429 and non-HTTP `OSError` still use the bounded linear backoff. - Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. Exhaustion tests now assert the linear backoff sequence. The contract now raises the production `HTTPError` 503 type so CWE-755 cannot recast that OSError subclass as an unretried trust-boundary failure. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. diff --git a/CLAUDE.md b/CLAUDE.md index f64a075c8..15a77c049 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,3 +129,5 @@ repeatable compile command. cross-repo references as `owner/repo#num` or full URLs; durable knowledge in the repo/Project, not private memory; one roadmap phase at a time) are defined in `docs/CWL-MASTER-CONTEXT.md` §7 and apply here. + +Trusted-uv download retries 5xx/429 only. See `ARCHITECTURE.md`. diff --git a/docs/doctoring/trusted-uv-transient-download-retry.md b/docs/doctoring/trusted-uv-transient-download-retry.md index a42dea269..9c30e2f84 100644 --- a/docs/doctoring/trusted-uv-transient-download-retry.md +++ b/docs/doctoring/trusted-uv-transient-download-retry.md @@ -10,11 +10,12 @@ OpenCode `REQUEST_CHANGES` on otherwise healthy heads ## Decision -Retry only `OSError` (including `HTTPError` / `URLError`) a bounded three -times with linear backoff. Trust-boundary violations (`RuntimeError` for -redirect, host/port, or oversized payload) fail on the first attempt and are -never retried. Each attempt still pins scheme, host, port, size, checksum, -and archive member. +Retry only transient `OSError` a bounded three times with linear backoff: +HTTP 5xx, 429, and non-HTTP network errors. HTTP 4xx other than 429 fail +on the first attempt (RFC 9110 client-side policy). Trust-boundary +violations (`RuntimeError` for redirect, host/port, or oversized payload) +are never retried. Each attempt still pins scheme, host, port, size, +checksum, and archive member. This follows the HTTP retry discipline for transient server/network failures and does not retry client-side policy failures (Fielding et al., 2022, diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 9a6bfa257..81e1fdcce 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -20,6 +20,7 @@ import tarfile import tempfile import time +import urllib.error import urllib.parse import urllib.request from typing import Any @@ -212,6 +213,17 @@ def _fetch_trusted_uv_archive_once() -> bytes: return bytes(payload) +def _trusted_uv_download_is_transient(exc: BaseException) -> bool: + """Return True when one failed fetch may be retried. + + RFC 9110 treats 4xx as client-side policy failures except 429. 5xx + and non-HTTP ``OSError`` (timeout, reset) stay retryable. + """ + if isinstance(exc, urllib.error.HTTPError): + return exc.code >= 500 or exc.code == 429 + return isinstance(exc, OSError) + + def _download_trusted_uv_archive() -> bytes: """Download the fixed uv release archive through one HTTPS trust boundary. @@ -234,6 +246,10 @@ def _download_trusted_uv_archive() -> bytes: try: return _fetch_trusted_uv_archive_once() except OSError as exc: + if not _trusted_uv_download_is_transient(exc): + raise RuntimeError( + f"trusted uv archive download failed: {type(exc).__name__}" + ) from exc last_error = exc continue raise RuntimeError( diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 5e8c3c727..2265a1175 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -643,6 +643,52 @@ def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: assert sleeps == [materializer.TRUSTED_UV_DOWNLOAD_RETRY_DELAY_SECONDS] +def test_download_trusted_uv_archive_does_not_retry_http_404( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A client 404 is a policy failure and must fail closed on the first try.""" + + calls = 0 + + def _urlopen(*_a: object, **_k: object) -> FakeHttpResponse: + nonlocal calls + calls += 1 + raise urllib.error.HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + 404, + "Not Found", + EmailMessage(), + io.BytesIO(b""), + ) + + monkeypatch.setattr(materializer.urllib.request, "urlopen", _urlopen) + sleeps: list[float] = [] + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._trusted_uv_download_is_transient( + urllib.error.HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + 429, + "Too Many Requests", + EmailMessage(), + io.BytesIO(b""), + ) + ) + assert not materializer._trusted_uv_download_is_transient( + urllib.error.HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + 404, + "Not Found", + EmailMessage(), + io.BytesIO(b""), + ) + ) + with pytest.raises(RuntimeError, match="HTTPError"): + materializer._download_trusted_uv_archive() + assert calls == 1 + assert sleeps == [] + + def test_verified_uv_binary_accepts_exact_archive( monkeypatch: pytest.MonkeyPatch, ) -> None: From f175471a2f64adb297bd12039b3de6cdae918db1 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 14 Aug 2026 00:22:11 +0900 Subject: [PATCH 5/5] fix(coverage): accept only bounded relative requirement includes Materialize a base Python lock only when every package line is an exact SHA-256 pin or a two-token relative -r/--requirement include of a candidate lock path. A lone --require-hashes directive, ./dotted paths, and -r other-hashes.txt no longer enter the trusted build context. --- AGENTS.md | 2 +- CHANGELOG.md | 1 + .../trusted-uv-transient-download-retry.md | 3 + .../materialize_base_python_requirements.py | 82 +++++++++++++++---- ...st_materialize_base_python_requirements.py | 19 ++++- 5 files changed, 90 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 51e57a70b..e39b0d8d5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,4 +3,4 @@ > **Agents: read the master context FIRST.** Before any work, read [`docs/CWL-MASTER-CONTEXT.md`](docs/CWL-MASTER-CONTEXT.md) (mission · naruon-as-platform + inter-component UML · cross-cutting disciplines · conventions · roadmap · current state), the live **GitHub Project #1** (work/roadmap source of truth), the full spec **ContextualWisdomLab/naruon#974**, and operate the Project per [`docs/agent-github-project-protocol.md`](docs/agent-github-project-protocol.md). The repo/Project — not any private agent memory — is the source of truth. -Trusted-uv download retries 5xx/429 only; other 4xx fail closed on the first attempt. See [`ARCHITECTURE.md`](ARCHITECTURE.md) and [`docs/doctoring/trusted-uv-transient-download-retry.md`](docs/doctoring/trusted-uv-transient-download-retry.md). +Trusted-uv download retries 5xx/429 only; other 4xx fail closed on the first attempt. Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. See [`ARCHITECTURE.md`](ARCHITECTURE.md) and [`docs/doctoring/trusted-uv-transient-download-retry.md`](docs/doctoring/trusted-uv-transient-download-retry.md). diff --git a/CHANGELOG.md b/CHANGELOG.md index e9750580d..856a1b46c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Trusted-uv download retry now fail-closes on the first HTTP 4xx except 429, so a missing archive or forbidden origin is not probed three times. 503/429 and non-HTTP `OSError` still use the bounded linear backoff. - Retried the trusted-uv archive download a bounded number of times on transient network failures instead of failing the whole `coverage-evidence` job on a single `HTTPError`/`URLError` from the shared `releases.astral.sh` origin, which every pull request's review across the organization fetches; every attempt still runs the unchanged redirect-rejection, host/port pin, size bound, and checksum/member verification, so the fix adds resilience without weakening the trust boundary. Exhaustion tests now assert the linear backoff sequence. The contract now raises the production `HTTPError` 503 type so CWE-755 cannot recast that OSError subclass as an unretried trust-boundary failure. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. diff --git a/docs/doctoring/trusted-uv-transient-download-retry.md b/docs/doctoring/trusted-uv-transient-download-retry.md index 9c30e2f84..799f7d253 100644 --- a/docs/doctoring/trusted-uv-transient-download-retry.md +++ b/docs/doctoring/trusted-uv-transient-download-retry.md @@ -2,6 +2,9 @@ ## Incident and buyer impact +Materialize accepts only exact SHA-256 pins or a bounded relative `-r` +include; a lone `--require-hashes` line is not lock evidence. + Every organization pull request runs `coverage-evidence`, which downloads one pinned `uv` archive from `releases.astral.sh`. A single transient `HTTPError` on that shared origin failed the gate and produced a false-negative diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 81e1fdcce..282ca3167 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -91,6 +91,57 @@ def _is_candidate_lock_name(name: str) -> bool: ) +def _is_candidate_lock_path(path: pathlib.PurePosixPath) -> bool: + """Return whether one safe tracked path can name a pip requirements lock. + + In addition to conventional ``requirements*.txt`` names, repositories often + keep concrete environment closures as direct children such as + ``requirements/ci.txt`` or ``service/requirements/package.txt``. Only direct + ``.txt`` children of a directory named ``requirements`` gain this path-based + eligibility; content must still pass the independent complete hash-pin + validation before it reaches the trusted image build context. + """ + return _is_candidate_lock_name(path.name) or ( + path.suffix == ".txt" and path.parent.name == "requirements" + ) + + +def _is_bounded_requirement_include(line: str) -> bool: + """Return whether one requirements include names a bounded relative file. + + Includes are accepted only as a two-token ``-r``/``--requirement`` form + whose target is itself a candidate lock path written as a normalized + relative POSIX path. Absolute paths, ``.`` or ``..`` components, double + slashes, URLs, option-like targets, shell/Windows path separators, + fragments, queries, extra inline options or hashes, and includes of + non-lock files are rejected before a base-owned file can enter the + trusted build context. + The downstream installer still proves that the candidate is an independently + complete hash closure; this predicate grants syntax eligibility only. + """ + fields = line.split() + if len(fields) != 2 or fields[0] not in {"-r", "--requirement"}: + return False + target = fields[1] + if ( + target.startswith(("-", "~")) + or "\\" in target + or ":" in target + or "?" in target + or "#" in target + ): + return False + include_path = pathlib.PurePosixPath(target) + return ( + bool(include_path.parts) + and target == include_path.as_posix() + and not include_path.is_absolute() + and "." not in include_path.parts + and ".." not in include_path.parts + and _is_candidate_lock_path(include_path) + ) + + def _requirement_lines(content: bytes) -> list[str]: """Return logical requirement lines, joining backslash line-continuations. @@ -111,23 +162,26 @@ def _requirement_lines(content: bytes) -> list[str]: def _is_hash_pinned(content: bytes) -> bool: - """Return whether content carries hash pins and is safe to preflight. - - Discovery is content-based rather than name-based so hash-pinned locks in any - location (a service subdirectory, ``requirements-dev.txt``, - ``requirements-test.txt``) can be considered for offline coverage, while an - unpinned or PR-mutable requirements file is still excluded from the networked - build context. Hash syntax cannot prove that a file includes every transitive - dependency, so the trusted image installer separately preflights every - candidate as an independent ``--require-hashes`` closure. An empty file - carries no installable dependency and is not materialized. + """Return whether content carries only trusted pins or bounded includes. + + Discovery is content-based rather than name-based so exact hash-pinned locks + in service subdirectories and role-specific requirements files can be + considered for offline coverage. Candidate syntax is deliberately stricter + than a substring search: each package line must be an exact ``==`` pin with + one or more complete SHA-256 hashes, or a bounded relative requirements + include. A global ``--require-hashes`` directive is not trust evidence by + itself. The downstream installer separately preflights every candidate as an + independent ``pip --require-hashes`` closure, so syntax eligibility never + substitutes for dependency-closure proof. """ lines = _requirement_lines(content) - if not lines: + requirement_lines = [line for line in lines if line != "--require-hashes"] + if not requirement_lines: return False - return any(line == "--require-hashes" for line in lines) or all( - "--hash=" in line or line.startswith(("-r ", "--requirement ")) - for line in lines + return all( + _is_fully_hash_pinned_requirement(line) + or _is_bounded_requirement_include(line) + for line in requirement_lines ) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 2265a1175..78b29d5d7 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -159,9 +159,24 @@ def test_lock_name_candidates_are_pip_requirements_files() -> None: def test_hash_pin_detection_includes_pinned_and_excludes_unpinned_or_empty() -> None: """Only fully hash-pinned, non-empty lock content is materialized.""" assert not materializer._is_hash_pinned(b"# comment only\n\n") - assert materializer._is_hash_pinned(b"--require-hashes\ndemo==1\n") + assert not materializer._is_hash_pinned(b"--require-hashes\ndemo==1\n") assert materializer._is_hash_pinned(b"demo==1 --hash=sha256:" + b"a" * 64 + b"\n") - assert materializer._is_hash_pinned(b"-r other-hashes.txt\n") + assert materializer._is_hash_pinned(b"-r requirements-other.txt\n") + assert not materializer._is_hash_pinned(b"-r other-hashes.txt\n") + assert not materializer._is_hash_pinned(b"-r ./requirements-other.txt\n") + assert not materializer._is_hash_pinned(b"-r ../escape.txt\n") + assert materializer._is_bounded_requirement_include( + "--requirement requirements-other.txt" + ) + assert not materializer._is_bounded_requirement_include("-r .") + assert not materializer._is_bounded_requirement_include("-r -evil.txt") + assert not materializer._is_bounded_requirement_include("-r ~evil.txt") + assert not materializer._is_bounded_requirement_include("-r C:foo.txt") + assert not materializer._is_bounded_requirement_include("-r foo?bar.txt") + assert not materializer._is_bounded_requirement_include("-r foo#bar.txt") + assert not materializer._is_bounded_requirement_include(r"-r foo\\bar.txt") + assert not materializer._is_bounded_requirement_include("-r") + assert not materializer._is_bounded_requirement_include("-r /abs/requirements.txt") assert not materializer._is_hash_pinned(b"untrusted==1\n") # uv export / pip-compile multi-line continuation format (spec, then --hash= lines). assert materializer._is_hash_pinned(