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

Every backup writes vaultik-destination.txt at the destination root.
While the local index holds snapshots, a destination without it is
not present: snapshot create refuses before writing, prune and
snapshot purge fail, and snapshot list and snapshot remove act as for
a destination that cannot be listed. snapshot create checks before it
drops incomplete snapshots, snapshot remove before it looks on the
destination. remote nuke deletes the marker last. The file:// backend
never creates its destination directory; that protects a directory on
a removable volume, not a mount point.

Tests back up into directories that exist; fixtures for a written
destination carry the marker.

Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, two blocks moved, unchanged, into
snapshotNamesToCreate and confirmRemoveSnapshot.

Model: opus-5-5

ARCHITECTURE.md

@@ -382,14 +382,14 @@ bucket/
         └── manifest.json.zst        # Blob list (for pruning/verification)
 ```
 
-A destination without `vaultik-destination.txt` is treated as not present
+While the local index holds snapshots, a destination without
-while the local index holds snapshots stored there (`checkDestinationMarker`
+`vaultik-destination.txt` is treated as not present
-in `internal/vaultik/destination_marker.go`): `snapshot create` refuses to
+(`checkDestinationMarker` in `internal/vaultik/destination_marker.go`):
-write to it, `prune` and `snapshot purge` fail, and `snapshot list` and
+`snapshot create` refuses to write to it, and listing it fails as for a
-`snapshot remove` behave as for a destination that cannot be listed. This
+destination that cannot be listed. An unplugged volume's empty mount point
-keeps those commands from taking an unplugged volume's empty mount point for
+is therefore never taken for a destination whose snapshots are all gone.
-a destination whose snapshots are all gone. The `file://` backend never
+The `file://` backend never creates its destination directory, so a write
-creates its destination directory, so a write to a missing one fails.
+to a missing one fails.
 
 The `{remote-key}` directory name is a one-way double SHA-256 hash of the human
 snapshot ID, so the human ID (hostname, snapshot name, timestamp) is never

README.md

@@ -340,9 +340,9 @@ gives: revert the configuration, or run `vaultik database delete`.
 If the destination store cannot be listed (unmounted volume,
 permission denied, network down), the command warns, falls back to the
 local index alone, and still exits zero. It does the same when the
-destination has no `vaultik-destination.txt` while the local index
+local index holds snapshots and the destination has no
-holds snapshots stored there, as when a volume mounted at the
+`vaultik-destination.txt`, as when a volume mounted at the destination
-destination is unplugged and leaves its empty mount point behind.
+is unplugged and leaves its empty mount point behind.
 * `--json`: Output in JSON format. Each entry carries `locally_tracked`
   (whether the snapshot is in the local index), `remote_key` (the full
   64-character storage key), and `remote_present` (whether it was seen
@@ -527,11 +527,11 @@ For an implementation-level view of the internals — the data model, the
 ```
 
 * `vaultik-destination.txt` holds one fixed line of text, with no host
-  name, path or time. A destination without it is treated as not
+  name, path or time. While the local index holds snapshots, a
-  present while the local index holds snapshots stored there:
+  destination without it is treated as not present: `snapshot create`
-  `snapshot create` refuses to write to it, `prune` and `snapshot purge`
+  refuses to write to it, `prune` and `snapshot purge` fail, and
-  fail, and `snapshot list` and `snapshot remove` behave as for a
+  `snapshot list` and `snapshot remove` behave as for a destination that
-  destination that cannot be listed. This keeps an unplugged volume's empty mount point
+  cannot be listed. This keeps an unplugged volume's empty mount point
   from being taken for a destination whose snapshots are all gone. Mount
   the volume if it is removable; if the destination was emptied on
   purpose, run `vaultik database delete` and back up again.

docs/REPOSTRUCTURE.md

@@ -27,7 +27,7 @@ The metadata subdirectory is named with the **remote key**, a one-way hash of th
 
 ## Marker File (`vaultik-destination.txt`)
 
-Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. A destination without it is treated as not present while the local index holds snapshots stored there: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove` and `snapshot list` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
+Every backup writes this file at the root before it uploads anything. It holds one fixed line of plain text and names no host, path or time. While the local index holds snapshots, a destination without it is treated as not present: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove` and `snapshot list` behave as for a destination that cannot be listed. An unplugged volume leaves an empty directory at its mount point, and without the marker that directory would read as a destination whose snapshots are all gone. `remote nuke` deletes the marker last.
 
 ## Blobs Directory (`blobs/`)
 

internal/vaultik/destination_marker.go

@@ -9,10 +9,10 @@ import (
 )
 
 // destinationMarkerKey is the object every backup writes at the root of
-// the backup destination store. While the local index holds a snapshot
+// the backup destination store. While the local index holds a snapshot,
-// stored there, a destination without it is treated as not present: an
+// a destination without it is treated as not present: an unplugged
-// unplugged volume leaves an empty directory at its mount point, which
+// volume leaves an empty directory at its mount point, which would
-// would otherwise read as a store whose snapshots are all gone.
+// otherwise read as a store whose snapshots are all gone.
 const destinationMarkerKey = "vaultik-destination.txt"
 
 // destinationMarkerText is the whole content of the marker. It names no
@@ -20,16 +20,13 @@ const destinationMarkerKey = "vaultik-destination.txt"
 const destinationMarkerText = "This is a vaultik backup destination store.\n"
 
 // errDestinationNotPresent is returned when the local index holds a
-// snapshot stored on the backup destination store and the store has no
+// snapshot and the backup destination store has no marker.
-// marker.
 var errDestinationNotPresent = errors.New("backup destination store is not present")
 
 // checkDestinationMarker returns errDestinationNotPresent when the local
-// index holds a snapshot stored on the backup destination store and the
+// index holds a snapshot and the backup destination store has no marker.
-// store has no marker. With no snapshot in the local index, as on a
+// With no snapshot in the local index, as on a first backup or on
-// first backup or on another machine, there is nothing to check; nor
+// another machine, there is nothing to check.
-// when the local index is bound to a different destination than the
-// configured one, since its snapshots are stored there.
 func (v *Vaultik) checkDestinationMarker() error {
 	ids, err := v.localSnapshotIDs()
 	if err != nil {
@@ -40,19 +37,6 @@ func (v *Vaultik) checkDestinationMarker() error {
 		return nil
 	}
 
-	// Config is nil only in tests, which then record no binding (see
-	// EnsureStorageBinding).
-	if v.Config != nil {
-		stored, configured, err := v.storageBinding()
-		if err != nil {
-			return err
-		}
-
-		if stored != "" && stored != configured {
-			return nil
-		}
-	}
-
 	_, err = v.Storage.Stat(v.ctx, destinationMarkerKey)
 	if errors.Is(err, storage.ErrNotFound) {
 		return fmt.Errorf("%w: %s has no %s, but the local index holds "+

internal/vaultik/destination_marker_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/vaultik/internal/database"
 	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/storage"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
 
@@ -222,44 +221,6 @@ func TestListSnapshotsWarnsWhenDestinationHasNoMarker(t *testing.T) {
 	assert.NotContains(t, out.String(), "not found in backup destination store")
 }
 
-// TestListSnapshotsAfterStorageURLChangeToEmptyDirectory checks that
-// snapshot list, after storage_url is changed to an empty directory,
-// warns only that the local index is bound to a different destination:
-// the local snapshots are stored at the old one, so the new one's lack
-// of a marker says nothing. With --json the local row is not on the new
-// destination, so remote_present is false, not unknown.
-func TestListSnapshotsAfterStorageURLChangeToEmptyDirectory(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	ctx := context.Background()
-	v, repos, out := backUpToFileDestination(ctx, t, t.TempDir())
-	id := localSnapshotID(ctx, t, repos, "first")
-
-	newDir := t.TempDir()
-	store, err := storage.NewFileStorer(newDir)
-	require.NoError(t, err)
-
-	v.Storage = store
-	v.Config.StorageURL = "file://" + newDir
-
-	require.NoError(t, v.ListSnapshots(false))
-
-	assert.Contains(t, out.String(), id)
-	assert.Contains(t, out.String(),
-		"local index is bound to a different backup destination")
-	assert.NotContains(t, out.String(), "Could not list backup destination store")
-	assert.NotContains(t, out.String(), markerFile)
-
-	out.Reset()
-	require.NoError(t, v.ListSnapshots(true))
-
-	rows := decodeListJSON(t, out.String())
-	require.Len(t, rows, 1)
-	require.NotNil(t, rows[0].RemotePresent)
-	assert.False(t, *rows[0].RemotePresent)
-}
-
 // TestBackupAfterNukeRemoteSucceeds checks that remote nuke leaves the
 // destination ready for a new first backup, which writes the marker
 // again.