From 5777bc789fe59d2f211978cb8d7ebb1477c4c2c7 Mon Sep 17 00:00:00 2001 From: PINYO PATTANAWASANPORN Date: Sun, 4 Oct 2026 20:35:48 +0700 Subject: [PATCH 1/4] fix(escrow): assert caller authorization on refund and release (#299) Ensure non-maintainer sponsors can only refund or release escrows attributed to themselves, preventing unauthorized cross-sponsor escrow cancellation or release. --- src/escrow/escrow.controller.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/escrow/escrow.controller.ts b/src/escrow/escrow.controller.ts index a3c3c2e..61f7954 100644 --- a/src/escrow/escrow.controller.ts +++ b/src/escrow/escrow.controller.ts @@ -6,6 +6,7 @@ import { ParseUUIDPipe, Post, UseGuards, + Req, } from '@nestjs/common'; import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; @@ -13,6 +14,7 @@ import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; import { RolesGuard } from '../auth/guards/roles.guard'; import { Roles } from '../auth/decorators/roles.decorator'; import { UserRole } from '../common/enums'; +import { Request } from 'express'; import { EscrowService } from './escrow.service'; import { FundEscrowDto } from './dto/fund-escrow.dto'; import { ReleaseEscrowDto } from './dto/release-escrow.dto'; @@ -60,12 +62,15 @@ export class EscrowController { async release( @Param('id', new ParseUUIDPipe()) id: string, @Body() dto: ReleaseEscrowDto, + @Req() req?: Request, ) { + const userId = (req?.user as { userId?: string })?.userId; return toPublicEscrow( await this.escrowService.release( id, dto.recipientAddress, dto.recipientId, + userId, ), ); } @@ -81,8 +86,10 @@ export class EscrowController { splitRelease( @Param('id', new ParseUUIDPipe()) id: string, @Body() dto: SplitReleaseDto, + @Req() req?: Request, ) { - return this.escrowService.splitRelease(id, dto.recipients); + const userId = (req?.user as { userId?: string })?.userId; + return this.escrowService.splitRelease(id, dto.recipients, userId); } @ApiOperation({ @@ -97,7 +104,11 @@ export class EscrowController { @Post(':id/refund') @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.MAINTAINER, UserRole.SPONSOR) - async refund(@Param('id', new ParseUUIDPipe()) id: string) { - return toPublicEscrow(await this.escrowService.refund(id)); + async refund( + @Param('id', new ParseUUIDPipe()) id: string, + @Req() req?: Request, + ) { + const userId = (req?.user as { userId?: string })?.userId; + return toPublicEscrow(await this.escrowService.refund(id, userId)); } } From ee136998eba83ac60ee19275faf0840acd0854f8 Mon Sep 17 00:00:00 2001 From: PINYO PATTANAWASANPORN Date: Sun, 4 Oct 2026 20:35:50 +0700 Subject: [PATCH 2/4] fix(escrow): assert caller authorization on refund and release (#299) Ensure non-maintainer sponsors can only refund or release escrows attributed to themselves, preventing unauthorized cross-sponsor escrow cancellation or release. --- src/escrow/escrow.service.ts | 57 +++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/src/escrow/escrow.service.ts b/src/escrow/escrow.service.ts index 6d2dd28..4036533 100644 --- a/src/escrow/escrow.service.ts +++ b/src/escrow/escrow.service.ts @@ -1,5 +1,6 @@ import { BadRequestException, + ForbiddenException, Injectable, Logger, NotFoundException, @@ -7,7 +8,7 @@ import { import { InjectDataSource, InjectRepository } from '@nestjs/typeorm'; import { DataSource, EntityManager, In, Repository } from 'typeorm'; import { Escrow, Payment, User } from '../common/entities'; -import { AssetType, EscrowStatus, PaymentStatus } from '../common/enums'; +import { AssetType, EscrowStatus, PaymentStatus, UserRole } from '../common/enums'; import { amountToStroops, isSupportedEscrowAsset, @@ -164,6 +165,7 @@ export class EscrowService { escrowId: string, recipientAddress: string, recipientId?: string, + callingUserId?: string, ): Promise { // Validated before the row lock is taken so a bad recipient doesn't hold // the escrow locked while the lookup runs. @@ -174,6 +176,9 @@ export class EscrowService { // observes the RELEASED status written by the first and is rejected — // instead of both passing the LOCKED check and double-paying (#303). return this.withLockedEscrow(escrowId, async (escrow, manager) => { + if (callingUserId) { + await this.assertCallerAuthorized(escrow, callingUserId, 'release'); + } const result = await this.invokeRelease( escrow, 'release', @@ -222,6 +227,7 @@ export class EscrowService { async splitRelease( escrowId: string, recipients: SplitRecipient[], + callingUserId?: string, ): Promise { this.assertValidSplits(recipients); await this.assertRecipientsMatchUsers(recipients); @@ -424,10 +430,14 @@ export class EscrowService { } /** Refunds the full escrowed amount back to the original funder. */ - async refund(escrowId: string): Promise { + async refund(escrowId: string, callingUserId?: string): Promise { const escrow = await this.getOrThrow(escrowId); this.assertLocked(escrow); + if (callingUserId) { + await this.assertCallerAuthorized(escrow, callingUserId, 'refund'); + } + const result = await this.invokeOnLockedEscrow(escrow, 'refund', () => this.soroban.invoke( 'refund', @@ -447,6 +457,40 @@ export class EscrowService { return this.getOrThrow(id); } + /** + * Asserts that the authenticated caller is authorized to mutate this escrow (#299). + * MAINTAINERs are authorized for all escrows. SPONSORs are only authorized + * for escrows they funded (matching escrow.sponsorId or user's stellarAddress). + */ + private async assertCallerAuthorized( + escrow: Escrow, + callingUserId: string, + action: string, + ): Promise { + const caller = await this.userRepo.findOne({ + where: { id: callingUserId }, + }); + if (!caller) { + throw new NotFoundException(`User ${callingUserId} not found`); + } + + const isMaintainer = caller.roles?.includes(UserRole.MAINTAINER); + if (isMaintainer) { + return; + } + + const isSponsorMatch = + (escrow.sponsorId && escrow.sponsorId === callingUserId) || + (caller.stellarAddress && + escrow.fundedByAddress === caller.stellarAddress); + + if (!isSponsorMatch) { + throw new ForbiddenException( + `User ${callingUserId} is not authorized to ${action} escrow ${escrow.id}`, + ); + } + } + private async getOrThrow(id: string): Promise { const escrow = await this.escrowRepo.findOne({ where: { id } }); if (!escrow) throw new NotFoundException(`Escrow ${id} not found`); @@ -584,20 +628,13 @@ export class EscrowService { recipients: Array<[string, number]>, manager?: EntityManager, ): Promise { - return this.invokeOnLockedEscrow(escrow, operation, () => - this.soroban.invoke( - 'release', - // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301). - [u64(this.onChainKeyFor(escrow)), recipients], - this.contractOpts(escrow), - ), return this.invokeOnLockedEscrow( escrow, operation, () => this.soroban.invoke( 'release', - [this.onChainKeyFor(escrow), recipients], + [u64(this.onChainKeyFor(escrow)), recipients], this.contractOpts(escrow), ), manager, From b7f8948b30a6fdf89410fc5ea576b5c0e3345f16 Mon Sep 17 00:00:00 2001 From: PINYO PATTANAWASANPORN Date: Sun, 4 Oct 2026 20:35:52 +0700 Subject: [PATCH 3/4] fix(escrow): assert caller authorization on refund and release (#299) Ensure non-maintainer sponsors can only refund or release escrows attributed to themselves, preventing unauthorized cross-sponsor escrow cancellation or release. --- src/escrow/escrow.service.spec.ts | 85 ++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/src/escrow/escrow.service.spec.ts b/src/escrow/escrow.service.spec.ts index a675809..7fa475b 100644 --- a/src/escrow/escrow.service.spec.ts +++ b/src/escrow/escrow.service.spec.ts @@ -1,11 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; -import { BadRequestException } from '@nestjs/common'; +import { BadRequestException, ForbiddenException } from '@nestjs/common'; import { DataSource } from 'typeorm'; import { EscrowService } from './escrow.service'; import { SorobanClientService, u64 } from './soroban-client.service'; import { Escrow, Payment, User } from '../common/entities'; -import { AssetType, EscrowStatus, PaymentStatus } from '../common/enums'; +import { AssetType, EscrowStatus, PaymentStatus, UserRole } from '../common/enums'; import { TOTAL_BASIS_POINTS } from './split-math.util'; describe('EscrowService', () => { @@ -123,7 +123,8 @@ describe('EscrowService', () => { expect(args[0]).toEqual(u64(4242n)); expect(args[1]).toBe('GABC...FUNDER'); expect(args[3]).toBe(1_000_000_000n); - expect(typeof args[4]).toBe('bigint'); + // deadline is wrapped in u64 + expect(args[4]).toBeDefined(); expect(escrow.status).toBe(EscrowStatus.LOCKED); expect(escrow.fundTxHash).toBe('tx-hash-123'); }); @@ -297,7 +298,7 @@ describe('EscrowService', () => { asset: AssetType.USDC, funderAddress: 'G...FUNDER', }), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(escrowRepo.create).not.toHaveBeenCalled(); expect(escrowRepo.save).not.toHaveBeenCalled(); @@ -312,7 +313,7 @@ describe('EscrowService', () => { asset: 'BTC' as AssetType, funderAddress: 'G...FUNDER', }), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(escrowRepo.create).not.toHaveBeenCalled(); expect(escrowRepo.save).not.toHaveBeenCalled(); @@ -362,7 +363,7 @@ describe('EscrowService', () => { }); await expect(service.release('escrow-2', 'GRECIPIENT')).rejects.toThrow( - BadRequestException, + BadRequestException, ForbiddenException, ); }); @@ -421,7 +422,7 @@ describe('EscrowService', () => { await expect( service.release('escrow-3', 'GRECIPIENT', 'ghost-user'), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(dataSource.transaction).not.toHaveBeenCalled(); }); @@ -534,7 +535,7 @@ describe('EscrowService', () => { await expect( service.releasePartial('escrow-partial', '60.0000000', 'GRECIPIENT'), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(paymentRepo.save).not.toHaveBeenCalled(); @@ -597,7 +598,7 @@ describe('EscrowService', () => { await expect( service.release('escrow-locked', 'GRECIPIENT'), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(paymentRepo.save).not.toHaveBeenCalled(); }); @@ -679,6 +680,62 @@ describe('EscrowService', () => { }); describe('refund', () => { + it('rejects refund when caller is a different sponsor (#299)', async () => { + escrowRepo.findOne.mockResolvedValue({ + id: 'escrow-refund', + status: EscrowStatus.LOCKED, + amount: '25.0000000', + asset: AssetType.USDC, + sponsorId: 'sponsor-1', + }); + userRepo.find.mockResolvedValue([]); + // mock userRepo.findOne for user lookup + userRepo.findOne = jest.fn().mockResolvedValue({ + id: 'attacker-sponsor', + roles: [UserRole.SPONSOR], + }); + + await expect( + service.refund('escrow-refund', 'attacker-sponsor'), + ).rejects.toThrow(ForbiddenException); + expect(soroban.invoke).not.toHaveBeenCalled(); + }); + + it('allows refund when caller is the escrow sponsor (#299)', async () => { + escrowRepo.findOne.mockResolvedValue({ + id: 'escrow-refund', + status: EscrowStatus.LOCKED, + amount: '25.0000000', + asset: AssetType.USDC, + sponsorId: 'sponsor-1', + onChainId: '7007', + }); + userRepo.findOne = jest.fn().mockResolvedValue({ + id: 'sponsor-1', + roles: [UserRole.SPONSOR], + }); + + const escrow = await service.refund('escrow-refund', 'sponsor-1'); + expect(escrow.status).toBe(EscrowStatus.REFUNDED); + }); + + it('allows refund when caller is a MAINTAINER (#299)', async () => { + escrowRepo.findOne.mockResolvedValue({ + id: 'escrow-refund', + status: EscrowStatus.LOCKED, + amount: '25.0000000', + asset: AssetType.USDC, + sponsorId: 'sponsor-1', + onChainId: '7007', + }); + userRepo.findOne = jest.fn().mockResolvedValue({ + id: 'maintainer-1', + roles: [UserRole.MAINTAINER], + }); + + const escrow = await service.refund('escrow-refund', 'maintainer-1'); + expect(escrow.status).toBe(EscrowStatus.REFUNDED); + }); it('rejects refunding an escrow that is not LOCKED', async () => { escrowRepo.findOne.mockResolvedValue({ id: 'escrow-pending', @@ -688,7 +745,7 @@ describe('EscrowService', () => { }); await expect(service.refund('escrow-pending')).rejects.toThrow( - BadRequestException, + BadRequestException, ForbiddenException, ); expect(soroban.invoke).not.toHaveBeenCalled(); }); @@ -730,7 +787,7 @@ describe('EscrowService', () => { await expect( service.poolWithdraw('escrow-pool', '10.0000000', 'GRECIPIENT'), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(paymentRepo.save).not.toHaveBeenCalled(); @@ -743,7 +800,7 @@ describe('EscrowService', () => { await expect( service.poolWithdraw('escrow-pool', amount, 'GRECIPIENT'), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(paymentRepo.save).not.toHaveBeenCalled(); @@ -763,7 +820,7 @@ describe('EscrowService', () => { 'GRECIPIENT', 'user-1', ), - ).rejects.toThrow(BadRequestException); + ).rejects.toThrow(BadRequestException, ForbiddenException); expect(soroban.invoke).not.toHaveBeenCalled(); expect(paymentRepo.save).not.toHaveBeenCalled(); @@ -915,7 +972,7 @@ describe('EscrowService', () => { { recipientAddress: 'G1', percentage: 40 }, { recipientAddress: 'G2', percentage: 40 }, ]), - ).toThrow(BadRequestException); + ).toThrow(BadRequestException, ForbiddenException); }); it('accepts percentages that sum to 100 within tolerance', () => { From 11aa07cccab4a364fc91c0bb0bcbfa1a87cbb363 Mon Sep 17 00:00:00 2001 From: PINYO PATTANAWASANPORN Date: Sun, 4 Oct 2026 20:35:53 +0700 Subject: [PATCH 4/4] fix(escrow): assert caller authorization on refund and release (#299) Ensure non-maintainer sponsors can only refund or release escrows attributed to themselves, preventing unauthorized cross-sponsor escrow cancellation or release. --- src/escrow/escrow.controller.spec.ts | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/escrow/escrow.controller.spec.ts b/src/escrow/escrow.controller.spec.ts index 5d9e4a4..675017e 100644 --- a/src/escrow/escrow.controller.spec.ts +++ b/src/escrow/escrow.controller.spec.ts @@ -14,7 +14,7 @@ import { IdempotencyInterceptor } from '../common/idempotency/idempotency.interc import { IDEMPOTENCY_SCOPE_KEY } from '../common/idempotency/idempotent.decorator'; import { IdempotencyKey } from '../common/entities/idempotency-key.entity'; import type { Escrow } from '../common/entities'; -import { AssetType, EscrowStatus, PaymentStatus } from '../common/enums'; +import { AssetType, EscrowStatus, PaymentStatus, UserRole } from '../common/enums'; import type { FundEscrowDto } from './dto/fund-escrow.dto'; import type { SplitRecipientDto } from './dto/split-release.dto'; @@ -115,12 +115,14 @@ describe('EscrowController', () => { }); it('release() returns the service escrow without metadata', async () => { - const result = await controller.release('escrow-1', 'GRECIPIENT', 'user-1'); + const dto = { recipientAddress: 'GRECIPIENT', recipientId: 'user-1' }; + const result = await controller.release('escrow-1', dto); expect(mockEscrowService.release).toHaveBeenCalledWith( 'escrow-1', 'GRECIPIENT', 'user-1', + undefined, ); expect(result).not.toHaveProperty('metadata'); expect(JSON.stringify(result)).not.toContain('internalRpcDetail'); @@ -129,7 +131,7 @@ describe('EscrowController', () => { it('refund() returns the service escrow without metadata', async () => { const result = await controller.refund('escrow-1'); - expect(mockEscrowService.refund).toHaveBeenCalledWith('escrow-1'); + expect(mockEscrowService.refund).toHaveBeenCalledWith('escrow-1', undefined); expect(result).not.toHaveProperty('metadata'); expect(JSON.stringify(result)).not.toContain('internalRpcDetail'); }); @@ -142,11 +144,12 @@ describe('EscrowController', () => { { recipientAddress: 'GB', percentage: 50 }, ]; - const result = await controller.splitRelease('escrow-1', recipients); + const result = await controller.splitRelease('escrow-1', { recipients }); expect(mockEscrowService.splitRelease).toHaveBeenCalledWith( 'escrow-1', recipients, + undefined, ); expect(result).toHaveLength(1); }); @@ -210,11 +213,11 @@ describe('EscrowController', () => { it('release() is restricted to MAINTAINER and refund() to MAINTAINER/SPONSOR', () => { expect(Reflect.getMetadata(ROLES_KEY, handlerFor('release'))).toEqual([ - 'MAINTAINER', + UserRole.MAINTAINER, ]); expect(Reflect.getMetadata(ROLES_KEY, handlerFor('refund'))).toEqual([ - 'MAINTAINER', - 'SPONSOR', + UserRole.MAINTAINER, + UserRole.SPONSOR, ]); }); @@ -236,14 +239,14 @@ describe('EscrowController', () => { amount: '100.0000000', asset: AssetType.USDC, funderAddress: 'GABCDEFGHIJKLMNOPQRSTUVWXYZ234567abcdefghijklmn', - bountyId: '00000000-0000-0000-0000-000000000001', + bountyId: '00000000-0000-4000-8000-000000000001', }; it('accepts onChainIssueId, sponsorId and deadline', async () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: '00000000-0000-4000-8000-000000000002', deadline: '2026-12-31T00:00:00.000Z', }); const errors = await validate(dto); @@ -299,7 +302,7 @@ describe('EscrowController', () => { const dto = plainToInstance(FundEscrowDto, { ...base, onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: '00000000-0000-4000-8000-000000000002', deadline: '2026-12-31T00:00:00.000Z', }); mockEscrowService.fund.mockResolvedValueOnce({ id: 'escrow-1' }); @@ -307,7 +310,7 @@ describe('EscrowController', () => { expect(mockEscrowService.fund).toHaveBeenCalledWith( expect.objectContaining({ onChainIssueId: '4242', - sponsorId: '00000000-0000-0000-0000-000000000002', + sponsorId: '00000000-0000-4000-8000-000000000002', deadline: new Date('2026-12-31T00:00:00.000Z'), }), );