Skip to content

Rewrite qhelp-pr-preview.yml #6995

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

Merged
merged 16 commits into from
Nov 4, 2021
Merged

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Oct 28, 2021

This pull request makes the qhelp-pr-preview workflow suitable to be run on pull requests from forks. Actions jobs on pull requests from forks are run with limited privileges and as a result the workflow was not allowed to post a comment containing the QHelp previews. To make this work I split the workflow in two, the first generates the query help in markdown format and uploads the result as an artefact. The second workflow uses a workflow_run trigger and takes care of downloading the generated markdown and posting it to the pull request. The first workflow is completely user controlled and runs with read-only permissions. The second workflow has elevated permissions pull-request: write to post the comment. However, this workflow is not user controlled and its inputs (a pull-request number and contents of the comment to be posted) are not used in dangerous ways. In addition the workflow double checks that the commit SHA of the head of the pull request and the one from the triggering workflow run match. This prevents a malicious user from writing spam comments on arbitrary pull requests.

An example run can be found at aibaars#5 (comment)

@aibaars aibaars force-pushed the aibaars/pr-qhelp-check branch 5 times, most recently from 25a7f24 to 6ac05dd Compare October 29, 2021 16:03
@aibaars aibaars force-pushed the aibaars/pr-qhelp-check branch from 6ac05dd to a0903c3 Compare October 29, 2021 16:18
@aibaars aibaars force-pushed the aibaars/pr-qhelp-check branch from a881a9e to 53b0315 Compare November 2, 2021 11:02
@aibaars aibaars marked this pull request as ready for review November 2, 2021 12:00
@aibaars aibaars requested a review from a team as a code owner November 3, 2021 14:14
@github-actions github-actions bot added the Ruby label Nov 3, 2021
@aibaars aibaars force-pushed the aibaars/pr-qhelp-check branch from 56ab35a to b9bf597 Compare November 3, 2021 14:15
Copy link
Collaborator

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good! A bunch of relatively minor suggestions, and one idea for getting the PR number without going through the artifact.

@aibaars aibaars force-pushed the aibaars/pr-qhelp-check branch from b61392d to 332bbf3 Compare November 3, 2021 16:41
adityasharad
adityasharad previously approved these changes Nov 3, 2021
Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com>
@aibaars aibaars merged commit 27bbddf into github:main Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants