fix(ci): do not run rpcimportable on forks #3089
Conversation
📝 WalkthroughWalkthroughThe pull request introduces modifications to the GitHub Actions workflows in two 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/reusable-e2e.yml (1)
43-43: LGTM! Security-conscious condition for Docker Hub authentication.The condition correctly restricts Docker Hub access to trusted contexts (direct pushes or PRs from the main repository), which is a security best practice for CI/CD workflows.
Consider adding a comment to improve readability:
- if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'zeta-chain/node' + # Allow Docker Hub login only for direct pushes or PRs from the main repository + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'zeta-chain/node'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/reusable-e2e.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/ci.yml (1)
101-101: LGTM! The conditional execution is properly secured.
The condition github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'zeta-chain/node' correctly prevents running the rpcimportable job on fork PRs while still allowing it to run:
- On direct pushes/merges to the main repository
- On PRs from the main repository
- In the merge queue
This is a security-conscious approach as it prevents potentially unsafe executions from forks while maintaining the validation where needed.
Let's verify the condition works as expected:
✅ Verification successful
Conditional execution for rpcimportable job is correctly configured
The verification confirms:
- The
rpcimportablejob appears only inci.yml, ensuring consistent fork handling across workflows - No conflicting conditions interfere with the job's execution logic
- The condition at line 101 is the primary control for this job's execution
The implementation aligns with security best practices by restricting fork PR executions while maintaining the required CI/CD flow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the workflow triggers correctly in different scenarios
# Test: Check if this job runs in other workflow files
# Expect: This should be the only occurrence to ensure consistent fork handling
rg -g '*.yml' -g '*.yaml' 'rpcimportable' .github/workflows/
# Test: Verify no other conditions might interfere with this logic
# Expect: No conflicting conditions for this job
ast-grep --pattern 'if: $_' .github/workflows/ci.yml
Length of output: 863
.github/workflows/reusable-e2e.yml (1)
43-43: Verify consistent security conditions across workflows.
Let's ensure this security pattern is consistently applied across all workflow files that interact with Docker Hub.
✅ Verification successful
Security condition is consistently applied across Docker Hub authentications
The security condition github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'zeta-chain/node' is correctly and consistently applied to Docker Hub login actions. The only Docker Hub authentication in the codebase is in the reusable workflow file, and it properly implements this security check to prevent unauthorized access from fork pull requests.
The GitHub Container Registry login doesn't require this condition as it uses GITHUB_TOKEN which is automatically scoped to the correct repository context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Docker Hub login actions and their conditions across all workflow files
echo "Searching for Docker Hub login actions in workflow files..."
fd -e yml -e yaml . .github/workflows/ --exec rg -A 5 'docker/login-action'
Length of output: 667
Script:
#!/bin/bash
# Get the full context of workflow files containing Docker login actions
echo "Full context of workflow files with Docker login:"
fd -e yml -e yaml . .github/workflows/ --exec rg -C 20 'docker/login-action'
# Also search for any other Docker-related authentication steps
echo -e "\nSearching for other Docker-related authentication patterns:"
fd -e yml -e yaml . .github/workflows/ --exec rg -C 10 'docker.*login|docker.*auth|registry.*login|registry.*auth'
Length of output: 2838
Script:
#!/bin/bash
# List all workflow files to ensure we haven't missed any
echo "All workflow files in the repository:"
fd -e yml -e yaml . .github/workflows/
# Get the content of each workflow file that might contain Docker-related steps
echo -e "\nContent of workflow files:"
fd -e yml -e yaml . .github/workflows/ --exec cat {}
Length of output: 52784
Try #3087 again
Also simplify e2e expression.
Making this PR from my fork this time to actually test it.
Summary by CodeRabbit
New Features
rpcimportablejob in the CI workflow, allowing it to run under broader conditions.Bug Fixes