-
Notifications
You must be signed in to change notification settings - Fork 92
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
fix: workflow fix for fork PRs #199
Conversation
WalkthroughThe recent modifications across various GitHub workflows signify a unified shift in event triggers from Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/ci.yml (1 hunks)
- .github/workflows/jest-coverage.yml (1 hunks)
- .github/workflows/uat.yml (1 hunks)
Additional comments: 3
.github/workflows/ci.yml (1)
- 3-3: Switching the event trigger from
pull_request
topull_request_target
enhances security and functionality for forked PRs by allowing workflows to run in the context of the base repository. This change is crucial for accessing secrets and other repository settings necessary for CI processes. Ensure that all necessary permissions and security implications have been considered to prevent unauthorized access to sensitive information..github/workflows/jest-coverage.yml (1)
- 3-3: The transition to
pull_request_target
for the Jest Coverage Report workflow is consistent with the objective to improve CI processes for forked PRs. This change allows the workflow to access necessary secrets and settings from the base repository, which is essential for generating accurate coverage reports. As with any change involvingpull_request_target
, ensure that security considerations are thoroughly evaluated to prevent potential misuse..github/workflows/uat.yml (1)
- 3-3: Switching the event trigger to
pull_request_target
for the AWS S3 Static Site Deploy workflow aligns with the goal of enhancing CI processes for forked PRs. This adjustment allows the workflow to operate with the necessary permissions and access to secrets within the base repository's context. It's important to ensure that the security implications of this change are fully considered to safeguard against unauthorized access.
name: AWS S3 Static Site Deploy | ||
on: | ||
pull_request: | ||
pull_request_target: | ||
branches: [master] | ||
jobs: | ||
aws_deply: |
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.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]
The use of variable interpolation ${{...}}
with github
context data in a run:
step poses a security risk, as it could potentially allow an attacker to inject their own code into the runner. To mitigate this risk, consider using an intermediate environment variable set in the env:
section of the job or step. This environment variable can then be safely used in the run:
script with proper quoting to prevent code injection.
- name: Build
env:
BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
run: export BRANCH_NAME="$BRANCH_NAME" && yarn build:uat
Ticket Link
Related Links
Description
ci workflow fix for fork PRs
Steps to Reproduce / Test
Checklist
yarn test
passesGIF's
Live Link
Summary by CodeRabbit