From 9282fe09d006adbb306cede65d715927d7322bb2 Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 04:45:55 -0400 Subject: [PATCH 1/7] feat(config): reject sync without backup_path unless allow_destructive=true --- internal/config/config.go | 27 +++++- internal/config/config_test.go | 101 ++++++++++++++++++++ testdata/config_sync_allow_destructive.toml | 7 ++ testdata/config_sync_no_backup_path.toml | 6 ++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 testdata/config_sync_allow_destructive.toml create mode 100644 testdata/config_sync_no_backup_path.toml diff --git a/internal/config/config.go b/internal/config/config.go index cc488ea..ea1d340 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -104,7 +104,13 @@ type Job struct { Mode string `toml:"mode"` BackupPath string `toml:"backup_path"` BackupRetentionDays int `toml:"backup_retention_days"` - FilterFile string `toml:"filter_file"` + // AllowDestructive acknowledges that an rclone job with mode="sync" + // and empty backup_path permanently deletes remote files when their + // local counterparts are removed. Required for that configuration to + // pass validation. Ignored on all other configurations (copy mode, + // sync with backup_path set, rsync jobs). + AllowDestructive bool `toml:"allow_destructive"` + FilterFile string `toml:"filter_file"` // Rclone per-job tuning overrides Transfers int `toml:"transfers"` @@ -346,6 +352,9 @@ func validateRcloneJob(job Job) error { } remoteSeen[r] = true } + if err := validateDestructiveSync(job); err != nil { + return err + } return validateMaxRuntime(job) } @@ -369,3 +378,19 @@ func validateMaxRuntime(job Job) error { } return nil } + +// validateDestructiveSync rejects rclone sync jobs that would permanently +// delete remote files without archival. Passes when mode is not "sync", +// when backup_path is set (deletions are archived), or when the job +// acknowledges the risk via AllowDestructive=true. The error message names +// both resolution paths so the user can fix the config without consulting +// documentation. +func validateDestructiveSync(job Job) error { + if job.Mode != ModeSync || job.BackupPath != "" || job.AllowDestructive { + return nil + } + return fmt.Errorf( + `job %q: mode=%q without backup_path permanently deletes remote files when their local counterparts are removed; set backup_path to archive deletions, or set allow_destructive=true to acknowledge this`, + job.Name, ModeSync, + ) +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c2f27ce..d8217f0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -699,3 +699,104 @@ max_runtime = "-1h" t.Errorf("error %q should mention job name", err.Error()) } } + +func TestValidate_RcloneSyncWithBackupPath_Passes(t *testing.T) { + tomlData := ` +[[job]] +name = "docs-safe" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "sync" +backup_path = "_archive" +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Jobs[0].AllowDestructive { + t.Error("AllowDestructive = true, want false (zero value)") + } +} + +func TestValidate_RcloneSyncNoBackupPath_Rejected(t *testing.T) { + _, err := config.LoadFile(testdataPath("config_sync_no_backup_path.toml")) + if err == nil { + t.Fatal("expected error for sync without backup_path or opt-in, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "unsafe-sync") { + t.Errorf("error %q should mention the job name", msg) + } + if !strings.Contains(msg, "allow_destructive") { + t.Errorf("error %q should mention allow_destructive", msg) + } + if !strings.Contains(msg, "backup_path") { + t.Errorf("error %q should mention backup_path", msg) + } +} + +func TestValidate_RcloneSyncAllowDestructive_Passes(t *testing.T) { + cfg, err := config.LoadFile(testdataPath("config_sync_allow_destructive.toml")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cfg.Jobs[0].AllowDestructive { + t.Error("AllowDestructive = false, want true") + } +} + +func TestValidate_RcloneCopyNoBackupPath_Passes(t *testing.T) { + tomlData := ` +[[job]] +name = "copy-job" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "copy" +` + if _, err := config.LoadBytes([]byte(tomlData)); err != nil { + t.Errorf("copy mode with empty backup_path should pass, got: %v", err) + } +} + +func TestValidate_RcloneSyncWithBothBackupPathAndAllowDestructive_Passes(t *testing.T) { + tomlData := ` +[[job]] +name = "belt-and-suspenders" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "sync" +backup_path = "_archive" +allow_destructive = true +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("sync with both backup_path and allow_destructive should pass, got: %v", err) + } + if !cfg.Jobs[0].AllowDestructive { + t.Error("AllowDestructive = false, want true") + } + if cfg.Jobs[0].BackupPath == "" { + t.Error("BackupPath should be set") + } +} + +func TestValidate_RsyncJobWithAllowDestructive_SilentlyIgnored(t *testing.T) { + tomlData := ` +[[job]] +name = "rsync-job" +engine = "rsync" +sources = ["/tmp/src"] +destination = "/tmp/dst" +allow_destructive = true +` + cfg, err := config.LoadBytes([]byte(tomlData)) + if err != nil { + t.Fatalf("allow_destructive on rsync job should be silently accepted, got: %v", err) + } + if !cfg.Jobs[0].AllowDestructive { + t.Error("AllowDestructive field should still unmarshal to true on rsync jobs") + } +} diff --git a/testdata/config_sync_allow_destructive.toml b/testdata/config_sync_allow_destructive.toml new file mode 100644 index 0000000..2350ba5 --- /dev/null +++ b/testdata/config_sync_allow_destructive.toml @@ -0,0 +1,7 @@ +[[job]] +name = "intentional-destructive" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "sync" +allow_destructive = true diff --git a/testdata/config_sync_no_backup_path.toml b/testdata/config_sync_no_backup_path.toml new file mode 100644 index 0000000..b160a94 --- /dev/null +++ b/testdata/config_sync_no_backup_path.toml @@ -0,0 +1,6 @@ +[[job]] +name = "unsafe-sync" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "sync" From 36a2db79d78473720c710fbbdfc2e9ed1cd327ec Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 04:51:41 -0400 Subject: [PATCH 2/7] style(config): gofmt after allow_destructive field addition --- internal/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index ea1d340..702bcd0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -109,8 +109,8 @@ type Job struct { // local counterparts are removed. Required for that configuration to // pass validation. Ignored on all other configurations (copy mode, // sync with backup_path set, rsync jobs). - AllowDestructive bool `toml:"allow_destructive"` - FilterFile string `toml:"filter_file"` + AllowDestructive bool `toml:"allow_destructive"` + FilterFile string `toml:"filter_file"` // Rclone per-job tuning overrides Transfers int `toml:"transfers"` From 0118f0b08f1b864f4582f062a23e516657d83d72 Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 04:54:07 -0400 Subject: [PATCH 3/7] docs: document allow_destructive opt-in for sync without backup_path --- README.md | 1 + config.example.toml | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/README.md b/README.md index 2214a49..e33359e 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,7 @@ Each `[[job]]` entry defines one sync operation. | `mode` | string | yes | `"copy"` (add files) or `"sync"` (mirror, may delete) | | `backup_path` | string | no | Remote path prefix for archived deleted files (sync mode only) | | `backup_retention_days` | int | no | Days to keep archived files before cleanup | +| `allow_destructive` | bool | no | Required when `mode = "sync"` and `backup_path` is empty. Acknowledges that deleted local files will be permanently removed from the remote on sync. Default: `false`. | | `filter_file` | string | no | Per-job filter file (overrides default) | Rclone jobs also accept all tuning fields from `[defaults.rclone]` as per-job overrides (`transfers`, `bwlimit`, etc.). diff --git a/config.example.toml b/config.example.toml index 4993a7d..a52a25c 100644 --- a/config.example.toml +++ b/config.example.toml @@ -60,6 +60,10 @@ delete = true # Rclone job: sync documents to multiple cloud remotes # Remotes must already be configured in rclone (run `rclone config`). +# Note: sync mode deletes remote files when local files are removed. +# Either set backup_path (shown here) to archive deletions for later +# recovery, or set allow_destructive = true to opt in to permanent +# deletion. [[job]] name = "documents-to-cloud" engine = "rclone" @@ -68,6 +72,9 @@ remotes = ["my_gdrive", "my_s3"] mode = "sync" backup_path = "_archive" backup_retention_days = 365 +# allow_destructive = true # required only if backup_path is empty; + # acknowledges that deleted local files will + # be permanently removed from the remote. # Rclone job: copy media (bandwidth-limited) [[job]] From 5ba8abf7a0c1e92aa694a461b74521aebaae781c Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 05:01:21 -0400 Subject: [PATCH 4/7] test(config): cover explicit allow_destructive=false and mode-literal assertion --- internal/config/config_test.go | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d8217f0..1b68362 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -800,3 +800,38 @@ allow_destructive = true t.Error("AllowDestructive field should still unmarshal to true on rsync jobs") } } + +func TestValidate_RcloneSyncExplicitFalse_Rejected(t *testing.T) { + // Pins that an explicit `allow_destructive = false` in TOML behaves + // identically to omitting the key. Guards against a future TOML + // library change that could distinguish zero-value from unset. + tomlData := ` +[[job]] +name = "explicit-false-sync" +engine = "rclone" +source = "/tmp/docs" +remotes = ["gdrive"] +mode = "sync" +allow_destructive = false +` + _, err := config.LoadBytes([]byte(tomlData)) + if err == nil { + t.Fatal("expected error for sync with explicit allow_destructive=false, got nil") + } + if !strings.Contains(err.Error(), "allow_destructive") { + t.Errorf("error %q should mention allow_destructive", err.Error()) + } +} + +func TestValidate_RcloneSyncNoBackupPath_ErrorMentionsModeString(t *testing.T) { + // The validator's error message interpolates ModeSync via %q. Asserting + // the literal "sync" appears in the output pins the diagnostic so a + // future refactor can't silently drop the mode value from the message. + _, err := config.LoadFile(testdataPath("config_sync_no_backup_path.toml")) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), `"sync"`) { + t.Errorf("error %q should contain the mode literal \"sync\"", err.Error()) + } +} From bec73f66504102f4018b294ff62332f9d2145cd0 Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 05:11:33 -0400 Subject: [PATCH 5/7] refactor(config): split destructive-sync error into two sentences --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 702bcd0..9ee7581 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -390,7 +390,7 @@ func validateDestructiveSync(job Job) error { return nil } return fmt.Errorf( - `job %q: mode=%q without backup_path permanently deletes remote files when their local counterparts are removed; set backup_path to archive deletions, or set allow_destructive=true to acknowledge this`, + `job %q: mode=%q without backup_path permanently deletes remote files when their local counterparts are removed. Set backup_path to archive deletions, or set allow_destructive=true to acknowledge this.`, job.Name, ModeSync, ) } From dabd4d558730f4f8a1b0a5ad8dd09ee65984fe18 Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 05:12:27 -0400 Subject: [PATCH 6/7] test(config): rename test and merge redundant mode assertion --- internal/config/config_test.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 1b68362..4038190 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -734,6 +734,9 @@ func TestValidate_RcloneSyncNoBackupPath_Rejected(t *testing.T) { if !strings.Contains(msg, "backup_path") { t.Errorf("error %q should mention backup_path", msg) } + if !strings.Contains(msg, "mode=") { + t.Errorf("error %q should name the offending mode via the mode= prefix", msg) + } } func TestValidate_RcloneSyncAllowDestructive_Passes(t *testing.T) { @@ -783,7 +786,7 @@ allow_destructive = true } } -func TestValidate_RsyncJobWithAllowDestructive_SilentlyIgnored(t *testing.T) { +func TestValidate_RsyncJobWithAllowDestructive_Accepted(t *testing.T) { tomlData := ` [[job]] name = "rsync-job" @@ -822,16 +825,3 @@ allow_destructive = false t.Errorf("error %q should mention allow_destructive", err.Error()) } } - -func TestValidate_RcloneSyncNoBackupPath_ErrorMentionsModeString(t *testing.T) { - // The validator's error message interpolates ModeSync via %q. Asserting - // the literal "sync" appears in the output pins the diagnostic so a - // future refactor can't silently drop the mode value from the message. - _, err := config.LoadFile(testdataPath("config_sync_no_backup_path.toml")) - if err == nil { - t.Fatal("expected error, got nil") - } - if !strings.Contains(err.Error(), `"sync"`) { - t.Errorf("error %q should contain the mode literal \"sync\"", err.Error()) - } -} From 3bc0108e132d7cb2d0d888b4afc555cb59de9909 Mon Sep 17 00:00:00 2001 From: jkleinne Date: Mon, 20 Apr 2026 05:14:50 -0400 Subject: [PATCH 7/7] fix(config): drop trailing period to satisfy error-string lint --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9ee7581..290230e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -390,7 +390,7 @@ func validateDestructiveSync(job Job) error { return nil } return fmt.Errorf( - `job %q: mode=%q without backup_path permanently deletes remote files when their local counterparts are removed. Set backup_path to archive deletions, or set allow_destructive=true to acknowledge this.`, + `job %q: mode=%q without backup_path permanently deletes remote files when their local counterparts are removed. Set backup_path to archive deletions, or set allow_destructive=true to acknowledge this`, job.Name, ModeSync, ) }