Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions docs/fixes/payment-hardening.md
Original file line number Diff line number Diff line change
@@ -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.
34 changes: 34 additions & 0 deletions src/common/decorators/max-json-size.decorator.ts
Original file line number Diff line number Diff line change
@@ -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`;
},
},
});
};
}
12 changes: 11 additions & 1 deletion src/payments/dto/batch-create-payment.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' })
Expand All @@ -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<string, any>;

@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;
}

Expand Down
14 changes: 12 additions & 2 deletions src/payments/dto/create-payment.dto.ts
Original file line number Diff line number Diff line change
@@ -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' })
Expand All @@ -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<string, any>;

@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;
}
4 changes: 4 additions & 0 deletions src/payments/entities/payment.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,6 +34,8 @@ export enum PaymentNetwork {
}

@Entity('payments')
@Index(['merchantId', 'status'])
@Index(['merchantId', 'createdAt'])
export class Payment {
@PrimaryGeneratedColumn('uuid')
id: string;
Expand All @@ -44,6 +47,7 @@ export class Payment {
@JoinColumn({ name: 'merchantId' })
merchant: Merchant;

@Index()
@Column()
merchantId: string;

Expand Down
4 changes: 2 additions & 2 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"baseUrl": "./",
"incremental": true,
"skipLibCheck": true,
"strictNullChecks": false,
"noImplicitAny": false,
"strictNullChecks": true,
"noImplicitAny": true,
"strictBindCallApply": false,
"forceConsistentCasingInFileNames": false,
"noFallthroughCasesInSwitch": false
Expand Down