fix(uploads): sign S3-compatible upload metadata as headers and allow S3_ENDPOINT in CSP (#8449)

* fix(uploads): sign S3-compatible upload metadata as headers and allow S3_ENDPOINT in CSP

* test(csp): pin S3_FORCE_PATH_STYLE unset in the CSP test env

Author: waleedlatif1

apps/docs/content/docs/platform/self-hosting/object-storage.mdx

@@ -416,7 +416,8 @@ Sim works with any S3-compatible store by pointing the S3 client at a custom end
   **The endpoint must be reachable from your users' browsers, and the bucket needs CORS.** Uploads use presigned `PUT` requests sent **directly from the browser** to `S3_ENDPOINT` (downloads are proxied back through the app, so they only need server-side reachability). This means:
 
   - A purely internal endpoint (e.g. `https://minio.internal:9000` that only the app pods can resolve) will let the server start cleanly but **uploads will fail in the browser**. Use an endpoint your users can reach.
-  - Configure a **CORS policy** on the bucket that allows your Sim origin (`PUT`, `GET`, and the `Authorization` / `Content-Type` / `x-amz-*` headers). This applies to AWS S3 too — R2 and MinIO are no different.
+  - Configure a **CORS policy** on the bucket that allows your Sim origin with `PUT` and `GET`, and `AllowedHeaders: ["*"]`. With a custom endpoint, the browser sends the upload's metadata as signed `x-amz-meta-*` headers alongside `Content-Type` and `If-None-Match`, because many S3-compatible stores ignore metadata passed in the URL. If you list headers individually, include all of them. MinIO applies its own server-wide CORS and needs no bucket rule.
+  - Sim adds the `S3_ENDPOINT` origin (and its bucket subdomains, unless `S3_FORCE_PATH_STYLE` is set) to its Content Security Policy, so the browser may upload there.
 </Callout>
 
 <Tabs items={['Cloudflare R2', 'MinIO', 'RustFS']}>

apps/sim/lib/core/security/csp.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, vi } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 
 await vi.hoisted(async () => {
   const { setEnv } = await import('@sim/testing/mocks/env.mock')
@@ -14,9 +14,12 @@ await vi.hoisted(async () => {
     NEXT_PUBLIC_BRAND_FAVICON_URL: 'https://brand.example.com/favicon.ico',
     NEXT_PUBLIC_PRIVACY_URL: 'https://legal.example.com/privacy',
     NEXT_PUBLIC_TERMS_URL: 'https://legal.example.com/terms',
+    S3_ENDPOINT: 'https://s3.de.io.cloud.ovh.net',
+    S3_FORCE_PATH_STYLE: undefined,
   })
 })
 
+import { setEnv } from '@sim/testing/mocks/env.mock'
 import { buildCSPString, generateRuntimeCSP, getChatEmbedCSPPolicy, getMainCSPPolicy } from './csp'
 
 describe('buildCSPString', () => {
@@ -32,7 +35,18 @@ describe('buildCSPString', () => {
   })
 })
 
+function connectSources(policy: string): string[] {
+  const directive = policy.split('; ').find((d) => d.startsWith('connect-src ')) ?? ''
+  return directive.split(' ').slice(1)
+}
+
 describe('getMainCSPPolicy', () => {
+  it('allows direct uploads to the build-time S3_ENDPOINT', () => {
+    const sources = connectSources(getMainCSPPolicy())
+    expect(sources).toContain('https://s3.de.io.cloud.ovh.net')
+    expect(sources).toContain('https://*.s3.de.io.cloud.ovh.net')
+  })
+
   it('keeps the restrictive security directives', () => {
     const policy = getMainCSPPolicy()
 
@@ -63,6 +77,40 @@ describe('generateRuntimeCSP', () => {
   })
 })
 
+describe('generateRuntimeCSP S3_ENDPOINT sources', () => {
+  afterEach(() => {
+    setEnv({ S3_ENDPOINT: 'https://s3.de.io.cloud.ovh.net', S3_FORCE_PATH_STYLE: undefined })
+  })
+
+  it('allows the endpoint and its bucket subdomains for virtual-hosted addressing', () => {
+    setEnv({ S3_ENDPOINT: 'https://acct.r2.cloudflarestorage.com/' })
+    const sources = connectSources(generateRuntimeCSP())
+    expect(sources).toContain('https://acct.r2.cloudflarestorage.com')
+    expect(sources).toContain('https://*.acct.r2.cloudflarestorage.com')
+  })
+
+  it('keeps a non-default port and drops the bucket wildcard under S3_FORCE_PATH_STYLE', () => {
+    setEnv({ S3_ENDPOINT: 'https://minio.example.com:9000', S3_FORCE_PATH_STYLE: 'true' })
+    const sources = connectSources(generateRuntimeCSP())
+    expect(sources).toContain('https://minio.example.com:9000')
+    expect(sources.some((s) => s.includes('*.minio.example.com'))).toBe(false)
+  })
+
+  it('does not build a wildcard over an IP endpoint, which is always path-style', () => {
+    setEnv({ S3_ENDPOINT: 'http://10.0.0.5:9000' })
+    const sources = connectSources(generateRuntimeCSP())
+    expect(sources).toContain('http://10.0.0.5:9000')
+    expect(sources.some((s) => s.includes('*.10.0.0.5'))).toBe(false)
+  })
+
+  it('ignores an endpoint without an http(s) scheme instead of emitting a broken source', () => {
+    setEnv({ S3_ENDPOINT: 'minio.example.com:9000' })
+    const csp = generateRuntimeCSP()
+    expect(csp).not.toContain('minio.example.com')
+    expect(connectSources(csp)).toContain("'self'")
+  })
+})
+
 describe('getChatEmbedCSPPolicy', () => {
   it('allows embedding and Office.js without relaxing object-src or base-uri', () => {
     const policy = getChatEmbedCSPPolicy()

apps/sim/lib/core/security/csp.ts

@@ -1,5 +1,5 @@
 import { CONSENT_BACKEND_URL } from '../../consent/constants'
-import { env, getEnv } from '../config/env'
+import { env, envBoolean, getEnv } from '../config/env'
 import { isDev, isHosted, isReactGrabEnabled } from '../config/env-flags'
 
 /**
@@ -44,6 +44,34 @@ function getHostnameFromUrl(url: string | undefined): string[] {
   }
 }
 
+const IPV4_HOSTNAME = /^\d{1,3}(\.\d{1,3}){3}$/
+
+/**
+ * Origins the browser PUTs presigned uploads to for a custom `S3_ENDPOINT`. The
+ * endpoint origin itself is always allowed: the S3 SDK falls back to path-style
+ * for IP hosts and bucket names that aren't DNS-safe even without
+ * `S3_FORCE_PATH_STYLE`. Virtual-hosted addressing also needs the bucket
+ * subdomains, which a `*.` source matches (never the bare host). Ports are kept
+ * because a host-source without one only matches the scheme's default port.
+ */
+function getS3EndpointSources(
+  endpoint: string | undefined,
+  forcePathStyle: string | undefined
+): string[] {
+  if (!endpoint) return []
+  let url: URL
+  try {
+    url = new URL(endpoint)
+  } catch {
+    return []
+  }
+  if (url.protocol !== 'https:' && url.protocol !== 'http:') return []
+  const origin = `${url.protocol}//${url.host}`
+  const isIpHost = IPV4_HOSTNAME.test(url.hostname) || url.hostname.startsWith('[')
+  if (envBoolean(forcePathStyle) || isIpHost) return [origin]
+  return [origin, `${url.protocol}//*.${url.host}`]
+}
+
 export interface CSPDirectives {
   'default-src'?: string[]
   'script-src'?: string[]
@@ -199,6 +227,7 @@ export const buildTimeCSPDirectives: CSPDirectives = {
     ...getHostnameFromUrl(env.NEXT_PUBLIC_BRAND_LOGO_URL),
     ...getHostnameFromUrl(env.NEXT_PUBLIC_PRIVACY_URL),
     ...getHostnameFromUrl(env.NEXT_PUBLIC_TERMS_URL),
+    ...getS3EndpointSources(env.S3_ENDPOINT, env.S3_FORCE_PATH_STYLE),
   ],
 
   'frame-src': [...STATIC_FRAME_SRC],
@@ -247,6 +276,10 @@ export function generateRuntimeCSP(): string {
   const brandLogoDomains = getHostnameFromUrl(getEnv('NEXT_PUBLIC_BRAND_LOGO_URL'))
   const privacyDomains = getHostnameFromUrl(getEnv('NEXT_PUBLIC_PRIVACY_URL'))
   const termsDomains = getHostnameFromUrl(getEnv('NEXT_PUBLIC_TERMS_URL'))
+  const s3EndpointSources = getS3EndpointSources(
+    getEnv('S3_ENDPOINT'),
+    getEnv('S3_FORCE_PATH_STYLE')
+  )
 
   const runtimeDirectives: CSPDirectives = {
     ...buildTimeCSPDirectives,
@@ -262,6 +295,7 @@ export function generateRuntimeCSP(): string {
       ...brandLogoDomains,
       ...privacyDomains,
       ...termsDomains,
+      ...s3EndpointSources,
     ],
   }
 

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

@@ -74,7 +74,6 @@ import {
   deleteFromS3,
   deleteS3ObjectVersion,
   downloadFromS3,
-  getS3PresignedUploadUrl,
   headS3Object,
   listS3MultipartParts,
   resetS3ClientForTesting,
@@ -225,35 +224,6 @@ describe('S3 Client', () => {
   })
 
   describe('direct upload primitives', () => {
-    it('signs metadata and a create-only condition without duplicate x-amz-meta headers', async () => {
-      mockGetSignedUrl.mockResolvedValueOnce('https://example.com/signed-put')
-
-      const result = await getS3PresignedUploadUrl({
-        key: 'workspace/workspace-1/file.bin',
-        contentType: 'application/octet-stream',
-        fileSize: 3,
-        metadata: { uploadId: 'upload-1', purpose: 'workspace_file' },
-        customConfig: mockS3Config,
-        expiresIn: 600,
-      })
-
-      expect(mockPutObjectCommand).toHaveBeenCalledWith({
-        Bucket: 'test-bucket',
-        Key: 'workspace/workspace-1/file.bin',
-        ContentType: 'application/octet-stream',
-        ContentLength: 3,
-        IfNoneMatch: '*',
-        Metadata: { uploadId: 'upload-1', purpose: 'workspace_file' },
-      })
-      expect(result).toEqual({
-        url: 'https://example.com/signed-put',
-        headers: {
-          'Content-Type': 'application/octet-stream',
-          'If-None-Match': '*',
-        },
-      })
-    })
-
     it('lists every provider part across pagination', async () => {
       mockSend
         .mockResolvedValueOnce({

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

@@ -182,9 +182,13 @@ export async function getPresignedUrlWithConfig(
 
 /**
  * Generates a create-only signed single-object PUT for a caller-selected final key.
- * The AWS presigner hoists `x-amz-meta-*` values into the signed query string,
- * so only ordinary transfer headers are returned. Repeating that metadata as
- * request headers makes S3 reject the otherwise-valid signature.
+ *
+ * By default the AWS presigner hoists `x-amz-meta-*` into the signed query string.
+ * AWS S3 stores that as object metadata, but many S3-compatible stores (e.g.
+ * OVHcloud) ignore it, so with a custom `S3_CONFIG.endpoint` the metadata is
+ * signed as headers instead and returned for the uploader to send verbatim. AWS
+ * keeps the query-string form so existing bucket CORS rules stay valid. A value
+ * must never be both hoisted and sent as a header: S3 rejects the unsigned copy.
  */
 export async function getS3PresignedUploadUrl(params: {
   key: string
@@ -203,12 +207,21 @@ export async function getS3PresignedUploadUrl(params: {
     IfNoneMatch: '*',
     Metadata: metadata,
   })
-  const url = await getSignedUrl(getS3Client(), command, { expiresIn: params.expiresIn })
+  const metadataHeaders: Record<string, string> = S3_CONFIG.endpoint
+    ? Object.fromEntries(
+        Object.entries(metadata).map(([key, value]) => [`x-amz-meta-${key.toLowerCase()}`, value])
+      )
+    : {}
+  const url = await getSignedUrl(getS3Client(), command, {
+    expiresIn: params.expiresIn,
+    unhoistableHeaders: new Set(Object.keys(metadataHeaders)),
+  })
   return {
     url,
     headers: {
       'Content-Type': params.contentType,
       'If-None-Match': '*',
+      ...metadataHeaders,
     },
   }
 }

apps/sim/lib/uploads/providers/s3/presigned-upload.test.ts

@@ -0,0 +1,82 @@
+/**
+ * Runs the real AWS SigV4 presigner (signing is local, no network) to pin where
+ * direct-upload metadata travels. A header the uploader must send but that is
+ * missing from `headers`, or a metadata key both signed as a header and hoisted
+ * into the query, makes the provider reject or strip the upload.
+ */
+import { resetEnvMock, setEnv } from '@sim/testing/mocks/env.mock'
+import { setUploadsConfig, uploadsConfigMock } from '@sim/testing/mocks/uploads-config.mock'
+import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
+
+import { getS3PresignedUploadUrl, resetS3ClientForTesting } from '@/lib/uploads/providers/s3/client'
+
+setEnv({ AWS_ACCESS_KEY_ID: 'test-access-key', AWS_SECRET_ACCESS_KEY: 'test-secret-key' })
+afterAll(resetEnvMock)
+
+/** Headers the browser sets itself and the uploader never supplies. */
+const TRANSPORT_HEADERS = new Set(['host', 'content-length'])
+
+function configure(endpoint: string | undefined, forcePathStyle = false) {
+  setUploadsConfig({
+    S3_CONFIG: { bucket: 'sim-files', region: 'de', endpoint, forcePathStyle },
+  })
+  resetS3ClientForTesting()
+}
+
+async function presign() {
+  const transfer = await getS3PresignedUploadUrl({
+    key: 'kb/upload-1/report.pdf',
+    contentType: 'application/pdf',
+    fileSize: 3,
+    metadata: { uploadId: 'upload-1', originalName: 'Q3 report.pdf' },
+    customConfig: { bucket: 'sim-files', region: 'de' },
+    expiresIn: 600,
+  })
+  const url = new URL(transfer.url)
+  const signedHeaders = (url.searchParams.get('X-Amz-SignedHeaders') ?? '').split(';')
+  const queryMetadata = [...url.searchParams.keys()].filter((k) =>
+    k.toLowerCase().startsWith('x-amz-meta-')
+  )
+  const suppliedHeaders = new Set(Object.keys(transfer.headers).map((k) => k.toLowerCase()))
+  return { transfer, signedHeaders, queryMetadata, suppliedHeaders }
+}
+
+describe('getS3PresignedUploadUrl', () => {
+  beforeEach(() => configure(undefined))
+
+  it('signs metadata as uploader-sent headers for a custom S3-compatible endpoint', async () => {
+    configure('https://s3.de.io.cloud.ovh.net')
+    const { transfer, signedHeaders, queryMetadata } = await presign()
+
+    expect(queryMetadata).toEqual([])
+    expect(signedHeaders).toEqual(
+      expect.arrayContaining(['x-amz-meta-uploadid', 'x-amz-meta-originalname'])
+    )
+    expect(transfer.headers).toMatchObject({
+      'x-amz-meta-uploadid': 'upload-1',
+      'x-amz-meta-originalname': 'Q3 report.pdf',
+    })
+  })
+
+  it('keeps AWS metadata in the signed query so existing bucket CORS rules still apply', async () => {
+    const { signedHeaders, queryMetadata } = await presign()
+
+    expect(queryMetadata.sort()).toEqual(['x-amz-meta-originalname', 'x-amz-meta-uploadid'])
+    expect(signedHeaders.some((h) => h.startsWith('x-amz-meta-'))).toBe(false)
+  })
+
+  it.each([
+    ['AWS', undefined],
+    ['custom endpoint', 'https://s3.de.io.cloud.ovh.net'],
+  ])('supplies every signed header the uploader controls (%s)', async (_, endpoint) => {
+    configure(endpoint)
+    const { signedHeaders, suppliedHeaders, queryMetadata } = await presign()
+
+    for (const header of signedHeaders) {
+      if (!TRANSPORT_HEADERS.has(header)) expect(suppliedHeaders).toContain(header)
+    }
+    for (const key of queryMetadata) expect(suppliedHeaders).not.toContain(key.toLowerCase())
+  })
+})