@uppy/companion: fix crash on missing filename (#6010)

Better to reject early than generating a filename when missing which
could break assumptions we don't foresee.

Author: Murderlon

.changeset/forty-buses-float.md

@@ -0,0 +1,5 @@
+---
+"@uppy/companion": patch
+---
+
+Reject request early instead of crashing on missing `filename` for /s3/multipart and /s3/params endpoints

packages/@uppy/companion/src/server/controllers/s3.js

@@ -58,6 +58,15 @@ export default function s3(config) {
 
     const { metadata = {}, filename } = req.query
 
+    // Validate filename is provided and non-empty
+    if (typeof filename !== 'string' || filename === '') {
+      res.status(400).json({
+        error:
+          's3: the "filename" query parameter is required and must be a non-empty string',
+      })
+      return
+    }
+
     const truncatedFilename = truncateFilename(
       filename,
       req.companion.options.maxFilenameLength,
@@ -126,6 +135,15 @@ export default function s3(config) {
 
     const { type, metadata = {}, filename } = req.body
 
+    // Validate filename is provided and non-empty
+    if (typeof filename !== 'string' || filename === '') {
+      res.status(400).json({
+        error:
+          's3: the "filename" field is required and must be a non-empty string',
+      })
+      return
+    }
+
     const truncatedFilename = truncateFilename(
       filename,
       req.companion.options.maxFilenameLength,

packages/@uppy/companion/test/companion.test.js

@@ -304,3 +304,118 @@ describe('respects allowLocalUrls, valid hostname that resolves to localhost', (
     expect(res.body).toEqual({ message: 'failed to fetch URL' })
   })
 })
+
+describe('S3 controller', () => {
+  test('createMultipartUpload rejects missing filename', async () => {
+    const server = await getServer({
+      COMPANION_AWS_KEY: 'test_key',
+      COMPANION_AWS_SECRET: 'test_secret',
+      COMPANION_AWS_BUCKET: 'test-bucket',
+      COMPANION_AWS_REGION: 'us-east-1',
+    })
+
+    return request(server)
+      .post('/s3/multipart')
+      .send({
+        type: 'image/png',
+        metadata: {},
+        // filename is intentionally missing
+      })
+      .expect(400)
+      .then((res) =>
+        expect(res.body.error).toBe(
+          's3: the "filename" field is required and must be a non-empty string',
+        ),
+      )
+  })
+
+  test('createMultipartUpload rejects empty filename', async () => {
+    const server = await getServer({
+      COMPANION_AWS_KEY: 'test_key',
+      COMPANION_AWS_SECRET: 'test_secret',
+      COMPANION_AWS_BUCKET: 'test-bucket',
+      COMPANION_AWS_REGION: 'us-east-1',
+    })
+
+    return request(server)
+      .post('/s3/multipart')
+      .send({
+        type: 'image/png',
+        metadata: {},
+        filename: '',
+      })
+      .expect(400)
+      .then((res) =>
+        expect(res.body.error).toBe(
+          's3: the "filename" field is required and must be a non-empty string',
+        ),
+      )
+  })
+
+  test('createMultipartUpload rejects non-string filename', async () => {
+    const server = await getServer({
+      COMPANION_AWS_KEY: 'test_key',
+      COMPANION_AWS_SECRET: 'test_secret',
+      COMPANION_AWS_BUCKET: 'test-bucket',
+      COMPANION_AWS_REGION: 'us-east-1',
+    })
+
+    return request(server)
+      .post('/s3/multipart')
+      .send({
+        type: 'image/png',
+        metadata: {},
+        filename: 12345,
+      })
+      .expect(400)
+      .then((res) =>
+        expect(res.body.error).toBe(
+          's3: the "filename" field is required and must be a non-empty string',
+        ),
+      )
+  })
+
+  test('getUploadParameters rejects missing filename', async () => {
+    const server = await getServer({
+      COMPANION_AWS_KEY: 'test_key',
+      COMPANION_AWS_SECRET: 'test_secret',
+      COMPANION_AWS_BUCKET: 'test-bucket',
+      COMPANION_AWS_REGION: 'us-east-1',
+    })
+
+    return request(server)
+      .get('/s3/params')
+      .query({
+        type: 'image/png',
+        // filename is intentionally missing
+      })
+      .expect(400)
+      .then((res) =>
+        expect(res.body.error).toBe(
+          's3: the "filename" query parameter is required and must be a non-empty string',
+        ),
+      )
+  })
+
+  test('getUploadParameters rejects empty filename', async () => {
+    const server = await getServer({
+      COMPANION_AWS_KEY: 'test_key',
+      COMPANION_AWS_SECRET: 'test_secret',
+      COMPANION_AWS_BUCKET: 'test-bucket',
+      COMPANION_AWS_REGION: 'us-east-1',
+    })
+
+    return request(server)
+      .get('/s3/params')
+      .query({
+        type: 'image/png',
+        filename: '',
+      })
+      .expect(400)
+      .then((res) =>
+        expect(res.body.error).toBe(
+          's3: the "filename" query parameter is required and must be a non-empty string',
+        ),
+      )
+  })
+})