Feat/jwt vc jsonld#1646
Merged
Merged
Conversation
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesJWT VC JSON-LD Format Addition
Verification DTO Version Support
Utility Type Safety
Sequence Diagram(s)sequenceDiagram
participant APIGateway
participant OID4VCIssuanceService
participant IssuanceNATS
participant CredentialBuilder
rect rgba(100, 149, 237, 0.5)
note over APIGateway,OID4VCIssuanceService: Template Create/Update
APIGateway->>OID4VCIssuanceService: createTemplate(JwtVcJsonLd, template+schemaUrl)
OID4VCIssuanceService->>OID4VCIssuanceService: prepend CREDENTIALS_CONTEXT_V1_URL to template.context
OID4VCIssuanceService->>IssuanceNATS: fetch schema by schemaId
IssuanceNATS-->>OID4VCIssuanceService: schema with attribute names
OID4VCIssuanceService->>OID4VCIssuanceService: recursively validate template attributes
OID4VCIssuanceService-->>APIGateway: BadRequestException or persist template
end
rect rgba(60, 179, 113, 0.5)
note over APIGateway,CredentialBuilder: Credential Offer
APIGateway->>OID4VCIssuanceService: buildCredentialOfferPayload(JwtVcJsonLd)
OID4VCIssuanceService->>CredentialBuilder: buildJwtVcJsonLdCredential(template, payload, signers)
CredentialBuilder->>CredentialBuilder: select DID/x5c signer, validate validity window
CredentialBuilder->>CredentialBuilder: wrap in JSON-LD envelope (`@context`, type, credentialSubject)
CredentialBuilder-->>OID4VCIssuanceService: signed JwtVcJsonLd credential payload
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🧹 Nitpick comments (1)
apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts (1)
395-399: ⚡ Quick winExtract nested ternary into a small explicit format-resolution block.
This is harder to read and was already flagged by static analysis; a simple
if/elsekeeps intent clearer.Proposed refactor
- const credentialConfig = buildCredentialConfig( - templateRow.name, - templateToBuild, - format === CredentialFormat.Mdoc - ? CredentialFormat.Mdoc - : format === CredentialFormat.JwtVcJsonLd - ? CredentialFormat.JwtVcJsonLd - : CredentialFormat.SdJwtVc - ); + let resolvedFormat: CredentialFormat; + if (format === CredentialFormat.Mdoc) { + resolvedFormat = CredentialFormat.Mdoc; + } else if (format === CredentialFormat.JwtVcJsonLd) { + resolvedFormat = CredentialFormat.JwtVcJsonLd; + } else { + resolvedFormat = CredentialFormat.SdJwtVc; + } + + const credentialConfig = buildCredentialConfig(templateRow.name, templateToBuild, resolvedFormat);🤖 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 `@apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts` around lines 395 - 399, Replace the nested ternary that resolves a CredentialFormat (the expression using `format === CredentialFormat.Mdoc ? ... : format === CredentialFormat.JwtVcJsonLd ? ... : CredentialFormat.SdJwtVc`) with a small explicit resolution block: create a local variable (e.g., `resolvedFormat`) and use an if/else-if/else (or switch) that checks `format` against `CredentialFormat.Mdoc` and `CredentialFormat.JwtVcJsonLd` and assigns the appropriate enum value, then use `resolvedFormat` where the ternary was used; this keeps the logic in the same function/closure but makes intent clear and satisfies the static analysis warning.
🤖 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 `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts`:
- Around line 232-235: The multiline arrow predicate for the `@ValidateIf`
decorator is causing an implicit-arrow-linebreak lint error; update the
predicate to be a single-line arrow function so ESLint accepts it. Locate the
`@ValidateIf` on CreateCredentialTemplateDto and replace the multiline predicate
with a single-line form such as (o: CreateCredentialTemplateDto) =>
CredentialFormat.SdJwtVc === o.format || CredentialFormat.JwtVcJsonLd ===
o.format, ensuring the decorator remains intact and the condition logic is
unchanged.
In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts`:
- Around line 544-549: The current truthy checks for nbf and exp in the
credential sessions builder can wrongly omit valid zero timestamps; update the
conditions in the block that sets wrappedPayload.nbf and wrappedPayload.exp to
use explicit undefined checks (e.g., check nbf !== undefined and exp !==
undefined) so that 0 is accepted; locate the logic around wrappedPayload
assignment within the credential-sessions.builder.ts (the code that reads nbf
and exp and assigns to wrappedPayload.nbf/wrappedPayload.exp) and replace the
truthy guards with explicit undefined comparisons.
---
Nitpick comments:
In `@apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts`:
- Around line 395-399: Replace the nested ternary that resolves a
CredentialFormat (the expression using `format === CredentialFormat.Mdoc ? ... :
format === CredentialFormat.JwtVcJsonLd ? ... : CredentialFormat.SdJwtVc`) with
a small explicit resolution block: create a local variable (e.g.,
`resolvedFormat`) and use an if/else-if/else (or switch) that checks `format`
against `CredentialFormat.Mdoc` and `CredentialFormat.JwtVcJsonLd` and assigns
the appropriate enum value, then use `resolvedFormat` where the ternary was
used; this keeps the logic in the same function/closure but makes intent clear
and satisfies the static analysis warning.
🪄 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
Run ID: 6335be8e-ad85-4bef-8950-9478ba732ab5
📒 Files selected for processing (6)
apps/api-gateway/src/oid4vc-issuance/dtos/issuer-sessions.dto.tsapps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.tsapps/oid4vc-issuance/interfaces/oid4vc-issuer-sessions.interfaces.tsapps/oid4vc-issuance/libs/helpers/credential-sessions.builder.tsapps/oid4vc-issuance/libs/helpers/issuer.metadata.tslibs/enum/src/enum.ts
Comment on lines
+232
to
+235
| @ValidateIf( | ||
| (o: CreateCredentialTemplateDto) => | ||
| CredentialFormat.SdJwtVc === o.format || CredentialFormat.JwtVcJsonLd === o.format | ||
| ) |
There was a problem hiding this comment.
Fix the lint-blocking ValidateIf arrow formatting.
The current multiline arrow predicate triggers the ESLint implicit-arrow-linebreak error and will fail CI.
Proposed fix
- `@ValidateIf`(
- (o: CreateCredentialTemplateDto) =>
- CredentialFormat.SdJwtVc === o.format || CredentialFormat.JwtVcJsonLd === o.format
- )
+ `@ValidateIf`(
+ (o: CreateCredentialTemplateDto) =>
+ CredentialFormat.SdJwtVc === o.format || CredentialFormat.JwtVcJsonLd === o.format
+ )🧰 Tools
🪛 ESLint
[error] 234-234: Expected no linebreak before this expression.
(implicit-arrow-linebreak)
🤖 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 `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts`
around lines 232 - 235, The multiline arrow predicate for the `@ValidateIf`
decorator is causing an implicit-arrow-linebreak lint error; update the
predicate to be a single-line arrow function so ESLint accepts it. Locate the
`@ValidateIf` on CreateCredentialTemplateDto and replace the multiline predicate
with a single-line form such as (o: CreateCredentialTemplateDto) =>
CredentialFormat.SdJwtVc === o.format || CredentialFormat.JwtVcJsonLd ===
o.format, ensuring the decorator remains intact and the condition logic is
unchanged.
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/common/src/common.utils.ts (1)
28-45:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
orderValuestype safety: constrain values before using>/<.
varA/varBare inferred asunknownfromRecord<string, unknown>, so the relational comparisons (>,<) are no longer type-safe without narrowing/casting.Suggested fix
+type SortableValue = string | number; export function orderValues( key: string, - order = 'asc' -): (a: Record<string, unknown>, b: Record<string, unknown>) => number { - return function innerSort(a: Record<string, unknown>, b: Record<string, unknown>): number { + order: 'asc' | 'desc' = 'asc' +): (a: Record<string, SortableValue>, b: Record<string, SortableValue>) => number { + return function innerSort(a: Record<string, SortableValue>, b: Record<string, SortableValue>): number {🤖 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 `@libs/common/src/common.utils.ts` around lines 28 - 45, orderValues' innerSort uses relational operators on varA/varB inferred as unknown; add explicit narrowing before comparing: after extracting varA/varB in orderValues/innerSort, narrow their types (e.g., check typeof === 'string' or typeof === 'number') and handle comparisons accordingly (use localeCompare for strings, numeric subtraction for numbers), and return 0 for unsupported types; this preserves type-safety without changing the function signature.
🧹 Nitpick comments (5)
apps/oid4vc-issuance/src/oid4vc-issuance.service.ts (2)
43-47: 💤 Low valueConsolidate duplicate imports from
@credebl/common/common.constant.
CommonConstants(line 24) anduuidRegex(line 47) are both imported from@credebl/common/common.constant. Consider merging into a single import statement.Proposed fix
- import { CommonConstants } from '`@credebl/common/common.constant`'; + import { CommonConstants, uuidRegex } from '`@credebl/common/common.constant`'; ... - import { uuidRegex } from '`@credebl/common/common.constant`';🤖 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 `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts` around lines 43 - 47, The imports from `@credebl/common/common.constant` are duplicated: merge the two import statements into one by combining CommonConstants and uuidRegex into a single import list (replace the separate imports that reference CommonConstants and uuidRegex with one consolidated import that includes both symbols) so only one import from `@credebl/common/common.constant` remains, keeping the existing exported names intact.
131-153: ⚡ Quick winUse
Setfor O(1) attribute lookup.
schemaAttributeNames.includes(attr.key)at line 145 has O(n) complexity per lookup. For templates with many attributes, using aSetimproves performance.Proposed fix
if (schemaResponse && schemaResponse.attributes) { const schemaAttributes = 'string' === typeof schemaResponse.attributes ? JSON.parse(schemaResponse.attributes) : schemaResponse.attributes; - const schemaAttributeNames = schemaAttributes.map((attr: any) => attr.attributeName); + const schemaAttributeNames = new Set(schemaAttributes.map((attr: any) => attr.attributeName)); const templateAttributes = template.attributes; const missingAttributes: string[] = []; const checkAttributes = (attrs: any[]) => { if (!attrs) { return; } for (const attr of attrs) { - if (!schemaAttributeNames.includes(attr.key)) { + if (!schemaAttributeNames.has(attr.key)) { missingAttributes.push(attr.key); }🤖 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 `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts` around lines 131 - 153, The current attribute existence check in checkAttributes uses schemaAttributeNames.includes(attr.key) which is O(n) per lookup; convert schemaAttributeNames into a Set (e.g., const schemaAttributeSet = new Set(schemaAttributes.map(...))) and replace includes checks with schemaAttributeSet.has(attr.key) inside checkAttributes (which iterates templateAttributes recursively) so lookups are O(1); ensure you create the Set once after parsing schemaResponse.attributes and reference it in the checkAttributes function.apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts (1)
302-307: 💤 Low valueType extraction logic duplicated with credential-sessions.builder.ts.
The
vctparsing logic for extractingtypeNameis nearly identical tobuildJwtVcJsonLdCredentialincredential-sessions.builder.ts(lines 539-546). Consider extracting to a shared helper to ensure consistent behavior and fix the trailing-slash edge case in one place.🤖 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 `@apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts` around lines 302 - 307, The vct-to-typeName extraction is duplicated and mishandles trailing slashes; extract a shared helper (e.g., getTypeNameFromVct(vct: unknown, fallback?: string)) and replace the inline logic in issuer.metadata.ts (variables template, vct, typeName) and in buildJwtVcJsonLdCredential in credential-sessions.builder.ts with calls to this helper; the helper should: treat non-string or empty vct as fallback (derived from name or 'GenericCredential'), trim vct, split on '/' and pick the last non-empty segment (so a trailing slash doesn't yield an empty type), and sanitize the result (remove non-alphanumeric chars) before returning.apps/api-gateway/src/oid4vc-holder/oid4vc-holder.service.ts (1)
21-39: ⚡ Quick winUse shared constants for NATS command names.
These command strings are a cross-service contract with
apps/oid4vc-holder/src/oid4vc-holder.controller.ts; keeping them as literals on both sides makes routing fragile to typos. Move them to shared constants and reference the same symbols in both modules.🤖 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 `@apps/api-gateway/src/oid4vc-holder/oid4vc-holder.service.ts` around lines 21 - 39, The NATS command strings used in oidcHolderResolveCredentialOffer, oidcHolderRequestCredential, oidcHolderResolveProofRequest, and oidcHolderAcceptProofRequest are hard-coded literals and should be replaced with shared constants to avoid typos; import the shared command constants (e.g., OID4VC_HOLDER_RESOLVE_CREDENTIAL_OFFER, OID4VC_HOLDER_REQUEST_CREDENTIAL, OID4VC_HOLDER_RESOLVE_PROOF_REQUEST, OID4VC_HOLDER_ACCEPT_PROOF_REQUEST) from the common/shared module and use them in the natsClient.sendNatsMessage calls (keeping the payload building and holderProxy usage unchanged) so both apps reference the same command symbols.apps/oid4vc-holder/src/oid4vc-holder.service.ts (1)
47-101: ⚡ Quick winExtract the repeated agent-forwarding flow into one helper.
The four holder methods duplicate key lookup, endpoint resolution, HTTP POST, and error handling; this makes fixes (like log sanitization/security hardening) easy to miss in one path. Centralizing this flow will reduce drift.
🤖 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 `@apps/oid4vc-holder/src/oid4vc-holder.service.ts` around lines 47 - 101, The four methods oidcHolderResolveCredentialOffer, oidcHolderRequestCredential, oidcHolderResolveProofRequest, and oidcHolderAcceptProofRequest duplicate agent API key lookup (getOrgAgentApiKey), endpoint resolution (_getAgentEndpoint), URL construction (getAgentUrl), HTTP POST (commonService.httpPost) and error logging; extract this into a single private helper (e.g., forwardToAgent or sendAgentPost) that accepts orgId, path (CommonConstants.*), and payload, performs getOrgAgentApiKey + _getAgentEndpoint + getAgentUrl + commonService.httpPost with consistent headers and centralized try/catch/logging, then replace each method body with a simple call to that helper and return its result to remove duplication and ensure uniform error handling/log sanitization.
🤖 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 `@apps/api-gateway/src/oid4vc-holder/dtos/oid4vc-holder.dto.ts`:
- Line 2: The credentialsToRequest array in the DTO is only using `@IsNotEmpty` on
the array itself, allowing empty/invalid entries; update the validation for the
credentialsToRequest property (in the DTO class containing credentialsToRequest,
e.g., Oid4vcHolderDto) to require a non-empty array and validate each member as
a non-empty string by adding decorators such as ArrayNotEmpty and per-item
validators (IsString({ each: true }) and IsNotEmpty({ each: true })) so every
element is a non-empty string.
In `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts`:
- Around line 199-206: The context property is only decorated with `@IsArray` and
`@IsOptional`, so non-string elements can slip through; update the DTO by adding
element-level string validation to the context?: string[] property (use the
class-validator decorator that validates each array element is a string, e.g.,
`@IsString`({ each: true })) alongside the existing decorators on the property in
the Oid4vc issuer template DTO.
In `@apps/oid4vc-holder/src/oid4vc-holder.service.ts`:
- Around line 56-57: Replace the unsafe JSON.stringify(error) logging in
oidcHolderResolveCredentialOffer (and the other similar logger.error calls at
the other locations) with a sanitized log payload that only includes operation
(e.g., "oidcHolderResolveCredentialOffer"), HTTP status
(error.response?.status), error.message, and any non-sensitive trace id
(error.response?.headers['x-trace-id'] or error.traceId) — do not log
error.config, headers, or full error objects; update the logger.error
invocations in oid4vc-holder.service.ts to extract and log only those sanitized
fields and reuse the same pattern for the other occurrences mentioned.
In `@apps/oid4vc-issuance/libs/helpers/credential-sessions.builder.ts`:
- Around line 539-546: The extraction of VC type from vct (in
credential-sessions.builder.ts) can produce an empty string when vct ends with
'/', so update the logic around the vct -> typeName computation to ignore
trailing slashes and pick the last non-empty path segment (or fall back to
templateRecord.name.replace(/\s+/g, '') if none exists). Specifically, change
the branch that handles 'string' vct to first strip trailing slashes or split on
'/' and pick the last non-empty element before assigning typeName, keeping the
existing else branch using templateRecord.name as fallback.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 120-129: The catch block inside validateTemplateAgainstSchema that
handles failures from issuanceServiceProxy.send (when fetching schema by
internalSchemaId into schemaResponse) should not silently return after logging;
instead rethrow the error (or throw a new Error with context) so callers like
createTemplate and updateTemplate receive the failure and abort persistence;
update the catch to log the error and then throw (or rethrow) to propagate the
schema-fetch failure up the call stack.
---
Outside diff comments:
In `@libs/common/src/common.utils.ts`:
- Around line 28-45: orderValues' innerSort uses relational operators on
varA/varB inferred as unknown; add explicit narrowing before comparing: after
extracting varA/varB in orderValues/innerSort, narrow their types (e.g., check
typeof === 'string' or typeof === 'number') and handle comparisons accordingly
(use localeCompare for strings, numeric subtraction for numbers), and return 0
for unsupported types; this preserves type-safety without changing the function
signature.
---
Nitpick comments:
In `@apps/api-gateway/src/oid4vc-holder/oid4vc-holder.service.ts`:
- Around line 21-39: The NATS command strings used in
oidcHolderResolveCredentialOffer, oidcHolderRequestCredential,
oidcHolderResolveProofRequest, and oidcHolderAcceptProofRequest are hard-coded
literals and should be replaced with shared constants to avoid typos; import the
shared command constants (e.g., OID4VC_HOLDER_RESOLVE_CREDENTIAL_OFFER,
OID4VC_HOLDER_REQUEST_CREDENTIAL, OID4VC_HOLDER_RESOLVE_PROOF_REQUEST,
OID4VC_HOLDER_ACCEPT_PROOF_REQUEST) from the common/shared module and use them
in the natsClient.sendNatsMessage calls (keeping the payload building and
holderProxy usage unchanged) so both apps reference the same command symbols.
In `@apps/oid4vc-holder/src/oid4vc-holder.service.ts`:
- Around line 47-101: The four methods oidcHolderResolveCredentialOffer,
oidcHolderRequestCredential, oidcHolderResolveProofRequest, and
oidcHolderAcceptProofRequest duplicate agent API key lookup (getOrgAgentApiKey),
endpoint resolution (_getAgentEndpoint), URL construction (getAgentUrl), HTTP
POST (commonService.httpPost) and error logging; extract this into a single
private helper (e.g., forwardToAgent or sendAgentPost) that accepts orgId, path
(CommonConstants.*), and payload, performs getOrgAgentApiKey + _getAgentEndpoint
+ getAgentUrl + commonService.httpPost with consistent headers and centralized
try/catch/logging, then replace each method body with a simple call to that
helper and return its result to remove duplication and ensure uniform error
handling/log sanitization.
In `@apps/oid4vc-issuance/libs/helpers/issuer.metadata.ts`:
- Around line 302-307: The vct-to-typeName extraction is duplicated and
mishandles trailing slashes; extract a shared helper (e.g.,
getTypeNameFromVct(vct: unknown, fallback?: string)) and replace the inline
logic in issuer.metadata.ts (variables template, vct, typeName) and in
buildJwtVcJsonLdCredential in credential-sessions.builder.ts with calls to this
helper; the helper should: treat non-string or empty vct as fallback (derived
from name or 'GenericCredential'), trim vct, split on '/' and pick the last
non-empty segment (so a trailing slash doesn't yield an empty type), and
sanitize the result (remove non-alphanumeric chars) before returning.
In `@apps/oid4vc-issuance/src/oid4vc-issuance.service.ts`:
- Around line 43-47: The imports from `@credebl/common/common.constant` are
duplicated: merge the two import statements into one by combining
CommonConstants and uuidRegex into a single import list (replace the separate
imports that reference CommonConstants and uuidRegex with one consolidated
import that includes both symbols) so only one import from
`@credebl/common/common.constant` remains, keeping the existing exported names
intact.
- Around line 131-153: The current attribute existence check in checkAttributes
uses schemaAttributeNames.includes(attr.key) which is O(n) per lookup; convert
schemaAttributeNames into a Set (e.g., const schemaAttributeSet = new
Set(schemaAttributes.map(...))) and replace includes checks with
schemaAttributeSet.has(attr.key) inside checkAttributes (which iterates
templateAttributes recursively) so lookups are O(1); ensure you create the Set
once after parsing schemaResponse.attributes and reference it in the
checkAttributes function.
🪄 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
Run ID: 9cafd9bd-6811-407d-be6f-ff622c6dd9aa
📒 Files selected for processing (20)
apps/api-gateway/src/app.module.tsapps/api-gateway/src/oid4vc-holder/dtos/oid4vc-holder.dto.tsapps/api-gateway/src/oid4vc-holder/oid4vc-holder.controller.tsapps/api-gateway/src/oid4vc-holder/oid4vc-holder.module.tsapps/api-gateway/src/oid4vc-holder/oid4vc-holder.service.tsapps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.tsapps/api-gateway/src/oid4vc-issuance/oid4vc-issuance.module.tsapps/api-gateway/src/oid4vc-verification/dtos/oid4vc-verifier-presentation.dto.tsapps/oid4vc-holder/src/interfaces/oid4vc-holder.interface.tsapps/oid4vc-holder/src/main.tsapps/oid4vc-holder/src/oid4vc-holder.controller.tsapps/oid4vc-holder/src/oid4vc-holder.module.tsapps/oid4vc-holder/src/oid4vc-holder.service.tsapps/oid4vc-issuance/constant/issuance.tsapps/oid4vc-issuance/interfaces/oid4vc-template.interfaces.tsapps/oid4vc-issuance/libs/helpers/credential-sessions.builder.tsapps/oid4vc-issuance/libs/helpers/issuer.metadata.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.tslibs/common/src/common.constant.tslibs/common/src/common.utils.ts
✅ Files skipped from review due to trivial changes (3)
- apps/oid4vc-issuance/interfaces/oid4vc-template.interfaces.ts
- apps/oid4vc-issuance/constant/issuance.ts
- apps/oid4vc-holder/src/interfaces/oid4vc-holder.interface.ts
| @@ -0,0 +1,40 @@ | |||
| import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; | |||
| import { IsArray, IsNotEmpty, IsOptional, IsString } from 'class-validator'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/api-gateway/src/oid4vc-holder/dtos/oid4vc-holder.dto.ts"
# Show the relevant area around the mentioned line(s)
nl -ba "$FILE" | sed -n '1,260p'Repository: credebl/platform
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/api-gateway/src/oid4vc-holder/dtos/oid4vc-holder.dto.ts"
# Sanity: file exists
test -f "$FILE"
# Print with line numbers (no nl available)
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '1,260p'Repository: credebl/platform
Length of output: 1409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs "credentialsToRequest" .Repository: credebl/platform
Length of output: 259
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the interface for credentialsToRequest
FILE_IF="apps/oid4vc-holder/src/interfaces/oid4vc-holder.interface.ts"
test -f "$FILE_IF"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE_IF" | sed -n '1,200p'
# Find where OidcRequestCredentialDto is used
rg -n --hidden --no-ignore-vcs "OidcRequestCredentialDto" .Repository: credebl/platform
Length of output: 1145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# controller
FILE_C="apps/api-gateway/src/oid4vc-holder/oid4vc-holder.controller.ts"
test -f "$FILE_C"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE_C" | sed -n '1,220p'
# service
FILE_S="apps/api-gateway/src/oid4vc-holder/oid4vc-holder.service.ts"
test -f "$FILE_S"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE_S" | sed -n '1,220p'Repository: credebl/platform
Length of output: 8010
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs "oid4vc-holder-request-credential" .Repository: credebl/platform
Length of output: 351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/oid4vc-holder/src/oid4vc-holder.controller.ts"
test -f "$FILE"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE" | sed -n '1,260p'Repository: credebl/platform
Length of output: 2143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs "holderPayload\.credentialsToRequest|credentialsToRequest" apps/oid4vc-holderRepository: credebl/platform
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs "ValidationPipe" apps/api-gateway .Repository: credebl/platform
Length of output: 12291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# main.ts around global pipes
FILE_MAIN="apps/api-gateway/src/main.ts"
test -f "$FILE_MAIN"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE_MAIN" | sed -n '100,170p'
# custom validation pipe implementation
FILE_PIPE="libs/common/src/custom-overrideable-validation-pipe.ts"
test -f "$FILE_PIPE"
awk '{printf "%6d\t%s\n", NR, $0}' "$FILE_PIPE" | sed -n '1,220p'Repository: credebl/platform
Length of output: 5729
credentialsToRequest validation is too weak for the request contract.
@IsNotEmpty() only guarantees the array itself isn’t empty; it doesn’t validate that each entry is a non-empty string, so invalid/empty members can reach the holder flow.
Suggested fix
-import { IsArray, IsNotEmpty, IsOptional, IsString } from 'class-validator';
+import { ArrayNotEmpty, IsArray, IsNotEmpty, IsOptional, IsString } from 'class-validator';
@@
`@ApiProperty`({ example: ['UniversityDegree'] })
`@IsArray`()
- `@IsNotEmpty`()
+ `@ArrayNotEmpty`()
+ `@IsString`({ each: true })
+ `@IsNotEmpty`({ each: true })
credentialsToRequest: string[];🤖 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 `@apps/api-gateway/src/oid4vc-holder/dtos/oid4vc-holder.dto.ts` at line 2, The
credentialsToRequest array in the DTO is only using `@IsNotEmpty` on the array
itself, allowing empty/invalid entries; update the validation for the
credentialsToRequest property (in the DTO class containing credentialsToRequest,
e.g., Oid4vcHolderDto) to require a non-empty array and validate each member as
a non-empty string by adding decorators such as ArrayNotEmpty and per-item
validators (IsString({ each: true }) and IsNotEmpty({ each: true })) so every
element is a non-empty string.
Comment on lines
+199
to
+206
| @ApiPropertyOptional({ | ||
| example: ['https://www.w3.org/2018/credentials/v1', 'https://json-ld.org/contexts/person.jsonld'], | ||
| description: 'JSON-LD context for the credential (optional, defaults to person schema for JSON-LD format)' | ||
| }) | ||
| @IsArray() | ||
| @IsOptional() | ||
| context?: string[]; | ||
|
|
There was a problem hiding this comment.
Add element-level string validation for context array.
The @IsArray decorator validates the field is an array, but doesn't validate that each element is a string. A malformed payload with non-string elements (e.g., context: [123, {}]) would pass validation.
Proposed fix
`@ApiPropertyOptional`({
example: ['https://www.w3.org/2018/credentials/v1', 'https://json-ld.org/contexts/person.jsonld'],
description: 'JSON-LD context for the credential (optional, defaults to person schema for JSON-LD format)'
})
`@IsArray`()
+ `@IsString`({ each: true })
`@IsOptional`()
context?: string[];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ApiPropertyOptional({ | |
| example: ['https://www.w3.org/2018/credentials/v1', 'https://json-ld.org/contexts/person.jsonld'], | |
| description: 'JSON-LD context for the credential (optional, defaults to person schema for JSON-LD format)' | |
| }) | |
| @IsArray() | |
| @IsOptional() | |
| context?: string[]; | |
| `@ApiPropertyOptional`({ | |
| example: ['https://www.w3.org/2018/credentials/v1', 'https://json-ld.org/contexts/person.jsonld'], | |
| description: 'JSON-LD context for the credential (optional, defaults to person schema for JSON-LD format)' | |
| }) | |
| `@IsArray`() | |
| `@IsString`({ each: true }) | |
| `@IsOptional`() | |
| context?: string[]; |
🤖 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 `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts`
around lines 199 - 206, The context property is only decorated with `@IsArray` and
`@IsOptional`, so non-string elements can slip through; update the DTO by adding
element-level string validation to the context?: string[] property (use the
class-validator decorator that validates each array element is a string, e.g.,
`@IsString`({ each: true })) alongside the existing decorators on the property in
the Oid4vc issuer template DTO.
Contributor
Author
There was a problem hiding this comment.
We do not expose or accept the context array directly from the user in the DTO. Instead, the backend automatically constructs it internally from the schemaUrl (which is strictly validated as a string via @IsString()) and a default constant. Therefore, this issue is already mitigated.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts (1)
199-205: 💤 Low valueConsider adding URL validation for
schemaUrl.The field is documented as a "JSON-LD schema URL" and the example shows a full URL, but validation only ensures it's a string. An invalid URL would pass DTO validation but fail later when the service attempts to fetch the schema.
Suggested improvement
+import { IsUrl } from 'class-validator'; ... `@ApiPropertyOptional`({ example: 'https://dev-schema.sovio.id/schemas/a99d55b6-c663-4af8-bcd3-4fe6699df91a', description: 'JSON-LD schema URL for the credential' }) - `@IsString`() + `@IsUrl`({}, { message: 'schemaUrl must be a valid URL' }) `@IsOptional`() schemaUrl?: string;🤖 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 `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts` around lines 199 - 205, The schemaUrl property in the DTO class currently only validates that it is a string using `@IsString`() decorator, but since it is documented as a "JSON-LD schema URL" and should contain a valid URL, add the `@IsUrl`() decorator (from class-validator) alongside the existing `@IsString`() and `@IsOptional`() decorators to ensure URL format validation occurs during DTO validation rather than failing later when the service attempts to fetch the schema.
🤖 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.
Nitpick comments:
In `@apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.ts`:
- Around line 199-205: The schemaUrl property in the DTO class currently only
validates that it is a string using `@IsString`() decorator, but since it is
documented as a "JSON-LD schema URL" and should contain a valid URL, add the
`@IsUrl`() decorator (from class-validator) alongside the existing `@IsString`() and
`@IsOptional`() decorators to ensure URL format validation occurs during DTO
validation rather than failing later when the service attempts to fetch the
schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfca1d7a-4bde-4f17-9ada-9dc16f7dd0b2
📒 Files selected for processing (3)
apps/api-gateway/src/oid4vc-issuance/dtos/oid4vc-issuer-template.dto.tsapps/oid4vc-issuance/interfaces/oid4vc-template.interfaces.tsapps/oid4vc-issuance/src/oid4vc-issuance.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/oid4vc-issuance/interfaces/oid4vc-template.interfaces.ts
…ration Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…, and fix linting Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
RinkalBhojani
approved these changes
Jun 24, 2026
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…ration Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
…, and fix linting Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
RinkalBhojani
force-pushed
the
feat/jwt_vc_jsonld
branch
from
4afb9e1 to
3a70c87
Compare
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
|
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
Improvements