-
Notifications
You must be signed in to change notification settings - Fork 132
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
Improve security of GitHub Actions workflows #2510
Conversation
add stuff
This reverts commit 936dadc.
Test ci.yml: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2510 +/- ##
=======================================
Coverage 51.00% 51.00%
=======================================
Files 124 124
Lines 5384 5384
Branches 1162 1162
=======================================
Hits 2746 2746
Misses 2348 2348
Partials 290 290 ☔ View full report in Codecov by Sentry. |
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.
Thanks for the work @KevinEyo1! This is pretty awesome - thank you for doing the research and implementing this.
Just one small nit and I think we should be good to go.
Co-authored-by: Chan Yu Cheng <[email protected]>
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.
LGTM!
I amended the PR message slightly - I think "Make updates for security" is super vague", so I added that mainly what was done was restrict permissions to read. Can the merger amend it if there are other details I missed?
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.
LGTM!
What is the purpose of this pull request?
Overview of changes:
Fixes #2488
Refactor code and improve security of workflows based on research on security best practices
Anything you'd like to highlight/discuss:
Removed explicit stating of
GITHUB_TOKEN
inci.yml
, not sure if there is a need for it as there is no documentation of why it was added.Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
GitHub Actions: improve security
Security best practices need to be enforced to ensure no avenues
of attack and security breaches.
Let's update the workflows following security best practices,
particularly restricting permissions to read permissions
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major
,r.Minor
,r.Patch
.Breaking change release note preparation (if applicable):