Fix 403 when posting lint comments on fork PRs - #466
Merged
openshift-merge-bot[bot] merged 5 commits intoMay 12, 2026
Merged
Conversation
Switch from pull_request to pull_request_target so the GITHUB_TOKEN has write access for fork PRs. Explicitly checkout the PR head SHA since pull_request_target defaults to the base branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci
Bot
added
the
approved
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughRemoves ChangesLint Plugins permission change
Lint Review workflow addition
Sequence DiagramsequenceDiagram
participant LintPlugins as Lint Plugins workflow
participant GH as GitHub Actions
participant LintReview as Lint Review workflow
participant Skillsaw as stbenjam/skillsaw/review
LintPlugins->>GH: complete run
GH->>LintReview: workflow_run (completed)
LintReview->>GH: check event == 'pull_request'
LintReview->>GH: checkout repository (GITHUB_TOKEN)
LintReview->>Skillsaw: run review action (uses pull-requests: write)
Skillsaw->>GH: create PR review comments
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested Labels
Suggested Reviewers
🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/lint-plugins.yml (1)
4-20:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCritical:
pull_request_targetwith checkout of untrusted PR code enables arbitrary code execution.This workflow has an exploitable vulnerability. The
pull_request_targettrigger grants the job a write-capableGITHUB_TOKEN, and the workflow checks out untrusted code fromgithub.event.pull_request.head.sha. More critically, skillsaw's custom rule in.skillsaw-custom.pyactively executes arbitrary Python scripts from the checked-out repository (lines 54–60 and 74–80 inPluginsDocUpToDateRule.check()):subprocess.run(["python3", str(script_path)], ...) # Runs scripts/generate_plugin_docs.py from PR subprocess.run(["python3", str(website_script_path)], ...) # Runs scripts/build-website.py from PRAn attacker can submit a PR with a malicious
scripts/generate_plugin_docs.pyfile. When the workflow runs, skillsaw will execute it with write access to the repository and secrets—enabling repository compromise.Required fix: Either (1) remove
pull_request_targetand usepull_requestinstead, or (2) implement the two-workflow pattern: an unprivilegedpull_requestworkflow that runs the linter, and a separateworkflow_run-triggered workflow that posts comments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lint-plugins.yml around lines 4 - 20, The workflow uses the dangerous pull_request_target trigger and checks out PR HEAD via actions/checkout with ref: ${{ github.event.pull_request.head.sha || github.sha }}, which combined with .skillsaw-custom.py's PluginsDocUpToDateRule.check() that calls subprocess.run(["python3", str(script_path)], ...) and subprocess.run(["python3", str(website_script_path)], ...) allows execution of untrusted PR code with elevated token permissions; fix by replacing pull_request_target with pull_request (or switch to the two-workflow pattern: an unprivileged pull_request job to run the linter and a separate workflow_run-triggered job to post comments), and ensure the checkout step does not fetch PR HEAD code with elevated permissions (use default checkout behavior for pull_request or fetch only safe refs); update the workflow triggers and the actions/checkout usage accordingly so PluginsDocUpToDateRule.check() runs only on code from the trusted repo context or on the unprivileged runner.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/lint-plugins.yml:
- Around line 4-20: The workflow uses the dangerous pull_request_target trigger
and checks out PR HEAD via actions/checkout with ref: ${{
github.event.pull_request.head.sha || github.sha }}, which combined with
.skillsaw-custom.py's PluginsDocUpToDateRule.check() that calls
subprocess.run(["python3", str(script_path)], ...) and
subprocess.run(["python3", str(website_script_path)], ...) allows execution of
untrusted PR code with elevated token permissions; fix by replacing
pull_request_target with pull_request (or switch to the two-workflow pattern: an
unprivileged pull_request job to run the linter and a separate
workflow_run-triggered job to post comments), and ensure the checkout step does
not fetch PR HEAD code with elevated permissions (use default checkout behavior
for pull_request or fetch only safe refs); update the workflow triggers and the
actions/checkout usage accordingly so PluginsDocUpToDateRule.check() runs only
on code from the trusted repo context or on the unprivileged runner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: af79568e-1cf5-4bc9-9ba9-7da1a7754542
📒 Files selected for processing (1)
.github/workflows/lint-plugins.yml
Revert pull_request_target — it's unsafe because .skillsaw-custom.py runs subprocess on scripts from the checked-out code, which would execute with a write-capable token on fork PRs. Instead, keep pull_request (read-only token) for linting. The skillsaw action uploads the report as an artifact. A new lint-review.yml workflow triggers on workflow_run completion and uses the skillsaw review action to post PR comments with a privileged token, without ever checking out untrusted code. Also removes pull-requests: write from the lint job since it no longer posts comments directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lint-review.yml:
- Line 10: The workflow's if guard only checks github.event.workflow_run.event
and should also require the upstream run succeeded; update the conditional on
the job to include github.event.workflow_run.conclusion == 'success' (e.g.,
replace or extend the existing if expression that references
github.event.workflow_run.event to also check
github.event.workflow_run.conclusion == 'success') so the review job only runs
for successful upstream lint runs.
- Around line 12-20: The permissions block currently only grants pull-requests:
write which removes other scopes causing actions/checkout and artifact retrieval
to fail; either remove the unnecessary checkout step (the actions/checkout
usage) if the review action doesn’t need the repository files, or update the
permissions to include contents: read and actions: read so actions/checkout and
stbenjam/skillsaw/review can operate; modify the permissions entry (not the
step) and/or delete the Checkout code step accordingly to restore required
scopes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bc01e6cd-138c-4f6c-97d6-5bd842e32c94
📒 Files selected for processing (2)
.github/workflows/lint-plugins.yml.github/workflows/lint-review.yml
💤 Files with no reviewable changes (1)
- .github/workflows/lint-plugins.yml
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
|
/lgtm |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cblecker, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pull_requesttrigger (read-only token, safe for fork PRs that may run custom linter rules via subprocess)lint-review.ymlworkflow that triggers onworkflow_runcompletion and uses skillsaw'sreview/action to post PR comments with a write-capable tokenpull-requests: writefrom the lint job since it no longer posts comments directlyWhy not
pull_request_target?.skillsaw-custom.pyrunssubprocess.run(["python3", ...])on scripts from the checked-out repo. Withpull_request_target, a malicious fork PR could replace those scripts and get arbitrary code execution with a write token.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit