diff --git a/README.md b/README.md index d819909..f4079f5 100644 --- a/README.md +++ b/README.md @@ -203,4 +203,5 @@ venv/bin/python -m pytest -q ## Отчёты ревью - [Ревью кодовой базы 2026-09-27](docs/reviews/2026-09-27-codebase-review.md) (→ 018–021) -- [Повторное ревью 2026-09-28](docs/reviews/2026-09-28-1243-codebase-review.md) (→ 022; открыты п. 11, 12, 14–16) +- [Повторное ревью 2026-09-28 12:43](docs/reviews/2026-09-28-1243-codebase-review.md) (→ 022, 024) +- [Третье ревью 2026-09-28 17:35](docs/reviews/2026-09-28-1735-codebase-review.md) (открыты п. 11, 12, 14–19) diff --git a/docs/reviews/2026-09-28-1735-codebase-review.md b/docs/reviews/2026-09-28-1735-codebase-review.md new file mode 100644 index 0000000..50923df --- /dev/null +++ b/docs/reviews/2026-09-28-1735-codebase-review.md @@ -0,0 +1,68 @@ +# Ревью кодовой базы ros_control — 2026-09-28 17:35 MSK + +Третье ревью. Состояние на коммит `7c830be`. Тесты: 33/33 проходят. +Предыдущие ревью: [`2026-09-27`](2026-09-27-codebase-review.md) (коммит `26dd1f8`), [`2026-09-28 12:43`](2026-09-28-1243-codebase-review.md) (коммит `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.