False-positive `body-leading-blank` warnings for GPG-signed commits.

mschilli87 commented 2 years ago

This is a follow-up to https://github.com/conventional-changelog/commitlint/issues/876.

The upshot is that I am getting

⚠ body must have leading blank line [body-leading-blank]

warnings for GPG-signed commits that actually do have leading blanks.

Furthermore, the very same commit messages do not trigger that warning when passed in from STDIN or when not GPG-signing the commits.

My guess is that the original issue might have been 'resolved' when switching to the other computer by different Git default configurations on the two systems involved (the first one GPG-signing by default, the second one not).

Expected Behavior

When a message contains a blank line between header and body it should not trigger a body-leading-blank warning, regardless whether the corresponding commit was GPG signed or not.

Current Behavior

When a message contains a blank line between header and body it does not trigger a body-leading-blank warning when passed in via STDIN or queried from the Git log via a non-GPG-signed commit, but does so when queried from the Git log via a GPG-signed commit.

Affected packages

[X] cli
[ ] core
[ ] prompt
[ ] config-angular

Steps to Reproduce (for bugs)

Create test repository:
```
mkdir test
cd test
git init --quiet
```

Write test commit message to a text file.

cat > commit-message << CommitMessage
test: a totally valid commit message

This message has a body that is separated from the header line by an
empty line.
CommitMessage

Create default commitlint config:

cat > commitlint.config.js << CommitlintConfig
module.exports = {extends: ["@commitlint/config-conventional"]}
CommitlintConfig

Ensure that commit message does not trigger a body-leading-blank warning when passed in via STDIN:
```
commitlint \
   --config=commitlint.config.js \
   < commit-message
```

Create non-GPG-signed commit using that exact same message:

git add .
git commit \
   --quiet \
   --no-gpg-sign \
   --file=commit-message

Ensure that commit message does not trigger a body-leading-blank warning when passed in via the non-GPG-signed commit:
```
commitlint \
   --config=commitlint.config.js \
   --to=HEAD
```

Create a temporary GnuPG home directory:

export GNUPGHOME="$(pwd)/gpg"
mkdir "$GNUPGHOME"
chmod 700 "$GNUPGHOME"

Generate a temporary GPG key:

# WARNING: This is meant for testing purposes only!
# This is not my recommended way of generating GPG keys for production
# use. Please refer to the appropriate documentation when setting up GPG
# to sign your actual commits.
gpg \
   --quiet \
   --batch \
   --gen-key << GnuPGKeyGenConfig
Key-Type: 1
Key-Length: 2048
Subkey-Type: 1
Subkey-Length: 2048
%no-protection
Name-Real: Test User
Name-Email: user@test.tld
Expire-Date: 0
GnuPGKeyGenConfig

Amend the commit to GPG-sign it without changing the commit message:

git commit \
   --quiet \
   --amend \
   --gpg-sign='Test user <user@test.tld>' \
   --no-edit

Observe the false-positive body-leading-blank warning when queried from the Git log via the GPG-signed commit:

commitlint \
    --config=commitlint.config.js \
    --to=HEAD

⧗   input: test: a totally valid commit message
⚠   body must have leading blank line [body-leading-blank]

⚠   found 0 problems, 1 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

commitlint.config.js

```js module.exports = {extends: ["@commitlint/config-conventional"]} ```

Context

See this blogpost by Mike Gerwitz for a nice writeup of why GPG-signing commits is a good idea. If I have to decide between using commitlint or GPG-signing my commits, I'll choose the latter.

Your Environment

This is on Gentoo using Node.js and Git from Portage but commitlint installed via

npm install --global '@commitlint/cli'

Furthermore, the Conventional Commits configuration module was installed via

npm install --global '@commitlint/config-conventional'

For it to be found, I also had to add the following to my shell environment:

export NODE_PATH="$HOME/.npm-global/lib64/node_modules"

Executable	Version
`commitlint --version`	`@commitlint/cli@16.1.0`
`git --version`	`git version 2.34.1`
`node --version`	`v14.17.6`

``

escapedcat commented 2 years ago

Thanks for this! Before having a closer look, do you think these are related?:

mschilli87 commented 2 years ago

@escapedcat:

[...] [D]o you think these are related?:

allow to require signed commits #321 (comment)

I don't see how that one would be related beyond the fact that they both deal with signed commits. It is asking for a new feature allowing to enforce signed commits. I am just asking to fix a bug where an unrelated already existing check is broken when applied to signed commits.

Signed-off-by error #59 (comment)

That one is entirely unrelated: It is about signing off commits using -s/--signoff which simply adds a specific line to the commit message footer and is commoly used as some sort of 'signature' in the sense that the commiter confirms code ownership or such things. My issue is with commits signed cryptographically via -S/--gpg-signed to prevent untrusted parties from modifying the source code once the commit is published.

escapedcat commented 2 years ago

Alright, thanks for you feedback.
Would you be motivated and have time to look into this and maybe create a PR for this?

mschilli87 commented 2 years ago

@escapedcat

Would you be motivated and have time to look into this and maybe create a PR for this?

I am sorry but I have no experience with TypeScript or even simple JavaScript so I doubt I'll be able to come up with a viable solution by myself in a reasonable timeframe.

escapedcat commented 2 years ago

I am sorry but I have no experience with TypeScript or even simple JavaScript so I doubt I'll be able to come up with a viable solution by myself in a reasonable timeframe.

Alright, no worries

conventional-changelog / commitlint