Leave remote info orphan figures unknown when a manifest is unreadable (closes #228)

When a manifest could not be read, remote info skipped it, counted that
snapshot's blobs as orphaned and advised running prune. The orphan
figures are now unknown in that case: the report says how many
manifests could not be read and gives no prune advice, and --json gives
orphaned_blob_count and orphaned_blob_size as null and lists the remote
keys in unreadable_manifests.

Directory names under metadata/ were used unchecked and printed raw, so
control characters in one reached the terminal. A name that is not a
remote key (64 lowercase hex characters) is now skipped with a warning,
as the snapshot listing already does.

The closing log line of remote info now carries the unreadable manifest
count in place of the orphan count.

Model: opus-5-5

internal/vaultik/info.go

@@ -206,9 +206,14 @@ type RemoteInfoResult struct {
 	ReferencedBlobCount int   `json:"referenced_blob_count"`
 	ReferencedBlobSize  int64 `json:"referenced_blob_size"`
 
-	// Orphaned blobs
-	OrphanedBlobCount int   `json:"orphaned_blob_count"`
-	OrphanedBlobSize  int64 `json:"orphaned_blob_size"`
+	// Orphaned blobs. Both stay nil (null in the JSON) when a manifest
+	// could not be read, since that snapshot's blobs would be counted
+	// as orphaned.
+	OrphanedBlobCount *int   `json:"orphaned_blob_count"`
+	OrphanedBlobSize  *int64 `json:"orphaned_blob_size"`
+
+	// Remote key of each snapshot whose manifest could not be read
+	UnreadableManifests []string `json:"unreadable_manifests,omitempty"`
 }
 
 // RemoteInfo displays information about remote storage
@@ -243,7 +248,9 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error {
 		v.stdoutf("Downloading %d manifest(s)...\n", len(snapshotIDs))
 	}
 
-	referencedBlobs := v.collectReferencedBlobsFromManifests(snapshotIDs, snapshotMetadata)
+	referencedBlobs, unreadableManifests := v.collectReferencedBlobsFromManifests(
+		snapshotIDs, snapshotMetadata)
+	result.UnreadableManifests = unreadableManifests
 
 	v.populateRemoteInfoResult(result, snapshotMetadata, snapshotIDs, referencedBlobs)
 
@@ -256,7 +263,7 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error {
 		"snapshots", result.TotalMetadataCount,
 		"total_blobs", result.TotalBlobCount,
 		"referenced_blobs", result.ReferencedBlobCount,
-		"orphaned_blobs", result.OrphanedBlobCount)
+		"unreadable_manifests", len(result.UnreadableManifests))
 
 	if jsonOutput {
 		enc := json.NewEncoder(v.Stdout)
@@ -292,6 +299,16 @@ func (v *Vaultik) collectSnapshotMetadata() (
 
 		snapshotID := parts[1]
 
+		// The name comes from the destination store, which is not
+		// trusted, and is printed in the report. Accept it only in the
+		// form of a remote key.
+		if !isBlobHash(snapshotID) {
+			log.Warn("Skipping non-conforming key under metadata/",
+				"key", obj.Key)
+
+			continue
+		}
+
 		if _, exists := snapshotMetadata[snapshotID]; !exists {
 			snapshotMetadata[snapshotID] = &SnapshotMetadataInfo{SnapshotID: snapshotID}
 		}
@@ -319,12 +336,15 @@ func (v *Vaultik) collectSnapshotMetadata() (
 }
 
 // collectReferencedBlobsFromManifests downloads manifests and returns
-// referenced blob hashes with sizes.
+// referenced blob hashes with sizes, and the remote keys of the
+// manifests it could not read.
 func (v *Vaultik) collectReferencedBlobsFromManifests(
 	snapshotIDs []string, snapshotMetadata map[string]*SnapshotMetadataInfo,
-) map[string]int64 {
+) (map[string]int64, []string) {
 	referencedBlobs := make(map[string]int64)
 
+	var unreadable []string
+
 	for _, snapshotID := range snapshotIDs {
 		// snapshotIDs here are remote keys, taken straight from the
 		// metadata/ listing. downloadManifestByKey is the single reader
@@ -333,6 +353,8 @@ func (v *Vaultik) collectReferencedBlobsFromManifests(
 		if err != nil {
 			log.Warn("Failed to read manifest", "snapshot", snapshotID, "error", err)
 
+			unreadable = append(unreadable, snapshotID)
+
 			continue
 		}
 
@@ -349,7 +371,7 @@ func (v *Vaultik) collectReferencedBlobsFromManifests(
 		info.BlobsSize = blobsSize
 	}
 
-	return referencedBlobs
+	return referencedBlobs, unreadable
 }
 
 // populateRemoteInfoResult fills in the result's snapshot and
@@ -378,8 +400,9 @@ func (v *Vaultik) populateRemoteInfoResult(
 }
 
 // scanRemoteBlobStorage lists all blobs on remote and computes orphan
-// stats. showText is true only when the human report is being printed
-// (not --json, not --quiet), gating the progress line.
+// stats when every manifest was read. showText is true only when the
+// human report is being printed (not --json, not --quiet), gating the
+// progress line.
 func (v *Vaultik) scanRemoteBlobStorage(
 	result *RemoteInfoResult, referencedBlobs map[string]int64, showText bool,
 ) error {
@@ -406,13 +429,27 @@ func (v *Vaultik) scanRemoteBlobStorage(
 		result.TotalBlobSize += obj.Size
 	}
 
+	// A blob named only by a manifest that could not be read would be
+	// counted as orphaned, so the orphan figures stay unknown.
+	if len(result.UnreadableManifests) > 0 {
+		return nil
+	}
+
+	var (
+		orphanedCount int
+		orphanedSize  int64
+	)
+
 	for hash, size := range allBlobs {
 		if _, referenced := referencedBlobs[hash]; !referenced {
-			result.OrphanedBlobCount++
-			result.OrphanedBlobSize += size
+			orphanedCount++
+			orphanedSize += size
 		}
 	}
 
+	result.OrphanedBlobCount = &orphanedCount
+	result.OrphanedBlobSize = &orphanedSize
+
 	return nil
 }
 
@@ -465,11 +502,20 @@ func (v *Vaultik) printRemoteInfoTable(result *RemoteInfoResult) {
 	v.stdoutf("Referenced by snapshots:    %s (%s)\n",
 		humanize.Comma(int64(result.ReferencedBlobCount)),
 		ubytes(result.ReferencedBlobSize))
-	v.stdoutf("Orphaned (unreferenced):    %s (%s)\n",
-		humanize.Comma(int64(result.OrphanedBlobCount)),
-		ubytes(result.OrphanedBlobSize))
 
-	if result.OrphanedBlobCount > 0 {
+	if result.OrphanedBlobCount == nil {
+		v.stdoutf("Orphaned (unreferenced):    unknown "+
+			"(%d manifest(s) could not be read)\n",
+			len(result.UnreadableManifests))
+
+		return
+	}
+
+	v.stdoutf("Orphaned (unreferenced):    %s (%s)\n",
+		humanize.Comma(int64(*result.OrphanedBlobCount)),
+		ubytes(*result.OrphanedBlobSize))
+
+	if *result.OrphanedBlobCount > 0 {
 		v.stdoutf("\nRun 'vaultik prune' to remove orphaned blobs.\n")
 	}
 }

internal/vaultik/remote_info_test.go

@@ -0,0 +1,88 @@
+package vaultik_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/snapshot"
+)
+
+// TestRemoteInfo_UnreadableManifestLeavesOrphansUnknown checks that a
+// manifest remote info cannot read makes the orphan figures unknown. A
+// blob referenced only by that snapshot would otherwise be counted as
+// orphaned, and the report would advise running prune.
+func TestRemoteInfo_UnreadableManifestLeavesOrphansUnknown(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	const testBlobHashB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" +
+		"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+
+	env := newListEnv(t)
+
+	// The readable manifest references blob A only.
+	env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
+	addBlob(t, env.store.testStorer, testBlobHashA)
+	addBlob(t, env.store.testStorer, testBlobHashB)
+
+	// With every manifest readable, blob B is orphaned.
+	require.NoError(t, env.v.RemoteInfo(true))
+
+	var doc map[string]any
+
+	require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
+	assert.InDelta(t, 1, doc["orphaned_blob_count"], 0)
+
+	// A second snapshot whose manifest cannot be decoded. Blob B may be
+	// one of its blobs.
+	unreadableKey := snapshot.RemoteSnapshotKey(listLocalID)
+	require.NoError(t, env.store.Put(context.Background(),
+		"metadata/"+unreadableKey+"/manifest.json.zst",
+		bytes.NewReader([]byte("not a valid manifest"))))
+
+	env.stdout.Reset()
+	require.NoError(t, env.v.RemoteInfo(false))
+
+	text := env.stdout.String()
+	assert.Contains(t, text,
+		"Orphaned (unreferenced):    unknown (1 manifest(s) could not be read)")
+	assert.NotContains(t, text, "vaultik prune")
+
+	env.stdout.Reset()
+	require.NoError(t, env.v.RemoteInfo(true))
+
+	doc = nil
+	require.NoError(t, json.Unmarshal(env.stdout.Bytes(), &doc))
+	assert.Contains(t, doc, "orphaned_blob_count")
+	assert.Nil(t, doc["orphaned_blob_count"])
+	assert.Contains(t, doc, "orphaned_blob_size")
+	assert.Nil(t, doc["orphaned_blob_size"])
+	assert.Equal(t, []any{unreadableKey}, doc["unreadable_manifests"])
+}
+
+// TestRemoteInfo_SkipsNonConformingMetadataName checks that a directory
+// under metadata/ whose name is not a remote key is left out of the
+// report. The name comes from the destination store; printed raw, its
+// control characters would reach the terminal.
+func TestRemoteInfo_SkipsNonConformingMetadataName(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	env := newListEnv(t)
+	env.addRemote(t, listRemoteID, time.Date(2026, 3, 2, 0, 0, 0, 0, time.UTC))
+	require.NoError(t, env.store.Put(context.Background(),
+		"metadata/\x1b[31mred/manifest.json.zst",
+		bytes.NewReader([]byte("not a valid manifest"))))
+
+	require.NoError(t, env.v.RemoteInfo(false))
+
+	out := env.stdout.String()
+	assert.NotContains(t, out, "\x1b")
+	assert.Contains(t, out, "Total (1 snapshots)")
+}