Работая в команде Sheverev, иногда хочется кричать от радости за новые и крутые проекты: мы и разработкой масштабных сервисов занимаемся, и мобильными приложениями, а также аутстаффом, аудитом...и вот как раз с месяц назад завершили плотненький аудит большой системы перед ее масштабированием. Зачем и как это было? Расскажу понятно и коротко.

С самого-самого начала стоило бы хапнуть вопрос самого понятия аудита. Но разбирать морфемы мы не будем. Пока разберемся в его назначении и "нужности".

Что такое аудит и зачем он вообще нужен?

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

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

Кто клиент

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

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

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

Ревью кодовой базы на 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)


  1. YoungSkipper
    28.08.2023 14:33

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


    1. AlexSheverev Автор
      28.08.2023 14:33

      к сожалению, стоимость разглашать не можем (в процентном соотношении это куда меньше). Но вот если говорить про сроки, то потратили около 2,5 месяцев.


      1. YoungSkipper
        28.08.2023 14:33

        Стоимость не нужно. Нужно какой процент стоимость аудита состовляет от стоимости проекта. Потому что если это 50%+ то его смысл от меня ускользает. Учитывая, что судя по всему по результатам аудита нужно еще добаить +50% к стоимости проекта. А вот если это 10% от стоимости проекта, то класс - можно к вам обращаться. Опять же вот эти 2.5 месяца по отношению к времени разработки проекта - какое отношение? Если проект делался 2.5 месяца и аудит делался 2.5 месяца - то опять же смысл в аудите не понятен. А если проект делался 2.5 года и вы сделали на него аудит за 2.5 месяца - вы молодцы, можно к вам обращаться.


        1. AlexSheverev Автор
          28.08.2023 14:33

          Конечно, аудит не стоит 50%, далеко не: если представить, что проект стоит NNN, то аудит его 12-nnn часть. То есть аудит = +-5% от стоимости такого проекта. Мы не делали разработку, точные сроки не знаем. По количеству кода/функционала сроки разработки такой системы  будет около 1,5 лет.


  1. slavanikolsky
    28.08.2023 14:33

    Хорошо Аудит, кто проверяет исправления? или это уже новый аудит?


    1. AlexSheverev Автор
      28.08.2023 14:33

      Аудит же предоставляет рекомендации к исправлениям, ну а разработчики имеют право сопротивляться и оспаривать. Смысл аудита не в "накосячить еще на один аудит", а исключить косяки и помочь исправлять уже существующие. И, да, у аудитора нет выгоды в "накосячить еще больше".

      После аудита мы связывались с разработчиками чтобы обсудить рекомендации, интересующие вопросы и т.д.


      1. slavanikolsky
        28.08.2023 14:33

        Вы не ответили, вы принимаете исправления? Или это не включено?


        1. AlexSheverev Автор
          28.08.2023 14:33

          Если вы имеете ввиду, исправляем ли мы чужие косяки, то да, но это уже не аудит, а разработка. Аудит подразумевает "найти" любой косяк, а не "решить" его. Все зависит от договоренности: если клиент уволил первого разработчика и договорился с аудиторами на переоформление работы - то да, конечно.


          1. slavanikolsky
            28.08.2023 14:33

            Я имею ввиду проверить работу над ошибками. Найти ошибку это одно, а проверить, что ее исправили это другое.


            1. AlexSheverev Автор
              28.08.2023 14:33

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

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

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


              1. slavanikolsky
                28.08.2023 14:33

                Какие языки знаете? Уровень знания?

                У хороших сайтов после третьей попытки будет вылетать капча или как-то блочиться юзер на n-минут, а у плохих нет, что фактически означает то, что можно будет подобрать пароль к аккаунту),

                Уровень знания информационной безопасности? Забыли шифрование баз, хранение ключей и ...


  1. igor_suhorukov
    28.08.2023 14:33

    SonarQube можно развернуть в бесплатной версии, прогнать проект и получить почти ту же информацию самим разработчикам. У меня был опыт экспресс аудита Big Data системы от подрядчика за полтора дня, где были тесты без assert и куча закоментированных участков кода. После четких цифр, их PM прям проникся уважением. На их проекте команда разбегалась и собиралась новая(могу даже понять их почему).

    И был опыт когда мой проект проходил дорогой аудит у одной индийской компании, вот тут SonarQube(который и так был мной интегрирован в CI сборку) и статический анализ кода IntelliJ Idea справился лучше их изысканий длиной в месяц. Все общение сводилось к парированию ложноположительного срабатывания анализатора)

    Есть аудиты с целью реально помочь, а есть с целью потратить деньги организации.


    1. AlexSheverev Автор
      28.08.2023 14:33

      ну мы как раз про помочь, думаю, вы тож). За опыт спасибо, интересно!


  1. Maksclub
    28.08.2023 14:33

    ЗаDRY-ли ценой связанности? Вынесли все в общий "клей"-функционал? :)


  1. zubrbonasus
    28.08.2023 14:33

    Разработка документированного кода очевидно стоит дорого потому что на коменты нужно потратить время + время на тесты + время на архитектуру + время на ER диаграмму и что-то из полезного. Очевидно что стоимость разработки будет 3N x 3. А если за это не платили и ждать собственного нечего. В коммерческой разработке всегда так.