Caveman Review コードレビュー

I wouldn't merge this yet. There are 3 bugs that will break things, 1 SQL injection, and 1 risk. The original code was correct before this change.

src/orders/service.ts

• L15: 🔴 bug: the loop runs one step too far. i <= order.items.length reads order.items[length], which is undefined, so .price throws a TypeError on every call. Use <, or keep the original reduce, which was correct and shorter.
• L22: 🔴 bug: await was removed from this.coupons.find(code), so coupon is now a Promise. coupon.expiresAt is undefined and undefined < Date.now() is false, so the expiry check never fires and expired coupons are accepted. coupon.id is also undefined. Put the await back.
• L24: 🔴 bug: the update is fire-and-forget and .catch(() => {}) hides any failure. Callers see success even when the coupon was never saved, and nothing is logged. Put back await this.repo.update(...) and let errors reach the caller.
• L22-23: 🟡 risk: if find returns null for an unknown code, coupon.expiresAt throws a TypeError instead of a clear error. Add if (!coupon) throw new Error("invalid coupon").

L28: 🔴 security: SQL injection in listOrders. userId is put straight into the SQL string. A value like ' OR '1'='1 returns every user's orders, and with stacked queries an attacker could change or delete data (CWE-89, OWASP A03). Even if userId comes from an auth token today, this function can't assume that, and a single caller passing a request param makes it exploitable. Use a parameterized query, e.g. this.db.query("SELECT * FROM orders WHERE user_id = $1", [userId]) (use whatever placeholder syntax your driver expects).

• L28: 🔵 nit: SELECT * returns every column and will pick up any column added later. List the columns you need.
• L27: 🔵 nit: no return type, unlike getOrderTotal. Add Promise<Order[]>.