From b03ce534ab91bf8832c39872df28db5ae2555a9f Mon Sep 17 00:00:00 2001 From: Gabriel Taveira Date: Mon, 31 Aug 2026 12:08:25 -0300 Subject: [PATCH] fix(api): constrain image upload grants --- .../ProfileImageUploader/utils/index.ts | 36 ++-- .../app/api/queues/cleanup-upload/route.ts | 10 + packages/api/src/queue/enqueue.ts | 2 + .../api/src/queue/handlers/process-image.ts | 3 +- .../api/src/queue/handlers/upload.test.ts | 48 +++++ packages/api/src/queue/handlers/upload.ts | 29 +++ packages/api/src/queue/topics.ts | 7 + packages/api/src/routes/image.ts | 13 +- packages/api/src/routes/input-schemas.ts | 6 + .../api/src/routes/input-validation.test.ts | 26 +++ packages/api/src/services/dog-service.test.ts | 33 ++++ packages/api/src/services/dog-service.ts | 26 ++- .../api/src/services/image-service.test.ts | 178 +++++++++++++++++- packages/api/src/services/image-service.ts | 173 ++++++++++++++++- .../api/src/services/user-service.test.ts | 27 ++- packages/api/src/services/user-service.ts | 29 ++- packages/api/src/shared/file-upload.ts | 39 +++- .../migration.sql | 22 +++ packages/database/schema.prisma | 17 ++ packages/shared/constants/constants.ts | 1 + packages/shared/errors/errors.ts | 30 +++ vercel.json | 9 + 22 files changed, 712 insertions(+), 52 deletions(-) create mode 100644 apps/nextjs/src/app/api/queues/cleanup-upload/route.ts create mode 100644 packages/api/src/queue/handlers/upload.test.ts create mode 100644 packages/api/src/queue/handlers/upload.ts create mode 100644 packages/database/migrations/20260831150000_add_upload_grants/migration.sql diff --git a/apps/mobile/src/components/ProfileImageUploader/utils/index.ts b/apps/mobile/src/components/ProfileImageUploader/utils/index.ts index 25d2106c..413f873d 100644 --- a/apps/mobile/src/components/ProfileImageUploader/utils/index.ts +++ b/apps/mobile/src/components/ProfileImageUploader/utils/index.ts @@ -243,10 +243,9 @@ export class ProfileImageUploadError extends Error { * the Maestro placeholder skip affordance (`shouldOfferMaestroPlaceholder`). * * Steps: - * 1. presign — request an upload descriptor from the API: method, url, - * headers to send, and the object's canonical public URL - * 2. compress — re-encode to WEBP @ 0.8 quality (expensive, kept after the - * caller has already shown optimistic visual feedback) + * 1. compress — re-encode to WEBP @ 0.8 quality + * 2. presign — send the exact byte length and request an upload descriptor: + * method, url, headers, and the object's canonical public URL * 3. upload — send the bytes exactly as the descriptor says, via * expo-file-system's BINARY_CONTENT mode * 4. finalize — return the descriptor's `publicUrl` @@ -264,9 +263,29 @@ export const uploadProfileImage = async ( localUri: string, onProgress?: (stage: ProfileImageUploadStage) => void, ): Promise => { + onProgress?.("compress"); + const compressedImage = await compressImage(localUri).catch((error) => { + throw new ProfileImageUploadError("compress", "compressImage failed", { + cause: error, + }); + }); + const compressedImageInfo = await getInfoAsync(compressedImage.uri); + if ( + !compressedImageInfo.exists || + typeof compressedImageInfo.size !== "number" + ) { + throw new ProfileImageUploadError( + "compress", + "Compressed photo size is unavailable", + ); + } + onProgress?.("presign"); const upload = await getTrcpContext() - .image.signedUpload.fetch() + .image.signedUpload.fetch({ + contentLength: compressedImageInfo.size, + contentType: "image/webp", + }) .catch((error) => { throw new ProfileImageUploadError( "presign", @@ -275,13 +294,6 @@ export const uploadProfileImage = async ( ); }); - onProgress?.("compress"); - const compressedImage = await compressImage(localUri).catch((error) => { - throw new ProfileImageUploadError("compress", "compressImage failed", { - cause: error, - }); - }); - onProgress?.("upload"); const response = await uploadAsync(upload.url, compressedImage.uri, { mimeType: getMimeType(compressedImage.uri), diff --git a/apps/nextjs/src/app/api/queues/cleanup-upload/route.ts b/apps/nextjs/src/app/api/queues/cleanup-upload/route.ts new file mode 100644 index 00000000..8dce4839 --- /dev/null +++ b/apps/nextjs/src/app/api/queues/cleanup-upload/route.ts @@ -0,0 +1,10 @@ +import type { ICleanupUploadJobData } from "@pegada/api/queue/topics"; + +import { handleCleanupUpload } from "@pegada/api/queue/handlers/upload"; +import { handleCallback } from "@vercel/queue"; + +const handler = handleCallback(async (message: ICleanupUploadJobData) => { + await handleCleanupUpload(message); +}); + +export const POST = (request: Request): Promise => handler(request); diff --git a/packages/api/src/queue/enqueue.ts b/packages/api/src/queue/enqueue.ts index 945b35df..d710238a 100644 --- a/packages/api/src/queue/enqueue.ts +++ b/packages/api/src/queue/enqueue.ts @@ -30,6 +30,8 @@ const INLINE_HANDLERS: { import("./handlers/push").then((m) => m.handleSendPushNotification), [TOPICS.CHECK_PUSH_RECEIPTS]: () => import("./handlers/push").then((m) => m.handleCheckPushReceipts), + [TOPICS.CLEANUP_UPLOAD]: () => + import("./handlers/upload").then((m) => m.handleCleanupUpload), }; const isVercelQueueAvailable = () => diff --git a/packages/api/src/queue/handlers/process-image.ts b/packages/api/src/queue/handlers/process-image.ts index 859aa3b0..039c8244 100644 --- a/packages/api/src/queue/handlers/process-image.ts +++ b/packages/api/src/queue/handlers/process-image.ts @@ -1,5 +1,6 @@ import type { IProcessImageJobData } from "../topics"; +import { MAX_IMAGE_BYTES } from "@pegada/shared/constants/constants"; import { IMAGE_STATUS } from "@pegada/shared/schemas/dog-schema"; import { sendError } from "../../errors/errors"; @@ -7,7 +8,7 @@ import { ImageProcessingService } from "../../services/image-processing-service" import { ImageService } from "../../services/image-service"; import { assertAllowedImageUrl } from "../../shared/image-url"; -export const MAX_IMAGE_BYTES = 10 * 1024 * 1024; +export { MAX_IMAGE_BYTES }; const MAX_REDIRECTS = 5; const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); diff --git a/packages/api/src/queue/handlers/upload.test.ts b/packages/api/src/queue/handlers/upload.test.ts new file mode 100644 index 00000000..4502604c --- /dev/null +++ b/packages/api/src/queue/handlers/upload.test.ts @@ -0,0 +1,48 @@ +import { ImageService } from "../../services/image-service"; +import { enqueue } from "../enqueue"; +import { TOPICS } from "../topics"; +import { handleCleanupUpload } from "./upload"; + +jest.mock("../../services/image-service", () => ({ + ImageService: { + cleanupUploadGrant: jest.fn(async () => undefined), + cleanupExpiredTemporaryUpload: jest.fn(async () => undefined), + pruneUploadGrant: jest.fn(async () => undefined), + }, + UPLOAD_GRANT_WINDOW_SECONDS: 3600, +})); + +jest.mock("../enqueue", () => ({ + enqueue: jest.fn(async () => undefined), +})); + +const cleanupUploadGrant = jest.mocked(ImageService.cleanupUploadGrant); +const cleanupExpiredTemporaryUpload = jest.mocked( + ImageService.cleanupExpiredTemporaryUpload, +); +const pruneUploadGrant = jest.mocked(ImageService.pruneUploadGrant); +const enqueueJob = jest.mocked(enqueue); + +it("deletes stale objects before scheduling grant pruning", async () => { + await handleCleanupUpload({ grantId: "grant-1", phase: "object" }); + + expect(cleanupExpiredTemporaryUpload).toHaveBeenCalledWith("grant-1"); + expect(cleanupUploadGrant).not.toHaveBeenCalled(); + expect(enqueueJob).toHaveBeenCalledWith( + TOPICS.CLEANUP_UPLOAD, + { grantId: "grant-1", phase: "record" }, + { + delaySeconds: 3600, + idempotencyKey: "upload-prune:grant-1", + }, + ); +}); + +it("prunes the grant record after its rate-limit window", async () => { + await handleCleanupUpload({ grantId: "grant-2", phase: "record" }); + + expect(cleanupUploadGrant).toHaveBeenCalledWith("grant-2"); + expect(cleanupExpiredTemporaryUpload).not.toHaveBeenCalled(); + expect(pruneUploadGrant).toHaveBeenCalledWith("grant-2"); + expect(enqueueJob).not.toHaveBeenCalled(); +}); diff --git a/packages/api/src/queue/handlers/upload.ts b/packages/api/src/queue/handlers/upload.ts new file mode 100644 index 00000000..f12db309 --- /dev/null +++ b/packages/api/src/queue/handlers/upload.ts @@ -0,0 +1,29 @@ +import type { ICleanupUploadJobData } from "../topics"; + +import { + ImageService, + UPLOAD_GRANT_WINDOW_SECONDS, +} from "../../services/image-service"; +import { enqueue } from "../enqueue"; +import { TOPICS } from "../topics"; + +export const handleCleanupUpload = async ({ + grantId, + phase, +}: ICleanupUploadJobData) => { + if (phase === "record") { + await ImageService.cleanupUploadGrant(grantId); + await ImageService.pruneUploadGrant(grantId); + return; + } + + await ImageService.cleanupExpiredTemporaryUpload(grantId); + await enqueue( + TOPICS.CLEANUP_UPLOAD, + { grantId, phase: "record" }, + { + delaySeconds: UPLOAD_GRANT_WINDOW_SECONDS, + idempotencyKey: `upload-prune:${grantId}`, + }, + ); +}; diff --git a/packages/api/src/queue/topics.ts b/packages/api/src/queue/topics.ts index 78004a64..fceaeb96 100644 --- a/packages/api/src/queue/topics.ts +++ b/packages/api/src/queue/topics.ts @@ -8,6 +8,7 @@ export const TOPICS = { PROCESS_IMAGE: "process-image", SEND_PUSH: "send-push", CHECK_PUSH_RECEIPTS: "check-push-receipts", + CLEANUP_UPLOAD: "cleanup-upload", } as const; export type Topic = (typeof TOPICS)[keyof typeof TOPICS]; @@ -26,9 +27,15 @@ export type ICheckPushNotificationReceiptsJobData = { receipts?: { id: string; pushToken: string }[]; }; +export type ICleanupUploadJobData = { + grantId: string; + phase: "object" | "record"; +}; + export type TopicPayloads = { [TOPICS.MAIL]: IMailJobData; [TOPICS.PROCESS_IMAGE]: IProcessImageJobData; [TOPICS.SEND_PUSH]: ISendNotificationJobData; [TOPICS.CHECK_PUSH_RECEIPTS]: ICheckPushNotificationReceiptsJobData; + [TOPICS.CLEANUP_UPLOAD]: ICleanupUploadJobData; }; diff --git a/packages/api/src/routes/image.ts b/packages/api/src/routes/image.ts index 2d6898bc..19a9ef97 100644 --- a/packages/api/src/routes/image.ts +++ b/packages/api/src/routes/image.ts @@ -1,5 +1,6 @@ import { ImageService } from "../services/image-service"; import { createTRPCRouter, protectedProcedure } from "../trpc"; +import { signedUploadInputSchema } from "./input-schemas"; export const imageRouter = createTRPCRouter({ /** @@ -8,13 +9,15 @@ export const imageRouter = createTRPCRouter({ * backing storage (S3) are frozen until those binaries are sunset via * MIN_APP_VERSION. New code uses `signedUpload`. */ - signedUrl: protectedProcedure.query(async () => { - const presignedUrl = await ImageService.getSignedUrl(); + signedUrl: protectedProcedure.query(async ({ ctx }) => { + const presignedUrl = await ImageService.getSignedUrl(ctx.session.user.id); return presignedUrl.url; }), /** Storage-agnostic upload descriptor — see `SignedUpload` in ImageService. */ - signedUpload: protectedProcedure.query(() => { - return ImageService.getSignedUpload(); - }), + signedUpload: protectedProcedure + .input(signedUploadInputSchema.optional()) + .query(({ ctx, input }) => + ImageService.getSignedUpload(ctx.session.user.id, input), + ), }); diff --git a/packages/api/src/routes/input-schemas.ts b/packages/api/src/routes/input-schemas.ts index 6d1cf0b4..599564b6 100644 --- a/packages/api/src/routes/input-schemas.ts +++ b/packages/api/src/routes/input-schemas.ts @@ -1,5 +1,11 @@ +import { MAX_IMAGE_BYTES } from "@pegada/shared/constants/constants"; import { z } from "zod"; +export const signedUploadInputSchema = z.object({ + contentLength: z.number().int().min(1).max(MAX_IMAGE_BYTES), + contentType: z.literal("image/webp"), +}); + export const messageListInputSchema = z.object({ matchId: z.string().uuid(), limit: z.coerce.number().int().min(1).max(100).optional().default(10), diff --git a/packages/api/src/routes/input-validation.test.ts b/packages/api/src/routes/input-validation.test.ts index 34c81baa..cefa2c82 100644 --- a/packages/api/src/routes/input-validation.test.ts +++ b/packages/api/src/routes/input-validation.test.ts @@ -1,6 +1,9 @@ +import { MAX_IMAGE_BYTES } from "@pegada/shared/constants/constants"; + import { messageListInputSchema, messageSendInputSchema, + signedUploadInputSchema, swipeQueryInputSchema, } from "./input-schemas"; @@ -57,3 +60,26 @@ describe("swipe input limits", () => { ).toBe(false); }); }); + +describe("upload input limits", () => { + it("accepts only bounded WEBP uploads", () => { + expect( + signedUploadInputSchema.safeParse({ + contentLength: MAX_IMAGE_BYTES, + contentType: "image/webp", + }).success, + ).toBe(true); + expect( + signedUploadInputSchema.safeParse({ + contentLength: MAX_IMAGE_BYTES + 1, + contentType: "image/webp", + }).success, + ).toBe(false); + expect( + signedUploadInputSchema.safeParse({ + contentLength: 1024, + contentType: "text/html", + }).success, + ).toBe(false); + }); +}); diff --git a/packages/api/src/services/dog-service.test.ts b/packages/api/src/services/dog-service.test.ts index 3a4b323b..c05a7fb2 100644 --- a/packages/api/src/services/dog-service.test.ts +++ b/packages/api/src/services/dog-service.test.ts @@ -1,6 +1,8 @@ import prisma from "@pegada/database"; import { Gender } from "@prisma/client"; +import { config } from "../shared/config"; +import { deleteImageFromS3 } from "../shared/file-upload"; import { DogService } from "./dog-service"; // `enqueue` pulls in `errors.ts` -> `observability.ts` -> the ESM-only @@ -12,11 +14,25 @@ jest.mock("../errors/errors", () => ({ errorDebug: () => undefined, })); +jest.mock("../shared/file-upload", () => { + const actual = jest.requireActual( + "../shared/file-upload", + ); + + return { + ...actual, + deleteImageFromS3: jest.fn(async () => undefined), + }; +}); + +const deleteStoredImage = jest.mocked(deleteImageFromS3); + afterAll(async () => { await prisma.$disconnect(); }); beforeEach(async () => { + await prisma.uploadGrant.deleteMany(); await prisma.message.deleteMany(); await prisma.match.deleteMany(); await prisma.interest.deleteMany(); @@ -66,3 +82,20 @@ describe("DogService.updateDog", () => { ).resolves.toMatchObject({ position: 0 }); }); }); + +describe("DogService.deleteDog", () => { + it("deletes the public object before removing its database row", async () => { + const storedUrl = `https://${config.AWS_S3_BUCKET_NAME}.s3.${config.AWS_REGION}.amazonaws.com/dogs/delete.webp`; + const dog = await seedDog("delete-image-owner@pegada.app", storedUrl); + + await DogService.deleteDog(dog.id); + + expect(deleteStoredImage).toHaveBeenCalledWith(storedUrl); + await expect( + prisma.image.count({ where: { dogId: dog.id } }), + ).resolves.toBe(0); + await expect( + prisma.dog.findUniqueOrThrow({ where: { id: dog.id } }), + ).resolves.toMatchObject({ deletedAt: expect.any(Date) }); + }); +}); diff --git a/packages/api/src/services/dog-service.ts b/packages/api/src/services/dog-service.ts index fa05c0eb..529d2ee3 100644 --- a/packages/api/src/services/dog-service.ts +++ b/packages/api/src/services/dog-service.ts @@ -78,8 +78,10 @@ export class DogService { } const nonEmptyImages = dogInput.images.filter((image) => image.url); - const images = - await ImageService.makeTemporaryImagesPermanent(nonEmptyImages); + const images = await ImageService.makeTemporaryImagesPermanent( + nonEmptyImages, + dogInput.userId, + ); const dog = await prisma.dog.create({ data: { @@ -103,6 +105,10 @@ export class DogService { } static async updateDog(id: string, dogInput: Partial) { + const dog = await prisma.dog.findUniqueOrThrow({ + where: { id }, + select: { userId: true }, + }); const existingImages = dogInput.images ? await prisma.image.findMany({ where: { dogId: id } }) : []; @@ -111,7 +117,10 @@ export class DogService { this.#classifyImages(existingImages, dogInput.images ?? []); const imagesToCreatePermanent = - await ImageService.makeTemporaryImagesPermanent(imagesToCreate); + await ImageService.makeTemporaryImagesPermanent( + imagesToCreate, + dog.userId, + ); const dogTransaction = await prisma.$transaction([ ...imagesToUpdate.map((image) => @@ -243,6 +252,17 @@ export class DogService { } static async deleteDog(id: string) { + const imageUrls = await prisma.image.findMany({ + where: { dogId: id }, + select: { url: true }, + }); + + await Promise.all( + imageUrls + .filter(({ url }) => isAllowedImageUrl(url)) + .map(({ url }) => deleteImageFromS3(url)), + ); + // Cascade soft-delete await prisma.$transaction([ prisma.dog.update({ diff --git a/packages/api/src/services/image-service.test.ts b/packages/api/src/services/image-service.test.ts index de06f8ca..3fbac371 100644 --- a/packages/api/src/services/image-service.test.ts +++ b/packages/api/src/services/image-service.test.ts @@ -1,4 +1,55 @@ -import { createTemporaryUploadKey } from "./image-service"; +import prisma from "@pegada/database"; +import { + InvalidUploadGrantError, + UploadLimitReachedError, +} from "@pegada/shared/errors/errors"; + +import { deleteImageFromS3, moveImageToFolder } from "../shared/file-upload"; +import { + createTemporaryUploadKey, + ImageService, + UPLOAD_GRANT_LIMIT, + UPLOAD_URL_TTL_SECONDS, +} from "./image-service"; + +jest.mock("../queue/enqueue", () => ({ + enqueue: jest.fn(async () => undefined), +})); + +jest.mock("../shared/file-upload", () => { + const actual = jest.requireActual( + "../shared/file-upload", + ); + + return { + ...actual, + deleteImageFromS3: jest.fn(async () => undefined), + moveImageToFolder: jest.fn(async (url: string, folder: string) => + actual.getMovedImageUrl(url, folder), + ), + }; +}); + +const deleteStoredImage = jest.mocked(deleteImageFromS3); +const moveStoredImage = jest.mocked(moveImageToFolder); + +beforeEach(async () => { + await prisma.uploadGrant.deleteMany(); +}); + +afterEach(async () => { + const grants = await prisma.uploadGrant.findMany({ select: { id: true } }); + await Promise.all( + grants.map(async ({ id }) => { + await ImageService.cleanupUploadGrant(id); + await ImageService.pruneUploadGrant(id); + }), + ); +}); + +afterAll(async () => { + await prisma.$disconnect(); +}); describe("createTemporaryUploadKey", () => { it("creates distinct UUID-backed keys", () => { @@ -12,3 +63,128 @@ describe("createTemporaryUploadKey", () => { } }); }); + +describe("upload grants", () => { + it("signs the exact WEBP size and expires the URL after ten minutes", async () => { + const upload = await ImageService.getSignedUpload("user-one", { + contentLength: 4, + contentType: "image/webp", + }); + const signedUrl = new URL(upload.url); + + expect(upload.headers).toEqual({ + "Content-Length": "4", + "Content-Type": "image/webp", + }); + expect(signedUrl.searchParams.get("X-Amz-Expires")).toBe( + String(UPLOAD_URL_TTL_SECONDS), + ); + expect(signedUrl.searchParams.get("X-Amz-SignedHeaders")).toContain( + "content-length", + ); + await expect( + prisma.uploadGrant.findUnique({ + where: { temporaryUrl: upload.publicUrl }, + }), + ).resolves.toMatchObject({ userId: "user-one", consumedAt: null }); + }); + + it("serializes concurrent grant requests at the hourly limit", async () => { + const results = await Promise.allSettled( + Array.from({ length: UPLOAD_GRANT_LIMIT + 2 }, () => + ImageService.getSignedUpload("limited-user"), + ), + ); + + expect(results.filter(({ status }) => status === "fulfilled")).toHaveLength( + UPLOAD_GRANT_LIMIT, + ); + expect(results.filter(({ status }) => status === "rejected")).toEqual([ + expect.objectContaining({ + reason: expect.any(UploadLimitReachedError), + }), + expect.objectContaining({ + reason: expect.any(UploadLimitReachedError), + }), + ]); + }); + + it("lets only the grant owner promote an uploaded object", async () => { + const upload = await ImageService.getSignedUpload("owner", { + contentLength: 4, + contentType: "image/webp", + }); + await expect( + ImageService.makeTemporaryImagesPermanent( + [{ url: upload.publicUrl, position: 0 }], + "other-user", + ), + ).rejects.toBeInstanceOf(InvalidUploadGrantError); + + const [image] = await ImageService.makeTemporaryImagesPermanent( + [{ url: upload.publicUrl, position: 0 }], + "owner", + ); + expect(image?.url).toContain("/dogs/"); + expect(moveStoredImage).toHaveBeenCalledWith(upload.publicUrl, "dogs"); + }); + + it("deletes unused uploads and their grant record", async () => { + const upload = await ImageService.getSignedUpload("cleanup-user", { + contentLength: 4, + contentType: "image/webp", + }); + const grant = await prisma.uploadGrant.findUniqueOrThrow({ + where: { temporaryUrl: upload.publicUrl }, + }); + + await ImageService.cleanupUploadGrant(grant.id); + expect(deleteStoredImage).toHaveBeenCalledWith(upload.publicUrl); + await expect( + prisma.uploadGrant.findUniqueOrThrow({ where: { id: grant.id } }), + ).resolves.toMatchObject({ cleanedAt: expect.any(Date) }); + + await ImageService.pruneUploadGrant(grant.id); + await expect( + prisma.uploadGrant.findUnique({ where: { id: grant.id } }), + ).resolves.toBeNull(); + }); + + it("leaves active and consumed grants for the later orphan cleanup", async () => { + const activeUpload = await ImageService.getSignedUpload("active-cleanup", { + contentLength: 4, + contentType: "image/webp", + }); + const consumedUpload = await ImageService.getSignedUpload( + "consumed-cleanup", + { + contentLength: 4, + contentType: "image/webp", + }, + ); + const [activeGrant, consumedGrant] = await Promise.all([ + prisma.uploadGrant.findUniqueOrThrow({ + where: { temporaryUrl: activeUpload.publicUrl }, + }), + prisma.uploadGrant.update({ + where: { temporaryUrl: consumedUpload.publicUrl }, + data: { consumedAt: new Date(), expiresAt: new Date(0) }, + }), + ]); + + await ImageService.cleanupExpiredTemporaryUpload(activeGrant.id); + await ImageService.cleanupExpiredTemporaryUpload(consumedGrant.id); + + expect(deleteStoredImage).not.toHaveBeenCalledWith(activeUpload.publicUrl); + expect(deleteStoredImage).not.toHaveBeenCalledWith( + consumedUpload.publicUrl, + ); + + await prisma.uploadGrant.update({ + where: { id: activeGrant.id }, + data: { expiresAt: new Date(0) }, + }); + await ImageService.cleanupExpiredTemporaryUpload(activeGrant.id); + expect(deleteStoredImage).toHaveBeenCalledWith(activeUpload.publicUrl); + }); +}); diff --git a/packages/api/src/services/image-service.ts b/packages/api/src/services/image-service.ts index e0763322..ad0342be 100644 --- a/packages/api/src/services/image-service.ts +++ b/packages/api/src/services/image-service.ts @@ -6,10 +6,20 @@ import { randomUUID } from "node:crypto"; import { PutObjectCommand } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; import prisma from "@pegada/database"; +import { + InvalidUploadGrantError, + UploadLimitReachedError, +} from "@pegada/shared/errors/errors"; +import { addSeconds } from "date-fns/addSeconds"; +import { subHours } from "date-fns/subHours"; +import { enqueue } from "../queue/enqueue"; +import { TOPICS } from "../queue/topics"; import { config } from "../shared/config"; import { client, + deleteImageFromS3, + getMovedImageUrl, getPublicUrl, moveImageToFolder, r2Client, @@ -18,6 +28,15 @@ import { } from "../shared/file-upload"; const PERMANENT_STORAGE_FOLDER = "dogs"; +export const UPLOAD_GRANT_LIMIT = 12; +export const UPLOAD_GRANT_WINDOW_SECONDS = 60 * 60; +export const UPLOAD_URL_TTL_SECONDS = 10 * 60; +export const UPLOAD_CLEANUP_DELAY_SECONDS = UPLOAD_URL_TTL_SECONDS + 60; + +export type SignedUploadInput = { + contentLength?: number; + contentType?: "image/webp"; +}; export const createTemporaryUploadKey = () => `${TEMPORARY_UPLOAD_PREFIX}/${randomUUID()}`; @@ -40,13 +59,57 @@ export type SignedUpload = { }; export class ImageService { + static async #recordUploadGrant(userId: string, temporaryUrl: string) { + const grant = await prisma.$transaction(async (tx) => { + await tx.$queryRaw` + SELECT pg_advisory_xact_lock( + hashtextextended(${`upload-grant:${userId}`}, 0) + )::text + `; + + const recentGrants = await tx.uploadGrant.count({ + where: { + userId, + createdAt: { gte: subHours(new Date(), 1) }, + }, + }); + + if (recentGrants >= UPLOAD_GRANT_LIMIT) { + throw new UploadLimitReachedError(); + } + + return tx.uploadGrant.create({ + data: { + userId, + temporaryUrl, + expiresAt: addSeconds(new Date(), UPLOAD_URL_TTL_SECONDS), + }, + select: { id: true }, + }); + }); + + try { + await enqueue( + TOPICS.CLEANUP_UPLOAD, + { grantId: grant.id, phase: "object" }, + { + delaySeconds: UPLOAD_CLEANUP_DELAY_SECONDS, + idempotencyKey: `upload-cleanup:${grant.id}`, + }, + ); + } catch (error) { + await prisma.uploadGrant.deleteMany({ where: { id: grant.id } }); + throw error; + } + } + /** * LEGACY path — shipped app binaries call this and derive the public URL * by stripping the presigned URL's query string, so the response shape * AND the presigned host (S3) must stay exactly as they are. Remove once * MIN_APP_VERSION is past the release that switched to `signedUpload`. */ - static async getSignedUrl() { + static async getSignedUrl(userId: string) { const key = createTemporaryUploadKey(); const command = new PutObjectCommand({ @@ -56,9 +119,11 @@ export class ImageService { }); const url = await getSignedUrl(client, command, { - expiresIn: 60 * 60, + expiresIn: UPLOAD_URL_TTL_SECONDS, }); + await this.#recordUploadGrant(userId, url.split("?")[0] as string); + return { url }; } @@ -68,13 +133,25 @@ export class ImageService { * legacy S3/MinIO otherwise (dev/e2e), and any future storage backend * just builds its own descriptor here. */ - static async getSignedUpload(): Promise { + static async getSignedUpload( + userId: string, + input: SignedUploadInput = {}, + ): Promise { const key = createTemporaryUploadKey(); + const contentType = input.contentType ?? "image/webp"; + const headers = { + "Content-Type": contentType, + ...(input.contentLength + ? { "Content-Length": String(input.contentLength) } + : {}), + }; if (r2UploadsEnabled) { const command = new PutObjectCommand({ Bucket: config.R2_BUCKET_NAME, Key: key, + ContentType: contentType, + ...(input.contentLength ? { ContentLength: input.contentLength } : {}), // No ACL: R2 has no ACL concept — public access is via the // bucket's custom domain (PUBLIC_IMAGES_BASE_URL). }); @@ -83,10 +160,13 @@ export class ImageService { // Non-null: guaranteed by the r2UploadsEnabled gate. r2Client as NonNullable, command, - { expiresIn: 60 * 60 }, + { expiresIn: UPLOAD_URL_TTL_SECONDS }, ); - return { method: "PUT", url, headers: {}, publicUrl: getPublicUrl(key) }; + const publicUrl = getPublicUrl(key); + await this.#recordUploadGrant(userId, publicUrl); + + return { method: "PUT", url, headers, publicUrl }; } // R2 not configured (dev/e2e): presign against the legacy S3/MinIO @@ -96,17 +176,21 @@ export class ImageService { Bucket: config.AWS_S3_BUCKET_NAME, Key: key, ACL: "public-read", + ContentType: contentType, + ...(input.contentLength ? { ContentLength: input.contentLength } : {}), }); const url = await getSignedUrl(client, command, { - expiresIn: 60 * 60, + expiresIn: UPLOAD_URL_TTL_SECONDS, }); + const publicUrl = url.split("?")[0] as string; + await this.#recordUploadGrant(userId, publicUrl); return { method: "PUT", url, - headers: {}, - publicUrl: url.split("?")[0] as string, + headers, + publicUrl, }; } @@ -126,12 +210,83 @@ export class ImageService { /** * Move new images to another s3 folder that won't expire. */ - static makeTemporaryImagesPermanent = (images: DogServerSchema["images"]) => { + static makeTemporaryImagesPermanent = ( + images: DogServerSchema["images"], + userId: string, + ) => { const permanentImages = images.map(async (image) => { + const permanentUrl = getMovedImageUrl( + image.url, + PERMANENT_STORAGE_FOLDER, + ); + const claim = await prisma.uploadGrant.updateMany({ + where: { + userId, + temporaryUrl: image.url, + consumedAt: null, + expiresAt: { gt: new Date() }, + }, + data: { + consumedAt: new Date(), + permanentUrl, + }, + }); + + if (claim.count !== 1) throw new InvalidUploadGrantError(); + const url = await moveImageToFolder(image.url, PERMANENT_STORAGE_FOLDER); + if (url !== permanentUrl) throw new InvalidUploadGrantError(); return { ...image, url }; }); return Promise.all(permanentImages); }; + + static async cleanupUploadGrant(grantId: string) { + const grant = await prisma.uploadGrant.findUnique({ + where: { id: grantId }, + }); + if (!grant) return; + + const permanentImageInUse = grant.permanentUrl + ? await prisma.image.count({ where: { url: grant.permanentUrl } }) + : 0; + const urls = new Set([ + grant.temporaryUrl, + ...(grant.permanentUrl && permanentImageInUse === 0 + ? [grant.permanentUrl] + : []), + ]); + + await Promise.all([...urls].map((url) => deleteImageFromS3(url))); + await prisma.uploadGrant.update({ + where: { id: grant.id }, + data: { cleanedAt: new Date() }, + }); + } + + static async cleanupExpiredTemporaryUpload(grantId: string) { + const grant = await prisma.uploadGrant.findFirst({ + where: { + id: grantId, + consumedAt: null, + expiresAt: { lte: new Date() }, + }, + }); + if (!grant) return; + + await deleteImageFromS3(grant.temporaryUrl); + await prisma.uploadGrant.updateMany({ + where: { + id: grant.id, + consumedAt: null, + expiresAt: { lte: new Date() }, + }, + data: { cleanedAt: new Date() }, + }); + } + + static pruneUploadGrant(grantId: string) { + return prisma.uploadGrant.deleteMany({ where: { id: grantId } }); + } } diff --git a/packages/api/src/services/user-service.test.ts b/packages/api/src/services/user-service.test.ts index 08348d6f..aafd2b7e 100644 --- a/packages/api/src/services/user-service.test.ts +++ b/packages/api/src/services/user-service.test.ts @@ -33,8 +33,7 @@ jest.mock("../shared/file-upload", () => { const deleteStoredImage = jest.mocked(deleteImageFromS3); -/** .env.test leaves R2 and AWS_S3_ENDPOINT unset, so this is the one origin - * this service is configured to operate on. */ +/** The legacy virtual-hosted S3 origin remains allowed in the test config. */ const BUCKET_URL = `https://${config.AWS_S3_BUCKET_NAME}.s3.${config.AWS_REGION}.amazonaws.com`; const STORED_PHOTO = `${BUCKET_URL}/dogs/luna.webp`; @@ -43,6 +42,7 @@ afterAll(async () => { }); beforeEach(async () => { + await prisma.uploadGrant.deleteMany(); await prisma.message.deleteMany(); await prisma.match.deleteMany(); await prisma.interest.deleteMany(); @@ -76,6 +76,29 @@ test("removes stored photos before deleting an account", async () => { ).resolves.toBeNull(); }); +test("removes outstanding upload objects and grants with the account", async () => { + const user = await seedAccount(); + const temporaryUrl = `${BUCKET_URL}/dogs-temporary/pending.webp`; + const permanentUrl = `${BUCKET_URL}/dogs/orphaned.webp`; + await prisma.uploadGrant.create({ + data: { + userId: user.id, + temporaryUrl, + permanentUrl, + expiresAt: new Date(Date.now() + 60_000), + consumedAt: new Date(), + }, + }); + + await UserService.deleteAccount(user.id); + + expect(deleteStoredImage).toHaveBeenCalledWith(temporaryUrl); + expect(deleteStoredImage).toHaveBeenCalledWith(permanentUrl); + await expect( + prisma.uploadGrant.count({ where: { userId: user.id } }), + ).resolves.toBe(0); +}); + test("keeps the account when a stored photo cannot be removed", async () => { const user = await seedAccount(); deleteStoredImage.mockRejectedValueOnce(new Error("storage unavailable")); diff --git a/packages/api/src/services/user-service.ts b/packages/api/src/services/user-service.ts index 5f5e3ff5..f7891152 100644 --- a/packages/api/src/services/user-service.ts +++ b/packages/api/src/services/user-service.ts @@ -17,19 +17,29 @@ export class UserService { * A storage failure leaves the account row in place so deletion can be * retried. * - * Order: messages → matches → interests → images → dogs → user. + * Order: messages → matches → interests → images → dogs → upload grants → user. * Messages and matches reference dogs; interests reference dogs and * matches; images reference dogs; dogs reference the user. */ static async deleteAccount(userId: string) { - const dogs = await prisma.dog.findMany({ - where: { userId }, - select: { id: true, images: { select: { url: true } } }, - }); + const [dogs, uploadGrants] = await Promise.all([ + prisma.dog.findMany({ + where: { userId }, + select: { id: true, images: { select: { url: true } } }, + }), + prisma.uploadGrant.findMany({ + where: { userId }, + select: { temporaryUrl: true, permanentUrl: true }, + }), + ]); const dogIds = dogs.map((d) => d.id); - const imageUrls = dogs.flatMap((dog) => - dog.images.map((image) => image.url), - ); + const imageUrls = new Set([ + ...dogs.flatMap((dog) => dog.images.map((image) => image.url)), + ...uploadGrants.flatMap(({ temporaryUrl, permanentUrl }) => [ + temporaryUrl, + ...(permanentUrl ? [permanentUrl] : []), + ]), + ]); // Delete public objects before their database references disappear. A // failed storage deletion leaves the account intact so the request can be @@ -45,7 +55,7 @@ export class UserService { await Promise.all( // Not point-free: `isAllowedImageUrl`'s second parameter is the origin // set, and `filter` would hand it the array index. - imageUrls + [...imageUrls] .filter((url) => isAllowedImageUrl(url)) .map((url) => deleteImageFromS3(url)), ); @@ -77,6 +87,7 @@ export class UserService { await tx.dog.deleteMany({ where: { id: { in: dogIds } } }); } + await tx.uploadGrant.deleteMany({ where: { userId } }); await tx.user.delete({ where: { id: userId } }); }); } diff --git a/packages/api/src/shared/file-upload.ts b/packages/api/src/shared/file-upload.ts index 4b723017..1750d124 100644 --- a/packages/api/src/shared/file-upload.ts +++ b/packages/api/src/shared/file-upload.ts @@ -203,20 +203,39 @@ export const deleteImageFromS3 = async (url: string) => { await storageClient.send(command); }; +const planImageMove = (url: string, folder: string) => { + const storage = storageForUrl(url); + const oldKey = keyFromUrl(url, storage.bucket); + + assertTemporaryUploadKey(oldKey); + + const fileName = oldKey.split("/").at(-1) as string; + const newKey = `${folder}/${fileName}`; + + return { + ...storage, + oldKey, + newKey, + publicUrl: storage.urlForKey(newKey), + }; +}; + +export const getMovedImageUrl = (url: string, folder: string) => + planImageMove(url, folder).publicUrl; + // Move image to another folder in S3, receives the url of the image and the new folder name and returns the new url export const moveImageToFolder = async (url: string, folder: string) => { // Routed by URL host: images uploaded through the legacy path live on // S3/MinIO, images from the new path live on R2. Copies never cross // providers — temp and permanent folders are in the same bucket. - const { client: storageClient, bucket, isR2, urlForKey } = storageForUrl(url); - - const oldKey = keyFromUrl(url, bucket); - - // Ahead of the copy and the delete below, both of which act on this key. - assertTemporaryUploadKey(oldKey); - - const fileName = oldKey.split("/").at(-1); - const newKey = `${folder}/${fileName}`; + const { + client: storageClient, + bucket, + isR2, + oldKey, + newKey, + publicUrl, + } = planImageMove(url, folder); const command = new CopyObjectCommand({ Bucket: bucket, @@ -238,5 +257,5 @@ export const moveImageToFolder = async (url: string, folder: string) => { // Rebuilt from the storage's own base URL rather than patched out of the // caller's string, so nothing about the incoming URL other than the file // name reaches the database. - return urlForKey(newKey); + return publicUrl; }; diff --git a/packages/database/migrations/20260831150000_add_upload_grants/migration.sql b/packages/database/migrations/20260831150000_add_upload_grants/migration.sql new file mode 100644 index 00000000..09c3587f --- /dev/null +++ b/packages/database/migrations/20260831150000_add_upload_grants/migration.sql @@ -0,0 +1,22 @@ +CREATE TABLE "UploadGrant" ( + "id" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "temporaryUrl" TEXT NOT NULL, + "permanentUrl" TEXT, + "expiresAt" TIMESTAMP(3) NOT NULL, + "consumedAt" TIMESTAMP(3), + "cleanedAt" TIMESTAMP(3), + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "UploadGrant_pkey" PRIMARY KEY ("id") +); + +CREATE UNIQUE INDEX "UploadGrant_temporaryUrl_key" +ON "UploadGrant"("temporaryUrl"); + +CREATE INDEX "UploadGrant_userId_createdAt_idx" +ON "UploadGrant"("userId", "createdAt"); + +CREATE INDEX "UploadGrant_expiresAt_consumedAt_idx" +ON "UploadGrant"("expiresAt", "consumedAt"); diff --git a/packages/database/schema.prisma b/packages/database/schema.prisma index 8cfdf7d6..0a4c23ca 100644 --- a/packages/database/schema.prisma +++ b/packages/database/schema.prisma @@ -167,6 +167,23 @@ model Image { @@index([dogId, status]) } +model UploadGrant { + id String @id @default(cuid()) + + userId String + temporaryUrl String @unique + permanentUrl String? + + expiresAt DateTime + consumedAt DateTime? + cleanedAt DateTime? + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + @@index([userId, createdAt]) + @@index([expiresAt, consumedAt]) +} + model Breed { id String @id(map: "idx_57659_PRIMARY") @default(uuid()) name String @unique(map: "breed_name_key") diff --git a/packages/shared/constants/constants.ts b/packages/shared/constants/constants.ts index 823a61d9..2789f841 100644 --- a/packages/shared/constants/constants.ts +++ b/packages/shared/constants/constants.ts @@ -1 +1,2 @@ export const FREE_DAILY_SWIPE_LIMIT = 10; +export const MAX_IMAGE_BYTES = 10 * 1024 * 1024; diff --git a/packages/shared/errors/errors.ts b/packages/shared/errors/errors.ts index 09112885..43652ad3 100644 --- a/packages/shared/errors/errors.ts +++ b/packages/shared/errors/errors.ts @@ -90,3 +90,33 @@ export class DogUnavailableError extends IntentionalError { this.name = "DogUnavailableError"; } } + +export class UploadLimitReachedError extends IntentionalError { + static message = "Too many photo uploads. Try again later."; + static error_code = "UPLOAD_LIMIT_REACHED"; + error_code = UploadLimitReachedError.error_code; + + constructor() { + super({ + code: "TOO_MANY_REQUESTS", + message: UploadLimitReachedError.message, + }); + + this.name = "UploadLimitReachedError"; + } +} + +export class InvalidUploadGrantError extends IntentionalError { + static message = "This photo upload has expired. Try uploading it again."; + static error_code = "INVALID_UPLOAD_GRANT"; + error_code = InvalidUploadGrantError.error_code; + + constructor() { + super({ + code: "BAD_REQUEST", + message: InvalidUploadGrantError.message, + }); + + this.name = "InvalidUploadGrantError"; + } +} diff --git a/vercel.json b/vercel.json index d8a9b284..4f31a37b 100644 --- a/vercel.json +++ b/vercel.json @@ -1,6 +1,15 @@ { "$schema": "https://openapi.vercel.sh/vercel.json", "functions": { + "src/app/api/queues/cleanup-upload/route.ts": { + "experimentalTriggers": [ + { + "type": "queue/v2beta", + "topic": "cleanup-upload", + "retryAfterSeconds": 60 + } + ] + }, "src/app/api/queues/mail/route.ts": { "experimentalTriggers": [ {