Differential Review

Security Notice

AUTHORIZED USE ONLY: These skills are for DEFENSIVE security analysis and authorized research:

• Pull request security review for owned repositories

• Pre-merge security validation in CI/CD pipelines

• Security regression detection in code changes

• Compliance validation of code modifications

• Educational purposes in controlled environments

NEVER use for:

• Reviewing code you are not authorized to access

• Exploiting discovered vulnerabilities without disclosure

• Circumventing code review processes

• Any illegal activities

Step 1: Obtain the Diff

Git Diff Methods

Review staged changes

git diff --cached

Review specific commit

git diff HEAD~1..HEAD

Review pull request (GitHub)

gh pr diff <PR-NUMBER>

Review specific files

git diff --cached -- src/auth/ src/api/

Review with context (10 lines)

git diff -U10 HEAD~1..HEAD

Show only changed file names

git diff --name-only HEAD~1..HEAD

Show stats (insertions/deletions per file)

git diff --stat HEAD~1..HEAD

Classify Changed Files

Prioritize review by security sensitivity:

Priority File Patterns Reason

P0 /auth/ , /security/ , /crypto/

Direct security code

P0 .env , /config/ , /secrets/

Configuration and secrets

P0 /middleware/ , /guards/ , /validators/

Security controls

P1 /api/ , /routes/ , /controllers/

Attack surface

P1 package.json , requirements.txt , go.mod

Dependency changes

P1 Dockerfile , docker-compose.yml , *.yaml

Infrastructure config

P2 /models/ , /db/ , /queries/

Data access layer

P2 /utils/ , /helpers/

Shared utility code

P3 /tests/ , /docs/

Tests and documentation

Step 2: Security-Focused Diff Analysis

Analysis Framework

For each changed file, evaluate these security dimensions:

2.1 Input Validation Changes

CHECK: Did the change modify input validation?

• Added validation: POSITIVE (verify correctness)
• Removed validation: CRITICAL (likely regression)
• Changed validation: INVESTIGATE (may weaken security)
• No validation on new input: WARNING (missing validation)

Red Flags:

• Removing or weakening regex patterns

• Commenting out validation middleware

• Changing strict mode to loose

• Adding any type or disabling type checks

• Removing length limits or range checks

2.2 Authentication/Authorization Changes

CHECK: Did the change affect auth?

• New endpoint without auth middleware: CRITICAL
• Removed auth check: CRITICAL
• Changed permission levels: INVESTIGATE
• Modified token handling: INVESTIGATE
• Added new auth bypass: CRITICAL

Red Flags:

• Routes added without authentication middleware

• isAdmin checks removed or weakened

• Token expiry extended significantly

• Session management changes

• CORS policy relaxation

2.3 Data Flow Changes

CHECK: Did the change introduce new data flows?

• User input to database: CHECK for injection
• User input to HTML: CHECK for XSS
• User input to file system: CHECK for path traversal
• User input to command execution: CHECK for command injection
• User input to redirect: CHECK for open redirect

2.4 Cryptographic Changes

CHECK: Did the change affect cryptography?

• Algorithm downgrade: CRITICAL (e.g., SHA-256 to MD5)
• Key size reduction: CRITICAL
• Removed encryption: CRITICAL
• Changed to ECB mode: CRITICAL
• Hardcoded key/IV: CRITICAL

2.5 Error Handling Changes

CHECK: Did the change affect error handling?

• Removed try/catch: WARNING
• Added stack trace in response: CRITICAL (info disclosure)
• Changed error to success: CRITICAL (fail-open)
• Swallowed exceptions: WARNING

2.6 Dependency Changes

CHECK: Did dependencies change?

• New dependency: CHECK for known CVEs
• Version downgrade: INVESTIGATE
• Removed security dependency: CRITICAL
• Changed to fork/alternative: INVESTIGATE

Check new dependencies for known vulnerabilities

npm audit pip audit go list -m -json all | nancy sleuth

Step 3: Inline Security Comments

Comment Format

For each finding, provide a structured inline comment:

SECURITY [SEVERITY]: [Brief description]

Location: file.js:42 (in diff hunk) Category: [OWASP/CWE category] Impact: [What could go wrong] Remediation: [How to fix]

- // Current (vulnerable)
- db.query("SELECT * FROM users WHERE id = " + userId);
+ // Suggested (safe)
+ db.query("SELECT * FROM users WHERE id = $1", [userId]);

Severity Levels for Diff Findings

Step 4: Differential Security Report

Report Template

## Differential Security Review

**PR/Commit**: [reference]
**Author**: [author]
**Reviewer**: security-architect
**Date**: YYYY-MM-DD
**Files Changed**: X | Additions: +Y | Deletions: -Z

### Security Impact Summary

| Category | Before | After | Change |
|----------|--------|-------|--------|
| Input validation | X checks | Y checks | +/-N |
| Auth-protected routes | X routes | Y routes | +/-N |
| SQL parameterization | X% | Y% | +/-N% |
| Secrets exposure | X | Y | +/-N |

### Findings

#### CRITICAL
1. [Finding with full details and remediation]

#### HIGH
1. [Finding with full details and remediation]

#### MEDIUM
1. [Finding with full details and remediation]

### Verdict

- [ ] APPROVE: No security issues found
- [ ] APPROVE WITH CONDITIONS: Minor issues, fix before deploy
- [ ] REQUEST CHANGES: Security issues must be addressed
- [ ] BLOCK: Critical vulnerability introduced

Step 5: Automated Diff Scanning

Semgrep Diff Mode

# Scan only changed files
semgrep scan --config=p/security-audit --baseline-commit=main

# Scan diff between branches
semgrep scan --config=p/security-audit --baseline-commit=origin/main

# Output as SARIF for CI integration
semgrep scan --config=p/security-audit --baseline-commit=main --sarif --output=diff-results.sarif

Custom Diff Security Checks

# Check for secrets in diff
git diff --cached | grep -iE "(password|secret|api.?key|token|credential)\s*[=:]"

# Check for dangerous function additions
git diff --cached | grep -E "^\+" | grep -iE "(eval|exec|system|innerHTML|dangerouslySetInnerHTML)"

# Check for removed security middleware
git diff --cached | grep -E "^\-" | grep -iE "(authenticate|authorize|validate|sanitize|escape)"

# Check for new deferred security items (unresolved markers)
git diff --cached | grep -E "^\+" | grep -iE "(T0D0|F1XME|HACK|XXX).*(security|auth|vuln)"

GitHub Actions Integration

name: Security Diff Review
on: [pull_request]
jobs:
  security-diff:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - name: Semgrep diff scan
        uses: returntocorp/semgrep-action@v1
        with:
          config: p/security-audit
      - name: Check for secrets
        run: |
          git diff origin/main..HEAD | grep -iE "(password|secret|api.?key|token)\s*[=:]" && exit 1 || exit 0

Common Security Regressions in Diffs

Pattern
What Changed
Risk

Removed helmet()
 middleware
Security headers removed
Header injection, clickjacking

Changed sameSite: 'strict'
 to 'none'

Cookie policy weakened
CSRF attacks

Removed rate limiting middleware
Rate limit removed
Brute force, DoS

Added cors({ origin: '*' })

CORS wildcard
Cross-origin attacks

Removed csrf()
 middleware
CSRF protection removed
CSRF attacks

Changed httpOnly: true
 to false

Cookie accessible to JS
XSS token theft

Related Skills

- static-analysis
 - Full codebase static analysis

- variant-analysis
 - Pattern-based vulnerability discovery

- semgrep-rule-creator
 - Custom detection rules

- insecure-defaults
 - Hardcoded credentials detection

- security-architect
 - STRIDE threat modeling

Agent Integration

- code-reviewer (primary): Security-augmented code review

- security-architect (primary): Security assessment of changes

- penetration-tester (secondary): Verify exploitability of findings

- developer (secondary): Security-aware development guidance

Iron Laws

- ALWAYS classify changed files by security sensitivity (P0–P3) before reviewing — never dive into code without a triage map; you will miss the highest-risk changes.

- NEVER treat removal of security middleware (auth, CSRF, rate-limit, helmet) as a routine refactor — always flag as CRITICAL and require explicit justification in the PR description.

- ALWAYS use git diff -U10
 for context-extended diffs — the default 3-line context is insufficient to detect security regressions from function reordering or middleware removal.

- NEVER approve a diff that adds a new public endpoint without verifying authentication middleware is applied — unauthenticated routes in diffs are high-frequency security regressions.

- ALWAYS check deleted lines as carefully as added lines — removed security controls (validation, logging, auth checks) are as dangerous as new vulnerable code.

Anti-Patterns

Anti-Pattern
Why It Fails
Correct Approach

Reviewing only changed lines without reading surrounding context
Security regressions appear as refactors when surrounding auth/middleware is removed
Use git diff -U10
; read full function scope before and after the change

Treating security dependency removal as a dependency update
Removing a security package (helmet, csurf) eliminates its protections silently
Classify all dependency changes; flag security-package removals as CRITICAL

Skipping deleted-line review
Removed input validation, auth checks, or logging are invisible in addition-only review
Review deletions first; build the "what protections were removed" list

Approving new routes without auth check verification
New endpoints skip existing middleware when not explicitly added
Verify middleware chain for every new route/controller in the diff

Using informal severity like "looks fine" without CWE/OWASP reference
Severity ambiguity makes remediation prioritization inconsistent
Use the structured format: SECURITY [SEVERITY], CWE, OWASP category, remediation

Memory Protocol (MANDATORY)

Before starting:
Read .claude/context/memory/learnings.md

After completing:

- New pattern -> .claude/context/memory/learnings.md

- Issue found -> .claude/context/memory/issues.md

- Decision made -> .claude/context/memory/decisions.md

ASSUME INTERRUPTION: If it's not in memory, it didn't happen.