Security Review

Security Review

Overview

Dedicated security review for code handling authentication, authorization, user input, APIs, databases, or credentials.

Core principle: Security issues require specialized attention beyond general code review.

Trigger: This review is MANDATORY when changes touch security-sensitive paths.

Announce at start: "I'm performing a security review of this code."

When Required

This skill is MANDATORY when ANY of these files are modified:

Check with:

git diff --name-only HEAD~1 | grep -E '(auth|security|middleware|api|password|token|secret|credential|session|routes|\.sql)'

OWASP Top 10 Checklist

Review against each category:

1. Injection (A03:2021)

// VULNERABLE
db.query(`SELECT * FROM users WHERE id = '${userId}'`);

// SECURE
db.query('SELECT * FROM users WHERE id = ?', [userId]);

2. Broken Authentication (A07:2021)

3. Sensitive Data Exposure (A02:2021)

# Check for hardcoded secrets
grep -rE '(password|secret|api_key|token)\s*[:=]\s*["\047][^"\047]+["\047]' src/

4. XML External Entities (A05:2021)

5. Broken Access Control (A01:2021)

6. Security Misconfiguration (A05:2021)

7. Cross-Site Scripting (A03:2021)

// VULNERABLE
element.innerHTML = userInput;

// SECURE
element.textContent = userInput;
// OR
element.innerHTML = DOMPurify.sanitize(userInput);

8. Insecure Deserialization (A08:2021)

9. Using Components with Known Vulnerabilities (A06:2021)

# Run dependency audit
pnpm audit --prod
# or
pip-audit

10. Insufficient Logging & Monitoring (A09:2021)

Review Process

Step 1: Identify Security-Sensitive Changes

# List changed files matching security patterns
git diff --name-only HEAD~1 | grep -E '(auth|security|middleware|api|password|token|secret|session)'

Step 2: Review Each File

For each security-sensitive file:

1. Read the entire file (not just diff)
2. Check against all 10 OWASP categories
3. Note any findings with severity

Step 3: Check Dependencies

# Audit dependencies
pnpm audit --prod

# Check for outdated
pnpm outdated

Step 4: Document Findings

Use severity levels:

Step 5: Add Security Section to Review Artifact

Add to the main review artifact:

### Security Review

**Security-Sensitive:** YES
**Reviewed By:** [WORKER_ID or security-reviewer subagent]
**OWASP Categories Checked:** 10/10

#### Security Findings

| # | OWASP Category | Severity | Finding | Status |
|---|----------------|----------|---------|--------|
| 1 | A03 Injection | CRITICAL | SQL injection in findUser() | FIXED |
| 2 | A02 Sensitive Data | HIGH | API key in config.ts | DEFERRED #456 |
| 3 | A01 Access Control | MEDIUM | Missing auth on /admin | FIXED |

#### Dependency Audit

pnpm audit: 0 vulnerabilities

**Security Review Status:** [PASS|ISSUES_FIXED|ISSUES_DEFERRED]

Security Review Artifact (Standalone)

If security review is extensive, post as separate comment:

<!-- SECURITY_REVIEW:START -->
## Security Review

| Property | Value |
|----------|-------|
| Issue | #123 |
| Reviewer | `security-reviewer` subagent |
| Reviewed | 2025-12-29T10:30:00Z |

### Files Reviewed

- src/auth/login.ts
- src/middleware/authenticate.ts
- src/api/users.ts

### OWASP Checklist Results

| # | Category | Status | Notes |
|---|----------|--------|-------|
| A01 | Broken Access Control | PASS | Auth middleware on all protected routes |
| A02 | Cryptographic Failures | PASS | bcrypt for passwords, TLS enforced |
| A03 | Injection | FIXED | Parameterized SQL queries now |
| A04 | Insecure Design | PASS | - |
| A05 | Security Misconfiguration | PASS | - |
| A06 | Vulnerable Components | PASS | pnpm audit clean |
| A07 | Auth Failures | PASS | Rate limiting, secure sessions |
| A08 | Data Integrity Failures | PASS | - |
| A09 | Logging Failures | NOTE | Consider adding auth failure logging |
| A10 | SSRF | N/A | No server-side requests |

### Dependency Audit

found 0 vulnerabilities

**Security Review Status:** PASS
<!-- SECURITY_REVIEW:END -->

Checklist

Before completing security review:

• All changed security-sensitive files reviewed
• All 10 OWASP categories checked
• Dependency audit run
• All CRITICAL/HIGH findings fixed
• Any deferred findings have tracking issues
• Security section added to review artifact
• Security-Sensitive marked YES in main artifact

Integration

This skill is triggered by:

• Changes to security-sensitive file patterns
• Explicit invocation
• security-reviewer subagent

This skill integrates with:

• comprehensive-review - Security is criterion #4
• review-gate - Verifies security review for sensitive changes

This skill is enforced by:

• .claude/rules/security-sensitive.md conditional rules
• PreToolUse hook (PR blocked if security-sensitive without review)