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

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

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

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

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

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

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

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

Thanks Junio and Elijah for reviewing and commenting on v1.

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

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

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

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

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

CI tests ========

They have all passed, see:

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

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

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

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

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