ros_control/docs/reviews/2026-09-28-1735-codebase-review.md

Ревью кодовой базы ros_control — 2026-09-28 17:35 MSK

Третье ревью. Состояние на коммит 7c830be. Тесты: 33/33 проходят. Предыдущие ревью: 2026-09-27 (коммит 26dd1f8), 2026-09-28 12:43 (коммит c407972).

Итог

Из 16 замечаний закрыто 11, частично — 1 (п. 13), открыто 4 (п. 11, 12, 15, 16; п. 14 — открыт, но зависит от появления reverse-proxy). С предыдущего ревью закрыт п. 9 (остаток), п. 13 задокументирован с неточностью.

Изменение	Коммит	Пункты
018-correctness-consistency	123b5ab	5, 6, 7
019-performance-scaling	ba8ac7b	8, 9 (частично), 10
020-custom-select-menus	6b5c521	— (доработка UI по запросу)
021-security-hardening	c407972	1, 2, 3, 4
022-async-db-remainder	0bda003	9 (остаток)
023-ros-downgrade	ae826fc	— (исправление отображения и новая функция по запросу)
024-readme-restructure	7c830be	13 (документация)

Статус замечаний

Безопасность, корректность, производительность (п. 1–10)

№	Замечание	Статус	Подтверждение
1	Небезопасные значения секретов по умолчанию	✅ 021	app/config.py::insecure_settings, проверка в lifespan
2	Вход в UI без защиты от перебора	✅ 021	Блокировка по IP в app/ui/routes.py::login
3	Cookie сессии без флага Secure	✅ 021	SESSION_COOKIE_SECURE
4	Открытый редирект	✅ 021	_safe_next
5	Одиночное удаление бэкапа в обход сервиса	✅ 018	backups.delete_many([key])
6	Две системы миграций	✅ 018	Всё в migrations.run
7	Синхронная групповая смена канала	✅ 018	Задачи set_channel
8	Каждое открытие «Бэкапов» перечитывает бакет	✅ 019	backups.bucket_objects с TTL
9	Синхронная БД в async-коде	✅ 019 + 022	14 оставшихся мест переведены на asyncio.to_thread; тест-линтер test_no_sync_db_calls_in_async_functions не даёт регресса
10	Состояние в памяти процесса	✅ 019	app/process_lock.py

Эксплуатация и новые замечания (п. 11–16)

№	Замечание	Приоритет	Статус	Подтверждение
11	Нет healthcheck, логирование не настроено	Низкий	❌ Открыт	Нет HEALTHCHECK в Dockerfile и docker-compose.yml, нет /healthz; logging не настраивается
12	Тесты в одном файле	Низкий	❌ Открыт, растёт	tests/test_app.py: 570 → 753 → 936 строк, 20 → 28 → 33 теста
13	Копирование БД в режиме WAL не описано	Средний	🟡 024, неточность	README «Эксплуатация» советует sqlite3 <БД> ".backup …", но БД — в Docker-томе, а в контейнере (python:3.12-slim) нет утилиты sqlite3; на хосте она есть, но том напрямую не смонтирован. Рабочий способ — sqlite3.Connection.backup через docker exec … python -c …
14	За reverse-proxy блокировка входа видит IP прокси	Низкий	❌ Открыт	Разбора X-Forwarded-For и доверенных прокси нет; прокси сейчас нет, ограничение описано в README
15	refresh=1 остаётся в адресе «Бэкапов»	Низкий	❌ Открыт	app/ui/routes.py:485 — refresh_url; повторные перезагрузки идут мимо кэша
16	Порт стенда не параметризован	Низкий	❌ Открыт	docker-compose.yml:8 — "8000:8000"; порт 8000 хоста занят посторонним процессом, стенд работает на 8001 через override-файл вне репозитория

Новые наблюдения

№	Наблюдение	Где	Приоритет
17	Откат ROS (023) не проверен на реальном устройстве: поведение RouterOS 7 update/install при версии канала старше установленной подтверждено только статусом устройства (New version is available)	app/ros/operations.py::downgrade_ros	Средний — до первого реального отката
18	После отката ROS прошивка RouterBOARD остаётся новее; отдельно не обрабатывается и не показывается как особое состояние	app/ros/operations.py, колонка «Upgrade FW»	Низкий
19	Тест-линтер п. 9 видит одну ступень транзитивности: функция, обращающаяся к БД через две и более промежуточных, не будет обнаружена	tests/test_app.py::test_no_sync_db_calls_in_async_functions	Низкий

Проверка UI

Ручная проверка интерфейса пользователем по изменениям 019–023 не подтверждена (отмечено в summary.md и сообщениях коммитов); для 020 и 023 это основной способ проверки — автотестов JS нет, реальный откат выполняет пользователь.

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

1. Изменение 025 — эксплуатация: п. 11 (/healthz, HEALTHCHECK, базовая настройка логов), п. 15 (редирект без refresh=1), п. 16 (${HTTP_PORT:-8000} в compose — override-файл больше не нужен), п. 13 (правка совета о копировании БД в README).
2. Реальная проверка отката (п. 17) пользователем на выбранном устройстве; по результату — при необходимости п. 18.
3. П. 12 — разбить тесты по модулям отдельным изменением; заодно п. 19 (транзитивное замыкание в линтере).
4. П. 14 — при появлении reverse-proxy.