From 3d0cd2658c9577984c0dc5fae1d3f4420657ff9f Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Sep 2026 12:01:49 +0000 Subject: [PATCH] Check blob sizes and the database in shallow verify (closes #169) Shallow `snapshot verify` only checked that each blob object existed and then reported "All blobs verified", overstating what it did. It now compares each blob's stored size against the manifest's compressed_size, using the same comparison as the deep path, and checks that the snapshot's encrypted database (db.zst.age) is present. A blob of the wrong size no longer counts as verified. The final line reports only what was checked: presence and size, not contents. The README verify description and the CLI short/long text are corrected to match. Removed the now-unused resolveAndDownloadManifest helper and errBlobsMissing sentinel. Model: opus-4-8 --- README.md | 6 +- internal/cli/snapshot.go | 7 +- internal/vaultik/shallow_verify_test.go | 191 ++++++++++++++++++++++++ internal/vaultik/snapshot.go | 133 ++++++++++++----- internal/vaultik/snapshot_identifier.go | 14 -- internal/vaultik/verify.go | 23 +-- 6 files changed, 311 insertions(+), 63 deletions(-) create mode 100644 internal/vaultik/shallow_verify_test.go diff --git a/README.md b/README.md index fa5e3d4..a7d26c2 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ Requirements that no existing tool meets: ## daily use ```sh -# verify a snapshot (shallow: checks all blobs exist) +# verify a snapshot (shallow: checks all blobs are present with the listed size) vaultik snapshot verify # deep verify (downloads and cryptographically verifies every blob) @@ -312,7 +312,9 @@ local index alone, and still exits zero. logger, so stdout stays a single parseable document. **`snapshot verify`**: Verify snapshot integrity. -* Default (shallow): checks that all blobs referenced in the manifest exist in storage +* Default (shallow): checks that every blob the manifest lists is present in + storage with the size the manifest records, and that the encrypted database is + present. It does not read blob contents. * `--deep`: Downloads and decrypts each blob, verifies chunk hashes against the encrypted metadata database * Accepts the same identifiers as `snapshot restore`: a snapshot ID, or a diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index fe32d60..2184e23 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -188,8 +188,11 @@ func newSnapshotVerifyCommand() *cobra.Command { cmd := &cobra.Command{ Use: "verify ", - Short: "Verify snapshot integrity", - Long: "Verifies that all blobs referenced in a snapshot exist.\n\n" + + Short: "Check a snapshot's blobs are present with the listed size", + Long: "Checks that every blob the snapshot's manifest lists is present\n" + + "in storage with the size the manifest records, and that the\n" + + "snapshot's encrypted database is present. It does not read blob\n" + + "contents; use --deep to download and cryptographically verify them.\n\n" + "The snapshot may be named by its ID or, on a host with no local\n" + "index, by the remote key that 'snapshot list' prints for a\n" + "remote-only snapshot (an unambiguous leading part is enough).", diff --git a/internal/vaultik/shallow_verify_test.go b/internal/vaultik/shallow_verify_test.go new file mode 100644 index 0000000..e4b56fd --- /dev/null +++ b/internal/vaultik/shallow_verify_test.go @@ -0,0 +1,191 @@ +package vaultik_test + +import ( + "bytes" + "context" + "encoding/json" + "io" + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestShallowVerifyDetectsWrongBlobSize backs up a real snapshot, runs +// shallow verify (which passes and reports exactly the blobs it checked), +// then grows one stored blob so its size no longer matches the manifest. +// Shallow verify must then fail, count the grown blob as a size mismatch, +// and drop it from the verified count rather than continuing to report it +// as checked. +func TestShallowVerifyDetectsWrongBlobSize(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + + dataDir := filepath.Join(tempDir, "source") + storeDir := filepath.Join(tempDir, "remote") + dbPath := filepath.Join(tempDir, "index.sqlite") + + chunkSize := int64(32 * 1024) + maxBlobSize := int64(128 * 1024) + + // Enough data to span several blobs, so the mismatch count and the + // dropped verified count are both meaningful. + require.NoError(t, fs.MkdirAll(dataDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(dataDir, "data.bin"), + bytesPattern("shallow-", int(maxBlobSize*4)), 0o644)) + + ctx := context.Background() + + cfg, storer, snapshotID := runFileStorageBackup( + ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize) + + newVerifier := func(out io.Writer) *vaultik.Vaultik { + v := &vaultik.Vaultik{ + Config: cfg, + Storage: storer, + Fs: fs, + Stdout: out, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + v.SetContext(ctx) + + return v + } + + var out bytes.Buffer + + require.NoError(t, + newVerifier(&out).VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{JSON: true}), + "shallow verify should pass on a healthy snapshot") + + healthy := decodeVerifyResult(t, out.Bytes()) + require.Equal(t, "ok", healthy.Status) + require.Positive(t, healthy.BlobCount) + require.Equal(t, healthy.BlobCount, healthy.Verified, + "shallow verify must report exactly the blobs it checked") + require.Zero(t, healthy.Mismatched) + + // Grow one stored blob so its size no longer matches the manifest. + growOneBlob(t, fs, filepath.Join(storeDir, "blobs")) + + out.Reset() + err := newVerifier(&out).VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{JSON: true}) + require.Error(t, err, + "shallow verify must fail when a blob's stored size differs from the manifest") + + bad := decodeVerifyResult(t, out.Bytes()) + require.Equal(t, "failed", bad.Status) + require.Equal(t, 1, bad.Mismatched) + require.Equal(t, healthy.BlobCount-1, bad.Verified, + "the wrong-sized blob must not be counted as verified") +} + +// TestShallowVerifyDetectsMissingDatabase backs up a real snapshot, +// confirms shallow verify passes, then deletes the snapshot's encrypted +// database. Shallow verify must fail: a snapshot without its database is +// not restorable, even when every blob is present. +func TestShallowVerifyDetectsMissingDatabase(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + + dataDir := filepath.Join(tempDir, "source") + storeDir := filepath.Join(tempDir, "remote") + dbPath := filepath.Join(tempDir, "index.sqlite") + + chunkSize := int64(32 * 1024) + maxBlobSize := int64(128 * 1024) + + require.NoError(t, fs.MkdirAll(dataDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(dataDir, "data.bin"), + bytesPattern("shallow-db-", int(maxBlobSize*2)), 0o644)) + + ctx := context.Background() + + cfg, storer, snapshotID := runFileStorageBackup( + ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize) + + newVerifier := func() *vaultik.Vaultik { + v := &vaultik.Vaultik{ + Config: cfg, + Storage: storer, + Fs: fs, + Stdout: io.Discard, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + v.SetContext(ctx) + + return v + } + + require.NoError(t, + newVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{}), + "shallow verify should pass on a healthy snapshot") + + // The database lives under the hashed remote key, not the human ID. + dbObject := filepath.Join(storeDir, "metadata", + snapshot.RemoteSnapshotKey(snapshotID), "db.zst.age") + require.NoError(t, os.Remove(dbObject)) + + require.Error(t, + newVerifier().VerifySnapshotWithOptions( + snapshotID, &vaultik.VerifyOptions{}), + "shallow verify must fail when db.zst.age is absent") +} + +// growOneBlob appends bytes to the first blob file found under blobsDir, +// changing its on-disk size so it no longer matches the manifest. +func growOneBlob(t *testing.T, fs afero.Fs, blobsDir string) { + t.Helper() + + var blobPath string + + err := afero.Walk(fs, blobsDir, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if blobPath == "" && !info.IsDir() { + blobPath = path + } + + return nil + }) + require.NoError(t, err) + require.NotEmpty(t, blobPath, "expected at least one blob on disk") + + data, err := afero.ReadFile(fs, blobPath) + require.NoError(t, err) + + data = append(data, []byte("extra")...) + require.NoError(t, afero.WriteFile(fs, blobPath, data, 0o644)) +} + +func decodeVerifyResult(t *testing.T, b []byte) vaultik.VerifyResult { + t.Helper() + + var result vaultik.VerifyResult + + require.NoError(t, json.Unmarshal(b, &result)) + + return result +} diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 9582463..bd2586c 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -20,7 +20,6 @@ import ( var ( errSnapshotNotInConfig = errors.New("snapshot not found in config") errNoSnapshotsInConfig = errors.New("no snapshots configured") - errBlobsMissing = errors.New("blobs are missing") errSnapshotVerifyFailed = errors.New("verification failed") errRemoveAllNeedsForce = errors.New("--all requires --force") errInvalidTableName = errors.New("invalid table name") @@ -670,11 +669,24 @@ func (v *Vaultik) VerifySnapshotWithOptions( v.printVerifyHeader(snapshotID, opts) - // Resolve the identifier to the snapshot's remote key and download the - // manifest. 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. - manifest, err := v.resolveAndDownloadManifest(snapshotID) + // 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. + remoteKey, err := v.resolveSnapshotRemoteKey(snapshotID) + if err != nil { + if opts.JSON { + result.Status = verifyStatusFailed + result.ErrorMessage = fmt.Sprintf("resolving snapshot identifier: %v", err) + + return v.outputVerifyJSON(result) + } + + return fmt.Errorf("resolving snapshot identifier: %w", err) + } + + manifest, err := v.downloadManifestByKey(remoteKey) if err != nil { if opts.JSON { result.Status = verifyStatusFailed @@ -704,14 +716,24 @@ func (v *Vaultik) VerifySnapshotWithOptions( v.printlnStdout() - // Check each blob exists - v.stdoutf("Checking blob existence...\n") + // Check each blob is present with the size the manifest records. + v.stdoutf("Checking blob presence and sizes...\n") } - result.Verified, result.Missing, result.MissingSize = - v.verifyManifestBlobsExist(manifest, opts) + // A snapshot is only restorable if its encrypted database is present + // alongside the blobs. Shallow verify checks that the object exists; it + // does not decrypt it (that is deep verify's job). + dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey) - return v.formatVerifyResult(result, manifest, opts) + _, dbErr := v.Storage.Stat(v.ctx, dbPath) + if dbErr != nil { + result.DatabaseMissing = true + } + + result.Verified, result.Missing, result.Mismatched, result.MissingSize = + v.verifyManifestBlobs(manifest, opts) + + return v.formatVerifyResult(result, opts) } // printVerifyHeader prints the snapshot ID and parsed timestamp for @@ -736,25 +758,27 @@ func (v *Vaultik) printVerifyHeader(snapshotID string, opts *VerifyOptions) { } } -// verifyManifestBlobsExist checks that each blob in the manifest exists -// in storage, returning the verified count, missing count, and total -// missing bytes. -func (v *Vaultik) verifyManifestBlobsExist( +// verifyManifestBlobs checks that each blob in the manifest is present in +// storage with the size the manifest records, returning the counts of +// blobs that were present with the right size, absent, and present but the +// wrong size, plus the total bytes of the absent blobs. It does not read +// blob contents; deep verification (RunDeepVerify) does that. The size +// comparison matches the deep path (see verifyBlobExistenceFromDB). +func (v *Vaultik) verifyManifestBlobs( manifest *snapshot.Manifest, opts *VerifyOptions, -) (int, int, int64) { +) (int, int, int, int64) { var ( - verified, missing int - missingSize int64 + verified, missing, mismatched int + missingSize int64 ) for _, blob := range manifest.Blobs { blobPath := fmt.Sprintf("blobs/%s/%s/%s", blob.Hash[:2], blob.Hash[2:4], blob.Hash) - // Shallow: check existence only (deep verification is handled - // by RunDeepVerify). - _, err := v.Storage.Stat(v.ctx, blobPath) - if err != nil { + stat, err := v.Storage.Stat(v.ctx, blobPath) + switch { + case err != nil: if !opts.JSON { v.stdoutf(" Missing: %s (%s)\n", blob.Hash, ubytes(blob.CompressedSize)) @@ -762,23 +786,32 @@ func (v *Vaultik) verifyManifestBlobsExist( missing++ missingSize += blob.CompressedSize - } else { + case stat.Size != blob.CompressedSize: + if !opts.JSON { + v.stdoutf(" Wrong size: %s (store has %s, manifest lists %s)\n", + blob.Hash, ubytes(stat.Size), ubytes(blob.CompressedSize)) + } + + mismatched++ + default: verified++ } } - return verified, missing, missingSize + return verified, missing, mismatched, missingSize } // formatVerifyResult outputs the final verification results as JSON or // human-readable text. func (v *Vaultik) formatVerifyResult( - result *VerifyResult, manifest *snapshot.Manifest, opts *VerifyOptions, + result *VerifyResult, opts *VerifyOptions, ) error { + failure := shallowVerifyFailure(result) + if opts.JSON { - if result.Missing > 0 { + if failure != "" { result.Status = verifyStatusFailed - result.ErrorMessage = fmt.Sprintf("%d blobs are missing", result.Missing) + result.ErrorMessage = failure } else { result.Status = "ok" } @@ -787,29 +820,57 @@ func (v *Vaultik) formatVerifyResult( } v.stdoutf("\nVerification complete:\n") - v.stdoutf(" Verified: %d blobs (%s)\n", result.Verified, - ubytes(manifest.TotalCompressedSize-result.MissingSize)) + v.stdoutf(" Present with listed size: %d blobs\n", result.Verified) if result.Missing > 0 { - v.stdoutf(" Missing: %d blobs (%s)\n", + v.stdoutf(" Missing: %d blobs (%s)\n", result.Missing, ubytes(result.MissingSize)) - } else { - v.stdoutf(" Missing: 0 blobs\n") + } + + if result.Mismatched > 0 { + v.stdoutf(" Wrong size: %d blobs\n", result.Mismatched) + } + + if result.DatabaseMissing { + v.stdoutf(" Encrypted database: missing\n") } v.stdoutf(" Status: ") - if result.Missing > 0 { - v.stdoutf("FAILED - %d blobs are missing\n", result.Missing) + if failure != "" { + v.stdoutf("FAILED - %s\n", failure) - return fmt.Errorf("%d %w", result.Missing, errBlobsMissing) + return fmt.Errorf("%w: %s", errSnapshotVerifyFailed, failure) } - v.stdoutf("OK - All blobs verified\n") + // Report only what was actually checked: presence and size, not contents. + v.stdoutf("OK - all %d blobs listed in the manifest are present with the "+ + "listed size; contents not checked (use --deep)\n", result.Verified) return nil } +// shallowVerifyFailure returns a human-readable description of everything +// that failed shallow verification, or the empty string if it passed. +func shallowVerifyFailure(result *VerifyResult) string { + var parts []string + + if result.Missing > 0 { + parts = append(parts, fmt.Sprintf("%d blobs are missing", result.Missing)) + } + + if result.Mismatched > 0 { + parts = append(parts, + fmt.Sprintf("%d blobs have the wrong size", result.Mismatched)) + } + + if result.DatabaseMissing { + parts = append(parts, "the encrypted database is missing") + } + + return strings.Join(parts, "; ") +} + // outputVerifyJSON outputs the verification result as JSON func (v *Vaultik) outputVerifyJSON(result *VerifyResult) error { encoder := json.NewEncoder(v.Stdout) diff --git a/internal/vaultik/snapshot_identifier.go b/internal/vaultik/snapshot_identifier.go index 5f3903d..a65cf3f 100644 --- a/internal/vaultik/snapshot_identifier.go +++ b/internal/vaultik/snapshot_identifier.go @@ -69,20 +69,6 @@ func (v *Vaultik) resolveSnapshotRemoteKey(identifier string) (string, error) { } } -// resolveAndDownloadManifest resolves a snapshot identifier to its remote -// key (see resolveSnapshotRemoteKey) and downloads that snapshot's -// manifest. -func (v *Vaultik) resolveAndDownloadManifest( - identifier string, -) (*snapshot.Manifest, error) { - remoteKey, err := v.resolveSnapshotRemoteKey(identifier) - if err != nil { - return nil, err - } - - return v.downloadManifestByKey(remoteKey) -} - // isRemoteKeyOrPrefix reports whether s is a full remote key or the // leading part of one: 1 to 64 lowercase hex characters. A human snapshot // ID is never all hex, so this shape test is enough to tell the two apart. diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index feea3df..818d419 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -47,15 +47,20 @@ type VerifyOptions struct { // //nolint:tagliatelle // snake_case is the established JSON output format type VerifyResult struct { - SnapshotID string `json:"snapshot_id"` - Status string `json:"status"` // "ok" or "failed" - Mode string `json:"mode"` // "shallow" or "deep" - BlobCount int `json:"blob_count"` - TotalSize int64 `json:"total_size"` - Verified int `json:"verified"` - Missing int `json:"missing"` - MissingSize int64 `json:"missing_size,omitempty"` - ErrorMessage string `json:"error,omitempty"` + SnapshotID string `json:"snapshot_id"` + Status string `json:"status"` // "ok" or "failed" + Mode string `json:"mode"` // "shallow" or "deep" + BlobCount int `json:"blob_count"` + TotalSize int64 `json:"total_size"` + Verified int `json:"verified"` + Missing int `json:"missing"` + MissingSize int64 `json:"missing_size,omitempty"` + Mismatched int `json:"mismatched,omitempty"` + // DatabaseMissing is set by shallow verify when the snapshot's + // encrypted database (metadata//db.zst.age) is absent, which + // makes the snapshot unrestorable regardless of the blobs. + DatabaseMissing bool `json:"database_missing,omitempty"` + ErrorMessage string `json:"error,omitempty"` } // deepVerifyFailure records a failure in the result and returns it appropriately