feat(xattr): carry and apply symlink xattrs (protocol 2.29.0)

tests/integration/test_fault_injection.py

@@ -36,7 +36,7 @@ from common import (  # noqa: E402
     verify_transfer,
 )
 
-PROTOCOL_VERSION = b"2.28.0"
+PROTOCOL_VERSION = b"2.29.0"
 STATUS_MANIFEST = 5
 STATUS_OK = 0
 

tests/integration/test_features.py

@@ -6637,6 +6637,56 @@ class TestExtendedAttributes:
         received = get_dest_received_dir(dest, source)
         assert os.getxattr(os.path.join(received, "data.txt"), "user.k") == b"v"
 
+    @pytest.mark.ci
+    def test_symlink_own_xattrs_never_referent(self, shared_server):
+        """Protocol 2.29.0: a symlink's STATUS_SYMLINK frame carries a trailing
+        xattr block captured with llistxattr/lgetxattr (no follow) and applied
+        with lsetxattr on the link itself.  Linux's VFS refuses to associate
+        xattrs with a symlink at all, so the portable guarantee asserted here is
+        the no-follow one: a referent that carries user.* must NOT have those
+        attributes appear on the destination symlink entry (the old
+        path-following capture would have copied the referent's attrs onto the
+        link).  On a platform/filesystem that does support symlink xattrs the
+        full round-trip of the link's own attribute is asserted too."""
+        source, dest = self._source_and_dest("symlink_xattr")
+        target = os.path.join(source, "target.txt")
+        with open(target, "wb") as fh:
+            fh.write(b"referent payload\n")
+        if not _xattr_supported(target):
+            pytest.skip("filesystem does not support user xattrs")
+        os.setxattr(target, "user.referent-only", b"referent-value")
+
+        link = os.path.join(source, "link")
+        os.symlink("target.txt", link)
+        link_xattr_supported = False
+        try:
+            os.setxattr(link, "user.link-own", b"link-value", follow_symlinks=False)
+            link_xattr_supported = os.getxattr(
+                link, "user.link-own", follow_symlinks=False
+            ) == b"link-value"
+        except (OSError, AttributeError, NotImplementedError):
+            link_xattr_supported = False
+
+        result, _ = run_client(source, dest, flags=["-aX"], port=shared_server.port)
+        assert result.returncode == 0, \
+            f"-aX symlink sync failed: {(result.stderr or result.stdout)[:300]}"
+        received = get_dest_received_dir(dest, source)
+        dst_link = os.path.join(received, "link")
+        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 symlink entry.
+        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 link_xattr_supported:
+            assert os.getxattr(
+                dst_link, "user.link-own", follow_symlinks=False
+            ) == b"link-value", "the symlink's own xattr did not round-trip"
+
     @pytest.mark.ci
     def test_acls_via_posix_acl_xattr(self, shared_server):
         source, dest = self._source_and_dest("acl")

tests/integration/test_preflight.py

@@ -133,14 +133,14 @@ def _seed_protocol_source(source):
 class TestProtocol:
     @pytest.mark.ci
     def test_protocol_current_version_accepted(self, shared_server):
-        """--protocol=2.28.0 (the current PROTOCOL_VERSION) is accepted and the
+        """--protocol=2.29.0 (the current PROTOCOL_VERSION) is accepted and the
         transfer completes normally."""
         source = os.path.join(TEST_DATA_DIR, "proto_ok_src")
         dest = os.path.join(TEST_DATA_DIR, "proto_ok_dst")
         shutil.rmtree(dest, ignore_errors=True)
         os.makedirs(dest)
         _seed_protocol_source(source)
-        result, _ = run_client(source, dest, flags=["--protocol=2.28.0"],
+        result, _ = run_client(source, dest, flags=["--protocol=2.29.0"],
                                port=shared_server.port)
         assert result.returncode == 0, \
             f"--protocol current run failed: {(result.stderr or result.stdout)[:400]}"

tests/test_client_cli.c

@@ -352,7 +352,7 @@ static void test_parse_args_protocol_accept_current() {
   Config* cfg = valid_client_config();
   EXPECT_NOT_NULL(cfg);
   char* argv_equals[] = {"fastsync",   "--source-dir", "/src",
-                         "--dest-dir", "/dst",         "--protocol=2.28.0"};
+                         "--dest-dir", "/dst",         "--protocol=2.29.0"};
   int positional_args[2];
   int positional_count = 0;
   EXPECT_EQ_INT(parse_args(cfg, 6, argv_equals, positional_args, &positional_count), 0);
@@ -362,7 +362,7 @@ static void test_parse_args_protocol_accept_current() {
   cfg = valid_client_config();
   EXPECT_NOT_NULL(cfg);
   char* argv_space[] = {"fastsync", "--source-dir", "/src",  "--dest-dir",
-                        "/dst",     "--protocol",   "2.28.0"};
+                        "/dst",     "--protocol",   "2.29.0"};
   positional_count = 0;
   EXPECT_EQ_INT(parse_args(cfg, 7, argv_space, positional_args, &positional_count), 0);
   EXPECT_EQ_STR(cfg->version, PROTOCOL_VERSION);
@@ -375,7 +375,7 @@ static void test_parse_args_protocol_rejects_other_versions() {
   static const char* const bad_versions[] = {"2.17",   "2.16",   "2.15.0", "2.16.0", "2.17.0",
                                              "2.18.0", "2.19.0", "2.20.0", "2.21.0", "2.22.0",
                                              "2.23.0", "2.24.0", "2.25.0", "2.26.0", "2.27.0",
-                                             "216",    "31",     "abc",    ""};
+                                             "2.28.0", "216",    "31",     "abc",    ""};
   for (size_t i = 0; i < sizeof(bad_versions) / sizeof(bad_versions[0]); i++) {
     Config* cfg = valid_client_config();
     EXPECT_NOT_NULL(cfg);

tests/test_config.c

@@ -2924,7 +2924,7 @@ static void golden_config_populate(Config* c) {
   array_list_add(c->filters, str_dup("- /sub/dir/"));
 }
 
-/* The pinned golden frame (protocol 2.28.0).  The values below are the only
+/* The pinned golden frame (protocol 2.29.0).  The values below are the only
  * thing that ties the generated table to the historical wire format; update
  * them ONLY with a PROTOCOL_VERSION bump and a documented reason.  The 2.24.0
  * delete-plan wave changed only the version string; 2.25.0 appended the
@@ -2933,10 +2933,13 @@ static void golden_config_populate(Config* c) {
  * appended the receiver-side delete-protection rule block (the STATUS_STATS
  * body also grew, but that is not part of this frame).  Track 5a appends the
  * FastSync-only verify_basis bool to the basis block WITHOUT a version bump
- * (project decision), so the frame grew by one int to 886 bytes.  The
- * byte-exact values are recomputed for the merged layout. */
+ * (project decision), so the frame grew by one int to 886 bytes.  The 2.29.0
+ * symlink-xattr wave changes only the version string: the config-frame layout
+ * is unchanged (use_xattrs already crosses the wire); the STATUS_SYMLINK frame
+ * body grows instead.  The byte-exact values are recomputed for the merged
+ * layout. */
 #define GOLDEN_WIRE_LEN 886
-#define GOLDEN_WIRE_HASH 5809509022716816757ULL
+#define GOLDEN_WIRE_HASH 17827864270611927842ULL
 
 static unsigned long long fnv1a_64(const unsigned char* buf, size_t len) {
   unsigned long long h = 1469598103934665603ULL;
@@ -3018,7 +3021,7 @@ static unsigned long long capture_wire_hash(const Config* cfg, size_t* out_len)
   return h;
 }
 
-/* Byte-for-byte wire compatibility guard (protocol 2.28.0).  The expected hash
+/* Byte-for-byte wire compatibility guard (protocol 2.29.0).  The expected hash
  * pins the pre-X-macro byte stream; the refactor MUST NOT change it. */
 static void test_config_wire_golden() {
   if (is_running_under_valgrind())

tests/test_xattr.c

@@ -1,9 +1,12 @@
 #include "test_xattr.h"
 #include "xattr.h"
+#include "charset.h"
 #include "config.h"
 #include "file.h"
+#include "file_receive.h"
 #include "file_save.h"
 #include "identity.h"
+#include "metadata.h"
 #include "protocol.h"
 #include "test_utils.h"
 #include <fcntl.h>
@@ -11,6 +14,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/wait.h>
 #include <sys/xattr.h>
@@ -660,8 +664,187 @@ static void test_file_save_directory_applies_xattrs() {
   rmdir(root);
 }
 
+/* Symlink xattrs (protocol 2.29.0): the no-follow capture must read the LINK's
+ * OWN attributes and never the REFERENT's.  Linux's VFS refuses to associate
+ * xattrs with a symlink at all, so the nofollow capture returns NULL while the
+ * path-following capture sees the referent's attribute -- which is exactly the
+ * bug the no-follow variant exists to prevent (a symlink entry must not carry
+ * its target's attributes).  Guarded on filesystem xattr support. */
+static void test_xattr_capture_symlink_nofollow() {
+  const char* target = "test_symlink_xattr_capture_target";
+  const char* link = "test_symlink_xattr_capture_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;
+  }
+
+  /* The no-follow capture must never pick up the referent's attributes. */
+  FileXattrList* nofollow = xattr_capture_path_nofollow(link, false);
+  EXPECT_NULL(nofollow);
+
+  /* The path-following capture does, proving the referent really carries one
+     and that the no-follow variant differs. */
+  FileXattrList* follow = xattr_capture_path(link, false);
+  bool saw = false;
+  for (int i = 0; follow && i < follow->count; i++) {
+    if (strcmp(follow->items[i].name, "user.symref") == 0)
+      saw = true;
+  }
+  EXPECT_TRUE(saw);
+  xattr_list_free(follow);
+  xattr_list_free(nofollow);
+  unlink(link);
+  unlink(target);
+}
+
+/* Symlink xattrs (protocol 2.29.0): the no-follow apply must target the LINK,
+ * never its referent.  On Linux the LSETXATTR is refused (the VFS does not
+ * allow symlink xattrs), but the critical guarantee is observable: the
+ * referent's attributes are UNCHANGED.  A regression from lsetxattr to the
+ * path-following setxattr would rewrite the referent here and fail this test. */
+static void test_xattr_apply_path_nofollow_does_not_follow() {
+  const char* root = "test_symlink_xattr_apply_tmp";
+  const char* target = "test_symlink_xattr_apply_tmp/target";
+  const char* link = "test_symlink_xattr_apply_tmp/link";
+  unlink(link);
+  unlink(target);
+  rmdir(root);
+  EXPECT_EQ_INT(mkdir(root, 0700), 0);
+  int tfd = open(target, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+  if (tfd < 0) {
+    rmdir(root);
+    return;
+  }
+  bool has_xattr = setxattr(target, "user.orig", "orig", 4, 0) == 0;
+  close(tfd);
+  if (!has_xattr) {
+    unlink(target);
+    rmdir(root);
+    return; /* filesystem without xattr support */
+  }
+  EXPECT_EQ_INT(symlink("target", link), 0);
+
+  FileXattrList* list = xattr_list_new();
+  EXPECT_NOT_NULL(list);
+  EXPECT_TRUE(xattr_list_append(list, "user.orig", "hacked", 6));
+  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(0, "", list));
+  EXPECT_FALSE(xattr_apply_path_nofollow(0, "a/b", list));
+  EXPECT_FALSE(xattr_apply_path_nofollow(0, "link", NULL));
+
+  /* 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));
+  close(dir_fd);
+
+  /* The referent must be untouched: a following apply would have set user.orig
+     to "hacked" and created user.added on the target. */
+  char buf[16];
+  ssize_t got = getxattr(target, "user.orig", buf, sizeof(buf));
+  EXPECT_EQ_INT(4, (int)got);
+  if (got == 4)
+    EXPECT_TRUE(memcmp(buf, "orig", 4) == 0);
+  EXPECT_TRUE(getxattr(target, "user.added", buf, sizeof(buf)) < 0);
+
+  /* If the platform DOES support symlink xattrs, they must have landed on the
+     link itself; on Linux the VFS refuses them, so the link stays empty. */
+  if (llistxattr(link, NULL, 0) > 0) {
+    ssize_t n = lgetxattr(link, "user.added", buf, sizeof(buf));
+    EXPECT_EQ_INT(1, (int)n);
+    if (n == 1)
+      EXPECT_TRUE(buf[0] == 'x');
+  }
+
+  xattr_list_free(list);
+  unlink(link);
+  unlink(target);
+  rmdir(root);
+}
+
+/* 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. */
+static void run_recv_symlink_with_xattrs(int fd) {
+  /* Stack-allocated so the forked child leaks nothing at _exit() (a
+     config_create() in the parent would be inherited and never freed here). */
+  Config config;
+  memset(&config, 0, sizeof(config));
+  config.use_metadata = true;
+  config.use_xattrs = true;
+  config.preserve_xattrs = true;
+  File* file = file_receive_symlink(fd, &config);
+  if (!file)
+    _exit(1);
+  bool ok = file->is_symlink && file->symlink_target != NULL &&
+            strcmp(file->symlink_target, "target") == 0;
+  ok = ok && file->xattrs != NULL && file->xattrs->count == 1 &&
+       strcmp(file->xattrs->items[0].name, "user.sym") == 0 &&
+       file->xattrs->items[0].value_len == 3 && memcmp(file->xattrs->items[0].value, "sym", 3) == 0;
+  file_destroy(file);
+  _exit(ok ? 0 : 1);
+}
+
+static void test_symlink_frame_carries_xattrs() {
+  int p[2];
+  EXPECT_EQ_INT(socketpair(AF_UNIX, SOCK_STREAM, 0, p), 0);
+  io_set_fds(p[0], p[1]);
+  io_set_bwlimit(0);
+
+  pid_t pid = fork();
+  if (pid == 0) {
+    close(p[1]);
+    io_set_fds(p[0], p[0]);
+    io_set_bwlimit(0);
+    run_recv_symlink_with_xattrs(p[0]);
+  }
+  close(p[0]);
+  io_set_fds(p[1], p[1]);
+  io_set_bwlimit(0);
+
+  FileMetadata m;
+  memset(&m, 0, sizeof(m));
+  m.mode = S_IFLNK | 0777;
+  m.uid = (uint32_t)geteuid();
+  m.gid = (uint32_t)getegid();
+  m.mtime_sec = 1700000000;
+
+  FileXattrList* list = xattr_list_new();
+  EXPECT_NOT_NULL(list);
+  EXPECT_TRUE(xattr_list_append(list, "user.sym", "sym", 3));
+
+  /* Exactly the sender's order: path, target, metadata, xattr block. */
+  bool wrote = send_wire_str(p[1], "link") && send_wire_str(p[1], "target") &&
+               metadata_send(p[1], &m) && xattr_send(p[1], list);
+  xattr_list_free(list);
+  close(p[1]);
+
+  int status = 0;
+  waitpid(pid, &status, 0);
+  EXPECT_TRUE(wrote);
+  EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0);
+}
+
 void test_xattr() {
   test_xattr_list_clone();
+  test_xattr_capture_symlink_nofollow();
+  test_xattr_apply_path_nofollow_does_not_follow();
+  test_symlink_frame_carries_xattrs();
   test_xattr_wire_roundtrip();
   test_xattr_reject_privileged_namespace();
   test_xattr_reject_oversized_value();