Silence prune and snapshot removal status under --cron and --quiet (closes #294)

PruneBlobs, CleanupLocalSnapshots, RemoveSnapshot, RemoveAllSnapshots
and snapshot purge wrote their status lines straight to stdout, so
`snapshot create --cron --prune` printed on every successful run and
--quiet did not silence `prune`, `snapshot purge`, `snapshot remove` or
`remote nuke`. Those lines now go through internal/ui, which drops them
under --cron and --quiet; warnings, errors and --json documents still
print.

The confirmation prompts still write straight to stdout, as the
`database delete` prompt does, together with what they ask to delete:
the snapshot list before the purge prompt, and the blob count and size
before the prune prompt. With --force those lines are status lines.

The two purge test fixtures gained the UI writer production always sets.

Model: opus-5-5

README.md

@@ -382,7 +382,9 @@ on the destination in one go, use `vaultik remote nuke --force`.
   store — its remote key (or an unambiguous leading part of it) as shown
   by `snapshot list`. See
   [restoring on another machine](#restoring-on-another-machine).
-* Optional path arguments to restore specific files/directories (default: all)
+* Optional path arguments to restore specific files/directories (default: all).
+  A path that matches nothing in the snapshot is named in a warning, and the
+  command exits non-zero after restoring the paths that match.
 * Preserves file permissions, timestamps, ownership (ownership requires root),
   symlinks, and empty directories
 * `--verify`: After restoring, verify every restored file's chunk hashes

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_prune_quiet_test.go

@@ -10,11 +10,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// Flags used to build the argument vectors below.
+// flagCron is used to build the argument vectors below.
-const (
+const flagCron = "--cron"
-	flagCron  = "--cron"
-	flagForce = "--force"
-)
 
 // plantUnreferencedBlob writes a blob that no snapshot references to the
 // destination store at storeDir, so that a prune deletes it. Returns its

internal/cli/entry_snapshot_purge_test.go

@@ -0,0 +1,68 @@
+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)
+
+	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, _, _ := runEntry(t, flagConfig, configPath,
+		cmdSnapshot, cmdPurge, flagOlderThan, "30d", flagForce)
+
+	assert.Equal(t, 1, code)
+	assert.FileExists(t, manifestPath,
+		"the snapshot is still in the local index, so its metadata must stay")
+}

internal/cli/snapshot_restore.go

@@ -30,6 +30,8 @@ This command will restore files from the specified snapshot to the
 target directory.
 If no paths are specified, all files are restored.
 If paths are specified, only matching files/directories are restored.
+A path that matches nothing in the snapshot is named in a warning, and
+the command exits non-zero after restoring the paths that match.
 
 The snapshot may be named by its ID or, when restoring on a host with no
 local index, by the remote key that 'snapshot list' prints for a

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
+	defaultBlobSizeLimit    = Size(10 * 1000 * 1000 * 1000) // 10GB
-	defaultChunkSize        = Size(10 * 1024 * 1024)        // 10MB
+	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/vaultik/restore.go

@@ -27,6 +27,7 @@ import (
 // Sentinel errors for restore failures.
 var (
 	errFilesFailedRestore    = errors.New("file(s) failed to restore")
+	errPathsNotInSnapshot    = errors.New("requested path(s) not in the snapshot")
 	errFilesFailedVerify     = errors.New("files failed verification")
 	errDecryptionKeyRequired = errors.New(
 		"decryption key required for restore\n\n" +
@@ -150,16 +151,20 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 	repos := database.NewRepositories(tempDB)
 
 	// Step 2: Get list of files to restore
-	files, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
+	files, unmatched, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
 	if err != nil {
 		return fmt.Errorf("getting files to restore: %w", err)
 	}
 
+	// Named before anything is restored, so the names are shown even
+	// when the restore stops on an error.
+	unmatchedErr := v.reportUnmatchedPaths(unmatched)
+
 	if len(files) == 0 {
 		log.Warn("No files found to restore")
 		v.UI.Warningf("No files found to restore.")
 
-		return nil
+		return unmatchedErr
 	}
 
 	log.Info("Found files to restore", "count", len(files))
@@ -185,6 +190,20 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 
 	result.Duration = time.Since(startTime)
 
+	return v.finishRestore(repos, files, unmatchedErr, opts, result)
+}
+
+// finishRestore prints the restore summary and the post-restore
+// warnings, runs optional verification, and converts any failed-file
+// count into an error. Otherwise it returns unmatchedErr, the error for
+// the requested paths that matched nothing (nil when there are none).
+func (v *Vaultik) finishRestore(
+	repos *database.Repositories,
+	files []*database.File,
+	unmatchedErr error,
+	opts *RestoreOptions,
+	result *RestoreResult,
+) error {
 	log.Info("Restore complete",
 		"files_restored", result.FilesRestored,
 		"bytes_restored", ubytes(result.BytesRestored),
@@ -199,17 +218,6 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 		v.UI.Duration(result.Duration),
 	)
 
-	return v.finishRestore(repos, files, opts, result)
-}
-
-// finishRestore emits the post-restore warnings, runs optional
-// verification, and converts any failed-file count into an error.
-func (v *Vaultik) finishRestore(
-	repos *database.Repositories,
-	files []*database.File,
-	opts *RestoreOptions,
-	result *RestoreResult,
-) error {
 	if os.Geteuid() != 0 {
 		v.UI.Warningf("Restore did not preserve file ownership: chown(2) " +
 			"requires root. Re-run as root (e.g. with sudo) if you need " +
@@ -236,7 +244,23 @@ func (v *Vaultik) finishRestore(
 		return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedRestore)
 	}
 
-	return nil
+	return unmatchedErr
+}
+
+// reportUnmatchedPaths warns, by name, about each requested path that
+// matched nothing in the snapshot, and returns an error counting them,
+// or nil when there are none.
+func (v *Vaultik) reportUnmatchedPaths(unmatched []string) error {
+	if len(unmatched) == 0 {
+		return nil
+	}
+
+	for _, path := range unmatched {
+		log.Warn("Requested path matches nothing in the snapshot", "path", path)
+		v.UI.Warningf("%s matches nothing in the snapshot.", v.UI.Path(path))
+	}
+
+	return fmt.Errorf("%d %w", len(unmatched), errPathsNotInSnapshot)
 }
 
 // restoreIdentities parses the configured age secret key once into every
@@ -840,27 +864,34 @@ func (v *Vaultik) materializeSnapshotDB(
 	return db, tempDir, nil
 }
 
-// getFilesToRestore returns the list of files to restore based on path filters
+// getFilesToRestore returns the list of files to restore based on path
+// filters, and the filters, as given, that matched no file.
 func (v *Vaultik) getFilesToRestore(
 	ctx context.Context, repos *database.Repositories, pathFilters []string,
-) ([]*database.File, error) {
+) ([]*database.File, []string, error) {
 	// If no filters, get all files
 	if len(pathFilters) == 0 {
-		return repos.Files.ListAll(ctx)
+		files, err := repos.Files.ListAll(ctx)
+
+		return files, nil, err
 	}
 
 	// Get files matching the path filters
-	var result []*database.File
+	var (
+		result    []*database.File
+		unmatched []string
+	)
 
 	seen := make(map[string]bool)
 
 	for _, filter := range pathFilters {
-		// Normalize the filter path
+		files, err := repos.Files.ListUnderPath(ctx, filepath.Clean(filter))
-		filter = filepath.Clean(filter)
-
-		files, err := repos.Files.ListUnderPath(ctx, filter)
 		if err != nil {
-			return nil, fmt.Errorf("listing files under %s: %w", filter, err)
+			return nil, nil, fmt.Errorf("listing files under %s: %w", filter, err)
+		}
+
+		if len(files) == 0 {
+			unmatched = append(unmatched, filter)
 		}
 
 		for _, file := range files {
@@ -871,7 +902,7 @@ func (v *Vaultik) getFilesToRestore(
 		}
 	}
 
-	return result, nil
+	return result, unmatched, nil
 }
 
 // buildChunkToBlobMap creates a mapping from chunk hash to blob information

internal/vaultik/restore_unmatched_path_test.go

@@ -0,0 +1,202 @@
+package vaultik_test
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/config"
+	"sneak.berlin/go/vaultik/internal/database"
+	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/storage"
+	"sneak.berlin/go/vaultik/internal/storage/faultstore"
+	"sneak.berlin/go/vaultik/internal/ui"
+	"sneak.berlin/go/vaultik/internal/vaultik"
+)
+
+// These tests cover https://git.eeqj.de/sneak/vaultik/issues/309: a path
+// given to restore that matches nothing in the snapshot is named in a
+// warning and makes the restore fail, after the paths that do match are
+// restored.
+
+func TestRestoreNamesUnmatchedPathAndRestoresTheMatchingOne(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	storeDir := filepath.Join(tempDir, "remote")
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	ctx := context.Background()
+	cfg := faultTestConfig()
+	files := writeFaultSourceTree(t, fs, dataDir)
+
+	store, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+
+	id := fullFaultBackup(ctx, t, fs, store, cfg,
+		database.NewRepositories(db), dataDir, dbPath, "unmatched")
+	require.NoError(t, db.Close())
+
+	matched := filepath.Join(dataDir, "small.txt")
+	unmatched := filepath.Join(dataDir, "typo.txt")
+
+	var out bytes.Buffer
+
+	v := newReaderVaultik(ctx, cfg, store, nil, fs)
+	v.UI = ui.NewWithColor(&out, false)
+
+	err = v.Restore(&vaultik.RestoreOptions{
+		SnapshotID: id,
+		TargetDir:  restoreDir,
+		Paths:      []string{matched, unmatched},
+	})
+
+	require.Error(t, err, "a path that matches nothing must fail the restore")
+	assert.Contains(t, out.String(), unmatched,
+		"the path that matches nothing must be named")
+	assert.NotContains(t, out.String(), matched,
+		"the path that matches must not be reported")
+
+	got, err := afero.ReadFile(fs, filepath.Join(restoreDir, matched))
+	require.NoError(t, err, "the path that matches must still be restored")
+	assert.Equal(t, files[matched], got)
+}
+
+// When no requested path matches, nothing is restored and each path is
+// named. One path is relative. The other goes through the symlink that
+// is the snapshot's configured path, while the entries are stored under
+// the directory the symlink points to.
+func TestRestoreNamesEveryPathWhenNothingMatches(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	const snapshotName = "linked"
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	linkDir := filepath.Join(tempDir, "srclink")
+	storeDir := filepath.Join(tempDir, "remote")
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	ctx := context.Background()
+
+	writeFaultSourceTree(t, fs, dataDir)
+	require.NoError(t, os.Symlink(dataDir, linkDir))
+
+	cfg := faultTestConfig()
+	cfg.IndexPath = dbPath
+	cfg.ChunkSize = config.Size(faultChunkSize)
+	cfg.Snapshots = map[string]config.SnapshotConfig{
+		snapshotName: {Paths: []string{linkDir}},
+	}
+
+	store, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+
+	repos := database.NewRepositories(db)
+	v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
+
+	require.NoError(t, backUp(v, snapshotName))
+
+	id := localSnapshotID(ctx, t, repos, snapshotName)
+	require.NoError(t, db.Close())
+
+	throughLink := filepath.Join(linkDir, "sub")
+	relative := strings.TrimPrefix(filepath.Join(dataDir, "small.txt"), "/")
+
+	var out bytes.Buffer
+
+	reader := newReaderVaultik(ctx, cfg, store, nil, fs)
+	reader.UI = ui.NewWithColor(&out, false)
+
+	err = reader.Restore(&vaultik.RestoreOptions{
+		SnapshotID: id,
+		TargetDir:  restoreDir,
+		Paths:      []string{throughLink, relative},
+	})
+
+	require.Error(t, err, "a restore that matches nothing must fail")
+	assert.Contains(t, out.String(), throughLink,
+		"the path through the symlink must be named")
+	assert.Contains(t, out.String(), relative,
+		"the relative path must be named")
+
+	_, err = fs.Stat(restoreDir)
+	assert.True(t, os.IsNotExist(err), "nothing must be restored")
+}
+
+// Without --skip-errors a damaged blob stops the restore part way. The
+// path that matches nothing must still be named.
+func TestRestoreNamesUnmatchedPathWhenADamagedBlobStopsIt(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+	dataDir := filepath.Join(tempDir, "src")
+	storeDir := filepath.Join(tempDir, "remote")
+	restoreDir := filepath.Join(tempDir, "restored")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	ctx := context.Background()
+	cfg := faultTestConfig()
+
+	writeFaultSourceTree(t, fs, dataDir)
+
+	inner, err := storage.NewFileStorer(storeDir)
+	require.NoError(t, err)
+
+	db, err := database.New(ctx, dbPath)
+	require.NoError(t, err)
+
+	id := fullFaultBackup(ctx, t, fs, inner, cfg,
+		database.NewRepositories(db), dataDir, dbPath, "damaged")
+	require.NoError(t, db.Close())
+
+	damaged := faultstore.New(inner)
+	damaged.OnGet = func(key string) faultstore.GetFault {
+		if strings.HasPrefix(key, "blobs/") {
+			return faultstore.GetCorrupt
+		}
+
+		return faultstore.GetNormal
+	}
+
+	matched := filepath.Join(dataDir, "small.txt")
+	unmatched := filepath.Join(dataDir, "typo.txt")
+
+	var out bytes.Buffer
+
+	v := newReaderVaultik(ctx, cfg, damaged, nil, fs)
+	v.UI = ui.NewWithColor(&out, false)
+
+	err = v.Restore(&vaultik.RestoreOptions{
+		SnapshotID: id,
+		TargetDir:  restoreDir,
+		Paths:      []string{matched, unmatched},
+	})
+
+	require.Error(t, err, "a damaged blob must stop the restore")
+	assert.Contains(t, err.Error(), "blob",
+		"the restore must stop on the damaged blob")
+	assert.Contains(t, out.String(), unmatched,
+		"the path that matches nothing must be named")
+}

internal/vaultik/snapshot.go

@@ -678,10 +678,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",
+			return fmt.Errorf("deleting snapshot %s from local index: %w", snapshotID, err)
-				"snapshot_id", snapshotID, "error", err)
 		}
 
 		_, err = v.deleteRemoteSnapshotByKey(snapshot.RemoteSnapshotKey(snapshotID))