From 26ff936df038b6a62ec42cf18db27b2ea2bb8ec3 Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Sun, 25 Oct 2026 22:33:38 +0900 Subject: [PATCH] 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 | 1 + src/reader.go | 42 +++++++++++++-------------- src/reader_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01874e41..7ba811ea 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/reader.go b/src/reader.go index f05d311c..c85a488f 100644 --- a/src/reader.go +++ b/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 - // roots so we can skip symlinks that point to an ancestor. fastwalk's - // built-in loop detection (shouldTraverse) catches loops on the second - // pass, but a single pass through a symlink like z: -> / already - // traverses the entire root filesystem, causing severe resource - // exhaustion. Skipping ancestor symlinks prevents this entirely. - 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)) - } - } - } - } + // When following symlinks, skip targets that point to an ancestor of the + // current walker root. fastwalk's built-in loop detection (shouldTraverse) + // catches loops on the second pass, but a single pass through a symlink + // like z: -> / already traverses the entire root filesystem, causing + // severe resource exhaustion. Skipping ancestor symlinks prevents this. + var absRoot string 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)) { - return filepath.SkipDir - } + if absRoot == abs || strings.HasPrefix(absRoot, abs+string(os.PathSeparator)) { + 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 diff --git a/src/reader_test.go b/src/reader_test.go index cab4c017..71eb68bc 100644 --- a/src/reader_test.go +++ b/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) + } +}