diff --git a/.github/workflows/relock.yml b/.github/workflows/relock.yml index 9d58e766..e5ad6341 100644 --- a/.github/workflows/relock.yml +++ b/.github/workflows/relock.yml @@ -159,9 +159,14 @@ jobs: persist-credentials: false sparse-checkout: scripts/check_lock_hosts.py sparse-checkout-cone-mode: false + # Outside the checkout, not over it: on top of `main` the lock is a + # local change, and git will not carry a local change onto a base + # revision whose own uv.lock differs (#481). It is copied in only + # after the base revision is checked out, in the last step. - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: relocked-lock + path: ${{ runner.temp }}/relocked-lock # Re-check what actually arrived. The other job validated the lock # before running the suite, and code from the dependencies it just @@ -170,7 +175,7 @@ jobs: # ran none of that code, and before the token is exposed. The checker # comes from this clean checkout, not from the artifact. - name: Refuse a lock that names anything but PyPI - run: python scripts/check_lock_hosts.py uv.lock + run: python scripts/check_lock_hosts.py "$RUNNER_TEMP/relocked-lock/uv.lock" # The digest was taken before any resolved code ran. Matching it here # is what makes "verified" mean these bytes rather than some other @@ -178,7 +183,7 @@ jobs: - name: Confirm these are the bytes the other job verified env: DIGEST: ${{ needs.relock.outputs.digest }} - run: echo "$DIGEST uv.lock" | sha256sum --check --strict + run: echo "$DIGEST $RUNNER_TEMP/relocked-lock/uv.lock" | sha256sum --check --strict - name: Open a pull request with the verified lock env: @@ -186,6 +191,7 @@ jobs: PACKAGE: ${{ inputs.package }} BASE: ${{ inputs.base }} BASE_SHA: ${{ needs.relock.outputs.base_sha }} + DIGEST: ${{ needs.relock.outputs.digest }} run: | target="${PACKAGE:-all dependencies}" branch="relock/$(date +%Y%m%d-%H%M%S)" @@ -196,6 +202,11 @@ jobs: # against, not from whatever the base branch points at now. git fetch --depth 1 origin "$BASE_SHA" git checkout -b "$branch" "$BASE_SHA" + # Only now does the verified lock enter the checkout, and the + # file that is about to be committed is matched to the digest + # once more. + cp "$RUNNER_TEMP/relocked-lock/uv.lock" uv.lock + echo "$DIGEST uv.lock" | sha256sum --check --strict # --sparse: the checkout includes only the validator, so uv.lock # is outside the sparse definition and a plain `git add` refuses # to stage it. diff --git a/docs/dev/quality-gates.md b/docs/dev/quality-gates.md index 9a3e7c03..9daa7294 100644 --- a/docs/dev/quality-gates.md +++ b/docs/dev/quality-gates.md @@ -102,7 +102,11 @@ repository rejects that. Three paths out, in order of preference: re-runs the host check with that trusted copy, and matches the digest before the token is exposed. Nothing that ran during verification can reach its credential, plant a `pre-push` hook, or substitute a - different lock that merely looks acceptable. + different lock that merely looks acceptable. The lock waits outside the + checkout until the tested base revision is checked out, so the base can + be any branch, including one whose own `uv.lock` differs from `main`'s. + It is then copied in and matched to the digest once more right before + it is staged. One repository setting is a prerequisite: **Settings → Actions → "Allow GitHub Actions to create and approve pull requests"**. With it off, diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index 98ea7dda..4a9a2593 100644 --- a/tests/test_lockfile.py +++ b/tests/test_lockfile.py @@ -16,13 +16,18 @@ from __future__ import annotations +import hashlib import importlib.util +import os import re import shlex +import subprocess +import sys import tempfile import tomllib from collections.abc import Callable from pathlib import Path +from typing import Any from urllib.parse import urlsplit import pytest @@ -31,6 +36,7 @@ from packaging.utils import canonicalize_name from packaging.version import Version +from tests.platforms import bash_executable from tests.release_contracts import run_scripts, workflow_jobs #: The only hosts the lockfile may name. @@ -373,6 +379,19 @@ def test_the_committed_bytes_are_the_bytes_that_were_verified() -> None: assert "sha256sum --check --strict" in propose_scripts, ( "the committing job trusts the artifact's shape but not its identity" ) + # The verified lock waits outside the checkout and is copied in only + # after the base revision is checked out (#481), so the bytes that get + # staged are matched to the digest after that copy, not just before it. + commit = next( + step["run"] for step in propose["steps"] if "git add --sparse" in step.get("run", "") + ) + copy = 'cp "$RUNNER_TEMP/relocked-lock/uv.lock" uv.lock' + recheck = 'echo "$DIGEST uv.lock" | sha256sum --check --strict' + assert copy in commit, "the verified lock never enters the checkout from outside it" + assert recheck in commit, "the lock is staged without matching the copied file to the digest" + assert commit.index(copy) < commit.index(recheck) < commit.index("git add --sparse uv.lock"), ( + "the digest must be matched after the copy and before the lock is staged" + ) def test_the_validator_comes_from_a_revision_that_is_not_under_review() -> None: @@ -487,6 +506,193 @@ def test_the_lock_can_actually_be_staged_from_the_sparse_checkout() -> None: ) +def _git(cwd: Path, *args: str) -> str: + result = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True, check=False) + assert result.returncode == 0, f"git {' '.join(args)} failed: {result.stderr}" + return result.stdout.strip() + + +def _relock_origin(tmp_path: Path, *, base_lock: str | None) -> tuple[Path, str]: + """A bare origin with `main` and, unless `base_lock` is None, a `feature` + branch one commit ahead of it that locks `base_lock`. + + Returns the origin and the revision the relock job checked out and + tested: the `feature` tip, or `main` itself when there is no branch. + """ + seed = tmp_path / "seed" + seed.mkdir() + _git(seed, "init", "-q", "-b", "main") + _git(seed, "config", "user.name", "seed") + _git(seed, "config", "user.email", "seed@example.invalid") + (seed / "scripts").mkdir() + checker = (_ROOT / "scripts" / "check_lock_hosts.py").read_text(encoding="utf-8") + (seed / "scripts" / "check_lock_hosts.py").write_text(checker, encoding="utf-8") + (seed / "uv.lock").write_text("main's lock\n", encoding="utf-8") + _git(seed, "add", ".") + _git(seed, "commit", "-q", "-m", "main") + if base_lock is not None: + _git(seed, "checkout", "-q", "-b", "feature") + (seed / "feature.txt").write_text("the change under review\n", encoding="utf-8") + (seed / "uv.lock").write_text(base_lock, encoding="utf-8") + _git(seed, "add", ".") + _git(seed, "commit", "-q", "-m", "feature") + origin = tmp_path / "origin.git" + _git(tmp_path, "clone", "-q", "--bare", str(seed), str(origin)) + return origin, _git(seed, "rev-parse", "HEAD") + + +def _checkout_main_sparsely(origin: Path, work: Path) -> None: + """What `actions/checkout` does for `ref: main` with the validator only.""" + _git(origin.parent, "clone", "-q", "--no-checkout", origin.as_uri(), str(work)) + _git(work, "sparse-checkout", "set", "--no-cone", "scripts/check_lock_hosts.py") + _git(work, "checkout", "-q", "main") + + +def _download_artifact( + steps: list[dict[str, Any]], work: Path, runner_temp: Path, lock: bytes +) -> None: + """Place the artifact where the workflow's `download-artifact` step puts it.""" + download = next( + step for step in steps if str(step.get("uses", "")).startswith("actions/download-artifact") + ) + path = download.get("with", {}).get("path") + target = ( + work if path is None else Path(path.replace("${{ runner.temp }}", runner_temp.as_posix())) + ) + assert "${{" not in str(target), f"unresolved expression in the artifact path {path!r}" + target.mkdir(parents=True, exist_ok=True) + (target / "uv.lock").write_bytes(lock) + + +def _run_propose_steps( + steps: list[dict[str, Any]], work: Path, env: dict[str, str], expressions: dict[str, str] +) -> None: + """Run every `run` step of the propose job, as GitHub would, in order.""" + prelude = ( + # Each call's exact argv, NUL-separated, in its own numbered file, and + # only the two calls the job is meant to make succeed: anything else, + # such as a `gh pr merge`, fails the step and is still on record. + "gh() {\n" + ' printf "%s\\0" "$@" > "$FAKE_GH_DIR/$(ls "$FAKE_GH_DIR" | wc -l | tr -d " ")"\n' + ' case "$1 $2" in\n' + ' "auth setup-git" | "pr create") return 0 ;;\n' + ' *) echo "unexpected gh call: $*" >&2; return 1 ;;\n' + " esac\n" + "}\n" + # macOS ships a BSD sha256sum without --check/--strict; the runner's + # is GNU coreutils, and shasum accepts the same options. + "if ! sha256sum --version 2>/dev/null | grep -q GNU; then\n" + ' sha256sum() { shasum -a 256 "$@"; }\n' + "fi\n" + ) + + def resolve(value: str) -> str: + for expression, actual in expressions.items(): + value = value.replace(expression, actual) + assert "${{" not in value, f"unresolved expression in {value!r}" + return value + + for step in steps: + if "run" not in step: + continue + step_env = {name: resolve(str(value)) for name, value in step.get("env", {}).items()} + result = subprocess.run( + [ + bash_executable(), + "--noprofile", + "--norc", + "-e", + "-c", + prelude + resolve(step["run"]), + ], + cwd=work, + env={**env, **step_env}, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, f"{step.get('name')!r} failed:\n{result.stderr}" + + +@pytest.mark.skipif(sys.platform == "win32", reason="the Relock workflow runs on Linux runners") +@pytest.mark.parametrize( + ("base", "base_lock"), + [("main", None), ("feature", "main's lock\n"), ("feature", "the branch's own lock\n")], + ids=["base-is-main", "branch-shares-main-lock", "branch-has-its-own-lock"], +) +def test_the_verified_lock_is_committed_onto_any_base( + tmp_path: Path, base: str, base_lock: str | None +) -> None: + """`base` may be any branch, not just `main`. + + Run 40155755520 relocked filelock onto a branch with its own `uv.lock`, + and `propose` died at `git checkout`: the verified lock it had downloaded + over the sparse `main` checkout counted as a local change git would not + carry onto another revision (#481). Every earlier run used `main`, whose + tip is the tested revision, so the checkout changed nothing. This runs + the job's own steps against a sparse checkout of `main` and expects the + verified bytes, and nothing else, committed onto the tested revision. + """ + steps = workflow_jobs(_RELOCK_WORKFLOW)["propose"]["steps"] + origin, base_sha = _relock_origin(tmp_path, base_lock=base_lock) + work = tmp_path / "work" + _checkout_main_sparsely(origin, work) + runner_temp = tmp_path / "runner-temp" + runner_temp.mkdir() + verified = (_ROOT / "uv.lock").read_bytes() + _download_artifact(steps, work, runner_temp, verified) + gh_calls = tmp_path / "gh-calls" + gh_calls.mkdir() + env = { + "PATH": f"{Path(sys.executable).parent}{os.pathsep}{os.environ.get('PATH', '')}", + "HOME": tmp_path.as_posix(), + "GIT_CONFIG_NOSYSTEM": "1", + "RUNNER_TEMP": runner_temp.as_posix(), + "FAKE_GH_DIR": gh_calls.as_posix(), + } + expressions = { + "${{ needs.relock.outputs.digest }}": hashlib.sha256(verified).hexdigest(), + "${{ needs.relock.outputs.base_sha }}": base_sha, + "${{ inputs.package }}": "filelock", + "${{ inputs.base }}": base, + "${{ github.token }}": "not-a-token", + } + + _run_propose_steps(steps, work, env, expressions) + + # `--git-dir`, not the bare repository as the working directory: git can + # be configured to refuse implicit bare repositories (safe.bareRepository). + remote = ("--git-dir", str(origin)) + pushed = _git( + tmp_path, *remote, "for-each-ref", "--format=%(refname:short)", "refs/heads/relock/" + ) + assert len(pushed.splitlines()) == 1, f"expected one relock branch, got {pushed!r}" + assert _git(tmp_path, *remote, "rev-parse", f"{pushed}^") == base_sha + changed = _git(tmp_path, *remote, "diff", "--name-only", base_sha, pushed) + assert changed.splitlines() == ["uv.lock"] + committed = subprocess.run( + ["git", *remote, "show", f"{pushed}:uv.lock"], capture_output=True, check=True + ).stdout + assert committed == verified, "the committed lock is not the verified artifact" + calls = [ + (gh_calls / name).read_bytes().decode("utf-8").split("\0")[:-1] + for name in sorted(os.listdir(gh_calls), key=int) + ] + # The whole call set, not just the one that matters: the job authenticates + # git, then opens the pull request, and calls gh for nothing else. + assert [call[:2] for call in calls] == [["auth", "setup-git"], ["pr", "create"]], calls + assert calls[0] == ["auth", "setup-git"] + # Every option of the pull request, in order, so an added `--repo` or a + # stray argument cannot ride along on a call whose start still matches. + options = calls[1][2:] + assert options[0::2] == ["--base", "--head", "--title", "--body"], options + created = dict(zip(options[0::2], options[1::2], strict=True)) + assert created["--base"] == base + assert created["--head"] == pushed + assert created["--title"] == "build: relock filelock" + assert base_sha in created["--body"], "the body must name the tested revision" + + def test_the_relock_pull_request_does_not_promise_codeql_it_cannot_deliver() -> None: """CodeQL runs only for pull requests targeting `main`.