set safe.directory when getting config for git-annex-shell or git remotes

Fix more breakage caused by git's fix for CVE-2022-24765, this time
involving a remote (either local or ssh) that is a repository not owned by
the current user.

Sponsored-by: Dartmouth College's DANDI project

CHANGELOG

@@ -1,5 +1,8 @@
 git-annex (10.20230829) UNRELEASED; urgency=medium
 
+  * Fix more breakage caused by git's fix for CVE-2022-24765, this time
+    involving a remote (either local or ssh) that is a repository not owned
+    by the current user.
   * Removed the vendored git-lfs and the GitLfs build flag.
   * Fix linker optimisation in linux standalone tarballs.
 

CmdLine/GitAnnexShell.hs

@@ -1,6 +1,6 @@
 {- git-annex-shell main program
  -
- - 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.
  -}
@@ -18,6 +18,7 @@ import CmdLine.GitAnnexShell.Checks
 import CmdLine.GitAnnexShell.Fields
 import Remote.GCrypt (getGCryptUUID)
 import P2P.Protocol (ServerMode(..))
+import Git.Types
 
 import qualified Command.ConfigList
 import qualified Command.NotifyChanges
@@ -123,7 +124,12 @@ builtin cmd dir params = do
 	mkrepo = do
 		r <- Git.Construct.repoAbsPath (toRawFilePath dir)
 			>>= Git.Construct.fromAbsPath
-		Git.Config.read r
+		{- Since the path to the repository was specified
+		 - explicitly, CVE-2022-24765 is not a concern,
+		 - so tell git to treat the repository directory as safe.
+		 -}
+		let r' = r { safeDirectory = True }
+		Git.Config.read r'
 			`catchIO` \_ -> do
 				hn <- fromMaybe "unknown" <$> getHostname
 				giveup $ "failed to read git config of git repository in " ++ hn ++ " on " ++ dir ++ "; perhaps this repository is not set up correctly or has moved"

Git/Config.hs

@@ -72,12 +72,19 @@ read' repo = go repo
 	go _ = assertLocal repo $ error "internal"
 	git_config addparams d = withCreateProcess p (git_config' p)
 	  where
-		params = addparams ++ ["config", "--null", "--list"]
+		params = addparams ++ safedirparam 
+			++ ["config", "--null", "--list"]
 		p = (proc "git" params)
 			{ cwd = Just (fromRawFilePath d)
 			, env = gitEnv repo
 			, std_out = CreatePipe 
 			}
+		safedirparam = if safeDirectory repo 
+			-- Use * rather than d, because git treats
+			-- "dir/" differently than "dir" when comparing for
+			-- safe.directory purposes.
+			then ["-c", "safe.directory=*"]
+			else []
 	git_config' p _ (Just hout) _ pid = 
 		forceSuccessProcess p pid
 			`after`

Git/Construct.hs

@@ -287,5 +287,6 @@ newFrom l = Repo
 	, gitEnvOverridesGitDir = False
 	, gitGlobalOpts = []
 	, gitDirSpecifiedExplicitly = False
+	, safeDirectory = False
 	}
 

Git/Types.hs

@@ -53,6 +53,8 @@ data Repo = Repo
 	, gitGlobalOpts :: [CommandParam]
 	-- True only when --git-dir or GIT_DIR was used
 	, gitDirSpecifiedExplicitly :: Bool
+	-- Set -c safe.directory when using this repository.
+	, safeDirectory :: Bool
 	} deriving (Show, Eq, Ord)
 
 newtype ConfigKey = ConfigKey S.ByteString

Remote/Git.hs

@@ -339,7 +339,12 @@ tryGitConfigRead autoinit r hasuuid
 				warning $ UnquotedString $ "Remote " ++ Git.repoDescribe r ++
 					": "  ++ show e
 			Annex.getState Annex.repo
-		s <- newLocal r
+		{- Since the path to the repository was specified
+		 - explicitly, CVE-2022-24765 is not a concern,
+		 - so tell git to treat the repository directory as safe.
+		 -}
+		let r' = r { Git.safeDirectory = True }
+		s <- newLocal r'
 		liftIO $ Annex.eval s $ check
 			`finally` quiesce True
 		

doc/bugs/enableremote_fails_with___34__wrong_reason__34___stated.mdwn

@@ -42,3 +42,5 @@ so, ideally `git annex enableremote` should provide a similar diagnostic output
 [[!tag projects/dandi]]
 
 ```
+
+> [[fixed|done]] --[[Joey]]

doc/bugs/enableremote_fails_with___34__wrong_reason__34___stated/comment_1_c23de4c4ff536a214c795a805957c9f1._comment

@@ -0,0 +1,24 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2023-09-07T17:01:41Z"
+ content="""
+I wonder if it even makes sense for git-annex-shell to replicate this git
+security check, or would it be better for it to instruct git to trust the
+repository, so it can be used on it?
+
+git's CVE-2022-24765 involves a malicious creation of a .git repository
+above the victim's cwd, with a .git/config that causes things like eg shell
+prompts that run git to execute attacker-controlled commands.
+
+git-annex-shell commands all take the directory that the repository is
+in, and uses that repository. So it doesn't traverse above looking for
+other .git directories.
+
+And, `git clone` will happily clone a remote repsository that's owned 
+by another user, including over ssh. And pull and push etc work with such a
+remote. So git-annex-shell should too.
+
+(For that matter, other git-annex-shell commands do work, it's only the
+command that reads the git config that fails to work.)
+"""]]

doc/bugs/enableremote_fails_with___34__wrong_reason__34___stated/comment_2_c75d3a033b1b8699e512df4641a38548._comment

@@ -0,0 +1,13 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 2"""
+ date="2023-09-07T18:21:30Z"
+ content="""
+Closely related, when a local repo is owned by someone else, cloning it and
+using it as a git-annex remote also fails, at the same config listing
+stage.
+
+I think the same reasoning applies to that, the path to the repo is
+explicitly specified in the remote url, so it should treat it as a safe
+repo for the purposes of listing its config.
+"""]]

doc/bugs/enableremote_fails_with___34__wrong_reason__34___stated/comment_3_39e13618336181485175dddc7185160a._comment

@@ -0,0 +1,8 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 3"""
+ date="2023-09-07T18:32:57Z"
+ content="""
+Basically the same fix works for both the ssh remote and the local
+remote cases.
+"""]]