From 1b52fc412d5fca552560db85c3807c3538d69027 Mon Sep 17 00:00:00 2001 From: Federico Justus Denkena Date: Wed, 9 Sep 2026 23:53:01 +0200 Subject: [PATCH] bisync/dropbox: fail closed on listing collapse and cursor races Refuse --delta-list without --check-access. Abort if a listing goes empty or shrinks past --max-delete versus the prior snapshot (full relist and local walk included). Persist cursor before listing so a crash cannot pair a new listing with a stale cursor. Reconstruct delta remotes from path_lower plus leaf casing; ListR probes a non-recursive list when the recursive result is empty. --- backend/dropbox/dropbox.go | 53 ++++++++++++--- backend/dropbox/dropbox_internal_test.go | 2 + cmd/bisync/delta_list.go | 87 +++++++++++++++++++++++- cmd/bisync/delta_list_run.go | 33 +++++++-- cmd/bisync/delta_list_test.go | 9 +++ cmd/bisync/operations.go | 4 ++ 6 files changed, 172 insertions(+), 16 deletions(-) diff --git a/backend/dropbox/dropbox.go b/backend/dropbox/dropbox.go index 507cefff6..95e3682c2 100644 --- a/backend/dropbox/dropbox.go +++ b/backend/dropbox/dropbox.go @@ -1382,6 +1382,26 @@ func (f *Fs) ListR(ctx context.Context, dir string, callback fs.ListRCallback) ( } } + if len(items) == 0 { + arg := files.NewListFolderArg(f.opt.Enc.FromStandardPath(root)) + arg.Recursive = false + arg.Limit = 1 + if root == "/" { + arg.Path = "" + } + var probe *files.ListFolderResult + err = f.pacer.Call(func() (bool, error) { + probe, err = f.srv.ListFolderContext(ctx, arg) + return shouldRetry(ctx, err) + }) + if err != nil { + return fmt.Errorf("dropbox ListR: empty recursive listing and probe failed: %w", err) + } + if probe != nil && len(probe.Entries) > 0 { + return errors.New("dropbox ListR: recursive listing empty but directory has children; refusing incomplete listing") + } + } + skipPrefixes := make([]string, 0) for _, it := range items { if it.folder == nil || it.folder.SharingInfo == nil || it.folder.SharingInfo.SharedFolderId == "" { @@ -2630,30 +2650,45 @@ func (f *Fs) ListFolderDeltas(ctx context.Context, cursor string) (newCursor str func (f *Fs) listingChangeFromEntry(entry files.IsMetadata) (ListingChange, bool) { var ( - ch ListingChange - entryPath string + ch ListingChange + md *files.Metadata + lower string ) switch info := entry.(type) { case *files.FolderMetadata: ch.IsDir = true - entryPath = info.PathDisplay + md = &info.Metadata + lower = info.PathLower case *files.FileMetadata: - entryPath = info.PathDisplay + md = &info.Metadata + lower = info.PathLower ch.Size = int64(info.Size) ch.ModTime = time.Time(info.ClientModified) ch.Hash = info.ContentHash case *files.DeletedMetadata: ch.Deleted = true - ch.IsDir = true - entryPath = info.PathDisplay + md = &info.Metadata + lower = info.PathLower default: return ListingChange{}, false } - entryPath = trimPrefixFold(entryPath, f.slashRootSlash) - if entryPath == "" { + if lower == "" { return ListingChange{}, false } - ch.Remote = f.opt.Enc.ToStandardPath(entryPath) + leaf := listRLeaf(md) + rel := trimPrefixFold(lower, f.slashRootSlash) + if rel == "" { + return ListingChange{}, false + } + parent := path.Dir(rel) + if parent == "." || parent == "/" { + parent = "" + } + if parent == "" { + ch.Remote = f.opt.Enc.ToStandardName(leaf) + } else { + ch.Remote = path.Join(f.opt.Enc.ToStandardPath(parent), f.opt.Enc.ToStandardName(leaf)) + } return ch, true } diff --git a/backend/dropbox/dropbox_internal_test.go b/backend/dropbox/dropbox_internal_test.go index b44789e6e..6988424bd 100644 --- a/backend/dropbox/dropbox_internal_test.go +++ b/backend/dropbox/dropbox_internal_test.go @@ -545,6 +545,7 @@ func TestListingChangeFromEntry(t *testing.T) { Metadata: files.Metadata{ Name: "a.txt", PathDisplay: "/Dropbox/sub/a.txt", + PathLower: "/dropbox/sub/a.txt", }, Size: 42, ContentHash: "abc", @@ -561,6 +562,7 @@ func TestListingChangeFromEntry(t *testing.T) { Metadata: files.Metadata{ Name: "sub", PathDisplay: "/Dropbox/sub", + PathLower: "/dropbox/sub", }, } ch, ok = f.listingChangeFromEntry(del) diff --git a/cmd/bisync/delta_list.go b/cmd/bisync/delta_list.go index b559f0e5b..f963cb694 100644 --- a/cmd/bisync/delta_list.go +++ b/cmd/bisync/delta_list.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "path" "path/filepath" "strings" "time" @@ -17,6 +18,67 @@ var errDeltaTooManyDeletes = errors.New("delta list: too many deletes") // The in-memory listing must be left unchanged so the caller can full-relist. var errDeltaCursorReset = errors.New("delta list: cursor reset") +// errDeltaListingEmptied is returned when a prior non-empty listing would be replaced by zero files. +var errDeltaListingEmptied = errors.New("delta list: listing became empty while prior listing had files") + +// errDeltaListingCollapsed is returned when file count dropped beyond --max-delete. +var errDeltaListingCollapsed = errors.New("delta list: listing shrank beyond --max-delete") + +func countListingFiles(ls *fileList) int { + if ls == nil { + return 0 + } + n := 0 + for _, name := range ls.list { + if !ls.isDir(name) { + n++ + } + } + return n +} + +func guardFileCount(oldC, newC int, opt deltaApplyOpts) error { + if opt.Force || oldC == 0 { + return nil + } + if newC == 0 { + return errDeltaListingEmptied + } + if opt.MaxDelete >= 0 { + dropped := oldC - newC + if dropped > 0 && float64(dropped)/float64(oldC) > float64(opt.MaxDelete)/100.0 { + return errDeltaListingCollapsed + } + } + return nil +} + +func guardListingShrink(prior, next *fileList, opt deltaApplyOpts) error { + if prior == nil || next == nil { + return nil + } + return guardFileCount(countListingFiles(prior), countListingFiles(next), opt) +} + +func caseDeltaRemote(ls *fileList, remote string) string { + parent := path.Dir(remote) + if parent == "." { + parent = "" + } + if parent == "" { + return remote + } + if ls.has(parent) { + return path.Join(parent, path.Base(remote)) + } + for _, name := range ls.list { + if strings.EqualFold(name, parent) { + return path.Join(name, path.Base(remote)) + } + } + return remote +} + type listingDelta struct { Remote string Deleted bool @@ -63,6 +125,20 @@ func applyListingDeltas(ls *fileList, deltas []listingDelta, opt deltaApplyOpts) deletedFiles := 0 for _, d := range deltas { + if d.Deleted { + if ls.has(d.Remote) { + // keep + } else { + for _, name := range ls.list { + if strings.EqualFold(name, d.Remote) { + d.Remote = name + break + } + } + } + } else { + d.Remote = caseDeltaRemote(ls, d.Remote) + } if d.Deleted { // Dropbox DeletedMetadata: remove the path and all children. prefix := d.Remote @@ -114,13 +190,18 @@ func saveListingAndCursor(listing, cursor, root string, ls *fileList) error { _ = os.Remove(tmpListing) return err } - if err := os.Rename(tmpListing, listing); err != nil { + // Cursor first: a crash here leaves old listing + new cursor (missed + // updates, not a mass-delete). Listing-first would leave a new listing + // with a stale cursor, which can look like deletes on the next run. + if err := os.Rename(tmpCursor, cursorPath(listing)); err != nil { _ = os.Remove(tmpListing) _ = os.Remove(tmpCursor) return err } - if err := os.Rename(tmpCursor, cursorPath(listing)); err != nil { - return fmt.Errorf("listing saved but cursor rename failed (next run will full-list): %w", err) + if err := os.Rename(tmpListing, listing); err != nil { + _ = os.Remove(tmpListing) + _ = os.Remove(cursorPath(listing)) + return fmt.Errorf("cursor saved but listing rename failed (cursor removed to force full-list): %w", err) } return nil } diff --git a/cmd/bisync/delta_list_run.go b/cmd/bisync/delta_list_run.go index 80e5f0411..4240019e5 100644 --- a/cmd/bisync/delta_list_run.go +++ b/cmd/bisync/delta_list_run.go @@ -43,6 +43,11 @@ func (b *bisyncRun) makeDeltaListing(ctx context.Context) (*fileList, *fileList, } else { fs.Infof(nil, "Path1 full listing (no USN/journal; local walk)") err = b.listOneFs(ctx, true) + if err == nil { + if prior, lerr := b.loadListing(b.listing1); lerr == nil { + err = guardListingShrink(prior, b.march.ls1, deltaApplyOpts{MaxDelete: b.opt.MaxDelete, Force: b.opt.Force}) + } + } } if err != nil { b.handleErr("delta-list", "error listing Path1", err, true, true) @@ -56,6 +61,11 @@ func (b *bisyncRun) makeDeltaListing(ctx context.Context) (*fileList, *fileList, } else { fs.Infof(nil, "Path2 full listing (no USN/journal; local walk)") err = b.listOneFs(ctx, false) + if err == nil { + if prior, lerr := b.loadListing(b.listing2); lerr == nil { + err = guardListingShrink(prior, b.march.ls2, deltaApplyOpts{MaxDelete: b.opt.MaxDelete, Force: b.opt.Force}) + } + } } if err != nil { b.handleErr("delta-list", "error listing Path2", err, true, true) @@ -97,15 +107,19 @@ func (b *bisyncRun) refreshDropboxListing(ctx context.Context, dbx *dropbox.Fs, cursor = "" } + var prior *fileList if cursor != "" { - prior, err := b.loadListing(listing) + loaded, err := b.loadListing(listing) if err != nil { fs.Infof(nil, "No usable prior listing (%v); full Dropbox relist", err) cursor = "" } else { - ls = prior + ls = loaded ls.hash = b.hashTypeForFs(f) + prior = loaded } + } else if loaded, err := b.loadListing(listing); err == nil { + prior = loaded } newCursor, reset, changes, err := dbx.ListFolderDeltas(ctx, cursor) @@ -152,13 +166,24 @@ func (b *bisyncRun) refreshDropboxListing(ctx context.Context, dbx *dropbox.Fs, }) } - applyErr := applyListingDeltas(ls, deltas, deltaApplyOpts{ + applyOpts := deltaApplyOpts{ MaxDelete: b.opt.MaxDelete, Force: b.opt.Force, - }) + } + priorCount := 0 + hadPrior := prior != nil + if hadPrior { + priorCount = countListingFiles(prior) + } + applyErr := applyListingDeltas(ls, deltas, applyOpts) if applyErr != nil { return nil, applyErr } + if hadPrior { + if err := guardFileCount(priorCount, countListingFiles(ls), applyOpts); err != nil { + return nil, err + } + } if b.opt.DryRun { fs.Infof(nil, "dry-run: not saving Dropbox listing cursor") diff --git a/cmd/bisync/delta_list_test.go b/cmd/bisync/delta_list_test.go index c86bf85b4..5f6d31c33 100644 --- a/cmd/bisync/delta_list_test.go +++ b/cmd/bisync/delta_list_test.go @@ -120,3 +120,12 @@ func TestLoadCursor_MissingIsEmpty(t *testing.T) { _, err = os.Stat(cursorPath(listing)) assert.True(t, os.IsNotExist(err)) } + +func TestGuardFileCount(t *testing.T) { + opt := deltaApplyOpts{MaxDelete: 50} + require.NoError(t, guardFileCount(0, 0, opt)) + require.NoError(t, guardFileCount(100, 80, opt)) + require.ErrorIs(t, guardFileCount(100, 0, opt), errDeltaListingEmptied) + require.ErrorIs(t, guardFileCount(100, 40, opt), errDeltaListingCollapsed) + require.NoError(t, guardFileCount(100, 0, deltaApplyOpts{MaxDelete: 50, Force: true})) +} diff --git a/cmd/bisync/operations.go b/cmd/bisync/operations.go index a7f648995..d53e23483 100644 --- a/cmd/bisync/operations.go +++ b/cmd/bisync/operations.go @@ -124,6 +124,10 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) { return err } + if opt.DeltaList && !opt.CheckAccess && !opt.Force { + return fmt.Errorf("--delta-list requires --check-access (or --force to bypass)") + } + // Handle lock file err = b.setLockFile() if err != nil {