Check walker ancestor symlinks against the current root only

- A symlink in one root was skipped when its target was an ancestor of
  another root, hiding files reachable only through it
- Fix and tests by @mabunemeh

Close #4944

CHANGELOG.md

@@ -28,6 +28,7 @@ CHANGELOG
     - The completion integration now requires nushell 0.116.1 or above, where an interactive completer can take the terminal through the `@interactive` attribute (#4934) (@sim590)
 - Fixed `--no-sort` being ignored in filter mode with `--sync`, `--tac`, or `--bench` (#4940) (@NavpreetSSidhu)
 - Vim plugin: Fixed garbage text on screen in Neovim with `set title` when the fzf command contains ANSI escape sequences (#4941)
+- Fixed `--walker=follow` skipping a symlink in one `--walker-root` when its target is an ancestor of another root (#4944) (@mabunemeh)
 - Fixed O(n^2) action parsing in `--listen`. A single ~1 MB request could block the server for ~30 s (@zachariah-mithani, AploSec)
 
 0.74.5

src/reader.go

@@ -275,22 +275,12 @@ func (r *Reader) readFiles(roots []string, opts walkerOpts, ignores []string) bo
 		Sort:    fastwalk.SortFilesFirst,
 	}
 
-	// When following symlinks, precompute the absolute real paths of walker
+	// When following symlinks, skip targets that point to an ancestor of the
-	// roots so we can skip symlinks that point to an ancestor. fastwalk's
+	// current walker root. fastwalk's built-in loop detection (shouldTraverse)
-	// built-in loop detection (shouldTraverse) catches loops on the second
+	// catches loops on the second pass, but a single pass through a symlink
-	// pass, but a single pass through a symlink like z: -> / already
+	// like z: -> / already traverses the entire root filesystem, causing
-	// traverses the entire root filesystem, causing severe resource
+	// severe resource exhaustion. Skipping ancestor symlinks prevents this.
-	// exhaustion. Skipping ancestor symlinks prevents this entirely.
+	var absRoot string
-	var absRoots []string
-	if opts.follow {
-		for _, root := range roots {
-			if real, err := filepath.EvalSymlinks(root); err == nil {
-				if abs, err := filepath.Abs(real); err == nil {
-					absRoots = append(absRoots, filepath.Clean(abs))
-				}
-			}
-		}
-	}
 
 	ignoresBase := []string{}
 	ignoresFull := []string{}
@@ -326,19 +316,17 @@ func (r *Reader) readFiles(roots []string, opts walkerOpts, ignores []string) bo
 				return filepath.SkipDir
 			}
 			// Skip symlinks whose target is an ancestor of (or equal to)
-			// any walker root. Following such symlinks would traverse a
+			// the current walker root. Following such symlinks would traverse a
 			// superset of the tree we're already walking.
-			if isDirSymlink && len(absRoots) > 0 {
+			if isDirSymlink && absRoot != "" {
 				if target, err := filepath.EvalSymlinks(path); err == nil {
 					if abs, err := filepath.Abs(target); err == nil {
 						abs = filepath.Clean(abs)
 						if abs == string(os.PathSeparator) {
 							return filepath.SkipDir
 						}
-						for _, absRoot := range absRoots {
+						if absRoot == abs || strings.HasPrefix(absRoot, abs+string(os.PathSeparator)) {
-							if absRoot == abs || strings.HasPrefix(absRoot, abs+string(os.PathSeparator)) {
+							return filepath.SkipDir
-								return filepath.SkipDir
-							}
 						}
 					}
 				}
@@ -377,6 +365,16 @@ func (r *Reader) readFiles(roots []string, opts walkerOpts, ignores []string) bo
 	}
 	noerr := true
 	for _, root := range roots {
+		// Compare against this root only. Safe to update here because
+		// fastwalk.Walk returns after all its workers finish.
+		absRoot = ""
+		if opts.follow {
+			if real, err := filepath.EvalSymlinks(root); err == nil {
+				if abs, err := filepath.Abs(real); err == nil {
+					absRoot = filepath.Clean(abs)
+				}
+			}
+		}
 		noerr = noerr && (fastwalk.Walk(&conf, root, fn) == nil)
 	}
 	return noerr

src/reader_test.go

@@ -1,6 +1,10 @@
 package fzf
 
 import (
+	"os"
+	"path/filepath"
+	"slices"
+	"sync"
 	"testing"
 	"time"
 
@@ -66,3 +70,70 @@ func TestReadFromCommand(t *testing.T) {
 		t.Error("EvtReadFin should be set")
 	}
 }
+
+func walkFiles(t *testing.T, roots ...string) []string {
+	t.Helper()
+	var mu sync.Mutex
+	files := []string{}
+	reader := NewReader(func(data []byte) bool {
+		mu.Lock()
+		defer mu.Unlock()
+		files = append(files, filepath.FromSlash(string(data)))
+		return true
+	}, util.NewEventBox(), nil, false, false)
+	if !reader.readFiles(roots, walkerOpts{file: true, follow: true}, nil) {
+		t.Fatal("file walk failed")
+	}
+	slices.Sort(files)
+	return files
+}
+
+func TestReadFilesSymlinkAcrossRoots(t *testing.T) {
+	base := t.TempDir()
+	app := filepath.Join(base, "app")
+	shared := filepath.Join(base, "shared")
+	frontend := filepath.Join(shared, "frontend")
+	os.MkdirAll(app, 0755)
+	os.MkdirAll(frontend, 0755)
+	os.WriteFile(filepath.Join(shared, "docs.txt"), nil, 0644)
+	os.WriteFile(filepath.Join(frontend, "index.js"), nil, 0644)
+	if err := os.Symlink(filepath.Join("..", "shared"), filepath.Join(app, "shared")); err != nil {
+		t.Skipf("cannot create symlink: %v", err)
+	}
+
+	viaApp := []string{
+		filepath.Join(app, "shared", "docs.txt"),
+		filepath.Join(app, "shared", "frontend", "index.js"),
+	}
+	both := append(slices.Clone(viaApp), filepath.Join(frontend, "index.js"))
+	slices.Sort(both)
+	for _, tc := range []struct {
+		roots []string
+		want  []string
+	}{
+		{[]string{app}, viaApp},
+		{[]string{app, frontend}, both},
+		{[]string{frontend, app}, both},
+	} {
+		if files := walkFiles(t, tc.roots...); !slices.Equal(files, tc.want) {
+			t.Errorf("roots = %v, files = %v, want %v", tc.roots, files, tc.want)
+		}
+	}
+}
+
+func TestReadFilesSkipsAncestorSymlinks(t *testing.T) {
+	base := t.TempDir()
+	project := filepath.Join(base, "project")
+	os.Mkdir(project, 0755)
+	file := filepath.Join(project, "file.txt")
+	os.WriteFile(file, nil, 0644)
+	os.WriteFile(filepath.Join(base, "outside.txt"), nil, 0644)
+	for name, target := range map[string]string{"parent": "..", "self": "."} {
+		if err := os.Symlink(target, filepath.Join(project, name)); err != nil {
+			t.Skipf("cannot create symlink: %v", err)
+		}
+	}
+	if files := walkFiles(t, project); !slices.Equal(files, []string{file}) {
+		t.Errorf("files = %v, want [%s]", files, file)
+	}
+}