fix: Add file extension check to filename parameter in body (#4219)

theopensystemslab · Jan 28, 2025 · 71d8127 · 71d8127
1 parent 2c32832
commit 71d8127
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 4 deletions.
diff --git 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
@@ -128,14 +128,29 @@ 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();
           expect(res.body.error).toMatch(/Missing file/);
         });
     });
 
+    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,14 +237,30 @@ 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();
           expect(res.body.error).toMatch(/Missing file/);
         });
     });
 
+    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
@@ -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);
 };