diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index f02caeb..86ee32d 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -128,10 +128,15 @@ 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, identities) + tempDB, tempDir, err := v.downloadSnapshotDB(opts.SnapshotID, remoteKey, identities) if err != nil { return fmt.Errorf("downloading snapshot database: %w", err) } @@ -754,19 +759,12 @@ func (v *Vaultik) handleRestoreVerification( return nil } -// 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. +// downloadSnapshotDB downloads and decrypts the metadata database of the +// snapshot at remoteKey, which resolveSnapshotRemoteKey returned for +// snapshotID. func (v *Vaultik) downloadSnapshotDB( - snapshotID string, identities []age.Identity, + snapshotID, remoteKey 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) diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 663656e..d1e1cb5 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -969,25 +969,22 @@ func (v *Vaultik) VerifySnapshotWithOptions( Mode: "shallow", } - 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. + // 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 = fmt.Sprintf("resolving snapshot identifier: %v", err) + result.ErrorMessage = err.Error() return v.outputVerifyJSON(result) } - return fmt.Errorf("resolving snapshot identifier: %w", err) + return err } + v.printVerifyHeader(snapshotID, opts) + manifest, err := v.downloadManifestByKey(remoteKey) if err != nil { if opts.JSON { @@ -1480,7 +1477,7 @@ func (v *Vaultik) RemoveSnapshot( return result, err } - inLocalIndex, err := v.findSnapshotToRemove(snapshotID, opts) + inLocalIndex, err := v.findSnapshot(snapshotID, opts.LocalOnly) if err != nil { return result, err } @@ -1543,17 +1540,15 @@ func (v *Vaultik) RemoveSnapshot( return result, nil } -// 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) { +// findSnapshot reports whether the local index has a row for +// snapshotID, and returns errSnapshotNotFound when there is 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) findSnapshot(snapshotID string, localOnly bool) (bool, error) { if v.Repositories != nil { snap, err := v.Repositories.Snapshots.GetByID(v.ctx, snapshotID) if err != nil { @@ -1565,7 +1560,7 @@ func (v *Vaultik) findSnapshotToRemove( } } - if opts.LocalOnly { + if localOnly { return false, fmt.Errorf("%w in the local index: %s", errSnapshotNotFound, snapshotID) } diff --git a/internal/vaultik/snapshot_identifier.go b/internal/vaultik/snapshot_identifier.go index a65cf3f..2c12025 100644 --- a/internal/vaultik/snapshot_identifier.go +++ b/internal/vaultik/snapshot_identifier.go @@ -28,8 +28,9 @@ 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; the store is -// not consulted. +// 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 findSnapshot). // - 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 @@ -41,6 +42,11 @@ var ( // RFC3339 timestamp). func (v *Vaultik) resolveSnapshotRemoteKey(identifier string) (string, error) { if !isRemoteKeyOrPrefix(identifier) { + _, err := v.findSnapshot(identifier, false) + if err != nil { + return "", err + } + return snapshot.RemoteSnapshotKey(identifier), nil } diff --git a/internal/vaultik/unknown_snapshot_test.go b/internal/vaultik/unknown_snapshot_test.go new file mode 100644 index 0000000..33246ef --- /dev/null +++ b/internal/vaultik/unknown_snapshot_test.go @@ -0,0 +1,62 @@ +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()) + }) + } +} diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index ba6c2d3..c8f9ff5 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -120,12 +120,17 @@ 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, opts, result, identities) + snapshotID, remoteKey, opts, result, identities) if err != nil { return err } @@ -170,21 +175,13 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error { return nil } -// loadVerificationData downloads manifest, database, and blob list for verification +// loadVerificationData downloads manifest, database, and blob list for +// verification of the snapshot at remoteKey, which +// resolveSnapshotRemoteKey returned for snapshotID. func (v *Vaultik) loadVerificationData( - snapshotID string, opts *VerifyOptions, result *VerifyResult, + snapshotID, remoteKey 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)