fix(ci): allow fork PRs to post DB entity comments#2714
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWorkflow trigger changed to ChangesPull Request Target Security Hardening
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
xsahil03x
force-pushed
the
fix/ci-check-db-entities-fork-perms
branch
from
2c872ca to
9462a1f
Compare
xsahil03x
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/check_db_entities.yml:
- Around line 53-54: The workflow uses mutable version tags (e.g.,
actions/checkout@v6, dorny/paths-filter@v3, peter-evans/find-comment@v4,
peter-evans/create-or-update-comment@v5) which must be pinned to immutable
commit SHAs for security when running in pull_request_target; update each uses:
entry to the corresponding full commit SHA (replace the tag after each
identifier with the repo's specific commit hash) so actions/checkout,
dorny/paths-filter, peter-evans/find-comment and
peter-evans/create-or-update-comment are referenced by commit SHAs rather than
version tags.
- Around line 14-16: The concurrency.group currently uses github.workflow and
github.ref which under pull_request_target resolves to the base branch and
causes unrelated PR runs to collide; update the concurrency.group expression
(the key used under concurrency) to include a per-PR identifier such as
github.event.pull_request.number or github.head_ref (e.g., use
github.workflow-${{ github.event.pull_request.number }} or include
github.head_ref) so each PR run has a unique concurrency key when using
pull_request_target.
- Around line 18-21: The workflow currently grants pull-requests: write at the
top-level permissions; restrict that write permission to only the job that needs
it by removing or changing the top-level pull-requests permission and adding
permissions: pull-requests: write under the check_entity_modifications job.
Specifically, keep top-level permissions.contents: read (or set pull-requests:
none), and add a job-level permissions block inside the
check_entity_modifications job that sets pull-requests: write so the gate job
(which runs dorny/paths-filter) does not receive write access.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e724ef79-3f9a-4bbc-9ce3-3ed0dac582dd
📒 Files selected for processing (1)
.github/workflows/check_db_entities.yml
xsahil03x
force-pushed
the
fix/ci-check-db-entities-fork-perms
branch
2 times, most recently
from
1a1d6af to
73a780b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/check_db_entities.yml (1)
68-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't swallow
git difffailures.Line 68 currently treats "no
.dartmatches" and "couldn't compute the diff" the same way because the pipeline ends with|| true. If ref resolution or diffing breaks, this job will incorrectly emithas_changes=falseand skip the warning comment.Suggested change
- git diff --name-only "origin/$BASE_BRANCH...$PR_HEAD_SHA" -- $ENTITY_DIR | grep -E '\.dart$' > $TEMP_FILE || true + ALL_CHANGED_FILE="${TEMP_FILE}.all" + git diff --name-only "origin/$BASE_BRANCH...$PR_HEAD_SHA" -- "$ENTITY_DIR" > "$ALL_CHANGED_FILE" + grep -E '\.dart$' "$ALL_CHANGED_FILE" > "$TEMP_FILE" || true🤖 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/check_db_entities.yml around lines 68 - 74, The pipeline is currently masking failures of the git diff + grep pipeline due to the trailing "|| true"; enable proper failure propagation by turning on pipefail (e.g., set -o pipefail) before running the pipeline and remove the "|| true" so any git diff or grep failure surfaces; keep the existing check that inspects TEMP_FILE (and writes has_changes=false to GITHUB_OUTPUT when it's empty) but ensure that if the git diff pipeline fails you exit with an error instead of treating it as "no changes" — reference the pipeline command that writes to TEMP_FILE (git diff --name-only "origin/$BASE_BRANCH...$PR_HEAD_SHA" -- $ENTITY_DIR | grep -E '\.dart$' > $TEMP_FILE) and the TEMP_FILE / has_changes / GITHUB_OUTPUT logic when making the change.
🤖 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/check_db_entities.yml:
- Around line 14-16: The workflow-level permissions block is missing issues:
write which is required by the peter-evans/create-or-update-comment@v5 action;
update the permissions map in .github/workflows/check_db_entities.yml by adding
issues: write alongside pull-requests: write and contents: read so the
create-or-update-comment step can post comments (refer to the permissions block
and the peter-evans/create-or-update-comment@v5 action name to locate the
change).
---
Outside diff comments:
In @.github/workflows/check_db_entities.yml:
- Around line 68-74: The pipeline is currently masking failures of the git diff
+ grep pipeline due to the trailing "|| true"; enable proper failure propagation
by turning on pipefail (e.g., set -o pipefail) before running the pipeline and
remove the "|| true" so any git diff or grep failure surfaces; keep the existing
check that inspects TEMP_FILE (and writes has_changes=false to GITHUB_OUTPUT
when it's empty) but ensure that if the git diff pipeline fails you exit with an
error instead of treating it as "no changes" — reference the pipeline command
that writes to TEMP_FILE (git diff --name-only
"origin/$BASE_BRANCH...$PR_HEAD_SHA" -- $ENTITY_DIR | grep -E '\.dart$' >
$TEMP_FILE) and the TEMP_FILE / has_changes / GITHUB_OUTPUT logic when making
the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: effa0ddf-d2e4-4954-82ed-1340011e76f6
📒 Files selected for processing (1)
.github/workflows/check_db_entities.yml
xsahil03x
force-pushed
the
fix/ci-check-db-entities-fork-perms
branch
from
73a780b to
4aea8bc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/check_db_entities.yml:
- Around line 63-69: The workflow currently shallow-fetches the PR head with
--depth=1 and silences git diff failures with "|| true", which can hide
merge-base issues; update the fetch and diff steps to fetch sufficient history
for merge-base resolution (ensure both origin/$BASE_BRANCH and the PR head ref
are fetched without --depth=1 or explicitly fetch the merge-base with git fetch
--unshallow) and remove the trailing "|| true" so a failing git diff/grep
returns a non-zero exit code; reference the PR_HEAD_SHA, BASE_BRANCH, ENTITY_DIR
and TEMP_FILE variables when making these changes and keep the git diff command
using the three-dot "origin/$BASE_BRANCH...$PR_HEAD_SHA".
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45def8a3-ba63-4d51-bc4a-e6e48fb44a98
📒 Files selected for processing (1)
.github/workflows/check_db_entities.yml
xsahil03x
force-pushed
the
fix/ci-check-db-entities-fork-perms
branch
4 times, most recently
from
2c97a44 to
c9b9232
Compare
VelikovPetar
approved these changes
Jun 3, 2026
The check_db_entities workflow used `on: pull_request`, which gives GITHUB_TOKEN read-only perms on fork PRs and made peter-evans/create-or-update-comment fail with HTTP 403. Switch to pull_request_target with least-privilege permissions (contents: read, pull-requests: write). Avoid the usual pull_request_target footgun by never checking out the PR head into a working tree: fetch the PR head SHA as a ref and diff between SHAs, so attacker-controlled file contents never reach checkout/smudge filters/hooks while the write token is in scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xsahil03x
force-pushed
the
fix/ci-check-db-entities-fork-perms
branch
from
c9b9232 to
4efdc45
Compare
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
check_db_entitieswas failing on fork PRs (e.g. #2700 run) with:on: pull_requestfrom a fork givesGITHUB_TOKENread-only perms, sopeter-evans/create-or-update-commentcan't POST.Changes
on: pull_request→on: pull_request_targetso the bot gets a write token regardless of PR origin.${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}). Underpull_request_target,github.refis the base branch, so the previous key would have collapsed all PRs targeting the same base into one concurrency group and cancelled each other.permissions:block scoped tocontents: read+pull-requests: write.git diff-index(working tree vs. base) togit diff base...pr_head(between refs). Needed becausepull_request_targetchecks out the base branch by default; using diff-between-refs also means PR-controlled file contents never reach a working tree under the privileged token, so smudge filters / hooks / scripts in the PR can't execute.Security notes
git fetch,git diff --name-only,grep, and two API-only JS actions.${{ steps.check_entities.outputs.modified_files }}is interpolated into the comment body (not a shell step), so attacker-crafted filenames can only produce odd markdown, not RCE.pull-requests: write) — leaked token can only post PR comments.Test plan
pull_request_targetalways uses the base-branch copy of the workflow, so the fix is only verifiable on the next fork PR after merge.)packages/stream_chat_persistence/lib/src/entity/*.dart, the bot posts/updates the "Database Entity Files Modified" comment.🤖 Generated with Claude Code
Summary by CodeRabbit
Note: This release contains only internal infrastructure improvements with no visible changes to end-user functionality.