From 42ecc99d68a9054e1b2dc8fea311661de424d5c1 Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Wed, 23 Sep 2026 22:10:14 +0300 Subject: [PATCH 1/2] factory: claim #28 by runner-47545-1790190614809 From 7b1689188c0295a8b4b10f1f45d7bfe447b22b2b Mon Sep 17 00:00:00 2001 From: Param Harrison Date: Wed, 23 Sep 2026 22:17:06 +0300 Subject: [PATCH 2/2] factory: build #28 --- src/expenses.ts | 15 ++++----------- tests/routes.test.ts | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/expenses.ts b/src/expenses.ts index 1da7163..c56e0d4 100644 --- a/src/expenses.ts +++ b/src/expenses.ts @@ -53,13 +53,6 @@ export interface ListExpensesOptions { /** * List (and optionally search) a group's expenses, newest first, keyset * paginated. - * - * SEEDED DEFECT (issue #4, SQL injection): the free-text `query` is spliced - * directly into the SQL string instead of being bound as a parameter, so a - * crafted `q` such as `' OR 1=1 --` breaks out of the LIKE clause and can - * read expenses belonging to OTHER groups. Baseline tests only search with - * plain words, so this ships undetected. The fix binds `query` as a `?` - * parameter (see .claude/skills/fixing-a-vulnerability). */ export function listExpenses(db: Database, groupId: string, options: ListExpensesOptions = {}): ExpenseWithPayer[] { const limit = Math.min(options.limit ?? 20, 100); @@ -69,10 +62,10 @@ export function listExpenses(db: Database, groupId: string, options: ListExpense conditions.push("e.created_at < (SELECT created_at FROM expenses WHERE id = ?)"); params.push(options.cursor); } - if (options.query) { - // VULNERABLE: spliced directly instead of bound as a parameter. See the - // SEEDED DEFECT note on this function's doc comment (issue #4). - conditions.push(`e.description LIKE '%${options.query}%'`); + const { query } = options; + if (query) { + conditions.push("e.description LIKE ?"); + params.push(`%${query}%`); } const sql = ` SELECT e.id, e.group_id as groupId, e.payer_id as payerId, e.amount_cents as amountCents, diff --git a/tests/routes.test.ts b/tests/routes.test.ts index 25cac54..569ef3e 100644 --- a/tests/routes.test.ts +++ b/tests/routes.test.ts @@ -28,7 +28,7 @@ interface GroupDetailBody { balances: { memberId: string; netCents: number }[]; } interface ExpensesListBody { - expenses: { id: string; description: string }[]; + expenses: { id: string; groupId: string; description: string }[]; } interface CreateExpenseBody { expense: { id: string }; @@ -88,6 +88,26 @@ describe("GET /api/groups/:id/expenses", () => { expect(body.expenses.length).toBe(1); expect(body.expenses[0]?.description).toBe("Fuel"); }); + + test("search does not leak other groups' expenses", async () => { + const app = setupApp(); + const q = encodeURIComponent("' OR 1=1 --"); + const res = await app.request(`/api/groups/goa-trip/expenses?q=${q}&limit=100`, { headers: auth("goa-asha") }); + expect(res.status).toBe(200); + const body = await asJson(res); + const descriptions = body.expenses.map((e) => e.description); + expect(descriptions).not.toContain("Electricity bill"); + expect(descriptions).not.toContain("Internet bill"); + for (const e of body.expenses) expect(e.groupId).toBe("goa-trip"); + }); + + test("search treats quotes in q as literal text", async () => { + const app = setupApp(); + const res = await app.request(`/api/groups/goa-trip/expenses?q=${encodeURIComponent("O'Brien")}`, { headers: auth("goa-asha") }); + expect(res.status).toBe(200); + const body = await asJson(res); + expect(body.expenses).toEqual([]); + }); }); describe("POST and DELETE /api/groups/:id/expenses", () => {