List a file:// destination through a symlinked directory (closes #296)
check / check (push) Waiting to run
check / check (push) Waiting to run
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 listed prefix that is a symlink whose target is missing, such as a link to a volume that is not mounted, fails the listing, as a missing destination directory does. 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:
@@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-13: 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 whose target is
|
||||||
|
missing, such as one to a volume that is not mounted, or a link
|
||||||
|
pointing back up the tree makes them fail.
|
||||||
|
|
||||||
- 2026-10-13: Stopped `remote info` counting a file under `blobs/` that
|
- 2026-10-13: Stopped `remote info` counting a file under `blobs/` that
|
||||||
is not a blob as an orphaned blob
|
is not a blob as an orphaned blob
|
||||||
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray
|
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray
|
||||||
|
|||||||
+45
-10
@@ -120,9 +120,10 @@ 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 or the
|
||||||
// empty listing.
|
// prefix is a symlink whose target is missing; a missing prefix under the
|
||||||
|
// destination directory 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
|
||||||
|
|
||||||
@@ -133,8 +134,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
|
|||||||
|
|
||||||
basePath := f.fullPath(prefix)
|
basePath := f.fullPath(prefix)
|
||||||
|
|
||||||
// Check if the prefix exists
|
exists, err := f.prefixExists(basePath)
|
||||||
exists, err := afero.Exists(f.fs, basePath)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("checking path: %w", err)
|
return nil, fmt.Errorf("checking path: %w", err)
|
||||||
}
|
}
|
||||||
@@ -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,9 @@ 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 or the prefix is a symlink whose
|
||||||
|
// target 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)
|
||||||
|
|
||||||
@@ -192,8 +194,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
|
|||||||
|
|
||||||
basePath := f.fullPath(prefix)
|
basePath := f.fullPath(prefix)
|
||||||
|
|
||||||
// Check if the prefix exists
|
exists, err := f.prefixExists(basePath)
|
||||||
exists, err := afero.Exists(f.fs, basePath)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ch <- ObjectInfo{Err: fmt.Errorf("checking path: %w", err)}
|
ch <- ObjectInfo{Err: fmt.Errorf("checking path: %w", err)}
|
||||||
|
|
||||||
@@ -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,40 @@ 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)
|
||||||
|
}
|
||||||
|
|
||||||
|
// prefixExists is afero.Exists, except that it does not follow a symlink
|
||||||
|
// at path. A symlinked prefix whose target is missing, such as a link to
|
||||||
|
// a volume that is not mounted, therefore exists, and walk then fails on
|
||||||
|
// it; with afero.Exists it would list as empty, and prune would delete
|
||||||
|
// every blob.
|
||||||
|
func (f *FileStorer) prefixExists(path string) (bool, error) {
|
||||||
|
lstater, ok := f.fs.(afero.Lstater)
|
||||||
|
if !ok {
|
||||||
|
return afero.Exists(f.fs, path)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, _, err := lstater.LstatIfPossible(path)
|
||||||
|
if err == nil {
|
||||||
|
return true, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if os.IsNotExist(err) {
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return false, err
|
||||||
|
}
|
||||||
|
|
||||||
// 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)
|
||||||
|
|||||||
@@ -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,151 @@ 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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestFileStorerListFailsOnSymlinkWithMissingTarget checks that List and
|
||||||
|
// ListStream fail on a symlinked metadata/ or blobs/ directory whose target
|
||||||
|
// is missing, such as a link to a volume that is not mounted, as they do on
|
||||||
|
// a missing destination directory, instead of listing it as empty.
|
||||||
|
func TestFileStorerListFailsOnSymlinkWithMissingTarget(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
for _, dir := range []string{"metadata", "blobs"} {
|
||||||
|
t.Run(dir, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
base := t.TempDir()
|
||||||
|
|
||||||
|
s, err := storage.NewFileStorer(base)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("NewFileStorer: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
unmounted := filepath.Join(t.TempDir(), "unmounted")
|
||||||
|
|
||||||
|
err = os.Symlink(unmounted, filepath.Join(base, dir))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Symlink: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, prefix := range []string{"", dir + "/"} {
|
||||||
|
keys, err := s.List(ctx, prefix)
|
||||||
|
if !errors.Is(err, fs.ErrNotExist) {
|
||||||
|
t.Errorf("List(%q) = %v, %v; want a not-exist error",
|
||||||
|
prefix, keys, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
var streamErr error
|
||||||
|
|
||||||
|
for object := range s.ListStream(ctx, prefix) {
|
||||||
|
if object.Err != nil {
|
||||||
|
streamErr = object.Err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !errors.Is(streamErr, fs.ErrNotExist) {
|
||||||
|
t.Errorf("ListStream(%q) error = %v, want a not-exist error",
|
||||||
|
prefix, streamErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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,121 @@
|
|||||||
|
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)
|
||||||
|
blobFiles := blobFilesIn(t, storeDir)
|
||||||
|
moveBehindSymlink(t, storeDir, dir)
|
||||||
|
|
||||||
|
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")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestPruneFailsOnSymlinkedDirectoryWithMissingTarget checks that prune
|
||||||
|
// fails, deleting no blob and keeping the local snapshot record, when the
|
||||||
|
// metadata/ or blobs/ directory of a file:// destination is a symlink
|
||||||
|
// whose target is missing, as a link to a volume that is not mounted is.
|
||||||
|
func TestPruneFailsOnSymlinkedDirectoryWithMissingTarget(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)
|
||||||
|
blobFiles := blobFilesIn(t, storeDir)
|
||||||
|
targetPath := moveBehindSymlink(t, storeDir, dir)
|
||||||
|
|
||||||
|
// Unmount the volume the link points to, and mount it again
|
||||||
|
// once prune has run.
|
||||||
|
unmountedPath := targetPath + "-unmounted"
|
||||||
|
require.NoError(t, os.Rename(targetPath, unmountedPath))
|
||||||
|
|
||||||
|
err := v.Prune(&vaultik.PruneOptions{Force: true})
|
||||||
|
require.ErrorIs(t, err, fs.ErrNotExist)
|
||||||
|
|
||||||
|
require.NoError(t, os.Rename(unmountedPath, targetPath))
|
||||||
|
|
||||||
|
// 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")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// blobFilesIn returns the paths of the blob files under storeDir/blobs, and
|
||||||
|
// fails the test when there are none.
|
||||||
|
func blobFilesIn(t *testing.T, storeDir string) []string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
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)
|
||||||
|
|
||||||
|
return blobFiles
|
||||||
|
}
|
||||||
|
|
||||||
|
// moveBehindSymlink moves storeDir/dir to a directory elsewhere, puts a
|
||||||
|
// symlink to it in its place, and returns where it went.
|
||||||
|
func moveBehindSymlink(t *testing.T, storeDir, dir string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
linkPath := filepath.Join(storeDir, dir)
|
||||||
|
targetPath := filepath.Join(t.TempDir(), dir)
|
||||||
|
require.NoError(t, os.Rename(linkPath, targetPath))
|
||||||
|
require.NoError(t, os.Symlink(targetPath, linkPath))
|
||||||
|
|
||||||
|
return targetPath
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user