ARCHITECTURE.md

@@ -312,7 +312,7 @@ Repository interfaces:
 ```
 CreateSnapshot(opts)
     │
-    ├─► markDestination()   // Refuse a destination without the marker, then write it
+    ├─► markDestination()   // Check the marker (see Storage Layout in S3), then write it
     │
     ├─► PruneDatabase()   // Critical: avoid dedup errors
     │
@@ -382,15 +382,20 @@ bucket/
         └── manifest.json.zst        # Blob list (for pruning/verification)
 ```
 
-A destination without `vaultik-destination.txt` is treated as not present
+A destination that answers a look-up of `vaultik-destination.txt` with not
-while the local index holds snapshots stored there (`checkDestinationMarker`
+found is treated as not present while the local index holds snapshots stored
-in `internal/vaultik/destination_marker.go`): `snapshot create` refuses to
+there (`checkDestinationMarker` in `internal/vaultik/destination_marker.go`):
-write to it, `prune`, `snapshot purge` and `remote nuke` fail, and
+`snapshot create` refuses to write to it, `prune`, `snapshot purge` and
-`snapshot list` and `snapshot remove` behave as for a destination that cannot
+`remote nuke` fail, and `snapshot list` and `snapshot remove` behave as for a
-be listed. This keeps those commands from taking an unplugged volume's empty
+destination that cannot be listed. This keeps those commands from taking an
-mount point for a destination whose snapshots are all gone. The `file://`
+unplugged volume's empty mount point for a destination whose snapshots are
-backend never creates its destination directory, so a write to a missing one
+all gone. When the look-up fails for any reason other than not found, for
-fails.
+example when S3 refuses it to credentials that allow only PUT, the check is
+skipped, so a host with such credentials does not notice an emptied
+destination; see
+[docs/REPOSTRUCTURE.md](docs/REPOSTRUCTURE.md#marker-file-vaultik-destinationtxt).
+The `file://` backend never creates its destination directory, so a write 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

@@ -332,7 +332,9 @@ value where a list belongs or a word where a number belongs), is refused.
   command fails before the backup starts.
 * Fails before it writes anything when a `file://` destination directory
   does not exist, or when the local index holds snapshots and the
-  destination has no `vaultik-destination.txt` (see
+  destination answers that it has no `vaultik-destination.txt`. When that
+  look-up fails for any reason other than not found, for example when S3
+  refuses it to credentials that allow only PUT, the check is skipped (see
   [remote storage layout](#remote-storage-layout))
 
 **`snapshot list`**: Show every snapshot known to this host — the union
@@ -368,9 +370,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
+destination answers that it has no `vaultik-destination.txt` while the
-holds snapshots stored there, as when a volume mounted at the
+local index holds snapshots stored there, as when a volume mounted at
-destination is unplugged and leaves its empty mount point behind.
+the destination 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
@@ -413,8 +415,8 @@ with `--local-only`, the snapshot must be in the local index. Local row
 cleanup (files, chunks, blobs the snapshot was the last referrer for)
 runs automatically. If removing the snapshot's metadata from the
 destination store fails (for example because the store is unreachable,
-or the local index holds snapshots and the destination has no
+or the local index holds snapshots and the destination answers that it
-`vaultik-destination.txt`), the local-DB removal still completes and a
+has no `vaultik-destination.txt`), the local-DB removal still completes and a
 warning is emitted; run `vaultik snapshot remove <snapshot-id>` again
 once the store is reachable to remove the snapshot's metadata from it
 (`vaultik prune` does not). For a snapshot that is only on the
@@ -457,9 +459,9 @@ be deleted. Step (3) reads all remote manifests — network cost scales with
 the number of snapshots.
 `snapshot create --prune` runs the same cleanup automatically; this is the
 manual entry point for the same work. If the destination store cannot
-be listed, or the local index holds snapshots and the destination has
+be listed, or the local index holds snapshots and the destination answers
-no `vaultik-destination.txt` (an unplugged volume's empty mount point),
+that it has no `vaultik-destination.txt` (an unplugged volume's empty mount
-prune fails and deletes nothing. `snapshot purge` does the same. If a
+point), prune fails and deletes nothing. `snapshot purge` does the same. If a
 blob cannot be deleted, the command (and `snapshot create --prune`)
 exits non-zero.
 * `--force`: Skip confirmation prompt
@@ -494,8 +496,8 @@ and irreversible. This is the single supported way to wipe the entire
 destination store. A snapshot is removed from the local index only once
 its metadata is deleted from the destination store. If the destination
 store cannot be listed, or the local index holds snapshots and the
-destination has no `vaultik-destination.txt` (an unplugged volume's
+destination answers that it has no `vaultik-destination.txt` (an unplugged
-empty mount point), the command fails before it removes anything. If a
+volume's empty mount point), the command fails before it removes anything. If a
 listing afterwards still finds anything under `metadata/` or `blobs/`,
 the command names each object left and exits non-zero; running it again
 deletes what is left.
@@ -577,15 +579,22 @@ 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 stored
-  present while the local index holds snapshots stored there:
+  there, a destination that answers a look-up of it with not found is
-  `snapshot create` refuses to write to it, `prune`, `snapshot purge`
+  treated as not present: `snapshot create` refuses to write to it,
-  and `remote nuke` fail, and `snapshot list` and `snapshot remove`
+  `prune`, `snapshot purge` and `remote nuke` fail, and `snapshot list`
-  behave as for a destination that cannot be listed. This keeps an
+  and `snapshot remove` behave as for a destination that cannot be
-  unplugged volume's empty mount point from being taken for a
+  listed. This keeps an unplugged volume's empty mount point from being
-  destination whose snapshots are all gone. Mount
+  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.
+  When the look-up fails for any reason other than not found, the check
+  is skipped and the command goes on. S3 refuses it on every run to
+  credentials that allow only PUT, so a host with such credentials does
+  not notice a destination emptied since its last backup, and its next
+  backup would leave out the data its local index records as already
+  stored. After the destination is emptied, run
+  `vaultik database delete` on such a host before it backs up again.
 * Blobs are two-level directory sharded using the first 4 hex chars of the blob hash
 * `db.zst.age` is a binary SQLite database (zstd compressed, age encrypted)
   containing all file metadata, chunk mappings, and relationships for the snapshot

docs/REPOSTRUCTURE.md

@@ -27,7 +27,9 @@ 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`, `snapshot list` and `remote nuke` 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 stored there, a destination that answers a look-up of the marker with not found is treated as not present: `snapshot create` refuses to write to it, and `prune`, `snapshot purge`, `snapshot remove`, `snapshot list` and `remote nuke` 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.
+
+When the look-up of the marker fails for any reason other than not found, the check is skipped and the command goes on. S3 refuses it on every run to credentials that allow only PUT, so a host with such credentials does not notice a destination emptied since its last backup, and its next backup would leave out the data its local index records as already stored. After the destination is emptied, run `vaultik database delete` on such a host before it backs up again.
 
 ## Blobs Directory (`blobs/`)
 

internal/cli/entry_prune_json_test.go

@@ -134,7 +134,7 @@ func writeHermeticPruneConfig(t *testing.T, seedStale bool) string {
 		seedStaleSnapshotRecord(t, indexPath)
 
 		// While the local index holds a snapshot, prune fails on a
-		// destination without the marker a backup writes.
+		// destination that answers that it has no marker.
 		require.NoError(t, os.WriteFile(
 			filepath.Join(storeDir, "vaultik-destination.txt"), nil, 0o600))
 	}

internal/cli/entry_snapshot_purge_test.go

@@ -42,7 +42,7 @@ func TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(t *testing.T)
 	seedStaleSnapshotRecord(t, indexPath)
 
 	// While the local index holds a snapshot, purge fails on a
-	// destination without the marker a backup writes.
+	// destination that answers that it has no marker.
 	require.NoError(t, os.WriteFile(
 		filepath.Join(dir, "store", "vaultik-destination.txt"), nil, 0o600))
 

internal/storage/faultstore/faultstore.go

@@ -2,15 +2,17 @@
 // faults on demand, so tests can reproduce the failure modes a real
 // backend exhibits: an upload that fails partway, a backend that reports
 // success while storing nothing, reads that return corrupt or truncated
-// bytes, and a delete the backend refuses. It is the seam called for by
+// bytes, and a delete or a stat the backend refuses. It is the seam
-// the fault-injection tests (sneak/vaultik issue 72) and is meant to be
+// called for by the fault-injection tests (sneak/vaultik issue 72) and
-// reused by future tests rather than re-implemented per case.
+// is meant to be reused by future tests rather than re-implemented per
+// case.
 //
-// The wrapper delegates every method to the inner Storer. Three hooks
+// The wrapper delegates every method to the inner Storer. Four hooks
 // change that: OnPut decides the fate of each write, OnGet decides how
-// each read's bytes are returned, and OnDelete decides whether each
+// each read's bytes are returned, OnDelete decides whether each delete
-// delete fails. All are keyed by the object key, so a test can fault
+// fails, and OnStat whether each stat fails. All are keyed by the object
-// only blobs, only metadata, or a single object.
+// key, so a test can fault only blobs, only metadata, or a single
+// object.
 package faultstore
 
 import (
@@ -30,6 +32,11 @@ var ErrInjectedUpload = errors.New("faultstore: injected upload failure")
 // fail.
 var ErrInjectedDelete = errors.New("faultstore: injected delete failure")
 
+// ErrInjectedStat is returned by a Stat the OnStat hook chose to fail.
+// It is not storage.ErrNotFound: the backend refused the request, as S3
+// refuses a HEAD to credentials that allow only PUT.
+var ErrInjectedStat = errors.New("faultstore: injected stat failure")
+
 // PutAction is the disposition OnPut assigns to a write.
 type PutAction int
 
@@ -76,6 +83,10 @@ type Storer struct {
 	// object key. Returning true fails the delete and leaves the object
 	// in place.
 	OnDelete func(key string) bool
+
+	// OnStat, when set, is consulted before every Stat with the object
+	// key. Returning true fails the stat with ErrInjectedStat.
+	OnStat func(key string) bool
 }
 
 // New wraps inner. inner must be non-nil.
@@ -159,8 +170,12 @@ func damage(fault GetFault, data []byte) []byte {
 	return out
 }
 
-// Stat delegates unchanged.
+// Stat returns the object's info unless OnStat fails the stat.
 func (f *Storer) Stat(ctx context.Context, key string) (*storage.ObjectInfo, error) {
+	if f.OnStat != nil && f.OnStat(key) {
+		return nil, fmt.Errorf("%w for %q", ErrInjectedStat, key)
+	}
+
 	return f.inner.Stat(ctx, key)
 }
 

internal/vaultik/destination_marker.go

@@ -5,14 +5,16 @@ import (
 	"fmt"
 	"strings"
 
+	"sneak.berlin/go/vaultik/internal/log"
 	"sneak.berlin/go/vaultik/internal/storage"
 )
 
 // destinationMarkerKey is the object every backup writes at the root of
 // the backup destination store. While the local index holds a snapshot
-// stored there, a destination without it is treated as not present: an
+// stored there, a destination that answers a look-up of it with not
-// unplugged volume leaves an empty directory at its mount point, which
+// found is treated as not present: an unplugged volume leaves an empty
-// would otherwise read as a store whose snapshots are all gone.
+// directory at its mount point, which would 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 +22,24 @@ 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 stored on the backup destination store and the store answers
-// marker.
+// that it has no 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
-// store has no marker. With no snapshot in the local index, as on a
+// store answers that it has no marker. With no snapshot in the local
-// first backup or on another machine, there is nothing to check; nor
+// index, as on a first backup or on another machine, there is nothing to
-// when the local index is bound to a different destination than the
+// check; nor when the local index is bound to a different destination
-// configured one, since its snapshots are stored there.
+// than the configured one, since its snapshots are stored there.
+//
+// A look-up of the marker that fails for any other reason than not
+// found is logged and otherwise ignored, because the check could not be
+// made: S3 refuses it to credentials that allow only PUT, which is all
+// a backup needs. The check exists for a file:// volume's empty mount
+// point, which always answers not found, and a destination that cannot
+// be reached fails on the next request made to it, such as the backup's
+// PUT of the marker.
 func (v *Vaultik) checkDestinationMarker() error {
 	ids, err := v.localSnapshotIDs()
 	if err != nil {
@@ -63,8 +73,11 @@ func (v *Vaultik) checkDestinationMarker() error {
 			destinationMarkerKey)
 	}
 
+	// Info, not a warning: --cron must stay silent on a successful
+	// backup from a host whose credentials allow only PUT.
 	if err != nil {
-		return fmt.Errorf("checking for %s: %w", destinationMarkerKey, err)
+		log.Info("Could not check the backup destination store for its marker",
+			"key", destinationMarkerKey, "error", err)
 	}
 
 	return nil

internal/vaultik/destination_marker_test.go

@@ -13,15 +13,16 @@ import (
 	"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"
 )
 
 // These tests cover https://git.eeqj.de/sneak/vaultik/issues/239: a
 // file:// destination at a volume's mount point leaves an empty directory
 // behind when the volume is unplugged. While the local index holds
-// snapshots, a destination without vaultik-destination.txt is not present,
+// snapshots, a destination that answers that it has no
-// so no command may write to it or conclude from it that the local
+// vaultik-destination.txt is not present, so no command may write to it
-// snapshots are gone.
+// or conclude from it that the local snapshots are gone.
 
 // markerFile is the name of the file a backup writes at the root of the
 // destination.
@@ -93,6 +94,39 @@ func TestBackupRefusesDestinationWithoutMarker(t *testing.T) {
 	assertUntouched(ctx, t, repos, storeDir)
 }
 
+// TestBackupGoesOnWhenMarkerCannotBeChecked checks that a backup goes on
+// and writes the marker when looking it up fails with an error other
+// than not found, as S3 answers credentials that allow only PUT
+// (https://git.eeqj.de/sneak/vaultik/issues/389), and that a marker
+// that is not found still stops it.
+func TestBackupGoesOnWhenMarkerCannotBeChecked(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	ctx := context.Background()
+	storeDir := t.TempDir()
+	v, repos, _ := backUpToFileDestination(ctx, t, storeDir)
+
+	// Removed so the test sees the backup write it again: with the
+	// look-up failing, the backup cannot tell that it is gone.
+	markerPath := filepath.Join(storeDir, markerFile)
+	require.NoError(t, os.Remove(markerPath))
+
+	store := faultstore.New(v.Storage)
+	store.OnStat = func(key string) bool { return key == markerFile }
+	v.Storage = store
+
+	require.NoError(t, backUp(v, "second"))
+	localSnapshotID(ctx, t, repos, "second")
+	assert.FileExists(t, markerPath)
+
+	require.NoError(t, os.Remove(markerPath))
+
+	store.OnStat = nil
+
+	require.ErrorContains(t, backUp(v, "third"), markerFile)
+}
+
 // TestBackupChecksMarkerBeforeDroppingIncompleteSnapshots checks that
 // snapshot create writes nothing when the local index holds only an
 // incomplete snapshot and the destination has no marker. The backup drops

internal/vaultik/prune.go

@@ -58,8 +58,8 @@ func (v *Vaultik) NukeRemote(force bool) error {
 
 	// Checked before anything is deleted: running nuke again would fail
 	// the same way, so the advice below must not follow these errors.
-	// The listing also fails on a destination without the marker while
+	// The listing also fails on a destination that answers that it has
-	// the local index holds snapshots.
+	// no marker while the local index holds snapshots.
 	err := v.EnsureStorageBinding()
 	if err != nil {
 		return err