From cf4129fa4e3327e16b1a50a6bdecb96e1500176f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 3 Jun 2026 14:11:45 -0700 Subject: [PATCH 1/3] 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). --- apps/sim/lib/uploads/providers/s3/client.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/uploads/providers/s3/client.ts b/apps/sim/lib/uploads/providers/s3/client.ts index 42b0c62fac..18442b9ebc 100644 --- a/apps/sim/lib/uploads/providers/s3/client.ts +++ b/apps/sim/lib/uploads/providers/s3/client.ts @@ -394,18 +394,22 @@ 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(encodeURIComponent).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}` } /** From 16fc7339fbd7e71c2905a56be5c66c1b04c068ca Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 3 Jun 2026 14:16:59 -0700 Subject: [PATCH 2/3] refactor(storage): clearer per-segment key encoding in fallback URL --- apps/sim/lib/uploads/providers/s3/client.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/uploads/providers/s3/client.ts b/apps/sim/lib/uploads/providers/s3/client.ts index 18442b9ebc..7ddae7bbf8 100644 --- a/apps/sim/lib/uploads/providers/s3/client.ts +++ b/apps/sim/lib/uploads/providers/s3/client.ts @@ -399,7 +399,10 @@ export async function getS3MultipartPartUrls( * 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(encodeURIComponent).join('/') + const encodedKey = key + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') if (S3_CONFIG.endpoint) { const base = S3_CONFIG.endpoint.replace(/\/+$/, '') if (S3_CONFIG.forcePathStyle) { From 33836350f7cd1725cfabe4a1d526bd2fe80c9555 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 3 Jun 2026 14:40:02 -0700 Subject: [PATCH 3/3] test(storage): cover multipart fallback URL (AWS, R2 virtual-hosted, MinIO path-style, key encoding) --- .../lib/uploads/providers/s3/client.test.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/apps/sim/lib/uploads/providers/s3/client.test.ts b/apps/sim/lib/uploads/providers/s3/client.test.ts index 48c017d2cb..75eea9a3dd 100644 --- a/apps/sim/lib/uploads/providers/s3/client.test.ts +++ b/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' + ) + }) + }) })