Skip to content

Commit 6637e5c

Browse files
committed
fix(web): reject negative part sizes, clamp POST size range, harden cleanup
1 parent 4c0d4e7 commit 6637e5c

2 files changed

Lines changed: 58 additions & 22 deletions

File tree

apps/web/app/api/upload/[...route]/multipart.ts

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,9 @@ app.post(
291291
z.object({
292292
partNumber: z.number(),
293293
etag: z.string(),
294-
size: z.number(),
294+
// Non-negative so a negative size can't drag the summed total
295+
// below the cap and bypass the upload-size limit.
296+
size: z.number().nonnegative(),
295297
}),
296298
),
297299
durationInSecs: stringOrNumberOptional,
@@ -413,16 +415,35 @@ app.post(
413415
}
414416
if (totalUploadSize > MAX_UPLOAD_BYTES) {
415417
// Avoid leaving the parts as incomplete-MPU storage and a stale
416-
// videoUploads row, mirroring the free-plan rejection cleanup. The
417-
// 413 stands regardless of cleanup success.
418+
// videoUploads row, mirroring the free-plan rejection cleanup. Each
419+
// step is caught independently so a failed abort doesn't skip the DB
420+
// cleanup (and vice versa). The 413 stands regardless of either.
418421
yield* Effect.gen(function* () {
419422
const [bucket] = yield* Storage.getAccessForVideo(video);
420-
yield* bucket.multipart.abort(fileKey, uploadId);
421-
yield* db.use((db) =>
422-
db
423-
.delete(Db.videoUploads)
424-
.where(eq(Db.videoUploads.videoId, videoId)),
425-
);
423+
yield* bucket.multipart
424+
.abort(fileKey, uploadId)
425+
.pipe(
426+
Effect.catchAll((error) =>
427+
Effect.logError(
428+
"Failed to abort rejected oversized multipart upload",
429+
error,
430+
),
431+
),
432+
);
433+
yield* db
434+
.use((db) =>
435+
db
436+
.delete(Db.videoUploads)
437+
.where(eq(Db.videoUploads.videoId, videoId)),
438+
)
439+
.pipe(
440+
Effect.catchAll((error) =>
441+
Effect.logError(
442+
"Failed to delete videoUploads row for rejected upload",
443+
error,
444+
),
445+
),
446+
);
426447
}).pipe(
427448
Effect.catchAll((error) =>
428449
Effect.logError(

packages/web-backend/src/S3Buckets/S3BucketAccess.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -274,23 +274,38 @@ export const createS3BucketAccess = Effect.gen(function* () {
274274
Effect.map((client) => {
275275
// Enforce an upper bound on the uploaded object size. The POST
276276
// policy rejects the upload at S3 if the body exceeds this,
277-
// closing the unbounded-storage hole for presigned POSTs. If a
278-
// caller already supplies a content-length-range we defer to it
279-
// rather than emitting a second, conflicting range.
277+
// closing the unbounded-storage hole for presigned POSTs. We emit
278+
// exactly one content-length-range whose max never exceeds
279+
// MAX_UPLOAD_BYTES, even if a caller supplied a looser one, so a
280+
// caller can tighten but never raise/disable the cap.
280281
const callerConditions = args.Conditions ?? [];
281-
const hasContentLengthRange = callerConditions.some(
282-
(condition) =>
283-
Array.isArray(condition) &&
284-
condition[0] === "content-length-range",
282+
const isLengthRange = (condition: unknown): boolean =>
283+
Array.isArray(condition) &&
284+
condition[0] === "content-length-range";
285+
const callerRange = callerConditions.find(isLengthRange) as
286+
| [unknown, unknown, unknown]
287+
| undefined;
288+
const callerMin =
289+
callerRange && typeof callerRange[1] === "number"
290+
? Math.max(0, callerRange[1])
291+
: 0;
292+
const callerMax =
293+
callerRange && typeof callerRange[2] === "number"
294+
? callerRange[2]
295+
: MAX_UPLOAD_BYTES;
296+
const otherConditions = callerConditions.filter(
297+
(condition) => !isLengthRange(condition),
285298
);
286299
return createPresignedPost(client, {
287300
...args,
288-
Conditions: hasContentLengthRange
289-
? callerConditions
290-
: [
291-
["content-length-range", 0, MAX_UPLOAD_BYTES],
292-
...callerConditions,
293-
],
301+
Conditions: [
302+
[
303+
"content-length-range",
304+
callerMin,
305+
Math.min(callerMax, MAX_UPLOAD_BYTES),
306+
],
307+
...otherConditions,
308+
],
294309
Bucket: provider.bucket,
295310
Key: key,
296311
});

0 commit comments

Comments
 (0)