List a file:// destination through a symlinked directory (closes #296)
check / check (push) Failing after 3m40s

List and ListStream walked the destination with afero.Walk, which uses
Lstat and so reported a symlinked metadata/ or blobs/ directory as one
file, while Put, Get and Stat follow the link. The store then listed as
empty, and prune removed the local snapshot records and deleted every
blob. Both listings now walk a view of the filesystem without its Lstat
method, so afero falls back to Stat and enters the link.

A symlink loop is not detected: it is followed until the system refuses
the path for too many levels of links, and the listing fails.

Model: opus-5-5
This commit is contained in:
2026-10-13 00:18:47 +00:00
parent c9a519c233
commit f172b42d85
4 changed files with 191 additions and 6 deletions
+10
View File
@@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps # Completed Steps
- 2026-10-12: Made a `file://` destination list through a symlinked
directory ([issue #296](https://git.eeqj.de/sneak/vaultik/issues/296)).
A `metadata/` or `blobs/` directory that was a symlink to a directory
elsewhere was listed as one file, so the destination store listed as
empty while backups and restores still worked through the link.
`prune` then removed the local snapshot records and deleted every blob,
and `remote info` and `remote nuke` found no blobs. The listings now
read through the link as `Get` and `Stat` do; a link pointing back up
the tree makes them fail.
- 2026-10-12: Rejected a retention duration longer than about 292 years - 2026-10-12: Rejected a retention duration longer than about 292 years
([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The ([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The
number given to `--older-than` or `--keep-newer-than` was multiplied number given to `--older-than` or `--keep-newer-than` was multiplied
+18 -6
View File
@@ -120,9 +120,9 @@ func (f *FileStorer) Delete(_ context.Context, key string) error {
return nil return nil
} }
// List returns all keys with the given prefix. It fails when the // List returns all keys with the given prefix, reading through symlinked
// destination directory is missing; a missing prefix under it is an // directories. It fails when the destination directory is missing; a
// empty listing. // missing prefix under it is an empty listing.
func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) { func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) {
var keys []string var keys []string
@@ -143,7 +143,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
return keys, nil // Empty list for non-existent prefix return keys, nil // Empty list for non-existent prefix
} }
err = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error { err = f.walk(basePath, func(path string, info os.FileInfo, err error) error {
if err != nil { if err != nil {
return err return err
} }
@@ -176,7 +176,8 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
} }
// ListStream returns a channel of ObjectInfo for large result sets. Like // ListStream returns a channel of ObjectInfo for large result sets. Like
// List, it sends an error when the destination directory is missing. // List, it reads through symlinked directories and sends an error when
// the destination directory is missing.
func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo { func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo {
ch := make(chan ObjectInfo) ch := make(chan ObjectInfo)
@@ -204,7 +205,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
return // Empty channel for non-existent prefix return // Empty channel for non-existent prefix
} }
_ = afero.Walk(f.fs, basePath, func(path string, info os.FileInfo, err error) error { _ = f.walk(basePath, func(path string, info os.FileInfo, err error) error {
// Check context cancellation // Check context cancellation
select { select {
case <-ctx.Done(): case <-ctx.Done():
@@ -352,6 +353,17 @@ func (f *FileStorer) writeAtomic(
return nil return nil
} }
// walk is afero.Walk, except that it reads through a symlinked directory
// the way Get and Stat do, where afero.Walk reports the link as one file.
// afero.Walk uses Lstat, which does not follow a link, whenever the
// filesystem has one; the struct below carries only the afero.Fs methods,
// so the walk uses Stat instead. A link back up the tree is followed until
// the operating system refuses the path for too many levels of links, and
// the walk then fails.
func (f *FileStorer) walk(root string, fn filepath.WalkFunc) error {
return afero.Walk(struct{ afero.Fs }{f.fs}, root, fn)
}
// fullPath returns the full filesystem path for a key. // fullPath returns the full filesystem path for a key.
func (f *FileStorer) fullPath(key string) string { func (f *FileStorer) fullPath(key string) string {
return filepath.Join(f.basePath, key) return filepath.Join(f.basePath, key)
+100
View File
@@ -4,7 +4,10 @@ import (
"context" "context"
"errors" "errors"
"io/fs" "io/fs"
"os"
"path/filepath" "path/filepath"
"reflect"
"strings"
"testing" "testing"
"sneak.berlin/go/vaultik/internal/storage" "sneak.berlin/go/vaultik/internal/storage"
@@ -60,3 +63,100 @@ func TestFileStorerListMissingDestination(t *testing.T) {
t.Errorf("ListStream error = %v, want a not-exist error", streamErr) t.Errorf("ListStream error = %v, want a not-exist error", streamErr)
} }
} }
// TestFileStorerListsThroughSymlinkedDirectory checks that List and
// ListStream read through a symlinked metadata/ or blobs/ directory, as Get
// and Stat do, instead of listing the link as one key. Listing the whole
// destination meets the link part-way through the walk; listing its prefix
// starts the walk at the link.
func TestFileStorerListsThroughSymlinkedDirectory(t *testing.T) {
t.Parallel()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
base := t.TempDir()
key := dir + "/aa/bb/object"
s, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
err = s.Put(context.Background(), key, strings.NewReader("data"))
if err != nil {
t.Fatalf("Put: %v", err)
}
linkPath := filepath.Join(base, dir)
movedPath := filepath.Join(t.TempDir(), dir)
err = os.Rename(linkPath, movedPath)
if err != nil {
t.Fatalf("moving %s away: %v", dir, err)
}
err = os.Symlink(movedPath, linkPath)
if err != nil {
t.Fatalf("linking %s: %v", dir, err)
}
want := []string{key}
for _, prefix := range []string{"", dir + "/"} {
got := listSorted(t, s, prefix)
if !reflect.DeepEqual(got, want) {
t.Errorf("List(%q) = %v, want %v", prefix, got, want)
}
got = listStreamKeys(t, s, prefix)
if !reflect.DeepEqual(got, want) {
t.Errorf("ListStream(%q) = %v, want %v", prefix, got, want)
}
}
})
}
}
// TestFileStorerListFailsOnSymlinkLoop checks that a symlink pointing back
// at the destination makes List and ListStream fail instead of walking it
// forever.
func TestFileStorerListFailsOnSymlinkLoop(t *testing.T) {
t.Parallel()
ctx := context.Background()
base := t.TempDir()
s, err := storage.NewFileStorer(base)
if err != nil {
t.Fatalf("NewFileStorer: %v", err)
}
err = s.Put(ctx, testBlobKey, strings.NewReader("data"))
if err != nil {
t.Fatalf("Put: %v", err)
}
err = os.Symlink(base, filepath.Join(base, "blobs", "loop"))
if err != nil {
t.Fatalf("Symlink: %v", err)
}
keys, err := s.List(ctx, "blobs/")
if err == nil {
t.Errorf("List = %d keys and no error, want an error", len(keys))
}
var streamErr error
for object := range s.ListStream(ctx, "blobs/") {
if object.Err != nil {
streamErr = object.Err
}
}
if streamErr == nil {
t.Error("ListStream sent no error, want one")
}
}
@@ -0,0 +1,63 @@
package vaultik_test
import (
"context"
"io/fs"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestPruneDeletesNothingThroughSymlinkedDirectory checks that prune keeps
// every blob and the local snapshot record when the metadata/ or blobs/
// directory of a file:// destination is a symlink to a directory elsewhere
// (https://git.eeqj.de/sneak/vaultik/issues/296). A backup writes through
// such a link, so prune has to list through it too.
func TestPruneDeletesNothingThroughSymlinkedDirectory(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
for _, dir := range []string{"metadata", "blobs"} {
t.Run(dir, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
storeDir := filepath.Join(t.TempDir(), "store")
v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
var blobFiles []string
err := filepath.WalkDir(filepath.Join(storeDir, "blobs"),
func(path string, entry fs.DirEntry, err error) error {
if err == nil && !entry.IsDir() {
blobFiles = append(blobFiles, path)
}
return err
})
require.NoError(t, err)
require.NotEmpty(t, blobFiles)
linkPath := filepath.Join(storeDir, dir)
movedPath := filepath.Join(t.TempDir(), dir)
require.NoError(t, os.Rename(linkPath, movedPath))
require.NoError(t, os.Symlink(movedPath, linkPath))
require.NoError(t, v.Prune(&vaultik.PruneOptions{Force: true}))
// When blobs/ is the link, these paths run through it.
for _, blobFile := range blobFiles {
assert.FileExists(t, blobFile, "prune must delete no blob")
}
snapshots, err := repos.Snapshots.ListRecent(ctx, listRecentTestLimit)
require.NoError(t, err)
assert.Len(t, snapshots, 1, "prune must delete no local snapshot record")
})
}
}