Skip to content

Commit

Permalink
fix: Improve typechecking for file upload
Browse files Browse the repository at this point in the history
  • Loading branch information
DafyddLlyr committed Dec 7, 2024
1 parent cb83df3 commit 96b5b8e
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
24 changes: 21 additions & 3 deletions api.planx.uk/modules/file/service/uploadFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import mime from "mime";
import { customAlphabet } from "nanoid";
import { isLiveEnv } from "../../../helpers.js";
import { s3Factory } from "./utils.js";
import { Readable } from "stream";
const nanoid = customAlphabet("1234567890abcdefghijklmnopqrstuvwxyz", 8);

export const uploadPublicFile = async (
Expand All @@ -28,10 +29,14 @@ export const uploadPublicFile = async (
};

export const uploadPrivateFile = async (
file: Express.Multer.File,
file: Express.Multer.File | Buffer,
filename: string,
filekey?: string,
) => {
if (file instanceof Buffer) {
file = convertToMulterFile(file);
}

const s3 = s3Factory();

const { params, key, fileType } = generateFileParams(file, filename, filekey);
Expand Down Expand Up @@ -63,6 +68,19 @@ const buildFileUrl = async (key: string, path: "public" | "private") => {
return `${process.env.API_URL_EXT}/file/${path}${s3Pathname}`;
};

const convertToMulterFile = (buffer: Buffer): Express.Multer.File => ({
buffer: buffer,
originalname: "${data.id}.json",
mimetype: "application/json",
size: buffer.length,

Check failure

Code scanning / CodeQL

Type confusion through parameter tampering Critical

Potential type confusion as
this HTTP request parameter
may be either an array or a string.
fieldname: "file",
encoding: "7bit",
stream: Readable.from(buffer),
destination: "",
filename: "",
path: "",
});

export function generateFileParams(
file: Express.Multer.File,
filename: string,
Expand All @@ -79,9 +97,9 @@ export function generateFileParams(
ACL: "public-read",
Bucket: process.env.AWS_S3_BUCKET,
Key: key,
Body: file.buffer || JSON.stringify(file),
Body: file.buffer,
ContentDisposition: `inline;filename="${filename}"`,
ContentType: file.mimetype || "application/json",
ContentType: file.mimetype,
};

return {
Expand Down
16 changes: 9 additions & 7 deletions api.planx.uk/modules/send/s3/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import { markSessionAsSubmitted } from "../../saveAndReturn/service/utils.js";
import { isApplicationTypeSupported } from "../utils/helpers.js";
import type { SendIntegrationController } from "../types.js";

// TS fails to validate this complex type when awaited
// Represent as a simple object to allow enough typechecking to kick in
type DigitalPlanningPayload = Record<string, unknown>;

interface CreateS3Application {
insertS3Application: {
id: string;
Expand Down Expand Up @@ -45,15 +49,13 @@ const sendToS3: SendIntegrationController = async (_req, res, next) => {

// Generate the ODP Schema JSON, skipping validation if not a supported application type
const doValidation = isApplicationTypeSupported(passport);
const exportData = doValidation
? await $api.export.digitalPlanningDataPayload(sessionId)
: await $api.export.digitalPlanningDataPayload(sessionId, true);
const exportData: DigitalPlanningPayload =
await $api.export.digitalPlanningDataPayload(sessionId, doValidation);

const buffer = Buffer.from(JSON.stringify(exportData));

// Create and upload the data as an S3 file
const { fileUrl } = await uploadPrivateFile(
exportData,
`${sessionId}.json`,
);
const { fileUrl } = await uploadPrivateFile(buffer, `${sessionId}.json`);

// Send a notification with the file URL to the Power Automate webhook
const webhookRequest: AxiosRequestConfig = {
Expand Down

0 comments on commit 96b5b8e

Please sign in to comment.