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

Correct looking Signoff not recognised #151

Open vonneudeck opened 3 years ago

vonneudeck commented 3 years ago

It is a mystery to me, but the dco probot complains about two equal strings being unequal. I even copied the strings and hexdumped them to see if they truly are the same (to rule out non-breaking spaces). I don’t understand what is wrong, but I remember that I had this issue before with your probot.

[mon@localhost ~]$ echo Nicolai von Neudeck nicolai@vonneudeck.com | hexdump
0000000 694e 6f63 616c 2069 6f76 206e 654e 6475
0000010 6365 206b 696e 6f63 616c 4069 6f76 6e6e
0000020 7565 6564 6b63 632e 6d6f 000a          
000002b
[mon@localhost ~]$ echo Nicolai von Neudeck nicolai@vonneudeck.com | hexdump
0000000 694e 6f63 616c 2069 6f76 206e 654e 6475
0000010 6365 206b 696e 6f63 616c 4069 6f76 6e6e
0000020 7565 6564 6b63 632e 6d6f 000a          
000002b

You can see the error message at the bottom here: https://github.com/cortexproject/cortex/pull/4009/checks?check_run_id=2194259050

vonneudeck commented 3 years ago

If I understood this code correctly it should not be possible to even get that error message because sig.name and sig.email clearly have values and they are the same as commit.author.name and commit.author.email https://github.com/probot/dco/blob/5b242dc14a4bc459b6819b16c43f3fb8da3d29ae/lib/dco.js#L53-L54

I checked the regex in regexr.com and it seems okay, but I am confused about the function of the $/img But as the results returned by commit.author.name and commit.author.email look fine, this can't be it, can it? https://github.com/probot/dco/blob/5b242dc14a4bc459b6819b16c43f3fb8da3d29ae/lib/dco.js#L74

So I was wondering, could this be a race condition? Is there a way that the constants authors or email in https://github.com/probot/dco/blob/5b242dc14a4bc459b6819b16c43f3fb8da3d29ae/lib/dco.js#L48-L49 are populated exactly between the execution of https://github.com/probot/dco/blob/5b242dc14a4bc459b6819b16c43f3fb8da3d29ae/lib/dco.js#L53 and https://github.com/probot/dco/blob/5b242dc14a4bc459b6819b16c43f3fb8da3d29ae/lib/dco.js#L54 ?

I also wondered if maybe the two spaces in my name make the regex slower than with other people’s names so that a race condition would not be triggered by them?

scottrigby commented 1 year ago

Found the issue. There is a trailing space after your display name:

Screen Shot 2022-09-07 at 10 36 49 AM

But the DCO bot regex does not allow for this:

https://github.com/dcoapp/app/blob/266f33e73f1968ff6798866a92528aec2560565d/lib/dco.js#L122

scottrigby commented 1 year ago

For now, a user can fix this by updating their GitHub display name. Ultimately make your DCO match the currently expected pattern – only one space between name and <email>.

If a repo maintainer wants to relax this rule for other people on their repo, perhaps a configuration could allow for this. Something like:

allowNameWhitespace: true
- const regex = /^Signed-off-by: (.*) <(.*)>\s*$/gim;
+ if (allowNameWhitespace) {
+   const regex = /^Signed-off-by: (.*)\s <(.*)>\s*$/gim;
+ } else {
+   const regex = /^Signed-off-by: (.*) <(.*)>\s*$/gim;
+ }
vonneudeck commented 1 year ago

Which trailing space do you mean exactly? The one after "Neudeck"? That one is done by Github. Or the one after "von"? That one is part of my legal name. It can't be removed. It would be like cutting of the "Rig" from "Rigby".