fix: root-of-push snapshots were never hash-verified before storage (musehub#93)
Picking up musehub#93/#60 properly now. Root cause found: the push handler's integrity check --
if _parent_sid and hash_snapshot(_base, _snap_dirs or None) != _sid: raise ValueError(...)
-- only ran when _parent_sid was truthy. Every snapshot that is the root of a push batch (parent_snapshot_id=None: the first commit sent, or a brand-new repo's first commit) has _parent_sid falsy, and was therefore NEVER checked against its own declared snapshot_id before being persisted. A wire/transport bug that lost or mangled `directories` (or, in principle, the manifest) for exactly this kind of entry would sail through completely unverified and be stored permanently -- undetectable until a client later re-hashed it on clone and rejected the mismatch. This is exactly the `entries=1062 dirs=0` corruption found live on gabriel/muse@staging that prompted #93 (manifest correct, directories silently lost, corruption only in stored data -- not the source repo).
Verified this reproduces and is fixed with a real RED/GREEN cycle, not just inspection: temporarily restored the old `if _parent_sid and ...` gate, confirmed the new rejection test fails against it (the bad push succeeds silently, logs show a normal INSERT), then restored the fix and confirmed both new tests pass.
Fix: run the hash check unconditionally for every incoming snapshot, root or not. For a root entry, `_base` is built entirely from `delta_upsert` (the client's `_build_snapshot_deltas` always sends a root's full manifest as delta_upsert -- see muse/core/mpack.py), so it legitimately equals the full manifest and the check is exactly as valid there as it already was for delta entries with a real parent. Split the error message by case (missing/unreconstructable parent vs. self-inconsistent root) so the two distinct failure modes stay distinguishable in logs.
2 new tests in test_wire_push_root_snapshot_integrity.py: a root snapshot whose directories don't reproduce its own declared ID is now rejected outright and never persisted (RED-verified against the pre-fix gate); a correctly-hashed root snapshot with real directories still pushes successfully and is stored with those directories intact (guards against the unconditional check becoming a false-positive regression).
Also picked up one of #93's other checklist items in a separate commit: `muse read-snapshot` was silently dropping the `directories` field from its own output (musehub#93 follow-up, muse repo, commit ec9e0eaa7).
Tests: 81/81 across the full push/mpack/snapshot-integrity targeted set (test_wire_push_root_snapshot_integrity, test_wire_push_external_parent_ reconstruction, test_push_delta_only_parent_manifest, test_wire_mpack_unpack_step3_*, test_mpack_delta_e2e, test_push_ff_check, test_push_xs_unit, test_authz_wire_push, test_identity_push_validator).
Remaining #93/#60 checklist items not yet done (tracked, next commits): - Close the empty-manifest delta-base bug (separate mechanism: external delta-only parent resolving to {}). - Repair the 4 known-corrupt gabriel/muse staging snapshots via wire_repair_snapshot, using correct data from the source repo. - #60's schema hardening (snapshot.directories: list[str] -> dict[str,str] with real EMPTY_DIR_OID-based object IDs) -- larger, cross-repo, phased change; not started yet.
Semantic Changes
24 symbols
0 comments
muse hub commit comment sha256:c3910cc561368d2b40576c1fbb0841b5d3abefd0a65c96c85114a7238222c77c --body "your comment"
No comments yet. Be the first to start the discussion.