fix(receive): enforce dry-run no-mutation centrally
--dry-run --read-batch=FILE still wrote to the destination because batch_read_apply -> file_save_to_disk_full bypassed the per-caller !dry_run guards. Guard file_save_to_disk_full and manifest_delete_all directly (return SKIPPED/no-op) so every save/delete path is mutation-free in dry-run, and keep the per-caller guards. Reject --dry-run combined with --read-batch/--only-write-batch at CLI validation with a clear error (a dry-run of a local batch apply is not meaningful).
This commit is contained in:
@@ -22,6 +22,17 @@ bool validate_config(const Config* config) {
|
||||
"--write-batch, --only-write-batch, and --read-batch are mutually exclusive");
|
||||
return false;
|
||||
}
|
||||
/* A dry-run of a local batch apply is not meaningful: --read-batch bypasses
|
||||
the client-side scan/server decision entirely, so dry-run would have no
|
||||
wire state to report (and must not be used as a mutation escape hatch).
|
||||
--only-write-batch likewise never contacts a receiver. Reject both up
|
||||
front instead of silently ignoring --dry-run. */
|
||||
if (config->dry_run && (read_batch || only_write_batch)) {
|
||||
log_message(LOG_LEVEL_ERROR,
|
||||
"--dry-run cannot be combined with --read-batch or --only-write-batch; "
|
||||
"a dry-run of a local batch apply is not meaningful");
|
||||
return false;
|
||||
}
|
||||
if (read_batch) {
|
||||
if (!config->receive_root_directory) {
|
||||
log_message(LOG_LEVEL_ERROR, "--read-batch requires a destination directory");
|
||||
|
||||
@@ -569,6 +569,13 @@ static FileSaveResult file_save_write_device(const char* root_directory, const F
|
||||
|
||||
FileSaveResult file_save_to_disk_full(const char* root_directory, const File* file,
|
||||
const Config* config) {
|
||||
/* Central no-mutation guard: a server-contacting --dry-run (or a local batch
|
||||
apply that somehow carries dry_run) must never touch the destination, no
|
||||
matter which caller reached this primitive. The per-caller guards remain,
|
||||
but this is the last line of defense for every save path. Report SKIPPED
|
||||
so a --remove-source-files sender correctly keeps its source. */
|
||||
if (config && config->dry_run)
|
||||
return FILE_SAVE_SKIPPED;
|
||||
/* Backups are incompatible with ignore-existing: moving the entry first
|
||||
would make a concurrent no-replace commit overwrite its old name. */
|
||||
bool backup_enabled = config && config->backup && !config->ignore_existing;
|
||||
@@ -2941,6 +2948,11 @@ bool manifest_delete_missing_args(const Config* config, DeleteManifest* manifest
|
||||
bool manifest_delete_all(const Config* config, DeleteManifest* manifest) {
|
||||
if (!config || !manifest)
|
||||
return false;
|
||||
/* Central no-mutation guard: a dry-run never deletes. No manifest is sent on
|
||||
the dry-run path, but a hostile/buggy peer could; treat it as a no-op so
|
||||
the receiver can never remove anything. */
|
||||
if (config->dry_run)
|
||||
return true;
|
||||
if (config->delete_missing_args && !manifest_delete_missing_args(config, manifest))
|
||||
return false;
|
||||
if (config->use_delete && !manifest_delete_extras(config, manifest))
|
||||
|
||||
Reference in New Issue
Block a user