[moisesja]

Fixes #145.

Problem

NetDid rejects duplicate JSON members, which makes System.Text.Json decode every member name while parsing. A lone surrogate escape in a name ("\uD800":1) therefore throws InvalidOperationException from JsonDocument.Parse, and two boundaries caught only JsonException:

• a fetched did:webvh log resolved as notFound instead of invalidDidLog;
• Core DidDocumentSerializer.Deserialize leaked a raw exception.

Later rounds found more defects of the same kind: hostile input escaping a trust boundary as a raw exception, or being accepted and failing later. They also found the write-side mirror image (text silently altered on publish) and gaps against the DID Core id and URI rules. All of it is fixed here:

• adversarial passes after every round;
• your two reviews (#pullrequestreview-5684631583, #pullrequestreview-5688590368);
• a DID Core id/URI syntax round;
• two final adversarial passes.

The full history is in tasks/todo20261028-issue145.md; the user-facing summary is the #145 entry in CHANGELOG.md.

Changes

Read side: every boundary reports its documented error

• did:webvh logs: an undecodable string anywhere in an entry is invalidDidLog. UpdateAsync and DeactivateAsync throw their documented FormatException.

• Core Deserialize throws JsonException for:

  • lone surrogates and ill-formed UTF-8;
  • a malformed publicKeyJwk, or one without kty;
  • shapes DID Core forbids, such as missing or non-string ids and types, null entries, or a missing, blank, empty or nested serviceEndpoint.

  A closing net maps any other JSON-access failure, so every document that is read can be written again.

• did:peer: service blocks are consumed under the same rules as a document (duplicate members, reserved or colliding keys, depth). The embedded JSON is decoded as strict UTF-8.

• did:peer:2 follows the DIF abbreviation rules: member names are abbreviated at every depth (also inside serviceEndpoint), and only the type value DIDCommMessaging becomes dm. The spec's example DID is produced byte for byte and resolved with accept/routingKeys. Explicit relative service ids are kept ("service id attributes MUST be relative"), and absolute ones are rejected.

• did:ethr:

  • RPC client: undecodable or duplicate result members throw EthereumInteractionException, and receipt errors never echo node text.
  • Service values are decoded like ethr-did-resolver: strict UTF-8, JSON.parse keeping the last of a duplicate member, raw-string fallback, and its name grammar.
  • Labels: bytes32 labels are strict UTF-8. An event whose label is not valid UTF-8 is filtered out of the history, as the reference does.

Write side: never publish altered or unreadable output

• Every writer checks the text it emits, including the contents of caller JsonElements, and throws ArgumentException instead of writing U+FFFD. This covers Core Serialize, did:webvh state, did.json, parameters and witness proofs, did:peer:2/4 Create, and did:ethr endpoints.
• Writers also reject:
  • null required members and null list entries;
  • duplicate member names;
  • reserved service extension keys;
  • non-public JWK members (every public RFC 7517/7518/8037 member is kept).
• ServiceEndpointValue.FromSet enforces DID Core §5.4 (no null entries, no nested sets) and snapshots its entries.
• Caller JsonSerializerOptions now produce a real DID document; NetDid's converters always come first.
• did:webvh builds did.json before signing, so an invalid projection never costs a signature.
• did:peer:2 and did:peer:4 Create read the caller's input once and return the document their DID resolves to, so a caller list that reads differently twice cannot make Create report a different document.

DID Core id and URI syntax (§3.2, §5.1.3, §5.2, §5.4)

• DIDs and DID URLs follow RFC 3986: \z anchoring, no raw whitespace, controls, non-ASCII or format characters, and no malformed % escapes.
• Read and write both check:
  • verification-method ids and relationship references (absolute or relative DID URLs, §3.2.2: #key-1, /keys/1, keys/1#k, ?versionId=1#key-1; network-path //host is rejected);
  • service ids (URIs or relative DID URLs) and their uniqueness;
  • serviceEndpoint strings and alsoKnownAs entries (absolute URIs: RFC 3986 hier-part, IPv6/IPvFuture-only IP-literals).
• Ids are compared after RFC 3986 §5 resolution against the DID (authority = method:method-specific-id), so keys/1#k, /keys/1#k and <did>/keys/1#k are one id. The one documented exception is NetDid's bare shorthand ([High] did:webvh implicit #files / #whois services are neither materialized in the resolved document nor resolvable as DID URLs #136): one plain segment files means #files.
• That one comparison is shared by duplicate detection, the dereferencer and DefaultVerificationRelationshipResolver. Its cost is linear even for a megabyte-long did:peer DID, and an id of another DID never matches by fragment alone.
• The dereferencer selects <did>/path#fragment for a DID URL with a path and a fragment (it used to ignore the path), and ?service= selects by fragment.
• did:peer:2 Create and Resolve run the same producer checks. did:peer:4 rewrites a template's placeholder DID in ids as in references, at a DID boundary.
• did:ethr UpdateAsync refuses service values and names ethr-did-resolver would drop. Prefix checks on untrusted text are ordinal (ICU ignores NUL).

Behavior changes

All of these apply to malformed, hostile, or non-conformant input only:

• Typed errors (invalidDidLog, invalidDid, JsonException, FormatException, EthereumInteractionException, ArgumentException) replace notFound and raw exceptions.
• Documents, DID URLs and did:ethr service values that break DID Core rules are rejected or dropped.
• Nested serviceEndpoint sets cannot be built.
• did:peer:2 Create refuses a member name that would resolve as another (t, s, r, a), the type dm, an absolute service id, and a key string containing .. It keeps explicit relative service ids instead of replacing them with #service.
• A JWK needs kty.
• Caller JsonSerializerOptions are locked on first use.
• did:ethr public API:
  • Erc1056EventParser.Parse can return the new FilteredEvent.
  • AbiDecoder.DecodeDelegateChangedData and DecodeAttributeChangedData return the strict-UTF-8 label, or null when it is not valid UTF-8.
  • AbiDecoder.TryDecodeBytes32AsUtf8 is new.

NetDidVersion is not bumped; the changes are listed under [Unreleased].

Verification

• Fail-first: every defect test failed against the pre-fix code of its round, by red-commit or by mutation of the committed fix. Positive controls pass.

• Build: Release, 0 warnings / 0 errors.

• Tests (vs main):

  Project	main	Now
  Core	439	748 (+309)
  WebVh	545	604 (+59)
  Ethr	491	529 (+38)
  Peer	48	114 (+66)
  Key	52	54 (+2)
  DI	26	26
  W3C conformance	233	233 (100%)

• Real-EVM suite: 18/18 with NETDID_ETHR_INTEGRATION=1.

• Samples: all five exit 0.

• Interop: the DIF did:webvh vector replay is identical before and after the id/URI round.

• Hygiene: git diff origin/main...HEAD --check and git diff --check are clean.

Note: unrequested "update by push" events moved this branch three times (10:07, 10:37, 10:38). Each pushed SHA was one of my local commits, and main and tags were untouched. The cause is not established; see the plan file.

🤖 Generated with Claude Code

[moisesja]

I would not approve this yet. Two reproducible P2 concerns remain: the new read-side endpoint restriction lets did:peer:4 Create succeed with a DID that immediately resolves as notFound, and the writer hardening still silently replaces malformed text inside caller-supplied JsonElements. Details and requested regression tests are inline.

The 20 commit subjects are clear. The 23-file scope is broad but coherent for this shared boundary defect; the main concern is incomplete coverage of the new guarantees, not arbitrary scope. Independent review found no additional blocker in the ethr or webvh changes.

Validation at e20ec80: Release build 0 warnings/errors; default suite 2,015 passed, 7 opt-in EVM tests skipped (Core 533, Key 52, Peer 56, WebVh 598, Ethr 506, DI 26, W3C 233, integration gate 11). Separate real-EVM run: 18/18 passed. All five offline samples passed; diff whitespace check clean; GitHub CI green. Those results do not cover the two failing targeted scenarios below.

[moisesja]

Final pass on the id/URI round: all findings fixed (2daf183..abf8d84)

The final adversarial pass over the id/URI round found nine defects (F1–F9). I then ran one more independent adversarial pass over those fixes. It found five more (A1–A5), and re-reading surfaced two of the same kind. All are fixed in this push, each with fail-first tests. Nothing is deferred.

Final pass (F1–F9)

Tests are in 76de5d0; 21 defect rows were red against 9633c6b.

• did:peer:2 Create and Resolve now run Core's full producer check (DidDocumentSerializer.ValidateForProduction).
  • A bare service beside the auto-generated #service is a duplicate.
  • Invalid service ids or endpoints are rejected: ArgumentException on Create, invalidDid on Resolve.
• did:webvh did.json: a did:web:<host>#whois (or #files) service is the override there, so no second implicit service is added. did.json is now built before signing in Create and Update. A counting signer pins that no signature is produced for a refused document.
• Fragment dereferencing compares ids with the shared normalization, so an id of another DID (did:other:1#x) no longer matches #x.
• IsAbsoluteUri follows RFC 3986 scheme ":" hier-part:
  • [/] are allowed only around an IP-literal host;
  • // always starts an authority (http://host:abc is rejected).
• The bare-id shorthand is one segment with no : / ? #. //evil.example/x, /files, ?service=x and a/b are no longer read as #….
• did:ethr:
  • Service names follow ethr-did-resolver's ^did/svc/(\w+)(/\w+)?(/\w+)?$, typed by the first segment, and every did/svc/ event is still counted.
  • JSON values with duplicate members keep the last value at the first position, as JSON.parse does.
  • UpdateAsync rejects AddServices/AddAttributes values the resolver would drop.
  • BrickProbeTests now plant their junk endpoints with a raw setAttribute, because NetDid itself no longer writes them.

Adversarial pass over those fixes (A1–A5)

Tests are in a839a31; 16 defect rows were red against 22f78f9.

#	Severity	Defect	Fix
A1	High	Duplicate detection built <did>#x per service. A did:peer DID embeds its document, so one hostile 446 KB did:peer:2 allocated 7 GB, and a ~1 MB one killed the process. (Introduced earlier in this PR; never released.)	DidParser.ComparableId compares own ids as #x, so cost follows the ids. Pinned by an allocation bound.
A2	Medium	The dereferencer rebuilt <did>#x on every comparison (14 GB measured). The relationship branch searched every VM once per entry.	Target built once; VM search hoisted.
A3	Low–Med	did:ethr decoded bytes32 labels as ASCII and counted events whose label isn't UTF-8. ethr-did-resolver filters those. The did/pub and did/svc prefix checks were culture-sensitive (ICU ignores NUL).	Strict-UTF-8 labels. Undecodable events become FilteredEvent: they link the history walk but are not counted and set no versionId/updated/nextVersionId. Prefix checks are ordinal. End-to-end test with real transactions on the emulator.
A4	Low	did:peer:4 rewrote a template's placeholder DID in references but not in ids, so its own authentication reference stopped dereferencing. The reference rewrite had no DID boundary.	Ids are rewritten too, only at a DID boundary.
A5	Low	The IP-literal grammar accepted [foo] and [:].	RFC 3986 IPv6address / IPvFuture. Timed linear on 100K-char hostile inputs.
—	Low	Same class as A3: culture-sensitive did:key: check on a webvh proof verificationMethod (returned a bogus key, never a match), and on the EcdsaSecp256k1 prefix used for context selection.	Ordinal.

Public API note (did:ethr):

• Erc1056EventParser.Parse can now return the new FilteredEvent.
• AbiDecoder.DecodeDelegateChangedData and DecodeAttributeChangedData return the strict-UTF-8 label, or null when it isn't valid UTF-8.
• AbiDecoder.TryDecodeBytes32AsUtf8 is new.

Verification

• Release build: 0 warnings, 0 errors.
• Tests:
  • Core 706, Ethr 529, WebVh 604, Peer 98, Key 54, DI 26.
  • W3C conformance 233 (100%).
  • Real-EVM 18/18 with NETDID_ETHR_INTEGRATION=1.
• All five samples exit 0, and git diff --check is clean.
• DIF did:webvh vector replay: identical to the 2daf183 baseline.
• Docs: the CHANGELOG [Unreleased] did:webvh: an unpaired-surrogate escape in a JSON member name resolves notFound instead of invalidDidLog #145 entry, the PRD (id/URI syntax, did:peer:4 contextualization, did:ethr label decoding), and the Review section in tasks/todo20261028-issue145.md.

The PR is ready for your review; I have not merged it.

🤖 Generated with Claude Code

[moisesja]

Re-reviewed at 0e8d868. Both original findings are fixed: nested endpoint sets are rejected and privately snapshotted, and malformed caller JsonElements now throw ArgumentException instead of publishing replacement text. I reran the original probes.

I still would not approve this revision. The expanded changes introduce one conformance regression and leave the new peer:2 producer validation bypassable. Both are reproduced through public APIs; the two inline comments give the exact triggers and requested tests.

Validation: Release build 0 warnings/errors; 2,261 default tests passed, with 7 opt-in EVM cases skipped (Core 706, Key 54, Peer 98, WebVh 604, Ethr 529, DI 26, W3C 233, integration gate 11). Separate real-EVM run 18/18; all five offline samples exit 0; CI run 40754984649 succeeded. Independent review found no additional actionable issue in the new ethr/webvh changes.

The PR has grown to 62 commits and 44 files. Commit subjects remain specific, and CHANGELOG/PRD explain the expansion, but please update the PR body: it still describes the initial scope and test counts, omitting the new URI/parser, identifier-rewriting and ethr service-decoding changes. Minor hygiene: git diff origin/main...HEAD --check also reports new blank lines at EOF in the Core Issue145 test file and DidKeyMethodTests.cs.

[moisesja]

Review round 2: addressed (0e8d868..6f05f34)

Thanks for the re-review. Both P2 findings and the hygiene notes are fixed. Details are in the two inline threads.

• Relative DID URLs (P2): DID Core §3.2.2 relative path, query and fragment references are accepted on read and write, and compared by RFC 3986 §5 resolution against the DID. The one documented exception is the bare shorthand from [High] did:webvh implicit #files / #whois services are neither materialized in the resolved document nor resolvable as DID URLs #136. DefaultVerificationRelationshipResolver now uses the same comparison.
• did:peer Create snapshot (P2): did:peer:2 Create reads its input once and returns Resolve(encodedDid). did:peer:4 Create builds from the JSON it hashed.
• Hygiene:
  • The two EOF blank lines are removed.
  • net-did-verify now also runs git diff origin/main...HEAD --check. My gate only checked the working tree, which is how they got past it.
  • The PR description is updated to the current scope, including this round.

Adversarial pass over these fixes: five confirmed findings, all fixed with fail-first tests.

• ?service= must select by fragment. My first version had regressed ?service=agent/v1.
• A DID URL with a path and a fragment ignored the path (pre-existing). <did>/k#x could return #x, a different key in a different relationship.
• A non-URI id such as //evil#k could equal a valid one in the relationship resolver.
• A did:peer:2 key string containing . could inject segments.
• did:peer:2 abbreviations didn't follow the DIF spec: one table for names and values, no recursion into serviceEndpoint. NetDid couldn't produce or correctly resolve the spec's own example DID, and it silently replaced explicit relative service ids.

Behavior changes this round (CHANGELOG has the details):

• relative DID URLs are now valid;
• did:peer:2 Create refuses names that would resolve as something else (t/s/r/a), the type dm, absolute service ids and key strings with .;
• explicit relative service ids are kept.

Fail-first: 26 defect rows were red against 0e8d868, and 20 more against 3e70eb0. Controls and your negative rows pass.

Verification at 6f05f34:

• Release build: 0 warnings, 0 errors.
• Tests: Core 748, Peer 114, WebVh 604, Ethr 529, Key 54, DI 26, W3C 233 (100%).
• Real-EVM: 18/18.
• All five samples exit 0.
• DIF did:webvh vector replay is identical to the baseline.
• git diff origin/main...HEAD --check is clean.

Ready for another look. I haven't merged.

🤖 Generated with Claude Code

[moisesja]

Both review-round-2 findings are fixed and verified at 6f05f34. The fixes were already on the branch when this implementation pass began; I validated them rather than duplicate them.

• Relative path/query DID URLs now pass the original public read/write probes, with shared RFC 3986 comparison and negative controls retained.
• peer:2 validates and returns the document decoded from its exact encoded DID. The original split-indexer/enumerator regression now rejects the invalid input; normal Create/Resolve equality controls pass. peer:4 uses its encoded JSON too.

No remaining blocker found in one independent adversarial pass over these fixes. The two original round-1 findings remain fixed as well. I am resolving my four review threads.

Fresh verification: Release build 0 warnings/errors; 2,319 default tests passed (7 opt-in EVM cases skipped), separate real-EVM suite 18/18, W3C 233/233, all five offline samples exit 0, both diff checks clean. CI 40762475189 passed. Ready for merge from my review perspective; not merged.