diff --git a/templates/assembly-root/scripts/validate-pins.py b/templates/assembly-root/scripts/validate-pins.py index 18524467..b32b0755 100755 --- a/templates/assembly-root/scripts/validate-pins.py +++ b/templates/assembly-root/scripts/validate-pins.py @@ -45,7 +45,8 @@ recheckable neutral-product pin disagrees 1 a FINDING: drift a reviewer can act on 2 a REFUSAL: the question could not be asked — an uninitialized submodule, - an unresolvable commit, an unreadable pin. An unanswerable pin question + an unresolvable commit, an unreadable pin or workflow file (one that is + not UTF-8, or that this user cannot open). An unanswerable pin question is never an implicit pass. No git to run is one too (`git-not-runnable`, #412), a git that is missing, will not run or cannot be executed: the gitlinks and the @@ -62,6 +63,7 @@ from collections.abc import Iterator from contextlib import contextmanager from pathlib import Path +from typing import NamedTuple sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 @@ -115,17 +117,99 @@ def unused(self, text: str) -> None: self.notes.append(f" note {text}") +class _Unreadable(NamedTuple): + """One workflow file that could not be read (#359): the line that names it, + and whether it failed on its ENCODING (`True`) or on being opened.""" + + line: str + encoding: bool + + +def _read_workflow(root: Path, path: Path) -> tuple[str, _Unreadable | None]: + """`(text, None)` for a workflow file read as UTF-8, or `("", why)` for one + that cannot be (#359). + + The line is `: not UTF-8 at byte N ()`, in + the words `repo_shape.load_yaml` gives the same failure (#192), so a file + this script cannot read is named the way its other readers name one. + POSIX, on every platform, and never the absolute path: the reader pastes + it into git and into an editor that opens it from the root. + A file this user cannot open (`OSError`) is the same line with the + operating system's own reason, because it is the same unguarded read. + """ + rel = path.relative_to(root).as_posix() + try: + return path.read_text(encoding="utf-8"), None + except UnicodeDecodeError as exc: + return "", _Unreadable( + f"{rel}: not UTF-8 at byte {exc.start} ({exc.reason})", True) + except OSError as exc: + return "", _Unreadable( + f"{rel}: cannot be read ({exc.strerror or type(exc).__name__})", + False) + + +def _workflow_unreadable(files: list[_Unreadable]) -> Refusal: + """The ONE refusal for every workflow file that could not be read (#359). + + EVERY FILE IS NAMED, in one refusal, so the operator repairs them in one + pass instead of meeting them one run at a time; `_workflow_refs` reads + them all before it raises. The first line follows the id and the rest + follow it, one file to a line, so `shape-doctor.py`, which quotes the + first `REFUSED` line of this script's output, quotes a file's name. + """ + advice = "" + if any(one.encoding for one in files): + advice += ( + "In each `not UTF-8` line the byte is counted from 0 at the start " + "of the file and is the first that is not UTF-8: re-save the file " + "as UTF-8 (a UTF-16 file with a byte-order mark needs converting, " + "not editing), or replace that byte with the character it was " + "meant to be, and commit it. ") + if not all(one.encoding for one in files): + advice += ( + "A `cannot be read` file is not an encoding failure: this user " + "cannot open it, so restore read access to it. ") + return Refusal( + "workflow-unreadable", + "\n".join(one.line for one in files), + f"Remediation: {len(files)} workflow file(s) under " + ".github/workflows/ cannot be read, and every one is named above. " + "This validator reads every `.yml` and `.yaml` file there, whether or " + "not it names a leg, and neither skips one nor reads one lossily: a " + "workflow it skipped could hide a `@` reference that is out of " + "lockstep, which is what it exists to find, and a lossy read would " + "call green a tree that the project's other readers refuse. " + + advice + "Then run the same command again.") + + def _workflow_refs(root: Path) -> list[tuple[Path, str, str]]: - """Every (file, owner/repo, sha) `@` reference in the workflows.""" + """Every (file, owner/repo, sha) `@` reference in the workflows, or + ONE refusal naming each workflow file that cannot be read (#359). + + EVERY FILE IS READ, whether or not it names a leg: the file is opened + before it is known to hold a reference, so a UTF-16 workflow that names no + leg used to stop the run in a `UnicodeDecodeError` (a `ValueError`, which + `main()` does not catch: a traceback, exit 1 -- the code this validator + uses for a finding). It is a REFUSAL, exit 2, `workflow-unreadable`, as a + pin that is not UTF-8 is `yaml-unreadable`: the question cannot be asked. + """ out: list[tuple[Path, str, str]] = [] + unreadable: list[_Unreadable] = [] workflows = root / ".github" / "workflows" if not workflows.is_dir(): return out for path in sorted(workflows.iterdir()): if path.suffix not in (".yml", ".yaml") or not path.is_file(): continue - for match in WORKFLOW_REF_RE.finditer(path.read_text(encoding="utf-8")): + text, problem = _read_workflow(root, path) + if problem is not None: + unreadable.append(problem) + continue + for match in WORKFLOW_REF_RE.finditer(text): out.append((path, match.group("repo"), match.group("sha").lower())) + if unreadable: + raise _workflow_unreadable(unreadable) return out diff --git a/tests/test_validate_pins_unreadable_workflow.py b/tests/test_validate_pins_unreadable_workflow.py new file mode 100644 index 00000000..ffd66c78 --- /dev/null +++ b/tests/test_validate_pins_unreadable_workflow.py @@ -0,0 +1,238 @@ +# SPDX-License-Identifier: Apache-2.0 +"""A workflow file that cannot be read is REFUSED BY NAME, not a traceback (#359). + +`scripts/validate-pins.py` reads every `.yml` and `.yaml` file under +`.github/workflows/` to find the `@` references that name a leg. That read +went around `load_yaml`, so a workflow that was not UTF-8 -- a Latin-1 +`# café` comment, a UTF-16 file with a byte-order mark -- raised +`UnicodeDecodeError`. It is a `ValueError`, `main()` caught `Refusal` alone, +and the run ended in a Python traceback with exit 1: the code this validator +uses for a FINDING, with no line naming the file, where a pin that is not +UTF-8 is a clean `REFUSED yaml-unreadable`, exit 2. A file this user cannot +open (`PermissionError`) was the same unguarded line. + +The script is a template copy that every project carries (it is digest-pinned +in the project's own `contracts/shape-pin.yaml`), so these cases run the copy +a real scaffold wrote, over a committed tree. + +The files are written with `write_bytes`: the point is the bytes, and +`write_text` would re-encode them, and on Windows rewrite their line endings. + +NOTHING HERE REACHES A NETWORK. The scaffold uses `--local-remote-dir`, and no +case asks for a neutral-product pin. +""" + +from __future__ import annotations + +import json +import os +import stat + +import pytest + +from conftest import ORG, PROJECT, REPO, git, run_script + +DOCTOR = REPO / "shape-doctor.py" +WORKFLOWS = ".github/workflows" +OTHER_SHA = "0123456789abcdef0123456789abcdef01234567" + +CANNOT_RUN_AS_ROOT = pytest.mark.skipif( + os.name == "nt" or (hasattr(os, "geteuid") and os.geteuid() == 0), + reason="a mode-000 file is read by root, and Windows modes do not refuse " + "a read") + + +def validate(root): + return run_script(root / "scripts" / "validate-pins.py", cwd=root) + + +def leg_workflow(root, sha: str | None = None) -> bytes: + """A workflow naming the spec leg at its pinned commit (or at `sha`): the + file the lockstep rule exists for, so a skipped one could hide a stale + reference. UTF-8, with LF line endings, whatever the host.""" + pinned = sha or git("rev-parse", "HEAD:spec", cwd=root).stdout.strip() + return (f"on: [pull_request]\njobs:\n s:\n" + f" uses: {ORG}/{PROJECT}-spec/.github/workflows/x.yml@{pinned}\n" + ).encode("utf-8") + + +def commit_bytes(root, rel: str, data: bytes, message: str) -> None: + """Write `rel` as exactly `data` and commit it, as in the issue: the file + is wrong in the repository, not only in a working tree.""" + target = root / rel + target.write_bytes(data) + git("add", "--", rel, cwd=root) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "-m", message, "--", rel, cwd=root) + + +def not_utf8_line(rel: str, data: bytes) -> str: + """`: not UTF-8 at byte N ()`, with N and the reason asked of + Python's own decoder and not typed in here.""" + try: + data.decode("utf-8") + except UnicodeDecodeError as exc: + return f"{rel}: not UTF-8 at byte {exc.start} ({exc.reason})" + raise AssertionError(f"fixture drift: {rel} is UTF-8") + + +def refusal_lines(result) -> list[str]: + """The files the refusal names, one per line, as printed: the first + follows the id on the `REFUSED` line, and the rest follow it.""" + return [line.replace("REFUSED workflow-unreadable: ", "", 1) + for line in result.stderr.splitlines() + if ": not UTF-8 at byte " in line or ": cannot be read (" in line] + + +def assert_refused_by_name(result, root, *names_and_lines: str) -> None: + """One `workflow-unreadable` refusal, exit 2, no traceback, naming each + file relative to the root and never by its absolute path.""" + assert "Traceback" not in result.stderr, result.stderr + assert "UnicodeDecodeError" not in result.stderr, result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.count("REFUSED ") == 1, result.stderr + assert result.stderr.startswith("REFUSED workflow-unreadable: "), \ + result.stderr + assert "pins ok" not in result.stdout + assert refusal_lines(result) == list(names_and_lines), result.stderr + assert str(root) not in result.stderr, \ + "the files are named relative to the root, never by absolute path" + assert f"{len(names_and_lines)} workflow file(s)" in result.stderr + + +# --- the refusal ------------------------------------------------------------ + +LATIN_1 = "legs.yml", lambda root: leg_workflow(root) + b"# caf\xe9\n" +UTF_16 = "other.yml", lambda root: "on: [push]\n".encode("utf-16") + + +@pytest.mark.parametrize("name, build", [ + pytest.param(*LATIN_1, id="latin-1-comment-in-the-file-naming-the-leg"), + pytest.param(*UTF_16, id="utf-16-bom-in-a-file-naming-no-leg"), +]) +def test_a_workflow_that_is_not_utf8_is_refused_by_name(project, name, build): + """The first two rows of the issue: a committed Latin-1 comment, and a + UTF-16 file that names no leg and still stopped the run, because the file + is opened before it is known to hold a reference.""" + data = build(project) + commit_bytes(project, f"{WORKFLOWS}/{name}", data, f"workflow {name}") + result = validate(project) + assert_refused_by_name(result, project, + not_utf8_line(f"{WORKFLOWS}/{name}", data)) + # The remedy the operator can act on. + assert "re-save the file as UTF-8" in result.stderr, result.stderr + assert "needs converting, not editing" in result.stderr, result.stderr + assert "run the same command again" in result.stderr, result.stderr + + +def test_every_workflow_that_is_not_utf8_is_named_in_the_one_refusal(project): + """Row (c): two files, one run. `.yaml` is read as `.yml` is, and the files + that ARE readable -- the scaffold's `validate.yml` and a good `ok.yml` -- + are not named.""" + latin = leg_workflow(project) + b"# caf\xe9\n" + utf16 = "on: [push]\n".encode("utf-16") + commit_bytes(project, f"{WORKFLOWS}/ok.yml", leg_workflow(project), "ok") + commit_bytes(project, f"{WORKFLOWS}/legs.yml", latin, "latin-1") + commit_bytes(project, f"{WORKFLOWS}/other.yaml", utf16, "utf-16") + result = validate(project) + assert_refused_by_name( + result, project, + not_utf8_line(f"{WORKFLOWS}/legs.yml", latin), + not_utf8_line(f"{WORKFLOWS}/other.yaml", utf16)) + assert "validate.yml" not in result.stderr, result.stderr + assert "ok.yml" not in result.stderr, result.stderr + + +def test_a_workflow_is_refused_and_neither_skipped_nor_read_lossily(project): + """THE REFUSAL IS THE POINT. The file names the spec leg at a commit that + is NOT the gitlink, which is exactly what this gate exists to find. Skipped, + the run would say `pins ok`; read with `errors="replace"`, it would report + `pin-workflow-ref-mismatch` and exit 1 over a file the project's other + readers refuse. Neither: the question cannot be asked, so it is refused.""" + data = leg_workflow(project, OTHER_SHA) + b"# caf\xe9\n" + commit_bytes(project, f"{WORKFLOWS}/legs.yml", data, "stale and latin-1") + result = validate(project) + assert_refused_by_name(result, project, + not_utf8_line(f"{WORKFLOWS}/legs.yml", data)) + assert "pin-workflow-ref-mismatch" not in result.stderr, result.stderr + + +@CANNOT_RUN_AS_ROOT +def test_a_workflow_this_user_cannot_open_is_refused_by_name(project): + """The last row of the issue: not an encoding failure but the same + unguarded read -- a `PermissionError` traceback. The refusal says what the + operating system said, and its remedy is access, not a re-save.""" + target = project / WORKFLOWS / "legs.yml" + commit_bytes(project, f"{WORKFLOWS}/legs.yml", leg_workflow(project), + "legs") + target.chmod(0) + try: + result = validate(project) + finally: + target.chmod(stat.S_IREAD | stat.S_IWRITE) + assert "Traceback" not in result.stderr, result.stderr + assert "PermissionError" not in result.stderr, result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED workflow-unreadable: {WORKFLOWS}/legs.yml: cannot be read " + "("), result.stderr + assert str(project) not in result.stderr, result.stderr + assert "restore read access" in result.stderr, result.stderr + assert "re-save" not in result.stderr, result.stderr + + +def test_the_doctors_pins_row_names_the_file(project): + """`shape-doctor.py` quotes the first `REFUSED` line of the project's own + validator in its `pins` row, so the file's name reaches a reader who never + runs the validator: before, the row quoted the last line of a traceback.""" + data = leg_workflow(project) + b"# caf\xe9\n" + commit_bytes(project, f"{WORKFLOWS}/legs.yml", data, "latin-1") + result = run_script(DOCTOR, "--root", str(project), "--json") + assert "Traceback" not in result.stderr, result.stderr + row = {r["id"]: r for r in json.loads(result.stdout)["rows"]}["pins"] + assert row["status"] == "FINDING", row + assert row["detail"]["exit"] == 2, row + assert "refuses (exit 2)" in row["reason"], row["reason"] + assert ("REFUSED workflow-unreadable: " + + not_utf8_line(f"{WORKFLOWS}/legs.yml", data)) in row["reason"], \ + row["reason"] + assert "UnicodeDecodeError" not in row["reason"], row["reason"] + + +# --- controls: what must not change ----------------------------------------- + +def test_a_pin_that_is_not_utf8_is_still_yaml_unreadable(project): + """Row (d): the refusal the workflow's one is modelled on, unchanged. Its + path is the absolute one `load_yaml` has always printed; the workflow + refusal is relative to the root, as the issue asks.""" + pin = project / "contracts" / "spec-pin.yaml" + commit_bytes(project, "contracts/spec-pin.yaml", + pin.read_bytes() + b"# caf\xe9\n", "latin-1 in the pin") + result = validate(project) + assert "Traceback" not in result.stderr, result.stderr + assert result.returncode == 2, result.stdout + result.stderr + assert "REFUSED yaml-unreadable: " in result.stderr, result.stderr + assert "spec-pin.yaml: not UTF-8 at byte " in result.stderr, result.stderr + assert "workflow-unreadable" not in result.stderr, result.stderr + + +@pytest.mark.parametrize("name, build", [ + pytest.param("legs.yml", + lambda root: leg_workflow(root) + "# café\n".encode("utf-8"), + id="utf-8-comment"), + pytest.param("legs.yml", + lambda root: b"\xef\xbb\xbf" + leg_workflow(root), + id="utf-8-byte-order-mark"), + pytest.param("notes.txt", lambda root: b"# caf\xe9\n", + id="a-latin-1-file-that-is-not-a-workflow"), +]) +def test_what_is_readable_or_not_a_workflow_still_passes(project, name, build): + """Row (e), and its two neighbours: UTF-8 with a non-ASCII comment, UTF-8 + with a byte-order mark (which `utf-8` keeps as a character and does not + refuse), and a file under `.github/workflows/` that is neither `.yml` nor + `.yaml`, which this validator has never read.""" + commit_bytes(project, f"{WORKFLOWS}/{name}", build(project), + f"workflow {name}") + result = validate(project) + assert result.returncode == 0, result.stdout + result.stderr + assert "pins ok" in result.stdout