snapshot purge deletes every local snapshot record and then purges nothing #160

New Issue

2026-09-22T00:55:14+02:00

clawbot commented

2026-09-22 00:55:14 +02:00

Found in passing by the security review #73; outside its seven scope items. Severity: medium, functional bug (no confidentiality or integrity loss; remote blobs stay referenced by remote manifests). Line numbers are as of next at 6fcd8e1.

What is wrong

syncWithRemote (internal/vaultik/snapshot.go:933-990) collects parts[1] of each metadata/{remote-key}/ listing entry, which is the hashed remote key, and then looks up the human snapshot ID in that set (:970-971). Nothing matches, so every local snapshot row, with its snapshot_files, snapshot_blobs and uploads rows, is deleted. The comment at :945 still describes the old plaintext layout.

PurgeSnapshotsWithOptions calls it first (:472) and then reads the now-empty local list, so snapshot purge and the purge half of snapshot create --prune remove nothing: retention never applies and the store grows without bound. The next snapshot create runs PruneDatabase first (:73), which drops all file, chunk and blob rows as orphans, so that backup rescans everything.

The purge tests pass only because the helper writes stubs under metadata/{human id}/ (internal/vaultik/purge_per_name_test.go:64), a layout production never writes. CleanupLocalSnapshots does the same comparison correctly through snapshot.RemoteSnapshotKey (:847-866).

Acceptable

syncWithRemote builds its set from listAllRemoteSnapshotKeys() and tests snapshot.RemoteSnapshotKey(id).
Do not simply call CleanupLocalSnapshots instead: it prints prose to stdout (:892-896), which would break the quiet purge used by --prune.
setupPurgeTest writes its stubs under the hashed key.

Definition of done

Test on the production layout: purge with --keep-latest deletes the older snapshot's remote metadata and keeps the newest local row.
Test: after a purge, local rows for snapshots that still exist remotely are intact.
No existing assertion weakened; make check green.

model: fable-5-1

Found in passing by the security review https://git.eeqj.de/sneak/vaultik/issues/73; outside its seven scope items. Severity: **medium**, functional bug (no confidentiality or integrity loss; remote blobs stay referenced by remote manifests). Line numbers are as of `next` at `6fcd8e1`. ## What is wrong `syncWithRemote` (`internal/vaultik/snapshot.go:933-990`) collects `parts[1]` of each `metadata/{remote-key}/` listing entry, which is the hashed remote key, and then looks up the human snapshot ID in that set (`:970-971`). Nothing matches, so every local snapshot row, with its `snapshot_files`, `snapshot_blobs` and uploads rows, is deleted. The comment at `:945` still describes the old plaintext layout. `PurgeSnapshotsWithOptions` calls it first (`:472`) and then reads the now-empty local list, so `snapshot purge` and the purge half of `snapshot create --prune` remove nothing: retention never applies and the store grows without bound. The next `snapshot create` runs `PruneDatabase` first (`:73`), which drops all file, chunk and blob rows as orphans, so that backup rescans everything. The purge tests pass only because the helper writes stubs under `metadata/{human id}/` (`internal/vaultik/purge_per_name_test.go:64`), a layout production never writes. `CleanupLocalSnapshots` does the same comparison correctly through `snapshot.RemoteSnapshotKey` (`:847-866`). ## Acceptable - `syncWithRemote` builds its set from `listAllRemoteSnapshotKeys()` and tests `snapshot.RemoteSnapshotKey(id)`. - Do not simply call `CleanupLocalSnapshots` instead: it prints prose to stdout (`:892-896`), which would break the quiet purge used by `--prune`. - `setupPurgeTest` writes its stubs under the hashed key. ## Definition of done 1. Test on the production layout: purge with `--keep-latest` deletes the older snapshot's remote metadata and keeps the newest local row. 2. Test: after a purge, local rows for snapshots that still exist remotely are intact. 3. No existing assertion weakened; `make check` green. model: fable-5-1

clawbot referenced this issue from a commit

2026-09-22 11:56:08 +02:00

Reconcile purge against remote by hashed key, not human ID (closes #160)

clawbot referenced a pull request that will close this issue

2026-09-22 11:56:24 +02:00

Reconcile purge against remote by hashed key, not human ID #183

clawbot commented

2026-09-22 11:56:54 +02:00

Fixed in #183 (base next).

syncWithRemote compared human snapshot IDs against the hashed metadata/<key>/ directory names, which never match, so the reconcile step deleted every local snapshot record and the purge then removed nothing. It now reconciles via listAllRemoteSnapshotKeys() and snapshot.RemoteSnapshotKey(id), the same logic CleanupLocalSnapshots uses, kept as its own quiet function so --prune output stays clean.

The purge test stubs now use the production hashed-key layout. Two new tests prove remotely-backed local rows survive the reconcile and that a purge removes the local row and remote metadata together. make check passes.

Model: opus-4-8

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/183 (base `next`). `syncWithRemote` compared human snapshot IDs against the hashed `metadata/<key>/` directory names, which never match, so the reconcile step deleted every local snapshot record and the purge then removed nothing. It now reconciles via `listAllRemoteSnapshotKeys()` and `snapshot.RemoteSnapshotKey(id)`, the same logic `CleanupLocalSnapshots` uses, kept as its own quiet function so `--prune` output stays clean. The purge test stubs now use the production hashed-key layout. Two new tests prove remotely-backed local rows survive the reconcile and that a purge removes the local row and remote metadata together. `make check` passes. Model: opus-4-8

clawbot closed this issue

2026-09-22 12:11:49 +02:00

clawbot referenced this issue from a commit

2026-09-22 12:11:50 +02:00

Reconcile purge against remote by hashed key, not human ID (closes #160)

clawbot referenced this issue

2026-09-22 12:14:07 +02:00

next: accumulated work for the upcoming release #114

Sign in to join this conversation.