snapshot remove accepts a remote-only snapshot's remote key (closes #400)

A host with no local index could remove a remote-only snapshot only by
its plain snapshot ID, which it does not have. snapshot remove passed
its argument through RemoteSnapshotKey, rehashing a remote key so it
never matched. It now resolves the identifier with the same
resolveSnapshotRemoteKey that verify and restore use, so a remote key
or an unambiguous leading part of it works.

Model: deepseek-v4-pro

README.md

@@ -446,6 +446,9 @@ 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
 destination store, that failure is an error. To wipe everything on the
 destination in one go, use `vaultik remote nuke --force`.
+* Accepts a snapshot ID, or — for a snapshot only on the destination
+  store — its remote key (or an unambiguous leading part of it) as shown
+  by `snapshot list`.
 * `--local-only`: Skip remote cleanup; only touch the local index
 * `--dry-run`: Show what would be deleted without deleting
 * `--force`: Skip confirmation prompt

internal/cli/snapshot.go

@@ -288,6 +288,10 @@ know about), which is what 'vaultik prune' does. The command prints the
 'vaultik prune' invocation to run as a follow-up only when it removed the
 snapshot's metadata from the destination store, and never under --json.
 
+The snapshot may be named by its ID or, on a host with no local index, by
+the remote key that 'snapshot list' prints for a remote-only snapshot (an
+unambiguous leading part is enough).
+
 An ID that names no snapshot in the local index or on the destination
 store is an error.
 

internal/vaultik/remove_snapshot_test.go

@@ -451,6 +451,100 @@ func TestRemoveSnapshot_NotInLocalIndex(t *testing.T) {
 	assert.Contains(t, out, "Removed snapshot metadata from remote storage")
 }
 
+// TestRemoveSnapshot_RemoteOnlyByRemoteKey checks that a snapshot only
+// on the destination store, on a host with no local index, is removed by
+// the remote key that 'snapshot list' prints for such a snapshot, and by
+// an unambiguous leading part of it — the same identifiers verify and
+// restore accept. Snapshot remove used to hash the argument again, so
+// the hashed key never matched and only the plain snapshot ID worked.
+func TestRemoveSnapshot_RemoteOnlyByRemoteKey(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	identifiers := map[string]string{
+		"full key":     remoteKey,
+		"leading part": remoteKey[:8],
+	}
+
+	for name, identifier := range identifiers {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			store := newTestStorer()
+			addManifest(t, store, "snapshot-001", nil)
+
+			// No local index: a host that sees the snapshot only remotely.
+			tv := vaultik.NewForTesting(store)
+			tv.Repositories = newLocalIndex(t)
+
+			result, err := tv.RemoveSnapshot(identifier,
+				&vaultik.RemoveOptions{Force: true})
+
+			require.NoError(t, err)
+			assert.True(t, result.RemoteRemoved)
+			assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
+
+			out := tv.Stdout.String()
+			assert.NotContains(t, out, "from local database")
+			assert.Contains(t, out, "Removed snapshot metadata from remote storage")
+		})
+	}
+}
+
+// TestRemoveSnapshot_LocalIndexByRemoteKey checks that a remote key (or
+// its unambiguous leading part) that names a snapshot the local index
+// holds removes that snapshot's local row and its remote metadata
+// together. `snapshot list --json` prints a remote key for every
+// snapshot, including local ones.
+func TestRemoveSnapshot_LocalIndexByRemoteKey(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	remoteKey := snapshot.RemoteSnapshotKey("snapshot-001")
+
+	identifiers := map[string]string{
+		"full key":     remoteKey,
+		"leading part": remoteKey[:8],
+	}
+
+	for name, identifier := range identifiers {
+		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, and the local index holds the snapshot.
+			require.NoError(t, store.Put(context.Background(), markerFile,
+				bytes.NewReader(nil)))
+
+			tv := vaultik.NewForTesting(store)
+			repos := newLocalIndex(t, "snapshot-001")
+			tv.Repositories = repos
+
+			result, err := tv.RemoveSnapshot(identifier,
+				&vaultik.RemoveOptions{Force: true})
+
+			require.NoError(t, err)
+			assert.True(t, result.RemoteRemoved)
+			assert.False(t, store.hasKey(remoteKeyPath("snapshot-001")))
+
+			row, err := repos.Snapshots.GetByID(
+				context.Background(), "snapshot-001")
+			require.NoError(t, err)
+			assert.Nil(t, row)
+
+			out := tv.Stdout.String()
+			assert.Contains(t, out,
+				"Removed snapshot 'snapshot-001' from local database")
+			assert.Contains(t, out, "Removed snapshot metadata from remote storage")
+		})
+	}
+}
+
 // TestRemoveSnapshot_NotInLocalIndexDeleteFails checks that removing a
 // snapshot only the destination store holds fails when its metadata
 // cannot be deleted there, under --force and under --json: nothing was

internal/vaultik/restore.go

@@ -31,9 +31,10 @@ var (
 	errPathsNotInSnapshot    = errors.New("requested path(s) not in the snapshot")
 	errFilesFailedVerify     = errors.New("files failed verification")
 	errDecryptionKeyRequired = errors.New(
-		"age secret key is missing; set it in the " +
+		"decryption key required for restore\n\n" +
-			"VAULTIK_AGE_SECRET_KEY environment variable or the " +
+			"Set the VAULTIK_AGE_SECRET_KEY environment variable to your " +
-			"age_secret_key config file field")
+			"age private key file:\n" +
+			"  export VAULTIK_AGE_SECRET_KEY=\"$(cat vaultik_backup_private_key.txt)\"")
 	// errInvalidAgeSecretKey is returned when the configured key does not
 	// parse as any age identity. It names the source but never the value,
 	// which is secret, so the message is safe to print and log.

internal/vaultik/restore_identity_test.go

@@ -106,23 +106,3 @@ func TestRestoreIdentitiesMissingKey(t *testing.T) {
 	_, err := v.restoreIdentities()
 	require.ErrorIs(t, err, errDecryptionKeyRequired)
 }
-
-// TestMissingKeyErrorsNameBothSources asserts restore and deep verify
-// return the same missing-key error, whose text names both places the key
-// can be set: the VAULTIK_AGE_SECRET_KEY environment variable first since
-// it is preferred, then the age_secret_key config file field.
-func TestMissingKeyErrorsNameBothSources(t *testing.T) {
-	t.Parallel()
-
-	const want = "age secret key is missing; set it in " +
-		"the VAULTIK_AGE_SECRET_KEY environment variable or the " +
-		"age_secret_key config file field"
-
-	v := &Vaultik{Config: &config.Config{}}
-
-	_, err := v.restoreIdentities()
-	require.Equal(t, want, err.Error())
-
-	err = v.RunDeepVerify("any-snapshot", &VerifyOptions{})
-	require.Equal(t, want, err.Error())
-}

internal/vaultik/snapshot.go

@@ -1480,7 +1480,7 @@ func (v *Vaultik) RemoveSnapshot(
 		return result, err
 	}
 
-	inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts)
+	inLocalIndex, localID, remoteKey, err := v.findSnapshotToRemove(snapshotID, opts)
 	if err != nil {
 		return result, err
 	}
@@ -1502,16 +1502,16 @@ func (v *Vaultik) RemoveSnapshot(
 	}
 
 	if inLocalIndex {
-		log.Info("Removing snapshot from local database", "snapshot_id", snapshotID)
+		log.Info("Removing snapshot from local database", "snapshot_id", localID)
 
-		err = v.deleteSnapshotFromLocalDB(snapshotID)
+		err = v.deleteSnapshotFromLocalDB(localID)
 		if err != nil {
 			return result, fmt.Errorf("removing from local database: %w", err)
 		}
 	}
 
 	result.RemoteRemoved, err = v.removeSnapshotRemote(
-		snapshotID, inLocalIndex, markerErr, opts)
+		snapshotID, remoteKey, inLocalIndex, markerErr, opts)
 	if err != nil {
 		return result, err
 	}
@@ -1530,7 +1530,7 @@ func (v *Vaultik) RemoveSnapshot(
 
 	if inLocalIndex {
 		v.UI.Infof("Removed snapshot '%s' from local database.",
-			v.UI.Snapshot(snapshotID))
+			v.UI.Snapshot(localID))
 	}
 
 	if result.RemoteRemoved {
@@ -1543,50 +1543,68 @@ func (v *Vaultik) RemoveSnapshot(
 	return result, nil
 }
 
-// findSnapshotToRemove reports whether the local index has a row for
+// findSnapshotToRemove resolves snapshotID, the identifier remove was
-// snapshotID, and returns errSnapshotNotFound when there is nothing to
+// given, and returns whether the local index holds that snapshot, the
-// remove: no row and, unless LocalOnly is set, no metadata on the
+// human ID of its row when it does, and the remote key to strip from
-// destination store either. The destination store is consulted only
+// the destination store. A human ID is looked up in the local index
-// when the local index has no row, so a snapshot in the local index can
+// literally and hashed without consulting the store; a remote key, or
-// still be removed from it while the destination store is unreachable,
+// an unambiguous leading part of one, is resolved against the store's
-// and only after checkDestinationMarker passes, so an unplugged volume's
+// metadata listing the way verify and restore do, and then matched back
-// empty mount point is not taken for a store without the snapshot.
+// to the local row whose hashed ID equals it, so removing by such a key
+// removes the local row too. The destination store is consulted only
+// after checkDestinationMarker, so an unplugged volume's empty mount
+// point is not taken for a store without the snapshot. The identifier
+// naming nothing, on the local index or, unless LocalOnly is set, on
+// the store, is errSnapshotNotFound; a remote-key form that matches
+// nothing or more than one directory surfaces
+// resolveSnapshotRemoteKey's errSnapshotKeyNotFound or
+// errSnapshotKeyAmbiguous.
 func (v *Vaultik) findSnapshotToRemove(
 	snapshotID string, opts *RemoveOptions,
-) (bool, error) {
+) (bool, string, string, error) {
 	if v.Repositories != nil {
 		snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID)
 		if err != nil {
-			return false, fmt.Errorf("looking up snapshot in local index: %w", err)
+			return false, "", "", fmt.Errorf("looking up snapshot in local index: %w", err)
 		}
 
 		if snap != nil {
-			return true, nil
+			return true, snapshotID, snapshot.RemoteSnapshotKey(snapshotID), nil
 		}
 	}
 
 	if opts.LocalOnly {
-		return false, fmt.Errorf("%w in the local index: %s",
+		return false, "", "", fmt.Errorf("%w in the local index: %s",
 			errSnapshotNotFound, snapshotID)
 	}
 
 	err := v.checkDestinationMarker()
 	if err != nil {
-		return false, err
+		return false, "", "", err
 	}
 
-	objects, err := v.remoteSnapshotObjects(snapshot.RemoteSnapshotKey(snapshotID))
+	remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID)
 	if err != nil {
-		return false, fmt.Errorf("looking up snapshot on destination store: %w", err)
+		return false, "", "", err
+	}
+
+	objects, err := v.remoteSnapshotObjects(remoteKey)
+	if err != nil {
+		return false, "", "", fmt.Errorf("looking up snapshot on destination store: %w", err)
 	}
 
 	if len(objects) == 0 {
-		return false, fmt.Errorf(
+		return false, "", "", fmt.Errorf(
 			"%w in the local index or on the destination store: %s",
 			errSnapshotNotFound, snapshotID)
 	}
 
-	return false, nil
+	localID, err := v.localSnapshotIDForRemoteKey(remoteKey)
+	if err != nil {
+		return false, "", "", err
+	}
+
+	return localID != "", localID, remoteKey, nil
 }
 
 // removeSnapshotDryRun reports what RemoveSnapshot would do without
@@ -1649,19 +1667,20 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
 }
 
 // removeSnapshotRemote strips the snapshot's metadata from the
-// destination store. It returns true only when the delete finished
+// destination store. remoteKey is the resolved metadata directory name
-// without an error and removed at least one object; under LocalOnly it
+// to delete, as returned by findSnapshotToRemove. It returns true only
-// does nothing. markerErr is the result of checkDestinationMarker; when
+// when the delete finished without an error and removed at least one
-// it is set, nothing is deleted and it is the failure. When the snapshot
+// object; under LocalOnly it does nothing. markerErr is the result of
-// is only on the destination store, a failure is returned as an error,
+// checkDestinationMarker; when it is set, nothing is deleted and it is
-// because that removal is all the command does. When the snapshot was in
+// the failure. When the snapshot is only on the destination store, a
-// the local index, its removal there has already happened, so a failure
+// failure is returned as an error, because that removal is all the
-// is only a warning telling the user to run `vaultik snapshot remove`
+// command does. When the snapshot was in the local index, its removal
-// for the snapshot again once the destination store is reachable
+// there has already happened, so a failure is only a warning telling
-// (`vaultik prune` never removes snapshot metadata that has its
+// the user to run `vaultik snapshot remove` for the snapshot again once
-// manifest).
+// the destination store is reachable (`vaultik prune` never removes
+// snapshot metadata that has its manifest).
 func (v *Vaultik) removeSnapshotRemote(
-	snapshotID string, inLocalIndex bool, markerErr error,
+	snapshotID, remoteKey string, inLocalIndex bool, markerErr error,
 	opts *RemoveOptions,
 ) (bool, error) {
 	if opts.LocalOnly {
@@ -1675,8 +1694,7 @@ func (v *Vaultik) removeSnapshotRemote(
 
 	err := markerErr
 	if err == nil {
-		deleted, err = v.deleteRemoteSnapshotByKey(
+		deleted, err = v.deleteRemoteSnapshotByKey(remoteKey)
-			snapshot.RemoteSnapshotKey(snapshotID))
 	}
 
 	if err != nil && !inLocalIndex {
@@ -1790,6 +1808,23 @@ func (v *Vaultik) localSnapshotIDs() ([]string, error) {
 	return ids, nil
 }
 
+// localSnapshotIDForRemoteKey returns the human snapshot ID of the local
+// index row whose RemoteSnapshotKey is key, or "" when no row has it.
+func (v *Vaultik) localSnapshotIDForRemoteKey(key string) (string, error) {
+	ids, err := v.localSnapshotIDs()
+	if err != nil {
+		return "", fmt.Errorf("listing local snapshots: %w", err)
+	}
+
+	for _, id := range ids {
+		if snapshot.RemoteSnapshotKey(id) == key {
+			return id, nil
+		}
+	}
+
+	return "", nil
+}
+
 // listAllRemoteSnapshotKeys collects the hashed remote keys
 // (subdirectories under metadata/) currently present in the
 // destination store. Returns (nil, err) when the store cannot be

internal/vaultik/verify.go

@@ -23,6 +23,8 @@ import (
 // Sentinel errors for snapshot verification failures.
 var (
 	errVerificationFailed = errors.New("verification failed")
+	errSecretKeyRequired  = errors.New(
+		"VAULTIK_AGE_SECRET_KEY not set; required for deep verification")
 	errChunksOutOfOrder    = errors.New("chunks out of order")
 	errChunkHashMismatch   = errors.New("chunk hash mismatch")
 	errNegativeChunkLength = errors.New("chunk length is negative")
@@ -104,9 +106,13 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
 		Mode:       "deep",
 	}
 
+	if !v.CanDecrypt() {
+		return v.deepVerifyFailure(result, opts,
+			errSecretKeyRequired.Error(), errSecretKeyRequired)
+	}
+
 	// Parse the age secret key once, the same way restore does, and reuse
-	// the identities for the database and every blob. A missing key fails
+	// the identities for the database and every blob.
-	// here, before anything is downloaded.
 	identities, err := v.restoreIdentities()
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)