[ci] Added automated backport workflow#208
Conversation
|
Warning Rate limit exceeded
⌛ 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 (1)
📝 WalkthroughWalkthroughA new GitHub Actions workflow file is added to enable automatic backporting of changes. The workflow supports two trigger mechanisms: automated backporting on push events to main/master branches, and manual backporting via issue comments on merged pull requests. Both trigger paths delegate execution to a reusable backport workflow, passing relevant identifiers (commit SHA or PR number) and bot authentication credentials. The workflow is configured with concurrency grouping and specifies required permissions for repository contents and pull request management. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/backport.yml:
- Line 22: Replace the mutable branch pointer in the reusable workflow reference
"uses: openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@master"
with a specific commit SHA (e.g. @<commit-sha>) to pin the invocation to an
immutable revision; fetch the desired commit SHA from the
openwisp/openwisp-utils repository (the commit that matches the workflow version
you want) and update both occurrences of that uses: line (the one shown and the
other at lines referenced in the review) so the workflow calls the exact commit
instead of `@master`.
- Line 34: Remove the redundant guard expression "github.event.issue.state ==
'closed'" from the workflow condition because it is implied by the existing
check "github.event.issue.pull_request.merged_at != null"; update the
conditional that currently contains both expressions so it only checks
"github.event.issue.pull_request.merged_at != null" (leave the rest of the
surrounding condition unchanged).
- Around line 12-13: The current workflow-level concurrency key "backport-${{
github.workflow }}-${{ github.ref }}" serializes unrelated backport runs; move
the concurrency block from top-level into each job and replace the key with a
run- or event-scoped identifier (e.g. use "backport-${{ github.workflow }}-${{
github.run_id }}" for unique runs, or "backport-${{ github.workflow }}-issue-${{
github.event.issue.number }}" / "backport-${{ github.workflow }}-pr-${{
github.event.pull_request.number }}" for comment/PR triggers) and keep
"cancel-in-progress: false" or adjust as needed so only identical triggered
entities serialize instead of all runs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/backport.yml
📜 Review details
⏰ 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). (11)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
🔇 Additional comments (1)
.github/workflows/backport.yml (1)
40-40: The security concern is adequately addressed in the reusable workflow.The
reusable-backport.ymlin openwisp-utils already follows the recommended best practice: it storescomment_bodyin an environment variable (COMMENT_BODY: ${{ inputs.comment_body }}) rather than directly interpolating it. The input is safely piped tosedwith proper quoting ("$COMMENT_BODY"), preventing shell injection.Additionally, the extracted branch name is validated using
git check-ref-format --branch, which enforces Git's branch naming rules and prevents crafted payloads from reaching shell or git commands. A secondary check (git ls-remote) ensures the branch exists before use.While the guard conditions on line 35-36 of backport.yml (MEMBER/OWNER gate and
/backportprefix check) provide initial filtering, the reusable workflow's implementation provides defense-in-depth validation that makes the injection concern non-exploitable.
| group: backport-${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
Concurrency key serializes all backport jobs regardless of PR — cross-event collision too.
For both push and issue_comment events targeting master/main, github.ref resolves to the same default-branch ref (e.g. refs/heads/master). This means every workflow run — whether triggered by a push or a PR comment — lands in the identical concurrency group. With cancel-in-progress: false all concurrent backport runs queue up sequentially, including backport commands on different, unrelated PRs.
The fix is to move concurrency to each job and use a key that naturally scopes to the triggering entity:
⚙️ Proposed fix: job-level concurrency with distinct keys
-concurrency:
- group: backport-${{ github.workflow }}-${{ github.ref }}
- cancel-in-progress: false
-
jobs:
backport-on-push:
+ concurrency:
+ group: backport-push-${{ github.sha }}
+ cancel-in-progress: false
if: github.event_name == 'push'
uses: openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@master
...
backport-on-comment:
+ concurrency:
+ group: backport-comment-${{ github.event.issue.number }}
+ cancel-in-progress: false
if: >
...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/backport.yml around lines 12 - 13, The current
workflow-level concurrency key "backport-${{ github.workflow }}-${{ github.ref
}}" serializes unrelated backport runs; move the concurrency block from
top-level into each job and replace the key with a run- or event-scoped
identifier (e.g. use "backport-${{ github.workflow }}-${{ github.run_id }}" for
unique runs, or "backport-${{ github.workflow }}-issue-${{
github.event.issue.number }}" / "backport-${{ github.workflow }}-pr-${{
github.event.pull_request.number }}" for comment/PR triggers) and keep
"cancel-in-progress: false" or adjust as needed so only identical triggered
entities serialize instead of all runs.
| jobs: | ||
| backport-on-push: | ||
| if: github.event_name == 'push' | ||
| uses: openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@master |
There was a problem hiding this comment.
Pin the reusable workflow to a commit SHA, not @master.
Both job references use a mutable branch pointer. Any push to the master branch of openwisp/openwisp-utils — including a compromised commit — takes effect immediately on all consuming repos without any review gate. Note that there is risk to this approach even if you trust the author, because a tag can be moved or deleted if a bad actor gains access to the repository storing the action.
Pin each call to a specific commit SHA and track the SHA deliberately:
🔒 Proposed fix: pin to a commit SHA
- uses: openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@master
+ uses: openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@<COMMIT_SHA> # renovate: tag=masterAlso applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/backport.yml at line 22, Replace the mutable branch
pointer in the reusable workflow reference "uses:
openwisp/openwisp-utils/.github/workflows/reusable-backport.yml@master" with a
specific commit SHA (e.g. @<commit-sha>) to pin the invocation to an immutable
revision; fetch the desired commit SHA from the openwisp/openwisp-utils
repository (the commit that matches the workflow version you want) and update
both occurrences of that uses: line (the one shown and the other at lines
referenced in the review) so the workflow calls the exact commit instead of
`@master`.
| github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
| github.event.issue.pull_request.merged_at != null && | ||
| github.event.issue.state == 'closed' && |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant state == 'closed' guard — remove it.
github.event.issue.state == 'closed' is fully implied by github.event.issue.pull_request.merged_at != null on line 33: a PR can only have a non-null merged_at when it is closed.
♻️ Proposed cleanup
if: >
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
github.event.issue.pull_request.merged_at != null &&
- github.event.issue.state == 'closed' &&
contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association) &&
startsWith(github.event.comment.body, '/backport')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| github.event.issue.state == 'closed' && | |
| if: > | |
| github.event_name == 'issue_comment' && | |
| github.event.issue.pull_request && | |
| github.event.issue.pull_request.merged_at != null && | |
| contains(fromJSON('["MEMBER", "OWNER"]'), github.event.comment.author_association) && | |
| startsWith(github.event.comment.body, '/backport') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/backport.yml at line 34, Remove the redundant guard
expression "github.event.issue.state == 'closed'" from the workflow condition
because it is implied by the existing check
"github.event.issue.pull_request.merged_at != null"; update the conditional that
currently contains both expressions so it only checks
"github.event.issue.pull_request.merged_at != null" (leave the rest of the
surrounding condition unchanged).
3 participants
Checklist
Reference to Existing Issue
openwisp/openwisp-utils#501