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

Update README to use github.actor for validating Dependabot PRs #164

Closed
wants to merge 1 commit into from

Conversation

phillipuniverse
Copy link

@phillipuniverse phillipuniverse commented Feb 26, 2022

Using the user associated to the original PR creation prevents any other user from adding additional commits on top of a Dependabot PR as the action will fail during the fetch-metadata step.

Here's what it looks like for an auto-approve process using github.event.pull_request.user.login == 'dependabot[bot]' after adding a commit on top of a Dependabot commit:

image

Error: PR is not from Dependabot, nothing to do.

In my case this was to fix an incompatibility with the PR that Dependabot proposed originally so I needed to make additional changes to consume the version update.

Using the user associated to the original PR creation prevents any other user from adding additional commits on top of a Dependabot PR as the action will fail during the fetch-metadata step
@phillipuniverse phillipuniverse requested a review from a team as a code owner February 26, 2022 00:19
@phillipuniverse phillipuniverse changed the title Use github.actor for validating Dependabot auto-merge Update README to use github.actor for validating Dependabot PRs Feb 26, 2022
@mwaddell
Copy link
Contributor

@phillipuniverse Sorry for the confusion - the README is still referencing v1.1.1 but the use of the author instead of the actor requires v1.2.1. This confusion should be fixed moving forward with #163, but we'll need to manually update it this time (#165).

If you update your github action to use v1.2.1 instead of v1.1.1 it will work with the pull_request.user.login. If you use v1.1.1, then you need to use actor in your action instead.

@mwaddell
Copy link
Contributor

@phillipuniverse - The README has been updated to reference the correct version. Please let me know if this addresses your issue. Again, sorry for the confusion.

@phillipuniverse
Copy link
Author

@mwaddell got it, thanks for the update! I’ll try it out shortly.

One question - if I update to 1.2.1 for the action would you expect both the actor version and the original author version to work? And I suppose since you changed the behavior it would be better to use the original author version?

@mwaddell
Copy link
Contributor

Yes, if you update to v1.2.1, then both the actor version and author version would work.

fetch-metadata will only run on PRs created by dependabot (since it relies on the metadata present in the original commit messages that dependabot adds).

In v1.1.1 this was being enforced by: (a) confirming that the PR's author was dependabot, (b) that the PR only contains a single verified commit from dependabot, and (c) that the fetch-metadata action was triggered by dependabot. This meant that the user could not manually rerun a failed action that used fetch-metadata or use it in response to other actions besides dependabot's own (such as a manual PR reopened).

In v1.2.1, we removed the third of these checks to allow users to manually rerun a failed action and have more flexibility in how actions using fetch-metadata can be triggered.

@phillipuniverse
Copy link
Author

phillipuniverse commented Feb 26, 2022

@mwaddell got it, this all makes perfect sense. I now see #137 for the additional context here available in 1.2.1.

I just tried this out on my repo and we are all good! You're right both the actor and PR creator version work in 1.2.1, but using the actor version prevents me (or any other user) from re-executing an action on a Dependabot PR that contains the metadata step.

Thanks again for your help in clearing this up!

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.

2 participants