## Паспорт документа

- Статус документа: historical snapshot
- Актуально на: 28 марта 2026 года
- Владелец: backend/platform-команда
- Пересмотр: при следующем цикле аудита, security review или ревизии противоречий в документации и спеках
- Область применения: audit-слой проекта: снимки состояния, реестры расхождений и результаты ревизий
- Связанные документы:
  - [Индекс документации](../README.md)
  - [Текущее состояние](../project/current-state.md)
  - [Roadmap](../project/roadmap.md)

## Ревью — 2026-03-27 | исторический снимок legacy checkout `/data/uzbek`

**Суть изменения:** На момент review кодовая база жила в legacy checkout `/data/uzbek`, а roadmap/docs-портал был встроен в product web-приложение. В текущем production это уже не так: runtime переведён на `qadam-core` и `qadam-web`, а roadmap вынесен в отдельный standalone-сервис `apps/roadmap`.

## Статус исполнения review

На текущий момент замечания из этого review больше не являются открытым блокером для продолжения работ:

- roadmap-портал больше не доступен через primary host, а mutation routes дополнительно ограничены trusted roadmap host и same-origin проверкой;
- refresh token consume переведён на атомарный Redis-script;
- throttling behind proxy теперь работает с доверенным loopback proxy вместо адреса nginx;
- API runtime дополнительно переведён на loopback-only bind, чтобы production-контур не обходился мимо nginx;
- public seller profile больше не включает скрытые/архивные айтемы;
- lead/review write-paths принимают только публично доступные айтемы;
- `SELLER_STAFF` больше не редиректится во frontend в неподдержанный seller area.

Сам review остаётся полезным как исторический снимок найденных дефектов и их приоритета на момент обнаружения.

Важно:

- file references ниже отражают pre-split layout на момент обнаружения проблемы;
- текущие пути и runtime нужно брать из `docs/project/current-state.md`, `docs/operations/deployment-runbook.md` и `specs/qadam-platform/implementation.md`;
- для roadmap-портала актуальный код теперь находится в `/data/qadam-web/apps/roadmap`, а не в `apps/web/src/app/roadmap`.

---

### Замечания

#### [КРИТИЧНО] — Roadmap-портал открыт и изменяем без серверной аутентификации
- **Файл:** исторически `apps/web/src/proxy.ts`, `apps/web/src/app/roadmap/manage/route.ts`, `apps/web/src/app/roadmap/files/[documentId]/route.ts`; в текущем runtime релевантные остаточные проверки нужно искать в `/data/qadam-web/apps/web/src/proxy.ts` и `/data/qadam-web/apps/roadmap/src/app/*`
- **Категория:** Безопасность
- **Описание:** `/roadmap` не входит в `PROTECTED_PREFIXES`, поэтому на основном домене эти маршруты остаются публичными. При этом `manage` без каких-либо server-side проверок принимает upload/delete/comment POST-запросы, а `files/[documentId]` и сама библиотека документов отдают внутренние markdown-файлы всем посетителям. Это полностью обходит ожидание, что портал защищён только `basic auth` на отдельном хосте, и превращает внутреннюю документацию и её изменяемое хранилище в анонимно доступный контур.
- **Направление исправления:** Защиту roadmap нужно перенести в серверный код Next.js: жёстко запрещать доступ на primary host и/или требовать независимую аутентификацию в layout/route handlers, а не полагаться на nginx и client-side rewrite.

---

#### [ВЫСОКИЙ] — Ротация refresh token не атомарна и допускает гонку повторного использования
- **Файл:** `apps/api/src/modules/auth/refresh-token.store.ts` (строка 20), `apps/api/src/modules/auth/auth.service.ts` (строка 123)
- **Категория:** Безопасность
- **Описание:** `consume()` читает JTI через `GET`, а удаляет отдельным `DEL`. Два параллельных refresh-запроса могут оба успеть прочитать один и тот же token family member до удаления, после чего оба получат новые токены. В таком состоянии защита от reuse/theft перестаёт быть надёжной: single-use семантика refresh token фактически не гарантируется.
- **Направление исправления:** Поглощение refresh token должно быть атомарным (`GETDEL`, Lua script или эквивалентная Redis-транзакция), а повторный параллельный запрос должен детектироваться до выдачи новой сессии.

---

#### [ВЫСОКИЙ] — Throttling “behind proxy” настроен без trust proxy и считает IP nginx
- **Файл:** `apps/api/src/main.ts` (строка 23), `apps/api/src/common/guards/throttler-behind-proxy.guard.ts` (строка 5)
- **Категория:** Безопасность
- **Описание:** Guard рассчитывает tracker из `req.ips[0]`/`req.ip`, но `FastifyAdapter` создаётся без `trustProxy`. На production за nginx это обычно означает, что rate limit для `login`/`register`/`refresh` будет вестись по адресу reverse proxy, а не реального клиента. Один атакующий сможет исчерпать лимит сразу для всех пользователей, а сами ограничения потеряют смысл как anti-abuse механизм.
- **Направление исправления:** Нужно явно включить `trustProxy` на Fastify-уровне и проверить e2e-сценарием, что throttling действительно различает клиентов по `X-Forwarded-For`.

---

#### [СРЕДНИЙ] — Публичный seller profile возвращает скрытые/архивные айтемы
- **Файл:** `apps/api/src/modules/seller/repositories/seller.repository.ts` (строка 58), `apps/api/src/modules/item/item.service.ts` (строка 140)
- **Категория:** Корректность
- **Описание:** Публичный seller query фильтрует айтемы только по `moderationStatus: 'ACTIVE'`. Одновременно `archiveItem()` скрывает айтем через `isVisible = false`, не меняя moderation status. В результате архивный айтем перестаёт показываться в каталоге, но остаётся видимым на публичной странице продавца, что ломает статусную модель публикации.
- **Направление исправления:** Для публичного seller profile нужно применять тот же предикат публикации, что и в каталоге (`ACTIVE` + `isVisible = true`), желательно из одного общего доменного правила.

---

#### [СРЕДНИЙ] — Лиды и отзывы можно создавать по непубличным айтемам
- **Файл:** `apps/api/src/modules/lead/repositories/lead.repository.ts` (строка 107), `apps/api/src/modules/review/repositories/review.repository.ts` (строка 63), `apps/api/src/modules/catalog/repositories/catalog.repository.ts` (строка 74)
- **Категория:** Корректность
- **Описание:** Запись лида и отзыва проверяет только факт существования `itemId`, тогда как публичные read-paths обслуживают лишь `ACTIVE` + `isVisible` айтемы. Если buyer знает UUID айтема из предыдущего состояния системы, seller/admin-интерфейса или лога, он может создавать обращения и отзывы по скрытым, отклонённым или ещё не опубликованным сущностям. Это засоряет операционные данные и нарушает бизнес-правило “работаем только с допустимыми к публикации айтемами”.
- **Направление исправления:** В write-paths нужна та же доменная проверка доступности айтема, что и в публичных read-paths, либо отдельное явное правило для допуска только нужных статусов.

---

#### [СРЕДНИЙ] — Роль SELLER_STAFF маршрутизируется в seller area, но backend её не поддерживает
- **Файл:** `apps/web/src/proxy.ts` (строка 13), `apps/api/src/modules/seller/seller.controller.ts` (строка 64), `apps/api/src/modules/item/item.controller.ts` (строка 46), `apps/api/src/modules/staff/staff.controller.ts` (строка 102)
- **Категория:** Архитектура
- **Описание:** Web routing и post-login redirect считают `SELLER_STAFF` допустимым пользователем для `/seller`, но seller-контроллеры API почти везде жёстко требуют `user.type === 'SELLER'`. В итоге аккаунты сотрудников создаются и логинятся, однако попадают в кабинет, где базовые endpoint'ы сразу отвечают `403`. Это не просто UX-дефект, а рассинхрон контракта авторизации между слоями.
- **Направление исправления:** Нужно либо реализовать серверную permission-модель для `SELLER_STAFF`, либо перестать направлять эту роль в seller area до появления поддержанного контракта.

---

### Общая оценка
[КРИТИЧНО]
Текущее состояние не выглядит безопасным для дальнейшего мержа без закрытия roadmap access control и auth/throttling дефектов. Дополнительно видно, что зелёные quality gates не покрывают web-регрессии: `pnpm test` проходит, но `apps/web` фактически отвечает заглушкой `no tests yet`, поэтому часть уже найденных проблем не имеет автоматической защиты от повторного появления.

## Ревью — 2026-03-28 | /data/qadam-core working tree (requirements-api-registration completion)

**Суть изменения:** Пакет завершает `requirements-api-registration` и затрагивает seller profile, Telegram verify, admin seller status management и upload endpoint. В дельте появились новые проверки и response-поля, но часть поведения расходится с требованиями и не вся критичная логика закрыта корректным regression coverage.

---

### Замечания

#### [ВЫСОКИЙ] — Seller profile начал запрещать валидные contact phone/email по семантике аккаунта
- **Файл:** `apps/api/src/modules/seller/seller.service.ts` (строка 132)
- **Категория:** Корректность
- **Описание:** `createProfile()` и `updateProfile()` теперь проверяют `dto.phone`/`dto.email` через `SellerRepository.isPhoneTaken()` и `isEmailTaken()`, а те смотрят только в `Account.phone`/`Account.email` (`apps/api/src/modules/seller/repositories/seller.repository.ts`, строки 129-155). Это делает контактные поля seller-профиля глобально уникальными на уровне учётных данных, хотя требования для unified seller registration явно фиксируют другое правило: уникальны только account `phone`/`email`, а `contactPhone`/`contactEmail` лишь предзаполняются из них и могут отличаться (`docs/product/requirements-api-registration.md`, строки 482-495). Регресс дополнительно закреплён тестами на `PHONE_TAKEN`/`EMAIL_TAKEN` для contact-полей (`apps/api/src/modules/seller/seller.service.spec.ts`, строки 274-301).
- **Направление исправления:** Нужно развести уникальность account credentials и контактных данных seller-профиля и синхронизировать legacy profile endpoints с контрактом unified registration.

---

#### [СРЕДНИЙ] — Telegram verification code остаётся re-usable при гонке параллельных запросов
- **Файл:** `apps/api/src/modules/seller/seller.service.ts` (строка 295)
- **Категория:** Безопасность
- **Описание:** `verifyTelegram()` сначала читает код и проверяет `used` вне транзакции, а затем `bindTelegram()` просто делает `update({ where: { code }, data: { used: true } })` (`apps/api/src/modules/seller/repositories/seller.repository.ts`, строки 471-510). При двух параллельных запросах с одним и тем же кодом оба могут пройти pre-check до записи `used = true`, после чего обе операции завершатся успешно. Для одноразового verification code это нарушение single-use semantics и реальный race condition. Покрытие ограничено happy-path тестом возврата `username` (`apps/api/src/modules/seller/seller.service.spec.ts`, строки 304-356); негативного/concurrency regression test нет.
- **Направление исправления:** Одноразовость verification code нужно обеспечивать атомарно на write-path, а не отдельным чтением и последующим update.

---

#### [СРЕДНИЙ] — После блокировки seller access-token запросы теряют доменную причину отказа и маскируются под generic 401
- **Файл:** `apps/api/src/common/guards/jwt-auth.guard.ts` (строка 34)
- **Категория:** Корректность
- **Описание:** Hardening действительно перечитывает аккаунт через `AuthService.getMe()` и тем самым режет уже выданные access token при `BLOCKED`/`UNDER_REVIEW` (`apps/api/src/modules/auth/auth.service.ts`, строки 194-211). Но `JwtAuthGuard` ловит эти `DomainException` и превращает их в `UnauthorizedException('Invalid or expired token')`. В результате protected routes больше не отдают ожидаемые доменные причины `ACCOUNT_BLOCKED` / `ACCOUNT_UNDER_REVIEW`; blocked account выглядит как просроченный токен, а поведение guarded routes расходится с `AuthService` и refresh flow.
- **Направление исправления:** Guard должен сохранять доменные ошибки проверки статуса, а не нормализовать их до generic invalid token.

---

#### [НИЗКИЙ] — Regression coverage не закрывает новый access-token invalidation path
- **Файл:** `apps/api/src/common/guards/jwt-auth.guard.spec.ts` (строка 34)
- **Категория:** Качество
- **Описание:** Тест guard'а остался на старом контракте: он создаёт `JwtAuthGuard` без `AuthService` и проверяет `canActivate()` как синхронный метод (`apps/api/src/common/guards/jwt-auth.guard.spec.ts`, строки 31-79), хотя в новой реализации guard стал async и критично зависит от `authService.getMe()`. Параллельно `admin.service.spec.ts` проверяет только вызов `revokeAllRefreshTokens()` (`apps/api/src/modules/admin/admin.service.spec.ts`, строки 260-287), но не фиксирует, что уже выданный access token после смены статуса действительно отклоняется и с правильной ошибкой.
- **Направление исправления:** Нужен рабочий guard/controller-level regression test на поведение существующей access-сессии после admin status change.

---

### Общая оценка
[ТРЕБУЕТ ВНИМАНИЯ]
Пакет закрыл часть предыдущих дефектов, включая стабильность upload-валидации и refresh-token revocation при смене статуса seller. Но текущая дельта всё ещё содержит одну прямую контрактную регрессию в seller profile semantics, race condition в Telegram verify и незавершённый hardening на уровне HTTP/error contract и regression coverage.
