feat: add W3C Verifiable Presentation signing - #32
Merged
Conversation
Add `trustvc vp-sign`, which creates and signs a presentation so a holder can
prove they own credentials issued to them, and route presentations through the
existing `verify` command.
vp-sign
- The credentials prompt takes a directory, a single file, or comma-separated
files. A directory presents every file in it, unfiltered — anything that is
not presentable is reported by the signing step, which names the file rather
than trustvc's positional "credential at index 2". Dot-files and
sub-directories are skipped as OS noise.
- The holder DID is read from the key pair and printed, not prompted. trustvc
enforces that the signing key's DID IS the holder, so any other answer could
only fail. A key pair with no controller (the bare keypair.json from
key-pair-generation) is rejected up front, pointing at did-web.
- Lifetime is mandatory at the trustvc layer, so it is always asked for:
seconds from now, or an explicit validUntil.
verify
- One verify command for every document type; there is deliberately no
vp-verify. Presentations are detected by shape, ignoring `proof`, so an
unsigned presentation is reported INVALID rather than skipped.
- A valid presentation adds one line, "N embedded credentials verified.",
because the three fragment lines read identically over one credential or
five. Failure output is unchanged.
- No challenge support: an anti-replay challenge can only be checked by the
verifier that issued it, and verify has no way to take one, so presentations
carry an assertionMethod proof.
Also:
- Bump @trustvc/trustvc to ^2.15.1, which makes credentialStatus and expiry
non-strippable at issuance (w3c-vc 2.4.2).
- Suppress the punycode DeprecationWarning in main.ts. Transitive deps still
require Node's deprecated module and the warning printed mid-prompt garbled
the interactive display.
- Anchor the command-output entries in .gitignore. Unanchored, `didKeyPairs.json`
and `signed_vc.json` matched at any depth and would silently swallow
identically-named files under tests/fixtures/. Adds the missing signed_vp.json.
- Add CLAUDE.md, matching the trustvc and w3c repos.
Tests: 35 new (22 unit, 13 real-crypto end-to-end). The integration tests mint
their own presentations at runtime rather than using stored fixtures, because a
presentation always carries an expiry. tests/fixtures/vp/ holds a manual-test kit
— generator and README only; the generated files carry throwaway private keys and
are gitignored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe CLI adds interactive W3C Verifiable Presentation signing and verification. It introduces presentation-specific types and detection, validates holder and credential inputs, reports embedded credential results, adds cryptographic integration tests and fixtures, updates the TrustVC dependency, and documents the new workflows. ChangesVerifiable Presentation support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
actor Operator
participant vp-sign
participant signW3CPresentation
participant Filesystem
Operator->>vp-sign: Provide credential paths and holder key pair
vp-sign->>Filesystem: Read credential and key-pair files
vp-sign->>signW3CPresentation: Submit credentials, holder, and lifetime
signW3CPresentation-->>vp-sign: Return signed presentation
vp-sign->>Filesystem: Write signed_vp.json
sequenceDiagram
actor Operator
participant verify
participant isVerifiablePresentation
participant verifyDocument
Operator->>verify: Provide a presentation document
verify->>isVerifiablePresentation: Detect presentation shape
isVerifiablePresentation-->>verify: Return document classification
verify->>verifyDocument: Verify presentation integrity and status
verifyDocument-->>verify: Return verification fragments
verify-->>Operator: Report verification results and credential count
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/commands/w3c/vp-sign.ts (1)
225-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuild the output path with
path.join.Line 225 concatenates the directory and file name with a literal
/. The module already importspathand usespath.joinat line 61. Usepath.joinfor consistency and correct separators.If you apply this change, update the assertion at tests/commands/w3c/vp-sign.test.ts line 378:
path.join('.', 'signed_vp.json')returnssigned_vp.json, not./signed_vp.json.♻️ Proposed refactor
- const signedVpPath = `${outputPath}/signed_vp.json`; + const signedVpPath = path.join(outputPath, 'signed_vp.json'); writeFile(signedVpPath, signed, true);🤖 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/commands/w3c/vp-sign.ts` around lines 225 - 227, Update the signedVpPath construction in the VP signing flow to use the imported path.join with outputPath and signed_vp.json instead of literal slash concatenation. Adjust the corresponding assertion in the VP signing test to expect path.join('.', 'signed_vp.json') behavior, which produces signed_vp.json without a ./ prefix.tests/commands/w3c/vp.integration.test.ts (1)
132-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dependency on the previous test's output file.
This test reads
signed_vp.jsonleft by an earlier test. If it runs in isolation,sign()returnsundefinedand line 138 passes for the wrong reason. Assert that no file was written for this run instead.♻️ Proposed refactor
it('refuses to sign when the holder does not match the credential subject', async () => { - const vp = await sign({ holder: 'did:example:someone-else' }); + const signedVpPath = path.join(outputPath, 'signed_vp.json'); + fs.rmSync(signedVpPath, { force: true }); + const vp = await sign({ holder: 'did:example:someone-else' }); - // Nothing was written for this run, so the file still holds the previous test's VP. expect(signaleErrorMock).toHaveBeenCalled(); expect(String(signaleErrorMock.mock.calls[0][0])).toMatch(/does not match the holder/); - expect(vp?.holder).not.toBe('did:example:someone-else'); + // Nothing is written when signing fails. + expect(vp).toBeUndefined(); }, 60000);🤖 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 `@tests/commands/w3c/vp.integration.test.ts` around lines 132 - 139, Update the test case “refuses to sign when the holder does not match the credential subject” to assert that sign() returns undefined when signing is rejected, rather than checking vp.holder from the previous output file. Keep the existing error-log assertions and ensure the test verifies no VP was produced for the current run.
🤖 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 `@CLAUDE.md`:
- Around line 17-23: Update the fenced blocks in CLAUDE.md lines 17-23 and
README.md lines 468-473 to use the text language identifier on their opening
fences, preserving the existing block contents.
In `@tests/commands/w3c/vp.integration.test.ts`:
- Around line 247-253: Add the repository’s supported secret-scanner allowlist
annotation to the published test key value in the hostedKey fixture, targeting
the public test key flagged by Betterleaks. Keep the real did:web test key
unchanged and limit the annotation to this intentional fixture value.
In `@tests/fixtures/vp/README.md`:
- Around line 32-35: Update the introductory statement in README.md to limit
offline-verification claims to fixtures that avoid status-list fetching and
issuer DID resolution. Explicitly document revoked.json and
unresolvable_issuer.json as exceptions requiring network-backed status or issuer
resolution, while preserving the existing did:key and holder-binding explanation
for the remaining fixtures.
- Around line 20-25: Add the text language identifier to both fenced code blocks
in the README, including the directory-listing block and the path-only block, so
each opening fence uses text while preserving their contents.
---
Nitpick comments:
In `@src/commands/w3c/vp-sign.ts`:
- Around line 225-227: Update the signedVpPath construction in the VP signing
flow to use the imported path.join with outputPath and signed_vp.json instead of
literal slash concatenation. Adjust the corresponding assertion in the VP
signing test to expect path.join('.', 'signed_vp.json') behavior, which produces
signed_vp.json without a ./ prefix.
In `@tests/commands/w3c/vp.integration.test.ts`:
- Around line 132-139: Update the test case “refuses to sign when the holder
does not match the credential subject” to assert that sign() returns undefined
when signing is rejected, rather than checking vp.holder from the previous
output file. Keep the existing error-log assertions and ensure the test verifies
no VP was produced for the current run.
🪄 Autofix
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: 0520d0fd-d2b3-4e40-a215-522f043fd88b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.gitignoreCLAUDE.mdREADME.mdpackage.jsonsrc/commands/verify.tssrc/commands/w3c/vp-sign.tssrc/main.tssrc/types.tssrc/utils/document-verification.tstests/commands/w3c/vp-sign.test.tstests/commands/w3c/vp.integration.test.tstests/fixtures/vp/README.mdtests/fixtures/vp/generate-fixtures.cjs
- Correct an inaccurate claim in the fixtures README. "Every fixture verifies offline" was wrong: revoked.json fetches its status list from trustvc.github.io, and unresolvable_issuer.json only fails the way it should once DID resolution has been attempted. Both are named explicitly now. - Strengthen the holder-mismatch test. It asserted `vp?.holder` read back from whatever file an earlier test left behind, so in isolation `vp` was undefined and the assertion passed vacuously. It now clears the output first and asserts nothing was written. Verified by mutation: allowing the mismatch makes it fail. - Use path.join for the output path instead of string concatenation. The four sibling commands still concatenate; worth a sweep separately. - Note the provenance of the did:web test key and mark it gitleaks:allow. It is published material for a published test DID — the same key pair committed in trustvc's own sign, presentation and vpFragments fixtures. - Add language identifiers to three fenced code blocks (markdownlint MD040). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RishabhS7
approved these changes
Aug 7, 2026
|
🎉 This PR is included in version 1.2.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
New Features
Documentation
Bug Fixes
Tests