Feat/jwt vc jsonld#1646
Open
sagarkhole4 wants to merge 4 commits into
Open
Conversation
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
Signed-off-by: Sagar Khole <sagar.khole@ayanworks.com>
📝 WalkthroughWalkthroughAdds jwt_vc_json-ld credential format support across enums, DTOs, builders, issuer metadata, and template validation; introduces a new OID4VC holder microservice and gateway endpoints with NATS wiring and common constant/URL updates; adds presentation DTO versioning and validator typing. ChangesJWT VC JSON-LD Format Addition
OIDC4VC Holder Microservice & API
Presentation DTO and validator typing
Sequence Diagram — OIDC4VC Holder flow sequenceDiagram
participant Client
participant ApiGatewayController
participant Oid4vcHolderService
participant NATS_Holder_Service
participant OrgAgent
Client->>ApiGatewayController: POST /orgs/:orgId/oid4vc/holder/{action} (holder DTO)
ApiGatewayController->>Oid4vcHolderService: call oidcHolder{Action}(orgId, holderPayload)
Oid4vcHolderService->>NATS_Holder_Service: send NATS message { orgId, holderPayload }
NATS_Holder_Service->>Oid4vcHolderService: response (holder result)
Oid4vcHolderService->>OrgAgent: HTTP POST to resolved agent URL with Authorization header
OrgAgent-->>Oid4vcHolderService: HTTP response
Oid4vcHolderService-->>ApiGatewayController: return result
ApiGatewayController-->>Client: HTTP 200 { statusCode, message, data }
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 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 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: 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}
| const templateContext = (templateRecord.attributes as any)?.context; | ||
| const context = Array.isArray(templateContext) ? templateContext : ['https://www.w3.org/2018/credentials/v1']; | ||
|
|
||
| if (!context.includes('https://www.w3.org/2018/credentials/v1')) { |
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.
Comment on lines
+56
to
+57
| this.logger.error(`Error in oidcHolderResolveCredentialOffer in holder service : ${JSON.stringify(error)}`); | ||
| throw error; |
There was a problem hiding this comment.
Stop logging raw serialized error objects from outbound agent calls.
JSON.stringify(error) can serialize HTTP client config, including headers, which risks leaking the decrypted authorization API key into logs. Log sanitized fields only (operation, status, message, trace id), not full error objects.
Also applies to: 70-71, 84-85, 98-99
🤖 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 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.
Comment on lines
+539
to
+546
| const vct = (templateRecord.attributes as any)?.vct; | ||
| let typeName = ''; | ||
| if ('string' === typeof vct) { | ||
| const lastSlash = vct.lastIndexOf('/'); | ||
| typeName = -1 !== lastSlash ? vct.substring(lastSlash + 1) : vct; | ||
| } else { | ||
| typeName = templateRecord.name.replace(/\s+/g, ''); | ||
| } |
There was a problem hiding this comment.
Handle edge case where vct ends with /.
If vct is a URL ending with / (e.g., https://example.org/credentials/), substring(lastSlash + 1) returns an empty string, resulting in a VC type of ['VerifiableCredential', ''].
Proposed fix
const vct = (templateRecord.attributes as any)?.vct;
let typeName = '';
if ('string' === typeof vct) {
const lastSlash = vct.lastIndexOf('/');
- typeName = -1 !== lastSlash ? vct.substring(lastSlash + 1) : vct;
+ const extracted = lastSlash !== -1 ? vct.substring(lastSlash + 1) : vct;
+ typeName = extracted || templateRecord.name.replace(/\s+/g, '');
} else {
typeName = templateRecord.name.replace(/\s+/g, '');
}📝 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
| const vct = (templateRecord.attributes as any)?.vct; | |
| let typeName = ''; | |
| if ('string' === typeof vct) { | |
| const lastSlash = vct.lastIndexOf('/'); | |
| typeName = -1 !== lastSlash ? vct.substring(lastSlash + 1) : vct; | |
| } else { | |
| typeName = templateRecord.name.replace(/\s+/g, ''); | |
| } | |
| const vct = (templateRecord.attributes as any)?.vct; | |
| let typeName = ''; | |
| if ('string' === typeof vct) { | |
| const lastSlash = vct.lastIndexOf('/'); | |
| const extracted = lastSlash !== -1 ? vct.substring(lastSlash + 1) : vct; | |
| typeName = extracted || templateRecord.name.replace(/\s+/g, ''); | |
| } else { | |
| typeName = templateRecord.name.replace(/\s+/g, ''); | |
| } |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 543-543: Unexpected negated condition.
[warning] 539-539: This assertion is unnecessary since it does not change the type of the expression.
🤖 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/credential-sessions.builder.ts` around
lines 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.
Comment on lines
+120
to
+129
| if (internalSchemaId) { | ||
| let schemaResponse; | ||
| try { | ||
| schemaResponse = await this.issuanceServiceProxy | ||
| .send({ cmd: 'get-schema-by-id' }, { schemaId: internalSchemaId, orgId }) | ||
| .toPromise(); | ||
| } catch (error) { | ||
| this.logger.error(`Error fetching schema ${internalSchemaId} from ledger: ${error.message}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are other places where schema fetch failures are handled
rg -n "get-schema-by-id" --type ts -A 5 -B 2Repository: credebl/platform
Length of output: 2156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/oid4vc-issuance/src/oid4vc-issuance.service.ts"
# Show the method context around the reported lines
sed -n '90,170p' "$FILE" | nl -ba | sed -n '1,120p'
# Show the remainder of the block where schemaResponse is used (if any)
rg -n "internalSchemaId" "$FILE" -n
rg -n "schemaResponse" "$FILE" -nRepository: credebl/platform
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/oid4vc-issuance/src/oid4vc-issuance.service.ts"
echo "=== Context: lines 90-170 ==="
sed -n '90,170p' "$FILE" | cat -n
echo "=== Where internalSchemaId is referenced ==="
rg -n "internalSchemaId" "$FILE"
echo "=== Where schemaResponse is referenced ==="
rg -n "schemaResponse" "$FILE"Repository: credebl/platform
Length of output: 4204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/oid4vc-issuance/src/oid4vc-issuance.service.ts"
echo "=== Find calls to validateTemplateAgainstSchema ==="
rg -n "validateTemplateAgainstSchema\(" "$FILE"
echo "=== Show caller contexts (a bit around each match) ==="
rg -n "validateTemplateAgainstSchema\(" "$FILE" -C 5Repository: credebl/platform
Length of output: 1929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/oid4vc-issuance/src/oid4vc-issuance.service.ts"
echo "=== Template creation method(s) around lines 360-430 ==="
sed -n '340,440p' "$FILE" | cat -n | sed -n '1,130p'
echo "=== Template normalization / create flow around lines 460-540 ==="
sed -n '450,560p' "$FILE" | cat -n | sed -n '1,160p'Repository: credebl/platform
Length of output: 11603
Propagate schema-fetch failures instead of silently returning.
When get-schema-by-id throws (lines 126-129), validateTemplateAgainstSchema only logs and returns, but createTemplate and updateTemplate just await that method and continue to persist the template—so templates with invalid/unavailable schema references can be created/updated without strict validation.
Throw an exception after logging (or otherwise make the failure block template persistence) so the caller can surface the error to the client.
🤖 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 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.
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
2 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