Dependabot-core: Feature-request: PGP signed commits and verified merges from Dependabot

Created on 3 Jun 2019  ·  21Comments  ·  Source: dependabot/dependabot-core

I work in a team using dependabot where all team members are advised to sign their commits with PGP keys. This is to make sure (to some degree) that everything done is signed and things altered by people or bots with bad intentions stick out.

Dependabot however, doesn't sign its commits, nor are the squashed commits verified Github merges. Would it be possible to make this possible?

example commits log with Dependabot on a PGP enforced team:
Screenshot 2019-06-03 at 10 01 39

Thanks!

pull-requests P1 feature-request

Most helpful comment

I believe they are now!

All 21 comments

Hmmm, Dependabot does sign its commits - where are you seeing that they're not? And are you finding that squash merges are signed for other users? I thought it was the case that squash merges created in the GitHub UI are never signed (which is an issue unrelated to Dependabot).

@HendrikPetertje quick chase on the above.

Also encountering this on our repo https://github.com/reactiveui/Pharmacist/commits/master

@glennawatson looks like you're also using squash and merge. Is the signature showing up as verified for any pull requests you merge through the GitHub UI in that way (non-Dependabot ones)?

Yes, good examples are in https://github.com/reactiveui/splat/commits/master -- where the last 3 commits are squash and commit and marked as verified.

OK, thanks. I'm going to pass this on to @laserlemon who knows more about GitHub's automated bot commit signing than I do.

Thanks for looking into it :)

👋 I've reproduced the issue and found that the squash/merge commit lacks verification whenever the _merging user_ of a pull request is different than the _opening user_ for that pull request. Of course, this is always the case for Dependabot pull requests that are merged by a human.

I'll dig into the reasoning behind this behavior. Thank you for raising the issue!

Here's what I've learned… GitHub's ✨ _magic_ ✨ commit signing takes advantage of Git's distinction between a commit's "author" and its "committer." When somebody uses the "squash and merge" button, there are a couple of possibilities for how those two bits of information are set:

| Condition | Author | Committer |
|:--- |:--- |:--- |
| The pull request opener is also the merger | The pull request opener | GitHub's verification user |
| The pull request merger is somebody else | The pull request opener | The pull request merger |

The problem lies in the fact that we want three bits of attribution:

  • the person who opened the pull request
  • the person who merged the pull request
  • GitHub's verification user

…but we only have two fields in which to store that attribution. The best way forward (in my opinion) to ensure squash/merge commit signing is to choose either the pull request opener or merger to attribute as the author, leaving the committer spot always open for GitHub's verification user. However, I fear that change may be too breaking to actually implement. I'll poke around nonetheless!

Another possibility is to set the pull request opener as the author and the pull request merger as a co-author, leaving the committer spot available for the verification user.

Another possibility is to set the pull request opener as the author and the pull request merger as a co-author, leaving the committer spot available for the verification user.

Ooh, that sounds like it would make a tonne of sense! Thanks for your work digging into this @laserlemon!

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within seven days. Thank you for your contributions.

Is this work being covered elsewhere @laserlemon?

Ran into this issue, our repository requires that all members sign commits and without that only admins can merge. Anything the community can do to help move this forward?

There's been some new work on squash commits and their co-authorship. The work is in this pull request and it's described in this Changelog entry.

This doesn't solve the problem… yet.

The next possible step, if we have the will to make this breaking change, would be to stop using the pull request squasher/merger as the commit's _committer_. Instead, we could drop that user's attribution entirely (since they didn't really _write_ any of it necessarily), or add them as a co-author. I have a slight preference for dropping attribution for the person who pushes the merge button. It's not always indicative of who did the review work anyway.

So my proposed behavior is:

  • Alice opens a pull request. She didn't write a single commit in the pull request.
  • Bob wrote commits present in the pull request.
  • Charlie is a co-author on one of Bob's commits.
  • Diane is yet another commit author for the pull request, just like Bob.
  • Emily squashes/merges the pull request.

And the result would be:

  • The squash commit is authored by Alice, with three co-authors: Bob, Charlie, and Diane. Emily is credited nowhere. GitHub's ✨magic✨ verification user is the squash commit's _committer_ so that it can be PGP signed and verified.

Another option exists where we add support for a trailer like Co-authored-by, except Merged-by.

I'm 👍 on the user who merges not being credited as the committer in a squash and merge scenario. Doesn't feel inconsistent with other merge case (because the merge commit committer is already GitHub's verification user). In fact, it feels kind of inconsistent that they are at the moment.

I could go either way on the co-author - I essentially don't have strong feelings on it. My hunch is to therefore exclude it, and add it if/when we have community feedback, either as Co-authored-by or Merged-by.

Has there been an issue within the past couple of days where Dependabot is not signing commits? We have our repos set to require signing, and have been using Dependabot successfully like this (as all of the commits are signed.) for quite some time. However, in the past couple of days, the commits are coming in unsigned.

Commit merges are still signed for me but squash merges aren't signed when merged.

I believe they are now!

@laserlemon Was this confirmed? I am using "rebase and merge" strategy for dependabot and come across the following behaviour:

  • initial commit in the PR branch is signed (GitHub badge "verified" is there)
  • however, once I have merged the PR, there is no badge anymore:

PR branch:
Screenshot 2020-06-21 01 43 47

Rebased and merged branch:
Screenshot 2020-06-21 01 44 24

Was this page helpful?
0 / 5 - 0 ratings

Related issues

v1sion picture v1sion  ·  3Comments

rebelagentm picture rebelagentm  ·  3Comments

bennycode picture bennycode  ·  3Comments

Spomky picture Spomky  ·  4Comments

tjwallace picture tjwallace  ·  3Comments