Skip to content

Commit 4d64e46

Browse files
committed
feat: implement graceful fork PR degradation and document workflow_run pattern
- Add Job Summary notice when PR comments are skipped for fork PRs (not just a ::warning:: log line, but also visible in GH UI) - Update warning message to reference new documentation section - Add comprehensive Fork PR Comments section to README with: - Two-workflow (workflow_run) pattern as recommended best practice - pull_request_target alternative with security warnings - Add unit tests for the new Job Summary behavior Closes #143
1 parent c7245f6 commit 4d64e46

3 files changed

Lines changed: 252 additions & 6 deletions

File tree

README.md

Lines changed: 185 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ A GitHub Action for checking commit message formatting, branch naming, committer
2626
* [Optional Inputs](#optional-inputs)
2727
* [GitHub Action Job Summary](#github-action-job-summary)
2828
* [GitHub Pull Request Comments](#github-pull-request-comments)
29+
* [Fork PR Comments](#fork-pr-comments)
2930
* [Badging Your Repository](#badging-your-repository)
3031
* [Versioning](#versioning)
3132

@@ -123,7 +124,12 @@ jobs:
123124
> [!IMPORTANT]
124125
> `pr-comments` is an experimental feature. By default, it's disabled.
125126
>
126-
> PR comments are skipped for pull requests from forked repositories. For more details, refer to issue [`#143`](https://github.com/commit-check/commit-check-action/issues/143).
127+
> PR comments are skipped for pull requests from forked repositories. See
128+
> [Fork PR Comments](#fork-pr-comments) for details on how to enable this feature
129+
> for fork contributions.
130+
>
131+
> Note: write-access to pull-requests requires the `pull-requests: write` permission.
132+
> See [usage example](#usage).
127133

128134
Note: the default rule of above inputs is following [this configuration](https://github.com/commit-check/commit-check-action/blob/main/commit-check.toml). If you want to customize, just add your [`commit-check.toml`](https://commit-check.github.io/commit-check/configuration.html) config file under your repository root directory.
129135

@@ -149,6 +155,184 @@ By default, commit-check-action results are shown on the job summary page of the
149155

150156
![Failure pull request comment](https://github.com/commit-check/.github/blob/main/screenshot/failure-pr-comments.png)
151157

158+
## Fork PR Comments
159+
160+
When a pull request is opened from a **forked repository**, the `GITHUB_TOKEN` used by the
161+
`pull_request` event has **read-only** permissions by design (GitHub security policy).
162+
This means `pr-comments: true` cannot write a comment back to the PR.
163+
164+
By default, commit-check-action handles this gracefully:
165+
166+
- PR comment writing is **skipped** with a `::warning::` message in the logs
167+
- A **notice is added to the Job Summary** explaining why and how to fix it
168+
- The commit checks themselves **still run normally**
169+
170+
> **For most projects, this is sufficient** — contributors can see check results in the
171+
> action Job Summary. But if you *must* have PR comments on fork contributions, there
172+
> are two recommended approaches.
173+
174+
---
175+
176+
### Option 1: Two-workflow pattern (recommended)
177+
178+
This is the **official GitHub-recommended best practice** for writing PR comments from
179+
fork PRs. It uses the [`workflow_run`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run)
180+
event with **no security risks**.
181+
182+
**How it works:**
183+
184+
```
185+
pull_request workflow_run
186+
│ │
187+
▼ ▼
188+
┌──────────────┐ ┌──────────────────┐
189+
│ Workflow A │ │ Workflow B │
190+
│ (checks) │────►│ (comment writer) │
191+
│ │ │ │
192+
│ Token: READ │ │ Token: WRITE │
193+
│ Saves result │ │ Reads artifact │
194+
│ as artifact │ │ Posts PR comment │
195+
└──────────────┘ └──────────────────┘
196+
```
197+
198+
**Workflow A** — `.github/workflows/commit-check.yml` (triggered by `pull_request`):
199+
200+
```yaml
201+
name: Commit Check
202+
203+
on:
204+
pull_request:
205+
branches: ["main"]
206+
207+
jobs:
208+
check:
209+
runs-on: ubuntu-latest
210+
steps:
211+
- uses: actions/checkout@v5
212+
with:
213+
fetch-depth: 0
214+
- uses: commit-check/commit-check-action@v2
215+
with:
216+
message: true
217+
branch: true
218+
pr-comments: false # comments handled by Workflow B
219+
job-summary: true
220+
221+
# Save results so Workflow B can post a PR comment
222+
- uses: actions/upload-artifact@v4
223+
with:
224+
name: commit-check-result-${{ github.event.number }}
225+
path: result.txt
226+
```
227+
228+
**Workflow B**`.github/workflows/commit-check-comment.yml` (triggered by `workflow_run`):
229+
230+
```yaml
231+
name: Commit Check Comment
232+
233+
on:
234+
workflow_run:
235+
workflows: ["Commit Check"] # must match Workflow A's name exactly
236+
types: [completed]
237+
238+
jobs:
239+
comment:
240+
runs-on: ubuntu-latest
241+
permissions:
242+
pull-requests: write
243+
actions: read # needed to download artifacts
244+
steps:
245+
- uses: actions/download-artifact@v4
246+
with:
247+
name: commit-check-result-${{ github.event.workflow_run.pull_requests[0].number }}
248+
run-id: ${{ github.event.workflow_run.id }}
249+
github-token: ${{ github.token }}
250+
251+
- name: Read result and post PR comment
252+
uses: actions/github-script@v7
253+
with:
254+
script: |
255+
const fs = require('fs');
256+
const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }};
257+
const resultText = fs.readFileSync('result.txt', 'utf8').trim();
258+
259+
const successTitle = '# Commit-Check ✔️';
260+
const failureTitle = '# Commit-Check ❌';
261+
const body = resultText
262+
? `${failureTitle}\n\`\`\`\n${resultText}\n\`\`\``
263+
: successTitle;
264+
265+
const { data: comments } = await github.rest.issues.listComments({
266+
...context.repo,
267+
issue_number: prNumber,
268+
});
269+
270+
const existing = comments.find(c =>
271+
c.body.startsWith(successTitle) || c.body.startsWith(failureTitle)
272+
);
273+
274+
if (existing) {
275+
await github.rest.issues.updateComment({
276+
...context.repo,
277+
comment_id: existing.id,
278+
body,
279+
});
280+
} else {
281+
await github.rest.issues.createComment({
282+
...context.repo,
283+
issue_number: prNumber,
284+
body,
285+
});
286+
}
287+
```
288+
289+
> **Key security benefits:**
290+
> - Workflow B runs in the **base repository's context**, so `GITHUB_TOKEN` has full write
291+
> permissions (you explicitly grant `pull-requests: write`)
292+
> - Workflow B **does not checkout the PR code**, so untrusted fork code never runs
293+
> with elevated permissions
294+
> - The artifact only contains `result.txt` — no code or secrets
295+
296+
---
297+
298+
### Option 2: pull_request_target (advanced, use with caution)
299+
300+
If you understand the security implications, you can use
301+
[`pull_request_target`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target)
302+
which runs in the base repository's context with **write token access**.
303+
304+
> **⚠️ Security warning:** Never check out (`actions/checkout`) the PR's HEAD commit
305+
> when using `pull_request_target`. Always check out the base branch or use the
306+
> default merge commit. Otherwise, fork code could exfiltrate your repository's secrets.
307+
308+
```yaml
309+
name: Commit Check
310+
311+
on:
312+
pull_request_target:
313+
branches: ["main"]
314+
315+
jobs:
316+
commit-check:
317+
runs-on: ubuntu-latest
318+
permissions:
319+
contents: read
320+
pull-requests: write
321+
steps:
322+
# SAFE: checkout the merge commit, NOT the PR head
323+
- uses: actions/checkout@v5
324+
with:
325+
fetch-depth: 0
326+
- uses: commit-check/commit-check-action@v2
327+
with:
328+
message: true
329+
branch: true
330+
pr-comments: true
331+
```
332+
333+
> **When to use this:** Only if the two-workflow pattern is too complex for your setup
334+
> and you have thoroughly reviewed the security implications.
335+
152336
## Badging Your Repository
153337

154338
You can add a badge to your repository to show your contributors/users that you use commit-check!

main.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,26 @@ def add_pr_comments() -> int:
271271
# Fork PRs triggered by the pull_request event receive a read-only token;
272272
# the GitHub API will always reject comment writes with 403.
273273
if is_fork_pr():
274-
print(
275-
"::warning::Skipping PR comment: pull requests from forked repositories "
274+
msg = (
275+
"Skipping PR comment: pull requests from forked repositories "
276276
"cannot write comments via the pull_request event (GITHUB_TOKEN is "
277-
"read-only for forks). Use the pull_request_target event or the "
278-
"two-workflow artifact pattern instead. "
279-
"See https://github.com/commit-check/commit-check-action/issues/77"
277+
"read-only for forks). "
278+
"See https://github.com/commit-check/commit-check-action#fork-pr-comments "
279+
"for how to enable PR comments on fork PRs."
280280
)
281+
print(f"::warning::{msg}")
282+
if JOB_SUMMARY_ENABLED:
283+
with open(GITHUB_STEP_SUMMARY, "a") as f:
284+
f.write(
285+
"\n---\n"
286+
"### \u2139\ufe0f PR Comment Skipped\n\n"
287+
"Pull requests from forked repositories cannot write comments "
288+
"using the `pull_request` event because `GITHUB_TOKEN` has "
289+
"read-only permissions.\n\n"
290+
"> **\U0001f4a1 Tip:** To enable PR comments on fork PRs, see "
291+
"[Enabling PR Comments on Fork Pull Requests]"
292+
"(https://github.com/commit-check/commit-check-action#fork-pr-comments).\n"
293+
)
281294
return 0
282295

283296
try:

main_test.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,55 @@ def test_failure_writes_failure_title(self):
467467
self.assertIn("bad commit message", content)
468468

469469

470+
class TestAddPrComments(unittest.TestCase):
471+
def setUp(self):
472+
import tempfile
473+
474+
self._orig_dir = os.getcwd()
475+
self._tmpdir = tempfile.mkdtemp()
476+
os.chdir(self._tmpdir)
477+
with open("result.txt", "w", encoding="utf-8"):
478+
pass
479+
480+
def tearDown(self):
481+
os.chdir(self._orig_dir)
482+
483+
def test_disabled_returns_zero(self):
484+
with patch("main.PR_COMMENTS_ENABLED", False):
485+
rc = main.add_pr_comments()
486+
self.assertEqual(rc, 0)
487+
488+
def test_fork_pr_skips_comment_and_warns(self):
489+
with (
490+
patch("main.PR_COMMENTS_ENABLED", True),
491+
patch("main.is_fork_pr", return_value=True),
492+
patch("main.JOB_SUMMARY_ENABLED", False),
493+
patch("builtins.print") as mock_print,
494+
):
495+
rc = main.add_pr_comments()
496+
self.assertEqual(rc, 0)
497+
printed = mock_print.call_args[0][0]
498+
self.assertIn("::warning::", printed)
499+
self.assertIn("read-only", printed)
500+
501+
def test_fork_pr_writes_job_summary_hint(self):
502+
summary_path = os.path.join(self._tmpdir, "summary.txt")
503+
with (
504+
patch("main.PR_COMMENTS_ENABLED", True),
505+
patch("main.is_fork_pr", return_value=True),
506+
patch("main.JOB_SUMMARY_ENABLED", True),
507+
patch("main.GITHUB_STEP_SUMMARY", summary_path),
508+
patch("builtins.print"),
509+
):
510+
rc = main.add_pr_comments()
511+
self.assertEqual(rc, 0)
512+
with open(summary_path, encoding="utf-8") as f:
513+
content = f.read()
514+
self.assertIn("PR Comment Skipped", content)
515+
self.assertIn("read-only", content)
516+
self.assertIn("fork-pr-comments", content)
517+
518+
470519
class TestIsForkPr(unittest.TestCase):
471520
def test_no_event_path(self):
472521
with patch.dict(os.environ, {}, clear=True):

0 commit comments

Comments
 (0)