From 17cfeb974bda7381f8939959e046ddf8d066a3de Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 6 Feb 2026 13:29:03 -0600 Subject: [PATCH] fix(SEC-API-19+20): Validate brain search length and limit params - Add @MaxLength(500) to BrainQueryDto.query and BrainQueryDto.search fields - Create BrainSearchDto with validated q (max 500 chars) and limit (1-100) fields - Update BrainController.search to use BrainSearchDto instead of raw query params - Add defensive validation in BrainService.search and BrainService.query methods: - Reject search terms exceeding 500 characters with BadRequestException - Clamp limit to valid range [1, 100] for defense-in-depth - Add comprehensive tests for DTO validation and service-level guards - Update existing controller tests for new search method signature Co-Authored-By: Claude Opus 4.6 --- .../src/brain/brain-search-validation.spec.ts | 234 ++++++++++++++++++ apps/api/src/brain/brain.controller.test.ts | 26 +- apps/api/src/brain/brain.controller.ts | 12 +- apps/api/src/brain/brain.service.ts | 39 ++- apps/api/src/brain/dto/brain-query.dto.ts | 17 ++ apps/api/src/brain/dto/index.ts | 1 + 6 files changed, 299 insertions(+), 30 deletions(-) create mode 100644 apps/api/src/brain/brain-search-validation.spec.ts diff --git a/apps/api/src/brain/brain-search-validation.spec.ts b/apps/api/src/brain/brain-search-validation.spec.ts new file mode 100644 index 0000000..1ed8ca4 --- /dev/null +++ b/apps/api/src/brain/brain-search-validation.spec.ts @@ -0,0 +1,234 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { validate } from "class-validator"; +import { plainToInstance } from "class-transformer"; +import { BadRequestException } from "@nestjs/common"; +import { BrainSearchDto, BrainQueryDto } from "./dto"; +import { BrainService } from "./brain.service"; +import { PrismaService } from "../prisma/prisma.service"; + +describe("Brain Search Validation", () => { + describe("BrainSearchDto", () => { + it("should accept a valid search query", async () => { + const dto = plainToInstance(BrainSearchDto, { q: "meeting notes", limit: 10 }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it("should accept empty query params", async () => { + const dto = plainToInstance(BrainSearchDto, {}); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it("should reject search query exceeding 500 characters", async () => { + const longQuery = "a".repeat(501); + const dto = plainToInstance(BrainSearchDto, { q: longQuery }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const qError = errors.find((e) => e.property === "q"); + expect(qError).toBeDefined(); + expect(qError?.constraints?.maxLength).toContain("500"); + }); + + it("should accept search query at exactly 500 characters", async () => { + const maxQuery = "a".repeat(500); + const dto = plainToInstance(BrainSearchDto, { q: maxQuery }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it("should reject negative limit", async () => { + const dto = plainToInstance(BrainSearchDto, { q: "test", limit: -1 }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const limitError = errors.find((e) => e.property === "limit"); + expect(limitError).toBeDefined(); + expect(limitError?.constraints?.min).toContain("1"); + }); + + it("should reject zero limit", async () => { + const dto = plainToInstance(BrainSearchDto, { q: "test", limit: 0 }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const limitError = errors.find((e) => e.property === "limit"); + expect(limitError).toBeDefined(); + }); + + it("should reject limit exceeding 100", async () => { + const dto = plainToInstance(BrainSearchDto, { q: "test", limit: 101 }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const limitError = errors.find((e) => e.property === "limit"); + expect(limitError).toBeDefined(); + expect(limitError?.constraints?.max).toContain("100"); + }); + + it("should accept limit at boundaries (1 and 100)", async () => { + const dto1 = plainToInstance(BrainSearchDto, { limit: 1 }); + const errors1 = await validate(dto1); + expect(errors1).toHaveLength(0); + + const dto100 = plainToInstance(BrainSearchDto, { limit: 100 }); + const errors100 = await validate(dto100); + expect(errors100).toHaveLength(0); + }); + + it("should reject non-integer limit", async () => { + const dto = plainToInstance(BrainSearchDto, { limit: 10.5 }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const limitError = errors.find((e) => e.property === "limit"); + expect(limitError).toBeDefined(); + }); + }); + + describe("BrainQueryDto search and query length validation", () => { + it("should reject query exceeding 500 characters", async () => { + const longQuery = "a".repeat(501); + const dto = plainToInstance(BrainQueryDto, { + workspaceId: "550e8400-e29b-41d4-a716-446655440000", + query: longQuery, + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const queryError = errors.find((e) => e.property === "query"); + expect(queryError).toBeDefined(); + expect(queryError?.constraints?.maxLength).toContain("500"); + }); + + it("should reject search exceeding 500 characters", async () => { + const longSearch = "b".repeat(501); + const dto = plainToInstance(BrainQueryDto, { + workspaceId: "550e8400-e29b-41d4-a716-446655440000", + search: longSearch, + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const searchError = errors.find((e) => e.property === "search"); + expect(searchError).toBeDefined(); + expect(searchError?.constraints?.maxLength).toContain("500"); + }); + + it("should accept query at exactly 500 characters", async () => { + const maxQuery = "a".repeat(500); + const dto = plainToInstance(BrainQueryDto, { + workspaceId: "550e8400-e29b-41d4-a716-446655440000", + query: maxQuery, + }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it("should accept search at exactly 500 characters", async () => { + const maxSearch = "b".repeat(500); + const dto = plainToInstance(BrainQueryDto, { + workspaceId: "550e8400-e29b-41d4-a716-446655440000", + search: maxSearch, + }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + }); + + describe("BrainService.search defensive validation", () => { + let service: BrainService; + let prisma: { + task: { findMany: ReturnType }; + event: { findMany: ReturnType }; + project: { findMany: ReturnType }; + }; + + beforeEach(() => { + prisma = { + task: { findMany: vi.fn().mockResolvedValue([]) }, + event: { findMany: vi.fn().mockResolvedValue([]) }, + project: { findMany: vi.fn().mockResolvedValue([]) }, + }; + service = new BrainService(prisma as unknown as PrismaService); + }); + + it("should throw BadRequestException for search term exceeding 500 characters", async () => { + const longTerm = "x".repeat(501); + await expect(service.search("workspace-id", longTerm)).rejects.toThrow(BadRequestException); + await expect(service.search("workspace-id", longTerm)).rejects.toThrow("500"); + }); + + it("should accept search term at exactly 500 characters", async () => { + const maxTerm = "x".repeat(500); + await expect(service.search("workspace-id", maxTerm)).resolves.toBeDefined(); + }); + + it("should clamp limit to max 100 when higher value provided", async () => { + await service.search("workspace-id", "test", 200); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 100 })); + }); + + it("should clamp limit to min 1 when negative value provided", async () => { + await service.search("workspace-id", "test", -5); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 1 })); + }); + + it("should clamp limit to min 1 when zero provided", async () => { + await service.search("workspace-id", "test", 0); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 1 })); + }); + + it("should pass through valid limit values unchanged", async () => { + await service.search("workspace-id", "test", 50); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 50 })); + }); + }); + + describe("BrainService.query defensive validation", () => { + let service: BrainService; + let prisma: { + task: { findMany: ReturnType }; + event: { findMany: ReturnType }; + project: { findMany: ReturnType }; + }; + + beforeEach(() => { + prisma = { + task: { findMany: vi.fn().mockResolvedValue([]) }, + event: { findMany: vi.fn().mockResolvedValue([]) }, + project: { findMany: vi.fn().mockResolvedValue([]) }, + }; + service = new BrainService(prisma as unknown as PrismaService); + }); + + it("should throw BadRequestException for search field exceeding 500 characters", async () => { + const longSearch = "y".repeat(501); + await expect( + service.query({ workspaceId: "workspace-id", search: longSearch }) + ).rejects.toThrow(BadRequestException); + }); + + it("should throw BadRequestException for query field exceeding 500 characters", async () => { + const longQuery = "z".repeat(501); + await expect( + service.query({ workspaceId: "workspace-id", query: longQuery }) + ).rejects.toThrow(BadRequestException); + }); + + it("should clamp limit to max 100 in query method", async () => { + await service.query({ workspaceId: "workspace-id", limit: 200 }); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 100 })); + }); + + it("should clamp limit to min 1 in query method when negative", async () => { + await service.query({ workspaceId: "workspace-id", limit: -10 }); + expect(prisma.task.findMany).toHaveBeenCalledWith(expect.objectContaining({ take: 1 })); + }); + + it("should accept valid query and search within limits", async () => { + await expect( + service.query({ + workspaceId: "workspace-id", + query: "test query", + search: "test search", + limit: 50, + }) + ).resolves.toBeDefined(); + }); + }); +}); diff --git a/apps/api/src/brain/brain.controller.test.ts b/apps/api/src/brain/brain.controller.test.ts index ccdffc1..9dcb5b2 100644 --- a/apps/api/src/brain/brain.controller.test.ts +++ b/apps/api/src/brain/brain.controller.test.ts @@ -250,39 +250,33 @@ describe("BrainController", () => { }); describe("search", () => { - it("should call service.search with parameters", async () => { - const result = await controller.search("test query", "10", mockWorkspaceId); + it("should call service.search with parameters from DTO", async () => { + const result = await controller.search({ q: "test query", limit: 10 }, mockWorkspaceId); expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "test query", 10); expect(result).toEqual(mockQueryResult); }); - it("should use default limit when not provided", async () => { - await controller.search("test", undefined as unknown as string, mockWorkspaceId); + it("should use default limit when not provided in DTO", async () => { + await controller.search({ q: "test" }, mockWorkspaceId); expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "test", 20); }); - it("should cap limit at 100", async () => { - await controller.search("test", "500", mockWorkspaceId); + it("should handle empty search DTO", async () => { + await controller.search({}, mockWorkspaceId); - expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "test", 100); + expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "", 20); }); - it("should handle empty search term", async () => { - await controller.search(undefined as unknown as string, "10", mockWorkspaceId); + it("should handle undefined q in DTO", async () => { + await controller.search({ limit: 10 }, mockWorkspaceId); expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "", 10); }); - it("should handle invalid limit", async () => { - await controller.search("test", "invalid", mockWorkspaceId); - - expect(mockService.search).toHaveBeenCalledWith(mockWorkspaceId, "test", 20); - }); - it("should return search result structure", async () => { - const result = await controller.search("test", "10", mockWorkspaceId); + const result = await controller.search({ q: "test", limit: 10 }, mockWorkspaceId); expect(result).toHaveProperty("tasks"); expect(result).toHaveProperty("events"); diff --git a/apps/api/src/brain/brain.controller.ts b/apps/api/src/brain/brain.controller.ts index 532254c..a0c9f18 100644 --- a/apps/api/src/brain/brain.controller.ts +++ b/apps/api/src/brain/brain.controller.ts @@ -3,6 +3,7 @@ import { BrainService } from "./brain.service"; import { IntentClassificationService } from "./intent-classification.service"; import { BrainQueryDto, + BrainSearchDto, BrainContextDto, ClassifyIntentDto, IntentClassificationResultDto, @@ -67,13 +68,10 @@ export class BrainController { */ @Get("search") @RequirePermission(Permission.WORKSPACE_ANY) - async search( - @Query("q") searchTerm: string, - @Query("limit") limit: string, - @Workspace() workspaceId: string - ) { - const parsedLimit = limit ? Math.min(parseInt(limit, 10) || 20, 100) : 20; - return this.brainService.search(workspaceId, searchTerm || "", parsedLimit); + async search(@Query() searchDto: BrainSearchDto, @Workspace() workspaceId: string) { + const searchTerm = searchDto.q ?? ""; + const limit = searchDto.limit ?? 20; + return this.brainService.search(workspaceId, searchTerm, limit); } /** diff --git a/apps/api/src/brain/brain.service.ts b/apps/api/src/brain/brain.service.ts index 2a641c8..96b8ff7 100644 --- a/apps/api/src/brain/brain.service.ts +++ b/apps/api/src/brain/brain.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from "@nestjs/common"; +import { Injectable, BadRequestException } from "@nestjs/common"; import { EntityType, TaskStatus, ProjectStatus } from "@prisma/client"; import { PrismaService } from "../prisma/prisma.service"; import type { BrainQueryDto, BrainContextDto, TaskFilter, EventFilter, ProjectFilter } from "./dto"; @@ -80,6 +80,11 @@ export interface BrainContext { }[]; } +/** Maximum allowed length for search query strings */ +const MAX_SEARCH_LENGTH = 500; +/** Maximum allowed limit for search results per entity type */ +const MAX_SEARCH_LIMIT = 100; + /** * @description Service for querying and aggregating workspace data for AI/brain operations. * Provides unified access to tasks, events, and projects with filtering and search capabilities. @@ -97,15 +102,28 @@ export class BrainService { */ async query(queryDto: BrainQueryDto): Promise { const { workspaceId, entities, search, limit = 20 } = queryDto; + if (search && search.length > MAX_SEARCH_LENGTH) { + throw new BadRequestException( + `Search term must not exceed ${String(MAX_SEARCH_LENGTH)} characters` + ); + } + if (queryDto.query && queryDto.query.length > MAX_SEARCH_LENGTH) { + throw new BadRequestException( + `Query must not exceed ${String(MAX_SEARCH_LENGTH)} characters` + ); + } + const clampedLimit = Math.max(1, Math.min(limit, MAX_SEARCH_LIMIT)); const includeEntities = entities ?? [EntityType.TASK, EntityType.EVENT, EntityType.PROJECT]; const includeTasks = includeEntities.includes(EntityType.TASK); const includeEvents = includeEntities.includes(EntityType.EVENT); const includeProjects = includeEntities.includes(EntityType.PROJECT); const [tasks, events, projects] = await Promise.all([ - includeTasks ? this.queryTasks(workspaceId, queryDto.tasks, search, limit) : [], - includeEvents ? this.queryEvents(workspaceId, queryDto.events, search, limit) : [], - includeProjects ? this.queryProjects(workspaceId, queryDto.projects, search, limit) : [], + includeTasks ? this.queryTasks(workspaceId, queryDto.tasks, search, clampedLimit) : [], + includeEvents ? this.queryEvents(workspaceId, queryDto.events, search, clampedLimit) : [], + includeProjects + ? this.queryProjects(workspaceId, queryDto.projects, search, clampedLimit) + : [], ]); // Build filters object conditionally for exactOptionalPropertyTypes @@ -259,10 +277,17 @@ export class BrainService { * @throws PrismaClientKnownRequestError if database query fails */ async search(workspaceId: string, searchTerm: string, limit = 20): Promise { + if (searchTerm.length > MAX_SEARCH_LENGTH) { + throw new BadRequestException( + `Search term must not exceed ${String(MAX_SEARCH_LENGTH)} characters` + ); + } + const clampedLimit = Math.max(1, Math.min(limit, MAX_SEARCH_LIMIT)); + const [tasks, events, projects] = await Promise.all([ - this.queryTasks(workspaceId, undefined, searchTerm, limit), - this.queryEvents(workspaceId, undefined, searchTerm, limit), - this.queryProjects(workspaceId, undefined, searchTerm, limit), + this.queryTasks(workspaceId, undefined, searchTerm, clampedLimit), + this.queryEvents(workspaceId, undefined, searchTerm, clampedLimit), + this.queryProjects(workspaceId, undefined, searchTerm, clampedLimit), ]); return { diff --git a/apps/api/src/brain/dto/brain-query.dto.ts b/apps/api/src/brain/dto/brain-query.dto.ts index 1ec56f7..c23ca34 100644 --- a/apps/api/src/brain/dto/brain-query.dto.ts +++ b/apps/api/src/brain/dto/brain-query.dto.ts @@ -7,6 +7,7 @@ import { IsInt, Min, Max, + MaxLength, IsDateString, IsArray, ValidateNested, @@ -105,6 +106,7 @@ export class BrainQueryDto { @IsOptional() @IsString() + @MaxLength(500, { message: "query must not exceed 500 characters" }) query?: string; @IsOptional() @@ -129,6 +131,7 @@ export class BrainQueryDto { @IsOptional() @IsString() + @MaxLength(500, { message: "search must not exceed 500 characters" }) search?: string; @IsOptional() @@ -162,3 +165,17 @@ export class BrainContextDto { @Max(30) eventDays?: number; } + +export class BrainSearchDto { + @IsOptional() + @IsString() + @MaxLength(500, { message: "q must not exceed 500 characters" }) + q?: string; + + @IsOptional() + @Type(() => Number) + @IsInt({ message: "limit must be an integer" }) + @Min(1, { message: "limit must be at least 1" }) + @Max(100, { message: "limit must not exceed 100" }) + limit?: number; +} diff --git a/apps/api/src/brain/dto/index.ts b/apps/api/src/brain/dto/index.ts index 5eb72a7..25c4a51 100644 --- a/apps/api/src/brain/dto/index.ts +++ b/apps/api/src/brain/dto/index.ts @@ -1,5 +1,6 @@ export { BrainQueryDto, + BrainSearchDto, TaskFilter, EventFilter, ProjectFilter,