validate-pins.py (assembly-root template): a workflow file under .github/workflows/ that is not UTF-8, or that this user cannot open, is ONE workflow-unreadable refusal naming every such file, exit 2, so the project's pins gate no longer ends in a UnicodeDecodeError traceback, exit 1, with no line naming the file (#359)

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 `<path relative to root>: not UTF-8 at byte N (<reason>)`, 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 `@<sha>` 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) `@<sha>` reference in the workflows."""
+    """Every (file, owner/repo, sha) `@<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
 
 

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 `@<sha>` 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:
+    """`<rel>: not UTF-8 at byte N (<reason>)`, 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