Agent Skills: Security Review Skill

Comprehensive security audit for code changes. Use this skill when implementing authentication, authorization, user input handling, API endpoints, secrets/credentials, payment features, or file uploads. Provides security checklists, vulnerability patterns, and remediation guidance. Integrates with implement-phase as a security quality gate.

UncategorizedID: mhylle/claude-skills-collection/security-review

Install this agent skill to your local

pnpm dlx add-skill https://github.com/mhylle/claude-skills-collection/tree/HEAD/skills/security-review

Skill Files

Browse the full folder contents for security-review.

Download Skill

Loading file tree…

skills/security-review/SKILL.md

Skill Metadata

Name
security-review
Description
Comprehensive security audit for code changes. Use this skill when implementing authentication, authorization, user input handling, API endpoints, secrets/credentials, payment features, or file uploads. Provides security checklists, vulnerability patterns, and remediation guidance. Integrates with implement-phase and tt-implement-phase as a security quality gate. Invoked as devflow:security-review; Claude Code's built-in /security-review is a different review of the pending branch changes.

Security Review Skill

<!-- contents: generated by tests/skill_structure.py --write; edit headings, not this block -->

Contents

Reference files (open one when a step points to it):

  • references/security-checklist.md: Security Review Checklist
<!-- /contents -->

This skill ensures all code follows security best practices and identifies potential vulnerabilities before they reach production.

Design Philosophy

Security is not optional. This skill acts as a security quality gate that validates code against common vulnerability patterns (OWASP Top 10) and project-specific security requirements. One vulnerability can compromise the entire platform.

When to Activate

Trigger this skill when code involves:

  • Authentication or authorization - Login flows, session management, role checks
  • User input handling - Forms, query parameters, file uploads
  • API endpoints - New routes, especially public-facing
  • Secrets or credentials - API keys, database connections, tokens
  • Payment features - Financial transactions, billing, subscriptions
  • Sensitive data - PII, health data, financial records
  • Third-party API integration - External service connections
  • Database queries - Especially raw SQL or dynamic queries

Security Checklist

1. Secrets Management

BAD - Never Do This

const apiKey = "sk-proj-xxxxx"  // Hardcoded secret
const dbPassword = "password123" // In source code
const config = {
  stripe_key: "sk_live_xxxxx"  // Committed to repo
}

GOOD - Always Do This

const apiKey = process.env.OPENAI_API_KEY
const dbUrl = process.env.DATABASE_URL

// Verify secrets exist at startup
if (!apiKey) {
  throw new Error('OPENAI_API_KEY not configured')
}

// Use secret management services for production
// AWS Secrets Manager, HashiCorp Vault, etc.

Verification Checklist

  • [ ] No hardcoded API keys, tokens, or passwords in source
  • [ ] All secrets loaded from environment variables
  • [ ] .env, .env.local, .env.production in .gitignore
  • [ ] No secrets in git history (run git log -p | grep -i "api_key\|secret\|password")
  • [ ] Production secrets in secure secret management (Vercel, Railway, AWS SM)
  • [ ] Different secrets for dev/staging/production environments

2. Input Validation

BAD - Never Trust User Input

// DANGEROUS: No validation
async function createUser(req: Request) {
  const { email, name, role } = req.body
  return db.users.create({ email, name, role }) // role injection!
}

// DANGEROUS: Client-side only validation
if (formData.email.includes('@')) { /* good enough? NO */ }

GOOD - Validate Everything Server-Side

import { z } from 'zod'

const CreateUserSchema = z.object({
  email: z.string().email().max(255),
  name: z.string().min(1).max(100).regex(/^[a-zA-Z\s]+$/),
  age: z.number().int().min(0).max(150).optional()
  // Note: role is NOT accepted from user input
})

export async function createUser(input: unknown) {
  const validated = CreateUserSchema.parse(input)
  return await db.users.create({
    ...validated,
    role: 'user' // Role set server-side, never from input
  })
}

File Upload Validation

function validateFileUpload(file: File) {
  // Size check (5MB max)
  const maxSize = 5 * 1024 * 1024
  if (file.size > maxSize) {
    throw new Error('File too large (max 5MB)')
  }

  // MIME type check (verify actual content, not just header)
  const allowedTypes = ['image/jpeg', 'image/png', 'image/gif']
  if (!allowedTypes.includes(file.type)) {
    throw new Error('Invalid file type')
  }

  // Extension check (defense in depth)
  const allowedExtensions = ['.jpg', '.jpeg', '.png', '.gif']
  const extension = file.name.toLowerCase().match(/\.[^.]+$/)?.[0]
  if (!extension || !allowedExtensions.includes(extension)) {
    throw new Error('Invalid file extension')
  }

  // Sanitize filename
  const sanitizedName = file.name.replace(/[^a-zA-Z0-9.-]/g, '_')

  return { ...file, name: sanitizedName }
}

Verification Checklist

  • [ ] All user inputs validated with schemas (zod, yup, joi)
  • [ ] Validation happens server-side (client validation is UX only)
  • [ ] File uploads restricted by size, type, and extension
  • [ ] File contents verified (not just extensions)
  • [ ] Whitelist validation preferred over blacklist
  • [ ] Error messages don't leak sensitive information
  • [ ] No direct use of user input in file paths or system commands

3. SQL Injection Prevention

BAD - Never Concatenate SQL

// CRITICAL VULNERABILITY - SQL Injection
const query = `SELECT * FROM users WHERE email = '${userEmail}'`
await db.query(query)

// Also dangerous with template literals
const search = `SELECT * FROM products WHERE name LIKE '%${term}%'`

GOOD - Always Use Parameterized Queries

// Safe - parameterized query with Supabase
const { data } = await supabase
  .from('users')
  .select('*')
  .eq('email', userEmail)

// Safe - parameterized raw SQL
await db.query(
  'SELECT * FROM users WHERE email = $1',
  [userEmail]
)

// Safe - ORM with proper escaping
const user = await prisma.user.findUnique({
  where: { email: userEmail }
})

// Safe - LIKE queries with parameterization
await db.query(
  'SELECT * FROM products WHERE name LIKE $1',
  [`%${term}%`]
)

Verification Checklist

  • [ ] All database queries use parameterized queries or ORM
  • [ ] No string concatenation or interpolation in SQL
  • [ ] Query builders used correctly (not bypassed)
  • [ ] Stored procedures use parameterized inputs
  • [ ] Database user has minimal required permissions

4. Authentication & Authorization

BAD - Insecure Token Storage

// WRONG: localStorage vulnerable to XSS
localStorage.setItem('token', token)
localStorage.setItem('user', JSON.stringify(user))

// WRONG: No authorization check
async function deleteUser(userId: string) {
  await db.users.delete({ where: { id: userId } }) // Anyone can delete!
}

GOOD - Secure Token Handling

// CORRECT: httpOnly cookies (server-side)
res.setHeader('Set-Cookie', [
  `token=${token}; HttpOnly; Secure; SameSite=Strict; Max-Age=3600; Path=/`
])

// CORRECT: Authorization check before action
export async function deleteUser(userId: string, requesterId: string) {
  const requester = await db.users.findUnique({
    where: { id: requesterId },
    select: { id: true, role: true }
  })

  // Check ownership OR admin role
  if (requester.id !== userId && requester.role !== 'admin') {
    throw new ForbiddenError('Not authorized to delete this user')
  }

  await db.users.delete({ where: { id: userId } })
}

Row Level Security (Supabase/PostgreSQL)

-- Enable RLS on all tables with user data
ALTER TABLE users ENABLE ROW LEVEL SECURITY;
ALTER TABLE posts ENABLE ROW LEVEL SECURITY;

-- Users can only view their own data
CREATE POLICY "Users view own data"
  ON users FOR SELECT
  USING (auth.uid() = id);

-- Users can only update their own data
CREATE POLICY "Users update own data"
  ON users FOR UPDATE
  USING (auth.uid() = id);

-- Admin policy (if needed)
CREATE POLICY "Admins can view all"
  ON users FOR SELECT
  USING (
    EXISTS (
      SELECT 1 FROM users WHERE id = auth.uid() AND role = 'admin'
    )
  );

Verification Checklist

  • [ ] Tokens stored in httpOnly cookies (not localStorage/sessionStorage)
  • [ ] Secure and SameSite flags set on cookies
  • [ ] Authorization check before every sensitive operation
  • [ ] Row Level Security enabled in database (Supabase)
  • [ ] Role-based access control implemented correctly
  • [ ] Session timeout configured appropriately
  • [ ] Password reset tokens single-use and time-limited
  • [ ] Failed login attempts tracked and limited

5. XSS Prevention

BAD - Rendering Unsanitized Content

// DANGEROUS: Direct HTML injection
function Comment({ html }) {
  return <div dangerouslySetInnerHTML={{ __html: html }} />
}

// DANGEROUS: Eval-like functions
const userCode = getUserInput()
eval(userCode)
new Function(userCode)()

GOOD - Sanitize All User HTML

import DOMPurify from 'isomorphic-dompurify'

function SafeComment({ html }: { html: string }) {
  const clean = DOMPurify.sanitize(html, {
    ALLOWED_TAGS: ['b', 'i', 'em', 'strong', 'p', 'br'],
    ALLOWED_ATTR: [] // No attributes allowed
  })
  return <div dangerouslySetInnerHTML={{ __html: clean }} />
}

// Better: Use markdown instead of HTML
import { marked } from 'marked'
import DOMPurify from 'isomorphic-dompurify'

function MarkdownContent({ markdown }: { markdown: string }) {
  const html = marked(markdown)
  const clean = DOMPurify.sanitize(html)
  return <div dangerouslySetInnerHTML={{ __html: clean }} />
}

Content Security Policy

// next.config.js
const securityHeaders = [
  {
    key: 'Content-Security-Policy',
    value: [
      "default-src 'self'",
      "script-src 'self'", // Remove 'unsafe-inline' 'unsafe-eval' if possible
      "style-src 'self' 'unsafe-inline'", // Needed for most CSS-in-JS
      "img-src 'self' data: https:",
      "font-src 'self'",
      "connect-src 'self' https://api.example.com",
      "frame-ancestors 'none'",
      "base-uri 'self'",
      "form-action 'self'"
    ].join('; ')
  },
  {
    key: 'X-Content-Type-Options',
    value: 'nosniff'
  },
  {
    key: 'X-Frame-Options',
    value: 'DENY'
  }
]

Verification Checklist

  • [ ] All user-provided HTML sanitized with DOMPurify or similar
  • [ ] dangerouslySetInnerHTML only used with sanitized content
  • [ ] CSP headers configured and tested
  • [ ] No eval(), new Function(), or innerHTML with user data
  • [ ] React's built-in XSS protection not bypassed unnecessarily
  • [ ] URL parameters validated before use in links (javascript: protocol blocked)

6. CSRF Protection

BAD - No CSRF Protection

// VULNERABLE: State-changing GET request
app.get('/api/delete-account', async (req, res) => {
  await deleteAccount(req.user.id)
})

// VULNERABLE: No CSRF token verification
app.post('/api/transfer', async (req, res) => {
  await transferMoney(req.body.amount, req.body.to)
})

GOOD - Implement CSRF Protection

import { csrf } from '@/lib/csrf'

export async function POST(request: Request) {
  // Verify CSRF token from header
  const token = request.headers.get('X-CSRF-Token')

  if (!csrf.verify(token)) {
    return NextResponse.json(
      { error: 'Invalid CSRF token' },
      { status: 403 }
    )
  }

  // Process request
}

// Client-side: Include CSRF token in requests
fetch('/api/action', {
  method: 'POST',
  headers: {
    'X-CSRF-Token': csrfToken, // From meta tag or cookie
    'Content-Type': 'application/json'
  },
  body: JSON.stringify(data)
})

SameSite Cookies (Primary Defense)

// Modern browsers: SameSite provides CSRF protection
res.setHeader('Set-Cookie', [
  `session=${sessionId}; HttpOnly; Secure; SameSite=Strict; Path=/`
])

Verification Checklist

  • [ ] All state-changing operations use POST/PUT/DELETE (not GET)
  • [ ] CSRF tokens on forms and AJAX requests
  • [ ] SameSite=Strict on all authentication cookies
  • [ ] Origin header verified on sensitive endpoints
  • [ ] Double-submit cookie pattern for APIs (if needed)

7. Rate Limiting

BAD - No Rate Limiting

// VULNERABLE: No limits on expensive operation
app.post('/api/search', async (req, res) => {
  const results = await expensiveSearch(req.body.query)
  return res.json(results)
})

// VULNERABLE: No limits on auth endpoints
app.post('/api/login', async (req, res) => {
  // Brute force attack possible
})

GOOD - Implement Rate Limiting

import rateLimit from 'express-rate-limit'

// General API rate limit
const apiLimiter = rateLimit({
  windowMs: 15 * 60 * 1000, // 15 minutes
  max: 100, // 100 requests per window
  message: { error: 'Too many requests, please try again later' },
  standardHeaders: true,
  legacyHeaders: false
})

// Strict rate limit for auth endpoints
const authLimiter = rateLimit({
  windowMs: 15 * 60 * 1000,
  max: 5, // 5 attempts per 15 minutes
  message: { error: 'Too many login attempts' },
  skipSuccessfulRequests: true
})

// Very strict for expensive operations
const searchLimiter = rateLimit({
  windowMs: 60 * 1000, // 1 minute
  max: 10,
  message: { error: 'Too many search requests' }
})

app.use('/api/', apiLimiter)
app.use('/api/auth/', authLimiter)
app.use('/api/search', searchLimiter)

Verification Checklist

  • [ ] Rate limiting on all API endpoints
  • [ ] Stricter limits on authentication endpoints
  • [ ] Stricter limits on expensive operations (search, AI, etc.)
  • [ ] IP-based rate limiting for unauthenticated requests
  • [ ] User-based rate limiting for authenticated requests
  • [ ] Rate limit headers returned to clients
  • [ ] Graceful handling of rate limit exceeded

8. Sensitive Data Exposure

BAD - Leaking Sensitive Data

// WRONG: Logging sensitive data
console.log('User login:', { email, password })
console.log('Payment processed:', { cardNumber, cvv, amount })
console.log('Request body:', req.body) // May contain passwords

// WRONG: Exposing internal errors
catch (error) {
  return res.json({
    error: error.message,
    stack: error.stack, // Stack trace exposed!
    query: sqlQuery // Query exposed!
  })
}

// WRONG: Returning full user object
return res.json({ user }) // Includes password hash, internal IDs, etc.

GOOD - Protect Sensitive Data

// CORRECT: Redact sensitive data in logs
console.log('User login:', { email, userId: user.id })
console.log('Payment processed:', {
  last4: card.number.slice(-4),
  amount,
  userId
})

// CORRECT: Generic error messages to clients
catch (error) {
  // Log full error server-side
  logger.error('Payment failed', {
    error: error.message,
    userId,
    // Never log card numbers, even partial
  })

  // Return generic message to client
  return res.status(500).json({
    error: 'Payment processing failed. Please try again.'
  })
}

// CORRECT: Select specific fields
const user = await db.users.findUnique({
  where: { id: userId },
  select: {
    id: true,
    email: true,
    name: true,
    // Explicitly exclude: passwordHash, internalNotes, etc.
  }
})
return res.json({ user })

Verification Checklist

  • [ ] No passwords, tokens, or full card numbers in logs
  • [ ] Error messages generic for end users
  • [ ] Detailed errors only in server-side logs
  • [ ] No stack traces exposed to clients
  • [ ] API responses select specific fields (not SELECT *)
  • [ ] Sensitive data encrypted at rest
  • [ ] Sensitive data encrypted in transit (HTTPS enforced)
  • [ ] PII handling compliant with regulations (GDPR, CCPA)

9. Blockchain Security (Conditional)

Only applicable when project involves blockchain/crypto functionality.

BAD - Insecure Blockchain Handling

// WRONG: Not verifying wallet ownership
async function claimReward(walletAddress: string) {
  await sendReward(walletAddress) // Anyone can claim!
}

// WRONG: Blind transaction signing
async function processTransaction(tx: any) {
  await wallet.signAndSend(tx) // No validation!
}

GOOD - Verify Everything

import { verify } from '@solana/web3.js'
import nacl from 'tweetnacl'

// Verify wallet ownership with signed message
async function verifyWalletOwnership(
  publicKey: string,
  signature: string,
  message: string
): Promise<boolean> {
  try {
    const messageBytes = new TextEncoder().encode(message)
    const signatureBytes = Buffer.from(signature, 'base64')
    const publicKeyBytes = Buffer.from(publicKey, 'base64')

    return nacl.sign.detached.verify(
      messageBytes,
      signatureBytes,
      publicKeyBytes
    )
  } catch {
    return false
  }
}

// Validate transaction before signing
async function processTransaction(transaction: Transaction) {
  // Verify recipient is expected
  if (transaction.to !== KNOWN_RECIPIENT) {
    throw new Error('Invalid recipient address')
  }

  // Verify amount is within limits
  if (transaction.amount > MAX_TRANSACTION_AMOUNT) {
    throw new Error('Amount exceeds limit')
  }

  // Verify user has sufficient balance
  const balance = await getBalance(transaction.from)
  if (balance < transaction.amount + GAS_BUFFER) {
    throw new Error('Insufficient balance')
  }

  // Log transaction for audit
  await auditLog.record({
    type: 'transaction',
    from: transaction.from,
    to: transaction.to,
    amount: transaction.amount,
    timestamp: new Date()
  })

  return await signAndSend(transaction)
}

Verification Checklist

  • [ ] Wallet signatures verified before granting access
  • [ ] Transaction details validated before signing
  • [ ] Balance checks before transactions
  • [ ] No blind transaction signing
  • [ ] Transaction limits enforced
  • [ ] Audit logging for all transactions
  • [ ] Private keys never exposed in logs or errors
  • [ ] Smart contract interactions validated

10. Dependency Security

Regular Security Audits

# Check for known vulnerabilities
npm audit

# Automatically fix what's possible
npm audit fix

# Check for outdated packages
npm outdated

# Update dependencies
npm update

# For major updates, use interactive mode carefully
npx npm-check-updates -i

Lock File Management

# ALWAYS commit lock files
git add package-lock.json  # or yarn.lock, pnpm-lock.yaml

# In CI/CD, use clean install for reproducible builds
npm ci  # Not npm install

# Verify integrity
npm ci --ignore-scripts  # Then run scripts separately if needed

Verification Checklist

  • [ ] No known vulnerabilities (npm audit clean or exceptions documented)
  • [ ] Dependencies regularly updated
  • [ ] Lock files committed to repository
  • [ ] Dependabot or similar enabled on GitHub
  • [ ] Unused dependencies removed
  • [ ] Dependencies from trusted sources only
  • [ ] License compliance verified (no GPL in proprietary code, etc.)
  • [ ] Postinstall scripts reviewed for new dependencies

Integration with implement-phase

When invoked as part of the implement-phase pipeline (Step 3), this skill provides structured output for orchestration.

Input Context

Security Review for Phase [N]

Context:
- Plan: [path to plan file]
- Phase: [phase number and name]
- Changed files: [list of modified/added files]
- Security-relevant changes: [auth, input, api, secrets, payment, uploads]

Output Format

STATUS: PASS | PASS_WITH_ISSUES | FAIL
CATEGORIES_CHECKED: [count of applicable categories reviewed]
ISSUES_FOUND:
- [CRITICAL] [Category]: [Description]
- [HIGH] [Category]: [Description]
- [MEDIUM] [Category]: [Description]
- [LOW] [Category]: [Description]
SEVERITY: CRITICAL | HIGH | MEDIUM | LOW | NONE
RECOMMENDATIONS:
- [Non-blocking improvement suggestions]
REPORT: [Path to detailed report if written]

Severity Levels

| Level | Meaning | Action Required | |-------|---------|-----------------| | CRITICAL | Active vulnerability, exploitable now | Block deployment, fix immediately | | HIGH | Serious vulnerability, likely exploitable | Block deployment, fix before merge | | MEDIUM | Potential vulnerability, context-dependent | Should fix, may proceed with documented exception | | LOW | Best practice violation, minimal risk | Note for improvement |

Integration Point in Pipeline

implement-phase Pipeline:
  Step 1: Implementation
  Step 2: Functional verification (tests pass)
  Step 3: Code review
  Step 4: SECURITY REVIEW (this skill) <--
  Step 5: Plan synchronization
  Step 6: Completion report

On FAIL:
  - Block phase completion
  - Report blocking issues
  - Require fixes before retry

Pre-Deployment Security Checklist

Before ANY production deployment, verify:

Secrets & Configuration

  • [ ] No hardcoded secrets in code
  • [ ] All secrets in environment variables
  • [ ] Production secrets in secure management
  • [ ] Different secrets per environment

Input & Data

  • [ ] All user input validated server-side
  • [ ] File uploads validated and sanitized
  • [ ] All database queries parameterized
  • [ ] Sensitive data encrypted at rest

Authentication & Authorization

  • [ ] Tokens in httpOnly cookies
  • [ ] Authorization checks on all operations
  • [ ] Row Level Security enabled
  • [ ] Session management secure

Web Security

  • [ ] XSS protection (CSP, sanitization)
  • [ ] CSRF protection enabled
  • [ ] HTTPS enforced
  • [ ] Security headers configured

Infrastructure

  • [ ] Rate limiting on all endpoints
  • [ ] Error messages don't leak info
  • [ ] Logging doesn't include secrets
  • [ ] Dependencies up to date

Compliance

  • [ ] CORS properly configured
  • [ ] Privacy policy accurate
  • [ ] Data handling compliant (GDPR/CCPA)

References

For detailed checklists and examples:


Security is everyone's responsibility. When in doubt, ask for a security review.