[PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

The `--signed-commits=<mode>` option in `git fast-import` allows users
to decide what should be done when commits with signatures are
imported.

For tools like `git filter-repo`, it would be useful to be able to
strip signatures when they are invalid, so let's add a new
'strip-if-invalid' mode for that purpose.

Maybe this new mode should become the default mode, but this would be
breaking backward compatibility, and perhaps this could be decided
after other new modes that might be even better default modes have
been added. So we leave that for future work.

This 'strip-if-invalid' mode should also be added to
`--signed-tags=<mode>`, but we leave that for future work too.

CI tests
========

They have all passed, see:

https://github.com/chriscool/git/actions/runs/19091593841/job/54543228129

Christian Couder (3):
  fast-import: refactor finalize_commit_buffer()
  commit: refactor verify_commit_buffer()
  fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

 Documentation/git-fast-import.adoc |  28 ++++---
 builtin/fast-export.c              |  46 ++++++++---
 builtin/fast-import.c              |  74 +++++++++++++++---
 commit.c                           |  17 ++++-
 commit.h                           |   7 ++
 gpg-interface.c                    |   2 +
 gpg-interface.h                    |   1 +
 t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-
 8 files changed, 260 insertions(+), 33 deletions(-)

-- 
2.52.0.rc0.3.gf264cd25e5

[PATCH 1/3] fast-import: refactor finalize_commit_buffer()

[Christian Couder]

In a following commit we are going to finalize commit buffers with or
without signatures in order to check the signatures and possibly drop
them.

To do so easily and without duplication, let's refactor the current
code that finalizes commit buffers into a new finalize_commit_buffer()
function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fast-import.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 54d3e592c6..493de57ef6 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2815,6 +2815,18 @@ static void import_one_signature(struct signature_data *sig_sha1,
 		die(_("parse_one_signature() returned unknown hash algo"));
 }
 
+static void finalize_commit_buffer(struct strbuf *new_data,
+				   struct signature_data *sig_sha1,
+				   struct signature_data *sig_sha256,
+				   struct strbuf *msg)
+{
+	add_gpgsig_to_commit(new_data, "gpgsig ", sig_sha1);
+	add_gpgsig_to_commit(new_data, "gpgsig-sha256 ", sig_sha256);
+
+	strbuf_addch(new_data, '\n');
+	strbuf_addbuf(new_data, msg);
+}
+
 static void parse_new_commit(const char *arg)
 {
 	static struct strbuf msg = STRBUF_INIT;
@@ -2950,11 +2962,8 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	add_gpgsig_to_commit(&new_data, "gpgsig ", &sig_sha1);
-	add_gpgsig_to_commit(&new_data, "gpgsig-sha256 ", &sig_sha256);
+	finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
-	strbuf_addch(&new_data, '\n');
-	strbuf_addbuf(&new_data, &msg);
 	free(author);
 	free(committer);
 	free(encoding);
-- 
2.52.0.rc0.3.gf264cd25e5

[PATCH 2/3] commit: refactor verify_commit_buffer()

[Christian Couder]

In a following commit, we are going to check commit signatures, but we
won't have a commit yet, only a commit buffer, and we are going to
discard this commit buffer if the signature is invalid. So it would be
wasteful to create a commit that we might discard, just to be able to
check a commit signature.

It would be simpler instead to be able to check commit signatures
using only a commit buffer instead of a commit.

To be able to do that, let's extract some code from the
check_commit_signature() function into a new verify_commit_buffer()
function, and then let's make check_commit_signature() call
verify_commit_buffer().

Note that this doesn't fundamentally change how
check_commit_signature() works. It used to call parse_signed_commit()
which calls repo_get_commit_buffer(), parse_buffer_signed_by_header()
and repo_unuse_commit_buffer(). Now these 3 functions are called
directly by verify_commit_buffer().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 commit.c | 17 +++++++++++++++--
 commit.h |  7 +++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index 16d91b2bfc..709c9eed58 100644
--- a/commit.c
+++ b/commit.c
@@ -1315,7 +1315,8 @@ static void handle_signed_tag(const struct commit *parent, struct commit_extra_h
 	free(buf);
 }
 
-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+int verify_commit_buffer(const char *buffer, size_t size,
+			 struct signature_check *sigc)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
@@ -1323,7 +1324,8 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
 
 	sigc->result = 'N';
 
-	if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
+	if (parse_buffer_signed_by_header(buffer, size, &payload,
+					  &signature, the_hash_algo) <= 0)
 		goto out;
 
 	sigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;
@@ -1337,6 +1339,17 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
 	return ret;
 }
 
+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+{
+	unsigned long size;
+	const char *buffer = repo_get_commit_buffer(the_repository, commit, &size);
+	int ret = verify_commit_buffer(buffer, size, sigc);
+
+	repo_unuse_commit_buffer(the_repository, commit, buffer);
+
+	return ret;
+}
+
 void verify_merge_signature(struct commit *commit, int verbosity,
 			    int check_trust)
 {
diff --git a/commit.h b/commit.h
index 1d6e0c7518..5406dd2663 100644
--- a/commit.h
+++ b/commit.h
@@ -333,6 +333,13 @@ int remove_signature(struct strbuf *buf);
  */
 int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
 
+/*
+ * Same as check_commit_signature() but accepts a commit buffer and
+ * its size, instead of a `struct commit *`.
+ */
+int verify_commit_buffer(const char *buffer, size_t size,
+			 struct signature_check *sigc);
+
 /* record author-date for each commit object */
 struct author_date_slab;
 void record_author_date(struct author_date_slab *author_date,
-- 
2.52.0.rc0.3.gf264cd25e5

[PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

Tools like `git filter-repo`[1] use `git fast-export` and
`git fast-import` to rewrite repository history. When rewriting
history using one such tool though, commit signatures might become
invalid because the commits they sign changed due to the changes
in the repository history made by the tool between the fast-export
and the fast-import steps.

Having invalid signatures in a rewritten repository could be
confusing, so users rewritting history might prefer to simply
discard signatures that are invalid at the fast-import step.

To let them do that, let's add a new 'strip-if-invalid' mode to the
`--signed-commits=<mode>` option of `git fast-import`.

It would be interesting for the `--signed-tags=<mode>` option to
have this mode too, but we leave that for a future improvement.

It might also be possible for `git fast-export` to have such a mode
in its `--signed-commits=<mode>` and `--signed-tags=<mode>`
options, but the use cases for it are much less clear, so we also
leave that for possible future improvements.

For now let's just die() if 'strip-if-invalid' is passed to these
options where it hasn't been implemented yet.

While at it, let's also mark for translation some error messages
linked to the `--signed-commits=<mode>` and `--signed-tags=<mode>`
in `git fast-export`.

[1]: https://github.com/newren/git-filter-repo

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-fast-import.adoc |  28 ++++---
 builtin/fast-export.c              |  46 ++++++++---
 builtin/fast-import.c              |  59 +++++++++++++--
 gpg-interface.c                    |   2 +
 gpg-interface.h                    |   1 +
 t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-
 6 files changed, 226 insertions(+), 28 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index b74179a6c8..c9e49497cd 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,15 +66,25 @@ fast-import stream! This option is enabled automatically for
 remote-helpers that use the `import` capability, as they are
 already trusted to run their own code.
 
---signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
-	Specify how to handle signed tags.  Behaves in the same way
-	as the same option in linkgit:git-fast-export[1], except that
-	default is 'verbatim' (instead of 'abort').
-
---signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
-	Specify how to handle signed commits.  Behaves in the same way
-	as the same option in linkgit:git-fast-export[1], except that
-	default is 'verbatim' (instead of 'abort').
+`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::
+	Specify how to handle signed tags. Behaves in the same way as
+	the `--signed-commits=<mode>` below, except that the
+	`strip-if-invalid` mode is not yet supported. Like for signed
+	commits, the default mode is `verbatim`.
+
+`--signed-commits=<mode>`::
+	Specify how to handle signed commits. The following <mode>s
+	are supported:
++
+* `verbatim`, which is the default, will silently import commit
+  signatures.
+* `warn-verbatim` will import them, but will display a warning.
+* `abort` will make this program die when encountering a signed
+  commit.
+* `strip` will silently make the commits unsigned.
+* `warn-strip` will make them unsigned, but will display a warning.
+* `strip-if-invalid` will check signatures and, if they are invalid,
+  will strip them and display a warning.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7adbc55f0d..1ad195b639 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -797,12 +797,10 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 	       (int)(committer_end - committer), committer);
 	if (signatures.nr) {
 		switch (signed_commit_mode) {
-		case SIGN_ABORT:
-			die("encountered signed commit %s; use "
-			    "--signed-commits=<mode> to handle it",
-			    oid_to_hex(&commit->object.oid));
+
+		/* Exporting modes */
 		case SIGN_WARN_VERBATIM:
-			warning("exporting %"PRIuMAX" signature(s) for commit %s",
+			warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
 				(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
 			/* fallthru */
 		case SIGN_VERBATIM:
@@ -811,12 +809,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 				print_signature(item->string, item->util);
 			}
 			break;
+
+		/* Stripping modes */
 		case SIGN_WARN_STRIP:
-			warning("stripping signature(s) from commit %s",
+			warning(_("stripping signature(s) from commit %s"),
 				oid_to_hex(&commit->object.oid));
 			/* fallthru */
 		case SIGN_STRIP:
 			break;
+
+		/* Aborting modes */
+		case SIGN_ABORT:
+			die(_("encountered signed commit %s; use "
+			      "--signed-commits=<mode> to handle it"),
+			    oid_to_hex(&commit->object.oid));
+		case SIGN_STRIP_IF_INVALID:
+			die(_("'strip-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);
 		}
 		string_list_clear(&signatures, 0);
 	}
@@ -934,23 +945,34 @@ static void handle_tag(const char *name, struct tag *tag)
 		size_t sig_offset = parse_signed_buffer(message, message_size);
 		if (sig_offset < message_size)
 			switch (signed_tag_mode) {
-			case SIGN_ABORT:
-				die("encountered signed tag %s; use "
-				    "--signed-tags=<mode> to handle it",
-				    oid_to_hex(&tag->object.oid));
+
+			/* Exporting modes */
 			case SIGN_WARN_VERBATIM:
-				warning("exporting signed tag %s",
+				warning(_("exporting signed tag %s"),
 					oid_to_hex(&tag->object.oid));
 				/* fallthru */
 			case SIGN_VERBATIM:
 				break;
+
+			/* Stripping modes */
 			case SIGN_WARN_STRIP:
-				warning("stripping signature from tag %s",
+				warning(_("stripping signature from tag %s"),
 					oid_to_hex(&tag->object.oid));
 				/* fallthru */
 			case SIGN_STRIP:
 				message_size = sig_offset;
 				break;
+
+			/* Aborting modes */
+			case SIGN_ABORT:
+				die(_("encountered signed tag %s; use "
+				      "--signed-tags=<mode> to handle it"),
+				    oid_to_hex(&tag->object.oid));
+			case SIGN_STRIP_IF_INVALID:
+				die(_("'strip-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 493de57ef6..e2c6894461 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2772,7 +2772,7 @@ static void add_gpgsig_to_commit(struct strbuf *commit_data,
 {
 	struct string_list siglines = STRING_LIST_INIT_NODUP;
 
-	if (!sig->hash_algo)
+	if (!sig || !sig->hash_algo)
 		return;
 
 	strbuf_addstr(commit_data, header);
@@ -2827,6 +2827,45 @@ 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)
+{
+	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);
+
+		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);
+
+		finalize_commit_buffer(new_data, NULL, NULL, msg);
+	} else {
+		strbuf_swap(new_data, &tmp_buf);
+	}
+
+	signature_check_clear(&signature_check);
+	strbuf_release(&tmp_buf);
+}
+
 static void parse_new_commit(const char *arg)
 {
 	static struct strbuf msg = STRBUF_INIT;
@@ -2878,6 +2917,7 @@ static void parse_new_commit(const char *arg)
 			warning(_("importing a commit signature verbatim"));
 			/* fallthru */
 		case SIGN_VERBATIM:
+		case SIGN_STRIP_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -2962,7 +3002,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
+	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
+		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+	else
+		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
 	free(author);
 	free(committer);
@@ -2984,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	switch (signed_tag_mode) {
 
 	/* First, modes that don't change anything */
-	case SIGN_ABORT:
-		die(_("encountered signed tag; use "
-		      "--signed-tags=<mode> to handle it"));
 	case SIGN_WARN_VERBATIM:
 		warning(_("importing a tag signature verbatim for tag '%s'"), name);
 		/* fallthru */
@@ -3003,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 		strbuf_setlen(msg, sig_offset);
 		break;
 
-	/* Third, BUG */
+	/* Third, aborting modes */
+	case SIGN_ABORT:
+		die(_("encountered signed tag; use "
+		      "--signed-tags=<mode> to handle it"));
+	case SIGN_STRIP_IF_INVALID:
+		die(_("'strip-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 d1e88da8c1..fe653b2464 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1146,6 +1146,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
 		*mode = SIGN_WARN_STRIP;
 	else if (!strcmp(arg, "strip"))
 		*mode = SIGN_STRIP;
+	else if (!strcmp(arg, "strip-if-invalid"))
+		*mode = SIGN_STRIP_IF_INVALID;
 	else
 		return -1;
 	return 0;
diff --git a/gpg-interface.h b/gpg-interface.h
index 50487aa148..71dde8cb80 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -111,6 +111,7 @@ enum sign_mode {
 	SIGN_VERBATIM,
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
+	SIGN_STRIP_IF_INVALID,
 };
 
 /*
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index c2b4271658..db77ace472 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -79,7 +79,7 @@ test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-
 	echo B >explicit-sha256/B &&
 	git -C explicit-sha256 add B &&
 	test_tick &&
-	git -C explicit-sha256 commit -S -m "signed" B &&
+	git -C explicit-sha256 commit -S -m "signed commit" B &&
 	SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
 
 	# Create the corresponding SHA-1 commit
@@ -103,4 +103,120 @@ 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 GPG 'keep valid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
+	rm -rf new &&
+	git init new &&
+
+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
+	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
+
+	git -C new cat-file commit refs/heads/dual-signed >actual &&
+	test_grep -E "^gpgsig " actual &&
+	test_grep -E "^gpgsig-sha256 " actual &&
+	test_must_be_empty log &&
+
+	IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
+	if test "$GIT_DEFAULT_HASH" = "sha1"
+	then
+		test $SHA1_B = $IMPORTED
+	else
+		test $SHA256_B = $IMPORTED
+	fi
+'
+
+test_expect_success GPG 'strip both invalid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
+	rm -rf new &&
+	git init new &&
+
+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >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/signed commit/forged commit/" output >modified &&
+
+	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
+
+	git -C new cat-file commit refs/heads/dual-signed >actual &&
+	test_grep ! -E "^gpgsig " actual &&
+	test_grep ! -E "^gpgsig-sha256 " actual &&
+
+	IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
+	if test "$GIT_DEFAULT_HASH" = "sha1"
+	then
+		test $SHA1_B != $IMPORTED
+	else
+		test $SHA256_B != $IMPORTED
+	fi &&
+
+	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
+'
+
 test_done
-- 
2.52.0.rc0.3.gf264cd25e5

Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

>  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-
>  6 files changed, 226 insertions(+), 28 deletions(-)

Unfortunately all these tests that assume that explicit-sha256
repository as a subdirectory exists would fail when the topic is
merged to 'seen' and the tree is built without the optional Rust
support.  This is because brian's f6581e23 (repository: require Rust
support for interoperability, 2025-10-27) changes a couple of tests
to require RUST prerequisite.  One of them is what creates the
explicit-sha256 repository.

I do not think this topic to preserve or strip GPG signatures
particularly cares about the dual hash interoperability, so can you
rearrange the tests in this series to avoid crashing with the other
topic?

Thanks.

Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

On Sat, Nov 8, 2025 at 7:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> >  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-
> >  6 files changed, 226 insertions(+), 28 deletions(-)
>
> Unfortunately all these tests that assume that explicit-sha256
> repository as a subdirectory exists would fail when the topic is
> merged to 'seen' and the tree is built without the optional Rust
> support.  This is because brian's f6581e23 (repository: require Rust
> support for interoperability, 2025-10-27) changes a couple of tests
> to require RUST prerequisite.  One of them is what creates the
> explicit-sha256 repository.
>
> I do not think this topic to preserve or strip GPG signatures
> particularly cares about the dual hash interoperability, so can you
> rearrange the tests in this series to avoid crashing with the other
> topic?

I will do that in the v2 I hope I can send soon.

Thanks for telling me about this issue.

Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> On Sat, Nov 8, 2025 at 7:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Christian Couder <christian.couder@gmail.com> writes:
>>
>> >  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-
>> >  6 files changed, 226 insertions(+), 28 deletions(-)
>>
>> Unfortunately all these tests that assume that explicit-sha256
>> repository as a subdirectory exists would fail when the topic is
>> merged to 'seen' and the tree is built without the optional Rust
>> support.  This is because brian's f6581e23 (repository: require Rust
>> support for interoperability, 2025-10-27) changes a couple of tests
>> to require RUST prerequisite.  One of them is what creates the
>> explicit-sha256 repository.
>>
>> I do not think this topic to preserve or strip GPG signatures
>> particularly cares about the dual hash interoperability, so can you
>> rearrange the tests in this series to avoid crashing with the other
>> topic?
>
> I will do that in the v2 I hope I can send soon.

Thanks.

Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> The `--signed-commits=<mode>` option in `git fast-import` allows users
> to decide what should be done when commits with signatures are
> imported.
>
> For tools like `git filter-repo`, it would be useful to be able to
> strip signatures when they are invalid, so let's add a new
> 'strip-if-invalid' mode for that purpose.

Sorry, but I do not get it.  What is your definition of a signature
being "invalid", and what is your assumptions of how accurate a
validity check ought to be?  For example, are you assuming that you
have all the necessary public keys, revocation data and accurate
clock?  Even if you are not changing a single bit in the import,
some of your early commits' signatures do not "validate" and may
need to be stripped, and after that happens, wouldn't signatures of
all later commits become unusable (i.e, you may be able to verify
that the signature on the original commit object may still be valid,
but because the commit has to become a child of a rewritten commit,
in the resulting history the signature would no longer match)?

Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Elijah Newren]

On Wed, Nov 5, 2025 at 6:40 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > The `--signed-commits=<mode>` option in `git fast-import` allows users
> > to decide what should be done when commits with signatures are
> > imported.
> >
> > For tools like `git filter-repo`, it would be useful to be able to
> > strip signatures when they are invalid, so let's add a new
> > 'strip-if-invalid' mode for that purpose.
>
> Sorry, but I do not get it.  What is your definition of a signature
> being "invalid", and what is your assumptions of how accurate a
> validity check ought to be?  For example, are you assuming that you
> have all the necessary public keys, revocation data and accurate
> clock?  Even if you are not changing a single bit in the import,
> some of your early commits' signatures do not "validate" and may
> need to be stripped, and after that happens, wouldn't signatures of
> all later commits become unusable (i.e, you may be able to verify
> that the signature on the original commit object may still be valid,
> but because the commit has to become a child of a rewritten commit,
> in the resulting history the signature would no longer match)?

Good questions.  Let me step back and perhaps motivate the change a bit:

There's a fairly significant chunk of `git filter-repo` users who also
have git histories with commit or tag signatures in their history.
They often want to specify rules for rewriting history which happen to
only affect "recent" commits.  While they could try to specify commit
ranges corresponding to "recent" commits, they worry about getting it
wrong and want to just automatically rewrite everything, expecting
older commit signatures to be untouched (since the modification rules
didn't need to modify older commits), and get new commit OIDs starting
with the first commit that was modified by one of the rewrite rules.
Unfortunately, when fast-export exports history, it does so without
signatures, and thus they get every commit rewritten, not just the
recent history.

Christian's previous series allows us to have fast-export also export
the signatures, but then we run into the problem of determining
whether those signatures are still valid and what to do if they
aren't.  This series attempts to help us determine if they are valid,
and implements one choice when they aren't (strip), in addition to one
that the previous series implemented (keep-it-anyway), while leaving
another (re-sign) for future work.

So, yeah, I'd presume this mode would have to assume the user had all
the necessary public keys in order for fast-import to be able to check
validity.  Perhaps that is a tall order for a small percentage of
repos out there, but for them, is there any good alternative?

As far as signature handling goes:
  * Since fast-export doesn't know what changes filter-repo may make
to the stream, it can't know whether the signatures will still be
valid
  * Since filter-repo doesn't know what history canonicalizations
fast-export performed (and it performs a few), it can't know whether
the signatures will still be valid
  * Therefore, fast-import is the only process in the pipeline that
can know whether a specified signature remains valid

I guess one alternative would be having fast-export include for any
signed commit, what that signed commit's OID would have been had it
been unsigned.  That would allow fast-import to check what the commit
OID would be without the signature, and if it matches, then just keep
the signature without checking whether it's actually valid.  It'd be a
change to the fast-export & fast-import format to get such an extra
piece of data, but perhaps that would be a preferable strategy?  It's
the only alternative I can think of to what Christian is doing here;
am I missing others?

Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

On Sat, Nov 8, 2025 at 1:35 AM Elijah Newren <newren@gmail.com> wrote:

> Good questions.  Let me step back and perhaps motivate the change a bit:
>
> There's a fairly significant chunk of `git filter-repo` users who also
> have git histories with commit or tag signatures in their history.
> They often want to specify rules for rewriting history which happen to
> only affect "recent" commits.  While they could try to specify commit
> ranges corresponding to "recent" commits, they worry about getting it
> wrong and want to just automatically rewrite everything, expecting
> older commit signatures to be untouched (since the modification rules
> didn't need to modify older commits), and get new commit OIDs starting
> with the first commit that was modified by one of the rewrite rules.
> Unfortunately, when fast-export exports history, it does so without
> signatures, and thus they get every commit rewritten, not just the
> recent history.
>
> Christian's previous series allows us to have fast-export also export
> the signatures, but then we run into the problem of determining
> whether those signatures are still valid and what to do if they
> aren't.  This series attempts to help us determine if they are valid,
> and implements one choice when they aren't (strip), in addition to one
> that the previous series implemented (keep-it-anyway), while leaving
> another (re-sign) for future work.

Thanks for a great description of the context motivating this series.

> So, yeah, I'd presume this mode would have to assume the user had all
> the necessary public keys in order for fast-import to be able to check
> validity.  Perhaps that is a tall order for a small percentage of
> repos out there, but for them, is there any good alternative?
>
> As far as signature handling goes:
>   * Since fast-export doesn't know what changes filter-repo may make
> to the stream, it can't know whether the signatures will still be
> valid
>   * Since filter-repo doesn't know what history canonicalizations
> fast-export performed (and it performs a few), it can't know whether
> the signatures will still be valid
>   * Therefore, fast-import is the only process in the pipeline that
> can know whether a specified signature remains valid

I agree with this analysis.

> I guess one alternative would be having fast-export include for any
> signed commit, what that signed commit's OID would have been had it
> been unsigned.  That would allow fast-import to check what the commit
> OID would be without the signature, and if it matches, then just keep
> the signature without checking whether it's actually valid.  It'd be a
> change to the fast-export & fast-import format to get such an extra
> piece of data, but perhaps that would be a preferable strategy?

It would be a different strategy. Perhaps useful for some people, but
I think it could have drawbacks.

For example since signatures are not checked at export time, it's
possible that some invalid signatures at export time would still be
imported back. Also what if the signature becomes invalid between
export and import times because for example some keys are revoked?

If invalid signatures can actually be imported, then a name like
'strip-if-invalid' could be deceptive, so such a strategy should
probably have a different name.

> It's
> the only alternative I can think of to what Christian is doing here;
> am I missing others?

I think what I am implementing is what most people would expect. So I
think it's worth implementing even if in some cases another strategy
might be better.

Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

On Wed, Nov 5, 2025 at 3:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> > The `--signed-commits=<mode>` option in `git fast-import` allows users
> > to decide what should be done when commits with signatures are
> > imported.
> >
> > For tools like `git filter-repo`, it would be useful to be able to
> > strip signatures when they are invalid, so let's add a new
> > 'strip-if-invalid' mode for that purpose.
>
> Sorry, but I do not get it.  What is your definition of a signature
> being "invalid", and what is your assumptions of how accurate a
> validity check ought to be?

The definition of "valid" is the same as the definition used by `git
verify-commit`. The description of this command is:

"Validates the GPG signature created by `git commit -S` on the commit
objects given on the command line."

Here we just also "validate" commit signatures in the same way and
using the same underlying code. If `git verify-commit` would return 0,
we consider the commit signature valid, otherwise we consider it
invalid.

I will add such clarification to the documentation of the feature in
the v2 I plan to send soon.

> For example, are you assuming that you
> have all the necessary public keys, revocation data and accurate
> clock?

Yes, we assume all that, like `git verify-commit` assumes it has all that too.

If we want to be clearer about what is needed to make sure that commit
signatures can be properly validated, I think we should start with
working on `git verify-commit` and improve its related documentation,
and perhaps even some of its features. It would be simpler to have all
the docs and features about this there, and just refer to that command
(using for example "see git-verify-commit(1)") in other places. Such
`git verify-commit` improvements could be in a separate patch series
though.

Or maybe there is a better place, like perhaps the `git tag`
documentation, or a dedicated gitsignature(7) page, where all the
information about tag and commit signatures could be. Anyway such
improvements could also be in a separate series.

> Even if you are not changing a single bit in the import,
> some of your early commits' signatures do not "validate" and may
> need to be stripped, and after that happens, wouldn't signatures of
> all later commits become unusable (i.e, you may be able to verify
> that the signature on the original commit object may still be valid,
> but because the commit has to become a child of a rewritten commit,
> in the resulting history the signature would no longer match)?

Yes, I agree it could be an optimization to consider all the
subsequent signatures invalid after one of them is invalid, but it
would require making sure that the commit history that `git
fast-import` receives is completely linear or that we properly track
commit history when it's not not linear. I think it's better to start
with a relatively simpler implementation like this one though.

Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

>> Even if you are not changing a single bit in the import,
>> some of your early commits' signatures do not "validate" and may
>> need to be stripped, and after that happens, wouldn't signatures of
>> all later commits become unusable (i.e, you may be able to verify
>> that the signature on the original commit object may still be valid,
>> but because the commit has to become a child of a rewritten commit,
>> in the resulting history the signature would no longer match)?
>
> Yes, I agree it could be an optimization to consider all the
> subsequent signatures invalid after one of them is invalid, but it
> would require making sure that the commit history that `git
> fast-import` receives is completely linear or that we properly track
> commit history when it's not not linear. I think it's better to start
> with a relatively simpler implementation like this one though.

Note that I wasn't suggesting any optimization.

I was saying that the cascading effect would mean strip-if-invalid
may have to strip all the later commits of their signatures anyway,
which makes us question the usefulness of the feature.

The other message from Elijah made it clear what the piece this
series implements fits in a bigger picture, so I actually am OK as
long as the overall use case fits what he described in that message.

Thanks.

[PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

Introduction
============

Tools like `git filter-repo` are often used to rewrite recent
history. When there are tags or commit signatures, such signatures
related to the rewritten history would become invalid though. A way to
address this issue could be to strip signatures when they have become
invalid.

The `--signed-commits=<mode>` option in `git fast-import` allows users
to decide what should be done when commits with signatures are
imported.

So let's add a new 'strip-if-invalid' <mode> to that option.

Maybe this new mode should become the default mode, but this would be
breaking backward compatibility, and perhaps this could be decided
after other new modes that might be even better default modes have
been added. So we leave that for future work.

This 'strip-if-invalid' mode should also be added to
`--signed-tags=<mode>`, but we leave that for future work too.

Changes since v1
================

Thanks Junio and Elijah for reviewing and commenting on v1.

There are no code changes in this v2, only commit message,
documentation and test changes:

* Rebased on current 'master'. This avoids the need to mark some
  strings for translation as a recent series doing that has been
  recently merged to 'master'.

* In patch 3/3, improved the commit message to better justify the new
  feature using some sentences from Elijah.

* In patch 3/3, removed tests with dual signatures. This avoids a
  conflict with a separate series from brian carlson that adds a
  "RUST" prereq that is then needed to run tests with dual signatures.

* In patch 3/3, improved documentation of the new option to say that
  validation behaves as the validation performed by `git
  verify-commit`.

CI tests
========

They have all passed, see:

https://github.com/chriscool/git/actions/runs/19390756104

Range diff vs v1
================

1:  02ce924afd = 1:  ec2afd95d6 fast-import: refactor finalize_commit_buffer()
2:  1593adc7b2 = 2:  d22b753817 commit: refactor verify_commit_buffer()
3:  f264cd25e5 ! 3:  e325533de4 fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>
    @@ Commit message
         in the repository history made by the tool between the fast-export
         and the fast-import steps.
     
    +    Note that as far as signature handling goes:
    +
    +      * Since fast-export doesn't know what changes filter-repo may make
    +    to the stream, it can't know whether the signatures will still be
    +    valid.
    +
    +      * Since filter-repo doesn't know what history canonicalizations
    +    fast-export performed (and it performs a few), it can't know whether
    +    the signatures will still be valid.
    +
    +      * Therefore, fast-import is the only process in the pipeline that
    +    can know whether a specified signature remains valid.
    +
         Having invalid signatures in a rewritten repository could be
         confusing, so users rewritting history might prefer to simply
         discard signatures that are invalid at the fast-import step.
     
    +    For example a common use case is to rewrite only "recent" history.
    +    While specifying commit ranges corresponding to "recent" commits
    +    could work, users worry about getting it wrong and want to just
    +    automatically rewrite everything, expecting older commit signatures
    +    to be untouched.
    +
         To let them do that, let's add a new 'strip-if-invalid' mode to the
         `--signed-commits=<mode>` option of `git fast-import`.
     
    @@ Commit message
         For now let's just die() if 'strip-if-invalid' is passed to these
         options where it hasn't been implemented yet.
     
    -    While at it, let's also mark for translation some error messages
    -    linked to the `--signed-commits=<mode>` and `--signed-tags=<mode>`
    -    in `git fast-export`.
    -
         [1]: https://github.com/newren/git-filter-repo
     
    +    Helped-by: Elijah Newren <newren@gmail.com>
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## Documentation/git-fast-import.adoc ##
    @@ Documentation/git-fast-import.adoc: fast-import stream! This option is enabled a
     +* `strip` will silently make the commits unsigned.
     +* `warn-strip` will make them unsigned, but will display a warning.
     +* `strip-if-invalid` will check signatures and, if they are invalid,
    -+  will strip them and display a warning.
    ++  will strip them and display a warning. The validation is performed
    ++  in the same way as linkgit:git-verify-commit[1] does it.
      
      Options for Frontends
      ~~~~~~~~~~~~~~~~~~~~~
    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
      	if (signatures.nr) {
      		switch (signed_commit_mode) {
     -		case SIGN_ABORT:
    --			die("encountered signed commit %s; use "
    --			    "--signed-commits=<mode> to handle it",
    +-			die(_("encountered signed commit %s; use "
    +-			      "--signed-commits=<mode> to handle it"),
     -			    oid_to_hex(&commit->object.oid));
     +
     +		/* Exporting modes */
      		case SIGN_WARN_VERBATIM:
    --			warning("exporting %"PRIuMAX" signature(s) for commit %s",
    -+			warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
    + 			warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
      				(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
    - 			/* fallthru */
    - 		case SIGN_VERBATIM:
     @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,
      				print_signature(item->string, item->util);
      			}
    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r
     +
     +		/* Stripping modes */
      		case SIGN_WARN_STRIP:
    --			warning("stripping signature(s) from commit %s",
    -+			warning(_("stripping signature(s) from commit %s"),
    + 			warning(_("stripping signature(s) from commit %s"),
      				oid_to_hex(&commit->object.oid));
      			/* fallthru */
      		case SIGN_STRIP:
    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
      		if (sig_offset < message_size)
      			switch (signed_tag_mode) {
     -			case SIGN_ABORT:
    --				die("encountered signed tag %s; use "
    --				    "--signed-tags=<mode> to handle it",
    +-				die(_("encountered signed tag %s; use "
    +-				      "--signed-tags=<mode> to handle it"),
     -				    oid_to_hex(&tag->object.oid));
     +
     +			/* Exporting modes */
      			case SIGN_WARN_VERBATIM:
    --				warning("exporting signed tag %s",
    -+				warning(_("exporting signed tag %s"),
    + 				warning(_("exporting signed tag %s"),
      					oid_to_hex(&tag->object.oid));
      				/* fallthru */
      			case SIGN_VERBATIM:
    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
     +
     +			/* Stripping modes */
      			case SIGN_WARN_STRIP:
    --				warning("stripping signature from tag %s",
    -+				warning(_("stripping signature from tag %s"),
    + 				warning(_("stripping signature from tag %s"),
      					oid_to_hex(&tag->object.oid));
    - 				/* fallthru */
    +@@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)
      			case SIGN_STRIP:
      				message_size = sig_offset;
      				break;
    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s
     +	test_grep "stripping invalid signature" log
     +'
     +
    -+test_expect_success GPG 'keep valid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
    -+	rm -rf new &&
    -+	git init new &&
    -+
    -+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
    -+	git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
    -+
    -+	git -C new cat-file commit refs/heads/dual-signed >actual &&
    -+	test_grep -E "^gpgsig " actual &&
    -+	test_grep -E "^gpgsig-sha256 " actual &&
    -+	test_must_be_empty log &&
    -+
    -+	IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
    -+	if test "$GIT_DEFAULT_HASH" = "sha1"
    -+	then
    -+		test $SHA1_B = $IMPORTED
    -+	else
    -+		test $SHA256_B = $IMPORTED
    -+	fi
    -+'
    -+
    -+test_expect_success GPG 'strip both invalid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
    -+	rm -rf new &&
    -+	git init new &&
    -+
    -+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >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/signed commit/forged commit/" output >modified &&
    -+
    -+	git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
    -+
    -+	git -C new cat-file commit refs/heads/dual-signed >actual &&
    -+	test_grep ! -E "^gpgsig " actual &&
    -+	test_grep ! -E "^gpgsig-sha256 " actual &&
    -+
    -+	IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
    -+	if test "$GIT_DEFAULT_HASH" = "sha1"
    -+	then
    -+		test $SHA1_B != $IMPORTED
    -+	else
    -+		test $SHA256_B != $IMPORTED
    -+	fi &&
    -+
    -+	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 &&


Christian Couder (3):
  fast-import: refactor finalize_commit_buffer()
  commit: refactor verify_commit_buffer()
  fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

 Documentation/git-fast-import.adoc | 29 ++++++++----
 builtin/fast-export.c              | 38 +++++++++++----
 builtin/fast-import.c              | 74 ++++++++++++++++++++++++++----
 commit.c                           | 17 ++++++-
 commit.h                           |  7 +++
 gpg-interface.c                    |  2 +
 gpg-interface.h                    |  1 +
 t/t9305-fast-import-signatures.sh  | 69 +++++++++++++++++++++++++++-
 8 files changed, 208 insertions(+), 29 deletions(-)

-- 
2.52.0.rc2.6.g1f299c9613

[PATCH v2 1/3] fast-import: refactor finalize_commit_buffer()

[Christian Couder]

In a following commit we are going to finalize commit buffers with or
without signatures in order to check the signatures and possibly drop
them.

To do so easily and without duplication, let's refactor the current
code that finalizes commit buffers into a new finalize_commit_buffer()
function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fast-import.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 7c194e71cb..cb0d2f635e 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2815,6 +2815,18 @@ static void import_one_signature(struct signature_data *sig_sha1,
 		die(_("parse_one_signature() returned unknown hash algo"));
 }
 
+static void finalize_commit_buffer(struct strbuf *new_data,
+				   struct signature_data *sig_sha1,
+				   struct signature_data *sig_sha256,
+				   struct strbuf *msg)
+{
+	add_gpgsig_to_commit(new_data, "gpgsig ", sig_sha1);
+	add_gpgsig_to_commit(new_data, "gpgsig-sha256 ", sig_sha256);
+
+	strbuf_addch(new_data, '\n');
+	strbuf_addbuf(new_data, msg);
+}
+
 static void parse_new_commit(const char *arg)
 {
 	static struct strbuf msg = STRBUF_INIT;
@@ -2950,11 +2962,8 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	add_gpgsig_to_commit(&new_data, "gpgsig ", &sig_sha1);
-	add_gpgsig_to_commit(&new_data, "gpgsig-sha256 ", &sig_sha256);
+	finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
-	strbuf_addch(&new_data, '\n');
-	strbuf_addbuf(&new_data, &msg);
 	free(author);
 	free(committer);
 	free(encoding);
-- 
2.52.0.rc2.6.g1f299c9613

[PATCH v2 2/3] commit: refactor verify_commit_buffer()

[Christian Couder]

In a following commit, we are going to check commit signatures, but we
won't have a commit yet, only a commit buffer, and we are going to
discard this commit buffer if the signature is invalid. So it would be
wasteful to create a commit that we might discard, just to be able to
check a commit signature.

It would be simpler instead to be able to check commit signatures
using only a commit buffer instead of a commit.

To be able to do that, let's extract some code from the
check_commit_signature() function into a new verify_commit_buffer()
function, and then let's make check_commit_signature() call
verify_commit_buffer().

Note that this doesn't fundamentally change how
check_commit_signature() works. It used to call parse_signed_commit()
which calls repo_get_commit_buffer(), parse_buffer_signed_by_header()
and repo_unuse_commit_buffer(). Now these 3 functions are called
directly by verify_commit_buffer().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 commit.c | 17 +++++++++++++++--
 commit.h |  7 +++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/commit.c b/commit.c
index 16d91b2bfc..709c9eed58 100644
--- a/commit.c
+++ b/commit.c
@@ -1315,7 +1315,8 @@ static void handle_signed_tag(const struct commit *parent, struct commit_extra_h
 	free(buf);
 }
 
-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+int verify_commit_buffer(const char *buffer, size_t size,
+			 struct signature_check *sigc)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
@@ -1323,7 +1324,8 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
 
 	sigc->result = 'N';
 
-	if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
+	if (parse_buffer_signed_by_header(buffer, size, &payload,
+					  &signature, the_hash_algo) <= 0)
 		goto out;
 
 	sigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;
@@ -1337,6 +1339,17 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
 	return ret;
 }
 
+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+{
+	unsigned long size;
+	const char *buffer = repo_get_commit_buffer(the_repository, commit, &size);
+	int ret = verify_commit_buffer(buffer, size, sigc);
+
+	repo_unuse_commit_buffer(the_repository, commit, buffer);
+
+	return ret;
+}
+
 void verify_merge_signature(struct commit *commit, int verbosity,
 			    int check_trust)
 {
diff --git a/commit.h b/commit.h
index 1d6e0c7518..5406dd2663 100644
--- a/commit.h
+++ b/commit.h
@@ -333,6 +333,13 @@ int remove_signature(struct strbuf *buf);
  */
 int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
 
+/*
+ * Same as check_commit_signature() but accepts a commit buffer and
+ * its size, instead of a `struct commit *`.
+ */
+int verify_commit_buffer(const char *buffer, size_t size,
+			 struct signature_check *sigc);
+
 /* record author-date for each commit object */
 struct author_date_slab;
 void record_author_date(struct author_date_slab *author_date,
-- 
2.52.0.rc2.6.g1f299c9613

[PATCH v2 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

Tools like `git filter-repo`[1] use `git fast-export` and
`git fast-import` to rewrite repository history. When rewriting
history using one such tool though, commit signatures might become
invalid because the commits they sign changed due to the changes
in the repository history made by the tool between the fast-export
and the fast-import steps.

Note that as far as signature handling goes:

  * Since fast-export doesn't know what changes filter-repo may make
to the stream, it can't know whether the signatures will still be
valid.

  * Since filter-repo doesn't know what history canonicalizations
fast-export performed (and it performs a few), it can't know whether
the signatures will still be valid.

  * Therefore, fast-import is the only process in the pipeline that
can know whether a specified signature remains valid.

Having invalid signatures in a rewritten repository could be
confusing, so users rewritting history might prefer to simply
discard signatures that are invalid at the fast-import step.

For example a common use case is to rewrite only "recent" history.
While specifying commit ranges corresponding to "recent" commits
could work, users worry about getting it wrong and want to just
automatically rewrite everything, expecting older commit signatures
to be untouched.

To let them do that, let's add a new 'strip-if-invalid' mode to the
`--signed-commits=<mode>` option of `git fast-import`.

It would be interesting for the `--signed-tags=<mode>` option to
have this mode too, but we leave that for a future improvement.

It might also be possible for `git fast-export` to have such a mode
in its `--signed-commits=<mode>` and `--signed-tags=<mode>`
options, but the use cases for it are much less clear, so we also
leave that for possible future improvements.

For now let's just die() if 'strip-if-invalid' is passed to these
options where it hasn't been implemented yet.

[1]: https://github.com/newren/git-filter-repo

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-fast-import.adoc | 29 +++++++++----
 builtin/fast-export.c              | 38 ++++++++++++----
 builtin/fast-import.c              | 59 ++++++++++++++++++++++---
 gpg-interface.c                    |  2 +
 gpg-interface.h                    |  1 +
 t/t9305-fast-import-signatures.sh  | 69 +++++++++++++++++++++++++++++-
 6 files changed, 174 insertions(+), 24 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index b74179a6c8..479c4081da 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,15 +66,26 @@ fast-import stream! This option is enabled automatically for
 remote-helpers that use the `import` capability, as they are
 already trusted to run their own code.
 
---signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
-	Specify how to handle signed tags.  Behaves in the same way
-	as the same option in linkgit:git-fast-export[1], except that
-	default is 'verbatim' (instead of 'abort').
-
---signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
-	Specify how to handle signed commits.  Behaves in the same way
-	as the same option in linkgit:git-fast-export[1], except that
-	default is 'verbatim' (instead of 'abort').
+`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::
+	Specify how to handle signed tags. Behaves in the same way as
+	the `--signed-commits=<mode>` below, except that the
+	`strip-if-invalid` mode is not yet supported. Like for signed
+	commits, the default mode is `verbatim`.
+
+`--signed-commits=<mode>`::
+	Specify how to handle signed commits. The following <mode>s
+	are supported:
++
+* `verbatim`, which is the default, will silently import commit
+  signatures.
+* `warn-verbatim` will import them, but will display a warning.
+* `abort` will make this program die when encountering a signed
+  commit.
+* `strip` will silently make the commits unsigned.
+* `warn-strip` will make them unsigned, but will display a warning.
+* `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.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0421360ab7..e3fc34b311 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -797,10 +797,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 	       (int)(committer_end - committer), committer);
 	if (signatures.nr) {
 		switch (signed_commit_mode) {
-		case SIGN_ABORT:
-			die(_("encountered signed commit %s; use "
-			      "--signed-commits=<mode> to handle it"),
-			    oid_to_hex(&commit->object.oid));
+
+		/* Exporting modes */
 		case SIGN_WARN_VERBATIM:
 			warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
 				(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
@@ -811,12 +809,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 				print_signature(item->string, item->util);
 			}
 			break;
+
+		/* Stripping modes */
 		case SIGN_WARN_STRIP:
 			warning(_("stripping signature(s) from commit %s"),
 				oid_to_hex(&commit->object.oid));
 			/* fallthru */
 		case SIGN_STRIP:
 			break;
+
+		/* Aborting modes */
+		case SIGN_ABORT:
+			die(_("encountered signed commit %s; use "
+			      "--signed-commits=<mode> to handle it"),
+			    oid_to_hex(&commit->object.oid));
+		case SIGN_STRIP_IF_INVALID:
+			die(_("'strip-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);
 		}
 		string_list_clear(&signatures, 0);
 	}
@@ -935,16 +946,16 @@ static void handle_tag(const char *name, struct tag *tag)
 		size_t sig_offset = parse_signed_buffer(message, message_size);
 		if (sig_offset < message_size)
 			switch (signed_tag_mode) {
-			case SIGN_ABORT:
-				die(_("encountered signed tag %s; use "
-				      "--signed-tags=<mode> to handle it"),
-				    oid_to_hex(&tag->object.oid));
+
+			/* Exporting modes */
 			case SIGN_WARN_VERBATIM:
 				warning(_("exporting signed tag %s"),
 					oid_to_hex(&tag->object.oid));
 				/* fallthru */
 			case SIGN_VERBATIM:
 				break;
+
+			/* Stripping modes */
 			case SIGN_WARN_STRIP:
 				warning(_("stripping signature from tag %s"),
 					oid_to_hex(&tag->object.oid));
@@ -952,6 +963,17 @@ static void handle_tag(const char *name, struct tag *tag)
 			case SIGN_STRIP:
 				message_size = sig_offset;
 				break;
+
+			/* Aborting modes */
+			case SIGN_ABORT:
+				die(_("encountered signed tag %s; use "
+				      "--signed-tags=<mode> to handle it"),
+				    oid_to_hex(&tag->object.oid));
+			case SIGN_STRIP_IF_INVALID:
+				die(_("'strip-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 cb0d2f635e..78052d33ed 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2772,7 +2772,7 @@ static void add_gpgsig_to_commit(struct strbuf *commit_data,
 {
 	struct string_list siglines = STRING_LIST_INIT_NODUP;
 
-	if (!sig->hash_algo)
+	if (!sig || !sig->hash_algo)
 		return;
 
 	strbuf_addstr(commit_data, header);
@@ -2827,6 +2827,45 @@ 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)
+{
+	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);
+
+		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);
+
+		finalize_commit_buffer(new_data, NULL, NULL, msg);
+	} else {
+		strbuf_swap(new_data, &tmp_buf);
+	}
+
+	signature_check_clear(&signature_check);
+	strbuf_release(&tmp_buf);
+}
+
 static void parse_new_commit(const char *arg)
 {
 	static struct strbuf msg = STRBUF_INIT;
@@ -2878,6 +2917,7 @@ static void parse_new_commit(const char *arg)
 			warning(_("importing a commit signature verbatim"));
 			/* fallthru */
 		case SIGN_VERBATIM:
+		case SIGN_STRIP_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -2962,7 +3002,11 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 
-	finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
+	if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
+	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
+		handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
+	else
+		finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
 
 	free(author);
 	free(committer);
@@ -2984,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	switch (signed_tag_mode) {
 
 	/* First, modes that don't change anything */
-	case SIGN_ABORT:
-		die(_("encountered signed tag; use "
-		      "--signed-tags=<mode> to handle it"));
 	case SIGN_WARN_VERBATIM:
 		warning(_("importing a tag signature verbatim for tag '%s'"), name);
 		/* fallthru */
@@ -3003,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 		strbuf_setlen(msg, sig_offset);
 		break;
 
-	/* Third, BUG */
+	/* Third, aborting modes */
+	case SIGN_ABORT:
+		die(_("encountered signed tag; use "
+		      "--signed-tags=<mode> to handle it"));
+	case SIGN_STRIP_IF_INVALID:
+		die(_("'strip-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 f680ed38c0..10853b517d 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1146,6 +1146,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
 		*mode = SIGN_WARN_STRIP;
 	else if (!strcmp(arg, "strip"))
 		*mode = SIGN_STRIP;
+	else if (!strcmp(arg, "strip-if-invalid"))
+		*mode = SIGN_STRIP_IF_INVALID;
 	else
 		return -1;
 	return 0;
diff --git a/gpg-interface.h b/gpg-interface.h
index ead1ed6967..789d1ffac4 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -111,6 +111,7 @@ enum sign_mode {
 	SIGN_VERBATIM,
 	SIGN_WARN_STRIP,
 	SIGN_STRIP,
+	SIGN_STRIP_IF_INVALID,
 };
 
 /*
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index c2b4271658..022dae02e4 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -79,7 +79,7 @@ test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-
 	echo B >explicit-sha256/B &&
 	git -C explicit-sha256 add B &&
 	test_tick &&
-	git -C explicit-sha256 commit -S -m "signed" B &&
+	git -C explicit-sha256 commit -S -m "signed commit" B &&
 	SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
 
 	# Create the corresponding SHA-1 commit
@@ -103,4 +103,71 @@ 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
+'
+
 test_done
-- 
2.52.0.rc2.6.g1f299c9613

Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Elijah Newren]

On Sun, Nov 16, 2025 at 8:35 PM Christian Couder
<christian.couder@gmail.com> wrote:
> There are no code changes in this v2, only commit message,
> documentation and test changes:
>
> * Rebased on current 'master'. This avoids the need to mark some
>   strings for translation as a recent series doing that has been
>   recently merged to 'master'.
>
> * In patch 3/3, improved the commit message to better justify the new
>   feature using some sentences from Elijah.
>
> * In patch 3/3, removed tests with dual signatures. This avoids a
>   conflict with a separate series from brian carlson that adds a
>   "RUST" prereq that is then needed to run tests with dual signatures.

I'm a bit surprised; from
https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you
were going to rearrange the tests to avoid the conflict, not delete
them.  Are no tests of this new functionality needed?

> * In patch 3/3, improved documentation of the new option to say that
>   validation behaves as the validation performed by `git
>   verify-commit`.

Looking over the range diff, the other changes look good.

Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Christian Couder]

On Mon, Nov 17, 2025 at 8:52 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Sun, Nov 16, 2025 at 8:35 PM Christian Couder
> <christian.couder@gmail.com> wrote:
> > There are no code changes in this v2, only commit message,
> > documentation and test changes:
> >
> > * Rebased on current 'master'. This avoids the need to mark some
> >   strings for translation as a recent series doing that has been
> >   recently merged to 'master'.
> >
> > * In patch 3/3, improved the commit message to better justify the new
> >   feature using some sentences from Elijah.
> >
> > * In patch 3/3, removed tests with dual signatures. This avoids a
> >   conflict with a separate series from brian carlson that adds a
> >   "RUST" prereq that is then needed to run tests with dual signatures.
>
> I'm a bit surprised; from
> https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you
> were going to rearrange the tests to avoid the conflict, not delete
> them.  Are no tests of this new functionality needed?

There are still 5 new tests left in patch 3/3 that are testing the new
'strip-if-invalid' functionality after I removed the 2 tests that are
related to dual signatures.

In "t/t9305-fast-import-signatures.sh", dual signatures are already
tested to work with `git fast-import --signed-commits=<mode>` by the
tests that brian's f6581e23 (repository: require Rust support for
interoperability, 2025-10-27) modifies.

f6581e23 not only adds the RUST prereq to these tests, but it also
introduces the RUST prereq itself in "t/test-lib.sh" with:

+test_lazy_prereq RUST '
+       test "$(build_option rust)" = enabled
+'

So it's much simpler to just remove the 2 new dual signature tests
that will need the RUST prereq when f6581e23 is merged. We can still
add back these 2 new tests after f6581e23 is merged if we think it's
worth it.

To avoid the conflict I could introduce the RUST prereq itself in
"t/test-lib.sh" with the same code that f6581e23 uses, but then how do
I justify it? What happens if f6581e23 is not actually merged?

It seems to me that if we really want the 2 new dual signature tests
in this series, we would have to wait until f6581e23 is merged or
discarded.

> > * In patch 3/3, improved documentation of the new option to say that
> >   validation behaves as the validation performed by `git
> >   verify-commit`.
>
> Looking over the range diff, the other changes look good.

Thanks for your review.

Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> In "t/t9305-fast-import-signatures.sh", dual signatures are already
> tested to work with `git fast-import --signed-commits=<mode>` by the
> tests that brian's f6581e23 (repository: require Rust support for
> interoperability, 2025-10-27) modifies.
> ...
> Thanks for your review.

Sounds good.

Let's see how well this round interact with others (I do not
anticipate any more fallout---knock wood), and then mark the topic
for 'next'.

Thanks.

Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>

[Elijah Newren]

On Tue, Nov 18, 2025 at 10:30 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> On Mon, Nov 17, 2025 at 8:52 PM Elijah Newren <newren@gmail.com> wrote:
> >
> > On Sun, Nov 16, 2025 at 8:35 PM Christian Couder
> > <christian.couder@gmail.com> wrote:
> > > There are no code changes in this v2, only commit message,
> > > documentation and test changes:
> > >
> > > * Rebased on current 'master'. This avoids the need to mark some
> > >   strings for translation as a recent series doing that has been
> > >   recently merged to 'master'.
> > >
> > > * In patch 3/3, improved the commit message to better justify the new
> > >   feature using some sentences from Elijah.
> > >
> > > * In patch 3/3, removed tests with dual signatures. This avoids a
> > >   conflict with a separate series from brian carlson that adds a
> > >   "RUST" prereq that is then needed to run tests with dual signatures.
> >
> > I'm a bit surprised; from
> > https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you
> > were going to rearrange the tests to avoid the conflict, not delete
> > them.  Are no tests of this new functionality needed?
>
> There are still 5 new tests left in patch 3/3 that are testing the new
> 'strip-if-invalid' functionality after I removed the 2 tests that are
> related to dual signatures.
>
> In "t/t9305-fast-import-signatures.sh", dual signatures are already
> tested to work with `git fast-import --signed-commits=<mode>` by the
> tests that brian's f6581e23 (repository: require Rust support for
> interoperability, 2025-10-27) modifies.
>
> f6581e23 not only adds the RUST prereq to these tests, but it also
> introduces the RUST prereq itself in "t/test-lib.sh" with:
>
> +test_lazy_prereq RUST '
> +       test "$(build_option rust)" = enabled
> +'
>
> So it's much simpler to just remove the 2 new dual signature tests
> that will need the RUST prereq when f6581e23 is merged. We can still
> add back these 2 new tests after f6581e23 is merged if we think it's
> worth it.
>
> To avoid the conflict I could introduce the RUST prereq itself in
> "t/test-lib.sh" with the same code that f6581e23 uses, but then how do
> I justify it? What happens if f6581e23 is not actually merged?
>
> It seems to me that if we really want the 2 new dual signature tests
> in this series, we would have to wait until f6581e23 is merged or
> discarded.

Oh, right, there were other tests.  Sorry about that, I should have
double checked the patches instead of only looking at the range-diff.

> > > * In patch 3/3, improved documentation of the new option to say that
> > >   validation behaves as the validation performed by `git
> > >   verify-commit`.
> >
> > Looking over the range diff, the other changes look good.
>
> Thanks for your review.

Yeah, I think the series is good to advance.