diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index e58bb331..dbc73713 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -4,13 +4,19 @@ on: push: branches: ["master", "main"] pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: ["master", "main"] permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.run_attempt == 1 && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: quality: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} runs-on: ubuntu-latest steps: @@ -27,6 +33,11 @@ jobs: python3 -m pip install --user yamllint python3 -m yamllint .yamllint.yml .github/dependabot.yml .github/workflows/*.yml + - name: Validate workflow concurrency contract + run: | + python3 scripts/ci/test_workflow_concurrency_contract_unit.py + python3 scripts/ci/test_workflow_concurrency_contract.py + - name: Lint markdown docs run: | npm install -g markdownlint-cli2@0.18.1 diff --git a/.github/workflows/r.yml b/.github/workflows/r.yml index 54eef61a..01dab867 100644 --- a/.github/workflows/r.yml +++ b/.github/workflows/r.yml @@ -4,6 +4,7 @@ on: push: branches: ["master", "main"] pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: ["master", "main"] workflow_dispatch: @@ -11,11 +12,12 @@ permissions: contents: read concurrency: - group: r-cmd-check-${{ github.ref }} - cancel-in-progress: true + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.run_attempt == 1 && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: check: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} runs-on: ubuntu-latest env: R_PROFILE_USER: /dev/null diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml index b6e46e2a..3273528f 100644 --- a/.github/workflows/security-audit.yml +++ b/.github/workflows/security-audit.yml @@ -4,13 +4,19 @@ on: push: branches: ["master", "main"] pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: ["master", "main"] permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.run_attempt == 1 && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: secret-and-workflow-audit: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} runs-on: ubuntu-latest steps: diff --git a/scripts/ci/test_workflow_concurrency_contract.py b/scripts/ci/test_workflow_concurrency_contract.py new file mode 100644 index 00000000..68b82e28 --- /dev/null +++ b/scripts/ci/test_workflow_concurrency_contract.py @@ -0,0 +1,140 @@ +"""Validate repository-owned workflow concurrency without parsing lookalike text.""" + +from pathlib import Path + + +WORKFLOWS = Path(".github/workflows") +EXPECTED_GROUP = ( + "${{ github.workflow }}-${{ github.repository }}-" + "${{ github.event_name == 'pull_request' && github.run_attempt == 1 && " + "github.event.pull_request.number || github.run_id }}" +) +EXPECTED_CANCEL = "${{ github.event_name == 'pull_request' }}" +EXPECTED_PR_TYPES = "types: [opened, synchronize, reopened, ready_for_review]" +EXPECTED_PR_ADMISSION = ( + "${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }}" +) + + +def discover_workflows(root: Path = WORKFLOWS) -> list[Path]: + """Return every YAML workflow file under the repository workflow directory.""" + return sorted({*root.glob("*.yml"), *root.glob("*.yaml")}) + + +def _top_level_concurrency_lines(path: Path, text: str) -> list[str]: + """Return non-comment entries from the sole top-level concurrency block.""" + lines = text.splitlines() + starts = [ + index + for index, line in enumerate(lines) + if line == "concurrency:" + ] + assert len(starts) == 1, f"{path}: expected exactly one top-level concurrency block" + + start = starts[0] + 1 + end = len(lines) + for index in range(start, len(lines)): + stripped = lines[index].strip() + if not stripped or stripped.startswith("#"): + continue + if lines[index][0] not in " \t": + end = index + break + + return [ + line[2:] + for line in lines[start:end] + if line.startswith(" ") and not line.startswith(" ") + ] + + +def _has_pull_request_trigger(text: str) -> bool: + """Return whether the workflow has a top-level pull-request trigger block.""" + lines = text.splitlines() + try: + start = lines.index("on:") + 1 + except ValueError: + return False + for line in lines[start:]: + if line and not line[0].isspace(): + break + if line == " pull_request:": + return True + return False + + +def _pull_request_types(text: str) -> list[str]: + """Return direct entries from the top-level pull-request trigger.""" + lines = text.splitlines() + start = lines.index(" pull_request:") + 1 + entries: list[str] = [] + for line in lines[start:]: + if line and ( + not line[0].isspace() + or (line.startswith(" ") and not line.startswith(" ")) + ): + break + if line.startswith(" ") and not line.startswith(" "): + entries.append(line[4:]) + return entries + + +def _job_admissions(text: str) -> list[str]: + """Return direct ``if`` values for every top-level job.""" + lines = text.splitlines() + start = lines.index("jobs:") + 1 + admissions: list[str] = [] + for index in range(start, len(lines)): + line = lines[index] + if line and not line[0].isspace(): + break + if line.startswith(" ") and not line.startswith(" ") and line.endswith(":"): + job_end = next( + ( + candidate + for candidate in range(index + 1, len(lines)) + if lines[candidate].startswith(" ") + and not lines[candidate].startswith(" ") + ), + len(lines), + ) + direct_if = [ + entry[8:] + for entry in lines[index + 1 : job_end] + if entry.startswith(" if: ") + ] + admissions.extend(direct_if or [""]) + return admissions + + +def validate_workflow_text(path: Path, text: str) -> None: + """Require exact group and PR-only cancellation values in top-level concurrency.""" + entries = _top_level_concurrency_lines(path, text) + groups = [entry for entry in entries if entry.startswith("group:")] + cancellations = [entry for entry in entries if entry.startswith("cancel-in-progress:")] + + assert groups == [f"group: {EXPECTED_GROUP}"], f"{path}: unsafe concurrency group" + assert cancellations == [ + f"cancel-in-progress: {EXPECTED_CANCEL}" + ], f"{path}: unsafe cancellation policy" + assert _has_pull_request_trigger( + text + ), f"{path}: missing structured pull-request trigger" + assert EXPECTED_PR_TYPES in _pull_request_types( + text + ), f"{path}: incomplete pull-request lifecycle" + assert _job_admissions(text) and all( + admission == EXPECTED_PR_ADMISSION for admission in _job_admissions(text) + ), f"{path}: draft pull requests occupy a runner" + + +def main() -> None: + """Validate every source-backed workflow in the repository.""" + files = discover_workflows() + assert files, "no workflows found" + for path in files: + validate_workflow_text(path, path.read_text()) + + +if __name__ == "__main__": + main() diff --git a/scripts/ci/test_workflow_concurrency_contract_unit.py b/scripts/ci/test_workflow_concurrency_contract_unit.py new file mode 100644 index 00000000..4e58f609 --- /dev/null +++ b/scripts/ci/test_workflow_concurrency_contract_unit.py @@ -0,0 +1,109 @@ +import tempfile +import unittest +from pathlib import Path + +from test_workflow_concurrency_contract import discover_workflows, validate_workflow_text + + +VALID = """name: Example +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] +concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name == 'pull_request' && github.run_attempt == 1 && github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} +jobs: + check: + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.draft == false }} + runs-on: ubuntu-latest +""" + + +class WorkflowConcurrencyContractTest(unittest.TestCase): + """Exercise discovery and top-level concurrency parsing edge cases.""" + + def test_discovers_yml_and_yaml(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "a.yml").write_text("name: a\n") + (root / "b.yaml").write_text("name: b\n") + (root / "ignored.txt").write_text("name: ignored\n") + self.assertEqual([path.name for path in discover_workflows(root)], ["a.yml", "b.yaml"]) + + def test_accepts_exact_top_level_contract(self) -> None: + validate_workflow_text(Path("valid.yml"), VALID) + + def test_rejects_noop_pull_request_lifecycle_events(self) -> None: + malformed = VALID.replace( + "types: [opened, synchronize, reopened, ready_for_review]", + "types: [opened, synchronize, reopened, ready_for_review, converted_to_draft, closed]", + ) + with self.assertRaisesRegex(AssertionError, "pull-request lifecycle"): + validate_workflow_text(Path("noop-events.yml"), malformed) + + def test_rejects_nested_lookalike(self) -> None: + malformed = """name: Example +on: pull_request +jobs: + check: + concurrency: + group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + runs-on: ubuntu-latest +""" + with self.assertRaisesRegex(AssertionError, "top-level concurrency"): + validate_workflow_text(Path("nested.yml"), malformed) + + def test_rejects_nested_concurrency_entries(self) -> None: + malformed = VALID.replace(" group:", " policy:\n group:").replace( + " cancel-in-progress:", " cancel-in-progress:" + ) + with self.assertRaisesRegex(AssertionError, "unsafe concurrency group"): + validate_workflow_text(Path("nested-entries.yml"), malformed) + + def test_rejects_duplicate_top_level_concurrency(self) -> None: + with self.assertRaisesRegex(AssertionError, "exactly one top-level concurrency"): + validate_workflow_text(Path("duplicate.yml"), VALID + "\nconcurrency:\n group: duplicate\n") + + def test_rejects_wrong_group(self) -> None: + malformed = VALID.replace("github.repository", "github.ref") + with self.assertRaisesRegex(AssertionError, "unsafe concurrency group"): + validate_workflow_text(Path("wrong-group.yml"), malformed) + + def test_rejects_unconditional_cancellation(self) -> None: + malformed = VALID.replace( + "cancel-in-progress: ${{ github.event_name == 'pull_request' }}", + "cancel-in-progress: true", + ) + with self.assertRaisesRegex(AssertionError, "unsafe cancellation policy"): + validate_workflow_text(Path("wrong-cancel.yml"), malformed) + + def test_rejects_draft_runner_admission(self) -> None: + malformed = VALID.replace( + "github.event.pull_request.draft == false", + "github.event.pull_request.draft == true", + ) + with self.assertRaisesRegex(AssertionError, "draft pull requests"): + validate_workflow_text(Path("draft.yml"), malformed) + + def test_rejects_rerun_that_shares_the_pull_request_group(self) -> None: + malformed = VALID.replace(" && github.run_attempt == 1", "") + with self.assertRaisesRegex(AssertionError, "unsafe concurrency group"): + validate_workflow_text(Path("rerun.yml"), malformed) + + def test_rejects_flow_style_pull_request_trigger(self) -> None: + malformed = VALID.replace( + "on:\n pull_request:\n types: [opened, synchronize, reopened, ready_for_review]", + "on: [pull_request]", + ) + with self.assertRaisesRegex(AssertionError, "pull-request trigger"): + validate_workflow_text(Path("flow.yml"), malformed) + + def test_ignores_comment_lookalikes(self) -> None: + malformed = VALID.replace(" pull_request:", " # pull_request:") + with self.assertRaisesRegex(AssertionError, "pull-request trigger"): + validate_workflow_text(Path("comment.yml"), malformed) + + +if __name__ == "__main__": + unittest.main()