-
-
Notifications
You must be signed in to change notification settings - Fork 3
fix: handle fork PR permission issue for pr-comments gracefully #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
faa418a
fce5b5a
bba7f45
1741ed5
21bb90a
530b572
b6390fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| venv/ | ||
| .venv/ | ||
| __pycache__/ | ||
| result.txt |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| # Fork PR Comments | ||
|
|
||
| When a pull request is opened from a **forked repository**, the `GITHUB_TOKEN` used by the | ||
| `pull_request` event has **read-only** permissions by design (GitHub security policy). | ||
| This means `pr-comments: true` cannot write a comment back to the PR. | ||
|
|
||
| By default, commit-check-action handles this gracefully: | ||
|
|
||
| - PR comment writing is **skipped** with a `::warning::` message in the logs | ||
| - A **notice is added to the Job Summary** explaining why and how to fix it | ||
| - The commit checks themselves **still run normally** | ||
|
|
||
| > **For most projects, this is sufficient** — contributors can see check results in the | ||
| > action Job Summary. But if you *must* have PR comments on fork contributions, there | ||
| > are two recommended approaches. | ||
|
|
||
| --- | ||
|
|
||
| ## Option 1: Two-workflow pattern (recommended) | ||
|
|
||
| This is the **official GitHub-recommended best practice** for writing PR comments from | ||
| fork PRs. It uses the [`workflow_run`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run) | ||
| event with **no security risks**. | ||
|
|
||
| > 📁 Ready-to-use files: [`examples/commit-check-workflow-a.yml`](../examples/commit-check-workflow-a.yml) | ||
| > and [`examples/commit-check-workflow-b.yml`](../examples/commit-check-workflow-b.yml) | ||
|
|
||
| **How it works:** | ||
|
|
||
| ``` | ||
| pull_request workflow_run | ||
| │ │ | ||
| ▼ ▼ | ||
| ┌──────────────┐ ┌──────────────────┐ | ||
| │ Workflow A │ │ Workflow B │ | ||
| │ (checks) │────►│ (comment writer) │ | ||
| │ │ │ │ | ||
| │ Token: READ │ │ Token: WRITE │ | ||
| │ Saves result │ │ Reads artifact │ | ||
| │ as artifact │ │ Posts PR comment │ | ||
| └──────────────┘ └──────────────────┘ | ||
| ``` | ||
|
|
||
| ### Workflow A | ||
|
|
||
| `.github/workflows/commit-check.yml` (triggered by `pull_request`): | ||
|
|
||
| ```yaml | ||
| name: Commit Check | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: ["main"] | ||
|
|
||
| jobs: | ||
| check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: commit-check/commit-check-action@v2 | ||
| with: | ||
| message: true | ||
| branch: true | ||
| pr-comments: false # comments handled by Workflow B | ||
| job-summary: true | ||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: commit-check-result-${{ github.event.number }} | ||
| path: result.txt # saved for Workflow B | ||
| ``` | ||
|
|
||
| > 📄 Full file: [`examples/commit-check-workflow-a.yml`](../examples/commit-check-workflow-a.yml) | ||
|
|
||
| ### Workflow B | ||
|
|
||
| `.github/workflows/commit-check-comment.yml` (triggered by `workflow_run`): | ||
|
|
||
| ```yaml | ||
| name: Commit Check Comment | ||
|
|
||
| on: | ||
| workflow_run: | ||
| workflows: ["Commit Check"] # must match Workflow A's name exactly | ||
| types: [completed] | ||
|
|
||
| jobs: | ||
| comment: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write | ||
| actions: read # needed to download artifacts | ||
| steps: | ||
| - uses: actions/download-artifact@v4 | ||
| with: | ||
| name: commit-check-result-${{ github.event.workflow_run.pull_requests[0].number }} | ||
| run-id: ${{ github.event.workflow_run.id }} | ||
| github-token: ${{ github.token }} | ||
| - name: Read result and post PR comment | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| // See examples/commit-check-workflow-b.yml for full script | ||
| const fs = require('fs'); | ||
| const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }}; | ||
| const resultText = fs.readFileSync('result.txt', 'utf8').trim(); | ||
| const body = resultText | ||
| ? '# Commit-Check ❌\n```\n' + resultText + '\n```' | ||
| : '# Commit-Check ✔️'; | ||
| // Creates or updates the matching PR comment | ||
| ``` | ||
|
|
||
| > 📄 Full file: [`examples/commit-check-workflow-b.yml`](../examples/commit-check-workflow-b.yml) | ||
|
|
||
| ### Key security benefits | ||
|
|
||
| - Workflow B runs in the **base repository's context**, so `GITHUB_TOKEN` has full write | ||
| permissions (you explicitly grant `pull-requests: write`) | ||
| - Workflow B **does not checkout the PR code**, so untrusted fork code never runs | ||
| with elevated permissions | ||
| - The artifact only contains `result.txt` — no code or secrets | ||
|
|
||
| --- | ||
|
|
||
| ## Option 2: pull_request_target (advanced, use with caution) | ||
|
|
||
| If you understand the security implications, you can use | ||
| [`pull_request_target`](https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target) | ||
| which runs in the base repository's context with **write token access**. | ||
|
|
||
| > **⚠️ Security warning:** Never check out (`actions/checkout`) the PR's HEAD commit | ||
| > when using `pull_request_target`. Always check out the base branch or use the | ||
| > default merge commit. Otherwise, fork code could exfiltrate your repository's secrets. | ||
|
|
||
| ```yaml | ||
| name: Commit Check | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| branches: ["main"] | ||
|
|
||
| jobs: | ||
| commit-check: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| steps: | ||
| # SAFE: checkout the merge commit, NOT the PR head | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: commit-check/commit-check-action@v2 | ||
| with: | ||
| message: true | ||
| branch: true | ||
| pr-comments: true | ||
| ``` | ||
|
|
||
| > ✅ With `pull_request_target`, `pr-comments: true` **does work** on fork PRs — | ||
| > the token has the workflow's configured permissions regardless of whether the PR | ||
| > is from a fork. | ||
| > | ||
| > **When to use this:** Only if the two-workflow pattern is too complex for your setup | ||
| > and you have thoroughly reviewed the security implications. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Workflow A: Run commit checks on pull_request events. | ||
| # | ||
| # This workflow is triggered by pull_request and runs commit checks. | ||
| # It uploads the result as an artifact so Workflow B (commit-check-comment.yml) | ||
| # can read it and post a PR comment with full write permissions. | ||
| # | ||
| # See https://github.com/commit-check/commit-check-action#fork-pr-comments | ||
|
|
||
| name: Commit Check | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: ["main"] | ||
|
|
||
| jobs: | ||
| check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: commit-check/commit-check-action@v2 | ||
| with: | ||
| message: true | ||
| branch: true | ||
| pr-comments: false # comments handled by Workflow B | ||
| job-summary: true | ||
|
|
||
| # Save results so Workflow B can post a PR comment | ||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: commit-check-result-${{ github.event.number }} | ||
| path: result.txt | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,68 @@ | ||||||||||||||||
| # Workflow B: Post PR comment after commit checks complete. | ||||||||||||||||
| # | ||||||||||||||||
| # This workflow is triggered by the workflow_run event from Workflow A. | ||||||||||||||||
| # It runs in the base repository's context with full write permissions, | ||||||||||||||||
| # making it safe for fork PRs (no checkout of fork code). | ||||||||||||||||
| # | ||||||||||||||||
| # Prerequisites: | ||||||||||||||||
| # - Workflow A (commit-check.yml) must exist and upload an artifact named | ||||||||||||||||
| # commit-check-result-<PR-number> containing result.txt | ||||||||||||||||
| # | ||||||||||||||||
| # See https://github.com/commit-check/commit-check-action#fork-pr-comments | ||||||||||||||||
|
|
||||||||||||||||
| name: Commit Check Comment | ||||||||||||||||
|
|
||||||||||||||||
| on: | ||||||||||||||||
| workflow_run: | ||||||||||||||||
| workflows: ["Commit Check"] # must match Workflow A's name exactly | ||||||||||||||||
| types: [completed] | ||||||||||||||||
|
|
||||||||||||||||
| jobs: | ||||||||||||||||
| comment: | ||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||
| permissions: | ||||||||||||||||
| pull-requests: write | ||||||||||||||||
| actions: read # needed to download artifacts | ||||||||||||||||
|
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -name "commit-check-workflow-b.yml" -type fRepository: commit-check/commit-check-action Length of output: 115 🏁 Script executed: cat -n ./examples/commit-check-workflow-b.ymlRepository: commit-check/commit-check-action Length of output: 2864 🌐 Web query:
💡 Result: To interact with issue and pull request comments via the GitHub REST API, the required permissions primarily fall under the Issues and Pull Requests categories [1][2]. For standard issue comments and pull request timeline comments, the required permission is generally 'issues' [2]. Creating, updating, and listing these comments typically requires 'write' access to the 'issues' permission [2]. Because every pull request is considered an issue by the API, the 'issues' permission covers comment actions across both [1]. For more specific comment types, requirements may differ: 1. Issue Comments (including PR timeline comments): Requires 'issues' permission ('read' for listing, 'write' for creating/updating) [1][2]. 2. Pull Request Review Comments (comments on the diff): Requires 'pull_requests' permission ('read' for listing, 'write' for creating/updating) [2][3]. 3. Commit Comments: Often requires 'contents' permission ('read' for listing, 'write' for creating/updating) [4][5]. When configuring fine-grained personal access tokens or GitHub App permissions: - 'Write' access always includes 'read' access [2]. - If you are unsure of the specific requirements for a particular endpoint, you can check the 'X-Accepted-GitHub-Permissions' header returned in the API response, which explicitly states the necessary permissions for that call [6][7][8]. - For GitHub Actions workflows using the 'GITHUB_TOKEN', you can define these permissions in your workflow YAML file using the 'permissions' key (e.g., 'issues: write' and 'pull-requests: write') [9]. Citations:
Add This workflow uses Proposed fix permissions:
+ issues: write
pull-requests: write
actions: read # needed to download artifacts📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| steps: | ||||||||||||||||
| - uses: actions/download-artifact@v4 | ||||||||||||||||
| with: | ||||||||||||||||
| name: commit-check-result-${{ github.event.workflow_run.pull_requests[0].number }} | ||||||||||||||||
| run-id: ${{ github.event.workflow_run.id }} | ||||||||||||||||
| github-token: ${{ github.token }} | ||||||||||||||||
|
|
||||||||||||||||
| - name: Read result and post PR comment | ||||||||||||||||
| uses: actions/github-script@v7 | ||||||||||||||||
| with: | ||||||||||||||||
| script: | | ||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||
| const prNumber = ${{ github.event.workflow_run.pull_requests[0].number }}; | ||||||||||||||||
| const resultText = fs.readFileSync('result.txt', 'utf8').trim(); | ||||||||||||||||
|
|
||||||||||||||||
| const successTitle = '# Commit-Check ✔️'; | ||||||||||||||||
| const failureTitle = '# Commit-Check ❌'; | ||||||||||||||||
| const body = resultText | ||||||||||||||||
| ? `${failureTitle}\n\`\`\`\n${resultText}\n\`\`\`` | ||||||||||||||||
| : successTitle; | ||||||||||||||||
|
|
||||||||||||||||
| const { data: comments } = await github.rest.issues.listComments({ | ||||||||||||||||
| ...context.repo, | ||||||||||||||||
| issue_number: prNumber, | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| const existing = comments.find(c => | ||||||||||||||||
| c.body.startsWith(successTitle) || c.body.startsWith(failureTitle) | ||||||||||||||||
| ); | ||||||||||||||||
|
|
||||||||||||||||
| if (existing) { | ||||||||||||||||
| await github.rest.issues.updateComment({ | ||||||||||||||||
| ...context.repo, | ||||||||||||||||
| comment_id: existing.id, | ||||||||||||||||
| body, | ||||||||||||||||
| }); | ||||||||||||||||
| } else { | ||||||||||||||||
| await github.rest.issues.createComment({ | ||||||||||||||||
| ...context.repo, | ||||||||||||||||
| issue_number: prNumber, | ||||||||||||||||
| body, | ||||||||||||||||
| }); | ||||||||||||||||
| } | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: commit-check/commit-check-action
Length of output: 154
🏁 Script executed:
Repository: commit-check/commit-check-action
Length of output: 150
🌐 Web query:
GitHub Actions default step behavior on failure - do steps skip by default when previous steps fail💡 Result:
Yes, GitHub Actions steps are skipped by default when a previous step fails [1][2]. By default, every step in a job has an implicit condition of success, which means subsequent steps will only execute if all previous steps have completed successfully [2]. If a step fails (i.e., returns a non-zero exit code), the runner stops executing the remaining steps in that job [1][3]. You can override this default behavior using the following methods: 1. always conditional: You can add if: always to a step to ensure it executes regardless of whether previous steps succeeded or failed [4][2]. This is commonly used for cleanup or reporting tasks [4][2]. 2. continue-on-error: If you set continue-on-error: true on a specific step, the job will treat that step as a success even if it fails, allowing subsequent steps to execute normally [1][3]. Note that the job's final status will still reflect the failure unless otherwise handled [5]. 3. Conditional logic with outcome: You can use the steps.<step_id>.outcome context to check if a specific previous step failed and run subsequent steps conditionally, such as using if: failure && steps..outcome == 'failure' [5][2][3]. Additionally, note that shell steps (bash/sh) in GitHub Actions run with set -e enabled by default, which ensures that a non-zero exit code in any part of a command script triggers the step failure [6].
Citations:
🌐 Web query:
GitHub Actions if: always() condition when to use conditional steps💡 Result:
In GitHub Actions, the always function is a conditional status check that causes a step or job to execute regardless of the status of preceding steps or jobs, including when the workflow is canceled [1][2][3]. When to use always Use always for tasks that must run regardless of the outcome of previous steps, such as: - Sending notifications or logs [1][2]. - Uploading test artifacts or diagnostics after a failure [4]. - Performing essential cleanup operations [5]. Important Considerations and Alternatives While always is powerful, it should be used with caution because it overrides the default behavior where subsequent steps are skipped if a previous step fails or is canceled [1][2][6]. 1. Cancellation Behavior: Because always returns true even if the workflow is canceled, it can trigger tasks you might not want to run during a cancellation, such as deployment or external notifications [4][6]. 2. Critical Failures: It is recommended to avoid using always for tasks that could suffer from critical failures (e.g., source checkout), as the workflow might hang until it reaches a timeout [1][2]. 3. Recommended Alternative: If you want a step to run regardless of success or failure but still respect a user's decision to cancel the workflow, use the following conditional instead [1][6]: if: ${{ success || failure }} This alternative ensures the step runs if the previous steps succeeded or failed, but prevents it from running if the job itself was explicitly canceled [4][6]. Technical Context By default, GitHub Actions applies a hidden success check to all steps [2]. If you provide an if: condition, you must explicitly include status functions if you want to override this default behavior. For example, if you want a step to run only on failure, you must use if: ${{ failure }} [1][7]. Always effectively overrides all default status checks, ensuring execution [1][2].
Citations:
🏁 Script executed:
# Read the full commit-check-workflow-a.yml file cat -n examples/commit-check-workflow-a.ymlRepository: commit-check/commit-check-action
Length of output: 1252
🏁 Script executed:
# Read the full commit-check-workflow-b.yml file to understand the dependency cat -n examples/commit-check-workflow-b.ymlRepository: commit-check/commit-check-action
Length of output: 2864
🏁 Script executed:
Repository: commit-check/commit-check-action
Length of output: 89
🏁 Script executed:
Repository: commit-check/commit-check-action
Length of output: 86
🌐 Web query:
commit-check-action exit code behavior on failed checks - does it fail the step or just output results💡 Result:
The commit-check-action is designed to fail the GitHub Actions step when checks fail [1][2]. By default, when commit-check-action identifies a failed validation, it exits with a non-zero exit code (typically 1), which causes the GitHub Actions step to be marked as failed [1][3][2]. In addition to failing the step, the action also outputs detailed information about the failed checks, which can be viewed in the workflow logs or on the job summary page [2][4]. If you wish to prevent the step from failing while still running the checks (for example, to see results without stopping the workflow), you can use the dry-run mode [1][5]. When dry-run is enabled, the action will output the results of the checks but will always return an exit code of 0, allowing the step to pass regardless of the validation outcome [1][5][2].
Citations:
Ensure artifact upload runs even when checks fail.
actions/upload-artifactcurrently runs only on success. Whencommit-check-actiondetects failures (at line 22), it exits with a non-zero code, causing the subsequent upload step to be skipped. This breaks Workflow B's ability to post a PR comment on failing checks, since it cannot download the missingresult.txtartifact.Proposed fix
# Save results so Workflow B can post a PR comment - uses: actions/upload-artifact@v4 + if: always() with: name: commit-check-result-${{ github.event.number }} path: result.txt🤖 Prompt for AI Agents