refactor(snapshot): type receipt entries while preserving the content_id invariant - #493
Conversation
…rrowed Typing the entries changed the digest from 'every key the entry carried' to 'path, size_bytes, sha256'. An entry with an extra key now hashes the same where it used to hash differently: verified against main, which raises on that marker while this branch certifies it. That is the right trade, because it lets a later format revision add metadata without invalidating every snapshot already exported, and the receipt was never a tamper defence. But it was silent in both the code and the tests, so it gets a case and a paragraph.
kstonekuan
left a comment
There was a problem hiding this comment.
LGTM. Merging.
I verified the invariant the way it needed verifying, which is across versions rather than within one. Your test proves the two code paths agree with each other; it cannot prove the digest did not move, because both halves of it live on this branch. So I exported a copy-mode snapshot (7 tables, 1 asset) using main, kept the format.json, then ran this branch's verifier against that exact directory:
verify of a main-exported snapshot: ok=True, findings=0
stored : 29082d4516daa3d7c143676d9f1595625da523eab01f2beb8d23bf44a7e64e0a
recomputed : 29082d4516daa3d7c143676d9f1595625da523eab01f2beb8d23bf44a7e64e0a
It holds. That is the result the PR turned on.
Splitting the exit code is the right call for the reason you gave: a numeric sha256 was being reported as damaged bytes, which tells the recipient their data is corrupt when the truth is that whoever wrote the receipt got it wrong. Different person, different fix, different exit code.
One thing the refactor changed that neither the code nor the tests mentioned, and I pushed a commit for it.
Hashing raw dicts meant the digest depended on every key an entry happened to carry. Hashing records means it depends on exactly path, size_bytes and sha256. So an entry with an extra key now hashes the same. Measured both ways on the same marker, with one field injected into one table entry:
main raised ValueError: ... recomputed inventory content_id '117864fc...' != stored
branch status=ok ok=True findings=0
I think that is right, and I want it written down rather than discovered. It means a later format revision can add metadata to entries without invalidating the digest of every snapshot already exported, which is a real compatibility property. What it costs is catching a marker someone edited to add a field, which costs nothing: this receipt travels unsigned inside the file it describes and was never a tamper defence, and the guarantee the docs actually make, that a deleted member stays visible, is untouched. My commit adds the case and a paragraph on FileIntegrityRecord, mostly so nobody later "tightens" it back to raw-dict hashing and trades a real property for an imaginary one.
Validated on the rebased result: ruff, ruff format, ty, 1824 passed / 6 skipped.
That is #472, #473 and #489 all in, and this one had the sharpest constraint of the three. #474 is yours next as you said; my note on that issue still stands, that the one-line flag lands every check in the Errored channel rather than producing the diagnosed refusal the issue asks for. #469 stays with VARUN3WARE.
|
Fair point on the cross-version check. I proved the two paths agreed on the branch, which only proves internal consistency, not stability. Exporting on main to verify against the branch is exactly the blind spot I missed. The extra-key delta is a sharp catch too. I was so focused on byte-identity for today's shape that I didn't map out the input space for future metadata. I will carry that four-point checklist (added, removed, reordered, retyped) into anything involving serialization from now on. I am starting the lane-entry validation for #474 now. |
refactor(snapshot): type receipt entries while preserving the content_id invariant
Closes #489.
What changed
Receipt entries no longer travel as
dict[str, str | int]through the verifier. A frozenFileIntegrityRecord(path: str, size_bytes: int, sha256: str) is parsed once per entry at the marker-reading boundary, next to the #473 content_id gate, and everything after the boundary is typed: the inventory gate, the per-file loop, and the sha256 comparison at what was :942.The boundary refuses without coercion, naming the exact field: a receipt whose
sha256arrived as a JSON number used to reach a comparison that can never succeed and was reported as damaged bytes (exit 1, "your data is damaged"); the truth is that the receipt is malformed, which is unreadable input (exit 2, "your receipt is written wrong"). The same refusal covers a non-string path, a non-int size_bytes, and bool masquerading as int.The invariant, proven
The hard constraint:
_inventory_content_idserializes entries withjson.dumps(sorted-by-path, sort_keys=True, separators=(",", ":")), and #483 made every verify compare that digest against the storedcontent_id. Moving one byte breaks every snapshot ever exported.Proof from this branch: the exact serialized string over an 8-entry inventory (7 tables, 1 asset, 1000 characters) and its digest are byte-identical before and after the refactor. Digest before and after:
c117d138cfdd8cd4c613906b8813aa92a89a96192c89ee6522e7a30fc9b69b9a. The bridge isto_dict_for_hashing(), which rebuilds exactly the dict shape the exporter has always serialized, and the digest is computed over that dict form, never over the dataclass.Durable pins in the suite: an invariant test that computes the digest the old raw-dict way and the new records way and asserts both equal a computed golden value, and a refusal test feeding a numeric sha256 and asserting the boundary raises naming the field.
Exporter unchanged, proven
The exporter now builds records and serializes them through
to_dict_for_hashingwhen writing the marker, so the emittedformat.jsoncarries the identical dict shapes. The dataset-snapshot suite, which pins the marker's structure and recomputes itscontent_id, passes unchanged apart from parsing records where it recomputes.Mutation
With the parser changed to accept an int
sha256, exactly the refusal test fails and the other 42 stay green. Restored, all green.Validation
43 passed across tests/test_snapshot_verify.py and tests/test_dataset_snapshot.py, ruff check, ruff format, ty clean on the changed files, zero non-ASCII and zero em dashes. Boundary-family note: #486 (format identity) and #483 (inventory gate) are merged; #469 (path containment) stays with VARUN3WARE; #474 (post-sync CRC reads) is claimed and next.