-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: fix permissions for GITHUB_TOKEN on dependabot workflows #22547
Conversation
📝 WalkthroughWalkthroughThe pull request introduces modifications to two GitHub workflow files: Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
4983426
to
a704ad0
Compare
a704ad0
to
99fdac8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
.github/workflows/dependabot-update-all.yml (2)
Line range hint
22-24
: Fix incorrect Go version specification.The specified Go version
1.23
does not exist (latest stable is 1.21.x). This will cause the workflow to fail.Apply this change:
with: - go-version: "1.23" + go-version: "1.21" check-latest: true
Line range hint
28-34
: Add error handling for dependency extraction.The current extraction logic assumes a specific PR title format and might fail silently if the format differs. Consider adding error checking.
Add error handling:
run: | + # Validate PR title format + if ! echo "$PR_TITLE" | grep -q "^build(deps): Bump .* from .* to .*$"; then + echo "Error: Unexpected PR title format" + exit 1 + fi echo "name=$(echo "$PR_TITLE" | cut -d ' ' -f 3)" >> $GITHUB_OUTPUT echo "version=$(echo "$PR_TITLE" | cut -d ' ' -f 7)" >> $GITHUB_OUTPUT.github/workflows/test.yml (2)
Line range hint
271-274
: Standardize Go versions across jobsThere's inconsistency in Go versions across different test jobs:
- Most jobs use Go 1.23
test-depinject
uses Go 1.21test-errors
andtest-math
use Go 1.20test-schema
uses Go 1.12This version inconsistency could lead to compatibility issues or inconsistent test results.
Consider standardizing to Go 1.23 across all jobs unless there's a specific requirement for older versions.
Also applies to: 363-366, 435-438
Line range hint
156-170
: Consider enhancing coverage reporting strategyThe current implementation only generates coverage reports when changes are detected (
if: env.GIT_DIFF
). While this is efficient, it might miss reporting coverage for unchanged but dependent code.Consider one of these approaches:
- Always generate coverage reports but only upload on changes
- Implement dependency tracking to detect indirect impacts
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/dependabot-update-all.yml
(1 hunks).github/workflows/test.yml
(1 hunks)
🔇 Additional comments (3)
.github/workflows/dependabot-update-all.yml (1)
5-6
: LGTM! Permission changes align with workflow requirements.
The addition of contents: write
permission is necessary for this workflow to modify repository contents when updating dependencies. This change:
- Enables the workflow to commit dependency updates
- Maintains the principle of least privilege by only granting required permissions
- Aligns with similar permission updates in other workflow files
Let's verify the permission consistency across workflow files:
✅ Verification successful
Permission changes are consistent with similar workflows
The verification shows that the permissions are correctly set:
test.yml
has identical permissions (contents: write
andpull-requests: write
)- Other workflows have appropriate permissions based on their specific needs:
- Release workflows use
contents: write
for creating releases - PR-related workflows use
pull-requests: write
for PR interactions - Most read-only operations use
contents: read
- Some jobs explicitly use
contents: none
for maximum security
- Release workflows use
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check permissions in other workflow files
# Expected: Similar write permissions for contents and pull-requests
# Search for permission blocks in workflow files
rg -A 5 "permissions:" .github/workflows/
Length of output: 12577
.github/workflows/test.yml (2)
Line range hint 18-38
: Well-structured test parallelization setup
The implementation of test splitting and parallel execution is well-designed:
- Splits tests into 4 parts for parallel execution
- Uses artifacts for sharing test files between jobs
- Properly handles file uploads for each part
10-11
: Verify the necessity of elevated permissions
The changes grant write permissions for both repository contents and pull requests. While this aligns with the PR objective to fix GITHUB_TOKEN permissions, we should ensure these elevated permissions are strictly necessary.
Let's analyze where these permissions are used:
✅ Verification successful
Write permissions are necessary for artifact uploads and SonarCloud integration
The elevated permissions (contents: write
and pull-requests: write
) are required for the workflow to function properly. This is justified by:
- Multiple artifact upload operations throughout the workflow for test results and coverage reports
- SonarCloud integration that needs to write analysis results back to the PR
- Coverage report aggregation across multiple test jobs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usage of permissions in the workflow
# Check for actions that typically require write permissions
rg -A 5 "uses: actions/(upload|create|update)-" .github/workflows/test.yml
# Check for operations that modify PR/repository
rg -A 5 "(git commit|git push|gh pr)" .github/workflows/test.yml
Length of output: 1233
Script:
#!/bin/bash
# Let's check the entire workflow file to understand the complete context
cat .github/workflows/test.yml
# Also check for any codecov or coverage-related actions that might need PR write permissions
rg "uses: codecov/" .github/workflows/test.yml
Length of output: 50154
Script:
#!/bin/bash
# Let's check for any SonarCloud or coverage-related steps that might need write permissions
rg -A 5 "sonarcloud|coverage" .github/workflows/test.yml
# Also check for any artifact upload/download operations
rg -A 5 "(upload-artifact|download-artifact)" .github/workflows/test.yml
Length of output: 22118
* main: build(deps): Bump cosmossdk.io/math from 1.3.0 to 1.4.0 (#22580) fix(server/v2/api/telemetry): enable global metrics (#22571) refactor(server/v2/cometbft): add `codec.Codec` and clean-up APIs (#22566) feat(core/coretesting): make memDB satisfy db.Db interface (#22570) Merge commit from fork fix(server(/v2)): fix fallback genesis path (#22564) fix: only overwrite context chainID when necessary (#22568) docs(client): Update setFeeGranter and setFeePayer comments (#22526) fix(baseapp): populate header info in `NewUncachedContext` (#22557) build(deps): Bump buf.build/gen/go/cometbft/cometbft/protocolbuffers/go from 1.35.1-20240701160653-fedbb9acfd2f.1 to 1.35.2-20240701160653-fedbb9acfd2f.1 in /api (#22551) build(deps): Bump github.com/creachadair/atomicfile from 0.3.5 to 0.3.6 in /tools/confix (#22552) docs: Update reference of Approximation (#22550) fix(server/v2/comebft): wire missing services + fix simulation (#21964) ci: fix permissions for GITHUB_TOKEN on dependabot workflows (#22547) ci: fix permissions for GITHUB_TOKEN in spell check workflow (#22545) build(deps): Bump google.golang.org/protobuf from 1.35.1 to 1.35.2 (#22537) fix(cosmovisor): premature upgrade on restart (#22528) fix(store/v2/pebble): handle version 0 in keys (#22524) refactor(server/v2/telemetry): swap redirects (#22520) docs: Update content in CODE_OF_CONDUCT.md (#22518)
Description
Closes: Try to fix github actions not triggered anymore by automation in dependabot related workflows.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Improvements
Chores