Skip to content

Commit fb162b4

Browse files
committed
fix: validate payload before writing artifact; replace continue-on-error with explicit pre-check
- capture-pr-review.yml: validate reviewer login and PR number before writing the artifact; throw with a clear message if the payload is malformed so the capture job fails visibly instead of publishing a bad artifact downstream - on-review.yml: remove continue-on-error on the download step; add an explicit artifact pre-check step that lists artifacts for the triggering run and sets an output; all subsequent steps gate on that output so a missing artifact (bot actor skip) is logged as a notice and the job exits cleanly, while an unexpected download failure now surfaces as a proper job failure Signed-off-by: Mounil Kanakhara <mounilkankhara@gmail.com>
1 parent 5d72e1d commit fb162b4

2 files changed

Lines changed: 29 additions & 7 deletions

File tree

.github/workflows/capture-pr-review.yml

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,18 @@ jobs:
2525
with:
2626
script: |
2727
const fs = require('fs');
28+
const reviewer = context.payload.review?.user?.login;
29+
const prNumber = context.payload.pull_request?.number;
30+
if (
31+
typeof reviewer !== 'string' ||
32+
reviewer.length === 0 ||
33+
!Number.isInteger(prNumber) ||
34+
prNumber <= 0
35+
) {
36+
throw new Error(`Invalid pull_request_review payload: reviewer=${reviewer}, pr_number=${prNumber}`);
37+
}
2838
fs.mkdirSync('review-data', { recursive: true });
29-
const reviewer = context.payload.review?.user?.login ?? null;
30-
const pr_number = context.payload.pull_request?.number ?? null;
31-
fs.writeFileSync('review-data/review.json', JSON.stringify({ reviewer, pr_number }));
39+
fs.writeFileSync('review-data/review.json', JSON.stringify({ reviewer, pr_number: prNumber }));
3240
3341
- name: Upload review metadata
3442
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1

.github/workflows/on-review.yml

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,37 @@ jobs:
6969
with:
7070
egress-policy: audit
7171

72+
- name: Check for review artifact
73+
id: check
74+
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
75+
with:
76+
script: |
77+
const name = `review-data-${context.payload.workflow_run.id}`;
78+
const { data } = await github.rest.actions.listWorkflowRunArtifacts({
79+
owner: context.repo.owner,
80+
repo: context.repo.repo,
81+
run_id: context.payload.workflow_run.id
82+
});
83+
const found = data.artifacts.some(a => a.name === name && !a.expired);
84+
if (!found) core.notice('Review artifact not found; capture job was likely skipped for a bot actor. Nothing to do.');
85+
core.setOutput('found', String(found));
86+
7287
- name: Download review metadata
73-
id: download
88+
if: steps.check.outputs.found == 'true'
7489
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
7590
with:
7691
name: review-data-${{ github.event.workflow_run.id }}
7792
run-id: ${{ github.event.workflow_run.id }}
7893
github-token: ${{ secrets.GITHUB_TOKEN }}
79-
continue-on-error: true
8094

8195
- name: Checkout repository
82-
if: steps.download.outcome == 'success'
96+
if: steps.check.outputs.found == 'true'
8397
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
8498
with:
8599
persist-credentials: false
86100

87101
- name: Run Remove Reviewer from Assignees
88-
if: steps.download.outcome == 'success'
102+
if: steps.check.outputs.found == 'true'
89103
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
90104
with:
91105
script: |

0 commit comments

Comments
 (0)