diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 834397686..fa53d52fe 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, 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. +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` (a language whose code didn't change uploads `main`'s analysis of the commit it was diffed against again, in place of a new one), 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 4ac652cad..bc348bfce 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 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. +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. A language whose code, CodeQL config and `codeql.yml` didn't change uploads `main`'s analysis of the commit it was diffed against for this commit, where `main` has one, rather than analysing again. 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 15b3898bb..8831ac8e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,8 @@ 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_unchanged: ${{ steps.scope.outputs.codeql_unchanged }} + codeql_base: ${{ steps.scope.outputs.codeql_base }} 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,6 +360,19 @@ jobs: - '.github/rulesets/**' - 'tools/ruleset.sh' - '.github/workflows/repo-checks.yml' + # What CodeQL extracts for each language without a build, its config, and the + # workflow that pins its version: a language none of them changed for has the same + # results as the commit this one is diffed against. + 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 @@ -383,6 +398,7 @@ jobs: DRAFT: ${{ steps.pr.outputs.draft }} ALL_OS: ${{ steps.pr.outputs.all_os }} BASE: ${{ github.base_ref }} + LAST_PASSED: ${{ steps.base.outputs.sha }} run: | matched() { [ "$(jq -r --arg f "$1" '.[$f] // "false"' <<< "$FILTER")" = true ]; } all=false @@ -450,9 +466,20 @@ jobs: # every language too. codeql=false if [ "$DRAFT" != true ] && { [ "$EVENT" != pull_request ] || [ "$BASE" = main ]; }; then codeql=true; fi + # A language whose code didn't change (not `all`: a change to this workflow doesn't change + # what CodeQL finds) reuses the analysis of the commit this one was diffed against: + # main's last passed one for a push or a merge queue group, the base a pull request's + # merge commit sits on for a pull request (codeql.yml finds that one). + unchanged=() + if [ "$EVENT" = pull_request ] || [ -n "$LAST_PASSED" ]; then + if ! matched codeql_javascript; then unchanged+=(javascript-typescript); fi + if ! matched codeql_rust; then unchanged+=(rust); fi + fi { for name in "${!out[@]}"; do echo "$name=${out[$name]}"; done echo "codeql=$codeql" + echo "codeql_unchanged=$(jq -cn '$ARGS.positional' --args "${unchanged[@]}")" + echo "codeql_base=$LAST_PASSED" echo "draft=${DRAFT:-false}" } >> "$GITHUB_OUTPUT" # The tools, areas and kinds of file the pull request changes, as labels (.github/labeler.yml). @@ -561,6 +588,9 @@ jobs: # Uploading the results to code scanning. security-events: write uses: ./.github/workflows/codeql.yml + with: + unchanged: ${{ needs.changes.outputs.codeql_unchanged }} + base: ${{ needs.changes.outputs.codeql_base }} # 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 67bcd5bf1..6ea1dd8aa 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -12,8 +12,22 @@ name: CodeQL # 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. +# +# A language whose code, CodeQL config and this workflow are as they were at the commit ci.yml +# diffed against has that commit's results, so its job uploads that commit's analysis from main +# again, for this commit, in place of analysing it: about a minute where Rust takes 15 to 19. +# With no such analysis on main yet (its push run hasn't got there), it analyses. on: workflow_call: + inputs: + unchanged: + description: The languages whose code didn't change, as a JSON list. + type: string + required: true + base: + description: The commit ci.yml diffed a push or a merge queue group against; empty for a pull request, whose merge commit's first parent is the one. + type: string + required: true permissions: {} @@ -36,13 +50,43 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false + # A pull request's merge commit and its first parent. + fetch-depth: 2 + # main's analysis of the base commit in this language, when its code is unchanged and main + # has one. The list is filtered here (see .github/actions/last-passed). + - id: reuse + if: contains(fromJSON(inputs.unchanged), matrix.language) + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + BASE: ${{ inputs.base }} + CATEGORY: /language:${{ matrix.language }} + run: | + base=${BASE:-$(git rev-parse HEAD^1)} + id=$(gh api "repos/$GH_REPO/code-scanning/analyses?ref=refs/heads/main&tool_name=CodeQL&per_page=100" | + jq -r --arg category "$CATEGORY" --arg sha "$base" \ + '[.[] | select(.category == $category and .commit_sha == $sha and .error == "")][0].id // empty') + if [ -z "$id" ]; then + echo "main has no $CATEGORY analysis of $base yet, so this one analyses" + exit 0 + fi + gh api -H "Accept: application/sarif+json" "repos/$GH_REPO/code-scanning/analyses/$id" > "$RUNNER_TEMP/main.sarif" + echo "Reusing analysis $id of $base" + echo "sarif=$RUNNER_TEMP/main.sarif" >> "$GITHUB_OUTPUT" + - if: steps.reuse.outputs.sarif != '' + uses: github/codeql-action/upload-sarif@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 + with: + sarif_file: ${{ steps.reuse.outputs.sarif }} + category: /language:${{ matrix.language }} # Nothing is built: Rust and TypeScript are read from the source, which is also what lets # paths-ignore leave files out. - - uses: github/codeql-action/init@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 + - if: steps.reuse.outputs.sarif == '' + uses: github/codeql-action/init@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 with: languages: ${{ matrix.language }} build-mode: none config-file: ./.github/codeql/codeql-config.yml - - uses: github/codeql-action/analyze@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 + - if: steps.reuse.outputs.sarif == '' + uses: github/codeql-action/analyze@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4.39.0 with: category: /language:${{ matrix.language }} diff --git a/tools/policy/workflows.rego b/tools/policy/workflows.rego index 5f15a7ec8..7db3ef4d4 100644 --- a/tools/policy/workflows.rego +++ b/tools/policy/workflows.rego @@ -267,3 +267,21 @@ deny contains msg if { not filtered("locallibrary", path) msg := sprintf("%s: the Rust code embeds it, so add it to the locallibrary paths filter", [path]) } + +# The filters that say a language's CodeQL results can be reused (ci.yml's codeql_*). +codeql_filters contains name if { + some _, wf in gated + some step in wf.jobs.changes.steps + startswith(step.uses, "dorny/paths-filter@") + some name, _ in yaml.unmarshal(step["with"].filters) + startswith(name, "codeql_") +} + +# CodeQL's results change with its config and with the action codeql.yml pins, so a change to +# either analyses every language again rather than reusing main's. +deny contains msg if { + some name in codeql_filters + some path in [".github/codeql/codeql-config.yml", ".github/workflows/codeql.yml"] + not filtered(name, path) + msg := sprintf("%s: add it to the %s paths filter; CodeQL's results change with it", [path, name]) +} diff --git a/tools/policy/workflows_test.rego b/tools/policy/workflows_test.rego index 99bae1698..364ff2252 100644 --- a/tools/policy/workflows_test.rego +++ b/tools/policy/workflows_test.rego @@ -217,3 +217,20 @@ test_embedded_file_outside_the_filter if { test_embedded_file_the_filter_excludes if { "crates/x/README.md: the Rust code embeds it, so add it to the locallibrary paths filter" in workflows.deny with input as embedded(["crates/x/README.md"]) } + +with_codeql_filter(globs) := with_ci(json.patch(ci, [{"op": "replace", "path": "/jobs/changes/steps/1/with/filters", "value": sprintf("run:\n - 'apps/**'\n - 'packages/**'\ncodeql_rust:\n%s", [concat("", [sprintf(" - '%s'\n", [g]) | some g in globs])])}])) + +test_codeql_filter_without_its_workflow if { + ".github/workflows/codeql.yml: add it to the codeql_rust paths filter; CodeQL's results change with it" in workflows.deny with input as with_codeql_filter(["**/*.rs", ".github/codeql/**"]) +} + +test_codeql_filter_without_its_config if { + ".github/codeql/codeql-config.yml: add it to the codeql_rust paths filter; CodeQL's results change with it" in workflows.deny with input as with_codeql_filter(["**/*.rs", ".github/workflows/codeql.yml"]) +} + +test_codeql_filter_with_both if { + denied := workflows.deny with input as with_codeql_filter(["**/*.rs", ".github/codeql/**", ".github/workflows/codeql.yml"]) + every msg in denied { + not contains(msg, "codeql_rust") + } +}