app icon indicating copy to clipboard operation
app copied to clipboard

DCO doesn't respect Verified status by GitHub

Open abitrolly opened this issue 5 years ago • 16 comments

Bug Report

Current Behavior

When editing docs from GitHub, the DCO creates unnecessary hassle by the need to checkout the PR and amend it with -s option. But GitHub already checked such commits and added Verified label to them.

image

https://github.com/buildpacks/docs/commit/4b6354c0ee94e8731f4a329b3ec6cc4b3dcfeda0

DCO fails to validate them.

image

Expected behavior/code

DCO successfully detects verified commits and passes checks.

abitrolly avatar Nov 09 '20 06:11 abitrolly

Thanks for opening this issue. A contributor should be by to give feedback soon. In the meantime, please check out the contributing guidelines and explore other ways you can get involved.

welcome[bot] avatar Nov 09 '20 06:11 welcome[bot]

this seems like a reasonable request, but this repository is for the probot framework. there is a dedicated repository for DCO. it would be better to file this issue there

travi avatar Nov 09 '20 07:11 travi

It is possible to move the issue but I have no perms.

abitrolly avatar Nov 09 '20 08:11 abitrolly

It is possible to move the issue but I have no perms.

Done 👍🏼

gr2m avatar Nov 09 '20 16:11 gr2m

I agree it's a reasonable request, happy to accept a PR with tests

gr2m avatar Nov 09 '20 16:11 gr2m

The 'verified' indicator on github ensures the commit was signed with a GPG key. Signing your commit with a GPG key is not equivalent in most circles to agreeing to the DCO (in fact there was once a Probot app to enforce that alone! https://github.com/jarrodldavis/probot-gpg).

So please make sure the PR by default does not include this functionality and is opt-in only (by way of a config option in the .github/dco.yml)

GitHub
A GitHub App that enforces GPG signatures on pull requests (no longer maintained) - jarrodldavis/probot-gpg

hiimbex avatar Nov 09 '20 16:11 hiimbex

Good to know thank you Bex!

gr2m avatar Nov 09 '20 16:11 gr2m

Signing your commit with a GPG key is not equivalent in most circles to agreeing to the DCO.

Not sure how's that different from signing it with -s key. Those "most circles" can hardly serve as a reference.

abitrolly avatar Nov 09 '20 17:11 abitrolly

@abitrolly note that @hiimbex built this app so they know a lot about the usage of DCO.

I don't see a reason not to implement it as opt-in, also given that this request didn't come up before in the past years as far as I know.

gr2m avatar Nov 09 '20 17:11 gr2m

My apologizes for being mistrustful - in the past I had been bitten by a lawyer, and the disease is progressing since then. :D

abitrolly avatar Nov 09 '20 17:11 abitrolly

No worries. Most of the guidance from this repo has come from the Linux Foundation who are heavy DCO users (and usually confer with their lawyers on these things - I am not a lawyer)

My understanding is that the DCO 'sign-off' serves as a very explicit agreement to this text: https://developercertificate.org/ and i'm not really sure GPG does the same? Probably @brianwarner could say more?

hiimbex avatar Nov 10 '20 01:11 hiimbex

With the caveat that I'm not a lawyer and this isn't legal advice...

The current behavior is correct, because -s is about saying you have the rights to contribute the code, whereas -S is about proving who you are. The DCO is only about the first. Put differently, if I give you an apple and you ask if you can eat it, and I show you my passport in response, you would rightfully tell me I answered the wrong question. :-)

It's unfortunate that -s and -S use the same letter, because this is a common source of confusion. Now, of course, I think it's valid to say that -S enhances the DCO signoff, because not only are you making the DCO commitment you are also attesting to your identity. However, if you look to the kernel, where the DCO was developed, only the presence of the Signed-off-by line (whether written by hand, or added automatically using the -s flag) provides a valid signoff.

To summarize:

  • git commit -s automatically adds the Signed-off-by: Your Name <[email protected]> line
    • Should always pass
  • git commit -S does not add the Signed-off-by: Your Name <[email protected]> line, but signs the commit
    • Should never pass, but separately, others can verify it originated from you
  • git commit -S and you manually type Signed-off-by: Your Name <[email protected]> in the commit message
    • Should always pass, and separately, others can verify it originated from you
  • git commit -s -S automatically adds the signoff line and also signs the commit
    • Should always pass, and separately, others can verify it originated from you

brianwarner avatar Nov 10 '20 04:11 brianwarner

Although nothing stops people from committing non-owned code either way, Even pasting it into GitHub edit form. What is important is that they've read about DCO and know what it is, so a simple checkbox on each PR should be sufficient. I think.

  • [x] I commit my own code

As for Linux kernel, I am not sure how a PR submitting interface looks like there.

abitrolly avatar Nov 10 '20 04:11 abitrolly

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

stale[bot] avatar May 10 '21 18:05 stale[bot]

This does seem as if it's still relevant, as the original point @abitrolly raised is still valid: Editing online via GitHub's web interface results in "un-signed-off" commits. (Unless the user takes pains to manually type out a correctly-formatted "Signed-off-by:" trailer in their commit message... something absolutely nobody is going to do.) If a user editing via the web creates un-signed-off commits, they're then forced to clone their fork locally, pull down the branch in question, check it out, amend their commit(s) with a signoff, and force-push them back to the PR branch. That's a lot of friction.

The point is also well-made (IMHO) that this is probably better solved through a statement in the PR, than through signoffs on commits. It's when a PR is created, that you really want to ensure that people have agreed to the DCO.

What's more, now that GitHub supports PR templates, it's even possible to include the actual text of the DCO in a PR template (as a comment only visible to authors). That text can then be followed by a checkbox like the one @abitrolly proposed above. That's arguably a much more affirmative statement than adding -s to the git commit command, an action that doesn't really give any reliable indication that the user has even seen the DCO text.

ferdnyc avatar Nov 29 '23 10:11 ferdnyc

Hi @ferdnyc, this friction was the original motivation for requesting that GitHub provide an option for repo owners to specify that all web-based commits be signed off by default. When an owner has done that, the web UI adds the signoff for them. That support was added two years ago, and is available at both the repo and the org level. Owners simply have to turn it on.

The reason for doing this at the commit (vs. PR) level is that commits are immutable and come along with the repo when it's forked or cloned.

And finally, the original idea of using verified status is well-intended but not quite on target. Verification, like -S signed commits, tell you who someone is, whereas the -s DCO signoff tells you what commitments they're making. They're similar looking sides of two different coins.

brianwarner avatar Nov 29 '23 12:11 brianwarner