fix(SEC-API-19+20): Validate brain search length and limit params
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
- 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 <noreply@anthropic.com>
This commit is contained in:
234
apps/api/src/brain/brain-search-validation.spec.ts
Normal file
234
apps/api/src/brain/brain-search-validation.spec.ts
Normal file
@@ -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<typeof vi.fn> };
|
||||
event: { findMany: ReturnType<typeof vi.fn> };
|
||||
project: { findMany: ReturnType<typeof vi.fn> };
|
||||
};
|
||||
|
||||
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<typeof vi.fn> };
|
||||
event: { findMany: ReturnType<typeof vi.fn> };
|
||||
project: { findMany: ReturnType<typeof vi.fn> };
|
||||
};
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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");
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<BrainQueryResult> {
|
||||
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<BrainQueryResult> {
|
||||
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 {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
export {
|
||||
BrainQueryDto,
|
||||
BrainSearchDto,
|
||||
TaskFilter,
|
||||
EventFilter,
|
||||
ProjectFilter,
|
||||
|
||||
Reference in New Issue
Block a user