# Ревью серверной части — Этап 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 <device_token>` (см. пункт 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` |
