Author SHA1 Message Date
clawbot f172b42d85 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 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 00:18:47 +00:00
11 changed files with 218 additions and 172 deletions
+2 -6
View File
@@ -412,8 +412,7 @@ destination store.
entirely. Remote storage is unaffected; the next backup will do a full
scan and re-deduplicate against existing remote blobs, and the local
index will re-bind to the currently configured storage destination.
Use this after changing `storage_url`, or `s3.endpoint`, `s3.bucket` or
`s3.prefix`, to a different destination.
Use this after changing `storage_url` to a different destination.
* `--force`: Skip confirmation prompt
---
@@ -427,10 +426,7 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on
a local or mounted filesystem. Useful for testing or backing up to a NAS. The
path must be absolute; a relative one is refused when the config loads.
Percent-escapes in the path are decoded, so `my%20stick` and `my stick` name
the same directory.
a local or mounted filesystem. Useful for testing or backing up to a NAS.
**Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`).
+9 -19
View File
@@ -22,25 +22,15 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps
- 2026-10-13: Refused a `file://` `storage_url` whose path is relative
([issue #299](https://git.eeqj.de/sneak/vaultik/issues/299)). The text
after `file://` was used as written, so `file://dest` resolved against
the working directory: a backup run from another directory wrote to a
different destination, still exited 0, and left a snapshot that could
not be restored. Such a URL now fails when the config loads, with an
error asking for an absolute path. Percent-escapes in the path are now
decoded, so `my%20stick` names the directory `my stick` instead of
creating one named `my%20stick`; a path written with a literal space
works as before.
- 2026-10-12: Bound the local index to the destination an `s3.*` config
names ([issue #298](https://git.eeqj.de/sneak/vaultik/issues/298)).
Only `storage_url` was checked, so after `s3.bucket` or `s3.prefix`
changed, the next backup treated every chunk as already stored,
uploaded nothing, and completed a snapshot that could not be restored.
Without `storage_url`, the local index is now bound to `s3.endpoint`,
`s3.bucket` and `s3.prefix`, and a change is refused as a changed
`storage_url` is.
- 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
+2 -12
View File
@@ -61,9 +61,6 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://")
errFileURLNotAbsolute = errors.New(
"a file:// storage_url needs an absolute path, " +
"as in file:///path/to/backup")
errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials")
@@ -464,18 +461,11 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.*
// credential fields. A file URL must name an absolute path: a relative one
// would resolve against the working directory, so the destination would
// move with it.
// credential fields.
func (c *Config) validateStorageURL() error {
switch {
case strings.HasPrefix(c.StorageURL, "file://"):
// storage.ParseStorageURL applies the same rule, but config
// cannot import storage, which imports config.
if !strings.HasPrefix(c.StorageURL, "file:///") {
return errFileURLNotAbsolute
}
// File storage doesn't need S3 credentials
return nil
case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config
-29
View File
@@ -337,35 +337,6 @@ func TestLoadS3PartSize(t *testing.T) {
})
}
// TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
// relative path fails at load. Accepted, it would resolve against the
// working directory, so a backup run from another directory would write
// to a different destination and still succeed.
func TestLoadRejectsRelativeFileURL(t *testing.T) {
t.Parallel()
const relativeConfig = "snapshots:\n" +
" test:\n" +
" paths: [/tmp/vaultik-test-source]\n" +
"storage_url: file://dest\n"
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(relativeConfig), 0o600)
if err != nil {
t.Fatalf("write config: %v", err)
}
_, err = Load(path)
if !errors.Is(err, errFileURLNotAbsolute) {
t.Fatalf("Load() error = %v, want errFileURLNotAbsolute", err)
}
if !strings.Contains(err.Error(), "absolute path") {
t.Errorf("error %q does not say to use an absolute path", err.Error())
}
}
// TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry —
// least of all a pasted secret key — is echoed in the error. An empty list
+18 -6
View File
@@ -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)
+100
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,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")
}
}
+12 -26
View File
@@ -17,10 +17,8 @@ const (
// Sentinel errors for storage URL parsing.
var (
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrRelativeFilePath = errors.New(
"file URL needs an absolute path, as in file:///path/to/backup")
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New(
@@ -53,8 +51,17 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL
}
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
return parseFileURL(after)
path := after
if path == "" {
return nil, ErrEmptyFilePath
}
return &URL{
Scheme: schemeFile,
Prefix: path,
}, nil
}
if strings.HasPrefix(rawURL, "s3://") {
@@ -68,27 +75,6 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme
}
// parseFileURL parses the path that follows file://. The path must be
// absolute: a relative one would resolve against the working directory,
// so the destination would move with it. Percent-escapes are decoded, so
// my%20stick names the directory "my stick"; a literal space is kept.
func parseFileURL(rawPath string) (*URL, error) {
if rawPath == "" {
return nil, ErrEmptyFilePath
}
if !strings.HasPrefix(rawPath, "/") {
return nil, ErrRelativeFilePath
}
path, err := url.PathUnescape(rawPath)
if err != nil {
return nil, fmt.Errorf("invalid file URL path: %w", err)
}
return &URL{Scheme: schemeFile, Prefix: path}, nil
}
// parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and
// ssl, so a credential-bearing URL is never stored or echoed.
+2 -16
View File
@@ -14,10 +14,7 @@ import (
func TestParseStorageURLValid(t *testing.T) {
t.Parallel()
const (
bucket = "mybucket"
fileScheme = "file"
)
const bucket = "mybucket"
cases := []struct {
name string
@@ -27,17 +24,7 @@ func TestParseStorageURLValid(t *testing.T) {
{
name: "file absolute path",
raw: "file:///var/backups/vaultik",
want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"},
},
{
name: "file percent-escaped path is decoded",
raw: "file:///Volumes/my%20stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
},
{
name: "file path with a literal space",
raw: "file:///Volumes/my stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
},
{
name: "s3 bucket and prefix, ssl defaults on",
@@ -103,7 +90,6 @@ func TestParseStorageURLErrors(t *testing.T) {
}{
{"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
+10 -10
View File
@@ -9,7 +9,7 @@ import (
)
// errStorageBindingMismatch is returned when the local index database is
// bound to a different destination than the configured one.
// bound to a different destination than the configured storage_url.
var errStorageBindingMismatch = errors.New(
"local index is bound to a different backup destination")
@@ -24,9 +24,8 @@ var errStorageBindingMismatch = errors.New(
//
// Behaviour:
// - On first use (empty stored value), record the configured
// destination (storage_url, or the s3.* endpoint, bucket and
// prefix) and log the binding.
// - When the stored value matches the configured destination, do
// storage_url and log the binding.
// - When the stored value matches the configured storage_url, do
// nothing and return nil.
// - When the two differ, refuse with an error that tells the user
// how to recover (revert the config, or run `vaultik database
@@ -47,10 +46,12 @@ func (v *Vaultik) EnsureStorageBinding() error {
configured := v.Config.StorageURL
if configured == "" {
// The s3.* form: bind on its endpoint, bucket and prefix,
// written the way storage_url would name the same destination.
s3 := v.Config.S3
configured = "s3://" + s3.Bucket + "/" + s3.Prefix + "?endpoint=" + s3.Endpoint
// Some legacy configs still use the split s3.* keys instead of
// storage_url. Falling back to a synthetic URL for those would
// hide the fact that the binding is loose. Instead, treat
// unset as "nothing to bind against" — the check is
// necessarily best-effort for those configs.
return nil
}
stored, err := v.Repositories.LocalMeta.Get(v.ctx, database.LocalMetaKeyStorageURL)
@@ -95,8 +96,7 @@ func buildBindingMismatchMessage(stored, configured string) string {
"future snapshots referencing blobs that don't exist at the new destination.\n" +
"\n" +
"To proceed, either:\n" +
" - revert storage_url (or the s3.* endpoint, bucket and prefix) in your\n" +
" config to the bound destination, or\n" +
" - revert storage_url in your config to the bound destination, or\n" +
" - run 'vaultik database delete' to discard the local index and rebuild it\n" +
" from a fresh full backup against the new destination"
}
-48
View File
@@ -76,51 +76,3 @@ func TestEnsureStorageBinding_MismatchRefuses(t *testing.T) {
assert.Contains(t, err.Error(), "file:///mnt/backups/new")
assert.Contains(t, err.Error(), "vaultik database delete")
}
// A config with no storage_url names its destination with s3.endpoint,
// s3.bucket and s3.prefix; a change to any of them must be refused just
// as a changed storage_url is.
func TestEnsureStorageBinding_S3FieldsChangeRefuses(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
bound := config.S3Config{
Endpoint: "https://s3.example.com",
Bucket: "bucket-a",
Prefix: "backups",
}
tests := []struct {
name string
changed config.S3Config
}{
{"bucket", config.S3Config{
Endpoint: bound.Endpoint, Bucket: "bucket-b", Prefix: bound.Prefix,
}},
{"prefix", config.S3Config{
Endpoint: bound.Endpoint, Bucket: bound.Bucket, Prefix: "other",
}},
{"endpoint", config.S3Config{
Endpoint: "https://s3.other.com", Bucket: bound.Bucket, Prefix: bound.Prefix,
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tv, _ := buildBindTestVaultik(t, "")
tv.Config.S3 = bound
require.NoError(t, tv.EnsureStorageBinding(), "first use binds")
require.NoError(t, tv.EnsureStorageBinding(), "same destination passes")
tv.Config.S3 = tt.changed
err := tv.EnsureStorageBinding()
require.Error(t, err)
assert.Contains(t, err.Error(),
"local index is bound to a different backup destination")
assert.Contains(t, err.Error(), "vaultik database delete")
})
}
}
@@ -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")
})
}
}