From 4ff00ce8fc6197e5a5d71f9a52815649c38f756e Mon Sep 17 00:00:00 2001 From: Eno Compton Date: Thu, 18 Jul 2024 14:15:12 -0600 Subject: [PATCH] chore: remove pull request target usages The pull request target is inherently fragile and prone to security vulnerabilities. The only reason we used it was to put our testing project name, our testing service account email (not key), and WIF provider pool ID into secrets. In fact, all three of those values aren't necessarily secrets and work just as well in environment variables. We will still need to vet a PR before clicking "approve and run," but that's a much smaller attack surface area than pull request target. --- .github/labels.yml | 4 -- .github/workflows/codeql.yml | 6 --- .github/workflows/govulncheck.yaml | 2 - .github/workflows/lint.yaml | 19 ---------- .github/workflows/sample-tests.yaml | 28 +------------- .github/workflows/scorecard.yml | 4 +- .github/workflows/tests.yaml | 59 +++++------------------------ 7 files changed, 13 insertions(+), 109 deletions(-) diff --git a/.github/labels.yml b/.github/labels.yml index c0aba7e6..b3c48e01 100644 --- a/.github/labels.yml +++ b/.github/labels.yml @@ -69,7 +69,3 @@ - name: 'autorelease: tagged' color: ededed description: Release please has completed a release for this. - -- name: 'tests: run' - color: 3ded97 - description: Label to trigger Github Action tests. diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 3bcfa238..77cc80ed 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -22,18 +22,12 @@ on: paths-ignore: - '**/*.md' - '**/*.txt' - pull_request_target: - types: [labeled] - paths-ignore: - - '**/*.md' - - '**/*.txt' # Declare default permissions as read only. permissions: read-all jobs: analyze: - if: "${{ github.event.action != 'labeled' || github.event.label.name == 'tests: run' }}" name: Analyze runs-on: ubuntu-latest permissions: diff --git a/.github/workflows/govulncheck.yaml b/.github/workflows/govulncheck.yaml index 5ba5b26a..afbeb1ef 100644 --- a/.github/workflows/govulncheck.yaml +++ b/.github/workflows/govulncheck.yaml @@ -22,8 +22,6 @@ on: branches: - 'main' pull_request: - pull_request_target: - types: [labeled] schedule: - cron: '0 2 * * *' diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 7bb933f7..bbcc1715 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -15,37 +15,18 @@ name: lint on: pull_request: - pull_request_target: - types: [labeled] # Declare default permissions as read only. permissions: read-all jobs: lint: - if: "${{ github.event.action != 'labeled' || github.event.label.name == 'tests: run' }}" name: run lint runs-on: ubuntu-latest permissions: issues: write pull-requests: write steps: - - name: Remove PR Label - if: "${{ github.event.action == 'labeled' && github.event.label.name == 'tests: run' }}" - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - try { - await github.rest.issues.removeLabel({ - name: 'tests: run', - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number - }); - } catch (e) { - console.log('Failed to remove label. Another job may have already removed it!'); - } - name: Setup Go uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 with: diff --git a/.github/workflows/sample-tests.yaml b/.github/workflows/sample-tests.yaml index a02fd9fe..5f7ed0a1 100644 --- a/.github/workflows/sample-tests.yaml +++ b/.github/workflows/sample-tests.yaml @@ -17,8 +17,6 @@ on: push: branches: - main - pull_request_target: - types: [labeled] schedule: - cron: '0 2 * * *' @@ -28,12 +26,6 @@ permissions: read-all jobs: # job to run change detection changes: - # run job on proper workflow event triggers (skip job for pull_request event from forks and only run pull_request_target for "tests: run" label) - if: | - (github.event.action != 'labeled' && - github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name) || - github.event.label.name == 'tests: run' && - github.event_name != 'schedule' runs-on: ubuntu-latest # Required permissions permissions: @@ -46,22 +38,6 @@ jobs: go: ${{ steps.filter.outputs.go }} python: ${{ steps.filter.outputs.python }} steps: - - name: Remove PR label - if: "${{ github.event.action == 'labeled' && github.event.label.name == 'tests: run' }}" - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - try { - await github.rest.issues.removeLabel({ - name: 'tests: run', - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number - }); - } catch (e) { - console.log('Failed to remove label. Another job may have already removed it!'); - } - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 id: filter @@ -92,8 +68,8 @@ jobs: id: 'auth' uses: google-github-actions/auth@71fee32a0bb7e97b4d33d548e7d957010649d8fa # v2.1.3 with: - workload_identity_provider: ${{ secrets.PROVIDER_NAME }} - service_account: ${{ secrets.SERVICE_ACCOUNT }} + workload_identity_provider: ${{ var.PROVIDER_NAME }} + service_account: ${{ var.SERVICE_ACCOUNT }} - name: Set up Cloud SDK uses: google-github-actions/setup-gcloud@98ddc00a17442e89a24bbf282954a3b65ce6d200 # v2.1.0 diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index c6d40575..e92000e1 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -44,7 +44,7 @@ jobs: with: results_file: results.sarif results_format: sarif - + - name: Filter SARIF to skip false positives # filter out DangerousWorkflow alerts as they do not account for safe use of labels to trigger actions env: @@ -53,7 +53,7 @@ jobs: SCORECARD_SKIPPED_RULE_IDS_JSON=$(echo $SCORECARD_SKIPPED_RULE_IDS | jq -cR 'split(",")') # Trim the SARIF file to remove false positive detections cat results.sarif | jq '.runs[].results |= map(select(.ruleId as $id | '$SCORECARD_SKIPPED_RULE_IDS_JSON' | all($id != .)))' > resultsFiltered.sarif - + # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 06438d1e..f628379f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -18,8 +18,6 @@ on: push: branches: - main - pull_request_target: - types: [labeled] schedule: - cron: '0 2 * * *' @@ -28,7 +26,6 @@ permissions: read-all jobs: compilation: - if: "${{ (github.event.action != 'labeled' && github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name) || github.event.label.name == 'tests: run' }}" name: FreeBSD and OpenBSD compilation check runs-on: ubuntu-latest steps: @@ -45,8 +42,6 @@ jobs: CGO_ENABLED=0 GOOS=freebsd go build CGO_ENABLED=0 GOOS=openbsd go build integration: - # run job on proper workflow event triggers (skip job for pull_request event from forks and only run pull_request_target for "tests: run" label) - if: "${{ (github.event.action != 'labeled' && github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name) || github.event.label.name == 'tests: run' }}" runs-on: [self-hosted, linux, x64] name: "integration tests (linux)" permissions: @@ -55,23 +50,6 @@ jobs: issues: write pull-requests: write steps: - - name: Remove PR label - if: "${{ github.event.action == 'labeled' && github.event.label.name == 'tests: run' }}" - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - try { - await github.rest.issues.removeLabel({ - name: 'tests: run', - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number - }); - } catch (e) { - console.log('Failed to remove label. Another job may have already removed it!'); - } - - name: Setup Go uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 with: @@ -87,8 +65,8 @@ jobs: name: Authenticate to Google Cloud uses: google-github-actions/auth@71fee32a0bb7e97b4d33d548e7d957010649d8fa # v2.1.3 with: - workload_identity_provider: ${{ secrets.PROVIDER_NAME }} - service_account: ${{ secrets.SERVICE_ACCOUNT }} + workload_identity_provider: ${{ var.PROVIDER_NAME }} + service_account: ${{ var.SERVICE_ACCOUNT }} - name: Set up Cloud SDK uses: google-github-actions/setup-gcloud@98ddc00a17442e89a24bbf282954a3b65ce6d200 # v2.1.0 @@ -98,11 +76,11 @@ jobs: uses: google-github-actions/get-secretmanager-secrets@dc4a1392bad0fd60aee00bb2097e30ef07a1caae # v2.1.3 with: secrets: |- - ALLOYDB_INSTANCE_URI:${{ secrets.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_INSTANCE_NAME - ALLOYDB_CLUSTER_PASS:${{ secrets.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_CLUSTER_PASS - IMPERSONATED_USER:${{ secrets.GOOGLE_CLOUD_PROJECT }}/IMPERSONATED_USER - ALLOYDB_IAM_USER:${{ secrets.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_PROXY_IAM_USER - ALLOYDB_PSC_INSTANCE_URI:${{ secrets.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_PSC_INSTANCE_URI + ALLOYDB_INSTANCE_URI:${{ var.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_INSTANCE_NAME + ALLOYDB_CLUSTER_PASS:${{ var.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_CLUSTER_PASS + IMPERSONATED_USER:${{ var.GOOGLE_CLOUD_PROJECT }}/IMPERSONATED_USER + ALLOYDB_IAM_USER:${{ var.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_PROXY_IAM_USER + ALLOYDB_PSC_INSTANCE_URI:${{ var.GOOGLE_CLOUD_PROJECT }}/ALLOYDB_PSC_INSTANCE_URI - name: Run tests env: @@ -133,8 +111,6 @@ jobs: ./flakybot --repo ${{github.repository}} --commit_hash ${{github.sha}} --build_url https://github.com/${{github.repository}}/actions/runs/${{github.run_id}} unit: - # run job on proper workflow event triggers (skip job for pull_request event from forks and only run pull_request_target for "tests: run" label) - if: "${{ (github.event.action != 'labeled' && github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name) || github.event.label.name == 'tests: run' }}" name: "unit tests" runs-on: ${{ matrix.os }} strategy: @@ -147,23 +123,6 @@ jobs: issues: write pull-requests: write steps: - - name: Remove PR label - if: "${{ github.event.action == 'labeled' && github.event.label.name == 'tests: run' }}" - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - try { - await github.rest.issues.removeLabel({ - name: 'tests: run', - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number - }); - } catch (e) { - console.log('Failed to remove label. Another job may have already removed it!'); - } - - name: Setup Go uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2 with: @@ -181,8 +140,8 @@ jobs: if: ${{ github.event_name == 'schedule' || github.event_name == 'push' }} uses: google-github-actions/auth@71fee32a0bb7e97b4d33d548e7d957010649d8fa # v2.1.3 with: - workload_identity_provider: ${{ secrets.PROVIDER_NAME }} - service_account: ${{ secrets.SERVICE_ACCOUNT }} + workload_identity_provider: ${{ var.PROVIDER_NAME }} + service_account: ${{ var.SERVICE_ACCOUNT }} - name: Run tests # specifying bash shell ensures a failure in a piped process isn't lost by using `set -eo pipefail`