feat!: did-resolver interface + spec-conformant query parameters (#89, #130) - #4
feat!: did-resolver interface + spec-conformant query parameters (#89, #130)#4brianorwhatever wants to merge 18 commits into
Conversation
…hape - delete removed verificationMethod resolution tests - convert resolution-failure assertions from throws to error results - add getResolver integration + resolver-result/verifier unit tests - apply biome formatting
URLSearchParams applies application/x-www-form-urlencoded rules where '+' decodes to a space. A DID URL query is a URI component (RFC 3986) where '+' is literal, so a versionTime with a +HH:MM timezone offset was corrupted into an Invalid Date. Parse the query manually with decodeURIComponent instead.
71d3b8c to
c340fc0
Compare
|
@macroscope review |
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
Just FYI for future @mentions, I'm Review in progress. Results will be posted as check runs when complete. |
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Major version bump (v3.0.0) introducing W3C did-resolver interface integration with breaking API changes. New feature capabilities and an unresolved review comment about incorrect handling of DID URL paths/fragments warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- resolver.ts: reject NaN versionNumber / Invalid Date versionTime as invalidDidUrl - resolver-result.ts: tighten mapErrorToCode so validation errors embedding '404'/'not found' (attacker-controlled log data) aren't misclassified as notFound; only genuine log-fetch absence maps to notFound - cli.ts: surface resolution errors in update/deactivate instead of TypeError on meta.updateKeys - examples: return the resolution error before file-serving fallback (avoid https:/// fetch) - tests: cover the new validation paths; reclassify invalid-update-key read as invalidDid
|
@macroscope-app review |
|
Review in progress. Results will be posted as check runs when complete. |
- resolver-result.ts: preserve a valid earlier document when returned alongside a warning-level error (e.g. explicit version selector resolves while a later entry fails witness verification) instead of dropping it - resolver-result.ts: toErrorResult now synthesizes RFC9457 problemDetails (type/title/detail) so fetch/validation/invalidDidUrl errors keep structured error info - cli.ts: resolve a deactivated DID's identifier from the log instead of printing an empty id - tests: cover preserved-doc and problemDetails behavior
|
@macroscope-app review |
|
Review in progress. Results will be posted as check runs when complete. |
| } | ||
|
|
||
| // parsed.did is the bare DID without query/fragment. | ||
| return resolveDID(parsed.did, selector); |
There was a problem hiding this comment.
🟡 Medium src/resolver.ts:79
getResolver() ignores parsed.path and parsed.fragment when building the selector, so DID URLs with paths or fragments (e.g., did:webvh:...#key-1) return the full DID document instead of the dereferenced resource. did-resolver passes these components to method resolvers specifically for dereferencing, so discarding them produces incorrect results for valid inputs.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/resolver.ts around line 79:
`getResolver()` ignores `parsed.path` and `parsed.fragment` when building the selector, so DID URLs with paths or fragments (e.g., `did:webvh:...#key-1`) return the full DID document instead of the dereferenced resource. `did-resolver` passes these components to method resolvers specifically for dereferencing, so discarding them produces incorrect results for valid inputs.
There was a problem hiding this comment.
Intentionally not changing this. Per the W3C DID spec, resolve() returns the DID document; dereferencing a path/fragment to a specific resource is a separate operation (dereferenceURL). did-resolver method resolvers (did:web, did:ethr, …) all return the full DID document and leave fragment/path handling to the caller — so discarding parsed.path/parsed.fragment here is the correct, standard behavior. did:webvh file/path dereferencing is handled separately by the resolver servers (see the /resolve/:id/* routes in the examples).
- resolver-result.ts: add 'internalError' code; classify 5xx/network/transport failures as internalError instead of invalidDid (document-validation errors stay invalidDid). Synthesize problemDetails for it too. - examples/express-resolver.ts: map resolution error codes to HTTP status (notFound->404, invalid*->400, internalError->500) instead of a blanket 400. - tests: cover internalError classification; update network-failure expectation.
|
@macroscope-app review |
|
Review in progress. Results will be posted as check runs when complete. |
…rning error Only bail on resolution error when didDocument is null; a valid earlier version can be returned alongside warning-level error metadata (consistent with toResolutionResult preserving the doc).
|
@macroscope-app review |
|
Review in progress. Results will be posted as check runs when complete. |
A valid DID served from an unauthorized (401/403), gone (410), rate-limited (429), or failing (5xx) endpoint is a resolver-side internal error, not a document-validation failure. 404/empty-log stay notFound; document-validation errors stay invalidDid.
|
@macroscope-app review |
|
Review in progress. Results will be posted as check runs when complete. |
|
Superseded by the upstream PR at decentralized-identity#138 (same branch, rebased onto current DIF |
Summary
Adopts the standard W3C
did-resolverinterface and settles the open query-parameter questions. Breaking change — major version bump to 3.0.0.Resolves decentralized-identity#89 (support did-resolver interface) and decentralized-identity#130 (query parameter support).
What changed
resolveDID/resolveDIDFromLogreturn the standard resolution shape{ didResolutionMetadata, didDocument, didDocumentMetadata }— a clean break from{ did, doc, meta, controlled }. They no longer throw on failure; errors come back as a result withdidDocument: nulland a standarderrorcode.getResolver(config?)returns{ webvh: DIDResolver }, registrable in adid-resolverResolver—did:webvhis now a drop-in alongsidedid:web,did:ethr, etc. Works zero-config via a built-indefaultVerifier(Ed25519), overridable withgetResolver({ verifier }).verificationMethodresolution removed entirely — implementation-specific, not in any spec, and a footgun (returned a historical document version).versionId/versionTime/versionNumber; any combination returnsinvalidDidUrl.updateDID/deactivateDIDkeep using the rich per-version core resolver unchanged.New modules
src/verifier.ts—defaultVerifier(Ed25519 via@stablelib/ed25519).src/resolver-result.ts— pure mapping (toResolutionResult,toErrorResult,assertSingleVersionSelector,mapErrorToCode,InvalidDidUrlError) + result types.src/resolver.ts—getResolver(); parses DID-URL query params, enforces selector rules, returnsDIDResolutionResult.Implementation note
did-resolverexposes?-query parameters as the raw, undecodedparsed.querystring (only;-matrix params populateparsed.params). Since the did:webvh spec uses?query parameters,getResolverparsesparsed.queryitself. It decodes per RFC 3986 (decodeURIComponent,+is literal) rather thanURLSearchParams(form-encoding,+→ space) so aversionTimewith a+HH:MMtimezone offset isn't corrupted.Testing
defaultVerifier, the mapping layer, and the mutual-exclusivity guard.getResolver()in a realdid-resolverResolver— fresh DID,?versionId/?versionNumber/?versionTime, selector-combination →invalidDidUrl, not-found, deactivated, zero-config.verificationMethodresolution tests.Gates (all green locally)
bun test— 273 pass / 0 failbunx biome ci .— cleanbun run check— cleanbun run build— succeeds🤖 Generated with Claude Code
Note
Implement W3C did-resolver interface and spec-conformant query parameters for did:webvh
resolveDIDandresolveDIDFromLognow returnDIDResolutionResult(withdidDocument,didDocumentMetadata,didResolutionMetadata) instead of a custom{ did, doc, meta }shape; errors are reported viadidResolutionMetadata.errorrather than thrown exceptions.src/resolver.tswith agetResolverfactory for registering did:webvh with thedid-resolverlibrary, including robust RFC 3986 decoding of version selector query parameters (versionId,versionTime,versionNumber).src/resolver-result.tswith helperstoResolutionResult,toErrorResult,mapErrorToCode, andassertSingleVersionSelector; standardized error codes includeinvalidDid,invalidDidUrl,notFound, andinternalError.src/verifier.tswithdefaultVerifier, a built-in Ed25519 verifier used automatically when callers omit a verifier option.verificationMethodas a resolution selector from both v0.5 and v1.0 method implementations and fromResolutionOptions.{ did, doc, meta }destructuring toDIDResolutionResultfields and from catching thrown errors to checkingdidResolutionMetadata.error.Macroscope summarized 5e08585.