diff --git a/.github/workflows/trusted-uv-materializer-quality-ci.yml b/.github/workflows/trusted-uv-materializer-quality-ci.yml index 95642b55c..8f6ab968c 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_requirements_directory_lock_materialization.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_requirements_directory_lock_materialization.py" - "tests/test_trusted_uv*.py" - "tests/test_uv*.py" - "tests/test_repository_branch_coverage_*.py" @@ -126,6 +128,7 @@ jobs: python -m coverage run -m pytest \ tests/test_materialize_base_python_requirements.py \ tests/test_materialize_uv_export_hash_contract.py \ + tests/test_requirements_directory_lock_materialization.py \ tests/test_trusted_uv_download_contract.py \ tests/test_trusted_uv_portability_and_streaming.py \ tests/test_uv_export_isolation_contract.py \ @@ -152,6 +155,7 @@ jobs: scripts/ci/materialize_base_python_requirements.py \ tests/test_materialize_base_python_requirements.py \ tests/test_materialize_uv_export_hash_contract.py \ + tests/test_requirements_directory_lock_materialization.py \ tests/test_trusted_uv_download_contract.py \ tests/test_trusted_uv_portability_and_streaming.py \ tests/test_uv_export_isolation_contract.py \ diff --git a/CHANGELOG.md b/CHANGELOG.md index e601de81b..25e54054d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,17 @@ Semantic Versioning where the repository publishes a release. ### Added +- Added a trusted pull-request comment router for `@cwl-noema-review` and review-only `@opencode-agent` dispatches, with an organization sweep, exact-head receipts, repository allowlisting, fixed runners, immutable checkout pins, and a permanent 100% statement/branch/docstring quality gate. - Added exact-base `uv.lock` materialization that reconstructs standalone nested projects with a checksum-pinned official `uv` exporter, isolated frozen/offline execution, strict exact-pin and SHA-256 output validation, and complete Python 3.10/3.14 quality evidence. ### 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. + +### Changed + +- Materialize complete hash-pinned `requirements/ci.txt` and other direct `requirements/*.txt` base-owned closures so isolated OpenCode coverage imports repository runtime dependencies without trusting pull-request metadata or broadening network access. diff --git a/docs/doctoring/trusted-requirements-directory-lock-discovery.md b/docs/doctoring/trusted-requirements-directory-lock-discovery.md new file mode 100644 index 000000000..e21ee7f3b --- /dev/null +++ b/docs/doctoring/trusted-requirements-directory-lock-discovery.md @@ -0,0 +1,53 @@ +# Trusted requirements-directory lock discovery + +## Decision + +The central OpenCode coverage image materializes dependency closures only from +regular files in the authenticated pull-request base commit. In addition to the +conventional `requirements*.txt` and `requirements.lock` names, it recognizes a +`.txt` file that is a **direct child** of a directory named `requirements`, such +as `requirements/ci.txt` or `services/scoring_service/requirements/package.txt`. + +The path rule grants candidate status only. A global `--require-hashes` +directive is not trust evidence by itself: every non-directive logical +requirement must still carry an inline hash or a bounded requirement include. +The existing content boundary records the exact trusted source path in the +manifest and preflights each candidate as an independently installable +`pip --require-hashes` closure. Unpinned notes, directive-only files, input +files, nested descendants, symbolic links, pull-request-only files, and +malformed Git tree entries remain excluded. + +## Operational reason + +Concrete environment locks are frequently organized below a `requirements` +directory and use role names such as `ci.txt` or `package.txt`. Ignoring those +safe base-owned locks leaves isolated coverage without runtime dependencies even +when the repository maintains a complete generated closure. The resulting import +failure measures the coverage image rather than the changed production code. + +## Verification + +- A failing contract first proved that `requirements/ci.txt` was undiscoverable. +- Direct `requirements/*.txt` and nested-service equivalents are accepted. +- A deeper `requirements/nested/ci.txt` path and unrelated `docs/ci.txt` remain + ineligible. +- A global `--require-hashes` directive combined with an unpinned requirement is + rejected rather than promoted into the networked coverage image. +- Only the hash-pinned candidate is emitted from a realistic temporary Git base; + unpinned `.in` and human-readable `.txt` files remain absent. +- The focused materializer suite, complete central suite, statement and branch + coverage, docstring gate, compilation, and exact-head security workflows are + required before merge. + +## References + +Python Packaging Authority. (2026). *Install requires vs requirements files*. +Python Packaging User Guide. +https://packaging.python.org/en/latest/discussions/install-requires-vs-requirements/ + +Python Packaging Authority. (2026). *Repeatable installs*. pip documentation. +https://pip.pypa.io/en/stable/topics/repeatable-installs/ + +Python Packaging Authority. (2026). *Requirements file format*. pip +documentation. +https://pip.pypa.io/en/stable/reference/requirements-file-format/ diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py old mode 100755 new mode 100644 index 98cdad459..807c33aa4 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -87,6 +87,21 @@ 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 _requirement_lines(content: bytes) -> list[str]: """Return logical requirement lines, joining backslash line-continuations. @@ -115,15 +130,19 @@ def _is_hash_pinned(content: bytes) -> bool: 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. + candidate as an independent ``--require-hashes`` closure. A global + ``--require-hashes`` directive is not trust evidence by itself: every + non-directive logical requirement must still carry an inline hash or a + bounded requirement include. An empty file or directive-only file carries no + installable dependency and is not materialized. """ 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( + return all( "--hash=" in line or line.startswith(("-r ", "--requirement ")) - for line in lines + for line in requirement_lines ) @@ -459,7 +478,7 @@ def base_hash_locks(repo_root: pathlib.Path, base_sha: str) -> list[tuple[str, b regular_paths = {path for path, _candidate in regular_blobs} locks: list[tuple[str, bytes]] = [] for path, candidate in regular_blobs: - if _is_candidate_lock_name(candidate.name): + if _is_candidate_lock_path(candidate): content = _git(repo_root, "show", f"{base_sha}:{path}") if _is_hash_pinned(content): locks.append((path, content)) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 8a383f0c2..0b7907ae9 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -150,7 +150,7 @@ 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 not materializer._is_hash_pinned(b"untrusted==1\n") diff --git a/tests/test_requirements_directory_lock_materialization.py b/tests/test_requirements_directory_lock_materialization.py new file mode 100644 index 000000000..d9f850e54 --- /dev/null +++ b/tests/test_requirements_directory_lock_materialization.py @@ -0,0 +1,178 @@ +"""Regression contracts for trusted locks kept in a requirements directory.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path, PurePosixPath + +import pytest + +from scripts.ci import materialize_base_python_requirements as materializer + + +def _git(repo: Path, *args: str) -> str: + """Run one deterministic Git command in a temporary fixture repository.""" + return subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def test_requirements_directory_txt_is_a_candidate_lock_path() -> None: + """A direct ``requirements/*.txt`` lock is discoverable by its safe path.""" + assert materializer._is_candidate_lock_path(PurePosixPath("requirements/ci.txt")) + assert materializer._is_candidate_lock_path( + PurePosixPath("services/scoring_service/requirements/package.txt") + ) + assert not materializer._is_candidate_lock_path( + PurePosixPath("requirements/nested/ci.txt") + ) + assert not materializer._is_candidate_lock_path(PurePosixPath("docs/ci.txt")) + + +def test_materializes_hash_pinned_requirements_directory_lock( + tmp_path: Path, +) -> None: + """The exact base ``requirements/ci.txt`` closure reaches offline coverage.""" + repo = tmp_path / "repo" + requirements_dir = repo / "requirements" + requirements_dir.mkdir(parents=True) + _git(repo, "init") + _git(repo, "config", "user.name", "Test") + _git(repo, "config", "user.email", "test@example.invalid") + + (requirements_dir / "ci.txt").write_text( + "numpy==2.5.1 --hash=sha256:" + ("a" * 64) + "\n", + encoding="utf-8", + ) + (requirements_dir / "ci.in").write_text("numpy>=2\n", encoding="utf-8") + (requirements_dir / "notes.txt").write_text( + "human-readable notes only\n", encoding="utf-8" + ) + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + base_sha = _git(repo, "rev-parse", "HEAD") + + output = tmp_path / "output" + manifest = materializer.materialize(repo, base_sha, output) + + assert manifest == [ + {"file": "requirements-000.txt", "source": "requirements/ci.txt"} + ] + assert (output / "requirements-000.txt").read_text(encoding="utf-8").startswith( + "numpy==2.5.1" + ) + + +@pytest.mark.parametrize( + ("content", "expected"), + ( + (b"--require-hashes\n", False), + (b"--require-hashes\ndemo==1\n", False), + ( + b"--require-hashes\ndemo==1 --hash=sha256:" + + (b"a" * 64) + + b"\n", + True, + ), + ( + b"pinned==1 --hash=sha256:" + + (b"b" * 64) + + b"\nunpinned==2\n", + False, + ), + (b"--index-url https://packages.example.invalid/simple\n", False), + (b"--requirement other.txt\n", True), + ), +) +def test_global_hash_directive_does_not_replace_per_requirement_trust( + content: bytes, + expected: bool, +) -> None: + """Only substantive hashed pins or bounded requirement includes qualify.""" + assert materializer._is_hash_pinned(content) is expected + + +@pytest.mark.parametrize( + "unsafe_content", + ( + b"demo>=1 --hash=sha256:" + (b"a" * 64) + b"\n", + b"demo==1 --hash=sha256:not-a-complete-digest\n", + b"--index-url https://packages.example.invalid/simple --hash=sha256:" + + (b"a" * 64) + + b"\n", + b"-r /tmp/absolute.txt\n", + b"--requirement ../parent.txt\n", + b"-r nested/../../escape.txt\n", + b"--requirement other.txt --hash=sha256:" + (b"a" * 64) + b"\n", + ), +) +def test_unsafe_requirement_lines_are_rejected_before_materialization( + unsafe_content: bytes, +) -> None: + """Unsafe package and include syntax never gains trusted candidate status.""" + assert not materializer._is_hash_pinned(unsafe_content) + + +@pytest.mark.parametrize( + "unsafe_text", + ( + "demo>=1 --hash=sha256:" + ("a" * 64) + "\n", + "demo==1 --hash=sha256:not-a-complete-digest\n", + "--index-url https://packages.example.invalid/simple --hash=sha256:" + + ("a" * 64) + + "\n", + "-r /tmp/absolute.txt\n", + "--requirement ../parent.txt\n", + ), +) +def test_unsafe_requirements_directory_candidate_is_excluded_from_manifest( + tmp_path: Path, + unsafe_text: str, +) -> None: + """Unsafe direct-child content is excluded before entering the build context.""" + repo = tmp_path / "repo" + requirements_dir = repo / "requirements" + requirements_dir.mkdir(parents=True) + _git(repo, "init") + _git(repo, "config", "user.name", "Test") + _git(repo, "config", "user.email", "test@example.invalid") + (requirements_dir / "ci.txt").write_text(unsafe_text, encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + base_sha = _git(repo, "rev-parse", "HEAD") + + output = tmp_path / "output" + manifest = materializer.materialize(repo, base_sha, output) + + assert manifest == [] + assert (output / "manifest.json").read_text(encoding="utf-8") == "[]\n" + + +def test_rejects_global_hash_directive_with_unpinned_requirement( + tmp_path: Path, +) -> None: + """A global directive cannot make an unpinned direct-child lock trusted.""" + repo = tmp_path / "repo" + requirements_dir = repo / "requirements" + requirements_dir.mkdir(parents=True) + _git(repo, "init") + _git(repo, "config", "user.name", "Test") + _git(repo, "config", "user.email", "test@example.invalid") + + (requirements_dir / "ci.txt").write_text( + "--require-hashes\ndemo==1\n", + encoding="utf-8", + ) + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + base_sha = _git(repo, "rev-parse", "HEAD") + + output = tmp_path / "output" + manifest = materializer.materialize(repo, base_sha, output) + + assert manifest == [] + assert not materializer._is_hash_pinned(b"--require-hashes\ndemo==1\n") + assert (output / "manifest.json").read_text(encoding="utf-8") == "[]\n" \ No newline at end of file