{"thread":{"id":"64443","subject":"[PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","startedAt":"2025-11-05T06:19:43Z","lastAt":"2025-11-18T19:04:39Z","messageCount":20,"participants":["Christian Couder","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"530233","messageId":"20251105061918.3688870-1-christian.couder@gmail.com","threadId":"64443","inReplyTo":null,"subject":"[PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-05T06:19:15Z","receivedAt":"2025-11-05T06:19:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The `--signed-commits=<mode>` option in `git fast-import` allows users\nto decide what should be done when commits with signatures are\nimported.\n\nFor tools like `git filter-repo`, it would be useful to be able to\nstrip signatures when they are invalid, so let's add a new\n'strip-if-invalid' mode for that purpose.\n\nMaybe this new mode should become the default mode, but this would be\nbreaking backward compatibility, and perhaps this could be decided\nafter other new modes that might be even better default modes have\nbeen added. So we leave that for future work.\n\nThis 'strip-if-invalid' mode should also be added to\n`--signed-tags=<mode>`, but we leave that for future work too.\n\nCI tests\n========\n\nThey have all passed, see:\n\nhttps://github.com/chriscool/git/actions/runs/19091593841/job/54543228129\n\nChristian Couder (3):\n  fast-import: refactor finalize_commit_buffer()\n  commit: refactor verify_commit_buffer()\n  fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>\n\n Documentation/git-fast-import.adoc |  28 ++++---\n builtin/fast-export.c              |  46 ++++++++---\n builtin/fast-import.c              |  74 +++++++++++++++---\n commit.c                           |  17 ++++-\n commit.h                           |   7 ++\n gpg-interface.c                    |   2 +\n gpg-interface.h                    |   1 +\n t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-\n 8 files changed, 260 insertions(+), 33 deletions(-)\n\n-- \n2.52.0.rc0.3.gf264cd25e5\n\n"},{"id":"530234","messageId":"20251105061918.3688870-2-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251105061918.3688870-1-christian.couder@gmail.com","subject":"[PATCH 1/3] fast-import: refactor finalize_commit_buffer()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-05T06:19:16Z","receivedAt":"2025-11-05T06:19:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In a following commit we are going to finalize commit buffers with or\nwithout signatures in order to check the signatures and possibly drop\nthem.\n\nTo do so easily and without duplication, let's refactor the current\ncode that finalizes commit buffers into a new finalize_commit_buffer()\nfunction.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/fast-import.c | 17 +++++++++++++----\n 1 file changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 54d3e592c6..493de57ef6 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -2815,6 +2815,18 @@ static void import_one_signature(struct signature_data *sig_sha1,\n \t\tdie(_(\"parse_one_signature() returned unknown hash algo\"));\n }\n \n+static void finalize_commit_buffer(struct strbuf *new_data,\n+\t\t\t\t   struct signature_data *sig_sha1,\n+\t\t\t\t   struct signature_data *sig_sha256,\n+\t\t\t\t   struct strbuf *msg)\n+{\n+\tadd_gpgsig_to_commit(new_data, \"gpgsig \", sig_sha1);\n+\tadd_gpgsig_to_commit(new_data, \"gpgsig-sha256 \", sig_sha256);\n+\n+\tstrbuf_addch(new_data, '\\n');\n+\tstrbuf_addbuf(new_data, msg);\n+}\n+\n static void parse_new_commit(const char *arg)\n {\n \tstatic struct strbuf msg = STRBUF_INIT;\n@@ -2950,11 +2962,8 @@ static void parse_new_commit(const char *arg)\n \t\t\t\"encoding %s\\n\",\n \t\t\tencoding);\n \n-\tadd_gpgsig_to_commit(&new_data, \"gpgsig \", &sig_sha1);\n-\tadd_gpgsig_to_commit(&new_data, \"gpgsig-sha256 \", &sig_sha256);\n+\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n \n-\tstrbuf_addch(&new_data, '\\n');\n-\tstrbuf_addbuf(&new_data, &msg);\n \tfree(author);\n \tfree(committer);\n \tfree(encoding);\n-- \n2.52.0.rc0.3.gf264cd25e5\n\n"},{"id":"530235","messageId":"20251105061918.3688870-3-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251105061918.3688870-1-christian.couder@gmail.com","subject":"[PATCH 2/3] commit: refactor verify_commit_buffer()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-05T06:19:17Z","receivedAt":"2025-11-05T06:19:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In a following commit, we are going to check commit signatures, but we\nwon't have a commit yet, only a commit buffer, and we are going to\ndiscard this commit buffer if the signature is invalid. So it would be\nwasteful to create a commit that we might discard, just to be able to\ncheck a commit signature.\n\nIt would be simpler instead to be able to check commit signatures\nusing only a commit buffer instead of a commit.\n\nTo be able to do that, let's extract some code from the\ncheck_commit_signature() function into a new verify_commit_buffer()\nfunction, and then let's make check_commit_signature() call\nverify_commit_buffer().\n\nNote that this doesn't fundamentally change how\ncheck_commit_signature() works. It used to call parse_signed_commit()\nwhich calls repo_get_commit_buffer(), parse_buffer_signed_by_header()\nand repo_unuse_commit_buffer(). Now these 3 functions are called\ndirectly by verify_commit_buffer().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n commit.c | 17 +++++++++++++++--\n commit.h |  7 +++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 16d91b2bfc..709c9eed58 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1315,7 +1315,8 @@ static void handle_signed_tag(const struct commit *parent, struct commit_extra_h\n \tfree(buf);\n }\n \n-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)\n+int verify_commit_buffer(const char *buffer, size_t size,\n+\t\t\t struct signature_check *sigc)\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n@@ -1323,7 +1324,8 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n+\tif (parse_buffer_signed_by_header(buffer, size, &payload,\n+\t\t\t\t\t  &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tsigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;\n@@ -1337,6 +1339,17 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \treturn ret;\n }\n \n+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)\n+{\n+\tunsigned long size;\n+\tconst char *buffer = repo_get_commit_buffer(the_repository, commit, &size);\n+\tint ret = verify_commit_buffer(buffer, size, sigc);\n+\n+\trepo_unuse_commit_buffer(the_repository, commit, buffer);\n+\n+\treturn ret;\n+}\n+\n void verify_merge_signature(struct commit *commit, int verbosity,\n \t\t\t    int check_trust)\n {\ndiff --git a/commit.h b/commit.h\nindex 1d6e0c7518..5406dd2663 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -333,6 +333,13 @@ int remove_signature(struct strbuf *buf);\n  */\n int check_commit_signature(const struct commit *commit, struct signature_check *sigc);\n \n+/*\n+ * Same as check_commit_signature() but accepts a commit buffer and\n+ * its size, instead of a `struct commit *`.\n+ */\n+int verify_commit_buffer(const char *buffer, size_t size,\n+\t\t\t struct signature_check *sigc);\n+\n /* record author-date for each commit object */\n struct author_date_slab;\n void record_author_date(struct author_date_slab *author_date,\n-- \n2.52.0.rc0.3.gf264cd25e5\n\n"},{"id":"530236","messageId":"20251105061918.3688870-4-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251105061918.3688870-1-christian.couder@gmail.com","subject":"[PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-05T06:19:18Z","receivedAt":"2025-11-05T06:19:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Tools like `git filter-repo`[1] use `git fast-export` and\n`git fast-import` to rewrite repository history. When rewriting\nhistory using one such tool though, commit signatures might become\ninvalid because the commits they sign changed due to the changes\nin the repository history made by the tool between the fast-export\nand the fast-import steps.\n\nHaving invalid signatures in a rewritten repository could be\nconfusing, so users rewritting history might prefer to simply\ndiscard signatures that are invalid at the fast-import step.\n\nTo let them do that, let's add a new 'strip-if-invalid' mode to the\n`--signed-commits=<mode>` option of `git fast-import`.\n\nIt would be interesting for the `--signed-tags=<mode>` option to\nhave this mode too, but we leave that for a future improvement.\n\nIt might also be possible for `git fast-export` to have such a mode\nin its `--signed-commits=<mode>` and `--signed-tags=<mode>`\noptions, but the use cases for it are much less clear, so we also\nleave that for possible future improvements.\n\nFor now let's just die() if 'strip-if-invalid' is passed to these\noptions where it hasn't been implemented yet.\n\nWhile at it, let's also mark for translation some error messages\nlinked to the `--signed-commits=<mode>` and `--signed-tags=<mode>`\nin `git fast-export`.\n\n[1]: https://github.com/newren/git-filter-repo\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-fast-import.adoc |  28 ++++---\n builtin/fast-export.c              |  46 ++++++++---\n builtin/fast-import.c              |  59 +++++++++++++--\n gpg-interface.c                    |   2 +\n gpg-interface.h                    |   1 +\n t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-\n 6 files changed, 226 insertions(+), 28 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc\nindex b74179a6c8..c9e49497cd 100644\n--- a/Documentation/git-fast-import.adoc\n+++ b/Documentation/git-fast-import.adoc\n@@ -66,15 +66,25 @@ fast-import stream! This option is enabled automatically for\n remote-helpers that use the `import` capability, as they are\n already trusted to run their own code.\n \n---signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n-\tSpecify how to handle signed tags.  Behaves in the same way\n-\tas the same option in linkgit:git-fast-export[1], except that\n-\tdefault is 'verbatim' (instead of 'abort').\n-\n---signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n-\tSpecify how to handle signed commits.  Behaves in the same way\n-\tas the same option in linkgit:git-fast-export[1], except that\n-\tdefault is 'verbatim' (instead of 'abort').\n+`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::\n+\tSpecify how to handle signed tags. Behaves in the same way as\n+\tthe `--signed-commits=<mode>` below, except that the\n+\t`strip-if-invalid` mode is not yet supported. Like for signed\n+\tcommits, the default mode is `verbatim`.\n+\n+`--signed-commits=<mode>`::\n+\tSpecify how to handle signed commits. The following <mode>s\n+\tare supported:\n++\n+* `verbatim`, which is the default, will silently import commit\n+  signatures.\n+* `warn-verbatim` will import them, but will display a warning.\n+* `abort` will make this program die when encountering a signed\n+  commit.\n+* `strip` will silently make the commits unsigned.\n+* `warn-strip` will make them unsigned, but will display a warning.\n+* `strip-if-invalid` will check signatures and, if they are invalid,\n+  will strip them and display a warning.\n \n Options for Frontends\n ~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 7adbc55f0d..1ad195b639 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -797,12 +797,10 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t       (int)(committer_end - committer), committer);\n \tif (signatures.nr) {\n \t\tswitch (signed_commit_mode) {\n-\t\tcase SIGN_ABORT:\n-\t\t\tdie(\"encountered signed commit %s; use \"\n-\t\t\t    \"--signed-commits=<mode> to handle it\",\n-\t\t\t    oid_to_hex(&commit->object.oid));\n+\n+\t\t/* Exporting modes */\n \t\tcase SIGN_WARN_VERBATIM:\n-\t\t\twarning(\"exporting %\"PRIuMAX\" signature(s) for commit %s\",\n+\t\t\twarning(_(\"exporting %\"PRIuMAX\" signature(s) for commit %s\"),\n \t\t\t\t(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));\n \t\t\t/* fallthru */\n \t\tcase SIGN_VERBATIM:\n@@ -811,12 +809,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t\t\t\tprint_signature(item->string, item->util);\n \t\t\t}\n \t\t\tbreak;\n+\n+\t\t/* Stripping modes */\n \t\tcase SIGN_WARN_STRIP:\n-\t\t\twarning(\"stripping signature(s) from commit %s\",\n+\t\t\twarning(_(\"stripping signature(s) from commit %s\"),\n \t\t\t\toid_to_hex(&commit->object.oid));\n \t\t\t/* fallthru */\n \t\tcase SIGN_STRIP:\n \t\t\tbreak;\n+\n+\t\t/* Aborting modes */\n+\t\tcase SIGN_ABORT:\n+\t\t\tdie(_(\"encountered signed commit %s; use \"\n+\t\t\t      \"--signed-commits=<mode> to handle it\"),\n+\t\t\t    oid_to_hex(&commit->object.oid));\n+\t\tcase SIGN_STRIP_IF_INVALID:\n+\t\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t\t      \"git fast-export with --signed-commits=<mode>\"));\n+\t\tdefault:\n+\t\t\tBUG(\"invalid signed_commit_mode value %d\", signed_commit_mode);\n \t\t}\n \t\tstring_list_clear(&signatures, 0);\n \t}\n@@ -934,23 +945,34 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tsize_t sig_offset = parse_signed_buffer(message, message_size);\n \t\tif (sig_offset < message_size)\n \t\t\tswitch (signed_tag_mode) {\n-\t\t\tcase SIGN_ABORT:\n-\t\t\t\tdie(\"encountered signed tag %s; use \"\n-\t\t\t\t    \"--signed-tags=<mode> to handle it\",\n-\t\t\t\t    oid_to_hex(&tag->object.oid));\n+\n+\t\t\t/* Exporting modes */\n \t\t\tcase SIGN_WARN_VERBATIM:\n-\t\t\t\twarning(\"exporting signed tag %s\",\n+\t\t\t\twarning(_(\"exporting signed tag %s\"),\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n \t\t\t\t/* fallthru */\n \t\t\tcase SIGN_VERBATIM:\n \t\t\t\tbreak;\n+\n+\t\t\t/* Stripping modes */\n \t\t\tcase SIGN_WARN_STRIP:\n-\t\t\t\twarning(\"stripping signature from tag %s\",\n+\t\t\t\twarning(_(\"stripping signature from tag %s\"),\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n \t\t\t\t/* fallthru */\n \t\t\tcase SIGN_STRIP:\n \t\t\t\tmessage_size = sig_offset;\n \t\t\t\tbreak;\n+\n+\t\t\t/* Aborting modes */\n+\t\t\tcase SIGN_ABORT:\n+\t\t\t\tdie(_(\"encountered signed tag %s; use \"\n+\t\t\t\t      \"--signed-tags=<mode> to handle it\"),\n+\t\t\t\t    oid_to_hex(&tag->object.oid));\n+\t\t\tcase SIGN_STRIP_IF_INVALID:\n+\t\t\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t\t\t      \"git fast-export with --signed-tags=<mode>\"));\n+\t\t\tdefault:\n+\t\t\t\tBUG(\"invalid signed_commit_mode value %d\", signed_commit_mode);\n \t\t\t}\n \t}\n \ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 493de57ef6..e2c6894461 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -2772,7 +2772,7 @@ static void add_gpgsig_to_commit(struct strbuf *commit_data,\n {\n \tstruct string_list siglines = STRING_LIST_INIT_NODUP;\n \n-\tif (!sig->hash_algo)\n+\tif (!sig || !sig->hash_algo)\n \t\treturn;\n \n \tstrbuf_addstr(commit_data, header);\n@@ -2827,6 +2827,45 @@ static void finalize_commit_buffer(struct strbuf *new_data,\n \tstrbuf_addbuf(new_data, msg);\n }\n \n+static void handle_strip_if_invalid(struct strbuf *new_data,\n+\t\t\t\t    struct signature_data *sig_sha1,\n+\t\t\t\t    struct signature_data *sig_sha256,\n+\t\t\t\t    struct strbuf *msg)\n+{\n+\tstruct strbuf tmp_buf = STRBUF_INIT;\n+\tstruct signature_check signature_check = { 0 };\n+\tint ret;\n+\n+\t/* Check signature in a temporary commit buffer */\n+\tstrbuf_addbuf(&tmp_buf, new_data);\n+\tfinalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);\n+\tret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);\n+\n+\tif (ret) {\n+\t\tconst char *signer = signature_check.signer ?\n+\t\t\tsignature_check.signer : _(\"unknown\");\n+\t\tconst char *subject;\n+\t\tint subject_len = find_commit_subject(msg->buf, &subject);\n+\n+\t\tif (subject_len > 100)\n+\t\t\twarning(_(\"stripping invalid signature for commit '%.100s...'\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), subject, signer);\n+\t\telse if (subject_len > 0)\n+\t\t\twarning(_(\"stripping invalid signature for commit '%.*s'\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), subject_len, subject, signer);\n+\t\telse\n+\t\t\twarning(_(\"stripping invalid signature for commit\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), signer);\n+\n+\t\tfinalize_commit_buffer(new_data, NULL, NULL, msg);\n+\t} else {\n+\t\tstrbuf_swap(new_data, &tmp_buf);\n+\t}\n+\n+\tsignature_check_clear(&signature_check);\n+\tstrbuf_release(&tmp_buf);\n+}\n+\n static void parse_new_commit(const char *arg)\n {\n \tstatic struct strbuf msg = STRBUF_INIT;\n@@ -2878,6 +2917,7 @@ static void parse_new_commit(const char *arg)\n \t\t\twarning(_(\"importing a commit signature verbatim\"));\n \t\t\t/* fallthru */\n \t\tcase SIGN_VERBATIM:\n+\t\tcase SIGN_STRIP_IF_INVALID:\n \t\t\timport_one_signature(&sig_sha1, &sig_sha256, v);\n \t\t\tbreak;\n \n@@ -2962,7 +3002,11 @@ static void parse_new_commit(const char *arg)\n \t\t\t\"encoding %s\\n\",\n \t\t\tencoding);\n \n-\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n+\tif (signed_commit_mode == SIGN_STRIP_IF_INVALID &&\n+\t    (sig_sha1.hash_algo || sig_sha256.hash_algo))\n+\t\thandle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);\n+\telse\n+\t\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n \n \tfree(author);\n \tfree(committer);\n@@ -2984,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)\n \tswitch (signed_tag_mode) {\n \n \t/* First, modes that don't change anything */\n-\tcase SIGN_ABORT:\n-\t\tdie(_(\"encountered signed tag; use \"\n-\t\t      \"--signed-tags=<mode> to handle it\"));\n \tcase SIGN_WARN_VERBATIM:\n \t\twarning(_(\"importing a tag signature verbatim for tag '%s'\"), name);\n \t\t/* fallthru */\n@@ -3003,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)\n \t\tstrbuf_setlen(msg, sig_offset);\n \t\tbreak;\n \n-\t/* Third, BUG */\n+\t/* Third, aborting modes */\n+\tcase SIGN_ABORT:\n+\t\tdie(_(\"encountered signed tag; use \"\n+\t\t      \"--signed-tags=<mode> to handle it\"));\n+\tcase SIGN_STRIP_IF_INVALID:\n+\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t      \"git fast-import with --signed-tags=<mode>\"));\n \tdefault:\n \t\tBUG(\"invalid signed_tag_mode value %d from tag '%s'\",\n \t\t    signed_tag_mode, name);\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex d1e88da8c1..fe653b2464 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1146,6 +1146,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)\n \t\t*mode = SIGN_WARN_STRIP;\n \telse if (!strcmp(arg, \"strip\"))\n \t\t*mode = SIGN_STRIP;\n+\telse if (!strcmp(arg, \"strip-if-invalid\"))\n+\t\t*mode = SIGN_STRIP_IF_INVALID;\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 50487aa148..71dde8cb80 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -111,6 +111,7 @@ enum sign_mode {\n \tSIGN_VERBATIM,\n \tSIGN_WARN_STRIP,\n \tSIGN_STRIP,\n+\tSIGN_STRIP_IF_INVALID,\n };\n \n /*\ndiff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh\nindex c2b4271658..db77ace472 100755\n--- a/t/t9305-fast-import-signatures.sh\n+++ b/t/t9305-fast-import-signatures.sh\n@@ -79,7 +79,7 @@ test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-\n \techo B >explicit-sha256/B &&\n \tgit -C explicit-sha256 add B &&\n \ttest_tick &&\n-\tgit -C explicit-sha256 commit -S -m \"signed\" B &&\n+\tgit -C explicit-sha256 commit -S -m \"signed commit\" B &&\n \tSHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&\n \n \t# Create the corresponding SHA-1 commit\n@@ -103,4 +103,120 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war\n \ttest_line_count = 2 out\n '\n \n+test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '\n+\tgit fast-export main >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim openpgp-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&\n+\ttest $OPENPGP_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim openpgp-signing >output &&\n+\n+\t# Change the commit message, which invalidates the signature.\n+\t# The commit message length should not change though, otherwise the\n+\t# corresponding `data <length>` command would have to be changed too.\n+\tsed \"s/OpenPGP signed commit/OpenPGP forged commit/\" output >modified &&\n+\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&\n+\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&\n+\ttest $OPENPGP_SIGNING != $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep ! -E \"^gpgsig\" actual &&\n+\ttest_grep \"stripping invalid signature\" log\n+'\n+\n+test_expect_success GPG 'keep valid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\n+\tgit -C new cat-file commit refs/heads/dual-signed >actual &&\n+\ttest_grep -E \"^gpgsig \" actual &&\n+\ttest_grep -E \"^gpgsig-sha256 \" actual &&\n+\ttest_must_be_empty log &&\n+\n+\tIMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&\n+\tif test \"$GIT_DEFAULT_HASH\" = \"sha1\"\n+\tthen\n+\t\ttest $SHA1_B = $IMPORTED\n+\telse\n+\t\ttest $SHA256_B = $IMPORTED\n+\tfi\n+'\n+\n+test_expect_success GPG 'strip both invalid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&\n+\n+\t# Change the commit message, which invalidates the signature.\n+\t# The commit message length should not change though, otherwise the\n+\t# corresponding `data <length>` command would have to be changed too.\n+\tsed \"s/signed commit/forged commit/\" output >modified &&\n+\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&\n+\n+\tgit -C new cat-file commit refs/heads/dual-signed >actual &&\n+\ttest_grep ! -E \"^gpgsig \" actual &&\n+\ttest_grep ! -E \"^gpgsig-sha256 \" actual &&\n+\n+\tIMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&\n+\tif test \"$GIT_DEFAULT_HASH\" = \"sha1\"\n+\tthen\n+\t\ttest $SHA1_B != $IMPORTED\n+\telse\n+\t\ttest $SHA256_B != $IMPORTED\n+\tfi &&\n+\n+\ttest_grep \"stripping invalid signature\" log\n+'\n+\n+test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim x509-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&\n+\ttest $X509_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\ttest_config -C new gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\n+\tgit fast-export --signed-commits=verbatim ssh-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&\n+\ttest $SSH_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n test_done\n-- \n2.52.0.rc0.3.gf264cd25e5\n\n"},{"id":"530250","messageId":"xmqqjz04mtji.fsf@gitster.g","threadId":"64443","inReplyTo":"20251105061918.3688870-1-christian.couder@gmail.com","subject":"Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-05T14:40:49Z","receivedAt":"2025-11-05T14:40:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> The `--signed-commits=<mode>` option in `git fast-import` allows users\n> to decide what should be done when commits with signatures are\n> imported.\n>\n> For tools like `git filter-repo`, it would be useful to be able to\n> strip signatures when they are invalid, so let's add a new\n> 'strip-if-invalid' mode for that purpose.\n\nSorry, but I do not get it.  What is your definition of a signature\nbeing \"invalid\", and what is your assumptions of how accurate a\nvalidity check ought to be?  For example, are you assuming that you\nhave all the necessary public keys, revocation data and accurate\nclock?  Even if you are not changing a single bit in the import,\nsome of your early commits' signatures do not \"validate\" and may\nneed to be stripped, and after that happens, wouldn't signatures of\nall later commits become unusable (i.e, you may be able to verify\nthat the signature on the original commit object may still be valid,\nbut because the commit has to become a child of a rewritten commit,\nin the resulting history the signature would no longer match)?\n"},{"id":"530395","messageId":"CABPp-BFWem8iWFQn0Sq7JhHigm7rZsa81D6r7zbsQSh3+ZH91Q@mail.gmail.com","threadId":"64443","inReplyTo":"xmqqjz04mtji.fsf@gitster.g","subject":"Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-11-08T00:34:48Z","receivedAt":"2025-11-08T00:35:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Nov 5, 2025 at 6:40 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> > The `--signed-commits=<mode>` option in `git fast-import` allows users\n> > to decide what should be done when commits with signatures are\n> > imported.\n> >\n> > For tools like `git filter-repo`, it would be useful to be able to\n> > strip signatures when they are invalid, so let's add a new\n> > 'strip-if-invalid' mode for that purpose.\n>\n> Sorry, but I do not get it.  What is your definition of a signature\n> being \"invalid\", and what is your assumptions of how accurate a\n> validity check ought to be?  For example, are you assuming that you\n> have all the necessary public keys, revocation data and accurate\n> clock?  Even if you are not changing a single bit in the import,\n> some of your early commits' signatures do not \"validate\" and may\n> need to be stripped, and after that happens, wouldn't signatures of\n> all later commits become unusable (i.e, you may be able to verify\n> that the signature on the original commit object may still be valid,\n> but because the commit has to become a child of a rewritten commit,\n> in the resulting history the signature would no longer match)?\n\nGood questions.  Let me step back and perhaps motivate the change a bit:\n\nThere's a fairly significant chunk of `git filter-repo` users who also\nhave git histories with commit or tag signatures in their history.\nThey often want to specify rules for rewriting history which happen to\nonly affect \"recent\" commits.  While they could try to specify commit\nranges corresponding to \"recent\" commits, they worry about getting it\nwrong and want to just automatically rewrite everything, expecting\nolder commit signatures to be untouched (since the modification rules\ndidn't need to modify older commits), and get new commit OIDs starting\nwith the first commit that was modified by one of the rewrite rules.\nUnfortunately, when fast-export exports history, it does so without\nsignatures, and thus they get every commit rewritten, not just the\nrecent history.\n\nChristian's previous series allows us to have fast-export also export\nthe signatures, but then we run into the problem of determining\nwhether those signatures are still valid and what to do if they\naren't.  This series attempts to help us determine if they are valid,\nand implements one choice when they aren't (strip), in addition to one\nthat the previous series implemented (keep-it-anyway), while leaving\nanother (re-sign) for future work.\n\nSo, yeah, I'd presume this mode would have to assume the user had all\nthe necessary public keys in order for fast-import to be able to check\nvalidity.  Perhaps that is a tall order for a small percentage of\nrepos out there, but for them, is there any good alternative?\n\nAs far as signature handling goes:\n  * Since fast-export doesn't know what changes filter-repo may make\nto the stream, it can't know whether the signatures will still be\nvalid\n  * Since filter-repo doesn't know what history canonicalizations\nfast-export performed (and it performs a few), it can't know whether\nthe signatures will still be valid\n  * Therefore, fast-import is the only process in the pipeline that\ncan know whether a specified signature remains valid\n\nI guess one alternative would be having fast-export include for any\nsigned commit, what that signed commit's OID would have been had it\nbeen unsigned.  That would allow fast-import to check what the commit\nOID would be without the signature, and if it matches, then just keep\nthe signature without checking whether it's actually valid.  It'd be a\nchange to the fast-export & fast-import format to get such an extra\npiece of data, but perhaps that would be a preferable strategy?  It's\nthe only alternative I can think of to what Christian is doing here;\nam I missing others?\n"},{"id":"530404","messageId":"xmqqjz00e5ns.fsf@gitster.g","threadId":"64443","inReplyTo":"20251105061918.3688870-4-christian.couder@gmail.com","subject":"Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-08T18:32:55Z","receivedAt":"2025-11-08T18:32:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-\n>  6 files changed, 226 insertions(+), 28 deletions(-)\n\nUnfortunately all these tests that assume that explicit-sha256\nrepository as a subdirectory exists would fail when the topic is\nmerged to 'seen' and the tree is built without the optional Rust\nsupport.  This is because brian's f6581e23 (repository: require Rust\nsupport for interoperability, 2025-10-27) changes a couple of tests\nto require RUST prerequisite.  One of them is what creates the\nexplicit-sha256 repository.\n\nI do not think this topic to preserve or strip GPG signatures\nparticularly cares about the dual hash interoperability, so can you\nrearrange the tests in this series to avoid crashing with the other\ntopic?\n\nThanks.\n"},{"id":"530564","messageId":"CAP8UFD1YqadtkYriePJKUBjzhXAyYjNEk-9rj55ZxbGLRAOd2g@mail.gmail.com","threadId":"64443","inReplyTo":"xmqqjz04mtji.fsf@gitster.g","subject":"Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-12T07:19:49Z","receivedAt":"2025-11-12T07:20:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Nov 5, 2025 at 3:40 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> > The `--signed-commits=<mode>` option in `git fast-import` allows users\n> > to decide what should be done when commits with signatures are\n> > imported.\n> >\n> > For tools like `git filter-repo`, it would be useful to be able to\n> > strip signatures when they are invalid, so let's add a new\n> > 'strip-if-invalid' mode for that purpose.\n>\n> Sorry, but I do not get it.  What is your definition of a signature\n> being \"invalid\", and what is your assumptions of how accurate a\n> validity check ought to be?\n\nThe definition of \"valid\" is the same as the definition used by `git\nverify-commit`. The description of this command is:\n\n\"Validates the GPG signature created by `git commit -S` on the commit\nobjects given on the command line.\"\n\nHere we just also \"validate\" commit signatures in the same way and\nusing the same underlying code. If `git verify-commit` would return 0,\nwe consider the commit signature valid, otherwise we consider it\ninvalid.\n\nI will add such clarification to the documentation of the feature in\nthe v2 I plan to send soon.\n\n> For example, are you assuming that you\n> have all the necessary public keys, revocation data and accurate\n> clock?\n\nYes, we assume all that, like `git verify-commit` assumes it has all that too.\n\nIf we want to be clearer about what is needed to make sure that commit\nsignatures can be properly validated, I think we should start with\nworking on `git verify-commit` and improve its related documentation,\nand perhaps even some of its features. It would be simpler to have all\nthe docs and features about this there, and just refer to that command\n(using for example \"see git-verify-commit(1)\") in other places. Such\n`git verify-commit` improvements could be in a separate patch series\nthough.\n\nOr maybe there is a better place, like perhaps the `git tag`\ndocumentation, or a dedicated gitsignature(7) page, where all the\ninformation about tag and commit signatures could be. Anyway such\nimprovements could also be in a separate series.\n\n> Even if you are not changing a single bit in the import,\n> some of your early commits' signatures do not \"validate\" and may\n> need to be stripped, and after that happens, wouldn't signatures of\n> all later commits become unusable (i.e, you may be able to verify\n> that the signature on the original commit object may still be valid,\n> but because the commit has to become a child of a rewritten commit,\n> in the resulting history the signature would no longer match)?\n\nYes, I agree it could be an optimization to consider all the\nsubsequent signatures invalid after one of them is invalid, but it\nwould require making sure that the commit history that `git\nfast-import` receives is completely linear or that we properly track\ncommit history when it's not not linear. I think it's better to start\nwith a relatively simpler implementation like this one though.\n"},{"id":"530565","messageId":"CAP8UFD10yqwmdDEbkq19ANtgxfG93_Mcw7tK50Ouyu-G7MwGWQ@mail.gmail.com","threadId":"64443","inReplyTo":"CABPp-BFWem8iWFQn0Sq7JhHigm7rZsa81D6r7zbsQSh3+ZH91Q@mail.gmail.com","subject":"Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-12T07:22:46Z","receivedAt":"2025-11-12T07:23:00Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Nov 8, 2025 at 1:35 AM Elijah Newren <newren@gmail.com> wrote:\n\n> Good questions.  Let me step back and perhaps motivate the change a bit:\n>\n> There's a fairly significant chunk of `git filter-repo` users who also\n> have git histories with commit or tag signatures in their history.\n> They often want to specify rules for rewriting history which happen to\n> only affect \"recent\" commits.  While they could try to specify commit\n> ranges corresponding to \"recent\" commits, they worry about getting it\n> wrong and want to just automatically rewrite everything, expecting\n> older commit signatures to be untouched (since the modification rules\n> didn't need to modify older commits), and get new commit OIDs starting\n> with the first commit that was modified by one of the rewrite rules.\n> Unfortunately, when fast-export exports history, it does so without\n> signatures, and thus they get every commit rewritten, not just the\n> recent history.\n>\n> Christian's previous series allows us to have fast-export also export\n> the signatures, but then we run into the problem of determining\n> whether those signatures are still valid and what to do if they\n> aren't.  This series attempts to help us determine if they are valid,\n> and implements one choice when they aren't (strip), in addition to one\n> that the previous series implemented (keep-it-anyway), while leaving\n> another (re-sign) for future work.\n\nThanks for a great description of the context motivating this series.\n\n> So, yeah, I'd presume this mode would have to assume the user had all\n> the necessary public keys in order for fast-import to be able to check\n> validity.  Perhaps that is a tall order for a small percentage of\n> repos out there, but for them, is there any good alternative?\n>\n> As far as signature handling goes:\n>   * Since fast-export doesn't know what changes filter-repo may make\n> to the stream, it can't know whether the signatures will still be\n> valid\n>   * Since filter-repo doesn't know what history canonicalizations\n> fast-export performed (and it performs a few), it can't know whether\n> the signatures will still be valid\n>   * Therefore, fast-import is the only process in the pipeline that\n> can know whether a specified signature remains valid\n\nI agree with this analysis.\n\n> I guess one alternative would be having fast-export include for any\n> signed commit, what that signed commit's OID would have been had it\n> been unsigned.  That would allow fast-import to check what the commit\n> OID would be without the signature, and if it matches, then just keep\n> the signature without checking whether it's actually valid.  It'd be a\n> change to the fast-export & fast-import format to get such an extra\n> piece of data, but perhaps that would be a preferable strategy?\n\nIt would be a different strategy. Perhaps useful for some people, but\nI think it could have drawbacks.\n\nFor example since signatures are not checked at export time, it's\npossible that some invalid signatures at export time would still be\nimported back. Also what if the signature becomes invalid between\nexport and import times because for example some keys are revoked?\n\nIf invalid signatures can actually be imported, then a name like\n'strip-if-invalid' could be deceptive, so such a strategy should\nprobably have a different name.\n\n> It's\n> the only alternative I can think of to what Christian is doing here;\n> am I missing others?\n\nI think what I am implementing is what most people would expect. So I\nthink it's worth implementing even if in some cases another strategy\nmight be better.\n"},{"id":"530566","messageId":"CAP8UFD3G6kn-n1_rXJgcZf1djUE4Ner5xd1YaNr5tz5h8d_Ypw@mail.gmail.com","threadId":"64443","inReplyTo":"xmqqjz00e5ns.fsf@gitster.g","subject":"Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-12T07:25:20Z","receivedAt":"2025-11-12T07:25:33Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Nov 8, 2025 at 7:32 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Christian Couder <christian.couder@gmail.com> writes:\n>\n> >  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-\n> >  6 files changed, 226 insertions(+), 28 deletions(-)\n>\n> Unfortunately all these tests that assume that explicit-sha256\n> repository as a subdirectory exists would fail when the topic is\n> merged to 'seen' and the tree is built without the optional Rust\n> support.  This is because brian's f6581e23 (repository: require Rust\n> support for interoperability, 2025-10-27) changes a couple of tests\n> to require RUST prerequisite.  One of them is what creates the\n> explicit-sha256 repository.\n>\n> I do not think this topic to preserve or strip GPG signatures\n> particularly cares about the dual hash interoperability, so can you\n> rearrange the tests in this series to avoid crashing with the other\n> topic?\n\nI will do that in the v2 I hope I can send soon.\n\nThanks for telling me about this issue.\n"},{"id":"530601","messageId":"xmqqqzu3ry7w.fsf@gitster.g","threadId":"64443","inReplyTo":"CAP8UFD1YqadtkYriePJKUBjzhXAyYjNEk-9rj55ZxbGLRAOd2g@mail.gmail.com","subject":"Re: [PATCH 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-12T16:51:15Z","receivedAt":"2025-11-12T16:51:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n>> Even if you are not changing a single bit in the import,\n>> some of your early commits' signatures do not \"validate\" and may\n>> need to be stripped, and after that happens, wouldn't signatures of\n>> all later commits become unusable (i.e, you may be able to verify\n>> that the signature on the original commit object may still be valid,\n>> but because the commit has to become a child of a rewritten commit,\n>> in the resulting history the signature would no longer match)?\n>\n> Yes, I agree it could be an optimization to consider all the\n> subsequent signatures invalid after one of them is invalid, but it\n> would require making sure that the commit history that `git\n> fast-import` receives is completely linear or that we properly track\n> commit history when it's not not linear. I think it's better to start\n> with a relatively simpler implementation like this one though.\n\nNote that I wasn't suggesting any optimization.\n\nI was saying that the cascading effect would mean strip-if-invalid\nmay have to strip all the later commits of their signatures anyway,\nwhich makes us question the usefulness of the feature.\n\nThe other message from Elijah made it clear what the piece this\nseries implements fits in a bigger picture, so I actually am OK as\nlong as the overall use case fits what he described in that message.\n\nThanks.\n\n"},{"id":"530602","messageId":"xmqqms4rry7f.fsf@gitster.g","threadId":"64443","inReplyTo":"CAP8UFD3G6kn-n1_rXJgcZf1djUE4Ner5xd1YaNr5tz5h8d_Ypw@mail.gmail.com","subject":"Re: [PATCH 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-12T16:51:32Z","receivedAt":"2025-11-12T16:51:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Sat, Nov 8, 2025 at 7:32 PM Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Christian Couder <christian.couder@gmail.com> writes:\n>>\n>> >  t/t9305-fast-import-signatures.sh  | 118 ++++++++++++++++++++++++++++-\n>> >  6 files changed, 226 insertions(+), 28 deletions(-)\n>>\n>> Unfortunately all these tests that assume that explicit-sha256\n>> repository as a subdirectory exists would fail when the topic is\n>> merged to 'seen' and the tree is built without the optional Rust\n>> support.  This is because brian's f6581e23 (repository: require Rust\n>> support for interoperability, 2025-10-27) changes a couple of tests\n>> to require RUST prerequisite.  One of them is what creates the\n>> explicit-sha256 repository.\n>>\n>> I do not think this topic to preserve or strip GPG signatures\n>> particularly cares about the dual hash interoperability, so can you\n>> rearrange the tests in this series to avoid crashing with the other\n>> topic?\n>\n> I will do that in the v2 I hope I can send soon.\n\nThanks.\n"},{"id":"530788","messageId":"20251117043450.322644-1-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251105061918.3688870-1-christian.couder@gmail.com","subject":"[PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-17T04:34:47Z","receivedAt":"2025-11-17T04:35:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Introduction\n============\n\nTools like `git filter-repo` are often used to rewrite recent\nhistory. When there are tags or commit signatures, such signatures\nrelated to the rewritten history would become invalid though. A way to\naddress this issue could be to strip signatures when they have become\ninvalid.\n\nThe `--signed-commits=<mode>` option in `git fast-import` allows users\nto decide what should be done when commits with signatures are\nimported.\n\nSo let's add a new 'strip-if-invalid' <mode> to that option.\n\nMaybe this new mode should become the default mode, but this would be\nbreaking backward compatibility, and perhaps this could be decided\nafter other new modes that might be even better default modes have\nbeen added. So we leave that for future work.\n\nThis 'strip-if-invalid' mode should also be added to\n`--signed-tags=<mode>`, but we leave that for future work too.\n\nChanges since v1\n================\n\nThanks Junio and Elijah for reviewing and commenting on v1.\n\nThere are no code changes in this v2, only commit message,\ndocumentation and test changes:\n\n* Rebased on current 'master'. This avoids the need to mark some\n  strings for translation as a recent series doing that has been\n  recently merged to 'master'.\n\n* In patch 3/3, improved the commit message to better justify the new\n  feature using some sentences from Elijah.\n\n* In patch 3/3, removed tests with dual signatures. This avoids a\n  conflict with a separate series from brian carlson that adds a\n  \"RUST\" prereq that is then needed to run tests with dual signatures.\n\n* In patch 3/3, improved documentation of the new option to say that\n  validation behaves as the validation performed by `git\n  verify-commit`.\n\nCI tests\n========\n\nThey have all passed, see:\n\nhttps://github.com/chriscool/git/actions/runs/19390756104\n\nRange diff vs v1\n================\n\n1:  02ce924afd = 1:  ec2afd95d6 fast-import: refactor finalize_commit_buffer()\n2:  1593adc7b2 = 2:  d22b753817 commit: refactor verify_commit_buffer()\n3:  f264cd25e5 ! 3:  e325533de4 fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>\n    @@ Commit message\n         in the repository history made by the tool between the fast-export\n         and the fast-import steps.\n     \n    +    Note that as far as signature handling goes:\n    +\n    +      * Since fast-export doesn't know what changes filter-repo may make\n    +    to the stream, it can't know whether the signatures will still be\n    +    valid.\n    +\n    +      * Since filter-repo doesn't know what history canonicalizations\n    +    fast-export performed (and it performs a few), it can't know whether\n    +    the signatures will still be valid.\n    +\n    +      * Therefore, fast-import is the only process in the pipeline that\n    +    can know whether a specified signature remains valid.\n    +\n         Having invalid signatures in a rewritten repository could be\n         confusing, so users rewritting history might prefer to simply\n         discard signatures that are invalid at the fast-import step.\n     \n    +    For example a common use case is to rewrite only \"recent\" history.\n    +    While specifying commit ranges corresponding to \"recent\" commits\n    +    could work, users worry about getting it wrong and want to just\n    +    automatically rewrite everything, expecting older commit signatures\n    +    to be untouched.\n    +\n         To let them do that, let's add a new 'strip-if-invalid' mode to the\n         `--signed-commits=<mode>` option of `git fast-import`.\n     \n    @@ Commit message\n         For now let's just die() if 'strip-if-invalid' is passed to these\n         options where it hasn't been implemented yet.\n     \n    -    While at it, let's also mark for translation some error messages\n    -    linked to the `--signed-commits=<mode>` and `--signed-tags=<mode>`\n    -    in `git fast-export`.\n    -\n         [1]: https://github.com/newren/git-filter-repo\n     \n    +    Helped-by: Elijah Newren <newren@gmail.com>\n         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n     \n      ## Documentation/git-fast-import.adoc ##\n    @@ Documentation/git-fast-import.adoc: fast-import stream! This option is enabled a\n     +* `strip` will silently make the commits unsigned.\n     +* `warn-strip` will make them unsigned, but will display a warning.\n     +* `strip-if-invalid` will check signatures and, if they are invalid,\n    -+  will strip them and display a warning.\n    ++  will strip them and display a warning. The validation is performed\n    ++  in the same way as linkgit:git-verify-commit[1] does it.\n      \n      Options for Frontends\n      ~~~~~~~~~~~~~~~~~~~~~\n    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r\n      \tif (signatures.nr) {\n      \t\tswitch (signed_commit_mode) {\n     -\t\tcase SIGN_ABORT:\n    --\t\t\tdie(\"encountered signed commit %s; use \"\n    --\t\t\t    \"--signed-commits=<mode> to handle it\",\n    +-\t\t\tdie(_(\"encountered signed commit %s; use \"\n    +-\t\t\t      \"--signed-commits=<mode> to handle it\"),\n     -\t\t\t    oid_to_hex(&commit->object.oid));\n     +\n     +\t\t/* Exporting modes */\n      \t\tcase SIGN_WARN_VERBATIM:\n    --\t\t\twarning(\"exporting %\"PRIuMAX\" signature(s) for commit %s\",\n    -+\t\t\twarning(_(\"exporting %\"PRIuMAX\" signature(s) for commit %s\"),\n    + \t\t\twarning(_(\"exporting %\"PRIuMAX\" signature(s) for commit %s\"),\n      \t\t\t\t(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));\n    - \t\t\t/* fallthru */\n    - \t\tcase SIGN_VERBATIM:\n     @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct rev_info *rev,\n      \t\t\t\tprint_signature(item->string, item->util);\n      \t\t\t}\n    @@ builtin/fast-export.c: static void handle_commit(struct commit *commit, struct r\n     +\n     +\t\t/* Stripping modes */\n      \t\tcase SIGN_WARN_STRIP:\n    --\t\t\twarning(\"stripping signature(s) from commit %s\",\n    -+\t\t\twarning(_(\"stripping signature(s) from commit %s\"),\n    + \t\t\twarning(_(\"stripping signature(s) from commit %s\"),\n      \t\t\t\toid_to_hex(&commit->object.oid));\n      \t\t\t/* fallthru */\n      \t\tcase SIGN_STRIP:\n    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)\n      \t\tif (sig_offset < message_size)\n      \t\t\tswitch (signed_tag_mode) {\n     -\t\t\tcase SIGN_ABORT:\n    --\t\t\t\tdie(\"encountered signed tag %s; use \"\n    --\t\t\t\t    \"--signed-tags=<mode> to handle it\",\n    +-\t\t\t\tdie(_(\"encountered signed tag %s; use \"\n    +-\t\t\t\t      \"--signed-tags=<mode> to handle it\"),\n     -\t\t\t\t    oid_to_hex(&tag->object.oid));\n     +\n     +\t\t\t/* Exporting modes */\n      \t\t\tcase SIGN_WARN_VERBATIM:\n    --\t\t\t\twarning(\"exporting signed tag %s\",\n    -+\t\t\t\twarning(_(\"exporting signed tag %s\"),\n    + \t\t\t\twarning(_(\"exporting signed tag %s\"),\n      \t\t\t\t\toid_to_hex(&tag->object.oid));\n      \t\t\t\t/* fallthru */\n      \t\t\tcase SIGN_VERBATIM:\n    @@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)\n     +\n     +\t\t\t/* Stripping modes */\n      \t\t\tcase SIGN_WARN_STRIP:\n    --\t\t\t\twarning(\"stripping signature from tag %s\",\n    -+\t\t\t\twarning(_(\"stripping signature from tag %s\"),\n    + \t\t\t\twarning(_(\"stripping signature from tag %s\"),\n      \t\t\t\t\toid_to_hex(&tag->object.oid));\n    - \t\t\t\t/* fallthru */\n    +@@ builtin/fast-export.c: static void handle_tag(const char *name, struct tag *tag)\n      \t\t\tcase SIGN_STRIP:\n      \t\t\t\tmessage_size = sig_offset;\n      \t\t\t\tbreak;\n    @@ t/t9305-fast-import-signatures.sh: test_expect_success GPG 'strip both OpenPGP s\n     +\ttest_grep \"stripping invalid signature\" log\n     +'\n     +\n    -+test_expect_success GPG 'keep valid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '\n    -+\trm -rf new &&\n    -+\tgit init new &&\n    -+\n    -+\tgit -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&\n    -+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n    -+\n    -+\tgit -C new cat-file commit refs/heads/dual-signed >actual &&\n    -+\ttest_grep -E \"^gpgsig \" actual &&\n    -+\ttest_grep -E \"^gpgsig-sha256 \" actual &&\n    -+\ttest_must_be_empty log &&\n    -+\n    -+\tIMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&\n    -+\tif test \"$GIT_DEFAULT_HASH\" = \"sha1\"\n    -+\tthen\n    -+\t\ttest $SHA1_B = $IMPORTED\n    -+\telse\n    -+\t\ttest $SHA256_B = $IMPORTED\n    -+\tfi\n    -+'\n    -+\n    -+test_expect_success GPG 'strip both invalid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '\n    -+\trm -rf new &&\n    -+\tgit init new &&\n    -+\n    -+\tgit -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&\n    -+\n    -+\t# Change the commit message, which invalidates the signature.\n    -+\t# The commit message length should not change though, otherwise the\n    -+\t# corresponding `data <length>` command would have to be changed too.\n    -+\tsed \"s/signed commit/forged commit/\" output >modified &&\n    -+\n    -+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&\n    -+\n    -+\tgit -C new cat-file commit refs/heads/dual-signed >actual &&\n    -+\ttest_grep ! -E \"^gpgsig \" actual &&\n    -+\ttest_grep ! -E \"^gpgsig-sha256 \" actual &&\n    -+\n    -+\tIMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&\n    -+\tif test \"$GIT_DEFAULT_HASH\" = \"sha1\"\n    -+\tthen\n    -+\t\ttest $SHA1_B != $IMPORTED\n    -+\telse\n    -+\t\ttest $SHA256_B != $IMPORTED\n    -+\tfi &&\n    -+\n    -+\ttest_grep \"stripping invalid signature\" log\n    -+'\n    -+\n     +test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '\n     +\trm -rf new &&\n     +\tgit init new &&\n\n\nChristian Couder (3):\n  fast-import: refactor finalize_commit_buffer()\n  commit: refactor verify_commit_buffer()\n  fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>\n\n Documentation/git-fast-import.adoc | 29 ++++++++----\n builtin/fast-export.c              | 38 +++++++++++----\n builtin/fast-import.c              | 74 ++++++++++++++++++++++++++----\n commit.c                           | 17 ++++++-\n commit.h                           |  7 +++\n gpg-interface.c                    |  2 +\n gpg-interface.h                    |  1 +\n t/t9305-fast-import-signatures.sh  | 69 +++++++++++++++++++++++++++-\n 8 files changed, 208 insertions(+), 29 deletions(-)\n\n-- \n2.52.0.rc2.6.g1f299c9613\n\n"},{"id":"530789","messageId":"20251117043450.322644-2-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251117043450.322644-1-christian.couder@gmail.com","subject":"[PATCH v2 1/3] fast-import: refactor finalize_commit_buffer()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-17T04:34:48Z","receivedAt":"2025-11-17T04:35:13Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In a following commit we are going to finalize commit buffers with or\nwithout signatures in order to check the signatures and possibly drop\nthem.\n\nTo do so easily and without duplication, let's refactor the current\ncode that finalizes commit buffers into a new finalize_commit_buffer()\nfunction.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin/fast-import.c | 17 +++++++++++++----\n 1 file changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 7c194e71cb..cb0d2f635e 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -2815,6 +2815,18 @@ static void import_one_signature(struct signature_data *sig_sha1,\n \t\tdie(_(\"parse_one_signature() returned unknown hash algo\"));\n }\n \n+static void finalize_commit_buffer(struct strbuf *new_data,\n+\t\t\t\t   struct signature_data *sig_sha1,\n+\t\t\t\t   struct signature_data *sig_sha256,\n+\t\t\t\t   struct strbuf *msg)\n+{\n+\tadd_gpgsig_to_commit(new_data, \"gpgsig \", sig_sha1);\n+\tadd_gpgsig_to_commit(new_data, \"gpgsig-sha256 \", sig_sha256);\n+\n+\tstrbuf_addch(new_data, '\\n');\n+\tstrbuf_addbuf(new_data, msg);\n+}\n+\n static void parse_new_commit(const char *arg)\n {\n \tstatic struct strbuf msg = STRBUF_INIT;\n@@ -2950,11 +2962,8 @@ static void parse_new_commit(const char *arg)\n \t\t\t\"encoding %s\\n\",\n \t\t\tencoding);\n \n-\tadd_gpgsig_to_commit(&new_data, \"gpgsig \", &sig_sha1);\n-\tadd_gpgsig_to_commit(&new_data, \"gpgsig-sha256 \", &sig_sha256);\n+\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n \n-\tstrbuf_addch(&new_data, '\\n');\n-\tstrbuf_addbuf(&new_data, &msg);\n \tfree(author);\n \tfree(committer);\n \tfree(encoding);\n-- \n2.52.0.rc2.6.g1f299c9613\n\n"},{"id":"530790","messageId":"20251117043450.322644-3-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251117043450.322644-1-christian.couder@gmail.com","subject":"[PATCH v2 2/3] commit: refactor verify_commit_buffer()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-17T04:34:49Z","receivedAt":"2025-11-17T04:35:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"In a following commit, we are going to check commit signatures, but we\nwon't have a commit yet, only a commit buffer, and we are going to\ndiscard this commit buffer if the signature is invalid. So it would be\nwasteful to create a commit that we might discard, just to be able to\ncheck a commit signature.\n\nIt would be simpler instead to be able to check commit signatures\nusing only a commit buffer instead of a commit.\n\nTo be able to do that, let's extract some code from the\ncheck_commit_signature() function into a new verify_commit_buffer()\nfunction, and then let's make check_commit_signature() call\nverify_commit_buffer().\n\nNote that this doesn't fundamentally change how\ncheck_commit_signature() works. It used to call parse_signed_commit()\nwhich calls repo_get_commit_buffer(), parse_buffer_signed_by_header()\nand repo_unuse_commit_buffer(). Now these 3 functions are called\ndirectly by verify_commit_buffer().\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n commit.c | 17 +++++++++++++++--\n commit.h |  7 +++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 16d91b2bfc..709c9eed58 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1315,7 +1315,8 @@ static void handle_signed_tag(const struct commit *parent, struct commit_extra_h\n \tfree(buf);\n }\n \n-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)\n+int verify_commit_buffer(const char *buffer, size_t size,\n+\t\t\t struct signature_check *sigc)\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n@@ -1323,7 +1324,8 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \n \tsigc->result = 'N';\n \n-\tif (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)\n+\tif (parse_buffer_signed_by_header(buffer, size, &payload,\n+\t\t\t\t\t  &signature, the_hash_algo) <= 0)\n \t\tgoto out;\n \n \tsigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;\n@@ -1337,6 +1339,17 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \treturn ret;\n }\n \n+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)\n+{\n+\tunsigned long size;\n+\tconst char *buffer = repo_get_commit_buffer(the_repository, commit, &size);\n+\tint ret = verify_commit_buffer(buffer, size, sigc);\n+\n+\trepo_unuse_commit_buffer(the_repository, commit, buffer);\n+\n+\treturn ret;\n+}\n+\n void verify_merge_signature(struct commit *commit, int verbosity,\n \t\t\t    int check_trust)\n {\ndiff --git a/commit.h b/commit.h\nindex 1d6e0c7518..5406dd2663 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -333,6 +333,13 @@ int remove_signature(struct strbuf *buf);\n  */\n int check_commit_signature(const struct commit *commit, struct signature_check *sigc);\n \n+/*\n+ * Same as check_commit_signature() but accepts a commit buffer and\n+ * its size, instead of a `struct commit *`.\n+ */\n+int verify_commit_buffer(const char *buffer, size_t size,\n+\t\t\t struct signature_check *sigc);\n+\n /* record author-date for each commit object */\n struct author_date_slab;\n void record_author_date(struct author_date_slab *author_date,\n-- \n2.52.0.rc2.6.g1f299c9613\n\n"},{"id":"530791","messageId":"20251117043450.322644-4-christian.couder@gmail.com","threadId":"64443","inReplyTo":"20251117043450.322644-1-christian.couder@gmail.com","subject":"[PATCH v2 3/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-17T04:34:50Z","receivedAt":"2025-11-17T04:35:16Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Tools like `git filter-repo`[1] use `git fast-export` and\n`git fast-import` to rewrite repository history. When rewriting\nhistory using one such tool though, commit signatures might become\ninvalid because the commits they sign changed due to the changes\nin the repository history made by the tool between the fast-export\nand the fast-import steps.\n\nNote that as far as signature handling goes:\n\n  * Since fast-export doesn't know what changes filter-repo may make\nto the stream, it can't know whether the signatures will still be\nvalid.\n\n  * Since filter-repo doesn't know what history canonicalizations\nfast-export performed (and it performs a few), it can't know whether\nthe signatures will still be valid.\n\n  * Therefore, fast-import is the only process in the pipeline that\ncan know whether a specified signature remains valid.\n\nHaving invalid signatures in a rewritten repository could be\nconfusing, so users rewritting history might prefer to simply\ndiscard signatures that are invalid at the fast-import step.\n\nFor example a common use case is to rewrite only \"recent\" history.\nWhile specifying commit ranges corresponding to \"recent\" commits\ncould work, users worry about getting it wrong and want to just\nautomatically rewrite everything, expecting older commit signatures\nto be untouched.\n\nTo let them do that, let's add a new 'strip-if-invalid' mode to the\n`--signed-commits=<mode>` option of `git fast-import`.\n\nIt would be interesting for the `--signed-tags=<mode>` option to\nhave this mode too, but we leave that for a future improvement.\n\nIt might also be possible for `git fast-export` to have such a mode\nin its `--signed-commits=<mode>` and `--signed-tags=<mode>`\noptions, but the use cases for it are much less clear, so we also\nleave that for possible future improvements.\n\nFor now let's just die() if 'strip-if-invalid' is passed to these\noptions where it hasn't been implemented yet.\n\n[1]: https://github.com/newren/git-filter-repo\n\nHelped-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-fast-import.adoc | 29 +++++++++----\n builtin/fast-export.c              | 38 ++++++++++++----\n builtin/fast-import.c              | 59 ++++++++++++++++++++++---\n gpg-interface.c                    |  2 +\n gpg-interface.h                    |  1 +\n t/t9305-fast-import-signatures.sh  | 69 +++++++++++++++++++++++++++++-\n 6 files changed, 174 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc\nindex b74179a6c8..479c4081da 100644\n--- a/Documentation/git-fast-import.adoc\n+++ b/Documentation/git-fast-import.adoc\n@@ -66,15 +66,26 @@ fast-import stream! This option is enabled automatically for\n remote-helpers that use the `import` capability, as they are\n already trusted to run their own code.\n \n---signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n-\tSpecify how to handle signed tags.  Behaves in the same way\n-\tas the same option in linkgit:git-fast-export[1], except that\n-\tdefault is 'verbatim' (instead of 'abort').\n-\n---signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n-\tSpecify how to handle signed commits.  Behaves in the same way\n-\tas the same option in linkgit:git-fast-export[1], except that\n-\tdefault is 'verbatim' (instead of 'abort').\n+`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::\n+\tSpecify how to handle signed tags. Behaves in the same way as\n+\tthe `--signed-commits=<mode>` below, except that the\n+\t`strip-if-invalid` mode is not yet supported. Like for signed\n+\tcommits, the default mode is `verbatim`.\n+\n+`--signed-commits=<mode>`::\n+\tSpecify how to handle signed commits. The following <mode>s\n+\tare supported:\n++\n+* `verbatim`, which is the default, will silently import commit\n+  signatures.\n+* `warn-verbatim` will import them, but will display a warning.\n+* `abort` will make this program die when encountering a signed\n+  commit.\n+* `strip` will silently make the commits unsigned.\n+* `warn-strip` will make them unsigned, but will display a warning.\n+* `strip-if-invalid` will check signatures and, if they are invalid,\n+  will strip them and display a warning. The validation is performed\n+  in the same way as linkgit:git-verify-commit[1] does it.\n \n Options for Frontends\n ~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 0421360ab7..e3fc34b311 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -797,10 +797,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t       (int)(committer_end - committer), committer);\n \tif (signatures.nr) {\n \t\tswitch (signed_commit_mode) {\n-\t\tcase SIGN_ABORT:\n-\t\t\tdie(_(\"encountered signed commit %s; use \"\n-\t\t\t      \"--signed-commits=<mode> to handle it\"),\n-\t\t\t    oid_to_hex(&commit->object.oid));\n+\n+\t\t/* Exporting modes */\n \t\tcase SIGN_WARN_VERBATIM:\n \t\t\twarning(_(\"exporting %\"PRIuMAX\" signature(s) for commit %s\"),\n \t\t\t\t(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));\n@@ -811,12 +809,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t\t\t\tprint_signature(item->string, item->util);\n \t\t\t}\n \t\t\tbreak;\n+\n+\t\t/* Stripping modes */\n \t\tcase SIGN_WARN_STRIP:\n \t\t\twarning(_(\"stripping signature(s) from commit %s\"),\n \t\t\t\toid_to_hex(&commit->object.oid));\n \t\t\t/* fallthru */\n \t\tcase SIGN_STRIP:\n \t\t\tbreak;\n+\n+\t\t/* Aborting modes */\n+\t\tcase SIGN_ABORT:\n+\t\t\tdie(_(\"encountered signed commit %s; use \"\n+\t\t\t      \"--signed-commits=<mode> to handle it\"),\n+\t\t\t    oid_to_hex(&commit->object.oid));\n+\t\tcase SIGN_STRIP_IF_INVALID:\n+\t\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t\t      \"git fast-export with --signed-commits=<mode>\"));\n+\t\tdefault:\n+\t\t\tBUG(\"invalid signed_commit_mode value %d\", signed_commit_mode);\n \t\t}\n \t\tstring_list_clear(&signatures, 0);\n \t}\n@@ -935,16 +946,16 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tsize_t sig_offset = parse_signed_buffer(message, message_size);\n \t\tif (sig_offset < message_size)\n \t\t\tswitch (signed_tag_mode) {\n-\t\t\tcase SIGN_ABORT:\n-\t\t\t\tdie(_(\"encountered signed tag %s; use \"\n-\t\t\t\t      \"--signed-tags=<mode> to handle it\"),\n-\t\t\t\t    oid_to_hex(&tag->object.oid));\n+\n+\t\t\t/* Exporting modes */\n \t\t\tcase SIGN_WARN_VERBATIM:\n \t\t\t\twarning(_(\"exporting signed tag %s\"),\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n \t\t\t\t/* fallthru */\n \t\t\tcase SIGN_VERBATIM:\n \t\t\t\tbreak;\n+\n+\t\t\t/* Stripping modes */\n \t\t\tcase SIGN_WARN_STRIP:\n \t\t\t\twarning(_(\"stripping signature from tag %s\"),\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n@@ -952,6 +963,17 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t\tcase SIGN_STRIP:\n \t\t\t\tmessage_size = sig_offset;\n \t\t\t\tbreak;\n+\n+\t\t\t/* Aborting modes */\n+\t\t\tcase SIGN_ABORT:\n+\t\t\t\tdie(_(\"encountered signed tag %s; use \"\n+\t\t\t\t      \"--signed-tags=<mode> to handle it\"),\n+\t\t\t\t    oid_to_hex(&tag->object.oid));\n+\t\t\tcase SIGN_STRIP_IF_INVALID:\n+\t\t\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t\t\t      \"git fast-export with --signed-tags=<mode>\"));\n+\t\t\tdefault:\n+\t\t\t\tBUG(\"invalid signed_commit_mode value %d\", signed_commit_mode);\n \t\t\t}\n \t}\n \ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex cb0d2f635e..78052d33ed 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -2772,7 +2772,7 @@ static void add_gpgsig_to_commit(struct strbuf *commit_data,\n {\n \tstruct string_list siglines = STRING_LIST_INIT_NODUP;\n \n-\tif (!sig->hash_algo)\n+\tif (!sig || !sig->hash_algo)\n \t\treturn;\n \n \tstrbuf_addstr(commit_data, header);\n@@ -2827,6 +2827,45 @@ static void finalize_commit_buffer(struct strbuf *new_data,\n \tstrbuf_addbuf(new_data, msg);\n }\n \n+static void handle_strip_if_invalid(struct strbuf *new_data,\n+\t\t\t\t    struct signature_data *sig_sha1,\n+\t\t\t\t    struct signature_data *sig_sha256,\n+\t\t\t\t    struct strbuf *msg)\n+{\n+\tstruct strbuf tmp_buf = STRBUF_INIT;\n+\tstruct signature_check signature_check = { 0 };\n+\tint ret;\n+\n+\t/* Check signature in a temporary commit buffer */\n+\tstrbuf_addbuf(&tmp_buf, new_data);\n+\tfinalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);\n+\tret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);\n+\n+\tif (ret) {\n+\t\tconst char *signer = signature_check.signer ?\n+\t\t\tsignature_check.signer : _(\"unknown\");\n+\t\tconst char *subject;\n+\t\tint subject_len = find_commit_subject(msg->buf, &subject);\n+\n+\t\tif (subject_len > 100)\n+\t\t\twarning(_(\"stripping invalid signature for commit '%.100s...'\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), subject, signer);\n+\t\telse if (subject_len > 0)\n+\t\t\twarning(_(\"stripping invalid signature for commit '%.*s'\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), subject_len, subject, signer);\n+\t\telse\n+\t\t\twarning(_(\"stripping invalid signature for commit\\n\"\n+\t\t\t\t  \"  allegedly by %s\"), signer);\n+\n+\t\tfinalize_commit_buffer(new_data, NULL, NULL, msg);\n+\t} else {\n+\t\tstrbuf_swap(new_data, &tmp_buf);\n+\t}\n+\n+\tsignature_check_clear(&signature_check);\n+\tstrbuf_release(&tmp_buf);\n+}\n+\n static void parse_new_commit(const char *arg)\n {\n \tstatic struct strbuf msg = STRBUF_INIT;\n@@ -2878,6 +2917,7 @@ static void parse_new_commit(const char *arg)\n \t\t\twarning(_(\"importing a commit signature verbatim\"));\n \t\t\t/* fallthru */\n \t\tcase SIGN_VERBATIM:\n+\t\tcase SIGN_STRIP_IF_INVALID:\n \t\t\timport_one_signature(&sig_sha1, &sig_sha256, v);\n \t\t\tbreak;\n \n@@ -2962,7 +3002,11 @@ static void parse_new_commit(const char *arg)\n \t\t\t\"encoding %s\\n\",\n \t\t\tencoding);\n \n-\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n+\tif (signed_commit_mode == SIGN_STRIP_IF_INVALID &&\n+\t    (sig_sha1.hash_algo || sig_sha256.hash_algo))\n+\t\thandle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);\n+\telse\n+\t\tfinalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);\n \n \tfree(author);\n \tfree(committer);\n@@ -2984,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)\n \tswitch (signed_tag_mode) {\n \n \t/* First, modes that don't change anything */\n-\tcase SIGN_ABORT:\n-\t\tdie(_(\"encountered signed tag; use \"\n-\t\t      \"--signed-tags=<mode> to handle it\"));\n \tcase SIGN_WARN_VERBATIM:\n \t\twarning(_(\"importing a tag signature verbatim for tag '%s'\"), name);\n \t\t/* fallthru */\n@@ -3003,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)\n \t\tstrbuf_setlen(msg, sig_offset);\n \t\tbreak;\n \n-\t/* Third, BUG */\n+\t/* Third, aborting modes */\n+\tcase SIGN_ABORT:\n+\t\tdie(_(\"encountered signed tag; use \"\n+\t\t      \"--signed-tags=<mode> to handle it\"));\n+\tcase SIGN_STRIP_IF_INVALID:\n+\t\tdie(_(\"'strip-if-invalid' is not a valid mode for \"\n+\t\t      \"git fast-import with --signed-tags=<mode>\"));\n \tdefault:\n \t\tBUG(\"invalid signed_tag_mode value %d from tag '%s'\",\n \t\t    signed_tag_mode, name);\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex f680ed38c0..10853b517d 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -1146,6 +1146,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)\n \t\t*mode = SIGN_WARN_STRIP;\n \telse if (!strcmp(arg, \"strip\"))\n \t\t*mode = SIGN_STRIP;\n+\telse if (!strcmp(arg, \"strip-if-invalid\"))\n+\t\t*mode = SIGN_STRIP_IF_INVALID;\n \telse\n \t\treturn -1;\n \treturn 0;\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex ead1ed6967..789d1ffac4 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -111,6 +111,7 @@ enum sign_mode {\n \tSIGN_VERBATIM,\n \tSIGN_WARN_STRIP,\n \tSIGN_STRIP,\n+\tSIGN_STRIP_IF_INVALID,\n };\n \n /*\ndiff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh\nindex c2b4271658..022dae02e4 100755\n--- a/t/t9305-fast-import-signatures.sh\n+++ b/t/t9305-fast-import-signatures.sh\n@@ -79,7 +79,7 @@ test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-\n \techo B >explicit-sha256/B &&\n \tgit -C explicit-sha256 add B &&\n \ttest_tick &&\n-\tgit -C explicit-sha256 commit -S -m \"signed\" B &&\n+\tgit -C explicit-sha256 commit -S -m \"signed commit\" B &&\n \tSHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&\n \n \t# Create the corresponding SHA-1 commit\n@@ -103,4 +103,71 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war\n \ttest_line_count = 2 out\n '\n \n+test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '\n+\tgit fast-export main >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim openpgp-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&\n+\ttest $OPENPGP_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim openpgp-signing >output &&\n+\n+\t# Change the commit message, which invalidates the signature.\n+\t# The commit message length should not change though, otherwise the\n+\t# corresponding `data <length>` command would have to be changed too.\n+\tsed \"s/OpenPGP signed commit/OpenPGP forged commit/\" output >modified &&\n+\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&\n+\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&\n+\ttest $OPENPGP_SIGNING != $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep ! -E \"^gpgsig\" actual &&\n+\ttest_grep \"stripping invalid signature\" log\n+'\n+\n+test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\tgit fast-export --signed-commits=verbatim x509-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&\n+\ttest $X509_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n+test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '\n+\trm -rf new &&\n+\tgit init new &&\n+\n+\ttest_config -C new gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\n+\tgit fast-export --signed-commits=verbatim ssh-signing >output &&\n+\tgit -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&\n+\tIMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&\n+\ttest $SSH_SIGNING = $IMPORTED &&\n+\tgit -C new cat-file commit \"$IMPORTED\" >actual &&\n+\ttest_grep -E \"^gpgsig(-sha256)? \" actual &&\n+\ttest_must_be_empty log\n+'\n+\n test_done\n-- \n2.52.0.rc2.6.g1f299c9613\n\n"},{"id":"530834","messageId":"CABPp-BHY4SLmWY=V5aHJ6igN0GWeg6V1MoWDwszPe2O38wqBhw@mail.gmail.com","threadId":"64443","inReplyTo":"20251117043450.322644-1-christian.couder@gmail.com","subject":"Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-11-17T19:52:03Z","receivedAt":"2025-11-17T19:52:15Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, Nov 16, 2025 at 8:35 PM Christian Couder\n<christian.couder@gmail.com> wrote:\n> There are no code changes in this v2, only commit message,\n> documentation and test changes:\n>\n> * Rebased on current 'master'. This avoids the need to mark some\n>   strings for translation as a recent series doing that has been\n>   recently merged to 'master'.\n>\n> * In patch 3/3, improved the commit message to better justify the new\n>   feature using some sentences from Elijah.\n>\n> * In patch 3/3, removed tests with dual signatures. This avoids a\n>   conflict with a separate series from brian carlson that adds a\n>   \"RUST\" prereq that is then needed to run tests with dual signatures.\n\nI'm a bit surprised; from\nhttps://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you\nwere going to rearrange the tests to avoid the conflict, not delete\nthem.  Are no tests of this new functionality needed?\n\n> * In patch 3/3, improved documentation of the new option to say that\n>   validation behaves as the validation performed by `git\n>   verify-commit`.\n\nLooking over the range diff, the other changes look good.\n"},{"id":"530915","messageId":"CAP8UFD03YK47nONVRV_wqOEanC8Oth1iRzsFv=eFhbFs6Q5mPA@mail.gmail.com","threadId":"64443","inReplyTo":"CABPp-BHY4SLmWY=V5aHJ6igN0GWeg6V1MoWDwszPe2O38wqBhw@mail.gmail.com","subject":"Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-11-18T18:29:51Z","receivedAt":"2025-11-18T18:30:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Nov 17, 2025 at 8:52 PM Elijah Newren <newren@gmail.com> wrote:\n>\n> On Sun, Nov 16, 2025 at 8:35 PM Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > There are no code changes in this v2, only commit message,\n> > documentation and test changes:\n> >\n> > * Rebased on current 'master'. This avoids the need to mark some\n> >   strings for translation as a recent series doing that has been\n> >   recently merged to 'master'.\n> >\n> > * In patch 3/3, improved the commit message to better justify the new\n> >   feature using some sentences from Elijah.\n> >\n> > * In patch 3/3, removed tests with dual signatures. This avoids a\n> >   conflict with a separate series from brian carlson that adds a\n> >   \"RUST\" prereq that is then needed to run tests with dual signatures.\n>\n> I'm a bit surprised; from\n> https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you\n> were going to rearrange the tests to avoid the conflict, not delete\n> them.  Are no tests of this new functionality needed?\n\nThere are still 5 new tests left in patch 3/3 that are testing the new\n'strip-if-invalid' functionality after I removed the 2 tests that are\nrelated to dual signatures.\n\nIn \"t/t9305-fast-import-signatures.sh\", dual signatures are already\ntested to work with `git fast-import --signed-commits=<mode>` by the\ntests that brian's f6581e23 (repository: require Rust support for\ninteroperability, 2025-10-27) modifies.\n\nf6581e23 not only adds the RUST prereq to these tests, but it also\nintroduces the RUST prereq itself in \"t/test-lib.sh\" with:\n\n+test_lazy_prereq RUST '\n+       test \"$(build_option rust)\" = enabled\n+'\n\nSo it's much simpler to just remove the 2 new dual signature tests\nthat will need the RUST prereq when f6581e23 is merged. We can still\nadd back these 2 new tests after f6581e23 is merged if we think it's\nworth it.\n\nTo avoid the conflict I could introduce the RUST prereq itself in\n\"t/test-lib.sh\" with the same code that f6581e23 uses, but then how do\nI justify it? What happens if f6581e23 is not actually merged?\n\nIt seems to me that if we really want the 2 new dual signature tests\nin this series, we would have to wait until f6581e23 is merged or\ndiscarded.\n\n> > * In patch 3/3, improved documentation of the new option to say that\n> >   validation behaves as the validation performed by `git\n> >   verify-commit`.\n>\n> Looking over the range diff, the other changes look good.\n\nThanks for your review.\n"},{"id":"530916","messageId":"xmqqikf7duze.fsf@gitster.g","threadId":"64443","inReplyTo":"CAP8UFD03YK47nONVRV_wqOEanC8Oth1iRzsFv=eFhbFs6Q5mPA@mail.gmail.com","subject":"Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-18T19:03:01Z","receivedAt":"2025-11-18T19:03:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> In \"t/t9305-fast-import-signatures.sh\", dual signatures are already\n> tested to work with `git fast-import --signed-commits=<mode>` by the\n> tests that brian's f6581e23 (repository: require Rust support for\n> interoperability, 2025-10-27) modifies.\n> ...\n> Thanks for your review.\n\nSounds good.\n\nLet's see how well this round interact with others (I do not\nanticipate any more fallout---knock wood), and then mark the topic\nfor 'next'.\n\nThanks.\n\n\n"},{"id":"530917","messageId":"CABPp-BE=wcFicz0H7n4CQSFmLF0hv1-0tJQuWdsjKi0rWAQHFg@mail.gmail.com","threadId":"64443","inReplyTo":"CAP8UFD03YK47nONVRV_wqOEanC8Oth1iRzsFv=eFhbFs6Q5mPA@mail.gmail.com","subject":"Re: [PATCH v2 0/3] fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2025-11-18T19:04:26Z","receivedAt":"2025-11-18T19:04:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Nov 18, 2025 at 10:30 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> On Mon, Nov 17, 2025 at 8:52 PM Elijah Newren <newren@gmail.com> wrote:\n> >\n> > On Sun, Nov 16, 2025 at 8:35 PM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> > > There are no code changes in this v2, only commit message,\n> > > documentation and test changes:\n> > >\n> > > * Rebased on current 'master'. This avoids the need to mark some\n> > >   strings for translation as a recent series doing that has been\n> > >   recently merged to 'master'.\n> > >\n> > > * In patch 3/3, improved the commit message to better justify the new\n> > >   feature using some sentences from Elijah.\n> > >\n> > > * In patch 3/3, removed tests with dual signatures. This avoids a\n> > >   conflict with a separate series from brian carlson that adds a\n> > >   \"RUST\" prereq that is then needed to run tests with dual signatures.\n> >\n> > I'm a bit surprised; from\n> > https://lore.kernel.org/git/xmqqms4rry7f.fsf@gitster.g/, I thought you\n> > were going to rearrange the tests to avoid the conflict, not delete\n> > them.  Are no tests of this new functionality needed?\n>\n> There are still 5 new tests left in patch 3/3 that are testing the new\n> 'strip-if-invalid' functionality after I removed the 2 tests that are\n> related to dual signatures.\n>\n> In \"t/t9305-fast-import-signatures.sh\", dual signatures are already\n> tested to work with `git fast-import --signed-commits=<mode>` by the\n> tests that brian's f6581e23 (repository: require Rust support for\n> interoperability, 2025-10-27) modifies.\n>\n> f6581e23 not only adds the RUST prereq to these tests, but it also\n> introduces the RUST prereq itself in \"t/test-lib.sh\" with:\n>\n> +test_lazy_prereq RUST '\n> +       test \"$(build_option rust)\" = enabled\n> +'\n>\n> So it's much simpler to just remove the 2 new dual signature tests\n> that will need the RUST prereq when f6581e23 is merged. We can still\n> add back these 2 new tests after f6581e23 is merged if we think it's\n> worth it.\n>\n> To avoid the conflict I could introduce the RUST prereq itself in\n> \"t/test-lib.sh\" with the same code that f6581e23 uses, but then how do\n> I justify it? What happens if f6581e23 is not actually merged?\n>\n> It seems to me that if we really want the 2 new dual signature tests\n> in this series, we would have to wait until f6581e23 is merged or\n> discarded.\n\nOh, right, there were other tests.  Sorry about that, I should have\ndouble checked the patches instead of only looking at the range-diff.\n\n> > > * In patch 3/3, improved documentation of the new option to say that\n> > >   validation behaves as the validation performed by `git\n> > >   verify-commit`.\n> >\n> > Looking over the range diff, the other changes look good.\n>\n> Thanks for your review.\n\nYeah, I think the series is good to advance.\n"}]}