Skip to content

Commit 24273bf

Browse files
Fix pwn request vulnerability in PR quality checks workflow
The pull_request_target workflow checked out and executed Go scripts from the PR head, allowing attackers to inject arbitrary code via init() functions with access to a write-scoped GITHUB_TOKEN. This was confirmed exploited in the wild (ref: StepSecurity blog). Checkout now targets the base branch so only trusted scripts execute. PR head SHA is fetched as data-only for diffing via a new PR_HEAD_SHA env var. Write operations (comments, labels) are isolated in a separate report job that never checks out code. All job permissions follow least privilege — quality runs read-only, report holds the write token. fixed: #6083 Signed-off-by: Avelino <31996+avelino@users.noreply.github.com> Co-Authored-By: Thierry Abalea <thierry.abalea@shipfox.io>
1 parent 470fa15 commit 24273bf

3 files changed

Lines changed: 62 additions & 17 deletions

File tree

.github/scripts/check-pr-diff/main.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,15 @@ func getDiff() string {
207207
if base == "" {
208208
base = "main"
209209
}
210-
out, err := exec.Command("git", "diff", "origin/"+base+"...HEAD", "--", "README.md").Output()
210+
head := os.Getenv("PR_HEAD_SHA")
211+
if head == "" {
212+
head = "HEAD"
213+
}
214+
out, err := exec.Command("git", "diff", "origin/"+base+"..."+head, "--", "README.md").Output()
211215
if err == nil && len(out) > 0 {
212216
return string(out)
213217
}
214-
out, err = exec.Command("git", "diff", "HEAD~1", "--", "README.md").Output()
218+
out, err = exec.Command("git", "diff", head+"~1", "--", "README.md").Output()
215219
if err == nil {
216220
return string(out)
217221
}
@@ -223,11 +227,15 @@ func getChangedFiles() []string {
223227
if base == "" {
224228
base = "main"
225229
}
226-
out, err := exec.Command("git", "diff", "--name-only", "origin/"+base+"...HEAD").Output()
230+
head := os.Getenv("PR_HEAD_SHA")
231+
if head == "" {
232+
head = "HEAD"
233+
}
234+
out, err := exec.Command("git", "diff", "--name-only", "origin/"+base+"..."+head).Output()
227235
if err == nil && len(out) > 0 {
228236
return splitLines(string(out))
229237
}
230-
out, err = exec.Command("git", "diff", "--name-only", "HEAD~1").Output()
238+
out, err = exec.Command("git", "diff", "--name-only", head+"~1").Output()
231239
if err == nil {
232240
return splitLines(string(out))
233241
}
@@ -291,7 +299,14 @@ func extractRepoName(rawURL string) string {
291299
// --- README parsing ---
292300

293301
func getCategoryItemCount(readmePath, entryURL string) (category string, count int) {
294-
data, err := os.ReadFile(readmePath)
302+
var data []byte
303+
var err error
304+
head := os.Getenv("PR_HEAD_SHA")
305+
if head != "" {
306+
data, err = exec.Command("git", "show", head+":README.md").Output()
307+
} else {
308+
data, err = os.ReadFile(readmePath)
309+
}
295310
if err != nil {
296311
return "unknown", -1
297312
}

.github/workflows/pr-quality-check.yaml

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ on:
55
types: [opened, edited, synchronize, reopened]
66

77
permissions:
8-
pull-requests: write
9-
contents: write
8+
contents: read
9+
pull-requests: read
1010

1111
jobs:
1212
detect:
@@ -36,16 +36,30 @@ jobs:
3636
runs-on: ubuntu-latest
3737
environment: action
3838
container: golang:latest
39+
permissions:
40+
contents: read
41+
pull-requests: read
42+
outputs:
43+
comment: ${{ steps.quality.outputs.comment }}
44+
labels: ${{ steps.quality.outputs.labels }}
45+
fail: ${{ steps.quality.outputs.fail }}
46+
diff_comment: ${{ steps.diff.outputs.diff_comment }}
47+
diff_fail: ${{ steps.diff.outputs.diff_fail }}
3948
steps:
4049
- uses: actions/checkout@v6
4150
with:
42-
ref: ${{ github.event.pull_request.head.sha }}
51+
ref: ${{ github.event.pull_request.base.sha }}
52+
persist-credentials: false
4353
fetch-depth: 0
4454

45-
- name: Fetch base branch
55+
- name: Fetch base branch and PR head
56+
env:
57+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
4658
run: |
4759
git config --global --add safe.directory "$GITHUB_WORKSPACE"
48-
git fetch origin ${{ github.base_ref }}
60+
AUTH="$(printf '%s' "x-access-token:${GITHUB_TOKEN}" | base64 -w0)"
61+
git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${AUTH}" fetch origin "${{ github.base_ref }}"
62+
git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${AUTH}" fetch origin "+refs/pull/${{ github.event.pull_request.number }}/head"
4963
5064
- name: Run quality checks
5165
id: quality
@@ -59,28 +73,38 @@ jobs:
5973
continue-on-error: true
6074
env:
6175
GITHUB_BASE_REF: ${{ github.base_ref }}
76+
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
6277
run: go run ./.github/scripts/check-pr-diff/
6378

79+
report:
80+
name: Post quality report
81+
needs: [detect, quality]
82+
if: always() && needs.detect.outputs.is_package_pr == 'true' && needs.quality.result != 'cancelled'
83+
runs-on: ubuntu-latest
84+
permissions:
85+
pull-requests: write
86+
contents: write
87+
steps:
6488
- name: Post quality report comment
6589
uses: marocchino/sticky-pull-request-comment@v2
6690
with:
6791
header: pr-quality-check
6892
message: |
69-
${{ steps.quality.outputs.comment }}
93+
${{ needs.quality.outputs.comment }}
7094
7195
---
7296
73-
${{ steps.diff.outputs.diff_comment }}
97+
${{ needs.quality.outputs.diff_comment }}
7498
7599
- name: Sync labels
100+
if: needs.quality.outputs.labels != ''
76101
uses: actions-ecosystem/action-add-labels@v1
77-
if: ${{ steps.quality.outputs.labels != '' }}
78102
with:
79103
github_token: ${{ secrets.GITHUB_TOKEN }}
80-
labels: ${{ join(fromJson(steps.quality.outputs.labels), '\n') }}
104+
labels: ${{ join(fromJson(needs.quality.outputs.labels), '\n') }}
81105

82106
- name: Fail if critical checks failed
83-
if: ${{ steps.quality.outputs.fail == 'true' || steps.diff.outputs.diff_fail == 'true' }}
107+
if: needs.quality.outputs.fail == 'true' || needs.quality.outputs.diff_fail == 'true'
84108
run: |
85109
echo "Quality or diff checks failed."
86110
exit 1
@@ -90,6 +114,8 @@ jobs:
90114
needs: detect
91115
if: needs.detect.outputs.is_package_pr == 'false'
92116
runs-on: ubuntu-latest
117+
permissions:
118+
pull-requests: write
93119
steps:
94120
- name: Post skip notice
95121
uses: marocchino/sticky-pull-request-comment@v2
@@ -104,9 +130,12 @@ jobs:
104130
105131
auto-merge:
106132
name: Enable auto-merge
107-
needs: quality
108-
if: always() && needs.quality.result == 'success'
133+
needs: [quality, report]
134+
if: always() && needs.quality.result == 'success' && needs.report.result == 'success'
109135
runs-on: ubuntu-latest
136+
permissions:
137+
contents: write
138+
pull-requests: write
110139
steps:
111140
- name: Enable auto-merge via squash
112141
env:

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
out/
22
awesome-go
33
.cache/
4+
check-*
45

56
# Folders
67
.idea

0 commit comments

Comments
 (0)