All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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.com \
    --cc=git@vger.kernel.org \
    --cc=hlein@korelogic.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.