P0 upload: verify video ownership in complete and resumable handlers #46

Activity

1. SlavaKuntsov

   added

   phase:backlogBacklogBacklog

   area:uploadarea:uploadarea:upload

   phase:16Phase 16 security hotfix: CRITICAL chain + CVEPhase 16 security hotfix: CRITICAL chain + CVE

   and removed

   phase:backlogBacklogBacklog

   on Sep 13, 2026
2. SlavaKuntsov

   self-assigned this

   on Sep 14, 2026
3. SlavaKuntsov commented on Sep 17, 2026

   

   SlavaKuntsov

   on Sep 17, 2026

   OwnerAuthor

   More actions

   Agent review — iteration 1

   Implemented

   • Complete, GET/PUT resumable (services/upload/internal/handler/presign.go, resumable.go) перед любой работой с raw/{id}/* проверяют владельца через metadata internal-API — новый requireOwnership (ownership.go): чужой id → 403, несуществующий → 404, metadata недоступна → 502 (fail-closed).
   • MetadataClient.GetVideoOwner (client/metadata.go): GET /internal/videos/{id} + X-Internal-Token (из INTERNAL_TOKEN), url.PathEscape; INTERNAL_TOKEN добавлен в env upload в deploy/docker-compose.yml.
   • Complete дополнительно сверяет объект: StatObjectInfo (новый метод MinioClient) — пустой объект и не-video/* content-type → 400.

   Findings

   • [Medium] не было теста на контракт клиента (ошибка «status 404» ↔ классификация not-found в requireOwnership) — исправлено: client/metadata_test.go (200/404/401, заголовок/omit).
   • [Low] классификация 404 по строке ошибки хрупка → добавлен url.PathEscape; полностью типизированную ошибку можно сделать отдельным тикетом.
   • [Low, остался] StatObject в интерфейсах теперь используется только фейками тестов; логирование событий отказа ownership не добавлено (статус-код виден в request-логе).

   Verdict

   ready for PR

   Verification

   • go test ./... (upload) — pass, включая 9 новых IDOR-тестов (чужой id complete/resumable GET/PUT → 403, nothing stored; unknown → 404; metadata down → 502; empty/non-video → 400)
   • make lint-go — 0 issues
   • make test-go — 9 пакетов ok

   Ready for review

   Acceptance criteria

   • Чужой id → 403 в complete и resumable (unit-тесты)
   • complete сверяет размер/content-type объекта

   Review cycle

   • Iterations: 1
   • Remaining risks: make e2e не запускался (нужен Docker-стек)

   Suggested PR title

   fix(upload): verify video ownership in complete and resumable
4. SlavaKuntsov commented on Sep 17, 2026

   

   SlavaKuntsov

   on Sep 17, 2026

   OwnerAuthor

   More actions

   Pull request opened

   #74 — will auto-close via Closes #46 on merge.

   Project status: In review (synced after PR creation).

   Verification

   • go test ./... (upload) — pass, включая 9 новых IDOR-тестов
   • make lint-go — 0 issues
   • make test-go — все 9 пакетов ok

   Remaining risks

   • make e2e не запускался (нужен полный Docker-стек).
5. SlavaKuntsov

   closed this as completedin #74on Sep 17, 2026