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

[Justin Tobler <jltobler@gmail.com>]

Greetings,

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

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

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

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

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

Thanks,
-Justin

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

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

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

base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4
-- 
2.53.0.381.g628a66ccf6