From d0e1e61aa21a7736a2085d8c0acf3e0b2ffacf88 Mon Sep 17 00:00:00 2001 From: Basharameez Date: Wed, 9 Sep 2026 20:08:22 +0530 Subject: [PATCH 1/2] fix: enforce required file size limit on presigned upload route and R2 PutObjectCommand --- app/api/upload/presign/route.test.ts | 127 +++++++++++++++++++++++++++ app/api/upload/presign/route.ts | 12 ++- lib/r2/upload.test.ts | 14 +++ lib/r2/upload.ts | 2 + 4 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 app/api/upload/presign/route.test.ts diff --git a/app/api/upload/presign/route.test.ts b/app/api/upload/presign/route.test.ts new file mode 100644 index 0000000..61a224c --- /dev/null +++ b/app/api/upload/presign/route.test.ts @@ -0,0 +1,127 @@ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const hashIp = vi.fn(() => "hashed-ip"); +const getClientIp = vi.fn(() => "127.0.0.1"); +const consumeSharedRateLimit = vi.fn(async () => ({ + allowed: true, + remaining: 9, + resetAt: new Date().toISOString(), + currentCount: 1, +})); +const getPresignedUploadUrl = vi.fn(async () => ({ + uploadUrl: "https://upload.example/signed", + publicUrl: "https://cdn.example/screenshots/test.png", + key: "screenshots/test.png", +})); + +vi.mock("@/lib/utils/hash", () => ({ + hashIp, + getClientIp, +})); + +vi.mock("@/lib/rate-limit/shared", () => ({ + consumeSharedRateLimit, +})); + +vi.mock("@/lib/observability/events", () => ({ + logEvent: vi.fn(), +})); + +vi.mock("@/lib/r2/upload", () => ({ + getPresignedUploadUrl, +})); + +function createRequest(body: unknown) { + return new NextRequest("http://localhost/api/upload/presign", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +describe("POST /api/upload/presign", () => { + beforeEach(() => { + vi.resetModules(); + vi.clearAllMocks(); + consumeSharedRateLimit.mockResolvedValue({ + allowed: true, + remaining: 9, + resetAt: new Date().toISOString(), + currentCount: 1, + }); + }); + + it("returns 400 if size is missing from request body", async () => { + const { POST } = await import("./route"); + const response = await POST( + createRequest({ + filename: "screenshot.png", + contentType: "image/png", + }), + ); + + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toBeDefined(); + expect(getPresignedUploadUrl).not.toHaveBeenCalled(); + }); + + it("returns 400 if size exceeds 5 MB limit", async () => { + const { POST } = await import("./route"); + const response = await POST( + createRequest({ + filename: "large.png", + contentType: "image/png", + size: 6 * 1024 * 1024, + }), + ); + + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toBe("File must be under 5 MB."); + expect(getPresignedUploadUrl).not.toHaveBeenCalled(); + }); + + it("generates presigned upload URL with ContentLength when valid size is provided", async () => { + const { POST } = await import("./route"); + const response = await POST( + createRequest({ + filename: "valid.png", + contentType: "image/png", + size: 2 * 1024 * 1024, + }), + ); + + expect(response.status).toBe(200); + const data = await response.json(); + expect(data.url).toBe("https://upload.example/signed"); + expect(getPresignedUploadUrl).toHaveBeenCalledWith( + "valid.png", + "image/png", + "screenshots", + 2 * 1024 * 1024, + ); + }); + + it("returns 429 when rate limit is exceeded", async () => { + consumeSharedRateLimit.mockResolvedValue({ + allowed: false, + remaining: 0, + resetAt: new Date().toISOString(), + currentCount: 11, + }); + + const { POST } = await import("./route"); + const response = await POST( + createRequest({ + filename: "valid.png", + contentType: "image/png", + size: 1024, + }), + ); + + expect(response.status).toBe(429); + expect(getPresignedUploadUrl).not.toHaveBeenCalled(); + }); +}); diff --git a/app/api/upload/presign/route.ts b/app/api/upload/presign/route.ts index 479e0d0..7a0b733 100644 --- a/app/api/upload/presign/route.ts +++ b/app/api/upload/presign/route.ts @@ -13,7 +13,10 @@ const schema = z.object({ contentType: z.string().refine((t) => ALLOWED_TYPES.includes(t), { message: "Only JPEG, PNG, WEBP, and GIF images are allowed.", }), - size: z.number().max(MAX_SIZE_BYTES, "File must be under 5 MB.").optional(), + size: z + .number({ required_error: "File size is required." }) + .min(1, "File size must be greater than 0 bytes.") + .max(MAX_SIZE_BYTES, "File must be under 5 MB."), }); const WINDOW_SECONDS = 10 * 60; @@ -52,17 +55,12 @@ export async function POST(req: NextRequest) { } const { filename, contentType, size } = parsed.data; - if (size != null && size > MAX_SIZE_BYTES) { - return NextResponse.json( - { error: "File must be under 5 MB." }, - { status: 400 }, - ); - } const result = await getPresignedUploadUrl( filename, contentType, "screenshots", + size, ); return NextResponse.json({ diff --git a/lib/r2/upload.test.ts b/lib/r2/upload.test.ts index 5e672a3..8056695 100644 --- a/lib/r2/upload.test.ts +++ b/lib/r2/upload.test.ts @@ -82,6 +82,20 @@ describe("getPresignedUploadUrl", () => { expect(command.Metadata?.["original-filename"]).toBe("my_photo__1_.png"); }); + it("includes ContentLength in PutObjectCommand when size is provided", async () => { + await upload.getPresignedUploadUrl( + "photo.png", + "image/png", + "screenshots", + 1048576, + ); + + const command = putObjectCommand.mock.calls[0][0] as { + ContentLength?: number; + }; + expect(command.ContentLength).toBe(1048576); + }); + it("reuses one client for upload and read presigns", async () => { await upload.getPresignedUploadUrl("first.png", "image/png"); await upload.getPresignedReadUrl("screenshots/existing.png"); diff --git a/lib/r2/upload.ts b/lib/r2/upload.ts index 1d64c09..e7586b3 100644 --- a/lib/r2/upload.ts +++ b/lib/r2/upload.ts @@ -71,6 +71,7 @@ export async function getPresignedUploadUrl( filename: string, contentType: string, folder = "screenshots", + contentLength?: number, ): Promise { const ext = extensionForContentType(contentType); const key = `${folder}/${randomUUID()}.${ext}`; @@ -80,6 +81,7 @@ export async function getPresignedUploadUrl( Bucket: bucketName, Key: key, ContentType: contentType, + ...(contentLength != null ? { ContentLength: contentLength } : {}), Metadata: { "original-filename": sanitizeFilenameForMetadata(filename) }, }); From 962b796f4ec1f4537c9f9c9e3638f61a6659f715 Mon Sep 17 00:00:00 2001 From: Basharameez Date: Wed, 9 Sep 2026 20:13:50 +0530 Subject: [PATCH 2/2] fix(presign): add integer validation to size schema and edge case test coverage --- app/api/upload/presign/route.test.ts | 57 ++++++++++++++++++++++++++++ app/api/upload/presign/route.ts | 3 +- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/api/upload/presign/route.test.ts b/app/api/upload/presign/route.test.ts index 61a224c..8e18954 100644 --- a/app/api/upload/presign/route.test.ts +++ b/app/api/upload/presign/route.test.ts @@ -83,6 +83,63 @@ describe("POST /api/upload/presign", () => { expect(getPresignedUploadUrl).not.toHaveBeenCalled(); }); + it("returns 400 if size is 0 or negative", async () => { + const { POST } = await import("./route"); + const responseZero = await POST( + createRequest({ + filename: "zero.png", + contentType: "image/png", + size: 0, + }), + ); + expect(responseZero.status).toBe(400); + + const responseNegative = await POST( + createRequest({ + filename: "negative.png", + contentType: "image/png", + size: -500, + }), + ); + expect(responseNegative.status).toBe(400); + expect(getPresignedUploadUrl).not.toHaveBeenCalled(); + }); + + it("returns 400 if size is not an integer", async () => { + const { POST } = await import("./route"); + const response = await POST( + createRequest({ + filename: "float.png", + contentType: "image/png", + size: 1024.5, + }), + ); + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toBe("File size must be an integer byte count."); + expect(getPresignedUploadUrl).not.toHaveBeenCalled(); + }); + + it("allows exactly 5 MB upload size", async () => { + const { POST } = await import("./route"); + const exact5MB = 5 * 1024 * 1024; + const response = await POST( + createRequest({ + filename: "exact5mb.png", + contentType: "image/png", + size: exact5MB, + }), + ); + + expect(response.status).toBe(200); + expect(getPresignedUploadUrl).toHaveBeenCalledWith( + "exact5mb.png", + "image/png", + "screenshots", + exact5MB, + ); + }); + it("generates presigned upload URL with ContentLength when valid size is provided", async () => { const { POST } = await import("./route"); const response = await POST( diff --git a/app/api/upload/presign/route.ts b/app/api/upload/presign/route.ts index 7a0b733..308fe4d 100644 --- a/app/api/upload/presign/route.ts +++ b/app/api/upload/presign/route.ts @@ -14,7 +14,8 @@ const schema = z.object({ message: "Only JPEG, PNG, WEBP, and GIF images are allowed.", }), size: z - .number({ required_error: "File size is required." }) + .number() + .int("File size must be an integer byte count.") .min(1, "File size must be greater than 0 bytes.") .max(MAX_SIZE_BYTES, "File must be under 5 MB."), });