Refuse a backup destination an unplugged volume left empty (closes #239)

Every backup writes vaultik-destination.txt at the destination root.
While the local index holds snapshots, a destination without it is
not present: snapshot create refuses before writing, prune and
snapshot purge fail, snapshot list and snapshot remove warn. remote
nuke deletes it last. The file:// backend no longer creates its
destination directory and checks for it on every write, which
protects a directory on a removable volume but not a mount point.

Tests back up into directories that exist; fixtures standing for a
written destination carry the marker, and two tests check they get
past it.

Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, snapshot name selection moved
into snapshotNamesToCreate and the --force/--json test into
confirmRemoveSnapshot, both unchanged.

Model: opus-5-5

config.example.yml

@@ -302,7 +302,8 @@ storage_url: "rclone://myremote/path/to/backups"
 
 # Average chunk size for content-defined chunking
 # Smaller chunks = better deduplication but more metadata
-# Supports: 10MB, 5M, 1GB, 500KB, 64MiB, etc.
+# Minimum 1MB
+# Supports: 10MB, 5M, 1GB, 64MiB, etc.
 # Default: 10MB
 #chunk_size: 10MB
 

internal/cli/config.go

@@ -224,7 +224,7 @@ storage_url: ""
 
 # Average chunk size for content-defined chunking (FastCDC).
 # Smaller = better deduplication but more metadata overhead.
-# Accepts: 1MB, 10M, 64KB, etc.
+# Minimum 1MB. Accepts: 1MB, 10M, 64MiB, etc.
 # Default: 10MB
 # chunk_size: 10MB
 

internal/cli/entry_snapshot_purge_test.go

@@ -0,0 +1,75 @@
+package cli //nolint:testpackage // shares runEntry and the hermetic config helpers
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+)
+
+// Words and flags of `snapshot purge` used to build argument vectors
+// below.
+const (
+	cmdPurge      = "purge"
+	flagOlderThan = "--older-than"
+	flagForce     = "--force"
+)
+
+// TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails makes
+// the local index refuse to delete an old snapshot and checks that
+// `snapshot purge` exits non-zero and leaves that snapshot's metadata on
+// the destination store. It used to log the error, delete the metadata
+// and exit 0.
+//
+// `snapshot create --prune` runs the same purge, but cannot be driven
+// this way: the refusal also breaks the backup's metadata export, which
+// deletes the other snapshots from its copy of the local index.
+//
+//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
+func TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(t *testing.T) {
+	configPath := writeHermeticPruneConfig(t, false)
+	dir := filepath.Dir(configPath)
+	indexPath := filepath.Join(dir, "index.sqlite")
+
+	// seedStaleSnapshotRecord writes an old snapshot to the local index
+	// only; the manifest planted here puts it on the destination store
+	// too, so the purge selects it rather than dropping it as stale.
+	seedStaleSnapshotRecord(t, indexPath)
+
+	// While the local index holds a snapshot, purge fails on a
+	// destination without the marker a backup writes.
+	require.NoError(t, os.WriteFile(
+		filepath.Join(dir, "store", "vaultik-destination.txt"), nil, 0o600))
+
+	manifestDir := filepath.Join(dir, "store", "metadata",
+		snapshot.RemoteSnapshotKey(stalePruneSnapshotID))
+	require.NoError(t, os.MkdirAll(manifestDir, 0o750))
+
+	manifestPath := filepath.Join(manifestDir, "manifest.json.zst")
+	require.NoError(t, os.WriteFile(manifestPath, []byte("stub"), 0o600))
+
+	ctx := context.Background()
+
+	db, err := database.New(ctx, indexPath)
+	require.NoError(t, err)
+
+	_, err = db.Conn().ExecContext(ctx, `
+		CREATE TRIGGER refuse_snapshot_delete BEFORE DELETE ON snapshots
+		BEGIN SELECT RAISE(ABORT, 'simulated index error'); END`)
+	require.NoError(t, err)
+	require.NoError(t, db.Close())
+
+	code, _, stderr := runEntry(t, flagConfig, configPath,
+		cmdSnapshot, cmdPurge, flagOlderThan, "30d", flagForce)
+
+	assert.Equal(t, 1, code)
+	assert.Contains(t, stderr, "simulated index error",
+		"the purge must fail at the local-index delete")
+	assert.FileExists(t, manifestPath,
+		"the snapshot is still in the local index, so its metadata must stay")
+}

internal/config/config.go

@@ -30,13 +30,15 @@ const appName = "vaultik"
 //nolint:gosec // G101: marker for detecting a pasted secret key, not a credential
 const secretKeyPrefix = "AGE-SECRET-KEY-"
 
-// Defaults and validation bounds for tunable settings.
+// Defaults and validation bounds for tunable settings. MB and GB are
+// 1000-based, as the size parser reads them; the S3 part sizes are S3's own
+// limits, which are 1024-based (MiB, GiB).
 const (
-	defaultBlobSizeLimit    = Size(10 * 1024 * 1024 * 1024) // 10GB
-	defaultChunkSize        = Size(10 * 1024 * 1024)        // 10MB
+	defaultBlobSizeLimit    = Size(10 * 1000 * 1000 * 1000) // 10GB
+	defaultChunkSize        = Size(10 * 1000 * 1000)        // 10MB
 	defaultS3PartSize       = Size(5 * 1024 * 1024)         // 5MiB
 	defaultCompressionLevel = 3
-	minChunkSize            = 1024 * 1024 // 1MB
+	minChunkSize            = 1000 * 1000 // 1MB
 	minCompressionLevel     = 1
 	maxCompressionLevel     = 19
 	// S3 accepts a multipart upload part from 5MiB to 5GiB.

internal/config/config_test.go

@@ -337,6 +337,81 @@ func TestLoadS3PartSize(t *testing.T) {
 	})
 }
 
+// TestDocumentedSizeDefaults checks that the defaults are the documented
+// chunk_size 10MB and blob_size_limit 10GB as the parser reads those values
+// (1000-based), so writing a default out in the config file changes nothing.
+func TestDocumentedSizeDefaults(t *testing.T) {
+	t.Parallel()
+
+	cfg, err := Parse([]byte("chunk_size: 10MB\nblob_size_limit: 10GB\n"))
+	if err != nil {
+		t.Fatalf("Parse() unexpected error: %v", err)
+	}
+
+	if cfg.ChunkSize != defaultChunkSize {
+		t.Errorf("chunk_size: 10MB is %d bytes, the default is %d",
+			cfg.ChunkSize, defaultChunkSize)
+	}
+
+	if cfg.BlobSizeLimit != defaultBlobSizeLimit {
+		t.Errorf("blob_size_limit: 10GB is %d bytes, the default is %d",
+			cfg.BlobSizeLimit, defaultBlobSizeLimit)
+	}
+}
+
+// TestDocumentedSizeExamples checks the 1MB minimum chunk_size and that
+// every size the README, config.example.yml and the config init template
+// list as an example is accepted.
+func TestDocumentedSizeExamples(t *testing.T) {
+	t.Parallel()
+
+	const withoutSizes = "snapshots:\n" +
+		"  test:\n" +
+		"    paths: [/tmp/vaultik-test-source]\n" +
+		"storage_url: file:///tmp/vaultik-test-storage\n"
+
+	tests := []struct {
+		setting string
+		wantErr error
+	}{
+		{"chunk_size: 1MB", nil},
+		{"chunk_size: 999999", errChunkSizeTooSmall},
+		{"chunk_size: 5M", nil},
+		{"chunk_size: 10M", nil},
+		{"chunk_size: 10MB", nil},
+		{"chunk_size: 64MiB", nil},
+		{"chunk_size: 1GB", nil},
+		// Four times the default chunk_size, the documented minimum.
+		{"blob_size_limit: 40MB", nil},
+		{"blob_size_limit: 500MB", nil},
+		{"blob_size_limit: 1GB", nil},
+		{"blob_size_limit: 1GiB", nil},
+		{"blob_size_limit: 10G", nil},
+		{"blob_size_limit: 10GB", nil},
+		{"s3: {part_size: 5MiB}", nil},
+		{"s3: {part_size: 10MB}", nil},
+		{"s3: {part_size: 16MiB}", nil},
+		{"s3: {part_size: 100MiB}", nil},
+		{"s3: {part_size: 5GiB}", nil},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.setting, func(t *testing.T) {
+			t.Parallel()
+
+			cfg, err := Parse([]byte(withoutSizes + tt.setting + "\n"))
+			if err != nil {
+				t.Fatalf("Parse() unexpected error: %v", err)
+			}
+
+			err = cfg.Validate()
+			if !errors.Is(err, tt.wantErr) {
+				t.Fatalf("Validate() error = %v, want %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
 // 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

internal/snapshot/exclude_test.go

@@ -578,3 +578,54 @@ func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
 	require.Equal(t, 1, result.FilesScanned,
 		"**/ file pattern should match at the top level and nested")
 }
+
+func TestExcludePatterns_AnchoredDoubleStarAtEveryDepth(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, /**/ also matches directly under the source dir
+	fs := setupAnchoredTestFS(t)
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"/**/projectname"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// /**/projectname should exclude BOTH:
+	// - /backup/projectname/file.txt
+	// - /backup/otherproject/projectname/file.txt
+	// Total files: 4, excluded: 2
+	require.Equal(t, 2, result.FilesScanned,
+		"/**/ pattern should match at the top level and nested")
+}
+
+func TestExcludePatterns_DoubleStarInMiddleMatchesZeroDirectories(t *testing.T) {
+	t.Parallel()
+
+	// As in gitignore, a/**/b matches a/b as well as a/c/b. The last part
+	// is a glob: with a plain name the glob library already matched a/b.
+	fs := &afero.MemMapFs{}
+	files := []string{"/backup/a/x.log", "/backup/a/c/y.log", "/backup/a/c/kept.txt"}
+
+	for _, path := range files {
+		require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0755))
+		require.NoError(t, afero.WriteFile(fs, path, []byte(path), 0644))
+	}
+
+	scanner, repos, cleanup := createTestScanner(t, fs, []string{"a/**/*.log"})
+	defer cleanup()
+
+	ctx := context.Background()
+	createSnapshotRecord(ctx, t, repos, "test-snapshot")
+
+	result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
+	require.NoError(t, err)
+
+	// a/**/*.log should exclude /backup/a/x.log and /backup/a/c/y.log
+	// Total files: 3, excluded: 2
+	require.Equal(t, 1, result.FilesScanned,
+		"a/**/*.log should match a/x.log as well as a/c/y.log")
+}

internal/snapshot/scanner.go

@@ -2035,33 +2035,60 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
 		pattern = strings.TrimSuffix(pattern, "/")
 
 		// A leading "**/" demands a "/" before the name and so misses an
-		// entry directly under the source path. An unanchored pattern
-		// already matches at every depth, so drop the prefix: as in
-		// gitignore, "**/x" then also matches a top-level "x".
-		if !anchored {
-			pattern = strings.TrimPrefix(pattern, "**/")
+		// entry directly under the source path. It means "at every depth",
+		// which is what an unanchored pattern already does, so drop the
+		// prefix and match unanchored: as in gitignore, "**/x" and
+		// "/**/x" then also match a top-level "x".
+		if rest, found := strings.CutPrefix(pattern, "**/"); found {
+			pattern = rest
+			anchored = false
 		}
 
-		// Compile the glob pattern
-		// For patterns without path separators, we need to match them as components
-		// e.g., ".git" should match ".git" anywhere in the path
-		g, err := glob.Compile(pattern, '/')
-		if err != nil {
-			log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
+		// Likewise "/**/" inside a pattern demands two slashes, so
+		// "a/**/*.log" misses "a/x.log". As in gitignore it stands for
+		// zero or more directories, so also compile the pattern with it
+		// made a single "/". The glob library's {a,b} is no help: with a
+		// "**" inside it in mid-pattern, it misses paths it should match.
+		for _, variant := range doubleStarVariants(pattern) {
+			// For patterns without path separators, we need to match them as components
+			// e.g., ".git" should match ".git" anywhere in the path
+			g, err := glob.Compile(variant, '/')
+			if err != nil {
+				log.Warn("Invalid exclude pattern, skipping", "pattern", p, "error", err)
 
-			continue
+				break
+			}
+
+			compiled = append(compiled, compiledPattern{
+				pattern:  g,
+				anchored: anchored,
+				original: p,
+			})
 		}
-
-		compiled = append(compiled, compiledPattern{
-			pattern:  g,
-			anchored: anchored,
-			original: p,
-		})
 	}
 
 	return compiled
 }
 
+// doubleStarVariants returns pattern once for every way of keeping each
+// "/**/" in it or making it a single "/".
+func doubleStarVariants(pattern string) []string {
+	const doubleStar = "/**/"
+
+	before, after, found := strings.Cut(pattern, doubleStar)
+	if !found {
+		return []string{pattern}
+	}
+
+	var variants []string
+
+	for _, rest := range doubleStarVariants(after) {
+		variants = append(variants, before+doubleStar+rest, before+"/"+rest)
+	}
+
+	return variants
+}
+
 // shouldExclude checks if a path should be excluded based on exclude patterns
 // filePath is the full path to the file
 // rootPath is the root of the backup source directory

internal/storage/rclone.go

@@ -7,10 +7,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 	"time"
 
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/config"
 	"github.com/rclone/rclone/fs/config/configfile"
 	"github.com/rclone/rclone/fs/operations"
 
@@ -33,6 +35,15 @@ type RcloneStorer struct {
 // The remote parameter is the rclone remote name (as configured via `rclone config`).
 // The path parameter is the path within the remote.
 func NewRcloneStorer(ctx context.Context, remote, path string) (*RcloneStorer, error) {
+	// rclone reads RCLONE_CONFIG only as the value of its --config flag,
+	// which vaultik does not have, so it is applied here as that flag would.
+	if configPath, ok := os.LookupEnv("RCLONE_CONFIG"); ok {
+		err := config.SetConfigPath(configPath)
+		if err != nil {
+			return nil, fmt.Errorf("using RCLONE_CONFIG %q: %w", configPath, err)
+		}
+	}
+
 	// Install the default config file handler
 	configfile.Install()
 

internal/storage/rclone_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/config"
 	"github.com/rclone/rclone/fs/config/configmap"
 	"sneak.berlin/go/vaultik/internal/storage"
 )
@@ -390,3 +391,41 @@ func TestNewRcloneStorerUnknownRemote(t *testing.T) {
 		t.Errorf("NewRcloneStorer error = %v, want ErrRemoteNotFound", err)
 	}
 }
+
+// TestNewRcloneStorerReadsRcloneConfigEnv checks that a remote defined only
+// in the file RCLONE_CONFIG names is found and written to, as rclone itself
+// would.
+func TestNewRcloneStorerReadsRcloneConfigEnv(t *testing.T) {
+	// rclone keeps the config file path in a process-global variable, which
+	// NewRcloneStorer leaves pointing at this test's file.
+	previous := config.GetConfigPath()
+
+	t.Cleanup(func() { _ = config.SetConfigPath(previous) })
+
+	confFile := filepath.Join(t.TempDir(), "rclone.conf")
+
+	err := os.WriteFile(confFile, []byte("[loc]\ntype = local\n"), 0o600)
+	if err != nil {
+		t.Fatalf("writing rclone config: %v", err)
+	}
+
+	t.Setenv("RCLONE_CONFIG", confFile)
+
+	dir := t.TempDir()
+	ctx := context.Background()
+
+	s, err := storage.NewRcloneStorer(ctx, "loc", dir)
+	if err != nil {
+		t.Fatalf("NewRcloneStorer: %v", err)
+	}
+
+	err = s.Put(ctx, testBlobKey, strings.NewReader("blob-bytes"))
+	if err != nil {
+		t.Fatalf("Put: %v", err)
+	}
+
+	_, err = os.Stat(filepath.Join(dir, testBlobKey))
+	if err != nil {
+		t.Errorf("object is not in the remote's directory: %v", err)
+	}
+}

internal/vaultik/fault_injection_test.go

@@ -441,6 +441,10 @@ func TestBackupSurvivesMetadataExportInterruption(t *testing.T) {
 
 	repos := database.NewRepositories(db)
 
+	// snapshot create writes the marker; this backup runs its steps one
+	// by one, so the marker is written here.
+	require.NoError(t, inner.Put(ctx, markerFile, strings.NewReader("")))
+
 	// Back up and complete with a working backend.
 	sm := newFaultSnapshotManager(fs, inner, cfg, repos)
 	scanner := newFaultScanner(fs, inner, cfg, repos)
@@ -489,8 +493,14 @@ func TestBackupSurvivesMetadataExportInterruption(t *testing.T) {
 		"shallow verify must report the missing manifest")
 
 	// snapshot list must not crash on the partial snapshot.
+	var listOut strings.Builder
+
+	reader.UI = ui.NewWithColor(&listOut, false)
 	require.NoError(t, reader.ListSnapshots(false),
 		"snapshot list must tolerate a partially-exported snapshot")
+	assert.NotContains(t, listOut.String(),
+		"Could not list backup destination store",
+		"snapshot list must read the partial snapshot from the destination")
 }
 
 // Scenario 2, repair: the process dies during the metadata export of a

internal/vaultik/snapshot.go

@@ -686,10 +686,11 @@ func (v *Vaultik) confirmAndExecutePurge(
 		snapshotID := snap.ID.String()
 		log.Info("Deleting snapshot", "id", snapshotID)
 
+		// Stop before the remote delete, so a snapshot still in the local
+		// index keeps its metadata on the destination store.
 		err := v.deleteSnapshotFromLocalDB(snapshotID)
 		if err != nil {
-			log.Error("Failed to delete from local database",
-				"snapshot_id", snapshotID, "error", err)
+			return fmt.Errorf("deleting snapshot %s from local index: %w", snapshotID, err)
 		}
 
 		_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))