Nail/SQS-service
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
ac06572e255c8bffb162038b45bacdd271552f80
SQS-service/doc/thinking/opus-report-2026-10-05.md
T

16 KiB
Raw Blame History

Отчёт Opus — КРАКО и план доработок shared-sqs (2026-10-05)

Полный текст ответа Opus (без сокращений) на промпт opus-review-prompt-2026-10-05.md. Сохранён для истории и критического анализа.

1. КРАКО — критический разбор архитектуры

Что сделано хорошо

  • Изоляция тенантов проста и надёжна в ядре. Ключ очереди {AccessKey}:{queueName} (tenant_helpers.go) делает кросс-тенантный доступ через SQS физически невозможным: тенант берётся из контекста, ключ строится из его AccessKey. Простое и верное решение.
  • SigV4 реально проверяется (вопреки формулировке «подпись не проверяется» — это про JWT, не про SQS). В auth_middleware.go полноценный canonical request, subtle.ConstantTimeCompare, проверка clock skew, поддержка presigned. Это сильная сторона.
  • Аккуратно закрыты классические гонки. Атомарный receive под одним Lock (устранён TOCTOU «потери» сообщений, receive_message.go), double-check под Lock в send_message.go, пер-очередной Lock в PeriodicTasks вместо глобального на весь обход (gosqs.go).
  • OOM-защита по нескольким фронтам: лимит тела 3 MB, MaxHeaderBytes, пер-очередной лимит сообщений, глобальный лимит тенантов.
  • Продуманный graceful shutdown и HTTP-таймауты с обоснованием под long-poll (goaws.go).
  • IDOR-защита в UI и урезанные права UI-тенанта (не может создавать/удалять тенантов, видит только себя) — admin.go.
  • Персистентность с write-sequence guard (last-write-wins по ключу) и синхронным удалением, чтобы удалённые очереди/сообщения не «воскресали» из Redis.

Главные слабости и риски (по коду)

🔴 Критичные (безопасность)

  1. SecretKey детерминированно выводится из email + захардкоженная соль в исходнике. tenant_store.go: secretKey = SHA256(lower(email) + ":" + "shared-sqs:secret-key:v1"), accessKey = SHA256(email)[:12]. Email — не секрет. Кто знает email уже залогинившегося пользователя (тенант существует) и соль (она в репозитории форка), тот вычисляет AccessKey+SecretKey и получает полный доступ к чужим очередям напрямую через SQS-эндпоинт (JWT там не нужен — только SigV4). Это нарушает базовый принцип: секрет обязан быть случайным, а не функцией публичного идентификатора.

  2. Идентичность тенанта берётся из НЕподписанного JWT (claim email), а из email выводятся креды. jwt.go подпись не проверяет; доверие целиком на PingNubesAPI, который принимает токен, если хоть один стенд вернул не 401/403 (jwt.go). Если шлюз при GET /svc не связывает жёстко токен ↔ email, появляется вектор подмены email → провиженинг/доступ под чужой личностью. Плюс кэш валидации даёт отозванному токену жить до 5–7 мин (cache.go).

🟠 Высокие (production-readiness)

  1. Один глобальный мьютекс SyncQueues на весь сервис. Любой send/receive/tick берёт единственный Lock (globals.go). receiveMessagesUnderLock держит глобальный Lock на время O(N)-скана сообщений, а long-poll переобходит его каждые 100 мс. Это главный архитектурный боттлнек — ровно он давал всплески latency, упомянутые в комментариях. Не масштабируется по числу очередей/тенантов.

  2. Невозможна горизонтальная масштабируемость → один под = SPOF. Память — источник правды, Redis — write-through-кэш одной реплики. Две реплики разъедутся (каждая со своей map, last-write-wins затирает). Значит сервис принципиально single-instance: нет HA, рестарт = простой + окно потери данных.

  3. Асинхронная запись в Redis = окно потери подтверждённых сообщений. SendMessage отвечает клиенту 200, а HSet уходит в горутину позже (redis.go). Краш пода между append-в-память и async-записью → подтверждённое сообщение потеряно. Для очереди это нарушение durability (AWS SQS персистит до ack).

  4. MessageRetentionPeriod не реализован (// TODO - not used, models.go). Сообщения не истекают по времени, растут до пер-очередного лимита, после чего новые SendMessage отклоняются OverLimit. Нет TTL-эвикции — скрытая деградация.

  5. Сообщения хранятся плоским слайсом []SqsMessage. Receive/Delete/visibility-tick — O(N) на операцию; при лимите 120k сообщений и глобальном локе это дорого и усугубляет п.3. Нет индексов по ReceiptHandle/состоянию.

🟡 Средние

  1. Пересчёт legacy-тенанта в CreateFromJWT сиротит очереди (tenant_store.go): очереди остаются под старым AccessKey, новый тенант видит 0 очередей, данные зависают в памяти+Redis навсегда (утечка + «пропажа» данных для пользователя).

  2. Повсеместные O(total) скан-по-префиксу под глобальным локом: countTenantQueues, listTenantQueues, createTenantQueue (лимит), deleteTenant, gauge-updater. Квадратичная тенденция при росте тенант×очередь.

  3. Admin-токен сравнивается небезопасно по времени — authHeader != expected (admin.go), в отличие от UI-demo-токена (там ConstantTimeCompare). Timing-side-channel.

  4. Хардкод demo-кредов и публичного UI-bypass-токена в исходнике (seed.go, admin.go). Сами же комментарии требуют удалить это перед продом. Включается env-флагами, но риск реальный.

  5. Два расходящихся пути записи/создания. admin.createTenantQueue/sendMessageToQueue (admin.go) создают очередь и кладут сообщение в обход gosqs-логики: без FIFO-детекта, без проверки размера, без MD5/дедупликации, с другими дефолтами. Поведение UI ≠ поведение SQS API.

  6. Биллинг: горутина на каждую операцию + пул 5 коннектов (billing.go). Под нагрузкой — навал горутин, ждущих коннект. Плюс msg_count всегда 1, даже для SendMessageBatch → неточный учёт.

  7. /metrics публичен и отдаёт per-tenant данные (ID тенантов, число очередей/сообщений) — утечка бизнес-метрик наружу.

  8. Хрупкое определение протокола — строго по Content-Type == application/x-amz-json-1.0 (router.go); иные версии JSON-протокола не распознаются.

  9. Легаси-остатки GoAWS/SNS: Topics/Subscriptions в goaws.yaml, SNS-поля в моделях, config.go кладёт yaml-очереди под голым именем queue.Name (без tenant-префикса → такие очереди недостижимы tenant-хендлерами; сейчас спасает Queues: []).

2. Приоритизированный план доработок

P0 — блокеры продакшна (безопасность и durability)

  • P0.1. Убрать вывод SecretKey из email. Файлы: tenant_store.go (secretKeyFromEmail/accessKeyFromEmail). Генерировать AccessKey/SecretKey через crypto/rand при первом провиженинге и хранить в Redis; идентичность по email держать отдельно от материала ключей. Детерминированным оставить максимум tenantID (не секрет).
  • P0.2. Усилить доверие к JWT. Файлы: jwt.go. Либо проверять подпись по JWKS Nubes, либо брать email/ sub из ответа валидирующего API (а не из незаверенного payload), чтобы claim нельзя было подменить. Сузить «принять токен, если любой стенд не вернул 401/403» до явного 2xx от доверенного эндпоинта.
  • P0.3. Durability для SendMessage. Файлы: redis.go, send_message.go. Сделать персист сообщения синхронным до ответа 200 (как уже сделано для delete), либо явно задокументировать «best-effort» семантику и снять претензию на SQS-совместимость по надёжности.

P1 — архитектурная зрелость

  • P1.1. Разнести блокировку. Файлы: globals.go, все gosqs-хендлеры. Перейти на per-queue mutex (map под коротким RLock + своя блокировка в Queue). Снимает главный боттлнек и O(N)-скан под глобальным локом.
  • P1.2. Индексировать сообщения. Файлы: models.go. Хранить map[receiptHandle] / структуры готовности, чтобы receive/delete/visibility были не O(N).
  • P1.3. Реализовать MessageRetentionPeriod. Файлы: gosqs.go (processQueueTick). Эвиктить просроченные сообщения по retention в периодической задаче + чистить Redis.
  • P1.4. Индекс тенанта → его очереди. Файлы: tenant_helpers.go, admin.go. Держать map[accessKey]set(queueName) — убрать O(total) скан-по-префиксу в create/list/delete/gauge.
  • P1.5. Убрать demo-bypass из прод-сборки. Файлы: seed.go, admin.go. Вынести за build-tag или удалить ветку authenticateUIDemoToken и фиксированные креды.

P2 — надёжность, корректность, чистка

  • P2.1. BYOC Admin API (уже TODO в byoc-credentials.md). Добавить access_key/secret_key в createTenantRequest + ветку на CreateFixed в admin.go. generateTenantID экспортировать из tenant-пакета.
  • P2.2. Чинить сиротство очередей при пересчёте legacy-тенанта — tenant_store.go: либо мигрировать ключи очередей на новый AccessKey, либо запретить смену ключей.
  • P2.3. Constant-time сравнение admin-токена — admin.go.
  • P2.4. Единый путь создания/отправки — admin-хендлеры должны переиспользовать gosqs-логику (FIFO/размер/MD5/дедуп), а не дублировать с иными дефолтами.
  • P2.5. Биллинг: батч-вставки через воркер/буфер вместо горутины-на-операцию; корректный msg_count для batch (billing.go, router.go actionHandler).
  • P2.6. Защитить/урезать /metrics (auth или internal-only), чтобы не светить per-tenant данные.
  • P2.7. Чистка легаси: убрать SNS Topics/Subscriptions из goaws.yaml, мёртвый путь загрузки yaml-очередей без tenant-префикса в config.go, SNS-поля в моделях, deployments/**.
  • P2.8. Redis reconnect/health — сейчас после обрыва Client остаётся не-nil, записи молча падают в лог (redis.go).

3. Противоречия док ↔ код

  1. Идентичность JWT-тенанта. tenant-isolation.md утверждает: тенант создаётся из claim sub, TenantID = SHA256(sub) → "sless-{16hex}", индекс bySub/GetBySub. Код: идентичность по email, TenantID = t-{16hex} из email, индекс byEmail/GetByEmail (tenant_store.go). Доковое описание полностью устарело.
  2. Генерация SecretKey. Док: «SecretKey = 32 random bytes». Код: для email-тенантов SecretKey детерминированный SHA256(email+соль) — не случайный.
  3. Версии (дрейф). README deploy-примеры: v0.1.25; раздел «История» в README — до v0.1.14; HANDOFF — прод v0.1.35. Три разных значения.
  4. NUBES_ENDPOINT. README: дефолт https://deck-api.ngcloud.ru/api/v1. Код: стенды https://lk-api-gateway*.ngcloud.ru/api/v1/svc (jwt.go). Разные хосты/пути.
  5. BYOC статус. byoc-credentials.md честно помечает Admin API/UI как TODO — совпадает с кодом (в createTenantRequest только Name, MaxQueues). Это не противоречие, а подтверждение открытого вопроса №1.
  6. Метрики. billing-and-metrics.md: пример http://localhost:9090/metrics, тогда как сервис слушает 4100 и отдаёт /metrics на основном порту (router.go). Также ссылка на deployments/k8s/vmservicescrape.yaml, помеченный в README как легаси «не использовать».
  7. Дерево в README перечисляет auth/ — AWS Signature V4 верификация, но не упоминает JWT-путь, billing/, metrics/. Неполно относительно фактической структуры.
  8. /metrics → PUBLIC в tenant-isolation.md соответствует коду, но сам факт публичности per-tenant метрик в доке не отмечен как риск.

Заключительная реплика Opus: «Если нужно — могу превратить это в файл-отчёт в doc/thinking/ или углубиться в любой из пунктов (например, расписать PoC-эксплуатацию P0.1/P0.2 или конкретный дизайн per-queue-локов). Код я не трогал, как и требовалось.»

Reference in New Issue View Git Blame Copy Permalink