[PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commits. Building upon this functionality, a user may want to
re-sign these invalid commit signatures. This series introduces the
`re-sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatability structure would also
require us to reconstruct the compatability object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not competely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be adressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Thanks,
-Justin

Justin Tobler (2):
  commit: remove unused forward declaration
  fast-import: add mode to re-sign invalid commit signatures

 Documentation/git-fast-import.adoc |   3 +
 builtin/fast-export.c              |   6 ++
 builtin/fast-import.c              |  43 +++++++--
 commit.h                           |   2 -
 gpg-interface.c                    |   2 +
 gpg-interface.h                    |   1 +
 t/t9305-fast-import-signatures.sh  | 142 +++++++++++++++--------------
 7 files changed, 125 insertions(+), 74 deletions(-)


base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0

[PATCH 1/2] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0

[PATCH 2/2] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to re-sign the commit as opposed
to just stripping the signature. To faciliate this, introduce a
"re-sign-if-invalid" mode for the `--signed-commits` option.

Note that commits are re-signed using only the repository object format
hash algorithm. If a commit has an additional signature due to the
`compatObjectFormat` repository extension being set, the other signature
is stripped.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   3 +
 builtin/fast-export.c              |   6 ++
 builtin/fast-import.c              |  43 +++++++--
 gpg-interface.c                    |   2 +
 gpg-interface.h                    |   1 +
 t/t9305-fast-import-signatures.sh  | 142 +++++++++++++++--------------
 6 files changed, 125 insertions(+), 72 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..b902a6e2b0 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,9 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `re-sign-if-invalid` is the same as `strip-if-invalid`, but additionally the
+  commits with invalid signatures are signed again, so that old invalid
+  signatures are replaced with new valid ones.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..76fad1dec5 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_RESIGN_IF_INVALID:
+			die(_("'re-sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_RESIGN_IF_INVALID:
+				die(_("'re-sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..e34a373d2f 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2836,10 +2836,11 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void handle_invalid_signature(struct strbuf *new_data,
+				     struct signature_data *sig_sha1,
+				     struct signature_data *sig_sha256,
+				     struct strbuf *msg,
+				     enum sign_mode mode)
 {
 	struct strbuf tmp_buf = STRBUF_INIT;
 	struct signature_check signature_check = { 0 };
@@ -2866,6 +2867,30 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 			warning(_("stripping invalid signature for commit\n"
 				  "  allegedly by %s"), signer);
 
+		if (mode == SIGN_RESIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+			char *key = get_signing_key();
+
+			/*
+			 * Commits are resigned using the repository object
+			 * format hash algorithm only. Consequently if
+			 * extensions.compatObjectFormat is set, the
+			 * compatability hash is not currently used to
+			 * additionally sign the commit. If the commit payload
+			 * were reconstructed in the compatability format, it
+			 * would be possible to generate the other signature
+			 * accordingly though.
+			 */
+			strbuf_addstr(&payload, signature_check.payload);
+			sign_buffer(&payload, &signature, key);
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+			free(key);
+		}
+
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
 		strbuf_swap(new_data, &tmp_buf);
@@ -2927,6 +2952,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_RESIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3037,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_RESIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_invalid_signature(&new_data, &sig_sha1, &sig_sha256, &msg,
+					 signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3088,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_RESIGN_IF_INVALID:
+		die(_("'re-sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..e7eb42d9d6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1156,6 +1156,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
 		*mode = SIGN_STRIP;
 	else if (!strcmp(arg, "strip-if-invalid"))
 		*mode = SIGN_STRIP_IF_INVALID;
+	else if (!strcmp(arg, "re-sign-if-invalid"))
+		*mode = SIGN_RESIGN_IF_INVALID;
 	else
 		return -1;
 	return 0;
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..2ab2a21e1a 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -112,6 +112,7 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_RESIGN_IF_INVALID,
 };
 
 /*
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..b52fb75976 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,71 +103,81 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-
-	# Change the commit message, which invalidates the signature.
-	# The commit message length should not change though, otherwise the
-	# corresponding `data <length>` command would have to be changed too.
-	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
-
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
-
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING != $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
-
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
+for mode in strip-if-invalid re-sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "strip signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep "stripping invalid signature" log &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
 
 test_done
-- 
2.53.0

Re: [PATCH 2/2] fast-import: add mode to re-sign invalid commit signatures

[Patrick Steinhardt]

On Mon, Feb 23, 2026 at 01:41:46PM -0600, Justin Tobler wrote:
> With git-fast-import(1), handling of signed commits is controlled via
> the `--signed-commits=<mode>` option. When an invalid signature is
> encountered, a user may want the option to re-sign the commit as opposed
> to just stripping the signature. To faciliate this, introduce a
> "re-sign-if-invalid" mode for the `--signed-commits` option.
> 
> Note that commits are re-signed using only the repository object format
> hash algorithm. If a commit has an additional signature due to the
> `compatObjectFormat` repository extension being set, the other signature
> is stripped.

This part here might use some explanation why this part is not done so
that a future reader that ends up here doesn't have to wonder whether
this is done with intent, or whether this was done because it was hard
to do.

> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 479c4081da..b902a6e2b0 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> @@ -86,6 +86,9 @@ already trusted to run their own code.
>  * `strip-if-invalid` will check signatures and, if they are invalid,
>    will strip them and display a warning. The validation is performed
>    in the same way as linkgit:git-verify-commit[1] does it.
> +* `re-sign-if-invalid` is the same as `strip-if-invalid`, but additionally the
> +  commits with invalid signatures are signed again, so that old invalid
> +  signatures are replaced with new valid ones.

Okay. It's a bit curious to say it's the "same as `strip-if-invalid`",
but I get what you mean by this, and I think a user would, too.

> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index b8a7757cfd..e34a373d2f 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2836,10 +2836,11 @@ static void finalize_commit_buffer(struct strbuf *new_data,
>  	strbuf_addbuf(new_data, msg);
>  }
>  
> -static void handle_strip_if_invalid(struct strbuf *new_data,
> -				    struct signature_data *sig_sha1,
> -				    struct signature_data *sig_sha256,
> -				    struct strbuf *msg)
> +static void handle_invalid_signature(struct strbuf *new_data,
> +				     struct signature_data *sig_sha1,
> +				     struct signature_data *sig_sha256,
> +				     struct strbuf *msg,
> +				     enum sign_mode mode)
>  {
>  	struct strbuf tmp_buf = STRBUF_INIT;
>  	struct signature_check signature_check = { 0 };

Should we maybe call this `handle_signature_if_invalid()`? Otherwise it
sounds as if we already know the signature was invalid.

> @@ -2866,6 +2867,30 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
>  			warning(_("stripping invalid signature for commit\n"
>  				  "  allegedly by %s"), signer);

I wonder: does it still make sense to warn about those stripped
signatures in case we re-sign anyway?

> +		if (mode == SIGN_RESIGN_IF_INVALID) {
> +			struct strbuf signature = STRBUF_INIT;
> +			struct strbuf payload = STRBUF_INIT;
> +			char *key = get_signing_key();
> +
> +			/*
> +			 * Commits are resigned using the repository object

Poor commits. Maybe s/resigned/re-signed/?

> +			 * format hash algorithm only. Consequently if
> +			 * extensions.compatObjectFormat is set, the
> +			 * compatability hash is not currently used to
> +			 * additionally sign the commit. If the commit payload
> +			 * were reconstructed in the compatability format, it
> +			 * would be possible to generate the other signature
> +			 * accordingly though.
> +			 */

Same as in the commit message, we should document whether this is done
intentionally, or whether it may require more work going forward. If the
latter, it might make sense to add a NEEDSWORK comment.

I think meanwhile though it's okay that we don't handle compatibility
hashes yet.

> diff --git a/gpg-interface.c b/gpg-interface.c
> index 87fb6605fb..e7eb42d9d6 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -1156,6 +1156,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
>  		*mode = SIGN_STRIP;
>  	else if (!strcmp(arg, "strip-if-invalid"))
>  		*mode = SIGN_STRIP_IF_INVALID;
> +	else if (!strcmp(arg, "re-sign-if-invalid"))
> +		*mode = SIGN_RESIGN_IF_INVALID;
>  	else
>  		return -1;
>  	return 0;

One thing I wonder here is which signing key is actually in use, and how
the user would specify it. In git-commit(1) you can for example pass
"--gpg-sign=<key-id>" to specify the key. Do we want to allow the same
here, where you can pass "--signed-commits=re-sign-if-invalid[=<gpg-key>]"?

Thanks!

Patrick

Re: [PATCH 1/2] commit: remove unused forward declaration

[Patrick Steinhardt]

On Mon, Feb 23, 2026 at 01:41:45PM -0600, Justin Tobler wrote:
> In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
> `sign_with_header()` was removed, but its forward declaration in
> "commit.h" was left. Remove the unused declaration.

Indeed, the definition of that function doesn't exist anymore.

Patrick

Re: [PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[Christian Couder]

Hi,

On Mon, Feb 23, 2026 at 8:42 PM Justin Tobler <jltobler@gmail.com> wrote:
>
> Greetings,
>
> With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
> --signed-commits=<mode>, 2025-11-17), it became possible to remove
> invalid signatures from commits via git-fast-import(1) while maintaining
> valid commits. Building upon this functionality, a user may want to

s/valid commits/valid commit signatures/

> re-sign these invalid commit signatures. This series introduces the
> `re-sign-if-invalid` mode to do so accordingly.
>
> The newly added mode in this series currently ignores
> `extensions.compatObjectFormat` when generating the new signatures. From
> my understanding, to generate the compatability structure would also

Here and below: s/compatability/compatibility/

> require us to reconstruct the compatability object for the object being
> signed. I think this would be possible to do, but would require getting
> the mapped OIDs for the commit parents and tree. I'm not competely sure

s/competely/completely/

> of a good way to go about this yet though. I'm also not completely
> certain if this is something that should be adressed as part of this

s/adressed/addressed/

> series, or could be done later down the road. So for now I've opted to
> delay its implementation. I'm open going down the other route if that is
> preferred though.

That's a reasonable approach to me.

Thanks for taking over this.

Re: [PATCH 2/2] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/02/24 10:33AM, Patrick Steinhardt wrote:
> On Mon, Feb 23, 2026 at 01:41:46PM -0600, Justin Tobler wrote:
> > With git-fast-import(1), handling of signed commits is controlled via
> > the `--signed-commits=<mode>` option. When an invalid signature is
> > encountered, a user may want the option to re-sign the commit as opposed
> > to just stripping the signature. To faciliate this, introduce a
> > "re-sign-if-invalid" mode for the `--signed-commits` option.
> > 
> > Note that commits are re-signed using only the repository object format
> > hash algorithm. If a commit has an additional signature due to the
> > `compatObjectFormat` repository extension being set, the other signature
> > is stripped.
> 
> This part here might use some explanation why this part is not done so
> that a future reader that ends up here doesn't have to wonder whether
> this is done with intent, or whether this was done because it was hard
> to do.

Good point. I'll expand the explaination here in the next version.

> > diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> > index 479c4081da..b902a6e2b0 100644
> > --- a/Documentation/git-fast-import.adoc
> > +++ b/Documentation/git-fast-import.adoc
> > @@ -86,6 +86,9 @@ already trusted to run their own code.
> >  * `strip-if-invalid` will check signatures and, if they are invalid,
> >    will strip them and display a warning. The validation is performed
> >    in the same way as linkgit:git-verify-commit[1] does it.
> > +* `re-sign-if-invalid` is the same as `strip-if-invalid`, but additionally the
> > +  commits with invalid signatures are signed again, so that old invalid
> > +  signatures are replaced with new valid ones.
> 
> Okay. It's a bit curious to say it's the "same as `strip-if-invalid`",
> but I get what you mean by this, and I think a user would, too.

Ya, maybe it would be better to say that it is "similar to
`strip-if-invalid`". I'll try to rework the documentation here a little
bit in the next version.

> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index b8a7757cfd..e34a373d2f 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -2836,10 +2836,11 @@ static void finalize_commit_buffer(struct strbuf *new_data,
> >  	strbuf_addbuf(new_data, msg);
> >  }
> >  
> > -static void handle_strip_if_invalid(struct strbuf *new_data,
> > -				    struct signature_data *sig_sha1,
> > -				    struct signature_data *sig_sha256,
> > -				    struct strbuf *msg)
> > +static void handle_invalid_signature(struct strbuf *new_data,
> > +				     struct signature_data *sig_sha1,
> > +				     struct signature_data *sig_sha256,
> > +				     struct strbuf *msg,
> > +				     enum sign_mode mode)
> >  {
> >  	struct strbuf tmp_buf = STRBUF_INIT;
> >  	struct signature_check signature_check = { 0 };
> 
> Should we maybe call this `handle_signature_if_invalid()`? Otherwise it
> sounds as if we already know the signature was invalid.

That sounds better. Will adapt.

> 
> > @@ -2866,6 +2867,30 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
> >  			warning(_("stripping invalid signature for commit\n"
> >  				  "  allegedly by %s"), signer);
> 
> I wonder: does it still make sense to warn about those stripped
> signatures in case we re-sign anyway?

Ya, good point. I was originally thinking it would still make sense to
keep these messages since we are still stripping the signatures, but it
might be misleading if are also re-signing them. We could keep these,
but add an additional message if re-signing. That might be a little
noisy though. Maybe we just adapt the warning message when re-signing.

> > +		if (mode == SIGN_RESIGN_IF_INVALID) {
> > +			struct strbuf signature = STRBUF_INIT;
> > +			struct strbuf payload = STRBUF_INIT;
> > +			char *key = get_signing_key();
> > +
> > +			/*
> > +			 * Commits are resigned using the repository object
> 
> Poor commits. Maybe s/resigned/re-signed/?

Poor commits indeed, will change. XD

> > +			 * format hash algorithm only. Consequently if
> > +			 * extensions.compatObjectFormat is set, the
> > +			 * compatability hash is not currently used to
> > +			 * additionally sign the commit. If the commit payload
> > +			 * were reconstructed in the compatability format, it
> > +			 * would be possible to generate the other signature
> > +			 * accordingly though.
> > +			 */
> 
> Same as in the commit message, we should document whether this is done
> intentionally, or whether it may require more work going forward. If the
> latter, it might make sense to add a NEEDSWORK comment.

Ya, I think it should be possible to support compatibility hashes in the
future. I'll explain this better in a NEEDSWORK comment.

> I think meanwhile though it's okay that we don't handle compatibility
> hashes yet.
> 
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index 87fb6605fb..e7eb42d9d6 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -1156,6 +1156,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
> >  		*mode = SIGN_STRIP;
> >  	else if (!strcmp(arg, "strip-if-invalid"))
> >  		*mode = SIGN_STRIP_IF_INVALID;
> > +	else if (!strcmp(arg, "re-sign-if-invalid"))
> > +		*mode = SIGN_RESIGN_IF_INVALID;
> >  	else
> >  		return -1;
> >  	return 0;
> 
> One thing I wonder here is which signing key is actually in use, and how
> the user would specify it. In git-commit(1) you can for example pass
> "--gpg-sign=<key-id>" to specify the key. Do we want to allow the same
> here, where you can pass "--signed-commits=re-sign-if-invalid[=<gpg-key>]"?

This seems sensible. I'll explore this in the next version.

Thanks for the review. :)

-Justin

Re: [PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

On 2026-02-23 at 19:41:44, Justin Tobler wrote:
> Greetings,
> 
> With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
> --signed-commits=<mode>, 2025-11-17), it became possible to remove
> invalid signatures from commits via git-fast-import(1) while maintaining
> valid commits. Building upon this functionality, a user may want to
> re-sign these invalid commit signatures. This series introduces the
> `re-sign-if-invalid` mode to do so accordingly.
> 
> The newly added mode in this series currently ignores
> `extensions.compatObjectFormat` when generating the new signatures. From
> my understanding, to generate the compatability structure would also
> require us to reconstruct the compatability object for the object being
> signed. I think this would be possible to do, but would require getting
> the mapped OIDs for the commit parents and tree. I'm not competely sure
> of a good way to go about this yet though. I'm also not completely
> certain if this is something that should be adressed as part of this
> series, or could be done later down the road. So for now I've opted to
> delay its implementation. I'm open going down the other route if that is
> preferred though.

There's an API for converting object IDs to another algorithm:
`repo_oid_to_algop`.  If you want to convert a non-blob object, there's
`convert_object_file`, which will serialize the object in the other
format (blobs are invariant in the hash algorithm transformation, so
converting them is not necessary).  Those are present right now in the
codebase and using them would be a good idea.

If you want to test your code in interoperability mode, you can rebase
onto the `sha256-interop` branch of https://github.com/bk2204/git.git
and run with `GIT_TEST_DEFAULT_HASH=sha256:sha1`.

If you're _not_ going to implement that in interoperability mode, then
I'd rather you just die in that case so that the test fails and then I
or someone else will fix it.  `extensions.compatObjectFormat` is
presently experimental and the data formats will change, so nobody
should be relying on it working as it stands right now.  There _will_ be
more compatibility breakage coming in future series, for instance.

I _would_ recommend regardless that you add a test like in t7004's
"signed tag with embedded PGP message" if you apply this to tags as well
as commits.  That requires a special case in our interoperability code
(since it normally converts things that look like signatures, but when
we're _generating_ a tag, we don't want to do that since there are no
signatures yet) and making sure we do the same thing in fast-import will
avoid corruption in our conversions.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> If you're _not_ going to implement that in interoperability mode, then
> I'd rather you just die in that case so that the test fails and then I
> or someone else will fix it.  `extensions.compatObjectFormat` is
> presently experimental and the data formats will change, so nobody
> should be relying on it working as it stands right now.  There _will_ be
> more compatibility breakage coming in future series, for instance.

It sounds like a very prudent thing to do to die as unsupported.
Thanks!

> I _would_ recommend regardless that you add a test like in t7004's
> "signed tag with embedded PGP message" if you apply this to tags as well
> as commits.  That requires a special case in our interoperability code
> (since it normally converts things that look like signatures, but when
> we're _generating_ a tag, we don't want to do that since there are no
> signatures yet) and making sure we do the same thing in fast-import will
> avoid corruption in our conversions.

Re: [PATCH 0/2] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/02/24 10:41PM, brian m. carlson wrote:
> If you're _not_ going to implement that in interoperability mode, then
> I'd rather you just die in that case so that the test fails and then I
> or someone else will fix it.  `extensions.compatObjectFormat` is
> presently experimental and the data formats will change, so nobody
> should be relying on it working as it stands right now.  There _will_ be
> more compatibility breakage coming in future series, for instance.

That sounds very sensible. In the next version I'll update to instead
die() as unsupported if we attempt to re-sign commit signatures in
interoperability mode.

> I _would_ recommend regardless that you add a test like in t7004's
> "signed tag with embedded PGP message" if you apply this to tags as well
> as commits.  That requires a special case in our interoperability code
> (since it normally converts things that look like signatures, but when
> we're _generating_ a tag, we don't want to do that since there are no
> signatures yet) and making sure we do the same thing in fast-import will
> avoid corruption in our conversions.

Thanks, I'll look into this. This patch series currently only applies
this new mode to commits, but I plan to tackle tag signatures in a
separate followup series.

Thanks,
-Justin

[PATCH v2 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commit signatures. Building upon this functionality, a user may
want to re-sign these invalid commit signatures. This series introduces
the `re-sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatibility structure would also
require us to reconstruct the compatibility object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not completely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be addressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Changes since V1:
- Improved commit messages and comments to better explain why
  interoperability mode is not currently supported.
- Clarified documentation for re-sign-if-invalid mode.
- Renamed `handle_invalid_signature()` to `handle_signature_if_invalid()`.
- Added warning messages specific to commit resigning.
- Fixed some small typos.
- Added support for explicitly specifying the signing key ID via
  `--signed-commits=re-sign-if-invalid[=<keyid>]` similar to how it can
  specified in git-commit(1).
- We now die() as unsupported when attempting to re-sign an invalid
  commit signature in interoperability mode.
- We now die() when failing to re-sign a commit.

Thanks,
-Justin

Justin Tobler (3):
  commit: remove unused forward declaration
  gpg-interface: introduce sign_buffer_with_key()
  fast-import: add mode to re-sign invalid commit signatures

 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              |  79 ++++++++++++----
 commit.c                           |  16 +---
 commit.h                           |   2 -
 gpg-interface.c                    |  36 ++++++--
 gpg-interface.h                    |  14 ++-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 8 files changed, 205 insertions(+), 94 deletions(-)

Range-diff against v1:
1:  0d00b72ee0 = 1:  0d00b72ee0 commit: remove unused forward declaration
-:  ---------- > 2:  499025532c gpg-interface: introduce sign_buffer_with_key()
2:  16e4022616 ! 3:  bea1a42eb9 fast-import: add mode to re-sign invalid commit signatures
    @@ Commit message
         With git-fast-import(1), handling of signed commits is controlled via
         the `--signed-commits=<mode>` option. When an invalid signature is
         encountered, a user may want the option to re-sign the commit as opposed
    -    to just stripping the signature. To faciliate this, introduce a
    -    "re-sign-if-invalid" mode for the `--signed-commits` option.
    +    to just stripping the signature. To facilitate this, introduce a
    +    "re-sign-if-invalid" mode for the `--signed-commits` option. Optionally,
    +    a key ID may be explicitly provided in the form
    +    `re-sign-if-invalid[=<keyid>]` to specify which signing key should be
    +    used when re-signing invalid commit signatures.
     
    -    Note that commits are re-signed using only the repository object format
    -    hash algorithm. If a commit has an additional signature due to the
    -    `compatObjectFormat` repository extension being set, the other signature
    -    is stripped.
    +    Note that to properly support interoperability mode when re-signing
    +    commit signatures, the commit buffer must be created in both the
    +    repository and compatability object formats to generate the appropriate
    +    signatures accordingly. As currently implemented, the commit buffer for
    +    the compatability object format is not reconstructed and thus re-signing
    +    commits in interoperability mode is not yet supported. Support may be
    +    added in the future.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
    @@ Documentation/git-fast-import.adoc: already trusted to run their own code.
      * `strip-if-invalid` will check signatures and, if they are invalid,
        will strip them and display a warning. The validation is performed
        in the same way as linkgit:git-verify-commit[1] does it.
    -+* `re-sign-if-invalid` is the same as `strip-if-invalid`, but additionally the
    -+  commits with invalid signatures are signed again, so that old invalid
    -+  signatures are replaced with new valid ones.
    ++* `re-sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
    ++  commit signatures and replaces invalid signatures with newly created ones.
    ++  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
    ++  used for re-signing; otherwise the configured default signing key is used.
      
      Options for Frontends
      ~~~~~~~~~~~~~~~~~~~~~
     
      ## builtin/fast-export.c ##
    +@@ builtin/fast-export.c: static int parse_opt_sign_mode(const struct option *opt,
    + 	if (unset)
    + 		return 0;
    + 
    +-	if (parse_sign_mode(arg, val))
    ++	if (parse_sign_mode(arg, val, NULL))
    + 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
    + 
    + 	return 0;
     @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
      		case SIGN_STRIP_IF_INVALID:
      			die(_("'strip-if-invalid' is not a valid mode for "
    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
      			}
     
      ## builtin/fast-import.c ##
    +@@ builtin/fast-import.c: static const char *global_prefix;
    + 
    + static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
    + static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
    ++static const char *signed_commit_keyid;
    + 
    + /* Memory pools */
    + static struct mem_pool fi_mem_pool = {
     @@ builtin/fast-import.c: static void finalize_commit_buffer(struct strbuf *new_data,
      	strbuf_addbuf(new_data, msg);
      }
    @@ builtin/fast-import.c: static void finalize_commit_buffer(struct strbuf *new_dat
     -				    struct signature_data *sig_sha1,
     -				    struct signature_data *sig_sha256,
     -				    struct strbuf *msg)
    -+static void handle_invalid_signature(struct strbuf *new_data,
    -+				     struct signature_data *sig_sha1,
    -+				     struct signature_data *sig_sha256,
    -+				     struct strbuf *msg,
    -+				     enum sign_mode mode)
    ++static void handle_signature_if_invalid(struct strbuf *new_data,
    ++					struct signature_data *sig_sha1,
    ++					struct signature_data *sig_sha256,
    ++					struct strbuf *msg,
    ++					enum sign_mode mode)
      {
      	struct strbuf tmp_buf = STRBUF_INIT;
      	struct signature_check signature_check = { 0 };
     @@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_data,
    - 			warning(_("stripping invalid signature for commit\n"
    - 				  "  allegedly by %s"), signer);
    + 		const char *subject;
    + 		int subject_len = find_commit_subject(msg->buf, &subject);
      
    -+		if (mode == SIGN_RESIGN_IF_INVALID) {
    +-		if (subject_len > 100)
    +-			warning(_("stripping invalid signature for commit '%.100s...'\n"
    +-				  "  allegedly by %s"), subject, signer);
    +-		else if (subject_len > 0)
    +-			warning(_("stripping invalid signature for commit '%.*s'\n"
    +-				  "  allegedly by %s"), subject_len, subject, signer);
    +-		else
    +-			warning(_("stripping invalid signature for commit\n"
    +-				  "  allegedly by %s"), signer);
    ++		if (mode == SIGN_STRIP_IF_INVALID) {
    ++			if (subject_len > 100)
    ++				warning(_("stripping invalid signature for commit '%.100s...'\n"
    ++					  "  allegedly by %s"), subject, signer);
    ++			else if (subject_len > 0)
    ++				warning(_("stripping invalid signature for commit '%.*s'\n"
    ++					  "  allegedly by %s"), subject_len, subject, signer);
    ++			else
    ++				warning(_("stripping invalid signature for commit\n"
    ++					  "  allegedly by %s"), signer);
    ++		} else if (mode == SIGN_RESIGN_IF_INVALID) {
     +			struct strbuf signature = STRBUF_INIT;
     +			struct strbuf payload = STRBUF_INIT;
    -+			char *key = get_signing_key();
    ++
    ++			if (subject_len > 100)
    ++				warning(_("re-signing invalid signature for commit '%.100s...'\n"
    ++					  "  allegedly by %s"), subject, signer);
    ++			else if (subject_len > 0)
    ++				warning(_("re-signing invalid signature for commit '%.*s'\n"
    ++					  "  allegedly by %s"), subject_len, subject, signer);
    ++			else
    ++				warning(_("re-signing invalid signature for commit\n"
    ++					  "  allegedly by %s"), signer);
     +
     +			/*
    -+			 * Commits are resigned using the repository object
    -+			 * format hash algorithm only. Consequently if
    -+			 * extensions.compatObjectFormat is set, the
    -+			 * compatability hash is not currently used to
    -+			 * additionally sign the commit. If the commit payload
    -+			 * were reconstructed in the compatability format, it
    -+			 * would be possible to generate the other signature
    -+			 * accordingly though.
    ++			 * NEEDSWORK: To properly support interoperability mode
    ++			 * when re-signing commit signatures, the commit buffer
    ++			 * must be provided in both the repository and
    ++			 * compatability object formats. As currently
    ++			 * implemented, only the repository object format is
    ++			 * considered meaning compatability signatures cannot be
    ++			 * generated. Thus, attempting to re-sign commit
    ++			 * signatures in interoperability mode is currently
    ++			 * unsupported.
     +			 */
    ++			if (the_repository->compat_hash_algo)
    ++				die(_("re-signing signatures in interoperability mode is unsupported"));
    ++
     +			strbuf_addstr(&payload, signature_check.payload);
    -+			sign_buffer(&payload, &signature, key);
    ++			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
    ++				die(_("failed to sign commit object"));
     +			add_header_signature(new_data, &signature, the_hash_algo);
     +
     +			strbuf_release(&signature);
     +			strbuf_release(&payload);
    -+			free(key);
     +		}
    -+
    + 
      		finalize_commit_buffer(new_data, NULL, NULL, msg);
      	} else {
    - 		strbuf_swap(new_data, &tmp_buf);
     @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      			/* fallthru */
      		case SIGN_VERBATIM:
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
     +	     signed_commit_mode == SIGN_RESIGN_IF_INVALID) &&
      	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
     -		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
    -+		handle_invalid_signature(&new_data, &sig_sha1, &sig_sha256, &msg,
    -+					 signed_commit_mode);
    ++		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
    ++					    &msg, signed_commit_mode);
      	else
      		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
      
    @@ builtin/fast-import.c: static void handle_tag_signature(struct strbuf *msg, cons
      	default:
      		BUG("invalid signed_tag_mode value %d from tag '%s'",
      		    signed_tag_mode, name);
    +@@ builtin/fast-import.c: static int parse_one_option(const char *option)
    + 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
    + 		option_export_pack_edges(option);
    + 	} else if (skip_prefix(option, "signed-commits=", &option)) {
    +-		if (parse_sign_mode(option, &signed_commit_mode))
    ++		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
    + 			usagef(_("unknown --signed-commits mode '%s'"), option);
    + 	} else if (skip_prefix(option, "signed-tags=", &option)) {
    +-		if (parse_sign_mode(option, &signed_tag_mode))
    ++		if (parse_sign_mode(option, &signed_tag_mode, NULL))
    + 			usagef(_("unknown --signed-tags mode '%s'"), option);
    + 	} else if (!strcmp(option, "quiet")) {
    + 		show_stats = 0;
     
      ## gpg-interface.c ##
    -@@ gpg-interface.c: int parse_sign_mode(const char *arg, enum sign_mode *mode)
    +@@ gpg-interface.c: static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
    + 	return ret;
    + }
    + 
    +-int parse_sign_mode(const char *arg, enum sign_mode *mode)
    ++int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
    + {
    +-	if (!strcmp(arg, "abort"))
    ++	if (!strcmp(arg, "abort")) {
    + 		*mode = SIGN_ABORT;
    +-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
    ++	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
    + 		*mode = SIGN_VERBATIM;
    +-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
    ++	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
    + 		*mode = SIGN_WARN_VERBATIM;
    +-	else if (!strcmp(arg, "warn-strip"))
    ++	} else if (!strcmp(arg, "warn-strip")) {
    + 		*mode = SIGN_WARN_STRIP;
    +-	else if (!strcmp(arg, "strip"))
    ++	} else if (!strcmp(arg, "strip")) {
      		*mode = SIGN_STRIP;
    - 	else if (!strcmp(arg, "strip-if-invalid"))
    +-	else if (!strcmp(arg, "strip-if-invalid"))
    ++	} else if (!strcmp(arg, "strip-if-invalid")) {
      		*mode = SIGN_STRIP_IF_INVALID;
    -+	else if (!strcmp(arg, "re-sign-if-invalid"))
    +-	else
    ++	} else if (!strcmp(arg, "re-sign-if-invalid")) {
     +		*mode = SIGN_RESIGN_IF_INVALID;
    - 	else
    ++	} else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) {
    ++		*mode = SIGN_RESIGN_IF_INVALID;
    ++		if (keyid)
    ++			*keyid = arg;
    ++	} else {
      		return -1;
    ++	}
      	return 0;
    + }
     
      ## gpg-interface.h ##
     @@ gpg-interface.h: enum sign_mode {
    @@ gpg-interface.h: enum sign_mode {
      };
      
      /*
    +  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
    +- * otherwise.
    ++ * otherwise. If the parsed mode is SIGN_RESIGN_IF_INVALID and GPG key provided
    ++ * in the arguments in the form `re-sign-if-invalid=<keyid>`, the key-ID is
    ++ * parsed into `char **keyid`.
    +  */
    +-int parse_sign_mode(const char *arg, enum sign_mode *mode);
    ++int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
    + 
    + #endif
     
      ## t/t9305-fast-import-signatures.sh ##
     @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     -'
     -
     -test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
    --	rm -rf new &&
    --	git init new &&
    --
    --	git fast-export --signed-commits=verbatim openpgp-signing >output &&
    --
    --	# Change the commit message, which invalidates the signature.
    --	# The commit message length should not change though, otherwise the
    --	# corresponding `data <length>` command would have to be changed too.
    --	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
    --
    --	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
    --
    --	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
    --	test $OPENPGP_SIGNING != $IMPORTED &&
    --	git -C new cat-file commit "$IMPORTED" >actual &&
    --	test_grep ! -E "^gpgsig" actual &&
    --	test_grep "stripping invalid signature" log
    --'
    --
    --test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
    --	rm -rf new &&
    --	git init new &&
    --
    --	git fast-export --signed-commits=verbatim x509-signing >output &&
    --	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    --	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
    --	test $X509_SIGNING = $IMPORTED &&
    --	git -C new cat-file commit "$IMPORTED" >actual &&
    --	test_grep -E "^gpgsig(-sha256)? " actual &&
    --	test_must_be_empty log
    --'
    --
    --test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
    --	rm -rf new &&
    --	git init new &&
    --
    --	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    --
    --	git fast-export --signed-commits=verbatim ssh-signing >output &&
    --	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    --	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
    --	test $SSH_SIGNING = $IMPORTED &&
    --	git -C new cat-file commit "$IMPORTED" >actual &&
    --	test_grep -E "^gpgsig(-sha256)? " actual &&
    --	test_must_be_empty log
    --'
     +for mode in strip-if-invalid re-sign-if-invalid
     +do
     +	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +		test_must_be_empty log
     +	'
     +
    -+	test_expect_success GPG "strip signature invalidated by message change with --signed-commits=$mode" '
    ++	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
     +		rm -rf new &&
     +		git init new &&
     +
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
     +		test $OPENPGP_SIGNING != $IMPORTED &&
     +		git -C new cat-file commit "$IMPORTED" >actual &&
    -+		test_grep "stripping invalid signature" log &&
     +
     +		if test "$mode" = strip-if-invalid
     +		then
    ++			test_grep "stripping invalid signature" log &&
     +			test_grep ! -E "^gpgsig" actual
     +		else
    ++			test_grep "re-signing invalid signature" log &&
     +			test_grep -E "^gpgsig(-sha256)? " actual &&
     +			git -C new verify-commit "$IMPORTED"
     +		fi
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +		test_must_be_empty log
     +	'
     +done
    ++
    ++test_expect_success GPGSSH "re-sign invalid commit with explicit keyid" '
    + 	rm -rf new &&
    + 	git init new &&
    + 
    +@@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip signature invalidated by message change with --si
    + 	# corresponding `data <length>` command would have to be changed too.
    + 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
    + 
    +-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
    ++	# Configure the target repository with an invalid default signing key.
    ++	test_config -C new user.signingkey "not-a-real-key-id" &&
    ++	test_config -C new gpg.format ssh &&
    ++	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    ++	test_must_fail git -C new fast-import --quiet \
    ++		--signed-commits=re-sign-if-invalid <modified >/dev/null 2>&1 &&
    ++
    ++	# Import using explicitly provided signing key.
    ++	git -C new fast-import --quiet \
    ++		--signed-commits=re-sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
    + 
    + 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
    + 	test $OPENPGP_SIGNING != $IMPORTED &&
    + 	git -C new cat-file commit "$IMPORTED" >actual &&
    +-	test_grep ! -E "^gpgsig" actual &&
    +-	test_grep "stripping invalid signature" log
    +-'
    +-
    +-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
    +-	rm -rf new &&
    +-	git init new &&
    +-
    +-	git fast-export --signed-commits=verbatim x509-signing >output &&
    +-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    +-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
    +-	test $X509_SIGNING = $IMPORTED &&
    +-	git -C new cat-file commit "$IMPORTED" >actual &&
    + 	test_grep -E "^gpgsig(-sha256)? " actual &&
    +-	test_must_be_empty log
    +-'
    +-
    +-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
    +-	rm -rf new &&
    +-	git init new &&
    +-
    +-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    +-
    +-	git fast-export --signed-commits=verbatim ssh-signing >output &&
    +-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    +-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
    +-	test $SSH_SIGNING = $IMPORTED &&
    +-	git -C new cat-file commit "$IMPORTED" >actual &&
    +-	test_grep -E "^gpgsig(-sha256)? " actual &&
    +-	test_must_be_empty log
    ++	git -C new verify-commit "$IMPORTED"
    + '
      
      test_done

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6

[PATCH v2 1/3] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0.381.g628a66ccf6

[PATCH v2 2/3] gpg-interface: introduce sign_buffer_with_key()

[Justin Tobler]

The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when resigning invalid commit signatures.
Introduce `sign_buffer_with_key()` to centralize signing key resolution
in gpg-interface to allow callers to reuse the same behavior without
duplicating logic.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.c        | 16 ++--------------
 gpg-interface.c | 13 +++++++++++++
 gpg-interface.h |  7 +++++++
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/commit.c b/commit.c
index d16ae73345..1677b1ef25 100644
--- a/commit.c
+++ b/commit.c
@@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 	return 0;
 }
 
-static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
-{
-	char *keyid_to_free = NULL;
-	int ret = 0;
-	if (!keyid || !*keyid)
-		keyid = keyid_to_free = get_signing_key();
-	if (sign_buffer(buf, sig, keyid))
-		ret = -1;
-	free(keyid_to_free);
-	return ret;
-}
-
 int parse_signed_commit(const struct commit *commit,
 			struct strbuf *payload, struct strbuf *signature,
 			const struct git_hash_algo *algop)
@@ -1737,7 +1725,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
 	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
-	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
+	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
 		result = -1;
 		goto out;
 	}
@@ -1769,7 +1757,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
 
-		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
+		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
 			result = -1;
 			goto out;
 		}
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..a72fa35061 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -980,6 +980,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	return use_format->sign_buffer(buffer, signature, signing_key);
 }
 
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key)
+{
+	char *keyid_to_free = NULL;
+	int ret = 0;
+	if (!signing_key || !*signing_key)
+		signing_key = keyid_to_free = get_signing_key();
+	if (sign_buffer(buffer, signature, signing_key))
+		ret = -1;
+	free(keyid_to_free);
+	return ret;
+}
+
 /*
  * Strip CR from the line endings, in case we are on Windows.
  * NEEDSWORK: make it trim only CRs before LFs and rename
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..a32741aeda 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+/*
+ * Similar to `sign_buffer()`, but uses the default configured signing key as
+ * returned by `get_signing_key()` when the provided "signing_key" is NULL or
+ * empty. Returns 0 on success, non-zero on failure.
+ */
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key);
 
 /*
  * Returns corresponding string in lowercase for a given member of
-- 
2.53.0.381.g628a66ccf6

[PATCH v2 3/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to re-sign the commit as opposed
to just stripping the signature. To facilitate this, introduce a
"re-sign-if-invalid" mode for the `--signed-commits` option. Optionally,
a key ID may be explicitly provided in the form
`re-sign-if-invalid[=<keyid>]` to specify which signing key should be
used when re-signing invalid commit signatures.

Note that to properly support interoperability mode when re-signing
commit signatures, the commit buffer must be created in both the
repository and compatability object formats to generate the appropriate
signatures accordingly. As currently implemented, the commit buffer for
the compatability object format is not reconstructed and thus re-signing
commits in interoperability mode is not yet supported. Support may be
added in the future.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              |  79 ++++++++++++----
 gpg-interface.c                    |  23 +++--
 gpg-interface.h                    |   7 +-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 6 files changed, 183 insertions(+), 78 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..08f7d5d89a 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,10 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `re-sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
+  commit signatures and replaces invalid signatures with newly created ones.
+  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
+  used for re-signing; otherwise the configured default signing key is used.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..0ab8465ae3 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val))
+	if (parse_sign_mode(arg, val, NULL))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_RESIGN_IF_INVALID:
+			die(_("'re-sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_RESIGN_IF_INVALID:
+				die(_("'re-sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..f6bd8556f5 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -190,6 +190,7 @@ static const char *global_prefix;
 
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+static const char *signed_commit_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2836,10 +2837,11 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void handle_signature_if_invalid(struct strbuf *new_data,
+					struct signature_data *sig_sha1,
+					struct signature_data *sig_sha256,
+					struct strbuf *msg,
+					enum sign_mode mode)
 {
 	struct strbuf tmp_buf = STRBUF_INIT;
 	struct signature_check signature_check = { 0 };
@@ -2856,15 +2858,52 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 		const char *subject;
 		int subject_len = find_commit_subject(msg->buf, &subject);
 
-		if (subject_len > 100)
-			warning(_("stripping invalid signature for commit '%.100s...'\n"
-				  "  allegedly by %s"), subject, signer);
-		else if (subject_len > 0)
-			warning(_("stripping invalid signature for commit '%.*s'\n"
-				  "  allegedly by %s"), subject_len, subject, signer);
-		else
-			warning(_("stripping invalid signature for commit\n"
-				  "  allegedly by %s"), signer);
+		if (mode == SIGN_STRIP_IF_INVALID) {
+			if (subject_len > 100)
+				warning(_("stripping invalid signature for commit '%.100s...'\n"
+					  "  allegedly by %s"), subject, signer);
+			else if (subject_len > 0)
+				warning(_("stripping invalid signature for commit '%.*s'\n"
+					  "  allegedly by %s"), subject_len, subject, signer);
+			else
+				warning(_("stripping invalid signature for commit\n"
+					  "  allegedly by %s"), signer);
+		} else if (mode == SIGN_RESIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+
+			if (subject_len > 100)
+				warning(_("re-signing invalid signature for commit '%.100s...'\n"
+					  "  allegedly by %s"), subject, signer);
+			else if (subject_len > 0)
+				warning(_("re-signing invalid signature for commit '%.*s'\n"
+					  "  allegedly by %s"), subject_len, subject, signer);
+			else
+				warning(_("re-signing invalid signature for commit\n"
+					  "  allegedly by %s"), signer);
+
+			/*
+			 * NEEDSWORK: To properly support interoperability mode
+			 * when re-signing commit signatures, the commit buffer
+			 * must be provided in both the repository and
+			 * compatability object formats. As currently
+			 * implemented, only the repository object format is
+			 * considered meaning compatability signatures cannot be
+			 * generated. Thus, attempting to re-sign commit
+			 * signatures in interoperability mode is currently
+			 * unsupported.
+			 */
+			if (the_repository->compat_hash_algo)
+				die(_("re-signing signatures in interoperability mode is unsupported"));
+
+			strbuf_addstr(&payload, signature_check.payload);
+			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
+				die(_("failed to sign commit object"));
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+		}
 
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
@@ -2927,6 +2966,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_RESIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3051,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_RESIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
+					    &msg, signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3102,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_RESIGN_IF_INVALID:
+		die(_("'re-sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3649,10 +3694,10 @@ static int parse_one_option(const char *option)
 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
 		option_export_pack_edges(option);
 	} else if (skip_prefix(option, "signed-commits=", &option)) {
-		if (parse_sign_mode(option, &signed_commit_mode))
+		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode))
+		if (parse_sign_mode(option, &signed_tag_mode, NULL))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/gpg-interface.c b/gpg-interface.c
index a72fa35061..e028984546 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1155,21 +1155,28 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	return ret;
 }
 
-int parse_sign_mode(const char *arg, enum sign_mode *mode)
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 {
-	if (!strcmp(arg, "abort"))
+	if (!strcmp(arg, "abort")) {
 		*mode = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
 		*mode = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
 		*mode = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
+	} else if (!strcmp(arg, "warn-strip")) {
 		*mode = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
+	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
-	else if (!strcmp(arg, "strip-if-invalid"))
+	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
-	else
+	} else if (!strcmp(arg, "re-sign-if-invalid")) {
+		*mode = SIGN_RESIGN_IF_INVALID;
+	} else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) {
+		*mode = SIGN_RESIGN_IF_INVALID;
+		if (keyid)
+			*keyid = arg;
+	} else {
 		return -1;
+	}
 	return 0;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index a32741aeda..8f1fad43e9 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -119,12 +119,15 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_RESIGN_IF_INVALID,
 };
 
 /*
  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
- * otherwise.
+ * otherwise. If the parsed mode is SIGN_RESIGN_IF_INVALID and GPG key provided
+ * in the arguments in the form `re-sign-if-invalid=<keyid>`, the key-ID is
+ * parsed into `char **keyid`.
  */
-int parse_sign_mode(const char *arg, enum sign_mode *mode);
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
 
 #endif
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..2a3f04b42d 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,26 +103,85 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
+for mode in strip-if-invalid re-sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep "stripping invalid signature" log &&
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep "re-signing invalid signature" log &&
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH "re-sign invalid commit with explicit keyid" '
 	rm -rf new &&
 	git init new &&
 
@@ -133,41 +192,22 @@ test_expect_success GPG 'strip signature invalidated by message change with --si
 	# corresponding `data <length>` command would have to be changed too.
 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
+	# Configure the target repository with an invalid default signing key.
+	test_config -C new user.signingkey "not-a-real-key-id" &&
+	test_config -C new gpg.format ssh &&
+	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C new fast-import --quiet \
+		--signed-commits=re-sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C new fast-import --quiet \
+		--signed-commits=re-sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
 
 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
 	test $OPENPGP_SIGNING != $IMPORTED &&
 	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
 	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
-
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
+	git -C new verify-commit "$IMPORTED"
 '
 
 test_done
-- 
2.53.0.381.g628a66ccf6

Re: [PATCH v2 2/3] gpg-interface: introduce sign_buffer_with_key()

[Christian Couder]

On Fri, Mar 6, 2026 at 9:54 PM Justin Tobler <jltobler@gmail.com> wrote:
>
> The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
> logic to get the default configured signing key when a key is not
> provided and handles generating the commit signature accordingly. This
> signing operation is not really specific to commits as any arbitrary
> buffer can be signed. Also, in a subsequent commit, this same logic is
> reused by git-fast-import(1) when resigning invalid commit signatures.

Nit: s/resigning/re-signing/

> Introduce `sign_buffer_with_key()` to centralize signing key resolution
> in gpg-interface to allow callers to reuse the same behavior without
> duplicating logic.

Nit: I think it would be a bit clearer if the change was described as:

- moving the `sign_commit_to_strbuf()` helper from "commit.c" to
"gpg-interface.c",
- renaming it to `sign_buffer_with_key()`, and
- exporting it (so it can later be used by both "commit.c" and
"builtin/fast-import.c").

Or did I miss something?

Re: [PATCH v2 3/3] fast-import: add mode to re-sign invalid commit signatures

[Christian Couder]

On Fri, Mar 6, 2026 at 9:54 PM Justin Tobler <jltobler@gmail.com> wrote:

> @@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
>                 case SIGN_STRIP_IF_INVALID:
>                         die(_("'strip-if-invalid' is not a valid mode for "
>                               "git fast-export with --signed-commits=<mode>"));
> +               case SIGN_RESIGN_IF_INVALID:

Everywhere in this patch, I think "RE_SIGN" might be more consistent
than "RESIGN" for this name.

> +                       die(_("'re-sign-if-invalid' is not a valid mode for "
> +                             "git fast-export with --signed-commits=<mode>"));

[...]

> @@ -2856,15 +2858,52 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
>                 const char *subject;
>                 int subject_len = find_commit_subject(msg->buf, &subject);
>
> -               if (subject_len > 100)
> -                       warning(_("stripping invalid signature for commit '%.100s...'\n"
> -                                 "  allegedly by %s"), subject, signer);
> -               else if (subject_len > 0)
> -                       warning(_("stripping invalid signature for commit '%.*s'\n"
> -                                 "  allegedly by %s"), subject_len, subject, signer);
> -               else
> -                       warning(_("stripping invalid signature for commit\n"
> -                                 "  allegedly by %s"), signer);
> +               if (mode == SIGN_STRIP_IF_INVALID) {
> +                       if (subject_len > 100)
> +                               warning(_("stripping invalid signature for commit '%.100s...'\n"
> +                                         "  allegedly by %s"), subject, signer);
> +                       else if (subject_len > 0)
> +                               warning(_("stripping invalid signature for commit '%.*s'\n"
> +                                         "  allegedly by %s"), subject_len, subject, signer);
> +                       else
> +                               warning(_("stripping invalid signature for commit\n"
> +                                         "  allegedly by %s"), signer);
> +               } else if (mode == SIGN_RESIGN_IF_INVALID) {
> +                       struct strbuf signature = STRBUF_INIT;
> +                       struct strbuf payload = STRBUF_INIT;
> +
> +                       if (subject_len > 100)
> +                               warning(_("re-signing invalid signature for commit '%.100s...'\n"
> +                                         "  allegedly by %s"), subject, signer);
> +                       else if (subject_len > 0)
> +                               warning(_("re-signing invalid signature for commit '%.*s'\n"
> +                                         "  allegedly by %s"), subject_len, subject, signer);
> +                       else
> +                               warning(_("re-signing invalid signature for commit\n"
> +                                         "  allegedly by %s"), signer);

Maybe a helper function could be used to avoid duplicating the warning logic.

> +                       /*
> +                        * NEEDSWORK: To properly support interoperability mode
> +                        * when re-signing commit signatures, the commit buffer
> +                        * must be provided in both the repository and
> +                        * compatability object formats. As currently

s/compatability/compatibility/

> +                        * implemented, only the repository object format is
> +                        * considered meaning compatability signatures cannot be

s/compatability/compatibility/

> +                        * generated. Thus, attempting to re-sign commit
> +                        * signatures in interoperability mode is currently
> +                        * unsupported.
> +                        */
> +                       if (the_repository->compat_hash_algo)
> +                               die(_("re-signing signatures in interoperability mode is unsupported"));
> +
> +                       strbuf_addstr(&payload, signature_check.payload);
> +                       if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
> +                               die(_("failed to sign commit object"));
> +                       add_header_signature(new_data, &signature, the_hash_algo);
> +
> +                       strbuf_release(&signature);
> +                       strbuf_release(&payload);
> +               }

Except for these small issues and the few nits in the previous patch,
this looks good to me. Thanks for working on it.

Re: [PATCH v2 2/3] gpg-interface: introduce sign_buffer_with_key()

[Justin Tobler]

On 26/03/10 10:01AM, Christian Couder wrote:
> On Fri, Mar 6, 2026 at 9:54 PM Justin Tobler <jltobler@gmail.com> wrote:
> >
> > The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
> > logic to get the default configured signing key when a key is not
> > provided and handles generating the commit signature accordingly. This
> > signing operation is not really specific to commits as any arbitrary
> > buffer can be signed. Also, in a subsequent commit, this same logic is
> > reused by git-fast-import(1) when resigning invalid commit signatures.
> 
> Nit: s/resigning/re-signing/

Will fix.

> > Introduce `sign_buffer_with_key()` to centralize signing key resolution
> > in gpg-interface to allow callers to reuse the same behavior without
> > duplicating logic.
> 
> Nit: I think it would be a bit clearer if the change was described as:
> 
> - moving the `sign_commit_to_strbuf()` helper from "commit.c" to
> "gpg-interface.c",
> - renaming it to `sign_buffer_with_key()`, and
> - exporting it (so it can later be used by both "commit.c" and
> "builtin/fast-import.c").
> 
> Or did I miss something?

That's correct. I'll update the commit message in the next version to
try to be a bit more clear here. Thanks.

-Justin

Re: [PATCH v2 3/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/03/10 10:27AM, Christian Couder wrote:
> On Fri, Mar 6, 2026 at 9:54 PM Justin Tobler <jltobler@gmail.com> wrote:
> 
> > @@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
> >                 case SIGN_STRIP_IF_INVALID:
> >                         die(_("'strip-if-invalid' is not a valid mode for "
> >                               "git fast-export with --signed-commits=<mode>"));
> > +               case SIGN_RESIGN_IF_INVALID:
> 
> Everywhere in this patch, I think "RE_SIGN" might be more consistent
> than "RESIGN" for this name.

That's fair, will change.

> > +                       die(_("'re-sign-if-invalid' is not a valid mode for "
> > +                             "git fast-export with --signed-commits=<mode>"));
> 
> [...]
> 
> > @@ -2856,15 +2858,52 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
> >                 const char *subject;
> >                 int subject_len = find_commit_subject(msg->buf, &subject);
> >
> > -               if (subject_len > 100)
> > -                       warning(_("stripping invalid signature for commit '%.100s...'\n"
> > -                                 "  allegedly by %s"), subject, signer);
> > -               else if (subject_len > 0)
> > -                       warning(_("stripping invalid signature for commit '%.*s'\n"
> > -                                 "  allegedly by %s"), subject_len, subject, signer);
> > -               else
> > -                       warning(_("stripping invalid signature for commit\n"
> > -                                 "  allegedly by %s"), signer);
> > +               if (mode == SIGN_STRIP_IF_INVALID) {
> > +                       if (subject_len > 100)
> > +                               warning(_("stripping invalid signature for commit '%.100s...'\n"
> > +                                         "  allegedly by %s"), subject, signer);
> > +                       else if (subject_len > 0)
> > +                               warning(_("stripping invalid signature for commit '%.*s'\n"
> > +                                         "  allegedly by %s"), subject_len, subject, signer);
> > +                       else
> > +                               warning(_("stripping invalid signature for commit\n"
> > +                                         "  allegedly by %s"), signer);
> > +               } else if (mode == SIGN_RESIGN_IF_INVALID) {
> > +                       struct strbuf signature = STRBUF_INIT;
> > +                       struct strbuf payload = STRBUF_INIT;
> > +
> > +                       if (subject_len > 100)
> > +                               warning(_("re-signing invalid signature for commit '%.100s...'\n"
> > +                                         "  allegedly by %s"), subject, signer);
> > +                       else if (subject_len > 0)
> > +                               warning(_("re-signing invalid signature for commit '%.*s'\n"
> > +                                         "  allegedly by %s"), subject_len, subject, signer);
> > +                       else
> > +                               warning(_("re-signing invalid signature for commit\n"
> > +                                         "  allegedly by %s"), signer);
> 
> Maybe a helper function could be used to avoid duplicating the warning logic.

Ya, I could extract this out to a helper that prints the appropriate
warning. Due to being translated, I'm not quite sure if there would be a
good way to make the message strings more generic though. Will update in
the next version.

-Justin

[PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commit signatures. Building upon this functionality, a user may
want to re-sign these invalid commit signatures. This series introduces
the `re-sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatibility structure would also
require us to reconstruct the compatibility object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not completely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be addressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Changes since V2:
- Adapted commit message in second patch to improve clarity.
- Fixed typos.
- Renamed SIGN_RESIGN_IF_INVALID to SIGN_RE_SIGN_IF_INVALID.
- Created separate helper function to handle printing invalid signature
  warnings.

Changes since V1:
- Improved commit messages and comments to better explain why
  interoperability mode is not currently supported.
- Clarified documentation for re-sign-if-invalid mode.
- Renamed `handle_invalid_signature()` to `handle_signature_if_invalid()`.
- Added warning messages specific to commit resigning.
- Fixed some small typos.
- Added support for explicitly specifying the signing key ID via
  `--signed-commits=re-sign-if-invalid[=<keyid>]` similar to how it can
  specified in git-commit(1).
- We now die() as unsupported when attempting to re-sign an invalid
  commit signature in interoperability mode.
- We now die() when failing to re-sign a commit.

Thanks,
-Justin

Justin Tobler (3):
  commit: remove unused forward declaration
  gpg-interface: introduce sign_buffer_with_key()
  fast-import: add mode to re-sign invalid commit signatures

 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 ++++++++++++++++-----
 commit.c                           |  16 +---
 commit.h                           |   2 -
 gpg-interface.c                    |  36 ++++++--
 gpg-interface.h                    |  14 ++-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 8 files changed, 222 insertions(+), 99 deletions(-)

Range-diff against v2:
1:  0d00b72ee0 = 1:  0d00b72ee0 commit: remove unused forward declaration
2:  499025532c ! 2:  0b0a06347d gpg-interface: introduce sign_buffer_with_key()
    @@ Commit message
         provided and handles generating the commit signature accordingly. This
         signing operation is not really specific to commits as any arbitrary
         buffer can be signed. Also, in a subsequent commit, this same logic is
    -    reused by git-fast-import(1) when resigning invalid commit signatures.
    -    Introduce `sign_buffer_with_key()` to centralize signing key resolution
    -    in gpg-interface to allow callers to reuse the same behavior without
    -    duplicating logic.
    +    reused by git-fast-import(1) when re-signing invalid commit signatures.
    +
    +    Move the `sign_commit_to_strbuf()` helper from "commit.c" to
    +    "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
    +    this function so it can be used by "commit.c" and
    +    "builtin/fast-import.c" in the subsequent commit.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
3:  bea1a42eb9 ! 3:  57a27ccc61 fast-import: add mode to re-sign invalid commit signatures
    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
      		case SIGN_STRIP_IF_INVALID:
      			die(_("'strip-if-invalid' is not a valid mode for "
      			      "git fast-export with --signed-commits=<mode>"));
    -+		case SIGN_RESIGN_IF_INVALID:
    ++		case SIGN_RE_SIGN_IF_INVALID:
     +			die(_("'re-sign-if-invalid' is not a valid mode for "
     +			      "git fast-export with --signed-commits=<mode>"));
      		default:
    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
      			case SIGN_STRIP_IF_INVALID:
      				die(_("'strip-if-invalid' is not a valid mode for "
      				      "git fast-export with --signed-tags=<mode>"));
    -+			case SIGN_RESIGN_IF_INVALID:
    ++			case SIGN_RE_SIGN_IF_INVALID:
     +				die(_("'re-sign-if-invalid' is not a valid mode for "
     +				      "git fast-export with --signed-tags=<mode>"));
      			default:
    @@ builtin/fast-import.c: static void finalize_commit_buffer(struct strbuf *new_dat
     -				    struct signature_data *sig_sha1,
     -				    struct signature_data *sig_sha256,
     -				    struct strbuf *msg)
    ++static void warn_invalid_signature(struct signature_check *check,
    ++				   const char *msg, enum sign_mode mode)
    + {
    +-	struct strbuf tmp_buf = STRBUF_INIT;
    +-	struct signature_check signature_check = { 0 };
    +-	int ret;
    +-
    +-	/* Check signature in a temporary commit buffer */
    +-	strbuf_addbuf(&tmp_buf, new_data);
    +-	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
    +-	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
    +-
    +-	if (ret) {
    +-		const char *signer = signature_check.signer ?
    +-			signature_check.signer : _("unknown");
    +-		const char *subject;
    +-		int subject_len = find_commit_subject(msg->buf, &subject);
    ++	const char *signer = check->signer ? check->signer : _("unknown");
    ++	const char *subject;
    ++	int subject_len = find_commit_subject(msg, &subject);
    + 
    ++	switch (mode) {
    ++	case SIGN_STRIP_IF_INVALID:
    + 		if (subject_len > 100)
    + 			warning(_("stripping invalid signature for commit '%.100s...'\n"
    + 				  "  allegedly by %s"), subject, signer);
    +@@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_data,
    + 		else
    + 			warning(_("stripping invalid signature for commit\n"
    + 				  "  allegedly by %s"), signer);
    ++		break;
    ++	case SIGN_RE_SIGN_IF_INVALID:
    ++		if (subject_len > 100)
    ++			warning(_("re-signing invalid signature for commit '%.100s...'\n"
    ++				  "  allegedly by %s"), subject, signer);
    ++		else if (subject_len > 0)
    ++			warning(_("re-signing invalid signature for commit '%.*s'\n"
    ++				  "  allegedly by %s"), subject_len, subject, signer);
    ++		else
    ++			warning(_("re-signing invalid signature for commit\n"
    ++				  "  allegedly by %s"), signer);
    ++		break;
    ++	default:
    ++		BUG("unsupported signing mode");
    ++	}
    ++}
    ++
     +static void handle_signature_if_invalid(struct strbuf *new_data,
     +					struct signature_data *sig_sha1,
     +					struct signature_data *sig_sha256,
     +					struct strbuf *msg,
     +					enum sign_mode mode)
    - {
    - 	struct strbuf tmp_buf = STRBUF_INIT;
    - 	struct signature_check signature_check = { 0 };
    -@@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_data,
    - 		const char *subject;
    - 		int subject_len = find_commit_subject(msg->buf, &subject);
    - 
    --		if (subject_len > 100)
    --			warning(_("stripping invalid signature for commit '%.100s...'\n"
    --				  "  allegedly by %s"), subject, signer);
    --		else if (subject_len > 0)
    --			warning(_("stripping invalid signature for commit '%.*s'\n"
    --				  "  allegedly by %s"), subject_len, subject, signer);
    --		else
    --			warning(_("stripping invalid signature for commit\n"
    --				  "  allegedly by %s"), signer);
    -+		if (mode == SIGN_STRIP_IF_INVALID) {
    -+			if (subject_len > 100)
    -+				warning(_("stripping invalid signature for commit '%.100s...'\n"
    -+					  "  allegedly by %s"), subject, signer);
    -+			else if (subject_len > 0)
    -+				warning(_("stripping invalid signature for commit '%.*s'\n"
    -+					  "  allegedly by %s"), subject_len, subject, signer);
    -+			else
    -+				warning(_("stripping invalid signature for commit\n"
    -+					  "  allegedly by %s"), signer);
    -+		} else if (mode == SIGN_RESIGN_IF_INVALID) {
    ++{
    ++	struct strbuf tmp_buf = STRBUF_INIT;
    ++	struct signature_check signature_check = { 0 };
    ++	int ret;
    ++
    ++	/* Check signature in a temporary commit buffer */
    ++	strbuf_addbuf(&tmp_buf, new_data);
    ++	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
    ++	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
    ++
    ++	if (ret) {
    ++		warn_invalid_signature(&signature_check, msg->buf, mode);
    ++
    ++		if (mode == SIGN_RE_SIGN_IF_INVALID) {
     +			struct strbuf signature = STRBUF_INIT;
     +			struct strbuf payload = STRBUF_INIT;
     +
    -+			if (subject_len > 100)
    -+				warning(_("re-signing invalid signature for commit '%.100s...'\n"
    -+					  "  allegedly by %s"), subject, signer);
    -+			else if (subject_len > 0)
    -+				warning(_("re-signing invalid signature for commit '%.*s'\n"
    -+					  "  allegedly by %s"), subject_len, subject, signer);
    -+			else
    -+				warning(_("re-signing invalid signature for commit\n"
    -+					  "  allegedly by %s"), signer);
    -+
     +			/*
     +			 * NEEDSWORK: To properly support interoperability mode
     +			 * when re-signing commit signatures, the commit buffer
     +			 * must be provided in both the repository and
    -+			 * compatability object formats. As currently
    ++			 * compatibility object formats. As currently
     +			 * implemented, only the repository object format is
    -+			 * considered meaning compatability signatures cannot be
    ++			 * considered meaning compatibility signatures cannot be
     +			 * generated. Thus, attempting to re-sign commit
     +			 * signatures in interoperability mode is currently
     +			 * unsupported.
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      			/* fallthru */
      		case SIGN_VERBATIM:
      		case SIGN_STRIP_IF_INVALID:
    -+		case SIGN_RESIGN_IF_INVALID:
    ++		case SIGN_RE_SIGN_IF_INVALID:
      			import_one_signature(&sig_sha1, &sig_sha256, v);
      			break;
      
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      
     -	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
     +	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
    -+	     signed_commit_mode == SIGN_RESIGN_IF_INVALID) &&
    ++	     signed_commit_mode == SIGN_RE_SIGN_IF_INVALID) &&
      	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
     -		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
     +		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
    @@ builtin/fast-import.c: static void handle_tag_signature(struct strbuf *msg, cons
      	case SIGN_STRIP_IF_INVALID:
      		die(_("'strip-if-invalid' is not a valid mode for "
      		      "git fast-import with --signed-tags=<mode>"));
    -+	case SIGN_RESIGN_IF_INVALID:
    ++	case SIGN_RE_SIGN_IF_INVALID:
     +		die(_("'re-sign-if-invalid' is not a valid mode for "
     +		      "git fast-import with --signed-tags=<mode>"));
      	default:
    @@ gpg-interface.c: static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf
      		*mode = SIGN_STRIP_IF_INVALID;
     -	else
     +	} else if (!strcmp(arg, "re-sign-if-invalid")) {
    -+		*mode = SIGN_RESIGN_IF_INVALID;
    ++		*mode = SIGN_RE_SIGN_IF_INVALID;
     +	} else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) {
    -+		*mode = SIGN_RESIGN_IF_INVALID;
    ++		*mode = SIGN_RE_SIGN_IF_INVALID;
     +		if (keyid)
     +			*keyid = arg;
     +	} else {
    @@ gpg-interface.h: enum sign_mode {
      	SIGN_WARN_STRIP,
      	SIGN_STRIP,
      	SIGN_STRIP_IF_INVALID,
    -+	SIGN_RESIGN_IF_INVALID,
    ++	SIGN_RE_SIGN_IF_INVALID,
      };
      
      /*
       * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
     - * otherwise.
    -+ * otherwise. If the parsed mode is SIGN_RESIGN_IF_INVALID and GPG key provided
    ++ * otherwise. If the parsed mode is SIGN_RE_SIGN_IF_INVALID and GPG key provided
     + * in the arguments in the form `re-sign-if-invalid=<keyid>`, the key-ID is
     + * parsed into `char **keyid`.
       */

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6

[PATCH v3 1/3] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0.381.g628a66ccf6

[PATCH v3 2/3] gpg-interface: introduce sign_buffer_with_key()

[Justin Tobler]

The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when re-signing invalid commit signatures.

Move the `sign_commit_to_strbuf()` helper from "commit.c" to
"gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
this function so it can be used by "commit.c" and
"builtin/fast-import.c" in the subsequent commit.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.c        | 16 ++--------------
 gpg-interface.c | 13 +++++++++++++
 gpg-interface.h |  7 +++++++
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/commit.c b/commit.c
index d16ae73345..1677b1ef25 100644
--- a/commit.c
+++ b/commit.c
@@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 	return 0;
 }
 
-static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
-{
-	char *keyid_to_free = NULL;
-	int ret = 0;
-	if (!keyid || !*keyid)
-		keyid = keyid_to_free = get_signing_key();
-	if (sign_buffer(buf, sig, keyid))
-		ret = -1;
-	free(keyid_to_free);
-	return ret;
-}
-
 int parse_signed_commit(const struct commit *commit,
 			struct strbuf *payload, struct strbuf *signature,
 			const struct git_hash_algo *algop)
@@ -1737,7 +1725,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
 	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
-	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
+	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
 		result = -1;
 		goto out;
 	}
@@ -1769,7 +1757,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
 
-		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
+		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
 			result = -1;
 			goto out;
 		}
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..a72fa35061 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -980,6 +980,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	return use_format->sign_buffer(buffer, signature, signing_key);
 }
 
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key)
+{
+	char *keyid_to_free = NULL;
+	int ret = 0;
+	if (!signing_key || !*signing_key)
+		signing_key = keyid_to_free = get_signing_key();
+	if (sign_buffer(buffer, signature, signing_key))
+		ret = -1;
+	free(keyid_to_free);
+	return ret;
+}
+
 /*
  * Strip CR from the line endings, in case we are on Windows.
  * NEEDSWORK: make it trim only CRs before LFs and rename
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..a32741aeda 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+/*
+ * Similar to `sign_buffer()`, but uses the default configured signing key as
+ * returned by `get_signing_key()` when the provided "signing_key" is NULL or
+ * empty. Returns 0 on success, non-zero on failure.
+ */
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key);
 
 /*
  * Returns corresponding string in lowercase for a given member of
-- 
2.53.0.381.g628a66ccf6

[PATCH v3 3/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to re-sign the commit as opposed
to just stripping the signature. To facilitate this, introduce a
"re-sign-if-invalid" mode for the `--signed-commits` option. Optionally,
a key ID may be explicitly provided in the form
`re-sign-if-invalid[=<keyid>]` to specify which signing key should be
used when re-signing invalid commit signatures.

Note that to properly support interoperability mode when re-signing
commit signatures, the commit buffer must be created in both the
repository and compatability object formats to generate the appropriate
signatures accordingly. As currently implemented, the commit buffer for
the compatability object format is not reconstructed and thus re-signing
commits in interoperability mode is not yet supported. Support may be
added in the future.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 ++++++++++++++++-----
 gpg-interface.c                    |  23 +++--
 gpg-interface.h                    |   7 +-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 6 files changed, 200 insertions(+), 83 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..08f7d5d89a 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,10 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `re-sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
+  commit signatures and replaces invalid signatures with newly created ones.
+  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
+  used for re-signing; otherwise the configured default signing key is used.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..2067613267 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val))
+	if (parse_sign_mode(arg, val, NULL))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_RE_SIGN_IF_INVALID:
+			die(_("'re-sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_RE_SIGN_IF_INVALID:
+				die(_("'re-sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..472d9ab712 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -190,6 +190,7 @@ static const char *global_prefix;
 
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+static const char *signed_commit_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2836,26 +2837,15 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void warn_invalid_signature(struct signature_check *check,
+				   const char *msg, enum sign_mode mode)
 {
-	struct strbuf tmp_buf = STRBUF_INIT;
-	struct signature_check signature_check = { 0 };
-	int ret;
-
-	/* Check signature in a temporary commit buffer */
-	strbuf_addbuf(&tmp_buf, new_data);
-	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
-	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
-
-	if (ret) {
-		const char *signer = signature_check.signer ?
-			signature_check.signer : _("unknown");
-		const char *subject;
-		int subject_len = find_commit_subject(msg->buf, &subject);
+	const char *signer = check->signer ? check->signer : _("unknown");
+	const char *subject;
+	int subject_len = find_commit_subject(msg, &subject);
 
+	switch (mode) {
+	case SIGN_STRIP_IF_INVALID:
 		if (subject_len > 100)
 			warning(_("stripping invalid signature for commit '%.100s...'\n"
 				  "  allegedly by %s"), subject, signer);
@@ -2865,6 +2855,67 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 		else
 			warning(_("stripping invalid signature for commit\n"
 				  "  allegedly by %s"), signer);
+		break;
+	case SIGN_RE_SIGN_IF_INVALID:
+		if (subject_len > 100)
+			warning(_("re-signing invalid signature for commit '%.100s...'\n"
+				  "  allegedly by %s"), subject, signer);
+		else if (subject_len > 0)
+			warning(_("re-signing invalid signature for commit '%.*s'\n"
+				  "  allegedly by %s"), subject_len, subject, signer);
+		else
+			warning(_("re-signing invalid signature for commit\n"
+				  "  allegedly by %s"), signer);
+		break;
+	default:
+		BUG("unsupported signing mode");
+	}
+}
+
+static void handle_signature_if_invalid(struct strbuf *new_data,
+					struct signature_data *sig_sha1,
+					struct signature_data *sig_sha256,
+					struct strbuf *msg,
+					enum sign_mode mode)
+{
+	struct strbuf tmp_buf = STRBUF_INIT;
+	struct signature_check signature_check = { 0 };
+	int ret;
+
+	/* Check signature in a temporary commit buffer */
+	strbuf_addbuf(&tmp_buf, new_data);
+	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
+	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
+
+	if (ret) {
+		warn_invalid_signature(&signature_check, msg->buf, mode);
+
+		if (mode == SIGN_RE_SIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+
+			/*
+			 * NEEDSWORK: To properly support interoperability mode
+			 * when re-signing commit signatures, the commit buffer
+			 * must be provided in both the repository and
+			 * compatibility object formats. As currently
+			 * implemented, only the repository object format is
+			 * considered meaning compatibility signatures cannot be
+			 * generated. Thus, attempting to re-sign commit
+			 * signatures in interoperability mode is currently
+			 * unsupported.
+			 */
+			if (the_repository->compat_hash_algo)
+				die(_("re-signing signatures in interoperability mode is unsupported"));
+
+			strbuf_addstr(&payload, signature_check.payload);
+			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
+				die(_("failed to sign commit object"));
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+		}
 
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
@@ -2927,6 +2978,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_RE_SIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3063,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_RE_SIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
+					    &msg, signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3114,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_RE_SIGN_IF_INVALID:
+		die(_("'re-sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3649,10 +3706,10 @@ static int parse_one_option(const char *option)
 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
 		option_export_pack_edges(option);
 	} else if (skip_prefix(option, "signed-commits=", &option)) {
-		if (parse_sign_mode(option, &signed_commit_mode))
+		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode))
+		if (parse_sign_mode(option, &signed_tag_mode, NULL))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/gpg-interface.c b/gpg-interface.c
index a72fa35061..2570b641f2 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1155,21 +1155,28 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	return ret;
 }
 
-int parse_sign_mode(const char *arg, enum sign_mode *mode)
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 {
-	if (!strcmp(arg, "abort"))
+	if (!strcmp(arg, "abort")) {
 		*mode = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
 		*mode = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
 		*mode = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
+	} else if (!strcmp(arg, "warn-strip")) {
 		*mode = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
+	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
-	else if (!strcmp(arg, "strip-if-invalid"))
+	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
-	else
+	} else if (!strcmp(arg, "re-sign-if-invalid")) {
+		*mode = SIGN_RE_SIGN_IF_INVALID;
+	} else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) {
+		*mode = SIGN_RE_SIGN_IF_INVALID;
+		if (keyid)
+			*keyid = arg;
+	} else {
 		return -1;
+	}
 	return 0;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index a32741aeda..e9f451f366 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -119,12 +119,15 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_RE_SIGN_IF_INVALID,
 };
 
 /*
  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
- * otherwise.
+ * otherwise. If the parsed mode is SIGN_RE_SIGN_IF_INVALID and GPG key provided
+ * in the arguments in the form `re-sign-if-invalid=<keyid>`, the key-ID is
+ * parsed into `char **keyid`.
  */
-int parse_sign_mode(const char *arg, enum sign_mode *mode);
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
 
 #endif
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..2a3f04b42d 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,26 +103,85 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
+for mode in strip-if-invalid re-sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep "stripping invalid signature" log &&
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep "re-signing invalid signature" log &&
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH "re-sign invalid commit with explicit keyid" '
 	rm -rf new &&
 	git init new &&
 
@@ -133,41 +192,22 @@ test_expect_success GPG 'strip signature invalidated by message change with --si
 	# corresponding `data <length>` command would have to be changed too.
 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
+	# Configure the target repository with an invalid default signing key.
+	test_config -C new user.signingkey "not-a-real-key-id" &&
+	test_config -C new gpg.format ssh &&
+	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C new fast-import --quiet \
+		--signed-commits=re-sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C new fast-import --quiet \
+		--signed-commits=re-sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
 
 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
 	test $OPENPGP_SIGNING != $IMPORTED &&
 	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
 	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
-
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
+	git -C new verify-commit "$IMPORTED"
 '
 
 test_done
-- 
2.53.0.381.g628a66ccf6

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
> --signed-commits=<mode>, 2025-11-17), it became possible to remove
> invalid signatures from commits via git-fast-import(1) while maintaining
> valid commit signatures. Building upon this functionality, a user may
> want to re-sign these invalid commit signatures. This series introduces
> the `re-sign-if-invalid` mode to do so accordingly.

I know that this "re-sign" used to be "resign", and the update is
indeed a replacement, but I wonder if we can just say "sign"?

When we see a signature on an object we are rewriting, we either
"strip" it, or we "sign" it (afresh).  It is not like we are
retaining the old signature, and signing on top of it.  We are
discarding the old one so there is no difference from signing the
object that never had a signature, no?

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/03/10 01:49PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
> > --signed-commits=<mode>, 2025-11-17), it became possible to remove
> > invalid signatures from commits via git-fast-import(1) while maintaining
> > valid commit signatures. Building upon this functionality, a user may
> > want to re-sign these invalid commit signatures. This series introduces
> > the `re-sign-if-invalid` mode to do so accordingly.
> 
> I know that this "re-sign" used to be "resign", and the update is
> indeed a replacement, but I wonder if we can just say "sign"?
> 
> When we see a signature on an object we are rewriting, we either
> "strip" it, or we "sign" it (afresh).  It is not like we are
> retaining the old signature, and signing on top of it.  We are
> discarding the old one so there is no difference from signing the
> object that never had a signature, no?

From my perspective, "re-sign" implies that the signature was previously
signed, but we are now going to sign it again. Indeed, the resulting
commit signing is functionally the same as if the object never had a
previous signature though. Also, "if-invalid" already implies that the
object is signed, but its signature is invalid. So it could be argued
that "re-sign" is already redundant.

Ultimately, I don't feel super strongly, but I can send another version
that changes this option to "sign-if-invalid". It's probably a bit
simpler this way too. I guess the enum value would need to be changed to
"SIGN_SIGN_IF_INVALID"? Or maybe just "SIGN_IF_INVALID"?

Thanks,
-Justin

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> From my perspective, "re-sign" implies that the signature was previously
> signed, but we are now going to sign it again. Indeed, the resulting
> commit signing is functionally the same as if the object never had a
> previous signature though. Also, "if-invalid" already implies that the
> object is signed, but its signature is invalid. So it could be argued
> that "re-sign" is already redundant.

Yup.  if-invalid part indeed was why I thought "re-" was redundant.

Also, if a project is redoing its history with such a bulk
operation, I wonder if it _still_ makes sense to tie this re-signing
to the --signed-{tags,commits} option.  Adding signature to commits
that were not signed is not covered well with the
"--signed-commits=<mode>" option.

A project may have required that all commits and tags to be signed,
in which case "--signed-*=sign-if-invalid" would create a new
history with everything freshly signed, but if the original history
has signed and unsigned commits, and if they want to sign all the
objects while rewriting their history, they may find it more handy
if we let them do --signed-commits=strip-if-invalid --sign-commits
i.e., drop the invalid ones and make sure all commits are signed.

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/03/10 02:20PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > From my perspective, "re-sign" implies that the signature was previously
> > signed, but we are now going to sign it again. Indeed, the resulting
> > commit signing is functionally the same as if the object never had a
> > previous signature though. Also, "if-invalid" already implies that the
> > object is signed, but its signature is invalid. So it could be argued
> > that "re-sign" is already redundant.
> 
> Yup.  if-invalid part indeed was why I thought "re-" was redundant.
> 
> Also, if a project is redoing its history with such a bulk
> operation, I wonder if it _still_ makes sense to tie this re-signing
> to the --signed-{tags,commits} option.  Adding signature to commits
> that were not signed is not covered well with the
> "--signed-commits=<mode>" option.

Ya, the --signed-{tags,commits} option is really only intended to
specify how already signed objects should be handled. Adding a mode to
sign unsigned objects likely wouldn't fit well. I do think this
"re-signing" mode still makes sense though since it is limited to
the subset of objects that were previously signed and the signature
invalid.

> A project may have required that all commits and tags to be signed,
> in which case "--signed-*=sign-if-invalid" would create a new
> history with everything freshly signed, but if the original history
> has signed and unsigned commits, and if they want to sign all the
> objects while rewriting their history, they may find it more handy
> if we let them do --signed-commits=strip-if-invalid --sign-commits
> i.e., drop the invalid ones and make sure all commits are signed.

This certainly seems like a reasonable use case, but if we want to
support leaving previously unsigned objects unsigned too,
`--signed-commits=strip-if-invalid --signed-commits` wouldn't be
granular enough. My thinking is that users may want such targeted object
re-signing when bulk rewriting history via tools such as
git-filter-repo. I do think that it could make sense to still add a
separate `--signed-commits` option in the future though that targets the
remaining unsigned objects.

Thanks,
-Justin

Re: [PATCH v3 1/3] commit: remove unused forward declaration

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
> `sign_with_header()` was removed, but its forward declaration in
> "commit.h" was left. Remove the unused declaration.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  commit.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/commit.h b/commit.h
> index 1635de418b..f0c38cb444 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
>  int run_commit_hook(int editor_is_used, const char *index_file,
>  		    int *invoked_hook, const char *name, ...);
>  
> -/* Sign a commit or tag buffer, storing the result in a header. */
> -int sign_with_header(struct strbuf *buf, const char *keyid);
>  /* Parse the signature out of a header. */
>  int parse_buffer_signed_by_header(const char *buffer,
>  				  unsigned long size,

Nice and obvious clean-up.

Re: [PATCH v3 2/3] gpg-interface: introduce sign_buffer_with_key()

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
> logic to get the default configured signing key when a key is not
> provided and handles generating the commit signature accordingly. This
> signing operation is not really specific to commits as any arbitrary
> buffer can be signed. Also, in a subsequent commit, this same logic is
> reused by git-fast-import(1) when re-signing invalid commit signatures.
>
> Move the `sign_commit_to_strbuf()` helper from "commit.c" to
> "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
> this function so it can be used by "commit.c" and
> "builtin/fast-import.c" in the subsequent commit.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
>  commit.c        | 16 ++--------------
>  gpg-interface.c | 13 +++++++++++++
>  gpg-interface.h |  7 +++++++
>  3 files changed, 22 insertions(+), 14 deletions(-)

Sennsible restructuring that makes the machinery easier to reuse.
Updated function is named more appropriately for public consumption.

Overall a very welcome preparation step.

> diff --git a/commit.c b/commit.c
> index d16ae73345..1677b1ef25 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
>  	return 0;
>  }
>  
> -static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
> -{
> -	char *keyid_to_free = NULL;
> -	int ret = 0;
> -	if (!keyid || !*keyid)
> -		keyid = keyid_to_free = get_signing_key();
> -	if (sign_buffer(buf, sig, keyid))
> -		ret = -1;
> -	free(keyid_to_free);
> -	return ret;
> -}
> -
>  int parse_signed_commit(const struct commit *commit,
>  			struct strbuf *payload, struct strbuf *signature,
>  			const struct git_hash_algo *algop)
> @@ -1737,7 +1725,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		oidcpy(&parent_buf[i++], &p->item->object.oid);
>  
>  	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
> -	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
> +	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
>  		result = -1;
>  		goto out;
>  	}
> @@ -1769,7 +1757,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
>  		free_commit_extra_headers(compat_extra);
>  		free(mapped_parents);
>  
> -		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
> +		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
>  			result = -1;
>  			goto out;
>  		}
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 87fb6605fb..a72fa35061 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -980,6 +980,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
>  	return use_format->sign_buffer(buffer, signature, signing_key);
>  }
>  
> +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> +			 const char *signing_key)
> +{
> +	char *keyid_to_free = NULL;
> +	int ret = 0;
> +	if (!signing_key || !*signing_key)
> +		signing_key = keyid_to_free = get_signing_key();
> +	if (sign_buffer(buffer, signature, signing_key))
> +		ret = -1;
> +	free(keyid_to_free);
> +	return ret;
> +}
> +
>  /*
>   * Strip CR from the line endings, in case we are on Windows.
>   * NEEDSWORK: make it trim only CRs before LFs and rename
> diff --git a/gpg-interface.h b/gpg-interface.h
> index 789d1ffac4..a32741aeda 100644
> --- a/gpg-interface.h
> +++ b/gpg-interface.h
> @@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
>  int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
>  		const char *signing_key);
>  
> +/*
> + * Similar to `sign_buffer()`, but uses the default configured signing key as
> + * returned by `get_signing_key()` when the provided "signing_key" is NULL or
> + * empty. Returns 0 on success, non-zero on failure.
> + */
> +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> +			 const char *signing_key);
>  
>  /*
>   * Returns corresponding string in lowercase for a given member of

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> This certainly seems like a reasonable use case, but if we want to
> support leaving previously unsigned objects unsigned too,
> `--signed-commits=strip-if-invalid --signed-commits` wouldn't be
> granular enough.

Yes, --signed-commits=(re-)sign-if-invalid is a perfect match for
that use case.  I am just saying that if you add the machinery
needed to re-sign, you would be able to reuse it to sign objects
that weren't signed in the first place, so that is wherea yet
another feature "--sign-commits=all" may fit.

Re: [PATCH v3 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/03/10 03:39PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > This certainly seems like a reasonable use case, but if we want to
> > support leaving previously unsigned objects unsigned too,
> > `--signed-commits=strip-if-invalid --signed-commits` wouldn't be
> > granular enough.
> 
> Yes, --signed-commits=(re-)sign-if-invalid is a perfect match for
> that use case.  I am just saying that if you add the machinery
> needed to re-sign, you would be able to reuse it to sign objects
> that weren't signed in the first place, so that is wherea yet
> another feature "--sign-commits=all" may fit.

Ah ok. Ya, adding a signed-commits mode to cover signing all rewritten
objects could make sense. I am planning to also add an
--abort-if-invalid mode in a followup series. I'll may explore adding
this other mode too.

Thanks,
-Justin

[PATCH v4 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commit signatures. Building upon this functionality, a user may
want to re-sign these invalid commit signatures. This series introduces
the `sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatibility structure would also
require us to reconstruct the compatibility object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not completely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be addressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Changes since V3:
- Rename the `re-sign-if-invalid` mode to `sign-if-invalid`. The
  "if-invalid" already implies the signatures was previously signed
  making "re-sign" redundant.

Changes since V2:
- Adapted commit message in second patch to improve clarity.
- Fixed typos.
- Renamed SIGN_RESIGN_IF_INVALID to SIGN_RE_SIGN_IF_INVALID.
- Created separate helper function to handle printing invalid signature
  warnings.

Changes since V1:
- Improved commit messages and comments to better explain why
  interoperability mode is not currently supported.
- Clarified documentation for re-sign-if-invalid mode.
- Renamed `handle_invalid_signature()` to `handle_signature_if_invalid()`.
- Added warning messages specific to commit resigning.
- Fixed some small typos.
- Added support for explicitly specifying the signing key ID via
  `--signed-commits=re-sign-if-invalid[=<keyid>]` similar to how it can
  specified in git-commit(1).
- We now die() as unsupported when attempting to re-sign an invalid
  commit signature in interoperability mode.
- We now die() when failing to re-sign a commit.

Thanks,
-Justin

Justin Tobler (3):
  commit: remove unused forward declaration
  gpg-interface: introduce sign_buffer_with_key()
  fast-import: add mode to sign commits with invalid signatures

 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 100 ++++++++++++++++-----
 commit.c                           |  16 +---
 commit.h                           |   2 -
 gpg-interface.c                    |  36 ++++++--
 gpg-interface.h                    |  14 ++-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 8 files changed, 221 insertions(+), 99 deletions(-)

Range-diff against v3:
1:  0d00b72ee0 = 1:  0d00b72ee0 commit: remove unused forward declaration
2:  0b0a06347d ! 2:  87f590f1f8 gpg-interface: introduce sign_buffer_with_key()
    @@ Commit message
         provided and handles generating the commit signature accordingly. This
         signing operation is not really specific to commits as any arbitrary
         buffer can be signed. Also, in a subsequent commit, this same logic is
    -    reused by git-fast-import(1) when re-signing invalid commit signatures.
    +    reused by git-fast-import(1) when signing commits with invalid
    +    signatures.
     
         Move the `sign_commit_to_strbuf()` helper from "commit.c" to
         "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
3:  57a27ccc61 ! 3:  8b01ad1570 fast-import: add mode to re-sign invalid commit signatures
    @@ Metadata
     Author: Justin Tobler <jltobler@gmail.com>
     
      ## Commit message ##
    -    fast-import: add mode to re-sign invalid commit signatures
    +    fast-import: add mode to sign commits with invalid signatures
     
         With git-fast-import(1), handling of signed commits is controlled via
         the `--signed-commits=<mode>` option. When an invalid signature is
    -    encountered, a user may want the option to re-sign the commit as opposed
    -    to just stripping the signature. To facilitate this, introduce a
    -    "re-sign-if-invalid" mode for the `--signed-commits` option. Optionally,
    -    a key ID may be explicitly provided in the form
    -    `re-sign-if-invalid[=<keyid>]` to specify which signing key should be
    -    used when re-signing invalid commit signatures.
    +    encountered, a user may want the option to sign the commit again as
    +    opposed to just stripping the signature. To facilitate this, introduce a
    +    "sign-if-invalid" mode for the `--signed-commits` option. Optionally, a
    +    key ID may be explicitly provided in the form
    +    `sign-if-invalid[=<keyid>]` to specify which signing key should be used
    +    when signing invalid commit signatures.
     
    -    Note that to properly support interoperability mode when re-signing
    -    commit signatures, the commit buffer must be created in both the
    -    repository and compatability object formats to generate the appropriate
    -    signatures accordingly. As currently implemented, the commit buffer for
    -    the compatability object format is not reconstructed and thus re-signing
    +    Note that to properly support interoperability mode when signing commit
    +    signatures, the commit buffer must be created in both the repository and
    +    compatability object formats to generate the appropriate signatures
    +    accordingly. As currently implemented, the commit buffer for the
    +    compatability object format is not reconstructed and thus signing
         commits in interoperability mode is not yet supported. Support may be
         added in the future.
     
    @@ Documentation/git-fast-import.adoc: already trusted to run their own code.
      * `strip-if-invalid` will check signatures and, if they are invalid,
        will strip them and display a warning. The validation is performed
        in the same way as linkgit:git-verify-commit[1] does it.
    -+* `re-sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
    ++* `sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
     +  commit signatures and replaces invalid signatures with newly created ones.
     +  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
    -+  used for re-signing; otherwise the configured default signing key is used.
    ++  used for signing; otherwise the configured default signing key is used.
      
      Options for Frontends
      ~~~~~~~~~~~~~~~~~~~~~
    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
      		case SIGN_STRIP_IF_INVALID:
      			die(_("'strip-if-invalid' is not a valid mode for "
      			      "git fast-export with --signed-commits=<mode>"));
    -+		case SIGN_RE_SIGN_IF_INVALID:
    -+			die(_("'re-sign-if-invalid' is not a valid mode for "
    ++		case SIGN_SIGN_IF_INVALID:
    ++			die(_("'sign-if-invalid' is not a valid mode for "
     +			      "git fast-export with --signed-commits=<mode>"));
      		default:
      			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
      			case SIGN_STRIP_IF_INVALID:
      				die(_("'strip-if-invalid' is not a valid mode for "
      				      "git fast-export with --signed-tags=<mode>"));
    -+			case SIGN_RE_SIGN_IF_INVALID:
    -+				die(_("'re-sign-if-invalid' is not a valid mode for "
    ++			case SIGN_SIGN_IF_INVALID:
    ++				die(_("'sign-if-invalid' is not a valid mode for "
     +				      "git fast-export with --signed-tags=<mode>"));
      			default:
      				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
    @@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_da
      			warning(_("stripping invalid signature for commit\n"
      				  "  allegedly by %s"), signer);
     +		break;
    -+	case SIGN_RE_SIGN_IF_INVALID:
    ++	case SIGN_SIGN_IF_INVALID:
     +		if (subject_len > 100)
    -+			warning(_("re-signing invalid signature for commit '%.100s...'\n"
    ++			warning(_("signing commit with invalid signature for '%.100s...'\n"
     +				  "  allegedly by %s"), subject, signer);
     +		else if (subject_len > 0)
    -+			warning(_("re-signing invalid signature for commit '%.*s'\n"
    ++			warning(_("signing commit with invalid signature for '%.*s'\n"
     +				  "  allegedly by %s"), subject_len, subject, signer);
     +		else
    -+			warning(_("re-signing invalid signature for commit\n"
    ++			warning(_("signing commit with invalid signature\n"
     +				  "  allegedly by %s"), signer);
     +		break;
     +	default:
    @@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_da
     +	if (ret) {
     +		warn_invalid_signature(&signature_check, msg->buf, mode);
     +
    -+		if (mode == SIGN_RE_SIGN_IF_INVALID) {
    ++		if (mode == SIGN_SIGN_IF_INVALID) {
     +			struct strbuf signature = STRBUF_INIT;
     +			struct strbuf payload = STRBUF_INIT;
     +
     +			/*
     +			 * NEEDSWORK: To properly support interoperability mode
    -+			 * when re-signing commit signatures, the commit buffer
    ++			 * when signing commit signatures, the commit buffer
     +			 * must be provided in both the repository and
     +			 * compatibility object formats. As currently
     +			 * implemented, only the repository object format is
     +			 * considered meaning compatibility signatures cannot be
    -+			 * generated. Thus, attempting to re-sign commit
    -+			 * signatures in interoperability mode is currently
    -+			 * unsupported.
    ++			 * generated. Thus, attempting to sign commit signatures
    ++			 * in interoperability mode is currently unsupported.
     +			 */
     +			if (the_repository->compat_hash_algo)
    -+				die(_("re-signing signatures in interoperability mode is unsupported"));
    ++				die(_("signing signatures in interoperability mode is unsupported"));
     +
     +			strbuf_addstr(&payload, signature_check.payload);
     +			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      			/* fallthru */
      		case SIGN_VERBATIM:
      		case SIGN_STRIP_IF_INVALID:
    -+		case SIGN_RE_SIGN_IF_INVALID:
    ++		case SIGN_SIGN_IF_INVALID:
      			import_one_signature(&sig_sha1, &sig_sha256, v);
      			break;
      
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      
     -	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
     +	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
    -+	     signed_commit_mode == SIGN_RE_SIGN_IF_INVALID) &&
    ++	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
      	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
     -		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
     +		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
    @@ builtin/fast-import.c: static void handle_tag_signature(struct strbuf *msg, cons
      	case SIGN_STRIP_IF_INVALID:
      		die(_("'strip-if-invalid' is not a valid mode for "
      		      "git fast-import with --signed-tags=<mode>"));
    -+	case SIGN_RE_SIGN_IF_INVALID:
    -+		die(_("'re-sign-if-invalid' is not a valid mode for "
    ++	case SIGN_SIGN_IF_INVALID:
    ++		die(_("'sign-if-invalid' is not a valid mode for "
     +		      "git fast-import with --signed-tags=<mode>"));
      	default:
      		BUG("invalid signed_tag_mode value %d from tag '%s'",
    @@ gpg-interface.c: static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf
     +	} else if (!strcmp(arg, "strip-if-invalid")) {
      		*mode = SIGN_STRIP_IF_INVALID;
     -	else
    -+	} else if (!strcmp(arg, "re-sign-if-invalid")) {
    -+		*mode = SIGN_RE_SIGN_IF_INVALID;
    -+	} else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) {
    -+		*mode = SIGN_RE_SIGN_IF_INVALID;
    ++	} else if (!strcmp(arg, "sign-if-invalid")) {
    ++		*mode = SIGN_SIGN_IF_INVALID;
    ++	} else if (skip_prefix(arg, "sign-if-invalid=", &arg)) {
    ++		*mode = SIGN_SIGN_IF_INVALID;
     +		if (keyid)
     +			*keyid = arg;
     +	} else {
    @@ gpg-interface.h: enum sign_mode {
      	SIGN_WARN_STRIP,
      	SIGN_STRIP,
      	SIGN_STRIP_IF_INVALID,
    -+	SIGN_RE_SIGN_IF_INVALID,
    ++	SIGN_SIGN_IF_INVALID,
      };
      
      /*
       * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
     - * otherwise.
    -+ * otherwise. If the parsed mode is SIGN_RE_SIGN_IF_INVALID and GPG key provided
    -+ * in the arguments in the form `re-sign-if-invalid=<keyid>`, the key-ID is
    -+ * parsed into `char **keyid`.
    ++ * otherwise. If the parsed mode is SIGN_SIGN_IF_INVALID and GPG key provided in
    ++ * the arguments in the form `sign-if-invalid=<keyid>`, the key-ID is parsed
    ++ * into `char **keyid`.
       */
     -int parse_sign_mode(const char *arg, enum sign_mode *mode);
     +int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     -'
     -
     -test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
    -+for mode in strip-if-invalid re-sign-if-invalid
    ++for mode in strip-if-invalid sign-if-invalid
     +do
     +	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
     +		git fast-export main >output &&
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +			test_grep "stripping invalid signature" log &&
     +			test_grep ! -E "^gpgsig" actual
     +		else
    -+			test_grep "re-signing invalid signature" log &&
    ++			test_grep "signing commit with invalid signature" log &&
     +			test_grep -E "^gpgsig(-sha256)? " actual &&
     +			git -C new verify-commit "$IMPORTED"
     +		fi
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +	'
     +done
     +
    -+test_expect_success GPGSSH "re-sign invalid commit with explicit keyid" '
    ++test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
      	rm -rf new &&
      	git init new &&
      
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip signature inva
     +	test_config -C new gpg.format ssh &&
     +	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
     +	test_must_fail git -C new fast-import --quiet \
    -+		--signed-commits=re-sign-if-invalid <modified >/dev/null 2>&1 &&
    ++		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
     +
     +	# Import using explicitly provided signing key.
     +	git -C new fast-import --quiet \
    -+		--signed-commits=re-sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
    ++		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
      
      	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
      	test $OPENPGP_SIGNING != $IMPORTED &&

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6

[PATCH v4 1/3] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0.381.g628a66ccf6

[PATCH v4 2/3] gpg-interface: introduce sign_buffer_with_key()

[Justin Tobler]

The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when signing commits with invalid
signatures.

Move the `sign_commit_to_strbuf()` helper from "commit.c" to
"gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
this function so it can be used by "commit.c" and
"builtin/fast-import.c" in the subsequent commit.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.c        | 16 ++--------------
 gpg-interface.c | 13 +++++++++++++
 gpg-interface.h |  7 +++++++
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/commit.c b/commit.c
index d16ae73345..1677b1ef25 100644
--- a/commit.c
+++ b/commit.c
@@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 	return 0;
 }
 
-static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
-{
-	char *keyid_to_free = NULL;
-	int ret = 0;
-	if (!keyid || !*keyid)
-		keyid = keyid_to_free = get_signing_key();
-	if (sign_buffer(buf, sig, keyid))
-		ret = -1;
-	free(keyid_to_free);
-	return ret;
-}
-
 int parse_signed_commit(const struct commit *commit,
 			struct strbuf *payload, struct strbuf *signature,
 			const struct git_hash_algo *algop)
@@ -1737,7 +1725,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
 	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
-	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
+	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
 		result = -1;
 		goto out;
 	}
@@ -1769,7 +1757,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
 
-		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
+		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
 			result = -1;
 			goto out;
 		}
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..a72fa35061 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -980,6 +980,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	return use_format->sign_buffer(buffer, signature, signing_key);
 }
 
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key)
+{
+	char *keyid_to_free = NULL;
+	int ret = 0;
+	if (!signing_key || !*signing_key)
+		signing_key = keyid_to_free = get_signing_key();
+	if (sign_buffer(buffer, signature, signing_key))
+		ret = -1;
+	free(keyid_to_free);
+	return ret;
+}
+
 /*
  * Strip CR from the line endings, in case we are on Windows.
  * NEEDSWORK: make it trim only CRs before LFs and rename
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..a32741aeda 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 		const char *signing_key);
 
+/*
+ * Similar to `sign_buffer()`, but uses the default configured signing key as
+ * returned by `get_signing_key()` when the provided "signing_key" is NULL or
+ * empty. Returns 0 on success, non-zero on failure.
+ */
+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
+			 const char *signing_key);
 
 /*
  * Returns corresponding string in lowercase for a given member of
-- 
2.53.0.381.g628a66ccf6

[PATCH v4 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to sign the commit again as
opposed to just stripping the signature. To facilitate this, introduce a
"sign-if-invalid" mode for the `--signed-commits` option. Optionally, a
key ID may be explicitly provided in the form
`sign-if-invalid[=<keyid>]` to specify which signing key should be used
when signing invalid commit signatures.

Note that to properly support interoperability mode when signing commit
signatures, the commit buffer must be created in both the repository and
compatability object formats to generate the appropriate signatures
accordingly. As currently implemented, the commit buffer for the
compatability object format is not reconstructed and thus signing
commits in interoperability mode is not yet supported. Support may be
added in the future.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 100 ++++++++++++++++-----
 gpg-interface.c                    |  23 +++--
 gpg-interface.h                    |   7 +-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 6 files changed, 199 insertions(+), 83 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..b3f42d4637 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,10 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
+  commit signatures and replaces invalid signatures with newly created ones.
+  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
+  used for signing; otherwise the configured default signing key is used.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..13621b0d6a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val))
+	if (parse_sign_mode(arg, val, NULL))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_SIGN_IF_INVALID:
+			die(_("'sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_SIGN_IF_INVALID:
+				die(_("'sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..d6281ff119 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -190,6 +190,7 @@ static const char *global_prefix;
 
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+static const char *signed_commit_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2836,26 +2837,15 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void warn_invalid_signature(struct signature_check *check,
+				   const char *msg, enum sign_mode mode)
 {
-	struct strbuf tmp_buf = STRBUF_INIT;
-	struct signature_check signature_check = { 0 };
-	int ret;
-
-	/* Check signature in a temporary commit buffer */
-	strbuf_addbuf(&tmp_buf, new_data);
-	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
-	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
-
-	if (ret) {
-		const char *signer = signature_check.signer ?
-			signature_check.signer : _("unknown");
-		const char *subject;
-		int subject_len = find_commit_subject(msg->buf, &subject);
+	const char *signer = check->signer ? check->signer : _("unknown");
+	const char *subject;
+	int subject_len = find_commit_subject(msg, &subject);
 
+	switch (mode) {
+	case SIGN_STRIP_IF_INVALID:
 		if (subject_len > 100)
 			warning(_("stripping invalid signature for commit '%.100s...'\n"
 				  "  allegedly by %s"), subject, signer);
@@ -2865,6 +2855,66 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 		else
 			warning(_("stripping invalid signature for commit\n"
 				  "  allegedly by %s"), signer);
+		break;
+	case SIGN_SIGN_IF_INVALID:
+		if (subject_len > 100)
+			warning(_("signing commit with invalid signature for '%.100s...'\n"
+				  "  allegedly by %s"), subject, signer);
+		else if (subject_len > 0)
+			warning(_("signing commit with invalid signature for '%.*s'\n"
+				  "  allegedly by %s"), subject_len, subject, signer);
+		else
+			warning(_("signing commit with invalid signature\n"
+				  "  allegedly by %s"), signer);
+		break;
+	default:
+		BUG("unsupported signing mode");
+	}
+}
+
+static void handle_signature_if_invalid(struct strbuf *new_data,
+					struct signature_data *sig_sha1,
+					struct signature_data *sig_sha256,
+					struct strbuf *msg,
+					enum sign_mode mode)
+{
+	struct strbuf tmp_buf = STRBUF_INIT;
+	struct signature_check signature_check = { 0 };
+	int ret;
+
+	/* Check signature in a temporary commit buffer */
+	strbuf_addbuf(&tmp_buf, new_data);
+	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
+	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
+
+	if (ret) {
+		warn_invalid_signature(&signature_check, msg->buf, mode);
+
+		if (mode == SIGN_SIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+
+			/*
+			 * NEEDSWORK: To properly support interoperability mode
+			 * when signing commit signatures, the commit buffer
+			 * must be provided in both the repository and
+			 * compatibility object formats. As currently
+			 * implemented, only the repository object format is
+			 * considered meaning compatibility signatures cannot be
+			 * generated. Thus, attempting to sign commit signatures
+			 * in interoperability mode is currently unsupported.
+			 */
+			if (the_repository->compat_hash_algo)
+				die(_("signing signatures in interoperability mode is unsupported"));
+
+			strbuf_addstr(&payload, signature_check.payload);
+			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
+				die(_("failed to sign commit object"));
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+		}
 
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
@@ -2927,6 +2977,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_SIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3062,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
+					    &msg, signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3113,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_SIGN_IF_INVALID:
+		die(_("'sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3649,10 +3705,10 @@ static int parse_one_option(const char *option)
 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
 		option_export_pack_edges(option);
 	} else if (skip_prefix(option, "signed-commits=", &option)) {
-		if (parse_sign_mode(option, &signed_commit_mode))
+		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode))
+		if (parse_sign_mode(option, &signed_tag_mode, NULL))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/gpg-interface.c b/gpg-interface.c
index a72fa35061..2dd428ee2c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1155,21 +1155,28 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	return ret;
 }
 
-int parse_sign_mode(const char *arg, enum sign_mode *mode)
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 {
-	if (!strcmp(arg, "abort"))
+	if (!strcmp(arg, "abort")) {
 		*mode = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
 		*mode = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
 		*mode = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
+	} else if (!strcmp(arg, "warn-strip")) {
 		*mode = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
+	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
-	else if (!strcmp(arg, "strip-if-invalid"))
+	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
-	else
+	} else if (!strcmp(arg, "sign-if-invalid")) {
+		*mode = SIGN_SIGN_IF_INVALID;
+	} else if (skip_prefix(arg, "sign-if-invalid=", &arg)) {
+		*mode = SIGN_SIGN_IF_INVALID;
+		if (keyid)
+			*keyid = arg;
+	} else {
 		return -1;
+	}
 	return 0;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index a32741aeda..25f6209fcf 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -119,12 +119,15 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_SIGN_IF_INVALID,
 };
 
 /*
  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
- * otherwise.
+ * otherwise. If the parsed mode is SIGN_SIGN_IF_INVALID and GPG key provided in
+ * the arguments in the form `sign-if-invalid=<keyid>`, the key-ID is parsed
+ * into `char **keyid`.
  */
-int parse_sign_mode(const char *arg, enum sign_mode *mode);
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
 
 #endif
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..38b3e3b537 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,26 +103,85 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
+for mode in strip-if-invalid sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep "stripping invalid signature" log &&
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep "signing commit with invalid signature" log &&
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
 	rm -rf new &&
 	git init new &&
 
@@ -133,41 +192,22 @@ test_expect_success GPG 'strip signature invalidated by message change with --si
 	# corresponding `data <length>` command would have to be changed too.
 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
+	# Configure the target repository with an invalid default signing key.
+	test_config -C new user.signingkey "not-a-real-key-id" &&
+	test_config -C new gpg.format ssh &&
+	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
 
 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
 	test $OPENPGP_SIGNING != $IMPORTED &&
 	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
 	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
-
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
+	git -C new verify-commit "$IMPORTED"
 '
 
 test_done
-- 
2.53.0.381.g628a66ccf6

Re: [PATCH v4 2/3] gpg-interface: introduce sign_buffer_with_key()

[Patrick Steinhardt]

On Wed, Mar 11, 2026 at 12:31:46PM -0500, Justin Tobler wrote:
> diff --git a/gpg-interface.h b/gpg-interface.h
> index 789d1ffac4..a32741aeda 100644
> --- a/gpg-interface.h
> +++ b/gpg-interface.h
> @@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
>  int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
>  		const char *signing_key);
>  
> +/*
> + * Similar to `sign_buffer()`, but uses the default configured signing key as
> + * returned by `get_signing_key()` when the provided "signing_key" is NULL or
> + * empty. Returns 0 on success, non-zero on failure.
> + */
> +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> +			 const char *signing_key);

I think this interface is a bit confusing, as you wouldn't really be
able to tell what the difference between `sign_buffer()` and
`sign_buffer_with_key()` is without having a deeper look. Naively, I
would expect the latter function to be the one that actually mandates
that the user provides a key, but it's the other way round.

Would it be preferable to instead extend `sign_buffer()` to take a flags
parameter and then introduce `SIGN_BUFFER_USE_DEFAULT_KEY` to make it
fall back to the configured signing key? If so, we could drop
`sign_commit_to_strbuf()` completely.

Patrick

Re: [PATCH v4 3/3] fast-import: add mode to sign commits with invalid signatures

[Patrick Steinhardt]

On Wed, Mar 11, 2026 at 12:31:47PM -0500, Justin Tobler wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index b8a7757cfd..d6281ff119 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2865,6 +2855,66 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
>  		else
>  			warning(_("stripping invalid signature for commit\n"
>  				  "  allegedly by %s"), signer);
> +		break;
> +	case SIGN_SIGN_IF_INVALID:
> +		if (subject_len > 100)
> +			warning(_("signing commit with invalid signature for '%.100s...'\n"
> +				  "  allegedly by %s"), subject, signer);
> +		else if (subject_len > 0)
> +			warning(_("signing commit with invalid signature for '%.*s'\n"
> +				  "  allegedly by %s"), subject_len, subject, signer);
> +		else
> +			warning(_("signing commit with invalid signature\n"
> +				  "  allegedly by %s"), signer);
> +		break;
> +	default:
> +		BUG("unsupported signing mode");
> +	}
> +}

I'm still not convinced that it makes sense to warn about this case.
After all the user has asked us to re-sign such commits, so they
probably expect such cases. These warnings would thus result in a ton of
noise in a repository where most commits are signed, drowning out the
potentially-useful warnings.

Anyway, I won't insist on a change here.

> +static void handle_signature_if_invalid(struct strbuf *new_data,
> +					struct signature_data *sig_sha1,
> +					struct signature_data *sig_sha256,
> +					struct strbuf *msg,
> +					enum sign_mode mode)
> +{
> +	struct strbuf tmp_buf = STRBUF_INIT;
> +	struct signature_check signature_check = { 0 };
> +	int ret;
> +
> +	/* Check signature in a temporary commit buffer */
> +	strbuf_addbuf(&tmp_buf, new_data);
> +	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
> +	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
> +
> +	if (ret) {
> +		warn_invalid_signature(&signature_check, msg->buf, mode);
> +
> +		if (mode == SIGN_SIGN_IF_INVALID) {
> +			struct strbuf signature = STRBUF_INIT;
> +			struct strbuf payload = STRBUF_INIT;
> +
> +			/*
> +			 * NEEDSWORK: To properly support interoperability mode
> +			 * when signing commit signatures, the commit buffer
> +			 * must be provided in both the repository and
> +			 * compatibility object formats. As currently
> +			 * implemented, only the repository object format is
> +			 * considered meaning compatibility signatures cannot be
> +			 * generated. Thus, attempting to sign commit signatures
> +			 * in interoperability mode is currently unsupported.
> +			 */
> +			if (the_repository->compat_hash_algo)
> +				die(_("signing signatures in interoperability mode is unsupported"));

"signing signatures"? You probably meant "signing commits"?

Patrick

Re: [PATCH v4 2/3] gpg-interface: introduce sign_buffer_with_key()

[Justin Tobler]

On 26/03/12 11:22AM, Patrick Steinhardt wrote:
> On Wed, Mar 11, 2026 at 12:31:46PM -0500, Justin Tobler wrote:
> > diff --git a/gpg-interface.h b/gpg-interface.h
> > index 789d1ffac4..a32741aeda 100644
> > --- a/gpg-interface.h
> > +++ b/gpg-interface.h
> > @@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size);
> >  int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
> >  		const char *signing_key);
> >  
> > +/*
> > + * Similar to `sign_buffer()`, but uses the default configured signing key as
> > + * returned by `get_signing_key()` when the provided "signing_key" is NULL or
> > + * empty. Returns 0 on success, non-zero on failure.
> > + */
> > +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
> > +			 const char *signing_key);
> 
> I think this interface is a bit confusing, as you wouldn't really be
> able to tell what the difference between `sign_buffer()` and
> `sign_buffer_with_key()` is without having a deeper look. Naively, I
> would expect the latter function to be the one that actually mandates
> that the user provides a key, but it's the other way round.
> 
> Would it be preferable to instead extend `sign_buffer()` to take a flags
> parameter and then introduce `SIGN_BUFFER_USE_DEFAULT_KEY` to make it
> fall back to the configured signing key? If so, we could drop
> `sign_commit_to_strbuf()` completely.

That's fair, and this suggestion sounds completely sensible to me. There
are only a handful to `sign_buffer()` callers, so it should create too
much churn either. Will send another version adapted accordingly.

-Justin

Re: [PATCH v4 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

On 26/03/12 11:23AM, Patrick Steinhardt wrote:
> On Wed, Mar 11, 2026 at 12:31:47PM -0500, Justin Tobler wrote:
> > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > index b8a7757cfd..d6281ff119 100644
> > --- a/builtin/fast-import.c
> > +++ b/builtin/fast-import.c
> > @@ -2865,6 +2855,66 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
> >  		else
> >  			warning(_("stripping invalid signature for commit\n"
> >  				  "  allegedly by %s"), signer);
> > +		break;
> > +	case SIGN_SIGN_IF_INVALID:
> > +		if (subject_len > 100)
> > +			warning(_("signing commit with invalid signature for '%.100s...'\n"
> > +				  "  allegedly by %s"), subject, signer);
> > +		else if (subject_len > 0)
> > +			warning(_("signing commit with invalid signature for '%.*s'\n"
> > +				  "  allegedly by %s"), subject_len, subject, signer);
> > +		else
> > +			warning(_("signing commit with invalid signature\n"
> > +				  "  allegedly by %s"), signer);
> > +		break;
> > +	default:
> > +		BUG("unsupported signing mode");
> > +	}
> > +}
> 
> I'm still not convinced that it makes sense to warn about this case.
> After all the user has asked us to re-sign such commits, so they
> probably expect such cases. These warnings would thus result in a ton of
> noise in a repository where most commits are signed, drowning out the
> potentially-useful warnings.
> 
> Anyway, I won't insist on a change here.

I'm not really against removing these warning as I also agree it creates
a bunch of noise. If we get rid of them for "sign-if-invalid" though,
shouldn't we also get rid of them for "strip-if-invalid"? If the user
asks to strip commits, I figure they would expect such cases as well. If
we think removing the warning altogether is sensible, I can add another
prepatory commit that simply removes the warning for the
"strip-if-invalid" case.

> > +static void handle_signature_if_invalid(struct strbuf *new_data,
> > +					struct signature_data *sig_sha1,
> > +					struct signature_data *sig_sha256,
> > +					struct strbuf *msg,
> > +					enum sign_mode mode)
> > +{
> > +	struct strbuf tmp_buf = STRBUF_INIT;
> > +	struct signature_check signature_check = { 0 };
> > +	int ret;
> > +
> > +	/* Check signature in a temporary commit buffer */
> > +	strbuf_addbuf(&tmp_buf, new_data);
> > +	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
> > +	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
> > +
> > +	if (ret) {
> > +		warn_invalid_signature(&signature_check, msg->buf, mode);
> > +
> > +		if (mode == SIGN_SIGN_IF_INVALID) {
> > +			struct strbuf signature = STRBUF_INIT;
> > +			struct strbuf payload = STRBUF_INIT;
> > +
> > +			/*
> > +			 * NEEDSWORK: To properly support interoperability mode
> > +			 * when signing commit signatures, the commit buffer
> > +			 * must be provided in both the repository and
> > +			 * compatibility object formats. As currently
> > +			 * implemented, only the repository object format is
> > +			 * considered meaning compatibility signatures cannot be
> > +			 * generated. Thus, attempting to sign commit signatures
> > +			 * in interoperability mode is currently unsupported.
> > +			 */
> > +			if (the_repository->compat_hash_algo)
> > +				die(_("signing signatures in interoperability mode is unsupported"));
> 
> "signing signatures"? You probably meant "signing commits"?

Ah yes! Will fix in the next version. Thanks for reading closely :)

-Justin

Re: [PATCH v4 3/3] fast-import: add mode to sign commits with invalid signatures

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 09:08:46AM -0500, Justin Tobler wrote:
> On 26/03/12 11:23AM, Patrick Steinhardt wrote:
> > On Wed, Mar 11, 2026 at 12:31:47PM -0500, Justin Tobler wrote:
> > > diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> > > index b8a7757cfd..d6281ff119 100644
> > > --- a/builtin/fast-import.c
> > > +++ b/builtin/fast-import.c
> > > @@ -2865,6 +2855,66 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
> > >  		else
> > >  			warning(_("stripping invalid signature for commit\n"
> > >  				  "  allegedly by %s"), signer);
> > > +		break;
> > > +	case SIGN_SIGN_IF_INVALID:
> > > +		if (subject_len > 100)
> > > +			warning(_("signing commit with invalid signature for '%.100s...'\n"
> > > +				  "  allegedly by %s"), subject, signer);
> > > +		else if (subject_len > 0)
> > > +			warning(_("signing commit with invalid signature for '%.*s'\n"
> > > +				  "  allegedly by %s"), subject_len, subject, signer);
> > > +		else
> > > +			warning(_("signing commit with invalid signature\n"
> > > +				  "  allegedly by %s"), signer);
> > > +		break;
> > > +	default:
> > > +		BUG("unsupported signing mode");
> > > +	}
> > > +}
> > 
> > I'm still not convinced that it makes sense to warn about this case.
> > After all the user has asked us to re-sign such commits, so they
> > probably expect such cases. These warnings would thus result in a ton of
> > noise in a repository where most commits are signed, drowning out the
> > potentially-useful warnings.
> > 
> > Anyway, I won't insist on a change here.
> 
> I'm not really against removing these warning as I also agree it creates
> a bunch of noise. If we get rid of them for "sign-if-invalid" though,
> shouldn't we also get rid of them for "strip-if-invalid"? If the user
> asks to strip commits, I figure they would expect such cases as well. If
> we think removing the warning altogether is sensible, I can add another
> prepatory commit that simply removes the warning for the
> "strip-if-invalid" case.

Yeah, it kind of falls into the same space, agreed. As said, I won't
insist on changing this. Maybe the right way to approach this is to keep
it as-is for now and create a follow-up patch where you propose to strip
it from both sites?

Patrick

Re: [PATCH v4 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

On 26/03/12 03:22PM, Patrick Steinhardt wrote:
> On Thu, Mar 12, 2026 at 09:08:46AM -0500, Justin Tobler wrote:
> > On 26/03/12 11:23AM, Patrick Steinhardt wrote:
> > > I'm still not convinced that it makes sense to warn about this case.
> > > After all the user has asked us to re-sign such commits, so they
> > > probably expect such cases. These warnings would thus result in a ton of
> > > noise in a repository where most commits are signed, drowning out the
> > > potentially-useful warnings.
> > > 
> > > Anyway, I won't insist on a change here.
> > 
> > I'm not really against removing these warning as I also agree it creates
> > a bunch of noise. If we get rid of them for "sign-if-invalid" though,
> > shouldn't we also get rid of them for "strip-if-invalid"? If the user
> > asks to strip commits, I figure they would expect such cases as well. If
> > we think removing the warning altogether is sensible, I can add another
> > prepatory commit that simply removes the warning for the
> > "strip-if-invalid" case.
> 
> Yeah, it kind of falls into the same space, agreed. As said, I won't
> insist on changing this. Maybe the right way to approach this is to keep
> it as-is for now and create a follow-up patch where you propose to strip
> it from both sites?

That's fair. I'll leave it as-is for now and submit a separate follow up
patch after this gets merged that proposes removing the warnings
altogether. We can see what folks think about it there.

Thanks,
-Justin

[PATCH v5 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commit signatures. Building upon this functionality, a user may
want to re-sign these invalid commit signatures. This series introduces
the `sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatibility structure would also
require us to reconstruct the compatibility object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not completely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be addressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Changes since V4:
- Instead of introducing a separate `sign_buffer_with_key()` helper,
  extend `sign_buffer()` to support a SIGN_BUFFER_USE_DEFAULT_KEY flag.
- Fixed message in die().

Changes since V3:
- Rename the `re-sign-if-invalid` mode to `sign-if-invalid`. The
  "if-invalid" already implies the signatures was previously signed
  making "re-sign" redundant.

Changes since V2:
- Adapted commit message in second patch to improve clarity.
- Fixed typos.
- Renamed SIGN_RESIGN_IF_INVALID to SIGN_RE_SIGN_IF_INVALID.
- Created separate helper function to handle printing invalid signature
  warnings.

Changes since V1:
- Improved commit messages and comments to better explain why
  interoperability mode is not currently supported.
- Clarified documentation for re-sign-if-invalid mode.
- Renamed `handle_invalid_signature()` to `handle_signature_if_invalid()`.
- Added warning messages specific to commit resigning.
- Fixed some small typos.
- Added support for explicitly specifying the signing key ID via
  `--signed-commits=re-sign-if-invalid[=<keyid>]` similar to how it can
  specified in git-commit(1).
- We now die() as unsupported when attempting to re-sign an invalid
  commit signature in interoperability mode.
- We now die() when failing to re-sign a commit.

Thanks,
-Justin

Justin Tobler (3):
  commit: remove unused forward declaration
  gpg-interface: allow sign_buffer() to use default signing key
  fast-import: add mode to sign commits with invalid signatures

 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 ++++++++++++++++-----
 builtin/tag.c                      |   4 +-
 commit.c                           |  19 ++--
 commit.h                           |   2 -
 gpg-interface.c                    |  36 +++++---
 gpg-interface.h                    |  19 +++-
 send-pack.c                        |   2 +-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 10 files changed, 229 insertions(+), 106 deletions(-)

Range-diff against v4:
1:  0d00b72ee0 = 1:  0d00b72ee0 commit: remove unused forward declaration
2:  87f590f1f8 ! 2:  7a0deed77b gpg-interface: introduce sign_buffer_with_key()
    @@ Metadata
     Author: Justin Tobler <jltobler@gmail.com>
     
      ## Commit message ##
    -    gpg-interface: introduce sign_buffer_with_key()
    +    gpg-interface: allow sign_buffer() to use default signing key
     
         The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
         logic to get the default configured signing key when a key is not
    @@ Commit message
         reused by git-fast-import(1) when signing commits with invalid
         signatures.
     
    -    Move the `sign_commit_to_strbuf()` helper from "commit.c" to
    -    "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export
    -    this function so it can be used by "commit.c" and
    -    "builtin/fast-import.c" in the subsequent commit.
    +    Remove the `sign_commit_to_strbuf()` helper from "commit.c" and extend
    +    `sign_buffer()` in "gpg-interface.c" to support using the default key as
    +    a fallback when the `SIGN_BUFFER_USE_DEFAULT_KEY` flag is provided. Call
    +    sites are updated accordingly.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
    + ## builtin/tag.c ##
    +@@ builtin/tag.c: static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
    + 	char *keyid = get_signing_key();
    + 	int ret = -1;
    + 
    +-	if (sign_buffer(buffer, &sig, keyid))
    ++	if (sign_buffer(buffer, &sig, keyid, 0))
    + 		goto out;
    + 
    + 	if (compat) {
    +@@ builtin/tag.c: static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
    + 		if (convert_object_file(the_repository ,&compat_buf, algo, compat,
    + 					buffer->buf, buffer->len, OBJ_TAG, 1))
    + 			goto out;
    +-		if (sign_buffer(&compat_buf, &compat_sig, keyid))
    ++		if (sign_buffer(&compat_buf, &compat_sig, keyid, 0))
    + 			goto out;
    + 		add_header_signature(&compat_buf, &sig, algo);
    + 		strbuf_addbuf(&compat_buf, &compat_sig);
    +
      ## commit.c ##
     @@ commit.c: int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
      	return 0;
    @@ commit.c: int commit_tree_extended(const char *msg, size_t msg_len,
      
      	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
     -	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
    -+	if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) {
    ++	if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
    ++				       SIGN_BUFFER_USE_DEFAULT_KEY)) {
      		result = -1;
      		goto out;
      	}
    @@ commit.c: int commit_tree_extended(const char *msg, size_t msg_len,
      		free(mapped_parents);
      
     -		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
    -+		if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) {
    ++		if (sign_commit && sign_buffer(&compat_buffer, &compat_sig,
    ++					       sign_commit,
    ++					       SIGN_BUFFER_USE_DEFAULT_KEY)) {
      			result = -1;
      			goto out;
      		}
     
      ## gpg-interface.c ##
    -@@ gpg-interface.c: int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
    - 	return use_format->sign_buffer(buffer, signature, signing_key);
    +@@ gpg-interface.c: const char *gpg_trust_level_to_str(enum signature_trust_level level)
    + 	return sigcheck_gpg_trust_level[level].display_key;
      }
      
    -+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
    -+			 const char *signing_key)
    -+{
    +-int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
    ++int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
    ++		const char *signing_key, enum sign_buffer_flags flags)
    + {
     +	char *keyid_to_free = NULL;
     +	int ret = 0;
    -+	if (!signing_key || !*signing_key)
    ++
    + 	gpg_interface_lazy_init();
    + 
    +-	return use_format->sign_buffer(buffer, signature, signing_key);
    ++	if (flags & SIGN_BUFFER_USE_DEFAULT_KEY && (!signing_key || !*signing_key))
     +		signing_key = keyid_to_free = get_signing_key();
    -+	if (sign_buffer(buffer, signature, signing_key))
    -+		ret = -1;
    ++
    ++	ret = use_format->sign_buffer(buffer, signature, signing_key);
     +	free(keyid_to_free);
     +	return ret;
    -+}
    -+
    + }
    + 
      /*
    -  * Strip CR from the line endings, in case we are on Windows.
    -  * NEEDSWORK: make it trim only CRs before LFs and rename
     
      ## gpg-interface.h ##
    +@@ gpg-interface.h: int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct
    +  */
    + size_t parse_signed_buffer(const char *buf, size_t size);
    + 
    ++/* Flags for sign_buffer(). */
    ++enum sign_buffer_flags {
    ++	/*
    ++	 * Use the default configured signing key as returned by `get_signing_key()`
    ++	 * when the provided "signing_key" is NULL or empty.
    ++	 */
    ++	SIGN_BUFFER_USE_DEFAULT_KEY = (1 << 0),
    ++};
    ++
    + /*
    +  * Create a detached signature for the contents of "buffer" and append
    +  * it after "signature"; "buffer" and "signature" can be the same
     @@ gpg-interface.h: size_t parse_signed_buffer(const char *buf, size_t size);
    +  * at the end.  Returns 0 on success, non-zero on failure.
    +  */
      int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
    - 		const char *signing_key);
    - 
    -+/*
    -+ * Similar to `sign_buffer()`, but uses the default configured signing key as
    -+ * returned by `get_signing_key()` when the provided "signing_key" is NULL or
    -+ * empty. Returns 0 on success, non-zero on failure.
    -+ */
    -+int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature,
    -+			 const char *signing_key);
    +-		const char *signing_key);
    +-
    ++		const char *signing_key, enum sign_buffer_flags flags);
      
      /*
       * Returns corresponding string in lowercase for a given member of
    +
    + ## send-pack.c ##
    +@@ send-pack.c: static int generate_push_cert(struct strbuf *req_buf,
    + 	if (!update_seen)
    + 		goto free_return;
    + 
    +-	if (sign_buffer(&cert, &cert, signing_key))
    ++	if (sign_buffer(&cert, &cert, signing_key, 0))
    + 		die(_("failed to sign the push certificate"));
    + 
    + 	packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string);
3:  8b01ad1570 ! 3:  e659971e84 fast-import: add mode to sign commits with invalid signatures
    @@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_da
     +			 * in interoperability mode is currently unsupported.
     +			 */
     +			if (the_repository->compat_hash_algo)
    -+				die(_("signing signatures in interoperability mode is unsupported"));
    ++				die(_("signing commits in interoperability mode is unsupported"));
     +
     +			strbuf_addstr(&payload, signature_check.payload);
    -+			if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid))
    ++			if (sign_buffer(&payload, &signature, signed_commit_keyid,
    ++					SIGN_BUFFER_USE_DEFAULT_KEY))
     +				die(_("failed to sign commit object"));
     +			add_header_signature(new_data, &signature, the_hash_algo);
     +

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6

[PATCH v5 1/3] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0.381.g628a66ccf6

[PATCH v5 2/3] gpg-interface: allow sign_buffer() to use default signing key

[Justin Tobler]

The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when signing commits with invalid
signatures.

Remove the `sign_commit_to_strbuf()` helper from "commit.c" and extend
`sign_buffer()` in "gpg-interface.c" to support using the default key as
a fallback when the `SIGN_BUFFER_USE_DEFAULT_KEY` flag is provided. Call
sites are updated accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/tag.c   |  4 ++--
 commit.c        | 19 +++++--------------
 gpg-interface.c | 13 +++++++++++--
 gpg-interface.h | 12 ++++++++++--
 send-pack.c     |  2 +-
 5 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index aeb04c487f..540d783c67 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -167,7 +167,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
 	char *keyid = get_signing_key();
 	int ret = -1;
 
-	if (sign_buffer(buffer, &sig, keyid))
+	if (sign_buffer(buffer, &sig, keyid, 0))
 		goto out;
 
 	if (compat) {
@@ -176,7 +176,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
 		if (convert_object_file(the_repository ,&compat_buf, algo, compat,
 					buffer->buf, buffer->len, OBJ_TAG, 1))
 			goto out;
-		if (sign_buffer(&compat_buf, &compat_sig, keyid))
+		if (sign_buffer(&compat_buf, &compat_sig, keyid, 0))
 			goto out;
 		add_header_signature(&compat_buf, &sig, algo);
 		strbuf_addbuf(&compat_buf, &compat_sig);
diff --git a/commit.c b/commit.c
index d16ae73345..1b9b2d4499 100644
--- a/commit.c
+++ b/commit.c
@@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 	return 0;
 }
 
-static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
-{
-	char *keyid_to_free = NULL;
-	int ret = 0;
-	if (!keyid || !*keyid)
-		keyid = keyid_to_free = get_signing_key();
-	if (sign_buffer(buf, sig, keyid))
-		ret = -1;
-	free(keyid_to_free);
-	return ret;
-}
-
 int parse_signed_commit(const struct commit *commit,
 			struct strbuf *payload, struct strbuf *signature,
 			const struct git_hash_algo *algop)
@@ -1737,7 +1725,8 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
 	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
-	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
+	if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
+				       SIGN_BUFFER_USE_DEFAULT_KEY)) {
 		result = -1;
 		goto out;
 	}
@@ -1769,7 +1758,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
 
-		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
+		if (sign_commit && sign_buffer(&compat_buffer, &compat_sig,
+					       sign_commit,
+					       SIGN_BUFFER_USE_DEFAULT_KEY)) {
 			result = -1;
 			goto out;
 		}
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..ce935908cc 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -973,11 +973,20 @@ const char *gpg_trust_level_to_str(enum signature_trust_level level)
 	return sigcheck_gpg_trust_level[level].display_key;
 }
 
-int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
+int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
+		const char *signing_key, enum sign_buffer_flags flags)
 {
+	char *keyid_to_free = NULL;
+	int ret = 0;
+
 	gpg_interface_lazy_init();
 
-	return use_format->sign_buffer(buffer, signature, signing_key);
+	if (flags & SIGN_BUFFER_USE_DEFAULT_KEY && (!signing_key || !*signing_key))
+		signing_key = keyid_to_free = get_signing_key();
+
+	ret = use_format->sign_buffer(buffer, signature, signing_key);
+	free(keyid_to_free);
+	return ret;
 }
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..37f3ac42db 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -74,6 +74,15 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct
  */
 size_t parse_signed_buffer(const char *buf, size_t size);
 
+/* Flags for sign_buffer(). */
+enum sign_buffer_flags {
+	/*
+	 * Use the default configured signing key as returned by `get_signing_key()`
+	 * when the provided "signing_key" is NULL or empty.
+	 */
+	SIGN_BUFFER_USE_DEFAULT_KEY = (1 << 0),
+};
+
 /*
  * Create a detached signature for the contents of "buffer" and append
  * it after "signature"; "buffer" and "signature" can be the same
@@ -81,8 +90,7 @@ size_t parse_signed_buffer(const char *buf, size_t size);
  * at the end.  Returns 0 on success, non-zero on failure.
  */
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
-		const char *signing_key);
-
+		const char *signing_key, enum sign_buffer_flags flags);
 
 /*
  * Returns corresponding string in lowercase for a given member of
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..07ecfae4de 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -391,7 +391,7 @@ static int generate_push_cert(struct strbuf *req_buf,
 	if (!update_seen)
 		goto free_return;
 
-	if (sign_buffer(&cert, &cert, signing_key))
+	if (sign_buffer(&cert, &cert, signing_key, 0))
 		die(_("failed to sign the push certificate"));
 
 	packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string);
-- 
2.53.0.381.g628a66ccf6

[PATCH v5 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to sign the commit again as
opposed to just stripping the signature. To facilitate this, introduce a
"sign-if-invalid" mode for the `--signed-commits` option. Optionally, a
key ID may be explicitly provided in the form
`sign-if-invalid[=<keyid>]` to specify which signing key should be used
when signing invalid commit signatures.

Note that to properly support interoperability mode when signing commit
signatures, the commit buffer must be created in both the repository and
compatability object formats to generate the appropriate signatures
accordingly. As currently implemented, the commit buffer for the
compatability object format is not reconstructed and thus signing
commits in interoperability mode is not yet supported. Support may be
added in the future.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 ++++++++++++++++-----
 gpg-interface.c                    |  23 +++--
 gpg-interface.h                    |   7 +-
 t/t9305-fast-import-signatures.sh  | 140 ++++++++++++++++++-----------
 6 files changed, 200 insertions(+), 83 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..b3f42d4637 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,10 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
+  commit signatures and replaces invalid signatures with newly created ones.
+  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
+  used for signing; otherwise the configured default signing key is used.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..13621b0d6a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val))
+	if (parse_sign_mode(arg, val, NULL))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_SIGN_IF_INVALID:
+			die(_("'sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_SIGN_IF_INVALID:
+				die(_("'sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..50de88e2ea 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -190,6 +190,7 @@ static const char *global_prefix;
 
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+static const char *signed_commit_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2836,26 +2837,15 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void warn_invalid_signature(struct signature_check *check,
+				   const char *msg, enum sign_mode mode)
 {
-	struct strbuf tmp_buf = STRBUF_INIT;
-	struct signature_check signature_check = { 0 };
-	int ret;
-
-	/* Check signature in a temporary commit buffer */
-	strbuf_addbuf(&tmp_buf, new_data);
-	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
-	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
-
-	if (ret) {
-		const char *signer = signature_check.signer ?
-			signature_check.signer : _("unknown");
-		const char *subject;
-		int subject_len = find_commit_subject(msg->buf, &subject);
+	const char *signer = check->signer ? check->signer : _("unknown");
+	const char *subject;
+	int subject_len = find_commit_subject(msg, &subject);
 
+	switch (mode) {
+	case SIGN_STRIP_IF_INVALID:
 		if (subject_len > 100)
 			warning(_("stripping invalid signature for commit '%.100s...'\n"
 				  "  allegedly by %s"), subject, signer);
@@ -2865,6 +2855,67 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 		else
 			warning(_("stripping invalid signature for commit\n"
 				  "  allegedly by %s"), signer);
+		break;
+	case SIGN_SIGN_IF_INVALID:
+		if (subject_len > 100)
+			warning(_("signing commit with invalid signature for '%.100s...'\n"
+				  "  allegedly by %s"), subject, signer);
+		else if (subject_len > 0)
+			warning(_("signing commit with invalid signature for '%.*s'\n"
+				  "  allegedly by %s"), subject_len, subject, signer);
+		else
+			warning(_("signing commit with invalid signature\n"
+				  "  allegedly by %s"), signer);
+		break;
+	default:
+		BUG("unsupported signing mode");
+	}
+}
+
+static void handle_signature_if_invalid(struct strbuf *new_data,
+					struct signature_data *sig_sha1,
+					struct signature_data *sig_sha256,
+					struct strbuf *msg,
+					enum sign_mode mode)
+{
+	struct strbuf tmp_buf = STRBUF_INIT;
+	struct signature_check signature_check = { 0 };
+	int ret;
+
+	/* Check signature in a temporary commit buffer */
+	strbuf_addbuf(&tmp_buf, new_data);
+	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
+	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
+
+	if (ret) {
+		warn_invalid_signature(&signature_check, msg->buf, mode);
+
+		if (mode == SIGN_SIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+
+			/*
+			 * NEEDSWORK: To properly support interoperability mode
+			 * when signing commit signatures, the commit buffer
+			 * must be provided in both the repository and
+			 * compatibility object formats. As currently
+			 * implemented, only the repository object format is
+			 * considered meaning compatibility signatures cannot be
+			 * generated. Thus, attempting to sign commit signatures
+			 * in interoperability mode is currently unsupported.
+			 */
+			if (the_repository->compat_hash_algo)
+				die(_("signing commits in interoperability mode is unsupported"));
+
+			strbuf_addstr(&payload, signature_check.payload);
+			if (sign_buffer(&payload, &signature, signed_commit_keyid,
+					SIGN_BUFFER_USE_DEFAULT_KEY))
+				die(_("failed to sign commit object"));
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+		}
 
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
@@ -2927,6 +2978,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_SIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3063,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
+					    &msg, signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3114,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_SIGN_IF_INVALID:
+		die(_("'sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3649,10 +3706,10 @@ static int parse_one_option(const char *option)
 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
 		option_export_pack_edges(option);
 	} else if (skip_prefix(option, "signed-commits=", &option)) {
-		if (parse_sign_mode(option, &signed_commit_mode))
+		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode))
+		if (parse_sign_mode(option, &signed_tag_mode, NULL))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/gpg-interface.c b/gpg-interface.c
index ce935908cc..c26bd32120 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1151,21 +1151,28 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	return ret;
 }
 
-int parse_sign_mode(const char *arg, enum sign_mode *mode)
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 {
-	if (!strcmp(arg, "abort"))
+	if (!strcmp(arg, "abort")) {
 		*mode = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
 		*mode = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
 		*mode = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
+	} else if (!strcmp(arg, "warn-strip")) {
 		*mode = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
+	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
-	else if (!strcmp(arg, "strip-if-invalid"))
+	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
-	else
+	} else if (!strcmp(arg, "sign-if-invalid")) {
+		*mode = SIGN_SIGN_IF_INVALID;
+	} else if (skip_prefix(arg, "sign-if-invalid=", &arg)) {
+		*mode = SIGN_SIGN_IF_INVALID;
+		if (keyid)
+			*keyid = arg;
+	} else {
 		return -1;
+	}
 	return 0;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index 37f3ac42db..a365586ce1 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -120,12 +120,15 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_SIGN_IF_INVALID,
 };
 
 /*
  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
- * otherwise.
+ * otherwise. If the parsed mode is SIGN_SIGN_IF_INVALID and GPG key provided in
+ * the arguments in the form `sign-if-invalid=<keyid>`, the key-ID is parsed
+ * into `char **keyid`.
  */
-int parse_sign_mode(const char *arg, enum sign_mode *mode);
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
 
 #endif
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..38b3e3b537 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,26 +103,85 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
+for mode in strip-if-invalid sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep "stripping invalid signature" log &&
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep "signing commit with invalid signature" log &&
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
 	rm -rf new &&
 	git init new &&
 
@@ -133,41 +192,22 @@ test_expect_success GPG 'strip signature invalidated by message change with --si
 	# corresponding `data <length>` command would have to be changed too.
 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
+	# Configure the target repository with an invalid default signing key.
+	test_config -C new user.signingkey "not-a-real-key-id" &&
+	test_config -C new gpg.format ssh &&
+	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
 
 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
 	test $OPENPGP_SIGNING != $IMPORTED &&
 	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
 	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
-
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
+	git -C new verify-commit "$IMPORTED"
 '
 
 test_done
-- 
2.53.0.381.g628a66ccf6

Re: [PATCH v5 0/3] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> Changes since V4:
> - Instead of introducing a separate `sign_buffer_with_key()` helper,
>   extend `sign_buffer()` to support a SIGN_BUFFER_USE_DEFAULT_KEY flag.
> - Fixed message in die().


I left small comments on two patches, but everything looks quite
well done to me in this iteration.

Re: [PATCH v5 2/3] gpg-interface: allow sign_buffer() to use default signing key

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> -	return use_format->sign_buffer(buffer, signature, signing_key);
> +	if (flags & SIGN_BUFFER_USE_DEFAULT_KEY && (!signing_key || !*signing_key))
> +		signing_key = keyid_to_free = get_signing_key();

Micronit.

I would have preferred to see an extra pair of parentheses here, i.e.,

	if ((flags & SIGN_BUFFER_USE_DEFAULT_KEY) &&
	    (!signing_key || !*signing_key))
                    
It would make it more obvious what two conditions are required to
enter the body, even to those who well know the operator precedence
rules between & and &&.

Re: [PATCH v5 3/3] fast-import: add mode to sign commits with invalid signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> +	case SIGN_SIGN_IF_INVALID:
> +		if (subject_len > 100)
> +			warning(_("signing commit with invalid signature for '%.100s...'\n"
> +				  "  allegedly by %s"), subject, signer);
> +		else if (subject_len > 0)
> +			warning(_("signing commit with invalid signature for '%.*s'\n"
> +				  "  allegedly by %s"), subject_len, subject, signer);
> +		else
> +			warning(_("signing commit with invalid signature\n"
> +				  "  allegedly by %s"), signer);

A very minor point, but my reading hiccuped around these messages,
sounding as if we are adding an invalid signature to the commit.

Perhaps "replacing an invalid signature for commit" or "re-signing
commit that has an invalid signature" or along that lines would
reduce the chance of confusion?

Re: [PATCH v5 2/3] gpg-interface: allow sign_buffer() to use default signing key

[Justin Tobler]

On 26/03/12 01:20PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > -	return use_format->sign_buffer(buffer, signature, signing_key);
> > +	if (flags & SIGN_BUFFER_USE_DEFAULT_KEY && (!signing_key || !*signing_key))
> > +		signing_key = keyid_to_free = get_signing_key();
> 
> Micronit.
> 
> I would have preferred to see an extra pair of parentheses here, i.e.,
> 
> 	if ((flags & SIGN_BUFFER_USE_DEFAULT_KEY) &&
> 	    (!signing_key || !*signing_key))
>                     
> It would make it more obvious what two conditions are required to
> enter the body, even to those who well know the operator precedence
> rules between & and &&.

That's completely fair. I don't mind fixing and sending another version.

Thanks,
-Justin

Re: [PATCH v5 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

On 26/03/12 01:20PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > +	case SIGN_SIGN_IF_INVALID:
> > +		if (subject_len > 100)
> > +			warning(_("signing commit with invalid signature for '%.100s...'\n"
> > +				  "  allegedly by %s"), subject, signer);
> > +		else if (subject_len > 0)
> > +			warning(_("signing commit with invalid signature for '%.*s'\n"
> > +				  "  allegedly by %s"), subject_len, subject, signer);
> > +		else
> > +			warning(_("signing commit with invalid signature\n"
> > +				  "  allegedly by %s"), signer);
> 
> A very minor point, but my reading hiccuped around these messages,
> sounding as if we are adding an invalid signature to the commit.
> 
> Perhaps "replacing an invalid signature for commit" or "re-signing
> commit that has an invalid signature" or along that lines would
> reduce the chance of confusion?

Ya, maybe "replacing invalid signature for commit ..." would be better.
I know Patrick is suggesting we consider getting rid of these warning
messages altogether in a followup series. For now though, I'll update it
in the next version.

Thanks,
-Justin

Re: [PATCH v5 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

On 26/03/12 01:20PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Changes since V4:
> > - Instead of introducing a separate `sign_buffer_with_key()` helper,
> >   extend `sign_buffer()` to support a SIGN_BUFFER_USE_DEFAULT_KEY flag.
> > - Fixed message in die().
> 
> I left small comments on two patches, but everything looks quite
> well done to me in this iteration.

Thanks, I've amended locally and will sent another version a little bit
latter today. :)

-Justin

Re: [PATCH v5 3/3] fast-import: add mode to sign commits with invalid signatures

[Jeff King]

On Thu, Mar 12, 2026 at 02:22:28PM -0500, Justin Tobler wrote:

> +test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
>  	rm -rf new &&
>  	git init new &&

This test is failing on Windows in GitHub's CI.

You can't see it from the context, but the next line of this test is:

	git fast-export --signed-commits=verbatim openpgp-signing >output &&

But the openpgp-signing branch will only have been created if the GPG
prereq is set. Should this be referencing ssh-signing instead? Or should
it be using GPG,GPGSSH as prereqs?

-Peff

Re: [PATCH v5 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

On 26/03/12 07:58PM, Jeff King wrote:
> On Thu, Mar 12, 2026 at 02:22:28PM -0500, Justin Tobler wrote:
> 
> > +test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
> >  	rm -rf new &&
> >  	git init new &&
> 
> This test is failing on Windows in GitHub's CI.
> 
> You can't see it from the context, but the next line of this test is:
> 
> 	git fast-export --signed-commits=verbatim openpgp-signing >output &&
> 
> But the openpgp-signing branch will only have been created if the GPG
> prereq is set. Should this be referencing ssh-signing instead? Or should
> it be using GPG,GPGSSH as prereqs?

Ahh, it look like this should be referencing ssh-signing. I'll include
this fix in my next version. Thanks! :)

-Justin

[PATCH v6 0/3] fast-import: add mode to re-sign invalid commit signatures

[Justin Tobler]

Greetings,

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), it became possible to remove
invalid signatures from commits via git-fast-import(1) while maintaining
valid commit signatures. Building upon this functionality, a user may
want to re-sign these invalid commit signatures. This series introduces
the `sign-if-invalid` mode to do so accordingly.

The newly added mode in this series currently ignores
`extensions.compatObjectFormat` when generating the new signatures. From
my understanding, to generate the compatibility structure would also
require us to reconstruct the compatibility object for the object being
signed. I think this would be possible to do, but would require getting
the mapped OIDs for the commit parents and tree. I'm not completely sure
of a good way to go about this yet though. I'm also not completely
certain if this is something that should be addressed as part of this
series, or could be done later down the road. So for now I've opted to
delay its implementation. I'm open going down the other route if that is
preferred though.

The first commit is a simple cleanup for something I noticed while
reading though commit signing code. The second commit actually
introduces the new `--signed-commits` mode.

Changes since V5:
- Fixed a test that was incorrectly referencing the openpgp-signing
  branch when it should be using the ssh-signing branch.
- Changed warning message wording.
- Added some parentheses in a conditional statement to clarify operation
  order.

Changes since V4:
- Instead of introducing a separate `sign_buffer_with_key()` helper,
  extend `sign_buffer()` to support a SIGN_BUFFER_USE_DEFAULT_KEY flag.
- Fixed message in die().

Changes since V3:
- Rename the `re-sign-if-invalid` mode to `sign-if-invalid`. The
  "if-invalid" already implies the signatures was previously signed
  making "re-sign" redundant.

Changes since V2:
- Adapted commit message in second patch to improve clarity.
- Fixed typos.
- Renamed SIGN_RESIGN_IF_INVALID to SIGN_RE_SIGN_IF_INVALID.
- Created separate helper function to handle printing invalid signature
  warnings.

Changes since V1:
- Improved commit messages and comments to better explain why
  interoperability mode is not currently supported.
- Clarified documentation for re-sign-if-invalid mode.
- Renamed `handle_invalid_signature()` to `handle_signature_if_invalid()`.
- Added warning messages specific to commit resigning.
- Fixed some small typos.
- Added support for explicitly specifying the signing key ID via
  `--signed-commits=re-sign-if-invalid[=<keyid>]` similar to how it can
  specified in git-commit(1).
- We now die() as unsupported when attempting to re-sign an invalid
  commit signature in interoperability mode.
- We now die() when failing to re-sign a commit.

Thanks,
-Justin

Justin Tobler (3):
  commit: remove unused forward declaration
  gpg-interface: allow sign_buffer() to use default signing key
  fast-import: add mode to sign commits with invalid signatures

 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 +++++++++++++++-----
 builtin/tag.c                      |   4 +-
 commit.c                           |  19 +---
 commit.h                           |   2 -
 gpg-interface.c                    |  36 ++++++--
 gpg-interface.h                    |  19 +++-
 send-pack.c                        |   2 +-
 t/t9305-fast-import-signatures.sh  | 144 ++++++++++++++++++-----------
 10 files changed, 231 insertions(+), 108 deletions(-)

Range-diff against v5:
1:  0d00b72ee0 = 1:  0d00b72ee0 commit: remove unused forward declaration
2:  7a0deed77b ! 2:  5224c1766f gpg-interface: allow sign_buffer() to use default signing key
    @@ gpg-interface.c: const char *gpg_trust_level_to_str(enum signature_trust_level l
      	gpg_interface_lazy_init();
      
     -	return use_format->sign_buffer(buffer, signature, signing_key);
    -+	if (flags & SIGN_BUFFER_USE_DEFAULT_KEY && (!signing_key || !*signing_key))
    ++	if ((flags & SIGN_BUFFER_USE_DEFAULT_KEY) && (!signing_key || !*signing_key))
     +		signing_key = keyid_to_free = get_signing_key();
     +
     +	ret = use_format->sign_buffer(buffer, signature, signing_key);
3:  e659971e84 ! 3:  d9ad73e05b fast-import: add mode to sign commits with invalid signatures
    @@ builtin/fast-import.c: static void handle_strip_if_invalid(struct strbuf *new_da
     +		break;
     +	case SIGN_SIGN_IF_INVALID:
     +		if (subject_len > 100)
    -+			warning(_("signing commit with invalid signature for '%.100s...'\n"
    ++			warning(_("replacing invalid signature for commit '%.100s...'\n"
     +				  "  allegedly by %s"), subject, signer);
     +		else if (subject_len > 0)
    -+			warning(_("signing commit with invalid signature for '%.*s'\n"
    ++			warning(_("replacing invalid signature for commit '%.*s'\n"
     +				  "  allegedly by %s"), subject_len, subject, signer);
     +		else
    -+			warning(_("signing commit with invalid signature\n"
    ++			warning(_("replacing invalid signature for commit\n"
     +				  "  allegedly by %s"), signer);
     +		break;
     +	default:
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +			test_grep "stripping invalid signature" log &&
     +			test_grep ! -E "^gpgsig" actual
     +		else
    -+			test_grep "signing commit with invalid signature" log &&
    ++			test_grep "replacing invalid signature" log &&
     +			test_grep -E "^gpgsig(-sha256)? " actual &&
     +			git -C new verify-commit "$IMPORTED"
     +		fi
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
      	rm -rf new &&
      	git init new &&
      
    -@@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip signature invalidated by message change with --si
    - 	# corresponding `data <length>` command would have to be changed too.
    - 	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
    +-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
    ++	git fast-export --signed-commits=verbatim ssh-signing >output &&
      
    + 	# Change the commit message, which invalidates the signature.
    + 	# The commit message length should not change though, otherwise the
    + 	# corresponding `data <length>` command would have to be changed too.
    +-	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
    +-
     -	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
    -+	# Configure the target repository with an invalid default signing key.
    -+	test_config -C new user.signingkey "not-a-real-key-id" &&
    -+	test_config -C new gpg.format ssh &&
    -+	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    -+	test_must_fail git -C new fast-import --quiet \
    -+		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
    -+
    -+	# Import using explicitly provided signing key.
    -+	git -C new fast-import --quiet \
    -+		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
    - 
    - 	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
    - 	test $OPENPGP_SIGNING != $IMPORTED &&
    - 	git -C new cat-file commit "$IMPORTED" >actual &&
    +-
    +-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
    +-	test $OPENPGP_SIGNING != $IMPORTED &&
    +-	git -C new cat-file commit "$IMPORTED" >actual &&
     -	test_grep ! -E "^gpgsig" actual &&
     -	test_grep "stripping invalid signature" log
     -'
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip signature inva
     -	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
     -	test $X509_SIGNING = $IMPORTED &&
     -	git -C new cat-file commit "$IMPORTED" >actual &&
    - 	test_grep -E "^gpgsig(-sha256)? " actual &&
    +-	test_grep -E "^gpgsig(-sha256)? " actual &&
     -	test_must_be_empty log
     -'
     -
     -test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
     -	rm -rf new &&
     -	git init new &&
    --
    --	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    --
    ++	sed "s/SSH signed commit/SSH forged commit/" output >modified &&
    + 
    ++	# Configure the target repository with an invalid default signing key.
    ++	test_config -C new user.signingkey "not-a-real-key-id" &&
    ++	test_config -C new gpg.format ssh &&
    + 	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
    ++	test_must_fail git -C new fast-import --quiet \
    ++		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
    ++
    ++	# Import using explicitly provided signing key.
    ++	git -C new fast-import --quiet \
    ++		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
    + 
     -	git fast-export --signed-commits=verbatim ssh-signing >output &&
     -	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    --	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
    + 	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
     -	test $SSH_SIGNING = $IMPORTED &&
    --	git -C new cat-file commit "$IMPORTED" >actual &&
    --	test_grep -E "^gpgsig(-sha256)? " actual &&
    ++	test $SSH_SIGNING != $IMPORTED &&
    + 	git -C new cat-file commit "$IMPORTED" >actual &&
    + 	test_grep -E "^gpgsig(-sha256)? " actual &&
     -	test_must_be_empty log
     +	git -C new verify-commit "$IMPORTED"
      '

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6

[PATCH v6 1/3] commit: remove unused forward declaration

[Justin Tobler]

In 6206089cbd (commit: write commits for both hashes, 2023-10-01),
`sign_with_header()` was removed, but its forward declaration in
"commit.h" was left. Remove the unused declaration.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 commit.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/commit.h b/commit.h
index 1635de418b..f0c38cb444 100644
--- a/commit.h
+++ b/commit.h
@@ -390,8 +390,6 @@ LAST_ARG_MUST_BE_NULL
 int run_commit_hook(int editor_is_used, const char *index_file,
 		    int *invoked_hook, const char *name, ...);
 
-/* Sign a commit or tag buffer, storing the result in a header. */
-int sign_with_header(struct strbuf *buf, const char *keyid);
 /* Parse the signature out of a header. */
 int parse_buffer_signed_by_header(const char *buffer,
 				  unsigned long size,
-- 
2.53.0.381.g628a66ccf6

[PATCH v6 2/3] gpg-interface: allow sign_buffer() to use default signing key

[Justin Tobler]

The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
logic to get the default configured signing key when a key is not
provided and handles generating the commit signature accordingly. This
signing operation is not really specific to commits as any arbitrary
buffer can be signed. Also, in a subsequent commit, this same logic is
reused by git-fast-import(1) when signing commits with invalid
signatures.

Remove the `sign_commit_to_strbuf()` helper from "commit.c" and extend
`sign_buffer()` in "gpg-interface.c" to support using the default key as
a fallback when the `SIGN_BUFFER_USE_DEFAULT_KEY` flag is provided. Call
sites are updated accordingly.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/tag.c   |  4 ++--
 commit.c        | 19 +++++--------------
 gpg-interface.c | 13 +++++++++++--
 gpg-interface.h | 12 ++++++++++--
 send-pack.c     |  2 +-
 5 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index aeb04c487f..540d783c67 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -167,7 +167,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
 	char *keyid = get_signing_key();
 	int ret = -1;
 
-	if (sign_buffer(buffer, &sig, keyid))
+	if (sign_buffer(buffer, &sig, keyid, 0))
 		goto out;
 
 	if (compat) {
@@ -176,7 +176,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
 		if (convert_object_file(the_repository ,&compat_buf, algo, compat,
 					buffer->buf, buffer->len, OBJ_TAG, 1))
 			goto out;
-		if (sign_buffer(&compat_buf, &compat_sig, keyid))
+		if (sign_buffer(&compat_buf, &compat_sig, keyid, 0))
 			goto out;
 		add_header_signature(&compat_buf, &sig, algo);
 		strbuf_addbuf(&compat_buf, &compat_sig);
diff --git a/commit.c b/commit.c
index d16ae73345..1b9b2d4499 100644
--- a/commit.c
+++ b/commit.c
@@ -1148,18 +1148,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 	return 0;
 }
 
-static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
-{
-	char *keyid_to_free = NULL;
-	int ret = 0;
-	if (!keyid || !*keyid)
-		keyid = keyid_to_free = get_signing_key();
-	if (sign_buffer(buf, sig, keyid))
-		ret = -1;
-	free(keyid_to_free);
-	return ret;
-}
-
 int parse_signed_commit(const struct commit *commit,
 			struct strbuf *payload, struct strbuf *signature,
 			const struct git_hash_algo *algop)
@@ -1737,7 +1725,8 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		oidcpy(&parent_buf[i++], &p->item->object.oid);
 
 	write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
-	if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) {
+	if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
+				       SIGN_BUFFER_USE_DEFAULT_KEY)) {
 		result = -1;
 		goto out;
 	}
@@ -1769,7 +1758,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		free_commit_extra_headers(compat_extra);
 		free(mapped_parents);
 
-		if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) {
+		if (sign_commit && sign_buffer(&compat_buffer, &compat_sig,
+					       sign_commit,
+					       SIGN_BUFFER_USE_DEFAULT_KEY)) {
 			result = -1;
 			goto out;
 		}
diff --git a/gpg-interface.c b/gpg-interface.c
index 87fb6605fb..dca192d5c4 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -973,11 +973,20 @@ const char *gpg_trust_level_to_str(enum signature_trust_level level)
 	return sigcheck_gpg_trust_level[level].display_key;
 }
 
-int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
+int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
+		const char *signing_key, enum sign_buffer_flags flags)
 {
+	char *keyid_to_free = NULL;
+	int ret = 0;
+
 	gpg_interface_lazy_init();
 
-	return use_format->sign_buffer(buffer, signature, signing_key);
+	if ((flags & SIGN_BUFFER_USE_DEFAULT_KEY) && (!signing_key || !*signing_key))
+		signing_key = keyid_to_free = get_signing_key();
+
+	ret = use_format->sign_buffer(buffer, signature, signing_key);
+	free(keyid_to_free);
+	return ret;
 }
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 789d1ffac4..37f3ac42db 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -74,6 +74,15 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct
  */
 size_t parse_signed_buffer(const char *buf, size_t size);
 
+/* Flags for sign_buffer(). */
+enum sign_buffer_flags {
+	/*
+	 * Use the default configured signing key as returned by `get_signing_key()`
+	 * when the provided "signing_key" is NULL or empty.
+	 */
+	SIGN_BUFFER_USE_DEFAULT_KEY = (1 << 0),
+};
+
 /*
  * Create a detached signature for the contents of "buffer" and append
  * it after "signature"; "buffer" and "signature" can be the same
@@ -81,8 +90,7 @@ size_t parse_signed_buffer(const char *buf, size_t size);
  * at the end.  Returns 0 on success, non-zero on failure.
  */
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
-		const char *signing_key);
-
+		const char *signing_key, enum sign_buffer_flags flags);
 
 /*
  * Returns corresponding string in lowercase for a given member of
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..07ecfae4de 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -391,7 +391,7 @@ static int generate_push_cert(struct strbuf *req_buf,
 	if (!update_seen)
 		goto free_return;
 
-	if (sign_buffer(&cert, &cert, signing_key))
+	if (sign_buffer(&cert, &cert, signing_key, 0))
 		die(_("failed to sign the push certificate"));
 
 	packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string);
-- 
2.53.0.381.g628a66ccf6

[PATCH v6 3/3] fast-import: add mode to sign commits with invalid signatures

[Justin Tobler]

With git-fast-import(1), handling of signed commits is controlled via
the `--signed-commits=<mode>` option. When an invalid signature is
encountered, a user may want the option to sign the commit again as
opposed to just stripping the signature. To facilitate this, introduce a
"sign-if-invalid" mode for the `--signed-commits` option. Optionally, a
key ID may be explicitly provided in the form
`sign-if-invalid[=<keyid>]` to specify which signing key should be used
when signing invalid commit signatures.

Note that to properly support interoperability mode when signing commit
signatures, the commit buffer must be created in both the repository and
compatability object formats to generate the appropriate signatures
accordingly. As currently implemented, the commit buffer for the
compatability object format is not reconstructed and thus signing
commits in interoperability mode is not yet supported. Support may be
added in the future.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |   4 +
 builtin/fast-export.c              |   8 +-
 builtin/fast-import.c              | 101 +++++++++++++++-----
 gpg-interface.c                    |  23 +++--
 gpg-interface.h                    |   7 +-
 t/t9305-fast-import-signatures.sh  | 144 ++++++++++++++++++-----------
 6 files changed, 202 insertions(+), 85 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 479c4081da..b3f42d4637 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -86,6 +86,10 @@ already trusted to run their own code.
 * `strip-if-invalid` will check signatures and, if they are invalid,
   will strip them and display a warning. The validation is performed
   in the same way as linkgit:git-verify-commit[1] does it.
+* `sign-if-invalid[=<keyid>]`, similar to `strip-if-invalid`, verifies
+  commit signatures and replaces invalid signatures with newly created ones.
+  Valid signatures are left unchanged. If `<keyid>` is provided, that key is
+  used for signing; otherwise the configured default signing key is used.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0c5d2386d8..13621b0d6a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val))
+	if (parse_sign_mode(arg, val, NULL))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
+		case SIGN_SIGN_IF_INVALID:
+			die(_("'sign-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
+			case SIGN_SIGN_IF_INVALID:
+				die(_("'sign-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index b8a7757cfd..935e688e33 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -190,6 +190,7 @@ static const char *global_prefix;
 
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+static const char *signed_commit_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2836,26 +2837,15 @@ static void finalize_commit_buffer(struct strbuf *new_data,
 	strbuf_addbuf(new_data, msg);
 }
 
-static void handle_strip_if_invalid(struct strbuf *new_data,
-				    struct signature_data *sig_sha1,
-				    struct signature_data *sig_sha256,
-				    struct strbuf *msg)
+static void warn_invalid_signature(struct signature_check *check,
+				   const char *msg, enum sign_mode mode)
 {
-	struct strbuf tmp_buf = STRBUF_INIT;
-	struct signature_check signature_check = { 0 };
-	int ret;
-
-	/* Check signature in a temporary commit buffer */
-	strbuf_addbuf(&tmp_buf, new_data);
-	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
-	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
-
-	if (ret) {
-		const char *signer = signature_check.signer ?
-			signature_check.signer : _("unknown");
-		const char *subject;
-		int subject_len = find_commit_subject(msg->buf, &subject);
+	const char *signer = check->signer ? check->signer : _("unknown");
+	const char *subject;
+	int subject_len = find_commit_subject(msg, &subject);
 
+	switch (mode) {
+	case SIGN_STRIP_IF_INVALID:
 		if (subject_len > 100)
 			warning(_("stripping invalid signature for commit '%.100s...'\n"
 				  "  allegedly by %s"), subject, signer);
@@ -2865,6 +2855,67 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
 		else
 			warning(_("stripping invalid signature for commit\n"
 				  "  allegedly by %s"), signer);
+		break;
+	case SIGN_SIGN_IF_INVALID:
+		if (subject_len > 100)
+			warning(_("replacing invalid signature for commit '%.100s...'\n"
+				  "  allegedly by %s"), subject, signer);
+		else if (subject_len > 0)
+			warning(_("replacing invalid signature for commit '%.*s'\n"
+				  "  allegedly by %s"), subject_len, subject, signer);
+		else
+			warning(_("replacing invalid signature for commit\n"
+				  "  allegedly by %s"), signer);
+		break;
+	default:
+		BUG("unsupported signing mode");
+	}
+}
+
+static void handle_signature_if_invalid(struct strbuf *new_data,
+					struct signature_data *sig_sha1,
+					struct signature_data *sig_sha256,
+					struct strbuf *msg,
+					enum sign_mode mode)
+{
+	struct strbuf tmp_buf = STRBUF_INIT;
+	struct signature_check signature_check = { 0 };
+	int ret;
+
+	/* Check signature in a temporary commit buffer */
+	strbuf_addbuf(&tmp_buf, new_data);
+	finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
+	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
+
+	if (ret) {
+		warn_invalid_signature(&signature_check, msg->buf, mode);
+
+		if (mode == SIGN_SIGN_IF_INVALID) {
+			struct strbuf signature = STRBUF_INIT;
+			struct strbuf payload = STRBUF_INIT;
+
+			/*
+			 * NEEDSWORK: To properly support interoperability mode
+			 * when signing commit signatures, the commit buffer
+			 * must be provided in both the repository and
+			 * compatibility object formats. As currently
+			 * implemented, only the repository object format is
+			 * considered meaning compatibility signatures cannot be
+			 * generated. Thus, attempting to sign commit signatures
+			 * in interoperability mode is currently unsupported.
+			 */
+			if (the_repository->compat_hash_algo)
+				die(_("signing commits in interoperability mode is unsupported"));
+
+			strbuf_addstr(&payload, signature_check.payload);
+			if (sign_buffer(&payload, &signature, signed_commit_keyid,
+					SIGN_BUFFER_USE_DEFAULT_KEY))
+				die(_("failed to sign commit object"));
+			add_header_signature(new_data, &signature, the_hash_algo);
+
+			strbuf_release(&signature);
+			strbuf_release(&payload);
+		}
 
 		finalize_commit_buffer(new_data, NULL, NULL, msg);
 	} else {
@@ -2927,6 +2978,7 @@ static void parse_new_commit(const char *arg)
 			/* fallthru */
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
+		case SIGN_SIGN_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3011,9 +3063,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
+	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
-		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
+					    &msg, signed_commit_mode);
 	else
 		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
@@ -3060,6 +3114,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
+	case SIGN_SIGN_IF_INVALID:
+		die(_("'sign-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3649,10 +3706,10 @@ static int parse_one_option(const char *option)
 	} else if (skip_prefix(option, "export-pack-edges=", &option)) {
 		option_export_pack_edges(option);
 	} else if (skip_prefix(option, "signed-commits=", &option)) {
-		if (parse_sign_mode(option, &signed_commit_mode))
+		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode))
+		if (parse_sign_mode(option, &signed_tag_mode, NULL))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/gpg-interface.c b/gpg-interface.c
index dca192d5c4..32f2880976 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1151,21 +1151,28 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	return ret;
 }
 
-int parse_sign_mode(const char *arg, enum sign_mode *mode)
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 {
-	if (!strcmp(arg, "abort"))
+	if (!strcmp(arg, "abort")) {
 		*mode = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+	} else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) {
 		*mode = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+	} else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) {
 		*mode = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
+	} else if (!strcmp(arg, "warn-strip")) {
 		*mode = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
+	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
-	else if (!strcmp(arg, "strip-if-invalid"))
+	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
-	else
+	} else if (!strcmp(arg, "sign-if-invalid")) {
+		*mode = SIGN_SIGN_IF_INVALID;
+	} else if (skip_prefix(arg, "sign-if-invalid=", &arg)) {
+		*mode = SIGN_SIGN_IF_INVALID;
+		if (keyid)
+			*keyid = arg;
+	} else {
 		return -1;
+	}
 	return 0;
 }
diff --git a/gpg-interface.h b/gpg-interface.h
index 37f3ac42db..a365586ce1 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -120,12 +120,15 @@ enum sign_mode {
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
 	SIGN_STRIP_IF_INVALID,
+	SIGN_SIGN_IF_INVALID,
 };
 
 /*
  * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
- * otherwise.
+ * otherwise. If the parsed mode is SIGN_SIGN_IF_INVALID and GPG key provided in
+ * the arguments in the form `sign-if-invalid=<keyid>`, the key-ID is parsed
+ * into `char **keyid`.
  */
-int parse_sign_mode(const char *arg, enum sign_mode *mode);
+int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid);
 
 #endif
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 022dae02e4..ac4228127a 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,71 +103,111 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
 	test_line_count = 2 out
 '
 
-test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
-	git fast-export main >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
+for mode in strip-if-invalid sign-if-invalid
+do
+	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
+		git fast-export main >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim openpgp-signing >output &&
+
+		# Change the commit message, which invalidates the signature.
+		# The commit message length should not change though, otherwise the
+		# corresponding `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
+
+		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+		test $OPENPGP_SIGNING != $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep "stripping invalid signature" log &&
+			test_grep ! -E "^gpgsig" actual
+		else
+			test_grep "replacing invalid signature" log &&
+			test_grep -E "^gpgsig(-sha256)? " actual &&
+			git -C new verify-commit "$IMPORTED"
+		fi
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		git fast-export --signed-commits=verbatim x509-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" '
+		rm -rf new &&
+		git init new &&
+
+		test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-commits=verbatim ssh-signing >output &&
+		git -C new fast-import --quiet --signed-commits=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
+		test $SSH_SIGNING = $IMPORTED &&
+		git -C new cat-file commit "$IMPORTED" >actual &&
+		test_grep -E "^gpgsig(-sha256)? " actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH "sign invalid commit with explicit keyid" '
 	rm -rf new &&
 	git init new &&
 
-	git fast-export --signed-commits=verbatim openpgp-signing >output &&
+	git fast-export --signed-commits=verbatim ssh-signing >output &&
 
 	# Change the commit message, which invalidates the signature.
 	# The commit message length should not change though, otherwise the
 	# corresponding `data <length>` command would have to be changed too.
-	sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
-
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
-
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-	test $OPENPGP_SIGNING != $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep ! -E "^gpgsig" actual &&
-	test_grep "stripping invalid signature" log
-'
-
-test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
-
-	git fast-export --signed-commits=verbatim x509-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
-	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
-	test $X509_SIGNING = $IMPORTED &&
-	git -C new cat-file commit "$IMPORTED" >actual &&
-	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
-'
-
-test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
-	rm -rf new &&
-	git init new &&
+	sed "s/SSH signed commit/SSH forged commit/" output >modified &&
 
+	# Configure the target repository with an invalid default signing key.
+	test_config -C new user.signingkey "not-a-real-key-id" &&
+	test_config -C new gpg.format ssh &&
 	test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C new fast-import --quiet \
+		--signed-commits=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
 
-	git fast-export --signed-commits=verbatim ssh-signing >output &&
-	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
 	IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
-	test $SSH_SIGNING = $IMPORTED &&
+	test $SSH_SIGNING != $IMPORTED &&
 	git -C new cat-file commit "$IMPORTED" >actual &&
 	test_grep -E "^gpgsig(-sha256)? " actual &&
-	test_must_be_empty log
+	git -C new verify-commit "$IMPORTED"
 '
 
 test_done
-- 
2.53.0.381.g628a66ccf6

Re: [PATCH v6 0/3] fast-import: add mode to re-sign invalid commit signatures

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> Changes since V5:
> - Fixed a test that was incorrectly referencing the openpgp-signing
>   branch when it should be using the ssh-signing branch.
> - Changed warning message wording.
> - Added some parentheses in a conditional statement to clarify operation
>   order.

All three of the above look familiar ;-)  Looking good.

Will replace.  Thanks.

Re: [PATCH v6 2/3] gpg-interface: allow sign_buffer() to use default signing key

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 08:39:37PM -0500, Justin Tobler wrote:
> The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback
> logic to get the default configured signing key when a key is not
> provided and handles generating the commit signature accordingly. This
> signing operation is not really specific to commits as any arbitrary
> buffer can be signed. Also, in a subsequent commit, this same logic is
> reused by git-fast-import(1) when signing commits with invalid
> signatures.
> 
> Remove the `sign_commit_to_strbuf()` helper from "commit.c" and extend
> `sign_buffer()` in "gpg-interface.c" to support using the default key as
> a fallback when the `SIGN_BUFFER_USE_DEFAULT_KEY` flag is provided. Call
> sites are updated accordingly.

Thanks, this looks much nicer to me now.

Patrick

Re: [PATCH v6 3/3] fast-import: add mode to sign commits with invalid signatures

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 08:39:38PM -0500, Justin Tobler wrote:
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index b8a7757cfd..935e688e33 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2865,6 +2855,67 @@ static void handle_strip_if_invalid(struct strbuf *new_data,
>  		else
>  			warning(_("stripping invalid signature for commit\n"
>  				  "  allegedly by %s"), signer);
> +		break;
> +	case SIGN_SIGN_IF_INVALID:
> +		if (subject_len > 100)
> +			warning(_("replacing invalid signature for commit '%.100s...'\n"
> +				  "  allegedly by %s"), subject, signer);
> +		else if (subject_len > 0)
> +			warning(_("replacing invalid signature for commit '%.*s'\n"
> +				  "  allegedly by %s"), subject_len, subject, signer);
> +		else
> +			warning(_("replacing invalid signature for commit\n"
> +				  "  allegedly by %s"), signer);
> +		break;
> +	default:
> +		BUG("unsupported signing mode");
> +	}
> +}

The wording of those warnings also reads better than before now.

Patrick

Re: [PATCH v6 0/3] fast-import: add mode to re-sign invalid commit signatures

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 09:29:37PM -0700, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > Changes since V5:
> > - Fixed a test that was incorrectly referencing the openpgp-signing
> >   branch when it should be using the ssh-signing branch.
> > - Changed warning message wording.
> > - Added some parentheses in a conditional statement to clarify operation
> >   order.
> 
> All three of the above look familiar ;-)  Looking good.
> 
> Will replace.  Thanks.

I'm happy with this version. Thanks!

Patrick