Работая в команде 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 кода в редакторе нужны были скриншоты, так что добавлять их приходилось вручную. Также был расширен раздел с рекомендациями.
Заключение
Так, на примере одного из наших кейсов мы постарались описать, что такое аудит и с чем (или без чего) его едят. Наши действия могли и не понадобиться заказчику, если бы до этого все действия по проекту были бы выполнены сглажено и более адекватно. Однако из-за проверки были найдены многие уязвимости, так или иначе влияющие на настоящее состояние проекта, а также и на возможные будущие убытки по многим ресурсам.
Хорошо, когда аудит не приносит множество багов в формате “срочно решите”, ведь в таком случае работу можно считать отлично проделанной: ваш заказчик не запорол вам проект, а вы, ну и конечно ваши деньги и нервы в порядке. Однако в случае, если аудит выявил кучу неоднозначных моментов, стоит задуматься, насколько плодотворной была работа ваших подрядчиков и стоит ли с ними работать дальше. Но это уже совсем другая история.
Как-то так. Желаю вам качественно выполненных проектов и внимательных аудиторов. Как говорится – берегите себя и свой код.
Комментарии (15)
slavanikolsky
28.08.2023 14:33Хорошо Аудит, кто проверяет исправления? или это уже новый аудит?
AlexSheverev Автор
28.08.2023 14:33Аудит же предоставляет рекомендации к исправлениям, ну а разработчики имеют право сопротивляться и оспаривать. Смысл аудита не в "накосячить еще на один аудит", а исключить косяки и помочь исправлять уже существующие. И, да, у аудитора нет выгоды в "накосячить еще больше".
После аудита мы связывались с разработчиками чтобы обсудить рекомендации, интересующие вопросы и т.д.
slavanikolsky
28.08.2023 14:33Вы не ответили, вы принимаете исправления? Или это не включено?
AlexSheverev Автор
28.08.2023 14:33Если вы имеете ввиду, исправляем ли мы чужие косяки, то да, но это уже не аудит, а разработка. Аудит подразумевает "найти" любой косяк, а не "решить" его. Все зависит от договоренности: если клиент уволил первого разработчика и договорился с аудиторами на переоформление работы - то да, конечно.
slavanikolsky
28.08.2023 14:33Я имею ввиду проверить работу над ошибками. Найти ошибку это одно, а проверить, что ее исправили это другое.
AlexSheverev Автор
28.08.2023 14:33Понял. Да, проверка по исправлению возможна, только уже в последующем аудите, то есть новом. Первый, изначальный, подразумевает только "найти" ошибки уже существующие, а не то, как их подлатали после тыка на них.
Но, предполагаю, что бывает и тот случай, когда договоренность есть и на последующую проверку по исправлению в контексте изначального аудита. В общем, это уже индивидуально все, а так, как и писал ранее, - аудит проверяет, разработчик исправляет, а аудитор в дальнейшем готовит новый аудит, если он требуется.
*Для избежания подобных косяков нужна изначально хорошая команда, кьюа-ребята, ну и внимательность, конечно. Но у любого проекта есть некая требовательность к аудиту, особенно в случае масштабирования крупного, который по срокам и деньгам весит прилично.
slavanikolsky
28.08.2023 14:33Какие языки знаете? Уровень знания?
У хороших сайтов после третьей попытки будет вылетать капча или как-то блочиться юзер на n-минут, а у плохих нет, что фактически означает то, что можно будет подобрать пароль к аккаунту),
Уровень знания информационной безопасности? Забыли шифрование баз, хранение ключей и ...
igor_suhorukov
28.08.2023 14:33SonarQube можно развернуть в бесплатной версии, прогнать проект и получить почти ту же информацию самим разработчикам. У меня был опыт экспресс аудита Big Data системы от подрядчика за полтора дня, где были тесты без assert и куча закоментированных участков кода. После четких цифр, их PM прям проникся уважением. На их проекте команда разбегалась и собиралась новая(могу даже понять их почему).
И был опыт когда мой проект проходил дорогой аудит у одной индийской компании, вот тут SonarQube(который и так был мной интегрирован в CI сборку) и статический анализ кода IntelliJ Idea справился лучше их изысканий длиной в месяц. Все общение сводилось к парированию ложноположительного срабатывания анализатора)
Есть аудиты с целью реально помочь, а есть с целью потратить деньги организации.
AlexSheverev Автор
28.08.2023 14:33ну мы как раз про помочь, думаю, вы тож). За опыт спасибо, интересно!
zubrbonasus
28.08.2023 14:33Разработка документированного кода очевидно стоит дорого потому что на коменты нужно потратить время + время на тесты + время на архитектуру + время на ER диаграмму и что-то из полезного. Очевидно что стоимость разработки будет 3N x 3. А если за это не платили и ждать собственного нечего. В коммерческой разработке всегда так.
YoungSkipper
Понимания все же зачем это нужно, хотелось бы представлять стоимость такого аудита, по отношению к стоимости разработки проекта. По описанию выглядит, что аудит стоит примерно процентов 50 от разработки, судя по описанным косякам. А реальности какой процент?
AlexSheverev Автор
к сожалению, стоимость разглашать не можем (в процентном соотношении это куда меньше). Но вот если говорить про сроки, то потратили около 2,5 месяцев.
YoungSkipper
Стоимость не нужно. Нужно какой процент стоимость аудита состовляет от стоимости проекта. Потому что если это 50%+ то его смысл от меня ускользает. Учитывая, что судя по всему по результатам аудита нужно еще добаить +50% к стоимости проекта. А вот если это 10% от стоимости проекта, то класс - можно к вам обращаться. Опять же вот эти 2.5 месяца по отношению к времени разработки проекта - какое отношение? Если проект делался 2.5 месяца и аудит делался 2.5 месяца - то опять же смысл в аудите не понятен. А если проект делался 2.5 года и вы сделали на него аудит за 2.5 месяца - вы молодцы, можно к вам обращаться.
AlexSheverev Автор
Конечно, аудит не стоит 50%, далеко не: если представить, что проект стоит NNN, то аудит его 12-nnn часть. То есть аудит = +-5% от стоимости такого проекта. Мы не делали разработку, точные сроки не знаем. По количеству кода/функционала сроки разработки такой системы будет около 1,5 лет.