[PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

[PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

[Grayson Tinker]

"show-signature" errors when the allowed signers file is not configured,
which means that the user can't see the key that the ref was signed with
without creating and configuring the file. Change the logic so that the file
is only used when configured, and so the signature status is always displayed.

Example of previous output:
```
error: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification
commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
No signature
Author: Grayson Tinker <graysontinker@gmail.com>
```

Example of new output:
```
commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
hint: Configure gpg.ssh.allowedSignersFile for automatic principal matching
Good "git" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw
Author: Grayson Tinker <graysontinker@gmail.com>
```

Signed-off-by: Grayson Tinker <graysontinker@gmail.com>
---
 gpg-interface.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index dafd5371fa..ef3e1a0aa0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "git-compat-util.h"
+#include "advice.h"
 #include "commit.h"
 #include "config.h"
 #include "date.h"
@@ -480,11 +481,6 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		.local = 1,
 	};
 
-	if (!ssh_allowed_signers) {
-		error(_("gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification"));
-		return -1;
-	}
-
 	buffer_file = mks_tempfile_t(".git_vtag_tmpXXXXXX");
 	if (!buffer_file)
 		return error_errno(_("could not create temporary file"));
@@ -500,22 +496,26 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		strbuf_addf(&verify_time, "-Overify-time=%s",
 			show_date(sigc->payload_timestamp, 0, verify_date_mode));
 
-	/* Find the principal from the signers */
-	strvec_pushl(&ssh_keygen.args, fmt->program,
-		     "-Y", "find-principals",
-		     "-f", ssh_allowed_signers,
-		     "-s", buffer_file->filename.buf,
-		     verify_time.buf,
-		     NULL);
-	ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
-			   &ssh_principals_err, 0);
-	if (ret && strstr(ssh_principals_err.buf, "usage:")) {
-		error(_("ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)"));
-		goto out;
+	if (ssh_allowed_signers) {
+		/* Find the principal from the signers */
+		strvec_pushl(&ssh_keygen.args, fmt->program,
+				"-Y", "find-principals",
+				"-f", ssh_allowed_signers,
+				"-s", buffer_file->filename.buf,
+				verify_time.buf,
+				NULL);
+		ret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,
+				&ssh_principals_err, 0);
+		if (ret && strstr(ssh_principals_err.buf, "usage:")) {
+			error(_("ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)"));
+			goto out;
+		}
 	}
-	if (ret || !ssh_principals_out.len) {
+
+	if (!ssh_allowed_signers || ret || !ssh_principals_out.len) {
 		/*
-		 * We did not find a matching principal in the allowedSigners
+		 * We did not find a matching principal in the allowedSigners,
+		 * or no allowedSigners file was configured
 		 * Check without validation
 		 */
 		child_process_init(&ssh_keygen);
@@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
 		pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
 				   &ssh_keygen_out, 0, &ssh_keygen_err, 0);
 
+		if (!ssh_allowed_signers) {
+			advise(_("Configure gpg.ssh.allowedSignersFile for automatic principal matching\n"));
+		}
+
 		/*
 		 * Fail on unknown keys
 		 * we still call check-novalidate to display the signature info
-- 
2.54.0

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

[Grayson Tinker]

Friendly ping on this :)

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

[Junio C Hamano]

Grayson Tinker <graysontinker@gmail.com> writes:

> "show-signature" errors when the allowed signers file is not configured,
> which means that the user can't see the key that the ref was signed with
> without creating and configuring the file. Change the logic so that the file
> is only used when configured, and so the signature status is always displayed.
>
> Example of previous output:
> ```
> error: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification
> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
> No signature
> Author: Grayson Tinker <graysontinker@gmail.com>
> ```
>
> Example of new output:
> ```
> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)
> hint: Configure gpg.ssh.allowedSignersFile for automatic principal matching
> Good "git" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw
> Author: Grayson Tinker <graysontinker@gmail.com>
> ```

While I haven't closely looked at the parts of this patch that I did
not quote here, this specific section caught my eye:

> @@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,
>  		pipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,
>  				   &ssh_keygen_out, 0, &ssh_keygen_err, 0);
>  
> +		if (!ssh_allowed_signers) {
> +			advise(_("Configure gpg.ssh.allowedSignersFile for automatic principal matching\n"));
> +		}

If a user runs 'git log --show-signature -100', they will be spammed
with this message 100 times.  Because it bypasses the
advise_if_enabled() mechanism, there is no way for them to disable
it.

Since I don't use SSH signing, I'm curious: how common or useful is
it to run log --show-signature without allowedSignersFile
configured?  If it serves no purpose at all, then perhaps this
warning is acceptable, as users would have to configure the variable
to get any utility out of the command. 

However, doesn't cryptographic verification still provide value on
its own?  Even without allowedSignersFile, the signature at least
guarantees the commit content hasn't been modified since it was
signed, even if the signer's identity remains unverified.  If some
users rely on this purely cryptographic validation, they probably
won't want to maintain an allowed signers file, and they would
definitely want a way to squelch this repetitive advice. 

Thanks.

Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set

[Grayson Tinker]

On Thu, Jul 9, 2026 at 6:59 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> If a user runs 'git log --show-signature -100', they will be spammed
> with this message 100 times.  Because it bypasses the
> advise_if_enabled() mechanism, there is no way for them to disable
> it.

The downside to using advise_if_enabled is that this single line would get
turned into three, with a newline in the middle, which would decently
disrupt the view of the log until the hint is disabled. I'm not sure what the
best solution is here; I lean towards keeping it as is to reduce the overall
noise level, but perhaps those who use this feature more would disagree.

(The previous message was also printed every time, FWIW. So at the
very least this isn't worse behavior.)

I'll make the hint disableable if you'd prefer.

> However, doesn't cryptographic verification still provide value on
> its own?  Even without allowedSignersFile, the signature at least
> guarantees the commit content hasn't been modified since it was
> signed, even if the signer's identity remains unverified.  If some
> users rely on this purely cryptographic validation, they probably
> won't want to maintain an allowed signers file, and they would
> definitely want a way to squelch this repetitive advice.

Allowing for this usecase was exactly the intent of this patch; I am
this type of user and had some annoyance with this.

Thanks!