ci(docs-preview): Acquire PR context via gh CLI#4267
Conversation
|
TL;DR: Prefer Solution B (which this PR adapts to). This is mostly for my benefit to document all of this research in one place along with full workflow configs to compare. It might also be a helpful resource to others weighing up which approach to implement on their projects. These were all initially adapted from a community discussion started in 2021. I've since posted an answer there with a summary of the 3 solutions detailed below. Reference - Solutions ComparisonTo focus more on the comparing differences:
Each solution highlights some bullet points of differences. Beyond that they are effectively the same in functionality. Should further context be needed, solutions B and C both link to a PR each which provides full commentary in the PR files source. UPDATE: Solution C has been deemed high risk. I will keep it documented here, but heavily discourage it given the wider attack surface when the PR author can run untrusted code vs the reduced risk of
Solution A -
|
Thanks a lot for this!! ⭐ Really helpful Just a small correction: there are two Solution B , I can't |
Glad to hear that 🎉
Whoops! Thanks for pointing that out, I've corrected it 😅 |
| # Required by `set-pr-context`: | ||
| contents: read | ||
| # Required by `marocchino/sticky-pull-request-comment` (write) + `set-pr-context` (read): | ||
| pull-requests: write |
There was a problem hiding this comment.
Better to move the write permissions to the job that needs them (deploy-preview)
| && github.event.workflow_run.event == 'pull_request' | ||
| && contains(github.event.workflow_run.pull_requests.*.head.sha, github.event.workflow_run.head_sha) | ||
| PR_HEADSHA: ${{ steps.set-pr-context.outputs.head-sha }} | ||
| PR_NUMBER: ${{ steps.set-pr-context.outputs.number }} |
There was a problem hiding this comment.
| PR_NUMBER: ${{ steps.set-pr-context.outputs.number }} | |
| PR_NUMBER: ${{ steps.set-pr-context.outputs.number }} |
Description
A
pull_request+workflow_runsolution that should work well with PRs from forks.Adapted from Solution 4 description. Related solutions overview detailed here.My preference is still to
pull_request_target+workflow_call,but that is awaiting confirmation that it was implemented securely(EDIT: It is not secure, unless the untrusted code is only executed in an environment like a container).UPDATE: Due to review feedback of #4264 (Solution C), while I do prefer that approach I am not comfortable moving forward with it for the project and will favor this PR (Solution B).
Type of change
Checklist
CHANGELOG.md