fix: updated empty credential verifier check#110
Conversation
WalkthroughReplaced W3C signature verification with a boolean type-guard check using Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Verifier as w3cEmptyCredentialStatus.verify
note over Verifier: Uses isSignedDocument(doc) boolean guard
Caller->>Verifier: verify(document)
alt isSignedDocument(document) == true
Verifier-->>Caller: { status: VALID, data: true }
else
Verifier-->>Caller: { status: INVALID, data: false, error: "Document is not a valid SignedVerifiableCredential" }
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts (1)
25-32: Guard against null/undefined documents in test() to avoid runtime TypeError.Accessing
doc.credentialStatuswithout optional chaining will throw whendocumentisnull/undefined. Maketestnull-safe.Apply this diff:
- test: (document: unknown) => { - const doc = document as SignedVerifiableCredential; - return ( - !!doc.credentialStatus === false || - (Array.isArray(doc.credentialStatus) && doc.credentialStatus.length === 0) || - Object.keys(doc.credentialStatus)?.length === 0 - ); - }, + test: (document: unknown) => { + const cs = (document as Partial<SignedVerifiableCredential> | null | undefined)?.credentialStatus; + return ( + !cs || + (Array.isArray(cs) && cs.length === 0) || + (!Array.isArray(cs) && typeof cs === 'object' && cs !== null && Object.keys(cs).length === 0) + ); + },
🧹 Nitpick comments (1)
src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts (1)
7-10: DRY: use a shared isSignedVerifiableCredential helper instead of duplicating.This guard appears elsewhere. Centralize it (e.g.,
src/verify/utils/isSignedVerifiableCredential.ts) and import here for consistency and easier updates.Example helper:
// src/verify/utils/isSignedVerifiableCredential.ts import { SignedVerifiableCredential } from '@trustvc/w3c-vc'; export function isSignedVerifiableCredential(document: unknown): document is SignedVerifiableCredential { return typeof document === 'object' && document !== null && 'proof' in (document as object); }Then:
- function isSignedVerifiableCredential(document: unknown): document is SignedVerifiableCredential { - return typeof document === 'object' && document !== null && 'proof' in document; - } + import { isSignedVerifiableCredential } from '../../../utils/isSignedVerifiableCredential';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts (3)
src/verify/fragments/document-status/w3cCredentialStatus.ts (2)
document(57-117)document(41-55)src/verify/fragments/document-integrity/ecdsaW3CSignatureIntegrity.ts (2)
document(33-93)isSignedVerifiableCredential(10-12)src/verify/fragments/document-integrity/w3cSignatureIntegrity.ts (1)
document(26-47)
| verify: async (document: unknown) => { | ||
| const doc = document as SignedVerifiableCredential; | ||
| const verificationResult = await verifyW3CSignature(doc, verifierOptions); | ||
| if (verificationResult.verified) { | ||
| const verificationResult = isSignedVerifiableCredential(doc); | ||
| if (verificationResult) { | ||
| return { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Keep verify(document, verifierOptions) signature and tighten naming.
Maintain the second parameter for backward compatibility and rename the boolean for clarity.
- verify: async (document: unknown) => {
+ verify: async (document: unknown, _verifierOptions: VerifierOptions) => {
const doc = document as SignedVerifiableCredential;
- const verificationResult = isSignedVerifiableCredential(doc);
- if (verificationResult) {
+ const isSigned = isSignedVerifiableCredential(doc);
+ if (isSigned) {
return {
type,
name,
data: true,
status: 'VALID',
};
} else {
return {
type,
name,
data: false,
reason: {
- message: 'Document is not a valid SignedVerifiableCredential',
+ message: 'Document is not a valid SignedVerifiableCredential',
},
status: 'INVALID',
};
}
},Run to confirm signature parity across fragments:
Also applies to: 50-55
🏁 Script executed:
#!/bin/bash
# Verify other fragments still accept verifierOptions
rg -nP -C1 "verify:\s*async\s*\(\s*document:\s*unknown,\s*verifierOptions" src/verify
# Check this fragment’s usage and name occurrences
rg -n "W3CEmptyCredentialStatus|w3cEmptyCredentialStatus" -C2Length of output: 5130
Add missing verifierOptions parameter & rename verificationResult
In src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts (lines 34–38, 50–55):
- Change
to
verify: async (document: unknown) =>
verify: async (document: unknown, _verifierOptions: VerifierOptions) =>
- Rename
to
const verificationResult = isSignedVerifiableCredential(doc); if (verificationResult) { … }
const isSigned = isSignedVerifiableCredential(doc); if (isSigned) { … }
🤖 Prompt for AI Agents
In src/verify/fragments/document-status/w3cEmptyCredentialStatus/index.ts around
lines 34–38 and 50–55, the verify function is missing the verifierOptions
parameter and the variable name verificationResult is unclear; update the
function signature to accept (document: unknown, _verifierOptions:
VerifierOptions) and rename the local const verificationResult to isSigned (and
update its uses) where you call isSignedVerifiableCredential(doc) so the
parameter is present for future use and the boolean variable name accurately
reflects its purpose.
|
## [2.0.4](v2.0.3...v2.0.4) (2025-08-28) ### Bug Fixes * updated empty credential verifier check ([#110](#110)) ([b7f99c4](b7f99c4))
|
🎉 This PR is included in version 2.0.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor