rrsync 3.4 broke incremental backups (fix not sufficient / working)

[stapelberg]

I generated the following report using Claude Opus 5 (but did review and did verify the reproducer works).

See also #922 for a previous report; however I don’t think the fix works, or at least not for my situation.

---

I noticed this in my nightly backup log:

rrsync error: unsafe arg: / ['', '/srv/backup/frigaten']
rsync: connection unexpectedly closed (0 bytes received so far) [sender]
rsync error: error in rsync protocol data stream (code 12) at io.c(232) [sender=3.4.4]

and, a few minutes later, a transfer summary that should not have been possible:

Number of created files: 1,141
Literal data: 949,186,488 bytes
Matched data: 0 bytes

The cause is a one-line change in support/rrsync that shipped in rsync 3.4. It is
present in v3.4, v3.4.2, v3.4.3 and v3.4.4, and still present in master today.

How the backup is supposed to work

Each backed-up host pushes to its own restricted directory on the NAS. The
authorized_keys entry is the usual rrsync forced command:

command="/nix/store/…-rrsync-3.4.4/bin/rrsync /srv/backup/midna" ssh-ed25519 AAAA… root@midna

The client asks the server what snapshots already exist, then hardlinks against the
most recent one:

rsync --list-only -e ssh "$dest:/"
rsync -ax --relative --numeric-ids --link-dest=/2026-07-09 / "$dest:/2026-07-31"

Both paths are absolute, anchored at the root of the restricted directory. That is
rrsync's documented convention: a leading / means "the root of DIR", and rrsync
rewrites it to the real path.

Two things broke

The listing broke first. A bare / is now mangled into the empty string, and rrsync
then fails its own safety check against it — that is the unsafe arg: / line. Because
the listing runs in backticks with its exit status ignored, the client concluded
"no previous backups exist", skipped --link-dest entirely, and wrote a full copy.

Changing the listing to "$dest:." fixed that. It also made the second, quieter bug
visible. The client now correctly finds 2026-07-09 and passes
--link-dest=/2026-07-09 — and rsync answers:

--link-dest arg does not exist: 2026-07-09

Note the missing leading slash. That is the whole bug, and note also that this is a
warning: rsync carries on without the link-dest and produces a complete, correct,
enormous full copy. Nothing exits non-zero. Nothing in my monitoring noticed for
three weeks.

Why the path comes out wrong

rrsync chdirs into the restricted directory and validates each path argument
relative to that CWD. rsync resolves a relative --link-dest relative to the
destination directory. Those two are the same place only if the destination is
the restricted root — and in a dated-snapshot layout it never is.

So once the leading slash is stripped, 2026-07-09 is looked for inside
/srv/backup/midna/2026-07-31/, where it obviously does not exist.

There is no way to spell around this from the client:

• absolute (/2026-07-09) — the slash is stripped, and the code that would re-anchor
  it is now unreachable;
• parent-relative (../2026-07-09) — rejected outright by rrsync's .. check;
• destination-at-the-root, with the date moved into the transferred path — I tried
  this; it fails, because --link-dest matches files by their path within the
  transfer, so it goes looking for 2026-07-09/2026-07-31/sub/a.txt.

Upstream added an -absolute flag in 0d0399bb1420cea150931245f152feb4768e1e06 and
closed the corresponding bug report as fixed by it. It does not help — see below.

The upstream change was a security fix

My first instinct was that the change was simply wrong and should be reverted. It is
not, and it should not.

The commit is d4c4f6754eff0d8ea6fdb327abf5c874bfccb8dd, "fixed remove multiple
leading slashes", authored 2025-06-11:

     if arg.startswith('./'):
         arg = arg[1:]
     arg = arg.replace('//', '/')
+    arg = arg.lstrip('/')
     if args.dir != '/':
         if HAS_DOT_DOT_RE.search(arg):
             die("do not use .. in", opt, …)
         if arg.startswith('/'):
             arg = args.dir + arg

str.replace('//', '/') is a single non-recursive left-to-right pass, so it does not
collapse runs of slashes:

'//foo'   -> '/foo'
'///foo'  -> '//foo'    # survives
'////foo' -> '//foo'

That surviving double slash is a sandbox escape. Trace ///etc/hostname with
DIR=/srv/backup/midna:

step	value
after replace('//', '/')	//etc/hostname
after joining with args.dir	/srv/backup/midna//etc/hostname
os.path.realpath()	/srv/backup/midna/etc/hostname — check passes
after arg[dir_slash_len:]	/etc/hostname

realpath() normalizes the double slash away, so the safety check is satisfied. Then
the final slice removes /srv/backup/midna/ and leaves a leading slash behind, and
rsync treats the result as an absolute path. The client reads — or writes — anywhere
on the server, as whatever user the forced command runs as. For a backup NAS that is
root.

I confirmed both directions against the pre-change script: pulling
fake:///etc/hostname returned the server's real /etc/hostname, and pushing to an
absolute path outside the restricted directory landed the file there.

So the fix was necessary. It just reached one character too far: it removes all
leading slashes when it only needed to collapse them to one.

Reproducer

Self-contained; takes the rrsync to test as its only argument. It touches nothing
outside its own temporary directory — the "secret" it tries to steal is a file the
script plants next to the restricted directory, so there is no dependency on what
/etc happens to look like.

#!/bin/sh
# Demonstrate the rsync 3.4 rrsync --link-dest regression, and the sandbox
# escape that the offending commit was closing.
#
# Usage: repro.sh /path/to/rrsync

die() { echo "ERROR: $*" >&2; exit 2; }
nlink() { stat -c %h "$1" 2>/dev/null || stat -f %l "$1" 2>/dev/null; }

RRSYNC=$1
[ -n "$RRSYNC" ] || die "usage: $0 /path/to/rrsync   (try: $0 \"\$(command -v rrsync)\")"
[ -x "$RRSYNC" ] || die "not executable: $RRSYNC"
command -v rsync >/dev/null 2>&1 || die "no rsync in PATH"

echo "rsync:  $(rsync --version 2>/dev/null | head -1)"
echo "rrsync: $RRSYNC"
echo

tmp=$(mktemp -d) || die "mktemp failed"
trap 'rm -rf "$tmp"' EXIT
cd "$tmp" || die "cannot cd to $tmp"

# The restricted dir holds yesterday's snapshot. secret.txt sits outside it.
mkdir -p restricted/2026-07-09/sub src/sub out
echo hello > src/sub/a.txt
cp -a src/sub/a.txt restricted/2026-07-09/sub/a.txt
echo "you should not be able to read this" > secret.txt

# Stands in for ssh: drop the hostname, hand the rest to rrsync as a forced command.
cat > fakessh <<EOF
#!/bin/sh
shift
SSH_ORIGINAL_COMMAND="\$*" exec "$RRSYNC" "$tmp/restricted"
EOF
chmod +x fakessh

echo "=== 1. incremental backup against yesterday, via --link-dest=/2026-07-09 ==="
rsync -a --stats -e "$tmp/fakessh" --link-dest=/2026-07-09 src/ fake:/2026-07-31 >rsync.out 2>&1
rc=$?
interesting=$(grep -E 'link-dest|Literal data|Matched data|error|rrsync' rsync.out)
[ -n "$interesting" ] || interesting=$(cat rsync.out)
printf '%s\n' "$interesting" | sed 's/^/    /'
[ $rc -eq 0 ] || echo "    rsync exited $rc"
if [ ! -f restricted/2026-07-31/sub/a.txt ]; then
    echo "  RESULT: no transfer happened at all (rsync rc=$rc)"
elif [ "$(nlink restricted/2026-07-31/sub/a.txt)" -gt 1 ]; then
    echo "  RESULT: OK — hardlinked to yesterday (st_nlink=$(nlink restricted/2026-07-31/sub/a.txt))"
else
    echo "  RESULT: BROKEN — --link-dest ignored, full copy written (st_nlink=1)"
fi

echo
echo "=== 2. client asks for a path outside the restricted dir (3 leading slashes) ==="
echo "    requesting fake://$tmp/secret.txt"
rsync -a -e "$tmp/fakessh" "fake://$tmp/secret.txt" out/ >esc.out 2>&1
rc=$?
sed 's/^/    /' esc.out
if [ -s out/secret.txt ]; then
    echo "    got: $(cat out/secret.txt)"
    echo "  RESULT: ESCAPED — this rrsync leaks files outside the restricted dir"
else
    echo "    rsync exited $rc, nothing fetched"
    echo "  RESULT: OK — confined"
fi

Against the rrsync from rsync 3.4.4:

=== 1. incremental backup against yesterday, via --link-dest=/2026-07-09 ===
    --link-dest arg does not exist: 2026-07-09
    Literal data: 6 bytes
    Matched data: 0 bytes
  RESULT: BROKEN — --link-dest ignored, full copy written (st_nlink=1)

=== 2. client asks for a path outside the restricted dir (3 leading slashes) ===
    requesting fake:///tmp/tmp.tPo5ppUE5N/secret.txt
    rsync: [sender] change_dir "/tmp/tmp.tPo5ppUE5N/restricted/tmp/tmp.tPo5ppUE5N" failed: No such file or directory (2)
    rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1876) [Receiver=3.4.4]
    rsync exited 23, nothing fetched
  RESULT: OK — confined

and against the same script with d4c4f6754eff0d8ea6fdb327abf5c874bfccb8dd backed out,
which is the trade the commit was making:

=== 1. incremental backup against yesterday, via --link-dest=/2026-07-09 ===
    Literal data: 0 bytes
    Matched data: 0 bytes
  RESULT: OK — hardlinked to yesterday (st_nlink=2)

=== 2. client asks for a path outside the restricted dir (3 leading slashes) ===
    requesting fake:///tmp/tmp.kAEvjuOpY3/secret.txt
    got: you should not be able to read this
  RESULT: ESCAPED — this rrsync leaks files outside the restricted dir

The two properties are in tension in every released version:

rrsync	--link-dest	hardlinks	escape via ///
before d4c4f675…	works	2	escapes
3.4 … 3.4.4, master	ignored (warning only)	1	confined
master + -absolute	ignored (warning only)	1	confined
3.4.4 + fix	works	2	confined

-absolute is not a fix

The listing half of this is reported upstream as issue #922: rsync host:/ . failing
with unsafe arg: /. It is closed with "fixed in #890 in master", where #890 is the
-absolute commit. Two commenters on the issue say that is not really a fix, and
they are right — but it is worse than they say, because it does not fix the command
in the issue's own title either.

I built support/rrsync from master and ran it both ways. With -absolute,
--link-dest still writes a full copy in every spelling:

--link-dest=	destination	result
/2026-07-09	/2026-07-31	full copy
/2026-07-09	$RESTRICTED/2026-07-31	full copy
$RESTRICTED/2026-07-09	/2026-07-31	full copy
$RESTRICTED/2026-07-09	$RESTRICTED/2026-07-31	full copy

One clause explains all four rows:

is_absolute_arg = args.absolute and opt == 'arg' and …

--link-dest is declared as long_opts['link-dest'] = 2 and reaches validated_arg()
with opt='link-dest', so it never qualifies as an absolute arg and is still
lstriped.

The reported case fares no better. rsync host:/ . still dies with unsafe arg: /
with the flag on, because / is neither equal to args.dir nor prefixed by it.
Sending the full server-side path instead trades that for a new failure —
ERROR: rejecting unrequested file-list name: 2026-07-09 — which is rsync 3.4's own
client-side file-list hardening tripping over the args.dir → . rewrite that
-absolute performs.

There is a design objection too: -absolute requires the client to name the server's
restricted directory, so both ends now have to agree on a string they previously did
not share. My clients derive nothing from the server beyond "the root of my own
restricted dir", which is exactly what the leading / used to mean.

The fix

Two versions, because 0d0399bb… changed the surrounding code.

Against 3.4.4 — and anything else in the 3.4 series, where the re-anchoring is still
present but unreachable — collapse the leading slashes instead of removing them:

-    arg = arg.lstrip('/')
+    arg = re.sub(r'^/+', '/', arg)

With it, ///etc/hostname becomes /etc/hostname, gets anchored to
/srv/backup/midna/etc/hostname with no doubled separator, and the final slice yields
a relative path as intended. --link-dest=/2026-07-09 survives as an absolute path
again. Both escape directions stay closed, and --list-only "$dest:/" works again too.

Against master it takes two hunks, because 0d0399bb… also deleted the anchoring:

-        arg = arg.lstrip('/')
+        arg = re.sub(r'^/+', '/', arg)
     if args.dir != '/':
         if HAS_DOT_DOT_RE.search(arg):
             die("do not use .. in", opt, …)
+        if arg.startswith('/'):
+            arg = args.dir + arg

The collapse alone is not enough on master, and is in fact worse than the status quo:
with the anchoring gone, a preserved leading slash goes straight through as a real
absolute path. I measured that variant — it escapes, and the backup half tries to
mkdir "/2026-07-31" at the filesystem root. The fix suggested in issue #922,
while arg != (arg := arg.replace("//", "/")): pass, has the same problem if applied
to master unchanged; on 3.4.4 it is equivalent to the one-liner above.

With both hunks, master passes everything:

list host:/                                  rc=0
--link-dest=/2026-07-09  dest=/2026-07-31    OK hardlinked
escape via ///                               confined