ipam_control/docs/reviews/2026-09-26-codebase-review-2.md

Повторный анализ кодовой базы IPAM Manager — 2026-09-26

Состояние: коммит 13e17fb (задачи 001–024). Объём: app/ и web/app.js — около 3 400 строк, миграции 0001–0008. Метод: чтение кода с упором на участки, которые изменения 010–024 затронули или усилили. Подозрительные места проверены на стенде ipam_control_006 на временных данных, после проверки данные удалены. Предыдущие отчёты: 2026-09-26-codebase-review.md (13 находок) и 2026-09-26-changes-011-023-review.md (7 находок).

Пометки: ✅ — воспроизведено на стенде, 🔎 — вывод по коду.

Итог

Все 13 находок первого ревью закрыты, остаточные замечания указаны ниже. Все 7 находок ревью изменений 011–023 закрыты изменением 024. Состояние системы:

• alembic check без расхождений;
• 14 тестов проходят;
• контейнер работает от непривилегированного пользователя и в статусе healthy;
• UI работает под CSP без ошибок в консоли.

Новый анализ выявил одну заметную ошибку: неверная ёмкость и загрузка частично разбитого префикса (№ 1). Её усиливает сценарий автовыделения подсетей из изменения 010. Кроме неё — один риск доступности (№ 2) и несколько низких замечаний.

#	Серьёзность	Область	Кратко
1	Средняя	Данные / отображение	Ёмкость родителя равна сумме ёмкостей листьев: после выделения одного /30 у /24 ёмкость 254 → 2, загрузка 16 % → 100 % ✅
2	Средняя	Доступность	Анонимный клиент может держать администратора заблокированным: 5 запросов каждые 10 минут, верный пароль тоже отклоняется ✅
3	Низкая	UI	Экран «Префиксы» загружает не больше 1 000 префиксов и показывает число загруженных, а не total; дерево молча усекается 🔎
4	Низкая	Тесты / эксплуатация	Тесты работают с БД рабочего стенда: создают данные и сбрасывают настройки журнала на значения по умолчанию, а не на исходные 🔎
5	Низкая	Конкурентность	Дерево префиксов (parent_id) поддерживается только кодом: параллельное создание пересекающихся префиксов может оставить неверного родителя 🔎
6	Низкая	API	Явный parent_id при создании префикса может указать не самого узкого родителя, и дерево разойдётся с вложенностью CIDR 🔎
7	Низкая	Безопасность	Лимит в 20 попыток по IP не сериализуется между разными логинами (остаточное замечание 024) 🔎
8	Инфо	API	PATCH /organizations/{id} требует полное тело (OrgIn), частичное обновление даёт 422 🔎
9	Инфо	Эксплуатация	Быстрый старт в README (docker compose up) поднимает проект ipam_control, а рабочий стенд — ipam_control_006; легко перепутать поставки 🔎

---

Находки

1. Ёмкость и загрузка частично разбитого префикса ✅

_capacities (app/api/v1/prefixes.py) считает ёмкость родителя как сумму ёмкостей листьев, а used (_usage) — по всем адресам поддерева, включая адреса самого родителя вне дочерних префиксов. После изменения 010 («Добавить вложенный (авто)») частичное разбиение стало обычным сценарием, и расхождение проявляется сразу. Воспроизведение: 172.20.0.0/24 с 40 адресами — used 40 / capacity 254 / 16 %. После автовыделения одного /30 — used 40 / capacity 2 / 100 %. Экран адресов того же префикса по-прежнему показывает ёмкость 254. Та же причина искажает «Обзор»: assigned считается по всем адресам IPv4, а capacity — только по листьям. Адреса в нелистовых префиксах завышают загрузку. Рекомендация:

• ёмкость любого префикса — размер его собственной подсети (capacity(prefix));
• на «Обзоре» ёмкость — сумма по корневым префиксам (без родителя) каждого VRF;
• used оставить как есть: считается по поддереву, дублей нет благодаря изменению 011.

Так числа станут согласованы с экраном адресов.

2. Блокировка входа администратора анонимным клиентом ✅

Лимит по логину (5 неудач за 10 минут, изменение 012) действует для всех IP. Верный пароль во время блокировки тоже отклоняется. Проверено при сверке 012: 429 и Retry-After на верный пароль. Любой, кто знает логин (например, admin из README), может отправлять 5 неверных попыток раз в 10 минут и держать администратора заблокированным. Риск был отмечен в плане 012; ниже вариант смягчения. Рекомендация:

• блокировать по паре «логин + IP» (5 попыток), а общий по логину порог сделать выше (например, 50) и рассматривать его как сигнал в журнале;
• альтернатива: не блокировать вход с IP, с которого этот пользователь успешно входил за последние N дней.

3. Усечение списка префиксов в UI 🔎

screens.prefixes и экран адресов запрашивают /prefixes?limit=1000, а заголовок, вкладки и подвал показывают all.length, то есть число загруженных префиксов, а не total. Родительский список в «Новом префиксе» и умолчание размера в «Добавить вложенный» тоже строятся по усечённому набору. Организация, у которой /22 разбит на /30 (256 префиксов), быстро выходит за 1 000. Рекомендация:

• минимум: показывать total и предупреждение «загружено 1 000 из N»;
• лучше: догружать страницы (offset) или получать дерево для конкретного VRF и ветки.

4. Тесты работают с данными стенда 🔎

tests/conftest.py работает с API и БД из .env, то есть со стендом ipam_control_006, где лежат демо-данные.

• test_rotation_by_age_and_count в finally выставляет retention_days=90, max_entries=100000 вместо значений, которые были до теста. Пользовательские настройки ротации теряются.
• Остатки неудачных прогонов (организации test-*) видны в UI.

Рекомендация:

• отдельный compose-проект или БД для тестов (например, docker compose -p ipam_test с другим .env);
• в тесте ротации сохранять и восстанавливать исходные настройки.

5. Дерево префиксов при параллельных изменениях 🔎

attach_to_tree определяет родителя и забирает вложенные префиксы по снимку данных в транзакции. Два параллельных запроса могут создать в одном VRF пересекающиеся префиксы разной длины (ручное создание .0/29 и автовыделение .4/30). Уникальность (vrf_id, prefix) это не ловит, и parent_id останется неверным. FOR UPDATE в allocate_subnet блокирует только строку родителя. Рекомендация: в create_prefix, allocate_subnet и _move_to_vrf брать pg_advisory_xact_lock по vrf_id: изменения дерева одного VRF выполняются по одному. Дешёвый и полный вариант.

6. Явный parent_id против вложенности CIDR 🔎

create_prefix с parent_id (keep_parent=True) проверяет только, что новый префикс входит в указанный родитель. Если существует более узкий префикс, который тоже содержит новый, дерево будет указывать на более широкого родителя. _capacities и _depths работают по parent_id, _usage — по вложенности CIDR, и эти расчёты разойдутся. Рекомендация: если указан не самый узкий родитель, возвращать 422 или игнорировать parent_id и вычислять родителя автоматически, как без параметра.

7. Лимит по IP между разными логинами 🔎

Сериализация из изменения 024 работает по логину. Параллельный перебор многих разных логинов с одного IP проходит проверку лимита по IP одновременно и может превысить 20 попыток на степень параллелизма. Рекомендация: вторая advisory-блокировка по hashtext(ip), брать её после блокировки логина, всегда в одном порядке.

8. PATCH организации требует полное тело 🔎

update_org принимает OrgIn (обязательные name и inn), поэтому PATCH с одним полем возвращает 422. UI отправляет полную форму, так что пользователи этого не замечают, но семантика PATCH нарушена. Рекомендация: схема OrgUpdate с необязательными полями и exclude_unset, как у остальных PATCH.

9. Две поставки стенда 🔎

В README быстрый старт использует docker compose up -d --build, то есть проект ipam_control: он собирает старые контейнеры ipam_control-* на тех же портах. Актуальный стенд поднят как docker compose -p ipam_control_006. Рекомендация: задать имя проекта в docker-compose.yml (name: ipam_control) и перенести стенд на него, либо описать в README, какой проект рабочий.

---

Статус находок первого ревью

№	Находка	Статус
1	Один IP дважды в VRF	✅ закрыта (011); остаток: старые адреса сети/broadcast блокируют охватывающий префикс (на стенде 0)
2	Перебор паролей и вытеснение журнала	✅ закрыта (012, 024); остаток — № 2 и № 7 этого отчёта
3	500 на отрицательных limit/offset	✅ закрыта (013)
4	Ресурсоёмкий экран адресов	✅ закрыта (014)
5	Адрес сети/broadcast	✅ закрыта (015, 024)
6	Роль по умолчанию admin	✅ закрыта (016)
7	Встроенный JWT-секрет	✅ закрыта (017)
8	null в PATCH → ложный 409	✅ закрыта (018)
9	N+1	✅ в основном закрыта (019); «Обзор» и _prefix_outs по-прежнему читают всё дерево организации
10	Автоназначение и вложенные префиксы	✅ закрыта (020)
11	Гонки	✅ закрыта (021); новая гонка по дереву префиксов — № 5
12	Эксплуатация	✅ закрыта (022); APP_BIND по умолчанию 0.0.0.0 (сознательно)
13	Мелочи	✅ закрыта (023)

Предлагаемый порядок

1. № 1 — ёмкость префиксов: небольшая правка в _capacities и overview, заметна пользователю сразу.
2. № 2 и № 7 — политика блокировки входа.
3. № 3 и № 5 — масштаб и целостность дерева префиксов.
4. № 4 и № 9 — изоляция тестового окружения и порядок в поставках.
5. № 6 и № 8 — по желанию.