-
Notifications
You must be signed in to change notification settings - Fork 364
fix(ci): make sure dependabot is secure and able to update everthing #7241
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
Conversation
Overall package sizeSelf size: 4.42 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7241 +/- ##
=======================================
Coverage 86.26% 86.26%
=======================================
Files 513 513
Lines 22067 22067
=======================================
Hits 19035 19035
Misses 3032 3032 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-01-20 09:56:07 Comparing candidate commit 88e1340 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 229 metrics, 31 unstable metrics. |
rochdev
left a comment
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.
Let's get the security team to look at it since we use pull_request_target in combination with actions/checkout.
|
@BridgeAR Can you add a PR description? Reading through the PR it seems like a pretty big and complicated addition and it's unclear why it's needed at all. |
|
@rochdev done |
fc456a4 to
88e1340
Compare
In what way is it safer? With the current setup, any bad actor would be able to just change the workflow so the contents of the individual jobs doesn't really matter, what matters is the Octo STS claims. I'm a bit worried about adding so much untestable shell scripting if there isn't a strong argument for, so let's make that argument clear 😄 |
KSerrania
left a comment
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.
In what way is it safer? With the current setup, any bad actor would be able to just change the workflow so the contents of the individual jobs doesn't really matter, what matters is the Octo STS claims.
The main idea behind separating steps is not to protect against someone already able to write to the repository.- in this case, they do not need to go through such lengths to get contents: write and pull-requests: write access, since they already have it.
The goal is to prevent malicious code that could run as part of the vendoring scripts (e.g. if it ends up being vulnerable to arbitrary code execution) from being able to get access to valuable credentials (write access to the repository).
Giving as few steps as possible sensitive access within a workflow limits this risk, which contributes to improving the security of the repository. However, security is about trade-offs - here, between keeping these scripts maintainable and reducing the access they may provide. If you deem this approach harmful to the maintainability of your repository, we can try to think about alternative ways to secure the repository.
This is safer, since it does not allow the install step to push to the branch.