fix(snapshot): enforce the receipt inventory content_id at verify time - #483
Conversation
…cs#473 gate The gate is two refusals. The mismatch branch is held by the pinning test; deleting the missing-or-malformed branch left the whole suite green. Four shapes: absent, empty, non-string, wrong type, each through the API and the CLI exit code.
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM. Merging.
The exit-2 argument is right, and the reason it is right is narrower than the PR body says: integrity and content_id arrived in the same commit (#401), so no snapshot hflow has ever written carries one without the other. A marker with an integrity block and no content_id did not come from us, which is what makes it unreadable input rather than damage.
I verified the two halves separately, because both raise a message containing "content_id" and your match= cannot tell them apart:
mismatch branch removed, missing-id kept FAILED test_removed_receipt_entry_and_file_raise_inventory_mismatch
missing/malformed content_id branch removed 31 passed (nothing noticed)
inventory no longer sorted by path 15 failed
exporter stops writing content_id 15 failed
The missing-id branch had no case, so I pushed one: four shapes (absent, empty, non-string, wrong type) through both the API and the CLI exit code.
The last two rows are the nice part. The gate makes the whole verify suite sensitive to the exporter's side of the receipt, so the normalization at :153 and the write at :194 are now pinned by fifteen tests that previously said nothing about them. Neither had a direct test before.
Your negative control is the thing more PRs should carry. Deleting the file while keeping its entry passes with and without the gate, which is what proves the pinning test is testing the gate and not just the deletion.
Validated on the rebased result: ruff, ruff format, ty, 1729 passed / 6 skipped.
#472 next, if you want it. The good first issue pool is best left for newcomers, and you are well past that, so the other thing worth your time is running HFlow against a real corpus (Egocentric-10K or Egocentric-100K on Hugging Face) and reporting what breaks or drags. That feedback is the kind we cannot generate here.
|
Thanks @kstonekuan . #472 is next then i will see what i can do on (Egocentric-10K or Egocentric-100K on Hugging Face) |
fix(snapshot): enforce the receipt inventory content_id at verify time
Closes #473.
What changed
verify_dataset_snapshotnow enforces theintegrity.content_idguarantee documented at docs/how-to/export-dataset-snapshot.md:77. After the receipt is parsed and before the per-file loop, the verifier recomputes the inventory hash from the receipt's own table and asset entries (the identical normalization the exporter uses at snapshot.py:194, via the shared_inventory_content_idhelper) and compares it against the stored value.The exit-2 decision
A
content_idthat disagrees with the recomputed inventory means the marker is internally inconsistent, which points at tampering or a truncated write, not at damaged bytes. Damaged bytes are what findings and exit 1 are for; an inconsistent receipt is unreadable input. The gate therefore raises through the sameValueErrorthe unparsable-marker path uses (snapshot.py:862), so the CLI maps it to exit 2 with no new exit code. A DAMAGED finding was considered and rejected: findings are evidence about delivered files, and the failure here is in the receipt itself, which per-file findings cannot express without inventing a new reason code for a non-file.The same handling covers a missing or malformed
content_idon a marker that carries anintegrityblock: one gate, one behavior, one exit code. Pre-#401 markers with nointegritykey keep the existingno-receiptUNVERIFIABLE path untouched.Test shape, and why
content_idcatches: the surviving entries agree with each other, so every per-file check passes; the comparison against the hash of the original set is the only witness.missingpath and passes without the fix; under the mutation run it stayed green while the pinning test went red, proving the two tests pin different behaviors.Related issues, separate PRs
Gates
15 passed in tests/test_snapshot_verify.py (13 existing + pinning test + negative control), 31 passed across the verify and dataset-snapshot suites, ruff check / ruff format / ty clean, zero non-ASCII and zero em dashes in changed files. Mutation run: inventory comparison neutered, pinning test red, negative control green, restored, all green.