# Ревью серверной части — Этап 1: Security, Auth, Device Integration **Дата:** 2026-06-02 **Охват:** `App.php`, `Tools/DeviceAPI/Base.php`, `Models/Devices.php`, `Controllers/EventsController.php`, `Tools/RateLimiter.php`, `Sessions.php`, `Middleware/Entity.php`, Entity-классы. --- ## 🔴 Критические ### 1. RateLimiter не работает в PHP-FPM/Apache (`RateLimiter.php`) `RateLimiter` хранит `$requests` в **static property процесса**. При стандартном deployment (PHP-FPM, Apache mod_php) каждый HTTP-запуск — новый процесс, поэтому счётчик сбрасывается. Rate limit фактически отсутствует. **Рекомендация:** перенести на Redis / memcached / DB, или хотя бы на file-based storage. ### 2. `/events/new` не требует авторизации (`Routes.php`, `EventsController.php`) `check_api_auth()` пропускает любой URI, не начинающийся с `/api/v1/`. `/events/new` — вне этого префикса, поэтому endpoint публичен. Любой может отправить события от имени любого устройства, зная его `device_hard_id`. **Рекомендация:** добавить `device_token` в заголовок Authorization и проверять его в `EventsController::new_event` перед `by_hard_id`, или перенести endpoint под `/api/v1/` с auth guard. ### 3. Race condition в `connect_new_device()` (`Models/Devices.php`) Нет atomic проверки статуса устройства. Два параллельных запроса могут одновременно увидеть `"setup"`, создать две записи в `devices`+`device_auth`, и оба пройдут. Устройство перейдёт в normal mode, но в БД останутся дубли. **Рекомендация:** `UNIQUE` constraint на `device_hard_id` + `status='active'`, или distributed lock через `SELECT ... FOR UPDATE` внутри транзакции. ### 4. `Entity::update()` unconditionally требует `update_at` (`Middleware/Entity.php`) `Entity::update()` всегда inject-ит `update_at` в UPDATE SQL. `DeviceAuth` (таблица `device_auth`) **не содержит** `update_at` в `$fields`, и скорее всего нет и в production schema. Вызов `DeviceAuth::kill()` → `update()` вызовет SQL error. **Рекомендация:** добавить `update_at TEXT` в production schema `device_auth`, или сделать `update_at` опциональным в Entity (например, проверять `in_array($this->field_name_of_update_at, static::$fields)`). --- ## 🟡 Высокие ### 5. Post-commit remote state inconsistency в `connect_new_device()` Транзакция commit-ится, а затем вызывается: - `$device->set_device_token($device_token)` → `remote_set_token` на устройство - `$device->device_api()->set_device_name($name)` → `POST /set_device_name` Если устройство offline или не отвечает после commit, в БД устройство зарегистрировано, но физически оно не знает токен/имя. Управление невозможно. **Рекомендация:** либо делать remote setup **до** commit (и rollback при fail), либо иметь флаг `setup_pending` + background retry job. ### 6. `DeviceAuth::kill()` не инвалидирует кэш `device_auth_instance` `Device::auth()` кэширует `device_auth_instance`. После `$device_auth->kill()` статус меняется на `"killed"`, но кэш в `$device->device_auth_instance` остаётся со старым статусом `"active"`. Последующие вызовы `$device->auth()->is_active()` вернут `true`. **Рекомендация:** сбрасывать `$device->device_auth_instance = null` в `DeviceAuth::kill()` или в `Device::set_device_token()`. ### 7. `EventsController::new_event` — `device_id` приходит извне без подписи Устройство доказывает свою идентичность только через `device_hard_id`, который передаётся в POST body. Нет HMAC, timestamp, nonce. Replay attack возможен. **Рекомендация:** добавить `Authorization: Bearer ` (см. пункт 2) и/или подпись полезной нагрузки. --- ## 🟢 Средние ### 8. `generate_token(16)` — низкая энтропия 16 hex-символов = 64 бита. Для устройственных токенов в локальной сети это терпимо, но на грани. **Рекомендация:** поднять до 32 (128 бит) для `device_token`. ### 9. `Base::request()` — нет валидации `$ip_address` `http://` + произвольная строка. Если `$ip_address` содержит path (`192.168.1.1/path`), SSRF возможен. Контролируется server-side, но всё же. **Рекомендация:** валидировать `filter_var($ip_address, FILTER_VALIDATE_IP)` в конструкторе. ### 10. `Entity::update()` — тихий success при SQL error? `Entity::update()` вызывает `$this->thin_builder()->update(...)` и не проверяет return value. `ThinBuilder::update` возвращает `Statement` или `null`? Возможно, exception вылетит (PDO ERRMODE_EXCEPTION), но если нет — тихий fail. **Рекомендация:** явно проверять результат `thin_builder()->update()` и возвращать `false` при ошибке. ### 11. `Sessions::get_current_session()` — лишний UPDATE на каждый запрос Обновление `last_using_at` при каждом вызове создаёт write load. Для домашнего сервера не критично, но при масштабе заметно. **Рекомендация:** обновлять раз в N минут (например, если delta > 60s). ### 12. `EventsController` — `set_time_limit(10)` может обрезать handlers Если `channel_alias_device_event_call` и последующие вызовы занимают >10s — fatal. Это фоновая обработка после flush. **Рекомендация:** `set_time_limit(0)` или background queue (cron / worker). ### 13. `check_api_auth()` — Bearer token parsing `substr($auth_header, 7)` без проверки длины. Header `"Bearer"` (без пробела) даст пустой token, который пройдёт в `!$token` → 401. Не критично, но стоит использовать `str_starts_with` + `explode(' ', $auth_header, 2)[1] ?? null`. --- ## ✅ Что сделано хорошо - **Auth guard разделён** на `check_api_auth()` (чистая логика) и `api_auth_guard()` (side-effects). Это позволяет тестировать без `exit`. - **Cookie security flags** — `httponly`, `secure`, `samesite=Strict`. - **Retry/backoff** в `DeviceAPI\Base` — exponential backoff, clear separation через `executeCurl`/`getCurlInfo`. - **Транзакции** в `connect_new_device` — rollback при ошибке. - **PDO prepared statements** в ThinBuilder — SQL injection минимизирован. --- ## 📋 Action Items (итог) | Приоритет | Задача | Файл | |-----------|--------|------| | 🔴 Крит | Починить rate limiter для stateless PHP | `RateLimiter.php` | | 🔴 Крит | Защитить `/events/new` токеном устройства | `Routes.php`, `EventsController.php` | | 🔴 Крит | Убрать race condition в `connect_new_device` | `Models/Devices.php` | | 🔴 Крит | Добавить `update_at` в `device_auth` schema или сделать optional | `Middleware/Entity.php` / DB schema | | 🟡 Выс | Post-commit remote calls → pre-commit или pending flag | `Models/Devices.php` | | 🟡 Выс | Инвалидировать `device_auth_instance` при `kill()` | `Entities/DeviceAuth.php` | | 🟡 Выс | Replay-защита на `/events/new` | `EventsController.php` | | 🟢 Сред | Увеличить энтропию device_token | `Utils.php` / `Models/Devices.php` | | 🟢 Сред | Валидировать IP в `DeviceAPI\Base` | `Tools/DeviceAPI/Base.php` | | 🟢 Сред | Проверять return value в `Entity::update()` | `Middleware/Entity.php` | | 🟢 Сред | Throttle `last_using_at` updates | `Sessions.php` | | 🟢 Сред | `set_time_limit(0)` в EventsController | `Controllers/EventsController.php` |