Security code review

// ❌ VULNERABLE: Direct string interpolation
const getUser = async (req: Request, res: Response) => {
    const userId = req.query.id as string;
    // Attacker can inject: 1 OR 1=1
    const query = `SELECT * FROM users WHERE id = '${userId}'`;
    const result = await db.query(query);
    res.json(resu

lt.rows); };

**Secure Implementation:** Parameterized queries separate code from data.

```typescript
// ✅ SECURE: Parameterized query
const getUser = async (req: Request, res: Response) => {
    const userId = req.query.id as string;
    
    // Validate input type and format
    if (!/^\d+$/.test(userId)) {
        return res.status(400).json({ error: 'Invalid ID format' });
    }

    // Use parameters to prevent injection
    const query = 'SELECT * FROM users WHERE id = $1';
    const result = await db.query(query, [userId]);
    res.json(result.rows);
};

Review Check: Verify all database interactions use parameterization. Check for ORM misuse that might generate raw SQL.

Example 2: Insecure Random Number Generation

Vulnerable Pattern: Math.random() is predictable and unsuitable for security tokens.

// ❌ VULNERABLE: Predictable RNG
const generateResetToken = () => {
    return Math.random().toString(36).substring(2);
};

Secure Implementation: Use cryptographically secure random values.

// ✅ SECURE: Cryptographic RNG
import { randomBytes } from 'crypto';

const generateResetToken = (): string => {
    return randomBytes(32).toString('hex');
};

Review Check: Scan for Math.random, Date.now(), or predictable seeds in security contexts. Ensure crypto module usage.

Example 3: Authorization Bypass

Vulnerable Pattern: Client-supplied role used for authorization decisions.

// ❌ VULNERABLE: Trusting client input
const updateUser = async (req: Request, res: Response) => {
    const { userId, role } = req.body;
    // Attacker can set role to 'admin'
    await db.updateUser(userId, { role });
    res.status(200).send('Updated');
};

Secure Implementation: Authorization based on server-side session/token claims.

// ✅ SECURE: Server-side authorization
const updateUser = async (req: Request, res: Response) => {
    const { userId, role } = req.body;
    const requesterRole = req.user.role; // From verified JWT/session

    // Enforce business rule: Only admins can change roles
    if (requesterRole !== 'admin') {
        return res.status(403).json({ error: 'Forbidden' });
    }

    // Validate allowed role values
    if (!['user', 'moderator'].includes(role)) {
        return res.status(400).json({ error: 'Invalid role' });
    }

    await db.updateUser(userId, { role });
    res.status(200).send('Updated');
};

Review Check: Ensure authorization checks use trusted server-side state. Verify role assignments are restricted to authorized actors.

Architecture Decisions

• Review Integration: Embed security checks in CI/CD. Use GitHub Actions or GitLab CI to enforce review gates.
• Reviewer Rotation: Rotate reviewers to prevent blindness and spread security knowledge.
• Checklist-Driven: Use a dynamic checklist generated from the threat model to guide reviews.
• Feedback Loop: Log review findings to train developers and improve automated rules.

Pitfall Guide

1. Relying Solely on Automated Tools

Mistake: Assuming a clean SAST scan means secure code. Explanation: Tools miss logic flaws, race conditions, and context-dependent vulnerabilities. Manual review is essential for high-risk modules. Best Practice: Use SAST for filtering; reserve manual review for business logic and trust boundaries.

2. Reviewing Without Context

Mistake: Analyzing code in isolation without understanding the threat model. Explanation: Reviewers may focus on low-risk issues while missing critical attacks relevant to the system. Best Practice: Always review against the threat model. Know the assets and trust boundaries before starting.

3. Ignoring Third-Party Dependencies

Mistake: Focusing only on custom code and ignoring libraries. Explanation: Vulnerabilities in dependencies can compromise the entire application. Best Practice: Integrate SCA tools. Review dependency updates and known CVEs during code review.

4. "Rubber Stamping" Reviews

Mistake: Approving PRs without thorough analysis due to pressure. Explanation: Creates a culture where security is a formality, not a practice. Best Practice: Empower reviewers to block merges. Implement metrics on review quality, not just speed.

5. Focusing Only on Syntax

Mistake: Checking for typos and style instead of security logic. Explanation: Syntax errors are caught by compilers; security flaws require semantic analysis. Best Practice: Use linters for syntax. Focus reviews on data flow, validation, and authorization.

6. Missing Configuration Files

Mistake: Reviewing only source code and ignoring infrastructure-as-code or config. Explanation: Misconfigurations in CI/CD, cloud resources, or environment variables are common attack vectors. Best Practice: Include IaC and configuration files in the review scope. Check for hardcoded secrets.

7. Delayed Feedback

Mistake: Reviewing code days after submission. Explanation: Context is lost, and remediation becomes harder. Best Practice: Integrate reviews into the PR flow. Provide feedback within hours.

Production Bundle

Action Checklist

• Define Scope: Identify high-risk modules and trust boundaries based on threat model.
• Run Pre-screen: Execute SAST/SCA tools and resolve critical findings before manual review.
• Validate Inputs: Verify all external inputs are validated against strict whitelists.
• Check AuthZ: Ensure authorization checks use server-side state and are enforced close to data.
• Audit Crypto: Confirm use of approved algorithms and secure key management.
• Review Error Handling: Ensure no sensitive data is leaked in logs or responses.
• Inspect Dependencies: Check for known vulnerabilities in third-party libraries.
• Verify Config: Review environment variables and IaC for security misconfigurations.

Decision Matrix

Scenario	Recommended Approach	Why	Cost Impact
Startup MVP	Lightweight Checklist + SAST	Speed is critical; focus on critical vulns only.	Low
Enterprise Legacy	Full Manual Review + SAST + Threat Model	High risk; complex logic; compliance requirements.	High
Open Source	Automated Gates + Community Review	Scalability; transparency; diverse reviewer base.	Medium
High-Frequency Trading	Formal Verification + Manual Review	Zero tolerance for latency or flaws; math proofs.	Very High
Regulated Industry	Compliance-Driven Review + Audit Trail	Legal requirements; need for evidence and traceability.	High

Configuration Template

GitHub Actions Workflow for Security Review:

name: Security Review Pipeline

on:
  pull_request:
    branches: [ main ]

jobs:
  security-scan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      
      - name: Run Semgrep SAST
        uses: returntocorp/semgrep-action@v1
        with:
          config: >-
            p/security-audit
            p/owasp-top-ten
        env:
          SEMGREP_APP_TOKEN: ${{ secrets.SEMGREP_APP_TOKEN }}

      - name: Run SCA Check
        uses: aquasecurity/trivy-action@master
        with:
          scan-type: 'fs'
          severity: 'CRITICAL,HIGH'
          
      - name: Enforce Review Gate
        if: failure()
        run: exit 1

  security-review:
    runs-on: ubuntu-latest
    needs: security-scan
    steps:
      - name: Comment PR with Review Checklist
        uses: actions/github-script@v6
        with:
          script: |
            github.rest.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: '🔒 **Security Review Required**\n\nPlease verify:\n- [ ] Input validation\n- [ ] Authorization logic\n- [ ] Crypto usage\n- [ ] Error handling\n- [ ] Dependencies\n\nRefer to the threat model for context.'
            })

Quick Start Guide

1. Select Tools: Integrate a SAST tool (e.g., Semgrep, CodeQL) and SCA tool into your CI/CD pipeline. Configure rules to match your stack.
2. Create Checklist: Develop a security review checklist based on your threat model. Include items for input validation, auth, crypto, and error handling.
3. Train Reviewers: Conduct a workshop on secure coding patterns and review techniques. Focus on common pitfalls in your codebase.
4. Integrate into PR Flow: Add the checklist as a PR template. Configure CI to run automated scans and block merges on critical findings.
5. Execute First Review: Assign a reviewer to a high-risk PR. Use the checklist and threat model to guide the analysis. Provide feedback and iterate.

Security code review is not a bottleneck; it is an investment in system resilience. By combining automated efficiency with human insight, organizations can detect critical vulnerabilities early, reduce remediation costs, and deliver software that withstands adversarial pressure. Implement this framework to transform security from an afterthought into a core engineering discipline.