From 2d843fed14312cae53d2da406c155dd7a1a5919e Mon Sep 17 00:00:00 2001 From: Chris Graf Date: Wed, 1 Jul 2026 20:44:31 -0500 Subject: [PATCH] Harden the registry dispatch-table parser against benign reformatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary Make the registry validator's shell dispatch scan tolerant of cosmetic formatting changes so a purely stylistic edit to the dispatchers can no longer brick every format/lint/plan run. `_shell_dispatch_functions` reconstructs the adapter-to-function dispatch table by scraping the `case` arms of `_format_dispatch`/`_lint_dispatch` in `autoformat.sh`/`autolint.sh`, and it runs on EVERY `load_registry` (so on every `checkrun` format/lint/plan/explain, including the commit hook). The old scan required the handler on the same line as the `pattern)` label and detected the function end with an exact, unindented `}`. A formatting-only change — moving a handler to the next line, indenting `esac`, a shell formatter splitting an arm — could silently drop arms or empty the table, which then raises `RegistryError` and bricks the tool. - rewrite the scan to accept the handler on the same or a following line, tolerate blank lines between a label and its handler, and stop at an `esac`/closing brace regardless of indentation - preserve the loud failure on an unreadable (empty) table: the parser never silently returns an empty mapping that would make every selector look undispatched, and a wrong function value is still caught loudly by the existing `_validate_invariants` dispatch-mismatch check - add a prominent WHY-comment: this still derives control flow from shell source (a coupling the durable fix removes by making the dispatch mapping data the registry owns in `registry.json`), documents the reformats that are tolerated, and notes that alternation (`a|b)`) and stacked bare labels are intentionally unsupported and fail loudly - leave `_shell_functions` (an existence-set check whose miss surfaces loudly as "adapter is not implemented") unchanged as lower-risk The new parser is byte-identical to the old one on the production dispatchers (21 format arms, 27 lint arms, same keys and values). Testing - new `registry-test` case: a reformatted-but-equivalent dispatcher (next-line handlers, blank lines, indented `esac`) parses to the exact expected table, and an unreadable/empty dispatcher raises `RegistryError` with the "could not be read" message - verified red-green: the old same-line-only parser yields an empty table (which raises) on the reformatted fixture - full suite green (`./test/checkrun-test`: ok); `checkrun lint` clean --- lib/checkrun/registry.py | 52 ++++++++++++++++++++---- test/suites/registry-test | 83 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 7 deletions(-) diff --git a/lib/checkrun/registry.py b/lib/checkrun/registry.py index c194eaf..91fcb77 100755 --- a/lib/checkrun/registry.py +++ b/lib/checkrun/registry.py @@ -105,8 +105,25 @@ def _shell_dispatch_functions(phase: str) -> dict[str, str]: # cross the Python-to-shell boundary. Function existence alone is not enough: # a custom registry can point at a real helper such as `_lint_ruff`, but the # shell entrypoint still needs an adapter arm that calls that same helper with - # the right arguments. This narrow parser intentionally supports Checkrun's - # one-line dispatch arms instead of trying to understand arbitrary shell. + # the right arguments. + # + # WHY THIS PARSES SHELL: deriving control flow (the case -> function mapping) + # by scraping shell source is inherently coupled to how that source is + # formatted, and this runs on EVERY registry load — so a purely cosmetic edit + # to the dispatchers could otherwise brick all format/lint/plan. The durable + # fix is to make the dispatch mapping data the registry owns (declared in + # registry.json, validated by presence) rather than re-derived from shell; + # until then this scanner is deliberately tolerant of the benign reformats a + # shell formatter or a human introduces without changing behavior: + # - the handler on the same line as the pattern: `ruff-format) _f ... ;;` + # - the handler on the following line(s): `ruff-format)\n _f ...` + # - an indented `esac`/closing brace. + # It intentionally does NOT understand alternation (`a|b)`) or stacked bare + # labels sharing one handler; Checkrun's dispatchers use one adapter id per + # arm, and such a rewrite fails LOUDLY (empty/short table below) rather than + # mapping an arm to the wrong function. Anything it genuinely cannot read + # likewise fails loud rather than dropping arms silently, and is covered by + # regression tests. try: source, function = _SHELL_DISPATCH[phase] except KeyError as exc: @@ -118,7 +135,10 @@ def _shell_dispatch_functions(phase: str) -> dict[str, str]: in_function = False dispatch: dict[str, str] = {} - arm = re.compile(r"^\s+([A-Za-z0-9_.+-]+)\)\s+([A-Za-z_][A-Za-z0-9_]*)\b") + pending: list[str] = [] # case patterns whose handler is on a later line + inline = re.compile(r"^\s*([A-Za-z0-9_.+-]+)\)\s+([A-Za-z_][A-Za-z0-9_]*)\b") + label = re.compile(r"^\s*([A-Za-z0-9_.+-]+)\)\s*$") + command = re.compile(r"^\s*([A-Za-z_][A-Za-z0-9_]*)\b") for line in lines: if not in_function: stripped = line.strip() @@ -127,11 +147,29 @@ def _shell_dispatch_functions(phase: str) -> dict[str, str]: ): in_function = True continue - if line == "}": + stripped = line.strip() + if stripped == "": + continue # blank line: any pending pattern keeps waiting for its handler + # Arms live only inside the case block, so stop at its end (or the + # function's) regardless of indentation. + if stripped in ("}", "esac"): break - match = arm.match(line) - if match and match.group(1) != "*": - dispatch[match.group(1)] = match.group(2) + match = inline.match(line) + if match: + pending = [] # a new arm supersedes any orphaned label above it + if match.group(1) != "*": + dispatch[match.group(1)] = match.group(2) + continue + match = label.match(line) + if match: + pending = [match.group(1)] if match.group(1) != "*" else [] + continue + if pending: + match = command.match(line) + if match: + for adapter_id in pending: + dispatch[adapter_id] = match.group(1) + pending = [] if not dispatch: raise RegistryError(f"{phase}: dispatch adapter table could not be read") return dispatch diff --git a/test/suites/registry-test b/test/suites/registry-test index 619acbd..507abc5 100755 --- a/test/suites/registry-test +++ b/test/suites/registry-test @@ -533,6 +533,89 @@ else: PY _assert_exit "registry: unknown shell dispatch adapter is rejected" 0 "$?" +echo "=== checkrun: dispatch parser tolerates benign reformatting ===" + +# The dispatch table is re-derived from shell source on every registry load, so +# a purely cosmetic reformat of the dispatchers must not change what the parser +# reads (or it would brick all format/lint/plan). Assert the parser is tolerant +# of the realistic reformats, and that an unreadable table still fails LOUDLY +# instead of silently returning an empty mapping. +python3 - "$CHECKRUN_REPO_ROOT" "$_td/reformatted-dispatch.sh" "$_td/empty-dispatch.sh" <<'PY' +import importlib.util +import sys +from pathlib import Path + +root = Path(sys.argv[1]) +reformatted = Path(sys.argv[2]) +empty = Path(sys.argv[3]) + +spec = importlib.util.spec_from_file_location("checkrun_registry", root / "lib/checkrun/registry.py") +mod = importlib.util.module_from_spec(spec) +assert spec.loader is not None +spec.loader.exec_module(mod) + +# Semantically identical to the inline style, but reformatted the way a shell +# formatter or a human edit might: handler on the line after the pattern, extra +# blank lines, and an indented `esac`. +reformatted.write_text( + ''' +_format_dispatch() { + local adapter="$1" + case "$adapter" in + ruff-format) + _format_ruff "$file" "$dir" ;; + + rustfmt) + _format_rustfmt "$file" ;; + *) + echo "unknown" >&2 + return 125 + ;; + esac +} + +_lint_dispatch() { + case "$adapter" in + shellcheck) _lint_sh "$file" ;; + ruff-lint) + _lint_ruff "$file" ;; + esac +} +''', + encoding="utf-8", +) +mod._SHELL_DISPATCH = { + "format": (reformatted, "_format_dispatch()"), + "lint": (reformatted, "_lint_dispatch()"), +} +fmt = mod._shell_dispatch_functions("format") +assert fmt == {"ruff-format": "_format_ruff", "rustfmt": "_format_rustfmt"}, fmt +lint = mod._shell_dispatch_functions("lint") +assert lint == {"shellcheck": "_lint_sh", "ruff-lint": "_lint_ruff"}, lint + +# A dispatcher with no readable arms must fail loudly, never silently return an +# empty table that would make every selector look undispatched. +empty.write_text( + ''' +_format_dispatch() { + local adapter="$1" + case "$adapter" in + *) return 125 ;; + esac +} +''', + encoding="utf-8", +) +mod._SHELL_DISPATCH = {"format": (empty, "_format_dispatch()")} +try: + mod._shell_dispatch_functions("format") +except mod.RegistryError as error: + assert "could not be read" in str(error), str(error) +else: + raise AssertionError("empty dispatch table was accepted") +PY +_assert_exit "registry: dispatch parser tolerates reformatting and fails loudly when empty" 0 "$?" + cp "$REGISTRY" "$_td/registry.json" python3 - "$_td/registry.json" <<'PY' import json