git-stack picks up a wrong GPG key for signing commits

[bemyak]

Please complete the following tasks

• I have searched the discussions
• I have searched the existing issues

Description

Git-stack picks up the first GPG key to sign commits.

I have a set of GPG keys set up in the following order (gpg --list-secret-keys --keyid-format LONG):

1. old-work@email.com (an email left from the previous job, not mentioned in any git configs or env vars)
2. private@email.com
3. work@email.com

Whenever I run git sync, it always tries to sign commits with the first key from the list (i.e. old-work@email.com).

Version

0.10.10

Steps to reproduce

1. Run gpg --full-generate-key multiple times to create a couple of GPG keys. To make testing easier, I recommend setting different passphrases for them.
   1. email1@email.com
   2. email2@email.com
   3. email3@email.com
2. Set git email to email2@email.com: git config --global user.email "email2@email.com"
3. Force git to sign all commits: git config --global commit.gpgSign true
4. Set env variables to email3@email.com:
   1. export GIT_AUTHOR_EMAIL=email3@email.com
   2. export GIT_COMMITTER_EMAIL=email3@email.com
5. Run git sync in any repo that will require rebasing and resigning commits

Actual Behaviour

Git-sync signs commits with email1@email.com key

Expected Behaviour

Git-sync should pick up a key in the following priority:

• GIT_COMMITTER_EMAIL
• ~/.config/git/config
• ~/.gitconfig

Debug Output

No response

[epage]

Could you confirm if the committer (not author) for rewritten commits is respecting GIT_COMMITTER_NAME / GIT_COMMITTER_EMAIL or not? That'll confirm where the bug is.

Investigation notes

We do a fallback of

• user.signingkey
• repo signature as reported by libgit2
  • libgit2 seems to exclusively read user.name/user.email

git's fallback is

• configured_signing_key
  • which comes from set_signing_key
    • which comes from user.signingkey
• get_default_key
  • which is skipped for gpg
• git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
  • I made the best guess that this would align with the repo's signature but that was harder to follow
  • Looks like it reads the env for GIT_COMMITTER_NAME / GIT_COMMITER_EMAIL first
  • It then fallsback to author.name / author.email

From looking at this, it seems like git2 isn't reading GIT_COMMITTER_NAME / GIT_COMMITER_EMAIL but should be reading the configs in the correct order. The way to confirm this is for any other place we read the repo signature and see if its wrong. We generate a new signature for the committer when we rewrite history, using the repo's signature.

[bemyak]

Given the setup I described in the reproduction steps, the final result of running git sync (as shown by git log --format=full --show-signature) is:

signature: email1@email.com
Author: email3@email.com
Commit: email2@email.com

So, it looks like the author field respects GIT_AUTHOR_EMAIL and GIT_AUTHOR_NAME

The committer is taken from the git config file (~/.gitconfig) and the env vars GIT_COMMITTER_EMAIL and GIT_COMMITTER_NAME are ignored

And the signature is the first signature from the list, this email isn't used anywhere in git configs.

[epage]

The committer is taken from the git config file (~/.gitconfig) and the env vars GIT_COMMITTER_EMAIL and GIT_COMMITTER_NAME are ignored

Can you open a bug about this?

And the signature is the first signature from the list, this email isn't used anywhere in git configs.

The fact that the signature and committer are using different emails is bizarre as they should be coming from the same source.

Everytime I've used gpg, it has been an abysmal failure to the point I don't touch it. I wrote all of this code blind, just reading git's source code. Would you be able to help out in debugging this?

• https://github.com/gitext-rs/git2-ext/blob/main/src/ops.rs#L310 is where we read the email
• https://github.com/gitext-rs/git2-ext/blob/main/src/ops.rs#L395 is where we pass it in to sign with

To use a local copy of git2-ext with git-stack, you can add the following to Cargo.toml:

[patch.crates-io]
git2-ext = { path = "<put path in here>" }

[bemyak]

I came up with a partial fix in the PR linked above. Now the key matches the committer, at least.

However, the committer field still doesn't respect the env variables. I skimmed through the code and found only one suspicion line here, where committer is set to an author, but it is an irrelevant part of the code.

In which project can the issue be?

[epage]

However, the committer field still doesn't respect the env variables.

The call that is a problem is repo.signature() as libgit2 is returning the bad information. The way I would fix this is add a new signature function to git2-ext that checks the env variables and then falls back to repo.signature(). It'll need to mimic what git does when only one or the other env variable is set. It should also read the env using std::env::var_os as std::env_var can panic.

[bemyak]

BTW, have you considered switching to gitoxide? This case seems to be handled correctly there. (Not asking to rewrite the entire set of gitext tools to gitoxide, just wondering 🙂 )

[epage]

I've been in talks with Byron: GitoxideLabs/gitoxide#311.

[bemyak]

std::env_var can panic

According to the docs, it should not. It returns an error if the var contains an invalid Unicode. So I'll use it as we are not handling non-Unicode symbols elsewhere.

[epage]

Good call. I got it mixed up with std::env::args (love that inconsistency)

[bemyak]

I did one more fix: gitext-rs/git2-ext#28

Now the signature is correct, but the committer is not :)

[bemyak]

I can't find a section in the code where commits are created.

Here only a name() is used, so I can't figure out where does email come from, and why is it different from the original commit.