ssh signing: better error message when key not in agent

From: Adam Szkoda <[email protected]>

When signing a commit with a SSH key, with the private key missing from ssh-agent, a confusing error message is produced:

error: Load key "/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7":
invalid format? fatal: failed to write commit object

The temporary file .git_signing_key_tmpkArSj7 created by git contains a valid *public* key. The error message comes from `ssh-keygen -Y sign' and is caused by a fallback mechanism in ssh-keygen whereby it tries to interpret .git_signing_key_tmpkArSj7 as a *private* key if it can't find in the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to 'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent.

As a result, when the private key is missing from the agent, a more accurate error message gets produced:

error: Couldn't find key in agent

[1] https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Signed-off-by Adam Szkoda <[email protected]>

---
ssh signing: better error message when key not in agent

When signing a commit with a SSH key, with the private key missing from ssh-agent, a confusing error message is produced:

error: Load key "/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7": invalid format?
fatal: failed to write commit object

The temporary file .git_signing_key_tmpkArSj7 created by git contains a valid public key. The error message comes from `ssh-keygen -Y sign' and is caused by a fallback mechanism in ssh-keygen whereby it tries to interpret .git_signing_key_tmpkArSj7 as a private key if it can't find in the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to 'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent.

As a result, when the private key is missing from the agent, a more accurate error message gets produced:

error: Couldn't find key in agent

[1] https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1270%2Fradicle-dev%2Fmaint-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1270/radicle-dev/maint-v1
Pull-Request: https://github.com/git/git/pull/1270

  gpg-interface.c | 1 +
  1 file changed, 1 insertion(+)

gpg-interface.c +1 -0

index 280f1fa1a58..4a5913ae942 100644
@@ -1022,6 +1022,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
  	strvec_pushl(&signer.args, use_format->program,
  		     "-Y", "sign",
  		     "-n", "git",
+		     "-U",
  		     "-f", ssh_signing_key_file,
  		     buffer_file->filename.buf,
  		     NULL);

base-commit: e54793a95afeea1e10de1e5ad7eab914e7416250

Quoted 5 lines

the agent [1].  A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call.  With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent.

The documentation for user.signingKey says

 If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 7 lines

The documentation for user.signingKey says

If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

Hi Phillip,

Good point! My first thought is to try doing a stat() syscall on the path from 'user.signingKey' to see if it exists and if not, treat it as a public key (and pass the -U option). If that sounds reasonable, I can update the patch.

Best
— Adam

Quoted 24 lines · Phillip Wood

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 7 lines

The documentation for user.signingKey says

If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

Hi Adam

I've cc'd Fabian who knows more about the ssh signing code that I do.

On 18/01/2023 15:28, Adam Szkoda wrote:

Quoted 6 lines

Hi Phillip,

Good point! My first thought is to try doing a stat() syscall on the path from 'user.signingKey' to see if it exists and if not, treat it as a public key (and pass the -U option). If that sounds reasonable, I can update the patch.

My reading of the documentation is that user.signingKey may point to a public or private key so I'm not sure how stat()ing would help. Looking at the code in sign_buffer_ssh() we have a function is_literal_ssh_key() that checks if the config value is a public key. When the user passes the path to a key we could read the file check use is_literal_ssh_key() to check if it is a public key (or possibly just check if the file begins with "ssh-"). Fabian - does that sound reasonable?

Best Wishes

Phillip

Quoted 29 lines

Best
— Adam

Quoted 24 lines · Phillip Wood

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 7 lines

The documentation for user.signingKey says

If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

Quoted 12 lines · Phillip Wood

Hi Adam

I've cc'd Fabian who knows more about the ssh signing code that I do.

Quoted 6 lines · Adam Szkoda

Hi Phillip,

Good point!  My first thought is to try doing a stat() syscall on the path from 'user.signingKey' to see if it exists and if not, treat it as a public key (and pass the -U option).  If that sounds reasonable, I can update the patch.

Quoted 1 line

My reading of the documentation is that user.signingKey may point to a public or private key so I'm not sure how stat()ing would help. Looking at the code in sign_buffer_ssh() we have a function is_literal_ssh_key() that checks if the config value is a public key. When the user passes the path to a key we could read the file check use is_literal_ssh_key() to check if it is a public key (or possibly just check if the file begins with "ssh-"). Fabian - does that sound reasonable?

Hi,
I have encountered the mentioned problem before as well and tried to fix it but did not find a good / reasonable way to do so. Git just passes the user.signingKey to ssh-keygen which states:
The key used for signing is specified using the -f option and may refer to either a private key, or a public key with the private half available via ssh-agent(1)

I don't think it's a good idea for git to parse the key and try to determine if it's public or private. The fix should probably be in openssh (different error message) but when looking into it last time i remember that the logic for using the key is quite deeply embedded into the ssh code and not easily adjusted for the signing use case. At the moment I don't have the time to look into it but the openssh code for signing is quite readable so feel free to give it a try. Maybe you find a good way.

Best regards,
Fabian

Quoted 34 lines

Best Wishes

Phillip

Quoted 29 lines

Best
— Adam

Quoted 24 lines · Phillip Wood

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1].  A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call.  With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 7 lines

The documentation for user.signingKey says

  If gpg.format is set to ssh this can contain the path to either your private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

Quoted 15 lines · Fabian Stelzer

Quoted 13 lines · Phillip Wood

Hi Adam

I've cc'd Fabian who knows more about the ssh signing code that I do.

Quoted 6 lines · Adam Szkoda

Hi Phillip,

Good point!  My first thought is to try doing a stat() syscall on the path from 'user.signingKey' to see if it exists and if not, treat it as a public key (and pass the -U option).  If that sounds reasonable, I can update the patch.

My reading of the documentation is that user.signingKey may point to a public or private key so I'm not sure how stat()ing would help. Looking at the code in sign_buffer_ssh() we have a function is_literal_ssh_key() that checks if the config value is a public key. When the user passes the path to a key we could read the file check use is_literal_ssh_key() to check if it is a public key (or possibly just check if the file begins with "ssh-"). Fabian - does that sound reasonable?

Quoted 5 lines

Hi,
I have encountered the mentioned problem before as well and tried to fix it but did not find a good / reasonable way to do so. Git just passes the user.signingKey to ssh-keygen which states:
The key used for signing is specified using the -f option and may refer to either a private key, or a public key with the private half available via ssh-agent(1)

I don't think it's a good idea for git to parse the key and try to determine if it's public or private. The fix should probably be in openssh (different error message) but when looking into it last time i remember that the logic for using the key is quite deeply embedded into the ssh code and not easily adjusted for the signing use case. At the moment I don't have the time to look into it but the openssh code for signing is quite readable so feel free to give it a try. Maybe you find a good way.

Thanks Fabian, perhaps the easiest way forward is for us to only pass "-U" when we have a literal key in user.signingKey as we know it must a be public key in that case.

Best Wishes

Phillip

Quoted 37 lines

Best regards,
Fabian

Quoted 34 lines

Best Wishes

Phillip

Quoted 29 lines

Best
— Adam

Quoted 24 lines · Phillip Wood

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1].  A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call.  With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 7 lines

The documentation for user.signingKey says

  If gpg.format is set to ssh this can contain the path to either your private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

It should be safe to use -U even for older versions. It won't require the agent (as openssh-9.1 will) but it won't cause an error.

Quoted 40 lines · Phillip Wood

Quoted 30 lines · Fabian Stelzer

Quoted 12 lines · Phillip Wood

Hi Adam

I've cc'd Fabian who knows more about the ssh signing code that I do.

Quoted 6 lines · Adam Szkoda

Hi Phillip,

Good point! My first thought is to try doing a stat() syscall on the path from 'user.signingKey' to see if it exists and if not, treat it as a public key (and pass the -U option). If that sounds reasonable, I can update the patch.

Quoted 8 lines

My reading of the documentation is that user.signingKey may point to a public or private key so I'm not sure how stat()ing would help. Looking at the code in sign_buffer_ssh() we have a function is_literal_ssh_key() that checks if the config value is a public key. When the user passes the path to a key we could read the file check use is_literal_ssh_key() to check if it is a public key (or possibly just check if the file begins with "ssh-"). Fabian - does that sound reasonable?

Hi,
I have encountered the mentioned problem before as well and tried to fix it but did not find a good / reasonable way to do so. Git just passes the user.signingKey to ssh-keygen which states:
The key used for signing is specified using the -f option and may refer to either a private key, or a public key with the private half available via ssh-agent(1)

Quoted 8 lines

I don't think it's a good idea for git to parse the key and try to determine if it's public or private. The fix should probably be in openssh (different error message) but when looking into it last time i remember that the logic for using the key is quite deeply embedded into the ssh code and not easily adjusted for the signing use case. At the moment I don't have the time to look into it but the openssh code for signing is quite readable so feel free to give it a try. Maybe you find a good way.

Quoted 3 lines

Thanks Fabian, perhaps the easiest way forward is for us to only pass "-U" when we have a literal key in user.signingKey as we know it must a be public key in that case.

Yes, i think that's a good idea as long as the -U flag is ignored in older ssh versions and shouldn't be too hard to implement. And it should work just as well when using defaultKeyCommand.

Best,
Fabian

Quoted 44 lines

Best Wishes

Phillip

Quoted 39 lines

Best regards,
Fabian

Quoted 36 lines

Best Wishes

Phillip

Quoted 31 lines

Best
— Adam

Quoted 25 lines · Phillip Wood

Quoted 8 lines · Phillip Wood

Quoted 7 lines

the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that
needs to be done is to pass an additional backward-compatible option -U to
'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file
as public key and expects to find the private key in the agent.

Quoted 8 lines

The documentation for user.signingKey says

If gpg.format is set to ssh this can contain the path to either your
private ssh key or the public key when ssh-agent is used.

If I've understood correctly passing -U will prevent users from setting
this to a private key.

If there is an easy way to tell if the user has given us a public key then we could pass "-U" in that case.

Best Wishes

Phillip

From: Adam Szkoda <[email protected]>

When signing a commit with a SSH key, with the private key missing from ssh-agent, a confusing error message is produced:

error: Load key "/var/folders/t5/cscwwl_n3n1_8_5j_00x_3t40000gn/T//.git_signing_key_tmpkArSj7":
invalid format? fatal: failed to write commit object

The temporary file .git_signing_key_tmpkArSj7 created by git contains a valid *public* key. The error message comes from `ssh-keygen -Y sign' and is caused by a fallback mechanism in ssh-keygen whereby it tries to interpret .git_signing_key_tmpkArSj7 as a *private* key if it can't find in the agent [1]. A fix is scheduled to be released in OpenSSH 9.1. All that needs to be done is to pass an additional backward-compatible option -U to 'ssh-keygen -Y sign' call. With '-U', ssh-keygen always interprets the file as public key and expects to find the private key in the agent.

As a result, when the private key is missing from the agent, a more accurate error message gets produced:

error: Couldn't find key in agent

[1] https://bugzilla.mindrot.org/show_bug.cgi?id=3429

Signed-off-by Adam Szkoda <[email protected]>

---

Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1270/radicle-dev/maint-v2
Pull-Request: https://github.com/git/git/pull/1270

Range-diff vs v1:

 1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
 -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent

 gpg-interface.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

gpg-interface.c +2 -0

index f877a1ea564..33899a450eb 100644
@@ -998,6 +998,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	char *ssh_signing_key_file = NULL;
 	struct strbuf ssh_signature_filename = STRBUF_INIT;
 	const char *literal_key = NULL;
+	int literal_ssh_key = 0;
 
 	if (!signing_key || signing_key[0] == '\0')
 		return error(

@@ -1005,6 +1006,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 
 	if (is_literal_ssh_key(signing_key, &literal_key)) {
 		/* A literal ssh key */
+		literal_ssh_key = 1;
 		key_file = mks_tempfile_t(".git_signing_key_tmpXXXXXX");
 		if (!key_file)
 			return error_errno(

@@ -1036,11 +1038,14 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	}
 

gpg-interface.c +0 -0

 	sigchain_push(SIGPIPE, SIG_IGN);
 	ret = pipe_command(&signer, NULL, 0, NULL, 0, &signer_stderr, 0);
 

base-commit: 844ede312b4e988881b6e27e352f469d8ab80b2a

Quoted 7 lines

Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1270/radicle-dev/maint-v2
Pull-Request: https://github.com/git/git/pull/1270

Range-diff vs v1:

 1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
 -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent

This is a fairly useless range-diff.

Even when a range-diff shows the differences in the patches, mechanically generated range-diff can only show _what_ changed. It is helpful to explain the changes in your own words to highlight _why_ such changes are done, and this place after the "---" line and the diffstat we see below is the place to do so.

Does GitGitGadget allow its users to describe the differences since the previous iteration yourself?

Quoted 26 lines

 gpg-interface.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

gpg-interface.c +2 -0

index f877a1ea564..33899a450eb 100644
@@ -998,6 +998,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
      char *ssh_signing_key_file = NULL;
      struct strbuf ssh_signature_filename = STRBUF_INIT;
      const char *literal_key = NULL;
+     int literal_ssh_key = 0;
 
      if (!signing_key || signing_key[0] == '\0')
              return error(

@@ -1005,6 +1006,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 
      if (is_literal_ssh_key(signing_key, &literal_key)) {
              /* A literal ssh key */
+             literal_ssh_key = 1;
              key_file = mks_tempfile_t(".git_signing_key_tmpXXXXXX");
              if (!key_file)
                      return error_errno(

@@ -1036,11 +1038,14 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
      }
 

gpg-interface.c static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, · line 1044

Quoted 10 lines

gpg-interface.c +4 -5

      strvec_pushl(&signer.args, use_format->program,
-                  "-Y", "sign",
-                  "-n", "git",
-                  "-f", ssh_signing_key_file,
-                  buffer_file->filename.buf,
-                  NULL);
+                     "-Y", "sign",
+                     "-n", "git",
+                     "-f", ssh_signing_key_file,
+                     NULL);

Please avoid making a pointless indentation change like this.

We do
not pass filename yet with this pushl(), because ...

gpg-interface.c static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, · line 1047

Quoted 3 lines

gpg-interface.c +3 -0

+     if (literal_ssh_key) {
+             strvec_push(&signer.args, "-U");
+     }

... when we give a literal key, we want to insert "-U" in front, and then

gpg-interface.c static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, · line 1048

Quoted 1 line

gpg-interface.c +1 -0

+     strvec_push(&signer.args, buffer_file->filename.buf);

... the filename. Which makes sense.

The insertion of "-U" is a single statement as the body of a if() statement. We do not want {} around it, by the way.

Quoted 16 lines · Junio C Hamano

Quoted 7 lines

Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1270/radicle-dev/maint-v2
Pull-Request: https://github.com/git/git/pull/1270

Range-diff vs v1:

 1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
 -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent

This is a fairly useless range-diff.

Even when a range-diff shows the differences in the patches, mechanically generated range-diff can only show _what_ changed. It is helpful to explain the changes in your own words to highlight _why_ such changes are done, and this place after the "---" line and the diffstat we see below is the place to do so.

Quoted 2 lines

Does GitGitGadget allow its users to describe the differences since the previous iteration yourself?

No, I don't think it does. It got generated automatically without
giving me an opportunity to edit.

Yep, removed. It was largely accidental.
...
Removed the superfluous {}.

Thanks

Quoted 9 lines

We do
not pass filename yet with this pushl(), because ...

gpg-interface.c static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, · line 1047

Quoted 3 lines

gpg-interface.c +3 -0

+     if (literal_ssh_key) {
+             strvec_push(&signer.args, "-U");
+     }

... when we give a literal key, we want to insert "-U" in front, and then

Quoted 3 lines

Quoted 1 line

+ strvec_push(&signer.args, buffer_file->filename.buf);

... the filename. Which makes sense.

I'm not sure what you mean in this paragraph. If there's something
more that needs to be done, I'd appreciate it if you could rephrase it.

Quoted 13 lines · Junio C Hamano

Quoted 4 lines

Range-diff vs v1:

 1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
 -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent

This is a fairly useless range-diff.

Even when a range-diff shows the differences in the patches, mechanically generated range-diff can only show _what_ changed. It is helpful to explain the changes in your own words to highlight _why_ such changes are done, and this place after the "---" line and the diffstat we see below is the place to do so.

Quoted 2 lines

Does GitGitGadget allow its users to describe the differences since the previous iteration yourself?

No, I don't think it does. It got generated automatically without
giving me an opportunity to edit.

Quoted 14 lines · Adam Szkoda

Quoted 13 lines · Junio C Hamano

Quoted 4 lines

Range-diff vs v1:

 1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
 -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent

This is a fairly useless range-diff.

Even when a range-diff shows the differences in the patches, mechanically generated range-diff can only show _what_ changed. It is helpful to explain the changes in your own words to highlight _why_ such changes are done, and this place after the "---" line and the diffstat we see below is the place to do so.

Quoted 5 lines

Quoted 2 lines

Does GitGitGadget allow its users to describe the differences since the previous iteration yourself?

No, I don't think it does. It got generated automatically without
giving me an opportunity to edit.

Yes, the user can describe the differences since the previous iteration by editing the pull-request's description. Specifically, when ready to send a new iteration:

(1) force push the new iteration to the same branch on GitHub

(2) edit the pull-request description; this is the very first "comment" on the pull-request page; press the "..." button on that comment and choose the "Edit" menu item; revise the text to describe the changes since the previous revision, then press the "Update comment" button to save

(3) post a "/submit" comment to the pull-request to tell GitGitGadget to send the new revision to the Git mailing list

Quoted 14 lines · Eric Sunshine

Quoted 6 lines · Adam Szkoda

Quoted 2 lines · Junio C Hamano

Does GitGitGadget allow its users to describe the differences since the previous iteration yourself?

No, I don't think it does. It got generated automatically without
giving me an opportunity to edit.

Yes, the user can describe the differences since the previous iteration by editing the pull-request's description. Specifically, when ready to send a new iteration:

(1) force push the new iteration to the same branch on GitHub

Quoted 8 lines

(2) edit the pull-request description; this is the very first "comment" on the pull-request page; press the "..." button on that comment and choose the "Edit" menu item; revise the text to describe the changes since the previous revision, then press the "Update comment" button to save

(3) post a "/submit" comment to the pull-request to tell GitGitGadget to send the new revision to the Git mailing list

Thanks. I thought the above would make a good addition to our documentation set. Documentation/MyFirstContribution.txt does have this to say:

Next, go look at your pull request against GitGitGadget; you should see the CI has been kicked off again. Now while the CI is running is a good time for you to modify your description at the top of the pull request thread; it will be used again as the cover letter. You should use this space to describe what has changed since your previous version, so that your reviewers have some idea of what they're looking at. When the CI is done running, you can comment once more with /submit - GitGitGadget will automatically add a v2 mark to your changes.

before it talks about doing the "/submit" again. Expanding the above into a bulletted list form like you did might make it easier to follow through, perhaps? I dunno.