diff --git a/docs/fixes/payment-hardening.md b/docs/fixes/payment-hardening.md new file mode 100644 index 00000000..78a358e9 --- /dev/null +++ b/docs/fixes/payment-hardening.md @@ -0,0 +1,61 @@ +# Payment hardening fixes + +Summary of four issues fixed on branch `fix/payment-index-validation-tsconfig-strictness`. + +## 1. Missing indexes on `payments.merchantId` (bug, tech-debt) + +`src/payments/entities/payment.entity.ts` had no `@Index` decorators. Since +`merchantId` is the WHERE clause for `PaymentsService.findAll()`, +`findOne()`, and `getStats()` (`src/payments/payments.service.ts`), and is +also the AML velocity-check join key (`AmlService.checkAndFlag`), every +merchant-scoped query was a sequential scan. + +**Fix:** added `@Index()` on `merchantId`, plus composite +`@Index(['merchantId', 'status'])` and `@Index(['merchantId', 'createdAt'])` +on the `Payment` entity to match actual query patterns (status filtering, +date-range/list queries). A migration will need to be generated/run against +the target database to materialize these indexes. + +## 2. Unbounded `amountUsd` and `metadata` on payment creation (bug, security) + +`src/payments/dto/create-payment.dto.ts` validated `amountUsd` with only +`@IsNumber()`/`@IsPositive()` (no ceiling) and `metadata` with only +`@IsObject()` (no size limit), storing directly into a `jsonb` column. + +**Fix:** +- Added `@Max(1_000_000)` to `amountUsd` in both `CreatePaymentDto` and + `BatchPaymentItemDto`. +- Added a new reusable custom validator, + `src/common/decorators/max-json-size.decorator.ts` (`@MaxJsonSize`), which + rejects a field whose serialized JSON size exceeds a configured byte + limit. Applied `@MaxJsonSize(4096)` to `metadata` on both the single and + batch payment DTOs. + +## 3. `expiryMinutes` had no bounds on single-payment creation (bug) + +`CreatePaymentDto.expiryMinutes` only had `@IsNumber()`, unlike +`BatchPaymentItemDto.expiryMinutes` which already had `@IsPositive()`. This +allowed `expiryMinutes: 0`/negative (payment expired at creation) or an +arbitrarily large value (never-expiring payment). + +**Fix:** added `@IsPositive()` and `@Max(1440)` (24h cap) to +`CreatePaymentDto.expiryMinutes`, aligning it with the batch DTO (which also +got the same `@Max(1440)` cap for consistency). + +## 4. `strictNullChecks` / `noImplicitAny` disabled (tech-debt) + +`tsconfig.json` had both flags set to `false`, disabling two of +TypeScript's most important safety nets for a codebase full of +`nullable: true` entity columns and optional DTO fields. + +**Fix:** flipped `strictNullChecks` and `noImplicitAny` to `true` in +`tsconfig.json`. + +**Follow-up required:** enabling these flags project-wide will surface +existing null/undefined and implicit-`any` type errors across services +(e.g. `payment.customerWalletAddress`, `merchant.customFeeRate`, +`catch (err) { err.message }` patterns). Those call sites will need to be +fixed incrementally (nullish checks, explicit typing on catch clauses, +etc.) before this change can compile cleanly in CI. This commit intentionally +does not attempt that codebase-wide migration — it only flips the compiler +flags as requested, per the issue's own suggested incremental approach. diff --git a/src/common/decorators/max-json-size.decorator.ts b/src/common/decorators/max-json-size.decorator.ts new file mode 100644 index 00000000..b03a5406 --- /dev/null +++ b/src/common/decorators/max-json-size.decorator.ts @@ -0,0 +1,34 @@ +import { registerDecorator, ValidationOptions, ValidationArguments } from 'class-validator'; + +/** + * Validates that the JSON-serialized size of a property does not exceed + * maxBytes. Used to cap arbitrary/unbounded object fields (e.g. metadata) + * stored directly into jsonb columns, preventing oversized payloads. + */ +export function MaxJsonSize(maxBytes: number, validationOptions?: ValidationOptions) { + return (object: object, propertyName: string) => { + registerDecorator({ + name: 'maxJsonSize', + target: object.constructor, + propertyName, + constraints: [maxBytes], + options: validationOptions, + validator: { + validate(value: unknown, args: ValidationArguments) { + if (value === undefined || value === null) return true; + const [limit] = args.constraints; + try { + const size = Buffer.byteLength(JSON.stringify(value), 'utf8'); + return size <= limit; + } catch { + return false; + } + }, + defaultMessage(args: ValidationArguments) { + const [limit] = args.constraints; + return `${args.property} must not exceed ${limit} bytes when serialized`; + }, + }, + }); + }; +} diff --git a/src/payments/dto/batch-create-payment.dto.ts b/src/payments/dto/batch-create-payment.dto.ts index fc58f8af..f855b89b 100644 --- a/src/payments/dto/batch-create-payment.dto.ts +++ b/src/payments/dto/batch-create-payment.dto.ts @@ -11,14 +11,20 @@ import { ValidateNested, IsNotEmpty, MinLength, + Max, } from 'class-validator'; import { Type, Transform } from 'class-transformer'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { MaxJsonSize } from '../../common/decorators/max-json-size.decorator'; + +const MAX_AMOUNT_USD = 1_000_000; +const MAX_METADATA_BYTES = 4096; export class BatchPaymentItemDto { @ApiProperty({ example: 50.0, description: 'Amount in USD — must be greater than 0' }) @IsNumber() @IsPositive() + @Max(MAX_AMOUNT_USD) amountUsd: number; @ApiProperty({ example: 'Order #123', description: 'Non-empty memo for this payment' }) @@ -37,12 +43,16 @@ export class BatchPaymentItemDto { @ApiPropertyOptional() @IsOptional() @IsObject() + @MaxJsonSize(MAX_METADATA_BYTES, { + message: `metadata must not exceed ${MAX_METADATA_BYTES} bytes when serialized`, + }) metadata?: Record; - @ApiPropertyOptional({ example: 30, description: 'Expiry in minutes (default 30)' }) + @ApiPropertyOptional({ example: 30, description: 'Expiry in minutes (default 30, max 1440)' }) @IsOptional() @IsNumber() @IsPositive() + @Max(1440) expiryMinutes?: number; } diff --git a/src/payments/dto/create-payment.dto.ts b/src/payments/dto/create-payment.dto.ts index 1b1c8c12..17261903 100644 --- a/src/payments/dto/create-payment.dto.ts +++ b/src/payments/dto/create-payment.dto.ts @@ -1,11 +1,16 @@ -import { IsNumber, IsPositive, IsString, IsOptional, IsEmail, IsObject } from 'class-validator'; +import { IsNumber, IsPositive, IsString, IsOptional, IsEmail, IsObject, Max } from 'class-validator'; import { Transform } from 'class-transformer'; import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { MaxJsonSize } from '../../common/decorators/max-json-size.decorator'; + +const MAX_AMOUNT_USD = 1_000_000; +const MAX_METADATA_BYTES = 4096; export class CreatePaymentDto { @ApiProperty({ example: 50.0 }) @IsNumber() @IsPositive() + @Max(MAX_AMOUNT_USD) amountUsd: number; @ApiPropertyOptional({ example: 'Payment for order #123' }) @@ -23,10 +28,15 @@ export class CreatePaymentDto { @ApiPropertyOptional() @IsOptional() @IsObject() + @MaxJsonSize(MAX_METADATA_BYTES, { + message: `metadata must not exceed ${MAX_METADATA_BYTES} bytes when serialized`, + }) metadata?: Record; - @ApiPropertyOptional({ example: 30, description: 'Expiry in minutes (default 30)' }) + @ApiPropertyOptional({ example: 30, description: 'Expiry in minutes (default 30, max 1440)' }) @IsOptional() @IsNumber() + @IsPositive() + @Max(1440) expiryMinutes?: number; } diff --git a/src/payments/entities/payment.entity.ts b/src/payments/entities/payment.entity.ts index 91444f33..9efaee0e 100644 --- a/src/payments/entities/payment.entity.ts +++ b/src/payments/entities/payment.entity.ts @@ -7,6 +7,7 @@ import { DeleteDateColumn, ManyToOne, JoinColumn, + Index, } from 'typeorm'; import { Merchant } from '../../merchants/entities/merchant.entity'; import { Settlement } from '../../settlements/entities/settlement.entity'; @@ -33,6 +34,8 @@ export enum PaymentNetwork { } @Entity('payments') +@Index(['merchantId', 'status']) +@Index(['merchantId', 'createdAt']) export class Payment { @PrimaryGeneratedColumn('uuid') id: string; @@ -44,6 +47,7 @@ export class Payment { @JoinColumn({ name: 'merchantId' }) merchant: Merchant; + @Index() @Column() merchantId: string; diff --git a/tsconfig.json b/tsconfig.json index 95f5641c..c5555e22 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,8 +12,8 @@ "baseUrl": "./", "incremental": true, "skipLibCheck": true, - "strictNullChecks": false, - "noImplicitAny": false, + "strictNullChecks": true, + "noImplicitAny": true, "strictBindCallApply": false, "forceConsistentCasingInFileNames": false, "noFallthroughCasesInSwitch": false