Skip to content

Commit 8bcf9ce

Browse files
shenxianpengclaude
andcommitted
fix(docs): fork checkout needs allow-unsafe-pr-checkout; name only the surfaces that exist
- The pull_request_target fallback could not work as documented: since actions/checkout v7 the step refuses to place fork code in such a job without allow-unsafe-pr-checkout: true, so the workflow never reached this action. The input is now in the example, and the warning is rewritten around what checkout itself says: the job holds the base repository's token, secrets, cache scope and runner, and the switch is how you acknowledge that. - The fork skip warning promised findings "in this job's summary" even with job-summary: false, where no summary is written. It now names the annotations alone in that case. A test pins both wordings and fails on the previous unconditional text. - The log excerpt in the docs gets a text language tag (MD040). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6
1 parent a793898 commit 8bcf9ce

3 files changed

Lines changed: 54 additions & 8 deletions

File tree

docs/fork-pr-comments.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@ Everything except the comment. The action does not degrade on a fork PR:
1919
So a contributor pushing to a fork already gets the red check, the per-finding annotations
2020
on their diff, and the whole report in the job summary. The action says so in the log:
2121

22-
```
22+
```text
2323
::warning::Skipping PR comment: pull requests from forked repositories cannot write
2424
comments via the pull_request event (GITHUB_TOKEN is read-only for forks). The findings
2525
are in this job's summary and in the annotations on the Files changed tab.
2626
```
2727

28+
(with `job-summary: false` the message names the annotations alone, since there is no
29+
summary to read.)
30+
2831
The run is **not** failed by this: `pr-comments: true` on a fork PR is a no-op, not an error.
2932

3033
## If you want feedback on the pull request itself
@@ -73,6 +76,10 @@ jobs:
7376
with:
7477
ref: refs/pull/${{ github.event.number }}/merge # the PR's commits
7578
fetch-depth: 0
79+
# Required since checkout v7: without it the step refuses to place
80+
# fork code in a pull_request_target job, and this workflow never
81+
# reaches the action below.
82+
allow-unsafe-pr-checkout: true
7683
- uses: commit-check/commit-check-action@v2
7784
with:
7885
message: true
@@ -81,11 +88,15 @@ jobs:
8188
```
8289
8390
> [!WARNING]
84-
> `pull_request_target` grants a writable token to a workflow whose checkout contains the
85-
> fork's code. commit-check only *reads* commit metadata and never executes the checked-out
86-
> tree, but any other step you add to this job runs with that token. Keep the job to the
87-
> checkout and this action, never cache or build from it, and never expose secrets to it.
88-
> See [GitHub's guidance on `pull_request_target`](https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/).
91+
> This is the "pwn request" shape, and `allow-unsafe-pr-checkout: true` is the switch
92+
> `actions/checkout` makes you flip to acknowledge it. The job runs with the base
93+
> repository's `GITHUB_TOKEN`, its secrets, its cache scope and its runner, and the
94+
> checkout puts the fork's code on that runner. commit-check only *reads* commit metadata
95+
> and never executes the checked-out tree, but any other step you add to this job would.
96+
> Keep the job to exactly these two steps, never build, install or cache from the tree,
97+
> and never expose secrets to it. Read
98+
> [Securely using `pull_request_target`](https://gh.io/securely-using-pull_request_target)
99+
> before turning this on.
89100

90101
## What this page used to describe
91102

main.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,11 +1477,15 @@ def add_pr_comments(results: list[ScopeResult]) -> int:
14771477
# the GitHub API will always reject comment writes with 403.
14781478
# pull_request_target events always have the configured token permissions.
14791479
if is_fork_pr_with_readonly_token():
1480+
# Name only the surfaces this run actually produced: with
1481+
# job-summary: false there is no summary to send the reader to.
1482+
where = "in the annotations on the Files changed tab"
1483+
if JOB_SUMMARY_ENABLED and GITHUB_STEP_SUMMARY:
1484+
where = f"in this job's summary and {where}"
14801485
msg = (
14811486
"Skipping PR comment: pull requests from forked repositories "
14821487
"cannot write comments via the pull_request event (GITHUB_TOKEN is "
1483-
"read-only for forks). The findings are in this job's summary and "
1484-
"in the annotations on the Files changed tab. "
1488+
f"read-only for forks). The findings are {where}. "
14851489
"See https://github.com/commit-check/commit-check-action/blob/main/docs/fork-pr-comments.md"
14861490
)
14871491
print(f"::warning::{msg}")

main_test.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,6 +1900,37 @@ def test_fork_pr_skips_comment_and_warns(self):
19001900
self.assertIn("::warning::", printed)
19011901
self.assertIn("read-only", printed)
19021902

1903+
def test_fork_pr_warning_names_only_the_surfaces_that_exist(self):
1904+
"""With job-summary: false the warning must not send the reader to a
1905+
summary this run never wrote; the annotations are still named."""
1906+
summary_path = os.path.join(tempfile.mkdtemp(), "summary.txt")
1907+
with (
1908+
patch("main.PR_COMMENTS_ENABLED", True),
1909+
patch.dict(os.environ, {"GITHUB_EVENT_NAME": "pull_request"}),
1910+
patch("main.is_fork_pr", return_value=True),
1911+
patch("main.JOB_SUMMARY_ENABLED", False),
1912+
patch("main.GITHUB_STEP_SUMMARY", summary_path),
1913+
patch("builtins.print") as mock_print,
1914+
):
1915+
rc = main.add_pr_comments([pass_scope()])
1916+
self.assertEqual(rc, 0)
1917+
printed = mock_print.call_args[0][0]
1918+
self.assertIn("annotations on the Files changed tab", printed)
1919+
self.assertNotIn("job's summary", printed)
1920+
self.assertFalse(os.path.exists(summary_path))
1921+
1922+
with (
1923+
patch("main.PR_COMMENTS_ENABLED", True),
1924+
patch.dict(os.environ, {"GITHUB_EVENT_NAME": "pull_request"}),
1925+
patch("main.is_fork_pr", return_value=True),
1926+
patch("main.JOB_SUMMARY_ENABLED", True),
1927+
patch("main.GITHUB_STEP_SUMMARY", summary_path),
1928+
patch("builtins.print") as mock_print,
1929+
):
1930+
main.add_pr_comments([pass_scope()])
1931+
printed = mock_print.call_args[0][0]
1932+
self.assertIn("in this job's summary and in the annotations", printed)
1933+
19031934
def test_fork_pr_writes_job_summary_hint(self):
19041935
summary_path = os.path.join(tempfile.mkdtemp(), "summary.txt")
19051936
with (

0 commit comments

Comments
 (0)