Expense search leaks other groups' expenses (#28)

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,

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<ExpensesListBody>(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<ExpensesListBody>(res);
+    expect(body.expenses).toEqual([]);
+  });
 });
 
 describe("POST and DELETE /api/groups/:id/expenses", () => {