Как мы заKISSили и заDRYили огромный аудит ✔️

Привет! Мы — команда Sheverev, занимаемся разработкой масштабных сервисов и мобильных приложений, а также аудитом, аутстаффом и разными крутыми штуками. Почитать наши предыдущие статьи (а особенно про NFT и танцы) можно тут.

Целью аудита является обеспечение надежности и правильной работы программных продуктов, а также защита от возможных рисков и уязвимостей.

Самое главное, что предоставляет аудит — помощь заказчику в проверке своего подрядчика на “порядочность”.Так, аудит способен выявить настоящее состояние проекта, проверить, есть ли в нем уязвимости, потенциальные ошибки и недостатки, которые могут грозить потерей данных, денег и нервов.

Нашим клиентом в данном аудите выступила крупная строительная компания. На одном из этапов проекта она обратились к нам с просьбой проверки четкого выполнения ТЗ их подрядчиком.

Да, случается такое, что даже хорошую работу стоит/нужно проверить на наличие мелких косяков и неточностей, которые в будущем выльются в довольное большие проблемы. Но есть и большие проблемы технического долга: недоработки, совершенно не оптимальная архитектура, которая выльется в реальные проблемы проекта.

Нашей же задачей было проверить качество кода, а также узнать, оптимальна ли архитектура, есть ли уязвимости и так далее. Еще одна задача заключалась в разработке шаблона, по которому и будет проводится аудит проекта для Node JS-проектов.

1. Проверка стека и соответствие его задачам.Сначала описываем стек, после — проверяем библиотеки и их версии (есть ли в них уязвимости). Также мы описали саму структуру папок и подпапок для лучшего понимания структуры проекта.

2. Далее — проверка анализаторами кода на предмет копипасты, следования гайд-лайнам, проверка оставленных токенов в коде, которыми в будущем мог воспользоваться злоумышленник, а также оставленных TODO комментариев.

3. Ревью кодовой базы более чем 600+ файлов на предмет ошибок/непроизводительного кода.

4. Ревью документации и комментариев в коде. Проверяем наличие и качество документации и кода.

5. Ревью тестов и логирования. Смотрим, есть ли они, какой процент покрытия кодом тестами, есть ли логирование в проекте.

6. Ревью безопасности и отказоустойчивости.

7. Ревью архитектуры. Анализируем, насколько это хорошая архитектура с точки зрения поддержки, масштабируемости и функциональности.

8. Ревью базы данных. Смотрим в базу, на запросы, на структуру таблиц, если это реляционные, и на структуры, если это NoSQL, а также нет ли кода который сильно замедляет производительность.

Анализируем насколько стек соответствует отрасли, а также:

• README-файлы и инструкции по развертыванию проекта,

• версионность библиотек и их актуальность,

• структура проекта, файлов и папок,

• описываем какие базы данных и фреймворки используются,

• настроен ли СI/DI,

• используются ли переменные окружения,

• комментарии к конфигурационным файлам,

• (в нашем случае) описание скриптов в package.json,

• требования к инфраструктуре,

• использование типизации (в нашем случае это TypeScript).

На этом этапе вырисовывается общее представление о проекте.Всплывают такие вещи как: самописные фреймворки исполнителя, которые подгружаются через приватный репозиторий, на который у заказчика нет документации.

При смене исполнителя можно столкнутся с интересным челенджем по поддержке всего этого добра, который будет стоить ваших денег и времени. А отсутствие комментариев к скриптам, конфигурационным файлам и readme ухудшает онбординг новых разработчиков. То есть новым разработчикам потребуется достаточно много времени для погружения в проект, что несет за собой немалые убытки как по времени, так и по деньгам.

Я использовал mega-linter которые имеет пресеты для JavaScript, прогнал код на копипасту, следование гайдлайнам линтера, наличием в коде токенов, TODO комментариев, а также мелочи использования, например var, инструкции по отключению линтера, ну и наличием документации в целом.

Это, наверное, самый объемный кусок отчета. Поскольку заказчик просил нас оформить отчет именно в 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 кода в редакторе нужны были скриншоты, так что добавлять их приходилось вручную. Также был расширен раздел с рекомендациями.

Так, на примере одного из наших кейсов мы постарались описать, что такое аудит и с чем (или без чего) его едят. Наши действия могли и не понадобиться заказчику, если бы до этого все действия по проекту были бы выполнены сглажено и более адекватно. Однако из-за проверки были найдены многие уязвимости, так или иначе влияющие на настоящее состояние проекта, а также и на возможные будущие убытки по многим ресурсам.

Хорошо, когда аудит не приносит множество багов в формате “срочно решите”, ведь в таком случае работу можно считать отлично проделанной: ваш заказчик не запорол вам проект, а вы, ваши деньги и нервы в порядке. Однако в случае, если аудит выявил кучу неоднозначных моментов, стоит задуматься, насколько плодотворной была работа ваших подрядчиков и стоит ли с ними работать дальше. Но это уже совсем другая история.