[CI] Run Pronto on GitHub Actions. #8429
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm kinda tired of maintaining our own webhook receiver and pronto runner on the project server. :) Also, sorry, another long essay.
This is surprisingly hard to get right, because GitHub Action's permission system is nearly where I want it to be for this. The default
on: pull_request
trigger works fine for team-internal PRs, but the access token it gets is always read-only for everything if the PR is coming from an external contributor. So this won't fly, especially since Pronto hasn't a good internal fallback to "just silently pass". If we run Pronto with thegithub_status
andgithub_pr
formatters, then it wants to leave an approving review and set the status to green. If it can't, it explodes.A possible workaround to this would be to simply run Pronto as you'd run on your local setup, and have it dump the linting errors/warnings to the log and fail with a non-zero status code. This wouldn't require any access token, and just like our test suite, the status would just turn red. However, this means that if the linting fails, you'd have to look into the job log to figure out why. Both me and @SuperTux88 agreed in a side-channel chat that we kinda really like the way that Pronto leaves inline-comments.
A possible solution is to run the action
on: pull_request_target
. What this does is a bit complicated, but essentially, this runs the job for a PR as if it was triggered from within the repo or from a team member. The good thing is that this allows us to run jobs for an external contributor's PR with elevated permissions, but the bad thing is that this allows us to run jobs for an external contributor's PR with elevated permissions. If you're not careful, a malicious contributor could open a PR that changesbin/bundle
, for example, with a script that pushes the access token to a malicious webservice and uses that token to do evil stuff or something. In theory, the scope of this security issue is limited as the token is supposed to expire when the job ends, but in practice, this solves absolutely nothing (as you could just do malicious stuff within your custom patch, or just block the job for a while - timeouts are large).To avoid running unchecked third-party code with a GitHub Token with write access in scope, this job implementation does a few things. I'm not going to explain them here, the steps this is taking are documented in the workflow definition. Additionally, this repo is set to not run Actions for first-time contributors without approval. This isn't much, but it helps a bit.
I'm not sure if there's a better way to achieve this.
To review this, you have essentially two options:
I'm happy to PR the same change to the Federation repo, but I'll hold off on that until this one is merged, just in case there are revisions! Please take a look, @jhass and @SuperTux88 - both of you, just because more eyes are more better.