Skip to content

fix(upload): verify video ownership in complete and resumable - #74

Merged
SlavaKuntsov merged 2 commits into
mainfrom
fix/issue-46-upload-ownership
Sep 17, 2026
Merged

SlavaKuntsov merged 2 commits into
mainfrom
fix/issue-46-upload-ownership

Conversation

@SlavaKuntsov

Copy link
Copy Markdown
Owner

Closes #46

Summary

HIGH IDOR: Complete and GET/PUT resumable in the upload service did not verify ownership — any logged-in user knowing a video UUID could append/overwrite someone else's raw/{id}/original.mp4 and trigger transcoding of their video.

Now every write/read path verifies the owner via the metadata internal API before touching storage:

  • services/upload/internal/handler/ownership.go (new) — OwnershipChecker interface + requireOwnership helper: foreign id → 403, unknown video → 404, metadata unavailable → 502 (fail closed).
  • MetadataClient.GetVideoOwner — GET /internal/videos/{id} with X-Internal-Token (from INTERNAL_TOKEN), url.PathEscape; INTERNAL_TOKEN added to the upload service env in deploy/docker-compose.yml.
  • Complete additionally verifies the stored object via new StatObjectInfo: empty object or non-video/* content type → 400.
  • README updated with the ownership section.

Acceptance criteria

  • Foreign video id → 403 in complete and resumable handlers (unit tests)
  • complete verifies object size/content-type (empty → 400, non-video → 400)

Verification

  • go test ./... (upload) — pass, including 9 new IDOR tests (foreign complete/resumable GET/PUT → 403 with nothing stored; unknown → 404; metadata down → 502; client 200/404/401 contract)
  • make lint-go — 0 issues
  • make test-go — all 9 packages ok
  • make e2e — not run (requires the full Docker stack)

Review

  • Independent review iteration 1: no Critical/High findings; one Medium (missing client 404-contract test) fixed in this diff; remaining Lows documented in the issue comment.

@SlavaKuntsov SlavaKuntsov self-assigned this Sep 17, 2026
@SlavaKuntsov
SlavaKuntsov merged commit 8e072d0 into main Sep 17, 2026
4 checks passed
@SlavaKuntsov
SlavaKuntsov deleted the fix/issue-46-upload-ownership branch September 17, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P0 upload: verify video ownership in complete and resumable handlers

1 participant