diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c04f5338..ee6803955 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,8 +3,11 @@ name: CI on: push: branches: [main] + # No base-branch filter. A `branches:` allowlist here gates on the PR's *base*, + # so a PR stacked onto another PR's branch ran no CI at all (see #1440, which + # targets #1381's branch and has zero CI runs). Every PR gets the gate; the + # push trigger above stays scoped to main. pull_request: - branches: [main] permissions: contents: read diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 4948f9539..bdad9e215 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -3,8 +3,9 @@ name: "CodeQL Analysis" on: push: branches: [ "main" ] + # No base-branch filter — see the note in ci.yml. CodeQL is a security gate; + # a stacked PR must not be able to skip it by targeting a non-main base. pull_request: - branches: [ "main" ] schedule: - cron: '0 6 * * 1' diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 521c11795..417bb83d7 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -6,8 +6,8 @@ on: paths-ignore: - "**/*.md" - "**/*.txt" + # No base-branch filter — see the note in ci.yml. pull_request: - branches: [main, develop] paths-ignore: - "**/*.md" - "**/*.txt" diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index a7efcad8c..1ff5a5eb5 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -1,8 +1,8 @@ name: 🔍 Dependency Review on: + # No base-branch filter — see the note in ci.yml. pull_request: - branches: [ main, develop ] types: [opened, synchronize, reopened, ready_for_review] permissions: diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 547c4b356..3bc529fd7 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -3,8 +3,8 @@ name: E2E Tests on: push: branches: [main] + # No base-branch filter — see the note in ci.yml. pull_request: - branches: [main] permissions: contents: read diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 9f622ad5f..dee5f5ce8 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -3,8 +3,9 @@ name: Security Scan on: push: branches: [main] + # No base-branch filter — see the note in ci.yml. A stacked PR must not be + # able to skip the security scan by targeting a branch other than main. pull_request: - branches: [main] schedule: - cron: '0 0 * * 0' # Weekly on Sunday diff --git a/tests/unit/test_workflow_pr_gate_coverage.py b/tests/unit/test_workflow_pr_gate_coverage.py new file mode 100644 index 000000000..64ebc39d1 --- /dev/null +++ b/tests/unit/test_workflow_pr_gate_coverage.py @@ -0,0 +1,77 @@ +"""Every PR quality gate must run on every PR, whatever its base branch. + +A `pull_request.branches:` allowlist filters on the PR's *base*, not its head. +Six gates carried `branches: [main]`, so a PR stacked onto another PR's branch +matched none of them and ran with no CI, no CodeQL, no security scan, no +coverage, no e2e and no dependency review. #1440 -- a security fix targeting +#1381's branch -- reached "ready for review" with zero CI runs that way. + +These tests pin the trigger shape so the allowlist cannot come back. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + +WORKFLOWS = Path(__file__).resolve().parents[2] / ".github/workflows" + +# PR-triggered quality gates. Each must fire regardless of the PR's base branch. +GATED_WORKFLOWS = [ + "ci.yml", + "security.yml", + "coverage.yml", + "e2e-tests.yml", + "codeql-analysis.yml", + "dependency-review.yml", +] + + +def _triggers(filename: str) -> dict: + path = WORKFLOWS / filename + assert path.exists(), f"{filename} should exist" + workflow = yaml.safe_load(path.read_text()) + # PyYAML parses the YAML 'on' key as the Python bool True. + return workflow[True] + + +@pytest.mark.parametrize("filename", GATED_WORKFLOWS) +def test_gate_triggers_on_pull_request(filename: str) -> None: + assert "pull_request" in _triggers(filename), ( + f"{filename} must run on pull_request to gate PRs at all" + ) + + +@pytest.mark.parametrize("filename", GATED_WORKFLOWS) +def test_gate_has_no_base_branch_allowlist(filename: str) -> None: + """The defect itself: a base-branch allowlist lets stacked PRs skip the gate.""" + config = _triggers(filename)["pull_request"] + # A bare `pull_request:` parses to None, which is the shape we want. + if config is None: + return + assert "branches" not in config, ( + f"{filename} filters pull_request on base branch {config['branches']!r}; " + "a PR stacked onto a non-main branch would skip this gate entirely" + ) + assert "branches-ignore" not in config, ( + f"{filename} uses branches-ignore on pull_request, which has the same " + "stacked-PR bypass as a branches allowlist" + ) + + +@pytest.mark.parametrize("filename", GATED_WORKFLOWS) +def test_push_trigger_stays_scoped_to_main(filename: str) -> None: + """Widening the PR trigger must not widen the push trigger with it. + + Un-scoping `push` would run the full suite twice on every branch push. + """ + triggers = _triggers(filename) + if "push" not in triggers: + return + branches = triggers["push"].get("branches") + assert branches, f"{filename} push trigger should stay pinned to a branch list" + assert set(branches) <= {"main", "develop"}, ( + f"{filename} push trigger widened to {branches!r}" + )