71 lines
8.3 KiB
Markdown
71 lines
8.3 KiB
Markdown
 
|
# Ревью и тестирование изменений 025–029 — 2026-09-26
|
|||
|
|
|
|||
|
|
**Объём:** незакоммиченные изменения поверх `13e17fb`, выполненные агентом (Sonnet 5) по планам `docs/changes/025…029`.
|
|||
|
|
Изменены `app/api/v1/{auth,overview,prefixes}.py`, `app/models.py`, `app/rotation.py`, `web/app.js`, `README.md`; добавлена миграция `0009_known_logins`.
|
|||
|
|
|
|||
|
|
**Метод:**
|
|||
|
|
- чтение diff;
|
|||
|
|
- автотесты проекта;
|
|||
|
|
- собственные сквозные сценарии на стенде `ipam_control_006`: API, прямые вставки в БД для эмуляции других IP, headless Chromium для UI. Скрипты лежат во временной папке, в репозиторий не добавлены. Временные данные удалены.
|
|||
|
|
|
|||
|
|
## Итог
|
|||
|
|
|
|||
|
|
Все пять изменений реализованы по планам и работают.
|
|||
|
|
|
|||
|
|
| Проверка | Результат |
|
|||
|
|
|---|---|
|
|||
|
|
| `import app.main`, `node --check web/app.js` | ✅ |
|
|||
|
|
| Стенд: `app` и `db` healthy, миграция `0009`, `alembic check` | ✅ расхождений нет |
|
|||
|
|
| `pytest -q` | ✅ **14 passed**, после приведения одной проверки к семантике 025 (см. ниже) |
|
|||
|
|
| Собственные сценарии 025–029 | ✅ **14/14** |
|
|||
|
|
|
|||
|
|
**Правка тестов.** `test_tree_utilization_and_next_free` проверял прежнее правило «ёмкость родителя = сумма листьев» (`capacity == 6`), которое изменение 025 намеренно отменило. Проверка заменена на `capacity == 65534` (размер `/16`), `used == 2`. Агенту трогать тесты было запрещено, поэтому до этой правки тест падал ожидаемо.
|
|||
|
|
|
|||
|
|
## Результаты сценариев
|
|||
|
|
|
|||
|
|
| Изм. | Сценарий | Результат |
|
|||
|
|
|---|---|---|
|
|||
|
|
| 025 | `/24` с 40 адресами, затем автовыделение `/30`: у родителя `40 / 254 / 16 %` (раньше было `40 / 2 / 100 %`) | ✅ |
|
|||
|
|
| 025 | Ёмкость родителя совпадает с `summary.capacity` экрана адресов | ✅ |
|
|||
|
|
| 025 | «Обзор» совпадает с независимым SQL-расчётом: ёмкость — сумма корневых активных IPv4-префиксов, «назначено» — адреса внутри корней | ✅ |
|
|||
|
|
| 026 | Успешный вход добавляет IP в `known_logins` | ✅ |
|
|||
|
|
| 026 | 5 неудач с чужого IP не блокируют вход владельца с его IP | ✅ |
|
|||
|
|
| 026 | 55 неудач по логину с разных IP: вход с известного IP — 200, с неизвестного — 429 | ✅ |
|
|||
|
|
| 026 | 5 неудач с одного IP: 401×4, затем 429; верный пароль с этого IP тоже 429; `session.locked` с `scope=login_ip` | ✅ |
|
|||
|
|
| 027 | 15 накопленных неудач IP, затем 16 параллельных входов на разные логины: проверено ≤ 5 паролей, остальные 429 | ✅ |
|
|||
|
|
| 028 | Организация с 1 101 префиксом: UI показывает «1101 префикс» (total), предупреждения об усечении нет, ошибок в консоли нет | ✅ |
|
|||
|
|
| 029 | 20 гонок «ручной `/29` ∥ автовыделение `/30`» в одном `/24` (15 раз первым выполнился ручной, 5 — автоматический): у всех префиксов родитель — самый узкий охватывающий, 0 расхождений | ✅ |
|
|||
|
|
|
|||
|
|
Замечание о методике. В первом прогоне сценарий 029 давал ложные «FAIL»: эталон ожидал `.4/30`, а при ручном `/29` первым автовыделение законно выбирает `.8/30`. Сценарий переписан на проверку инварианта дерева (родитель — самый узкий охватывающий префикс). Той же причины касается замечание агента о «5 ложных срабатываниях».
|
|||
|
|
|
|||
|
|
## Замечания ревью
|
|||
|
|
|
|||
|
|
| # | Серьёзность | Изм. | Суть |
|
|||
|
|
|---|---|---|---|
|
|||
|
|
| 1 | Низкая | 029 | `delete_prefix`, `update_prefix` и `create_address` читают префикс **до** блокировки VRF 🔎 |
|
|||
|
|
| 2 | Низкая | 026 | `app/rotation.py` импортирует константу из API-модуля (`app.api.v1.auth.KNOWN_IP_DAYS`) 🔎 |
|
|||
|
|
| 3 | Инфо | 028 | Экран адресов загружает все префиксы организации, до 20 000, ради крошек и диалогов 🔎 |
|
|||
|
|
| 4 | Инфо | 026 | Компромисс политики: распределённый перебор получает до 5 попыток на IP 🔎 |
|
|||
|
|
|
|||
|
|
### 1. Чтение префикса до блокировки VRF (029)
|
|||
|
|
- В `delete_prefix` `p.parent_id` читается до `_lock_vrf`. Если параллельно между `p` и его родителем создан промежуточный префикс, дочерние элементы `p` при удалении будут переподвешены к устаревшему родителю.
|
|||
|
|
- В `update_prefix` и `create_address` блокировка берётся по `vrf_id`, прочитанному до неё. Если в это время префикс переносят в другой VRF, будет заблокирован не тот VRF.
|
|||
|
|
|
|||
|
|
Окно гонки узкое. **Исправление:** как в `allocate_subnet`, сначала `SELECT vrf_id`, затем `_lock_vrf`, затем чтение префикса. Другой вариант — `db.refresh(p)` сразу после блокировки.
|
|||
|
|
|
|||
|
|
### 2. Зависимость ротации от API-слоя (026)
|
|||
|
|
`rotation.py` → `app.api.v1.auth`: служебный модуль зависит от роутера, а при импорте ротации вычисляется фиктивный argon2-хэш. Циклического импорта сейчас нет, но связь хрупкая.
|
|||
|
|
**Исправление:** перенести `KNOWN_IP_DAYS` и пороги входа в `app/services.py` или `app/config.py`.
|
|||
|
|
|
|||
|
|
### 3. Объём загрузки на экране адресов (028)
|
|||
|
|
`screens.address` вызывает `loadPrefixes`, то есть загружает до 20 страниц по 1 000 префиксов, причём каждая страница на сервере пересчитывает глубины (`_depths`) по всей организации. Для крупной организации открытие каждого экрана адресов станет заметно медленнее.
|
|||
|
|
**Рекомендация (отдельной задачей):**
|
|||
|
|
- эндпоинт цепочки предков для хлебных крошек;
|
|||
|
|
- загрузка префиксов VRF для диалогов по требованию, при открытии окна.
|
|||
|
|
|
|||
|
|
### 4. Компромисс политики блокировки (026)
|
|||
|
|
После 026 перебор с N разных IP даёт до 5 попыток на каждый IP и до 50 в сумме по логину для неизвестных IP. До 026 общий лимит был 5 на логин. Это осознанная цена за то, что владельца больше нельзя заблокировать анонимно.
|
|||
|
|
Лимит по IP (20) по-прежнему позволяет заблокировать вход всем пользователям за общим NAT. Если это существенно, можно исключать из лимита по IP известные пары «логин + IP».
|
|||
|
|
|
|||
|
|
## Вывод
|
|||
|
|
Замечания 1–2 можно закрыть небольшой правкой, 3–4 — по желанию, отдельными задачами. Изменения 025–029 вместе с правкой теста готовы к коммиту.
|