From 5f34e2de3fff4640be1adb38eeb0b0a7a34d8de6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 16:56:59 +0900 Subject: [PATCH 01/11] fix(coverage): trust validated Python head locks --- .../workflows/opencode-review-dispatch.yml | 1 + CHANGELOG.md | 4 + .../opencode-python-head-lock-trust.md | 58 +++++ .../materialize_base_python_requirements.py | 83 ++++++- ...st_materialize_base_python_requirements.py | 212 +++++++++++++++++- tests/test_opencode_agent_contract.py | 1 + ...t_pr_review_autofix_nvidia_nim_contract.py | 2 +- 7 files changed, 354 insertions(+), 7 deletions(-) create mode 100644 docs/doctoring/opencode-python-head-lock-trust.md diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index e4058a86c4..699052c87a 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -632,6 +632,7 @@ jobs: python3 -I "$GITHUB_WORKSPACE/scripts/ci/materialize_base_python_requirements.py" \ --repo-root "$COVERAGE_SOURCE_WORKDIR" \ --base-sha "$PR_BASE_SHA" \ + --head-sha "$PR_HEAD_SHA" \ --output-dir "$coverage_build_dir/base-python-requirements" python3 -I "$GITHUB_WORKSPACE/scripts/ci/materialize_base_javascript_packages.py" \ --repo-root "$COVERAGE_SOURCE_WORKDIR" \ diff --git a/CHANGELOG.md b/CHANGELOG.md index e9717d09a9..e1aaa7dac0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- Let OpenCode coverage replace a changed base Python lock with the validated + current-head flat lock, while rejecting unbounded or include-based HEAD locks; + this keeps Python dependency updates measurable without weakening + `--require-hashes` or `--only-binary=:all:`. - Give stacked pull requests a separately bounded organization-sweep OpenCode dispatch budget, so default-branch review traffic cannot leave a stacked PR at `OpenCode review absent` without changing the protected merge diff --git a/docs/doctoring/opencode-python-head-lock-trust.md b/docs/doctoring/opencode-python-head-lock-trust.md new file mode 100644 index 0000000000..91c4cecab7 --- /dev/null +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -0,0 +1,58 @@ +# OpenCode coverage validated-head Python locks + +검토 기준일: **2026-08-29** + +## Decision + +OpenCode coverage may use a pull request's changed Python requirements lock when +the lock is read from the authenticated, validated HEAD SHA and every logical +requirement is an exact `==` pin with one or more complete SHA-256 hashes. The +lock must be flat: relative includes, URLs, VCS sources, and unpinned lines do +not cross into the networked coverage-image build context. + +An unchanged lock remains materialized from the validated base SHA. When a +tracked base lock changes, the validated flat HEAD lock replaces it rather than +installing both revisions. A base lock that becomes unbounded fails closed. + +## Root cause + +The coverage image was built only from `PR_BASE_SHA` Python locks. A legitimate +dependency pull request could therefore add a platform wheel hash to its +current-head lock, while the image builder still saw the older base file. Pip +then failed before PR tests because the compatible wheel's hash was absent from +the stale lock. The JavaScript materializer already handled this case by +validating and recording changed HEAD locks; Python had no equivalent path. + +## Security boundary + +The central workflow still fetches and validates the base and head revisions +before materialization. Only a regular candidate lock from the exact HEAD is +read, and only a flat SHA-256-pinned file can replace a base lock. The image +installer retains `pip install --require-hashes --only-binary=:all:`; source +distributions, build backends, VCS dependencies, relative include graphs, and +unbounded requirements remain rejected or outside this path. The later PR +sandbox remains networkless and credential-free. + +This is a provenance and compatibility repair, not an approval or merge +mechanism. Current-head OpenCode, Strix, other required Checks, review threads, +and protected-branch rules remain independent gates. + +## Verification contract + +Regression coverage proves that a changed exact-head lock replaces stale base +content and that an unbounded changed lock is rejected before image build. +Workflow contract coverage proves that the materializer receives both +`PR_BASE_SHA` and `PR_HEAD_SHA`. The central quality gate must retain complete +statement, branch, and docstring coverage. + +## APA 7th references + +Python Packaging Authority. (n.d.). *Secure installs*. pip documentation. +Retrieved August 29, 2026, from +https://pip.pypa.io/en/stable/topics/secure-installs/ + +Python Packaging Authority. (n.d.). *pip install*. pip documentation. Retrieved +August 29, 2026, from https://pip.pypa.io/en/stable/cli/pip_install/ + +Supply-chain Levels for Software Artifacts. (2025). *SLSA specification (version +1.2)*. https://slsa.dev/spec/v1.2/ diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index a052123547..f38e20324a 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Materialize hash-pinned Python locks from a validated pull-request base commit.""" +"""Materialize hash-pinned Python locks from validated pull-request revisions.""" from __future__ import annotations @@ -667,6 +667,65 @@ def base_hash_locks(repo_root: pathlib.Path, base_sha: str) -> list[tuple[str, b return _base_python_inputs(repo_root, base_sha)[0] +def _candidate_lock_blobs( + repo_root: pathlib.Path, revision_sha: str +) -> dict[str, tuple[bytes, str]]: + """Return candidate lock content and blob IDs from one exact revision.""" + if not SHA_RE.fullmatch(revision_sha): + raise ValueError("revision SHA must be exactly 40 hexadecimal characters") + entries = _git(repo_root, "ls-tree", "-r", "-z", "--full-tree", revision_sha) + candidates: dict[str, tuple[bytes, str]] = {} + for path, candidate in _regular_base_blob_paths(entries): + if not _is_candidate_lock_path(candidate): + continue + content = _git(repo_root, "show", f"{revision_sha}:{path}") + blob = _git(repo_root, "rev-parse", f"{revision_sha}:{path}") + blob_sha = blob.decode("ascii", errors="strict").strip().lower() + if not SHA_RE.fullmatch(blob_sha): + raise RuntimeError(f"git rev-parse returned an invalid blob SHA for {path}") + candidates[path] = (content, blob_sha) + return candidates + + +def _select_python_locks( + repo_root: pathlib.Path, + base_sha: str, + base_locks: list[tuple[str, bytes]], + head_sha: str, +) -> list[tuple[str, bytes]]: + """Replace changed base locks with complete, exact-head flat locks.""" + base_candidates = { + source_path: content + for source_path, content in base_locks + if _is_candidate_lock_path(pathlib.PurePosixPath(source_path)) + } + selected = dict(base_locks) + head_candidates = _candidate_lock_blobs(repo_root, head_sha) + + for source_path in base_candidates: + if source_path not in head_candidates: + selected.pop(source_path, None) + + for source_path, (content, head_blob) in head_candidates.items(): + if source_path not in base_candidates: + if _is_flat_materializable_lock(content): + selected[source_path] = content + continue + base_blob = _git(repo_root, "rev-parse", f"{base_sha}:{source_path}") + base_blob_sha = base_blob.decode("ascii", errors="strict").strip().lower() + if base_blob_sha == head_blob: + continue + if _is_flat_materializable_lock(content): + selected[source_path] = content + continue + raise ValueError( + f"current-head Python lock {source_path} is not a complete " + "SHA-256-pinned flat lock" + ) + + return sorted(selected.items(), key=lambda item: item[0]) + + def _included_base_lock_blobs( repo_root: pathlib.Path, base_sha: str, @@ -725,8 +784,9 @@ def materialize( repo_root: pathlib.Path, base_sha: str, output_dir: pathlib.Path, + head_sha: str | None = None, ) -> list[dict[str, str]]: - """Write base locks and resolvable bounded includes into a safe context.""" + """Write trusted base locks and safe exact-head lock replacements.""" 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) @@ -737,6 +797,10 @@ def materialize( path for path, _candidate in _regular_base_blob_paths(entries) } locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) + if head_sha is not None: + locks = _select_python_locks( + resolved_repo, base_sha, locks, head_sha + ) manifest: list[dict[str, str]] = [] for index, (source_path, content) in enumerate(locks): generated_name = f"requirements-{index:03d}.txt" @@ -774,15 +838,24 @@ def materialize( def main(argv: list[str] | None = None) -> int: - """Materialize base locks and report exactly which trusted paths were selected.""" + """Materialize trusted locks and report exactly which paths were selected.""" parser = argparse.ArgumentParser() parser.add_argument("--repo-root", required=True, type=pathlib.Path) parser.add_argument("--base-sha", required=True) + parser.add_argument("--head-sha") parser.add_argument("--output-dir", required=True, type=pathlib.Path) args = parser.parse_args(argv) try: - manifest = materialize(args.repo_root, args.base_sha, args.output_dir) + if args.head_sha is None: + manifest = materialize(args.repo_root, args.base_sha, args.output_dir) + else: + manifest = materialize( + args.repo_root, + args.base_sha, + args.output_dir, + head_sha=args.head_sha, + ) except (OSError, RuntimeError, ValueError) as exc: print( f"::error::Could not materialize base Python locks: {exc}", file=sys.stderr @@ -792,7 +865,7 @@ def main(argv: list[str] | None = None) -> int: if manifest: for entry in manifest: print( - "Materialized trusted base Python lock " + "Materialized trusted Python lock " f"{entry['source']} as {entry['file']}." ) else: diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 58ded37400..09ee923211 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -104,6 +104,180 @@ def test_materializes_only_regular_hash_locks_from_exact_base(tmp_path: Path) -> assert (output / "vcs-manifest.json").read_text(encoding="utf-8") == "[]\n" +def test_materializes_changed_head_hash_lock_instead_of_stale_base( + tmp_path: Path, +) -> None: + """A changed exact-head lock replaces the stale base lock in the image context.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "requirements-quality.txt").write_text( + "demo==1 --hash=sha256:" + ("a" * 64) + "\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "base") + base_sha = git(repo, "rev-parse", "HEAD") + + (repo / "requirements-quality.txt").write_text( + "demo==1 --hash=sha256:" + ("a" * 64) + "\n" + "demo-platform==2 --hash=sha256:" + ("b" * 64) + "\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "head") + head_sha = git(repo, "rev-parse", "HEAD") + + manifest = materializer.materialize( + repo, base_sha, tmp_path / "output", head_sha=head_sha + ) + + assert manifest == [ + {"file": "requirements-000.txt", "source": "requirements-quality.txt"} + ] + assert ( + (tmp_path / "output" / "requirements-000.txt").read_text(encoding="utf-8") + == (repo / "requirements-quality.txt").read_text(encoding="utf-8") + ) + + +def test_rejects_changed_head_lock_without_complete_hash_pins(tmp_path: Path) -> None: + """An unbounded current-head lock cannot enter the networked image context.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "requirements-quality.txt").write_text( + "demo==1 --hash=sha256:" + ("a" * 64) + "\n", + encoding="utf-8", + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "base") + base_sha = git(repo, "rev-parse", "HEAD") + + (repo / "requirements-quality.txt").write_text("untrusted==9\n", encoding="utf-8") + git(repo, "add", ".") + git(repo, "commit", "-m", "unbounded head") + head_sha = git(repo, "rev-parse", "HEAD") + + with pytest.raises(ValueError, match="current-head Python lock"): + materializer.materialize( + repo, base_sha, tmp_path / "output", head_sha=head_sha + ) + + +def test_candidate_lock_blobs_rejects_invalid_revision_sha(tmp_path: Path) -> None: + """Current-head discovery refuses symbolic or abbreviated revisions.""" + with pytest.raises(ValueError, match="revision SHA must be exactly 40"): + materializer._candidate_lock_blobs(tmp_path, "HEAD") + + +def test_candidate_lock_blobs_skips_non_lock_blobs( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """HEAD discovery considers only eligible regular lock paths.""" + tree = ( + b"100644 blob " + + b"a" * 40 + + b"\tREADME.md\0" + + b"100644 blob " + + b"b" * 40 + + b"\trequirements.txt\0" + ) + monkeypatch.setattr( + materializer, + "_git", + lambda _repo, *args: ( + tree + if args[0] == "ls-tree" + else b"demo==1 --hash=sha256:" + b"a" * 64 + if args[0] == "show" + else b"b" * 40 + ), + ) + + assert list(materializer._candidate_lock_blobs(tmp_path, "c" * 40)) == [ + "requirements.txt" + ] + + +def test_candidate_lock_blobs_rejects_invalid_blob_sha( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A malformed Git blob identity cannot authenticate a HEAD lock.""" + tree = b"100644 blob " + b"a" * 40 + b"\trequirements.txt\0" + + def fake_git(_repo: Path, *args: str) -> bytes: + if args[0] == "ls-tree": + return tree + if args[0] == "show": + return b"demo==1 --hash=sha256:" + b"a" * 64 + return b"not-a-sha" + + monkeypatch.setattr(materializer, "_git", fake_git) + + with pytest.raises(RuntimeError, match="invalid blob SHA"): + materializer._candidate_lock_blobs(tmp_path, "c" * 40) + + +def test_select_python_locks_handles_new_removed_and_unchanged_head_locks( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Selection adds new locks, removes deleted locks, and preserves equal blobs.""" + new_content = b"new==1 --hash=sha256:" + b"b" * 64 + monkeypatch.setattr( + materializer, + "_candidate_lock_blobs", + lambda _repo, _head: {"requirements-new.txt": (new_content, "b" * 40)}, + ) + assert materializer._select_python_locks( + tmp_path, + "a" * 40, + [], + "c" * 40, + ) == [("requirements-new.txt", new_content)] + + monkeypatch.setattr( + materializer, + "_candidate_lock_blobs", + lambda _repo, _head: { + "requirements-unpinned.txt": (b"untrusted==9\n", "f" * 40) + }, + ) + assert materializer._select_python_locks( + tmp_path, + "a" * 40, + [], + "c" * 40, + ) == [] + + monkeypatch.setattr(materializer, "_candidate_lock_blobs", lambda *_args: {}) + old_content = b"old==1 --hash=sha256:" + b"c" * 64 + assert materializer._select_python_locks( + tmp_path, + "a" * 40, + [("requirements-old.txt", old_content)], + "c" * 40, + ) == [] + + same_content = b"same==1 --hash=sha256:" + b"d" * 64 + monkeypatch.setattr( + materializer, + "_candidate_lock_blobs", + lambda *_args: {"requirements-same.txt": (same_content, "e" * 40)}, + ) + monkeypatch.setattr(materializer, "_git", lambda *_args: b"e" * 40) + assert materializer._select_python_locks( + tmp_path, + "a" * 40, + [("requirements-same.txt", same_content)], + "c" * 40, + ) == [("requirements-same.txt", same_content)] + + def test_materializes_exact_vcs_sources_in_a_separate_manifest( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -589,11 +763,47 @@ def fake_materialize( == 0 ) assert ( - "Materialized trusted base Python lock backend/requirements-hashes.txt " + "Materialized trusted Python lock backend/requirements-hashes.txt " "as requirements-000.txt." in capsys.readouterr().out ) +def test_main_forwards_optional_head_sha( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The CLI passes an explicitly supplied HEAD revision to materialization.""" + captured: dict[str, str] = {} + + def fake_materialize( + _repo_root: Path, + _base_sha: str, + _output_dir: Path, + *, + head_sha: str, + ) -> list[dict[str, str]]: + captured["head_sha"] = head_sha + return [] + + monkeypatch.setattr(materializer, "materialize", fake_materialize) + + assert ( + materializer.main( + [ + "--repo-root", + str(tmp_path), + "--base-sha", + "a" * 40, + "--head-sha", + "b" * 40, + "--output-dir", + str(tmp_path / "output"), + ] + ) + == 0 + ) + assert captured == {"head_sha": "b" * 40} + + def test_main_reports_when_no_locks_exist( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 7dfb9efaf7..31be3f21c1 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2407,6 +2407,7 @@ def test_opencode_privileged_review_security_boundaries_are_fail_closed(): assert "materialize_base_python_requirements.py" in measure assert "install_base_python_locks.py" in measure assert "base-python-requirements" in measure + assert '--head-sha "$PR_HEAD_SHA"' in measure assert "strictly registry/hash-bounded npm inputs from the live-validated" in measure assert 'chmod 0444 "$implementation_changed_files"' in measure assert "npm ci \\" in coverage_job diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 2aba930965..c93e8311b8 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "e4058a86c48527df458e6b4d3e5e3d696b738b1c" +REVIEW_DISPATCH_BLOB_SHA = "699052c87a74f809a918067e88a1fce2357c6832" def _workflow_text(path: Path) -> str: From b9c2c66db536eb036bf1311e68242d1eb39d5372 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 17:08:58 +0900 Subject: [PATCH 02/11] fix(coverage): revalidate head requirement includes --- CHANGELOG.md | 8 +- .../opencode-python-head-lock-trust.md | 32 +++++--- .../materialize_base_python_requirements.py | 38 ++++++++-- ...st_materialize_base_python_requirements.py | 74 +++++++++++++++++++ 4 files changed, 132 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1aaa7dac0..132d678000 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,11 @@ Semantic Versioning where the repository publishes a release. ## [Unreleased] - Let OpenCode coverage replace a changed base Python lock with the validated - current-head flat lock, while rejecting unbounded or include-based HEAD locks; - this keeps Python dependency updates measurable without weakening - `--require-hashes` or `--only-binary=:all:`. + current-head flat lock, and revalidate bounded includes beneath unchanged + parents against the exact HEAD tree; changed includes must remain flat and + pinned, while deleted or invalid includes fail closed. This keeps Python + dependency updates measurable without weakening `--require-hashes` or + `--only-binary=:all:`. - Give stacked pull requests a separately bounded organization-sweep OpenCode dispatch budget, so default-branch review traffic cannot leave a stacked PR at `OpenCode review absent` without changing the protected merge diff --git a/docs/doctoring/opencode-python-head-lock-trust.md b/docs/doctoring/opencode-python-head-lock-trust.md index 91c4cecab7..bdc7f00343 100644 --- a/docs/doctoring/opencode-python-head-lock-trust.md +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -6,13 +6,17 @@ OpenCode coverage may use a pull request's changed Python requirements lock when the lock is read from the authenticated, validated HEAD SHA and every logical -requirement is an exact `==` pin with one or more complete SHA-256 hashes. The -lock must be flat: relative includes, URLs, VCS sources, and unpinned lines do -not cross into the networked coverage-image build context. +requirement is an exact `==` pin with one or more complete SHA-256 hashes. A +changed HEAD lock must be flat: URLs, VCS sources, relative includes, and +unpinned lines do not cross into the networked coverage-image build context. An unchanged lock remains materialized from the validated base SHA. When a tracked base lock changes, the validated flat HEAD lock replaces it rather than -installing both revisions. A base lock that becomes unbounded fails closed. +installing both revisions. For an unchanged base lock with a bounded relative +include, the include blob is compared with the exact HEAD tree: a changed +include is materialized only when it remains flat and fully SHA-256 pinned, while +deleted or invalid content fails closed. A base lock that becomes unbounded +fails closed. ## Root cause @@ -27,10 +31,13 @@ validating and recording changed HEAD locks; Python had no equivalent path. The central workflow still fetches and validates the base and head revisions before materialization. Only a regular candidate lock from the exact HEAD is -read, and only a flat SHA-256-pinned file can replace a base lock. The image -installer retains `pip install --require-hashes --only-binary=:all:`; source -distributions, build backends, VCS dependencies, relative include graphs, and -unbounded requirements remain rejected or outside this path. The later PR +read, and only a flat SHA-256-pinned file can replace a base lock. A bounded +include beneath an unchanged base lock is likewise read from HEAD only after +its exact base/head blob comparison and regular-file check; its content must be +flat and fully pinned. The image installer retains +`pip install --require-hashes --only-binary=:all:`; source distributions, build +backends, VCS dependencies, unbounded requirements, deleted includes, and +invalid include content remain rejected or outside this path. The later PR sandbox remains networkless and credential-free. This is a provenance and compatibility repair, not an approval or merge @@ -40,10 +47,11 @@ and protected-branch rules remain independent gates. ## Verification contract Regression coverage proves that a changed exact-head lock replaces stale base -content and that an unbounded changed lock is rejected before image build. -Workflow contract coverage proves that the materializer receives both -`PR_BASE_SHA` and `PR_HEAD_SHA`. The central quality gate must retain complete -statement, branch, and docstring coverage. +content, and that a changed, deleted, or invalid include beneath an unchanged +parent is handled from the exact HEAD before image build. Workflow contract +coverage proves that the materializer receives both `PR_BASE_SHA` and +`PR_HEAD_SHA`. The central quality gate must retain complete statement, branch, +and docstring coverage. ## APA 7th references diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index f38e20324a..bf1e01e149 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -732,8 +732,11 @@ def _included_base_lock_blobs( source_path: str, content: bytes, regular_paths: set[str], + *, + head_sha: str | None = None, + head_regular_paths: set[str] | None = None, ) -> list[tuple[pathlib.PurePosixPath, bytes]]: - """Load direct bounded includes from the exact base as complete closures.""" + """Load direct bounded includes from the selected revision as closures.""" source_parent = pathlib.PurePosixPath(source_path).parent included: dict[pathlib.PurePosixPath, bytes] = {} for line in _requirement_lines(content): @@ -742,14 +745,29 @@ def _included_base_lock_blobs( continue resolved = source_parent / target resolved_path = resolved.as_posix() - if resolved_path not in regular_paths: + selected_sha = base_sha + selected_paths = regular_paths + revision_label = "base" + if head_sha is not None: + if head_regular_paths is None: + raise ValueError("current-head regular paths are required") + selected_paths = head_regular_paths + revision_label = "current-head" + if resolved_path not in selected_paths: raise RuntimeError( - f"bounded include {target} from {source_path} is not a regular base blob" + f"bounded include {target} from {source_path} is not a regular " + f"{revision_label} blob" ) - included_content = _git(repo_root, "show", f"{base_sha}:{resolved_path}") + if head_sha is not None: + base_blob = _git(repo_root, "rev-parse", f"{base_sha}:{resolved_path}") + head_blob = _git(repo_root, "rev-parse", f"{head_sha}:{resolved_path}") + if base_blob != head_blob: + selected_sha = head_sha + included_content = _git(repo_root, "show", f"{selected_sha}:{resolved_path}") if not _is_flat_materializable_lock(included_content): raise RuntimeError( - f"bounded include {resolved_path} must contain only exact SHA-256 pins" + f"{revision_label} bounded include {resolved_path} must contain " + "only exact SHA-256 pins" ) included[target] = included_content return sorted(included.items(), key=lambda item: item[0].as_posix()) @@ -796,6 +814,14 @@ def materialize( regular_paths = { path for path, _candidate in _regular_base_blob_paths(entries) } + head_regular_paths: set[str] | None = None + if head_sha is not None: + head_entries = _git( + resolved_repo, "ls-tree", "-r", "-z", "--full-tree", head_sha + ) + head_regular_paths = { + path for path, _candidate in _regular_base_blob_paths(head_entries) + } locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) if head_sha is not None: locks = _select_python_locks( @@ -811,6 +837,8 @@ def materialize( source_path, content, regular_paths, + head_sha=head_sha, + head_regular_paths=head_regular_paths, ) for relative_target, included_content in included: destination = output_dir / include_directory / pathlib.Path(*relative_target.parts) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 09ee923211..448d4bb944 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -572,6 +572,80 @@ def test_materialization_rejects_missing_or_nested_include(tmp_path: Path) -> No ) +@pytest.mark.parametrize( + ("head_child", "expected_child", "expected_error"), + [ + ( + "demo==2 --hash=sha256:" + ("e" * 64) + "\n", + "demo==2 --hash=sha256:" + ("e" * 64) + "\n", + None, + ), + ( + "demo==1 --hash=sha256:" + ("d" * 64) + "\n", + "demo==1 --hash=sha256:" + ("d" * 64) + "\n", + None, + ), + (None, None, "not a regular current-head blob"), + ("untrusted==2\n", None, "current-head bounded include"), + ], +) +def test_materialization_revalidates_includes_at_current_head( + tmp_path: Path, + head_child: str | None, + expected_child: str | None, + expected_error: str | None, +) -> None: + """An unchanged parent cannot retain stale or unsafe HEAD include content.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + (repo / "requirements.txt").write_text("-r child.txt\n", encoding="utf-8") + (repo / "child.txt").write_text( + "demo==1 --hash=sha256:" + ("d" * 64) + "\n", encoding="utf-8" + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "base") + base_sha = git(repo, "rev-parse", "HEAD") + + child = repo / "child.txt" + if head_child is None: + child.unlink() + else: + child.write_text(head_child, encoding="utf-8") + git(repo, "add", "-A") + git(repo, "commit", "--allow-empty", "-m", "head") + head_sha = git(repo, "rev-parse", "HEAD") + + if expected_error is not None: + with pytest.raises(RuntimeError, match=expected_error): + materializer.materialize( + repo, base_sha, tmp_path / "output", head_sha=head_sha + ) + return + + materializer.materialize( + repo, base_sha, tmp_path / "output", head_sha=head_sha + ) + assert (tmp_path / "output" / "includes-000" / "child.txt").read_text( + encoding="utf-8" + ) == expected_child + + +def test_included_head_lock_requires_head_tree_paths(tmp_path: Path) -> None: + """Current-head include validation cannot run without its exact tree paths.""" + with pytest.raises(ValueError, match="current-head regular paths are required"): + materializer._included_base_lock_blobs( + tmp_path, + "a" * 40, + "requirements.txt", + b"-r child.txt\n", + {"child.txt"}, + head_sha="b" * 40, + ) + + def test_bounded_repair_driver_runs_against_a_staged_fixture( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From fe131b7b994ea87224cf4270a25a8bcbef892207 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 17:25:02 +0900 Subject: [PATCH 03/11] fix(coverage): refresh changed uv inputs --- ARCHITECTURE.md | 8 +- CHANGELOG.md | 3 + .../opencode-exact-vcs-dependency-evidence.md | 18 +- .../opencode-python-head-lock-trust.md | 21 +- .../trusted-uv-lock-materialization.md | 40 ++-- .../materialize_base_python_requirements.py | 117 +++++++++- ...st_materialize_base_python_requirements.py | 200 ++++++++++++++++++ 7 files changed, 364 insertions(+), 43 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a003f64016..316f94aa74 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -148,9 +148,11 @@ tests pin workflow structure and governance prose so drift fails closed. The trusted `uv` exporter is downloaded from the literal GitHub Releases URL for `uv` 0.12.1; `releases.astral.sh` is not the network sink. An exact-base `uv.lock` may additionally expose source from an organization-owned -GitHub repository pinned to a full commit: the secret-free image build verifies -the fetched revision and makes its source importable without running package -build or installation hooks. Pull-request execution remains networkless. +GitHub repository pinned to a full commit. When a `uv.lock` project changes, the +same export and source validation may use the exact current-head lock and sibling +metadata; unchanged projects remain base-bound. The secret-free image build +verifies the fetched revision and makes its source importable without running +package build or installation hooks. Pull-request execution remains networkless. Root-level lock files are independent environments unless an explicit include relationship says otherwise; only one unambiguous two-file supplement pair may be recovered together, so unrelated toolchains cannot create a synthetic diff --git a/CHANGELOG.md b/CHANGELOG.md index 132d678000..a5d2ed0f97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ Semantic Versioning where the repository publishes a release. pinned, while deleted or invalid includes fail closed. This keeps Python dependency updates measurable without weakening `--require-hashes` or `--only-binary=:all:`. +- Re-export changed or newly added `uv.lock` projects from the exact HEAD with + the existing frozen/offline exporter, while removing deleted projects' base + registry and VCS inputs and retaining unchanged projects at the base revision. - Give stacked pull requests a separately bounded organization-sweep OpenCode dispatch budget, so default-branch review traffic cannot leave a stacked PR at `OpenCode review absent` without changing the protected merge diff --git a/docs/doctoring/opencode-exact-vcs-dependency-evidence.md b/docs/doctoring/opencode-exact-vcs-dependency-evidence.md index a9b26ae474..357c6b894a 100644 --- a/docs/doctoring/opencode-exact-vcs-dependency-evidence.md +++ b/docs/doctoring/opencode-exact-vcs-dependency-evidence.md @@ -3,11 +3,14 @@ ## Decision The OpenCode coverage image may expose a Python dependency directly from source -only when the validated base branch's frozen `uv.lock` names an HTTPS GitHub -repository owned by `ContextualWisdomLab` and a full 40-character Git commit. -Registry dependencies remain exact-version, SHA-256-pinned `pip` installs. - -The trusted materializer separates those two dependency classes. The networked, +only when the validated base branch's frozen `uv.lock`, or a changed exact-head +`uv.lock` project selected for coverage, names an HTTPS GitHub repository owned +by `ContextualWisdomLab` and a full 40-character Git commit. Registry +dependencies remain exact-version, SHA-256-pinned `pip` installs. + +The trusted materializer separates those two dependency classes. Unchanged +projects remain base-bound; changed or newly added projects are exported from +the exact HEAD with the same isolated frozen/offline exporter. The networked, secret-free image build fetches each approved source revision, verifies that `FETCH_HEAD` and the checked-out `HEAD` equal the locked commit, removes Git metadata, verifies a normalized package import root, and records only that @@ -31,8 +34,9 @@ product's current-head tests passing. organization origin fail closed. - Duplicate references to one repository must resolve to one commit; conflicting revisions fail before the image build. -- Only metadata read from the validated base SHA can select a dependency. Pull - request source cannot modify the networked image build inputs. +- Only metadata read from the validated base SHA or a changed project in the + exact validated HEAD can select a dependency. Pull request source cannot + modify the networked image build inputs outside those immutable revisions. - Source dependencies are import-only. No `pip install`, PEP 517 backend, setup hook, or dependency lifecycle script runs while the network is available. - The source repository must be publicly fetchable without credentials, expose diff --git a/docs/doctoring/opencode-python-head-lock-trust.md b/docs/doctoring/opencode-python-head-lock-trust.md index bdc7f00343..309c2454dd 100644 --- a/docs/doctoring/opencode-python-head-lock-trust.md +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -16,7 +16,10 @@ installing both revisions. For an unchanged base lock with a bounded relative include, the include blob is compared with the exact HEAD tree: a changed include is materialized only when it remains flat and fully SHA-256 pinned, while deleted or invalid content fails closed. A base lock that becomes unbounded -fails closed. +fails closed. A changed or newly added `uv.lock` project with a regular sibling +`pyproject.toml` is re-exported from the exact HEAD through the existing frozen, +offline, checksum-validating exporter; deleted projects remove their base +registry and VCS inputs, and unchanged projects remain base-bound. ## Root cause @@ -31,7 +34,10 @@ validating and recording changed HEAD locks; Python had no equivalent path. The central workflow still fetches and validates the base and head revisions before materialization. Only a regular candidate lock from the exact HEAD is -read, and only a flat SHA-256-pinned file can replace a base lock. A bounded +read, and only a flat SHA-256-pinned file can replace a base lock. Changed +`uv.lock` projects reuse the same isolated exporter against exact HEAD +`uv.lock` and sibling metadata, then apply the established registry hash and +organization-owned full-commit VCS validation. A bounded include beneath an unchanged base lock is likewise read from HEAD only after its exact base/head blob comparison and regular-file check; its content must be flat and fully pinned. The image installer retains @@ -47,11 +53,12 @@ and protected-branch rules remain independent gates. ## Verification contract Regression coverage proves that a changed exact-head lock replaces stale base -content, and that a changed, deleted, or invalid include beneath an unchanged -parent is handled from the exact HEAD before image build. Workflow contract -coverage proves that the materializer receives both `PR_BASE_SHA` and -`PR_HEAD_SHA`. The central quality gate must retain complete statement, branch, -and docstring coverage. +content; a changed, deleted, or invalid include beneath an unchanged parent is +handled from the exact HEAD; and changed, deleted, registry-only, VCS-only, and +mixed `uv.lock` projects replace or remove every base export component before +image build. Workflow contract coverage proves that the materializer receives +both `PR_BASE_SHA` and `PR_HEAD_SHA`. The central quality gate must retain +complete statement, branch, and docstring coverage. ## APA 7th references diff --git a/docs/doctoring/trusted-uv-lock-materialization.md b/docs/doctoring/trusted-uv-lock-materialization.md index 2d83e8bda8..7a1f12b6ae 100644 --- a/docs/doctoring/trusted-uv-lock-materialization.md +++ b/docs/doctoring/trusted-uv-lock-materialization.md @@ -4,9 +4,12 @@ Central coverage automation may translate a tracked `uv.lock` from the exact validated pull-request base revision into a pip-compatible, hash-pinned -requirements closure. The translation must not depend on a mutable runner tool, -repository-head dependency metadata, ambient runner configuration, or network -access during export. +requirements closure. When current-head materialization is requested, a changed +or newly added `uv.lock` project is translated from its exact HEAD lock and +sibling metadata; deleted projects remove their base export, and unchanged +projects stay base-bound. The translation must not depend on a mutable runner +tool, repository-head dependency metadata outside the selected exact revision, +ambient runner configuration, or network access during export. The implementation therefore: @@ -16,9 +19,13 @@ The implementation therefore: same immutable revision; an absent sibling is an explicit orphan, while a read failure for an inventoried blob is fatal and cannot be misclassified as absence; -3. installs one process-wide urllib opener with an empty proxy map and a redirect +3. compares the base and current-head `uv.lock` plus sibling metadata blob IDs + before re-exporting a changed project; a deleted project removes its prior + registry and VCS entries, while a changed project uses the same isolated + exporter and exact registry/VCS validators against HEAD; +4. installs one process-wide urllib opener with an empty proxy map and a redirect handler that rejects every redirect before urllib creates a target request; -4. downloads one fixed official `uv` archive from the literal GitHub Releases +5. downloads one fixed official `uv` archive from the literal GitHub Releases HTTPS URL and accepts a response only when its parsed origin remains HTTPS on `github.com`, `release-assets.githubusercontent.com`, or `objects.githubusercontent.com` with the absent or explicit default port 443; @@ -28,22 +35,22 @@ The implementation therefore: because that vanity host now returns HTTP 403 for the pinned 0.12.1 archive (ContextualWisdomLab/.github#1109) while the GitHub Releases asset keeps the same SHA-256 digest; -5. verifies the bounded archive with a pinned SHA-256 digest before extraction; -6. accepts only the expected regular-file tar member within explicit size bounds; -7. writes the executable with mode `0755` and verifies that it reports the exact +6. verifies the bounded archive with a pinned SHA-256 digest before extraction; +7. accepts only the expected regular-file tar member within explicit size bounds; +8. writes the executable with mode `0755` and verifies that it reports the exact pinned `uv` version; -8. executes `uv export` with `--frozen`, `--offline`, `--no-cache`, +9. executes `uv export` with `--frozen`, `--offline`, `--no-cache`, `--no-progress`, `--color never`, `--no-emit-project`, and `--no-editable` in an isolated temporary project; -9. supplies a minimal environment with isolated home, temporary, cache, and +10. supplies a minimal environment with isolated home, temporary, cache, and configuration directories, disables dotenv loading and managed Python downloads, and does not inherit arbitrary runner variables; -10. keeps project metadata discovery enabled because the reconstructed +11. keeps project metadata discovery enabled because the reconstructed `pyproject.toml` is an authoritative input; `--no-config` is deliberately not used because uv documents that it disables `pyproject.toml` discovery; -11. rejects every nonempty export unless every logical line is an exact normalized +12. rejects every nonempty export unless every logical line is an exact normalized package `==` pin followed only by complete SHA-256 hashes; and -12. exposes only generated requirements files and a source manifest to the later +13. exposes only generated requirements files and a source manifest to the later networkless coverage environment. ## Standards and current-tool rationale @@ -92,8 +99,9 @@ executable payload identity. Nested standalone services are supported: a repository may contain several independent directories, each with its own sibling `pyproject.toml` and `uv.lock`; each pair is read and exported independently from the immutable base -revision. This fits the organization’s standalone-product plus reusable-module -MSA contract without copying central review logic into product repositories. +revision, or from the exact HEAD when its lock or metadata changed. This fits +the organization’s standalone-product plus reusable-module MSA contract without +copying central review logic into product repositories. A true uv workspace can require member `pyproject.toml` files in addition to the root lock and root project metadata. The current materializer does not @@ -123,6 +131,8 @@ Regression coverage must prove: - continued project metadata discovery with no `--no-config` regression; - timeout, process, parse, and exporter failures fail closed; - orphan locks and empty third-party closures remain nonfatal and explicit; +- changed, deleted, registry-only, VCS-only, and mixed current-head `uv.lock` + projects replace or remove their base registry and VCS manifests; - every nonempty line is a normalized exact package pin with one or more complete SHA-256 hashes; and - `pyproject.toml` enables branch measurement and the changed production module diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index bf1e01e149..223dd643f6 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -662,6 +662,18 @@ def _base_python_inputs( ) +def _regular_uv_lock_paths( + regular_blobs: list[tuple[str, pathlib.PurePosixPath]], + regular_paths: set[str], +) -> set[str]: + """Return regular uv projects whose sibling metadata is also regular.""" + return { + path + for path, candidate in regular_blobs + if candidate.name == "uv.lock" and _uv_pyproject_path(path) in regular_paths + } + + def base_hash_locks(repo_root: pathlib.Path, base_sha: str) -> list[tuple[str, bytes]]: """Return regular hash-lock blobs from the exact validated base commit.""" return _base_python_inputs(repo_root, base_sha)[0] @@ -679,14 +691,80 @@ def _candidate_lock_blobs( if not _is_candidate_lock_path(candidate): continue content = _git(repo_root, "show", f"{revision_sha}:{path}") - blob = _git(repo_root, "rev-parse", f"{revision_sha}:{path}") - blob_sha = blob.decode("ascii", errors="strict").strip().lower() - if not SHA_RE.fullmatch(blob_sha): - raise RuntimeError(f"git rev-parse returned an invalid blob SHA for {path}") - candidates[path] = (content, blob_sha) + candidates[path] = (content, _git_blob_sha(repo_root, revision_sha, path)) return candidates +def _git_blob_sha(repo_root: pathlib.Path, revision_sha: str, path: str) -> str: + """Return one exact validated Git blob identity.""" + blob = _git(repo_root, "rev-parse", f"{revision_sha}:{path}") + blob_sha = blob.decode("ascii", errors="strict").strip().lower() + if not SHA_RE.fullmatch(blob_sha): + raise RuntimeError(f"git rev-parse returned an invalid blob SHA for {path}") + return blob_sha + + +def _changed_uv_lock_paths( + repo_root: pathlib.Path, + base_sha: str, + base_paths: set[str], + head_sha: str, + head_paths: set[str], +) -> set[str]: + """Return uv projects whose lock or sibling metadata changed at HEAD.""" + changed = base_paths ^ head_paths + for lock_path in sorted(base_paths & head_paths): + compared_paths = (lock_path, _uv_pyproject_path(lock_path)) + if any( + _git_blob_sha(repo_root, base_sha, path) + != _git_blob_sha(repo_root, head_sha, path) + for path in compared_paths + ): + changed.add(lock_path) + return changed + + +def _replace_changed_uv_inputs( + repo_root: pathlib.Path, + head_sha: str, + locks: list[tuple[str, bytes]], + vcs_manifest: list[dict[str, str]], + changed_paths: set[str], + head_paths: set[str], +) -> tuple[list[tuple[str, bytes]], list[dict[str, str]]]: + """Replace changed uv exports with complete exact-head registry/VCS inputs.""" + locks = [ + (source_path, content) + for source_path, content in locks + if source_path not in changed_paths + ] + vcs_by_repository = { + str(dependency["repository"]).casefold(): dependency + for dependency in vcs_manifest + if dependency.get("source") not in changed_paths + } + for lock_path in sorted(changed_paths & head_paths): + exported = _export_uv_lock(repo_root, head_sha, lock_path) + if exported is None: + continue + registry_content, vcs_dependencies = exported + if registry_content: + locks.append((lock_path, registry_content)) + for dependency in vcs_dependencies: + dependency = {**dependency, "source": lock_path} + repository_key = dependency["repository"].casefold() + previous = vcs_by_repository.get(repository_key) + if previous is not None and previous["commit"] != dependency["commit"]: + raise RuntimeError( + "Python uv locks pin one VCS repository to conflicting commits" + ) + vcs_by_repository[repository_key] = dependency + return sorted(locks, key=lambda item: item[0]), sorted( + vcs_by_repository.values(), + key=lambda dependency: dependency["repository"].casefold(), + ) + + def _select_python_locks( repo_root: pathlib.Path, base_sha: str, @@ -811,22 +889,39 @@ def materialize( resolved_repo = repo_root.resolve() entries = _git(resolved_repo, "ls-tree", "-r", "-z", "--full-tree", base_sha) - regular_paths = { - path for path, _candidate in _regular_base_blob_paths(entries) - } + regular_blobs = _regular_base_blob_paths(entries) + regular_paths = {path for path, _candidate in regular_blobs} + base_uv_paths = _regular_uv_lock_paths(regular_blobs, regular_paths) head_regular_paths: set[str] | None = None + head_uv_paths: set[str] = set() if head_sha is not None: head_entries = _git( resolved_repo, "ls-tree", "-r", "-z", "--full-tree", head_sha ) - head_regular_paths = { - path for path, _candidate in _regular_base_blob_paths(head_entries) - } + head_regular_blobs = _regular_base_blob_paths(head_entries) + head_regular_paths = {path for path, _candidate in head_regular_blobs} + head_uv_paths = _regular_uv_lock_paths(head_regular_blobs, head_regular_paths) locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) if head_sha is not None: locks = _select_python_locks( resolved_repo, base_sha, locks, head_sha ) + changed_uv_paths = _changed_uv_lock_paths( + resolved_repo, + base_sha, + base_uv_paths, + head_sha, + head_uv_paths, + ) + if changed_uv_paths: + locks, vcs_manifest = _replace_changed_uv_inputs( + resolved_repo, + head_sha, + locks, + vcs_manifest, + changed_uv_paths, + head_uv_paths, + ) manifest: list[dict[str, str]] = [] for index, (source_path, content) in enumerate(locks): generated_name = f"requirements-{index:03d}.txt" diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 448d4bb944..a7da0e59cb 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1107,6 +1107,206 @@ def test_uv_lock_with_empty_dependency_closure_materializes_nothing( assert materializer.materialize(repo, base_sha, tmp_path / "output") == [] +@pytest.mark.parametrize( + ("head_registry", "head_vcs"), + [ + ( + b"head-dep==2 --hash=sha256:" + b"b" * 64 + b"\n", + [], + ), + ( + b"", + [ + { + "package": "head-source", + "import_name": "head_source", + "repository": "head-repository", + "commit": "b" * 40, + } + ], + ), + ( + b"head-dep==2 --hash=sha256:" + b"b" * 64 + b"\n", + [ + { + "package": "head-source", + "import_name": "head_source", + "repository": "head-repository", + "commit": "b" * 40, + } + ], + ), + ], +) +def test_changed_uv_lock_replaces_base_registry_and_vcs_inputs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + head_registry: bytes, + head_vcs: list[dict[str, str]], +) -> None: + """A changed exact-head uv project replaces every base export component.""" + repo, base_sha = _uv_repo(tmp_path, with_pyproject=True) + (repo / "uv.lock").write_text("version = 2\n", encoding="utf-8") + git(repo, "add", "uv.lock") + git(repo, "commit", "-m", "head") + head_sha = git(repo, "rev-parse", "HEAD") + + base_registry = b"base-dep==1 --hash=sha256:" + b"a" * 64 + b"\n" + base_vcs = [ + { + "package": "base-source", + "import_name": "base_source", + "repository": "base-repository", + "commit": "a" * 40, + } + ] + + def export(_repo: Path, revision: str, _lock_path: str): + return ( + (base_registry, base_vcs) + if revision == base_sha + else (head_registry, head_vcs) + ) + + monkeypatch.setattr(materializer, "_export_uv_lock", export) + + output = tmp_path / "output" + manifest = materializer.materialize(repo, base_sha, output, head_sha=head_sha) + + expected_manifest = ( + [{"file": "requirements-000.txt", "source": "uv.lock"}] + if head_registry + else [] + ) + assert manifest == expected_manifest + if head_registry: + assert (output / "requirements-000.txt").read_bytes() == head_registry + assert json.loads((output / "vcs-manifest.json").read_text()) == [ + {**dependency, "source": "uv.lock"} for dependency in head_vcs + ] + + +def test_deleted_uv_lock_removes_base_registry_and_vcs_inputs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Deleting a uv project cannot leave its base dependency export installed.""" + repo, base_sha = _uv_repo(tmp_path, with_pyproject=True) + (repo / "uv.lock").unlink() + git(repo, "add", "-A") + git(repo, "commit", "-m", "delete uv lock") + head_sha = git(repo, "rev-parse", "HEAD") + base_registry = b"base-dep==1 --hash=sha256:" + b"a" * 64 + b"\n" + base_vcs = [ + { + "package": "base-source", + "import_name": "base_source", + "repository": "base-repository", + "commit": "a" * 40, + } + ] + monkeypatch.setattr( + materializer, + "_export_uv_lock", + lambda _repo, _revision, _path: (base_registry, base_vcs), + ) + + output = tmp_path / "output" + assert materializer.materialize(repo, base_sha, output, head_sha=head_sha) == [] + assert json.loads((output / "vcs-manifest.json").read_text()) == [] + + +def test_unchanged_uv_lock_preserves_base_export( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unchanged uv project stays base-bound when another file changes.""" + repo, base_sha = _uv_repo(tmp_path, with_pyproject=True) + (repo / "README.md").write_text("unrelated\n", encoding="utf-8") + git(repo, "add", "README.md") + git(repo, "commit", "-m", "unrelated head change") + head_sha = git(repo, "rev-parse", "HEAD") + base_registry = b"base-dep==1 --hash=sha256:" + b"a" * 64 + b"\n" + monkeypatch.setattr( + materializer, + "_export_uv_lock", + lambda _repo, _revision, _path: (base_registry, []), + ) + + output = tmp_path / "output" + assert materializer.materialize(repo, base_sha, output, head_sha=head_sha) == [ + {"file": "requirements-000.txt", "source": "uv.lock"} + ] + assert (output / "requirements-000.txt").read_bytes() == base_registry + + +def test_changed_uv_lock_with_empty_head_export_removes_base_inputs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A valid HEAD project with no third-party closure removes old inputs.""" + monkeypatch.setattr( + materializer, + "_export_uv_lock", + lambda _repo, _revision, _path: None, + ) + assert materializer._replace_changed_uv_inputs( + tmp_path, + "b" * 40, + [("uv.lock", b"base")], + [ + { + "package": "base-source", + "import_name": "base_source", + "repository": "base-repository", + "commit": "a" * 40, + "source": "uv.lock", + } + ], + {"uv.lock"}, + {"uv.lock"}, + ) == ([], []) + + +def test_changed_uv_lock_rejects_conflicting_replaced_vcs_revision( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A changed HEAD export cannot conflict with an unchanged source pin.""" + monkeypatch.setattr( + materializer, + "_export_uv_lock", + lambda _repo, _revision, _path: ( + b"", + [ + { + "package": "head-source", + "import_name": "head_source", + "repository": "shared-repository", + "commit": "b" * 40, + } + ], + ), + ) + with pytest.raises(RuntimeError, match="conflicting commits"): + materializer._replace_changed_uv_inputs( + tmp_path, + "b" * 40, + [], + [ + { + "package": "base-source", + "import_name": "base_source", + "repository": "shared-repository", + "commit": "a" * 40, + "source": "other/uv.lock", + } + ], + {"uv.lock"}, + {"uv.lock"}, + ) + + def _trusted_uv_archive( binary: bytes = b"verified-uv", *, From 366ffd9d34b0d42e3b28dce8e654c2d47daa6141 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 17:41:10 +0900 Subject: [PATCH 04/11] fix(coverage): preserve shared uv source ownership --- .../materialize_base_python_requirements.py | 21 ++--- ...st_materialize_base_python_requirements.py | 76 +++++++++++++++++++ 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 223dd643f6..c176e4aa86 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -626,7 +626,8 @@ def _base_python_inputs( regular_blobs = _regular_base_blob_paths(entries) regular_paths = {path for path, _candidate in regular_blobs} locks: list[tuple[str, bytes]] = [] - vcs_by_repository: dict[str, dict[str, str]] = {} + vcs_commits: dict[str, str] = {} + vcs_dependencies: list[dict[str, str]] = [] for path, candidate in regular_blobs: if _is_candidate_lock_path(candidate): content = _git(repo_root, "show", f"{base_sha}:{path}") @@ -637,26 +638,26 @@ def _base_python_inputs( continue exported = _export_uv_lock(repo_root, base_sha, path) if exported is not None: - registry_content, vcs_dependencies = exported + registry_content, exported_vcs_dependencies = exported if registry_content: locks.append((path, registry_content)) - for dependency in vcs_dependencies: + for dependency in exported_vcs_dependencies: dependency = {**dependency, "source": path} repository_key = dependency["repository"].casefold() - previous = vcs_by_repository.get(repository_key) - if ( - previous is not None - and previous["commit"] != dependency["commit"] - ): + previous_commit = vcs_commits.get(repository_key) + if previous_commit is not None and previous_commit != dependency[ + "commit" + ]: raise RuntimeError( "base uv locks pin one VCS repository " "to conflicting commits" ) - vcs_by_repository[repository_key] = dependency + vcs_commits[repository_key] = dependency["commit"] + vcs_dependencies.append(dependency) return ( sorted(locks, key=lambda item: item[0]), sorted( - vcs_by_repository.values(), + vcs_dependencies, key=lambda dependency: dependency["repository"].casefold(), ), ) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index a7da0e59cb..98aecf062c 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1307,6 +1307,82 @@ def test_changed_uv_lock_rejects_conflicting_replaced_vcs_revision( ) +@pytest.mark.parametrize( + ("head_dependencies", "expected_source", "expected_error"), + [ + ([], "first/uv.lock", None), + ( + [ + { + "package": "shared-source", + "import_name": "shared_source", + "repository": "shared-repository", + "commit": "b" * 40, + } + ], + None, + "conflicting commits", + ), + ], +) +def test_changed_uv_project_preserves_unaffected_shared_vcs_owner( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + head_dependencies: list[dict[str, str]], + expected_source: str | None, + expected_error: str | None, +) -> None: + """A collapsed VCS repository entry cannot hide an unchanged project owner.""" + repo = tmp_path / "repo" + repo.mkdir() + git(repo, "init") + git(repo, "config", "user.name", "Test") + git(repo, "config", "user.email", "test@example.invalid") + for project in ("first", "second"): + project_root = repo / project + project_root.mkdir() + (project_root / "uv.lock").write_text("version = 1\n", encoding="utf-8") + (project_root / "pyproject.toml").write_text( + f"[project]\nname = '{project}'\nversion = '0'\n", encoding="utf-8" + ) + git(repo, "add", ".") + git(repo, "commit", "-m", "base") + base_sha = git(repo, "rev-parse", "HEAD") + + (repo / "second" / "uv.lock").write_text("version = 2\n", encoding="utf-8") + git(repo, "add", "second/uv.lock") + git(repo, "commit", "-m", "change second uv lock") + head_sha = git(repo, "rev-parse", "HEAD") + shared_dependency = { + "package": "shared-source", + "import_name": "shared_source", + "repository": "shared-repository", + "commit": "a" * 40, + } + + def export(_repo: Path, revision: str, _lock_path: str): + return ( + (b"", [shared_dependency]) + if revision == base_sha + else (b"", head_dependencies) + ) + + monkeypatch.setattr(materializer, "_export_uv_lock", export) + + if expected_error is not None: + with pytest.raises(RuntimeError, match=expected_error): + materializer.materialize( + repo, base_sha, tmp_path / "output", head_sha=head_sha + ) + return + + output = tmp_path / "output" + assert materializer.materialize(repo, base_sha, output, head_sha=head_sha) == [] + assert json.loads((output / "vcs-manifest.json").read_text()) == [ + {**shared_dependency, "source": expected_source} + ] + + def _trusted_uv_archive( binary: bytes = b"verified-uv", *, From 2ef42b67637c43097401ddbd3c3ad5878b9dc4e0 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 18:15:27 +0900 Subject: [PATCH 05/11] docs(doctoring): include newly added VCS projects --- .../opencode-exact-vcs-dependency-evidence.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/doctoring/opencode-exact-vcs-dependency-evidence.md b/docs/doctoring/opencode-exact-vcs-dependency-evidence.md index 357c6b894a..05a709b632 100644 --- a/docs/doctoring/opencode-exact-vcs-dependency-evidence.md +++ b/docs/doctoring/opencode-exact-vcs-dependency-evidence.md @@ -3,9 +3,9 @@ ## Decision The OpenCode coverage image may expose a Python dependency directly from source -only when the validated base branch's frozen `uv.lock`, or a changed exact-head -`uv.lock` project selected for coverage, names an HTTPS GitHub repository owned -by `ContextualWisdomLab` and a full 40-character Git commit. Registry +only when the validated base branch's frozen `uv.lock`, or a changed or newly +added exact-head `uv.lock` project selected for coverage, names an HTTPS GitHub +repository owned by `ContextualWisdomLab` and a full 40-character Git commit. Registry dependencies remain exact-version, SHA-256-pinned `pip` installs. The trusted materializer separates those two dependency classes. Unchanged @@ -34,8 +34,9 @@ product's current-head tests passing. organization origin fail closed. - Duplicate references to one repository must resolve to one commit; conflicting revisions fail before the image build. -- Only metadata read from the validated base SHA or a changed project in the - exact validated HEAD can select a dependency. Pull request source cannot +- Only metadata read from the validated base SHA or a changed or newly added + project in the exact validated HEAD can select a dependency. Pull request + source cannot modify the networked image build inputs outside those immutable revisions. - Source dependencies are import-only. No `pip install`, PEP 517 backend, setup hook, or dependency lifecycle script runs while the network is available. From 727e7245876a62fcb3d0612f22993a7f5a57d232 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 30 Aug 2026 00:38:32 +0900 Subject: [PATCH 06/11] fix(coverage): preserve exact-head uv repairs --- CHANGELOG.md | 4 +- .../opencode-python-head-lock-trust.md | 4 +- .../trusted-uv-lock-materialization.md | 29 ++++++----- .../materialize_base_python_requirements.py | 28 ++++++++--- scripts/ci/pingora_edge_policy.py | 7 ++- ...st_materialize_base_python_requirements.py | 48 +++++++++++++++++++ tests/test_pingora_edge_policy.py | 14 +++++- 7 files changed, 109 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca773a1046..8f05a70959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,9 @@ Semantic Versioning where the repository publishes a release. `--only-binary=:all:`. - Re-export changed or newly added `uv.lock` projects from the exact HEAD with the existing frozen/offline exporter, while removing deleted projects' base - registry and VCS inputs and retaining unchanged projects at the base revision. + registry and VCS inputs and retaining unchanged projects at the base revision; + changed or deleted projects are inventoried before any base export so a stale + base project cannot block its exact-head repair. - Skip trusted base Python lock materialization for exact-head reviews with no Python source or dependency-manifest changes, while preserving the fail-closed wheel-only path when Python coverage is relevant. diff --git a/docs/doctoring/opencode-python-head-lock-trust.md b/docs/doctoring/opencode-python-head-lock-trust.md index 309c2454dd..0e26940146 100644 --- a/docs/doctoring/opencode-python-head-lock-trust.md +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -19,7 +19,9 @@ deleted or invalid content fails closed. A base lock that becomes unbounded fails closed. A changed or newly added `uv.lock` project with a regular sibling `pyproject.toml` is re-exported from the exact HEAD through the existing frozen, offline, checksum-validating exporter; deleted projects remove their base -registry and VCS inputs, and unchanged projects remain base-bound. +registry and VCS inputs, and unchanged projects remain base-bound. Base and HEAD +`uv.lock` inventories are completed before export, so a changed or deleted HEAD +project never requires its stale base export to succeed first. ## Root cause diff --git a/docs/doctoring/trusted-uv-lock-materialization.md b/docs/doctoring/trusted-uv-lock-materialization.md index 7a1f12b6ae..69028ff56d 100644 --- a/docs/doctoring/trusted-uv-lock-materialization.md +++ b/docs/doctoring/trusted-uv-lock-materialization.md @@ -19,13 +19,16 @@ The implementation therefore: same immutable revision; an absent sibling is an explicit orphan, while a read failure for an inventoried blob is fatal and cannot be misclassified as absence; -3. compares the base and current-head `uv.lock` plus sibling metadata blob IDs - before re-exporting a changed project; a deleted project removes its prior - registry and VCS entries, while a changed project uses the same isolated +3. inventories the base and current-head `uv.lock` projects plus sibling metadata + blob IDs before export; unchanged projects use the base revision, changed or + newly added projects use HEAD, and deleted projects remove their base registry + and VCS entries; +4. compares the base and current-head `uv.lock` plus sibling metadata blob IDs + before re-exporting a changed project; a changed project uses the same isolated exporter and exact registry/VCS validators against HEAD; -4. installs one process-wide urllib opener with an empty proxy map and a redirect +5. installs one process-wide urllib opener with an empty proxy map and a redirect handler that rejects every redirect before urllib creates a target request; -5. downloads one fixed official `uv` archive from the literal GitHub Releases +6. downloads one fixed official `uv` archive from the literal GitHub Releases HTTPS URL and accepts a response only when its parsed origin remains HTTPS on `github.com`, `release-assets.githubusercontent.com`, or `objects.githubusercontent.com` with the absent or explicit default port 443; @@ -35,22 +38,22 @@ The implementation therefore: because that vanity host now returns HTTP 403 for the pinned 0.12.1 archive (ContextualWisdomLab/.github#1109) while the GitHub Releases asset keeps the same SHA-256 digest; -6. verifies the bounded archive with a pinned SHA-256 digest before extraction; -7. accepts only the expected regular-file tar member within explicit size bounds; -8. writes the executable with mode `0755` and verifies that it reports the exact +7. verifies the bounded archive with a pinned SHA-256 digest before extraction; +8. accepts only the expected regular-file tar member within explicit size bounds; +9. writes the executable with mode `0755` and verifies that it reports the exact pinned `uv` version; -9. executes `uv export` with `--frozen`, `--offline`, `--no-cache`, +10. executes `uv export` with `--frozen`, `--offline`, `--no-cache`, `--no-progress`, `--color never`, `--no-emit-project`, and `--no-editable` in an isolated temporary project; -10. supplies a minimal environment with isolated home, temporary, cache, and +11. supplies a minimal environment with isolated home, temporary, cache, and configuration directories, disables dotenv loading and managed Python downloads, and does not inherit arbitrary runner variables; -11. keeps project metadata discovery enabled because the reconstructed +12. keeps project metadata discovery enabled because the reconstructed `pyproject.toml` is an authoritative input; `--no-config` is deliberately not used because uv documents that it disables `pyproject.toml` discovery; -12. rejects every nonempty export unless every logical line is an exact normalized +13. rejects every nonempty export unless every logical line is an exact normalized package `==` pin followed only by complete SHA-256 hashes; and -13. exposes only generated requirements files and a source manifest to the later +14. exposes only generated requirements files and a source manifest to the later networkless coverage environment. ## Standards and current-tool rationale diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index c176e4aa86..e99b23d3f1 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -616,12 +616,16 @@ def _regular_base_blob_paths(entries: bytes) -> list[tuple[str, pathlib.PurePosi def _base_python_inputs( - repo_root: pathlib.Path, base_sha: str + repo_root: pathlib.Path, + base_sha: str, + *, + excluded_uv_paths: set[str] | None = None, ) -> tuple[list[tuple[str, bytes]], list[dict[str, str]]]: - """Return hash locks and exact VCS sources from one validated base commit.""" + """Return selected hash locks and exact VCS sources from one base commit.""" if not SHA_RE.fullmatch(base_sha): raise ValueError("base SHA must be exactly 40 hexadecimal characters") + excluded_uv_paths = excluded_uv_paths or set() entries = _git(repo_root, "ls-tree", "-r", "-z", "--full-tree", base_sha) regular_blobs = _regular_base_blob_paths(entries) regular_paths = {path for path, _candidate in regular_blobs} @@ -634,6 +638,8 @@ def _base_python_inputs( if _is_hash_pinned(content): locks.append((path, content)) elif candidate.name == "uv.lock": + if path in excluded_uv_paths: + continue if _uv_pyproject_path(path) not in regular_paths: continue exported = _export_uv_lock(repo_root, base_sha, path) @@ -895,6 +901,7 @@ def materialize( base_uv_paths = _regular_uv_lock_paths(regular_blobs, regular_paths) head_regular_paths: set[str] | None = None head_uv_paths: set[str] = set() + changed_uv_paths: set[str] = set() if head_sha is not None: head_entries = _git( resolved_repo, "ls-tree", "-r", "-z", "--full-tree", head_sha @@ -902,11 +909,6 @@ def materialize( head_regular_blobs = _regular_base_blob_paths(head_entries) head_regular_paths = {path for path, _candidate in head_regular_blobs} head_uv_paths = _regular_uv_lock_paths(head_regular_blobs, head_regular_paths) - locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) - if head_sha is not None: - locks = _select_python_locks( - resolved_repo, base_sha, locks, head_sha - ) changed_uv_paths = _changed_uv_lock_paths( resolved_repo, base_sha, @@ -914,6 +916,18 @@ def materialize( head_sha, head_uv_paths, ) + if changed_uv_paths: + locks, vcs_manifest = _base_python_inputs( + resolved_repo, + base_sha, + excluded_uv_paths=changed_uv_paths, + ) + else: + locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) + if head_sha is not None: + locks = _select_python_locks( + resolved_repo, base_sha, locks, head_sha + ) if changed_uv_paths: locks, vcs_manifest = _replace_changed_uv_inputs( resolved_repo, diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 706fe69fc1..05eaaa734d 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -240,11 +240,14 @@ def _load_changed_files(api_url: str, repository: str, pull_request: int, token: """Load every changed-file page while enforcing shape and pagination bounds.""" files: list[ChangedFile] = [] - for page in range(1, 32): + page = 1 + while True: url = f"{api_url}/repos/{repository}/pulls/{pull_request}/files?per_page=100&page={page}" payload = opener(url, token) if not isinstance(payload, list): raise PolicyError("GitHub changed-file evidence is not a JSON array") + if page == 31 and len(payload) >= 100: + raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") for item in payload: if not isinstance(item, Mapping): raise PolicyError("GitHub changed-file entry is not an object") @@ -271,7 +274,7 @@ def _load_changed_files(api_url: str, repository: str, pull_request: int, token: raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") if len(payload) < 100: return tuple(files) - raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") + page += 1 def _load_file_content(api_url: str, repository: str, path: str, head_sha: str, token: str, opener: OpenJson) -> str: diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 98aecf062c..1b4966ac32 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1186,6 +1186,32 @@ def export(_repo: Path, revision: str, _lock_path: str): ] +def test_changed_uv_lock_does_not_require_a_base_export( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A repaired HEAD uv project replaces a base project that no longer exports.""" + repo, base_sha = _uv_repo(tmp_path, with_pyproject=True) + (repo / "uv.lock").write_text("version = 2\n", encoding="utf-8") + git(repo, "add", "uv.lock") + git(repo, "commit", "-m", "repair uv lock") + head_sha = git(repo, "rev-parse", "HEAD") + head_registry = b"head-dep==2 --hash=sha256:" + b"b" * 64 + b"\n" + + def export(_repo: Path, revision: str, _lock_path: str): + if revision == base_sha: + raise RuntimeError("base uv lock is stale") + return head_registry, [] + + monkeypatch.setattr(materializer, "_export_uv_lock", export) + + output = tmp_path / "output" + assert materializer.materialize(repo, base_sha, output, head_sha=head_sha) == [ + {"file": "requirements-000.txt", "source": "uv.lock"} + ] + assert (output / "requirements-000.txt").read_bytes() == head_registry + + def test_deleted_uv_lock_removes_base_registry_and_vcs_inputs( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -1216,6 +1242,28 @@ def test_deleted_uv_lock_removes_base_registry_and_vcs_inputs( assert json.loads((output / "vcs-manifest.json").read_text()) == [] +def test_deleted_uv_lock_does_not_require_a_base_export( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Deleting a broken base uv project omits it without exporting the stale lock.""" + repo, base_sha = _uv_repo(tmp_path, with_pyproject=True) + (repo / "uv.lock").unlink() + git(repo, "add", "-A") + git(repo, "commit", "-m", "delete stale uv lock") + head_sha = git(repo, "rev-parse", "HEAD") + + def fail_base_export(_repo: Path, revision: str, _lock_path: str): + assert revision == base_sha + raise RuntimeError("base uv lock is stale") + + monkeypatch.setattr(materializer, "_export_uv_lock", fail_base_export) + + output = tmp_path / "output" + assert materializer.materialize(repo, base_sha, output, head_sha=head_sha) == [] + assert json.loads((output / "vcs-manifest.json").read_text()) == [] + + def test_unchanged_uv_lock_preserves_base_export( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index 584e540749..d3cb528f23 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -290,7 +290,19 @@ def test_changed_file_evidence_shape_is_fail_closed(payload: object, message: st def test_changed_file_pagination_bound_is_fail_closed() -> None: """More than 3,000 changed files cannot silently truncate policy evidence.""" - page = [{"filename": f"f-{index}", "status": "modified", "patch": ""} for index in range(100)] + oversized_page = [ + {"filename": f"f-{index}", "status": "modified", "patch": ""} + for index in range(3_001) + ] + with pytest.raises(policy.PolicyError, match="3,000"): + policy._load_changed_files( + "api", "a/b", 1, "x", lambda _url, _token: oversized_page + ) + + page = [ + {"filename": f"f-{index}", "status": "modified", "patch": ""} + for index in range(100) + ] with pytest.raises(policy.PolicyError, match="3,000"): policy._load_changed_files("api", "a/b", 1, "x", lambda _url, _token: page) From 8c0869a5392fb6244b1ebc6eb47adaa2405948c4 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 30 Aug 2026 00:47:56 +0900 Subject: [PATCH 07/11] fix(coverage): classify exact-head include changes --- .github/workflows/opencode-review-dispatch.yml | 14 +++++++++++--- CHANGELOG.md | 3 +++ .../doctoring/opencode-python-head-lock-trust.md | 8 ++++++-- .../ci/materialize_base_python_requirements.py | 4 ++++ .../test_materialize_base_python_requirements.py | 16 ++++++++++++++++ tests/test_opencode_agent_contract.py | 7 ++++++- ...test_pr_review_autofix_nvidia_nim_contract.py | 2 +- 7 files changed, 47 insertions(+), 7 deletions(-) diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 2f188a8d05..552a81bfa1 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -649,9 +649,17 @@ jobs: break ;; esac - # Any text file below a requirements directory may be a bounded - # include target of a base lock, so retain materialization - # conservatively instead of relying on path depth alone. + # Any tracked .txt file may be a bounded include target of a base + # lock, so retain materialization conservatively instead of + # relying on path depth alone. + case "$changed_basename" in + *.txt) + python_coverage_required=1 + break + ;; + esac + # Includes with the other supported suffix remain recognized + # below requirements directories. case "/$changed_path/" in */requirements/*) case "$changed_basename" in diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f05a70959..c43c20659c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ Semantic Versioning where the repository publishes a release. registry and VCS inputs and retaining unchanged projects at the base revision; changed or deleted projects are inventoried before any base export so a stale base project cannot block its exact-head repair. +- Keep exact-head Python coverage materialization enabled for every tracked + `.txt` change because a bounded include cannot be identified safely from its + path, and reject malformed base or HEAD SHAs before any Git read. - Skip trusted base Python lock materialization for exact-head reviews with no Python source or dependency-manifest changes, while preserving the fail-closed wheel-only path when Python coverage is relevant. diff --git a/docs/doctoring/opencode-python-head-lock-trust.md b/docs/doctoring/opencode-python-head-lock-trust.md index 0e26940146..82f6b878bc 100644 --- a/docs/doctoring/opencode-python-head-lock-trust.md +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -23,6 +23,10 @@ registry and VCS inputs, and unchanged projects remain base-bound. Base and HEAD `uv.lock` inventories are completed before export, so a changed or deleted HEAD project never requires its stale base export to succeed first. +The workflow keeps this materialization enabled for every tracked `.txt` change: +the path alone cannot prove that a file is not a bounded include target. Content +validation still decides whether that candidate can enter the trusted image. + ## Root cause The coverage image was built only from `PR_BASE_SHA` Python locks. A legitimate @@ -34,8 +38,8 @@ validating and recording changed HEAD locks; Python had no equivalent path. ## Security boundary -The central workflow still fetches and validates the base and head revisions -before materialization. Only a regular candidate lock from the exact HEAD is +The central workflow and materializer validate the exact base and head revisions +before any Git tree read. Only a regular candidate lock from the exact HEAD is read, and only a flat SHA-256-pinned file can replace a base lock. Changed `uv.lock` projects reuse the same isolated exporter against exact HEAD `uv.lock` and sibling metadata, then apply the established registry hash and diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index e99b23d3f1..a05e284e74 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -890,6 +890,10 @@ def materialize( head_sha: str | None = None, ) -> list[dict[str, str]]: """Write trusted base locks and safe exact-head lock replacements.""" + if not SHA_RE.fullmatch(base_sha): + raise ValueError("base SHA must be exactly 40 hexadecimal characters") + if head_sha is not None and not SHA_RE.fullmatch(head_sha): + raise ValueError("head SHA must be exactly 40 hexadecimal characters") 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) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 1b4966ac32..6852a89cb3 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -175,6 +175,22 @@ def test_candidate_lock_blobs_rejects_invalid_revision_sha(tmp_path: Path) -> No materializer._candidate_lock_blobs(tmp_path, "HEAD") +def test_materialize_rejects_invalid_head_sha_before_git_access( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Materialization validates the exact HEAD revision before reading Git.""" + monkeypatch.setattr( + materializer, + "_git", + lambda *_args: pytest.fail("invalid HEAD must be rejected before Git access"), + ) + + with pytest.raises(ValueError, match="head SHA must be exactly 40"): + materializer.materialize( + tmp_path, "a" * 40, tmp_path / "output", head_sha="HEAD" + ) + + def test_candidate_lock_blobs_skips_non_lock_blobs( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index fd97dc6d8c..7d5d399fc2 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -608,7 +608,11 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert "requirements.lock|requirements*.txt|requirements*.in" in measure_step assert "*/requirements/*" in measure_step assert "materialize_base_javascript_packages.py" in measure_step - assert '--head-sha "$PR_HEAD_SHA"' in measure_step + python_materializer = measure_step.split( + "materialize_base_python_requirements.py", 1 + )[1].split("--output-dir", 1)[0] + assert '--base-sha "$PR_BASE_SHA"' in python_materializer + assert '--head-sha "$PR_HEAD_SHA"' in python_materializer assert "COPY base-javascript-packages /tmp/base-javascript-packages" in measure_step assert ( "install -m 0444 /tmp/base-javascript-packages/manifest.json" @@ -901,6 +905,7 @@ def test_opencode_python_lock_classifier_covers_materializer_paths(tmp_path: Pat ("requirements/docs/notes.txt", "1", False), ("services/requirements/docs/notes.txt", "1", False), ("services/requirements/requirements-extra.txt", "1", False), + ("locks/child.txt", "1", False), ("deleted.py", "1", True), ("README.md", "0", False), ("package-lock.json", "0", False), diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index b4fcd1f74d..3d09720127 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "2f188a8d054bb877c6260738ee68b7660258bf6f" +REVIEW_DISPATCH_BLOB_SHA = "552a81bfa174e3d0b64a5a332c10b4428d54e9f0" def _workflow_text(path: Path) -> str: From f018f1dcbfeed8a39a03f92785f8160f033d9541 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 18:22:45 -0700 Subject: [PATCH 08/11] fix: preserve VCS owners and bound Pingora evidence --- .../materialize_base_python_requirements.py | 54 +++++++++----- scripts/ci/pingora_edge_policy.py | 41 ++++++++++- ...st_materialize_base_python_requirements.py | 56 +++++++++++++++ tests/test_pingora_edge_policy.py | 70 +++++++++++++++++++ tests/test_uv_export_isolation_contract.py | 31 ++++++++ 5 files changed, 233 insertions(+), 19 deletions(-) diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index a05e284e74..f9b1d7cb2c 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -296,7 +296,8 @@ def _is_fully_hash_pinned_export(content: bytes) -> bool: def _partition_uv_export(content: bytes) -> tuple[bytes, list[dict[str, str]]]: """Separate registry hash pins from exact organization VCS source pins.""" registry_requirements: list[str] = [] - vcs_by_repository: dict[str, dict[str, str]] = {} + vcs_dependencies: list[dict[str, str]] = [] + vcs_commits: dict[str, str] = {} for line in _requirement_lines(content): if _is_fully_hash_pinned_requirement(line): registry_requirements.append(line) @@ -313,10 +314,11 @@ def _partition_uv_export(content: bytes) -> tuple[bytes, list[dict[str, str]]]: "commit": match.group("commit").lower(), } repository_key = dependency["repository"].casefold() - previous = vcs_by_repository.get(repository_key) - if previous is not None and previous["commit"] != dependency["commit"]: + previous_commit = vcs_commits.get(repository_key) + if previous_commit is not None and previous_commit != dependency["commit"]: raise ValueError("uv export pins one VCS repository to conflicting commits") - vcs_by_repository[repository_key] = dependency + vcs_commits[repository_key] = dependency["commit"] + vcs_dependencies.append(dependency) registry_content = ( ("\n".join(registry_requirements) + "\n").encode("utf-8") @@ -324,8 +326,12 @@ def _partition_uv_export(content: bytes) -> tuple[bytes, list[dict[str, str]]]: else b"" ) return registry_content, sorted( - vcs_by_repository.values(), - key=lambda dependency: dependency["repository"].casefold(), + vcs_dependencies, + key=lambda dependency: ( + dependency["repository"].casefold(), + dependency["package"].casefold(), + dependency["import_name"].casefold(), + ), ) @@ -745,30 +751,46 @@ def _replace_changed_uv_inputs( for source_path, content in locks if source_path not in changed_paths ] - vcs_by_repository = { - str(dependency["repository"]).casefold(): dependency + vcs_dependencies = [ + dependency for dependency in vcs_manifest if dependency.get("source") not in changed_paths - } + ] + vcs_commits: dict[str, str] = {} + for dependency in vcs_dependencies: + repository_key = str(dependency["repository"]).casefold() + commit = str(dependency["commit"]) + previous_commit = vcs_commits.get(repository_key) + if previous_commit is not None and previous_commit != commit: + raise RuntimeError( + "Python uv locks pin one VCS repository to conflicting commits" + ) + vcs_commits[repository_key] = commit for lock_path in sorted(changed_paths & head_paths): exported = _export_uv_lock(repo_root, head_sha, lock_path) if exported is None: continue - registry_content, vcs_dependencies = exported + registry_content, exported_vcs_dependencies = exported if registry_content: locks.append((lock_path, registry_content)) - for dependency in vcs_dependencies: + for dependency in exported_vcs_dependencies: dependency = {**dependency, "source": lock_path} repository_key = dependency["repository"].casefold() - previous = vcs_by_repository.get(repository_key) - if previous is not None and previous["commit"] != dependency["commit"]: + previous_commit = vcs_commits.get(repository_key) + if previous_commit is not None and previous_commit != dependency["commit"]: raise RuntimeError( "Python uv locks pin one VCS repository to conflicting commits" ) - vcs_by_repository[repository_key] = dependency + vcs_commits[repository_key] = dependency["commit"] + vcs_dependencies.append(dependency) return sorted(locks, key=lambda item: item[0]), sorted( - vcs_by_repository.values(), - key=lambda dependency: dependency["repository"].casefold(), + vcs_dependencies, + key=lambda dependency: ( + dependency["repository"].casefold(), + dependency.get("source", ""), + dependency.get("package", "").casefold(), + dependency.get("import_name", "").casefold(), + ), ) diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 05eaaa734d..27a5a1b68c 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -24,6 +24,8 @@ MAX_FILE_BYTES = 1_048_576 MAX_RESPONSE_BYTES = 16_777_216 +MAX_CONTENT_REQUESTS = 256 +MAX_TOTAL_CONTENT_BYTES = 16_777_216 REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") SHA_RE = re.compile(r"^[0-9a-f]{40}$") GITHUB_API_ORIGIN = "https://api.github.com" @@ -277,7 +279,16 @@ def _load_changed_files(api_url: str, repository: str, pull_request: int, token: page += 1 -def _load_file_content(api_url: str, repository: str, path: str, head_sha: str, token: str, opener: OpenJson) -> str: +def _load_file_content( + api_url: str, + repository: str, + path: str, + head_sha: str, + token: str, + opener: OpenJson, + *, + max_bytes: int = MAX_FILE_BYTES, +) -> str: """Load one final head file as bounded UTF-8 text from the Contents API.""" encoded_path = quote(path, safe="/") @@ -289,8 +300,15 @@ def _load_file_content(api_url: str, repository: str, path: str, head_sha: str, raise PolicyError(f"GitHub content evidence for {path} is not a regular base64 file") encoded = payload.get("content") declared_size = payload.get("size") - if not isinstance(encoded, str) or not isinstance(declared_size, int) or declared_size < 0 or declared_size > MAX_FILE_BYTES: + if ( + not isinstance(encoded, str) + or not isinstance(declared_size, int) + or declared_size < 0 + or declared_size > MAX_FILE_BYTES + ): raise PolicyError(f"GitHub content evidence for {path} exceeds or violates the size contract") + if declared_size > max_bytes: + raise PolicyError("GitHub policy evidence exceeded the content byte budget") try: raw = base64.b64decode("".join(encoded.split()), validate=True) except (ValueError, TypeError) as exc: @@ -344,10 +362,27 @@ def evaluate_pull_request( raise PolicyError("GITHUB_TOKEN is required for policy evidence") changed_files = _load_changed_files(api_url.rstrip("/"), repository, pull_request, token, opener) violations: list[Violation] = [] + content_requests = 0 + total_content_bytes = 0 for changed in changed_files: if not _needs_content_scan(changed): continue - content = _load_file_content(api_url.rstrip("/"), repository, changed.path, head_sha, token, opener) + if content_requests >= MAX_CONTENT_REQUESTS: + raise PolicyError("GitHub policy evidence exceeded the content request budget") + remaining_bytes = MAX_TOTAL_CONTENT_BYTES - total_content_bytes + if remaining_bytes <= 0: + raise PolicyError("GitHub policy evidence exceeded the content byte budget") + content = _load_file_content( + api_url.rstrip("/"), + repository, + changed.path, + head_sha, + token, + opener, + max_bytes=remaining_bytes, + ) + content_requests += 1 + total_content_bytes += len(content.encode("utf-8")) violations.extend(scan_content(changed.path, content)) return tuple(violations) diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 6852a89cb3..76ee11df1e 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -1371,6 +1371,62 @@ def test_changed_uv_lock_rejects_conflicting_replaced_vcs_revision( ) +def test_changed_uv_inputs_preserve_distinct_shared_repository_owners( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Replacing a uv project cannot collapse distinct owners of one revision.""" + commit = "a" * 40 + owners = [ + { + "package": package, + "import_name": package.replace("-", "_"), + "repository": "shared-repository", + "commit": commit, + } + for package in ("first-owner", "second-owner") + ] + monkeypatch.setattr( + materializer, + "_export_uv_lock", + lambda _repo, _revision, _path: (b"", owners), + ) + + assert materializer._replace_changed_uv_inputs( + tmp_path, + "b" * 40, + [], + [], + {"uv.lock"}, + {"uv.lock"}, + ) == ([], [{**owner, "source": "uv.lock"} for owner in owners]) + + +def test_changed_uv_inputs_reject_conflicting_retained_repository_revisions( + tmp_path: Path, +) -> None: + """A pre-existing manifest conflict cannot survive an unrelated replacement.""" + with pytest.raises(RuntimeError, match="conflicting commits"): + materializer._replace_changed_uv_inputs( + tmp_path, + "b" * 40, + [], + [ + { + "repository": "shared-repository", + "commit": commit, + "source": source, + } + for commit, source in ( + ("a" * 40, "first/uv.lock"), + ("b" * 40, "second/uv.lock"), + ) + ], + {"third/uv.lock"}, + set(), + ) + + @pytest.mark.parametrize( ("head_dependencies", "expected_source", "expected_error"), [ diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index d3cb528f23..c78f94dd77 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -232,6 +232,76 @@ def opener(url: str, _token: str) -> object: assert [item.rule for item in result] == ["nginx_container_image"] +def test_evaluate_pull_request_enforces_cumulative_content_request_budget( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A broad diff stops before issuing a content request beyond the budget.""" + monkeypatch.setattr(policy, "MAX_CONTENT_REQUESTS", 2) + content_calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + if "/pulls/10/files" in url: + return [ + { + "filename": f"deploy/runtime-{index}.yaml", + "status": "modified", + "patch": "+image: app", + } + for index in range(3) + ] + content_calls.append(url) + return encoded_file("image: cwl-pingora-proxy:0.1.0\n") + + with pytest.raises(policy.PolicyError, match="content request budget"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=10, + head_sha="c" * 40, + event_action="opened", + token="token", + opener=opener, + ) + + assert len(content_calls) == 2 + + +@pytest.mark.parametrize("byte_budget", [6, 8]) +def test_evaluate_pull_request_enforces_cumulative_content_byte_budget( + monkeypatch: pytest.MonkeyPatch, + byte_budget: int, +) -> None: + """Decoded evidence stops before a file would exceed the aggregate budget.""" + monkeypatch.setattr(policy, "MAX_TOTAL_CONTENT_BYTES", byte_budget) + content_calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + if "/pulls/11/files" in url: + return [ + { + "filename": f"deploy/runtime-{index}.yaml", + "status": "modified", + "patch": "+image: app", + } + for index in range(3) + ] + content_calls.append(url) + return encoded_file("edge") + + with pytest.raises(policy.PolicyError, match="content byte budget"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=11, + head_sha="d" * 40, + event_action="opened", + token="token", + opener=opener, + ) + + assert len(content_calls) == 2 + + def test_closed_event_skips_without_credentials_or_identity_validation() -> None: """Closed-event cleanup remains a no-op for the required-workflow context.""" diff --git a/tests/test_uv_export_isolation_contract.py b/tests/test_uv_export_isolation_contract.py index 76b72fdc7f..8bd8634075 100644 --- a/tests/test_uv_export_isolation_contract.py +++ b/tests/test_uv_export_isolation_contract.py @@ -149,6 +149,37 @@ def test_uv_export_rejects_conflicting_commits_for_one_repository() -> None: ) +def test_uv_export_preserves_distinct_owners_from_one_repository_revision() -> None: + """Distinct package owners sharing one immutable repository remain auditable.""" + commit = "a" * 40 + + registry, vcs_sources = materializer._partition_uv_export( + ( + "first-owner @ git+https://github.com/ContextualWisdomLab/demo.git@" + + commit + + "\nsecond-owner @ git+https://github.com/ContextualWisdomLab/Demo.git@" + + commit + + "\n" + ).encode() + ) + + assert registry == b"" + assert vcs_sources == [ + { + "package": "first-owner", + "import_name": "first_owner", + "repository": "demo", + "commit": commit, + }, + { + "package": "second-owner", + "import_name": "second_owner", + "repository": "Demo", + "commit": commit, + }, + ] + + def test_tracked_pyproject_read_failure_is_not_misclassified_as_orphan( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 4db20006fa5aa167c97e3c9b6365904f8283533d Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 18:31:35 -0700 Subject: [PATCH 09/11] fix: accept empty changed dependency locks --- .../materialize_base_python_requirements.py | 3 +++ ...st_materialize_base_python_requirements.py | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index f9b1d7cb2c..05c3f1066f 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -822,6 +822,9 @@ def _select_python_locks( base_blob_sha = base_blob.decode("ascii", errors="strict").strip().lower() if base_blob_sha == head_blob: continue + if not _requirement_lines(content): + selected.pop(source_path, None) + continue if _is_flat_materializable_lock(content): selected[source_path] = content continue diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index 76ee11df1e..cd786f18b3 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -294,6 +294,29 @@ def test_select_python_locks_handles_new_removed_and_unchanged_head_locks( ) == [("requirements-same.txt", same_content)] +def test_changed_python_lock_accepts_an_empty_dependency_closure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Removing every dependency drops the stale base lock without publishing one.""" + old_content = b"old==1 --hash=sha256:" + b"a" * 64 + b"\n" + monkeypatch.setattr( + materializer, + "_candidate_lock_blobs", + lambda _repo, _head: { + "requirements.txt": (b"# Dependencies intentionally removed.\n", "b" * 40) + }, + ) + monkeypatch.setattr(materializer, "_git", lambda *_args: b"a" * 40) + + assert materializer._select_python_locks( + tmp_path, + "a" * 40, + [("requirements.txt", old_content)], + "b" * 40, + ) == [] + + def test_materializes_exact_vcs_sources_in_a_separate_manifest( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From ca084e7c12066f54b86754c1548baec5ea95a5b3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 29 Aug 2026 18:39:47 -0700 Subject: [PATCH 10/11] fix(coverage): reject malformed empty head locks --- .../materialize_base_python_requirements.py | 6 +++++ ...st_materialize_base_python_requirements.py | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/scripts/ci/materialize_base_python_requirements.py b/scripts/ci/materialize_base_python_requirements.py index 05c3f1066f..c4df56d1d2 100755 --- a/scripts/ci/materialize_base_python_requirements.py +++ b/scripts/ci/materialize_base_python_requirements.py @@ -822,6 +822,12 @@ def _select_python_locks( base_blob_sha = base_blob.decode("ascii", errors="strict").strip().lower() if base_blob_sha == head_blob: continue + try: + content.decode("utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise ValueError( + f"current-head Python lock {source_path} is not valid UTF-8" + ) from exc if not _requirement_lines(content): selected.pop(source_path, None) continue diff --git a/tests/test_materialize_base_python_requirements.py b/tests/test_materialize_base_python_requirements.py index cd786f18b3..6e5b24c8ba 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -317,6 +317,31 @@ def test_changed_python_lock_accepts_an_empty_dependency_closure( ) == [] +def test_changed_python_lock_rejects_malformed_empty_dependency_closure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Undecodable bytes cannot impersonate an intentional empty closure.""" + old_content = b"old==1 --hash=sha256:" + b"a" * 64 + b"\n" + monkeypatch.setattr( + materializer, + "_candidate_lock_blobs", + lambda _repo, _head: {"requirements.txt": (b"\xff\xfe", "b" * 40)}, + ) + monkeypatch.setattr(materializer, "_git", lambda *_args: b"a" * 40) + + with pytest.raises( + ValueError, + match="current-head Python lock requirements.txt is not valid UTF-8", + ): + materializer._select_python_locks( + tmp_path, + "a" * 40, + [("requirements.txt", old_content)], + "b" * 40, + ) + + def test_materializes_exact_vcs_sources_in_a_separate_manifest( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 68a51d7e49c79c32878538c7917a91f95c14ebe2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 14:08:33 +0000 Subject: [PATCH 11/11] fix(pingora-edge-policy): make PDF verification share the content evidence budget Devin Review confirmed: evaluate_pull_request applied MAX_CONTENT_REQUESTS / MAX_TOTAL_CONTENT_BYTES only to the ordinary _load_file_content scanning path. _pdf_evidence_confirms_binary made its own separate Contents API request and could read up to MAX_FILE_BYTES (1 MiB) without consulting or updating either aggregate counter, so a pull request with many patchless documentation PDFs could exhaust the required check's GitHub API quota and wall-clock budget despite the budget mechanism existing specifically to prevent that. _pdf_evidence_confirms_binary now takes the same max_bytes remaining-budget parameter _load_file_content already uses, and returns the number of raw bytes actually decoded (zero for the oversized-PDF exemption, since no content is ever fetched in that case). A new _reserve_content_budget helper centralizes the request/byte budget check so both the PDF-verification path and the ordinary scan path share identical accounting and both fail closed consistently once either budget is spent. The oversized-PDF exemption itself is preserved -- only its resource cost now counts against the shared budget. Added regression tests: many small patchless PDFs collectively exceeding MAX_TOTAL_CONTENT_BYTES, a set of PDFs crossing MAX_CONTENT_REQUESTS, and the oversized-PDF exemption itself consuming one request from the budget. 100% statement/branch coverage and 100% docstring coverage retained. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX --- scripts/ci/pingora_edge_policy.py | 51 +++++++++++--- tests/test_pingora_edge_policy.py | 112 ++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 11 deletions(-) diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index c5ea89a4d6..05b23baca4 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -446,7 +446,8 @@ def _pdf_evidence_confirms_binary( head_sha: str, token: str, opener: OpenJson, -) -> bool: + max_bytes: int, +) -> tuple[bool, int]: """Return whether a claimed binary documentation PDF is genuinely binary. A missing diff ``patch`` alone is not proof of binary content: GitHub @@ -461,13 +462,21 @@ def _pdf_evidence_confirms_binary( malformed API response, corrupt base64, a declared size that does not match the decoded bytes) propagates and fails the whole check closed, same as for any other file that needs scanning. + + ``max_bytes`` bounds this read against the caller's remaining aggregate + content-byte budget, exactly like ``_load_file_content``'s own + ``max_bytes``, so a PDF-verification read cannot itself exhaust + resources the shared budget exists to cap. The returned byte count is + the number of raw bytes actually decoded (zero when the size-exceeded + exemption fires without ever reading content), so the caller can charge + it against the same aggregate counter the ordinary scan path uses. """ try: - raw = _load_raw_file_bytes(api_url, repository, changed.path, head_sha, token, opener) + raw = _load_raw_file_bytes(api_url, repository, changed.path, head_sha, token, opener, max_bytes=max_bytes) except ContentSizeExceededError: - return True - return raw.startswith(_PDF_MAGIC_PREFIX) + return True, 0 + return raw.startswith(_PDF_MAGIC_PREFIX), len(raw) def _needs_content_scan(changed: ChangedFile) -> bool: @@ -495,6 +504,25 @@ def _needs_content_scan(changed: ChangedFile) -> bool: return "nginx" in changed.patch.lower() +def _reserve_content_budget(content_requests: int, total_content_bytes: int) -> int: + """Raise a typed ``PolicyError`` if either aggregate evidence budget is spent. + + Returns the bytes remaining under ``MAX_TOTAL_CONTENT_BYTES`` so the + caller can bound its next Contents API read. Both PDF binary-content + verification and ordinary text scanning call this before making a + request, so neither path can exhaust the required check's GitHub API + quota or wall-clock budget on its own -- a partially scanned pull + request fails closed rather than reporting a false pass. + """ + + if content_requests >= MAX_CONTENT_REQUESTS: + raise PolicyError("GitHub policy evidence exceeded the content request budget") + remaining_bytes = MAX_TOTAL_CONTENT_BYTES - total_content_bytes + if remaining_bytes <= 0: + raise PolicyError("GitHub policy evidence exceeded the content byte budget") + return remaining_bytes + + def evaluate_pull_request( *, api_url: str, @@ -532,22 +560,23 @@ def evaluate_pull_request( # removed file has no head content to fetch at all -- _needs_content_scan # already special-cases this the same way for every other file. if changed.status != "removed" and _is_binary_documentation_pdf(changed): - if _pdf_evidence_confirms_binary( + remaining_pdf_bytes = _reserve_content_budget(content_requests, total_content_bytes) + confirmed_binary, pdf_bytes_read = _pdf_evidence_confirms_binary( changed, api_url=api_url.rstrip("/"), repository=repository, head_sha=head_sha, token=token, opener=opener, - ): + max_bytes=remaining_pdf_bytes, + ) + content_requests += 1 + total_content_bytes += pdf_bytes_read + if confirmed_binary: continue elif not _needs_content_scan(changed): continue - if content_requests >= MAX_CONTENT_REQUESTS: - raise PolicyError("GitHub policy evidence exceeded the content request budget") - remaining_bytes = MAX_TOTAL_CONTENT_BYTES - total_content_bytes - if remaining_bytes <= 0: - raise PolicyError("GitHub policy evidence exceeded the content byte budget") + remaining_bytes = _reserve_content_budget(content_requests, total_content_bytes) content = _load_file_content( api_url.rstrip("/"), repository, diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index a4b4c7bc65..12d757423d 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -480,6 +480,118 @@ def opener(url: str, _token: str) -> object: assert result == () +def test_evaluate_pull_request_enforces_byte_budget_across_pdf_verification_reads( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Many patchless documentation PDFs must share the aggregate byte budget. + + Regression coverage for Devin Review's finding that + ``_pdf_evidence_confirms_binary`` made its own separate Contents API + read outside both aggregate counters, so many one-megabyte + documentation PDFs collected in one pull request could exhaust the + required check's resources despite the budget mechanism existing + specifically to prevent that. + """ + pdf_body = "%PDF-1.7\nresearch paper body\n" + monkeypatch.setattr(policy, "MAX_TOTAL_CONTENT_BYTES", len(pdf_body.encode())) + content_calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + if "/pulls/20/files" in url: + return [ + {"filename": f"docs/papers/paper-{index}.pdf", "status": "added"} + for index in range(3) + ] + content_calls.append(url) + return encoded_file(pdf_body) + + with pytest.raises(policy.PolicyError, match="content byte budget"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=20, + head_sha="00" + "1" * 38, + event_action="opened", + token="token", + opener=opener, + ) + + # The first PDF's decoded bytes exactly consume the shared byte budget, + # so the second PDF-verification request is never made. + assert len(content_calls) == 1 + + +def test_evaluate_pull_request_enforces_request_budget_across_pdf_verification_reads( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Many patchless documentation PDFs must share the aggregate request budget. + + Regression coverage for Devin Review's finding that PDF-verification + reads bypassed ``MAX_CONTENT_REQUESTS``: without shared accounting, a + pull request containing many documentation PDFs could issue an + unbounded number of Contents API requests. + """ + monkeypatch.setattr(policy, "MAX_CONTENT_REQUESTS", 2) + content_calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + if "/pulls/21/files" in url: + return [ + {"filename": f"docs/papers/paper-{index}.pdf", "status": "added"} + for index in range(3) + ] + content_calls.append(url) + return encoded_file("%PDF-1.7\nresearch paper body\n") + + with pytest.raises(policy.PolicyError, match="content request budget"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=21, + head_sha="00" + "2" * 38, + event_action="opened", + token="token", + opener=opener, + ) + + assert len(content_calls) == 2 + + +def test_evaluate_pull_request_charges_oversized_pdf_exemption_against_request_budget( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The oversized-PDF exemption still consumes one request from the budget. + + The exemption path (``encoding: "none"``, no fetchable content) reads + zero bytes, so it must not be free of charge on the request budget -- + only the byte budget is naturally unaffected since nothing was decoded. + """ + monkeypatch.setattr(policy, "MAX_CONTENT_REQUESTS", 1) + content_calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + if "/pulls/22/files" in url: + return [ + {"filename": "docs/papers/big-paper.pdf", "status": "added"}, + {"filename": "docs/papers/second-paper.pdf", "status": "added"}, + ] + content_calls.append(url) + return {"type": "file", "encoding": "none", "size": policy.MAX_FILE_BYTES + 1, "content": ""} + + with pytest.raises(policy.PolicyError, match="content request budget"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=22, + head_sha="00" + "3" * 38, + event_action="opened", + token="token", + opener=opener, + ) + + assert len(content_calls) == 1 + + def test_closed_event_skips_without_credentials_or_identity_validation() -> None: """Closed-event cleanup remains a no-op for the required-workflow context."""