diff mbox series

[6/8] hooks(clone protections): special-case current Git LFS hooks

Message ID 98465797e72cf039ace4138ab1e03e4fc7465ea2.1715987756.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Various fixes for v2.45.1 and friends | expand

Commit Message

Johannes Schindelin May 17, 2024, 11:15 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

A notable regression in v2.45.1 and friends (all the way down to
v2.39.4) has been that Git LFS-enabled clones error out with a message
indicating that the `post-checkout` hook has been tampered with while
cloning, and as a safety measure it is not executed.

A generic fix for benign third-party applications wishing to write hooks
during clone operations has been implemented in the parent of this
commit: said applications are expected to add `safe.hook.sha256` values
to a protected config.

However, the current version of Git LFS, v3.5.1, cannot be adapted
retroactively; Therefore, let's just hard-code the SHA-256 values for
this version. That way, Git LFS usage will no longer be broken, and the
next Git LFS version can be taught to add those `safe.hook.sha256`
entries.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 hook.c          | 11 +++++++++++
 t/t1800-hook.sh | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+)

Comments

Junio C Hamano May 18, 2024, 12:20 a.m. UTC | #1 
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> +	/* Hard-code known-safe values for Git LFS v3.4.0..v3.5.1 */
> +	if ((!strcmp("pre-push", name) &&
> +	     !strcmp(sha256, "df5417b2daa3aa144c19681d1e997df7ebfe144fb7e3e05138bd80ae998008e4")) ||
> +	    (!strcmp("post-checkout", name) &&
> +	     !strcmp(sha256, "791471b4ff472aab844a4fceaa48bbb0a12193616f971e8e940625498b4938a6")) ||
> +	    (!strcmp("post-commit", name) &&
> +	     !strcmp(sha256, "21e961572bb3f43a5f2fbafc1cc764d86046cc2e5f0bbecebfe9684a0b73b664")) ||
> +	    (!strcmp("post-merge", name) &&
> +	     !strcmp(sha256, "75da0da66a803b4b030ad50801ba57062c6196105eb1d2251590d100edb9390b")))
> +		return 1;
> +
>  	if (!safe_hook_sha256s_initialized) {
>  		safe_hook_sha256s_initialized = 1;
>  		git_protected_config(safe_hook_cb, &safe_hook_sha256s);


Not that it makes a huge difference, but if I were doing this patch
I would have instead added the special case hashes to the
safe_hook_sha256s hashmap inside this "if we haven't been
initialized" block.  That way, the checking of the hash can be done
at the central place with the same code as used to check other
custom hooks.
diff mbox series

Patch

diff --git a/hook.c b/hook.c
index a2479738451..f810ee133be 100644
--- a/hook.c
+++ b/hook.c
@@ -75,6 +75,17 @@  static int is_hook_safe_during_clone(const char *name, const char *path, char *s
 	if (get_sha256_of_file_contents(path, sha256) < 0)
 		return 0;
 
+	/* Hard-code known-safe values for Git LFS v3.4.0..v3.5.1 */
+	if ((!strcmp("pre-push", name) &&
+	     !strcmp(sha256, "df5417b2daa3aa144c19681d1e997df7ebfe144fb7e3e05138bd80ae998008e4")) ||
+	    (!strcmp("post-checkout", name) &&
+	     !strcmp(sha256, "791471b4ff472aab844a4fceaa48bbb0a12193616f971e8e940625498b4938a6")) ||
+	    (!strcmp("post-commit", name) &&
+	     !strcmp(sha256, "21e961572bb3f43a5f2fbafc1cc764d86046cc2e5f0bbecebfe9684a0b73b664")) ||
+	    (!strcmp("post-merge", name) &&
+	     !strcmp(sha256, "75da0da66a803b4b030ad50801ba57062c6196105eb1d2251590d100edb9390b")))
+		return 1;
+
 	if (!safe_hook_sha256s_initialized) {
 		safe_hook_sha256s_initialized = 1;
 		git_protected_config(safe_hook_cb, &safe_hook_sha256s);
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 0f74c9154d0..af66999aff3 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -192,4 +192,24 @@  test_expect_success '`safe.hook.sha256` and clone protections' '
 	test "called hook" = "$(cat safe-hook/safe-hook.log)"
 '
 
+write_lfs_pre_push_hook () {
+	write_script "$1" <<-\EOF
+	command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting the 'pre-push' file in the hooks directory (set by 'core.hookspath'; usually '.git/hooks').\n"; exit 2; }
+	git lfs pre-push "$@"
+	EOF
+}
+
+test_expect_success 'Git LFS special-handling in clone protections' '
+	git init lfs-hooks &&
+	write_lfs_pre_push_hook lfs-hooks/.git/hooks/pre-push &&
+	write_script git-lfs <<-\EOF &&
+	echo "called $*" >fake-git-lfs.log
+	EOF
+
+	PATH="$PWD:$PATH" GIT_CLONE_PROTECTION_ACTIVE=true \
+		git -C lfs-hooks hook run pre-push &&
+	test_write_lines "called pre-push" >expect &&
+	test_cmp lfs-hooks/fake-git-lfs.log expect
+'
+
 test_done