From 40a874a0d8017286f9f4661215e2f814bbf68f91 Mon Sep 17 00:00:00 2001 From: nielash Date: Tue, 11 Jul 2023 04:56:12 -0400 Subject: [PATCH] bisync: enforce --check-access during --resync --check-access is now enforced during --resync, preventing data loss in certain user error scenarios https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=%2D%2Dcheck%2Daccess%20doesn%27t%20always%20fail%20when%20it%20should --- cmd/bisync/operations.go | 32 +++++++++++++++++++ .../test_check_filename/golden/test.log | 4 ++- .../testdata/test_check_filename/scenario.txt | 2 +- docs/content/bisync.md | 23 +++++++++---- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/cmd/bisync/operations.go b/cmd/bisync/operations.go index 85b1e43d1..5ac46d3c2 100644 --- a/cmd/bisync/operations.go +++ b/cmd/bisync/operations.go @@ -341,6 +341,38 @@ func (b *bisyncRun) resync(octx, fctx context.Context, listing1, listing2 string return err } + // Check access health on the Path1 and Path2 filesystems + // enforce even though this is --resync + if b.opt.CheckAccess { + fs.Infof(nil, "Checking access health") + + ds1 := &deltaSet{ + checkFiles: bilib.Names{}, + } + + ds2 := &deltaSet{ + checkFiles: bilib.Names{}, + } + + for _, file := range filesNow1.list { + if filepath.Base(file) == b.opt.CheckFilename { + ds1.checkFiles.Add(file) + } + } + + for _, file := range filesNow2.list { + if filepath.Base(file) == b.opt.CheckFilename { + ds2.checkFiles.Add(file) + } + } + + err = b.checkAccess(ds1.checkFiles, ds2.checkFiles) + if err != nil { + b.critical = true + return err + } + } + copy2to1 := []string{} for _, file := range filesNow2.list { if !filesNow1.has(file) { diff --git a/cmd/bisync/testdata/test_check_filename/golden/test.log b/cmd/bisync/testdata/test_check_filename/golden/test.log index 78d0555ac..d008b934d 100644 --- a/cmd/bisync/testdata/test_check_filename/golden/test.log +++ b/cmd/bisync/testdata/test_check_filename/golden/test.log @@ -39,10 +39,12 @@ Bisync error: bisync aborted (10) : move-listings path2-missing (11) : test 3. put the remote subdir .chk_file back, run resync. -(12) : copy-file {path1/}subdir/.chk_file {path2/} +(12) : copy-file {path1/}subdir/.chk_file {path2/}subdir/ (13) : bisync check-access resync check-filename=.chk_file INFO : Synching Path1 "{path1/}" with Path2 "{path2/}" INFO : Copying unique Path2 files to Path1 +INFO : Checking access health +INFO : Found 2 matching ".chk_file" files on both paths INFO : Resynching Path1 to Path2 INFO : Resync updating listings INFO : Bisync successful diff --git a/cmd/bisync/testdata/test_check_filename/scenario.txt b/cmd/bisync/testdata/test_check_filename/scenario.txt index 3d0b0ab49..d20a3ee48 100644 --- a/cmd/bisync/testdata/test_check_filename/scenario.txt +++ b/cmd/bisync/testdata/test_check_filename/scenario.txt @@ -20,7 +20,7 @@ bisync check-access check-filename=.chk_file move-listings path2-missing test 3. put the remote subdir .chk_file back, run resync. -copy-file {path1/}subdir/.chk_file {path2/} +copy-file {path1/}subdir/.chk_file {path2/}subdir/ bisync check-access resync check-filename=.chk_file test 4. run sync with check-access. should pass. diff --git a/docs/content/bisync.md b/docs/content/bisync.md index 17778868f..4f8422510 100644 --- a/docs/content/bisync.md +++ b/docs/content/bisync.md @@ -165,13 +165,23 @@ Access check files are an additional safety measure against data loss. bisync will ensure it can find matching `RCLONE_TEST` files in the same places in the Path1 and Path2 filesystems. `RCLONE_TEST` files are not generated automatically. -For `--check-access`to succeed, you must first either: -**A)** Place one or more `RCLONE_TEST` files in the Path1 or Path2 filesystem -and then do either a run without `--check-access` or a [--resync](#resync) to -set matching files on both filesystems, or +For `--check-access` to succeed, you must first either: +**A)** Place one or more `RCLONE_TEST` files in both systems, or **B)** Set `--check-filename` to a filename already in use in various locations -throughout your sync'd fileset. -Time stamps and file contents are not important, just the names and locations. +throughout your sync'd fileset. Recommended methods for **A)** include: +* `rclone touch Path1/RCLONE_TEST` (create a new file) +* `rclone copyto Path1/RCLONE_TEST Path2/RCLONE_TEST` (copy an existing file) +* `rclone copy Path1/RCLONE_TEST Path2/RCLONE_TEST --include "RCLONE_TEST"` (copy multiple files at once, recursively) +* create the files manually (outside of rclone) +* run `bisync` once *without* `--check-access` to set matching files on both filesystems +will also work, but is not preferred, due to potential for user error +(you are temporarily disabling the safety feature). + +Note that `--check-access` is still enforced on `--resync`, so `bisync --resync --check-access` +will not work as a method of initially setting the files (this is to ensure that bisync can't +[inadvertently circumvent its own safety switch](https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=3.%20%2D%2Dcheck%2Daccess%20doesn%27t%20always%20fail%20when%20it%20should).) + +Time stamps and file contents for `RCLONE_TEST` files are not important, just the names and locations. If you have symbolic links in your sync tree it is recommended to place `RCLONE_TEST` files in the linked-to directory tree to protect against bisync assuming a bunch of deleted files if the linked-to tree should not be @@ -1108,3 +1118,4 @@ about _Unison_ and synchronization in general. ### `v1.64` * Fixed an [issue](https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=1.%20Dry%20runs%20are%20not%20completely%20dry) causing dry runs to inadvertently commit filter changes +* `--check-access` is now enforced during `--resync`, preventing data loss in [certain user error scenarios](https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=%2D%2Dcheck%2Daccess%20doesn%27t%20always%20fail%20when%20it%20should)