fix(delete): guard the per-directory delete commit against dry-run

delete_plan_session_commit() lacked the central no-mutation guard that
manifest_delete_all() has, so a server-contacting -n run (or a hostile plan
frame) could still remove --delete-missing-args mirrors on the per-directory
timing path.  Return DELETE_COMMIT_OK immediately when the session is a
dry-run, and gate the receiver/server commit call sites too.  Add a unit test
that streams a plan naming an existing destination file and asserts it
survives.
This commit is contained in:
2026-09-17 01:02:22 +02:00
parent 946aa934cc
commit b02799327d
4 changed files with 65 additions and 1 deletions
+4
View File
@@ -489,6 +489,10 @@ int receiver_process_pending(Config* config, int file_descriptor, const Receiver
if (pending_plans) {
*pending_plans = plan_session;
plan_session = NULL;
} else if (config->dry_run) {
/* Central dry-run no-op: never commit a deletion for a -n run. */
delete_plan_session_destroy(plan_session);
plan_session = NULL;
} else {
DeleteCommitResult deletion = delete_plan_session_commit(plan_session, config);
bool limit = delete_plan_session_limit_reached(plan_session);
+5 -1
View File
@@ -970,7 +970,11 @@ void handler(int file_descriptor) {
arrived; with the disk writer drained, commit the deferred removals.
--delete-during already applied its plans on the receive thread. */
if (context->deferred_plans) {
DeleteCommitResult deletion = delete_plan_session_commit(context->deferred_plans, config);
/* Defence in depth (the enclosing block already excludes dry-run): a
-n run never commits a deletion. */
DeleteCommitResult deletion = config->dry_run ? DELETE_COMMIT_OK
: delete_plan_session_commit(
context->deferred_plans, config);
if (deletion == DELETE_COMMIT_ERROR) {
transfer_ok = false;
} else if (deletion == DELETE_COMMIT_LIMIT_REACHED) {
+5
View File
@@ -850,6 +850,11 @@ static bool apply_deferred_path(DeletePlanSession* session, const Config* config
DeleteCommitResult delete_plan_session_commit(DeletePlanSession* session, const Config* config) {
if (!session || !config)
return DELETE_COMMIT_ERROR;
/* Central no-mutation guard (mirrors manifest_delete_all): a dry-run never
deletes. The receive path already skips plan application, but a hostile or
buggy peer could still reach the commit, so treat it as a no-op. */
if (session->dry_run)
return DELETE_COMMIT_OK;
bool ok = true;
if (session->defer) {
for (int i = 0; i < session->deferred->size && ok; i++)
+51
View File
@@ -1128,6 +1128,56 @@ static void test_receive_manifest_total_entry_cap() {
config_delete(cfg);
}
/* A server-contacting --dry-run must never delete, even on the per-directory
(--delete-during/--delete-delay) commit path. The receive path already skips
plan application under -n, but a plan frame carrying --delete-missing-args
exact deletions used to be honored by delete_plan_session_commit(). Seed a
destination mirror, stream a plan naming it, and prove it survives. */
static void test_dry_run_delete_plan_commit_does_not_delete() {
char* root = make_check_root("drydelplan");
EXPECT_NOT_NULL(root);
write_check_file(root, "victim.txt", "must survive");
Config* cfg = config_create();
EXPECT_NOT_NULL(cfg);
cfg->send_directory = str_dup("/src");
cfg->receive_root_directory = str_dup(root);
cfg->use_delete = true;
cfg->delete_during = true;
cfg->delete_missing_args = true;
cfg->dry_run = true;
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);
EXPECT_TRUE(send_status(p[1], STATUS_DELETE_PLAN));
EXPECT_TRUE(send_int(p[1], 1)); /* first frame carries the config sections */
EXPECT_TRUE(send_int(p[1], 0)); /* protected prefixes */
EXPECT_TRUE(send_int(p[1], 0)); /* size-skipped prefixes */
EXPECT_TRUE(send_int(p[1], 1)); /* missing-args exact deletions */
EXPECT_TRUE(send_str(p[1], "victim.txt"));
EXPECT_TRUE(send_str(p[1], ".")); /* receive root plan */
EXPECT_TRUE(send_int(p[1], 0)); /* kept child directories */
EXPECT_TRUE(send_int(p[1], 0)); /* kept child files */
EXPECT_TRUE(send_status(p[1], STATUS_FINISHED));
ReceiverSink sink = {.send_success = true};
EXPECT_EQ_INT(receiver_process_pending(cfg, p[0], &sink, NULL, NULL), 0);
char path[1024];
snprintf(path, sizeof(path), "%s/victim.txt", root);
EXPECT_EQ_INT(access(path, F_OK), 0);
close(p[0]);
close(p[1]);
config_delete(cfg);
remove(path);
rmdir(root);
free(root);
}
void test_server() {
test_special_socket_path_log_escaped();
if (!is_running_under_valgrind()) {
@@ -1150,5 +1200,6 @@ void test_server() {
test_receive_manifest_three_sections();
test_manifest_delete_missing_args();
test_receiver_pending_commits_missing_args();
test_dry_run_delete_plan_commit_does_not_delete();
}
}