{"thread":{"id":"65871","subject":"[PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set","startedAt":"2026-06-25T19:43:47Z","lastAt":"2026-07-10T03:43:12Z","messageCount":4,"participants":["Grayson Tinker","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"546427","messageId":"20260625194330.3711-1-graysontinker@gmail.com","threadId":"65871","inReplyTo":null,"subject":"[PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set","fromName":"Grayson Tinker","fromEmail":"graysontinker@gmail.com","sentAt":"2026-06-25T19:43:11Z","receivedAt":"2026-06-25T19:43:47Z","isPatch":true,"body":"\"show-signature\" errors when the allowed signers file is not configured,\nwhich means that the user can't see the key that the ref was signed with\nwithout creating and configuring the file. Change the logic so that the file\nis only used when configured, and so the signature status is always displayed.\n\nExample of previous output:\n```\nerror: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification\ncommit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)\nNo signature\nAuthor: Grayson Tinker \n```\n\nExample of new output:\n```\ncommit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)\nhint: Configure gpg.ssh.allowedSignersFile for automatic principal matching\nGood \"git\" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw\nAuthor: Grayson Tinker \n```\n\nSigned-off-by: Grayson Tinker \n---\n gpg-interface.c | 42 +++++++++++++++++++++++-------------------\n 1 file changed, 23 insertions(+), 19 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex dafd5371fa..ef3e1a0aa0 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1,6 +1,7 @@\n #define USE_THE_REPOSITORY_VARIABLE\n \n #include \"git-compat-util.h\"\n+#include \"advice.h\"\n #include \"commit.h\"\n #include \"config.h\"\n #include \"date.h\"\n@@ -480,11 +481,6 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\t.local = 1,\n \t};\n \n-\tif (!ssh_allowed_signers) {\n-\t\terror(_(\"gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification\"));\n-\t\treturn -1;\n-\t}\n-\n \tbuffer_file = mks_tempfile_t(\".git_vtag_tmpXXXXXX\");\n \tif (!buffer_file)\n \t\treturn error_errno(_(\"could not create temporary file\"));\n@@ -500,22 +496,26 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\tstrbuf_addf(&verify_time, \"-Overify-time=%s\",\n \t\t\tshow_date(sigc->payload_timestamp, 0, verify_date_mode));\n \n-\t/* Find the principal from the signers */\n-\tstrvec_pushl(&ssh_keygen.args, fmt->program,\n-\t\t \"-Y\", \"find-principals\",\n-\t\t \"-f\", ssh_allowed_signers,\n-\t\t \"-s\", buffer_file->filename.buf,\n-\t\t verify_time.buf,\n-\t\t NULL);\n-\tret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,\n-\t\t\t &ssh_principals_err, 0);\n-\tif (ret && strstr(ssh_principals_err.buf, \"usage:\")) {\n-\t\terror(_(\"ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)\"));\n-\t\tgoto out;\n+\tif (ssh_allowed_signers) {\n+\t\t/* Find the principal from the signers */\n+\t\tstrvec_pushl(&ssh_keygen.args, fmt->program,\n+\t\t\t\t\"-Y\", \"find-principals\",\n+\t\t\t\t\"-f\", ssh_allowed_signers,\n+\t\t\t\t\"-s\", buffer_file->filename.buf,\n+\t\t\t\tverify_time.buf,\n+\t\t\t\tNULL);\n+\t\tret = pipe_command(&ssh_keygen, NULL, 0, &ssh_principals_out, 0,\n+\t\t\t\t&ssh_principals_err, 0);\n+\t\tif (ret && strstr(ssh_principals_err.buf, \"usage:\")) {\n+\t\t\terror(_(\"ssh-keygen -Y find-principals/verify is needed for ssh signature verification (available in openssh version 8.2p1+)\"));\n+\t\t\tgoto out;\n+\t\t}\n \t}\n-\tif (ret || !ssh_principals_out.len) {\n+\n+\tif (!ssh_allowed_signers || ret || !ssh_principals_out.len) {\n \t\t/*\n-\t\t * We did not find a matching principal in the allowedSigners\n+\t\t * We did not find a matching principal in the allowedSigners,\n+\t\t * or no allowedSigners file was configured\n \t\t * Check without validation\n \t\t */\n \t\tchild_process_init(&ssh_keygen);\n@@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n \t\tpipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,\n \t\t\t\t &ssh_keygen_out, 0, &ssh_keygen_err, 0);\n \n+\t\tif (!ssh_allowed_signers) {\n+\t\t\tadvise(_(\"Configure gpg.ssh.allowedSignersFile for automatic principal matching\\n\"));\n+\t\t}\n+\n \t\t/*\n \t\t * Fail on unknown keys\n \t\t * we still call check-novalidate to display the signature info\n-- \n2.54.0\n\n"},{"id":"547658","messageId":"CAAr3fC2BRQ-pesXuQbNz6avBBbBOUSTtBQ8uRFX6AELszzH=Ug@mail.gmail.com","threadId":"65871","inReplyTo":"20260625194330.3711-1-graysontinker@gmail.com","subject":"Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set","fromName":"Grayson Tinker","fromEmail":"graysontinker@gmail.com","sentAt":"2026-07-09T23:50:04Z","receivedAt":"2026-07-09T23:50:16Z","isPatch":true,"body":"Friendly ping on this :)\n"},{"id":"547660","messageId":"xmqqtsq7haev.fsf@gitster.g","threadId":"65871","inReplyTo":"20260625194330.3711-1-graysontinker@gmail.com","subject":"Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-10T01:59:36Z","receivedAt":"2026-07-10T01:59:39Z","isPatch":true,"body":"Grayson Tinker writes:\n\n> \"show-signature\" errors when the allowed signers file is not configured,\n> which means that the user can't see the key that the ref was signed with\n> without creating and configuring the file. Change the logic so that the file\n> is only used when configured, and so the signature status is always displayed.\n>\n> Example of previous output:\n> ```\n> error: gpg.ssh.allowedSignersFile needs to be configured and exist for ssh signature verification\n> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)\n> No signature\n> Author: Grayson Tinker \n> ```\n>\n> Example of new output:\n> ```\n> commit b437db5ddc38ebda223bbae2087eee90a7b1c6e2 (HEAD -> master)\n> hint: Configure gpg.ssh.allowedSignersFile for automatic principal matching\n> Good \"git\" signature with ED25519-SK key SHA256:yTU4KFs/g6MY7biDSlVStB63Gi1rCKg7dOFDXbe0yuw\n> Author: Grayson Tinker \n> ```\n\nWhile I haven't closely looked at the parts of this patch that I did\nnot quote here, this specific section caught my eye:\n\n> @@ -528,6 +528,10 @@ static int verify_ssh_signed_buffer(struct signature_check *sigc,\n> \t\tpipe_command(&ssh_keygen, sigc->payload, sigc->payload_len,\n> \t\t\t\t &ssh_keygen_out, 0, &ssh_keygen_err, 0);\n> \n> +\t\tif (!ssh_allowed_signers) {\n> +\t\t\tadvise(_(\"Configure gpg.ssh.allowedSignersFile for automatic principal matching\\n\"));\n> +\t\t}\n\nIf a user runs 'git log --show-signature -100', they will be spammed\nwith this message 100 times. Because it bypasses the\nadvise_if_enabled() mechanism, there is no way for them to disable\nit.\n\nSince I don't use SSH signing, I'm curious: how common or useful is\nit to run log --show-signature without allowedSignersFile\nconfigured? If it serves no purpose at all, then perhaps this\nwarning is acceptable, as users would have to configure the variable\nto get any utility out of the command. \n\nHowever, doesn't cryptographic verification still provide value on\nits own? Even without allowedSignersFile, the signature at least\nguarantees the commit content hasn't been modified since it was\nsigned, even if the signer's identity remains unverified. If some\nusers rely on this purely cryptographic validation, they probably\nwon't want to maintain an allowed signers file, and they would\ndefinitely want a way to squelch this repetitive advice. \n\nThanks.\n"},{"id":"547669","messageId":"CAAr3fC29Mkn08B4-TWF9Vuhng0TcjV+mRGFa9DjkxgKixB_hXQ@mail.gmail.com","threadId":"65871","inReplyTo":"xmqqtsq7haev.fsf@gitster.g","subject":"Re: [PATCH] gpg-interface: still print ssh signatures when allowed signers file is not set","fromName":"Grayson Tinker","fromEmail":"graysontinker@gmail.com","sentAt":"2026-07-10T03:42:59Z","receivedAt":"2026-07-10T03:43:12Z","isPatch":true,"body":"On Thu, Jul 9, 2026 at 6:59 PM Junio C Hamano wrote:\n>\n> If a user runs 'git log --show-signature -100', they will be spammed\n> with this message 100 times. Because it bypasses the\n> advise_if_enabled() mechanism, there is no way for them to disable\n> it.\n\nThe downside to using advise_if_enabled is that this single line would get\nturned into three, with a newline in the middle, which would decently\ndisrupt the view of the log until the hint is disabled. I'm not sure what the\nbest solution is here; I lean towards keeping it as is to reduce the overall\nnoise level, but perhaps those who use this feature more would disagree.\n\n(The previous message was also printed every time, FWIW. So at the\nvery least this isn't worse behavior.)\n\nI'll make the hint disableable if you'd prefer.\n\n> However, doesn't cryptographic verification still provide value on\n> its own? Even without allowedSignersFile, the signature at least\n> guarantees the commit content hasn't been modified since it was\n> signed, even if the signer's identity remains unverified. If some\n> users rely on this purely cryptographic validation, they probably\n> won't want to maintain an allowed signers file, and they would\n> definitely want a way to squelch this repetitive advice.\n\nAllowing for this usecase was exactly the intent of this patch; I am\nthis type of user and had some annoyance with this.\n\nThanks!\n"}]}