[PATCH v2 0/2] fast-import: start controlling how commit signatures are handled

[PATCH v2 0/2] fast-import: start controlling how commit signatures are handled

[Christian Couder]

Tools like `git-filter-repo` should be able to control how commit
signatures are handled when regenerating repository content after it
has been filtered (see
https://github.com/newren/git-filter-repo/issues/139). For this
purpose, they need a way for `git fast-import` to control how commit
signatures are handled.

This small patch series starts to implement such a way by adding a new
`--signed-commits=<mode>` option to `git fast-import`.

For now this new option behaves in a very similar way as the option
with the same name that already exists in `git fast-export`.
Especially it supports exactly the same <mode>s and the same aliases
for these modes. For example "ignore" is a synonym for "verbatim".

In the future I want to implement new <mode>s like "strip-if-invalid",
"re-sign", "re-sign-if-invalid" that might be a bit more complex for
this option. But for now I prefer to start with the simple modes to
validate the general design of the new option.

In the future I also plan to add a similar `--signed-tags=<mode>` so
that the import of tags can also be controlled. But I prefer to
validate the general design of a single new option first.

In particular, I am interested in feedback about the following:

  - Should we keep "ignore" as a synonym for "verbatim" and "warn" as
    a synonym for "warn-verbatim"? My opinion is that they might be
    confusing, so we might want to remove them for `git fast-import`
    even if we keep them for `git fast-export`. The parsing code might
    be a bit more complex if we do that though, so for now I have kept
    the synonyms.

  - Are we still fine with most <mode>s having a "warn-*" variant
    (like the "warn-strip" variant of "strip" for example)? Or should
    we have a separate `--verbose` or maybe `--signed-commits-verbose`
    option dedicated to switching warnings on/off? I think it's good
    to decide about this before the number of <mode>s increases a lot
    with new <mode>s like "strip-if-invalid", "re-sign",
    "re-sign-if-invalid" and possibly others.

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

Thanks to Junio and Patrick for reviewing V1.

In patch 1/2:

  - the commit message now says that a single option will be added in
    the following commit

  - instead of returning success in the `!parse_sign_mode(arg, val)`
    case, we return error in the `parse_sign_mode(arg, val)` case,

  - a '.' has been added at the end of a code comment.

In patch 2/2:

  - the import_signature() function has been removed and we just call
    parse_one_signature() in all cases except `SIGN_ABORT`, before
    handling the signature according to `signed_commit_mode`,

  - we now mark for translation all new strings that could be
    displayed to users,

  - usagef() is used instead of die() when the mode is unknown,

  - the warning for `warn-verbatim` has been improved,

  - the tests have been improved by using more idiomatic and
    simplified commands

CI tests:
=========

They have all passed. See:

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

range-diff vs V1:
=================

1:  7c8216a701 ! 1:  87149ae92d gpg-interface: refactor 'enum sign_mode' parsing
    @@ Commit message
         only command with '--signed-tags=<mode>' or '--signed-commits=<mode>'
         options.
     
    -    In a following commit, we are going to add such options to `git
    +    In a following commit, we are going to add a similar option to `git
         fast-import`, which will be simpler, easier and cleaner if we can reuse
         the 'enum sign_mode' defintion and parsing code.
     
    @@ builtin/fast-export.c: static struct hashmap anonymized_seeds;
     -	else if (!strcmp(arg, "strip"))
     -		*val = SIGN_STRIP;
     -	else
    --		return error("Unknown %s mode: %s", opt->long_name, arg);
    --	return 0;
     +
    -+	if (!parse_sign_mode(arg, val))
    -+		return 0;
    ++	if (parse_sign_mode(arg, val))
    + 		return error("Unknown %s mode: %s", opt->long_name, arg);
     +
    -+	return error("Unknown %s mode: %s", opt->long_name, arg);
    + 	return 0;
      }
      
    - static int parse_opt_tag_of_filtered_mode(const struct option *opt,
     
      ## gpg-interface.c ##
     @@ gpg-interface.c: static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
    @@ gpg-interface.h: int check_signature(struct signature_check *sigc,
      void print_signature_buffer(const struct signature_check *sigc,
      			    unsigned flags);
      
    -+/* Modes for --signed-tags=<mode> and --signed-commits=<mode> options */
    ++/* Modes for --signed-tags=<mode> and --signed-commits=<mode> options. */
     +enum sign_mode {
     +	SIGN_ABORT,
     +	SIGN_WARN_VERBATIM,
2:  0294f05ae6 ! 2:  e34f015aea fast-import: add '--signed-commits=<mode>' option
    @@ builtin/fast-import.c: static int global_argc;
      /* Memory pools */
      static struct mem_pool fi_mem_pool = {
      	.block_alloc = 2*1024*1024 - sizeof(struct mp_block),
    -@@ builtin/fast-import.c: static void store_signature(struct signature_data *stored_sig,
    - 	}
    - }
    - 
    -+/* Process signatures (up to 2: one "sha1" and one "sha256") */
    -+static void import_signature(struct signature_data *sig_sha1,
    -+			     struct signature_data *sig_sha256,
    -+			     const char *v)
    -+{
    -+	struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    -+
    -+	parse_one_signature(&sig, v);
    -+
    -+	if (!strcmp(sig.hash_algo, "sha1"))
    -+		store_signature(sig_sha1, &sig, "SHA-1");
    -+	else if (!strcmp(sig.hash_algo, "sha256"))
    -+		store_signature(sig_sha256, &sig, "SHA-256");
    -+	else
    -+		BUG("parse_one_signature() returned unknown hash algo");
    -+}
    -+
    - static void parse_new_commit(const char *arg)
    - {
    - 	static struct strbuf msg = STRBUF_INIT;
     @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      	if (!committer)
      		die("Expected committer but didn't get one");
      
     -	/* Process signatures (up to 2: one "sha1" and one "sha256") */
      	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
    --		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    --
    + 		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    + 
     -		parse_one_signature(&sig, v);
    --
    ++		if (signed_commit_mode == SIGN_ABORT)
    ++			die(_("encountered signed commit; use "
    ++			      "--signed-commits=<mode> to handle it"));
    + 
     -		if (!strcmp(sig.hash_algo, "sha1"))
     -			store_signature(&sig_sha1, &sig, "SHA-1");
     -		else if (!strcmp(sig.hash_algo, "sha256"))
     -			store_signature(&sig_sha256, &sig, "SHA-256");
     -		else
     -			BUG("parse_one_signature() returned unknown hash algo");
    --
    -+		struct strbuf data = STRBUF_INIT;
    ++		parse_one_signature(&sig, v);
    + 
     +		switch (signed_commit_mode) {
     +		case SIGN_ABORT:
    -+			die("encountered signed commit; use "
    -+			    "--signed-commits=<mode> to handle it");
    ++			BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
    ++			break;
     +		case SIGN_WARN_VERBATIM:
    -+			warning("importing a commit signature");
    ++			warning(_("importing a commit signature verbatim"));
     +			/* fallthru */
     +		case SIGN_VERBATIM:
    -+			import_signature(&sig_sha1, &sig_sha256, v);
    ++			if (!strcmp(sig.hash_algo, "sha1"))
    ++				store_signature(&sig_sha1, &sig, "SHA-1");
    ++			else if (!strcmp(sig.hash_algo, "sha256"))
    ++				store_signature(&sig_sha256, &sig, "SHA-256");
    ++			else
    ++				die(_("parse_one_signature() returned unknown hash algo"));
     +			break;
     +		case SIGN_WARN_STRIP:
    -+			warning("stripping a commit signature");
    ++			warning(_("stripping a commit signature"));
     +			/* fallthru */
     +		case SIGN_STRIP:
    -+			/* Read signature data and discard it */
    -+			read_next_command();
    -+			parse_data(&data, 0, NULL);
    -+			strbuf_release(&data);
    ++			/* Just discard signature data */
    ++			strbuf_release(&sig.data);
    ++			free(sig.hash_algo);
     +			break;
     +		}
      		read_next_command();
    @@ builtin/fast-import.c: static int parse_one_option(const char *option)
      		option_export_pack_edges(option);
     +	} else if (skip_prefix(option, "signed-commits=", &option)) {
     +		if (parse_sign_mode(option, &signed_commit_mode))
    -+			die("unknown --signed-commits mode '%s'", option);
    ++			usagef(_("unknown --signed-commits mode '%s'"), option);
      	} else if (!strcmp(option, "quiet")) {
      		show_stats = 0;
      		quiet = 1;
    @@ t/t9305-fast-import-signatures.sh (new)
     +test_description='git fast-import --signed-commits=<mode>'
     +
     +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
    -+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
     +
     +. ./test-lib.sh
     +. "$TEST_DIRECTORY/lib-gpg.sh"
     +
     +test_expect_success 'set up unsigned initial commit and import repo' '
     +	test_commit first &&
    -+	mkdir new &&
    -+	git --git-dir=new/.git init
    ++	git init new
     +'
     +
     +test_expect_success GPG 'set up OpenPGP signed commit' '
    @@ t/t9305-fast-import-signatures.sh (new)
     +	SHA1_B=$(git -C explicit-sha256 rev-parse --output-object-format=sha1 dual-signed) &&
     +
     +	# Check that the resulting SHA-1 commit has both signatures
    -+	echo $SHA1_B | git -C explicit-sha256 cat-file --batch >out &&
    ++	git -C explicit-sha256 cat-file -p $SHA1_B >out &&
     +	test_grep -E "^gpgsig " out &&
     +	test_grep -E "^gpgsig-sha256 " out
     +'


Christian Couder (2):
  gpg-interface: refactor 'enum sign_mode' parsing
  fast-import: add '--signed-commits=<mode>' option

 Documentation/git-fast-import.adoc |   5 ++
 builtin/fast-export.c              |  19 ++----
 builtin/fast-import.c              |  41 ++++++++---
 gpg-interface.c                    |  17 +++++
 gpg-interface.h                    |  15 ++++
 t/meson.build                      |   1 +
 t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
 7 files changed, 182 insertions(+), 22 deletions(-)
 create mode 100755 t/t9305-fast-import-signatures.sh

-- 
2.51.0.195.gf8f8f06677

[PATCH v2 1/2] gpg-interface: refactor 'enum sign_mode' parsing

[Christian Couder]

The definition of 'enum sign_mode' as well as its parsing code are in
"builtin/fast-export.c". This was fine because `git fast-export` was the
only command with '--signed-tags=<mode>' or '--signed-commits=<mode>'
options.

In a following commit, we are going to add a similar option to `git
fast-import`, which will be simpler, easier and cleaner if we can reuse
the 'enum sign_mode' defintion and parsing code.

So let's move that definition and parsing code from
"builtin/fast-export.c" to "gpg-interface.{c,h}".

While at it, let's fix a small indentation issue with the arguments of
parse_opt_sign_mode().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fast-export.c | 19 +++++--------------
 gpg-interface.c       | 17 +++++++++++++++++
 gpg-interface.h       | 15 +++++++++++++++
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c06ee0b213..dc2486f9a8 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -37,8 +37,6 @@ static const char *const fast_export_usage[] = {
 	NULL
 };
 
-enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
-
 static int progress;
 static enum sign_mode signed_tag_mode = SIGN_ABORT;
 static enum sign_mode signed_commit_mode = SIGN_STRIP;
@@ -59,23 +57,16 @@ static struct hashmap anonymized_seeds;
 static struct revision_sources revision_sources;
 
 static int parse_opt_sign_mode(const struct option *opt,
-				     const char *arg, int unset)
+			       const char *arg, int unset)
 {
 	enum sign_mode *val = opt->value;
+
 	if (unset)
 		return 0;
-	else if (!strcmp(arg, "abort"))
-		*val = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
-		*val = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
-		*val = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
-		*val = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
-		*val = SIGN_STRIP;
-	else
+
+	if (parse_sign_mode(arg, val))
 		return error("Unknown %s mode: %s", opt->long_name, arg);
+
 	return 0;
 }
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 06e7fb5060..2f4f0e32cb 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1125,3 +1125,20 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	FREE_AND_NULL(ssh_signing_key_file);
 	return ret;
 }
+
+int parse_sign_mode(const char *arg, enum sign_mode *mode)
+{
+	if (!strcmp(arg, "abort"))
+		*mode = SIGN_ABORT;
+	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+		*mode = SIGN_VERBATIM;
+	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+		*mode = SIGN_WARN_VERBATIM;
+	else if (!strcmp(arg, "warn-strip"))
+		*mode = SIGN_WARN_STRIP;
+	else if (!strcmp(arg, "strip"))
+		*mode = SIGN_STRIP;
+	else
+		return -1;
+	return 0;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
index 60ddf8bbfa..50487aa148 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -104,4 +104,19 @@ int check_signature(struct signature_check *sigc,
 void print_signature_buffer(const struct signature_check *sigc,
 			    unsigned flags);
 
+/* Modes for --signed-tags=<mode> and --signed-commits=<mode> options. */
+enum sign_mode {
+	SIGN_ABORT,
+	SIGN_WARN_VERBATIM,
+	SIGN_VERBATIM,
+	SIGN_WARN_STRIP,
+	SIGN_STRIP,
+};
+
+/*
+ * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
+ * otherwise.
+ */
+int parse_sign_mode(const char *arg, enum sign_mode *mode);
+
 #endif
-- 
2.51.0.195.gf8f8f06677

[PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Christian Couder]

A '--signed-commits=<mode>' option is already available when using
`git fast-export` to decide what should be done at export time about
commit signatures. At import time though, there is no option, or
other way, in `git fast-import` to decide about commit signatures.

To remediate that, let's add a '--signed-commits=<mode>' option to
`git fast-import` too.

For now the supported <mode>s are the same as those supported by
`git fast-export`.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-fast-import.adoc |   5 ++
 builtin/fast-import.c              |  41 ++++++++---
 t/meson.build                      |   1 +
 t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
 4 files changed, 145 insertions(+), 8 deletions(-)
 create mode 100755 t/t9305-fast-import-signatures.sh

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 3144ffcdb6..90f242d058 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,6 +66,11 @@ OPTIONS
 	remote-helpers that use the `import` capability, as they are
 	already trusted to run their own code.
 
+--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').
+
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2c35f9345d..890f05de4d 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -188,6 +188,8 @@ static int global_argc;
 static const char **global_argv;
 static const char *global_prefix;
 
+static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
 	.block_alloc = 2*1024*1024 - sizeof(struct mp_block),
@@ -2817,19 +2819,39 @@ static void parse_new_commit(const char *arg)
 	if (!committer)
 		die("Expected committer but didn't get one");
 
-	/* Process signatures (up to 2: one "sha1" and one "sha256") */
 	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
 		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
 
-		parse_one_signature(&sig, v);
+		if (signed_commit_mode == SIGN_ABORT)
+			die(_("encountered signed commit; use "
+			      "--signed-commits=<mode> to handle it"));
 
-		if (!strcmp(sig.hash_algo, "sha1"))
-			store_signature(&sig_sha1, &sig, "SHA-1");
-		else if (!strcmp(sig.hash_algo, "sha256"))
-			store_signature(&sig_sha256, &sig, "SHA-256");
-		else
-			BUG("parse_one_signature() returned unknown hash algo");
+		parse_one_signature(&sig, v);
 
+		switch (signed_commit_mode) {
+		case SIGN_ABORT:
+			BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
+			break;
+		case SIGN_WARN_VERBATIM:
+			warning(_("importing a commit signature verbatim"));
+			/* fallthru */
+		case SIGN_VERBATIM:
+			if (!strcmp(sig.hash_algo, "sha1"))
+				store_signature(&sig_sha1, &sig, "SHA-1");
+			else if (!strcmp(sig.hash_algo, "sha256"))
+				store_signature(&sig_sha256, &sig, "SHA-256");
+			else
+				die(_("parse_one_signature() returned unknown hash algo"));
+			break;
+		case SIGN_WARN_STRIP:
+			warning(_("stripping a commit signature"));
+			/* fallthru */
+		case SIGN_STRIP:
+			/* Just discard signature data */
+			strbuf_release(&sig.data);
+			free(sig.hash_algo);
+			break;
+		}
 		read_next_command();
 	}
 
@@ -3501,6 +3523,9 @@ static int parse_one_option(const char *option)
 		option_active_branches(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))
+			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
 		quiet = 1;
diff --git a/t/meson.build b/t/meson.build
index 82af229be3..08ad6938e2 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1032,6 +1032,7 @@ integration_tests = [
   't9302-fast-import-unpack-limit.sh',
   't9303-fast-import-compression.sh',
   't9304-fast-import-marks.sh',
+  't9305-fast-import-signatures.sh',
   't9350-fast-export.sh',
   't9351-fast-export-anonymize.sh',
   't9400-git-cvsserver-server.sh',
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
new file mode 100755
index 0000000000..c2b4271658
--- /dev/null
+++ b/t/t9305-fast-import-signatures.sh
@@ -0,0 +1,106 @@
+#!/bin/sh
+
+test_description='git fast-import --signed-commits=<mode>'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success 'set up unsigned initial commit and import repo' '
+	test_commit first &&
+	git init new
+'
+
+test_expect_success GPG 'set up OpenPGP signed commit' '
+	git checkout -b openpgp-signing main &&
+	echo "Content for OpenPGP signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "OpenPGP signed commit" &&
+	OPENPGP_SIGNING=$(git rev-parse --verify openpgp-signing)
+'
+
+test_expect_success GPG 'import OpenPGP signature with --signed-commits=verbatim' '
+	git fast-export --signed-commits=verbatim openpgp-signing >output &&
+	git -C new fast-import --quiet --signed-commits=verbatim <output >log 2>&1 &&
+	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+	test $OPENPGP_SIGNING = $IMPORTED &&
+	test_must_be_empty log
+'
+
+test_expect_success GPGSM 'set up X.509 signed commit' '
+	git checkout -b x509-signing main &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo "Content for X.509 signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "X.509 signed commit" &&
+	X509_SIGNING=$(git rev-parse HEAD)
+'
+
+test_expect_success GPGSM 'import X.509 signature fails with --signed-commits=abort' '
+	git fast-export --signed-commits=verbatim x509-signing >output &&
+	test_must_fail git -C new fast-import --quiet --signed-commits=abort <output
+'
+
+test_expect_success GPGSM 'import X.509 signature with --signed-commits=warn-verbatim' '
+	git -C new fast-import --quiet --signed-commits=warn-verbatim <output >log 2>&1 &&
+	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+	test $X509_SIGNING = $IMPORTED &&
+	test_grep "importing a commit signature" log
+'
+
+test_expect_success GPGSSH 'set up SSH signed commit' '
+	git checkout -b ssh-signing main &&
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "Content for SSH signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "SSH signed commit" &&
+	SSH_SIGNING=$(git rev-parse HEAD)
+'
+
+test_expect_success GPGSSH 'strip SSH signature with --signed-commits=strip' '
+	git fast-export --signed-commits=verbatim ssh-signing >output &&
+	git -C new fast-import --quiet --signed-commits=strip <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" actual &&
+	test_must_be_empty log
+'
+
+test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-1 and SHA-256 formats' '
+	# Create a signed SHA-256 commit
+	git init --object-format=sha256 explicit-sha256 &&
+	git -C explicit-sha256 config extensions.compatObjectFormat sha1 &&
+	git -C explicit-sha256 checkout -b dual-signed &&
+	test_commit -C explicit-sha256 A &&
+	echo B >explicit-sha256/B &&
+	git -C explicit-sha256 add B &&
+	test_tick &&
+	git -C explicit-sha256 commit -S -m "signed" B &&
+	SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
+
+	# Create the corresponding SHA-1 commit
+	SHA1_B=$(git -C explicit-sha256 rev-parse --output-object-format=sha1 dual-signed) &&
+
+	# Check that the resulting SHA-1 commit has both signatures
+	git -C explicit-sha256 cat-file -p $SHA1_B >out &&
+	test_grep -E "^gpgsig " out &&
+	test_grep -E "^gpgsig-sha256 " out
+'
+
+test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=warn-strip' '
+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
+	test_grep -E "^gpgsig sha1 openpgp" output &&
+	test_grep -E "^gpgsig sha256 openpgp" output &&
+	git -C new fast-import --quiet --signed-commits=warn-strip <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_grep "stripping a commit signature" log >out &&
+	test_line_count = 2 out
+'
+
+test_done
-- 
2.51.0.195.gf8f8f06677

Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Christian Couder]

Sorry I forgot to use the
--in-reply-to='20250910080839.2142651-1-christian.couder@gmail.com'
option when sending this series. It is related to:

https://lore.kernel.org/git/20250910080839.2142651-1-christian.couder@gmail.com/

On Fri, Sep 12, 2025 at 2:40 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> A '--signed-commits=<mode>' option is already available when using
> `git fast-export` to decide what should be done at export time about
> commit signatures. At import time though, there is no option, or
> other way, in `git fast-import` to decide about commit signatures.
>
> To remediate that, let's add a '--signed-commits=<mode>' option to
> `git fast-import` too.
>
> For now the supported <mode>s are the same as those supported by
> `git fast-export`.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/git-fast-import.adoc |   5 ++
>  builtin/fast-import.c              |  41 ++++++++---
>  t/meson.build                      |   1 +
>  t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
>  4 files changed, 145 insertions(+), 8 deletions(-)
>  create mode 100755 t/t9305-fast-import-signatures.sh
>
> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 3144ffcdb6..90f242d058 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> @@ -66,6 +66,11 @@ OPTIONS
>         remote-helpers that use the `import` capability, as they are
>         already trusted to run their own code.
>
> +--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').
> +
>  Options for Frontends
>  ~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 2c35f9345d..890f05de4d 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -188,6 +188,8 @@ static int global_argc;
>  static const char **global_argv;
>  static const char *global_prefix;
>
> +static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
> +
>  /* Memory pools */
>  static struct mem_pool fi_mem_pool = {
>         .block_alloc = 2*1024*1024 - sizeof(struct mp_block),
> @@ -2817,19 +2819,39 @@ static void parse_new_commit(const char *arg)
>         if (!committer)
>                 die("Expected committer but didn't get one");
>
> -       /* Process signatures (up to 2: one "sha1" and one "sha256") */
>         while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
>                 struct signature_data sig = { NULL, NULL, STRBUF_INIT };
>
> -               parse_one_signature(&sig, v);
> +               if (signed_commit_mode == SIGN_ABORT)
> +                       die(_("encountered signed commit; use "
> +                             "--signed-commits=<mode> to handle it"));
>
> -               if (!strcmp(sig.hash_algo, "sha1"))
> -                       store_signature(&sig_sha1, &sig, "SHA-1");
> -               else if (!strcmp(sig.hash_algo, "sha256"))
> -                       store_signature(&sig_sha256, &sig, "SHA-256");
> -               else
> -                       BUG("parse_one_signature() returned unknown hash algo");
> +               parse_one_signature(&sig, v);
>
> +               switch (signed_commit_mode) {
> +               case SIGN_ABORT:
> +                       BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> +                       break;
> +               case SIGN_WARN_VERBATIM:
> +                       warning(_("importing a commit signature verbatim"));
> +                       /* fallthru */
> +               case SIGN_VERBATIM:
> +                       if (!strcmp(sig.hash_algo, "sha1"))
> +                               store_signature(&sig_sha1, &sig, "SHA-1");
> +                       else if (!strcmp(sig.hash_algo, "sha256"))
> +                               store_signature(&sig_sha256, &sig, "SHA-256");
> +                       else
> +                               die(_("parse_one_signature() returned unknown hash algo"));
> +                       break;
> +               case SIGN_WARN_STRIP:
> +                       warning(_("stripping a commit signature"));
> +                       /* fallthru */
> +               case SIGN_STRIP:
> +                       /* Just discard signature data */
> +                       strbuf_release(&sig.data);
> +                       free(sig.hash_algo);
> +                       break;
> +               }
>                 read_next_command();
>         }
>
> @@ -3501,6 +3523,9 @@ static int parse_one_option(const char *option)
>                 option_active_branches(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))
> +                       usagef(_("unknown --signed-commits mode '%s'"), option);
>         } else if (!strcmp(option, "quiet")) {
>                 show_stats = 0;
>                 quiet = 1;
> diff --git a/t/meson.build b/t/meson.build
> index 82af229be3..08ad6938e2 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -1032,6 +1032,7 @@ integration_tests = [
>    't9302-fast-import-unpack-limit.sh',
>    't9303-fast-import-compression.sh',
>    't9304-fast-import-marks.sh',
> +  't9305-fast-import-signatures.sh',
>    't9350-fast-export.sh',
>    't9351-fast-export-anonymize.sh',
>    't9400-git-cvsserver-server.sh',
> diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
> new file mode 100755
> index 0000000000..c2b4271658
> --- /dev/null
> +++ b/t/t9305-fast-import-signatures.sh
> @@ -0,0 +1,106 @@
> +#!/bin/sh
> +
> +test_description='git fast-import --signed-commits=<mode>'
> +
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +
> +. ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-gpg.sh"
> +
> +test_expect_success 'set up unsigned initial commit and import repo' '
> +       test_commit first &&
> +       git init new
> +'
> +
> +test_expect_success GPG 'set up OpenPGP signed commit' '
> +       git checkout -b openpgp-signing main &&
> +       echo "Content for OpenPGP signing." >file-sign &&
> +       git add file-sign &&
> +       git commit -S -m "OpenPGP signed commit" &&
> +       OPENPGP_SIGNING=$(git rev-parse --verify openpgp-signing)
> +'
> +
> +test_expect_success GPG 'import OpenPGP signature with --signed-commits=verbatim' '
> +       git fast-export --signed-commits=verbatim openpgp-signing >output &&
> +       git -C new fast-import --quiet --signed-commits=verbatim <output >log 2>&1 &&
> +       IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
> +       test $OPENPGP_SIGNING = $IMPORTED &&
> +       test_must_be_empty log
> +'
> +
> +test_expect_success GPGSM 'set up X.509 signed commit' '
> +       git checkout -b x509-signing main &&
> +       test_config gpg.format x509 &&
> +       test_config user.signingkey $GIT_COMMITTER_EMAIL &&
> +       echo "Content for X.509 signing." >file-sign &&
> +       git add file-sign &&
> +       git commit -S -m "X.509 signed commit" &&
> +       X509_SIGNING=$(git rev-parse HEAD)
> +'
> +
> +test_expect_success GPGSM 'import X.509 signature fails with --signed-commits=abort' '
> +       git fast-export --signed-commits=verbatim x509-signing >output &&
> +       test_must_fail git -C new fast-import --quiet --signed-commits=abort <output
> +'
> +
> +test_expect_success GPGSM 'import X.509 signature with --signed-commits=warn-verbatim' '
> +       git -C new fast-import --quiet --signed-commits=warn-verbatim <output >log 2>&1 &&
> +       IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
> +       test $X509_SIGNING = $IMPORTED &&
> +       test_grep "importing a commit signature" log
> +'
> +
> +test_expect_success GPGSSH 'set up SSH signed commit' '
> +       git checkout -b ssh-signing main &&
> +       test_config gpg.format ssh &&
> +       test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
> +       echo "Content for SSH signing." >file-sign &&
> +       git add file-sign &&
> +       git commit -S -m "SSH signed commit" &&
> +       SSH_SIGNING=$(git rev-parse HEAD)
> +'
> +
> +test_expect_success GPGSSH 'strip SSH signature with --signed-commits=strip' '
> +       git fast-export --signed-commits=verbatim ssh-signing >output &&
> +       git -C new fast-import --quiet --signed-commits=strip <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" actual &&
> +       test_must_be_empty log
> +'
> +
> +test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-1 and SHA-256 formats' '
> +       # Create a signed SHA-256 commit
> +       git init --object-format=sha256 explicit-sha256 &&
> +       git -C explicit-sha256 config extensions.compatObjectFormat sha1 &&
> +       git -C explicit-sha256 checkout -b dual-signed &&
> +       test_commit -C explicit-sha256 A &&
> +       echo B >explicit-sha256/B &&
> +       git -C explicit-sha256 add B &&
> +       test_tick &&
> +       git -C explicit-sha256 commit -S -m "signed" B &&
> +       SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
> +
> +       # Create the corresponding SHA-1 commit
> +       SHA1_B=$(git -C explicit-sha256 rev-parse --output-object-format=sha1 dual-signed) &&
> +
> +       # Check that the resulting SHA-1 commit has both signatures
> +       git -C explicit-sha256 cat-file -p $SHA1_B >out &&
> +       test_grep -E "^gpgsig " out &&
> +       test_grep -E "^gpgsig-sha256 " out
> +'
> +
> +test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=warn-strip' '
> +       git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
> +       test_grep -E "^gpgsig sha1 openpgp" output &&
> +       test_grep -E "^gpgsig sha256 openpgp" output &&
> +       git -C new fast-import --quiet --signed-commits=warn-strip <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_grep "stripping a commit signature" log >out &&
> +       test_line_count = 2 out
> +'
> +
> +test_done
> --
> 2.51.0.195.gf8f8f06677
>

Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Patrick Steinhardt]

On Fri, Sep 12, 2025 at 02:40:42PM +0200, Christian Couder wrote:
> diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
> index 3144ffcdb6..90f242d058 100644
> --- a/Documentation/git-fast-import.adoc
> +++ b/Documentation/git-fast-import.adoc
> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 2c35f9345d..890f05de4d 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -2817,19 +2819,39 @@ static void parse_new_commit(const char *arg)
>  	if (!committer)
>  		die("Expected committer but didn't get one");
>  
> -	/* Process signatures (up to 2: one "sha1" and one "sha256") */
>  	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
>  		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
>  
> -		parse_one_signature(&sig, v);
> +		if (signed_commit_mode == SIGN_ABORT)
> +			die(_("encountered signed commit; use "
> +			      "--signed-commits=<mode> to handle it"));
>  
> -		if (!strcmp(sig.hash_algo, "sha1"))
> -			store_signature(&sig_sha1, &sig, "SHA-1");
> -		else if (!strcmp(sig.hash_algo, "sha256"))
> -			store_signature(&sig_sha256, &sig, "SHA-256");
> -		else
> -			BUG("parse_one_signature() returned unknown hash algo");
> +		parse_one_signature(&sig, v);
>  
> +		switch (signed_commit_mode) {
> +		case SIGN_ABORT:
> +			BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> +			break;

Let's be defensive and convert this into a `default:` case so that any
unhandled value will cause a BUG.

> +		case SIGN_WARN_VERBATIM:
> +			warning(_("importing a commit signature verbatim"));
> +			/* fallthru */
> +		case SIGN_VERBATIM:
> +			if (!strcmp(sig.hash_algo, "sha1"))
> +				store_signature(&sig_sha1, &sig, "SHA-1");
> +			else if (!strcmp(sig.hash_algo, "sha256"))
> +				store_signature(&sig_sha256, &sig, "SHA-256");
> +			else
> +				die(_("parse_one_signature() returned unknown hash algo"));
> +			break;
> +		case SIGN_WARN_STRIP:
> +			warning(_("stripping a commit signature"));
> +			/* fallthru */
> +		case SIGN_STRIP:
> +			/* Just discard signature data */
> +			strbuf_release(&sig.data);
> +			free(sig.hash_algo);
> +			break;
> +		}
>  		read_next_command();
>  	}
>  

Other than that the patch looks good to me, thanks!

Patrick

Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Christian Couder]

On Mon, Sep 15, 2025 at 8:27 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Fri, Sep 12, 2025 at 02:40:42PM +0200, Christian Couder wrote:

> >
> > -     /* Process signatures (up to 2: one "sha1" and one "sha256") */
> >       while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
> >               struct signature_data sig = { NULL, NULL, STRBUF_INIT };
> >
> > -             parse_one_signature(&sig, v);
> > +             if (signed_commit_mode == SIGN_ABORT)
> > +                     die(_("encountered signed commit; use "
> > +                           "--signed-commits=<mode> to handle it"));
> >
> > -             if (!strcmp(sig.hash_algo, "sha1"))
> > -                     store_signature(&sig_sha1, &sig, "SHA-1");
> > -             else if (!strcmp(sig.hash_algo, "sha256"))
> > -                     store_signature(&sig_sha256, &sig, "SHA-256");
> > -             else
> > -                     BUG("parse_one_signature() returned unknown hash algo");
> > +             parse_one_signature(&sig, v);
> >
> > +             switch (signed_commit_mode) {
> > +             case SIGN_ABORT:
> > +                     BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> > +                     break;
>
> Let's be defensive and convert this into a `default:` case so that any
> unhandled value will cause a BUG.

Ok, maybe something like BUG("invalid signed_commit_mode value %d",
signed_commit_mode) then?

Note that if we later develop new modes like "re-sign" or
"strip-if-invalid", and users tries one such mode with an old version
of Git, that should already be handled by the following code in
parse_one_option():

    } else if (skip_prefix(option, "signed-commits=", &option)) {
        if (parse_sign_mode(option, &signed_commit_mode))
            usagef(_("unknown --signed-commits mode '%s'"), option);
    } ...

Thanks,
Christian.

Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Patrick Steinhardt]

On Mon, Sep 15, 2025 at 12:17:33PM +0200, Christian Couder wrote:
> On Mon, Sep 15, 2025 at 8:27 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Fri, Sep 12, 2025 at 02:40:42PM +0200, Christian Couder wrote:
> 
> > >
> > > -     /* Process signatures (up to 2: one "sha1" and one "sha256") */
> > >       while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
> > >               struct signature_data sig = { NULL, NULL, STRBUF_INIT };
> > >
> > > -             parse_one_signature(&sig, v);
> > > +             if (signed_commit_mode == SIGN_ABORT)
> > > +                     die(_("encountered signed commit; use "
> > > +                           "--signed-commits=<mode> to handle it"));
> > >
> > > -             if (!strcmp(sig.hash_algo, "sha1"))
> > > -                     store_signature(&sig_sha1, &sig, "SHA-1");
> > > -             else if (!strcmp(sig.hash_algo, "sha256"))
> > > -                     store_signature(&sig_sha256, &sig, "SHA-256");
> > > -             else
> > > -                     BUG("parse_one_signature() returned unknown hash algo");
> > > +             parse_one_signature(&sig, v);
> > >
> > > +             switch (signed_commit_mode) {
> > > +             case SIGN_ABORT:
> > > +                     BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> > > +                     break;
> >
> > Let's be defensive and convert this into a `default:` case so that any
> > unhandled value will cause a BUG.
> 
> Ok, maybe something like BUG("invalid signed_commit_mode value %d",
> signed_commit_mode) then?

Yeah, that should do the job.

Thanks!

Patrick

[PATCH v3 0/2] fast-import: start controlling how commit signatures are handled

[Christian Couder]

Tools like `git-filter-repo` should be able to control how commit
signatures are handled when regenerating repository content after it
has been filtered (see
https://github.com/newren/git-filter-repo/issues/139). For this
purpose, they need a way for `git fast-import` to control how commit
signatures are handled.

This small patch series starts to implement such a way by adding a new
`--signed-commits=<mode>` option to `git fast-import`.

For now this new option behaves in a very similar way as the option
with the same name that already exists in `git fast-export`.
Especially it supports exactly the same <mode>s and the same aliases
for these modes. For example "ignore" is a synonym for "verbatim".

In the future I want to implement new <mode>s like "strip-if-invalid",
"re-sign", "re-sign-if-invalid" that might be a bit more complex for
this option. But for now I prefer to start with the simple modes to
validate the general design of the new option.

In the future I also plan to add a similar `--signed-tags=<mode>` so
that the import of tags can also be controlled. But I prefer to
validate the general design of a single new option first.

In particular, I am interested in feedback about the following:

  - Should we keep "ignore" as a synonym for "verbatim" and "warn" as
    a synonym for "warn-verbatim"? My opinion is that they might be
    confusing, so we might want to remove them for `git fast-import`
    even if we keep them for `git fast-export`. The parsing code might
    be a bit more complex if we do that though, so for now I have kept
    the synonyms.

  - Are we still fine with most <mode>s having a "warn-*" variant
    (like the "warn-strip" variant of "strip" for example)? Or should
    we have a separate `--verbose` or maybe `--signed-commits-verbose`
    option dedicated to switching warnings on/off? I think it's good
    to decide about this before the number of <mode>s increases a lot
    with new <mode>s like "strip-if-invalid", "re-sign",
    "re-sign-if-invalid" and possibly others.

Changes since v2:
=================

Thanks to Junio and Patrick for reviewing previous versions.

In patch 2/2:

  - The commit message has been improved to explain why new functions
    are added and how the 'strip' and 'warn-strip' modes are handled.

  - The code responsible for consuming a signature has been refactored
    into two new functions: import_one_signature() and
    discard_one_signature() functions. This makes it easier to follow
    the logic and add new modes in the future.

  - The SIGN_ABORT case is not handled separately from the other cases
    anymore.

  - Code comments have been added to the
    `switch (signed_commit_mode) { ... }` that handles the different
    cases, and a `default: ...` case that calls BUG() has been added
    to it.

CI tests:
=========

They have all passed. See:

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

Range-diff vs V2:
=================

1:  87149ae92d = 1:  87149ae92d gpg-interface: refactor 'enum sign_mode' parsing
2:  e34f015aea ! 2:  cf6ce66e1d fast-import: add '--signed-commits=<mode>' option
    @@ Commit message
         For now the supported <mode>s are the same as those supported by
         `git fast-export`.
     
    +    The code responsible for consuming a signature is refactored into
    +    the import_one_signature() and discard_one_signature() functions,
    +    which makes it easier to follow the logic and add new modes in the
    +    future.
    +
    +    In the 'strip' and 'warn-strip' modes, we deliberately use
    +    discard_one_signature() to discard the signature without parsing it.
    +    This ensures that even malformed signatures, which would cause the
    +    parser to fail, can be successfully stripped from a commit.
    +
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## Documentation/git-fast-import.adoc ##
    @@ builtin/fast-import.c: static int global_argc;
      /* Memory pools */
      static struct mem_pool fi_mem_pool = {
      	.block_alloc = 2*1024*1024 - sizeof(struct mp_block),
    +@@ builtin/fast-import.c: static void parse_one_signature(struct signature_data *sig, const char *v)
    + 	parse_data(&sig->data, 0, NULL);
    + }
    + 
    ++static void discard_one_signature(void)
    ++{
    ++	struct strbuf data = STRBUF_INIT;
    ++
    ++	read_next_command();
    ++	parse_data(&data, 0, NULL);
    ++	strbuf_release(&data);
    ++}
    ++
    + static void add_gpgsig_to_commit(struct strbuf *commit_data,
    + 				 const char *header,
    + 				 struct signature_data *sig)
    +@@ builtin/fast-import.c: static void store_signature(struct signature_data *stored_sig,
    + 	}
    + }
    + 
    ++static void import_one_signature(struct signature_data *sig_sha1,
    ++				 struct signature_data *sig_sha256,
    ++				 const char *v)
    ++{
    ++	struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    ++
    ++	parse_one_signature(&sig, v);
    ++
    ++	if (!strcmp(sig.hash_algo, "sha1"))
    ++		store_signature(sig_sha1, &sig, "SHA-1");
    ++	else if (!strcmp(sig.hash_algo, "sha256"))
    ++		store_signature(sig_sha256, &sig, "SHA-256");
    ++	else
    ++		die(_("parse_one_signature() returned unknown hash algo"));
    ++}
    ++
    + static void parse_new_commit(const char *arg)
    + {
    + 	static struct strbuf msg = STRBUF_INIT;
     @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
      	if (!committer)
      		die("Expected committer but didn't get one");
      
     -	/* Process signatures (up to 2: one "sha1" and one "sha256") */
      	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
    - 		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    - 
    +-		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
    +-
     -		parse_one_signature(&sig, v);
    -+		if (signed_commit_mode == SIGN_ABORT)
    -+			die(_("encountered signed commit; use "
    -+			      "--signed-commits=<mode> to handle it"));
    ++		switch (signed_commit_mode) {
    ++
    ++		/* First, modes that don't need the signature to be parsed */
    ++		case SIGN_ABORT:
    ++			die("encountered signed commit; use "
    ++			    "--signed-commits=<mode> to handle it");
    ++		case SIGN_WARN_STRIP:
    ++			warning(_("stripping a commit signature"));
    ++			/* fallthru */
    ++		case SIGN_STRIP:
    ++			discard_one_signature();
    ++			break;
      
     -		if (!strcmp(sig.hash_algo, "sha1"))
     -			store_signature(&sig_sha1, &sig, "SHA-1");
    @@ builtin/fast-import.c: static void parse_new_commit(const char *arg)
     -			store_signature(&sig_sha256, &sig, "SHA-256");
     -		else
     -			BUG("parse_one_signature() returned unknown hash algo");
    -+		parse_one_signature(&sig, v);
    - 
    -+		switch (signed_commit_mode) {
    -+		case SIGN_ABORT:
    -+			BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
    -+			break;
    ++		/* Second, modes that parse the signature */
     +		case SIGN_WARN_VERBATIM:
     +			warning(_("importing a commit signature verbatim"));
     +			/* fallthru */
     +		case SIGN_VERBATIM:
    -+			if (!strcmp(sig.hash_algo, "sha1"))
    -+				store_signature(&sig_sha1, &sig, "SHA-1");
    -+			else if (!strcmp(sig.hash_algo, "sha256"))
    -+				store_signature(&sig_sha256, &sig, "SHA-256");
    -+			else
    -+				die(_("parse_one_signature() returned unknown hash algo"));
    -+			break;
    -+		case SIGN_WARN_STRIP:
    -+			warning(_("stripping a commit signature"));
    -+			/* fallthru */
    -+		case SIGN_STRIP:
    -+			/* Just discard signature data */
    -+			strbuf_release(&sig.data);
    -+			free(sig.hash_algo);
    ++			import_one_signature(&sig_sha1, &sig_sha256, v);
     +			break;
    + 
    ++		/* Third, BUG */
    ++		default:
    ++			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
     +		}
      		read_next_command();
      	}


Christian Couder (2):
  gpg-interface: refactor 'enum sign_mode' parsing
  fast-import: add '--signed-commits=<mode>' option

 Documentation/git-fast-import.adoc |   5 ++
 builtin/fast-export.c              |  19 ++----
 builtin/fast-import.c              |  63 ++++++++++++++---
 gpg-interface.c                    |  17 +++++
 gpg-interface.h                    |  15 ++++
 t/meson.build                      |   1 +
 t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
 7 files changed, 202 insertions(+), 24 deletions(-)
 create mode 100755 t/t9305-fast-import-signatures.sh

-- 
2.51.0.195.ge34f015aea.dirty

[PATCH v3 1/2] gpg-interface: refactor 'enum sign_mode' parsing

[Christian Couder]

The definition of 'enum sign_mode' as well as its parsing code are in
"builtin/fast-export.c". This was fine because `git fast-export` was the
only command with '--signed-tags=<mode>' or '--signed-commits=<mode>'
options.

In a following commit, we are going to add a similar option to `git
fast-import`, which will be simpler, easier and cleaner if we can reuse
the 'enum sign_mode' defintion and parsing code.

So let's move that definition and parsing code from
"builtin/fast-export.c" to "gpg-interface.{c,h}".

While at it, let's fix a small indentation issue with the arguments of
parse_opt_sign_mode().

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fast-export.c | 19 +++++--------------
 gpg-interface.c       | 17 +++++++++++++++++
 gpg-interface.h       | 15 +++++++++++++++
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c06ee0b213..dc2486f9a8 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -37,8 +37,6 @@ static const char *const fast_export_usage[] = {
 	NULL
 };
 
-enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_WARN_VERBATIM, SIGN_WARN_STRIP };
-
 static int progress;
 static enum sign_mode signed_tag_mode = SIGN_ABORT;
 static enum sign_mode signed_commit_mode = SIGN_STRIP;
@@ -59,23 +57,16 @@ static struct hashmap anonymized_seeds;
 static struct revision_sources revision_sources;
 
 static int parse_opt_sign_mode(const struct option *opt,
-				     const char *arg, int unset)
+			       const char *arg, int unset)
 {
 	enum sign_mode *val = opt->value;
+
 	if (unset)
 		return 0;
-	else if (!strcmp(arg, "abort"))
-		*val = SIGN_ABORT;
-	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
-		*val = SIGN_VERBATIM;
-	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
-		*val = SIGN_WARN_VERBATIM;
-	else if (!strcmp(arg, "warn-strip"))
-		*val = SIGN_WARN_STRIP;
-	else if (!strcmp(arg, "strip"))
-		*val = SIGN_STRIP;
-	else
+
+	if (parse_sign_mode(arg, val))
 		return error("Unknown %s mode: %s", opt->long_name, arg);
+
 	return 0;
 }
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 06e7fb5060..2f4f0e32cb 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1125,3 +1125,20 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
 	FREE_AND_NULL(ssh_signing_key_file);
 	return ret;
 }
+
+int parse_sign_mode(const char *arg, enum sign_mode *mode)
+{
+	if (!strcmp(arg, "abort"))
+		*mode = SIGN_ABORT;
+	else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
+		*mode = SIGN_VERBATIM;
+	else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn"))
+		*mode = SIGN_WARN_VERBATIM;
+	else if (!strcmp(arg, "warn-strip"))
+		*mode = SIGN_WARN_STRIP;
+	else if (!strcmp(arg, "strip"))
+		*mode = SIGN_STRIP;
+	else
+		return -1;
+	return 0;
+}
diff --git a/gpg-interface.h b/gpg-interface.h
index 60ddf8bbfa..50487aa148 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -104,4 +104,19 @@ int check_signature(struct signature_check *sigc,
 void print_signature_buffer(const struct signature_check *sigc,
 			    unsigned flags);
 
+/* Modes for --signed-tags=<mode> and --signed-commits=<mode> options. */
+enum sign_mode {
+	SIGN_ABORT,
+	SIGN_WARN_VERBATIM,
+	SIGN_VERBATIM,
+	SIGN_WARN_STRIP,
+	SIGN_STRIP,
+};
+
+/*
+ * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1
+ * otherwise.
+ */
+int parse_sign_mode(const char *arg, enum sign_mode *mode);
+
 #endif
-- 
2.51.0.195.ge34f015aea.dirty

[PATCH v3 2/2] fast-import: add '--signed-commits=<mode>' option

[Christian Couder]

A '--signed-commits=<mode>' option is already available when using
`git fast-export` to decide what should be done at export time about
commit signatures. At import time though, there is no option, or
other way, in `git fast-import` to decide about commit signatures.

To remediate that, let's add a '--signed-commits=<mode>' option to
`git fast-import` too.

For now the supported <mode>s are the same as those supported by
`git fast-export`.

The code responsible for consuming a signature is refactored into
the import_one_signature() and discard_one_signature() functions,
which makes it easier to follow the logic and add new modes in the
future.

In the 'strip' and 'warn-strip' modes, we deliberately use
discard_one_signature() to discard the signature without parsing it.
This ensures that even malformed signatures, which would cause the
parser to fail, can be successfully stripped from a commit.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-fast-import.adoc |   5 ++
 builtin/fast-import.c              |  63 ++++++++++++++---
 t/meson.build                      |   1 +
 t/t9305-fast-import-signatures.sh  | 106 +++++++++++++++++++++++++++++
 4 files changed, 165 insertions(+), 10 deletions(-)
 create mode 100755 t/t9305-fast-import-signatures.sh

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 3144ffcdb6..90f242d058 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,6 +66,11 @@ OPTIONS
 	remote-helpers that use the `import` capability, as they are
 	already trusted to run their own code.
 
+--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').
+
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2c35f9345d..2010e78475 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -188,6 +188,8 @@ static int global_argc;
 static const char **global_argv;
 static const char *global_prefix;
 
+static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
+
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
 	.block_alloc = 2*1024*1024 - sizeof(struct mp_block),
@@ -2752,6 +2754,15 @@ static void parse_one_signature(struct signature_data *sig, const char *v)
 	parse_data(&sig->data, 0, NULL);
 }
 
+static void discard_one_signature(void)
+{
+	struct strbuf data = STRBUF_INIT;
+
+	read_next_command();
+	parse_data(&data, 0, NULL);
+	strbuf_release(&data);
+}
+
 static void add_gpgsig_to_commit(struct strbuf *commit_data,
 				 const char *header,
 				 struct signature_data *sig)
@@ -2785,6 +2796,22 @@ static void store_signature(struct signature_data *stored_sig,
 	}
 }
 
+static void import_one_signature(struct signature_data *sig_sha1,
+				 struct signature_data *sig_sha256,
+				 const char *v)
+{
+	struct signature_data sig = { NULL, NULL, STRBUF_INIT };
+
+	parse_one_signature(&sig, v);
+
+	if (!strcmp(sig.hash_algo, "sha1"))
+		store_signature(sig_sha1, &sig, "SHA-1");
+	else if (!strcmp(sig.hash_algo, "sha256"))
+		store_signature(sig_sha256, &sig, "SHA-256");
+	else
+		die(_("parse_one_signature() returned unknown hash algo"));
+}
+
 static void parse_new_commit(const char *arg)
 {
 	static struct strbuf msg = STRBUF_INIT;
@@ -2817,19 +2844,32 @@ static void parse_new_commit(const char *arg)
 	if (!committer)
 		die("Expected committer but didn't get one");
 
-	/* Process signatures (up to 2: one "sha1" and one "sha256") */
 	while (skip_prefix(command_buf.buf, "gpgsig ", &v)) {
-		struct signature_data sig = { NULL, NULL, STRBUF_INIT };
-
-		parse_one_signature(&sig, v);
+		switch (signed_commit_mode) {
+
+		/* First, modes that don't need the signature to be parsed */
+		case SIGN_ABORT:
+			die("encountered signed commit; use "
+			    "--signed-commits=<mode> to handle it");
+		case SIGN_WARN_STRIP:
+			warning(_("stripping a commit signature"));
+			/* fallthru */
+		case SIGN_STRIP:
+			discard_one_signature();
+			break;
 
-		if (!strcmp(sig.hash_algo, "sha1"))
-			store_signature(&sig_sha1, &sig, "SHA-1");
-		else if (!strcmp(sig.hash_algo, "sha256"))
-			store_signature(&sig_sha256, &sig, "SHA-256");
-		else
-			BUG("parse_one_signature() returned unknown hash algo");
+		/* Second, modes that parse the signature */
+		case SIGN_WARN_VERBATIM:
+			warning(_("importing a commit signature verbatim"));
+			/* fallthru */
+		case SIGN_VERBATIM:
+			import_one_signature(&sig_sha1, &sig_sha256, v);
+			break;
 
+		/* Third, BUG */
+		default:
+			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
+		}
 		read_next_command();
 	}
 
@@ -3501,6 +3541,9 @@ static int parse_one_option(const char *option)
 		option_active_branches(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))
+			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
 		quiet = 1;
diff --git a/t/meson.build b/t/meson.build
index 82af229be3..08ad6938e2 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -1032,6 +1032,7 @@ integration_tests = [
   't9302-fast-import-unpack-limit.sh',
   't9303-fast-import-compression.sh',
   't9304-fast-import-marks.sh',
+  't9305-fast-import-signatures.sh',
   't9350-fast-export.sh',
   't9351-fast-export-anonymize.sh',
   't9400-git-cvsserver-server.sh',
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
new file mode 100755
index 0000000000..c2b4271658
--- /dev/null
+++ b/t/t9305-fast-import-signatures.sh
@@ -0,0 +1,106 @@
+#!/bin/sh
+
+test_description='git fast-import --signed-commits=<mode>'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success 'set up unsigned initial commit and import repo' '
+	test_commit first &&
+	git init new
+'
+
+test_expect_success GPG 'set up OpenPGP signed commit' '
+	git checkout -b openpgp-signing main &&
+	echo "Content for OpenPGP signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "OpenPGP signed commit" &&
+	OPENPGP_SIGNING=$(git rev-parse --verify openpgp-signing)
+'
+
+test_expect_success GPG 'import OpenPGP signature with --signed-commits=verbatim' '
+	git fast-export --signed-commits=verbatim openpgp-signing >output &&
+	git -C new fast-import --quiet --signed-commits=verbatim <output >log 2>&1 &&
+	IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
+	test $OPENPGP_SIGNING = $IMPORTED &&
+	test_must_be_empty log
+'
+
+test_expect_success GPGSM 'set up X.509 signed commit' '
+	git checkout -b x509-signing main &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo "Content for X.509 signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "X.509 signed commit" &&
+	X509_SIGNING=$(git rev-parse HEAD)
+'
+
+test_expect_success GPGSM 'import X.509 signature fails with --signed-commits=abort' '
+	git fast-export --signed-commits=verbatim x509-signing >output &&
+	test_must_fail git -C new fast-import --quiet --signed-commits=abort <output
+'
+
+test_expect_success GPGSM 'import X.509 signature with --signed-commits=warn-verbatim' '
+	git -C new fast-import --quiet --signed-commits=warn-verbatim <output >log 2>&1 &&
+	IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
+	test $X509_SIGNING = $IMPORTED &&
+	test_grep "importing a commit signature" log
+'
+
+test_expect_success GPGSSH 'set up SSH signed commit' '
+	git checkout -b ssh-signing main &&
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "Content for SSH signing." >file-sign &&
+	git add file-sign &&
+	git commit -S -m "SSH signed commit" &&
+	SSH_SIGNING=$(git rev-parse HEAD)
+'
+
+test_expect_success GPGSSH 'strip SSH signature with --signed-commits=strip' '
+	git fast-export --signed-commits=verbatim ssh-signing >output &&
+	git -C new fast-import --quiet --signed-commits=strip <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" actual &&
+	test_must_be_empty log
+'
+
+test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-1 and SHA-256 formats' '
+	# Create a signed SHA-256 commit
+	git init --object-format=sha256 explicit-sha256 &&
+	git -C explicit-sha256 config extensions.compatObjectFormat sha1 &&
+	git -C explicit-sha256 checkout -b dual-signed &&
+	test_commit -C explicit-sha256 A &&
+	echo B >explicit-sha256/B &&
+	git -C explicit-sha256 add B &&
+	test_tick &&
+	git -C explicit-sha256 commit -S -m "signed" B &&
+	SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
+
+	# Create the corresponding SHA-1 commit
+	SHA1_B=$(git -C explicit-sha256 rev-parse --output-object-format=sha1 dual-signed) &&
+
+	# Check that the resulting SHA-1 commit has both signatures
+	git -C explicit-sha256 cat-file -p $SHA1_B >out &&
+	test_grep -E "^gpgsig " out &&
+	test_grep -E "^gpgsig-sha256 " out
+'
+
+test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=warn-strip' '
+	git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
+	test_grep -E "^gpgsig sha1 openpgp" output &&
+	test_grep -E "^gpgsig sha256 openpgp" output &&
+	git -C new fast-import --quiet --signed-commits=warn-strip <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_grep "stripping a commit signature" log >out &&
+	test_line_count = 2 out
+'
+
+test_done
-- 
2.51.0.195.ge34f015aea.dirty

Re: [PATCH v2 2/2] fast-import: add '--signed-commits=<mode>' option

[Christian Couder]

On Mon, Sep 15, 2025 at 12:56 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Sep 15, 2025 at 12:17:33PM +0200, Christian Couder wrote:
> > On Mon, Sep 15, 2025 at 8:27 AM Patrick Steinhardt <ps@pks.im> wrote:
> > >
> > > On Fri, Sep 12, 2025 at 02:40:42PM +0200, Christian Couder wrote:

> > > >
> > > > +             switch (signed_commit_mode) {
> > > > +             case SIGN_ABORT:
> > > > +                     BUG("SIGN_ABORT should be handled before calling parse_one_signature()");
> > > > +                     break;
> > >
> > > Let's be defensive and convert this into a `default:` case so that any
> > > unhandled value will cause a BUG.
> >
> > Ok, maybe something like BUG("invalid signed_commit_mode value %d",
> > signed_commit_mode) then?
>
> Yeah, that should do the job.

In the V3 I just sent, I have changed the switch (...) { ... } but
there is a `default: ...` case with the BUG() instruction we
discussed. Hopefully it still does the job.

Thanks.