From a55f7ddced785a7d8631d027b06af9574f6b181b Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 24 May 2025 14:05:26 +0200 Subject: [PATCH] workflows: checkout nixpkgs into trusted/untrusted directories By consistently checking out nixpkgs into the same location in every workflow, it's easier to reason about the different workflows at once. We also use crystal-clear names to make clear, which checkouts are considered trusted, because they only contain target-branch-code and which checkouts are untrusted, because they contain code from the head branch. By naming the checkout directories trusted/untrusted, it's obvious at the call-site. One example of where we likely did the wrong thing is the nixpkgs-vet workflow: Fetching the toolVersion from the untrusted checkout opens the door for an injection into the download URL, thus code could be downloaded from anywhere. This is not a problem, because this workflow does not run with elevated privileges, but it's a scary oversight nonetheless. (cherry picked from commit 6720d254294220cdfce18c3f981a8aabffb3de94) --- .github/workflows/check-cherry-picks.yml | 3 ++- .github/workflows/check-format.yml | 3 ++- .github/workflows/check-shell.yml | 3 ++- .github/workflows/codeowners-v2.yml | 15 +++++++++------ .github/workflows/eval-aliases.yml | 6 +++--- .github/workflows/eval.yml | 22 +++++++++++----------- .github/workflows/lib-tests.yml | 3 ++- .github/workflows/manual-nixos-v2.yml | 3 ++- .github/workflows/manual-nixpkgs-v2.yml | 3 ++- .github/workflows/nix-parse-v2.yml | 3 ++- .github/workflows/nixpkgs-vet.yml | 9 ++++----- 11 files changed, 41 insertions(+), 32 deletions(-) diff --git a/.github/workflows/check-cherry-picks.yml b/.github/workflows/check-cherry-picks.yml index 70dfdfba1e5f..e85fa59bb699 100644 --- a/.github/workflows/check-cherry-picks.yml +++ b/.github/workflows/check-cherry-picks.yml @@ -21,10 +21,11 @@ jobs: with: fetch-depth: 0 filter: blob:none + path: trusted - 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" + ./trusted/maintainers/scripts/check-cherry-picks.sh "$BASE_SHA" "$HEAD_SHA" diff --git a/.github/workflows/check-format.yml b/.github/workflows/check-format.yml index 58c76f97da6b..ec83ffcce07c 100644 --- a/.github/workflows/check-format.yml +++ b/.github/workflows/check-format.yml @@ -23,6 +23,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -33,7 +34,7 @@ jobs: # Note that it's fine to run this on untrusted code because: # - There's no secrets accessible here # - The build is sandboxed - if ! nix-build ci -A fmt.check; then + if ! nix-build untrusted/ci -A fmt.check; then echo "Some files are not properly formatted" echo "Please format them by going to the Nixpkgs root directory and running one of:" echo " nix-shell --run treefmt" diff --git a/.github/workflows/check-shell.yml b/.github/workflows/check-shell.yml index 01dd64913b6a..39fb722ae87a 100644 --- a/.github/workflows/check-shell.yml +++ b/.github/workflows/check-shell.yml @@ -40,8 +40,9 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 - name: Build shell - run: nix-build ci -A shell + run: nix-build untrusted/ci -A shell diff --git a/.github/workflows/codeowners-v2.yml b/.github/workflows/codeowners-v2.yml index e22ccfcea52d..085abced061c 100644 --- a/.github/workflows/codeowners-v2.yml +++ b/.github/workflows/codeowners-v2.yml @@ -63,10 +63,11 @@ jobs: # so it's important this is not the PRs code. - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - path: base + ref: ${{ steps.get-merge-commit.outputs.targetSha }} + path: trusted - name: Build codeowners validator - run: nix-build base/ci -A codeownersValidator + run: nix-build trusted/ci -A codeownersValidator - uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6 if: vars.OWNER_RO_APP_ID @@ -80,14 +81,14 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} - path: pr + path: untrusted - name: Validate codeowners if: steps.app-token.outputs.token env: - OWNERS_FILE: pr/${{ env.OWNERS_FILE }} + OWNERS_FILE: untrusted/${{ env.OWNERS_FILE }} GITHUB_ACCESS_TOKEN: ${{ steps.app-token.outputs.token }} - REPOSITORY_PATH: pr + REPOSITORY_PATH: untrusted OWNER_CHECKER_REPOSITORY: ${{ github.repository }} # Set this to "notowned,avoid-shadowing" to check that all files are owned by somebody EXPERIMENTAL_CHECKS: "avoid-shadowing" @@ -104,6 +105,8 @@ jobs: # Important: Because we use pull_request_target, this checks out the base branch of the PR, not the PR head. # This is intentional, because we need to request the review of owners as declared in the base branch. - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + path: trusted - uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6 if: vars.OWNER_APP_ID @@ -116,7 +119,7 @@ jobs: permission-pull-requests: write - name: Build review request package - run: nix-build ci -A requestReviews + run: nix-build trusted/ci -A requestReviews - name: Request reviews if: steps.app-token.outputs.token diff --git a/.github/workflows/eval-aliases.yml b/.github/workflows/eval-aliases.yml index 941ffd378ef7..dc5bad6572fb 100644 --- a/.github/workflows/eval-aliases.yml +++ b/.github/workflows/eval-aliases.yml @@ -24,7 +24,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} - path: nixpkgs + path: untrusted - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 @@ -32,8 +32,8 @@ jobs: extra_nix_config: sandbox = true - name: Ensure flake outputs on all systems still evaluate - run: nix flake check --all-systems --no-build ./nixpkgs + run: nix flake check --all-systems --no-build ./untrusted - name: Query nixpkgs with aliases enabled to check for basic syntax errors run: | - time nix-env -I ./nixpkgs -f ./nixpkgs -qa '*' --option restrict-eval true --option allow-import-from-derivation false >/dev/null + time nix-env -I ./untrusted -f ./untrusted -qa '*' --option restrict-eval true --option allow-import-from-derivation false >/dev/null diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 3acfbdf25e37..0063ad1a12e9 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -61,7 +61,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ needs.prepare.outputs.mergedSha }} - path: nixpkgs + path: untrusted - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 @@ -72,7 +72,7 @@ jobs: env: MATRIX_SYSTEM: ${{ matrix.system }} run: | - nix-build nixpkgs/ci -A eval.singleSystem \ + nix-build untrusted/ci -A eval.singleSystem \ --argstr evalSystem "$MATRIX_SYSTEM" \ --arg chunkSize 10000 # If it uses too much memory, slightly decrease chunkSize @@ -101,7 +101,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ needs.prepare.outputs.mergedSha }} - path: nixpkgs + path: untrusted - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 @@ -110,7 +110,7 @@ jobs: - name: Combine all output paths and eval stats run: | - nix-build nixpkgs/ci -A eval.combine \ + nix-build untrusted/ci -A eval.combine \ --arg resultsDir ./intermediate \ -o prResult @@ -167,13 +167,13 @@ jobs: env: AUTHOR_ID: ${{ github.event.pull_request.user.id }} run: | - git -C nixpkgs fetch --depth 1 origin ${{ needs.prepare.outputs.targetSha }} - git -C nixpkgs worktree add ../target ${{ needs.prepare.outputs.targetSha }} - git -C nixpkgs diff --name-only ${{ needs.prepare.outputs.targetSha }} \ + git -C untrusted fetch --depth 1 origin ${{ needs.prepare.outputs.targetSha }} + git -C untrusted worktree add ../trusted ${{ needs.prepare.outputs.targetSha }} + git -C untrusted diff --name-only ${{ needs.prepare.outputs.targetSha }} \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info - nix-build target/ci -A eval.compare \ + nix-build trusted/ci -A eval.compare \ --arg beforeResultDir ./targetResult \ --arg afterResultDir "$(realpath prResult)" \ --arg touchedFilesJson ./touched-files.json \ @@ -222,15 +222,15 @@ jobs: # Important: This workflow job runs with extra permissions, # so we need to make sure to not run untrusted code from PRs - - name: Check out Nixpkgs at the base commit + - name: Check out Nixpkgs at the target commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ needs.prepare.outputs.targetSha }} - path: base + path: trusted sparse-checkout: ci - name: Build the requestReviews derivation - run: nix-build base/ci -A requestReviews + run: nix-build trusted/ci -A requestReviews - name: Labelling pull request if: ${{ github.event_name == 'pull_request_target' && github.repository_owner == 'NixOS' }} diff --git a/.github/workflows/lib-tests.yml b/.github/workflows/lib-tests.yml index 345d59bb9cc4..5e3a7a5cba66 100644 --- a/.github/workflows/lib-tests.yml +++ b/.github/workflows/lib-tests.yml @@ -26,6 +26,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -33,4 +34,4 @@ jobs: - name: Building Nixpkgs lib-tests run: | - nix-build ci -A lib-tests + nix-build untrusted/ci -A lib-tests diff --git a/.github/workflows/manual-nixos-v2.yml b/.github/workflows/manual-nixos-v2.yml index 5d5acdd377e7..ce5313c8135f 100644 --- a/.github/workflows/manual-nixos-v2.yml +++ b/.github/workflows/manual-nixos-v2.yml @@ -42,6 +42,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -55,7 +56,7 @@ jobs: - name: Build NixOS manual id: build-manual - run: NIX_PATH=nixpkgs=$(pwd) nix-build --option restrict-eval true ci -A manual-nixos --argstr system ${{ matrix.system }} + run: NIX_PATH=nixpkgs=$(pwd)/untrusted nix-build --option restrict-eval true untrusted/ci -A manual-nixos --argstr system ${{ matrix.system }} - name: Upload NixOS manual uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 diff --git a/.github/workflows/manual-nixpkgs-v2.yml b/.github/workflows/manual-nixpkgs-v2.yml index b31a6b4949e1..5cb63a2c88bd 100644 --- a/.github/workflows/manual-nixpkgs-v2.yml +++ b/.github/workflows/manual-nixpkgs-v2.yml @@ -29,6 +29,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -41,4 +42,4 @@ jobs: authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}' - name: Building Nixpkgs manual - run: NIX_PATH=nixpkgs=$(pwd) nix-build --option restrict-eval true ci -A manual-nixpkgs -A manual-nixpkgs-tests + run: NIX_PATH=nixpkgs=$(pwd)/untrusted nix-build --option restrict-eval true untrusted/ci -A manual-nixpkgs -A manual-nixpkgs-tests diff --git a/.github/workflows/nix-parse-v2.yml b/.github/workflows/nix-parse-v2.yml index 6ae66e964474..ecbb9c843389 100644 --- a/.github/workflows/nix-parse-v2.yml +++ b/.github/workflows/nix-parse-v2.yml @@ -24,6 +24,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} + path: untrusted - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -33,4 +34,4 @@ jobs: - name: Parse all nix files run: | # Tests multiple versions at once, let's make sure all of them run, so keep-going. - nix-build ci -A parse --keep-going + nix-build untrusted/ci -A parse --keep-going diff --git a/.github/workflows/nixpkgs-vet.yml b/.github/workflows/nixpkgs-vet.yml index 761ecbf08f2f..aefed2b267ce 100644 --- a/.github/workflows/nixpkgs-vet.yml +++ b/.github/workflows/nixpkgs-vet.yml @@ -36,12 +36,11 @@ jobs: ref: ${{ steps.get-merge-commit.outputs.mergedSha }} # Fetches the merge commit and its parents fetch-depth: 2 + path: untrusted - name: Checking out target branch run: | - target=$(mktemp -d) - git worktree add "$target" "$(git rev-parse HEAD^1)" - echo "target=$target" >> "$GITHUB_ENV" + git -C untrusted worktree add ../trusted ${{ steps.get-merge-commit.outputs.targetSha }} - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 @@ -49,7 +48,7 @@ jobs: # Update the pinned version using ci/nixpkgs-vet/update-pinned-tool.sh run: | # The pinned version of the tooling to use. - toolVersion=$(