Skip to content

Fix resolving chained symlinks with the WinFSP VFS#213

Draft
tomgr wants to merge 1 commit intobuildbarn:mainfrom
tomgr:bugfix/windows-chained-symlinks
Draft

Fix resolving chained symlinks with the WinFSP VFS#213
tomgr wants to merge 1 commit intobuildbarn:mainfrom
tomgr:bugfix/windows-chained-symlinks

Conversation

@tomgr
Copy link
Contributor

@tomgr tomgr commented Feb 16, 2026

It turns out that WinFSP's FspFileSystemFindReparsePoint only splits on backslashes when calculating reparse depth. Forward slashes in symlink targets (e.g. from VirtualSymlink) caused chained symlinks to fail. We now convert forward slashes to back slashes when returning the reparse point.

@aspect-workflows
Copy link

aspect-workflows bot commented Feb 16, 2026

Test

17 test targets passed

Targets
//pkg/blobstore:blobstore_test [k8-fastbuild]                                         124ms
//pkg/builder:builder_test [k8-fastbuild]                                             179ms
//pkg/cas:cas_test [k8-fastbuild]                                                     121ms
//pkg/cleaner:cleaner_test [k8-fastbuild]                                             164ms
//pkg/clock:clock_test [k8-fastbuild]                                                 177ms
//pkg/filesystem/access:access_test [k8-fastbuild]                                    49ms
//pkg/filesystem/pool:pool_test [k8-fastbuild]                                        158ms
//pkg/filesystem/virtual/fuse:fuse_test [k8-fastbuild]                                164ms
//pkg/filesystem/virtual/nfsv4:nfsv4_test [k8-fastbuild]                              203ms
//pkg/filesystem/virtual:virtual_test [k8-fastbuild]                                  189ms
//pkg/filesystem:filesystem_test [k8-fastbuild]                                       142ms
//pkg/runner:runner_test [k8-fastbuild]                                               132ms
//pkg/scheduler/initialsizeclass:initialsizeclass_test [k8-fastbuild]                 82ms
//pkg/scheduler/platform:platform_test [k8-fastbuild]                                 118ms
//pkg/scheduler/routing:routing_test [k8-fastbuild]                                   130ms
//pkg/scheduler:scheduler_test [k8-fastbuild]                                         286ms
//pkg/sync:sync_test [k8-fastbuild]                                                   128ms

Total test execution time was 3s. 2 tests (10.5%) were fully cached saving 187ms.

@tomgr tomgr force-pushed the bugfix/windows-chained-symlinks branch from c953f2d to 3bd8efc Compare February 16, 2026 22:40
@tomgr tomgr marked this pull request as draft February 17, 2026 07:54
@tomgr
Copy link
Contributor Author

tomgr commented Feb 17, 2026

Just converting to draft whilst I do a bit more testing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change this:

if err := path.Resolve(path.LocalFormat.NewParser(string(target)), &w); err != nil {

to:

targetParser := path.LocalFormat.NewParser(string(target))
if err := path.Resolve(targetParser, &w); err != nil {

Then we can also the path library to do the sanitization:

cleanPathBuilder, scopeWalker := path.EmptyPath.Join(path.VoidScopeWalker)
if err := path.Resolve(targetParser, scopeWalker); err != nil {
	return 0, err
}
cleanPath, err := cleanPathBuilder.GetWindowsString()
	return 0, err
}

I know it's more code, but at the same time it's a bit more future proof. At some point we may consider changing VirtualSymlink() and VirtualReadlink() to accept/return a path.Parser. That way the VFS API is completely oblivious of path separators. If/when that happens, this code will continue to work as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this but it required some work on the path library to make it correctly handle more exotic windows paths: see buildbarn/bb-storage#329.

Once that's merged I'll update this PR with a reference to the appropriate commit.

@tomgr tomgr force-pushed the bugfix/windows-chained-symlinks branch from 3bd8efc to 077e871 Compare February 17, 2026 12:57
@tomgr tomgr force-pushed the bugfix/windows-chained-symlinks branch from 077e871 to 2b574c1 Compare March 9, 2026 12:05
It turns out that WinFSP's FspFileSystemFindReparsePoint only splits on
backslashes when calculating reparse depth. Forward slashes in symlink
targets (e.g. from VirtualSymlink) caused chained symlinks to fail. We
now convert forward slashes to back slashes when returning the reparse
point.
@tomgr tomgr force-pushed the bugfix/windows-chained-symlinks branch from 2b574c1 to 93aca6b Compare March 9, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants