diff --git a/.github/workflows/README.md b/.github/workflows/README.md new file mode 100644 index 0000000000000..7089501d5e405 --- /dev/null +++ b/.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//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. diff --git a/.github/workflows/check-maintainers-sorted.yml b/.github/workflows/check-maintainers-sorted.yml index 266e56fe989ed..07cd525e85428 100644 --- a/.github/workflows/check-maintainers-sorted.yml +++ b/.github/workflows/check-maintainers-sorted.yml @@ -5,8 +5,7 @@ on: paths: - 'maintainers/maintainer-list.nix' -permissions: - contents: read +permissions: {} jobs: nixos: @@ -15,7 +14,6 @@ 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 # Only these directories to perform the check sparse-checkout: | @@ -24,7 +22,6 @@ jobs: - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true - name: Check that maintainer-list.nix is sorted diff --git a/.github/workflows/check-nix-format.yml b/.github/workflows/check-nix-format.yml index b276b3575eed5..a70e132dc459a 100644 --- a/.github/workflows/check-nix-format.yml +++ b/.github/workflows/check-nix-format.yml @@ -8,11 +8,9 @@ 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: @@ -26,7 +24,6 @@ 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 @@ -49,7 +46,6 @@ jobs: - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true nix_path: nixpkgs=${{ env.url }} diff --git a/.github/workflows/check-nixf-tidy.yml b/.github/workflows/check-nixf-tidy.yml index 8907e003852f8..8b148ba33bc44 100644 --- a/.github/workflows/check-nixf-tidy.yml +++ b/.github/workflows/check-nixf-tidy.yml @@ -4,8 +4,7 @@ on: pull_request_target: types: [opened, synchronize, reopened, edited] -permissions: - contents: read +permissions: {} jobs: nixos: @@ -15,7 +14,6 @@ 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 @@ -38,7 +36,6 @@ jobs: - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true nix_path: nixpkgs=${{ env.url }} diff --git a/.github/workflows/check-shell.yml b/.github/workflows/check-shell.yml index 3a153860a09ce..e1f079619dc37 100644 --- a/.github/workflows/check-shell.yml +++ b/.github/workflows/check-shell.yml @@ -25,7 +25,6 @@ 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 - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 diff --git a/.github/workflows/codeowners-v2.yml b/.github/workflows/codeowners-v2.yml index ca68edeaab21b..8b5267b25c630 100644 --- a/.github/workflows/codeowners-v2.yml +++ b/.github/workflows/codeowners-v2.yml @@ -26,7 +26,6 @@ on: pull_request_target: types: [opened, ready_for_review, synchronize, reopened, edited] -# We don't need any default GitHub token permissions: {} env: diff --git a/.github/workflows/editorconfig-v2.yml b/.github/workflows/editorconfig-v2.yml index ecf0a075430e2..bd48be1650f1a 100644 --- a/.github/workflows/editorconfig-v2.yml +++ b/.github/workflows/editorconfig-v2.yml @@ -1,12 +1,9 @@ name: "Checking EditorConfig v2" on: - # avoids approving first time contributors pull_request_target: -permissions: - pull-requests: read - contents: read +permissions: {} jobs: get-merge-commit: @@ -33,7 +30,6 @@ jobs: - 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 }} - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 diff --git a/.github/workflows/eval-lib-tests.yml b/.github/workflows/eval-lib-tests.yml index 03efa973ef8c8..065fe8fdb282c 100644 --- a/.github/workflows/eval-lib-tests.yml +++ b/.github/workflows/eval-lib-tests.yml @@ -5,8 +5,7 @@ on: paths: - 'lib/**' -permissions: - contents: read +permissions: {} jobs: get-merge-commit: @@ -20,12 +19,10 @@ 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 }} - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true - name: Building Nixpkgs lib-tests diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 3021fb12d7e19..28a93773f5f0a 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -12,8 +12,7 @@ on: - haskell-updates - python-updates -permissions: - contents: read +permissions: {} jobs: get-merge-commit: @@ -23,7 +22,6 @@ jobs: name: Attributes runs-on: ubuntu-24.04 needs: get-merge-commit - # Skip this and dependent steps if the PR can't be merged if: needs.get-merge-commit.outputs.mergedSha outputs: targetSha: ${{ steps.targetSha.outputs.targetSha }} @@ -45,6 +43,8 @@ jobs: - name: Install Nix uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 + with: + extra_nix_config: sandbox = true - name: Evaluate the list of all attributes and get the systems matrix id: systems @@ -71,6 +71,8 @@ jobs: - name: Install Nix uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 + with: + extra_nix_config: sandbox = true - name: Query nixpkgs with aliases enabled to check for basic syntax errors run: | @@ -106,6 +108,8 @@ jobs: - name: Install Nix uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 + with: + extra_nix_config: sandbox = true - name: Evaluate the ${{ matrix.system }} output paths for all derivation attributes env: @@ -145,6 +149,8 @@ jobs: - name: Install Nix uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 + with: + extra_nix_config: sandbox = true - name: Combine all output paths and eval stats run: | diff --git a/.github/workflows/get-merge-commit.yml b/.github/workflows/get-merge-commit.yml index 827c86316b8b5..a32595ae1ad44 100644 --- a/.github/workflows/get-merge-commit.yml +++ b/.github/workflows/get-merge-commit.yml @@ -7,7 +7,6 @@ on: description: "The merge commit SHA" value: ${{ jobs.resolve-merge-commit.outputs.mergedSha }} -# We need a token to query the API, but it doesn't need any special permissions permissions: {} jobs: diff --git a/.github/workflows/manual-nixos-v2.yml b/.github/workflows/manual-nixos-v2.yml index e022f27ac693e..c83d53e8a51aa 100644 --- a/.github/workflows/manual-nixos-v2.yml +++ b/.github/workflows/manual-nixos-v2.yml @@ -7,8 +7,7 @@ on: paths: - 'nixos/**' -permissions: - contents: read +permissions: {} jobs: nixos: @@ -17,12 +16,10 @@ 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 - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true - uses: cachix/cachix-action@ad2ddac53f961de1989924296a1f236fcfbaa4fc # v15 diff --git a/.github/workflows/manual-nixpkgs-v2.yml b/.github/workflows/manual-nixpkgs-v2.yml index cdbafd63054b2..2eb84dfd327e1 100644 --- a/.github/workflows/manual-nixpkgs-v2.yml +++ b/.github/workflows/manual-nixpkgs-v2.yml @@ -9,8 +9,7 @@ on: - 'lib/**' - 'pkgs/tools/nix/nixdoc/**' -permissions: - contents: read +permissions: {} jobs: nixpkgs: @@ -19,12 +18,10 @@ 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 - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: - # explicitly enable sandbox extra_nix_config: sandbox = true - uses: cachix/cachix-action@ad2ddac53f961de1989924296a1f236fcfbaa4fc # v15 diff --git a/.github/workflows/nix-parse-v2.yml b/.github/workflows/nix-parse-v2.yml index 71d5a29af7769..2f8e97d3a8a0e 100644 --- a/.github/workflows/nix-parse-v2.yml +++ b/.github/workflows/nix-parse-v2.yml @@ -1,12 +1,9 @@ name: "Check whether nix files are parseable v2" on: - # avoids approving first time contributors pull_request_target: -permissions: - pull-requests: read - contents: read +permissions: {} jobs: get-merge-commit: @@ -32,12 +29,12 @@ jobs: - 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 }} if: ${{ env.CHANGED_FILES && env.CHANGED_FILES != '' }} - uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 with: + extra_nix_config: sandbox = true nix_path: nixpkgs=channel:nixpkgs-unstable - name: Parse all changed or added nix files diff --git a/.github/workflows/nixpkgs-vet.yml b/.github/workflows/nixpkgs-vet.yml index 70ab6da49a489..0b2f4e1c96d36 100644 --- a/.github/workflows/nixpkgs-vet.yml +++ b/.github/workflows/nixpkgs-vet.yml @@ -6,7 +6,6 @@ name: Vet nixpkgs on: - # Using pull_request_target instead of pull_request avoids having to approve first time contributors. pull_request_target: # This workflow depends on the base branch of the PR, but changing the base branch is not included in the default trigger events, which would be `opened`, `synchronize` or `reopened`. # Instead it causes an `edited` event, so we need to add it explicitly here. @@ -34,7 +33,6 @@ 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