[PATCH 08/12] Revert "core.hooksPath: add some protection while cloning"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 21, 2024, 19:56 UTC
- Message-ID
- <20240521195659.870714-9-gitster@pobox.com>
- In-Reply-To
- <20240521195659.870714-1-gitster@pobox.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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> --- config.c | 13 +------------ t/t1800-hook.sh | 15 --------------- 2 files changed, 1 insertion(+), 27 deletions(-)
diff --git a/config.c b/config.c index 85b37f2ee0..8c1c4071f0 100644 --- a/config.c +++ b/config.c @@ -1525,19 +1525,8 @@ static int git_default_core_config(const char *var, const char *value, void *cb) if (!strcmp(var, "core.attributesfile")) return git_config_pathname(&git_attributes_file, var, value); - if (!strcmp(var, "core.hookspath")) { - if (current_config_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 7ee12e6f48..2ef3579fa7 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -177,19 +177,4 @@ test_expect_success 'git hook run a hook with a bad shebang' ' 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 && - grep "Hook ran" err && - test_must_fail env GIT_CLONE_PROTECTION_ACTIVE=true \ - git hook run test-hook 2>err && - grep "active .core.hooksPath" err && - ! grep "Hook ran" err -' - test_done
-- 2.45.1-216-g4365c6fcf9