fix(payments): harden payment flow with idempotency keys, amount validation, and magic byte file validation

- Add dedicated idempotencyKey column with unique constraint (userId, provider, idempotencyKey) to prevent duplicate payments at DB level
- Add @Min(1) @Max(100B) validators on amountVND in CreatePaymentDto to reject invalid amounts at API boundary
- Replace read-check-write callback handler with atomic updateIfStatus to eliminate race condition on concurrent callbacks
- Add magic byte verification in FileValidationPipe to validate file content matches declared MIME type server-side

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Ho Ngoc Hai
2026-04-08 06:18:26 +07:00
parent be0deddeed
commit 9583d1cb66
9 changed files with 148 additions and 43 deletions

View File

@@ -48,9 +48,22 @@ export class HandleCallbackHandler implements ICommandHandler<HandleCallbackComm
}); });
} }
// Find payment by orderId (which is the payment ID) // Atomically transition payment status to prevent race conditions
const payment = await this.paymentRepo.findById(result.orderId); // on concurrent callbacks. Only PENDING/PROCESSING payments can be updated.
if (!payment) { const targetStatus = result.isSuccess ? 'COMPLETED' : 'FAILED';
const updated = await this.paymentRepo.updateIfStatus(
result.orderId,
['PENDING', 'PROCESSING'],
{
status: targetStatus as any,
callbackData: result.rawData,
},
);
if (!updated) {
// Either payment doesn't exist or is already in a terminal state
const existing = await this.paymentRepo.findById(result.orderId);
if (!existing) {
this.logger.warn(`Payment not found for orderId=${result.orderId}`); this.logger.warn(`Payment not found for orderId=${result.orderId}`);
throw new NotFoundException({ throw new NotFoundException({
code: ErrorCode.NOT_FOUND, code: ErrorCode.NOT_FOUND,
@@ -58,40 +71,36 @@ export class HandleCallbackHandler implements ICommandHandler<HandleCallbackComm
}); });
} }
// Idempotency: if already completed/failed, return current state // Already processed — return idempotent response
if (payment.status === 'COMPLETED' || payment.status === 'FAILED' || payment.status === 'REFUNDED') {
this.logger.log( this.logger.log(
`Payment ${payment.id} already in terminal state: ${payment.status}`, `Payment ${existing.id} already in terminal state: ${existing.status}`,
); );
return { return {
paymentId: payment.id, paymentId: existing.id,
status: payment.status, status: existing.status,
isSuccess: payment.status === 'COMPLETED', isSuccess: existing.status === 'COMPLETED',
}; };
} }
// Update payment status // Reconstruct domain entity and publish events
if (result.isSuccess) { if (result.isSuccess) {
payment.markCompleted(result.rawData); updated.emitCompleted();
} else { } else {
payment.markFailed(result.rawData); updated.emitFailed();
} }
await this.paymentRepo.update(payment); const events = updated.clearDomainEvents();
// Publish domain events
const events = payment.clearDomainEvents();
for (const event of events) { for (const event of events) {
this.eventBus.publish(event); this.eventBus.publish(event);
} }
this.logger.log( this.logger.log(
`Payment ${payment.id} callback processed: status=${payment.status}`, `Payment ${updated.id} callback processed: status=${updated.status}`,
); );
return { return {
paymentId: payment.id, paymentId: updated.id,
status: payment.status, status: updated.status,
isSuccess: result.isSuccess, isSuccess: result.isSuccess,
}; };
} }

View File

@@ -115,6 +115,20 @@ export class PaymentEntity extends AggregateRoot<string> {
); );
} }
/** Emit completed event without modifying state (used when DB was already updated atomically). */
emitCompleted(): void {
this.addDomainEvent(
new PaymentCompletedEvent(this.id, this._userId, this._provider, this._amount.value),
);
}
/** Emit failed event without modifying state (used when DB was already updated atomically). */
emitFailed(): void {
this.addDomainEvent(
new PaymentFailedEvent(this.id, this._userId, this._provider),
);
}
markRefunded(): void { markRefunded(): void {
if (this._status !== 'COMPLETED') { if (this._status !== 'COMPLETED') {
throw new Error('Chỉ có thể hoàn tiền cho thanh toán đã hoàn tất'); throw new Error('Chỉ có thể hoàn tiền cho thanh toán đã hoàn tất');

View File

@@ -14,4 +14,6 @@ export interface IPaymentRepository {
}): Promise<{ items: PaymentEntity[]; total: number }>; }): Promise<{ items: PaymentEntity[]; total: number }>;
save(payment: PaymentEntity): Promise<void>; save(payment: PaymentEntity): Promise<void>;
update(payment: PaymentEntity): Promise<void>; update(payment: PaymentEntity): Promise<void>;
/** Atomically update payment status only if it is currently in one of the expected statuses. Returns null if no matching row. */
updateIfStatus(id: string, expectedStatuses: PaymentStatus[], data: { status: PaymentStatus; providerTxId?: string; callbackData?: unknown }): Promise<PaymentEntity | null>;
} }

View File

@@ -1,6 +1,6 @@
import { Injectable } from '@nestjs/common'; import { Injectable } from '@nestjs/common';
import { PrismaService } from '@modules/shared/infrastructure/prisma.service'; import { PrismaService } from '@modules/shared/infrastructure/prisma.service';
import { type Payment as PrismaPayment, type PaymentStatus } from '@prisma/client'; import { Prisma, type Payment as PrismaPayment, type PaymentStatus } from '@prisma/client';
import { type IPaymentRepository } from '../../domain/repositories/payment.repository'; import { type IPaymentRepository } from '../../domain/repositories/payment.repository';
import { PaymentEntity, type PaymentProps } from '../../domain/entities/payment.entity'; import { PaymentEntity, type PaymentProps } from '../../domain/entities/payment.entity';
import { Money } from '../../domain/value-objects/money.vo'; import { Money } from '../../domain/value-objects/money.vo';
@@ -23,12 +23,7 @@ export class PrismaPaymentRepository implements IPaymentRepository {
async findByIdempotencyKey(key: string): Promise<PaymentEntity | null> { async findByIdempotencyKey(key: string): Promise<PaymentEntity | null> {
const payment = await this.prisma.payment.findFirst({ const payment = await this.prisma.payment.findFirst({
where: { where: { idempotencyKey: key },
callbackData: {
path: ['idempotencyKey'],
equals: key,
},
},
}); });
return payment ? this.toDomain(payment) : null; return payment ? this.toDomain(payment) : null;
} }
@@ -70,6 +65,7 @@ export class PrismaPaymentRepository implements IPaymentRepository {
status: entity.status, status: entity.status,
providerTxId: entity.providerTxId, providerTxId: entity.providerTxId,
callbackData: entity.callbackData as any, callbackData: entity.callbackData as any,
idempotencyKey: entity.idempotencyKey,
}, },
}); });
} }
@@ -85,6 +81,33 @@ export class PrismaPaymentRepository implements IPaymentRepository {
}); });
} }
async updateIfStatus(
id: string,
expectedStatuses: PaymentStatus[],
data: { status: PaymentStatus; providerTxId?: string; callbackData?: unknown },
): Promise<PaymentEntity | null> {
try {
const updated = await this.prisma.payment.update({
where: {
id,
status: { in: expectedStatuses },
},
data: {
status: data.status,
...(data.providerTxId !== undefined ? { providerTxId: data.providerTxId } : {}),
...(data.callbackData !== undefined ? { callbackData: data.callbackData as any } : {}),
},
});
return this.toDomain(updated);
} catch (error) {
// P2025: Record not found (status didn't match or ID doesn't exist)
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') {
return null;
}
throw error;
}
}
private toDomain(raw: PrismaPayment): PaymentEntity { private toDomain(raw: PrismaPayment): PaymentEntity {
const amount = Money.create(raw.amountVND).unwrap(); const amount = Money.create(raw.amountVND).unwrap();
@@ -97,7 +120,7 @@ export class PrismaPaymentRepository implements IPaymentRepository {
status: raw.status, status: raw.status,
providerTxId: raw.providerTxId, providerTxId: raw.providerTxId,
callbackData: raw.callbackData, callbackData: raw.callbackData,
idempotencyKey: (raw.callbackData as any)?.idempotencyKey ?? null, idempotencyKey: raw.idempotencyKey ?? null,
}; };
return new PaymentEntity(raw.id, props, raw.createdAt, raw.updatedAt); return new PaymentEntity(raw.id, props, raw.createdAt, raw.updatedAt);

View File

@@ -62,7 +62,7 @@ export class PaymentsController {
user.sub, user.sub,
dto.provider, dto.provider,
dto.type, dto.type,
dto.amountVND, BigInt(dto.amountVND),
dto.description, dto.description,
dto.returnUrl, dto.returnUrl,
ip || '127.0.0.1', ip || '127.0.0.1',

View File

@@ -1,9 +1,12 @@
import { import {
IsEnum, IsEnum,
IsNotEmpty, IsNotEmpty,
IsNumber,
IsOptional, IsOptional,
IsString, IsString,
IsUrl, IsUrl,
Max,
Min,
MinLength, MinLength,
} from 'class-validator'; } from 'class-validator';
import { Transform } from 'class-transformer'; import { Transform } from 'class-transformer';
@@ -19,10 +22,17 @@ export class CreatePaymentDto {
@IsEnum(PaymentType) @IsEnum(PaymentType)
type!: PaymentType; type!: PaymentType;
@ApiProperty({ type: Number, description: 'Amount in VND', example: 500000 }) @ApiProperty({ type: Number, description: 'Amount in VND (1 100,000,000,000)', example: 500000 })
@IsNotEmpty() @IsNotEmpty()
@Transform(({ value }) => BigInt(value)) @IsNumber()
amountVND!: bigint; @Min(1, { message: 'Số tiền phải lớn hơn 0' })
@Max(100_000_000_000, { message: 'Số tiền vượt quá giới hạn cho phép (100 tỷ VND)' })
@Transform(({ value }) => {
const num = Number(value);
if (!Number.isFinite(num) || !Number.isInteger(num)) return value;
return num;
}, { toClassOnly: true })
amountVND!: number;
@ApiProperty({ description: 'Payment description', example: 'Listing fee' }) @ApiProperty({ description: 'Payment description', example: 'Listing fee' })
@IsString() @IsString()

View File

@@ -14,6 +14,8 @@ export interface FileValidationOptions {
maxSizeBytes?: number; maxSizeBytes?: number;
/** Allowed MIME types. Default: common image types + PDF */ /** Allowed MIME types. Default: common image types + PDF */
allowedMimeTypes?: string[]; allowedMimeTypes?: string[];
/** Whether to verify file content matches declared MIME type via magic bytes. Default: true */
verifyMagicBytes?: boolean;
} }
const DEFAULT_MAX_SIZE = 5 * 1024 * 1024; // 5 MB const DEFAULT_MAX_SIZE = 5 * 1024 * 1024; // 5 MB
@@ -25,18 +27,33 @@ const DEFAULT_ALLOWED_MIMES = [
'application/pdf', 'application/pdf',
]; ];
/** Magic byte signatures for supported file types. */
const MAGIC_BYTES: Record<string, { offset: number; bytes: number[] }[]> = {
'image/jpeg': [{ offset: 0, bytes: [0xFF, 0xD8, 0xFF] }],
'image/png': [{ offset: 0, bytes: [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A] }],
'image/webp': [{ offset: 0, bytes: [0x52, 0x49, 0x46, 0x46] }], // "RIFF" header
'image/gif': [
{ offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x37, 0x61] }, // GIF87a
{ offset: 0, bytes: [0x47, 0x49, 0x46, 0x38, 0x39, 0x61] }, // GIF89a
],
'application/pdf': [{ offset: 0, bytes: [0x25, 0x50, 0x44, 0x46] }], // %PDF
'video/mp4': [{ offset: 4, bytes: [0x66, 0x74, 0x79, 0x70] }], // "ftyp" at offset 4
};
/** /**
* Validates uploaded files for size and MIME type to prevent * Validates uploaded files for size, MIME type, and file content
* malicious file uploads and resource exhaustion. * (magic bytes) to prevent malicious file uploads and resource exhaustion.
*/ */
@Injectable() @Injectable()
export class FileValidationPipe implements PipeTransform { export class FileValidationPipe implements PipeTransform {
private readonly maxSize: number; private readonly maxSize: number;
private readonly allowedMimes: string[]; private readonly allowedMimes: string[];
private readonly verifyMagicBytes: boolean;
constructor(options?: FileValidationOptions) { constructor(options?: FileValidationOptions) {
this.maxSize = options?.maxSizeBytes ?? DEFAULT_MAX_SIZE; this.maxSize = options?.maxSizeBytes ?? DEFAULT_MAX_SIZE;
this.allowedMimes = options?.allowedMimeTypes ?? DEFAULT_ALLOWED_MIMES; this.allowedMimes = options?.allowedMimeTypes ?? DEFAULT_ALLOWED_MIMES;
this.verifyMagicBytes = options?.verifyMagicBytes ?? true;
} }
transform(file: UploadedFile): UploadedFile { transform(file: UploadedFile): UploadedFile {
@@ -56,6 +73,29 @@ export class FileValidationPipe implements PipeTransform {
); );
} }
if (this.verifyMagicBytes) {
this.validateMagicBytes(file);
}
return file; return file;
} }
private validateMagicBytes(file: UploadedFile): void {
const signatures = MAGIC_BYTES[file.mimetype];
if (!signatures) {
// No known signature for this type — skip magic byte check
return;
}
const matches = signatures.some((sig) => {
if (file.buffer.length < sig.offset + sig.bytes.length) return false;
return sig.bytes.every((byte, i) => file.buffer[sig.offset + i] === byte);
});
if (!matches) {
throw new BadRequestException(
`File content does not match declared type '${file.mimetype}'. The file may be corrupted or mislabeled.`,
);
}
}
} }

View File

@@ -0,0 +1,5 @@
-- AlterTable
ALTER TABLE "Payment" ADD COLUMN "idempotencyKey" TEXT;
-- CreateIndex
CREATE UNIQUE INDEX "Payment_idempotency_unique" ON "Payment"("userId", "provider", "idempotencyKey");

View File

@@ -370,9 +370,11 @@ model Payment {
status PaymentStatus @default(PENDING) status PaymentStatus @default(PENDING)
providerTxId String? providerTxId String?
callbackData Json? callbackData Json?
idempotencyKey String?
createdAt DateTime @default(now()) createdAt DateTime @default(now())
updatedAt DateTime @updatedAt updatedAt DateTime @updatedAt
@@unique([userId, provider, idempotencyKey], name: "Payment_idempotency_unique")
@@index([userId]) @@index([userId])
@@index([transactionId]) @@index([transactionId])
@@index([status]) @@index([status])