diff --git a/changelog/unreleased/pull-4958 b/changelog/unreleased/pull-4958 new file mode 100644 index 000000000..bbb28a97b --- /dev/null +++ b/changelog/unreleased/pull-4958 @@ -0,0 +1,7 @@ +Bugfix: Don't ignore metadata-setting errors during restore + +Restic was accidentally ignoring errors when setting timestamps, +attributes, or file modes during restore. It will now report those +errors (unless it's just a permission error when not running as root). + +https://github.com/restic/restic/pull/4958 diff --git a/internal/restic/node.go b/internal/restic/node.go index 51c6071b7..7c1988227 100644 --- a/internal/restic/node.go +++ b/internal/restic/node.go @@ -229,6 +229,13 @@ func (node *Node) CreateAt(ctx context.Context, path string, repo BlobLoader) er func (node Node) RestoreMetadata(path string, warn func(msg string)) error { err := node.restoreMetadata(path, warn) if err != nil { + // It is common to have permission errors for folders like /home + // unless you're running as root, so ignore those. + if os.Geteuid() > 0 && errors.Is(err, os.ErrPermission) { + debug.Log("not running as root, ignoring permission error for %v: %v", + path, err) + return nil + } debug.Log("restoreMetadata(%s) error %v", path, err) } @@ -239,33 +246,26 @@ func (node Node) restoreMetadata(path string, warn func(msg string)) error { var firsterr error if err := lchown(path, int(node.UID), int(node.GID)); err != nil { - // Like "cp -a" and "rsync -a" do, we only report lchown permission errors - // if we run as root. - if os.Geteuid() > 0 && os.IsPermission(err) { - debug.Log("not running as root, ignoring lchown permission error for %v: %v", - path, err) - } else { - firsterr = errors.WithStack(err) - } + firsterr = errors.WithStack(err) } if err := node.RestoreTimestamps(path); err != nil { debug.Log("error restoring timestamps for dir %v: %v", path, err) - if firsterr != nil { + if firsterr == nil { firsterr = err } } if err := node.restoreExtendedAttributes(path); err != nil { debug.Log("error restoring extended attributes for %v: %v", path, err) - if firsterr != nil { + if firsterr == nil { firsterr = err } } if err := node.restoreGenericAttributes(path, warn); err != nil { debug.Log("error restoring generic attributes for %v: %v", path, err) - if firsterr != nil { + if firsterr == nil { firsterr = err } } @@ -275,7 +275,7 @@ func (node Node) restoreMetadata(path string, warn func(msg string)) error { // calls above would fail. if node.Type != "symlink" { if err := fs.Chmod(path, node.Mode); err != nil { - if firsterr != nil { + if firsterr == nil { firsterr = errors.WithStack(err) } } diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index 6e0f31e21..7991d33e0 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/test" rtest "github.com/restic/restic/internal/test" ) @@ -382,3 +383,14 @@ func TestSymlinkSerializationFormat(t *testing.T) { test.Assert(t, n2.LinkTargetRaw == nil, "quoted link target is just a helper field and must be unset after decoding") } } + +func TestNodeRestoreMetadataError(t *testing.T) { + tempdir := t.TempDir() + + node := nodeTests[0] + nodePath := filepath.Join(tempdir, node.Name) + + // This will fail because the target file does not exist + err := node.RestoreMetadata(nodePath, func(msg string) { rtest.OK(t, fmt.Errorf("Warning triggered for path: %s: %s", nodePath, msg)) }) + test.Assert(t, errors.Is(err, os.ErrNotExist), "failed for an unexpected reason") +}