diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 2b3119fcd..834397686 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -61,7 +61,7 @@ On a pull request, each tool's workflow keeps one comment up to date with its co Pull requests that change a lockfile, a manifest or a workflow also get a dependency review (`dependency-review.yml`). It fails when a dependency the pull request adds or updates (dev tooling aside) has a known advisory or a licence `deny.toml` doesn't allow (fonts may also be OFL-1.1), and then comments on the pull request with what it found. -CodeQL (`codeql.yml`, which `ci.yml` calls) scans the workflows on every pull request into `main`, merge queue group and push to `main`, and the TypeScript and the Rust when their code changed, and `main` won't take a pull request that adds a high or critical security alert, or an error. The Rust tests are left out (`.github/codeql/codeql-config.yml`), so a new `#[cfg(test)]` module needs one of the names listed there, which `ast-grep scan` checks. +CodeQL (`codeql.yml`, which `ci.yml` calls) scans the workflows, the TypeScript and the Rust on every pull request into `main`, merge queue group and push to `main`, and `main` won't take a pull request that adds a high or critical security alert, or an error. The Rust tests are left out (`.github/codeql/codeql-config.yml`), so a new `#[cfg(test)]` module needs one of the names listed there, which `ast-grep scan` checks. ## Pull requests diff --git a/.github/NEW_PROJECT.md b/.github/NEW_PROJECT.md index aa3562e43..eb67f3ef9 100644 --- a/.github/NEW_PROJECT.md +++ b/.github/NEW_PROJECT.md @@ -71,7 +71,7 @@ Jobs that only report have `continue-on-error: true`, so they never fail their c Pull requests run on Linux. The plan's 5 macOS runners can't keep up with every push, so locallibrary's macOS and Windows job (`other-os`) runs once for each merge queue group, which runs nothing else but CodeQL, or on a pull request with the `all-os` label. `locallibrary-cache.yml` runs it nightly on `main`, which is where its caches are saved, when what they're keyed on changed or they were evicted. A push to `main`, whose commit is the merge queue group's, runs only what pull requests use from it: the website's and localtoolbox's coverage and sizes, which the reports compare with, the Rust and Docker caches when what they're keyed on changed, CodeQL, and the ruleset check when the ruleset file changed. -The "Protect Main" ruleset requires "CI passed", which passes when it's skipped, so a tool's checks never block a pull request that doesn't touch it. The ruleset is `.github/rulesets/protect-main.json`: an admin applies it with `tools/ruleset.sh apply` once the pull request that changes it merges, and the Repo checks workflow fails on `main` while GitHub's copy differs. A pull request that renames or drops a required check is the exception: GitHub won't merge it while the old name is required, so the admin applies the file from its branch first, and open pull requests then merge `main` to report the new name. It merges through a merge queue, which builds at most 2 groups of up to 5 pull requests at once, and requires CI passed and code scanning, which is `codeql.yml`'s CodeQL analysis of the workflows, the TypeScript and the Rust, leaving out the Rust tests (`.github/codeql/codeql-config.yml`). `ci.yml` calls it with the workflows every time, so each commit has a result, and the other languages when their code changed. +The "Protect Main" ruleset requires "CI passed", which passes when it's skipped, so a tool's checks never block a pull request that doesn't touch it. The ruleset is `.github/rulesets/protect-main.json`: an admin applies it with `tools/ruleset.sh apply` once the pull request that changes it merges, and the Repo checks workflow fails on `main` while GitHub's copy differs. A pull request that renames or drops a required check is the exception: GitHub won't merge it while the old name is required, so the admin applies the file from its branch first, and open pull requests then merge `main` to report the new name. It merges through a merge queue, which builds at most 2 groups of up to 5 pull requests at once, and requires CI passed and code scanning, which is `codeql.yml`'s CodeQL analysis of the workflows, the TypeScript and the Rust, leaving out the Rust tests (`.github/codeql/codeql-config.yml`). `ci.yml` calls it with every language on every pull request, group and push, whatever changed: the rule holds a commit until it has a result in each category `main` has. Changes reach `main` only through a pull request, merged as a merge commit. Each one needs an approval from a code owner (`.github/CODEOWNERS`) given after its last push, and a new push dismisses earlier approvals, so an agent's pull request waits for a person to approve it. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a60bd3370..81c78eb54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,7 +71,6 @@ jobs: repo_checks_lexicons: ${{ steps.scope.outputs.repo_checks_lexicons }} repo_checks_ruleset: ${{ steps.scope.outputs.repo_checks_ruleset }} codeql: ${{ steps.scope.outputs.codeql }} - codeql_languages: ${{ steps.scope.outputs.codeql_languages }} draft: ${{ steps.scope.outputs.draft }} steps: # A pull request's files come from the API. A push is diffed against the last commit this @@ -358,17 +357,6 @@ jobs: - '.github/rulesets/**' - 'tools/ruleset.sh' - '.github/workflows/repo-checks.yml' - # What CodeQL extracts for each language without a build, and its config. - codeql_rust: - - '**/*.rs' - - '**/Cargo.toml' - - 'Cargo.lock' - - '.github/codeql/**' - - '.github/workflows/codeql.yml' - codeql_javascript: - - '**/*.{js,mjs,cjs,jsx,ts,mts,cts,tsx,svelte,vue,html}' - - '.github/codeql/**' - - '.github/workflows/codeql.yml' # Whether the pull request is a draft now, and has the `all-os` label, from the API. The event # has the pull request as it was when the event was made: #211 was marked ready a second after # a push, GitHub started a run for the push and none for ready_for_review, and the push's event @@ -453,20 +441,17 @@ jobs: set_if web any web_verify web_integration web_e2e web_lighthouse web_docker set_if repo_checks any repo_checks_policy repo_checks_lexicons repo_checks_ruleset - # CodeQL on every event but a draft's and a pull request's into another branch than main: - # the code scanning rule holds a pull request or a merge queue group until it has CodeQL's - # results for its commit, and pull requests' alerts are compared with main's. The workflows' analysis always runs (it takes about a minute), - # so every commit has a result; Rust's and TypeScript's only when their code changed. - languages=() - if [ "$DRAFT" != true ] && { [ "$EVENT" != pull_request ] || [ "$BASE" = main ]; }; then - languages+=(actions) - if changed codeql_javascript; then languages+=(javascript-typescript); fi - if changed codeql_rust; then languages+=(rust); fi - fi + # CodeQL, every language, on every event but a draft's and a pull request's into another + # branch than main. The code scanning rule holds a pull request or a merge queue group until + # its commit has a result in every category main has: #347, which changed no Rust and got + # no Rust analysis, couldn't join the queue ("Code scanning is still expecting 1 result + # from CodeQL"). Pull requests' alerts are compared with main's, so main's pushes analyse + # every language too. + codeql=false + if [ "$DRAFT" != true ] && { [ "$EVENT" != pull_request ] || [ "$BASE" = main ]; }; then codeql=true; fi { for name in "${!out[@]}"; do echo "$name=${out[$name]}"; done - echo "codeql=$([ ${#languages[@]} -gt 0 ] && echo true || echo false)" - echo "codeql_languages=$(jq -cn '$ARGS.positional' --args "${languages[@]}")" + echo "codeql=$codeql" echo "draft=${DRAFT:-false}" } >> "$GITHUB_OUTPUT" # The tools, areas and kinds of file the pull request changes, as labels (.github/labeler.yml). @@ -575,8 +560,6 @@ jobs: # Uploading the results to code scanning. security-events: write uses: ./.github/workflows/codeql.yml - with: - languages: ${{ needs.changes.outputs.codeql_languages }} # The check the "Protect Main" ruleset requires. It starts a runner only to fail: when a tool's # workflow failed or was cancelled, or was skipped while its changes output was true (it didn't diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 7174b2a2d..67bcd5bf1 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -6,19 +6,14 @@ name: CodeQL # used, so its alerts carry over. # # ci.yml calls it for every pull request into main, merge queue group and push to main but a -# draft's, with the languages whose code changed. The "Protect Main" ruleset's code scanning rule -# waits for an analysis of a pull request's commit, and of each merge queue group's, and holds it -# until it gets one (netresearch/.github#170 had every queued pull request stuck on "Code scanning -# is still expecting 1 result from CodeQL"), so the workflows' analysis, a minute long, runs -# every time. CodeQL's own result is that check. No weekly run: main gets pushes most days, and -# each analyses with the queries of the action's pinned bundle. +# draft's, with every language whatever changed. The "Protect Main" ruleset's code scanning rule +# holds a pull request or a merge queue group until its commit has a result in every category +# main has (netresearch/.github#170 had every queued pull request stuck on "Code scanning is still +# expecting 1 result from CodeQL", and so did #347 when only the languages that changed ran). +# CodeQL's own result is that check. No weekly run: main gets pushes most days, and each analyses +# with the queries of the action's pinned bundle. on: workflow_call: - inputs: - languages: - description: The languages to analyse, as a JSON list. - type: string - required: true permissions: {} @@ -36,7 +31,7 @@ jobs: strategy: fail-fast: false matrix: - language: ${{ fromJSON(inputs.languages) }} + language: [actions, javascript-typescript, rust] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/tools/policy/ruleset.rego b/tools/policy/ruleset.rego index 9ee013a67..0b18c0efd 100644 --- a/tools/policy/ruleset.rego +++ b/tools/policy/ruleset.rego @@ -55,3 +55,14 @@ codeql_on_merge_groups if { some _, job in wf.jobs job.uses == "./.github/workflows/codeql.yml" } + +# The code scanning rule also holds a commit until it has a result in every category main has, so +# CodeQL analyses every language each time: #347 couldn't join the queue when only the languages +# whose code changed ran. +deny contains msg if { + some scanning in rules + scanning.type == "code_scanning" + some id, job in lib.file(".github/workflows/codeql.yml").jobs + not is_array(job.strategy.matrix.language) + msg := sprintf(".github/workflows/codeql.yml: list every language in job %q's matrix; the code scanning rule holds a commit until each category main has gets a result", [id]) +} diff --git a/tools/policy/ruleset_test.rego b/tools/policy/ruleset_test.rego index 6faed01a8..b8c93432c 100644 --- a/tools/policy/ruleset_test.rego +++ b/tools/policy/ruleset_test.rego @@ -34,7 +34,9 @@ test_merge_queue_with_merge_group if { scanning := {"type": "code_scanning", "parameters": {"code_scanning_tools": [{"tool": "CodeQL"}]}} -codeql(on) := {"path": ".github/workflows/codeql.yml", "contents": {"true": on, "jobs": {"analyze": {}}}} +every_language := {"strategy": {"matrix": {"language": ["actions", "rust"]}}} + +codeql(on) := {"path": ".github/workflows/codeql.yml", "contents": {"true": on, "jobs": {"analyze": every_language}}} with_queue_and_scanning(on) := array.concat( fixtures.with_file(".github/rulesets/protect-main.json", json.patch(fixtures.file(".github/rulesets/protect-main.json"), [ @@ -70,3 +72,15 @@ test_required_check_never_reports if { repo := fixtures.without_file(".github/workflows/deps.yml") ".github/rulesets/protect-main.json: no workflow reports \"Deps passed\", so nothing can merge" in ruleset.deny with input as repo } + +languages_msg := ".github/workflows/codeql.yml: list every language in job \"analyze\"'s matrix; the code scanning rule holds a commit until each category main has gets a result" + +test_codeql_with_every_language if { + not languages_msg in ruleset.deny with input as with_queue_and_scanning({"workflow_call": null}) +} + +test_codeql_with_languages_that_changed if { + some_languages := {"path": ".github/workflows/codeql.yml", "contents": {"true": {"workflow_call": null}, "jobs": {"analyze": {"strategy": {"matrix": {"language": "${{ fromJSON(inputs.languages) }}"}}}}}} + repo := array.concat([f | some f in with_queue_and_scanning({"workflow_call": null}); f.path != ".github/workflows/codeql.yml"], [some_languages]) + languages_msg in ruleset.deny with input as repo +}