feat: added consent notice and intent mapping logic#1586
Merged
tipusinghaw merged 7 commits intomainfrom Mar 18, 2026
Merged
Conversation
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>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
📝 WalkthroughWalkthroughAdds intent-notice management across API Gateway, Ecosystem, and OID4VC services: new DTOs/interfaces, controllers, service and repository methods, URL/consent helpers, Prisma schema + migrations, and NATS-driven propagation. Also adds optional ecosystemId to existing intent template retrieval paths. Changes
Sequence DiagramsequenceDiagram
participant Client as API Client
participant Gateway as API Gateway<br/>(IntentController)
participant EcosystemSvc as EcosystemService
participant Repo as EcosystemRepository
participant DB as Database
participant Oid4vcSvc as Oid4vcVerificationService
participant Helper as Helpers (validateNoticeUrl / fetchConsentNotice)
Client->>Gateway: POST /intent/notice (CreateIntentNoticeDto)
Gateway->>EcosystemSvc: createIntentNotice(dto, user)
EcosystemSvc->>EcosystemSvc: validateEcosystemLead(userId, ecosystemId?)
EcosystemSvc->>Helper: validateNoticeUrl(noticeUrl)
Helper-->>EcosystemSvc: validated
EcosystemSvc->>Repo: intentNoticeExists(intentId, orgId?)
Repo->>DB: SELECT ...
DB-->>Repo: exists?
Repo-->>EcosystemSvc: boolean
EcosystemSvc->>Repo: createIntentNotice(...)
Repo->>DB: INSERT intent_notices
DB-->>Repo: created row
Repo-->>EcosystemSvc: notice object
EcosystemSvc-->>Gateway: created notice
Gateway-->>Client: 201 Created + notice
Note over Oid4vcSvc, Repo: When creating verification presentation
Client->>Gateway: POST /oid4vc/intent/presentation (includes ecosystemId)
Gateway->>Oid4vcSvc: createIntentBasedVerificationPresentation(..., ecosystemId)
Oid4vcSvc->>Repo: getIntentNoticeByIntentId(intentId, orgId?)
Repo->>DB: SELECT noticeUrl
DB-->>Repo: noticeUrl?
Repo-->>Oid4vcSvc: noticeUrl
Oid4vcSvc->>Helper: fetchConsentNotice(noticeUrl, transactionId)
Helper-->>Oid4vcSvc: consentNoticeUrl
Oid4vcSvc-->>Gateway: session + consentNoticeUrl
Gateway-->>Client: 201 Created + session
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
tipusinghaw
changed the title
tipusinghaw
changed the title
Contributor
Author
|
review @coderabbitai |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts`:
- Around line 636-679: The getIntentNotices controller currently allows unscoped
reads because id and intentId are optional and no ecosystem context is enforced;
fix by requiring and validating caller ecosystem scope and passing it to the
service: extract the caller's ecosystemId from request/user context (or require
an ecosystemId query/path param), validate it (UUID and that the user has
access) and then call ecosystemService.getIntentNotices(id, intentId,
ecosystemId) or reject the request if the caller is out-of-scope; update the
EcosystemRolesGuard or add an explicit check in getIntentNotices (and update the
service signature) so notices are only returned for the scoped ecosystem, or
deprecate this route in favor of a scoped endpoint like GET
/intent/ecosystem/:ecosystemId/notice.
In `@apps/ecosystem/repositories/ecosystem.repository.ts`:
- Around line 1701-1711: The createIntentNotice(intentId, noticeUrl, userId,
orgId) is race-prone when combined with intentNoticeExists; change
createIntentNotice to perform an atomic operation (e.g., use
prisma.intent_notices.upsert or a create with unique constraint handling) so
duplicates cannot be inserted under concurrent requests, and add a DB-level
unique constraint on (intentId, orgId) in the Prisma schema followed by a
migration; also update any callers or intentNoticeExists logic to rely on the DB
uniqueness rather than a separate existence check.
- Around line 814-823: The intent-template lookup in
getIntentTemplateByIntentAndOrg currently treats ecosystemId as optional which
can return wrong templates across ecosystems; update the
prisma.intent_templates.findFirst call to always include ecosystemId in the
intent filter (remove the conditional spread ...(ecosystemId && { ecosystemId })
and use intent: { is: { name: intentName, ecosystemId } }), and add a guard at
the start of getIntentTemplateByIntentAndOrg to throw or validate when
ecosystemId is missing since intents are always created with a non-null
ecosystemId.
In `@apps/ecosystem/src/ecosystem.helper.ts`:
- Around line 12-15: The fetch call is not wired to the AbortController and the
timeout can leak if fetch throws; update the code that creates
controller/timeout and calls fetch(noticeUrl) so you pass controller.signal into
fetch (fetch(noticeUrl, { signal: controller.signal })) and wrap the await
fetch(...) in a try/finally so clearTimeout(timeout) (and optionally
controller.abort() on errors) always runs; reference the AbortController
instance named controller, the timeout variable, and the fetch(noticeUrl) call
when making the change.
In `@apps/ecosystem/src/ecosystem.service.ts`:
- Around line 1044-1051: The org membership check currently calls
getEcosystemOrg and does not validate the membership status; change it to use
findEcosystemOrg with the same active-status validation used elsewhere (replace
getEcosystemOrg(intent['ecosystemId'], orgId) with
findEcosystemOrg(intent['ecosystemId'], orgId) and verify the returned object's
status is EcosystemOrgStatus.ACTIVE, throwing the same RpcException
(HttpStatus.FORBIDDEN / 'The provided orgId is not a member or lead of this
ecosystem.') if the record is missing or not ACTIVE so behavior matches the
template assignment path.
- Around line 1054-1062: Add partitioned unique constraints for intent_notices
and handle P2002 in the repository: create a new migration that adds two unique
indexes like intent_templates (one WHERE "orgId" IS NOT NULL on
("orgId","intentId"), and one WHERE "orgId" IS NULL on ("intentId")) to prevent
duplicates, then update ecosystemRepository.createIntentNotice to catch Prisma
P2002/unique constraint errors and rethrow an RpcException with
HttpStatus.CONFLICT (mirror the createEcosystemInvitation error-mapping
pattern); keep the existing intentNoticeExists check if desired but rely on DB
constraint + repo-level P2002 handling to guarantee uniqueness.
In `@apps/oid4vc-verification/src/oid4vc-verification.helper.ts`:
- Around line 6-8: Build the consent URL using a proper URL/URLSearchParams API
instead of string concatenation: create a new URL(noticeUrl) (or new URL(base)
then set pathname) and append transactionId with
url.searchParams.set('transactionId', transactionId) to avoid double '?' when
noticeUrl already has query params; then perform the outbound call using fetch
guarded by an AbortController with a timeout (create AbortController, pass
signal to fetch, and clear the timeout on completion) so the request cannot hang
indefinitely; update the references to consentNoticeUrl and the fetch call in
the helper where they are used.
In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts`:
- Around line 410-427: The code may throw inside the consent enrichment block
after the verification session is already created (createdSession,
fetchConsentNotice), causing the API to fail and clients to retry; wrap the
consent-fetching logic so failures are caught and do not propagate: when
intentId and intentNotice?.noticeUrl are present call fetchConsentNotice inside
a try/catch (around the await fetchConsentNotice call), log the error (using the
same logger or processLogger) including intentId and noticeUrl, and skip setting
createdSession.consentNoticeUrl on error; keep the existing
this.natsClient.sendNatsMessage and intentNotice lookup unchanged but ensure any
exception from fetchConsentNotice does not bubble up to the caller.
In `@libs/prisma-service/prisma/schema.prisma`:
- Around line 743-757: The intent_notices model allows duplicates because
UNIQUE(intentId, orgId) won't prevent multiple NULL orgId rows; add two partial
unique indexes to enforce DB-level uniqueness by creating a unique index on
(intentId, orgId) WHERE orgId IS NOT NULL and a unique index on intentId WHERE
orgId IS NULL (use descriptive index names like intent_notices_unique_with_org
and intent_notices_unique_no_org), or if you prefer to rely on the existing
application-level check (intentNoticeExists), explicitly document that choice
and remove the current indexes to avoid misleading constraints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8c1efb6-1373-45c6-9e81-2277ba03bc93
📒 Files selected for processing (17)
apps/api-gateway/src/ecosystem/ecosystem.service.tsapps/api-gateway/src/ecosystem/intent/intent.controller.tsapps/api-gateway/src/oid4vc-verification/dtos/create-intent-notice.dto.tsapps/api-gateway/src/oid4vc-verification/oid4vc-verification.controller.tsapps/api-gateway/src/oid4vc-verification/oid4vc-verification.service.tsapps/ecosystem/repositories/ecosystem.repository.tsapps/ecosystem/src/ecosystem.controller.tsapps/ecosystem/src/ecosystem.helper.tsapps/ecosystem/src/ecosystem.service.tsapps/oid4vc-verification/interfaces/intent-notice.interfaces.tsapps/oid4vc-verification/src/oid4vc-verification.controller.tsapps/oid4vc-verification/src/oid4vc-verification.helper.tsapps/oid4vc-verification/src/oid4vc-verification.service.tslibs/common/src/response-messages/index.tslibs/prisma-service/prisma/migrations/20260312082536_added_intent_notice_table/migration.sqllibs/prisma-service/prisma/migrations/20260312084927_add_org_id_to_intent_notices/migration.sqllibs/prisma-service/prisma/schema.prisma
Comment on lines
+636
to
+679
| @Get('/notice') | ||
| @ApiBearerAuth() | ||
| @Roles(OrgRoles.ECOSYSTEM_LEAD, OrgRoles.ECOSYSTEM_MEMBER) | ||
| @UseGuards(AuthGuard('jwt'), EcosystemRolesGuard) | ||
| @ApiOperation({ | ||
| summary: 'Get intent notices', | ||
| description: 'Retrieves intent notices. Filter by notice id or intentId (both optional).' | ||
| }) | ||
| @ApiQuery({ name: 'id', required: false, type: String, description: 'Filter by notice PK UUID (optional)' }) | ||
| @ApiQuery({ name: 'intentId', required: false, type: String, description: 'Filter by intent UUID (optional)' }) | ||
| @ApiResponse({ status: HttpStatus.OK, description: 'Intent notices fetched successfully', type: ApiResponseDto }) | ||
| async getIntentNotices( | ||
| @Res() res: Response, | ||
| @Query( | ||
| 'id', | ||
| new ParseUUIDPipe({ | ||
| version: '4', | ||
| optional: true, | ||
| exceptionFactory: (): Error => { | ||
| throw new BadRequestException('Invalid notice ID'); | ||
| } | ||
| }) | ||
| ) | ||
| id?: string, | ||
| @Query( | ||
| 'intentId', | ||
| new ParseUUIDPipe({ | ||
| version: '4', | ||
| optional: true, | ||
| exceptionFactory: (): Error => { | ||
| throw new BadRequestException('Invalid intent ID'); | ||
| } | ||
| }) | ||
| ) | ||
| intentId?: string | ||
| ): Promise<Response> { | ||
| const result = await this.ecosystemService.getIntentNotices(id, intentId); | ||
| const finalResponse: IResponse = { | ||
| statusCode: HttpStatus.OK, | ||
| message: ResponseMessages.intentNotice.success.fetchAll, | ||
| data: result | ||
| }; | ||
| return res.status(HttpStatus.OK).json(finalResponse); | ||
| } |
There was a problem hiding this comment.
GET /intent/notice allows unscoped cross-ecosystem reads.
At Line 647-Line 672, both filters are optional and no ecosystem/user scope is sent downstream. A user with ecosystem role access can retrieve notices outside their intended scope.
🔧 Suggested direction
async getIntentNotices(
@@
- ): Promise<Response> {
+ ): Promise<Response> {
+ if (!id && !intentId) {
+ throw new BadRequestException('Either id or intentId is required');
+ }Additionally, prefer a scoped contract (ecosystemId + caller context) for this read path, or deprecate this route in favor of GET /intent/ecosystem/:ecosystemId/notice.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Get('/notice') | |
| @ApiBearerAuth() | |
| @Roles(OrgRoles.ECOSYSTEM_LEAD, OrgRoles.ECOSYSTEM_MEMBER) | |
| @UseGuards(AuthGuard('jwt'), EcosystemRolesGuard) | |
| @ApiOperation({ | |
| summary: 'Get intent notices', | |
| description: 'Retrieves intent notices. Filter by notice id or intentId (both optional).' | |
| }) | |
| @ApiQuery({ name: 'id', required: false, type: String, description: 'Filter by notice PK UUID (optional)' }) | |
| @ApiQuery({ name: 'intentId', required: false, type: String, description: 'Filter by intent UUID (optional)' }) | |
| @ApiResponse({ status: HttpStatus.OK, description: 'Intent notices fetched successfully', type: ApiResponseDto }) | |
| async getIntentNotices( | |
| @Res() res: Response, | |
| @Query( | |
| 'id', | |
| new ParseUUIDPipe({ | |
| version: '4', | |
| optional: true, | |
| exceptionFactory: (): Error => { | |
| throw new BadRequestException('Invalid notice ID'); | |
| } | |
| }) | |
| ) | |
| id?: string, | |
| @Query( | |
| 'intentId', | |
| new ParseUUIDPipe({ | |
| version: '4', | |
| optional: true, | |
| exceptionFactory: (): Error => { | |
| throw new BadRequestException('Invalid intent ID'); | |
| } | |
| }) | |
| ) | |
| intentId?: string | |
| ): Promise<Response> { | |
| const result = await this.ecosystemService.getIntentNotices(id, intentId); | |
| const finalResponse: IResponse = { | |
| statusCode: HttpStatus.OK, | |
| message: ResponseMessages.intentNotice.success.fetchAll, | |
| data: result | |
| }; | |
| return res.status(HttpStatus.OK).json(finalResponse); | |
| } | |
| `@Get`('/notice') | |
| `@ApiBearerAuth`() | |
| `@Roles`(OrgRoles.ECOSYSTEM_LEAD, OrgRoles.ECOSYSTEM_MEMBER) | |
| `@UseGuards`(AuthGuard('jwt'), EcosystemRolesGuard) | |
| `@ApiOperation`({ | |
| summary: 'Get intent notices', | |
| description: 'Retrieves intent notices. Filter by notice id or intentId (both optional).' | |
| }) | |
| `@ApiQuery`({ name: 'id', required: false, type: String, description: 'Filter by notice PK UUID (optional)' }) | |
| `@ApiQuery`({ name: 'intentId', required: false, type: String, description: 'Filter by intent UUID (optional)' }) | |
| `@ApiResponse`({ status: HttpStatus.OK, description: 'Intent notices fetched successfully', type: ApiResponseDto }) | |
| async getIntentNotices( | |
| `@Res`() res: Response, | |
| `@Query`( | |
| 'id', | |
| new ParseUUIDPipe({ | |
| version: '4', | |
| optional: true, | |
| exceptionFactory: (): Error => { | |
| throw new BadRequestException('Invalid notice ID'); | |
| } | |
| }) | |
| ) | |
| id?: string, | |
| `@Query`( | |
| 'intentId', | |
| new ParseUUIDPipe({ | |
| version: '4', | |
| optional: true, | |
| exceptionFactory: (): Error => { | |
| throw new BadRequestException('Invalid intent ID'); | |
| } | |
| }) | |
| ) | |
| intentId?: string | |
| ): Promise<Response> { | |
| if (!id && !intentId) { | |
| throw new BadRequestException('Either id or intentId is required'); | |
| } | |
| const result = await this.ecosystemService.getIntentNotices(id, intentId); | |
| const finalResponse: IResponse = { | |
| statusCode: HttpStatus.OK, | |
| message: ResponseMessages.intentNotice.success.fetchAll, | |
| data: result | |
| }; | |
| return res.status(HttpStatus.OK).json(finalResponse); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api-gateway/src/ecosystem/intent/intent.controller.ts` around lines 636
- 679, The getIntentNotices controller currently allows unscoped reads because
id and intentId are optional and no ecosystem context is enforced; fix by
requiring and validating caller ecosystem scope and passing it to the service:
extract the caller's ecosystemId from request/user context (or require an
ecosystemId query/path param), validate it (UUID and that the user has access)
and then call ecosystemService.getIntentNotices(id, intentId, ecosystemId) or
reject the request if the caller is out-of-scope; update the EcosystemRolesGuard
or add an explicit check in getIntentNotices (and update the service signature)
so notices are only returned for the scoped ecosystem, or deprecate this route
in favor of a scoped endpoint like GET /intent/ecosystem/:ecosystemId/notice.
Contributor
Author
There was a problem hiding this comment.
The ecosystem filter is applied in DB query
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check getIntentNotices implementation in the ecosystem service and repository
rg -n "getIntentNotices" --type ts -A 20 -B 2
Length of output: 29429
@tipusinghaw, that's correct — when intentId is supplied, the repository does derive the intent's ecosystemId and scopes the DB query accordingly (Lines 1726–1735 in ecosystem.repository.ts). Thanks for the clarification.
That said, two edge cases remain unscoped:
- Only
idsupplied — thewhereclause becomes{ id }with no ecosystem filter, so a caller can fetch any notice by its PK regardless of ecosystem membership. - Neither
idnorintentIdsupplied — thewhereclause is{}, which returns allintent_noticesacross every ecosystem.
A minimal guard at the controller level would close both gaps:
async getIntentNotices(
`@Res`() res: Response,
id?: string,
intentId?: string
): Promise<Response> {
if (!id && !intentId) {
throw new BadRequestException('Either id or intentId is required');
}
const result = await this.ecosystemService.getIntentNotices(id, intentId);
...
}And for the id-only path, passing the caller's verified ecosystemId (from the JWT / EcosystemRolesGuard context) down to the repository so it can add intent: { is: { ecosystemId } } to the where clause would fully close the cross-ecosystem read.
Would you like to address these two residual cases, or is the GET /intent/notice route intentionally an admin/internal endpoint where cross-ecosystem reads are acceptable?
🧠 Learnings used
Learnt from: tipusinghaw
Repo: credebl/platform PR: 1584
File: apps/api-gateway/src/oid4vc-verification/dtos/create-intent-notice.dto.ts:10-13
Timestamp: 2026-03-18T12:02:15.511Z
Learning: In the credebl/platform repository, `noticeUrl` URL validation for intent notices is intentionally handled at the service layer via the `validateNoticeUrl` helper in `apps/ecosystem/src/ecosystem.helper.ts`, which performs a live HTTP reachability check, rather than purely at the DTO layer with `IsUrl()`. Do not flag the absence of `IsUrl()` on `noticeUrl` in `CreateIntentNoticeDto` as a missing validation.
Learnt from: pranalidhanavade
Repo: credebl/platform PR: 1547
File: libs/prisma-service/prisma/migrations/20260119130717_intent_table_creation/migration.sql:4-11
Timestamp: 2026-01-22T12:39:42.299Z
Learning: In the intents table (libs/prisma-service/prisma/schema.prisma), ecosystemId is NOT NULL because intent records are never created without an ecosystemId in the application workflow. This differs from ecosystem_invitations where ecosystemId is nullable to support deferred ecosystem creation.
Learnt from: sujitaw
Repo: credebl/platform PR: 1545
File: libs/prisma-service/prisma/schema.prisma:793-793
Timestamp: 2026-01-19T10:03:19.860Z
Learning: In the ecosystem_invitations model, deletions involving invitedOrg are handled at the application level with conditions matching both ecosystemId and orgId, rather than relying on database-level cascade behavior.
Learnt from: sujitaw
Repo: credebl/platform PR: 1545
File: libs/prisma-service/prisma/schema.prisma:792-792
Timestamp: 2026-01-19T09:58:13.669Z
Learning: In the ecosystem_invitations model (libs/prisma-service/prisma/schema.prisma), ecosystemId is intentionally nullable to support a workflow where users create their ecosystem first and the ecosystemId is added later. Application-level validation prevents duplicate records with the same email when both ecosystemId and orgId are null, addressing the PostgreSQL NULL uniqueness behavior.
Comment on lines
+814
to
823
| async getIntentTemplateByIntentAndOrg( | ||
| intentName: string, | ||
| verifierOrgId: string, | ||
| ecosystemId?: string | ||
| ): Promise<intent_templates | null> { | ||
| try { | ||
| const template = await this.prisma.intent_templates.findFirst({ | ||
| where: { | ||
| intent: { is: { name: intentName } }, | ||
| intent: { is: { name: intentName, ...(ecosystemId && { ecosystemId }) } }, | ||
| OR: [{ orgId: verifierOrgId }, { orgId: null }] |
There was a problem hiding this comment.
Require ecosystem scoping for intent-template lookup.
At Line 822, ecosystemId is optional in the intent filter. If the same intent name exists in multiple ecosystems, this can return the wrong template.
🔧 Proposed fix
- async getIntentTemplateByIntentAndOrg(
+ async getIntentTemplateByIntentAndOrg(
intentName: string,
verifierOrgId: string,
- ecosystemId?: string
+ ecosystemId: string
): Promise<intent_templates | null> {
@@
- intent: { is: { name: intentName, ...(ecosystemId && { ecosystemId }) } },
+ intent: { is: { name: intentName, ecosystemId } },Based on learnings: In the intents table (libs/prisma-service/prisma/schema.prisma), ecosystemId is NOT NULL because intent records are never created without an ecosystemId in the application workflow.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async getIntentTemplateByIntentAndOrg( | |
| intentName: string, | |
| verifierOrgId: string, | |
| ecosystemId?: string | |
| ): Promise<intent_templates | null> { | |
| try { | |
| const template = await this.prisma.intent_templates.findFirst({ | |
| where: { | |
| intent: { is: { name: intentName } }, | |
| intent: { is: { name: intentName, ...(ecosystemId && { ecosystemId }) } }, | |
| OR: [{ orgId: verifierOrgId }, { orgId: null }] | |
| async getIntentTemplateByIntentAndOrg( | |
| intentName: string, | |
| verifierOrgId: string, | |
| ecosystemId: string | |
| ): Promise<intent_templates | null> { | |
| try { | |
| const template = await this.prisma.intent_templates.findFirst({ | |
| where: { | |
| intent: { is: { name: intentName, ecosystemId } }, | |
| OR: [{ orgId: verifierOrgId }, { orgId: null }] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ecosystem/repositories/ecosystem.repository.ts` around lines 814 - 823,
The intent-template lookup in getIntentTemplateByIntentAndOrg currently treats
ecosystemId as optional which can return wrong templates across ecosystems;
update the prisma.intent_templates.findFirst call to always include ecosystemId
in the intent filter (remove the conditional spread ...(ecosystemId && {
ecosystemId }) and use intent: { is: { name: intentName, ecosystemId } }), and
add a guard at the start of getIntentTemplateByIntentAndOrg to throw or validate
when ecosystemId is missing since intents are always created with a non-null
ecosystemId.
Comment on lines
+1701
to
+1711
| async createIntentNotice(intentId: string, noticeUrl: string, userId: string, orgId?: string): Promise<object> { | ||
| try { | ||
| return await this.prisma.intent_notices.create({ | ||
| data: { | ||
| intent: { connect: { id: intentId } }, | ||
| ...(orgId && { organisation: { connect: { id: orgId } } }), | ||
| noticeUrl, | ||
| createdBy: userId, | ||
| lastChangedBy: userId | ||
| } | ||
| }); |
There was a problem hiding this comment.
Intent notice creation is race-prone without DB uniqueness.
intentNoticeExists (Line 1802) + createIntentNotice (Line 1703) is non-atomic. Concurrent requests can create duplicates for the same (intentId, orgId).
🔧 Proposed hardening
async createIntentNotice(intentId: string, noticeUrl: string, userId: string, orgId?: string): Promise<object> {
try {
return await this.prisma.intent_notices.create({
@@
});
} catch (error) {
+ if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') {
+ throw new BadRequestException('Intent notice already exists for this intent and organization');
+ }
this.logger.error(`createIntentNotice error: ${error}`);
throw error;
}
}Also add a DB-level unique constraint on (intentId, orgId) in Prisma schema + migration to make this truly safe.
Also applies to: 1800-1805
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ecosystem/repositories/ecosystem.repository.ts` around lines 1701 -
1711, The createIntentNotice(intentId, noticeUrl, userId, orgId) is race-prone
when combined with intentNoticeExists; change createIntentNotice to perform an
atomic operation (e.g., use prisma.intent_notices.upsert or a create with unique
constraint handling) so duplicates cannot be inserted under concurrent requests,
and add a DB-level unique constraint on (intentId, orgId) in the Prisma schema
followed by a migration; also update any callers or intentNoticeExists logic to
rely on the DB uniqueness rather than a separate existence check.
Contributor
Author
There was a problem hiding this comment.
const isAlreadyExists = await this.ecosystemRepository.intentNoticeExists(intentId, orgId ?? null); validation exist in service file
There was a problem hiding this comment.
@tipusinghaw, thanks for pointing that out — the service-level guard does cover the sequential case correctly. However, the concern is specifically about concurrent requests:
- Request A calls
intentNoticeExists(intentId, orgId)→ returnsfalse(no record yet). - Request B calls
intentNoticeExists(intentId, orgId)→ also returnsfalse(Request A hasn't committed yet). - Request A calls
createIntentNotice(...)→ succeeds. - Request B calls
createIntentNotice(...)→ also succeeds → duplicate row.
This is the classic TOCTOU (Time-of-Check / Time-of-Use) race. The service-layer check can't prevent it because the check and the insert are two separate, non-atomic DB round-trips.
The recommended hardening is to add a DB-level unique constraint on (intentId, orgId) so that the database itself rejects duplicates, and then catch the P2002 error in createIntentNotice:
async createIntentNotice(intentId: string, noticeUrl: string, userId: string, orgId?: string): Promise<object> {
try {
return await this.prisma.intent_notices.create({
data: {
intent: { connect: { id: intentId } },
...(orgId && { organisation: { connect: { id: orgId } } }),
noticeUrl,
createdBy: userId,
lastChangedBy: userId
}
});
} catch (error) {
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') {
throw new RpcException({
status: HttpStatus.CONFLICT,
message: 'Intent notice already exists for this intent and organization'
});
}
this.logger.error(`createIntentNotice error: ${error}`);
throw error;
}
}And in the Prisma schema/migration:
ALTER TABLE intent_notices ADD CONSTRAINT intent_notices_intentId_orgId_key UNIQUE ("intentId", "orgId");This makes the uniqueness guarantee atomic and reliable under any level of concurrency. Would you like to address this, or should I open a follow-up issue?
🧠 Learnings used
Learnt from: tipusinghaw
Repo: credebl/platform PR: 1584
File: apps/api-gateway/src/oid4vc-verification/dtos/create-intent-notice.dto.ts:10-13
Timestamp: 2026-03-18T12:02:15.511Z
Learning: In the credebl/platform repository, `noticeUrl` URL validation for intent notices is intentionally handled at the service layer via the `validateNoticeUrl` helper in `apps/ecosystem/src/ecosystem.helper.ts`, which performs a live HTTP reachability check, rather than purely at the DTO layer with `IsUrl()`. Do not flag the absence of `IsUrl()` on `noticeUrl` in `CreateIntentNoticeDto` as a missing validation.
Learnt from: pranalidhanavade
Repo: credebl/platform PR: 1547
File: libs/prisma-service/prisma/migrations/20260119130717_intent_table_creation/migration.sql:4-11
Timestamp: 2026-01-22T12:39:42.299Z
Learning: In the intents table (libs/prisma-service/prisma/schema.prisma), ecosystemId is NOT NULL because intent records are never created without an ecosystemId in the application workflow. This differs from ecosystem_invitations where ecosystemId is nullable to support deferred ecosystem creation.
Learnt from: sujitaw
Repo: credebl/platform PR: 1545
File: libs/prisma-service/prisma/schema.prisma:792-792
Timestamp: 2026-01-19T09:58:13.669Z
Learning: In the ecosystem_invitations model (libs/prisma-service/prisma/schema.prisma), ecosystemId is intentionally nullable to support a workflow where users create their ecosystem first and the ecosystemId is added later. Application-level validation prevents duplicate records with the same email when both ecosystemId and orgId are null, addressing the PostgreSQL NULL uniqueness behavior.
Comment on lines
+1044
to
+1051
| if (orgId) { | ||
| const orgEcosystemMembership = await this.ecosystemRepository.getEcosystemOrg(intent['ecosystemId'], orgId); | ||
| if (!orgEcosystemMembership) { | ||
| throw new RpcException({ | ||
| statusCode: HttpStatus.FORBIDDEN, | ||
| message: 'The provided orgId is not a member or lead of this ecosystem.' | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository helper definitions:"
rg -n -A10 -B3 'async\s+(findEcosystemOrg|getEcosystemOrg)\s*\(' apps/ecosystem/repositories/ecosystem.repository.ts
echo
echo "Service call sites:"
rg -n -A2 -B2 '\b(findEcosystemOrg|getEcosystemOrg)\s*\(' apps/ecosystem/src/ecosystem.service.tsRepository: credebl/platform
Length of output: 1629
Use findEcosystemOrg with status validation to match the existing org membership check pattern.
Line 1045 uses getEcosystemOrg without checking status !== EcosystemOrgStatus.ACTIVE, but the identical org membership validation for templates (line 630) uses findEcosystemOrg with the active status check. This inconsistency means notice creation accepts inactive org members while template assignment rejects them.
Suggested fix
- const orgEcosystemMembership = await this.ecosystemRepository.getEcosystemOrg(intent['ecosystemId'], orgId);
- if (!orgEcosystemMembership) {
+ const orgEcosystemMembership = await this.ecosystemRepository.findEcosystemOrg(intent['ecosystemId'], orgId);
+ if (!orgEcosystemMembership || orgEcosystemMembership.status !== EcosystemOrgStatus.ACTIVE) {
throw new RpcException({
statusCode: HttpStatus.FORBIDDEN,
message: 'The provided orgId is not a member or lead of this ecosystem.'
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ecosystem/src/ecosystem.service.ts` around lines 1044 - 1051, The org
membership check currently calls getEcosystemOrg and does not validate the
membership status; change it to use findEcosystemOrg with the same active-status
validation used elsewhere (replace getEcosystemOrg(intent['ecosystemId'], orgId)
with findEcosystemOrg(intent['ecosystemId'], orgId) and verify the returned
object's status is EcosystemOrgStatus.ACTIVE, throwing the same RpcException
(HttpStatus.FORBIDDEN / 'The provided orgId is not a member or lead of this
ecosystem.') if the record is missing or not ACTIVE so behavior matches the
template assignment path.
Comment on lines
+1054
to
+1062
| const isAlreadyExists = await this.ecosystemRepository.intentNoticeExists(intentId, orgId ?? null); | ||
| if (isAlreadyExists) { | ||
| const slotLabel = orgId ? `orgId ${orgId}` : 'no orgId'; | ||
| throw new RpcException({ | ||
| statusCode: HttpStatus.CONFLICT, | ||
| message: `An intent notice with ${slotLabel} already exists for this intent.` | ||
| }); | ||
| } | ||
| return await this.ecosystemRepository.createIntentNotice(intentId, noticeUrl, userId, orgId); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Intent notice schema / migrations:"
rg -n -C3 'intent_notice|intent_notices|@@unique|UNIQUE|intentId|orgId' libs/prisma-service/prisma
echo
echo "Conflict handling for createIntentNotice:"
rg -n -C3 'createIntentNotice|intentNoticeExists|P2002|ConflictException|HttpStatus\.CONFLICT' \
apps/ecosystem/src/ecosystem.service.ts \
apps/ecosystem/repositories/ecosystem.repository.tsRepository: credebl/platform
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for createIntentNotice method in repository
echo "=== Repository method createIntentNotice ==="
rg -n -A10 'createIntentNotice' apps/ecosystem/repositories/ecosystem.repository.ts
echo
echo "=== Error/exception handling for P2002 (unique constraint) ==="
rg -n 'P2002|ConflictException|UniqueConstraint|UNIQUE.*VIOLATION' apps/ecosystem/repositories/ecosystem.repository.ts
echo
echo "=== RPC exception handling in ecosystem service ==="
rg -n -B2 -A5 'catch.*Error\|Prisma.*Error' apps/ecosystem/src/ecosystem.service.ts | head -40Repository: credebl/platform
Length of output: 1104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check context around line 86 P2002 handling
echo "=== P2002 handling context around line 86 ==="
sed -n '75,100p' apps/ecosystem/repositories/ecosystem.repository.ts
echo
echo "=== Verify intent_notices table has NO unique constraint ==="
rg -A20 'model intent_notices' libs/prisma-service/prisma/schema.prisma
echo
echo "=== Compare to intent_templates which has proper partitioned unique indexes ==="
rg -A25 'model intent_templates' libs/prisma-service/prisma/schema.prismaRepository: credebl/platform
Length of output: 3172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check all migrations for intent_notices to confirm no unique constraint
echo "=== All migrations mentioning intent_notices ==="
rg -l 'intent_notices' libs/prisma-service/prisma/migrations/
echo
echo "=== Full content of intent_notices migration files ==="
for file in $(rg -l 'intent_notices' libs/prisma-service/prisma/migrations/); do
echo "=== File: $file ==="
cat "$file"
echo
doneRepository: credebl/platform
Length of output: 1594
Add partitioned unique constraints to prevent duplicate intent notices.
The code uses a read-then-create pattern (intentNoticeExists followed by createIntentNotice) without database-level uniqueness enforcement. Two concurrent requests can both pass the check and insert duplicates, since there is no unique constraint on (intentId, orgId).
The intent_notices table has only simple indexes on intentId and orgId, but lacks any composite uniqueness constraint. Meanwhile, the codebase already demonstrates the correct pattern in the intent_templates migration (20251223061418), which uses partitioned unique indexes to properly handle nullable orgId:
CREATE UNIQUE INDEX intent_templates_unique_org
ON intent_templates ("orgId", "intentId", "templateId")
WHERE "orgId" IS NOT NULL;
CREATE UNIQUE INDEX intent_templates_unique_no_org
ON intent_templates ("intentId", "templateId")
WHERE "orgId" IS NULL;Apply the same approach to intent_notices with a migration, and add P2002 error handling in the repository's createIntentNotice method to map database constraint violations to HTTP 409 Conflict (similar to the pattern in createEcosystemInvitation).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ecosystem/src/ecosystem.service.ts` around lines 1054 - 1062, Add
partitioned unique constraints for intent_notices and handle P2002 in the
repository: create a new migration that adds two unique indexes like
intent_templates (one WHERE "orgId" IS NOT NULL on ("orgId","intentId"), and one
WHERE "orgId" IS NULL on ("intentId")) to prevent duplicates, then update
ecosystemRepository.createIntentNotice to catch Prisma P2002/unique constraint
errors and rethrow an RpcException with HttpStatus.CONFLICT (mirror the
createEcosystemInvitation error-mapping pattern); keep the existing
intentNoticeExists check if desired but rely on DB constraint + repo-level P2002
handling to guarantee uniqueness.
Comment on lines
+6
to
+8
| const consentNoticeUrl = `${noticeUrl}?transactionId=${transactionId}`; | ||
|
|
||
| const response = await fetch(consentNoticeUrl); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current helper implementation and runtime baseline for native fetch/AbortSignal usage.
rg -n --type=ts -C2 'fetchConsentNotice|fetch\(|AbortSignal\.timeout|transactionId' apps/oid4vc-verification/src/oid4vc-verification.helper.ts
rg -n --type=json -C2 '"engines"|"\bnode\b"' package.jsonRepository: credebl/platform
Length of output: 857
🏁 Script executed:
#!/bin/bash
# Verify the context and usage of fetchConsentNotice
rg -n 'fetchConsentNotice' apps/oid4vc-verification/src/ --type ts -B2 -A2
# Check if there's try-catch or error handling around the fetch
rg -n 'fetchConsentNotice\|try\|catch' apps/oid4vc-verification/src/oid4vc-verification.service.ts --type ts -B3 -A3
# Verify if noticeUrl is validated before being passed to this helper
rg -n 'noticeUrl' apps/oid4vc-verification/src/oid4vc-verification.service.ts --type ts -B2 -A2
# Check for any existing fetch timeout patterns elsewhere in the codebase
rg -n 'AbortSignal|signal.*timeout|fetch.*signal' apps/ --type ts | head -20Repository: credebl/platform
Length of output: 1909
🏁 Script executed:
#!/bin/bash
# Check the full context of the fetchConsentNotice call and error handling
sed -n '410,430p' apps/oid4vc-verification/src/oid4vc-verification.service.ts
# Check if there's try-catch wrapping the entire session creation
sed -n '400,440p' apps/oid4vc-verification/src/oid4vc-verification.service.ts | cat -n
# Verify the complete fetchConsentNotice implementation including error handling
cat apps/oid4vc-verification/src/oid4vc-verification.helper.tsRepository: credebl/platform
Length of output: 3358
Build the consent URL safely and guard the outbound call with a timeout.
Line 6 manually concatenates ?transactionId=...; this produces malformed URLs when noticeUrl already contains query parameters (e.g., http://example.com/notice?foo=bar becomes http://example.com/notice?foo=bar?transactionId=xyz). Also, Line 8 has no timeout, so a slow or unresponsive endpoint can block this request indefinitely.
🔧 Proposed fix
- const consentNoticeUrl = `${noticeUrl}?transactionId=${transactionId}`;
-
- const response = await fetch(consentNoticeUrl);
+ const consentNoticeEndpoint = new URL(noticeUrl);
+ consentNoticeEndpoint.searchParams.set('transactionId', transactionId);
+
+ const response = await fetch(consentNoticeEndpoint.toString(), {
+ signal: AbortSignal.timeout(5000)
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const consentNoticeUrl = `${noticeUrl}?transactionId=${transactionId}`; | |
| const response = await fetch(consentNoticeUrl); | |
| const consentNoticeEndpoint = new URL(noticeUrl); | |
| consentNoticeEndpoint.searchParams.set('transactionId', transactionId); | |
| const response = await fetch(consentNoticeEndpoint.toString(), { | |
| signal: AbortSignal.timeout(5000) | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/oid4vc-verification/src/oid4vc-verification.helper.ts` around lines 6 -
8, Build the consent URL using a proper URL/URLSearchParams API instead of
string concatenation: create a new URL(noticeUrl) (or new URL(base) then set
pathname) and append transactionId with url.searchParams.set('transactionId',
transactionId) to avoid double '?' when noticeUrl already has query params; then
perform the outbound call using fetch guarded by an AbortController with a
timeout (create AbortController, pass signal to fetch, and clear the timeout on
completion) so the request cannot hang indefinitely; update the references to
consentNoticeUrl and the fetch call in the helper where they are used.
Comment on lines
+743
to
+757
| model intent_notices { | ||
| id String @id @default(uuid()) @db.Uuid | ||
| intentId String @db.Uuid | ||
| orgId String? @db.Uuid | ||
| noticeUrl String | ||
| createDateTime DateTime @default(now()) @db.Timestamptz(6) | ||
| createdBy String @db.Uuid | ||
| lastChangedDateTime DateTime @default(now()) @db.Timestamptz(6) | ||
| lastChangedBy String | ||
| intent intents @relation(fields: [intentId], references: [id]) | ||
| organisation organisation? @relation(fields: [orgId], references: [id]) | ||
|
|
||
| @@index([intentId]) | ||
| @@index([orgId]) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether runtime retrieval assumes single-row semantics.
rg -n --type=ts -C3 'get-intent-notice-by-intent-id|intentNotice|noticeUrl' apps/oid4vc-verification apps/ecosystem apps/api-gateway
rg -n --type=sql -C3 'intent_notices|UNIQUE|unique' libs/prisma-service/prisma/migrationsRepository: credebl/platform
Length of output: 50372
🏁 Script executed:
# Check the intent_templates pattern for handling nullable columns with uniqueness
rg -n "intent_templates_unique" libs/prisma-service/prisma/migrations --type=sql -A5 -B5
# Examine schema pattern for intent_notices
rg -n "intent_notices" libs/prisma-service/prisma/schema.prisma -B5 -A15
# Check if there are any database constraints already in the migration SQL
rg -n "intent_notices.*UNIQUE\|CONSTRAINT.*intent_notices" libs/prisma-service/prisma/migrations --type=sqlRepository: credebl/platform
Length of output: 4027
Refine uniqueness constraint for nullable orgId column.
The model allows duplicate notices per (intentId, orgId) pair, but the verification flow expects deterministic single-row lookup. Application-level validation prevents duplicates at insertion, but database-level enforcement should account for PostgreSQL's NULL behavior: a simple UNIQUE(intentId, orgId) allows multiple NULL orgId values.
Follow the pattern used in intent_templates (migration 20251223061418) with partial unique indexes:
♻️ Suggested SQL migration
CREATE UNIQUE INDEX "intent_notices_unique_with_org"
ON "intent_notices"("intentId", "orgId")
WHERE "orgId" IS NOT NULL;
CREATE UNIQUE INDEX "intent_notices_unique_no_org"
ON "intent_notices"("intentId")
WHERE "orgId" IS NULL;Alternatively, if application-level validation via intentNoticeExists() suffices, document this intentional choice and remove the indexes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/prisma-service/prisma/schema.prisma` around lines 743 - 757, The
intent_notices model allows duplicates because UNIQUE(intentId, orgId) won't
prevent multiple NULL orgId rows; add two partial unique indexes to enforce
DB-level uniqueness by creating a unique index on (intentId, orgId) WHERE orgId
IS NOT NULL and a unique index on intentId WHERE orgId IS NULL (use descriptive
index names like intent_notices_unique_with_org and
intent_notices_unique_no_org), or if you prefer to rely on the existing
application-level check (intentNoticeExists), explicitly document that choice
and remove the current indexes to avoid misleading constraints.
Signed-off-by: Tipu_Singh <tipu.singh@ayanworks.com>
|
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/ecosystem/src/ecosystem.helper.ts (2)
11-27:⚠️ Potential issue | 🔴 CriticalAdd request timeout/abort wiring for fetch.
Line 12 uses
fetch(noticeUrl)withoutAbortController.signal, so long-hanging requests are not bounded. Add a timeout +finallycleanup.⏱️ Suggested fix
export async function validateNoticeUrl(noticeUrl: string): Promise<void> { if (!noticeUrl || !noticeUrl.trim()) { throw new RpcException({ statusCode: HttpStatus.BAD_REQUEST, message: 'noticeUrl must not be empty.' }); } + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 5000); try { - const response = await fetch(noticeUrl); + const response = await fetch(noticeUrl, { signal: controller.signal }); if (!response.ok) { throw new RpcException({ statusCode: HttpStatus.BAD_REQUEST, message: `noticeUrl is not reachable (HTTP ${response.status}).` }); } } catch (err) { if (err instanceof RpcException) { throw err; } throw new RpcException({ statusCode: HttpStatus.BAD_REQUEST, message: `noticeUrl could not be resolved: ${err?.message ?? 'unreachable'}` }); + } finally { + clearTimeout(timeout); } }#!/bin/bash # Verify fetch timeout/abort wiring in validateNoticeUrl. rg -nP 'AbortController|setTimeout\\(|clearTimeout\\(|fetch\\(noticeUrl\\s*,\\s*\\{\\s*signal\\s*:' apps/ecosystem/src/ecosystem.helper.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ecosystem/src/ecosystem.helper.ts` around lines 11 - 27, The fetch(noticeUrl) call lacks AbortController wiring and a timeout, so requests can hang; update the validateNoticeUrl logic to create an AbortController, start a timer (e.g. configurable timeout) that calls controller.abort(), pass { signal: controller.signal } into fetch(noticeUrl, { signal }), and clear the timer in a finally block to avoid leaks; preserve existing error handling that throws RpcException with HttpStatus.BAD_REQUEST but include a clear message for aborted/timeouts (inspect err.name or err.message) and ensure the timer is cleared regardless of outcome.
12-12:⚠️ Potential issue | 🔴 CriticalBlock SSRF targets before outbound fetch.
Line 12 fetches a user-controlled URL directly. This can be used to probe internal/private hosts unless protocol/host/IP policy is enforced first.
🔒 Suggested hardening
export async function validateNoticeUrl(noticeUrl: string): Promise<void> { if (!noticeUrl || !noticeUrl.trim()) { throw new RpcException({ statusCode: HttpStatus.BAD_REQUEST, message: 'noticeUrl must not be empty.' }); } + const parsedUrl = new URL(noticeUrl); + if (!['http:', 'https:'].includes(parsedUrl.protocol)) { + throw new RpcException({ + statusCode: HttpStatus.BAD_REQUEST, + message: 'noticeUrl must use http/https.' + }); + } + // Enforce allowlist/private-network blocking policy before fetch. + // At minimum block localhost, loopback, link-local, RFC1918, and metadata hosts. try { const response = await fetch(noticeUrl);#!/bin/bash # Verify whether URL policy checks exist before fetch in validateNoticeUrl. rg -nP 'validateNoticeUrl|new URL\\(|protocol|hostname|localhost|127\\.0\\.0\\.1|::1|169\\.254\\.169\\.254|10\\.|192\\.168|172\\.(1[6-9]|2[0-9]|3[0-1])|fetch\\(noticeUrl' apps/ecosystem/src/ecosystem.helper.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ecosystem/src/ecosystem.helper.ts` at line 12, The fetch call uses a user-controlled noticeUrl and must be protected against SSRF: in the validateNoticeUrl (or wherever noticeUrl is accepted) enforce allowed protocols (only http/https), parse the URL with new URL(noticeUrl), resolve the hostname to an IP and block private/internal ranges (localhost, 127.0.0.0/8, ::1, 169.254.169.254, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, link-local, etc.), and reject hostnames that resolve to those addresses or to non-routable names before calling fetch(noticeUrl); additionally consider allowing an explicit allowlist of trusted hosts and failing fast with a clear error when validation fails.apps/oid4vc-verification/src/oid4vc-verification.service.ts (1)
420-429:⚠️ Potential issue | 🟠 MajorGuard the session ID before consent enrichment.
Line 423 still dereferences
createdSession.verificationSession.idbefore the promise.catch(...)can run. If that nested field is missing, this method will still throw after the verification session has already been created, so a client retry can create duplicates.🛡️ Suggested hardening
- if (intentNotice?.noticeUrl) { - createdSession.consentNoticeUrl = await fetchConsentNotice( - intentNotice.noticeUrl, - createdSession.verificationSession.id - ).catch((err) => { + const transactionId = createdSession?.verificationSession?.id; + if (intentNotice?.noticeUrl && transactionId) { + createdSession.consentNoticeUrl = await fetchConsentNotice( + intentNotice.noticeUrl, + transactionId + ).catch((err) => { this.logger.warn( `[createIntentBasedVerificationPresentation] consent notice enrichment failed: ${err?.message}` ); return null; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts` around lines 420 - 429, The code dereferences createdSession.verificationSession.id inline when calling fetchConsentNotice which can throw before the .catch runs; to fix, first read and validate the session id into a local variable (e.g., const sessionId = createdSession?.verificationSession?.id) and only call fetchConsentNotice if sessionId is defined, otherwise skip enrichment (set consentNoticeUrl to null). Update the call site that uses fetchConsentNotice (inside createIntentBasedVerificationPresentation) to use the guarded sessionId and keep the existing .catch to log warnings so enrichment failures remain non-fatal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/ecosystem/src/ecosystem.helper.ts`:
- Around line 11-27: The fetch(noticeUrl) call lacks AbortController wiring and
a timeout, so requests can hang; update the validateNoticeUrl logic to create an
AbortController, start a timer (e.g. configurable timeout) that calls
controller.abort(), pass { signal: controller.signal } into fetch(noticeUrl, {
signal }), and clear the timer in a finally block to avoid leaks; preserve
existing error handling that throws RpcException with HttpStatus.BAD_REQUEST but
include a clear message for aborted/timeouts (inspect err.name or err.message)
and ensure the timer is cleared regardless of outcome.
- Line 12: The fetch call uses a user-controlled noticeUrl and must be protected
against SSRF: in the validateNoticeUrl (or wherever noticeUrl is accepted)
enforce allowed protocols (only http/https), parse the URL with new
URL(noticeUrl), resolve the hostname to an IP and block private/internal ranges
(localhost, 127.0.0.0/8, ::1, 169.254.169.254, 10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16, link-local, etc.), and reject hostnames that resolve to those
addresses or to non-routable names before calling fetch(noticeUrl); additionally
consider allowing an explicit allowlist of trusted hosts and failing fast with a
clear error when validation fails.
In `@apps/oid4vc-verification/src/oid4vc-verification.service.ts`:
- Around line 420-429: The code dereferences
createdSession.verificationSession.id inline when calling fetchConsentNotice
which can throw before the .catch runs; to fix, first read and validate the
session id into a local variable (e.g., const sessionId =
createdSession?.verificationSession?.id) and only call fetchConsentNotice if
sessionId is defined, otherwise skip enrichment (set consentNoticeUrl to null).
Update the call site that uses fetchConsentNotice (inside
createIntentBasedVerificationPresentation) to use the guarded sessionId and keep
the existing .catch to log warnings so enrichment failures remain non-fatal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73512e1f-6123-40d9-8782-d1d7c9272ea0
📒 Files selected for processing (2)
apps/ecosystem/src/ecosystem.helper.tsapps/oid4vc-verification/src/oid4vc-verification.service.ts
shitrerohit
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores