Differential Review

Security-focused code review that adapts depth to codebase size and change risk. Goes beyond style -- finds vulnerabilities, logic errors, and blast radius.

Review Depth Modes

DEEP (Small codebase, < 5K lines changed)

• Line-by-line analysis of every changed file
• Full control flow tracing through changed paths
• Cross-reference every function call to its definition
• Check all error paths and edge cases

FOCUSED (Medium codebase, 5K-50K lines)

• Prioritize files touching auth, crypto, input parsing, state mutation
• Trace data flow from inputs to outputs through changed code
• Skip cosmetic changes (formatting, comments, renames)
• Deep-dive only on security-sensitive paths

SURGICAL (Large codebase, > 50K lines)

• Review only the diff, not surrounding code
• Focus exclusively on: new attack surface, removed security controls, changed trust boundaries
• Flag anything that needs a separate deep review

Review Process

Phase 1: Blast Radius Assessment

Before reading any code:

# What changed?
git diff --stat <base>...<head>

# How much changed?
git diff --shortstat <base>...<head>

# Which files are security-sensitive?
git diff --name-only <base>...<head> | grep -iE '(auth|crypto|token|secret|permission|middleware|validator|sanitiz)'

Classify the change:

• Surface area: How many files, functions, modules touched?
• Trust boundary crossing: Does data flow between trust levels?
• Security control modification: Are auth/authz/validation/crypto paths changed?
• Data model change: Are schemas, types, or storage formats modified?

Phase 2: Git History Correlation

Check if the changed code has a history of bugs:

# How often has this file been changed? (churn = risk)
git log --oneline --follow <file> | wc -l

# Were there recent security fixes in this area?
git log --oneline --grep="fix\|vuln\|security\|CVE" -- <file>

# Who else has touched this code?
git log --format='%an' -- <file> | sort | uniq -c | sort -rn

High churn + security fix history = increase review depth.

Phase 3: Structured Review

For each changed file, analyze in this order:

1. Input validation: Are new inputs validated? Are existing validations preserved?
2. Authentication/Authorization: Do access controls apply to new code paths?
3. Data flow: Can untrusted data reach sensitive operations?
4. Error handling: Do error paths leak information or skip cleanup?
5. State mutation: Are state changes atomic? Race conditions possible?
6. Crypto usage: Correct algorithms, key sizes, modes, IVs?
7. Logging: Are sensitive values logged? Are security events NOT logged?

Finding Format

## [SEVERITY] Finding Title

**Location**: file.ts:42-58
**Category**: [Input Validation | Auth | Crypto | Data Flow | State | Logic]
**Confidence**: [HIGH | MEDIUM | LOW]

**Description**:
What the vulnerability is, in one paragraph.

**Impact**:
What an attacker can achieve by exploiting this.

**Proof**:
The specific code path or data flow that demonstrates the issue.

**Recommendation**:
Concrete fix with code example if possible.

Severity Classification

Severity	Criteria	Examples
CRITICAL	Remote exploitation, no auth required, data breach	SQL injection, auth bypass, RCE
HIGH	Requires some access, significant impact	Privilege escalation, IDOR, stored XSS
MEDIUM	Limited impact or complex exploitation	Reflected XSS, info disclosure, CSRF
LOW	Minimal impact, defense-in-depth	Missing headers, verbose errors, weak config
INFO	Best practice, no direct vulnerability	Code quality, missing rate limit, logging gap

Rationalizations to Reject

Common excuses that lead to missed findings. Do NOT accept these:

Rationalization	Why It's Wrong	Required Action
"It's behind auth"	Auth can be bypassed	Verify auth is enforced AND correct
"We trust this input"	Trust boundaries change	Validate at every boundary
"It's just internal"	Internal networks get compromised	Apply defense in depth
"Nobody would do that"	Attackers do unexpected things	Test the unexpected case
"We'll fix it later"	Later never comes in security	Flag it NOW with severity
"The framework handles it"	Frameworks have bypasses	Verify the framework actually applies
"It's the same as before"	Before might have been wrong too	Review the original if suspicious

Anti-Hallucination Rules

• Never say "It probably..." -- say "Unclear; need to inspect X"
• Never assume a function is safe without reading its implementation
• Never skip a finding because it seems minor -- document everything
• Every claim must reference a specific file and line number
• If you haven't read the code, say so -- don't infer behavior from names

Diff Review Checklist

[ ] Blast radius assessed (files, trust boundaries, security controls)
[ ] Git history checked for churn and past security fixes
[ ] All new inputs validated
[ ] Auth/authz applied to new endpoints/paths
[ ] Error handling doesn't leak sensitive info
[ ] No hardcoded secrets or credentials
[ ] State mutations are atomic
[ ] Crypto usage follows current best practices
[ ] Logging doesn't include sensitive data
[ ] Removed code didn't contain security controls that are now missing
[ ] Dependencies added/updated are from trusted sources
[ ] Test coverage exists for security-critical paths

Integration with vibecosystem

• code-reviewer agent: Use this skill for security-focused review depth
• security-reviewer agent: Primary consumer of this skill
• coroner agent: Use blast radius analysis for post-mortem propagation
• /review skill: Automatically applies differential review to PRs

Inspired by Trail of Bits differential-review plugin.