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 — по желанию.