From 909c2e51547acd1ea9b3328cd857230c8bd2ec5c Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Mon, 5 Oct 2026 09:23:03 +0700 Subject: [PATCH] ci(tools): parse the scripts inside single-quoted bash -c arguments check_run_syntax ran bash -n on each run: script, and its own docstring said what that misses: a `docker run ... bash -c '...'` script is one string to the outer shell, so an error inside it was not seen unless it broke the outer quoting. (The apostrophe that broke run 37253683063 did break the outer quoting, and main's gate already reports it; an unclosed `if` or a stray `fi` inside the string it does not.) Once the outer script parses, each single-quoted `bash -c '...'` and `sh -c '...'` argument is now read out the way the outer shell reads it -- '\'' is a quote, a splice of an outer value ('"$PART"', '$X') is one word -- and parsed with bash -n / sh -n. A closing quote glued to anything else, and a quote that never closes, are "cannot tell" (exit 2), not clean. Double-quoted bash -c "..." is not read: the outer shell expands it first. Measured on main's 122 workflows: 24 such scripts, 8 of them with '"$VAR"' splices, all 24 parse. The self-test plants 12 new cases (75 of 75 pass); main's gate run against the new cases fails the 7 that expect a non-zero exit. Co-Authored-By: Claude Opus 5.5 --- tools/check_run_syntax.py | 86 ++++++++++++++++++++++++++++++++-- tools/test_check_run_syntax.py | 29 ++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/tools/check_run_syntax.py b/tools/check_run_syntax.py index 18d0c1bd5..da316bb96 100644 --- a/tools/check_run_syntax.py +++ b/tools/check_run_syntax.py @@ -26,7 +26,13 @@ `bash -n` reads syntax only: a script inside a quoted `bash -c '...'` argument is one string to it, so an error INSIDE that string is not seen -- unless, as -in both cases above, it breaks the quoting of the outer script. +in both cases above, it breaks the quoting of the outer script. So once the +outer script parses, every single-quoted `bash -c '...'` / `sh -c '...'` +argument in it (the `docker run ... bash -c '` scripts, 24 on main) is read +out as the outer shell reads it -- `'\''` is a quote, and a splice of an outer +value, `'"$PART"'` or `'$X'`, is one word -- and parsed with `bash -n` / `sh -n` +in turn. A quote followed by anything else is "cannot tell". A double-quoted +`bash -c "..."` is not read: the outer shell expands it first. Exit 0 clean, 1 a step that does not parse, 2 a workflow this could not read (cannot tell is not clean). tools/test_check_run_syntax.py plants every case. @@ -165,7 +171,58 @@ def _shell_named(step: dict, job: dict, wf: dict) -> tuple[str, ...]: return () # pwsh, python, cmd, a custom shell: not ours to parse +# `bash -c '`, `sh -lc '`, `bash -euo pipefail -c '`: the opening quote of a +# script argument. `\b` keeps `zsh -c` and `fish -c` out. +INNER_OPEN = re.compile(r"\b(bash|sh)(?:[ \t]+(?:-[a-zA-Z]*o[ \t]+[a-z]+|-[a-zA-Z]+))*?" + r"[ \t]+-[a-zA-Z]*c[a-zA-Z]*[ \t]+'") +# What may sit between a closing quote and the reopening one: an outer value. +SPLICE = re.compile(r'"(?:[^"\\]|\\.)*"|\$\{[^}]*\}|\$[A-Za-z_][A-Za-z0-9_]*') +# Stands in for a spliced value: bash -n needs a word there, not its contents. +OUTER_VALUE = "__outer__" +WORD_BREAK = " \t\n;|&)" +# How many -c scripts check() parsed since main() started, for its summary line. +_INNER_SEEN = 0 + + +def inner_scripts(script: str) -> list[tuple[str, str]]: + """(interpreter, text) of every single-quoted -c script argument in `script`.""" + found = [] + for m in INNER_OPEN.finditer(script): + i, buf = m.end(), [] + while True: + j = script.find("'", i) + unclosed = j < 0 + if unclosed: + raise CannotTell(f"a {m.group(1)} -c '...' script with no closing quote") + buf.append(script[i:j]) + k = j + 1 + escaped_quote = script.startswith("\\''", k) # '\'' : a quote, then reopen + if escaped_quote: + buf.append("'") + i = k + 3 + continue + spliced = False + s = SPLICE.match(script, k) + while s: + spliced = True + k = s.end() + s = SPLICE.match(script, k) + if spliced: + buf.append(OUTER_VALUE) + reopens = script.startswith("'", k) + if reopens: + i = k + 1 + continue + ends = k == len(script) or script[k] in WORD_BREAK + if not ends: + raise CannotTell(f"a {m.group(1)} -c '...' script is glued to {script[k:k + 12]!r}") + break + found.append((m.group(1), "".join(buf))) + return found + + def check(path: pathlib.Path) -> tuple[list[str], str | None]: + global _INNER_SEEN import yaml # inside: a missing PyYAML is "cannot tell", not a crash try: wf = yaml.safe_load(path.read_text()) @@ -186,13 +243,31 @@ def check(path: pathlib.Path) -> tuple[list[str], str | None]: shells = shell_of(step, job, wf) except CannotTell as e: return [], f"{path.name} :: {jn} :: step {k}: {e}" + where = f"{path.relative_to(ROOT)} :: {jn} :: {step.get('name') or f'step {k}'}" + outer_parses = bool(shells) for sh in shells: r = subprocess.run([sh, "-n"], input=str(step["run"]), capture_output=True, text=True) if r.returncode: err = " ".join(r.stderr.split())[:240] - bad.append(f"{path.relative_to(ROOT)} :: {jn} :: {step.get('name') or f'step {k}'}" - f" [{sh} -n]\n {err}") + bad.append(f"{where} [{sh} -n]\n {err}") + outer_parses = False break + # Only a script whose own quoting is sound can be read for the strings + # it hands to bash -c; a broken one was reported above. + if not outer_parses: + continue + try: + inner = inner_scripts(str(step["run"])) + except CannotTell as e: + return [], f"{path.name} :: {jn} :: step {k}: {e}" + _INNER_SEEN += len(inner) + for n, (ish, text) in enumerate(inner, 1): + r = subprocess.run([ish, "-n"], input=text, capture_output=True, text=True) + inner_bad = r.returncode != 0 + if inner_bad: + err = " ".join(r.stderr.split())[:240] + bad.append(f"{where} [{ish} -n on {ish} -c script {n}, lines from its opening quote]" + f"\n {err}") return bad, None @@ -206,6 +281,8 @@ def main() -> int: if not shutil.which(sh): print(f"cannot tell: no {sh} on PATH") return 2 + global _INNER_SEEN + _INNER_SEEN = 0 files = sorted(list(WORKFLOWS.glob("*.yml")) + list(WORKFLOWS.glob("*.yaml"))) bad, unread = [], [] for f in files: @@ -213,7 +290,8 @@ def main() -> int: bad += b if err: unread.append(err) - print(f"workflows read: {len(files) - len(unread)} of {len(files)}; steps that do not parse: {len(bad)}") + print(f"workflows read: {len(files) - len(unread)} of {len(files)}; bash -c '...' scripts read out" + f" of them: {_INNER_SEEN}; steps and scripts that do not parse: {len(bad)}") for b in bad: print(" " + b) for u in unread: diff --git a/tools/test_check_run_syntax.py b/tools/test_check_run_syntax.py index 80f044c0e..6f4ab2ecb 100644 --- a/tools/test_check_run_syntax.py +++ b/tools/test_check_run_syntax.py @@ -34,6 +34,35 @@ def git_show(rev_path: str) -> str | None: ("'\\'' inside single quotes is a quote", 0, H + " steps:\n - run: |\n bash -c 'grep -oE '\\''A.+'\\'' x'\n"), ("an unclosed if", 1, H + " steps:\n - run: |\n if true; then\n echo x\n"), + # Inside a single-quoted -c script: one string to the outer bash -n. + ("an unclosed if inside bash -c '...'", 1, H + + " steps:\n - run: |\n docker run img bash -c '\n" + " if true; then\n make\n '\n"), + ("a clean bash -c '...' script", 0, H + + " steps:\n - run: |\n docker run img bash -c '\n" + " if true; then\n make\n fi\n '\n"), + ("'\"$X\"' splices an outer value as one word", 0, H + + " steps:\n - run: |\n docker run img bash -c '\n" + " git checkout '\"$REV\"' && make /w/'\"$PART\"'.bin\n '\n"), + ("an error after a splice is still seen", 1, H + + " steps:\n - run: |\n docker run img bash -c '\n" + " git checkout '\"$REV\"'\n if true; then make\n '\n"), + ("a bare '$X' splice is one word", 0, H + + " steps:\n - run: |\n bash -c 'echo '$X' '${Y}' done'\n"), + ("sh -c '...' is checked with sh", 1, H + + " steps:\n - run: |\n docker run img sh -c 'cat <(echo x)'\n"), + ("bash -lc '...' is checked", 1, H + + " steps:\n - run: |\n bash -lc 'if true; then echo x'\n"), + ("bash -euo pipefail -c '...' is checked", 1, H + + " steps:\n - run: |\n bash -euo pipefail -c 'if true; then echo x'\n"), + ("two scripts in one step: the second is checked too", 1, H + + " steps:\n - run: |\n bash -c 'echo ok'\n bash -c 'if true; then echo x'\n"), + ("a -c script glued to a word = cannot tell", 2, H + + " steps:\n - run: |\n bash -c 'echo x'y\n"), + ("zsh -c '...' is not read", 0, H + + " steps:\n - run: |\n zsh -c 'if true; then'\n"), + ("a double-quoted bash -c \"...\" is not read", 0, H + + " steps:\n - run: |\n bash -c \"if true; then\"\n"), ("${{ }} is left as written", 0, H + " steps:\n - run: echo \"${{ github.sha }}\"\n"), # `cat <(echo x)` parses in bash and not in sh (dash, or bash in POSIX