atomic-code-audit-2026-06-30.md 5.2 KB

Code Audit 2026-06-30 — Новые находки

Контекст: Полный аудит кодовой базы после исправления предыдущего техдолга. Часть проблем исправлена, но обнаружены новые.

Суть

Аудит выявил 10 новых проблем (1 security, 3 reliability, 6 code quality) в дополнение к уже известным из [[architecture-overview]].

Найденные проблемы

🔴 Security

  1. Эскалация привилегий через AdminUpdateUserhandlers/users.go:166-186
    • Модератор может повысить роль любого пользователя до superadmin (кроме своей). Ограничение только на собственную роль.
    • Надо добавить проверку: модератор не может устанавливать роль superadmin.

🟡 Reliability

  1. Отсутствует статус по умолчанию при создании пользователяrepository/users.go:22-29

    • INSERT не задаёт status, в БД нет DEFAULT. У нового пользователя Status = "".
    • Миграция должна задавать status DEFAULT 'active', либо код должен явно проставлять.
  2. BookingRepo.Create не проверяет статус местаrepository/bookings.go:25-63

    • Можно забронировать место в статусе pending_moderation, draft или rejected. Проверка только на deleted_at IS NULL.
    • Нужна проверка status = 'published'.
  3. RoleMiddleware type assertion без проверкиmiddleware/auth.go:54

    • r.Context().Value(UserRoleKey).(string) запаникует, если в контексте не строка.
    • В отличие от безопасного GetUserRole(), эта функция делает unsafe type assertion.

🔵 Code Quality

  1. N+1 в GetByID для serviceshandlers/services.go:80-85

    • Сначала GetByID, потом отдельно GetTags. Можно объединить в один JOIN-запрос.
    • Аналогично было с places, но там уже исправлено.
  2. DeleteTag: странный парсинг тела DELETEhandlers/tags.go:79-84

    • Сначала JSON body, при ошибке — query param. Это не RESTful и путает.
    • DELETE с body — редкая практика, лучше только query.
  3. broadcastVisitors двойной lock без необходимостиhandlers/websocket.go:91-127

    • Сначала под mutex собирает список, отпускает, потом снова берёт для рассылки.
    • Между сбором и рассылкой список клиентов может измениться.
  4. logout на фронте подавляет ошибкиhooks/useAuth.tsx:65

    • catch {} скрывает ошибки сети при logout.
    • Пользователь не видит, что выход не удался.
  5. fetchPlaces подавляет ошибкуcomponents/MapView.tsx:37-39

    • Ошибка загрузки мест игнорируется, пользователь не видит уведомления.
  6. next.config.js: только cdn.photoplaces.ru и localhostnext.config.js:5-8

    • В production на тестовом сервере S3/MinIO раздаётся через api.{DOMAIN}/s3/, что не добавлено в remotePatterns.
    • Изображения могут не загружаться через Next.js Image Optimization.

Что было проверено и признано корректным

  • Валидация на всех эндпоинтах (go-playground/validator)
  • Безопасность паролей (bcrypt)
  • Защита от SQL-инъекций (pgx параметризованные запросы)
  • Refresh token rotation + SHA-256 хранение
  • Rate limiter с fail-open/closed поддержкой
  • Проверка сигнатуры файлов при загрузке (magic bytes)
  • CORS настроен
  • Мягкое удаление (soft delete)
  • Graceful shutdown
  • Консистентность координат (lat, lng)

Рекомендации

  1. Срочно (P0): Добавить проверку в AdminUpdateUser — модератор не может ставить роль superadmin
  2. Высоко (P1): Добавить DEFAULT 'active' для status в users (миграция 000012)
  3. Высоко (P1): Добавить проверку status = 'published' при создании брони
  4. Средне (P2): Исправить RoleMiddleware, N+1 в services, broadcastVisitors lock, next.config.js

Связанные заметки

  • [[architecture-overview]]
  • [[atomic-toctou-race-registration]]
  • [[atomic-mutex-defer-panic]]
  • [[atomic-error-swallowing-frontend]]

Теги

#code-review #security #backend #frontend #bug #2026-06