workflows: small refactors (NixOS#371216)

Author: infinisil

.github/workflows/README.md

@@ -0,0 +1,20 @@
+# GitHub Actions Workflows
+
+Some architectural notes about key decisions and concepts in our workflows:
+
+- Instead of `pull_request` we use [`pull_request_target`](https://docs.github.com/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#pull_request_target) for all PR-related workflows. This has the advantage that those workflows will run without prior approval for external contributors.
+
+- Running on `pull_request_target` also optionally provides us with a GH_TOKEN with elevated privileges (write access), which we need to do things like adding labels, requesting reviewers or pushing branches. **Note about security:** We need to be careful to limit the scope of elevated privileges as much as possible. Thus they should be lowered to the minimum with `permissions: {}` in every workflow by default.
+
+- By definition `pull_request_target` runs in the context of the **base** of the pull request. This means, that the workflow files to run will be taken from the base branch, not the PR, and actions/checkout will not checkout the PR, but the base branch, by default. To protect our secrets, we need to make sure to **never execute code** from the pull request and always evaluate or build nix code from the pull request with the **sandbox enabled**.
+
+- To test the pull request's contents, we checkout the "test merge commit". This is a temporary commit that GitHub creates automatically as "what would happen, if this PR was merged into the base branch now?". The checkout could be done via the virtual branch `refs/pull/<pr-number>/merge`, but doing so would cause failures when this virtual branch doesn't exist (anymore). This can happen when the PR has conflicts, in which case the virtual branch is not created, or when the PR is getting merged while workflows are still running, in which case the branch won't exist anymore at the time of checkout. Thus, we use the `get-merge-commit.yml` workflow to check whether the PR is mergeable and the test merge commit exists and only then run the relevant jobs.
+
+- Various workflows need to make comparisons against the base branch. In this case, we checkout the parent of the "test merge commit" for best results. Note, that this is not necessarily the same as the default commit that actions/checkout would use, which is also a commit from the base branch (see above), but might be older.
+
+## Terminology
+
+- **base commit**: The pull_request_target event's context commit, i.e. the base commit given by GitHub Actions. Same as `github.event.pull_request.base.sha`.
+- **head commit**: The HEAD commit in the pull request's branch. Same as `github.event.pull_request.head.sha`.
+- **merge commit**: The temporary "test merge commit" that GitHub Actions creates and updates for the pull request. Same as `refs/pull/${{ github.event.pull_request.number }}/merge`.
+- **target commit**: The base branch's parent of the "test merge commit" to compare against.

.github/workflows/backport.yml

@@ -1,13 +1,14 @@
-name: Backport
-on:
-  pull_request_target:
-    types: [closed, labeled]
-
 # WARNING:
 # When extending this action, be aware that $GITHUB_TOKEN allows write access to
 # the GitHub repository. This means that it should not evaluate user input in a
 # way that allows code injection.
 
+name: Backport
+
+on:
+  pull_request_target:
+    types: [closed, labeled]
+
 permissions: {}
 
 jobs:
@@ -23,10 +24,12 @@ jobs:
         with:
           app-id: ${{ vars.BACKPORT_APP_ID }}
           private-key: ${{ secrets.BACKPORT_PRIVATE_KEY }}
+
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
           ref: ${{ github.event.pull_request.head.sha }}
           token: ${{ steps.app-token.outputs.token }}
+
       - name: Create backport PRs
         uses: korthout/backport-action@be567af183754f6a5d831ae90f648954763f17f5 # v3.1.0
         with:

.github/workflows/basic-eval.yml

@@ -1,10 +1,11 @@
 name: "Check cherry-picks"
+
 on:
   pull_request_target:
     branches:
-     - 'release-**'
-     - 'staging-**'
-     - '!staging-next'
+      - 'release-**'
+      - 'staging-**'
+      - '!staging-next'
 
 permissions: {}
 
@@ -14,13 +15,14 @@ jobs:
     runs-on: ubuntu-24.04
     if: github.repository_owner == 'NixOS'
     steps:
-    - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
-      with:
-        fetch-depth: 0
-        filter: blob:none
-    - name: Check cherry-picks
-      env:
-        BASE_SHA: ${{ github.event.pull_request.base.sha }}
-        HEAD_SHA: ${{ github.event.pull_request.head.sha }}
-      run: |
-        ./maintainers/scripts/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA"
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          fetch-depth: 0
+          filter: blob:none
+
+      - name: Check cherry-picks
+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        run: |
+          ./maintainers/scripts/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA"

.github/workflows/check-cherry-picks.yml

@@ -4,26 +4,25 @@ on:
   pull_request_target:
     paths:
       - 'maintainers/maintainer-list.nix'
-permissions:
-  contents: read
+
+permissions: {}
 
 jobs:
   nixos:
     name: maintainer-list-check
     runs-on: ubuntu-24.04
-    if: github.repository_owner == 'NixOS'
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
-          # pull_request_target checks out the base branch by default
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           # Only these directories to perform the check
           sparse-checkout: |
             lib
             maintainers
+
       - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30
         with:
-          # explicitly enable sandbox
           extra_nix_config: sandbox = true
+
       - name: Check that maintainer-list.nix is sorted
         run: nix-instantiate --eval maintainers/scripts/check-maintainers-sorted.nix

…/workflows/check-maintainers-sorted.yaml …b/workflows/check-maintainers-sorted.yml.github/workflows/check-maintainers-sorted.yaml renamed to .github/workflows/check-maintainers-sorted.yml .github/workflows/check-maintainers-sorted.yaml renamed to .github/workflows/check-maintainers-sorted.yml

@@ -3,14 +3,14 @@
 # https://github.com/NixOS/rfcs/pull/166.
 # Because of this, this action is not yet enabled for all files -- only for
 # those who have opted in.
+
 name: Check that Nix files are formatted
 
 on:
   pull_request_target:
-    # See the comment at the same location in ./nixpkgs-vet.yml
     types: [opened, synchronize, reopened, edited]
-permissions:
-  contents: read
+
+permissions: {}
 
 jobs:
   get-merge-commit:
@@ -24,31 +24,34 @@ jobs:
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
-          # pull_request_target checks out the base branch by default
           ref: ${{ needs.get-merge-commit.outputs.mergedSha }}
           # Fetches the merge commit and its parents
           fetch-depth: 2
-      - name: Checking out base branch
+
+      - name: Checking out target branch
         run: |
-          base=$(mktemp -d)
-          baseRev=$(git rev-parse HEAD^1)
-          git worktree add "$base" "$baseRev"
-          echo "baseRev=$baseRev" >> "$GITHUB_ENV"
-          echo "base=$base" >> "$GITHUB_ENV"
+          target=$(mktemp -d)
+          targetRev=$(git rev-parse HEAD^1)
+          git worktree add "$target" "$targetRev"
+          echo "targetRev=$targetRev" >> "$GITHUB_ENV"
+          echo "target=$target" >> "$GITHUB_ENV"
+
       - name: Get Nixpkgs revision for nixfmt
         run: |
           # pin to a commit from nixpkgs-unstable to avoid e.g. building nixfmt
           # from staging
           # This should not be a URL, because it would allow PRs to run arbitrary code in CI!
           rev=$(jq -r .rev ci/pinned-nixpkgs.json)
           echo "url=https://github.com/NixOS/nixpkgs/archive/$rev.tar.gz" >> "$GITHUB_ENV"
+
       - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30
         with:
-          # explicitly enable sandbox
           extra_nix_config: sandbox = true
           nix_path: nixpkgs=${{ env.url }}
+
       - name: Install nixfmt
         run: "nix-env -f '<nixpkgs>' -iAP nixfmt-rfc-style"
+
       - name: Check that Nix files are formatted according to the RFC style
         run: |
           unformattedFiles=()
@@ -78,12 +81,12 @@ jobs:
             esac
 
             # Ignore files that weren't already formatted
-            if [[ -n "$source" ]] && ! nixfmt --check ${{ env.base }}/"$source" 2>/dev/null; then
-              echo "Ignoring file $file because it's not formatted in the base commit"
+            if [[ -n "$source" ]] && ! nixfmt --check ${{ env.target }}/"$source" 2>/dev/null; then
+              echo "Ignoring file $file because it's not formatted in the target commit"
             elif ! nixfmt --check "$dest"; then
               unformattedFiles+=("$dest")
             fi
-          done < <(git diff -z --name-status ${{ env.baseRev }} -- '*.nix')
+          done < <(git diff -z --name-status ${{ env.targetRev }} -- '*.nix')
 
           if (( "${#unformattedFiles[@]}" > 0 )); then
             echo "Some new/changed Nix files are not properly formatted"

.github/workflows/check-nix-format.yml

@@ -3,8 +3,8 @@ name: Check changed Nix files with nixf-tidy (experimental)
 on:
   pull_request_target:
     types: [opened, synchronize, reopened, edited]
-permissions:
-  contents: read
+
+permissions: {}
 
 jobs:
   nixos:
@@ -14,32 +14,35 @@ jobs:
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
-          # pull_request_target checks out the base branch by default
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           # Fetches the merge commit and its parents
           fetch-depth: 2
-      - name: Checking out base branch
+
+      - name: Checking out target branch
         run: |
-          base=$(mktemp -d)
-          baseRev=$(git rev-parse HEAD^1)
-          git worktree add "$base" "$baseRev"
-          echo "baseRev=$baseRev" >> "$GITHUB_ENV"
-          echo "base=$base" >> "$GITHUB_ENV"
+          target=$(mktemp -d)
+          targetRev=$(git rev-parse HEAD^1)
+          git worktree add "$target" "$targetRev"
+          echo "targetRev=$targetRev" >> "$GITHUB_ENV"
+          echo "target=$target" >> "$GITHUB_ENV"
+
       - name: Get Nixpkgs revision for nixf
         run: |
           # pin to a commit from nixpkgs-unstable to avoid e.g. building nixf
           # from staging
           # This should not be a URL, because it would allow PRs to run arbitrary code in CI!
           rev=$(jq -r .rev ci/pinned-nixpkgs.json)
           echo "url=https://github.com/NixOS/nixpkgs/archive/$rev.tar.gz" >> "$GITHUB_ENV"
+
       - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30
         with:
-          # explicitly enable sandbox
           extra_nix_config: sandbox = true
           nix_path: nixpkgs=${{ env.url }}
+
       - name: Install nixf and jq
         # provided jq is incompatible with our expression
         run: "nix-env -f '<nixpkgs>' -iAP nixf jq"
+
       - name: Check that Nix files pass nixf-tidy
         run: |
           # Filtering error messages we don't like
@@ -85,8 +88,8 @@ jobs:
                 continue
             esac
 
-            if [[ -n "$source" ]] && [[ "$(nixf_wrapper ${{ env.base }}/"$source")" != '[]' ]] 2>/dev/null; then
-              echo "Ignoring file $file because it doesn't pass nixf-tidy in the base commit"
+            if [[ -n "$source" ]] && [[ "$(nixf_wrapper ${{ env.target }}/"$source")" != '[]' ]] 2>/dev/null; then
+              echo "Ignoring file $file because it doesn't pass nixf-tidy in the target commit"
               echo # insert blank line
             else
               nixf_report="$(nixf_wrapper "$dest")"
@@ -113,7 +116,7 @@ jobs:
                 failedFiles+=("$dest")
               fi
             fi
-          done < <(git diff -z --name-status ${{ env.baseRev }} -- '*.nix')
+          done < <(git diff -z --name-status ${{ env.targetRev }} -- '*.nix')
 
           if [[ -n "$DONT_REPORT_ERROR" ]]; then
             echo "Edited the PR but didn't change the base branch, only the description/title."

.github/workflows/check-nixf-tidy.yml

@@ -9,26 +9,25 @@ on:
 permissions: {}
 
 jobs:
-  x86_64-linux:
-    name: shell-check-x86_64-linux
-    runs-on: ubuntu-24.04
-    steps:
-      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
-        with:
-          # pull_request_target checks out the base branch by default
-          ref: refs/pull/${{ github.event.pull_request.number }}/merge
-      - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30
-      - name: Build shell
-        run: nix-build shell.nix
+  shell-check:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - runner: ubuntu-24.04
+            system: x86_64-linux
+          - runner: macos-14
+            system: aarch64-darwin
+
+    name: shell-check-${{ matrix.system }}
+    runs-on: ${{ matrix.runner }}
 
-  aarch64-darwin:
-    name: shell-check-aarch64-darwin
-    runs-on: macos-14
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
-          # pull_request_target checks out the base branch by default
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
+
       - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30
+
       - name: Build shell
         run: nix-build shell.nix