diff --git a/.github/actions/repo-gate/repo_gate.py b/.github/actions/repo-gate/repo_gate.py index 275cb9da..2f0ab8be 100755 --- a/.github/actions/repo-gate/repo_gate.py +++ b/.github/actions/repo-gate/repo_gate.py @@ -52,6 +52,7 @@ # Reading any of those as absence fails a correct pin, which is the direction that costs most. ABSENT = {"404", "422"} GH_TIMEOUT = 20 +CONTROL = re.compile(r"[\x00-\x1f\x7f-\x9f]") # How a check says it did less than its name. # A gate that quietly degrades to a weaker reading prints the same clean line as one that ran. @@ -62,7 +63,12 @@ def sh(*args: str) -> str: return subprocess.run( - args, capture_output=True, text=True, encoding="utf-8", check=False + args, + capture_output=True, + text=True, + encoding="utf-8", + errors="surrogateescape", + check=False, ).stdout @@ -125,18 +131,51 @@ def tracked(root: Path, exclude: list[str] | None = None) -> list[str]: A caller vendoring a subtree it does not author, per GOVERNANCE.md's carry-versus-reach test, can scope every check out of that subtree this way. No check itself needs to change. Additive only: an empty or absent `exclude` scans exactly what it always has. + + Git's quoting is pinned on, so the listing is ASCII whatever a config inherits and each quoted + name reaches `unquote_path` in the one form it decodes. Git's stderr carries no such quoting, and + a failure naming a root that is not UTF-8 echoes that name raw, so the decode tolerates it. """ - args = ["git", "-C", str(root), "ls-files"] + args = ["git", "-C", str(root), "-c", "core.quotePath=true", "ls-files"] if exclude: args += ["--", *(f":!{pattern}" for pattern in exclude)] - result = subprocess.run(args, capture_output=True, text=True, encoding="utf-8", check=False) + result = subprocess.run( + args, + capture_output=True, + text=True, + encoding="utf-8", + errors="surrogateescape", + check=False, + ) if result.returncode != 0: # A failed command's stdout is never trusted, even where it is non-empty. # A partial listing read as complete is a scan that missed files and said nothing. reason = result.stderr.strip() or f"exit {result.returncode}, no stderr" - print(f"git ls-files failed: {reason}", file=sys.stderr) + print(f"git ls-files failed: {printable(reason)}", file=sys.stderr) return [] - return [l for l in result.stdout.split("\n") if l] + return [unquote_path(l) for l in result.stdout.split("\n") if l] + + +def unquote_path(name: str) -> str: + """The real name behind one `git ls-files` line, which git quotes when the name needs it. + + Git quotes a name holding any byte at or above 0x80, and one holding a quote, a backslash, + or a control character, escaping it the way C does. Read as a literal path, the quoted form + names no file on disk, so a check reading the file would pass over it and report clean. + + The caller pins `core.quotePath=true`, which is what makes a quoted line ASCII and so what + this decode assumes. Turning the setting off instead would not do: it stops git quoting the + first of those three routes and leaves the other two quoting a name whose non-ASCII bytes sit + raw inside the quotes, which this decode cannot carry. + + A byte that is not valid UTF-8 comes back as a surrogate escape, the form `Path` and + `resolved_eol` both encode back to the original byte. Latin-1 carries each unescaped byte + through unchanged on the way there. + """ + if name.startswith('"') and name.endswith('"') and len(name) > 1: + unescaped = name[1:-1].encode("latin-1", "backslashreplace").decode("unicode-escape") + return unescaped.encode("latin-1", "surrogateescape").decode("utf-8", "surrogateescape") + return name def workflow_files(files: list[str]) -> list[str]: @@ -360,7 +399,36 @@ def check_eol_coverage(root: Path, files: list[str]) -> list[str]: CHECKS = {"sha-pin": check_sha_pin, "eol": check_eol, "eol-coverage": check_eol_coverage} +def printable(line: str) -> str: + """`line` with each control character spelled as an escape, so one finding prints as one line. + + `unquote_path` hands back a tracked name exactly as it is on disk, and a name may hold a + newline or an escape sequence. Printed raw, such a name forges a line of the gate's own output + or drives the reader's terminal, so each C0 or C1 control and DEL is escaped here, where it is + shown. A byte that is not UTF-8 arrives as a lone surrogate, which a strict stream refuses to + encode, so it is spelled as an escape too, leaving the result safe on any stream. + """ + escaped = CONTROL.sub(lambda m: f"\\x{ord(m.group()):02x}", line) + return escaped.encode("utf-8", "backslashreplace").decode("utf-8") + + +def report_paths_that_are_not_utf8() -> None: + """Let a path holding a byte that is not UTF-8 print rather than ending the run. + + `unquote_path` decodes such a name with surrogateescape so it opens on disk, which leaves the + lone surrogate in the name to reach this program's own output. Encoding it strictly raises at + the line printing that name, so every check after it is lost along with the run's verdict, and + the exit code becomes a traceback's rather than the gate's. Escaping it costs the reader one + unreadable byte in one name. + """ + for stream in (sys.stdout, sys.stderr): + reconfigure = getattr(stream, "reconfigure", None) + if reconfigure is not None: + reconfigure(errors="backslashreplace") + + def main(argv: list[str] | None = None) -> int: + report_paths_that_are_not_utf8() ap = argparse.ArgumentParser() ap.add_argument("--root", default=".") ap.add_argument("--check", action="append", choices=sorted(CHECKS)) @@ -410,10 +478,10 @@ def main(argv: list[str] | None = None) -> int: status = "FAIL" if hits else "ok" print(f"[{status:4}] {name:12} {len(hits)} issue(s)") for h in hits: - print(f" {h}") + print(f" {printable(h)}") # After the findings and outside the count, since a note is not one. for note in NOTES: - print(f" note: {note}") + print(f" note: {printable(note)}") total += len(hits) return 1 if total else 0 diff --git a/scripts/tests/test_repo_gate.py b/scripts/tests/test_repo_gate.py index 0ce17d69..be6a25c8 100755 --- a/scripts/tests/test_repo_gate.py +++ b/scripts/tests/test_repo_gate.py @@ -12,6 +12,7 @@ import contextlib import io +import os import re import shutil import subprocess @@ -467,6 +468,81 @@ def test_the_note_carries_every_count_including_the_zeroes(self) -> None: ) +class TestQuotedNames(GitTreeCase): + """A tracked name git quotes reaches every check as the name on disk. + + The constructed name holds a byte that is not valid UTF-8, the case that both crashed the + listing under `core.quotePath=false` and, at the default, left an escaped spelling no file + answers to. + """ + + def setUp(self) -> None: + super().setUp() + try: + self.NAME = os.fsdecode(b"run-\xff-tool") + (self.tmp / self.NAME).write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + except (OSError, ValueError): + self.skipTest("this filesystem refuses a name that is not valid UTF-8") + + def test_an_inherited_quote_path_false_lists_the_real_name_rather_than_raising(self) -> None: + self.git("config", "core.quotePath", "false") + self.git("add", "-A") + self.assertEqual([self.NAME], repo_gate.tracked(self.tmp)) + + def test_a_quoted_shebang_name_is_still_checked(self) -> None: + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + hits = self.coverage(gitattributes, {}) + self.assertIn(self.NAME, repo_gate.tracked(self.tmp)) + self.assertTrue(any(f"{self.NAME}: tracked shebang path" in hit for hit in hits), hits) + + def test_the_run_prints_such_a_name_under_a_strict_output_encoding(self) -> None: + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + self.coverage(gitattributes, {}) + out = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + err = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = repo_gate.main(["--root", str(self.tmp), "--check", "eol-coverage"]) + out.flush() + self.assertEqual(1, rc) + self.assertIn(b"run-\\udcff-tool: tracked shebang path", out.buffer.getvalue()) + + def test_a_missing_root_named_in_bytes_that_are_not_utf8_fails_without_raising(self) -> None: + missing = self.tmp / self.NAME / "gone" + err = io.TextIOWrapper(io.BytesIO(), encoding="utf-8", errors="strict") + with contextlib.redirect_stderr(err): + self.assertEqual([], repo_gate.tracked(missing)) + err.flush() + self.assertIn(b"git ls-files failed", err.buffer.getvalue()) + self.assertIn(b"run-\\udcff-tool", err.buffer.getvalue()) + + +class TestQuotedPlainNames(GitTreeCase): + def test_a_name_quoted_for_a_quote_or_backslash_decodes_to_itself(self) -> None: + odd = 'say "hi"\\now' + try: + (self.tmp / odd).write_text("x\n", encoding="utf-8") + except OSError: + self.skipTest("this filesystem refuses a quote or a backslash in a name") + self.git("add", "-A") + self.assertIn(odd, repo_gate.tracked(self.tmp)) + + def test_a_control_character_in_a_name_prints_escaped_rather_than_raw(self) -> None: + name = "run\n[ok ] forged\x1b[2J" + gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1) + try: + (self.tmp / name).write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + except OSError: + self.skipTest("this filesystem refuses a control character in a name") + self.coverage(gitattributes, {}) + out = io.StringIO() + with contextlib.redirect_stdout(out): + rc = repo_gate.main(["--root", str(self.tmp), "--check", "eol-coverage"]) + self.assertEqual(1, rc) + self.assertIn("run\\x0a[ok ] forged\\x1b[2J: tracked shebang path", out.getvalue()) + self.assertNotIn("\x1b", out.getvalue()) + self.assertFalse(any(l.startswith("[ok ] forged") for l in out.getvalue().splitlines())) + + class TestGovernanceCoupling(unittest.TestCase): def test_the_exception_set_matches_what_the_doc_documents(self) -> None: """The doc calls it the one documented exception, so the code must not carry a second."""