Add scheduler action - #80
Conversation
sverhoeven
left a comment
There was a problem hiding this comment.
Should work as is.
In docs for editor team (DEV.md?) I would mention this scheduler,
Also mention that PRs from forks can not be scheduled, see https://github.com/gr2m/merge-schedule-action/blob/master/README.md?plain=1#L69
Below are some hardening suggestions
I would pin gr2m/merge-schedule-action action to commit a9b6ddcf1282dfd66907bda8e7941dff59f03bad (v2.7.0) with gr2m/merge-schedule-action@a9b6ddcf1282dfd66907bda8e7941dff59f03bad, to lessen supply chain attacks.
On https://github.com/NLeSC/blog/settings/actions the Approval for running fork pull request workflows from contributors is already set to Require approval for first-time contributors which is good. You can tighten it by setting it to Require approval for all external contributors, On same page you could uncheck Allow GitHub Actions to create and approve pull requests so you do not get recursion (),
At https://github.com/NLeSC/blog/settings/branch_protection_rules/82026112 you could tighten protections by enabling
Dismiss stale pull request approvals when new commits are pushedRestrict who can dismiss pull request reviewsRequire status checks to pass before mergingRequire conversation resolution before merging
|
Dear @sverhoeven, I implemented all the changes you suggested. Thanks for taking a look into this!
|
sverhoeven
left a comment
There was a problem hiding this comment.
Looks good.
I checked settings and you applied all my suggestions, nice to see, hopefully not to burdensome to contributors.
I just have one tiny inline suggestion.
Co-authored-by: Stefan Verhoeven <s.verhoeven@esciencecenter.nl>
|
Ironically, accepting your suggestion automatically dismissed your previous review 😅 |
This automation works. I tried it in this toy example, and it is as elegant as it gets.
What to review then?
Let's think about security for a moment.
Without PR rules, this automation could be vulnerable to last-minute edits getting published in our institutional blog.
This is the scenario I want to avoid:
@sverhoeven, can you please take a look at the rules and double check that they are safe enough?