Security Review

Run before merging any code that handles authentication, authorization, payments, user input, file uploads, or external integrations. Check every item. Flag findings as CRITICAL (block merge), HIGH (request changes), MEDIUM (note in review), or LOW (optional).

Authentication & sessions

• Passwords hashed with bcrypt / argon2 / scrypt — never SHA family alone.
• Session tokens are cryptographically random (≥128 bits) and rotate on privilege change.
• Cookies set with HttpOnly, Secure, and SameSite=Lax (or Strict where it doesn't break flow).
• No JWTs in localStorage for browser apps. Use HttpOnly cookies.
• Token expiry enforced both client and server side.
• Account lockout or rate limit on login (e.g. 5 failed attempts in 10 min).
• MFA available and required for high-value operations.

Authorization

• Every endpoint that mutates state checks the caller's permission, not just authentication.
• Object-level access enforced (cannot access /orders/:id for another user's order).
• Role-based decisions live in a single policy module, not scattered through handlers.
• No client-supplied role / permission claims trusted server-side.

Input validation

• All external input parsed via schema (zod, pydantic, etc.) at the boundary.
• Allowlist > denylist for fields, types, and enum values.
• Length limits on every string field.
• File uploads check MIME type, magic bytes, AND size.
• User-supplied URLs blocked from internal IP ranges (SSRF prevention).

Injection

• SQL uses parameterized queries / ORM bind parameters — never string concatenation.
• HTML rendering uses framework escaping. Avoid React's raw-HTML escape hatch (dangerously* prop), Vue's v-html, and direct innerHTML assignment when the content includes any user data.
• Shell commands built from arrays, not concatenated strings. Better: avoid exec entirely.
• LDAP, XPath, NoSQL queries use the language's safe-API path.
• No eval, Function(), or dynamic import() of user-controlled paths.

Cross-site

• CSRF protection on every state-changing endpoint (token, double-submit cookie, or SameSite=Strict).
• CORS allowlist is explicit. Never Access-Control-Allow-Origin: * with credentials.
• Content Security Policy header set in production with nonce-based script policy.

Secrets

• No secrets in source code, comments, commit history, or test fixtures.
• Secrets read from environment variables or a secret manager.
• App fails fast at startup if a required secret is missing — never falls back to a default.
• .env* files in .gitignore.
• Logs scrubbed of tokens, API keys, full credit card numbers.

Cryptography

• Use a vetted library (libsodium, Web Crypto, .NET CryptographyNG). Never roll your own.
• AES-GCM or ChaCha20-Poly1305 for symmetric encryption. Never ECB or CBC without HMAC.
• HMAC comparisons use constant-time equality (timingSafeEqual).
• TLS 1.2+ only. No SSLv3, TLS 1.0, TLS 1.1.

Dependencies

• npm audit / pip-audit / cargo audit run in CI; high-severity findings block merge.
• Lockfiles committed and respected (npm ci, pnpm install --frozen-lockfile).
• No packages added without a quick threat-model: who maintains it, how often updated, transitive deps.

Logging & errors

• Error messages returned to users contain no internal paths, stack traces, or DB errors.
• Detailed errors logged server-side with request ID for correlation.
• PII redacted from logs (emails, addresses, full names where unnecessary).
• Audit log for security-relevant events: login, password change, role change, data export.

File handling

• User-supplied paths sanitized — block .., absolute paths, null bytes.
• Uploaded files stored outside the web root, served via a signed URL endpoint that re-checks auth.
• Image processing uses a sandbox or vetted library (libvips, sharp). Never ImageMagick policy-default.

Output

End the review with a one-line verdict: APPROVE, REQUEST CHANGES (list CRITICAL / HIGH only), or BLOCK (CRITICAL findings — merge would expose users).