From e73d39b798a1cc4f22907dff6a32b68f2447b0b8 Mon Sep 17 00:00:00 2001 From: maaz519 Date: Thu, 28 May 2026 00:31:43 +0530 Subject: [PATCH] fix(api): escape filter values, clamp pagination, remove redundant ConfigModule import Co-Authored-By: Claude Sonnet 4.6 --- apps/api/src/modules/search/search.module.ts | 2 -- .../src/modules/search/search.service.spec.ts | 18 +++++++++++++ apps/api/src/modules/search/search.service.ts | 25 ++++++++++++------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/apps/api/src/modules/search/search.module.ts b/apps/api/src/modules/search/search.module.ts index 793aebc..238d0e8 100644 --- a/apps/api/src/modules/search/search.module.ts +++ b/apps/api/src/modules/search/search.module.ts @@ -1,10 +1,8 @@ import { Module } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; import { SearchController } from './search.controller'; import { SearchService } from './search.service'; @Module({ - imports: [ConfigModule], controllers: [SearchController], providers: [SearchService], }) diff --git a/apps/api/src/modules/search/search.service.spec.ts b/apps/api/src/modules/search/search.service.spec.ts index 69245e6..f932582 100644 --- a/apps/api/src/modules/search/search.service.spec.ts +++ b/apps/api/src/modules/search/search.service.spec.ts @@ -85,4 +85,22 @@ describe('SearchService', () => { expect.objectContaining({ page: 1, hitsPerPage: 20 }), ); }); + + it('escapes double-quotes in groupId to prevent filter injection', async () => { + mockSearch.mockResolvedValue({ hits: [], totalHits: 0 }); + await service.search('hello', 'grp"1"OR id EXISTS'); + expect(mockSearch).toHaveBeenCalledWith( + 'hello', + expect.objectContaining({ filter: 'sourceGroupId = "grp\\"1\\"OR id EXISTS"' }), + ); + }); + + it('clamps page to minimum 1 and limit to maximum 100', async () => { + mockSearch.mockResolvedValue({ hits: [], totalHits: 0 }); + await service.search('hello', undefined, undefined, 0, 999); + expect(mockSearch).toHaveBeenCalledWith( + 'hello', + expect.objectContaining({ page: 1, hitsPerPage: 100 }), + ); + }); }); diff --git a/apps/api/src/modules/search/search.service.ts b/apps/api/src/modules/search/search.service.ts index ac9bb42..50002b0 100644 --- a/apps/api/src/modules/search/search.service.ts +++ b/apps/api/src/modules/search/search.service.ts @@ -19,6 +19,10 @@ export class SearchService implements OnModuleInit { ); } + private static escapeFilterValue(value: string): string { + return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); + } + async onModuleInit(): Promise { await configureIndex(this.client).catch((err) => console.warn('Failed to configure Meilisearch index:', err), @@ -32,22 +36,25 @@ export class SearchService implements OnModuleInit { page = 1, limit = 20, ): Promise<{ hits: MeiliDocument[]; total: number; page: number; limit: number; query: string }> { - const filters: string[] = []; - if (groupId) filters.push(`sourceGroupId = "${groupId}"`); - if (tags?.length) filters.push(...tags.map((t) => `tags = "${t}"`)); + const safePage = Math.max(1, Math.floor(Number.isFinite(page) ? page : 1)); + const safeLimit = Math.min(100, Math.max(1, Math.floor(Number.isFinite(limit) ? limit : 20))); - const result = await this.client.index(MESSAGES_INDEX).search(query, { + const filters: string[] = []; + if (groupId) filters.push(`sourceGroupId = "${SearchService.escapeFilterValue(groupId)}"`); + if (tags?.length) filters.push(...tags.map((t) => `tags = "${SearchService.escapeFilterValue(t)}"`)); + + const result = await this.client.index(MESSAGES_INDEX).search(query, { filter: filters.length ? filters.join(' AND ') : undefined, - page, - hitsPerPage: limit, + page: safePage, + hitsPerPage: safeLimit, sort: ['approvedAt:desc'], }); return { - hits: result.hits as MeiliDocument[], + hits: result.hits, total: result.totalHits ?? 0, - page, - limit, + page: safePage, + limit: safeLimit, query, }; }