ci: run external contributor policy on pull_request_target with relaxed limits

[dyoshikawa-claw]

Summary

• Switch .github/workflows/pr-policy.yml trigger from pull_request to pull_request_target
• Keep permissions minimal (contents: read, pull-requests: write)
• Add an explicit safety note that this job does not checkout or execute PR code
• Relax addition line limit from 400 to 1,000 lines
• Update CONTRIBUTING.md to recommend ~400 lines with a hard limit of 1,000
• Clarify permissions comment to 'Minimum required'

Why

• pull_request from forks can be read-only for PR write operations, which caused policy enforcement to fail when trying to comment/close violating PRs
• The previous 400-line limit was too restrictive for meaningful contributions

Risk Mitigation

• No checkout step
• No execution of PR code
• Least-privilege job permissions

Validation

• pnpm cicheck (pass)

[github-actions]

PR #1252 Review - Mergeability: ✅ APPROVED

Code Review Findings

1. Security justification is sound: The switch from pull_request to pull_request_target is appropriate for the stated use case. Since forks can trigger read-only pull_request events that prevent write operations (commenting, closing PRs), pull_request_target is the correct solution. The risk mitigation strategy—no checkout, no PR code execution—is well-implemented.

2. Comment on line 9 is now inaccurate: The comment # Default for pull_request events refers to pull_request events, but the trigger has been changed to pull_request_target. Consider updating this to # Default, safe for pull_request_target without checkout or simply removing the outdated reference.

3. Safety comment could be more explicit: The added comment # Safety: do not check out or execute PR code in this job. is good, but consider expanding it to explain WHY this matters with pull_request_target. For example: # SAFETY: This job runs on pull_request_target with write permissions. Do NOT checkout or execute PR code, as it could be malicious.

4. Script injection handling is correct: Untrusted inputs from the PR (like PR_AUTHOR, PR_NUMBER) are properly passed through environment variables and safely quoted in shell commands. This follows the project's GitHub Actions security guidelines correctly.

5. Permissions are appropriately minimal: The contents: read and pull-requests: write permissions follow the principle of least privilege. No additional permissions are granted that could be exploited.

6. Consider adding a defensive assertion: To ensure future maintainers don't accidentally add a checkout step, consider adding a comment or even a step that fails if a checkout is detected. While not strictly necessary, it provides defense-in-depth.

7. Line 64's $PR_AUTHOR usage is safe but could be validated: The gh pr list --author "$PR_AUTHOR" command properly quotes the variable. However, since PR_AUTHOR comes from github.event.pull_request.user.login (user-controlled), consider adding validation that it only contains valid GitHub username characters (^[a-zA-Z0-9-]+$).

8. Heredoc usage in COMMENT_BODY is safe: The shell variables interpolated into the comment body ($PR_ADDITIONS, $GH_REPO, etc.) are either constants or come from trusted GitHub API responses, not directly from user input. This is correctly implemented.

9. Documentation value: Consider adding a brief comment in the on: section explaining why pull_request_target is used instead of pull_request, to help future reviewers understand the security trade-off that was made.

Security Review Findings

10. pull_request_target usage is appropriate with current mitigations - The switch to pull_request_target is necessary because fork PRs under pull_request events have read-only token permissions, preventing the workflow from commenting on or closing PRs. The critical security mitigation is that this workflow explicitly does NOT checkout PR code (actions/checkout is absent), eliminating the primary attack vector where malicious PR code could execute in a trusted context with secret access.

11. Permissions are correctly scoped to least privilege - The declared permissions (contents: read, pull-requests: write) are minimal and directly necessary for the workflow's function. The pull-requests: write permission is required for gh pr comment and gh pr close operations, and contents: read is a safe default that doesn't expose sensitive data.

12. Environment variable pattern correctly avoids script injection - The workflow follows the project's security guidelines by passing GitHub context values (e.g., PR_AUTHOR, PR_NUMBER) through environment variables rather than directly interpolating ${{ }} expressions into the run: script. This prevents script injection at the GitHub Actions level.

13. Shell quoting is properly applied - The PR_AUTHOR variable (which comes from github.event.pull_request.user.login and is user-controllable) is always double-quoted when used (e.g., --author "$PR_AUTHOR"), preventing shell word splitting and globbing attacks even if a malicious username contains spaces or special characters.

14. Low-risk attack surface remains via malicious usernames - While properly quoted, the PR_AUTHOR value is passed to gh pr list --author. If the gh CLI has any argument injection vulnerabilities with specially crafted usernames, this could be a vector. However, this is an extremely low-risk scenario as it would require a vulnerability in the GitHub CLI itself, and the impact would be limited (no code execution, just potential API manipulation).

15. AUTHOR_ASSOCIATION check is trustworthy - The security gate that skips policy checks for OWNER, MEMBER, or COLLABORATOR relies on github.event.pull_request.author_association, which is controlled entirely by GitHub and cannot be spoofed by external PR authors. This is a safe trust boundary.

16. No secret exposure risk - The workflow only uses secrets.GITHUB_TOKEN, which is automatically scoped and temporary. There are no repository secrets, deploy keys, or other sensitive credentials that could be exfiltrated by a malicious actor exploiting this workflow.

17. Safety comment is a good practice but not a technical control - The added comment # Safety: do not check out or execute PR code in this job. serves as documentation for maintainers but provides no runtime enforcement. Future modifications to this workflow could inadvertently add a checkout step. Consider adding a code ownership rule or branch protection requiring security review for changes to this file.

18. Branch filter provides additional defense - The branches: [main] constraint limits the workflow to PRs targeting the main branch, reducing the attack surface compared to running on all branches.

19. Overall risk assessment: LOW - The combination of no code checkout, minimal permissions, proper environment variable handling, and appropriate quoting makes this a safe implementation of pull_request_target. The change addresses a legitimate functional need (enabling policy enforcement on fork PRs) without introducing significant new security risks. The mitigations are sufficient for the threat model of a CLI tool repository.

Summary

The PR is ready to merge. All identified issues are suggestions for improvement (medium or low severity) and not merge blockers. The change successfully addresses the functional need while maintaining good security practices.

github run