Fix resolving chained symlinks with the WinFSP VFS

MODULE.bazel

@@ -23,8 +23,8 @@ git_override(
 
 git_override(
     module_name = "com_github_buildbarn_bb_storage",
-    commit = "6002cad335378bbdcdda4225d47c7dc2c08a6d94",
-    remote = "https://github.com/buildbarn/bb-storage.git",
+    commit = "acd8c86d8c0d72d367447e5a1cb251aacea67beb",
+    remote = "https://github.com/tomgr/bb-storage.git",
 )
 
 git_override(

pkg/filesystem/virtual/winfsp/BUILD.bazel

@@ -74,6 +74,7 @@ go_test(
             "//pkg/proto/configuration/filesystem/virtual",
             "@com_github_buildbarn_bb_storage//pkg/blockdevice",
             "@com_github_buildbarn_bb_storage//pkg/clock",
+            "@com_github_buildbarn_bb_storage//pkg/filesystem/path",
             "@com_github_buildbarn_bb_storage//pkg/program",
             "@com_github_buildbarn_bb_storage//pkg/util",
             "@com_github_stretchr_testify//require",

pkg/filesystem/virtual/winfsp/file_system.go

@@ -481,15 +481,11 @@ func (d *directoryResolver) OnAbsolute() (path.ComponentWalker, error) {
 	return d, nil
 }
 
-func (directoryResolver) OnDriveLetter(drive rune) (path.ComponentWalker, error) {
-	return nil, windows.STATUS_OBJECT_NAME_NOT_FOUND
-}
-
 func (directoryResolver) OnRelative() (path.ComponentWalker, error) {
 	return nil, windows.STATUS_OBJECT_NAME_NOT_FOUND
 }
 
-func (directoryResolver) OnShare(server, share string) (path.ComponentWalker, error) {
+func (directoryResolver) OnWindowsRoot(root path.WindowsRootKind) (path.ComponentWalker, error) {
 	return nil, windows.STATUS_OBJECT_NAME_NOT_FOUND
 }
 
@@ -1318,30 +1314,6 @@ func FillSymlinkReparseBuffer(target string, flags uint32, buffer []byte) (int,
 	return requiredSize, nil
 }
 
-type relativePathChecker struct {
-	isRelative bool
-}
-
-func (d *relativePathChecker) OnAbsolute() (path.ComponentWalker, error) {
-	// This counts as relative as it's interpreted as relative to the
-	// current drive.
-	d.isRelative = true
-	return path.VoidComponentWalker, nil
-}
-
-func (relativePathChecker) OnDriveLetter(drive rune) (path.ComponentWalker, error) {
-	return path.VoidComponentWalker, nil
-}
-
-func (d *relativePathChecker) OnRelative() (path.ComponentWalker, error) {
-	d.isRelative = true
-	return path.VoidComponentWalker, nil
-}
-
-func (relativePathChecker) OnShare(server, share string) (path.ComponentWalker, error) {
-	return path.VoidComponentWalker, nil
-}
-
 func getReparsePointForLeaf(ctx context.Context, leaf virtual.Leaf, buffer []byte) (int, error) {
 	target, status := leaf.VirtualReadlink(ctx)
 	if status != virtual.StatusOK {
@@ -1352,17 +1324,32 @@ func getReparsePointForLeaf(ctx context.Context, leaf virtual.Leaf, buffer []byt
 		return 0, nil
 	}
 
-	// Parse the path to determine if it's absolute
-	w := relativePathChecker{}
-	if err := path.Resolve(path.LocalFormat.NewParser(string(target)), &w); err != nil {
+	// Normalize to backslashes. After reparse resolution the
+	// I/O manager re-issues the request with the substituted
+	// path. FspFileSystemFindReparsePoint splits on backslashes
+	// only, so forward slashes cause it to misidentify the
+	// reparse point's depth, breaking chained symlinks.
+	targetParser := path.LocalFormat.NewParser(string(target))
+	cleanPathBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
+	if err := path.Resolve(targetParser, scopeWalker); err != nil {
 		return 0, err
 	}
-	var flags int
-	if w.isRelative {
-		flags = windowsext.SYMLINK_FLAG_RELATIVE
+	var flags uint32
+	switch cleanPathBuilder.WindowsPathKind() {
+	case path.WindowsPathKindRelative, path.WindowsPathKindDriveRelative:
+		flags = uint32(windowsext.SYMLINK_FLAG_RELATIVE)
+	case path.WindowsPathKindAbsolute:
+	}
+	// Use NoTrailingSeparator to suppress the trailing "\"
+	// directory marker. WinFSP's FspFileSystemResolveReparsePointsInternal
+	// concatenates the substitute name with the remaining path
+	// (which starts with "\"), and FspFileNameIsValid rejects the
+	// resulting consecutive backslashes.
+	targetStr, err := cleanPathBuilder.GetWindowsString(path.WindowsPathFormatNoTrailingSeparator)
+	if err != nil {
+		return 0, err
 	}
-
-	return FillSymlinkReparseBuffer(string(target), uint32(flags), buffer)
+	return FillSymlinkReparseBuffer(targetStr, flags, buffer)
 }
 
 func (fs *FileSystem) GetReparsePoint(ref *ffi.FileSystemRef, file uintptr, name string, buffer []byte) (int, error) {

pkg/filesystem/virtual/winfsp/file_system_integration_test.go

@@ -21,6 +21,7 @@ import (
 	virtual_pb "github.com/buildbarn/bb-remote-execution/pkg/proto/configuration/filesystem/virtual"
 	"github.com/buildbarn/bb-storage/pkg/blockdevice"
 	"github.com/buildbarn/bb-storage/pkg/clock"
+	bb_path "github.com/buildbarn/bb-storage/pkg/filesystem/path"
 	"github.com/buildbarn/bb-storage/pkg/program"
 	"github.com/buildbarn/bb-storage/pkg/util"
 	"github.com/stretchr/testify/require"
@@ -39,7 +40,7 @@ func findFreeDriveLetter() (string, error) {
 	return "", fmt.Errorf("no free drive letters available")
 }
 
-func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice) {
+func createWinFSPForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice, virtual_configuration.Mount, virtual.Directory) {
 	// We can't run winfsp-tests at a directory path due to
 	// https://github.com/winfsp/winfsp/issues/279. Instead find a free drive
 	// letter and run it there instead.
@@ -75,38 +76,41 @@ func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, case
 
 	// Create a virtual directory to hold new files.
 	defaultAttributesSetter := func(requested virtual.AttributesMask, attributes *virtual.Attributes) {}
-	err = mount.Expose(
-		terminationGroup,
-		virtual.NewInMemoryPrepopulatedDirectory(
-			virtual.NewHandleAllocatingFileAllocator(
-				virtual.NewPoolBackedFileAllocator(
-					pool.NewBlockDeviceBackedFilePool(
-						bd,
-						pool.NewBitmapSectorAllocator(uint32(sectorCount)),
-						sectorSizeBytes,
-					),
-					util.DefaultErrorLogger,
-					defaultAttributesSetter,
-					virtual.NoNamedAttributesFactory,
+	rootDir := virtual.NewInMemoryPrepopulatedDirectory(
+		virtual.NewHandleAllocatingFileAllocator(
+			virtual.NewPoolBackedFileAllocator(
+				pool.NewBlockDeviceBackedFilePool(
+					bd,
+					pool.NewBitmapSectorAllocator(uint32(sectorCount)),
+					sectorSizeBytes,
 				),
-				handleAllocator,
-			),
-			virtual.NewHandleAllocatingSymlinkFactory(
-				virtual.BaseSymlinkFactory,
-				handleAllocator.New(),
+				util.DefaultErrorLogger,
+				defaultAttributesSetter,
+				virtual.NoNamedAttributesFactory,
 			),
-			util.DefaultErrorLogger,
 			handleAllocator,
-			sort.Sort,
-			func(s string) bool { return false },
-			clock.SystemClock,
-			normalizer,
-			defaultAttributesSetter,
-			virtual.NoNamedAttributesFactory,
 		),
+		virtual.NewHandleAllocatingSymlinkFactory(
+			virtual.BaseSymlinkFactory,
+			handleAllocator.New(),
+		),
+		util.DefaultErrorLogger,
+		handleAllocator,
+		sort.Sort,
+		func(s string) bool { return false },
+		clock.SystemClock,
+		normalizer,
+		defaultAttributesSetter,
+		virtual.NoNamedAttributesFactory,
 	)
-	require.NoError(t, err, "Failed to expose mount point")
 
+	return vfsPath, bd, mount, rootDir
+}
+
+func createWinFSPMountForTest(t *testing.T, terminationGroup program.Group, caseSensitive bool) (string, blockdevice.BlockDevice) {
+	vfsPath, bd, mount, rootDir := createWinFSPForTest(t, terminationGroup, caseSensitive)
+	err := mount.Expose(terminationGroup, rootDir)
+	require.NoError(t, err, "Failed to expose mount point")
 	return vfsPath, bd
 }
 
@@ -417,6 +421,112 @@ func TestWinFSPFileSystemGetSecurityByNameIntegration(t *testing.T) {
 	})
 }
 
+func TestWinFSPFileSystemStatFollowsSymlink(t *testing.T) {
+	program.RunLocal(context.Background(), func(ctx context.Context, siblingsGroup, dependenciesGroup program.Group) error {
+		// Pre-populate the virtual directory through the VFS API
+		// (not os.Symlink).
+		vfsPath, bd, mount, rootDir := createWinFSPForTest(t, dependenciesGroup, false)
+		defer bd.Close()
+
+		// Build a pnpm-style node_modules layout through the VFS
+		// API with chained symlinks.
+		var attrs virtual.Attributes
+
+		// The real directory.
+		storeDir, _, s := rootDir.VirtualMkdir(
+			bb_path.MustNewComponent("store"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+		_, _, s = storeDir.VirtualMkdir(
+			bb_path.MustNewComponent("pkg"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+
+		// Single symlink: store-link -> store/pkg.
+		_, _, s = rootDir.VirtualSymlink(
+			ctx,
+			[]byte("store/pkg"),
+			bb_path.MustNewComponent("store-link"),
+			0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+
+		// Create node_modules/.pnpm/pkg@1.0.0/node_modules/.
+		nmDir, _, s := rootDir.VirtualMkdir(
+			bb_path.MustNewComponent("node_modules"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+		pnpmDir, _, s := nmDir.VirtualMkdir(
+			bb_path.MustNewComponent(".pnpm"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+		pkgVerDir, _, s := pnpmDir.VirtualMkdir(
+			bb_path.MustNewComponent("pkg@1.0.0"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+		innerNmDir, _, s := pkgVerDir.VirtualMkdir(
+			bb_path.MustNewComponent("node_modules"), 0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+
+		// Inner symlink.
+		_, _, s = innerNmDir.VirtualSymlink(
+			ctx,
+			[]byte("../../../../store/pkg"),
+			bb_path.MustNewComponent("pkg"),
+			0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+
+		// Outer symlink.
+		_, _, s = nmDir.VirtualSymlink(
+			ctx,
+			[]byte(".pnpm/pkg@1.0.0/node_modules/pkg"),
+			bb_path.MustNewComponent("pkg"),
+			0, &attrs)
+		require.Equal(t, virtual.StatusOK, s)
+
+		require.NoError(t, mount.Expose(dependenciesGroup, rootDir))
+
+		// Write a file into the real directory after mounting.
+		testContent := []byte(`{"name":"pkg"}`)
+		require.NoError(t, os.WriteFile(
+			filepath.Join(vfsPath, "store", "pkg", "package.json"),
+			testContent, 0o644,
+		))
+
+		t.Run("SingleSymlink", func(t *testing.T) {
+			singleSymlinkPath := filepath.Join(vfsPath, "store-link")
+			info, err := os.Stat(singleSymlinkPath)
+			require.NoError(t, err)
+			require.True(t, info.IsDir())
+
+			content, err := os.ReadFile(filepath.Join(singleSymlinkPath, "package.json"))
+			require.NoError(t, err)
+			require.Equal(t, testContent, content)
+		})
+
+		t.Run("ChainedSymlinks", func(t *testing.T) {
+			symlinkPath := filepath.Join(vfsPath, "node_modules", "pkg")
+
+			info, err := os.Lstat(symlinkPath)
+			require.NoError(t, err)
+			require.NotZero(t, info.Mode()&os.ModeSymlink)
+
+			target, err := os.Readlink(symlinkPath)
+			require.NoError(t, err)
+			require.Equal(t, `.pnpm\pkg@1.0.0\node_modules\pkg`, target)
+
+			info, err = os.Stat(symlinkPath)
+			require.NoError(t, err)
+			require.True(t, info.IsDir())
+
+			content, err := os.ReadFile(filepath.Join(symlinkPath, "package.json"))
+			require.NoError(t, err)
+			require.Equal(t, testContent, content)
+
+			entries, err := os.ReadDir(symlinkPath)
+			require.NoError(t, err)
+			require.Len(t, entries, 1)
+			require.Equal(t, "package.json", entries[0].Name())
+		})
+
+		return nil
+	})
+}
+
 func TestWinFSPFileSystemCasePreserving(t *testing.T) {
 	program.RunLocal(context.Background(), func(ctx context.Context, siblingsGroup, dependenciesGroup program.Group) error {
 		vfsPath, bd := createWinFSPMountForTest(t, dependenciesGroup, false)