diff --git a/src/client/client_cli.c b/src/client/client_cli.c index d8caf41..2b685ec 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -219,6 +219,7 @@ static const OptionEntry OPTION_TABLE[] = { {"--human-readable", "-h", OPT_FLAG, offsetof(Config, human_readable)}, {"--partial", NULL, OPT_FLAG, offsetof(Config, partial)}, {"--secluded-args", NULL, OPT_NOOP, 0}, + {"--update", "-u", OPT_FLAG, offsetof(Config, update)}, {"--links", "-l", OPT_FLAG, offsetof(Config, follow_symlinks)}, {"--copy-links", NULL, OPT_FLAG, offsetof(Config, copy_links)}, {"--safe-links", NULL, OPT_FLAG, offsetof(Config, safe_links)}, @@ -267,6 +268,8 @@ static int apply_table_option(Config* config, const OptionEntry* entry, const ch switch (entry->kind) { case OPT_FLAG: *(bool*)field = true; + if (entry->offset == offsetof(Config, update)) + config->use_metadata = true; return 0; case OPT_NOOP: return 0; diff --git a/src/client/usage.c b/src/client/usage.c index 7043adb..f8574f6 100644 --- a/src/client/usage.c +++ b/src/client/usage.c @@ -33,6 +33,7 @@ void print_usage(void) { printf(" --size-only Skip incremental files matching in size, ignoring mtime\n"); printf(" -I, --ignore-times Transfer files even when size and mtime match\n"); printf(" -@, --modify-window Modification time tolerance\n"); + printf(" -u, --update Skip files newer than the source on receiver\n"); printf(" --delta Delta transfer for changed files (requires --incremental)\n"); printf(" -W, --whole-file Transfer changed files without delta processing\n"); printf(" --delta-block Delta block size in bytes (default: %d)\n", diff --git a/src/shared/file.c b/src/shared/file.c index f96bcab..377ea0d 100644 --- a/src/shared/file.c +++ b/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,9 +316,9 @@ bool file_rename_secure(const char* old_path, const char* new_path) { return ok; } -bool file_to_disk_secure_with_fsync(const char* path, const void* data, - unsigned long long data_size, bool inplace, bool sparse, - const FileMetadata* metadata, bool use_fsync) { +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, bool use_fsync) { char* leaf = NULL; int dirfd = file_open_secure_parent(path, &leaf, true); if (dirfd < 0) @@ -312,17 +326,38 @@ bool file_to_disk_secure_with_fsync(const char* path, const void* data, 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); - if (ok && use_fsync) - ok = fsync(fd) == 0; + 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 (!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); + if (ok && use_fsync) + ok = fsync(fd) == 0; + } } } 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); @@ -354,7 +389,19 @@ bool file_to_disk_secure_with_fsync(const char* path, const void* data, 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_with_fsync(path, data, data_size, inplace, sparse, metadata, false); + return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, 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, false); +} + +bool file_to_disk_secure_with_fsync(const char* path, const void* data, + unsigned long long data_size, bool inplace, bool sparse, + const FileMetadata* metadata, bool use_fsync) { + return file_to_disk_secure_impl(path, data, data_size, inplace, sparse, metadata, false, + use_fsync); } bool file_write_to_disk(const char* path, const void* data, unsigned long long data_size, diff --git a/src/shared/file.h b/src/shared/file.h index 7a67203..6de7eea 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -27,6 +27,7 @@ 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); @@ -35,5 +36,10 @@ bool file_to_disk_secure(const char* path, const void* data, unsigned long long bool file_to_disk_secure_with_fsync(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse, const FileMetadata* metadata, bool use_fsync); +/* 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 diff --git a/src/shared/file_receive.c b/src/shared/file_receive.c index f8dad31..42eb14c 100644 --- a/src/shared/file_receive.c +++ b/src/shared/file_receive.c @@ -61,16 +61,13 @@ 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) { - struct stat destination_stat; - if (file_stat_secure(disk_path, &destination_stat) && file->metadata && - destination_stat.st_mtime > file->metadata->mtime_sec) { - free(confined_backup); - free(confined_partial); - free(disk_path); - return true; - } + /* --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); + return true; } if (backup_enabled) { @@ -103,8 +100,12 @@ bool file_save_to_disk(const char* root_directory, const File* file, const Confi } } - bool ok = file_to_disk_secure_with_fsync(disk_path, file->data->data, file->data->size, inplace, - sparse, file->metadata, config && config->use_fsync); + 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_with_fsync(disk_path, file->data->data, file->data->size, + inplace, sparse, file->metadata, + config && config->use_fsync); free(parent_copy); free(backup_path); free(confined_backup); diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index eb36c73..bb9d7d2 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -474,6 +474,61 @@ class TestIncremental: assert not mismatches, f"Mismatch: {mismatches}" +class TestUpdate: + def test_update_skips_older_destination_and_allows_equal_or_newer_source(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 = get_dest_received_dir(DEST_DIR, SOURCE_DIR) + source_file = os.path.join(SOURCE_DIR, "small.txt") + received_file = os.path.join(received, "small.txt") + source_stat = os.stat(source_file) + + with open(received_file, "wb") as f: + f.write(b"newer destination\n") + 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, 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, 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"hello world\n" + + class TestDelete: def test_delete_removes_extra_files(self, shared_server): clean_dir(DEST_DIR) diff --git a/tests/test_client_cli.c b/tests/test_client_cli.c index 7afa6cf..5f8f0e8 100644 --- a/tests/test_client_cli.c +++ b/tests/test_client_cli.c @@ -394,8 +394,6 @@ static void test_parse_args_rejects_unimplemented_options() { "--out-format", "--info", "--list-only", - "-u", - "--update", "--append", "--append-verify", "--delete-excluded", @@ -465,6 +463,20 @@ static void test_parse_args_human_readable() { } } +static void test_parse_args_update() { + Config* cfg = config_create(); + char* argv[] = {"fastsync", "-u", "/src", "/dst"}; + int positional_args[2]; + int positional_count = 0; + + EXPECT_EQ_INT(parse_args(cfg, 4, argv, positional_args, &positional_count), 0); + EXPECT_TRUE(cfg->update); + EXPECT_TRUE(cfg->use_metadata); + EXPECT_EQ_INT(positional_count, 2); + + config_delete(cfg); +} + /* Test parse_args with --archive flag */ static void test_parse_args_archive() { Config* cfg = config_create(); @@ -634,6 +646,7 @@ void test_client_cli() { test_parse_args_rejects_unimplemented_options(); test_parse_args_quiet(); test_parse_args_human_readable(); + test_parse_args_update(); test_parse_args_archive(); test_parse_args_fsync(); test_parse_args_ignore_times();