fix: LinkBot fork permission issue #1363
Conversation
|
Hi, this is WorkflowBot.
|
Mounil2005
changed the title
Signed-off-by: Mounil <[email protected]>
Mounil2005
force-pushed
the
fix/linkbot-clean
branch
from
5ee6289 to
574a8aa
Compare
|
Warning Rate limit exceeded@Mounil2005 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughWorkflow trigger changed to Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/bot-pr-missing-linked-issue.js.github/workflows/bot-pr-missing-linked-issue.ymlCHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
.github/scripts/**/*.js
⚙️ CodeRabbit configuration file
.github/scripts/**/*.js: Review JavaScript scripts as long-lived automation code.Scripts must remain:
Focused
Readable
Purpose-built
All
context.payloadfields MUST be validatedFree-form text MUST NOT be trusted
Dynamic code execution is prohibited
Avoid
child_process.exec; preferexecFileif neededAll async operations MUST be wrapped in try/catch
Errors MUST include contextual metadata
Duplicate API calls MUST be avoided
Marker-based deduplication is required
Scripts MUST NOT assume write access
Permission failures MUST be handled gracefully
Files:
.github/scripts/bot-pr-missing-linked-issue.js
.github/workflows/**/*
⚙️ CodeRabbit configuration file
.github/workflows/**/*: Review workflows as security-sensitive infrastructure.A good workflow is small, focused, and boring.
If a workflow is clever, generic, or overly flexible, it is a risk.
PRIORITY 0 — ABSOLUTE REQUIREMENTS
- All third-party actions MUST be pinned to full commit SHAs, similar to other workflows.
permissions:MUST be explicitly declared and minimally scoped.- Workflows MUST behave safely when executed from forks.
- YAML MUST orchestrate steps, not implement business logic.
- Any workflow that mutates GitHub state MUST support dry-run mode.
- Dry-run behavior must be explicit and visible in logs.
- Workflows MUST NOT modify repository source code outside
.github/.
PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
- The title of each workflow must be relevant, match similar naming schemes, and match its script filename.
- Each workflow MUST have a single, clearly defined objective and SHOULD document this in a top-level comment.
- Flag workflows that:
- Attempt to be generic “frameworks”
- Include speculative or future-facing logic
- Perform actions unrelated to the stated goal
- Over-abstraction and excess flexibility are maintenance risks.
PRIORITY 2 — INPUT HARDENING
- Treat ALL GitHub event data as potentially hostile input, including:
- issue titles, bodies, and comments
- labels, usernames, branch names
- Free-form user input MUST NOT be passed directly into:
- shell commands
- gh CLI arguments
- Node.js exec / spawn calls
- Require strict allowlists or exact string matches.
- Flag any use of:
- eval or bash -c
- backticks or $(...) with user-controlled input
------------------...
Files:
.github/workflows/bot-pr-missing-linked-issue.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
CHANGELOG.md (1)
158-158: LGTM - Changelog entry is clear and appropriate.The changelog entry accurately documents the fix for the LinkBot permission issue on fork PRs.
.github/workflows/bot-pr-missing-linked-issue.yml (3)
4-16: Verify pull_request_target behavior with fork PRs.The trigger change from
pull_requesttopull_request_targetis correct for fixing fork PR permission issues. This allows the workflow to run with write permissions while checking out trusted code (main branch).Ensure you've tested this with an actual fork PR to confirm:
- The workflow runs successfully on fork PRs
- Comments are posted correctly
- The workflow doesn't execute any untrusted code from the fork
Based on coding guidelines, workflows must behave safely when executed from forks.
18-21: LGTM - Permissions are correctly scoped.The permissions are explicitly declared and minimal:
issues: write- needed for posting commentspull-requests: write- needed for PR operationscontents: read- needed for checkoutBased on coding guidelines, PRIORITY 7 — PERMISSION CORRECTNESS.
39-40: LGTM - Correct use of ref: main for security.Checking out
mainis essential when usingpull_request_targetto avoid executing untrusted code from fork PRs. This ensures only the trusted script from the base repository is executed.Based on coding guidelines, workflows must behave safely when executed from forks.
.github/scripts/bot-pr-missing-linked-issue.js (3)
5-19: LGTM - Good handling of both trigger types.The script correctly handles both
pull_request_target(wherecontext.payload.pull_requestexists) andworkflow_dispatch(where PR data must be fetched). This provides good flexibility for testing.
30-37: LGTM - Proper idempotent comment detection.The marker-based detection (
includes("this is LinkBot")) prevents duplicate comments, which is essential for idempotent workflow execution.Based on coding guidelines, duplicate prevention is required.
59-72: LGTM - Excellent dry-run implementation.The dry-run logic is clear and visible in logs, showing exactly what would be posted without actually mutating state. This matches the guidelines perfectly.
Based on coding guidelines, PRIORITY 3 — DRY-RUN & SAFE OPERATION.
Mounil2005
changed the title
Mounil2005
force-pushed
the
fix/linkbot-clean
branch
from
574a8aa to
f65d2cc
Compare
Signed-off-by: Mounil <[email protected]>
Signed-off-by: Mounil <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/bot-pr-missing-linked-issue.js.github/workflows/bot-pr-missing-linked-issue.ymlCHANGELOG.md
🧰 Additional context used
📓 Path-based instructions (2)
.github/scripts/**/*.js
⚙️ CodeRabbit configuration file
.github/scripts/**/*.js: Review JavaScript scripts as long-lived automation code.Scripts must remain:
Focused
Readable
Purpose-built
All
context.payloadfields MUST be validatedFree-form text MUST NOT be trusted
Dynamic code execution is prohibited
Avoid
child_process.exec; preferexecFileif neededAll async operations MUST be wrapped in try/catch
Errors MUST include contextual metadata
Duplicate API calls MUST be avoided
Marker-based deduplication is required
Scripts MUST NOT assume write access
Permission failures MUST be handled gracefully
Files:
.github/scripts/bot-pr-missing-linked-issue.js
.github/workflows/**/*
⚙️ CodeRabbit configuration file
.github/workflows/**/*: Review workflows as security-sensitive infrastructure.A good workflow is small, focused, and boring.
If a workflow is clever, generic, or overly flexible, it is a risk.
PRIORITY 0 — ABSOLUTE REQUIREMENTS
- All third-party actions MUST be pinned to full commit SHAs, similar to other workflows.
permissions:MUST be explicitly declared and minimally scoped.- Workflows MUST behave safely when executed from forks.
- YAML MUST orchestrate steps, not implement business logic.
- Any workflow that mutates GitHub state MUST support dry-run mode.
- Dry-run behavior must be explicit and visible in logs.
- Workflows MUST NOT modify repository source code outside
.github/.
PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
- The title of each workflow must be relevant, match similar naming schemes, and match its script filename.
- Each workflow MUST have a single, clearly defined objective and SHOULD document this in a top-level comment.
- Flag workflows that:
- Attempt to be generic “frameworks”
- Include speculative or future-facing logic
- Perform actions unrelated to the stated goal
- Over-abstraction and excess flexibility are maintenance risks.
PRIORITY 2 — INPUT HARDENING
- Treat ALL GitHub event data as potentially hostile input, including:
- issue titles, bodies, and comments
- labels, usernames, branch names
- Free-form user input MUST NOT be passed directly into:
- shell commands
- gh CLI arguments
- Node.js exec / spawn calls
- Require strict allowlists or exact string matches.
- Flag any use of:
- eval or bash -c
- backticks or $(...) with user-controlled input
------------------...
Files:
.github/workflows/bot-pr-missing-linked-issue.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
CHANGELOG.md (1)
158-158: LGTM!The changelog entry accurately documents the fix for fork PR permissions by noting the trigger change to
pull_request_targetand the addition of proper permissions..github/scripts/bot-pr-missing-linked-issue.js (1)
59-72: DRY_RUN implementation and marker-based deduplication look good.The script correctly logs intended actions instead of posting when
DRY_RUN=true, and uses marker-based detection to prevent duplicate comments..github/workflows/bot-pr-missing-linked-issue.yml (3)
37-40: Security pattern correctly implemented.Using
pull_request_targetwithref: maincheckout is the correct approach. This ensures the workflow has write permissions for fork PRs while only executing trusted code from the base branch, not untrusted code from the fork.
18-21: Permissions and action SHA pins are correctly configured.Actions are pinned to full commit SHAs as required. Permissions are explicitly declared and appropriately scoped:
pull-requests: writefor PR operationscontents: readfor checkoutissues: writefor posting comments (GitHub API treats PR comments as issue comments)Also applies to: 32-48
43-46: Environment variable configuration looks correct.The
DRY_RUNandPR_NUMBERvariables are properly sourced from the appropriate event context with fallbacks for bothpull_request_targetandworkflow_dispatchtriggers.
Signed-off-by: Mounil <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1363 +/- ##
=======================================
Coverage 92.29% 92.29%
=======================================
Files 139 139
Lines 8515 8515
=======================================
Hits 7859 7859
Misses 656 656 🚀 New features to boost your workflow:
|
2 participants
Description:
Fix LinkBot permission issue for fork PRs by changing trigger to pull_request_target
Related issue(s):
Fixes #1183
Notes for reviewer:
The core issue was that GITHUB_TOKEN has restricted permissions for fork PRs under pull_request trigger, causing 403 "Resource not accessible by integration" errors when LinkBot tried to comment. The pull_request_target trigger runs in the base repository context with proper permissions, resolving the permission issue while maintaining security by not executing untrusted code from forks.
Added DRY_RUN functionality allows contributors to safely test the workflow on forks without actually posting comments - it only logs what would happen.
Checklist