[PATCH 0/4] fast-import: extend signed object handling modes

[PATCH 0/4] fast-import: extend signed object handling modes

[Justin Tobler]

Greetings,

The '--signed-{commits,tags}=<mode>' options for git-fast-import(1)
allow users to configure how signed objects should be handled at time of
import. With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17) and ee66c793f8 (fast-import: add
mode to sign commits with invalid signatures, 2026-03-12), the
'strip-if-invalid' and 'sign-if-invalid' modes were added for the
'--signed-commits' option only.

This series extends '--signed-commits' by adding an 'abort-if-invalid'
mode which aborts the entire import operation when a commit signature
fails verification. Additionally, the '--signed-tags' option is brought
into parity with '--signed-commits' by supporting equivalent,
'strip-if-invalid', 'sign-if-invalid', and 'abort-if-invalid' modes.

This series is built on top of 1080981ddb (The 19th batch, 2026-03-23)
with ee66c793f8 (fast-import: add mode to sign commits with invalid
signatures, 2026-03-12) merged into it.

Thanks,
-Justin

Justin Tobler (4):
  fast-import: add 'abort-if-invalid' mode to '--signed-commits=<mode>'
  fast-import: add 'strip-if-invalid' mode to '--signed-tags=<mode>'
  fast-import: add 'sign-if-invalid' mode to '--signed-tags=<mode>'
  fast-import: add 'abort-if-invalid' mode to '--signed-tags=<mode>'

 Documentation/git-fast-import.adoc |   9 ++-
 builtin/fast-export.c              |   6 ++
 builtin/fast-import.c              |  71 ++++++++++++++---
 gpg-interface.c                    |   2 +
 gpg-interface.h                    |   1 +
 t/t9305-fast-import-signatures.sh  |  10 ++-
 t/t9306-fast-import-signed-tags.sh | 118 +++++++++++++++++++++++++++++
 7 files changed, 201 insertions(+), 16 deletions(-)

-- 
2.53.0.381.g628a66ccf6

[PATCH 1/4] fast-import: add 'abort-if-invalid' mode to '--signed-commits=<mode>'

[Justin Tobler]

The '--signed-commits=<mode>' option for git-fast-import(1) configures
how signed commits are handled when encountered. In cases where an
invalid commit signature is encountered, a user may wish to abort the
operation entirely. Introduce an 'abort-if-invalid' mode to do so.

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

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index b3f42d4637..288f2b2a7e 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -90,6 +90,8 @@ already trusted to run their own code.
   commit signatures and replaces invalid signatures with newly created ones.
   Valid signatures are left unchanged. If `<keyid>` is provided, that key is
   used for signing; otherwise the configured default signing key is used.
+* `abort-if-invalid` will make this program die when encountering a signed
+  commit that is unable to be verified.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 13621b0d6a..dcbc5bc82d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -822,6 +822,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 			die(_("encountered signed commit %s; use "
 			      "--signed-commits=<mode> to handle it"),
 			    oid_to_hex(&commit->object.oid));
+		case SIGN_ABORT_IF_INVALID:
+			die(_("'abort-if-invalid' is not a valid mode for "
+			      "git fast-export with --signed-commits=<mode>"));
 		case SIGN_STRIP_IF_INVALID:
 			die(_("'strip-if-invalid' is not a valid mode for "
 			      "git fast-export with --signed-commits=<mode>"));
@@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag)
 				die(_("encountered signed tag %s; use "
 				      "--signed-tags=<mode> to handle it"),
 				    oid_to_hex(&tag->object.oid));
+			case SIGN_ABORT_IF_INVALID:
+				die(_("'abort-if-invalid' is not a valid mode for "
+				      "git fast-export with --signed-tags=<mode>"));
 			case SIGN_STRIP_IF_INVALID:
 				die(_("'strip-if-invalid' is not a valid mode for "
 				      "git fast-export with --signed-tags=<mode>"));
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 9fc6c35b74..08ea27242d 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -2892,6 +2892,9 @@ static void handle_signature_if_invalid(struct strbuf *new_data,
 	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
 
 	if (ret) {
+		if (mode == SIGN_ABORT_IF_INVALID)
+			die(_("aborting due to invalid signature"));
+
 		warn_invalid_signature(&signature_check, msg->buf, mode);
 
 		if (mode == SIGN_SIGN_IF_INVALID) {
@@ -2983,6 +2986,7 @@ static void parse_new_commit(const char *arg)
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
 		case SIGN_SIGN_IF_INVALID:
+		case SIGN_ABORT_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3068,7 +3072,8 @@ static void parse_new_commit(const char *arg)
 			encoding);
 
 	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
-	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
+	     signed_commit_mode == SIGN_SIGN_IF_INVALID ||
+	     signed_commit_mode == SIGN_ABORT_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
 		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
 					    &msg, signed_commit_mode);
@@ -3115,6 +3120,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_ABORT:
 		die(_("encountered signed tag; use "
 		      "--signed-tags=<mode> to handle it"));
+	case SIGN_ABORT_IF_INVALID:
+		die(_("'abort-if-invalid' is not a valid mode for "
+		      "git fast-import with --signed-tags=<mode>"));
 	case SIGN_STRIP_IF_INVALID:
 		die(_("'strip-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
diff --git a/gpg-interface.c b/gpg-interface.c
index d517425034..dafd5371fa 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -1164,6 +1164,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 		*mode = SIGN_WARN_STRIP;
 	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
+	} else if (!strcmp(arg, "abort-if-invalid")) {
+		*mode = SIGN_ABORT_IF_INVALID;
 	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
 	} else if (!strcmp(arg, "sign-if-invalid")) {
diff --git a/gpg-interface.h b/gpg-interface.h
index a365586ce1..3d95f5ec14 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -115,6 +115,7 @@ void print_signature_buffer(const struct signature_check *sigc,
 /* Modes for --signed-tags=<mode> and --signed-commits=<mode> options. */
 enum sign_mode {
 	SIGN_ABORT,
+	SIGN_ABORT_IF_INVALID,
 	SIGN_WARN_VERBATIM,
 	SIGN_VERBATIM,
 	SIGN_WARN_STRIP,
diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh
index 18707b3f6c..5667693afd 100755
--- a/t/t9305-fast-import-signatures.sh
+++ b/t/t9305-fast-import-signatures.sh
@@ -103,7 +103,7 @@ test_expect_success RUST,GPG 'strip both OpenPGP signatures with --signed-commit
 	test_line_count = 2 out
 '
 
-for mode in strip-if-invalid sign-if-invalid
+for mode in strip-if-invalid sign-if-invalid abort-if-invalid
 do
 	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
 		git fast-export main >output &&
@@ -135,6 +135,14 @@ do
 		# corresponding `data <length>` command would have to be changed too.
 		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
+		if test "$mode" = abort-if-invalid
+		then
+			test_must_fail git -C new fast-import --quiet \
+				--signed-commits=$mode <modified >log 2>&1 &&
+			test_grep "aborting due to invalid signature" log &&
+			return 0
+		fi &&
+
 		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
 
 		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
-- 
2.53.0.381.g628a66ccf6

[PATCH 2/4] fast-import: add 'strip-if-invalid' mode to '--signed-tags=<mode>'

[Justin Tobler]

With c20f112e51 (fast-import: add 'strip-if-invalid' mode to
--signed-commits=<mode>, 2025-11-17), git-fast-import(1) learned to
verify commit signatures during import and strip signatures that fail
verification. Extend the same behavior to signed tag objects by
introducing a 'strip-if-invalid' mode for the '--signed-tags' option.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 Documentation/git-fast-import.adoc |  7 ++-
 builtin/fast-import.c              | 40 +++++++++++++---
 t/t9306-fast-import-signed-tags.sh | 73 ++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 288f2b2a7e..d68bc52b7e 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -66,11 +66,10 @@ 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)`::
+`--signed-tags=<mode>`::
 	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`.
+	the `--signed-commits=<mode>` below. Like for signed commits,
+	the default mode is `verbatim`.
 
 `--signed-commits=<mode>`::
 	Specify how to handle signed commits. The following <mode>s
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 08ea27242d..5e89829aea 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3089,7 +3089,34 @@ static void parse_new_commit(const char *arg)
 	b->last_commit = object_count_by_type[OBJ_COMMIT];
 }
 
-static void handle_tag_signature(struct strbuf *msg, const char *name)
+static void handle_tag_signature_if_invalid(struct strbuf *buf,
+					    struct strbuf *msg,
+					    size_t sig_offset)
+{
+	struct strbuf signature = STRBUF_INIT;
+	struct strbuf payload = STRBUF_INIT;
+	struct signature_check sigc = { 0 };
+
+	strbuf_addbuf(&payload, buf);
+	strbuf_addch(&payload, '\n');
+	strbuf_add(&payload, msg->buf, sig_offset);
+	strbuf_add(&signature, msg->buf + sig_offset, msg->len - sig_offset);
+
+	sigc.payload_type = SIGNATURE_PAYLOAD_TAG;
+	sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
+
+	if (!check_signature(&sigc, signature.buf, signature.len))
+		goto out;
+
+	strbuf_setlen(msg, sig_offset);
+
+out:
+	signature_check_clear(&sigc);
+	strbuf_release(&signature);
+	strbuf_release(&payload);
+}
+
+static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const char *name)
 {
 	size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
 
@@ -3115,6 +3142,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 		/* Truncate the buffer to remove the signature */
 		strbuf_setlen(msg, sig_offset);
 		break;
+	case SIGN_STRIP_IF_INVALID:
+		handle_tag_signature_if_invalid(buf, msg, sig_offset);
+		break;
 
 	/* Third, aborting modes */
 	case SIGN_ABORT:
@@ -3123,9 +3153,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 	case SIGN_ABORT_IF_INVALID:
 		die(_("'abort-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
-	case SIGN_STRIP_IF_INVALID:
-		die(_("'strip-if-invalid' is not a valid mode for "
-		      "git fast-import with --signed-tags=<mode>"));
 	case SIGN_SIGN_IF_INVALID:
 		die(_("'sign-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
@@ -3198,8 +3225,6 @@ static void parse_new_tag(const char *arg)
 	/* tag payload/message */
 	parse_data(&msg, 0, NULL);
 
-	handle_tag_signature(&msg, t->name);
-
 	/* build the tag object */
 	strbuf_reset(&new_data);
 
@@ -3211,6 +3236,9 @@ static void parse_new_tag(const char *arg)
 	if (tagger)
 		strbuf_addf(&new_data,
 			    "tagger %s\n", tagger);
+
+	handle_tag_signature(&new_data, &msg, t->name);
+
 	strbuf_addch(&new_data, '\n');
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
index 363619e7d1..fd43b0b52a 100755
--- a/t/t9306-fast-import-signed-tags.sh
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -77,4 +77,77 @@ test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
 	test_grep ! "SSH SIGNATURE" out
 '
 
+for mode in strip-if-invalid
+do
+	test_expect_success GPG "import tag with no signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim openpgp-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
+		test $OPENPGP_SIGNED = $IMPORTED &&
+		git -C import cat-file tag "$IMPORTED" >actual &&
+		test_grep -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim openpgp-signed >output &&
+
+		# Change the tag message, which invalidates the signature. The tag
+		# message length should not change though, otherwise the corresponding
+		# `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed tag/OpenPGP forged tag/" output >modified &&
+
+		git -C import fast-import --quiet --signed-tags=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
+		test $OPENPGP_SIGNED != $IMPORTED &&
+		git -C import cat-file tag "$IMPORTED" >actual &&
+		test_grep ! -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim x509-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/x509-signed) &&
+		test $X509_SIGNED = $IMPORTED &&
+		git -C import cat-file tag x509-signed >actual &&
+		test_grep -E "^-----BEGIN SIGNED MESSAGE-----" actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		test_config -C import gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-tags=verbatim ssh-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/ssh-signed) &&
+		test $SSH_SIGNED = $IMPORTED &&
+		git -C import cat-file tag ssh-signed >actual &&
+		test_grep -E "^-----BEGIN SSH SIGNATURE-----" actual &&
+		test_must_be_empty log
+	'
+done
+
 test_done
-- 
2.53.0.381.g628a66ccf6

[PATCH 3/4] fast-import: add 'sign-if-invalid' mode to '--signed-tags=<mode>'

[Justin Tobler]

With ee66c793f8 (fast-import: add mode to sign commits with invalid
signatures, 2026-03-12), git-fast-import(1) learned to verify commit
signatures during import and replace signatures that fail verification
with a newly generated one. Extend the same behavior to signed tag
objects by introducing a 'sign-if-invalid' mode for the '--signed-tags'
option.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/fast-import.c              | 20 ++++++++++++---
 t/t9306-fast-import-signed-tags.sh | 41 ++++++++++++++++++++++++++++--
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 5e89829aea..783e0e7ab4 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -191,6 +191,7 @@ static const char *global_prefix;
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
 static const char *signed_commit_keyid;
+static const char *signed_tag_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -3110,6 +3111,19 @@ static void handle_tag_signature_if_invalid(struct strbuf *buf,
 
 	strbuf_setlen(msg, sig_offset);
 
+	if (signed_tag_mode == SIGN_SIGN_IF_INVALID) {
+		strbuf_attach(&payload, sigc.payload, sigc.payload_len,
+			      sigc.payload_len + 1);
+		sigc.payload = NULL;
+		strbuf_reset(&signature);
+
+		if (sign_buffer(&payload, &signature, signed_tag_keyid,
+				SIGN_BUFFER_USE_DEFAULT_KEY))
+			die(_("failed to sign tag object"));
+
+		strbuf_addbuf(msg, &signature);
+	}
+
 out:
 	signature_check_clear(&sigc);
 	strbuf_release(&signature);
@@ -3142,6 +3156,7 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c
 		/* Truncate the buffer to remove the signature */
 		strbuf_setlen(msg, sig_offset);
 		break;
+	case SIGN_SIGN_IF_INVALID:
 	case SIGN_STRIP_IF_INVALID:
 		handle_tag_signature_if_invalid(buf, msg, sig_offset);
 		break;
@@ -3153,9 +3168,6 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c
 	case SIGN_ABORT_IF_INVALID:
 		die(_("'abort-if-invalid' is not a valid mode for "
 		      "git fast-import with --signed-tags=<mode>"));
-	case SIGN_SIGN_IF_INVALID:
-		die(_("'sign-if-invalid' is not a valid mode for "
-		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3749,7 +3761,7 @@ static int parse_one_option(const char *option)
 		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode, NULL))
+		if (parse_sign_mode(option, &signed_tag_mode, &signed_tag_keyid))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;
diff --git a/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
index fd43b0b52a..bb4c8008ef 100755
--- a/t/t9306-fast-import-signed-tags.sh
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -77,7 +77,7 @@ test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
 	test_grep ! "SSH SIGNATURE" out
 '
 
-for mode in strip-if-invalid
+for mode in strip-if-invalid sign-if-invalid
 do
 	test_expect_success GPG "import tag with no signature with --signed-tags=$mode" '
 		test_when_finished rm -rf import &&
@@ -117,7 +117,15 @@ do
 		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
 		test $OPENPGP_SIGNED != $IMPORTED &&
 		git -C import cat-file tag "$IMPORTED" >actual &&
-		test_grep ! -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep ! -E "^-----BEGIN PGP SIGNATURE-----" actual
+		else
+			test_grep -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+			git -C import verify-tag "$IMPORTED"
+		fi &&
+
 		test_must_be_empty log
 	'
 
@@ -150,4 +158,33 @@ do
 	'
 done
 
+test_expect_success GPGSSH 'sign invalid tag with explicit keyid' '
+	test_when_finished rm -rf import &&
+	git init import &&
+
+	git fast-export --signed-tags=verbatim ssh-signed >output &&
+
+	# Change the tag message, which invalidates the signature. The tag
+	# message length should not change though, otherwise the corresponding
+	# `data <length>` command would have to be changed too.
+	sed "s/SSH signed tag/SSH forged tag/" output >modified &&
+
+	# Configure the target repository with an invalid default signing key.
+	test_config -C import user.signingkey "not-a-real-key-id" &&
+	test_config -C import gpg.format ssh &&
+	test_config -C import gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C import fast-import --quiet \
+		--signed-tags=sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C import fast-import --quiet \
+		--signed-tags=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
+
+	IMPORTED=$(git -C import rev-parse --verify refs/tags/ssh-signed) &&
+	test $SSH_SIGNED != $IMPORTED &&
+	git -C import cat-file tag "$IMPORTED" >actual &&
+	test_grep -E "^-----BEGIN SSH SIGNATURE-----" actual &&
+	git -C import verify-tag "$IMPORTED"
+'
+
 test_done
-- 
2.53.0.381.g628a66ccf6

[PATCH 4/4] fast-import: add 'abort-if-invalid' mode to '--signed-tags=<mode>'

[Justin Tobler]

In git-fast-import(1), the 'abort-if-invalid' mode for the
'--signed-commits' option verifies commit signatures during import and
aborts the entire operation when verification fails. Extend the same
behavior to signed tag objects by introducing an 'abort-if-invalid' mode
for the '--signed-tags' option.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 builtin/fast-import.c              |  7 ++++---
 t/t9306-fast-import-signed-tags.sh | 10 +++++++++-
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 783e0e7ab4..cd1181023d 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3109,6 +3109,9 @@ static void handle_tag_signature_if_invalid(struct strbuf *buf,
 	if (!check_signature(&sigc, signature.buf, signature.len))
 		goto out;
 
+	if (signed_tag_mode == SIGN_ABORT_IF_INVALID)
+		die(_("aborting due to invalid signature"));
+
 	strbuf_setlen(msg, sig_offset);
 
 	if (signed_tag_mode == SIGN_SIGN_IF_INVALID) {
@@ -3156,6 +3159,7 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c
 		/* Truncate the buffer to remove the signature */
 		strbuf_setlen(msg, sig_offset);
 		break;
+	case SIGN_ABORT_IF_INVALID:
 	case SIGN_SIGN_IF_INVALID:
 	case SIGN_STRIP_IF_INVALID:
 		handle_tag_signature_if_invalid(buf, msg, sig_offset);
@@ -3165,9 +3169,6 @@ static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const c
 	case SIGN_ABORT:
 		die(_("encountered signed tag; use "
 		      "--signed-tags=<mode> to handle it"));
-	case SIGN_ABORT_IF_INVALID:
-		die(_("'abort-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/t/t9306-fast-import-signed-tags.sh b/t/t9306-fast-import-signed-tags.sh
index bb4c8008ef..ec2b241cdb 100755
--- a/t/t9306-fast-import-signed-tags.sh
+++ b/t/t9306-fast-import-signed-tags.sh
@@ -77,7 +77,7 @@ test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
 	test_grep ! "SSH SIGNATURE" out
 '
 
-for mode in strip-if-invalid sign-if-invalid
+for mode in strip-if-invalid sign-if-invalid abort-if-invalid
 do
 	test_expect_success GPG "import tag with no signature with --signed-tags=$mode" '
 		test_when_finished rm -rf import &&
@@ -112,6 +112,14 @@ do
 		# `data <length>` command would have to be changed too.
 		sed "s/OpenPGP signed tag/OpenPGP forged tag/" output >modified &&
 
+		if test "$mode" = abort-if-invalid
+		then
+			test_must_fail git -C import fast-import --quiet \
+				--signed-tags=$mode <modified >log 2>&1 &&
+			test_grep "aborting due to invalid signature" log &&
+			return 0
+		fi &&
+
 		git -C import fast-import --quiet --signed-tags=$mode <modified >log 2>&1 &&
 
 		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
-- 
2.53.0.381.g628a66ccf6

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

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 13621b0d6a..dcbc5bc82d 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -822,6 +822,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
>  			die(_("encountered signed commit %s; use "
>  			      "--signed-commits=<mode> to handle it"),
>  			    oid_to_hex(&commit->object.oid));
> +		case SIGN_ABORT_IF_INVALID:
> +			die(_("'abort-if-invalid' is not a valid mode for "
> +			      "git fast-export with --signed-commits=<mode>"));
>  		case SIGN_STRIP_IF_INVALID:
>  			die(_("'strip-if-invalid' is not a valid mode for "
>  			      "git fast-export with --signed-commits=<mode>"));

There are a few similar hunks in this patch to fast-export, but I am
not sure what is going on here.

I may be misreading the code, but this error, and the similar one
for strip-if-invalid that is already there, trigger if the command
is given "--signed-commits=abort-if-invalid" on its command line,
and when we see a signed commit in the range we walk to export the
commits.  Why shouldn't the user get the error immediately while the
command is parsing the command line options?  You may be sharing the
underlying parse_sign_mode() with import side that may support more
variants, but that is not a good excuse to make these two

    git fast-export --signed-commits=abort-if-invalid
    git fast-export --signed-commits=i-dont-know-what-i-am-doing

behave completely differently, no?

You can either move these "no, these subset of options are not
available here" to fast-export.c::parse_opt_sign_mode(), or even
better yet, teach parse_sign_mode() an option to say "hey, you are
being called from fast-export, so pretend that you have never heard
of options that are only available on fast-import" and error out
right there, can't you?  Or would it be too _early_ to give errors
to users?

Thanks.

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

[Justin Tobler]

On 26/03/24 03:22PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> > diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> > index 13621b0d6a..dcbc5bc82d 100644
> > --- a/builtin/fast-export.c
> > +++ b/builtin/fast-export.c
> > @@ -822,6 +822,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
> >  			die(_("encountered signed commit %s; use "
> >  			      "--signed-commits=<mode> to handle it"),
> >  			    oid_to_hex(&commit->object.oid));
> > +		case SIGN_ABORT_IF_INVALID:
> > +			die(_("'abort-if-invalid' is not a valid mode for "
> > +			      "git fast-export with --signed-commits=<mode>"));
> >  		case SIGN_STRIP_IF_INVALID:
> >  			die(_("'strip-if-invalid' is not a valid mode for "
> >  			      "git fast-export with --signed-commits=<mode>"));
> 
> There are a few similar hunks in this patch to fast-export, but I am
> not sure what is going on here.

git-fast-export(1) does not support the `{strip,sign,abort}-if-invalid`
mode for the `--signed-{commits,tags}` options. Therefore we die in
cases where we parse this option with an unsupported mode.

> I may be misreading the code, but this error, and the similar one
> for strip-if-invalid that is already there, trigger if the command
> is given "--signed-commits=abort-if-invalid" on its command line,
> and when we see a signed commit in the range we walk to export the
> commits.  Why shouldn't the user get the error immediately while the
> command is parsing the command line options?  You may be sharing the
> underlying parse_sign_mode() with import side that may support more
> variants, but that is not a good excuse to make these two
> 
>     git fast-export --signed-commits=abort-if-invalid
>     git fast-export --signed-commits=i-dont-know-what-i-am-doing
> 
> behave completely differently, no?

Ya, that is a completely fair point. If the mode is unsupported, it
doesn't make sense to wait until we encounter a signed commit/tag to
declare the user provided an invalid signed mode for the command. 

> You can either move these "no, these subset of options are not
> available here" to fast-export.c::parse_opt_sign_mode(), or even
> better yet, teach parse_sign_mode() an option to say "hey, you are
> being called from fast-export, so pretend that you have never heard
> of options that are only available on fast-import" and error out
> right there, can't you?  Or would it be too _early_ to give errors
> to users?

I think it definately makes sense to move signing mode verification to
be around the same time the option is initially parsed. With this patch
series, the `--signed-commits` and `--signed-tags` options for
git-fast-import(1) will support the same modes. It will only be
git-fast-export(1) that supports a subset of the signing modes. It
should be easy enough to update "fast-export.c:parse_opt_sign_mode()" to
explictly handle the unsupported signing modes upfront.

I'll update in the next version accordingly.

Thanks,
-Justin