fix(storage): percent-encode object key in multipart fallback URL (#4872)

* fix(storage): percent-encode object key in multipart fallback URL

buildObjectFallbackUrl built the object URL from a raw key. Keys with spaces
or reserved characters (and the pre-existing AWS branch) would produce a
structurally invalid location. Encode the key per path segment (preserving
'/' separators) across all branches (AWS, custom path-style, virtual-hosted).

* refactor(storage): clearer per-segment key encoding in fallback URL

* test(storage): cover multipart fallback URL (AWS, R2 virtual-hosted, MinIO path-style, key encoding)

Author: waleedlatif1

apps/sim/lib/uploads/providers/s3/client.test.ts

@@ -12,6 +12,7 @@ const {
   mockPutObjectCommand,
   mockGetObjectCommand,
   mockDeleteObjectCommand,
+  mockCompleteMultipartUploadCommand,
   mockGetSignedUrl,
   mockEnv,
   mockS3Config,
@@ -51,6 +52,7 @@ const {
     mockPutObjectCommand: vi.fn().mockImplementation(class {}),
     mockGetObjectCommand: vi.fn().mockImplementation(class {}),
     mockDeleteObjectCommand: vi.fn().mockImplementation(class {}),
+    mockCompleteMultipartUploadCommand: vi.fn().mockImplementation(class {}),
     mockGetSignedUrl: vi.fn(),
     mockEnv,
   }
@@ -61,6 +63,7 @@ vi.mock('@aws-sdk/client-s3', () => ({
   PutObjectCommand: mockPutObjectCommand,
   GetObjectCommand: mockGetObjectCommand,
   DeleteObjectCommand: mockDeleteObjectCommand,
+  CompleteMultipartUploadCommand: mockCompleteMultipartUploadCommand,
 }))
 
 vi.mock('@aws-sdk/s3-request-presigner', () => ({
@@ -92,6 +95,7 @@ vi.mock('@/lib/uploads/config', () => ({
 }))
 
 import {
+  completeS3MultipartUpload,
   deleteFromS3,
   downloadFromS3,
   getPresignedUrl,
@@ -398,4 +402,70 @@ describe('S3 Client', () => {
       })
     })
   })
+
+  describe('completeS3MultipartUpload fallback location', () => {
+    const parts = [{ ETag: 'etag-1', PartNumber: 1 }]
+
+    it('uses the SDK-provided Location when present', async () => {
+      mockSend.mockResolvedValueOnce({ Location: 'https://provided.example.com/object' })
+
+      const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe('https://provided.example.com/object')
+      expect(result.key).toBe('kb/uuid-file.txt')
+      expect(result.path).toBe('/api/files/serve/kb%2Fuuid-file.txt')
+    })
+
+    it('falls back to an AWS virtual-hosted URL when Location is absent', async () => {
+      mockSend.mockResolvedValueOnce({})
+
+      const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe(
+        'https://test-kb-bucket.s3.test-region.amazonaws.com/kb/uuid-file.txt'
+      )
+    })
+
+    it('builds a path-style fallback URL for a custom endpoint with forcePathStyle', async () => {
+      mockS3Config.endpoint = 'https://minio.example.com'
+      mockS3Config.forcePathStyle = true
+      mockSend.mockResolvedValueOnce({})
+
+      const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe('https://minio.example.com/test-kb-bucket/kb/uuid-file.txt')
+    })
+
+    it('builds a virtual-hosted fallback URL for a custom endpoint without forcePathStyle', async () => {
+      mockS3Config.endpoint = 'https://account.r2.cloudflarestorage.com'
+      mockS3Config.forcePathStyle = false
+      mockSend.mockResolvedValueOnce({})
+
+      const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe(
+        'https://test-kb-bucket.account.r2.cloudflarestorage.com/kb/uuid-file.txt'
+      )
+    })
+
+    it('strips a trailing slash from the custom endpoint before appending the key', async () => {
+      mockS3Config.endpoint = 'https://minio.example.com/'
+      mockS3Config.forcePathStyle = true
+      mockSend.mockResolvedValueOnce({})
+
+      const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe('https://minio.example.com/test-kb-bucket/kb/uuid-file.txt')
+    })
+
+    it('percent-encodes special characters per path segment, preserving slashes', async () => {
+      mockSend.mockResolvedValueOnce({})
+
+      const result = await completeS3MultipartUpload('kb/uuid-my file.txt', 'upload-1', parts)
+
+      expect(result.location).toBe(
+        'https://test-kb-bucket.s3.test-region.amazonaws.com/kb/uuid-my%20file.txt'
+      )
+    })
+  })
 })

apps/sim/lib/uploads/providers/s3/client.ts

@@ -394,18 +394,25 @@ export async function getS3MultipartPartUrls(
  * addressing mode — path-style for MinIO/Ceph (`forcePathStyle`), virtual-hosted
  * (bucket as a subdomain) for R2 and friends. Falls back to the AWS
  * virtual-hosted host when no custom endpoint is set.
+ *
+ * The key is percent-encoded per path segment (preserving `/` separators) so
+ * keys containing spaces or reserved characters still yield a valid URL.
  */
 function buildObjectFallbackUrl(bucket: string, region: string, key: string): string {
+  const encodedKey = key
+    .split('/')
+    .map((segment) => encodeURIComponent(segment))
+    .join('/')
   if (S3_CONFIG.endpoint) {
     const base = S3_CONFIG.endpoint.replace(/\/+$/, '')
     if (S3_CONFIG.forcePathStyle) {
-      return `${base}/${bucket}/${key}`
+      return `${base}/${bucket}/${encodedKey}`
     }
     const url = new URL(base)
     url.hostname = `${bucket}.${url.hostname}`
-    return `${url.origin}/${key}`
+    return `${url.origin}/${encodedKey}`
   }
-  return `https://${bucket}.s3.${region}.amazonaws.com/${key}`
+  return `https://${bucket}.s3.${region}.amazonaws.com/${encodedKey}`
 }
 
 /**