Senior code review that catches real bugs

See it in action

What this prompt produces with example details filled in:

Verdict: Request changes.

Findings (worst first):

1. Blocker — Broken access control (IDOR): any user can read anyone's orders.
Location: `const userId = req.query.userId;` used in the WHERE clause.
Why it's wrong: The handler trusts the client-supplied `userId` query parameter instead of the authenticated user's id. Failure scenario: an authenticated attacker calls `GET /orders?userId=1043` and receives user 1043's entire order history. Being "behind auth middleware" only proves the caller is logged in — it does not scope them to their own data.
Confidence: Confirmed.
Fix: Derive the id from the verified session/token, never the query string: `const userId = req.user.id;` and remove `req.query.userId` entirely.

2. Blocker — SQL injection in the orders query.
Location: `` `SELECT * FROM orders WHERE user_id = ${userId}` ``
Why it's wrong: Client input is interpolated straight into SQL. Failure scenario: `GET /orders?userId=0 OR 1=1` returns every order in the table; on a driver that allows multiple statements, `userId=0; DROP TABLE orders;--` is catastrophic.
Confidence: Confirmed.
Fix: Use a parameterized query: `db.query('SELECT ... FROM orders WHERE user_id = $1', [userId])`.

3. Blocker — Off-by-one crashes every request.
Location: `for (let i = 0; i <= orders.length; i++)`
Why it's wrong: `<=` runs one iteration past the end. On the final pass `orders[i]` is `undefined`, so `orders[i].id` throws `TypeError: Cannot read properties of undefined (reading 'id')`. Failure scenario: any call that reaches the loop — including one returning zero orders — throws and returns a 500.
Confidence: Confirmed.
Fix: Use `i < orders.length`.

4. High — N+1 query pattern.
Location: the per-order `SELECT * FROM order_items` inside the loop.
Why it's wrong: One extra database round-trip per order; 200 orders means 201 sequential queries and latency that grows linearly with history size. Failure scenario: a heavy user makes the endpoint slow and hammers the DB under load.
Confidence: Confirmed.
Fix: Fetch all items in one query and group in memory: `SELECT * FROM order_items WHERE order_id = ANY($1)` with the list of order ids, then bucket by `order_id` — or use a single JOIN.

5. Medium — Unhandled rejection on the async route.
Location: the whole handler.
Why it's wrong: If either `db.query` rejects, the rejection isn't caught. Failure scenario: on Express 4 it never reaches an error handler, so the client's request hangs until timeout and you get an unhandled-rejection log.
Confidence: Plausible (depends on your Express version and error middleware).
Fix: Wrap the body in try/catch with `res.status(500).json(...)`, use an async-error wrapper, or move to Express 5.

6. Low — `SELECT *` over-fetches and leaks schema.
Location: both queries.
Why it's wrong: Returns every column (including internal, soft-deleted, or PII fields) and breaks silently when the schema changes.
Fix: Select the explicit columns the client actually needs.

Looks good: the routing shape is clean, the response is assembled in one place, and enriching each order with its items is the right API shape — keep that structure once the query is de-N+1'd.

Missing context I'd want: confirmation that `req.user` is populated by the auth middleware, and whether the DB driver permits multiple statements (that raises finding 2 from severe to catastrophic).