feat(snapshot): verify delivered snapshots against integrity receipts - #457
Conversation
kstonekuan
left a comment
There was a problem hiding this comment.
You carried the #454 finding across before I asked, and that is the part worth naming: test_moved_root_verifies_from_the_new_root_alone and test_damage_is_reported_from_the_verified_root_not_the_export_root are exactly the pair that would have caught the bug on the other PR.
Here it is correct by construction rather than by care: delivered_path = resolved_directory / relative_path at :887, and the snapshot receipt stores relative paths, so there is no absolute URI to be tempted by. Your tests pin it anyway, which is right. A property that holds by accident of the data format is one refactor away from not holding.
Validated: full gate clean, 1620 passed / 6 skipped. Every branch carries its own case:
missing-file branch never fires FAILED test_missing_file_reports_missing_alone
FAILED test_partial_transfer_reports_every_mismatch_in_one_report
size mismatch never reported FAILED test_truncated_file_reports_size_mismatch_and_skips_the_hash
resolution ignores the given root 12 of 13 tests red
Four things I want to record because they are the decisions rather than the code:
REASON_NO_RECEIPT as a finding rather than a raise is the right call. A pre-#401 format.json is a valid v1 snapshot, and reporting it as corrupt would punish people for having exported last week. Reporting it as unverifiable with exit 3 says the true thing: we cannot tell.
Size as a prefilter that skips the hash, with a test asserting the skip, is a nice touch on a copy-mode snapshot holding video.
test_verify_is_read_only_against_the_delivery is the test I would not have thought to ask for. A verifier that mutates what it verifies is a special kind of useless.
Ignoring unlisted files under assets/ matches what you and @VARUN3WARE agreed on #432, and the test says so out loud.
Merging. This sets the shape and #454 imports it, which is the order you two settled on.
Import VerificationReport from the landed verification module (Hebbian-Robotics#457) and keep the import-delivery half in lerobot_verify: resolve landing/<basename> under the verified root, treat empty episodes as ok, and cover Kevin's copy cases.
feat(snapshot): verify delivered snapshots against integrity receipts
Closes #428.
Problem
#401 stamps every exported snapshot with an integrity receipt (per-file size and sha256, plus an inventory content_id), but nothing checks a delivered snapshot against it. A truncated transfer looks exactly like a clean one.
What this adds
verify_dataset_snapshot(<directory>)andhflow verify snapshot <directory>re-read every table and copied asset named in theintegrityblock and compare size first, then sha256 (the hash read is skipped when the size already differs).0clean,1damaged,2unreadable input,3unverifiable.format.jsonwith nointegritykey is reported as ano-receiptfinding (unverifiable), not corruption. A missing directory or an unparsableformat.jsonraises to exit2: the wrong input, not damage.format.jsonit describes.Shared types, landed here first
The report shape lives in
src/hflow/verification.py:VerificationStatus(ok / damaged / unverifiable),VerificationFinding(uri, reason, detail),VerificationReportwith.okderived from the status,exit_code_for, and the reason constantsmissing,size-mismatch,content-id-mismatch,no-receipt. This is the shape agreed in the #432 contract with VARUN3WARE; #454 imports these types rather than redefining them.Recorded receipt paths are joined only onto the handed directory, so a copied delivery verifies in place.
Moved-root proof
test_moved_root_verifies_from_the_new_root_aloneexports a snapshot to root A, copies the whole delivery to root B, deletes A entirely, and verifies B clean, then damages one file under B and verifies B damaged. With A gone, any read outside the handed root would see nothing, so both reports can only come from B's own bytes. A variant keeps A alive while B is damaged and asserts the damage is still reported from B, not masked by A.Cost
One streaming sha256 pass per receipted file, plus the size stats. A size mismatch short-circuits that file's hash read. No catalog, storage, or media fetch traffic: verification reads only the handed directory.
Tests
13 tests in
tests/test_snapshot_verify.py: clean verifies in both media modes, same-size content damage, missing file, truncation reported by size with the hash read skipped (spy on_sha256_hex), copied-asset damage, pre-#401 no-receipt, unlisted extras ignored, multi-finding partial transfer, CLI exit codes 0/1/3/2 including empty and binary-garbageformat.json, read-only verification, and the two moved-root tests.Mutation check: with the size and sha256 comparisons disabled, the damage tests go red, including the spy assertion; restored, all green.