ripe-cidr-collector/docs/review-2026-09-21.md

# Ревью кодовой базы (2026-09-21)

**Метод:** структура по графу знаний (405 узлов; код: 7 модулей, 1382 строки), затем чтение кода. Часть находок подтверждена запуском (отмечено). Состояние на момент ревью: коммит `02f7b49`, 20 тестов.

## Структура по графу
- **Связанность модулей:** `api_server` ↔ `db` (13 связей), `api_server` ↔ `cidr_collector` (11), `cidr_collector` ↔ `collector_daemon` (8). Зависимости направлены в основном одним способом, слои читаемы.
- **Хабы:** `api_server.py` (посредничество 0,38), `db.py` (0,23), `cidr_collector.py` (0,20).
- **Циклическая зависимость:** `cidr_collector` импортирует `db`, а `db` берёт пути и настройки из `cidr_collector` отложенным импортом внутри функций.
- **Мёртвый код не найден.** Узлы кода со степенью не более 1 - обработчики, регистрируемые декораторами, и функции через словарь диспетчеризации (`_frr`, `_mikrotik`): ложные срабатывания.

## Находки

| # | Серьёзность | Находка | Где | Статус |
|---|---|---|---|---|
| 1 | **Высокая** | **Пустой список после порчи базы без копий (подтверждено запуском).** Первый запрос даёт 503, затем создаётся новая пустая база, и `/addresses` отвечает `200 []`. Если демон жив, `/health` показывает `ok` при `counts = 0`. Потребители, забирающие список по расписанию, могут стереть свои списки. | `db.py:83-112` | Исправлено (`summary-loss-guard.md`) |
| 2 | Средняя | **500 вместо 401 при нелатинском `X-API-Key` (подтверждено).** `secrets.compare_digest` для `str` работает только с ASCII. Обхода авторизации нет. | `api_server.py:105` | Исправлено (`summary-input-hardening.md`) |
| 3 | Средняя | **500 при некорректном `since` (подтверждено):** `since=²` (`str.isdigit()` истинно для символов юникода, а `int()` их не принимает), число длиннее 4300 цифр (лимит `int`), крайние даты с поясом (`0001-01-01T00:00:00+05:00`, `9999-12-31T23:59:59-05:00`: `OverflowError`). | `api_server.py:152` | Исправлено (`summary-input-hardening.md`) |
| 4 | Средняя | **DNS-адреса без фильтрации (подтверждено).** В списки попадают любые ответы: `127.0.0.1`, `10.x`, `0.0.0.0`. Оставлять нужно только глобальные адреса. | `cidr_collector.py:176-179` | Исправлено (`summary-input-hardening.md`) |
| 5 | Низкая | `/addresses` открывает три соединения и три разных снимка (курсор, ASN, FQDN): части ответа могут относиться к разным моментам, соединения тратят время на PRAGMA и проверку схемы. Достаточно одной сессии с одной транзакцией. | `api_server.py:133-141` | Исправлено (`summary-review-fixes-5-10.md`) |
| 6 | Низкая | `/health` без авторизации отдаёт внутренние пути и текст ошибки (`last_restore`). | `api_server.py:214-226` | Исправлено (`summary-review-fixes-5-10.md`) |
| 7 | Низкая | `run_job` меняет `job_state` вне `_state_lock`, хотя `write_status` читает под блокировкой (гонка безвредна благодаря GIL). | `collector_daemon.py:72` | Исправлено (`summary-review-fixes-5-10.md`) |
| 8 | Низкая | `Dockerfile` копирует модули явным списком: новый модуль не попадёт в образ, ошибка проявится при запуске (healthcheck поймает, но поздно). | `Dockerfile:12` | Исправлено (`summary-review-fixes-5-10.md`) |
| 9 | Низкая | Запросы к RIPEstat без повторов и без параметра `sourceapp`. | `cidr_collector.py:119` | Исправлено (`summary-review-fixes-5-10.md`) |
| 10 | Низкая | `get_changes`: по запросу на каждое значение при проверке наличия; на десятках тысяч изменений станет заметно. | `db.py` | Исправлено (`summary-review-fixes-5-10.md`) |
| 11 | Низкая-средняя | **Восстановление из пустой копии давало `200 []` без метки (найдено трассировкой `_recover()` по графу, подтверждено запуском).** Копия, снятая до первого сбора или во время его сбоев, при настроенных источниках восстанавливалась без защиты из находки 1. | `db.py` (`_recover`, `backup_database`) | Исправлено (`summary-empty-backup-guard.md`) |

## Архитектура и сопровождение
- Цикл `db` ↔ `cidr_collector` обходится отложенным импортом; чище вынести пути и значения по умолчанию в `settings.py`.
- `cidr_collector.py` совмещает четыре роли: помощники конфигурации, файловый обмен с демоном, два сборщика, CLI (связность сообщества 0,09).
- `api_server.py` растёт: схемы, проверки и все эндпоинты в одном модуле (326 строк, наблюдение 11 анализа).

## Что сделано хорошо
- Токен проверяется безопасным сравнением и отказывает по умолчанию (503 без токена).
- Запись атомарна везде (JSON через `os.replace`, SQLite в транзакциях, копии через `.tmp`).
- Сетевые запросы выполняются до транзакции, блокировка записи короткая.
- Ошибки хранилища сведены к одному типу (`StorageError`).
- Порча данных не приводит к потере: карантин и копии.

## Пробелы тестов
Не было проверок задания `backup` в демоне, поля `last_restore` в `/health` и граничных значений `X-API-Key` и `since`; закрыты доработками `input-hardening` и `review-fixes-5-10`.

## Ограничения ревью
- Граф не заменяет чтение кода: 14 связей типа INFERRED между модулями по отдельности не проверялись.
- Сборка MikroTik и FRR на реальном ПО не проверялась (риск 4 анализа).

## Рекомендуемый порядок
1. Доработка «Безопасность ввода и данных»: п. 2, 3, 4 и тесты (`plan-input-hardening.md`).
2. Находка 1: после порчи без копий отвечать 503 до первого успешного сбора (`plan-loss-guard.md`, выполнено).
3. Находка 11 добавлена после трассировки графа и исправлена (`plan-empty-backup-guard.md`). Пункты 5-10 исправлены отдельной доработкой (`plan-review-fixes-5-10.md`, выполнено); из ревью остаются только архитектурные замечания (вынос путей в `settings.py`, разделение `api_server.py` и `cidr_collector.py`).