SQS-service/HISTORY/2026-10-11-session-log.md

История правок shared-sqs — 2026-10-11 (внешнее код-ревью и фиксы)

Запись по требованию «документировать всё, включая собственные ошибки» (приказ 2026-09-24). Предыдущие: HISTORY/2026-10-10-session-log.md, doc/thinking/*-2026-10-08.md.

1. Задача

Провести внешнее код-ревью двух сервисов (SQS-service + IoT) моделью Claude Sonnet (в режиме Plan — правки не вносит, только находит), затем своими силами исправить найденное. Промпт для ревью — CLAUDE-REVIEW-PROMPT.md в корне монорепо (со списком файлов, фокус-точками и строгим форматом ответа; явно запрещены правки, оценка тестов/UI/legacy/стиля).

Ревью дало 40 пунктов (10 🔴 / 29 🟡 / 1 🟢). Разбор вопросов ревьюера и уточнение некорректных выводов — в п.3.

2. Что исправлено (SQS-service)

🔴 Критичные (10/10)

Пункт	Файл	Суть	Коммит
🔴1	app/admin/admin.go	jwtMiddleware пропускал любой путь с суффиксом /auth (обход JWT + IDOR на DELETE .../queues/auth). Теперь — точное сравнение /ui/api/auth	1241093
🔴2	app/gosqs/queue_attributes.go	Запрет self-/циклического RedrivePolicy: перенос в DLQ берёт второй mu под первым → deadlock	9b9bbe1
🔴3	app/gosqs/gosqs.go, models.go, change_message_visibility*.go	Перенос в DLQ не персистился (сообщение «воскрешало» в исходной и пропадало из DLQ)	1e334c2
🔴4	app/persistence/redis.go, app/tenant/tenant_store.go	DeleteTenant не чистил индекс ssq:tenant_ak — отозванный тенант возвращался через read-through	f68b0a4
🔴5	app/persistence/redis.go, app/cmd/goaws.go	Глобальный Client перезаписывался в reconnect() без синхронизации (data race). → atomic.Pointer	d03ce36
🔴6	app/conf/config.go	return ports без SyncQueues.Unlock() при ошибке setQueueRedrivePolicy → вечный deadlock	1520753
🔴7	app/billing/billing.go	Close() писал db=nil параллельно с usageWorker (nil-паника). → мьютекс + остановка воркера до db.Close	4e36ba0
🔴10	app/auth/auth_middleware.go	X-Amz-Content-Sha256 не сверялся с телом — подмена тела под подписанным заголовком	2918a41

(🔴8/🔴9 — на стороне IoT, см. IoT/HISTORY/2026-10-11-session-log.md.)

🟡 Важные

Пункт	Файл	Суть	Коммит
🟡20	app/admin/admin.go	createTenantQueue не валидировал имя очереди (:,/ ломали ключ)	efd8c8e
🟡23	app/auth/jwt.go	Токен без exp принимался бессрочно → требуем exp	efd8c8e
🟡24	app/leader/leader.go	При сбое RefreshLock инстанс оставался лидером (split-brain) → сброс лидерства после TTL	efd8c8e
🟡21	app/tenant/tenant_store.go	Запрет : в BYOC access_key (коллизии ключа {ak}:{name})	dc4815d
🟡22	app/admin/admin.go	sendMessageToQueue без лимитов + async persist → лимит MaxMessagesForQueue + SaveMessageSync	dc4815d
🟡33	app/models/models.go	InitDuplicatation падал на nil-map Duplicates → ленивая инициализация	dc4815d
🟡35	app/models/models.go	Reset() не чистил FIFOMessages (после purge группы не выдавались)	dc4815d
🟡36	app/gosqs/receive_message.go	ApproximateReceiveCount был завышен на 1	dc4815d
🟡38	app/cmd/goaws.go	billing.Close() вызывался ДО Shutdown серверов → перенесён после	dc4815d
🟡26	app/gosqs/delete_message_batch.go	Ключ map по ReceiptHandle (два entry с одним handle теряли Id) → по Id	e0b5f24
🟡27	app/gosqs/send_message.go	DelaySeconds не валидировался → 0–900	e0b5f24
🟡25	app/gosqs/send_message_batch.go	Dedup регистрировался после цикла (два entry с одним id оба добавлялись); NextSequenceNumber до проверки дубликата; per-entry DelaySeconds не применялся	6f69755
🟡29	app/gosqs/delete_message*.go, persistence/redis.go	Ошибка HDel только логировалась (клиент получал 200, после рестарта сообщение воскрешало) → DeleteMessage(s)Persist возвращает ошибку, 500 клиенту	5d38118
🟡31	app/persistence/redis.go, app/cmd/goaws.go	Ошибка загрузки сообщений очереди давала continue (очередь стартовала пустой) → fail-fast (log.Fatalf)	5d38118
🟡30	app/persistence/redis.go	Async SaveMessage без порядковой нумерации (старое состояние могло перезаписать новое) → seq по `{queueKey}	{uuid}` + очистка при удалении
🟡32	app/gosqs/gosqs.go	PeriodicTasks без recover (паника убивала процесс)	dc4815d
🟢	app/utils/utils.go	SqsErrors[errKey] без проверки → nil-deref на неизвестном ключе	5d38118
(🟡 new)	app/gosqs/create_queue.go	Очередь из API получала VisibilityTimeout=0 и пр. (дефолты не подставлялись)	5d38118

Осознанно НЕ менялось — РЕШЕНИЯ (2026-10-11)

• 🟡34 (атрибуты «0 = не передано»): решение — не менять совместимость. Различать «не передано» и явный 0 потребовало бы перевести QueueAttributes на указатели — ломает JSON-протокол и Terraform-drift. Явный 0 — редкий сценарий.
• 🟡37 (дефолт SHARED_SQS_MAX_HEAP_MB): решение — оставить выключенным (0). Безопасное число зависит от RAM конкретной ВМ и числа тенантов — выбор владельца, не хардкод в коде.
• 🟡28 (сеть под queue.mu): решение — оставить как есть (вариант A). См. п.4.

3. Уточнение выводов ревьюера

Ревьюер задал вопросы по коду, ответы найдены в коде (файл:строка):

• ACL EMQX — единственный барьер для топиков {ns}/telemetry/{deviceId}? Да: bridge/handler.go берёт ns/deviceID из топика без сверки принадлежности, барьер — EMQX authz (emqx/emqx.conf). Добавлена defense-in-depth проверка формата (IoT).
• Self-/циклический RedrivePolicy реально задаётся через API? Да (setQueueAttributesV1 не проверял) — отсюда фикс 🔴2.
• Пустой adminToken? Недопустим — goaws.go:~74 log.Fatal при отсутствии.
• Дефолты очереди расходятся (create_queue.go vs conf/config.go:100-109)? Да — отсюда фикс (🟡 new).
• utils.go паника на неизвестном ключе? Текущей паники нет (все ключи есть в models.SqsErrors), но добавлена защита.

Также ревьюер отметил некорректный пункт (удалён из списка по лимиту 40): nil-map Duplicates (🟡33) фактически не воспроизводился в текущем коде — но защита добавлена как дешёвая страховка.

4. 🟡28 — РЕШЕНИЕ (2026-10-11)

• Проблема: синхронный SaveMessageSync (до 3 с) выполняется под queue.mu (send_message.go:150, delete_message.go, admin.go). Медленный Redis блокирует всю очередь.
• Варианты: A) оставить; B) укоротить таймаут Redis-записи; C) полный рефакторинг с per-message синхронизацией.
• Выбрано A — оставить. Обоснование:
  1. in-memory ядро очередей — одиночка, блокировка queue.mu локальна (одна очередь).
  2. Синхронный персист под mu = гарантия «нет потери подтверждённого сообщения при крахе пода» — приоритет проекта (надёжность, а не масштабирование).
  3. Вынос сети за mu вносит окно между Unlock/Lock: два конкурентных SendMessage с одним deduplicationId оба пройдут IsDuplicate (дубль), либо при откате dedup теряется сообщение — хуже кратковременной блокировки.
  4. Redis-авария — глобальная: деградирует всё, 2–3 с на очередь не критично.
• 🟡34/🟡37 — см. п.2.

5. Верификация

• go build ./..., go vet ./..., go test -race ./... — зелёные на всех коммитах.
• Push: 2c6d2d7..f80e1b4 в origin/master.

6. Деплой на прод (ВМ 213) и smoke

• make build → scp shared-sqs → /home/naeel/sqs-test/shared-sqs.new → systemctl stop shared-sqs; mv .new → shared-sqs; start. Бэкап — shared-sqs.prev.
• Проверка: systemctl is-active = active; /health → 200 v0.1.35; в логе «загружено 5 очередей из Redis» (fail-fast-загрузка сработала, не упала).
• Smoke по https://sqs.sinefo.ru (tests/api_test.sh, aws cli, тенант demo-service): PASS=22 FAIL=0 (17 базовых операций + FIFO CreateQueue/Send/Receive/Delete/DeleteQueue). Вывод: 18 фиксов ревью не сломали AWS-совместимость на проде.

7. Образы → Gitea registry

• Реестр — Gitea gitea.services.ngcloud.ru (НЕ Docker Hub). Makefile/Dockerfile/доки исправлены.
• Залито: nail/shared-sqs:v0.1.35 (+latest; было только v0.1.25), nail/iot-service:v0.1.13, nail/iot-emqx:v0.2.4.
• Push: e30bf74..f80e1b4.