From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Brooke Kuhlmann <brooke@alchemists.io>, git@vger.kernel.org
Subject: Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?
Date: Thu, 16 May 2024 07:56:28 -0700 [thread overview]
Message-ID: <xmqq4jaxvm8z.fsf@gitster.g> (raw)
In-Reply-To: <5b3b0882-eb98-558b-3fd1-40cc4cec3ba0@gmx.de> (Johannes Schindelin's message of "Thu, 16 May 2024 14:13:57 +0200 (CEST)")
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I plan on sending out a patch series later either today or tomorrow to
> address a couple of regressions introduced by v2.45.1, and this patch
> would address your specific scenario:
>
> -- snip --
> diff --git a/config.c b/config.c
> index 85b37f2ee09..380f7777a6e 100644
> --- a/config.c
> +++ b/config.c
> @@ -1527,6 +1527,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
>
> if (!strcmp(var, "core.hookspath")) {
> if (current_config_scope() == CONFIG_SCOPE_LOCAL &&
> + (!value || (*value && strcmp(value, "/dev/null"))) &&
> git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0))
> die(_("active `core.hooksPath` found in the local "
> "repository config:\n\t%s\nFor security "
This does not make much sense to me. Why is /dev/null so special,
compared to say /etc/passwd?
I do think the defence-in-depth aspect of the other half of what
went into 2.45.1 and friends, around the "hooks" theme has merit,
i.e. "any activated hooks in the resulting $GIT_DIR/hooks/ directory
that is different from what came from the templates directory is
suspicious". It has a plausible attack scenario to realize such a
suspicious configuration by using directory name munging and other
tricks to confuse "git clone" into thinking what the repository sent
as a part of its payload belongs to $GIT_DIR/. It did have fallout
as the way "git lfs" mucked with user repository's metadata by
abusing the overly wide trust the user gave to its smudge filter [*]
crashed directly with the reasoning behind the "hooks must match
template" protection, which is "Until the clone finishes and gives
control back to the end user, external influence like hooks must not
muck with the contents checked out without user's knowledge and
consent before the user has a chance to inspect the resulting
repository". And it is a reasonable expectation to have when
cloning a repository that has not proven to be trustworthy. So
instead of throwing the protection with bathwater, we should add a
reasonable (i.e. easy to use for "git lfs" developers to follow)
escape hatch that is a bit more nuanced than "rip out the whole
protection" revert or "disable all of the GIT_CLONE_PROTECTION
mechanisms" escape hatch 2.45.1 and friends had.
But I cannot quite tell what the threat model this "core.hookspath"
one is trying to protect against. If some attacker manages to muck
with the configuration file, it is already game over, and they have
better ways than pointing your hookspath to other places to take
advantage of their ability to write to your configuration file to
attack you.
So, my recommendation for this one is to just rip the whole new
logic added in 2.45.1 and friends out of the "core.hookspath"
handling.
[Footnote]
* The user most likely consented only to allow the smudge filter to
transform small token recorded in the object store into a large
blob taken from elsewhere, which means it can read from the
object store and write to the working tree, but the user may not
necessarily agreed to give it full read/write/delete/create
control over anything in $GIT_dir/ or in the configuration files.
next prev parent reply other threads:[~2024-05-16 14:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 15:53 Git 2.45.1 - What is the right way to clone with global hooks disabled? Brooke Kuhlmann
2024-05-15 22:25 ` brian m. carlson
2024-05-16 14:59 ` Junio C Hamano
2024-05-16 12:13 ` Johannes Schindelin
2024-05-16 13:05 ` Brooke Kuhlmann
2024-05-16 14:56 ` Junio C Hamano [this message]
2024-05-17 13:08 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4jaxvm8z.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=brooke@alchemists.io \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).