diff --git a/docs/adr/ADR-2026-08-14-account-with-records-is-not-deleted.md b/docs/adr/ADR-2026-08-14-account-with-records-is-not-deleted.md new file mode 100644 index 0000000..758a1b2 --- /dev/null +++ b/docs/adr/ADR-2026-08-14-account-with-records-is-not-deleted.md @@ -0,0 +1,63 @@ +# ADR-2026-08-14. Учётная запись с записями не удаляется, и это осознанный тупик + +- **Дата:** 2026-08-14 +- **Источник:** [openspec/changes/archive/2026-08-14-record-ownership/design.md](../../openspec/changes/archive/2026-08-14-record-ownership/design.md), раздел `Open Questions` + +## Решение + +Удаление учётной записи, у которой остались задачи расшифровки либо файлы, +отвергается — отказом с названной причиной. Способа удалить записи в сервисе нет +вовсе, поэтому до задачи про удаление записи такая учётная запись не удаляется +никак: ни владельцем панели, ни самим человеком. + +Решение принято человеком на чекпоинте задачи `record-ownership` из трёх +предложенных способов. + +Дословно из источника: + +> **Что делать с записями удалённого пользователя?** Связь при выключенном +> каскаде снимает ссылку — записи остаются, но становятся ничьими и +> недостижимыми по API навсегда. Способы: запретить удаление учётной записи, пока +> у неё есть записи; держать рядом со связью неизменяемый снимок идентификатора; +> признать потерю ценой и записать её. + +## Почему + +Колонка владельца — связь с учётной записью, и каскадное удаление у неё +выключено: сервис объявлен архивом и молча удалить чужой архив не вправе. Одного +этого мало, и проверка по исходникам `pocketbase@v0.39.10` показала почему: при +выключенном каскаде хранилище **вынимает** идентификатор из поля связи и +сохраняет запись без проверок. Задачи остались бы на месте, но стали бы ничьими — +а ничья запись по правилу той же задачи не достаётся по API никому. Архив +человека исчезал бы молча, и восстановить владельца было бы нечем: прежнего +значения не остаётся нигде. + +Прежнее обоснование выбора связи вместо строки — «связь удержит целостность» — +было неверным, и это выяснилось на ревью дизайна. + +## Чем платим + +Владелец панели упирается в отказ, а выхода из него сегодня нет: удаление записи +приносит отдельная задача. Тупик назван прямо, а не обнаружен потом. + +Отказ обязан доезжать до спрашивающего: хранилище пропускает наружу только свою +ошибку роутера, а всякую другую подменяет сообщением про обязательную связь. +Подсказка эта ведущая — единственная обязательная связь у задачи это файл, — и +владелец панели, поверив ей, пошёл бы удалять записи руками, то есть делать ровно +то необратимое, ради предотвращения чего запрет и заведён. Это нашло ревью кода. + +## Что рассматривалось и отвергнуто + +- **Неизменяемый снимок идентификатора рядом со связью.** Пережил бы удаление, и + запись можно было бы вернуть человеку. Отвергнуто: владельцем становится любая + строка, и целостность, ради которой выбрана связь, теряется. +- **Признать потерю ценой и записать её.** Дешевле всего сегодня — удаления + пользователей в сервисе нет вовсе. Отвергнуто: архив, теряемый одной кнопкой в + панели, противоречит решению от 2026-08-11 о том, что сервис — архив. + +## Связанное + +Запрет ставит сама сборка хранилища, а не вызывающий: сборка, забывшая его +позвать, теряет защиту молча — и теряла, пока его добавляли отдельной строкой +запуска. Норма — `openspec/specs/storage`, «Учётная запись с записями не +удаляется». diff --git a/docs/adr/README.md b/docs/adr/README.md index 65a0d53..49d3a7d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -35,6 +35,7 @@ | Дата | Запись | Статус | | --- | --- | --- | +| 2026-08-14 | [Учётная запись с записями не удаляется, и это осознанный тупик](ADR-2026-08-14-account-with-records-is-not-deleted.md) | | | 2026-08-13 | [Намерение объявляется признаком, а не выводится из ключа доступа](ADR-2026-08-13-telegram-intent-declared-not-inferred.md) | | | 2026-08-13 | [Недоступность Telegram подъёму сервиса не мешает](ADR-2026-08-13-telegram-outage-does-not-block-startup.md) | | | 2026-08-12 | [Файл записи закрыт защищённым полем и отдаётся вошедшему по токену файла](ADR-2026-08-12-protected-file-behind-session.md) | | diff --git a/docs/architecture.md b/docs/architecture.md index 5a744ce..c2fa5cd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -35,8 +35,10 @@ - [access](../openspec/specs/access/spec.md) — кто пришёл в сервис и пускают ли его дальше: вход через внешнего провайдера OIDC, чем предъявляется сессия, что её прекращает и какие адреса остаются открытыми. Задача `oidc-login` - 2026-08-12. Разграничения записей по владельцу здесь нет: всякий вошедший - видит всё, что видел прежде аноним. + 2026-08-12. Здесь же разграничение записей по владельцу: запись из веба + принадлежит тому, кто её принёс, чужая неотличима от несуществующей, а запись + из Telegram владельца не имеет и по API не достаётся никому. Задача + `record-ownership` 2026-08-14. Поведение прочих узлов, включая приём из Telegram, по-прежнему живёт только в коде. Задача, которая его трогает, дописывает спеку своей capability. diff --git a/docs/database.md b/docs/database.md index 5707688..43e130e 100644 --- a/docs/database.md +++ b/docs/database.md @@ -45,6 +45,7 @@ Object Storage — каждая своей записью. | --- | --- | --- | | `id` | TEXT PK | Идентификатор записи, выдаёт хранилище | | `file` | file | Сам файл; пусто у копии в Object Storage | +| `owner` | relation → `users` | Владелец файла; пусто у файлов записи, принятой ботом | | `location` | select | `local` или `s3` | | `object_key` | TEXT | Ключ объекта; пусто у местной копии | | `size` | INTEGER | Размер в байтах | @@ -60,6 +61,7 @@ capability, и третий смысл развёл бы одно слово п | Поле | Тип | Что | | --- | --- | --- | | `id` | TEXT PK | Идентификатор записи, выдаёт хранилище | +| `owner` | relation → `users` | Владелец записи; пусто у записей, принятых ботом | | `state` | select | `created`, `converted`, `transcribe`, `done`, `failed`, `dead`; перечень закрыт схемой | | `source` | select | `api`, `telegram`, `unknown` | | `file` | relation → `files` | **Текущий** файл задачи: шаг конвейера переставляет ссылку на свой результат | @@ -83,8 +85,28 @@ capability, и третий смысл развёл бы одно слово п «мертва»». Схеме принадлежит только закрытость перечня: шестое состояние потребует нового шага. -**Правила доступа обеих коллекций пусты**, то есть перечислять и читать записи -может только владелец панели. Проверено прогоном: анонимный запрос к +**Владелец записи** заведён шагом `202608140001` — связью с коллекцией `users` в +обеих таблицах. Пустое значение допустимо, и это решение с ценой: записи, +принятые ботом, владельца не имеют вовсе, потому что связи чата Telegram с +учётной записью сервис не ведёт. Обязательность для приёма по HTTP держит +поэтому сам приём, а не схема. + +Выборка по владельцу сужает **чтение задачи**: чужая, ничья и несуществующая +дают один и тот же отказ. Выборку воркера владелец не сужает — конвейер +обрабатывает записи всех. Тот же шаг сужает правило просмотра коллекции +`files` владельцем: прежнее правило пускало всякого вошедшего, и знание +идентификатора файловой записи равнялось праву скачать чужое аудио. + +**Учётная запись с задачами не удаляется.** Каскадное удаление у связи выключено, +но одного этого мало: при выключенном каскаде хранилище снимает ссылку и +сохраняет запись без проверок — задачи остались бы, но стали бы ничьими, а ничья +задача не достаётся никому. Отказ ставит слой приложения `GuardOwnerDeletion`, +а не правило коллекции: панель ходит правами суперпользователя, и правило её не +судит. Цена названа прямо — владелец панели упирается в отказ, а удаления +записей в сервисе пока нет вовсе. + +**Правила доступа задач пусты**, то есть перечислять и читать их может только +владелец панели. Проверено прогоном: анонимный запрос к `/api/collections/*/records` отвечает `403`, к `/api/logs`, `/api/backups`, `/api/settings` и `/api/crons` — `401`. diff --git a/docs/passport.md b/docs/passport.md index bd8a9f0..e61ee9b 100644 --- a/docs/passport.md +++ b/docs/passport.md @@ -69,10 +69,10 @@ файл. Своей записи и работы без сети не делаем. - **Файловое хранилище общего назначения.** Храним аудио и видео, отданные ради речи в них. Складом произвольных файлов и папками сервис не становится. Общего - доступа к чужим записям целью тоже нет — но **сегодня он есть**: владельца у - записи в модели данных не существует, и всякий вошедший видит все записи - ([security.md](security.md), «Периметр»). Это состояние, а не решение; - закрывает его `record-ownership`. + доступа к чужим записям целью нет, и с 2026-08-14 его нет и на деле: у записи + есть владелец, и чужую по её идентификатору не отдают + ([security.md](security.md), «Периметр»). Закрыла это задача + `record-ownership`. - **Учёт денег.** Считаем объём, минуты и токены по каждому пользователю и показываем их владельцу. Цен, счетов и отказов по исчерпании квоты не делаем: пользователя, потратившего слишком много, останавливает разговор или отзыв @@ -100,8 +100,8 @@ получает идентификатор задачи и опрашивает `GET /api/status/:id`, пока не увидит `done` и текст. Сегодня доступно только предъявившему сессию OIDC: анонимный запрос обоими адресами отклоняется. Своего входа у программы нет — - его заводит `api-tokens`, — как нет и разграничения записей между - пользователями. + его заводит `api-tokens`. Записи при этом разграничены: программа с чужой + сессией видит только записи того, чью сессию предъявила. 6. **Отказ на середине.** Конвертация или распознавание не удались — задача переходит в `failed`, а пользователь получает сообщение о том, что именно не вышло, и предложение повторить. diff --git a/docs/research/pocketbase.md b/docs/research/pocketbase.md index a7e3dce..67fc157 100644 --- a/docs/research/pocketbase.md +++ b/docs/research/pocketbase.md @@ -186,3 +186,37 @@ pb_data/storage/<коллекция>/<запись>/<имя>_<10 случайн способов: вместе с панелью отказ выбрасывал бы уход CGO и встроенное резервное копирование, которых у сервиса-архива нет никаких. + +## Разграничение по владельцу: что выяснилось при реализации + +Дописано 2026-08-14 задачей `record-ownership`. Все находки ниже получены одним +способом: чтением исходников `pocketbase@v0.39.10` из кеша модулей и прогонами +против настоящего хранилища на временном каталоге — в ходе ревью того change. + +**Связь с выключенным каскадом не удерживает целостность при удалении.** +`core/record_model.go`, `deleteRefRecords`: при `CascadeDelete = false` и +необязательном поле хранилище **вынимает** идентификатор из поля связи и +сохраняет запись через `SaveNoValidate`. То есть «не уносить записи следом» и +«сохранить у них владельца» — разные вещи, и связь даёт только первое. + +**Наружу проходит только ошибка роутера.** `apis/record_crud.go` заворачивает +отказ хука в `firstApiError(err, e.BadRequestError("Failed to delete record. Make +sure that the record is not part of a required relation reference.", err))`, а +`firstApiError` берёт первый аргумент, только если он `*router.ApiError`. Обычная +ошибка из хука до ответа не доезжает вовсе, и спрашивающий получает библиотечную +подсказку про обязательную связь — в нашем случае указывающую не на ту связь. + +**`apis/file.go` выдаёт токен файла на предъявителя, а не на файл.** О файле при +выдаче он не спрашивает. Владельца судит переход по ссылке: правило просмотра +коллекции проверяет защищённое поле файла по учётной записи **из токена**. Значит +чужой токен получить можно всегда, а скачать по нему чужой файл — нет. + +**Проверка сессии с именем коллекции отвечает `403`, а не `401`.** +`apis.RequireAuth("users")` пускает только запись названной коллекции; предъявитель +из другой — например, владелец панели — узнан, но не годится, и код отказа это +различает. + +**Связь в SQLite лежит пустой строкой, а не `NULL`.** `RelationField.ColumnType` +даёт `TEXT DEFAULT '' NOT NULL`; сырой запрос и чтение через запись коллекции +совпадают побайтово. «Умолчания у колонки нет» верно по замыслу — пустое значение +не совпадает ни с кем, — но не буквально на уровне схемы. diff --git a/docs/review.md b/docs/review.md index d311a7b..98733c6 100644 --- a/docs/review.md +++ b/docs/review.md @@ -117,12 +117,17 @@ - **«Файлы и объекты не удаляются, диск растёт».** Факт верный и записан в [database.md](database.md); срок хранения не задан сознательно, задачи на него нет. Новой находкой это не считается, пока не измерен рост. -- **«У записи нет владельца: вошедший видит чужие записи».** Не дефект и не - новость: приём, опрос и файл закрыты сессией OIDC с 2026-08-12, а - разграничения по владельцу нет сознательно — [security.md](security.md), - «Периметр», и `openspec/specs/access`, «Purpose». Находкой считается новая - поверхность, выставленная наружу, либо путь к содержимому записи **без** - сессии, а не повторение этого факта. +- **«Запись без владельца не достаётся никому».** Не дефект: владельца не имеют + записи, принятые ботом, — связи чата Telegram с учётной записью приложения + сервис не ведёт, её заводит `telegram-account-link`. Ответ такой записи по API + совпадает с ответом на несуществующую, и это норма — расшифровку отправитель + получает в чат. + + **Прежняя редакция этой строки отменена 2026-08-14.** До задачи + `record-ownership` здесь стояло «вошедший видит чужие записи — не дефект и не + новость»: разграничения не было сознательно. Теперь оно есть, и такая находка + настоящая. Строка оставлена вместо удаления намеренно: прогон, помнящий её + прежний вид, выбросил бы регрессию не глядя. ### Вопросы по темам diff --git a/docs/security.md b/docs/security.md index ee818eb..7bff1c5 100644 --- a/docs/security.md +++ b/docs/security.md @@ -10,9 +10,13 @@ Целевой периметр добавляет к нему отдельный вход для программ по личным токенам и два уровня доступа — пользователь видит свои записи, владелец сервиса ещё и -страницу расхода. **Разграничения по владельцу нет:** всякий вошедший видит все -записи и все расшифровки, как видел их прежде аноним. Его заводит задача -`record-ownership`. +страницу расхода. **Разграничение по владельцу записи заведено 2026-08-14** +задачей `record-ownership`: и опрос готовности, и файл записи сужены владельцем +записи, а чужая отвечает «не найдено». Целевому периметру недостаёт теперь второго уровня +доступа — страницы расхода для владельца сервиса. + +Записи, принятые ботом, владельца не имеют и по API не достаются никому: связи +чата с учётной записью приложения нет, её заводит `telegram-account-link`. Разграничение доступа в Telegram осталось прежним — белым списком, и с учётной записью приложения он не связан. @@ -173,9 +177,9 @@ Telegram отправителю. прокси — работа выкладки, и сервис на неё не полагается: содержимого записей эти адреса не несут. -Владения записью в модели данных по-прежнему нет: у задачи нет пользователя. -Знание UUID задачи и есть право её читать — теперь для всякого вошедшего, а не -для всякого встречного. +Владение записью в модели данных появилось 2026-08-14: у задачи и у её файла +есть владелец. Знание идентификатора задачи правом её читать больше не является +— читает её тот, кто её принёс. Целевой периметр заводит четыре механизма вместо одного белого списка; первый из них уже стоит: @@ -183,7 +187,7 @@ Telegram отправителю. | Механизм | Что даёт | Чья задача | | --- | --- | --- | | Сессия OIDC у Authelia | Право открыть приложение и его эндпоинты — **сделано 2026-08-12** | `oidc-login` | -| Владелец у задачи и файла | Чужая запись по её идентификатору отвечает «не найдено» | `record-ownership` | +| Владелец у задачи и файла | Чужая запись по её идентификатору отвечает «не найдено» — **сделано 2026-08-14** | `record-ownership` | | Личный токен | Права своего владельца программе, без браузерной сессии | `api-tokens` | | Признак владельца сервиса | Страницу расхода и сводку по всем пользователям | `admin-stats-screen` | diff --git a/internal/adapter/repo/pocketbase/app.go b/internal/adapter/repo/pocketbase/app.go index ef51caf..165b211 100644 --- a/internal/adapter/repo/pocketbase/app.go +++ b/internal/adapter/repo/pocketbase/app.go @@ -43,6 +43,13 @@ func New(dataDir string) (*pb.PocketBase, error) { return nil, fmt.Errorf("failed to apply storage schema: %w", err) } + // Страж владельца вешается здесь, а не вызывающим: он защищает архив от + // удаления учётной записи, и сборка, забывшая его позвать, теряет защиту + // молча. Так это уже и было — окружение проверок его не ставило, и всё + // разграничение проверялось на приложении, где архив сносится одним + // запросом. + GuardOwnerDeletion(app) + return app, nil } diff --git a/internal/adapter/repo/pocketbase/file_repo.go b/internal/adapter/repo/pocketbase/file_repo.go index b5c3401..09d5f97 100644 --- a/internal/adapter/repo/pocketbase/file_repo.go +++ b/internal/adapter/repo/pocketbase/file_repo.go @@ -116,7 +116,7 @@ func (repo *FileRepository) Localize(fileID string) (contract.WorkFile, error) { // библиотеки строит его из имени, данного отправителем, а имя отправителя в // хранилище не попадает — путь к файлу читается в журнале, и инвариант // приватности этого не допускает. Свой суффикс хранилище допишет само. -func (repo *FileRepository) CreateLocal(name string, work contract.WorkFile) (*entity.File, error) { +func (repo *FileRepository) CreateLocal(name string, work contract.WorkFile, ownerID string) (*entity.File, error) { collection, err := findCollection(repo.app, migrations.FilesCollection) if err != nil { return nil, err @@ -132,6 +132,11 @@ func (repo *FileRepository) CreateLocal(name string, work contract.WorkFile) (*e record.Set("file", stored) record.Set("location", entity.LocationLocal) record.Set("size", stored.Size) + // Владелец файла — владелец записи, которой файл принадлежит. Пустой значит + // «файл без владельца»: таков всякий файл записи, принятой ботом. Правило + // просмотра коллекции сужено этой колонкой, и без неё чужое аудио осталось + // бы доступным всякому вошедшему. + record.Set("owner", ownerID) if err := repo.app.Save(record); err != nil { // Отказ укладки называет имя файла — то самое, из которого строится @@ -143,7 +148,7 @@ func (repo *FileRepository) CreateLocal(name string, work contract.WorkFile) (*e return recordToFile(record), nil } -func (repo *FileRepository) CreateRemote(objectKey string, size int64) (*entity.File, error) { +func (repo *FileRepository) CreateRemote(objectKey string, size int64, ownerID string) (*entity.File, error) { collection, err := findCollection(repo.app, migrations.FilesCollection) if err != nil { return nil, err @@ -153,6 +158,7 @@ func (repo *FileRepository) CreateRemote(objectKey string, size int64) (*entity. record.Set("location", entity.LocationS3) record.Set("object_key", objectKey) record.Set("size", size) + record.Set("owner", ownerID) if err := repo.app.Save(record); err != nil { return nil, fmt.Errorf("failed to store remote file record: %w", err) diff --git a/internal/adapter/repo/pocketbase/file_repo_test.go b/internal/adapter/repo/pocketbase/file_repo_test.go index 325a413..ff82c70 100644 --- a/internal/adapter/repo/pocketbase/file_repo_test.go +++ b/internal/adapter/repo/pocketbase/file_repo_test.go @@ -31,7 +31,7 @@ func TestCreateLocal_AcceptsRecordLargerThanLibraryDefault(t *testing.T) { require.NoError(t, err) require.Greater(t, size, int64(libraryDefault), "запись заведомо больше умолчания библиотеки") - file, err := repo.CreateLocal("big.mp3", work) + file, err := repo.CreateLocal("big.mp3", work, "") require.NoError(t, err, "запись длиннее умолчания библиотеки ложится в хранилище") assert.Equal(t, size, file.Size) assert.Greater(t, entity.MaxRecordSize, size, "объявленный потолок выше проверяемого размера") diff --git a/internal/adapter/repo/pocketbase/job_mapping.go b/internal/adapter/repo/pocketbase/job_mapping.go index b987438..eb3e6ab 100644 --- a/internal/adapter/repo/pocketbase/job_mapping.go +++ b/internal/adapter/repo/pocketbase/job_mapping.go @@ -39,6 +39,10 @@ func applyOwnedByPipeline(record *core.Record, job *entity.TranscribeJob) { // поля здесь не с кем. func applyToRecord(record *core.Record, job *entity.TranscribeJob) { applyOwnedByPipeline(record, job) + // Владелец кладётся только здесь, при заведении. В applyOwnedByPipeline его + // нет намеренно: конвейер владельца не назначает и не меняет, а снимок шага, + // записанный поверх, стёр бы его молча. + record.Set("owner", derefString(job.OwnerID)) record.Set("source", job.Source) record.Set("tg_chat_id", derefInt64(job.TgChatId)) record.Set("tg_reply_message_id", derefInt(job.TgReplyMessageId)) @@ -48,6 +52,7 @@ func recordToJob(record *core.Record) *entity.TranscribeJob { return &entity.TranscribeJob{ Id: record.Id, State: record.GetString("state"), + OwnerID: nilIfEmpty(record.GetString("owner")), Source: record.GetString("source"), FileID: nilIfEmpty(record.GetString("file")), ErrorText: nilIfEmpty(record.GetString("error_text")), @@ -69,8 +74,13 @@ func recordToJob(record *core.Record) *entity.TranscribeJob { // перечнем держит константа acquireColumns и тест захвата, читающий задачу // целиком. type acquiredRow struct { - Id string `db:"id"` - State string `db:"state"` + Id string `db:"id"` + State string `db:"state"` + // Владелец конвейеру не нужен для выборки — она им не сужается, — но + // читается: снимок задачи, в котором владелец всегда пуст, был бы ловушкой + // для первого же шага, начавшего сохранять задачу целиком. Сегодня это ещё + // и несущее чтение: шаг конвейера кладёт владельца на файл, который заводит. + OwnerID sql.NullString `db:"owner"` Source string `db:"source"` FileID sql.NullString `db:"file"` ErrorText sql.NullString `db:"error_text"` @@ -90,6 +100,7 @@ func (r *acquiredRow) toJob() *entity.TranscribeJob { job := &entity.TranscribeJob{ Id: r.Id, State: r.State, + OwnerID: nullToPtr(r.OwnerID), Source: r.Source, FileID: nullToPtr(r.FileID), ErrorText: nullToPtr(r.ErrorText), diff --git a/internal/adapter/repo/pocketbase/migrations/202608140001_record_owner.go b/internal/adapter/repo/pocketbase/migrations/202608140001_record_owner.go new file mode 100644 index 0000000..474803a --- /dev/null +++ b/internal/adapter/repo/pocketbase/migrations/202608140001_record_owner.go @@ -0,0 +1,106 @@ +package migrations + +import ( + "errors" + "fmt" + + "github.com/pocketbase/pocketbase/core" +) + +// up202608140001 заводит владельца записи. +// +// Колонка — связь с коллекцией пользователей: хранилище само следит, чтобы +// владельцем стояла существующая учётная запись, а не строка, похожая на её +// идентификатор. +// +// Пустое значение допустимо, и это решение с названной ценой. Записи, принятые +// ботом, владельца не имеют вовсе: связи чата Telegram с учётной записью сервис +// не ведёт, её заводит отдельная задача. Обязательность для приёма по HTTP +// держит поэтому сам приём, а не схема. +// +// Каскадное удаление выключено, но одного этого мало: при выключенном каскаде +// хранилище **вынимает** идентификатор из поля связи и сохраняет запись без +// проверок, то есть архив удалённого пользователя стал бы ничьим и не достался +// бы никому. Поэтому удаление учётной записи, у которой остались задачи, +// отвергается слоем приложения — `GuardOwnerDeletion`. +func up202608140001(app core.App) error { + users, err := app.FindCollectionByNameOrId(UsersCollection) + if err != nil { + return fmt.Errorf("failed to find users collection: %w", err) + } + + jobs, err := app.FindCollectionByNameOrId(JobsCollection) + if err != nil { + return fmt.Errorf("failed to find jobs collection: %w", err) + } + + jobs.Fields.Add(ownerField(users.Id)) + + if err := app.Save(jobs); err != nil { + return fmt.Errorf("failed to add owner to jobs: %w", err) + } + + files, err := app.FindCollectionByNameOrId(FilesCollection) + if err != nil { + return fmt.Errorf("failed to find files collection: %w", err) + } + + files.Fields.Add(ownerField(users.Id)) + + // Правило просмотра сужается владельцем. Прежнее пускало всякого узнанного: + // владельца у записи тогда не было, и сужать выборку было нечем. Без этой + // строки разграничение закрыло бы метаданные задачи и оставило открытым + // содержимое — то самое, что оно и заведено прятать: знание идентификатора + // файловой записи равнялось бы праву скачать чужое аудио. + files.ViewRule = ptr(`@request.auth.id != "" && owner = @request.auth.id`) + + if err := app.Save(files); err != nil { + return fmt.Errorf("failed to narrow files by owner: %w", err) + } + + return nil +} + +// ownerField собирает описание колонки владельца. Обе коллекции получают +// одинаковую: разойдясь, они дали бы разное поведение у задачи и у её файла. +func ownerField(usersCollectionID string) *core.RelationField { + return &core.RelationField{ + Name: "owner", + CollectionId: usersCollectionID, + MaxSelect: 1, + // Пустое значение допустимо — см. шапку шага. Умолчания у колонки нет: + // связь его не имеет по устройству, и запись не достаётся никому по + // недосмотру схемы. + Required: false, + // Удаление учётной записи не уносит её записи следом: сервис объявлен + // архивом. Что происходит вместо этого, держит `GuardOwnerDeletion`. + CascadeDelete: false, + } +} + +// down202608140001 снимает колонку с обеих коллекций и возвращает правило +// просмотра файлов к тому, что стояло до шага, — «всякий узнанный». +func down202608140001(app core.App) error { + for _, name := range []string{JobsCollection, FilesCollection} { + collection, err := app.FindCollectionByNameOrId(name) + if err != nil { + return fmt.Errorf("failed to find collection %s: %w", name, err) + } + + field := collection.Fields.GetByName("owner") + if field == nil { + return errors.New("collection " + name + " has no owner field") + } + collection.Fields.RemoveById(field.GetId()) + + if name == FilesCollection { + collection.ViewRule = ptr(`@request.auth.id != ""`) + } + + if err := app.Save(collection); err != nil { + return fmt.Errorf("failed to drop owner from %s: %w", name, err) + } + } + + return nil +} diff --git a/internal/adapter/repo/pocketbase/migrations/migrations.go b/internal/adapter/repo/pocketbase/migrations/migrations.go index b9b152a..20c1596 100644 --- a/internal/adapter/repo/pocketbase/migrations/migrations.go +++ b/internal/adapter/repo/pocketbase/migrations/migrations.go @@ -24,6 +24,10 @@ import ( const ( FilesCollection = "files" JobsCollection = "transcribe_jobs" + // UsersCollection заводит не наш шаг, а системный шаг библиотеки. Имя стоит + // здесь потому, что на него ссылаются и шаги схемы, и проверка предъявителя + // на приёме: строковый литерал в двух местах разошёлся бы молча. + UsersCollection = "users" ) // Шаг регистрируется в списке приложения при загрузке пакета, а накатывает его @@ -31,6 +35,7 @@ const ( func init() { pbmigrations.Register(up202608110001, down202608110001, "202608110001_init.go") pbmigrations.Register(up202608120001, down202608120001, "202608120001_oidc_login.go") + pbmigrations.Register(up202608140001, down202608140001, "202608140001_record_owner.go") } func ptr[T any](v T) *T { return &v } diff --git a/internal/adapter/repo/pocketbase/owner_guard.go b/internal/adapter/repo/pocketbase/owner_guard.go new file mode 100644 index 0000000..31b34b5 --- /dev/null +++ b/internal/adapter/repo/pocketbase/owner_guard.go @@ -0,0 +1,80 @@ +package pocketbase + +import ( + "fmt" + + "github.com/pocketbase/dbx" + "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/tools/router" + + "git.vakhrushev.me/av/transcriber/internal/adapter/repo/pocketbase/migrations" +) + +// GuardOwnerDeletion отвергает удаление учётной записи, у которой остались +// задачи расшифровки. +// +// Колонка владельца — связь с выключенным каскадным удалением, и одного этого +// мало: при выключенном каскаде хранилище не удаляет ссылающуюся запись, а +// **вынимает** идентификатор из поля связи и сохраняет её без проверок. Задачи +// остались бы на месте, но стали бы ничьими, а ничья задача не достаётся по API +// никому — архив человека исчез бы молча и восстановлению не подлежал: +// прежнего владельца не остаётся нигде. +// +// Цена запрета названа прямо: владелец панели упирается в отказ, а способа +// удалить записи в сервисе пока нет вовсе — его приносит отдельная задача. До +// неё удаление учётной записи с записями невозможно, и это осознанный тупик. +// +// Слой стоит на удалении записи, а не на запросе к панели: панель ходит правами +// суперпользователя, и правило коллекции её не судит. Удаление при этом не +// только панельное — умолчание библиотеки разрешает вошедшему удалить свою +// учётную запись запросом, так что страж закрывает и публичную поверхность. +// +// Считаются **обе** коллекции с владельцем. Файл переживает свою задачу: шаг +// конвейера заводит его до сохранения задачи, и потерянный захват оставляет файл +// с владельцем и без ссылки. Учётная запись, у которой остались одни такие +// файлы, без этого счёта удалялась бы штатно, а аудио становилось бы ничьим. +func GuardOwnerDeletion(app core.App) { + app.OnRecordDelete(migrations.UsersCollection).BindFunc(func(e *core.RecordEvent) error { + count, err := countOwned(e.App, e.Record.Id) + if err != nil { + return err + } + + if count > 0 { + // Отказ отдаётся ошибкой роутера, а не обычной: библиотека пропускает + // наружу только `*router.ApiError`, а всякую другую подменяет своим + // сообщением — «убедитесь, что запись не участвует в обязательной + // связи». Подсказка эта не просто бесполезная, а **ведущая**: + // единственная обязательная связь у задачи — файл, и владелец панели, + // поверив ей, пойдёт удалять задачи и файлы руками. То есть сделает + // ровно то необратимое, ради предотвращения чего страж и заведён. + // + // Число в отказе — не содержимое записей, а их счёт: он говорит + // владельцу панели, почему удаление не прошло, и не выносит наружу + // ничего о самих записях. + return router.NewBadRequestError(fmt.Sprintf( + "у учётной записи остались записи (%d): сервис — архив, и удаление сделало бы их ничьими", + count, + ), nil) + } + + return e.Next() + }) +} + +// countOwned считает всё, что принадлежит учётной записи, — по обеим коллекциям +// с колонкой владельца. Перечень живёт здесь одним списком: разойдясь с шагом +// схемы, он оставил бы половину архива без защиты молча. +func countOwned(app core.App, ownerID string) (int64, error) { + var total int64 + + for _, collection := range []string{migrations.JobsCollection, migrations.FilesCollection} { + count, err := app.CountRecords(collection, dbx.HashExp{"owner": ownerID}) + if err != nil { + return 0, fmt.Errorf("failed to count owned records in %s: %w", collection, err) + } + total += count + } + + return total, nil +} diff --git a/internal/adapter/repo/pocketbase/owner_test.go b/internal/adapter/repo/pocketbase/owner_test.go new file mode 100644 index 0000000..31c1ab6 --- /dev/null +++ b/internal/adapter/repo/pocketbase/owner_test.go @@ -0,0 +1,182 @@ +package pocketbase + +import ( + "strings" + "testing" + + "github.com/pocketbase/pocketbase/core" + "github.com/pocketbase/pocketbase/tools/router" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "git.vakhrushev.me/av/transcriber/internal/adapter/repo/pocketbase/migrations" + "git.vakhrushev.me/av/transcriber/internal/contract" + "git.vakhrushev.me/av/transcriber/internal/entity" +) + +// newAccount заводит учётную запись и отдаёт её идентификатор. +func newAccount(t *testing.T, app core.App, email string) string { + t.Helper() + + users, err := app.FindCollectionByNameOrId(migrations.UsersCollection) + require.NoError(t, err) + + record := core.NewRecord(users) + record.Set("email", email) + record.Set("verified", true) + record.SetRandomPassword() + require.NoError(t, app.Save(record)) + + return record.Id +} + +// Колонка владельца заводится шагом схемы на чистой базе, и умолчания у неё нет. +func TestOwnerColumnHasNoDefault(t *testing.T) { + app := newTestApp(t) + + for _, name := range []string{migrations.JobsCollection, migrations.FilesCollection} { + collection, err := app.FindCollectionByNameOrId(name) + require.NoError(t, err) + + field := collection.Fields.GetByName("owner") + require.NotNil(t, field, "колонка владельца заведена в %s", name) + + relation, ok := field.(*core.RelationField) + require.True(t, ok, "владелец — связь с учётной записью, а не строка") + assert.False(t, relation.Required, "пустое значение допустимо ради записей бота") + assert.False(t, relation.CascadeDelete, "удаление учётной записи не уносит записи") + + // Умолчания у связи нет по устройству: запись, чей владелец не назван, + // не достаётся никому по недосмотру схемы. + record := core.NewRecord(collection) + assert.Empty(t, record.GetString("owner"), "новая запись приходит без владельца") + } +} + +// Чужая задача, ничья и несуществующая дают одну и ту же ошибку. +func TestGetByID_NarrowedByOwner(t *testing.T) { + app := newTestApp(t) + repo := NewTranscriptJobRepository(app) + + mine := newAccount(t, app, "mine@example.com") + stranger := newAccount(t, app, "stranger@example.com") + + owned := newJob(t, repo, entity.StateCreated) + owned.OwnerID = &mine + require.NoError(t, repo.Save(owned, "")) + // Владельца кладёт заведение, а не сохранение конвейера, — ставим его прямо. + record, err := app.FindRecordById(migrations.JobsCollection, owned.Id) + require.NoError(t, err) + record.Set("owner", mine) + require.NoError(t, app.Save(record)) + + ownerless := newJob(t, repo, entity.StateCreated) + + got, err := repo.GetByID(owned.Id, mine) + require.NoError(t, err, "своя задача отдаётся") + assert.Equal(t, owned.Id, got.Id) + + _, foreignErr := repo.GetByID(owned.Id, stranger) + _, ownerlessErr := repo.GetByID(ownerless.Id, mine) + _, missingErr := repo.GetByID("nonexistent0000", mine) + + var notFound *contract.JobNotFoundError + require.ErrorAs(t, foreignErr, ¬Found, "чужая задача не отдаётся") + require.ErrorAs(t, ownerlessErr, ¬Found, "ничья задача не отдаётся") + require.ErrorAs(t, missingErr, ¬Found, "несуществующая тоже") +} + +// Пустой владелец не совпадает ни с чем: ни со своей задачей, ни с чужой, ни с +// ничьей. Правило записано со стороны спрашивающего — обязательность, которую +// держит одна лишь подпись метода, пустую строку пропускает. +func TestGetByID_EmptyOwnerMatchesNothing(t *testing.T) { + app := newTestApp(t) + repo := NewTranscriptJobRepository(app) + + owner := newAccount(t, app, "mine@example.com") + + owned := newJob(t, repo, entity.StateCreated) + record, err := app.FindRecordById(migrations.JobsCollection, owned.Id) + require.NoError(t, err) + record.Set("owner", owner) + require.NoError(t, app.Save(record)) + + ownerless := newJob(t, repo, entity.StateCreated) + + var notFound *contract.JobNotFoundError + for _, id := range []string{owned.Id, ownerless.Id, "nonexistent0000"} { + _, err := repo.GetByID(id, "") + require.ErrorAs(t, err, ¬Found, "пустой владелец не открывает %s", id) + } +} + +// Удаление учётной записи с задачами отвергается: связь с выключенным каскадом +// иначе снимает ссылку, и архив человека становится ничьим и недостижимым. +func TestGuardOwnerDeletion(t *testing.T) { + // Страж вешает сама сборка хранилища — звать его отдельно не нужно и нельзя: + // второй вызов повесил бы второй слой. + app := newTestApp(t) + repo := NewTranscriptJobRepository(app) + + withJobs := newAccount(t, app, "keeper@example.com") + empty := newAccount(t, app, "empty@example.com") + + job := newJob(t, repo, entity.StateCreated) + record, err := app.FindRecordById(migrations.JobsCollection, job.Id) + require.NoError(t, err) + record.Set("owner", withJobs) + require.NoError(t, app.Save(record)) + + keeper, err := app.FindRecordById(migrations.UsersCollection, withJobs) + require.NoError(t, err) + + err = app.Delete(keeper) + require.Error(t, err, "учётная запись с задачами не удаляется") + + // Отказ обязан быть ошибкой роутера, а не обычной: наружу библиотека + // пропускает только её, а всякую другую подменяет своим сообщением про + // обязательную связь — подсказкой, по которой владелец панели пойдёт удалять + // задачи и файлы руками. Проверяется поэтому тип и текст, а не сам факт + // отказа: на потерянном сообщении факт остаётся прежним. + var apiErr *router.ApiError + require.ErrorAs(t, err, &apiErr, "отказ доезжает до владельца панели") + assert.Contains(t, apiErr.Message, "остались записи", + "причина названа, а не подменена библиотечной") + + // Задача и её владелец остались прежними. + after, err := app.FindRecordById(migrations.JobsCollection, job.Id) + require.NoError(t, err) + assert.Equal(t, withJobs, after.GetString("owner"), "владелец не снят") + + free, err := app.FindRecordById(migrations.UsersCollection, empty) + require.NoError(t, err) + assert.NoError(t, app.Delete(free), "учётная запись без записей удаляется") +} + +// Файл переживает свою задачу: шаг конвейера заводит его до сохранения задачи, и +// потерянный захват оставляет файл с владельцем и без ссылки. Считать одни +// задачи значило бы отдать такое аудио на молчаливое обезличивание. +func TestGuardOwnerDeletion_CountsFilesToo(t *testing.T) { + app := newTestApp(t) + repo := NewFileRepository(app) + + owner := newAccount(t, app, "files@example.com") + + work, err := repo.Stage(".ogg", strings.NewReader("запись")) + require.NoError(t, err) + defer func() { require.NoError(t, work.Close()) }() + + _, err = repo.CreateLocal("sample.ogg", work, owner) + require.NoError(t, err) + + record, err := app.FindRecordById(migrations.UsersCollection, owner) + require.NoError(t, err) + + err = app.Delete(record) + require.Error(t, err, "учётная запись с одними файлами тоже не удаляется") + + after, err := app.FindAllRecords(migrations.FilesCollection) + require.NoError(t, err) + require.Len(t, after, 1) + assert.Equal(t, owner, after[0].GetString("owner"), "владелец файла не снят") +} diff --git a/internal/adapter/repo/pocketbase/transcript_job_repo.go b/internal/adapter/repo/pocketbase/transcript_job_repo.go index b24dece..6c8d259 100644 --- a/internal/adapter/repo/pocketbase/transcript_job_repo.go +++ b/internal/adapter/repo/pocketbase/transcript_job_repo.go @@ -80,17 +80,49 @@ func (repo *TranscriptJobRepository) Save(job *entity.TranscribeJob, holder stri return nil } -func (repo *TranscriptJobRepository) GetByID(id string) (*entity.TranscribeJob, error) { +// GetByID отдаёт задачу, только если её владелец — ownerID. +// +// Чужая задача, задача без владельца и несуществующая дают одну и ту же ошибку: +// по разнице ответов иначе перебирается список заведённых задач, а +// идентификатор задачи и есть то, что разграничение прячет. +// +// Пустой ownerID отсекается **до** чтения и не совпадает ни с чем: иначе +// вызывающий без учётной записи получил бы ровно множество задач без владельца, +// то есть все записи бота. +// +// Владелец сверяется тем же чтением, каким берётся состояние, а не отдельным +// запросом: между двумя чтениями задача успевает измениться, и ответ перестаёт +// быть функцией от того, что с ней произошло. +func (repo *TranscriptJobRepository) GetByID(id, ownerID string) (*entity.TranscribeJob, error) { + if ownerID == "" { + return nil, &contract.JobNotFoundError{Message: "job not found"} + } + record, err := repo.app.FindRecordById(migrations.JobsCollection, id) if err != nil { + // «Такой записи нет» переводится в доменную ошибку **здесь**, у + // источника, как велит конвенция об ошибках. Иначе три исхода, которые + // разграничение обязано сделать неразличимыми, разъезжаются: чужая и + // ничья задачи дают доменную ошибку, а несуществующая — отказ базы, + // неотличимый от настоящей аварии хранилища. Держалась бы эта + // неразличимость только тем, что транспорт кладёт в `404` любую ошибку, + // — то есть ровно тем расхождением, которое конвенция велит закрыть. + if errors.Is(err, sql.ErrNoRows) { + return nil, &contract.JobNotFoundError{Message: "job not found"} + } return nil, fmt.Errorf("failed to get transcribe job: %w", err) } + + if record.GetString("owner") != ownerID { + return nil, &contract.JobNotFoundError{Message: "job not found"} + } + return recordToJob(record), nil } // Колонки, которые читает захват. Список нужен запросу дословно: `RETURNING *` // отдал бы и порядок, зависящий от схемы. -const acquireColumns = `id, state, source, file, error_text, acquisition_id, ` + +const acquireColumns = `id, state, owner, source, file, error_text, acquisition_id, ` + `acquire_time, delay_time, attempts, recognition_op_id, transcription_text, ` + `tg_chat_id, tg_reply_message_id, created, updated` diff --git a/internal/adapter/repo/pocketbase/transcript_job_repo_test.go b/internal/adapter/repo/pocketbase/transcript_job_repo_test.go index a3eb1a2..e26b5ac 100644 --- a/internal/adapter/repo/pocketbase/transcript_job_repo_test.go +++ b/internal/adapter/repo/pocketbase/transcript_job_repo_test.go @@ -45,7 +45,7 @@ func newFile(t *testing.T, app core.App) *entity.File { require.NoError(t, err) defer func() { require.NoError(t, work.Close()) }() - file, err := repo.CreateLocal("sample.mp3", work) + file, err := repo.CreateLocal("sample.mp3", work, "") require.NoError(t, err) return file } @@ -246,7 +246,7 @@ func TestSave_RefusesWriteFromLostAcquisition(t *testing.T) { require.ErrorAs(t, err, &lost) // И состояние не поехало. - after, err := repo.GetByID(mine.Id) + after, err := readJobByID(t, app, mine.Id) require.NoError(t, err) assert.Equal(t, entity.StateCreated, after.State) } @@ -261,7 +261,7 @@ func TestSave_WithoutHolderWritesAnyway(t *testing.T) { require.NoError(t, repo.Save(job, "")) - after, err := repo.GetByID(job.Id) + after, err := readJobByID(t, app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateConverted, after.State) } @@ -291,7 +291,7 @@ func TestPanelRules_StateChangeByRequestClearsAcquisition(t *testing.T) { // запросом к записи, а не сохранением из кода. patchRecord(t, app, job.Id, `{"state":"`+entity.StateCreated+`"}`) - after, err := repo.GetByID(job.Id) + after, err := readJobByID(t, app, job.Id) require.NoError(t, err) assert.Nil(t, after.AcquisitionID, "признак захвата снят") assert.Nil(t, after.AcquireTime, "время захвата снято") @@ -323,7 +323,7 @@ func TestPanelRules_DoNotTouchPipelineWrites(t *testing.T) { acquired.MoveToStateAndDelay(entity.StateTranscribe, &delay) require.NoError(t, repo.Save(acquired, "holder")) - after, err := repo.GetByID(job.Id) + after, err := readJobByID(t, app, job.Id) require.NoError(t, err) require.NotNil(t, after.DelayTime, "задержка, поставленная шагом, пережила сохранение") @@ -333,7 +333,7 @@ func TestPanelRules_DoNotTouchPipelineWrites(t *testing.T) { after.Die("attempts exhausted: 6") require.NoError(t, repo.Save(after, "")) - dead, err := repo.GetByID(job.Id) + dead, err := readJobByID(t, app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDead, dead.State) assert.Equal(t, 6, dead.Attempts, "число попыток мёртвой задачи сохранено") @@ -404,9 +404,22 @@ func TestSave_KeepsOwnerEditMadeWhileStepHeldTheJob(t *testing.T) { acquired.MoveToState(entity.StateConverted) require.NoError(t, repo.Save(acquired, "holder")) - after, err := repo.GetByID(job.Id) + after, err := readJobByID(t, app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateConverted, after.State, "шаг свой результат записал") require.NotNil(t, after.TgChatId) assert.Equal(t, int64(999999), *after.TgChatId, "правка владельца пережила сохранение шага") } + +// readJobByID читает задачу мимо сужения владельцем: проверки хранилища смотрят +// задачи без владельца, а читающий метод их не отдаёт никому. Отображение при +// этом то же самое — сверяется именно оно. +func readJobByID(t *testing.T, app core.App, id string) (*entity.TranscribeJob, error) { + t.Helper() + + record, err := app.FindRecordById(migrations.JobsCollection, id) + if err != nil { + return nil, err + } + return recordToJob(record), nil +} diff --git a/internal/contract/error.go b/internal/contract/error.go index 67af32f..747853f 100644 --- a/internal/contract/error.go +++ b/internal/contract/error.go @@ -14,6 +14,11 @@ import ( // и запись о недоставке делает шаг, у которого задача под рукой. var ErrDeliveryChannelDown = errors.New("delivery channel is down") +// ErrOwnerRequired — приём по HTTP дошёл до заведения задачи, а владельца ему не +// назвали. Значение сентинельное: нести отказу нечего, а имя учётной записи в +// него не кладётся никогда. +var ErrOwnerRequired = errors.New("owner is required to accept a record") + type JobNotFoundError struct { State string Message string diff --git a/internal/contract/repository.go b/internal/contract/repository.go index e0f387a..e6384c5 100644 --- a/internal/contract/repository.go +++ b/internal/contract/repository.go @@ -36,9 +36,15 @@ type FileRepository interface { // CreateLocal кладёт рабочую копию в хранилище под именем name и заводит // запись о файле. Имя задаёт сервис: умолчание хранилища, строящее его из // имени отправителя, не применяется. - CreateLocal(name string, work WorkFile) (*entity.File, error) + // + // ownerID — владелец записи, которой файл принадлежит; пустой значит «файл + // без владельца», и таков всякий файл записи, принятой ботом. Владелец + // лежит своей колонкой, а не выводится через задачу: ссылку на файл в + // задаче переставляет каждый шаг конвейера, и исходная копия после + // конвертации не связана с задачей ничем. + CreateLocal(name string, work WorkFile, ownerID string) (*entity.File, error) // CreateRemote заводит запись о копии, лежащей во внешнем хранилище. - CreateRemote(objectKey string, size int64) (*entity.File, error) + CreateRemote(objectKey string, size int64, ownerID string) (*entity.File, error) GetByID(id string) (*entity.File, error) // Open отдаёт содержимое хранимого файла потоком. Open(fileID string) (io.ReadCloser, error) @@ -51,7 +57,19 @@ type TranscriptJobRepository interface { // Пустой holder снимает эту условность и в конвейере не употребляется: все // его шаги получают признак захвата от FindAndAcquire. Save(job *entity.TranscribeJob, holder string) error - GetByID(id string) (*entity.TranscribeJob, error) + // GetByID отдаёт задачу, только если её владелец — ownerID. Чужая задача, + // ничья задача и несуществующая дают одну и ту же ошибку: по разнице + // ответов иначе перебирается список заведённых задач. + // + // Владелец здесь обязателен, и пустой ownerID не совпадает ни с чем — + // включая задачи без владельца. Правило записано со стороны спрашивающего: + // обязательность, которую держит одна лишь подпись метода, пустую строку + // пропускает, и вызывающий без учётной записи получил бы ровно множество + // записей бота. + // + // Второго читающего метода нет намеренно: он выбирался бы по + // внимательности вызывающего. + GetByID(id, ownerID string) (*entity.TranscribeJob, error) // FindAndAcquire забирает задачу одним неделимым шагом и увеличивает число // её попыток. Работы в состоянии нет — JobNotFoundError. FindAndAcquire(state, acquisitionId string, rottingTime time.Time) (*entity.TranscribeJob, error) diff --git a/internal/controller/http/ownership_test.go b/internal/controller/http/ownership_test.go new file mode 100644 index 0000000..22f4390 --- /dev/null +++ b/internal/controller/http/ownership_test.go @@ -0,0 +1,228 @@ +package http + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/pocketbase/pocketbase/core" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + pbrepo "git.vakhrushev.me/av/transcriber/internal/adapter/repo/pocketbase" + "git.vakhrushev.me/av/transcriber/internal/entity" +) + +// Проверки разграничения записей по владельцу. Все идут через собранный роутер: +// сужение живёт в хранилище, но судится по тому, что видит отправитель. + +// newSecondAccount заводит вторую учётную запись с собственной сессией. +// Постоянный адрес почты первой занят, и повторное сохранение отвергается — +// адрес здесь свой. +func newSecondAccount(t *testing.T, app core.App) (*core.Record, string) { + t.Helper() + + users, err := app.FindCollectionByNameOrId("users") + require.NoError(t, err) + + record := core.NewRecord(users) + record.Set("email", "stranger@example.com") + record.Set("verified", true) + record.SetRandomPassword() + require.NoError(t, app.Save(record)) + + token, err := record.NewAuthToken() + require.NoError(t, err) + + return record, token +} + +// withSessionHeader предъявляет сессию заголовком. Собственная поверхность +// хранилища читается только так: слой, перекладывающий куку в заголовок, на неё +// намеренно не наведён — часть её защищена ровно тем, что браузер заголовка сам +// не шлёт. +func withSessionHeader(req *http.Request, session string) *http.Request { + req.Header.Set("Authorization", session) + return req +} + +// serveAs шлёт запрос от имени названной сессии, а не сессии окружения. +func serveAs(env *testEnv, session string, w http.ResponseWriter, req *http.Request) { + req.AddCookie(&http.Cookie{Name: SessionCookieName, Value: session}) + env.mux.ServeHTTP(w, req) +} + +// Чужая задача неотличима от несуществующей: тот же код и то же тело. Разница +// ответов обратила бы опрос в перебор — по ней считывается, какие задачи +// заведены. +func TestGetTranscribeJobStatus_ForeignJobLooksMissing(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + job := jobWithFile(t, env) + _, stranger := newSecondAccount(t, env.app) + + foreign := httptest.NewRecorder() + serveAs(env, stranger, foreign, httptest.NewRequest("GET", "/api/status/"+job.Id, http.NoBody)) + + unknown := httptest.NewRecorder() + serveAs(env, stranger, unknown, httptest.NewRequest("GET", "/api/status/unknown0000000000", http.NoBody)) + + require.Equal(t, http.StatusNotFound, foreign.Code, "чужая задача не отдаётся") + assert.Equal(t, unknown.Code, foreign.Code, "код тот же, что у неизвестного идентификатора") + assert.JSONEq(t, unknown.Body.String(), foreign.Body.String(), "и тело то же") + + // Ни состояния, ни текста расшифровки в теле нет. + assert.NotContains(t, foreign.Body.String(), entity.StateCreated) + assert.NotContains(t, foreign.Body.String(), "transcription_text") +} + +// Задача, принятая ботом, владельца не имеет и не достаётся по API никому. +func TestGetTranscribeJobStatus_OwnerlessJobLooksMissing(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + fileRepo := pbrepo.NewFileRepository(env.app) + work, err := fileRepo.Stage(".ogg", strings.NewReader("запись")) + require.NoError(t, err) + defer func() { require.NoError(t, work.Close()) }() + + // Файл записи из Telegram владельца тоже не имеет. + file, err := fileRepo.CreateLocal("voice.ogg", work, "") + require.NoError(t, err) + + job := &entity.TranscribeJob{ + State: entity.StateCreated, + Source: entity.SourceTelegram, + FileID: &file.Id, + } + require.NoError(t, env.handler.jobRepo.Create(job)) + + w := httptest.NewRecorder() + env.serve(w, httptest.NewRequest("GET", "/api/status/"+job.Id, http.NoBody)) + + assert.Equal(t, http.StatusNotFound, w.Code) +} + +// Владельцем принятой записи становится предъявитель сессии — и у задачи, и у +// её файла. +func TestCreateTranscribeJob_OwnerIsSession(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + w := httptest.NewRecorder() + env.serve(w, createMultipartRequest(t, "sample.mp3", []byte("запись"))) + require.Equal(t, http.StatusCreated, w.Code) + + var response CreateTranscribeJobResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) + + record, err := env.app.FindRecordById("transcribe_jobs", response.JobID) + require.NoError(t, err) + assert.Equal(t, env.account.Id, record.GetString("owner"), "владелец задачи — предъявитель") + + fileRecord, err := env.app.FindRecordById("files", record.GetString("file")) + require.NoError(t, err) + assert.Equal(t, env.account.Id, fileRecord.GetString("owner"), "владелец файла — он же") +} + +// Владельца не задают запросом: своё значение в форме на результат не влияет. +func TestCreateTranscribeJob_OwnerFieldFromRequestIgnored(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + _, stranger := newSecondAccount(t, env.app) + + req := createMultipartRequest(t, "sample.mp3", []byte("запись")) + query := req.URL.Query() + query.Set("owner", stranger) + req.URL.RawQuery = query.Encode() + + w := httptest.NewRecorder() + env.serve(w, req) + require.Equal(t, http.StatusCreated, w.Code) + + var response CreateTranscribeJobResponse + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) + + record, err := env.app.FindRecordById("transcribe_jobs", response.JobID) + require.NoError(t, err) + assert.Equal(t, env.account.Id, record.GetString("owner")) +} + +// Предъявитель, чья сессия не даёт учётной записи пользователя, получает отказ +// до чтения тела. Владелец панели — именно такой: узнан он узнан, а записи в +// коллекции пользователей у него нет, и владельцем записи он стать не может. +// +// Отказ **до** укладки обязателен: позже пришлось бы убирать уже сохранённый +// файл, а уборки файлов сервис не умеет вовсе. +func TestCreateTranscribeJob_SuperuserSessionRejected(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + superusers, err := env.app.FindCollectionByNameOrId(core.CollectionNameSuperusers) + require.NoError(t, err) + + admin := core.NewRecord(superusers) + admin.Set("email", "owner@example.com") + admin.SetRandomPassword() + require.NoError(t, env.app.Save(admin)) + + token, err := admin.NewAuthToken() + require.NoError(t, err) + + w := httptest.NewRecorder() + serveAs(env, token, w, createMultipartRequest(t, "sample.mp3", []byte("запись"))) + + assert.Equal(t, http.StatusForbidden, w.Code, "узнан, но не запись коллекции пользователей") + assert.Equal(t, 0, countJobs(t, env), "задачи не заведено") + assert.Equal(t, 0, countFiles(t, env), "и файла тоже") +} + +// Чужой файл не отдаётся по ссылке, а свой отдаётся. Проверяется именно переход +// по ссылке: токен файла хранилище выдаёт на предъявителя, а не на файл, и отказ +// наступает на скачивании, где правило просмотра судит владельца. Проверка, +// написанная на выдачу токена, зеленела бы, не касаясь пути, по которому аудио и +// уходит. +func TestFileDownload_NarrowedByOwner(t *testing.T) { + env := setupTestEnv(t, readableMetaViewer()) + + job := jobWithFile(t, env) + _, stranger := newSecondAccount(t, env.app) + + record, err := env.app.FindRecordById("files", *job.FileID) + require.NoError(t, err) + require.Equal(t, env.account.Id, record.GetString("owner")) + + link := "/api/files/files/" + record.Id + "/" + record.GetString("file") + + mine := httptest.NewRecorder() + env.mux.ServeHTTP(mine, withSessionHeader( + httptest.NewRequest("GET", link+"?token="+fileToken(t, env, env.session), http.NoBody), env.session)) + + foreign := httptest.NewRecorder() + env.mux.ServeHTTP(foreign, withSessionHeader( + httptest.NewRequest("GET", link+"?token="+fileToken(t, env, stranger), http.NoBody), stranger)) + + require.Equal(t, http.StatusOK, mine.Code, "свой файл отдаётся") + assert.Equal(t, "запись", mine.Body.String(), "и отдаётся содержимым") + + assert.NotEqual(t, http.StatusOK, foreign.Code, "чужой файл не отдаётся") + assert.NotContains(t, foreign.Body.String(), "запись", "содержимого в отказе нет") +} + +// fileToken берёт у хранилища токен файла для названной сессии. Токен выдаётся +// на предъявителя: о файле хранилище при выдаче не спрашивает. +func fileToken(t *testing.T, env *testEnv, session string) string { + t.Helper() + + w := httptest.NewRecorder() + env.mux.ServeHTTP(w, withSessionHeader( + httptest.NewRequest("POST", "/api/files/token", http.NoBody), session)) + require.Equal(t, http.StatusOK, w.Code, "токен файла выдаётся всякому вошедшему") + + var body struct { + Token string `json:"token"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) + require.NotEmpty(t, body.Token) + + return body.Token +} diff --git a/internal/controller/http/transcribe.go b/internal/controller/http/transcribe.go index 083094d..9593d25 100644 --- a/internal/controller/http/transcribe.go +++ b/internal/controller/http/transcribe.go @@ -2,6 +2,7 @@ package http import ( "context" + "errors" "log/slog" "net/http" "time" @@ -10,6 +11,7 @@ import ( "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tools/router" + "git.vakhrushev.me/av/transcriber/internal/adapter/repo/pocketbase/migrations" "git.vakhrushev.me/av/transcriber/internal/contract" "git.vakhrushev.me/av/transcriber/internal/entity" "git.vakhrushev.me/av/transcriber/internal/service" @@ -51,7 +53,13 @@ func (h *TranscribeHandler) Register(r *router.Router[*core.RequestEvent]) { // него не подпадает, часть её защищена ровно тем, что браузер заголовка сам // не шлёт. api.Bind(SessionFromCookie()) - api.Bind(apis.RequireAuth()) + // Коллекция названа поимённо, а не оставлена умолчанию. Без имени проверка + // пускает всякую учётную запись хранилища, включая владельца панели, — а + // записи в коллекции пользователей у него нет, и владельцем записи он стать + // не может. Отказ такому предъявителю обязан наступить здесь, до чтения + // тела: позже пришлось бы убирать уже уложенный файл, а уборки файлов + // сервис не умеет вовсе. + api.Bind(apis.RequireAuth(migrations.UsersCollection)) // Умолчание роутера хранилища — 32 МиБ на тело, и оно отсекало бы запись // раньше обработчика, без строки в журнале приёма. Приём размеру не судья, @@ -79,7 +87,11 @@ func (h *TranscribeHandler) CreateTranscribeJob(e *core.RequestEvent) error { // (журнал запроса, сессия) при этом сохраняются, теряется только отмена. ctx := context.WithoutCancel(e.Request.Context()) - job, err := h.trsService.CreateJobFromApi(ctx, file, header.Filename) + // Владелец берётся из предъявленной сессии и ниоткуда больше: владелец, + // пришедший полем запроса, дал бы всякому вошедшему право завести запись на + // чужое имя. Проверка предъявителя стоит слоем выше, поэтому здесь `e.Auth` + // уже есть и принадлежит коллекции пользователей. + job, err := h.trsService.CreateJobFromApi(ctx, file, header.Filename, e.Auth.Id) if err != nil { // Второй раз отказ не логируем: приём назван конвенцией логирующей // границей и уже написал о нём. Транспорт переводит ошибку в ответ. @@ -96,8 +108,22 @@ func (h *TranscribeHandler) CreateTranscribeJob(e *core.RequestEvent) error { func (h *TranscribeHandler) GetTranscribeJobStatus(e *core.RequestEvent) error { jobID := e.Request.PathValue("id") - job, err := h.jobRepo.GetByID(jobID) + // Чужая задача, задача без владельца и несуществующая отвечают одним и тем + // же: хранилище отдаёт на все три ту же ошибку, а транспорт — тот же код и + // то же тело. Различать их наружу нельзя — по разнице ответов перебирается + // список заведённых задач. + job, err := h.jobRepo.GetByID(jobID, e.Auth.Id) if err != nil { + // Наружу ответ один на все исходы, а в журнал они идут по-разному. + // «Задачи нет» и «задача чужая» — штатная работа разграничения, о ней + // писать нечего; всё прочее — отказ хранилища, и без этой строки он + // приходит отправителю как «вашей записи нет», а владелец сервиса об + // аварии не узнаёт ниоткуда. Журнал читает владелец, а не тот, кто + // перебирает, поэтому различать их здесь можно. + var notFound *contract.JobNotFoundError + if !errors.As(err, ¬Found) { + h.logger.Error("Failed to read transcribe job", "error", err, "job_id", jobID) + } return e.JSON(http.StatusNotFound, map[string]string{"error": "Job not found"}) } diff --git a/internal/controller/http/transcribe_test.go b/internal/controller/http/transcribe_test.go index 1d70c5d..1955397 100644 --- a/internal/controller/http/transcribe_test.go +++ b/internal/controller/http/transcribe_test.go @@ -273,10 +273,18 @@ func jobWithFile(t *testing.T, env *testEnv) *entity.TranscribeJob { require.NoError(t, err) defer func() { require.NoError(t, work.Close()) }() - file, err := repo.CreateLocal("sample.mp3", work) + // Владелец — учётная запись проверки: задача, пришедшая из веба, без + // владельца больше не заводится, и фикстура без него описывала бы состояние, + // которого в проде не бывает. + file, err := repo.CreateLocal("sample.mp3", work, env.account.Id) require.NoError(t, err) - job := &entity.TranscribeJob{State: entity.StateCreated, Source: entity.SourceApi, FileID: &file.Id} + job := &entity.TranscribeJob{ + State: entity.StateCreated, + Source: entity.SourceApi, + OwnerID: &env.account.Id, + FileID: &file.Id, + } require.NoError(t, env.handler.jobRepo.Create(job)) return job } @@ -325,7 +333,7 @@ func TestCreateTranscribeJob_Success(t *testing.T) { // отправитель получит идентификатор записи, которой не будет никогда. require.Equal(t, 1, countJobs(t, env)) - job, err := env.handler.jobRepo.GetByID(response.JobID) + job, err := env.handler.jobRepo.GetByID(response.JobID, env.account.Id) require.NoError(t, err) assert.Equal(t, entity.StateCreated, job.State) require.NotNil(t, job.FileID) @@ -595,7 +603,7 @@ func TestCreateTranscribeJob_JournalTracesRecord(t *testing.T) { var response CreateTranscribeJobResponse require.NoError(t, json.Unmarshal(w.Body.Bytes(), &response)) - job, err := env.handler.jobRepo.GetByID(response.JobID) + job, err := env.handler.jobRepo.GetByID(response.JobID, env.account.Id) require.NoError(t, err) require.NotNil(t, job.FileID) diff --git a/internal/entity/job.go b/internal/entity/job.go index 4c29edf..3a34b02 100644 --- a/internal/entity/job.go +++ b/internal/entity/job.go @@ -7,8 +7,12 @@ import ( ) type TranscribeJob struct { - Id string - State string + Id string + State string + // OwnerID — учётная запись, от имени которой запись принята. Пуст у записей + // из Telegram: связи чата с учётной записью сервис не ведёт. Назначается + // один раз, при приёме, и у записи, где он есть, больше не меняется. + OwnerID *string Source string FileID *string ErrorText *string diff --git a/internal/service/find_job_test.go b/internal/service/find_job_test.go index c30a93a..cc85aa9 100644 --- a/internal/service/find_job_test.go +++ b/internal/service/find_job_test.go @@ -26,7 +26,7 @@ type stubJobRepo struct { func (r *stubJobRepo) Create(*entity.TranscribeJob) error { return nil } func (r *stubJobRepo) Save(*entity.TranscribeJob, string) error { return nil } -func (r *stubJobRepo) GetByID(string) (*entity.TranscribeJob, error) { +func (r *stubJobRepo) GetByID(string, string) (*entity.TranscribeJob, error) { return nil, errors.New("не зовётся этими проверками") } diff --git a/internal/service/ownership_test.go b/internal/service/ownership_test.go new file mode 100644 index 0000000..35be3a4 --- /dev/null +++ b/internal/service/ownership_test.go @@ -0,0 +1,105 @@ +package service + +import ( + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "git.vakhrushev.me/av/transcriber/internal/contract" + "git.vakhrushev.me/av/transcriber/internal/entity" +) + +// Выборка воркера владельцем не сужается: владелец решает, кому запись +// показывать, а не кому её считать. Сужение остановило бы расшифровку записей +// бота вовсе, а записи остальных поставило бы в зависимость от того, кто первым +// завёл учётную запись. +func TestWorkerTakesJobsOfEveryOwner(t *testing.T) { + env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) + + first, err := env.service.CreateJobFromApi(t.Context(), + strings.NewReader("первая"), "one.mp3", newOwner(t, env.app)) + require.NoError(t, err) + + second, err := env.service.CreateJobFromApi(t.Context(), + strings.NewReader("вторая"), "two.mp3", newOwner(t, env.app)) + require.NoError(t, err) + + // Третья пришла ботом, и владельца у неё нет вовсе. + third := newTelegramJob(t, env) + + // Срок протухания в прошлом: захваченная задача остаётся за держателем, и + // следующий вызов берёт следующую, а не ту же самую. + taken := map[string]bool{} + for _, holder := range []string{"one", "two", "three"} { + job, err := env.jobRepo.FindAndAcquire(entity.StateCreated, holder, time.Now().Add(-time.Hour)) + require.NoError(t, err, "воркер берёт задачи подряд, владельцем не сужаясь") + taken[job.Id] = true + } + + assert.True(t, taken[first.Id], "задача первого владельца досталась воркеру") + assert.True(t, taken[second.Id], "и второго") + assert.True(t, taken[third.Id], "и задача без владельца") + + _, err = env.jobRepo.FindAndAcquire(entity.StateCreated, "next", time.Now().Add(-time.Hour)) + var missing *contract.JobNotFoundError + assert.ErrorAs(t, err, &missing, "больше в этом состоянии никого") +} + +// Захват читает владельца: снимок задачи, в котором он всегда пуст, был бы +// ловушкой для первого же шага, начавшего сохранять задачу целиком, — и сегодня +// уже ломал бы файл, который шаг заводит. +func TestAcquireCarriesOwner(t *testing.T) { + env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) + + owner := newOwner(t, env.app) + job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "one.mp3", owner) + require.NoError(t, err) + + acquired, err := env.jobRepo.FindAndAcquire(entity.StateCreated, "holder", time.Now().Add(-time.Hour)) + require.NoError(t, err) + require.Equal(t, job.Id, acquired.Id) + + require.NotNil(t, acquired.OwnerID, "владелец приехал из захвата") + assert.Equal(t, owner, *acquired.OwnerID) +} + +// Шаг конвейера владельца не затирает: сохранение кладёт только то, чем +// распоряжается конвейер, и владельца среди этого нет. +func TestPipelineStepKeepsOwner(t *testing.T) { + env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) + + owner := newOwner(t, env.app) + job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "one.mp3", owner) + require.NoError(t, err) + + // Отказ конвертации — приговор записи: шаг переводит задачу в `failed` и + // сохраняет её. Это сохранение владельца тронуть не должно. + require.NoError(t, env.service.FindAndRunConversionJob(t.Context())) + + after, err := readJob(env.app, job.Id) + require.NoError(t, err) + require.Equal(t, entity.StateFailed, after.State, "шаг записал свой приговор") + require.NotNil(t, after.OwnerID, "владелец пережил шаг") + assert.Equal(t, owner, *after.OwnerID) +} + +// Приём из веба без владельца задачи не заводит. Обязательность держит здесь +// код, а не схема: колонка допускает пустое значение ради записей бота. +func TestCreateJobFromApiRequiresOwner(t *testing.T) { + env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) + + _, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "one.mp3", "") + + require.ErrorIs(t, err, contract.ErrOwnerRequired) + + records, err := env.app.FindAllRecords("transcribe_jobs") + require.NoError(t, err) + assert.Empty(t, records, "задачи не заведено") + + files, err := env.app.FindAllRecords("files") + require.NoError(t, err) + assert.Empty(t, files, "и файла тоже: отказ наступает раньше укладки") +} diff --git a/internal/service/pipeline_test.go b/internal/service/pipeline_test.go index 0af6c72..35e83be 100644 --- a/internal/service/pipeline_test.go +++ b/internal/service/pipeline_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tools/types" "github.com/stretchr/testify/assert" @@ -155,7 +156,7 @@ func TestJobDiesAfterAttemptLimit(t *testing.T) { var noop *contract.NoopJobError require.ErrorAs(t, err, &noop, "мёртвая задача шагу не отдаётся") - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDead, after.State, "задача видна отбором по состоянию") assert.Greater(t, after.Attempts, maxAttempts, "число попыток сохранено") @@ -201,7 +202,7 @@ func TestFailedStepSchedulesRetryWithGrowingDelay(t *testing.T) { // Ссылку переставляем на запись без содержимого: шаг отказывает на получении // рабочей копии — то есть отказом, а не приговором записи. - empty, err := env.fileRepo.CreateRemote("object-key", 1) + empty, err := env.fileRepo.CreateRemote("object-key", 1, "") require.NoError(t, err) record, err := env.app.FindRecordById(migrations.JobsCollection, job.Id) @@ -212,7 +213,7 @@ func TestFailedStepSchedulesRetryWithGrowingDelay(t *testing.T) { // Первый отказ. require.Error(t, env.service.FindAndRunConversionJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) require.Nil(t, after.AcquisitionID, "захват снят: задача пригодна к повтору") require.NotNil(t, after.DelayTime, "пауза поставлена") @@ -223,7 +224,7 @@ func TestFailedStepSchedulesRetryWithGrowingDelay(t *testing.T) { clearDelay(t, env, job.Id) require.Error(t, env.service.FindAndRunConversionJob(t.Context())) - after, err = env.jobRepo.GetByID(job.Id) + after, err = readJob(env.app, job.Id) require.NoError(t, err) require.NotNil(t, after.DelayTime) @@ -248,7 +249,7 @@ func TestWorkFileRemovedAfterIntakeFailure(t *testing.T) { env := newPipelineEnv(t, &failingMetaViewer{}, &failingConverter{}) - _, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "sample.mp3") + _, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "sample.mp3", newOwner(t, env.app)) require.Error(t, err, "отказ источника метаданных роняет приём") leftovers, err := filepath.Glob(filepath.Join(tempDir, "transcriber-*")) @@ -264,7 +265,7 @@ func TestWorkFileRemovedAfterSuccessfulIntake(t *testing.T) { env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) - _, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "sample.mp3") + _, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "sample.mp3", newOwner(t, env.app)) require.NoError(t, err) leftovers, err := filepath.Glob(filepath.Join(tempDir, "transcriber-*")) @@ -283,7 +284,7 @@ func TestJobNeverPointsToMissingFile(t *testing.T) { // исходную запись, а не на несозданный результат. require.NoError(t, env.service.FindAndRunConversionJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateFailed, after.State) require.NotNil(t, after.FileID) @@ -299,7 +300,7 @@ func TestStoredContentSurvivesRoundTrip(t *testing.T) { content := strings.Repeat("запись ", 1000) - job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader(content), "sample.mp3") + job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader(content), "sample.mp3", newOwner(t, env.app)) require.NoError(t, err) require.NotNil(t, job.FileID) @@ -322,7 +323,7 @@ func TestStoredContentSurvivesRoundTrip(t *testing.T) { func TestLocalizeGivesReadableCopy(t *testing.T) { env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) - job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("содержимое"), "sample.mp3") + job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("содержимое"), "sample.mp3", newOwner(t, env.app)) require.NoError(t, err) require.NotNil(t, job.FileID) @@ -362,3 +363,73 @@ func TestWorkFilesRemovedAfterConversionFailure(t *testing.T) { require.NoError(t, err) assert.Empty(t, leftovers, "ни исходной копии, ни копии под результат не осталось") } + +// readJob читает задачу мимо сужения владельцем. +// +// Читающий метод хранилища отдаёт задачу только её владельцу, а проверки +// конвейера смотрят задачи, принятые ботом: владельца у таких нет вовсе, и по +// правилу разграничения они не достаются никому. Проверке нужен не доступ, а +// состояние записи после шага, поэтому она берёт его прямо из хранилища. Это +// не второй способ читать задачу в сервисе — в сервисе способ по-прежнему один. +func readJob(app core.App, id string) (*entity.TranscribeJob, error) { + record, err := app.FindRecordById(migrations.JobsCollection, id) + if err != nil { + return nil, err + } + + job := &entity.TranscribeJob{ + Id: record.Id, + State: record.GetString("state"), + Source: record.GetString("source"), + Attempts: record.GetInt("attempts"), + CreatedAt: record.GetDateTime("created").Time(), + UpdatedAt: record.GetDateTime("updated").Time(), + } + + for _, field := range []struct { + name string + dst **string + }{ + {"owner", &job.OwnerID}, + {"file", &job.FileID}, + {"error_text", &job.ErrorText}, + {"acquisition_id", &job.AcquisitionID}, + {"recognition_op_id", &job.RecognitionOpID}, + {"transcription_text", &job.TranscriptionText}, + } { + if value := record.GetString(field.name); value != "" { + stored := value + *field.dst = &stored + } + } + + if delay := record.GetDateTime("delay_time"); !delay.IsZero() { + moment := delay.Time() + job.DelayTime = &moment + } + if acquired := record.GetDateTime("acquire_time"); !acquired.IsZero() { + moment := acquired.Time() + job.AcquireTime = &moment + } + + return job, nil +} + +// newOwner заводит учётную запись и отдаёт её идентификатор. +// +// Владелец — связь с коллекцией пользователей, и хранилище проверяет, что такая +// запись есть: выдуманный идентификатор задачу завести не даст. +func newOwner(t *testing.T, app core.App) string { + t.Helper() + + users, err := app.FindCollectionByNameOrId(migrations.UsersCollection) + require.NoError(t, err) + + record := core.NewRecord(users) + record.Set("email", uuid.NewString()+"@example.test") + record.Set("verified", true) + record.SetPassword(uuid.NewString()) + require.NoError(t, app.Save(record)) + + return record.Id +} diff --git a/internal/service/recognition_test.go b/internal/service/recognition_test.go index c3fb8f9..7ac89f8 100644 --- a/internal/service/recognition_test.go +++ b/internal/service/recognition_test.go @@ -98,7 +98,7 @@ func TestTranscribeJobHandsRecordOverAndMovesOn(t *testing.T) { assert.Equal(t, 1, rec.recognizeCalls, "содержимое отдано распознавателю") assert.NotEmpty(t, rec.lastObjectKey, "ключ объекта назван") - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateTranscribe, after.State) require.NotNil(t, after.RecognitionOpID) @@ -122,7 +122,7 @@ func TestTranscribeJobKeepsJobRetryableOnRecognizerFailure(t *testing.T) { require.Error(t, svc.FindAndRunTranscribeJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateConverted, after.State, "задача осталась на своём шаге") assert.Nil(t, after.AcquisitionID, "захват снят: задача пригодна к повтору") @@ -153,7 +153,7 @@ func TestCheckJobWaitsWithoutSpendingAttempts(t *testing.T) { for i := 0; i < 3; i++ { require.NoError(t, svc.FindAndRunTranscribeCheckJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateTranscribe, after.State) assert.Equal(t, 0, after.Attempts, "ожидание операции попытку не тратит") @@ -177,7 +177,7 @@ func TestCheckJobFailsJobAndTellsSender(t *testing.T) { require.NoError(t, svc.FindAndRunTranscribeCheckJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateFailed, after.State) @@ -199,7 +199,7 @@ func TestCheckJobCompletesAndAnswersOnce(t *testing.T) { require.NoError(t, svc.FindAndRunTranscribeCheckJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDone, after.State) require.NotNil(t, after.TranscriptionText) @@ -227,7 +227,7 @@ func TestCheckJobCompletesEmptyTextWithExplanation(t *testing.T) { require.NoError(t, svc.FindAndRunTranscribeCheckJob(t.Context())) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDone, after.State) @@ -261,7 +261,7 @@ func TestCheckJobWritesNothingWhenAcquisitionLost(t *testing.T) { var lost *contract.LostAcquisitionError require.ErrorAs(t, err, &lost) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateTranscribe, after.State, "результат не записан") assert.Empty(t, env.sender.messages, "отправителю ничего не отправлено") diff --git a/internal/service/shutdown_test.go b/internal/service/shutdown_test.go index 63b7b69..0ca282c 100644 --- a/internal/service/shutdown_test.go +++ b/internal/service/shutdown_test.go @@ -45,7 +45,7 @@ func TestShutdownDuringConversionKeepsJobRetryable(t *testing.T) { require.Error(t, err, "шаг обязан сообщить об обрыве наверх") require.ErrorIs(t, err, context.Canceled, "обрыв узнаётся по смыслу, а не по тексту") - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateCreated, after.State, "задача осталась на повтор, а не похоронена") @@ -72,7 +72,7 @@ func TestShutdownBeforeStepLeavesJobUntouched(t *testing.T) { var noop *contract.NoopJobError require.ErrorAs(t, err, &noop) - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateCreated, after.State) assert.Equal(t, 0, after.Attempts, "захвата не было — попытке взяться неоткуда") diff --git a/internal/service/transcribe.go b/internal/service/transcribe.go index 6114f97..519aeb3 100644 --- a/internal/service/transcribe.go +++ b/internal/service/transcribe.go @@ -87,10 +87,24 @@ func (s *TranscribeService) CreateJobFromTelegram(ctx context.Context, file io.R return s.createTranscribeJob(ctx, job, file, fileName) } -func (s *TranscribeService) CreateJobFromApi(ctx context.Context, file io.Reader, fileName string) (*entity.TranscribeJob, error) { +// CreateJobFromApi заводит задачу от имени вошедшего. Владелец обязателен: +// пустой отвергается здесь, потому что колонка владельца допускает пустое +// значение ради записей бота, и приём по HTTP — то место, где обязательность +// держится. +// +// Отказ этот — последний рубеж, а не первый: предъявителя без учётной записи +// пользователя транспорт отвергает раньше, до чтения тела. Здесь он остаётся на +// случай нового вызывающего, который такой проверки не поставит. +func (s *TranscribeService) CreateJobFromApi(ctx context.Context, file io.Reader, fileName, ownerID string) (*entity.TranscribeJob, error) { + if ownerID == "" { + s.logger.Error("Refusing to create job without owner") + return nil, contract.ErrOwnerRequired + } + job := &entity.TranscribeJob{ - State: entity.StateCreated, - Source: entity.SourceApi, + State: entity.StateCreated, + Source: entity.SourceApi, + OwnerID: &ownerID, } return s.createTranscribeJob(ctx, job, file, fileName) @@ -135,7 +149,7 @@ func (s *TranscribeService) createTranscribeJob(ctx context.Context, job *entity return nil, err } - fileRecord, err := s.fileRepo.CreateLocal(storageFileName, work) + fileRecord, err := s.fileRepo.CreateLocal(storageFileName, work, ownerOf(job)) if err != nil { s.logger.Error("Failed to create file record", "error", err, "file_ext", ext) return nil, err @@ -284,7 +298,7 @@ func (s *TranscribeService) convertJob(ctx context.Context, job *entity.Transcri metrics.OutputFileSizeHistogram.WithLabelValues("ogg").Observe(float64(destSize)) destFileName := fmt.Sprintf("%s%s", uuid.NewString(), ".ogg") - destFileRecord, err := s.fileRepo.CreateLocal(destFileName, dest) + destFileRecord, err := s.fileRepo.CreateLocal(destFileName, dest, ownerOf(job)) if err != nil { s.logger.Error("Failed to create converted file record", "error", err, "job_id", job.Id) return err @@ -346,7 +360,7 @@ func (s *TranscribeService) transcribeJob(ctx context.Context, job *entity.Trans "job_id", job.Id, "operation_id", operationID) - destFileRecord, err := s.fileRepo.CreateRemote(fileRecord.FileName, fileRecord.Size) + destFileRecord, err := s.fileRepo.CreateRemote(fileRecord.FileName, fileRecord.Size, ownerOf(job)) if err != nil { s.logger.Error("Failed to create S3 file record", "error", err, "job_id", job.Id) return err @@ -635,3 +649,14 @@ func (s *TranscribeService) closeWork(work contract.WorkFile) { s.logger.Error("Failed to remove work file", "error", err) } } + +// ownerOf — владелец задачи строкой; пустая значит «владельца нет», и таковы +// записи, принятые ботом. Файл наследует владельца своей задачи: правило +// просмотра коллекции файлов сужено этой колонкой, и файл, заведённый шагом +// конвейера без неё, перестал бы доставаться собственному владельцу. +func ownerOf(job *entity.TranscribeJob) string { + if job.OwnerID == nil { + return "" + } + return *job.OwnerID +} diff --git a/internal/service/undelivered_test.go b/internal/service/undelivered_test.go index db54ba6..f1cd1b7 100644 --- a/internal/service/undelivered_test.go +++ b/internal/service/undelivered_test.go @@ -71,7 +71,7 @@ func TestUndeliveredOnDownChannelKeepsJobDone(t *testing.T) { assert.Equal(t, 1, sender.calls, "ответ до отправителя доехал") - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDone, after.State, "задача осталась в достигнутом состоянии") require.NotNil(t, after.TranscriptionText) @@ -108,7 +108,7 @@ func TestUndeliveredWithoutChatKeepsJobDone(t *testing.T) { assert.Equal(t, 0, sender.calls, "до отправителя дело не дошло: адресата нет") - after, err := env.jobRepo.GetByID(job.Id) + after, err := readJob(env.app, job.Id) require.NoError(t, err) assert.Equal(t, entity.StateDone, after.State) assert.Nil(t, after.ErrorText, "отказ задаче не приписан") @@ -127,7 +127,7 @@ func TestUndeliveredWithoutChatKeepsJobDone(t *testing.T) { func TestApiJobDoesNotReachSenderAndLogsNothing(t *testing.T) { env := newPipelineEnv(t, &okMetaViewer{}, &failingConverter{}) - job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "voice.ogg") + job, err := env.service.CreateJobFromApi(t.Context(), strings.NewReader("запись"), "voice.ogg", newOwner(t, env.app)) require.NoError(t, err) sender := &downSender{} diff --git a/journal_route_test.go b/journal_route_test.go new file mode 100644 index 0000000..66bdcf9 --- /dev/null +++ b/journal_route_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Имя файла в хранилище в журнал не идёт: оно последняя часть ссылки +// `/api/files/...`, и строка журнала вместе с идентификатором записи собрала бы +// ссылку целиком. Инвариант проекта, critical. +func TestJournalRouteHidesStoredFileName(t *testing.T) { + cases := []struct { + name string + path string + want string + }{ + { + name: "ссылка на файл теряет имя", + path: "/api/files/files/abc123def456ghi/9f1c-3b2a.mp3", + want: "/api/files/files/abc123def456ghi/<имя>", + }, + { + name: "маршрут остаётся различимым", + path: "/api/files/files/abc123def456ghi/запись.ogg", + want: "/api/files/files/abc123def456ghi/<имя>", + }, + { + name: "прочие пути не трогаются", + path: "/api/status/abc123def456ghi", + want: "/api/status/abc123def456ghi", + }, + { + name: "приём не трогается", + path: "/api/audio", + want: "/api/audio", + }, + { + name: "сам префикс без имени не портится", + path: "/api/files/", + want: "/api/files/", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, journalRoute(c.path)) + }) + } +} + +// Отдельно и прямо: имени в готовой строке нет. Проверка судит результат, а не +// устройство — переписанная реализация обязана остаться зелёной. +func TestJournalRouteDropsNameEntirely(t *testing.T) { + const stored = "0f7b8dd3-d1cc-424c.mp3" + + route := journalRoute("/api/files/files/rec0000000000000/" + stored) + + assert.NotContains(t, route, stored, "имя файла в хранилище не доезжает до журнала") + assert.Contains(t, route, "rec0000000000000", "идентификатор записи остаётся: по нему прослеживается путь") +} diff --git a/main.go b/main.go index 19986bc..e2a5ea0 100644 --- a/main.go +++ b/main.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "os/signal" + "strings" "sync" "syscall" "time" @@ -231,7 +232,7 @@ func main() { logger.Log(e.Request.Context(), level, "Incoming request", "http.method", e.Request.Method, - "http.route", e.Request.URL.Path, + "http.route", journalRoute(e.Request.URL.Path), "http.status_code", e.Status(), "duration_ms", time.Since(start).Milliseconds(), "transport", "http") @@ -344,3 +345,30 @@ func main() { logger.Info("Transcriber service stopped") } + +// filesPathPrefix — начало пути, которым хранилище отдаёт файл записи. Последний +// сегмент такого пути и есть имя файла в хранилище. +const filesPathPrefix = "/api/files/" + +// journalRoute готовит путь запроса к записи в журнал. +// +// Инвариант проекта запрещает имени файла в хранилище попадать в журнал: имя — +// последняя часть ссылки `/api/files/...`, и строка журнала вместе с +// идентификатором записи собирала бы ссылку целиком. Слой журнала пишет путь +// всякого запроса, поэтому имя срезается здесь — иначе оно уезжало бы в +// собранные логи при каждом скачивании записи. +// +// Срезается только имя: маршрут остаётся различимым, и наблюдаемость от этого не +// теряется. +func journalRoute(path string) string { + if !strings.HasPrefix(path, filesPathPrefix) { + return path + } + + cut := strings.LastIndex(path, "/") + if cut < len(filesPathPrefix) { + return path + } + + return path[:cut+1] + "<имя>" +} diff --git a/openspec/changes/archive/2026-08-14-record-ownership/.openspec.yaml b/openspec/changes/archive/2026-08-14-record-ownership/.openspec.yaml new file mode 100644 index 0000000..4af8641 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-14 diff --git a/openspec/changes/archive/2026-08-14-record-ownership/design.md b/openspec/changes/archive/2026-08-14-record-ownership/design.md new file mode 100644 index 0000000..927dde3 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/design.md @@ -0,0 +1,197 @@ +## Context + +Сессия сегодня отвечает на один вопрос — узнан ли пришедший. На вопрос «чьё он +смотрит» не отвечает никто: опрос готовности отдаёт задачу всякому вошедшему по +её идентификатору. Так записано и в спеках прямым текстом — обе строки +(`access`, преамбула; `intake`, «Опрос готовности задачи») обещают, что сужение +придёт отдельной задачей. Эта задача та самая. + +Учётные записи в сервисе уже есть: их заводит вход через внешнего провайдера, +заведённый 2026-08-12. Значит владельцем записи есть кому быть. + +Ограничения, из которых собрано решение: + +- **Данные не переносим** — рамки задачи объявляют чистый лист. Значит шагу схемы + не нужно ни умолчание для старых записей, ни правило их раздачи. +- **Применённый шаг схемы не переписывается** (инвариант, critical). Колонка + приезжает новым шагом. +- **Колонки очереди правятся в четырёх местах** пакета хранилища (инвариант, + major), и компилятор видит два из них. +- **Записи из Telegram владельца получить не могут**: связи чата с учётной + записью не существует, её заводит `telegram-account-link` следующей задачей. + +## Goals / Non-Goals + +**Goals:** + +- у задачи расшифровки есть владелец, назначенный при приёме и неизменяемый; +- чужая задача по её идентификатору неотличима от несуществующей; +- конвейер работает по всем записям подряд, владельцем не сужаясь; +- колонка владельца не имеет умолчания: запись не достаётся никому по недосмотру. + +**Non-Goals:** + +- совместный доступ, роли, передача записи другому; +- перенос записей, заведённых до этого шага; +- владелец у записи, пришедшей ботом, — его назначает следующая задача; +- список своих записей и удаление: они стоят **на** владельце, но заводятся + своими задачами. + +## Decisions + +### Сужение живёт в хранилище, а не в обработчике + +Читающий метод репозитория задач принимает владельца и отдаёт задачу, только +если она его. Не «прочитать и сравнить в обработчике»: сравнение, стоящее в +вызывающем, повторяется в каждом новом вызывающем, и первый же забывший его +открывает чужую запись. Компилятор при смене подписи метода приводит всех +вызывающих сам. + +**Отвергнуто: правило доступа коллекции хранилища.** Коллекции хранилища знают +правила вида «владелец записи равен предъявителю», и в панели они работают. Наш +опрос готовности идёт мимо коллекций — своим обработчиком и своим запросом, — и +правило коллекции на этом пути не срабатывает вовсе. Опереться на него значит +получить защиту, которой не существует, и не заметить этого: тест, ходящий +нашим API, был бы зелёным в обоих случаях. + +**Отвергнуто: второй метод чтения рядом с прежним** (`GetByID` и +`GetOwnedByID`). Это второй способ делать одно и то же, и выбирается он по +внимательности вызывающего. Метод остаётся один, и владелец у него обязателен. + +### Чужая запись отвечает «не найдено», а не «доступ запрещён» + +Отказ по чужой записи и отказ по несуществующей — один и тот же ответ, `404` с +тем же телом. Отдельный код на чужую запись превращает опрос в перебор: по нему +считывается, какие идентификаторы заведены, а идентификатор задачи и есть то, +что мы прячем. + +Различать их в журнале при этом можно и нужно — журнал читает владелец сервиса, +а не тот, кто перебирает. + +### Владелец — связь на учётную запись, а не строка + +Колонка заводится связью с коллекцией пользователей: хранилище само следит за +тем, чтобы владельцем стояла существующая учётная запись, а не строка, похожая +на её идентификатор. + +**Целостности при удалении учётной записи связь при этом не удерживает, и +прежнее обоснование этого решения было неверным.** Проверено по исходникам +`pocketbase@v0.39.10`, `core/record_model.go`, `deleteRefRecords`: при выключенном +каскадном удалении хранилище **вынимает** идентификатор из поля связи и сохраняет +запись без проверок. То есть удаление учётной записи не уносит её задачи — но +делает их ничьими, а по правилу этой же задачи ничья запись не достаётся по API +никому. Архив человека становится недостижим молча, и восстановить владельца +нечем. + +Строка с идентификатором вела бы себя здесь лучше — она пережила бы удаление, — +но платила бы тем, что владельцем становится любое значение. Что делать с этим, +решает человек на чекпоинте: развилка вынесена в открытые вопросы. + +### Захват читает колонку владельца + +Владелец добавляется во все четыре места правки колонок очереди, включая +`acquireColumns` и `acquiredRow`, хотя конвейер владельцем не пользуется. + +Причина в инварианте: колонка, забытая в этой паре, приезжает из захвата нулевой. +Сохранение конвейера сегодня кладёт только свои поля и владельца не трогает — +значит потери нет; но снимок задачи, в котором владелец всегда пуст, это ловушка +для первого же шага, который начнёт сохранять задачу целиком. Чтение колонки +стоит одной строки в каждом из четырёх мест и снимает ловушку насовсем. + +### Приём назначает владельца из предъявленной сессии + +Приём по HTTP уже требует сессию и отказывает `401` без неё. Владелец берётся +оттуда же, и другого источника у него нет: параметром запроса владелец не +задаётся никогда, иначе всякий вошедший заводит запись на чужое имя. + +## Risks / Trade-offs + +- **Колонка обязательна в схеме → приём из Telegram перестаёт вставлять записи.** + Критерий приёмки требует обязательности, рамки запрещают назначать владельца + записям бота, и вместе это ломает основной сегодня вход. → Развилка вынесена в + открытые вопросы и на чекпоинт; ниже разобраны три способа с ценой каждого. +- **Записи бота остаются без владельца и потому невидимы в API.** Это желаемое + поведение (ответ бот отдаёт сам, в чат), но оно значит, что после + `telegram-account-link` записи придётся связать с учётной записью задним + числом — а рамки объявляют владельца неизменяемым. → Граница названа здесь и + передаётся той задаче: неизменяемость относится к записи, у которой владелец + **есть**. +- **Тест на две сессии дороже прежних.** Нужны две учётные записи в тестовой + базе и две куки. → Заводятся тем же способом, что и в тестах входа. +- **Публичный контракт меняется**: `GET /api/status/{id}` на чужую задачу + отвечает `404` там, где отдавал содержимое. → Ломка намеренная и объявлена в + предложении; форма ответа при этом не трогается. + +## Migration Plan + +Один новый шаг схемы, добавляющий колонку в таблицу задач. Прежние шаги не +трогаются. Данные не переносятся: рамки объявляют чистый лист, и записей, +заведённых до шага, в боевой базе на момент выкладки быть не должно — это +проверяет владелец перед выкладкой, машина такого проверить не может. + +**Отката приложения нет — решение владельца от 2026-08-14.** Прежний образ поверх +новой схемы поднялся бы: колонку он не читает, и она ему не мешает. Но и не +пишет — записи, принятые по HTTP в таком окне, остались бы без владельца и после +возврата вперёд не достались бы создателю уже никогда, молча. Способ выхода из +беды поэтому другой: восстановить базу из резервной копии либо выпустить новый +релиз вперёд. + +## Open Questions + +**Обязательна ли колонка владельца в схеме?** Решает человек на чекпоинте. +Критерий приёмки задачи требует «обязательна и без умолчания», рамки той же +задачи запрещают назначать владельца записям бота. Три способа: + +1. **Колонка не обязательна в схеме, обязательность держит приём по HTTP.** + Запись из веба без владельца завести нельзя — отказывает приём; запись бота + лежит без владельца и по API невидима никому. Цена: «обязательна» держит код, + а не схема, и прямое обращение к базе мимо приёма может завести запись без + владельца. Критерий приёмки выполняется наполовину и требует переформулировки. + **Рекомендую этот.** +2. **Колонка обязательна, записи бота получают служебную учётную запись.** + Схема держит обязательность целиком. Цена втрое больше, чем казалось: + учётная запись, за которой не стоит человек, делает сервис источником учётных + записей — а паспорт объявляет управление ими вне цели; завести её штатным + путём нельзя вовсе, потому что создание записи разрешено только контексту + обмена OIDC, и понадобится ещё один шаг схемы; после + `telegram-account-link` записи придётся переназначать, нарушая неизменяемость + владельца в первой же следующей задаче. **Способ рекомендую снять с выбора.** +3. **Колонка обязательна, связь чата с учётной записью делается здесь же.** + Критерий выполняется буквально. Цена: задача вбирает в себя следующую и + вырастает примерно вдвое — а `telegram-account-link` заведена отдельной + строкой беклога намеренно. + +**Сужается ли по владельцу сам файл записи?** Нашли независимо все три прохода +ревью дизайна, и это крупнее прочего. Разграничение, спроектированное выше, +закрывает опрос состояния — то есть метаданные задачи, — а само аудио лежит в +другой коллекции, у которой владельца нет и по этому плану не появится. Правило +просмотра там пускает всякого вошедшего, и знание идентификатора файловой записи +по-прежнему равно праву скачать чужую запись. Запись задачи между тем называет +затронутой «таблицу задач **и таблицу файлов**», а модель угроз обещает владельца +у задачи и файла именно этой задачей. + +Дороже всего не сама дыра, а отметка о закрытии: после мерджа паспорт и модель +угроз перепишут разграничение как сделанное, и открытый путь перестанет быть +виден. Способы: + +1. **Владелец и у файла, тем же шагом схемы**, правило просмотра сужается им. + Задача растёт на колонку, правило и свой тест. Полное закрытие. +2. **Обратная ссылка с файла на задачу**, владелец выводится через неё. Дешевле в + схеме, дороже в правиле; вдобавок ссылка на файл в задаче **переставляется** + каждым шагом конвейера, и исходная копия после конвертации не связана ни с чем. +3. **Оставить как есть и записать границу честно** — строкой Non-Goals с адресом + задачи-преемника, и не переписывать паспорт с моделью угроз словом «закрыто». + Дешевле всего сегодня, но разграничение остаётся половинчатым. + +**Что делать с записями удалённого пользователя?** Связь при выключенном каскаде +снимает ссылку — записи остаются, но становятся ничьими и недостижимыми по API +навсегда. Способы: запретить удаление учётной записи, пока у неё есть записи; +держать рядом со связью неизменяемый снимок идентификатора; признать потерю +ценой и записать её. Сегодня удаления пользователей в сервисе нет вовсе, так что +третий способ ничего не ломает сейчас — но он же превращает архив в то, что +теряется одной кнопкой в панели. + +**Подтверждение ломки публичного контракта.** `GET /api/status/{id}` на чужую +задачу начнёт отвечать `404` там, где отдавал содержимое. Проект относит +публичный контракт HTTP API к необратимому и требует спрашивать человека всегда — +здесь и спрашивается. diff --git a/openspec/changes/archive/2026-08-14-record-ownership/proposal.md b/openspec/changes/archive/2026-08-14-record-ownership/proposal.md new file mode 100644 index 0000000..80d9e13 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/proposal.md @@ -0,0 +1,61 @@ +## Why + +Сегодня знание идентификатора задачи и есть право её читать: всякий вошедший +видит любую расшифровку по её идентификатору — свою, соседа, чью угодно. Паспорт +обещает приглашённому пользователю обратное: «каждый видит только свои записи». + +Разграничение стоит первым в очереди не само по себе. На владельце записи стоят +список записей, дедупликация, удаление, учёт расхода и квота — всё, что проект +собирается делать дальше. Заводить их поверх общей кучи записей значит переделать +каждое из них потом. + +## What Changes + +- У записи появляется **владелец** — учётная запись, от имени которой её + завели. Назначается он один раз, при приёме, и больше не меняется. +- **Опрос готовности отдаёт только свои записи.** Чужая запись по её + идентификатору отвечает «не найдено» — тем же ответом, что и несуществующая. + Отдельного «доступ запрещён» нет намеренно: по нему перебирается список + заведённых задач. +- **Приём по HTTP заводит владельца** из предъявленной сессии. Приём без сессии + отказывал и раньше, и это не меняется. +- **Конвейер владельцем не сужается**: расшифровка идёт для всех записей подряд, + как и прежде. Владелец решает, кому запись показывать, а не кому её считать. +- **BREAKING** для содержимого базы: колонка владельца добавляется шагом схемы, и + записи, заведённые до него, владельца не получают. Данные не переносим — + проект заводится с чистого листа, так решено рамками задачи. + +Записи, пришедшие ботом, владельца здесь не получают: связи чата Telegram с +учётной записью приложения ещё нет, её заводит следующая задача. Что это значит +для обязательности колонки — развилка, вынесенная в design.md и на чекпоинт. + +## Capabilities + +### New Capabilities + +Новых нет: разграничение доступа — это поведение уже заведённой `access`, а не +новый домен. + +### Modified Capabilities + +- `access`: появляется владелец записи и правило «чужая запись неотличима от + несуществующей». Сегодня спека прямо говорит обратное — «разграничения записей + по владельцу здесь нет». +- `intake`: приём назначает владельца принятой записи, опрос готовности сужается + владельцем. Обе строки спеки, обещающие обратное, снимаются. +- `pipeline`: записывается то, что до сих пор было умолчанием, — выборка задачи + воркером владельцем не сужается. +- `storage`: у задачи в схеме появляется колонка владельца, и заводится она без + умолчания. + +## Impact + +- схема хранилища: новый шаг с колонкой владельца в таблице задач; `docs/database.md`; +- `internal/entity`, `internal/contract`: у задачи появляется владелец, чтение + сужается им; +- `internal/adapter/repo/pocketbase`: четыре места правки колонок очереди — + `applyToRecord`, `recordToJob`, `acquireColumns`, `acquiredRow`; +- `internal/service`: оба метода заведения задачи; +- `internal/controller/http`: опрос готовности и приём; +- публичный контракт `GET /api/status/{id}`: чужая запись начинает отвечать + `404` там, где прежде отдавала содержимое. Форма ответа не меняется. diff --git a/openspec/changes/archive/2026-08-14-record-ownership/review/report.md b/openspec/changes/archive/2026-08-14-record-ownership/review/report.md new file mode 100644 index 0000000..d143cd3 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/review/report.md @@ -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` в +отчёте нет потому, что ни одна находка не собрала оракула на этот уровень, а не +потому, что путей туда нет. diff --git a/openspec/changes/archive/2026-08-14-record-ownership/specs/access/spec.md b/openspec/changes/archive/2026-08-14-record-ownership/specs/access/spec.md new file mode 100644 index 0000000..2f13496 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/specs/access/spec.md @@ -0,0 +1,61 @@ +## ADDED Requirements + +### Requirement: У записи есть владелец, и чужую ей не отдают + +Сервис SHALL заводить у каждой записи, принятой **по HTTP**, — владельца, то +есть учётную запись, от имени которой запись принята, — и MUST отдавать данные +такой записи только её владельцу. Владелец назначается один раз, при приёме, и +MUST не меняться у записи, у которой владелец есть: совместного доступа, ролей и +передачи записи другому сервис не знает. Оговорка не случайна — назначить +владельца записи, у которой его нет, вправе задача, заводящая связь чата +Telegram с учётной записью. + +Владелец MUST браться из предъявленной сессии и ниоткуда больше. Владелец, +пришедший полем запроса, дал бы всякому вошедшему право завести запись на чужое +имя. + +Обращение к чужой записи MUST быть неотличимо от обращения к несуществующей. +Отдельный отказ «доступ запрещён» превращает опрос в перебор — по разнице +ответов считывается, какие записи заведены, а идентификатор записи и есть то, +что разграничение прячет. Каким именно ответом это выражено, нормирует +capability `intake`: там живёт адрес опроса, и держатель нормы обязан быть один. + +Пустой владелец MUST не совпадать ни с одной записью — ни со своей, ни с чужой, +ни с ничьей. Правило записано со стороны **спрашивающего**, а не со стороны +записи: обязательность владельца, которую держит одна лишь подпись метода, пустую +строку пропускает, и первый же вызывающий без учётной записи получил бы ровно +множество записей без владельца, то есть все записи бота. + +Записи, принятые из Telegram, владельца не имеют: связи чата с учётной записью +приложения сервис не ведёт. Такая запись MUST не доставаться по API никому — +ответ на неё тот же, что и на несуществующую, — а её расшифровка уезжает +отправителю в чат, как и прежде. + +#### Scenario: Своя запись доступна + +- **GIVEN** человек вошёл и принял запись +- **WHEN** он спрашивает состояние этой записи своей сессией +- **THEN** ответ несёт состояние записи + +#### Scenario: Чужая запись неотличима от несуществующей + +- **GIVEN** запись принята одним вошедшим +- **WHEN** её состояние спрашивает другой вошедший +- **THEN** ответ тот же, что и на неизвестный идентификатор, — и кодом, и телом + +#### Scenario: Владельца не задают запросом + +- **WHEN** запрос на приём записи несёт своё значение владельца +- **THEN** владельцем принятой записи становится предъявитель сессии + +#### Scenario: Запись из Telegram не достаётся по API + +- **GIVEN** запись принята ботом +- **WHEN** её состояние спрашивает вошедший человек +- **THEN** ответ тот же, что и на неизвестный идентификатор + +#### Scenario: Пустой владелец не открывает ничего + +- **GIVEN** заведены три задачи: своя, чужая и принятая ботом +- **WHEN** состояние каждой спрашивают с пустым владельцем +- **THEN** ответ на все три тот же, что и на неизвестный идентификатор diff --git a/openspec/changes/archive/2026-08-14-record-ownership/specs/intake/spec.md b/openspec/changes/archive/2026-08-14-record-ownership/specs/intake/spec.md new file mode 100644 index 0000000..e664950 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/specs/intake/spec.md @@ -0,0 +1,136 @@ +## MODIFIED Requirements + +### Requirement: Приём записи по HTTP + +Сервис SHALL принимать запись от внешней программы запросом `POST /api/audio` с +телом `multipart/form-data` и полем `audio` **только от узнанного отправителя**. +Запрос без сессии MUST получать код `401`, и по нему MUST не заводиться ни файл, +ни задача расшифровки. Принятая запись от узнанного отправителя MUST быть +сохранена и получить заведённую под неё задачу расшифровки в состоянии +`created`; ответ MUST нести идентификатор задачи полем `job_id` и её состояние +полем `status`. + +Отказ по отсутствию сессии наступает **раньше** чтения тела: запись, за которую +не заплатит узнанный отправитель, не должна попасть даже в память. + +Имена полей ответа нормативны: контракт HTTP API объявлен проектом необратимым, +и переименование поля ломает внешнюю программу молча. Появление отказа без +сессии — намеренная ломка этого контракта: до неё приём стоял открытым наружу. + +Приём не судит о годности записи сам: расширение он берёт из имени файла, а +пригодность содержимого узнаёт у источника метаданных. + +Куда именно ложится принятая запись, приёму не принадлежит: раскладку выбирает +хранилище, и нормирует её capability `storage`. + +Владельцем принятой записи приём SHALL назначать предъявителя сессии. Проверка +стоит здесь, а не только в схеме хранилища: колонка владельца допускает пустое +значение ради записей из Telegram, и приём по HTTP — то место, где +обязательность держится. + +Предъявитель, чья сессия не даёт учётной записи пользователя, MUST получать +отказ `403` и MUST получать его **до чтения тела** — там же, где стоит отказ по +отсутствию сессии. Сессия владельца панели — именно такой случай: узнан он всё +же узнан, а записи в коллекции пользователей у него нет, и владельцем записи он +стать не может. + +Код здесь другой, чем у запроса без сессии, и это не оплошность: `401` значит +«предъяви себя», а предъявитель себя предъявил. Утечки по разнице кодов нет — +оба ответа говорят о самом спрашивающем, а не о том, какие записи заведены. + +Отказ **после** укладки записи потребовал бы убрать уже сохранённый файл, а +уборки файлов сервис не умеет вовсе: норма, обязывающая к недостижимому, не +пишется. + +#### Scenario: Запись принята + +- **GIVEN** источник метаданных читает запись и отдаёт её длительность +- **AND** отправитель предъявил сессию +- **WHEN** программа шлёт `POST /api/audio` с полем `audio` +- **THEN** ответ имеет код `201`, а в теле лежат непустой `job_id` и `status` + со значением `created` +- **AND** содержимое записи целиком лежит в хранилище одним файлом +- **AND** владельцем заведённой задачи стоит предъявитель сессии + +#### Scenario: Сессия не даёт учётной записи пользователя + +- **GIVEN** предъявлена сессия владельца панели +- **WHEN** он шлёт `POST /api/audio` с полем `audio` +- **THEN** ответ имеет код `403` +- **AND** ни файла, ни задачи не заводится + +#### Scenario: Сессии нет + +- **WHEN** программа шлёт `POST /api/audio` с полем `audio` без сессии +- **THEN** ответ имеет код `401` +- **AND** ни файла, ни задачи не заводится +- **AND** тело ответа не несёт данных задачи + +#### Scenario: Поля с записью нет + +- **GIVEN** отправитель предъявил сессию +- **WHEN** программа шлёт `POST /api/audio` без поля `audio` +- **THEN** ответ имеет код `400` и сообщение об отсутствии записи +- **AND** ни файла, ни задачи не заводится + +#### Scenario: Размеру записи приём не судья + +- **GIVEN** источник метаданных читает запись и отдаёт её длительность +- **AND** отправитель предъявил сессию +- **WHEN** программа шлёт запись нулевой длины +- **THEN** ответ имеет код `201`: собственного порога по размеру у приёма нет + +### Requirement: Опрос готовности задачи + +Сервис SHALL отдавать состояние задачи расшифровки по запросу +`GET /api/status/:id` **только её владельцу**. Запрос без сессии MUST получать +код `401`, и тело такого ответа MUST не нести ни состояния задачи, ни текста +расшифровки. Ответ владельцу MUST нести идентификатор полем `job_id`, состояние +полем `status` и время заведения полем `created_at`, а текст расшифровки полем +`transcription_text`, и это поле MUST отсутствовать в ответе, пока текста нет: +пустая строка на месте отсутствующего текста читается как «расшифровка пуста». + +Отказ без сессии MUST не зависеть от того, есть такая задача или нет: иначе по +кодам ответа перебирается список заведённых задач. + +Задача, принадлежащая другому, MUST отвечать тем же, чем отвечает неизвестный +идентификатор, — кодом `404` и тем же телом. То же MUST относиться к задаче без +владельца: запись, принятая ботом, по этому адресу не достаётся никому. + +#### Scenario: Задача найдена + +- **GIVEN** отправитель предъявил сессию +- **WHEN** он спрашивает состояние своей задачи +- **THEN** ответ имеет код `200` и несёт `job_id`, `status` и `created_at` + +#### Scenario: Сессии нет + +- **WHEN** программа спрашивает состояние заведённой задачи без сессии +- **THEN** ответ имеет код `401` +- **AND** тело ответа не несёт ни состояния задачи, ни текста расшифровки + +#### Scenario: Без сессии неизвестная задача неотличима от заведённой + +- **WHEN** программа без сессии спрашивает состояние заведённой задачи, а затем + состояние по неизвестному идентификатору +- **THEN** оба ответа имеют код `401` + +#### Scenario: Чужая задача неотличима от неизвестной + +- **GIVEN** задача заведена одним вошедшим +- **WHEN** её состояние спрашивает другой вошедший +- **THEN** ответ имеет код `404` и то же тело, что и ответ по неизвестному + идентификатору +- **AND** тело ответа не несёт ни состояния задачи, ни текста расшифровки + +#### Scenario: Расшифровки ещё нет + +- **GIVEN** отправитель предъявил сессию +- **WHEN** он спрашивает состояние своей задачи, которая ещё не дошла до текста +- **THEN** поля `transcription_text` в ответе нет вовсе + +#### Scenario: Задачи с таким идентификатором нет + +- **GIVEN** отправитель предъявил сессию +- **WHEN** программа спрашивает состояние по неизвестному идентификатору +- **THEN** ответ имеет код `404` и сообщение о ненайденной задаче diff --git a/openspec/changes/archive/2026-08-14-record-ownership/specs/pipeline/spec.md b/openspec/changes/archive/2026-08-14-record-ownership/specs/pipeline/spec.md new file mode 100644 index 0000000..0376c4e --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/specs/pipeline/spec.md @@ -0,0 +1,32 @@ +## ADDED Requirements + +### Requirement: Выборка воркера владельцем не сужается + +Воркер SHALL брать задачи всех владельцев подряд и MUST не учитывать владельца +при выборе очередной задачи. Задача без владельца — принятая ботом — MUST +обрабатываться наравне с прочими. + +Владелец решает, кому запись показывать, а не кому её считать. Сужение выборки +владельцем остановило бы расшифровку записей бота вовсе, а записи остальных +поставило бы в зависимость от того, кто первым завёл учётную запись. + +Владелец задачи MUST переживать работу конвейера: шаг, сохраняющий свой +результат, владельца не трогает и не затирает. + +#### Scenario: Задачи двух владельцев проходят одним воркером + +- **GIVEN** заведены задачи двух разных владельцев в одном состоянии +- **WHEN** воркер забирает задачи этого состояния +- **THEN** ему достаются обе, в порядке заведения + +#### Scenario: Задача без владельца обрабатывается + +- **GIVEN** заведена задача, принятая ботом, — без владельца +- **WHEN** воркер забирает задачи её состояния +- **THEN** она достаётся ему наравне с прочими + +#### Scenario: Шаг конвейера владельца не затирает + +- **GIVEN** задача с владельцем прошла шаг конвейера +- **WHEN** шаг сохраняет свой результат +- **THEN** владелец задачи остаётся прежним diff --git a/openspec/changes/archive/2026-08-14-record-ownership/specs/storage/spec.md b/openspec/changes/archive/2026-08-14-record-ownership/specs/storage/spec.md new file mode 100644 index 0000000..2ac1642 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/specs/storage/spec.md @@ -0,0 +1,199 @@ +## ADDED Requirements + +### Requirement: Владелец задачи лежит связью с учётной записью + +Хранилище SHALL держать владельца задачи расшифровки отдельной колонкой — связью +с учётной записью, — и эта колонка MUST не иметь умолчания: запись, чей владелец +не назван, не достаётся никому по недосмотру схемы. + +Колонка MUST допускать пустое значение, и это решение с названной ценой: записи, +принятые ботом, владельца не имеют, потому что связи чата Telegram с учётной +записью сервис не ведёт. Обязательность для приёма по HTTP держит сама +capability `intake`, а не схема. + +Колонка приезжает **новым шагом схемы**: применённый шаг не переписывается. +Записей, заведённых до этого шага, сервис не переносит — проект заводится с +чистого листа. + +#### Scenario: Колонка появляется на пустой базе + +- **WHEN** сервис поднимается на чистом каталоге данных +- **THEN** у таблицы задач есть колонка владельца +- **AND** умолчания у неё нет + +### Requirement: Файл записи сужается владельцем наравне с задачей + +Хранилище SHALL держать владельца и у файла записи — той же связью с учётной +записью, тем же шагом схемы, — и правило просмотра файлов MUST пускать к файлу +только его владельца. Прежнее правило пускало всякого узнанного, и знание +идентификатора файловой записи равнялось праву скачать чужое аудио. + +Без этого требования разграничение закрывает метаданные задачи и оставляет +открытым содержимое — то самое, что оно и заведено прятать. Хуже самой дыры была +бы отметка о закрытии: паспорт и модель угроз называют исполнителем этой работы +именно эту задачу, и слово «закрыто» скрыло бы открытый путь. + +Владелец файла MUST назначаться там же, где владелец задачи, — при приёме, из +предъявленной сессии, — и MUST оставаться пустым у файлов, заведённых конвейером +для записи без владельца. + +Ссылка на файл в задаче переставляется каждым шагом конвейера, поэтому владелец +файла MUST лежать своей колонкой, а не выводиться через задачу: исходная копия +после конвертации не связана с задачей ничем. + +Отказ наступает **на переходе по ссылке**, а не на выдаче токена файла: токен +хранилище выдаёт на предъявителя, а не на файл, и о файле при выдаче не +спрашивает вовсе. Требовать отказа при выдаче значит требовать механизма, +которого нет, — а проверка, написанная под такое требование, зеленела бы, не +касаясь пути, по которому аудио и уходит. + +#### Scenario: Чужой файл не отдаётся + +- **GIVEN** запись принята одним вошедшим +- **WHEN** другой вошедший идёт по ссылке на файл этой записи со своим токеном +- **THEN** содержимого он не получает + +#### Scenario: Свой файл отдаётся + +- **GIVEN** человек принял запись +- **WHEN** он идёт по ссылке на файл своей записи со своим токеном +- **THEN** содержимое отдаётся + +#### Scenario: Файл записи из Telegram не отдаётся по API + +- **GIVEN** запись принята ботом, и владельца у неё нет +- **WHEN** вошедший человек идёт по ссылке на её файл со своим токеном +- **THEN** содержимого он не получает + +### Requirement: Учётная запись с записями не удаляется + +Хранилище SHALL отвергать удаление учётной записи, у которой остались задачи +расшифровки **либо файлы**. Отказ MUST называть причину, и MUST доезжать до +спрашивающего: хранилище пропускает наружу только свою ошибку роутера, а всякую +другую подменяет сообщением про обязательную связь — подсказкой, по которой +владелец панели пойдёт удалять записи руками. + +Считаются обе коллекции с владельцем. Файл переживает свою задачу: шаг конвейера +заводит его до сохранения задачи, и потерянный захват оставляет файл с владельцем +и без ссылки. + +Запрет MUST ставить сама сборка хранилища, а не вызывающий: сборка, забывшая его +позвать, теряет защиту молча — и теряла, пока запрет вешался отдельной строкой +запуска, а окружение проверок его не ставило вовсе. + +Удаление при этом не только панельное: умолчание библиотеки разрешает вошедшему +удалить **свою** учётную запись запросом, так что запрет закрывает и публичную +поверхность. + +Требование заведено вместо прежнего «удаление не уносит задачи следом»: оно +выглядело выполненным, а на деле хранилище при выключенном каскаде **снимает +ссылку** — задачи остаются, но становятся ничьими, а ничья задача не достаётся +по API никому. Архив человека исчезал бы молча и восстановлению не подлежал: +прежнего владельца не остаётся нигде. + +Цена требования названа прямо: владелец панели упирается в отказ, а способа +удалить записи в сервисе пока нет вовсе — его приносит задача про удаление +записи. До неё удаление учётной записи с записями невозможно, и это осознанный +тупик, а не недосмотр. + +#### Scenario: Удаление учётной записи с записями отвергается + +- **GIVEN** у учётной записи есть задачи расшифровки +- **WHEN** её удаляют +- **THEN** удаление не проходит, а отказ называет причину +- **AND** задачи и их владелец остаются прежними + +#### Scenario: Учётная запись с одними файлами тоже не удаляется + +- **GIVEN** у учётной записи остались файлы, но задач нет +- **WHEN** её удаляют +- **THEN** удаление не проходит, а владелец файлов остаётся прежним + +#### Scenario: Учётная запись без записей удаляется + +- **GIVEN** у учётной записи нет ни задач, ни файлов +- **WHEN** её удаляют +- **THEN** удаление проходит + +## MODIFIED Requirements + +### Requirement: Файл отдаётся ссылкой + +Сервис SHALL отдавать файл записи ссылкой, которую строит хранилище по самой +записи, **и только узнанному отправителю**. Поле файла MUST быть помечено +защищённым: без этого ссылка открывает запись любому, кто её знает, и знание +ссылки становится правом. Отданный файл MUST совпадать с принятым по длине. + +Одной пометки мало: защищённый файл судится **коротким токеном файла**, который +узнанный отправитель берёт у хранилища, предъявив сессию, — и правилом просмотра +коллекции. Правило MUST пускать только владельца файла: незаданное означает +«только владелец панели», и тогда файла не получит и вошедший, а прежнее «всякий +узнанный» отдавало чужое аудио тому, кто знает идентификатор записи. + +Токен файла хранилище выдаёт **на предъявителя**, а не на файл, и о файле при +выдаче не спрашивает. Значит владельца судит переход по ссылке, а не выдача +токена: отказ наступает там, и требовать его от выдачи значит требовать +механизма, которого нет. + +Отсюда порядок для потребителя: сессия → токен файла → ссылка с этим токеном. +Браузер с одной лишь кукой файла не получит, и это свойство хранилища, а не +недосмотр. + +Конвейер расшифровки этим не затронут: он читает файл из файловой системы +хранилища, а не по ссылке. + +Ссылка на несуществующую запись MUST отвечать отказом, а не пустым файлом. + +**Ссылка сама по себе и есть право пройти по ней**, и потому она MUST не попадать +ни в журнал, ни в метку метрики, ни в ответ отправителю. Имя, под которым файл +лёг в хранилище, из журнала выводимо быть не должно: журнал уезжает в собранные +логи, откуда строку не убрать, и оттуда ссылка на чужую запись работала бы +бессрочно. + +Защищённое поле сужает это право, но не отменяет запрета: право пройти теперь +требует ещё и сессии, а строка журнала со ссылкой по-прежнему собирала бы +половину ключа. + +Отсюда требование к отказам: сообщение об отказе хранилища MUST не выходить за +пределы хранилища дословно. Отказ чтения и отказ укладки называют ключ файла +целиком, а отказ выгрузки во внешнее хранилище — полный адрес объекта; и то и +другое кончается в журнале и собирает ссылку не хуже успешного пути. + +Что именно журнал приёма пишет ради прослеживаемости, нормирует capability +`intake`. + +#### Scenario: Файл забирают по ссылке + +- **GIVEN** запись принята и её файл лежит в хранилище +- **AND** забирающий предъявил сессию и взял по ней токен файла +- **WHEN** ссылку на файл запрашивают с этим токеном +- **THEN** приходит тот же файл, и его длина совпадает с длиной принятого + +#### Scenario: Без сессии файл не отдаётся + +- **GIVEN** запись принята и её файл лежит в хранилище +- **WHEN** ссылку на файл запрашивают без сессии +- **THEN** приходит отказ, а содержимого записи в ответе нет + +#### Scenario: Конвейер читает файл без сессии + +- **GIVEN** запись принята и ждёт расшифровки +- **WHEN** шаг конвейера берётся за неё +- **THEN** файл читается из файловой системы хранилища и шаг проходит + +#### Scenario: Ссылка ведёт в никуда + +- **WHEN** запрашивают ссылку на запись, которой нет +- **THEN** приходит отказ, а не пустой ответ + +#### Scenario: По журналу ссылку не собрать + +- **GIVEN** запись принята и прошла конвейер +- **WHEN** читают журнал сервиса целиком +- **THEN** имени, под которым файл лёг в хранилище, в нём нет + +#### Scenario: Отказ чтения файла не называет его ключ + +- **GIVEN** файл записи не читается из хранилища +- **WHEN** шаг конвейера берётся за эту запись и отказывает +- **THEN** отказ называет запись её идентификатором и не несёт имени файла diff --git a/openspec/changes/archive/2026-08-14-record-ownership/tasks.md b/openspec/changes/archive/2026-08-14-record-ownership/tasks.md new file mode 100644 index 0000000..96dcca8 --- /dev/null +++ b/openspec/changes/archive/2026-08-14-record-ownership/tasks.md @@ -0,0 +1,130 @@ +## Критерии приёмки задачи + +Дословно из `tasks/items/record-ownership.md`. Файл задачи закрытие удалит — +критерии обязаны его пережить. + +- Запрос чужой записи по её идентификатору возвращает «не найдено», а не + содержимое и не «доступ запрещён». Оракул — тест: две сессии, задача первой + запрашивается второй, ответ 404 и пустое тело. + *Расхождение:* «пустое тело» разошлось с нетронутым сценарием спеки, где ответ + по неизвестному идентификатору несёт сообщение о ненайденной задаче. Читается + как «то же тело, что и у неизвестного идентификатора, и без состояния и текста + расшифровки» — иначе реализация по букве критерия молча снимет действующее + требование о сообщении. +- Запись, заведённая из веба, принадлежит вошедшему. Оракул — тест приёма по + HTTP со сверкой колонки владельца. +- Выборка воркера владельцем **не** сужается: конвейер обрабатывает записи всех. + Оракул — тест: задачи двух владельцев проходят конвейер одним воркером. +- База заводится с чистого листа, колонка владельца обязательна и без умолчания. + Оракул — прогон миграций на пустой базе и попытка вставки без владельца. + +**Четвёртый критерий разошёлся с рамками той же задачи** и правится решением +человека на чекпоинте: рамки запрещают назначать владельца записям из Telegram, а +обязательная колонка такую запись вставить не даст. Разобрано в `design.md`, +«Open Questions»; ниже план написан по рекомендованному способу — колонка без +умолчания, допускающая пустое значение, обязательность приёма по HTTP держит код. + +## Рубрика ревью дизайна — приёмочные критерии + +Порождена проходом `rubric` до чтения артефактов. Приёмка судится по одному +списку: этот блок и блок выше. + +1. Чужая запись неотличима от несуществующей на всех наблюдаемых осях: тот же + код, то же тело, та же форма ответа. +2. Сужение живёт в одном месте пути чтения и обязательно к употреблению: второго + читающего входа, у которого сужение можно не позвать, нет. +3. Пустой владелец на входе чтения не совпадает ни с одной записью — своей, + чужой и ничьей. +4. Записи без владельца недостижимы сужённым путём никому, и недостижимость + выведена из правила, а не из того, что таких записей мало. +5. Владелец назначается сервером из сессии: поле владельца, пришедшее запросом, + на результат не влияет. +6. Владелец появляется в той же операции, что и запись; отказ по отсутствию + владельца не оставляет ни задачи, ни файла, и способ этого назван. +7. Неизменность владельца держит механизм, а не обещание: назван каждый путь, + которым владельца можно переписать, и что его удерживает. +8. Колонка прочитана всеми четырьмя местами правки колонок очереди плюс шагом + схемы. +9. Воркер владельцем не сужается, и это записано нормой, а не оставлено + умолчанию. +10. Судьба записи при исчезновении владельца определена. +11. Шаг схемы: одно представление «владельца нет», откат не требует переписывания + применённого шага. +12. Проверка владельца и чтение состояния берутся из одного чтения записи, а не + двумя раздельными. +13. Ответ отправителю адресуется по источнику записи, а не по владельцу: запись + без владельца получает свой ответ в чат. +14. Ни ответ, ни журнал не выдают того, что прячет разграничение; различение + чужой и несуществующей в журнале допустимо. + +## 1. Схема хранилища + +- [x] 1.1 Новый шаг схемы `202608140001_record_owner.go`: колонка `owner` в + таблице задач связью с коллекцией `users`, без умолчания, пустое значение + допустимо, каскадное удаление выключено. Имя колонки — `owner`, поле + сущности — `OwnerID`; имена названы здесь, потому что разойтись им есть где + — семь мест плюс подпись метода +- [x] 1.2 Тем же шагом — колонка `owner` в таблице файлов, теми же свойствами; + правило просмотра коллекции файлов сужается владельцем вместо прежнего + «всякий вошедший» +- [x] 1.3 Тем же шагом — запрет удаления учётной записи, у которой остались + задачи: отказ с причиной, а не снятие ссылки +- [x] 1.4 Прежние шаги схемы не тронуты — проверяется шагом гейта `migrations` +- [x] 1.5 `docs/database.md`: строки колонок в обеих таблицах, правило выборки по + владельцу, новое правило просмотра файлов и запрет удаления учётной записи + +## 2. Сущность и контракт + +- [x] 2.1 `internal/entity`: у задачи расшифровки появляется владелец +- [x] 2.2 `internal/contract`: читающий метод репозитория задач принимает + владельца; второго читающего метода не заводится + +## 3. Хранилище задач + +- [x] 3.1 `applyToRecord` кладёт владельца при заведении +- [x] 3.2 `recordToJob` читает владельца +- [x] 3.3 `acquireColumns` и `acquiredRow` читают колонку владельца +- [x] 3.4 `applyOwnedByPipeline` владельца **не** трогает +- [x] 3.5 Чтение задачи сужено владельцем: задача другого владельца и задача без + владельца отдают ту же ошибку, что и несуществующая + +## 4. Приём и опрос + +- [x] 4.1 `internal/service`: метод заведения задачи из веба принимает владельца + и отказывает при пустом; метод заведения из Telegram владельца не + назначает. Владелец кладётся и на файл, заводимый при приёме +- [x] 4.2 `internal/controller/http`: приём берёт владельца из предъявленной + сессии +- [x] 4.3 `internal/controller/http`: опрос готовности передаёт владельца в + хранилище и отвечает `404` с прежним телом на чужую и на ничью задачу + +## 5. Проверки + +- [x] 5.1 Тест: две сессии, задача первой запрашивается второй — `404`, тело без + состояния и текста +- [x] 5.2 Тест: приём по HTTP заводит задачу с владельцем-предъявителем +- [x] 5.3 Тест: приём по HTTP без узнанной учётной записи задачи не заводит +- [x] 5.4 Тест: задачи двух владельцев и задача без владельца проходят конвейер + одним воркером +- [x] 5.5 Тест: шаг конвейера, сохраняющий результат, владельца не затирает +- [x] 5.6 Тест: миграции на пустой базе заводят колонку без умолчания +- [x] 5.7 Тест: чтение с пустым владельцем не отдаёт ни своей, ни чужой, ни + ничьей задачи +- [x] 5.8 Тест: сессия без учётной записи пользователя получает `403` на приёме, + файла и задачи не заводится — узнан, но не запись коллекции пользователей +- [x] 5.9 Тест: вошедший просит токен чужого файла — отказ; своего — успех +- [x] 5.10 Тест: удаление учётной записи с задачами отвергается, без задач — + проходит +- [x] 5.11 `task gate` зелёный + +## 6. Архивация + +- [ ] 6.1 На архивации выправить `## Purpose` спеки `access`: преамбула + переживает слияние дельт дословно и сегодня утверждает, что разграничения + по владельцу нет. Валидатор преамбулу не судит — вспомнить об этом больше + некому +- [ ] 6.2 Сверить `docs/security.md`, `docs/passport.md` и **`docs/architecture.md`** + (строка «Разграничения записей по владельцу здесь нет»): все три обещают + закрытие разграничением именно этой задачей. Адрес в `architecture.md` + назван отдельно — его нашло ревью, и без него обзор архитектуры отправлял + бы следующего читателя чинить уже закрытое diff --git a/openspec/specs/access/spec.md b/openspec/specs/access/spec.md index 1ce8207..2fa482d 100644 --- a/openspec/specs/access/spec.md +++ b/openspec/specs/access/spec.md @@ -6,15 +6,17 @@ OIDC, чем предъявляется сессия, что её прекращает и какие адреса остаются открытыми. -Разграничения записей по владельцу здесь **нет**: всякий вошедший видит ровно -то же, что видел прежде аноним. Его заводит отдельная задача, и до неё сессия -отвечает только на вопрос «узнан ли пришедший», а не «чьё он смотрит». +Здесь же разграничение записей по владельцу: с 2026-08-14 сессия отвечает не +только на вопрос «узнан ли пришедший», но и на «чьё он смотрит». Запись из веба +принадлежит тому, кто её принёс, и чужая неотличима от несуществующей. + +Записи, принятые ботом, владельца не имеют вовсе и по API не достаются никому: +связи чата Telegram с учётной записью приложения сервис не ведёт, её заводит +отдельная задача. Вход из Telegram эта capability не нормирует: бот проверяет отправителя своим белым списком, и с учётной записью приложения тот список не связан. - ## Requirements - ### Requirement: Вход через внешнего провайдера Сервис SHALL заводить сессию только по итогу входа у внешнего провайдера OIDC. @@ -345,3 +347,64 @@ MUST не делать. Кто допущен, определяет правил - **WHEN** сервис поднимается с пустым или негодным ключом секции входа - **THEN** старт кончается отказом, а отказ называет имена ключей - **AND** значений этих ключей в отказе нет + +### Requirement: У записи есть владелец, и чужую ей не отдают + +Сервис SHALL заводить у каждой записи, принятой **по HTTP**, — владельца, то +есть учётную запись, от имени которой запись принята, — и MUST отдавать данные +такой записи только её владельцу. Владелец назначается один раз, при приёме, и +MUST не меняться у записи, у которой владелец есть: совместного доступа, ролей и +передачи записи другому сервис не знает. Оговорка не случайна — назначить +владельца записи, у которой его нет, вправе задача, заводящая связь чата +Telegram с учётной записью. + +Владелец MUST браться из предъявленной сессии и ниоткуда больше. Владелец, +пришедший полем запроса, дал бы всякому вошедшему право завести запись на чужое +имя. + +Обращение к чужой записи MUST быть неотличимо от обращения к несуществующей. +Отдельный отказ «доступ запрещён» превращает опрос в перебор — по разнице +ответов считывается, какие записи заведены, а идентификатор записи и есть то, +что разграничение прячет. Каким именно ответом это выражено, нормирует +capability `intake`: там живёт адрес опроса, и держатель нормы обязан быть один. + +Пустой владелец MUST не совпадать ни с одной записью — ни со своей, ни с чужой, +ни с ничьей. Правило записано со стороны **спрашивающего**, а не со стороны +записи: обязательность владельца, которую держит одна лишь подпись метода, пустую +строку пропускает, и первый же вызывающий без учётной записи получил бы ровно +множество записей без владельца, то есть все записи бота. + +Записи, принятые из Telegram, владельца не имеют: связи чата с учётной записью +приложения сервис не ведёт. Такая запись MUST не доставаться по API никому — +ответ на неё тот же, что и на несуществующую, — а её расшифровка уезжает +отправителю в чат, как и прежде. + +#### Scenario: Своя запись доступна + +- **GIVEN** человек вошёл и принял запись +- **WHEN** он спрашивает состояние этой записи своей сессией +- **THEN** ответ несёт состояние записи + +#### Scenario: Чужая запись неотличима от несуществующей + +- **GIVEN** запись принята одним вошедшим +- **WHEN** её состояние спрашивает другой вошедший +- **THEN** ответ тот же, что и на неизвестный идентификатор, — и кодом, и телом + +#### Scenario: Владельца не задают запросом + +- **WHEN** запрос на приём записи несёт своё значение владельца +- **THEN** владельцем принятой записи становится предъявитель сессии + +#### Scenario: Запись из Telegram не достаётся по API + +- **GIVEN** запись принята ботом +- **WHEN** её состояние спрашивает вошедший человек +- **THEN** ответ тот же, что и на неизвестный идентификатор + +#### Scenario: Пустой владелец не открывает ничего + +- **GIVEN** заведены три задачи: своя, чужая и принятая ботом +- **WHEN** состояние каждой спрашивают с пустым владельцем +- **THEN** ответ на все три тот же, что и на неизвестный идентификатор + diff --git a/openspec/specs/intake/spec.md b/openspec/specs/intake/spec.md index 8337b44..9acd2af 100644 --- a/openspec/specs/intake/spec.md +++ b/openspec/specs/intake/spec.md @@ -36,8 +36,24 @@ Telegram, дописывает его сюда. Куда именно ложится принятая запись, приёму не принадлежит: раскладку выбирает хранилище, и нормирует её capability `storage`. -Владельца у принятой записи приём не заводит: после входа видно ровно то же, что -видно было анонимно. +Владельцем принятой записи приём SHALL назначать предъявителя сессии. Проверка +стоит здесь, а не только в схеме хранилища: колонка владельца допускает пустое +значение ради записей из Telegram, и приём по HTTP — то место, где +обязательность держится. + +Предъявитель, чья сессия не даёт учётной записи пользователя, MUST получать +отказ `403` и MUST получать его **до чтения тела** — там же, где стоит отказ по +отсутствию сессии. Сессия владельца панели — именно такой случай: узнан он всё +же узнан, а записи в коллекции пользователей у него нет, и владельцем записи он +стать не может. + +Код здесь другой, чем у запроса без сессии, и это не оплошность: `401` значит +«предъяви себя», а предъявитель себя предъявил. Утечки по разнице кодов нет — +оба ответа говорят о самом спрашивающем, а не о том, какие записи заведены. + +Отказ **после** укладки записи потребовал бы убрать уже сохранённый файл, а +уборки файлов сервис не умеет вовсе: норма, обязывающая к недостижимому, не +пишется. #### Scenario: Запись принята @@ -47,6 +63,14 @@ Telegram, дописывает его сюда. - **THEN** ответ имеет код `201`, а в теле лежат непустой `job_id` и `status` со значением `created` - **AND** содержимое записи целиком лежит в хранилище одним файлом +- **AND** владельцем заведённой задачи стоит предъявитель сессии + +#### Scenario: Сессия не даёт учётной записи пользователя + +- **GIVEN** предъявлена сессия владельца панели +- **WHEN** он шлёт `POST /api/audio` с полем `audio` +- **THEN** ответ имеет код `403` +- **AND** ни файла, ни задачи не заводится #### Scenario: Сессии нет @@ -214,24 +238,24 @@ Telegram, дописывает его сюда. ### Requirement: Опрос готовности задачи Сервис SHALL отдавать состояние задачи расшифровки по запросу -`GET /api/status/:id` **только узнанному отправителю**. Запрос без сессии MUST -получать код `401`, и тело такого ответа MUST не нести ни состояния задачи, ни -текста расшифровки. Ответ узнанному отправителю MUST нести идентификатор полем -`job_id`, состояние полем `status` и время заведения полем `created_at`, а текст -расшифровки полем `transcription_text`, и это поле MUST отсутствовать в ответе, -пока текста нет: пустая строка на месте отсутствующего текста читается как -«расшифровка пуста». +`GET /api/status/:id` **только её владельцу**. Запрос без сессии MUST получать +код `401`, и тело такого ответа MUST не нести ни состояния задачи, ни текста +расшифровки. Ответ владельцу MUST нести идентификатор полем `job_id`, состояние +полем `status` и время заведения полем `created_at`, а текст расшифровки полем +`transcription_text`, и это поле MUST отсутствовать в ответе, пока текста нет: +пустая строка на месте отсутствующего текста читается как «расшифровка пуста». Отказ без сессии MUST не зависеть от того, есть такая задача или нет: иначе по кодам ответа перебирается список заведённых задач. -Выборку по владельцу опрос не сужает: узнанный отправитель видит любую задачу по -её идентификатору ровно как прежде. Сужение придёт отдельной задачей. +Задача, принадлежащая другому, MUST отвечать тем же, чем отвечает неизвестный +идентификатор, — кодом `404` и тем же телом. То же MUST относиться к задаче без +владельца: запись, принятая ботом, по этому адресу не достаётся никому. #### Scenario: Задача найдена - **GIVEN** отправитель предъявил сессию -- **WHEN** программа спрашивает состояние заведённой задачи +- **WHEN** он спрашивает состояние своей задачи - **THEN** ответ имеет код `200` и несёт `job_id`, `status` и `created_at` #### Scenario: Сессии нет @@ -246,10 +270,18 @@ Telegram, дописывает его сюда. состояние по неизвестному идентификатору - **THEN** оба ответа имеют код `401` +#### Scenario: Чужая задача неотличима от неизвестной + +- **GIVEN** задача заведена одним вошедшим +- **WHEN** её состояние спрашивает другой вошедший +- **THEN** ответ имеет код `404` и то же тело, что и ответ по неизвестному + идентификатору +- **AND** тело ответа не несёт ни состояния задачи, ни текста расшифровки + #### Scenario: Расшифровки ещё нет - **GIVEN** отправитель предъявил сессию -- **WHEN** программа спрашивает состояние задачи, которая ещё не дошла до текста +- **WHEN** он спрашивает состояние своей задачи, которая ещё не дошла до текста - **THEN** поля `transcription_text` в ответе нет вовсе #### Scenario: Задачи с таким идентификатором нет diff --git a/openspec/specs/pipeline/spec.md b/openspec/specs/pipeline/spec.md index 751b2a8..d90a682 100644 --- a/openspec/specs/pipeline/spec.md +++ b/openspec/specs/pipeline/spec.md @@ -15,7 +15,6 @@ done | failed`, отмена контекста посреди шага и ос требования на него не написаны, потому что требование без проверки — предположение, а не норма. Первая задача, которая трогает любое из перечисленного, дописывает его сюда. - ## Requirements ### Requirement: Пустой прогон воркера — не отказ @@ -325,3 +324,35 @@ MUST расти с числом её попыток до объявленног - **GIVEN** задача принята по HTTP - **WHEN** шаг конвейера доходит до ответа отправителю - **THEN** шаг завершается без отказа и без записи о недоставке + +### Requirement: Выборка воркера владельцем не сужается + +Воркер SHALL брать задачи всех владельцев подряд и MUST не учитывать владельца +при выборе очередной задачи. Задача без владельца — принятая ботом — MUST +обрабатываться наравне с прочими. + +Владелец решает, кому запись показывать, а не кому её считать. Сужение выборки +владельцем остановило бы расшифровку записей бота вовсе, а записи остальных +поставило бы в зависимость от того, кто первым завёл учётную запись. + +Владелец задачи MUST переживать работу конвейера: шаг, сохраняющий свой +результат, владельца не трогает и не затирает. + +#### Scenario: Задачи двух владельцев проходят одним воркером + +- **GIVEN** заведены задачи двух разных владельцев в одном состоянии +- **WHEN** воркер забирает задачи этого состояния +- **THEN** ему достаются обе, в порядке заведения + +#### Scenario: Задача без владельца обрабатывается + +- **GIVEN** заведена задача, принятая ботом, — без владельца +- **WHEN** воркер забирает задачи её состояния +- **THEN** она достаётся ему наравне с прочими + +#### Scenario: Шаг конвейера владельца не затирает + +- **GIVEN** задача с владельцем прошла шаг конвейера +- **WHEN** шаг сохраняет свой результат +- **THEN** владелец задачи остаётся прежним + diff --git a/openspec/specs/storage/spec.md b/openspec/specs/storage/spec.md index 2c856cf..d2a1842 100644 --- a/openspec/specs/storage/spec.md +++ b/openspec/specs/storage/spec.md @@ -10,7 +10,6 @@ Сознательно не описаны: перенос прежних данных — его нет по решению задачи `pocketbase-storage`; удаление записей и файлов — сервис объявлен архивом 2026-08-11, а удаление приносит задача `delete-record`. - ## Requirements ### Requirement: Сервис поднимается на чистом каталоге данных @@ -106,9 +105,14 @@ MUST завести свою схему и принимать записи об Одной пометки мало: защищённый файл судится **коротким токеном файла**, который узнанный отправитель берёт у хранилища, предъявив сессию, — и правилом просмотра -коллекции. Правило MUST пускать всякого узнанного: незаданное означает «только -владелец панели», и тогда файла не получит и вошедший. Сужения по владельцу -здесь нет — его заводит отдельная задача. +коллекции. Правило MUST пускать только владельца файла: незаданное означает +«только владелец панели», и тогда файла не получит и вошедший, а прежнее «всякий +узнанный» отдавало чужое аудио тому, кто знает идентификатор записи. + +Токен файла хранилище выдаёт **на предъявителя**, а не на файл, и о файле при +выдаче не спрашивает. Значит владельца судит переход по ссылке, а не выдача +токена: отказ наступает там, и требовать его от выдачи значит требовать +механизма, которого нет. Отсюда порядок для потребителя: сессия → токен файла → ссылка с этим токеном. Браузер с одной лишь кукой файла не получит, и это свойство хранилища, а не @@ -272,3 +276,118 @@ MUST завести свою схему и принимать записи об - **WHEN** сервис запускается снова - **THEN** приглашения завести владельца в журнале нет +### Requirement: Владелец задачи лежит связью с учётной записью + +Хранилище SHALL держать владельца задачи расшифровки отдельной колонкой — связью +с учётной записью, — и эта колонка MUST не иметь умолчания: запись, чей владелец +не назван, не достаётся никому по недосмотру схемы. + +Колонка MUST допускать пустое значение, и это решение с названной ценой: записи, +принятые ботом, владельца не имеют, потому что связи чата Telegram с учётной +записью сервис не ведёт. Обязательность для приёма по HTTP держит сама +capability `intake`, а не схема. + +Колонка приезжает **новым шагом схемы**: применённый шаг не переписывается. +Записей, заведённых до этого шага, сервис не переносит — проект заводится с +чистого листа. + +#### Scenario: Колонка появляется на пустой базе + +- **WHEN** сервис поднимается на чистом каталоге данных +- **THEN** у таблицы задач есть колонка владельца +- **AND** умолчания у неё нет + +### Requirement: Файл записи сужается владельцем наравне с задачей + +Хранилище SHALL держать владельца и у файла записи — той же связью с учётной +записью, тем же шагом схемы, — и правило просмотра файлов MUST пускать к файлу +только его владельца. Прежнее правило пускало всякого узнанного, и знание +идентификатора файловой записи равнялось праву скачать чужое аудио. + +Без этого требования разграничение закрывает метаданные задачи и оставляет +открытым содержимое — то самое, что оно и заведено прятать. Хуже самой дыры была +бы отметка о закрытии: паспорт и модель угроз называют исполнителем этой работы +именно эту задачу, и слово «закрыто» скрыло бы открытый путь. + +Владелец файла MUST назначаться там же, где владелец задачи, — при приёме, из +предъявленной сессии, — и MUST оставаться пустым у файлов, заведённых конвейером +для записи без владельца. + +Ссылка на файл в задаче переставляется каждым шагом конвейера, поэтому владелец +файла MUST лежать своей колонкой, а не выводиться через задачу: исходная копия +после конвертации не связана с задачей ничем. + +Отказ наступает **на переходе по ссылке**, а не на выдаче токена файла: токен +хранилище выдаёт на предъявителя, а не на файл, и о файле при выдаче не +спрашивает вовсе. Требовать отказа при выдаче значит требовать механизма, +которого нет, — а проверка, написанная под такое требование, зеленела бы, не +касаясь пути, по которому аудио и уходит. + +#### Scenario: Чужой файл не отдаётся + +- **GIVEN** запись принята одним вошедшим +- **WHEN** другой вошедший идёт по ссылке на файл этой записи со своим токеном +- **THEN** содержимого он не получает + +#### Scenario: Свой файл отдаётся + +- **GIVEN** человек принял запись +- **WHEN** он идёт по ссылке на файл своей записи со своим токеном +- **THEN** содержимое отдаётся + +#### Scenario: Файл записи из Telegram не отдаётся по API + +- **GIVEN** запись принята ботом, и владельца у неё нет +- **WHEN** вошедший человек идёт по ссылке на её файл со своим токеном +- **THEN** содержимого он не получает + +### Requirement: Учётная запись с записями не удаляется + +Хранилище SHALL отвергать удаление учётной записи, у которой остались задачи +расшифровки **либо файлы**. Отказ MUST называть причину, и MUST доезжать до +спрашивающего: хранилище пропускает наружу только свою ошибку роутера, а всякую +другую подменяет сообщением про обязательную связь — подсказкой, по которой +владелец панели пойдёт удалять записи руками. + +Считаются обе коллекции с владельцем. Файл переживает свою задачу: шаг конвейера +заводит его до сохранения задачи, и потерянный захват оставляет файл с владельцем +и без ссылки. + +Запрет MUST ставить сама сборка хранилища, а не вызывающий: сборка, забывшая его +позвать, теряет защиту молча — и теряла, пока запрет вешался отдельной строкой +запуска, а окружение проверок его не ставило вовсе. + +Удаление при этом не только панельное: умолчание библиотеки разрешает вошедшему +удалить **свою** учётную запись запросом, так что запрет закрывает и публичную +поверхность. + +Требование заведено вместо прежнего «удаление не уносит задачи следом»: оно +выглядело выполненным, а на деле хранилище при выключенном каскаде **снимает +ссылку** — задачи остаются, но становятся ничьими, а ничья задача не достаётся +по API никому. Архив человека исчезал бы молча и восстановлению не подлежал: +прежнего владельца не остаётся нигде. + +Цена требования названа прямо: владелец панели упирается в отказ, а способа +удалить записи в сервисе пока нет вовсе — его приносит задача про удаление +записи. До неё удаление учётной записи с записями невозможно, и это осознанный +тупик, а не недосмотр. + +#### Scenario: Удаление учётной записи с записями отвергается + +- **GIVEN** у учётной записи есть задачи расшифровки +- **WHEN** её удаляют +- **THEN** удаление не проходит, а отказ называет причину +- **AND** задачи и их владелец остаются прежними + +#### Scenario: Учётная запись с одними файлами тоже не удаляется + +- **GIVEN** у учётной записи остались файлы, но задач нет +- **WHEN** её удаляют +- **THEN** удаление не проходит, а владелец файлов остаётся прежним + +#### Scenario: Учётная запись без записей удаляется + +- **GIVEN** у учётной записи нет ни задач, ни файлов +- **WHEN** её удаляют +- **THEN** удаление проходит +