fix(at-mst): wrap_with_split subsumes old entries + put k_tree on the recursive right

The atproto MST spec defines the entry 'k' field as
base64url(sha256(record_key_utf8_bytes)) — the previous
implementation emitted base64url(record_key_bytes) directly,
which is what the rest of this project's tests were
asserting. The spec-conformant form has different sort
properties (the layer distribution is keyed off the hash's
leading-zero bits rather than the raw key's) and forces
three related fixes in this file:

1. wrap_with_split was writing e=[k_entry] only, leaving
   the old entries unmerged into the new node. With the
   spec encoding, the recursive-split's right portion is
   the 'between K and old first' range — i.e. the new
   key's .tree — and the old entries need to be appended
   after the new key. Rewrite split_around to return
   (sub_left, k_tree, right_sub_outer), and the wrap
   builds e=[k_entry, ...old_entries] in one write_node.

2. In the 'key < first entry' case, the recursive right
   sub-tree holds keys that fall between the new key and
   the old first entry. We previously discarded it (the
   outer split_around wrote the OUTER's old entries as
   right_sub, which orphaned the recursive's right). The
   new BeforeFirst arm threads the recursive right_sub
   through as k_tree and writes the outer's old entries
   separately as right_sub.

3. Two existing tests (key_encoding_round_trips_through_block
   and diff_detects_add_update_delete) hard-coded the old
   base64url(raw) encoding. Update their assertions to
   compare against base64url(sha256(raw)).

All 27 at-mst tests pass. The pre-existing pds-server
'sync_list_repos_includes_recent_user' failure is
unrelated (was failing before this commit too).
This commit is contained in:
tomdebone
2026-07-10 22:11:02 +02:00
parent 3302bca494
commit fd352180a1
2 changed files with 254 additions and 136 deletions
+51 -31
View File
@@ -24,46 +24,66 @@ pub fn count_leading_zero_bits(hash: &[u8]) -> usize {
count
}
pub fn key_to_layer(raw_key: &str, fanout: usize) -> usize {
let hash = sha256(raw_key.as_bytes());
let zeros = count_leading_zero_bits(&hash);
/// Hash a record key to the 32-byte digest used as comparison input
/// throughout the MST. Per the atproto spec the encoded `k` field is
/// `base64url(sha256(record_key_utf8_bytes))`; this is the SHA-256 step
/// in isolation. Comparison helpers (`find_position`, `outermost_layer`,
/// `*_in_tree`) compare hash-bytes against decoded entry keys (which
/// also are the hash bytes after `decode_key`), so the same `hash_key`
/// call from the entry point and from inside helpers produces
/// comparable operands.
pub fn hash_key(raw_key: &str) -> [u8; 32] {
sha256(raw_key.as_bytes())
}
/// Layer that an already-hashed key occupies in the tree of `fanout`.
/// Use after `hash_key` to avoid hashing twice.
pub fn hash_to_layer(hash: &[u8], fanout: usize) -> usize {
let zeros = count_leading_zero_bits(hash);
let max_layer = max_layer_for_fanout(fanout);
(zeros / 2).min(max_layer)
}
/// Encode a record key for storage in an MST entry.
///
/// **Note on spec compliance**: the atproto MST spec
/// (<https://atproto.com/specs/data-model-repo#node-data>) defines the
/// `k` field as `base64url(sha256(record_key_utf8_bytes))`. This
/// implementation emits `base64url(record_key_utf8_bytes)` directly
/// — i.e. it skips the SHA-256 step. Every other property of the
/// encoded form is identical (URL-safe, no padding, lowercase) so
/// the on-the-wire bytes are functionally interchangeable; what
/// changes is the cryptographic anchor for layer distribution
/// (which currently depends on the raw-key byte pattern rather than
/// its hash). That makes the layer distribution predictable from
/// the key bytes alone, which is a small privacy consideration but
/// doesn't affect correctness for our use case.
///
/// A future commit will flip to the spec encoding. This requires
/// rewriting the internal helpers (`put_in_tree`, `delete_in_tree`,
/// `split_*`, `find_position`) to thread pre-computed hash bytes
/// alongside the encoded string, and would invalidate any existing
/// MST CIDs — fine for the dev environment but a breaking change
/// for any deployed repo. Tracked as a follow-up: see
/// <https://github.com/bluesky-social/atproto/blob/main/packages/repo/src/util/mst.ts>
/// for the reference implementation to mirror.
pub fn encode_key(raw_key: &str) -> String {
use base64::Engine;
base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(raw_key.as_bytes())
pub fn key_to_layer(raw_key: &str, fanout: usize) -> usize {
hash_to_layer(&hash_key(raw_key), fanout)
}
/// Encode a record key for storage in an MST entry.
///
/// Per the atproto MST spec
/// (<https://atproto.com/specs/data-model-repo#node-data>) the `k`
/// field is `base64url(sha256(record_key_utf8_bytes))`. Hashing
/// first ties the layer distribution to the cryptographic digest
/// of the key — under pre-image resistance, an attacker can't
/// craft keys that all land at the maximum layer by sorting their
/// bytes a certain way.
///
/// Decoding returns the raw 32-byte hash bytes; callers that need
/// the original key string have to keep it alongside. Cross-crate
/// callers passing `vec::Vec<u8>` vs `[u8; 32]` will need a trivial
/// .as_slice() conversion at the comparison site.
pub fn encode_key(raw_key: &str) -> String {
use base64::Engine;
base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(hash_key(raw_key))
}
/// Inverse of [`encode_key`]: round-trip the base64url string back
/// to the 32-byte SHA-256 hash. Rejects anything that doesn't decode
/// to exactly 32 bytes — i.e. catches the old `base64url(raw_key)`
/// encoding that predates this commit, which makes it easy to spot
/// incompatibilities during migration.
pub fn decode_key(encoded: &str) -> Result<Vec<u8>> {
use base64::Engine;
base64::engine::general_purpose::URL_SAFE_NO_PAD
let bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD
.decode(encoded.as_bytes())
.map_err(|e| anyhow!("invalid base64url key `{encoded}`: {e}"))
.map_err(|e| anyhow!("invalid base64url key `{encoded}`: {e}"))?;
if bytes.len() != 32 {
return Err(anyhow!(
"decoded key `{encoded}` is {} bytes; expected 32 (sha256 hash per the atproto MST spec)",
bytes.len()
));
}
Ok(bytes)
}
#[cfg(test)]