From 65bb095948c39864dae69a71ddd30e2150c75778 Mon Sep 17 00:00:00 2001 From: Winter Date: Sat, 5 Apr 2025 20:22:19 -0400 Subject: [PATCH 1/2] ci: fix issues found by zizmor Co-authored-by: Thomas Gerbet --- .github/workflows/backport.yml | 1 + .github/workflows/build.yml | 1 + .github/workflows/check.yml | 2 ++ .github/workflows/eval.yml | 8 +++++++- .github/workflows/labels.yml | 1 + .github/workflows/lint.yml | 3 +++ .github/workflows/merge-group.yml | 1 + .github/workflows/periodic-merge-24h.yml | 3 ++- .github/workflows/periodic-merge-6h.yml | 3 ++- .github/workflows/periodic-merge.yml | 5 +++++ .github/workflows/pr.yml | 1 + .github/workflows/reviewers.yml | 4 +++- .github/workflows/test.yml | 1 + 13 files changed, 30 insertions(+), 4 deletions(-) diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml index 011e09a0023e..17a103c4ca7d 100644 --- a/.github/workflows/backport.yml +++ b/.github/workflows/backport.yml @@ -40,6 +40,7 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha }} token: ${{ steps.app-token.outputs.token }} + persist-credentials: false - name: Log current API rate limits env: diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 918c82c94bbe..26d71b846369 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -46,6 +46,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout the merge commit uses: ./.github/actions/checkout diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 0ee1146a4982..f55ff86dfdcf 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -34,6 +34,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false path: trusted sparse-checkout: | ci/github-script @@ -73,6 +74,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout merge and target commits uses: ./.github/actions/checkout diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index dee31e1f5328..97d5c114acb0 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -34,6 +34,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false path: trusted sparse-checkout: | ci/supportedVersions.nix @@ -41,6 +42,7 @@ jobs: - name: Check out the PR at the test merge commit uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false ref: ${{ inputs.mergedSha }} path: untrusted sparse-checkout: | @@ -84,6 +86,7 @@ jobs: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Check out the PR at merged and target commits uses: ./.github/actions/checkout @@ -155,6 +158,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Check out the PR at the target commit uses: ./.github/actions/checkout @@ -181,8 +185,9 @@ jobs: - name: Compare against the target branch env: AUTHOR_ID: ${{ github.event.pull_request.user.id }} + TARGET_SHA: ${{ inputs.mergedSha }} run: | - git -C nixpkgs/trusted diff --name-only ${{ inputs.mergedSha }} \ + git -C nixpkgs/trusted diff --name-only "$TARGET_SHA" \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info @@ -318,6 +323,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout the merge commit uses: ./.github/actions/checkout diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 3e520139bc28..74838ebb52ae 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -46,6 +46,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: | ci/github-script diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cb7a61edad83..358e090a7b2b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -26,6 +26,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout the merge commit uses: ./.github/actions/checkout @@ -60,6 +61,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout the merge commit uses: ./.github/actions/checkout @@ -87,6 +89,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: .github/actions - name: Checkout merge and target commits uses: ./.github/actions/checkout diff --git a/.github/workflows/merge-group.yml b/.github/workflows/merge-group.yml index 6ae96f0900f7..24ab1095ea7d 100644 --- a/.github/workflows/merge-group.yml +++ b/.github/workflows/merge-group.yml @@ -24,6 +24,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout: | ci/supportedSystems.json diff --git a/.github/workflows/periodic-merge-24h.yml b/.github/workflows/periodic-merge-24h.yml index 63e3dfd13887..481a54191349 100644 --- a/.github/workflows/periodic-merge-24h.yml +++ b/.github/workflows/periodic-merge-24h.yml @@ -43,4 +43,5 @@ jobs: from: ${{ matrix.pairs.from }} into: ${{ matrix.pairs.into }} name: ${{ matrix.pairs.name || format('{0} → {1}', matrix.pairs.from, matrix.pairs.into) }} - secrets: inherit + secrets: + NIXPKGS_CI_APP_PRIVATE_KEY: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/periodic-merge-6h.yml b/.github/workflows/periodic-merge-6h.yml index 054ac713fe74..edd4cb0b2df6 100644 --- a/.github/workflows/periodic-merge-6h.yml +++ b/.github/workflows/periodic-merge-6h.yml @@ -42,4 +42,5 @@ jobs: from: ${{ matrix.pairs.from }} into: ${{ matrix.pairs.into }} name: ${{ format('{0} → {1}', matrix.pairs.from, matrix.pairs.into) }} - secrets: inherit + secrets: + NIXPKGS_CI_APP_PRIVATE_KEY: ${{ secrets.NIXPKGS_CI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/periodic-merge.yml b/.github/workflows/periodic-merge.yml index 04c80ae9735c..606c03866e6f 100644 --- a/.github/workflows/periodic-merge.yml +++ b/.github/workflows/periodic-merge.yml @@ -11,6 +11,9 @@ on: description: Target branch to merge into. required: true type: string + secrets: + NIXPKGS_CI_APP_PRIVATE_KEY: + required: true defaults: run: @@ -32,6 +35,8 @@ jobs: permission-pull-requests: write - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + with: + persist-credentials: false - name: Find merge base between two branches if: contains(inputs.from, ' ') diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index d443d68cdad9..1ba548a4c44e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -34,6 +34,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout-cone-mode: true # default, for clarity sparse-checkout: | ci/github-script diff --git a/.github/workflows/reviewers.yml b/.github/workflows/reviewers.yml index 245970531477..22ac1bb5bcd4 100644 --- a/.github/workflows/reviewers.yml +++ b/.github/workflows/reviewers.yml @@ -29,6 +29,7 @@ jobs: - name: Check out the PR at the base commit uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false path: trusted sparse-checkout: ci @@ -146,6 +147,7 @@ jobs: if: ${{ steps.app-token.outputs.token }} env: GH_TOKEN: ${{ github.token }} + APP_GH_TOKEN: ${{ steps.app-token.outputs.token }} REPOSITORY: ${{ github.repository }} NUMBER: ${{ github.event.number }} AUTHOR: ${{ github.event.pull_request.user.login }} @@ -156,7 +158,7 @@ jobs: # There appears to be no API to request reviews based on GitHub IDs jq -r 'keys[]' comparison/maintainers.json \ | while read -r id; do gh api /user/"$id" --jq .login; done \ - | GH_TOKEN=${{ steps.app-token.outputs.token }} result/bin/request-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR" + | GH_TOKEN="$APP_GH_TOKEN" result/bin/request-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR" - name: Log current API rate limits (app-token) if: ${{ steps.app-token.outputs.token }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4431e89954f3..67b416a6dfb2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -21,6 +21,7 @@ jobs: steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: + persist-credentials: false sparse-checkout-cone-mode: true # default, for clarity sparse-checkout: | ci/github-script From 1a9867167dd7c954a01ef78b03cf8ca7a7d0a30d Mon Sep 17 00:00:00 2001 From: Winter Date: Sat, 5 Apr 2025 20:22:25 -0400 Subject: [PATCH 2/2] ci: add zizmor check and configuration `zizmor` is a tool that uses static analysis to find potential security issues in GitHub Actions [0]. (Yes, it's a bit absurd that GitHub made a CI system so complicated that tools like this were created, but I digress.) Given our increase in GHA usage recently, I think this is a good step towards keeping our security posture in tip-top shape. (It also keeps with the theme of automating as many things as possible!) The rule related to the usages of dangerous-triggers have been disabled to avoid false-positives. Explanations about the usage of `pull_request_target` and expectations around its usage can be found in `.github/workflows/README.md`. [0]: https://woodruffw.github.io/zizmor/ Co-authored-by: Thomas Gerbet --- .github/zizmor.yml | 12 ++++++++++++ ci/default.nix | 2 ++ 2 files changed, 14 insertions(+) create mode 100644 .github/zizmor.yml diff --git a/.github/zizmor.yml b/.github/zizmor.yml new file mode 100644 index 000000000000..f1b71580ebca --- /dev/null +++ b/.github/zizmor.yml @@ -0,0 +1,12 @@ +# This file defines the ignore rules for zizmor. +# +# For rules that contain a high number of false positives, prefer listing them here +# instead of adding ignore comments. Note that zizmor cannot ignore by line-within-a-string, so +# there are some ignore items that encompass multiple problems within one `run` block. An issue +# tracking this is at https://github.com/woodruffw/zizmor/issues/648. +# +# For more info, see the documentation: https://woodruffw.github.io/zizmor/usage/#ignoring-results + +rules: + dangerous-triggers: + disable: true diff --git a/ci/default.nix b/ci/default.nix index 7b7424f4f5be..972c679a1492 100644 --- a/ci/default.nix +++ b/ci/default.nix @@ -136,6 +136,8 @@ let [ "--config=${config}" ]; includes = [ "*.md" ]; }; + + programs.zizmor.enable = true; }; fs = pkgs.lib.fileset; nixFilesSrc = fs.toSource {