Skip to content

Commit ced4c78

Browse files
tmihalactmihalac
committed
fix: Removed usage of pull_request_target as much as possible to prevent security concerns
Signed-off-by: Theodor Mihalache <[email protected]>
1 parent 290bb5c commit ced4c78

File tree

7 files changed

+45
-33
lines changed

7 files changed

+45
-33
lines changed

‎.github/fork_workflows/fork_pr_integration_tests_aws.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ jobs:
2727
steps:
2828
- uses: actions/checkout@v4
2929
with:
30-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
31-
ref: ${{ github.ref }} # Uses the ref from the event
32-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
30+
# pull_request_target runs the workflow in the context of the base repo
31+
# as such actions/checkout needs to be explicit configured to retrieve
32+
# code from the PR.
33+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
3334
submodules: recursive
3435
- name: Setup Python
3536
uses: actions/setup-python@v5

‎.github/fork_workflows/fork_pr_integration_tests_gcp.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ jobs:
2727
steps:
2828
- uses: actions/checkout@v4
2929
with:
30-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
31-
ref: ${{ github.ref }} # Uses the ref from the event
32-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
30+
# pull_request_target runs the workflow in the context of the base repo
31+
# as such actions/checkout needs to be explicit configured to retrieve
32+
# code from the PR.
33+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
3334
submodules: recursive
3435
- name: Setup Python
3536
uses: actions/setup-python@v5

‎.github/fork_workflows/fork_pr_integration_tests_snowflake.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ jobs:
2727
steps:
2828
- uses: actions/checkout@v4
2929
with:
30-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
31-
ref: ${{ github.ref }} # Uses the ref from the event
32-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
30+
# pull_request_target runs the workflow in the context of the base repo
31+
# as such actions/checkout needs to be explicit configured to retrieve
32+
# code from the PR.
33+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
3334
submodules: recursive
3435
- name: Setup Python
3536
uses: actions/setup-python@v5

‎.github/workflows/java_pr.yml‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
name: java_pr
22

33
on:
4-
pull_request:
4+
pull_request_target:
55
types:
66
- opened
77
- synchronize
88
- labeled
99

10+
permissions:
11+
pull-requests: read
12+
1013
jobs:
1114
lint-java:
12-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
15+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
1316
if:
1417
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
1518
(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')))) &&
@@ -18,15 +21,17 @@ jobs:
1821
steps:
1922
- uses: actions/checkout@v4
2023
with:
21-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
22-
ref: ${{ github.ref }} # Uses the ref from the event
23-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
24+
# pull_request_target runs the workflow in the context of the base repo
25+
# as such actions/checkout needs to be explicit configured to retrieve
26+
# code from the PR.
27+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
2428
submodules: recursive
29+
persist-credentials: false
2530
- name: Lint java
2631
run: make lint-java
2732

2833
unit-test-java:
29-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
34+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
3035
if:
3136
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
3237
(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')))) &&
@@ -36,10 +41,12 @@ jobs:
3641
steps:
3742
- uses: actions/checkout@v4
3843
with:
39-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
40-
ref: ${{ github.ref }} # Uses the ref from the event
41-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
44+
# pull_request_target runs the workflow in the context of the base repo
45+
# as such actions/checkout needs to be explicit configured to retrieve
46+
# code from the PR.
47+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
4248
submodules: recursive
49+
persist-credentials: false
4350
- name: Set up JDK 11
4451
uses: actions/setup-java@v1
4552
with:
@@ -66,7 +73,7 @@ jobs:
6673
path: ${{ github.workspace }}/docs/coverage/java/target/site/jacoco-aggregate/
6774

6875
build-docker-image-java:
69-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
76+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
7077
if:
7178
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
7279
(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')))) &&
@@ -82,6 +89,7 @@ jobs:
8289
- uses: actions/checkout@v4
8390
with:
8491
submodules: 'true'
92+
persist-credentials: false
8593
- name: Setup Python
8694
uses: actions/setup-python@v5
8795
id: setup-python
@@ -101,7 +109,7 @@ jobs:
101109
run: make build-${{ matrix.component }}-docker REGISTRY=${REGISTRY} VERSION=${GITHUB_SHA}
102110

103111
integration-test-java-pr:
104-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
112+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
105113
if:
106114
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
107115
(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')))) &&
@@ -113,11 +121,12 @@ jobs:
113121
steps:
114122
- uses: actions/checkout@v4
115123
with:
116-
# pull_request runs the workflow in the context of the base repo
124+
# pull_request_target runs the workflow in the context of the base repo
117125
# as such actions/checkout needs to be explicit configured to retrieve
118126
# code from the PR.
119127
ref: refs/pull/${{ github.event.pull_request.number }}/merge
120128
submodules: recursive
129+
persist-credentials: false
121130
- name: Set up JDK 11
122131
uses: actions/setup-java@v1
123132
with:

‎.github/workflows/lint_pr.yml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,10 @@ on:
77
- edited
88
- synchronize
99

10-
permissions:
11-
# read-only perms specified due to use of pull_request in lieu of security label check
12-
pull-requests: read
13-
1410
jobs:
1511
validate-title:
1612
if:
17-
github.repository == 'feast-dev/feast'
13+
github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
1814
name: Validate PR title
1915
runs-on: ubuntu-latest
2016
steps:

‎.github/workflows/pr_integration_tests.yml‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: pr-integration-tests
22

33
on:
4-
pull_request:
4+
pull_request_target:
55
types:
66
- opened
77
- synchronize
@@ -11,10 +11,13 @@ on:
1111
#concurrency:
1212
# group: pr-integration-tests-${{ github.event.pull_request.number }}
1313
# cancel-in-progress: true
14+
permissions:
15+
actions: write
16+
pull-requests: read
1417

1518
jobs:
1619
integration-test-python:
17-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
20+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
1821
if:
1922
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
2023
(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')))) &&
@@ -41,10 +44,12 @@ jobs:
4144
steps:
4245
- uses: actions/checkout@v4
4346
with:
44-
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
45-
ref: ${{ github.ref }} # Uses the ref from the event
46-
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
47+
# pull_request_target runs the workflow in the context of the base repo
48+
# as such actions/checkout needs to be explicit configured to retrieve
49+
# code from the PR.
50+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
4751
submodules: recursive
52+
persist-credentials: false
4853
- name: Setup Python
4954
uses: actions/setup-python@v5
5055
id: setup-python

‎.github/workflows/pr_local_integration_tests.yml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@ on:
1010

1111
jobs:
1212
integration-test-python-local:
13-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
1413
if:
1514
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
1615
(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')))) &&
17-
github.repository == 'feast-dev/feast'
16+
github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
1817
runs-on: ${{ matrix.os }}
1918
strategy:
2019
fail-fast: false

0 commit comments

Comments
 (0)