lib/fs: Don't panic when watching a folder with symlinked root (#4846)

lib/fs/basicfs.go

@@ -25,7 +25,8 @@ var (
 // The BasicFilesystem implements all aspects by delegating to package os.
 // All paths are relative to the root and cannot (should not) escape the root directory.
 type BasicFilesystem struct {
-	root string
+	root                 string
+	rootSymlinkEvaluated string
 }
 
 func newBasicFilesystem(root string) *BasicFilesystem {
@@ -52,21 +53,34 @@ func newBasicFilesystem(root string) *BasicFilesystem {
 		}
 	}
 
+	rootSymlinkEvaluated, err := filepath.EvalSymlinks(root)
+	if err != nil {
+		rootSymlinkEvaluated = root
+	}
+
+	return &BasicFilesystem{
+		root:                 adjustRoot(root),
+		rootSymlinkEvaluated: adjustRoot(rootSymlinkEvaluated),
+	}
+}
+
+func adjustRoot(root string) string {
 	// Attempt to enable long filename support on Windows. We may still not
 	// have an absolute path here if the previous steps failed.
 	if runtime.GOOS == "windows" {
 		if filepath.IsAbs(root) && !strings.HasPrefix(root, `\\`) {
 			root = `\\?\` + root
 		}
-		// If we're not on Windows, we want the path to end with a slash to
-		// penetrate symlinks. On Windows, paths must not end with a slash.
-	} else if root[len(root)-1] != filepath.Separator {
-		root = root + string(filepath.Separator)
+		return root
 	}
 
-	return &BasicFilesystem{
-		root: root,
+	// If we're not on Windows, we want the path to end with a slash to
+	// penetrate symlinks. On Windows, paths must not end with a slash.
+	if root[len(root)-1] != filepath.Separator {
+		root = root + string(filepath.Separator)
 	}
+
+	return root
 }
 
 // rooted expands the relative path to the full path that is then used with os
@@ -109,7 +123,15 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) {
 }
 
 func (f *BasicFilesystem) unrooted(path string) string {
-	return strings.TrimPrefix(strings.TrimPrefix(path, f.root), string(PathSeparator))
+	return rel(path, f.root)
+}
+
+func (f *BasicFilesystem) unrootedSymlinkEvaluated(path string) string {
+	return rel(path, f.rootSymlinkEvaluated)
+}
+
+func rel(path, prefix string) string {
+	return strings.TrimPrefix(strings.TrimPrefix(path, prefix), string(PathSeparator))
 }
 
 func (f *BasicFilesystem) Chmod(name string, mode FileMode) error {

lib/fs/basicfs_watch.go

@@ -12,6 +12,7 @@ import (
 	"context"
 	"errors"
 	"path/filepath"
+	"strings"
 
 	"github.com/syncthing/notify"
 )
@@ -47,12 +48,12 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 		return nil, err
 	}
 
-	go f.watchLoop(name, absName, backendChan, outChan, ignore, ctx)
+	go f.watchLoop(name, backendChan, outChan, ignore, ctx)
 
 	return outChan, nil
 }
 
-func (f *BasicFilesystem) watchLoop(name string, absName string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) {
+func (f *BasicFilesystem) watchLoop(name string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) {
 	for {
 		// Detect channel overflow
 		if len(backendChan) == backendBuffer {
@@ -105,12 +106,11 @@ func (f *BasicFilesystem) eventType(notifyType notify.Event) EventType {
 // special case when the given path is the folder root without a trailing
 // pathseparator.
 func (f *BasicFilesystem) unrootedChecked(absPath string) string {
-	if absPath+string(PathSeparator) == f.root {
+	if absPath+string(PathSeparator) == f.rootSymlinkEvaluated {
 		return "."
 	}
-	relPath := f.unrooted(absPath)
-	if relPath == absPath {
+	if !strings.HasPrefix(absPath, f.rootSymlinkEvaluated) {
 		panic("bug: Notify backend is processing a change outside of the watched path: " + absPath)
 	}
-	return relPath
+	return f.unrootedSymlinkEvaluated(absPath)
 }

lib/fs/basicfs_watch_test.go

@@ -123,7 +123,7 @@ func TestWatchOutside(t *testing.T) {
 			}
 			cancel()
 		}()
-		fs.watchLoop(".", testDirAbs, backendChan, outChan, fakeMatcher{}, ctx)
+		fs.watchLoop(".", backendChan, outChan, fakeMatcher{}, ctx)
 	}()
 
 	backendChan <- fakeEventInfo(filepath.Join(filepath.Dir(testDirAbs), "outside"))
@@ -139,7 +139,7 @@ func TestWatchSubpath(t *testing.T) {
 	fs := newBasicFilesystem(testDirAbs)
 
 	abs, _ := fs.rooted("sub")
-	go fs.watchLoop("sub", abs, backendChan, outChan, fakeMatcher{}, ctx)
+	go fs.watchLoop("sub", backendChan, outChan, fakeMatcher{}, ctx)
 
 	backendChan <- fakeEventInfo(filepath.Join(abs, "file"))
 
@@ -208,6 +208,54 @@ func TestWatchErrorLinuxInterpretation(t *testing.T) {
 	}
 }
 
+func TestWatchSymlinkedRoot(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Involves symlinks")
+	}
+
+	name := "symlinkedRoot"
+	if err := testFs.MkdirAll(name, 0755); err != nil {
+		panic(fmt.Sprintf("Failed to create directory %s: %s", name, err))
+	}
+	defer testFs.RemoveAll(name)
+
+	root := filepath.Join(name, "root")
+	if err := testFs.MkdirAll(root, 0777); err != nil {
+		panic(err)
+	}
+	link := filepath.Join(name, "link")
+
+	if err := testFs.CreateSymlink(filepath.Join(testFs.URI(), root), link); err != nil {
+		panic(err)
+	}
+
+	linkedFs := NewFilesystem(FilesystemTypeBasic, filepath.Join(testFs.URI(), link))
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	if _, err := linkedFs.Watch(".", fakeMatcher{}, ctx, false); err != nil {
+		panic(err)
+	}
+
+	if err := linkedFs.MkdirAll("foo", 0777); err != nil {
+		panic(err)
+	}
+
+	// Give the panic some time to happen
+	sleepMs(100)
+}
+
+func TestUnrootedChecked(t *testing.T) {
+	var unrooted string
+	defer func() {
+		if recover() == nil {
+			t.Fatal("unrootedChecked did not panic on outside path, but returned", unrooted)
+		}
+	}()
+	fs := newBasicFilesystem(testDirAbs)
+	unrooted = fs.unrootedChecked("/random/other/path")
+}
+
 // path relative to folder root, also creates parent dirs if necessary
 func createTestFile(name string, file string) string {
 	joined := filepath.Join(name, file)