Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions apps/web/app/api/upload/[...route]/multipart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
richiemcilroy marked this conversation as resolved.
Outdated
Comment thread
richiemcilroy marked this conversation as resolved.
Outdated

const runPromiseAnyEnv = runPromise as <A, E>(
effect: Effect.Effect<A, E, unknown>,
Expand Down Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth calling out: this guard trusts part.size from the request body. A malicious client can under-report sizes and still complete an oversized MPU (S3 only cares about PartNumber/ETag). If the intent is a hard storage cap, you probably need to sum authoritative sizes from S3 (ListParts) or enforce Content-Length on the presigned part PUTs.

// are client-reported, so this raises the bar rather than enforcing
// authoritatively.
const totalUploadSize = parts.reduce((acc, part) => acc + part.size, 0);
Comment thread
richiemcilroy marked this conversation as resolved.
Outdated
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* () {
Comment thread
richiemcilroy marked this conversation as resolved.
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);

Expand Down
13 changes: 13 additions & 0 deletions packages/web-backend/src/S3Buckets/S3BucketAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof Readable.fromWeb>[0];

const wrapS3Promise = <T>(
Expand Down Expand Up @@ -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: [
Comment thread
richiemcilroy marked this conversation as resolved.
["content-length-range", 0, MAX_UPLOAD_BYTES],
...(args.Conditions ?? []),
],
Comment on lines +288 to +312

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One edge case: if the caller supplies a content-length-range where min exceeds MAX_UPLOAD_BYTES (or min > max), we’ll generate an invalid policy (min > max). Might be worth clamping + normalizing so we always emit a valid range.

Suggested change
const callerMin =
callerRange &&
typeof callerRange[1] === "number" &&
Number.isFinite(callerRange[1])
? Math.max(0, callerRange[1])
: 0;
const callerMax =
callerRange &&
typeof callerRange[2] === "number" &&
Number.isFinite(callerRange[2])
? Math.max(0, callerRange[2])
: MAX_UPLOAD_BYTES;
const otherConditions = callerConditions.filter(
(condition) => !isLengthRange(condition),
);
return createPresignedPost(client, {
...args,
Conditions: [
[
"content-length-range",
callerMin,
Math.min(callerMax, MAX_UPLOAD_BYTES),
],
...otherConditions,
],
const callerMin =
callerRange &&
typeof callerRange[1] === "number" &&
Number.isFinite(callerRange[1])
? Math.max(0, callerRange[1])
: 0;
const callerMax =
callerRange &&
typeof callerRange[2] === "number" &&
Number.isFinite(callerRange[2])
? Math.max(0, callerRange[2])
: MAX_UPLOAD_BYTES;
const minBytes = Math.min(callerMin, MAX_UPLOAD_BYTES);
const maxBytes = Math.min(Math.max(callerMax, minBytes), MAX_UPLOAD_BYTES);
const otherConditions = callerConditions.filter(
(condition) => !isLengthRange(condition),
);
return createPresignedPost(client, {
...args,
Conditions: [
["content-length-range", minBytes, maxBytes],
...otherConditions,
],
Bucket: provider.bucket,
Key: key,
});

Bucket: provider.bucket,
Key: key,
}),
Expand Down
Loading