Compare commits

..
Author SHA1 Message Date
clawbot b894f6e5e9 List a file:// destination through a symlinked directory (closes #296)
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
2026-10-13 16:44:25 +00:00
7 changed files with 341 additions and 166 deletions
+1 -3
View File
@@ -375,9 +375,7 @@ on the destination in one go, use `vaultik remote nuke --force`.
* Optional path arguments to restore specific files/directories (default: all)
* Preserves file permissions, timestamps, ownership (ownership requires root),
symlinks, and empty directories
* `--verify`: After restoring, verify every restored file's chunk hashes
match. A file skipped under `--skip-errors` is reported as failed to
restore and is not verified.
* `--verify`: After restoring, verify every file's chunk hashes match
**`prune`**: Tidy up everything that isn't needed. Runs three passes:
(1) reconcile the local index against the destination store — any
+11
View File
@@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to
# 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
is not a blob as an orphaned blob
([issue #295](https://git.eeqj.de/sneak/vaultik/issues/295)). A stray
+45 -10
View File
@@ -120,9 +120,10 @@ func (f *FileStorer) Delete(_ context.Context, key string) error {
return nil
}
// List returns all keys with the given prefix. It fails when the
// destination directory is missing; a missing prefix under it is an
// empty listing.
// List returns all keys with the given prefix, reading through symlinked
// directories. It fails when the destination directory is missing or the
// 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) {
var keys []string
@@ -133,8 +134,7 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
basePath := f.fullPath(prefix)
// Check if the prefix exists
exists, err := afero.Exists(f.fs, basePath)
exists, err := f.prefixExists(basePath)
if err != nil {
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
}
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 {
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
// 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 {
ch := make(chan ObjectInfo)
@@ -192,8 +194,7 @@ func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan Objec
basePath := f.fullPath(prefix)
// Check if the prefix exists
exists, err := afero.Exists(f.fs, basePath)
exists, err := f.prefixExists(basePath)
if err != nil {
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
}
_ = 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
select {
case <-ctx.Done():
@@ -352,6 +353,40 @@ func (f *FileStorer) writeAtomic(
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.
func (f *FileStorer) fullPath(key string) string {
return filepath.Join(f.basePath, key)
+151
View File
@@ -4,7 +4,10 @@ import (
"context"
"errors"
"io/fs"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"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)
}
}
// 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")
}
}
+10 -25
View File
@@ -103,12 +103,11 @@ type RestoreResult struct {
BlobsDownloaded int
BytesDownloaded int64
Duration time.Duration
FilesFailed int
FailedFiles []string // Paths of files that failed to restore
// Verification results (only populated if Verify option is set)
FilesVerified int
BytesVerified int64
VerifyFailedFiles []string // Paths of restored files that failed verification
FilesFailed int
FailedFiles []string // Paths of files that failed verification
}
// Restore restores files from a snapshot to the target directory
@@ -659,34 +658,20 @@ func (v *Vaultik) handleRestoreVerification(
opts *RestoreOptions,
result *RestoreResult,
) error {
// A file that failed to restore was not written and is already
// reported as such, so it is not verified.
failedToRestore := make(map[string]bool, len(result.FailedFiles))
for _, path := range result.FailedFiles {
failedToRestore[path] = true
}
restored := make([]*database.File, 0, len(files))
for _, file := range files {
if !failedToRestore[file.Path.String()] {
restored = append(restored, file)
}
}
err := v.verifyRestoredFiles(v.ctx, repos, restored, opts.TargetDir, result)
err := v.verifyRestoredFiles(v.ctx, repos, files, opts.TargetDir, result)
if err != nil {
return fmt.Errorf("verification failed: %w", err)
}
if len(result.VerifyFailedFiles) > 0 {
if result.FilesFailed > 0 {
v.UI.Errorf("Verification failed: %s files did not match expected checksums.",
v.UI.Count(len(result.VerifyFailedFiles)))
v.UI.Count(result.FilesFailed))
for _, path := range result.VerifyFailedFiles {
for _, path := range result.FailedFiles {
v.UI.Detailf("%s", v.UI.Path(path))
}
return fmt.Errorf("%d %w", len(result.VerifyFailedFiles), errFilesFailedVerify)
return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedVerify)
}
v.UI.Completef("Verified %s files (%s).",
@@ -1446,8 +1431,8 @@ func (v *Vaultik) verifyRestoredFiles(
if err != nil {
log.Error("File verification failed", "path", file.Path, "error", err)
result.VerifyFailedFiles = append(
result.VerifyFailedFiles, file.Path.String())
result.FilesFailed++
result.FailedFiles = append(result.FailedFiles, file.Path.String())
}
bytesProcessed += file.Size
@@ -1463,7 +1448,7 @@ func (v *Vaultik) verifyRestoredFiles(
log.Info("Verification complete",
"files_verified", result.FilesVerified,
"bytes_verified", ubytes(result.BytesVerified),
"files_failed", len(result.VerifyFailedFiles),
"files_failed", result.FilesFailed,
)
return nil
@@ -4,9 +4,7 @@ import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"github.com/spf13/afero"
@@ -106,130 +104,6 @@ func assertRestoreSkipsLostFiles(
}
}
// TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles restores with
// SkipErrors and Verify after one blob of a two-blob snapshot was
// deleted. Verification must check only the files that were written, so
// a file of the deleted blob is reported as failed to restore and not
// also as failing verification, and Restore must still return an error.
func TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
backup := backupThenDeleteOneBlob(ctx, t)
var out bytes.Buffer
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, backup.fs)
v.UI = ui.NewWithColor(&out, false)
err := v.Restore(&vaultik.RestoreOptions{
SnapshotID: backup.snapshotID,
TargetDir: backup.restoreDir,
Verify: true,
SkipErrors: true,
})
require.Error(t, err, "restore must fail when files were skipped")
assert.Contains(t, err.Error(),
fmt.Sprintf("%d file(s) failed to restore", len(backup.lost)))
written := len(backup.files) - len(backup.lost)
assert.Contains(t, out.String(), fmt.Sprintf("Verifying %d files", written))
assert.Contains(t, out.String(), fmt.Sprintf("Verified %d files", written))
assert.NotContains(t, out.String(), "Verification failed")
}
// TestRestoreSkipErrorsVerifyReportsFailuresApart restores with
// SkipErrors and Verify after one blob of a two-blob snapshot was
// deleted, while one file that does restore is written one byte too long.
// The restore warning must list only the files of the deleted blob, the
// verification summary only the long file, each path once, and Restore
// must return an error.
func TestRestoreSkipErrorsVerifyReportsFailuresApart(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
ctx := context.Background()
backup := backupThenDeleteOneBlob(ctx, t)
var long string
for path := range backup.files {
if !backup.lost[path] {
long = path
break
}
}
var out bytes.Buffer
fs := trailingByteFs{Fs: backup.fs, suffix: long}
v := newReaderVaultik(ctx, backup.cfg, backup.storer, nil, fs)
v.UI = ui.NewWithColor(&out, false)
err := v.Restore(&vaultik.RestoreOptions{
SnapshotID: backup.snapshotID,
TargetDir: backup.restoreDir,
Verify: true,
SkipErrors: true,
})
require.Error(t, err, "restore must fail when files failed")
_, afterWarning, found := strings.Cut(out.String(),
fmt.Sprintf("%d file(s) failed to restore:", len(backup.lost)))
require.Truef(t, found, "no restore warning in:\n%s", out.String())
warning, summary, found := strings.Cut(afterWarning,
"Verification failed: 1 files did not match")
require.Truef(t, found, "no verification summary in:\n%s", out.String())
for path := range backup.lost {
assert.Equalf(t, 1, strings.Count(warning, path),
"%s must be listed once as failed to restore", path)
assert.NotContainsf(t, summary, path,
"%s was not restored and must not fail verification", path)
}
assert.NotContainsf(t, warning, long,
"%s was restored and must not be listed as failed to restore", long)
assert.Equalf(t, 1, strings.Count(summary, long),
"%s must be listed once as failing verification", long)
}
// trailingByteFs appends one byte, on close, to the file it opens whose
// path ends in suffix, so that file is restored one byte too long and
// fails verification.
type trailingByteFs struct {
afero.Fs
suffix string
}
//nolint:ireturn // afero.Fs.OpenFile is defined to return the interface
func (fs trailingByteFs) OpenFile(
name string, flag int, perm os.FileMode,
) (afero.File, error) {
f, err := fs.Fs.OpenFile(name, flag, perm)
if err != nil || !strings.HasSuffix(name, fs.suffix) {
return f, err
}
return trailingByteFile{File: f}, nil
}
type trailingByteFile struct {
afero.File
}
func (f trailingByteFile) Close() error {
_, _ = f.Write([]byte{'x'})
return f.File.Close()
}
// TestRestoreMissingBlobAbortsWithoutSkipErrors checks that a deleted blob
// still ends the restore with an error when SkipErrors is not set.
func TestRestoreMissingBlobAbortsWithoutSkipErrors(t *testing.T) {
@@ -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
}