Merge pull request #5112 from MichaelEischer/fix-vss-root-volume

Fix VSS metadata error (master)

changelog/unreleased/issue-5107

@@ -0,0 +1,15 @@
+Bugfix: Fix metadata error on Windows for backups using VSS
+
+Since restic 0.17.2, when creating a backup on Windows using `--use-fs-snapshot`,
+restic would report an error like the following:
+
+```
+error: incomplete metadata for C:\: get EA failed while opening file handle for path \\?\GLOBALROOT\Device\HarddiskVolumeShadowCopyXX\, with: The process cannot access the file because it is being used by another process.
+```
+
+This has now been fixed by correctly handling paths that refer to volume
+shadow copy snapshots.
+
+https://github.com/restic/restic/issues/5107
+https://github.com/restic/restic/pull/5110
+https://github.com/restic/restic/pull/5112

cmd/restic/cmd_backup_integration_test.go

@@ -31,7 +31,7 @@ func testRunBackupAssumeFailure(t testing.TB, dir string, target []string, opts
 
 func testRunBackup(t testing.TB, dir string, target []string, opts BackupOptions, gopts GlobalOptions) {
 	err := testRunBackupAssumeFailure(t, dir, target, opts, gopts)
-	rtest.Assert(t, err == nil, "Error while backing up")
+	rtest.Assert(t, err == nil, "Error while backing up: %v", err)
 }
 
 func TestBackup(t *testing.T) {
@@ -52,14 +52,14 @@ func testBackup(t *testing.T, useFsSnapshot bool) {
 	opts := BackupOptions{UseFsSnapshot: useFsSnapshot}
 
 	// first backup
-	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts)
+	testRunBackup(t, "", []string{env.testdata}, opts, env.gopts)
 	testListSnapshots(t, env.gopts, 1)
 
 	testRunCheck(t, env.gopts)
 	stat1 := dirStats(env.repo)
 
 	// second backup, implicit incremental
-	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts)
+	testRunBackup(t, "", []string{env.testdata}, opts, env.gopts)
 	snapshotIDs := testListSnapshots(t, env.gopts, 2)
 
 	stat2 := dirStats(env.repo)
@@ -71,7 +71,7 @@ func testBackup(t *testing.T, useFsSnapshot bool) {
 	testRunCheck(t, env.gopts)
 	// third backup, explicit incremental
 	opts.Parent = snapshotIDs[0].String()
-	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts)
+	testRunBackup(t, "", []string{env.testdata}, opts, env.gopts)
 	snapshotIDs = testListSnapshots(t, env.gopts, 3)
 
 	stat3 := dirStats(env.repo)
@@ -84,7 +84,7 @@ func testBackup(t *testing.T, useFsSnapshot bool) {
 	for i, snapshotID := range snapshotIDs {
 		restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i))
 		t.Logf("restoring snapshot %v to %v", snapshotID.Str(), restoredir)
-		testRunRestore(t, env.gopts, restoredir, snapshotID)
+		testRunRestore(t, env.gopts, restoredir, snapshotID.String()+":"+toPathInSnapshot(filepath.Dir(env.testdata)))
 		diff := directoriesContentsDiff(env.testdata, filepath.Join(restoredir, "testdata"))
 		rtest.Assert(t, diff == "", "directories are not equal: %v", diff)
 	}
@@ -92,6 +92,20 @@ func testBackup(t *testing.T, useFsSnapshot bool) {
 	testRunCheck(t, env.gopts)
 }
 
+func toPathInSnapshot(path string) string {
+	// use path as is on most platforms, but convert it on windows
+	if runtime.GOOS == "windows" {
+		// the path generated by the test is always local so take the shortcut
+		vol := filepath.VolumeName(path)
+		if vol[len(vol)-1] != ':' {
+			panic(fmt.Sprintf("unexpected path: %q", path))
+		}
+		path = vol[:len(vol)-1] + string(filepath.Separator) + path[len(vol)+1:]
+		path = filepath.ToSlash(path)
+	}
+	return path
+}
+
 func TestBackupWithRelativePath(t *testing.T) {
 	env, cleanup := withTestEnvironment(t)
 	defer cleanup()
@@ -557,7 +571,7 @@ func TestHardLink(t *testing.T) {
 	for i, snapshotID := range snapshotIDs {
 		restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i))
 		t.Logf("restoring snapshot %v to %v", snapshotID.Str(), restoredir)
-		testRunRestore(t, env.gopts, restoredir, snapshotID)
+		testRunRestore(t, env.gopts, restoredir, snapshotID.String())
 		diff := directoriesContentsDiff(env.testdata, filepath.Join(restoredir, "testdata"))
 		rtest.Assert(t, diff == "", "directories are not equal %v", diff)
 

cmd/restic/cmd_copy_integration_test.go

@@ -62,11 +62,11 @@ func TestCopy(t *testing.T) {
 	for i, snapshotID := range snapshotIDs {
 		restoredir := filepath.Join(env.base, fmt.Sprintf("restore%d", i))
 		origRestores[restoredir] = struct{}{}
-		testRunRestore(t, env.gopts, restoredir, snapshotID)
+		testRunRestore(t, env.gopts, restoredir, snapshotID.String())
 	}
 	for i, snapshotID := range copiedSnapshotIDs {
 		restoredir := filepath.Join(env2.base, fmt.Sprintf("restore%d", i))
-		testRunRestore(t, env2.gopts, restoredir, snapshotID)
+		testRunRestore(t, env2.gopts, restoredir, snapshotID.String())
 		foundMatch := false
 		for cmpdir := range origRestores {
 			diff := directoriesContentsDiff(restoredir, cmpdir)

cmd/restic/cmd_restore_integration_test.go

@@ -17,17 +17,17 @@ import (
 	"github.com/restic/restic/internal/ui/termstatus"
 )
 
-func testRunRestore(t testing.TB, opts GlobalOptions, dir string, snapshotID restic.ID) {
+func testRunRestore(t testing.TB, opts GlobalOptions, dir string, snapshotID string) {
 	testRunRestoreExcludes(t, opts, dir, snapshotID, nil)
 }
 
-func testRunRestoreExcludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, excludes []string) {
+func testRunRestoreExcludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID string, excludes []string) {
 	opts := RestoreOptions{
 		Target: dir,
 	}
 	opts.Excludes = excludes
 
-	rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts))
+	rtest.OK(t, testRunRestoreAssumeFailure(snapshotID, opts, gopts))
 }
 
 func testRunRestoreAssumeFailure(snapshotID string, opts RestoreOptions, gopts GlobalOptions) error {
@@ -197,7 +197,7 @@ func TestRestoreFilter(t *testing.T) {
 	snapshotID := testListSnapshots(t, env.gopts, 1)[0]
 
 	// no restore filter should restore all files
-	testRunRestore(t, env.gopts, filepath.Join(env.base, "restore0"), snapshotID)
+	testRunRestore(t, env.gopts, filepath.Join(env.base, "restore0"), snapshotID.String())
 	for _, testFile := range testfiles {
 		rtest.OK(t, testFileSize(filepath.Join(env.base, "restore0", "testdata", testFile.name), int64(testFile.size)))
 	}
@@ -219,7 +219,7 @@ func TestRestoreFilter(t *testing.T) {
 
 	// restore with excludes
 	restoredir := filepath.Join(env.base, "restore-with-excludes")
-	testRunRestoreExcludes(t, env.gopts, restoredir, snapshotID, excludePatterns)
+	testRunRestoreExcludes(t, env.gopts, restoredir, snapshotID.String(), excludePatterns)
 	testRestoredFileExclusions(t, restoredir)
 
 	// Create an exclude file with some patterns
@@ -339,7 +339,7 @@ func TestRestoreWithPermissionFailure(t *testing.T) {
 
 	_ = withRestoreGlobalOptions(func() error {
 		globalOptions.stderr = io.Discard
-		testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshots[0])
+		testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshots[0].String())
 		return nil
 	})
 

cmd/restic/integration_test.go

@@ -35,7 +35,7 @@ func TestCheckRestoreNoLock(t *testing.T) {
 	testRunCheck(t, env.gopts)
 
 	snapshotIDs := testListSnapshots(t, env.gopts, 4)
-	testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshotIDs[0])
+	testRunRestore(t, env.gopts, filepath.Join(env.base, "restore"), snapshotIDs[0].String())
 }
 
 // a listOnceBackend only allows listing once per filetype

internal/fs/ea_windows.go

@@ -8,7 +8,6 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"strings"
 	"syscall"
 	"unsafe"
 
@@ -299,20 +298,3 @@ func pathSupportsExtendedAttributes(path string) (supported bool, err error) {
 	supported = (fileSystemFlags & windows.FILE_SUPPORTS_EXTENDED_ATTRIBUTES) != 0
 	return supported, nil
 }
-
-// getVolumePathName returns the volume path name for the given path.
-func getVolumePathName(path string) (volumeName string, err error) {
-	utf16Path, err := windows.UTF16PtrFromString(path)
-	if err != nil {
-		return "", err
-	}
-	// Get the volume path (e.g., "D:")
-	var volumePath [windows.MAX_PATH + 1]uint16
-	err = windows.GetVolumePathName(utf16Path, &volumePath[0], windows.MAX_PATH+1)
-	if err != nil {
-		return "", err
-	}
-	// Trim any trailing backslashes
-	volumeName = strings.TrimRight(windows.UTF16ToString(volumePath[:]), "\\")
-	return volumeName, nil
-}

internal/fs/ea_windows_test.go

@@ -10,7 +10,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"strings"
 	"syscall"
 	"testing"
 	"unsafe"
@@ -278,46 +277,3 @@ func TestPathSupportsExtendedAttributes(t *testing.T) {
 		t.Error("Expected an error for non-existent path, but got nil")
 	}
 }
-
-func TestGetVolumePathName(t *testing.T) {
-	tempDirVolume := filepath.VolumeName(os.TempDir())
-	testCases := []struct {
-		name           string
-		path           string
-		expectedPrefix string
-	}{
-		{
-			name:           "Root directory",
-			path:           os.Getenv("SystemDrive") + `\`,
-			expectedPrefix: os.Getenv("SystemDrive"),
-		},
-		{
-			name:           "Nested directory",
-			path:           os.Getenv("SystemDrive") + `\Windows\System32`,
-			expectedPrefix: os.Getenv("SystemDrive"),
-		},
-		{
-			name:           "Temp directory",
-			path:           os.TempDir() + `\`,
-			expectedPrefix: tempDirVolume,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			volumeName, err := getVolumePathName(tc.path)
-			if err != nil {
-				t.Fatalf("Unexpected error: %v", err)
-			}
-			if !strings.HasPrefix(volumeName, tc.expectedPrefix) {
-				t.Errorf("Expected volume name to start with %s, but got %s", tc.expectedPrefix, volumeName)
-			}
-		})
-	}
-
-	// Test with an invalid path
-	_, err := getVolumePathName("Z:\\NonExistentPath")
-	if err == nil {
-		t.Error("Expected an error for non-existent path, but got nil")
-	}
-}

internal/fs/file_windows.go

@@ -18,19 +18,28 @@ func fixpath(name string) string {
 	abspath, err := filepath.Abs(name)
 	if err == nil {
 		// Check if \\?\UNC\ already exist
-		if strings.HasPrefix(abspath, `\\?\UNC\`) {
+		if strings.HasPrefix(abspath, uncPathPrefix) {
+			return abspath
+		}
+		// Check if \\?\GLOBALROOT exists which marks volume shadow copy snapshots
+		if strings.HasPrefix(abspath, globalRootPrefix) {
+			if strings.Count(abspath, `\`) == 5 {
+				// Append slash if this just a volume name, e.g. `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopyXX`
+				// Without the trailing slash any access to the volume itself will fail.
+				return abspath + string(filepath.Separator)
+			}
 			return abspath
 		}
 		// Check if \\?\ already exist
-		if strings.HasPrefix(abspath, `\\?\`) {
+		if strings.HasPrefix(abspath, extendedPathPrefix) {
 			return abspath
 		}
 		// Check if path starts with \\
 		if strings.HasPrefix(abspath, `\\`) {
-			return strings.Replace(abspath, `\\`, `\\?\UNC\`, 1)
+			return strings.Replace(abspath, `\\`, uncPathPrefix, 1)
 		}
 		// Normal path
-		return `\\?\` + abspath
+		return extendedPathPrefix + abspath
 	}
 	return name
 }

internal/fs/fs_local_vss.go

@@ -176,7 +176,7 @@ func (fs *LocalVss) snapshotPath(path string) string {
 		return path
 	}
 
-	fixPath = strings.TrimPrefix(fixpath(path), `\\?\`)
+	fixPath = strings.TrimPrefix(fixPath, `\\?\`)
 	fixPathLower := strings.ToLower(fixPath)
 	volumeName := filepath.VolumeName(fixPath)
 	volumeNameLower := strings.ToLower(volumeName)

internal/fs/node_windows.go

@@ -325,8 +325,11 @@ func nodeFillGenericAttributes(node *restic.Node, path string, stat *ExtendedFil
 		return false, nil
 	}
 
-	if strings.HasSuffix(filepath.Clean(path), `\`) {
+	isVolume, err := isVolumePath(path)
-		// filepath.Clean(path) ends with '\' for Windows root volume paths only
+	if err != nil {
+		return false, err
+	}
+	if isVolume {
 		// Do not process file attributes, created time and sd for windows root volume paths
 		// Security descriptors are not supported for root volume paths.
 		// Though file attributes and created time are supported for root volume paths,
@@ -335,7 +338,7 @@ func nodeFillGenericAttributes(node *restic.Node, path string, stat *ExtendedFil
 		if err != nil {
 			return false, err
 		}
-		return allowExtended, nil
+		return allowExtended, err
 	}
 
 	var sd *[]byte
@@ -420,6 +423,35 @@ func checkAndStoreEASupport(path string) (isEASupportedVolume bool, err error) {
 	return isEASupportedVolume, err
 }
 
+// getVolumePathName returns the volume path name for the given path.
+func getVolumePathName(path string) (volumeName string, err error) {
+	utf16Path, err := windows.UTF16PtrFromString(path)
+	if err != nil {
+		return "", err
+	}
+	// Get the volume path (e.g., "D:")
+	var volumePath [windows.MAX_PATH + 1]uint16
+	err = windows.GetVolumePathName(utf16Path, &volumePath[0], windows.MAX_PATH+1)
+	if err != nil {
+		return "", err
+	}
+	// Trim any trailing backslashes
+	volumeName = strings.TrimRight(windows.UTF16ToString(volumePath[:]), "\\")
+	return volumeName, nil
+}
+
+// isVolumePath returns whether a path refers to a volume
+func isVolumePath(path string) (bool, error) {
+	volName, err := prepareVolumeName(path)
+	if err != nil {
+		return false, err
+	}
+
+	cleanPath := filepath.Clean(path)
+	cleanVolume := filepath.Clean(volName + `\`)
+	return cleanPath == cleanVolume, nil
+}
+
 // prepareVolumeName prepares the volume name for different cases in Windows
 func prepareVolumeName(path string) (volumeName string, err error) {
 	// Check if it's an extended length path

internal/fs/node_windows_test.go

@@ -451,10 +451,17 @@ func TestPrepareVolumeName(t *testing.T) {
 			expectError:         false,
 			expectedEASupported: false,
 		},
+		{
+			name:                "Volume Shadow Copy root",
+			path:                `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`,
+			expectedVolume:      `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`,
+			expectError:         false,
+			expectedEASupported: false,
+		},
 		{
 			name:                "Volume Shadow Copy path",
-			path:                `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1\Users\test`,
+			path:                `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555\Users\test`,
-			expectedVolume:      `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1`,
+			expectedVolume:      `\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy5555`,
 			expectError:         false,
 			expectedEASupported: false,
 		},
@@ -526,3 +533,46 @@ func getOSVolumeGUIDPath(t *testing.T) string {
 
 	return windows.UTF16ToString(volumeGUID[:])
 }
+
+func TestGetVolumePathName(t *testing.T) {
+	tempDirVolume := filepath.VolumeName(os.TempDir())
+	testCases := []struct {
+		name           string
+		path           string
+		expectedPrefix string
+	}{
+		{
+			name:           "Root directory",
+			path:           os.Getenv("SystemDrive") + `\`,
+			expectedPrefix: os.Getenv("SystemDrive"),
+		},
+		{
+			name:           "Nested directory",
+			path:           os.Getenv("SystemDrive") + `\Windows\System32`,
+			expectedPrefix: os.Getenv("SystemDrive"),
+		},
+		{
+			name:           "Temp directory",
+			path:           os.TempDir() + `\`,
+			expectedPrefix: tempDirVolume,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			volumeName, err := getVolumePathName(tc.path)
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if !strings.HasPrefix(volumeName, tc.expectedPrefix) {
+				t.Errorf("Expected volume name to start with %s, but got %s", tc.expectedPrefix, volumeName)
+			}
+		})
+	}
+
+	// Test with an invalid path
+	_, err := getVolumePathName("Z:\\NonExistentPath")
+	if err == nil {
+		t.Error("Expected an error for non-existent path, but got nil")
+	}
+}