Bind the local index only after a backup writes the marker (closes #420)

A first backup that failed before writing anything, on a missing bucket, a refused credential or missing recipients, still bound the local index to the configured destination, so correcting a mistyped storage_url was refused afterwards. A backup now records the binding only after it has written the marker file at the destination. prune, snapshot purge, snapshot remove and remote nuke only refuse a different binding and leave an unbound index unbound.

Model: opus-5-5

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
-	// EnsureStorageBinding).
+	// checkStorageBinding).
 	if v.Config != nil {
 		stored, configured, err := v.storageBinding()
 		if err != nil {
@@ -83,11 +83,19 @@ func (v *Vaultik) checkDestinationMarker() error {
 	return nil
 }
 
-// markDestination checks the backup destination store with
+// markDestination refuses a local index bound to a different
-// checkDestinationMarker and then writes the marker, so a first backup
+// destination, checks the backup destination store for its marker,
-// marks its destination and a later one writes the same text again.
+// 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.
 func (v *Vaultik) markDestination() error {
-	err := v.checkDestinationMarker()
+	err := v.checkStorageBinding()
+	if err != nil {
+		return err
+	}
+
+	err = v.checkDestinationMarker()
 	if err != nil {
 		return err
 	}
@@ -99,5 +107,5 @@ func (v *Vaultik) markDestination() error {
 			destinationMarkerKey, err)
 	}
 
-	return nil
+	return v.EnsureStorageBinding()
 }

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.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	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.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	if err != nil {
 		return err
 	}

internal/vaultik/snapshot.go

@@ -115,14 +115,9 @@ 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
 	}
@@ -749,7 +744,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.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -1277,7 +1272,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.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	if err != nil {
 		return err
 	}
@@ -1467,7 +1462,7 @@ func (v *Vaultik) RemoveSnapshot(
 		SnapshotID: snapshotID,
 	}
 
-	err := v.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	if err != nil {
 		return result, err
 	}
@@ -1707,7 +1702,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.EnsureStorageBinding()
+	err := v.checkStorageBinding()
 	if err != nil {
 		return nil, err
 	}

internal/vaultik/storage_bind.go

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

internal/vaultik/storage_bind_test.go

@@ -2,6 +2,7 @@ package vaultik_test
 
 import (
 	"context"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -9,9 +10,126 @@ 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.