fix: Removed usage of pull_request_target as much as possible to prev…

…ent security concerns (feast-dev#4549) * Test workflow changes: on from pull_request_target to pull_request Signed-off-by: Theodor Mihalache <[email protected]> * fix: Removed usage of pull_request_target as much as possible to prevent security concerns Signed-off-by: Theodor Mihalache <[email protected]> --------- Signed-off-by: Theodor Mihalache <[email protected]>
lokeshrangineni · Sep 25, 2024 · 3198371 · 3198371
1 parent 354c059
commit 3198371
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 13 deletions.
diff --git a/.github/workflows/java_pr.yml b/.github/workflows/java_pr.yml
@@ -7,6 +7,9 @@ on:
       - synchronize
       - labeled
 
+permissions:
+  pull-requests: read
+
 jobs:
   lint-java:
     # when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
@@ -23,6 +26,7 @@ jobs:
           # code from the PR.
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           submodules: recursive
+          persist-credentials: false
       - name: Lint java
         run: make lint-java
 
@@ -42,6 +46,7 @@ jobs:
           # code from the PR.
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           submodules: recursive
+          persist-credentials: false
       - name: Set up JDK 11
         uses: actions/setup-java@v1
         with:
@@ -84,6 +89,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: 'true'
+          persist-credentials: false
       - name: Setup Python
         uses: actions/setup-python@v5
         id: setup-python
@@ -120,6 +126,7 @@ jobs:
           # code from the PR.
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           submodules: recursive
+          persist-credentials: false
       - name: Set up JDK 11
         uses: actions/setup-java@v1
         with:

diff --git a/.github/workflows/lint_pr.yml b/.github/workflows/lint_pr.yml
@@ -1,20 +1,16 @@
 name: lint-pr
 
 on:
-  pull_request_target:
+  pull_request:
     types:
       - opened
       - edited
       - synchronize
 
-permissions:
-  # read-only perms specified due to use of pull_request_target in lieu of security label check
-  pull-requests: read
-
 jobs:
   validate-title:
     if:
-      github.repository == 'feast-dev/feast'
+      github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
     name: Validate PR title
     runs-on: ubuntu-latest
     steps:

diff --git a/.github/workflows/pr_integration_tests.yml b/.github/workflows/pr_integration_tests.yml
@@ -11,6 +11,9 @@ on:
 #concurrency:
 #  group: pr-integration-tests-${{ github.event.pull_request.number }}
 #  cancel-in-progress: true
+permissions:
+  actions: write
+  pull-requests: read
 
 jobs:
   integration-test-python:
@@ -46,6 +49,7 @@ jobs:
           # code from the PR.
           ref: refs/pull/${{ github.event.pull_request.number }}/merge
           submodules: recursive
+          persist-credentials: false
       - name: Setup Python
         uses: actions/setup-python@v5
         id: setup-python

diff --git a/.github/workflows/pr_local_integration_tests.yml b/.github/workflows/pr_local_integration_tests.yml
@@ -2,19 +2,18 @@ name: pr-local-integration-tests
 # This runs local tests with containerized stubs of online stores. This is the main dev workflow
 
 on:
-  pull_request_target:
+  pull_request:
     types:
       - opened
       - synchronize
       - labeled
 
 jobs:
   integration-test-python-local:
-    # when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
     if:
       ((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
       (github.event.action != 'labeled' && (contains(github.event.pull_request.labels.*.name, 'ok-to-test') || contains(github.event.pull_request.labels.*.name, 'approved') || contains(github.event.pull_request.labels.*.name, 'lgtm')))) &&
-      github.repository == 'feast-dev/feast'
+      github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: false
@@ -27,10 +26,9 @@ jobs:
     steps:
       - uses: actions/checkout@v4
         with:
-          # pull_request_target runs the workflow in the context of the base repo
-          # as such actions/checkout needs to be explicit configured to retrieve
-          # code from the PR.
-          ref: refs/pull/${{ github.event.pull_request.number }}/merge
+          repository: ${{ github.event.repository.full_name }}  # Uses the full repository name
+          ref: ${{ github.ref }}                                  # Uses the ref from the event
+          token: ${{ secrets.GITHUB_TOKEN }}                     # Automatically provided token
           submodules: recursive
       - name: Setup Python
         uses: actions/setup-python@v5