From 5c0bd34ff9c4202706025c8445dc2384a73fc881 Mon Sep 17 00:00:00 2001 From: Patrick Kaeding Date: Fri, 14 Aug 2026 13:35:42 -0400 Subject: [PATCH] fix(ci): de-privilege fork unit tests to close PWN-request exposure Wiz issue f0dca03e (CRITICAL, "Publicly exposed CI workflow with access to secrets vulnerable to PWN request") flags four IaC findings in .github/workflows/test-fork-pr.yml: an untrusted fork checkout plus a local action invoked from that checkout, in each of the go-test and e2e-tests jobs. The go-test findings are avoidable. Unit tests need no repository secrets -- only the "Publish JUnit Tests" step does, and that step already carries its own same-repo `if:` guard. So fork unit tests do not need pull_request_target at all: - main.yml: drop the job-level same-repo gate on go-test so fork PRs get unit test coverage under the unprivileged `pull_request` trigger. The Datadog publish step stays gated, so DATADOG_API_KEY is never present in a job that runs fork code. - test-fork-pr.yml: delete the now-redundant privileged go-test job, and update check-success's `needs:`. This removes two of Wiz's four findings and takes DATADOG_API_KEY out of every privileged fork context entirely. The e2e job is irreducibly privileged and stays put: its whole purpose is to run the fork's own action code against the LaunchDarkly API, so it needs both the untrusted checkout and LD_ACCESS_TOKEN_WRITER. It keeps the existing safe-to-test label gate. As defence in depth, all checkouts that handle fork-authored code now set `persist-credentials: false` so poisoned code cannot reuse the workflow token from .git/config. Note that `go-test` is a required status check. It is still produced for fork PRs -- by main.yml now rather than test-fork-pr.yml -- and no longer requires a maintainer label to report. Fork runs remain gated by the repository's "require approval for all external contributors" Actions setting. Verified with actionlint: no new findings (SC2209 count drops from 2 to 1 because the duplicated test block is gone). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/main.yml | 13 +++-- .github/workflows/test-fork-pr.yml | 79 ++++-------------------------- 2 files changed, 19 insertions(+), 73 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9aa0cfac..3be62064 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,8 +1,10 @@ # Run test file with command: # act pull_request -e testdata/act/pull-request.json # -# Fork PRs only run jobs that do not need repository secrets here. See test-fork-pr.yml -# for the label-gated workflow that runs go-test and e2e tests on forks. +# Fork PRs only run jobs that do not need repository secrets here. go-test and +# generate-docs run for forks under this unprivileged trigger; the steps inside them that +# need secrets stay gated to same-repo PRs. See test-fork-pr.yml for the label-gated +# workflow that runs the e2e tests on forks. name: Test and Generate Docs @@ -10,11 +12,13 @@ on: pull_request jobs: go-test: - if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest steps: - name: Checkout uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 + with: + # This job runs fork-authored code. Don't leave a usable token in .git/config. + persist-credentials: false - name: Set up Node.js uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 @@ -235,6 +239,9 @@ jobs: steps: - name: Checkout uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 + with: + # This job runs fork-authored code. Don't leave a usable token in .git/config. + persist-credentials: false - name: Set up Python uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 diff --git a/.github/workflows/test-fork-pr.yml b/.github/workflows/test-fork-pr.yml index 9ab24a26..2048bd41 100644 --- a/.github/workflows/test-fork-pr.yml +++ b/.github/workflows/test-fork-pr.yml @@ -1,4 +1,4 @@ -# Runs go-test and e2e tests on fork PRs after a maintainer adds the "safe-to-test" label. +# Runs the e2e tests on fork PRs after a maintainer adds the "safe-to-test" label. # This uses pull_request_target so repository secrets are available (fork PRs cannot # access secrets via the regular pull_request trigger). The label gate ensures untrusted # code is reviewed before it runs with access to secrets. @@ -6,6 +6,11 @@ # Security: Only the "labeled" event triggers this workflow. New pushes to the fork do NOT # re-trigger tests — see remove-safe-to-test-label.yml which strips the label on new commits, # forcing a maintainer to re-review and re-label. +# +# Scope: only the e2e job lives here. It is irreducibly privileged — it exists to run the +# fork's own action code against the LaunchDarkly API, so it needs both the untrusted +# checkout and LD_ACCESS_TOKEN_WRITER. Unit tests need no secrets and therefore run under +# the unprivileged `pull_request` trigger in main.yml instead of here. name: "Test (Fork PRs)" on: @@ -16,74 +21,6 @@ permissions: contents: read jobs: - go-test: - if: >- - github.event.label.name == 'safe-to-test' - && github.event.pull_request.head.repo.full_name != github.repository - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 - with: - ref: ${{ github.event.pull_request.head.sha }} - - - name: Set up Node.js - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 - with: - node-version: '18.x' - - - name: Install datadog-ci - run: npm install -g @datadog/datadog-ci - - - name: Set up Python - uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 - with: - python-version: '3.x' - - - name: Set up Go - uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 - with: - go-version-file: go.mod - cache: false - - - name: Install pre-commit - run: | - python -m pip install --upgrade pip - pip install pre-commit - - - name: Run pre-commit hooks - run: pre-commit run --all-files - - - name: Install dependencies - run: go mod tidy - - - name: Install gotestsum - run: go install gotest.tools/gotestsum@latest - - - name: Run tests with gotestsum - run: | - mkdir -p ${{ github.workspace }}/artifacts - mkdir -p ${{ github.workspace }}/reports - - GONFALON_MODE=test \ - gotestsum --packages="./..." \ - --junitfile ${{ github.workspace }}/reports/go-test_go_tests.xml \ - --jsonfile ${{ github.workspace }}/artifacts/go-test_go_tests.json \ - --rerun-fails=2 \ - --rerun-fails-max-failures=10 \ - --rerun-fails-report ${{ github.workspace }}/artifacts/rerun_tests_go_tests.txt \ - -- -tags=launchdarkly_easyjson -p=1 - - - name: Publish JUnit Tests - uses: ./.github/actions/publish-junit - env: - DD_API_KEY: ${{ secrets.DATADOG_API_KEY }} - with: - files: ${{ github.workspace }}/reports/go-test_go_tests.xml - name: find-code-references-in-pull-request - datadog: 'true' - github: 'true' - e2e-tests: if: >- github.event.label.name == 'safe-to-test' @@ -97,6 +34,9 @@ jobs: uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 with: ref: ${{ github.event.pull_request.head.sha }} + # Untrusted fork code runs in this job. Keep the workflow token out of + # .git/config so poisoned code cannot reuse it. + persist-credentials: false - name: Find LaunchDarkly feature flags in diff uses: ./ # Uses an action in the root directory id: find-flags @@ -140,7 +80,6 @@ jobs: check-success: name: "Check Success (Fork)" needs: - - go-test - e2e-tests if: always() runs-on: ubuntu-latest