From 9cbe5068f837d71dbcfd884d5ebfcb105600a85d Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 7 Apr 2017 11:35:20 -0400 Subject: Disable git-annex's support for GIT_SSH and GIT_SSH_COMMAND, unless GIT_ANNEX_USE_GIT_SSH=1 is also set in the environment. This is necessary because as feared, the extra -n parameter that git-annex passes breaks uses of these environment variables that expect exactly the parameters that git passes. For example, see https://github.com/datalad/datalad/issues/1456 It would of course be possible to pre-close stdin before running ssh so not needing the -n, and I think that would not even break ssh's password caching. But it would probably involve a lot of work, possibly would need to deal with some layering violations, and would be error-prone. The really clean fix would be to make all the ssh stuff return a CreateProcess, which could have the handle closed when appropriate, but that would be a large reworing of the code base. This commit was supported by the NSF-funded DataLad project. --- Annex/Ssh.hs | 19 ++++++++++++++----- CHANGELOG | 5 +++++ doc/git-annex.mdwn | 5 ++++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Annex/Ssh.hs b/Annex/Ssh.hs index aa357544d..bf13a02c4 100644 --- a/Annex/Ssh.hs +++ b/Annex/Ssh.hs @@ -52,10 +52,13 @@ data ConsumeStdin = ConsumeStdin | NoConsumeStdin {- Generates a command to ssh to a given host (or user@host) on a given - port. This includes connection caching parameters, and any ssh-options. - - If GIT_SSH or GIT_SSH_COMMAND is set, they are used instead. -} + - If GIT_SSH or GIT_SSH_COMMAND is enabled, they are used instead. -} sshCommand :: ConsumeStdin -> (SshHost, Maybe SshPort) -> RemoteGitConfig -> SshCommand -> Annex (FilePath, [CommandParam]) -sshCommand cs (host, port) gc remotecmd = maybe go return - =<< liftIO (gitSsh' host port remotecmd (consumeStdinParams cs)) +sshCommand cs (host, port) gc remotecmd = ifM (liftIO safe_GIT_SSH) + ( maybe go return + =<< liftIO (gitSsh' host port remotecmd (consumeStdinParams cs)) + , go + ) where go = do ps <- sshOptions cs (host, port) gc [] @@ -81,6 +84,12 @@ sshOptions cs (host, port) gc opts = go =<< sshCachingInfo (host, port) , [Param "-T"] ] +{- Due to passing -n to GIT_SSH and GIT_SSH_COMMAND, some settings + - of those that expect exactly git's parameters will break. So only + - use those if the user set GIT_ANNEX_USE_GIT_SSH to say it's ok. -} +safe_GIT_SSH :: IO Bool +safe_GIT_SSH = (== Just "1") <$> getEnv "GIT_ANNEX_USE_GIT_SSH" + consumeStdinParams :: ConsumeStdin -> [CommandParam] consumeStdinParams ConsumeStdin = [] consumeStdinParams NoConsumeStdin = [Param "-n"] @@ -305,13 +314,13 @@ inRepoWithSshOptionsTo remote gc a = - to set GIT_SSH=git-annex, and set sshOptionsEnv when running git - commands. - - - If GIT_SSH or GIT_SSH_COMMAND are set, this has no effect. -} + - If GIT_SSH or GIT_SSH_COMMAND are enabled, this has no effect. -} sshOptionsTo :: Git.Repo -> RemoteGitConfig -> Git.Repo -> Annex Git.Repo sshOptionsTo remote gc localr | not (Git.repoIsUrl remote) || Git.repoIsHttp remote = unchanged | otherwise = case Git.Url.hostuser remote of Nothing -> unchanged - Just host -> ifM (liftIO gitSshEnvSet) + Just host -> ifM (liftIO $ safe_GIT_SSH <&&> gitSshEnvSet) ( unchanged , do (msockfile, _) <- sshCachingInfo (host, Git.Url.port remote) diff --git a/CHANGELOG b/CHANGELOG index 0a9e56f1b..f36445549 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,11 @@ git-annex (6.20170322) UNRELEASED; urgency=medium * Added remote..annex-push and remote..annex-pull which can be useful to make remotes that don't get fully synced with local changes. + * Disable git-annex's support for GIT_SSH and GIT_SSH_COMMAND, unless + GIT_ANNEX_USE_GIT_SSH=1 is also set in the environment. This is + necessary because as feared, the extra -n parameter that git-annex + passes breaks uses of these environment variables that expect exactly + the parameters that git passes. -- Joey Hess Wed, 29 Mar 2017 12:41:46 -0400 diff --git a/doc/git-annex.mdwn b/doc/git-annex.mdwn index 2bebd9f49..56be4bcf9 100644 --- a/doc/git-annex.mdwn +++ b/doc/git-annex.mdwn @@ -1434,7 +1434,10 @@ These environment variables are used by git-annex when set: Handled similarly to the same as described in git(1). The one difference is that git-annex will sometimes pass an additional "-n" parameter to these, as the first parameter, to prevent ssh from - reading from stdin. + reading from stdin. Since that can break existing uses of these + environment variables that don't expect the extra parameter, you will + need to set `GIT_ANNEX_USE_GIT_SSH=1` to make git-annex support + these. Note that setting either of these environment variables prevents git-annex from automatically enabling ssh connection caching -- cgit v1.2.3