From: Junio C Hamano <gitster@pobox•com>
To: Hank Leininger <hlein@korelogic•com>
Cc: git@vger•kernel.org
Subject: Re: [BUG] Git 2.39.0+ Git.pm ignores Directory=> argument for bare repos
Date: Mon, 31 Jul 2023 08:44:15 -0700 [thread overview]
Message-ID: <xmqqa5vc13xs.fsf@gitster.g> (raw)
In-Reply-To: <20230730140659.0cc15491-00d2-4e58-af57-aaf5e74b5827@korelogic.com> (Hank Leininger's message of "Sun, 30 Jul 2023 14:42:44 -0600")
Hank Leininger <hlein@korelogic•com> writes:
> Recent git versions (2.39.0 through 2.41.0) Git.pm seems to forget its
> Directory argument for bare repos. Initial creation of a
> Git->repository object will succeed, but subsequent $repo->command()
> fails unless the repo is in pwd or is set in the GIT_DIR environment
> argument.
$ git log --oneline v2.38.0..v2.39.0 -- perl/Git.pm
20da61f25f Git.pm: trust rev-parse to find bare repositories
77a1310e6b Git.pm: add semicolon after catch statement
My guess is 20da61f25f is likely the source of the differences, but
it is unclear if that should be called a "bug", as it was done as a
fix for misbehaviour.
commit 20da61f25f8f61a2b581b60f8820ad6116f88e6f
Author: Jeff King <peff@peff•net>
Date: Sat Oct 22 18:08:59 2022 -0400
Git.pm: trust rev-parse to find bare repositories
When initializing a repository object, we run "git rev-parse --git-dir"
to let the C version of Git find the correct directory. But curiously,
if this fails we don't automatically say "not a git repository".
Instead, we do our own pure-perl check to see if we're in a bare
repository.
This makes little sense, as rev-parse will report both bare and non-bare
directories. This logic comes from d5c7721d58 (Git.pm: Add support for
subdirectories inside of working copies, 2006-06-24), but I don't see
any reason given why we can't just rely on rev-parse. Worse, because we
treat any non-error response from rev-parse as a non-bare repository,
we'll erroneously set the object's WorkingCopy, even in a bare
repository.
But it gets worse. Since 8959555cee (setup_git_directory(): add an owner
check for the top-level directory, 2022-03-02), it's actively wrong (and
dangerous). The perl code doesn't implement the same ownership checks.
And worse, after "finding" the bare repository, it sets GIT_DIR in the
environment, which tells any subsequent Git commands that we've
confirmed the directory is OK, and to trust us. I.e., it re-opens the
vulnerability plugged by 8959555cee when using Git.pm's repository
discovery code.
We can fix this by just relying on rev-parse to tell us when we're not
in a repository, which fixes the vulnerability. Furthermore, we'll ask
its --is-bare-repository function to tell us if we're bare or not, and
rely on that.
next prev parent reply other threads:[~2023-07-31 15:44 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-30 20:42 [BUG] Git 2.39.0+ Git.pm ignores Directory=> argument for bare repos Hank Leininger
2023-07-31 15:44 ` Junio C Hamano [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-08-01 8:35 Hank Leininger
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=xmqqa5vc13xs.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=hlein@korelogic$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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