Honor NO_COLOR in the terminal log handler (closes #402)

The TTY log handler always wrote ANSI color, unlike internal/ui, which
disables color when NO_COLOR is set. Add a color flag to TTYHandler,
set from NO_COLOR with the same test internal/ui uses, and route every
color code through a paint helper that returns plain text when color is
off. The README's color sentence now covers log lines too.

Model: deepseek-v4-pro (implementation); opus-5-5 (rebase)

README.md

@@ -313,10 +313,12 @@ when the next path element is a number (so
 `vaultik config set age_recipients.0 age1...` works on a file without
 `age_recipients`), otherwise as a map. If the config loads, a change after which
 it would not, such as `vaultik config set compression_level 99`, is refused and
-the file is left unchanged. A config that does not load is checked one setting
-at a time. Examples are the file `config init` writes before `storage_url` is
-set, and one with a `${ENV:...}` reference to a variable not set in this shell.
-In such a config, an unknown key or a value of the wrong type is refused.
+the file is left unchanged. A config that does not load, such as the one
+`config init` writes before `storage_url` is set or one with a `${ENV:...}`
+reference to a variable not set in this shell, is checked one setting at a time:
+only a change after which the changed setting cannot be read, such as one giving
+it the wrong type (a single value where a list belongs or a word where a number
+belongs), is refused.
 
 **`snapshot create`**: Perform incremental backup of configured snapshots.
 
@@ -829,7 +831,8 @@ Work planned after 1.0. Loosely ordered by priority.
 
 Every command's user-facing output is governed by `internal/ui`, in one of two
 ways. Color is enabled when the stream written to is a TTY and the `NO_COLOR`
-environment variable is unset (https://no-color.org/).
+environment variable is unset (https://no-color.org/). The same rule governs the
+colorized log lines `internal/log` writes to a terminal.
 
 - **Status, progress, warnings, and errors** go through the `internal/ui`
   message methods below: marker-prefixed, colored on a TTY, and — except

internal/blobgen/negative_test.go

@@ -2,10 +2,8 @@ package blobgen_test
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"testing"
-	"testing/iotest"
 
 	"filippo.io/age"
 	"github.com/stretchr/testify/assert"
@@ -122,38 +120,6 @@ func TestNewReaderTrailingAndGarbage(t *testing.T) {
 	})
 }
 
-// TestNewReaderGarbageErrorQuotesNoBytes checks that lines of text in
-// place of a blob fail with ErrNotAgeStream, and that the error carries
-// none of their bytes. The lines end in a newline because age's own error
-// quotes the data's first line only when it reads a whole line.
-func TestNewReaderGarbageErrorQuotesNoBytes(t *testing.T) {
-	t.Parallel()
-
-	id, _ := makeIdentity(t)
-	garbage := bytes.Repeat([]byte("garbage-bytes-\n"), 100)
-
-	_, err := blobgen.NewReader(bytes.NewReader(garbage), id)
-
-	require.ErrorIs(t, err, blobgen.ErrNotAgeStream)
-	assert.NotContains(t, err.Error(), "garbage-bytes-")
-}
-
-// errConnectionReset stands for a storage read that fails.
-var errConnectionReset = errors.New("connection reset")
-
-// TestNewReaderReadFailure checks that a failed read of the age header is
-// returned as that read error, not as ErrNotAgeStream.
-func TestNewReaderReadFailure(t *testing.T) {
-	t.Parallel()
-
-	id, _ := makeIdentity(t)
-
-	_, err := blobgen.NewReader(iotest.ErrReader(errConnectionReset), id)
-
-	require.ErrorIs(t, err, errConnectionReset)
-	assert.NotErrorIs(t, err, blobgen.ErrNotAgeStream)
-}
-
 // TestNewWriterInvalidLevel covers the rejected end of issue case 9: an
 // out-of-range compression level errors and writes nothing to the destination.
 func TestNewWriterInvalidLevel(t *testing.T) {

internal/blobgen/reader.go

@@ -11,11 +11,6 @@ import (
 	"github.com/klauspost/compress/zstd"
 )
 
-// ErrNotAgeStream is returned by NewReader when the data cannot be opened
-// as an age stream, for example because its header is malformed. It
-// replaces age's own error, which can quote the data's first bytes.
-var ErrNotAgeStream = errors.New("not a valid age stream")
-
 // Reader wraps decompression and decryption with SHA256 verification
 type Reader struct {
 	reader       io.Reader
@@ -29,25 +24,11 @@ type Reader struct {
 // NewReader creates a new Reader that decrypts, decompresses, and verifies
 // data. Every supplied identity is offered to age.Decrypt, so a blob
 // encrypted to any one of them can be read.
-//
-// A failure to read r is returned as that error, and a blob encrypted to
-// none of the identities as age.NoIdentityMatchError. Any other failure
-// to open the age stream is ErrNotAgeStream.
 func NewReader(r io.Reader, identities ...age.Identity) (*Reader, error) {
-	src := &readErrorRecorder{reader: r}
-
-	decReader, err := age.Decrypt(src, identities...)
+	// Create decryption reader
+	decReader, err := age.Decrypt(r, identities...)
 	if err != nil {
-		var noMatch *age.NoIdentityMatchError
-
-		switch {
-		case src.err != nil:
-			return nil, fmt.Errorf("reading age header: %w", src.err)
-		case errors.As(err, &noMatch):
-			return nil, fmt.Errorf("creating decryption reader: %w", err)
-		default:
-			return nil, ErrNotAgeStream
-		}
+		return nil, fmt.Errorf("creating decryption reader: %w", err)
 	}
 
 	// Create decompression reader
@@ -71,23 +52,6 @@ func NewReader(r io.Reader, identities ...age.Identity) (*Reader, error) {
 	}, nil
 }
 
-// readErrorRecorder passes reads through to reader and keeps the first
-// error other than io.EOF, so NewReader can tell a failed read from bad
-// data.
-type readErrorRecorder struct {
-	reader io.Reader
-	err    error
-}
-
-func (r *readErrorRecorder) Read(p []byte) (int, error) {
-	n, err := r.reader.Read(p)
-	if err != nil && !errors.Is(err, io.EOF) && r.err == nil {
-		r.err = err
-	}
-
-	return n, err
-}
-
 // Read implements io.Reader
 func (r *Reader) Read(p []byte) (int, error) {
 	n, err := r.teeReader.Read(p)

internal/cli/config.go

@@ -443,10 +443,11 @@ number, otherwise as a map.
 
 If the config loads, a change after which it would not, such as
 compression_level 99, is refused and the file is left unchanged. A
-config that does not load is checked one setting at a time. Examples
-are the file config init writes before storage_url is set, and one
-with a ${ENV:...} reference to a variable not set in this shell. In
-such a config, an unknown key or a value of the wrong type is refused.
+config that does not load, such as the one config init writes before
+storage_url is set or one with a ${ENV:...} reference to a variable
+not set here, is checked one setting at a time: only a change after
+which the changed setting cannot be read, such as one giving it the
+wrong type, is refused.
 
 Examples:
   vaultik config set storage_url "file:///mnt/backups"
@@ -483,13 +484,6 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
 
 	keys := strings.Split(key, ".")
 
-	if keys[0] != config.EnvSection {
-		_, err = config.SettingType(keys)
-		if err != nil {
-			return err
-		}
-	}
-
 	err = yamlPathSet(root, keys, value)
 	if err != nil {
 		return err
@@ -745,11 +739,41 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error {
 }
 
 // configKeyIsString reports whether the dotted key names a string in
-// config.Config, as s3.access_key_id and snapshots.home.exclude.0 do.
+// config.Config, following the fields' yaml tags, as s3.access_key_id and
+// snapshots.home.exclude.0 do.
 func configKeyIsString(keys []string) bool {
-	typ, err := config.SettingType(keys)
+	typ := reflect.TypeFor[config.Config]()
 
-	return err == nil && typ.Kind() == reflect.String
+	for _, key := range keys {
+		switch {
+		case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
+			// The key is a snapshot name or a list index.
+			typ = typ.Elem()
+		case typ.Kind() == reflect.Struct:
+			field, ok := yamlField(typ, key)
+			if !ok {
+				return false
+			}
+
+			typ = field.Type
+		default:
+			return false
+		}
+	}
+
+	return typ.Kind() == reflect.String
+}
+
+// yamlField returns the field of struct type typ whose yaml tag names key.
+func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
+	for field := range typ.Fields() {
+		name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",")
+		if name == key {
+			return field, true
+		}
+	}
+
+	return reflect.StructField{}, false
 }
 
 // yamlSetInMapping resolves (creating if needed) the value node for the

internal/cli/config_test.go

@@ -225,7 +225,7 @@ func TestConfigSetIndexUnderMissingKey(t *testing.T) {
 // change after which the config would not load, with an error giving the
 // reason, and leaves the file as it was. On the file `config init` writes,
 // which does not load until storage_url is set, and on one that cannot be
-// read here, only an unknown key or a value of the wrong type is refused.
+// read here, only a value of the wrong type is refused.
 func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 	t.Parallel()
 
@@ -237,14 +237,6 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		value  string
 		reason string
 	}{
-		// Keys that no setting has, on a config that loads and on one
-		// that does not.
-		{configWithoutRecipients, "nosuchkey", "1",
-			`unknown config key "nosuchkey"`},
-		{configWithoutRecipients, "snapshots.home.exlude", "/tmp",
-			`unknown config key "snapshots.home.exlude"`},
-		{defaultConfigText(macOS), "compression_lvl", "9",
-			`unknown config key "compression_lvl"`},
 		// Index 1 of the empty list the missing key gives.
 		{configWithoutRecipients, "age_recipients.1", testRecipient,
 			"index out of range"},
@@ -252,7 +244,7 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		{configWithoutRecipients, "snapshots.home.paths", "/srv",
 			"into []string"},
 		{configWithoutRecipients, "chunk_size", "fast",
-			`chunk_size: "fast" is not a size`},
+			"invalid size format"},
 		// Values of the right type that fail validation.
 		{configWithoutRecipients, "compression_level", "99",
 			"compression_level must be between 1 and 19"},
@@ -343,39 +335,6 @@ func TestConfigSetRefusesOnlyItsOwnFailure(t *testing.T) {
 	}
 }
 
-// TestConfigSetWritesEnvSection checks that config set writes a key under
-// the top-level env section, whose keys are not settings, on a config that
-// loads.
-func TestConfigSetWritesEnvSection(t *testing.T) {
-	const name = "VAULTIK_TEST_CONFIG_SET_ENV"
-
-	// Loading the changed config sets the variable; t.Setenv restores it
-	// after the test.
-	t.Setenv(name, "")
-
-	path := filepath.Join(t.TempDir(), "config.yml")
-
-	err := os.WriteFile(path, []byte(configWithoutRecipients), configFileMode)
-	if err != nil {
-		t.Fatalf("write config: %v", err)
-	}
-
-	err = writeConfigSet(ui.NewWithColor(&bytes.Buffer{}, false),
-		path, "env."+name, "value")
-	if err != nil {
-		t.Fatalf("config set env.%s: %v", name, err)
-	}
-
-	data, err := os.ReadFile(path) //nolint:gosec // G304: test-controlled path
-	if err != nil {
-		t.Fatalf("read config: %v", err)
-	}
-
-	if !strings.Contains(string(data), "env:\n  "+name+": value\n") {
-		t.Errorf("config set env.%s wrote:\n%s", name, data)
-	}
-}
-
 // TestConfigSetStringLooksLikeNumber sets string settings to values that
 // YAML reads as numbers or booleans when they are unquoted, and checks that
 // config.Load returns each one unchanged.

internal/cli/database.go

@@ -7,7 +7,6 @@ import (
 	"github.com/spf13/cobra"
 	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
 // NewDatabaseCommand creates the database command group
@@ -91,12 +90,12 @@ Use --force to skip the confirmation prompt.`,
 
 				var confirm string
 
-				// A failure to read the answer means no confirmation.
-				_, err = fmt.Fscanln(cmd.InOrStdin(), &confirm)
+				_, err = fmt.Scanln(&confirm)
 				if err != nil || confirm != "yes" {
 					_, _ = fmt.Fprintln(w, "Aborted.")
 
-					return vaultik.ErrNotConfirmed
+					//nolint:nilerr // a failed/aborted confirmation is a clean abort
+					return nil
 				}
 			}
 

internal/cli/database_test.go

@@ -1,48 +0,0 @@
-package cli //nolint:testpackage // sets the unexported rootFlags directly
-
-import (
-	"bytes"
-	"errors"
-	"io"
-	"os"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestDatabaseDeleteDeclinedIsError checks that answering anything but
-// "yes" keeps the database and returns ErrNotConfirmed, so the command
-// exits non-zero.
-//
-//nolint:paralleltest // mutates the process-global rootFlags
-func TestDatabaseDeleteDeclinedIsError(t *testing.T) {
-	dir := t.TempDir()
-	dbPath, cfgPath := seedDatabaseDeleteConfig(t, dir)
-
-	setRootFlags(t, RootFlags{ConfigPath: cfgPath})
-
-	var out bytes.Buffer
-
-	cmd := newDatabaseDeleteCommand()
-	cmd.SetOut(&out)
-	cmd.SetErr(io.Discard)
-	cmd.SetIn(strings.NewReader("no\n"))
-	cmd.SetArgs(nil)
-
-	err := cmd.Execute()
-	if !errors.Is(err, vaultik.ErrNotConfirmed) {
-		t.Errorf("declined database delete returned %v, want %v",
-			err, vaultik.ErrNotConfirmed)
-	}
-
-	_, err = os.Stat(dbPath)
-	if err != nil {
-		t.Errorf("declined database delete removed the file: %v", err)
-	}
-
-	if !strings.Contains(out.String(), "Aborted.") {
-		t.Errorf("declined database delete printed %q, want Aborted.",
-			out.String())
-	}
-}

internal/config/config.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"reflect"
 	"sort"
 	"strings"
 	"time"
@@ -287,39 +286,15 @@ func Load(path string) (*Config, error) {
 }
 
 // Parse reads YAML config data as Load does, without validating it. It
-// refuses a key that no setting has and a size setting whose value is not a
-// size, naming the setting. It applies default values for optional
-// fields, fills in ${...} references using smartconfig, and expands ~ in
-// paths.
+// applies default values for optional fields, fills in ${...} references
+// using smartconfig, and expands ~ in paths.
 func Parse(data []byte) (*Config, error) {
-	// smartconfig drops line numbers, so keys are checked in the data as
-	// written.
-	var root yaml.Node
-
-	err := yaml.Unmarshal(data, &root)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse config: %w", err)
-	}
-
-	err = checkKeys(&root, reflect.TypeFor[Config](), "",
-		map[*yaml.Node]bool{}, map[nodeAndType]bool{})
-	if err != nil {
-		return nil, err
-	}
-
 	// Load config using smartconfig for interpolation
 	sc, err := smartconfig.NewFromReader(bytes.NewReader(data))
 	if err != nil {
 		return nil, fmt.Errorf("failed to load config file: %w", err)
 	}
 
-	// A ${...} reference can bring in keys that are not in the file, and
-	// is the value of a size setting only once it is filled in.
-	err = checkFilledConfig(sc.Data())
-	if err != nil {
-		return nil, err
-	}
-
 	cfg := &Config{
 		// Set defaults
 		BlobSizeLimit:    defaultBlobSizeLimit,

internal/config/keys.go

@@ -1,220 +0,0 @@
-package config
-
-import (
-	"errors"
-	"fmt"
-	"reflect"
-	"strconv"
-	"strings"
-
-	"gopkg.in/yaml.v3"
-)
-
-// yamlMergeTag is the tag YAML gives a merge key (<<), which adds the keys
-// of the map it names to the map it sits in.
-const yamlMergeTag = "!!merge"
-
-// EnvSection is the top-level key of a map of environment variables that
-// smartconfig sets while it loads the config. It is not a vaultik setting,
-// so its keys are not checked.
-const EnvSection = "env"
-
-var (
-	errUnknownKey           = errors.New("unknown config key")
-	errAnchorContainsItself = errors.New("anchor contains itself")
-)
-
-// nodeAndType is a node of a parsed config and the setting type it is
-// checked against.
-type nodeAndType struct {
-	node *yaml.Node
-	typ  reflect.Type
-}
-
-// SettingType returns the type of the setting that the dotted key names in
-// Config, following the fields' yaml tags, as for s3.bucket or
-// snapshots.home.exclude.0. A snapshot name or a list index may be any
-// word. It returns an unknown key error when no setting has that key.
-func SettingType(keys []string) (reflect.Type, error) {
-	typ := reflect.TypeFor[Config]()
-
-	for _, key := range keys {
-		switch {
-		case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
-			typ = typ.Elem()
-		case typ.Kind() == reflect.Struct:
-			field, ok := yamlField(typ, key)
-			if !ok {
-				return nil, fmt.Errorf("%w %q", errUnknownKey, strings.Join(keys, "."))
-			}
-
-			typ = field.Type
-		default:
-			return nil, fmt.Errorf("%w %q", errUnknownKey, strings.Join(keys, "."))
-		}
-	}
-
-	return typ, nil
-}
-
-// yamlField returns the field of struct type typ whose yaml tag names key.
-func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
-	for field := range typ.Fields() {
-		name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",")
-		if name == key && name != "-" {
-			return field, true
-		}
-	}
-
-	return reflect.StructField{}, false
-}
-
-// checkKeys returns an error naming the first key in node, a parsed config
-// or part of one at the dotted path, that no setting in typ has. The error
-// gives the key's line in the file, when it has one. A value of the wrong
-// type is left for the YAML decoder to report. following holds the aliases
-// the walk is inside. Meeting one of them again means its anchor contains
-// itself, which is an error. checked holds each anchored node the walk has
-// checked completely, with the type it was checked against. An alias to
-// that node is not followed again for that type.
-func checkKeys(node *yaml.Node, typ reflect.Type, path string,
-	following map[*yaml.Node]bool, checked map[nodeAndType]bool,
-) error {
-	switch node.Kind {
-	case yaml.DocumentNode:
-		for _, child := range node.Content {
-			err := checkKeys(child, typ, path, following, checked)
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.AliasNode:
-		if following[node] {
-			return fmt.Errorf("%w: %q on line %d",
-				errAnchorContainsItself, node.Value, node.Line)
-		}
-
-		target := nodeAndType{node.Alias, typ}
-		if checked[target] {
-			return nil
-		}
-
-		following[node] = true
-		defer delete(following, node)
-
-		err := checkKeys(node.Alias, typ, path, following, checked)
-		if err != nil {
-			return err
-		}
-
-		checked[target] = true
-	case yaml.SequenceNode:
-		if typ.Kind() != reflect.Slice {
-			return nil
-		}
-
-		for i, child := range node.Content {
-			err := checkKeys(child, typ.Elem(), joinKey(path, strconv.Itoa(i)),
-				following, checked)
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.MappingNode:
-		return checkMappingKeys(node, typ, path, following, checked)
-	case yaml.ScalarNode:
-		// A scalar has no keys.
-	}
-
-	return nil
-}
-
-// checkFilledConfig checks data, the config after smartconfig has filled in
-// its ${...} references: its keys with checkKeys, then its size settings
-// with checkSizes. A key that a reference brings in is not in the file, and
-// the copy that Encode makes has no line numbers.
-func checkFilledConfig(data map[string]any) error {
-	var filled yaml.Node
-
-	err := filled.Encode(data)
-	if err != nil {
-		return fmt.Errorf("failed to marshal config data: %w", err)
-	}
-
-	err = checkKeys(&filled, reflect.TypeFor[Config](), "",
-		map[*yaml.Node]bool{}, map[nodeAndType]bool{})
-	if err != nil {
-		return err
-	}
-
-	return checkSizes(&filled, nil)
-}
-
-// checkMappingKeys is checkKeys for a map.
-func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string,
-	following map[*yaml.Node]bool, checked map[nodeAndType]bool,
-) error {
-	for i := 0; i+1 < len(node.Content); i += 2 {
-		key, value := node.Content[i], node.Content[i+1]
-
-		if key.Tag == yamlMergeTag {
-			// The value is one map or a list of maps.
-			merged := []*yaml.Node{value}
-			if value.Kind == yaml.SequenceNode {
-				merged = value.Content
-			}
-
-			for _, m := range merged {
-				err := checkKeys(m, typ, path, following, checked)
-				if err != nil {
-					return err
-				}
-			}
-
-			continue
-		}
-
-		keyPath := joinKey(path, key.Value)
-		if keyPath == EnvSection {
-			continue
-		}
-
-		var valueType reflect.Type
-
-		switch {
-		case typ.Kind() == reflect.Map:
-			valueType = typ.Elem()
-		case typ.Kind() == reflect.Struct:
-			field, ok := yamlField(typ, key.Value)
-			if !ok && key.Line == 0 {
-				// A key that a ${...} reference brought in has no line.
-				return fmt.Errorf("%w %q", errUnknownKey, keyPath)
-			}
-
-			if !ok {
-				return fmt.Errorf("%w %q on line %d",
-					errUnknownKey, keyPath, key.Line)
-			}
-
-			valueType = field.Type
-		default:
-			return nil
-		}
-
-		err := checkKeys(value, valueType, keyPath, following, checked)
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// joinKey appends key to the dotted path.
-func joinKey(path, key string) string {
-	if path == "" {
-		return key
-	}
-
-	return path + "." + key
-}

internal/config/keys_test.go

@@ -1,163 +0,0 @@
-package config //nolint:testpackage // checks the unexported errUnknownKey
-
-import (
-	"errors"
-	"fmt"
-	"os"
-	"path/filepath"
-	"slices"
-	"strconv"
-	"strings"
-	"testing"
-)
-
-// keysTestConfig loads; each case below adds one line to it.
-const keysTestConfig = "storage_url: file:///tmp/vaultik-test-storage\n" +
-	"s3:\n" +
-	"  region: us-east-1\n" +
-	"snapshots:\n" +
-	"  test:\n" +
-	"    paths: [/tmp/vaultik-test-source]\n"
-
-// TestParseRefusesUnknownKey checks that a key no setting has, at any
-// depth, fails the parse with an error naming the key and its line. A key
-// that a ${...} reference brings in is not in the file, so its error gives
-// no line.
-func TestParseRefusesUnknownKey(t *testing.T) {
-	t.Parallel()
-
-	snapshotPath := filepath.Join(t.TempDir(), "snapshot.yml")
-
-	err := os.WriteFile(snapshotPath, []byte("other:\n"+
-		"  paths: [/tmp/vaultik-test-source]\n"+
-		"  exlude: [/tmp/cache]\n"), 0o600)
-	if err != nil {
-		t.Fatalf("write snapshot: %v", err)
-	}
-
-	tests := []struct {
-		name string
-		yaml string
-		want string
-	}{
-		{"top level", keysTestConfig + "compression_lvl: 9\n",
-			`unknown config key "compression_lvl" on line 7`},
-		{"in a snapshot", keysTestConfig + "    exlude: [/tmp/cache]\n",
-			`unknown config key "snapshots.test.exlude" on line 7`},
-		{"in s3", strings.Replace(keysTestConfig, "  region:",
-			"  bukcet: b\n  region:", 1),
-			`unknown config key "s3.bukcet" on line 3`},
-		{"from a reference",
-			keysTestConfig + "  other: ${YAML:" + snapshotPath + ":other}\n",
-			`unknown config key "snapshots.other.exlude"`},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(tt.yaml))
-			if !errors.Is(err, errUnknownKey) {
-				t.Fatalf("Parse() error = %v, want errUnknownKey", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseRefusesAnchorThatContainsItself checks that a YAML merge key (<<)
-// naming the anchor of the map it sits in fails the parse with an error.
-func TestParseRefusesAnchorThatContainsItself(t *testing.T) {
-	t.Parallel()
-
-	data := strings.Replace(keysTestConfig, "  test:", "  test: &test", 1) +
-		"    <<: *test\n"
-
-	_, err := Parse([]byte(data))
-	if !errors.Is(err, errAnchorContainsItself) {
-		t.Fatalf("Parse() error = %v, want errAnchorContainsItself", err)
-	}
-
-	want := `anchor contains itself: "test" on line 7`
-	if err.Error() != want {
-		t.Errorf("Parse() error = %q, want %q", err.Error(), want)
-	}
-}
-
-// TestParseRefusesExcessiveAliasing checks that Parse returns an error for
-// a file whose merge keys (<<) fan out. Each of eight snapshots merges the
-// one before it ten times. A key check that followed every alias would walk
-// the first snapshot over 10^8 times and run past the package's test
-// timeout.
-func TestParseRefusesExcessiveAliasing(t *testing.T) {
-	t.Parallel()
-
-	const snapshots, merges = 8, 10
-
-	var data strings.Builder
-
-	data.WriteString(strings.Replace(keysTestConfig, "  test:", "  test: &s0", 1))
-
-	for i := 1; i <= snapshots; i++ {
-		aliases := slices.Repeat([]string{"*s" + strconv.Itoa(i-1)}, merges)
-		fmt.Fprintf(&data, "  s%d: &s%d\n    <<: [%s]\n",
-			i, i, strings.Join(aliases, ", "))
-	}
-
-	_, err := Parse([]byte(data.String()))
-	if err == nil {
-		t.Fatal("Parse() returned no error")
-	}
-}
-
-// TestParseAcceptsKnownKeys checks that a config with a ${...} reference
-// and a YAML merge key (<<) still parses.
-func TestParseAcceptsKnownKeys(t *testing.T) {
-	t.Parallel()
-
-	levelPath := filepath.Join(t.TempDir(), "level")
-
-	err := os.WriteFile(levelPath, []byte("9"), 0o600)
-	if err != nil {
-		t.Fatalf("write level: %v", err)
-	}
-
-	data := strings.Replace(keysTestConfig, "  test:", "  test: &test", 1) +
-		"  other:\n" +
-		"    <<: *test\n" +
-		"    exclude: [/tmp/vaultik-test-source/cache]\n" +
-		"compression_level: ${FILE:" + levelPath + "}\n"
-
-	cfg, err := Parse([]byte(data))
-	if err != nil {
-		t.Fatalf("Parse() unexpected error: %v", err)
-	}
-
-	if cfg.CompressionLevel != 9 {
-		t.Errorf("compression_level = %d, want 9", cfg.CompressionLevel)
-	}
-
-	other := cfg.Snapshots["other"]
-	if len(other.Paths) != 1 || len(other.Exclude) != 1 {
-		t.Errorf("snapshot other = %+v, want the paths of test and one exclude",
-			other)
-	}
-}
-
-// TestParseAcceptsEnvSection checks that a config with a top-level env
-// section still parses. smartconfig sets the environment variables it
-// lists, so its keys are not settings.
-func TestParseAcceptsEnvSection(t *testing.T) {
-	const name = "VAULTIK_TEST_ENV_SECTION"
-
-	// Parse sets the variable; t.Setenv restores it after the test.
-	t.Setenv(name, "")
-
-	_, err := Parse([]byte(keysTestConfig + "env:\n  " + name + ": set\n"))
-	if err != nil {
-		t.Fatalf("Parse() unexpected error: %v", err)
-	}
-}

internal/config/size.go

@@ -4,20 +4,13 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"reflect"
-	"slices"
-	"strconv"
-	"strings"
 
 	"github.com/dustin/go-humanize"
-	"gopkg.in/yaml.v3"
 )
 
 var (
 	errSizeType     = errors.New("size must be a number or string")
 	errSizeTooLarge = errors.New("size exceeds maximum supported value")
-	errNotASize     = errors.New(
-		"is not a size; use a number of bytes or a unit such as 10MB")
 )
 
 // Size represents a byte size that can be specified in configuration files.
@@ -47,64 +40,17 @@ func (s *Size) UnmarshalYAML(unmarshal func(any) error) error {
 		return errSizeType
 	}
 
-	size, err := ParseSize(strVal)
+	// Parse the string using go-humanize
+	bytes, err := humanize.ParseBytes(strVal)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid size format: %w", err)
 	}
 
-	*s = size
-
-	return nil
-}
-
-// checkSizes returns an error naming the first size setting in node whose
-// value is not a size. node is the config, or the part of it at the dotted
-// keys, after smartconfig has filled in its ${...} references.
-func checkSizes(node *yaml.Node, keys []string) error {
-	typ, err := SettingType(keys)
-	if err != nil {
-		// Unknown keys are refused before this check, so this is the env
-		// section, which holds no settings.
-		return nil //nolint:nilerr // not an error here
+	if bytes > math.MaxInt64 {
+		return fmt.Errorf("%w: %s", errSizeTooLarge, strVal)
 	}
 
-	if typ == reflect.TypeFor[Size]() {
-		var size Size
-
-		err = node.Decode(&size)
-		if err != nil {
-			return fmt.Errorf("%s: %w", strings.Join(keys, "."), err)
-		}
-
-		return nil
-	}
-
-	switch node.Kind {
-	case yaml.DocumentNode:
-		for _, child := range node.Content {
-			err = checkSizes(child, keys)
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.MappingNode:
-		for i := 0; i+1 < len(node.Content); i += 2 {
-			err = checkSizes(node.Content[i+1],
-				slices.Concat(keys, []string{node.Content[i].Value}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.SequenceNode:
-		for i, child := range node.Content {
-			err = checkSizes(child, slices.Concat(keys, []string{strconv.Itoa(i)}))
-			if err != nil {
-				return err
-			}
-		}
-	case yaml.ScalarNode, yaml.AliasNode:
-		// A scalar holds no settings, and the filled config has no aliases.
-	}
+	*s = Size(bytes)
 
 	return nil
 }
@@ -124,12 +70,11 @@ func (s Size) String() string {
 	return humanize.Bytes(uint64(s))
 }
 
-// ParseSize parses a size string, such as "10MB", into a Size value. An
-// error quotes s.
+// ParseSize parses a size string into a Size value
 func ParseSize(s string) (Size, error) {
 	bytes, err := humanize.ParseBytes(s)
 	if err != nil {
-		return 0, fmt.Errorf("%q %w", s, errNotASize)
+		return 0, fmt.Errorf("invalid size format: %w", err)
 	}
 
 	if bytes > math.MaxInt64 {

internal/config/size_test.go

@@ -1,89 +0,0 @@
-package config //nolint:testpackage // checks the unexported errNotASize
-
-import (
-	"errors"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-)
-
-// TestParseRefusesValueThatIsNotASize checks that a size setting whose
-// value is not a size fails the parse with an error naming the setting and
-// quoting the value, also when a ${...} reference fills the value in.
-func TestParseRefusesValueThatIsNotASize(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("lots"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	tests := []struct {
-		name string
-		yaml string
-		want string
-	}{
-		{"chunk_size", keysTestConfig + "chunk_size: banana\n",
-			`chunk_size: "banana" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"blob_size_limit", keysTestConfig + "blob_size_limit: 10XB\n",
-			`blob_size_limit: "10XB" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"s3.part_size", strings.Replace(keysTestConfig, "  region:",
-			"  part_size: big\n  region:", 1),
-			`s3.part_size: "big" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-		{"from a reference",
-			keysTestConfig + "chunk_size: ${FILE:" + sizePath + "}\n",
-			`chunk_size: "lots" is not a size; ` +
-				"use a number of bytes or a unit such as 10MB"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := Parse([]byte(tt.yaml))
-			if !errors.Is(err, errNotASize) {
-				t.Fatalf("Parse() error = %v, want errNotASize", err)
-			}
-
-			if err.Error() != tt.want {
-				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
-			}
-		})
-	}
-}
-
-// TestParseAcceptsSizes checks that size settings given as a number of
-// bytes, as a unit, and through a ${...} reference still parse.
-func TestParseAcceptsSizes(t *testing.T) {
-	t.Parallel()
-
-	sizePath := filepath.Join(t.TempDir(), "size")
-
-	err := os.WriteFile(sizePath, []byte("20MB"), 0o600)
-	if err != nil {
-		t.Fatalf("write size: %v", err)
-	}
-
-	data := strings.Replace(keysTestConfig, "  region:",
-		"  part_size: 6000000\n  region:", 1) +
-		"chunk_size: ${FILE:" + sizePath + "}\n" +
-		"blob_size_limit: 1GB\n"
-
-	cfg, err := Parse([]byte(data))
-	if err != nil {
-		t.Fatalf("Parse() unexpected error: %v", err)
-	}
-
-	if cfg.ChunkSize != 20_000_000 || cfg.BlobSizeLimit != 1_000_000_000 ||
-		cfg.S3.PartSize != 6_000_000 {
-		t.Errorf("chunk_size, blob_size_limit, s3.part_size = %d, %d, %d, "+
-			"want 20000000, 1000000000, 6000000",
-			cfg.ChunkSize, cfg.BlobSizeLimit, cfg.S3.PartSize)
-	}
-}

internal/log/tty_handler.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"os"
 	"strconv"
 	"strings"
 	"sync"
@@ -60,6 +61,12 @@ type TTYHandler struct {
 	mu   *sync.Mutex
 	out  io.Writer
 
+	// color reports whether records are written with ANSI color. It is
+	// false when NO_COLOR is set, matching internal/ui's shouldColor
+	// test; the caller already chose this handler for a terminal, so
+	// TTY-ness is not re-checked here.
+	color bool
+
 	// attrs are the attributes accumulated through WithAttrs, emitted
 	// ahead of each record's own attributes. Their keys already carry
 	// the group path that was open when they were added, so no
@@ -72,16 +79,19 @@ type TTYHandler struct {
 	groups []string
 }
 
-// NewTTYHandler creates a new TTY handler with colored output.
+// NewTTYHandler creates a new TTY handler. Color is enabled unless the
+// NO_COLOR environment variable is set to a non-empty value, the same
+// test internal/ui applies to status lines.
 func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
 	if opts == nil {
 		opts = &slog.HandlerOptions{}
 	}
 
 	return &TTYHandler{
-		out:  out,
-		opts: *opts,
-		mu:   &sync.Mutex{},
+		out:   out,
+		opts:  *opts,
+		mu:    &sync.Mutex{},
+		color: os.Getenv("NO_COLOR") == "",
 	}
 }
 
@@ -124,10 +134,10 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
 	// are written around it: it can carry text from an untrusted source
 	// (a storage error, for one), and a raw control character would
 	// otherwise reach the terminal.
-	_, _ = fmt.Fprintf(h.out, "%s%s%s %s%s%s %s%s%s",
-		colorGray, timestamp, colorReset,
-		levelColor, level, colorReset,
-		colorBold, sanitize(r.Message), colorReset)
+	_, _ = fmt.Fprintf(h.out, "%s %s %s",
+		h.paint(colorGray, timestamp),
+		h.paint(levelColor, level),
+		h.paint(colorBold, sanitize(r.Message)))
 
 	// Attributes carried by the handler come first, then the record's
 	// own. Handler attributes were qualified when they were added; the
@@ -236,6 +246,7 @@ func (h *TTYHandler) clone() *TTYHandler {
 		opts:   h.opts,
 		mu:     h.mu,
 		out:    h.out,
+		color:  h.color,
 		attrs:  make([]slog.Attr, len(h.attrs), len(h.attrs)+1),
 		groups: make([]string, len(h.groups), len(h.groups)+1),
 	}
@@ -273,9 +284,19 @@ func (h *TTYHandler) writeAttr(a slog.Attr) {
 	// the snapshot database — so a control character in one of them must
 	// be rendered as an escape sequence rather than reaching the terminal,
 	// where it could move the cursor or inject its own colours.
-	_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s",
-		colorCyan, sanitize(a.Key), colorReset,
-		colorBlue, sanitize(value), colorReset)
+	_, _ = fmt.Fprintf(h.out, " %s=%s",
+		h.paint(colorCyan, sanitize(a.Key)),
+		h.paint(colorBlue, sanitize(value)))
+}
+
+// paint wraps s in the given ANSI color when color is enabled, and
+// returns s unchanged otherwise.
+func (h *TTYHandler) paint(color, s string) string {
+	if !h.color {
+		return s
+	}
+
+	return color + s + colorReset
 }
 
 // sanitize returns s unchanged when every rune in it is printable, and a

internal/log/tty_handler_test.go

@@ -421,3 +421,44 @@ func TestTTYHandlerEmptyGroupAndAttrsAreNoOps(t *testing.T) {
 	assert.Contains(t, plain, "kept=yes")
 	assert.NotContains(t, plain, " =")
 }
+
+// TestTTYHandlerRespectsNoColor checks that only a non-empty NO_COLOR
+// turns color off, matching internal/ui's shouldColor test. The handler
+// is only ever chosen for a terminal, so this is the one remaining
+// condition that can still leave color in log output.
+func TestTTYHandlerRespectsNoColor(t *testing.T) {
+	cases := []struct {
+		name    string
+		noColor string
+		colored bool
+	}{
+		{
+			name:    "NO_COLOR turns color off",
+			noColor: "1",
+		},
+		{
+			name:    "empty NO_COLOR keeps color on",
+			noColor: "",
+			colored: true,
+		},
+	}
+
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Setenv("NO_COLOR", testCase.noColor)
+
+			var buf bytes.Buffer
+
+			logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
+			logger.Info("hello", "key", "value")
+
+			if testCase.colored {
+				assert.Contains(t, buf.String(), "\x1b[",
+					"an empty NO_COLOR must leave ANSI color in log output")
+			} else {
+				assert.NotContains(t, buf.String(), "\x1b[",
+					"NO_COLOR must suppress every ANSI escape in log output")
+			}
+		})
+	}
+}

internal/vaultik/destination_marker.go

@@ -51,7 +51,7 @@ func (v *Vaultik) checkDestinationMarker() error {
 	}
 
 	// Config is nil only in tests, which then record no binding (see
-	// checkStorageBinding).
+	// EnsureStorageBinding).
 	if v.Config != nil {
 		stored, configured, err := v.storageBinding()
 		if err != nil {
@@ -83,19 +83,11 @@ func (v *Vaultik) checkDestinationMarker() error {
 	return nil
 }
 
-// markDestination refuses a local index bound to a different
-// destination, checks the backup destination store for its marker,
-// writes the marker, and then records the binding. A first backup marks
-// its destination and a later one writes the same text again. The
-// binding is recorded last, so a first backup that cannot write to the
-// destination leaves the local index unbound.
+// markDestination checks the backup destination store with
+// checkDestinationMarker and then writes the marker, so a first backup
+// marks its destination and a later one writes the same text again.
 func (v *Vaultik) markDestination() error {
-	err := v.checkStorageBinding()
-	if err != nil {
-		return err
-	}
-
-	err = v.checkDestinationMarker()
+	err := v.checkDestinationMarker()
 	if err != nil {
 		return err
 	}
@@ -107,5 +99,5 @@ func (v *Vaultik) markDestination() error {
 			destinationMarkerKey, err)
 	}
 
-	return v.EnsureStorageBinding()
+	return nil
 }

internal/vaultik/missing_destination_test.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 	"testing"
 
 	"github.com/spf13/afero"
@@ -25,28 +24,19 @@ import (
 // that is not plugged in, cannot be listed. It is not an empty store, so
 // no command may conclude from it that the local snapshots are gone.
 
-// newFileDestinationVaultik writes the fault source tree and returns a
-// Vaultik that backs it up to a file:// destination at storeDir.
+// newFileDestinationVaultik returns a Vaultik that backs up the
+// snapshot named "first" to a file:// destination at storeDir.
 func newFileDestinationVaultik(
 	ctx context.Context, t *testing.T, storeDir string,
 ) (*vaultik.Vaultik, *database.Repositories) {
 	t.Helper()
 
+	osFs := afero.NewOsFs()
 	tempDir := t.TempDir()
 	dataDir := filepath.Join(tempDir, "src")
-	writeFaultSourceTree(t, afero.NewOsFs(), dataDir)
+	dbPath := filepath.Join(tempDir, "index.sqlite")
 
-	return openFileDestinationVaultik(ctx, t, storeDir, dataDir,
-		filepath.Join(tempDir, "index.sqlite"))
-}
-
-// openFileDestinationVaultik returns a Vaultik that backs up dataDir to a
-// file:// destination at storeDir, with its local index at dbPath. The
-// index is closed when the test ends.
-func openFileDestinationVaultik(
-	ctx context.Context, t *testing.T, storeDir, dataDir, dbPath string,
-) (*vaultik.Vaultik, *database.Repositories) {
-	t.Helper()
+	writeFaultSourceTree(t, osFs, dataDir)
 
 	store, err := storage.NewFileStorer(storeDir)
 	require.NoError(t, err)
@@ -58,84 +48,22 @@ func openFileDestinationVaultik(
 	repos := database.NewRepositories(db)
 	cfg := changedFileConfig(dataDir, dbPath)
 
-	return newBackupVaultik(ctx, cfg, store, repos, db, afero.NewOsFs()), repos
-}
-
-// firstBackup is one backup of the snapshot named "first" to a file://
-// destination. dir holds its source tree in src, its store in store and
-// its closed local index in index/index.sqlite. The first test that
-// needs it makes it, and TestMain removes it. Every test that copies it
-// backs up the same source tree, so no test may change that tree.
-//
-//nolint:gochecknoglobals // made once and shared by the tests that copy it
-var firstBackup struct {
-	once  sync.Once
-	dir   string
-	ready bool
-}
-
-// TestMain removes firstBackup once every test has run.
-func TestMain(m *testing.M) {
-	code := m.Run()
-
-	_ = os.RemoveAll(firstBackup.dir)
-
-	os.Exit(code)
-}
-
-// makeFirstBackup writes the fault source tree and backs it up as the
-// snapshot named "first", all in a new directory that it records in
-// firstBackup.
-func makeFirstBackup(ctx context.Context, t *testing.T) {
-	t.Helper()
-
-	dir, err := os.MkdirTemp("", "vaultik-first-backup-")
-	require.NoError(t, err)
-
-	firstBackup.dir = dir
-	storeDir := filepath.Join(dir, "store")
-	dataDir := filepath.Join(dir, "src")
-	indexDir := filepath.Join(dir, "index")
-
-	require.NoError(t, os.Mkdir(storeDir, 0o750))
-	require.NoError(t, os.Mkdir(indexDir, 0o750))
-	writeFaultSourceTree(t, afero.NewOsFs(), dataDir)
-
-	v, _ := openFileDestinationVaultik(ctx, t, storeDir, dataDir,
-		filepath.Join(indexDir, "index.sqlite"))
-	require.NoError(t, backUp(v, "first"))
-	require.NoError(t, v.DB.Close())
-
-	firstBackup.ready = true
+	return newBackupVaultik(ctx, cfg, store, repos, db, osFs), repos
 }
 
 // backUpToFileDestination creates the directory storeDir, as a user must
-// before the first backup, and returns a Vaultik whose file://
-// destination there holds a backup of the snapshot named "first". The
-// store and the local index are copies of firstBackup's, which is faster
-// than backing up in every test. The returned Vaultik backs up a source
-// tree shared with every other caller, so no test may change that tree.
-// Everything the returned Vaultik prints after the backup goes to the
-// returned buffer.
+// before the first backup, and backs up the snapshot named "first" to a
+// file:// destination there. Everything the returned Vaultik prints
+// after the backup goes to the returned buffer.
 func backUpToFileDestination(
 	ctx context.Context, t *testing.T, storeDir string,
 ) (*vaultik.Vaultik, *database.Repositories, *bytes.Buffer) {
 	t.Helper()
 
-	firstBackup.once.Do(func() { makeFirstBackup(ctx, t) })
-	require.True(t, firstBackup.ready, "another test failed to make firstBackup")
-
 	require.NoError(t, os.MkdirAll(storeDir, 0o750))
-	require.NoError(t, os.CopyFS(storeDir,
-		os.DirFS(filepath.Join(firstBackup.dir, "store"))))
 
-	indexDir := t.TempDir()
-	require.NoError(t, os.CopyFS(indexDir,
-		os.DirFS(filepath.Join(firstBackup.dir, "index"))))
-
-	v, repos := openFileDestinationVaultik(ctx, t, storeDir,
-		filepath.Join(firstBackup.dir, "src"),
-		filepath.Join(indexDir, "index.sqlite"))
+	v, repos := newFileDestinationVaultik(ctx, t, storeDir)
+	require.NoError(t, backUp(v, "first"))
 
 	out := &bytes.Buffer{}
 	v.Stdout = out

internal/vaultik/prune.go

@@ -68,7 +68,7 @@ func (v *Vaultik) NukeRemote(force bool) error {
 	// the same way, so the advice below must not follow these errors.
 	// The listing also fails on a destination that answers that it has
 	// no marker while the local index holds snapshots.
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -192,7 +192,7 @@ type PruneBlobsResult struct {
 // PruneDatabase or PruneBlobs individually unless it specifically wants
 // one half.
 func (v *Vaultik) Prune(opts *PruneOptions) error {
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -267,7 +267,7 @@ func (v *Vaultik) PruneBlobs(opts *PruneOptions) error {
 
 	if !v.confirmPrune(opts, len(unreferencedBlobs), totalSize,
 		dirsWithoutManifest) {
-		return ErrNotConfirmed
+		return nil
 	}
 
 	v.deleteUnreferencedBlobs(unreferencedBlobs, allBlobs, result)

internal/vaultik/prune_metadata_dir_test.go

@@ -176,8 +176,7 @@ func TestPruneDeclinedKeepsMetadataDirWithoutManifest(t *testing.T) {
 
 	env.v.Stdin = strings.NewReader("n\n")
 
-	require.ErrorIs(t, env.v.PruneBlobs(&vaultik.PruneOptions{}),
-		vaultik.ErrNotConfirmed)
+	require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{}))
 
 	assert.True(t, env.store.hasKey(leftDB),
 		"a declined prune must keep the directory")

internal/vaultik/remove_quiet_test.go

@@ -54,10 +54,9 @@ func TestPurgeSnapshots_QuietPromptListsSnapshots(t *testing.T) {
 	v.UI.SetQuiet(true)
 
 	// Stdin is empty, so the prompt reads no answer and cancels.
-	err := v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
+	require.NoError(t, v.PurgeSnapshotsWithOptions(&vaultik.SnapshotPurgeOptions{
 		KeepLatest: true,
-	})
-	require.ErrorIs(t, err, vaultik.ErrNotConfirmed)
+	}))
 
 	assert.Contains(t, out.String(), snapHomeT0)
 	assert.Contains(t, out.String(), "Delete 1 snapshot(s)? [y/N]")
@@ -78,8 +77,7 @@ func TestPruneBlobs_QuietPromptShowsBlobTotal(t *testing.T) {
 	addBlob(t, env.store.testStorer, testBlobHashA)
 
 	// Stdin is empty, so the prompt reads no answer and cancels.
-	require.ErrorIs(t, env.v.PruneBlobs(&vaultik.PruneOptions{}),
-		vaultik.ErrNotConfirmed)
+	require.NoError(t, env.v.PruneBlobs(&vaultik.PruneOptions{}))
 
 	assert.Contains(t, env.stdout.String(),
 		"Found 1 unreferenced blob(s) totaling")

internal/vaultik/remove_snapshot_test.go

@@ -337,27 +337,6 @@ func TestRemoveSnapshot_DefaultRemovesMetadataNotBlobs(t *testing.T) {
 	assert.Contains(t, out, "vaultik prune")
 }
 
-// TestRemoveSnapshot_DeclinedIsError checks that answering no to the
-// prompt keeps the snapshot and returns ErrNotConfirmed, so the command
-// exits non-zero.
-func TestRemoveSnapshot_DeclinedIsError(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	store := newTestStorer()
-	addManifest(t, store, "snapshot-001", nil)
-
-	tv := vaultik.NewForTesting(store)
-	tv.Repositories = newLocalIndex(t, "snapshot-001")
-	tv.Stdin.WriteString("n\n")
-
-	_, err := tv.RemoveSnapshot("snapshot-001", &vaultik.RemoveOptions{})
-
-	require.ErrorIs(t, err, vaultik.ErrNotConfirmed)
-	assert.True(t, store.hasKey(remoteKeyPath("snapshot-001")))
-	assert.Contains(t, tv.Stdout.String(), "Cancelled")
-}
-
 func TestRemoveSnapshot_DryRun(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()

internal/vaultik/restore.go

@@ -106,12 +106,7 @@ type RestoreResult struct {
 	BytesDownloaded int64
 	Duration        time.Duration
 	FilesFailed     int
-	FailedFiles     []string // Paths of files that failed to restore for their own reason
-	// Blobs that could not be downloaded, and the paths of the files
-	// skipped because they need one of them. Those files are counted in
-	// FilesFailed but not listed in the summary.
-	BlobsFailed            int
-	FilesNeedingFailedBlob []string
+	FailedFiles     []string // Paths of files that failed to restore
 	// Verification results (only populated if Verify option is set)
 	FilesVerified     int
 	BytesVerified     int64
@@ -133,15 +128,10 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
 		"paths", opts.Paths,
 	)
 
-	remoteKey, err := v.resolveSnapshotRemoteKey(opts.SnapshotID)
-	if err != nil {
-		return err
-	}
-
 	// Step 1: Download and decrypt the snapshot metadata database
 	log.Info("Downloading snapshot metadata...")
 
-	tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, remoteKey, identities)
+	tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, identities)
 	if err != nil {
 		return fmt.Errorf("downloading snapshot database: %w", err)
 	}
@@ -236,11 +226,6 @@ func (v *Vaultik) finishRestore(
 	if result.FilesFailed > 0 {
 		v.UI.Warningf("%d file(s) failed to restore:", result.FilesFailed)
 
-		if result.BlobsFailed > 0 {
-			v.UI.Detailf("%d of them need a blob that could not be read (%d blob(s) in all).",
-				len(result.FilesNeedingFailedBlob), result.BlobsFailed)
-		}
-
 		for _, path := range result.FailedFiles {
 			v.UI.Detailf("%s", v.UI.Path(path))
 		}
@@ -509,8 +494,8 @@ func (v *Vaultik) runRestoreLoop(
 // Returns false when nothing is pending download (the caller stops).
 //
 // A blob that cannot be downloaded is reported through
-// handleRestoreBlobError, so it aborts the restore unless --skip-errors
-// is set.
+// handleRestoreFileError for every pending file that references it, so
+// it aborts the restore unless --skip-errors is set.
 func (s *restoreSession) downloadNextBlobSet(
 	plan *restorePlan, filesByID map[types.FileID]*database.File,
 ) (bool, error) {
@@ -543,11 +528,12 @@ func (s *restoreSession) downloadNextBlobSet(
 				return false, err
 			}
 
-			blobErr := s.v.handleRestoreBlobError(
-				plan, s.opts, s.result, filesByID,
-				plan.filesReferencingBlob(hash), err)
-			if blobErr != nil {
-				return false, blobErr
+			for _, fileID := range plan.filesReferencingBlob(hash) {
+				fileErr := s.v.handleRestoreFileError(
+					plan, s.opts, s.result, filesByID[fileID], fileID, err)
+				if fileErr != nil {
+					return false, fileErr
+				}
 			}
 
 			// next is among the failed files, so the rest of its blob set
@@ -662,41 +648,6 @@ func (v *Vaultik) handleRestoreFileError(
 	return nil
 }
 
-// handleRestoreBlobError records a blob that could not be downloaded.
-// fileIDs are the pending files that need it, never empty. Without
-// --skip-errors the first of them fails the restore. With it, the blob is
-// reported once, and its files are counted as failed and dropped from
-// the plan without being reported one by one.
-func (v *Vaultik) handleRestoreBlobError(
-	plan *restorePlan, opts *RestoreOptions, result *RestoreResult,
-	filesByID map[types.FileID]*database.File, fileIDs []types.FileID,
-	err error,
-) error {
-	if !opts.SkipErrors {
-		return v.handleRestoreFileError(
-			plan, opts, result, filesByID[fileIDs[0]], fileIDs[0], err)
-	}
-
-	log.Error("Failed to restore files", "files", len(fileIDs), "error", err)
-	v.UI.Errorf("Failed to restore %d file(s): %v. Skipping them (--skip-errors).",
-		len(fileIDs), err)
-
-	result.BlobsFailed++
-
-	for _, fileID := range fileIDs {
-		path := filesByID[fileID].Path.String()
-
-		log.Debug("Skipped file that needs a failed blob", "path", path)
-
-		result.FilesFailed++
-		result.FilesNeedingFailedBlob = append(result.FilesNeedingFailedBlob, path)
-
-		plan.finishFile(fileID)
-	}
-
-	return nil
-}
-
 // printRestoreProgress emits a periodic restore-phase status line via
 // the UI writer, mirroring scanner.printProcessingProgress so the two
 // long-running commands have the same on-screen rhythm.
@@ -768,8 +719,8 @@ func (v *Vaultik) handleRestoreVerification(
 ) 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, result.FilesFailed)
-	for _, path := range slices.Concat(result.FailedFiles, result.FilesNeedingFailedBlob) {
+	failedToRestore := make(map[string]bool, len(result.FailedFiles))
+	for _, path := range result.FailedFiles {
 		failedToRestore[path] = true
 	}
 
@@ -803,12 +754,19 @@ func (v *Vaultik) handleRestoreVerification(
 	return nil
 }
 
-// downloadSnapshotDB downloads and decrypts the metadata database of the
-// snapshot at remoteKey, which resolveSnapshotRemoteKey returned for
-// snapshotID.
+// downloadSnapshotDB downloads and decrypts the snapshot metadata
+// database. The identifier is resolved to the snapshot's remote key: a
+// human ID is hashed, and a remote key (or its abbreviation, as printed
+// for a remote-only snapshot) is used as-is, so a host with no local
+// index can restore the snapshots it can only see on the store.
 func (v *Vaultik) downloadSnapshotDB(
-	snapshotID, remoteKey string, identities []age.Identity,
+	snapshotID string, identities []age.Identity,
 ) (*database.DB, string, error) {
+	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
+	if err != nil {
+		return nil, "", err
+	}
+
 	// Download encrypted database from storage
 	dbKey := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey)
 

internal/vaultik/restore_locality_test.go

@@ -46,11 +46,11 @@ import (
 //     slice out a few KB — currently the dominant cost in restore.
 //
 // The test deliberately constructs an adversarial scenario: three
-// blobs A/B/C of five 64 KiB chunks each, nine files distributed across
-// them, and path-ordered names that interleave the blobs (a1, b1, c1,
-// a2, b2, c2, …) so naive path-order processing would touch every blob
-// before finishing any of them.
-// localitySource is one 64 KiB fixture file used by the locality test.
+// blobs A/B/C of ~6 MB each, nine files distributed across them, and
+// path-ordered names that interleave the blobs (a1, b1, c1, a2, b2,
+// c2, …) so naive path-order processing would touch every blob before
+// finishing any of them.
+// localitySource is one 1 MiB fixture file used by the locality test.
 type localitySource struct {
 	path string
 	data []byte
@@ -65,10 +65,10 @@ type localityCopy struct {
 
 // localitySourceBytes is the size of each source file of the locality
 // fixture.
-const localitySourceBytes = 64 * 1024
+const localitySourceBytes = 1024 * 1024
 
 // buildLocalityFixture writes the adversarial source layout described
-// in TestRestoreLocalityAndReadAt: 15 sources of 64 KiB each (which the
+// in TestRestoreLocalityAndReadAt: 15 sources of 1 MiB each (which the
 // backup packs into 3 blobs of 5 chunks) plus 9 interleaved-name copies
 // (3 per blob group).
 func buildLocalityFixture(

internal/vaultik/restore_skip_errors_test.go

@@ -51,72 +51,27 @@ func TestRestoreSkipErrorsSkipsFilesOfMissingBlob(t *testing.T) {
 	t.Parallel()
 
 	ctx := context.Background()
-	backup := backupThenDeleteOneBlob(ctx, t)
-	out := assertRestoreSkipsLostFiles(ctx, t, backup)
-
-	assertLostFilesCountedNotListed(t, backup, out)
-}
-
-// TestRestoreSkipErrorsReportsUnreadableBlobOnce restores with SkipErrors
-// after one blob of a two-blob snapshot was overwritten with other bytes
-// of the same size. The blob must be reported once, by its hash, as not a
-// valid age stream, without quoting its bytes, and its files must be
-// counted, not listed.
-func TestRestoreSkipErrorsReportsUnreadableBlobOnce(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	backup, lostBlob := backupThenOverwriteOneBlob(ctx, t)
-	out := assertRestoreSkipsLostFiles(ctx, t, backup)
-
-	assert.Equal(t, 1, strings.Count(out, "not a valid age stream"), out)
-	assert.Contains(t, out, lostBlob[:16])
-	assert.NotContains(t, out, overwriteBlobPattern)
-	assertLostFilesCountedNotListed(t, backup, out)
+	assertRestoreSkipsLostFiles(ctx, t, backupThenDeleteOneBlob(ctx, t))
 }
 
 // TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob restores with
 // SkipErrors a snapshot whose metadata lists chunks that no blob holds, as
-// earlier backup defects left behind. Each such file fails for its own
-// reason, so each is listed.
+// earlier backup defects left behind.
 func TestRestoreSkipErrorsSkipsFilesWithChunkInNoBlob(t *testing.T) {
 	log.Initialize(log.Config{})
 	t.Parallel()
 
 	ctx := context.Background()
-	backup := backupThenDropOneBlobFromMetadata(ctx, t)
-	out := assertRestoreSkipsLostFiles(ctx, t, backup)
-
-	for path := range backup.lost {
-		assert.Containsf(t, out, path, "%s must be reported", path)
-	}
+	assertRestoreSkipsLostFiles(ctx, t, backupThenDropOneBlobFromMetadata(ctx, t))
 }
 
-// assertLostFilesCountedNotListed checks that the restore output out
-// counts the files of the lost blob and names none of them.
-func assertLostFilesCountedNotListed(
-	t *testing.T, backup *missingBlobBackup, out string,
-) {
-	t.Helper()
-
-	assert.Contains(t, out, fmt.Sprintf(
-		"%d of them need a blob that could not be read (1 blob(s) in all)",
-		len(backup.lost)))
-
-	for path := range backup.lost {
-		assert.NotContainsf(t, out, path,
-			"%s needs the lost blob and must be counted, not listed", path)
-	}
-}
-
-// assertRestoreSkipsLostFiles restores backup with SkipErrors and returns
-// what it printed. Every file stored in the lost blob must be left
-// absent, every other file must be restored intact, and Restore must
-// still return an error.
+// assertRestoreSkipsLostFiles restores backup with SkipErrors. Every file
+// stored in the lost blob must be reported as failed and left absent,
+// every other file must be restored intact, and Restore must still return
+// an error.
 func assertRestoreSkipsLostFiles(
 	ctx context.Context, t *testing.T, backup *missingBlobBackup,
-) string {
+) {
 	t.Helper()
 
 	var out bytes.Buffer
@@ -138,6 +93,8 @@ func assertRestoreSkipsLostFiles(
 		restored := filepath.Join(backup.restoreDir, path)
 
 		if backup.lost[path] {
+			assert.Containsf(t, out.String(), path,
+				"%s needs the lost blob and must be reported", path)
 			assert.NoFileExists(t, restored)
 
 			continue
@@ -147,8 +104,6 @@ func assertRestoreSkipsLostFiles(
 		require.NoErrorf(t, err, "%s does not need the lost blob", path)
 		assert.Equalf(t, content, got, "%s restored with wrong content", path)
 	}
-
-	return out.String()
 }
 
 // TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles restores with
@@ -189,9 +144,9 @@ func TestRestoreSkipErrorsVerifyChecksOnlyRestoredFiles(t *testing.T) {
 // 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 count the files of the deleted blob without
-// listing them, the verification summary must list only the long file,
-// once, and Restore must return an error.
+// 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()
@@ -231,9 +186,9 @@ func TestRestoreSkipErrorsVerifyReportsFailuresApart(t *testing.T) {
 		"Verification failed: 1 files did not match")
 	require.Truef(t, found, "no verification summary in:\n%s", out.String())
 
-	assertLostFilesCountedNotListed(t, backup, warning)
-
 	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)
 	}
@@ -337,35 +292,6 @@ func backupThenDeleteOneBlob(
 	return backup
 }
 
-// overwriteBlobPattern repeats, each time followed by a newline, through
-// the bytes that backupThenOverwriteOneBlob writes over a blob. age's own
-// error quotes the data's first line only when it reads a whole line.
-const overwriteBlobPattern = "overwritten-blob-bytes-"
-
-// backupThenOverwriteOneBlob backs up missingBlobFileCount single-chunk
-// files, then overwrites the blob holding the first of them with as many
-// bytes of overwriteBlobPattern lines. It also returns that blob's hash.
-func backupThenOverwriteOneBlob(
-	ctx context.Context, t *testing.T,
-) (*missingBlobBackup, string) {
-	t.Helper()
-
-	backup, db, lostBlob := backupSingleChunkFiles(ctx, t)
-	require.NoError(t, db.Close())
-
-	key := fmt.Sprintf("blobs/%s/%s/%s", lostBlob[:2], lostBlob[2:4], lostBlob)
-
-	info, err := backup.storer.Stat(ctx, key)
-	require.NoError(t, err)
-
-	line := overwriteBlobPattern + "\n"
-	pattern := bytes.Repeat([]byte(line), int(info.Size)/len(line)+1)
-	require.NoError(t, backup.storer.Put(
-		ctx, key, bytes.NewReader(pattern[:info.Size])))
-
-	return backup, lostBlob
-}
-
 // backupThenDropOneBlobFromMetadata backs up missingBlobFileCount
 // single-chunk files, then removes the blob holding the first of them from
 // the snapshot and exports the snapshot metadata again. The exported

internal/vaultik/restore_sweeper_integration_test.go

@@ -27,10 +27,10 @@ import (
 // TestRestoreSweeperEvictsBlobs exercises the reference-counted blob
 // disk cache eviction during restore.
 //
-// The scenario: 30 unique 128 KiB random files plus 10 duplicates of
-// those (40 files total, 3.75 MiB of unique content) get backed up with
-// a 1 MiB blob_size_limit. After backup the snapshot's encrypted blobs
-// are restored through Vaultik.Restore, and per-key Get counts on the
+// The scenario: 30 unique 1 MB random files plus 10 duplicates of those
+// (40 files total, 30 MB of unique content) get backed up with a 10 MB
+// blob_size_limit. After backup the snapshot's encrypted blobs are
+// restored through Vaultik.Restore, and per-key Get counts on the
 // storage layer are recorded. Each blob in the snapshot MUST be
 // downloaded exactly once — re-downloads would mean the sweeper either
 // evicted a blob that was still needed (LRU regression) or that the
@@ -41,7 +41,7 @@ import (
 // original AND every duplicate referencing its chunks have been
 // restored.
 // buildSweeperFixture writes the source layout for the sweeper test: 30
-// unique 128 KiB random files plus 10 duplicates of a random subset. The
+// unique 1 MB random files plus 10 duplicates of a random subset. The
 // PRNG seed is fixed so failures are reproducible; the entropy is what
 // matters here — the FastCDC chunker needs realistic-looking data to
 // pick chunk boundaries naturally. Returns the expected content by
@@ -51,7 +51,7 @@ func buildSweeperFixture(
 ) map[string][]byte {
 	t.Helper()
 
-	const fileSize = 128 * 1024
+	const fileSize = 1 * 1024 * 1024
 
 	rng := rand.New(rand.NewSource(42)) //nolint:gosec // G404: fixture data only
 
@@ -159,7 +159,7 @@ func runSweeperBackup(
 	require.NoError(t, err)
 	require.Equal(t, uniqueFiles+duplicateFiles, scanResult.FilesScanned)
 	require.Greater(t, scanResult.BlobsCreated, 1,
-		"3.75 MiB of unique data at 1 MiB blob size should yield multiple blobs")
+		"30 MB of unique data at 10 MB blob size should yield multiple blobs")
 
 	require.NoError(t, sm.CompleteSnapshot(ctx, snapshotID))
 	require.NoError(t, sm.ExportSnapshotMetadata(ctx, dbPath, snapshotID))
@@ -199,8 +199,8 @@ func TestRestoreSweeperEvictsBlobs(t *testing.T) {
 
 	expected := buildSweeperFixture(t, fs, dataDir, uniqueFiles, duplicateFiles)
 
-	chunkSize := int64(16 * 1024)
-	maxBlobSize := int64(1024 * 1024)
+	chunkSize := int64(64 * 1024)
+	maxBlobSize := int64(10 * 1024 * 1024)
 
 	storer, err := storage.NewFileStorer(storeDir)
 	require.NoError(t, err)

internal/vaultik/snapshot.go

@@ -115,9 +115,14 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error {
 		"index_path", v.Config.IndexPath,
 	)
 
+	err := v.EnsureStorageBinding()
+	if err != nil {
+		return err
+	}
+
 	// Before PruneDatabase: it drops incomplete snapshots, which could
 	// leave the local index without one and so skip the marker check.
-	err := v.markDestination()
+	err = v.markDestination()
 	if err != nil {
 		return err
 	}
@@ -744,7 +749,7 @@ type SnapshotPurgeOptions struct {
 // snapshot name, not the latest globally. This prevents `home` and `system`
 // snapshots from cannibalizing each other.
 func (v *Vaultik) PurgeSnapshotsWithOptions(opts *SnapshotPurgeOptions) error {
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -876,12 +881,17 @@ func (v *Vaultik) confirmAndExecutePurge(
 
 		var confirm string
 
-		// A failure to read the answer means no confirmation.
 		_, err := v.scanStdin(&confirm)
-		if err != nil || strings.ToLower(confirm) != "y" {
+		if err != nil {
 			v.printlnStdout("Cancelled")
 
-			return ErrNotConfirmed
+			return nil //nolint:nilerr // treat EOF or read error as "no"
+		}
+
+		if strings.ToLower(confirm) != "y" {
+			v.printlnStdout("Cancelled")
+
+			return nil
 		}
 	} else if !quiet {
 		v.UI.Infof("The following snapshots will be deleted:")
@@ -959,22 +969,25 @@ func (v *Vaultik) VerifySnapshotWithOptions(
 		Mode:       "shallow",
 	}
 
-	// The key is kept so we can also check for the snapshot's encrypted
-	// database below.
+	v.printVerifyHeader(snapshotID, opts)
+
+	// Resolve the identifier to the snapshot's remote key. A human ID is
+	// hashed; a remote key (or its abbreviation, as printed for a
+	// remote-only snapshot) is used as-is, so a host with no local index
+	// can verify a snapshot it can only see on the store. The key is kept
+	// so we can also check for the snapshot's encrypted database below.
 	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
 	if err != nil {
 		if opts.JSON {
 			result.Status = verifyStatusFailed
-			result.ErrorMessage = err.Error()
+			result.ErrorMessage = fmt.Sprintf("resolving snapshot identifier: %v", err)
 
 			return v.outputVerifyJSON(result)
 		}
 
-		return err
+		return fmt.Errorf("resolving snapshot identifier: %w", err)
 	}
 
-	v.printVerifyHeader(snapshotID, opts)
-
 	manifest, err := v.downloadManifestByKey(remoteKey)
 	if err != nil {
 		if opts.JSON {
@@ -1272,7 +1285,7 @@ func (v *Vaultik) outputVerifyJSON(result *VerifyResult) error {
 // jq` fail (issue #108). The narration is duplicated as log records,
 // which go to stderr and so cannot corrupt the document.
 func (v *Vaultik) CleanupLocalSnapshots(opts *PruneOptions) error {
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -1462,12 +1475,12 @@ func (v *Vaultik) RemoveSnapshot(
 		SnapshotID: snapshotID,
 	}
 
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return result, err
 	}
 
-	inLocalIndex, err := v.checkSnapshotExists(snapshotID, opts.LocalOnly)
+	inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
 	if err != nil {
 		return result, err
 	}
@@ -1477,7 +1490,7 @@ func (v *Vaultik) RemoveSnapshot(
 	}
 
 	if !v.confirmRemoveSnapshot(snapshotID, opts) {
-		return result, ErrNotConfirmed
+		return result, nil
 	}
 
 	// Checked before the local record is deleted: it may be the last
@@ -1530,20 +1543,17 @@ func (v *Vaultik) RemoveSnapshot(
 	return result, nil
 }
 
-// checkSnapshotExists returns errSnapshotNotFound when the local index
-// has no row for snapshotID and, unless localOnly is set, the
-// destination store has no metadata for it either. inLocalIndex reports
-// whether the local index has that row. The destination store is
-// consulted only when the local index has no row, so a snapshot in the
-// local index can still be removed from it while the destination store
-// is unreachable, and only after checkDestinationMarker passes, so an
-// unplugged volume's empty mount point is not taken for a store without
-// the snapshot.
-//
-//nolint:nonamedreturns // the result name says what the bool means
-func (v *Vaultik) checkSnapshotExists(
-	snapshotID string, localOnly bool,
-) (inLocalIndex bool, err error) {
+// findSnapshotToRemove reports whether the local index has a row for
+// snapshotID, and returns errSnapshotNotFound when there is nothing to
+// remove: no row and, unless LocalOnly is set, no metadata on the
+// destination store either. The destination store is consulted only
+// when the local index has no row, so a snapshot in the local index can
+// still be removed from it while the destination store is unreachable,
+// and only after checkDestinationMarker passes, so an unplugged volume's
+// empty mount point is not taken for a store without the snapshot.
+func (v *Vaultik) findSnapshotToRemove(
+	snapshotID string, opts *RemoveOptions,
+) (bool, error) {
 	if v.Repositories != nil {
 		snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
 		if err != nil {
@@ -1555,12 +1565,12 @@ func (v *Vaultik) checkSnapshotExists(
 		}
 	}
 
-	if localOnly {
+	if opts.LocalOnly {
 		return false, fmt.Errorf("%w in the local index: %s",
 			errSnapshotNotFound, snapshotID)
 	}
 
-	err = v.checkDestinationMarker()
+	err := v.checkDestinationMarker()
 	if err != nil {
 		return false, err
 	}
@@ -1702,7 +1712,7 @@ func (v *Vaultik) removeSnapshotRemote(
 // before it removes anything when the destination store cannot be
 // listed or is not present by checkDestinationMarker.
 func (v *Vaultik) RemoveAllSnapshots(opts *RemoveOptions) (*RemoveResult, error) {
-	err := v.checkStorageBinding()
+	err := v.EnsureStorageBinding()
 	if err != nil {
 		return nil, err
 	}

internal/vaultik/snapshot_identifier.go

@@ -28,9 +28,8 @@ var (
 // Two forms are accepted, matching the two things a host can know:
 //
 //   - A human snapshot ID (hostname_name_timestamp), which a host holding
-//     the local index has. It is hashed to its remote key. An ID that
-//     names no snapshot in the local index or on the destination store
-//     is an error (see checkSnapshotExists).
+//     the local index has. It is hashed to its remote key; the store is
+//     not consulted.
 //   - A remote key, or the leading part of one, which is all a host with
 //     no local index can know — it is exactly what `snapshot list` prints
 //     for a remote-only snapshot (see formatRemoteOnlyID). It is resolved
@@ -42,11 +41,6 @@ var (
 // RFC3339 timestamp).
 func (v *Vaultik) resolveSnapshotRemoteKey(identifier string) (string, error) {
 	if !isRemoteKeyOrPrefix(identifier) {
-		_, err := v.checkSnapshotExists(identifier, false)
-		if err != nil {
-			return "", err
-		}
-
 		return snapshot.RemoteSnapshotKey(identifier), nil
 	}
 

internal/vaultik/snapshot_list_test.go

@@ -187,17 +187,6 @@ func (e *listEnv) addRemoteRawTimestamp(
 	t.Helper()
 
 	remoteKey := snapshot.RemoteSnapshotKey(snapshotID)
-	e.putRemoteManifest(t, remoteKey,
-		encodeRemoteManifest(t, remoteKey, timestamp))
-
-	return remoteKey
-}
-
-// encodeRemoteManifest returns the encoded manifest of a one-blob
-// snapshot whose remote key is remoteKey.
-func encodeRemoteManifest(t *testing.T, remoteKey, timestamp string) []byte {
-	t.Helper()
-
 	manifest := &snapshot.Manifest{
 		// Note: the hashed key, never the human ID. That is precisely
 		// why a remote-only snapshot cannot be named.
@@ -213,17 +202,11 @@ func encodeRemoteManifest(t *testing.T, remoteKey, timestamp string) []byte {
 	data, err := snapshot.EncodeManifest(manifest, 3)
 	require.NoError(t, err)
 
-	return data
-}
-
-// putRemoteManifest stores an encoded manifest on the destination store
-// under remoteKey.
-func (e *listEnv) putRemoteManifest(t *testing.T, remoteKey string, data []byte) {
-	t.Helper()
-
-	err := e.store.Put(context.Background(),
+	err = e.store.Put(context.Background(),
 		"metadata/"+remoteKey+"/manifest.json.zst", bytes.NewReader(data))
 	require.NoError(t, err)
+
+	return remoteKey
 }
 
 // Fixtures shared across the listing tests.
@@ -362,9 +345,8 @@ func TestSnapshotListAndPurgePrintSizesLikeRemoteInfo(t *testing.T) {
 	// Without --force and with nothing on stdin the purge lists what it
 	// would delete, reads no confirmation, and deletes nothing.
 	env.stdout.Reset()
-	require.ErrorIs(t, env.v.PurgeSnapshotsWithOptions(
-		&vaultik.SnapshotPurgeOptions{OlderThan: "1d"}),
-		vaultik.ErrNotConfirmed)
+	require.NoError(t, env.v.PurgeSnapshotsWithOptions(
+		&vaultik.SnapshotPurgeOptions{OlderThan: "1d"}))
 	assert.Contains(t, findTableRow(t, env.stdout.String(), listLocalID),
 		wantSize, "snapshot purge")
 }
@@ -809,17 +791,9 @@ func TestListSnapshots_JSONReportsTruncation(t *testing.T) {
 
 	timestamp := time.Date(2026, 3, 2, 11, 22, 33, 0, time.UTC)
 
-	// One past the cap, so exactly one snapshot is omitted. The listing
-	// names a snapshot by the key its manifest is stored under, never by
-	// the manifest's own ID. So all of them share one encoded manifest:
-	// encoding 1001 took seconds.
-	data := encodeRemoteManifest(t,
-		snapshot.RemoteSnapshotKey("otherhost_bulk"),
-		timestamp.Format(time.RFC3339))
-
+	// One past the cap, so exactly one snapshot is omitted.
 	for i := range maxRemoteOnlyRowsForTest + 1 {
-		env.putRemoteManifest(t, snapshot.RemoteSnapshotKey(
-			fmt.Sprintf("otherhost_bulk_%04d", i)), data)
+		env.addRemote(t, fmt.Sprintf("otherhost_bulk_%04d", i), timestamp)
 	}
 
 	stderr := captureProcessStderr(t, func() {

internal/vaultik/storage_bind.go

@@ -13,30 +13,35 @@ import (
 var errStorageBindingMismatch = errors.New(
 	"local index is bound to a different backup destination")
 
-// checkStorageBinding refuses when the local index database is bound to
-// a different destination than the configured one, with an error that
-// tells the user how to recover (revert the config, or run `vaultik
-// database delete`). Every mutating command calls it before touching
-// either the local index or the destination store, because the two live
-// in lockstep: the local index records which chunks/blobs already exist
-// at the destination, and a mismatched pair produces silent corruption
-// (the scanner sees "known" chunks and skips uploads, then writes
-// snapshots whose manifests reference blobs that aren't on the new
-// destination).
+// EnsureStorageBinding guarantees that the local index database is
+// bound to the currently-configured storage destination. Every mutating
+// command must call this before touching either the local index or the
+// destination store, because the two live in lockstep: the local index
+// records which chunks/blobs already exist at the destination, and
+// mismatched destination + local index produces silent corruption (the
+// scanner sees "known" chunks and skips uploads, then writes snapshots
+// whose manifests reference blobs that aren't on the new destination).
 //
-// It records nothing. Only a backup binds the local index, through
-// EnsureStorageBinding, and it does so before adding anything to it. An
-// unbound local index therefore holds nothing from any destination.
+// 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
+//     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
+//     delete` to discard the local index and rebuild against the new
+//     destination on the next backup).
 //
 // Read-only inspection commands (remote info, snapshot list, etc.)
 // deliberately don't call this: they can be run against a bare
 // destination store without any binding state.
-func (v *Vaultik) checkStorageBinding() error {
+func (v *Vaultik) EnsureStorageBinding() error {
 	if v.Repositories == nil || v.Config == nil {
 		// NewForTesting builds a Vaultik with no DB or config;
-		// there's no binding to check. Callers exercising the bind
-		// path use a real DB and populate Config explicitly (see
-		// storage_bind_test.go).
+		// there's nothing to bind and no binding to check. Callers
+		// exercising the bind path use a real DB and populate Config
+		// explicitly (see storage_bind_test.go).
 		return nil
 	}
 
@@ -45,24 +50,6 @@ func (v *Vaultik) checkStorageBinding() error {
 		return err
 	}
 
-	if stored == "" || stored == configured {
-		return nil
-	}
-
-	return fmt.Errorf("%w\n%s",
-		errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
-}
-
-// EnsureStorageBinding records the configured destination (storage_url,
-// or the s3.* endpoint, bucket and prefix) as the binding of an unbound
-// local index. It refuses as checkStorageBinding does when the local
-// index is bound to a different destination.
-func (v *Vaultik) EnsureStorageBinding() error {
-	stored, configured, err := v.storageBinding()
-	if err != nil {
-		return err
-	}
-
 	if stored == "" {
 		err = v.Repositories.LocalMeta.Set(
 			v.ctx, database.LocalMetaKeyStorageURL, configured)
@@ -75,12 +62,12 @@ func (v *Vaultik) EnsureStorageBinding() error {
 		return nil
 	}
 
-	if stored != configured {
-		return fmt.Errorf("%w\n%s",
-			errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
+	if stored == configured {
+		return nil
 	}
 
-	return nil
+	return fmt.Errorf("%w\n%s",
+		errStorageBindingMismatch, buildBindingMismatchMessage(stored, configured))
 }
 
 // storageBinding returns the destination the local index is bound to

internal/vaultik/storage_bind_test.go

@@ -2,7 +2,6 @@ package vaultik_test
 
 import (
 	"context"
-	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -10,126 +9,9 @@ import (
 	"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/vaultik"
 )
 
-// storageBindingOf returns the destination the local index is bound to,
-// or "" when it is not bound.
-func storageBindingOf(
-	ctx context.Context, t *testing.T, repos *database.Repositories,
-) string {
-	t.Helper()
-
-	got, err := repos.LocalMeta.Get(ctx, database.LocalMetaKeyStorageURL)
-	require.NoError(t, err)
-
-	return got
-}
-
-// TestFailedFirstBackupLeavesIndexUnbound checks that a first backup
-// the destination refuses, as a mistyped bucket would, leaves the local
-// index unbound, so a backup with the corrected destination succeeds.
-func TestFailedFirstBackupLeavesIndexUnbound(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	storeDir := t.TempDir()
-	v, repos := newFileDestinationVaultik(ctx, t, storeDir)
-	working := v.Storage
-
-	refusing := faultstore.New(working)
-	refusing.OnPut = func(string) faultstore.PutAction { return faultstore.PutFail }
-	refusing.OnStat = func(string) bool { return true }
-	v.Storage = refusing
-	v.Config.StorageURL = "file://" + filepath.Join(t.TempDir(), "typo")
-
-	require.ErrorIs(t, backUp(v, "first"), faultstore.ErrInjectedUpload)
-	assert.Empty(t, storageBindingOf(ctx, t, repos))
-
-	v.Storage = working
-	v.Config.StorageURL = "file://" + storeDir
-
-	require.NoError(t, backUp(v, "first"))
-	assert.Equal(t, "file://"+storeDir, storageBindingOf(ctx, t, repos))
-}
-
-// TestBackupToOtherDestinationWritesNoMarker checks that a backup
-// refuses a local index bound to a different destination before it
-// writes the marker to the configured one.
-func TestBackupToOtherDestinationWritesNoMarker(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	firstDir := t.TempDir()
-	v, _ := newFileDestinationVaultik(ctx, t, firstDir)
-	v.Config.StorageURL = "file://" + firstDir
-
-	require.NoError(t, backUp(v, "first"))
-
-	secondDir := t.TempDir()
-	secondStore, err := storage.NewFileStorer(secondDir)
-	require.NoError(t, err)
-
-	v.Storage = secondStore
-	v.Config.StorageURL = "file://" + secondDir
-
-	require.ErrorContains(t, backUp(v, "first"),
-		"local index is bound to a different backup destination")
-	assert.NoFileExists(t, filepath.Join(secondDir, markerFile))
-}
-
-// TestCommandsOtherThanBackupLeaveIndexUnbound checks that commands
-// other than a backup do not bind an unbound local index, whether or not
-// they fail. Each runs on a fresh local index.
-func TestCommandsOtherThanBackupLeaveIndexUnbound(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		run  func(v *vaultik.Vaultik) error
-	}{
-		{"prune", func(v *vaultik.Vaultik) error {
-			return v.Prune(&vaultik.PruneOptions{Force: true})
-		}},
-		{"snapshot purge", func(v *vaultik.Vaultik) error {
-			return v.PurgeSnapshotsWithOptions(
-				&vaultik.SnapshotPurgeOptions{KeepLatest: true, Force: true})
-		}},
-		{"snapshot remove", func(v *vaultik.Vaultik) error {
-			_, err := v.RemoveSnapshot("host_first_2026-01-02T03:04:05Z",
-				&vaultik.RemoveOptions{Force: true})
-
-			return err
-		}},
-		{"snapshot remove --all", func(v *vaultik.Vaultik) error {
-			_, err := v.RemoveAllSnapshots(&vaultik.RemoveOptions{Force: true})
-
-			return err
-		}},
-		{"remote nuke", func(v *vaultik.Vaultik) error {
-			return v.NukeRemote(true)
-		}},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			ctx := context.Background()
-			v, repos := newFileDestinationVaultik(ctx, t, t.TempDir())
-
-			_ = tt.run(v)
-
-			assert.Empty(t, storageBindingOf(ctx, t, repos))
-		})
-	}
-}
-
 // buildBindTestVaultik returns a minimal Vaultik wired with a real
 // in-memory DB and a config carrying the given StorageURL. Enough to
 // exercise EnsureStorageBinding without spinning up storage or fx.

internal/vaultik/unknown_snapshot_test.go

@@ -1,62 +0,0 @@
-package vaultik_test
-
-import (
-	"bytes"
-	"context"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/config"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestUnknownSnapshotID checks that restore and verify, given an ID that
-// names no snapshot in the local index or on the destination store, say
-// so in the words `snapshot remove` uses and print nothing else.
-func TestUnknownSnapshotID(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	const unknownID = "host1_home_2026-01-01T00:00:00Z"
-
-	commands := map[string]func(v *vaultik.Vaultik) error{
-		"restore": func(v *vaultik.Vaultik) error {
-			return v.Restore(&vaultik.RestoreOptions{
-				SnapshotID: unknownID,
-				TargetDir:  t.TempDir(),
-			})
-		},
-		"verify": func(v *vaultik.Vaultik) error {
-			return v.VerifySnapshotWithOptions(unknownID, &vaultik.VerifyOptions{})
-		},
-		"verify --deep": func(v *vaultik.Vaultik) error {
-			return v.VerifySnapshotWithOptions(unknownID,
-				&vaultik.VerifyOptions{Deep: true})
-		},
-	}
-
-	for name, run := range commands {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			store := newTestStorer()
-			addManifest(t, store, "snapshot-001", nil)
-
-			// A backup wrote to this destination, so it carries the marker.
-			require.NoError(t, store.Put(context.Background(), markerFile,
-				bytes.NewReader(nil)))
-
-			tv := vaultik.NewForTesting(store)
-			tv.Config = &config.Config{AgeSecretKey: testAgeSecretKey}
-			tv.Repositories = newLocalIndex(t, "snapshot-001")
-
-			err := run(tv.Vaultik)
-
-			require.EqualError(t, err, "snapshot not found in the local "+
-				"index or on the destination store: "+unknownID)
-			assert.Empty(t, tv.Stdout.String())
-		})
-	}
-}

internal/vaultik/vaultik.go

@@ -5,7 +5,6 @@ package vaultik
 import (
 	"bytes"
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -185,12 +184,6 @@ func (v *Vaultik) printlnStdout(args ...any) {
 	_, _ = fmt.Fprintln(v.Stdout, args...)
 }
 
-// ErrNotConfirmed is returned when a confirmation prompt gets any answer
-// but the one it asks for (`y` or `Y`, or `yes` for `database delete`), or
-// stdin ends first. Nothing the prompt asked about is deleted, and the
-// command exits non-zero.
-var ErrNotConfirmed = errors.New("deletion not confirmed")
-
 // scanStdin reads a line of input from stdin.
 func (v *Vaultik) scanStdin(a ...any) (int, error) {
 	return fmt.Fscanln(v.Stdin, a...)

internal/vaultik/verify.go

@@ -120,17 +120,12 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
 
 	log.Info("Starting snapshot verification", "snapshot_id", snapshotID, "mode", "deep")
 
-	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
-	if err != nil {
-		return v.deepVerifyFailure(result, opts, err.Error(), err)
-	}
-
 	if !opts.JSON && !v.UI.Quiet() {
 		v.stdoutf("Deep verification of snapshot: %s\n\n", snapshotID)
 	}
 
 	manifest, tempDB, dbBlobs, err := v.loadVerificationData(
-		snapshotID, remoteKey, opts, result, identities)
+		snapshotID, opts, result, identities)
 	if err != nil {
 		return err
 	}
@@ -175,13 +170,21 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
 	return nil
 }
 
-// loadVerificationData downloads manifest, database, and blob list for
-// verification of the snapshot at remoteKey, which
-// resolveSnapshotRemoteKey returned for snapshotID.
+// loadVerificationData downloads manifest, database, and blob list for verification
 func (v *Vaultik) loadVerificationData(
-	snapshotID, remoteKey string, opts *VerifyOptions, result *VerifyResult,
+	snapshotID string, opts *VerifyOptions, result *VerifyResult,
 	identities []age.Identity,
 ) (*snapshot.Manifest, *tempDB, []snapshot.BlobInfo, error) {
+	// Resolve the identifier to the snapshot's remote key. A human ID is
+	// hashed; a remote key (or its abbreviation, as printed for a
+	// remote-only snapshot) is used as-is, so a host with no local index
+	// can verify a snapshot it can only see on the store.
+	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
+	if err != nil {
+		return nil, nil, nil, v.deepVerifyFailure(result, opts,
+			fmt.Sprintf("resolving snapshot identifier: %v", err), err)
+	}
+
 	// Download manifest. downloadManifestByKey is the single reader for
 	// remote manifests; see its doc comment.
 	log.Info("Downloading manifest", "remote_key", remoteKey)