Report blobs shallow verify could not check apart from missing ones (closes #318)

Shallow snapshot verify counted every error from checking a blob or the
encrypted database as "missing" and printed the error nowhere, so a
permission error or an S3 403 read as data loss. Now only a not-found
answer from storage counts as missing. Any other error is reported as
could not be checked, with the error storage returned, in the text
report and under --json (the new unchecked list and
database_check_error field), and the run fails. Under --quiet, which
drops the report, each of those errors is logged as a warning on
stderr.

The database check moved into its own small function because the verify
function was at the linter's length limit.

Model: opus-5-5

internal/blob/packer.go

@@ -245,6 +245,17 @@ func (p *Packer) Flush(ctx context.Context) error {
 	return nil
 }
 
+// DiscardCurrentBlob closes and removes the temp file of the blob being
+// packed, if any, and drops that blob. A caller that stops before Flush
+// must call it, or the temp file is left behind. Thread-safe.
+func (p *Packer) DiscardCurrentBlob() {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+
+	p.cleanupTempFile()
+	p.currentBlob = nil
+}
+
 // FinalizeBlob finalizes the current blob being assembled.
 // This compresses the accumulated chunks, encrypts the result, and computes
 // the content-addressed hash. The finalized blob is either passed to the

internal/snapshot/scanner.go

@@ -1295,6 +1295,11 @@ func (s *Scanner) batchAddFilesToSnapshot(
 func (s *Scanner) processPhase(
 	ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult,
 ) error {
+	// On an error or cancellation the blob being packed is never
+	// finalized, so its temp file must be removed here. After a
+	// successful flush there is no such blob and this does nothing.
+	defer s.packer.DiscardCurrentBlob()
+
 	// Calculate total bytes to process
 	var totalBytes int64
 	for _, f := range filesToProcess {

internal/snapshot/skip_errors_test.go

@@ -381,3 +381,60 @@ func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
 		})
 	}
 }
+
+// TestScannerRemovesBlobTempFileWhenRunStops checks that a run stopped
+// part-way, by a file it cannot read or by a cancellation, removes the
+// temp file of the blob it was packing.
+func TestScannerRemovesBlobTempFileWhenRunStops(t *testing.T) {
+	t.Parallel()
+
+	// a.txt is backed up first and starts a blob; the run stops at b.txt.
+	const target = "/source/b.txt"
+
+	tests := []struct {
+		name  string
+		newFs func(cancel context.CancelFunc) afero.Fs
+	}{
+		{
+			name: "file cannot be read",
+			newFs: func(context.CancelFunc) afero.Fs {
+				return &readFailFs{Fs: afero.NewMemMapFs(), target: target}
+			},
+		},
+		{
+			name: "cancelled",
+			newFs: func(cancel context.CancelFunc) afero.Fs {
+				return &cancelOnOpenFs{
+					Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			fs := tt.newFs(cancel)
+			writeSkipErrorTestFile(t, fs, "/source/a.txt", "first file content")
+			writeSkipErrorTestFile(t, fs, target, "second file content")
+
+			_, err := runSkipErrorScan(ctx, t, fs, "/source", false, nil)
+			if err == nil {
+				t.Fatal("expected the scan to stop with an error, got nil")
+			}
+
+			left, err := afero.Glob(fs, filepath.Join(os.TempDir(), "vaultik-blob-*"))
+			if err != nil {
+				t.Fatalf("listing blob temp files: %v", err)
+			}
+
+			if len(left) != 0 {
+				t.Fatalf("expected no blob temp file left, got %v", left)
+			}
+		})
+	}
+}

internal/vaultik/shallow_verify_test.go

@@ -268,6 +268,81 @@ func TestShallowVerifyReportsStatErrorsApartFromMissing(t *testing.T) {
 	require.Contains(t, text, "FAILED")
 }
 
+// TestShallowVerifyQuietFailsOnStatErrorAlone backs up a healthy snapshot,
+// then makes storage fail Stat, with an error other than not-found, for
+// one blob alone and then for the encrypted database alone. Each run must
+// fail verify, and under --quiet, which drops the report, the error
+// storage returned must still reach stderr.
+//
+//nolint:paralleltest // captureProcessStderr replaces os.Stderr
+func TestShallowVerifyQuietFailsOnStatErrorAlone(t *testing.T) {
+	log.Initialize(log.Config{})
+
+	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-quiet-", int(maxBlobSize*2)), 0o644))
+
+	ctx := context.Background()
+
+	cfg, storer, snapshotID := runFileStorageBackup(
+		ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
+
+	newQuietVerifier := func(s storage.Storer) *vaultik.Vaultik {
+		v := &vaultik.Vaultik{
+			Config:  cfg,
+			Storage: s,
+			Fs:      fs,
+			Stdout:  io.Discard,
+			Stderr:  io.Discard,
+			UI:      ui.NewWithColor(io.Discard, false),
+		}
+		v.SetContext(ctx)
+		v.UI.SetQuiet(true)
+
+		return v
+	}
+
+	require.NoError(t,
+		newQuietVerifier(storer).VerifySnapshotWithOptions(
+			snapshotID, &vaultik.VerifyOptions{}),
+		"shallow verify should pass on a healthy snapshot")
+
+	blobKeys := storedBlobKeys(t, storeDir)
+	require.NotEmpty(t, blobKeys)
+
+	dbKey := "metadata/" + snapshot.RemoteSnapshotKey(snapshotID) + "/db.zst.age"
+
+	for _, failKey := range []string{blobKeys[0], dbKey} {
+		v := newQuietVerifier(statFailingStorer{
+			Storer:   storer,
+			failKeys: []string{failKey},
+		})
+
+		var err error
+
+		stderr := captureProcessStderr(t, func() {
+			err = v.VerifySnapshotWithOptions(
+				snapshotID, &vaultik.VerifyOptions{})
+		})
+
+		require.Error(t, err,
+			"verify must fail when %s could not be checked", failKey)
+		require.Contains(t, stderr, errStatDenied.Error(),
+			"under --quiet the error for %s must still reach stderr", failKey)
+	}
+}
+
 // storedBlobKeys returns the storage key of every blob file under
 // storeDir, sorted.
 func storedBlobKeys(t *testing.T, storeDir string) []string {

internal/vaultik/snapshot.go

@@ -911,9 +911,19 @@ func (v *Vaultik) formatVerifyResult(
 		return v.outputVerifyJSON(result)
 	}
 
-	// Under --quiet a failure is still returned, and the cli layer
+	// --quiet drops the report but not the errors storage returned, which
-	// prints it on stderr.
+	// are logged on stderr. A failure is still returned either way, and
-	if !v.UI.Quiet() {
+	// the cli layer prints it on stderr.
+	if v.UI.Quiet() {
+		for _, blob := range result.Unchecked {
+			log.Warn("Could not check blob", "blob", blob.Hash, "error", blob.Error)
+		}
+
+		if result.DatabaseCheckError != "" {
+			log.Warn("Could not check the encrypted database",
+				"snapshot", result.SnapshotID, "error", result.DatabaseCheckError)
+		}
+	} else {
 		v.printVerifySummary(result, failure)
 	}