Files
ripe-cidr-collector/docs/review-2026-09-21.md
T
ayurishchevandClaude Sonnet 5 0155fd3f38 Withhold addresses after the database is recreated without a backup
When ripe.db is corrupted and no valid backup exists, a new empty database
is created with a db_recreated.json marker; /addresses and /addresses/diff
answer 503 until the collector gathers data again, /health reports
db_recreated. Tests now isolate all state files via conftest.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-21 09:56:43 +03:00

7.0 KiB
Raw Blame History

Ревью кодовой базы (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 Не начато
6 Низкая /health без авторизации отдаёт внутренние пути и текст ошибки (last_restore). api_server.py:214-226 Не начато
7 Низкая run_job меняет job_state вне _state_lock, хотя write_status читает под блокировкой (гонка безвредна благодаря GIL). collector_daemon.py:72 Не начато
8 Низкая Dockerfile копирует модули явным списком: новый модуль не попадёт в образ, ошибка проявится при запуске (healthcheck поймает, но поздно). Dockerfile:12 Не начато
9 Низкая Запросы к RIPEstat без повторов и без параметра sourceapp. cidr_collector.py:119 Не начато
10 Низкая get_changes: по запросу на каждое значение при проверке наличия; на десятках тысяч изменений станет заметно. db.py Не начато

Архитектура и сопровождение

  • Цикл 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.

Ограничения ревью

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

Рекомендуемый порядок

  1. Доработка «Безопасность ввода и данных»: п. 2, 3, 4 и тесты (plan-input-hardening.md).
  2. Находка 1: после порчи без копий отвечать 503 до первого успешного сбора (plan-loss-guard.md, выполнено).
  3. Пункты 5-10 объединить с доработкой наблюдаемости (п. 2 плана из анализа).