From 3c9b77e81a0ea200439bd9f59dd6c6f2c624b06b Mon Sep 17 00:00:00 2001 From: Samuel1-ona Date: Thu, 24 Sep 2026 08:52:03 +0100 Subject: [PATCH] fix(v2): centralize request validation and pagination bounds --- src/bootstrap.spec.ts | 47 +++++++++---- src/bootstrap.ts | 4 ++ .../v2/dto/claim-feed-query.dto.spec.ts | 49 +++++++++++++ src/claims/v2/dto/claim-feed-query.dto.ts | 13 +++- .../common/dto/pagination-query.dto.spec.ts | 68 +++++++++++++++++++ src/v2/common/dto/pagination-query.dto.ts | 27 ++++++++ src/v2/disputes/disputes.controller.ts | 8 +-- .../verification/verification.controller.ts | 15 ++-- 8 files changed, 203 insertions(+), 28 deletions(-) create mode 100644 src/claims/v2/dto/claim-feed-query.dto.spec.ts create mode 100644 src/v2/common/dto/pagination-query.dto.spec.ts create mode 100644 src/v2/common/dto/pagination-query.dto.ts diff --git a/src/bootstrap.spec.ts b/src/bootstrap.spec.ts index db5ca346..33aeb3b1 100644 --- a/src/bootstrap.spec.ts +++ b/src/bootstrap.spec.ts @@ -1,29 +1,48 @@ import { ValidationPipe } from '@nestjs/common'; +import { describe, expect, it, jest } from '@jest/globals'; import { configureApp } from './bootstrap'; -jest.mock('@nestjs/swagger', () => { - const actual = jest.requireActual('@nestjs/swagger'); - return { - ...actual, - SwaggerModule: { - createDocument: jest.fn().mockReturnValue({}), - setup: jest.fn(), - }, - }; -}); +jest.mock('@nestjs/swagger', () => ({ + SwaggerModule: { + createDocument: jest.fn().mockReturnValue({}), + setup: jest.fn(), + }, + DocumentBuilder: jest.fn().mockImplementation(() => ({ + setTitle: jest.fn().mockReturnThis(), + setDescription: jest.fn().mockReturnThis(), + setVersion: jest.fn().mockReturnThis(), + addBearerAuth: jest.fn().mockReturnThis(), + addTag: jest.fn().mockReturnThis(), + build: jest.fn().mockReturnValue({}), + })), +})); describe('configureApp', () => { - it('registers a single strict global validation pipe', () => { - const httpAdapter = { set: jest.fn() }; + it('registers strict global validation and bounded body parsers', () => { + const httpAdapter = { + set: jest.fn(), + use: jest.fn(), + }; + const app = { useLogger: jest.fn(), get: jest.fn(), - getHttpAdapter: jest.fn().mockReturnValue({ getInstance: () => httpAdapter }), + getHttpAdapter: jest.fn().mockReturnValue({ + getInstance: () => httpAdapter, + }), useGlobalPipes: jest.fn(), } as any; configureApp(app); + expect(httpAdapter.use).toHaveBeenCalledTimes(2); + + const [jsonParser] = httpAdapter.use.mock.calls[0]; + const [urlencodedParser] = httpAdapter.use.mock.calls[1]; + + expect(jsonParser).toHaveProperty('name', 'jsonParser'); + expect(urlencodedParser).toHaveProperty('name', 'urlencodedParser'); + expect(app.useGlobalPipes).toHaveBeenCalledTimes(1); const [pipe] = app.useGlobalPipes.mock.calls[0]; @@ -43,4 +62,4 @@ describe('configureApp', () => { enableImplicitConversion: true, }); }); -}); \ No newline at end of file +}); diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 000cddbf..dbed1011 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -1,5 +1,6 @@ import { INestApplication, ValidationPipe } from '@nestjs/common'; import { DocumentBuilder, SwaggerModule } from '@nestjs/swagger'; +import { json, urlencoded } from 'express'; import { Logger } from 'nestjs-pino'; export function createGlobalValidationPipe() { @@ -26,6 +27,9 @@ export function configureApp(app: INestApplication) { httpAdapter.set('trust proxy', false); } + httpAdapter.use(json({ limit: '100kb' })); + httpAdapter.use(urlencoded({ extended: true, limit: '100kb' })); + app.useGlobalPipes(createGlobalValidationPipe()); const config = new DocumentBuilder() diff --git a/src/claims/v2/dto/claim-feed-query.dto.spec.ts b/src/claims/v2/dto/claim-feed-query.dto.spec.ts new file mode 100644 index 00000000..db4b8b5d --- /dev/null +++ b/src/claims/v2/dto/claim-feed-query.dto.spec.ts @@ -0,0 +1,49 @@ +import { BadRequestException, ValidationPipe } from '@nestjs/common'; +import { describe, expect, it } from '@jest/globals'; +import { ClaimFeedQueryDto } from './claim-feed-query.dto'; + +describe('ClaimFeedQueryDto', () => { + const pipe = new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + }); + + const validate = async (value: Record) => + pipe.transform(value, { + type: 'query', + metatype: ClaimFeedQueryDto, + data: '', + }); + + it('accepts a valid Ethereum creator address', async () => { + const result = await validate({ + creator: '0x1111111111111111111111111111111111111111', + }); + + expect(result).toBeInstanceOf(ClaimFeedQueryDto); + expect((result as ClaimFeedQueryDto).creator).toBe( + '0x1111111111111111111111111111111111111111', + ); + }); + + it.each([ + '', + 'not-an-address', + '0x1234', + '0xgggggggggggggggggggggggggggggggggggggggg', + ])('rejects an invalid creator address: %s', async (creator) => { + await expect(validate({ creator })).rejects.toBeInstanceOf( + BadRequestException, + ); + }); + + it('rejects unknown query fields', async () => { + await expect( + validate({ + creator: '0x1111111111111111111111111111111111111111', + unexpected: 'value', + }), + ).rejects.toBeInstanceOf(BadRequestException); + }); +}); diff --git a/src/claims/v2/dto/claim-feed-query.dto.ts b/src/claims/v2/dto/claim-feed-query.dto.ts index 28f9905d..32129f98 100644 --- a/src/claims/v2/dto/claim-feed-query.dto.ts +++ b/src/claims/v2/dto/claim-feed-query.dto.ts @@ -1,5 +1,14 @@ import { ApiPropertyOptional } from '@nestjs/swagger'; -import { IsOptional, IsString, IsInt, Min, Max, IsIn, IsDateString } from 'class-validator'; +import { + IsOptional, + IsString, + IsInt, + Min, + Max, + IsIn, + IsDateString, + IsEthereumAddress, +} from 'class-validator'; import { Type } from 'class-transformer'; export const CLAIM_FEED_MAX_LIMIT = 100; @@ -35,7 +44,7 @@ export class ClaimFeedQueryDto { @ApiPropertyOptional({ description: 'Filter by creator wallet address' }) @IsOptional() - @IsString() + @IsEthereumAddress() creator?: string; @ApiPropertyOptional({ diff --git a/src/v2/common/dto/pagination-query.dto.spec.ts b/src/v2/common/dto/pagination-query.dto.spec.ts new file mode 100644 index 00000000..04ab62de --- /dev/null +++ b/src/v2/common/dto/pagination-query.dto.spec.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from '@jest/globals'; +import { BadRequestException, ValidationPipe } from '@nestjs/common'; +import { PaginationQueryDto } from './pagination-query.dto'; + +describe('PaginationQueryDto', () => { + const pipe = new ValidationPipe({ + transform: true, + whitelist: true, + forbidNonWhitelisted: true, + }); + + const validate = async (value: Record) => + pipe.transform(value, { + type: 'query', + metatype: PaginationQueryDto, + data: '', + }); + + it('uses the default page limit when limit is omitted', async () => { + const result = await validate({}); + + expect(result).toBeInstanceOf(PaginationQueryDto); + expect((result as PaginationQueryDto).limit).toBe(20); + }); + + it('accepts the minimum and maximum page limits', async () => { + await expect(validate({ limit: '1' })).resolves.toMatchObject({ + limit: 1, + }); + + await expect(validate({ limit: '100' })).resolves.toMatchObject({ + limit: 100, + }); + }); + + it.each(['0', '-1', '101'])( + 'rejects an out-of-range limit: %s', + async (limit) => { + await expect(validate({ limit })).rejects.toBeInstanceOf( + BadRequestException, + ); + }, + ); + + it('rejects malformed numeric input instead of partially parsing it', async () => { + await expect(validate({ limit: '20abc' })).rejects.toBeInstanceOf( + BadRequestException, + ); + }); + + it('rejects unknown query fields', async () => { + await expect( + validate({ limit: '20', unexpected: 'value' }), + ).rejects.toBeInstanceOf(BadRequestException); + }); + + it('accepts an optional string cursor', async () => { + const result = await validate({ + cursor: 'opaque-cursor', + limit: '20', + }); + + expect(result).toMatchObject({ + cursor: 'opaque-cursor', + limit: 20, + }); + }); +}); diff --git a/src/v2/common/dto/pagination-query.dto.ts b/src/v2/common/dto/pagination-query.dto.ts new file mode 100644 index 00000000..56959bec --- /dev/null +++ b/src/v2/common/dto/pagination-query.dto.ts @@ -0,0 +1,27 @@ +import { ApiPropertyOptional } from '@nestjs/swagger'; +import { Type } from 'class-transformer'; +import { IsInt, IsOptional, IsString, Max, Min } from 'class-validator'; + +export const DEFAULT_PAGE_LIMIT = 20; +export const MAX_PAGE_LIMIT = 100; + +export class PaginationQueryDto { + @ApiPropertyOptional({ + description: 'Opaque cursor from a previous response', + }) + @IsOptional() + @IsString() + cursor?: string; + + @ApiPropertyOptional({ + default: DEFAULT_PAGE_LIMIT, + minimum: 1, + maximum: MAX_PAGE_LIMIT, + }) + @IsOptional() + @Type(() => Number) + @IsInt() + @Min(1) + @Max(MAX_PAGE_LIMIT) + limit: number = DEFAULT_PAGE_LIMIT; +} diff --git a/src/v2/disputes/disputes.controller.ts b/src/v2/disputes/disputes.controller.ts index 48d1568d..72853fb3 100644 --- a/src/v2/disputes/disputes.controller.ts +++ b/src/v2/disputes/disputes.controller.ts @@ -1,5 +1,6 @@ import { Controller, Get, Param, Query } from '@nestjs/common'; import { DisputesQueryService } from './disputes-query.service'; +import { PaginationQueryDto } from '../common/dto/pagination-query.dto'; /** * Read-only V2 dispute endpoints. No write handlers: dispute state is @@ -13,10 +14,9 @@ export class DisputesController { @Get() async listForClaim( @Param('claimId') claimId: string, - @Query('limit') limit?: string, - @Query('cursor') cursor?: string, + @Query() query: PaginationQueryDto, ) { - return this.queryService.listForClaim(claimId, limit ? parseInt(limit, 10) : 20, cursor); + return this.queryService.listForClaim(claimId, query.limit, query.cursor); } @Get(':originalRoundId') @@ -26,4 +26,4 @@ export class DisputesController { ) { return this.queryService.getByOriginalRound(claimId, originalRoundId); } -} \ No newline at end of file +} diff --git a/src/v2/verification/verification.controller.ts b/src/v2/verification/verification.controller.ts index 3d719f82..7b3697b6 100644 --- a/src/v2/verification/verification.controller.ts +++ b/src/v2/verification/verification.controller.ts @@ -1,5 +1,6 @@ -import { Controller, Get, Param, Query, ParseUUIDPipe } from '@nestjs/common'; +import { Controller, Get, Param, Query } from '@nestjs/common'; import { VerificationQueryService } from './verification-query.service'; +import { PaginationQueryDto } from '../common/dto/pagination-query.dto'; /** * Read-only V2 verification endpoints. No write handlers: round and @@ -13,10 +14,9 @@ export class VerificationController { @Get('claims/:claimId/verification-rounds') async listRounds( @Param('claimId') claimId: string, - @Query('limit') limit?: string, - @Query('cursor') cursor?: string, + @Query() query: PaginationQueryDto, ) { - return this.queryService.listRounds(claimId, limit ? parseInt(limit, 10) : 20, cursor); + return this.queryService.listRounds(claimId, query.limit, query.cursor); } @Get('verification-rounds/:roundId') @@ -27,9 +27,8 @@ export class VerificationController { @Get('verification-rounds/:roundId/positions') async listPositions( @Param('roundId') roundId: string, - @Query('limit') limit?: string, - @Query('cursor') cursor?: string, + @Query() query: PaginationQueryDto, ) { - return this.queryService.listPositions(roundId, limit ? parseInt(limit, 10) : 20, cursor); + return this.queryService.listPositions(roundId, query.limit, query.cursor); } -} \ No newline at end of file +}