From c1ef4a74812baacb21fea597861113e017f02509 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 14 Feb 2023 14:00:36 -0400 Subject: [PATCH] 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 | 2 +- CHANGELOG | 3 +++ Git/Config.hs | 31 ++++++++++++++++++------ Git/Construct.hs | 16 ++++++------ Git/CurrentRepo.hs | 2 +- Remote/Git.hs | 2 +- doc/bugs/bare_remote_safe_directory.mdwn | 13 +--------- 7 files changed, 39 insertions(+), 30 deletions(-) diff --git a/Assistant/MakeRepo.hs b/Assistant/MakeRepo.hs index bad4951b1d..06a0a659d0 100644 --- 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 4a520d5459..ef5c220fdb 100644 --- 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 Mon, 06 Feb 2023 13:39:18 -0400 diff --git a/Git/Config.hs b/Git/Config.hs index e788a2da55..7e12a568e0 100644 --- 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 89b1e1fafe..f82a3e91a1 100644 --- a/Git/Construct.hs +++ b/Git/Construct.hs @@ -1,6 +1,6 @@ {- Construction of Git Repo objects - - - Copyright 2010-2021 Joey Hess + - Copyright 2010-2023 Joey Hess - - 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 3b607d7bab..54e05f4ac5 100644 --- 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 34fb902c56..d42b0fa396 100644 --- 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 e21eca86a6..b05c490ec4 100644 --- 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]] -- 2.30.2