You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The file includes two definitions of createSignedVCSDJWT, which will cause a duplicate method error. Remove the old implementation or consolidate into a single method.
Add validation to ensure each type metadata document contains required fields before encoding. The current implementation blindly encodes any object or string without validating the structure or content.
if (typeMetadataGlueDocuments && typeMetadataGlueDocuments.length > 0) {
header.vctm = typeMetadataGlueDocuments.map((doc) => {
const docString = typeof doc === 'string' ? doc : JSON.stringify(doc);
+ if (!docString.trim()) {+ throw new SDJWTVCError('Type metadata document cannot be empty');+ }
return base64encode(docString);
});
}
Suggestion importance[1-10]: 4
__
Why: The suggestion adds basic validation for empty documents, which improves robustness. However, it only checks for empty strings and doesn't validate the actual structure or required fields as mentioned in the description.
Low
Add JSON parsing error handling
Add error handling around JSON parsing operations to prevent test failures when base64 decoding returns invalid JSON. The current code assumes successful parsing without handling potential JSON syntax errors.
-const decodedDoc1 = JSON.parse(base64decode((headerWithVctm.vctm as any[])[0]));+let decodedDoc1, decodedDoc2;+try {+ decodedDoc1 = JSON.parse(base64decode((headerWithVctm.vctm as any[])[0]));+ decodedDoc2 = JSON.parse(base64decode((headerWithVctm.vctm as any[])[1]));+} catch (error) {+ throw new Error(`Failed to parse decoded metadata documents: ${error}`);+}-const decodedDoc2 = JSON.parse(base64decode((headerWithVctm.vctm as any[])[1]));-
Suggestion importance[1-10]: 3
__
Why: While error handling is generally good practice, this is test code where failures should be visible. The suggestion adds unnecessary complexity to a test that should fail if the JSON parsing fails, as that would indicate a real issue.
Low
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
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.
User description
Added support for embedding Type Metadata documents in the JWS unprotected header via the vctm parameter (as per SD-JWT VC Spec Section 6.3.5.
PR Type
Enhancement, Tests, Documentation
Description
Embed type metadata in vctm header
Extend createSignedVCSDJWT options signature
Remove deprecated createVCSDJWT method
Add tests covering vctm header behavior
Changes walkthrough 📝
issuer.spec.ts
Add vctm tests and clean importssrc/issuer.spec.ts
issuer.ts
Implement vctm header in createSignedVCSDJWTsrc/issuer.ts
types.ts
Add typeMetadataGlueDocuments to optssrc/types.ts
CHANGELOG.md
Update changelog for vctm and removalCHANGELOG.md