From 71d8127f8b21bf4db2bf70773edaddfe322a34e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Tue, 28 Jan 2025 11:27:23 +0000 Subject: [PATCH] fix: Add file extension check to filename parameter in body (#4219) --- api.planx.uk/modules/file/controller.ts | 7 +++- api.planx.uk/modules/file/file.test.ts | 35 +++++++++++++++++-- .../modules/file/middleware/useFileUpload.ts | 2 +- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/api.planx.uk/modules/file/controller.ts b/api.planx.uk/modules/file/controller.ts index 480c8b5c67..918ca38d4d 100644 --- a/api.planx.uk/modules/file/controller.ts +++ b/api.planx.uk/modules/file/controller.ts @@ -5,6 +5,7 @@ import { getFileFromS3 } from "./service/getFile.js"; import { z } from "zod"; import type { ValidatedRequestHandler } from "../../shared/middleware/validate.js"; import { ServerError } from "../../errors/index.js"; +import { validateExtension } from "./middleware/useFileUpload.js"; assert(process.env.AWS_S3_BUCKET); assert(process.env.AWS_S3_REGION); @@ -18,7 +19,11 @@ interface UploadFileResponse { export const uploadFileSchema = z.object({ body: z.object({ - filename: z.string().trim().min(1), + filename: z + .string() + .trim() + .min(1) + .refine(validateExtension, { message: "Unsupported file type" }), }), }); diff --git a/api.planx.uk/modules/file/file.test.ts b/api.planx.uk/modules/file/file.test.ts index f19431d160..4fdedacbb6 100644 --- a/api.planx.uk/modules/file/file.test.ts +++ b/api.planx.uk/modules/file/file.test.ts @@ -128,7 +128,7 @@ describe("File upload", () => { it("should not upload without file", async () => { await supertest(app) .post(PRIVATE_ENDPOINT) - .field("filename", "some filename") + .field("filename", "some_filename.png") .expect(500) .then((res) => { expect(mockPutObject).not.toHaveBeenCalled(); @@ -136,6 +136,21 @@ describe("File upload", () => { }); }); + it("should not upload a file with an invalid extension in the filename parameter (no attachment)", async () => { + await supertest(app) + .post(PRIVATE_ENDPOINT) + .field("filename", "my_file.exe") // filename does not match multer.file.filename + .attach("file", Buffer.from("some data"), { + filename: "my_file.jpg", + contentType: "image/jpg", + }) + .expect(500) + .then((res) => { + expect(mockPutObject).not.toHaveBeenCalled(); + expect(res.body.error).toMatch(/Unsupported file type/); + }); + }); + it("should upload file", async () => { vi.stubEnv("API_URL_EXT", "https://api.editor.planx.dev"); vi.stubEnv("AWS_S3_BUCKET", "myBucketName"); @@ -222,7 +237,7 @@ describe("File upload", () => { await supertest(app) .post(PUBLIC_ENDPOINT) .set(auth) - .field("filename", "some filename") + .field("filename", "some_filename.jpg") .expect(500) .then((res) => { expect(mockPutObject).not.toHaveBeenCalled(); @@ -230,6 +245,22 @@ describe("File upload", () => { }); }); + it("should not upload a file with an invalid extension in the filename parameter (no attachment)", async () => { + await supertest(app) + .post(PUBLIC_ENDPOINT) + .set(auth) + .field("filename", "my_file.exe") // filename does not match multer.file.filename + .attach("file", Buffer.from("some data"), { + filename: "my_file.jpg", + contentType: "image/jpg", + }) + .expect(500) + .then((res) => { + expect(mockPutObject).not.toHaveBeenCalled(); + expect(res.body.error).toMatch(/Unsupported file type/); + }); + }); + it("should upload file", async () => { await supertest(app) .post(PUBLIC_ENDPOINT) diff --git a/api.planx.uk/modules/file/middleware/useFileUpload.ts b/api.planx.uk/modules/file/middleware/useFileUpload.ts index 8c9ef84287..5088d48a8a 100644 --- a/api.planx.uk/modules/file/middleware/useFileUpload.ts +++ b/api.planx.uk/modules/file/middleware/useFileUpload.ts @@ -14,7 +14,7 @@ const FILE_SIZE_LIMIT = 30 * 1024 * 1024; const ALLOWED_MIME_TYPES = ["image/jpeg", "image/png", "application/pdf"]; const ALLOWED_EXTENSIONS = [".jpg", ".jpeg", ".png", ".pdf"]; -const validateExtension = (filename: string): boolean => { +export const validateExtension = (filename: string): boolean => { const extension = path.extname(filename).toLowerCase(); return ALLOWED_EXTENSIONS.includes(extension); };