diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,7 @@ +# Review changes to the shared release pipeline and the checker itself. +# +# This file only takes effect when the repository's branch ruleset enables +# "Require review from Code Owners"; that is an owner setting, not a file in this PR. +/.github/workflows/ @dylanmtaylor @echelon @skylartaylor +/actions/ @dylanmtaylor @echelon @skylartaylor +/scripts/ @dylanmtaylor @echelon @skylartaylor diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,50 @@ +# Lint this repository itself: every external `uses:` must name an immutable ref, and +# actionlint checks the workflow syntax. This is a check, not enforcement: the repository +# ruleset must require it before a change can be blocked. +name: CI + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + pinned-refs: + name: Pinned refs + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Test the checker + run: python3 -m unittest discover -s scripts -p 'test_*.py' -v + - name: Check external action refs + run: python3 scripts/check_pinned_refs.py + + actionlint: + name: actionlint + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install actionlint + env: + ACTIONLINT_VERSION: "1.7.12" + ACTIONLINT_SHA256: "8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8" + run: | + curl -fsSL -o "$RUNNER_TEMP/actionlint.tar.gz" \ + "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz" + echo "${ACTIONLINT_SHA256} $RUNNER_TEMP/actionlint.tar.gz" | sha256sum -c - + tar -xz -C "$RUNNER_TEMP" -f "$RUNNER_TEMP/actionlint.tar.gz" actionlint + echo "$RUNNER_TEMP" >> "$GITHUB_PATH" + - name: Install shellcheck + run: sudo apt-get update && sudo apt-get install -y --no-install-recommends shellcheck + - name: actionlint + run: actionlint -color diff --git a/.github/workflows/packaging-lint.yml b/.github/workflows/packaging-lint.yml --- a/.github/workflows/packaging-lint.yml +++ b/.github/workflows/packaging-lint.yml @@ -14,6 +14,9 @@ on: required: false default: "" +permissions: + contents: read + jobs: lint: runs-on: ubuntu-latest diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -297,6 +297,20 @@ Individual actions are also available for composing custom pipelines: This repository is hosted at `storytold/craft-common-actions` for all Storytold / ArtCraft craftapps. +## Repository checks + +`.github/workflows/ci.yml` runs two checks on pull requests and pushes to `main`: + +- **Pinned refs**: every external `uses:` in `.github/workflows/` and `actions/` must name + a full commit SHA or an immutable Docker `sha256:` digest. The repository's own actions + and workflows at `@main` are the documented exception (issue #6); they are allowlisted in + `scripts/check_pinned_refs.py`. +- **actionlint**: validates the workflow syntax and runs shellcheck on the `run:` blocks. + +The checks only block a merge when the repository ruleset requires them. Requiring these +checks, code-owner review from `.github/CODEOWNERS`, and no force-pushes or deletions are +repository settings (owner action), not files in this repository. + ## App-specific packaging checks Keep app-specific packaging assertions in the caller repository and pass their diff --git a/scripts/check_pinned_refs.py b/scripts/check_pinned_refs.py new file mode 100644 --- /dev/null +++ b/scripts/check_pinned_refs.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python3 +"""Reject mutable external `uses:` refs in this repository's workflows and actions. + +GitHub resolves a `uses:` ref when a run starts. A branch or tag can move, so a workflow +that calls one runs code that is not in this repository's history. Every external ref must +name a full 40-character commit SHA or an immutable Docker `sha256:` digest. + +The one exception is this repository's own actions and workflows at `@main`: callers are +meant to trail that branch so a fix here reaches every app without a PR in each repository +(see issue #6). The allowlist below keeps the exception explicit. +""" + +from __future__ import annotations + +import argparse +import string +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SCAN_DIRS = (".github/workflows", "actions") + +# (owner/repo, ref, reason) +ALLOWED = ( + ( + "storytold/craft-common-actions", + "main", + "first-party actions and workflows are intentionally tracked at @main (issue #6)", + ), +) + + +def is_hex(value: str) -> bool: + return bool(value) and all(char in string.hexdigits for char in value) + + +def is_immutable(reference: str) -> bool: + """A full commit SHA or a Docker `sha256:` digest. Tags and branches are mutable.""" + if len(reference) == 40 and is_hex(reference): + return True + prefix = "sha256:" + return ( + reference.startswith(prefix) + and len(reference) == len(prefix) + 64 + and is_hex(reference[len(prefix):]) + ) + + +def repo_of(target: str) -> str | None: + """`owner/repo` from `owner/repo/path`.""" + parts = target.split("/", 2) + if len(parts) < 2 or not parts[0] or not parts[1]: + return None + return f"{parts[0]}/{parts[1]}" + + +def uses_value(line: str) -> str | None: + """The value of a `uses:` line, without a YAML list marker, quotes or a trailing comment.""" + rest = line.lstrip() + if rest.startswith("-"): + rest = rest[1:].lstrip() + if not rest.startswith("uses:"): + return None + rest = rest[len("uses:"):].strip() + if not rest or rest.startswith("#"): + return None + token = rest.split()[0] + return token.strip("'\"") + + +def scan_file(root: Path, path: Path, checked: int, violations: list[str], exceptions: list[str]): + name = path.relative_to(root) + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), start=1): + uses = uses_value(line) + if uses is None: + continue + checked += 1 + if uses.startswith("./"): + continue + target, separator, reference = uses.rpartition("@") + if not separator: + violations.append(f"{name}:{number}: `uses: {uses}` names no `@ref`") + continue + if is_immutable(reference): + continue + allowed = next( + ( + reason + for repo, ref, reason in ALLOWED + if repo == repo_of(target) and ref == reference + ), + None, + ) + if allowed is not None: + exceptions.append(f"{name}:{number}: {target}@{reference} ({allowed})") + continue + violations.append( + f"{name}:{number}: `uses: {uses}` is mutable; pin it to a full 40-character " + "commit SHA (or an immutable Docker digest), or add an allowlist entry with a reason" + ) + return checked, violations, exceptions + + +def find_violations(root: Path) -> tuple[int, list[str], list[str]]: + checked = 0 + violations: list[str] = [] + exceptions: list[str] = [] + for scan_dir in SCAN_DIRS: + paths = sorted( + path + for path in (root / scan_dir).rglob("*") + if path.is_file() and path.suffix in {".yml", ".yaml"} + ) + for path in paths: + checked, violations, exceptions = scan_file( + root, path, checked, violations, exceptions + ) + return checked, violations, exceptions + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--root", + type=Path, + default=ROOT, + help="repository root (default: the parent of scripts/)", + ) + args = parser.parse_args() + checked, violations, exceptions = find_violations(args.root) + print(f"pinned refs: checked {checked} `uses:` ref(s)") + if exceptions: + print(f"pinned refs: {len(exceptions)} allowlisted mutable ref(s):") + for exception in exceptions: + print(f" - {exception}") + if violations: + print(f"error: {len(violations)} mutable `uses:` ref(s):", file=sys.stderr) + for violation in violations: + print(f" {violation}", file=sys.stderr) + return 1 + print("OK: no unapproved mutable refs.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_check_pinned_refs.py b/scripts/test_check_pinned_refs.py new file mode 100644 --- /dev/null +++ b/scripts/test_check_pinned_refs.py @@ -0,0 +1,109 @@ +import tempfile +import unittest +from pathlib import Path + +from check_pinned_refs import find_violations, is_immutable, repo_of, uses_value + +SHA = "0123456789abcdef0123456789abcdef01234567" + + +class PinnedRefTests(unittest.TestCase): + def test_full_commit_sha_is_immutable(self): + self.assertTrue(is_immutable(SHA)) + self.assertFalse(is_immutable("v4")) + self.assertFalse(is_immutable("main")) + self.assertFalse(is_immutable(SHA[:-1])) + + def test_docker_digest_is_immutable(self): + self.assertTrue(is_immutable("sha256:" + "a" * 64)) + self.assertFalse(is_immutable("sha256:latest")) + self.assertFalse(is_immutable("sha256:" + "a" * 63)) + + def test_repo_of(self): + self.assertEqual( + repo_of("storytold/craft-common-actions/actions/foo"), + "storytold/craft-common-actions", + ) + self.assertEqual(repo_of("actions/checkout"), "actions/checkout") + self.assertIsNone(repo_of("actions")) + self.assertIsNone(repo_of("/actions")) + self.assertIsNone(repo_of("actions/")) + + def test_uses_value(self): + self.assertEqual( + uses_value(" - uses: actions/checkout@v4"), "actions/checkout@v4" + ) + self.assertEqual( + uses_value(' uses: "actions/checkout@v4" # v4'), "actions/checkout@v4" + ) + self.assertEqual( + uses_value(" uses: 'actions/checkout@v4'"), "actions/checkout@v4" + ) + self.assertIsNone(uses_value(" uses: # none")) + self.assertIsNone(uses_value(" # uses: actions/checkout@v4")) + self.assertIsNone(uses_value(" - name: checkout")) + + def test_mutable_ref_is_a_violation(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + workflows = root / ".github" / "workflows" + workflows.mkdir(parents=True) + (workflows / "test.yml").write_text( + "jobs:\n x:\n steps:\n - uses: actions/checkout@v4\n", + encoding="utf-8", + ) + checked, violations, exceptions = find_violations(root) + self.assertEqual(checked, 1) + self.assertEqual(len(violations), 1) + self.assertIn("test.yml:4", violations[0]) + self.assertEqual(exceptions, []) + + def test_first_party_main_is_allowlisted(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + workflows = root / ".github" / "workflows" + workflows.mkdir(parents=True) + (workflows / "test.yml").write_text( + "jobs:\n x:\n steps:\n" + " - uses: storytold/craft-common-actions/actions/foo@main\n", + encoding="utf-8", + ) + checked, violations, exceptions = find_violations(root) + self.assertEqual(checked, 1) + self.assertEqual(violations, []) + self.assertEqual(len(exceptions), 1) + + def test_pinned_and_local_refs_pass(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + workflows = root / ".github" / "workflows" + workflows.mkdir(parents=True) + (workflows / "test.yml").write_text( + "jobs:\n x:\n steps:\n" + f" - uses: actions/checkout@{SHA}\n" + " - uses: ./actions/local\n" + " - uses: docker://alpine@sha256:" + "a" * 64 + "\n", + encoding="utf-8", + ) + checked, violations, exceptions = find_violations(root) + self.assertEqual(checked, 3) + self.assertEqual(violations, []) + self.assertEqual(exceptions, []) + + def test_action_files_are_scanned(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + actions = root / "actions" / "foo" + actions.mkdir(parents=True) + (actions / "action.yml").write_text( + "runs:\n using: composite\n steps:\n" + " - uses: actions/checkout@v4\n", + encoding="utf-8", + ) + checked, violations, _ = find_violations(root) + self.assertEqual(checked, 1) + self.assertEqual(len(violations), 1) + + +if __name__ == "__main__": + unittest.main()