feat: add Verifiable Presentation sign/verify + fragments - #154
Merged
Conversation
Consume @trustvc/w3c-vc 2.3.0 and expose an opinionated VP surface: - signW3CPresentation / verifyW3CPresentation (src/w3c/presentation.ts) that ENFORCE trustvc policies: fullDisclosure (auto-derive underived credentials), holder binding (signer DID == holder == every credentialSubject.id), a mandatory lifetime, and a locked v2 envelope. TransferableRecords credentials are blocked. - Three VP verification fragments (src/verify/fragments/presentation) wired into verifyDocument(): W3CVpSignatureIntegrity (requires a holder proof + enforces holder binding), W3CVpCredentialStatus (VP expiry + embedded revocation), W3CVpIssuerIdentity (embedded issuers resolve). - Bump @trustvc/w3c* 2.2.0 -> 2.3.0. - Tests (28): create/sign/verify, invalid-key matrix, TransferableRecords block, underived auto-disclosure, missing subject id, different (did:web) issuer, v1.1-in-v2 envelope, revoked/not-revoked status, full verifyDocument() pipeline. - CLAUDE.md: repo guidance, VP policies, gotchas, and a self-maintenance rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds public W3C VP signing and verification wrappers with enforced policies, aggregate VP verification fragments, routing integration, comprehensive tests, dependency updates, repository guidance, and updated Amoy test endpoints. ChangesW3C Verifiable Presentation support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant signW3CPresentation
participant createPresentation
participant signPresentation
participant verifyW3CPresentation
participant verifyPresentation
Caller->>signW3CPresentation: credentials, keyPair, lifetime
signW3CPresentation->>createPresentation: enforced VP policies
createPresentation-->>signW3CPresentation: presentation
signW3CPresentation->>signPresentation: presentation and keyPair
signPresentation-->>Caller: signed VP
Caller->>verifyW3CPresentation: signed VP and options
verifyW3CPresentation->>verifyPresentation: holder-binding options
verifyPresentation-->>Caller: verification result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/__tests__/w3c/presentation.test.ts (2)
11-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting
assertDefinedinto a shared test helper.The same helper is likely needed by the other new VP suite (
src/__tests__/w3c/vpFragments.test.ts, which currently uses!), so a shared util avoids per-file duplication.As per coding guidelines, "Do not use
!non-null assertions in tests; use theassertDefinedhelper instead."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/w3c/presentation.test.ts` around lines 11 - 15, Extract assertDefined into a shared test utility, then import and reuse it in presentation.test.ts and vpFragments.test.ts. Replace the non-null assertions in vpFragments.test.ts with assertDefined calls while preserving the existing test behavior.Source: Coding guidelines
296-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis test depends on live resolution of
did:web:trustvc.github.io:did:1.Network-dependent assertions make CI flaky when the hosted DID document is unreachable or rotated. Consider a stubbed document loader for the issuer DID while keeping one integration-tagged test for the live path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/w3c/presentation.test.ts` around lines 296 - 327, Update the test around signW3C and verifyW3CPresentation to use a stubbed document loader or equivalent fixture for ISSUER_DID, so its credential verification does not depend on live did:web resolution. Preserve the existing issuer/holder distinction and add or retain a separately integration-tagged test for validating the live resolution path.src/__tests__/w3c/vpFragments.test.ts (1)
161-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
verifyDocument(signed)is called withoutrpcProviderUrl/provider.Per
src/core/verify.ts(lines 51-81) the builder constructs a defaultJsonRpcProvider, which can add slow/failing RPC calls in CI even though only VP fragments are asserted. Consider passing an explicit provider or stub.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/w3c/vpFragments.test.ts` around lines 161 - 176, The full verifyDocument pipeline test invokes verifyDocument without an explicit RPC configuration, allowing the default JsonRpcProvider to make unnecessary CI calls. Update the test around verifyDocument(signed) to pass a deterministic explicit provider or rpcProviderUrl stub, while preserving the existing VP fragment assertions.src/verify/fragments/presentation/w3cVpVerifier.ts (1)
205-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the nested ternary for readability.
♻️ Proposed extraction
- const raw = cred.credentialStatus; - const statuses = (Array.isArray(raw) ? raw : raw ? [raw] : []) as CredentialStatus[]; + const raw = cred.credentialStatus; + if (!raw) return []; + const statuses = (Array.isArray(raw) ? raw : [raw]) as CredentialStatus[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/verify/fragments/presentation/w3cVpVerifier.ts` at line 205, Refactor the statuses assignment to remove the nested ternary: first normalize raw into an array using clear conditional logic, preserving the existing handling for arrays, single truthy values, and absent values, then cast the result to CredentialStatus[].Source: Linters/SAST tools
src/w3c/presentation.ts (1)
71-78: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider wrapping
signPresentationin the same try/catch ascreatePresentation.Create errors are normalized to
{ error }, but an unexpected throw fromsignPresentationpropagates as a rejection, so the documented result contract isn't uniform.♻️ Proposed normalization
- return signPresentation(presentation, keyPair, enforced); + try { + return await signPresentation(presentation, keyPair, enforced); + } catch (err: unknown) { + return { error: err instanceof Error ? err.message : 'Failed to sign the presentation.' }; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/w3c/presentation.ts` around lines 71 - 78, Update the presentation creation and signing flow around createPresentation and signPresentation so both operations are covered by the same try/catch. Normalize any signing exception to the existing { error } result shape, preserving Error.message when available and the current fallback message for non-Error throws.src/verify/verify.ts (1)
39-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVP fragments are added to
w3cVerifiersbut not to the exportedverifiersmap.
verifiers.documentIntegrity/documentStatus/issuerIdentity(lines 47-63) lists the VC-level w3c fragments; consumers building custom pipelines from that map won't see the VP ones. Confirm the omission is intentional.Also applies to: 76-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/verify/verify.ts` around lines 39 - 43, Update the exported verifiers map in verify.ts to include w3cVpCredentialStatus, w3cVpIssuerIdentity, and w3cVpSignatureIntegrity alongside the existing VC-level fragments. Ensure consumers using verifiers can access the same VP fragments already registered in w3cVerifiers, without removing the current entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/w3c/vpFragments.test.ts`:
- Around line 44-48: Replace the non-null assertions on signCredential and
deriveCredential results with assertDefined calls in both affected sites:
src/__tests__/w3c/vpFragments.test.ts lines 44-48 and 111-113. Apply this to
s.signed and the derived credential value while preserving the existing test
flow.
In `@src/verify/fragments/presentation/w3cVpVerifier.ts`:
- Around line 200-216: Update the embedded-credential status processing around
getCredentials and statusChecks so any credentialStatus entry with an
unsupported type is surfaced as an ERROR/INVALID verification result rather than
filtered out. Preserve verification through verifyCredentialStatus for
BitstringStatusListEntry and StatusList2021Entry, and ensure every embedded
credential’s status is accounted for so unsupported types cannot yield VALID
with checked: 0.
- Around line 42-65: Update checkVpHolderBinding to inspect every
credentialSubject rather than only the first returned by getFirstSubject. For
array-valued subjects, iterate each subject, require a credentialSubject.id, and
enforce exact equality with the holder/signer DID; preserve the existing
behavior for single-object subjects and report the corresponding credential
index when validation fails.
- Around line 267-277: Update the issuer validation around getCredentials and
the issuers mapping so every embedded credential must have a resolvable issuer.
Detect any credential whose issuer is missing or cannot be read and return the
existing INVALID ISSUER_IDENTITY result, while preserving the current successful
path only when all credentials resolve.
---
Nitpick comments:
In `@src/__tests__/w3c/presentation.test.ts`:
- Around line 11-15: Extract assertDefined into a shared test utility, then
import and reuse it in presentation.test.ts and vpFragments.test.ts. Replace the
non-null assertions in vpFragments.test.ts with assertDefined calls while
preserving the existing test behavior.
- Around line 296-327: Update the test around signW3C and verifyW3CPresentation
to use a stubbed document loader or equivalent fixture for ISSUER_DID, so its
credential verification does not depend on live did:web resolution. Preserve the
existing issuer/holder distinction and add or retain a separately
integration-tagged test for validating the live resolution path.
In `@src/__tests__/w3c/vpFragments.test.ts`:
- Around line 161-176: The full verifyDocument pipeline test invokes
verifyDocument without an explicit RPC configuration, allowing the default
JsonRpcProvider to make unnecessary CI calls. Update the test around
verifyDocument(signed) to pass a deterministic explicit provider or
rpcProviderUrl stub, while preserving the existing VP fragment assertions.
In `@src/verify/fragments/presentation/w3cVpVerifier.ts`:
- Line 205: Refactor the statuses assignment to remove the nested ternary: first
normalize raw into an array using clear conditional logic, preserving the
existing handling for arrays, single truthy values, and absent values, then cast
the result to CredentialStatus[].
In `@src/verify/verify.ts`:
- Around line 39-43: Update the exported verifiers map in verify.ts to include
w3cVpCredentialStatus, w3cVpIssuerIdentity, and w3cVpSignatureIntegrity
alongside the existing VC-level fragments. Ensure consumers using verifiers can
access the same VP fragments already registered in w3cVerifiers, without
removing the current entries.
In `@src/w3c/presentation.ts`:
- Around line 71-78: Update the presentation creation and signing flow around
createPresentation and signPresentation so both operations are covered by the
same try/catch. Normalize any signing exception to the existing { error } result
shape, preserving Error.message when available and the current fallback message
for non-Error throws.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 759ff572-e886-4ca4-8a3f-28c413d01dd6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
CLAUDE.mdpackage.jsonsrc/__tests__/core/verify.pol.test.tssrc/__tests__/w3c/presentation.test.tssrc/__tests__/w3c/vpFragments.test.tssrc/verify/fragments/index.tssrc/verify/fragments/presentation/w3cVpVerifier.tssrc/verify/verify.tssrc/w3c/index.tssrc/w3c/presentation.ts
Review fixes to the VP support: - w3cVpSignatureIntegrity: holder binding now checks EVERY credentialSubject of every credential (was only the first). - w3cVpCredentialStatus: surface unsupported credentialStatus types as ERROR instead of silently dropping them (revocation must not be silently unenforced). - w3cVpIssuerIdentity: a credential with no issuer now fails (INVALID) instead of being dropped by filter(Boolean). - fragments/index.ts: re-export the VP verifiers via `export … from` (Sonar S7763). - Extract a toArray helper (removes a nested ternary, Sonar S3358). - Tests: use assertDefined instead of `!`; +2 tests (unsupported-status ERROR, missing-issuer INVALID). Regenerate core/verify.test snapshots for the 3 VP fragments now emitted (SKIPPED) on non-VP documents. Polygon public RPC deprecation: - Replace https://rpc-amoy.polygon.technology with https://polygon-amoy-bor-rpc.publicnode.com in the affected tests. Docs: - README: add a "Verifiable Presentations (VP)" section and renumber the rest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rifier
A VP has no top-level credentialStatus, so this VC-only verifier's test() matched
it and then reported INVALID ("not a valid SignedVerifiableCredential") — marking
every VP (even a valid one) as INVALID for DOCUMENT_STATUS. Exclude presentations
in test() so it SKIPs them; VP status is handled by W3CVpCredentialStatus.
Strengthen the full-pipeline test to assert a valid VP yields only VALID/SKIPPED
fragments (and W3CEmptyCredentialStatus is SKIPPED).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
w3c, w3c-vc, w3c-context and w3c-credential-status to ^2.4.0. w3c-issuer stays at ^2.3.0 (no 2.4.0 was published — latest is 2.3.0). Type-check and VP tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts`:
- Around line 22-30: Update the Verifiable Presentation guard in the
document-status verifier to first validate that the input is a non-null object
before using the `in` operator or accessing its properties. Ensure primitive
`document` values skip this VP-specific check without throwing, while preserving
the existing `types` and `verifiableCredential` detection for object inputs.
- Around line 29-30: Update the bypass condition in the document-status
verification logic to require both a type containing “VerifiablePresentation”
and the presence of “verifiableCredential”, matching the complete VP shape used
by isVpDocument(). Keep proof validation out of this routing check so only
documents satisfying both fields bypass the VC fragment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fae2b60-0e28-40d8-8ec9-60c9ca7a3faa
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.jsonsrc/__tests__/w3c/vpFragments.test.tssrc/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/tests/w3c/vpFragments.test.ts
…ations - Bump @trustvc/w3c and @trustvc/w3c-vc to ^2.4.1, which decouples credentialStatus field-format validation from _checkCredential's verify path. A malformed tokenNetwork.chainId now surfaces as a DOCUMENT_STATUS problem instead of a signature-integrity failure, so the TransferableRecords verifier reports it and the pol "chainId is missing" test passes without any trustvc workaround. - Harden the empty-credential-status VP guard (CodeRabbit): guard non-object input before the `in` operator, and require the full VP shape (type includes VerifiablePresentation AND has verifiableCredential) to skip, matching isVpDocument. - README: clarify that challenge/domain are OPTIONAL when signing a VP (a plain assertionMethod proof needs only a lifetime); add a minimal no-challenge example. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
RishabhS7
approved these changes
Jul 31, 2026
nghaninn
pushed a commit
that referenced
this pull request
Jul 31, 2026
## [2.15.0](v2.14.1...v2.15.0) (2026-07-31) ### Features * add Verifiable Presentation sign/verify + fragments ([#154](#154)) ([d8d51f1](d8d51f1))
|
🎉 This PR is included in version 2.15.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Summary by CodeRabbit