Merge branch 'feat/transport-g2' into feat/transport-xattr

src/shared/file_save.c

@@ -926,12 +926,14 @@ static FileSaveResult file_save_symlink_to_disk(const FileSavePlan* plan, bool*
   /* -X/-A: apply the symlink's OWN xattrs with a no-follow primitive.  The
      confined parent directory is the anchor and the final component is applied
      with lsetxattr, so the referent is never touched.  Best-effort: on Linux
-     the VFS refuses xattrs on symlinks, so this is normally a no-op. */
+     the VFS refuses xattrs on symlinks, so this is normally a no-op.  Hoist the
-  if (ok && config && config->use_xattrs && file->xattrs) {
+     empty-list check so the common Linux case (NULL/empty xattrs) does not pay
+     an open/close of the parent per symlink. */
+  if (ok && config && config->use_xattrs && file->xattrs && file->xattrs->count > 0) {
     char* leaf = NULL;
     int parent_fd = file_open_secure_parent(link_path, &leaf, false);
     if (parent_fd >= 0) {
-      xattr_apply_path_nofollow(parent_fd, leaf, file->xattrs);
+      xattr_apply_path_nofollow(parent_fd, leaf, file->xattrs, config->preserve_acls);
       close(parent_fd);
     }
     free(leaf);

src/shared/xattr.c

@@ -383,8 +383,17 @@ bool xattr_apply_fd(int fd, const FileXattrList* list) {
  * referent.  fsetxattr cannot be used (no *at xattr syscall exists, and the
  * kernel rejects xattr syscalls on an O_PATH descriptor), so the already-open,
  * confinement-checked parent directory is addressed through /proc/self/fd and
- * the final component is applied with lsetxattr, which does not follow it. */
+ * the final component is applied with lsetxattr, which does not follow it.
-bool xattr_apply_path_nofollow(int parent_fd, const char* leaf, const FileXattrList* list) {
+ *
+ * The list is trusted to come from xattr_receive() (already whitelisted), but
+ * every name is re-validated here so this path-based primitive is confined on
+ * its own -- this is the only apply primitive that addresses a path, and the
+ * header promises a whitelisted apply.  The apply is best-effort: if /proc is
+ * not mounted (the anchor cannot be formed) or the kernel refuses the set, the
+ * failure is skipped and never fails the transfer.  See xattr.h for the bounded
+ * residual TOCTOU between link creation and lsetxattr. */
+bool xattr_apply_path_nofollow(int parent_fd, const char* leaf, const FileXattrList* list,
+                               bool preserve_acls) {
   if (parent_fd < 0 || !leaf || leaf[0] == '\0' || strchr(leaf, '/') != NULL || !list)
     return false;
   if (list->count == 0)
@@ -403,9 +412,10 @@ bool xattr_apply_path_nofollow(int parent_fd, const char* leaf, const FileXattrL
   int first_errno = 0;
   for (int i = 0; i < list->count; i++) {
     const FileXattr* xa = &list->items[i];
-    /* Defense in depth: even a hand-crafted list can never apply the reserved
+    /* Defense in depth: re-validate against the receiver's full whitelist, so a
-       --fake-super key (only fake_super_store_fd may write it). */
+       hand-crafted list can never apply a privileged namespace or the reserved
-    if (strcmp(xa->name, FAKESUPER_XATTR) == 0)
+       --fake-super key through this path-based primitive. */
+    if (!xattr_name_appliable(xa->name, preserve_acls))
       continue;
     if (lsetxattr(path, xa->name, xa->value, xa->value_len, 0) != 0) {
       if (!warned) {

src/shared/xattr.h

@@ -110,16 +110,32 @@ bool xattr_apply_fd(int fd, const FileXattrList* list);
 
 /* Receiver: apply every entry to the symlink named by (parent_fd, leaf) WITHOUT
  * following it, via lsetxattr() on the confined path
- * "/proc/self/fd/<parent_fd>/<leaf>".  A symlink cannot be targeted by the
+ * "/proc/self/fd/<parent_fd>/<leaf>".  Every incoming name is independently
- * fd-relative fsetxattr() path: there is no *at() xattr syscall and the kernel
+ * re-validated against xattr_name_appliable() with `preserve_acls`, exactly like
- * rejects xattr syscalls on an O_PATH descriptor, so the already-opened,
+ * xattr_apply_fd(): a non-whitelisted namespace (including the reserved
+ * --fake-super key) is skipped, so this primitive stays confined even if handed
+ * a hand-crafted list.  A symlink cannot be targeted by the fd-relative
+ * fsetxattr() path: there is no *at() xattr syscall and the kernel rejects
+ * xattr syscalls on an O_PATH descriptor, so the already-opened,
  * confinement-checked parent directory is the anchor and only the final
  * component is the (no-follow) link.  `leaf` must be a single path component.
- * Best-effort exactly like xattr_apply_fd(): a per-attribute failure (on Linux
+ *
- * every set on a symlink fails with EPERM) is logged once and skipped, never
+ * Portability: the "/proc/self/fd/<parent_fd>" anchor requires a mounted /proc.
- * fatal.  Returns false only for an invalid anchor/list; true when an apply was
+ * Where /proc is unavailable (or the fd cannot be addressed that way) the
- * attempted.  The reserved --fake-super key is never applied. */
+ * lsetxattr simply fails and is skipped -- the apply is best-effort exactly like
-bool xattr_apply_path_nofollow(int parent_fd, const char* leaf, const FileXattrList* list);
+ * xattr_apply_fd(), so no error is propagated and the transfer continues.  A
+ * per-attribute failure (on Linux every set on a symlink fails with EPERM) is
+ * logged once and skipped, never fatal.  Returns false only for an invalid
+ * anchor/list; true when an apply was attempted.
+ *
+ * Residual TOCTOU: `leaf` is a caller-supplied name resolved by path in the
+ * parent, so a local writer could replace the just-created symlink between its
+ * creation and lsetxattr().  This is bounded: it requires write access to the
+ * confinement-checked destination directory (already trusted), can only install
+ * a whitelisted user namespace or POSIX-ACL name, and never follows the link (a
+ * replacement symlink is still applied to as the final, no-follow component). */
+bool xattr_apply_path_nofollow(int parent_fd, const char* leaf, const FileXattrList* list,
+                               bool preserve_acls);
 
 /* --fake-super: write the source uid/gid/mode/rdev record into the reserved
  * FAKESUPER_XATTR on `fd`, using rsync 3.4.1's exact grammar (see the key

tests/integration/test_features.py

@@ -6675,13 +6675,25 @@ class TestExtendedAttributes:
         assert os.path.islink(dst_link), "destination link entry is not a symlink"
         assert os.readlink(dst_link) == "target.txt"
 
-        # The no-follow guarantee: the referent's attribute must never leak onto
+        # The no-follow guarantee.  Checking only the link's own xattr list is
-        # the symlink entry.
+        # vacuous on Linux (lsetxattr on a symlink always fails EPERM), so also
+        # prove the apply never followed the link: the destination REFERENT must
+        # keep its own user.* value untouched.
+        dst_target = os.path.join(received, "target.txt")
+        assert os.getxattr(dst_target, "user.referent-only") == b"referent-value", (
+            "the destination symlink apply followed the link and rewrote the "
+            "referent's xattr"
+        )
         link_names = os.listxattr(dst_link, follow_symlinks=False)
         assert "user.referent-only" not in link_names, (
             "the destination symlink captured its REFERENT's xattr "
             "(path-following capture bug)"
         )
+        if sys.platform.startswith("linux"):
+            assert link_names == [], (
+                "Linux associates no xattrs with a symlink; the link entry must "
+                "carry none"
+            )
         if link_xattr_supported:
             assert os.getxattr(
                 dst_link, "user.link-own", follow_symlinks=False

tests/test_xattr.c

@@ -8,6 +8,7 @@
 #include "identity.h"
 #include "metadata.h"
 #include "protocol.h"
+#include "scanner_internal.h"
 #include "test_utils.h"
 #include <fcntl.h>
 #include <stdint.h>
@@ -741,16 +742,16 @@ static void test_xattr_apply_path_nofollow_does_not_follow() {
   EXPECT_TRUE(xattr_list_append(list, "user.added", "x", 1));
 
   /* Invalid anchors are refused before any syscall (no fd/leaf/list). */
-  EXPECT_FALSE(xattr_apply_path_nofollow(-1, "link", list));
+  EXPECT_FALSE(xattr_apply_path_nofollow(-1, "link", list, false));
-  EXPECT_FALSE(xattr_apply_path_nofollow(0, "", list));
+  EXPECT_FALSE(xattr_apply_path_nofollow(0, "", list, false));
-  EXPECT_FALSE(xattr_apply_path_nofollow(0, "a/b", list));
+  EXPECT_FALSE(xattr_apply_path_nofollow(0, "a/b", list, false));
-  EXPECT_FALSE(xattr_apply_path_nofollow(0, "link", NULL));
+  EXPECT_FALSE(xattr_apply_path_nofollow(0, "link", NULL, false));
 
   /* The confined parent directory is the anchor; the final component is the
      link.  Best-effort: returns true even when the kernel refuses. */
   int dir_fd = open(root, O_RDONLY | O_DIRECTORY);
   EXPECT_TRUE(dir_fd >= 0);
-  EXPECT_TRUE(xattr_apply_path_nofollow(dir_fd, "link", list));
+  EXPECT_TRUE(xattr_apply_path_nofollow(dir_fd, "link", list, false));
   close(dir_fd);
 
   /* The referent must be untouched: a following apply would have set user.orig
@@ -777,6 +778,57 @@ static void test_xattr_apply_path_nofollow_does_not_follow() {
   rmdir(root);
 }
 
+/* Protocol 2.29.0 scanner wiring: scanner_capture_xattrs() must choose the
+ * NO-FOLLOW capture for a symlink entry, so the link's FileXattrList never
+ * carries the REFERENT's user.* attributes.  xattr_capture_path_nofollow() is
+ * already covered directly above; this exercises the scanner CALL SITE, which is
+ * what makes the no-follow variant actually reach symlink entries.  If the
+ * scanner regressed to the path-following capture, file->xattrs would contain
+ * user.symref and this test fails.  Guarded on filesystem xattr support. */
+static void test_scanner_symlink_capture_is_nofollow() {
+  const char* target = "test_scanner_symlink_xattr_target";
+  const char* link = "test_scanner_symlink_xattr_link";
+  unlink(link);
+  unlink(target);
+  int fd = open(target, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+  if (fd < 0)
+    return;
+  bool has_xattr = setxattr(target, "user.symref", "referent", 8, 0) == 0;
+  close(fd);
+  if (!has_xattr) {
+    unlink(target);
+    return; /* filesystem without xattr support */
+  }
+  if (symlink(target, link) != 0) {
+    unlink(target);
+    return;
+  }
+
+  DirectoryScanner scanner;
+  memset(&scanner, 0, sizeof(scanner));
+  scanner.options.preserve_xattrs = true;
+  File* file = file_create(link);
+  EXPECT_NOT_NULL(file);
+  file->is_symlink = true;
+
+  scanner_capture_xattrs(&scanner, file);
+
+  /* The referent's attribute must not appear on the symlink's captured list. */
+  bool leaked = false;
+  for (int i = 0; file->xattrs && i < file->xattrs->count; i++) {
+    if (strcmp(file->xattrs->items[i].name, "user.symref") == 0)
+      leaked = true;
+  }
+  EXPECT_FALSE(leaked);
+  /* On Linux the VFS associates no xattrs with a symlink, so the capture is
+     NULL (never an empty-but-valid list). */
+  EXPECT_NULL(file->xattrs);
+
+  file_destroy(file);
+  unlink(link);
+  unlink(target);
+}
+
 /* Protocol 2.29.0: a STATUS_SYMLINK frame followed by an -X/-A xattr block is
  * decoded by file_receive_symlink() with the block attached to the File.  This
  * is the wire round-trip for the new trailing symlink xattr block. */
@@ -843,6 +895,7 @@ static void test_symlink_frame_carries_xattrs() {
 void test_xattr() {
   test_xattr_list_clone();
   test_xattr_capture_symlink_nofollow();
+  test_scanner_symlink_capture_is_nofollow();
   test_xattr_apply_path_nofollow_does_not_follow();
   test_symlink_frame_carries_xattrs();
   test_xattr_wire_roundtrip();