Security Review
security-review
Security-focused code review with attack surface mapping and risk classification. Use when reviewing PRs for security, auditing code changes, or analyzing potential vulnerabilities. Triggers on: 'security review', 'use security mode', 'audit this', 'check for vulnerabilities', 'is this secure', '...
SKILL.md
Full skill instructions
Security Review
Systematic security analysis of code changes.
Core Approach
"Assume the user is the attacker. Find where trust is misplaced."
Risk Classification
| Risk Level | Triggers |
|---|---|
| HIGH | Auth, crypto, external calls, value transfer, validation removal |
| MEDIUM | Business logic, state changes, new public APIs |
| LOW | Comments, tests, UI, logging |
Attack Surface Mapping
For each change, identify:
- User inputs - request params, headers, body, URL components
- Database queries - any SQL/ORM operations
- Auth/authz checks - where permissions are verified
- External calls - APIs, services, file system
- Cryptographic operations - hashing, encryption, tokens
Security Checklist
Input Validation
- All user input validated before use
- Validation happens at trust boundary (not just client)
- Type coercion handled safely
- Size/length limits enforced
Authentication/Authorization
- Auth checks present on all protected paths
- No privilege escalation paths
- Session handling is secure
- Token expiration enforced
Data Exposure
- No secrets in logs or responses
- Sensitive data filtered from error messages
- PII handling follows policy
- Debug endpoints disabled in production
Injection Prevention
- Parameterized queries for SQL
- Output encoding for XSS
- Command injection prevented
- Path traversal blocked
Cryptography
- No custom crypto implementations
- Strong algorithms used (no MD5/SHA1 for security)
- Secrets not hardcoded
- Key rotation possible
Blast Radius Analysis
For HIGH risk changes:
- Count direct callers
- Trace transitive dependencies
- Identify failure modes
- Check rollback feasibility
- Assess data exposure scope
Red Flags (Stop and Escalate)
- 🔴 Removed validation without replacement
- 🔴 Access control modifiers weakened
- 🔴 External calls added without error handling
- 🔴 Crypto operations changed
- 🔴 Auth bypass paths introduced
- 🔴 Secrets in source code
- 🔴
eval()or dynamic code execution - 🔴 Disabled security controls (even "temporarily")
Common Vulnerability Patterns
| Pattern | Look For |
|---|---|
| IDOR | User-controlled IDs without ownership check |
| Mass Assignment | Binding request body directly to models |
| SSRF | User-controlled URLs in server requests |
| Path Traversal | User input in file paths without sanitization |
| Race Condition | Check-then-use without locking |
| Insecure Deserialization | Deserializing untrusted data |
Output Format
For each finding:
**File**: `path/to/file.py:42`
**Risk**: HIGH | MEDIUM | LOW
**Category**: [Input Validation | Auth | Data Exposure | Injection | Crypto]
**Issue**: [Brief description of what's wrong]
**Evidence**: [Specific code or pattern that demonstrates the issue]
**Recommendation**: [What should be done - without implementing it]
Review Summary Template
## Security Review Summary
**Scope**: [Files/changes reviewed]
**Risk Level**: [Overall: HIGH/MEDIUM/LOW]
### Attack Surface
- Inputs: [list]
- External calls: [list]
- Auth points: [list]
### Findings
| # | Risk | Category | File:Line | Issue |
| --- | ---- | -------- | ---------- | ------------------------ |
| 1 | HIGH | Auth | file.py:42 | Missing permission check |
### Recommendations
1. [Priority-ordered list of fixes]
### Not Reviewed
[Areas that need separate review or were out of scope]
What NOT to Do
- ❌ Fix the issues (identify only)
- ❌ Assume "internal only" means safe
- ❌ Skip test files (they often reveal behavior)
- ❌ Trust comments that say "safe" or "validated elsewhere"
- ❌ Ignore configuration files
The Security Reviewer's Creed
"I'm not here to approve—I'm here to find what's missed."
Trust nothing. Verify everything. Document clearly.