diff --git a/src/audit-log/audit-log.entity.ts b/src/audit-log/audit-log.entity.ts index c47a390b..39711c2e 100644 --- a/src/audit-log/audit-log.entity.ts +++ b/src/audit-log/audit-log.entity.ts @@ -26,7 +26,7 @@ export enum HttpMethod { @Index(['action', 'timestamp']) @Index(['category', 'timestamp']) @Index(['severity', 'timestamp']) -@Index(['entityType', 'entityId']) +@Index(['entityType', 'entityId', 'timestamp']) @Index(['ipAddress', 'timestamp']) @Index(['timestamp']) @Index(['retentionUntil']) // required for efficient retention policy deletes diff --git a/src/audit-log/services/audit-query.service.spec.ts b/src/audit-log/services/audit-query.service.spec.ts new file mode 100644 index 00000000..80dbc712 --- /dev/null +++ b/src/audit-log/services/audit-query.service.spec.ts @@ -0,0 +1,61 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { AuditQueryService } from './audit-query.service'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { AuditLog } from '../audit-log.entity'; + +describe('AuditQueryService Pagination Limits', () => { + let service: AuditQueryService; + let mockRepo: any; + + beforeEach(async () => { + mockRepo = { + find: jest.fn().mockResolvedValue([]), + }; + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuditQueryService, + { + provide: getRepositoryToken(AuditLog), + useValue: mockRepo, + }, + ], + }).compile(); + + service = module.get(AuditQueryService); + }); + + it('should enforce a hard maximum limit of 1000 for findAll', async () => { + await service.findAll(0, 999999); + expect(mockRepo.find).toHaveBeenCalledWith( + expect.objectContaining({ + take: 1000, + }), + ); + }); + + it('should enforce a hard maximum limit of 1000 for findByUser', async () => { + await service.findByUser('user123', 0, 999999); + expect(mockRepo.find).toHaveBeenCalledWith( + expect.objectContaining({ + take: 1000, + }), + ); + }); + + it('should apply a default 30-day time window if dates are not provided', async () => { + await service.findAll(0, 10); + const callArgs = mockRepo.find.mock.calls[0][0]; + const whereClause = callArgs.where.timestamp as any; + + // We expect a Between() operator + expect(whereClause._type).toEqual('between'); + const [start, end] = whereClause._value; + + expect(end).toBeInstanceOf(Date); + expect(start).toBeInstanceOf(Date); + + const diffDays = Math.round((end.getTime() - start.getTime()) / (1000 * 60 * 60 * 24)); + expect(diffDays).toBe(30); + }); +}); diff --git a/src/audit-log/services/audit-query.service.ts b/src/audit-log/services/audit-query.service.ts index cf2bac14..66652719 100644 --- a/src/audit-log/services/audit-query.service.ts +++ b/src/audit-log/services/audit-query.service.ts @@ -8,7 +8,22 @@ import { clampLimit } from '../../common/utils/pagination.utils'; import { PaginationService } from '../../common/services/pagination.service'; -/** +import { PaginationService } from '../../common/services/pagination.service'; + +const MAX_PAGINATION_LIMIT = 1000; +const DEFAULT_PAGINATION_LIMIT = 100; + +function getBoundedTimeWindow(startDate?: Date, endDate?: Date): { startDate: Date, endDate: Date } { + const end = endDate || new Date(); + const start = startDate || new Date(end.getTime() - 30 * 24 * 60 * 60 * 1000); // 30 days default + return { startDate: start, endDate: end }; +} + +function clampPagination(skip?: number, take?: number): { skip: number; take: number } { + const resolvedSkip = skip !== undefined && skip >= 0 ? skip : 0; + const resolvedTake = take !== undefined && take > 0 ? Math.min(take, MAX_PAGINATION_LIMIT) : DEFAULT_PAGINATION_LIMIT; + return { skip: resolvedSkip, take: resolvedTake }; +} * Provides audit log query operations. * Responsible for searching and retrieving audit logs. * Single Responsibility: Querying audit logs from the database. @@ -102,6 +117,13 @@ export class AuditQueryService { queryBuilder.andWhere('audit.timestamp >= :startDate', { startDate: filters.startDate }); } else if (filters.endDate) { queryBuilder.andWhere('audit.timestamp <= :endDate', { endDate: filters.endDate }); + } else { + // Default to 30 days if unbounded + const window = getBoundedTimeWindow(); + queryBuilder.andWhere('audit.timestamp BETWEEN :startDate AND :endDate', { + startDate: window.startDate, + endDate: window.endDate, + }); } const clampedLimit = clampLimit(limit); @@ -117,34 +139,44 @@ export class AuditQueryService { } /** - * Find all logs (with limit) + * Find all logs (with limit and skip) */ - async findAll(limit: number = 100): Promise { + async findAll(skip: number = 0, limit: number = DEFAULT_PAGINATION_LIMIT, startDate?: Date, endDate?: Date): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ + where: { timestamp: Between(window.startDate, window.endDate) }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } /** * Find logs by user */ - async findByUser(userId: string, limit: number = 100): Promise { + async findByUser(userId: string, skip: number = 0, limit: number = DEFAULT_PAGINATION_LIMIT, startDate?: Date, endDate?: Date): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ - where: { userId }, + where: { userId, timestamp: Between(window.startDate, window.endDate) }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } /** * Find logs by action */ - async findByAction(action: AuditAction, limit: number = 100): Promise { + async findByAction(action: AuditAction, skip: number = 0, limit: number = DEFAULT_PAGINATION_LIMIT, startDate?: Date, endDate?: Date): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ - where: { action }, + where: { action, timestamp: Between(window.startDate, window.endDate) }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } @@ -154,36 +186,82 @@ export class AuditQueryService { async findByEntity( entityType: string, entityId: string, - limit: number = 100, + skip: number = 0, + limit: number = DEFAULT_PAGINATION_LIMIT, + startDate?: Date, + endDate?: Date ): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ - where: { entityType, entityId }, + where: { entityType, entityId, timestamp: Between(window.startDate, window.endDate) }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } /** * Find logs by IP address */ - async findByIpAddress(ipAddress: string, limit: number = 100): Promise { + async findByIpAddress(ipAddress: string, skip: number = 0, limit: number = DEFAULT_PAGINATION_LIMIT, startDate?: Date, endDate?: Date): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ - where: { ipAddress }, + where: { ipAddress, timestamp: Between(window.startDate, window.endDate) }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } /** * Find logs by date range */ - async findByDateRange(startDate: Date, endDate: Date, limit: number = 1000): Promise { + async findByDateRange(startDate: Date, endDate: Date, skip: number = 0, limit: number = MAX_PAGINATION_LIMIT): Promise { + const { skip: clampedSkip, take: clampedTake } = clampPagination(skip, limit); + const window = getBoundedTimeWindow(startDate, endDate); return this.auditRepo.find({ where: { - timestamp: Between(startDate, endDate), + timestamp: Between(window.startDate, window.endDate), }, order: { timestamp: 'DESC' }, - take: limit, + skip: clampedSkip, + take: clampedTake, }); } + + /** + * For genuine bulk export needs, provide a streaming export path + * rather than an unbounded find. + */ + async streamAll(filters: IAuditLogSearchFilters = {}): Promise { + const queryBuilder = this.auditRepo.createQueryBuilder('audit'); + + if (filters.userId) queryBuilder.andWhere('audit.userId = :userId', { userId: filters.userId }); + if (filters.userEmail) queryBuilder.andWhere('audit.userEmail = :userEmail', { userEmail: filters.userEmail }); + if (filters.actions && filters.actions.length > 0) queryBuilder.andWhere('audit.action IN (:...actions)', { actions: filters.actions }); + if (filters.categories && filters.categories.length > 0) queryBuilder.andWhere('audit.category IN (:...categories)', { categories: filters.categories }); + if (filters.severities && filters.severities.length > 0) queryBuilder.andWhere('audit.severity IN (:...severities)', { severities: filters.severities }); + if (filters.entityType) queryBuilder.andWhere('audit.entityType = :entityType', { entityType: filters.entityType }); + if (filters.entityId) queryBuilder.andWhere('audit.entityId = :entityId', { entityId: filters.entityId }); + if (filters.ipAddress) queryBuilder.andWhere('audit.ipAddress = :ipAddress', { ipAddress: filters.ipAddress }); + if (filters.sessionId) queryBuilder.andWhere('audit.sessionId = :sessionId', { sessionId: filters.sessionId }); + if (filters.tenantId) queryBuilder.andWhere('audit.tenantId = :tenantId', { tenantId: filters.tenantId }); + + if (filters.startDate && filters.endDate) { + queryBuilder.andWhere('audit.timestamp BETWEEN :startDate AND :endDate', { startDate: filters.startDate, endDate: filters.endDate }); + } else if (filters.startDate) { + queryBuilder.andWhere('audit.timestamp >= :startDate', { startDate: filters.startDate }); + } else if (filters.endDate) { + queryBuilder.andWhere('audit.timestamp <= :endDate', { endDate: filters.endDate }); + } else { + const window = getBoundedTimeWindow(); + queryBuilder.andWhere('audit.timestamp BETWEEN :startDate AND :endDate', { startDate: window.startDate, endDate: window.endDate }); + } + + queryBuilder.orderBy('audit.timestamp', 'DESC'); + + return await queryBuilder.stream(); + } } diff --git a/src/main.ts b/src/main.ts index 74112777..2d5f4af8 100644 --- a/src/main.ts +++ b/src/main.ts @@ -253,7 +253,15 @@ async function bootstrapWorker(): Promise { const contentLength = parseInt(contentLengthValue || '', 10); - if (!Number.isNaN(contentLength) && contentLength > fileUploadMaxBytes) { + if (Number.isNaN(contentLength)) { + const transferEncoding = req.headers['transfer-encoding']; + if (!transferEncoding || !transferEncoding.includes('chunked')) { + res.status(411).json({ + message: 'Length Required', + }); + return; + } + } else if (contentLength > fileUploadMaxBytes) { res.status(413).json({ message: 'File upload too large', maxBytes: fileUploadMaxBytes, @@ -261,6 +269,14 @@ async function bootstrapWorker(): Promise { return; } + let bytesRead = 0; + req.on('data', (chunk: Buffer) => { + bytesRead += chunk.length; + if (bytesRead > fileUploadMaxBytes) { + req.destroy(new Error('Payload Too Large')); + } + }); + next(); }); diff --git a/src/media/validation/upload-validation.util.ts b/src/media/validation/upload-validation.util.ts index cac180da..462e945e 100644 --- a/src/media/validation/upload-validation.util.ts +++ b/src/media/validation/upload-validation.util.ts @@ -18,9 +18,14 @@ export interface IUploadValidationFileLike { export interface IUploadFilterCallback { (error: Error | null, acceptFile: boolean): void; } +const fileUploadMaxBytes = parseInt( + process.env.FILE_UPLOAD_MAX_BYTES || `${MAX_UPLOAD_FILE_SIZE}`, + 10, +); + export const MEDIA_UPLOAD_INTERCEPTOR_OPTIONS = { limits: { - fileSize: MAX_UPLOAD_FILE_SIZE, + fileSize: fileUploadMaxBytes, files: 1, }, fileFilter: ( @@ -52,6 +57,6 @@ export function buildUploadValidationDetails() { return { allowedTypes: ALL_ALLOWED_FILE_TYPES, sizeLimits: FILE_SIZE_LIMITS, - maxUploadSize: MAX_UPLOAD_FILE_SIZE, + maxUploadSize: fileUploadMaxBytes, }; } diff --git a/test/security/upload-bypass.e2e-spec.ts b/test/security/upload-bypass.e2e-spec.ts new file mode 100644 index 00000000..b6f38624 --- /dev/null +++ b/test/security/upload-bypass.e2e-spec.ts @@ -0,0 +1,153 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { INestApplication, Controller, Post, UseInterceptors, UploadedFile } from '@nestjs/common'; +import { FileInterceptor } from '@nestjs/platform-express'; +import { Request, Response, NextFunction } from 'express'; +import * as http from 'http'; +import { MEDIA_UPLOAD_INTERCEPTOR_OPTIONS } from '../../src/media/validation/upload-validation.util'; + +// Recreate the file size guard from main.ts for accurate e2e testing +function getFileSizeGuard(fileUploadMaxBytes: number) { + return (req: Request, res: Response, next: NextFunction): void => { + const contentType = req.headers['content-type']; + const contentLengthHeader = req.headers['content-length']; + + const isMultipart = + typeof contentType === 'string' && contentType.toLowerCase().includes('multipart/form-data'); + + if (!isMultipart) return next(); + + const contentLengthValue = Array.isArray(contentLengthHeader) + ? contentLengthHeader[0] + : contentLengthHeader; + + const contentLength = parseInt(contentLengthValue || '', 10); + + if (Number.isNaN(contentLength)) { + const transferEncoding = req.headers['transfer-encoding']; + if (!transferEncoding || !transferEncoding.includes('chunked')) { + res.status(411).json({ + message: 'Length Required', + }); + return; + } + } else if (contentLength > fileUploadMaxBytes) { + res.status(413).json({ + message: 'File upload too large', + maxBytes: fileUploadMaxBytes, + }); + return; + } + + let bytesRead = 0; + req.on('data', (chunk: Buffer) => { + bytesRead += chunk.length; + if (bytesRead > fileUploadMaxBytes) { + req.destroy(new Error('Payload Too Large')); + } + }); + + next(); + }; +} + +@Controller('upload') +class UploadController { + @Post() + @UseInterceptors(FileInterceptor('file', MEDIA_UPLOAD_INTERCEPTOR_OPTIONS)) + uploadFile(@UploadedFile() file: any) { + return { success: true, size: file?.size }; + } +} + +describe('Upload Size Guard Bypass Security (e2e)', () => { + let app: INestApplication; + const maxBytes = 100; + let server: http.Server; + let port: number; + + beforeAll(async () => { + process.env.FILE_UPLOAD_MAX_BYTES = '100'; + + const moduleFixture: TestingModule = await Test.createTestingModule({ + controllers: [UploadController], + }).compile(); + + app = moduleFixture.createNestApplication(); + app.use(getFileSizeGuard(maxBytes)); + + await app.init(); + + server = app.getHttpServer(); + server.listen(0); + port = (server.address() as any).port; + }); + + afterAll(async () => { + await app.close(); + }); + + const sendRawRequest = ( + headers: any, + bodyChunks: Buffer[], + ): Promise<{ status?: number; error?: Error }> => { + return new Promise((resolve) => { + const options = { + hostname: 'localhost', + port, + path: '/upload', + method: 'POST', + headers, + }; + + const req = http.request(options, (res) => { + resolve({ status: res.statusCode }); + }); + + req.on('error', (e) => { + resolve({ error: e }); + }); + + bodyChunks.forEach((chunk) => req.write(chunk)); + req.end(); + }); + }; + + it('should reject a chunked multipart upload exceeding the limit', async () => { + const boundary = '---boundary'; + const payload = Buffer.from( + `--${boundary}\r\nContent-Disposition: form-data; name="file"; filename="test.txt"\r\n\r\n${'A'.repeat(200)}\r\n--${boundary}--\r\n`, + ); + + const headers = { + 'Content-Type': `multipart/form-data; boundary=${boundary}`, + 'Transfer-Encoding': 'chunked', + }; + + const result = await sendRawRequest(headers, [payload]); + + // When req.destroy() is called, it causes a socket hang up (ECONNRESET) on the client side. + expect(result.error).toBeDefined(); + if (result.error) { + expect(result.error.message).toMatch(/socket hang up|ECONNRESET/); + } + }); + + it('should reject an understated Content-Length exceeding the limit', async () => { + const boundary = '---boundary'; + const payload = Buffer.from( + `--${boundary}\r\nContent-Disposition: form-data; name="file"; filename="test.txt"\r\n\r\n${'A'.repeat(200)}\r\n--${boundary}--\r\n`, + ); + + const headers = { + 'Content-Type': `multipart/form-data; boundary=${boundary}`, + 'Content-Length': '10', // Understated! + }; + + const result = await sendRawRequest(headers, [payload]); + + expect(result.error).toBeDefined(); + if (result.error) { + expect(result.error.message).toMatch(/socket hang up|ECONNRESET/); + } + }); +});