Sync with 2.41.2

* maint-2.41:
  Git 2.41.2
  Git 2.40.3
  Git 2.39.5
  Revert "Add a helper function to compare file contents"
  hooks(clone protections): simplify templates hooks validation
  hooks(clone protections): special-case current Git LFS hooks
  hook(clone protections): add escape hatch
  tests: verify that `clone -c core.hooksPath=/dev/null` works again
  Revert "core.hooksPath: add some protection while cloning"
  init: use the correct path of the templates directory again
  hook: plug a new memory leak
  ci: stop installing "gcc-13" for osx-gcc
  ci: avoid bare "gcc" for osx-gcc job
  ci: drop mention of BREW_INSTALL_PACKAGES variable

Author: dscho

.github/workflows/main.yml

@@ -278,8 +278,7 @@ jobs:
             cc: clang
             pool: macos-13
           - jobname: osx-gcc
-            cc: gcc
-            cc_package: gcc-13
+            cc: gcc-13
             pool: macos-13
           - jobname: linux-gcc-default
             cc: gcc

Documentation/RelNotes/2.39.5.txt

@@ -0,0 +1,26 @@
+Git 2.39.5 Release Notes
+========================
+
+Relative to Git 2.39.5, this release has fixes for regressions that
+were introduced in 2.39.4, most notably the error message shown when
+cloning Git LFS-enabled repositories. It also contains a fix for the
+`osx-gcc` CI job.
+
+Fixes since Git 2.39.4
+----------------------
+
+ * Hooks that are intentionally installed _during_ a clone operation
+   (like Git LFS does in its `smudge` filter) can now be marked as
+   intentional by appending `safe.hook.sha256` values to a protected
+   config.
+
+   The hooks installed by the current Git LFS version (3.5.1) are
+   hard-coded as being safe.
+
+ * The `core.hooksPath` setting is allowed in repository-local
+   configs again; The benefits of making it protected were
+   outweighed by the cost.
+
+ * Fix a memory leak.
+
+ * CI fix.

Documentation/RelNotes/2.40.3.txt

@@ -0,0 +1,6 @@
+Git v2.40.3 Release Notes
+=========================
+
+This release merges up the regression bug fixes in v2.39.5,
+most notably the bug where cloning Git LFS-enabled repositories
+failed; see the release notes for that version for details.

Documentation/RelNotes/2.41.2.txt

@@ -0,0 +1,7 @@
+Git v2.41.2 Release Notes
+=========================
+
+This release merges up the regression bug fixes in v2.39.5 and
+v2.40.3, most notably the bug where cloning Git LFS-enabled
+repositories failed; see the release notes for these versions
+for details.

Documentation/config/safe.txt

@@ -59,3 +59,9 @@ which id the original user has.
 If that is not what you would prefer and want git to only trust
 repositories that are owned by root instead, then you can remove
 the `SUDO_UID` variable from root's environment before invoking git.
+
+safe.hook.sha256::
+	The value is the SHA-256 of hooks that are considered to be safe
+	to run during a clone operation.
++
+Multiple values can be added via `git config --global --add`.

ci/install-dependencies.sh

@@ -34,8 +34,6 @@ macos-*)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
-	test -z "$BREW_INSTALL_PACKAGES" ||
-	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
 	mkdir -p $HOME/bin
 	(

config.c

@@ -1558,19 +1558,8 @@ static int git_default_core_config(const char *var, const char *value,
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
-	if (!strcmp(var, "core.hookspath")) {
-		if (ctx->kvi && ctx->kvi->scope == CONFIG_SCOPE_LOCAL &&
-		    git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0))
-			die(_("active `core.hooksPath` found in the local "
-			      "repository config:\n\t%s\nFor security "
-			      "reasons, this is disallowed by default.\nIf "
-			      "this is intentional and the hook should "
-			      "actually be run, please\nrun the command "
-			      "again with "
-			      "`GIT_CLONE_PROTECTION_ACTIVE=false`"),
-			    value);
+	if (!strcmp(var, "core.hookspath"))
 		return git_config_pathname(&git_hooks_path, var, value);
-	}
 
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);

copy.c

@@ -70,61 +70,3 @@ int copy_file_with_time(const char *dst, const char *src, int mode)
 		return copy_times(dst, src);
 	return status;
 }
-
-static int do_symlinks_match(const char *path1, const char *path2)
-{
-	struct strbuf buf1 = STRBUF_INIT, buf2 = STRBUF_INIT;
-	int ret = 0;
-
-	if (!strbuf_readlink(&buf1, path1, 0) &&
-	    !strbuf_readlink(&buf2, path2, 0))
-		ret = !strcmp(buf1.buf, buf2.buf);
-
-	strbuf_release(&buf1);
-	strbuf_release(&buf2);
-	return ret;
-}
-
-int do_files_match(const char *path1, const char *path2)
-{
-	struct stat st1, st2;
-	int fd1 = -1, fd2 = -1, ret = 1;
-	char buf1[8192], buf2[8192];
-
-	if ((fd1 = open_nofollow(path1, O_RDONLY)) < 0 ||
-	    fstat(fd1, &st1) || !S_ISREG(st1.st_mode)) {
-		if (fd1 < 0 && errno == ELOOP)
-			/* maybe this is a symbolic link? */
-			return do_symlinks_match(path1, path2);
-		ret = 0;
-	} else if ((fd2 = open_nofollow(path2, O_RDONLY)) < 0 ||
-		   fstat(fd2, &st2) || !S_ISREG(st2.st_mode)) {
-		ret = 0;
-	}
-
-	if (ret)
-		/* to match, neither must be executable, or both */
-		ret = !(st1.st_mode & 0111) == !(st2.st_mode & 0111);
-
-	if (ret)
-		ret = st1.st_size == st2.st_size;
-
-	while (ret) {
-		ssize_t len1 = read_in_full(fd1, buf1, sizeof(buf1));
-		ssize_t len2 = read_in_full(fd2, buf2, sizeof(buf2));
-
-		if (len1 < 0 || len2 < 0 || len1 != len2)
-			ret = 0; /* read error or different file size */
-		else if (!len1) /* len2 is also 0; hit EOF on both */
-			break; /* ret is still true */
-		else
-			ret = !memcmp(buf1, buf2, len1);
-	}
-
-	if (fd1 >= 0)
-		close(fd1);
-	if (fd2 >= 0)
-		close(fd2);
-
-	return ret;
-}

copy.h

@@ -7,18 +7,4 @@ int copy_fd(int ifd, int ofd);
 int copy_file(const char *dst, const char *src, int mode);
 int copy_file_with_time(const char *dst, const char *src, int mode);
 
-/*
- * Compare the file mode and contents of two given files.
- *
- * If both files are actually symbolic links, the function returns 1 if the link
- * targets are identical or 0 if they are not.
- *
- * If any of the two files cannot be accessed or in case of read failures, this
- * function returns 0.
- *
- * If the file modes and contents are identical, the function returns 1,
- * otherwise it returns 0.
- */
-int do_files_match(const char *path1, const char *path2);
-
 #endif /* COPY_H */

hook.c

@@ -9,37 +9,96 @@
 #include "strbuf.h"
 #include "environment.h"
 #include "setup.h"
-#include "copy.h"
+#include "hex.h"
+#include "strmap.h"
 
-static int identical_to_template_hook(const char *name, const char *path)
+static struct strset safe_hook_sha256s = STRSET_INIT;
+static int safe_hook_sha256s_initialized;
+
+static int get_sha256_of_file_contents(const char *path, char *sha256)
 {
-	const char *env = getenv("GIT_CLONE_TEMPLATE_DIR");
-	const char *template_dir = get_template_dir(env && *env ? env : NULL);
-	struct strbuf template_path = STRBUF_INIT;
-	int found_template_hook, ret;
+	struct strbuf sb = STRBUF_INIT;
+	int fd;
+	ssize_t res;
 
-	strbuf_addf(&template_path, "%s/hooks/%s", template_dir, name);
-	found_template_hook = access(template_path.buf, X_OK) >= 0;
-#ifdef STRIP_EXTENSION
-	if (!found_template_hook) {
-		strbuf_addstr(&template_path, STRIP_EXTENSION);
-		found_template_hook = access(template_path.buf, X_OK) >= 0;
+	git_hash_ctx ctx;
+	const struct git_hash_algo *algo = &hash_algos[GIT_HASH_SHA256];
+	unsigned char hash[GIT_MAX_RAWSZ];
+
+	if ((fd = open(path, O_RDONLY)) < 0)
+		return -1;
+	res = strbuf_read(&sb, fd, 400);
+	close(fd);
+	if (res < 0)
+		return -1;
+
+	algo->init_fn(&ctx);
+	algo->update_fn(&ctx, sb.buf, sb.len);
+	strbuf_release(&sb);
+	algo->final_fn(hash, &ctx);
+
+	hash_to_hex_algop_r(sha256, hash, algo);
+
+	return 0;
+}
+
+void add_safe_hook(const char *path)
+{
+	char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
+
+	if (!get_sha256_of_file_contents(path, sha256)) {
+		char *p;
+
+		strset_add(&safe_hook_sha256s, sha256);
+
+		/* support multi-process operations e.g. recursive clones */
+		p = xstrfmt("safe.hook.sha256=%s", sha256);
+		git_config_push_parameter(p);
+		free(p);
 	}
-#endif
-	if (!found_template_hook)
+}
+
+static int safe_hook_cb(const char *key, const char *value,
+			const struct config_context *ctx UNUSED, void *d)
+{
+	struct strset *set = d;
+
+	if (value && !strcmp(key, "safe.hook.sha256"))
+		strset_add(set, value);
+
+	return 0;
+}
+
+static int is_hook_safe_during_clone(const char *name, const char *path, char *sha256)
+{
+	if (get_sha256_of_file_contents(path, sha256) < 0)
 		return 0;
 
-	ret = do_files_match(template_path.buf, path);
+	if (!safe_hook_sha256s_initialized) {
+		safe_hook_sha256s_initialized = 1;
 
-	strbuf_release(&template_path);
-	return ret;
+		/* Hard-code known-safe values for Git LFS v3.4.0..v3.5.1 */
+		/* pre-push */
+		strset_add(&safe_hook_sha256s, "df5417b2daa3aa144c19681d1e997df7ebfe144fb7e3e05138bd80ae998008e4");
+		/* post-checkout */
+		strset_add(&safe_hook_sha256s, "791471b4ff472aab844a4fceaa48bbb0a12193616f971e8e940625498b4938a6");
+		/* post-commit */
+		strset_add(&safe_hook_sha256s, "21e961572bb3f43a5f2fbafc1cc764d86046cc2e5f0bbecebfe9684a0b73b664");
+		/* post-merge */
+		strset_add(&safe_hook_sha256s, "75da0da66a803b4b030ad50801ba57062c6196105eb1d2251590d100edb9390b");
+
+		git_protected_config(safe_hook_cb, &safe_hook_sha256s);
+	}
+
+	return strset_contains(&safe_hook_sha256s, sha256);
 }
 
 const char *find_hook(const char *name)
 {
 	static struct strbuf path = STRBUF_INIT;
 
 	int found_hook;
+	char sha256[GIT_SHA256_HEXSZ + 1] = { '\0' };
 
 	strbuf_reset(&path);
 	strbuf_git_path(&path, "hooks/%s", name);
@@ -71,13 +130,13 @@ const char *find_hook(const char *name)
 		return NULL;
 	}
 	if (!git_hooks_path && git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0) &&
-	    !identical_to_template_hook(name, path.buf))
+	    !is_hook_safe_during_clone(name, path.buf, sha256))
 		die(_("active `%s` hook found during `git clone`:\n\t%s\n"
 		      "For security reasons, this is disallowed by default.\n"
-		      "If this is intentional and the hook should actually "
-		      "be run, please\nrun the command again with "
-		      "`GIT_CLONE_PROTECTION_ACTIVE=false`"),
-		    name, path.buf);
+		      "If this is intentional and the hook is safe to run, "
+		      "please run the following command and try again:\n\n"
+		      "  git config --global --add safe.hook.sha256 %s"),
+		    name, path.buf, sha256);
 	return path.buf;
 }
 

hook.h

@@ -87,4 +87,14 @@ int run_hooks(const char *hook_name);
  * hook. This function behaves like the old run_hook_le() API.
  */
 int run_hooks_l(const char *hook_name, ...);
+
+/**
+ * Mark the contents of the provided path as safe to run during a clone
+ * operation.
+ *
+ * This function is mainly used when copying templates to mark the
+ * just-copied hooks as benign.
+ */
+void add_safe_hook(const char *path);
+
 #endif

setup.c

@@ -18,6 +18,8 @@
 #include "trace2.h"
 #include "worktree.h"
 #include "exec-cmd.h"
+#include "run-command.h"
+#include "hook.h"
 
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
@@ -1793,6 +1795,7 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	size_t path_baselen = path->len;
 	size_t template_baselen = template_path->len;
 	struct dirent *de;
+	int is_hooks_dir = ends_with(template_path->buf, "/hooks/");
 
 	/* Note: if ".git/hooks" file exists in the repository being
 	 * re-initialized, /etc/core-git/templates/hooks/update would
@@ -1845,6 +1848,10 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 			strbuf_release(&lnk);
 		}
 		else if (S_ISREG(st_template.st_mode)) {
+			if (is_hooks_dir &&
+			    is_executable(template_path->buf))
+				add_safe_hook(template_path->buf);
+
 			if (copy_file(path->buf, template_path->buf, st_template.st_mode))
 				die_errno(_("cannot copy '%s' to '%s'"),
 					  template_path->buf, path->buf);

t/helper/test-path-utils.c

@@ -501,16 +501,6 @@ int cmd__path_utils(int argc, const char **argv)
 		return !!res;
 	}
 
-	if (argc == 4 && !strcmp(argv[1], "do_files_match")) {
-		int ret = do_files_match(argv[2], argv[3]);
-
-		if (ret)
-			printf("equal\n");
-		else
-			printf("different\n");
-		return !ret;
-	}
-
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;