Security Review
Evaluate code for security vulnerabilities, focusing on the OWASP Top 10 and secure coding practices.
Cross-reference: security/knowledge-owasp-top-10 for vulnerability details and mitigations.
When to Use
- Before merging code that handles auth, user input, or sensitive data
- When adding new API endpoints or external integrations
- When changing infrastructure, deployment, or secrets configuration
- Periodic security audit of existing code
Severity Levels
| Level |
Meaning |
| CRITICAL |
Exploitable vulnerability — block merge |
| WARNING |
Security weakness that increases attack surface — fix before production |
| SUGGESTION |
Defense-in-depth improvement — implement when practical |
Review Checklist by OWASP Category
A01: Broken Access Control
| Check |
Severity if violated |
| Every endpoint enforces authentication |
CRITICAL |
| Authorization checks use deny-by-default |
CRITICAL |
| No IDOR (Insecure Direct Object References) — users can't access others' resources by changing IDs |
CRITICAL |
CORS is restrictive, not Access-Control-Allow-Origin: * |
WARNING |
| Rate limiting on sensitive endpoints (login, password reset, API) |
WARNING |
| Admin functions are not accessible by guessing URLs |
CRITICAL |
| JWT/session tokens are validated on every request, not just at login |
CRITICAL |
A02: Cryptographic Failures
| Check |
Severity if violated |
| Passwords hashed with bcrypt/scrypt/argon2, never MD5/SHA1 |
CRITICAL |
| Sensitive data encrypted at rest and in transit (TLS) |
CRITICAL |
| No secrets in source code, logs, or error messages |
CRITICAL |
| Encryption keys are rotatable and not hardcoded |
WARNING |
| Sensitive data not stored in JWT payload |
WARNING |
A03: Injection
| Check |
Severity if violated |
| SQL queries use parameterized statements, never string concatenation |
CRITICAL |
| NoSQL queries are parameterized |
CRITICAL |
| OS command execution uses allowlists, never user input directly |
CRITICAL |
| Template rendering escapes user input (XSS prevention) |
CRITICAL |
| LDAP, XML, and path traversal injection vectors are addressed |
CRITICAL |
A04: Insecure Design
| Check |
Severity if violated |
| Threat model exists for security-critical flows |
WARNING |
| Business logic abuse scenarios are considered (e.g., coupon reuse) |
WARNING |
| Security requirements are explicit, not assumed |
SUGGESTION |
A05: Security Misconfiguration
| Check |
Severity if violated |
| Debug mode / verbose errors disabled in production |
CRITICAL |
| Default credentials changed |
CRITICAL |
| Security headers set (CSP, X-Frame-Options, HSTS, X-Content-Type-Options) |
WARNING |
| Unnecessary features/endpoints/ports disabled |
WARNING |
| Directory listing disabled |
WARNING |
A06: Vulnerable Components
| Check |
Severity if violated |
| Dependencies are pinned to specific versions |
WARNING |
| No known CVEs in dependency tree |
CRITICAL |
| Lock files committed and reviewed |
WARNING |
| Unused dependencies removed |
SUGGESTION |
A07: Authentication Failures
| Check |
Severity if violated |
| Multi-factor auth available for sensitive operations |
SUGGESTION |
| Session tokens regenerated after login |
WARNING |
| Session expiry and idle timeout configured |
WARNING |
| Password complexity requirements enforced |
WARNING |
| Brute-force protection (lockout/exponential backoff) |
WARNING |
A08: Data Integrity Failures
| Check |
Severity if violated |
| CI/CD pipeline integrity (signed commits, protected branches) |
WARNING |
| Deserialization of untrusted data is avoided or validated |
CRITICAL |
| Software updates verified with signatures |
SUGGESTION |
A09: Logging and Monitoring Failures
| Check |
Severity if violated |
| Auth events (login, failure, logout) are logged |
WARNING |
| Logs don't contain sensitive data (passwords, tokens, PII) |
CRITICAL |
| Tamper-evident logging in place |
SUGGESTION |
| Alerting on suspicious patterns (repeated auth failures) |
WARNING |
A10: Server-Side Request Forgery (SSRF)
| Check |
Severity if violated |
| User-supplied URLs are validated against an allowlist |
CRITICAL |
| Internal network addresses blocked from user-controlled requests |
CRITICAL |
| URL redirects validated |
WARNING |
Additional Checks
Secrets Management
| Check |
Severity |
| Secrets loaded from env vars or vault, never config files in repo |
CRITICAL |
.env / credentials files in .gitignore |
CRITICAL |
| API keys scoped to minimum required permissions |
WARNING |
| Secrets rotation process documented |
SUGGESTION |
Input Validation
| Check |
Severity |
| All external input validated at system boundary |
CRITICAL |
| Validation is allowlist-based (accept known good), not denylist |
WARNING |
| File uploads validated (type, size, content) |
CRITICAL |
| Request size limits enforced |
WARNING |
Output Format
## Security Review: [Component/Service Name]
**Scope**: [What was reviewed]
**Overall**: [PASS | PASS WITH WARNINGS | FAIL]
### Findings by OWASP Category
#### A03: Injection
##### [CRITICAL] SQL Injection in User Search
**Location**: `src/users/search.ts:87`
**Issue**: User input concatenated into SQL query
**Exploit**: Attacker can extract/modify arbitrary data
**Fix**: Use parameterized query: `db.query('SELECT * FROM users WHERE name = $1', [input])`
...
### Summary
| OWASP Category | Critical | Warning | Suggestion |
|----------------|----------|---------|------------|
| A01: Broken Access Control | 0 | 1 | 0 |
| A03: Injection | 1 | 0 | 0 |
| ... | ... | ... | ... |
| **Total** | **N** | **N** | **N** |
### Recommendations
1. <Prioritized remediation>
2. ...
1---2name: review-security3description: Identify security vulnerabilities, auth weaknesses, injection risks, secrets exposure, and access control issues. Categorizes findings by OWASP Top 10. Produces a severity-rated security review report.4---56# Security Review78Evaluate code for security vulnerabilities, focusing on the OWASP Top 10 and secure coding practices.910**Cross-reference**: `security/knowledge-owasp-top-10` for vulnerability details and mitigations.1112## When to Use1314- Before merging code that handles auth, user input, or sensitive data15- When adding new API endpoints or external integrations16- When changing infrastructure, deployment, or secrets configuration17- Periodic security audit of existing code1819## Severity Levels2021| Level | Meaning |22|-------|---------|23| **CRITICAL** | Exploitable vulnerability — block merge |24| **WARNING** | Security weakness that increases attack surface — fix before production |25| **SUGGESTION** | Defense-in-depth improvement — implement when practical |2627## Review Checklist by OWASP Category2829### A01: Broken Access Control3031| Check | Severity if violated |32|-------|---------------------|33| Every endpoint enforces authentication | CRITICAL |34| Authorization checks use deny-by-default | CRITICAL |35| No IDOR (Insecure Direct Object References) — users can't access others' resources by changing IDs | CRITICAL |36| CORS is restrictive, not `Access-Control-Allow-Origin: *` | WARNING |37| Rate limiting on sensitive endpoints (login, password reset, API) | WARNING |38| Admin functions are not accessible by guessing URLs | CRITICAL |39| JWT/session tokens are validated on every request, not just at login | CRITICAL |4041### A02: Cryptographic Failures4243| Check | Severity if violated |44|-------|---------------------|45| Passwords hashed with bcrypt/scrypt/argon2, never MD5/SHA1 | CRITICAL |46| Sensitive data encrypted at rest and in transit (TLS) | CRITICAL |47| No secrets in source code, logs, or error messages | CRITICAL |48| Encryption keys are rotatable and not hardcoded | WARNING |49| Sensitive data not stored in JWT payload | WARNING |5051### A03: Injection5253| Check | Severity if violated |54|-------|---------------------|55| SQL queries use parameterized statements, never string concatenation | CRITICAL |56| NoSQL queries are parameterized | CRITICAL |57| OS command execution uses allowlists, never user input directly | CRITICAL |58| Template rendering escapes user input (XSS prevention) | CRITICAL |59| LDAP, XML, and path traversal injection vectors are addressed | CRITICAL |6061### A04: Insecure Design6263| Check | Severity if violated |64|-------|---------------------|65| Threat model exists for security-critical flows | WARNING |66| Business logic abuse scenarios are considered (e.g., coupon reuse) | WARNING |67| Security requirements are explicit, not assumed | SUGGESTION |6869### A05: Security Misconfiguration7071| Check | Severity if violated |72|-------|---------------------|73| Debug mode / verbose errors disabled in production | CRITICAL |74| Default credentials changed | CRITICAL |75| Security headers set (CSP, X-Frame-Options, HSTS, X-Content-Type-Options) | WARNING |76| Unnecessary features/endpoints/ports disabled | WARNING |77| Directory listing disabled | WARNING |7879### A06: Vulnerable Components8081| Check | Severity if violated |82|-------|---------------------|83| Dependencies are pinned to specific versions | WARNING |84| No known CVEs in dependency tree | CRITICAL |85| Lock files committed and reviewed | WARNING |86| Unused dependencies removed | SUGGESTION |8788### A07: Authentication Failures8990| Check | Severity if violated |91|-------|---------------------|92| Multi-factor auth available for sensitive operations | SUGGESTION |93| Session tokens regenerated after login | WARNING |94| Session expiry and idle timeout configured | WARNING |95| Password complexity requirements enforced | WARNING |96| Brute-force protection (lockout/exponential backoff) | WARNING |9798### A08: Data Integrity Failures99100| Check | Severity if violated |101|-------|---------------------|102| CI/CD pipeline integrity (signed commits, protected branches) | WARNING |103| Deserialization of untrusted data is avoided or validated | CRITICAL |104| Software updates verified with signatures | SUGGESTION |105106### A09: Logging and Monitoring Failures107108| Check | Severity if violated |109|-------|---------------------|110| Auth events (login, failure, logout) are logged | WARNING |111| Logs don't contain sensitive data (passwords, tokens, PII) | CRITICAL |112| Tamper-evident logging in place | SUGGESTION |113| Alerting on suspicious patterns (repeated auth failures) | WARNING |114115### A10: Server-Side Request Forgery (SSRF)116117| Check | Severity if violated |118|-------|---------------------|119| User-supplied URLs are validated against an allowlist | CRITICAL |120| Internal network addresses blocked from user-controlled requests | CRITICAL |121| URL redirects validated | WARNING |122123## Additional Checks124125### Secrets Management126127| Check | Severity |128|-------|----------|129| Secrets loaded from env vars or vault, never config files in repo | CRITICAL |130| `.env` / credentials files in `.gitignore` | CRITICAL |131| API keys scoped to minimum required permissions | WARNING |132| Secrets rotation process documented | SUGGESTION |133134### Input Validation135136| Check | Severity |137|-------|----------|138| All external input validated at system boundary | CRITICAL |139| Validation is allowlist-based (accept known good), not denylist | WARNING |140| File uploads validated (type, size, content) | CRITICAL |141| Request size limits enforced | WARNING |142143## Output Format144145```markdown146## Security Review: [Component/Service Name]147148**Scope**: [What was reviewed]149**Overall**: [PASS | PASS WITH WARNINGS | FAIL]150151### Findings by OWASP Category152153#### A03: Injection154##### [CRITICAL] SQL Injection in User Search155**Location**: `src/users/search.ts:87`156**Issue**: User input concatenated into SQL query157**Exploit**: Attacker can extract/modify arbitrary data158**Fix**: Use parameterized query: `db.query('SELECT * FROM users WHERE name = $1', [input])`159160...161162### Summary163| OWASP Category | Critical | Warning | Suggestion |164|----------------|----------|---------|------------|165| A01: Broken Access Control | 0 | 1 | 0 |166| A03: Injection | 1 | 0 | 0 |167| ... | ... | ... | ... |168| **Total** | **N** | **N** | **N** |169170### Recommendations1711. <Prioritized remediation>1722. ...173```
DeanKelly751