Security Code Review — чек-лист

Security Code Review — систематическая проверка исходного кода с целью выявления уязвимостей безопасности до попадания кода в production. Чек-лист стандартизирует этот процесс и обеспечивает покрытие наиболее критичных категорий.

Зачем нужно

Code review без security-фокуса пропускает целые классы уязвимостей: XSS, SQLi, IDOR, hardcoded secrets. Чек-лист задаёт вопросы, которые нужно задать при review каждого PR, и снижает зависимость от экспертизы конкретного ревьюера.

Где используется

  • Pull Request review для всех изменений, затрагивающих аутентификацию, авторизацию, данные
  • Онбординг новых разработчиков в security-практики
  • Pre-release аудит критических функций

Основной контент

Аутентификация и авторизация

// Вопросы при review:
// ✓ Все приватные эндпоинты защищены middleware?
// ✓ Проверяется принадлежность ресурса (anti-IDOR)?
// ✓ JWT верифицируется (алгоритм, expiry, signature)?

// Красный флаг:
app.get('/api/admin/users', (req, res) => { /* нет проверки роли */ });

// Правильно:
app.get('/api/admin/users', authenticate, requireRole('admin'), handler);

Валидация ввода

// Вопросы:
// ✓ Валидация на сервере (не только на клиенте)?
// ✓ Параметризованные SQL-запросы?
// ✓ Whitelist, а не blacklist для разрешённых значений?

// Красный флаг: конкатенация в SQL, eval, innerHTML без sanitize

Секреты и конфигурация

# Grep-чек на PR:
git diff HEAD~1 | grep -iE '(password|secret|key|token)\s*=\s*["\x27][^"\x27]{4,}'

# Красные флаги:
# API_KEY = "sk-live-abc123"
# password = "hardcoded123"

Обработка ошибок

// Вопросы:
// ✓ Внутренние ошибки (stack trace, SQL) не попадают в ответ?
// ✓ Одинаковое время ответа для valid/invalid пользователя (timing attack)?

// Плохо:
catch (err) { res.json({ error: err.message, stack: err.stack }); }

// Хорошо:
catch (err) {
  logger.error({ err, userId: req.user?.id }); // В логи полностью
  res.status(500).json({ error: 'Internal server error' }); // Клиенту минимум
}

HTTP-заголовки безопасности

# Чек заголовков через curl:
curl -I https://staging.example.com | grep -E '(X-Frame|CSP|HSTS|X-Content)'

# Должны присутствовать:
# Content-Security-Policy
# X-Frame-Options: DENY
# X-Content-Type-Options: nosniff
# Strict-Transport-Security

Полный чек-лист (краткий)

## Auth & Access Control
-  Middleware аутентификации на всех приватных роутах
-  Проверка владельца ресурса (anti-IDOR)
-  Роль не берётся из пользовательского ввода

## Input / Output
-  Параметризованные SQL-запросы
-  Санитизация HTML-вывода (XSS)
-  Валидация схемы входных данных

## Secrets
-  Нет hardcoded credentials
-  Секреты в env-переменных

## Error Handling
-  Stack trace не в ответе API
-  Логирование без PII в значениях

## Dependencies
-  npm audit пройден
-  Нет новых зависимостей с * в версии

Частые ошибки

  • Security review только для "security features" — уязвимости прячутся в обычном бизнес-коде
  • Поверхностный review без запуска кода — некоторые уязвимости видны только в динамике
  • Игнорирование зависимостей в PR — новый пакет может принести уязвимость

Связанные темы

Ресурсы