review-code-security

Code security review criteria

Review the worker's code draft against security + correctness checks. Strict: when in doubt, REVISE rather than APPROVE.

Critical checks (any failure = REVISE)

1. No SQL injection vectors:

   • String concatenation into queries: REVISE
   • User input passed to sql.unsafe() without param array: REVISE
   • Drizzle / parameterised queries: OK

2. No secret leakage:

   • Hardcoded API keys, tokens, passwords in the draft: REVISE
   • Logging full request bodies that might contain secrets: REVISE
   • Returning provider error responses verbatim (often contain API keys in URLs): REVISE

3. Auth checks present:

   • Routes that read/write user data must check c.get('userId') or equivalent
   • Routes that mutate must check ownership (the row's userId matches the caller's userId)
   • Admin-only routes must check role

4. No prototype pollution:

   • Object.assign(target, untrustedInput) where target is shared state: REVISE
   • Deep merges of user input into config: REVISE

5. No SSRF:

   • fetch(userProvidedUrl) without scheme/host validation: REVISE
   • URL parsing with allowlist: OK

Quality checks (single failure = REVISE; multiple = REJECT)

1. Error handling at boundaries:

   • External fetches wrapped in try/catch
   • Returns structured errors (not raw provider responses)

2. No silent failures:

   • Empty catch blocks: REVISE
   • Errors logged then swallowed without explanation: REVISE

3. Project conventions:

   • Uses Drizzle, not raw D1 (where available)
   • Uses Hono context, not Express patterns
   • Imports from @/server/... not relative ../../../

When to APPROVE

• All 5 critical checks pass
• All quality checks pass or have one minor REVISE-worthy item that was already addressed

When to REVISE

• 1-2 specific issues from the lists above. Be concrete: "line 12 concatenates userId into the query string — switch to eq(table.userId, userId)"