Avoid Git.Config.updateLocation adding "/.git" to the end of the repo

path to a bare repo when git config is not allowed to list the configs
due to the CVE-2022-24765 fix.

That resulted in a confusing error message, and prevented the nice
message that explains how to mark the repo as safe to use.

Made isBare a tristate so that the case where core.bare is not returned can
be handled.

The handling in updateLocation is to check if the directory
contains config and objects and if so assume it's bare.
Note that if that heuristic is somehow wrong, it would construct a repo
that thinks it's bare but is not. That could cause follow-on problems,
but since git-annex then checks checkRepoConfigInaccessible, and skips
using the repo anyway, a wrong guess should not be a problem.

Sponsored-by: Luke Shumaker on Patreon

diff --git a/Assistant/MakeRepo.hs b/Assistant/MakeRepo.hs
index bad4951b1df29dd72f60eb709ac5a3f82b04094c..06a0a659d01da5042344a5f2d1760e1e3b761263 100644 (file)
--- a/Assistant/MakeRepo.hs
+++ b/Assistant/MakeRepo.hs
@@ -57,7 +57,7 @@ initRepo True primary_assistant_repo dir desc mgroup = inDir dir $ do
        initRepo' desc mgroup
        {- Initialize the master branch, so things that expect
         - to have it will work, before any files are added. -}
-       unlessM (Git.Config.isBare <$> gitRepo) $ do
+       unlessM (fromMaybe False . Git.Config.isBare <$> gitRepo) $ do
                cmode <- annexCommitMode <$> Annex.getGitConfig
                void $ inRepo $ Git.Branch.commitCommand cmode
                        (Git.Branch.CommitQuiet True)

diff --git a/CHANGELOG b/CHANGELOG
index 4a520d545963a3e94deda6838c352969cd26dc67..ef5c220fdb36cc1d36529cf71da2af6c186d0836 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -24,6 +24,9 @@ git-annex (10.20230127) UNRELEASED; urgency=medium
   * info, enableremotemote, renameremote: Avoid a confusing message when more
     than one repository matches the user provided name.
   * info: Exit nonzero when the input is not supported.
+  * Fix more breakage caused by git's fix for CVE-2022-24765, this time
+    involving a remote that is a local bare repository not owned by the
+    current user.
 
  -- Joey Hess <id@joeyh.name>  Mon, 06 Feb 2023 13:39:18 -0400
 

diff --git a/Git/Config.hs b/Git/Config.hs
index e788a2da5509f9bcf004ed8f3d41aceb07a34d52..7e12a568e0bea6438ea75c35892b957fb0474935 100644 (file)
--- a/Git/Config.hs
+++ b/Git/Config.hs
@@ -133,14 +133,28 @@ store' k v repo = repo
  - based on the core.bare and core.worktree settings.
  -}
 updateLocation :: Repo -> IO Repo
-updateLocation r@(Repo { location = LocalUnknown d })
-       | isBare r = ifM (doesDirectoryExist (fromRawFilePath dotgit))
-                       ( updateLocation' r $ Local dotgit Nothing
-                       , updateLocation' r $ Local d Nothing
-                       )
-       | otherwise = updateLocation' r $ Local dotgit (Just d)
+updateLocation r@(Repo { location = LocalUnknown d }) = case isBare r of
+       Just True -> ifM (doesDirectoryExist (fromRawFilePath dotgit))
+               ( updateLocation' r $ Local dotgit Nothing
+               , updateLocation' r $ Local d Nothing
+               )
+       Just False -> mknonbare
+       {- core.bare not in config, probably because safe.directory
+        - did not allow reading the config -}
+       Nothing -> ifM (Git.Construct.isBareRepo (fromRawFilePath d))
+               ( mkbare
+               , mknonbare
+               )
   where
        dotgit = d P.</> ".git"
+       -- git treats eg ~/foo as a bare git repository located in
+       -- ~/foo/.git if ~/foo/.git/config has core.bare=true
+       mkbare = ifM (doesDirectoryExist (fromRawFilePath dotgit))
+               ( updateLocation' r $ Local dotgit Nothing
+               , updateLocation' r $ Local d Nothing
+               )
+       mknonbare = updateLocation' r $ Local dotgit (Just d)
+
 updateLocation r@(Repo { location = l@(Local {}) }) = updateLocation' r l
 updateLocation r = return r
 
@@ -212,8 +226,9 @@ boolConfig' :: Bool -> S.ByteString
 boolConfig' True = "true"
 boolConfig' False = "false"
 
-isBare :: Repo -> Bool
-isBare r = fromMaybe False $ isTrueFalse' =<< getMaybe coreBare r
+{- Note that repoIsLocalBare is often better to use than this. -}
+isBare :: Repo -> Maybe Bool
+isBare r = isTrueFalse' =<< getMaybe coreBare r
 
 coreBare :: ConfigKey
 coreBare = "core.bare"

diff --git a/Git/Construct.hs b/Git/Construct.hs
index 89b1e1fafed1ecddacde1edf7a3a7b6358805d81..f82a3e91a14160446db9e282fc7180e48b0f8f83 100644 (file)
--- a/Git/Construct.hs
+++ b/Git/Construct.hs
@@ -1,6 +1,6 @@
 {- Construction of Git Repo objects
  -
- - Copyright 2010-2021 Joey Hess <id@joeyh.name>
+ - Copyright 2010-2023 Joey Hess <id@joeyh.name>
  -
  - Licensed under the GNU AGPL version 3 or higher.
  -}
@@ -23,6 +23,7 @@ module Git.Construct (
        checkForRepo,
        newFrom,
        adjustGitDirFile,
+       isBareRepo,
 ) where
 
 #ifndef mingw32_HOST_OS
@@ -216,7 +217,7 @@ checkForRepo :: FilePath -> IO (Maybe RepoLocation)
 checkForRepo dir = 
        check isRepo $
                check (checkGitDirFile (toRawFilePath dir)) $
-                       check isBareRepo $
+                       check (checkdir (isBareRepo dir)) $
                                return Nothing
   where
        check test cont = maybe cont (return . Just) =<< test
@@ -225,16 +226,17 @@ checkForRepo dir =
                , return Nothing
                )
        isRepo = checkdir $ 
-               gitSignature (".git" </> "config")
+               doesFileExist (dir </> ".git" </> "config")
                        <||>
                -- A git-worktree lacks .git/config, but has .git/gitdir.
                -- (Normally the .git is a file, not a symlink, but it can
                -- be converted to a symlink and git will still work;
                -- this handles that case.)
-               gitSignature (".git" </> "gitdir")
-       isBareRepo = checkdir $ gitSignature "config"
-               <&&> doesDirectoryExist (dir </> "objects")
-       gitSignature file = doesFileExist $ dir </> file
+               doesFileExist (dir </>  ".git" </> "gitdir")
+
+isBareRepo :: FilePath -> IO Bool
+isBareRepo dir = doesFileExist (dir </> "config")
+       <&&> doesDirectoryExist (dir </> "objects")
 
 -- Check for a .git file.
 checkGitDirFile :: RawFilePath -> IO (Maybe RepoLocation)

diff --git a/Git/CurrentRepo.hs b/Git/CurrentRepo.hs
index 3b607d7babc616c543d9a8f8b60b0d59ce1a9e21..54e05f4ac58d1edb23364a0627e7161c01599c73 100644 (file)
--- a/Git/CurrentRepo.hs
+++ b/Git/CurrentRepo.hs
@@ -81,7 +81,7 @@ get = do
                        }
                r <- Git.Config.read $ (newFrom loc)
                        { gitDirSpecifiedExplicitly = True }
-               return $ if Git.Config.isBare r
+               return $ if fromMaybe False (Git.Config.isBare r)
                        then r { location = (location r) { worktree = Nothing } }
                        else r
        configure Nothing Nothing = giveup "Not in a git repository."

diff --git a/Remote/Git.hs b/Remote/Git.hs
index 34fb902c56c33597481b83ebcc5757bc98add8f5..d42b0fa39611c7fc41e593aaa1829ce2aa27d386 100644 (file)
--- a/Remote/Git.hs
+++ b/Remote/Git.hs
@@ -304,7 +304,7 @@ tryGitConfigRead autoinit r hasuuid
                        Right r' -> do
                                -- Cache when http remote is not bare for
                                -- optimisation.
-                               unless (Git.Config.isBare r') $
+                               unless (fromMaybe False $ Git.Config.isBare r') $
                                        setremote setRemoteBare False
                                return r'
                        Left err -> do

diff --git a/doc/bugs/bare_remote_safe_directory.mdwn b/doc/bugs/bare_remote_safe_directory.mdwn
index e21eca86a6ad5a7ce443d2d7fd82e728dc1924f5..b05c490ec4c6561843cf2863c09114b90ff64913 100644 (file)
--- a/doc/bugs/bare_remote_safe_directory.mdwn
+++ b/doc/bugs/bare_remote_safe_directory.mdwn
@@ -12,15 +12,4 @@ This is specific to bare git remotes, for non-bare it
 detects and warns that safe.directory is needed to use the
 remote. --[[Joey]]
 
-> What's causing this is that Git.Config.read is called
-> on the repo, but git refuses to list the repo's config,
-> so updateLocation does not see that the repo is bare
-> when it checks isBare. And so it proceeds to set gitdir
-> to the default non-bare "dir/.git" value.
-> 
-> One way to deal with this would be to make isBare a tristate,
-> since core.bare is not in the listed config at all.
-> 
-> Or, make Git.Construct.fromPath detect when a repo is bare
-> w/o parsing config, and indicate that in the Repo it
-> generates.
+> [[fixed|done]] --[[Joey]]