у записи появился владелец: чужую больше не отдают
- колонка `owner` связью с `users` в обеих коллекциях новым шагом схемы `202608140001`; чтение задачи сужено владельцем, и чужая, ничья и несуществующая дают один ответ; правило просмотра файлов сужено им же - приём по HTTP берёт владельца из сессии, а предъявителя без учётной записи пользователя отвергает до чтения тела: позже пришлось бы убирать уложенный файл, а уборки файлов сервис не умеет. Выборка воркера владельцем не сужается - удаление учётной записи с записями отвергается стражем, и вешает его сама сборка хранилища: сборка, забывшая его позвать, теряла защиту молча
This commit is contained in:
@@ -0,0 +1,204 @@
|
||||
# Ревью изменения `record-ownership` — финальный триаж
|
||||
|
||||
## Сводка
|
||||
|
||||
- **Размер:** среднее, пограничное с крупным. **Сложность:** незнакомое
|
||||
(`docs/review.md`, «Триггеры метки»: вход через OIDC и разграничение доступа до
|
||||
начала работы назвать нельзя). **Метка:** `large`. **Режим:** по графу.
|
||||
- **Изменение не закоммичено**, судилось рабочее дерево (19 изменённых файлов,
|
||||
5 новых).
|
||||
- **Гейт зелёный**, проверено независимо от прохода `autotests`: на чистой копии
|
||||
дерева `go build`, `go vet`, `go test ./... -count=1` — все пакеты `ok`.
|
||||
- **Сигнал о заниженной метке:** `review-code` метку проверил, занижением не
|
||||
считает. `review-basics` не запускался, поэтому второго независимого голоса
|
||||
нет. Возражений против `large` не подал никто.
|
||||
- **Находок на входе:** 26 (autotests 1, specs 5, code 3, adversary 6, ops 4,
|
||||
architecture 7) плюс 3 наблюдения. **Осталось:** 7 в основном списке,
|
||||
4 понижены в гипотезы, 3 в promote.
|
||||
|
||||
### План разметки задачи с исходом по каждой теме
|
||||
|
||||
| Тема | Дом | Глубина | Кто закрывает | Исход |
|
||||
|---|---|---|---|---|
|
||||
| requirements | дельты + `openspec/specs/{access,intake,pipeline,storage}` | разбор | `specs` | закрыта, 5 находок |
|
||||
| autotests | `CLAUDE.md` «Гейт» | — | `autotests` | закрыта, 1 находка |
|
||||
| conventions | `docs/conventions/` | разбор | `code` | закрыта, 3 находки; конвенционных 0 из 4 |
|
||||
| architecture | `docs/architecture.md` + `passport.md` | доказательство | `architecture` | закрыта, 7 находок |
|
||||
| security | `docs/security.md` | доказательство | `adversary` | закрыта, 3 пути + 3 свойства |
|
||||
| operations | `docs/architecture.md` «Эксплуатация» + `database.md` | доказательство | `ops` | закрыта, 4 находки |
|
||||
|
||||
Тем без отчёта нет. Тем без дома нет.
|
||||
|
||||
## Блокирует мердж
|
||||
|
||||
### 1. Владелец панели видит на отказе удаления чужую подсказку и по ней идёт сносить архив руками
|
||||
|
||||
- Файл: `internal/adapter/repo/pocketbase/owner_guard.go:42-45`
|
||||
- Severity: major. Confidence: high. Действие: **инлайн**
|
||||
- Оракул: прогон через собранный роутер. Удаление учётной записи с задачей даёт
|
||||
`400 {"message":"Failed to delete record. Make sure that the record is not part
|
||||
of a required relation reference."}` — текст стража не доезжает вовсе
|
||||
(`apis/record_crud.go` и `firstApiError` подменяют неклассифицированную ошибку
|
||||
своей). Починка проверена тем же прогоном: `router.NewBadRequestError` доезжает
|
||||
дословно.
|
||||
- Последствие: требование дельты `storage` «Отказ MUST называть причину владельцу
|
||||
панели» не выполнено. Подсказка библиотеки **ведущая**: единственная
|
||||
обязательная связь у задачи — `file`, и владелец панели, поверив ей, пойдёт
|
||||
удалять задачи и файлы руками. Это ровно то необратимое удаление архива,
|
||||
которое страж и заведён предотвращать.
|
||||
- Найдено проходами: `specs`, `code`, `adversary` (одна причина, три
|
||||
формулировки).
|
||||
|
||||
### 2. Право на архив снимается удалением учётной записи: страж считает задачи и не считает файлы, а вешается мимо сборки хранилища
|
||||
|
||||
- Файл: `internal/adapter/repo/pocketbase/owner_guard.go:28-49`, `main.go:208-213`
|
||||
- Severity: major. Confidence: high. Действие: **развилка**
|
||||
- Оракул: два прогона через роутер. **Без стража** — окружение `setupTestEnv`
|
||||
ставит `BindPanelRules`, но `GuardOwnerDeletion` не ставит: удаление своей
|
||||
учётной записи собственной сессией даёт `204`, у задачи и у файла владелец
|
||||
снимается. **Со стражем** — учётная запись, у которой есть файлы и нет задач,
|
||||
удаляется штатно, владелец файла снимается.
|
||||
- Последствие: `users.DeleteRule` умолчанием библиотеки равен
|
||||
`id = @request.auth.id` — вошедший сносит себя сам, и это публичная поверхность,
|
||||
а не только панель. Единственная защита архива — одна строка в `main.go`, и она
|
||||
покрывает одну коллекцию из двух. Потеря необратима: прежнего владельца не
|
||||
остаётся нигде.
|
||||
- Найдено проходами: `adversary`, `specs`, `architecture`.
|
||||
|
||||
### 3. Канон продолжит утверждать, что разграничения по владельцу нет
|
||||
|
||||
- Файл: `openspec/specs/storage/spec.md:109-111`, `docs/architecture.md:38`,
|
||||
`docs/review.md:120-126`
|
||||
- Severity: major. Confidence: high. Действие: **инлайн**
|
||||
- Оракул: дословный текст. Действующая спека `storage` держит «Сужения по
|
||||
владельцу здесь нет — его заводит отдельная задача»; дельта заводит сужение
|
||||
секцией `## ADDED`, не сняв это. `docs/review.md:120` держит запись в разделе
|
||||
«Типовые ложноположительные» — прямое указание будущему проходу выбросить
|
||||
такую находку.
|
||||
- Последствие: следующая задача вправе вернуть правило «всякий узнанный» и снова
|
||||
открыть чужое аудио, а ревью эту регрессию выбросит не глядя.
|
||||
- Найдено проходами: `specs` (две находки), `architecture`.
|
||||
|
||||
## Стоит исправить сейчас
|
||||
|
||||
### 4. Сценарий «Чужой файл не отдаётся» нормирует механизм, которого нет
|
||||
|
||||
- Файл: дельта `storage`, `internal/controller/http/ownership_test.go:180-206`
|
||||
- Severity: major. Confidence: high. Действие: **инлайн**
|
||||
- Оракул: прогон. Чужой просит токен файла — `200` и валидный токен: токен
|
||||
выдаётся **на предъявителя**, а не на файл. Он же идёт по настоящей ссылке
|
||||
`GET /api/files/files/{recordId}/{name}?token=…` — `404`; владелец — `200` и
|
||||
содержимое.
|
||||
- Последствие: спека обязывает к отказу, которого не будет никогда. Единственный
|
||||
механизм, закрывающий чужое аудио, сквозной проверки не имеет: снятие правила
|
||||
просмотра гейт не покраснит.
|
||||
- Найдено проходом: `specs`.
|
||||
|
||||
### 5. Сбой хранилища приходит отправителю как «задачи нет» и не оставляет строки в журнале
|
||||
|
||||
- Файл: `internal/adapter/repo/pocketbase/transcript_job_repo.go:96-111`,
|
||||
`internal/controller/http/transcribe.go:107-118`
|
||||
- Severity: minor. Confidence: high. Действие: **инлайн**
|
||||
- Оракул: прогон. Запрос недостижимой задачи — `404`, прибавка к журналу пустая.
|
||||
Обработчик кладёт в `404` **любую** ошибку, включая отказ базы.
|
||||
- Последствие: `design.md` обещает «различать в журнале» — не реализовано.
|
||||
Отправитель на аварию хранилища получает «записи нет», владелец сервиса об
|
||||
аварии не узнаёт ниоткуда.
|
||||
- Найдено проходами: `code`, `ops`.
|
||||
|
||||
### 6. Окно отката образа заводит записи без владельца — навсегда недостижимые создателю и молча
|
||||
|
||||
- Файл: `internal/adapter/repo/pocketbase/migrations/202608140001_record_owner.go`,
|
||||
`internal/service/transcribe.go:581-584`
|
||||
- Severity: minor (ущерб крупный, наступает только при откате). Confidence: high.
|
||||
Действие: **развилка**
|
||||
- Оракул: прогон, воспроизводящий состояние окна отката: создатель просит
|
||||
состояние — `404`, журнал пуст. `send()` для источника, отличного от Telegram,
|
||||
возвращает `nil` без отправки — иных каналов доставки у записи из API нет.
|
||||
- Последствие: шаг схемы применён и не откатывается вместе с образом; прежний
|
||||
бинарь колонку не пишет.
|
||||
|
||||
### 7. Имя файла в хранилище уезжает в журнал контейнера при каждом скачивании
|
||||
|
||||
- Файл: `main.go:236-240`
|
||||
- Severity: major. Confidence: high. Действие: **развилка**
|
||||
- Оракул: `CLAUDE.md`, «Инварианты», дословно. Форма маршрута подтверждена
|
||||
прогоном: скачивание идёт по `GET /api/files/files/{recordId}/{name}`, а слой
|
||||
журнала пишет `URL.Path` целиком.
|
||||
- Последствие: инвариант нарушен буквально, второй случай того же класса в
|
||||
журнале проекта. До `critical` не поднято: названная инвариантом цена — «строка
|
||||
журнала стала бы бессрочным ключом» — этой же задачей снимается. Находка **вне
|
||||
дельты**.
|
||||
|
||||
## Гипотезы без доказательства
|
||||
|
||||
- **Счёт задач по `owner` идёт полным сканом, индекса нет** (`ops`). Замер: 42 мс
|
||||
против 0,07 мс на 150 тыс. строк. Понижено: объёма, на котором снят замер, у
|
||||
проекта нет.
|
||||
- **Гонка «удаление учётной записи против приёма»** — не воспроизведена.
|
||||
- **Разница во времени ответа на свою и чужую запись** — замера нет.
|
||||
- **`users.UpdateRule` открыт правке своей записи** — построенного пути нет.
|
||||
- **Ветка отказа `CountRecords` не покрыта** — поведение fail-closed; после
|
||||
починки находки 1 покроется тем же тестом.
|
||||
- **Неизменяемость владельца механизмом не держится** — панель правит `owner`
|
||||
свободно, но панель в модели угроз доверена.
|
||||
|
||||
## Promote candidates
|
||||
|
||||
- **Именование колонок-связей.** `owner` — третий смысл «владельца» в проекте при
|
||||
живом прецеденте `location`/`storage`. Правило именования стоит завести в
|
||||
`docs/conventions/`.
|
||||
- **Одно отображение записи в задачу — и в тестах тоже.** `readJob` в
|
||||
`internal/service/pipeline_test.go` — второе, неполное отображение. Кандидат в
|
||||
расширение инварианта либо в правило `archrules`.
|
||||
- **Индекс на колонке, по которой ходит счёт внутри транзакции удаления.**
|
||||
Кандидат в `docs/database.md`.
|
||||
|
||||
## Границы покрытия
|
||||
|
||||
**Что запускалось.** Метка `large`, режим по графу: `specs`, `autotests`, `code`,
|
||||
`architecture`, `adversary`, `ops` — шесть именных проходов, все вернули отчёт.
|
||||
|
||||
**Что не запускалось и почему.** `review-basics` — план сказал, что своих тем
|
||||
проекта нет и все темы разобраны именными проходами. Следствие: у сигнала о
|
||||
заниженной метке остался один голос.
|
||||
|
||||
**Чего проходы не могли проверить в принципе.** Живой откат образа прежним
|
||||
бинарём; поведение под реальным потоком; настоящая панель как интерфейс;
|
||||
настоящие Telegram, SpeechKit и Object Storage.
|
||||
|
||||
**Что осталось на человеке** (`docs/review.md`, «Недоступно проверке»): поведение
|
||||
внешних сервисов под нагрузкой, реальный профиль нагрузки, стойкость `ffmpeg` к
|
||||
вредоносному входу, поведение настоящей Authelia, поведение браузера с куками.
|
||||
Сознательно перестали проверять: разбор вывода настоящего `ffprobe`, работу с
|
||||
настоящими внешними собеседниками.
|
||||
|
||||
**Сработавшие потолки.** Ни один проход не сообщил своего потолка и того, что
|
||||
осталось за срезом. Контракт требует этого от каждого — **это находка о прогоне**,
|
||||
а не о коде.
|
||||
|
||||
**Что не влезло в потолок 7** (ничего не выброшено молча):
|
||||
|
||||
- `RequireAuth(users)` висит на всей группе `/api`, поэтому и опрос отвечает
|
||||
`403`; дельта `intake` нормирует `403` только у приёма;
|
||||
- `design.md` прямым текстом отвергает правила коллекций, а реализация ими
|
||||
пользуется; «Единые точки проекта» о владельце не знают;
|
||||
- `ErrOwnerRequired` заведена без потребителя;
|
||||
- три вопроса, объявленных решаемыми человеком, возведены в норму реализацией —
|
||||
частично снято тем, что человек четыре решения на чекпоинте принял;
|
||||
- `RelationField.ColumnType` даёт `TEXT DEFAULT '' NOT NULL`: «умолчания нет»
|
||||
верно по замыслу, но не буквально.
|
||||
|
||||
**Чего в конвейере нет вовсе:**
|
||||
|
||||
1. **Решения проекта не сверялись** — `docs/adr/` процессный документ, прогон его
|
||||
не открывает. Расхождение ловит `av-dev:doc-healthcheck`.
|
||||
2. **Записанные наблюдения не использовались** — `docs/research/` не открывался.
|
||||
3. **Поимённая сверка с руководствами по стилю Go** не задавалась ни одним
|
||||
проходом.
|
||||
4. **Альтернативной реализации, с которой можно сдиффить решения, нет.** Для
|
||||
изменения со сложностью «незнакомое» это самый дорогой пробел прогона.
|
||||
|
||||
Формулировка «критичных проблем не обнаружено» не употребляется: `critical` в
|
||||
отчёте нет потому, что ни одна находка не собрала оракула на этот уровень, а не
|
||||
потому, что путей туда нет.
|
||||
Reference in New Issue
Block a user