Files
transcriber/openspec/changes/archive/2026-08-23-storage-without-pocketbase/review/triage-2026-08-23.md
T
av c9b7765646 хранилище переехало с PocketBase на SQLite со своим каталогом файлов
- база своя: два пула, захват одним UPDATE ... RETURNING, шаги схемы на goose
  под файловым замком, одна миграция начальной схемы вместо семи прежних
- транспорт переписан на net/http: свои слои, свой ограничитель частоты,
  отдача файла с проверкой владельца; панель /_/ и пространство /api/ исчезли
- по находкам ревью: журнал не пишет путь под корнем приложения, ключ бюджета
  читается справа налево, узнавание известного идёт читающим пулом
2026-08-23 08:06:04 +03:00

94 lines
25 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Отчёт триажа ревью кода — storage-without-pocketbase
Дата: 2026-08-23
Режим: прогон по графу, база диффа HEAD, работа незакоммичена. Метка large — задана владельцем прогона, оси не мерились. Гейт зелёный целиком. Сигнал о метке от review-code: large подтверждена. Находок на входе 27 именованных плюс ~20 подпороговых; осталось 7 в основных секциях, 2 понижены в гипотезы, 8 в урожай, 2 в promote.
План и исход по темам: requirements (дельта-спеки + openspec/specs, разбор, specs) — закрыта, 6 находок; autotests (CLAUDE.md «Гейт», autotests) — закрыта, 4 находки; conventions (docs/conventions/, разбор, code) — закрыта, 6 находок, потолок 4/4 сработал; architecture (docs/architecture.md + passport.md, доказательство, architecture) — закрыта, 3 находки, потолок 3 сработал; security (docs/security.md, доказательство, adversary) — закрыта, 5 находок, пути построены и прогнаны; operations (docs/architecture.md «Эксплуатация» + docs/database.md, доказательство, ops) — закрыта, 3 находки с замерами. Тем без отчёта нет. basics не запускался: своих тем проекта нет, темы ядра разобраны именными проходами.
## 1. Блокирует мердж
### Аноним пишет свой текст в журнал владельца со скоростью 121 МиБ/с, и ограничитель этому не мешает
Файл: internal/controller/http/journal.go:80-95 (JournalRoute), :55-75. Severity: critical. Confidence: high.
Оракул: TestAdversary_AnonymousWritesIntoJournalUnderAppRoot — строка журнала несёт путь дословно при ответе 401. TestAdversary_JournalLineGrowsWithRequestedPath: путь 1 044 480 знаков → прирост журнала 1 044 632 байта. TestAdversary_JournalThroughput: одно соединение, 1.003 с → 122 запроса, 121.5 МиБ журнала (121.2 МиБ/с). TestAdversary_RefusedRequestStillWritesJournal: 120 отказов ограничителя → 240 строк журнала, 1 019 617 байт: слой журнала стоит снаружи ограничителя. Конвенция docs/conventions/logging.md:113 запрещает это дословно.
Последствие: неузнанный снаружи наполняет журнал контейнера своим текстом с произвольной скоростью. Диск сервера общий с data/ — под ним и база, и записи живых людей; исчерпание места приходит на укладку записи, где причина отказа к тому же теряется. Собранные логи чужой текст уносят навсегда.
Предложение: JournalRoute возвращает webappRoute для всего, что накрыто корнем приложения, а не только для чужих путей; длину отдавать полем http.path_length. Точные адреса (/health, /metrics) пишутся дословно — они из закрытого перечня.
Найдено проходом: adversary (V2), code/конвенции (C3). Действие: инлайн.
### Ограничитель частоты ключуется значением, которое пишет сам спрашивающий: бюджет обходится с первого запроса
Файл: internal/controller/http/rate_limit.go:86-101 (clientAddress). Severity: critical. Confidence: high.
Оракул: TestAdversary_RateLimitBypassedByForwardedFor — ограничитель пропустил 1200 запросов одного спрашивающего при бюджете 120. TestAdversary_RateLimiterBudgetsGrowth — 200 000 ключей в карте, прирост кучи 19 810 376 байт (99 байт на ключ). Код берёт strings.Cut(r.Header.Get(ForwardedForHeader), ",") — левое значение цепочки, а Caddy заголовок по умолчанию дописывает, а не заменяет.
Последствие: единственный бюджет на корне приложения не действует ни для одного противника, знающего про заголовок. Ключ карты выбирает он же, поэтому карта растёт линейно от числа выдуманных адресов. Складывается с предыдущей находкой: обход бюджета снимает потолок, который мог бы ограничить поток в журнал.
Предложение: сканировать X-Forwarded-For справа налево, отбрасывая доверенные адреса, и брать первый недоверенный; читать r.Header.Values, а не Get. Отдельно — предел числу ключей карты.
Найдено проходом: adversary (V1), specs (S5). Действие: инлайн.
### Инвариант про колонки записи потерял предмет: он называет функции, которых в коде больше нет, а третье место чтения не механизировано ничем
Файл: CLAUDE.md:105, internal/adapter/repo/sqlite/record_mapping.go:111-173, internal/archrules/arch_test.go:333-361. Severity: major. Confidence: high.
Оракул: grep по applyOwnedByPipeline/applyToRecord/recordToAudioRecord — пусто, при том что инвариант называет ровно эти три имени. grep rowToAudioRecord в internal/archrules — пусто: правило сверяет writeOwnedByPipeline+writeRecord против readRecordColumns, а функция, заполняющая сущность, в предмет правила не входит.
Последствие: мест стало три (readRecordColumns — что спрошено, recordRow — куда лягут, rowToAudioRecord — что доедет до сущности), механизировано одно. Колонка, забытая в rowToAudioRecord, даёт зелёный гейт: значение не доезжает до сущности, а ближайший Save пишет нулевое поверх сохранённого — тихая порча данных, ровно та, которую инвариант объявляет закрытой. Сам инвариант стал неверифицируемым.
Предложение: обновить текст инварианта на действующие имена и действительное число мест; расширить правило internal/archrules на rowToAudioRecord.
Найдено проходом: architecture (R1). Действие: инлайн.
## 2. Стоит исправить сейчас
### Всякий запрос берёт пишущую транзакцию единственного пишущего соединения, а очередь к нему не ограничена ничем
Файл: internal/adapter/repo/sqlite/identity.go:40-58, db.go:56-64,114-118, internal/controller/http/identity.go:125. Severity: major. Confidence: high.
Оракул: падающий тест триажа — при BusyTimeoutMs 100 и занятом пишущем соединении EnsureUser ждал 606.494 мс и вернул nil: ожидание свободного соединения busy_timeout_ms не ограничено вовсе, границу задаёт только контекст, а контекст — context.Background(). EnsureUser зовётся слоем узнавания, одетым на весь корень приложения, значит BEGIN IMMEDIATE берётся на 100% узнанного потока. Требование storage/spec.md:26-32 составной операцией называет заведение учётной записи первым обращением, а не всякое.
Последствие: зависший (не отказавший) диск останавливает приём, опрос карточек и конвейер разом, без предела ожидания; сторож застревания стоит в той же очереди. Отказа не будет — будет тишина.
Предложение: искать учётную запись читающим пулом и уходить в пишущую транзакцию только когда не нашлось; ветвь повторного поиска после гонки уже написана (identity.go:74-80).
Найдено проходом: ops (O1), specs (S1), architecture (R3). Действие: развилка — вопрос владельцу, три варианта: (а) снять с горячего пути только узнавание; (б) то же плюс предел ожидания на обращениях, рождённых HTTP-запросом; (в) оставить и записать цену нормой в спеку storage.
Оговорка: довод «NoopJobError не считается, поэтому конвейер встал и очередь пуста неотличимы» опирается на то, что считать NoopJobError запрещено инвариантом. Законна только просьба про heartbeat воркера — она в урожае.
### Диагностика укладки настроена ровно наоборот: причина отказа отброшена там, где нужна, и путь внутри каталога данных уехал в журнал там, где не должен
Файл: internal/adapter/repo/sqlite/store.go:64-112 (Put, Open, Remove), internal/service/transcribe.go:222-224. Severity: major. Confidence: high.
Оракул: падающий тест триажа. Отказ записи вернул: failed to store a copy of record 01M0NV5JFP5AYNVKR5FF8FW9HR / write /tmp/.../records/01M0NV5JFP5AYNVKR5FF8FW9HR/.partial-01m0nvcvk21w1qz0qhhy54bry4: file too large — полный путь внутри каталога данных в цепочке, при том что комментарий store.go:80-81 утверждает обратное. Отказ по правам вернул failed to create the directory of record …, и os.IsPermission(err) = false: причина отброшена целиком. Конвенция errors.md:26-34: обёртка %w — умолчание.
Последствие: у владельца единственная поверхность диагностики, и на ней ENOSPC, EACCES и EROFS неразличимы — сервис говорит одно и то же на три поломки, требующие трёх разных действий. Одновременно одна ветвь из семи делает обратное — кладёт полный путь в журнал.
Предложение: во всех семи местах обернуть причину %w, сохранив errors.Is до fs.ErrPermission и syscall.ENOSPC; путь снять — заворачивать не *os.PathError целиком, а его .Err.
Найдено проходом: code (C4 — причина), specs (S3 — путь). Чинить порознь нельзя: вторая правка отменит первую. Действие: инлайн.
### Тип содержимого ответа выбирает отправитель: запись.html отдаётся text/html; charset=utf-8 с inline
Файл: internal/controller/http/file.go:16-33,119-126,171-180. Severity: minor. Confidence: high.
Оракул: TestAdversary_HostileExtension. запись.html → Content-Type text/html; charset=utf-8, Content-Disposition inline; запись.svg → image/svg+xml inline; запись.xhtml → application/xhtml+xml. Перечень contentTypes не знает mkv/mov/avi, которые сервис сам объявляет диалогу выбора файла, и откатывается на mime.TypeByExtension — в alpine нет /etc/mime.types, у разработчика есть: ответ становится функцией машины сборки. Инвариант CLAUDE.md: наружу расширение выходит только приведённым к перечню известных форматов; единая точка metrics.FormatLabel существует, транспорт ходит мимо неё.
Последствие: сегодня цена нулевая — файл видит только владелец. Появляется у первой задачи со вторым читателем, и появляется молча. Плюс уже действующая ошибка: mkv/mov/avi отдаются типом, зависящим от образа.
Предложение: тип содержимого выводить из закрытого перечня той же единой точки, что и метку метрики; всё, чего в перечне нет, — application/octet-stream с attachment; откат на mime.TypeByExtension убрать.
Найдено проходом: code (C1), architecture (R2), adversary (V4). Действие: инлайн. Вторая половина R2 — своя реализация диапазонов против http.ServeContent — в урожай.
### Три решающих ветви отказа не проверены ничем, и одна из них — та, что держит процесс живым
Файл: internal/config/config.go:88-98, internal/controller/http/errors.go:184-190, internal/controller/http/app.go:459-462,490-493. Severity: major. Confidence: high.
Оракул: go tool cover -func = 0.0% на всех трёх местах, grep по тестам пуст. Все три ветви StorageConfig.Validate() не покрыты, config_test.go не упоминает StorageConfig вовсе; Recover не вызывается ни одним тестом; contract.ErrTextNotReady не проверяется во всём пакете, включая отображение в 409.
Последствие: журнал проекта знает три записи класса «проверка не могла упасть» и «тесты обработчика ни разу не были зелёными» (docs/review.md, 2026-08-10, 2026-08-11, 2026-08-15). Здесь тот же класс на новом коде.
Предложение: три теста — таблица на три ветви Validate, обработчик с паникой через полную цепочку слоёв, запрос текста у записи без готового текста с проверкой кода 409 и тела.
Найдено проходом: autotests (A1, A2, A3). Действие: инлайн.
## 3. Гипотезы без доказательства
A4 — класс «репозиторий отказал во время запроса» не проверен нигде (minor). Оракула нет: инъекции отказов в HTTP-тестах не существует. Понижено до наблюдения.
V5 — владение судится у записи, а файл открывается по её ссылке без сверки files.record_id/files.owner_id (minor, свойство без пути). Находка о будущем: delete-record и long-audio-chunking будут править обе стороны. В урожай.
S2 — goose_db_version.tstamp несёт вид времени и умолчание вне объявленной нормы (minor). Исход — выбор нормы, не правка кода. В урожай развилкой.
## 3б. Урожай — реальное, но не для этого мерджа
1. /%6detrics отдаёт метрики байт в байт (adversary V3, mounts.go:57-62). Проверено сырыми запросами. Понижено: docs/security.md:14 объявляет метрики открытыми без узнавания. Задача — судить по EscapedPath() либо вынести /metrics на отдельный слушатель.
2. Своя реализация диапазонов вместо http.ServeContent (R2): ~110 строк семантики HTTP, которую стандартная библиотека делает сама.
3. Остановка закрывает базу под живой горутиной (C2 + O3, main.go:249-263, db.go:157-172): по истечении ForceShutdownTimeout run() возвращается, отложенный db.Close() обнуляет пулы, брошенный воркер разыменует nil и роняет процесс паникой; замер — Close() вернулся за 2.8 мкс, пока другая горутина держала пишущую транзакцию, и та закоммитила после. Захват остаётся до 8 часов, следа нет.
4. Heartbeat воркера (законная половина O1): «конвейер встал» и «очередь пуста» неотличимы. Считать NoopJobError нельзя — инвариант.
5. Две строки ERROR на один транзиентный отказ шага (O2, воспроизведено дословным выводом). Вопрос записан в docs/review.md от 2026-08-10 и остаётся открытым.
6. sql.ErrNoRows не транслирован в доменную ошибку в трёх репозиториях (C6), при том что record_repo.go:139-141 в том же пакете правило исполняет.
7. cmd/devtools/resume.go не проверен ничем (S4): go test ./cmd/... даёт no test files, а тест воспроизводит тело подкоманды руками. Вынести тело из main-пакета в вызываемую функцию.
8. Развилка владельца по goose_db_version (S2): сузить требование и назвать таблицу учёта либо расширить сторож на всю применённую схему с поимённым исключением.
9. Запись решения ADR переписана на месте (S6): смена двух решений описана как уточнение формулировки. Это работа av-dev:doc-healthcheck.
10. Мелочь: имена копий <ULID><ext> не читаются без базы; defaultListLimit = 30 рядом с DefaultPageLimit = 30; RecordEventRepository.Append не заполняет event.CreatedAt; мёртвый довод prefix у selectList.
Выброшено как вкусовщина: ident.Timestamp/store.HasTemporary как экспортированная поверхность ради тестов; nullString/bytesReader; имя параметра copy против view; busy_timeout_ms как ключ с единицей измерения в имени; C5 (устаревший абзац logging.md:217-223) — работа сверки документов.
Проверено против «Типовые ложноположительные» docs/review.md:120-169: совпадение одно — молчание воркера на NoopJobError, снято из O1.
## 4. Promote candidates
- Правило сканера на rowToAudioRecord: перечень колонок чтения и перечень присвоений в сущность обязаны сверяться механически.
- Конвенция: значение, которым распоряжается спрашивающий, не идёт в журнал дословно ни под каким корнем. Сегодня logging.md:113 формулирует это только для запроса, отданного приложению, и находка V2 прошла в зазор.
## 5. Границы покрытия
Запускались specs, autotests, code, architecture, adversary, ops — все на метке large, режим по графу. basics не запускался: своих тем проекта нет. Разметчик review-scope в обычном виде не отрабатывал — метка задана владельцем, размер и сложность не мерились, обоснования разметки у этого прогона не существует. Корректор метки: сигнал от review-code, занижения нет; второго голоса нет.
Сработавшие потолки: architecture — 3, за срезом четыре подпороговых наблюдения и четыре «дешевле переделать»; code — конвенций 4/4, за срезом мёртвый довод prefix и незаполненный event.CreatedAt; specs, ops, autotests, adversary своих потолков не сообщили — сказать, сколько осталось за их срезом, нельзя; потолок триажа — 7 мест на 27 находок, за срез уехали V3, вторая половина R2, C2+O3, heartbeat, O2, C6, S4, S2, S6, все названы поимённо в урожае. Молча не выброшено ничего.
Чего проходы не могли проверить: adversary не поднимал настоящую Authelia и настоящий обратный прокси — весь барьер входа держится им, браузера в прогоне нет; ops не имел реального профиля нагрузки; autotests судил покрытие, а не способность теста упасть — мутационная сверка в проекте запрещена.
Осталось на человеке (docs/review.md:289-335): поведение SpeechKit и Object Storage под нагрузкой; реальный профиль нагрузки; стойкость ffmpeg к вредоносному входу; поведение настоящей Authelia и правило обратного прокси — и это прямо задевает две находки: обход X-Forwarded-For вменяется прокси в чужом репозитории, а лечится здесь, а достижимость /%6detrics зависит от правила прокси, которого отсюда не видно; поведение браузера с куками. Перестали проверять сознательно: разбор вывода настоящего ffprobe; работа с настоящими внешними собеседниками.
Четыре строки, которых не принёс ни один проход: решения проекта не сверялись (docs/adr/ процессный, расхождение ловит doc-healthcheck); записанные наблюдения не использовались (docs/research/ не открывался, каждое число снято на этом прогоне); поимённая сверка с руководствами по стилю Go не задавалась; альтернативной реализации, с которой можно сдиффить решения, у конвейера нет — «не знаю, чего не знаю» на изменении, переносящем всё хранилище, не достаёт никто.
Поразрядная деградация одна и своя: инвариант про колонки записи существует, но потерял предмет, поэтому сослаться на него как на оракул было нельзя — вынесено отдельной находкой.