Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-03 Thread Sean Whitton
Hello,

On Tue 03 May 2022 at 12:13PM -04, Joey Hess wrote:

> Fixed with attached patch.

Cool, thanks.  Seems like there might be a release soon so I'll hold off
patching Debian's version?

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-03 Thread Joey Hess
Fixed with attached patch.

-- 
see shy jo
From 43701759a32e38613c61de6dc923c24069f435d6 Mon Sep 17 00:00:00 2001
From: Joey Hess 
Date: Tue, 3 May 2022 12:12:25 -0400
Subject: [PATCH] disable shellescape for rsync 3.2.4

rsync 3.2.4 broke backwards-compatability by preventing exposing filenames
to the shell. Made the rsync and gcrypt special remotes detect this and
disable shellescape.

An alternative fix would have been to always set RSYNC_OLD_ARGS=1.
Which would avoid the overhead of probing rsync --help for each affected
remote. But that is really very fast to run, and it seemed better to switch
to the modern code path rather than keeping on using the bad old code path.

Sponsored-by: Tobias Ammann on Patreon
---
 CHANGELOG  |  3 +++
 Remote/GCrypt.hs   |  3 ++-
 Remote/Rsync.hs| 27 ++-
 doc/special_remotes/rsync.mdwn | 15 ---
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 263da8e6e..6f53e3e1a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -11,6 +11,9 @@ git-annex (10.20220323) UNRELEASED; urgency=medium
   * Fix test failure on NFS when cleaning up gpg temp directory.
   * Fix a build failure with ghc 9.2.2.
 Thanks, gnezdo for the patch.
+  * rsync 3.2.4 broke backwards-compatability by preventing exposing
+filenames to the shell. Made the rsync and gcrypt special remotes
+detect this and disable shellescape. Closes: #1010397
 
  -- Joey Hess   Mon, 28 Mar 2022 14:46:10 -0400
 
diff --git a/Remote/GCrypt.hs b/Remote/GCrypt.hs
index 9662e75d4..0b120806a 100644
--- a/Remote/GCrypt.hs
+++ b/Remote/GCrypt.hs
@@ -130,7 +130,8 @@ gen' r u c gc rs = do
 	cst <- remoteCost gc $
 		if repoCheap r then nearlyCheapRemoteCost else expensiveRemoteCost
 	let (rsynctransport, rsyncurl, accessmethod) = rsyncTransportToObjects r gc
-	let rsyncopts = Remote.Rsync.genRsyncOpts c gc rsynctransport rsyncurl
+	protectsargs <- liftIO Remote.Rsync.probeRsyncProtectsArgs
+	let rsyncopts = Remote.Rsync.genRsyncOpts protectsargs c gc rsynctransport rsyncurl
 	let this = Remote 
 		{ uuid = u
 		, cost = cst
diff --git a/Remote/Rsync.hs b/Remote/Rsync.hs
index 81d60311e..e7e9ff740 100644
--- a/Remote/Rsync.hs
+++ b/Remote/Rsync.hs
@@ -16,7 +16,8 @@ module Remote.Rsync (
 	withRsyncScratchDir,
 	rsyncRemoteConfigs,
 	genRsyncOpts,
-	RsyncOpts
+	RsyncOpts,
+	probeRsyncProtectsArgs,
 ) where
 
 import Annex.Common
@@ -36,6 +37,7 @@ import Remote.Rsync.RsyncUrl
 import Crypto
 import Utility.Rsync
 import Utility.CopyFile
+import Utility.Process.Transcript
 import Messages.Progress
 import Utility.Metered
 import Types.Transfer
@@ -74,7 +76,8 @@ gen r u rc gc rs = do
 	cst <- remoteCost gc expensiveRemoteCost
 	(transport, url) <- rsyncTransport gc $
 		fromMaybe (giveup "missing rsyncurl") $ remoteAnnexRsyncUrl gc
-	let o = genRsyncOpts c gc transport url
+	protectsargs <- liftIO probeRsyncProtectsArgs
+	let o = genRsyncOpts protectsargs c gc transport url
 	let islocal = rsyncUrlIsPath $ rsyncUrl o
 	return $ Just $ specialRemote c
 		(fileStorer $ store o)
@@ -124,6 +127,18 @@ gen r u rc gc rs = do
 			, remoteStateHandle = rs
 			}
 
+-- | Since 3.2.4, rsync protects filenames from being exposed to the shell.
+newtype RsyncProtectsArgs = RsyncProtectsArgs Bool
+
+probeRsyncProtectsArgs :: IO RsyncProtectsArgs
+probeRsyncProtectsArgs = do
+	(helpoutput, _) <- processTranscript "rsync" ["--help"] Nothing
+	-- The --old-args option was added to disable the new arg
+	-- protection, so use it to detect when that feature is supported
+	-- by rsync, rather than parsing versions.
+	return (RsyncProtectsArgs $ "--old-args" `isInfixOf` helpoutput)
+
+
 -- Things used by genRsyncOpts
 rsyncRemoteConfigs :: [RemoteConfigFieldParser]
 rsyncRemoteConfigs = 
@@ -131,15 +146,17 @@ rsyncRemoteConfigs =
 		(FieldDesc "set to no to avoid usual shell escaping (not recommended)")
 	]
 
-genRsyncOpts :: ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts
-genRsyncOpts c gc transport url = RsyncOpts
+genRsyncOpts :: RsyncProtectsArgs -> ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts
+genRsyncOpts (RsyncProtectsArgs protectsargs) c gc transport url = RsyncOpts
 	{ rsyncUrl = url
 	, rsyncOptions = appendtransport $ opts []
 	, rsyncUploadOptions = appendtransport $
 		opts (remoteAnnexRsyncUploadOptions gc)
 	, rsyncDownloadOptions = appendtransport $
 		opts (remoteAnnexRsyncDownloadOptions gc)
-	, rsyncShellEscape = fromMaybe True (getRemoteConfigValue shellEscapeField c)
+	, rsyncShellEscape = if protectsargs
+		then False
+		else fromMaybe True (getRemoteConfigValue shellEscapeField c)
 	}
   where
 	appendtransport l = (++ l) <$> transport
diff --git a/doc/special_remotes/rsync.mdwn b/doc/special_remotes/rsync.mdwn
index 96e452b10..e9f54dc68 100644
--- a/doc/special_remotes/rsync.mdwn
+++ b/doc/special_remotes/rsync.mdwn
@@ -26,13 

Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-03 Thread Joey Hess
> > git-annex enableremote foo shellescape=no

I've confirmed that this workaround works.

Also, the client's version of rsync is what matters. 3.2.3 client and
3.2.4 server does not have the problem.

-- 
see shy jo


signature.asc
Description: PGP signature


Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-02 Thread Joey Hess
Sean Whitton wrote:
> > git-annex can be configured to not do its own shell escaping when accessing
> > a rsync special remote. If your remote is named "foo", you can configure it
> > that way as follows:
> >
> > git-annex enableremote foo shellescape=no
> 
> Thanks for looking into it.  Do you see this as a workaround or what
> people using newer rsync should use going forward?

That's a workaround. If it works, perhaps git-annex could probe the
rsync version and do the same thing automatically. Depends on it 
only the local rsync version matters, not the remote one.

-- 
see shy jo


signature.asc
Description: PGP signature


Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-02 Thread Sean Whitton
Hello,

On Mon 02 May 2022 at 12:51PM -04, Joey Hess wrote:

> Joey Hess wrote:
>> The rsync command that git-annex runs has a trailing close quote, as
>> seen in the first excerpt above. But rsync then complains about the path
>> with that close quote removed.
>>
>> I'm having a hard time not seeing this as a bug in rsync.
>
> # NEWS for rsync 3.2.4 (15 Apr 2022)
>
> ## Changes in this version:
>
> ### BEHAVIOR CHANGES:
>
>  - A new form of arg protection was added that works similarly to the older
>[`--protect-args`](rsync.1#opt) (`-s`) option but in a way that avoids
>breaking things like rrsync (the restricted rsync script): rsync now uses
>backslash escaping for sending "shell-active" characters to the remote
>shell. This includes spaces, so fetching a remote file via a simple quoted
>filename value now works by default without any extra quoting
>
> This change must be the cause of the problem.
>
> git-annex can be configured to not do its own shell escaping when accessing
> a rsync special remote. If your remote is named "foo", you can configure it
> that way as follows:
>
>   git-annex enableremote foo shellescape=no

Thanks for looking into it.  Do you see this as a workaround or what
people using newer rsync should use going forward?

-- 
Sean Whitton



Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-02 Thread Joey Hess
Joey Hess wrote:
> The rsync command that git-annex runs has a trailing close quote, as
> seen in the first excerpt above. But rsync then complains about the path
> with that close quote removed.
> 
> I'm having a hard time not seeing this as a bug in rsync.

# NEWS for rsync 3.2.4 (15 Apr 2022)

## Changes in this version:

### BEHAVIOR CHANGES:

 - A new form of arg protection was added that works similarly to the older
   [`--protect-args`](rsync.1#opt) (`-s`) option but in a way that avoids
   breaking things like rrsync (the restricted rsync script): rsync now uses
   backslash escaping for sending "shell-active" characters to the remote
   shell. This includes spaces, so fetching a remote file via a simple quoted
   filename value now works by default without any extra quoting

This change must be the cause of the problem.

git-annex can be configured to not do its own shell escaping when accessing
a rsync special remote. If your remote is named "foo", you can configure it
that way as follows:

git-annex enableremote foo shellescape=no

-- 
see shy jo


signature.asc
Description: PGP signature


Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1

2022-05-02 Thread Joey Hess
>   [2022-04-30 13:00:20.39881781] (Utility.Process) process [1635096] read: 
> rsync ["-e","'ssh' '-S' '../.git/annex/ssh/user@host-key' '-o' 
> 'ControlMaster=auto' '-o' 'ControlPersist=yes' '-l' 'user' 
> '-T'","--progress","--inplace",
> "user@host-key:/backup/dspace/repositories/repository.git/ff7/84a/'GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851/GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851'"
> ,"../.git/annex/tmp/GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851"]

>   rsync: [sender] change_dir 
> "/backup/dspace/repositories/repository.git/ff7/84a/'GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851"
>  failed: No such file or directory (2)

The rsync command that git-annex runs has a trailing close quote, as
seen in the first excerpt above. But rsync then complains about the path
with that close quote removed.

I'm having a hard time not seeing this as a bug in rsync.

-- 
see shy jo


signature.asc
Description: PGP signature