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

Use fork method to have auto starting manifest PRs #4090

Merged
merged 1 commit into from Apr 25, 2023

Conversation

jcscottiii
Copy link
Collaborator

Using the mentioned method here: https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#push-pull-request-branches-to-a-fork

Why did we create a new bot instead of using wptfyibot?

  • It currently has write access to the repo. And generating a PAT for it is a security risk in case the third party peter-evans/create-pull-request action is compromised. It could then would have write access to this repository.
    • A future optimization: Once finer grain PATs are out of beta and someone tests it with peter-evans/create-pull-request, we might can consolidate back to using only wptfyibot. More about finer grain PATs can be found here

In a future commit, we will update resolve_merge_conflict.yml to use the v5 of the same action

Fixes: #1444

Using the mentioned method here: https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#push-pull-request-branches-to-a-fork

Why did we create a new bot instead of using wptfyibot?
- It currently has write access to the repo. And generating a PAT for it
  is a security risk in case the third party
  peter-evans/create-pull-request action is compromised. It could then
  would have write access to this repository.
  - A future optimization: Once finer grain PATs are out of beta and
    someone tests it with peter-evans/create-pull-request, we might can
    consolidate back to using only wptfyibot. More about finer grain PATs can
    be found [here](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token#creating-a-fine-grained-personal-access-token)

In a future commit, we will update resolve_merge_conflict.yml to use the
v5 of the same action
@jcscottiii jcscottiii added the do not merge yet Disable auto-merge label Apr 25, 2023
@github-actions github-actions bot merged commit 9b80e8d into master Apr 25, 2023
4 of 5 checks passed
@github-actions github-actions bot deleted the use-new-bot branch April 25, 2023 20:41
@jcscottiii
Copy link
Collaborator Author

Did not mean for this to auto merge. Forgot to label this with do not merge yet

jcscottiii added a commit that referenced this pull request Apr 26, 2023
Moving to the least privileged model for creating the PR in #4090
brought a new problem: The forked PR is unable to be auto approved.

This is because PR runs do not have access to GITHUB_TOKEN which is
needed for the auto approver. GitHub tightened up their security model a
few years ago to prevent this. Details in this
[doc](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)

Also, in the doc shows the recommended implementation which this commit
uses:

- Use the required "Run Tests" workflow as the unprivileged run that
  runs when the PR is made.
- Move the auto-approval to be triggered after the "Run Tests" workflow.
  This is privileged and has access to the GITHUB_TOKEN

Examples using this same way:
- https://github.com/MaibornWolff/codecharta/blob/main/.github/workflows/auto-approve-and-merge.yml

Other changes:
- Migrate to use hmarr/auto-approve-action@v3. Remove the explicit need
  for GITHUB_TOKEN in v3.
- Name the test.yml workflow "Run tests"
jcscottiii added a commit that referenced this pull request Apr 26, 2023
Moving to the least privileged model for creating the PR in #4090
brought a new problem: The forked PR is unable to be auto approved.

This is because PR runs do not have access to GITHUB_TOKEN which is
needed for the auto approver. GitHub tightened up their security model a
few years ago to prevent this. Details in this
[doc](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)

Also, in the doc shows the recommended implementation which this commit
uses:

- Use the required "Run Tests" workflow as the unprivileged run that
  runs when the PR is made.
- Move the auto-approval to be triggered after the "Run Tests" workflow.
  This is privileged and has access to the GITHUB_TOKEN

Examples using this same way:
- https://github.com/MaibornWolff/codecharta/blob/main/.github/workflows/auto-approve-and-merge.yml

Other changes:
- Migrate to use hmarr/auto-approve-action@v3. Remove the explicit need
  for GITHUB_TOKEN in v3.
- Name the test.yml workflow "Run tests"
jcscottiii added a commit that referenced this pull request Apr 26, 2023
Moving to the least privileged model for creating the PR in #4090
brought a new problem: The forked PR is unable to be auto approved.

This is because PR runs do not have access to GITHUB_TOKEN which is
needed for the auto approver. GitHub tightened up their security model a
few years ago to prevent this. Details in this
[doc](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)

Also, in the doc shows the recommended implementation which this commit
uses:

- Use the required "test" workflow as the unprivileged run that
  runs when the PR is made.
- Move the auto-approval to be triggered after the "Run Tests" workflow.
  This is privileged and has access to the GITHUB_TOKEN

Examples using this same way:
- https://github.com/MaibornWolff/codecharta/blob/main/.github/workflows/auto-approve-and-merge.yml

Other changes:
- Migrate to use hmarr/auto-approve-action@v3. Remove the explicit need
  for GITHUB_TOKEN in v3.
github-actions bot pushed a commit that referenced this pull request Apr 27, 2023
Moving to the least privileged model for creating the PR in #4090
brought a new problem: The forked PR is unable to be auto approved.

This is because PR runs do not have access to GITHUB_TOKEN which is
needed for the auto approver. GitHub tightened up their security model a
few years ago to prevent this. Details in this
[doc](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/)

Also, in the doc shows the recommended implementation which this commit
uses:

- Use the required "test" workflow as the unprivileged run that
  runs when the PR is made.
- Move the auto-approval to be triggered after the "Run Tests" workflow.
  This is privileged and has access to the GITHUB_TOKEN

Examples using this same way:
- https://github.com/MaibornWolff/codecharta/blob/main/.github/workflows/auto-approve-and-merge.yml

Other changes:
- Migrate to use hmarr/auto-approve-action@v3. Remove the explicit need
  for GITHUB_TOKEN in v3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge yet Disable auto-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitHub checks not running update_wpt_manifest.yml
1 participant