fix(serder)!: guard version-string fixed widths and validate at every deserialize entry#139
Merged
Merged
Conversation
… deserialize entry
Three defects on the serder edge:
- VersionString::to_str() rendered major/minor/size with unguarded {:x}/
{:06x}: any value beyond its fixed hex width (major/minor > 0xF, size
> 0xFFFFFF) silently widened the documented 17-byte version string and
corrupted the frame. to_str() now returns Result and rejects overflow
with the new SerderError::VersionStringOverflow variant.
- All five event serializers mapped the measured-length u32 conversion
failure into SerderError::DigestError (wrong failure domain) and never
checked the 6-hex-digit size capacity; a >16 MiB event serialized to a
corrupt frame. They now reject it as VersionStringOverflow.
- Only deserialize_event validated the version string; the per-ilk public
entry points (deserialize_inception/rotation/interaction/
delegated_inception/delegated_rotation) skipped it, so raw whose length
contradicts its version-string size — e.g. whitespace-padded JSON with
an intact SAID — was accepted. Every public deserializer now calls
validate_version_string first.
BREAKING CHANGE: VersionString::to_str() returns Result<String,
SerderError>; SerderError gains the VersionStringOverflow variant.
Bug probes (red before fix): to_str_rejects_size_beyond_fixed_width,
to_str_rejects_{major,minor}_beyond_one_hex_digit,
serialize_ixn_rejects_event_beyond_version_size_capacity,
all_public_deserializers_reject_length_mismatched_raw.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
joeldsouzax
added a commit
that referenced
this pull request
Jul 10, 2026
…d in #139 Answers the review question 'why do we need serde at all?': deserialization happens once at the edge and the write path uses none of serde's data model, so the end state is a serde-free production path with serde_json demoted to a dev-dependency oracle. Read path becomes a strict canonical parser (borrowed fields + SAID offsets, feeding C-a #129); strictness is itself a conformance feature — the whitespace-acceptance defect fixed in #139 is the proof case. Staged migration spelled out in §6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
Open
joeldsouzax
added a commit
that referenced
this pull request
Jul 10, 2026
… seam (#138) * docs(serder): #79 research write-up — pluggable serialization backend seam design Research-first deliverable for #79: status-quo allocation audit (3x to_string per serialize, 2-3x from_slice + re-render per deserialize), body-encoder seam design with fixed-width slot backpatching, crate survey (zero new deps), cross-backend conformance strategy, and a latent version-string size-overflow defect to fix under this card. Refs #79 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(serder): #79 write-up — serde-free endgame (§3.5) + defects fixed in #139 Answers the review question 'why do we need serde at all?': deserialization happens once at the edge and the write path uses none of serde's data model, so the end state is a serde-free production path with serde_json demoted to a dev-dependency oracle. Read path becomes a strict canonical parser (borrowed fields + SAID offsets, feeding C-a #129); strictness is itself a conformance feature — the whitespace-acceptance defect fixed in #139 is the proof case. Staged migration spelled out in §6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Open
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three defects on the serder edge, found during the #79 research phase (write-up in #138). Each landed with a bug probe that fails while the defect exists.
1.
VersionString::to_str()silently corrupted the 17-byte frame{:x}/{:06x}rendering had no width guards:major/minor>0xForsize>0xFF_FFFF(16 MiB − 1) widened the version string past its documented 17 bytes, corrupting the frame instead of erroring.to_str()now returnsResultand rejects overflow with the newSerderError::VersionStringOverflowvariant.2. Serializers never checked the size-field capacity (and misfiled the error)
All five event serializers could produce a corrupt frame for a > 16 MiB event, and mapped the
u32length-conversion failure intoSerderError::DigestError— the wrong failure domain. Both fixed; an end-to-end probe serializes a 340k-anchor interaction event and asserts the typed error.3. Per-ilk deserializers skipped version-string validation
Only
deserialize_eventcalledvalidate_version_string; the five per-ilk public entry points did not. Because SAID verification hashes the re-serialized compact form, whitespace-padded raw — valid JSON, intact SAID, length contradicting the version-string size — was accepted bydeserialize_inceptionet al. Every public deserializer now validates first (probe:deserialize_*_rejects_length_mismatched_raw, red before the fix).Breaking change (0.x minor bump)
VersionString::to_str()→Result<String, SerderError>SerderErrorgainsVersionStringOverflow { field, max }Both intentional; per-ilk deserializers returning
InvalidVersionStringon inputs previously accepted is the bug fix itself.Verification
nix flake checkgreen (pre-push hook, committed tree)Refs #79 (research: #138)
🤖 Generated with Claude Code