Code Review

import hashlib

def authenticate(username, password, db):
    query = f"SELECT password_hash FROM users WHERE username = '{username}'"
    row = db.execute(query).fetchone()
    if row is None:
        return False
    stored_hash = row[0]
    input_hash = hashlib.md5(password.encode()).hexdigest()
    return input_hash == stored_hash

## Review: src/auth.py — authenticate()

### Critical — SQL Injection (line 4)
The query uses f-string interpolation with `username` directly in the SQL
string. An attacker can pass `' OR '1'='1` as the username to bypass
authentication entirely.

**Fix:** Use parameterized queries.
- row = db.execute(query).fetchone()
+ row = db.execute(
+     "SELECT password_hash FROM users WHERE username = ?", (username,)
+ ).fetchone()

### Critical — Weak Hashing Algorithm (line 9)
MD5 is cryptographically broken and unsuitable for password hashing. It is
vulnerable to rainbow table attacks and can be brute-forced in seconds on
modern hardware.

**Fix:** Use bcrypt or argon2 with a per-user salt.
- input_hash = hashlib.md5(password.encode()).hexdigest()
+ import bcrypt
+ return bcrypt.checkpw(password.encode(), stored_hash.encode())

### Warning — Timing Attack (line 10)
Direct string comparison of hashes with `==` leaks information through
timing differences. Use a constant-time comparison function.

**Fix:**
- return input_hash == stored_hash
+ import hmac
+ return hmac.compare_digest(input_hash, stored_hash)

### Info — Missing Type Annotations
Adding type hints improves readability and enables static analysis:
  def authenticate(username: str, password: str, db: Connection) -> bool:

**Summary:** 2 critical, 1 warning, 1 info. This function must not be
deployed until the SQL injection and hashing issues are resolved.

@@ -12,6 +12,20 @@ const express = require("express");
 const router = express.Router();

+router.get("/orders", async (req, res) => {
+  const userId = req.query.userId;
+  const orders = await db.query("SELECT * FROM orders WHERE user_id = $1", [userId]);
+  const enriched = [];
+  for (const order of orders.rows) {
+    const items = await db.query("SELECT * FROM order_items WHERE order_id = $1", [order.id]);
+    order.items = items.rows;
+    enriched.push(order);
+  }
+  res.json(enriched);
+});

## Review: src/api/orders.js — GET /orders

### Warning — N+1 Query (lines 18-21)
Each order triggers a separate SELECT for its items. For a user with 200
orders this executes 201 database queries. This will degrade significantly
under load.

**Fix:** Use a single JOIN or a batched IN query:
  const items = await db.query(
    "SELECT * FROM order_items WHERE order_id = ANY($1)",
    [orders.rows.map(o => o.id)]
  );
  // Then group items by order_id in application code.

### Warning — Missing Authorization Check (line 14)
The endpoint reads `userId` from the query string without verifying that
the authenticated user is allowed to view that user's orders. Any user
can read any other user's order history by changing the query parameter.

**Fix:** Validate that `req.user.id === userId` or that the requester
has an admin role before executing the query.

### Info — SELECT * Usage (lines 16, 19)
Selecting all columns returns data the client may not need (e.g.,
internal timestamps, soft-delete flags). Specify only the required columns
to reduce payload size and avoid leaking internal fields.

**Summary:** 0 critical, 2 warning, 1 info.