Qadam Roadmap
← Назад к библиотеке
проектdocs/audits/security-review.md

security-review.md

Обновлён 1 апр. 2026 г., 12:41 · 0 комментариев

ИсходникСкачать

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

  • Статус документа: historical snapshot
  • Актуально на: 28 марта 2026 года
  • Владелец: backend/platform-команда
  • Пересмотр: при следующем цикле аудита, security review или ревизии противоречий в документации и спеках
  • Область применения: audit-слой проекта: снимки состояния, реестры расхождений и результаты ревизий
  • Связанные документы:

Ревью — 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.