docs: all-or-nothing TOCTOU caveat and two-section manifest budget

The walker's all-or-nothing guarantee holds only while the destination is not
concurrently modified (rehearsal and delete are separate walks).  The
protected-prefix list shares the 16 MB MAX_MANIFEST_BYTES budget with the
keep-set and each section is capped at MAX_MANIFEST_ENTRIES; an over-budget
frame is rejected on the receiver with STATUS_ERROR rather than truncated.

RSYNC_COMPAT.md

@@ -154,17 +154,24 @@ then reported an error (a truncated deletion); it now removes nothing and fails
 with an error naming the bound. Directories count toward the bound. A directory
 that still holds entries the walker leaves in place (a protected excluded file,
 a kept manifest entry, a symlink) is left behind rather than failing the run —
-matching rsync's "cannot delete non-empty directory" behaviour.
+matching rsync's "cannot delete non-empty directory" behaviour. The
+all-or-nothing guarantee holds only while the destination is not concurrently
+modified: rehearsal and delete are two separate walks, so a concurrent change
+between them (another process adding or removing destination entries) can make
+the actual deletion diverge from the counted set.
 
 Manifest size: the sender's keep-set and protected-prefix collections (streaming
 or early pre-scan) are unbounded, but the receiver rejects a manifest beyond
-`MAX_MANIFEST_ENTRIES` (1 048 576 entries) / `MAX_MANIFEST_BYTES` (16 MB of
-paths, counted across both sections) as a hard protocol error. In the commit
+`MAX_MANIFEST_ENTRIES` (1 048 576 entries, applied to EACH section — a frame can
+therefore total up to 2 097 152 entries) / `MAX_MANIFEST_BYTES` (16 MB of paths,
+counted across BOTH sections) as a hard protocol error. A heavily filtered
+source whose exclusion list grows large thus fails the run cleanly on the
+receiver (STATUS_ERROR) instead of being silently truncated. In the commit
 modes this only means the deletion is refused after the data already arrived; in
 the early modes (`--delete-before`/`--delete-during`) the manifest is the first
-frame, so an oversized keep-set aborts the whole transfer BEFORE any data is
-sent. Keep the source tree small enough for the receiver's manifest caps when
-using the early timing.
+frame, so an oversized keep-set or protected list aborts the whole transfer
+BEFORE any data is sent. Keep the source tree small enough for the receiver's
+manifest caps when using the early timing.
 
 Early-delete ACK wait: after committing a large deletion (up to
 `MAX_SERVER_DELETE_COUNT` removals) the receiver's `STATUS_OK`/`STATUS_ERROR`

src/client/client_send.c

@@ -628,7 +628,12 @@ static int send_list_only(const Config* config) {
 /* Send the delete manifest (keep-set paths plus the protected excluded
    prefixes) to the server. Returns 0 on success, -1 on failure.  When
    --delete-excluded is given `protected` is empty: excluded destination
-   mirrors are then ordinary extras and are removed. */
+   mirrors are then ordinary extras and are removed.  Both sections are
+   unbounded on the sender; the receiver enforces MAX_MANIFEST_ENTRIES per
+   section and a single MAX_MANIFEST_BYTES budget shared across the two
+   sections, rejecting (with STATUS_ERROR) an over-budget frame.  A heavily
+   filtered source whose exclusion list is large therefore fails the run
+   cleanly on the receiver rather than being truncated. */
 static int send_delete_manifest(int fd, ArrayList* manifest, ArrayList* protected_prefixes) {
   if (!manifest)
     return -1;
@@ -1048,6 +1053,18 @@ static int send_chunks_multithreaded(void* pipeline_context) {
         return thrd_error;
       }
       if (context->config->use_delete && !context->early_delete) {
+        /* Empty keep-set + scan I/O error must not delete the whole destination
+           (the source may not be genuinely empty -- see send_files). */
+        bool empty_io;
+        mtx_lock(&context->mutex_scanner);
+        empty_io = context->scan_had_io_error && context->manifest && context->manifest->size == 0;
+        mtx_unlock(&context->mutex_scanner);
+        if (empty_io) {
+          log_message(LOG_LEVEL_ERROR,
+                      "source scan hit an I/O error before finding any file; refusing to delete "
+                      "with an empty keep-set (--delete)");
+          goto send_fail;
+        }
         if (send_delete_manifest(client->file_descriptor, context->manifest,
                                  context->excluded_paths) != 0)
           goto send_fail;
@@ -1171,6 +1188,11 @@ static int scan_directory_multithreaded(void* pipeline_context) {
       break;
     }
   }
+  /* Capture the scanner results BEFORE destroying the scanner objects (the
+     io_error flag lives on the scanner, so reading it after destroy would be a
+     use-after-free). */
+  bool had_io =
+      dirs_mode ? directory_scanner_had_io_error(dscanner) : parallel_scanner_had_io_error(scanner);
   if (dirs_mode)
     directory_scanner_destroy(dscanner);
   else
@@ -1188,8 +1210,6 @@ static int scan_directory_multithreaded(void* pipeline_context) {
   }
   /* --ignore-errors: an unreadable subdirectory was skipped (workers recorded
      io_error, not failure); the deletion still runs but the run reports it. */
-  bool had_io =
-      dirs_mode ? directory_scanner_had_io_error(dscanner) : parallel_scanner_had_io_error(scanner);
   if (had_io) {
     mtx_lock(&context->mutex_scanner);
     context->scan_had_io_error = true;
@@ -1360,8 +1380,21 @@ int send_files(Config* config) {
       goto send_fail;
     bool prescan_ok = scan_paths_only(config, &prepared.options, early_manifest, &had_scan_io);
     bool early_ok = false;
-    if (prescan_ok)
-      early_ok = send_delete_manifest_early(client, early_manifest, excluded);
+    if (prescan_ok) {
+      /* A scan that hit an I/O error and produced NO keep entries is ambiguous
+         (the source may not be genuinely empty -- part of it was unreadable),
+         and an empty keep-set would delete the whole destination.  Refuse to
+         delete; the genuine-empty-source case has no io_error and still sends
+         its (empty) keep-set. */
+      if (had_scan_io && early_manifest->size == 0) {
+        log_message(LOG_LEVEL_ERROR,
+                    "source scan hit an I/O error before finding any file; refusing to delete "
+                    "with an empty keep-set (--delete)");
+        prescan_ok = false;
+      } else {
+        early_ok = send_delete_manifest_early(client, early_manifest, excluded);
+      }
+    }
     array_list_delete(early_manifest);
     /* The keep-set (and its protected prefixes) are already on the wire; the
        data pass must not append to the exclusion list again. */
@@ -1436,6 +1469,15 @@ int send_files(Config* config) {
     goto send_fail;
   if (directory_scanner_had_io_error(scanner))
     had_scan_io = true;
+  if (had_scan_io && manifest && manifest->size == 0) {
+    /* A scan that hit an I/O error and produced no keep entries is ambiguous;
+       an empty keep-set would delete the whole destination.  Refuse to delete
+       (see the early-timing comment above). */
+    log_message(LOG_LEVEL_ERROR,
+                "source scan hit an I/O error before finding any file; refusing to delete with "
+                "an empty keep-set (--delete)");
+    goto send_fail;
+  }
   if (manifest) {
     /* Late (commit) ordering: all file data is out; transmit the keep-set
        manifest so the receiver deletes only after the transfer succeeds. */
@@ -1562,6 +1604,14 @@ int send_files_multithreaded(Config** config_ptr) {
       bool prebuilt = prepared_ok && scan_paths_only(config, &prepared.options, context->manifest,
                                                      &context->scan_had_io_error);
       prepared_scanner_destroy(&prepared);
+      if (prebuilt && context->scan_had_io_error && context->manifest->size == 0) {
+        /* Empty keep-set + scan I/O error: refusing an empty keep-set manifest
+           would have deleted the whole destination (see send_files). */
+        log_message(LOG_LEVEL_ERROR,
+                    "source scan hit an I/O error before finding any file; refusing to delete "
+                    "with an empty keep-set (--delete)");
+        prebuilt = false;
+      }
       if (!prebuilt) {
         pipeline_context_sender_destroy(context);
         return 1;

src/client/scanner.c

@@ -502,11 +502,20 @@ static int open_next_directory(DirectoryScanner* scanner) {
     if (scanner->current_dir == NULL) {
       scanner->io_error = true;
       log_perror("Could not open directory");
+      /* The transfer ROOT (a sequential scanner's seed directory) must be
+         readable even under --ignore-errors: an unreadable root would produce
+         an empty scan whose keep-set would delete the whole destination.  Only
+         subdirectories discovered during an otherwise-successful root scan are
+         skippable.  (The parallel scanner never reaches this for the root: its
+         root open failure aborts scanner creation; worker seeds are assigned
+         subdirectories with a non-empty relative path and stay skippable.) */
+      bool is_root_seed = scanner->current_rel != NULL && scanner->current_rel[0] == '\0' &&
+                          scanner->current_depth == 0;
       free(scanner->current_rel);
       scanner->current_rel = NULL;
       free(scanner->current_path);
       scanner->current_path = NULL;
-      if (!scanner->ignore_io_errors) {
+      if (!scanner->ignore_io_errors || is_root_seed) {
         scanner->failed = true;
         return -1;
       }

src/shared/utils.h

@@ -37,7 +37,11 @@ typedef struct {
    max_delete is not SIZE_MAX the run is all-or-nothing: extras are counted
    first and DELETE_WALK_LIMIT_EXCEEDED is returned (with nothing removed) when
    the count would exceed the cap.  `deleted_out` optionally receives the number
-   of entries actually removed. */
+   of entries actually removed.  The all-or-nothing guarantee holds only while
+   the destination tree is not being concurrently modified: the rehearsal pass
+   and the delete pass are two separate walks, so a concurrent change between
+   them (another process adding/removing entries) can make the second pass
+   delete a different set than the first one counted. */
 DeleteWalkResult delete_extras_limited(const char* dest_root, ArrayList* manifest,
                                        size_t max_delete, const DeleteSkipEntry* skips,
                                        int skip_count, size_t* deleted_out);

tests/integration/test_features.py

@@ -2136,12 +2136,13 @@ class TestDeletePolicy:
             fh.write(content)
 
     @pytest.mark.parametrize("mt", [False, True])
-    @pytest.mark.parametrize("timing", ["--delete", "--delete-before"])
+    @pytest.mark.parametrize("timing",
+                             ["--delete", "--delete-before", "--delete-after", "--delete-delay"])
     def test_delete_protects_excluded_by_default_and_delete_excluded_removes(self, mt, timing):
-        """rsync parity: with --delete a destination mirror path whose source was
-        excluded survives (protected by default); --delete-excluded opts back
-        into deleting it.  Verified single-threaded, -m, and an early timing
-        run (--delete-before) where the manifest arrives before any data."""
+        """rsync parity: with a --delete timing the destination mirror path whose
+        source was excluded survives (protected by default); --delete-excluded
+        opts back into deleting it.  Verified single-threaded and -m across every
+        timing (commit and early)."""
         source = os.path.join(TEST_DATA_DIR, f"delexcl_{timing.strip('-')}_{mt}_src")
         clean_dir(source)
         entries = {
@@ -2307,8 +2308,34 @@ class TestDeletePolicy:
             assert os.path.isfile(os.path.join(received, "sub")), \
                 "--force did not replace the directory with the file"
             assert _read_file(os.path.join(received, "sub")) == b"now a file\n"
-            assert not os.path.exists(os.path.join(received, "sub", "old.txt")), \
-                "--force left the old directory content behind"
+
+    def test_force_inert_under_delay_updates(self):
+        """Documented divergence: --force acts on the immediate-install path; a
+        --delay-updates run stages into its own tree and its publication renames
+        over regular files only, so a blocking directory is not cleared and the
+        run fails."""
+        source = os.path.join(TEST_DATA_DIR, "force_delay_src")
+        clean_dir(source)
+        self._write(os.path.join(source, "sub", "old.txt"), b"old\n")
+        self._write(os.path.join(source, "keep.txt"), b"kept\n")
+        dest = os.path.join(TEST_DATA_DIR, "force_delay_dst")
+        clean_dir(dest)
+        with ServerManager() as server:
+            server.start(extra_args=["--allow-delete"])
+            result, _ = run_client(source, dest, port=server.port)
+            assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
+            received = get_dest_received_dir(dest, source)
+            os.unlink(os.path.join(source, "sub", "old.txt"))
+            os.rmdir(os.path.join(source, "sub"))
+            self._write(os.path.join(source, "sub"), b"now a file\n")
+            result, _ = run_client(source, dest, flags=["--force", "--delay-updates"],
+                                   port=server.port)
+            assert result.returncode != 0, \
+                "--force --delay-updates unexpectedly replaced the blocking directory"
+            assert os.path.isdir(os.path.join(received, "sub")), \
+                "blocking directory was cleared although --delay-updates should keep --force inert"
+            assert os.path.exists(os.path.join(received, "sub", "old.txt")), \
+                "blocking directory content was lost"
 
     @pytest.mark.parametrize("mt", [False, True])
     def test_prune_empty_dirs_dirs_mode(self, mt):
@@ -2396,14 +2423,22 @@ class TestDeletePolicy:
             assert os.path.exists(os.path.join(received, "a", "keep.log")), \
                 "excluded file mirror was deleted under --delete (rsync protects it)"
 
-    def test_ignore_errors_keeps_deletion_active_on_scan_error(self):
-        """A source I/O error (unreadable directory) aborts the run so no
+    def _run_client_as_nobody(self, source, dest, port, flags):
+        cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
+                            "--save-to-disk", "--server-port", str(port)] + flags
+        return subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
+                               "--clear-groups"] + cmd, text=True, capture_output=True)
+
+    @pytest.mark.parametrize("mt", [False, True])
+    def test_ignore_errors_keeps_deletion_active_on_scan_error(self, mt):
+        """A source I/O error (unreadable subdirectory) aborts the run so no
         deletion happens by default; --ignore-errors continues, still transfers
-        the readable tree and still deletes.  Run as an unprivileged user so the
-        mode-000 directory is genuinely unreadable."""
+        the readable tree and still deletes, single-threaded and under -m.  Run
+        as an unprivileged user so the mode-000 directory is genuinely
+        unreadable."""
         if os.geteuid() != 0 or shutil.which("setpriv") is None:
             pytest.skip("requires root + setpriv to drop privileges for the client")
-        tag = f"ioerr_{os.getpid()}"
+        tag = f"ioerr_{os.getpid()}_{mt}"
         source = os.path.join(TEST_DATA_DIR, f"{tag}_src")
         clean_dir(source)
         self._write(os.path.join(source, "top.txt"), b"top\n")
@@ -2422,29 +2457,86 @@ class TestDeletePolicy:
 
                 # Default: scan error aborts the run; nothing is deleted.
                 self._write(os.path.join(received, "extra.txt"), b"extra\n")
-                cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
-                                    "--save-to-disk", "--server-port", str(server.port),
-                                    "--delete"]
-                result = subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
-                                         "--clear-groups"] + cmd, text=True, capture_output=True)
+                flags = ["--delete"] + (["-m"] if mt else [])
+                result = self._run_client_as_nobody(source, dest, server.port, flags)
                 assert result.returncode != 0, "unreadable source dir did not fail the run"
                 assert os.path.exists(os.path.join(received, "extra.txt")), \
                     "default run deleted although the scan hit an I/O error"
 
                 # --ignore-errors: the readable tree transfers, deletion still runs.
                 self._write(os.path.join(received, "extra.txt"), b"extra\n")
-                cmd = CLIENT_CMD + ["--source-dir", source, "--dest-dir", dest,
-                                    "--save-to-disk", "--server-port", str(server.port),
-                                    "--delete", "--ignore-errors"]
-                result = subprocess.run(["setpriv", "--reuid=65534", "--regid=65534",
-                                         "--clear-groups"] + cmd, text=True, capture_output=True)
+                flags = ["--delete", "--ignore-errors"] + (["-m"] if mt else [])
+                result = self._run_client_as_nobody(source, dest, server.port, flags)
                 assert not os.path.exists(os.path.join(received, "extra.txt")), \
                     f"--ignore-errors did not keep deletion active: {result.stderr[:300]}"
                 assert not os.path.exists(os.path.join(received, "locked")), \
-                    "mirror of the unreadable dir was not treated as an extra"
+                    "mirror of the unreadable dir was left behind (should be an extra)"
             finally:
                 os.chmod(os.path.join(source, "locked"), 0o755)
 
+    @pytest.mark.parametrize("mt", [False, True])
+    @pytest.mark.parametrize("timing", ["--delete", "--delete-before"])
+    def test_ignore_errors_unreadable_root_never_deletes(self, mt, timing):
+        """An unreadable SOURCE ROOT must never be treated as a skippable scan
+        error: with --ignore-errors the sequential scanner treats the root as
+        fatal (matching the -m path, which cannot even create its scanner), so
+        no empty keep-set manifest is sent and the destination is never wiped.
+        Run as an unprivileged user so the mode-000 root is genuinely
+        unreadable."""
+        if os.geteuid() != 0 or shutil.which("setpriv") is None:
+            pytest.skip("requires root + setpriv to drop privileges for the client")
+        tag = f"rootio_{os.getpid()}_{mt}_{timing.strip('-')}"
+        source = os.path.join(TEST_DATA_DIR, f"{tag}_src")
+        clean_dir(source)
+        self._write(os.path.join(source, "file.txt"), b"content\n")
+        dest = os.path.join(TEST_DATA_DIR, f"{tag}_dst")
+        clean_dir(dest)
+        with ServerManager() as server:
+            server.start(extra_args=["--allow-delete"])
+            result, _ = run_client(source, dest, port=server.port)
+            assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
+            received = get_dest_received_dir(dest, source)
+            try:
+                os.chmod(source, 0)
+                self._write(os.path.join(received, "extra.txt"), b"extra\n")
+                flags = [timing, "--ignore-errors"] + (["-m"] if mt else [])
+                result = self._run_client_as_nobody(source, dest, server.port, flags)
+                assert result.returncode != 0, \
+                    f"unreadable source root with {timing} (mt={mt}) unexpectedly succeeded"
+                assert os.path.exists(os.path.join(received, "file.txt")), \
+                    f"{timing} (mt={mt}) wiped a kept destination file"
+                assert os.path.exists(os.path.join(received, "extra.txt")), \
+                    f"{timing} (mt={mt}) deleted the extra although the scan could not read the root"
+            finally:
+                os.chmod(source, 0o755)
+
+    def test_delete_excluded_protection_is_sender_derived(self):
+        """Plain --delete protects destination mirrors of files the SOURCE scan
+        excluded, but a destination-only file that merely matches an exclude
+        rule is still an extra and is removed (protection never re-applies rules
+        to the destination)."""
+        source = os.path.join(TEST_DATA_DIR, "senderderived_src")
+        clean_dir(source)
+        self._write(os.path.join(source, "keep.txt"), b"kept\n")
+        self._write(os.path.join(source, "secret.log"), b"secret\n")
+        dest = os.path.join(TEST_DATA_DIR, "senderderived_dst")
+        clean_dir(dest)
+        with ServerManager() as server:
+            server.start(extra_args=["--allow-delete"])
+            result, _ = run_client(source, dest, port=server.port)
+            assert result.returncode == 0, f"seed sync failed: {result.stderr[:200]}"
+            received = get_dest_received_dir(dest, source)
+            # A destination-only file that happens to match the exclude rule.
+            self._write(os.path.join(received, "stray.log"), b"never on the source\n")
+            result, _ = run_client(source, dest, flags=["--exclude", "*.log", "--delete"],
+                                   port=server.port)
+            assert result.returncode == 0, \
+                f"delete sync failed: {(result.stderr or result.stdout)[:300]}"
+            assert os.path.exists(os.path.join(received, "secret.log")), \
+                "source-excluded mirror was deleted under plain --delete"
+            assert not os.path.exists(os.path.join(received, "stray.log")), \
+                "destination-only file matching the exclude rule was left (should be deleted)"
+
 
 def _pin_mtime(path, ts):
     os.utime(path, (ts, ts))

tests/test_shared_utils.c

@@ -205,6 +205,53 @@ static void test_walker_unlimited_deletes_all() {
   free(root);
 }
 
+/* The 100000-entry server hard bound (MAX_SERVER_DELETE_COUNT, which this test
+   exercises through a literal to avoid reaching into file_receive.c) is also
+   all-or-nothing: a destination holding more extras than the bound must be left
+   completely untouched.  Skipped under valgrind: 100k file creations would be
+   far too slow under instrumentation. */
+static void test_walker_hard_bound_all_or_nothing() {
+  if (is_running_under_valgrind())
+    return;
+  enum { HARD_BOUND = 100000 };
+  char* root = make_walk_root("hardbound");
+  EXPECT_NOT_NULL(root);
+  int rootfd = open(root, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
+  EXPECT_TRUE(rootfd >= 0);
+  bool created = rootfd >= 0;
+  for (int i = 0; created && i < HARD_BOUND + 1; i++) {
+    char name[32];
+    snprintf(name, sizeof(name), "f%d", i);
+    int fd = openat(rootfd, name, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+    if (fd < 0)
+      created = false;
+    else
+      close(fd);
+  }
+  EXPECT_TRUE(created);
+  const char* keeps[1] = {NULL};
+  ArrayList* manifest = make_manifest_strings(keeps, 0);
+  EXPECT_NOT_NULL(manifest);
+  size_t deleted = 999;
+  DeleteWalkResult result = delete_extras_limited(root, manifest, HARD_BOUND, NULL, 0, &deleted);
+  EXPECT_EQ_INT((int)result, (int)DELETE_WALK_LIMIT_EXCEEDED);
+  EXPECT_EQ_INT((int)deleted, 0);
+  EXPECT_TRUE(file_exists(root, "f0"));
+  EXPECT_TRUE(file_exists(root, "f100000"));
+  array_list_delete(manifest);
+  /* Fast cleanup: unlink every created name through the still-open root fd. */
+  if (rootfd >= 0) {
+    for (int i = 0; i < HARD_BOUND + 1; i++) {
+      char name[32];
+      snprintf(name, sizeof(name), "f%d", i);
+      (void)unlinkat(rootfd, name, 0);
+    }
+    close(rootfd);
+  }
+  rmdir(root);
+  free(root);
+}
+
 typedef struct {
   bool eight_bit_output;
   const char* expected;
@@ -227,6 +274,7 @@ void test_shared_utils() {
   test_walker_max_delete_exceeded_deletes_nothing();
   test_walker_max_delete_exact_bound_deletes();
   test_walker_unlimited_deletes_all();
+  test_walker_hard_bound_all_or_nothing();
 
   char formatted[32];
   EXPECT_TRUE(format_human_bytes(0, formatted, sizeof(formatted)));