Pin and Decode Git's Quoting in repo_gate.py's ls-files Read

.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
 

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."""