diff --git a/TODO.md b/TODO.md index 018c80c..c1fcaff 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to # 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 ([issue #289](https://git.eeqj.de/sneak/vaultik/issues/289)). The number given to `--older-than` or `--keep-newer-than` was multiplied diff --git a/internal/storage/file.go b/internal/storage/file.go index 7bb74d1..b332351 100644 --- a/internal/storage/file.go +++ b/internal/storage/file.go @@ -120,9 +120,9 @@ 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; a +// missing prefix under it is an empty listing. func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) { 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 } - 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,8 @@ 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. func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-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 } - _ = 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,17 @@ 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) +} + // fullPath returns the full filesystem path for a key. func (f *FileStorer) fullPath(key string) string { return filepath.Join(f.basePath, key) diff --git a/internal/storage/file_backend_test.go b/internal/storage/file_backend_test.go index f92331e..09b5c15 100644 --- a/internal/storage/file_backend_test.go +++ b/internal/storage/file_backend_test.go @@ -4,7 +4,10 @@ import ( "context" "errors" "io/fs" + "os" "path/filepath" + "reflect" + "strings" "testing" "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) } } + +// 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") + } +} diff --git a/internal/vaultik/symlinked_destination_test.go b/internal/vaultik/symlinked_destination_test.go new file mode 100644 index 0000000..1b6bfa2 --- /dev/null +++ b/internal/vaultik/symlinked_destination_test.go @@ -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") + }) + } +}