-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Security improvements to GitHub Actions #17520
Security improvements to GitHub Actions #17520
Conversation
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Florent Poinsard <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17520 +/- ##
==========================================
+ Coverage 67.69% 67.71% +0.01%
==========================================
Files 1584 1584
Lines 254541 254509 -32
==========================================
+ Hits 172315 172340 +25
+ Misses 82226 82169 -57 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[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.
Thanks, @frouioui !
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
@@ -13,6 +13,7 @@ env: | |||
|
|||
jobs: | |||
build: | |||
timeout-minutes: 60 |
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.
👍🏽
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.
❤️
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]>
Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]>
Description
In this PR the
Assign Milestone
andRelease
GitHub actions were modified into to harden our GitHub Actions usage. The changes brought to these two workflows are made to avoid and limit potential pwn requests and cache poisoning attacks.In addition to these two files, I have bumped the version of
actions/checkout
and made sure to disablepersist-credentials
where it was not needed.Let's backport / do something similar on all release branches.
Assign Milestone
This workflow uses the
pull_request_target
trigger, meaning that the workflow is executed in the context of the PR's base commit, which can quickly become problematic if not handled correctly. I made the workflow a bit tighter by removing thesetup-go
step (which uses caching by default), limiting the permissions, by adding a sanity check on the./go/vt/servenv/version.go
file that we read, and most importantly by not checking out the PR's HEAD but rather the base's HEAD.Release
I have added a verification of
fpm
's checksum and disabled caching for thesetup-go
step. Disabling the cache in that workflow is a SLSA level 3 requirement:This workflow was tested here: https://github.com/frouioui/vitess/actions/runs/12776723633/job/35616102340