diff --git a/changelog/unreleased/issue-5667 b/changelog/unreleased/issue-5667 new file mode 100644 index 000000000..8dcbc45d0 --- /dev/null +++ b/changelog/unreleased/issue-5667 @@ -0,0 +1,9 @@ +Bugfix: Skip inaccessible `backup` source paths + +The `backup` command only skipped source paths that did not exist. A path that +could not be accessed for another reason, such as a malformed path on Windows, +was kept and produced an empty snapshot. Restic now skips any such path and +aborts if none remain. + +https://github.com/restic/restic/issues/5667 +https://github.com/restic/restic/pull/21852 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index e0f901738..94348880f 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -171,13 +171,17 @@ var ErrInvalidSourceData = errors.New("at least one source file could not be rea // ErrNoSourceData is used to report that no source data was found var ErrNoSourceData = errors.Fatal("all source directories/files do not exist") -// filterExisting returns a slice of all existing items, or an error if no -// items exist at all. +// filterExisting returns the items that exist and can be accessed. It returns +// ErrNoSourceData if none remain, or ErrInvalidSourceData if some were skipped. func filterExisting(items []string, warnf func(msg string, args ...interface{})) (result []string, err error) { for _, item := range items { _, err := fs.Lstat(item) - if errors.Is(err, os.ErrNotExist) { - warnf("%v does not exist, skipping\n", item) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + warnf("%v does not exist, skipping\n", item) + } else { + warnf("%v cannot be accessed, skipping\n", item) + } continue } diff --git a/cmd/restic/cmd_backup_test.go b/cmd/restic/cmd_backup_test.go index e6f00aa5d..07b62c335 100644 --- a/cmd/restic/cmd_backup_test.go +++ b/cmd/restic/cmd_backup_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/restic/restic/internal/errors" rtest "github.com/restic/restic/internal/test" ) @@ -76,6 +77,28 @@ func TestCollectTargets(t *testing.T) { rtest.Assert(t, err == ErrInvalidSourceData, "expected error when not all targets exist") } +func TestFilterExistingUnreadable(t *testing.T) { + dir := rtest.TempDir(t) + + existing := filepath.Join(dir, "existing") + rtest.OK(t, os.Mkdir(existing, 0755)) + + file := filepath.Join(dir, "file") + rtest.OK(t, os.WriteFile(file, []byte("x"), 0600)) + + // Regression test for #5667. A target whose Lstat fails with an error other + // than ErrNotExist must be skipped (ENOTDIR on unix, NUL byte everywhere). + for _, unreadable := range []string{filepath.Join(file, "child"), "invalid\x00path"} { + result, err := filterExisting([]string{unreadable}, t.Logf) + rtest.Assert(t, errors.Is(err, ErrNoSourceData), "input %q: expected ErrNoSourceData; got %v", unreadable, err) + rtest.Assert(t, len(result) == 0, "input %q: expected no targets; got %v", unreadable, result) + + result, err = filterExisting([]string{existing, unreadable}, t.Logf) + rtest.Assert(t, errors.Is(err, ErrInvalidSourceData), "input %q: expected ErrInvalidSourceData; got %v", unreadable, err) + rtest.Equals(t, []string{existing}, result) + } +} + func TestReadFilenamesRaw(t *testing.T) { // These should all be returned exactly as-is. expected := []string{