diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9f26ca1ae..45c19d661 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4,7 +4,7 @@ name: "Lint" # bandit -- had Makefile targets and no CI presence at all, so nothing noticed # when their output changed. (`isort` does appear in cron-vendor.yml, but as a # formatter that rewrites the generated constants, not as a check.) This workflow -# is where they run. +# is where they run, with actionlint over the workflows themselves (#1967). # # Every one of them is advisory -- `continue-on-error: true` on each step -- and # that is a measurement result rather than a preference. This header records no @@ -111,6 +111,53 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + # #1967: actionlint checks every workflow under .github/workflows, and runs + # the image's shellcheck over each `run:` block. First, as it needs no + # Python and is the quickest verdict. Advisory like the steps below, since + # shellcheck still reports style and info findings across the workflows. + # + # The release binary is pinned by version and sha256, from the release's + # checksums file. Dependabot does not see this pin; bump both together. + # These files use YAML anchors, which actionlint reads from 1.7.10 on. + # + # The `-ignore` hides a false positive. `concurrency.queue` is a documented + # GitHub key (project-status.yml uses it) that no actionlint release knows + # yet: rhysd/actionlint#657, with the fix in rhysd/actionlint#654. Drop the + # pattern once a release with that fix is pinned here. + - name: actionlint + continue-on-error: true + timeout-minutes: 5 + env: + ACTIONLINT_VERSION: 1.7.12 + ACTIONLINT_SHA256: 8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 + run: | + archive="actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" + curl -fsSL --retry 3 -o "$RUNNER_TEMP/$archive" \ + "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/$archive" + echo "$ACTIONLINT_SHA256 $RUNNER_TEMP/$archive" | sha256sum -c - + tar -xzf "$RUNNER_TEMP/$archive" -C "$RUNNER_TEMP" actionlint + "$RUNNER_TEMP/actionlint" -version + + set +e + "$RUNNER_TEMP/actionlint" -oneline -no-color \ + -ignore '^unexpected key "queue" for "concurrency" section\.' 2>&1 | tee actionlint.log + status=${PIPESTATUS[0]} + set -e + + # Without pipefail a pipeline's status is uniq's, so an empty count + # is caught by its output, not its status. + rules=$(grep -oE '\[[a-z-]+\]$' actionlint.log | sort | uniq -c) + shellcheck=$(grep -oE 'SC[0-9]+:[a-z]+' actionlint.log | sort | uniq -c) + { + echo '### actionlint' + echo '```' + printf 'findings by rule:\n%s\n' "${rules:-none}" + printf 'shellcheck findings by code:\n%s\n' "${shellcheck:-none}" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + + exit "$status" + # One interpreter, not a matrix. Unit Tests and Python Compatibility sweep # the supported tier, plus the non-blocking KTLO legs (#1633), because # they check that the package *runs* everywhere; these tools read the diff --git a/docs/source/contributing/workflows.rst b/docs/source/contributing/workflows.rst index 856d20ec8..df9c3b747 100644 --- a/docs/source/contributing/workflows.rst +++ b/docs/source/contributing/workflows.rst @@ -1188,6 +1188,31 @@ and when :file:`.github/dependabot.yml` drops the ``github-actions`` ecosystem. ``pinned()`` and ``versions()`` in :file:`tests/_workflows.py` read a pin, for a test that names a particular action. +Workflow Lint (`#1967 `__) +------------------------------------------------------------------------------ + +Lint's ``lint`` job runs actionlint over every workflow in its ``actionlint`` step, +first, as it needs no Python. The step downloads the release that +``ACTIONLINT_VERSION`` names and checks it against ``ACTIONLINT_SHA256``, from that +release's checksums file, before unpacking it. Dependabot does not read this pin, +so a bump changes both. The workflows use YAML anchors, which actionlint reads from +1.7.10 on, and the pin is 1.7.12 or newer. + +actionlint runs the image's shellcheck over each ``run:`` block, and shellcheck +still reports style and info findings, so the step is advisory like the others in +Lint. Its run summary counts the findings by actionlint rule and by shellcheck code. + +The step's ``-ignore`` pattern, which matches a message, not a file, hides a false +positive: ``concurrency.queue``, which ``project-status.yml`` sets and GitHub +documents, is a key no actionlint release knows yet (`rhysd/actionlint#657 +`__). The pattern goes once a +release that knows it is pinned. + +:file:`tests/project/test_actionlint_in_lint_1967.py` fails when the step is gone, +pins an older release or no checksum, unpacks before it verifies, or passes an +``-ignore`` pattern other than that, and when no workflow sets +``concurrency.queue`` any more. + Dependency Updates (`#1968 `__) ----------------------------------------------------------------------------------- diff --git a/tests/project/test_actionlint_in_lint_1967.py b/tests/project/test_actionlint_in_lint_1967.py new file mode 100644 index 000000000..3d41457b5 --- /dev/null +++ b/tests/project/test_actionlint_in_lint_1967.py @@ -0,0 +1,125 @@ +# -*- coding: utf-8 -*- +"""#1967: Lint runs actionlint, pinned to a checksum-verified release. + +Nothing ran actionlint over the workflows before #1967. Lint's ``lint`` job now +has an ``actionlint`` step that downloads one release, named by +``ACTIONLINT_VERSION``, checks it against ``ACTIONLINT_SHA256`` before unpacking +it, and runs it over every workflow. The version is 1.7.12 or newer: the +workflows use YAML anchors, which 1.7.7 cannot read. + +* :class:`TestTheStep` pins the step: one of it, the version, the checksum, and + the order download, verify, unpack, run. +* :class:`TestTheIgnores` pins each ``-ignore`` pattern to the false positive it + exists for, so a blanket pattern fails here, and checks that a workflow still + uses the key, so the pattern goes when the key does. + +""" + +from __future__ import annotations + +import re +import unittest +from typing import TYPE_CHECKING + +from tests import _workflows + +if TYPE_CHECKING: + from typing import Any + +#: Test tier (#1958): reads the workflow files, a question about the tree. +__tier__ = 'LATEST' + +WORKFLOW = 'lint.yml' +JOB = 'lint' +STEP = 'actionlint' +#: The oldest release the step may pin. +MINIMUM = (1, 7, 12) +#: Each ``-ignore`` pattern the step may pass, with a message it must hide and +#: messages it must not. A pattern is matched against the message alone. +IGNORES = { + # rhysd/actionlint#657: GitHub documents ``concurrency.queue``, which + # project-status.yml sets; no actionlint release knows the key yet. + r'^unexpected key "queue" for "concurrency" section\.': ( + 'unexpected key "queue" for "concurrency" section. expected one of ' + '"cancel-in-progress", "group"', + ( + 'unexpected key "queues" for "concurrency" section. expected one of ' + '"cancel-in-progress", "group"', + 'unexpected key "queue" for "job" section. expected one of "runs-on"', + 'unexpected key "group" for "concurrency" section', + ), + ), +} +#: An ``-ignore`` argument in the step's script, quoted either way. +IGNORE = re.compile(r'''-ignore\s+(?P['"])(?P.*?)(?P=quote)''') + + +def step() -> 'dict[str, Any]': + """The ``actionlint`` step of Lint's ``lint`` job.""" + found = [entry for entry in _workflows.steps(WORKFLOW, JOB) if entry.get('name') == STEP] + if len(found) != 1: + raise AssertionError(f'expected one {STEP!r} step in {WORKFLOW}:{JOB}, found {len(found)}') + return found[0] + + +def concurrency_keys(workflow: 'dict[str, Any]') -> 'set[str]': + """Every key of every ``concurrency:`` mapping in ``workflow``, workflow and job level.""" + blocks = [workflow.get('concurrency')] + blocks.extend(spec.get('concurrency') for spec in (workflow.get('jobs') or {}).values()) + return {key for block in blocks if isinstance(block, dict) for key in block} + + +class TestTheStep(unittest.TestCase): + """The ``actionlint`` step downloads a pinned release, verifies it, then runs it.""" + + def test_the_version_is_pinned_and_new_enough(self) -> None: + version = str(step()['env']['ACTIONLINT_VERSION']) + self.assertRegex(version, r'^\d+\.\d+\.\d+$') + self.assertGreaterEqual(tuple(map(int, version.split('.'))), MINIMUM) + + def test_the_checksum_is_pinned(self) -> None: + self.assertRegex(str(step()['env']['ACTIONLINT_SHA256']), r'^[0-9a-f]{64}$') + + def test_it_downloads_verifies_unpacks_then_runs(self) -> None: + script = step()['run'] + order = [ + ('download', r'curl\b.*releases/download/v\$\{ACTIONLINT_VERSION\}/'), + ('verify', r'\$ACTIONLINT_SHA256\b.*\|\s*sha256sum -c -'), + ('unpack', r'tar -xzf\b'), + ('run', r'"\$RUNNER_TEMP/actionlint" -oneline\b'), + ] + positions = [] + for what, pattern in order: + match = re.search(pattern, script, re.S) + self.assertIsNotNone(match, f'the step does not {what}: {pattern!r}') + positions.append(match.start()) # type: ignore[union-attr] + self.assertEqual(positions, sorted(positions), 'download, verify, unpack and run are out of order') + + def test_it_bounds_its_time(self) -> None: + minutes = step().get('timeout-minutes') + self.assertIsInstance(minutes, int) + self.assertLess(minutes, _workflows.job(WORKFLOW, JOB)['timeout-minutes']) + + +class TestTheIgnores(unittest.TestCase): + """Each ``-ignore`` hides one known false positive, and only while it can occur.""" + + def test_every_pattern_is_a_known_one(self) -> None: + patterns = [match['pattern'] for match in IGNORE.finditer(step()['run'])] + self.assertEqual(sorted(patterns), sorted(IGNORES)) + + def test_each_pattern_hides_only_its_message(self) -> None: + for pattern, (hidden, shown) in IGNORES.items(): + with self.subTest(pattern=pattern): + self.assertRegex(hidden, pattern) + for message in shown: + self.assertNotRegex(message, pattern) + + def test_a_workflow_still_sets_concurrency_queue(self) -> None: + users = sorted(path.name for path in _workflows.WORKFLOWS.glob('*.yml') + if 'queue' in concurrency_keys(_workflows.load(path))) + self.assertTrue(users, 'no workflow sets concurrency.queue; drop its -ignore pattern from Lint') + + +if __name__ == '__main__': + unittest.main()