Preserve signed child JSON during typed compound serialization - #1051
Merged
dahlia merged 2 commits intoSep 26, 2026
Merged
Conversation
Signing a Note with signObject(), assigning it to a typed Create and serializing the parent used to rebuild the child under the parent's JSON-LD context. The child kept its proofValue but lost its own document and proof contexts, so the extracted child no longer verified, while the outer proof stayed valid over the mangled child. Producing a FEP-ef61 compound document therefore meant assembling the JSON by hand. signObject() now captures the secured JSON document its proof covers and keeps it on the object it returns. The document is not re-derived from the vocabulary object: it is the value createProof() hashed, plus the serialized proof, checked against the message digest, the proof configuration digest and the proof value, so a captured document verifies by construction. Nested serialization emits a placeholder node reference for a child that carries one, and the outermost toJsonLd() frame puts the captured document back after compaction. Placeholders are used on both encoder paths, because an ancestor frame may still compact a value that an inner frame embedded. The captured document is a snapshot, independent of anything done to the returned object afterwards. clone() never carries it, since a clone may differ from the document the proof covers. Retention is skipped, and ordinary serialization applies, for an object parsed with fromJsonLd(), an object that already carried a proof, a document too large or too deep to validate, and a toJsonLd() context that could hide the placeholder. A placeholder that cannot be put back throws rather than reaching the wire. Two adjacent paths could still destroy a secured child: - Outgoing JSON-LD compatibility normalization rewrote nested documents. It now leaves a nested self-contained secured document untouched under an opt-in option that only the producer path passes, so verifyProof()'s inbound fallback is unchanged. - Fanout reparsed the activity and serialized it again. It now reuses the document the activity was already serialized into, and an explicit Ed25519 sender's activity is signed before that serialization rather than by the delivery worker, while the typed child still carries its snapshot. An activity that already has a proof keeps its own. Fixes fedify-dev#1044 fedify-dev#288 fedify-dev#1041 Assisted-by: Claude Code:claude-opus-5 Assisted-by: OpenCode:deepseek-flash Assisted-by: Codex:gpt-6-astra Assisted-by: Claude Code:claude-fable-5-1
dahlia
added
component/vocab
✅ Deploy Preview for fedify-json-schema canceled.
|
dahlia
moved this to In review
in STF 2025–2026 Milestone 7: Advanced Features and Standards (FEP-ef61, new FEPs)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe change retains the secured JSON-LD document captured by ChangesSigned child preservation
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant signObject
participant ProofCapture
participant TypedSerializer
participant OutgoingNormalizer
participant FanoutDelivery
signObject->>ProofCapture: create proof and capture secured JSON-LD
ProofCapture-->>signObject: retain secured representation
TypedSerializer->>signObject: serialize signed child in typed parent
signObject-->>TypedSerializer: return retained child document
TypedSerializer->>OutgoingNormalizer: normalize parent JSON-LD
OutgoingNormalizer-->>TypedSerializer: preserve nested secured child
FanoutDelivery->>OutgoingNormalizer: send queued serialized activity
OutgoingNormalizer-->>FanoutDelivery: reuse normalized JSON-LD
Merge Risk: ⚪ Minimal · up to Signed child documents retain their original contexts and proofs through typed serialization and fanout, preserving compound-proof verification. The change is ready to merge with normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The new signed-representation tests imported @std/assert, which is a JSR
package that @fedify/vocab does not declare. Deno resolves it through the
workspace import map, so the Deno suite passed, but the Node.js and Bun
suites run a tsdown bundle from dist-tests where the specifier is left
external and unresolvable:
Cannot find package '@std/assert' imported from
packages/vocab/dist-tests/signed-representation.test.mjs
Every other test in the package uses node:assert, which all three runtimes
resolve. Switch to it.
Member
|
I think resolving #1026 should come first. |
Member
|
I don't think there is a strong dependency tho this and #1026 |
Member
Author
|
@2chanhaeng I think #1026 can be fixed separately. Could you share what connects the two for you? I might be missing something. |
Member
|
Errors occurred in the |
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
3 participants
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.
Signing a
NotewithsignObject(), assigning it to a typedCreate, and serializing the parent rebuilt the child under the parent's JSON-LD context. The child kept itsproofValuebut lost its own document and proof contexts, so an extracted child no longer verified while the outer proof stayed valid over the altered child. #1041 shipped with a warning telling producers not to do this. This removes the warning.Fixes #1044.
Capturing what was signed
Serializing the signed object a second time and keeping the result would only assume the stored bytes are the ones that were hashed, and the assumption breaks the moment anything touches the object afterwards.
createProof()keeps the exact value it canonicalized instead, together with the serialized proof. The pair is retained only when its message digest, proof configuration digest, and proof value all reproduce the proof just created, so correctness follows from the construction rather than from a later check. Signing needs no extra serialization.The stored value is a snapshot, not a view.
clone()never carries it, because a clone may differ from the document the proof covers, and mutating a signed object in place does not change what gets embedded.Getting it through compaction
A retained document cannot just be placed in the encoder's output, since
jsonld.compact()re-expands whatever it finds there. Nested encoders emit a placeholder node reference, and the outermosttoJsonLd()frame puts the document back once compaction is done. Both encoder paths use placeholders, including the compactable fast path that writes JSON directly: aQuestionholding aNoteinoneOfcompacts its whole subtree, so a value embedded verbatim by an inner frame is destroyed by an ancestor anyway.Whether a placeholder can be recovered depends on the active context, and a compacted
@nestwrapper is indistinguishable from an@idalias by inspection alone. Rather than guess, the context is vetted before any placeholder is emitted. A context that aliases@idunder another term, declares@nest, uses an@idcontainer, shadows the placeholder'surnscheme, or cannot be resolved offline turns retention off and gets the existing behavior. If replacement still fails, serialization throws instead of emitting the placeholder.Normalization and fanout
Outgoing compatibility normalization rewrites scalar
attachmentvalues and public addressing across the whole document, embedded secured children included. A producer-only option now stops it at a nested self-contained secured document.verifyProof()does not pass that option, so its inbound fallback keeps its current digest behavior.Fanout reparsed and reserialized the activity, discarding the snapshot stored on the typed object. It now reuses the document the activity was already serialized into. For an explicit Ed25519 sender the activity is signed before that serialization: signing in the delivery worker would cover a child that had already been rebuilt.
Limits
An object parsed with
fromJsonLd()gets no snapshot, because parsing does not establish which representation was signed. The characterization test for that stays, as #1044 asks. An object that already carried a proof is not retained either, since proof sets are outside the map-local profile.