doc(sqs): промпт для Opus (allowlist файлов) + его отчёт КРАКО и план

This commit is contained in:
Repinoid
2026-10-05 21:38:03 +03:00
parent 60b15fdb1c
commit e8ccd8bae1
2 changed files with 226 additions and 0 deletions
+110
View File
@@ -0,0 +1,110 @@
# Отчёт 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)
3. **Один глобальный мьютекс `SyncQueues` на весь сервис.** Любой send/receive/tick берёт единственный `Lock` (`globals.go`). `receiveMessagesUnderLock` держит глобальный Lock на время O(N)-скана сообщений, а long-poll переобходит его каждые 100 мс. Это главный архитектурный боттлнек — ровно он давал всплески latency, упомянутые в комментариях. Не масштабируется по числу очередей/тенантов.
4. **Невозможна горизонтальная масштабируемость → один под = SPOF.** Память — источник правды, Redis — write-through-кэш одной реплики. Две реплики разъедутся (каждая со своей `map`, last-write-wins затирает). Значит сервис принципиально single-instance: нет HA, рестарт = простой + окно потери данных.
5. **Асинхронная запись в Redis = окно потери подтверждённых сообщений.** `SendMessage` отвечает клиенту 200, а `HSet` уходит в горутину позже (`redis.go`). Краш пода между append-в-память и async-записью → подтверждённое сообщение потеряно. Для очереди это нарушение durability (AWS SQS персистит до ack).
6. **`MessageRetentionPeriod` не реализован** (`// TODO - not used`, `models.go`). Сообщения не истекают по времени, растут до пер-очередного лимита, после чего новые `SendMessage` отклоняются `OverLimit`. Нет TTL-эвикции — скрытая деградация.
7. **Сообщения хранятся плоским слайсом `[]SqsMessage`.** Receive/Delete/visibility-tick — O(N) на операцию; при лимите 120k сообщений и глобальном локе это дорого и усугубляет п.3. Нет индексов по ReceiptHandle/состоянию.
### 🟡 Средние
8. **Пересчёт legacy-тенанта в `CreateFromJWT` сиротит очереди** (`tenant_store.go`): очереди остаются под старым AccessKey, новый тенант видит 0 очередей, данные зависают в памяти+Redis навсегда (утечка + «пропажа» данных для пользователя).
9. **Повсеместные O(total) скан-по-префиксу** под глобальным локом: `countTenantQueues`, `listTenantQueues`, `createTenantQueue` (лимит), `deleteTenant`, gauge-updater. Квадратичная тенденция при росте тенант×очередь.
10. **Admin-токен сравнивается небезопасно по времени** — `authHeader != expected` (`admin.go`), в отличие от UI-demo-токена (там `ConstantTimeCompare`). Timing-side-channel.
11. **Хардкод demo-кредов и публичного UI-bypass-токена в исходнике** (`seed.go`, `admin.go`). Сами же комментарии требуют удалить это перед продом. Включается env-флагами, но риск реальный.
12. **Два расходящихся пути записи/создания.** `admin.createTenantQueue`/`sendMessageToQueue` (`admin.go`) создают очередь и кладут сообщение в обход gosqs-логики: без FIFO-детекта, без проверки размера, без MD5/дедупликации, с другими дефолтами. Поведение UI ≠ поведение SQS API.
13. **Биллинг: горутина на каждую операцию + пул 5 коннектов** (`billing.go`). Под нагрузкой — навал горутин, ждущих коннект. Плюс `msg_count` всегда 1, даже для `SendMessageBatch` → неточный учёт.
14. **`/metrics` публичен** и отдаёт per-tenant данные (ID тенантов, число очередей/сообщений) — утечка бизнес-метрик наружу.
15. **Хрупкое определение протокола** — строго по `Content-Type == application/x-amz-json-1.0` (`router.go`); иные версии JSON-протокола не распознаются.
16. **Легаси-остатки 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-локов). Код я не
> трогал, как и требовалось.»
@@ -0,0 +1,116 @@
# Промпт для Opus — анализ архитектуры shared-sqs (2026-10-05)
> Назначение: дать модели Opus контекст и точный список файлов для чтения,
> чтобы она проанализировала архитектуру shared-sqs и выдала идеи
> архитектуры (в т.ч. «КРАКО» — критический разбор архитектуры) без чтения
> лишних файлов.
---
## ЗАДАЧА
Проанализируй архитектуру сервиса `shared-sqs` (SQS-совместимая очередь, Go,
мультитенантная, форк GoAWS) и выдай:
1. **Критический разбор архитектуры (КРАКО)** — слабые и сильные места,
риски, узкие места, что мешает назвать сервис готовым к продакшну.
2. **Идеи по архитектуре** — как довести сервис до production-ready:
приоритизированный список доработок (P0/P1/P2) с обоснованием.
Не меняй код. Только читай и анализируй.
## ЧТО ЧИТАТЬ (allowlist — ТОЛЬКО эти файлы, ничего больше)
**Точка входа и роутинг:**
- `app/cmd/goaws.go`
- `app/cmd/seed.go`
- `app/router/router.go`
**Конфигурация:**
- `app/conf/config.go`
- `app/conf/goaws.yaml`
**SQS-реализация (ядро):**
- `app/gosqs/gosqs.go`
- `app/gosqs/receive_message.go`
- `app/gosqs/send_message.go`
- `app/gosqs/create_queue.go`
- `app/gosqs/validation.go`
- `app/gosqs/tenant_helpers.go`
**Аутентификация / изоляция тенантов:**
- `app/auth/auth_middleware.go`
- `app/auth/jwt.go`
- `app/auth/cache.go`
- `app/tenant/tenant_store.go`
**Модели данных:**
- `app/models/models.go`
- `app/models/constants.go`
- `app/models/globals.go`
**Персистентность:**
- `app/persistence/redis.go`
**Биллинг и метрики:**
- `app/billing/billing.go`
- `app/metrics/metrics.go`
**Admin API:**
- `app/admin/admin.go`
**Интерфейсы:**
- `app/interfaces/interfaces.go`
**Документация (для контекста состояния):**
- `README.md`
- `doc/architecture/tenant-isolation.md`
- `doc/billing-and-metrics.md`
- `doc/byoc-credentials.md`
- `HANDOFF-NEW-CHAT.md`
## ⛔ ГДЕ НЕ ЛАЗИТЬ
- **Не читать** `doc/legacy/**` (исторический план, старые решения — устарели).
- **Не читать** `deployments/**` (легаси k8s/helm).
- **Не читать** `doc/thinking/nubes-*` (сетевые тикеты платформы — отдельная
задача, не относится к разбору архитектуры кода).
- **Не читать** тесты `*_test.go` — они нужны только как сигнал «что покрыто»;
для анализа архитектуры достаточно факта наличия из списка `app/gosqs/*`.
- Не искать файлы за пределами перечисленного списка.
## КЛЮЧЕВОЙ КОНТЕКСТ (факты, которые уже установлены — не перепроверять с нуля)
- **Стек**: Go 1.23, gorilla/mux v1.8.0, redis/go-redis/v9, lib/pq,
prometheus/client_golang. Сервис — один Go-бинарник.
- **Состояние**: прод v0.1.35, все тесты зелёные, нагрузка 168 591 операция
без сервисных ошибок. НО есть известные открытые вопросы (ниже).
- **Персистентность**: Redis write-through cache. Память — источник правды
для чтения; Redis — для восстановления после рестарта. Записи в Redis
асинхронные (кроме DeleteMessagePersist — синхронно).
- **Изоляция тенантов**: очередь хранится по ключу `{AccessKey}:{queueName}`;
`TenantStore` с тремя индексами (`byID`, `byAccessKey`, `byEmail`).
- **JWT-валидация**: подпись НЕ проверяется (trusted perimeter) — только
структура + `exp` + `PingNubesAPI()` (проверка token через API Gateway).
- **Биллинг**: асинхронная запись в PostgreSQL (таблица `sqs_usage_records`),
опционально (включается при `BILLING_PG_HOST`).
- **Внешняя проблема (НЕ код, на анализ кода не влияет)**: периодические
сетевые таймауты ~31–33с на внешнем шлюзе Nubes + MSS-баг для больших тел.
Разобрано в тикете — это задача платформы, не кода.
## ИЗВЕСТНЫЕ ОТКРЫТЫЕ ВОПРОСЫ (кандидаты в «доделать» — проверь их актуальность по коду)
1. **BYOC-credentials** — backend `CreateFixed` есть, но Admin API/UI
(`POST /admin/tenants` с произвольными ключами) помечен TODO.
2. **Подпись JWT не проверяется** — насколько это приемлемо для multi-tenant
прод-сервиса, или нужно усилить.
3. **Дрейф доков** — версии в README/Makefile/HANDOFF не всегда совпадают.
4. **Легаси** — невычищенные следы старого k8s/helm-деплоя и SNS.
## ФОРМАТ ОТВЕТА
1. **КРАКО** — короткий критический разбор: что хорошо, что плохо, главные
риски (по коду, а не по платформе).
2. **Приоритизированный план доработок** (P0/P1/P2), с указанием конкретных
файлов и сути правки.
3. Список **противоречий/несоответствий** между доками и кодом, если найдёшь.