Stop the Doc Gates' Printed Names Forging Workflow Commands

.github/actions/prose-gate/action.yml

@@ -84,7 +84,9 @@ runs:
             excluded_paths+=("$excluded")
             exclude_args+=(--exclude "$excluded")
           done < "$exclude_file"
-          echo "Excluding ${#excluded_paths[@]} path(s) from $exclude_file: ${excluded_paths[*]}"
+          inert_excludes="${excluded_paths[*]}"
+          inert_excludes="${inert_excludes//$'\r'/'\x0d'}"
+          echo "Excluding ${#excluded_paths[@]} path(s) from $exclude_file: ${inert_excludes//'##['/'##\x5b'}"
         fi
         # The context is preferred and the default environment variable is the fallback, so a reference form that populates only one of them still names the pin.
         # Naming a repository with no ref, or a ref with no repository, would be worse than naming nothing, since the value exists to identify one commit.

.github/actions/prose-gate/prose_lint.py

@@ -899,6 +899,26 @@ def quoted(paths, cap: int | None = None) -> str:
     return ", ".join(repr(str(p)) for p in shown) + more
 
 
+CONTROL = re.compile(r"[\x00-\x1f\x7f-\x9f]")
+COMMAND_LEAD = re.compile(r"\A(\s*):(?=:)")
+LEGACY_COMMAND = "##["
+
+
+def printable(line: str) -> str:
+    """`line` spelled so it prints as one line of this gate's output and nothing else.
+
+    A tracked name and a file's own text both reach a finding, and either may hold a newline, an
+    escape sequence, or a workflow command. The Actions runner executes a line opening on `::`
+    once leading whitespace is trimmed, and a line holding `##[` anywhere. So each C0 or C1
+    control and DEL is escaped, as are the colon opening such a line and the bracket of each
+    `##[`. A lone surrogate from a name that is not UTF-8 is escaped too, for a strict stream.
+    """
+    escaped = CONTROL.sub(lambda m: f"\\x{ord(m.group()):02x}", line)
+    escaped = COMMAND_LEAD.sub(lambda m: f"{m.group(1)}\\x3a", escaped)
+    escaped = escaped.replace(LEGACY_COMMAND, "##\\x5b")
+    return escaped.encode("utf-8", "backslashreplace").decode("utf-8")
+
+
 def repo_root(path: Path) -> str:
     """The repository top level containing `path`, or '' when git cannot say."""
     start = path if path.is_dir() else path.parent
@@ -1001,7 +1021,9 @@ def discover(
         # It also printed that git could not describe a tree git describes fine.
         if candidates is None and not repo_root(base):
             print(
-                f"warning: git cannot describe {base}, falling back to a filesystem walk",
+                printable(
+                    f"warning: git cannot describe {base}, falling back to a filesystem walk"
+                ),
                 file=sys.stderr,
             )
             # A walk reports what is on disk, so it carries the untracked files already.
@@ -3288,18 +3310,22 @@ def main(argv: list[str] | None = None) -> int:
     if unusable:
         # Quoted, since a path holding a space or a comma is unreadable in a bare comma-joined list.
         print(
-            f"error: requested path(s) are not a file or a directory: {quoted(unusable)}. "
-            "Refusing rather than falling back to the current directory, which would scan one "
-            "tree and choose the rule set from another.",
+            printable(
+                f"error: requested path(s) are not a file or a directory: {quoted(unusable)}. "
+                "Refusing rather than falling back to the current directory, which would scan "
+                "one tree and choose the rule set from another."
+            ),
             file=sys.stderr,
         )
         return 2
     git_roots = {found for found in (repo_root(Path(p)) for p in scan_paths) if found}
     if len(git_roots) > 1:
         print(
-            f"error: the requested paths span more than one repository ({quoted(git_roots)}). "
-            "Each declares its own workflow model, so no single rule set is correct for all of "
-            "them. Run the gate once per repository.",
+            printable(
+                f"error: the requested paths span more than one repository ({quoted(git_roots)}). "
+                "Each declares its own workflow model, so no single rule set is correct for all "
+                "of them. Run the gate once per repository."
+            ),
             file=sys.stderr,
         )
         return 2
@@ -3355,7 +3381,7 @@ def main(argv: list[str] | None = None) -> int:
 
     if a.list_files:
         for f in files:
-            print(keys[f])
+            print(printable(keys[f]))
         return 0
 
     discovered = len(files)
@@ -3368,10 +3394,12 @@ def main(argv: list[str] | None = None) -> int:
         # A CI adoption hits this first, where an unresolvable base walls off the first run.
         # Scoping to nothing instead would report a false clean, so neither default is honest.
         print(
-            f"error: cannot diff against {a.diff!r}, so the run cannot be scoped to changed "
-            "lines. Refusing to scan the whole tree instead, since that reports the existing "
-            "backlog as though this change introduced it. Check the ref exists and that the "
-            "checkout carries its history.",
+            printable(
+                f"error: cannot diff against {a.diff!r}, so the run cannot be scoped to changed "
+                "lines. Refusing to scan the whole tree instead, since that reports the existing "
+                "backlog as though this change introduced it. Check the ref exists and that the "
+                "checkout carries its history."
+            ),
             file=sys.stderr,
         )
         return 2
@@ -3390,12 +3418,14 @@ def main(argv: list[str] | None = None) -> int:
             if unread:
                 shown = quoted(unread, 5)
                 print(
-                    f"error: the diff against {a.diff!r} names {len(unread)} readable file(s) "
-                    f"this run was asked about, and the scan matched none of them: {shown}. "
-                    "Refusing to report a clean run, since a gate that read nothing is "
-                    "indistinguishable from a gate with nothing to read. Check that the run "
-                    "starts at the repository top level and that the requested paths cover "
-                    "the change.",
+                    printable(
+                        f"error: the diff against {a.diff!r} names {len(unread)} readable "
+                        f"file(s) this run was asked about, and the scan matched none of them: "
+                        f"{shown}. Refusing to report a clean run, since a gate that read nothing "
+                        "is indistinguishable from a gate with nothing to read. Check that the "
+                        "run starts at the repository top level and that the requested paths "
+                        "cover the change."
+                    ),
                     file=sys.stderr,
                 )
                 return 2
@@ -3415,18 +3445,20 @@ def main(argv: list[str] | None = None) -> int:
             bykind[kind] = bykind.get(kind, 0) + 1
             byfile[keys[f]] = byfile.get(keys[f], 0) + 1
             if not a.summary:
-                print(f"{keys[f]}:{ln}: {kind}: {msg}")
+                print(printable(f"{keys[f]}:{ln}: {kind}: {msg}"))
 
     inscope = sum(len(scope[keys[f]]) for f in files) if scope is not None else None
     print(
-        scope_note(
-            len(files),
-            discovered,
-            inscope,
-            a.diff,
-            gate_provenance(a.provenance),
-            [keys[f] for f in refused],
-            [keys[f] for f in undecoded],
+        printable(
+            scope_note(
+                len(files),
+                discovered,
+                inscope,
+                a.diff,
+                gate_provenance(a.provenance),
+                [keys[f] for f in refused],
+                [keys[f] for f in undecoded],
+            )
         ),
         file=sys.stderr,
     )
@@ -3436,7 +3468,7 @@ def main(argv: list[str] | None = None) -> int:
             print(f"  {k:16} {v}", file=sys.stderr)
         if a.summary:
             for k, v in sorted(byfile.items(), key=lambda kv: -kv[1])[:15]:
-                print(f"  {v:5}  {k}", file=sys.stderr)
+                print(printable(f"  {v:5}  {k}"), file=sys.stderr)
     return 1 if total else 0
 
 

.github/actions/repo-gate/repo_gate.py

@@ -56,6 +56,8 @@
 ABSENT = {"404", "422"}
 GH_TIMEOUT = 20
 CONTROL = re.compile(r"[\x00-\x1f\x7f-\x9f]")
+COMMAND_LEAD = re.compile(r"\A(\s*):(?=:)")
+LEGACY_COMMAND = "##["
 
 # 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.
@@ -783,8 +785,14 @@ def printable(line: str) -> str:
     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.
+
+    The Actions runner executes a line opening on `::` once leading whitespace is trimmed. It also
+    executes a line holding `##[` anywhere. A name shaped like either forges a workflow command.
+    The colon opening such a line and the bracket of each `##[` are escaped for that reason.
     """
     escaped = CONTROL.sub(lambda m: f"\\x{ord(m.group()):02x}", line)
+    escaped = COMMAND_LEAD.sub(lambda m: f"{m.group(1)}\\x3a", escaped)
+    escaped = escaped.replace(LEGACY_COMMAND, "##\\x5b")
     return escaped.encode("utf-8", "backslashreplace").decode("utf-8")
 
 

tests/runner_commands.py

@@ -0,0 +1,35 @@
+"""Which output lines the GitHub Actions runner would read as a workflow command.
+
+Modeled on the runner's own two parsers rather than on the escaping a gate applies. A test
+asserting through it measures the runner's reading of the output, not the gate's intent. The
+current form opens on `::` once leading whitespace is trimmed. The legacy form is `##[` anywhere
+in the line.
+
+`FORGING_NAMES` holds constructed names, one per way a line reaches either parser. The
+ideographic space is whitespace the runner's trim removes, as is the plain indent a gate prints.
+`FORGING_LINES` adds edge shapes, and `ORDINARY_LINES` holds lines a gate must print unchanged.
+"""
+
+from __future__ import annotations
+
+FORGING_NAMES = (
+    "::error title=forged::all checks passed",
+    "\u3000::warning::forged",
+    "run-##[error]forged",
+)
+FORGING_LINES = (*FORGING_NAMES, "  ::add-mask::x", "::", "a::b ##[ ##[")
+ORDINARY_LINES = ("docs/a.md:3: dupword: x", "a::b: mid-line colons", "# [heading]")
+
+
+def opens_a_command(line: str) -> bool:
+    """Whether the runner would hand `line` to a command parser at all."""
+    return line.lstrip().startswith("::") or "##[" in line
+
+
+def forged_lines(output: str) -> list[str]:
+    """Every line of `output` the runner would read as a command.
+
+    `str.splitlines` splits on more boundaries than the runner's line reader does. That only ever
+    adds candidate lines, so a clean answer here is a clean answer there.
+    """
+    return [line for line in output.splitlines() if opens_a_command(line)]

tests/test_prose_lint.py

@@ -30,6 +30,7 @@
 from unittest import mock
 
 from host_capability import bash_or_skip
+from runner_commands import FORGING_LINES, FORGING_NAMES, ORDINARY_LINES, forged_lines
 from workflow_steps import step_script, yaml_step, yaml_value
 
 try:
@@ -4432,6 +4433,16 @@ def test_a_declared_vendored_path_does_not_block_the_gate(self) -> None:
         self.assertEqual(0, result.returncode, result.stdout + result.stderr)
         self.assertIn("Excluding 1 path(s)", result.stdout)
 
+    def test_an_exclusion_shaped_like_a_command_is_echoed_inert(self) -> None:
+        (self.root / ".github" / "prose-gate-excludes").write_bytes(
+            b"vendor/x##[error]forged\nvendor/a\r::error::forged\n"
+        )
+        result = self.run_action()
+        self.assertEqual(0, result.returncode, result.stdout + result.stderr)
+        self.assertEqual([], forged_lines(result.stdout + result.stderr))
+        self.assertIn("vendor/x##\\x5berror]forged", result.stdout)
+        self.assertIn("vendor/a\\x0d::error::forged", result.stdout)
+
     def test_an_empty_contract_is_no_exclusions(self) -> None:
         (self.root / ".github" / "prose-gate-excludes").write_bytes(b"")
         result = self.run_action()
@@ -5397,6 +5408,68 @@ def test_a_filename_that_is_not_utf8_is_scanned_and_reported(self) -> None:
         self.assertNotIn("UnicodeEncodeError", done.stderr)
 
 
+class TestNamesCannotForgeWorkflowCommands(unittest.TestCase):
+    """A tracked name or file text shaped like a workflow command prints as inert text.
+
+    The finding line opens on the tracked name with no indent at all. The runner reads both of
+    the gate's streams, so each is checked, on every output mode that prints a name.
+    """
+
+    NAMES = (*FORGING_NAMES, "split.md\n::error::forged")
+    UNDECODED = "run-##[error]undecoded.md"
+
+    def test_printable_leaves_no_line_a_command_parser_reads(self) -> None:
+        for line in FORGING_LINES:
+            with self.subTest(line=line):
+                self.assertEqual([], forged_lines(prose_lint.printable(line)))
+
+    def test_printable_leaves_an_ordinary_line_as_it_was(self) -> None:
+        for line in ORDINARY_LINES:
+            with self.subTest(line=line):
+                self.assertEqual(line, prose_lint.printable(line))
+
+    def tree(self) -> Path:
+        root = Path(self.enterContext(tempfile.TemporaryDirectory()))
+        subprocess.run(["git", "init", "-q", str(root)], check=True)
+        try:
+            for name in self.NAMES:
+                (root / f"{name}.md").write_text("It has a dupword dupword in it.\n", "utf-8")
+            (root / self.UNDECODED).write_bytes(b"caf\xe9\n")
+        except OSError:
+            self.skipTest("this filesystem refuses a colon, a bracket, or a newline in a name")
+        subprocess.run(["git", "-C", str(root), "add", "-A"], check=True)
+        return root
+
+    def run_gate(self, root: Path, *args: str) -> subprocess.CompletedProcess[str]:
+        done = subprocess.run(
+            [sys.executable, str(PROSE_LINT_SCRIPT), str(root), "--check", "dupword", *args],
+            capture_output=True,
+            encoding="utf-8",
+            env={**os.environ, "PYTHONIOENCODING": "utf-8"},
+            check=False,
+            cwd=root,
+        )
+        self.assertEqual([], forged_lines(done.stdout))
+        self.assertEqual([], forged_lines(done.stderr))
+        return done
+
+    def test_a_forging_name_raises_its_finding_without_forging_a_command(self) -> None:
+        done = self.run_gate(self.tree())
+        self.assertEqual(1, done.returncode, done.stderr)
+        self.assertEqual(len(self.NAMES), done.stdout.count(": dupword: "), done.stdout)
+        self.assertIn("\\x3a:error title=forged::all checks passed.md:1: dupword", done.stdout)
+        self.assertIn("split.md\\x0a::error::forged.md:1: dupword", done.stdout)
+        self.assertIn("run-##\\x5berror]undecoded.md", done.stderr)
+
+    def test_the_summary_and_the_file_listing_print_a_forging_name_inert(self) -> None:
+        root = self.tree()
+        summary = self.run_gate(root, "--summary")
+        self.assertIn("run-##\\x5berror]forged.md", summary.stderr)
+        listing = self.run_gate(root, "--list-files")
+        self.assertEqual(0, listing.returncode, listing.stderr)
+        self.assertIn("\\x3a:error title=forged::all checks passed.md", listing.stdout)
+
+
 class TestDiffScopeReachesAQuotedName(unittest.TestCase):
     """A name git quotes in a diff header must still land in scope.
 

tests/test_repo_gate.py

@@ -27,6 +27,7 @@
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[1] / ".github/actions/repo-gate"))
 import repo_gate
+from runner_commands import FORGING_LINES, FORGING_NAMES, ORDINARY_LINES, forged_lines
 
 REPO = Path(__file__).resolve().parent.parent
 GOVERNANCE = REPO / "GOVERNANCE.md"
@@ -586,6 +587,46 @@ def test_a_control_character_in_a_name_prints_escaped_rather_than_raw(self) -> N
         self.assertFalse(any(l.startswith("[ok  ] forged") for l in out.getvalue().splitlines()))
 
 
+class TestNamesCannotForgeWorkflowCommands(GitTreeCase):
+    """A tracked name shaped like a workflow command prints as text the runner does not execute."""
+
+    def test_printable_leaves_no_line_a_command_parser_reads(self) -> None:
+        for line in FORGING_LINES:
+            with self.subTest(line=line):
+                self.assertEqual([], forged_lines(f"         {repo_gate.printable(line)}"))
+
+    def test_printable_leaves_an_ordinary_line_as_it_was(self) -> None:
+        for line in ORDINARY_LINES:
+            with self.subTest(line=line):
+                self.assertEqual(line, repo_gate.printable(line))
+
+    def test_both_gates_escape_a_line_identically(self) -> None:
+        """Each gate is fetched as one file, so each holds its own copy, and this keeps them equal."""
+        prose_gate = REPO / ".github/actions/prose-gate"
+        with mock.patch.object(sys, "path", [str(prose_gate), *sys.path]):
+            prose_lint = importlib.import_module("prose_lint")
+        for line in (*FORGING_LINES, *ORDINARY_LINES, "tab\there", "\x85::x", "bad\udcffbyte"):
+            with self.subTest(line=line):
+                self.assertEqual(repo_gate.printable(line), prose_lint.printable(line))
+
+    def test_a_forging_name_raises_its_finding_without_forging_a_command(self) -> None:
+        gitattributes = TestEolCoverage.GITATTRIBUTES.replace("eol=lf", "eol=crlf", 1)
+        tree = {name: "#!/bin/sh\nexit 0\n" for name in FORGING_NAMES}
+        try:
+            self.coverage(gitattributes, tree)
+        except OSError:
+            self.skipTest("this filesystem refuses a colon, a bracket, or a wide space in a name")
+        out = io.StringIO()
+        with contextlib.redirect_stdout(out):
+            rc = repo_gate.main(["--root", str(self.tmp), "--check", "eol-coverage"])
+        self.assertEqual(1, rc)
+        self.assertEqual([], forged_lines(out.getvalue()))
+        self.assertIn(
+            "\\x3a:error title=forged::all checks passed: tracked shebang", out.getvalue()
+        )
+        self.assertIn("run-##\\x5berror]forged: tracked shebang", out.getvalue())
+
+
 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."""