учёт: заведён урожай ревью входа по конфигу
- пять задач партии review-2026-08-23: многозначность Remote-Name и Remote-Email, мелочи входа, механизация правила о зависимостях конфига, снятие мёртвого читателя .env, управляющие знаки в расширении - две существующие разведки дополнены находками того же прогона: адрес объекта в тексте отказа SpeechKit и выводимость имени копии из журнала
This commit is contained in:
@@ -0,0 +1,42 @@
|
||||
# 🧹 Механизировать правило «internal/config не знает транспорта»
|
||||
|
||||
- **Тип:** chore
|
||||
- **Категория:** Очередь — Оба правят проверки, которым потом верят: правило направления зависимостей встаёт рядом с покрытием оснастки, до задач, что опираются на зелёный гейт
|
||||
- **Зачем:** правило записано конвенцией, а сканера у него нет; проект его уже нарушает — go test ./internal/config линкует всю поверхность HTTP, и будущая надобность конфига в транспорте даст цикл импорта на уровне проверок
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Направление зависимостей в проекте держат тесты-сканеры `internal/archrules`, а
|
||||
не договорённость. У правила «`internal/config` не знает транспорта» сканера нет,
|
||||
и проект его уже нарушает — файлом проверок, а не рабочим кодом.
|
||||
|
||||
Нашёл прогон ревью `config-test-headers-login` 2026-08-23: норма записана в
|
||||
`docs/conventions/config.md`, а `grep -n "config" internal/archrules/arch_test.go`
|
||||
не даёт ни одного совпадения. Нарушает её сегодня файл проверок —
|
||||
`internal/config/test_headers_test.go` объявлен `package config` и импортирует
|
||||
`internal/controller/http`. В рабочем бинарнике этого ребра нет, но
|
||||
`go test ./internal/config` линкует всю поверхность HTTP.
|
||||
|
||||
## Затрагивает
|
||||
|
||||
- `internal/config/test_headers_test.go` — перевод во внешний пакет
|
||||
`package config_test`;
|
||||
- `internal/archrules/arch_test.go` — новое правило;
|
||||
- `docs/conventions/config.md` — источник нормы, которую правило механизирует.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
- Правило существует и зелёное. **Оракул:** `internal/archrules/arch_test.go`
|
||||
содержит правило «`internal/config` не знает транспорта», и
|
||||
`go test ./internal/archrules` зелёный.
|
||||
- Файл проверок объявлен `package config_test`. **Оракул:**
|
||||
`head -1 internal/config/test_headers_test.go` даёт `package config_test`,
|
||||
`go test ./internal/config` зелёный.
|
||||
- Импорта `internal/controller/http` в пакете `config` нет. **Оракул:**
|
||||
`go list -deps ./internal/config` не содержит `internal/controller/http`, и то
|
||||
же говорит новое правило `internal/archrules`.
|
||||
|
||||
## Рамки
|
||||
|
||||
Внутренний порядок задан: сначала перенос файла во внешний пакет, потом правило,
|
||||
— иначе правило по всем файлам уронит гейт. Норма не пересматривается: правится
|
||||
то, чем она проверяется, а не она сама.
|
||||
@@ -0,0 +1,37 @@
|
||||
# 🧹 Снять мёртвый godotenv.Load() вместе с зависимостью
|
||||
|
||||
- **Тип:** chore
|
||||
- **Категория:** Очередь — Чистка старта сервиса идёт вместе с задачей о локальном запуске: обе трогают одну точку входа
|
||||
- **Зачем:** сервис на каждом старте печатает предупреждение, приглашающее завести .env, читать который некому: это вторая точка, где может завестись секрет, и путь к нарушению критического инварианта сервис подсказывает сам
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Настройки сервис берёт из файла конфига целиком, а переменных окружения не
|
||||
читает нигде. Вызов `godotenv.Load()` остался от прежней раскладки и на каждом
|
||||
старте печатает предупреждение, приглашающее завести `.env`, — то есть завести
|
||||
вторую точку, где может осесть секрет.
|
||||
|
||||
Нашёл прогон ревью `config-test-headers-login` 2026-08-23.
|
||||
|
||||
## Затрагивает
|
||||
|
||||
- `cmd/transcriber/main.go` — вызов `godotenv.Load()` и его импорт;
|
||||
- `go.mod` — зависимость `github.com/joho/godotenv`;
|
||||
- `.dockerignore` — строка `.env`.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
- Переменных окружения не читает никто. **Оракул:**
|
||||
`grep -rn "os.Getenv\|os.LookupEnv" --include=*.go internal/ cmd/ | grep -v _test`
|
||||
→ пусто.
|
||||
- Зависимости в `go.mod` нет. **Оракул:** `grep godotenv go.mod go.sum` → пусто,
|
||||
`go build ./...` зелёный.
|
||||
- Предупреждения на старте нет. **Оракул:** прогон бинарника сегодня печатает
|
||||
`level=WARN msg="Warning: .env file not found, using system environment variables"`;
|
||||
после правки такой строки в выводе старта нет.
|
||||
|
||||
## Рамки
|
||||
|
||||
Если переменные окружения кому-то понадобятся, у них заводится свой читатель, и
|
||||
это другая задача. Строку `.env` в `.gitignore` уже добавило слияние прежней
|
||||
задачи — она
|
||||
остаётся.
|
||||
@@ -0,0 +1,68 @@
|
||||
# 🐞 Судить многозначность Remote-Name и Remote-Email тем же правилом, что и логин
|
||||
|
||||
- **Тип:** fix
|
||||
- **Категория:** Очередь — Правило узнавания правится там же, где личность связывается с учётной записью: обе задачи трогают один сбор contract.Identity
|
||||
- **Зачем:** чужой адрес, занятый первым пришедшим, навсегда и молча оставляет без почты настоящего владельца: EnsureUser найденную запись не переписывает, ветвь отката заводит владельца без почты и без строки журнала
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Многозначный `Remote-User` даёт `401` — многозначный `Remote-Email` даёт `200`,
|
||||
и в базу ложится первое значение. Правило узнавания судит логин и не судит
|
||||
остальные заголовки личности.
|
||||
|
||||
Один и тот же адрес почты, занятый чужим логином, достаётся первому пришедшему
|
||||
навсегда: `EnsureUser` найденную запись не переписывает, а ветвь отката
|
||||
уникальности заводит настоящего владельца **без почты** — и делает это молча,
|
||||
без строки журнала.
|
||||
|
||||
Нашёл прогон ревью `config-test-headers-login` 2026-08-23, оракул воспроизведён.
|
||||
|
||||
## Воспроизведение
|
||||
|
||||
Свой падающий сценарий, прогнан на настоящей поверхности сервиса (`setupEnv` +
|
||||
`AppChain`, база SQLite во временном каталоге, маршрут `/app/me`). Два шага:
|
||||
|
||||
1. Запрос с `Remote-User: squatter` и `Remote-Email: victim@corp.example` с
|
||||
доверенного адреса → `200`, `SELECT email FROM users WHERE
|
||||
provider_login='squatter'` → `victim@corp.example`.
|
||||
2. Запрос настоящего владельца `Remote-User: victim`, `Remote-Email:
|
||||
victim@corp.example` → `200`, `SELECT email … WHERE provider_login='victim'`
|
||||
→ `""`, строк журнала о занятом адресе ноль.
|
||||
|
||||
Ожидалось: многозначный заголовок судится тем же правилом, что и логин, а откат
|
||||
по занятому адресу виден владельцу сервиса строкой журнала.
|
||||
|
||||
## Затрагивает
|
||||
|
||||
- `internal/controller/http/identity.go:134-138` — сбор `contract.Identity` из
|
||||
заголовков: `Login` берётся из проверенного перечня значений, `Name` и `Email`
|
||||
— через `Header.Get`, то есть первым значением без суждения;
|
||||
- `internal/adapter/repo/sqlite/identity.go:77-110` — ветвь отката уникальности
|
||||
почты: заводит запись с пустым адресом и не пишет ни строки;
|
||||
- требование спеки `access`: сегодня оно нормирует многозначность только для
|
||||
`Remote-User`;
|
||||
- `docs/security.md`, раздел «Периметр» — что именно сервис принимает от контура
|
||||
на веру.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
- Многозначный `Remote-Email` даёт пустое значение, а не первое. **Оракул:**
|
||||
тест на настоящей поверхности сервиса — запрос с двумя значениями `Remote-Email`
|
||||
заводит учётную запись с пустым адресом; тот же тест на `Remote-Name`.
|
||||
- Узнавание принимает однозначные `Remote-Name` и `Remote-Email` по-прежнему.
|
||||
**Оракул:** тот же тест — одно значение доезжает до колонки без изменений.
|
||||
- Ветвь отката уникальности почты оставляет строку журнала видимого уровня.
|
||||
**Оракул:** тест хранилища — второй логин с занятым адресом заводится, и в
|
||||
журнале стоит строка о занятом адресе; значения адреса в ней нет.
|
||||
|
||||
## Рамки
|
||||
|
||||
Правится боевой путь узнавания, а не отладочная подстановка заголовков.
|
||||
Владение адресом почты между учётными записями не пересматривается: занятый
|
||||
адрес по-прежнему достаётся тому, кто завёлся первым, — задача только о том,
|
||||
чтобы это перестало происходить молча и по многозначному заголовку.
|
||||
|
||||
Отдельно: требование к контуру про `X-Forwarded-For` не записано ни в модели
|
||||
угроз, ни в спеке и этой правкой не покрывается — правило «справа налево до
|
||||
первого недоверенного» верно постольку, поскольку правое значение приписал
|
||||
прокси, а шаблон лежит в `files/caddyproxy/Caddyfile.template` репозитория
|
||||
`pet-project-server`. Отдельной записи не заводится — решение владельца.
|
||||
@@ -9,8 +9,9 @@
|
||||
каталогу на диске не сказать ни чья запись, ни какого уровня копия: имя читается
|
||||
только вместе с базой.
|
||||
|
||||
Пока записей мало, это не мешает. Цена растёт с каждой уложенной записью:
|
||||
раскладка каталога данных объявлена необратимой, и переименование задним числом
|
||||
Пока на боевом сервере записей нет вовсе (стадия «стройка»), это не мешает.
|
||||
Цена растёт с каждой уложенной записью: раскладка каталога данных объявлена
|
||||
необратимой, и переименование задним числом
|
||||
стоит прохода по всему каталогу и по всем ссылкам в базе.
|
||||
|
||||
Разведка отвечает не «переименовать ли», а «нужна ли читаемость без базы вообще»
|
||||
@@ -20,8 +21,29 @@
|
||||
|
||||
## Вопрос
|
||||
|
||||
Нужно ли имени копии нести что-то, по чему запись узнаётся без обращения к базе,
|
||||
и какой ценой это берётся сейчас против той, что придётся заплатить потом?
|
||||
Вопроса два, и оба об одном имени копии.
|
||||
|
||||
1. Нужно ли имени копии нести что-то, по чему запись узнаётся без обращения к
|
||||
базе, и какой ценой это берётся сейчас против той, что придётся заплатить
|
||||
потом?
|
||||
2. Допустимо ли, что имя копии выводится из строки журнала арифметикой, и если
|
||||
нет — чем платить за невыводимость, не потеряв порядок имён?
|
||||
|
||||
Второй вопрос пришёл прогоном ревью `config-test-headers-login` 2026-08-23, и
|
||||
выводимость там уже установлена: `TestProbeStorageNameDerivesFromRecordID` даёт
|
||||
на `record_id 01m0pwf0r1cp9v0zt83w79ccbn` имя копии
|
||||
`01m0pwf0r1cp9v0zt83w79ccbp.mp3`, ровно `инкремент(record_id)+ext`, а
|
||||
`TestProbeDerivationRate` — 49 из 50 приёмов подряд. Журнал несёт `record_id` и
|
||||
`file_ext`, то есть имя собирается без обращения к каталогу данных, и инвариант
|
||||
«Имя файла на диске задаёт сервис, а в журнал не идёт» соблюдён по букве, но не
|
||||
по назначению. Триаж тест не воспроизводил. Спрашивается поэтому не факт, а его цена.
|
||||
|
||||
Первый вопрос от этого дешевеет: часть читаемости без базы уже есть — просто не
|
||||
по назначению и не решением.
|
||||
|
||||
Оба вопроса решает одна разведка, и отдельной записи второй не получает —
|
||||
решение владельца. Довод: раскладка каталога данных объявлена необратимой, и два
|
||||
ответа порознь дали бы две необратимые правки одного каталога.
|
||||
|
||||
## Куда ляжет ответ
|
||||
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
# 🐞 Отвергать расширение с управляющими знаками на приёме, а не отдавать 500
|
||||
|
||||
- **Тип:** fix
|
||||
- **Категория:** Очередь — Вторая половина того же разреза: границы полей журнала задаются там же, где судится состав расширения
|
||||
- **Зачем:** комментарий кода объявляет этот исход недопустимым и ради него заведён предел длины расширения, но разрез сделан по длине и не сделан по составу знаков: отправитель получает 500, владелец — строку ERROR, неотличимую от аварии хранилища
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Расширение приходит из имени, данного отправителем, и приём судит у него только
|
||||
длину. Управляющий знак внутри расширения проходит предел длины, доезжает до
|
||||
заведения временной копии и роняет его: отправитель получает `500` за вход, за
|
||||
который отвечает сам, а владелец — строку `ERROR`, неотличимую от аварии
|
||||
хранилища. Ровно этот исход комментарий кода объявляет недопустимым и ради него
|
||||
заводит предел длины.
|
||||
|
||||
Нашёл прогон ревью `config-test-headers-login` 2026-08-23. Триаж тест не
|
||||
воспроизводил, поэтому находка понижена до гипотезы.
|
||||
|
||||
## Воспроизведение
|
||||
|
||||
Принять запись, чьё расширение несёт управляющий знак: ответ `500`, в журнале
|
||||
строка `ERROR`. Ожидался отказ приёма своим кодом.
|
||||
|
||||
Падающий тест прохода `adversary`: имя доезжает формой RFC 2231
|
||||
(`filename*=UTF-8''запись.mp%003`), ответ `500`, журнал
|
||||
`level=ERROR msg="Failed to stage uploaded file" error="failed to create work file: open /tmp/transcriber-….mp\x003: invalid argument"`.
|
||||
|
||||
## Затрагивает
|
||||
|
||||
- `internal/service/transcribe.go:171-188` — разрез расширения по длине и сборка
|
||||
имени копии;
|
||||
- `internal/adapter/repo/sqlite/file_repo.go:56-67` — заведение временной копии,
|
||||
которое и отказывает.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
- Приём судит состав знаков расширения так же, как судит длину. **Оракул:** тест
|
||||
приёма — имя с управляющим знаком в расширении даёт отказ приёма своим кодом,
|
||||
а не `500`; в журнале нет строки `ERROR` об отказе хранилища.
|
||||
- Приём принимает годное расширение по-прежнему. **Оракул:** тот же тест — `.mp3`,
|
||||
`.ogg` и имя без расширения дают прежний исход.
|
||||
|
||||
## Рамки
|
||||
|
||||
Половину про путь из `*os.PathError` в журнале держит
|
||||
`journal-fields-bounded-by-service`, здесь она не переписывается.
|
||||
@@ -3,6 +3,7 @@
|
||||
- **Тип:** research
|
||||
- **Категория:** Очередь — Тот же род утечки в журнал, но вне темы входа — идёт следом за ней.
|
||||
- **Зачем:** Текст отказа операции приходит от Yandex и уезжает в журнал и в колонку error_text: если он несёт URI объекта, из журнала снова собирается ссылка на чужую запись.
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Гипотеза ревью задачи `pocketbase-storage` 2026-08-12, оставшаяся без оракула:
|
||||
поднять настоящий отказ Yandex нечем, а прогон на реальных ключах запрещён.
|
||||
@@ -10,6 +11,11 @@
|
||||
Object Storage больше не называют ключ, — но текст, пришедший **чужими руками**,
|
||||
идёт в журнал и в задачу как есть.
|
||||
|
||||
Подтверждено прогоном ревью `config-test-headers-login` 2026-08-23: та же
|
||||
находка пришла второй раз. Оракула у неё нет и быть не может — нужен
|
||||
настоящий SpeechKit, а прогон на реальных ключах запрещён («Yandex Cloud за
|
||||
деньги»).
|
||||
|
||||
## Вопрос
|
||||
|
||||
Несёт ли текст отказа операции распознавания адрес объекта, который мы отдали
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# 🧹 Разобрать мелочи входа по доверенному заголовку
|
||||
|
||||
- **Тип:** chore
|
||||
- **Категория:** Очередь — Место машинное, конец очереди: мелочи ниже потолка триажа никого не держат и ничего не требуют — берутся, когда дойдут
|
||||
- **Зачем:** три пункта ниже потолка триажа остались от одной задачи: функция строки старта не вызвана ни одним тестом, форма записи необязательного поля конфига стала неотличима от обязательного, два комментария описывают снятую подкоманду и не то число звеньев цепочки
|
||||
- **Теги:** review-2026-08-23
|
||||
|
||||
Три пункта ниже потолка триажа, оставшиеся от задачи `config-test-headers-login`.
|
||||
Наблюдаемое поведение сервиса ни один из них не меняет: правится защита от
|
||||
регрессии, форма записи умолчаний и два неверных комментария.
|
||||
|
||||
Нашёл прогон ревью `config-test-headers-login` 2026-08-23.
|
||||
|
||||
## Затрагивает
|
||||
|
||||
- `internal/controller/http/substitute.go` — функция имён подставляемых
|
||||
заголовков и её тест;
|
||||
- `internal/config/config.go`, `defaultConfig()` — форма записи умолчаний
|
||||
необязательного поля;
|
||||
- `Dockerfile:49-50` — комментарий об исключении `cmd/devtools` из сборки;
|
||||
- `internal/controller/http/mounts.go:54-56` — комментарий, перечисляющий звенья
|
||||
цепочки слоёв приложения.
|
||||
|
||||
## Критерии приёмки
|
||||
|
||||
- Функцию имён подставляемых заголовков зовёт тест. **Оракул:**
|
||||
`SubstitutedHeaderNames` не вызвана ни одним тестом (`go tool cover -func` →
|
||||
0,0 %). Прогон настоящего бинарника печатает `headers=[Remote-User]` — имена,
|
||||
не значения: дефекта сегодня нет, нет защиты от регрессии против требования
|
||||
спеки «строка старта MUST не нести значений».
|
||||
- Умолчания необязательных полей заданы в одном месте с прочими. **Оракул:**
|
||||
умолчания `Debug` и `TestHeaders` заданы нулём типа, а не в `defaultConfig()`.
|
||||
Нарушена `docs/conventions/config.md`, «Структура в коде»: «Умолчания задаются
|
||||
в `defaultConfig()`… Новое поле требует правки обоих мест», и там же форма
|
||||
обязательного поля — «умолчания нет ни в `defaultConfig()`, ни по нулевому
|
||||
значению типа».
|
||||
- Оба комментария описывают то, что есть. **Оракул:** комментарий
|
||||
`Dockerfile:49-50` объясняет исключение `cmd/devtools` словами «оснастка
|
||||
разработчика (подставной прокси)», а подкоманды больше нет; приёмочный шаг
|
||||
искал следы по строке `devtools proxy` и эту не поймал.
|
||||
`internal/controller/http/mounts.go:54-56` перечисляет три звена цепочки, тогда
|
||||
как `AppChain` одевает четыре.
|
||||
|
||||
## Рамки
|
||||
|
||||
Поведение сервиса не меняется ни одним пунктом. Имя ключа конфига не трогается:
|
||||
правится место, где задано его умолчание, а не сам ключ.
|
||||
Reference in New Issue
Block a user