Skip to content
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

Support workflow_run triggered by push event workflows #138

Conversation

electrofelix
Copy link
Contributor

Updates to enhance security around github token handling means that PRs
merged to a target branch by github native dependabot no longer receive
a write access token by default. Instead it is required to have a push
workflow trigger a workflow_run subsequently that is provided with write
access.

To support this add a route and handler function for workflow_run events
that reuses much of the functionality from the push handler by moving
the common capabilities to a shared function.

Includes a check to skip such a workflow if it is triggered via
pull_request events instead until such time as someone can add the
required support using the existing pull_request handler for the common
pieces.

Adds tests following the same pattern for the handlePull function.

Fixes: #137

Updates to enhance security around github token handling means that PRs
merged to a target branch by github native dependabot no longer receive
a write access token by default. Instead it is required to have a push
workflow trigger a workflow_run subsequently that is provided with write
access.

To support this add a route and handler function for workflow_run events
that reuses much of the functionality from the push handler by moving
the common capabilities to a shared function.

Includes a check to skip such a workflow if it is triggered via
pull_request events instead until such time as someone can add the
required support using the existing pull_request handler for the common
pieces.

Adds tests following the same pattern for the handlePull function.
@chinthakagodawita chinthakagodawita self-requested a review April 11, 2021 05:56
@chinthakagodawita
Copy link
Owner

Nice one, thanks @electrofelix! It all looks good from a cursory glance, I'll take a proper look/test within the next week and cut a release with this soon

@electrofelix
Copy link
Contributor Author

@chinthakagodawita the tests were very straightforward to follow. I will admit I'm a little unsure as to what happens during a workflow_run triggered by a PR (or even how multiple PRs could appear in the event data though that is documented by GitHub's api) so I've just made the assumption that the head_branch might be empty in that case and that may mean I'm doubling up on checking that it's been triggered via an action running on a push event. I'll try to see what GitHub does in the second case to at least ensure whether that test is needed or if the one further down can just be moved up and have one check.

@electrofelix
Copy link
Contributor Author

Turned out to be easier to check than I thought, https://github.com/electrofelix/workflow_run-test/runs/2316504875?check_suite_focus=true contains debug output of a run. The good news is I can probably simplify the checking of the workflow_run.event as that is either push when notified by another action started by a push event, or pull_request in the case of the debug output from the run I've linked above.

@electrofelix
Copy link
Contributor Author

Hi @chinthakagodawita, is there anything else I can do to help with this change?

@chinthakagodawita
Copy link
Owner

Amazing work @electrofelix - I've tested this and it all looks great. I've made some minor tweaks and also added support for the pull_request event as part of #147.

Released as part of v1.3.0 🎉

@electrofelix electrofelix deleted the support-workflow-run-event-for-push branch April 17, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Security improvements means this action doesn't work if triggered by push from dependabot
2 participants