- база своя: два пула, захват одним UPDATE ... RETURNING, шаги схемы на goose под файловым замком, одна миграция начальной схемы вместо семи прежних - транспорт переписан на net/http: свои слои, свой ограничитель частоты, отдача файла с проверкой владельца; панель /_/ и пространство /api/ исчезли - по находкам ревью: журнал не пишет путь под корнем приложения, ключ бюджета читается справа налево, узнавание известного идёт читающим пулом
25 KiB
Отчёт триажа ревью кода — 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б. Урожай — реальное, но не для этого мерджа
- /%6detrics отдаёт метрики байт в байт (adversary V3, mounts.go:57-62). Проверено сырыми запросами. Понижено: docs/security.md:14 объявляет метрики открытыми без узнавания. Задача — судить по EscapedPath() либо вынести /metrics на отдельный слушатель.
- Своя реализация диапазонов вместо http.ServeContent (R2): ~110 строк семантики HTTP, которую стандартная библиотека делает сама.
- Остановка закрывает базу под живой горутиной (C2 + O3, main.go:249-263, db.go:157-172): по истечении ForceShutdownTimeout run() возвращается, отложенный db.Close() обнуляет пулы, брошенный воркер разыменует nil и роняет процесс паникой; замер — Close() вернулся за 2.8 мкс, пока другая горутина держала пишущую транзакцию, и та закоммитила после. Захват остаётся до 8 часов, следа нет.
- Heartbeat воркера (законная половина O1): «конвейер встал» и «очередь пуста» неотличимы. Считать NoopJobError нельзя — инвариант.
- Две строки ERROR на один транзиентный отказ шага (O2, воспроизведено дословным выводом). Вопрос записан в docs/review.md от 2026-08-10 и остаётся открытым.
- sql.ErrNoRows не транслирован в доменную ошибку в трёх репозиториях (C6), при том что record_repo.go:139-141 в том же пакете правило исполняет.
- cmd/devtools/resume.go не проверен ничем (S4): go test ./cmd/... даёт no test files, а тест воспроизводит тело подкоманды руками. Вынести тело из main-пакета в вызываемую функцию.
- Развилка владельца по goose_db_version (S2): сузить требование и назвать таблицу учёта либо расширить сторож на всю применённую схему с поимённым исключением.
- Запись решения ADR переписана на месте (S6): смена двух решений описана как уточнение формулировки. Это работа av-dev:doc-healthcheck.
- Мелочь: имена копий не читаются без базы; 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 не задавалась; альтернативной реализации, с которой можно сдиффить решения, у конвейера нет — «не знаю, чего не знаю» на изменении, переносящем всё хранилище, не достаёт никто. Поразрядная деградация одна и своя: инвариант про колонки записи существует, но потерял предмет, поэтому сослаться на него как на оракул было нельзя — вынесено отдельной находкой.