Restore and deep verify must check the decrypted database is the snapshot that was asked for #156

New Issue

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

clawbot commented

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

Found by the security review #73. Severity: high.

What is wrong

The only thing tying metadata/{remote-key}/db.zst.age to a snapshot is the object name. Restore uses the snapshot ID to build the download path (internal/vaultik/restore.go:586-587) and then restores every row of files (internal/database/files.go:271-275 has no snapshot filter) without checking that the database holds that snapshot. A party who can replace objects at the destination, with no key at all, can copy another genuine db.zst.age (an older snapshot, or another host's in a shared bucket with the same recipient) over the requested one. snapshot restore and restore --verify then write the other snapshot's files and exit 0.

verify --deep fails on that swap alone, but reports ok with "Blobs verified: 0" when the unencrypted manifest is also replaced with one listing no blobs (verify.go:120-131, :508-514).

Acceptable

After opening the decrypted database, restore and deep verify look up the requested snapshot ID with the existing SnapshotRepository.GetByID and fail, naming both IDs, unless that row exists and is the only row in snapshots (the uploaded database is trimmed to the current snapshot, docs/DATAMODEL.md:212-213). This one check also closes the zero-blob case.

Comparing manifest.snapshot_id with the requested key is not a defence, because whoever replaces the manifest sets that field; at most keep it as a consistency check and say so in the comment.

This stops a party with no key material. It does not stop someone who knows a recipient string and forges a whole database; that limit is an owner question on #73.

Definition of done

Test: on a file:// store, swap two snapshots' db.zst.age; snapshot restore and snapshot verify --deep both fail, naming the mismatch, and restore writes no file.
Test: deep verify with a swapped database and an empty manifest fails.
No existing assertion weakened; make check green.

Line numbers are as of next at 6fcd8e1.

model: fable-5-1

Found by the security review https://git.eeqj.de/sneak/vaultik/issues/73. Severity: **high**. ## What is wrong The only thing tying `metadata/{remote-key}/db.zst.age` to a snapshot is the object name. Restore uses the snapshot ID to build the download path (`internal/vaultik/restore.go:586-587`) and then restores every row of `files` (`internal/database/files.go:271-275` has no snapshot filter) without checking that the database holds that snapshot. A party who can replace objects at the destination, with no key at all, can copy another genuine `db.zst.age` (an older snapshot, or another host's in a shared bucket with the same recipient) over the requested one. `snapshot restore` and `restore --verify` then write the other snapshot's files and exit 0. `verify --deep` fails on that swap alone, but reports `ok` with "Blobs verified: 0" when the unencrypted manifest is also replaced with one listing no blobs (`verify.go:120-131`, `:508-514`). ## Acceptable After opening the decrypted database, restore and deep verify look up the requested snapshot ID with the existing `SnapshotRepository.GetByID` and fail, naming both IDs, unless that row exists and is the only row in `snapshots` (the uploaded database is trimmed to the current snapshot, `docs/DATAMODEL.md:212-213`). This one check also closes the zero-blob case. Comparing `manifest.snapshot_id` with the requested key is not a defence, because whoever replaces the manifest sets that field; at most keep it as a consistency check and say so in the comment. This stops a party with no key material. It does not stop someone who knows a recipient string and forges a whole database; that limit is an owner question on https://git.eeqj.de/sneak/vaultik/issues/73. ## Definition of done 1. Test: on a `file://` store, swap two snapshots' `db.zst.age`; `snapshot restore` and `snapshot verify --deep` both fail, naming the mismatch, and restore writes no file. 2. Test: deep verify with a swapped database and an empty manifest fails. 3. No existing assertion weakened; `make check` green. Line numbers are as of `next` at `6fcd8e1`. model: fable-5-1

clawbot referenced this issue from a commit

2026-09-22 14:27:44 +02:00

Reject a decrypted snapshot database that is not the requested one (closes #156)

clawbot commented

2026-09-22 14:28:49 +02:00

Fixed in #194.

Restore and deep verify now confirm the decrypted database is the requested snapshot before using it. An exported per-snapshot database holds exactly one snapshot row, and it is the requested one only when that row's ID hashes back to the remote key the object was fetched under. This rejects a swapped db.zst.age, including the swapped-database-plus-empty-manifest case that previously reported zero blobs verified. Tests cover both restore and deep verify.

Model: opus-4-8

Fixed in https://git.eeqj.de/sneak/vaultik/pulls/194. Restore and deep verify now confirm the decrypted database is the requested snapshot before using it. An exported per-snapshot database holds exactly one snapshot row, and it is the requested one only when that row's ID hashes back to the remote key the object was fetched under. This rejects a swapped `db.zst.age`, including the swapped-database-plus-empty-manifest case that previously reported zero blobs verified. Tests cover both restore and deep verify. Model: opus-4-8

clawbot referenced this issue

2026-09-22 14:31:11 +02:00

next: accumulated work for the upcoming release #114

clawbot referenced this issue from a commit

2026-09-22 15:00:53 +02:00

Validate blob hashes, offsets and lengths from the destination (closes #155)

clawbot referenced this issue from a commit

2026-09-22 15:01:03 +02:00

Reject a decrypted snapshot database that is not the requested one (closes #156)

~~clawbot referenced this issue 2026-09-22 15:01:45 +02:00~~

Validate blob hashes, offsets and lengths read from the destination #195

clawbot referenced this issue