diff --git a/.changeset/fix-sql-injection-search.md b/.changeset/fix-sql-injection-search.md new file mode 100644 index 0000000..612cd45 --- /dev/null +++ b/.changeset/fix-sql-injection-search.md @@ -0,0 +1,5 @@ +--- +"workers-research": patch +--- + +Fix SQL injection vulnerability in research list search endpoint by replacing string interpolation with parameterized queries using workers-qb's built-in parameter binding. diff --git a/src/index.tsx b/src/index.tsx index 7968031..0bd7266 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -27,7 +27,7 @@ import { TopBar, } from "./templates/layout"; import type { ResearchType, ResearchTypeDB } from "./types"; -import { formatDuration, getModel, normalizeDomain } from "./utils"; +import { buildSearchFilters, formatDuration, getModel, normalizeDomain } from "./utils"; export { ResearchWorkflow } from "./workflows"; @@ -141,18 +141,8 @@ app.get("/", async (c) => { const pageSize = 5; // Items per page const offset = (Number.parseInt(page, 10) - 1) * pageSize; - // Build where conditions - const conditions: string[] = []; - if (q?.trim()) { - // Search in title and query fields (escape single quotes to prevent SQL injection) - const searchTerm = q.trim().replace(/'/g, "''"); - conditions.push( - `(title LIKE '%${searchTerm}%' OR query LIKE '%${searchTerm}%')`, - ); - } - if (status && ["1", "2", "3"].includes(status)) { - conditions.push(`status = ${status}`); - } + // Build parameterized where conditions to prevent SQL injection + const { conditions, params: whereParams } = buildSearchFilters(q, status); // Build sort order let orderBy = "created_at desc nulls last"; @@ -177,7 +167,7 @@ app.get("/", async (c) => { let queryBuilder = qb.select("researches").orderBy(orderBy); if (conditions.length > 0) { - queryBuilder = queryBuilder.where(conditions.join(" AND ")); + queryBuilder = queryBuilder.where(conditions, whereParams); } // Fetch paginated results @@ -186,7 +176,7 @@ app.get("/", async (c) => { // Fetch total count for pagination (with filters) let countQuery = qb.select("researches"); if (conditions.length > 0) { - countQuery = countQuery.where(conditions.join(" AND ")); + countQuery = countQuery.where(conditions, whereParams); } const totalCount = (await countQuery.count()).results.total; diff --git a/src/utils.ts b/src/utils.ts index c53fd60..536e964 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -101,6 +101,31 @@ export function normalizeDomain(input: string): string { return domain; } +/** + * Build parameterized search filter conditions for research list queries. + * Uses parameterized queries to prevent SQL injection. + */ +export function buildSearchFilters( + q?: string, + status?: string, +): { conditions: string[]; params: (string | number)[] } { + const conditions: string[] = []; + const params: (string | number)[] = []; + + if (q?.trim()) { + const searchTerm = `%${q.trim()}%`; + conditions.push("(title LIKE ? OR query LIKE ?)"); + params.push(searchTerm, searchTerm); + } + + if (status && ["1", "2", "3"].includes(status)) { + conditions.push("status = ?"); + params.push(Number.parseInt(status, 10)); + } + + return { conditions, params }; +} + export function formatDuration(ms: number): string { // Handle negative or zero values if (ms <= 0) { diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index 4aded73..209dddf 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; -import { formatDuration, normalizeDomain, timeAgo } from "../../src/utils"; +import { buildSearchFilters, formatDuration, normalizeDomain, timeAgo } from "../../src/utils"; describe("timeAgo", () => { beforeEach(() => { @@ -131,6 +131,93 @@ describe("normalizeDomain", () => { }); }); +describe("buildSearchFilters", () => { + test("should return empty conditions and params when no filters provided", () => { + const result = buildSearchFilters(); + expect(result.conditions).toEqual([]); + expect(result.params).toEqual([]); + }); + + test("should return empty conditions for empty query string", () => { + const result = buildSearchFilters("", undefined); + expect(result.conditions).toEqual([]); + expect(result.params).toEqual([]); + }); + + test("should return empty conditions for whitespace-only query", () => { + const result = buildSearchFilters(" ", undefined); + expect(result.conditions).toEqual([]); + expect(result.params).toEqual([]); + }); + + test("should build parameterized LIKE condition for search query", () => { + const result = buildSearchFilters("test search", undefined); + expect(result.conditions).toEqual(["(title LIKE ? OR query LIKE ?)"]); + expect(result.params).toEqual(["%test search%", "%test search%"]); + }); + + test("should trim search query whitespace", () => { + const result = buildSearchFilters(" hello ", undefined); + expect(result.conditions).toEqual(["(title LIKE ? OR query LIKE ?)"]); + expect(result.params).toEqual(["%hello%", "%hello%"]); + }); + + test("should build parameterized status condition for valid status", () => { + const result = buildSearchFilters(undefined, "1"); + expect(result.conditions).toEqual(["status = ?"]); + expect(result.params).toEqual([1]); + }); + + test("should accept all valid status values (1, 2, 3)", () => { + for (const s of ["1", "2", "3"]) { + const result = buildSearchFilters(undefined, s); + expect(result.conditions).toEqual(["status = ?"]); + expect(result.params).toEqual([Number.parseInt(s, 10)]); + } + }); + + test("should ignore invalid status values", () => { + const result = buildSearchFilters(undefined, "4"); + expect(result.conditions).toEqual([]); + expect(result.params).toEqual([]); + }); + + test("should ignore SQL injection attempts in status parameter", () => { + const result = buildSearchFilters(undefined, "1 OR 1=1"); + expect(result.conditions).toEqual([]); + expect(result.params).toEqual([]); + }); + + test("should combine search and status filters", () => { + const result = buildSearchFilters("test", "2"); + expect(result.conditions).toEqual([ + "(title LIKE ? OR query LIKE ?)", + "status = ?", + ]); + expect(result.params).toEqual(["%test%", "%test%", 2]); + }); + + test("should safely parameterize SQL injection attempts in search query", () => { + const result = buildSearchFilters( + "'; DROP TABLE researches; --", + undefined, + ); + expect(result.conditions).toEqual(["(title LIKE ? OR query LIKE ?)"]); + // Malicious input is safely passed as a parameter, never interpolated into SQL + expect(result.params).toEqual([ + "%'; DROP TABLE researches; --%", + "%'; DROP TABLE researches; --%", + ]); + // Conditions should never contain the raw search term + expect(result.conditions[0]).not.toContain("DROP TABLE"); + }); + + test("should handle special SQL characters in search query as parameters", () => { + const result = buildSearchFilters("100% complete", undefined); + expect(result.params).toEqual(["%100% complete%", "%100% complete%"]); + }); +}); + describe("formatDuration", () => { test("should return 0.0 seconds for zero", () => { expect(formatDuration(0)).toBe("0.0 seconds");