Skip to content

Commit 29928f3

Browse files
tmihalactmihalac
committed
fix: Removed usage of pull_request_target as much as possible to prevent security concerns
Signed-off-by: Theodor Mihalache <tmihalac@redhat.com>
1 parent 290bb5c commit 29928f3

7 files changed

+44
-33
lines changed

‎.github/fork_workflows/fork_pr_integration_tests_aws.yml

+4-3
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

+4-3
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

+4-3
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

+20-12
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
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+
1012
jobs:
1113
lint-java:
12-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
14+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
1315
if:
1416
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
1517
(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 +20,17 @@ jobs:
1820
steps:
1921
- uses: actions/checkout@v4
2022
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
23+
# pull_request_target runs the workflow in the context of the base repo
24+
# as such actions/checkout needs to be explicit configured to retrieve
25+
# code from the PR.
26+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
2427
submodules: recursive
28+
persist-credentials: false
2529
- name: Lint java
2630
run: make lint-java
2731

2832
unit-test-java:
29-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
33+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
3034
if:
3135
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
3236
(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 +40,12 @@ jobs:
3640
steps:
3741
- uses: actions/checkout@v4
3842
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
43+
# pull_request_target runs the workflow in the context of the base repo
44+
# as such actions/checkout needs to be explicit configured to retrieve
45+
# code from the PR.
46+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
4247
submodules: recursive
48+
persist-credentials: false
4349
- name: Set up JDK 11
4450
uses: actions/setup-java@v1
4551
with:
@@ -66,7 +72,7 @@ jobs:
6672
path: ${{ github.workspace }}/docs/coverage/java/target/site/jacoco-aggregate/
6773

6874
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.
75+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
7076
if:
7177
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
7278
(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 +88,7 @@ jobs:
8288
- uses: actions/checkout@v4
8389
with:
8490
submodules: 'true'
91+
persist-credentials: false
8592
- name: Setup Python
8693
uses: actions/setup-python@v5
8794
id: setup-python
@@ -101,7 +108,7 @@ jobs:
101108
run: make build-${{ matrix.component }}-docker REGISTRY=${REGISTRY} VERSION=${GITHUB_SHA}
102109

103110
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.
111+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
105112
if:
106113
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
107114
(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 +120,12 @@ jobs:
113120
steps:
114121
- uses: actions/checkout@v4
115122
with:
116-
# pull_request runs the workflow in the context of the base repo
123+
# pull_request_target runs the workflow in the context of the base repo
117124
# as such actions/checkout needs to be explicit configured to retrieve
118125
# code from the PR.
119126
ref: refs/pull/${{ github.event.pull_request.number }}/merge
120127
submodules: recursive
128+
persist-credentials: false
121129
- name: Set up JDK 11
122130
uses: actions/setup-java@v1
123131
with:

‎.github/workflows/lint_pr.yml

+1-5
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

+10-5
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

+1-2
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)