Работая в команде Sheverev, иногда хочется кричать от радости за новые и крутые проекты: мы и разработкой масштабных сервисов занимаемся, и мобильными приложениями, а также аутстаффом, аудитом...и вот как раз с месяц назад завершили плотненький аудит большой системы перед ее масштабированием. Зачем и как это было? Расскажу понятно и коротко.
С самого-самого начала стоило бы хапнуть вопрос самого понятия аудита. Но разбирать морфемы мы не будем. Пока разберемся в его назначении и "нужности".
Что такое аудит и зачем он вообще нужен?
Целью аудита является обеспечение надежности и правильной работы программных продуктов, а также защита от возможных рисков и уязвимостей.
Самое главное, что предоставляет аудит — помощь заказчику в проверке своего подрядчика на “порядочность”. А еще невероятно помогает при масштабировании ваших проектов: так, аудит способен выявить настоящее состояние проекта, проверить, есть ли в нем уязвимости, потенциальные ошибки и недостатки, которые могут грозить потерей данных, денег и нервов впоследствии.
Кто клиент
Нашим клиентом в данном аудите выступила крупная строительная компания. На одном из этапов проекта она обратились к нам с просьбой проверки четкого выполнения ТЗ их подрядчиком.
Да, случается такое, что даже хорошую работу стоит/нужно проверить на наличие мелких косяков и неточностей, которые в будущем выльются в довольное большие проблемы. Но есть и большие проблемы технического долга: недоработки, совершенно не оптимальная архитектура, которая выльется в реальные проблемы проекта.
Нашей же задачей было проверить качество кода, а также узнать, оптимальна ли архитектура, есть ли уязвимости и так далее. Еще одна задача заключалась в разработке шаблона, по которому и будет проводится аудит проекта для Node JS-проектов.
Наши задачки разделялись на этапы. Вот некоторые из них:
Проверка стека и соответствие его задачам.Сначала описываем стек, после — проверяем библиотеки и их версии (есть ли в них уязвимости). Также мы описали саму структуру папок и подпапок для лучшего понимания структуры проекта.
Далее — проверка анализаторами кода на предмет копипасты, следования гайд-лайнам, проверка оставленных токенов в коде, которыми в будущем мог воспользоваться злоумышленник, а также оставленных TODO комментариев.
Ревью кодовой базы более чем 600+ файлов на предмет ошибок/непроизводительного кода.
Ревью документации и комментариев в коде. Проверяем наличие и качество документации и кода.
Ревью тестов и логирования. Смотрим, есть ли они, какой процент покрытия кодом тестами, есть ли логирование в проекте.
Ревью безопасности и отказоустойчивости.
Ревью архитектуры. Анализируем, насколько это хорошая архитектура с точки зрения поддержки, масштабируемости и функциональности.
Ревью базы данных. Смотрим в базу, на запросы, на структуру таблиц, если это реляционные, и на структуры, если это NoSQL, а также нет ли кода который сильно замедляет производительность.
Описание стека
Анализируем насколько стек соответствует отрасли, а также:
README-файлы и инструкции по развертыванию проекта,
версионность библиотек и их актуальность,
структура проекта, файлов и папок,
описываем какие базы данных и фреймворки используются,
настроен ли СI/DI,
используются ли переменные окружения,
комментарии к конфигурационным файлам,
(в нашем случае) описание скриптов в package.json,
требования к инфраструктуре,
использование типизации (в нашем случае это TypeScript).
На этом этапе вырисовывается общее представление о проекте. Всплывают такие вещи как: самописные фреймворки исполнителя, которые подгружаются через приватный репозиторий, на который у заказчика нет документации.
При смене исполнителя можно столкнутся с интересным челенджем по поддержке всего этого добра, который будет стоить ваших денег и времени. А отсутствие комментариев к скриптам, конфигурационным файлам и readme ухудшает онбординг новых разработчиков. То есть новым разработчикам потребуется достаточно много времени для погружения в проект, что, опять таки, несет за собой немалые убытки как по времени, так и по деньгам.
Проверка автоматическими анализаторами кода
Я использовал mega-linter которые имеет пресеты для JavaScript, прогнал код на копипасту, следование гайдлайнам линтера, наличием в коде токенов, TODO комментариев, а также мелочи использования, например var, инструкции по отключению линтера, ну и наличием документации в целом.
Ревью кодовой базы на 600+ файлов
Это, наверное, самый объемный кусок отчета. Поскольку заказчик просил нас оформить отчет именно в doc.файле, нам нужно было как-то эту информацию структурировать. Начали с написания небольшой тулзы, которая просто ходит по файлам проекта, и находит специфические комментарии:
//! - критичное
//!@ - минорное
Строка и сам комментарий собирается в JSON, а дальше мы к нему дописываем рекомендации и приводим скриншот в формате “строка №Х-проблема-почему это нужно исправить”. В данном проекте было много повторяющихся ошибок.
Наиболее часто встречающийся проблемы конкретно в этом проекте:
использование длинных вложенных if-else конструкций,
перегруженные методы на 800 строк,
объемные запросы к базе, которые можно было бы упростить,
кириллические переменные КАРЛ ?,
много копипасты, не следуем принципу KISS (Keep It Simple Stupid) и DRY (Don't Repeat Yourself),
отсутствие комментариев и документации к методам.
К каждой проблеме мы прикладывали комментарий, в котором написано, почему ее нужно исправить, и почему клиент должен тратить деньги на исправление конкретно этой проблемы.
Ревью документации и комментариев
Тут у нас получилось сразу два вывода:
1. Код слабо аннотирован. Понять, что делает конкретный метод возможно только при прочтении кода внутри. Новым разработчикам будет сложно разобраться, а срок вхождения в проект увеличится кратно.
2. Ну и самое главное — отсутствие документации как таковой.
Ревью тестов и логирования
В ревью мы смотрим на процент покрытия тестами, проверяем, есть ли логирование. В данном проекте тесты были до 30% методов, а логирование использовалось точечно, не было какой-то системы по дебагу.
Ревью безопасности и отказоустойчивости
Помимо всего, мы проверили API на лимиты, отказоустойчивость при нагрузке, а также брутфорс (это когда робот пытается подобрать пароли к форме. У хороших сайтов после третьей попытки будет вылетать капча или как-то блочиться юзер на n-минут, а у плохих нет, что фактически означает то, что можно будет подобрать пароль к аккаунту), наличие системы для создания бекапов, методы для отката миграций в базе.
Ревью базы данных
В проекте используется 2 базы данных — postgresql и mongoDB. Мы в структуру баз данных не погружались сильно, это была скорее рекомендация к нормализации базы данных путем добавления ссылок на другие таблицы, а не “тулить все в одной”.
Если говорить о postgres, то в ней более 100 таблиц, для которых нет комментариев, как и нет ER диаграммы. Мы нормализовали структуру страниц, применив foreign key. У подавляющего большинства таблиц отсутствовали индексы по полям, потому добавлены только уникальные индексы и составные уникальные.
Также в коде используются запросы и большим количеством join, что увеличивает нагрузку на базу данных и уменьшает возможности для ее масштабирования. Также стоит отметить, что:
Отсутствие комментариев и ER-диаграммы делает трудным понимание структуры и взаимосвязей таблиц.
Отсутствие индексов на полях, участвующих в выборках, приводит к медленным запросам и увеличению нагрузки на базу данных.
Использование множественных join также увеличивает нагрузку на базу данных и ухудшает ее масштабируемость.
Кеширование
На схеме архитектуры backend был обозначен Redis для кэширования горячих данных. Однако кода, работающего с ним, нами не было обнаружено. Обнаружен код использующий как кэш БД mongoDB. Кстати, к кэшированию не относится, но на проекте еще где-то была обозначена нейронка, но мы ее тоже не нашли. Вот так.
Отсутствие использования Redis в соответствии с заявленной архитектурой может привести к неэффективному использованию ресурсов и замедлению работы приложения. Кроме того, кэширование данных в базе данных может привести к перегрузке БД и уменьшению производительности, что может отрицательно сказаться на пользовательском опыте.
Отчет
В итоге вышел подробный отчет на 100+ страниц, большинство информации из которого пришлось на код-ревью. В первой версии отчета были еще и описания методов API, которые собирались по фронту и описывались, но в итоговую версию они не вошли.
Заказчик вносил правки по структуре документа, которые он хотел видеть: вместо WYSIWYG кода в редакторе нужны были скриншоты, так что добавлять их приходилось вручную. Также был расширен раздел с рекомендациями.
Заключение
Так, на примере одного из наших кейсов мы постарались описать, что такое аудит и с чем (или без чего) его едят. Наши действия могли и не понадобиться заказчику, если бы до этого все действия по проекту были бы выполнены сглажено и более адекватно. Однако из-за проверки были найдены многие уязвимости, так или иначе влияющие на настоящее состояние проекта, а также и на возможные будущие убытки по многим ресурсам.
Хорошо, когда аудит не приносит множество багов в формате “срочно решите”, ведь в таком случае работу можно считать отлично проделанной: ваш заказчик не запорол вам проект, а вы, ну и конечно ваши деньги и нервы в порядке. Однако в случае, если аудит выявил кучу неоднозначных моментов, стоит задуматься, насколько плодотворной была работа ваших подрядчиков и стоит ли с ними работать дальше. Но это уже совсем другая история.
Как-то так. Желаю вам качественно выполненных проектов и внимательных аудиторов. Как говорится – берегите себя и свой код.
