{"thread":{"id":"60043","subject":"[BUG] Git 2.39.0+ Git.pm ignores Directory=> argument for bare repos","startedAt":"2023-07-30T20:52:00Z","lastAt":"2023-07-31T15:44:27Z","messageCount":2,"participants":["Hank Leininger","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"479986","messageId":"20230730140659.0cc15491-00d2-4e58-af57-aaf5e74b5827@korelogic.com","threadId":"60043","inReplyTo":null,"subject":"[BUG] Git 2.39.0+ Git.pm ignores Directory=> argument for bare repos","fromName":"Hank Leininger","fromEmail":"hlein@korelogic.com","sentAt":"2023-07-30T20:42:44Z","receivedAt":"2023-07-30T20:52:00Z","isPatch":false,"sender":{"key":"hlein@korelogic.com","avatar":null},"body":"Recent git versions (2.39.0 through 2.41.0) Git.pm seems to forget its\nDirectory argument for bare repos. Initial creation of a\nGit->repository object will succeed, but subsequent $repo->command()\nfails unless the repo is in pwd or is set in the GIT_DIR environment\nargument.\n\n  patsy@ultron ~/tmp/repo-test $ git init --bare test1.git\n  Initialized empty Git repository in /home/patsy/tmp/repo-test/test1.git/\n\n  # success with 2.38.4\n  patsy@ultron ~/tmp/repo-test $ git --version\n  git version 2.38.4\n  patsy@ultron ~/tmp/repo-test $ perl -MGit -e 'my $repo = Git->repository(Directory=>\"/home/patsy/tmp/repo-test/test1.git/\")||die \"git open failed: $!\\n\"; $repo->command(\"rev-list\", \"--all\", \"-1\"); print \"rev-list succeeded\\n\"'\n  rev-list succeeded\n\n  # fail with 2.41.0\n  patsy@ultron ~/tmp/repo-test $ git --version\n  git version 2.41.0\n  patsy@ultron ~/tmp/repo-test $ perl -MGit -e 'my $repo =\n  Git->repository(Directory=>\"/home/patsy/tmp/repo-test/test1.git/\")||die \"git open failed: $!\\n\"; $repo->command(\"rev-list\", \"--all\", \"-1\"); print \"rev-list succeeded\\n\"'\n  fatal: not a git repository: '/home/patsy/tmp/repo-test'\n  rev-list --all -1: command returned error: 128\n\n  # success with 2.41.0 after cd\n  patsy@ultron ~/tmp/repo-test $ cd test1.git/\n  patsy@ultron ~/tmp/repo-test/test1.git $ perl -MGit -e 'my $repo = Git->repository(Directory=>\"/home/patsy/tmp/repo-test/test1.git/\")||die \"git open failed: $!\\n\"; $repo->command(\"rev-list\", \"--all\", \"-1\"); print \"rev-list succeeded\\n\"'\n  rev-list succeeded\n\n  # success with 2.41.0 with GIT_DIR= set\n  patsy@ultron ~/tmp/repo-test $\n  GIT_DIR=/home/patsy/tmp/repo-test/test1.git/ perl -MGit -e 'my $repo = Git->repository(Directory=>\"/home/patsy/tmp/repo-test/test1.git/\")||die \"git open failed: $!\\n\"; $repo->command(\"rev-list\", \"--all\", \"-1\"); print \"rev-list succeeded\\n\"'\n  rev-list succeeded\n\nIt seems to be only bare repos that are a problem:\n\n  # success with 2.41.0 for a non-bare repo\n  patsy@ultron ~/tmp/repo-test $ git --version\n  git version 2.41.0\n  patsy@ultron ~/tmp/repo-test $ git init test2\n  Initialized empty Git repository in /home/patsy/tmp/repo-test/test2/.git/\n  patsy@ultron ~/tmp/repo-test $ perl -MGit -e 'my $repo = Git->repository(Directory=>\"/home/patsy/tmp/repo-test/test2\")||die \"git open failed: $!\\n\"; $repo->command(\"rev-list\", \"--all\", \"-1\"); print \"rev-list succeeded\\n\"'\n  rev-list succeeded\n\nWhen stracing both, I can see that Git.pm does an initial execve of\ngit rev-parse --is-bare-repository --git-dir, and it *does* cd prior to\nthat. But running the specified command is done in a new child which\ndoes not chdir first. For non-bare repos, the child executing the\ncommand does chdir.\n\nThanks,\n\n-- \n\nHank Leininger <hlein@korelogic.com>\n9606 3BF9 B593 4CBC E31A  A384 6200 F6E3 781E 3DD7\n"},{"id":"479998","messageId":"xmqqa5vc13xs.fsf@gitster.g","threadId":"60043","inReplyTo":"20230730140659.0cc15491-00d2-4e58-af57-aaf5e74b5827@korelogic.com","subject":"Re: [BUG] Git 2.39.0+ Git.pm ignores Directory=> argument for bare repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-31T15:44:15Z","receivedAt":"2023-07-31T15:44:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hank Leininger <hlein@korelogic.com> writes:\n\n> Recent git versions (2.39.0 through 2.41.0) Git.pm seems to forget its\n> Directory argument for bare repos. Initial creation of a\n> Git->repository object will succeed, but subsequent $repo->command()\n> fails unless the repo is in pwd or is set in the GIT_DIR environment\n> argument.\n\n$ git log --oneline v2.38.0..v2.39.0 -- perl/Git.pm\n20da61f25f Git.pm: trust rev-parse to find bare repositories\n77a1310e6b Git.pm: add semicolon after catch statement\n\nMy guess is 20da61f25f is likely the source of the differences, but\nit is unclear if that should be called a \"bug\", as it was done as a\nfix for misbehaviour.\n\ncommit 20da61f25f8f61a2b581b60f8820ad6116f88e6f\nAuthor: Jeff King <peff@peff.net>\nDate:   Sat Oct 22 18:08:59 2022 -0400\n\n    Git.pm: trust rev-parse to find bare repositories\n    \n    When initializing a repository object, we run \"git rev-parse --git-dir\"\n    to let the C version of Git find the correct directory. But curiously,\n    if this fails we don't automatically say \"not a git repository\".\n    Instead, we do our own pure-perl check to see if we're in a bare\n    repository.\n    \n    This makes little sense, as rev-parse will report both bare and non-bare\n    directories. This logic comes from d5c7721d58 (Git.pm: Add support for\n    subdirectories inside of working copies, 2006-06-24), but I don't see\n    any reason given why we can't just rely on rev-parse. Worse, because we\n    treat any non-error response from rev-parse as a non-bare repository,\n    we'll erroneously set the object's WorkingCopy, even in a bare\n    repository.\n    \n    But it gets worse. Since 8959555cee (setup_git_directory(): add an owner\n    check for the top-level directory, 2022-03-02), it's actively wrong (and\n    dangerous). The perl code doesn't implement the same ownership checks.\n    And worse, after \"finding\" the bare repository, it sets GIT_DIR in the\n    environment, which tells any subsequent Git commands that we've\n    confirmed the directory is OK, and to trust us. I.e., it re-opens the\n    vulnerability plugged by 8959555cee when using Git.pm's repository\n    discovery code.\n    \n    We can fix this by just relying on rev-parse to tell us when we're not\n    in a repository, which fixes the vulnerability. Furthermore, we'll ask\n    its --is-bare-repository function to tell us if we're bare or not, and\n    rely on that.\n"}]}