diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 26e8555967..cbd0c0c7cf 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -653,9 +653,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 @@ -671,6 +679,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" else mkdir -p "$coverage_build_dir/base-python-requirements" diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e12f33542d..e8b26d79ad 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -230,9 +230,11 @@ source evidence. 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 06b3dba425..0b2d810a20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,20 @@ 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, 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:`. +- 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; + 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. - **Pin `opencode-review-dispatch.yml` off the starved floating `ubuntu-latest` image.** The 2026-09-01 floating-image fix (see that entry below) pinned `strix.yml`, `opencode-review.yml`, and `noema-review.yml` -- the three required-check diff --git a/docs/doctoring/opencode-exact-vcs-dependency-evidence.md b/docs/doctoring/opencode-exact-vcs-dependency-evidence.md index a9b26ae474..05a709b632 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 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 +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,10 @@ 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 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. - 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 new file mode 100644 index 0000000000..82f6b878bc --- /dev/null +++ b/docs/doctoring/opencode-python-head-lock-trust.md @@ -0,0 +1,79 @@ +# 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. 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. 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. 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. 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 +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 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 +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 +`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 +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; 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 + +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/docs/doctoring/trusted-uv-lock-materialization.md b/docs/doctoring/trusted-uv-lock-materialization.md index 2d83e8bda8..69028ff56d 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,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. installs one process-wide urllib opener with an empty proxy map and a redirect +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; +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; -4. 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; @@ -28,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; -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 +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; -8. 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; -9. 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; -10. 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; -11. 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 -12. 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 @@ -92,8 +102,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 +134,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 a052123547..c4df56d1d2 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 @@ -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(), + ), ) @@ -616,65 +622,237 @@ 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} 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}") 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) 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(), ), ) +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] +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}") + 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_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, exported_vcs_dependencies = exported + if registry_content: + locks.append((lock_path, registry_content)) + for dependency in exported_vcs_dependencies: + dependency = {**dependency, "source": lock_path} + repository_key = dependency["repository"].casefold() + 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_commits[repository_key] = dependency["commit"] + vcs_dependencies.append(dependency) + return sorted(locks, key=lambda item: item[0]), sorted( + vcs_dependencies, + key=lambda dependency: ( + dependency["repository"].casefold(), + dependency.get("source", ""), + dependency.get("package", "").casefold(), + dependency.get("import_name", "").casefold(), + ), + ) + + +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 + 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 + 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, 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): @@ -683,14 +861,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()) @@ -725,18 +918,60 @@ 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 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) 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) - } - locks, vcs_manifest = _base_python_inputs(resolved_repo, base_sha) + 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() + 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 + ) + 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) + 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 = _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, + 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" @@ -747,6 +982,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) @@ -774,15 +1011,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 +1038,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/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 33e58ed876..43b0be7b32 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -25,6 +25,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" @@ -317,11 +319,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") @@ -348,23 +353,19 @@ 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) - # Unreachable by construction, not a live fallback: every one of the 31 - # `range(1, 32)` iterations that reaches this point already returned a - # page whose length is >= 100 (a page under 100 items hits the `return` - # two lines up first), so 31 such pages accumulate at least 3,100 files - # -- strictly more than the 3,000 cap above, which is checked after - # every single appended item, not just at page boundaries. That in-loop - # check therefore always raises no later than partway through the 31st - # page, before the `for` loop can ever exhaust its range. Kept as a - # structural fail-closed guard (so a future change to PAGE_COUNT, - # per_page, or the 3,000 cap that breaks this invariant fails loudly - # instead of silently truncating evidence) rather than deleted; see - # test_changed_file_pagination_bound_is_provably_unreachable, which - # pins the arithmetic relationship itself. - raise PolicyError("GitHub changed-file pagination exceeded 3,000 files") # pragma: no cover - - -def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str, token: str, opener: OpenJson) -> bytes: + page += 1 + + +def _load_raw_file_bytes( + api_url: str, + repository: str, + path: str, + head_sha: str, + token: str, + opener: OpenJson, + *, + max_bytes: int = MAX_FILE_BYTES, +) -> bytes: """Load one final head file's raw decoded bytes from the Contents API. Raises ``ContentSizeExceededError`` specifically when the declared size @@ -379,6 +380,15 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str ``encoding: "none"`` with an accurate ``size`` and no ``content`` at all. Both are treated as the same size-exceeded evidence; every other response shape still fails closed. + + ``max_bytes`` additionally bounds the declared size against a caller + -supplied remaining content budget (``evaluate_pull_request``'s + aggregate per-scan byte cap), independent of the fixed per-file + ``MAX_FILE_BYTES`` ceiling above. Exceeding only this narrower budget + raises the generic ``PolicyError`` budget failure, never + ``ContentSizeExceededError`` -- the PDF binary-content exemption must + not treat shared-budget exhaustion as evidence the file itself is + oversized. """ encoded_path = quote(path, safe="/") @@ -401,6 +411,8 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str raise PolicyError(f"GitHub content evidence for {path} has a malformed size or content field") if declared_size > MAX_FILE_BYTES: raise ContentSizeExceededError(f"GitHub content evidence for {path} exceeds 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: @@ -410,10 +422,19 @@ def _load_raw_file_bytes(api_url: str, repository: str, path: str, head_sha: str return raw -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.""" - raw = _load_raw_file_bytes(api_url, repository, path, head_sha, token, opener) + raw = _load_raw_file_bytes(api_url, repository, path, head_sha, token, opener, max_bytes=max_bytes) try: return raw.decode("utf-8") except UnicodeDecodeError as exc: @@ -428,7 +449,8 @@ def _binary_documentation_evidence_confirms( head_sha: str, token: str, opener: OpenJson, -) -> bool: + max_bytes: int, +) -> tuple[bool, int]: """Return whether a claimed binary documentation asset is genuine. A missing diff ``patch`` alone is not proof of binary content: GitHub @@ -444,16 +466,24 @@ def _binary_documentation_evidence_confirms( 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 PurePosixPath(changed.path).suffix.lower() == ".pdf" + return PurePosixPath(changed.path).suffix.lower() == ".pdf", 0 suffix = PurePosixPath(changed.path).suffix.lower() if suffix == ".png": - return _is_complete_png(raw) - return raw.startswith(BINARY_DOCUMENT_MAGIC[suffix]) + return _is_complete_png(raw), len(raw) + return raw.startswith(BINARY_DOCUMENT_MAGIC[suffix]), len(raw) def _png_unfilter_row(filtered: bytes, previous: bytes, filter_type: int, bytes_per_pixel: int) -> bytes: @@ -625,6 +655,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, @@ -649,6 +698,8 @@ 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: # A claimed binary documentation asset gets its own network-verified # check ahead of _needs_content_scan's patch-presence-only signal: @@ -660,18 +711,34 @@ 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_asset(changed): - if _binary_documentation_evidence_confirms( + remaining_asset_bytes = _reserve_content_budget(content_requests, total_content_bytes) + confirmed_binary, asset_bytes_read = _binary_documentation_evidence_confirms( changed, api_url=api_url.rstrip("/"), repository=repository, head_sha=head_sha, token=token, opener=opener, - ): + max_bytes=remaining_asset_bytes, + ) + content_requests += 1 + total_content_bytes += asset_bytes_read + if confirmed_binary: continue elif not _needs_content_scan(changed): continue - content = _load_file_content(api_url.rstrip("/"), repository, changed.path, head_sha, token, opener) + remaining_bytes = _reserve_content_budget(content_requests, total_content_bytes) + 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 a2da04ae25..b92d042321 100644 --- a/tests/test_materialize_base_python_requirements.py +++ b/tests/test_materialize_base_python_requirements.py @@ -102,6 +102,244 @@ 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_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: + """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_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_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, @@ -396,6 +634,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_rejects_invalid_base_sha(tmp_path: Path) -> None: """Git options and symbolic refs cannot cross the exact-SHA boundary.""" with pytest.raises(ValueError, match="40 hexadecimal"): @@ -479,11 +791,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, @@ -713,6 +1061,386 @@ 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_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, +) -> 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_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, +) -> 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 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"), + [ + ([], "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", *, diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 37ec068db9..67cf24f432 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), @@ -2560,6 +2565,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_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index c5d4e9d7a3..c1000f9c92 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -4,8 +4,6 @@ import base64 import importlib.util -import inspect -import re import sys import zlib from io import BytesIO @@ -298,6 +296,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_evaluate_pull_request_exempts_an_oversized_documentation_pdf() -> None: """A genuinely oversized documentation PDF still cannot be verified by content. @@ -312,7 +380,7 @@ def test_evaluate_pull_request_exempts_an_oversized_documentation_pdf() -> None: """ def opener(url: str, _token: str) -> object: - if "/pulls/11/files" in url: + if "/pulls/15/files" in url: return [ {"filename": "docs/papers/big-paper.pdf", "status": "added"}, ] @@ -322,8 +390,8 @@ def opener(url: str, _token: str) -> object: result = policy.evaluate_pull_request( api_url="https://api.github.test", repository="ContextualWisdomLab/example", - pull_request=11, - head_sha="c" * 40, + pull_request=15, + head_sha="00" + "c" * 38, event_action="opened", token="token", opener=opener, @@ -342,7 +410,7 @@ def test_evaluate_pull_request_scans_a_disguised_textual_pdf_without_a_patch() - """ def opener(url: str, _token: str) -> object: - if "/pulls/12/files" in url: + if "/pulls/16/files" in url: return [ {"filename": "docs/papers/not-really-a-pdf.pdf", "status": "added"}, ] @@ -352,8 +420,8 @@ def opener(url: str, _token: str) -> object: result = policy.evaluate_pull_request( api_url="https://api.github.test", repository="ContextualWisdomLab/example", - pull_request=12, - head_sha="d" * 40, + pull_request=16, + head_sha="00" + "d" * 38, event_action="opened", token="token", opener=opener, @@ -365,7 +433,7 @@ def test_evaluate_pull_request_exempts_a_real_pdf_under_the_size_ceiling() -> No """A genuine, fetchable PDF (verified by its magic bytes) is exempt too.""" def opener(url: str, _token: str) -> object: - if "/pulls/13/files" in url: + if "/pulls/17/files" in url: return [ {"filename": "docs/papers/small-paper.pdf", "status": "added"}, ] @@ -375,8 +443,8 @@ def opener(url: str, _token: str) -> object: result = policy.evaluate_pull_request( api_url="https://api.github.test", repository="ContextualWisdomLab/example", - pull_request=13, - head_sha="e" * 40, + pull_request=17, + head_sha="00" + "e" * 38, event_action="opened", token="token", opener=opener, @@ -580,7 +648,7 @@ def test_evaluate_pull_request_does_not_fetch_a_removed_binary_pdf() -> None: """ def opener(url: str, _token: str) -> object: - if "/pulls/14/files" in url: + if "/pulls/18/files" in url: return [ {"filename": "docs/papers/removed-paper.pdf", "status": "removed"}, ] @@ -589,8 +657,8 @@ def opener(url: str, _token: str) -> object: result = policy.evaluate_pull_request( api_url="https://api.github.test", repository="ContextualWisdomLab/example", - pull_request=14, - head_sha="f" * 40, + pull_request=18, + head_sha="00" + "f" * 38, event_action="opened", token="token", opener=opener, @@ -598,6 +666,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.""" @@ -656,11 +836,52 @@ 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) +def test_changed_file_pagination_stops_at_page_31_without_a_page_32_request() -> None: + """The page-31 terminal check raises before ever requesting a page 32. + + ``_load_changed_files`` no longer bounds itself with ``for page in + range(1, 32)``: it now loops with an explicit ``while True`` and its own + ``page == 31 and len(payload) >= 100`` check, fired immediately after + fetching each page and before that page's items are appended. This pins + that check as live, reachable code -- not dead code needing a coverage + pragma -- by asserting the opener is called exactly 31 times (one call + per page 1..31) and never for a 32nd page. + """ + + page = [ + {"filename": f"f-{index}", "status": "modified", "patch": ""} + for index in range(100) + ] + calls: list[str] = [] + + def opener(url: str, _token: str) -> object: + calls.append(url) + return page + + with pytest.raises(policy.PolicyError, match="3,000"): + policy._load_changed_files("api", "a/b", 1, "x", opener) + + assert len(calls) == 31 + assert calls[-1].endswith("page=31") + + def test_changed_file_pagination_accepts_the_inclusive_bound() -> None: """Exactly 3,000 changed files are accepted only after an empty next page.""" @@ -679,35 +900,6 @@ def opener(url: str, _token: str) -> object: assert calls[-1].endswith("page=31") -def test_changed_file_pagination_bound_is_provably_unreachable() -> None: - """Pin the arithmetic invariant that makes the loop's trailing raise dead code. - - ``_load_changed_files`` raises inside its item loop the moment - ``len(files) > 3_000`` (checked after every appended item, not only at - page boundaries) and returns early the moment one page has fewer than - 100 items -- so the ``# pragma: no cover``-marked ``raise`` after the - ``for page in range(...)`` loop can only execute if every one of that - many pages returns at least 100 items while the cumulative total never - exceeds 3,000. That requires ``page_count * per_page <= 3_000``, which - the real page count (31) and per_page (100) violate (3,100 > 3,000) -- - the in-loop raise always fires first. This test reads those literals - from the actual source rather than duplicating them, so it fails loudly - if a future edit to any of the three breaks the inequality -- exactly - when the trailing raise becomes reachable again and needs a real - covering test instead of the pragma. - """ - source = inspect.getsource(policy._load_changed_files) - start, stop = (int(n) for n in re.search(r"range\((\d+),\s*(\d+)\)", source).groups()) - page_count = len(range(start, stop)) - per_page = int(re.search(r"per_page=(\d+)", source).group(1)) - cap = int(re.search(r"len\(files\) > (\d[\d_]*)", source).group(1).replace("_", "")) - assert page_count * per_page > cap, ( - "the trailing pagination raise in _load_changed_files is no longer " - "provably unreachable; remove its '# pragma: no cover' and add a " - "test that actually covers it" - ) - - @pytest.mark.parametrize( ("payload", "message"), [ diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index 2d2304aaf1..dd6f876c42 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -17,7 +17,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 = "26e8555967171a5f3974602ac05700c27bddebf1" +REVIEW_DISPATCH_BLOB_SHA = "cbd0c0c7cf4e7c2ded45c08ce39b893912e0eb26" def _workflow_text(path: Path) -> str: 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,