dcoapp / app

GitHub App that enforces the Developer Certificate of Origin (DCO) on Pull Requests
https://github.com/apps/dco
ISC License
297 stars 75 forks source link

DCO doesn't respect Verified status by GitHub #141

Open abitrolly opened 3 years ago

abitrolly commented 3 years ago

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.

welcome[bot] commented 3 years ago

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.

travi commented 3 years ago

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

abitrolly commented 3 years ago

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

gr2m commented 3 years ago

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

Done 👍🏼

gr2m commented 3 years ago

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

hiimbex commented 3 years ago

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
jarrodldavis/probot-gpg
A GitHub App that enforces GPG signatures on pull requests (no longer maintained) - jarrodldavis/probot-gpg
gr2m commented 3 years ago

Good to know thank you Bex!

abitrolly commented 3 years ago

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.

gr2m commented 3 years ago

@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.

abitrolly commented 3 years ago

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

hiimbex commented 3 years ago

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?

Developer Certificate of Origin
brianwarner commented 3 years ago

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:

abitrolly commented 3 years ago

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.

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

stale[bot] commented 3 years ago

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

ferdnyc commented 8 months ago

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.

brianwarner commented 8 months ago

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.