[RFC PATCH 3/4] commit: add --hash=sha256 to sign a tree-sha256 header

[Scott Chacon <scott@gitbutler.net>]

Teach "git commit -S" the same "--hash=sha256" option as "git tag",
which adds the tree-sha256 of the tree being committed as an extra
header after "committer":

  tree <tree>
  parent <parent>
  author <ident>
  committer <ident>
  tree-sha256 <hex>
  gpgsig <signature>

Extra headers are written before the commit is signed, so the
signature covers it, and "git verify-commit" works as before.

When amending, we normally carry over the extra headers of the commit
being amended. Don't do that for tree-sha256, which would be wrong as
soon as the tree changes, and add a new one only if the amended commit
is signed with "--hash=sha256".

---
 Documentation/git-commit.adoc | 11 +++++++-
 builtin/commit.c              | 38 ++++++++++++++++++++++++---
 t/t7032-tree-sha256-signed.sh | 49 +++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc
index 8329c1034b..c027de2adb 100644
--- a/Documentation/git-commit.adoc
+++ b/Documentation/git-commit.adoc
@@ -15,7 +15,7 @@ git commit [-a | --interactive | --patch] [-s] [-v] [-u[<mode>]] [--amend]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	   [(--trailer <token>[(=|:)<value>])...] [-S[<keyid>]]
-	   [--] [<pathspec>...]
+	   [--hash=<algorithm>] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -400,6 +400,15 @@ changes to tracked files.
 	countermand both `commit.gpgSign` configuration variable, and
 	earlier `--gpg-sign`.
 
+`--hash=<algorithm>`::
+	When signing, add a `tree-sha256` header holding a SHA-256
+	digest of every file in the commit's tree, including the
+	contents of checked-out submodules, so that the signature covers
+	the content directly rather than only its SHA-1 object names.
+	_<algorithm>_ is `sha256`, or `none` (the default).
+	Giving `--hash=sha256` without signing is an error. All
+	submodules must be checked out.
+
 `--`::
 	Do not interpret any more arguments as options.
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 840b6b4083..871a2bdcd7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -43,6 +43,7 @@
 #include "commit-graph.h"
 #include "pretty.h"
 #include "trailer.h"
+#include "tree-sha256.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [-a | --interactive | --patch] [-s] [-v] [-u[<mode>]] [--amend]\n"
@@ -52,7 +53,7 @@ static const char * const builtin_commit_usage[] = {
 	   "           [--date=<date>] [--cleanup=<mode>] [--[no-]status]\n"
 	   "           [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]\n"
 	   "           [(--trailer <token>[(=|:)<value>])...] [-S[<keyid>]]\n"
-	   "           [--] [<pathspec>...]"),
+	   "           [--hash=<algorithm>] [--] [<pathspec>...]"),
 	NULL
 };
 
@@ -129,7 +130,8 @@ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static const char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static const char *sign_commit, *pathspec_from_file;
+static const char *sign_commit, *pathspec_from_file, *hash_arg;
+static int tree_hash;
 static struct strvec trailer_args = STRVEC_INIT;
 
 /*
@@ -1737,6 +1739,8 @@ int cmd_commit(int argc,
 			.flags = PARSE_OPT_OPTARG,
 			.defval = (intptr_t) "",
 		},
+		OPT_STRING(0, "hash", &hash_arg, N_("algorithm"),
+			   N_("sign a tree-sha256 header of the committed tree (sha256 or none)")),
 		/* end commit message options */
 
 		OPT_GROUP(N_("Commit contents options")),
@@ -1821,6 +1825,14 @@ int cmd_commit(int argc,
 	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
 					  builtin_commit_usage,
 					  prefix, current_head, &s);
+	if (hash_arg) {
+		tree_hash = parse_signing_hash(hash_arg);
+		if (tree_hash < 0)
+			die(_("unsupported --hash value '%s' (use 'sha256' or 'none')"),
+			    hash_arg);
+		if (tree_hash && !sign_commit)
+			die(_("--hash=%s requires a signed commit (-S)"), hash_arg);
+	}
 	if (trailer_args.nr)
 		trailer_config_init();
 
@@ -1928,13 +1940,31 @@ int cmd_commit(int argc,
 	}
 
 	if (amend) {
-		const char *exclude_gpgsig[3] = { "gpgsig", "gpgsig-sha256", NULL };
-		extra = read_commit_extra_headers(current_head, exclude_gpgsig);
+		const char *exclude[4] = {
+			"gpgsig", "gpgsig-sha256", TREE_SHA256_HEADER, NULL
+		};
+		extra = read_commit_extra_headers(current_head, exclude);
 	} else {
 		struct commit_extra_header **tail = &extra;
 		append_merge_tag_headers(parents, &tail);
 	}
 
+	if (sign_commit && tree_hash) {
+		struct commit_extra_header **tail = &extra;
+		struct strbuf hex = STRBUF_INIT;
+
+		if (tree_sha256_hex(the_repository,
+				    &the_repository->index->cache_tree->oid, &hex)) {
+			rollback_index_files();
+			die(_("unable to compute %s"), TREE_SHA256_HEADER);
+		}
+		while (*tail)
+			tail = &(*tail)->next;
+		CALLOC_ARRAY(*tail, 1);
+		(*tail)->key = xstrdup(TREE_SHA256_HEADER);
+		(*tail)->value = strbuf_detach(&hex, &(*tail)->len);
+	}
+
 	if (commit_tree_extended(sb.buf, sb.len, &the_repository->index->cache_tree->oid,
 				 parents, &oid, author_ident.buf, NULL,
 				 sign_commit, extra)) {
diff --git a/t/t7032-tree-sha256-signed.sh b/t/t7032-tree-sha256-signed.sh
index 083f25e665..44c363b5d2 100755
--- a/t/t7032-tree-sha256-signed.sh
+++ b/t/t7032-tree-sha256-signed.sh
@@ -73,4 +73,53 @@ test_expect_success GPGSSH 'tag --hash=sha256 needs an object with a tree' '
 	test_must_fail git rev-parse --verify v5
 '
 
+test_expect_success GPGSSH 'commit -S --hash=sha256 signs a tree-sha256 header' '
+	test_tick &&
+	git commit --allow-empty -S --hash=sha256 -m signed &&
+	header_of commit HEAD >actual &&
+	test_cmp expect actual &&
+	git verify-commit HEAD
+'
+
+test_expect_success GPGSSH 'commit -S without --hash has no header' '
+	test_tick &&
+	git commit --allow-empty -S -m signed &&
+	header_of commit HEAD >actual &&
+	test_must_be_empty actual &&
+	git commit --allow-empty -S --hash=none -m signed &&
+	header_of commit HEAD >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success GPGSSH 'commit --hash=sha256 requires signing' '
+	git rev-parse HEAD >before &&
+	test_must_fail git commit --allow-empty --hash=sha256 -m unsigned 2>err &&
+	test_grep "requires a signed commit" err &&
+	test_must_fail git commit --allow-empty -S --no-gpg-sign --hash=sha256 \
+		-m unsigned 2>err &&
+	test_grep "requires a signed commit" err &&
+	test_must_fail git commit --allow-empty -S --hash=md5 -m signed 2>err &&
+	test_grep "unsupported --hash value" err &&
+	git rev-parse HEAD >after &&
+	test_cmp before after
+'
+
+test_expect_success GPGSSH 'amending recomputes or drops the header' '
+	git commit --allow-empty -S --hash=sha256 -m signed &&
+	echo changed >dir/file &&
+	git add dir/file &&
+	test_tick &&
+	git commit --amend -S --hash=sha256 -m amended &&
+	test-tool tree-sha256 HEAD >expect-amended &&
+	! test_cmp expect expect-amended &&
+	header_of commit HEAD >actual &&
+	test_cmp expect-amended actual &&
+	git verify-commit HEAD &&
+
+	test_tick &&
+	git commit --amend -m "amended unsigned" &&
+	header_of commit HEAD >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)