fix: address update option review findings

src/shared/file.c

@@ -183,15 +183,29 @@ bool file_stat_secure(const char* path, struct stat* st) {
   int parent_fd = file_open_secure_parent(path, &leaf, false);
   if (parent_fd < 0)
     return false;
-  int fd = openat(parent_fd, leaf, O_RDONLY | O_NONBLOCK | O_CLOEXEC | O_NOFOLLOW);
-  bool exists = fd >= 0 && fstat(fd, st) == 0 && S_ISREG(st->st_mode);
-  if (fd >= 0)
-    close(fd);
+  bool exists = fstatat(parent_fd, leaf, st, AT_SYMLINK_NOFOLLOW) == 0 && S_ISREG(st->st_mode);
   close(parent_fd);
   free(leaf);
   return exists;
 }
 
+static bool stat_is_newer(const struct stat* st, const FileMetadata* metadata) {
+  if (!st || !metadata)
+    return false;
+#ifdef __linux__
+  long mtime_nsec = st->st_mtim.tv_nsec;
+#else
+  long mtime_nsec = 0;
+#endif
+  return st->st_mtime > metadata->mtime_sec ||
+         (st->st_mtime == metadata->mtime_sec && mtime_nsec > metadata->mtime_nsec);
+}
+
+bool file_destination_is_newer_secure(const char* path, const FileMetadata* metadata) {
+  struct stat st;
+  return file_stat_secure(path, &st) && stat_is_newer(&st, metadata);
+}
+
 int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs) {
   char* copy = str_dup(path);
   if (!copy)
@@ -302,8 +316,9 @@ bool file_rename_secure(const char* old_path, const char* new_path) {
   return ok;
 }
 
-bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
-                         bool inplace, bool sparse, const FileMetadata* metadata) {
+static bool file_to_disk_secure_impl(const char* path, const void* data,
+                                     unsigned long long data_size, bool inplace, bool sparse,
+                                     const FileMetadata* metadata, bool update) {
   char* leaf = NULL;
   int dirfd = file_open_secure_parent(path, &leaf, true);
   if (dirfd < 0)
@@ -311,15 +326,37 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long
   int fd = -1;
   bool ok = false;
   if (inplace) {
-    fd = openat(dirfd, leaf, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW, 0644);
+    fd = openat(dirfd, leaf, O_WRONLY | O_CREAT | O_CLOEXEC | O_NOFOLLOW, 0644);
     if (fd >= 0) {
-      if (!sparse || data_size == 0 || ftruncate(fd, (off_t)data_size) == 0)
-        ok = write_all(fd, data, data_size);
-      if (ok && metadata)
-        ok = file_restore_metadata_fd(fd, metadata);
+      struct stat destination_stat;
+      bool newer = false;
+      if (update && metadata && fstat(fd, &destination_stat) == 0 &&
+          S_ISREG(destination_stat.st_mode)) {
+        newer = stat_is_newer(&destination_stat, metadata);
+      }
+      if (newer) {
+        ok = true;
+      } else {
+        if (ftruncate(fd, 0) == 0 &&
+            (!sparse || data_size == 0 || ftruncate(fd, (off_t)data_size) == 0))
+          ok = write_all(fd, data, data_size);
+        if (ok && metadata)
+          ok = file_restore_metadata_fd(fd, metadata);
+      }
     }
   } else {
     char tmp[NAME_MAX];
+    if (update && metadata) {
+      /* This check protects the normal atomic path as far as possible.  A
+         concurrent replacement can still occur before the final rename. */
+      struct stat destination_stat;
+      if (fstatat(dirfd, leaf, &destination_stat, AT_SYMLINK_NOFOLLOW) == 0 &&
+          S_ISREG(destination_stat.st_mode) && stat_is_newer(&destination_stat, metadata)) {
+        close(dirfd);
+        free(leaf);
+        return true;
+      }
+    }
     for (unsigned int i = 0; i < 100 && !ok; ++i) {
       snprintf(tmp, sizeof(tmp), ".%s.tmp.%ld.%u", leaf, (long)getpid(), i);
       fd = openat(dirfd, tmp, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC | O_NOFOLLOW, 0600);
@@ -347,6 +384,16 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long
   return ok;
 }
 
+bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
+                         bool inplace, bool sparse, const FileMetadata* metadata) {
+  return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false);
+}
+
+bool file_to_disk_secure_update(const char* path, const void* data, unsigned long long data_size,
+                                bool inplace, bool sparse, const FileMetadata* metadata) {
+  return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, true);
+}
+
 bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size,
                         bool inplace, bool sparse) {
   if (!path || (!data && data_size != 0) || has_path_traversal(path))

src/shared/file.h

@@ -27,10 +27,16 @@ bool file_set_authorized_root(int fd, const char* canonical_path);
 /* Secure path/filesystem primitives (symlink-safe, O_NOFOLLOW, root-confined). */
 bool file_path_exists_secure(const char* path);
 bool file_stat_secure(const char* path, struct stat* st);
+bool file_destination_is_newer_secure(const char* path, const FileMetadata* metadata);
 int file_open_secure_parent(const char* path, char** leaf_out, bool create_dirs);
 bool file_ensure_directory_secure(const char* path);
 bool file_rename_secure(const char* old_path, const char* new_path);
 bool file_to_disk_secure(const char* path, const void* data, unsigned long long data_size,
                          bool inplace, bool sparse, const FileMetadata* metadata);
+/* With update enabled, an existing newer destination is left untouched.  The
+   check is descriptor-based for inplace writes; atomic replacement still has
+   an unavoidable final rename race without filesystem locking. */
+bool file_to_disk_secure_update(const char* path, const void* data, unsigned long long data_size,
+                                bool inplace, bool sparse, const FileMetadata* metadata);
 
 #endif

src/shared/file_receive.c

@@ -21,17 +21,6 @@
 #define MAX_SERVER_DELETE_COUNT 100000U
 #define MAX_FILE_DATA_SIZE MAX_RECEIVE_FILE_SIZE
 
-static bool destination_is_newer(const char* path, const FileMetadata* source_metadata) {
-  struct stat destination_stat;
-  if (!source_metadata || !file_stat_secure(path, &destination_stat))
-    return false;
-  time_t destination_sec = destination_stat.st_mtim.tv_sec;
-  long destination_nsec = destination_stat.st_mtim.tv_nsec;
-  return destination_sec > source_metadata->mtime_sec ||
-         (destination_sec == source_metadata->mtime_sec &&
-          destination_nsec > source_metadata->mtime_nsec);
-}
-
 bool file_save_to_disk(const char* root_directory, const File* file, const Config* config) {
   bool backup_enabled = config && config->backup;
   bool inplace = config && config->inplace;
@@ -72,8 +61,9 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi
     return false;
   }
 
-  /* --update is receiver-side policy: never replace a newer destination. */
-  if (config && config->update && destination_is_newer(disk_path, file->metadata)) {
+  /* --update is receiver-side policy: never replace a newer destination.
+     The secure stat does not require read permission on the destination. */
+  if (config && config->update && file_destination_is_newer_secure(disk_path, file->metadata)) {
     free(confined_backup);
     free(confined_partial);
     free(disk_path);
@@ -110,8 +100,11 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi
     }
   }
 
-  bool ok = file_to_disk_secure(disk_path, file->data->data, file->data->size, inplace, sparse,
-                                file->metadata);
+  bool ok = config && config->update
+                ? file_to_disk_secure_update(disk_path, file->data->data, file->data->size, inplace,
+                                             sparse, file->metadata)
+                : file_to_disk_secure(disk_path, file->data->data, file->data->size, inplace,
+                                      sparse, file->metadata);
   free(parent_copy);
   free(backup_path);
   free(confined_backup);

tests/integration/test_features.py

@@ -227,21 +227,42 @@ class TestUpdate:
 
         with open(received_file, "wb") as f:
             f.write(b"newer destination\n")
-        os.utime(received_file, (source_stat.st_mtime + 10, source_stat.st_mtime + 10))
+        os.utime(received_file, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns + 10_000_000_000))
         result, _ = run_client(SOURCE_DIR, DEST_DIR, flags=["-u"], port=shared_server.port)
         assert result.returncode == 0
         with open(received_file, "rb") as f:
             assert f.read() == b"newer destination\n"
 
-        os.utime(received_file, (source_stat.st_atime, source_stat.st_mtime))
+        os.utime(received_file, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns))
         result, _ = run_client(SOURCE_DIR, DEST_DIR, flags=["-u"], port=shared_server.port)
         assert result.returncode == 0
         with open(received_file, "rb") as f:
             assert f.read() == b"hello world\n"
 
+    def test_update_skips_unreadable_newer_destination(self, shared_server):
+        clean_dir(DEST_DIR)
+        result, _ = run_client(SOURCE_DIR, DEST_DIR, flags=["-u"], port=shared_server.port)
+        assert result.returncode == 0
+
+        received_file = os.path.join(get_dest_received_dir(DEST_DIR, SOURCE_DIR), "small.txt")
+        source_stat = os.stat(os.path.join(SOURCE_DIR, "small.txt"))
+        with open(received_file, "wb") as f:
+            f.write(b"protected destination\n")
+        os.utime(received_file, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns + 10_000_000_000))
+        original_mode = os.stat(received_file).st_mode
+        try:
+            os.chmod(received_file, 0)
+            result, _ = run_client(SOURCE_DIR, DEST_DIR, flags=["-u"], port=shared_server.port)
+            assert result.returncode == 0
+            os.chmod(received_file, original_mode)
+            with open(received_file, "rb") as f:
+                assert f.read() == b"protected destination\n"
+        finally:
+            os.chmod(received_file, original_mode)
+
         with open(received_file, "wb") as f:
             f.write(b"older destination\n")
-        os.utime(received_file, (source_stat.st_atime, source_stat.st_mtime - 10))
+        os.utime(received_file, ns=(source_stat.st_atime_ns, source_stat.st_mtime_ns - 10_000_000_000))
         result, _ = run_client(SOURCE_DIR, DEST_DIR, flags=["-u"], port=shared_server.port)
         assert result.returncode == 0
         with open(received_file, "rb") as f: