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.
GoogleCloudPlatform · Jul 18, 2024 · 5e8c50d · 5e8c50d
1 parent 57d3e80
commit 5e8c50d
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 109 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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`