Skip to content

merge: main sync (OID4VC/P) to main#368

Merged
tipusinghaw merged 152 commits into
mainfrom
merge/main-sync-to-main
May 5, 2026
Merged

merge: main sync (OID4VC/P) to main#368
tipusinghaw merged 152 commits into
mainfrom
merge/main-sync-to-main

Conversation

@tipusinghaw
Copy link
Copy Markdown
Contributor

@tipusinghaw tipusinghaw commented Apr 29, 2026

Summary by CodeRabbit

  • New Features

    • OpenID4VC issuance and verification support with issuer, holder, and verifier capabilities
    • X.509 certificate creation, import, and trust management
    • Record purging with NATS and cron scheduling
    • SD-JWT and mDOC credential format support
    • Status list-based credential revocation
  • Improvements

    • Enhanced DIDComm protocol support
    • Upgraded Node.js to 22.22.0 with security enhancements
    • Non-root container runtime with improved security
    • Trust service integration for certificate validation
    • Wallet persistence configuration updates

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Code Review Summary

Walkthrough

This PR upgrades Aries Framework JavaScript from Credo 0.5.15 to 0.6.2, migrating core credential/proof exchanges from legacy APIs to a new DIDComm module layer. It adds complete OpenID4VC issuer/verifier/holder support, X.509 certificate management, record purge scheduling via NATS/Cron, authentication flows, and non-root Docker images. Docker, CLI, agent setup, event handlers, and 50+ controller/service files are modified.

Changes

Core Infrastructure & Dependency Upgrades

Layer / File(s) Summary
Docker & Node Version
Dockerfile
Node.js upgraded to 22.22.0; production stage adds OS security updates, CA certificates, and creates non-root appuser with restricted /app ownership.
Dependency Resolution
package.json, patches/@credo-ts+*, patches/@digitalcredentials+*
Credo-ts core/anoncreds/tenants upgraded to 0.6.2; Askar, Polygon, OpenID4VC modules added; AnonCreds/Indy versions loosened; NATS/cron/axios pinned; patches consolidate type changes (ExtensibleCredoExtensible, revert format-data errors, remove abandoned messageType field).
TypeScript & Build Config
tsconfig.build.json, tsoa.json
lib expanded to ESNext; TSOA routes/spec now use tsconfig.build.json for consistent type resolution.

DIDComm Module Migration

Layer / File(s) Summary
Type Definitions
src/controllers/types.ts, src/controllers/examples.ts
Record identifiers, format payloads, and auto-accept types migrated from @credo-ts/core to @credo-ts/didcomm equivalents; CustomTenantConfig simplified; new enums and DTOs added.
Event Handlers
src/events/Connection*, Credential*, Proof*, ReuseConnection*, BasicMessage*, WebSocket*
Event listeners switched from core to didcomm modules; handlers become tenant-aware (detecting shared vs. dedicated agents, routing through tenant agents); WebSocket server type updated to named Server import.
Credential/Proof/Connection Controllers
src/controllers/didcomm/credentials/*, src/controllers/didcomm/proofs/*, src/controllers/didcomm/connections/*, src/controllers/didcomm/outofband/*
All endpoints migrated to use agent.modules.didcomm.* instead of core APIs; purge scheduling decorators added; peer DID creation now supplies returned keys; examples and return types updated to didcomm types.
AnonCreds Controllers
src/controllers/anoncreds/schema/*, src/controllers/anoncreds/cred-def/*
Return types annotated with explicit Promise<...> and DID-lookup helpers; credential definition error messages derive from state reason; endorser transaction controller commented out (migration pending).

OpenID4VC Implementation

Layer / File(s) Summary
Type Definitions
src/controllers/openid4vc/types/holder.types.ts, src/controllers/openid4vc/types/issuer.types.ts, src/controllers/openid4vc/types/verifier.types.ts
New request/response DTOs, enums, and configuration shapes for holder (credential resolution/request/proof), issuer (credential offer, signer config, revocation), and verifier (authorization request, presentation exchange, DCQL, SIOP).
Holder Implementation
src/controllers/openid4vc/holder/holder.Controller.ts, src/controllers/openid4vc/holder/holder.service.ts, src/controllers/openid4vc/holder/credentialBindingResolver.ts
Holder controller exposes SD-JWT/mdoc credential endpoints and proof-request handling; service implements credential offer resolution, authorization/token flow, and revocation-aware credential persistence; binding resolver derives DID/JWK bindings and selects KMS keys.
Issuer Implementation
src/controllers/openid4vc/issuers/issuer.Controller.ts, src/controllers/openid4vc/issuers/issuer.service.ts, src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts, src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts
Issuer controller/service provide CRUD for issuer records with metadata; issuance-sessions controller/service manage credential offer creation, validation, revocation config, and pre-authorized code flows.
Verifier Implementation
src/controllers/openid4vc/verifiers/verifier.Controller.ts, src/controllers/openid4vc/verifiers/verifier.service.ts, src/controllers/openid4vc/verifier-sessions/verification-sessions.Controller.ts, src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts
Verifier controller/service manage verifier records; verification-sessions controller/service handle authorization request creation, session querying, and verified response retrieval with presentation normalization (W3C/JWT/mdoc).
Event Webhooks
src/events/openId4VcIssuanceSessionEvents.ts, src/events/openId4VcVerificationSessionEvents.ts
Event handlers for issuance/verification session state changes emit to webhooks and websockets.

X.509 Certificate & Trust System

Layer / File(s) Summary
Type Definitions
src/controllers/x509/x509.types.ts
X.509 certificate DTOs including authority/subject keys, DN fields, validity, key/extended key usage, alternative names, constraints, and CRL distribution.
Core Utilities
src/controllers/x509/crypto-util.ts, src/utils/constant.ts, src/utils/helpers.ts
PEM/DER private-key extraction; algorithm-to-curve and curve-to-JWK mappings; certificate validity window computation; ISO image normalization; verification-method introspection; trust-service token caching and HTTP calls.
Certificate Management
src/controllers/x509/x509.Controller.ts, src/controllers/x509/x509.service.ts
X.509 controller exposes create/import/trusted-cert endpoints; service implements self-signed DCS creation, certificate creation with optional KMS keys, certificate import with key validation, and trusted-cert list management.
Trust & Status Lists
src/utils/statusListService.ts, src/utils/oid4vc-agent.ts, src/utils/auth.ts
Status list server interaction (create/revoke via HTTP with signing); trust-service auth validation and client token caching; credential mapper for mixed issuance flows (DID/X.509 signers, SD-JWT/mdoc/PresentationAuthorization formats).
Authentication
src/controllers/auth/AuthController.ts, src/utils/auth.ts
New auth controller endpoint for org token acquisition; auth-type system (NoAuth/ClientAuth) with environment-based validation.

Record Purge System

Layer / File(s) Summary
Type & Config Definitions
src/purge/PurgeTypes.ts, src/purge/PurgeConstants.ts
Purge record types, agent modes (shared/dedicated), NATS/cron config shapes; TTL parsing; record-type selection from environment; NATS stream/consumer/subject/webhook constants.
Validation & Utilities
src/purge/PurgeConfigValidator.ts, src/utils/NatsAuthenticator.ts, src/utils/NatsConstants.ts
NATS connection validation during startup; NATS authenticator builder supporting nkey/creds/username-password; NATS reconnect and error-code constants.
Purge Execution
src/purge/PurgeDeleteRecord.ts, src/purge/PurgeWebhook.ts, src/purge/PurgeWorker.ts, src/purge/decorators/SchedulePurge.ts
Record deletion dispatcher; webhook sender with retry backoff; worker that consumes NATS messages and executes deletions with tenant awareness; decorator to schedule purge on method return.
Schedulers
src/purge/schedulers/NatsPurgeScheduler.ts, src/purge/schedulers/CronPurgeScheduler.ts, src/purge/PurgeSchedulerFactory.ts
NATS scheduler provisions JetStream streams/consumers and workers, publishes scheduled purge jobs; cron scheduler runs periodic scans querying expired records and deleting them; factory manages singleton scheduler instances.

Agent & Configuration Updates

Layer / File(s) Summary
Agent Setup
src/utils/agent.ts, src/cliAgent.ts
Agent now accepts id/key instead of name; AskarModule configured with explicit store credentials; DIDComm module consolidated with OOB/mediation/message-pickup/v1/v2 protocols and auto-accept; peer DID creation supplies KMS-backed keys; /invitation endpoint uses DidCommConnectionInvitationMessage or OOB fallback.
CLI & Configuration
src/cli.ts, samples/sample.ts, samples/sampleWithApp.ts, samples/cliConfig.json
CLI loads .env via dotenv; apiKey defaults to env, updateJwtSecret parsed from env boolean; wallet config key renamed to database; sample configs updated with wallet id/key sample10, BCovrin endpoint, API key/secret removed.
Server & Module Wiring
src/server.ts, src/index.ts
OpenTelemetry SDK now conditional on OTEL_ENABLED; auth config validation on startup; OpenID4VC session event handlers registered when webhooks/sockets enabled; body-parser limits from APP_*_BODY_SIZE env vars; WebSocket server uses named import.
Error Handling
src/errorHandlingService.ts, src/authentication.ts
MessageSendingError imported from didcomm instead of core; authentication uses crypto.randomUUID instead of Credo utilities; UUID imports from Node crypto.
Enums & Constants
src/enums/enum.ts
IndicioAcceptanceMechanism replaced generic accept with specific mechanisms (at_submission, for_session, on_file, product_eula, service_agreement); new SignerMethod, KeyAlgorithmCurve, CredentialFormat enums added.

Sequence Diagram(s)

sequenceDiagram
    participant Issuer as Issuer Agent
    participant IssuerCtrl as Issuance<br/>Session Ctrl
    participant IssuerService as Issuance<br/>Session Service
    participant OpenID4VcMod as OpenID4VC<br/>Issuer Module
    participant KMS as KMS / Askar
    participant StatusList as Status List<br/>Server

    Issuer->>IssuerCtrl: POST /create-credential-offer<br/>(publicIssuerId, credentials, isRevocable)
    IssuerCtrl->>IssuerService: createCredentialOffer(options)
    IssuerService->>IssuerService: Resolve issuer by publicIssuerId
    IssuerService->>IssuerService: Validate credential formats & signer config
    alt isRevocable == true
        IssuerService->>StatusList: checkAndCreateStatusList (if needed)
        StatusList-->>IssuerService: {uri, idx}
        IssuerService->>IssuerService: Add status_list to credential payload
    end
    IssuerService->>OpenID4VcMod: createCredentialOffer(mappedCredentials, ..., issuanceMetadata)
    OpenID4VcMod-->>IssuerService: {credentialOffer, issuanceSession}
    IssuerService-->>IssuerCtrl: {credentialOffer, issuanceSession}
    IssuerCtrl-->>Issuer: 201 Created
Loading
sequenceDiagram
    participant Holder as Holder Agent
    participant HolderCtrl as Holder<br/>Controller
    participant HolderService as Holder<br/>Service
    participant DIDComm as DIDComm Module<br/>(CredentialExchange)
    participant Issuer as Issuer Server

    Holder->>HolderCtrl: POST /request-credential<br/>(credentialOfferUri, credentialsToRequest)
    HolderCtrl->>HolderService: requestCredential(credentialOfferUri, ...)
    HolderService->>HolderService: Resolve credential offer & validate
    alt Pre-authorized flow
        HolderService->>Issuer: POST /token<br/>(client_id, client_secret)
        Issuer-->>HolderService: {access_token, ...}
    else Authorization code flow
        HolderService-->>HolderCtrl: {authorizationUrl}
        HolderCtrl-->>Holder: Redirect to issuer auth endpoint
    end
    HolderService->>Issuer: POST /credential<br/>(access_token, credential_requests)
    Issuer-->>HolderService: {credential_responses}
    HolderService->>DIDComm: Store credential in holder wallet
    HolderService-->>HolderCtrl: {credentials, ...}
    HolderCtrl-->>Holder: 200 OK
Loading
sequenceDiagram
    participant Agent as Agent
    participant Purge as Purge Scheduler<br/>(NATS)
    participant JetStream as JetStream<br/>Stream
    participant Worker as Purge<br/>Worker
    participant RecordService as Record<br/>Service

    Note over Agent,Worker: Scheduled Purge Flow
    Agent->>Purge: schedulePurge(recordType, recordId, ttlSeconds)
    Purge->>Purge: Calculate fireAt = now + ttl
    Purge->>JetStream: Publish PurgeJob with Nats-Schedule header
    JetStream-->>JetStream: Schedule message for fireAt time

    Note over JetStream,Worker: At scheduled fireAt time
    JetStream->>Worker: Deliver PurgeJob message
    Worker->>RecordService: deletePurgeRecord(recordType, recordId)
    RecordService-->>Worker: Deleted / Not Found
    alt Success
        Worker->>JetStream: ACK message
        Worker->>Worker: Optional: Send webhook notification
    else Max Retries Exceeded
        Worker->>JetStream: ACK (drop message)
    else Transient Failure
        Worker->>JetStream: NAK (re-deliver with backoff)
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~180 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • shitrerohit
  • GHkrishna
  • RinkalBhojani

Poem

🐰 With DIDComm's might and OpenID4VC's shine,

X.509 trust and purge schedules align,

From core to modules, a migration so bold,

Non-root Docker images and Credo 0.6 gold!

The controller cascade awaits its review—

A rabbit's delight in the infrastructure anew! 🎉

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch merge/main-sync-to-main

Comment thread src/cliAgent.ts Fixed
tipusinghaw and others added 28 commits April 29, 2026 18:12
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
…de credential payload

Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: RinkalBhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* updated creat x509 method and few types

Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>

* corrected mapper function for mdoc payload

Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>

---------

Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
…ests

Signed-off-by: Rinkal Bhojani <rinkal.bhojani@ayanworks.com>
tipusinghaw and others added 7 commits April 29, 2026 18:35
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
* feat: add agent custom document loader

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* feat: add appropriate warning for incorrect custom document loader initiation

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: error handling

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: formatting

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: coderabbit suggestions

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

* fix: reviewer suggestions

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>

---------

Signed-off-by: Krishna Waske <krishna.waske@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
@tipusinghaw tipusinghaw force-pushed the merge/main-sync-to-main branch from e0ffd16 to 597987b Compare April 29, 2026 13:22
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
…yanworks.com>

On behalf of Deepak Nemade <deepak.nemade@ayanworks.com>, I, Tipu_Singh <tipu.singh@ayanworks.com>, hereby add my Signed-off-by to this commit: bcbc82e

Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/server.ts (1)

21-34: ⚠️ Potential issue | 🟡 Minor

Fix import-order lint errors in newly added imports.

The additions around Line 21 and Line 32-34 violate import/order grouping/ordering and will fail lint.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/server.ts` around lines 21 - 34, The imports violate ESLint import/order:
move the external package import TenantAgent (from '@credo-ts/tenants') into the
external/third-party import group and reorder groups so built-ins/external come
before local modules; then group and, if your project requires, alphabetize
within groups for symbols like validateAuthConfig, ErrorMessages, BaseError,
basicMessageEvents, connectionEvents, credentialEvents, proofEvents,
questionAnswerEvents, reuseConnectionEvents, RegisterRoutes, SecurityMiddleware,
openId4VcIssuanceSessionEvents, and openId4VcVerificationSessionEvents to
satisfy the import/order rule.
♻️ Duplicate comments (5)
src/events/WebSocketEvents.ts (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Use a type-only import for Server.

At Line 1, Server is only used in type position, so this should be import type to satisfy the lint rule and avoid runtime import overhead.

Suggested change
-import { Server } from 'ws'
+import type { Server } from 'ws'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/WebSocketEvents.ts` at line 1, Change the runtime import of Server
to a type-only import since Server is only used in type positions: replace the
current import of Server from 'ws' with a type import (i.e., use "import type {
Server } from 'ws'") in WebSocketEvents.ts so the linter warning is resolved and
no runtime import overhead occurs; update any related import usages that
reference Server as a type if necessary.
src/utils/agent.ts (1)

152-163: ⚠️ Potential issue | 🔴 Critical

Fix double-response flow and invitation domain construction.

Line 152-163 can send multiple responses for one request, and Line 162 concatenates a string[] into URL text. This can trigger runtime response errors and broken invitation URLs.

Suggested fix
       if (typeof req.query.d_m === 'string') {
         const invitation = await DidCommConnectionInvitationMessage.fromUrl(req.url.replace('d_m=', 'c_i='))
-        res.send(invitation.toJSON())
-      }
-      if (typeof req.query.c_i === 'string') {
+        return res.send(invitation.toJSON())
+      } else if (typeof req.query.c_i === 'string') {
         const invitation = await DidCommConnectionInvitationMessage.fromUrl(req.url)
-        res.send(invitation.toJSON())
+        return res.send(invitation.toJSON())
       } else {
         const { outOfBandInvitation } = await agent.modules.didcomm.oob.createInvitation()
-
-        res.send(outOfBandInvitation.toUrl({ domain: endpoints + '/invitation' }))
+        const domain = `${endpoints[0]}/invitation`
+        return res.send(outOfBandInvitation.toUrl({ domain }))
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/agent.ts` around lines 152 - 163, The handler can send multiple
responses because the two separate if blocks for req.query.d_m and req.query.c_i
both call res.send; change the second "if (typeof req.query.c_i === 'string')"
to "else if" so only one branch runs, and ensure you return or otherwise stop
after res.send to avoid fallthrough. Also fix invitation URL construction:
endpoints may be a string[] — use a single string (e.g.,
Array.isArray(endpoints) ? endpoints[0] : endpoints) or join it appropriately
before calling outOfBandInvitation.toUrl, i.e., compute a string baseUrl and
pass baseUrl + '/invitation' to outOfBandInvitation.toUrl; update references to
DidCommConnectionInvitationMessage.fromUrl,
agent.modules.didcomm.oob.createInvitation, outOfBandInvitation.toUrl,
endpoints, and res.send accordingly.
src/purge/schedulers/NatsPurgeScheduler.ts (1)

137-149: ⚠️ Potential issue | 🔴 Critical

Do not delete streams based only on subject prefix

Current logic can remove unrelated streams in shared NATS if they use purge.* subjects. Restrict deletion to streams you can prove are owned (e.g., exact name/metadata/exact-subject-set match).

🛡️ Safer ownership check direction
-      const isPurgeStream = stream.config.subjects?.some(
-        (s: string) => s.startsWith('purge.schedule.') || s.startsWith('purge.execute.'),
-      )
-      if (isPurgeStream) {
+      const sameName = stream.config.name === PURGE_STREAM
+      const sameSubjects =
+        Array.isArray(stream.config.subjects) &&
+        Array.isArray(_subjects) &&
+        stream.config.subjects.length === _subjects.length &&
+        stream.config.subjects.every((s: string) => _subjects.includes(s))
+      const ownedByUs = sameName || sameSubjects
+      if (ownedByUs) {
         await this.jsm.streams.delete(stream.config.name)
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/schedulers/NatsPurgeScheduler.ts` around lines 137 - 149,
deleteStaleStreams currently deletes any stream that has subjects starting with
'purge.schedule.' or 'purge.execute.' which can remove unrelated streams; update
deleteStaleStreams in NatsPurgeScheduler to only delete when you can prove
ownership by (1) checking a stream metadata flag (e.g.,
stream.config?.meta?.managedBy === 'purge-scheduler' or similar project-specific
key) and (2) verifying the subjects exactly match the expected set (compare
stream.config.subjects array to an expectedSubjects array or ensure there are no
extra subjects) before calling this.jsm.streams.delete(stream.config.name); keep
the existing skip for PURGE_STREAM and only perform deletion when both ownership
metadata and exact-subject-set checks pass.
src/controllers/x509/x509.service.ts (1)

82-82: ⚠️ Potential issue | 🔴 Critical

Stop logging secret-bearing material in certificate/key paths.

Line 82 logs full certificate options (can include seed/private inputs), and Line 223 logs generated seed directly. This is a direct secret-leak risk.

🔐 Suggested patch
-    agent.config.logger.debug(`createCertificate options:`, options)
+    agent.config.logger.debug('createCertificate called')

-    agent.config.logger.debug(`createKey: got seed ${seed}`)
+    agent.config.logger.debug('createKey: seed generated')

Also applies to: 223-223

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/x509.service.ts` at line 82, The debug statements in
createCertificate are logging secret-bearing values: remove or redact any
sensitive fields when calling agent.config.logger.debug (do not log full
`options`), and stop logging the generated seed value directly (where the seed
variable is logged around line 223); instead log only non-sensitive metadata
(e.g., presence/length or masked values) or a success message. Locate the debug
call to agent.config.logger.debug that prints `options` and the log that prints
the generated seed, and replace them with redacted-safe logging that omits
private keys, seeds, file paths containing secrets, or prints a masked
representation.
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts (1)

57-61: ⚠️ Potential issue | 🔴 Critical

Guard x5c access and only attach parsed cert in x5c mode.

Line 58 dereferences requestSigner.x5c[0] without validation, and Line 80 sets options.requestSigner.x5c even for DID signers. This can break authorization-request creation.

🐛 Suggested patch
     } else {
       requestSigner = dto.requestSigner as OpenId4VcIssuerX5cOptions
+      if (!Array.isArray(requestSigner.x5c) || typeof requestSigner.x5c[0] !== 'string') {
+        throw new Error('requestSigner.x5c[0] is required for x5c signer')
+      }

       parsedCertificate = X509Service.parseCertificate(agentReq.agent.context, {
         encodedCertificate: requestSigner.x5c[0],
       })
       requestSigner.issuer = parsedCertificate.sanUriNames[0]
       requestSigner.clientIdPrefix = dto.requestSigner.clientIdPrefix ?? ClientIdPrefix.X509Hash
     }
@@
-    if (parsedCertificate) {
-      parsedCertificate.publicJwk.keyId = requestSigner.keyId
-    }
-    options.requestSigner.x5c = [parsedCertificate]
+    if (parsedCertificate) {
+      parsedCertificate.publicJwk.keyId = requestSigner.keyId
+      options.requestSigner.x5c = [parsedCertificate]
+    }

Also applies to: 77-81

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`
around lines 57 - 61, Guard access to requestSigner.x5c and only attach
x5c-related data when running in X509 mode: before calling
X509Service.parseCertificate and reading requestSigner.x5c[0], verify
requestSigner.x5c is an array and has at least one element; only then call
X509Service.parseCertificate and set requestSigner.issuer and
parsedCertificate-based fields (e.g., clientIdPrefix defaulting to
ClientIdPrefix.X509Hash). Conversely, when handling DID signers, do not set
options.requestSigner.x5c — ensure any assignment to options.requestSigner.x5c
happens only in the branch where x5c exists. Update code paths around
parsedCertificate, X509Service.parseCertificate, requestSigner.x5c,
dto.requestSigner.clientIdPrefix and options.requestSigner.x5c accordingly.
🟠 Major comments (31)
src/controllers/didcomm/basic-messages/BasicMessageController.ts-11-58 (1)

11-58: ⚠️ Potential issue | 🟠 Major

Controller is fully disabled, effectively removing the API.

Lines 11–58 comment out all route decorators and handlers, so these endpoints are no longer served. If this is intentional, it should be an explicit API deprecation/removal change; otherwise restore the controller before merge.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/didcomm/basic-messages/BasicMessageController.ts` around
lines 11 - 58, The BasicMessageController class and its route handlers
(BasicMessageController, getBasicMessages, sendMessage) were commented out,
removing the DIDComm basic-messages API; restore the controller by uncommenting
the class declaration, decorators (`@Tags`, `@Route`, `@Security`, `@injectable`) and
the two methods (getBasicMessages and sendMessage) along with their route
decorators (`@Get`('/:connectionId') and `@Post`('/:connectionId')), request
parameter decorators (`@Request`, `@Path`, `@Body`), and error handling so the
endpoints are served again, or if removal was intended, replace the commented
code with an explicit deprecation notice and remove route decorators accordingly
to make the change intentional and visible in the codebase.
src/cli.ts-5-9 (1)

5-9: ⚠️ Potential issue | 🟠 Major

Load dotenv before any imports from modules that access environment variables.

In ESM, all imports are hoisted to module top and execute before module body code. Even though dotenv.config() appears at line 7 in source, the import of ./cliAgent.js at line 9 is also hoisted and executes first. Since cliAgent.ts accesses process.env at module-level (lines 151–152 in express app setup), dotenv runs too late.

Use import 'dotenv/config' as a side-effect import at the very top to ensure environment variables load before any module imports.

Suggested change
+import 'dotenv/config'
 import type { AriesRestConfig } from './cliAgent.js'
 
 import yargs from 'yargs'
 import { hideBin } from 'yargs/helpers'
-import dotenv from 'dotenv'
-
-dotenv.config()
 
 import { runRestAgent } from './cliAgent.js'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 5 - 9, Move dotenv initialization to run before any
module imports by replacing the current dotenv.config() usage with a top-level
side-effect import; ensure the very first import is the side-effect import of
dotenv (so environment variables are set before cliAgent.js executes), since
cliAgent.js (and its runRestAgent/exported setup) reads process.env at module
scope (e.g., the module-level express app config). Update the file so the
side-effect import occurs before importing runRestAgent/cliAgent.js and remove
or stop using dotenv.config() later in this module.
src/controllers/did/DidController.ts-494-504 (1)

494-504: ⚠️ Potential issue | 🟠 Major

Guard missing DID record before import.

When getCreatedDids returns empty, didDocument is undefined and the subsequent import call is unsafe.

Proposed fix
       const createdDid = await agent.dids.getCreatedDids({
         method: DidMethod.Key,
         did: didOptions.did,
       })
       didDocument = createdDid[0]?.didDocument
+      if (!didDocument) {
+        throw new BadRequestError(`DID not found in wallet: ${didOptions.did}`)
+      }

       await agent.dids.import({
         did,
         overwrite: true,
         didDocument,
       })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/did/DidController.ts` around lines 494 - 504, getCreatedDids
can return an empty array so didDocument may be undefined before calling
agent.dids.import; update the code around getCreatedDids/createdDid/didDocument
to check that createdDid[0] and createdDid[0].didDocument exist (or throw a
clear error) and only call agent.dids.import when didDocument is present.
Specifically, add a guard after the call to agent.dids.getCreatedDids({ method:
DidMethod.Key, did: didOptions.did }) to validate createdDid[0]?.didDocument (or
return/raise a descriptive error) before invoking agent.dids.import({ did,
overwrite: true, didDocument }).
src/controllers/did/DidController.ts-276-277 (1)

276-277: ⚠️ Potential issue | 🟠 Major

Validate INDICIO_NYM_URL before calling Axios.

This path currently assumes env presence and can fail with a low-signal runtime error.

Proposed fix
-        const INDICIO_NYM_URL = process.env.INDICIO_NYM_URL as string
+        if (!process.env.INDICIO_NYM_URL) {
+          throw new InternalServerError('INDICIO_NYM_URL is not set in environment variables')
+        }
+        const INDICIO_NYM_URL = process.env.INDICIO_NYM_URL as string
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/did/DidController.ts` around lines 276 - 277, The code uses
INDICIO_NYM_URL directly and calls axios.post(INDICIO_NYM_URL, key) without
validating the environment variable; update the DidController logic to check
that process.env.INDICIO_NYM_URL is present and is a valid URL before calling
axios.post, and handle the missing/invalid case by returning/throwing a clear
error or logging and aborting the request; locate the constant assignment
INDICIO_NYM_URL and the axios.post call in DidController.ts and add the
guard/validation (e.g., ensure non-empty string and URL.parse/RegExp or URL
constructor validation) and use the validated value for the axios.post call.
src/controllers/openid4vc/holder/credentialBindingResolver.ts-38-43 (1)

38-43: ⚠️ Potential issue | 🟠 Major

Clamp and validate batchSize before array allocation.

Current logic can exceed the stated max-10 rule when requestBatch is numeric, and invalid numeric values can break new Array(batchSize).

Proposed fix
-    const batchSize =
-      requestBatch === true
-        ? Math.min(issuerMaxBatchSize, 10)
-        : typeof requestBatch === 'number'
-          ? Math.min(issuerMaxBatchSize, requestBatch)
-          : 1
+    const requestedBatch =
+      requestBatch === true ? 10 : typeof requestBatch === 'number' ? requestBatch : 1
+    const batchSize = Math.max(1, Math.min(10, issuerMaxBatchSize, requestedBatch))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/holder/credentialBindingResolver.ts` around lines
38 - 43, The batchSize calculation can produce values >10 and
non-integer/invalid values which will break array allocation; update the logic
around batchSize (the calculation using requestBatch and issuerMaxBatchSize) to:
coerce numeric requestBatch to a safe integer, clamp it to a minimum of 1 and a
maximum of Math.min(issuerMaxBatchSize, 10), and fallback to 1 for non-numeric
or out-of-range inputs before any call to new Array(batchSize) so allocation
always receives a valid positive integer.
src/events/ProofEvents.ts-12-15 (1)

12-15: ⚠️ Potential issue | 🟠 Major

Harden tenant-id extraction from correlation id.

split('tenant-')[1] can produce undefined for unexpected formats, which can break tenant agent resolution.

Proposed fix
-    if (event.metadata.contextCorrelationId && event.metadata.contextCorrelationId !== 'default') {
+    if (
+      event.metadata.contextCorrelationId &&
+      event.metadata.contextCorrelationId !== 'default' &&
+      event.metadata.contextCorrelationId.startsWith('tenant-')
+    ) {
       const tenantAgent = await agent.modules.tenants.getTenantAgent({
         tenantId: event.metadata.contextCorrelationId.split('tenant-')[1],
       })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/ProofEvents.ts` around lines 12 - 15, The extraction of tenant id
using event.metadata.contextCorrelationId.split('tenant-')[1] is fragile; update
the logic in the ProofEvents handling code to safely parse and validate the
tenant id before calling agent.modules.tenants.getTenantAgent: check that
event.metadata.contextCorrelationId exists, that it contains the expected prefix
(e.g., startsWith('tenant-') or matches /tenant-([^]+)/), extract the capture
group or substring, verify it is not undefined/empty, and if invalid log or bail
out gracefully (or throw a clear error) instead of passing undefined to
getTenantAgent.
src/events/ProofEvents.ts-2-3 (1)

2-3: ⚠️ Potential issue | 🟠 Major

Split mixed import and use type-only imports.

Line 3 mixes type and runtime imports. DidCommProofStateChangedEvent is used only as a type annotation (line 9) and must use the type keyword. Separate it from DidCommProofEventTypes, which is a runtime value. This violates both @typescript-eslint/consistent-type-imports and import/order rules.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/ProofEvents.ts` around lines 2 - 3, The import mixes a type-only
symbol and a runtime value; change the import so DidCommProofStateChangedEvent
is imported as a type-only import (e.g., "import type {
DidCommProofStateChangedEvent } from '@credo-ts/didcomm'") while keeping
DidCommProofEventTypes as a normal (runtime) import, and ensure imports are
ordered consistently with other imports (Agent remains as is) so the file uses
type-only imports for DidCommProofStateChangedEvent and value import for
DidCommProofEventTypes.
src/controllers/openid4vc/types/issuer.types.ts-4-7 (1)

4-7: ⚠️ Potential issue | 🟠 Major

Remove unused imports and fix type-only import syntax.

These imports violate the type-import rule and include three entirely unused symbols. This blocks CI.

Proposed fix
-import { Kms } from '@credo-ts/core'
-import { OpenId4VciCreateCredentialOfferOptions, OpenId4VciSignCredentials } from '@credo-ts/openid4vc'
-
-import { SignerMethod } from '../../../enums/enum'
+import type { SignerMethod } from '../../../enums/enum'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/types/issuer.types.ts` around lines 4 - 7, Remove
the unused Kms import and convert the OpenId4VciCreateCredentialOfferOptions and
OpenId4VciSignCredentials imports to type-only imports to satisfy the
type-import rule: delete "Kms" from the imports, change the import from
'@credo-ts/openid4vc' to "import type { OpenId4VciCreateCredentialOfferOptions,
OpenId4VciSignCredentials } from '@credo-ts/openid4vc'", and keep the existing
import of SignerMethod as-is (SignerMethod) so only the required symbols remain.
src/authentication.ts-11-13 (1)

11-13: ⚠️ Potential issue | 🟠 Major

Fix import ordering and type-only import to pass CI.

The import block violates @typescript-eslint/consistent-type-imports and import/order rules. TenantAgent is used only as a type (in type annotations and function signatures) and must be a type-only import. Additionally, randomUUID from node:crypto (a builtin import) is currently positioned after sibling imports but should be grouped with external imports.

Proposed fix
 import type { RestAgentModules, RestMultiTenantAgentModules } from './cliAgent'
 import type { Request } from 'express'
+import type { TenantAgent } from '@credo-ts/tenants'
 
 import { Agent, LogLevel } from '@credo-ts/core'
 import jwt, { decode } from 'jsonwebtoken'
+import { randomUUID as uuid } from 'node:crypto'
 import { container } from 'tsyringe'
 
 import { AgentRole, ErrorMessages, SCOPES } from './enums'
 import { StatusException } from './errors'
 import { TsLogger } from './utils/logger'
-import { TenantAgent } from '@credo-ts/tenants'
-import { randomUUID as uuid } from 'node:crypto'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/authentication.ts` around lines 11 - 13, Change the import of TenantAgent
to a type-only import and reorder the imports so the built-in crypto import is
grouped with other builtins/externals: replace "import { TenantAgent } from
'@credo-ts/tenants'" with a type-only form ("import type { TenantAgent } ...")
and move "import { randomUUID as uuid } from 'node:crypto'" up into the
external/builtin import group ahead of package imports so it satisfies
import/order and `@typescript-eslint/consistent-type-imports`; ensure all
references to TenantAgent and uuid remain unchanged.
src/events/openId4VcIssuanceSessionEvents.ts-17-29 (1)

17-29: ⚠️ Potential issue | 🟠 Major

Decouple webhook failure from websocket emission.

At Line 18, if webhook delivery throws, websocket publishing at Line 22 is skipped. This can silently drop session updates on socket clients.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/openId4VcIssuanceSessionEvents.ts` around lines 17 - 29, The
webhook send (sendWebhookEvent) can throw and prevent the websocket send
(sendWebSocketEvent) from running; wrap the sendWebhookEvent call in a try/catch
that logs the error with agent.config.logger.error but does not rethrow, then
always proceed to call sendWebSocketEvent when config.socketServer is set; keep
the same payload shape (use body and event) and ensure errors from
sendWebhookEvent are recorded (include error details) while not affecting
websocket emission.
src/events/openId4VcVerificationSessionEvents.ts-15-27 (1)

15-27: ⚠️ Potential issue | 🟠 Major

Don’t let webhook failures suppress websocket delivery.

At Line 16, a thrown webhook error stops execution, so the websocket event at Line 20 won’t fire. This couples two delivery channels and can cause dropped notifications.

Suggested hardening
       if (config.webhookUrl) {
-        await sendWebhookEvent(config.webhookUrl + '/openid4vc-verification', body, agent.config.logger)
+        try {
+          await sendWebhookEvent(config.webhookUrl + '/openid4vc-verification', body, agent.config.logger)
+        } catch (error) {
+          agent.config.logger.error('[OpenID4VC] Failed to deliver verification webhook', error)
+        }
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/openId4VcVerificationSessionEvents.ts` around lines 15 - 27, The
webhook call (sendWebhookEvent) can throw and currently prevents the subsequent
sendWebSocketEvent from running; wrap the webhook delivery in a try/catch (or
fire-and-forget with error handling) so any exception is caught and logged via
agent.config.logger without rethrowing, then proceed to call sendWebSocketEvent
when config.socketServer is set; reference sendWebhookEvent, sendWebSocketEvent,
config.webhookUrl, config.socketServer, agent.config.logger, event, and body to
locate and modify the code.
src/utils/NatsAuthenticator.ts-11-29 (1)

11-29: ⚠️ Potential issue | 🟠 Major

Fail closed on invalid NATS_AUTH_TYPE values.

Current default branch returns {} for unknown values, so a typo can silently drop auth config and attempt unauthenticated connection.

Suggested fix
 export function buildNatsAuthenticator(nats: NatsConfig): { authenticator?: Authenticator } {
-  const authType = (process.env.NATS_AUTH_TYPE as NatsAuthType) || 'none'
+  const rawAuthType = process.env.NATS_AUTH_TYPE ?? 'none'
+  const allowed: ReadonlySet<string> = new Set(['nkey', 'creds', 'usernamePassword', 'none'])
+  if (!allowed.has(rawAuthType)) {
+    throw new Error(`[NATS] Unsupported NATS_AUTH_TYPE=${rawAuthType}`)
+  }
+  const authType = rawAuthType as NatsAuthType
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/NatsAuthenticator.ts` around lines 11 - 29, The switch on authType
currently falls through to the default returning {} and can silently ignore
typos; update the logic that evaluates authType (the authType variable and
switch in NatsAuthenticator.ts) so unknown values throw a clear Error instead of
returning an empty object—keep the explicit 'none' case returning {} but replace
the default branch with an Error that includes the invalid authType value (e.g.
referencing authType) so callers immediately fail on invalid NATS_AUTH_TYPE; no
change to the nkeyAuthenticator, credsAuthenticator, or
usernamePasswordAuthenticator calls other than preserving their existing checks.
src/cliAgent.ts-221-223 (1)

221-223: ⚠️ Potential issue | 🟠 Major

Explicit false can never disable auto-accept connections here.

Both the module config and the getModules / getWithTenantModules call sites use || true, so a valid false value is overwritten to true. That makes the REST config impossible to honor.

🔧 Suggested fix
-        autoAcceptConnections: autoAcceptConnections || true,
+        autoAcceptConnections: autoAcceptConnections ?? true,
-      autoAcceptConnections || true,
+      autoAcceptConnections ?? true,

Also applies to: 466-495

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 221 - 223, The config currently forces
autoAcceptConnections true because "autoAcceptConnections || true" overwrites
explicit false; change assignments to respect a boolean false by using the
nullish coalescing or explicit boolean check: replace "autoAcceptConnections:
autoAcceptConnections || true" with "autoAcceptConnections:
autoAcceptConnections ?? true" (or "autoAcceptConnections: typeof
autoAcceptConnections === 'boolean' ? autoAcceptConnections : true") and apply
the same fix to the other occurrences mentioned (the other
getModules/getWithTenantModules assignments around the referenced block).
src/events/CredentialEvents.ts-15-20 (1)

15-20: ⚠️ Potential issue | 🟠 Major

Preserve non-prefixed tenant IDs when normalizing contextCorrelationId.

split('tenant-')[1] returns undefined when the metadata already contains a bare tenant id, so the later lookups fall back to the base agent and outOfBandId / credentialData can be resolved from the wrong wallet. Keep the original value unless the tenant- prefix is actually present.

🔧 Suggested fix
-      const tenantId = (!event.metadata.contextCorrelationId || event.metadata.contextCorrelationId === 'default') ? event.metadata.contextCorrelationId : event.metadata.contextCorrelationId.split('tenant-')[1]
+      const rawContextCorrelationId = event.metadata.contextCorrelationId
+      const tenantId =
+        !rawContextCorrelationId || rawContextCorrelationId === 'default'
+          ? rawContextCorrelationId
+          : rawContextCorrelationId.startsWith('tenant-')
+            ? rawContextCorrelationId.slice('tenant-'.length)
+            : rawContextCorrelationId
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/events/CredentialEvents.ts` around lines 15 - 20, The normalization of
contextCorrelationId in CredentialEvents.ts incorrectly turns bare tenant IDs
into undefined because split('tenant-')[1] is used unconditionally; update the
tenantId computation (the variable you set from
event.metadata.contextCorrelationId) to: if contextCorrelationId is falsy or
exactly 'default' keep it as-is, otherwise if it startsWith('tenant-') extract
the portion after the prefix, else preserve the original value; update any usage
relying on tenantId (e.g., the body creation that sets contextCorrelationId) so
lookups use the preserved or extracted tenant id.
src/cliAgent.ts-208-212 (1)

208-212: ⚠️ Potential issue | 🟠 Major

The custom document-loader flag is wired backwards.

When isCustomDocumentLoaderEnabled() returns true, this branch creates the default W3cCredentialsModule and skips CustomDocumentLoader. The flag currently disables the custom loader instead of enabling it.

🔧 Suggested fix
-    w3cCredentials: isCustomDocumentLoaderEnabled()
-      ? new W3cCredentialsModule()
-      : new W3cCredentialsModule({
-          documentLoader: CustomDocumentLoader,
-        }),
+    w3cCredentials: isCustomDocumentLoaderEnabled()
+      ? new W3cCredentialsModule({
+          documentLoader: CustomDocumentLoader,
+        })
+      : new W3cCredentialsModule(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 208 - 212, The ternary that initializes
w3cCredentials is wired backwards: isCustomDocumentLoaderEnabled() currently
returns true but constructs the default W3cCredentialsModule, skipping
CustomDocumentLoader; fix it by inverting the branches so that when
isCustomDocumentLoaderEnabled() is true you instantiate new
W3cCredentialsModule({ documentLoader: CustomDocumentLoader }) and when false
you instantiate the default new W3cCredentialsModule(); update the expression
that references isCustomDocumentLoaderEnabled(), W3cCredentialsModule, and
CustomDocumentLoader accordingly.
src/controllers/agent/AgentController.ts-23-29 (1)

23-29: ⚠️ Potential issue | 🟠 Major

Don't repurpose label as the tenant correlation id.

Line 25 changes a stable display/config field into tenancy context. For the default agent this can become 'default' or undefined, and existing clients lose the actual configured agent label. Keep label sourced from agent config and expose tenant context under a separate field if you need both.

🔧 Suggested fix
       return {
-        label: request.agent.context.contextCorrelationId,
+        label: request.agent.config.label,
         endpoints: request.agent.modules.didcomm.config.endpoints,
         isInitialized: request.agent.isInitialized,
         publicDid: undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/agent/AgentController.ts` around lines 23 - 29, The return
payload is repurposing label to hold tenancy context; restore label to the
agent's configured display label (use the agent config property, e.g.
request.agent.config.label or request.agent.label) and stop assigning
request.agent.context.contextCorrelationId to label, and if you need the tenant
correlation id include it as a separate field such as tenantCorrelationId:
request.agent.context.contextCorrelationId while keeping endpoints
(request.agent.modules.didcomm.config.endpoints), isInitialized
(request.agent.isInitialized) and publicDid unchanged.
src/utils/auth.ts-25-30 (1)

25-30: ⚠️ Potential issue | 🟠 Major

Require an explicit auth mode instead of defaulting to NoAuth.

If TRUST_SERVICE_AUTH_TYPE is missing, startup silently drops into unauthenticated mode. For a security-sensitive trust-service flow, that is an unsafe misconfiguration path; fail fast unless this fallback is deliberately restricted to local development.

🔧 Suggested fix
 export function getAuthType(): AuthType {
-  const authType = process.env.TRUST_SERVICE_AUTH_TYPE as AuthType
+  const authType = process.env.TRUST_SERVICE_AUTH_TYPE as AuthType | undefined
   if (!authType) {
-    console.warn('[getAuthType] TRUST_SERVICE_AUTH_TYPE is not set — defaulting to NoAuth')
-    return AuthTypes.NoAuth
+    throw new Error('TRUST_SERVICE_AUTH_TYPE must be set explicitly')
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/auth.ts` around lines 25 - 30, The getAuthType function currently
defaults to AuthTypes.NoAuth when TRUST_SERVICE_AUTH_TYPE is unset; change it to
fail fast by throwing an Error if the env var is missing, except in an explicit
development override (e.g. allow when process.env.NODE_ENV === 'development' or
process.env.ALLOW_NO_AUTH === 'true'); update getAuthType to read
process.env.TRUST_SERVICE_AUTH_TYPE as AuthType, validate it against known
AuthTypes, and throw a descriptive error (including the variable name) when
absent or invalid unless the explicit dev override is present.
src/cliAgent.ts-592-599 (1)

592-599: ⚠️ Potential issue | 🟠 Major

Close the HTTP server and gracefully shut down the agent before forcing process exit.

The shutdown handler only stops the purge schedulers. It misses closing the HTTP server (app.close()), which cuts off in-flight requests, and omits any wallet/database cleanup or transport teardown. The immediate process.exit(0) prevents async cleanup operations from completing, making shutdown nondeterministic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 592 - 599, The shutdown handler currently only
calls stopPurgeSchedulers and immediately process.exit(0), which prevents
graceful teardown; update the shutdown function (shutdown) to sequentially and
asynchronously: log the shutdown start via agent.config.logger, stop purge
schedulers (stopPurgeSchedulers), await closing the HTTP server (await
app.close() or app.shutdown equivalent), invoke and await any
wallet/database/transport cleanup helpers (e.g., wallet.close(), db.close(),
transport.stop() or their actual teardown functions), catch and log errors, and
only call process.exit(0) after those awaitable cleanups complete (or use a
small timeout fallback) so in-flight requests and resources are properly closed.
src/purge/schedulers/CronPurgeScheduler.ts-190-193 (1)

190-193: ⚠️ Potential issue | 🟠 Major

Do not let webhook failures abort the purge batch.

At Line 191, webhook errors bubble out of deleteAndNotify, which causes the surrounding scan block to fail and skip remaining records for that type.

Suggested fix
     if (webhookUrl) {
-      await sendPurgeWebhook(webhookUrl, recordId, recordType, tenantId, status, logger)
+      try {
+        await sendPurgeWebhook(webhookUrl, recordId, recordType, tenantId, status, logger)
+      } catch (err: any) {
+        logger.error('[Purge] Webhook dispatch failed after delete', {
+          recordId,
+          recordType,
+          tenantId,
+          error: err?.message,
+        })
+      }
     }
 
     return true

Also applies to: 97-103

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/schedulers/CronPurgeScheduler.ts` around lines 190 - 193, The
webhook call in CronPurgeScheduler (calls to sendPurgeWebhook inside
deleteAndNotify) must not allow failures to propagate and abort the purge batch;
wrap each await sendPurgeWebhook(webhookUrl, recordId, recordType, tenantId,
status, logger) in a try/catch, log the error via the existing logger (including
context: recordId, recordType, tenantId, status) and continue without rethrowing
so the surrounding scan loop can proceed; apply the same change to both
occurrences (the block around lines where deleteAndNotify calls sendPurgeWebhook
and the earlier occurrence around lines 97-103) to ensure webhook failures don't
skip remaining records.
src/purge/schedulers/CronPurgeScheduler.ts-17-35 (1)

17-35: ⚠️ Potential issue | 🟠 Major

Prevent duplicate cron jobs on repeated starts.

Line 20 overwrites this.job without stopping an existing task. Re-initialization can run duplicate scans and duplicate deletions/webhooks.

Suggested fix
   async start(agent: Agent, config: PurgeConfig, webhookUrl: string | undefined): Promise<void> {
     const { cronConfig } = config
+    if (this.job) {
+      this.job.stop()
+      this.job = null
+      agent.config.logger.warn('[Purge] Existing cron job was running and has been replaced')
+    }
 
     this.job = cron.schedule(cronConfig.cronSchedule, () => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/schedulers/CronPurgeScheduler.ts` around lines 17 - 35, The start
method in CronPurgeScheduler overwrites this.job on repeated starts and can
create duplicate scheduled tasks; before creating a new cron.schedule in start
(method CronPurgeScheduler.start) check if this.job exists and gracefully
stop/destroy it (e.g. call this.job.stop() or this.job.destroy()), clear any
running flags if needed (this.isRunning = false only if appropriate), then
assign the new scheduled job and proceed to call runScan as before to avoid
duplicate scans/deletions/webhooks.
src/purge/PurgeWorker.ts-30-38 (1)

30-38: ⚠️ Potential issue | 🟠 Major

Keep the worker alive when consumer.consume() throws.

If Line 31 throws (e.g., transient broker/network issues), start() exits and this worker stops permanently.

Suggested fix
     while (true) {
-      const messages = await consumer.consume()
-      console.log(`[Purge][Worker] Consuming messages — consumer=${this.consumerName}`)
-      for await (const msg of messages) {
-        await this.processMessage(msg, agent)
-      }
-      console.warn(`[Purge][Worker] Consume loop ended — restarting consumer=${this.consumerName}`)
-      agent.config.logger.warn('[Purge] Consume loop ended — restarting', { consumer: this.consumerName })
+      try {
+        const messages = await consumer.consume()
+        console.log(`[Purge][Worker] Consuming messages — consumer=${this.consumerName}`)
+        for await (const msg of messages) {
+          await this.processMessage(msg, agent)
+        }
+        console.warn(`[Purge][Worker] Consume loop ended — restarting consumer=${this.consumerName}`)
+        agent.config.logger.warn('[Purge] Consume loop ended — restarting', { consumer: this.consumerName })
+      } catch (err: any) {
+        agent.config.logger.error('[Purge] consume() failed; retrying', {
+          consumer: this.consumerName,
+          error: err?.message,
+        })
+      }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/PurgeWorker.ts` around lines 30 - 38, The consume call can throw
and currently will exit the infinite loop; wrap the await
this.consumer.consume() (inside the start()/consume loop in PurgeWorker.ts) in a
try/catch so exceptions are logged (use agent.config.logger.error or
console.error) and do not break the loop, optionally implement a short
backoff/retry delay before continuing; keep the inner for-await (for await
(const msg of messages) { await this.processMessage(msg, agent) }) unchanged so
successful consumes still process messages, and include the consumerName in the
error log for context.
src/utils/statusListService.ts-91-93 (1)

91-93: ⚠️ Potential issue | 🟠 Major

Validate status-list size before allocating the backing array.

Line 91 can produce NaN/non-positive values (e.g., unset or invalid STATUS_LIST_DEFAULT_SIZE), and Line 92 then throws at runtime via new Array(size).

Suggested fix
-            const size = listSize || Number(process.env.STATUS_LIST_DEFAULT_SIZE);
-            const statusList = new StatusList(new Array(size).fill(0), 1);
+            const envDefaultSize = Number(process.env.STATUS_LIST_DEFAULT_SIZE)
+            const size = listSize ?? envDefaultSize
+            if (!Number.isInteger(size) || size <= 0) {
+                throw new Error(`Invalid status list size: ${size}`)
+            }
+            const statusList = new StatusList(new Array(size).fill(0), 1);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/statusListService.ts` around lines 91 - 93, The allocation uses
size = listSize || Number(process.env.STATUS_LIST_DEFAULT_SIZE) which can be
NaN, zero, negative, or non-integer and will blow up at new Array(size);
validate and normalize the size before allocating: parse and validate
process.env.STATUS_LIST_DEFAULT_SIZE (use parseInt/Number and isFinite), ensure
listSize (input) is an integer > 0, fall back to a defined safe default constant
(e.g., DEFAULT_STATUS_LIST_SIZE) if invalid, and optionally clamp or Math.floor
the value to an integer; update the code paths around the variables size,
listSize, and STATUS_LIST_DEFAULT_SIZE and then call new StatusList(new
Array(validSize).fill(0), 1) with the validated value.
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts-97-105 (1)

97-105: ⚠️ Potential issue | 🟠 Major

Use BadRequestError for credential config input failures.

Line 99 and Line 102 throw generic Error for client-provided invalid payloads, which will typically surface as 500 instead of 4xx.

Suggested fix
   private validateCredentialConfig(cred: any, supported: any) {
     if (!supported) {
-      throw new Error(`CredentialSupportedId '${cred.credentialSupportedId}' is not supported by issuer`)
+      throw new BadRequestError(`CredentialSupportedId '${cred.credentialSupportedId}' is not supported by issuer`)
     }
     if (supported.format !== cred.format) {
-      throw new Error(
+      throw new BadRequestError(
         `Format mismatch for '${cred.credentialSupportedId}': expected '${supported.format}', got '${cred.format}'`,
       )
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`
around lines 97 - 105, The validateCredentialConfig function currently throws
generic Error for client input problems (cred and supported checks); change
those throws to BadRequestError so the failures return 4xx, e.g., replace the
two Error throws in validateCredentialConfig with BadRequestError containing the
same messages and add the necessary import for BadRequestError at the top of the
file; keep the existing messages for credentialSupportedId and format mismatch
to preserve context.
src/purge/PurgeTypes.ts-55-70 (1)

55-70: ⚠️ Potential issue | 🟠 Major

Only validate TTL env vars for enabled purge modes.

Line 57 and Line 69 parse TTL values for both modes even when one mode is disabled. A bad disabled-mode env value can fail startup unexpectedly.

Suggested fix
 export function buildPurgeConfig(): PurgeConfig | undefined {
   if (process.env.PURGE_ENABLED !== 'true') return undefined
 
   const natsEnabled = process.env.PURGE_NATS_ENABLED === 'true'
   const cronEnabled = process.env.PURGE_CRON_ENABLED === 'true'
+  const defaultTtlSeconds = 2592000
+  const recordTypes = buildPurgeRecordTypes()
 
   if (!natsEnabled && !cronEnabled) return undefined
 
   return {
     natsConfig: {
       enabled: natsEnabled,
-      ttlSeconds: parseTtlSeconds(process.env.PURGE_NATS_TTL_SECONDS, 'PURGE_NATS_TTL_SECONDS'),
+      ttlSeconds: natsEnabled
+        ? parseTtlSeconds(process.env.PURGE_NATS_TTL_SECONDS, 'PURGE_NATS_TTL_SECONDS', defaultTtlSeconds)
+        : defaultTtlSeconds,
       nats: {
         servers: (process.env.NATS_SERVERS || 'nats://localhost:4222').split(',').map((s) => s.trim()).filter(Boolean),
         nkeySeed: process.env.NATS_NKEY_SEED,
         credentialsFile: process.env.NATS_CREDENTIALS_FILE,
         username: process.env.NATS_USER,
         password: process.env.NATS_PASSWORD,
       },
-      recordTypes: buildPurgeRecordTypes(),
+      recordTypes,
     },
     cronConfig: {
       enabled: cronEnabled,
-      ttlSeconds: parseTtlSeconds(process.env.PURGE_CRON_TTL_SECONDS, 'PURGE_CRON_TTL_SECONDS'),
+      ttlSeconds: cronEnabled
+        ? parseTtlSeconds(process.env.PURGE_CRON_TTL_SECONDS, 'PURGE_CRON_TTL_SECONDS', defaultTtlSeconds)
+        : defaultTtlSeconds,
       cronSchedule: process.env.PURGE_CRON_SCHEDULE || '0 * * * *',
-      recordTypes: buildPurgeRecordTypes(),
+      recordTypes,
     },
     webhookEnabled: process.env.PURGE_WEBHOOK_ENABLED !== 'false',
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/PurgeTypes.ts` around lines 55 - 70, The code is parsing TTL env
vars for both purge modes unconditionally (calls to parseTtlSeconds for
natsConfig and cronConfig) which can cause startup failures when a mode is
disabled; update the construction of natsConfig and cronConfig so that
ttlSeconds is only computed when the corresponding flag (natsEnabled for
natsConfig, cronEnabled for cronConfig) is true (otherwise set ttlSeconds to
undefined or a safe default), i.e., wrap or conditionally call
parseTtlSeconds(process.env.PURGE_NATS_TTL_SECONDS, 'PURGE_NATS_TTL_SECONDS')
and parseTtlSeconds(process.env.PURGE_CRON_TTL_SECONDS,
'PURGE_CRON_TTL_SECONDS') based on natsEnabled/cronEnabled while leaving other
fields like nats, recordTypes (buildPurgeRecordTypes()) and cronSchedule
unchanged.
src/purge/schedulers/NatsPurgeScheduler.ts-51-66 (1)

51-66: ⚠️ Potential issue | 🟠 Major

Replace console.* with structured logger

These lines trigger lint errors (no-console) and bypass tenant-aware structured logs. Please route all logs through the existing logger used in this class.

Also applies to: 93-93, 128-128, 147-147, 177-177, 201-205

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/schedulers/NatsPurgeScheduler.ts` around lines 51 - 66, Replace all
console.* calls in NatsPurgeScheduler with the class's structured logger to fix
linting and ensure tenant-aware logs: use agent.config.logger.info/warn/error
(or this.logger.* if that is the instance logger used in this class) instead of
console.log in the startup sequence around provisionStreams(),
provisionConsumers(), startWorkers(), and any other console usages (including
the occurrences near the provision/consumer/worker calls and the messages that
reference this.recordTypes and this.ttlSeconds). Ensure message text remains the
same and include contextual fields where appropriate (e.g., recordTypes,
ttlSeconds, natsConfig.nats.servers) so logs remain structured and consistent
with existing logging calls like agent.config.logger.info('[Purge] NATS streams
ready').
src/controllers/openid4vc/issuers/issuer.service.ts-29-34 (1)

29-34: ⚠️ Potential issue | 🟠 Major

Fail fast if issuer module is unavailable (avoid silent no-op)

At Line [29], optional chaining can silently skip metadata update and still return a response. This should throw, consistent with createIssuerAgent.

✅ Suggested fail-fast pattern
   public async updateIssuerMetadata(
     agentReq: Req,
     publicIssuerId: string,
     updateIssuerRecordOptions: any, // TODO: Replace with OpenId4VcUpdateIssuerRecordOptions
   ) {
-    await agentReq.agent.modules.openid4vc.issuer?.updateIssuerMetadata({
+    const issuer = agentReq.agent.modules.openid4vc.issuer
+    if (!issuer) {
+      throw new Error('OID4VC issuer module not initialized')
+    }
+    await issuer.updateIssuerMetadata({
       issuerId: publicIssuerId,
       ...updateIssuerRecordOptions,
     })
     return await this.getIssuer(agentReq, publicIssuerId)
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/issuers/issuer.service.ts` around lines 29 - 34,
The current optional chaining on
agentReq.agent.modules.openid4vc.issuer?.updateIssuerMetadata can silently
no-op; instead, fail fast by explicitly checking the issuer module before
calling updateIssuerMetadata (e.g., guard on
agentReq.agent.modules.openid4vc?.issuer) and throw a clear error (matching
createIssuerAgent behavior) if missing, then call
issuer.updateIssuerMetadata(...) and return this.getIssuer(agentReq,
publicIssuerId).
src/utils/helpers.ts-240-244 (1)

240-244: ⚠️ Potential issue | 🟠 Major

Add request timeouts to axios calls.

Both trust-service POST calls can hang indefinitely under network stalls, which can tie up request handlers and degrade availability.

⏱️ Suggested patch
-    tokenResponse = await axios.post<any>(
+    tokenResponse = await axios.post<any>(
       tokenUrl,
       { clientId, clientSecret },
-      { headers: { 'Content-Type': 'application/json', accept: 'application/json' } },
+      {
+        timeout: 10_000,
+        headers: { 'Content-Type': 'application/json', accept: 'application/json' },
+      },
     )

-    const matchResponse = await axios.post<{ matched: boolean }>(
+    const matchResponse = await axios.post<{ matched: boolean }>(
       matchUrl,
       { x509, ...(tenantId && { tenantId }) },
-      { headers: { 'Content-Type': 'application/json', accept: 'application/json', ...authHeaders } },
+      {
+        timeout: 10_000,
+        headers: { 'Content-Type': 'application/json', accept: 'application/json', ...authHeaders },
+      },
     )

Also applies to: 295-299

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` around lines 240 - 244, The axios.post calls that set
tokenResponse (variable tokenResponse) and the second trust-service POST (the
other axios.post around lines 295-299) need a request timeout to avoid hanging;
introduce a configurable timeout constant (e.g., TRUST_SERVICE_TIMEOUT_MS or
REQUEST_TIMEOUT_MS with a sensible default from env) and pass it into the axios
config object (merge with existing headers) for both axios.post calls so the
requests fail fast on network stalls.
src/utils/oid4vc-agent.ts-64-65 (1)

64-65: ⚠️ Potential issue | 🟠 Major

Fix unsafe DID verification-method access.

Line 64/65 can throw when verificationMethod exists but is empty ([0] is undefined).

🐛 Suggested patch
-        if (didDocument.verificationMethod?.[0].id) {
-          issuerDidVerificationMethod = didDocument.verificationMethod?.[0].id
+        const firstVerificationMethodId = didDocument.verificationMethod?.[0]?.id
+        if (firstVerificationMethodId) {
+          issuerDidVerificationMethod = firstVerificationMethodId
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/oid4vc-agent.ts` around lines 64 - 65, The access of
didDocument.verificationMethod?.[0].id in src/utils/oid4vc-agent.ts is unsafe
when verificationMethod is an empty array; change the assignment for
issuerDidVerificationMethod to first verify that didDocument.verificationMethod
has at least one element (e.g., check length or that verificationMethod[0] is
defined) or use safe optional chaining on the element
(verificationMethod?.[0]?.id) before reading .id, ensuring
issuerDidVerificationMethod only receives a value when the first
verificationMethod entry exists.
src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts-95-103 (1)

95-103: ⚠️ Potential issue | 🟠 Major

Fail fast when verifier module is missing in query endpoint.

Line 95 uses optional chaining and can return undefined instead of a clear error, unlike other methods in this service.

✅ Suggested patch
   ) {
-    return await agentReq.agent.modules.openid4vc.verifier?.findVerificationSessionsByQuery({
+    const verifier = agentReq.agent.modules.openid4vc.verifier
+    if (!verifier) {
+      throw new Error('OID4VC verifier module not initialized')
+    }
+    return await verifier.findVerificationSessionsByQuery({
       verifierId: publicVerifierId,
       payloadState,
       state,
       authorizationRequestUri,
       nonce,
       authorizationRequestId,
     })
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/verifier-sessions/verification-sessions.service.ts`
around lines 95 - 103, The current call to
agentReq.agent.modules.openid4vc.verifier?.findVerificationSessionsByQuery uses
optional chaining and may silently return undefined; update the containing
function to explicitly check that agentReq.agent.modules.openid4vc.verifier
exists and throw a clear Error (or return a rejected Promise) if it's missing
before calling findVerificationSessionsByQuery, so the endpoint fails fast and
matches the behavior of the other service methods that assume the verifier
module is present.
src/utils/oid4vc-agent.ts-318-318 (1)

318-318: ⚠️ Potential issue | 🟠 Major

Add timeout handling when fetching trust list URL.

Line 318 uses plain fetch without timeout/abort, so slow upstreams can stall request processing.

⏱️ Suggested patch
-  const response = await fetch(trustListUrl)
+  const controller = new AbortController()
+  const timeout = setTimeout(() => controller.abort(), 10_000)
+  const response = await fetch(trustListUrl, { signal: controller.signal })
+  clearTimeout(timeout)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/oid4vc-agent.ts` at line 318, The fetch(trustListUrl) call must use
an AbortController-based timeout to avoid hanging requests: create an
AbortController, pass controller.signal into fetch(trustListUrl, { signal }),
set a timer (e.g., via setTimeout) to call controller.abort() after a
configurable timeout, and clear the timer when the fetch resolves; catch the
abort error and surface a clear error/timeout log before proceeding. Update the
code around the existing response/response handling (the fetch/trustListUrl
usage and the variable response) to use this pattern so slow upstreams won't
stall processing.
src/utils/oid4vc-agent.ts-100-104 (1)

100-104: ⚠️ Potential issue | 🟠 Major

Replace hard-coded issuer value in PresentationAuthorization flow.

Line 103 uses 'ISSUER_HOST', which can produce invalid issuer metadata in issued credentials.

✅ Suggested patch
-            issuer: {
+            const leafCert = X509Certificate.fromEncodedCertificate(trustedCertificates[0])
+            issuer: {
               method: 'x5c',
               x5c: trustedCertificates.map((cert) => X509Certificate.fromEncodedCertificate(cert)),
-              issuer: 'ISSUER_HOST',
+              issuer: leafCert.sanUriNames?.[0] ?? leafCert.subjectName,
             },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/oid4vc-agent.ts` around lines 100 - 104, The issuer field currently
hard-codes 'ISSUER_HOST' in the issuer object (the PresentationAuthorization
flow where issuer: { method: 'x5c', x5c: trustedCertificates.map(...), issuer:
'ISSUER_HOST' }), which can yield invalid credential metadata; replace that
literal with the real issuer host pulled from configuration or environment
(e.g., an existing issuerHost/config.issuer variable or a getIssuer() helper) so
the issuer value reflects the runtime issuer endpoint instead of the fixed
string.

Comment thread src/purge/PurgeWorker.ts
Comment thread src/utils/helpers.ts Outdated
@tipusinghaw tipusinghaw changed the title merge: temp merge: main sync (OID4VC/P) to main May 4, 2026
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (1)
src/controllers/x509/x509.service.ts (1)

162-166: ⚡ Quick win

Replace console.error with agent logger for consistent observability.

Line 164 bypasses the configured logger pipeline. Use agent.config.logger here as well.

Suggested fix
-        // eslint-disable-next-line no-console
-        console.error('key already exists while importing certificate')
+        agent.config.logger.warn('Key already exists while importing certificate')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/x509.service.ts` around lines 162 - 166, Replace the
direct console.error call inside the Kms.KeyManagementKeyExistsError branch with
the configured agent logger to maintain consistent observability: in the error
handling block that checks Kms.KeyManagementKeyExistsError (in x509.service.ts),
call agent.config.logger (e.g., logger.warn or logger.error) instead of
console.error and include the same descriptive message and ideally the error
details so it mirrors the else branch's use of agent.config.logger.error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cliAgent.ts`:
- Around line 68-82: Reorder the import block so local/project imports are
grouped together and external/third-party imports are grouped separately to
satisfy the import-order linter; specifically move and group the imports for
local symbols (setupServer, buildPurgeConfig, validatePurgeConfig,
initPurgeSchedulers, stopPurgeSchedulers, getNatsPurgeScheduler,
getCronPurgeScheduler, isCustomDocumentLoaderEnabled, CustomDocumentLoader,
generateSecretKey, TsLogger, getMixedCredentialRequestToCredentialMapper,
getX509CertsByClientToken, getX509CertsByUrl, AuthTypes, getAuthType) into a
single contiguous group and place the external module imports
(PolygonDidRegistrar, PolygonDidResolver, PolygonModule from
'@ayanworks/credo-polygon-w3c-module') in their own group according to the
repository's import-order config, then run the linter/validate step to confirm
the errors on the previously failing lines are resolved.
- Around line 221-223: The connections config is forcing autoAcceptConnections
to true by using "autoAcceptConnections || true", which ignores an explicit
false; update the connections object to set autoAcceptConnections directly from
the variable (e.g., autoAcceptConnections) or use a proper nullish coalescing
default (e.g., autoAcceptConnections ?? true) so that false is preserved; change
all occurrences where "autoAcceptConnections || true" appears (including the
blocks that build the connections config around PeerDidNumAlgo.GenesisDoc) to
the direct variable or nullish-coalescing form to restore the ability to disable
DIDComm auto-accept.
- Around line 584-597: Shutdown currently stops purge schedulers then
immediately calls process.exit(0), which can abort HTTP/DIDComm teardown; modify
startup to capture the server instance returned by app.listen (e.g., const
server = app.listen(...)), then update shutdown to: stop accepting new requests
by calling await new Promise(resolve => server.close(resolve)), await the
agent's graceful shutdown method (e.g., agent.shutdown() or agent.stop(),
whichever exists), then call stopPurgeSchedulers(), and only after all
awaitables finish call process.exit(0); also add a short fallback timeout (e.g.,
setTimeout(() => process.exit(1), 10000)) to force exit if teardown hangs and
ensure process.on('SIGTERM'/'SIGINT') still invokes this new shutdown flow.
- Around line 208-212: The ternary for w3cCredentials is inverted: change the
call site that uses isCustomDocumentLoaderEnabled() so that when it returns true
you construct new W3cCredentialsModule with { documentLoader:
CustomDocumentLoader } and when false you call new W3cCredentialsModule()
without options; locate the expression using isCustomDocumentLoaderEnabled(),
W3cCredentialsModule and CustomDocumentLoader and swap the two branches
accordingly.

In `@src/controllers/x509/x509.service.ts`:
- Around line 129-145: ImportX509Certificates currently always calls
pemToRawEd25519PrivateKey with options.privateKey ?? '' and may pass an empty
string or wrong parser for non-Ed25519 keys; update the function to require a
non-empty options.privateKey (throw a clear error if missing) and perform
curve-aware parsing before converting to JWK: inspect options.keyType or use
getTypeFromCurve(keyType) and switch on the curve (e.g., Ed25519 ->
pemToRawEd25519PrivateKey, other curves -> the appropriate PEM-to-raw parser) so
you only call the matching parser and never pass an empty string to
pemToRawEd25519PrivateKey, then feed the resulting raw key into
TypedArrayEncoder.fromHex and transformPrivateKeyToPrivateJwk as before (refer
to ImportX509Certificates, pemToRawEd25519PrivateKey, getTypeFromCurve,
transformPrivateKeyToPrivateJwk, options.privateKey, options.keyType).
- Around line 98-123: The generated/imported subject key (subjectPublicKeyID) is
never passed into createCertificate, causing caller input to be ignored and
leaving orphaned KMS keys; update the createCertificate call in x509.service.ts
to include the subject public key (e.g., subjectPublicKey: subjectPublicKeyID or
subjectPublicKey: { publicJwk: subjectPublicKeyID } matching the agent.x509 API)
and ensure the value comes from the same branch that created/imported it (use
the correct agent.kms vs agentReq.agent.kms consistently: reference
subjectPublicKeyID, importedKey.publicJwk, and the agent.x509.createCertificate
invocation to locate where to add the parameter).
- Around line 29-31: The code currently derives AGENT_DNS by naively stripping
'https://' from createX509Options.issuerAlternativeNameURL (AGENT_HOST) which
fails for URLs with ports/paths; update the derivation to parse the URL and use
its hostname (e.g. const AGENT_DNS = new
URL(createX509Options.issuerAlternativeNameURL).hostname), and add a small
fallback if the constructor throws (prepend a default scheme before parsing) so
the DNS SAN used in X509Service.createCertificate is always a valid hostname.

In `@src/utils/agent.ts`:
- Around line 3-33: The import-order lint fails because PolygonModule is placed
after local imports; move the "PolygonModule" import from
'@ayanworks/credo-polygon-w3c-module' into the external imports block alongside
other third-party imports (near
AnonCredsModule/AskarModule/DidCommModule/IndyVdrModule imports), ensuring there
remains a single blank line separating that external imports block from the
local imports (e.g., TsLogger) to satisfy the import-order rule.
- Around line 149-163: The /invitation handler currently only checks for
req.query.d_m and req.query.c_i and falls back to creating a new invitation; add
handling for req.query.oob so incoming OOB URLs are parsed instead of creating a
fresh invitation. Specifically, in the httpInbound.app.get('/invitation', ...)
handler add an else-if branch for typeof req.query.oob === 'string' and
deserialize the incoming OOB URL (e.g., call
OutOfBandInvitationMessage.fromUrl(req.url) or the project-equivalent OOB
parser) and return its JSON, while keeping the existing
DidCommConnectionInvitationMessage.fromUrl handling for d_m/c_i and the
createInvitation() call for the fallback.

---

Nitpick comments:
In `@src/controllers/x509/x509.service.ts`:
- Around line 162-166: Replace the direct console.error call inside the
Kms.KeyManagementKeyExistsError branch with the configured agent logger to
maintain consistent observability: in the error handling block that checks
Kms.KeyManagementKeyExistsError (in x509.service.ts), call agent.config.logger
(e.g., logger.warn or logger.error) instead of console.error and include the
same descriptive message and ideally the error details so it mirrors the else
branch's use of agent.config.logger.error.
🪄 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: 3f68f89d-5db8-4513-97cc-56084f460f67

📥 Commits

Reviewing files that changed from the base of the PR and between 9655d5d and bd177e7.

📒 Files selected for processing (6)
  • patches/@credo-ts+core+0.6.2+001+fix: change version string type-import esm-export interface.patch
  • patches/@credo-ts+core+0.6.2+002+fix: export defaultDocumentLoader.patch
  • src/cliAgent.ts
  • src/controllers/x509/x509.service.ts
  • src/events/WebSocketEvents.ts
  • src/utils/agent.ts
✅ Files skipped from review due to trivial changes (2)
  • src/events/WebSocketEvents.ts
  • patches/@credo-ts+core+0.6.2+002+fix: export defaultDocumentLoader.patch

Comment thread src/cliAgent.ts Outdated
Comment thread src/cliAgent.ts Outdated
Comment thread src/cliAgent.ts
Comment thread src/cliAgent.ts Outdated
Comment on lines +584 to +597
app.listen(adminPort, () => {
logger.info(`Successfully started server on port ${adminPort}`)
})
}


// Graceful shutdown
const shutdown = async () => {
agent.config.logger.info('[Shutdown] Stopping services...')
await stopPurgeSchedulers()
process.exit(0)
}

process.on('SIGTERM', shutdown)
process.on('SIGINT', shutdown)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Close the server and agent before forcing process exit.

The new shutdown handler stops purge schedulers and then calls process.exit(0) immediately. That bypasses HTTP/DIDComm teardown and can drop in-flight work or leave wallet/NATS resources dirty on SIGTERM.

Suggested fix
-  app.listen(adminPort, () => {
+  const server = app.listen(adminPort, () => {
     logger.info(`Successfully started server on port ${adminPort}`)
   })

   // Graceful shutdown
   const shutdown = async () => {
     agent.config.logger.info('[Shutdown] Stopping services...')
+    await new Promise<void>((resolve, reject) => {
+      server.close((error) => (error ? reject(error) : resolve()))
+    })
     await stopPurgeSchedulers()
+    await agent.shutdown()
     process.exit(0)
   }
🧰 Tools
🪛 ESLint

[error] 588-589: Delete ⏎··

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 584 - 597, Shutdown currently stops purge
schedulers then immediately calls process.exit(0), which can abort HTTP/DIDComm
teardown; modify startup to capture the server instance returned by app.listen
(e.g., const server = app.listen(...)), then update shutdown to: stop accepting
new requests by calling await new Promise(resolve => server.close(resolve)),
await the agent's graceful shutdown method (e.g., agent.shutdown() or
agent.stop(), whichever exists), then call stopPurgeSchedulers(), and only after
all awaitables finish call process.exit(0); also add a short fallback timeout
(e.g., setTimeout(() => process.exit(1), 10000)) to force exit if teardown hangs
and ensure process.on('SIGTERM'/'SIGINT') still invokes this new shutdown flow.

Comment thread src/controllers/x509/x509.service.ts
Comment thread src/controllers/x509/x509.service.ts
Comment thread src/controllers/x509/x509.service.ts
Comment thread src/utils/agent.ts Outdated
Comment thread src/utils/agent.ts
@tipusinghaw tipusinghaw self-assigned this May 4, 2026
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (2)
src/utils/agent.ts (1)

3-34: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Import order lint error persists.

The PolygonModule import on line 34 occurs after the local import (TsLogger), violating ESLint's import/order rule. The commented-out import on line 3 should be removed, and the active import should be moved to the external imports block.

🔧 Proposed fix
-// import { PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
 import {
   AnonCredsModule,
   LegacyIndyDidCommProofFormatService,
   DidCommCredentialV1Protocol,
   DidCommProofV1Protocol,
   AnonCredsDidCommProofFormatService,
   LegacyIndyDidCommCredentialFormatService,
   AnonCredsDidCommCredentialFormatService,
 } from '@credo-ts/anoncreds'
 import { AskarModule } from '@credo-ts/askar'
 import { DidsModule, KeyDidRegistrar, KeyDidResolver, WebDidResolver, Agent, LogLevel } from '@credo-ts/core'
 import {
   DidCommHttpOutboundTransport,
   DidCommJsonLdCredentialFormatService,
   DidCommDifPresentationExchangeProofFormatService,
   DidCommAutoAcceptCredential,
   DidCommProofV2Protocol,
   DidCommCredentialV2Protocol,
   DidCommModule,
   DidCommConnectionInvitationMessage,
   parseInvitationUrl,
 } from '@credo-ts/didcomm'
 import { IndyVdrAnonCredsRegistry, IndyVdrModule } from '@credo-ts/indy-vdr'
 import { agentDependencies, DidCommHttpInboundTransport } from '@credo-ts/node'
 import { TenantsModule } from '@credo-ts/tenants'
 import { anoncreds } from '@hyperledger/anoncreds-nodejs'
 import { indyVdr } from '@hyperledger/indy-vdr-nodejs'
 import { askar } from '@openwallet-foundation/askar-nodejs'
+import { PolygonModule } from '@ayanworks/credo-polygon-w3c-module'

 import { TsLogger } from './logger'
-import { PolygonModule } from '@ayanworks/credo-polygon-w3c-module'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/agent.ts` around lines 3 - 34, Remove the commented-out
PolygonModule import at the top and relocate the active "PolygonModule" import
so it sits with the other external/package imports (the block importing
`@credo-ts/`*, `@hyperledger/`*, `@openwallet-foundation/`*, etc.), ensuring it
appears before the local import "TsLogger"; this fixes the import/order lint
error by keeping external module imports (including PolygonModule) grouped above
local file imports.
src/cliAgent.ts (1)

588-594: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

server.close() is not awaited, risking dropped in-flight requests.

server.close() is callback-based and returns before connections are fully closed. The current code proceeds to stop schedulers and agent while HTTP requests may still be in flight.

🔧 Suggested fix to await server closure
   const shutdown = async () => {
     agent.config.logger.info('[Shutdown] Stopping services...')
-    server.close()
+    await new Promise<void>((resolve, reject) => {
+      server.close((err) => (err ? reject(err) : resolve()))
+    })
     await stopPurgeSchedulers()
     await agent.shutdown()
     process.exit(0)
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 588 - 594, The shutdown sequence in shutdown
currently calls server.close() without awaiting its completion, which can drop
in-flight requests; modify shutdown to await server closure by promisifying the
server.close callback (or using a close/promise helper) and awaiting that
promise before calling stopPurgeSchedulers() and agent.shutdown(); ensure you
handle/propagate close errors (log via agent.config.logger.error) and still
proceed to process.exit(0) after successful/timeout closure to avoid hanging.
🧹 Nitpick comments (9)
src/utils/helpers.ts (5)

350-350: 💤 Low value

Fix line formatting.

This line exceeds the expected line length per prettier configuration.

-  console.log(`[${label}] agent type: ${isDedicated ? 'dedicated' : 'shared'}, certificate count: ${x509Certificates.length}`)
+  console.log(
+    `[${label}] agent type: ${isDedicated ? 'dedicated' : 'shared'}, certificate count: ${x509Certificates.length}`,
+  )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` at line 350, The console.log statement with template
interpolation is too long; split it into shorter expressions to satisfy
line-length rules by building a short message variable and logging that, e.g.,
compute a concise status string using the existing symbols label, isDedicated
and x509Certificates (or x509Certificates.length) and then call
console.log(message) inside the same module (helpers.ts) so the output remains
identical but each line stays within prettier's max line length.

55-70: 💤 Low value

Simplify boolean expressions and use Number.parseInt.

The ternary expressions === 'true' ? true : false are redundant since the comparison already returns a boolean.

 export function getCertificateValidityForSystem(IsRootCA = false) {
   let options: { validityYears?: number; startFromCurrentMonth?: boolean }
   if (IsRootCA) {
     options = {
-      validityYears: parseInt(process.env.ROOT_CA_VALIDITY_YEARS ?? '3'),
-      startFromCurrentMonth: (process.env.ROOT_CA_START_FROM_CURRENT_MONTH ?? 'true') === 'true' ? true : false,
+      validityYears: Number.parseInt(process.env.ROOT_CA_VALIDITY_YEARS ?? '3', 10),
+      startFromCurrentMonth: (process.env.ROOT_CA_START_FROM_CURRENT_MONTH ?? 'true') === 'true',
     }
   } else {
     options = {
-      validityYears: parseInt(process.env.DCS_VALIDITY_YEARS ?? '3'),
-      startFromCurrentMonth: (process.env.DCS_START_FROM_CURRENT_MONTH ?? 'true') === 'true' ? true : false,
+      validityYears: Number.parseInt(process.env.DCS_VALIDITY_YEARS ?? '3', 10),
+      startFromCurrentMonth: (process.env.DCS_START_FROM_CURRENT_MONTH ?? 'true') === 'true',
     }
   }
 
   return getCertificateValidity(options)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` around lines 55 - 70, In
getCertificateValidityForSystem, simplify the boolean expressions and use
Number.parseInt: replace the redundant ternaries that convert (process.env.X ??
'true') === 'true' ? true : false with the direct comparison (process.env.X ??
'true') === 'true', and use Number.parseInt(...) instead of parseInt(...) for
validityYears; keep the same option property names (validityYears,
startFromCurrentMonth) and still pass options into getCertificateValidity.

4-4: 💤 Low value

Use type-only imports for DidDocument and VerificationMethod.

These are only used as type annotations, not as runtime values.

-import { JsonEncoder, JsonTransformer, DidDocument, VerificationMethod } from '@credo-ts/core'
+import { JsonEncoder, JsonTransformer } from '@credo-ts/core'
+import type { DidDocument, VerificationMethod } from '@credo-ts/core'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` at line 4, The import line pulls in DidDocument and
VerificationMethod as runtime imports but they are only used for typing; change
the import from '@credo-ts/core' to use TypeScript type-only imports for
DidDocument and VerificationMethod (e.g., prefix with the `type` keyword) so
only JsonEncoder and JsonTransformer remain as runtime imports; update the
import statement that currently references JsonEncoder, JsonTransformer,
DidDocument, VerificationMethod accordingly to avoid emitting unused runtime
code.

9-10: In-memory token cache won't persist across instances.

The module-level tokenCache Map is local to each process instance. In horizontally-scaled deployments, each instance will independently fetch and cache tokens, leading to redundant token requests. Consider using Redis or another shared cache if the service scales horizontally and token fetch rate becomes a concern.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` around lines 9 - 10, The module-level tokenCache Map
(tokenCache) with TOKEN_EXPIRY_BUFFER_SECONDS is an in-memory cache and will not
be shared across process instances; replace it with a shared cache
implementation (e.g., Redis) or make the cache pluggable so horizontally scaled
services reuse tokens. Update the helper that reads/writes tokenCache to use a
Redis-backed get/set/ttl (or an injected Cache interface) and preserve the same
token shape ({ token, expiresAt }) and expiry-buffer logic
(TOKEN_EXPIRY_BUFFER_SECONDS) so existing token-fetching logic (where tokenCache
is referenced) continues to work; add configuration to choose in-memory vs Redis
and ensure async calls handle Redis latency/failures gracefully.

110-115: 💤 Low value

Silent default to P-256 may mask configuration errors.

When normalizedCurve is undefined (unrecognized input), the function silently defaults to EC P-256. Consider logging a warning or throwing an error for unrecognized curves to help with debugging.

   } else {
+    console.warn(`[getTypeFromCurve] Unrecognized curve/algorithm: ${key}, defaulting to EC P-256`)
     keyTypeInfo = {
       kty: 'EC',
       crv: 'P-256',
     }
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` around lines 110 - 115, The code silently defaults
unrecognized normalizedCurve to an EC P-256 key by setting keyTypeInfo = { kty:
'EC', crv: 'P-256' }, which can hide configuration errors; change this behavior
to either (a) log a warning including the unrecognized normalizedCurve value
(use the module's logger or console.warn) before setting the default, or (b)
throw a descriptive Error mentioning the invalid curve and listing accepted
values, so callers know the input was invalid—modify the branch handling
undefined normalizedCurve to implement one of these behaviors and keep the
existing keyTypeInfo assignment only after emitting the warning or performing
validation.
src/controllers/x509/crypto-util.ts (2)

8-20: 💤 Low value

Dead code: pemToRawEcPrivateKey is defined but never exported or used.

This function is not exported and appears unused. If it's intended for future use, consider removing it until needed to reduce maintenance burden.

Additionally, line 17 uses a non-null assertion (!) on jwk.d without validation, which would throw an unclear error if d is missing (unlike the Ed25519 function which has explicit validation at line 37).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/crypto-util.ts` around lines 8 - 20, The function
pemToRawEcPrivateKey is dead code and also uses a non-null assertion on jwk.d
which can throw unclear errors; either remove pemToRawEcPrivateKey if not needed
or export and use it, and if you keep it add explicit validation for the
exported JWK's d property (similar to the Ed25519 helper) before converting to
hex — check that keyObj.export(...).d exists and throw a clear error if missing,
then Buffer.from(d, 'base64').toString('hex') to return the raw key.

22-40: 💤 Low value

Naming mismatch: function processes DER but is named pemToRaw....

The function name and docstring say "PEM-encoded" but the parameter is named derKey, and the implementation processes DER format (base64 string or raw buffer with format: 'der'). This inconsistency could confuse future maintainers.

Consider renaming to derToRawEd25519PrivateKey or updating the docstring to reflect DER input.

Also, the async keyword is unnecessary since there are no await operations—createPrivateKey and export are synchronous.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/crypto-util.ts` around lines 22 - 40, The function
pemToRawEd25519PrivateKey is misnamed and marked async while it actually accepts
DER input (parameter derKey), uses createPrivateKey with format:'der' and
synchronously calls keyObj.export(); rename the function to
derToRawEd25519PrivateKey (or update the docstring to say DER) and remove the
unnecessary async keyword and Promise return type, keeping the same logic that
reads derKey, creates the private key, exports the JWK (jwk.d) and returns the
hex seed.
src/controllers/x509/x509.service.ts (1)

24-24: 💤 Low value

Class name should use PascalCase.

x509Service starts with lowercase, violating TypeScript naming conventions. This is also flagged by static analysis.

🔧 Suggested fix
-class x509Service {
+class X509Service {

And update the export:

-export const x509ServiceT = new x509Service()
+export const x509ServiceT = new X509Service()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/x509.service.ts` at line 24, The class name x509Service
uses camelCase; rename it to PascalCase X509Service and update its declaration
and any export (e.g., export default X509Service or export { X509Service }) as
well as all internal references/usages/imports across the codebase
(constructors, new x509Service(), type annotations, and import statements) to
use X509Service so TypeScript naming conventions and static analysis warnings
are resolved.
src/purge/PurgeWorker.ts (1)

27-27: ⚡ Quick win

console.* calls throughout the file violate no-console (ESLint errors) and duplicate the structured logger.

Every console.log/warn/error at lines 27, 32, 36, 66, 78, 91, 105, 109, and 113 is immediately paired with an agent.config.logger call that captures the same event and context. Removing the console.* calls eliminates all ESLint errors on these lines and leaves structured logging in place.

Also applies to: 32-32, 36-36, 66-66, 78-78, 91-91, 105-105, 109-109, 113-113

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/purge/PurgeWorker.ts` at line 27, Remove the direct console.* calls in
the PurgeWorker class and rely only on the structured logger
(agent.config.logger) that already logs the same events and context;
specifically, delete each console.log/warn/error call that duplicates messages
at the start of start/stop flows and error handlers (the calls paired with
agent.config.logger in methods surrounding the initialization log, record
processing, and error handling), ensure the existing agent.config.logger
invocations remain unchanged and include the same message/context, and run
ESLint to confirm no remaining no-console violations; target the console.*
occurrences in the PurgeWorker class (the standalone console calls shown
alongside the structured logger usages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cliAgent.ts`:
- Around line 463-477: The call uses an unnecessary await on
getWithTenantModules (which returns a plain object) so remove the await and
assign synchronously: change modules = await getWithTenantModules(...) to
modules = getWithTenantModules(...); also confirm getWithTenantModules is not
declared async (or if it must be async, change its implementation to return a
Promise) so the call-site and the function signature are consistent.

In `@src/controllers/x509/x509.service.ts`:
- Around line 160-171: The catch block in x509.service.ts currently logs the
entire error via JSON.stringify(error), which can leak sensitive key/certificate
material; replace this with a sanitized log that only records non-sensitive
metadata (e.g., error.name and error.message) and a clear context string, avoid
logging error.stack or full properties, and do not serialize the entire error
object; also replace the direct console.error in the
Kms.KeyManagementKeyExistsError branch with the structured logger
(agent.config.logger.warn or similar) to keep logging consistent and avoid
accidental sensitive output.

In `@src/purge/PurgeWorker.ts`:
- Around line 30-38: Wrap the call to consumer.consume() and the subsequent
for-await loop in a try/catch inside the existing while(true) (the start() loop
in PurgeWorker.ts) so any rejection or thrown exception from consumer.consume()
or from this.processMessage(msg, agent) is caught; on catch, log the error via
agent.config.logger.error (include this.consumerName), close or reset the
consumer if applicable, and await a backoff delay (exponential jitter) before
retrying to avoid busy-spin; ensure the for-await iterator is fully drained or
aborted on error so the loop can restart cleanly.

In `@src/utils/agent.ts`:
- Around line 150-168: Wrap the async '/invitation' route handler logic in a
try/catch and return proper HTTP errors on failure: catch errors thrown by
DidCommConnectionInvitationMessage.fromUrl, parseInvitationUrl, or
agent.modules.didcomm.oob.createInvitation and call res.status(500).send({
error: err.message || 'Internal Server Error' }) (or another appropriate status)
so the response is not left hanging; keep existing branches that call
DidCommConnectionInvitationMessage.fromUrl, parseInvitationUrl, and
agent.modules.didcomm.oob.createInvitation but move them inside the try block
and handle the caught error in the catch block with logging and an error
response.

---

Duplicate comments:
In `@src/cliAgent.ts`:
- Around line 588-594: The shutdown sequence in shutdown currently calls
server.close() without awaiting its completion, which can drop in-flight
requests; modify shutdown to await server closure by promisifying the
server.close callback (or using a close/promise helper) and awaiting that
promise before calling stopPurgeSchedulers() and agent.shutdown(); ensure you
handle/propagate close errors (log via agent.config.logger.error) and still
proceed to process.exit(0) after successful/timeout closure to avoid hanging.

In `@src/utils/agent.ts`:
- Around line 3-34: Remove the commented-out PolygonModule import at the top and
relocate the active "PolygonModule" import so it sits with the other
external/package imports (the block importing `@credo-ts/`*, `@hyperledger/`*,
`@openwallet-foundation/`*, etc.), ensuring it appears before the local import
"TsLogger"; this fixes the import/order lint error by keeping external module
imports (including PolygonModule) grouped above local file imports.

---

Nitpick comments:
In `@src/controllers/x509/crypto-util.ts`:
- Around line 8-20: The function pemToRawEcPrivateKey is dead code and also uses
a non-null assertion on jwk.d which can throw unclear errors; either remove
pemToRawEcPrivateKey if not needed or export and use it, and if you keep it add
explicit validation for the exported JWK's d property (similar to the Ed25519
helper) before converting to hex — check that keyObj.export(...).d exists and
throw a clear error if missing, then Buffer.from(d, 'base64').toString('hex') to
return the raw key.
- Around line 22-40: The function pemToRawEd25519PrivateKey is misnamed and
marked async while it actually accepts DER input (parameter derKey), uses
createPrivateKey with format:'der' and synchronously calls keyObj.export();
rename the function to derToRawEd25519PrivateKey (or update the docstring to say
DER) and remove the unnecessary async keyword and Promise return type, keeping
the same logic that reads derKey, creates the private key, exports the JWK
(jwk.d) and returns the hex seed.

In `@src/controllers/x509/x509.service.ts`:
- Line 24: The class name x509Service uses camelCase; rename it to PascalCase
X509Service and update its declaration and any export (e.g., export default
X509Service or export { X509Service }) as well as all internal
references/usages/imports across the codebase (constructors, new x509Service(),
type annotations, and import statements) to use X509Service so TypeScript naming
conventions and static analysis warnings are resolved.

In `@src/purge/PurgeWorker.ts`:
- Line 27: Remove the direct console.* calls in the PurgeWorker class and rely
only on the structured logger (agent.config.logger) that already logs the same
events and context; specifically, delete each console.log/warn/error call that
duplicates messages at the start of start/stop flows and error handlers (the
calls paired with agent.config.logger in methods surrounding the initialization
log, record processing, and error handling), ensure the existing
agent.config.logger invocations remain unchanged and include the same
message/context, and run ESLint to confirm no remaining no-console violations;
target the console.* occurrences in the PurgeWorker class (the standalone
console calls shown alongside the structured logger usages).

In `@src/utils/helpers.ts`:
- Line 350: The console.log statement with template interpolation is too long;
split it into shorter expressions to satisfy line-length rules by building a
short message variable and logging that, e.g., compute a concise status string
using the existing symbols label, isDedicated and x509Certificates (or
x509Certificates.length) and then call console.log(message) inside the same
module (helpers.ts) so the output remains identical but each line stays within
prettier's max line length.
- Around line 55-70: In getCertificateValidityForSystem, simplify the boolean
expressions and use Number.parseInt: replace the redundant ternaries that
convert (process.env.X ?? 'true') === 'true' ? true : false with the direct
comparison (process.env.X ?? 'true') === 'true', and use Number.parseInt(...)
instead of parseInt(...) for validityYears; keep the same option property names
(validityYears, startFromCurrentMonth) and still pass options into
getCertificateValidity.
- Line 4: The import line pulls in DidDocument and VerificationMethod as runtime
imports but they are only used for typing; change the import from
'@credo-ts/core' to use TypeScript type-only imports for DidDocument and
VerificationMethod (e.g., prefix with the `type` keyword) so only JsonEncoder
and JsonTransformer remain as runtime imports; update the import statement that
currently references JsonEncoder, JsonTransformer, DidDocument,
VerificationMethod accordingly to avoid emitting unused runtime code.
- Around line 9-10: The module-level tokenCache Map (tokenCache) with
TOKEN_EXPIRY_BUFFER_SECONDS is an in-memory cache and will not be shared across
process instances; replace it with a shared cache implementation (e.g., Redis)
or make the cache pluggable so horizontally scaled services reuse tokens. Update
the helper that reads/writes tokenCache to use a Redis-backed get/set/ttl (or an
injected Cache interface) and preserve the same token shape ({ token, expiresAt
}) and expiry-buffer logic (TOKEN_EXPIRY_BUFFER_SECONDS) so existing
token-fetching logic (where tokenCache is referenced) continues to work; add
configuration to choose in-memory vs Redis and ensure async calls handle Redis
latency/failures gracefully.
- Around line 110-115: The code silently defaults unrecognized normalizedCurve
to an EC P-256 key by setting keyTypeInfo = { kty: 'EC', crv: 'P-256' }, which
can hide configuration errors; change this behavior to either (a) log a warning
including the unrecognized normalizedCurve value (use the module's logger or
console.warn) before setting the default, or (b) throw a descriptive Error
mentioning the invalid curve and listing accepted values, so callers know the
input was invalid—modify the branch handling undefined normalizedCurve to
implement one of these behaviors and keep the existing keyTypeInfo assignment
only after emitting the warning or performing validation.
🪄 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: d481296c-c5d0-41df-84e8-2d2c73f9c1b1

📥 Commits

Reviewing files that changed from the base of the PR and between bd177e7 and 0ff8131.

📒 Files selected for processing (6)
  • src/cliAgent.ts
  • src/controllers/x509/crypto-util.ts
  • src/controllers/x509/x509.service.ts
  • src/purge/PurgeWorker.ts
  • src/utils/agent.ts
  • src/utils/helpers.ts

Comment thread src/cliAgent.ts
Comment thread src/controllers/x509/x509.service.ts
Comment thread src/purge/PurgeWorker.ts Outdated
Comment thread src/utils/agent.ts
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
src/utils/helpers.ts (1)

235-252: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove sensitive trust-flow logging and response-body echoing.

On Line 250/312 and Line 254/316, response payloads are logged/embedded into thrown errors, which can leak secrets or sensitive trust-service data. Also, console.* usage in these paths (Line 235 onward) bypasses structured redaction controls.

🔐 Suggested hardening patch
-  console.log(`[${label}] fetching token from:`, tokenUrl)
+  logger.debug(`[${label}] fetching token`)

-      console.error(`[${label}] token request failed:`, {
+      logger.error(`[${label}] token request failed`, {
         url: tokenUrl,
         status: error.response?.status,
         statusText: error.response?.statusText,
-        data: error.response?.data,
         message: error.message,
       })
       throw new Error(
-        `[${label}] platform token request failed with status ${error.response?.status ?? 'no response'}: ${JSON.stringify(error.response?.data ?? error.message)}`,
+        `[${label}] platform token request failed with status ${error.response?.status ?? 'no response'}`,
       )

-  console.log(
-    `[${label}] token cached for clientId:`,
-    clientId,
-    '| expires at:',
-    new Date(expiresAt * 1000).toISOString(),
-  )
+  logger.debug(`[${label}] token cached`, { expiresAt })

-  console.log(`[${label}] calling match API:`, matchUrl)
+  logger.debug(`[${label}] calling match API`)

-    console.log(`[${label}] match response status:`, matchResponse.status)
-    console.log(`[${label}] isTrusted:`, isTrusted)
+    logger.debug(`[${label}] match response`, { status: matchResponse.status, isTrusted })

-      console.error(`[${label}] match request failed:`, {
+      logger.error(`[${label}] match request failed`, {
         url: matchUrl,
         status: error.response?.status,
         statusText: error.response?.statusText,
-        data: error.response?.data,
         message: error.message,
       })
       throw new Error(
-        `[${label}] trust-service match request failed with status ${error.response?.status ?? 'no response'}: ${JSON.stringify(error.response?.data ?? error.message)}`,
+        `[${label}] trust-service match request failed with status ${error.response?.status ?? 'no response'}`,
       )

-  console.log(
-    `[${label}] agent type: ${isDedicated ? 'dedicated' : 'shared'}, certificate count: ${x509Certificates.length}`,
-  )
+  logger.info(`[${label}] trust check started`, { isDedicated, certificateCount: x509Certificates.length })

Also applies to: 267-272, 285-317, 350-352

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/helpers.ts` around lines 235 - 252, The code is logging/throwing
full HTTP response bodies and using console.* in the token fetch/error paths
which can leak secrets; in the axios.post token flow (tokenUrl, tokenResponse)
and any axios.isAxiosError branches replace console.log/console.error with the
structured logger used elsewhere (e.g., processLogger or injected logger),
remove/error out any inclusion of error.response.data or tokenResponse.data
(only log non-sensitive fields like status, statusText, and
error.message/error.code), and stop embedding raw response payloads into thrown
Error objects—throw errors with a short message and safe metadata
(status/statusText) or ensure sensitive fields (e.g., clientSecret,
access_token) are redacted before any logging or exception text; apply the same
changes to the other equivalent blocks referenced (the axios error handlers and
any places that echo response bodies).
src/controllers/x509/x509.service.ts (1)

130-148: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Import flow only supports Ed25519 keys.

The ImportX509Certificates method calls pemToRawEd25519PrivateKey regardless of options.keyType. This will fail silently or produce incorrect results for non-Ed25519 keys (e.g., P-256, secp256k1).

This was previously flagged but appears not fully addressed. The method should perform curve-aware parsing based on options.keyType before converting to JWK.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/x509.service.ts` around lines 130 - 148,
ImportX509Certificates currently unconditionally calls pemToRawEd25519PrivateKey
which only works for Ed25519; update the method to branch on options.keyType
(use the same symbol names: ImportX509Certificates, options.keyType,
pemToRawEd25519PrivateKey, transformPrivateKeyToPrivateJwk, getTypeFromCurve)
and perform curve-aware parsing: if keyType is Ed25519 call
pemToRawEd25519PrivateKey, for P-256/secp256k1 call the appropriate PEM-to-raw
parser (or add a generic pemToRawPrivateKey that accepts the curve) to produce
the raw private key bytes, throw a clear error for unsupported curves, then
continue converting the resulting raw key into a JWK via
transformPrivateKeyToPrivateJwk using getTypeFromCurve. Ensure the rest of the
flow (privateKey variable used below) uses the curve-specific raw bytes.
🧹 Nitpick comments (11)
tsconfig.build.json (2)

4-17: ⚡ Quick win

target: "ES2017" / lib: ["ESNext"] mismatch silences warnings for unpolyfilled runtime APIs

TypeScript's target only downlevels syntax; it does not polyfill runtime APIs. With lib: ["ESNext"], the compiler accepts calls to APIs such as Array.prototype.at(), Object.hasOwn(), structuredClone(), and Error.cause without error, but those calls will throw at runtime on any Node.js version that doesn't ship them natively. The type-checker will never surface this gap.

The official TypeScript Node Target Mapping recommends keeping lib and target at the same ES level for a given Node version — e.g., target: "ES2022" + lib: ["ES2022"] for Node 18+. Setting lib higher than target is intentional only when polyfills (e.g., core-js) are relied upon to provide those APIs at runtime.

Consider aligning target with the actual minimum supported Node.js version:

⚙️ Proposed alignment for Node 18 LTS
-    "target": "ES2017",
+    "target": "ES2022",
     ...
-    "lib": ["ESNext"]
+    "lib": ["ES2022"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tsconfig.build.json` around lines 4 - 17, The tsconfig currently sets
"target" to "ES2017" while "lib" is "ESNext", which lets TypeScript accept newer
runtime APIs without ensuring they're available at runtime; change the tsconfig
settings so "target" and "lib" align to the same ES level that matches our
minimum supported Node version (for example set both "target" and "lib" to
"ES2022" for Node 18+), or lower them if we must support older Node releases;
only keep lib > target if you intentionally add runtime polyfills (e.g.,
core-js), and update any CI/test matrix or docs to reflect the chosen minimum
Node version.

4-17: target: "ES2017" / lib: ["ESNext"] mismatch creates type-safety inconsistency

TypeScript's target downlevels syntax but does not polyfill runtime APIs. With lib: ["ESNext"], the compiler accepts calls to newer APIs (e.g., Array.prototype.at(), Object.hasOwn(), structuredClone()) without error. While the Dockerfile targets Node 22, which supports all ESNext features, the type-safe surface should still align with the compilation target for consistency and maintainability.

Align target with Node 22's capability by setting it to "ES2022" (the baseline for Node 18+), making the type-checker and emitted code consistent:

⚙️ Proposed alignment
-    "target": "ES2017",
+    "target": "ES2022",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tsconfig.build.json` around lines 4 - 17, The tsconfig has a mismatch between
"target": "ES2017" and "lib": ["ESNext"], causing the compiler to allow newer
runtime APIs while emitting older syntax; update the TypeScript config by
changing the "target" value from ES2017 to "ES2022" (or otherwise align "lib" to
the same ECMAScript level) so "target" and "lib" are consistent with Node 22's
capabilities; ensure the "target" and "lib" fields in tsconfig.build.json are
updated together to avoid the type/runtime inconsistency.
src/controllers/x509/x509.service.ts (1)

24-24: ⚡ Quick win

Rename class to follow PascalCase convention.

The class name x509Service should be X509Service to follow JavaScript/TypeScript naming conventions. This is also flagged by SonarCloud.

🔧 Proposed fix
-class x509Service {
+class X509Service {
   public async createSelfSignedDCS(createX509Options: BasicX509CreateCertificateConfig, agentReq: Req) {
...
-export const x509ServiceT = new x509Service()
+export const x509ServiceT = new X509Service()

Also applies to: 216-216

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/x509/x509.service.ts` at line 24, Rename the class identifier
x509Service to PascalCase X509Service everywhere it appears (class declaration
and any references/imports/usages) to follow TypeScript naming conventions and
satisfy SonarCloud; update the class declaration named x509Service to
X509Service and adjust all references (constructors, exports, imports, type
annotations, and any instantiation sites) to use X509Service so symbols remain
consistent.
src/utils/agent.ts (1)

50-50: ⚡ Quick win

Simplify redundant boolean expression.

The ternary is unnecessary since the comparison already returns a boolean.

🔧 Proposed fix
-    allowInsecureHttpUrls: process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() === 'true' ? true : false,
+    allowInsecureHttpUrls: process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() === 'true',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/agent.ts` at line 50, The property allowInsecureHttpUrls is using a
redundant ternary; replace the expression
"process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() === 'true' ? true : false"
with the direct boolean comparison
"process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() === 'true'" where
allowInsecureHttpUrls is defined in the agent configuration so the value is a
boolean without the unnecessary ? true : false.
src/cli.ts (1)

3-9: ⚡ Quick win

Fix import ordering to pass Validate check.

The dotenv import must occur before yargs to satisfy import ordering rules.

🔧 Proposed fix
+import dotenv from 'dotenv'
 import yargs from 'yargs'
 import { hideBin } from 'yargs/helpers'
-import dotenv from 'dotenv'

 dotenv.config()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli.ts` around lines 3 - 9, The import ordering is wrong: load
environment variables before importing modules that may depend on them; move the
dotenv import and dotenv.config() so they occur before importing yargs (i.e.,
ensure dotenv is imported and dotenv.config() runs prior to the yargs import
line), keeping the rest of imports (like runRestAgent from './cliAgent.js')
unchanged to satisfy the Validate check.
src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts (2)

22-22: 💤 Low value

Consider stronger typing for credential and status list structures.

Multiple any types reduce type safety and IDE assistance. The cred, supported, and offerStatusInfo types could leverage existing interfaces from issuer.types.ts.

Also applies to: 97-97, 126-131

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts` at
line 22, The declaration const offerStatusInfo: any[] = [] and other any usages
(e.g., variables named cred and supported around the processing logic) weaken
type safety—replace these anys with concrete interfaces from issuer.types.ts
(for example use IssuerCredential, CredentialSupportedFormat, OfferStatusInfo or
the appropriately named exported types) and update function/method signatures in
issuance-sessions.service.ts to use those types; adjust any array/field shapes
(e.g., offerStatusInfo: OfferStatusInfo[]), cred: IssuerCredential, supported:
CredentialSupportedFormat[] (or the exact exported names) and import the types
from issuer.types.ts so the compiler and IDE can validate structure and provide
autocomplete.

6-11: ⚡ Quick win

Fix import ordering to pass ESLint validation.

The static analysis correctly flags import order issues that will fail the Validate check.

🔧 Proposed fix
 import { CredentialFormat, SignerMethod } from '../../../enums/enum'
 import { BadRequestError, NotFoundError } from '../../../errors/errors'
-
-import { checkAndCreateStatusList, getServerUrl, revokeCredentialInStatusList } from '../../../utils/statusListService'
 import { STATUS_LISTS_PATH } from '../../../utils/constant'
+import { checkAndCreateStatusList, getServerUrl, revokeCredentialInStatusList } from '../../../utils/statusListService'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`
around lines 6 - 11, Reorder the import block to match the project's ESLint
import-order rules: group and sort internal imports so that enum/constant
imports (CredentialFormat, SignerMethod, STATUS_LISTS_PATH) are together, error
imports (BadRequestError, NotFoundError) follow, and utility imports
(checkAndCreateStatusList, getServerUrl, revokeCredentialInStatusList) come
after; update the import statements in issuance-sessions.service.ts to reflect
that grouping and alphabetical ordering within each group so ESLint passes.
src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts (1)

6-15: ⚡ Quick win

Fix import ordering to pass ESLint validation.

🔧 Proposed fix
 import { SCOPES } from '../../../enums'
-// eslint-disable-next-line import/order
 import ErrorHandlingService from '../../../errorHandlingService'
-import { SchedulePurge } from '../../../purge/decorators/SchedulePurge'
 import { PurgeRecordType } from '../../../purge/PurgeTypes'
-
-// import { AgentWithRootOrTenant } from '../../types/agent'
+import { SchedulePurge } from '../../../purge/decorators/SchedulePurge'
 import { OpenId4VcIssuanceSessionsCreateOffer } from '../types/issuer.types'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts`
around lines 6 - 15, Remove the inline ESLint disable and reorder the imports in
issuance-sessions.Controller.ts to satisfy import/order: delete the "//
eslint-disable-next-line import/order" line and group imports by type (external
libs first, then internal modules), sorting each group alphabetically; ensure
SCOPES, ErrorHandlingService, SchedulePurge, PurgeRecordType,
OpenId4VcIssuanceSessionsCreateOffer and issuanceSessionService remain imported
but reordered into proper groups so ESLint import/order no longer fails.
src/controllers/types.ts (1)

423-436: 💤 Low value

FIXME comments indicate unresolved type safety concerns.

Multiple keyType fields are typed as any with FIXME comments. These should be tracked and resolved to restore type safety, potentially using KeyAlgorithm from @openwallet-foundation/askar-nodejs consistently.

Also applies to: 454-456, 475-478, 491-494

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/types.ts` around lines 423 - 436, The FIXME marks show
multiple keyType fields typed as any (e.g., in SignDataOptions and
VerifyDataOptions and the other option types in this file); replace those any
types by importing KeyAlgorithm from `@openwallet-foundation/askar-nodejs` and use
it as the explicit type for keyType (or a narrow union like KeyAlgorithm |
string if compatibility is required), update all occurrences of keyType in the
file (including the other option/type declarations referenced) and adjust any
related code/consumers to satisfy the stronger type.
src/controllers/didcomm/proofs/ProofController.ts (1)

1-19: ⚡ Quick win

Fix import ordering to pass Validate check.

The static analysis correctly identifies import order violations that will fail CI.

🔧 Proposed fix
 import type { PeerDidNumAlgo2CreateOptions } from '@credo-ts/core'

 import {
   AcceptProofRequestOptions,
   DidCommProofExchangeRecordProps,
   ProofsProtocolVersionType,
   DidCommRouting,
 } from '@credo-ts/didcomm'
-
 import { PeerDidNumAlgo, createPeerDidDocumentFromServices } from '@credo-ts/core'
 import { Request as Req } from 'express'
 import { Body, Controller, Example, Get, Path, Post, Query, Route, Tags, Security, Request } from 'tsoa'
 import { injectable } from 'tsyringe'

 import { SCOPES } from '../../../enums'
 import ErrorHandlingService from '../../../errorHandlingService'
-import { SchedulePurge } from '../../../purge/decorators/SchedulePurge'
 import { PurgeRecordType } from '../../../purge/PurgeTypes'
+import { SchedulePurge } from '../../../purge/decorators/SchedulePurge'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/didcomm/proofs/ProofController.ts` around lines 1 - 19,
Reorder the import statements to satisfy the project's import-order/validate
rules: group external library imports together (e.g., '@credo-ts/core',
'@credo-ts/didcomm', 'express', 'tsoa', 'tsyringe') and then local project
imports (e.g., '../../../enums', '../../../errorHandlingService',
'../../../purge/decorators/SchedulePurge', '../../../purge/PurgeTypes',
'../../examples'), and within each group sort imports alphabetically; ensure
type-only imports like PeerDidNumAlgo2CreateOptions and local symbols like
ErrorHandlingService, SchedulePurge, PurgeRecordType, ProofRecordExample and
RecordId remain correctly imported but repositioned according to the grouping
and alphabetical ordering so Validate passes.
src/cliAgent.ts (1)

1-5: 💤 Low value

Duplicate import of @hyperledger/indy-vdr-nodejs.

The static analysis flags that @hyperledger/indy-vdr-nodejs is imported twice - once as a side-effect import (line 4) and later via named imports (implicit from line 60). If the side-effect import is intentional for module initialization order, consider adding a comment explaining why.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cliAgent.ts` around lines 1 - 5, There is a duplicate import of the same
module: the side-effect import at the top importing
"@hyperledger/indy-vdr-nodejs" and another import later (implicit named import)
causing the static analysis warning; either remove the later redundant import or
keep the side-effect top import and add a clear inline comment explaining the
initialization-order requirement (e.g., reference the top-level side-effect
import lines and the later named import occurrences) so reviewers understand the
intentional side-effect import for module initialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/events/CredentialEvents.ts`:
- Around line 65-70: The websocket event is enriching the payload under
payload.credentialRecord but consumers now read
payload.credentialExchangeRecord; update the sendWebSocketEvent call (the call
using config.socketServer and event) so that the enriched body is placed under
event.payload.credentialExchangeRecord (replace or add credentialExchangeRecord:
body) instead of credentialRecord, ensuring downstream handlers that reference
credentialExchangeRecord receive the updated record.

In `@src/purge/PurgeWorker.ts`:
- Around line 30-45: The infinite retry loop in PurgeWorker prevents shutdown
after NatsPurgeScheduler.stop() because while (true) never exits; add a shutdown
flag and cooperative cancellation: introduce a boolean (e.g., this.running)
that's set true when the worker starts and false in a new stop() method, change
the loop to while (this.running) and ensure any caught errors break/continue
only if this.running is true, and replace the blind sleep (await new Promise(...
PURGE_WORKER_RESTART_DELAY_MS)) with an abortable/waitable mechanism (e.g.,
Promise.race between a delay and a this.stopPromise or an AbortController) so
the delay can be cancelled when stop() is called; keep references to
consumer.consume(), processMessage(), PURGE_WORKER_RESTART_DELAY_MS and ensure
stop() drains/closes the consumer so consumer.consume() will not leave
background work running.

In `@src/purge/schedulers/CronPurgeScheduler.ts`:
- Around line 121-161: queryExpiredRecords currently loads all records (using
findAllByQuery({}) and repo.findByQuery(agent.context, {})) and filters
createdAt in-memory, which is expensive; modify queryExpiredRecords to push the
cutoff into the repository query or paginate: for DIDCOMM types call (agent as
any).modules.didcomm.credentials.findAllByQuery(...) and
didcomm.proofs.findAllByQuery(...) with a filter that expresses createdAt <
cutoff (use the same field name/format those APIs accept), and for OID4VC use
OpenId4VcIssuanceSessionRepository.findByQuery(agent.context, { createdAt_lt:
cutoffValue }) and OpenId4VcVerificationSessionRepository.findByQuery(...) or,
if the repos do not support createdAt filtering, implement server-side
pagination/batching (limit/offset or cursor) and loop fetching pages until
exhausted, collecting only ids older than cutoff before returning.

In `@src/utils/auth.ts`:
- Around line 47-54: The ClientAuth validator currently hard-requires
TRUST_LIST_URL causing valid ClientAuth startups to fail; update the ClientAuth
check in the validate/auth utility (the ClientAuth function) to NOT throw if
TRUST_LIST_URL is missing—only require TRUST_SERVICE_TOKEN_URL,
TRUST_SERVICE_CLIENT_ID and TRUST_SERVICE_CLIENT_SECRET there, and move or add
the TRUST_LIST_URL presence validation to the code path that calls
getX509CertsByClientToken (or the branch that actually uses the trust list) so
that TRUST_LIST_URL is enforced only when that functionality is used.

In `@src/utils/statusListService.ts`:
- Around line 149-175: The current read-modify-write around fetching and
PATCHing the status list (using getServerUrl/STATUS_LISTS_PATH and
fetchWithTimeout in the revoke flow that calls getListFromStatusListJWT,
setStatus, and signStatusList) is racy across replicas; replace it with
optimistic concurrency using the response ETag and an If-Match header on the
PATCH: read and capture res.headers.get('etag') when fetching the current JWT,
include that ETag in the PATCH request's If-Match header, and implement a retry
loop (bounded attempts) that on a 412/ETag-mismatch re-fetches the latest JWT,
recomputes statusList.setStatus(index, 1), re-signs via signStatusList, and
retries the PATCH; also surface a clear error after retries exhausted and stop
relying solely on the in-process statusListLocks for cross-instance safety.
- Around line 96-100: When creating the new StatusList, validate the computed
size before allocating the array: compute const size = listSize ??
Number(process.env.STATUS_LIST_DEFAULT_SIZE) and then assert
Number.isInteger(size) && size > 0 (and optionally enforce a max limit); if the
check fails, throw a clear configuration/validation error referencing
STATUS_LIST_DEFAULT_SIZE and listSize so callers fail fast instead of letting
new Array(size) throw. Update the allocation site (the StatusList(new
Array(size).fill(0), 1) call) to only run after this validation.

---

Duplicate comments:
In `@src/controllers/x509/x509.service.ts`:
- Around line 130-148: ImportX509Certificates currently unconditionally calls
pemToRawEd25519PrivateKey which only works for Ed25519; update the method to
branch on options.keyType (use the same symbol names: ImportX509Certificates,
options.keyType, pemToRawEd25519PrivateKey, transformPrivateKeyToPrivateJwk,
getTypeFromCurve) and perform curve-aware parsing: if keyType is Ed25519 call
pemToRawEd25519PrivateKey, for P-256/secp256k1 call the appropriate PEM-to-raw
parser (or add a generic pemToRawPrivateKey that accepts the curve) to produce
the raw private key bytes, throw a clear error for unsupported curves, then
continue converting the resulting raw key into a JWK via
transformPrivateKeyToPrivateJwk using getTypeFromCurve. Ensure the rest of the
flow (privateKey variable used below) uses the curve-specific raw bytes.

In `@src/utils/helpers.ts`:
- Around line 235-252: The code is logging/throwing full HTTP response bodies
and using console.* in the token fetch/error paths which can leak secrets; in
the axios.post token flow (tokenUrl, tokenResponse) and any axios.isAxiosError
branches replace console.log/console.error with the structured logger used
elsewhere (e.g., processLogger or injected logger), remove/error out any
inclusion of error.response.data or tokenResponse.data (only log non-sensitive
fields like status, statusText, and error.message/error.code), and stop
embedding raw response payloads into thrown Error objects—throw errors with a
short message and safe metadata (status/statusText) or ensure sensitive fields
(e.g., clientSecret, access_token) are redacted before any logging or exception
text; apply the same changes to the other equivalent blocks referenced (the
axios error handlers and any places that echo response bodies).

---

Nitpick comments:
In `@src/cli.ts`:
- Around line 3-9: The import ordering is wrong: load environment variables
before importing modules that may depend on them; move the dotenv import and
dotenv.config() so they occur before importing yargs (i.e., ensure dotenv is
imported and dotenv.config() runs prior to the yargs import line), keeping the
rest of imports (like runRestAgent from './cliAgent.js') unchanged to satisfy
the Validate check.

In `@src/cliAgent.ts`:
- Around line 1-5: There is a duplicate import of the same module: the
side-effect import at the top importing "@hyperledger/indy-vdr-nodejs" and
another import later (implicit named import) causing the static analysis
warning; either remove the later redundant import or keep the side-effect top
import and add a clear inline comment explaining the initialization-order
requirement (e.g., reference the top-level side-effect import lines and the
later named import occurrences) so reviewers understand the intentional
side-effect import for module initialization.

In `@src/controllers/didcomm/proofs/ProofController.ts`:
- Around line 1-19: Reorder the import statements to satisfy the project's
import-order/validate rules: group external library imports together (e.g.,
'@credo-ts/core', '@credo-ts/didcomm', 'express', 'tsoa', 'tsyringe') and then
local project imports (e.g., '../../../enums', '../../../errorHandlingService',
'../../../purge/decorators/SchedulePurge', '../../../purge/PurgeTypes',
'../../examples'), and within each group sort imports alphabetically; ensure
type-only imports like PeerDidNumAlgo2CreateOptions and local symbols like
ErrorHandlingService, SchedulePurge, PurgeRecordType, ProofRecordExample and
RecordId remain correctly imported but repositioned according to the grouping
and alphabetical ordering so Validate passes.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts`:
- Around line 6-15: Remove the inline ESLint disable and reorder the imports in
issuance-sessions.Controller.ts to satisfy import/order: delete the "//
eslint-disable-next-line import/order" line and group imports by type (external
libs first, then internal modules), sorting each group alphabetically; ensure
SCOPES, ErrorHandlingService, SchedulePurge, PurgeRecordType,
OpenId4VcIssuanceSessionsCreateOffer and issuanceSessionService remain imported
but reordered into proper groups so ESLint import/order no longer fails.

In `@src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts`:
- Line 22: The declaration const offerStatusInfo: any[] = [] and other any
usages (e.g., variables named cred and supported around the processing logic)
weaken type safety—replace these anys with concrete interfaces from
issuer.types.ts (for example use IssuerCredential, CredentialSupportedFormat,
OfferStatusInfo or the appropriately named exported types) and update
function/method signatures in issuance-sessions.service.ts to use those types;
adjust any array/field shapes (e.g., offerStatusInfo: OfferStatusInfo[]), cred:
IssuerCredential, supported: CredentialSupportedFormat[] (or the exact exported
names) and import the types from issuer.types.ts so the compiler and IDE can
validate structure and provide autocomplete.
- Around line 6-11: Reorder the import block to match the project's ESLint
import-order rules: group and sort internal imports so that enum/constant
imports (CredentialFormat, SignerMethod, STATUS_LISTS_PATH) are together, error
imports (BadRequestError, NotFoundError) follow, and utility imports
(checkAndCreateStatusList, getServerUrl, revokeCredentialInStatusList) come
after; update the import statements in issuance-sessions.service.ts to reflect
that grouping and alphabetical ordering within each group so ESLint passes.

In `@src/controllers/types.ts`:
- Around line 423-436: The FIXME marks show multiple keyType fields typed as any
(e.g., in SignDataOptions and VerifyDataOptions and the other option types in
this file); replace those any types by importing KeyAlgorithm from
`@openwallet-foundation/askar-nodejs` and use it as the explicit type for keyType
(or a narrow union like KeyAlgorithm | string if compatibility is required),
update all occurrences of keyType in the file (including the other option/type
declarations referenced) and adjust any related code/consumers to satisfy the
stronger type.

In `@src/controllers/x509/x509.service.ts`:
- Line 24: Rename the class identifier x509Service to PascalCase X509Service
everywhere it appears (class declaration and any references/imports/usages) to
follow TypeScript naming conventions and satisfy SonarCloud; update the class
declaration named x509Service to X509Service and adjust all references
(constructors, exports, imports, type annotations, and any instantiation sites)
to use X509Service so symbols remain consistent.

In `@src/utils/agent.ts`:
- Line 50: The property allowInsecureHttpUrls is using a redundant ternary;
replace the expression "process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() ===
'true' ? true : false" with the direct boolean comparison
"process.env.ALLOW_INSECURE_HTTP_URLS?.toLowerCase() === 'true'" where
allowInsecureHttpUrls is defined in the agent configuration so the value is a
boolean without the unnecessary ? true : false.

In `@tsconfig.build.json`:
- Around line 4-17: The tsconfig currently sets "target" to "ES2017" while "lib"
is "ESNext", which lets TypeScript accept newer runtime APIs without ensuring
they're available at runtime; change the tsconfig settings so "target" and "lib"
align to the same ES level that matches our minimum supported Node version (for
example set both "target" and "lib" to "ES2022" for Node 18+), or lower them if
we must support older Node releases; only keep lib > target if you intentionally
add runtime polyfills (e.g., core-js), and update any CI/test matrix or docs to
reflect the chosen minimum Node version.
- Around line 4-17: The tsconfig has a mismatch between "target": "ES2017" and
"lib": ["ESNext"], causing the compiler to allow newer runtime APIs while
emitting older syntax; update the TypeScript config by changing the "target"
value from ES2017 to "ES2022" (or otherwise align "lib" to the same ECMAScript
level) so "target" and "lib" are consistent with Node 22's capabilities; ensure
the "target" and "lib" fields in tsconfig.build.json are updated together to
avoid the type/runtime inconsistency.
🪄 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: 717bd97a-e240-488b-ad04-dec15560123d

📥 Commits

Reviewing files that changed from the base of the PR and between 0ff8131 and 8515bf7.

📒 Files selected for processing (21)
  • samples/cliConfig.json
  • src/cli.ts
  • src/cliAgent.ts
  • src/controllers/didcomm/proofs/ProofController.ts
  • src/controllers/openid4vc/issuance-sessions/issuance-sessions.Controller.ts
  • src/controllers/openid4vc/issuance-sessions/issuance-sessions.service.ts
  • src/controllers/types.ts
  • src/controllers/x509/x509.service.ts
  • src/events/CredentialEvents.ts
  • src/purge/PurgeConstants.ts
  • src/purge/PurgeTypes.ts
  • src/purge/PurgeWorker.ts
  • src/purge/decorators/SchedulePurge.ts
  • src/purge/schedulers/CronPurgeScheduler.ts
  • src/purge/schedulers/NatsPurgeScheduler.ts
  • src/utils/agent.ts
  • src/utils/auth.ts
  • src/utils/helpers.ts
  • src/utils/statusListService.ts
  • tsconfig.build.json
  • tsoa.json
✅ Files skipped from review due to trivial changes (2)
  • tsoa.json
  • src/purge/PurgeTypes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/purge/PurgeConstants.ts

Comment thread src/events/CredentialEvents.ts
Comment thread src/purge/PurgeWorker.ts Outdated
Comment thread src/purge/schedulers/CronPurgeScheduler.ts
Comment thread src/utils/auth.ts
Comment thread src/utils/statusListService.ts
Comment thread src/utils/statusListService.ts
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
@tipusinghaw tipusinghaw requested a review from RinkalBhojani May 5, 2026 07:53
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 5, 2026

@tipusinghaw tipusinghaw merged commit 21b0336 into main May 5, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants