From 24a0832f463f4fcb5bc11139a784385a258170ba Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Fri, 19 Jun 2026 09:47:13 +0100 Subject: [PATCH 1/4] fix(web): cap maximum upload size --- .../app/api/upload/[...route]/multipart.ts | 38 +++++++++++++++++++ .../src/S3Buckets/S3BucketAccess.ts | 13 +++++++ 2 files changed, 51 insertions(+) diff --git a/apps/web/app/api/upload/[...route]/multipart.ts b/apps/web/app/api/upload/[...route]/multipart.ts index 5947b0775b5..7988e9ddc05 100644 --- a/apps/web/app/api/upload/[...route]/multipart.ts +++ b/apps/web/app/api/upload/[...route]/multipart.ts @@ -34,6 +34,10 @@ const MEDIA_SERVER_PRESIGNED_PUT_EXPIRES_SECONDS = 3 * 60 * 60; // Clients stop at the cap and then finalize, so reported durations can land // slightly past the limit for honest recordings. const FREE_PLAN_DURATION_GRACE_SECONDS = 30; +// Upper bound on a completed multipart upload to prevent unbounded storage +// abuse. Generous on purpose so legitimate long/high-bitrate recordings are +// never blocked; kept in sync with MAX_UPLOAD_BYTES in S3BucketAccess.ts. +const MAX_UPLOAD_BYTES = 100 * 1024 * 1024 * 1024; // 100 GiB const runPromiseAnyEnv = runPromise as ( effect: Effect.Effect, @@ -399,6 +403,40 @@ app.post( } } + // Server-side backstop for the maximum upload size. Presigned POST URLs + // enforce a content-length-range policy, but presigned PUT part URLs + // cannot enforce a total size, so reject an oversized assembled upload + // here before persisting (and before paying to assemble it). Part sizes + // are client-reported, so this raises the bar rather than enforcing + // authoritatively. + const totalUploadSize = parts.reduce((acc, part) => acc + part.size, 0); + if (totalUploadSize > MAX_UPLOAD_BYTES) { + // Avoid leaving the parts as incomplete-MPU storage and a stale + // videoUploads row, mirroring the free-plan rejection cleanup. The + // 413 stands regardless of cleanup success. + yield* Effect.gen(function* () { + const [bucket] = yield* Storage.getAccessForVideo(video); + yield* bucket.multipart.abort(fileKey, uploadId); + yield* db.use((db) => + db + .delete(Db.videoUploads) + .where(eq(Db.videoUploads.videoId, videoId)), + ); + }).pipe( + Effect.catchAll((error) => + Effect.logError( + "Failed to clean up rejected oversized multipart upload", + error, + ), + ), + ); + + c.status(413); + return c.text( + "Upload exceeds the maximum allowed size and cannot be completed.", + ); + } + return yield* Effect.gen(function* () { const [bucket] = yield* Storage.getAccessForVideo(video); diff --git a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts index 106e98985a3..97120848187 100644 --- a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts +++ b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts @@ -14,6 +14,11 @@ import { S3BucketClientProvider } from "./S3BucketClientProvider.ts"; const DEFAULT_PRESIGNED_GET_EXPIRES_SECONDS = 3600; const DEFAULT_PRESIGNED_PUT_EXPIRES_SECONDS = 3600; +// Upper bound on a single upload to prevent unbounded storage abuse. Generous +// on purpose so legitimate long/high-bitrate recordings are never blocked; +// tune here if the product ever needs a larger ceiling. +export const MAX_UPLOAD_BYTES = 100 * 1024 * 1024 * 1024; // 100 GiB + type NodeReadableWebStream = Parameters[0]; const wrapS3Promise = ( @@ -269,6 +274,14 @@ export const createS3BucketAccess = Effect.gen(function* () { Effect.map((client) => createPresignedPost(client, { ...args, + // Enforce an upper bound on the uploaded object size. The POST + // policy rejects the upload at S3 if the body exceeds this, + // closing the unbounded-storage hole for presigned POSTs. + // Any caller-supplied conditions are preserved. + Conditions: [ + ["content-length-range", 0, MAX_UPLOAD_BYTES], + ...(args.Conditions ?? []), + ], Bucket: provider.bucket, Key: key, }), From 4c0d4e705664c6a61ee5210c0c32adb3b29f7a5c Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Fri, 19 Jun 2026 10:16:02 +0100 Subject: [PATCH 2/4] fix(web): single-source the upload size cap and guard duplicate POST conditions --- .../app/api/upload/[...route]/multipart.ts | 11 ++++--- .../src/S3Buckets/S3BucketAccess.ts | 33 ++++++++++++------- packages/web-backend/src/index.ts | 1 + 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/apps/web/app/api/upload/[...route]/multipart.ts b/apps/web/app/api/upload/[...route]/multipart.ts index 7988e9ddc05..cacb35f2042 100644 --- a/apps/web/app/api/upload/[...route]/multipart.ts +++ b/apps/web/app/api/upload/[...route]/multipart.ts @@ -4,6 +4,7 @@ import { serverEnv } from "@cap/env"; import { userIsPro } from "@cap/utils"; import { Database, + MAX_UPLOAD_BYTES, makeCurrentUserLayer, provideOptionalAuth, Storage, @@ -34,10 +35,6 @@ const MEDIA_SERVER_PRESIGNED_PUT_EXPIRES_SECONDS = 3 * 60 * 60; // Clients stop at the cap and then finalize, so reported durations can land // slightly past the limit for honest recordings. const FREE_PLAN_DURATION_GRACE_SECONDS = 30; -// Upper bound on a completed multipart upload to prevent unbounded storage -// abuse. Generous on purpose so legitimate long/high-bitrate recordings are -// never blocked; kept in sync with MAX_UPLOAD_BYTES in S3BucketAccess.ts. -const MAX_UPLOAD_BYTES = 100 * 1024 * 1024 * 1024; // 100 GiB const runPromiseAnyEnv = runPromise as ( effect: Effect.Effect, @@ -409,7 +406,11 @@ app.post( // here before persisting (and before paying to assemble it). Part sizes // are client-reported, so this raises the bar rather than enforcing // authoritatively. - const totalUploadSize = parts.reduce((acc, part) => acc + part.size, 0); + let totalUploadSize = 0; + for (const part of parts) { + totalUploadSize += part.size; + if (totalUploadSize > MAX_UPLOAD_BYTES) break; + } if (totalUploadSize > MAX_UPLOAD_BYTES) { // Avoid leaving the parts as incomplete-MPU storage and a stale // videoUploads row, mirroring the free-plan rejection cleanup. The diff --git a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts index 97120848187..3a37df80846 100644 --- a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts +++ b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts @@ -271,21 +271,30 @@ export const createS3BucketAccess = Effect.gen(function* () { ) => wrapS3Promise( provider.getPublic.pipe( - Effect.map((client) => - createPresignedPost(client, { + Effect.map((client) => { + // Enforce an upper bound on the uploaded object size. The POST + // policy rejects the upload at S3 if the body exceeds this, + // closing the unbounded-storage hole for presigned POSTs. If a + // caller already supplies a content-length-range we defer to it + // rather than emitting a second, conflicting range. + const callerConditions = args.Conditions ?? []; + const hasContentLengthRange = callerConditions.some( + (condition) => + Array.isArray(condition) && + condition[0] === "content-length-range", + ); + return createPresignedPost(client, { ...args, - // Enforce an upper bound on the uploaded object size. The POST - // policy rejects the upload at S3 if the body exceeds this, - // closing the unbounded-storage hole for presigned POSTs. - // Any caller-supplied conditions are preserved. - Conditions: [ - ["content-length-range", 0, MAX_UPLOAD_BYTES], - ...(args.Conditions ?? []), - ], + Conditions: hasContentLengthRange + ? callerConditions + : [ + ["content-length-range", 0, MAX_UPLOAD_BYTES], + ...callerConditions, + ], Bucket: provider.bucket, Key: key, - }), - ), + }); + }), ), ), multipart: { diff --git a/packages/web-backend/src/index.ts b/packages/web-backend/src/index.ts index 7cd60bc8fb6..2c235b94692 100644 --- a/packages/web-backend/src/index.ts +++ b/packages/web-backend/src/index.ts @@ -10,6 +10,7 @@ export { Organisations } from "./Organisations/index.ts"; export { OrganisationsPolicy } from "./Organisations/OrganisationsPolicy.ts"; export * from "./Rpcs.ts"; export { S3Buckets } from "./S3Buckets/index.ts"; +export { MAX_UPLOAD_BYTES } from "./S3Buckets/S3BucketAccess.ts"; export { Spaces } from "./Spaces/index.ts"; export { SpacesPolicy } from "./Spaces/SpacesPolicy.ts"; export * from "./Storage/GoogleDrive.ts"; From 6637e5cc113a526548f4e076ad740f90ff49c7a1 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Fri, 19 Jun 2026 10:50:18 +0100 Subject: [PATCH 3/4] fix(web): reject negative part sizes, clamp POST size range, harden cleanup --- .../app/api/upload/[...route]/multipart.ts | 39 ++++++++++++++---- .../src/S3Buckets/S3BucketAccess.ts | 41 +++++++++++++------ 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/apps/web/app/api/upload/[...route]/multipart.ts b/apps/web/app/api/upload/[...route]/multipart.ts index cacb35f2042..9771629af13 100644 --- a/apps/web/app/api/upload/[...route]/multipart.ts +++ b/apps/web/app/api/upload/[...route]/multipart.ts @@ -291,7 +291,9 @@ app.post( z.object({ partNumber: z.number(), etag: z.string(), - size: z.number(), + // Non-negative so a negative size can't drag the summed total + // below the cap and bypass the upload-size limit. + size: z.number().nonnegative(), }), ), durationInSecs: stringOrNumberOptional, @@ -413,16 +415,35 @@ app.post( } if (totalUploadSize > MAX_UPLOAD_BYTES) { // Avoid leaving the parts as incomplete-MPU storage and a stale - // videoUploads row, mirroring the free-plan rejection cleanup. The - // 413 stands regardless of cleanup success. + // videoUploads row, mirroring the free-plan rejection cleanup. Each + // step is caught independently so a failed abort doesn't skip the DB + // cleanup (and vice versa). The 413 stands regardless of either. yield* Effect.gen(function* () { const [bucket] = yield* Storage.getAccessForVideo(video); - yield* bucket.multipart.abort(fileKey, uploadId); - yield* db.use((db) => - db - .delete(Db.videoUploads) - .where(eq(Db.videoUploads.videoId, videoId)), - ); + yield* bucket.multipart + .abort(fileKey, uploadId) + .pipe( + Effect.catchAll((error) => + Effect.logError( + "Failed to abort rejected oversized multipart upload", + error, + ), + ), + ); + yield* db + .use((db) => + db + .delete(Db.videoUploads) + .where(eq(Db.videoUploads.videoId, videoId)), + ) + .pipe( + Effect.catchAll((error) => + Effect.logError( + "Failed to delete videoUploads row for rejected upload", + error, + ), + ), + ); }).pipe( Effect.catchAll((error) => Effect.logError( diff --git a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts index 3a37df80846..00577ba8ccc 100644 --- a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts +++ b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts @@ -274,23 +274,38 @@ export const createS3BucketAccess = Effect.gen(function* () { Effect.map((client) => { // Enforce an upper bound on the uploaded object size. The POST // policy rejects the upload at S3 if the body exceeds this, - // closing the unbounded-storage hole for presigned POSTs. If a - // caller already supplies a content-length-range we defer to it - // rather than emitting a second, conflicting range. + // closing the unbounded-storage hole for presigned POSTs. We emit + // exactly one content-length-range whose max never exceeds + // MAX_UPLOAD_BYTES, even if a caller supplied a looser one, so a + // caller can tighten but never raise/disable the cap. const callerConditions = args.Conditions ?? []; - const hasContentLengthRange = callerConditions.some( - (condition) => - Array.isArray(condition) && - condition[0] === "content-length-range", + const isLengthRange = (condition: unknown): boolean => + Array.isArray(condition) && + condition[0] === "content-length-range"; + const callerRange = callerConditions.find(isLengthRange) as + | [unknown, unknown, unknown] + | undefined; + const callerMin = + callerRange && typeof callerRange[1] === "number" + ? Math.max(0, callerRange[1]) + : 0; + const callerMax = + callerRange && typeof callerRange[2] === "number" + ? callerRange[2] + : MAX_UPLOAD_BYTES; + const otherConditions = callerConditions.filter( + (condition) => !isLengthRange(condition), ); return createPresignedPost(client, { ...args, - Conditions: hasContentLengthRange - ? callerConditions - : [ - ["content-length-range", 0, MAX_UPLOAD_BYTES], - ...callerConditions, - ], + Conditions: [ + [ + "content-length-range", + callerMin, + Math.min(callerMax, MAX_UPLOAD_BYTES), + ], + ...otherConditions, + ], Bucket: provider.bucket, Key: key, }); From a663a44eaaa391eaddaff8e7b4b6f4c818e4ad6a Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Fri, 19 Jun 2026 10:55:29 +0100 Subject: [PATCH 4/4] fix(web): require integer part sizes and ignore non-finite POST size bounds --- apps/web/app/api/upload/[...route]/multipart.ts | 7 ++++--- packages/web-backend/src/S3Buckets/S3BucketAccess.ts | 10 +++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/web/app/api/upload/[...route]/multipart.ts b/apps/web/app/api/upload/[...route]/multipart.ts index 9771629af13..0541bfcbd3a 100644 --- a/apps/web/app/api/upload/[...route]/multipart.ts +++ b/apps/web/app/api/upload/[...route]/multipart.ts @@ -291,9 +291,10 @@ app.post( z.object({ partNumber: z.number(), etag: z.string(), - // Non-negative so a negative size can't drag the summed total - // below the cap and bypass the upload-size limit. - size: z.number().nonnegative(), + // A non-negative integer (bytes) so neither a negative nor a + // fractional size can drag the summed total below the cap and + // bypass the upload-size limit. + size: z.number().int().nonnegative(), }), ), durationInSecs: stringOrNumberOptional, diff --git a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts index 00577ba8ccc..b5b7c11a0dc 100644 --- a/packages/web-backend/src/S3Buckets/S3BucketAccess.ts +++ b/packages/web-backend/src/S3Buckets/S3BucketAccess.ts @@ -286,12 +286,16 @@ export const createS3BucketAccess = Effect.gen(function* () { | [unknown, unknown, unknown] | undefined; const callerMin = - callerRange && typeof callerRange[1] === "number" + callerRange && + typeof callerRange[1] === "number" && + Number.isFinite(callerRange[1]) ? Math.max(0, callerRange[1]) : 0; const callerMax = - callerRange && typeof callerRange[2] === "number" - ? callerRange[2] + callerRange && + typeof callerRange[2] === "number" && + Number.isFinite(callerRange[2]) + ? Math.max(0, callerRange[2]) : MAX_UPLOAD_BYTES; const otherConditions = callerConditions.filter( (condition) => !isLengthRange(condition),