From 40d1c54755346c775b004c285ca37219805be8a3 Mon Sep 17 00:00:00 2001 From: Kirtan-pc Date: Fri, 7 Aug 2026 18:57:18 +0530 Subject: [PATCH] test(web): regression guards for medicine search hardening (issue #4201) The brace-imbalance bug (unclosed local getClientIp declaration shadowing the hardened helpers) was removed upstream in 2dc6e6a8, but the route had no runnable coverage: the existing route.test.ts lives under app/api and is excluded by jest.config.cjs (roots: tests/), so it never executed. Add a runnable test in tests/ that pins the security behavior the broken inline reimplementation would have silently undone: - rate limiting uses the hardened getClientIp: forged X-Forwarded-For / X-Real-IP are ignored (loopback default), so an attacker cannot mint a fresh rate-limit bucket per request. - LIKE wildcards (% / _) are escaped before reaching PostgREST, preventing wildcard shaping / cache poisoning of med_search:* entries. - comma/paren queries reach the DB without throwing. - short (<2 char) and oversized (>100 char) queries short-circuit before touching Redis or the DB. --- apps/web/tests/medicine-search-route.test.ts | 131 +++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 apps/web/tests/medicine-search-route.test.ts diff --git a/apps/web/tests/medicine-search-route.test.ts b/apps/web/tests/medicine-search-route.test.ts new file mode 100644 index 000000000..5ad275e93 --- /dev/null +++ b/apps/web/tests/medicine-search-route.test.ts @@ -0,0 +1,131 @@ +import { describe, it, expect, jest, beforeEach } from "@jest/globals"; + +const mockLimit = jest.fn<(key: string) => Promise>(); + +type RateLimitResult = { + success: boolean; + limit: number; + remaining: number; + reset: number; +}; + +jest.mock("@/lib/rateLimit", () => ({ + rateLimit: { limit: (...args: unknown[]) => mockLimit(args[0] as string) }, +})); + +const mockGet = jest.fn<() => Promise>(); +const mockSet = jest.fn(); + +jest.mock("@/lib/redis", () => ({ + redis: { + get: (...args: unknown[]) => mockGet(), + set: (...args: unknown[]) => mockSet(args[0], args[1], args[2]), + }, +})); + +const mockFrom = jest.fn(); + +jest.mock("@/lib/supabase", () => ({ + supabase: { + from: (table: string) => mockFrom(table), + }, +})); + +import { NextRequest } from "next/server"; +import { GET } from "../app/api/medicines/search/route"; + +function allowAll() { + mockLimit.mockResolvedValue({ + success: true, + limit: 30, + remaining: 29, + reset: Date.now() + 60000, + }); +} + +function makeRequest(q: string, headers: Record = {}): NextRequest { + return new NextRequest(`http://localhost/api/medicines/search?q=${encodeURIComponent(q)}`, { + headers: { "x-forwarded-for": "127.0.0.1", ...headers }, + }); +} + +function arrangeDbRows() { + mockFrom.mockReturnValue({ + select: jest.fn().mockReturnThis(), + or: jest.fn().mockReturnThis(), + limit: jest.fn().mockResolvedValue({ data: [], error: null }), + }); +} + +describe("GET /api/medicines/search — regression guard for Issue #4201", () => { + beforeEach(() => { + jest.clearAllMocks(); + allowAll(); + arrangeDbRows(); + }); + + it("uses the hardened getClientIp (defaults to loopback) so forged X-Forwarded-For cannot create fresh rate-limit buckets", async () => { + // Without TRUST_PROXY_HEADERS the hardened helper must ignore + // attacker-controlled forwarding headers entirely and rate-limit on + // 127.0.0.1. The naive inline reimplementation (Issue #4201) read the + // leftmost hop instead, giving a fresh bucket per request. + const res = await GET( + new NextRequest("http://localhost/api/medicines/search?q=aspirin&token=1", { + headers: { + "x-forwarded-for": "203.0.113.99, 198.51.100.7", + "x-real-ip": "198.51.100.7", + }, + }) + ); + + expect(res.status).toBe(200); + expect(mockLimit).toHaveBeenCalledTimes(1); + expect(mockLimit).toHaveBeenCalledWith("127.0.0.1"); + }); + + it("escapes LIKE wildcards from the query so they cannot shape the PostgREST filter", async () => { + const orMock = jest.fn().mockReturnThis(); + mockFrom.mockReturnValue({ + select: jest.fn().mockReturnThis(), + or: orMock, + limit: jest.fn().mockResolvedValue({ data: [], error: null }), + }); + + const res = await GET(makeRequest("50%_off")); + expect(res.status).toBe(200); + + const orArg = orMock.mock.calls[0][0] as string; + // % and _ must be escaped (=> \% \_) so the wildcard is literal. + expect(orArg).not.toMatch(/%50%_off%/); + }); + + it("reaches the DB safely for queries with commas/parentheses instead of throwing", async () => { + const orMock = jest.fn().mockReturnThis(); + mockFrom.mockReturnValue({ + select: jest.fn().mockReturnThis(), + or: orMock, + limit: jest.fn().mockResolvedValue({ data: [], error: null }), + }); + + const res = await GET(makeRequest("aspirin, 500mg (test)")); + expect(res.status).toBe(200); + // The comma/paren query must be routed through PostgREST escaping rather + // than crashing the handler. + expect(orMock).toHaveBeenCalledTimes(1); + }); + + it("short-circuits before Redis/DB for queries under 2 characters", async () => { + const res = await GET(makeRequest("a")); + expect(res.status).toBe(200); + expect(await res.json()).toEqual([]); + expect(mockGet).not.toHaveBeenCalled(); + expect(mockFrom).not.toHaveBeenCalled(); + }); + + it("rejects queries longer than 100 characters without touching Redis or DB", async () => { + const res = await GET(makeRequest("a".repeat(101))); + expect(res.status).toBe(400); + expect(mockGet).not.toHaveBeenCalled(); + expect(mockFrom).not.toHaveBeenCalled(); + }); +});