diff --git a/.github/validate-action.py b/.github/validate-action.py new file mode 100755 index 0000000..00813e7 --- /dev/null +++ b/.github/validate-action.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +"""Check that action.yml is one GitHub can actually load and run. + +v1.0.0 and v1 both once pointed at an action.yml that did not parse, so the +action could not be loaded at all by anyone who referenced them. Parsing alone +is not enough: a file can be valid YAML and still be unusable, so this also +checks the structure GitHub requires. + + validate-action.py [path] # default: action.yml + +Exits non-zero and prints every problem found, rather than only the first. +""" +import sys + +import yaml + +# Every step in a composite action must say how to run: either it delegates to +# another action (uses:) or it runs a script, which then requires a shell. +COMPOSITE_STEP_KEYS = ("uses", "run") + + +def problems(path): + try: + with open(path) as fh: + raw = fh.read() + except OSError as exc: + return [f"cannot read {path}: {exc}"] + + try: + doc = yaml.safe_load(raw) + except yaml.YAMLError as exc: + # The original failure mode: a dedented heredoc ended the block scalar + # and YAML read the script as a mapping. + return [f"not valid YAML: {str(exc).splitlines()[0]}"] + + if not isinstance(doc, dict): + return [f"top level must be a mapping, got {type(doc).__name__}"] + + found = [] + for key in ("name", "description", "runs"): + if key not in doc: + found.append(f"missing required top-level key: {key}") + + runs = doc.get("runs") + if not isinstance(runs, dict): + if runs is not None: + found.append("runs: must be a mapping") + return found + + using = runs.get("using") + if not using: + found.append("runs.using is required") + elif using == "composite": + steps = runs.get("steps") + if not isinstance(steps, list) or not steps: + found.append("composite actions need a non-empty runs.steps list") + else: + for i, step in enumerate(steps): + label = step.get("name", f"#{i}") if isinstance(step, dict) else f"#{i}" + if not isinstance(step, dict): + found.append(f"step {label}: must be a mapping") + continue + if not any(k in step for k in COMPOSITE_STEP_KEYS): + found.append(f"step {label}: needs either uses: or run:") + if "run" in step and not step.get("shell"): + # GitHub rejects the whole action, not just this step. + found.append(f"step {label}: run: requires shell:") + + # Inputs are optional, but a malformed block breaks the action at load time. + inputs = doc.get("inputs") + if inputs is not None and not isinstance(inputs, dict): + found.append("inputs: must be a mapping") + elif isinstance(inputs, dict): + for name, spec in inputs.items(): + if not isinstance(spec, dict): + found.append(f"input {name}: must be a mapping") + elif "description" not in spec: + found.append(f"input {name}: missing description") + + return found + + +def main(): + path = sys.argv[1] if len(sys.argv) > 1 else "action.yml" + found = problems(path) + if found: + print(f"{path} is not a loadable action:", file=sys.stderr) + for p in found: + print(f" - {p}", file=sys.stderr) + return 1 + print(f"{path} is a loadable action") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/validate_action_test.py b/.github/validate_action_test.py new file mode 100755 index 0000000..4fb1542 --- /dev/null +++ b/.github/validate_action_test.py @@ -0,0 +1,104 @@ +#!/usr/bin/env python3 +"""Tests for validate-action.py. + +A guardrail that cannot fail is not a guardrail, so every case here asserts +the direction it should resolve in — including the exact file that shipped as +v1.0.0 and could not be loaded. + + python3 .github/validate_action_test.py +""" +import importlib.util +import pathlib +import sys +import tempfile + +HERE = pathlib.Path(__file__).parent +spec = importlib.util.spec_from_file_location("va", HERE / "validate-action.py") +va = importlib.util.module_from_spec(spec) +spec.loader.exec_module(va) + +VALID = """\ +name: x +description: y +runs: + using: composite + steps: + - run: echo hi + shell: bash +""" + +# Each case is (label, content, must_be_rejected). +CASES = [ + ("valid minimal", VALID, False), + ( + "valid with uses: step", + "name: x\ndescription: y\nruns:\n using: composite\n steps:\n - uses: actions/checkout@v4\n", + False, + ), + # The v1.0.0 failure: a dedented script ended the block scalar. + ( + "script dedented out of block scalar", + "name: x\ndescription: y\nruns:\n using: composite\n steps:\n - run: |\n python3 -c 'import sys\nif True: pass\n'\n shell: bash\n", + True, + ), + ("missing runs", "name: x\ndescription: y\n", True), + ("missing name", "description: y\n" + VALID.split("\n", 1)[1], True), + ( + "run without shell", + "name: x\ndescription: y\nruns:\n using: composite\n steps:\n - run: echo hi\n", + True, + ), + ( + "empty steps", + "name: x\ndescription: y\nruns:\n using: composite\n steps: []\n", + True, + ), + ( + "step with neither uses nor run", + "name: x\ndescription: y\nruns:\n using: composite\n steps:\n - name: nothing\n", + True, + ), + ( + "runs without using", + "name: x\ndescription: y\nruns:\n steps: []\n", + True, + ), + ( + "input missing description", + "name: x\ndescription: y\ninputs:\n foo:\n default: '1'\n" + + VALID.split("description: y\n", 1)[1], + True, + ), + ("not a mapping", "- just\n- a list\n", True), +] + + +def main(): + failures = 0 + for label, content, must_reject in CASES: + with tempfile.NamedTemporaryFile("w", suffix=".yml", delete=False) as fh: + fh.write(content) + path = fh.name + found = va.problems(path) + rejected = bool(found) + if rejected != must_reject: + failures += 1 + want = "rejected" if must_reject else "accepted" + print(f"FAIL {label}: expected {want}, got {found or 'accepted'}") + else: + detail = f" ({found[0]})" if found else "" + print(f"ok {label}{detail}") + + # A missing file is a failure, not a crash. + if not va.problems("/nonexistent/action.yml"): + failures += 1 + print("FAIL missing file should be reported") + else: + print("ok missing file reported") + + print(f"\n{len(CASES) + 1} cases, {failures} failure(s)") + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7207287..fb2422f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,8 +17,14 @@ jobs: # action.yml shipped invalid YAML in v1.0.0 and v1: an embedded script was # dedented out of its block scalar, so GitHub could not load the action at # all. Parsing it on every change is the check that would have caught it. - - name: action.yml parses - run: python3 -c "import yaml,sys; yaml.safe_load(open('action.yml')); print('action.yml parses')" + # Structural check, not just a YAML parse: an action.yml can be valid + # YAML and still be unloadable (no runs:, a run: step without shell:). + - name: action is loadable + run: python3 .github/validate-action.py action.yml + + # A guardrail that cannot fail is not a guardrail. + - name: validator tests + run: python3 .github/validate_action_test.py - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a4f1087..a48fb30 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,6 +23,12 @@ jobs: with: fetch-depth: 0 # all tags, to find the latest version + # Gate the tag itself, not just the release. A pushed tag is immutable + # and public immediately, so validating afterwards is too late: v1.0.0 + # still names an action.yml that cannot be loaded, and always will. + - name: action is loadable + run: python3 .github/validate-action.py action.yml + - name: pick version id: pick env: @@ -64,11 +70,11 @@ jobs: ref: ${{ needs.version.outputs.tag }} fetch-depth: 0 - # An action whose action.yml does not parse cannot be loaded at all, and - # a tag is public the moment it is pushed. v1.0.0 shipped exactly that - # way, so verify before advertising the release. - - name: action.yml parses - run: python3 -c "import yaml,sys; yaml.safe_load(open('action.yml')); print('action.yml parses')" + # Re-checked at the tag being released, not just at main: a manual tag + # push enters here directly, and the major alias must never be moved to + # something unloadable. + - name: action is loadable + run: python3 .github/validate-action.py action.yml - name: move major alias env: diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..7a60b85 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +__pycache__/ +*.pyc diff --git a/README.md b/README.md index 5af5c0a..91ea0f4 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,12 @@ The major-version alias (`v1`) moves to each new release automatically, so `@v1` always resolves to the newest compatible version. Pushing a `v*` tag by hand still works for re-cuts. +`action.yml` is checked for loadability — valid YAML *and* the structure GitHub +requires to run it — before the tag is pushed and again before the alias moves. +A tag is public and immutable the instant it exists, so validating afterwards +would be too late: `v1.0.0` permanently names an `action.yml` that cannot be +loaded, and that is the failure these checks exist to prevent repeating. + Reference a release by SHA (preferred) or tag. ## License