Dear Maintainer,

I found a regression in rrsync, which came with the change from Perl (Bullseye)
to Python (Bookworm), and would like to propose a bug fix.


   * What led up to the situation?

I am using backupninja to create local backups from a remote server. With
Bullseye, I had to use the following line in the configuration file to make it
work with rrsync:
    include = .

backupninja converts this to the command
    rsync <arguments> user@remoteserver:/./ /localbackupdir/
to read the complete subdirectory defined by rrsync (see below) and store it in
the local backup directory.


authorized_keys on the remote server:
    command="/usr/bin/rrsync -ro /path/to/restricted/source/",restrict ssh-rsa
...


Starting with Bookworm, rrsync breaks the backup process with the error "unsafe
arg":
(Output of die('unsafe arg:', orig_arg, [arg, real_arg]) )
    unsafe arg: /./, [/path/to/restricted/source/., /path/to/restricted/source]

Note that arg is not equal to real_arg, since the trailing "/./" of the
original argument (orig_arg) is not completely stripped from arg.


   * What exactly did you do (or not do) that was effective (or ineffective)?

I found that in rrsync, there is a directory argument check that is too
restrictive to work with above (valid) string:

rrsync 3.2.7 (as in Bookworm):
https://salsa.debian.org/debian/rsync/-/blob/debian/3.2.7-1/support/rrsync?ref_type=tags#L309

    ret = [ ]
    for arg in got:
        if args.dir != '/' and arg != '.' and (typ == 3 or (typ == 2 and not
am_sender)):
            arg_has_trailing_slash = arg.endswith('/')
            if arg_has_trailing_slash:
                arg = arg[:-1]
            else:
                arg_has_trailing_slash_dot = arg.endswith('/.')
                if arg_has_trailing_slash_dot:
                    arg = arg[:-2]
            real_arg = os.path.realpath(arg)
            if arg != real_arg and not real_arg.startswith(args.dir_slash):
                die('unsafe arg:', orig_arg, [arg, real_arg])
            if arg_has_trailing_slash:
                arg += '/'
            elif arg_has_trailing_slash_dot:
                arg += '/.'
            if opt == 'arg' and arg.startswith(args.dir_slash):
                arg = arg[args.dir_slash_len:]
                if arg == '':
                    arg = '.'
        ret.append(arg)

After a small patch, the code seems still valid for me, but allowing a trailing
"/" after a trailing "/.", therefore allowing a trailing "/./".

My proposed patch:

    ret = [ ]
    for arg in got:
        if args.dir != '/' and arg != '.' and (typ == 3 or (typ == 2 and not
am_sender)):
1.          arg_has_trailing_slash = arg.endswith('/')
            if arg_has_trailing_slash:
                arg = arg[:-1]
2.          arg_has_trailing_slash_dot = arg.endswith('/.')
            if arg_has_trailing_slash_dot:
                arg = arg[:-2]
3.          real_arg = os.path.realpath(arg)
            if arg != real_arg and not real_arg.startswith(args.dir_slash):
                die('unsafe arg:', orig_arg, [arg, real_arg])
4.          if arg_has_trailing_slash_dot:
                arg += '/.'
5.          if arg_has_trailing_slash:
                arg += '/'
            if opt == 'arg' and arg.startswith(args.dir_slash):
                arg = arg[args.dir_slash_len:]
                if arg == '':
                    arg = '.'
        ret.append(arg)

1. Trailing "/" is removed.
2. Trailing "/." is removed.
3. Directory is checked.
4. Trailing "/." is appended again.
5. Trailing "/" is appended again.

Please check carefully, I am not familiar with the Python language.


   * What was the outcome of this action?

The check in line 320 (
https://salsa.debian.org/debian/rsync/-/blob/debian/3.2.7-1/support/rrsync?ref_type=tags#L320
) does not fail, which is the intended result.


Thank you in advance.