From dd9e11bc11e7f023f10f8c4c91610c82e25d4e9e Mon Sep 17 00:00:00 2001 From: hellices Date: Sun, 25 Oct 2026 14:00:08 +0900 Subject: [PATCH 1/3] ci: let Relock commit its lock onto any base branch The propose job downloaded the verified lock over its sparse checkout of main, then switched to the tested base revision. When the base's uv.lock differs from main's, git refuses to carry that local change across ("Your local changes ... would be overwritten by checkout"), so every relock onto such a branch failed after all its verification. Runs 40155755520 and 40158406513 hit it on the bundle branch, and their locks had to be committed by hand (#481). Earlier runs all used base=main, whose tip is the tested revision, so the switch changed nothing. The artifact now downloads to $RUNNER_TEMP/relocked-lock, outside the checkout. The host check and the digest match run on it there, with main's validator, before the token is exposed, as before. Only after the base revision is checked out is it copied in, and the file about to be staged is matched to the digest once more. test_the_verified_lock_is_committed_onto_any_base runs the job's own steps against a sparse checkout of main in a temporary repository, with a stub gh. It covers three bases: main itself, a branch that shares main's lock, and a branch with its own lock. The last failed with CI's exact error before the change, and all three now push the verified bytes, and nothing else, onto the tested revision. test_the_committed_bytes_are_the_bytes_that_were_verified now also pins the second digest match between the copy and `git add`; removing it or moving it before the copy fails that test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/relock.yml | 15 ++- docs/dev/quality-gates.md | 6 +- tests/test_lockfile.py | 184 +++++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 3 deletions(-) diff --git a/.github/workflows/relock.yml b/.github/workflows/relock.yml index 9d58e7660..e5ad63415 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 9a3e7c03b..9daa72942 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 98ea7dda0..dc78f7e70 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,171 @@ 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 = ( + 'gh() { printf "%s\\n" "$*" >> "$FAKE_GH_CALLS"; }\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.txt" + 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_CALLS": 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.read_text(encoding="utf-8").splitlines() + # The multi-line --body spreads one call over several recorded lines. + created = [line for line in calls if line.startswith("pr create ")] + assert len(created) == 1, calls + assert created[0].startswith(f"pr create --base {base} --head {pushed} "), created + + def test_the_relock_pull_request_does_not_promise_codeql_it_cannot_deliver() -> None: """CodeQL runs only for pull requests targeting `main`. From 455e6a6eaa3c6d122b30d78bba9a95068cf142b0 Mon Sep 17 00:00:00 2001 From: hellices Date: Sun, 25 Oct 2026 14:35:25 +0900 Subject: [PATCH 2/3] test: accept only the gh calls the propose job is meant to make The stub recorded every gh call and always succeeded, and the test only looked for the one `pr create` line. So a workflow that also ran, say, `gh pr merge "$branch" --squash || true` would pass while doing exactly what the repository forbids. The stub now writes each call's exact argv, NUL-separated, to its own numbered file, and accepts only `gh auth setup-git` and `gh pr create`; any other call fails the step and stays on record. The test asserts the whole call sequence, not one line. Injecting `gh pr merge "$branch" --squash || true` after the push now fails the call-set assertion, and the unguarded form fails the step with "unexpected gh call"; both passed before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_lockfile.py | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index dc78f7e70..23f442729 100644 --- a/tests/test_lockfile.py +++ b/tests/test_lockfile.py @@ -569,7 +569,16 @@ def _run_propose_steps( ) -> None: """Run every `run` step of the propose job, as GitHub would, in order.""" prelude = ( - 'gh() { printf "%s\\n" "$*" >> "$FAKE_GH_CALLS"; }\n' + # 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" @@ -632,13 +641,14 @@ def test_the_verified_lock_is_committed_onto_any_base( runner_temp.mkdir() verified = (_ROOT / "uv.lock").read_bytes() _download_artifact(steps, work, runner_temp, verified) - gh_calls = tmp_path / "gh-calls.txt" + 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_CALLS": gh_calls.as_posix(), + "FAKE_GH_DIR": gh_calls.as_posix(), } expressions = { "${{ needs.relock.outputs.digest }}": hashlib.sha256(verified).hexdigest(), @@ -664,11 +674,15 @@ def test_the_verified_lock_is_committed_onto_any_base( ["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.read_text(encoding="utf-8").splitlines() - # The multi-line --body spreads one call over several recorded lines. - created = [line for line in calls if line.startswith("pr create ")] - assert len(created) == 1, calls - assert created[0].startswith(f"pr create --base {base} --head {pushed} "), created + 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"] + assert calls[1][2:6] == ["--base", base, "--head", pushed], calls[1] def test_the_relock_pull_request_does_not_promise_codeql_it_cannot_deliver() -> None: From 6a63a41fa9f5eea936b23513613f434e4065e05e Mon Sep 17 00:00:00 2001 From: hellices Date: Sun, 25 Oct 2026 14:52:49 +0900 Subject: [PATCH 3/3] test: pin every option of the relock pull request call The stub accepted any `pr create`, and the test compared only the first four arguments after it, so appending `--repo other/target` (or a stray argument) to the workflow's call kept the recorded prefix and the call count intact and passed, while the real pull request would target another repository. The test now requires the options to be exactly --base, --head, --title and --body, in that order, pairs them, and checks each value: the base, the pushed branch, the relock title, and a body that names the tested revision. Comparing the whole argv would copy the body's prose into the test, so the option names pin the shape and the values pin what matters. Adding `--repo other/target` or a positional argument to the call now fails the test; both passed before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_lockfile.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index 23f442729..4a9a25933 100644 --- a/tests/test_lockfile.py +++ b/tests/test_lockfile.py @@ -682,7 +682,15 @@ def test_the_verified_lock_is_committed_onto_any_base( # 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"] - assert calls[1][2:6] == ["--base", base, "--head", pushed], calls[1] + # 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: