diff --git a/changelog/unreleased/issue-3600 b/changelog/unreleased/issue-3600 new file mode 100644 index 000000000..0da66d382 --- /dev/null +++ b/changelog/unreleased/issue-3600 @@ -0,0 +1,11 @@ +Bugfix: `backup` works if xattrs above the backup target cannot be read + +When backup targets are specified using absolute paths, then `backup` also +includes information about the parent folders of the backup targets in the +snapshot. If the extended attributes for some of these folders could not be +read due to missing permissions, this caused the backup to fail. This has been +fixed. + +https://github.com/restic/restic/issues/3600 +https://github.com/restic/restic/pull/4668 +https://forum.restic.net/t/parent-directories-above-the-snapshot-source-path-fatal-error-permission-denied/7216 diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 050a0e2c7..146ff3a7c 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -237,8 +237,8 @@ func (arch *Archiver) trackItem(item string, previous, current *restic.Node, s I } // nodeFromFileInfo returns the restic node from an os.FileInfo. -func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo) (*restic.Node, error) { - node, err := restic.NodeFromFileInfo(filename, fi) +func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { + node, err := restic.NodeFromFileInfo(filename, fi, ignoreXattrListError) if !arch.WithAtime { node.AccessTime = node.ModTime } @@ -289,7 +289,7 @@ func (arch *Archiver) wrapLoadTreeError(id restic.ID, err error) error { func (arch *Archiver) saveDir(ctx context.Context, snPath string, dir string, fi os.FileInfo, previous *restic.Tree, complete CompleteFunc) (d FutureNode, err error) { debug.Log("%v %v", snPath, dir) - treeNode, err := arch.nodeFromFileInfo(snPath, dir, fi) + treeNode, err := arch.nodeFromFileInfo(snPath, dir, fi, false) if err != nil { return FutureNode{}, err } @@ -444,7 +444,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous debug.Log("%v hasn't changed, using old list of blobs", target) arch.trackItem(snPath, previous, previous, ItemStats{}, time.Since(start)) arch.CompleteBlob(previous.Size) - node, err := arch.nodeFromFileInfo(snPath, target, fi) + node, err := arch.nodeFromFileInfo(snPath, target, fi, false) if err != nil { return FutureNode{}, false, err } @@ -540,7 +540,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous default: debug.Log(" %v other", target) - node, err := arch.nodeFromFileInfo(snPath, target, fi) + node, err := arch.nodeFromFileInfo(snPath, target, fi, false) if err != nil { return FutureNode{}, false, err } @@ -623,7 +623,9 @@ func (arch *Archiver) saveTree(ctx context.Context, snPath string, atree *Tree, } debug.Log("%v, dir node data loaded from %v", snPath, atree.FileInfoPath) - node, err = arch.nodeFromFileInfo(snPath, atree.FileInfoPath, fi) + // in some cases reading xattrs for directories above the backup target is not allowed + // thus ignore errors for such folders. + node, err = arch.nodeFromFileInfo(snPath, atree.FileInfoPath, fi, true) if err != nil { return FutureNode{}, 0, err } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 841c8f2ce..b1ea4b6b6 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -556,7 +556,7 @@ func rename(t testing.TB, oldname, newname string) { } func nodeFromFI(t testing.TB, filename string, fi os.FileInfo) *restic.Node { - node, err := restic.NodeFromFileInfo(filename, fi) + node, err := restic.NodeFromFileInfo(filename, fi, false) if err != nil { t.Fatal(err) } @@ -2230,7 +2230,7 @@ func TestMetadataChanged(t *testing.T) { // get metadata fi := lstat(t, "testfile") - want, err := restic.NodeFromFileInfo("testfile", fi) + want, err := restic.NodeFromFileInfo("testfile", fi, false) if err != nil { t.Fatal(err) } diff --git a/internal/archiver/archiver_unix_test.go b/internal/archiver/archiver_unix_test.go index 9462420dd..a6b1aad2e 100644 --- a/internal/archiver/archiver_unix_test.go +++ b/internal/archiver/archiver_unix_test.go @@ -48,7 +48,7 @@ func wrapFileInfo(fi os.FileInfo) os.FileInfo { func statAndSnapshot(t *testing.T, repo restic.Repository, name string) (*restic.Node, *restic.Node) { fi := lstat(t, name) - want, err := restic.NodeFromFileInfo(name, fi) + want, err := restic.NodeFromFileInfo(name, fi, false) rtest.OK(t, err) _, node := snapshot(t, repo, fs.Local{}, nil, name) diff --git a/internal/archiver/file_saver.go b/internal/archiver/file_saver.go index 7f11bff8a..d10334301 100644 --- a/internal/archiver/file_saver.go +++ b/internal/archiver/file_saver.go @@ -29,7 +29,7 @@ type FileSaver struct { CompleteBlob func(bytes uint64) - NodeFromFileInfo func(snPath, filename string, fi os.FileInfo) (*restic.Node, error) + NodeFromFileInfo func(snPath, filename string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) } // NewFileSaver returns a new file saver. A worker pool with fileWorkers is @@ -156,7 +156,7 @@ func (s *FileSaver) saveFile(ctx context.Context, chnker *chunker.Chunker, snPat debug.Log("%v", snPath) - node, err := s.NodeFromFileInfo(snPath, f.Name(), fi) + node, err := s.NodeFromFileInfo(snPath, f.Name(), fi, false) if err != nil { _ = f.Close() completeError(err) diff --git a/internal/archiver/file_saver_test.go b/internal/archiver/file_saver_test.go index ced9d796e..409bdedd0 100644 --- a/internal/archiver/file_saver_test.go +++ b/internal/archiver/file_saver_test.go @@ -49,8 +49,8 @@ func startFileSaver(ctx context.Context, t testing.TB) (*FileSaver, context.Cont } s := NewFileSaver(ctx, wg, saveBlob, pol, workers, workers) - s.NodeFromFileInfo = func(snPath, filename string, fi os.FileInfo) (*restic.Node, error) { - return restic.NodeFromFileInfo(filename, fi) + s.NodeFromFileInfo = func(snPath, filename string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { + return restic.NodeFromFileInfo(filename, fi, ignoreXattrListError) } return s, ctx, wg diff --git a/internal/restic/node.go b/internal/restic/node.go index e7688aada..9613cf3c2 100644 --- a/internal/restic/node.go +++ b/internal/restic/node.go @@ -134,7 +134,7 @@ func (node Node) String() string { // NodeFromFileInfo returns a new node from the given path and FileInfo. It // returns the first error that is encountered, together with a node. -func NodeFromFileInfo(path string, fi os.FileInfo) (*Node, error) { +func NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*Node, error) { mask := os.ModePerm | os.ModeType | os.ModeSetuid | os.ModeSetgid | os.ModeSticky node := &Node{ Path: path, @@ -148,7 +148,7 @@ func NodeFromFileInfo(path string, fi os.FileInfo) (*Node, error) { node.Size = uint64(fi.Size()) } - err := node.fillExtra(path, fi) + err := node.fillExtra(path, fi, ignoreXattrListError) return node, err } @@ -675,7 +675,7 @@ func lookupGroup(gid uint32) string { return group } -func (node *Node) fillExtra(path string, fi os.FileInfo) error { +func (node *Node) fillExtra(path string, fi os.FileInfo, ignoreXattrListError bool) error { stat, ok := toStatT(fi.Sys()) if !ok { // fill minimal info with current values for uid, gid @@ -719,7 +719,7 @@ func (node *Node) fillExtra(path string, fi os.FileInfo) error { allowExtended, err := node.fillGenericAttributes(path, fi, stat) if allowExtended { // Skip processing ExtendedAttributes if allowExtended is false. - errEx := node.fillExtendedAttributes(path) + errEx := node.fillExtendedAttributes(path, ignoreXattrListError) if err == nil { err = errEx } else { @@ -729,10 +729,13 @@ func (node *Node) fillExtra(path string, fi os.FileInfo) error { return err } -func (node *Node) fillExtendedAttributes(path string) error { +func (node *Node) fillExtendedAttributes(path string, ignoreListError bool) error { xattrs, err := Listxattr(path) debug.Log("fillExtendedAttributes(%v) %v %v", path, xattrs, err) if err != nil { + if ignoreListError && IsListxattrPermissionError(err) { + return nil + } return err } diff --git a/internal/restic/node_aix.go b/internal/restic/node_aix.go index def46bd60..8ee9022c9 100644 --- a/internal/restic/node_aix.go +++ b/internal/restic/node_aix.go @@ -33,6 +33,10 @@ func Listxattr(path string) ([]string, error) { return nil, nil } +func IsListxattrPermissionError(_ error) bool { + return false +} + // Setxattr is a no-op on AIX. func Setxattr(path, name string, data []byte) error { return nil diff --git a/internal/restic/node_netbsd.go b/internal/restic/node_netbsd.go index 1a47299be..cf1fa36bd 100644 --- a/internal/restic/node_netbsd.go +++ b/internal/restic/node_netbsd.go @@ -23,6 +23,10 @@ func Listxattr(path string) ([]string, error) { return nil, nil } +func IsListxattrPermissionError(_ error) bool { + return false +} + // Setxattr is a no-op on netbsd. func Setxattr(path, name string, data []byte) error { return nil diff --git a/internal/restic/node_openbsd.go b/internal/restic/node_openbsd.go index e60eb9dc8..4f1c0dacb 100644 --- a/internal/restic/node_openbsd.go +++ b/internal/restic/node_openbsd.go @@ -23,6 +23,10 @@ func Listxattr(path string) ([]string, error) { return nil, nil } +func IsListxattrPermissionError(_ error) bool { + return false +} + // Setxattr is a no-op on openbsd. func Setxattr(path, name string, data []byte) error { return nil diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index d9fa02ac8..ea271faab 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/restic/restic/internal/test" rtest "github.com/restic/restic/internal/test" ) @@ -31,7 +32,7 @@ func BenchmarkNodeFillUser(t *testing.B) { t.ResetTimer() for i := 0; i < t.N; i++ { - _, err := NodeFromFileInfo(path, fi) + _, err := NodeFromFileInfo(path, fi, false) rtest.OK(t, err) } @@ -55,7 +56,7 @@ func BenchmarkNodeFromFileInfo(t *testing.B) { t.ResetTimer() for i := 0; i < t.N; i++ { - _, err := NodeFromFileInfo(path, fi) + _, err := NodeFromFileInfo(path, fi, false) if err != nil { t.Fatal(err) } @@ -227,8 +228,11 @@ func TestNodeRestoreAt(t *testing.T) { fi, err := os.Lstat(nodePath) rtest.OK(t, err) - n2, err := NodeFromFileInfo(nodePath, fi) + n2, err := NodeFromFileInfo(nodePath, fi, false) rtest.OK(t, err) + n3, err := NodeFromFileInfo(nodePath, fi, true) + rtest.OK(t, err) + rtest.Assert(t, n2.Equals(*n3), "unexpected node info mismatch %v", cmp.Diff(n2, n3)) rtest.Assert(t, test.Name == n2.Name, "%v: name doesn't match (%v != %v)", test.Type, test.Name, n2.Name) diff --git a/internal/restic/node_unix_test.go b/internal/restic/node_unix_test.go index 374326bf7..9ea7b1725 100644 --- a/internal/restic/node_unix_test.go +++ b/internal/restic/node_unix_test.go @@ -128,7 +128,7 @@ func TestNodeFromFileInfo(t *testing.T) { return } - node, err := NodeFromFileInfo(test.filename, fi) + node, err := NodeFromFileInfo(test.filename, fi, false) if err != nil { t.Fatal(err) } diff --git a/internal/restic/node_windows.go b/internal/restic/node_windows.go index 5875c3ccd..7766a1ddf 100644 --- a/internal/restic/node_windows.go +++ b/internal/restic/node_windows.go @@ -78,6 +78,10 @@ func Listxattr(path string) ([]string, error) { return nil, nil } +func IsListxattrPermissionError(_ error) bool { + return false +} + // Setxattr associates name and data together as an attribute of path. func Setxattr(path, name string, data []byte) error { return nil diff --git a/internal/restic/node_windows_test.go b/internal/restic/node_windows_test.go index 501d5a98a..5a5a0a61c 100644 --- a/internal/restic/node_windows_test.go +++ b/internal/restic/node_windows_test.go @@ -165,7 +165,7 @@ func restoreAndGetNode(t *testing.T, tempDir string, testNode Node, warningExpec fi, err := os.Lstat(testPath) test.OK(t, errors.Wrapf(err, "Could not Lstat for path: %s", testPath)) - nodeFromFileInfo, err := NodeFromFileInfo(testPath, fi) + nodeFromFileInfo, err := NodeFromFileInfo(testPath, fi, false) test.OK(t, errors.Wrapf(err, "Could not get NodeFromFileInfo for path: %s", testPath)) return testPath, nodeFromFileInfo diff --git a/internal/restic/node_xattr.go b/internal/restic/node_xattr.go index 0b2d5d552..8b080e74f 100644 --- a/internal/restic/node_xattr.go +++ b/internal/restic/node_xattr.go @@ -25,6 +25,14 @@ func Listxattr(path string) ([]string, error) { return l, handleXattrErr(err) } +func IsListxattrPermissionError(err error) bool { + var xerr *xattr.Error + if errors.As(err, &xerr) { + return xerr.Op == "xattr.list" && errors.Is(xerr.Err, os.ErrPermission) + } + return false +} + // Setxattr associates name and data together as an attribute of path. func Setxattr(path, name string, data []byte) error { return handleXattrErr(xattr.LSet(path, name, data)) diff --git a/internal/restic/node_xattr_test.go b/internal/restic/node_xattr_test.go new file mode 100644 index 000000000..5ce77bd28 --- /dev/null +++ b/internal/restic/node_xattr_test.go @@ -0,0 +1,28 @@ +//go:build darwin || freebsd || linux || solaris +// +build darwin freebsd linux solaris + +package restic + +import ( + "os" + "testing" + + "github.com/pkg/xattr" + rtest "github.com/restic/restic/internal/test" +) + +func TestIsListxattrPermissionError(t *testing.T) { + xerr := &xattr.Error{ + Op: "xattr.list", + Name: "test", + Err: os.ErrPermission, + } + err := handleXattrErr(xerr) + rtest.Assert(t, err != nil, "missing error") + rtest.Assert(t, IsListxattrPermissionError(err), "expected IsListxattrPermissionError to return true for %v", err) + + xerr.Err = os.ErrNotExist + err = handleXattrErr(xerr) + rtest.Assert(t, err != nil, "missing error") + rtest.Assert(t, !IsListxattrPermissionError(err), "expected IsListxattrPermissionError to return false for %v", err) +} diff --git a/internal/restic/tree_test.go b/internal/restic/tree_test.go index da674eb1c..67ecec897 100644 --- a/internal/restic/tree_test.go +++ b/internal/restic/tree_test.go @@ -86,7 +86,7 @@ func TestNodeComparison(t *testing.T) { fi, err := os.Lstat("tree_test.go") rtest.OK(t, err) - node, err := restic.NodeFromFileInfo("tree_test.go", fi) + node, err := restic.NodeFromFileInfo("tree_test.go", fi, false) rtest.OK(t, err) n2 := *node @@ -127,7 +127,7 @@ func TestTreeEqualSerialization(t *testing.T) { for _, fn := range files[:i] { fi, err := os.Lstat(fn) rtest.OK(t, err) - node, err := restic.NodeFromFileInfo(fn, fi) + node, err := restic.NodeFromFileInfo(fn, fi, false) rtest.OK(t, err) rtest.OK(t, tree.Insert(node))