| From 7db946419c29e185f1cc6e544cfb47b442019ac7 Mon Sep 17 00:00:00 2001 |
| From: Johannes Schindelin <johannes.schindelin@gmx.de> |
| Date: Sat, 18 May 2024 10:32:41 +0000 |
| Subject: Revert "core.hooksPath: add some protection while cloning" |
| |
| commit f13e8e2ea56ceef593311b3cff1ba7ba1a493682 upstream. |
| |
| This defense-in-depth was intended to protect the clone operation |
| against future escalations where bugs in `git clone` would allow |
| attackers to write arbitrary files in the `.git/` directory would allow |
| for Remote Code Execution attacks via maliciously-placed hooks. |
| |
| However, it turns out that the `core.hooksPath` protection has |
| unintentional side effects so severe that they do not justify the |
| benefit of the protections. For example, it has been reported in |
| https://lore.kernel.org/git/FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io/ |
| that the following invocation, which is intended to make `git clone` |
| safer, is itself broken by that protective measure: |
| |
| git clone --config core.hooksPath=/dev/null <url> |
| |
| Since it turns out that the benefit does not justify the cost, let's revert |
| 20f3588efc6 (core.hooksPath: add some protection while cloning, |
| 2024-03-30). |
| |
| Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> |
| Signed-off-by: Junio C Hamano <gitster@pobox.com> |
| Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> |
| --- |
| config.c | 13 +------------ |
| t/t1800-hook.sh | 15 --------------- |
| 2 files changed, 1 insertion(+), 27 deletions(-) |
| |
| diff --git a/config.c b/config.c |
| index 77a0fd2d80e..ae3652b08fa 100644 |
| --- a/config.c |
| +++ b/config.c |
| @@ -1416,19 +1416,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); |
| diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh |
| index 1894ebeb0e8..8b0234cf2d5 100755 |
| --- a/t/t1800-hook.sh |
| +++ b/t/t1800-hook.sh |
| @@ -185,19 +185,4 @@ test_expect_success 'stdin to hooks' ' |
| test_cmp expect actual |
| ' |
| |
| -test_expect_success 'clone protections' ' |
| - test_config core.hooksPath "$(pwd)/my-hooks" && |
| - mkdir -p my-hooks && |
| - write_script my-hooks/test-hook <<-\EOF && |
| - echo Hook ran $1 |
| - EOF |
| - |
| - git hook run test-hook 2>err && |
| - test_grep "Hook ran" err && |
| - test_must_fail env GIT_CLONE_PROTECTION_ACTIVE=true \ |
| - git hook run test-hook 2>err && |
| - test_grep "active .core.hooksPath" err && |
| - test_grep ! "Hook ran" err |
| -' |
| - |
| test_done |
