diff --git a/.github/workflows/run-syntax-gate.yml b/.github/workflows/run-syntax-gate.yml index 59b47d736..1313a7dc1 100644 --- a/.github/workflows/run-syntax-gate.yml +++ b/.github/workflows/run-syntax-gate.yml @@ -4,6 +4,7 @@ name: Run syntax gate # parse each time they were dispatched, and nothing said so until one was run: # an apostrophe in a comment inside `bash -c '...'` (build-matrix) and a `'\''` # used outside single quotes (iddr-golden-diff). See tools/check_run_syntax.py. +# The run steps of composite actions under .github/actions/ are read too. on: push: diff --git a/tools/check_run_syntax.py b/tools/check_run_syntax.py index 18d0c1bd5..2896538d2 100644 --- a/tools/check_run_syntax.py +++ b/tools/check_run_syntax.py @@ -26,7 +26,23 @@ `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. + +A composite action (`.github/actions/**/action.yml` or `action.yaml`, +`runs.using: composite`) is read the same way: each of its `runs.steps` that +has a `run:` must name its `shell:` (GitHub refuses one that does not, so a +step without it is "cannot tell"), and there are no job or workflow defaults +to inherit. The runner is the caller's: every workflow job that uses the +action by its `./` path, directly or through another local action, and if any +of them may be macOS the step is "cannot tell" unless the gate's bash is 3.x. +An action no workflow here calls is still parsed. Node and Docker actions have +no `run:` steps and are skipped. 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,6 +181,85 @@ 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_run(script: str, shells: tuple[str, ...], where: str) -> list[str]: + """`bash -n` / `sh -n` on one step's script, then on each single-quoted -c + script in it. CannotTell from inner_scripts goes to the caller.""" + global _INNER_SEEN + bad = [] + outer_parses = bool(shells) + for sh in shells: + r = subprocess.run([sh, "-n"], input=script, capture_output=True, text=True) + if r.returncode: + err = " ".join(r.stderr.split())[:240] + 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: + return bad + inner = inner_scripts(script) + _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 + + def check(path: pathlib.Path) -> tuple[list[str], str | None]: import yaml # inside: a missing PyYAML is "cannot tell", not a crash try: @@ -182,17 +277,108 @@ def check(path: pathlib.Path) -> tuple[list[str], str | None]: return [], f"{path.name} :: {jn}: step {k} is not a mapping" if "run" not in step: continue + where = f"{path.relative_to(ROOT)} :: {jn} :: {step.get('name') or f'step {k}'}" try: - shells = shell_of(step, job, wf) + bad += check_run(str(step["run"]), shell_of(step, job, wf), where) except CannotTell as e: return [], f"{path.name} :: {jn} :: step {k}: {e}" - 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}") - break + return bad, None + + +def local_uses(steps) -> list[str]: + """The `./` actions a steps list calls, as repo-relative directories.""" + found = [] + for step in steps if isinstance(steps, list) else []: + uses = step.get("uses") if isinstance(step, dict) else None + local = isinstance(uses, str) and uses.startswith("./") + if local: + found.append(uses[2:].rstrip("/")) + return found + + +def action_dir(path: pathlib.Path) -> str: + return path.parent.relative_to(ROOT).as_posix() + + +def action_callers(workflows: list[pathlib.Path], actions: list[pathlib.Path]) -> dict[str, list[dict]]: + """Every workflow job that runs each local action, directly or through + another local action, keyed by the action's directory. A file that does + not load is skipped here; check() and check_action() report it.""" + import yaml + direct: dict[str, list[dict]] = {} + for f in workflows: + try: + wf = yaml.safe_load(f.read_text()) + except yaml.YAMLError: + continue + jobs = wf.get("jobs") if isinstance(wf, dict) else None + for job in jobs.values() if isinstance(jobs, dict) else []: + steps = job.get("steps") if isinstance(job, dict) else None + for rel in local_uses(steps): + direct.setdefault(rel, []).append(job) + parents: dict[str, list[str]] = {} + for f in actions: + try: + act = yaml.safe_load(f.read_text()) + except yaml.YAMLError: + continue + runs = act.get("runs") if isinstance(act, dict) else None + steps = runs.get("steps") if isinstance(runs, dict) else None + for child in local_uses(steps): + parents.setdefault(child, []).append(action_dir(f)) + + def jobs_of(rel: str, seen: frozenset) -> list[dict]: + jobs = list(direct.get(rel, [])) + for parent in parents.get(rel, []): + fresh = parent not in seen + if fresh: + jobs += jobs_of(parent, seen | {parent}) + return jobs + + return {action_dir(f): jobs_of(action_dir(f), frozenset({action_dir(f)})) for f in actions} + + +def check_action(path: pathlib.Path, callers: list[dict]) -> tuple[list[str], str | None]: + """check() for one action.yml; `callers` are the jobs that run it.""" + import yaml + rel = path.relative_to(ROOT).as_posix() + try: + act = yaml.safe_load(path.read_text()) + except yaml.YAMLError as e: + return [], f"{rel}: not YAML ({e.__class__.__name__})" + runs = act.get("runs") if isinstance(act, dict) else None + no_runs = not isinstance(runs, dict) + if no_runs: + return [], f"{rel}: no runs mapping" + composite = runs.get("using") == "composite" + if not composite: + return [], None # node and docker actions have no run: steps + steps = runs.get("steps") + no_steps = not isinstance(steps, list) + if no_steps: + return [], f"{rel}: a composite action with no steps list" + mac_caller = any(may_run_on_macos(job) for job in callers) + bad = [] + for k, step in enumerate(steps, 1): + not_mapping = not isinstance(step, dict) + if not_mapping: + return [], f"{rel}: step {k} is not a mapping" + no_run = "run" not in step + if no_run: + continue + unnamed_shell = not step.get("shell") + if unnamed_shell: + return [], f"{rel} :: step {k}: a composite run step must name its shell" + where = f"{rel} :: {step.get('name') or f'step {k}'}" + try: + shells = _shell_named(step, {}, {}) + # The same bash 3.2 question as shell_of, asked of the callers' runners. + old_bash_unchecked = bool(shells) and mac_caller and bash_major() != 3 + if old_bash_unchecked: + raise CannotTell(f"a caller may run on macOS (bash 3.2); this gate has bash {bash_major()}") + bad += check_run(str(step["run"]), shells, where) + except CannotTell as e: + return [], f"{rel} :: step {k}: {e}" return bad, None @@ -206,6 +392,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 +401,24 @@ 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)}") + wf_unread = len(unread) + actions_root = ROOT / ".github" / "actions" + has_actions = actions_root.is_dir() + actions = sorted(list(actions_root.rglob("action.yml")) + list(actions_root.rglob("action.yaml")) + if has_actions else []) + callers = action_callers(files, actions) + uncalled = 0 + for a in actions: + b, err = check_action(a, callers[action_dir(a)]) + bad += b + no_caller = not callers[action_dir(a)] + if no_caller: + uncalled += 1 + if err: + unread.append(err) + print(f"workflows read: {len(files) - wf_unread} of {len(files)}; actions read: " + f"{len(actions) - (len(unread) - wf_unread)} of {len(actions)} ({uncalled} with no caller here); " + f"bash -c '...' scripts read out 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..c2de82e02 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 @@ -201,15 +230,86 @@ def with_bash(major, f): "cannot tell"), ("a pwsh step on macos is skipped, not cannot tell", lambda: with_bash(5, lambda: crs.shell_of({"shell": "pwsh"}, {"runs-on": "macos-15"}, {})), ()), + # Composite actions, under a bash 5 gate whatever bash runs this test. + ("a composite action a macOS job calls, bash 5 gate: rc 2", + lambda: with_bash(5, lambda: run_case("on: push\njobs:\n j:\n runs-on: macos-15\n" + CALL, + {"a/action.yml": A + " - shell: bash\n run: echo ok\n"})[0]), + 2), + ("an action reached from a macOS job through another action, bash 5 gate: rc 2", + lambda: with_bash(5, lambda: run_case( + "on: push\njobs:\n j:\n runs-on: macos-15\n steps:\n - uses: ./.github/actions/outer\n", + {"outer/action.yml": A + " - uses: ./.github/actions/a\n", + "a/action.yml": A + " - shell: bash\n run: echo ok\n"})[0]), + 2), + ("the same action from an ubuntu job, bash 5 gate: rc 0", + lambda: with_bash(5, lambda: run_case(H + CALL, {"a/action.yml": A + " - shell: bash\n run: echo ok\n"})[0]), + 0), + ("uses: ./x/ names the directory x", + lambda: crs.local_uses([{"uses": "./.github/actions/a/"}, {"uses": "actions/cache@v4"}, "echo"]), + [".github/actions/a"]), ] -def run_case(text: str) -> tuple[int, str]: +# Composite actions: (name, expected rc, workflow text, {path under .github/actions: text}). +A = "runs:\n using: composite\n steps:\n" +CALL = " steps:\n - uses: ./.github/actions/a\n" +ACTION_CASES: list[tuple[str, int, str, dict[str, str]]] = [ + ("a clean composite action", 0, H + CALL, + {"a/action.yml": A + " - shell: bash\n run: echo ok\n"}), + ("an unbalanced quote in a composite run step", 1, H + CALL, + {"a/action.yml": A + " - shell: bash\n run: 'echo \"x'\n"}), + ("an unclosed if inside bash -c '...' in a composite step", 1, H + CALL, + {"a/action.yml": A + " - shell: bash\n run: |\n docker run img bash -c '\n" + " if true; then\n make\n '\n"}), + ("a composite run step with no shell = cannot tell", 2, H + CALL, + {"a/action.yml": A + " - run: echo ok\n"}), + ("a composite shell: sh is checked with sh", 1, H + CALL, + {"a/action.yml": A + " - shell: sh\n run: cat <(echo x)\n"}), + ("a composite shell: pwsh is skipped", 0, H + CALL, + {"a/action.yml": A + " - shell: pwsh\n run: Write-Host \"it's\n"}), + ("a composite uses: step is skipped", 0, H + CALL, + {"a/action.yml": A + " - uses: actions/cache/restore@v4\n"}), + ("a node action is skipped", 0, H + CALL, + {"a/action.yml": "runs:\n using: node20\n main: index.js\n"}), + ("an action that is not YAML = cannot tell", 2, H + CALL, {"a/action.yml": "runs: [\n"}), + ("an action with no runs mapping = cannot tell", 2, H + CALL, {"a/action.yml": "name: a\n"}), + ("a composite action with no steps list = cannot tell", 2, H + CALL, + {"a/action.yml": "runs:\n using: composite\n"}), + ("a composite step that is not a mapping = cannot tell", 2, H + CALL, + {"a/action.yml": A + " - echo\n"}), + ("an action no workflow calls is still parsed", 1, H + " steps:\n - run: echo ok\n", + {"a/action.yml": A + " - shell: bash\n run: 'echo \"x'\n"}), + ("action.yaml is read", 1, H + CALL, + {"a/action.yaml": A + " - shell: bash\n run: 'echo \"x'\n"}), + ("an action in a nested directory is read", 1, H + " steps:\n - uses: ./.github/actions/g/a/\n", + {"g/a/action.yml": A + " - shell: bash\n run: 'echo \"x'\n"}), + ("a composite action called from a macOS job: bash 3.2 there", 1 if crs.bash_major() == 3 else 2, + "on: push\njobs:\n j:\n runs-on: macos-15\n" + CALL, + {"a/action.yml": A + " - shell: bash\n run: make 2>&1 |& tee build.log\n"}), + ("an action called only by an action a macOS job calls: bash 3.2 there", + 1 if crs.bash_major() == 3 else 2, + "on: push\njobs:\n j:\n runs-on: macos-15\n steps:\n - uses: ./.github/actions/outer\n", + {"outer/action.yml": A + " - uses: ./.github/actions/a\n", + "a/action.yml": A + " - shell: bash\n run: make 2>&1 |& tee build.log\n"}), + # Under a bash 5 gate this and the two above differ only in the caller's runner. + ("an ubuntu caller is plain bash", 0, H + CALL, + {"a/action.yml": A + " - shell: bash\n run: echo ok\n"}), + ("two actions that call each other end", 0, H + CALL, + {"a/action.yml": A + " - uses: ./.github/actions/b\n - shell: bash\n run: echo ok\n", + "b/action.yml": A + " - uses: ./.github/actions/a\n"}), +] + + +def run_case(text: str, actions: dict[str, str] | None = None) -> tuple[int, str]: with tempfile.TemporaryDirectory() as d: root = pathlib.Path(d) wf = root / ".github" / "workflows" wf.mkdir(parents=True) (wf / "t.yml").write_text(text) + for rel, body in (actions or {}).items(): + p = root / ".github" / "actions" / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body) crs.ROOT, crs.WORKFLOWS = root, wf out = io.StringIO() with contextlib.redirect_stdout(out): @@ -226,6 +326,14 @@ def main() -> int: print(f"ok {name} (rc {rc})") else: print(f"BAD {name}: want rc {want}, got {rc}\n{out}") + for name, want, text, actions in ACTION_CASES: + rc, out = run_case(text, actions) + as_expected = rc == want + if as_expected: + good += 1 + print(f"ok {name} (rc {rc})") + else: + print(f"BAD {name}: want rc {want}, got {rc}\n{out}") for name, got, want in DIRECT: g = got() if g == want: @@ -233,7 +341,7 @@ def main() -> int: print(f"ok {name} ({g})") else: print(f"BAD {name}: want {want}, got {g}") - total = len(CASES) + len(DIRECT) + total = len(CASES) + len(ACTION_CASES) + len(DIRECT) print(f"\n{good} of {total} cases as expected") return 0 if good == total else 1