Expense search leaks other groups' expenses (#28)

[Param-Harrison]

Closes #28

Summary

Fixes a SQL injection in expense search. listExpenses in src/expenses.ts now binds the search term as a ? parameter (e.description LIKE ? with %${query}% pushed onto params) instead of splicing it into the SQL string, so a search only returns expenses from the group in the URL.

Acceptance criteria

AC	Evidence (command / test)	Result
AC-1: a crafted search of goa-trip returns no Flat 4B expense; every row has group_id goa-trip	bun test tests/routes.test.ts -t "does not leak other groups' expenses" → 1 pass, 0 fail	pass
AC-2: a quote in q is treated as literal text (200 with a list, no SQL error)	bun test tests/routes.test.ts -t "treats quotes in q as literal text" → 1 pass, 0 fail	pass
AC-3: existing search and pagination unchanged (?q=Fuel still returns exactly one expense)	bun test tests/routes.test.ts → 11 pass, 0 fail	pass
AC-4: options.query no longer in the SQL string; stale "SEEDED DEFECT / VULNERABLE" comments removed	grep -n 'options.query}' src/expenses.ts → no match	pass
AC-5: full gate green	make check → tsc --noEmit clean, 28 pass, 0 fail across 4 files, skills valid	pass

Non-goals respected

• NG-1: No edits to src/auth/**, .factory/**, .claude/**, .agents/** or .github/**. The diff is src/expenses.ts and tests/routes.test.ts only.
• NG-2: Route API shape unchanged; src/routes/expenses.ts not edited.
• NG-3: No %/_ wildcard escaping and no ESCAPE clause. Wildcard matching within the caller's own group is a separate question to file on its own.
• NG-4: Cursor, limit and ordering in listExpenses untouched; no changes to seed.ts, db.ts, the schema, csv.ts or cli.ts.
• NG-5: No exploit payload or reproduction steps in the commit message or this description. The test in the diff is the proof.

The test that bites

• Test: GET /api/groups/:id/expenses > search does not leak other groups' expenses
• Fails on main: with only src/expenses.ts reverted to the base, bun test tests/routes.test.ts -t "does not leak other groups' expenses" fails with Expected to not contain: "Electricity bill" (the response included Flat 4B expenses). The same revert also fails search treats quotes in q as literal text with a SQLite syntax error.
• Passes here: bun test tests/routes.test.ts -t "does not leak other groups' expenses" → 1 pass, 0 fail

Gate line

FACTORY_GATES: status=GREEN passed=3 failed=0 skipped=1 failed_gates=audit

audit is an optional gate. It flags existing hono advisories that are unrelated to this change.

Protected paths touched

none

Reviewer findings addressed

None. The reviewer confirmed that the bound params are in the same order as the ? placeholders, pagination is unchanged, no string splicing is left in src/expenses.ts, and no authz or secrets changed. Full evidence is in the verdict comment on #28.

Note from triage: a code comment cited this defect as "issue #4". If #4 is still open, it's a duplicate of this one.

Human checklist

• AC table has real evidence, not placeholders
• Non-goals respected
• Gate line is GREEN
• Protected paths are "none", or the change to them is expected and reviewed
• I ran the test that bites myself

🤖 Generated with Claude Code