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

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)

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
 

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 })
+updateLocation r@(Repo { location = LocalUnknown d }) = case isBare r of
-	| isBare r = ifM (doesDirectoryExist (fromRawFilePath dotgit))
+	Just True -> ifM (doesDirectoryExist (fromRawFilePath dotgit))
 		( updateLocation' r $ Local dotgit Nothing
 		, updateLocation' r $ Local d Nothing
 		)
-	| otherwise = updateLocation' r $ Local dotgit (Just d)
+	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
+{- Note that repoIsLocalBare is often better to use than this. -}
-isBare r = fromMaybe False $ isTrueFalse' =<< getMaybe coreBare r
+isBare :: Repo -> Maybe Bool
+isBare r = isTrueFalse' =<< getMaybe coreBare r
 
 coreBare :: ConfigKey
 coreBare = "core.bare"

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")
+		doesFileExist (dir </>  ".git" </> "gitdir")
-	isBareRepo = checkdir $ gitSignature "config"
+
+isBareRepo :: FilePath -> IO Bool
+isBareRepo dir = doesFileExist (dir </> "config")
 	<&&> doesDirectoryExist (dir </> "objects")
-	gitSignature file = doesFileExist $ dir </> file
 
 -- Check for a .git file.
 checkGitDirFile :: RawFilePath -> IO (Maybe RepoLocation)

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."

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

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
+> [[fixed|done]] --[[Joey]]
-> 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.