From 167adf8eb1561e69c200578173f1e1eab9fa39cc Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 7 Oct 2026 04:36:04 +0000 Subject: [PATCH] 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, with no prune advice; --json gives them as null and lists the unreadable remote keys in unreadable_manifests. Names under metadata/ were used unchecked and printed raw. A name that is not a remote key is now skipped with a warning. Its manifest is then not read either, so a skipped name also leaves the orphan figures unknown; --json counts such names in skipped_metadata_names without printing them. The closing log line carries the unreadable manifest count in place of the orphan count. Model: opus-5-5 --- README.md | 7 +- TODO.md | 14 ++++ internal/vaultik/info.go | 101 +++++++++++++++++++----- internal/vaultik/remote_info_test.go | 114 +++++++++++++++++++++++++++ 4 files changed, 214 insertions(+), 22 deletions(-) create mode 100644 internal/vaultik/remote_info_test.go diff --git a/README.md b/README.md index 72097c3..2f405bb 100644 --- a/README.md +++ b/README.md @@ -390,7 +390,12 @@ recipients, and local database statistics. **`remote info`**: Show storage backend type and location plus detailed remote storage inventory: per-snapshot metadata sizes, blob counts, and -orphaned blob detection. +orphaned blob detection. A name under `metadata/` that is not a remote +key is skipped with a warning and is not printed. If a manifest cannot be +read, or a name was skipped, the orphaned blob figures are reported as +unknown; `--json` gives them as `null`, lists the remote key of each +unreadable manifest in `unreadable_manifests` and counts the skipped +names in `skipped_metadata_names`. * `--json`: Output as JSON **`remote nuke`**: Delete every snapshot's metadata and every blob from the diff --git a/TODO.md b/TODO.md index 3383609..1b402d9 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,20 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-07: Made `remote info` stop reporting a snapshot's blobs as + orphaned when its manifest cannot be read, and stop printing raw + names from under `metadata/` + ([issue #228](https://git.eeqj.de/sneak/vaultik/issues/228)). A + manifest it failed to read was skipped, so that snapshot's blobs were + counted as orphaned and the report advised running `vaultik prune`. + The orphan figures are now unknown in that case, with no prune + advice, and `--json` gives them as `null` with the unreadable remote + keys in `unreadable_manifests`. A name under `metadata/` that is not + 64 lowercase hex characters is now skipped with a warning instead of + being printed, control characters included. Its manifest is then not + read either, so a skipped name also leaves the orphan figures unknown, + and `--json` counts such names in `skipped_metadata_names`. + - 2026-10-07: Made a backup notice a file rewritten with its size unchanged and a new mtime in the same second as the one in the index ([issue #226](https://git.eeqj.de/sneak/vaultik/issues/226)). The diff --git a/internal/vaultik/info.go b/internal/vaultik/info.go index 53e6d44..8ca2958 100644 --- a/internal/vaultik/info.go +++ b/internal/vaultik/info.go @@ -206,9 +206,19 @@ 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 or a name under metadata/ was skipped, 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"` + + // Number of names under metadata/ skipped because they are not + // remote keys. The names themselves are not reported: they come + // from the destination store and may hold control characters. + SkippedMetadataNames int `json:"skipped_metadata_names,omitempty"` } // RemoteInfo displays information about remote storage @@ -234,16 +244,20 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { v.stdoutf("Scanning snapshot metadata...\n") } - snapshotMetadata, snapshotIDs, err := v.collectSnapshotMetadata() + snapshotMetadata, snapshotIDs, skippedNames, err := v.collectSnapshotMetadata() if err != nil { return err } + result.SkippedMetadataNames = skippedNames + if showText { 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 +270,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) @@ -273,16 +287,18 @@ func (v *Vaultik) RemoteInfo(jsonOutput bool) error { } // collectSnapshotMetadata scans remote metadata and returns -// per-snapshot info and sorted IDs. +// per-snapshot info, sorted IDs and the number of names it skipped +// because they are not remote keys. func (v *Vaultik) collectSnapshotMetadata() ( - map[string]*SnapshotMetadataInfo, []string, error, + map[string]*SnapshotMetadataInfo, []string, int, error, ) { snapshotMetadata := make(map[string]*SnapshotMetadataInfo) + skippedNames := make(map[string]bool) metadataCh := v.Storage.ListStream(v.ctx, "metadata/") for obj := range metadataCh { if obj.Err != nil { - return nil, nil, fmt.Errorf("listing metadata: %w", obj.Err) + return nil, nil, 0, fmt.Errorf("listing metadata: %w", obj.Err) } parts := strings.Split(obj.Key, "/") @@ -292,6 +308,18 @@ 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) + + skippedNames[snapshotID] = true + + continue + } + if _, exists := snapshotMetadata[snapshotID]; !exists { snapshotMetadata[snapshotID] = &SnapshotMetadataInfo{SnapshotID: snapshotID} } @@ -315,16 +343,19 @@ func (v *Vaultik) collectSnapshotMetadata() ( sort.Strings(snapshotIDs) - return snapshotMetadata, snapshotIDs, nil + return snapshotMetadata, snapshotIDs, len(skippedNames), nil } // 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 +364,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 +382,7 @@ func (v *Vaultik) collectReferencedBlobsFromManifests( info.BlobsSize = blobsSize } - return referencedBlobs + return referencedBlobs, unreadable } // populateRemoteInfoResult fills in the result's snapshot and @@ -378,8 +411,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 and no name under metadata/ was +// skipped. 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 +440,28 @@ func (v *Vaultik) scanRemoteBlobStorage( result.TotalBlobSize += obj.Size } + // A blob named only by a manifest that could not be read, or by one + // under a skipped name, would be counted as orphaned, so the orphan + // figures stay unknown. + if len(result.UnreadableManifests) > 0 || result.SkippedMetadataNames > 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 +514,21 @@ 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, "+ + "%d name(s) under metadata/ skipped)\n", + len(result.UnreadableManifests), result.SkippedMetadataNames) + + 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") } } diff --git a/internal/vaultik/remote_info_test.go b/internal/vaultik/remote_info_test.go new file mode 100644 index 0000000..cd1a1b7 --- /dev/null +++ b/internal/vaultik/remote_info_test.go @@ -0,0 +1,114 @@ +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" +) + +// testBlobHashB is a blob that the manifest written by addRemote does +// not reference. +const testBlobHashB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + +// 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() + + 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, 0 name(s) under metadata/ skipped)") + 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, and that the orphan figures are then unknown. The name comes +// from the destination store; printed raw, its control characters would +// reach the terminal. Its manifest is not read, so a blob only it +// references would otherwise be counted as orphaned. +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)) + addBlob(t, env.store.testStorer, testBlobHashA) + addBlob(t, env.store.testStorer, testBlobHashB) + 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)) + + text := env.stdout.String() + assert.NotContains(t, text, "\x1b") + assert.NotContains(t, text, "31mred") + assert.Contains(t, text, "Total (1 snapshots)") + assert.Contains(t, text, "Orphaned (unreferenced): unknown "+ + "(0 manifest(s) could not be read, 1 name(s) under metadata/ skipped)") + assert.NotContains(t, text, "vaultik prune") + + env.stdout.Reset() + require.NoError(t, env.v.RemoteInfo(true)) + + out := env.stdout.String() + assert.NotContains(t, out, "31mred") + + var doc map[string]any + + require.NoError(t, json.Unmarshal([]byte(out), &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.InDelta(t, 1, doc["skipped_metadata_names"], 0) + assert.NotContains(t, doc, "unreadable_manifests") +}