Fix symlink handling in walk

Fixes gopasspw#2402

RELEASE_NOTES=[BUGFIX] Fix symlink deduplication.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

docs/config.md

@@ -47,6 +47,7 @@ Variables not exclusively used by gopass:
 | `GIT_AUTHOR_NAME`      | `string` | name of the author, used by the rcs backend to create a commit                                         |
 | `GIT_AUTHOR_EMAIL`     | `string` | email of the author, used by the rcs backend to create a commit                                        |
 | `NO_COLOR`             | `bool`   | disable color output. See [no-color.org](https://no-color.org) for more information.                   |
+| `MAXSYMLINKS`          | `int`    | Symlink resolution maximum depth. |
 
 ## Configuration Options
 

internal/backend/storage/fs/walk.go

@@ -4,52 +4,38 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-
-	"github.com/gopasspw/gopass/pkg/debug"
 )
 
 func walkSymlinks(path string, walkFn filepath.WalkFunc) error {
-	w := &walker{
-		seen: map[string]bool{},
-	}
-
-	return w.walk(path, path, walkFn)
+	return walk(path, path, walkFn)
 }
 
-type walker struct {
-	seen map[string]bool
-}
-
-func (w *walker) walk(filename, linkDir string, walkFn filepath.WalkFunc) error {
+func walk(filename, linkDir string, walkFn filepath.WalkFunc) error {
 	sWalkFn := func(path string, info fs.FileInfo, err error) error {
 		fname, err := filepath.Rel(filename, path)
 		if err != nil {
 			return err
 		}
 		path = filepath.Join(linkDir, fname)
 
-		if info.Mode()&fs.ModeSymlink == fs.ModeSymlink {
-			destPath, err := filepath.EvalSymlinks(path)
-			if err != nil {
-				return err
-			}
-
-			// avoid loops
-			if w.seen[destPath] {
-				debug.Log("Symlink loop detected at %s!", destPath)
+		// handle non-symlinks
+		if info.Mode()&fs.ModeSymlink != fs.ModeSymlink {
+			return walkFn(path, info, err)
+		}
 
-				return nil
-			}
-			w.seen[destPath] = true
+		// handle symlinks
+		destPath, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			return err
+		}
 
-			destInfo, err := os.Lstat(destPath)
-			if err != nil {
-				return walkFn(path, destInfo, err)
-			}
+		destInfo, err := os.Lstat(destPath)
+		if err != nil {
+			return walkFn(path, destInfo, err)
+		}
 
-			if destInfo.IsDir() {
-				return w.walk(destPath, path, walkFn)
-			}
+		if destInfo.IsDir() {
+			return walk(destPath, path, walkFn)
 		}
 
 		return walkFn(path, info, err)

internal/backend/storage/fs/walk_test.go

@@ -0,0 +1,97 @@
+package fs
+
+import (
+	"io/fs"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestWalkTooLong(t *testing.T) {
+	// Walking a path with a symlink loop should fail.
+
+	td := t.TempDir()
+	storeDir := filepath.Join(td, "store")
+	fn := filepath.Join(storeDir, "real", "file.txt")
+	assert.NoError(t, os.MkdirAll(filepath.Dir(fn), 0o700))
+	assert.NoError(t, ioutil.WriteFile(fn, []byte("foobar"), 0o600))
+
+	ptr := filepath.Join(storeDir, "path", "via", "link")
+
+	assert.NoError(t, os.MkdirAll(filepath.Dir(ptr), 0o700))
+
+	assert.NoError(t, os.Symlink(filepath.Join(storeDir, "path"), filepath.Join(storeDir, "path", "via", "loop")))
+
+	// test the walkFunc
+	assert.Error(t, walkSymlinks(storeDir, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() && strings.HasPrefix(info.Name(), ".") {
+			return fs.SkipDir
+		}
+		if info.IsDir() {
+			return nil
+		}
+		rPath := strings.TrimPrefix(path, storeDir)
+		if rPath == "" {
+			return nil
+		}
+
+		return nil
+	}))
+}
+
+func TestWalkSameFile(t *testing.T) {
+	// Two files visible via different link chains should both end up in the result set.
+
+	td := t.TempDir()
+	storeDir := filepath.Join(td, "store")
+	fn := filepath.Join(storeDir, "real", "file.txt")
+	assert.NoError(t, os.MkdirAll(filepath.Dir(fn), 0o700))
+	assert.NoError(t, ioutil.WriteFile(fn, []byte("foobar"), 0o600))
+
+	ptr1 := filepath.Join(storeDir, "path", "via", "one", "link")
+	ptr2 := filepath.Join(storeDir, "another", "path", "to", "this", "file")
+
+	assert.NoError(t, os.MkdirAll(filepath.Dir(ptr1), 0o700))
+	assert.NoError(t, os.MkdirAll(filepath.Dir(ptr2), 0o700))
+
+	assert.NoError(t, os.Symlink(fn, ptr1))
+	assert.NoError(t, os.Symlink(fn, ptr2))
+
+	// test the walkFunc
+	seen := map[string]bool{}
+	want := map[string]bool{
+		"another/path/to/this/file": true,
+		"path/via/one/link":         true,
+		"real/file.txt":             true,
+	}
+
+	assert.NoError(t, walkSymlinks(storeDir, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() && strings.HasPrefix(info.Name(), ".") {
+			return fs.SkipDir
+		}
+		if info.IsDir() {
+			return nil
+		}
+		rPath := strings.TrimPrefix(path, storeDir)
+		if rPath == "" {
+			return nil
+		}
+		rPath = strings.TrimPrefix(rPath, "/")
+		rPath = filepath.ToSlash(rPath) // support running this test on Windows
+		seen[rPath] = true
+
+		return nil
+	}))
+
+	assert.Equal(t, want, seen)
+}