Skip to content

Commit 1d726a3

Browse files
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 1d726a3

7 files changed

+48
-36
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

+24-15
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

1010
jobs:
1111
lint-java:
12-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
12+
permissions: read-all
13+
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,18 @@ 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
2528
- name: Lint java
2629
run: make lint-java
2730

2831
unit-test-java:
29-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
32+
permissions: read-all
33+
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,9 +41,10 @@ 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
4349
- name: Set up JDK 11
4450
uses: actions/setup-java@v1
@@ -66,7 +72,9 @@ 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+
permissions: read-all
76+
77+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
7078
if:
7179
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
7280
(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')))) &&
@@ -101,7 +109,9 @@ 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+
permissions: read-all
113+
114+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
105115
if:
106116
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
107117
(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,10 +123,9 @@ jobs:
113123
steps:
114124
- uses: actions/checkout@v4
115125
with:
116-
# pull_request runs the workflow in the context of the base repo
117-
# as such actions/checkout needs to be explicit configured to retrieve
118-
# code from the PR.
119-
ref: refs/pull/${{ github.event.pull_request.number }}/merge
126+
repository: ${{ github.event.repository.full_name }} # Uses the full repository name
127+
ref: ${{ github.ref }} # Uses the ref from the event
128+
token: ${{ secrets.GITHUB_TOKEN }} # Automatically provided token
120129
submodules: recursive
121130
- name: Set up JDK 11
122131
uses: actions/setup-java@v1

.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

+8-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
@@ -14,7 +14,9 @@ on:
1414

1515
jobs:
1616
integration-test-python:
17-
# when using pull_request, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
17+
permissions: read-all
18+
19+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
1820
if:
1921
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
2022
(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,9 +43,10 @@ jobs:
4143
steps:
4244
- uses: actions/checkout@v4
4345
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
46+
# pull_request_target runs the workflow in the context of the base repo
47+
# as such actions/checkout needs to be explicit configured to retrieve
48+
# code from the PR.
49+
ref: refs/pull/${{ github.event.pull_request.number }}/merge
4750
submodules: recursive
4851
- name: Setup Python
4952
uses: actions/setup-python@v5

.github/workflows/pr_local_integration_tests.yml

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ 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.
13+
# when using pull_request_target, all jobs MUST have this if check for 'ok-to-test' or 'approved' for security purposes.
14+
#TODO not sure this check is needed anymore, changed to on pull_request
1415
if:
1516
((github.event.action == 'labeled' && (github.event.label.name == 'approved' || github.event.label.name == 'lgtm' || github.event.label.name == 'ok-to-test')) ||
1617
(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'
18+
github.event.pull_request.base.repo.full_name == 'feast-dev/feast'
1819
runs-on: ${{ matrix.os }}
1920
strategy:
2021
fail-fast: false

0 commit comments

Comments
 (0)