Security Code Review

«LGTM» через сорок секунд после открытия PR, который трогает авторизацию. Вижу такое регулярно и считаю главным антипаттерном темы. Ревью прошло, галочка стоит, а никто не проверил ни авторизацию на новом endpoint, ни экранирование вывода, ни то, что свежая зависимость не тянет за собой уязвимую транзитивную версию. Security Code Review — это дисциплина проверки собственного кода на дефекты безопасности на этапе review: человеческое ревью на классы багов, которые линтер не ловит в принципе (broken access control, логика авторизации), плюс автоматика — SAST, SCA и secret scanning — поставленная как CI-гейт, а не как необязательный комментарий в PR. Граница с Threat Modeling: моделирование угроз на этапе проектирования отвечает, что может пойти не так, а ревью на этапе кода проверяет, что написанное этого не допускает. Граница с Vulnerability Management: тот лист про реакцию на уже известные CVE в зависимостях, этот — про дефекты в своём коде, пойманные до мержа.

Что должен уметь

Главный навык на уровне L4 — отличать находку по безопасности от придирки к стилю и проверять то, что SAST принципиально не видит. Поиск по шаблону хорошо ловит склейку SQL-строк или захардкоженный ключ, но broken access control — уязвимость №1 в OWASP Top 10 — это логика: «на этом endpoint проверяется, что пользователь владеет ресурсом, или любой авторизованный может прочитать чужой объект по id?». Такое находит человек, который читает код с вопросом «как это сломать», а не сканер. Я регулярно вижу команды, которые поставили SAST и считают security review закрытым — автоматика тут половина дела, вторая половина в голове ревьюера.

Знает категории OWASP Top 10 (injection, broken access control, insecure design, …); находит очевидные дефекты в собственном PR — захардкоженный секрет, конкатенация SQL, отсутствующая проверка авторизации, небезопасная десериализация.
Запускает SAST и поиск секретов локально перед пушем: хук на коммит (gitleaks, detect-secrets) плюс линтер языка (gosec, bandit, eslint-plugin-security).

Читает чужой PR глазами безопасности: проверка входных данных, авторизация на каждом endpoint, экранирование вывода, корректное использование криптографии (не самописной), обращение с секретами. Отличает находку от придирки.
Ставит SAST (Semgrep, CodeQL, gosec), SCA (govulncheck, Snyk, Dependabot) и поиск секретов в CI как обязательный гейт, а не как совет; настраивает правила, чтобы ложные срабатывания упали до уровня, при котором гейту верят.

Проектирует процесс review для команды: когда security review обязателен (risk-based — auth / crypto / payment / PII-код), как назначается ревьюер по безопасности, свод правил безопасного кодирования, критерии безопасности в определении готовности.
Пишет и поддерживает собственные правила SAST (реестр Semgrep) под антипаттерны своей организации; сбивает усталость от ложных срабатываний, из-за которой гейты и начинают игнорировать.

L6+

Внедряет secure SDLC: threat model → secure coding standard → SAST/SCA gates → security review → pen test; запускает программу security champions, чтобы экспертиза по безопасности не упиралась в одну команду.
Держит баланс между скоростью и строгостью гейтов; ведёт метрики (уязвимости, просочившиеся в прод, время до починки найденного в ревью, покрытие классов риска) и решает, где гейт роняет сборку, а где только предупреждает.

Материалы

Книги

Mark Dowd, John McDonald, Justin Schuh — The Art of Software Security Assessment (Addison-Wesley, 2006). Толстая и местами устаревшая по конкретике, но по методу чтения кода «как его сломать» — до сих пор образец. Если выбирать одну книгу именно про ручной аудит кода — эту.
Heather Adkins et al. — Building Secure and Reliable Systems (O’Reilly, 2020), главы 12–13 (writing / testing code for security). Взгляд Google: проверка безопасности встроена в обычное ревью кода, а не вынесена в отдельный этап.

Статьи и стандарты

OWASP Top 10. Базовый словарь классов уязвимостей; broken access control держит первое место и в редакции 2021, и в свежей 2025. Из заметных изменений 2025 года — отдельная категория под сбои цепочки поставок ПО, чего в прошлой редакции не было. Не чеклист, а карта того, на что смотреть в ревью.
OWASP ASVS — Application Security Verification Standard. Структурированный набор требований по уровням (L1/L2/L3) — основа для secure-coding guideline и чеклиста для ревью, привязанного к уровню риска сервиса.
OWASP Code Review Guide. Прямо про процесс ручного security review; методология, чеклисты по технологиям.
Apple goto fail (CVE-2014-1266) — разбор Adam Langley. Главный публичный кейс листа — см. ниже.

Инструменты

SAST: Semgrep (работает по правилам, быстрый, свои правила пишутся легко), CodeQL (глубокий семантический анализ, GitHub), gosec (Go), Bandit (Python), SonarQube. По моим наблюдениям, Semgrep чаще берут как первый шаг — низкий порог входа и читаемые правила.
SCA: govulncheck (Go, официальный, фильтрует по реально вызываемым путям), Snyk, Dependabot, Trivy.
Secret scanning: gitleaks, trufflehog — в pre-commit и в CI (см. Secrets Management).
DAST (дополняет, не заменяет): OWASP ZAP, Burp Suite — динамическая проверка работающего приложения.
В своём CLI jtsekret (менеджер личных секретов на Go) у меня в CI стоит минимальный базовый гейт — golangci-lint + govulncheck ./.... Это дешёвый уровень для небольшого проекта: govulncheck ловит уязвимые версии в графе модулей, причём только если уязвимая функция реально вызывается. Отдельно — решения по дизайну, влияющие на безопасность: секреты передаются в дочерний процесс через pipe, а не через argv (где они видны в ps) и не через env (где утекают в дочерние процессы и core dumps), локальный кэш шифрованный. Эти решения — ровно то, что должно всплывать в security review, а не оставаться в голове автора.

Best practices

Главный публичный кейс — Apple «goto fail» (CVE-2014-1266, февраль 2014). В коде проверки TLS-подписи продублировалась одна строка — goto fail; — без фигурных скобок вокруг if. В результате управление безусловно прыгало на метку, пропуская финальную проверку подписи целиком: соединение считалось валидным с любым сертификатом. Любой man-in-the-middle мог подделать TLS для миллионов устройств. Урок не «не дублируйте строки» — а то, что дефект жил в открытом исходнике около полутора лет, и его не поймали ни ревью, ни тесты, ни сборка. Простейший линтер на недостижимый код или на «if без скобок» поймал бы это за секунды; security-review с вопросом «а что если проверка не выполнится» — тоже. Это аргумент за защиту в глубину внутри самого процесса ревью, а не за «достаточно одного умного ревьюера».

SAST, SCA и secret scanning стоят как гейт, а не как совет. «Сканер напишет коммент в PR, а мы посмотрим» работает ровно месяц: дальше комменты сливаются с остальным шумом и перестают читаться. Критические находки блокируют мерж. Всё остальное проходит через явный разбор со сроком, а не через «потом посмотрим».

Само ревью распределяется по риску, а не равномерно по всем PR. Опечатка в README и новый endpoint приёма платежей не заслуживают одинакового внимания, а попытка проверять всё одинаково тщательно ломается сразу в обе стороны: получается либо очередь из PR, ждущих безопасника, либо внимание, размазанное до нуля. Обязательный ревьюер по безопасности нужен там, где код трогает auth, crypto, PII, платежи или десериализацию. Остальное едет обычным ревью.

Отдельно — стандарт secure coding, записанный, а не живущий в головах старших. «У нас опытная команда, и так всё знают» перестаёт работать в тот день, когда приходит новичок или когда заметную часть кода начинает предлагать LLM-ассистент. Хватает короткого документа на базе OWASP ASVS и собственных правил SAST, которые кодифицируют договорённости. Тогда знание проверяемо, а не устно.

Ложные срабатывания убивают SAST быстрее всего. Я регулярно вижу один и тот же сценарий: команда включает сканер с набором правил по умолчанию, получает на первом прогоне четыреста находок, из них триста восемьдесят ложные или нерелевантные, и через две недели гейт стоит в режиме «не блокировать», то есть его нет. Настройка правил — не разовое действие, а постоянная работа: зафиксировать текущие находки как базовую линию, отключить нерелевантные правила, написать свои под свой стек. Гейт, которому не доверяют, хуже отсутствующего — он создаёт ложное чувство покрытия.

Человек ловит то, чего поиск по шаблону не видит в принципе. SAST силён на синтаксических паттернах (SQL-конкатенация, слабый криптоалгоритм, захардкоженный ключ). Broken access control — №1 в OWASP Top 10 — это логика владения ресурсом, и её не поймать без понимания домена: «этот endpoint отдаёт объект по id, но проверяет ли он, что объект принадлежит текущему пользователю?». Поэтому security review — это и автоматика, и человек; убрать одно из двух нельзя, они закрывают разные классы.

Защита в глубину внутри процесса: хук на коммит → SAST и SCA в CI → человек → DAST и pen test. Ни один слой не полон. Хук ловит секрет до коммита, и это самый дешёвый уровень; гейт в CI отрабатывает на каждом PR; человек берёт логику и авторизацию; DAST и периодический pen test показывают то, что видно только на работающей системе. Кейс goto fail — пример того, как отсутствие нескольких слоёв сразу пропустило тривиальный дефект на полтора года.

Код, написанный ИИ-ассистентом, тоже проходит проверку на безопасность. Отдельная свежая грань: сгенерированный код выглядит уверенно и часто содержит классические дефекты (отсутствие валидации, устаревшие криптопрактики из обучающих данных, выдуманные или уязвимые версии пакетов). Я отношусь к PR, написанному моделью, как к коду незнакомого джуна: внимания к безопасности больше, а не меньше.

Связанные листья

Secrets Management — secret scanning пересекается напрямую: security code review ловит захардкоженный секрет до merge, а Secrets Management отвечает за жизнь того, что должно лежать в хранилище.
Threat Modeling — модель угроз на этапе проектирования говорит, что искать в ревью; ревью проверяет, что заявленные меры защиты в коде действительно есть.
Vulnerability Management — граница: ревью находит дефекты в своём коде до мержа, VM реагирует на известные CVE в зависимостях. Инструменты SAST и SCA у них общие.
Supply Chain Security — часть SCA внутри ревью, то есть проверка зависимостей, — частный случай гигиены цепочки поставок; воспроизводимые сборки и подпись артефактов лежат слоем рядом.
CI/CD — SAST / SCA / secret-scanning гейты живут в pipeline; именно CI превращает их из совета в обязательство.
Change Governance — обязательное ревью по безопасности для рискованных классов кода — часть политики изменений наряду со схемой согласования.
Security Chaos Engineering — SCR проверяет код на дефекты до деплоя; SCE проверяет, что контроли безопасности реально работают в проде.

Открытые вопросы

Глубина DAST и фаззинга — динамическое тестирование и фаззинг по покрытию (go-fuzz, libFuzzer) заслуживают отдельного разбора; здесь они упомянуты как соседний слой, не более.
Я не уверен, как корректно мерить эффективность security review. «Escaped vulnerabilities» считается только постфактум (когда уязвимость нашли в проде или на pen test), а найденные в ревью дефекты редко логируются как метрика. Если в вашей команде есть рабочая метрика результативности ревью — расскажите PR’ом.
Ревью безопасности с помощью моделей — инструменты, которые прогоняют PR через LLM, появляются быстро; честного публичного замера их полноты и точности на реальных уязвимостях я пока не видел.