From 81ea82143f0f7950d288499e53cb175fb704f52d Mon Sep 17 00:00:00 2001 From: Otavio Jacobi Date: Sun, 4 May 2025 17:22:38 -0300 Subject: [PATCH 1/2] Stop relying on customer fillable content to create s3 keys Using the filename on the s3 key is not a good idea as these can contain lots of weird characters and open ways for security breaches. The UUID should be enough to guarantee uniqueness on the keys and the fieldName should be enough to at least point to a direction on what this resource is. This is also not breaking as the href field of old webresources should still work fine but the new ones shall not use possibly skewed data. Change-type: patch --- lib/index.ts | 8 ++++---- test/index.test.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index 7299be5..247329b 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -101,7 +101,7 @@ export class S3Handler implements webResourceHandler.WebResourceHandler { resource: webResourceHandler.IncomingFile, ): Promise { let size = 0; - const key = this.getFileKey(resource.fieldname, resource.originalname); + const key = this.getFileKey(resource.fieldname); const params: PutObjectCommandInput = { Bucket: this.bucket, Key: key, @@ -154,7 +154,7 @@ export class S3Handler implements webResourceHandler.WebResourceHandler { fieldName: string, payload: BeginMultipartUploadPayload, ): Promise { - const fileKey = this.getFileKey(fieldName, payload.filename); + const fileKey = this.getFileKey(fieldName); const createMultiPartResponse = await this.client.send( new CreateMultipartUploadCommand({ @@ -225,8 +225,8 @@ export class S3Handler implements webResourceHandler.WebResourceHandler { return hrefWithoutParams.substring(hrefWithoutParams.lastIndexOf('/') + 1); } - private getFileKey(fieldName: string, fileName: string) { - return `${fieldName}_${randomUUID()}_${fileName}`; + private getFileKey(fieldName: string) { + return `${fieldName}_${randomUUID()}`; } private async getUploadParts( diff --git a/test/index.test.ts b/test/index.test.ts index 80659f7..99398b7 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -182,7 +182,7 @@ describe('S3Handler', function () { 'text/plain', ); expect(s3Mock.calls()[0].firstArg.input.Key).to.match( - new RegExp(`test_[0-9a-f-]+_${payload.filename}`), + new RegExp(`test_[0-9a-f-]+`), ); }; } @@ -270,7 +270,7 @@ describe('S3Handler', function () { 'text/plain', ); expect(s3Mock.calls()[0].firstArg.input.Key).to.match( - new RegExp(`test_[0-9a-f-]+_${payload.filename}`), + new RegExp(`test_[0-9a-f-]+`), ); }); }); From 63142d6dff760bb034bc162d4608c45541f7849a Mon Sep 17 00:00:00 2001 From: Otavio Jacobi Date: Mon, 5 May 2025 16:33:21 -0300 Subject: [PATCH 2/2] Forward original name on content-disposition header so it downloads with original name Change-type: patch --- lib/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/index.ts b/lib/index.ts index 247329b..8259098 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -107,6 +107,7 @@ export class S3Handler implements webResourceHandler.WebResourceHandler { Key: key, Body: resource.stream, ContentType: resource.mimetype, + ContentDisposition: `inline; filename=${resource.originalname}`, }; const upload = new Upload({ client: this.client, params });