From 2e3e32dfce0250a9c22ba7514cb38026de333e3f Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 May 2026 08:01:11 +1000 Subject: [PATCH 1/8] testsuite: verify --fuzzy actually selects a basis Both fuzzy tests asserted only that the final file content matched, which a full transfer that ignored --fuzzy would also satisfy -- so a broken fuzzy basis selection would pass undetected. Drive rsync directly with --debug=FUZZY and assert the generator reports the expected basis ("fuzzy basis selected for : ", generator.c find_fuzzy): rsync2.c for fuzzy, and the closest-named candidate archive-v1.tar for fuzzy-basis. fuzzy switches from checkit() to a manual run plus verify_dirs() so the output can be captured. Co-Authored-By: Claude Opus 4.7 (1M context) --- testsuite/fuzzy-basis_test.py | 12 ++++++++++-- testsuite/fuzzy_test.py | 18 +++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/testsuite/fuzzy-basis_test.py b/testsuite/fuzzy-basis_test.py index 77889439b..d91a3b9ed 100644 --- a/testsuite/fuzzy-basis_test.py +++ b/testsuite/fuzzy-basis_test.py @@ -11,7 +11,7 @@ from rsyncfns import ( FROMDIR, TODIR, - assert_same, make_data_file, makepath, rmtree, run_rsync, + assert_same, make_data_file, makepath, rmtree, run_rsync, test_fail, ) src = FROMDIR @@ -31,7 +31,15 @@ (TODIR / deepdir / 'archive-old.tar').write_bytes(base[:200_000]) (TODIR / deepdir / 'unrelated.dat').write_bytes(b'nothing alike' * 1000) -run_rsync('-a', '--fuzzy', '--no-whole-file', f'{src}/', f'{TODIR}/') +# Capture --debug=FUZZY: a correct final file alone would also result from a +# full transfer that ignored --fuzzy, so assert the scorer actually chose the +# closest-named candidate (archive-v1.tar) as the basis, not just that bytes match. +proc = run_rsync('-a', '--fuzzy', '--no-whole-file', '--debug=FUZZY', + f'{src}/', f'{TODIR}/', capture_output=True) +want = f'fuzzy basis selected for {newfile}: {os.path.join(deepdir, "archive-v1.tar")}' +if want not in proc.stdout: + test_fail(f"--fuzzy did not score archive-v1.tar as the closest basis; " + f"expected {want!r}, --debug=FUZZY output was:\n{proc.stdout}") assert_same(TODIR / newfile, src / newfile, label='fuzzy result') print("fuzzy-basis: --fuzzy candidate scoring (fuzzy_distance) verified at depth") diff --git a/testsuite/fuzzy_test.py b/testsuite/fuzzy_test.py index 7858fcc26..03c57301b 100644 --- a/testsuite/fuzzy_test.py +++ b/testsuite/fuzzy_test.py @@ -8,7 +8,9 @@ import time -from rsyncfns import FROMDIR, SRCDIR, TODIR, checkit, cp_p, cp_touch +from rsyncfns import ( + FROMDIR, SRCDIR, TODIR, cp_p, cp_touch, run_rsync, test_fail, verify_dirs, +) FROMDIR.mkdir(parents=True, exist_ok=True) @@ -18,5 +20,15 @@ cp_touch(FROMDIR / 'rsync.c', TODIR / 'rsync2.c') time.sleep(1) -checkit(['-avvi', '--no-whole-file', '--fuzzy', '--delete-delay', - f'{FROMDIR}/', f'{TODIR}/'], FROMDIR, TODIR) +# Drive rsync directly (rather than checkit) so we can capture --debug=FUZZY: +# a final tree match alone would also be produced by a full transfer that +# ignored --fuzzy, so assert the generator actually picked rsync2.c as the +# fuzzy basis for rsync.c (generator.c find_fuzzy / "fuzzy basis selected"). +proc = run_rsync('-avvi', '--no-whole-file', '--fuzzy', '--delete-delay', + '--debug=FUZZY', f'{FROMDIR}/', f'{TODIR}/', + capture_output=True) +if 'fuzzy basis selected for rsync.c: rsync2.c' not in proc.stdout: + test_fail("--fuzzy did not select rsync2.c as the basis for rsync.c; " + f"--debug=FUZZY output was:\n{proc.stdout}") +# ...and --delete-delay still removes the stale basis, leaving TODIR == FROMDIR. +verify_dirs(FROMDIR, TODIR) From 8554d4f8ba79fe467074c3f4543dc3b6148bd605 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 May 2026 08:02:41 +1000 Subject: [PATCH 2/8] testsuite: verify the negotiated compressor/checksum selection compress-options only checked that each requested algorithm yielded byte-identical output, which proves parsing/non-corruption but not that the advertised algorithm was actually used -- the test would pass if the choice were silently ignored. Capture --debug=NSTR (compat.c / checksum.c) and assert the selected compressor, compress level, and checksum match the request (anchored so zlib != zlibx). --skip-compress / --checksum-seed stay content checks: they have no comparable negotiation-string signal. Co-Authored-By: Claude Opus 4.7 (1M context) --- testsuite/compress-options_test.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/testsuite/compress-options_test.py b/testsuite/compress-options_test.py index 74e087712..c11741e67 100644 --- a/testsuite/compress-options_test.py +++ b/testsuite/compress-options_test.py @@ -9,10 +9,11 @@ """ import json +import re from rsyncfns import ( FROMDIR, TODIR, - assert_same, make_tree, rmtree, run_rsync, walk_files, + assert_same, make_tree, rmtree, run_rsync, test_fail, walk_files, ) src = FROMDIR @@ -34,14 +35,25 @@ def verify(rels, label): # --- --compress-choice for every advertised compressor ---------------------- +# Byte-identical output alone proves only that the option didn't corrupt data; +# assert via --debug=NSTR (compat.c) that the requested compressor was actually +# selected for the transfer. The trailing " (level" anchors so zlib != zlibx. for algo in compressors: rels = fresh() - run_rsync('-az', f'--compress-choice={algo}', f'{src}/', f'{TODIR}/') + proc = run_rsync('-az', f'--compress-choice={algo}', '--debug=NSTR', + f'{src}/', f'{TODIR}/', capture_output=True) + if not re.search(rf'compress: {re.escape(algo)} \(level', proc.stdout): + test_fail(f"--compress-choice={algo} was not the selected compressor; " + f"--debug=NSTR output:\n{proc.stdout}") verify(rels, f'--compress-choice={algo}') -# --- --compress-level ------------------------------------------------------- +# --- --compress-level (the requested level reaches the compressor) ---------- rels = fresh() -run_rsync('-az', '--compress-level=9', f'{src}/', f'{TODIR}/') +proc = run_rsync('-az', '--compress-level=9', '--debug=NSTR', + f'{src}/', f'{TODIR}/', capture_output=True) +if not re.search(r'compress: \S+ \(level 9\)', proc.stdout): + test_fail("--compress-level=9 was not applied; " + f"--debug=NSTR output:\n{proc.stdout}") verify(rels, '--compress-level=9') # --- --skip-compress (the file must still arrive intact) -------------------- @@ -52,9 +64,15 @@ def verify(rels, label): label='--skip-compress gz') # --- --checksum-choice for every advertised checksum ------------------------ +# As above: assert via --debug=NSTR (checksum.c) that the requested checksum was +# the one negotiated, not merely that the transfer succeeded. for algo in checksums: rels = fresh() - run_rsync('-a', '-c', f'--checksum-choice={algo}', f'{src}/', f'{TODIR}/') + proc = run_rsync('-a', '-c', f'--checksum-choice={algo}', '--debug=NSTR', + f'{src}/', f'{TODIR}/', capture_output=True) + if not re.search(rf'checksum: {re.escape(algo)}\b', proc.stdout): + test_fail(f"--checksum-choice={algo} was not the selected checksum; " + f"--debug=NSTR output:\n{proc.stdout}") verify(rels, f'--checksum-choice={algo}') # --- --checksum-seed -------------------------------------------------------- From e9894fec3d9a2bc61283580338926f3f3aa0c289 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 May 2026 08:04:56 +1000 Subject: [PATCH 3/8] testsuite: harden output-options checks Several subcases ran rsync without checking the exit status, so a silent failure could pass as the expected (often empty) output -- most notably -q, which only asserted empty stdout. Route every expected-success run through a helper that asserts the exit status, and verify -q actually transferred the tree. Replace the "-h/-8 didn't break the transfer" check with positive format assertions: -h must render byte counts with a K/M/G suffix (and the default must not), and -8 must leave a high-bit filename byte unescaped (\#371 absent) where the default escapes it -- best-effort, self-skipping where the platform can't store the raw byte. Co-Authored-By: Claude Opus 4.7 (1M context) --- testsuite/output-options_test.py | 74 ++++++++++++++++++++++++++++---- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/testsuite/output-options_test.py b/testsuite/output-options_test.py index 3a836e1cd..1d57c086a 100644 --- a/testsuite/output-options_test.py +++ b/testsuite/output-options_test.py @@ -5,25 +5,40 @@ checked for the documented output shape rather than at depth: --version, --help, --itemize-changes (-i), --dry-run (-n), --stats, --out-format, --list-only, --quiet (-q), --progress, -h, -8. + +Every rsync run that is expected to succeed has its exit status checked (a +silent failure must not pass as "no output"), and the format-changing options +(-h, -8) assert the documented difference rather than merely "didn't break". """ +import os +import re import subprocess from rsyncfns import ( FROMDIR, TODIR, - assert_not_exists, make_tree, rmtree, rsync_argv, test_fail, + assert_not_exists, make_data_file, make_tree, makepath, rmtree, rsync_argv, + test_fail, verify_dirs, ) src = FROMDIR -def out(*args): - return subprocess.run(rsync_argv(*args), capture_output=True, text=True) +def out(*args, want_rc=0, env=None, text=True): + """Run rsync capturing output. Unless want_rc is None, fail the test if the + exit status isn't want_rc -- so a broken transfer can't masquerade as the + expected (often empty) output.""" + p = subprocess.run(rsync_argv(*args), capture_output=True, text=text, env=env) + if want_rc is not None and p.returncode != want_rc: + err = p.stderr if text else p.stderr.decode('latin-1', 'replace') + test_fail(f"rsync {' '.join(args)} exited {p.returncode}, " + f"expected {want_rc}:\n{err}") + return p # --- --version / --help ----------------------------------------------------- p = out('--version') -if p.returncode != 0 or 'protocol version' not in p.stdout: +if 'protocol version' not in p.stdout: test_fail(f"--version output unexpected:\n{p.stdout}") p = out('--help') help_txt = p.stdout + p.stderr @@ -68,11 +83,14 @@ def out(*args): test_fail(f"--list-only did not list files:\n{p.stdout}") assert_not_exists(TODIR / 'f0', label='--list-only copied a file') -# --- --quiet suppresses normal stdout --------------------------------------- +# --- --quiet suppresses normal stdout BUT still transfers ------------------- +# Checking only for empty stdout would also pass if the transfer silently +# failed, so confirm the destination actually received the tree. rmtree(TODIR) p = out('-a', '-q', f'{src}/', f'{TODIR}/') if p.stdout.strip() != '': test_fail(f"--quiet produced stdout: {p.stdout!r}") +verify_dirs(src, TODIR, label='--quiet still transferred') # --- --progress shows a percentage ------------------------------------------ rmtree(TODIR) @@ -80,11 +98,49 @@ def out(*args): if '100%' not in p.stdout: test_fail(f"--progress did not show a percentage:\n{p.stdout}") -# --- -h / -8 do not break a transfer ---------------------------------------- +# --- -h / --human-readable formats byte counts with a unit suffix ----------- +# Without -h, --stats prints grouped digits ("50,000 bytes"); with -h it uses a +# K/M/G suffix ("50.00K"). Use a file big enough that the two forms differ. +rmtree(src) +rmtree(TODIR) +makepath(src) +make_data_file(src / 'big', 50_000) +plain = out('-a', '--stats', f'{src}/', f'{TODIR}/').stdout +rmtree(TODIR) +human = out('-a', '-h', '--stats', f'{src}/', f'{TODIR}/').stdout +suffix_re = r'Total file size: [\d.]+[KMG]' +if not re.search(suffix_re, human): + test_fail(f"-h did not use a human-readable unit suffix:\n{human}") +if re.search(suffix_re, plain): + test_fail(f"--stats without -h unexpectedly used a unit suffix:\n{plain}") + +# --- -8 / --8-bit-output leaves high-bit filename bytes unescaped ------------ +# rsync escapes non-printable name bytes as \#NNN; -8 prints 8-bit bytes raw. +# This needs a filename containing a high-bit byte and a C locale (where such a +# byte is non-printable). Best-effort: skip silently where the filesystem can't +# store the raw byte (macOS/Cygwin may reject or normalise it). +rmtree(src) rmtree(TODIR) -p = out('-a', '-h', '-8', '--stats', f'{src}/', f'{TODIR}/') -if p.returncode != 0: - test_fail(f"-h/-8 broke the transfer:\n{p.stderr}") +makepath(src) +weird = os.fsencode(str(src)) + b'/hi\xf9name' # 0xf9 -> octal 371 +try: + with open(weird, 'wb') as f: + f.write(b"x\n") + weird_ok = True +except OSError: + weird_ok = False + +if weird_ok: + cenv = dict(os.environ, LC_ALL='C') + makepath(TODIR) + noesc = out('-ai', f'{src}/', f'{TODIR}/', env=cenv, text=False) + if rb'\#371' in noesc.stdout: # FS preserved the raw byte as expected + rmtree(TODIR) + makepath(TODIR) + esc = out('-ai', '-8', f'{src}/', f'{TODIR}/', env=cenv, text=False) + if rb'\#371' in esc.stdout: + test_fail("-8 should leave the high-bit name byte unescaped, but " + f"the \\#371 escape was still present:\n{esc.stdout!r}") print("output-options: version/help/-i/-n/--stats/--out-format/--list-only/" "-q/--progress/-h/-8 verified") From 3b0d3d20f103db901fcf8483949f5ee59dd20d62 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 May 2026 08:08:42 +1000 Subject: [PATCH 4/8] testsuite: add positive controls to the symlink-race security tests The symlink-race tests only asserted that an outside sentinel was unchanged or unlisted while ignoring rsync's exit status, so an attack transfer/listing that failed before reaching the vulnerable receiver/sender path would pass without the security property ever being exercised. Add a positive control to each -- an ordinary in-module write (bare-do-open, chdir) or an in-module listing (sender-flist-leak) that must succeed -- so a globally broken/refusing daemon can no longer make the sentinel checks vacuous, and assert the attack run did not die from a signal. clean-fname-underflow now also enforces a non-zero exit: clean_fname() collapses "a/../test" to "test", whose merge file is absent, so rsync must reject it; accepting it (rc 0) would mean the crafted name was mis-collapsed. Co-Authored-By: Claude Opus 4.7 (1M context) --- testsuite/bare-do-open-symlink-race_test.py | 37 ++++++++++++++++++--- testsuite/chdir-symlink-race_test.py | 32 ++++++++++++++++-- testsuite/clean-fname-underflow_test.py | 11 ++++-- testsuite/sender-flist-symlink-leak_test.py | 19 +++++++++++ 4 files changed, 89 insertions(+), 10 deletions(-) diff --git a/testsuite/bare-do-open-symlink-race_test.py b/testsuite/bare-do-open-symlink-race_test.py index deff07314..6ad70a40d 100644 --- a/testsuite/bare-do-open-symlink-race_test.py +++ b/testsuite/bare-do-open-symlink-race_test.py @@ -92,10 +92,31 @@ def verify_outside_unchanged_or_absent(label: str, target: str) -> None: def run_attack(args): - subprocess.run( + """Run the (expected-to-be-contained) attack transfer and return its exit + code; output is discarded since the security assertion is on the filesystem.""" + return subprocess.run( rsync_argv(*args), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - ) + ).returncode + + +def positive_control(): + """Confirm the upload module accepts an ordinary write. Without this, every + scenario below would also pass if the daemon refused (or failed) before the + vulnerable code ran -- the sentinel-unchanged check alone can't tell a + securely-contained attack from one that never reached the receiver path.""" + for d in (mod, src): + rmtree(d) + d.mkdir(parents=True) + (src / 'legit.txt').write_text("LEGIT_IN_MODULE\n") + rc = run_attack(['-t', f'{src}/legit.txt', f'{daemon_url}/upload/legit.txt']) + if rc != 0 or not (mod / 'legit.txt').is_file() \ + or (mod / 'legit.txt').read_text() != "LEGIT_IN_MODULE\n": + test_fail(f"positive control: upload module did not accept a normal " + f"write (rc={rc}); attack scenarios would be vacuous") + + +positive_control() # Scenario 3b: --inplace --backup --backup-dir=cd @@ -105,10 +126,12 @@ def run_attack(args): (src / 'target.txt').write_text("NEW_DATA_FROM_SENDER\n") os.chmod(src / 'target.txt', 0o644) -run_attack([ +rc = run_attack([ '--inplace', '--backup', '--backup-dir=cd', f'{src}/target.txt', f'{daemon_url}/upload/target.txt', ]) +if rc >= 128: + test_fail(f"3b inplace+backup-dir=cd: rsync died from a signal (rc={rc})") verify_outside_unchanged("3b inplace+backup-dir=cd") @@ -117,7 +140,9 @@ def run_attack(args): (src / 'cd').mkdir() os.symlink('/etc/passwd', src / 'cd' / 'sym') -run_attack(['-rl', f'{src}/', f'{daemon_url}/upload_fake/']) +rc = run_attack(['-rl', f'{src}/', f'{daemon_url}/upload_fake/']) +if rc >= 128: + test_fail(f"3c-symlink: rsync died from a signal (rc={rc})") verify_outside_unchanged_or_absent("3c-symlink fake-super symlink push", "sym") @@ -132,5 +157,7 @@ def run_attack(args): if not stat.S_ISFIFO((src / 'cd' / 'fifo').stat().st_mode): test_skipped("mkfifo unavailable; cannot exercise 3c-mknod") -run_attack(['-rD', f'{src}/', f'{daemon_url}/upload_fake/']) +rc = run_attack(['-rD', f'{src}/', f'{daemon_url}/upload_fake/']) +if rc >= 128: + test_fail(f"3c-mknod: rsync died from a signal (rc={rc})") verify_outside_unchanged_or_absent("3c-mknod fake-super FIFO push", "fifo") diff --git a/testsuite/chdir-symlink-race_test.py b/testsuite/chdir-symlink-race_test.py index 80b314c04..623793c9f 100644 --- a/testsuite/chdir-symlink-race_test.py +++ b/testsuite/chdir-symlink-race_test.py @@ -87,13 +87,41 @@ def verify_unchanged(label: str) -> None: def run_attack(label: str, *args) -> None: reset_outside() - subprocess.run( + rc = subprocess.run( rsync_argv(*args), stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - ) + ).returncode + if rc >= 128: + test_fail(f"{label}: rsync died from a signal (rc={rc})") verify_unchanged(label) +def positive_control() -> None: + """Confirm the receiver writes into an ordinary in-module subdirectory, so + the symlink-escape scenarios below genuinely exercise the chdir path rather + than passing because the daemon refused (or failed) before reaching it.""" + real = mod / 'realdir' + rmtree(real) + real.mkdir() + # When this test runs as root the daemon serves as 'nobody' (the module + # sets no uid), so make the control target world-writable; push a single + # file with no attribute preservation so the write never needs to own/chmod + # the dir -- it should land purely on the receiver's normal write path. + os.chmod(real, 0o777) + rc = subprocess.run( + rsync_argv(f'{src}/subdir/target.txt', f'{url}upload/realdir/'), + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ).returncode + landed = real / 'target.txt' + if rc != 0 or not landed.is_file() \ + or not filecmp.cmp(landed, src / 'subdir' / 'target.txt', shallow=False): + test_fail(f"positive control: receiver did not write into an ordinary " + f"in-module subdir (rc={rc}); attack scenarios would be vacuous") + + +positive_control() + + # 1. Single file with --size-only -- receiver normally skips basis open and # goes straight to chmod; only the chdir-escape blocks it. run_attack("single-file --size-only", diff --git a/testsuite/clean-fname-underflow_test.py b/testsuite/clean-fname-underflow_test.py index 4b30155ce..638bf3ae4 100644 --- a/testsuite/clean-fname-underflow_test.py +++ b/testsuite/clean-fname-underflow_test.py @@ -3,8 +3,10 @@ # # Ensure clean_fname() does not read-before-buffer when collapsing "..". # Exercises the --server path where a crafted merge filename hits -# clean_fname(); a non-zero exit is expected (the input is bogus), but -# the test fails if rsync dies from a signal (status >= 128). +# clean_fname(); the test fails if rsync dies from a signal (status >= 128) +# AND if rsync exits 0 -- the bogus input must be rejected (clean_fname +# collapses "a/../test" to "test", whose merge file does not exist, so rsync +# errors out non-zero). Accepting it would mean the name was mis-collapsed. import os import shlex @@ -29,5 +31,8 @@ if proc.returncode >= 128: test_fail(f"rsync exited due to a signal (status={proc.returncode})") +if proc.returncode == 0: + test_fail("rsync accepted the bogus 'a/../test' merge filter (expected a " + "non-zero rejection); clean_fname() may have mis-collapsed it") -print("OK: clean_fname() handled 'a/../test' without crashing") +print("OK: clean_fname() handled 'a/../test' without crashing, and rejected it") diff --git a/testsuite/sender-flist-symlink-leak_test.py b/testsuite/sender-flist-symlink-leak_test.py index 67fc2f11b..97c1f602b 100644 --- a/testsuite/sender-flist-symlink-leak_test.py +++ b/testsuite/sender-flist-symlink-leak_test.py @@ -49,6 +49,11 @@ os.symlink(str(outside), mod / 'cd') +# A legitimate in-module file, used as a positive control so the leak check +# below can't pass simply because the daemon's listing machinery is broken. +(mod / 'realdir').mkdir() +(mod / 'realdir' / 'in_module.txt').write_text("INSIDE_THE_MODULE\n") + my_uid = get_testuid() root_uid = get_rootuid() root_gid = get_rootgid() @@ -71,10 +76,24 @@ url = start_test_daemon(conf, DAEMON_PORT) +# Positive control: a normal recursive listing of an in-module path must +# enumerate the in-module file. If this fails, the daemon's flist generation is +# broken and the leak check below would be vacuously satisfied. +ctl = subprocess.run( + rsync_argv('-nrv', f'{url}upload/realdir/', f'{SCRATCHDIR}/dst/'), + capture_output=True, text=True, +) +if ctl.returncode != 0 or 'in_module.txt' not in ctl.stdout: + test_fail("positive control: listing an in-module path did not enumerate " + f"in_module.txt (rc={ctl.returncode}); leak check would be vacuous" + f"\n{ctl.stdout}{ctl.stderr}") + proc = subprocess.run( rsync_argv('-nrv', f'{url}upload/cd/', f'{SCRATCHDIR}/dst/'), capture_output=True, text=True, ) +if proc.returncode >= 128: + test_fail(f"leak pull: rsync died from a signal (rc={proc.returncode})") listfile.write_text(proc.stdout + proc.stderr) if 'leak_marker.txt' in listfile.read_text(): From b187332583e816dd087bcc499c0ecc6d55215aac Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 25 May 2026 08:12:03 +1000 Subject: [PATCH 5/8] testsuite: verify destination content/listings in daemon tests These daemon tests confirmed refusals/exclusions but accepted the allowed transfers on exit status alone, so a transfer that exited cleanly while moving nothing would pass: daemon-refuse allowed() imported verify_dirs but never called it; now it confirms the allowed push/pull actually populated the dest. daemon-filter pull()/the incoming push ignored their exit status, and the outgoing-chmod loop iterated only files that exist -- a zero-file pull passed vacuously. Check the codes and require at least one file to have been mode-checked. daemon run_and_check's unused `expected` param is dropped; the hidden-module and glob listings now compare the exact set of listed paths (catching a leaked extra path), replacing the per-path containment check and the dead normalise() helper whose regex never matched the -r listing format anyway. Co-Authored-By: Claude Opus 4.7 (1M context) --- testsuite/daemon-filter_test.py | 22 ++++++-- testsuite/daemon-refuse_test.py | 11 ++-- testsuite/daemon_test.py | 92 ++++++++++++++------------------- 3 files changed, 64 insertions(+), 61 deletions(-) diff --git a/testsuite/daemon-filter_test.py b/testsuite/daemon-filter_test.py index cac387e8b..59566068c 100644 --- a/testsuite/daemon-filter_test.py +++ b/testsuite/daemon-filter_test.py @@ -40,8 +40,11 @@ def pull(mod, dest): rmtree(dest) makepath(dest) - subprocess.run(rsync_argv('-a', f'{url}{mod}/', f'{dest}/'), - stdout=subprocess.DEVNULL) + proc = subprocess.run(rsync_argv('-a', f'{url}{mod}/', f'{dest}/'), + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + text=True) + if proc.returncode not in (0, 23): + test_fail(f"pull from {mod} failed (rc={proc.returncode}): {proc.stderr}") # --- daemon exclude hides *.secret everywhere in the module ----------------- @@ -53,8 +56,11 @@ def pull(mod, dest): label='daemon exclude kept others') # --- incoming chmod rewrites pushed file modes at depth --------------------- -subprocess.run(rsync_argv('-a', f'{src}/', f'{url}inc/'), - stdout=subprocess.DEVNULL) +proc = subprocess.run(rsync_argv('-a', f'{src}/', f'{url}inc/'), + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + text=True) +if proc.returncode not in (0, 23): + test_fail(f"incoming push failed (rc={proc.returncode}): {proc.stderr}") checked = 0 for rel in rels: p = incdir / rel @@ -67,10 +73,16 @@ def pull(mod, dest): # --- outgoing chmod rewrites pulled file modes at depth --------------------- op = SCRATCHDIR / 'outpull' pull('out', op) +checked = 0 for rel in rels: p = op / rel - if p.is_file() and (os.stat(p).st_mode & 0o044): + if not p.is_file(): + continue + checked += 1 + if os.stat(p).st_mode & 0o044: test_fail(f"outgoing chmod did not clear group/other read on {rel}: " f"{oct(os.stat(p).st_mode & 0o777)}") +if checked == 0: + test_fail("outgoing chmod test pulled no files (loop was vacuous)") print("daemon-filter: exclude / incoming chmod / outgoing chmod verified at depth") diff --git a/testsuite/daemon-refuse_test.py b/testsuite/daemon-refuse_test.py index 770b36ba9..054f28099 100644 --- a/testsuite/daemon-refuse_test.py +++ b/testsuite/daemon-refuse_test.py @@ -44,19 +44,23 @@ def refused(args, what): return proc.stderr -def allowed(args, what): +def allowed(args, what, expected=None, actual=None): proc = subprocess.run(rsync_argv(*args), stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True) if proc.returncode not in (0, 23): test_fail(f"{what} was unexpectedly refused: {proc.stderr}") + # A 0/23 exit alone doesn't prove the transfer happened; verify the data + # actually landed for the allowed cases. + if expected is not None: + verify_dirs(expected, actual, label=what) # --- a named refused option (delete) ---------------------------------------- refused(['-a', '--delete', f'{src}/', f'{url}refuse-delete/'], "--delete on a refuse=delete module") allowed(['-a', f'{src}/', f'{url}refuse-delete/'], - "plain push to a refuse=delete module") + "plain push to a refuse=delete module", src, deldir) # --- a wildcard refused option (checksum*) ---------------------------------- dest = SCRATCHDIR / 'wilddest' @@ -67,7 +71,8 @@ def allowed(args, what): # --- the "* !a !v" allow-list: -av allowed, -z refused ---------------------- rmtree(dest) makepath(dest) -allowed(['-av', f'{url}only-av/', f'{dest}/'], "-av on an allow-list module") +allowed(['-av', f'{url}only-av/', f'{dest}/'], "-av on an allow-list module", + src, dest) refused(['-avz', f'{url}only-av/', f'{dest}/'], "-z on an allow-list module") diff --git a/testsuite/daemon_test.py b/testsuite/daemon_test.py index 9643e941d..4e022fa5c 100644 --- a/testsuite/daemon_test.py +++ b/testsuite/daemon_test.py @@ -7,7 +7,6 @@ # by using RSYNC_CONNECT_PROG to spawn the daemon as a child of rsync. import os -import re import subprocess from rsyncfns import ( @@ -22,20 +21,21 @@ SSH = f"{SRCDIR / 'support' / 'lsh.sh'} --no-cd" -# Replacements that hide the variable parts of `rsync -r` listings: tabs/ -# columns for file vs directory, and the date/time stamp. -_FILE_RE = re.compile(r'^([^d][^ ]*) *(\.{10}[0-9]) ', flags=re.MULTILINE) -_DIR_RE = re.compile(r'^(d[^ ]*) *[0-9][.,0-9]* ', flags=re.MULTILINE) -_LS_RE = re.compile( - r'[0-9]{4}/[0-9]{2}/[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}' -) - -def normalise(text: str) -> str: - out = _FILE_RE.sub(r'\1 \2 ', text) - out = _DIR_RE.sub(r'\1 DIR ', out) - out = _LS_RE.sub('####/##/## ##:##:##', out) - return out +def listed_paths(text: str) -> set: + """The set of path names in an `rsync -r` listing. Each listing line is + "