Merge pull request #5097 from MichaelEischer/fix-vss-metadata

backup: read extended metadata from snapshot

changelog/unreleased/issue-5063

@@ -0,0 +1,10 @@
+Bugfix: Correctly `backup` extended metadata when using VSS on Windows
+
+On Windows, when creating a backup using the `--use-fs-snapshot` option,
+then the extended metadata was not read from the filesystem snapshot. This
+could result in errors when files have been removed in the meantime.
+
+This issue has been resolved.
+
+https://github.com/restic/restic/issues/5063
+https://github.com/restic/restic/pull/5097

cmd/restic/cmd_backup.go

@@ -97,6 +97,7 @@ type BackupOptions struct {
 }
 
 var backupOptions BackupOptions
+var backupFSTestHook func(fs fs.FS) fs.FS
 
 // ErrInvalidSourceData is used to report an incomplete backup
 var ErrInvalidSourceData = errors.New("at least one source file could not be read")
@@ -582,6 +583,10 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 		targets = []string{filename}
 	}
 
+	if backupFSTestHook != nil {
+		targetFS = backupFSTestHook(targetFS)
+	}
+
 	// rejectFuncs collect functions that can reject items from the backup based on path and file info
 	rejectFuncs, err := collectRejectFuncs(opts, targets, targetFS)
 	if err != nil {

cmd/restic/cmd_backup_integration_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"testing"
+	"time"
 
 	"github.com/restic/restic/internal/fs"
 	"github.com/restic/restic/internal/restic"
@@ -111,6 +112,63 @@ func TestBackupWithRelativePath(t *testing.T) {
 	rtest.Assert(t, latestSn.Parent != nil && latestSn.Parent.Equal(firstSnapshotID), "second snapshot selected unexpected parent %v instead of %v", latestSn.Parent, firstSnapshotID)
 }
 
+type vssDeleteOriginalFS struct {
+	fs.FS
+	testdata   string
+	hasRemoved bool
+}
+
+func (f *vssDeleteOriginalFS) Lstat(name string) (os.FileInfo, error) {
+	if !f.hasRemoved {
+		// call Lstat to trigger snapshot creation
+		_, _ = f.FS.Lstat(name)
+		// nuke testdata
+		var err error
+		for i := 0; i < 3; i++ {
+			// The CI sometimes runs into "The process cannot access the file because it is being used by another process" errors
+			// thus try a few times to remove the data
+			err = os.RemoveAll(f.testdata)
+			if err == nil {
+				break
+			}
+			time.Sleep(10 * time.Millisecond)
+		}
+		if err != nil {
+			return nil, err
+		}
+		f.hasRemoved = true
+	}
+	return f.FS.Lstat(name)
+}
+
+func TestBackupVSS(t *testing.T) {
+	if runtime.GOOS != "windows" || fs.HasSufficientPrivilegesForVSS() != nil {
+		t.Skip("vss fs test can only be run on windows with admin privileges")
+	}
+
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testSetupBackupData(t, env)
+	opts := BackupOptions{UseFsSnapshot: true}
+
+	var testFS *vssDeleteOriginalFS
+	backupFSTestHook = func(fs fs.FS) fs.FS {
+		testFS = &vssDeleteOriginalFS{
+			FS:       fs,
+			testdata: env.testdata,
+		}
+		return testFS
+	}
+	defer func() {
+		backupFSTestHook = nil
+	}()
+
+	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts)
+	testListSnapshots(t, env.gopts, 1)
+	rtest.Equals(t, true, testFS.hasRemoved, "testdata was not removed")
+}
+
 func TestBackupParentSelection(t *testing.T) {
 	env, cleanup := withTestEnvironment(t)
 	defer cleanup()

cmd/restic/integration_helpers_test.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"sync"
 	"testing"
 
@@ -168,6 +169,16 @@ type testEnvironment struct {
 	gopts                                   GlobalOptions
 }
 
+type logOutputter struct {
+	t testing.TB
+}
+
+func (l *logOutputter) Write(p []byte) (n int, err error) {
+	l.t.Helper()
+	l.t.Log(strings.TrimSuffix(string(p), "\n"))
+	return len(p), nil
+}
+
 // withTestEnvironment creates a test environment and returns a cleanup
 // function which removes it.
 func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) {
@@ -200,8 +211,11 @@ func withTestEnvironment(t testing.TB) (env *testEnvironment, cleanup func()) {
 		Quiet:    true,
 		CacheDir: env.cache,
 		password: rtest.TestPassword,
-		stdout:   os.Stdout,
+		// stdout and stderr are written to by Warnf etc. That is the written data
-		stderr:   os.Stderr,
+		// usually consists of one or multiple lines and therefore can be handled well
+		// by t.Log.
+		stdout:   &logOutputter{t},
+		stderr:   &logOutputter{t},
 		extended: make(options.Options),
 
 		// replace this hook with "nil" if listing a filetype more than once is necessary

internal/archiver/file_saver.go

@@ -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, false)
+	node, err := s.NodeFromFileInfo(snPath, target, fi, false)
 	if err != nil {
 		_ = f.Close()
 		completeError(err)

internal/fs/fs_local_vss.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/options"
+	"github.com/restic/restic/internal/restic"
 )
 
 // VSSConfig holds extended options of windows volume shadow copy service.
@@ -127,17 +128,21 @@ func (fs *LocalVss) DeleteSnapshots() {
 
 // OpenFile wraps the Open method of the underlying file system.
 func (fs *LocalVss) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
-	return os.OpenFile(fs.snapshotPath(name), flag, perm)
+	return fs.FS.OpenFile(fs.snapshotPath(name), flag, perm)
 }
 
 // Stat wraps the Stat method of the underlying file system.
 func (fs *LocalVss) Stat(name string) (os.FileInfo, error) {
-	return os.Stat(fs.snapshotPath(name))
+	return fs.FS.Stat(fs.snapshotPath(name))
 }
 
 // Lstat wraps the Lstat method of the underlying file system.
 func (fs *LocalVss) Lstat(name string) (os.FileInfo, error) {
-	return os.Lstat(fs.snapshotPath(name))
+	return fs.FS.Lstat(fs.snapshotPath(name))
+}
+
+func (fs *LocalVss) NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) {
+	return fs.FS.NodeFromFileInfo(fs.snapshotPath(path), fi, ignoreXattrListError)
 }
 
 // isMountPointIncluded  is true if given mountpoint included by user.

internal/fs/fs_local_vss_test.go

@@ -5,13 +5,18 @@ package fs
 
 import (
 	"fmt"
+	"io"
+	"os"
+	"path/filepath"
 	"regexp"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
 
 	ole "github.com/go-ole/go-ole"
 	"github.com/restic/restic/internal/options"
+	rtest "github.com/restic/restic/internal/test"
 )
 
 func matchStrings(ptrs []string, strs []string) bool {
@@ -284,3 +289,56 @@ func TestParseProvider(t *testing.T) {
 		})
 	}
 }
+
+func TestVSSFS(t *testing.T) {
+	if runtime.GOOS != "windows" || HasSufficientPrivilegesForVSS() != nil {
+		t.Skip("vss fs test can only be run on windows with admin privileges")
+	}
+
+	cfg, err := ParseVSSConfig(options.Options{})
+	rtest.OK(t, err)
+
+	errorHandler := func(item string, err error) {
+		t.Fatalf("unexpected error (%v)", err)
+	}
+	messageHandler := func(msg string, args ...interface{}) {
+		if strings.HasPrefix(msg, "creating VSS snapshot for") || strings.HasPrefix(msg, "successfully created snapshot") {
+			return
+		}
+		t.Fatalf("unexpected message (%s)", fmt.Sprintf(msg, args))
+	}
+
+	localVss := NewLocalVss(errorHandler, messageHandler, cfg)
+	defer localVss.DeleteSnapshots()
+
+	tempdir := t.TempDir()
+	tempfile := filepath.Join(tempdir, "file")
+	rtest.OK(t, os.WriteFile(tempfile, []byte("example"), 0o600))
+
+	// trigger snapshot creation and
+	// capture FI while file still exists (should already be within the snapshot)
+	origFi, err := localVss.Stat(tempfile)
+	rtest.OK(t, err)
+
+	// remove original file
+	rtest.OK(t, os.Remove(tempfile))
+
+	statFi, err := localVss.Stat(tempfile)
+	rtest.OK(t, err)
+	rtest.Equals(t, origFi.Mode(), statFi.Mode())
+
+	lstatFi, err := localVss.Lstat(tempfile)
+	rtest.OK(t, err)
+	rtest.Equals(t, origFi.Mode(), lstatFi.Mode())
+
+	f, err := localVss.OpenFile(tempfile, os.O_RDONLY, 0)
+	rtest.OK(t, err)
+	data, err := io.ReadAll(f)
+	rtest.OK(t, err)
+	rtest.Equals(t, "example", string(data), "unexpected file content")
+	rtest.OK(t, f.Close())
+
+	node, err := localVss.NodeFromFileInfo(tempfile, statFi, false)
+	rtest.OK(t, err)
+	rtest.Equals(t, node.Mode, statFi.Mode())
+}

internal/fs/fs_reader.go

@@ -232,7 +232,7 @@ func (r *readerFile) Close() error {
 var _ File = &readerFile{}
 
 // fakeFile implements all File methods, but only returns errors for anything
-// except Stat() and Name().
+// except Stat()
 type fakeFile struct {
 	name string
 	os.FileInfo
@@ -257,10 +257,6 @@ func (f fakeFile) Stat() (os.FileInfo, error) {
 	return f.FileInfo, nil
 }
 
-func (f fakeFile) Name() string {
-	return f.name
-}
-
 // fakeDir implements Readdirnames and Readdir, everything else is delegated to fakeFile.
 type fakeDir struct {
 	entries []os.FileInfo

internal/fs/interface.go

@@ -34,5 +34,4 @@ type File interface {
 
 	Readdirnames(n int) ([]string, error)
 	Stat() (os.FileInfo, error)
-	Name() string
 }