From bf054c09d21a2f83d8974e34614eb700954057e5 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Wed, 31 Jan 2024 20:48:03 +0100
Subject: [PATCH] backup: Ignore xattr.list permission error for parent
 directories

On FreeBSD, limited users may not be able to even list xattrs for the
parent directories above the snapshot source paths. As this can cause
the backup to fail, just ignore those errors.
---
 changelog/unreleased/issue-3600         | 11 ++++++++++
 internal/archiver/archiver.go           | 14 +++++++------
 internal/archiver/archiver_test.go      |  4 ++--
 internal/archiver/archiver_unix_test.go |  2 +-
 internal/archiver/file_saver.go         |  4 ++--
 internal/archiver/file_saver_test.go    |  4 ++--
 internal/restic/node.go                 | 13 +++++++-----
 internal/restic/node_aix.go             |  4 ++++
 internal/restic/node_netbsd.go          |  4 ++++
 internal/restic/node_openbsd.go         |  4 ++++
 internal/restic/node_test.go            | 10 ++++++---
 internal/restic/node_unix_test.go       |  2 +-
 internal/restic/node_windows.go         |  4 ++++
 internal/restic/node_windows_test.go    |  2 +-
 internal/restic/node_xattr.go           |  8 +++++++
 internal/restic/node_xattr_test.go      | 28 +++++++++++++++++++++++++
 internal/restic/tree_test.go            |  4 ++--
 17 files changed, 97 insertions(+), 25 deletions(-)
 create mode 100644 changelog/unreleased/issue-3600
 create mode 100644 internal/restic/node_xattr_test.go

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))