diff --git a/.github/workflows/strix-changed-path-quality-ci.yml b/.github/workflows/strix-changed-path-quality-ci.yml index 74037c18c..75e9b7d8e 100644 --- a/.github/workflows/strix-changed-path-quality-ci.yml +++ b/.github/workflows/strix-changed-path-quality-ci.yml @@ -7,10 +7,12 @@ on: - ".github/workflows/strix-changed-path-quality-ci.yml" - "CHANGELOG.md" - "docs/doctoring/strix-legal-git-paths.md" + - "docs/doctoring/strix-quality-timeout-fixtures.md" - "scripts/ci/strix_quick_gate.sh" - "scripts/ci/test_strix_quick_gate.sh" - "tests/test_strix_changed_path_policy.py" - "tests/test_strix_workflow_dependency_hashes.py" + - "tests/test_strix_quality_timeout_fixture_budget.py" permissions: contents: read @@ -56,11 +58,14 @@ jobs: -r "${RUNNER_TEMP}/strix-quality-requirements.txt" - name: Verify exact-head path policy and syntax + env: + STRIX_TEST_PROCESS_TIMEOUT_SECONDS: "3" + STRIX_TEST_FAKE_SLEEP_SECONDS: "5" shell: bash --noprofile --norc -e -o pipefail {0} run: | test "$(git rev-parse HEAD)" = "${{ github.event.pull_request.head.sha || github.sha }}" python -m coverage run -m pytest tests -q bash scripts/ci/test_strix_quick_gate.sh - python -m compileall -q tests/test_strix_changed_path_policy.py tests/test_strix_workflow_dependency_hashes.py + python -m compileall -q tests/test_strix_changed_path_policy.py tests/test_strix_workflow_dependency_hashes.py tests/test_strix_quality_timeout_fixture_budget.py bash -n scripts/ci/strix_quick_gate.sh git diff --exit-code diff --git a/.github/workflows/trusted-uv-materializer-quality-ci.yml b/.github/workflows/trusted-uv-materializer-quality-ci.yml index 95642b55c..bd284199b 100644 --- a/.github/workflows/trusted-uv-materializer-quality-ci.yml +++ b/.github/workflows/trusted-uv-materializer-quality-ci.yml @@ -8,6 +8,7 @@ on: - "scripts/ci/materialize_base_python_requirements.py" - "tests/conftest.py" - "tests/test_materialize*.py" + - "tests/test_trusted_git_executable.py" - "tests/test_trusted_uv*.py" - "tests/test_uv*.py" - "tests/test_repository_branch_coverage_*.py" @@ -20,6 +21,7 @@ on: - "scripts/ci/materialize_base_python_requirements.py" - "tests/conftest.py" - "tests/test_materialize*.py" + - "tests/test_trusted_git_executable.py" - "tests/test_trusted_uv*.py" - "tests/test_uv*.py" - "tests/test_repository_branch_coverage_*.py" @@ -125,9 +127,13 @@ jobs: python -m coverage erase python -m coverage run -m pytest \ tests/test_materialize_base_python_requirements.py \ + tests/test_materialize_fifo_output_security.py \ + tests/test_materialize_output_directory_security.py \ tests/test_materialize_uv_export_hash_contract.py \ + tests/test_trusted_git_executable.py \ tests/test_trusted_uv_download_contract.py \ tests/test_trusted_uv_portability_and_streaming.py \ + tests/test_trusted_uv_retry_documentation.py \ tests/test_uv_export_isolation_contract.py \ tests/test_uv_redirect_and_coverage_contract.py \ tests/test_uv_redirect_boundary.py \ @@ -151,9 +157,13 @@ jobs: python -m compileall -q \ scripts/ci/materialize_base_python_requirements.py \ tests/test_materialize_base_python_requirements.py \ + tests/test_materialize_fifo_output_security.py \ + tests/test_materialize_output_directory_security.py \ tests/test_materialize_uv_export_hash_contract.py \ + tests/test_trusted_git_executable.py \ tests/test_trusted_uv_download_contract.py \ tests/test_trusted_uv_portability_and_streaming.py \ + tests/test_trusted_uv_retry_documentation.py \ tests/test_uv_export_isolation_contract.py \ tests/test_uv_redirect_and_coverage_contract.py \ tests/test_uv_redirect_boundary.py \ diff --git a/CHANGELOG.md b/CHANGELOG.md index 4215d4d04..cf3aea688 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- 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. - Bound both trusted-uv quality jobs to `github.event.pull_request.head.sha` and added a permanent two-checkout regression contract so exact-head compatibility, coverage, docstring, and compilation claims cannot silently measure GitHub's generated pull-request merge revision. - Made Strix treat only a single LiteLLM provider-error line containing NVIDIA NIM context and model-catalog 404 evidence as cross-model fallback evidence, rejecting cross-line signal assembly and provider-like target source literals; moved the public default to Nemotron 3 Super 120B and added a second NVIDIA hosted candidate before GitHub Models without neutralizing reported vulnerabilities. +- Pinned generated Python lock output to no-follow directory and file descriptors, rejected symbolic and multiply linked destinations before mutation, revalidated inode and single-link bindings after synchronized writes, and added deterministic regressions for output-path races, hard links introduced during writes, file swaps, and stalled writes. +- Opened existing generated-lock outputs non-blocking before regular-file validation so attacker-controlled FIFOs and other special entries cannot stall trusted materialization; `ENXIO` now fails closed and a permanent regression is included in the exact-head 100% coverage gate. +- Resolved Git only through the operating system default executable path and rejected missing or relative results before trusted base-lock materialization, preventing pull-request-controlled `PATH` selection. +- Restricted trusted uv retries to HTTP 408/425/429/500/502/503/504 and explicitly classified temporary DNS, timeout, connection, host, or network failures; every retry reuses the immutable request contract and discards failed-attempt bytes, while TLS, permanent DNS, malformed, and unclassified local errors fail after one attempt. diff --git a/docs/doctoring/strix-quality-timeout-fixtures.md b/docs/doctoring/strix-quality-timeout-fixtures.md new file mode 100644 index 000000000..a9588243b --- /dev/null +++ b/docs/doctoring/strix-quality-timeout-fixtures.md @@ -0,0 +1,51 @@ +# Strix quality timeout-fixture budget + +검토 기준일: **2026-08-07** + +## Incident + +`Strix Changed Path Quality CI`는 실제 Strix 모델 스캔이 아니라 중앙 정책과 `scripts/ci/test_strix_quick_gate.sh`의 결정적 회귀를 검증하는 품질 게이트입니다. 그러나 테스트 하네스의 timeout fixture가 기본적으로 실제 프로세스 제한 30초와 가짜 sleep 60초를 사용하면서 여러 timeout/fallback 경로를 순차 실행했습니다. + +PR #821 exact head `f92784f389317d512376a0725cbd78606b2e832c`의 품질 실행은 저장소 테스트 978개와 subtest 16개를 55.11초에 완료한 뒤 timeout fixture 구간을 수행하다가 job의 10분 제한에서 취소되었습니다. 동일 exact head의 rerun도 같은 단계에서 취소되었습니다. 이 결과는 소스 정책 실패가 아니라 결정적 테스트 fixture의 시간 스케일이 품질 job 예산과 맞지 않는다는 증거입니다. + +## Decision + +품질 workflow의 `Verify exact-head path policy and syntax` 단계에 테스트 전용 환경값만 전달합니다. + +- `STRIX_TEST_PROCESS_TIMEOUT_SECONDS=3` +- `STRIX_TEST_FAKE_SLEEP_SECONDS=5` + +`test_strix_quick_gate.sh`는 이미 두 값을 명시적 테스트 seam으로 제공하며, fake sleep이 process timeout보다 커야 한다고 fail closed 검증합니다. 따라서 timeout, cleanup, fallback의 순서와 결론은 그대로 유지하면서 wall-clock 대기만 축소합니다. + +다음 production scanner 설정은 이 변경에서 건드리지 않습니다. + +- `STRIX_PROCESS_TIMEOUT_SECONDS` +- `STRIX_TOTAL_TIMEOUT_SECONDS` +- `LLM_TIMEOUT` +- 실제 Strix workflow의 90분 process budget과 95분 total budget +- 모델, provider, credential, 권한, changed-path 정책 및 branch-protection 의미 + +테스트 전용 환경값은 해당 품질 step에만 존재해야 하며 production Strix 실행으로 전파되어서는 안 됩니다. + +## Verification contract + +`tests/test_strix_quality_timeout_fixture_budget.py`는 정확한 named step을 분리하여 다음을 고정합니다. + +1. 짧은 process/fake-sleep fixture 값이 모두 존재합니다. +2. 하네스 실행이 같은 step에서 유지됩니다. +3. production timeout 변수는 해당 step에서 override되지 않습니다. +4. workflow trigger가 이 회귀 파일 자체를 포함하여 이후 변경이 정확한 품질 gate를 다시 실행합니다. + +전체 `tests` suite, shell harness, Python compilation, Bash syntax 및 clean-worktree 검증은 계속 같은 exact-head quality step에서 수행합니다. 품질 gate의 성공은 실제 Strix 모델 security review, 독립 승인 또는 branch protection을 대체하지 않습니다. + +## Rollback + +3초/5초 fixture가 GitHub-hosted runner에서 재현 가능한 race margin을 제공하지 못한다는 결정적 실패가 관찰되면 테스트 전용 값만 가장 작은 재현 가능한 상한으로 올립니다. production scanner timeout을 낮추거나 품질 테스트를 삭제하여 문제를 숨기지 않습니다. 10분 job timeout 자체를 늘리는 것은 fixture 가속으로도 완료할 수 없다는 실행 증거가 있을 때 별도 검토합니다. + +## References (APA 7th) + +GitHub. (n.d.). *Workflow syntax for GitHub Actions*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax + +GitHub. (n.d.). *Contexts reference*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/actions/reference/workflows-and-actions/contexts + +GitHub. (n.d.). *Viewing job execution time*. GitHub Docs. Retrieved August 7, 2026, from https://docs.github.com/en/actions/how-tos/monitor-workflows/view-job-execution-time 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..c418889dc --- /dev/null +++ b/docs/doctoring/trusted-uv-transient-download-retry.md @@ -0,0 +1,97 @@ +# Trusted uv transient download retry boundary + +## Decision + +The central coverage materializer downloads one checksum-pinned uv archive from one literal Astral HTTPS URL. It performs at most **three total attempts**, separated by deterministic delays of one and two seconds, only for this closed availability set: + +- HTTP `408`, `425`, `429`, `500`, `502`, `503`, and `504`; +- temporary DNS resolution reported as `EAI_AGAIN`; +- `TimeoutError`; and +- connection aborted, refused, or reset, plus explicit host or network down, reset, unreachable, or timed-out operating-system errors. + +The fixed `GET` is safe and idempotent, so a bounded retry does not mutate remote or repository state. Each attempt repeats the same literal URL and exact timeout. The retry loop does not follow redirects, enable proxies, change the release URL, use repository-controlled headers, or accept an unverified payload. + +## Fail-closed exclusions + +The following conditions are never retried: + +- every HTTP response outside the exact closed set, including authorization, not-found, and unsupported-method failures; +- certificate verification or any other TLS failure; +- permanent DNS failure; +- a malformed or non-exception `URLError.reason`; +- local permission failures and every unclassified `OSError`; +- redirect attempts or a final origin or port outside the fixed Astral HTTPS origin; +- an oversized archive; +- SHA-256 mismatch; +- malformed archive members, incorrect executable size or type, unsupported runner architecture, or unexpected uv version; and +- offline export, exact-pin grammar, Git-tree, TOML, or workspace-boundary failures. + +A response body belongs to one attempt only. Partial bytes read before a transient failure are discarded before the next attempt. Retry exhaustion reports only a bounded HTTP status, transport errno, or exception class and the attempt count. It never includes exception text, URLs, response bodies, headers, credentials, or URL-derived user information. + +The base-commit reader resolves `git` with `shutil.which("git", path=os.defpath)` and accepts only an absolute result. The ambient process `PATH` cannot select the executable; missing or relative resolution fails before any repository command runs. + +## Descriptor-pinned output boundary + +The generated-lock output path is treated as an untrusted namespace rather than as a stable object. Every directory component is created or opened relative to an already-open parent descriptor with `O_DIRECTORY`, `O_NOFOLLOW`, and `O_CLOEXEC`. The materializer compares the path entry's device and inode to the pinned descriptor immediately after open and again before reporting success. Removing, replacing, or redirecting the output pathname therefore fails closed; subsequent writes never re-resolve that mutable pathname. + +Generated requirements and manifests are opened relative to the pinned output directory. A new file requires `O_CREAT | O_EXCL | O_NOFOLLOW`; a rerun opens an existing entry with `O_NONBLOCK | O_NOFOLLOW` before validating the descriptor as a singly linked regular file. This prevents an attacker-controlled FIFO with no reader from blocking `open()` before type validation; a non-blocking `ENXIO` is normalized to the same fail-closed regular-file rejection. Symbolic links, hard links, directories, FIFOs, and other special files are rejected before truncation. Each write is bounded by forward-progress checks and synchronized with `fsync`. After synchronization, both the published path and the pinned file descriptor must still identify the same singly linked regular inode; a hard link introduced during the write window therefore fails closed before success. The directory is then synchronized and revalidated. + +This contract intentionally uses the POSIX descriptor-relative interface represented by `openat()` and Python's `dir_fd` operations. It prevents the check-then-use gap reported against the earlier `Path.exists()`/`Path.is_symlink()` followed by `Path.mkdir()` sequence. The central GitHub runner is Linux; a platform that does not provide the required no-follow and non-blocking descriptor flags fails at import or execution rather than silently falling back to pathname-based writes. + +## Incident evidence + +Central OpenCode coverage run `31002427460` for `ContextualWisdomLab/newsdom-api#524` reached the exact trusted-uv materialization stage and failed with `trusted uv archive download failed: HTTPError`. The source PR changed only `AGENTS.md`; all repository-local checks were successful. A later workflow in the same operating window downloaded the pinned uv release successfully, supporting a bounded transient-retry response rather than weakening the immutable bootstrap or bypassing coverage. + +The same failure class later blocked exact-head OpenCode coverage for `ContextualWisdomLab/pg-llm-batch#53` in central workflow run `31022108085`. Repository-local CI, security, and SAST checks passed on that exact product head, while trusted uv archive materialization failed before PR-controlled tests ran. + +Exact-head Strix run `31076540331` for organization control-plane PR `ContextualWisdomLab/.github#790` identified a medium-severity time-of-check/time-of-use race between output-directory symlink inspection and directory creation. The finding was valid rather than stale or infrastructure-only. Test-first commit `a1dcc679c1767f7e806793d7c0225a1342a9a875` captured intermediate symlink, pathname removal and replacement, generated-file symlink and hard-link, post-open swap, zero-progress write, and root-output regressions before descriptor-pinned production remediation. + +A later exact-head independent review found a second valid race: a concurrent writer could add a hard link after the initial `st_nlink == 1` check while the descriptor remained bound to the same inode. RED commit `dc78b919e36011fa0f56e3ce9e334d3b1cb2261e` proved the existing implementation accepted that condition. The production fix revalidates regular-file type, device/inode identity, and single-link state after `fsync`, so the same race now fails closed. + +A further independent review identified a bounded-denial-of-service gap in the existing-file path: `O_NOFOLLOW | O_WRONLY` could block forever when an attacker pre-created `requirements-000.txt` as a FIFO with no reader, before the subsequent `fstat()` regular-file check. RED commit `83f5a051785c0b21df92bbf1d1e0a7b7912dff55` added a deterministic regression that refuses to call the real blocking open unless `O_NONBLOCK` is present. Production commit `cf5c29e5179cab4f982c0078aaa02bd1cd321a38` adds the non-blocking flag and converts `ENXIO` into the existing fail-closed special-file rejection without weakening symlink, inode, link-count, or unexpected-error handling. + +## Verification contract + +Permanent tests require: + +- every HTTP status in the exact closed set receives one bounded retry; +- representative permanent HTTP responses fail after one attempt and no sleep; +- temporary DNS, timeout, and connection-reset failures retry; +- certificate verification, permanent DNS, malformed transport reasons, and unclassified local errors fail after one attempt and no sleep; +- persistent transient failures stop after exactly three attempts and delays of one and two seconds; +- every attempt reuses the literal trusted URL and exact timeout; +- partial bytes from a failed response are absent from the next attempt; +- every output path component is opened without following symlinks and remains bound to the pinned descriptor; +- output-path removal or inode replacement fails closed after descriptor-relative writes; +- generated-file symlinks and multiply linked files are rejected before mutation; +- an existing FIFO without a reader is opened non-blocking and rejected without stalling the materializer; +- a hard link introduced after the initial file check but before final validation fails closed after the synchronized write; +- a singly linked regular generated file can be safely refreshed on a rerun; +- a post-open generated-file path swap and a zero-progress descriptor write fail closed; and +- the no-proxy opener, redirect rejection, final-origin validation, repeated bounded reads, maximum size, checksum, archive member, executable version, Python compatibility, offline export, full SHA-256 grammar, 100% statement and branch coverage, and production docstrings remain unchanged. + +A permanent documentation contract rejects broader legacy wording such as all `URLError` or `OSError` failures and generic `5xx` retries. + +## MSA and operational boundary + +This retry and output hardening belong to the organization-owned coverage control plane because every leaf repository consumes the same trusted bootstrap. Leaf repositories such as pg-llm-batch, NewsDOM, and naruon must not duplicate a downloader, pathname race workaround, or weakened review gate. If all three attempts fail or any output binding changes, the current-head review remains fail-closed and publishes bounded evidence; no approval or merge is synthesized. + +## Rollback + +Rollback of the transport slice removes the retry constants and loop while retaining every immutable-source, no-proxy, no-redirect, bounded-read, checksum, archive, executable-version, and offline-export control. Operators may also set the delay tuple to empty in a reviewed change to restore one attempt. Increasing attempts, delays, or the closed classifier requires a separate availability, security, and runner-budget review. + +The output-binding remediation must not be rolled back to pathname prechecks or blocking opens of untrusted existing entries. A safe rollback may stop materialization entirely or replace the implementation with an independently reviewed descriptor-relative or private-directory publication design that preserves no-follow opening, non-blocking rejection of special files, inode validation, regular-file validation, single-link validation before and after writes, and fail-closed behavior. + +## References + +Fielding, R. T., Nottingham, M., & Reschke, J. (2022). *HTTP semantics* (RFC 9110). RFC Editor. https://doi.org/10.17487/RFC9110 + +Nottingham, M., & Fielding, R. (2012). *Additional HTTP status codes* (RFC 6585). RFC Editor. https://doi.org/10.17487/RFC6585 + +Python Software Foundation. (2026). *os—Miscellaneous operating system interfaces*. Python 3.14 documentation. https://docs.python.org/3.14/library/os.html + +Python Software Foundation. (2026). *urllib.error—Exception classes raised by urllib.request*. Python 3.14 documentation. https://docs.python.org/3.14/library/urllib.error.html + +The Open Group. (2024). *open, openat—Open file relative to directory file descriptor*. In *The Open Group Base Specifications Issue 8, IEEE Std 1003.1-2024*. https://pubs.opengroup.org/onlinepubs/9799919799/functions/open.html + +Thomson, M., Nottingham, M., & Tarreau, W. (2018). *Using early data in HTTP* (RFC 8470). RFC Editor. https://doi.org/10.17487/RFC8470 diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 98cdad459..d52c85015 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -5,6 +5,7 @@ import argparse import atexit +import errno import fnmatch import functools import hashlib @@ -15,10 +16,15 @@ import platform import re import shutil +import socket +import ssl +import stat import subprocess import sys import tarfile import tempfile +import time +import urllib.error import urllib.parse import urllib.request from typing import Any @@ -47,9 +53,29 @@ ) TRUSTED_UV_ARCHIVE_MEMBER = "uv-x86_64-unknown-linux-gnu/uv" TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS = 120 +TRUSTED_UV_DOWNLOAD_RETRY_DELAYS_SECONDS = (1.0, 2.0) +TRUSTED_UV_RETRYABLE_HTTP_STATUS = frozenset( + {408, 425, 429, 500, 502, 503, 504} +) +TRUSTED_UV_TRANSIENT_ERRNO = frozenset( + { + errno.ECONNABORTED, + errno.ECONNREFUSED, + errno.ECONNRESET, + errno.EHOSTUNREACH, + errno.ENETDOWN, + errno.ENETRESET, + errno.ENETUNREACH, + errno.ETIMEDOUT, + } +) TRUSTED_UV_DOWNLOAD_MAX_BYTES = 64 * 1024 * 1024 TRUSTED_UV_BINARY_MAX_BYTES = 64 * 1024 * 1024 TRUSTED_UV_VERSION_TIMEOUT_SECONDS = 10 +SECURE_DIRECTORY_OPEN_FLAGS = ( + os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC +) +SECURE_FILE_OPEN_FLAGS = os.O_WRONLY | os.O_NONBLOCK | os.O_NOFOLLOW | os.O_CLOEXEC class _RejectTrustedUvRedirects(urllib.request.HTTPRedirectHandler): @@ -151,10 +177,19 @@ def _is_fully_hash_pinned_export(content: bytes) -> bool: return bool(lines) and all(_is_fully_hash_pinned_requirement(line) for line in lines) +@functools.cache +def _trusted_git_executable() -> str: + """Return Git resolved only from the operating system's default path.""" + resolved = shutil.which("git", path=os.defpath) + if resolved is None or not os.path.isabs(resolved): + raise RuntimeError("trusted Git executable could not be resolved absolutely") + return resolved + + def _git(repo_root: pathlib.Path, *args: str) -> bytes: """Run one read-only git command in the materialized repository.""" completed = subprocess.run( - ["git", "-C", str(repo_root), *args], + [_trusted_git_executable(), "-C", str(repo_root), *args], check=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, @@ -165,51 +200,102 @@ def _git(repo_root: pathlib.Path, *args: str) -> bytes: return completed.stdout +def _transport_failure_root(error: BaseException) -> BaseException: + """Return the bounded diagnostic root for one transport exception.""" + if ( + isinstance(error, urllib.error.URLError) + and isinstance(error.reason, BaseException) + ): + return error.reason + return error + + +def _transient_transport_failure_label( + error: BaseException, +) -> str | None: + """Return bounded evidence only for provably transient transport failures.""" + root = _transport_failure_root(error) + if isinstance(root, (ssl.SSLCertVerificationError, ssl.SSLError)): + return None + if isinstance(root, socket.gaierror): + return "temporary DNS" if root.errno == socket.EAI_AGAIN else None + if isinstance(root, TimeoutError): + return "timeout" + if isinstance(root, OSError) and root.errno in TRUSTED_UV_TRANSIENT_ERRNO: + return f"transport errno {root.errno}" + return None + + def _download_trusted_uv_archive() -> bytes: - """Download the fixed uv release archive through one HTTPS trust boundary.""" + """Download the fixed archive with bounded transient transport retries.""" _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: + attempt_limit = len(TRUSTED_UV_DOWNLOAD_RETRY_DELAYS_SECONDS) + 1 + for attempt in range(1, attempt_limit + 1): + 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) + + if len(payload) > TRUSTED_UV_DOWNLOAD_MAX_BYTES: + raise RuntimeError( + "trusted uv archive exceeded the bounded download size" + ) + return bytes(payload) + except urllib.error.HTTPError as exc: + if exc.code not in TRUSTED_UV_RETRYABLE_HTTP_STATUS: raise RuntimeError( - "trusted uv archive redirected outside the fixed " - "releases.astral.sh HTTPS origin" + f"trusted uv archive download failed: HTTP {exc.code}" ) from exc - if ( - (final_url.scheme, final_url.hostname) - != ("https", "releases.astral.sh") - or final_port not in (None, 443) - ): + failure_label = f"HTTP {exc.code}" + failure: BaseException = exc + except (urllib.error.URLError, OSError) as exc: + failure_label = _transient_transport_failure_label(exc) + if failure_label is None: + root = _transport_failure_root(exc) 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 + "trusted uv archive download failed: " + f"{type(root).__name__}" + ) from exc + failure = exc - if len(payload) > TRUSTED_UV_DOWNLOAD_MAX_BYTES: - raise RuntimeError("trusted uv archive exceeded the bounded download size") - return bytes(payload) + if attempt == attempt_limit: + raise RuntimeError( + "trusted uv archive download failed: " + f"{failure_label} after {attempt} attempts" + ) from failure + time.sleep(TRUSTED_UV_DOWNLOAD_RETRY_DELAYS_SECONDS[attempt - 1]) + + raise AssertionError("trusted uv retry loop must return or raise") # pragma: no cover def _verified_uv_binary(archive_payload: bytes) -> bytes: @@ -472,34 +558,175 @@ def base_hash_locks(repo_root: pathlib.Path, base_sha: str) -> list[tuple[str, b return sorted(locks, key=lambda item: item[0]) +def _validate_directory_binding( + parent_fd: int, + name: str, + directory_fd: int, +) -> None: + """Prove that a no-follow pathname still names the pinned directory inode.""" + + try: + path_metadata = os.stat(name, dir_fd=parent_fd, follow_symlinks=False) + except FileNotFoundError as exc: + raise ValueError( + "output directory changed during secure materialization" + ) from exc + descriptor_metadata = os.fstat(directory_fd) + if ( + not stat.S_ISDIR(path_metadata.st_mode) + or (path_metadata.st_dev, path_metadata.st_ino) + != (descriptor_metadata.st_dev, descriptor_metadata.st_ino) + ): + raise ValueError("output directory changed during secure materialization") + + +def _open_directory_component(parent_fd: int, name: str) -> int: + """Create or open one directory component without following symbolic links.""" + + try: + os.mkdir(name, mode=0o700, dir_fd=parent_fd) + except FileExistsError: + pass + try: + directory_fd = os.open( + name, + SECURE_DIRECTORY_OPEN_FLAGS, + dir_fd=parent_fd, + ) + except OSError as exc: + if exc.errno in {errno.ELOOP, errno.ENOTDIR}: + raise ValueError( + "output directory must not be a symlink; path must not contain symlinks" + ) from exc + raise + try: + _validate_directory_binding(parent_fd, name, directory_fd) + except Exception: + os.close(directory_fd) + raise + return directory_fd + + +def _open_pinned_output_directory( + output_dir: pathlib.Path, +) -> tuple[int, int, str]: + """Return parent and output descriptors pinned through a no-follow path walk.""" + + absolute_output = pathlib.Path(os.path.abspath(output_dir)) + if absolute_output.parent == absolute_output: + raise ValueError("output directory must not be the filesystem root") + + current_fd = os.open(os.path.sep, SECURE_DIRECTORY_OPEN_FLAGS) + try: + for component in absolute_output.parts[1:-1]: + next_fd = _open_directory_component(current_fd, component) + os.close(current_fd) + current_fd = next_fd + output_name = absolute_output.name + output_fd = _open_directory_component(current_fd, output_name) + return current_fd, output_fd, output_name + except Exception: + os.close(current_fd) + raise + + +def _validate_file_binding(directory_fd: int, name: str, file_fd: int) -> None: + """Prove that a generated name still references one singly linked regular file.""" + + try: + path_metadata = os.stat(name, dir_fd=directory_fd, follow_symlinks=False) + except FileNotFoundError as exc: + raise ValueError("output file changed during secure materialization") from exc + descriptor_metadata = os.fstat(file_fd) + if ( + not stat.S_ISREG(path_metadata.st_mode) + or not stat.S_ISREG(descriptor_metadata.st_mode) + or (path_metadata.st_dev, path_metadata.st_ino) + != (descriptor_metadata.st_dev, descriptor_metadata.st_ino) + ): + raise ValueError("output file changed during secure materialization") + if path_metadata.st_nlink != 1 or descriptor_metadata.st_nlink != 1: + raise ValueError("output files must be singly linked regular files") + + +def _write_pinned_output_file( + directory_fd: int, + name: str, + content: bytes, +) -> None: + """Write one generated file through a no-follow descriptor-relative binding.""" + + try: + file_fd = os.open( + name, + SECURE_FILE_OPEN_FLAGS | os.O_CREAT | os.O_EXCL, + 0o600, + dir_fd=directory_fd, + ) + except FileExistsError: + try: + file_fd = os.open( + name, + SECURE_FILE_OPEN_FLAGS, + dir_fd=directory_fd, + ) + except OSError as exc: + if exc.errno in {errno.ELOOP, errno.ENOTDIR}: + raise ValueError("output files must not be symlinks") from exc + if exc.errno == errno.ENXIO: + raise ValueError("output files must be singly linked regular files") from exc + raise + + try: + metadata = os.fstat(file_fd) + if not stat.S_ISREG(metadata.st_mode) or metadata.st_nlink != 1: + raise ValueError("output files must be singly linked regular files") + os.ftruncate(file_fd, 0) + remaining = memoryview(content) + while remaining: + written = os.write(file_fd, remaining) + if written <= 0: + raise OSError("output file write made no progress") + remaining = remaining[written:] + os.fsync(file_fd) + _validate_file_binding(directory_fd, name, file_fd) + finally: + os.close(file_fd) + + def materialize( repo_root: pathlib.Path, base_sha: str, output_dir: pathlib.Path, ) -> list[dict[str, str]]: - """Write base lock blobs under generated names safe for a Docker build context.""" - if output_dir.exists() and output_dir.is_symlink(): - raise ValueError("output directory must not be a symlink") - output_dir.mkdir(parents=True, exist_ok=True) - - manifest: list[dict[str, str]] = [] - for index, (source_path, content) in enumerate( - base_hash_locks(repo_root.resolve(), base_sha) - ): - generated_name = f"requirements-{index:03d}.txt" - destination = output_dir / generated_name - destination.write_bytes(content) - manifest.append({"file": generated_name, "source": source_path}) - - (output_dir / "manifest.json").write_text( - json.dumps(manifest, indent=2, sort_keys=True) + "\n", - encoding="utf-8", - ) - (output_dir / "manifest.txt").write_text( - "".join(f"{entry['file']}\n" for entry in manifest), - encoding="utf-8", - ) - return manifest + """Write trusted locks through descriptor-pinned, no-follow output bindings.""" + + parent_fd, output_fd, output_name = _open_pinned_output_directory(output_dir) + try: + manifest: list[dict[str, str]] = [] + for index, (source_path, content) in enumerate( + base_hash_locks(repo_root.resolve(), base_sha) + ): + generated_name = f"requirements-{index:03d}.txt" + _write_pinned_output_file(output_fd, generated_name, content) + manifest.append({"file": generated_name, "source": source_path}) + + _write_pinned_output_file( + output_fd, + "manifest.json", + (json.dumps(manifest, indent=2, sort_keys=True) + "\n").encode("utf-8"), + ) + _write_pinned_output_file( + output_fd, + "manifest.txt", + "".join(f"{entry['file']}\n" for entry in manifest).encode("utf-8"), + ) + os.fsync(output_fd) + _validate_directory_binding(parent_fd, output_name, output_fd) + return manifest + finally: + os.close(output_fd) + os.close(parent_fd) def main(argv: list[str] | None = None) -> int: diff --git a/tests/test_materialize_fifo_output_security.py b/tests/test_materialize_fifo_output_security.py new file mode 100644 index 000000000..e08698eaf --- /dev/null +++ b/tests/test_materialize_fifo_output_security.py @@ -0,0 +1,49 @@ +"""Regression coverage for non-blocking rejection of special output files.""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_python_requirements as materializer + + +def _one_lock() -> list[tuple[str, bytes]]: + """Return one deterministic trusted lock fixture.""" + + return [("requirements.lock", b"demo==1 --hash=sha256:" + b"a" * 64 + b"\n")] + + +def test_materializer_rejects_existing_fifo_without_blocking( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An existing FIFO is opened non-blocking and rejected before trusted writes.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + fifo_path = output_directory / "requirements-000.txt" + os.mkfifo(fifo_path) + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + + real_open = materializer.os.open + existing_open_flags: list[int] = [] + + def require_nonblocking_existing( + path: object, flags: int, *args: object, **kwargs: object + ) -> int: + if path == "requirements-000.txt" and not flags & os.O_CREAT: + existing_open_flags.append(flags) + if not flags & os.O_NONBLOCK: + raise AssertionError("existing output must be opened non-blocking") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(materializer.os, "open", require_nonblocking_existing) + + with pytest.raises(ValueError, match="singly linked regular files"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert len(existing_open_flags) == 1 + assert existing_open_flags[0] & os.O_NONBLOCK + assert fifo_path.exists() diff --git a/tests/test_materialize_output_directory_security.py b/tests/test_materialize_output_directory_security.py new file mode 100644 index 000000000..8b1c0db1d --- /dev/null +++ b/tests/test_materialize_output_directory_security.py @@ -0,0 +1,357 @@ +"""Security regressions for descriptor-pinned materializer output writes.""" + +from __future__ import annotations + +import errno +import os +from pathlib import Path + +import pytest + +from scripts.ci import materialize_base_python_requirements as materializer + + +def _one_lock() -> list[tuple[str, bytes]]: + """Return one deterministic trusted lock fixture.""" + + return [("requirements.lock", b"demo==1 --hash=sha256:" + b"a" * 64 + b"\n")] + + +def test_materializer_rejects_symlinked_output_parent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """No intermediate symlink may redirect descriptor-relative output creation.""" + + target_directory = tmp_path / "target_directory" + target_directory.mkdir() + linked_parent = tmp_path / "linked_parent" + linked_parent.symlink_to(target_directory, target_is_directory=True) + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: []) + + with pytest.raises(ValueError, match="must not contain symlinks"): + materializer.materialize( + tmp_path, + "a" * 40, + linked_parent / "generated_locks", + ) + + assert list(target_directory.iterdir()) == [] + + +def test_materializer_fails_closed_when_output_binding_disappears( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Removing the published path cannot turn pinned writes into success evidence.""" + + output_directory = tmp_path / "generated_locks" + moved_directory = tmp_path / "moved_locks" + + def move_output_before_return(*_args: object) -> list[tuple[str, bytes]]: + output_directory.rename(moved_directory) + return _one_lock() + + monkeypatch.setattr(materializer, "base_hash_locks", move_output_before_return) + + with pytest.raises(ValueError, match="changed during secure materialization"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert (moved_directory / "requirements-000.txt").read_bytes() == _one_lock()[0][1] + assert not output_directory.exists() + + +def test_materializer_fails_closed_when_output_binding_is_replaced( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Replacing the pathname with another directory cannot receive trusted writes.""" + + output_directory = tmp_path / "generated_locks" + pinned_directory = tmp_path / "pinned_locks" + replacement_directory = tmp_path / "replacement_locks" + + def replace_output_before_return(*_args: object) -> list[tuple[str, bytes]]: + output_directory.rename(pinned_directory) + replacement_directory.mkdir() + replacement_directory.rename(output_directory) + return _one_lock() + + monkeypatch.setattr(materializer, "base_hash_locks", replace_output_before_return) + + with pytest.raises(ValueError, match="changed during secure materialization"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert (pinned_directory / "requirements-000.txt").read_bytes() == _one_lock()[0][1] + assert list(output_directory.iterdir()) == [] + + +def test_materializer_rejects_symlinked_destination_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An existing generated-name symlink cannot redirect a trusted lock write.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + outside_file = tmp_path / "outside_file" + outside_file.write_bytes(b"unchanged") + (output_directory / "requirements-000.txt").symlink_to(outside_file) + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + + with pytest.raises(ValueError, match="must not be symlinks"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert outside_file.read_bytes() == b"unchanged" + + +def test_materializer_rejects_multiply_linked_destination_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A hard-linked generated name is rejected before truncation or mutation.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + outside_file = tmp_path / "outside_file" + outside_file.write_bytes(b"unchanged") + os.link(outside_file, output_directory / "requirements-000.txt") + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + + with pytest.raises(ValueError, match="singly linked regular files"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert outside_file.read_bytes() == b"unchanged" + + +def test_materializer_detects_hard_link_added_during_pinned_write( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A hard link added after the initial check must fail before write success.""" + + output_directory = tmp_path / "generated_locks" + outside_link = tmp_path / "captured_output" + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + real_fsync = materializer.os.fsync + linked = False + + def link_after_file_sync(file_descriptor: int) -> None: + nonlocal linked + real_fsync(file_descriptor) + destination = output_directory / "requirements-000.txt" + if linked or not destination.exists(): + return + descriptor_metadata = os.fstat(file_descriptor) + path_metadata = os.stat(destination, follow_symlinks=False) + if (descriptor_metadata.st_dev, descriptor_metadata.st_ino) != ( + path_metadata.st_dev, + path_metadata.st_ino, + ): + return + os.link(destination, outside_link) + linked = True + + monkeypatch.setattr(materializer.os, "fsync", link_after_file_sync) + + with pytest.raises(ValueError, match="singly linked regular files"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert linked is True + assert outside_link.read_bytes() == _one_lock()[0][1] + + +def test_materializer_safely_replaces_single_link_regular_output( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A rerun may truncate only the pinned, singly linked regular destination.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + destination = output_directory / "requirements-000.txt" + destination.write_bytes(b"stale") + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + + manifest = materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert manifest == [ + {"file": "requirements-000.txt", "source": "requirements.lock"} + ] + assert destination.read_bytes() == _one_lock()[0][1] + + +def test_materializer_detects_destination_swap_after_pinned_write( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A generated pathname swapped after open cannot become accepted evidence.""" + + output_directory = tmp_path / "generated_locks" + outside_file = tmp_path / "outside_file" + outside_file.write_bytes(b"unchanged") + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + real_fsync = materializer.os.fsync + swapped = False + + def swap_after_file_sync(file_descriptor: int) -> None: + nonlocal swapped + real_fsync(file_descriptor) + if swapped or not (output_directory / "requirements-000.txt").exists(): + return + swapped = True + (output_directory / "requirements-000.txt").unlink() + (output_directory / "requirements-000.txt").symlink_to(outside_file) + + monkeypatch.setattr(materializer.os, "fsync", swap_after_file_sync) + + with pytest.raises(ValueError, match="output file changed"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert outside_file.read_bytes() == b"unchanged" + + +def test_materializer_detects_destination_removal_after_pinned_write( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Removing a generated pathname after open is detected before success.""" + + output_directory = tmp_path / "generated_locks" + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + real_fsync = materializer.os.fsync + removed = False + + def remove_after_file_sync(file_descriptor: int) -> None: + nonlocal removed + real_fsync(file_descriptor) + destination = output_directory / "requirements-000.txt" + if removed or not destination.exists(): + return + removed = True + destination.unlink() + + monkeypatch.setattr(materializer.os, "fsync", remove_after_file_sync) + + with pytest.raises(ValueError, match="output file changed"): + materializer.materialize(tmp_path, "a" * 40, output_directory) + + assert not (output_directory / "requirements-000.txt").exists() + + +def test_materializer_fails_when_descriptor_write_makes_no_progress( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A zero-length descriptor write is an error rather than a truncated success.""" + + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + monkeypatch.setattr(materializer.os, "write", lambda *_args: 0) + + with pytest.raises(OSError, match="made no progress"): + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + + +def test_materializer_rejects_filesystem_root_output(tmp_path: Path) -> None: + """The filesystem root is never a valid generated-lock output directory.""" + + with pytest.raises(ValueError, match="must not be the filesystem root"): + materializer.materialize(tmp_path, "a" * 40, Path("/")) + + +def test_materializer_normalizes_directory_open_failures( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Platform no-follow failures remain bounded and operator-readable.""" + + real_open = materializer.os.open + + def fail_output_open(path: object, flags: int, *args: object, **kwargs: object) -> int: + if path == "generated_locks": + raise OSError(errno.ENOTDIR, "synthetic") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(materializer.os, "open", fail_output_open) + + with pytest.raises(ValueError, match="must not contain symlinks"): + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + + +def test_materializer_propagates_unclassified_directory_open_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Unexpected directory open failures are not mislabeled as symlink attacks.""" + + real_open = materializer.os.open + + def deny_output_open(path: object, flags: int, *args: object, **kwargs: object) -> int: + if path == "generated_locks": + raise PermissionError(errno.EACCES, "synthetic") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(materializer.os, "open", deny_output_open) + + with pytest.raises(PermissionError, match="synthetic"): + materializer.materialize( + tmp_path, + "a" * 40, + tmp_path / "generated_locks", + ) + + +def test_directory_component_closes_descriptor_after_binding_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An opened child descriptor is closed when inode validation fails.""" + + parent_fd = os.open(tmp_path, materializer.SECURE_DIRECTORY_OPEN_FLAGS) + opened_descriptors: list[int] = [] + real_open = materializer.os.open + + def capture_child_open( + path: object, flags: int, *args: object, **kwargs: object + ) -> int: + descriptor = real_open(path, flags, *args, **kwargs) + if path == "generated_locks": + opened_descriptors.append(descriptor) + return descriptor + + monkeypatch.setattr(materializer.os, "open", capture_child_open) + monkeypatch.setattr( + materializer, + "_validate_directory_binding", + lambda *_args: (_ for _ in ()).throw(RuntimeError("binding failed")), + ) + + try: + with pytest.raises(RuntimeError, match="binding failed"): + materializer._open_directory_component(parent_fd, "generated_locks") + finally: + os.close(parent_fd) + + assert len(opened_descriptors) == 1 + with pytest.raises(OSError) as raised: + os.fstat(opened_descriptors[0]) + assert raised.value.errno == errno.EBADF + + +def test_materializer_propagates_unclassified_existing_file_open_failure( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Unexpected existing-file failures remain their original fail-closed class.""" + + output_directory = tmp_path / "generated_locks" + output_directory.mkdir() + (output_directory / "requirements-000.txt").write_bytes(b"stale") + monkeypatch.setattr(materializer, "base_hash_locks", lambda *_args: _one_lock()) + real_open = materializer.os.open + + def deny_existing_file( + path: object, flags: int, *args: object, **kwargs: object + ) -> int: + if path == "requirements-000.txt" and not flags & os.O_CREAT: + raise PermissionError(errno.EACCES, "synthetic") + return real_open(path, flags, *args, **kwargs) + + monkeypatch.setattr(materializer.os, "open", deny_existing_file) + + with pytest.raises(PermissionError, match="synthetic"): + materializer.materialize(tmp_path, "a" * 40, output_directory) diff --git a/tests/test_strix_quality_timeout_fixture_budget.py b/tests/test_strix_quality_timeout_fixture_budget.py new file mode 100644 index 000000000..78fcc8a7a --- /dev/null +++ b/tests/test_strix_quality_timeout_fixture_budget.py @@ -0,0 +1,45 @@ +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +WORKFLOW_PATH = REPO_ROOT / ".github" / "workflows" / "strix-changed-path-quality-ci.yml" + + +def _named_step(workflow: str, name: str) -> str: + """Return one exact named workflow step without loading workflow YAML tags.""" + marker = f" - name: {name}\n" + start = workflow.index(marker) + try: + end = workflow.index("\n - name:", start + len(marker)) + except ValueError: + end = len(workflow) + return workflow[start:end] + + +def test_strix_quality_uses_short_fake_process_timeouts() -> None: + """Keep deterministic timeout fixtures well inside the quality-job budget.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + step = _named_step(workflow, "Verify exact-head path policy and syntax") + + assert 'STRIX_TEST_PROCESS_TIMEOUT_SECONDS: "3"' in step + assert 'STRIX_TEST_FAKE_SLEEP_SECONDS: "5"' in step + assert "bash scripts/ci/test_strix_quick_gate.sh" in step + + +def test_strix_quality_trigger_includes_fixture_contract_paths() -> None: + """Keep fixture behavior and doctoring changes inside the quality trigger.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + trigger = workflow[: workflow.index("\njobs:")] + + assert "docs/doctoring/strix-quality-timeout-fixtures.md" in trigger + assert "tests/test_strix_quality_timeout_fixture_budget.py" in trigger + + +def test_strix_quality_keeps_real_scanner_budgets_out_of_fixture_overrides() -> None: + """Fixture acceleration must not weaken production Strix scanner timeouts.""" + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + step = _named_step(workflow, "Verify exact-head path policy and syntax") + + assert "STRIX_PROCESS_TIMEOUT_SECONDS:" not in step + assert "STRIX_TOTAL_TIMEOUT_SECONDS:" not in step + assert "LLM_TIMEOUT:" not in step diff --git a/tests/test_trusted_git_executable.py b/tests/test_trusted_git_executable.py new file mode 100644 index 000000000..49419f9d1 --- /dev/null +++ b/tests/test_trusted_git_executable.py @@ -0,0 +1,92 @@ +"""Security regressions for the trusted Git executable boundary.""" + +from __future__ import annotations + +import os +import subprocess +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Iterator + +import pytest + +from scripts.ci import materialize_base_python_requirements as materializer + + +@dataclass(frozen=True) +class _CompletedGitCommand: + """Provide the bounded subprocess result consumed by the materializer.""" + + returncode: int = 0 + stdout: bytes = b"trusted-output" + stderr: bytes = b"" + + +@pytest.fixture(autouse=True) +def _clear_trusted_git_cache() -> Iterator[None]: + """Isolate cached Git resolution before and after every regression test.""" + + materializer._trusted_git_executable.cache_clear() + yield + materializer._trusted_git_executable.cache_clear() + + +def test_git_ignores_process_path_and_uses_absolute_default_path_executable( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """A pull-request-controlled PATH entry cannot select the Git executable.""" + + malicious_directory = tmp_path / "malicious-bin" + malicious_directory.mkdir() + monkeypatch.setenv("PATH", str(malicious_directory)) + + which_calls: list[tuple[str, str | None]] = [] + subprocess_calls: list[tuple[list[str], dict[str, Any]]] = [] + + def fake_which(command: str, *, path: str | None = None) -> str: + which_calls.append((command, path)) + return "/usr/bin/git" + + def fake_run( + command: list[str], + **kwargs: Any, + ) -> subprocess.CompletedProcess[bytes]: + subprocess_calls.append((command, kwargs)) + return _CompletedGitCommand() # type: ignore[return-value] + + monkeypatch.setattr(materializer.shutil, "which", fake_which) + monkeypatch.setattr(materializer.subprocess, "run", fake_run) + + assert materializer._git(tmp_path, "status", "--porcelain") == b"trusted-output" + assert which_calls == [("git", os.defpath)] + assert subprocess_calls[0][0] == [ + "/usr/bin/git", + "-C", + str(tmp_path), + "status", + "--porcelain", + ] + + +@pytest.mark.parametrize("resolved_git", [None, "git"]) +def test_git_fails_closed_when_default_path_has_no_absolute_executable( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + resolved_git: str | None, +) -> None: + """Missing or relative Git resolution cannot fall back to the process PATH.""" + + monkeypatch.setattr( + materializer.shutil, + "which", + lambda _command, *, path=None: resolved_git, + ) + + def unexpected_run(*_args: object, **_kwargs: object) -> None: + raise AssertionError("an untrusted Git command must never execute") + + monkeypatch.setattr(materializer.subprocess, "run", unexpected_run) + + with pytest.raises(RuntimeError, match="trusted Git executable"): + materializer._git(tmp_path, "status", "--porcelain") diff --git a/tests/test_trusted_uv_materializer_quality_workflow_contract.py b/tests/test_trusted_uv_materializer_quality_workflow_contract.py index 23a849bd8..1a0606dd6 100644 --- a/tests/test_trusted_uv_materializer_quality_workflow_contract.py +++ b/tests/test_trusted_uv_materializer_quality_workflow_contract.py @@ -22,6 +22,7 @@ def test_quality_workflow_runs_for_every_materializer_surface() -> None: '"scripts/ci/materialize_base_python_requirements.py"', '"tests/conftest.py"', '"tests/test_materialize*.py"', + '"tests/test_trusted_git_executable.py"', '"tests/test_trusted_uv*.py"', '"tests/test_uv*.py"', '"tests/test_repository_branch_coverage_*.py"', @@ -83,9 +84,13 @@ def test_full_quality_gate_proves_tests_coverage_docstrings_and_compilation() -> required_tests = ( "tests/test_materialize_base_python_requirements.py", + "tests/test_materialize_fifo_output_security.py", + "tests/test_materialize_output_directory_security.py", "tests/test_materialize_uv_export_hash_contract.py", "tests/test_trusted_uv_download_contract.py", + "tests/test_trusted_git_executable.py", "tests/test_trusted_uv_portability_and_streaming.py", + "tests/test_trusted_uv_retry_documentation.py", "tests/test_uv_export_isolation_contract.py", "tests/test_uv_redirect_and_coverage_contract.py", "tests/test_uv_redirect_boundary.py", diff --git a/tests/test_trusted_uv_portability_and_streaming.py b/tests/test_trusted_uv_portability_and_streaming.py index 34d8356c1..ba978033a 100644 --- a/tests/test_trusted_uv_portability_and_streaming.py +++ b/tests/test_trusted_uv_portability_and_streaming.py @@ -2,7 +2,12 @@ from __future__ import annotations +import errno +import io import platform +import socket +import ssl +import urllib.error from pathlib import Path import pytest @@ -13,8 +18,8 @@ class _ChunkedResponse: """Return deterministic short reads from one trusted final URL.""" - def __init__(self, chunks: list[bytes]) -> None: - """Store response chunks in the order an HTTP stream would expose them.""" + def __init__(self, chunks: list[bytes | BaseException]) -> None: + """Store response outcomes in the order an HTTP stream exposes them.""" self._chunks = iter(chunks) def __enter__(self) -> "_ChunkedResponse": @@ -30,8 +35,41 @@ def geturl() -> str: return materializer.TRUSTED_UV_ARCHIVE_URL def read(self, _size: int) -> bytes: - """Return one short chunk, followed by EOF when chunks are exhausted.""" - return next(self._chunks, b"") + """Return one short chunk, raise a scripted failure, or return EOF.""" + outcome = next(self._chunks, b"") + if isinstance(outcome, BaseException): + raise outcome + return outcome + + +def _http_error(status: int) -> urllib.error.HTTPError: + """Return one file-like HTTP failure for the fixed trusted archive URL.""" + + return urllib.error.HTTPError( + materializer.TRUSTED_UV_ARCHIVE_URL, + status, + "synthetic failure", + None, + io.BytesIO(b""), + ) + + +def _scripted_urlopen( + outcomes: list[object], + calls: list[tuple[str, int]], +): + """Return a fake urlopen that records the immutable request contract.""" + + remaining = iter(outcomes) + + def fake_urlopen(url: str, *, timeout: int) -> object: + calls.append((url, timeout)) + outcome = next(remaining) + if isinstance(outcome, BaseException): + raise outcome + return outcome + + return fake_urlopen def test_trusted_uv_download_collects_short_reads( @@ -64,6 +102,282 @@ def test_trusted_uv_download_rejects_oversize_across_short_reads( materializer._download_trusted_uv_archive() +@pytest.mark.parametrize("status", [408, 425, 429, 500, 502, 503, 504]) +def test_trusted_uv_download_retries_only_closed_http_status_set( + monkeypatch: pytest.MonkeyPatch, + status: int, +) -> None: + """Every explicitly transient HTTP status receives one bounded retry.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen( + [_http_error(status), _ChunkedResponse([b"archive", b""])], + calls, + ), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"archive" + assert calls == [ + ( + materializer.TRUSTED_UV_ARCHIVE_URL, + materializer.TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ), + ( + materializer.TRUSTED_UV_ARCHIVE_URL, + materializer.TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ), + ] + assert sleeps == [1.0] + + +@pytest.mark.parametrize("status", [400, 401, 403, 404, 405, 410, 422]) +def test_trusted_uv_download_does_not_retry_permanent_http_failure( + monkeypatch: pytest.MonkeyPatch, + status: int, +) -> None: + """Permanent source and authorization responses fail immediately.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([_http_error(status)], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=rf"HTTP {status}$"): + materializer._download_trusted_uv_archive() + + assert len(calls) == 1 + assert sleeps == [] + + +def test_trusted_uv_download_retries_temporary_dns_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Only the DNS resolver's temporary failure signal is retried.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + failure = urllib.error.URLError( + socket.gaierror(socket.EAI_AGAIN, "temporary DNS failure") + ) + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen( + [failure, _ChunkedResponse([b"archive", b""])], calls + ), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"archive" + assert len(calls) == 2 + assert sleeps == [1.0] + + +def test_trusted_uv_download_retries_timeout_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A real timeout receives one bounded retry with the exact request.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen( + [TimeoutError(errno.ETIMEDOUT, "timed out"), _ChunkedResponse([b"ok", b""])], + calls, + ), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"ok" + assert calls == [ + ( + materializer.TRUSTED_UV_ARCHIVE_URL, + materializer.TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ), + ( + materializer.TRUSTED_UV_ARCHIVE_URL, + materializer.TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ), + ] + assert sleeps == [1.0] + + +def test_trusted_uv_download_retries_connection_reset( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A connection reset receives one bounded retry with the same request.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen( + [ConnectionResetError(errno.ECONNRESET, "reset"), _ChunkedResponse([b"ok", b""])], + calls, + ), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"ok" + assert len(calls) == 2 + assert sleeps == [1.0] + + +def test_trusted_uv_download_does_not_retry_tls_certificate_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Certificate verification is an integrity failure, never availability noise.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + failure = urllib.error.URLError( + ssl.SSLCertVerificationError(1, "certificate verify failed") + ) + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([failure], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=r"SSLCertVerificationError$"): + materializer._download_trusted_uv_archive() + + assert len(calls) == 1 + assert sleeps == [] + + +def test_trusted_uv_download_does_not_retry_non_temporary_dns_failure( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unknown host is a permanent source failure rather than transient DNS.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + failure = urllib.error.URLError( + socket.gaierror(socket.EAI_NONAME, "name not known") + ) + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([failure], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=r"gaierror$"): + materializer._download_trusted_uv_archive() + + assert len(calls) == 1 + assert sleeps == [] + + +def test_trusted_uv_download_does_not_retry_malformed_urlerror_reason( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A non-exception URL reason fails once without leaking arbitrary text.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([urllib.error.URLError("malformed")], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises( + RuntimeError, + match=r"trusted uv archive download failed: URLError$", + ) as raised: + materializer._download_trusted_uv_archive() + + assert "malformed" not in str(raised.value) + assert calls == [ + ( + materializer.TRUSTED_UV_ARCHIVE_URL, + materializer.TRUSTED_UV_DOWNLOAD_TIMEOUT_SECONDS, + ) + ] + assert sleeps == [] + + +def test_trusted_uv_download_does_not_retry_unclassified_os_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Local or malformed OS failures cannot be promoted to network availability.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([OSError(errno.EINVAL, "invalid local state")], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=r"OSError$"): + materializer._download_trusted_uv_archive() + + assert len(calls) == 1 + assert sleeps == [] + + +def test_trusted_uv_download_exhausts_bounded_transient_retries( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Persistent transient failures stop after three total network attempts.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([_http_error(503), _http_error(503), _http_error(503)], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + with pytest.raises(RuntimeError, match=r"HTTP 503 after 3 attempts"): + materializer._download_trusted_uv_archive() + + assert len(calls) == 3 + assert sleeps == [1.0, 2.0] + + +def test_trusted_uv_download_discards_partial_bytes_before_retry( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Bytes read from a failed attempt never contaminate the next response.""" + + calls: list[tuple[str, int]] = [] + sleeps: list[float] = [] + first = _ChunkedResponse( + [b"partial-", ConnectionResetError(errno.ECONNRESET, "reset")] + ) + second = _ChunkedResponse([b"fresh", b""]) + monkeypatch.setattr( + materializer.urllib.request, + "urlopen", + _scripted_urlopen([first, second], calls), + ) + monkeypatch.setattr(materializer.time, "sleep", sleeps.append) + + assert materializer._download_trusted_uv_archive() == b"fresh" + assert len(calls) == 2 + assert sleeps == [1.0] + + @pytest.mark.parametrize( ("runner_platform", "runner_machine"), [("darwin", "x86_64"), ("linux", "aarch64")], diff --git a/tests/test_trusted_uv_retry_documentation.py b/tests/test_trusted_uv_retry_documentation.py new file mode 100644 index 000000000..731f9f737 --- /dev/null +++ b/tests/test_trusted_uv_retry_documentation.py @@ -0,0 +1,18 @@ +"""Documentation contracts for the closed trusted uv retry boundary.""" + +from pathlib import Path + + +def test_trusted_uv_retry_documentation_matches_closed_policy() -> None: + """Operator docs must not broaden the exact production retry classifier.""" + repository_root = Path(__file__).resolve().parents[1] + doctoring = ( + repository_root / "docs/doctoring/trusted-uv-transient-download-retry.md" + ).read_text(encoding="utf-8") + changelog = (repository_root / "CHANGELOG.md").read_text(encoding="utf-8") + normalized_doctoring = doctoring.replace("`", "") + + assert "HTTP 408, 425, 429, 500, 502, 503, and 504" in normalized_doctoring + assert "temporary DNS resolution reported as EAI_AGAIN" in normalized_doctoring + assert "connection-level urllib.error.URLError or OSError failures" not in normalized_doctoring + assert "408, 429, or 5xx" not in changelog