diff --git a/app/api/projects/[projectId]/videos/[videoId]/versions/route.ts b/app/api/projects/[projectId]/videos/[videoId]/versions/route.ts index a3ab33c..0ae9105 100644 --- a/app/api/projects/[projectId]/videos/[videoId]/versions/route.ts +++ b/app/api/projects/[projectId]/videos/[videoId]/versions/route.ts @@ -7,6 +7,7 @@ import { notifyProjectOwner } from '@/lib/notifications'; import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response'; import { readBunnyUploadGrant } from '@/lib/bunny-upload-token'; import { finalizeR2VideoUpload } from '@/lib/r2-video-finalize'; +import { UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { logError } from '@/lib/logger'; type RouteParams = { params: Promise<{ projectId: string; videoId: string }> }; @@ -234,6 +235,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { where: { id: finalizedR2Session.reservationId, billedUserId: finalizedR2Session.billedUserId, + purpose: UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, }, }); } @@ -243,7 +245,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // are never counted twice and never counted zero times. if (bunnyReservation) { await tx.uploadReservation.deleteMany({ - where: { id: bunnyReservation, billedUserId: video.project.workspace.ownerId }, + where: { + id: bunnyReservation, + billedUserId: video.project.workspace.ownerId, + purpose: UPLOAD_RESERVATION_PURPOSES.BUNNY, + }, }); } diff --git a/app/api/projects/[projectId]/videos/bunny-init/route.ts b/app/api/projects/[projectId]/videos/bunny-init/route.ts index 895a3b9..5a4231d 100644 --- a/app/api/projects/[projectId]/videos/bunny-init/route.ts +++ b/app/api/projects/[projectId]/videos/bunny-init/route.ts @@ -5,17 +5,14 @@ import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response import { rateLimit } from '@/lib/rate-limit'; import crypto from 'crypto'; import { cleanupBunnyStreamVideos } from '@/lib/bunny-stream-cleanup'; -import { - createBunnyUploadToken, - readBunnyUploadGrant, - verifyBunnyUploadToken, -} from '@/lib/bunny-upload-token'; +import { createBunnyUploadToken, readBunnyUploadGrant } from '@/lib/bunny-upload-token'; import { getMaxVideoUploadBytes, isBunnyUploadsEnabled } from '@/lib/feature-flags'; import { logError } from '@/lib/logger'; import { enforceStorageQuota, releaseStorageReservation, reserveStorageQuota, + UPLOAD_RESERVATION_PURPOSES, } from '@/lib/storage-quota'; import { parseDeclaredUploadSize } from '@/lib/upload-size'; @@ -99,6 +96,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const reserveResult = await reserveStorageQuota( billedUserId, declaredSize.sizeBytes, + UPLOAD_RESERVATION_PURPOSES.BUNNY, BUNNY_RESERVATION_TTL_MS ); if ('error' in reserveResult) return reserveResult.error; @@ -109,7 +107,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { process.env.BUNNY_STREAM_LIBRARY_ID || process.env.NEXT_PUBLIC_BUNNY_STREAM_LIBRARY_ID; if (!apiKey || !libraryId) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); return apiErrors.internalError('Bunny Stream is not configured correctly'); } @@ -125,7 +127,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { }); if (!bunnyRes.ok) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); logError('Failed to create Bunny Stream video', await bunnyRes.text()); return apiErrors.internalError('Failed to initialize video upload with provider'); } @@ -133,7 +139,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const bunnyVideo = await bunnyRes.json(); const videoId = bunnyVideo.guid; if (typeof videoId !== 'string' || videoId.length === 0) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); return apiErrors.internalError('Upload provider did not return a valid video identifier'); } @@ -197,12 +207,12 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { return apiErrors.badRequest('videoId and uploadToken are required'); } - const isValidUploadToken = verifyBunnyUploadToken(uploadToken, { + const grant = readBunnyUploadGrant(uploadToken, { userId: session.user.id, projectId, videoId, }); - if (!isValidUploadToken) { + if (!grant) { return apiErrors.forbidden('Invalid Bunny upload token'); } @@ -213,9 +223,9 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { // inside the signed token next to this video id, so releasing it costs the // caller the video it belongs to. await releaseStorageReservation( - readBunnyUploadGrant(uploadToken, { userId: session.user.id, projectId, videoId }) - ?.reservationId ?? null, - project.workspace.ownerId + grant.reservationId, + project.workspace.ownerId, + UPLOAD_RESERVATION_PURPOSES.BUNNY ); await cleanupBunnyStreamVideos([{ providerId: 'bunny', videoId }]); diff --git a/app/api/projects/[projectId]/videos/r2-complete/route.ts b/app/api/projects/[projectId]/videos/r2-complete/route.ts index d9d416a..648a071 100644 --- a/app/api/projects/[projectId]/videos/r2-complete/route.ts +++ b/app/api/projects/[projectId]/videos/r2-complete/route.ts @@ -7,7 +7,7 @@ import { parseR2UploadToken, verifyR2UploadToken } from '@/lib/r2-upload-token'; import { abortMultipartVideoUpload, completeMultipartVideoUpload } from '@/lib/r2'; import { isS3VideoUploadsEnabled } from '@/lib/feature-flags'; import { objectKeyToVideoProxyPath } from '@/lib/video-upload-validation'; -import { releaseStorageReservation } from '@/lib/storage-quota'; +import { releaseStorageReservation, UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { logError } from '@/lib/logger'; type RouteParams = { params: Promise<{ projectId: string }> }; @@ -155,7 +155,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { where: { id: uploadSession.id, status: 'INITIATED' }, data: { status: 'CANCELLED', consumedAt: new Date() }, }); - await releaseStorageReservation(uploadSession.reservationId, uploadSession.billedUserId); + await releaseStorageReservation( + uploadSession.reservationId, + uploadSession.billedUserId, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); return apiErrors.internalError('Failed to complete multipart upload'); } diff --git a/app/api/projects/[projectId]/videos/r2-init/route.ts b/app/api/projects/[projectId]/videos/r2-init/route.ts index 1adaf78..88bc7af 100644 --- a/app/api/projects/[projectId]/videos/r2-init/route.ts +++ b/app/api/projects/[projectId]/videos/r2-init/route.ts @@ -35,6 +35,7 @@ import { enforceStorageQuota, releaseStorageReservation, reserveStorageQuota, + UPLOAD_RESERVATION_PURPOSES, } from '@/lib/storage-quota'; import { createR2UploadSession } from '@/lib/r2-upload-session'; @@ -129,6 +130,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const reserveResult = await reserveStorageQuota( project.workspace.ownerId, sizeBytes + THUMBNAIL_RESERVE_BYTES, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, VIDEO_RESERVATION_TTL_MS ); if ('error' in reserveResult) return reserveResult.error; @@ -188,7 +190,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { ]); } } catch (error) { - await releaseStorageReservation(reserveResult.reservationId, project.workspace.ownerId); + await releaseStorageReservation( + reserveResult.reservationId, + project.workspace.ownerId, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); logError('Failed to create presigned video upload URL:', error); return apiErrors.internalError('Failed to initialize video upload'); } @@ -340,7 +346,11 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { logError('Failed to delete pending R2 video object:', error); } - await releaseStorageReservation(uploadSession.reservationId, uploadSession.billedUserId); + await releaseStorageReservation( + uploadSession.reservationId, + uploadSession.billedUserId, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); const response = successResponse({ message: 'Pending upload cleaned up' }); return withCacheControl(response, 'private, no-store'); diff --git a/app/api/projects/[projectId]/videos/route.ts b/app/api/projects/[projectId]/videos/route.ts index 5954779..92b468c 100644 --- a/app/api/projects/[projectId]/videos/route.ts +++ b/app/api/projects/[projectId]/videos/route.ts @@ -7,6 +7,7 @@ import { notifyProjectOwner } from '@/lib/notifications'; import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response'; import { readBunnyUploadGrant } from '@/lib/bunny-upload-token'; import { finalizeR2VideoUpload } from '@/lib/r2-video-finalize'; +import { UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { logError } from '@/lib/logger'; import { eventKey, recordEvent } from '@/lib/analytics/record'; @@ -237,6 +238,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { where: { id: finalizedR2Session.reservationId, billedUserId: finalizedR2Session.billedUserId, + purpose: UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, }, }); } @@ -246,7 +248,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // counted twice and never counted zero times. if (bunnyReservation) { await tx.uploadReservation.deleteMany({ - where: { id: bunnyReservation, billedUserId: project.workspace.ownerId }, + where: { + id: bunnyReservation, + billedUserId: project.workspace.ownerId, + purpose: UPLOAD_RESERVATION_PURPOSES.BUNNY, + }, }); } diff --git a/app/api/upload/audio/route.ts b/app/api/upload/audio/route.ts index 2e3fcf6..f2e6c59 100644 --- a/app/api/upload/audio/route.ts +++ b/app/api/upload/audio/route.ts @@ -13,7 +13,11 @@ import { enforceGuestUploadQuota, verifyGuestUploadToken, } from '@/lib/guest-upload-token'; -import { reserveStorageQuota, releaseStorageReservation } from '@/lib/storage-quota'; +import { + reserveStorageQuota, + releaseStorageReservation, + UPLOAD_RESERVATION_PURPOSES, +} from '@/lib/storage-quota'; import { logError } from '@/lib/logger'; const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB @@ -205,7 +209,11 @@ export async function POST(request: NextRequest) { // All paths use the advisory-locked reservation so concurrent uploads always // see each other's in-flight sizes, eliminating the TOCTOU race. const workspaceOwnerId = video.project.workspace.ownerId; - const reserveResult = await reserveStorageQuota(workspaceOwnerId, BigInt(file.size)); + const reserveResult = await reserveStorageQuota( + workspaceOwnerId, + BigInt(file.size), + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); if ('error' in reserveResult) return reserveResult.error; const reservationId = reserveResult.reservationId; @@ -214,7 +222,11 @@ export async function POST(request: NextRequest) { const strippedType = rawContentType.split(';')[0].trim().toLowerCase(); const contentType = MIME_ALIASES[strippedType] ?? strippedType; if (!ALLOWED_TYPES.has(contentType)) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); return apiErrors.badRequest(`Unsupported audio format: ${rawContentType}`); } @@ -231,12 +243,20 @@ export async function POST(request: NextRequest) { // Validate file content against magic bytes — rejects HTML/scripts masquerading as audio if (isHtmlContent(buffer)) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); return apiErrors.badRequest('File content does not match an audio format'); } const hasValidMagicBytes = hasValidAudioMagicBytes(buffer.slice(0, 16), contentType); if (!hasValidMagicBytes) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); return apiErrors.badRequest('File content does not match the declared audio format'); } @@ -251,7 +271,11 @@ export async function POST(request: NextRequest) { }) ); } catch (uploadError) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); throw uploadError; } diff --git a/app/api/upload/image/route.ts b/app/api/upload/image/route.ts index e9dc912..8a46d6d 100644 --- a/app/api/upload/image/route.ts +++ b/app/api/upload/image/route.ts @@ -20,7 +20,11 @@ import { verifyGuestUploadToken, } from '@/lib/guest-upload-token'; import { logError } from '@/lib/logger'; -import { reserveStorageQuota, releaseStorageReservation } from '@/lib/storage-quota'; +import { + reserveStorageQuota, + releaseStorageReservation, + UPLOAD_RESERVATION_PURPOSES, +} from '@/lib/storage-quota'; const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB const MAX_MULTIPART_BODY_SIZE = MAX_FILE_SIZE + 512 * 1024; // file + multipart overhead @@ -137,14 +141,22 @@ export async function POST(request: NextRequest) { // All paths use the advisory-locked reservation so concurrent uploads always // see each other's in-flight sizes, eliminating the TOCTOU race. const workspaceOwnerId = video.project.workspace.ownerId; - const reserveResult = await reserveStorageQuota(workspaceOwnerId, BigInt(file.size)); + const reserveResult = await reserveStorageQuota( + workspaceOwnerId, + BigInt(file.size), + UPLOAD_RESERVATION_PURPOSES.IMAGE + ); if ('error' in reserveResult) return reserveResult.error; const reservationId = reserveResult.reservationId; // Check content type const normalizedMime = normalizeImageMime(file.type); if (normalizedMime && !isAllowedImageType(normalizedMime)) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.IMAGE + ); return apiErrors.badRequest(`Unsupported image format: ${file.type}`); } @@ -153,7 +165,11 @@ export async function POST(request: NextRequest) { const buffer = Buffer.from(arrayBuffer); const detectedMime = detectImageMime(buffer); if (!detectedMime) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.IMAGE + ); return apiErrors.badRequest('Uploaded file content does not match an allowed image type'); } @@ -173,7 +189,11 @@ export async function POST(request: NextRequest) { }) ); } catch (uploadError) { - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + workspaceOwnerId, + UPLOAD_RESERVATION_PURPOSES.IMAGE + ); throw uploadError; } diff --git a/app/api/versions/[versionId]/comments/route.ts b/app/api/versions/[versionId]/comments/route.ts index b236f1b..5e81a92 100644 --- a/app/api/versions/[versionId]/comments/route.ts +++ b/app/api/versions/[versionId]/comments/route.ts @@ -21,7 +21,11 @@ import { } from '@/lib/video-assets'; import { validateAnnotationStrokes } from '@/lib/validation'; import { logError } from '@/lib/logger'; -import { reserveStorageQuota, releaseStorageReservation } from '@/lib/storage-quota'; +import { + reserveStorageQuota, + releaseStorageReservation, + UPLOAD_RESERVATION_PURPOSES, +} from '@/lib/storage-quota'; import { isValidEmailAddress, normalizeEmail } from '@/lib/email-validation'; type RouteParams = { params: Promise<{ versionId: string }> }; @@ -202,6 +206,9 @@ export async function GET(request: NextRequest, { params }: RouteParams) { // POST /api/versions/[versionId]/comments export async function POST(request: NextRequest, { params }: RouteParams) { let attachmentReservationId: string | null = null; + // Carried out of the try so the catch below can scope the release to the + // account the hold was opened against. + let attachmentBilledUserId: string | null = null; try { const limited = await rateLimit(request, 'comment'); if (limited) return limited; @@ -416,10 +423,12 @@ export async function POST(request: NextRequest, { params }: RouteParams) { if (totalAttachmentBytes > BigInt(0)) { const reserveResult = await reserveStorageQuota( project.workspace.ownerId, - totalAttachmentBytes + totalAttachmentBytes, + UPLOAD_RESERVATION_PURPOSES.ATTACHMENT ); if ('error' in reserveResult) return reserveResult.error; attachmentReservationId = reserveResult.reservationId; + attachmentBilledUserId = project.workspace.ownerId; } // Use a transaction to create both the comment and any asset rows atomically. @@ -427,7 +436,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const result = await db.$transaction(async (tx) => { if (attachmentReservationId) { await tx.uploadReservation.deleteMany({ - where: { id: attachmentReservationId, billedUserId: project.workspace.ownerId }, + where: { + id: attachmentReservationId, + billedUserId: project.workspace.ownerId, + purpose: UPLOAD_RESERVATION_PURPOSES.ATTACHMENT, + }, }); } const comment = await tx.comment.create({ @@ -586,7 +599,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { } return withCacheControl(response, 'private, no-store'); } catch (error) { - await releaseStorageReservation(attachmentReservationId); + await releaseStorageReservation( + attachmentReservationId, + attachmentBilledUserId, + UPLOAD_RESERVATION_PURPOSES.ATTACHMENT + ); logError('Error creating comment:', error); return apiErrors.internalError('Failed to create comment'); } diff --git a/app/api/videos/[videoId]/assets/bunny-init/route.ts b/app/api/videos/[videoId]/assets/bunny-init/route.ts index b666e28..da10aab 100644 --- a/app/api/videos/[videoId]/assets/bunny-init/route.ts +++ b/app/api/videos/[videoId]/assets/bunny-init/route.ts @@ -5,14 +5,15 @@ import { rateLimit } from '@/lib/rate-limit'; import { createBunnyUploadToken, readBunnyUploadGrant, - verifyBunnyUploadToken, + type BunnyUploadGrant, } from '@/lib/bunny-upload-token'; import { cleanupBunnyStreamVideos } from '@/lib/bunny-stream-cleanup'; import { createGuestUploadToken, deriveGuestUploadContext, enforceGuestUploadQuota, - verifyGuestUploadToken, + readGuestUploadGrant, + type GuestUploadGrant, } from '@/lib/guest-upload-token'; import { getMaxVideoUploadBytes, isBunnyUploadsEnabled } from '@/lib/feature-flags'; import { getShareSessionFromRequest } from '@/lib/share-session'; @@ -22,6 +23,7 @@ import { enforceStorageQuota, releaseStorageReservation, reserveStorageQuota, + UPLOAD_RESERVATION_PURPOSES, } from '@/lib/storage-quota'; import { parseDeclaredUploadSize } from '@/lib/upload-size'; @@ -31,6 +33,20 @@ type RouteParams = { params: Promise<{ videoId: string }> }; // Bunny's own reporting delay. const BUNNY_RESERVATION_TTL_MS = 2 * 60 * 60 * 1000; +/** + * A guest's hold lapses sooner than a member's. + * + * A guest is whoever opened the share link, and the hold is written against the + * workspace owner's quota rather than their own. Declaring a size and then + * walking away costs the guest nothing and costs the owner their whole remaining + * allowance, which on a trial is the entire account. Half an hour is the same + * window the R2 attachment paths already accept, and it bounds what a guest who + * never uploads can take away. A guest whose upload outruns it loses only the + * concurrency guard for the tail of the transfer; the bytes are still recorded + * from the signed size when the asset is created. + */ +const GUEST_BUNNY_RESERVATION_TTL_MS = 30 * 60 * 1000; + // POST /api/videos/[videoId]/assets/bunny-init export async function POST(request: NextRequest, { params }: RouteParams) { try { @@ -76,7 +92,8 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const reserveResult = await reserveStorageQuota( billedUserId, declaredSize.sizeBytes, - BUNNY_RESERVATION_TTL_MS + UPLOAD_RESERVATION_PURPOSES.BUNNY, + context.viewerUserId ? BUNNY_RESERVATION_TTL_MS : GUEST_BUNNY_RESERVATION_TTL_MS ); if ('error' in reserveResult) return reserveResult.error; const { reservationId } = reserveResult; @@ -85,7 +102,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const libraryId = process.env.BUNNY_STREAM_LIBRARY_ID || process.env.NEXT_PUBLIC_BUNNY_STREAM_LIBRARY_ID; if (!apiKey || !libraryId) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); return apiErrors.internalError('Bunny Stream is not configured correctly'); } @@ -100,7 +121,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { }); if (!bunnyRes.ok) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); logError('Failed to create Bunny Stream video asset', await bunnyRes.text()); return apiErrors.internalError('Failed to initialize Bunny upload'); } @@ -108,7 +133,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const bunnyVideo = await bunnyRes.json(); const bunnyVideoId = typeof bunnyVideo?.guid === 'string' ? bunnyVideo.guid.trim() : ''; if (!bunnyVideoId || !SAFE_BUNNY_VIDEO_ID.test(bunnyVideoId)) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); return apiErrors.internalError('Upload provider did not return a valid video identifier'); } @@ -125,29 +154,35 @@ export async function POST(request: NextRequest, { params }: RouteParams) { projectId: context.video.projectId, videoId: bunnyVideoId, reservationId, + declaredSizeBytes: declaredSize.sizeBytes, }, 3600 ); } else { const expectedContext = deriveGuestUploadContext(request, shareSession?.token ?? null); if (!expectedContext) { - await releaseStorageReservation(reservationId, billedUserId); + await releaseStorageReservation( + reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); return apiErrors.forbidden('Missing trusted client IP header'); } - // No reservation id in the guest grant, so a guest cancelling waits out the - // two hours instead of getting the quota back at once. The guest token is - // bound to our own video id and the caller's network context, not to the - // Bunny video being uploaded, so a released-on-request reservation could be - // dropped while the upload it stands for carried on. Guests are capped at - // four of these per quarter hour, which bounds what the wait can cost. - + // The guest grant carries the same three claims the signed-in one does, + // and is bound to the Bunny video as well as to ours. That binding is what + // makes releasing safe on the guest's say-so: presenting this token to + // cancel deletes the upload it stands for, so it cannot be used to drop the + // hold while the transfer carries on. uploadToken = createGuestUploadToken( { projectId: context.video.projectId, videoId: context.video.id, intent: 'bunny', context: expectedContext, + providerVideoId: bunnyVideoId, + reservationId, + declaredSizeBytes: declaredSize.sizeBytes, }, 3600 ); @@ -185,15 +220,18 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { return apiErrors.badRequest('videoId and uploadToken are required'); } + // Both grants are read rather than merely checked, because both carry the + // reservation this upload holds. Releasing on the caller's say-so is safe + // only because the id is signed next to this Bunny video id: presenting the + // token costs them the video, which is deleted immediately below. + let grant: BunnyUploadGrant | GuestUploadGrant | null = null; + if (context.viewerUserId) { - const isValidUploadToken = verifyBunnyUploadToken(uploadToken, { + grant = readBunnyUploadGrant(uploadToken, { userId: context.viewerUserId, projectId: context.video.projectId, videoId: bunnyVideoId, }); - if (!isValidUploadToken) { - return apiErrors.forbidden('Invalid Bunny upload token'); - } } else { const shareSession = getShareSessionFromRequest(request, context.video.id); const expectedContext = deriveGuestUploadContext(request, shareSession?.token ?? null); @@ -201,30 +239,28 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { return apiErrors.forbidden('Missing trusted client IP header'); } - const isValidUploadToken = verifyGuestUploadToken(uploadToken, { - projectId: context.video.projectId, - videoId: context.video.id, - intent: 'bunny', - context: expectedContext, - }); - if (!isValidUploadToken) { - return apiErrors.forbidden('Invalid Bunny upload token'); - } - } - - if (context.viewerUserId) { - // Safe on the caller's say-so because the reservation id is signed into the - // same token as this Bunny video id: releasing it costs them the video. - await releaseStorageReservation( - readBunnyUploadGrant(uploadToken, { - userId: context.viewerUserId, + grant = readGuestUploadGrant( + uploadToken, + { projectId: context.video.projectId, - videoId: bunnyVideoId, - })?.reservationId ?? null, - context.video.project.workspace.ownerId + videoId: context.video.id, + intent: 'bunny', + context: expectedContext, + }, + bunnyVideoId ); } + if (!grant) { + return apiErrors.forbidden('Invalid Bunny upload token'); + } + + await releaseStorageReservation( + grant.reservationId, + context.video.project.workspace.ownerId, + UPLOAD_RESERVATION_PURPOSES.BUNNY + ); + await cleanupBunnyStreamVideos([{ providerId: 'bunny', videoId: bunnyVideoId }]); const response = successResponse({ message: 'Pending upload cleaned up' }); return withCacheControl(response, 'private, no-store'); diff --git a/app/api/videos/[videoId]/assets/r2-init/route.ts b/app/api/videos/[videoId]/assets/r2-init/route.ts index 1f2ff05..899bac2 100644 --- a/app/api/videos/[videoId]/assets/r2-init/route.ts +++ b/app/api/videos/[videoId]/assets/r2-init/route.ts @@ -26,6 +26,7 @@ import { enforceStorageQuota, releaseStorageReservation, reserveStorageQuota, + UPLOAD_RESERVATION_PURPOSES, } from '@/lib/storage-quota'; import { createR2UploadSession } from '@/lib/r2-upload-session'; import { getVideoAssetAccessContext } from '@/lib/video-assets'; @@ -97,6 +98,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { const reserveResult = await reserveStorageQuota( billedUserId, sizeBytes + THUMBNAIL_RESERVE_BYTES, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, VIDEO_RESERVATION_TTL_MS ); if ('error' in reserveResult) return reserveResult.error; @@ -117,7 +119,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { createPresignedImagePutUrl(thumbnailObjectKey, 'image/jpeg'), ]); } catch (error) { - await releaseStorageReservation(reserveResult.reservationId, billedUserId); + await releaseStorageReservation( + reserveResult.reservationId, + billedUserId, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); logError('Failed to create presigned asset video upload URL:', error); return apiErrors.internalError('Failed to initialize video upload'); } @@ -261,7 +267,11 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { logError('Failed to delete pending R2 asset video object:', error); } - await releaseStorageReservation(uploadSession.reservationId, uploadSession.billedUserId); + await releaseStorageReservation( + uploadSession.reservationId, + uploadSession.billedUserId, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); const response = successResponse({ message: 'Pending upload cleaned up' }); return withCacheControl(response, 'private, no-store'); diff --git a/app/api/videos/[videoId]/assets/route.ts b/app/api/videos/[videoId]/assets/route.ts index e6b5b44..4573b54 100644 --- a/app/api/videos/[videoId]/assets/route.ts +++ b/app/api/videos/[videoId]/assets/route.ts @@ -7,7 +7,7 @@ import { rateLimit } from '@/lib/rate-limit'; import { db } from '@/lib/db'; import { r2Client, R2_BUCKET_NAME } from '@/lib/r2'; import { readBunnyUploadGrant } from '@/lib/bunny-upload-token'; -import { deriveGuestUploadContext, verifyGuestUploadToken } from '@/lib/guest-upload-token'; +import { deriveGuestUploadContext, readGuestUploadGrant } from '@/lib/guest-upload-token'; import { ensureGuestIdentityFromRequest, setGuestIdentityCookie } from '@/lib/guest-identity'; import { getShareSessionFromRequest } from '@/lib/share-session'; import { validateUrl, validateOptionalUrl } from '@/lib/validation'; @@ -33,6 +33,8 @@ import { reserveStorageQuota, releaseStorageReservation, getStorageLimitForUser, + UPLOAD_RESERVATION_PURPOSES, + type UploadReservationPurpose, } from '@/lib/storage-quota'; import { getCachedUserBunnyStorage } from '@/lib/admin-stats'; import { isStripeFeatureEnabled } from '@/lib/feature-flags'; @@ -298,6 +300,13 @@ export async function GET(request: NextRequest, { params }: RouteParams) { // POST /api/videos/[videoId]/assets export async function POST(request: NextRequest, { params }: RouteParams) { let reservationId: string | null = null; + // What the reservation above was opened for, and who it is billed to. A hold is + // only ever consumed by the flow that opened it: the id below can arrive in the + // request body, and every hold an account owns is billed to the same user, so + // the id alone would let an image being attached release a video upload that + // was still in flight. + let reservationPurpose: UploadReservationPurpose | null = null; + let reservationBilledUserId: string | null = null; let finalizedR2AssetSession: { sessionId: string; reservationId: string | null; @@ -342,6 +351,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { let assetSizeBytes = BigInt(0); const billedUserId = context.video.project.workspace.ownerId; + reservationBilledUserId = billedUserId; if (provider === VideoAssetProvider.R2_IMAGE) { sourceUrl = typeof body?.sourceUrl === 'string' ? body.sourceUrl.trim() : ''; @@ -359,8 +369,13 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // the client already supplied a reservationId (new upload flow) the // existing reservation is consumed in the transaction below. For the // backward-compat path (no reservationId) we create one here. + reservationPurpose = UPLOAD_RESERVATION_PURPOSES.IMAGE; if (!reservationId) { - const reserveResult = await reserveStorageQuota(billedUserId, assetSizeBytes); + const reserveResult = await reserveStorageQuota( + billedUserId, + assetSizeBytes, + UPLOAD_RESERVATION_PURPOSES.IMAGE + ); if ('error' in reserveResult) return reserveResult.error; reservationId = reserveResult.reservationId; } @@ -383,8 +398,13 @@ export async function POST(request: NextRequest, { params }: RouteParams) { assetSizeBytes = audioCheck.sizeBytes; // Same reservation logic as R2_IMAGE above + reservationPurpose = UPLOAD_RESERVATION_PURPOSES.AUDIO; if (!reservationId) { - const reserveResult = await reserveStorageQuota(billedUserId, assetSizeBytes); + const reserveResult = await reserveStorageQuota( + billedUserId, + assetSizeBytes, + UPLOAD_RESERVATION_PURPOSES.AUDIO + ); if ('error' in reserveResult) return reserveResult.error; reservationId = reserveResult.reservationId; } @@ -450,6 +470,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { assetSizeBytes = finalizeResult.sizeBytes; reservationId = finalizeResult.reservationId; + reservationPurpose = UPLOAD_RESERVATION_PURPOSES.R2_VIDEO; if (!thumbnailUrl) { thumbnailUrl = finalizeResult.thumbnailProxyUrl; } @@ -509,6 +530,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // does not include it. assetSizeBytes = grant.declaredSizeBytes ?? BigInt(0); reservationId = grant.reservationId; + reservationPurpose = UPLOAD_RESERVATION_PURPOSES.BUNNY; } else { const shareSession = getShareSessionFromRequest(request, context.video.id); const expectedContext = deriveGuestUploadContext(request, shareSession?.token ?? null); @@ -516,15 +538,28 @@ export async function POST(request: NextRequest, { params }: RouteParams) { return apiErrors.forbidden('Missing trusted client IP header'); } - const isValidGuestUploadToken = verifyGuestUploadToken(uploadToken, { - projectId: context.video.projectId, - videoId: context.video.id, - intent: 'bunny', - context: expectedContext, - }); - if (!isValidGuestUploadToken) { + // Read rather than merely verified, for the same reason as above: a guest + // upload that reads as zero bytes until Bunny finishes encoding is an hour + // of the owner's quota spent on nothing. The grant is bound to this Bunny + // video, so the size and the hold it names belong to this upload and no + // other. + const guestGrant = readGuestUploadGrant( + uploadToken, + { + projectId: context.video.projectId, + videoId: context.video.id, + intent: 'bunny', + context: expectedContext, + }, + providerVideoId + ); + if (!guestGrant) { return apiErrors.forbidden('Invalid Bunny upload token'); } + + assetSizeBytes = guestGrant.declaredSizeBytes ?? BigInt(0); + reservationId = guestGrant.reservationId; + reservationPurpose = UPLOAD_RESERVATION_PURPOSES.BUNNY; } displayName = sanitizeAssetDisplayName(requestedDisplayName, `Bunny ${providerVideoId}`); @@ -543,14 +578,12 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // Pre-fetch Bunny storage BEFORE entering the transaction to avoid making an // HTTP call while holding a DB connection open (connection-pool exhaustion // risk under adversarial load). Mirrors the discipline in reserveStorageQuota. - // Only needed for R2 providers where the invalid-reservation fallback quota - // check requires Bunny usage data. + // Needed by every provider that can reach the invalid-reservation fallback + // quota check below, Bunny included. Leaving Bunny out read its own storage as + // zero, and on an account whose storage is all Bunny that made the fallback a + // check that could not fail. const preFetchedBunnyData = - provider === VideoAssetProvider.R2_IMAGE || - provider === VideoAssetProvider.R2_AUDIO || - provider === VideoAssetProvider.R2_VIDEO - ? await getCachedUserBunnyStorage() - : null; + provider === VideoAssetProvider.YOUTUBE ? null : await getCachedUserBunnyStorage(); // The ceiling this account is actually held to, read for the fallback below. // It used to compare against the plan limit, which is 200 GiB whoever is @@ -561,7 +594,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // Create the VideoAsset and atomically consume the upload reservation (if any) // so the spot is never double-counted. const created = await db.$transaction(async (tx) => { - if (reservationId) { + if (reservationId && reservationPurpose) { // Acquire the per-user advisory lock unconditionally so both the happy path // (valid reservation) and the fallback path (fake/expired reservation ID) are // serialised — eliminating the TOCTOU race in the deleted.count === 0 branch. @@ -575,7 +608,12 @@ export async function POST(request: NextRequest, { params }: RouteParams) { // we fall back to a standard (non-locked) quota check so the bypass attempt // is caught rather than silently allowed. const deleted = await tx.uploadReservation.deleteMany({ - where: { id: reservationId, billedUserId, expiresAt: { gt: new Date() } }, + where: { + id: reservationId, + billedUserId, + purpose: reservationPurpose, + expiresAt: { gt: new Date() }, + }, }); if (deleted.count === 0) { // Reservation didn't exist — enforce quota the normal way inside the tx. @@ -676,7 +714,11 @@ export async function POST(request: NextRequest, { params }: RouteParams) { if (error instanceof QuotaExceededInTxError) { return apiErrors.storageExceeded() as NextResponse; } - await releaseStorageReservation(reservationId); + await releaseStorageReservation( + reservationId, + reservationBilledUserId, + reservationPurpose ?? undefined + ); logError('Error creating video asset:', error); return apiErrors.internalError('Failed to create asset'); } diff --git a/lib/guest-upload-token.ts b/lib/guest-upload-token.ts index 0ba9f55..b1336b0 100644 --- a/lib/guest-upload-token.ts +++ b/lib/guest-upload-token.ts @@ -20,6 +20,25 @@ interface GuestUploadTokenPayload { exp: number; intent: GuestUploadIntent; ctx: string; + /** + * The provider's own id for the video this grant was issued against. + * + * `vid` is our video, the one the asset will hang off. That is not enough to + * hand a guest the right to release a storage hold: the hold stands for one + * particular upload, and a grant that names only our video would let a guest + * drop it while the upload it stands for carried on. Binding the provider's id + * makes releasing cost the guest the upload itself, which is the same bargain + * the signed-in path already makes. + */ + bvid?: string; + /** The storage reservation this upload holds, when it holds one. */ + rid?: string; + /** + * The size the client declared when it asked for this grant, as a decimal + * string because JSON has no integer wide enough. Signed so a guest cannot + * declare one size to pass the quota check and another to be billed for. + */ + sz?: string; } interface GuestUploadTokenSubject { @@ -29,6 +48,20 @@ interface GuestUploadTokenSubject { context: string; } +interface GuestUploadTokenClaims { + /** The provider video id to bind this grant to. */ + providerVideoId?: string | null; + reservationId?: string | null; + declaredSizeBytes?: bigint | null; +} + +export interface GuestUploadGrant { + /** The storage reservation this upload holds, or null if it holds none. */ + reservationId: string | null; + /** What the client said it was uploading, or null on a grant that predates the claim. */ + declaredSizeBytes: bigint | null; +} + const TRUSTED_IP_PATTERN = /^[\da-fA-F.:]+$/; function getGuestUploadTokenSecret(): string { @@ -77,7 +110,10 @@ function isValidPayload(value: unknown): value is GuestUploadTokenPayload { typeof payload.exp === 'number' && Number.isFinite(payload.exp) && (payload.intent === 'audio' || payload.intent === 'image' || payload.intent === 'bunny') && - typeof payload.ctx === 'string' + typeof payload.ctx === 'string' && + (payload.bvid === undefined || typeof payload.bvid === 'string') && + (payload.rid === undefined || typeof payload.rid === 'string') && + (payload.sz === undefined || typeof payload.sz === 'string') ); } @@ -95,7 +131,7 @@ export function deriveGuestUploadContext( } export function createGuestUploadToken( - subject: GuestUploadTokenSubject, + subject: GuestUploadTokenSubject & GuestUploadTokenClaims, ttlSeconds = DEFAULT_GUEST_UPLOAD_TOKEN_TTL_SECONDS ): string { const now = Math.floor(Date.now() / 1000); @@ -107,6 +143,9 @@ export function createGuestUploadToken( exp: now + ttlSeconds, intent: subject.intent, ctx: subject.context, + ...(subject.providerVideoId ? { bvid: subject.providerVideoId } : {}), + ...(subject.reservationId ? { rid: subject.reservationId } : {}), + ...(subject.declaredSizeBytes ? { sz: subject.declaredSizeBytes.toString() } : {}), }; const encodedPayload = Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url'); @@ -114,39 +153,94 @@ export function createGuestUploadToken( return `${encodedPayload}.${signature}`; } -export function verifyGuestUploadToken(token: string, subject: GuestUploadTokenSubject): boolean { +/** + * The verified payload, or null when the token is not a genuine grant for this + * subject. + * + * `providerVideoId` is checked only when the grant carries one, so a token + * issued before that claim existed keeps working rather than failing an upload + * in flight. A grant that does carry one and does not match is a forgery as far + * as this is concerned. + */ +function readGuestUploadToken( + token: string, + subject: GuestUploadTokenSubject, + providerVideoId?: string | null +): GuestUploadTokenPayload | null { try { const parts = token.split('.'); - if (parts.length !== 2) return false; + if (parts.length !== 2) return null; const [encodedPayload, providedSignature] = parts; - if (!encodedPayload || !providedSignature) return false; + if (!encodedPayload || !providedSignature) return null; const expectedSignature = signPayload(encodedPayload); const providedBuffer = Buffer.from(providedSignature, 'utf8'); const expectedBuffer = Buffer.from(expectedSignature, 'utf8'); - if (providedBuffer.length !== expectedBuffer.length) return false; - if (!timingSafeEqual(providedBuffer, expectedBuffer)) return false; + if (providedBuffer.length !== expectedBuffer.length) return null; + if (!timingSafeEqual(providedBuffer, expectedBuffer)) return null; const payloadRaw = Buffer.from(encodedPayload, 'base64url').toString('utf8'); const payloadUnknown: unknown = JSON.parse(payloadRaw); - if (!isValidPayload(payloadUnknown)) return false; + if (!isValidPayload(payloadUnknown)) return null; const payload = payloadUnknown; const now = Math.floor(Date.now() / 1000); - if (payload.exp < now) return false; + if (payload.exp < now) return null; - return ( - payload.pid === subject.projectId && - payload.vid === subject.videoId && - payload.intent === subject.intent && - payload.ctx === subject.context - ); + if ( + payload.pid !== subject.projectId || + payload.vid !== subject.videoId || + payload.intent !== subject.intent || + payload.ctx !== subject.context + ) { + return null; + } + + if (payload.bvid !== undefined && payload.bvid !== providerVideoId) return null; + + return payload; } catch { - return false; + return null; } } +export function verifyGuestUploadToken( + token: string, + subject: GuestUploadTokenSubject, + providerVideoId?: string | null +): boolean { + return readGuestUploadToken(token, subject, providerVideoId) !== null; +} + +/** + * What a genuine guest grant for this subject carries, or null when the token is + * not one. + * + * Null and empty fields mean the same thing to every caller: there is nothing + * here to release and nothing to bill. + */ +export function readGuestUploadGrant( + token: string, + subject: GuestUploadTokenSubject, + providerVideoId?: string | null +): GuestUploadGrant | null { + const payload = readGuestUploadToken(token, subject, providerVideoId); + if (!payload) return null; + + let declaredSizeBytes: bigint | null = null; + if (payload.sz) { + try { + const parsed = BigInt(payload.sz); + declaredSizeBytes = parsed > BigInt(0) ? parsed : null; + } catch { + declaredSizeBytes = null; + } + } + + return { reservationId: payload.rid ?? null, declaredSizeBytes }; +} + export async function enforceGuestUploadQuota( request: Request, videoId: string, diff --git a/lib/storage-quota.ts b/lib/storage-quota.ts index 8fca0ac..c01d82b 100644 --- a/lib/storage-quota.ts +++ b/lib/storage-quota.ts @@ -29,6 +29,33 @@ export async function getStorageLimitForUser(userId: string): Promise { // TTL for upload reservations: 30 minutes is enough for R2 image/audio uploads const RESERVATION_TTL_MS = 30 * 60 * 1000; +/** + * What a hold was opened for. + * + * A reservation is only ever consumed by the flow that opened it, and the + * finalize routes match on this as well as on the id. Without it, naming a + * reservation would be enough to drop it: the asset route takes a reservation id + * from the request body, and every hold an account owns is billed to the same + * user, so an image being attached could quietly release a video upload that was + * still in flight. The ids are not secret. Two of them are handed to the client + * outright, and the rest ride inside signed-but-readable token payloads. + */ +export const UPLOAD_RESERVATION_PURPOSES = { + /** A comment attachment or standalone image going to R2. */ + IMAGE: 'IMAGE', + /** A voice note going to R2. */ + AUDIO: 'AUDIO', + /** Image and voice attachments weighed together when a comment is posted. */ + ATTACHMENT: 'ATTACHMENT', + /** A presigned direct upload to our own S3-compatible storage. */ + R2_VIDEO: 'R2_VIDEO', + /** A direct upload to Bunny, where the bytes never pass through us. */ + BUNNY: 'BUNNY', +} as const; + +export type UploadReservationPurpose = + (typeof UPLOAD_RESERVATION_PURPOSES)[keyof typeof UPLOAD_RESERVATION_PURPOSES]; + // Sentinel error thrown inside a Prisma transaction to signal quota exceeded class QuotaExceededError extends Error {} @@ -137,6 +164,7 @@ export async function enforceStorageQuota( export async function reserveStorageQuota( userId: string, incomingSizeBytes: bigint, + purpose: UploadReservationPurpose, reservationTtlMs: number = RESERVATION_TTL_MS ): Promise<{ reservationId: string | null } | { error: NextResponse }> { if (!isStripeFeatureEnabled()) { @@ -199,7 +227,7 @@ export async function reserveStorageQuota( } const reservation = await tx.uploadReservation.create({ - data: { billedUserId: userId, sizeBytes: incomingSizeBytes, expiresAt }, + data: { billedUserId: userId, sizeBytes: incomingSizeBytes, expiresAt, purpose }, select: { id: true }, }); @@ -218,16 +246,22 @@ export async function reserveStorageQuota( /** * Deletes an upload reservation created by `reserveStorageQuota`. * Safe to call with `null` (no-op) for flows where billing is disabled. + * + * Pass the purpose wherever the caller knows it. A release that names only an id + * will delete a hold opened for something else, which is the same hole the + * purpose column exists to close. */ export async function releaseStorageReservation( reservationId: string | null, - billedUserId?: string | null + billedUserId?: string | null, + purpose?: UploadReservationPurpose ): Promise { if (!reservationId) return; await db.uploadReservation.deleteMany({ where: { id: reservationId, ...(billedUserId ? { billedUserId } : {}), + ...(purpose ? { purpose } : {}), }, }); } diff --git a/prisma/migrations/20260818120000_add_upload_reservation_purpose/migration.sql b/prisma/migrations/20260818120000_add_upload_reservation_purpose/migration.sql new file mode 100644 index 0000000..04f11a3 --- /dev/null +++ b/prisma/migrations/20260818120000_add_upload_reservation_purpose/migration.sql @@ -0,0 +1,10 @@ +-- AlterTable: record what each hold was opened for, so a reservation can only be +-- consumed by the flow that opened it. Without this a caller who could name a +-- reservation id could drop any of their own holds through whichever finalize +-- route was cheapest, which defeats the point of holding one at all. +-- +-- Rows that predate this column get 'LEGACY', which matches no finalize route. +-- Those uploads fall through to the in-transaction quota check instead, and the +-- rows lapse on their own TTL within the hour. +ALTER TABLE "upload_reservations" ADD COLUMN "purpose" TEXT NOT NULL DEFAULT 'LEGACY'; +ALTER TABLE "upload_reservations" ALTER COLUMN "purpose" DROP DEFAULT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index b1ccefb..2c4a869 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -703,6 +703,10 @@ model UploadReservation { sizeBytes BigInt expiresAt DateTime createdAt DateTime @default(now()) + /// What this hold was opened for. A reservation can only be consumed by the + /// flow that opened it, so naming one is not enough to drop it. See + /// UPLOAD_RESERVATION_PURPOSES in lib/storage-quota.ts. + purpose String @@index([billedUserId, expiresAt]) @@map("upload_reservations") diff --git a/tests/api/bunny-upload-reservation.test.ts b/tests/api/bunny-upload-reservation.test.ts index f0e988a..6d36bc8 100644 --- a/tests/api/bunny-upload-reservation.test.ts +++ b/tests/api/bunny-upload-reservation.test.ts @@ -14,9 +14,11 @@ import { DELETE as cancelProjectBunnyUpload, POST as initProjectBunnyUpload, } from '@/app/api/projects/[projectId]/videos/bunny-init/route'; +import { POST as createAsset } from '@/app/api/videos/[videoId]/assets/route'; +import { UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { apiRequest, callRoute, readData, readError } from '../helpers/request'; import { signedInAs } from '../helpers/session'; -import { seedProject } from '../factories'; +import { createVideo, seedProject } from '../factories'; const GIB = BigInt(1024) * BigInt(1024) * BigInt(1024); @@ -177,3 +179,85 @@ describe('DELETE /api/projects/[projectId]/videos/bunny-init', () => { expect(await db.uploadReservation.count()).toBe(2); }); }); + +// The reservation id is not a secret and was never going to be one. The upload +// token is `base64url(payload).signature`, so the client can read every claim in +// it, and the two R2 upload routes hand their reservation ids to the client +// outright. What keeps a hold from being dropped by whoever can name it is that +// a reservation records what it was opened for, and every finalize route matches +// on that as well as on the id. +describe('a Bunny hold cannot be consumed by another flow', () => { + /** The claims the client can read out of an upload token without our help. */ + function claimsOf(uploadToken: string): Record { + return JSON.parse(Buffer.from(uploadToken.split('.')[0], 'base64url').toString('utf8')); + } + + it('puts the reservation id somewhere the client can read it', async () => { + const scenario = await seedProject(); + signedInAs(scenario.owner); + + const init = await initUpload(scenario.project.id, BigInt(1) * GIB); + const { uploadToken } = await readData(init); + + const reservation = (await db.uploadReservation.findMany())[0]; + expect(claimsOf(uploadToken).rid).toBe(reservation.id); + expect(reservation.purpose).toBe(UPLOAD_RESERVATION_PURPOSES.BUNNY); + }); + + // The attack the purpose column closes. Creating a YouTube asset costs nothing + // and consumes no storage, so quoting a Bunny reservation on one was a way to + // hand back the quota of an upload that was still running and then start + // another. Repeat and a trial worth three gigabytes uploads as much as it + // likes for as long as Bunny takes to report. + it('ignores a Bunny reservation id quoted on a YouTube asset create', async () => { + const scenario = await seedProject(); + const video = await createVideo({ projectId: scenario.project.id }); + signedInAs(scenario.owner); + + const init = await initUpload(scenario.project.id, BigInt(2) * GIB); + const { uploadToken } = await readData(init); + const reservationId = claimsOf(uploadToken).rid as string; + + const response = await callRoute( + createAsset, + apiRequest(`/api/videos/${video.id}/assets`, { + body: { + provider: 'YOUTUBE', + sourceUrl: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', + reservationId, + }, + }), + { videoId: video.id } + ); + + expect(response.status).toBe(201); + const reservations = await db.uploadReservation.findMany(); + expect(reservations).toHaveLength(1); + expect(reservations[0].id).toBe(reservationId); + }); + + // And with the hold still standing, the next init has to see it. + it('still refuses the next upload after the quoted release attempt', async () => { + const scenario = await seedProject(); + const video = await createVideo({ projectId: scenario.project.id }); + signedInAs(scenario.owner); + + const init = await initUpload(scenario.project.id, BigInt(2) * GIB); + const { uploadToken } = await readData(init); + + await callRoute( + createAsset, + apiRequest(`/api/videos/${video.id}/assets`, { + body: { + provider: 'YOUTUBE', + sourceUrl: 'https://www.youtube.com/watch?v=dQw4w9WgXcQ', + reservationId: claimsOf(uploadToken).rid, + }, + }), + { videoId: video.id } + ); + + const second = await initUpload(scenario.project.id, BigInt(2) * GIB); + expect(second.status).toBe(507); + }); +}); diff --git a/tests/api/lib-r2-upload-session.test.ts b/tests/api/lib-r2-upload-session.test.ts index 758f3d6..e049315 100644 --- a/tests/api/lib-r2-upload-session.test.ts +++ b/tests/api/lib-r2-upload-session.test.ts @@ -11,6 +11,7 @@ import { describe, expect, it } from 'vitest'; import { randomUUID } from 'crypto'; import { db } from '@/lib/db'; +import { UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { cancelR2UploadSession, createR2UploadSession } from '@/lib/r2-upload-session'; import { seedProject } from '../factories'; @@ -101,6 +102,7 @@ describe('createR2UploadSession', () => { data: { billedUserId: scenario.owner.id, sizeBytes: BigInt(4096), + purpose: UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, expiresAt: new Date(Date.now() + HOUR_MS), }, }); diff --git a/tests/api/lib-r2-video-finalize.test.ts b/tests/api/lib-r2-video-finalize.test.ts index c6151ba..5115f55 100644 --- a/tests/api/lib-r2-video-finalize.test.ts +++ b/tests/api/lib-r2-video-finalize.test.ts @@ -16,6 +16,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { randomUUID } from 'crypto'; import { db } from '@/lib/db'; +import { UPLOAD_RESERVATION_PURPOSES } from '@/lib/storage-quota'; import { deleteR2Object, deleteVideoObject, headVideoObject, readVideoObjectBytes } from '@/lib/r2'; import { createR2UploadToken } from '@/lib/r2-upload-token'; import { createR2UploadSession } from '@/lib/r2-upload-session'; @@ -515,6 +516,7 @@ describe('finalizeR2VideoUpload success', () => { data: { billedUserId: scenario.owner.id, sizeBytes: BigInt(4096), + purpose: UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, expiresAt: new Date(Date.now() + 30 * 60 * 1000), }, }); diff --git a/tests/api/storage-quota.test.ts b/tests/api/storage-quota.test.ts index d65357f..916f24d 100644 --- a/tests/api/storage-quota.test.ts +++ b/tests/api/storage-quota.test.ts @@ -15,6 +15,7 @@ import { db } from '@/lib/db'; import { getCachedUserBunnyStorage } from '@/lib/admin-stats'; import { PLAN_STORAGE_LIMIT_BYTES, + UPLOAD_RESERVATION_PURPOSES, enforceStorageQuota, getUserStorageInfo, getUserTotalStorageBytes, @@ -83,7 +84,11 @@ describe('the trial ceiling', () => { const user = await createUser(); await createUploadReservation({ billedUserId: user.id, sizeBytes: BigInt(2) * GIB }); - const result = await reserveStorageQuota(user.id, BigInt(2) * GIB); + const result = await reserveStorageQuota( + user.id, + BigInt(2) * GIB, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('error' in result).toBe(true); expect((result as { error: Response }).error.status).toBe(507); @@ -92,7 +97,11 @@ describe('the trial ceiling', () => { it('still lets a trial account upload inside its own ceiling', async () => { const user = await createUser(); - const result = await reserveStorageQuota(user.id, BigInt(1) * GIB); + const result = await reserveStorageQuota( + user.id, + BigInt(1) * GIB, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('reservationId' in result).toBe(true); }); @@ -277,7 +286,11 @@ describe('reserveStorageQuota', () => { it('writes a reservation row billed to the user with the requested size', async () => { const user = await createSubscribedUser(); - const result = await reserveStorageQuota(user.id, BigInt(4096)); + const result = await reserveStorageQuota( + user.id, + BigInt(4096), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('reservationId' in result).toBe(true); const reservation = await db.uploadReservation.findFirstOrThrow(); @@ -291,7 +304,11 @@ describe('reserveStorageQuota', () => { vi.stubEnv('OPENFRAME_ENABLE_STRIPE', 'false'); const user = await createSubscribedUser(); - const result = await reserveStorageQuota(user.id, BigInt(4096)); + const result = await reserveStorageQuota( + user.id, + BigInt(4096), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect(result).toEqual({ reservationId: null }); expect(await db.uploadReservation.count()).toBe(0); @@ -304,7 +321,11 @@ describe('reserveStorageQuota', () => { sizeBytes: PLAN_STORAGE_LIMIT_BYTES - BigInt(1024), }); - const result = await reserveStorageQuota(user.id, BigInt(2048)); + const result = await reserveStorageQuota( + user.id, + BigInt(2048), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('error' in result).toBe(true); expect('error' in result && result.error.status).toBe(507); @@ -321,7 +342,11 @@ describe('reserveStorageQuota', () => { sizeBytes: PLAN_STORAGE_LIMIT_BYTES - BigInt(100), }); - const result = await reserveStorageQuota(scenario.owner.id, BigInt(200)); + const result = await reserveStorageQuota( + scenario.owner.id, + BigInt(200), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('error' in result).toBe(true); expect(await db.uploadReservation.count()).toBe(0); @@ -331,7 +356,11 @@ describe('reserveStorageQuota', () => { const user = await createSubscribedUser(); bunnyStorage({ [user.id]: Number(PLAN_STORAGE_LIMIT_BYTES - BigInt(1024)) }); - const result = await reserveStorageQuota(user.id, BigInt(2048)); + const result = await reserveStorageQuota( + user.id, + BigInt(2048), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('error' in result).toBe(true); expect(await db.uploadReservation.count()).toBe(0); @@ -345,7 +374,11 @@ describe('reserveStorageQuota', () => { expiresInMs: -60_000, }); - const result = await reserveStorageQuota(user.id, BigInt(2048)); + const result = await reserveStorageQuota( + user.id, + BigInt(2048), + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('reservationId' in result).toBe(true); }); @@ -358,7 +391,11 @@ describe('reserveStorageQuota', () => { sizeBytes: PLAN_STORAGE_LIMIT_BYTES - BigInt(1024), }); - const result = await reserveStorageQuota(light.id, BigInt(10) * GIB); + const result = await reserveStorageQuota( + light.id, + BigInt(10) * GIB, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); expect('reservationId' in result).toBe(true); }); @@ -375,8 +412,8 @@ describe('reserveStorageQuota', () => { expect(headroom(used)).toBe(BigInt(30) * GIB); const [first, second] = await Promise.all([ - reserveStorageQuota(user.id, request), - reserveStorageQuota(user.id, request), + reserveStorageQuota(user.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), + reserveStorageQuota(user.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), ]); const granted = [first, second].filter((result) => 'reservationId' in result); @@ -401,8 +438,8 @@ describe('reserveStorageQuota', () => { const request = BigInt(20) * GIB; const results = await Promise.all([ - reserveStorageQuota(user.id, request), - reserveStorageQuota(user.id, request), + reserveStorageQuota(user.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), + reserveStorageQuota(user.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), ]); expect(results.every((result) => 'reservationId' in result)).toBe(true); @@ -417,7 +454,9 @@ describe('reserveStorageQuota', () => { const request = BigInt(20) * GIB; const results = await Promise.all( - Array.from({ length: 5 }, () => reserveStorageQuota(user.id, request)) + Array.from({ length: 5 }, () => + reserveStorageQuota(user.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO) + ) ); const granted = results.filter((result) => 'reservationId' in result); @@ -438,8 +477,8 @@ describe('reserveStorageQuota', () => { const request = BigInt(150) * GIB; const results = await Promise.all([ - reserveStorageQuota(first.id, request), - reserveStorageQuota(second.id, request), + reserveStorageQuota(first.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), + reserveStorageQuota(second.id, request, UPLOAD_RESERVATION_PURPOSES.R2_VIDEO), ]); expect(results.every((result) => 'reservationId' in result)).toBe(true); @@ -450,7 +489,11 @@ describe('reserveStorageQuota', () => { describe('releaseStorageReservation', () => { it('deletes the reservation and frees the headroom', async () => { const user = await createSubscribedUser(); - const result = await reserveStorageQuota(user.id, BigInt(10) * GIB); + const result = await reserveStorageQuota( + user.id, + BigInt(10) * GIB, + UPLOAD_RESERVATION_PURPOSES.R2_VIDEO + ); const reservationId = 'reservationId' in result ? result.reservationId : null; expect(reservationId).toBeTruthy(); expect(await getUserTotalStorageBytes(user.id)).toBe(BigInt(10) * GIB); @@ -484,6 +527,25 @@ describe('releaseStorageReservation', () => { expect(await db.uploadReservation.count()).toBe(1); }); + + // The same scoping in the other direction. Every hold an account owns is + // billed to the same user, so `billedUserId` alone does not separate them: an + // image being attached would release a Bunny upload that was still in flight + // if it could name it, and the ids are readable by the client. + it('refuses to delete a reservation opened for a different flow', async () => { + const owner = await createSubscribedUser(); + const reservation = await createUploadReservation({ + billedUserId: owner.id, + sizeBytes: BigInt(4096), + purpose: UPLOAD_RESERVATION_PURPOSES.BUNNY, + }); + + await releaseStorageReservation(reservation.id, owner.id, UPLOAD_RESERVATION_PURPOSES.IMAGE); + expect(await db.uploadReservation.count()).toBe(1); + + await releaseStorageReservation(reservation.id, owner.id, UPLOAD_RESERVATION_PURPOSES.BUNNY); + expect(await db.uploadReservation.count()).toBe(0); + }); }); describe('GET /api/settings/storage', () => { diff --git a/tests/factories/video.ts b/tests/factories/video.ts index c05ea04..3437ac7 100644 --- a/tests/factories/video.ts +++ b/tests/factories/video.ts @@ -7,6 +7,7 @@ import { type VideoVersion, } from '@prisma/client'; import { db } from '@/lib/db'; +import { UPLOAD_RESERVATION_PURPOSES, type UploadReservationPurpose } from '@/lib/storage-quota'; import { nextSeq } from './seq'; export interface CreateVideoInput { @@ -105,6 +106,8 @@ export interface CreateUploadReservationInput { sizeBytes: bigint; /** Milliseconds from now. Negative values produce an already-expired row. */ expiresInMs?: number; + /** Which flow the hold belongs to. Only that flow can consume it. */ + purpose?: UploadReservationPurpose; } export async function createUploadReservation( @@ -115,6 +118,7 @@ export async function createUploadReservation( billedUserId: input.billedUserId, sizeBytes: input.sizeBytes, expiresAt: new Date(Date.now() + (input.expiresInMs ?? 30 * 60 * 1000)), + purpose: input.purpose ?? UPLOAD_RESERVATION_PURPOSES.R2_VIDEO, }, }); } diff --git a/tests/setup/db-global.ts b/tests/setup/db-global.ts index ce68aed..f70eff3 100644 --- a/tests/setup/db-global.ts +++ b/tests/setup/db-global.ts @@ -67,6 +67,7 @@ const REVIEWED_MIGRATIONS = [ '20260614160000_add_project_allow_downloads', '20260627140000_add_video_upload_multipart_id', '20260801120000_add_acquisition_analytics', + '20260818120000_add_upload_reservation_purpose', ]; /** Objects POST_PUSH_SQL must have produced. Verified after it runs. */ diff --git a/tests/unit/lib/guest-upload-token.test.ts b/tests/unit/lib/guest-upload-token.test.ts new file mode 100644 index 0000000..04ee2a4 --- /dev/null +++ b/tests/unit/lib/guest-upload-token.test.ts @@ -0,0 +1,109 @@ +// The grant a share-link visitor gets for a direct upload. +// +// A guest's upload is billed to the workspace owner, not to the guest, so this +// token is the only thing tying what they declared and what they hold to the +// upload it was issued for. Two claims matter here beyond the existing subject +// binding: the provider's own video id, which is what makes releasing the hold +// on the guest's say-so safe, and the declared size, which is what the asset is +// charged until Bunny reports a figure of its own. + +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + createGuestUploadToken, + readGuestUploadGrant, + verifyGuestUploadToken, +} from '@/lib/guest-upload-token'; + +const SUBJECT = { + projectId: 'project-1', + videoId: 'video-1', + intent: 'bunny' as const, + context: '203.0.113.7:public', +}; + +const BUNNY_VIDEO_ID = 'bunnyvideo-1-abcdefgh'; + +beforeEach(() => { + vi.stubEnv('GUEST_UPLOAD_TOKEN_SECRET', 'test-guest-upload-token-secret'); +}); + +describe('readGuestUploadGrant', () => { + it('carries back the reservation and the declared size it was signed with', () => { + const token = createGuestUploadToken({ + ...SUBJECT, + providerVideoId: BUNNY_VIDEO_ID, + reservationId: 'reservation-1', + declaredSizeBytes: BigInt(4096), + }); + + expect(readGuestUploadGrant(token, SUBJECT, BUNNY_VIDEO_ID)).toEqual({ + reservationId: 'reservation-1', + declaredSizeBytes: BigInt(4096), + }); + }); + + // The binding that makes a guest release safe: presenting this token to cancel + // deletes the upload it stands for, so it cannot be used to drop the hold of + // an upload that is still running. + it('refuses a grant presented against a different provider video', () => { + const token = createGuestUploadToken({ + ...SUBJECT, + providerVideoId: BUNNY_VIDEO_ID, + reservationId: 'reservation-1', + }); + + expect(readGuestUploadGrant(token, SUBJECT, 'bunnyvideo-2-abcdefgh')).toBeNull(); + expect(readGuestUploadGrant(token, SUBJECT, null)).toBeNull(); + expect(verifyGuestUploadToken(token, SUBJECT, 'bunnyvideo-2-abcdefgh')).toBe(false); + }); + + it('still refuses a grant for another subject, bound video or not', () => { + const token = createGuestUploadToken({ + ...SUBJECT, + providerVideoId: BUNNY_VIDEO_ID, + reservationId: 'reservation-1', + }); + + expect( + readGuestUploadGrant(token, { ...SUBJECT, videoId: 'video-2' }, BUNNY_VIDEO_ID) + ).toBeNull(); + expect(readGuestUploadGrant(token, { ...SUBJECT, intent: 'image' }, BUNNY_VIDEO_ID)).toBeNull(); + expect( + readGuestUploadGrant(token, { ...SUBJECT, context: '198.51.100.9:public' }, BUNNY_VIDEO_ID) + ).toBeNull(); + }); + + // The image and audio grants carry none of this, and a grant issued before the + // claims existed keeps working rather than failing an upload in flight. + it('reads a grant with no claims as holding nothing', () => { + const token = createGuestUploadToken({ ...SUBJECT, intent: 'image' }); + const subject = { ...SUBJECT, intent: 'image' as const }; + + expect(readGuestUploadGrant(token, subject)).toEqual({ + reservationId: null, + declaredSizeBytes: null, + }); + expect(verifyGuestUploadToken(token, subject, BUNNY_VIDEO_ID)).toBe(true); + }); + + it('refuses a forged signature', () => { + const token = createGuestUploadToken({ + ...SUBJECT, + providerVideoId: BUNNY_VIDEO_ID, + reservationId: 'reservation-1', + }); + const [payload] = token.split('.'); + + expect(readGuestUploadGrant(`${payload}.forged`, SUBJECT, BUNNY_VIDEO_ID)).toBeNull(); + }); + + it('reads a non-positive declared size as nothing declared', () => { + const token = createGuestUploadToken({ + ...SUBJECT, + providerVideoId: BUNNY_VIDEO_ID, + declaredSizeBytes: BigInt(-1), + }); + + expect(readGuestUploadGrant(token, SUBJECT, BUNNY_VIDEO_ID)?.declaredSizeBytes).toBeNull(); + }); +});